Re: [COMMITTERS] pgsql: Add pg_audit, an auditing extension

On 05/28/2015 06:04 AM, Joshua D. Drake wrote:
> On 05/27/2015 07:02 PM, Stephen Frost wrote:
>> regardless of if they are included in the main git repo
>> or not.  Being in the repo represents the support of the overall
>> community and PGDG, which is, understandably in my view, highly valuable
>> to the organizations which look to PostgreSQL as an open source
>> solution for their needs.
>
> I can't get behind this. Yes, there is a mysticism about the "core"
> support of modules but it is just that "mysticism". People are going to
> run what the experts tell them to run. If pg_audit is that good the
> experts will tell people to use it...

Yeah, there are many popular tools and extensions that people use 
together with PostgreSQL. PostGIS, pgbouncer, slony, pgbarman etc. The 
criteria for what should be in contrib, in core, or a 3rd party 
extension is a contentious topic.

The argument here is basically that if something is in core, it's 
officially supported and endorsed by the PGDG. If that's the argument 
for putting this in contrib, then you have to ask yourself if the PGDG 
community is really willing to endorse this. I'm hearing a lot of 
pushback from other committers, which points to "no", even if all their 
technical arguments and worries turn out to be wrong.

I wrote the above without looking at the code or the documentation. I 
haven't followed the discussion at all. I'm now looking at the 
documentation, and have some comments based on just that:

* I think it's a bad idea for the audit records to go to the same log as 
other messages. Maybe that's useful as an option, but in general there 
is no reason to believe that the log format used for general logging is 
also a good format for audit logging. You probably wouldn't care about 
process ID for audit logging, for example. Also, how do you prevent 
spoofing audit records with something like "SELECT \nAUDIT: SESSION, 2, 
...". Maybe the log formatting options can be used to prevent that, but 
just by looking at the examples in the manual, I don't see how to do it.

* I don't understand how the pg_audit.role setting and the audit role 
system works.

* Using GUCs for configuring it seems like a bad idea, because:

1. it's not flexible enough. How do you specify that all READs on 
super_secret table must be logged, but on less_secret table, it's enough 
to log only WRITEs?

2. GUCs can be changed easily on-the-fly, and configured flexibly. But 
that's not really what you want for auditing. You want to have a clear 
specification in one place. You'd want it to be more like pg_hba.conf 
than postgresql.conf. Or more like Mandatory Access Control, rather than 
Discretionary Access Control.

A separate config file in $PGDATA would be a better way to configure 
this. You would then have all the configuration in one place, and that 
file could have a more flexible format, with per-schema rules etc. 
(Wouldn't have to implement all that in the first version, but that's 
the direction this should go to)


I recommend making pg_audit an external extension, not a contrib module. 
As an extension, it can have its own life-cycle and not be tied to 
PostgreSQL releases. That would be a huge benefit for pg_audit. There is 
a lot of potential for new features to be added: more flexible 
configuration, more details to be logged, more ways to log, email 
alerts, etc. As an extension, all of those things could be developed 
independent of the PostgreSQL life-cycle. If there is need to fix 
vulnerabilities or other bugs, those can also be fixed independently of 
PostgreSQL minor releases.

- Heikki