Re: pgaudit - an auditing extension for PostgreSQL

Hi

Here is an initial version of an auditing extension for Postgres to
generate log output suitable for compiling a comprehensive audit trail
of database operations.

Why auditing?

Various laws and regulations (HIPAA, PCI DSS, EU Data Protection
Directive etc.) as well as internal business requirements mandate
auditing at database level. While many proprietary and some open
source databases offer auditing facilities, Postgres does not currently
provide any kind of auditing feature. Availability of such a feature
will assist PostgreSQL's adoption in key sectors such as finance
and health.

About pgaudit

pgaudit uses Event Triggers to log unambiguous representation of DDL,
as well as a combination of executor and utility hooks for other
commands (DML, including SELECT, as well as other utility commands):

https://github.com/2ndQuadrant/pgaudit

To provide fully-featured auditing capability, pgaudit exploits the
capabilities of the new Event Trigger code, which 2ndQuadrant will be
submitting to core Postgres. Currently that means you'll have to
build against an enhanced version of Postgres [1]. However the
intention is that pgaudit will be both a useful module now (it is designed
to compile against 9.3 and 9.4), but will also serve as a demonstration
of features proposed for 9.5.

[1] "deparse" branch of git://git.postgresql.org/git/2ndquadrant_bdr.git

Here's some example log output:

LOG: [AUDIT],2014-04-30 17:13:55.202854+09,auditdb,ianb,ianb,DEFINITION,CREATE TABLE,TABLE,public.x,CREATE TABLE public.x (a pg_catalog.int4 , b pg_catalog.int4 ) WITH (oids=OFF)
LOG: [AUDIT],2014-04-30 17:14:06.548923+09,auditdb,ianb,ianb,WRITE,INSERT,TABLE,public.x,INSERT INTO x VALUES(1,1);
LOG: [AUDIT],2014-04-30 17:14:21.221879+09,auditdb,ianb,ianb,READ,SELECT,TABLE,public.x,SELECT * FROM x;
LOG: [AUDIT],2014-04-30 17:15:25.620213+09,auditdb,ianb,ianb,READ,SELECT,VIEW,public.v_x,SELECT * from v_x;
LOG: [AUDIT],2014-04-30 17:15:25.620262+09,auditdb,ianb,ianb,READ,SELECT,TABLE,public.x,SELECT * from v_x;
LOG: [AUDIT],2014-04-30 17:16:00.849868+09,auditdb,ianb,ianb,WRITE,UPDATE,TABLE,public.x,UPDATE x SET a=a+1;
LOG: [AUDIT],2014-04-30 17:16:18.291452+09,auditdb,ianb,ianb,ADMIN,VACUUM,,,VACUUM x;
LOG: [AUDIT],2014-04-30 17:18:01.08291+09,auditdb,ianb,ianb,DEFINITION,CREATE FUNCTION,FUNCTION,public.func_x(),CREATE FUNCTION public.func_x() RETURNS pg_catalog.int4 LANGUAGE sql VOLATILE CALLED ON NULL INPUT SECURITY INVOKER COST 100.000000 AS $dprs_$SELECT a FROM x LIMIT 1;$dprs_$
LOG: [AUDIT],2014-04-30 17:18:09.694755+09,auditdb,ianb,ianb,FUNCTION,EXECUTE,FUNCTION,public.func_x,SELECT * FROM func_x();
LOG: [AUDIT],2014-04-30 17:18:09.694865+09,auditdb,ianb,ianb,READ,SELECT,TABLE,public.x,SELECT * FROM func_x();
LOG: [AUDIT],2014-04-30 17:18:33.703007+09,auditdb,ianb,ianb,WRITE,DELETE,VIEW,public.v_x,DELETE FROM v_x;
LOG: [AUDIT],2014-04-30 17:18:33.703051+09,auditdb,ianb,ianb,WRITE,DELETE,TABLE,public.x,DELETE FROM v_x;
LOG: [AUDIT],2014-04-30 17:19:54.811244+09,auditdb,ianb,ianb,ADMIN,SET,,,set role ams;
LOG: [AUDIT],2014-04-30 17:19:57.039979+09,auditdb,ianb,ams,WRITE,INSERT,VIEW,public.v_x,INSERT INTO v_x VALUES(1,2);
LOG: [AUDIT],2014-04-30 17:19:57.040014+09,auditdb,ianb,ams,WRITE,INSERT,TABLE,public.x,INSERT INTO v_x VALUES(1,2);
LOG: [AUDIT],2014-04-30 17:20:02.059415+09,auditdb,ianb,ams,ADMIN,SET,,,SET role ianb;
LOG: [AUDIT],2014-04-30 17:20:09.840261+09,auditdb,ianb,ianb,DEFINITION,ALTER TABLE,TABLE,public.x,ALTER TABLE public.x ADD COLUMN c pg_catalog.int4
LOG: [AUDIT],2014-04-30 17:23:58.920342+09,auditdb,ianb,ianb,ADMIN,ALTER ROLE,,,ALTER USER ams SET search_path = 'foo';

How is this different to log_statement='all'?

1. pgaudit logs fully-qualified relation names, so you don't have to
wonder if "SELECT * FROM x" referred to "public.x" or "other.x".

2. pgaudit creates a log entry for each affected object, so you don't
have to wonder which tables "SELECT * FROM someview" accessed, and
it's easy to identify all accesses to a particular table.

3. pgaudit allows finer-grained control over what is logged. Commands
are classified into read, write, etc. and logging for these classes
can be individually enabled and disabled (either via pgaudit.log in
postgresql.conf, or as a per-database or per-user setting).

Here's a quick overview of how it works:

0. In 9.3 and 9.4, we build without USE_DEPARSE_FUNCTIONS. In the
deparse branch (which I'll call 9.5 for convenience), we build
with USE_DEPARSE_FUNCTIONS (set in the Makefile).

1. In 9.5, we create a ddl_command_end event trigger and use
pg_event_trigger_{get_creation_commands,expand_command} to log
a deparsed representation of any DDL commands supported by event
triggers.

2. We always use an sql_drop event trigger to log DROP commands, but
once 9.5 includes pg_event_trigger_get_deletion_commands() or some
equivalent, we'll use that functionality as well.

3. We use a ProcessUtility_hook to deal with other utility commands that
are not handled by #1 and #2. For example, DROP on global objects in
all versions and all non-DROP DDL for 9.3 or 9.4.

4. We use an ExecutorCheckPerms_hook to log SELECT and DML commands.

5. We use an object_access_hook and OAT_POST_CREATE/ALTER to handle
CREATE/ALTER on relations in 9.3/9.4. We use OAT_FUNCTION_EXECUTE
to log (non-catalog) function execution.

Planned future improvements include:

1. Additional logging facilities, including to a separate audit
log file and to syslog, and potentially logging to a table
(possibly via a bgworker process). Currently output is simply
emitted to the server log via ereport().

2. To implement per-object auditing configuration, it would be nice to use
extensible reloptions (or an equivalent mechanism)

Details such as output format, command classification etc. are provisional
and open to further discussion.

Authors: Ian Barwick, Abhijit Menon-Sen (2ndQuadrant).
See README.md for more details.

We welcome your feedback and suggestions.

Ian Barwick

The research leading to these results has received funding from the
European Union's Seventh Framework Programme (FP7/2007-2013) under
grant agreement n° 318633. http://axleproject.eu

--
Ian Barwick http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 05/01/2014 11:19 PM, Ian Barwick wrote:
>
> Here is an initial version of an auditing extension for Postgres to
> generate log output suitable for compiling a comprehensive audit trail
> of database operations.

Cool! Looking forward to seeing it around the 9.5 cycle.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

Ian,

* Ian Barwick (ian(at)2ndquadrant(dot)com) wrote:
> Here is an initial version of an auditing extension for Postgres to
> generate log output suitable for compiling a comprehensive audit trail
> of database operations.

Neat stuff.

> Why auditing?

Yeah, we really need to improve here. I've been hoping to make progress
on this and it looks like I'll finally have some time to.

> pgaudit uses Event Triggers to log unambiguous representation of DDL,
> as well as a combination of executor and utility hooks for other
> commands (DML, including SELECT, as well as other utility commands):

While certainly a good approach to minimize the changes needed to the
backend, I'd really like to see us be able to, say, log to a table and
have more fine-grained control over what is logged, without needing an
extension.

> 1. pgaudit logs fully-qualified relation names, so you don't have to
> wonder if "SELECT * FROM x" referred to "public.x" or "other.x".

Yeah, that's definitely an issue for any kind of real auditing.

> 2. pgaudit creates a log entry for each affected object, so you don't
> have to wonder which tables "SELECT * FROM someview" accessed, and
> it's easy to identify all accesses to a particular table.

Interesting- I'm a bit on the fence about this one. Perhaps you can
elaborate on the use-case for this?

> 3. pgaudit allows finer-grained control over what is logged. Commands
> are classified into read, write, etc. and logging for these classes
> can be individually enabled and disabled (either via pgaudit.log in
> postgresql.conf, or as a per-database or per-user setting).

This is something I've been mulling over for a couple of years (you can
see notes from the discussion at the 2011 hacker meeting on the wiki
about how we might change our logging system to allow for better
filtering).

> Planned future improvements include:
>
> 1. Additional logging facilities, including to a separate audit
> log file and to syslog, and potentially logging to a table
> (possibly via a bgworker process). Currently output is simply
> emitted to the server log via ereport().

Using the existing logging collector will almost certainly be a
contention point- we've seen that before. I've had thoughts about
an option to log to individual files from each backend (perhaps based
on that backend's position in the proc table) or directly from each
backend to a remote service (eg: rabbitMQ/AMQP or something).

Regarding background worker processes, a thought that's been kicked
around a bit is to actually change our existing logging collector to
be a background worker (or perhaps be able to have multiple?) which
is fed from a DSM queue and then logs to a file (or maybe files), or
a table or something else.

> 2. To implement per-object auditing configuration, it would be nice to use
> extensible reloptions (or an equivalent mechanism)

Yeah, that's another interesting challenge. This kind of auditing is
often about specific information (and therefore specific objects) and
it'd be ideal to have that set up and managed alongside the table
definition. Having the auditing done in core instead of through an
extension would make this easier to address though.

Thanks,

Stephen

On 05/02/2014 11:04 AM, Stephen Frost wrote:
> This is something I've been mulling over for a couple of years (you can
> see notes from the discussion at the 2011 hacker meeting on the wiki
> about how we might change our logging system to allow for better
> filtering).

Logging hooks. We really need some contrib/ modules which take
advantage of these.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

* Josh Berkus (josh(at)agliodbs(dot)com) wrote:
> Logging hooks. We really need some contrib/ modules which take
> advantage of these.

I'm aware and I really am not convinced that pushing all of this to
contrib modules using the hooks is the right approach- for one thing, it
certainly doesn't seem to me that we've actually gotten a lot of
traction from people to actually make use of them and keep them updated.
We've had many of those hooks for quite a while.

What 2Q has done here is great, but they also point out problems with
building this as a contrib module using the hooks. As we add more
capabilities and improve the overall PG system (new objects, etc), I'm
rather unconvinced that having to go, independently, update the contrib
modules to understand each new object is going to be a terribly workable
long-term solution.

Additionally, using triggers (either on the tables or the event
triggers), while good for many use-cases, doesn't completely answer the
auditing requirements (SELECT being the great example, but there are
others) and having to combine event triggers with various hooks just
doesn't strike me as a great design. (I don't intend to knock what 2Q
has done here at all- they're using a minimal-backend-hacking approach,
and under that constraint they've done exactly what makes sense).

Thanks,

Stephen

At 2014-05-02 14:04:27 -0400, sfrost(at)snowman(dot)net wrote:
>
> I'd really like to see us be able to, say, log to a table and have
> more fine-grained control over what is logged, without needing an
> extension.

There were several factors we considered in our work:

1. We did the minimum possible to produce something that gives us
demonstrably more than «log_statement=all» in 9.3/9.4/9.5.

2. We wanted to produce something that could be used *now*, i.e. with
9.3 and soon 9.4, to get wider feedback based on actual usage. I'm
hoping that by the time we make a submission for 9.5, we'll have a
clearer picture of what Postgres auditing should look like.

3. We steered clear of implementing different log targets. We know that
ereport() doesn't cut it, but decided that doing anything else would
be better after some feedback and wider discussion. Any suggestions
in this regard are very welcome.

(Stephen, I can see from your mail that you've already inferred at least
some of the above, so it's more a general statement of our approach than
a response to what you said.)

> > 2. pgaudit creates a log entry for each affected object […]
>
> Interesting- I'm a bit on the fence about this one. Perhaps you can
> elaborate on the use-case for this?

Who accessed public.x last month?

Answering that question would become much more difficult if one had to
account for every view that might refer to public.x. And did the view
refer to public.x before the schema change on the first Wednesday of
last month?

We don't have a "deparsed" representation of DML, so "select * from x"
is logged differently from "select * from other.x". Same with potential
complications like how exactly a join is written.

The way pgaudit does it, you can just "grep public.x" in your audit log
and be sure (modulo bugs, of course) you're seeing everything relevant.

> This kind of auditing is often about specific information (and
> therefore specific objects) and it'd be ideal to have that set
> up and managed alongside the table definition.

Yes, exactly.

-- Abhijit

At 2014-05-02 14:22:23 -0400, sfrost(at)snowman(dot)net wrote:
>
> I'm aware and I really am not convinced that pushing all of this to
> contrib modules using the hooks is the right approach- for one thing,
> it certainly doesn't seem to me that we've actually gotten a lot of
> traction from people to actually make use of them and keep them
> updated.

For what it's worth, I greatly appreciate *having* the hooks. Without
them, it would have been much more difficult to prototype pgaudit, and
it would have been impossible to do so in a way that could be used with
9.3/9.4.

As for whether auditing as a feature *should* be an extension, I do not
have a strong opinion yet. If a consensus formed around a better design
in-core, I certainly wouldn't object.

> I'm rather unconvinced that having to go, independently, update the
> contrib modules to understand each new object is going to be a
> terribly workable long-term solution.

(I am not expressing any opinion at this time on this larger question.)

> having to combine event triggers with various hooks just doesn't
> strike me as a great design.

Suggestions are welcome, but I have to say that I'm not a big fan of
reinventing what event trigger give us in the way of deparsing either.

-- Abhijit

Abhijit,

* Abhijit Menon-Sen (ams(at)2ndquadrant(dot)com) wrote:
> 3. We steered clear of implementing different log targets. We know that
> ereport() doesn't cut it, but decided that doing anything else would
> be better after some feedback and wider discussion. Any suggestions
> in this regard are very welcome.

I'm not anxious to try and replace ereport() either, but I don't see
that as necessary to have multiple log targets (we already have that,
after all..). The design that I had discussed w/ Magnus and at the
hacker's meeting in 2011 was around the notion of 'tags' and a
structured interface to the logging collector. That fits in nicely with
the idea of using a DSM queue, I'd think.

> Who accessed public.x last month?
>
> Answering that question would become much more difficult if one had to
> account for every view that might refer to public.x. And did the view
> refer to public.x before the schema change on the first Wednesday of
> last month?

This also addresses things like anonymous DO blocks and functions
then..? With enough information to be useful for forensics?

> We don't have a "deparsed" representation of DML, so "select * from x"
> is logged differently from "select * from other.x". Same with potential
> complications like how exactly a join is written.

This seems like an independently useful thing (would be nice to have in
our logs and in pg_stat_statements, imv..).

> > This kind of auditing is often about specific information (and
> > therefore specific objects) and it'd be ideal to have that set
> > up and managed alongside the table definition.
>
> Yes, exactly.

We'd need to also consider permissions and how these are managed.
Presumably, the 'owner' of a relation would be able to define and modify
its audit parameters, but it would be useful to have that capability
independently grant'able and also be sure that any changes made to the
auditing are clearly logged.

This gets into a much larger area of discussion around what can be
granted and what must be owner-only or superuser-only.

Thanks,

Stephen

* Abhijit Menon-Sen (ams(at)2ndquadrant(dot)com) wrote:
> At 2014-05-02 14:22:23 -0400, sfrost(at)snowman(dot)net wrote:
> > I'm aware and I really am not convinced that pushing all of this to
> > contrib modules using the hooks is the right approach- for one thing,
> > it certainly doesn't seem to me that we've actually gotten a lot of
> > traction from people to actually make use of them and keep them
> > updated.
>
> For what it's worth, I greatly appreciate *having* the hooks. Without
> them, it would have been much more difficult to prototype pgaudit, and
> it would have been impossible to do so in a way that could be used with
> 9.3/9.4.

I'm perfectly fine w/ having the hooks and they're great for exactly the
reasons you point out- it's at least *possible* to add some of this
without having to custom compile the backend. That doesn't mean it's
what we should hang our hat on as the 'one true solution'.

> > having to combine event triggers with various hooks just doesn't
> > strike me as a great design.
>
> Suggestions are welcome, but I have to say that I'm not a big fan of
> reinventing what event trigger give us in the way of deparsing either.

No, I wouldn't want us to reinvent or duplicate code either.

Thanks,

Stephen

At 2014-05-04 08:52:42 -0400, sfrost(at)snowman(dot)net wrote:
>
> This also addresses things like anonymous DO blocks and functions
> then..? With enough information to be useful for forensics?

For DML, it addresses anything that goes through InitPlan (which, via
ExecCheckRTPerms, calls the ExecutorCheckPerms_hook). Yes, that does
include table accesses within DO blocks:

LOG: [AUDIT],2014-05-04 19:15:06.130771+05:30,ams,ams,ams,WRITE,DELETE,TABLE,public.a,do $$begin delete from a; end;$$;

…and functions:

LOG: [AUDIT],2014-05-04 19:16:20.336499+05:30,ams,ams,ams,DEFINITION,CREATE FUNCTION,,,create function x(int) returns text as $$select b from a where a=$1;$$ language sql;
LOG: [AUDIT],2014-05-04 19:16:48.112725+05:30,ams,ams,ams,FUNCTION,EXECUTE,FUNCTION,public.x,select * from x(3);
LOG: [AUDIT],2014-05-04 19:16:48.112922+05:30,ams,ams,ams,READ,SELECT,TABLE,public.a,select * from x(3);

> We'd need to also consider permissions and how these are managed.

Yes. For pgaudit, we kept it simple and made pgaudit.log
superuser-only[1].

I haven't given much thought to this area, because I didn't know what
mechanism to use to set per-object auditing parameters. For a contrib
module, extensible reloptions would have been convenient. But in-core
auditing support could use a proper reloption, I suppose. It's a pity
that extensions can't add reloptions.

Personally, I don't think it's important to support GRANT-ing the
ability to set audit parameters. I think it would be reasonable even to
say that only the superuser could do it (but I can imagine people being
unhappy if the owner couldn't[2]).

Definitely lots to discuss.

-- Abhijit

1. I wish it were possible to prevent even the superuser from disabling
audit logging once it's enabled, so that if someone gained superuser
access without authorisation, their actions would still be logged.
But I don't think there's any way to do this.

2. On the other hand, I can also imagine a superuser being justifiably
annoyed if she were to carefully configure auditing, and random users
then enabled audit-everything for their newly-created tables and
filled the audit table with junk.

* Abhijit Menon-Sen (ams(at)2ndquadrant(dot)com) wrote:
> At 2014-05-04 08:52:42 -0400, sfrost(at)snowman(dot)net wrote:
> > This also addresses things like anonymous DO blocks and functions
> > then..? With enough information to be useful for forensics?
>
> For DML, it addresses anything that goes through InitPlan (which, via
> ExecCheckRTPerms, calls the ExecutorCheckPerms_hook). Yes, that does
> include table accesses within DO blocks:

Right, I figured, but wanted to clarify the usefullness of this goes
beyond just views.

> > We'd need to also consider permissions and how these are managed.
>
> Yes. For pgaudit, we kept it simple and made pgaudit.log
> superuser-only[1].

Yeah, for an initial version that makes sense, but I'm sure we'll need
more.

> I haven't given much thought to this area, because I didn't know what
> mechanism to use to set per-object auditing parameters. For a contrib
> module, extensible reloptions would have been convenient. But in-core
> auditing support could use a proper reloption, I suppose. It's a pity
> that extensions can't add reloptions.

Another reloption is one option, or an extension on the ACL system (for
that piece of it), or we could make a new catalog for it (ala
pg_seclabel), or perhaps add it on to one (pg_seclabel but rename it to
pg_security..?).

> Personally, I don't think it's important to support GRANT-ing the
> ability to set audit parameters. I think it would be reasonable even to
> say that only the superuser could do it (but I can imagine people being
> unhappy if the owner couldn't[2]).

I disagree. Perhaps it could be a role-level permission instead of one
which is per-table, but I don't think this should be superuser-only.

> 1. I wish it were possible to prevent even the superuser from disabling
> audit logging once it's enabled, so that if someone gained superuser
> access without authorisation, their actions would still be logged.
> But I don't think there's any way to do this.

Their actions should be logged up until they disable auditing and
hopefully those logs would be sent somewhere that they're unable to
destroy (eg: syslog). Of course, we make that difficult by not
supporting log targets based on criteria (logging EVERYTHING to syslog
would suck).

I don't see a way to fix this, except to minimize the amount of things
requiring superuser to reduce the chances of it being compromised, which
is something I've been hoping to see happen for a long time.

> 2. On the other hand, I can also imagine a superuser being justifiably
> annoyed if she were to carefully configure auditing, and random users
> then enabled audit-everything for their newly-created tables and
> filled the audit table with junk.

While I understand your concern, I'm not sure it's really what we should
be designing for. One of the oft-commented on distinctions is to have
roles which are specifically auditors- and they have very limited access
beyond that. To that point, a role-level attribute for 'can modify
auditing' seems key, but we wouldn't want such a role to also be an
owner of every relation they need to be able to modify auditing for, yet
it would be valuable to constrain the auditor to only being able to
modify auditing on certain sets of tables.

That seems to boil down to a GRANT'able option, since that gives the
per-table granularity. That would also make the role-level attribute
unnecessary, which is appealing. The downside of this is that the owner
ends up being untimately in control here- but then, the owner could
presumably drop and recreate the object anyway, and where would we be
then? Perhaps having it be up to the owner isn't such a bad approach.

The implementation-level concern here comes from the way we actually
store and manage those permissions. We're awful short of bits when we
start thinking about all the things we might want to make independently
grant'able.

Thanks,

Stephen

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> * Abhijit Menon-Sen (ams(at)2ndquadrant(dot)com) wrote:
>> 1. I wish it were possible to prevent even the superuser from disabling
>> audit logging once it's enabled, so that if someone gained superuser
>> access without authorisation, their actions would still be logged.
>> But I don't think there's any way to do this.

> Their actions should be logged up until they disable auditing and
> hopefully those logs would be sent somewhere that they're unable to
> destroy (eg: syslog). Of course, we make that difficult by not
> supporting log targets based on criteria (logging EVERYTHING to syslog
> would suck).

> I don't see a way to fix this, except to minimize the amount of things
> requiring superuser to reduce the chances of it being compromised, which
> is something I've been hoping to see happen for a long time.

Prohibiting actions to the superuser is a fundamentally flawed concept.
If you do that, you just end up having to invent a new "more super"
kind of superuser who *can* do whatever it is that needs to be done.

regards, tom lane

At 2014-05-04 11:03:56 -0400, sfrost(at)snowman(dot)net wrote:
>
> Another reloption is one option, or an extension on the ACL system
> (for that piece of it), or we could make a new catalog for it (ala
> pg_seclabel), or perhaps add it on to one (pg_seclabel but rename
> it to pg_security..?).

I'll look into those possibilities, thanks.

> Perhaps it could be a role-level permission instead of one which is
> per-table, but I don't think this should be superuser-only.

I like the idea of a role-level permission, or a (db,role)-level
permission (i.e. "role x is auditor for database y"), but I don't
feel I know enough about real-world auditing requirements to make
an informed decision here.

Ian did some research into how auditing is handled in other systems.
He's on vacation right now, and I'm not sure how much detail his report
has on this particular subject, but I'll have a look and try to present
a summary soon.

-- Abhijit

On May 4, 2014, at 10:12 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
>> * Abhijit Menon-Sen (ams(at)2ndquadrant(dot)com) wrote:
>>> 1. I wish it were possible to prevent even the superuser from disabling
>>> audit logging once it's enabled, so that if someone gained superuser
>>> access without authorisation, their actions would still be logged.
>>> But I don't think there's any way to do this.
>
>> Their actions should be logged up until they disable auditing and
>> hopefully those logs would be sent somewhere that they're unable to
>> destroy (eg: syslog). Of course, we make that difficult by not
>> supporting log targets based on criteria (logging EVERYTHING to syslog
>> would suck).
>
>> I don't see a way to fix this, except to minimize the amount of things
>> requiring superuser to reduce the chances of it being compromised, which
>> is something I've been hoping to see happen for a long time.
>
> Prohibiting actions to the superuser is a fundamentally flawed concept.
> If you do that, you just end up having to invent a new "more super"
> kind of superuser who *can* do whatever it is that needs to be done.

In getting approval for FDA validated systems, IIRC, they wanted to see the audit change permissions completely independent of the technical roles and responsibilities. Meaning that superuser or owner roles could not change the audit requirements once established and the audit role could not change any data or data definitions except add, change or remove auditing rules. Everything the auditor role did was logged, no exceptions.

If an owner or superuser dropped a table the auditors were completely fine with a log entry that the table/column was dropped or created by someone. The audit reporting system (external to the database) had notifications for these types of events. For example, by procedure these changes should have been done in conjunction with the auditors and the initial audit requirements should already have been improved by the auditors when the column/table was added back. Dropping a table/column without getting approval ahead of time was a procedure violation that could result in termination. Of course, there were a lot more details.

By monitoring creation/delete DDL events along with non changeable (by technical staff) audit rules the auditors were happy that they could manage the audit conformance.

And yes, the audit logs had to be written in a way they could not be easily tampered with. At the time we used an approved append only, read only hardware file/reporting system.

In considering how this might apply to PostgreSQL, it seems that once formal auditing is turned on, basic non-changeable level of audit reporting should be in place (i.e. log all create/drop/rename tables/columns/roles and log all superuser/audit role actions) and this basic audit reporting cannot be turned off or have the destination changed without considerable headache (something like init'ing the database?). Then data monitoring auditing rules can be added/changed/removed as necessary within the authorization framework. Formal auditing might also require other functionality like checksums.

Until these or similar requirements (for formal auditing) are in core, it makes no sense (to me) to not allow the superuser to manage auditing because any conformance requirements have to be procedure based, not system based. People often forget that procedure/people based audit conformance worked just fine before computers existed.

Neil

Neil,

Thanks for sharing- sounds very similar to what I've heard also. Your
input and experience with this is very much sought and appreciated-
please continue to help us understand, so we're able to provide
something concrete and useful. Further comments inline.

* Neil Tiffin (neilt(at)neiltiffin(dot)com) wrote:
> In considering how this might apply to PostgreSQL, it seems that once formal auditing is turned on, basic non-changeable level of audit reporting should be in place (i.e. log all create/drop/rename tables/columns/roles and log all superuser/audit role actions) and this basic audit reporting cannot be turned off or have the destination changed without considerable headache (something like init'ing the database?). Then data monitoring auditing rules can be added/changed/removed as necessary within the authorization framework. Formal auditing might also require other functionality like checksums.

Any system where there exists a role similar to 'superuser' in the PG
sense (a user who is equivilant to the Unix UID under which the rest of
the system is run) would be hard-pressed to provide a solution to this
issue. With SELinux it may be possible and I'd love to see an example
from someone who feels they've accomplished it. That said, if we can
reduce the need for a 'superuser' role sufficiently by having the
auditing able to be managed independently, then we may have reached the
level of "considerable headache".

As many have pointed out previously, there is a certain amount of risk
associated with running without *any* superuser role in the system
(though it's certainly possible to do so), as it becomes much more
difficult to do certain kinds of analysis and forensics associated with
trying to recover a corrupt system. Still, that risk may very well be
acceptable in some environments. I'd certainly like to see us get to a
point where a superuser role isn't absolutely required once the system
is up and running.

> Until these or similar requirements (for formal auditing) are in core, it makes no sense (to me) to not allow the superuser to manage auditing because any conformance requirements have to be procedure based, not system based. People often forget that procedure/people based audit conformance worked just fine before computers existed.

I do understand this and I expect we will always allow the roles which
are 'superuser' to modify these procedures, but we'll get to a point
where such a role doesn't have to exist (or it's a considerable headache
to get one into place) and that'll get us to the point which is required
to check the "formal auditing" box for the organizations which are
interested and willing to accept those trade-offs.

Thanks,

Stephen

On May 4, 2014, at 3:17 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:

> Neil,
>
> Thanks for sharing- sounds very similar to what I've heard also. Your
> input and experience with this is very much sought and appreciated-
> please continue to help us understand, so we're able to provide
> something concrete and useful. Further comments inline.
>
> * Neil Tiffin (neilt(at)neiltiffin(dot)com) wrote:
>> In considering how this might apply to PostgreSQL, it seems that once formal auditing is turned on, basic non-changeable level of audit reporting should be in place (i.e. log all create/drop/rename tables/columns/roles and log all superuser/audit role actions) and this basic audit reporting cannot be turned off or have the destination changed without considerable headache (something like init'ing the database?). Then data monitoring auditing rules can be added/changed/removed as necessary within the authorization framework. Formal auditing might also require other functionality like checksums.
>
> Any system where there exists a role similar to 'superuser' in the PG
> sense (a user who is equivilant to the Unix UID under which the rest of
> the system is run) would be hard-pressed to provide a solution to this
> issue.

Not sure I understand which issue you are referring to. If you are referring to 'cannot be turned off', I would think a reasonable first pass would be to handle it similar to '--data-checksums' in 'initdb'. For example, "This option can only be set during initialization, and cannot be changed later. If set, basic auditing is on for all objects, in all databases."

> With SELinux it may be possible and I'd love to see an example
> from someone who feels they've accomplished it. That said, if we can
> reduce the need for a 'superuser' role sufficiently by having the
> auditing able to be managed independently, then we may have reached the
> level of "considerable headache".
>
> As many have pointed out previously, there is a certain amount of risk
> associated with running without *any* superuser role in the system

If all of the superuser's actions are logged and it's not possible to turn off the logging (without considerable headache) then it may not matter what the superuser can do. If the superuser makes changes and they are logged then the auditors have sufficient information to see if the correct procedures were followed. Validated systems are based on tracking, not necessarily prohibiting. Select individuals that should be able to be trusted (which should apply to superusers) should be able to perform the actions necessary to support the organization.

> (though it's certainly possible to do so), as it becomes much more
> difficult to do certain kinds of analysis and forensics associated with
> trying to recover a corrupt system. Still, that risk may very well be
> acceptable in some environments. I'd certainly like to see us get to a
> point where a superuser role isn't absolutely required once the system
> is up and running.
>
>> Until these or similar requirements (for formal auditing) are in core, it makes no sense (to me) to not allow the superuser to manage auditing because any conformance requirements have to be procedure based, not system based. People often forget that procedure/people based audit conformance worked just fine before computers existed.
>
> I do understand this and I expect we will always allow the roles which
> are 'superuser' to modify these procedures, but we'll get to a point
> where such a role doesn't have to exist (or it's a considerable headache
> to get one into place) and that'll get us to the point which is required
> to check the "formal auditing" box for the organizations which are
> interested and willing to accept those trade-offs.
>

* Neil Tiffin (neilt(at)neiltiffin(dot)com) wrote:
> On May 4, 2014, at 3:17 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > Any system where there exists a role similar to 'superuser' in the PG
> > sense (a user who is equivilant to the Unix UID under which the rest of
> > the system is run) would be hard-pressed to provide a solution to this
> > issue.
>
> Not sure I understand which issue you are referring to. If you are referring to 'cannot be turned off', I would think a reasonable first pass would be to handle it similar to '--data-checksums' in 'initdb'. For example, "This option can only be set during initialization, and cannot be changed later. If set, basic auditing is on for all objects, in all databases."

Well, except that a superuser *could* effectively turn off checksums by
changing the the control file and doing a restart (perhaps modulo some
other hacking; I've not tried). That kind of trivial 'hole' isn't
acceptable from a security standpoint though and given that we couldn't
prevent a superuser from doing an LD_PRELOAD and overriding any system
call we make from the backend, it's kind of hard to see how we could
plug such a hole.

> > With SELinux it may be possible and I'd love to see an example
> > from someone who feels they've accomplished it. That said, if we can
> > reduce the need for a 'superuser' role sufficiently by having the
> > auditing able to be managed independently, then we may have reached the
> > level of "considerable headache".
> >
> > As many have pointed out previously, there is a certain amount of risk
> > associated with running without *any* superuser role in the system
>
> If all of the superuser's actions are logged and it's not possible to turn off the logging (without considerable headache) then it may not matter what the superuser can do. If the superuser makes changes and they are logged then the auditors have sufficient information to see if the correct procedures were followed. Validated systems are based on tracking, not necessarily prohibiting. Select individuals that should be able to be trusted (which should apply to superusers) should be able to perform the actions necessary to support the organization.

Fair enough- the question is just a matter of what exactly that level of
"headache" is.

Thanks!

Stephen

On May 4, 2014, at 5:27 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:

> * Neil Tiffin (neilt(at)neiltiffin(dot)com) wrote:
>> On May 4, 2014, at 3:17 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>>> Any system where there exists a role similar to 'superuser' in the PG
>>> sense (a user who is equivilant to the Unix UID under which the rest of
>>> the system is run) would be hard-pressed to provide a solution to this
>>> issue.
>>
>> Not sure I understand which issue you are referring to. If you are referring to 'cannot be turned off', I would think a reasonable first pass would be to handle it similar to '--data-checksums' in 'initdb'. For example, "This option can only be set during initialization, and cannot be changed later. If set, basic auditing is on for all objects, in all databases."
>
> Well, except that a superuser *could* effectively turn off checksums by
> changing the the control file and doing a restart (perhaps modulo some
> other hacking; I've not tried). That kind of trivial 'hole' isn't
> acceptable from a security standpoint though and given that we couldn't
> prevent a superuser from doing an LD_PRELOAD and overriding any system
> call we make from the backend, it's kind of hard to see how we could
> plug such a hole.
>

Ah, I thought it would be more difficult than that for checksums, but PostgreSQL does not have to prevent hacking in my experience, that is the responsibility of other systems and procedures. If the core code was such that once on, formal logging could not be turned off with any changes to config files, settings, or SQL then in my experience that would suffice.

>>> With SELinux it may be possible and I'd love to see an example
>>> from someone who feels they've accomplished it. That said, if we can
>>> reduce the need for a 'superuser' role sufficiently by having the
>>> auditing able to be managed independently, then we may have reached the
>>> level of "considerable headache".
>>>
>>> As many have pointed out previously, there is a certain amount of risk
>>> associated with running without *any* superuser role in the system
>>
>> If all of the superuser's actions are logged and it's not possible to turn off the logging (without considerable headache) then it may not matter what the superuser can do. If the superuser makes changes and they are logged then the auditors have sufficient information to see if the correct procedures were followed. Validated systems are based on tracking, not necessarily prohibiting. Select individuals that should be able to be trusted (which should apply to superusers) should be able to perform the actions necessary to support the organization.
>
> Fair enough- the question is just a matter of what exactly that level of
> "headache" is.

* Neil Tiffin (neilt(at)neiltiffin(dot)com) wrote:
> On May 4, 2014, at 5:27 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > * Neil Tiffin (neilt(at)neiltiffin(dot)com) wrote:
> > Well, except that a superuser *could* effectively turn off checksums by
> > changing the the control file and doing a restart (perhaps modulo some
> > other hacking; I've not tried). That kind of trivial 'hole' isn't
> > acceptable from a security standpoint though and given that we couldn't
> > prevent a superuser from doing an LD_PRELOAD and overriding any system
> > call we make from the backend, it's kind of hard to see how we could
> > plug such a hole.
> >
>
> Ah, I thought it would be more difficult than that for checksums, but PostgreSQL does not have to prevent hacking in my experience, that is the responsibility of other systems and procedures. If the core code was such that once on, formal logging could not be turned off with any changes to config files, settings, or SQL then in my experience that would suffice.

We could set it up similar to how security labels work, where the
config file (which could be owned by 'root' and therefore unable to be
changed by a superuser) has an auditing setting and changing it requires
a restart (meaning that the config file would have to be modified to
change it, and the database restarted). However, it might be possible
for a superuser to configure and start an independent postmaster with a
different configuration that points to the same database (or a copy of
it).

That's for a system-wide auditing setting, but if we actually want the
auditing to only be on certain database objects, it gets worse. We
need to track what objects need the auditing and we'd do that using the
catalog, which a superuser can modify. Security labels have
more-or-less the same issue, of course.

This is why we don't try to protect against superusers (and why I'm
hopeful that we can reduce the need for a superuser role to exist).
Again, we have to consider that a superuser essentially has a full shell
on the DB server as the user that the database runs under.

Thanks,

Stephen

On 5/2/14, 2:22 PM, Stephen Frost wrote:
> I'm aware and I really am not convinced that pushing all of this to
> contrib modules using the hooks is the right approach- for one thing, it
> certainly doesn't seem to me that we've actually gotten a lot of
> traction from people to actually make use of them and keep them updated.
> We've had many of those hooks for quite a while.

What is there to update? The stuff works and doesn't necessarily need
any changes.

* Peter Eisentraut (peter_e(at)gmx(dot)net) wrote:
> On 5/2/14, 2:22 PM, Stephen Frost wrote:
> > I'm aware and I really am not convinced that pushing all of this to
> > contrib modules using the hooks is the right approach- for one thing, it
> > certainly doesn't seem to me that we've actually gotten a lot of
> > traction from people to actually make use of them and keep them updated.
> > We've had many of those hooks for quite a while.
>
> What is there to update? The stuff works and doesn't necessarily need
> any changes.

I'm referring to auditing/logging systems built on top of those, not the
hooks themselves..

Thanks,

Stephen

On Sun, May 4, 2014 at 11:12:57AM -0400, Tom Lane wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > * Abhijit Menon-Sen (ams(at)2ndquadrant(dot)com) wrote:
> >> 1. I wish it were possible to prevent even the superuser from disabling
> >> audit logging once it's enabled, so that if someone gained superuser
> >> access without authorisation, their actions would still be logged.
> >> But I don't think there's any way to do this.
>
> > Their actions should be logged up until they disable auditing and
> > hopefully those logs would be sent somewhere that they're unable to
> > destroy (eg: syslog). Of course, we make that difficult by not
> > supporting log targets based on criteria (logging EVERYTHING to syslog
> > would suck).
>
> > I don't see a way to fix this, except to minimize the amount of things
> > requiring superuser to reduce the chances of it being compromised, which
> > is something I've been hoping to see happen for a long time.
>
> Prohibiting actions to the superuser is a fundamentally flawed concept.
> If you do that, you just end up having to invent a new "more super"
> kind of superuser who *can* do whatever it is that needs to be done.

We did create a "replication" role that could only read data, right? Is
that similar?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

* Bruce Momjian (bruce(at)momjian(dot)us) wrote:
> On Sun, May 4, 2014 at 11:12:57AM -0400, Tom Lane wrote:
> > Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > > * Abhijit Menon-Sen (ams(at)2ndquadrant(dot)com) wrote:
> > >> 1. I wish it were possible to prevent even the superuser from disabling
> > >> audit logging once it's enabled, so that if someone gained superuser
> > >> access without authorisation, their actions would still be logged.
> > >> But I don't think there's any way to do this.
> >
> > > Their actions should be logged up until they disable auditing and
> > > hopefully those logs would be sent somewhere that they're unable to
> > > destroy (eg: syslog). Of course, we make that difficult by not
> > > supporting log targets based on criteria (logging EVERYTHING to syslog
> > > would suck).
> >
> > > I don't see a way to fix this, except to minimize the amount of things
> > > requiring superuser to reduce the chances of it being compromised, which
> > > is something I've been hoping to see happen for a long time.
> >
> > Prohibiting actions to the superuser is a fundamentally flawed concept.
> > If you do that, you just end up having to invent a new "more super"
> > kind of superuser who *can* do whatever it is that needs to be done.
>
> We did create a "replication" role that could only read data, right? Is
> that similar?

Not sure which of the above discussions you're suggesting it's 'similar'
to, but a 'read-only' role (which is specifically *not* a superuser)
would definitely help reduce the number of things which need to run as
an actual 'superuser' (eg: pg_dump).

The above discussion was around having auditing which the superuser
couldn't change, which isn't really possible as a superuser can change
the code that's executing (modulo things like SELinux changing the
game, but that's outside PG to some extent).

Thanks,

Stephen

On Fri, May 2, 2014 at 3:19 PM, Ian Barwick <ian(at)2ndquadrant(dot)com> wrote:
> Hi
>
> Here is an initial version of an auditing extension for Postgres to
> generate log output suitable for compiling a comprehensive audit trail
> of database operations.

You added this into CF, but its patch has not been posted yet. Are you planning
to make a patch?

> Planned future improvements include:
>
> 1. Additional logging facilities, including to a separate audit
> log file and to syslog, and potentially logging to a table
> (possibly via a bgworker process). Currently output is simply
> emitted to the server log via ereport().
>
> 2. To implement per-object auditing configuration, it would be nice to use
> extensible reloptions (or an equivalent mechanism)

Is it possible to implement these outside PostgreSQL by using hooks?
If not, it might be better to implement audit feature in core from the
beginning.

Regards,

--
Fujii Masao

(I'm replying as co-author of pgaudit.)

At 2014-06-23 19:15:39 +0900, masao(dot)fujii(at)gmail(dot)com wrote:
>
> You added this into CF, but its patch has not been posted yet. Are you
> planning to make a patch?

It's a self-contained contrib module. I thought Ian had posted a
tarball, but it looks like he forgot to attach it (or decided to
provide only a Github link). I've attached a tarball here for
your reference.

> > Planned future improvements include:
> >
> > 1. Additional logging facilities, including to a separate audit
> > log file and to syslog, and potentially logging to a table
> > (possibly via a bgworker process). Currently output is simply
> > emitted to the server log via ereport().
> >
> > 2. To implement per-object auditing configuration, it would be nice
> > to use extensible reloptions (or an equivalent mechanism)
>
> Is it possible to implement these outside PostgreSQL by using hooks?

There are some unresolved questions with #2 because the extensible
reloptions patch seems to have lost favour, but I'm pretty sure we
could figure out some alternative.

> If not, it might be better to implement audit feature in core from the
> beginning.

Sure, we're open to that possibility. Do you have any ideas about what
an in-core implementation should do/look like?

-- Abhijit

On Mon, Jun 23, 2014 at 7:51 PM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> wrote:
> (I'm replying as co-author of pgaudit.)
>
> At 2014-06-23 19:15:39 +0900, masao(dot)fujii(at)gmail(dot)com wrote:
>>
>> You added this into CF, but its patch has not been posted yet. Are you
>> planning to make a patch?
>
> It's a self-contained contrib module. I thought Ian had posted a
> tarball, but it looks like he forgot to attach it (or decided to
> provide only a Github link). I've attached a tarball here for
> your reference.
>
>> > Planned future improvements include:
>> >
>> > 1. Additional logging facilities, including to a separate audit
>> > log file and to syslog, and potentially logging to a table
>> > (possibly via a bgworker process). Currently output is simply
>> > emitted to the server log via ereport().
>> >
>> > 2. To implement per-object auditing configuration, it would be nice
>> > to use extensible reloptions (or an equivalent mechanism)
>>
>> Is it possible to implement these outside PostgreSQL by using hooks?
>
> There are some unresolved questions with #2 because the extensible
> reloptions patch seems to have lost favour, but I'm pretty sure we
> could figure out some alternative.
>
>> If not, it might be better to implement audit feature in core from the
>> beginning.
>
> Sure, we're open to that possibility. Do you have any ideas about what
> an in-core implementation should do/look like?

I don't have good idea about that. But maybe we can merge pgaudit.log
into log_statement for more flexible settings of what to log.

BTW I found that pgaudit doesn't log bind parameters when prepared
statements are executed. I'm feeling this behavior is not good for audit
purpose. Thought?

Also I got only the following log message when I executed "PREPARE hoge AS
INSERT INTO mytbl VALUES($1)" and "EXECUTE hoge(1)". This means that
obviously we cannot track the statements via PREPARED command...

LOG: [AUDIT],2014-06-23
21:14:53.667059+09,postgres,postgres,postgres,WRITE,INSERT,TABLE,postgres.mytbl,execute
hoge(1);

Regards,

--
Fujii Masao

* Fujii Masao (masao(dot)fujii(at)gmail(dot)com) wrote:
> On Mon, Jun 23, 2014 at 7:51 PM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> wrote:
> > At 2014-06-23 19:15:39 +0900, masao(dot)fujii(at)gmail(dot)com wrote:
> >> You added this into CF, but its patch has not been posted yet. Are you
> >> planning to make a patch?
> >
> > It's a self-contained contrib module. I thought Ian had posted a
> > tarball, but it looks like he forgot to attach it (or decided to
> > provide only a Github link). I've attached a tarball here for
> > your reference.

I'm not a huge fan of adding this as a contrib module unless we can be
quite sure that there's a path forward from here to a rework of the
logging in core which would actually support the features pg_audit is
adding, without a lot of pain and upgrade issues. Those issues have
kept other contrib modules from being added to core.

Splitting up contrib into other pieces, one of which is a 'features'
area, might address that but we'd really need a way to have those pieces
be able to include/add catalog tables, at least..

> >> If not, it might be better to implement audit feature in core from the
> >> beginning.
> >
> > Sure, we're open to that possibility. Do you have any ideas about what
> > an in-core implementation should do/look like?
>
> I don't have good idea about that. But maybe we can merge pgaudit.log
> into log_statement for more flexible settings of what to log.

I'd expect a catalog table or perhaps changes to pg_class (maybe other
things also..) to define what gets logged.. I'd also like to see the
ability to log based on the connecting user, and we need to log under
what privileges a command is executing, and, really, a whole host of
other things..

Thanks,

Stephen

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> I'd expect a catalog table or perhaps changes to pg_class (maybe other
> things also..) to define what gets logged.

How exactly will that work for log messages generated in contexts where
we do not have working catalog access? (postmaster, crash recovery,
or pretty much anywhere where we're not in a valid transaction.)

This strikes me as much like the periodic suggestions we hear to get
rid of the GUC infrastructure in favor of keeping all those settings
in a table. It doesn't work because too much of that info is needed
below the level of working table access.

regards, tom lane

Tom,

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > I'd expect a catalog table or perhaps changes to pg_class (maybe other
> > things also..) to define what gets logged.
>
> How exactly will that work for log messages generated in contexts where
> we do not have working catalog access? (postmaster, crash recovery,
> or pretty much anywhere where we're not in a valid transaction.)

Certainly a great question and we may have to address it by not
supporting changes immediately (in other words, cache things at backend
start, or only at specific times), or by reviewing what logging we do at
those times and work out which of those can be controllerd through the
catalog and which can't. The logging which can't be managed through the
catalog would have to be managed through GUCs or in another way.

There's a difference between being able to have finer grained control
over certain log messages and having every single ereport() query the
catalog. I'm not advocating the latter.

> This strikes me as much like the periodic suggestions we hear to get
> rid of the GUC infrastructure in favor of keeping all those settings
> in a table. It doesn't work because too much of that info is needed
> below the level of working table access.

I'm not suggesting this.

Thanks,

Stephen

At 2014-06-23 08:50:33 -0400, sfrost(at)snowman(dot)net wrote:
>
> I'm not a huge fan of adding this as a contrib module

I added it to the CF because we're interested in auditing functionality
for 9.5, and as far as I can tell, there's nothing better available. At
the moment, the contrib module has the advantage that it exists :-) and
works with 9.[34] (and could be made to work with 9.2, though I didn't
bother for the initial submission).

> unless we can be
> quite sure that there's a path forward from here to a rework of the
> logging in core which would actually support the features pg_audit is
> adding, without a lot of pain and upgrade issues.

What sort of pain and upgrade issues did you have in mind?

> I'd expect a catalog table or perhaps changes to pg_class (maybe other
> things also..) to define what gets logged..

Please explain?

(I wish extensions were able to add reloptions. That would have made it
relatively easy for us to implement per-object audit logging.)

> I'd also like to see the ability to log based on the connecting user,
> and we need to log under what privileges a command is executing

I imagine it's not useful to point out that you can do the former with
pgaudit (using ALTER ROLE … SET), and that we log the effective userid
for the latter (though maybe you had something more in mind)…

> and, really, a whole host of other things..

…but there's not a whole lot I can do with that, either.

Is the minimal set of auditing features that we would need in-core very
different from what pgaudit already has?

-- Abhijit

* Abhijit Menon-Sen (ams(at)2ndquadrant(dot)com) wrote:
> At 2014-06-23 08:50:33 -0400, sfrost(at)snowman(dot)net wrote:
> > I'm not a huge fan of adding this as a contrib module
>
> I added it to the CF because we're interested in auditing functionality
> for 9.5, and as far as I can tell, there's nothing better available. At
> the moment, the contrib module has the advantage that it exists :-) and
> works with 9.[34] (and could be made to work with 9.2, though I didn't
> bother for the initial submission).

Sure, I understand this.

> > unless we can be
> > quite sure that there's a path forward from here to a rework of the
> > logging in core which would actually support the features pg_audit is
> > adding, without a lot of pain and upgrade issues.
>
> What sort of pain and upgrade issues did you have in mind?

I admit to having not looked in depth at it but my immediate concern
would be any objects which it creates in the database and worry about
the exact issues that hstore has. I also wouldn't want this to become
an excuse to not improve our current logging infrastructure, which is
how we got to the place we are wrt partitioning, imv.

> > I'd expect a catalog table or perhaps changes to pg_class (maybe other
> > things also..) to define what gets logged..
>
> Please explain?

...

> (I wish extensions were able to add reloptions. That would have made it
> relatively easy for us to implement per-object audit logging.)

Well, this, specifically. We can't control what gets audited through
the catalog and have to manage that outside of PG, right? Or are there
tables which are part of pg_audit to define this that then would have to
be addressed in an upgrade scenario..? As I recall from looking before,
it's the former.

> > I'd also like to see the ability to log based on the connecting user,
> > and we need to log under what privileges a command is executing
>
> I imagine it's not useful to point out that you can do the former with
> pgaudit (using ALTER ROLE … SET), and that we log the effective userid
> for the latter (though maybe you had something more in mind)…

Are both the connected user and the current role that the command is
running under logged? That's what I was getting at. As for ALTER ROLE-
if that can be done then that's an example of a potential upgrade
issue..

> > and, really, a whole host of other things..
>
> …but there's not a whole lot I can do with that, either.

It was more a comment on the goal of improving the logging
infrastructure in PG than a knock against pgaudit.

> Is the minimal set of auditing features that we would need in-core very
> different from what pgaudit already has?

It's funny because, in a way, I see this as having more-or-less the same
issues that the RLS patch has- more control is needed through the
catalog, through the grammar, and generally needs to be more integrated.
Perhaps pgaudit as a contrib module is able to avoid having everything
at the initial implementation which an in-core capability would have to
have, but for my 2c, I'd much rather have that in-core capability and I
worry that adding pgaudit as an external feature now would end up
preventing us from moving forward in this area for years to come..

That's not a particularly fair argument, but it's my current feeling on
the subject.

Thanks,

Stephen

At 2014-06-23 16:51:55 -0400, sfrost(at)snowman(dot)net wrote:
>
> We can't control what gets audited through the catalog and have to
> manage that outside of PG, right?

Right. I understand now.

> Are both the connected user and the current role that the command is
> running under logged?

Yes, they are. -------------------------------------+----+
| |
v v
LOG: [AUDIT],2014-04-30 17:19:54.811244+09,auditdb,ianb,ianb,ADMIN,SET,,,set role ams;
LOG: [AUDIT],2014-04-30 17:19:57.039979+09,auditdb,ianb,ams,WRITE,INSERT,VIEW,public.v_x,INSERT INTO v_x VALUES(1,2);

> I'd much rather have that in-core capability and I worry that adding
> pgaudit as an external feature now would end up preventing us from
> moving forward in this area for years to come..

OK. I've marked the patch as rejected in the CF, but of course we hope
to see further discussion about an in-core implementation for 9.5.

Thank you for your feedback.

-- Abhijit

On Mon, Jun 23, 2014 at 6:51 AM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> wrote:
> There are some unresolved questions with #2 because the extensible
> reloptions patch seems to have lost favour, but I'm pretty sure we
> could figure out some alternative.

I didn't particularly like the proposed *implementation* of extensible
reloptions, but I think the general concept has merit.

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

On Mon, Jun 23, 2014 at 9:50 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Fujii Masao (masao(dot)fujii(at)gmail(dot)com) wrote:
>> On Mon, Jun 23, 2014 at 7:51 PM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> wrote:
>> > At 2014-06-23 19:15:39 +0900, masao(dot)fujii(at)gmail(dot)com wrote:
>> >> You added this into CF, but its patch has not been posted yet. Are you
>> >> planning to make a patch?
>> >
>> > It's a self-contained contrib module. I thought Ian had posted a
>> > tarball, but it looks like he forgot to attach it (or decided to
>> > provide only a Github link). I've attached a tarball here for
>> > your reference.
>
> I'm not a huge fan of adding this as a contrib module unless we can be
> quite sure that there's a path forward from here to a rework of the
> logging in core which would actually support the features pg_audit is
> adding, without a lot of pain and upgrade issues. Those issues have
> kept other contrib modules from being added to core.
>
> Splitting up contrib into other pieces, one of which is a 'features'
> area, might address that but we'd really need a way to have those pieces
> be able to include/add catalog tables, at least..
>
>> >> If not, it might be better to implement audit feature in core from the
>> >> beginning.
>> >
>> > Sure, we're open to that possibility. Do you have any ideas about what
>> > an in-core implementation should do/look like?
>>
>> I don't have good idea about that. But maybe we can merge pgaudit.log
>> into log_statement for more flexible settings of what to log.
>
> I'd expect a catalog table or perhaps changes to pg_class (maybe other
> things also..) to define what gets logged..

I'm not sure if this is good idea because this basically means that master
and every standbys must have the same audit settings and a user cannot
set what standby logs in standby side. Of course I guess that the audit
settings in standby would be similar to that in master generally, but I'm
not sure if that's always true.

Regards,

--
Fujii Masao

* Fujii Masao (masao(dot)fujii(at)gmail(dot)com) wrote:
> I'm not sure if this is good idea because this basically means that master
> and every standbys must have the same audit settings and a user cannot
> set what standby logs in standby side. Of course I guess that the audit
> settings in standby would be similar to that in master generally, but I'm
> not sure if that's always true.

The main difference would be that the standby wouldn't be logging
anything about data changing.. but that's to be expected.

Certainly when auditing of select queries and similar actions are done
to satisfy government or industry compliance requirements, it's about
"who reads the data", regardless of where that data is..

Thanks,

Stephen

Abhijit,

* Abhijit Menon-Sen (ams(at)2ndQuadrant(dot)com) wrote:
> At 2014-06-23 16:51:55 -0400, sfrost(at)snowman(dot)net wrote:
> > Are both the connected user and the current role that the command is
> > running under logged?
>
> Yes, they are. -------------------------------------+----+

Ok, great, I couldn't remember. Wish we had that ability in the current
logging code...

> > I'd much rather have that in-core capability and I worry that adding
> > pgaudit as an external feature now would end up preventing us from
> > moving forward in this area for years to come..
>
> OK. I've marked the patch as rejected in the CF, but of course we hope
> to see further discussion about an in-core implementation for 9.5.

I'm certainly all for it, though I'm not sure if I'll have resources
myself to be able to make it happen this fall.. Will you (collectively)
be working in this direction for 9.5? That'd certainly be great news
from my quadrant (pun fully intended ;).

Thanks!

Stephen

At 2014-06-24 14:02:11 -0400, sfrost(at)snowman(dot)net wrote:
>
> Will you (collectively) be working in this direction for 9.5?

We have some time available to work on it, but not so much that I want
to write any more code without a clearer idea of what might be accepted
eventually for inclusion.

-- Abhijit

Abhijit,

* Abhijit Menon-Sen (ams(at)2ndquadrant(dot)com) wrote:
> At 2014-06-24 14:02:11 -0400, sfrost(at)snowman(dot)net wrote:
> >
> > Will you (collectively) be working in this direction for 9.5?
>
> We have some time available to work on it, but not so much that I want
> to write any more code without a clearer idea of what might be accepted
> eventually for inclusion.

You and me both... (see nearby discussion regarding the redesign of
RLS..). For my part, the nexts steps might be to consider how you'd
migrate what you've provided for configuration into catalog tables and
how we'd address the concerns raised elsewhere regarding catalog access
in cases where we're not in a transaction (or at least addressing those
areas and working out what the logging would do in those situations..).

We'd also end up re-working the code to be called as part of PG core
rather than through hook functions, of course, but I don't think those
changes would be too bad compared to figuring out the other issues.

Additionally, thought towards what the SQL-level syntax would be is
another key point- would the main command be 'ALTER AUDIT'? What would
the sub-commands of that look like for the DBA/auditor who is tasked
with defining/implementing the auditing for the system? How would we
include data in a structured, yet flexible way? (That is to say, the
set of tables and columsn logged could be varied, yet we'd want to see
the actual data logged- perhaps as JSON?).

Looking forward to your thoughts.

Thanks!

Stephen

Stephen Frost wrote:
> Abhijit,
>
> * Abhijit Menon-Sen (ams(at)2ndquadrant(dot)com) wrote:
> > At 2014-06-24 14:02:11 -0400, sfrost(at)snowman(dot)net wrote:
> > >
> > > Will you (collectively) be working in this direction for 9.5?
> >
> > We have some time available to work on it, but not so much that I want
> > to write any more code without a clearer idea of what might be accepted
> > eventually for inclusion.
>
> You and me both... (see nearby discussion regarding the redesign of
> RLS..). For my part, the nexts steps might be to consider how you'd
> migrate what you've provided for configuration into catalog tables and
> how we'd address the concerns raised elsewhere regarding catalog access
> in cases where we're not in a transaction (or at least addressing those
> areas and working out what the logging would do in those situations..).

I think the whole idea of storing audit info in catalogs should go away
entirely. There are, it seems to me, too many problems with that.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

At 2014-06-25 00:10:55 -0400, sfrost(at)snowman(dot)net wrote:
>
> For my part, the nexts steps might be to consider how you'd migrate
> what you've provided for configuration into catalog tables

I must confess that I do not understand what needs to be migrated into
the catalog tables, or why. Of course, pgaudit.log must be renamed, but
why can't it continue to be a GUC setting? (Fujii-san suggested that it
be integrated with log_statement. I'm not sure what I think of that, but
it's certainly one possibility.)

> and how we'd address the concerns raised elsewhere regarding catalog
> access in cases where we're not in a transaction

…by not putting things into the catalog?

If we implement per-object auditing configuration in-core, it can use a
real reloption. Apart from that, I don't see a really good reason yet to
put more things into the database.

> We'd also end up re-working the code to be called as part of PG core
> rather than through hook functions, of course, but I don't think those
> changes would be too bad compared to figuring out the other issues.

You're right (but we'd still want to use event triggers). Maybe it would
make sense to have an auditing hook that we can sprinkle calls to in all
the interesting places, though.

> Additionally, thought towards what the SQL-level syntax would be is
> another key point- would the main command be 'ALTER AUDIT'?

(I have some thoughts about that, but I'll discuss them later when I
have a bit more time to present them properly.)

-- Abhijit

Alvaro,

* Alvaro Herrera (alvherre(at)2ndquadrant(dot)com) wrote:
> Stephen Frost wrote:
> > * Abhijit Menon-Sen (ams(at)2ndquadrant(dot)com) wrote:
> > > We have some time available to work on it, but not so much that I want
> > > to write any more code without a clearer idea of what might be accepted
> > > eventually for inclusion.
> >
> > You and me both... (see nearby discussion regarding the redesign of
> > RLS..). For my part, the nexts steps might be to consider how you'd
> > migrate what you've provided for configuration into catalog tables and
> > how we'd address the concerns raised elsewhere regarding catalog access
> > in cases where we're not in a transaction (or at least addressing those
> > areas and working out what the logging would do in those situations..).
>
> I think the whole idea of storing audit info in catalogs should go away
> entirely. There are, it seems to me, too many problems with that.

I'm completely against the notion of managing auditing requirements and
configurations which reference tables, users, and other objects which
exist in the catalog by using flat files. To me, that's ridiculous on
the face of it. Other databases have had this kind of capability as a
matter of course for decades- we are far behind in this area.

Thanks,

Stephen

* Abhijit Menon-Sen (ams(at)2ndquadrant(dot)com) wrote:
> At 2014-06-25 00:10:55 -0400, sfrost(at)snowman(dot)net wrote:
> > For my part, the nexts steps might be to consider how you'd migrate
> > what you've provided for configuration into catalog tables
>
> I must confess that I do not understand what needs to be migrated into
> the catalog tables, or why. Of course, pgaudit.log must be renamed, but
> why can't it continue to be a GUC setting? (Fujii-san suggested that it
> be integrated with log_statement. I'm not sure what I think of that, but
> it's certainly one possibility.)

What I was getting at are things like "what tables are to be audited,
and for what users?". I thought pg_audit had this capability today
(isn't that part of the request for the rlsextension field..?) and I'd
want to see something like

AUDIT SELECT ON table1 FOR role1;

> > and how we'd address the concerns raised elsewhere regarding catalog
> > access in cases where we're not in a transaction
>
> …by not putting things into the catalog?

I just don't see that as viable.

> If we implement per-object auditing configuration in-core, it can use a
> real reloption. Apart from that, I don't see a really good reason yet to
> put more things into the database.

I don't think the auditing will be so simple as being able to be
supported by a single additional field or with just reloption. Consider
what's happening on the RLS thread.

> > We'd also end up re-working the code to be called as part of PG core
> > rather than through hook functions, of course, but I don't think those
> > changes would be too bad compared to figuring out the other issues.
>
> You're right (but we'd still want to use event triggers). Maybe it would
> make sense to have an auditing hook that we can sprinkle calls to in all
> the interesting places, though.

Not against using event triggers if they make sense.. I'm not 100% sure
that I agree that they do, but I haven't looked at it closely enough to
really form an opinion.

> > Additionally, thought towards what the SQL-level syntax would be is
> > another key point- would the main command be 'ALTER AUDIT'?
>
> (I have some thoughts about that, but I'll discuss them later when I
> have a bit more time to present them properly.)

I find it really helps to try and define an SQL syntax as it can help
build an understanding of what's going to be in the catalogs and what
features are expected.

Thanks,

Stephen

At 2014-06-25 10:36:07 -0400, sfrost(at)snowman(dot)net wrote:
>
> To me, that's ridiculous on the face of it. Other databases have had
> this kind of capability as a matter of course for decades- we are far
> behind in this area.

OK, I've done a bit more thinking, but I'm not convinced that it's so
cut-and-dried (as "ridiculous on the face of it").

Ian looked into the auditing capabilities of various (SQL and otherwise)
systems some time ago. Informix handles its auditing configuration
entirely outside the database. DB2 uses a mixture of in-database and
external configuration. Oracle stores everything in the database.

We're certainly not going to invent a mechanism other than GUC settings
or catalog tables for auditing information (à la Informix). What I think
you're saying is that without storing stuff in the catalog tables, we're
not going to be able to offer auditing capabilities worth having. I can
appreciate that, but I'm not sure I agree.

You're right, it isn't possible to sanely do something like "AUDIT
SELECT ON TABLE t1 FOR role1" without storing configuration in the
catalog tables. You're right, it would be nice to have that level
of control.

The part that I don't agree about is that using a GUC setting and
a reloption won't give us anything useful. We can use that to do
global, per-database, per-user, and per-object auditing with just
mechanisms that are familiar to Postgres users already (ALTER … SET).
Some other messages in the thread have suggested a similar mechanism,
which implies that at least some people wouldn't be unhappy with it.

No, we couldn't do combinations of TABLE/ROLE, but there's also no
terrible conflict with doing that via some new "AUDIT foo ON bar"
syntax later.

As you point out, we're decades behind, and I seriously doubt if
we're going to jump forward a few decades in time for 9.5.

What do others think of the tradeoff?

-- Abhijit

On 25 June 2014 15:40, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Abhijit Menon-Sen (ams(at)2ndquadrant(dot)com) wrote:
>> At 2014-06-25 00:10:55 -0400, sfrost(at)snowman(dot)net wrote:
>> > For my part, the nexts steps might be to consider how you'd migrate
>> > what you've provided for configuration into catalog tables
>>
>> I must confess that I do not understand what needs to be migrated into
>> the catalog tables, or why. Of course, pgaudit.log must be renamed, but
>> why can't it continue to be a GUC setting? (Fujii-san suggested that it
>> be integrated with log_statement. I'm not sure what I think of that, but
>> it's certainly one possibility.)
>
> What I was getting at are things like "what tables are to be audited,
> and for what users?". I thought pg_audit had this capability today
> (isn't that part of the request for the rlsextension field..?) and I'd
> want to see something like
>
> AUDIT SELECT ON table1 FOR role1;
>
>> > and how we'd address the concerns raised elsewhere regarding catalog
>> > access in cases where we're not in a transaction
>>
>> …by not putting things into the catalog?
>
> I just don't see that as viable.

"Which tables are audited" would be available via the reloptions
field. This is basically the difference between information being
stored within the reloptions field (think of its as an hstore column)
and being stored in a separate audit-specific column. I see no
difference between them, we just need to provide a view that extracts
the useful information. It is very important we add new things as
reloptions so that we don't have to continually add new code elsewhere
to handle all the new options we add.

I don't agree with adding "AUDIT ..." syntax to the parser and having
top-level SQL commands. That just bloats the parser for no particular
gain. With reloptions we already support all the required DDL. No
further work required.

We want an audit feature in core 9.5, but I don't see those adding SQL
and adding specific catalog columns as giving us anything at all. It
will still be an audit feature without them.

Many, many users do not want audit. Many do. So the idea is to allow
audit to be added in a way that doesn't affect everybody else.

So lets commit pgaudit now and spend time on genuine enhancements to
the functionality, not just rewriting the same thing into a different
form that adds no value.

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

On 23 June 2014 21:51, Stephen Frost <sfrost(at)snowman(dot)net> wrote:

> I also wouldn't want this to become
> an excuse to not improve our current logging infrastructure, which is
> how we got to the place we are wrt partitioning, imv.

Not the case at all.

I wrote the existing partitioning code in 6 weeks in 2005, with review
and rewrite by Tom. It was fairly obvious after a year or two that
there were major shortcomings that needed to be addressed.

Partitioning has been held back by not having someone that genuinely
understands both the problem and reasonable solutions from spending
time on this in the last 9 years. We've had 2.5 attempts over that
time, but the first two were completely wasted because they weren't
discussed on list first and when they were the ideas in those patches
were shot down in seconds, regrettably.

Nothing about this patch bears any resemblance to that situation.

I personally never had the time or money to fix that situation until
now, so I'm hoping and expecting that that will change in 9.5, as
discussed in developer meeting.

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

On 14/06/25 23:36, Stephen Frost wrote:
> Other databases have had this kind of capability as a
> matter of course for decades- we are far behind in this area.

On a related note, MySQL/MariaDB have had some sort of auditing
capability for, well, months. By no means as sophisticated as
some of the others, but still more than nothing.

https://www.mysql.com/products/enterprise/audit.html
https://mariadb.com/kb/en/about-the-mariadb-audit-plugin/

Regards

Ian Barwick

--
Ian Barwick http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Abhijit,

* Abhijit Menon-Sen (ams(at)2ndquadrant(dot)com) wrote:
> Ian looked into the auditing capabilities of various (SQL and otherwise)
> systems some time ago. Informix handles its auditing configuration
> entirely outside the database. DB2 uses a mixture of in-database and
> external configuration. Oracle stores everything in the database.

We'll certainly still have some external configuration, but not things
which are referring to specific roles or tables. We will also need a
way to manage that configuration such that an individual could have only
access to modify the audit parameters without also having lots of other
rights to the system. Having a separate auditing role is called out as
a requirement for quite a few different industries.

> We're certainly not going to invent a mechanism other than GUC settings
> or catalog tables for auditing information (à la Informix). What I think
> you're saying is that without storing stuff in the catalog tables, we're
> not going to be able to offer auditing capabilities worth having. I can
> appreciate that, but I'm not sure I agree.

This is going a bit father than I intended. I was trying to outline
where we want to be going with this and pointing out that if we put
pgaudit out there as "this is how you do auditing in PG" that we'll be
stuck with it more-or-less forever- and that it won't satisfy critical
use-cases and would minimize PG's ability to be used in those
industries. Further, if we go with this approach now, we will end up
creating a huge upgrade headache (which could end up being used as a
reason to not have the in-core capabilities).

A catalog-based approach which had only the specific features that
pgaudit has today, but is implemented in such a way that we'll be able
to add those other features in the future would be great, imv. What
this looks like, however, is a solution which only gets us half-way and
then makes it quite difficult to go farther.

> You're right, it isn't possible to sanely do something like "AUDIT
> SELECT ON TABLE t1 FOR role1" without storing configuration in the
> catalog tables. You're right, it would be nice to have that level
> of control.

Great, glad we agree on that. :)

> The part that I don't agree about is that using a GUC setting and
> a reloption won't give us anything useful. We can use that to do
> global, per-database, per-user, and per-object auditing with just
> mechanisms that are familiar to Postgres users already (ALTER … SET).

I've clearly miscommunicated and my apologies for that- it's not that
the feature set isn't useful or that using GUCs and reloptions wouldn't
be useful, it's much more than I don't want that to be our end-state and
I don't think we'll have an easy time moving from GUCs and reloptions to
the end-state that I'm looking at. What I was balking at was the notion
that GUCs and reloptions would satisfy the auditing requirements for all
(or even most) of our users. It would be sufficient and useful for
some, but certainly not all.

> Some other messages in the thread have suggested a similar mechanism,
> which implies that at least some people wouldn't be unhappy with it.

Sure.

> No, we couldn't do combinations of TABLE/ROLE, but there's also no
> terrible conflict with doing that via some new "AUDIT foo ON bar"
> syntax later.

We also wouldn't be able to provide fine-grained control over who is
allowed to define the auditing requirements and I don't see how you'd
support per-column auditing definitions or integrate auditing with RLS.

> As you point out, we're decades behind, and I seriously doubt if
> we're going to jump forward a few decades in time for 9.5.

Agreed- but as has been pointed out to me on the RLS thread, we don't
want to put something into 9.5 which will mean we can't ever catch up
because of backwards compatibility or pg_upgrade issues.

Thanks,

Stephen

Simon,

* Simon Riggs (simon(at)2ndQuadrant(dot)com) wrote:
> "Which tables are audited" would be available via the reloptions
> field.

RLS could be implemented through reloptions too. Would it be useful to
some people? Likely. Would it satisfy the users who, today, are
actually asking for that feature? No (or at least, not the ones that
I've talked with). We could expand quite a few things to work through
reloptions but I don't see it as a particularly good design for complex
subsystems, of which auditing is absolutely one of those.

I've gotten quite a bit of push-back and been working through a much
more detailed (but, all-in-all, good, imv) design and roadmap for RLS
with targets for 9.5 and beyond. I simply do not understand why these
same concerns apparently don't apply to pgaudit. Being an extension
which lives in contrib can be, in many ways, worse than having nothing
at all distributed with PG because it causes pg_upgrade and backwards
compatibility issues.

> This is basically the difference between information being
> stored within the reloptions field (think of its as an hstore column)
> and being stored in a separate audit-specific column. I see no
> difference between them, we just need to provide a view that extracts
> the useful information. It is very important we add new things as
> reloptions so that we don't have to continually add new code elsewhere
> to handle all the new options we add.

Auditing isn't going to be a polished and solved piece of core PG with
an implementation that uses only single additional column (even an
hstore or json column), imv.

> I don't agree with adding "AUDIT ..." syntax to the parser and having
> top-level SQL commands. That just bloats the parser for no particular
> gain. With reloptions we already support all the required DDL. No
> further work required.

I disagree that new top-level syntax is completely off the table, but
I'm open to another solution for SQL-based auditing specifications. I
would argue that we must consider how we will support a permissions
system around auditing which differs from the owner of the relation or
owner of the database and certainly don't want the control over auditing
to require superuser rights.

> We want an audit feature in core 9.5, but I don't see those adding SQL
> and adding specific catalog columns as giving us anything at all. It
> will still be an audit feature without them.

I'd love to see auditing happen in 9.5 too. I'd also like to see RLS in
9.5, but that doesn't mean I'm ignoring the concerns about making sure
that we have a good design which will add useful capabilities in 9.5
while allowing the more advanced capabilities to be added later.

> Many, many users do not want audit. Many do. So the idea is to allow
> audit to be added in a way that doesn't affect everybody else.

Sure.

> So lets commit pgaudit now and spend time on genuine enhancements to
> the functionality, not just rewriting the same thing into a different
> form that adds no value.

I think this will paint us into a corner such that we won't be able to
add the capabilities which the users who are most concerned about
auditing require. I really don't want that to happen as it will mean
that we have an auditing system for the users who like the idea of
auditing but don't have real security issues, while the large industries
we're targeting won't be able to use PG.

Thanks,

Stephen

On 26 June 2014 14:59, Stephen Frost <sfrost(at)snowman(dot)net> wrote:

> I think this will paint us into a corner such that we won't be able to
> add the capabilities which the users who are most concerned about
> auditing require.

I'm sorry, but this seems exactly the wrong way around to me.

The point here is that we have an extension, right now. Per table
auditing can be supported trivially via reloptions. The alternative is
to fatten the grammar with loads of noise words and invent some new
specific catalog stuff. That gives us no new features over what we
have here, plus it has a hard cost of at least 3 man months work -
starting that now endangers getting a feature into 9.5. Doing that
will force the audit feature to be an in-core only solution, meaning
it cannot be tailored easily for individual requirements and it will
evolve much more slowly towards where our users want it to be.

So I see your approach costing more, taking longer and endangering the
feature schedule, yet offering nothing new. The hard cost isn't
something that should be ignored, we could spend money adding new
features or we could waste it rewriting things. Cost may mean little
to some, but we need to realise that increasing costs may make
something infeasible. We have at most 1 more man month of funds to
assist here, after that we're into volunteer time, which never goes
far.

Anyway, what we should do now is talk about what features we want and
detail what the requirements are, so we stand a chance of assessing
things in the right context.

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

* Simon Riggs (simon(at)2ndQuadrant(dot)com) wrote:
> On 26 June 2014 14:59, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > I think this will paint us into a corner such that we won't be able to
> > add the capabilities which the users who are most concerned about
> > auditing require.
>
> I'm sorry, but this seems exactly the wrong way around to me.

Can you explain how we are going to migrate from the proposed pgaudit
approach to an in-core approach with the flexibility discussed? That's
really what I'd like to hear and to discuss.

I don't understand why we shouldn't be considering and discussing that
at this point.

> The point here is that we have an extension, right now.

I understand that. We have a patch for async I/O too. We also have an
RLS patch today (one which has been around for a few months...). That
doesn't mean these things can just go in without any further discussion.
I can't just go commit RLS and ignore the concerns raised by Robert and
others simply because I have an implementation today, yet that seems to
be the thrust of this argument.

> Per table
> auditing can be supported trivially via reloptions. The alternative is
> to fatten the grammar with loads of noise words and invent some new
> specific catalog stuff. That gives us no new features over what we
> have here, plus it has a hard cost of at least 3 man months work -
> starting that now endangers getting a feature into 9.5.

I'm happy to hear your concerns regarding the level of effort required
and that we need to be pushing to have things worked through and
hopefully committed earlier in this cycle rather than waiting til the
end and possibly missing 9.5

> Doing that
> will force the audit feature to be an in-core only solution, meaning
> it cannot be tailored easily for individual requirements and it will
> evolve much more slowly towards where our users want it to be.

This makes less than zero sense to me- having it in core allows us
flexibility with regard to what it does and how it works (more so than
an extension does, in fact.. the extension is obviously constrained in
terms of what it is capable of doing). I agree that changes to an
in-core solution will need to follow the PG release cycle- but the same
is true of contrib modules. If you want your own release cycle for
pgaudit which is independent of the PG one, then you should be putting
it up on PGXN rather than submitting it to be included in contrib.

The rate of evolution is defined by those who are working on it, with
the caveat that new features, etc, happen as part of the PG release
cycle which are snapshots of where we are at a given point.

> So I see your approach costing more, taking longer and endangering the
> feature schedule, yet offering nothing new.

Just because you choose to ignore the additional flexibility and
capability which an in-core solution would offer doesn't mean that it
doesn't exist. Having a real SQL syntax which DBAs can use through the
PG protocol and tailor to their needs, with a permissions model around
it, is a valuable feature over and above what is being proposed here.

> The hard cost isn't
> something that should be ignored, we could spend money adding new
> features or we could waste it rewriting things. Cost may mean little
> to some, but we need to realise that increasing costs may make
> something infeasible. We have at most 1 more man month of funds to
> assist here, after that we're into volunteer time, which never goes
> far.

I'm quite aware that there are costs to doing development. I do not
agree that means we should be compromising or putting ourselves in a
position which prevents us from moving forward.

> Anyway, what we should do now is talk about what features we want and
> detail what the requirements are, so we stand a chance of assessing
> things in the right context.

Sure, I had been doing that earlier in the thread before we got off on
this tangent..

Thanks,

Stephen

On Thu, Jun 26, 2014 at 3:50 AM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> wrote:
> At 2014-06-25 10:36:07 -0400, sfrost(at)snowman(dot)net wrote:
>>
>> To me, that's ridiculous on the face of it. Other databases have had
>> this kind of capability as a matter of course for decades- we are far
>> behind in this area.
>
> OK, I've done a bit more thinking, but I'm not convinced that it's so
> cut-and-dried (as "ridiculous on the face of it").
>
> Ian looked into the auditing capabilities of various (SQL and otherwise)
> systems some time ago. Informix handles its auditing configuration
> entirely outside the database. DB2 uses a mixture of in-database and
> external configuration. Oracle stores everything in the database.
>
> We're certainly not going to invent a mechanism other than GUC settings
> or catalog tables for auditing information (à la Informix). What I think
> you're saying is that without storing stuff in the catalog tables, we're
> not going to be able to offer auditing capabilities worth having. I can
> appreciate that, but I'm not sure I agree.
>
> You're right, it isn't possible to sanely do something like "AUDIT
> SELECT ON TABLE t1 FOR role1" without storing configuration in the
> catalog tables. You're right, it would be nice to have that level
> of control.
>
> The part that I don't agree about is that using a GUC setting and
> a reloption won't give us anything useful. We can use that to do
> global, per-database, per-user, and per-object auditing with just
> mechanisms that are familiar to Postgres users already (ALTER … SET).
> Some other messages in the thread have suggested a similar mechanism,
> which implies that at least some people wouldn't be unhappy with it.
>
> No, we couldn't do combinations of TABLE/ROLE, but there's also no
> terrible conflict with doing that via some new "AUDIT foo ON bar"
> syntax later.
>
> As you point out, we're decades behind, and I seriously doubt if
> we're going to jump forward a few decades in time for 9.5.
>
> What do others think of the tradeoff?

I agree with Stephen's statement that the audit configuration
shouldn't be managed in flat files. If somebody wants to do that in
an extension, fine, but I don't think we should commit anything into
our source tree that works that way. The right way to manage that
kind of configuration is with GUCs, or reloptions, or (security)
labels, or some other kind of catalog structure. Renaming an object
shouldn't be able to break auditing, but if the audit configuration is
a list of table names in a file somewhere, it will.

I disagree with Stephen's proposal that this should be in core, or
that it needs its own dedicated syntax. I think contrib is a great
place for extensions like this. That makes it a whole lot easier for
people to develop customized vesions that meet particular needs they
may have, and it avoids bloating core with a bunch of stuff that is
only of interest to a subset of users. We spent years getting sepgsql
into a form that could be shipped in contrib without substantially
burdening core, and I don't see why this feature should be handed any
differently.

In fact, considering how much variation there is likely to be between
auditing requirements at different sites, I'm not sure this should
even be in contrib at this point. It's clearly useful, but there is
no requirement that every useful extension must be bundled with the
core server, and in fact doing so severely limits our ability to make
further changes.

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

Robert,

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> I agree with Stephen's statement that the audit configuration
> shouldn't be managed in flat files. If somebody wants to do that in
> an extension, fine, but I don't think we should commit anything into
> our source tree that works that way. The right way to manage that
> kind of configuration is with GUCs, or reloptions, or (security)
> labels, or some other kind of catalog structure. Renaming an object
> shouldn't be able to break auditing, but if the audit configuration is
> a list of table names in a file somewhere, it will.

Thanks for your thoughts on this.

> I disagree with Stephen's proposal that this should be in core, or
> that it needs its own dedicated syntax. I think contrib is a great
> place for extensions like this. That makes it a whole lot easier for
> people to develop customized vesions that meet particular needs they
> may have, and it avoids bloating core with a bunch of stuff that is
> only of interest to a subset of users. We spent years getting sepgsql
> into a form that could be shipped in contrib without substantially
> burdening core, and I don't see why this feature should be handed any
> differently.

I don't exactly see how an extension or contrib module is going to be
able to have a reasonable catalog structure which avoids the risk that
renaming an object (role, schema, table, column, policy) will break the
configuration without being part of core. Add on to that the desire for
audit control to be a grant'able right along the lines of:

GRANT AUDIT ON TABLE x TO audit_role;

which allows a user who is not the table owner or a superuser to manage
the auditing. I'm not against implementing capabilities through contrib
modules, provided the goals of the feature can be met that way. In a
way, sepgsql is actually a good example for this- the module provides
the implementation, but all of the syntax and catalog information is
actually in core. An implementation similar to that for auditing might
be workable, but I don't see pgaudit as being that.

> In fact, considering how much variation there is likely to be between
> auditing requirements at different sites, I'm not sure this should
> even be in contrib at this point. It's clearly useful, but there is
> no requirement that every useful extension must be bundled with the
> core server, and in fact doing so severely limits our ability to make
> further changes.

For my part, I do view auditing as being a requirement we should be
addressing through core- and not just for the reasons mentioned above.
We might be able to manage all of the above through an extension,
however, as we continue to add capabilities and features, new types and
methods, ways to extend the system, and more, auditing needs to keep
pace and be considered a core feature as much as the GRANT system is.

Thanks,

Stephen

On Fri, Jun 27, 2014 at 2:23 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>> I disagree with Stephen's proposal that this should be in core, or
>> that it needs its own dedicated syntax. I think contrib is a great
>> place for extensions like this. That makes it a whole lot easier for
>> people to develop customized vesions that meet particular needs they
>> may have, and it avoids bloating core with a bunch of stuff that is
>> only of interest to a subset of users. We spent years getting sepgsql
>> into a form that could be shipped in contrib without substantially
>> burdening core, and I don't see why this feature should be handed any
>> differently.
>
> I don't exactly see how an extension or contrib module is going to be
> able to have a reasonable catalog structure which avoids the risk that
> renaming an object (role, schema, table, column, policy) will break the
> configuration without being part of core.

At some level, I guess that's true, but a custom GUC would get you
per-role and per-database and a custom reloption could take it
further. A security label provider specifically for auditing can
already associate custom metadata with just about any SQL-visible
object in the system.

> Add on to that the desire for
> audit control to be a grant'able right along the lines of:
>
> GRANT AUDIT ON TABLE x TO audit_role;
>
> which allows a user who is not the table owner or a superuser to manage
> the auditing.

As Tom would say, I think you just moved the goalposts into the next
county. I don't know off-hand what that syntax is supposed to allow,
and I don't think it's necessary for pgaudit to implement that (or
anything else that you may want) in order to be a viable facility.

>> In fact, considering how much variation there is likely to be between
>> auditing requirements at different sites, I'm not sure this should
>> even be in contrib at this point. It's clearly useful, but there is
>> no requirement that every useful extension must be bundled with the
>> core server, and in fact doing so severely limits our ability to make
>> further changes.
>
> For my part, I do view auditing as being a requirement we should be
> addressing through core- and not just for the reasons mentioned above.
> We might be able to manage all of the above through an extension,
> however, as we continue to add capabilities and features, new types and
> methods, ways to extend the system, and more, auditing needs to keep
> pace and be considered a core feature as much as the GRANT system is.

I think the fact that pgaudit does X and you think it should do Y is a
perfect example of why we're nowhere close to being ready to push
anything into core. We may very well want to do that someday, but not
yet.

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

Robert,

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> On Fri, Jun 27, 2014 at 2:23 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > I don't exactly see how an extension or contrib module is going to be
> > able to have a reasonable catalog structure which avoids the risk that
> > renaming an object (role, schema, table, column, policy) will break the
> > configuration without being part of core.
>
> At some level, I guess that's true, but a custom GUC would get you
> per-role and per-database and a custom reloption could take it
> further. A security label provider specifically for auditing can
> already associate custom metadata with just about any SQL-visible
> object in the system.

Ugh. Yes, we could implement a lot of things using per-object GUCs and
reloptions. We could do the same for columns with custom attoptions.
For my part, that design looks absolutely horrible to me for
more-or-less everything. I certainly don't feel like it's the solution
which extension authors are looking for and will be happy with, nor do I
feel it's the right answer for our users.

> > Add on to that the desire for
> > audit control to be a grant'able right along the lines of:
> >
> > GRANT AUDIT ON TABLE x TO audit_role;
> >
> > which allows a user who is not the table owner or a superuser to manage
> > the auditing.
>
> As Tom would say, I think you just moved the goalposts into the next
> county. I don't know off-hand what that syntax is supposed to allow,
> and I don't think it's necessary for pgaudit to implement that (or
> anything else that you may want) in order to be a viable facility.

Perhaps I should have used the same argument on the RLS thread..
Instead, I realized that you and others were right- we don't want to
implement and commit something which would make adding the features
being asked for very difficult to do in the future. The point of the
command above is to allow a role *other* than the table owner to
control the auditing on the table. It's important in certain places
that the auditor have the ability to control the auditing and *only*
the auditing- not drop the table or change its definition.

I'm also not asking for this to be implemented today in pgaudit but
rather to point out that we should have a design which at least allows
us to get to the point where these features can be added. Perhaps
there's a path to allowing the control I'm suggesting here with the
existing pgaudit design and with pgaudit as an extension- if so, please
speak up and outline it sufficiently that we can understand that path
and know that we're not putting ourselves into a situation where we
won't be able to support that flexibility.

> > For my part, I do view auditing as being a requirement we should be
> > addressing through core- and not just for the reasons mentioned above.
> > We might be able to manage all of the above through an extension,
> > however, as we continue to add capabilities and features, new types and
> > methods, ways to extend the system, and more, auditing needs to keep
> > pace and be considered a core feature as much as the GRANT system is.
>
> I think the fact that pgaudit does X and you think it should do Y is a
> perfect example of why we're nowhere close to being ready to push
> anything into core. We may very well want to do that someday, but not
> yet.

That's fine- but don't push something in which will make it difficult to
add these capabilities later (and, to be clear, I'm not asking out of
pipe dreams and wishes but rather after having very specific discussions
with users and reviewing documents like NIST 800-53, which is publically
available for anyone to peruse).

Thanks,

Stephen

At 2014-06-30 09:39:29 -0400, sfrost(at)snowman(dot)net wrote:
>
> I certainly don't feel like it's the solution which extension authors
> are looking for and will be happy with

I don't know if there are any other extension authors involved in this
discussion, but I'm not shedding any tears over the idea. (That may be
because I see operational compatibility with 9.[234] as a major plus,
not a minor footnote.)

> > As Tom would say, I think you just moved the goalposts into
> > the next county.

(And they're not even the same distance apart any more. ;-)

> That's fine- but don't push something in which will make it difficult
> to add these capabilities later

I've been trying to understand why a pgaudit extension (which already
exists) will make it difficult to add a hypothetical "GRANT AUDIT ON
goalpost TO referee" syntax later. About the only thing I've come up
with is people complaining about having to learn the new syntax when
they were used to the old one.

Surely that's not the sort of thing you mean? (You've mentioned
pg_upgrade and backwards compatibility too, and I don't really
understand those either.)

-- Abhijit

Abhijit,

* Abhijit Menon-Sen (ams(at)2ndQuadrant(dot)com) wrote:
> At 2014-06-30 09:39:29 -0400, sfrost(at)snowman(dot)net wrote:
> > I certainly don't feel like it's the solution which extension authors
> > are looking for and will be happy with
>
> I don't know if there are any other extension authors involved in this
> discussion, but I'm not shedding any tears over the idea. (That may be
> because I see operational compatibility with 9.[234] as a major plus,
> not a minor footnote.)

We're certainly not going to include this in the -contrib packages for
9.4-or-earlier, so I've not really been thinking about those concerns,
no. Indeed, if you want that to be supported by packagers, they'd
probably be happier with it being an independent extension distributed
through pgxn or similar.. Having an extension for 9.4-and-earlier which
isn't in -contrib then be moved to -contrib for 9.5 is at least an
annoyance when it comes to packaging.

> > That's fine- but don't push something in which will make it difficult
> > to add these capabilities later
>
> I've been trying to understand why a pgaudit extension (which already
> exists) will make it difficult to add a hypothetical "GRANT AUDIT ON
> goalpost TO referee" syntax later. About the only thing I've come up
> with is people complaining about having to learn the new syntax when
> they were used to the old one.

I do think people will complain a bit about such a change, but I expect
that to be on the level of grumblings rather than real issues.

> Surely that's not the sort of thing you mean? (You've mentioned
> pg_upgrade and backwards compatibility too, and I don't really
> understand those either.)

Where would you store the information about the GRANTs? In the
reloptions too? Or would you have to write pg_upgrade to detect if
pgaudit had been installed and had played with the reloptions field to
then extract out the values from it into proper columns or an actual
table, not just for grants but for any other custom reloptions which
were used? Were pgaudit to grow any tables, functions, etc, we'd have
to address how those would be handled by pg_upgrade also. I don't think
that's an issue currently, but we'd have to be darn careful to make sure
it doesn't happen or we end up with the same upgrade issues hstore has.

We've absolutely had push-back regarding breaking things for people who
have installed extensions from -contrib by moving those things into core
without a very clearly defined way to *not* break them. There have been
proposals in the past to explicitly allocate/reserve OIDs to address
this issue but those haven't been successful.

I'm not saying that I know it's going to be impossible- I'm asking
that we consider how this might move into core properly in a way that
will make it possible to pg_upgrade. Part of that does include
considering what the design of the core solution will offer and what
feature set it will have..

Thanks,

Stephen

On Mon, Jun 30, 2014 at 9:39 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>> I think the fact that pgaudit does X and you think it should do Y is a
>> perfect example of why we're nowhere close to being ready to push
>> anything into core. We may very well want to do that someday, but not
>> yet.
>
> That's fine- but don't push something in which will make it difficult to
> add these capabilities later (and, to be clear, I'm not asking out of
> pipe dreams and wishes but rather after having very specific discussions
> with users and reviewing documents like NIST 800-53, which is publically
> available for anyone to peruse).

I don't think that's a valid objection. If we someday have auditing
in core, and if it subsumes what pgaudit does, then whatever
interfaces pgaudit implements can be replaced with wrappers around the
core functionality, just as we did for text search.

But personally, I think this patch deserves to be reviewed on its own
merits, and not the extent to which it satisfies your requirements, or
those of NIST 800-53. As I said before, I think auditing is a
complicated topic and there's no guarantee that one solution will be
right for everyone. As long as we keep those solutions out of core,
there's no reason that multiple solutions can't coexist; people can
pick the one that best meets their requirements. As soon as we start
talking about something putting into core, the bar is a lot higher,
because we're not going to put two auditing solutions into core, so if
we do put one in, it had better be the right thing for everybody. I
don't even think we should be considering that at this point; I think
the interesting (and under-discussed) question on this thread is
whether it even makes sense to put this into contrib. That means we
need some review of the patch for what it is, which there hasn't been
much of, yet.

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

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> On Mon, Jun 30, 2014 at 9:39 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> >> I think the fact that pgaudit does X and you think it should do Y is a
> >> perfect example of why we're nowhere close to being ready to push
> >> anything into core. We may very well want to do that someday, but not
> >> yet.
> >
> > That's fine- but don't push something in which will make it difficult to
> > add these capabilities later (and, to be clear, I'm not asking out of
> > pipe dreams and wishes but rather after having very specific discussions
> > with users and reviewing documents like NIST 800-53, which is publically
> > available for anyone to peruse).
>
> I don't think that's a valid objection. If we someday have auditing
> in core, and if it subsumes what pgaudit does, then whatever
> interfaces pgaudit implements can be replaced with wrappers around the
> core functionality, just as we did for text search.

I actually doubt that we'd be willing to do what we did for text search
again. Perhaps I'm wrong, which would be great, but I doubt it.

> But personally, I think this patch deserves to be reviewed on its own
> merits, and not the extent to which it satisfies your requirements, or
> those of NIST 800-53.

eh? The focus of this patch is to add auditing to PG and having very
clearly drawn auditing requirements of a very large customer isn't
relevant? I don't follow that logic at all. In fact, I thought the
point of pgaudit was specifically to help address the issues which are
being raised from this section of our user base (perhaps not those who
follow NIST, but at least those organizations with similar interests..).

> As I said before, I think auditing is a
> complicated topic and there's no guarantee that one solution will be
> right for everyone. As long as we keep those solutions out of core,
> there's no reason that multiple solutions can't coexist; people can
> pick the one that best meets their requirements. As soon as we start
> talking about something putting into core, the bar is a lot higher,
> because we're not going to put two auditing solutions into core, so if
> we do put one in, it had better be the right thing for everybody.

I agree that auditing is a complicated topic. Given the dearth of
existing auditing solutions, I seriously doubt we're going to actually
see more than one arise. I'd much rather focus on an in-core solution
which allows the mechanism by which auditing logs are produced by
through an extension (perhaps through hooks in the right places) but the
actual definition of *what* gets audited to be defined through SQL with
a permissions system that works with the rest of the system.

What I'm getting at is this- sure, different users will want to have
their audit logs be in different formats (JSON, XML, CSV, whatever), or
sent via different means (logged to files, sent to syslog, forwarded on
to a RabbitMQ system, splunk, etc), but the definition of what gets
audited and allowing individuals other than the owners to be able to
modify the auditing is much more clear-cut to me as there's only so many
different objects in the system and only so many operations which can be
performed on them.. This strikes me as similar to security labels, as
was discussed previously.

> I
> don't even think we should be considering that at this point; I think
> the interesting (and under-discussed) question on this thread is
> whether it even makes sense to put this into contrib. That means we
> need some review of the patch for what it is, which there hasn't been
> much of, yet.

Evidently I wasn't very clear on this point but my concerns are about it
going into contrib, which is what's been proposed, because I worry about
an eventual upgrade path from contrib to an in-core solution. I'm also
a bit nervous that a contrib solution might appear 'good enough' to
enough committers that the push-back on an in-core solution would lead
to us never having one. These concerns are, perhaps, not very palatable
but I feel they're real enough to bring up.

contrib works quite well for stand-alone sets of functions which don't,
and never will, have any in-core direct grammar or catalog support.
While pgaudit doesn't have those today, I'd like to see *auditing* in
PG, eventually, which has both a real grammar and a catalog definition.

Thanks,

Stephen

I'm sorry to interrupt you, but I feel strong sympathy with Stephen-san.

From: "Robert Haas" <robertmhaas(at)gmail(dot)com>
> I don't think that's a valid objection. If we someday have auditing
> in core, and if it subsumes what pgaudit does, then whatever
> interfaces pgaudit implements can be replaced with wrappers around the
> core functionality, just as we did for text search.

Won't it be burden and a headache to maintain pgaudit code when it becomes
obsolete in the near future?

> But personally, I think this patch deserves to be reviewed on its own
> merits, and not the extent to which it satisfies your requirements, or
> those of NIST 800-53. As I said before, I think auditing is a
> complicated topic and there's no guarantee that one solution will be
> right for everyone. As long as we keep those solutions out of core,
> there's no reason that multiple solutions can't coexist; people can
> pick the one that best meets their requirements. As soon as we start
> talking about something putting into core, the bar is a lot higher,
> because we're not going to put two auditing solutions into core, so if
> we do put one in, it had better be the right thing for everybody. I
> don't even think we should be considering that at this point; I think
> the interesting (and under-discussed) question on this thread is
> whether it even makes sense to put this into contrib. That means we
> need some review of the patch for what it is, which there hasn't been
> much of, yet.

Then, what is this auditing capability for? I don't know whether various
regulations place so different requirements on auditing, but how about
targeting some real requirements? What would make many people happy? PCI
DSS?

I bet Japanese customers are severe from my experience, and I'm afraid they
would be disappointed if PostgreSQL provides auditing functionality which
does not conform to any real regulations like PCI DSS, NIST, etc, now that
other major vendors provide auditing for years. They wouldn't want to
customize contrib code because DBMS development is difficult. I wish for
in-core serious auditing functionality.

Regards
MauMau

On 30 June 2014 16:20, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
>> On Mon, Jun 30, 2014 at 9:39 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>> >> I think the fact that pgaudit does X and you think it should do Y is a
>> >> perfect example of why we're nowhere close to being ready to push
>> >> anything into core. We may very well want to do that someday, but not
>> >> yet.
>> >
>> > That's fine- but don't push something in which will make it difficult to
>> > add these capabilities later (and, to be clear, I'm not asking out of
>> > pipe dreams and wishes but rather after having very specific discussions
>> > with users and reviewing documents like NIST 800-53, which is publically
>> > available for anyone to peruse).
>>
>> I don't think that's a valid objection. If we someday have auditing
>> in core, and if it subsumes what pgaudit does, then whatever
>> interfaces pgaudit implements can be replaced with wrappers around the
>> core functionality, just as we did for text search.
>
> I actually doubt that we'd be willing to do what we did for text search
> again. Perhaps I'm wrong, which would be great, but I doubt it.
>
>> But personally, I think this patch deserves to be reviewed on its own
>> merits, and not the extent to which it satisfies your requirements, or
>> those of NIST 800-53.
>
> eh? The focus of this patch is to add auditing to PG and having very
> clearly drawn auditing requirements of a very large customer isn't
> relevant? I don't follow that logic at all. In fact, I thought the
> point of pgaudit was specifically to help address the issues which are
> being raised from this section of our user base (perhaps not those who
> follow NIST, but at least those organizations with similar interests..).

It's not clear to me how NIST 800-53 mandates the use of SQL (or any
other) language statements for auditing. I strongly doubt that it
does, but I am happy to hear summaries of large documents, or specific
references to pages and paragraphs, just as we would do for SQL
Standard. There is no other difference between a function and a
statement - both may create catalog entries; I agree we need catalog
entries so that auditing config can itself be audited.

The rest of this argument seems to revolve around the idea that
"in-core" and "extension"/contrib are different things. That is much
more of a general point and one that is basically about development
style. We've built logical replication around the idea that the
various plugins can be the "in-core" version sometime in the future,
so anything you say along those lines affects that as well as sepgsql,
pgcrypto, pg_stat_statements etc.. . Adding things to the grammar, to
pg_proc.h and other hardcoding locations is seriously painful and
timeconsuming, one of the reasons why extensions and background
workers exist.

But if you missed it before, I will say it again: we have a patch now
and 2ndQuadrant has a limit on further funding. If you want to reject
pgaudit and move on to something else, lets start discussing that
design and consider who has the time (and implicitly the money) to
make that happen. We can pick the best one for inclusion in 9.5 later
in the cycle.

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

Simon,

* Simon Riggs (simon(at)2ndQuadrant(dot)com) wrote:
> On 30 June 2014 16:20, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > eh? The focus of this patch is to add auditing to PG and having very
> > clearly drawn auditing requirements of a very large customer isn't
> > relevant? I don't follow that logic at all. In fact, I thought the
> > point of pgaudit was specifically to help address the issues which are
> > being raised from this section of our user base (perhaps not those who
> > follow NIST, but at least those organizations with similar interests..).
>
> It's not clear to me how NIST 800-53 mandates the use of SQL (or any
> other) language statements for auditing.

800-53 doesn't mandate SQL. If that was implied by me somewhere then I
apologize for the confusion. Having SQL support *has* been requested of
multiple users and in some of those cases has been considered a
requirement for their specific case; perhaps that was the confusion.

What 800-53 *does* discuss (under AU-9, control 4, from here:
http://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-53r4.pdf)
is the auditor role as being independent and able to control auditing.
Further, it discusses under AU-2 that organizations must determine the
set of events which are to be audited and then under AU-12 (in general,
but also specifically under AU-12 control 3), that authorized
individuals can change what is being audited.

Another point is a practical matter- it's extremely important to control
where the audit logs go as well, and ideally it'd be possible to send
audit logs for certain RLS policies to one location while other audit
logs are sent to a different (perhaps less secure) location. That's an
issue which I don't believe has to be addressed immediately, but until
it is addressed the audit log has to be at the highest level of control,
which makes it more difficult to manage and monitor.

> I strongly doubt that it
> does, but I am happy to hear summaries of large documents, or specific
> references to pages and paragraphs, just as we would do for SQL
> Standard. There is no other difference between a function and a
> statement - both may create catalog entries; I agree we need catalog
> entries so that auditing config can itself be audited.

Having functions to control the auditing would work, but it's not
exactly the ideal approach, imv, and the only reason it's being
discussed here is because it might be a way to allow an extension to
provide the auditing- not because it's actually a benefit to anyone.
However, if we have such functions in a contrib extension, I worry what
the pg_upgrade path is from that extension to an in-core solution.

Robert feels that's managable and perhaps he's right but we don't have
either the function-based design with custom reloptions nor a concrete
idea of what the in-core solution would be and so I'm skeptical that
we'd be able to easily provide a path from one to the other with
pg_upgrade.

> The rest of this argument seems to revolve around the idea that
> "in-core" and "extension"/contrib are different things. That is much
> more of a general point and one that is basically about development
> style.

The downside of extensions for this case is the inability to create and
integrate new catalog tables into the pg_catalog environment, and the
inability for extensions to extend the SQL grammar, as you mention.
I'd love to see both of those issues solved in a way which would allow
us to build the auditing system that I've been dreaming about as an
extension or "feature". I don't feel we should be holding off on
projects like auditing while we wait for that to happen though.

> But if you missed it before, I will say it again: we have a patch now
> and 2ndQuadrant has a limit on further funding. If you want to reject
> pgaudit and move on to something else, lets start discussing that
> design and consider who has the time (and implicitly the money) to
> make that happen. We can pick the best one for inclusion in 9.5 later
> in the cycle.

What I'd love to see is progress on both fronts, really. pgaudit is
great and it'd be good to have it for the older releases and for those
releases up until we get real auditing in PG- I just don't want to make
adding that in-PG-auditing later more difficult than it already is by
having to address upgrades from pgaudit. That's not an issue if pgaudit
lives outside of PG, but it could be an issue if it's in -contrib.

If we're willing to say that we won't worry about upgrades from pgaudit
to an in-core solution (or at least that the lack of such an upgrade
path won't prevent adding an in-core auditing system) then my concerns
about that would be addressed, but we don't get to change our minds
about that after pgaudit has been released as part of 9.5..

Thanks,

Stephen

At 2014-06-30 10:59:22 -0400, robertmhaas(at)gmail(dot)com wrote:
>
> That means we need some review of the patch for what it is, which
> there hasn't been much of, yet.

Regardless of the eventual fate of pgaudit, we'd certainly welcome some
review of the code.

-- Abhijit

At 2014-07-01 21:39:27 +0900, maumau307(at)gmail(dot)com wrote:
>
> Won't it be burden and a headache to maintain pgaudit code when it
> becomes obsolete in the near future?

Maybe it's a bit unfair to single out this statement to respond to,
because it seems at best tangential to your larger point, but:

If it were to really become obsolete (not sure about "the near future"),
it wouldn't need much maintenance. It already works about as well as it
ever will on older releases (e.g., we have no hopes of ever backporting
enough of event triggers to provide DDL deparsing in 9.3).

> I'm afraid they would be disappointed if PostgreSQL provides auditing
> functionality which does not conform to any real regulations like PCI
> DSS, NIST

I foresee lots of disappointment, then. I don't think even Stephen is
advocating NIST-compliance as the *baseline* for serious auditing in
core, just that we need a design that lets us get there sometime.

-- Abhijit

On 1 July 2014 18:32, Stephen Frost <sfrost(at)snowman(dot)net> wrote:

> Having functions to control the auditing would work, but it's not
> exactly the ideal approach, imv, and

What aspect is less than ideal?

> the only reason it's being
> discussed here is because it might be a way to allow an extension to
> provide the auditing- not because it's actually a benefit to anyone.

That is a false statement, as well as being a personal one. It's sad
to hear personal comments in this.

It seems strange to be advocating new grammar at a time when we are
actively reducing the size of it (see recent commits and long running
hackers discussions). Functions don't carry the same overhead, in fact
they cost nothing if you're not using them, which is the most
important point.

The right to execute functions can be delegated easily to any group
that wants access. There is no special benefit to SQL grammar on that
point.

> However, if we have such functions in a contrib extension, I worry what
> the pg_upgrade path is from that extension to an in-core solution.

Functions are already used heavily for many aspects of PostgreSQL.
http://www.postgresql.org/docs/devel/static/functions-admin.html

Presumably you don't advocate an "in core" solution to replace
pg_cancel_backend() etc?

My proposed route for making this "in-core" is simply to accept that
the extension is "in core". Auditing should, in my view, always be
optional, since not everyone needs it. Cryptographic functions aren't
in-core either and I'm guessing various security conscious
organizations will use them and be happy. How does pgaudit differ from
pgcrypto?

Given the tone of this discussion, I don't see it going anywhere
further anytime soon - that is good since there is no big rush.
pgaudit is a sincere attempt to add audit functionality to Postgres.
If you or anyone else wants to make a similarly sincere attempt to add
audit functionality to Postgres, lets see the design and its
connection to requirements.

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

* Abhijit Menon-Sen (ams(at)2ndQuadrant(dot)com) wrote:
> I foresee lots of disappointment, then. I don't think even Stephen is
> advocating NIST-compliance as the *baseline* for serious auditing in
> core, just that we need a design that lets us get there sometime.

Agreed. I'm not suggesting that we must be fully NIST/PCI-DSS/whatever
compliant before we add anything but rather that we have a plan for how
to get there. Note that the plan is needed primairly to ensure that we
don't end up in a situation that makes it difficult to move forward in
the future due to compatibility or upgrade issues.

Thanks,

Stephen

On Wed, Jul 2, 2014 at 5:19 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On 1 July 2014 18:32, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>> Having functions to control the auditing would work, but it's not
>> exactly the ideal approach, imv, and
>
> What aspect is less than ideal?
>
>> the only reason it's being
>> discussed here is because it might be a way to allow an extension to
>> provide the auditing- not because it's actually a benefit to anyone.
>
> That is a false statement, as well as being a personal one. It's sad
> to hear personal comments in this.

I am not sure that it was personal, but I agree it's false.

> Auditing should, in my view, always be
> optional, since not everyone needs it. Cryptographic functions aren't
> in-core either and I'm guessing various security conscious
> organizations will use them and be happy. How does pgaudit differ from
> pgcrypto?

+1.

> Given the tone of this discussion, I don't see it going anywhere
> further anytime soon - that is good since there is no big rush.
> pgaudit is a sincere attempt to add audit functionality to Postgres.
> If you or anyone else wants to make a similarly sincere attempt to add
> audit functionality to Postgres, lets see the design and its
> connection to requirements.

Agreed all around.

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

Simon,

* Simon Riggs (simon(at)2ndQuadrant(dot)com) wrote:
> On 1 July 2014 18:32, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > Having functions to control the auditing would work, but it's not
> > exactly the ideal approach, imv, and
>
> What aspect is less than ideal?

I certainly don't like passing table names to functions, just to point
out one item. It's necessary in cases where we want to programatically
get access to information (eg: pg_total_relation_size(),
has_*_privilege, etc). I do not see any functions today which take a
'name' type argument (or, at a quick look, any of the other ones) and
update a catalog object- and I don't think that's a mistake. Reading
further, I take it that you are advocating pgaudit have functions along
these lines:

-- Enable auditing for a table
pgaudit_add_audit_to_table(name);

-- Audit only a given role's actions on the table
pgaudit_add_audit_to_table(name,role);

-- Audit specific actions of a role on a table
pgaudit_add_audit_to_table(name,role,aclitem);

whereby this information would end up stored somehow through reloptions.
I'd be happy to be incorrect here as that's not a design that I like,
but if that isn't correct, getting feedback as to what the future plan
*is* would be great.

> > the only reason it's being
> > discussed here is because it might be a way to allow an extension to
> > provide the auditing- not because it's actually a benefit to anyone.
>
> That is a false statement, as well as being a personal one. It's sad
> to hear personal comments in this.

Simon, it wasn't intended as a personal attack. My frustration with
this thread lead me to overstate it but there has been very little
discussion about why functions are the *right* solution for controlling
auditing. The arguments for it today have been, to try and summarize:

- Keeps the grammar small

While this is a general benefit, I don't view it as a reason for using
functions instead of new grammar when it comes to modifying objects in
the system.

- Not everyone will want auditing

I fully support this- not everyone wants column level privileges
either though, or JSON in their database. I also don't feel this is
accurate- we get a lot of complaints about the lack of flexibility in
our logging system and my hope is that an auditing system will help to
address that. The two are not identical but I expect them to share a
lot of the infrastructure.

- Permissions can be set up on functions

I admit that I had not considered this, but even so- that's very
coarse- you can either execute the function or not. We might be able
to start with this but I'd expect us to need more control very
quickly- auditor A should be able to control the auditing for tables
in the 'low security' schema, while auditor B should be able to
control the auditing for tables in both the 'low security' and 'high
security' schemas. In the end, I don't see this as being a workable
solution- and then what? To add that control, we add more functions
to allow the admin to control who can define auditing for what, and
then have to implement the permissions handling inside of the audit
configuration functions, and then relax the permissions on those
functions? Perhaps you see a better approach and I'm just missing it,
but if so, it'd be great if you could outline that.

> It seems strange to be advocating new grammar at a time when we are
> actively reducing the size of it (see recent commits and long running
> hackers discussions). Functions don't carry the same overhead, in fact
> they cost nothing if you're not using them, which is the most
> important point.

I do not agree that new grammar for auditing should be off the table
because it adds to the grammar. I agree that we don't want to add new
grammar without good justification for it and further agree that we
should be looking at opportunities to keep the grammar small. I view
auditing as similar to the GRANT system though and while we could reduce
our grammar by moving everything done via GRANT into functions, I don't
feel that's a good solution.

> The right to execute functions can be delegated easily to any group
> that wants access. There is no special benefit to SQL grammar on that
> point.

Addressed above.

> > However, if we have such functions in a contrib extension, I worry what
> > the pg_upgrade path is from that extension to an in-core solution.
>
> Functions are already used heavily for many aspects of PostgreSQL.
> http://www.postgresql.org/docs/devel/static/functions-admin.html
>
> Presumably you don't advocate an "in core" solution to replace
> pg_cancel_backend() etc?

I don't view cancelling a backend as being part of the grammar's
responsibility. One of the grammar's roles is to define the objects
which exist in the system and to further support modification of those
objects. We don't have a function called 'create_table', for example.

> My proposed route for making this "in-core" is simply to accept that
> the extension is "in core". Auditing should, in my view, always be
> optional, since not everyone needs it.

Including auditing in the grammar does not require anyone to use it and
therefore it is optional. Column-level privileges are a good example of
a similar optional capability which exists in the grammar- there's no
requirement that it be part of the grammar, there's nothing preventing a
user from using views to emulate it, or simply accepting that table
level privileges are the only option and designing around that.

> Cryptographic functions aren't
> in-core either and I'm guessing various security conscious
> organizations will use them and be happy. How does pgaudit differ from
> pgcrypto?

pgcrypto does not include references or configuration associated with
the objects in the system. Are you suggesting that we will enhance
pgcrypto to support encrypting columns- such that there is a custom
attoption entry for a column which says "this column is encrypted with
pgcrypto" and further that selects against that table would have the
encrypted data from that column flow through a hook that decrypts it
using a key stored in a GUC or previously set up during the session?

While an interesting tangent, I'd be curious to see what that would buy
us over recommending views which happen to include pgcrypto functions
and further I'd like to take time to review what other RDBMS's do in
this area. In the end, I feel like I'd advocate extending the catalog
and providing grammar for setting up column or table encryption rather
than having the encryption configuration defined through functions.

> Given the tone of this discussion, I don't see it going anywhere
> further anytime soon - that is good since there is no big rush.

I'd like it to move further as I'd really like to see auditing happen
for PG sooner rather than later. :(

> pgaudit is a sincere attempt to add audit functionality to Postgres.

I'm very appreciative of it and expect to be using it in the future for
existing PG installations as it's far better than anything else
currently available.

> If you or anyone else wants to make a similarly sincere attempt to add
> audit functionality to Postgres, lets see the design and its
> connection to requirements.

I've made a few attempts to outline the requirements as I see them and
have also pointed to specific requirements documents. I'm a bit
occupied with RLS currently to flesh out a full design for auditing, but
it's definitely on my list of things to look at this fall. That said, I
do feel it's quite reasonable to ask about the future plans and designs
of a new extension which is being suggested for inclusion in PG.

Thanks,

Stephen

On Thu, Jun 26, 2014 at 09:59:59AM -0400, Stephen Frost wrote:
> Simon,
>
> * Simon Riggs (simon(at)2ndQuadrant(dot)com) wrote:
> > "Which tables are audited" would be available via the reloptions
> > field.
>
> RLS could be implemented through reloptions too. Would it be useful to
> some people? Likely. Would it satisfy the users who, today, are
> actually asking for that feature? No (or at least, not the ones that
> I've talked with). We could expand quite a few things to work through
> reloptions but I don't see it as a particularly good design for complex
> subsystems, of which auditing is absolutely one of those.

I saw many mentions of pg_upgrade in this old thread. I think the focus
should be in pg_dump, which is where the SQL syntax or custom reloptions
would be dumped and restored. pg_upgrade will just use that
functionality. In summary, I don't think there is anything
pg_upgrade-specific here, but rather the issue of how this information will
be dumped and restored, regardless of whether a major upgrade is taking
place.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On Tue, Jul 29, 2014 at 09:08:38AM -0400, Bruce Momjian wrote:
> On Thu, Jun 26, 2014 at 09:59:59AM -0400, Stephen Frost wrote:
> > Simon,
> >
> > * Simon Riggs (simon(at)2ndQuadrant(dot)com) wrote:
> > > "Which tables are audited" would be available via the reloptions
> > > field.
> >
> > RLS could be implemented through reloptions too. Would it be useful to
> > some people? Likely. Would it satisfy the users who, today, are
> > actually asking for that feature? No (or at least, not the ones that
> > I've talked with). We could expand quite a few things to work through
> > reloptions but I don't see it as a particularly good design for complex
> > subsystems, of which auditing is absolutely one of those.
>
> I saw many mentions of pg_upgrade in this old thread. I think the focus
> should be in pg_dump, which is where the SQL syntax or custom reloptions
> would be dumped and restored. pg_upgrade will just use that
> functionality. In summary, I don't think there is anything
> pg_upgrade-specific here, but rather the issue of how this information will
> be dumped and restored, regardless of whether a major upgrade is taking
> place.

Actually, thinking more, Stephen Frost mentioned that the auditing
system has to modify database _state_, and dumping/restoring the state
of an extension might be tricky.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

* Bruce Momjian (bruce(at)momjian(dot)us) wrote:
> Actually, thinking more, Stephen Frost mentioned that the auditing
> system has to modify database _state_, and dumping/restoring the state
> of an extension might be tricky.

This is really true of any extension which wants to attach information
or track things associated with roles or other database objects. What
I'd like to avoid is having an extension which does so through an extra
table or through reloptions or one of the other approaches which exists
in contrib and which implements a capability we're looking at adding to
core as we'd then have to figure out how to make pg_upgrade deal with
moving such a configuration from the extension's table or from
reloptions into SQL commands which work against the version of PG which
implements the capability in core.

Using auditing as an example, consider this scenario:

pgaudit grows a table which is used to say "only audit roles X, Y, Z"
(or specific tables, or connections from certain IPs, etc).

A patch for PG 10.1 is proposed which adds the ability to enable
auditing for specific roles.

My concern is:

pg_upgrade then has to detect, understand, and implement a migration
path from 10.0-with-pgaudit to 10.1-in-core-auditing.

or

The PG 10.1 patch has to ensure that it doesn't break, harm, or
interfere with what pgaudit is doing in its per-role auditing.

or

The PG 10.1 patch is bounced because what pgaudit does is considered
"good enough" and it's already in contrib (though I don't believe this
will ever be the case while pgaudit exists as an extension- see
below).

From my perspective, it's pretty clear that we don't have any good
way for any extension, today, to have metadata properly associated
with database objects- such that renames, upgrades, dependency
issues, etc, are properly addressed and handled; nor are extensions
able to extend the grammar; and there is a concern that extensions may
not always be properly loaded, a serious concern when the role of that
extension is auditing.

Hope that helps.

Thanks,

Stephen

On Wed, Jul 30, 2014 at 02:29:47PM -0400, Stephen Frost wrote:
> Using auditing as an example, consider this scenario:
>
> pgaudit grows a table which is used to say "only audit roles X, Y, Z"
> (or specific tables, or connections from certain IPs, etc).
>
> A patch for PG 10.1 is proposed which adds the ability to enable
> auditing for specific roles.
>
> My concern is:
>
> pg_upgrade then has to detect, understand, and implement a migration
> path from 10.0-with-pgaudit to 10.1-in-core-auditing.
>
> or
>
> The PG 10.1 patch has to ensure that it doesn't break, harm, or
> interfere with what pgaudit is doing in its per-role auditing.
>
> or
>
> The PG 10.1 patch is bounced because what pgaudit does is considered
> "good enough" and it's already in contrib (though I don't believe this
> will ever be the case while pgaudit exists as an extension- see
> below).

I think someone could write a Perl script that you run before the
upgrade to create SQL commands to restore the audit settings.

> From my perspective, it's pretty clear that we don't have any good
> way for any extension, today, to have metadata properly associated
> with database objects- such that renames, upgrades, dependency
> issues, etc, are properly addressed and handled; nor are extensions
> able to extend the grammar; and there is a concern that extensions may
> not always be properly loaded, a serious concern when the role of that
> extension is auditing.

That is the larger issue --- I can't think of any extension that has to
store state like that.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

Stephen Frost wrote:
> * Bruce Momjian (bruce(at)momjian(dot)us) wrote:
> > Actually, thinking more, Stephen Frost mentioned that the auditing
> > system has to modify database _state_, and dumping/restoring the state
> > of an extension might be tricky.
>
> This is really true of any extension which wants to attach information
> or track things associated with roles or other database objects. What
> I'd like to avoid is having an extension which does so through an extra
> table or through reloptions or one of the other approaches which exists
> in contrib and which implements a capability we're looking at adding to
> core as we'd then have to figure out how to make pg_upgrade deal with
> moving such a configuration from the extension's table or from
> reloptions into SQL commands which work against the version of PG which
> implements the capability in core.

I think you're making too much of this upgrade issue. Consider
autovacuum's history: in its initial form, we made it configurable per
table by asking users to insret rows in the pg_autovacuum catalog. It
was a pretty horrible user interface, it was very error prone, it didn't
survive dump/restore cycles (And autovacuum was a core feature, not an
extension). We were able to iron out a lot of issues during those
initial two releases with the accumulated experience. We eventually
replaced the interface with reloptions (ALTER TABLE SET), which is what
we have today. We didn't add pg_upgrade code to migrate settings from
the old way to the new one; users who wished to do this were responsible
for handling it for themselves.

Sure, we're a more mature project now and our standards are higher. So
I think we should provide documented procedures to upgrade from pgaudit
to integrated feature; but that should suffice.

> My concern is:
>
> pg_upgrade then has to detect, understand, and implement a migration
> path from 10.0-with-pgaudit to 10.1-in-core-auditing.

Or we can just ask users to run commands X, Y, Z to migrate their old
configuration to the new integrated thing. No need for pg_upgrade
procedures.

> The PG 10.1 patch has to ensure that it doesn't break, harm, or
> interfere with what pgaudit is doing in its per-role auditing.

Or pgaudit goes away, having fulfilled its mission in life which was to
serve as audit mechanism for the older releases.

> The PG 10.1 patch is bounced because what pgaudit does is considered
> "good enough" and it's already in contrib (though I don't believe this
> will ever be the case while pgaudit exists as an extension- see
> below).

I don't fear this will happen, either, so why mention it?

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

* Bruce Momjian (bruce(at)momjian(dot)us) wrote:
> I think someone could write a Perl script that you run before the
> upgrade to create SQL commands to restore the audit settings.

Is pg_upgrade going to detect that pgaudit is installed and know to run
this perl script? I don't doubt that it'd be possible to have such a
script, but there's an open question (in my mind anyway..) as to exactly
what would be acceptable to the community for such a migration.

What I wish to avoid is the situation which exists around hstore.
Perhaps I've got this wrong, but my recollection of the discussion
leads me to believe that we can't have hstore in core becasue there's
no simple migration path from an hstore-enabled installation to one
where hstore is in core instead. I'm not sure that a perl script would
be sufficient in that case (the issue involves the remapping of the OIDs
for the functions and operators installed, no?), but if it was, would
that be an acceptable way to deal with the issues?

> That is the larger issue --- I can't think of any extension that has to
> store state like that.

There's quite a few which would *like* to store such state and we see
requests regularly to use reloptions or similar to do so, but that's
really not a good solution as there isn't any way to prevent conflicts
or to have much more than simple tags. Fixing that has been discussed
also, but it's more than just a namespace issue- we'd also need to
provide a way to call into the extension when changes are made, and we'd
need to provide a similar capability for all of the objects which we
manage in the catalog. Implemneting this for just tables or just roles
or, worse, implementing it in a different way for each and every object
type we have, would really make it painful to write extensions which use
that facility.

Thanks,

Stephen

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> * Bruce Momjian (bruce(at)momjian(dot)us) wrote:
>> Actually, thinking more, Stephen Frost mentioned that the auditing
>> system has to modify database _state_, and dumping/restoring the state
>> of an extension might be tricky.

> This is really true of any extension which wants to attach information
> or track things associated with roles or other database objects. What
> I'd like to avoid is having an extension which does so through an extra
> table or through reloptions or one of the other approaches which exists
> in contrib and which implements a capability we're looking at adding to
> core

We have core code that uses reloptions --- autovacuum for instance ---
so I'm not exactly clear on why that's so unacceptable for this.

If the concern is that the required metadata is going to change over time,
I'd suggest that maybe an extension is the right place for it,
permanently. We have some infrastructure for extension version upgrades,
which could cope with metadata changes. There's not nearly as much
provision for changes of core state.

regards, tom lane

Alvaro,

* Alvaro Herrera (alvherre(at)2ndquadrant(dot)com) wrote:
> Stephen Frost wrote:
> > This is really true of any extension which wants to attach information
> > or track things associated with roles or other database objects. What
> > I'd like to avoid is having an extension which does so through an extra
> > table or through reloptions or one of the other approaches which exists
> > in contrib and which implements a capability we're looking at adding to
> > core as we'd then have to figure out how to make pg_upgrade deal with
> > moving such a configuration from the extension's table or from
> > reloptions into SQL commands which work against the version of PG which
> > implements the capability in core.
>
> I think you're making too much of this upgrade issue. Consider
> autovacuum's history: in its initial form, we made it configurable per
> table by asking users to insret rows in the pg_autovacuum catalog. It
> was a pretty horrible user interface, it was very error prone, it didn't
> survive dump/restore cycles (And autovacuum was a core feature, not an
> extension).

I'm amazed that we didn't support those with dump/restore.. I'd be very
surprised if we were to accept that today.

> We were able to iron out a lot of issues during those
> initial two releases with the accumulated experience. We eventually
> replaced the interface with reloptions (ALTER TABLE SET), which is what
> we have today.

> We didn't add pg_upgrade code to migrate settings from
> the old way to the new one; users who wished to do this were responsible
> for handling it for themselves.

We didn't have pg_upgrade then though, so I'm not sure that this is
really relevant..? We've now committed to supporting pg_upgrade. In
any case, I'm not trying to simply throw up roadblocks or come up with
unlikely scenarios- I'm looking at the existing example in hstore and
considering how that's a contrib-to-core migration which has never been
able to happen, and my recollection is that the reasons have largely
been technical migration/upgrade issues.

> Sure, we're a more mature project now and our standards are higher. So
> I think we should provide documented procedures to upgrade from pgaudit
> to integrated feature; but that should suffice.

I have a very hard time believing that this would be acceptable.. If it
is, and we're all willing to agree that it's acceptable to break an
existing installation where the user runs 'pg_upgrade' and has pgaudit
installed (but they didn't follow the prescribed procedures), then I'd
like to understand why we're not alright with doing that for hstore..
(If we are- then I'd like to propose that we move hstore into core..)

> > My concern is:
> >
> > pg_upgrade then has to detect, understand, and implement a migration
> > path from 10.0-with-pgaudit to 10.1-in-core-auditing.
>
> Or we can just ask users to run commands X, Y, Z to migrate their old
> configuration to the new integrated thing. No need for pg_upgrade
> procedures.

See above.

> > The PG 10.1 patch has to ensure that it doesn't break, harm, or
> > interfere with what pgaudit is doing in its per-role auditing.
>
> Or pgaudit goes away, having fulfilled its mission in life which was to
> serve as audit mechanism for the older releases.

I'd be fine with that. I'll note that it doesn't make any sense to add
pgaudit to contrib if it's goal is to handle older-than-current (9.4)
releases, but I'm guessing you mean "prior-to-10.1", per my scenario.

> > The PG 10.1 patch is bounced because what pgaudit does is considered
> > "good enough" and it's already in contrib (though I don't believe this
> > will ever be the case while pgaudit exists as an extension- see
> > below).
>
> I don't fear this will happen, either, so why mention it?

Because I'm concerned it will?

Thanks,

Stephen

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> What I wish to avoid is the situation which exists around hstore.
> Perhaps I've got this wrong, but my recollection of the discussion
> leads me to believe that we can't have hstore in core becasue there's
> no simple migration path from an hstore-enabled installation to one
> where hstore is in core instead.

The issues around hstore have to do with how we'd get from userland
datatype OIDs to fixed-at-initdb-time datatype OIDs, in view of the fact
that said OIDs exist in user data on-disk (in arrays for instance). This
is pretty much completely unrelated to reloptions or other catalog data,
and it's not at all clear to me why an auditing extension would have
any such issue. If an auditing extension has any impact on the storage of
user data, what it's doing is hardly just auditing, eh?

regards, tom lane

On Wed, Jul 30, 2014 at 02:49:25PM -0400, Alvaro Herrera wrote:
> Stephen Frost wrote:
> > * Bruce Momjian (bruce(at)momjian(dot)us) wrote:
> > > Actually, thinking more, Stephen Frost mentioned that the auditing
> > > system has to modify database _state_, and dumping/restoring the state
> > > of an extension might be tricky.
> >
> > This is really true of any extension which wants to attach information
> > or track things associated with roles or other database objects. What
> > I'd like to avoid is having an extension which does so through an extra
> > table or through reloptions or one of the other approaches which exists
> > in contrib and which implements a capability we're looking at adding to
> > core as we'd then have to figure out how to make pg_upgrade deal with
> > moving such a configuration from the extension's table or from
> > reloptions into SQL commands which work against the version of PG which
> > implements the capability in core.
>
> I think you're making too much of this upgrade issue. Consider
> autovacuum's history: in its initial form, we made it configurable per
> table by asking users to insret rows in the pg_autovacuum catalog. It
> was a pretty horrible user interface, it was very error prone, it didn't
> survive dump/restore cycles (And autovacuum was a core feature, not an
> extension). We were able to iron out a lot of issues during those
> initial two releases with the accumulated experience. We eventually
> replaced the interface with reloptions (ALTER TABLE SET), which is what
> we have today. We didn't add pg_upgrade code to migrate settings from
> the old way to the new one; users who wished to do this were responsible
> for handling it for themselves.
>
> Sure, we're a more mature project now and our standards are higher. So
> I think we should provide documented procedures to upgrade from pgaudit
> to integrated feature; but that should suffice.

Agreed. It is one thing to have gigabytes of data in hstore, and
another to have per-object audit settings.

pg_upgrade --check could detect pg_audit and suggest the user run the
Perl script before upgrading, and run the SQL commands through psql
after.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On Wed, Jul 30, 2014 at 2:49 PM, Alvaro Herrera
<alvherre(at)2ndquadrant(dot)com> wrote:
> I think you're making too much of this upgrade issue.

+1.

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

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> On Wed, Jul 30, 2014 at 2:49 PM, Alvaro Herrera
> <alvherre(at)2ndquadrant(dot)com> wrote:
> > I think you're making too much of this upgrade issue.
>
> +1.

Alright- if everyone feels that there won't be any migration issues then
I'll drop that concern.

Thanks,

Stephen

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > * Bruce Momjian (bruce(at)momjian(dot)us) wrote:
> >> Actually, thinking more, Stephen Frost mentioned that the auditing
> >> system has to modify database _state_, and dumping/restoring the state
> >> of an extension might be tricky.
>
> > This is really true of any extension which wants to attach information
> > or track things associated with roles or other database objects. What
> > I'd like to avoid is having an extension which does so through an extra
> > table or through reloptions or one of the other approaches which exists
> > in contrib and which implements a capability we're looking at adding to
> > core
>
> We have core code that uses reloptions --- autovacuum for instance ---
> so I'm not exactly clear on why that's so unacceptable for this.

There was a pretty good thread regarding reloptions and making it so
extensions could use them which seemed to end up with a proposal to turn
'security labels' into a more generic metadata capability. Using that
kind of a mechanism would at least address one of my concerns about
using reloptions (specifically that they're specific to relations and
don't account for the other objects in the system). Unfortunately, the
flexibility desired for auditing is more than just "all actions of this
role" or "all actions on this table" but also "actions of this role on
this table", which doesn't fit as well.

Still, if we're fine with perl scripts and similar hacks to facilitate
upgrading from pgaudit to an in-core solution, then I won't object to
including pgaudit until we've got that in-core capability.

> If the concern is that the required metadata is going to change over time,
> I'd suggest that maybe an extension is the right place for it,
> permanently. We have some infrastructure for extension version upgrades,
> which could cope with metadata changes. There's not nearly as much
> provision for changes of core state.

I don't follow this at all- we already deal with changes in core between
major versions via pg_dump/pg_upgrade and I have every expectation that
we'll continue to lean on that into the future.

Thanks,

Stephen

On 31 July 2014 22:34, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
>> Stephen Frost <sfrost(at)snowman(dot)net> writes:
>> > * Bruce Momjian (bruce(at)momjian(dot)us) wrote:
>> >> Actually, thinking more, Stephen Frost mentioned that the auditing
>> >> system has to modify database _state_, and dumping/restoring the state
>> >> of an extension might be tricky.
>>
>> > This is really true of any extension which wants to attach information
>> > or track things associated with roles or other database objects. What
>> > I'd like to avoid is having an extension which does so through an extra
>> > table or through reloptions or one of the other approaches which exists
>> > in contrib and which implements a capability we're looking at adding to
>> > core
>>
>> We have core code that uses reloptions --- autovacuum for instance ---
>> so I'm not exactly clear on why that's so unacceptable for this.
>
> There was a pretty good thread regarding reloptions and making it so
> extensions could use them which seemed to end up with a proposal to turn
> 'security labels' into a more generic metadata capability. Using that
> kind of a mechanism would at least address one of my concerns about
> using reloptions (specifically that they're specific to relations and
> don't account for the other objects in the system). Unfortunately, the
> flexibility desired for auditing is more than just "all actions of this
> role" or "all actions on this table" but also "actions of this role on
> this table", which doesn't fit as well.

Yes, there is a requirement, in some cases, for per role/relation
metadata. Grant and ACLs are a good example.

I spoke with Robert about a year ago that the patch he was most proud
of was the reloptions abstraction. Whatever we do in the future,
keeping metadata in a slightly more abstract form is very useful.

I hope we can get pgAudit in as a module for 9.5. I also hope that it
will stimulate the requirements/funding of further work in this area,
rather than squash it. My feeling is we have more examples of feature
sets that grow over time (replication, view handling, hstore/JSONB
etc) than we have examples of things languishing in need of attention
(partitioning).

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

Simon,

* Simon Riggs (simon(at)2ndquadrant(dot)com) wrote:
> On 31 July 2014 22:34, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > There was a pretty good thread regarding reloptions and making it so
> > extensions could use them which seemed to end up with a proposal to turn
> > 'security labels' into a more generic metadata capability. Using that
> > kind of a mechanism would at least address one of my concerns about
> > using reloptions (specifically that they're specific to relations and
> > don't account for the other objects in the system). Unfortunately, the
> > flexibility desired for auditing is more than just "all actions of this
> > role" or "all actions on this table" but also "actions of this role on
> > this table", which doesn't fit as well.
>
> Yes, there is a requirement, in some cases, for per role/relation
> metadata. Grant and ACLs are a good example.
>
> I spoke with Robert about a year ago that the patch he was most proud
> of was the reloptions abstraction. Whatever we do in the future,
> keeping metadata in a slightly more abstract form is very useful.

Agreed.

> I hope we can get pgAudit in as a module for 9.5. I also hope that it
> will stimulate the requirements/funding of further work in this area,
> rather than squash it. My feeling is we have more examples of feature
> sets that grow over time (replication, view handling, hstore/JSONB
> etc) than we have examples of things languishing in need of attention
> (partitioning).

I've come around to this also (which I think I commented on
previously..), as it sounds like the upgrade concerns I was worried
about can be addressed, and having pgAudit would certainly be better
than not having any kind of auditing support.

Thanks,

Stephen

On Tue, Oct 7, 2014 at 12:24 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> I spoke with Robert about a year ago that the patch he was most proud
> of was the reloptions abstraction. Whatever we do in the future,
> keeping metadata in a slightly more abstract form is very useful.

To slightly correct the record, I believe I was referring to the
generic EXPLAIN options syntax, since copied into a lot of other
places and used to unblock various patches that would have otherwise
died on the vine.

The reloptions stuff was Alvaro's work.

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

On Tue, Oct 7, 2014 at 1:24 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>
> On 31 July 2014 22:34, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> >> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> >> > * Bruce Momjian (bruce(at)momjian(dot)us) wrote:
> >> >> Actually, thinking more, Stephen Frost mentioned that the auditing
> >> >> system has to modify database _state_, and dumping/restoring the
state
> >> >> of an extension might be tricky.
> >>
> >> > This is really true of any extension which wants to attach
information
> >> > or track things associated with roles or other database objects.
What
> >> > I'd like to avoid is having an extension which does so through an
extra
> >> > table or through reloptions or one of the other approaches which
exists
> >> > in contrib and which implements a capability we're looking at adding
to
> >> > core
> >>
> >> We have core code that uses reloptions --- autovacuum for instance ---
> >> so I'm not exactly clear on why that's so unacceptable for this.
> >
> > There was a pretty good thread regarding reloptions and making it so
> > extensions could use them which seemed to end up with a proposal to turn
> > 'security labels' into a more generic metadata capability. Using that
> > kind of a mechanism would at least address one of my concerns about
> > using reloptions (specifically that they're specific to relations and
> > don't account for the other objects in the system). Unfortunately, the
> > flexibility desired for auditing is more than just "all actions of this
> > role" or "all actions on this table" but also "actions of this role on
> > this table", which doesn't fit as well.
>
> Yes, there is a requirement, in some cases, for per role/relation
> metadata. Grant and ACLs are a good example.
>
> I spoke with Robert about a year ago that the patch he was most proud
> of was the reloptions abstraction. Whatever we do in the future,
> keeping metadata in a slightly more abstract form is very useful.
>

When we discussed about the rejected patch "store-custom-reloptions" I
pointed my thoughts about it in
http://www.postgresql.org/message-id/CAFcNs+p+2OA2fg7o-8KWmckazjAYWue6mVNnUdpuRpT0PZ8D_g@mail.gmail.com

We can think in a mechanism to create "properties / options" and assign it
to objects (table, index, column, schema, ...) like COMMENT does.

A quickly thought:

CREATE OPTION [ IF NOT EXISTS ] name
VALIDATOR valfunction
[ DEFAULT value ];

ALTER TABLE name
SET OPTION optname { TO | = } { value | 'value' | DEFAULT };

It's just a simple thought of course. We must think better about the syntax
and purposes.

> I hope we can get pgAudit in as a module for 9.5. I also hope that it
> will stimulate the requirements/funding of further work in this area,
> rather than squash it. My feeling is we have more examples of feature
> sets that grow over time (replication, view handling, hstore/JSONB
> etc) than we have examples of things languishing in need of attention
> (partitioning).
>

+1

Regards.

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello

On 10/07/2014 09:44 AM, Fabrízio de Royes Mello wrote:
> We can think in a mechanism to create "properties / options" and assign it
> to objects (table, index, column, schema, ...) like COMMENT does.
>
> A quickly thought:
>
> CREATE OPTION [ IF NOT EXISTS ] name
> VALIDATOR valfunction
> [ DEFAULT value ];
>
> ALTER TABLE name
> SET OPTION optname { TO | = } { value | 'value' | DEFAULT };
>
> It's just a simple thought of course. We must think better about the syntax
> and purposes.

If we're going to do this (and it seems like a good idea), we really,
really, really need to include some general system views which expose
the options in a user-friendly format (like columns, JSON or an array).
It's already very hard for users to get information about what
reloptions have been set.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Wed, Oct 8, 2014 at 12:36 AM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
>
> On 10/07/2014 09:44 AM, Fabrízio de Royes Mello wrote:
> > We can think in a mechanism to create "properties / options" and assign
it
> > to objects (table, index, column, schema, ...) like COMMENT does.
> >
> > A quickly thought:
> >
> > CREATE OPTION [ IF NOT EXISTS ] name
> > VALIDATOR valfunction
> > [ DEFAULT value ];
> >
> > ALTER TABLE name
> > SET OPTION optname { TO | = } { value | 'value' | DEFAULT };
> >
> > It's just a simple thought of course. We must think better about the
syntax
> > and purposes.
>
> If we're going to do this (and it seems like a good idea), we really,
> really, really need to include some general system views which expose
> the options in a user-friendly format (like columns, JSON or an array).
> It's already very hard for users to get information about what
> reloptions have been set.
>

Maybe into "information_schema" ??

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello

On Wed, Oct 08, 2014 at 12:41:46AM -0300, Fabrízio de Royes Mello wrote:
> On Wed, Oct 8, 2014 at 12:36 AM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> >
> > On 10/07/2014 09:44 AM, Fabrízio de Royes Mello wrote:
> > > We can think in a mechanism to create "properties / options" and assign
> it
> > > to objects (table, index, column, schema, ...) like COMMENT does.
> > >
> > > A quickly thought:
> > >
> > > CREATE OPTION [ IF NOT EXISTS ] name
> > > VALIDATOR valfunction
> > > [ DEFAULT value ];
> > >
> > > ALTER TABLE name
> > > SET OPTION optname { TO | = } { value | 'value' | DEFAULT };
> > >
> > > It's just a simple thought of course. We must think better about the
> syntax
> > > and purposes.
> >
> > If we're going to do this (and it seems like a good idea), we really,
> > really, really need to include some general system views which expose
> > the options in a user-friendly format (like columns, JSON or an array).
> > It's already very hard for users to get information about what
> > reloptions have been set.
> >
>
> Maybe into "information_schema" ??

Not there. The information schema is defined pretty precisely in the
SQL standard and contains only some of this kind of information.

pg_catalog seems like a much more appropriate space for
PostgreSQL-specific settings.

Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

* Fabrízio de Royes Mello (fabriziomello(at)gmail(dot)com) wrote:
> On Tue, Oct 7, 2014 at 1:24 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> > I hope we can get pgAudit in as a module for 9.5. I also hope that it
> > will stimulate the requirements/funding of further work in this area,
> > rather than squash it. My feeling is we have more examples of feature
> > sets that grow over time (replication, view handling, hstore/JSONB
> > etc) than we have examples of things languishing in need of attention
> > (partitioning).
>
> +1

To this point, specifically, I'll volunteer to find time in Novemeber to
review pgAudit for inclusion as a contrib module. I feel a bit
responsible for it not being properly reviewed earlier due to my upgrade
and similar concerns.

Perhaps the latest version should be posted and added to the commitfest
for 2014-10 and I'll put myself down as a reviewer..? I don't see it
there now. I don't mean to be dismissive by suggesting it be added to
the commitfest- I honestly don't see myself having time before November
given the other things I'm involved in right now and pgConf.eu happening
in a few weeks.

Of course, if someone else has time to review, please do.

Thanks,

Stephen

On 14/10/09 7:06, Stephen Frost wrote:
> * Fabrízio de Royes Mello (fabriziomello(at)gmail(dot)com) wrote:
>> On Tue, Oct 7, 2014 at 1:24 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>>> I hope we can get pgAudit in as a module for 9.5. I also hope that it
>>> will stimulate the requirements/funding of further work in this area,
>>> rather than squash it. My feeling is we have more examples of feature
>>> sets that grow over time (replication, view handling, hstore/JSONB
>>> etc) than we have examples of things languishing in need of attention
>>> (partitioning).
>>
>> +1
>
> To this point, specifically, I'll volunteer to find time in Novemeber to
> review pgAudit for inclusion as a contrib module. I feel a bit
> responsible for it not being properly reviewed earlier due to my upgrade
> and similar concerns.
>
> Perhaps the latest version should be posted and added to the commitfest
> for 2014-10 and I'll put myself down as a reviewer..? I don't see it
> there now. I don't mean to be dismissive by suggesting it be added to
> the commitfest- I honestly don't see myself having time before November
> given the other things I'm involved in right now and pgConf.eu happening
> in a few weeks.

Thanks :) We're updating pgAudit for submission this for the upcoming commitfest,
it will be added within the next few days.

Regards

Ian Barwick

--
Ian Barwick http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Wed, Oct 8, 2014 at 2:55 AM, David Fetter <david(at)fetter(dot)org> wrote:
>
> On Wed, Oct 08, 2014 at 12:41:46AM -0300, Fabrízio de Royes Mello wrote:
> > On Wed, Oct 8, 2014 at 12:36 AM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> > >
> > > On 10/07/2014 09:44 AM, Fabrízio de Royes Mello wrote:
> > > > We can think in a mechanism to create "properties / options" and
assign
> > it
> > > > to objects (table, index, column, schema, ...) like COMMENT does.
> > > >
> > > > A quickly thought:
> > > >
> > > > CREATE OPTION [ IF NOT EXISTS ] name
> > > > VALIDATOR valfunction
> > > > [ DEFAULT value ];
> > > >
> > > > ALTER TABLE name
> > > > SET OPTION optname { TO | = } { value | 'value' | DEFAULT };
> > > >
> > > > It's just a simple thought of course. We must think better about the
> > syntax
> > > > and purposes.
> > >
> > > If we're going to do this (and it seems like a good idea), we really,
> > > really, really need to include some general system views which expose
> > > the options in a user-friendly format (like columns, JSON or an
array).
> > > It's already very hard for users to get information about what
> > > reloptions have been set.
> > >
> >
> > Maybe into "information_schema" ??
>
> Not there. The information schema is defined pretty precisely in the
> SQL standard and contains only some of this kind of information.
>

You're absolutely right.. thanks...

> pg_catalog seems like a much more appropriate space for
> PostgreSQL-specific settings.
>

We can add views and some functions to get this info too.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello

From: "Simon Riggs" <simon(at)2ndquadrant(dot)com>
> I hope we can get pgAudit in as a module for 9.5. I also hope that it
> will stimulate the requirements/funding of further work in this area,
> rather than squash it. My feeling is we have more examples of feature
> sets that grow over time (replication, view handling, hstore/JSONB
> etc) than we have examples of things languishing in need of attention
> (partitioning).

I'm hoping PostgreSQL will have an audit trail feature. I'd like to support
your work by reviewing and testing, although I'm not sure I can fully
understand the code soon.

Regards
MauMau

Hi.

As before, the pgaudit code is at https://github.com/2ndQuadrant/pgaudit
I did a quick round of testing to make sure things still work.

I'll re-add it to the next CF now.

-- Abhijit

At 2014-10-14 18:05:00 +0530, ams(at)2ndQuadrant(dot)com wrote:
>
> As before, the pgaudit code is at
> https://github.com/2ndQuadrant/pgaudit

Sorry, I forgot to attach the tarball.

-- Abhijit

Abhijit,

* Abhijit Menon-Sen (ams(at)2ndQuadrant(dot)com) wrote:
> As before, the pgaudit code is at https://github.com/2ndQuadrant/pgaudit
> I did a quick round of testing to make sure things still work.

Thanks!

I had a very interesting discussion about auditing rules and the need to
limit what gets audited by user, table, column, policy, etc recently and
an idea came up (not my own) about how to support that granularity
without having to modify the existing PG catalogs or use GUCs or
reloptions, etc. It goes something like this-

Create an 'audit' role.

Every command run by roles which are granted to the 'audit' role are
audited.

Every 'select' against tables which the 'audit' role has 'select' rights
on are audited. Similairly for every insert, update, delete.

Every 'select' against columns of tables which the 'audit' role has
'select' rights on are audited- and only those columns are logged.
Similairly for every insert, update, delete.

etc, etc, throughout the various objects which can have permissions.

We don't currently have more granular permissions for roles (it's
"all-or-nothing" currently regarding role membership) and so if we want
to support things like "audit all DML for this role" we need multiple
roles, eg:

Create an 'audit_rw' role. DML commands run by roles granted to the
'audit_rw' role are audited. Similairly for 'audit_ro' or other
permutations.

The 'audit*' roles would need to be configured for pgAudit in some
fashion, but that's acceptable and is much more reasonable than having
an independent config file which has to keep track of the specific roles
or tables being audited.

The 'audit_ro' and 'audit_rw' roles lead to an interesting thought about
supporting something like which I want to mention here before I forget
about it, but probably should be a new thread if folks think it's
interesting:

GRANT role1 (SELECT) to role2;

Such that 'role2' would have role1's SELECT rights, but not role1's
INSERT, or UPDATE, or DELETE rights. There would be more complexity if
we want to support this for more than just normal relations, of course.

Thanks,

Stephen

Hello,

I had a quick look through the code and did some testing. Let me give you
some comments. I will proceed with checking if pgaudit can meet PCI DSS
requirements.

By the way, I'd like to use pgaudit with 9.2. Is it possible with a slight
modification of the code? If it is, what features of pgaudit would be
unavailable? Could you support 9.2?

(1)
The build failed with PostgreSQL 9.5, although I know the README mentions
that pgaudit supports 9.3 and 9.4. The cause is T_AlterTableSpaceMoveStmt
macro is not present in 9.5. I could build and use pgaudit by removing two
lines referring to that macro. I tested pgaudit only with 9.5.

(2)
I could confirm that DECLARE is audit-logged as SELECT and FETCH/CLOSE are
not. This is just as expected. Nice.

(3)
SELECT against a view generated two audit log lines, one for the view
itself, and the other for the underlying table. Is this intended? I'm not
saying that's wrong but just asking.

(4)
I'm afraid audit-logging DML statements on temporary tables will annoy
users, because temporary tables are not interesting. In addition, in
applications which use the same temporary table in multiple types of
transactions as follows, audit log entries for the DDL statement are also
annoying.

BEGIN;
CREATE TEMPORARY TABLE mytemp ... ON COMMIT DROP;
DML;
COMMIT;

The workaround is "CREATE TEMPORARY TABLE mytemp IF NOT EXIST ... ON COMMIT
DELETE ROWS". However, users probably don't (or can't) modify applications
just for audit logging.

(5)
This is related to (4). As somebody mentioned, I think the ability to
select target objects of audit logging is definitely necessary. Without
that, huge amount of audit logs would be generated for uninteresting
objects. That would also impact performance.

(6)
What's the performance impact of audit logging? I bet many users will ask
"about what percentage would the throughtput decrease by?" I'd like to know
the concrete example, say, pgbench and DBT-2.

(7)
In README, COPY FROM/TO should be added to read and write respectively.

(8)
The code looks good. However, I'm worried about the maintenance. How can
developers notice that pgaudit.c needs modification when they add a new SQL
statement? What keyword can they use to grep the source code to find
pgaudit.c?

Regards
MauMau

Thanks for the review.

On 16 October 2014 23:23, MauMau <maumau307(at)gmail(dot)com> wrote:

> (3)
> SELECT against a view generated two audit log lines, one for the view
> itself, and the other for the underlying table. Is this intended? I'm not
> saying that's wrong but just asking.

Intended

> (4)
> I'm afraid audit-logging DML statements on temporary tables will annoy
> users, because temporary tables are not interesting.

Agreed

> (5)
> This is related to (4). As somebody mentioned, I think the ability to
> select target objects of audit logging is definitely necessary. Without
> that, huge amount of audit logs would be generated for uninteresting
> objects. That would also impact performance.

Discussed and agreed already

> (6)
> What's the performance impact of audit logging? I bet many users will ask
> "about what percentage would the throughtput decrease by?" I'd like to know
> the concrete example, say, pgbench and DBT-2.

Don't know, but its not hugely relevant. If you need it, you need it.

> (8)
> The code looks good. However, I'm worried about the maintenance. How can
> developers notice that pgaudit.c needs modification when they add a new SQL
> statement? What keyword can they use to grep the source code to find
> pgaudit.c?

Suggestions welcome.

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

Hello,

As I said in the previous mail, I looked into the latest PCI DSS 3.0 to find
out whether and how pgaudit fulfills the requirements related to auditing.
I believe that even the initial version of pgaudit needs to have enough
functionalities to meet the requirements of some well-known standard, in
order to demonstrate that PostgreSQL provides a really useful auditing
feature. I chose PCI DSS because it seems popular worldwide.

Most requirement items are met, but some are not or I'm not sure if and/or
how. I only listed items which I'd like to ask for opinions. I'm sorry I
can't have good suggestions, but I hope this report will lead to good
discussion and better auditing feature we can boast of. Of course, I'll
suggest any idea when I think of something.

[requirement]
3.4 Render PAN unreadable anywhere it
is stored (including on portable digital
media, backup media, and in logs) by
using any of the following approaches:
...
3.4.d Examine a sample of audit logs to confirm that the PAN is
rendered unreadable or removed from the logs.

[my comment]
Do the audit log entries always hide the actual bind parameter values (with
$1, $2, etc.) if the application uses parameterized SQL statements? Should
we advise users to use parameterized statements in the pgaudit
documentation? (I think so)

[requirement]
10.2.2 Verify all actions taken by any individual with root or
administrative privileges are logged.
10.2.6 Verify the following are logged:
. Initialization of audit logs
. Stopping or pausing of audit logs.

[my comment]
The database superuser can hide his activity by "SET pgaudit.log = '';", but
this SET is audit-logged. Therefore, I think these requirements is met
because the fact that the superuser's suspicious behavior (hiding activity)
is recorded. Do you think this interpretation is valid?

[requirement]
10.2.3 Verify access to all audit trails is logged.

Malicious users often attempt to alter audit logs to
hide their actions, and a record of access allows
an organization to trace any inconsistencies or
potential tampering of the logs to an individual
account. Having access to logs identifying
changes, additions, and deletions can help retrace
steps made by unauthorized personnel.

[my comment]
I'm totally unsure how this can be fulfilled.

[requirement]
10.2.4 Verify invalid logical access attempts are logged.

Malicious individuals will often perform multiple
access attempts on targeted systems. Multiple
invalid login attempts may be an indication of an
unauthorized user’s attempts to “brute force” or
guess a password.

[my comment]
Login attempts also need to be audit-logged to meet this requirement.

[requirement]
10.2.5.a Verify use of identification and authentication
mechanisms is logged.

Without knowing who was logged on at the time of
an incident, it is impossible to identify the
accounts that may have been used. Additionally,
malicious users may attempt to manipulate the
authentication controls with the intent of
bypassing them or impersonating a valid account.

[my comment]
Can we consider this is met because pgaudit records the session user name?

[requirement]
10.3 Record at least the following audit
trail entries for all system components for
each event:
10.3.4 Verify success or failure indication is included in log
entries.
10.3.5 Verify origination of event is included in log entries.

[my comment]
This doesn't seem to be fulfilled because the failure of SQL statements and
the client address are not part of the audit log entry.

[requirement]
10.5 Secure audit trails so they cannot
be altered.
10.5 Interview system administrators and examine system
configurations and permissions to verify that audit trails are
secured so that they cannot be altered as follows:
10.5.1 Only individuals who have a job-related need can view
audit trail files.
Adequate protection of the audit logs includes
strong access control (limit access to logs based
on “need to know” only), and use of physical or
network segregation to make the logs harder to
find and modify.
Promptly backing up the logs to a centralized log
server or media that is difficult to alter keeps the
logs protected even if the system generating the
logs becomes compromised.
10.5.2 Protect audit trail files from
unauthorized modifications.

[my comment]
I don't know how to achieve these, because the DBA (who starts/stops the
server) can modify and delete server log files without any record.

[requirement]
10.6 Review logs and security events for
all system components to identify
anomalies or suspicious activity.
Note: Log harvesting, parsing, and
alerting tools may be used to meet this
Requirement.
The log review process does not have to be
manual. The use of log harvesting, parsing, and
alerting tools can help facilitate the process by
identifying log events that need to be reviewed.

[my comment]
What commercial and open source products are well known as the "log
harvesting, parsing, and alerting tool"? Is it possible and reasonably easy
to integrate pgaudit with those tools? The purpose of audit logging feature
is not recording facts, but to enable timely detection of malicious actions.
So, I think the ease of integration with those tools must be evaluated. But
I don't know about such tools.

I feel the current output format of pgaudit is somewhat difficult to treat:

* The audit log entries are mixed with other logs in the server log files,
so the user has to extract the audit log lines from the server log files and
save them elsewhere. I think it is necessary to store audit logs in
separate files.

* Does the command text need "" around it in case it contains commas?

Regards
MauMau

On 18/10/14 06:13, MauMau wrote:
>
> [requirement]
> 10.2.3 Verify access to all audit trails is logged.
>
> Malicious users often attempt to alter audit logs to
> hide their actions, and a record of access allows
> an organization to trace any inconsistencies or
> potential tampering of the logs to an individual
> account. Having access to logs identifying
> changes, additions, and deletions can help retrace
> steps made by unauthorized personnel.
>
> [my comment]
> I'm totally unsure how this can be fulfilled.
>

This is more matter of configuration of the whole system than something
pg_audit itself needs to care about (see my answer to 10.5).

>
> [requirement]
> 10.3 Record at least the following audit
> trail entries for all system components for
> each event:
> 10.3.4 Verify success or failure indication is included in log
> entries.
> 10.3.5 Verify origination of event is included in log entries.
>
> [my comment]
> This doesn't seem to be fulfilled because the failure of SQL statements
> and the client address are not part of the audit log entry.
>

You can add it to the log_line_prefix though.

>
> [requirement]
> 10.5 Secure audit trails so they cannot
> be altered.
> 10.5 Interview system administrators and examine system
> configurations and permissions to verify that audit trails are
> secured so that they cannot be altered as follows:
> 10.5.1 Only individuals who have a job-related need can view
> audit trail files.
> Adequate protection of the audit logs includes
> strong access control (limit access to logs based
> on “need to know” only), and use of physical or
> network segregation to make the logs harder to
> find and modify.
> Promptly backing up the logs to a centralized log
> server or media that is difficult to alter keeps the
> logs protected even if the system generating the
> logs becomes compromised.
> 10.5.2 Protect audit trail files from
> unauthorized modifications.
>
> [my comment]
> I don't know how to achieve these, because the DBA (who starts/stops the
> server) can modify and delete server log files without any record.
>

Logging can be setup in a way that it's not even stored on the server
which DBA has access to. DBA can turn off logging (and the plugin)
altogether or change logging config but we'd get the shutdown log when
that happens so 10.2.2 and 10.2.6 will be fulfilled in that case I think.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 18 October 2014 05:13, MauMau <maumau307(at)gmail(dot)com> wrote:

> [requirement]
> 10.6 Review logs and security events for
> all system components to identify
> anomalies or suspicious activity.
> Note: Log harvesting, parsing, and
> alerting tools may be used to meet this
> Requirement.
> The log review process does not have to be
> manual. The use of log harvesting, parsing, and
> alerting tools can help facilitate the process by
> identifying log events that need to be reviewed.
>
> [my comment]
> What commercial and open source products are well known as the "log
> harvesting, parsing, and alerting tool"? Is it possible and reasonably easy
> to integrate pgaudit with those tools? The purpose of audit logging feature
> is not recording facts, but to enable timely detection of malicious actions.
> So, I think the ease of integration with those tools must be evaluated. But
> I don't know about such tools.
>
> I feel the current output format of pgaudit is somewhat difficult to treat:
>
> * The audit log entries are mixed with other logs in the server log files,
> so the user has to extract the audit log lines from the server log files and
> save them elsewhere. I think it is necessary to store audit logs in
> separate files.
>
> * Does the command text need "" around it in case it contains commas?

Audit entries are sent to the server log, yes.

The server log may be redirected to syslog, which allows various forms
of routing and manipulation that are outside of the reasonable domain
of pgaudit.

PostgreSQL also provides a logging hook that would allow you to filter
or redirect messages as desired.

Given those two ways of handling server log messages, the server log
is the obvious destination to provide for the recording/loggin part of
the audit requirement. pgaudit is designed to allow generating useful
messages, not be an out of the box compliance tool.

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

On Fri, Oct 17, 2014 at 7:44 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> Thanks for the review.
>
> On 16 October 2014 23:23, MauMau <maumau307(at)gmail(dot)com> wrote:
>
>> (3)
>> SELECT against a view generated two audit log lines, one for the view
>> itself, and the other for the underlying table. Is this intended? I'm not
>> saying that's wrong but just asking.
>
> Intended
>
>> (4)
>> I'm afraid audit-logging DML statements on temporary tables will annoy
>> users, because temporary tables are not interesting.
>
> Agreed.
>
>> (5)
>> This is related to (4). As somebody mentioned, I think the ability to
>> select target objects of audit logging is definitely necessary. Without
>> that, huge amount of audit logs would be generated for uninteresting
>> objects. That would also impact performance.
>
> Discussed and agreed already
>
>> (6)
>> What's the performance impact of audit logging? I bet many users will ask
>> "about what percentage would the throughtput decrease by?" I'd like to know
>> the concrete example, say, pgbench and DBT-2.
>
> Don't know, but its not hugely relevant. If you need it, you need it.

Do you have real numbers about the performance impact that this module
has when plugged in the server? If yes, it would be good to get an
idea of how much audit costs and if it has a clear performance impact
this should be documented to warn the user. Some users may really need
audit features as you mention, but the performance drop could be an
obstacle for others so they may prefer continue betting on performance
instead of pgaudit.

>> (8)
>> The code looks good. However, I'm worried about the maintenance. How can
>> developers notice that pgaudit.c needs modification when they add a new SQL
>> statement? What keyword can they use to grep the source code to find
>> pgaudit.c?
>
> Suggestions welcome.
Where are we on this? This patch has no activity for the last two
months... So marking it as returned with feedback. It would be good to
see actual numbers about the performance impact this involves.
Regards,
--
Michael

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

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

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

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

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

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

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

Thanks,

Stephen

On 16 December 2014 at 18:28, Stephen Frost <sfrost(at)snowman(dot)net> wrote:

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

My understanding is that was done.

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

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

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

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

Thanks!

Stephen

On 16 December 2014 at 21:45, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Simon Riggs (simon(at)2ndQuadrant(dot)com) wrote:
>> On 16 December 2014 at 18:28, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>>
>> > For the sake of the archives, the idea I had been trying to propose is
>> > to use a role's permissions as a mechanism to define what should be
>> > audited. An example is:
>>
>> My understanding is that was done.
>
> Based on the discussion I had w/ Abhijit on IRC, and what I saw him
> commit, it's not the same. I've been trying to catch up with him on IRC
> to get clarification but havn't managed to yet.
>
> Abhijit, could you comment on the above (or, well, my earlier email
> which had the details)? It's entirely possible that I've completely
> misunderstood as I haven't dug into the code yet but rather focused on
> the documentation.

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

AFAIK, it is intended to perform as you requested.

Please review...

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

At 2014-12-16 13:28:07 -0500, sfrost(at)snowman(dot)net wrote:
>
> The magic "audit" role has SELECT rights on a given table. When any
> user does a SELECT against that table, ExecCheckRTPerms is called and
> there's a hook there which the module can use to say "ok, does the
> audit role have any permissions here?" and, if the result is yes, then
> the command is audited.

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

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

-- Abhijit

Abhijit,

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

Ok, great, thanks!

Stephen

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

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

Also, what makes the "audit" role magical?

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

At 2014-12-22 08:05:57 -0500, robertmhaas(at)gmail(dot)com wrote:
>
> On Tue, Dec 16, 2014 at 1:28 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > … "ok, does the audit role have any permissions here?" and, if the
> > result is yes, then the command is audited. …
>
> This is a little weird because you're effectively granting an
> anti-permission.

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

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

I'm not sure either.

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

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

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

grant select on foo to audit;

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

> Also, what makes the "audit" role magical?

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

-- Abhijit