Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-03-02 Thread Bruce Momjian
On Thu, Feb 19, 2015 at 12:29:18PM +0900, Fujii Masao wrote:
  The pg_audit doesn't log BIND parameter values when prepared statement is 
  used.
  Seems this is an oversight of the patch. Or is this intentional?
 
  It's actually intentional - following the model I talked about in my
  earlier emails, the idea is to log statements only.
 
 Is this acceptable for audit purpose in many cases? Without the values,
 I'm afraid that it's hard to analyze what table records are affected by
 the statements from the audit logs. I was thinking that identifying the
 data affected is one of important thing for the audit. If I'm malicious DBA,
 I will always use the extended protocol to prevent the values from being
 audited when I execute the statement.

I added protocol-level bind() value logging for log_statement, and there
were many requests for this functionality before I implemented it.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-26 Thread David Steele
On 2/25/15 11:40 PM, Alvaro Herrera wrote:
 Fujii Masao wrote:
 On Tue, Feb 24, 2015 at 1:29 AM, David Steele da...@pgmasters.net wrote:
 
 1) Follow Oracle's as session option and only log each statement type
 against an object the first time it happens in a session.  This would
 greatly reduce logging, but might be too little detail.  It would
 increase the memory footprint of the module to add the tracking.
 
 Doesn't this mean that a user could conceal malicious activity simply by
 doing a innocuous query that silences all further activity?

True enough, but I thought it might be useful as an option.  I tend to
lock down users pretty tightly so there's not much they can do without
somehow getting superuser access, at which point all bets are off anyway.

In the case where users are highly constrained, the idea here would be
to just keeps tabs on the sort of things they are reading/writing for
financial audits and ISO compliance.

 2) Only log once per call to the backend.  Essentially, we would only
 log the statement you see in pg_stat_activity.  This could be a good
 option because it logs what the user accesses directly, rather than
 functions, views, etc. which hopefully are already going through a
 review process and can be audited that way.
 
 ... assuming the user does not create stuff on the fly behind your back.

Sure, but then the thing they created/modified would also be logged
somewhere earlier (assuming pg_audit.log = 'all').  It would require
some analysis to figure out what they did but I think that would be true
in many cases.

 Would either of those address your concerns?

 Before discussing how to implement, probably we need to consider the
 spec about this. For example, should we log even nested statements for
 the audit purpose? If yes, how should we treat the case where
 the user changes the setting so that only DDL is logged, and then
 the user-defined function which internally executes DDL is called?
 Since the top-level SQL (calling the function) is not the target of
 audit, we should not log even the nested DDL?
 
 Clearly if you log only DROP TABLE, and then the malicious user hides
 one such call inside a function that executes the drop (which is called
 via a SELECT top-level SQL), you're not going to be happy.

If the purpose of the auditing it to catch malicious activity then all
statements would need to be logged.  If only top-level statements are
logged then it might be harder to detect, but it would still be logged.

-- 
- David Steele
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-26 Thread David Steele
On 2/25/15 10:42 PM, Fujii Masao wrote:
 On Tue, Feb 24, 2015 at 1:29 AM, David Steele da...@pgmasters.net wrote:
 On 2/18/15 10:25 AM, David Steele wrote:
 On 2/18/15 6:11 AM, Fujii Masao wrote:
 The pg_audit doesn't log BIND parameter values when prepared statement is 
 used.
 Seems this is an oversight of the patch. Or is this intentional?

 It's actually intentional - following the model I talked about in my
 earlier emails, the idea is to log statements only.  This also follows
 on 2ndQuadrant's implementation.

 Unfortunately, I think it's beyond the scope of this module to log bind
 variables.
 
 Maybe I can live with that at least in the first version.
 
 I'm following not only 2ndQuadrant's implementation, but
 Oracle's as well.
 
 Oracle's audit_trail (e.g., = db, extended) can log even bind values.
 Also log_statement=on in PostgreSQL also can log bind values.
 Maybe we can reuse the same technique that log_statement uses.

I'll look at how this is done in the logging code and see if it can be
used in pg_audit.

 Imagine the case where you call the user-defined function which executes
 many nested statements. In this case, pg_audit logs only top-level 
 statement
 (i.e., issued directly by client) every time nested statement is executed.
 In fact, one call of such UDF can cause lots of *same* log messages. I 
 think
 this is problematic.

 I agree - not sure how to go about addressing it, though.  I've tried to
 cut down on the verbosity of the logging in general, but of course it
 can still be a problem.

 Using security definer and a different logging GUC for the defining role
 might work.  I'll add that to my unit tests and see what happens.

 That didn't work - but I didn't really expect it to.

 Here are two options I thought of:

 1) Follow Oracle's as session option and only log each statement type
 against an object the first time it happens in a session.  This would
 greatly reduce logging, but might be too little detail.  It would
 increase the memory footprint of the module to add the tracking.

 2) Only log once per call to the backend.  Essentially, we would only
 log the statement you see in pg_stat_activity.  This could be a good
 option because it logs what the user accesses directly, rather than
 functions, views, etc. which hopefully are already going through a
 review process and can be audited that way.

 Would either of those address your concerns?
 
 Before discussing how to implement, probably we need to consider the
 spec about this. For example, should we log even nested statements for
 the audit purpose? If yes, how should we treat the case where
 the user changes the setting so that only DDL is logged, and then
 the user-defined function which internally executes DDL is called?
 Since the top-level SQL (calling the function) is not the target of
 audit, we should not log even the nested DDL?

I think logging nested statements should at least be an option.  And
yes, I think that nested statements should be logged even if the
top-level SQL is not (depending on configuration). The main case for
this would be DO blocks which can be run by anybody.

-- 
- David Steele
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-25 Thread Fujii Masao
On Tue, Feb 24, 2015 at 1:29 AM, David Steele da...@pgmasters.net wrote:
 On 2/18/15 10:25 AM, David Steele wrote:
 On 2/18/15 6:11 AM, Fujii Masao wrote:
 The pg_audit doesn't log BIND parameter values when prepared statement is 
 used.
 Seems this is an oversight of the patch. Or is this intentional?

 It's actually intentional - following the model I talked about in my
 earlier emails, the idea is to log statements only.  This also follows
 on 2ndQuadrant's implementation.

 Unfortunately, I think it's beyond the scope of this module to log bind
 variables.

Maybe I can live with that at least in the first version.

 I'm following not only 2ndQuadrant's implementation, but
 Oracle's as well.

Oracle's audit_trail (e.g., = db, extended) can log even bind values.
Also log_statement=on in PostgreSQL also can log bind values.
Maybe we can reuse the same technique that log_statement uses.

 Logging values is interesting, but I'm sure the user would want to
 specify which columns to log, which I felt was beyond the scope of the
 patch.

 The pg_audit cannot log the statement like SELECT 1 which doesn't access 
 to
 the database object. Is this intentional? I think that there are many users 
 who
 want to audit even such statement.

 I think I see how to make this work.  I'll work on it for the next
 version of the patch.

 This has been fixed in the v2 patch.

Thanks!

 Imagine the case where you call the user-defined function which executes
 many nested statements. In this case, pg_audit logs only top-level statement
 (i.e., issued directly by client) every time nested statement is executed.
 In fact, one call of such UDF can cause lots of *same* log messages. I think
 this is problematic.

 I agree - not sure how to go about addressing it, though.  I've tried to
 cut down on the verbosity of the logging in general, but of course it
 can still be a problem.

 Using security definer and a different logging GUC for the defining role
 might work.  I'll add that to my unit tests and see what happens.

 That didn't work - but I didn't really expect it to.

 Here are two options I thought of:

 1) Follow Oracle's as session option and only log each statement type
 against an object the first time it happens in a session.  This would
 greatly reduce logging, but might be too little detail.  It would
 increase the memory footprint of the module to add the tracking.

 2) Only log once per call to the backend.  Essentially, we would only
 log the statement you see in pg_stat_activity.  This could be a good
 option because it logs what the user accesses directly, rather than
 functions, views, etc. which hopefully are already going through a
 review process and can be audited that way.

 Would either of those address your concerns?

Before discussing how to implement, probably we need to consider the
spec about this. For example, should we log even nested statements for
the audit purpose? If yes, how should we treat the case where
the user changes the setting so that only DDL is logged, and then
the user-defined function which internally executes DDL is called?
Since the top-level SQL (calling the function) is not the target of
audit, we should not log even the nested DDL?

Regards,

-- 
Fujii Masao


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-25 Thread Alvaro Herrera
Fujii Masao wrote:
 On Tue, Feb 24, 2015 at 1:29 AM, David Steele da...@pgmasters.net wrote:

  1) Follow Oracle's as session option and only log each statement type
  against an object the first time it happens in a session.  This would
  greatly reduce logging, but might be too little detail.  It would
  increase the memory footprint of the module to add the tracking.

Doesn't this mean that a user could conceal malicious activity simply by
doing a innocuous query that silences all further activity?

  2) Only log once per call to the backend.  Essentially, we would only
  log the statement you see in pg_stat_activity.  This could be a good
  option because it logs what the user accesses directly, rather than
  functions, views, etc. which hopefully are already going through a
  review process and can be audited that way.

... assuming the user does not create stuff on the fly behind your back.

  Would either of those address your concerns?
 
 Before discussing how to implement, probably we need to consider the
 spec about this. For example, should we log even nested statements for
 the audit purpose? If yes, how should we treat the case where
 the user changes the setting so that only DDL is logged, and then
 the user-defined function which internally executes DDL is called?
 Since the top-level SQL (calling the function) is not the target of
 audit, we should not log even the nested DDL?

Clearly if you log only DROP TABLE, and then the malicious user hides
one such call inside a function that executes the drop (which is called
via a SELECT top-level SQL), you're not going to be happy.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-25 Thread Fujii Masao
On Thu, Feb 26, 2015 at 1:40 PM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 Fujii Masao wrote:
 On Tue, Feb 24, 2015 at 1:29 AM, David Steele da...@pgmasters.net wrote:

  1) Follow Oracle's as session option and only log each statement type
  against an object the first time it happens in a session.  This would
  greatly reduce logging, but might be too little detail.  It would
  increase the memory footprint of the module to add the tracking.

 Doesn't this mean that a user could conceal malicious activity simply by
 doing a innocuous query that silences all further activity?

  2) Only log once per call to the backend.  Essentially, we would only
  log the statement you see in pg_stat_activity.  This could be a good
  option because it logs what the user accesses directly, rather than
  functions, views, etc. which hopefully are already going through a
  review process and can be audited that way.

 ... assuming the user does not create stuff on the fly behind your back.

  Would either of those address your concerns?

 Before discussing how to implement, probably we need to consider the
 spec about this. For example, should we log even nested statements for
 the audit purpose? If yes, how should we treat the case where
 the user changes the setting so that only DDL is logged, and then
 the user-defined function which internally executes DDL is called?
 Since the top-level SQL (calling the function) is not the target of
 audit, we should not log even the nested DDL?

 Clearly if you log only DROP TABLE, and then the malicious user hides
 one such call inside a function that executes the drop (which is called
 via a SELECT top-level SQL), you're not going to be happy.

Yep, so what SQL should be logged in this case? Only targeted nested DDL?
Both top and nested ones? Seems the later is better to me.

What about the case where the function A calls the function B executing DDL?
Every ancestor SQLs of the targeted DDL should be logged? Probably yes.

Regards,

-- 
Fujii Masao


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-23 Thread David Steele
On 2/18/15 10:25 AM, David Steele wrote:
 On 2/18/15 6:11 AM, Fujii Masao wrote:
 The pg_audit doesn't log BIND parameter values when prepared statement is 
 used.
 Seems this is an oversight of the patch. Or is this intentional?
 
 It's actually intentional - following the model I talked about in my
 earlier emails, the idea is to log statements only.  This also follows
 on 2ndQuadrant's implementation.

Unfortunately, I think it's beyond the scope of this module to log bind
variables.  I'm following not only 2ndQuadrant's implementation, but
Oracle's as well.

 Logging values is interesting, but I'm sure the user would want to
 specify which columns to log, which I felt was beyond the scope of the
 patch.
 
 The pg_audit cannot log the statement like SELECT 1 which doesn't access to
 the database object. Is this intentional? I think that there are many users 
 who
 want to audit even such statement.
 
 I think I see how to make this work.  I'll work on it for the next
 version of the patch.

This has been fixed in the v2 patch.

 Imagine the case where you call the user-defined function which executes
 many nested statements. In this case, pg_audit logs only top-level statement
 (i.e., issued directly by client) every time nested statement is executed.
 In fact, one call of such UDF can cause lots of *same* log messages. I think
 this is problematic.
 
 I agree - not sure how to go about addressing it, though.  I've tried to
 cut down on the verbosity of the logging in general, but of course it
 can still be a problem.
 
 Using security definer and a different logging GUC for the defining role
 might work.  I'll add that to my unit tests and see what happens.

That didn't work - but I didn't really expect it to.

Here are two options I thought of:

1) Follow Oracle's as session option and only log each statement type
against an object the first time it happens in a session.  This would
greatly reduce logging, but might be too little detail.  It would
increase the memory footprint of the module to add the tracking.

2) Only log once per call to the backend.  Essentially, we would only
log the statement you see in pg_stat_activity.  This could be a good
option because it logs what the user accesses directly, rather than
functions, views, etc. which hopefully are already going through a
review process and can be audited that way.

Would either of those address your concerns?

-- 
- David Steele
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-19 Thread David Steele
On 2/18/15 10:29 PM, Fujii Masao wrote:
 On Thu, Feb 19, 2015 at 12:25 AM, David Steele da...@pgmasters.net wrote:
 The pg_audit doesn't log BIND parameter values when prepared statement is 
 used.
 Seems this is an oversight of the patch. Or is this intentional?

 It's actually intentional - following the model I talked about in my
 earlier emails, the idea is to log statements only.
 
 Is this acceptable for audit purpose in many cases? Without the values,
 I'm afraid that it's hard to analyze what table records are affected by
 the statements from the audit logs. I was thinking that identifying the
 data affected is one of important thing for the audit. If I'm malicious DBA,
 I will always use the extended protocol to prevent the values from being
 audited when I execute the statement.

I agree with you, but I wonder how much is practical at this stage.
Let me think about it and see what I can come up with.

-- 
- David Steele
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-18 Thread Fujii Masao
On Wed, Feb 18, 2015 at 1:26 AM, David Steele da...@pgmasters.net wrote:
 On 2/17/15 10:23 AM, Simon Riggs wrote:
 I vote to include pgaudit in 9.5, albeit with any changes. In
 particular, David may have some changes to recommend, but I haven't
 seen a spec or a patch, just a new version of code (which isn't how we
 do things...).

 I submitted the new patch in my name under a separate thread Auditing
 extension for PostgreSQL (Take 2) (54e005cc.1060...@pgmasters.net)

I played the patch version of pg_audit a bit and have basic comments about
its spec.

The pg_audit doesn't log BIND parameter values when prepared statement is used.
Seems this is an oversight of the patch. Or is this intentional?

The pg_audit cannot log the statement like SELECT 1 which doesn't access to
the database object. Is this intentional? I think that there are many users who
want to audit even such statement.

Imagine the case where you call the user-defined function which executes
many nested statements. In this case, pg_audit logs only top-level statement
(i.e., issued directly by client) every time nested statement is executed.
In fact, one call of such UDF can cause lots of *same* log messages. I think
this is problematic.

Regards,

-- 
Fujii Masao


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-18 Thread Neil Tiffin
Stephen,

I meant it to go to the list, but hit the wrong button.

 On Feb 17, 2015, at 7:01 PM, Stephen Frost sfr...@snowman.net wrote:
 
 Neil,
 
 I noticed that you email'd me directly on this reply.  Not sure if you
 intended to or not, but I'm fine with my response going to the list.
 
 * Neil Tiffin (ne...@neiltiffin.com) wrote:
 On Feb 17, 2015, at 1:10 PM, Stephen Frost sfr...@snowman.net wrote:
 If the DB account isn't a superuser then everything changes.  There's no
 point fighting with the OS user- they can run some other PG binary which
 they've copied over locally and run SQL with that, or copy all the files
 over to another server which they have complete access to.  The fact
 that they can also connect to the DB and run SQL isn’t really an issue.
 
 Thats the point. If this environment matters then the db superuser would not 
 be an authorized os superuser (with root or even close privileges). And no, 
 they could not be running some other binary.
 
 One way to do pg superuser auditing is to utilize a file (outside of the pg 
 data directory, which probably violates something in pg) like 
 /etc/pg_su_audit that has os root rw and r for all others (or the equivalent 
 for other os’s) containing a URL.  If this file is present, send all db 
 superuser usage to the URL.  If this file is present with the wrong 
 privileges, then don’t start pg. Done.  No configuration in pg config files, 
 no GUCs, no nothing for the pg superuser to mess around with, not tables, no 
 ability for any pg superuser to configure or control.  
 
 This approach doesn't violate anything in PG and can be used with any of
 the pgaudit approaches being discussed.  The fact that it's
 postgresql.conf, which represents GUCs, doesn't change anything
 regarding what you’re getting it.
 

It changes everything, pg superusers have complete control of files in the pg 
data directory.  If set up correctly pg superusers have no control in /etc. 

 If they can copy or install a PG binary, then the OS auditing and security 
 failed. PG code need not be concerned.
 
 Sure someone could still break access to the URL, but again, not PG’s 
 concern.  Other security and auditing would have responsibility to pick that 
 up.
 
 It is a really simple use case, record everything any db superuser does and 
 send it to the audit system.  Done.  Then a designated role can control what 
 gets audited in production.  As long as everything the db superuser does can 
 be written to an audit log, then it no longer matters technically if the db 
 superuser can change the rest of the auditing.  If they do and it violates 
 policy, then when the audit picks it up, they lose their job plus depending 
 on the environment. 
 
 The issue is really around what we claim to provide with this auditing.
 We can't claim to provide *full* superuser auditing with any of these
 approaches since the superuser can disable auditing.  We can, however,
 provide auditing up until that happens, which is likely to be sufficient
 in many environments.  For those environments where full superuser
 auditing is required, an independent system must be used.
 
 Of course, it's important that any auditing mechanism which is used to
 audit superusers be impossible for the superuser to modify after the
 fact, meaning that syslog or similar needs to be used.
 

I’m still confused since you do do not differentiate between db superuser and 
os superuser and what you mean by an independent system?

With the scheme I described above, how does the db superuser disable auditing 
without os root privileges?  If they can, then pg security is fundamentally 
broken, and I don’t believe it is.

How can an independent system monitor what commands are issued inside the 
database?

I understand my comments do not cover what is being proposed or worked on and 
that is fine.  But saying it does not have value because the superuser could 
disable any system in pg, is wrong IMO.  Being able to reliability log db 
superusers without their ability to change the logging would be a fundamentally 
good security tool as companies become more serious about internal security.  
This is, and will be more, important since a lot of people consider insider 
breaches the biggest security challenge today.

Neil






-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-18 Thread Stephen Frost
Neil,

* Neil Tiffin (ne...@neiltiffin.com) wrote:
 I meant it to go to the list, but hit the wrong button.

No problem.

  On Feb 17, 2015, at 7:01 PM, Stephen Frost sfr...@snowman.net wrote:
  I noticed that you email'd me directly on this reply.  Not sure if you
  intended to or not, but I'm fine with my response going to the list.
  
  This approach doesn't violate anything in PG and can be used with any of
  the pgaudit approaches being discussed.  The fact that it's
  postgresql.conf, which represents GUCs, doesn't change anything
  regarding what you’re getting it.
  
 
 It changes everything, pg superusers have complete control of files in the pg 
 data directory.  If set up correctly pg superusers have no control in /etc. 

If set up correctly, postgresql.conf is in /etc. :)  That's distribution
specific though.  However, postgresql.auto.conf ends up causing problems
since it can't be disabled and it's forced to live in the PG data
directory.  Even so though, your argument was that, as long as the
system is set up to be auditing initially and whatever action the
superuser takes to disable auditing will be audited, and disabling
auditing is against policy, then it's sufficient to meet the
requirement.  That's true in either case.

  The issue is really around what we claim to provide with this auditing.
  We can't claim to provide *full* superuser auditing with any of these
  approaches since the superuser can disable auditing.  We can, however,
  provide auditing up until that happens, which is likely to be sufficient
  in many environments.  For those environments where full superuser
  auditing is required, an independent system must be used.
  
  Of course, it's important that any auditing mechanism which is used to
  audit superusers be impossible for the superuser to modify after the
  fact, meaning that syslog or similar needs to be used.
 
 I’m still confused since you do do not differentiate between db superuser and 
 os superuser and what you mean by an independent system?

When I'm talking about 'superuser' here, it's the PG superuser, not the
OS superuser.  What I mean by independent system is a process which is
not owned by the same user that the database is running as, and isn't
owned by the user who is connecting, that facilitates the connection
from the user to PG, but which logs everything that happens on that
connection.

 With the scheme I described above, how does the db superuser disable auditing 
 without os root privileges?  If they can, then pg security is fundamentally 
 broken, and I don’t believe it is.

The DB superuser can modify the running process, through mechanisms as
simple as changing GUCs to creating functions in untrusted languages
(including C) and then using them to change or break more-or-less
anything that's happening in the backend.

 How can an independent system monitor what commands are issued inside the 
 database?

It can log everything which happens on the connection between the user
and the database because it's in the middle.

 I understand my comments do not cover what is being proposed or worked on and 
 that is fine.  But saying it does not have value because the superuser could 
 disable any system in pg, is wrong IMO.  Being able to reliability log db 
 superusers without their ability to change the logging would be a 
 fundamentally good security tool as companies become more serious about 
 internal security.  This is, and will be more, important since a lot of 
 people consider insider breaches the biggest security challenge today.

It'd be a great tool, certainly, but it's not actually possible to do
completely inside the DB process given the capabilities the PG superuser
has.  Being able to create and run functions in untrusted languages
means that the superuser can completely hijack the process, that's why
those languages are untrusted.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-18 Thread Alvaro Herrera
Stephen Frost wrote:
 Abhijit,
 
 * Abhijit Menon-Sen (a...@2ndquadrant.com) wrote:

  I'm confused and unhappy about your characterisation of the state of
  this patch. You make it seem as though there was broad consensus about
  the changes that were needed, and that I left the patch sitting in the
  archives for a long time without addressing important issues.
 
 The specific issue which I was referring to there was the #ifdef's for
 the deparse branch.  I thought it was pretty clear, based on the
 feedback from multiple people, that all of that really needed to be
 taken out as we don't commit code to the main repo which has such
 external dependencies.

We can remove the #ifdef lines as soon as DDL deparse gets committed, of
course.  There is no external dependency here, only a dependency on a
patch that's being submitted in parallel.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-18 Thread Stephen Frost
Abhijit,

* Abhijit Menon-Sen (a...@2ndquadrant.com) wrote:
 At 2015-02-17 13:01:46 -0500, sfr...@snowman.net wrote:
  I have to admit that I'm confused by this.  Patches don't stabilise
  through sitting in the archives, they stabilise when the comments are
  being addressed, the patch updated, and further comments are
  addressing less important issues.  The issues which Robert and I had
  both commented on didn't appear to be getting addressed.
 
 I'm confused and unhappy about your characterisation of the state of
 this patch. You make it seem as though there was broad consensus about
 the changes that were needed, and that I left the patch sitting in the
 archives for a long time without addressing important issues.

The specific issue which I was referring to there was the #ifdef's for
the deparse branch.  I thought it was pretty clear, based on the
feedback from multiple people, that all of that really needed to be
taken out as we don't commit code to the main repo which has such
external dependencies.  That wasn't meant as a characterization of the
patch itself but rather a comment on the state of the process and that
I, at least, had been hoping for a new version which addressed those
bits.

 You revised and refined your GRANT proposal in stages, and I tried to
 adapt the code to suit. I'm sorry that my efforts were not fast enough 
 or responsive enough to make you feel that progress was being made. But
 nobody else commented in detail on the GRANT changes except to express
 general misgivings, and nobody else even disagreed when I inferred, in
 light of the lack of consensus that Robert pointed out, that the code
 was unlikely to make it into 9.5.

Progress was being made and I certainly appreciate your efforts.  I
don't think anyone would want to stand up and say it's going to be in
9.5 or not be in 9.5 which is likely why you didn't get any reaction
from your comment about it being unlikely.  I'm certainly hopeful that
something gets in along these lines as it's a pretty big capability that
we're currently missing.

 Given that I've maintained the code over the past year despite its being
 rejected in an earlier CF, and given the circumstances outlined above, I
 do not think it's reasonable to conclude after a couple of weeks without
 a new version that it was abandoned. As I had mentioned earlier, there
 are people who already use pgaudit as-is, and complain if I break it.

For my part, at least, I didn't intend to characterize it as abandoned
but rather that it simply didn't seem to be moving forward, perhaps due
to a lack of time to work on it or other priorities; I didn't mean to
imply that you wouldn't be continuing to maintain it.  As for the
comment about people depending on it, I'm not sure what that's getting
at, but I don't think that's really a consideration for us as it relates
to having a contrib module to provide this capability.  We certainly
want to consider what capabilities users are looking for and make sure
we cover those cases, if possible, but this is at a development stage
with regard to core and therefore things may break for existing users.

If that's an issue for your users then it would probably be good to have
a clean distinction between the stable code you're providing to them for
auditing and what's being submitted for inclusion in core, with an
expectation that users will need to make some changes when they switch
to the version which is included in core.

 Anyway, I guess there is no such thing as a constructive history
 discussion, so I'll drop it.

It's not my intent to continue this as I certainly agree that it seems
unlikely to be a constructive use of time, but I wanted to reply and
clarify that it wasn't my intent to characterize pgaudit as abandoned
and to apologize for my comments coming across that way.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-18 Thread David Steele
Hi Fujii,

Thanks for taking a look at the patch.  Comments below:

On 2/18/15 6:11 AM, Fujii Masao wrote:
 On Wed, Feb 18, 2015 at 1:26 AM, David Steele da...@pgmasters.net wrote:
 On 2/17/15 10:23 AM, Simon Riggs wrote:
 I vote to include pgaudit in 9.5, albeit with any changes. In
 particular, David may have some changes to recommend, but I haven't
 seen a spec or a patch, just a new version of code (which isn't how we
 do things...).

 I submitted the new patch in my name under a separate thread Auditing
 extension for PostgreSQL (Take 2) (54e005cc.1060...@pgmasters.net)
 
 I played the patch version of pg_audit a bit and have basic comments about
 its spec.
 
 The pg_audit doesn't log BIND parameter values when prepared statement is 
 used.
 Seems this is an oversight of the patch. Or is this intentional?

It's actually intentional - following the model I talked about in my
earlier emails, the idea is to log statements only.  This also follows
on 2ndQuadrant's implementation.

Logging values is interesting, but I'm sure the user would want to
specify which columns to log, which I felt was beyond the scope of the
patch.

 The pg_audit cannot log the statement like SELECT 1 which doesn't access to
 the database object. Is this intentional? I think that there are many users 
 who
 want to audit even such statement.

I think I see how to make this work.  I'll work on it for the next
version of the patch.

 
 Imagine the case where you call the user-defined function which executes
 many nested statements. In this case, pg_audit logs only top-level statement
 (i.e., issued directly by client) every time nested statement is executed.
 In fact, one call of such UDF can cause lots of *same* log messages. I think
 this is problematic.

I agree - not sure how to go about addressing it, though.  I've tried to
cut down on the verbosity of the logging in general, but of course it
can still be a problem.

Using security definer and a different logging GUC for the defining role
might work.  I'll add that to my unit tests and see what happens.

I appreciate your feedback!

-- 
- David Steele
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread Jim Nasby

On 2/17/15 1:10 PM, Stephen Frost wrote:

What I'd prefer to see is a way to decouple the OS account from any
DB account. Clearly that doesn't protect us from the OS user doing
something bad, but at least then there's no way for them to just
silently run SQL commands.

If the DB account isn't a superuser then everything changes.  There's no
point fighting with the OS user- they can run some other PG binary which
they've copied over locally and run SQL with that, or copy all the files
over to another server which they have complete access to.  The fact
that they can also connect to the DB and run SQL isn't really an issue.


I disagree. The difference here is that the OS can audit whatever 
commands they're running, but not what happens inside something like 
psql. Even if you did run a keylogger, trying to use that to interpret a 
psql session would be a real pain, if not impossible. So I don't think 
we can rely on the OS to audit SQL at all. But obviously if they did 
something like copy the files somewhere else, or bring in a new binary, 
the OS would at least have record that that happened.


Though... I wonder if there's some way we could disallow *all* superuser 
access to the database, and instead create a special non-interactive 
CLI. That would allow us to throw the problem over the wall to the OS.


In any case, I think it's clear that there's a lot of value in at least 
handling the non-SU case, so we should try and do that now. Even if it's 
only in contrib.


One thing that does occur to me though... perhaps we should specifically 
disable auditing of SU activities, so we're not providing a false sense 
of security.

--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread Stephen Frost
* Jim Nasby (jim.na...@bluetreble.com) wrote:
 On 2/17/15 1:10 PM, Stephen Frost wrote:
 What I'd prefer to see is a way to decouple the OS account from any
 DB account. Clearly that doesn't protect us from the OS user doing
 something bad, but at least then there's no way for them to just
 silently run SQL commands.
 If the DB account isn't a superuser then everything changes.  There's no
 point fighting with the OS user- they can run some other PG binary which
 they've copied over locally and run SQL with that, or copy all the files
 over to another server which they have complete access to.  The fact
 that they can also connect to the DB and run SQL isn't really an issue.
 
 I disagree. The difference here is that the OS can audit whatever
 commands they're running, but not what happens inside something like
 psql. Even if you did run a keylogger, trying to use that to
 interpret a psql session would be a real pain, if not impossible. So
 I don't think we can rely on the OS to audit SQL at all. But
 obviously if they did something like copy the files somewhere else,
 or bring in a new binary, the OS would at least have record that
 that happened.

From my experience, logging the commands is no where near enough..  psql
is hardly the only complex CLI process one can run.  Further, I'm not
suggesting that we rely on the OS to audit SQL, merely stating that
anything which deals with auditing the connection to PG needs to be
outside of the PG process space (and that of processes owned by the user
which PG is running as).

 Though... I wonder if there's some way we could disallow *all*
 superuser access to the database, and instead create a special
 non-interactive CLI. That would allow us to throw the problem over
 the wall to the OS.

That sounds like a horrible approach.  A non-interactive CLI would be
terrible and it's not clear to me how that'd really help.  What happens
when they run emacs or vim?

 In any case, I think it's clear that there's a lot of value in at
 least handling the non-SU case, so we should try and do that now.
 Even if it's only in contrib.

Absolutely.  Glad we agree on that. :)

 One thing that does occur to me though... perhaps we should
 specifically disable auditing of SU activities, so we're not
 providing a false sense of security.

I don't agree with this.  Done properly (auditing enabled on startup,
using a remote auditing target, etc), it might be possible for such
auditing to catch odd superuser behavior.  What we don't want to do is
claim that we provide full superuser auditing, as that's something we
can't provide.  Appropriate and clear documentation is absolutely
critical, of course.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread Abhijit Menon-Sen
At 2015-02-17 13:01:46 -0500, sfr...@snowman.net wrote:

 I have to admit that I'm confused by this.  Patches don't stabilise
 through sitting in the archives, they stabilise when the comments are
 being addressed, the patch updated, and further comments are
 addressing less important issues.  The issues which Robert and I had
 both commented on didn't appear to be getting addressed.

I'm confused and unhappy about your characterisation of the state of
this patch. You make it seem as though there was broad consensus about
the changes that were needed, and that I left the patch sitting in the
archives for a long time without addressing important issues.

You revised and refined your GRANT proposal in stages, and I tried to
adapt the code to suit. I'm sorry that my efforts were not fast enough 
or responsive enough to make you feel that progress was being made. But
nobody else commented in detail on the GRANT changes except to express
general misgivings, and nobody else even disagreed when I inferred, in
light of the lack of consensus that Robert pointed out, that the code
was unlikely to make it into 9.5.

Given that I've maintained the code over the past year despite its being
rejected in an earlier CF, and given the circumstances outlined above, I
do not think it's reasonable to conclude after a couple of weeks without
a new version that it was abandoned. As I had mentioned earlier, there
are people who already use pgaudit as-is, and complain if I break it.

Anyway, I guess there is no such thing as a constructive history
discussion, so I'll drop it.

-- Abhijit


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread Abhijit Menon-Sen
At 2015-02-17 10:52:55 -0500, sfr...@snowman.net wrote:

 From the old thread, David had offered to submit a pull request if
 there was interest and I didn't see any response...

For whatever it's worth, that's because I've been away from work, and
only just returned. I had it on my list to look at the code tomorrow.

-- Abhijit


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread Neil Tiffin

 On Feb 17, 2015, at 3:40 AM, Yeb Havinga yebhavi...@gmail.com wrote:
 
 Hi list,
 
. . . . . 

 Auditing superuser access means auditing beyond the running database.
 The superuser can dump a table, and pipe this data everywhere outside of
 the auditing domain. I cannot begin to imagine the kind of security
 restrictions you'd need to audit what happens with data after libpq has
 produced the results. My best guess would be to incorporate some kind of
 separation of duty mechanism; only allow certain superuser operations
 with two people involved.

My views are from working with FDA validated environments, and I don’t really 
understand the above.  It is not db auditing’s job to stop or control the 
access to data or to log what happens to data outside of PostgreSQL.  To audit 
a db superuser is very simple, keep a log of everything a super user does and 
to write that log to a write append, no read filesystem or location.  Since the 
db superuser can do anything there is no value in configuring what to log.  
This should be an option that once set, cannot be changed without reinstalling 
the PostgreSQL binary.  The responsibility for auditing/controlling any binary 
replacement is the operating system’s, not PostgreSQL.  In this environment the 
db superuser will not have authorized root access for the os.

The use case examples, that I am familiar with, are that procedural policies 
control what the db superuser can do.  If the policy says that the db superuser 
cannot dump a table and pipe this data someplace without being supervised by a 
third party auditor (building on the above), then what you want in the log is 
that the data was dumped by whom with a date and time.  Thats it.  Its up to 
policies, audit review, management, and third party audit tools, to pick up the 
violation.  Auditing’s job is to keep a complete report, not prevent.  
Prevention is the role of security.

Neil



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread Simon Riggs
On 17 February 2015 at 15:52, Stephen Frost sfr...@snowman.net wrote:
 * Simon Riggs (si...@2ndquadrant.com) wrote:
 I vote to include pgaudit in 9.5, albeit with any changes. In
 particular, David may have some changes to recommend, but I haven't
 seen a spec or a patch, just a new version of code (which isn't how we
 do things...).

 Hrm.  I thought David's new patch actually looked quite good and it's
 certainly quite a bit different from the initial patch (which didn't
 seem like it was moving forward..).  Guess I'm confused how a new patch
 is different from a 'new version of code' and I didn't see a spec for
 either patch.  From the old thread, David had offered to submit a pull
 request if there was interest and I didn't see any response...

My comment was that the cycle of development is discuss then develop.

David's work is potentially useful, but having two versions of a
feature slows things down. Since he is new to development here, I have
made those comments so he understands, not so you would pick up on
that.

didn't seem to be moving forwards is strange comment. We usually
wait for patches to stabilise, not for them to keep changing as
evidence of worth.

David, please submit your work to pgsql-hackers as a patch on
Abhijit's last version. There is no need for a pull request to
2ndQuadrant. Thanks.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, RemoteDBA, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread Stephen Frost
* Simon Riggs (si...@2ndquadrant.com) wrote:
 I vote to include pgaudit in 9.5, albeit with any changes. In
 particular, David may have some changes to recommend, but I haven't
 seen a spec or a patch, just a new version of code (which isn't how we
 do things...).

Hrm.  I thought David's new patch actually looked quite good and it's
certainly quite a bit different from the initial patch (which didn't
seem like it was moving forward..).  Guess I'm confused how a new patch
is different from a 'new version of code' and I didn't see a spec for
either patch.  From the old thread, David had offered to submit a pull
request if there was interest and I didn't see any response...

 I'm happy to do final review and commit. Assuming we are in agreement,
 what changes are needed prior to commit?

I'm all about getting something done here for 9.5 also and would
certainly prefer to focus on that.

The recent discussion has all moved towards the approach that I was
advocating where we use GRANT simimlar to how AUDIT exists in other
RDBMS's.  Both the latest version of the code from Abhijit and David's
code do that and I found what David did quite easy to follow- no big
#ifdef blocks (something I complained about earlier but didn't see any
progress on..) and no big switch statements that would likely get
out-dated very quickly.  I'm not against going back to the code
submitted by Abhijit, if it's cleaned up and has the #ifdef blocks and
whatnot removed that were discussed previously.  I don't fault David for
moving forward though, given the lack of feedback.

Perhaps there's an issue where the classes provided by David's approach
aren't granular enough but it's certainly better than what we have
today.  The event-trigger based approach depends on as-yet-uncommitted
code, as I understand it.  I'd certainly rather have fewer audit classes
which cover everything than more audit classes which end up not covering
everything because we don't have all the deparse code or event triggers
we need completed and committed yet.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread Stephen Frost
Yeb,

* Yeb Havinga (yebhavi...@gmail.com) wrote:
 On 20/01/15 23:03, Jim Nasby wrote: On 1/20/15 2:20 PM, Robert Haas wrote:
  +1. In particular I'm very concerned with the idea of doing this via
  roles, because that would make it trivial for any superuser to disable
  auditing.
 
 Rejecting the audit administration through the GRANT system, on the
 grounds that it easy for the superuser to disable it, seems unreasonable
 to me, since superusers are different from non-superusers in a
 fundamental way.

Agreed.

 The patch as it is, is targeted at auditing user/application level
 access to the database, and as such it matches the use case of auditing
 user actions.

Right, and that's a *very* worthwhile use-case.

 Auditing superuser access means auditing beyond the running database.

Exactly! :)

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread David Steele
On 2/17/15 4:40 AM, Yeb Havinga wrote:
 On 20/01/15 23:03, Jim Nasby wrote: On 1/20/15 2:20 PM, Robert Haas wrote:
 On Tue, Jan 20, 2015 at 1:05 AM, Abhijit
 Menon-Sena...@2ndquadrant.com  wrote:
 So when I'm trying to decide what to audit, I have to:

 (a) check if the current user is mentioned in .roles; if so,
 audit.

 (b) check if the current user is a descendant of one of the roles
 mentioned in .roles; if not, no audit.

 (c) check for permissions granted to the root role and see if
 that
 tells us to audit.

 Is that right? If so, it would work fine. I don't look forward to
 trying
 to explain it to people, but yes, it would work (for anything you could
 grant permissions for).
 I think this points to fundamental weakness in the idea of doing this
 through the GRANT system.  Some people are going want to audit
 everything a particular user does, some people are going to want to
 audit all access to particular objects, and some people will have more
 complicated requirements.  Some people will want to audit even
 super-users, others especially super-users, others only non
 super-users.  None of this necessarily matches up to the usual
 permissions framework.

 +1. In particular I'm very concerned with the idea of doing this via
 roles, because that would make it trivial for any superuser to disable
 auditing.
 
 Rejecting the audit administration through the GRANT system, on the
 grounds that it easy for the superuser to disable it, seems unreasonable
 to me, since superusers are different from non-superusers in a
 fundamental way.

Agreed.  This logic could be applied to any database object: why have
tables when a superuser can so easily drop them and cause data loss?

 The patch as it is, is targeted at auditing user/application level
 access to the database, and as such it matches the use case of auditing
 user actions.

Exactly.  This patch (and my rework) are focused entirely on auditing
the actions of normal users in the database.  While auditing can be
enabled for superusers, there's no guarantee that it's reliable since it
would be easy for a superuser to disable.  Normal users can be
configured to not have that capability, so auditing them is reliable.

-- 
- David Steele
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread Simon Riggs
On 17 February 2015 at 14:44, Stephen Frost sfr...@snowman.net wrote:

 The patch as it is, is targeted at auditing user/application level
 access to the database, and as such it matches the use case of auditing
 user actions.

 Right, and that's a *very* worthwhile use-case.

Agreed.

So, we are still at the same place we were at 7-8 months ago: Some
people would like an AUDIT command, but this isn't it. We have neither
a design nor a developer willing to implement it (or funding to do
so). That may change in the future, but if it does, we will not have
auditing in production version of Postgres before September 2016,
earliest if we wait for that.

I vote to include pgaudit in 9.5, albeit with any changes. In
particular, David may have some changes to recommend, but I haven't
seen a spec or a patch, just a new version of code (which isn't how we
do things...).

In my understanding, the following people are in favour of pgaudit, in
some form: Simon, Yeb, David, Stephen and other developers have spoken
earlier in favour of including it.

Abhijit, Jim and Robert have voiced recent doubts of various kinds,
but there seems to be no outstanding objection to including pgaudit,
only a wish that we had something better. (Please correct me).

I'm happy to do final review and commit. Assuming we are in agreement,
what changes are needed prior to commit?

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, RemoteDBA, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread Jim Nasby

On 2/17/15 12:07 PM, Stephen Frost wrote:

My views are from working with FDA validated environments, and I don’t really 
understand the above.  It is not db auditing’s job to stop or control the 
access to data or to log what happens to data outside of PostgreSQL.  To audit 
a db superuser is very simple, keep a log of everything a super user does and 
to write that log to a write append, no read filesystem or location.  Since the 
db superuser can do anything there is no value in configuring what to log.  
This should be an option that once set, cannot be changed without reinstalling 
the PostgreSQL binary.  The responsibility for auditing/controlling any binary 
replacement is the operating system’s, not PostgreSQL.  In this environment the 
db superuser will not have authorized root access for the os.

I agree that it's not the auditing job to stop or control access to
data, but it's not so simple to audit the superuser completely.  The
issue is that even if you have a hard-coded bit in the binary which says
audit everything, a superuser can change the running code to twiddle
that bit off, redirect the output of whatever auditing is happening,
gain OS-level (eg: shell) access to the system and then make changes to
the files under PG directly, etc.  Setting a bit in a binary and then
not allowing that binary to be unchanged does not actually solve the
issue.


If we've allowed a superuser *in the database* that kind of power at the 
OS level then we have a problem. There needs to be *something* that a 
database SU can't do at the OS level, otherwise we'll never be able to 
audit database SU activity.

--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread Jim Nasby

On 2/17/15 12:23 PM, Stephen Frost wrote:

* Jim Nasby (jim.na...@bluetreble.com) wrote:

On 2/17/15 12:07 PM, Stephen Frost wrote:

I agree that it's not the auditing job to stop or control access to
data, but it's not so simple to audit the superuser completely.  The
issue is that even if you have a hard-coded bit in the binary which says
audit everything, a superuser can change the running code to twiddle
that bit off, redirect the output of whatever auditing is happening,
gain OS-level (eg: shell) access to the system and then make changes to
the files under PG directly, etc.  Setting a bit in a binary and then
not allowing that binary to be unchanged does not actually solve the
issue.


If we've allowed a superuser *in the database* that kind of power at
the OS level then we have a problem. There needs to be *something*
that a database SU can't do at the OS level, otherwise we'll never
be able to audit database SU activity.


This isn't a question.  The database superuser has essentially OS-level
privileges as the user which PG runs as.

I'm all for coming up with a less powerful superuser and the work I've
been involved in around adding more role attributes is along the lines
to get us there, but I don't think we're ever going to really reduce the
power that the PG superuser has, for a variety of reasons.

Improving the documentation of what a superuser can do and how granting
such access is the same as giving OS shell-level access to the system as
the user that PG runs as would certainly be good.


It certainly would. I'm honestly not totally clear on what all the holes 
are.


We may need to bite the bullet and allow changing the user that the 
postgres process runs under so it doesn't match who owns the files. 
Maybe there's a way to allow that other than having the process start as 
root.


Or maybe there's some other way we could restrict what a DB superuser 
can do in the shell.

--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread Stephen Frost
* Simon Riggs (si...@2ndquadrant.com) wrote:
 David's work is potentially useful, but having two versions of a
 feature slows things down. Since he is new to development here, I have
 made those comments so he understands, not so you would pick up on
 that.

I have a bad tendency of replying to email which is replying to comments
which I made. :)

 didn't seem to be moving forwards is strange comment. We usually
 wait for patches to stabilise, not for them to keep changing as
 evidence of worth.

I have to admit that I'm confused by this.  Patches don't stabilise
through sitting in the archives, they stabilise when the comments are
being addressed, the patch updated, and further comments are addressing
less important issues.  The issues which Robert and I had both commented
on didn't appear to be getting addressed.  That seems to be due to
Abhijit being out, which is certainly fine, but that wasn't clear, at
least to me.

 David, please submit your work to pgsql-hackers as a patch on
 Abhijit's last version. There is no need for a pull request to
 2ndQuadrant. Thanks.

Ugh.  For my part, at least, having patches on top of patches does *not*
make things easier to work with or review.  I'm very glad to hear that
Abhijit is back and has time to work on this, but as it relates to
submitting patches for review to the list or to the commitfest, I'd
really like those to be complete patches and not just .c files or
patches on top of other patches.  To the extent that it helps move this
along, I'm not going to object if such patches are posted, but I would
object to patches-on-patches being included in the commmitfest.  I've
not spent much time with the new commitfest app yet, but hopefully it
won't be hard to note which patches are the complete ones.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread Stephen Frost
* Jim Nasby (jim.na...@bluetreble.com) wrote:
 On 2/17/15 12:07 PM, Stephen Frost wrote:
 I agree that it's not the auditing job to stop or control access to
 data, but it's not so simple to audit the superuser completely.  The
 issue is that even if you have a hard-coded bit in the binary which says
 audit everything, a superuser can change the running code to twiddle
 that bit off, redirect the output of whatever auditing is happening,
 gain OS-level (eg: shell) access to the system and then make changes to
 the files under PG directly, etc.  Setting a bit in a binary and then
 not allowing that binary to be unchanged does not actually solve the
 issue.
 
 If we've allowed a superuser *in the database* that kind of power at
 the OS level then we have a problem. There needs to be *something*
 that a database SU can't do at the OS level, otherwise we'll never
 be able to audit database SU activity.

This isn't a question.  The database superuser has essentially OS-level
privileges as the user which PG runs as.

I'm all for coming up with a less powerful superuser and the work I've
been involved in around adding more role attributes is along the lines
to get us there, but I don't think we're ever going to really reduce the
power that the PG superuser has, for a variety of reasons.

Improving the documentation of what a superuser can do and how granting
such access is the same as giving OS shell-level access to the system as
the user that PG runs as would certainly be good.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread Stephen Frost
Jim,

* Jim Nasby (jim.na...@bluetreble.com) wrote:
 We may need to bite the bullet and allow changing the user that the
 postgres process runs under so it doesn't match who owns the files.
 Maybe there's a way to allow that other than having the process
 start as root.

That's an interesting thought but it doesn't seem too likely to work out
for us.  The process still has to be able to read and write the files,
create new files in the PGDATA directories, etc.

 Or maybe there's some other way we could restrict what a DB
 superuser can do in the shell.

This could be done with SELinux and similar tools, but at the end of the
day the answer, in my view really, is to have fewer superusers and for
those superusers to be understood to have OS-level shell access.  We
don't want to deal with all of the security implications of trying to
provide a trusted superuser when that user can create functions in
untrusted languages, modify the catalog directly, etc, it really just
doesn't make sense.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread Jim Nasby

On 2/17/15 12:50 PM, Stephen Frost wrote:

Jim,

* Jim Nasby (jim.na...@bluetreble.com) wrote:

We may need to bite the bullet and allow changing the user that the
postgres process runs under so it doesn't match who owns the files.
Maybe there's a way to allow that other than having the process
start as root.


That's an interesting thought but it doesn't seem too likely to work out
for us.  The process still has to be able to read and write the files,
create new files in the PGDATA directories, etc.


Right, but if we don't allow things like loading C functions from 
wherever you please then it's a lot less likely that a DB SU could 
disable auditing.



Or maybe there's some other way we could restrict what a DB
superuser can do in the shell.


This could be done with SELinux and similar tools, but at the end of the
day the answer, in my view really, is to have fewer superusers and for
those superusers to be understood to have OS-level shell access.  We
don't want to deal with all of the security implications of trying to
provide a trusted superuser when that user can create functions in
untrusted languages, modify the catalog directly, etc, it really just
doesn't make sense.


The part I don't like about that is then you still have this highly 
trusted account that can also run SQL. The big issue with that is that 
no OS-level auditing is going to catch what happens inside the database 
itself (well, I guess short of a key logger...)


What I'd prefer to see is a way to decouple the OS account from any DB 
account. Clearly that doesn't protect us from the OS user doing 
something bad, but at least then there's no way for them to just 
silently run SQL commands.

--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread Stephen Frost
* Jim Nasby (jim.na...@bluetreble.com) wrote:
 On 2/17/15 12:50 PM, Stephen Frost wrote:
 * Jim Nasby (jim.na...@bluetreble.com) wrote:
 We may need to bite the bullet and allow changing the user that the
 postgres process runs under so it doesn't match who owns the files.
 Maybe there's a way to allow that other than having the process
 start as root.
 
 That's an interesting thought but it doesn't seem too likely to work out
 for us.  The process still has to be able to read and write the files,
 create new files in the PGDATA directories, etc.
 
 Right, but if we don't allow things like loading C functions from
 wherever you please then it's a lot less likely that a DB SU could
 disable auditing.

It's not just C functions, there's also a whole slew of untrusted
languages, and a superuser can modify the catalog directly.  They could
change the relfileno for a relation to some other relation that they're
really interested in, or use pageinspect, etc.

 Or maybe there's some other way we could restrict what a DB
 superuser can do in the shell.
 
 This could be done with SELinux and similar tools, but at the end of the
 day the answer, in my view really, is to have fewer superusers and for
 those superusers to be understood to have OS-level shell access.  We
 don't want to deal with all of the security implications of trying to
 provide a trusted superuser when that user can create functions in
 untrusted languages, modify the catalog directly, etc, it really just
 doesn't make sense.
 
 The part I don't like about that is then you still have this highly
 trusted account that can also run SQL. The big issue with that is
 that no OS-level auditing is going to catch what happens inside the
 database itself (well, I guess short of a key logger...)

Right- you would need an independent process which acts as an
intermediary between the database and the account connecting which
simply logs everything.  That's really not all *that* hard to do, but
it's clearly outside of the scope of PG.

 What I'd prefer to see is a way to decouple the OS account from any
 DB account. Clearly that doesn't protect us from the OS user doing
 something bad, but at least then there's no way for them to just
 silently run SQL commands.

If the DB account isn't a superuser then everything changes.  There's no
point fighting with the OS user- they can run some other PG binary which
they've copied over locally and run SQL with that, or copy all the files
over to another server which they have complete access to.  The fact
that they can also connect to the DB and run SQL isn't really an issue.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread Stephen Frost
Neil,

* Neil Tiffin (ne...@neiltiffin.com) wrote:
  On Feb 17, 2015, at 3:40 AM, Yeb Havinga yebhavi...@gmail.com wrote:
  Auditing superuser access means auditing beyond the running database.
  The superuser can dump a table, and pipe this data everywhere outside of
  the auditing domain. I cannot begin to imagine the kind of security
  restrictions you'd need to audit what happens with data after libpq has
  produced the results. My best guess would be to incorporate some kind of
  separation of duty mechanism; only allow certain superuser operations
  with two people involved.
 
 My views are from working with FDA validated environments, and I don’t really 
 understand the above.  It is not db auditing’s job to stop or control the 
 access to data or to log what happens to data outside of PostgreSQL.  To 
 audit a db superuser is very simple, keep a log of everything a super user 
 does and to write that log to a write append, no read filesystem or location. 
  Since the db superuser can do anything there is no value in configuring what 
 to log.  This should be an option that once set, cannot be changed without 
 reinstalling the PostgreSQL binary.  The responsibility for 
 auditing/controlling any binary replacement is the operating system’s, not 
 PostgreSQL.  In this environment the db superuser will not have authorized 
 root access for the os.

I agree that it's not the auditing job to stop or control access to
data, but it's not so simple to audit the superuser completely.  The
issue is that even if you have a hard-coded bit in the binary which says
audit everything, a superuser can change the running code to twiddle
that bit off, redirect the output of whatever auditing is happening,
gain OS-level (eg: shell) access to the system and then make changes to
the files under PG directly, etc.  Setting a bit in a binary and then
not allowing that binary to be unchanged does not actually solve the
issue.

 The use case examples, that I am familiar with, are that procedural policies 
 control what the db superuser can do.  If the policy says that the db 
 superuser cannot dump a table and pipe this data someplace without being 
 supervised by a third party auditor (building on the above), then what you 
 want in the log is that the data was dumped by whom with a date and time.  
 Thats it.  Its up to policies, audit review, management, and third party 
 audit tools, to pick up the violation.  Auditing’s job is to keep a complete 
 report, not prevent.  Prevention is the role of security.

Agreed, policy is how to handle superuser-level access and auditing can
assist with that but without having an independent process (one which
the PG superuser can't control, which means it needs to be a different
OS-level user), it's not possible to guarantee that the audit is
complete when the superuser is involved.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread David Steele
On 2/17/15 10:23 AM, Simon Riggs wrote:
 I vote to include pgaudit in 9.5, albeit with any changes. In
 particular, David may have some changes to recommend, but I haven't
 seen a spec or a patch, just a new version of code (which isn't how we
 do things...).

I submitted the new patch in my name under a separate thread Auditing
extension for PostgreSQL (Take 2) (54e005cc.1060...@pgmasters.net)
because I was not getting any response from the original authors and I
wanted to make sure the patch got in for the 2015-02 CF.

Of course credit should be given where it is due - to that end I sent
email to Ian and Abhijit over the weekend but haven't heard back from
them yet.  I appreciate that 2ndQuadrant spearheaded this effort and
though I made many changes, the resulting code follows the same general
design.

 I'm happy to do final review and commit. Assuming we are in agreement,
 what changes are needed prior to commit?

I've incorporated most of Stephen's changes but I won't have time to get
another patch out today.

However, since most of his comments were about comments, I'd be happy to
have your review as is and I appreciate your feedback.

-- 
- David Steele
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-17 Thread Yeb Havinga
Hi list,

On 20/01/15 23:03, Jim Nasby wrote: On 1/20/15 2:20 PM, Robert Haas wrote:
 On Tue, Jan 20, 2015 at 1:05 AM, Abhijit
 Menon-Sena...@2ndquadrant.com  wrote:
 So when I'm trying to decide what to audit, I have to:
 
  (a) check if the current user is mentioned in .roles; if so,
 audit.
 
  (b) check if the current user is a descendant of one of the roles
  mentioned in .roles; if not, no audit.
 
  (c) check for permissions granted to the root role and see if
 that
  tells us to audit.
 
 Is that right? If so, it would work fine. I don't look forward to
 trying
 to explain it to people, but yes, it would work (for anything you could
 grant permissions for).
 I think this points to fundamental weakness in the idea of doing this
 through the GRANT system.  Some people are going want to audit
 everything a particular user does, some people are going to want to
 audit all access to particular objects, and some people will have more
 complicated requirements.  Some people will want to audit even
 super-users, others especially super-users, others only non
 super-users.  None of this necessarily matches up to the usual
 permissions framework.

 +1. In particular I'm very concerned with the idea of doing this via
 roles, because that would make it trivial for any superuser to disable
 auditing.

Rejecting the audit administration through the GRANT system, on the
grounds that it easy for the superuser to disable it, seems unreasonable
to me, since superusers are different from non-superusers in a
fundamental way.

The superuser operates on the administration level of the database, in
contrast with users that need access to the actual information in the
data to perform their job. An organization that wants to implement an
auditing strategy needs to think in different terms to audit
user/application level actions, and administrator/superuser actions.
Consequently it should be no surprise when PostgreSQL mechanisms for
auditing behave different for superusers vs non superusers.

The patch as it is, is targeted at auditing user/application level
access to the database, and as such it matches the use case of auditing
user actions.

Auditing superuser access means auditing beyond the running database.
The superuser can dump a table, and pipe this data everywhere outside of
the auditing domain. I cannot begin to imagine the kind of security
restrictions you'd need to audit what happens with data after libpq has
produced the results. My best guess would be to incorporate some kind of
separation of duty mechanism; only allow certain superuser operations
with two people involved.

regards,
Yeb Havinga


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-13 Thread Michael Paquier
On Tue, Jan 27, 2015 at 6:08 PM, Abhijit Menon-Sen a...@2ndquadrant.com
wrote:

 Anyway, I think it's reasonably clear now that pgaudit is unlikely to
 make it into 9.5 in any form, so I'll find something else to do.


Well, I am marking this patch as rejected then... Let's in any case the
discussion continue. Perhaps we could reach a clearer spec about what we
want and how to shape it.
-- 
Michael


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-08 Thread David Steele
On 2/2/15 3:49 PM, David Steele wrote:

 The role-base approach being considered may strike some as a misuse of
 the role system, but to my eye it is syntactically very close to how
 Oracle does auditing prior to 12c.  Say you wanted to audit selects on
 the table hr.employee:

 Oracle: AUDIT SELECT ON hr.employee;
 pgaudit: GRANT SELECT ON hr.employee TO audit; (assuming audit is the
 role defined by pgaudit.roles)

 Object-based auditing in Oracle would be very easy to migrate to the
 grants needed for pgaudit.  In addition, if an AUDIT command were
 introduced later in core, it would be easy to migrate from pgaudit to
 AUDIT assuming the syntax was similar to grant, which seems plausible.

I decided to take a whack at this and see what I could come up with, starting 
with the code in master at https://github.com/2ndQuadrant/pgaudit.

I modified pgaudit.log to work similarly to Oracle's session-level logging, 
meaning user statements are logged instead of tables which are accessed. 
pgaudit.log still has the various classes of commands and only those commands 
which match one of the classes are logged. Note that the pgaudit.log GUC is 
SUSET but can be set at the cluster, database, or user level.

An example - log all statements that create an object or read data:

   DB: connect user postgres, database postgres
  SQL: set pgaudit.log = 'DEFINITION, READ'
  SQL: create user user1

   DB: connect user user1, database postgres
  SQL: create table account
   (
   id int,
   name text,
   password text,
   description text
   );
AUDIT: SESSION,DEFINITION,CREATE TABLE,TABLE,public.account,sql

  SQL: select *
 from account;
AUDIT: SESSION,READ,SELECT,,,statement

  SQL: insert into account (id, name, password, description)
values (1, 'user1', 'HASH1', 'blah, blah');
AUDIT: nothing logged

Object auditing is done via the grant system (similar to Oracle object 
auditing), but now there is now a single audit role (defined by the 
pgaudit.role GUC which can also be set at the cluster, database, or user level).

An example - using column-level grants since they are more interesting:

 DB: connect user postgres, database postgres
  SQL: set pgaudit.log = 'NONE'
  SQL: create role audit
  SQL: set pgaudit.role = 'audit'

   DB: connect user user1, database postgres

  SQL: grant select (password)
  on public.account
  to audit;
AUDIT: nothing logged

  SQL: select id, name
 from account;
AUDIT: nothing logged

  SQL: select password
 from account;
AUDIT: OBJECT,READ,SELECT,TABLE,public.account,sql

  SQL: grant update (name, password)
  on public.account
  to audit;
AUDIT: nothing logged

  SQL: update account
  set description = 'yada, yada';
AUDIT: nothing logged

  SQL: update account
  set password = 'HASH2';
AUDIT: OBJECT,WRITE,UPDATE,TABLE,public.account,sql

Session and object auditing can be used together so a statement that does not 
match on an object may still be session logged depending on the settings.

An example - in this case the pgaudit.log GUC will be set on the user and 
grants persist from the previous example.  Another table is added to show how 
that affects logging:

   DB: connect user postgres, database postgres
  SQL: alter role user1 set pgaudit.log = 'READ,WRITE';
AUDIT: nothing logged

   DB: connect user user1, database postgres
  SQL: create table account_role_map
   (
   account_id int,
   role_id int
   );
AUDIT: nothing logged

  SQL: grant select
  on public.account_role_map
  to audit;
AUDIT: nothing logged

  SQL: select account.password, 
  account_role_map.role_id
 from account
  inner join account_role_map
   on account.id = account_role_map.account_id
AUDIT: SESSION,READ,SELECT,,,sql
AUDIT: OBJECT,READ,SELECT,TABLE,public.account,sql
AUDIT: OBJECT,READ,SELECT,TABLE,public.account_role_map,sql

  SQL: update account
  set description = 'yada, yada';
AUDIT: SESSION,WRITE,UPDATE,,,sql

  SQL: update account
  set description = 'yada, yada'
where password = 'HASH2';
AUDIT: SESSION,WRITE,UPDATE,,,sql
AUDIT: OBJECT,WRITE,UPDATE,TABLE,public.account,sql

That about covers it.  I'd be happy to create a pull request to contribute the 
code back to 2ndQuadrant.  There are some things I'm still planning to do, but 
I think this draft looks promising.  pgaudit.c is attached.

Thoughts and suggestions are welcome.

-- - David Steele da...@pgmasters.net

/*
 * pgaudit/pgaudit.c
 *
 * Copyright © 2014-2015, PostgreSQL Global Development Group
 *
 * Permission to use, copy, modify, and distribute this software and
 * its documentation for any purpose, without fee, and without a
 * written agreement is hereby granted, provided that the above
 * copyright notice and this paragraph and the following two
 * paragraphs appear in all copies.
 *
 * IN NO 

Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-02-02 Thread David Steele
On 1/27/15 4:08 AM, Abhijit Menon-Sen wrote:

 Anyway, I think it's reasonably clear now that pgaudit is unlikely to
 make it into 9.5 in any form, so I'll find something else to do.

That's unfortunate.  I've been following this thread for a while with
some interest (and anticipation).

The role-base approach being considered may strike some as a misuse of
the role system, but to my eye it is syntactically very close to how
Oracle does auditing prior to 12c.  Say you wanted to audit selects on
the table hr.employee:

Oracle: AUDIT SELECT ON hr.employee;
pgaudit: GRANT SELECT ON hr.employee TO audit; (assuming audit is the
role defined by pgaudit.roles)

Object-based auditing in Oracle would be very easy to migrate to the
grants needed for pgaudit.  In addition, if an AUDIT command were
introduced later in core, it would be easy to migrate from pgaudit to
AUDIT assuming the syntax was similar to grant, which seems plausible.

Unified auditing in 12c brings together the AUDIT command and DBMS_FGA
under the concept of audit polices.  That type of granularity might be
something to shoot for eventually, but I think having a way to do
auditing similar to what is available in pre-12c covers most use cases
and would certainly be a big step forward for Postgres.

-- 
- David Steele
da...@pgmasters.net




signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-27 Thread Abhijit Menon-Sen
At 2015-01-26 17:45:52 -0500, robertmh...@gmail.com wrote:

  Based on the recent emails, it appears there's been a shift of
  preference to having it be in-core […]
 
 Well, I'm not sure that anyone else here agreed with me on that

Sure, an in-core AUDIT command would be great. Stephen has always said
that would be the most preferable solution; and if we had the code to
implement it, I doubt anyone would prefer the contrib module instead.
But we don't have that code now, and we won't have it in time for 9.5.

We had an opportunity to work on pgaudit in its current form, and I like
to think that the result is useful. To me, the question has always been
whether some variant of that code would be acceptable for 9.5's contrib.
If so, I had some time to work on that. If not… well, hard luck. But the
option to implement AUDIT was not available to me, which is why I have
not commented much on it recently.

 The basic dynamic here seems to be you asking for changes and Abhijit
 making them but without any real confidence, and I don't feel good
 about that. 

I understand how I might have given you that impression, but I didn't
mean to, and I don't really feel that way.

I appreciate Stephen's suggestions and, although it took me some time to
understand them fully, I think the use of GRANT to provide finer-grained
auditing configuration has improved pgaudit. I am slightly concerned by
the resulting complexity, but I think that can be addressed by examples
and so on. I wouldn't be unhappy if this code were to go into contrib.

(I should point out that it is also not the case that I do not hold any
opinions and would be happy with anything pgaudit-shaped being included.
For example, I strongly prefer GRANT to the 'alice:*:*' approach.)

Anyway, I think it's reasonably clear now that pgaudit is unlikely to
make it into 9.5 in any form, so I'll find something else to do.

-- Abhijit


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-27 Thread Robert Haas
On Tue, Jan 27, 2015 at 5:55 PM, Jim Nasby jim.na...@bluetreble.com wrote:
 BTW, I know that at least earlier versions of EnterpriseDB's version of
 Postgres (circa 2007) had an auditing feature. I never dealt with any
 customers who were using it when I was there, but perhaps some other folks
 could shed some light on what customers wanted to see an an auditing
 feature... (I'm looking at you, Jimbo!)

It's still there, but it's nothing like pgaudit.  What I hear is that
our customers are looking for something that has the capabilities of
DBMS_FGA.  I haven't researched either that or pgaudit enough to know
how similar they are.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-27 Thread Robert Haas
On Tue, Jan 27, 2015 at 6:54 PM, Jim Nasby jim.na...@bluetreble.com wrote:
 On 1/27/15 5:06 PM, Robert Haas wrote:

 On Tue, Jan 27, 2015 at 5:55 PM, Jim Nasby jim.na...@bluetreble.com
 wrote:

 BTW, I know that at least earlier versions of EnterpriseDB's version of
 Postgres (circa 2007) had an auditing feature. I never dealt with any
 customers who were using it when I was there, but perhaps some other
 folks
 could shed some light on what customers wanted to see an an auditing
 feature... (I'm looking at you, Jimbo!)


 It's still there, but it's nothing like pgaudit.  What I hear is that
 our customers are looking for something that has the capabilities of
 DBMS_FGA.  I haven't researched either that or pgaudit enough to know
 how similar they are.


 Do they really need the full capabilities of DBMS_FGA? At first glance, it
 looks even more sophisticated than anything that's been discussed so far. To
 wit (from
 http://docs.oracle.com/cd/B19306_01/appdev.102/b14258/d_fga.htm#CDEIECAG):

 DBMS_FGA.ADD_POLICY(
object_schema  VARCHAR2,
object_nameVARCHAR2,
policy_nameVARCHAR2,
audit_conditionVARCHAR2,
audit_column   VARCHAR2,
handler_schema VARCHAR2,
handler_module VARCHAR2,
enable BOOLEAN,
statement_typesVARCHAR2,
audit_trailBINARY_INTEGER IN DEFAULT,
audit_column_opts  BINARY_INTEGER IN DEFAULT);

I don't know.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-27 Thread Jim Nasby

On 1/26/15 4:45 PM, Robert Haas wrote:

On Mon, Jan 26, 2015 at 5:21 PM, Stephen Frost sfr...@snowman.net wrote:

I don't disagree with you about any of that.  I don't think you disagree
with my comment either.  What I'm not entirely clear on is how consensus
could be reached.  Leaving it dormant for the better part of a year
certainly doesn't appear to have helped that situation.  We've discussed
having it be part of the main server and having it be a contrib module
and until about a week ago, I had understood that having it in contrib
would be preferrable.  Based on the recent emails, it appears there's
been a shift of preference to having it be in-core, but clearly there's
no time left to do that in this release cycle.


Well, I'm not sure that anyone else here agreed with me on that, and
one person does not a consensus make no matter who it is.  The basic
problem here is that we don't seem to have even two people here who
agree on how this ought to be done.  The basic dynamic here seems to
be you asking for changes and Abhijit making them but without any real
confidence, and I don't feel good about that.  I'm willing to defer to
an emerging consensus here when there is one, but what Abhijit likes
best is not a consensus, and neither is what you like, and neither is
what I like.  What we need is some people agreeing with each other.


BTW, I know that at least earlier versions of EnterpriseDB's version of 
Postgres (circa 2007) had an auditing feature. I never dealt with any customers 
who were using it when I was there, but perhaps some other folks could shed 
some light on what customers wanted to see an an auditing feature... (I'm 
looking at you, Jimbo!)
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-27 Thread Jim Nasby

On 1/27/15 5:06 PM, Robert Haas wrote:

On Tue, Jan 27, 2015 at 5:55 PM, Jim Nasby jim.na...@bluetreble.com wrote:

BTW, I know that at least earlier versions of EnterpriseDB's version of
Postgres (circa 2007) had an auditing feature. I never dealt with any
customers who were using it when I was there, but perhaps some other folks
could shed some light on what customers wanted to see an an auditing
feature... (I'm looking at you, Jimbo!)


It's still there, but it's nothing like pgaudit.  What I hear is that
our customers are looking for something that has the capabilities of
DBMS_FGA.  I haven't researched either that or pgaudit enough to know
how similar they are.


Do they really need the full capabilities of DBMS_FGA? At first glance, it 
looks even more sophisticated than anything that's been discussed so far. To 
wit (from 
http://docs.oracle.com/cd/B19306_01/appdev.102/b14258/d_fga.htm#CDEIECAG):

DBMS_FGA.ADD_POLICY(
   object_schema  VARCHAR2,
   object_nameVARCHAR2,
   policy_nameVARCHAR2,
   audit_conditionVARCHAR2,
   audit_column   VARCHAR2,
   handler_schema VARCHAR2,
   handler_module VARCHAR2,
   enable BOOLEAN,
   statement_typesVARCHAR2,
   audit_trailBINARY_INTEGER IN DEFAULT,
   audit_column_opts  BINARY_INTEGER IN DEFAULT);
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-26 Thread Jim Nasby

On 1/23/15 2:15 PM, Stephen Frost wrote:

 I happen to like the idea specifically because it would allow regular
 roles to change the auditing settings (no need to be a superuser or to
 be able to modify postgresql.conf/postgresql.auto.conf)


Is there really a use case for non-superusers to be able to change auditing 
config? That seems like a bad idea.

What's a bad idea is having every auditor on the system running around
as superuser..


When it comes to looking at auditing data, I agree.

When it comes to changing auditing settings, I think that needs to be very 
restrictive. Really, it should be more (or differently) restrictive than SU, so 
that you can effectively audit your superusers with minimal worries about 
superusers tampering with auditing.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-26 Thread Robert Haas
On Fri, Jan 23, 2015 at 1:16 PM, Stephen Frost sfr...@snowman.net wrote:
 Well, I'm still of the view that there's little to lose by having this
 remain out-of-core for a release or three.  We don't really all agree
 on what we want, and non-core code can evolve a lot faster than core
 code, so what's the rush?

 Being out of core makes it unusable in production environments for a
 large number of organizations. :/

Tough.  Once we accept something into core, we're stuck maintaining it
forever.  We shouldn't do that unless we're absolutely confident the
design is solid. We are not close to having consensus on this.  If
somebody has a reason for accepting only core PostgreSQL and not
add-ons, it's either a stupid reason, or it's because they believe
that the core stuff will be better thought-out than the add-ons.  If
it's the latter, we shouldn't disappoint.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-26 Thread Robert Haas
On Mon, Jan 26, 2015 at 5:21 PM, Stephen Frost sfr...@snowman.net wrote:
 I don't disagree with you about any of that.  I don't think you disagree
 with my comment either.  What I'm not entirely clear on is how consensus
 could be reached.  Leaving it dormant for the better part of a year
 certainly doesn't appear to have helped that situation.  We've discussed
 having it be part of the main server and having it be a contrib module
 and until about a week ago, I had understood that having it in contrib
 would be preferrable.  Based on the recent emails, it appears there's
 been a shift of preference to having it be in-core, but clearly there's
 no time left to do that in this release cycle.

Well, I'm not sure that anyone else here agreed with me on that, and
one person does not a consensus make no matter who it is.  The basic
problem here is that we don't seem to have even two people here who
agree on how this ought to be done.  The basic dynamic here seems to
be you asking for changes and Abhijit making them but without any real
confidence, and I don't feel good about that.  I'm willing to defer to
an emerging consensus here when there is one, but what Abhijit likes
best is not a consensus, and neither is what you like, and neither is
what I like.  What we need is some people agreeing with each other.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-26 Thread Stephen Frost
Robert,

* Robert Haas (robertmh...@gmail.com) wrote:
 On Fri, Jan 23, 2015 at 1:16 PM, Stephen Frost sfr...@snowman.net wrote:
  Well, I'm still of the view that there's little to lose by having this
  remain out-of-core for a release or three.  We don't really all agree
  on what we want, and non-core code can evolve a lot faster than core
  code, so what's the rush?
 
  Being out of core makes it unusable in production environments for a
  large number of organizations. :/
 
 Tough.  Once we accept something into core, we're stuck maintaining it
 forever.  We shouldn't do that unless we're absolutely confident the
 design is solid. We are not close to having consensus on this.  If
 somebody has a reason for accepting only core PostgreSQL and not
 add-ons, it's either a stupid reason, or it's because they believe
 that the core stuff will be better thought-out than the add-ons.  If
 it's the latter, we shouldn't disappoint.

I don't disagree with you about any of that.  I don't think you disagree
with my comment either.  What I'm not entirely clear on is how consensus
could be reached.  Leaving it dormant for the better part of a year
certainly doesn't appear to have helped that situation.  We've discussed
having it be part of the main server and having it be a contrib module
and until about a week ago, I had understood that having it in contrib
would be preferrable.  Based on the recent emails, it appears there's
been a shift of preference to having it be in-core, but clearly there's
no time left to do that in this release cycle.

While I appreciate that it doesn't quite feel right to use the GRANT
system in the way I'm suggesting, I'm of the opinion that it's mostly
due to a lack of implementation with documentation and examples about
how it would work.  Using the GRANT system gives us nearly the best of
both worlds- an SQL-based syntax, a very fine-grained configuration
method, dependency handling, rename handling, and means you don't need
to be a superuser or have to restart the server to change the config,
nor is there any need to deal with CSV configuration via GUCs.

For my part, I'd still prefer an in-core system with top-level syntax
(eg: AUDIT), but that could clearly come later even if we included
pgaudit today (as Tom and others pointed out to me early this past
summer).  Auditing should definitely be a top-level capability in PG and
shouldn't be relegated to external modules or commercial solutions.
I've come around to feel that perhaps the first step should be a contrib
module rather than going in-core from the beginning as we'd get the
feedback which could lead to a better in-core solution.  I don't think
we're going to get it perfectly right the first time, either way, and
having it be completely outside the tree will mean we won't get any real
feedback on it.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-26 Thread Stephen Frost
Jim,

* Jim Nasby (jim.na...@bluetreble.com) wrote:
 When it comes to changing auditing settings, I think that needs to be very 
 restrictive. Really, it should be more (or differently) restrictive than SU, 
 so that you can effectively audit your superusers with minimal worries about 
 superusers tampering with auditing.

I continue to be of the opinion that you're not going to be able to
effectively audit your superusers with any mechanism that resides inside
of the process space which superusers control.  If you want to audit
superusers, you need something that operates outside of the postgres
process space.  I'm certainly interested in that, but it's an orthogonal
discussion to anything we're talking about here.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-23 Thread Robert Haas
On Wed, Jan 21, 2015 at 1:42 AM, Abhijit Menon-Sen a...@2ndquadrant.com wrote:
 At 2015-01-20 20:36:39 -0500, robertmh...@gmail.com wrote:
 I think this is throwing the baby out with the bathwater.  Neither C
 functions nor all-or-nothing are going to be of any practical use.

 Do you see some approach that has a realistic chance of making 9.5 and
 would also actually be worth putting into 9.5? Suggestions very welcome.

Well, I'm still of the view that there's little to lose by having this
remain out-of-core for a release or three.  We don't really all agree
on what we want, and non-core code can evolve a lot faster than core
code, so what's the rush?

I'm coming around to the view that we're going to need fairly deep
integration to make this work nicely.  It seems to me that the natural
way of controlling auditing of a table is with table or column options
on that table, but that requires either an extensible reloptions
framework that we don't have today, or that it be in the core server.
Similarly, the nice way of controlling options on a user is some
property attached to the user: audit operations X, Y, Z when performed
by this user.

If you held a gun to my head and said, we must have something this
release, I'd probably go for a GUC with a value that is a
comma-separated list of role:operation:object triplets, like this:

audit_stuff = 'alice:*:*, *:delete:*, bob:*:table secret'

...read as:

- Audit everything alice does.
- Audit all deletes by anyone.
- Audit all access by bob to table secret.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-23 Thread Robert Haas
On Wed, Jan 21, 2015 at 7:05 PM, Jim Nasby jim.na...@bluetreble.com wrote:
 I'm still nervous about overloading this onto the roles system; I think it
 will end up being very easy to accidentally break. But if others think it'll
 work then I guess I'm just being paranoid.

I agree with you.  I don't hear anyone who actually likes that
proposal except for Stephen.  The list of people who don't like it
seems to include the patch author, which strikes me as a rather
significant point.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-23 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote:
 On Wed, Jan 21, 2015 at 7:05 PM, Jim Nasby jim.na...@bluetreble.com wrote:
  I'm still nervous about overloading this onto the roles system; I think it
  will end up being very easy to accidentally break. But if others think it'll
  work then I guess I'm just being paranoid.

 I agree with you.  I don't hear anyone who actually likes that
 proposal except for Stephen.  The list of people who don't like it
 seems to include the patch author, which strikes me as a rather
 significant point.

Just to clarify- this concept isn't actually mine but was suggested by a
pretty sizable PG user who has a great deal of familiarity with other
databases.  I don't mean to try and invoke the 'silent majority' but
rather to make sure folks don't think this is my idea alone or that it's
only me who thinks it makes sense. :)  Simon had weighed in earlier
with, iirc, a comment that he thought it was a good approach also,
though that was a while ago and things have changed.

I happen to like the idea specifically because it would allow regular
roles to change the auditing settings (no need to be a superuser or to
be able to modify postgresql.conf/postgresql.auto.conf), it would
provide the level of granularity desired, would follow table, column,
role changes, renames, drops, recreations, the dependency system would
function as expected, etc, while keeping it as an extension, which I
understood to be pretty desirable, especially initially.

* Robert Haas (robertmh...@gmail.com) wrote:
 On Wed, Jan 21, 2015 at 1:42 AM, Abhijit Menon-Sen a...@2ndquadrant.com 
 wrote:
  At 2015-01-20 20:36:39 -0500, robertmh...@gmail.com wrote:
  I think this is throwing the baby out with the bathwater.  Neither C
  functions nor all-or-nothing are going to be of any practical use.
 
  Do you see some approach that has a realistic chance of making 9.5 and
  would also actually be worth putting into 9.5? Suggestions very welcome.

 Well, I'm still of the view that there's little to lose by having this
 remain out-of-core for a release or three.  We don't really all agree
 on what we want, and non-core code can evolve a lot faster than core
 code, so what's the rush?

Being out of core makes it unusable in production environments for a
large number of organizations. :/

 I'm coming around to the view that we're going to need fairly deep
 integration to make this work nicely.  It seems to me that the natural
 way of controlling auditing of a table is with table or column options
 on that table, but that requires either an extensible reloptions
 framework that we don't have today, or that it be in the core server.
 Similarly, the nice way of controlling options on a user is some
 property attached to the user: audit operations X, Y, Z when performed
 by this user.

This is basically the same position which I held about a year ago when
we were discussing this, but there was quite a bit of push-back on
having an in-core solution, from the additional catalog tables to making
the grammar larger and slowing things down.  For my 2c, auditing is more
than valuable enough to warrant those compromises, but I'm not anxious
to spend time developing an in-core solution only to get it shot down
after all the work has been put into it.

Still, if folks have come around to feeling like an in-core solution
makes sense, I'm certainly willing to work towards making that happen;
it's, after all, what I've been interested in for years. :)

 If you held a gun to my head and said, we must have something this
 release, I'd probably go for a GUC with a value that is a
 comma-separated list of role:operation:object triplets, like this:

 audit_stuff = 'alice:*:*, *:delete:*, bob:*:table secret'

 ...read as:

 - Audit everything alice does.
 - Audit all deletes by anyone.
 - Audit all access by bob to table secret.

And this approach doesn't address any of the issues mentioned above,
unfortunately, which would make it pretty difficult to really use.  I
think I'd rather deal with pgaudit being outside of the tree than pursue
an approach which has so many issues.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-23 Thread Jim Nasby

On 1/21/15 6:50 PM, Stephen Frost wrote:

I'm still nervous about overloading this onto the roles system; I think it will 
end up being very easy to accidentally break. But if others think it'll work 
then I guess I'm just being paranoid.

Break in which way..?  If you're saying it'll be easy for a user to
misconfigure then I might agree with you- but documentation and
examples can help to address that.


I'm worried about user misconfiguration. Setting up a good system of roles (as 
in, distinguishing between application accounts, users, account(s) used to 
deploy code changes, DBAs, etc) is already tricky because of all the different 
use cases to consider. I fear that adding auditing to that matrix is just going 
to make it worse.

I do like Robert's idea of role:action:object triplets more, though I'm not 
sure it's enough. For example, what happens if you

CREATE ROLE su SUPERUSER NOINHERIT NOLOGIN;
CREATE ROLE su_role IN ROLE su NOLOGIN;
GRANT su_role TO bob;

and have

su_role:*:*

Does bob get audited all the time then? Only when he does SET ROLE su? For that 
matter, how does SET ROLE affect auditing?
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-23 Thread Stephen Frost
* Jim Nasby (jim.na...@bluetreble.com) wrote:
 On 1/21/15 6:50 PM, Stephen Frost wrote:
 I'm still nervous about overloading this onto the roles system; I think it 
 will end up being very easy to accidentally break. But if others think 
 it'll work then I guess I'm just being paranoid.
 Break in which way..?  If you're saying it'll be easy for a user to
 misconfigure then I might agree with you- but documentation and
 examples can help to address that.
 
 I'm worried about user misconfiguration. Setting up a good system of roles 
 (as in, distinguishing between application accounts, users, account(s) used 
 to deploy code changes, DBAs, etc) is already tricky because of all the 
 different use cases to consider. I fear that adding auditing to that matrix 
 is just going to make it worse.

Even with an in-core solution, users would need to work out who should
be able to configure auditing..  I agree that seeing the permission
grants to the auditing roles might be confusing for folks who have not
seen it before, but I think that'll quickly resolve itself since the
only people who would see that are those who want to use pgaudit...

 I do like Robert's idea of role:action:object triplets more, though I'm not 
 sure it's enough. For example, what happens if you

I'd suggest considering what happens if you:

ALTER ROLE su_role RENAME TO new_su_role;

Or if you want to grant a non-superuser the ability to modify the
auditing rules..

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-23 Thread Jim Nasby

On 1/23/15 12:16 PM, Stephen Frost wrote:

Just to clarify- this concept isn't actually mine but was suggested by a
pretty sizable PG user who has a great deal of familiarity with other
databases.  I don't mean to try and invoke the 'silent majority' but
rather to make sure folks don't think this is my idea alone or that it's
only me who thinks it makes sense.:)   Simon had weighed in earlier
with, iirc, a comment that he thought it was a good approach also,
though that was a while ago and things have changed.


I know there's definitely demand for auditing. I'd love to see us support it.


I happen to like the idea specifically because it would allow regular
roles to change the auditing settings (no need to be a superuser or to
be able to modify postgresql.conf/postgresql.auto.conf)


Is there really a use case for non-superusers to be able to change auditing 
config? That seems like a bad idea.

Also, was there a solution to how to configure auditing on specific objects 
with a role-based mechanism? I think we really do need something akin to 
role:action:object tuples, and I don't see how to do that with roles alone.

BTW, I'm starting to feel like this needs a wiki page to get the design pulled 
together.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-23 Thread Stephen Frost
* Jim Nasby (jim.na...@bluetreble.com) wrote:
 On 1/23/15 12:16 PM, Stephen Frost wrote:
 Just to clarify- this concept isn't actually mine but was suggested by a
 pretty sizable PG user who has a great deal of familiarity with other
 databases.  I don't mean to try and invoke the 'silent majority' but
 rather to make sure folks don't think this is my idea alone or that it's
 only me who thinks it makes sense.:)   Simon had weighed in earlier
 with, iirc, a comment that he thought it was a good approach also,
 though that was a while ago and things have changed.
 
 I know there's definitely demand for auditing. I'd love to see us support it.
 
 I happen to like the idea specifically because it would allow regular
 roles to change the auditing settings (no need to be a superuser or to
 be able to modify postgresql.conf/postgresql.auto.conf)
 
 Is there really a use case for non-superusers to be able to change auditing 
 config? That seems like a bad idea.

What's a bad idea is having every auditor on the system running around
as superuser..

 Also, was there a solution to how to configure auditing on specific objects 
 with a role-based mechanism? I think we really do need something akin to 
 role:action:object tuples, and I don't see how to do that with roles alone.

That is supported with the grant-based proposal.

 BTW, I'm starting to feel like this needs a wiki page to get the design 
 pulled together.

I agree with that and was planning to offer help with the documentation
and building of such a wiki with examples, etc, once the implementation
was far enough along to demonstrate that the design will actually work..

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-21 Thread Stephen Frost
* Jim Nasby (jim.na...@bluetreble.com) wrote:
 On 1/21/15 5:38 PM, Stephen Frost wrote:
 Being startup-only won't help if the user is a superuser.
 
 Crap, I thought postgresql.auto.conf was handled as an #include and therefore 
 you could still preempt it via postgresql.conf

It's not just that..  Having superuser access should really be
considered equivilant to having a shell as the unix user that postgres
is running as.

 If this is being done for every execution of a query then I agree- SQL
 or plpgsql probably wouldn't be fast enough.  That doesn't mean it makes
 sense to have pgaudit support calling a C function, it simply means that
 we need to find another way to configure auditing (which is what I think
 I've done...).
 
 I'm still nervous about overloading this onto the roles system; I think it 
 will end up being very easy to accidentally break. But if others think it'll 
 work then I guess I'm just being paranoid.

Break in which way..?  If you're saying it'll be easy for a user to
misconfigure then I might agree with you- but documentation and
examples can help to address that.  If you're worried that future PG
hacking will break it, well, I tend to doubt the GRANT piece is the area
of concern there- the recent development work is really around event
triggers and adding new object classes; the GRANT components have been
reasonably stable for the past few years.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-21 Thread Abhijit Menon-Sen
[After a discussion on IRC with Stephen…]

At 2015-01-20 21:47:02 -0500, sfr...@snowman.net wrote:

 Review the opening of this email though and consider that we could
 look at what privileges has the audit role granted to the current
 role? as defining what is to be audited.

Right, I understand now how that would work. I'll try to find time to
(a) implement this, (b) remove the backwards-compatibility code, and
(c) split up the USE_DEPARSE_FUNCTIONS stuff.

  For example, what if I want to see all the tables created and
  dropped by a particular user?
 
 I hadn't been intending to address that with this scheme, but I think
 we have that by looking for privilege grants where the audit role is
 the grantee and the role-to-be-audited the grantor.

For CREATE, yes, with a bit of extra ACL-checking code in the utility
hook; but I don't think we'll get very far without the ability to log
ALTER/DROP too. :-) So there has to be some alternative mechanism for
that, and I'm hoping Robert (or anyone!) has something in mind.

 Well, I was primairly digging for someone to say yes, the object
 access stuff will be reworked to be based on event triggers, thus
 removing the need for the object access bits.

(This doesn't entirely make sense to me, but it's tangential anyway, so
I won't comment on it for now.)

-- Abhijit


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-21 Thread Stephen Frost
* Abhijit Menon-Sen (a...@2ndquadrant.com) wrote:
 At 2015-01-20 21:47:02 -0500, sfr...@snowman.net wrote:
  Review the opening of this email though and consider that we could
  look at what privileges has the audit role granted to the current
  role? as defining what is to be audited.
 
 Right, I understand now how that would work. I'll try to find time to
 (a) implement this, (b) remove the backwards-compatibility code, and
 (c) split up the USE_DEPARSE_FUNCTIONS stuff.

Great!  Thanks!

   For example, what if I want to see all the tables created and
   dropped by a particular user?
  
  I hadn't been intending to address that with this scheme, but I think
  we have that by looking for privilege grants where the audit role is
  the grantee and the role-to-be-audited the grantor.
 
 For CREATE, yes, with a bit of extra ACL-checking code in the utility
 hook; but I don't think we'll get very far without the ability to log
 ALTER/DROP too. :-) So there has to be some alternative mechanism for
 that, and I'm hoping Robert (or anyone!) has something in mind.

ALTER/DROP can be logged based on the USAGE privilege for the schema.
We can't differentiate those cases but we can at least handle them as a
group.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-20 Thread Robert Haas
On Tue, Jan 20, 2015 at 1:05 AM, Abhijit Menon-Sen a...@2ndquadrant.com wrote:
 So when I'm trying to decide what to audit, I have to:

 (a) check if the current user is mentioned in .roles; if so, audit.

 (b) check if the current user is a descendant of one of the roles
 mentioned in .roles; if not, no audit.

 (c) check for permissions granted to the root role and see if that
 tells us to audit.

 Is that right? If so, it would work fine. I don't look forward to trying
 to explain it to people, but yes, it would work (for anything you could
 grant permissions for).

I think this points to fundamental weakness in the idea of doing this
through the GRANT system.  Some people are going want to audit
everything a particular user does, some people are going to want to
audit all access to particular objects, and some people will have more
complicated requirements.  Some people will want to audit even
super-users, others especially super-users, others only non
super-users.  None of this necessarily matches up to the usual
permissions framework.

 Have you considered splitting pgaudit.c into multiple files, perhaps
 along the pre/post deparse lines?

 If such a split were done properly, then could the backwards-compatible
 version be more acceptable for inclusion in contrib in 9.5? If so, I'll
 look into it.

We're not going to include code in contrib that has leftovers in it
for compatibility with earlier source trees.  That's been discussed on
this mailing list many times and the policy is clear.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-20 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote:
 On Tue, Jan 20, 2015 at 1:05 AM, Abhijit Menon-Sen a...@2ndquadrant.com 
 wrote:
  So when I'm trying to decide what to audit, I have to:
 
  (a) check if the current user is mentioned in .roles; if so, audit.
 
  (b) check if the current user is a descendant of one of the roles
  mentioned in .roles; if not, no audit.
 
  (c) check for permissions granted to the root role and see if that
  tells us to audit.
 
  Is that right? If so, it would work fine. I don't look forward to trying
  to explain it to people, but yes, it would work (for anything you could
  grant permissions for).
 
 I think this points to fundamental weakness in the idea of doing this
 through the GRANT system.  Some people are going want to audit
 everything a particular user does, some people are going to want to
 audit all access to particular objects, and some people will have more
 complicated requirements.  Some people will want to audit even
 super-users, others especially super-users, others only non
 super-users.  None of this necessarily matches up to the usual
 permissions framework.

I'm hopeful that my email from a few minutes ago provides a way to cover
all of the above, while still making it easy for users who just want to
say audit everything this user does or audit everything which touches
this column.

Any auditing of superusers is going to be circumventable by those same
superusers if it's happening in the context of the PG process, so I'm
not sure why they would be any different under this scheme vs. some
other scheme.

Don't get me wrong- I agree completely that using the GRANT system isn't
ideal, but I don't see anyone proposing another way for an extension to
store metadata on catalog tables with the same granularity the GRANT
system provides and its dependency handling and ability to have default
values.  We've been over that ground a number of times and I certainly
don't feel like we're any closer to solving it and I'd hate to see that
block pgaudit.

Put another way, if the requirement for pgaudit is that it has to solve
the metadata-on-catalogs-for-extensions problem, I think I'd rather just
move pgaudit into core instead, give it catalog tables, make it part of
the dependency system, provide syntax for it, etc.  I'm pretty sure
that'd be easier and happen a lot faster.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-20 Thread Robert Haas
On Tue, Jan 20, 2015 at 5:03 PM, Jim Nasby jim.na...@bluetreble.com wrote:
 +1. In particular I'm very concerned with the idea of doing this via roles,
 because that would make it trivial for any superuser to disable auditing.
 The only good option I could see to provide this kind of flexibility would
 be allowing the user to provide a function that accepts role, object, etc
 and make return a boolean. The performance of that would presumably suck
 with anything but a C function, but we could provide some C functions to
 handle simple cases.

 That said, I think the best idea at this stage is either log everything or
 nothing. We can always expand upon that later.

I think this is throwing the baby out with the bathwater.  Neither C
functions nor all-or-nothing are going to be of any practical use.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-20 Thread Stephen Frost
Abhijit,

* Abhijit Menon-Sen (a...@2ndquadrant.com) wrote:
 At 2015-01-19 08:26:59 -0500, sfr...@snowman.net wrote:
  I'm confused by this statement..
 
 Let me see if I've understood your clarification:

Thanks much for the example use-case and for working this through with
me.  I actually think I've come up with a further specification which
might allow us to make this extremely flexible, but also simple for
those who want to keep it simple.

Consider this:

Everything is single-level to the roles mentioned in the GUC.

Is the logged in role a member of one of the GUC roles?
  Yes - Audit

Now to cover the user X for table Y case:

Did any of the GUC value roles grant SELECT rights for this table to the
current role?
  Yes - Audit SELECT on the table by the current role

Did any of the GUC value roles grant INSERT rights for this table to the
current role?
  Yes - Audit INSERT on the table by the current role

etc.

For the 'log all access to an object' case, under this scheme, I'm
afraid we'd need some special role to GRANT the access to.  We wouldn't
want that to simply be 'public' since then it might actually be
granted access rights that we don't want to.  We can't simply use the
same role because you need to grant that role whatever access 'with
grant option' in order for it to be able to re-grant the privilege.

With the special role, it becomes:

Does the special role have SELECT rights on the table?
  Yes - Audit SELECTs on the table

Does the special role have INSERT rights on the table?
  Yes - Audit INSERTs on the table

 Suppose you have pgaudit.roles set to 'r0, r1', and that you have
 granted r0 to u0.

Not quite- I wasn't thinking you'd grant r0 to u0 but rather the other
way around- u0 is granted to r0.  If you granted r0 to u0, then u0 would
have all of r0's rights which could be quite a bit larger than you want
u0 to have.  It only works in the other direction.

 Now, if you're connected to the database as r0 or r1, then everything
 you do is logged. But if you're connected as u0, then only those things
 are logged that can be derived (in a manner discussed later) from the
 permissions explicitly granted to r0 (but not u0)?

 So when I'm trying to decide what to audit, I have to:
 
 (a) check if the current user is mentioned in .roles; if so, audit.
 
 (b) check if the current user is a descendant of one of the roles
 mentioned in .roles; if not, no audit.
 
 (c) check for permissions granted to the root role and see if that
 tells us to audit.
 
 Is that right? If so, it would work fine. I don't look forward to trying
 to explain it to people, but yes, it would work (for anything you could
 grant permissions for).

This is pretty close- (a) and (b) are mostly correct, though I would
strongly discourage users from actually logging in as an audit role.
The one caveat with (b) is that 'if not, no audit' is not correct- all
cases are essentially OR'd together when it comes to auditing.  Roles
can be audited even if they are not descendants of the roles mentioned
in .roles.

Review the opening of this email though and consider that we could look
at what privileges has the audit role granted to the current role? as
defining what is to be audited.

  You can't say that you want r1 to have X actions logged, with r2
  having Y actions logged.
 
 Right. But how do you do that for non-GRANT-able actions in your scheme?
 For example, what if I want to see all the tables created and dropped by
 a particular user?

I hadn't been intending to address that with this scheme, but I think we
have that by looking for privilege grants where the audit role is the
grantee and the role-to-be-audited the grantor.

  Have you considered splitting pgaudit.c into multiple files, perhaps
  along the pre/post deparse lines?
 
 If such a split were done properly, then could the backwards-compatible
 version be more acceptable for inclusion in contrib in 9.5? If so, I'll
 look into it.

As Robert says, the short answer is 'no'- but it might make it easier to
get the 9.5 bits into 9.5.. :)

  The key part above is any.  We're using the grant system as a proxy
  for saying I want to audit column X.  That's different from I want
  to audit commands which would be allowed if I *only* had access to
  column X.  If I want to audit access to column X, then:
  
  select A, B, X from mytable;
  
  Should be audited, even if the audit role doesn't have access to
  columns A or B.
 
 Yes, that's what the code already does (modulo handling of the audit
 role's oid, which I tweaked to match the behaviour described earlier
 in this mail). I did the following:
 
 create table x(a int,b int,c int);
 insert into x(a,b,c) values (1,2,3);
 
 create user y;
 grant select on x to y;
 
 create role x;
 grant select (a) on table x to x;
 grant x to y;
 
 Then, as user y, I did «select a,b,c from x» and «select b,c from x».
 Only the former was logged:
 
 LOG:  AUDIT,2015-01-20 
 

Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-20 Thread Stephen Frost
Jim,

* Jim Nasby (jim.na...@bluetreble.com) wrote:
 +1. In particular I'm very concerned with the idea of doing this via roles, 
 because that would make it trivial for any superuser to disable auditing. The 
 only good option I could see to provide this kind of flexibility would be 
 allowing the user to provide a function that accepts role, object, etc and 
 make return a boolean. The performance of that would presumably suck with 
 anything but a C function, but we could provide some C functions to handle 
 simple cases.

Superusers will be able to bypass, trivially, anything that's done in
the process space of PG.  The only possible exception to that being an
SELinux or similar solution, but I don't think that's what you were
getting at.

I certainly don't think having the user provide a C function to specify
what should be audited as making any sense- if they can do that, they
can use the same hooks pgaudit is using and skip the middle-man.  As for
the performance concern you raise, I actually don't buy into it at all.
It's not like we worry about the performance of checking permissions on
objects in general and, for my part, I like to think that's because it's
pretty darn quick already.

 That said, I think the best idea at this stage is either log everything or 
 nothing. We can always expand upon that later.

We've already got those options and they are, clearly, insufficient for
a large number of our users.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-20 Thread Jim Nasby

On 1/20/15 2:20 PM, Robert Haas wrote:

On Tue, Jan 20, 2015 at 1:05 AM, Abhijit Menon-Sena...@2ndquadrant.com  wrote:

So when I'm trying to decide what to audit, I have to:

 (a) check if the current user is mentioned in .roles; if so, audit.

 (b) check if the current user is a descendant of one of the roles
 mentioned in .roles; if not, no audit.

 (c) check for permissions granted to the root role and see if that
 tells us to audit.

Is that right? If so, it would work fine. I don't look forward to trying
to explain it to people, but yes, it would work (for anything you could
grant permissions for).

I think this points to fundamental weakness in the idea of doing this
through the GRANT system.  Some people are going want to audit
everything a particular user does, some people are going to want to
audit all access to particular objects, and some people will have more
complicated requirements.  Some people will want to audit even
super-users, others especially super-users, others only non
super-users.  None of this necessarily matches up to the usual
permissions framework.


+1. In particular I'm very concerned with the idea of doing this via roles, 
because that would make it trivial for any superuser to disable auditing. The 
only good option I could see to provide this kind of flexibility would be 
allowing the user to provide a function that accepts role, object, etc and make 
return a boolean. The performance of that would presumably suck with anything 
but a C function, but we could provide some C functions to handle simple cases.

That said, I think the best idea at this stage is either log everything or 
nothing. We can always expand upon that later.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-20 Thread Abhijit Menon-Sen
At 2015-01-20 20:36:39 -0500, robertmh...@gmail.com wrote:

 I think this is throwing the baby out with the bathwater.  Neither C
 functions nor all-or-nothing are going to be of any practical use.

Do you see some approach that has a realistic chance of making 9.5 and
would also actually be worth putting into 9.5? Suggestions very welcome.

-- Abhijit


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-19 Thread Abhijit Menon-Sen
At 2015-01-19 08:26:59 -0500, sfr...@snowman.net wrote:

 I'm confused by this statement..

Let me see if I've understood your clarification:

Suppose you have pgaudit.roles set to 'r0, r1', and that you have
granted r0 to u0.

Now, if you're connected to the database as r0 or r1, then everything
you do is logged. But if you're connected as u0, then only those things
are logged that can be derived (in a manner discussed later) from the
permissions explicitly granted to r0 (but not u0)?

So when I'm trying to decide what to audit, I have to:

(a) check if the current user is mentioned in .roles; if so, audit.

(b) check if the current user is a descendant of one of the roles
mentioned in .roles; if not, no audit.

(c) check for permissions granted to the root role and see if that
tells us to audit.

Is that right? If so, it would work fine. I don't look forward to trying
to explain it to people, but yes, it would work (for anything you could
grant permissions for).

 You can't say that you want r1 to have X actions logged, with r2
 having Y actions logged.

Right. But how do you do that for non-GRANT-able actions in your scheme?
For example, what if I want to see all the tables created and dropped by
a particular user?

 Have you considered splitting pgaudit.c into multiple files, perhaps
 along the pre/post deparse lines?

If such a split were done properly, then could the backwards-compatible
version be more acceptable for inclusion in contrib in 9.5? If so, I'll
look into it.

 One thought might be to provide the intersection between LOGSTMT and
 ObjectTypes.

Hm. OK.

 The key part above is any.  We're using the grant system as a proxy
 for saying I want to audit column X.  That's different from I want
 to audit commands which would be allowed if I *only* had access to
 column X.  If I want to audit access to column X, then:
 
 select A, B, X from mytable;
 
 Should be audited, even if the audit role doesn't have access to
 columns A or B.

Yes, that's what the code already does (modulo handling of the audit
role's oid, which I tweaked to match the behaviour described earlier
in this mail). I did the following:

create table x(a int,b int,c int);
insert into x(a,b,c) values (1,2,3);

create user y;
grant select on x to y;

create role x;
grant select (a) on table x to x;
grant x to y;

Then, as user y, I did «select a,b,c from x» and «select b,c from x».
Only the former was logged:

LOG:  AUDIT,2015-01-20 
11:19:02.156441+05:30,postgres,y,y,READ,SELECT,TABLE,public.x,select a,b,c from 
x;

 Yeah- but are we always going to have to deal with a partial event
 trigger set?

As a practical matter, yes. I don't know if there are current plans to
extend event trigger support to the commands and object types that are
yet unsupported.

-- Abhijit


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-19 Thread Stephen Frost
Abhijit,

* Abhijit Menon-Sen (a...@2ndquadrant.com) wrote:
 At 2015-01-18 22:28:37 -0500, sfr...@snowman.net wrote:
 
   2. Set pgaudit.roles = 'r1, r2, …', which logs everything done by r1,
  r2, and any of their descendants.
 
  If these roles were the 'audit' roles as considered under #1 then
  couldn't administrators control what pgaudit.roles provide today using
  role memberships granted to the roles listed?
 
 Yes. But then there would be no convenient way to say just log
 everything this particular role does.

I'm confused by this statement..  Having the role listed in
pgaudit.roles would still mean that everything that role does is logged.
Adding other roles to be audited would simply be 'GRANT audit TO
role1;'.

Is there a specific action which you don't think would be audited
through this?

   3. Set pgaudit.log = 'read, write, …', which logs any events in any
  of the listed classes.
  
  Approach #1 addresses this too, doesn't it?
 
 Approach #1 applies only to things you can grant permissions for. What
 if you want to audit other commands?

You can grant roles to other roles, which is how the role-level
auditing would be handled.  Hopefully that clarifies the idea here.

  As currently implemented, role-level auditing is required to have DDL
  be logged, but you may very well want to log all DDL and no DML, for
  example.
 
 Well, as formerly implemented, you could do this by adding r1 to .roles
 and then setting .log appropriately. But you know that already and, if
 I'm not mistaken, have said you don't like it.

Right, because it's not flexible.  You can't say that you want r1 to
have X actions logged, with r2 having Y actions logged.

 I'm all for making it possible to configure fine-grained auditing, but
 I don't think that should come at the expense of being able to whack
 things with a simpler, heavier^Wmore inclusive hammer if you want to.

I agree that it's valuable to support simple use-cases with simple
configurations.

  My feeling is that we should strip down what goes into contrib to be
  9.5 based.
 
 I do not think I can express a constructive opinion about this. I will
 go along with whatever people agree is best.

One of the issues that I see is just how much of the code is under the
#ifdef's.  If this is going into contrib, we really shouldn't have
references and large code blocks that are #ifdef'd out implementations
based on out-of-core patches.  Further, the pieces which are under the
#ifdef's for DEPARSE would likely end up having to be under #if
PG_VERSION_NUM, as the deparse tree goes into 9.5.  Really, pgaudit
pre-deparse and post-deparse are quite different and having them all in
one pgaudit.c ends up being pretty grotty.  Have you considered
splitting pgaudit.c into multiple files, perhaps along the pre/post
deparse lines?

  I'm also wondering if pieces of that are now out-of-date with where
  core is.
 
 Yes, they are. I'll clean that up.

Ok, good.

  I don't particularly like how pgaudit will need to be kept in sync
  with what's in ProcessUtility (normal and slow).
 
 Sure, it's a pain.
 
  I'd really like to see this additional information regarding what kind
  a command is codified in core.  Ideally, we'd have a single place
  which tracks the kind of command and possibly other information which
  can then be addressed all at once when new commands are added.
 
 What would this look like, and is it a realistic goal for 9.5?

One thought might be to provide the intersection between LOGSTMT and
ObjectTypes.  We can learn what commands are DDL and what are
modification commands from GetCommandLogLevel.  The other distinctions
are mostly about different object types and we might be able to use
ObjectPropertyType and ObjectTypeMap for that, perhaps adding another
'kind' to ObjectProperty for the object categorization.

There are still further distinctions and I agree that it's very useful
to be able to identify which commands are privilege-related
(T_GrantStmt, T_GrantRoleStmt, T_CreatePolicyStmt, etc) vs. things like
Vacuum.

  Also, we could allow more granularity by using the actual classes
  which we already have for objects.
 
 Explain?

That thought was based on looking at ObjectTypeMap- we might be able to
provide a way for administrators to configure which objects they want to
be audited by naming them using the names provided by ObjectTypeMap.

   /*
* This module collects AuditEvents from various sources (event
* triggers, and executor/utility hooks) and passes them to the
* log_audit_event() function.
*
* An AuditEvent represents an operation that potentially affects a
* single object. If an underlying command affects multiple objects,
* multiple AuditEvents must be created to represent it.
*/
  
  The above doesn't appear to be accurate (perhaps it once was?) as
  log_audit_event() only takes a single AuditEvent structure at a time
  and it's not a list.  I'm not sure that needs to change, just pointing
  out 

Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-18 Thread Stephen Frost
Abhijit,

Apologies, I've been meaning to go through this for quite some time.

* Abhijit Menon-Sen (a...@2ndquadrant.com) wrote:
 I've changed pgaudit to work as you suggested.

Great, glad to see that.

 A quick note on the implementation: pgaudit was already installing an
 ExecutorCheckPerms_hook anyway; I adapted code from ExecRTECheckPerms
 to check if the audit role has been granted any of the permissions
 required for the operation.

Sure, makes sense to me.

 This means there are three ways to configure auditing:
 
 1. GRANT … ON … TO audit, which logs any operations that correspond to
the granted permissions.

I was thinking we would be able to configure which role this applies to,
rather than hard-coding 'audit' as *the* role.  Perhaps with a new GUC,
or the existing 'pgaudit.roles' GUC could be used.  Further, this can be
extrapolated out to cover auditing of roles by checking for role
membership in this role, see below.

 2. Set pgaudit.roles = 'r1, r2, …', which logs everything done by r1,
r2, and any of their descendants.

If these roles were the 'audit' roles as considered under #1 then
couldn't administrators control what pgaudit.roles provide today using
role memberships granted to the roles listed?

 3. Set pgaudit.log = 'read, write, …', which logs any events in any of
the listed classes.

Approach #1 addresses this too, doesn't it?  SELECT vs. UPDATE vs.
DELETE, etc?  I'm not sure that this really adds much as it isn't nearly
as granular as the GRANT-based approach since it applies to everything.

One of the really big and interesting distinctions to consider between
checking permissions granted to an 'audit' role vs. role membership is
how DDL is handled.  As currently implemented, role-level auditing is
required to have DDL be logged, but you may very well want to log all
DDL and no DML, for example.  What would be really helpful is to develop
a wiki to cover common use-cases and how to set up pgaudit for them.

 (This is a small change from the earlier behaviour where, if a role was
 listed in .roles, it was still subject to the .log setting. I find that
 more useful in practice, but since we're discussing Stephen's proposal,
 I implemented what he said.)

Right- the idea is to provide a very granular way of specifying what is
to be logged; much more than what the previous pair of GUCs provided.

 The new pgaudit.c is attached here for review. Nothing else has changed
 from the earlier submission; and everything is in the github repository
 (github.com/2ndQuadrant/pgaudit).

There's a few big questions we need to address with pgaudit to have it
go into our repo.

The first is what major version(s) we're targetting.  My feeling is that
we should strip down what goes into contrib to be 9.5 based.  I
certainly understand that you want to support earlier versions but I
don't think it makes sense to try and carry that baggage in contrib when
it won't ever be released with earlier versions.

The second is the USE_DEPARSE_FUNCTIONS-related bits.  I realize that
there's a goal to get additional things into 9.5 from that branch but it
doesn't make sense for the contrib pgaudit to include those components
prior to them actually being in core.  I'm also wondering if pieces of
that are now out-of-date with where core is.  If those parts are going
to go into 9.5 soon (which looks like it may happen..) then hopefully we
can just remove the #define's and clean up the code to use the new
capabilities.

Lastly, I'll echo a concern which Robert has raised which is- how do we
make sure that new commands, etc, are covered?  I don't particularly
like how pgaudit will need to be kept in sync with what's in
ProcessUtility (normal and slow).  I'm actually a bit hopeful that we
can work out a way for pgaudit to help with this through a regression
test which loops over all objects and tests logging each of them.

One of the important aspects of auditing is that what is requested to be
audited is and exactly that- no more, no less.  Reviewing the code makes
me pretty nervous about that actually happening properly, but that's
mostly due to the back-and-forth between different major versions and
the #ifdef/#ifndef's.

Additional comments in-line below.

 enum LogClass {
   LOG_NONE = 0,
 
   /* SELECT */
   LOG_READ = (1  0),
 
   /* INSERT, UPDATE, DELETE, TRUNCATE */
   LOG_WRITE = (1  1),
 
   /* GRANT, REVOKE, ALTER … */
   LOG_PRIVILEGE = (1  2),
 
   /* CREATE/DROP/ALTER ROLE */
   LOG_USER = (1  3),
 
   /* DDL: CREATE/DROP/ALTER */
   LOG_DEFINITION = (1  4),
 
   /* DDL: CREATE OPERATOR etc. */
   LOG_CONFIG = (1  5),
 
   /* VACUUM, REINDEX, ANALYZE */
   LOG_ADMIN = (1  6),
 
   /* Function execution */
   LOG_FUNCTION = (1  7),
 
   /* Absolutely everything; not available via pgaudit.log */
   LOG_ALL = ~(uint64)0
 };

I'd really like to see this additional information regarding what kind
a command is codified in 

Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2015-01-18 Thread Abhijit Menon-Sen
Hello Stephen.

Thanks very much for having a look at the revised pgaudit.

At 2015-01-18 22:28:37 -0500, sfr...@snowman.net wrote:

  2. Set pgaudit.roles = 'r1, r2, …', which logs everything done by r1,
 r2, and any of their descendants.

 If these roles were the 'audit' roles as considered under #1 then
 couldn't administrators control what pgaudit.roles provide today using
 role memberships granted to the roles listed?

Yes. But then there would be no convenient way to say just log
everything this particular role does.

  3. Set pgaudit.log = 'read, write, …', which logs any events in any
 of the listed classes.
 
 Approach #1 addresses this too, doesn't it?

Approach #1 applies only to things you can grant permissions for. What
if you want to audit other commands?

 As currently implemented, role-level auditing is required to have DDL
 be logged, but you may very well want to log all DDL and no DML, for
 example.

Well, as formerly implemented, you could do this by adding r1 to .roles
and then setting .log appropriately. But you know that already and, if
I'm not mistaken, have said you don't like it.

I'm all for making it possible to configure fine-grained auditing, but
I don't think that should come at the expense of being able to whack
things with a simpler, heavier^Wmore inclusive hammer if you want to.

 My feeling is that we should strip down what goes into contrib to be
 9.5 based.

I do not think I can express a constructive opinion about this. I will
go along with whatever people agree is best.

 I'm also wondering if pieces of that are now out-of-date with where
 core is.

Yes, they are. I'll clean that up.

 I don't particularly like how pgaudit will need to be kept in sync
 with what's in ProcessUtility (normal and slow).

Sure, it's a pain.

 I'd really like to see this additional information regarding what kind
 a command is codified in core.  Ideally, we'd have a single place
 which tracks the kind of command and possibly other information which
 can then be addressed all at once when new commands are added.

What would this look like, and is it a realistic goal for 9.5?

 Also, we could allow more granularity by using the actual classes
 which we already have for objects.

Explain?

  /*
   * This module collects AuditEvents from various sources (event
   * triggers, and executor/utility hooks) and passes them to the
   * log_audit_event() function.
   *
   * An AuditEvent represents an operation that potentially affects a
   * single object. If an underlying command affects multiple objects,
   * multiple AuditEvents must be created to represent it.
   */
 
 The above doesn't appear to be accurate (perhaps it once was?) as
 log_audit_event() only takes a single AuditEvent structure at a time
 and it's not a list.  I'm not sure that needs to change, just pointing
 out that the comment appears to be inaccurate.

If multiple AuditEvents are created (as the ExecutorCheckPerms_hook may
do), log_audit_event() will be called multiple times. The comment is
correct, but I'll try to make it more clear.

 I'm inclined to suggest that the decision be made earlier on about if
 a given action should be logged, as the earlier we do that the less
 work we have to do and we could avoid having to pass in things like
 'granted' to the log_audit_event function at all.

I considered that, but it makes it much harder to review all of the
places where such decisions are made. It's partly because I wrote the
code in this way that I was able to quickly change it to work the way
you suggested (at least once I understood what you suggested).

I prefer the one-line function and a few «if (pgaudit_configured())»
checks (to avoid creating AuditEvents if they won't be needed at all)
and centralising the decision-making rather than spreading the latter
around the whole code.

  /*
   * Takes a role OID and returns true if the role is mentioned in
   * pgaudit.roles or if it inherits from a role mentioned therein;
   * returns false otherwise.
   */
  
  static bool
  role_is_audited(Oid roleid)
  {
  …
 
 The results here should be cached, as was discussed earlier, iirc.

I'll look into that, but it's a peripheral matter.

 Whatever is considered 'noise' should at least be explicitly listed,
 so we know we're covering everything.

OK.

  /*
   * Takes an AuditEvent and, if it should_be_logged(), writes it to the
   * audit log. The AuditEvent is assumed to be completely filled in by
   * the caller (unknown values must be set to  so that they can be
   * logged without error checking).
   */
  
  static void
  log_audit_event(AuditEvent *e)
  {
 
 So, clearly, this is an area which could use some improvement.
 Specifically, being able to write to an independent file instead of just
 using ereport(), supporting other output formats (JSON, maybe even a
 table..).  Also, have you considered using a dynamic shared memory block
 to queue the logging messages to and then a background worker to write
 them out 

Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-12-29 Thread Abhijit Menon-Sen
Hi.

I've changed pgaudit to work as you suggested.

A quick note on the implementation: pgaudit was already installing an
ExecutorCheckPerms_hook anyway; I adapted code from ExecRTECheckPerms
to check if the audit role has been granted any of the permissions
required for the operation.

This means there are three ways to configure auditing:

1. GRANT … ON … TO audit, which logs any operations that correspond to
   the granted permissions.

2. Set pgaudit.roles = 'r1, r2, …', which logs everything done by r1,
   r2, and any of their descendants.

3. Set pgaudit.log = 'read, write, …', which logs any events in any of
   the listed classes.

(This is a small change from the earlier behaviour where, if a role was
listed in .roles, it was still subject to the .log setting. I find that
more useful in practice, but since we're discussing Stephen's proposal,
I implemented what he said.)

The new pgaudit.c is attached here for review. Nothing else has changed
from the earlier submission; and everything is in the github repository
(github.com/2ndQuadrant/pgaudit).

-- Abhijit
/*
 * pgaudit/pgaudit.c
 *
 * Copyright © 2014, PostgreSQL Global Development Group
 *
 * Permission to use, copy, modify, and distribute this software and
 * its documentation for any purpose, without fee, and without a
 * written agreement is hereby granted, provided that the above
 * copyright notice and this paragraph and the following two
 * paragraphs appear in all copies.
 *
 * IN NO EVENT SHALL THE AUTHOR BE LIABLE TO ANY PARTY FOR DIRECT,
 * INDIRECT, SPECIAL, INCIDENTAL, OR CONSEQUENTIAL DAMAGES, INCLUDING
 * LOST PROFITS, ARISING OUT OF THE USE OF THIS SOFTWARE AND ITS
 * DOCUMENTATION, EVEN IF THE UNIVERSITY OF CALIFORNIA HAS BEEN ADVISED
 * OF THE POSSIBILITY OF SUCH DAMAGE.
 *
 * THE AUTHOR SPECIFICALLY DISCLAIMS ANY WARRANTIES, INCLUDING, BUT NOT
 * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
 * A PARTICULAR PURPOSE.  THE SOFTWARE PROVIDED HEREUNDER IS ON AN AS
 * IS BASIS, AND THE AUTHOR HAS NO OBLIGATIONS TO PROVIDE MAINTENANCE,
 * SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
 */

#include postgres.h

#include access/htup_details.h
#include access/sysattr.h
#include access/xact.h
#include catalog/catalog.h
#include catalog/objectaccess.h
#include catalog/pg_class.h
#include commands/dbcommands.h
#include catalog/pg_proc.h
#include commands/event_trigger.h
#include executor/executor.h
#include executor/spi.h
#include miscadmin.h
#include libpq/auth.h
#include nodes/nodes.h
#include tcop/utility.h
#include utils/acl.h
#include utils/builtins.h
#include utils/guc.h
#include utils/lsyscache.h
#include utils/memutils.h
#include utils/rel.h
#include utils/syscache.h
#include utils/timestamp.h

PG_MODULE_MAGIC;

void _PG_init(void);

Datum pgaudit_func_ddl_command_end(PG_FUNCTION_ARGS);
Datum pgaudit_func_sql_drop(PG_FUNCTION_ARGS);

PG_FUNCTION_INFO_V1(pgaudit_func_ddl_command_end);
PG_FUNCTION_INFO_V1(pgaudit_func_sql_drop);

/*
 * pgaudit_roles_str is the string value of the pgaudit.roles
 * configuration variable, which is a list of role names.
 */

char *pgaudit_roles_str = NULL;

/*
 * pgaudit_log_str is the string value of the pgaudit.log configuration
 * variable, e.g. read, write, user. Each token corresponds to a flag
 * in enum LogClass below. We convert the list of tokens into a bitmap
 * in pgaudit_log for internal use.
 */

char *pgaudit_log_str = NULL;
static uint64 pgaudit_log = 0;

enum LogClass {
	LOG_NONE = 0,

	/* SELECT */
	LOG_READ = (1  0),

	/* INSERT, UPDATE, DELETE, TRUNCATE */
	LOG_WRITE = (1  1),

	/* GRANT, REVOKE, ALTER … */
	LOG_PRIVILEGE = (1  2),

	/* CREATE/DROP/ALTER ROLE */
	LOG_USER = (1  3),

	/* DDL: CREATE/DROP/ALTER */
	LOG_DEFINITION = (1  4),

	/* DDL: CREATE OPERATOR etc. */
	LOG_CONFIG = (1  5),

	/* VACUUM, REINDEX, ANALYZE */
	LOG_ADMIN = (1  6),

	/* Function execution */
	LOG_FUNCTION = (1  7),

	/* Absolutely everything; not available via pgaudit.log */
	LOG_ALL = ~(uint64)0
};

/*
 * This module collects AuditEvents from various sources (event
 * triggers, and executor/utility hooks) and passes them to the
 * log_audit_event() function.
 *
 * An AuditEvent represents an operation that potentially affects a
 * single object. If an underlying command affects multiple objects,
 * multiple AuditEvents must be created to represent it.
 */

typedef struct {
	NodeTag type;
	const char *object_id;
	const char *object_type;
	const char *command_tag;
	const char *command_text;
	bool granted;
} AuditEvent;

/*
 * Returns the oid of the hardcoded audit role.
 */

static Oid
audit_role_oid()
{
	HeapTuple roleTup;
	Oid oid = InvalidOid;

	roleTup = SearchSysCache1(AUTHNAME, PointerGetDatum(audit));
	if (HeapTupleIsValid(roleTup)) {
		oid = HeapTupleGetOid(roleTup);
		ReleaseSysCache(roleTup);
	}

	return oid;
}

/* Returns true if either pgaudit.roles or pgaudit.log is set. */

static inline bool
pgaudit_configured()
{
	return (pgaudit_roles_str  *pgaudit_roles_str) 

Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-12-27 Thread Simon Riggs
On 25 December 2014 at 17:09, Jaime Casanova ja...@2ndquadrant.com wrote:
 On Thu, Dec 25, 2014 at 5:42 AM, Abhijit Menon-Sen a...@2ndquadrant.com 
 wrote:

 To summarise for people who haven't followed the thread in detail, the
 idea is that you would do:

 grant select on foo to audit;

 ...and the server would audit-log any select ... from foo ... queries (by
 any user).

 what if i want to audit different things for different users?

That is supported - you can provide a list of roles to be audit roles.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-12-27 Thread Simon Riggs
On 25 December 2014 at 10:42, Abhijit Menon-Sen a...@2ndquadrant.com wrote:

 Stephen likes the idea, obviously; Simon also said he liked it, but I
 now wonder if he may have liked the part I implemented (which allows a
 hot standby to have a different auditing configuration than the primary)
 but not fully realised the remainder of the proposal.

I am happy with the proposal, I just thought you'd already done it.

 Before I go much further, how do others feel about it?

 To summarise for people who haven't followed the thread in detail, the
 idea is that you would do:

 grant select on foo to audit;

GRANT is understood and supported by many people and tools. The idea
makes auditing immediately accessible for wide usage.

 …and the server would audit-log any select … from foo … queries (by
 any user). One immediate consequence is that only things you could grant
 permissions for could be audited (by this mechanism), but I guess that's
 a problem only in the short term. Another consequence is that you can't
 audit selects on foo only by role x and selects on bar only by role y.

 Also, what makes the audit role magical?

 I think it's because it exists only to receive these negative grants,
 there's no other magic involved. Stephen also said «Note that this role,
 from core PG's perspective, wouldn't be special at all».

I don't see them as negative grants. You are simply specifying the
privilege and object you want logged.

Abhijit is right to point out that we can't specify all combinations
of actions, but solving that is considerably more complex. At the
moment we don't have strong evidence that we should wait for such
additional complexity. We can improve things in next release if it is
truly needed.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-12-27 Thread Abhijit Menon-Sen
At 2014-12-27 08:08:32 +, si...@2ndquadrant.com wrote:

  what if i want to audit different things for different users?
 
 That is supported - you can provide a list of roles to be audit roles.

You can do that, but maybe it isn't quite enough to do what Jaime is
asking for. Not always, at least. Consider:

1. You want to audit select on foo by user jaime (only).
2. You want to audit update on bar by user simon (only).

So you do something like this:

pgaudit.roles = 'audit1, audit2'

grant select on foo to audit1;
grant update on bar to audit2;

Now if Jaime does select on foo or if Simon does update on bar, it'll be
logged. If user jaime does not have permission to do update on bar, and
if simon doesn't have permission to select on foo, everything is fine.

But there's no way to say *don't* audit select on foo by simon.

You could do something like grant role audit1 to jaime and then check
the audit-permissions of whichever audit role jaime is found at runtime
to inherit from. But in Stephen's original proposal, granting any audit
role to any user meant that everything they did would be logged.

-- Abhijit


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-12-27 Thread Simon Riggs
On 27 December 2014 at 08:47, Abhijit Menon-Sen a...@2ndquadrant.com wrote:

 But there's no way to say *don't* audit select on foo by simon.

We can cover what it does and does not do in some doc examples.

When submitted, pgaudit didn't have very complex auditing rules.
Stephen's suggestion improves that considerably, but isn't the only
conceivable logging rule. But we'll need to see what else is needed; I
doubt we'll need everything, at least not in PG9.5

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-12-27 Thread Stephen Frost
* Simon Riggs (si...@2ndquadrant.com) wrote:
 On 27 December 2014 at 08:47, Abhijit Menon-Sen a...@2ndquadrant.com wrote:
 
  But there's no way to say *don't* audit select on foo by simon.
 
 We can cover what it does and does not do in some doc examples.
 
 When submitted, pgaudit didn't have very complex auditing rules.
 Stephen's suggestion improves that considerably, but isn't the only
 conceivable logging rule. But we'll need to see what else is needed; I
 doubt we'll need everything, at least not in PG9.5

Agreed, it allows us much more flexibility, but it isn't a panacea.  I'm
hopeful that it'll be flexibile enough for certain regulatory-required
use-cases.  In any case, it's much closer and is certainly worthwhile
even if it doesn't allow for every possible configuration or ends up not
meeting specific regulatory needs because it moves us to a place where
we can sensibly consider what else is needed?

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-12-25 Thread Abhijit Menon-Sen
At 2014-12-22 08:05:57 -0500, robertmh...@gmail.com wrote:

 On Tue, Dec 16, 2014 at 1:28 PM, Stephen Frost sfr...@snowman.net wrote:
  … ok, does the audit role have any permissions here? and, if the
  result is yes, then the command is audited. …
 
 This is a little weird because you're effectively granting an
 anti-permission.

Yes, it's a very clever solution, but also pretty weird. I think that's
why I didn't understand it. I've been looking into writing the code, but
I haven't quite gotten over the weirdness yet.

 I'm not sure whether that ought to be regarded as a serious problem,
 but it's a little surprising.

I'm not sure either.

Stephen likes the idea, obviously; Simon also said he liked it, but I
now wonder if he may have liked the part I implemented (which allows a
hot standby to have a different auditing configuration than the primary)
but not fully realised the remainder of the proposal.

Before I go much further, how do others feel about it?

To summarise for people who haven't followed the thread in detail, the
idea is that you would do:

grant select on foo to audit;

…and the server would audit-log any select … from foo … queries (by
any user). One immediate consequence is that only things you could grant
permissions for could be audited (by this mechanism), but I guess that's
a problem only in the short term. Another consequence is that you can't
audit selects on foo only by role x and selects on bar only by role y.

 Also, what makes the audit role magical?

I think it's because it exists only to receive these negative grants,
there's no other magic involved. Stephen also said «Note that this role,
from core PG's perspective, wouldn't be special at all».

-- Abhijit


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-12-25 Thread Jaime Casanova
On Thu, Dec 25, 2014 at 5:42 AM, Abhijit Menon-Sen a...@2ndquadrant.com wrote:

 To summarise for people who haven't followed the thread in detail, the
 idea is that you would do:

 grant select on foo to audit;

 ...and the server would audit-log any select ... from foo ... queries (by
 any user).

what if i want to audit different things for different users?

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-12-22 Thread Robert Haas
On Tue, Dec 16, 2014 at 1:28 PM, Stephen Frost sfr...@snowman.net wrote:
 The magic audit role has SELECT rights on a given table.  When any
 user does a SELECT against that table, ExecCheckRTPerms is called and
 there's a hook there which the module can use to say ok, does the audit
 role have any permissions here? and, if the result is yes, then the
 command is audited.  Note that this role, from core PG's perspective,
 wouldn't be special at all; it would just be that pgaudit would use the
 role's permissions as a way to figure out if a given command should be
 audited or not.

This is a little weird because you're effectively granting an
anti-permission.  I'm not sure whether that ought to be regarded as a
serious problem, but it's a little surprising.

Also, what makes the audit role magical?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-12-18 Thread Abhijit Menon-Sen
At 2014-12-16 13:28:07 -0500, sfr...@snowman.net wrote:

 The magic audit role has SELECT rights on a given table.  When any
 user does a SELECT against that table, ExecCheckRTPerms is called and
 there's a hook there which the module can use to say ok, does the
 audit role have any permissions here? and, if the result is yes, then
 the command is audited.

You're right, I did not understand that this is what you were proposing,
and this is not what the code does. I went back and read your original
description, and it seems I implemented only the subset I understood.

I'll look into changing the code sometime next week.

-- Abhijit


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-12-18 Thread Stephen Frost
Abhijit,

* Abhijit Menon-Sen (a...@2ndquadrant.com) wrote:
 At 2014-12-16 13:28:07 -0500, sfr...@snowman.net wrote:
 
  The magic audit role has SELECT rights on a given table.  When any
  user does a SELECT against that table, ExecCheckRTPerms is called and
  there's a hook there which the module can use to say ok, does the
  audit role have any permissions here? and, if the result is yes, then
  the command is audited.
 
 You're right, I did not understand that this is what you were proposing,
 and this is not what the code does. I went back and read your original
 description, and it seems I implemented only the subset I understood.
 
 I'll look into changing the code sometime next week.

Ok, great, thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-12-17 Thread Simon Riggs
On 16 December 2014 at 21:45, Stephen Frost sfr...@snowman.net wrote:
 * Simon Riggs (si...@2ndquadrant.com) wrote:
 On 16 December 2014 at 18:28, Stephen Frost sfr...@snowman.net wrote:

  For the sake of the archives, the idea I had been trying to propose is
  to use a role's permissions as a mechanism to define what should be
  audited.  An example is:

 My understanding is that was done.

 Based on the discussion I had w/ Abhijit on IRC, and what I saw him
 commit, it's not the same.  I've been trying to catch up with him on IRC
 to get clarification but havn't managed to yet.

 Abhijit, could you comment on the above (or, well, my earlier email
 which had the details)?  It's entirely possible that I've completely
 misunderstood as I haven't dug into the code yet but rather focused on
 the documentation.

Abhijit added the roles idea on Nov 27 after speaking with you.

AFAIK, it is intended to perform as you requested.

Please review...

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-12-16 Thread Stephen Frost
* Michael Paquier (michael.paqu...@gmail.com) wrote:
 Do you have real numbers about the performance impact that this module
 has when plugged in the server? If yes, it would be good to get an
 idea of how much audit costs and if it has a clear performance impact
 this should be documented to warn the user. Some users may really need
 audit features as you mention, but the performance drop could be an
 obstacle for others so they may prefer continue betting on performance
 instead of pgaudit.

While these performance numbers would be interesting, I don't feel it's
necessary to consider the performance of this module as part of the
question about if it should go into contrib or not.

 Where are we on this? This patch has no activity for the last two
 months... So marking it as returned with feedback. It would be good to
 see actual numbers about the performance impact this involves.

What I was hoping to see were the changes that I had suggested
up-thread, but I discovered about a week or two ago that there was a
misunderstanding between at least Abhijit and myself about what was
being suggested.

For the sake of the archives, the idea I had been trying to propose is
to use a role's permissions as a mechanism to define what should be
audited.  An example is:

The magic audit role has SELECT rights on a given table.  When any
user does a SELECT against that table, ExecCheckRTPerms is called and
there's a hook there which the module can use to say ok, does the audit
role have any permissions here? and, if the result is yes, then the
command is audited.  Note that this role, from core PG's perspective,
wouldn't be special at all; it would just be that pgaudit would use the
role's permissions as a way to figure out if a given command should be
audited or not.

That's not to say that we couldn't commit pgaudit without this
capability and add it later, but I had been expecting to see progress
along these lines prior to reviewing it.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-12-16 Thread Simon Riggs
On 16 December 2014 at 18:28, Stephen Frost sfr...@snowman.net wrote:

 For the sake of the archives, the idea I had been trying to propose is
 to use a role's permissions as a mechanism to define what should be
 audited.  An example is:

My understanding is that was done.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-12-16 Thread Stephen Frost
* Simon Riggs (si...@2ndquadrant.com) wrote:
 On 16 December 2014 at 18:28, Stephen Frost sfr...@snowman.net wrote:
 
  For the sake of the archives, the idea I had been trying to propose is
  to use a role's permissions as a mechanism to define what should be
  audited.  An example is:
 
 My understanding is that was done.

Based on the discussion I had w/ Abhijit on IRC, and what I saw him
commit, it's not the same.  I've been trying to catch up with him on IRC
to get clarification but havn't managed to yet.

Abhijit, could you comment on the above (or, well, my earlier email
which had the details)?  It's entirely possible that I've completely
misunderstood as I haven't dug into the code yet but rather focused on
the documentation.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-11-03 Thread Simon Riggs
On 14 October 2014 20:33, Abhijit Menon-Sen a...@2ndquadrant.com wrote:
 At 2014-10-14 20:09:50 +0100, si...@2ndquadrant.com wrote:

 I think that's a good idea.

 We could have pg_audit.roles = 'audit1, audit2'

 Yes, it's a neat idea, and we could certainly do that. But why is it any
 better than ALTER ROLE audit_rw SET pgaudit.log = … and granting that
 role to the users whose actions you want to audit?

That would make auditing visible to the user, who may then be able to
do something about it.

Stephen's suggestion allows auditing to be conducted without the
users/apps being aware it is taking place.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-11-03 Thread Abhijit Menon-Sen
At 2014-11-03 17:31:37 +, si...@2ndquadrant.com wrote:

 Stephen's suggestion allows auditing to be conducted without the
 users/apps being aware it is taking place.

OK, that makes sense. I'm working on a revised patch that does things
this way, and will post it in a few days.

-- Abhijit


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-11-03 Thread Abhijit Menon-Sen
Hi.

I could actually use some comments on the approach. I've attached
a prototype I've been working on (which is a cut down version of
my earlier code; but it's not terribly interesting and you don't
need to read it to comment on my questions below). The attached
patch does the following:

1. Adds a pgaudit.roles = 'role1, role2' GUC setting.

2. Adds a role_is_audited() function that returns true if the given
   role OID is mentioned in (or inherits from a role mentioned in)
   pgaudit.roles.

3. Adds a call to role_is_audited from log_audit_event with the current
   user id (GetSessionUserId in the patch, though it may be better to
   use GetUserId; but that's a minor detail).

Earlier, I was using a combination of check and assign hooks to convert
names to OIDs, but (as Andres pointed out) that would have problems with
cache invalidations. I was even playing with caching membership lookups,
but I ripped out all that code.

In the attached patch, role_is_audited does all the hard work to split
up the list of roles, look up the corresponding OIDs, and check if the
user is a member of any of those roles. It works fine, but it doesn't
seem desirable to repeat all that work for every statement.

So does anyone have suggestions about how to make this faster?

-- Abhijit
diff --git a/pgaudit.c b/pgaudit.c
index 30f729a..5ba9886 100644
--- a/pgaudit.c
+++ b/pgaudit.c
@@ -58,6 +58,13 @@ PG_FUNCTION_INFO_V1(pgaudit_func_ddl_command_end);
 PG_FUNCTION_INFO_V1(pgaudit_func_sql_drop);
 
 /*
+ * pgaudit_roles_str is the string value of the pgaudit.roles
+ * configuration variable, which is a list of role names.
+ */
+
+char *pgaudit_roles_str = NULL;
+
+/*
  * pgaudit_log_str is the string value of the pgaudit.log configuration
  * variable, e.g. read, write, user. Each token corresponds to a flag
  * in enum LogClass below. We convert the list of tokens into a bitmap
@@ -117,6 +124,40 @@ typedef struct {
 } AuditEvent;
 
 /*
+ * Takes a role OID and returns true or false depending on whether
+ * auditing it enabled for that roles according to the pgaudit.roles
+ * setting.
+ */
+
+static bool
+role_is_audited(Oid roleid)
+{
+	List *roles;
+	ListCell *lt;
+
+	if (!SplitIdentifierString(pgaudit_roles_str, ',', roles))
+		return false;
+
+	foreach(lt, roles)
+	{
+		char *name = (char *)lfirst(lt);
+		HeapTuple roleTup;
+
+		roleTup = SearchSysCache1(AUTHNAME, PointerGetDatum(name));
+		if (HeapTupleIsValid(roleTup))
+		{
+			Oid parentrole = HeapTupleGetOid(roleTup);
+
+			ReleaseSysCache(roleTup);
+			if (is_member_of_role(roleid, parentrole))
+return true;
+		}
+	}
+
+	return false;
+}
+
+/*
  * Takes an AuditEvent and returns true or false depending on whether
  * the event should be logged according to the pgaudit.log setting. If
  * it returns true, it also fills in the name of the LogClass which it
@@ -289,18 +330,24 @@ should_be_logged(AuditEvent *e, const char **classname)
 static void
 log_audit_event(AuditEvent *e)
 {
+	Oid userid;
 	const char *timestamp;
 	const char *database;
 	const char *username;
 	const char *eusername;
 	const char *classname;
 
+	userid = GetSessionUserId();
+
+	if (!role_is_audited(userid))
+		return;
+
 	if (!should_be_logged(e, classname))
 		return;
 
 	timestamp = timestamptz_to_str(GetCurrentTimestamp());
 	database = get_database_name(MyDatabaseId);
-	username = GetUserNameFromId(GetSessionUserId());
+	username = GetUserNameFromId(userid);
 	eusername = GetUserNameFromId(GetUserId());
 
 	/*
@@ -328,7 +375,7 @@ log_executor_check_perms(List *rangeTabls, bool abort_on_violation)
 {
 	ListCell *lr;
 
-	foreach (lr, rangeTabls)
+	foreach(lr, rangeTabls)
 	{
 		Relation rel;
 		AuditEvent e;
@@ -1127,6 +1174,32 @@ pgaudit_object_access_hook(ObjectAccessType access,
 }
 
 /*
+ * Take a pgaudit.roles value such as role1, role2 and verify that
+ * the string consists of comma-separated tokens.
+ */
+
+static bool
+check_pgaudit_roles(char **newval, void **extra, GucSource source)
+{
+	List *roles;
+	char *rawval;
+
+	/* Make sure we have a comma-separated list of tokens. */
+
+	rawval = pstrdup(*newval);
+	if (!SplitIdentifierString(rawval, ',', roles))
+	{
+		GUC_check_errdetail(List syntax is invalid);
+		list_free(roles);
+		pfree(rawval);
+		return false;
+	}
+	pfree(rawval);
+
+	return true;
+}
+
+/*
  * Take a pgaudit.log value such as read, write, user, verify that
  * each of the comma-separated tokens corresponds to a LogClass value,
  * and convert them into a bitmap that log_audit_event can check.
@@ -1232,6 +1305,23 @@ _PG_init(void)
  errmsg(pgaudit must be loaded via shared_preload_libraries)));
 
 	/*
+	 * pgaudit.roles = role1, role2
+	 *
+	 * This variable defines a list of roles for which auditing is
+	 * enabled.
+	 */
+
+	DefineCustomStringVariable(pgaudit.roles,
+			   Enable auditing for certain roles,
+			   NULL,
+			   pgaudit_roles_str,
+			   ,
+			   PGC_SUSET,
+			   GUC_LIST_INPUT | GUC_NOT_IN_SAMPLE,
+			   

Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-11-03 Thread Tom Lane
Abhijit Menon-Sen a...@2ndquadrant.com writes:
 Earlier, I was using a combination of check and assign hooks to convert
 names to OIDs, but (as Andres pointed out) that would have problems with
 cache invalidations. I was even playing with caching membership lookups,
 but I ripped out all that code.

 In the attached patch, role_is_audited does all the hard work to split
 up the list of roles, look up the corresponding OIDs, and check if the
 user is a member of any of those roles. It works fine, but it doesn't
 seem desirable to repeat all that work for every statement.

 So does anyone have suggestions about how to make this faster?

Have you read the code in acl.c that caches lookup results for
role-is-member-of checks?  Sounds pretty closely related.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-10-18 Thread Petr Jelinek

On 18/10/14 06:13, MauMau wrote:


[requirement]
10.2.3 Verify access to all audit trails is logged.

Malicious users often attempt to alter audit logs to
hide their actions, and a record of access allows
an organization to trace any inconsistencies or
potential tampering of the logs to an individual
account. Having access to logs identifying
changes, additions, and deletions can help retrace
steps made by unauthorized personnel.

[my comment]
I'm totally unsure how this can be fulfilled.



This is more matter of configuration of the whole system than something 
pg_audit itself needs to care about (see my answer to 10.5).




[requirement]
10.3 Record at least the following audit
trail entries for all system components for
each event:
10.3.4 Verify success or failure indication is included in log
entries.
10.3.5 Verify origination of event is included in log entries.

[my comment]
This doesn't seem to be fulfilled because the failure of SQL statements
and the client address are not part of the audit log entry.



You can add it to the log_line_prefix though.



[requirement]
10.5 Secure audit trails so they cannot
be altered.
10.5 Interview system administrators and examine system
configurations and permissions to verify that audit trails are
secured so that they cannot be altered as follows:
10.5.1 Only individuals who have a job-related need can view
audit trail files.
Adequate protection of the audit logs includes
strong access control (limit access to logs based
on “need to know” only), and use of physical or
network segregation to make the logs harder to
find and modify.
Promptly backing up the logs to a centralized log
server or media that is difficult to alter keeps the
logs protected even if the system generating the
logs becomes compromised.
10.5.2 Protect audit trail files from
unauthorized modifications.

[my comment]
I don't know how to achieve these, because the DBA (who starts/stops the
server) can modify and delete server log files without any record.



Logging can be setup in a way that it's not even stored on the server 
which DBA has access to. DBA can turn off logging (and the plugin) 
altogether or change logging config but we'd get the shutdown log when 
that happens so 10.2.2 and 10.2.6 will be fulfilled in that case I think.


--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-10-18 Thread Simon Riggs
On 18 October 2014 05:13, MauMau maumau...@gmail.com wrote:

 [requirement]
 10.6 Review logs and security events for
 all system components to identify
 anomalies or suspicious activity.
 Note: Log harvesting, parsing, and
 alerting tools may be used to meet this
 Requirement.
 The log review process does not have to be
 manual. The use of log harvesting, parsing, and
 alerting tools can help facilitate the process by
 identifying log events that need to be reviewed.

 [my comment]
 What commercial and open source products are well known as the log
 harvesting, parsing, and alerting tool?  Is it possible and reasonably easy
 to integrate pgaudit with those tools?  The purpose of audit logging feature
 is not recording facts, but to enable timely detection of malicious actions.
 So, I think the ease of integration with those tools must be evaluated.  But
 I don't know about such tools.

 I feel the current output format of pgaudit is somewhat difficult to treat:

 * The audit log entries are mixed with other logs in the server log files,
 so the user has to extract the audit log lines from the server log files and
 save them elsewhere.  I think it is necessary to store audit logs in
 separate files.

 * Does the command text need  around it in case it contains commas?

Audit entries are sent to the server log, yes.

The server log may be redirected to syslog, which allows various forms
of routing and manipulation that are outside of the reasonable domain
of pgaudit.

PostgreSQL also provides a logging hook that would allow you to filter
or redirect messages as desired.

Given those two ways of handling server log messages, the server log
is the obvious destination to provide for the recording/loggin part of
the audit requirement. pgaudit is designed to allow generating useful
messages, not be an out of the box compliance tool.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-10-17 Thread MauMau

Hello,

As I said in the previous mail, I looked into the latest PCI DSS 3.0 to find 
out whether and how pgaudit fulfills the requirements related to auditing. 
I believe that even the initial version of pgaudit needs to have enough 
functionalities to meet the requirements of some well-known standard, in 
order to demonstrate that PostgreSQL provides a really useful auditing 
feature.  I chose PCI DSS because it seems popular worldwide.


Most requirement items are met, but some are not or I'm not sure if and/or 
how.  I only listed items which I'd like to ask for opinions.  I'm sorry I 
can't have good suggestions, but I hope this report will lead to good 
discussion and better auditing feature we can boast of.  Of course, I'll 
suggest any idea when I think of something.



[requirement]
3.4 Render PAN unreadable anywhere it
is stored (including on portable digital
media, backup media, and in logs) by
using any of the following approaches:
...
3.4.d Examine a sample of audit logs to confirm that the PAN is
rendered unreadable or removed from the logs.

[my comment]
Do the audit log entries always hide the actual bind parameter values (with 
$1, $2, etc.) if the application uses parameterized SQL statements?  Should 
we advise users to use parameterized statements in the pgaudit 
documentation? (I think so)




[requirement]
10.2.2 Verify all actions taken by any individual with root or
administrative privileges are logged.
10.2.6 Verify the following are logged:
. Initialization of audit logs
. Stopping or pausing of audit logs.

[my comment]
The database superuser can hide his activity by SET pgaudit.log = '';, but 
this SET is audit-logged.  Therefore, I think these requirements is met 
because the fact that the superuser's suspicious behavior (hiding activity) 
is recorded.  Do you think this interpretation is valid?




[requirement]
10.2.3 Verify access to all audit trails is logged.

Malicious users often attempt to alter audit logs to
hide their actions, and a record of access allows
an organization to trace any inconsistencies or
potential tampering of the logs to an individual
account. Having access to logs identifying
changes, additions, and deletions can help retrace
steps made by unauthorized personnel.

[my comment]
I'm totally unsure how this can be fulfilled.



[requirement]
10.2.4 Verify invalid logical access attempts are logged.

Malicious individuals will often perform multiple
access attempts on targeted systems. Multiple
invalid login attempts may be an indication of an
unauthorized user’s attempts to “brute force” or
guess a password.

[my comment]
Login attempts also need to be audit-logged to meet this requirement.



[requirement]
10.2.5.a Verify use of identification and authentication
mechanisms is logged.

Without knowing who was logged on at the time of
an incident, it is impossible to identify the
accounts that may have been used. Additionally,
malicious users may attempt to manipulate the
authentication controls with the intent of
bypassing them or impersonating a valid account.

[my comment]
Can we consider this is met because pgaudit records the session user name?



[requirement]
10.3 Record at least the following audit
trail entries for all system components for
each event:
10.3.4 Verify success or failure indication is included in log
entries.
10.3.5 Verify origination of event is included in log entries.

[my comment]
This doesn't seem to be fulfilled because the failure of SQL statements and 
the client address are not part of the audit log entry.




[requirement]
10.5 Secure audit trails so they cannot
be altered.
10.5 Interview system administrators and examine system
configurations and permissions to verify that audit trails are
secured so that they cannot be altered as follows:
10.5.1 Only individuals who have a job-related need can view
audit trail files.
Adequate protection of the audit logs includes
strong access control (limit access to logs based
on “need to know” only), and use of physical or
network segregation to make the logs harder to
find and modify.
Promptly backing up the logs to a centralized log
server or media that is difficult to alter keeps the
logs protected even if the system generating the
logs becomes compromised.
10.5.2 Protect audit trail files from
unauthorized modifications.

[my comment]
I don't know how to achieve these, because the DBA (who starts/stops the 
server) can modify and delete server log files without any record.




[requirement]
10.6 Review logs and security events for
all system components to identify
anomalies or suspicious activity.
Note: Log harvesting, parsing, and
alerting tools may be used to meet this
Requirement.
The log review process does not have to be
manual. The use of log harvesting, parsing, and
alerting tools can help facilitate the process by
identifying log events that need to be reviewed.

[my comment]
What commercial and open source products are well known as the log 
harvesting, parsing, and 

Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-10-16 Thread MauMau

Hello,

I had a quick look through the code and did some testing.  Let me give you 
some comments.  I will proceed with checking if pgaudit can meet PCI DSS 
requirements.


By the way, I'd like to use pgaudit with 9.2.  Is it possible with a slight 
modification of the code?  If it is, what features of pgaudit would be 
unavailable?  Could you support 9.2?



(1)
The build failed with PostgreSQL 9.5, although I know the README mentions 
that pgaudit supports 9.3 and 9.4.  The cause is T_AlterTableSpaceMoveStmt 
macro is not present in 9.5.  I could build and use pgaudit by removing two 
lines referring to that macro.  I tested pgaudit only with 9.5.



(2)
I could confirm that DECLARE is audit-logged as SELECT and FETCH/CLOSE are 
not.  This is just as expected.  Nice.



(3)
SELECT against a view generated two audit log lines, one for the view 
itself, and the other for the underlying table.  Is this intended?  I'm not 
saying that's wrong but just asking.



(4)
I'm afraid audit-logging DML statements on temporary tables will annoy 
users, because temporary tables are not interesting.  In addition, in 
applications which use the same temporary table in multiple types of 
transactions as follows, audit log entries for the DDL statement are also 
annoying.


BEGIN;
CREATE TEMPORARY TABLE mytemp ... ON COMMIT DROP;
DML;
COMMIT;

The workaround is CREATE TEMPORARY TABLE mytemp IF NOT EXIST ... ON COMMIT 
DELETE ROWS.  However, users probably don't (or can't) modify applications 
just for audit logging.



(5)
This is related to (4).  As somebody mentioned, I think the ability to 
select target objects of audit logging is definitely necessary.  Without 
that, huge amount of audit logs would be generated for uninteresting 
objects.  That would also impact performance.



(6)
What's the performance impact of audit logging?  I bet many users will ask 
about what percentage would the throughtput decrease by?  I'd like to know 
the concrete example, say, pgbench and DBT-2.



(7)
In README, COPY FROM/TO should be added to read and write respectively.


(8)
The code looks good.  However, I'm worried about the maintenance.  How can 
developers notice that pgaudit.c needs modification when they add a new SQL 
statement?  What keyword can they use to grep the source code to find 
pgaudit.c?



Regards
MauMau



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-10-16 Thread Simon Riggs
Thanks for the review.

On 16 October 2014 23:23, MauMau maumau...@gmail.com wrote:

 (3)
 SELECT against a view generated two audit log lines, one for the view
 itself, and the other for the underlying table.  Is this intended?  I'm not
 saying that's wrong but just asking.

Intended

 (4)
 I'm afraid audit-logging DML statements on temporary tables will annoy
 users, because temporary tables are not interesting.

Agreed

 (5)
 This is related to (4).  As somebody mentioned, I think the ability to
 select target objects of audit logging is definitely necessary.  Without
 that, huge amount of audit logs would be generated for uninteresting
 objects.  That would also impact performance.

Discussed and agreed already

 (6)
 What's the performance impact of audit logging?  I bet many users will ask
 about what percentage would the throughtput decrease by?  I'd like to know
 the concrete example, say, pgbench and DBT-2.

Don't know, but its not hugely relevant. If you need it, you need it.

 (8)
 The code looks good.  However, I'm worried about the maintenance.  How can
 developers notice that pgaudit.c needs modification when they add a new SQL
 statement?  What keyword can they use to grep the source code to find
 pgaudit.c?

Suggestions welcome.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-10-14 Thread Abhijit Menon-Sen
Hi.

As before, the pgaudit code is at https://github.com/2ndQuadrant/pgaudit
I did a quick round of testing to make sure things still work.

I'll re-add it to the next CF now.

-- Abhijit


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-10-14 Thread Abhijit Menon-Sen
At 2014-10-14 18:05:00 +0530, a...@2ndquadrant.com wrote:

 As before, the pgaudit code is at
 https://github.com/2ndQuadrant/pgaudit

Sorry, I forgot to attach the tarball.

-- Abhijit


pgaudit.tar.gz
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-10-14 Thread Stephen Frost
Abhijit,

* Abhijit Menon-Sen (a...@2ndquadrant.com) wrote:
 As before, the pgaudit code is at https://github.com/2ndQuadrant/pgaudit
 I did a quick round of testing to make sure things still work.

Thanks!

I had a very interesting discussion about auditing rules and the need to
limit what gets audited by user, table, column, policy, etc recently and
an idea came up (not my own) about how to support that granularity
without having to modify the existing PG catalogs or use GUCs or
reloptions, etc.  It goes something like this-

Create an 'audit' role.

Every command run by roles which are granted to the 'audit' role are
audited.

Every 'select' against tables which the 'audit' role has 'select' rights
on are audited.  Similairly for every insert, update, delete.

Every 'select' against columns of tables which the 'audit' role has
'select' rights on are audited- and only those columns are logged.
Similairly for every insert, update, delete.

etc, etc, throughout the various objects which can have permissions.

We don't currently have more granular permissions for roles (it's
all-or-nothing currently regarding role membership) and so if we want
to support things like audit all DML for this role we need multiple
roles, eg:

Create an 'audit_rw' role.  DML commands run by roles granted to the
'audit_rw' role are audited.  Similairly for 'audit_ro' or other
permutations.

The 'audit*' roles would need to be configured for pgAudit in some
fashion, but that's acceptable and is much more reasonable than having
an independent config file which has to keep track of the specific roles
or tables being audited.

The 'audit_ro' and 'audit_rw' roles lead to an interesting thought about
supporting something like which I want to mention here before I forget
about it, but probably should be a new thread if folks think it's
interesting:

GRANT role1 (SELECT) to role2;

Such that 'role2' would have role1's SELECT rights, but not role1's
INSERT, or UPDATE, or DELETE rights.  There would be more complexity if
we want to support this for more than just normal relations, of course.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-10-14 Thread Simon Riggs
On 14 October 2014 13:57, Stephen Frost sfr...@snowman.net wrote:

 Create an 'audit' role.

 Every command run by roles which are granted to the 'audit' role are
 audited.

 Every 'select' against tables which the 'audit' role has 'select' rights
 on are audited.  Similairly for every insert, update, delete.

I think that's a good idea.

We could have pg_audit.roles = 'audit1, audit2'
so users can specify any audit roles they wish, which might even be
existing user names.

That is nice because it allows multiple completely independent
auditors to investigate whatever they choose without discussing with
other auditors.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-10-14 Thread Stephen Frost
* Simon Riggs (si...@2ndquadrant.com) wrote:
 On 14 October 2014 13:57, Stephen Frost sfr...@snowman.net wrote:
 
  Create an 'audit' role.
 
  Every command run by roles which are granted to the 'audit' role are
  audited.
 
  Every 'select' against tables which the 'audit' role has 'select' rights
  on are audited.  Similairly for every insert, update, delete.
 
 I think that's a good idea.
 
 We could have pg_audit.roles = 'audit1, audit2'
 so users can specify any audit roles they wish, which might even be
 existing user names.

Agreed.

 That is nice because it allows multiple completely independent
 auditors to investigate whatever they choose without discussing with
 other auditors.

Yes, also a good thought.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-10-14 Thread Abhijit Menon-Sen
At 2014-10-14 20:09:50 +0100, si...@2ndquadrant.com wrote:

 I think that's a good idea.
 
 We could have pg_audit.roles = 'audit1, audit2'

Yes, it's a neat idea, and we could certainly do that. But why is it any
better than ALTER ROLE audit_rw SET pgaudit.log = … and granting that
role to the users whose actions you want to audit?

-- Abhijit


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-10-09 Thread MauMau

From: Simon Riggs si...@2ndquadrant.com

I hope we can get pgAudit in as a module for 9.5. I also hope that it
will stimulate the requirements/funding of further work in this area,
rather than squash it. My feeling is we have more examples of feature
sets that grow over time (replication, view handling, hstore/JSONB
etc) than we have examples of things languishing in need of attention
(partitioning).


I'm hoping PostgreSQL will have an audit trail feature.  I'd like to support 
your work by reviewing and testing, although I'm not sure I can fully 
understand the code soon.



Regards
MauMau



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-10-08 Thread Stephen Frost
* Fabrízio de Royes Mello (fabriziome...@gmail.com) wrote:
 On Tue, Oct 7, 2014 at 1:24 PM, Simon Riggs si...@2ndquadrant.com wrote:
  I hope we can get pgAudit in as a module for 9.5. I also hope that it
  will stimulate the requirements/funding of further work in this area,
  rather than squash it. My feeling is we have more examples of feature
  sets that grow over time (replication, view handling, hstore/JSONB
  etc) than we have examples of things languishing in need of attention
  (partitioning).
 
 +1

To this point, specifically, I'll volunteer to find time in Novemeber to
review pgAudit for inclusion as a contrib module.  I feel a bit
responsible for it not being properly reviewed earlier due to my upgrade
and similar concerns.

Perhaps the latest version should be posted and added to the commitfest
for 2014-10 and I'll put myself down as a reviewer..?  I don't see it
there now.  I don't mean to be dismissive by suggesting it be added to
the commitfest- I honestly don't see myself having time before November
given the other things I'm involved in right now and pgConf.eu happening
in a few weeks.

Of course, if someone else has time to review, please do.

Thanks,

Stephen


signature.asc
Description: Digital signature


  1   2   >