Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL
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 http://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
On 2/26/15 1:00 AM, Fujii Masao wrote: > On Thu, Feb 26, 2015 at 1:40 PM, Alvaro Herrera >> 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. Currently only the targeted nested DDL would be logged. However, it would log the top-level statement as well as the object that was dropped. Here's an example from the unit tests: do $$ begin create table test_block (id int); drop table test_block; end; $$ When pg_audit.log = 'function, ddl' the output will be: AUDIT: SESSION,FUNCTION,DO,,,do $$ begin create table test_block (id int); drop table test_block; end; $$ AUDIT: SESSION,DDL,CREATE TABLE,TABLE,public.test_block,do $$ begin create table test_block (id int); drop table test_block; end; $$ AUDIT: SESSION,DDL,DROP TABLE,TABLE,public.test_block,do $$ begin create table test_block (id int); drop table test_block; end; $$ You can see that in the create and drop audit entries the fully-qualified name is given. The statement comes from debug_query_string so it shows the top-level statement, even though more detail is given in the other fields when possible. If pg_audit.log = 'ddl' then the DO entry would not be logged even though statements under it were logged. At least, that's the way it works currently. -- - David Steele da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL
On 2/25/15 11:40 PM, Alvaro Herrera wrote: > Fujii Masao wrote: >> On Tue, Feb 24, 2015 at 1:29 AM, David Steele 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
On 2/25/15 10:42 PM, Fujii Masao wrote: > On Tue, Feb 24, 2015 at 1:29 AM, David Steele 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
On Thu, Feb 26, 2015 at 1:40 PM, Alvaro Herrera wrote: > Fujii Masao wrote: >> On Tue, Feb 24, 2015 at 1:29 AM, David Steele 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
Fujii Masao wrote: > On Tue, Feb 24, 2015 at 1:29 AM, David Steele 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
On Tue, Feb 24, 2015 at 1:29 AM, David Steele 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
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
On 2/18/15 10:29 PM, Fujii Masao wrote: > On Thu, Feb 19, 2015 at 12:25 AM, David Steele 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
On Thu, Feb 19, 2015 at 12:25 AM, David Steele wrote: > 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 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. 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. 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
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 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
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
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
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 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
Stephen, I meant it to go to the list, but hit the wrong button. > On Feb 17, 2015, at 7:01 PM, Stephen Frost 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 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
On Wed, Feb 18, 2015 at 1:26 AM, David Steele 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
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
* 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
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
* 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
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
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
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
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
* 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
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
Neil, * Neil Tiffin (ne...@neiltiffin.com) wrote: > > On Feb 17, 2015, at 3:40 AM, Yeb Havinga 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
* 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
> On Feb 17, 2015, at 3:40 AM, Yeb Havinga 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
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
On 17 February 2015 at 15:52, Stephen Frost 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
* 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
On 17 February 2015 at 14:44, Stephen Frost 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
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
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-Sen 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
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-Sen 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
On Tue, Jan 27, 2015 at 6:08 PM, 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. > 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
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: select * from account; AUDIT: SESSION,READ,SELECT,,, SQL: insert into account (id, name, password, description) values (1, 'user1', 'HASH1', 'blah, blah'); AUDIT: 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: SQL: select id, name from account; AUDIT: SQL: select password from account; AUDIT: OBJECT,READ,SELECT,TABLE,public.account, SQL: grant update (name, password) on public.account to audit; AUDIT: SQL: update account set description = 'yada, yada'; AUDIT: SQL: update account set password = 'HASH2'; AUDIT: OBJECT,WRITE,UPDATE,TABLE,public.account, 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: DB: connect user user1, database postgres SQL: create table account_role_map ( account_id int, role_id int ); AUDIT: SQL: grant select on public.account_role_map to audit; AUDIT: 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,,, AUDIT: OBJECT,READ,SELECT,TABLE,public.account, AUDIT: OBJECT,READ,SELECT,TABLE,public.account_role_map, SQL: update account set description = 'yada, yada'; AUDIT: SESSION,WRITE,UPDATE,,, SQL: update account set description = 'yada, yada' where password = 'HASH2'; AUDIT: SESSION,WRITE,UPDATE,,, AUDIT: OBJECT,WRITE,UPDATE,TABLE,public.account, 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 EVENT SHALL THE AUTHOR BE LIABLE TO ANY PARTY FOR DIRECT, * INDIRECT, SPECIAL, INCIDENTAL, OR CONSEQUENTIAL DAMAGES, INCLUDING * LOST PRO
Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL
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
On Tue, Jan 27, 2015 at 6:54 PM, Jim Nasby wrote: > On 1/27/15 5:06 PM, Robert Haas wrote: >> >> On Tue, Jan 27, 2015 at 5:55 PM, Jim Nasby >> 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
On 1/27/15 5:06 PM, Robert Haas wrote: On Tue, Jan 27, 2015 at 5:55 PM, Jim Nasby 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
On Tue, Jan 27, 2015 at 5:55 PM, Jim Nasby 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
On 1/26/15 4:45 PM, Robert Haas wrote: On Mon, Jan 26, 2015 at 5:21 PM, Stephen Frost 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
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
On Mon, Jan 26, 2015 at 5:21 PM, Stephen Frost 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
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
Robert, * Robert Haas (robertmh...@gmail.com) wrote: > On Fri, Jan 23, 2015 at 1:16 PM, Stephen Frost 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
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
On Fri, Jan 23, 2015 at 1:16 PM, Stephen Frost 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
* 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
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
* 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
* Robert Haas (robertmh...@gmail.com) wrote: > On Wed, Jan 21, 2015 at 7:05 PM, Jim Nasby 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 > 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
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
On Wed, Jan 21, 2015 at 1:42 AM, Abhijit Menon-Sen 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
On Wed, Jan 21, 2015 at 7:05 PM, Jim Nasby 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
* 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
On 1/21/15 5:38 PM, Stephen Frost wrote: * Jim Nasby (jim.na...@bluetreble.com) wrote: On 1/20/15 9:01 PM, Stephen Frost wrote: * 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. Not if the GUC was startup-only. That would allow someone with OS access to the server to prevent a Postgres superuser from disabling it. That is not accurate. 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 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. I was only mentioning C because of performance concerns. If SQL or plpgsql is fast enough then there's no need. 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. -- 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
* Jim Nasby (jim.na...@bluetreble.com) wrote: > On 1/20/15 9:01 PM, Stephen Frost wrote: > >* 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. > > Not if the GUC was startup-only. That would allow someone with OS access to > the server to prevent a Postgres superuser from disabling it. That is not accurate. Being startup-only won't help if the user is a superuser. > >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. > > I was only mentioning C because of performance concerns. If SQL or plpgsql is > fast enough then there's no need. 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...). Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL
On 1/20/15 9:01 PM, Stephen Frost wrote: * 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. Not if the GUC was startup-only. That would allow someone with OS access to the server to prevent a Postgres superuser from disabling it. 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. I was only mentioning C because of performance concerns. If SQL or plpgsql is fast enough then there's no need. -- 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
* 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
[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
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
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
* Robert Haas (robertmh...@gmail.com) wrote: > On Tue, Jan 20, 2015 at 1:05 AM, Abhijit Menon-Sen > 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
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 fro
Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL
On Tue, Jan 20, 2015 at 5:03 PM, Jim Nasby 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
On 1/20/15 2:20 PM, Robert Haas wrote: On Tue, Jan 20, 2015 at 1:05 AM, Abhijit Menon-Sen 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
On Tue, Jan 20, 2015 at 1:05 AM, Abhijit Menon-Sen 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
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
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 si
Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL
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 dyna
Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL
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 ad
Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL
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
Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL
* Simon Riggs (si...@2ndquadrant.com) wrote: > On 27 December 2014 at 08:47, Abhijit Menon-Sen 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
On 27 December 2014 at 08:47, Abhijit Menon-Sen 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
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
On 25 December 2014 at 10:42, Abhijit Menon-Sen 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
On 25 December 2014 at 17:09, Jaime Casanova wrote: > On Thu, Dec 25, 2014 at 5:42 AM, Abhijit Menon-Sen > 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
On Thu, Dec 25, 2014 at 5:42 AM, Abhijit Menon-Sen 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
At 2014-12-22 08:05:57 -0500, robertmh...@gmail.com wrote: > > On Tue, Dec 16, 2014 at 1:28 PM, Stephen Frost 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
On Tue, Dec 16, 2014 at 1:28 PM, Stephen Frost 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
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
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
On 16 December 2014 at 21:45, Stephen Frost wrote: > * Simon Riggs (si...@2ndquadrant.com) wrote: >> On 16 December 2014 at 18:28, Stephen Frost 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
* Simon Riggs (si...@2ndquadrant.com) wrote: > On 16 December 2014 at 18:28, Stephen Frost 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
On 16 December 2014 at 18:28, Stephen Frost 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
* 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
On Fri, Oct 17, 2014 at 7:44 AM, Simon Riggs wrote: > Thanks for the review. > > On 16 October 2014 23:23, MauMau 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. 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. >> (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. 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. Regards, -- Michael -- 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
Abhijit Menon-Sen 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
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_SAMPL
Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL
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
On 14 October 2014 20:33, Abhijit Menon-Sen 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
On 18 October 2014 05:13, MauMau 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
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
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
Thanks for the review. On 16 October 2014 23:23, MauMau 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
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
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
* Simon Riggs (si...@2ndquadrant.com) wrote: > On 14 October 2014 13:57, Stephen Frost 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