Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
Robert, all, * Robert Haas (robertmh...@gmail.com) wrote: On Mon, May 11, 2015 at 9:07 PM, David Steele da...@pgmasters.net wrote: The attached v12 patch removes the code that became redundant with Alvaro committing the event trigger/deparse work. I've updated the regression tests to reflect the changes, which were fairly minor and add additional information to the output. There are no longer any #ifdef'd code blocks. This is not a full review, but just a few thoughts... Thanks for that. David and I worked through your suggestions, a number of my own, and some general cleanup and I've now pushed it. What happens if the server crashes? Presumably, audit records emitted just before the crash can be lost, possibly even if the transaction went on to commit. That's no worse than what is already the case for regular logging, of course, but it's maybe a bit more relevant here because of the intended use of this information. Right, if the server crashes then we may lose information- but there should be a log somewhere of the crash. I didn't do much in the way of changes to the documentation, but this is definitely an area where we should make it very clear what happens. Braces around single-statement blocks are not PostgreSQL style. Fixed those and a number of other things, like not having entire functions in if() blocks. I wonder if driving the auditing system off of the required permissions is really going to work well. That means that decisions we make about permissions enforcement will have knock-on effects on auditing. For example, the fact that we check permission on a view rather than on the underlying tables will (I think) flow through to how the auditing happens. The checks against the permissions are independent and don't go through our normal permission checking system, so I'm not too worried about this aspect. I agree that we need to be vigilant and consider the impact of changes to the permissions system, but there are also quite a few regression tests in pg_audit and those should catch a lot of potential issues. +stackItem-auditEvent.commandTag == T_DoStmt Use IsA(..., DoStmt) for this kind of test. There are many instances of this pattern in the patch; they should al be fixed. Unfortunately, that's not actually a Node, so we can't just use IsA. We considered making it one, but that would mean IsA() would return a T_DoStmt or similar for something that isn't actually a T_DoStmt (it's an auditEvent of a T_DoStmt). Still, I did go through and look at these cases and made changes to improve them and clean things up to be neater. The documentation and comments in this patch are, by my estimation, somewhat below our usual standard. For example: + DefineCustomBoolVariable(pg_audit.log_notice, +Raise a notice when logging, This was improved, but I'm sure more can be done. Suggestions welcome. Granted, there is a fuller explanation in the documentation, but that doesn't make that explanation particularly good. (One might also wonder why the log level isn't fully configurable instead of offering only two choices.) This was certainly a good point and we added support for choosing the log level to log it at. I don't want to focus too much on this particular example. The comments and documentation really deserve a bit more attention generally than they have gotten thus far. I am not saying they are bad. I am just saying that, IMHO, they are not as good as we typically try to make them. I've done quite a bit of rework of the comments and will be working on improving the documentation also. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
On Mon, May 11, 2015 at 9:07 PM, David Steele da...@pgmasters.net wrote:
On 5/1/15 5:58 AM, Sawada Masahiko wrote:
For now, since pg_audit patch has a pg_audit_ddl_command_end()
function which uses part of un-committed deparsing utility commands
patch API,
pg_audit patch is completely depend on that patch.
If API name, interface are changed, it would affect for pg_audit patch.
I'm not sure about progress of deparsing utility command patch but
you have any idea if that patch is not committed into 9.5 until May
15?
The attached v12 patch removes the code that became redundant with
Alvaro committing the event trigger/deparse work. I've updated the
regression tests to reflect the changes, which were fairly minor and add
additional information to the output. There are no longer any #ifdef'd
code blocks.
This is not a full review, but just a few thoughts...
What happens if the server crashes? Presumably, audit records emitted
just before the crash can be lost, possibly even if the transaction
went on to commit. That's no worse than what is already the case for
regular logging, of course, but it's maybe a bit more relevant here
because of the intended use of this information.
+if (audit_on_relation(relOid, auditOid, auditPerms))
+{
+auditEventStack-auditEvent.granted = true;
+}
Braces around single-statement blocks are not PostgreSQL style.
I wonder if driving the auditing system off of the required
permissions is really going to work well. That means that decisions
we make about permissions enforcement will have knock-on effects on
auditing. For example, the fact that we check permission on a view
rather than on the underlying tables will (I think) flow through to
how the auditing happens.
+stackItem-auditEvent.commandTag == T_DoStmt
Use IsA(..., DoStmt) for this kind of test. There are many instances
of this pattern in the patch; they should al be fixed.
Using auditLogNotice to switch the log level between LOG and NOTICE is
weird. Switching from LOG to NOTICE is an increase in the logging
level relative to client_min_messages, but a decrease relative to
log_min_messages. With default settings, logging at LOG will go only
to the log (not the client) and logging at NOTICE will go only to the
client (not the log).
The documentation and comments in this patch are, by my estimation,
somewhat below our usual standard. For example:
+ DefineCustomBoolVariable(pg_audit.log_notice,
+Raise a
notice when logging,
Granted, there is a fuller explanation in the documentation, but that
doesn't make that explanation particularly good. (One might also
wonder why the log level isn't fully configurable instead of offering
only two choices.)
Here's another example:
+ DefineCustomBoolVariable(pg_audit.log_parameter,
+Enable
statement parameter logging,
It would be far better to say lnclude the values of statement
parameters in auditing messages or something like that. It is quite
clear that this GUC doesn't enable some sort of general statement
parameter logging facility; it changes the contents of auditing
messages that would have been emitted either way.
I don't want to focus too much on this particular example. The
comments and documentation really deserve a bit more attention
generally than they have gotten thus far. I am not saying they are
bad. I am just saying that, IMHO, they are not as good as we
typically try to make them.
--
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] Auditing extension for PostgreSQL (Take 2)
On Thu, May 7, 2015 at 03:41:13PM -0400, Stephen Frost wrote: Bruce, What is our history of doing things in contrib because we are not sure what we want, then moving it into core? My general recollection is that there is usually something in the contrib version we don't want to add to core and people are locked into the contrib API, so we are left supporting it, e.g. xml2, though you could argue that auditing doesn't have application lock-in and xml2 was tied to an external library feature. That's exactly the argument that I'd make there. My recollection is that we did move pieces of hstore and have moved pieces of other contrib modules into core; perhaps we've not yet had a case where we've completely pulled one in, but given the relatively low level of dependency associated with pgAudit, I'm certainly hopeful that we'll be able to here. Lack of history which could be pointed to that's exactly what I'm suggesting here doesn't seem like a reason to not move forward here though; the concept of having a capability initially in contrib and then bringing it into core has certainly been discussed a number of times on other threads and generally makes sense, at least to me, especially when there's little API associated with the extension. OK, I am just asking so we remember this can go badly, not that it will. I guess the over-arching question is whether we have to put this into contrib so we can get feedback and change the API, or whether using from PGXN or incrementally adding it to core is the right approach. I'm surprised to hear this question of if we have to do X, Y, or Z. pgAudit brings a fantastic capability to PostgreSQL which users have been asking to have for many years and is a feature we should be itching to have included. That we can then take it and incrementally add it to core, to leverage things which are only available in core (as discussed last summer, including grammar and relation metadata), looks to me like a great direction to go in and one which we could use over and over to bring new features and capabilities to PG. Lack of auditing is one of the capabilities that users coming from other large RDBMS's see as preventing their ability to migrate to PostgreSQL. Other databases (open and closed source) have it and have had it for years and it's a serious shortcoming of ours that makes users either stick with their existing vendor or look to other closed-source or even open-source solutions. Yes, more extensive auditing is definitely needed. involved in this discussion will be also. Additionally, as discussed last summer, we can provide a migration path (which does not need to be automated or even feature compatible) from pgAudit to an in-core solution and then sunset pgAudit. Uh, that usually ends badly too. I'm confused by this, as it was the result of our discussion and your suggestion from last summer: 20140730192136.gm2...@momjian.us I certainly hope that hasn't substantially changed as that entire discussion is why we're even able to have this discussion about including pgAudit now. I was very much on-board with trying to work on an in-core solution until that thread convinced me that the upgrade concerns which I was worried about wouldn't be an issue for inclusion of an extension to provide the capability. I had forgotten about that. Yes, pg_upgrade can easily do this. Building an in-core solution, in my estimation at least, is going to require at least a couple of release cycles and having the feedback from users of pgAudit will be very valuable to building a good solution, but I don't believe we'll get that feedback without including it. See above --- is it jump through the user hoops and only then they will use it and give us feedback? How motivated can they be if they can't use the PGXN version? Why wouldn't we want to include this capability in PG? I also addressed the why not PGXN above. It it not a lack of motivation but the entire intent and design of the PGXN system which precludes most large organizations from using it, particularly for sensitive requirements such as auditing. So they trust the Postgres, but not the PGXN authors --- I guess legally that makes sense. The bottom line is that for the _years_ we ship pg_audit in /contrib, we will have some logging stuff in postgresql.conf and some in contrib/pg_audit and that distinction is going to look quite odd. To the extent you incrementally add to core, you will have duplicate functionality in both places. That's entirely correct, of course, but I'm not seeing it as an issue. I'm certainly prepared to support shipping pgAudit in contrib, as are others based on how this feature has been developed, for the years that we'll have 9.5, 9.6 (or 10.0, etc) supported- and that's also another reason why users will use it when they wouldn't use something on PGXN. Further, I look forward
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
Bruce, * Bruce Momjian (br...@momjian.us) wrote: On Thu, May 7, 2015 at 10:26:55AM -0400, Stephen Frost wrote: * Peter Eisentraut (pete...@gmx.net) wrote: On 5/4/15 8:37 PM, Stephen Frost wrote: I don't follow this logic. The concerns raised above are about changing our in-core logging. We haven't got in-core auditing and so I don't see how they apply to it. How is session auditing substantially different from statement logging? David had previously outlined the technical differences between the statement logging we have today and what pgAudit does, but I gather you're asking about it definitionally, though it ends up amounting to much the same to me. Auditing is about what happened whereas statement logging is log whatever statement the user sent. pgAudit bears this out by logging internal SQL statements and object information, unlike what we do with statement logging today. Well, what I was looking for is how auditing is _conceptually_ different from logging, e.g. I can clearly explain how authentication (prove who you are) and authorization (what you are allowed to do) are different. I'd say that auditing is one category of logging, just as statement/session logging is another category of logging. What we currently have in core is well defined and used extensively- but it's a specific kind of logging which is statement logging, and that's all we provide currently. That isn't to say that there isn't commonality, there certainly is, but pgAudit leverages that to a large extent already by using the logging infrastructure in the backend for everything which is logged. Your definition above seems to be more behavioral, e.g. what arrived vs. what happened. It is not clear to me why reporting such information is conceptually different and requires different infrastructure, i.e. we could not easily combine authentication and authorization into the same infrastructure, but logging and auditing seems similar. Similar to logging, Authentication and Authorization also fall under a more general category- access control. At least no one has disputed that yet. The only argument against has been that they don't want to touch the logging. I'm afraid we've been talking past each other here- I'm fully on-board with enhancing our in-core logging capabilities and even looking to the future at having object auditing included in core. It's not my intent to dispute that or to argue against it. Perhaps I've misunderstood the thrust of this sub-thread, so let me explain what I thought the discussion was. My understanding was that you were concerned about having session auditing included in pgAudit and, further, that you wanted to see our in-core statement logging be improved. I agree that we want to improve the in-core statement logging and, ideally, have an in-core auditing solution in the future. I was attempting to address the concern about having session logging in pgAudit by pointing out that it's valuable to have even if our in-core statement logging is augmented, and further, having it in pgAudit does not preclude or reduce our ability to improve the in-core statement logging in the future; indeed, it's my hope that we'll get good feedback from users of pgAudit which could guide our in-core implementation. As What is our history of doing things in contrib because we are not sure what we want, then moving it into core? My general recollection is that there is usually something in the contrib version we don't want to add to core and people are locked into the contrib API, so we are left supporting it, e.g. xml2, though you could argue that auditing doesn't have application lock-in and xml2 was tied to an external library feature. That's exactly the argument that I'd make there. My recollection is that we did move pieces of hstore and have moved pieces of other contrib modules into core; perhaps we've not yet had a case where we've completely pulled one in, but given the relatively low level of dependency associated with pgAudit, I'm certainly hopeful that we'll be able to here. Lack of history which could be pointed to that's exactly what I'm suggesting here doesn't seem like a reason to not move forward here though; the concept of having a capability initially in contrib and then bringing it into core has certainly been discussed a number of times on other threads and generally makes sense, at least to me, especially when there's little API associated with the extension. for the concern that pgAudit may end up rotting in the tree as some other contrib modules have, I can say with confidence that we will have users of it just as soon as they're able to move to a version of PG which includes it and therefore will be supporting it and addressing issues as we discover them, as I suspect the others who have been Uh, why are they not using the PGXN version of pg_audit, and if it is
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
On Thu, May 7, 2015 at 10:26:55AM -0400, Stephen Frost wrote: * Peter Eisentraut (pete...@gmx.net) wrote: On 5/4/15 8:37 PM, Stephen Frost wrote: I don't follow this logic. The concerns raised above are about changing our in-core logging. We haven't got in-core auditing and so I don't see how they apply to it. How is session auditing substantially different from statement logging? David had previously outlined the technical differences between the statement logging we have today and what pgAudit does, but I gather you're asking about it definitionally, though it ends up amounting to much the same to me. Auditing is about what happened whereas statement logging is log whatever statement the user sent. pgAudit bears this out by logging internal SQL statements and object information, unlike what we do with statement logging today. Well, what I was looking for is how auditing is _conceptually_ different from logging, e.g. I can clearly explain how authentication (prove who you are) and authorization (what you are allowed to do) are different. Your definition above seems to be more behavioral, e.g. what arrived vs. what happened. It is not clear to me why reporting such information is conceptually different and requires different infrastructure, i.e. we could not easily combine authentication and authorization into the same infrastructure, but logging and auditing seems similar. (Actually, pg_hba.conf contains authentication and some course-grained authorization only because it is a good place to do low-overhead authorization; there was a performance requirement to do it in pg_hba.conf to prevent DOS attacks.) At least no one has disputed that yet. The only argument against has been that they don't want to touch the logging. I'm afraid we've been talking past each other here- I'm fully on-board with enhancing our in-core logging capabilities and even looking to the future at having object auditing included in core. It's not my intent to dispute that or to argue against it. Perhaps I've misunderstood the thrust of this sub-thread, so let me explain what I thought the discussion was. My understanding was that you were concerned about having session auditing included in pgAudit and, further, that you wanted to see our in-core statement logging be improved. I agree that we want to improve the in-core statement logging and, ideally, have an in-core auditing solution in the future. I was attempting to address the concern about having session logging in pgAudit by pointing out that it's valuable to have even if our in-core statement logging is augmented, and further, having it in pgAudit does not preclude or reduce our ability to improve the in-core statement logging in the future; indeed, it's my hope that we'll get good feedback from users of pgAudit which could guide our in-core implementation. As What is our history of doing things in contrib because we are not sure what we want, then moving it into core? My general recollection is that there is usually something in the contrib version we don't want to add to core and people are locked into the contrib API, so we are left supporting it, e.g. xml2, though you could argue that auditing doesn't have application lock-in and xml2 was tied to an external library feature. for the concern that pgAudit may end up rotting in the tree as some other contrib modules have, I can say with confidence that we will have users of it just as soon as they're able to move to a version of PG which includes it and therefore will be supporting it and addressing issues as we discover them, as I suspect the others who have been Uh, why are they not using the PGXN version of pg_audit, and if it is because it isn't shipped with Postgres, then these seem like unmotivated users who will complain for some reason if we ever move it out of contrib. I guess the over-arching question is whether we have to put this into contrib so we can get feedback and change the API, or whether using from PGXN or incrementally adding it to core is the right approach. involved in this discussion will be also. Additionally, as discussed last summer, we can provide a migration path (which does not need to be automated or even feature compatible) from pgAudit to an in-core solution and then sunset pgAudit. Uh, that usually ends badly too. Building an in-core solution, in my estimation at least, is going to require at least a couple of release cycles and having the feedback from users of pgAudit will be very valuable to building a good solution, but I don't believe we'll get that feedback without including it. See above --- is it jump through the user hoops and only then they will use it and give us feedback? How motivated can they be if they can't use the PGXN version? The bottom line is that for the _years_ we ship pg_audit in /contrib, we will have some logging stuff in postgresql.conf and some in contrib/pg_audit and that
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
Peter, * Peter Eisentraut (pete...@gmx.net) wrote: On 5/7/15 10:26 AM, Stephen Frost wrote: Auditing is about what happened whereas statement logging is log whatever statement the user sent. pgAudit bears this out by logging internal SQL statements and object information, unlike what we do with statement logging today. I don't think this is quite correct. For example, log_min_duration_statement logs based on what happened. log_duration records what happened. log_checkpoints records what happened. log_statement also requires parsing before deciding whether to log. I'm not sure I agree, but it seems a relatively minor point (please correct me if you feel differently). You're certainly correct that log_min_duration_statement allows filtering of the statement logging based on what happened, but it's still statement logging. The other log options are more in-line with what happened kind of logging, but they also aren't user activity, so perhaps rephrasing my statement along the lines of what happened based on user activity would make more sense. On the other hand, log_checkpoints isn't statement or session logging, which is what we had been discussing, I thought. I agree that log_duration is more in-line with what happened. Generally, logging is what happened. The stuff in syslog is what happened on the system. Agreed, but I had thought we were primairly focusing on session / statement logging, which is the potential overlap in capability being discussed as related to pgAudit (I don't expect pgAudit to ever include checkpoint logging, for example). My email to Bruce, I believe, clarifies how I've been thinking about statement/session logging and the more general category of logging (which auditing certainly falls under also, as audit logging). Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
* Peter Eisentraut (pete...@gmx.net) wrote: On 5/4/15 8:37 PM, Stephen Frost wrote: I don't follow this logic. The concerns raised above are about changing our in-core logging. We haven't got in-core auditing and so I don't see how they apply to it. How is session auditing substantially different from statement logging? David had previously outlined the technical differences between the statement logging we have today and what pgAudit does, but I gather you're asking about it definitionally, though it ends up amounting to much the same to me. Auditing is about what happened whereas statement logging is log whatever statement the user sent. pgAudit bears this out by logging internal SQL statements and object information, unlike what we do with statement logging today. I think it is not, and we could tweak the logging facilities a bit to satisfy the audit trail case while making often-requested enhancement to the traditional logging use case as well at the same time. Changing our existing statement logging to be a what happened auditing system is possible, but I don't see it as a tweak. At least no one has disputed that yet. The only argument against has been that they don't want to touch the logging. I'm afraid we've been talking past each other here- I'm fully on-board with enhancing our in-core logging capabilities and even looking to the future at having object auditing included in core. It's not my intent to dispute that or to argue against it. Perhaps I've misunderstood the thrust of this sub-thread, so let me explain what I thought the discussion was. My understanding was that you were concerned about having session auditing included in pgAudit and, further, that you wanted to see our in-core statement logging be improved. I agree that we want to improve the in-core statement logging and, ideally, have an in-core auditing solution in the future. I was attempting to address the concern about having session logging in pgAudit by pointing out that it's valuable to have even if our in-core statement logging is augmented, and further, having it in pgAudit does not preclude or reduce our ability to improve the in-core statement logging in the future; indeed, it's my hope that we'll get good feedback from users of pgAudit which could guide our in-core implementation. As for the concern that pgAudit may end up rotting in the tree as some other contrib modules have, I can say with confidence that we will have users of it just as soon as they're able to move to a version of PG which includes it and therefore will be supporting it and addressing issues as we discover them, as I suspect the others who have been involved in this discussion will be also. Additionally, as discussed last summer, we can provide a migration path (which does not need to be automated or even feature compatible) from pgAudit to an in-core solution and then sunset pgAudit. Building an in-core solution, in my estimation at least, is going to require at least a couple of release cycles and having the feedback from users of pgAudit will be very valuable to building a good solution, but I don't believe we'll get that feedback without including it. Lastly, from the perspective of the session logging piece, the code footprint for that in pgAudit isn't the bulk or even terribly significant, most of the code would still be required for the object auditing capability. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
On 5/7/15 10:26 AM, Stephen Frost wrote: Auditing is about what happened whereas statement logging is log whatever statement the user sent. pgAudit bears this out by logging internal SQL statements and object information, unlike what we do with statement logging today. I don't think this is quite correct. For example, log_min_duration_statement logs based on what happened. log_duration records what happened. log_checkpoints records what happened. log_statement also requires parsing before deciding whether to log. Generally, logging is what happened. The stuff in syslog is what happened on the system. -- 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] Auditing extension for PostgreSQL (Take 2)
On 5/7/15 8:26 AM, Stephen Frost wrote: * Peter Eisentraut (pete...@gmx.net) wrote: On 5/4/15 8:37 PM, Stephen Frost wrote: I don't follow this logic. The concerns raised above are about changing our in-core logging. We haven't got in-core auditing and so I don't see how they apply to it. How is session auditing substantially different from statement logging? David had previously outlined the technical differences between the statement logging we have today and what pgAudit does, but I gather you're asking about it definitionally, though it ends up amounting to much the same to me. Auditing is about what happened whereas statement logging is log whatever statement the user sent. pgAudit bears this out by logging internal SQL statements and object information, unlike what we do with statement logging today. That's a great way to describe it and I'll see if I can incorporate this idea into the docs to hopefully make the topic a bit clearer. I think it is not, and we could tweak the logging facilities a bit to satisfy the audit trail case while making often-requested enhancement to the traditional logging use case as well at the same time. Changing our existing statement logging to be a what happened auditing system is possible, but I don't see it as a tweak. Agreed - not the least of which would be figuring out a more detailed statement classification system for core which would probably be the first step. Lastly, from the perspective of the session logging piece, the code footprint for that in pgAudit isn't the bulk or even terribly significant, most of the code would still be required for the object auditing capability. Both have a decent footprint but also a lot in common so it's fair to say that removing session audit logging would not reduce the amount of code significantly. -- - David Steele da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
On 5/4/15 8:37 PM, Stephen Frost wrote: I don't follow this logic. The concerns raised above are about changing our in-core logging. We haven't got in-core auditing and so I don't see how they apply to it. How is session auditing substantially different from statement logging? I think it is not, and we could tweak the logging facilities a bit to satisfy the audit trail case while making often-requested enhancement to the traditional logging use case as well at the same time. At least no one has disputed that yet. The only argument against has been that they don't want to touch the logging. -- 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] Auditing extension for PostgreSQL (Take 2)
* Peter Eisentraut (pete...@gmx.net) wrote: On 4/30/15 6:05 AM, Fujii Masao wrote: The specification of session audit logging seems to be still unclear to me. As I had mentioned previously, I would prefer session audit logging to be integrated with the normal statement logging configuration. I'd certainly love to see the logging in core be improved, but that's also rather tangential to this extension and we could certainly have both quite happily. Further, being able to configure and have consistent information from the extension is valuable even if options are added to the in-core logging to make it more flexible. One particular advantage of having the extension is that having it doesn't impact existing users of the in-core logging system. I don't recall any specific proposals for improving the in-core logging system to add the capabilities for session logging that the extension provides, but it seems likely to require changes to at least the CSV format, new log_line_prefix escape codes (which we're using quite a few of already...), new GUCs, and potentially behavior changes to make it work. As I say above, I'm happy to have those discussions (and I've been party to them quite a few times in the past...), but it seems unlikely to seriously reduce the usefulness of session logging being in the extension and producing consistent output with the object logging. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
* Peter Eisentraut (pete...@gmx.net) wrote: On 5/4/15 3:00 PM, Stephen Frost wrote: One particular advantage of having the extension is that having it doesn't impact existing users of the in-core logging system. I don't recall any specific proposals for improving the in-core logging system to add the capabilities for session logging that the extension provides, but it seems likely to require changes to at least the CSV format, new log_line_prefix escape codes (which we're using quite a few of already...), new GUCs, and potentially behavior changes to make it work. As I say above, I'm happy to have those discussions (and I've been party to them quite a few times in the past...), Well yeah, from my perspective, the reason not much has happened in the area of logging is that you and Magnus have repeatedly said you were planning some things. If that was holding anyone back from working on it, then I'm truely sorry. I would encourage anyone interesting in any particular feature to speak up and ask what the status is and if others are working on something, especially if they have time to spend advancing it. I was certainly quite happy when Abhijit posted the initial version of this nearly a year ago as it showed that there were individuals able to spend substantial time on it, as well as a use-case which would be solved through such an extension. I don't wish to lay claim to any particular feature nor to make any guarantees, but I will say that I'm happy to have moved into a position in the past year where I can devote a great deal more in time and resources towards PostgreSQL than I've ever been able to in the past. The reasons given above against changing logging just as easily apply to auditing. I don't follow this logic. The concerns raised above are about changing our in-core logging. We haven't got in-core auditing and so I don't see how they apply to it. This implementation is only a starting point. We don't know whether it will fulfill anyone's requirements. The requirements for logging are it feels good enough for an admin. The requirements for auditing are it satisfies this checklist. We need to be prepared to aggressively evolve this as we gather more information about requirements. Otherwise this will become something like contrib/isn, where we know it doesn't satisfy real-world uses anymore, but we're afraid to touch it because someone might be using it and we don't have the domain knowledge to tell them to stop. I agree that this is a starting point. From the discussions which I've had with PostgreSQL users (both our clients and others), this does fulfill a set of requirements for them. That isn't to say that it's a complete and total solution, nor that we will stop here (we certainly won't!), but I'm confident it *is* solving a real use-case for our users. I don't mean to speak for them, but my understanding is that the work which was done by Abhijit and 2ndQ was sponsored by an EU grant which had a specific set of requirements which this is intended to satisfy. Further, we are absolutely prepared to aggressively evolve this as we learn and understand how it's being used out in the field- but I don't believe we're ever going to really understand that until we put something out there. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
On 5/4/15 3:00 PM, Stephen Frost wrote: One particular advantage of having the extension is that having it doesn't impact existing users of the in-core logging system. I don't recall any specific proposals for improving the in-core logging system to add the capabilities for session logging that the extension provides, but it seems likely to require changes to at least the CSV format, new log_line_prefix escape codes (which we're using quite a few of already...), new GUCs, and potentially behavior changes to make it work. As I say above, I'm happy to have those discussions (and I've been party to them quite a few times in the past...), Well yeah, from my perspective, the reason not much has happened in the area of logging is that you and Magnus have repeatedly said you were planning some things. The reasons given above against changing logging just as easily apply to auditing. This implementation is only a starting point. We don't know whether it will fulfill anyone's requirements. The requirements for logging are it feels good enough for an admin. The requirements for auditing are it satisfies this checklist. We need to be prepared to aggressively evolve this as we gather more information about requirements. Otherwise this will become something like contrib/isn, where we know it doesn't satisfy real-world uses anymore, but we're afraid to touch it because someone might be using it and we don't have the domain knowledge to tell them to stop. -- 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] Auditing extension for PostgreSQL (Take 2)
On 4/30/15 6:05 AM, Fujii Masao wrote: The specification of session audit logging seems to be still unclear to me. As I had mentioned previously, I would prefer session audit logging to be integrated with the normal statement logging configuration. -- 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] Auditing extension for PostgreSQL (Take 2)
On Fri, May 1, 2015 at 6:24 AM, David Steele da...@pgmasters.net wrote: On 4/30/15 6:05 AM, Fujii Masao wrote: On Thu, Apr 30, 2015 at 12:57 PM, Sawada Masahiko sawada.m...@gmail.com wrote: I have changed the status this to Ready for Committer. The specification of session audit logging seems to be still unclear to me. For example, why doesn't session audit logging log queries accessing to The idea was that queries consisting of *only* catalog relations are essentially noise. This makes the log much quieter when tools like psql or PgAdmin are in use. Queries that contain a mix of catalog and user tables are logged. However, you make a good point, so in the spirit of cautious defaults I've changed the behavior to log in this case and allow admins to turn off the behavior if they choose with a new GUC, pg_audit.log_catalog. a catalog like pg_class? Why doesn't it log any queries executed in aborted transaction state? Since there is no such information in the document, The error that aborts a transaction can easily be logged via the standard logging facility. All prior statements in the transaction will be logged with pg_audit. This is acceptable from an audit logging perspective as long as it is documented behavior, which as you point out it currently is not. This has now been documented in the caveats sections which should make it clearer to users. I'm afraid that users would easily get confused with it. Even if we document it, I'm not sure if the current behavior is good for the audit purpose. For example, some users may want to log even queries accessing to the catalogs. Agreed, and this is now the default. I have no idea about when the current CommitFest will end. But probably we don't have that much time left. So I'm thinking that maybe we should pick up small, self-contained and useful part from the patch and focus on that. If we try to commit every features that the patch provides, we might get nothing at least in 9.5, I'm afraid. May 15th is the feature freeze, so that does give a little time. It's not clear to me what a self-contained part of the patch would be. If you have specific ideas on what could be broken out I'm interested to hear them. Patch v11 is attached with the changes discussed here plus some other improvements to the documentation suggested by Erik. For now, since pg_audit patch has a pg_audit_ddl_command_end() function which uses part of un-committed deparsing utility commands patch API, pg_audit patch is completely depend on that patch. If API name, interface are changed, it would affect for pg_audit patch. I'm not sure about progress of deparsing utility command patch but you have any idea if that patch is not committed into 9.5 until May 15? Regards, --- Sawada Masahiko -- 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] Auditing extension for PostgreSQL (Take 2)
On 5/1/15 5:58 AM, Sawada Masahiko wrote: On Fri, May 1, 2015 at 6:24 AM, David Steele da...@pgmasters.net wrote: May 15th is the feature freeze, so that does give a little time. It's not clear to me what a self-contained part of the patch would be. If you have specific ideas on what could be broken out I'm interested to hear them. Patch v11 is attached with the changes discussed here plus some other improvements to the documentation suggested by Erik. For now, since pg_audit patch has a pg_audit_ddl_command_end() function which uses part of un-committed deparsing utility commands patch API, pg_audit patch is completely depend on that patch. If API name, interface are changed, it would affect for pg_audit patch. I'm not sure about progress of deparsing utility command patch but you have any idea if that patch is not committed into 9.5 until May 15? Currently the deparse dependent code is ifdef'd so pg_audit compiles and operates just fine against master. Having the deparse code is nice because it allows less common object types to log with full-qualified names but it is in not a requirement for pg_audit. I'd like to see at least patch 0001 of deparse committed. Not only because it provides the functionality that deparse uses, but because it makes event triggers truly useful in pl/pgsql. -- - David Steele da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
On Thu, Apr 30, 2015 at 12:57 PM, Sawada Masahiko sawada.m...@gmail.com wrote: On Wed, Apr 29, 2015 at 12:17 AM, David Steele da...@pgmasters.net wrote: On 4/28/15 2:14 AM, Sawada Masahiko wrote: On Fri, Apr 24, 2015 at 3:23 AM, David Steele da...@pgmasters.net wrote: I've also added some checking to make sure that if anything looks funny on the stack an error will be generated. Thanks for the feedback! Thank you for updating the patch! I ran the postgres regression test on database which is enabled pg_audit, it works fine. Looks good to me. If someone don't have review comment or bug report, I will mark this as Ready for Committer. Thank you! I appreciate all your work reviewing this patch and of course everyone else who commented on, reviewed or tested the patch along the way. I have changed the status this to Ready for Committer. The specification of session audit logging seems to be still unclear to me. For example, why doesn't session audit logging log queries accessing to a catalog like pg_class? Why doesn't it log any queries executed in aborted transaction state? Since there is no such information in the document, I'm afraid that users would easily get confused with it. Even if we document it, I'm not sure if the current behavior is good for the audit purpose. For example, some users may want to log even queries accessing to the catalogs. I have no idea about when the current CommitFest will end. But probably we don't have that much time left. So I'm thinking that maybe we should pick up small, self-contained and useful part from the patch and focus on that. If we try to commit every features that the patch provides, we might get nothing at least in 9.5, I'm afraid. 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] Auditing extension for PostgreSQL (Take 2)
On Wed, Apr 29, 2015 at 12:17 AM, David Steele da...@pgmasters.net wrote: On 4/28/15 2:14 AM, Sawada Masahiko wrote: On Fri, Apr 24, 2015 at 3:23 AM, David Steele da...@pgmasters.net wrote: I've also added some checking to make sure that if anything looks funny on the stack an error will be generated. Thanks for the feedback! Thank you for updating the patch! I ran the postgres regression test on database which is enabled pg_audit, it works fine. Looks good to me. If someone don't have review comment or bug report, I will mark this as Ready for Committer. Thank you! I appreciate all your work reviewing this patch and of course everyone else who commented on, reviewed or tested the patch along the way. I have changed the status this to Ready for Committer. Regards, --- Sawada Masahiko -- 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] Auditing extension for PostgreSQL (Take 2)
On 4/28/15 2:14 AM, Sawada Masahiko wrote: On Fri, Apr 24, 2015 at 3:23 AM, David Steele da...@pgmasters.net wrote: I've also added some checking to make sure that if anything looks funny on the stack an error will be generated. Thanks for the feedback! Thank you for updating the patch! I ran the postgres regression test on database which is enabled pg_audit, it works fine. Looks good to me. If someone don't have review comment or bug report, I will mark this as Ready for Committer. Thank you! I appreciate all your work reviewing this patch and of course everyone else who commented on, reviewed or tested the patch along the way. -- - David Steele da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
On Fri, Apr 24, 2015 at 3:23 AM, David Steele da...@pgmasters.net wrote: On 4/23/15 5:49 AM, Sawada Masahiko wrote: I'm concerned that behaviour of pg_audit has been changed at a few times as far as I remember. Did we achieve consensus on this design? The original author Abhijit expressed support for the SESSION/OBJECT concept before I started working on the code and so has Stephen Frost. As far as I know all outstanding comments from the community have been addressed. Overall behavior has not changed very much since being submitted to the CF in February - mostly just tweaks and additional options. And one question; OBJECT logging of all tuple deletion (i.g. DELETE FROM hoge) seems like not work as follows. =# grant all on bar TO masahiko; (1) Delete all tuple =# insert into bar values(1); =# delete from bar ; NOTICE: AUDIT: SESSION,47,1,WRITE,DELETE,TABLE,public.bar,delete from bar ; DELETE 1 (2) Delete specified tuple (but same result as (1)) =# insert into bar values(1); =# delete from bar where col = 1; NOTICE: AUDIT: OBJECT,48,1,WRITE,DELETE,TABLE,public.bar,delete from bar where col = 1; NOTICE: AUDIT: SESSION,48,1,WRITE,DELETE,TABLE,public.bar,delete from bar where col = 1; DELETE 1 Definitely a bug. Object logging works in the second case because the select privileges on the col column trigger logging. I have fixed this and added a regression test. I also found a way to get the stack memory context under the query memory context. Because of the order of execution it requires moving the memory context but I still think it's a much better solution. I was able to remove most of the stack pops (except function logging) and the output remained stable. I've also added some checking to make sure that if anything looks funny on the stack an error will be generated. Thanks for the feedback! Thank you for updating the patch! I ran the postgres regression test on database which is enabled pg_audit, it works fine. Looks good to me. If someone don't have review comment or bug report, I will mark this as Ready for Committer. Regards, --- Sawada Masahiko -- 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] Auditing extension for PostgreSQL (Take 2)
On Mon, Apr 20, 2015 at 10:17 PM, David Steele da...@pgmasters.net wrote: On 4/20/15 4:40 AM, Sawada Masahiko wrote: Thank you for updating the patch. One question about regarding since v7 (or later) patch is; What is the different between OBJECT logging and SESSION logging? In brief, session logging can log anything that happens in a user session while object logging only targets DML and SELECT on selected relations. The pg_audit.log_relation setting is intended to mimic the prior behavior of pg_audit before object logging was added. In general, I would not expect pg_audit.log = 'read, write' to be used with pg_audit.role. In other words, session logging of DML and SELECT should probably not be turned on at the same time as object logging is in use. Object logging is intended to be a fine-grained version of pg_audit.log = 'read, write' (one or both). Thank you for your explanation! I understood about how to use two logging style. I used v9 patch with pg_audit.log_relation = on, and got quite similar but different log as follows. =# select * from hoge, bar where hoge.col = bar.col; NOTICE: AUDIT: OBJECT,8,1,READ,SELECT,TABLE,public.hoge,select * from hoge, bar where hoge.col = bar.col; NOTICE: AUDIT: SESSION,8,1,READ,SELECT,TABLE,public.hoge,select * from hoge, bar where hoge.col = bar.col; NOTICE: AUDIT: OBJECT,8,1,READ,SELECT,TABLE,public.bar,select * from hoge, bar where hoge.col = bar.col; NOTICE: AUDIT: SESSION,8,1,READ,SELECT,TABLE,public.bar,select * from hoge, bar where hoge.col = bar.col; The behaviour of SESSION log is similar to OBJECT log now, and SESSION log per session (i.g, pg_audit.log_relation = off) is also similar to 'log_statement = all'. (enhancing log_statement might be enough) So I couldn't understand needs of SESSION log. Session logging is quite different from 'log_statement = all'. See: http://www.postgresql.org/message-id/552323b2.8060...@pgmasters.net and/or the Why pg_audit? section of the pg_audit documentation. I agree that it may make sense in the future to merge session logging into log_statements, but for now it does provide important additional functionality for creating audit logs. I'm concerned that behaviour of pg_audit has been changed at a few times as far as I remember. Did we achieve consensus on this design? And one question; OBJECT logging of all tuple deletion (i.g. DELETE FROM hoge) seems like not work as follows. =# grant all on bar TO masahiko; (1) Delete all tuple =# insert into bar values(1); =# delete from bar ; NOTICE: AUDIT: SESSION,47,1,WRITE,DELETE,TABLE,public.bar,delete from bar ; DELETE 1 (2) Delete specified tuple (but same result as (1)) =# insert into bar values(1); =# delete from bar where col = 1; NOTICE: AUDIT: OBJECT,48,1,WRITE,DELETE,TABLE,public.bar,delete from bar where col = 1; NOTICE: AUDIT: SESSION,48,1,WRITE,DELETE,TABLE,public.bar,delete from bar where col = 1; DELETE 1 Regards, --- Sawada Masahiko -- 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] Auditing extension for PostgreSQL (Take 2)
On 4/20/15 4:40 AM, Sawada Masahiko wrote: Thank you for updating the patch. One question about regarding since v7 (or later) patch is; What is the different between OBJECT logging and SESSION logging? In brief, session logging can log anything that happens in a user session while object logging only targets DML and SELECT on selected relations. The pg_audit.log_relation setting is intended to mimic the prior behavior of pg_audit before object logging was added. In general, I would not expect pg_audit.log = 'read, write' to be used with pg_audit.role. In other words, session logging of DML and SELECT should probably not be turned on at the same time as object logging is in use. Object logging is intended to be a fine-grained version of pg_audit.log = 'read, write' (one or both). I used v9 patch with pg_audit.log_relation = on, and got quite similar but different log as follows. =# select * from hoge, bar where hoge.col = bar.col; NOTICE: AUDIT: OBJECT,8,1,READ,SELECT,TABLE,public.hoge,select * from hoge, bar where hoge.col = bar.col; NOTICE: AUDIT: SESSION,8,1,READ,SELECT,TABLE,public.hoge,select * from hoge, bar where hoge.col = bar.col; NOTICE: AUDIT: OBJECT,8,1,READ,SELECT,TABLE,public.bar,select * from hoge, bar where hoge.col = bar.col; NOTICE: AUDIT: SESSION,8,1,READ,SELECT,TABLE,public.bar,select * from hoge, bar where hoge.col = bar.col; The behaviour of SESSION log is similar to OBJECT log now, and SESSION log per session (i.g, pg_audit.log_relation = off) is also similar to 'log_statement = all'. (enhancing log_statement might be enough) So I couldn't understand needs of SESSION log. Session logging is quite different from 'log_statement = all'. See: http://www.postgresql.org/message-id/552323b2.8060...@pgmasters.net and/or the Why pg_audit? section of the pg_audit documentation. I agree that it may make sense in the future to merge session logging into log_statements, but for now it does provide important additional functionality for creating audit logs. Regards, -- - David Steele da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
On Thu, Apr 16, 2015 at 2:34 AM, David Steele da...@pgmasters.net wrote:
On 4/15/15 11:30 AM, Sawada Masahiko wrote:
On Wed, Apr 15, 2015 at 10:52 AM, Sawada Masahiko sawada.m...@gmail.com
wrote:
I tested v8 patch with CURSOR case I mentioned before, and got
segmentation fault again.
Here are log messages in my environment,
=# select test();
LOG: server process (PID 29730) was terminated by signal 11:
Segmentation fault
DETAIL: Failed process was running: select test();
LOG: terminating any other active server processes
WARNING: terminating connection because of crash of another server process
DETAIL: The postmaster has commanded this server process to roll back
the current transaction and exit, because another server process
exited abnormally and possibly corrupted shared memory.
HINT: In a moment you should be able to reconnect to the database and
repeat your command.
FATAL: the database system is in recovery mode
I investigated this problem and inform you my result here.
When we execute test() function I mentioned, the three stack items in
total are stored into auditEventStack.
The two of them are freed by stack_pop() - stack_free() (i.g,
stack_free() is called by stack_pop()).
One of them is freed by PortalDrop() - .. -
MemoryContextDeleteChildren() - ... - stack_free().
And it is freed at the same time as deleting pg_audit memory context,
and stack will be completely empty.
But after freeing all items, finish_xact_command() function could call
PortalDrop(), so ExecutorEnd() function could be called again.
pg_audit has ExecutorEnd_hook, so postgres tries to free that item.. SEGV.
In my environment, the following change fixes it.
--- pg_audit.c.org2015-04-15 14:21:07.541866525 +0900
+++ pg_audit.c2015-04-15 11:36:53.758877339 +0900
@@ -1291,7 +1291,7 @@
standard_ExecutorEnd(queryDesc);
/* Pop the audit event off the stack */
-if (!internalStatement)
+if (!internalStatement auditEventStack != NULL)
{
stack_pop(auditEventStack-stackId);
}
It might be good idea to add Assert() at before calling stack_pop().
I'm not sure this change is exactly correct, but I hope this
information helps you.
I appreciate you taking the time to look - this is the same conclusion I
came to. I also hardened another area that I thought might be vulnerable.
I've seen this problem with explicit cursors before (and fixed it in
another place a while ago). The memory context that is current in
ExecutorStart is freed before ExecutorEnd is called only in this case as
far as I can tell. I'm not sure this is very consistent behavior.
I have attached patch v9 which fixes this issue as you suggested, but
I'm not completely satisfied with it. It seems like there could be an
unintentional pop from the stack in a case of deeper nesting. This
might not be possible but it's hard to disprove.
I'll think about it some more, but meanwhile this patch addresses the
present issue.
Thank you for updating the patch.
One question about regarding since v7 (or later) patch is;
What is the different between OBJECT logging and SESSION logging?
I used v9 patch with pg_audit.log_relation = on, and got quite
similar but different log as follows.
=# select * from hoge, bar where hoge.col = bar.col;
NOTICE: AUDIT: OBJECT,8,1,READ,SELECT,TABLE,public.hoge,select *
from hoge, bar where hoge.col = bar.col;
NOTICE: AUDIT: SESSION,8,1,READ,SELECT,TABLE,public.hoge,select *
from hoge, bar where hoge.col = bar.col;
NOTICE: AUDIT: OBJECT,8,1,READ,SELECT,TABLE,public.bar,select * from
hoge, bar where hoge.col = bar.col;
NOTICE: AUDIT: SESSION,8,1,READ,SELECT,TABLE,public.bar,select *
from hoge, bar where hoge.col = bar.col;
The behaviour of SESSION log is similar to OBJECT log now, and SESSION
log per session (i.g, pg_audit.log_relation = off) is also similar to
'log_statement = all'. (enhancing log_statement might be enough)
So I couldn't understand needs of SESSION log.
Regards,
---
Sawada Masahiko
--
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] Auditing extension for PostgreSQL (Take 2)
On Wed, Apr 15, 2015 at 10:52 AM, Sawada Masahiko sawada.m...@gmail.com wrote:
On Wed, Apr 15, 2015 at 8:57 AM, David Steele da...@pgmasters.net wrote:
On 4/14/15 7:13 PM, Tatsuo Ishii wrote:
This patch does not apply cleanly due to the moving of pgbench (patch
to filelist.sgml failed).
Thank you for pointing that out!
Ironic that it was the commit directly after the one I was testing with
that broke the patch. It appears the end of the last CF is a very bad
time to be behind HEAD.
Fixed in attached v8 patch.
Thank you for updating the patch!
I applied the patch and complied them successfully without WARNING.
I tested v8 patch with CURSOR case I mentioned before, and got
segmentation fault again.
Here are log messages in my environment,
=# select test();
LOG: server process (PID 29730) was terminated by signal 11:
Segmentation fault
DETAIL: Failed process was running: select test();
LOG: terminating any other active server processes
WARNING: terminating connection because of crash of another server process
DETAIL: The postmaster has commanded this server process to roll back
the current transaction and exit, because another server process
exited abnormally and possibly corrupted shared memory.
HINT: In a moment you should be able to reconnect to the database and
repeat your command.
FATAL: the database system is in recovery mode
I hope that these messages helps you to address this problem.
I will also try to address this.
Regards,
---
Sawada Masahiko
I will also try to address this.
I investigated this problem and inform you my result here.
When we execute test() function I mentioned, the three stack items in
total are stored into auditEventStack.
The two of them are freed by stack_pop() - stack_free() (i.g,
stack_free() is called by stack_pop()).
One of them is freed by PortalDrop() - .. -
MemoryContextDeleteChildren() - ... - stack_free().
And it is freed at the same time as deleting pg_audit memory context,
and stack will be completely empty.
But after freeing all items, finish_xact_command() function could call
PortalDrop(), so ExecutorEnd() function could be called again.
pg_audit has ExecutorEnd_hook, so postgres tries to free that item.. SEGV.
In my environment, the following change fixes it.
--- pg_audit.c.org2015-04-15 14:21:07.541866525 +0900
+++ pg_audit.c2015-04-15 11:36:53.758877339 +0900
@@ -1291,7 +1291,7 @@
standard_ExecutorEnd(queryDesc);
/* Pop the audit event off the stack */
-if (!internalStatement)
+if (!internalStatement auditEventStack != NULL)
{
stack_pop(auditEventStack-stackId);
}
It might be good idea to add Assert() at before calling stack_pop().
I'm not sure this change is exactly correct, but I hope this
information helps you.
Regards,
---
Sawada Masahiko
--
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] Auditing extension for PostgreSQL (Take 2)
On 4/14/15 8:37 PM, Tatsuo Ishii wrote: BTW, in my understanding pg_audit allows to track a table access even if it's used in a view. I think this is a nice feature and it would be better explicitly stated in the document and the test case is better included in the regression test. Here is a sample session: CREATE TABLE test2 (id INT); CREATE VIEW vtest2 AS SELECT * FROM test2; GRANT SELECT ON TABLE public.test2 TO auditor; GRANT SELECT ON TABLE public.vtest2 TO auditor; SELECT * FROM vtest2; NOTICE: AUDIT: SESSION,1,1,READ,SELECT,,,SELECT * FROM vtest2; NOTICE: AUDIT: OBJECT,1,1,READ,SELECT,VIEW,public.vtest2,SELECT * FROM vtest2; NOTICE: AUDIT: OBJECT,1,1,READ,SELECT,TABLE,public.test2,SELECT * FROM vtest2; That's the idea! In the documentation I throw around the word relation pretty liberally, but you are right that some clarification would be helpful. I have added a few parenthetical statements to the docs that should make them clearer. I also took your suggestion and added a view regression test. Both are in patch v9 which I attached to my previous email on this thread. Thank you for taking the time to have a look. -- - David Steele da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
On 4/14/15 7:13 PM, Tatsuo Ishii wrote: This patch does not apply cleanly due to the moving of pgbench (patch to filelist.sgml failed). Thank you for pointing that out! Ironic that it was the commit directly after the one I was testing with that broke the patch. It appears the end of the last CF is a very bad time to be behind HEAD. Fixed in attached v8 patch. -- - David Steele da...@pgmasters.net diff --git a/contrib/Makefile b/contrib/Makefile index d63e441..ed9cf6a 100644 --- a/contrib/Makefile +++ b/contrib/Makefile @@ -28,6 +28,7 @@ SUBDIRS = \ oid2name\ pageinspect \ passwordcheck \ + pg_audit\ pg_buffercache \ pg_freespacemap \ pg_prewarm \ diff --git a/contrib/pg_audit/.gitignore b/contrib/pg_audit/.gitignore new file mode 100644 index 000..a5267cf --- /dev/null +++ b/contrib/pg_audit/.gitignore @@ -0,0 +1,5 @@ +log/ +results/ +tmp_check/ +regression.diffs +regression.out diff --git a/contrib/pg_audit/Makefile b/contrib/pg_audit/Makefile new file mode 100644 index 000..7b36011 --- /dev/null +++ b/contrib/pg_audit/Makefile @@ -0,0 +1,21 @@ +# pg_audit/Makefile + +MODULE = pg_audit +MODULE_big = pg_audit +OBJS = pg_audit.o + +EXTENSION = pg_audit +REGRESS = pg_audit +REGRESS_OPTS = --temp-config=$(top_srcdir)/contrib/pg_audit/pg_audit.conf +DATA = pg_audit--1.0.0.sql + +ifdef USE_PGXS +PG_CONFIG = pg_config +PGXS := $(shell $(PG_CONFIG) --pgxs) +include $(PGXS) +else +subdir = contrib/pg_audit +top_builddir = ../.. +include $(top_builddir)/src/Makefile.global +include $(top_srcdir)/contrib/contrib-global.mk +endif diff --git a/contrib/pg_audit/expected/pg_audit-deparse.out b/contrib/pg_audit/expected/pg_audit-deparse.out new file mode 100644 index 000..66fd20d --- /dev/null +++ b/contrib/pg_audit/expected/pg_audit-deparse.out @@ -0,0 +1,897 @@ +-- Load pg_audit module +create extension pg_audit; +-- +-- Create a superuser role that we know the name of for testing +CREATE USER super SUPERUSER; +\connect contrib_regression super; +-- +-- Create auditor role +CREATE ROLE auditor; +-- +-- Create first test user +CREATE USER user1; +ALTER ROLE user1 SET pg_audit.log = 'ddl, ROLE'; +ALTER ROLE user1 SET pg_audit.log_notice = on; +-- +-- Create, select, drop (select will not be audited) +\connect contrib_regression user1 +CREATE TABLE public.test (id INT); +NOTICE: AUDIT: SESSION,1,1,DDL,CREATE TABLE,TABLE,public.test,CREATE TABLE public.test (id INT); +SELECT * FROM test; + id + +(0 rows) + +DROP TABLE test; +NOTICE: AUDIT: SESSION,2,1,DDL,DROP TABLE,TABLE,public.test,DROP TABLE test; +-- +-- Create second test user +\connect contrib_regression super +CREATE USER user2; +ALTER ROLE user2 SET pg_audit.log = 'Read, writE'; +ALTER ROLE user2 SET pg_audit.log_notice = on; +ALTER ROLE user2 SET pg_audit.role = auditor; +\connect contrib_regression user2 +CREATE TABLE test2 (id INT); +GRANT SELECT ON TABLE public.test2 TO auditor; +-- +-- Role-based tests +CREATE TABLE test3 +( + id INT +); +SELECT count(*) + FROM +( + SELECT relname + FROM pg_class + LIMIT 1 +) SUBQUERY; + count +--- + 1 +(1 row) + +SELECT * + FROM test3, test2; +NOTICE: AUDIT: SESSION,1,1,READ,SELECT,,,SELECT * + FROM test3, test2; +NOTICE: AUDIT: OBJECT,1,1,READ,SELECT,TABLE,public.test2,SELECT * + FROM test3, test2; + id | id ++ +(0 rows) + +GRANT INSERT + ON TABLE public.test3 + TO auditor; +-- +-- Object logged because of: +-- insert on test3 +-- select on test2 +WITH CTE AS +( + SELECT id + FROM test2 +) +INSERT INTO test3 +SELECT id + FROM cte; +NOTICE: AUDIT: SESSION,2,1,WRITE,INSERT,,,WITH CTE AS +( + SELECT id + FROM test2 +) +INSERT INTO test3 +SELECT id + FROM cte; +NOTICE: AUDIT: OBJECT,2,1,WRITE,INSERT,TABLE,public.test3,WITH CTE AS +( + SELECT id + FROM test2 +) +INSERT INTO test3 +SELECT id + FROM cte; +NOTICE: AUDIT: OBJECT,2,1,READ,SELECT,TABLE,public.test2,WITH CTE AS +( + SELECT id + FROM test2 +) +INSERT INTO test3 +SELECT id + FROM cte; +-- +-- Object logged because of: +-- insert on test3 +WITH CTE AS +( + INSERT INTO test3 VALUES (1) + RETURNING id +) +INSERT INTO test2 +SELECT id + FROM cte; +NOTICE: AUDIT: SESSION,3,1,WRITE,INSERT,,,WITH CTE AS +( + INSERT INTO test3 VALUES (1) + RETURNING id +) +INSERT INTO test2 +SELECT id + FROM cte; +NOTICE: AUDIT: OBJECT,3,1,WRITE,INSERT,TABLE,public.test3,WITH CTE AS +( + INSERT INTO test3 VALUES (1) + RETURNING id +) +INSERT INTO test2 +SELECT id + FROM cte; +GRANT UPDATE ON TABLE public.test2 TO auditor; +-- +-- Object logged because of: +-- insert on test3 +-- update on test2 +WITH CTE AS +( + UPDATE test2 + SET id = 1 + RETURNING id +) +INSERT INTO test3 +SELECT id + FROM cte; +NOTICE:
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
Thank you for pointing that out! Ironic that it was the commit directly after the one I was testing with that broke the patch. It appears the end of the last CF is a very bad time to be behind HEAD. Fixed in attached v8 patch. Thank you for your quick response. BTW, in my understanding pg_audit allows to track a table access even if it's used in a view. I think this is a nice feature and it would be better explicitly stated in the document and the test case is better included in the regression test. Here is a sample session: CREATE TABLE test2 (id INT); CREATE VIEW vtest2 AS SELECT * FROM test2; GRANT SELECT ON TABLE public.test2 TO auditor; GRANT SELECT ON TABLE public.vtest2 TO auditor; SELECT * FROM vtest2; NOTICE: AUDIT: SESSION,1,1,READ,SELECT,,,SELECT * FROM vtest2; NOTICE: AUDIT: OBJECT,1,1,READ,SELECT,VIEW,public.vtest2,SELECT * FROM vtest2; NOTICE: AUDIT: OBJECT,1,1,READ,SELECT,TABLE,public.test2,SELECT * FROM vtest2; Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- 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] Auditing extension for PostgreSQL (Take 2)
On Wed, Apr 15, 2015 at 8:57 AM, David Steele da...@pgmasters.net wrote: On 4/14/15 7:13 PM, Tatsuo Ishii wrote: This patch does not apply cleanly due to the moving of pgbench (patch to filelist.sgml failed). Thank you for pointing that out! Ironic that it was the commit directly after the one I was testing with that broke the patch. It appears the end of the last CF is a very bad time to be behind HEAD. Fixed in attached v8 patch. Thank you for updating the patch! I applied the patch and complied them successfully without WARNING. I tested v8 patch with CURSOR case I mentioned before, and got segmentation fault again. Here are log messages in my environment, =# select test(); LOG: server process (PID 29730) was terminated by signal 11: Segmentation fault DETAIL: Failed process was running: select test(); LOG: terminating any other active server processes WARNING: terminating connection because of crash of another server process DETAIL: The postmaster has commanded this server process to roll back the current transaction and exit, because another server process exited abnormally and possibly corrupted shared memory. HINT: In a moment you should be able to reconnect to the database and repeat your command. FATAL: the database system is in recovery mode I hope that these messages helps you to address this problem. I will also try to address this. Regards, --- Sawada Masahiko -- 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] Auditing extension for PostgreSQL (Take 2)
This patch does not apply cleanly due to the moving of pgbench (patch to filelist.sgml failed). Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp Attached is the v7 pg_audit patch. I've tried to address Peter's documentation concerns by cleaning up the terminology and adding a real-world case plus usage recommendations. The word auditing has been expunged from the docs in favor of the term audit logging. Per Simon's request, there is now a pg_audit.log_relation setting that makes session audit logging exhaustively log all relations as it did before. The ROLE logging class is back as well. Simon also suggested a way that pg_audit could be tested with standard regression so I have converted all tests over and removed test.pl. Sawada, I'd certainly appreciate it if you'd try again and see if you are still getting a segfault with your test code (which you can find in the regression tests). Currently the patch will compile on master (I tested with b22a36a) or optionally with Alvaro's deparse patches applied (only 0001 0002 needed). I've supplied a different regression test out file (expected/pg_audit-deparse.out) which can be copied over the standard out file (expected/pg_audit.out) if you'd like to do regression on pg_audit with deparse. The small section of code that calls pg_event_trigger_ddl_commands() can be compiled by defining DEPARSE or removed the #ifdefs around that block. Please let me know if I've missed anything and I look forward to comments and questions. Thanks, -- - David Steele da...@pgmasters.net -- 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] Auditing extension for PostgreSQL (Take 2)
On 4/6/15 5:03 PM, Alvaro Herrera wrote: Simon Riggs wrote: The present version can trigger an audit trail event for a statement, without tracking the object that was being audited. This prevents you from searching for all SQL that touches table X, i.e. we know the statements were generated, but not which ones they were. IMHO that makes the resulting audit trail unusable for auditing purposes. I would like to see that functionality put back before it gets committed, if that occurs. Is there a consensus that the current version is the one that we should be reviewing, rather than the one Abhijit submitted? Last I checked, that wasn't at all clear. Well, this one is the commitfest thread of record. At quick glance, my comments about how does this map to specific customer requirements apply to the other submission as well. -- 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] Auditing extension for PostgreSQL (Take 2)
On 6 April 2015 at 16:34, Peter Eisentraut pete...@gmx.net wrote: On 2/14/15 9:34 PM, David Steele wrote: The patch I've attached satisfies the requirements that I've had from customers in the past. What I'm missing is a more precise description/documentation of what those requirements might be. Audit is a big word. It might imply regulatory or standards compliance on some level. We already have ways to log everything. If customers want auditing instead, they will hopefully have a precise requirements set, and we need a way to map that to a system configuration. I don't think we need auditing - oh there's this pg_audit thing, and it has a bunch of settings you can play with is going to be enough of a workflow. Yes, this needs better documentation, as does RLS. For starters, I would consider the textual server log to be potentially lossy in many circumstances, so there would need to be additional information on how to configure that to be robust. It was intended to be used with a log filter plugin, to allow it to be routed wherever is considered safe. (Also, more accurately, this is an audit trail, not an audit. An audit is an examination of a system, not a record of interactions with a system. An audit trail might be useful for an audit.) No problem with calling it pg_audit_trail I see value in what you call object auditing, which is something you can't easily do at the moment. But what you call session auditing seems hardly distinct from statement logging. If we enhance log_statements a little bit, there will not be a need for an extra module to do almost the same thing. Agreed: generating one line per statement isn't much different from log_statements. The earlier version of pg_audit generated different output. Specifically, it allowed you to generate output for each object tracked; one line per object. The present version can trigger an audit trail event for a statement, without tracking the object that was being audited. This prevents you from searching for all SQL that touches table X, i.e. we know the statements were generated, but not which ones they were. IMHO that makes the resulting audit trail unusable for auditing purposes. I would like to see that functionality put back before it gets committed, if that occurs. -- 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] Auditing extension for PostgreSQL (Take 2)
On 2/14/15 9:34 PM, David Steele wrote: The patch I've attached satisfies the requirements that I've had from customers in the past. What I'm missing is a more precise description/documentation of what those requirements might be. Audit is a big word. It might imply regulatory or standards compliance on some level. We already have ways to log everything. If customers want auditing instead, they will hopefully have a precise requirements set, and we need a way to map that to a system configuration. I don't think we need auditing - oh there's this pg_audit thing, and it has a bunch of settings you can play with is going to be enough of a workflow. For starters, I would consider the textual server log to be potentially lossy in many circumstances, so there would need to be additional information on how to configure that to be robust. (Also, more accurately, this is an audit trail, not an audit. An audit is an examination of a system, not a record of interactions with a system. An audit trail might be useful for an audit.) I see value in what you call object auditing, which is something you can't easily do at the moment. But what you call session auditing seems hardly distinct from statement logging. If we enhance log_statements a little bit, there will not be a need for an extra module to do almost the same thing. -- 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] Auditing extension for PostgreSQL (Take 2)
Simon Riggs wrote: The present version can trigger an audit trail event for a statement, without tracking the object that was being audited. This prevents you from searching for all SQL that touches table X, i.e. we know the statements were generated, but not which ones they were. IMHO that makes the resulting audit trail unusable for auditing purposes. I would like to see that functionality put back before it gets committed, if that occurs. Is there a consensus that the current version is the one that we should be reviewing, rather than the one Abhijit submitted? Last I checked, that wasn't at all clear. -- Á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] Auditing extension for PostgreSQL (Take 2)
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 4/6/15 4:34 PM, Peter Eisentraut wrote: On 2/14/15 9:34 PM, David Steele wrote: The patch I've attached satisfies the requirements that I've had from customers in the past. What I'm missing is a more precise description/documentation of what those requirements might be. Admittedly I'm not a financial or ISO certification auditor, but I've been in the position of providing data to auditors on many of occasions. The requests generally fall into three categories: 1) Data requests. Perhaps all the CDRs for a particular customer for a particular month. Bulk data requests are not addressed by pg_audit. 2) DDL log. A list of all DDL changes made to the database. For instance, the last time a function was updated and who did it. The auditor would like to be sure that the function update timestamp matches up with the last maintenance window and the person who is on record as having done the updates. 3) DML log. This can be done with triggers, but requires quite a bit of work and vigilance. Audit is a big word. It might imply regulatory or standards compliance on some level. We already have ways to log everything. If customers want auditing instead, they will hopefully have a precise requirements set, and we need a way to map that to a system configuration. I don't think we need auditing - oh there's this pg_audit thing, and it has a bunch of settings you can play with is going to be enough of a workflow. For starters, I would consider the textual server log to be potentially lossy in many circumstances, so there would need to be additional information on how to configure that to be robust. Nothing is perfect, but there's a big difference between being able to log everything and being able to use the data you logged to satisfy an audit. Auditors tend to be reasonably tech savvy but there are limits. An example of how pg_audit can provide better logging is at the end of this email. I agree that server logs are potentially lossy but that really describes anywhere audit records might be written. Agreed that there are better ways to do it (like writing back to the DB, or a remote DB), but I thought those methods should be saved for a future version. In my past experience having retention policies in place and being able to show that they normally work are enough to satisfy an auditor. Accidents happen and that's understood - as long as an explanation for the failure is given. Such as, We lost a backup tape, here's the ticket for the incident and the name of the employee who handled the case so you can follow up. Or, On this date we had a disk failure and lost the logs before the could be shipped, here's the ticket, etc. (Also, more accurately, this is an audit trail, not an audit. An audit is an examination of a system, not a record of interactions with a system. An audit trail might be useful for an audit.) You are correct and I'd be happy to call it pg_audit_trail (as Simon suggested) or pg_audit_log or something that's more descriptive. I see value in what you call object auditing, which is something you can't easily do at the moment. But what you call session auditing seems hardly distinct from statement logging. If we enhance log_statements a little bit, there will not be a need for an extra module to do almost the same thing. Even with session auditing you can have multiple log entries per backend call. This is particularly true for DO blocks and functions calls. Here's a relatively simple example, but it shows how complex this could get. Let's say we have a DO block with dynamic SQL: do $$ declare table_name text = 'do_table'; begin execute 'create table ' || table_name || ' (weird name int)'; execute 'drop table ' || table_name; end; $$ Setting log_statement=all will certain work but you only get the DO block logged: LOG: statement: do $$ declare table_name text = 'do_table'; begin execute 'create table ' || table_name || ' (weird name int)'; execute 'drop table ' || table_name; end; $$ With pg_audit you get (forgive the LFs that email added) much more: LOG: AUDIT: SESSION,38,1,FUNCTION,DO,,,do $$ declare table_name text = 'do_table'; begin execute 'create table ' || table_name || ' (weird name int)'; execute 'drop table ' || table_name; end; $$ LOG: AUDIT: SESSION,38,2,DDL,CREATE TABLE,TABLE,public.do_table,CREATE TABLE public.do_table (weird name pg_catalog.int4 ) WITH (oids=OFF) LOG: AUDIT: SESSION,38,3,DDL,DROP TABLE,TABLE,public.do_table,drop table do_table Not only is the DO block logged but each sub statement is logged as well. They are logically grouped by the statement ID (in this case 38) so it's clear they were run as a single command. The commands (DO, DROP TABLE, CREATE TABLE) and fully-qualified object names are provided and the statements are quoted and escaped when needed to making parsing easier. There's no question that this
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 4/6/15 4:47 PM, Simon Riggs wrote: On 6 April 2015 at 16:34, Peter Eisentraut pete...@gmx.net wrote: Audit is a big word. It might imply regulatory or standards compliance on some level. We already have ways to log everything. If customers want auditing instead, they will hopefully have a precise requirements set, and we need a way to map that to a system configuration. I don't think we need auditing - oh there's this pg_audit thing, and it has a bunch of settings you can play with is going to be enough of a workflow. Yes, this needs better documentation, as does RLS. Discussions like these definitely help when it comes to knowing what to put in the documentation. The what is hard enough, the why gets into some scary territory. Still, audit requirements vary wildly and I'm not sure how much justice I could do to the topic in the contrib docs. I think more discussion of what's technically possible might be more fruitful. For starters, I would consider the textual server log to be potentially lossy in many circumstances, so there would need to be additional information on how to configure that to be robust. It was intended to be used with a log filter plugin, to allow it to be routed wherever is considered safe. That would certainly work. (Also, more accurately, this is an audit trail, not an audit. An audit is an examination of a system, not a record of interactions with a system. An audit trail might be useful for an audit.) No problem with calling it pg_audit_trail Nor I. I see value in what you call object auditing, which is something you can't easily do at the moment. But what you call session auditing seems hardly distinct from statement logging. If we enhance log_statements a little bit, there will not be a need for an extra module to do almost the same thing. Agreed: generating one line per statement isn't much different from log_statements. The earlier version of pg_audit generated different output. Specifically, it allowed you to generate output for each object tracked; one line per object. That is still doable, but is covered by object-level auditing. Even so, multiple log entries are possible (and even likely) with session auditing. See my response to Peter for details. The present version can trigger an audit trail event for a statement, without tracking the object that was being audited. This prevents you from searching for all SQL that touches table X, i.e. we know the statements were generated, but not which ones they were. IMHO that makes the resulting audit trail unusable for auditing purposes. I would like to see that functionality put back before it gets committed, if that occurs. Bringing this back would be easy (it actually requires removing, not adding code) but I'd prefer to make it configurable. - -- - - David Steele da...@pgmasters.net -BEGIN PGP SIGNATURE- Version: GnuPG v2 iQIcBAEBCAAGBQJVIybuAAoJEIA2uIJQ5SFAm/8P/2gS2oSvxF2VjP3WBJjH0d8p QHlni3SDBIlJPPE1ZnNYbtUANWKSw2Ublpk50223TeejEnNZfORtA7TZ9qic+3Ei 83yK4SzQcSMA1xqMvGTDS621l4/Nkw/uWKO8BDGePTHRjsEPpgMxJxsHVfNddd5Z MTP26vXPgyzj1H1GE4jPCi1kR6iiKx3GiagawmNJNgzdOXf25hQijpQ7mR0puw/T V75MeNr0WNi4CtsyDgNnx0oVKBN4olG6aId6+q3jt+yuxboJ53Nq59xbfvxYUR+3 uPKX9jfwInZxQc+70g2CcKj+EglB9cDn4oaMUkAxqYWKWyRW0O2gs0IIkbQqk8qK SlfBvAaZA1wfDelCztr8GHc8hLIh+hwb3mJq4zoclPg3+36hUgVyVIyRUjzW42sJ shvd2KWkxP4iwN1+tru9YK3qZ1GXkZfodtXdJ7iY14k5eXTKBuRgHFO8BRXxW9xp /KwIgkLD9gEjht6cncgP83lBoaxMFjrQE9N3hzX1wMM5ZYDAisbKK7JkGE2+yCsH L/aiCOxyHbxaMZATopATbCBhULDMLKl9oICKY+jv17yeyGG5F5D78AWv0tuvk1jW 5enydtXPhcBIXRWIvTZgCfDpFs5Hv5r/+V70tiMQbJIzg2qvxHmC0VLEubxky0XE TGfavKbTvK9dmw1dhzk5 =5KkP -END PGP SIGNATURE- -- 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] Auditing extension for PostgreSQL (Take 2)
On 6 April 2015 at 20:38, David Steele da...@pgmasters.net wrote: The earlier version of pg_audit generated different output. Specifically, it allowed you to generate output for each object tracked; one line per object. That discussion covers recursive SQL. That is important too, but not what I am saying. My point is what we log when an SQL statement covers multiple tables, e.g. join SELECTs, or inheritance cases, views. That is still doable, but is covered by object-level auditing. Even so, multiple log entries are possible (and even likely) with session auditing. See my response to Peter for details. The present version can trigger an audit trail event for a statement, without tracking the object that was being audited. This prevents you from searching for all SQL that touches table X, i.e. we know the statements were generated, but not which ones they were. IMHO that makes the resulting audit trail unusable for auditing purposes. I would like to see that functionality put back before it gets committed, if that occurs. Bringing this back would be easy (it actually requires removing, not adding code) but I'd prefer to make it configurable. That is my preference also. My concern was raised when it was *removed* without confirming others agreed. Typical questions: Who has written to table X? Who has read data from table Y yesterday between time1 and time2? Has anyone accessed a table directly, rather than through a security view? -- 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] Auditing extension for PostgreSQL (Take 2)
On Fri, Apr 3, 2015 at 10:01 PM, David Steele da...@pgmasters.net wrote: On 4/3/15 3:59 AM, Sawada Masahiko wrote: On Thu, Apr 2, 2015 at 2:46 AM, David Steele da...@pgmasters.net wrote: Let me know if you see any other issues. I pulled HEAD, and then tried to compile source code after applied following deparsing utility command patch without #0001 and #0002. (because these two patches are already pushed) http://www.postgresql.org/message-id/20150325175954.gl3...@alvh.no-ip.org But I could not use new pg_audit patch due to compile error (that deparsing utility command patch might have). I think I'm missing something, but I'm not sure. Could you tell me which patch should I apply before compiling pg_audit? When Alvaro posted his last patch set he recommended applying them to b3196e65: http://www.postgresql.org/message-id/20150325175954.gl3...@alvh.no-ip.org Applying the 3/25 deparse patches onto b3196e65 (you'll see one trailing space error) then applying pg_audit-v6.patch works fine. Thank you for your advice! I'm testing pg_audit, so I will send report to you as soon as possible. Regards, --- Sawada Masahiko -- 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] Auditing extension for PostgreSQL (Take 2)
On Fri, Apr 3, 2015 at 10:01 PM, David Steele da...@pgmasters.net wrote: On 4/3/15 3:59 AM, Sawada Masahiko wrote: On Thu, Apr 2, 2015 at 2:46 AM, David Steele da...@pgmasters.net wrote: Let me know if you see any other issues. I pulled HEAD, and then tried to compile source code after applied following deparsing utility command patch without #0001 and #0002. (because these two patches are already pushed) http://www.postgresql.org/message-id/20150325175954.gl3...@alvh.no-ip.org But I could not use new pg_audit patch due to compile error (that deparsing utility command patch might have). I think I'm missing something, but I'm not sure. Could you tell me which patch should I apply before compiling pg_audit? When Alvaro posted his last patch set he recommended applying them to b3196e65: http://www.postgresql.org/message-id/20150325175954.gl3...@alvh.no-ip.org Applying the 3/25 deparse patches onto b3196e65 (you'll see one trailing space error) then applying pg_audit-v6.patch works fine. I tested v6 patch, but I got SEGV when I executed test() function I mentioned up thread. Could you address this problem? Regards, --- Sawada Masahiko -- 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] Auditing extension for PostgreSQL (Take 2)
On 4/6/15 8:40 AM, Sawada Masahiko wrote: On Fri, Apr 3, 2015 at 10:01 PM, David Steele da...@pgmasters.net wrote: On 4/3/15 3:59 AM, Sawada Masahiko wrote: On Thu, Apr 2, 2015 at 2:46 AM, David Steele da...@pgmasters.net wrote: Let me know if you see any other issues. I pulled HEAD, and then tried to compile source code after applied following deparsing utility command patch without #0001 and #0002. (because these two patches are already pushed) http://www.postgresql.org/message-id/20150325175954.gl3...@alvh.no-ip.org But I could not use new pg_audit patch due to compile error (that deparsing utility command patch might have). I think I'm missing something, but I'm not sure. Could you tell me which patch should I apply before compiling pg_audit? When Alvaro posted his last patch set he recommended applying them to b3196e65: http://www.postgresql.org/message-id/20150325175954.gl3...@alvh.no-ip.org Applying the 3/25 deparse patches onto b3196e65 (you'll see one trailing space error) then applying pg_audit-v6.patch works fine. I tested v6 patch, but I got SEGV when I executed test() function I mentioned up thread. Could you address this problem? Unfortunately I'm not able to reproduce the issue. Here's what I tried based on your original test: postgres=# create table hoge (id int); CREATE TABLE postgres=# create function test() returns int as $$ postgres$# declare postgres$# cur1 cursor for select * from hoge; postgres$# tmp int; postgres$# begin postgres$# open cur1; postgres$# fetch cur1 into tmp; postgres$#return tmp; postgres$# end$$ postgres-# language plpgsql ; CREATE FUNCTION postgres=# select test(); test -- (1 row) postgres=# insert into hoge values (1); INSERT 0 1 postgres=# select test(); test -- 1 (1 row) And the log output was: prefix LOG: statement: create table hoge (id int); prefix DEBUG: EventTriggerInvoke 16385 prefix LOG: AUDIT: SESSION,3,1,DDL,CREATE TABLE,TABLE,public.hoge,CREATE TABLE public.hoge (id pg_catalog.int4 ) WITH (oids=OFF) prefix LOG: statement: create function test() returns int as $$ declare cur1 cursor for select * from hoge; tmp int; begin open cur1; fetch cur1 into tmp; return tmp; end$$ language plpgsql ; prefix DEBUG: EventTriggerInvoke 16385 prefix LOG: AUDIT: SESSION,4,1,DDL,CREATE FUNCTION,FUNCTION,public.test(),CREATE FUNCTION public.test() RETURNS pg_catalog.int4 LANGUAGE plpgsql VOLATILE CALLED ON NULL INPUT SECURITY INVOKER COST 100 AS ' declare cur1 cursor for select * from hoge; tmp int; begin open cur1; fetch cur1 into tmp; return tmp; end' prefix LOG: statement: select test(); prefix LOG: AUDIT: SESSION,5,1,READ,SELECT,,,select test(); prefix LOG: AUDIT: SESSION,5,2,FUNCTION,EXECUTE,FUNCTION,public.test,select test(); prefix LOG: AUDIT: SESSION,5,3,READ,SELECT,,,select * from hoge prefix CONTEXT: PL/pgSQL function test() line 6 at OPEN prefix LOG: statement: insert into hoge values (1); prefix LOG: AUDIT: SESSION,6,1,WRITE,INSERT,,,insert into hoge values (1); prefix LOG: statement: select test(); prefix LOG: AUDIT: SESSION,7,1,READ,SELECT,,,select test(); prefix LOG: AUDIT: SESSION,7,2,FUNCTION,EXECUTE,FUNCTION,public.test,select test(); prefix LOG: AUDIT: SESSION,7,3,READ,SELECT,,,select * from hoge prefix CONTEXT: PL/pgSQL function test() line 6 at OPEN Does the above look like what you did? Any other information about your environment would also be helpful. I'm thinking it might be some kind of build issue. -- - David Steele da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
On Thu, Apr 2, 2015 at 2:46 AM, David Steele da...@pgmasters.net wrote: Hi Sawada, On 3/25/15 9:24 AM, David Steele wrote: On 3/25/15 7:46 AM, Sawada Masahiko wrote: 2. I got ERROR when executing function uses cursor. 1) create empty table (hoge table) 2) create test function as follows. create function test() returns int as $$ declare cur1 cursor for select * from hoge; tmp int; begin open cur1; fetch cur1 into tmp; return tmp; end$$ language plpgsql ; 3) execute test function (got ERROR) =# select test(); LOG: AUDIT: SESSION,6,1,READ,SELECT,,,selecT test(); LOG: AUDIT: SESSION,6,2,FUNCTION,EXECUTE,FUNCTION,public.test,selecT test(); LOG: AUDIT: SESSION,6,3,READ,SELECT,,,select * from hoge CONTEXT: PL/pgSQL function test() line 6 at OPEN ERROR: pg_audit stack is already empty STATEMENT: selecT test(); It seems like that the item in stack is already freed by deleting pg_audit memory context (in MemoryContextDelete()), before calling stack_pop in dropping of top-level Portal. This has been fixed and I have attached a new patch. I've seen this with cursors before where the parent MemoryContext is freed before control is returned to ProcessUtility. I think that's strange behavior but there's not a lot I can do about it. Thank you for updating the patch! The code I put in to deal with this situation was not quite robust enough so I had to harden it a bit more. Let me know if you see any other issues. I pulled HEAD, and then tried to compile source code after applied following deparsing utility command patch without #0001 and #0002. (because these two patches are already pushed) http://www.postgresql.org/message-id/20150325175954.gl3...@alvh.no-ip.org But I could not use new pg_audit patch due to compile error (that deparsing utility command patch might have). I think I'm missing something, but I'm not sure. Could you tell me which patch should I apply before compiling pg_audit? Regards, --- Sawada Masahiko -- 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] Auditing extension for PostgreSQL (Take 2)
On 4/3/15 3:59 AM, Sawada Masahiko wrote: On Thu, Apr 2, 2015 at 2:46 AM, David Steele da...@pgmasters.net wrote: Let me know if you see any other issues. I pulled HEAD, and then tried to compile source code after applied following deparsing utility command patch without #0001 and #0002. (because these two patches are already pushed) http://www.postgresql.org/message-id/20150325175954.gl3...@alvh.no-ip.org But I could not use new pg_audit patch due to compile error (that deparsing utility command patch might have). I think I'm missing something, but I'm not sure. Could you tell me which patch should I apply before compiling pg_audit? When Alvaro posted his last patch set he recommended applying them to b3196e65: http://www.postgresql.org/message-id/20150325175954.gl3...@alvh.no-ip.org Applying the 3/25 deparse patches onto b3196e65 (you'll see one trailing space error) then applying pg_audit-v6.patch works fine. -- - David Steele da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
Hi Sawada, On 3/25/15 9:24 AM, David Steele wrote: On 3/25/15 7:46 AM, Sawada Masahiko wrote: 2. I got ERROR when executing function uses cursor. 1) create empty table (hoge table) 2) create test function as follows. create function test() returns int as $$ declare cur1 cursor for select * from hoge; tmp int; begin open cur1; fetch cur1 into tmp; return tmp; end$$ language plpgsql ; 3) execute test function (got ERROR) =# select test(); LOG: AUDIT: SESSION,6,1,READ,SELECT,,,selecT test(); LOG: AUDIT: SESSION,6,2,FUNCTION,EXECUTE,FUNCTION,public.test,selecT test(); LOG: AUDIT: SESSION,6,3,READ,SELECT,,,select * from hoge CONTEXT: PL/pgSQL function test() line 6 at OPEN ERROR: pg_audit stack is already empty STATEMENT: selecT test(); It seems like that the item in stack is already freed by deleting pg_audit memory context (in MemoryContextDelete()), before calling stack_pop in dropping of top-level Portal. This has been fixed and I have attached a new patch. I've seen this with cursors before where the parent MemoryContext is freed before control is returned to ProcessUtility. I think that's strange behavior but there's not a lot I can do about it. The code I put in to deal with this situation was not quite robust enough so I had to harden it a bit more. Let me know if you see any other issues. Thanks, -- - David Steele da...@pgmasters.net diff --git a/contrib/Makefile b/contrib/Makefile index 195d447..d8e75f4 100644 --- a/contrib/Makefile +++ b/contrib/Makefile @@ -29,6 +29,7 @@ SUBDIRS = \ pageinspect \ passwordcheck \ pg_archivecleanup \ + pg_audit\ pg_buffercache \ pg_freespacemap \ pg_prewarm \ diff --git a/contrib/pg_audit/Makefile b/contrib/pg_audit/Makefile new file mode 100644 index 000..32bc6d9 --- /dev/null +++ b/contrib/pg_audit/Makefile @@ -0,0 +1,20 @@ +# pg_audit/Makefile + +MODULE = pg_audit +MODULE_big = pg_audit +OBJS = pg_audit.o + +EXTENSION = pg_audit + +DATA = pg_audit--1.0.0.sql + +ifdef USE_PGXS +PG_CONFIG = pg_config +PGXS := $(shell $(PG_CONFIG) --pgxs) +include $(PGXS) +else +subdir = contrib/pg_audit +top_builddir = ../.. +include $(top_builddir)/src/Makefile.global +include $(top_srcdir)/contrib/contrib-global.mk +endif diff --git a/contrib/pg_audit/pg_audit--1.0.0.sql b/contrib/pg_audit/pg_audit--1.0.0.sql new file mode 100644 index 000..9d9ee83 --- /dev/null +++ b/contrib/pg_audit/pg_audit--1.0.0.sql @@ -0,0 +1,22 @@ +/* pg_audit/pg_audit--1.0.0.sql */ + +-- complain if script is sourced in psql, rather than via CREATE EXTENSION +\echo Use CREATE EXTENSION pg_audit to load this file.\quit + +CREATE FUNCTION pg_audit_ddl_command_end() + RETURNS event_trigger + LANGUAGE C + AS 'MODULE_PATHNAME', 'pg_audit_ddl_command_end'; + +CREATE EVENT TRIGGER pg_audit_ddl_command_end + ON ddl_command_end + EXECUTE PROCEDURE pg_audit_ddl_command_end(); + +CREATE FUNCTION pg_audit_sql_drop() + RETURNS event_trigger + LANGUAGE C + AS 'MODULE_PATHNAME', 'pg_audit_sql_drop'; + +CREATE EVENT TRIGGER pg_audit_sql_drop + ON sql_drop + EXECUTE PROCEDURE pg_audit_sql_drop(); diff --git a/contrib/pg_audit/pg_audit.c b/contrib/pg_audit/pg_audit.c new file mode 100644 index 000..b34df5a --- /dev/null +++ b/contrib/pg_audit/pg_audit.c @@ -0,0 +1,1688 @@ +/*-- + * pg_audit.c + * + * An auditing extension for PostgreSQL. Improves on standard statement logging + * by adding more logging classes, object level logging, and providing + * fully-qualified object names for all DML and many DDL statements (See + * pg_audit.sgml for details). + * + * Copyright (c) 2014-2015, PostgreSQL Global Development Group + * + * IDENTIFICATION + * contrib/pg_audit/pg_audit.c + *-- + */ +#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 catalog/namespace.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); + +/* + * Event trigger prototypes + */ +Datum pg_audit_ddl_command_end(PG_FUNCTION_ARGS); +Datum pg_audit_sql_drop(PG_FUNCTION_ARGS); + +PG_FUNCTION_INFO_V1(pg_audit_ddl_command_end);
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
On 3/23/15 12:40 PM, David Steele wrote: On 3/23/15 1:31 AM, Abhijit Menon-Sen wrote: I'm experimenting with a few approaches to do this without reintroducing switch statements to test every command. That will require core changes, but I think we can find an acceptable arrangement. I'll post a proof of concept in a few days. Any progress on the POC? I'm interested in trying to get the ROLE class back in before the Commitfest winds up, but not very happy with my current string-matching options. + * Takes an AuditEvent and, if it log_check(), writes it to the audit log. I don't think log_check is the most useful name, because this sentence doesn't tell me what the function may do. Similarly, I would prefer to have log_acl_check be renamed acl_grants_audit or similar. (These are all static functions anyway, I don't think a log_ prefix is needed.) log_check() has become somewhat vestigial at this point since it is only called from one place - I've been considering removing it and merging into log_audit_event(). For the moment I've improved the comments. log_check() got rolled into log_audit_event(). I like acl_grants_audit() and agree that it's a clearer name. I'll incorporate that into the next version and apply the same scheme to the other ACL functionsas well as do a general review of naming. I ended up going with audit_on_acl, audit_on_relation, etc. and reworked some of the other function names. I attached the v6 patch to my previous email, or you can find it on the CF page. -- - David Steele da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
On 3/25/15 7:46 AM, Sawada Masahiko wrote:
On Wed, Mar 25, 2015 at 12:23 PM, David Steele da...@pgmasters.net wrote:
On Wed, Mar 25, 2015 at 12:38 AM, David Steele da...@pgmasters.net wrote:
2. OBJECT auditing does not work before adding acl info to
pg_class.rel_acl.
In following situation, pg_audit can not audit OBJECT log.
$ cat postgresql.conf | grep audit
shared_preload_libraries = 'pg_audit'
pg_audit.role = 'hoge_user'
pg_audit.log = 'read, write'
$ psql -d postgres -U hoge_user
=# create table hoge(col int);
=# select * from hoge;
LOG: AUDIT: SESSION,3,1,READ,SELECT,,,select * from hoge;
OBJECT audit log is not logged here since pg_class.rel_acl is empty
yet. (Only logged SESSION log)
So after creating another unconcerned role and grant any privilege to
that user,
OBJECT audit is logged successfully.
Yes, object auditing does not work until some grants have been made to
the audit role.
=# create role bar_user;
=# grant select on hoge to bar_user;
=# select * from hoge;
LOG: AUDIT: SESSION,4,1,READ,SELECT,,,select * from hoge;
LOG: AUDIT: OBJECT,4,1,READ,SELECT,TABLE,public.hoge,select * from hoge;
The both OBJCET and SESSION log are logged.
Looks right to me. If you don't want the session logging then disable
pg_audit.log.
Session and object logging are completely independent from each other:
one or the other, or both, or neither can be enabled at any time.
It means that OBJECT log is not logged just after creating table, even
if that table is touched by its owner.
To write OBJECT log, we need to grant privilege to role at least. right?
Exactly. Privileges must be granted to the audit role in order for
object auditing to work.
The table owner always can touch its table.
So does it lead that table owner can get its table information while
hiding OBJECT logging?
Yes, the table owner would be able to access the table without object
logging if grants to that table were not made to the audit role. That
would also be true for any other user that had grants on the table.
The purpose of object auditing is to allow more fine-grained control and
is intended to be used in situations where you only want to audit some
things, rather than all things. Logging everything is better done with
the session logging.
However, object logging does yield more information since it lists every
table that was touched by the statement, so there may be cases where
you'd like to object log everything. In that case I'd recommend writing
a bit of plpgsql code to create the grants.
Also I looked into latest patch again.
Here are two review comment.
1.
typedef struct
{
int64 statementId;
int64 substatementId;
Both statementId and substatementId could be negative number.
I think these should be uint64 instead.
True. I did this because printf formatting for uint64 seems to be vary
across platforms. int64 formatting is more standard and still gives
more than enough IDs.
I could change it back to uint64 if you have a portable way to modify
the sprintf at line 507.
2.
I got ERROR when executing function uses cursor.
1) create empty table (hoge table)
2) create test function as follows.
create function test() returns int as $$
declare
cur1 cursor for select * from hoge;
tmp int;
begin
open cur1;
fetch cur1 into tmp;
return tmp;
end$$
language plpgsql ;
3) execute test function (got ERROR)
=# select test();
LOG: AUDIT: SESSION,6,1,READ,SELECT,,,selecT test();
LOG: AUDIT: SESSION,6,2,FUNCTION,EXECUTE,FUNCTION,public.test,selecT test();
LOG: AUDIT: SESSION,6,3,READ,SELECT,,,select * from hoge
CONTEXT: PL/pgSQL function test() line 6 at OPEN
ERROR: pg_audit stack is already empty
STATEMENT: selecT test();
It seems like that the item in stack is already freed by deleting
pg_audit memory context (in MemoryContextDelete()),
before calling stack_pop in dropping of top-level Portal.
Good catch, I'll add this to my test cases and work on a fix. I think I
see a good way to approach it.
--
- David Steele
da...@pgmasters.net
signature.asc
Description: OpenPGP digital signature
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
On Wed, Mar 25, 2015 at 12:23 PM, David Steele da...@pgmasters.net wrote:
On Wed, Mar 25, 2015 at 12:38 AM, David Steele da...@pgmasters.net wrote:
2. OBJECT auditing does not work before adding acl info to
pg_class.rel_acl.
In following situation, pg_audit can not audit OBJECT log.
$ cat postgresql.conf | grep audit
shared_preload_libraries = 'pg_audit'
pg_audit.role = 'hoge_user'
pg_audit.log = 'read, write'
$ psql -d postgres -U hoge_user
=# create table hoge(col int);
=# select * from hoge;
LOG: AUDIT: SESSION,3,1,READ,SELECT,,,select * from hoge;
OBJECT audit log is not logged here since pg_class.rel_acl is empty
yet. (Only logged SESSION log)
So after creating another unconcerned role and grant any privilege to that
user,
OBJECT audit is logged successfully.
Yes, object auditing does not work until some grants have been made to
the audit role.
=# create role bar_user;
=# grant select on hoge to bar_user;
=# select * from hoge;
LOG: AUDIT: SESSION,4,1,READ,SELECT,,,select * from hoge;
LOG: AUDIT: OBJECT,4,1,READ,SELECT,TABLE,public.hoge,select * from hoge;
The both OBJCET and SESSION log are logged.
Looks right to me. If you don't want the session logging then disable
pg_audit.log.
Session and object logging are completely independent from each other:
one or the other, or both, or neither can be enabled at any time.
It means that OBJECT log is not logged just after creating table, even
if that table is touched by its owner.
To write OBJECT log, we need to grant privilege to role at least. right?
Exactly. Privileges must be granted to the audit role in order for
object auditing to work.
The table owner always can touch its table.
So does it lead that table owner can get its table information while
hiding OBJECT logging?
Also I looked into latest patch again.
Here are two review comment.
1.
typedef struct
{
int64 statementId;
int64 substatementId;
Both statementId and substatementId could be negative number.
I think these should be uint64 instead.
2.
I got ERROR when executing function uses cursor.
1) create empty table (hoge table)
2) create test function as follows.
create function test() returns int as $$
declare
cur1 cursor for select * from hoge;
tmp int;
begin
open cur1;
fetch cur1 into tmp;
return tmp;
end$$
language plpgsql ;
3) execute test function (got ERROR)
=# select test();
LOG: AUDIT: SESSION,6,1,READ,SELECT,,,selecT test();
LOG: AUDIT: SESSION,6,2,FUNCTION,EXECUTE,FUNCTION,public.test,selecT test();
LOG: AUDIT: SESSION,6,3,READ,SELECT,,,select * from hoge
CONTEXT: PL/pgSQL function test() line 6 at OPEN
ERROR: pg_audit stack is already empty
STATEMENT: selecT test();
It seems like that the item in stack is already freed by deleting
pg_audit memory context (in MemoryContextDelete()),
before calling stack_pop in dropping of top-level Portal.
Regards,
---
Sawada Masahiko
--
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] Auditing extension for PostgreSQL (Take 2)
On Tue, Mar 24, 2015 at 3:17 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Sawada Masahiko wrote: I tied to look into latest patch, but got following error. masahiko [pg_audit] $ LANG=C make gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -g -fpic -I. -I. -I../../src/include -D_GNU_SOURCE -c -o pg_audit.o pg_audit.c pg_audit.c: In function 'log_audit_event': pg_audit.c:456: warning: ISO C90 forbids mixed declarations and code pg_audit.c: In function 'pg_audit_ddl_command_end': pg_audit.c:1436: error: 'pg_event_trigger_expand_command' undeclared (first use in this function) You need to apply my deparsing patch first, last version of which I posted here: https://www.postgresql.org/message-id/20150316234406.gh3...@alvh.no-ip.org Thank you for the info. I've applied these patchese successfully. I looked into this module, and had a few comments as follows. 1. pg_audit audits only one role currently. In currently code, we can not multiple user as auditing user. Why? (Sorry if this topic already has been discussed.) 2. OBJECT auditing does not work before adding acl info to pg_class.rel_acl. In following situation, pg_audit can not audit OBJECT log. $ cat postgresql.conf | grep audit shared_preload_libraries = 'pg_audit' pg_audit.role = 'hoge_user' pg_audit.log = 'read, write' $ psql -d postgres -U hoge_user =# create table hoge(col int); =# select * from hoge; LOG: AUDIT: SESSION,3,1,READ,SELECT,,,select * from hoge; OBJECT audit log is not logged here since pg_class.rel_acl is empty yet. (Only logged SESSION log) So after creating another unconcerned role and grant any privilege to that user, OBJECT audit is logged successfully. =# create role bar_user; =# grant select on hoge to bar_user; =# select * from hoge; LOG: AUDIT: SESSION,4,1,READ,SELECT,,,select * from hoge; LOG: AUDIT: OBJECT,4,1,READ,SELECT,TABLE,public.hoge,select * from hoge; The both OBJCET and SESSION log are logged. 3. pg_audit logged OBJECT log even EXPLAIN command. EXPLAIN command does not touch the table actually, but pg_audit writes audit OBJECT log. I'm not sure we need to log it. Is it intentional? Regards, --- Sawada Masahiko -- 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] Auditing extension for PostgreSQL (Take 2)
Hi Sawada, Thank you for taking the time to look at the patch. On 3/24/15 10:28 AM, Sawada Masahiko wrote: I've applied these patchese successfully. I looked into this module, and had a few comments as follows. 1. pg_audit audits only one role currently. In currently code, we can not multiple user as auditing user. Why? (Sorry if this topic already has been discussed.) There is only one master audit role in a bid for simplicity. However, there are two ways you can practically have multiple audit roles (both are mentioned in the docs): 1) The audit role honors inheritance so you can grant all your audit roles to the master role set in pg_audit.role and all the roles will be audited. 2) Since pg_audit.role is a GUC, you can set a different audit role per database by using ALTER DATABASE ... SET. You can set the GUC per logon role as well though that would probably make things very complicated. The GUC is SUSET so normal users cannot tamper with it. 2. OBJECT auditing does not work before adding acl info to pg_class.rel_acl. In following situation, pg_audit can not audit OBJECT log. $ cat postgresql.conf | grep audit shared_preload_libraries = 'pg_audit' pg_audit.role = 'hoge_user' pg_audit.log = 'read, write' $ psql -d postgres -U hoge_user =# create table hoge(col int); =# select * from hoge; LOG: AUDIT: SESSION,3,1,READ,SELECT,,,select * from hoge; OBJECT audit log is not logged here since pg_class.rel_acl is empty yet. (Only logged SESSION log) So after creating another unconcerned role and grant any privilege to that user, OBJECT audit is logged successfully. Yes, object auditing does not work until some grants have been made to the audit role. =# create role bar_user; =# grant select on hoge to bar_user; =# select * from hoge; LOG: AUDIT: SESSION,4,1,READ,SELECT,,,select * from hoge; LOG: AUDIT: OBJECT,4,1,READ,SELECT,TABLE,public.hoge,select * from hoge; The both OBJCET and SESSION log are logged. Looks right to me. If you don't want the session logging then disable pg_audit.log. Session and object logging are completely independent from each other: one or the other, or both, or neither can be enabled at any time. 3. pg_audit logged OBJECT log even EXPLAIN command. EXPLAIN command does not touch the table actually, but pg_audit writes audit OBJECT log. I'm not sure we need to log it. Is it intentional? This is intentional. They are treated as queries since in production they should be relatively rare (that is, not part of a normal function or process) and it's good information to have because EXPLAIN can be used to determine the number of rows in a table, and could also be used to figure out when data is added or removed from a table. In essence, it is a query even if it does not return row data. If that sounds paranoid, well, auditing is all about paranoia! -- - David Steele da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
Hi David, Thank you for your answer! On Wed, Mar 25, 2015 at 12:38 AM, David Steele da...@pgmasters.net wrote: Hi Sawada, Thank you for taking the time to look at the patch. On 3/24/15 10:28 AM, Sawada Masahiko wrote: I've applied these patchese successfully. I looked into this module, and had a few comments as follows. 1. pg_audit audits only one role currently. In currently code, we can not multiple user as auditing user. Why? (Sorry if this topic already has been discussed.) There is only one master audit role in a bid for simplicity. However, there are two ways you can practically have multiple audit roles (both are mentioned in the docs): 1) The audit role honors inheritance so you can grant all your audit roles to the master role set in pg_audit.role and all the roles will be audited. 2) Since pg_audit.role is a GUC, you can set a different audit role per database by using ALTER DATABASE ... SET. You can set the GUC per logon role as well though that would probably make things very complicated. The GUC is SUSET so normal users cannot tamper with it. I understood. 2. OBJECT auditing does not work before adding acl info to pg_class.rel_acl. In following situation, pg_audit can not audit OBJECT log. $ cat postgresql.conf | grep audit shared_preload_libraries = 'pg_audit' pg_audit.role = 'hoge_user' pg_audit.log = 'read, write' $ psql -d postgres -U hoge_user =# create table hoge(col int); =# select * from hoge; LOG: AUDIT: SESSION,3,1,READ,SELECT,,,select * from hoge; OBJECT audit log is not logged here since pg_class.rel_acl is empty yet. (Only logged SESSION log) So after creating another unconcerned role and grant any privilege to that user, OBJECT audit is logged successfully. Yes, object auditing does not work until some grants have been made to the audit role. =# create role bar_user; =# grant select on hoge to bar_user; =# select * from hoge; LOG: AUDIT: SESSION,4,1,READ,SELECT,,,select * from hoge; LOG: AUDIT: OBJECT,4,1,READ,SELECT,TABLE,public.hoge,select * from hoge; The both OBJCET and SESSION log are logged. Looks right to me. If you don't want the session logging then disable pg_audit.log. Session and object logging are completely independent from each other: one or the other, or both, or neither can be enabled at any time. It means that OBJECT log is not logged just after creating table, even if that table is touched by its owner. To write OBJECT log, we need to grant privilege to role at least. right? 3. pg_audit logged OBJECT log even EXPLAIN command. EXPLAIN command does not touch the table actually, but pg_audit writes audit OBJECT log. I'm not sure we need to log it. Is it intentional? This is intentional. They are treated as queries since in production they should be relatively rare (that is, not part of a normal function or process) and it's good information to have because EXPLAIN can be used to determine the number of rows in a table, and could also be used to figure out when data is added or removed from a table. In essence, it is a query even if it does not return row data. Okay, I understood. Regards, --- Sawada Masahiko -- 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] Auditing extension for PostgreSQL (Take 2)
On Wed, Mar 25, 2015 at 12:38 AM, David Steele da...@pgmasters.net wrote: 2. OBJECT auditing does not work before adding acl info to pg_class.rel_acl. In following situation, pg_audit can not audit OBJECT log. $ cat postgresql.conf | grep audit shared_preload_libraries = 'pg_audit' pg_audit.role = 'hoge_user' pg_audit.log = 'read, write' $ psql -d postgres -U hoge_user =# create table hoge(col int); =# select * from hoge; LOG: AUDIT: SESSION,3,1,READ,SELECT,,,select * from hoge; OBJECT audit log is not logged here since pg_class.rel_acl is empty yet. (Only logged SESSION log) So after creating another unconcerned role and grant any privilege to that user, OBJECT audit is logged successfully. Yes, object auditing does not work until some grants have been made to the audit role. =# create role bar_user; =# grant select on hoge to bar_user; =# select * from hoge; LOG: AUDIT: SESSION,4,1,READ,SELECT,,,select * from hoge; LOG: AUDIT: OBJECT,4,1,READ,SELECT,TABLE,public.hoge,select * from hoge; The both OBJCET and SESSION log are logged. Looks right to me. If you don't want the session logging then disable pg_audit.log. Session and object logging are completely independent from each other: one or the other, or both, or neither can be enabled at any time. It means that OBJECT log is not logged just after creating table, even if that table is touched by its owner. To write OBJECT log, we need to grant privilege to role at least. right? Exactly. Privileges must be granted to the audit role in order for object auditing to work. -- - David Steele da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
Thanks for the review, Abhijit.
On 3/23/15 1:31 AM, Abhijit Menon-Sen wrote:
At 2015-02-24 11:22:41 -0500, da...@pgmasters.net wrote:
Patch v3 is attached.
+
+/* Function execution */
+LOG_MISC = (1 5),
The comment above LOG_MISC should be changed.
Fixed.
More fundamentally, this classification makes it easy to reuse LOGSTMT_*
(and a nice job you've done of that, with just a few additional special
cases), I don't think this level is quite enough for our needs. I think
it should at least be possible to specifically log commands that affect
privileges and roles.
I agree, but this turns out to be easier said than done. In the prior
code for instance, CREATE ROLE was classified as USER, while ALTER ROLE
.. RENAME was classified as DDL. This is because any rename gets the
command tag T_RenameStmt. CreateCommandTag does return ALTER ROLE,
but now we're in the realm of string-matching again which is not my
favorite thing. Let me see if there is a clean way to get this
accomplished. I've also felt this is the one thing I'd like to see
broken out.
I'm fond of finer categorisation for DDL as well, but I could live with
all DDL being lumped together.
I'm experimenting with a few approaches to do this without reintroducing
switch statements to test every command. That will require core changes,
but I think we can find an acceptable arrangement. I'll post a proof of
concept in a few days.
I also think finer-grained categorization would be best accomplished
with some core changes. It seemed too late to get those in for 9.5 so I
decided to proceed with what I knew could be done reliably with the idea
to improve it with core changes going forward.
I look forward to your proof-of-concept.
+ * Takes an AuditEvent and, if it log_check(), writes it to the audit
log.
I don't think log_check is the most useful name, because this sentence
doesn't tell me what the function may do. Similarly, I would prefer to
have log_acl_check be renamed acl_grants_audit or similar. (These are
all static functions anyway, I don't think a log_ prefix is needed.)
log_check() has become somewhat vestigial at this point since it is only
called from one place - I've been considering removing it and merging
into log_audit_event(). For the moment I've improved the comments.
I like acl_grants_audit() and agree that it's a clearer name. I'll
incorporate that into the next version and apply the same scheme to the
other ACL functionsas well as do a general review of naming.
+/* Free the column set */
+bms_free(tmpSet);
(An aside, really: there are lots of comments like this, which I don't
think add anything to understanding the code, and should be removed.)
I generally feel like you can't have too many comments. I think even
the less interesting/helpful comments help break the code into
functional sections for readability.
+/*
+ * We don't have access to the parsetree here, so we have to
generate
+ * the node type, object type, and command tag by decoding
+ * rte-requiredPerms and rte-relkind.
+ */
+auditEvent.logStmtLevel = LOGSTMT_MOD;
(I am also trying to find a way to avoid having to do this.)
That would be excellent.
+/* Set object type based on relkind */
+switch (class-relkind)
+{
+case RELKIND_RELATION:
+utilityAuditEvent.objectType =
OBJECT_TYPE_TABLE;
+break;
This occurs elsewhere too. But I suppose new relkinds are added less
frequently than new commands.
Well, that's the hope at least. I should mention that ALL statements
will be logged no matter what additional classification happens. The
amount of information returned may not be ideal, but nothing is ever
excluded from logging (depending on the classes selected, of course).
Again on a larger level, I'm not sure how I feel about _avoiding_ the
use of event triggers for audit logging. Regardless of whether we use
the deparse code (which I personally think is a good idea; Álvaro has
been working on it, and it looks very nice) to log extra information,
using the object access hook inevitably means we have to reimplement
the identification/classification code here.
In old pgaudit, I think that extra effort is justified by the need to
be backwards compatible with pre-event trigger releases. In a 9.5-only
version, I am not at all convinced that this makes sense.
Thoughts?
I was nervous about basing pg_audit on code that I wasn't sure would be
committed (at the time). Since pg_event_trigger_get_creation_commands()
is tied up with deparse, I honestly didn't feel like the triggers were
bringing much to the table.
That being said, I agree that the deparse code is very useful and now
looks certain to be committed for 9.5.
I have prepared a patch that brings event triggers and deparse back to
pg_audit
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
Sawada Masahiko wrote: I tied to look into latest patch, but got following error. masahiko [pg_audit] $ LANG=C make gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -g -fpic -I. -I. -I../../src/include -D_GNU_SOURCE -c -o pg_audit.o pg_audit.c pg_audit.c: In function 'log_audit_event': pg_audit.c:456: warning: ISO C90 forbids mixed declarations and code pg_audit.c: In function 'pg_audit_ddl_command_end': pg_audit.c:1436: error: 'pg_event_trigger_expand_command' undeclared (first use in this function) You need to apply my deparsing patch first, last version of which I posted here: https://www.postgresql.org/message-id/20150316234406.gh3...@alvh.no-ip.org -- Á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] Auditing extension for PostgreSQL (Take 2)
On Tue, Mar 24, 2015 at 1:40 AM, David Steele da...@pgmasters.net wrote:
Thanks for the review, Abhijit.
On 3/23/15 1:31 AM, Abhijit Menon-Sen wrote:
At 2015-02-24 11:22:41 -0500, da...@pgmasters.net wrote:
Patch v3 is attached.
+
+/* Function execution */
+LOG_MISC = (1 5),
The comment above LOG_MISC should be changed.
Fixed.
More fundamentally, this classification makes it easy to reuse LOGSTMT_*
(and a nice job you've done of that, with just a few additional special
cases), I don't think this level is quite enough for our needs. I think
it should at least be possible to specifically log commands that affect
privileges and roles.
I agree, but this turns out to be easier said than done. In the prior
code for instance, CREATE ROLE was classified as USER, while ALTER ROLE
.. RENAME was classified as DDL. This is because any rename gets the
command tag T_RenameStmt. CreateCommandTag does return ALTER ROLE,
but now we're in the realm of string-matching again which is not my
favorite thing. Let me see if there is a clean way to get this
accomplished. I've also felt this is the one thing I'd like to see
broken out.
I'm fond of finer categorisation for DDL as well, but I could live with
all DDL being lumped together.
I'm experimenting with a few approaches to do this without reintroducing
switch statements to test every command. That will require core changes,
but I think we can find an acceptable arrangement. I'll post a proof of
concept in a few days.
I also think finer-grained categorization would be best accomplished
with some core changes. It seemed too late to get those in for 9.5 so I
decided to proceed with what I knew could be done reliably with the idea
to improve it with core changes going forward.
I look forward to your proof-of-concept.
+ * Takes an AuditEvent and, if it log_check(), writes it to the audit
log.
I don't think log_check is the most useful name, because this sentence
doesn't tell me what the function may do. Similarly, I would prefer to
have log_acl_check be renamed acl_grants_audit or similar. (These are
all static functions anyway, I don't think a log_ prefix is needed.)
log_check() has become somewhat vestigial at this point since it is only
called from one place - I've been considering removing it and merging
into log_audit_event(). For the moment I've improved the comments.
I like acl_grants_audit() and agree that it's a clearer name. I'll
incorporate that into the next version and apply the same scheme to the
other ACL functionsas well as do a general review of naming.
+/* Free the column set */
+bms_free(tmpSet);
(An aside, really: there are lots of comments like this, which I don't
think add anything to understanding the code, and should be removed.)
I generally feel like you can't have too many comments. I think even
the less interesting/helpful comments help break the code into
functional sections for readability.
+/*
+ * We don't have access to the parsetree here, so we have to
generate
+ * the node type, object type, and command tag by decoding
+ * rte-requiredPerms and rte-relkind.
+ */
+auditEvent.logStmtLevel = LOGSTMT_MOD;
(I am also trying to find a way to avoid having to do this.)
That would be excellent.
+/* Set object type based on relkind */
+switch (class-relkind)
+{
+case RELKIND_RELATION:
+utilityAuditEvent.objectType =
OBJECT_TYPE_TABLE;
+break;
This occurs elsewhere too. But I suppose new relkinds are added less
frequently than new commands.
Well, that's the hope at least. I should mention that ALL statements
will be logged no matter what additional classification happens. The
amount of information returned may not be ideal, but nothing is ever
excluded from logging (depending on the classes selected, of course).
Again on a larger level, I'm not sure how I feel about _avoiding_ the
use of event triggers for audit logging. Regardless of whether we use
the deparse code (which I personally think is a good idea; Álvaro has
been working on it, and it looks very nice) to log extra information,
using the object access hook inevitably means we have to reimplement
the identification/classification code here.
In old pgaudit, I think that extra effort is justified by the need to
be backwards compatible with pre-event trigger releases. In a 9.5-only
version, I am not at all convinced that this makes sense.
Thoughts?
I was nervous about basing pg_audit on code that I wasn't sure would be
committed (at the time). Since pg_event_trigger_get_creation_commands()
is tied up with deparse, I honestly didn't feel like the triggers were
bringing much to the table.
That being said, I agree that the deparse code is very useful and now
looks certain to be
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
On 3/23/15 1:39 PM, Sawada Masahiko wrote: On Tue, Mar 24, 2015 at 1:40 AM, David Steele da...@pgmasters.net wrote: I have prepared a patch that brings event triggers and deparse back to pg_audit based on the Alvaro's dev/deparse branch at git://git.postgresql.org/git/2ndquadrant_bdr.git (commit 0447fc5). I've updated the unit tests accordingly. I've been hesitant to post this patch as it will not work in master (though it will compile), but I don't want to hold on to it any longer since the end of the CF is nominally just weeks away. If you want to run the patch in master, you'll need to disable the pg_audit_ddl_command_end trigger. Hi, I tied to look into latest patch, but got following error. masahiko [pg_audit] $ LANG=C make gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -g -fpic -I. -I. -I../../src/include -D_GNU_SOURCE -c -o pg_audit.o pg_audit.c pg_audit.c: In function 'log_audit_event': pg_audit.c:456: warning: ISO C90 forbids mixed declarations and code pg_audit.c: In function 'pg_audit_ddl_command_end': pg_audit.c:1436: error: 'pg_event_trigger_expand_command' undeclared (first use in this function) pg_audit.c:1436: error: (Each undeclared identifier is reported only once pg_audit.c:1436: error: for each function it appears in.) make: *** [pg_audit.o] Error 1 Am I missing something? It's my mistake. I indicated that this would compile under master - but that turns out not to be true because of this function. It will only compile cleanly in Alvaro's branch mentioned above. My apologies - this is why I have been hesitant to post this patch before. You are welcome to try with Alvaro's deparse branch or wait until it has been committed to master. I've attached patch v5 only to cleanup the warnings you saw. -- - David Steele da...@pgmasters.net diff --git a/contrib/Makefile b/contrib/Makefile index 195d447..d8e75f4 100644 --- a/contrib/Makefile +++ b/contrib/Makefile @@ -29,6 +29,7 @@ SUBDIRS = \ pageinspect \ passwordcheck \ pg_archivecleanup \ + pg_audit\ pg_buffercache \ pg_freespacemap \ pg_prewarm \ diff --git a/contrib/pg_audit/Makefile b/contrib/pg_audit/Makefile new file mode 100644 index 000..32bc6d9 --- /dev/null +++ b/contrib/pg_audit/Makefile @@ -0,0 +1,20 @@ +# pg_audit/Makefile + +MODULE = pg_audit +MODULE_big = pg_audit +OBJS = pg_audit.o + +EXTENSION = pg_audit + +DATA = pg_audit--1.0.0.sql + +ifdef USE_PGXS +PG_CONFIG = pg_config +PGXS := $(shell $(PG_CONFIG) --pgxs) +include $(PGXS) +else +subdir = contrib/pg_audit +top_builddir = ../.. +include $(top_builddir)/src/Makefile.global +include $(top_srcdir)/contrib/contrib-global.mk +endif diff --git a/contrib/pg_audit/pg_audit--1.0.0.sql b/contrib/pg_audit/pg_audit--1.0.0.sql new file mode 100644 index 000..9d9ee83 --- /dev/null +++ b/contrib/pg_audit/pg_audit--1.0.0.sql @@ -0,0 +1,22 @@ +/* pg_audit/pg_audit--1.0.0.sql */ + +-- complain if script is sourced in psql, rather than via CREATE EXTENSION +\echo Use CREATE EXTENSION pg_audit to load this file.\quit + +CREATE FUNCTION pg_audit_ddl_command_end() + RETURNS event_trigger + LANGUAGE C + AS 'MODULE_PATHNAME', 'pg_audit_ddl_command_end'; + +CREATE EVENT TRIGGER pg_audit_ddl_command_end + ON ddl_command_end + EXECUTE PROCEDURE pg_audit_ddl_command_end(); + +CREATE FUNCTION pg_audit_sql_drop() + RETURNS event_trigger + LANGUAGE C + AS 'MODULE_PATHNAME', 'pg_audit_sql_drop'; + +CREATE EVENT TRIGGER pg_audit_sql_drop + ON sql_drop + EXECUTE PROCEDURE pg_audit_sql_drop(); diff --git a/contrib/pg_audit/pg_audit.c b/contrib/pg_audit/pg_audit.c new file mode 100644 index 000..65c8ed2 --- /dev/null +++ b/contrib/pg_audit/pg_audit.c @@ -0,0 +1,1712 @@ +/*-- + * pg_audit.c + * + * An auditing extension for PostgreSQL. Improves on standard statement logging + * by adding more logging classes, object level logging, and providing + * fully-qualified object names for all DML and many DDL statements (See + * pg_audit.sgml for details). + * + * Copyright (c) 2014-2015, PostgreSQL Global Development Group + * + * IDENTIFICATION + * contrib/pg_audit/pg_audit.c + *-- + */ +#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 catalog/namespace.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
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
At 2015-02-24 11:22:41 -0500, da...@pgmasters.net wrote:
Patch v3 is attached.
[…]
+/* Log class enum used to represent bits in auditLogBitmap */
+enum LogClass
+{
+ LOG_NONE = 0,
+
+ /* SELECT */
+ LOG_READ = (1 0),
+
+ /* INSERT, UPDATE, DELETE, TRUNCATE */
+ LOG_WRITE = (1 1),
+
+ /* DDL: CREATE/DROP/ALTER */
+ LOG_DDL = (1 2),
+
+ /* Function execution */
+ LOG_FUNCTION = (1 4),
+
+ /* Function execution */
+ LOG_MISC = (1 5),
+
+ /* Absolutely everything */
+ LOG_ALL = ~(uint64)0
+};
The comment above LOG_MISC should be changed.
More fundamentally, this classification makes it easy to reuse LOGSTMT_*
(and a nice job you've done of that, with just a few additional special
cases), I don't think this level is quite enough for our needs. I think
it should at least be possible to specifically log commands that affect
privileges and roles.
I'm fond of finer categorisation for DDL as well, but I could live with
all DDL being lumped together.
I'm experimenting with a few approaches to do this without reintroducing
switch statements to test every command. That will require core changes,
but I think we can find an acceptable arrangement. I'll post a proof of
concept in a few days.
+ * Takes an AuditEvent and, if it log_check(), writes it to the audit
log.
I don't think log_check is the most useful name, because this sentence
doesn't tell me what the function may do. Similarly, I would prefer to
have log_acl_check be renamed acl_grants_audit or similar. (These are
all static functions anyway, I don't think a log_ prefix is needed.)
+ /* Free the column set */
+ bms_free(tmpSet);
(An aside, really: there are lots of comments like this, which I don't
think add anything to understanding the code, and should be removed.)
+ /*
+ * We don't have access to the parsetree here, so we have to
generate
+ * the node type, object type, and command tag by decoding
+ * rte-requiredPerms and rte-relkind.
+ */
+ auditEvent.logStmtLevel = LOGSTMT_MOD;
(I am also trying to find a way to avoid having to do this.)
+ /* Set object type based on relkind */
+ switch (class-relkind)
+ {
+ case RELKIND_RELATION:
+ utilityAuditEvent.objectType =
OBJECT_TYPE_TABLE;
+ break;
This occurs elsewhere too. But I suppose new relkinds are added less
frequently than new commands.
Again on a larger level, I'm not sure how I feel about _avoiding_ the
use of event triggers for audit logging. Regardless of whether we use
the deparse code (which I personally think is a good idea; Álvaro has
been working on it, and it looks very nice) to log extra information,
using the object access hook inevitably means we have to reimplement
the identification/classification code here.
In old pgaudit, I think that extra effort is justified by the need to
be backwards compatible with pre-event trigger releases. In a 9.5-only
version, I am not at all convinced that this makes sense.
Thoughts?
-- 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] Auditing extension for PostgreSQL (Take 2)
On 2/23/15 10:59 AM, David Steele wrote:
On 2/17/15 10:34 AM, Stephen Frost wrote:
There seems to be a number of places which are 'pgaudit' and a bunch
that are 'pg_audit'. I'm guessing you were thinking 'pg_audit', but
it'd be good to clean up and make them all consistent.
Fixed, though I still left the file name as pgaudit.sgml since all but
one module follow that convention.
It turns out there are a few places where _ is not allowed. I've
reverted a few places to fix the doc build while maintaining the name as
pg_audit in the visible docs.
Perhaps I missed it, but it'd be good to point out that GUCs can be set
at various levels. I know we probably say that somewhere else, but it's
particularly relevant for this.
I added notes as suggested.
Patch v3 is attached.
--
- David Steele
da...@pgmasters.net
diff --git a/contrib/Makefile b/contrib/Makefile
index 195d447..d8e75f4 100644
--- a/contrib/Makefile
+++ b/contrib/Makefile
@@ -29,6 +29,7 @@ SUBDIRS = \
pageinspect \
passwordcheck \
pg_archivecleanup \
+ pg_audit\
pg_buffercache \
pg_freespacemap \
pg_prewarm \
diff --git a/contrib/pg_audit/Makefile b/contrib/pg_audit/Makefile
new file mode 100644
index 000..32bc6d9
--- /dev/null
+++ b/contrib/pg_audit/Makefile
@@ -0,0 +1,20 @@
+# pg_audit/Makefile
+
+MODULE = pg_audit
+MODULE_big = pg_audit
+OBJS = pg_audit.o
+
+EXTENSION = pg_audit
+
+DATA = pg_audit--1.0.0.sql
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = contrib/pg_audit
+top_builddir = ../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/contrib/pg_audit/pg_audit--1.0.0.sql
b/contrib/pg_audit/pg_audit--1.0.0.sql
new file mode 100644
index 000..2eee3b9
--- /dev/null
+++ b/contrib/pg_audit/pg_audit--1.0.0.sql
@@ -0,0 +1,4 @@
+/* pg_audit/pg_audit--1.0.0.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use CREATE EXTENSION pg_audit to load this file.\quit
diff --git a/contrib/pg_audit/pg_audit.c b/contrib/pg_audit/pg_audit.c
new file mode 100644
index 000..ead65a8
--- /dev/null
+++ b/contrib/pg_audit/pg_audit.c
@@ -0,0 +1,1102 @@
+/*--
+ * pg_audit.c
+ *
+ * An auditing extension for PostgreSQL. Improves on standard statement logging
+ * by adding more logging classes, object level logging, and providing
+ * fully-qualified object names for all DML and many DDL statements (See
+ * pg_audit.sgml for details).
+ *
+ * Copyright (c) 2014-2015, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * contrib/pg_audit/pg_audit.c
+
*--
+ */
+#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 catalog/namespace.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);
+
+/*
+ * auditRole is the string value of the pgaudit.role GUC, which contains the
+ * role for grant-based auditing.
+ */
+char *auditRole = NULL;
+
+/*
+ * auditLog is the string value of the pgaudit.log GUC, e.g. read, write, ddl
+ * (it's not used by the module but is required by DefineCustomStringVariable).
+ * Each token corresponds to a flag in enum LogClass below. We convert the list
+ * of tokens into a bitmap in auditLogBitmap for internal use.
+ */
+char *auditLog = NULL;
+static uint64 auditLogBitmap = 0;
+
+/*
+ * String constants for audit types - used when logging to distinguish session
+ * vs. object auditing.
+ */
+#define AUDIT_TYPE_OBJECT OBJECT
+#define AUDIT_TYPE_SESSION SESSION
+
+/*
+ * String constants for log classes - used when processing tokens in the
+ * pgaudit.log GUC.
+ */
+#define CLASS_DDL DDL
+#define CLASS_FUNCTION FUNCTION
+#define CLASS_MISC MISC
+#define CLASS_READ READ
+#define CLASS_WRITEWRITE
+
+#define CLASS_ALL ALL
+#define CLASS_NONE NONE
+
+/* Log class enum used to represent bits in auditLogBitmap */
+enum LogClass
+{
+ LOG_NONE = 0,
+
+ /* SELECT */
+ LOG_READ = (1 0),
+
+ /* INSERT, UPDATE, DELETE, TRUNCATE */
+
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
Hi Stephen, Thanks for your review. All fixed except for comments below: On 2/17/15 10:34 AM, Stephen Frost wrote: +/* + * Check privileges granted indirectly via role memberships. We do this in + * a separate pass to minimize expensive indirect membership tests. In + * particular, it's worth testing whether a given ACL entry grants any + * privileges still of interest before we perform the has_privs_of_role + * test. + */ I'm a bit on the fence about this.. Can you provide a use-case where doing this makes sense..? Does this mean I could grant admin_role1 to audit and then get auditing on everything that user1 has access to? That seems like it might be useful for environments where such roles already exist though it might end up covering more than is intended.. The idea is that if there are already ready-made roles to be audited then they don't need to be reconstituted for the audit role. You could just do: grant admin_role to audit; grant user_role to audit; Of course, we could list multiple roles in the pg_audit.role GUC, but I thought this would be easier to use and maintain since there was some worry about GUCs being fragile when they refer to database objects. + +# test.pl - pgAudit Unit Tests + This is definitiely nice.. diff --git a/doc/src/sgml/pgaudit.sgml b/doc/src/sgml/pgaudit.sgml new file mode 100644 index 000..f3f4ab9 --- /dev/null +++ b/doc/src/sgml/pgaudit.sgml @@ -0,0 +1,316 @@ +!-- doc/src/sgml/pgaudit.sgml -- + +sect1 id=pgaudit xreflabel=pgaudit + titlepg_audit/title There seems to be a number of places which are 'pgaudit' and a bunch that are 'pg_audit'. I'm guessing you were thinking 'pg_audit', but it'd be good to clean up and make them all consistent. Fixed, though I still left the file name as pgaudit.sgml since all but one module follow that convention. +para + Session auditing allows the logging of all commands that are executed by + a user in the backend. Each command is logged with a single entry and + includes the audit type (e.g. literalSESSION/literal), command type + (e.g. literalCREATE TABLE/literal, literalSELECT/literal) and + statement (e.g. literalselect * from test/literal). + + Fully-qualified names and object types will be logged for + literalCREATE/literal, literalUPDATE/literal, and + literalDROP/literal commands on literalTABLE/literal, + literalMATVIEW/literal, literalVIEW/literal, + literalINDEX/literal, literalFOREIGN TABLE/literal, + literalCOMPOSITE TYPE/literal, literalINDEX/literal, and + literalSEQUENCE/literal objects as well as function calls. +/para Ah, you do have a list of what objects you get fully qualified names for, nice. Are there obvious omissions from that list..? If so, we might be able to change what happens with objectAccess to include them.. It seems like these are the objects where having a name really matters. I'm more interested in using the deparse code to handle fully-qualified names for additional objects rather than adding hooks. +sect3 + titleExamples/title + + para +Set literalpgaudit.log = 'read, ddl'/literal in +literalpostgresql.conf/literal. + /para Perhaps I missed it, but it'd be good to point out that GUCs can be set at various levels. I know we probably say that somewhere else, but it's particularly relevant for this. Yes, it's very relevant for this patch. Do you think it's enough to call out the functionality, or should I provide examples? Maybe a separate section just for this concept? Patch v2 is attached for all changes except the doc change above. -- - David Steele da...@pgmasters.net diff --git a/contrib/Makefile b/contrib/Makefile index 195d447..d8e75f4 100644 --- a/contrib/Makefile +++ b/contrib/Makefile @@ -29,6 +29,7 @@ SUBDIRS = \ pageinspect \ passwordcheck \ pg_archivecleanup \ + pg_audit\ pg_buffercache \ pg_freespacemap \ pg_prewarm \ diff --git a/contrib/pg_audit/Makefile b/contrib/pg_audit/Makefile new file mode 100644 index 000..32bc6d9 --- /dev/null +++ b/contrib/pg_audit/Makefile @@ -0,0 +1,20 @@ +# pg_audit/Makefile + +MODULE = pg_audit +MODULE_big = pg_audit +OBJS = pg_audit.o + +EXTENSION = pg_audit + +DATA = pg_audit--1.0.0.sql + +ifdef USE_PGXS +PG_CONFIG = pg_config +PGXS := $(shell $(PG_CONFIG) --pgxs) +include $(PGXS) +else +subdir = contrib/pg_audit +top_builddir = ../.. +include $(top_builddir)/src/Makefile.global +include $(top_srcdir)/contrib/contrib-global.mk +endif diff --git a/contrib/pg_audit/pg_audit--1.0.0.sql b/contrib/pg_audit/pg_audit--1.0.0.sql new file
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
On 15 February 2015 at 02:34, David Steele da...@pgmasters.net wrote: I've posted a couple of messages over the last few weeks about the work I've been doing on the pg_audit extension. The lack of response could be due to either universal acclaim or complete apathy, but in any case I think this is a very important topic so I want to give it another try. You mentioned you had been following the thread for some time and yet had not contributed to it. Did that indicate your acclaim for the earlier patch, or was that apathy? I think neither. People have been working on this feature for 9 months now, so you having to wait 9 days for a response is neither universal acclaim, nor apathy. I've waited much longer than that for Stephen to give the review he promised, but have not bad mouthed him for that wait, nor do I do so now. In your first post you had removed the author's email addresses, so they were likely unaware of your post. I certainly was. I've extensively reworked the code that was originally submitted by 2ndQuandrant. This is not an indictment of their work, but rather an attempt to redress concerns that were expressed by members of the community. I've used core functions to determine how audit events should be classified and simplified and tightened the code wherever possible. I've removed deparse and event triggers and opted for methods that rely only on existing hooks. In my last message I provided numerous examples of configuration, usage, and output which I hoped would alleviate concerns of complexity. I've also written a ton of unit tests to make sure that the code works as expected. Some people that have contributed ideas to this patch are from 2ndQuadrant, some are not. The main point is that we work together on things, rather than writing a slightly altered version and then claiming credit. If you want to help, please do. We give credit where its due, not to whoever touched the code last in some kind of bidding war. If we let this happen, we'd generate a flood of confusing patch versions and little would ever get committed. Let's keep to one thread and work to include everybody's ideas then we'll get something useful committed. -- 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] Auditing extension for PostgreSQL (Take 2)
On 2/18/15 8:25 AM, Simon Riggs wrote: On 15 February 2015 at 02:34, David Steele da...@pgmasters.net wrote: I've posted a couple of messages over the last few weeks about the work I've been doing on the pg_audit extension. The lack of response could be due to either universal acclaim or complete apathy, but in any case I think this is a very important topic so I want to give it another try. You mentioned you had been following the thread for some time and yet had not contributed to it. Did that indicate your acclaim for the earlier patch, or was that apathy? I think neither. In my case it actually was acclaim. I was happy with the direction things were going and had nothing in particular to add - and I didn't think a +1 from me was going to carry any weight with the community. I can see now that everyone's opinion matters here, so I'll be more active about weighing in when I think something is valuable. People have been working on this feature for 9 months now, so you having to wait 9 days for a response is neither universal acclaim, nor apathy. I've waited much longer than that for Stephen to give the review he promised, but have not bad mouthed him for that wait, nor do I do so now. In your first post you had removed the author's email addresses, so they were likely unaware of your post. I certainly was. I understand that, but with the CF closing I felt like I had to act. Abhijit's last comment on the thread was that he was no longer going to work on it in relation to 9.5. I felt that it was an important feature (and one that I have a lot of interest in), so that's when I got involved. I posted two messages, but I only addressed one of them directly to Abhijit. As you said, I'm new here and I'm still getting used to the way things are done. I've extensively reworked the code that was originally submitted by 2ndQuandrant. This is not an indictment of their work, but rather an attempt to redress concerns that were expressed by members of the community. I've used core functions to determine how audit events should be classified and simplified and tightened the code wherever possible. I've removed deparse and event triggers and opted for methods that rely only on existing hooks. In my last message I provided numerous examples of configuration, usage, and output which I hoped would alleviate concerns of complexity. I've also written a ton of unit tests to make sure that the code works as expected. Some people that have contributed ideas to this patch are from 2ndQuadrant, some are not. The main point is that we work together on things, rather than writing a slightly altered version and then claiming credit. If you want to help, please do. We give credit where its due, not to whoever touched the code last in some kind of bidding war. If we let this happen, we'd generate a flood of confusing patch versions and little would ever get committed. Agreed, and I apologize if I came off that way. It certainly wasn't my intention. I was hesitant because I had made so many changes and I wasn't sure how the authors would feel about it. I wrote to them privately to get their take on the situation. Let's keep to one thread and work to include everybody's ideas then we'll get something useful committed. I'm a little confused about how to proceed here. I created a new thread because the other patch had already been rejected. How should I handle that? -- - David Steele da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
David, I've CC'd Abhijit, the original author of pgaudit, as it seems likely he'd also be interested in this. * David Steele (da...@pgmasters.net) wrote: I've posted a couple of messages over the last few weeks about the work I've been doing on the pg_audit extension. The lack of response could be due to either universal acclaim or complete apathy, but in any case I think this is a very important topic so I want to give it another try. Thanks! It's certainly an important topic to a lot of folks; I imagine the lack of response is simply because people are busy. I've extensively reworked the code that was originally submitted by 2ndQuandrant. This is not an indictment of their work, but rather an attempt to redress concerns that were expressed by members of the community. I've used core functions to determine how audit events should be classified and simplified and tightened the code wherever possible. I've removed deparse and event triggers and opted for methods that rely only on existing hooks. In my last message I provided numerous examples of configuration, usage, and output which I hoped would alleviate concerns of complexity. I've also written a ton of unit tests to make sure that the code works as expected. The configuration approach you posted is definitely in-line with what I was trying to get at previously- thanks for putting some actual code behind it! While not a big fan of that other big RDBMS, I do like that this approach ends up being so similar in syntax. I realize this is not an ideal solution. Everybody (including me) wants something that is in core with real policies and more options. It's something that I am personally really eager to work on. But the reality is that's not going to happen for 9.5 and probably not for 9.6 either. Meanwhile, I believe the lack of some form of auditing is harming adoption of PostgreSQL, especially in the financial and government sectors. Agreed. The patch I've attached satisfies the requirements that I've had from customers in the past. I'm confident that if it gets out into the wild it will bring all kinds of criticism and comments which will be valuable in designing a robust in-core solution. This is definitely something that makes sense to me, particularly for such an important piece. I had argued previously that a contrib based solution would make it difficult to build an in-core solution, but others convinced me that it'd not only be possible but would probably be preferrable as we'd gain experience with the contrib module and, as you say, we'd be able to build a better in-core solution based on that experience. I'm submitting this patch to the Commitfest. I'll do everything I can to address the concerns of the community and I'm happy to provide more examples as needed. I'm hoping the sgml docs I've provided with the patch will cover any questions, but of course feedback is always appreciated. Glad you submitted it to the CommitFest. Just glancing through the code, it certainly looks a lot closer to being something which we could move forward with. Using existing functions to work out the categories (instead of massive switch statements) is certainly much cleaner and removing those large #ifdef blocks has made the code a lot easier to follow. Lastly, I really like all the unit tests.. Additional comments in-line follow. diff --git a/contrib/pg_audit/pg_audit.c b/contrib/pg_audit/pg_audit.c new file mode 100644 index 000..b3914ac --- /dev/null +++ b/contrib/pg_audit/pg_audit.c @@ -0,0 +1,1099 @@ +/*-- + * pg_audit.c + * + * An auditing extension for PostgreSQL. Improves on standard statement logging + * by adding more logging classes, object level logging, and providing + * fully-qualified object names for all DML and many DDL statements. It'd be good to quantify what 'many' means above. + * Copyright (c) 2014-2015, PostgreSQL Global Development Group + * + * IDENTIFICATION + * contrib/pg_prewarm/pg_prewarm.c Pretty sure this isn't pg_prewarm.c :) +/* + * String contants for audit types - used when logging to distinguish session + * vs. object auditing. + */ String constants +/* + * String contants for log classes - used when processing tokens in the + * pgaudit.log GUC. + */ Ditto. +/* String contants for logging commands */ Ditto. :) +/* + * This module collects AuditEvents from various sources (event triggers, and + * executor/utility hooks) and passes them to the log_audit_event() function. This isn't using event triggers any more, right? Doesn't look like it. I don't think that's a problem and it certainly seems to have simplified things quite a bit, but the comment should be updated. +/* + * Returns the oid of the role specified in pgaudit.role. + */ +static Oid +audit_role_oid() Couldn't you use get_role_oid() instead of having your own function..?
[HACKERS] Auditing extension for PostgreSQL (Take 2)
I've posted a couple of messages over the last few weeks about the work I've been doing on the pg_audit extension. The lack of response could be due to either universal acclaim or complete apathy, but in any case I think this is a very important topic so I want to give it another try. I've extensively reworked the code that was originally submitted by 2ndQuandrant. This is not an indictment of their work, but rather an attempt to redress concerns that were expressed by members of the community. I've used core functions to determine how audit events should be classified and simplified and tightened the code wherever possible. I've removed deparse and event triggers and opted for methods that rely only on existing hooks. In my last message I provided numerous examples of configuration, usage, and output which I hoped would alleviate concerns of complexity. I've also written a ton of unit tests to make sure that the code works as expected. Auditing has been a concern everywhere I've used or introduced PostgreSQL. Over time I've developed a reasonably comprehensive (but complex) system of triggers, tables and functions to provide auditing for customers. While this has addressed most requirements, there are always questions of auditing aborted transactions, DDL changes, configurability, etc. which I have been unable to satisfy. There has been some discussion of whether or not this module needs to be in contrib. One reason customers trust contrib is that the modules are assumed to be held to a higher standard than code found on GitHub. This has already been pointed out. But I believe another important reason is that they know packages will be made available for a variety of platforms, and bugs are more likely to be experienced by many users and not just a few (or one). For this reason my policy is not to run custom-compiled code in production. This is especially true for something as mission critical as a relational database. I realize this is not an ideal solution. Everybody (including me) wants something that is in core with real policies and more options. It's something that I am personally really eager to work on. But the reality is that's not going to happen for 9.5 and probably not for 9.6 either. Meanwhile, I believe the lack of some form of auditing is harming adoption of PostgreSQL, especially in the financial and government sectors. The patch I've attached satisfies the requirements that I've had from customers in the past. I'm confident that if it gets out into the wild it will bring all kinds of criticism and comments which will be valuable in designing a robust in-core solution. I'm submitting this patch to the Commitfest. I'll do everything I can to address the concerns of the community and I'm happy to provide more examples as needed. I'm hoping the sgml docs I've provided with the patch will cover any questions, but of course feedback is always appreciated. -- - David Steele da...@pgmasters.net diff --git a/contrib/Makefile b/contrib/Makefile index 195d447..d8e75f4 100644 --- a/contrib/Makefile +++ b/contrib/Makefile @@ -29,6 +29,7 @@ SUBDIRS = \ pageinspect \ passwordcheck \ pg_archivecleanup \ + pg_audit\ pg_buffercache \ pg_freespacemap \ pg_prewarm \ diff --git a/contrib/pg_audit/Makefile b/contrib/pg_audit/Makefile new file mode 100644 index 000..32bc6d9 --- /dev/null +++ b/contrib/pg_audit/Makefile @@ -0,0 +1,20 @@ +# pg_audit/Makefile + +MODULE = pg_audit +MODULE_big = pg_audit +OBJS = pg_audit.o + +EXTENSION = pg_audit + +DATA = pg_audit--1.0.0.sql + +ifdef USE_PGXS +PG_CONFIG = pg_config +PGXS := $(shell $(PG_CONFIG) --pgxs) +include $(PGXS) +else +subdir = contrib/pg_audit +top_builddir = ../.. +include $(top_builddir)/src/Makefile.global +include $(top_srcdir)/contrib/contrib-global.mk +endif diff --git a/contrib/pg_audit/pg_audit--1.0.0.sql b/contrib/pg_audit/pg_audit--1.0.0.sql new file mode 100644 index 000..2eee3b9 --- /dev/null +++ b/contrib/pg_audit/pg_audit--1.0.0.sql @@ -0,0 +1,4 @@ +/* pg_audit/pg_audit--1.0.0.sql */ + +-- complain if script is sourced in psql, rather than via CREATE EXTENSION +\echo Use CREATE EXTENSION pg_audit to load this file.\quit diff --git a/contrib/pg_audit/pg_audit.c b/contrib/pg_audit/pg_audit.c new file mode 100644 index 000..b3914ac --- /dev/null +++ b/contrib/pg_audit/pg_audit.c @@ -0,0 +1,1099 @@ +/*-- + * pg_audit.c + * + * An auditing extension for PostgreSQL. Improves on standard statement logging + * by adding more logging classes, object level logging, and providing + * fully-qualified object names for all DML and many DDL statements. + * + * Copyright (c) 2014-2015, PostgreSQL Global Development Group + * + * IDENTIFICATION + * contrib/pg_prewarm/pg_prewarm.c +
