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