Thread: 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.xADD 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
loggingfor these classes can be individually enabled and disabled (either via pgaudit.log in postgresql.conf, or as
aper-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}
tolog 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,
DROPon 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@2ndquadrant.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@agliodbs.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@snowman.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 actualusage. I'm hoping that by the time we make a submission for 9.5, we'll have a clearer picture of what Postgres auditingshould look like. 3. We steered clear of implementing different log targets. We know that ereport() doesn't cut it, but decided that doinganything 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@snowman.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@2ndquadrant.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@2ndquadrant.com) wrote:
> At 2014-05-02 14:22:23 -0400, sfrost@snowman.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@snowman.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 textas $$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 someonegained superuser access without authorisation, their actions would still be logged. But I don't think there's anyway 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 withjunk.
* Abhijit Menon-Sen (ams@2ndquadrant.com) wrote:
> At 2014-05-04 08:52:42 -0400, sfrost@snowman.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@snowman.net> writes:
> * Abhijit Menon-Sen (ams@2ndquadrant.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@snowman.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@sss.pgh.pa.us> wrote: > Stephen Frost <sfrost@snowman.net> writes: >> * Abhijit Menon-Sen (ams@2ndquadrant.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 independentof the technical roles and responsibilities. Meaning that superuser or owner roles could not change the auditrequirements once established and the audit role could not change any data or data definitions except add, change orremove 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 droppedor created by someone. The audit reporting system (external to the database) had notifications for these types ofevents. For example, by procedure these changes should have been done in conjunction with the auditors and the initialaudit requirements should already have been improved by the auditors when the column/table was added back. Droppinga 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 happythat 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 approvedappend 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-changeablelevel of audit reporting should be in place (i.e. log all create/drop/rename tables/columns/roles and log allsuperuser/audit role actions) and this basic audit reporting cannot be turned off or have the destination changed withoutconsiderable headache (something like init'ing the database?). Then data monitoring auditing rules can be added/changed/removedas necessary within the authorization framework. Formal auditing might also require other functionalitylike checksums. Until these or similar requirements (for formal auditing) are in core, it makes no sense (to me) to not allow the superuserto manage auditing because any conformance requirements have to be procedure based, not system based. People oftenforget 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@neiltiffin.com) wrote:
> In considering how this might apply to PostgreSQL, it seems that once formal auditing is turned on, basic
non-changeablelevel of audit reporting should be in place (i.e. log all create/drop/rename tables/columns/roles and log
allsuperuser/audit role actions) and this basic audit reporting cannot be turned off or have the destination changed
withoutconsiderable headache (something like init'ing the database?). Then data monitoring auditing rules can be
added/changed/removedas necessary within the authorization framework. Formal auditing might also require other
functionalitylike 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
superuserto manage auditing because any conformance requirements have to be procedure based, not system based. People
oftenforget 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@snowman.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@neiltiffin.com) wrote: >> In considering how this might apply to PostgreSQL, it seems that once formal auditing is turned on, basic non-changeablelevel of audit reporting should be in place (i.e. log all create/drop/rename tables/columns/roles and log allsuperuser/audit role actions) and this basic audit reporting cannot be turned off or have the destination changed withoutconsiderable headache (something like init'ing the database?). Then data monitoring auditing rules can be added/changed/removedas necessary within the authorization framework. Formal auditing might also require other functionalitylike 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 areasonable first pass would be to handle it similar to '--data-checksums' in 'initdb'. For example, "This option can onlybe 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 auditorshave 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 superuserto manage auditing because any conformance requirements have to be procedure based, not system based. People oftenforget 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@neiltiffin.com) wrote:
> On May 4, 2014, at 3:17 PM, Stephen Frost <sfrost@snowman.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
thinka reasonable first pass would be to handle it similar to '--data-checksums' in 'initdb'. For example, "This
optioncan only be set during initialization, and cannot be changed later. If set, basic auditing is on for all objects,
inall 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
auditorshave 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@snowman.net> wrote: > * Neil Tiffin (neilt@neiltiffin.com) wrote: >> On May 4, 2014, at 3:17 PM, Stephen Frost <sfrost@snowman.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 thinka reasonable first pass would be to handle it similar to '--data-checksums' in 'initdb'. For example, "This optioncan only be set during initialization, and cannot be changed later. If set, basic auditing is on for all objects, inall 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 loggingcould 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 auditorshave 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@neiltiffin.com) wrote:
> On May 4, 2014, at 5:27 PM, Stephen Frost <sfrost@snowman.net> wrote:
> > * Neil Tiffin (neilt@neiltiffin.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
myexperience, that is the responsibility of other systems and procedures. If the core code was such that once on,
formallogging could not be turned off with any changes to config files, settings, or SQL then in my experience that
wouldsuffice.
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@gmx.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@snowman.net> writes: > > * Abhijit Menon-Sen (ams@2ndquadrant.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@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
* Bruce Momjian (bruce@momjian.us) wrote:
> On Sun, May 4, 2014 at 11:12:57AM -0400, Tom Lane wrote:
> > Stephen Frost <sfrost@snowman.net> writes:
> > > * Abhijit Menon-Sen (ams@2ndquadrant.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@2ndquadrant.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.fujii@gmail.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
Attachment
On Mon, Jun 23, 2014 at 7:51 PM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote: > (I'm replying as co-author of pgaudit.) > > At 2014-06-23 19:15:39 +0900, masao.fujii@gmail.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.fujii@gmail.com) wrote:
> On Mon, Jun 23, 2014 at 7:51 PM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote:
> > At 2014-06-23 19:15:39 +0900, masao.fujii@gmail.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@snowman.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@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.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@snowman.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@2ndquadrant.com) wrote:
> At 2014-06-23 08:50:33 -0400, sfrost@snowman.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@snowman.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@2ndquadrant.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@snowman.net> wrote: > * Fujii Masao (masao.fujii@gmail.com) wrote: >> On Mon, Jun 23, 2014 at 7:51 PM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote: >> > At 2014-06-23 19:15:39 +0900, masao.fujii@gmail.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.fujii@gmail.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@2ndQuadrant.com) wrote:
> At 2014-06-23 16:51:55 -0400, sfrost@snowman.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@snowman.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@2ndquadrant.com) wrote:
> At 2014-06-24 14:02:11 -0400, sfrost@snowman.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@2ndquadrant.com) wrote: > > At 2014-06-24 14:02:11 -0400, sfrost@snowman.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@snowman.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@2ndquadrant.com) wrote:
> Stephen Frost wrote:
> > * Abhijit Menon-Sen (ams@2ndquadrant.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@2ndquadrant.com) wrote:
> At 2014-06-25 00:10:55 -0400, sfrost@snowman.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@snowman.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@snowman.net> wrote: > * Abhijit Menon-Sen (ams@2ndquadrant.com) wrote: >> At 2014-06-25 00:10:55 -0400, sfrost@snowman.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@snowman.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@2ndquadrant.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@2ndQuadrant.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@snowman.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@2ndQuadrant.com) wrote:
> On 26 June 2014 14:59, Stephen Frost <sfrost@snowman.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@2ndquadrant.com> wrote: > At 2014-06-25 10:36:07 -0400, sfrost@snowman.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@gmail.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@snowman.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@gmail.com) wrote:
> On Fri, Jun 27, 2014 at 2:23 PM, Stephen Frost <sfrost@snowman.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@snowman.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@2ndQuadrant.com) wrote:
> At 2014-06-30 09:39:29 -0400, sfrost@snowman.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@snowman.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@gmail.com) wrote:
> On Mon, Jun 30, 2014 at 9:39 AM, Stephen Frost <sfrost@snowman.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@gmail.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@snowman.net> wrote: > * Robert Haas (robertmhaas@gmail.com) wrote: >> On Mon, Jun 30, 2014 at 9:39 AM, Stephen Frost <sfrost@snowman.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@2ndQuadrant.com) wrote: > On 30 June 2014 16:20, Stephen Frost <sfrost@snowman.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@gmail.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@gmail.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@snowman.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@2ndQuadrant.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@2ndquadrant.com> wrote: > On 1 July 2014 18:32, Stephen Frost <sfrost@snowman.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@2ndQuadrant.com) wrote: > On 1 July 2014 18:32, Stephen Frost <sfrost@snowman.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 tomodifying 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'tfeel this is accurate- we get a lot of complaints about the lack of flexibility in our logging system and my hope isthat 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. Wemight be able to start with this but I'd expect us to need more control very quickly- auditor A should be able to controlthe auditing for tables in the 'low security' schema, while auditor B should be able to control the auditing for tablesin 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 auditingfor what, and then have to implement the permissions handling inside of the audit configuration functions, and thenrelax the permissions on those functions? Perhaps you see a better approach and I'm just missing it, but if so, it'dbe 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@2ndQuadrant.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@momjian.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@2ndQuadrant.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@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
* Bruce Momjian (bruce@momjian.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
Idon'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@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
Stephen Frost wrote: > * Bruce Momjian (bruce@momjian.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@momjian.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@snowman.net> writes:
> * Bruce Momjian (bruce@momjian.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@2ndquadrant.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@snowman.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@momjian.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@momjian.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@2ndquadrant.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@gmail.com) wrote:
> On Wed, Jul 30, 2014 at 2:49 PM, Alvaro Herrera
> <alvherre@2ndquadrant.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@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > * Bruce Momjian (bruce@momjian.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@snowman.net> wrote: > * Tom Lane (tgl@sss.pgh.pa.us) wrote: >> Stephen Frost <sfrost@snowman.net> writes: >> > * Bruce Momjian (bruce@momjian.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@2ndquadrant.com) wrote:
> On 31 July 2014 22:34, Stephen Frost <sfrost@snowman.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@2ndquadrant.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@2ndquadrant.com> wrote:
>
> On 31 July 2014 22:34, Stephen Frost <sfrost@snowman.net> wrote:
> > * Tom Lane (tgl@sss.pgh.pa.us) wrote:
> >> Stephen Frost <sfrost@snowman.net> writes:
> >> > * Bruce Momjian (bruce@momjian.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.
>
CREATE OPTION [ IF NOT EXISTS ] name
VALIDATOR valfunction
[ DEFAULT value ];
ALTER TABLE name
SET OPTION optname { TO | = } { value | 'value' | DEFAULT };
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
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
informationabout what
reloptions have been set.
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
<div dir="ltr"><br /><br />On Wed, Oct 8, 2014 at 12:36 AM, Josh Berkus <<a
href="mailto:josh@agliodbs.com">josh@agliodbs.com</a>>wrote:<br />><br />> On 10/07/2014 09:44 AM, Fabrízio de
RoyesMello wrote:<br />> > We can think in a mechanism to create "properties / options" and assign it<br />>
>to objects (table, index, column, schema, ...) like COMMENT does.<br />> ><br />> > A quickly
thought:<br/>> ><br />> > CREATE OPTION [ IF NOT EXISTS ] name<br />> > VALIDATOR valfunction<br
/>>> [ DEFAULT value ];<br />> ><br />> > ALTER TABLE name<br />> > SET OPTION optname
{TO | = } { value | 'value' | DEFAULT };<br />> ><br />> > It's just a simple thought of course. We must
thinkbetter about the syntax<br />> > and purposes.<br />><br />> If we're going to do this (and it seems
likea good idea), we really,<br />> really, really need to include some general system views which expose<br />>
theoptions in a user-friendly format (like columns, JSON or an array).<br />> It's already very hard for users to
getinformation about what<br />> reloptions have been set.<br />><br /><br />Maybe into "information_schema"
??<br/><br />Regards,<br /><br />--<br />Fabrízio de Royes Mello<br />Consultoria/Coaching PostgreSQL<br />>>
Timbira:<a href="http://www.timbira.com.br">http://www.timbira.com.br</a><br />>> Blog: <a
href="http://fabriziomello.github.io">http://fabriziomello.github.io</a><br/>>> Linkedin: <a
href="http://br.linkedin.com/in/fabriziomello">http://br.linkedin.com/in/fabriziomello</a><br/>>> Twitter: <a
href="http://twitter.com/fabriziomello">http://twitter.com/fabriziomello</a><br/>>> Github: <a
href="http://github.com/fabriziomello">http://github.com/fabriziomello</a></div>
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@agliodbs.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@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.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@gmail.com) wrote:
> On Tue, Oct 7, 2014 at 1:24 PM, Simon Riggs <simon@2ndquadrant.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@gmail.com) wrote: >> On Tue, Oct 7, 2014 at 1:24 PM, Simon Riggs <simon@2ndquadrant.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
<div dir="ltr"><div class="gmail_extra"><br />On Wed, Oct 8, 2014 at 2:55 AM, David Fetter <<a
href="mailto:david@fetter.org">david@fetter.org</a>>wrote:<br />><br />> On Wed, Oct 08, 2014 at 12:41:46AM
-0300,Fabrízio de Royes Mello wrote:<br />> > On Wed, Oct 8, 2014 at 12:36 AM, Josh Berkus <<a
href="mailto:josh@agliodbs.com">josh@agliodbs.com</a>>wrote:<br />> > ><br />> > > On 10/07/2014
09:44AM, Fabrízio de Royes Mello wrote:<br />> > > > We can think in a mechanism to create "properties /
options"and assign<br />> > it<br />> > > > to objects (table, index, column, schema, ...) like
COMMENTdoes.<br />> > > ><br />> > > > A quickly thought:<br />> > > ><br />>
>> > CREATE OPTION [ IF NOT EXISTS ] name<br />> > > > VALIDATOR valfunction<br />> >
>> [ DEFAULT value ];<br />> > > ><br />> > > > ALTER TABLE name<br />> > >
> SET OPTION optname { TO | = } { value | 'value' | DEFAULT };<br />> > > ><br />> > > >
It'sjust a simple thought of course. We must think better about the<br />> > syntax<br />> > > > and
purposes.<br/>> > ><br />> > > If we're going to do this (and it seems like a good idea), we
really,<br/>> > > really, really need to include some general system views which expose<br />> > >
theoptions in a user-friendly format (like columns, JSON or an array).<br />> > > It's already very hard for
usersto get information about what<br />> > > reloptions have been set.<br />> > ><br />> ><br
/>>> Maybe into "information_schema" ??<br />><br />> Not there. The information schema is defined pretty
preciselyin the<br />> SQL standard and contains only some of this kind of information.<br />><br /><br
/></div><divclass="gmail_extra">You're absolutely right.. thanks...<br /><br /></div><div class="gmail_extra"><br
/>>pg_catalog seems like a much more appropriate space for<br />> PostgreSQL-specific settings.<br />><br
/><br/></div><div class="gmail_extra">We can add views and some functions to get this info too.<br /></div><div
class="gmail_extra"><br/></div><div class="gmail_extra">Regards,<br /></div><div class="gmail_extra"><br />--<br
/>Fabríziode Royes Mello<br />Consultoria/Coaching PostgreSQL<br />>> Timbira: <a
href="http://www.timbira.com.br">http://www.timbira.com.br</a><br/>>> Blog: <a
href="http://fabriziomello.github.io">http://fabriziomello.github.io</a><br/>>> Linkedin: <a
href="http://br.linkedin.com/in/fabriziomello">http://br.linkedin.com/in/fabriziomello</a><br/>>> Twitter: <a
href="http://twitter.com/fabriziomello">http://twitter.com/fabriziomello</a><br/>>> Github: <a
href="http://github.com/fabriziomello">http://github.com/fabriziomello</a></div></div>
From: "Simon Riggs" <simon@2ndquadrant.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@2ndQuadrant.com wrote: > > As before, the pgaudit code is at > https://github.com/2ndQuadrant/pgaudit Sorry, I forgot to attach the tarball. -- Abhijit
Attachment
Abhijit, * Abhijit Menon-Sen (ams@2ndQuadrant.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
On 14 October 2014 13:57, Stephen Frost <sfrost@snowman.net> wrote: > Create an 'audit' role. > > Every command run by roles which are granted to the 'audit' role are > audited. > > Every 'select' against tables which the 'audit' role has 'select' rights > on are audited. Similairly for every insert, update, delete. I think that's a good idea. We could have pg_audit.roles = 'audit1, audit2' so users can specify any audit roles they wish, which might even be existing user names. That is nice because it allows multiple completely independent auditors to investigate whatever they choose without discussing with other auditors. -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
* Simon Riggs (simon@2ndQuadrant.com) wrote:
> On 14 October 2014 13:57, Stephen Frost <sfrost@snowman.net> wrote:
>
> > Create an 'audit' role.
> >
> > Every command run by roles which are granted to the 'audit' role are
> > audited.
> >
> > Every 'select' against tables which the 'audit' role has 'select' rights
> > on are audited. Similairly for every insert, update, delete.
>
> I think that's a good idea.
>
> We could have pg_audit.roles = 'audit1, audit2'
> so users can specify any audit roles they wish, which might even be
> existing user names.
Agreed.
> That is nice because it allows multiple completely independent
> auditors to investigate whatever they choose without discussing with
> other auditors.
Yes, also a good thought.
Thanks!
Stephen
At 2014-10-14 20:09:50 +0100, simon@2ndQuadrant.com wrote: > > I think that's a good idea. > > We could have pg_audit.roles = 'audit1, audit2' Yes, it's a neat idea, and we could certainly do that. But why is it any better than "ALTER ROLE audit_rw SET pgaudit.log = …" and granting that role to the users whose actions you want to audit? -- Abhijit
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@gmail.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@gmail.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 14 October 2014 20:33, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote: > At 2014-10-14 20:09:50 +0100, simon@2ndQuadrant.com wrote: >> >> I think that's a good idea. >> >> We could have pg_audit.roles = 'audit1, audit2' > > Yes, it's a neat idea, and we could certainly do that. But why is it any > better than "ALTER ROLE audit_rw SET pgaudit.log = …" and granting that > role to the users whose actions you want to audit? That would make auditing visible to the user, who may then be able to do something about it. Stephen's suggestion allows auditing to be conducted without the users/apps being aware it is taking place. -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
At 2014-11-03 17:31:37 +0000, simon@2ndQuadrant.com wrote: > > Stephen's suggestion allows auditing to be conducted without the > users/apps being aware it is taking place. OK, that makes sense. I'm working on a revised patch that does things this way, and will post it in a few days. -- Abhijit
Hi. I could actually use some comments on the approach. I've attached a prototype I've been working on (which is a cut down version of my earlier code; but it's not terribly interesting and you don't need to read it to comment on my questions below). The attached patch does the following: 1. Adds a pgaudit.roles = 'role1, role2' GUC setting. 2. Adds a role_is_audited() function that returns true if the given role OID is mentioned in (or inherits from a role mentioned in) pgaudit.roles. 3. Adds a call to role_is_audited from log_audit_event with the current user id (GetSessionUserId in the patch, though it may be better to use GetUserId; but that's a minor detail). Earlier, I was using a combination of check and assign hooks to convert names to OIDs, but (as Andres pointed out) that would have problems with cache invalidations. I was even playing with caching membership lookups, but I ripped out all that code. In the attached patch, role_is_audited does all the hard work to split up the list of roles, look up the corresponding OIDs, and check if the user is a member of any of those roles. It works fine, but it doesn't seem desirable to repeat all that work for every statement. So does anyone have suggestions about how to make this faster? -- Abhijit
Attachment
Abhijit Menon-Sen <ams@2ndQuadrant.com> writes:
> Earlier, I was using a combination of check and assign hooks to convert
> names to OIDs, but (as Andres pointed out) that would have problems with
> cache invalidations. I was even playing with caching membership lookups,
> but I ripped out all that code.
> In the attached patch, role_is_audited does all the hard work to split
> up the list of roles, look up the corresponding OIDs, and check if the
> user is a member of any of those roles. It works fine, but it doesn't
> seem desirable to repeat all that work for every statement.
> So does anyone have suggestions about how to make this faster?
Have you read the code in acl.c that caches lookup results for
role-is-member-of checks? Sounds pretty closely related.
regards, tom lane
On Fri, Oct 17, 2014 at 7:44 AM, Simon Riggs <simon@2ndquadrant.com> wrote: > Thanks for the review. > > On 16 October 2014 23:23, MauMau <maumau307@gmail.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.paquier@gmail.com) wrote:
> Do you have real numbers about the performance impact that this module
> has when plugged in the server? If yes, it would be good to get an
> idea of how much audit costs and if it has a clear performance impact
> this should be documented to warn the user. Some users may really need
> audit features as you mention, but the performance drop could be an
> obstacle for others so they may prefer continue betting on performance
> instead of pgaudit.
While these performance numbers would be interesting, I don't feel it's
necessary to consider the performance of this module as part of the
question about if it should go into contrib or not.
> Where are we on this? This patch has no activity for the last two
> months... So marking it as returned with feedback. It would be good to
> see actual numbers about the performance impact this involves.
What I was hoping to see were the changes that I had suggested
up-thread, but I discovered about a week or two ago that there was a
misunderstanding between at least Abhijit and myself about what was
being suggested.
For the sake of the archives, the idea I had been trying to propose is
to use a role's permissions as a mechanism to define what should be
audited. An example is:
The magic "audit" role has SELECT rights on a given table. When any
user does a SELECT against that table, ExecCheckRTPerms is called and
there's a hook there which the module can use to say "ok, does the audit
role have any permissions here?" and, if the result is yes, then the
command is audited. Note that this role, from core PG's perspective,
wouldn't be special at all; it would just be that pgaudit would use the
role's permissions as a way to figure out if a given command should be
audited or not.
That's not to say that we couldn't commit pgaudit without this
capability and add it later, but I had been expecting to see progress
along these lines prior to reviewing it.
Thanks,
Stephen
On 16 December 2014 at 18:28, Stephen Frost <sfrost@snowman.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@2ndQuadrant.com) wrote:
> On 16 December 2014 at 18:28, Stephen Frost <sfrost@snowman.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@snowman.net> wrote: > * Simon Riggs (simon@2ndQuadrant.com) wrote: >> On 16 December 2014 at 18:28, Stephen Frost <sfrost@snowman.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@snowman.net wrote: > > The magic "audit" role has SELECT rights on a given table. When any > user does a SELECT against that table, ExecCheckRTPerms is called and > there's a hook there which the module can use to say "ok, does the > audit role have any permissions here?" and, if the result is yes, then > the command is audited. You're right, I did not understand that this is what you were proposing, and this is not what the code does. I went back and read your original description, and it seems I implemented only the subset I understood. I'll look into changing the code sometime next week. -- Abhijit
Abhijit, * Abhijit Menon-Sen (ams@2ndQuadrant.com) wrote: > At 2014-12-16 13:28:07 -0500, sfrost@snowman.net wrote: > > > > The magic "audit" role has SELECT rights on a given table. When any > > user does a SELECT against that table, ExecCheckRTPerms is called and > > there's a hook there which the module can use to say "ok, does the > > audit role have any permissions here?" and, if the result is yes, then > > the command is audited. > > You're right, I did not understand that this is what you were proposing, > and this is not what the code does. I went back and read your original > description, and it seems I implemented only the subset I understood. > > I'll look into changing the code sometime next week. Ok, great, thanks! Stephen
On Tue, Dec 16, 2014 at 1:28 PM, Stephen Frost <sfrost@snowman.net> wrote: > The magic "audit" role has SELECT rights on a given table. When any > user does a SELECT against that table, ExecCheckRTPerms is called and > there's a hook there which the module can use to say "ok, does the audit > role have any permissions here?" and, if the result is yes, then the > command is audited. 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@gmail.com wrote: > > On Tue, Dec 16, 2014 at 1:28 PM, Stephen Frost <sfrost@snowman.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
On Thu, Dec 25, 2014 at 5:42 AM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote: > > To summarise for people who haven't followed the thread in detail, the > idea is that you would do: > > grant select on foo to audit; > > ...and the server would audit-log any "select ... from foo ..." queries (by > any user). what if i want to audit different things for different users? -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación Phone: +593 4 5107566 Cell: +593 987171157
On 25 December 2014 at 17:09, Jaime Casanova <jaime@2ndquadrant.com> wrote: > On Thu, Dec 25, 2014 at 5:42 AM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote: >> >> To summarise for people who haven't followed the thread in detail, the >> idea is that you would do: >> >> grant select on foo to audit; >> >> ...and the server would audit-log any "select ... from foo ..." queries (by >> any user). > > what if i want to audit different things for different users? That is supported - you can provide a list of roles to be audit roles. -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 25 December 2014 at 10:42, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote: > Stephen likes the idea, obviously; Simon also said he liked it, but I > now wonder if he may have liked the part I implemented (which allows a > hot standby to have a different auditing configuration than the primary) > but not fully realised the remainder of the proposal. I am happy with the proposal, I just thought you'd already done it. > Before I go much further, how do others feel about it? > > To summarise for people who haven't followed the thread in detail, the > idea is that you would do: > > grant select on foo to audit; GRANT is understood and supported by many people and tools. The idea makes auditing immediately accessible for wide usage. > …and the server would audit-log any "select … from foo …" queries (by > any user). One immediate consequence is that only things you could grant > permissions for could be audited (by this mechanism), but I guess that's > a problem only in the short term. Another consequence is that you can't > audit selects on foo only by role x and selects on bar only by role y. > >> Also, what makes the "audit" role magical? > > I think it's because it exists only to receive these "negative" grants, > there's no other magic involved. Stephen also said «Note that this role, > from core PG's perspective, wouldn't be special at all». I don't see them as "negative grants". You are simply specifying the privilege and object you want logged. Abhijit is right to point out that we can't specify all combinations of actions, but solving that is considerably more complex. At the moment we don't have strong evidence that we should wait for such additional complexity. We can improve things in next release if it is truly needed. -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
At 2014-12-27 08:08:32 +0000, simon@2ndQuadrant.com wrote: > > > what if i want to audit different things for different users? > > That is supported - you can provide a list of roles to be audit roles. You can do that, but maybe it isn't quite enough to do what Jaime is asking for. Not always, at least. Consider: 1. You want to audit select on foo by user jaime (only). 2. You want to audit update on bar by user simon (only). So you do something like this: pgaudit.roles = 'audit1, audit2' grant select on foo to audit1; grant update on bar to audit2; Now if Jaime does select on foo or if Simon does update on bar, it'll be logged. If user jaime does not have permission to do update on bar, and if simon doesn't have permission to select on foo, everything is fine. But there's no way to say *don't* audit select on foo by simon. You could do something like "grant role audit1 to jaime" and then check the audit-permissions of whichever audit role jaime is found at runtime to inherit from. But in Stephen's original proposal, granting any audit role to any user meant that everything they did would be logged. -- Abhijit
On 27 December 2014 at 08:47, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote: > But there's no way to say *don't* audit select on foo by simon. We can cover what it does and does not do in some doc examples. When submitted, pgaudit didn't have very complex auditing rules. Stephen's suggestion improves that considerably, but isn't the only conceivable logging rule. But we'll need to see what else is needed; I doubt we'll need everything, at least not in PG9.5 -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
* Simon Riggs (simon@2ndQuadrant.com) wrote:
> On 27 December 2014 at 08:47, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote:
>
> > But there's no way to say *don't* audit select on foo by simon.
>
> We can cover what it does and does not do in some doc examples.
>
> When submitted, pgaudit didn't have very complex auditing rules.
> Stephen's suggestion improves that considerably, but isn't the only
> conceivable logging rule. But we'll need to see what else is needed; I
> doubt we'll need everything, at least not in PG9.5
Agreed, it allows us much more flexibility, but it isn't a panacea. I'm
hopeful that it'll be flexibile enough for certain regulatory-required
use-cases. In any case, it's much closer and is certainly worthwhile
even if it doesn't allow for every possible configuration or ends up not
meeting specific regulatory needs because it moves us to a place where
we can sensibly consider "what else is needed?"
Thanks,
Stephen
Hi. I've changed pgaudit to work as you suggested. A quick note on the implementation: pgaudit was already installing an ExecutorCheckPerms_hook anyway; I adapted code from ExecRTECheckPerms to check if the audit role has been granted any of the permissions required for the operation. This means there are three ways to configure auditing: 1. GRANT … ON … TO audit, which logs any operations that correspond to the granted permissions. 2. Set pgaudit.roles = 'r1, r2, …', which logs everything done by r1, r2, and any of their descendants. 3. Set pgaudit.log = 'read, write, …', which logs any events in any of the listed classes. (This is a small change from the earlier behaviour where, if a role was listed in .roles, it was still subject to the .log setting. I find that more useful in practice, but since we're discussing Stephen's proposal, I implemented what he said.) The new pgaudit.c is attached here for review. Nothing else has changed from the earlier submission; and everything is in the github repository (github.com/2ndQuadrant/pgaudit). -- Abhijit
Attachment
Abhijit,
Apologies, I've been meaning to go through this for quite some time.
* Abhijit Menon-Sen (ams@2ndQuadrant.com) wrote:
> I've changed pgaudit to work as you suggested.
Great, glad to see that.
> A quick note on the implementation: pgaudit was already installing an
> ExecutorCheckPerms_hook anyway; I adapted code from ExecRTECheckPerms
> to check if the audit role has been granted any of the permissions
> required for the operation.
Sure, makes sense to me.
> This means there are three ways to configure auditing:
>
> 1. GRANT … ON … TO audit, which logs any operations that correspond to
> the granted permissions.
I was thinking we would be able to configure which role this applies to,
rather than hard-coding 'audit' as *the* role. Perhaps with a new GUC,
or the existing 'pgaudit.roles' GUC could be used. Further, this can be
extrapolated out to cover auditing of roles by checking for role
membership in this role, see below.
> 2. Set pgaudit.roles = 'r1, r2, …', which logs everything done by r1,
> r2, and any of their descendants.
If these roles were the 'audit' roles as considered under #1 then
couldn't administrators control what pgaudit.roles provide today using
role memberships granted to the roles listed?
> 3. Set pgaudit.log = 'read, write, …', which logs any events in any of
> the listed classes.
Approach #1 addresses this too, doesn't it? SELECT vs. UPDATE vs.
DELETE, etc? I'm not sure that this really adds much as it isn't nearly
as granular as the GRANT-based approach since it applies to everything.
One of the really big and interesting distinctions to consider between
checking permissions granted to an 'audit' role vs. role membership is
how DDL is handled. As currently implemented, role-level auditing is
required to have DDL be logged, but you may very well want to log all
DDL and no DML, for example. What would be really helpful is to develop
a wiki to cover common use-cases and how to set up pgaudit for them.
> (This is a small change from the earlier behaviour where, if a role was
> listed in .roles, it was still subject to the .log setting. I find that
> more useful in practice, but since we're discussing Stephen's proposal,
> I implemented what he said.)
Right- the idea is to provide a very granular way of specifying what is
to be logged; much more than what the previous pair of GUCs provided.
> The new pgaudit.c is attached here for review. Nothing else has changed
> from the earlier submission; and everything is in the github repository
> (github.com/2ndQuadrant/pgaudit).
There's a few big questions we need to address with pgaudit to have it
go into our repo.
The first is what major version(s) we're targetting. My feeling is that
we should strip down what goes into contrib to be 9.5 based. I
certainly understand that you want to support earlier versions but I
don't think it makes sense to try and carry that baggage in contrib when
it won't ever be released with earlier versions.
The second is the USE_DEPARSE_FUNCTIONS-related bits. I realize that
there's a goal to get additional things into 9.5 from that branch but it
doesn't make sense for the contrib pgaudit to include those components
prior to them actually being in core. I'm also wondering if pieces of
that are now out-of-date with where core is. If those parts are going
to go into 9.5 soon (which looks like it may happen..) then hopefully we
can just remove the #define's and clean up the code to use the new
capabilities.
Lastly, I'll echo a concern which Robert has raised which is- how do we
make sure that new commands, etc, are covered? I don't particularly
like how pgaudit will need to be kept in sync with what's in
ProcessUtility (normal and slow). I'm actually a bit hopeful that we
can work out a way for pgaudit to help with this through a regression
test which loops over all objects and tests logging each of them.
One of the important aspects of auditing is that what is requested to be
audited is and exactly that- no more, no less. Reviewing the code makes
me pretty nervous about that actually happening properly, but that's
mostly due to the back-and-forth between different major versions and
the #ifdef/#ifndef's.
Additional comments in-line below.
> enum LogClass {
> LOG_NONE = 0,
>
> /* SELECT */
> LOG_READ = (1 << 0),
>
> /* INSERT, UPDATE, DELETE, TRUNCATE */
> LOG_WRITE = (1 << 1),
>
> /* GRANT, REVOKE, ALTER … */
> LOG_PRIVILEGE = (1 << 2),
>
> /* CREATE/DROP/ALTER ROLE */
> LOG_USER = (1 << 3),
>
> /* DDL: CREATE/DROP/ALTER */
> LOG_DEFINITION = (1 << 4),
>
> /* DDL: CREATE OPERATOR etc. */
> LOG_CONFIG = (1 << 5),
>
> /* VACUUM, REINDEX, ANALYZE */
> LOG_ADMIN = (1 << 6),
>
> /* Function execution */
> LOG_FUNCTION = (1 << 7),
>
> /* Absolutely everything; not available via pgaudit.log */
> LOG_ALL = ~(uint64)0
> };
I'd really like to see this additional information regarding what kind
a command is codified in core. Ideally, we'd have a single place which
tracks the kind of command and possibly other information which can then
be addressed all at once when new commands are added.
Also, we could allow more granularity by using the actual classes
which we already have for objects.
> /*
> * This module collects AuditEvents from various sources (event
> * triggers, and executor/utility hooks) and passes them to the
> * log_audit_event() function.
> *
> * An AuditEvent represents an operation that potentially affects a
> * single object. If an underlying command affects multiple objects,
> * multiple AuditEvents must be created to represent it.
> */
The above doesn't appear to be accurate (perhaps it once was?) as
log_audit_event() only takes a single AuditEvent structure at a time and
it's not a list. I'm not sure that needs to change, just pointing out
that the comment appears to be inaccurate.
> typedef struct {
> NodeTag type;
> const char *object_id;
> const char *object_type;
> const char *command_tag;
> const char *command_text;
> bool granted;
> } AuditEvent;
I'm inclined to suggest that the decision be made earlier on about if a
given action should be logged, as the earlier we do that the less work
we have to do and we could avoid having to pass in things like 'granted'
to the log_audit_event function at all. Having the decision split with
the 'granted' flag being set from the callback and then the
'should_be_logged' happening inside log_audit_event doesn't strike me as
terribly sensible.
> /*
> * Returns the oid of the hardcoded "audit" role.
> */
>
> static Oid
> audit_role_oid()
> {
> HeapTuple roleTup;
> Oid oid = InvalidOid;
>
> roleTup = SearchSysCache1(AUTHNAME, PointerGetDatum("audit"));
> if (HeapTupleIsValid(roleTup)) {
> oid = HeapTupleGetOid(roleTup);
> ReleaseSysCache(roleTup);
> }
>
> return oid;
> }
The above could go away if we follow my suggestions above, of course.
> /* Returns true if either pgaudit.roles or pgaudit.log is set. */
>
> static inline bool
> pgaudit_configured()
> {
> return (pgaudit_roles_str && *pgaudit_roles_str) || pgaudit_log != 0;
> }
This feels, to me at least, like it shouldn't be necessary. If the
check to see if we should be logging the current action is moved up
sufficiently, then we may be able to do away with this function and the
checks associated with it- those would turn into "should we be logging
this?" and, if the lists are empty, would return "no" pretty quickly.
> /*
> * Takes a role OID and returns true if the role is mentioned in
> * pgaudit.roles or if it inherits from a role mentioned therein;
> * returns false otherwise.
> */
>
> static bool
> role_is_audited(Oid roleid)
> {
> List *roles;
> ListCell *lt;
>
> if (!pgaudit_roles_str || !*pgaudit_roles_str)
> return false;
>
> if (!SplitIdentifierString(pgaudit_roles_str, ',', &roles))
> return false;
>
> foreach(lt, roles)
> {
> char *name = (char *)lfirst(lt);
> HeapTuple roleTup;
>
> roleTup = SearchSysCache1(AUTHNAME, PointerGetDatum(name));
> if (HeapTupleIsValid(roleTup))
> {
> Oid parentrole = HeapTupleGetOid(roleTup);
>
> ReleaseSysCache(roleTup);
> if (is_member_of_role(roleid, parentrole))
> return true;
> }
> }
>
> return false;
> }
The results here should be cached, as was discussed earlier, iirc.
> /*
> * Takes a role OID and an AuditEvent and returns true or false
> * depending on whether the event should be logged according to the
> * pgaudit.roles/log settings. If it returns true, it also fills in the
> * name of the LogClass which it is to be logged under.
> */
>
> static bool
> should_be_logged(Oid userid, AuditEvent *e, const char **classname)
> {
> enum LogClass class = LOG_NONE;
> char *name;
>
> /*
> * Look at the type of the command and decide what LogClass needs to
> * be enabled for the command to be logged.
> */
>
> switch (e->type)
> {
> case T_SelectStmt:
> name = "READ";
> class = LOG_READ;
> break;
[... long list of every type we have ...]
I'm really not a fan of these big switch statements which attempt to
cover everything which currently exists. If we don't come up with a way
to make very sure that these areas are updated as things change, we're
going to end up missing things. As mentioned up-thread, I'd really like
to see a way to identify these types from core-based information
instead.
> /*
> * Anything that's left out of the list above is just noise,
> * and not very interesting from an auditing perspective. So
> * there's intentionally no way to enable LOG_ALL.
> */
This approach really worries me- what about some new type which is
added? How can we know that it's just "noise"? Whatever is considered
'noise' should at least be explicitly listed, so we know we're covering
everything.
> /*
> * Takes an AuditEvent and, if it should_be_logged(), writes it to the
> * audit log. The AuditEvent is assumed to be completely filled in by
> * the caller (unknown values must be set to "" so that they can be
> * logged without error checking).
> */
>
> static void
> log_audit_event(AuditEvent *e)
> {
So, clearly, this is an area which could use some improvement.
Specifically, being able to write to an independent file instead of just
using ereport(), supporting other output formats (JSON, maybe even a
table..). Also, have you considered using a dynamic shared memory block
to queue the logging messages to and then a background worker to write
them out in-order to a file? It'd clearly be similar to our existing
ereport() mechanism, but importantly you'd be able to send it to another
file. Being able to log directly to syslog or other message queues
would be great also.
These are things which could be added later, of course.
> /*
> * Create AuditEvents for DML operations via executor permissions
> * checks. We create an AuditEvent for each table in the list of
> * RangeTableEntries from the query.
> */
>
> static void
> log_executor_check_perms(Oid auditOid, List *rangeTabls, bool abort_on_violation)
> {
> ListCell *lr;
>
> foreach(lr, rangeTabls)
> {
> Oid relOid;
> Relation rel;
> AuditEvent e;
> RangeTblEntry *rte = lfirst(lr);
> char *relname;
> const char *tag;
> const char *reltype;
> NodeTag type;
>
> /* We only care about tables, and can ignore subqueries etc. */
> if (rte->rtekind != RTE_RELATION)
> continue;
>
> /*
> * Get the fully-qualified name of the relation.
> *
> * User queries against catalog tables (e.g. "\dt") are logged
> * here. Should we filter them out, as we do for functions in
> * pg_catalog?
> */
I'd probably say 'yes', regarding this, though it might be nice to have
it be optional..
> relOid = rte->relid;
> rel = relation_open(relOid, NoLock);
> relname = quote_qualified_identifier(get_namespace_name(RelationGetNamespace(rel)),
> RelationGetRelationName(rel));
> relation_close(rel, NoLock);
>
> /*
> * We don't have access to the parsetree here, so we have to
> * generate the node type, object type, and command tag by
> * decoding rte->requiredPerms and rte->relkind.
> */
This seems like it could be improved- have you considered suggesting a
hook which is, perhaps, in a better place and could pass in this
information instead of having to attempt to regenerate it?
> /*
> * If a role named "audit" exists, we check if it has been
> * granted permission to perform the operation identified above.
> * If so, we must log the event regardless of the static pgaudit
> * settings.
> */
This isn't quite what was intended. This isn't about what the audit
role could do but rather auditing specifically what the role has access
to. The idea is that the permission system is used as a proxy for
configuration at the action+column level.
As an example, if an UPDATE with a WHERE clause is executed and the
audit user has SELECT rights on the table, then the UPDATE should be
logged due to the WHERE clause. At least, that's been my thinking as it
mirrors our permissions system but, as Robert said, it's an
'anti-permission' approach. On the flip side, if UPDATE is granted to
the audit role, but an UPDATE with WHERE clause is run then we should
log that- even if the audit role doesn't have SELECT. This all matches
up (but is opposite of) our regular permission system. Deviating from
that strikes me as a bad idea.
One caveat on this is that I was thinking the permission would have to
be explicitly granted- that is, grants to PUBLIC would not count. That
may be more difficult to deal with though, but it'd certainly make
auditing of functions easier as execute is granted to public by default
there and would create a lot of noise otherwise.
> log_utility_command(Node *parsetree,
> const char *queryString,
> ProcessUtilityContext context,
> ParamListInfo params,
> DestReceiver *dest,
> char *completionTag)
Similar thoughts here wrt the huge switch statement, etc.
> /*
> * Create AuditEvents for certain kinds of CREATE and ALTER statements,
> * as detected by log_object_access() in lieu of event trigger support
> * for them.
> */
Just to comment on this, the object access framework feels like it
should, perhaps, be reworked to be based on event triggers.. I'm
nervous that we're ending up with so many different ways to deal with
what's happening under ProcessUtility, and that we've now split that in
half between what can be done with event triggers and what can't, with
no way for modules to easily know which is which.
> log_function_execution(Oid objectId)
> {
> HeapTuple proctup;
> Form_pg_proc proc;
> const char *name;
> AuditEvent e;
>
> proctup = SearchSysCache1(PROCOID, ObjectIdGetDatum(objectId));
> if (!proctup)
> elog(ERROR, "cache lookup failed for function %u", objectId);
> proc = (Form_pg_proc) GETSTRUCT(proctup);
>
> /*
> * Logging execution of all pg_catalog functions would
> * make the log unusably noisy.
> */
>
> if (IsSystemNamespace(proc->pronamespace))
> {
> ReleaseSysCache(proctup);
> return;
> }
>
> name = quote_qualified_identifier(get_namespace_name(proc->pronamespace),
> NameStr(proc->proname));
> ReleaseSysCache(proctup);
>
> e.type = T_ExecuteStmt;
> e.object_id = name;
> e.object_type = "FUNCTION";
> e.command_tag = "EXECUTE";
> if (debug_query_string)
> e.command_text = debug_query_string;
> else
> e.command_text = "";
> e.granted = false;
>
> log_audit_event(&e);
> }
This doesn't address overloaded functions, does it? It probably
should include the arguments and types in 'name'.
Thanks!
Stephen
Hello Stephen.
Thanks very much for having a look at the revised pgaudit.
At 2015-01-18 22:28:37 -0500, sfrost@snowman.net wrote:
>
> > 2. Set pgaudit.roles = 'r1, r2, …', which logs everything done by r1,
> > r2, and any of their descendants.
>
> If these roles were the 'audit' roles as considered under #1 then
> couldn't administrators control what pgaudit.roles provide today using
> role memberships granted to the roles listed?
Yes. But then there would be no convenient way to say "just log
everything this particular role does".
> > 3. Set pgaudit.log = 'read, write, …', which logs any events in any
> > of the listed classes.
>
> Approach #1 addresses this too, doesn't it?
Approach #1 applies only to things you can grant permissions for. What
if you want to audit other commands?
> As currently implemented, role-level auditing is required to have DDL
> be logged, but you may very well want to log all DDL and no DML, for
> example.
Well, as formerly implemented, you could do this by adding r1 to .roles
and then setting .log appropriately. But you know that already and, if
I'm not mistaken, have said you don't like it.
I'm all for making it possible to configure fine-grained auditing, but
I don't think that should come at the expense of being able to whack
things with a simpler, heavier^Wmore inclusive hammer if you want to.
> My feeling is that we should strip down what goes into contrib to be
> 9.5 based.
I do not think I can express a constructive opinion about this. I will
go along with whatever people agree is best.
> I'm also wondering if pieces of that are now out-of-date with where
> core is.
Yes, they are. I'll clean that up.
> I don't particularly like how pgaudit will need to be kept in sync
> with what's in ProcessUtility (normal and slow).
Sure, it's a pain.
> I'd really like to see this additional information regarding what kind
> a command is codified in core. Ideally, we'd have a single place
> which tracks the kind of command and possibly other information which
> can then be addressed all at once when new commands are added.
What would this look like, and is it a realistic goal for 9.5?
> Also, we could allow more granularity by using the actual classes
> which we already have for objects.
Explain?
> > /*
> > * This module collects AuditEvents from various sources (event
> > * triggers, and executor/utility hooks) and passes them to the
> > * log_audit_event() function.
> > *
> > * An AuditEvent represents an operation that potentially affects a
> > * single object. If an underlying command affects multiple objects,
> > * multiple AuditEvents must be created to represent it.
> > */
>
> The above doesn't appear to be accurate (perhaps it once was?) as
> log_audit_event() only takes a single AuditEvent structure at a time
> and it's not a list. I'm not sure that needs to change, just pointing
> out that the comment appears to be inaccurate.
If multiple AuditEvents are created (as the ExecutorCheckPerms_hook may
do), log_audit_event() will be called multiple times. The comment is
correct, but I'll try to make it more clear.
> I'm inclined to suggest that the decision be made earlier on about if
> a given action should be logged, as the earlier we do that the less
> work we have to do and we could avoid having to pass in things like
> 'granted' to the log_audit_event function at all.
I considered that, but it makes it much harder to review all of the
places where such decisions are made. It's partly because I wrote the
code in this way that I was able to quickly change it to work the way
you suggested (at least once I understood what you suggested).
I prefer the one-line function and a few «if (pgaudit_configured())»
checks (to avoid creating AuditEvents if they won't be needed at all)
and centralising the decision-making rather than spreading the latter
around the whole code.
> > /*
> > * Takes a role OID and returns true if the role is mentioned in
> > * pgaudit.roles or if it inherits from a role mentioned therein;
> > * returns false otherwise.
> > */
> >
> > static bool
> > role_is_audited(Oid roleid)
> > {
> > …
>
> The results here should be cached, as was discussed earlier, iirc.
I'll look into that, but it's a peripheral matter.
> Whatever is considered 'noise' should at least be explicitly listed,
> so we know we're covering everything.
OK.
> > /*
> > * Takes an AuditEvent and, if it should_be_logged(), writes it to the
> > * audit log. The AuditEvent is assumed to be completely filled in by
> > * the caller (unknown values must be set to "" so that they can be
> > * logged without error checking).
> > */
> >
> > static void
> > log_audit_event(AuditEvent *e)
> > {
>
> So, clearly, this is an area which could use some improvement.
> Specifically, being able to write to an independent file instead of just
> using ereport(), supporting other output formats (JSON, maybe even a
> table..). Also, have you considered using a dynamic shared memory block
> to queue the logging messages to and then a background worker to write
> them out in-order to a file? It'd clearly be similar to our existing
> ereport() mechanism, but importantly you'd be able to send it to another
> file. Being able to log directly to syslog or other message queues
> would be great also.
>
> These are things which could be added later, of course.
I have considered all of those things and, as I have said before, taken
a conscious decision to not do any of them. I strongly feel they should
be added only later, after we have agreed on the basic approach.
> > /*
> > * We don't have access to the parsetree here, so we have to
> > * generate the node type, object type, and command tag by
> > * decoding rte->requiredPerms and rte->relkind.
> > */
>
> This seems like it could be improved- have you considered suggesting a
> hook which is, perhaps, in a better place and could pass in this
> information instead of having to attempt to regenerate it?
No. (I don't see how that could work.)
> > /*
> > * If a role named "audit" exists, we check if it has been
> > * granted permission to perform the operation identified above.
> > * If so, we must log the event regardless of the static pgaudit
> > * settings.
> > */
>
> This isn't quite what was intended. This isn't about what the audit
> role could do but rather auditing specifically what the role has
> access to.
This is what you said earlier:
«The magic "audit" role has SELECT rights on a given table. When any user does a SELECT against that table,
ExecCheckRTPermsis called and there's a hook there which the module can use to say "ok, does the audit role have
anypermissions here?" and, if the result is yes, then the command is audited.»
As far as I can tell, that's exactly what the code does. In what way
have I misunderstood your suggestion now?
> As an example, if an UPDATE with a WHERE clause is executed and the
> audit user has SELECT rights on the table, then the UPDATE should be
> logged due to the WHERE clause.
Sorry, that makes my head hurt. I will think about it and respond later.
> > /*
> > * Create AuditEvents for certain kinds of CREATE and ALTER statements,
> > * as detected by log_object_access() in lieu of event trigger support
> > * for them.
> > */
>
> Just to comment on this, the object access framework feels like it
> should, perhaps, be reworked to be based on event triggers..
The point of that code in the object access hook is to stand in when
event triggers are not available.
> > log_function_execution(Oid objectId)
> > {
> > …
>
> This doesn't address overloaded functions, does it? It probably
> should include the arguments and types in 'name'.
OK. I'll look into that.
-- Abhijit
Abhijit,
* Abhijit Menon-Sen (ams@2ndQuadrant.com) wrote:
> At 2015-01-18 22:28:37 -0500, sfrost@snowman.net wrote:
> >
> > > 2. Set pgaudit.roles = 'r1, r2, …', which logs everything done by r1,
> > > r2, and any of their descendants.
> >
> > If these roles were the 'audit' roles as considered under #1 then
> > couldn't administrators control what pgaudit.roles provide today using
> > role memberships granted to the roles listed?
>
> Yes. But then there would be no convenient way to say "just log
> everything this particular role does".
I'm confused by this statement.. Having the role listed in
pgaudit.roles would still mean that everything that role does is logged.
Adding other roles to be audited would simply be 'GRANT audit TO
role1;'.
Is there a specific action which you don't think would be audited
through this?
> > > 3. Set pgaudit.log = 'read, write, …', which logs any events in any
> > > of the listed classes.
> >
> > Approach #1 addresses this too, doesn't it?
>
> Approach #1 applies only to things you can grant permissions for. What
> if you want to audit other commands?
You can grant roles to other roles, which is how the role-level
auditing would be handled. Hopefully that clarifies the idea here.
> > As currently implemented, role-level auditing is required to have DDL
> > be logged, but you may very well want to log all DDL and no DML, for
> > example.
>
> Well, as formerly implemented, you could do this by adding r1 to .roles
> and then setting .log appropriately. But you know that already and, if
> I'm not mistaken, have said you don't like it.
Right, because it's not flexible. You can't say that you want r1 to
have X actions logged, with r2 having Y actions logged.
> I'm all for making it possible to configure fine-grained auditing, but
> I don't think that should come at the expense of being able to whack
> things with a simpler, heavier^Wmore inclusive hammer if you want to.
I agree that it's valuable to support simple use-cases with simple
configurations.
> > My feeling is that we should strip down what goes into contrib to be
> > 9.5 based.
>
> I do not think I can express a constructive opinion about this. I will
> go along with whatever people agree is best.
One of the issues that I see is just how much of the code is under the
#ifdef's. If this is going into contrib, we really shouldn't have
references and large code blocks that are #ifdef'd out implementations
based on out-of-core patches. Further, the pieces which are under the
#ifdef's for DEPARSE would likely end up having to be under #if
PG_VERSION_NUM, as the deparse tree goes into 9.5. Really, pgaudit
pre-deparse and post-deparse are quite different and having them all in
one pgaudit.c ends up being pretty grotty. Have you considered
splitting pgaudit.c into multiple files, perhaps along the pre/post
deparse lines?
> > I'm also wondering if pieces of that are now out-of-date with where
> > core is.
>
> Yes, they are. I'll clean that up.
Ok, good.
> > I don't particularly like how pgaudit will need to be kept in sync
> > with what's in ProcessUtility (normal and slow).
>
> Sure, it's a pain.
>
> > I'd really like to see this additional information regarding what kind
> > a command is codified in core. Ideally, we'd have a single place
> > which tracks the kind of command and possibly other information which
> > can then be addressed all at once when new commands are added.
>
> What would this look like, and is it a realistic goal for 9.5?
One thought might be to provide the intersection between LOGSTMT and
ObjectTypes. We can learn what commands are DDL and what are
modification commands from GetCommandLogLevel. The other distinctions
are mostly about different object types and we might be able to use
ObjectPropertyType and ObjectTypeMap for that, perhaps adding another
'kind' to ObjectProperty for the object categorization.
There are still further distinctions and I agree that it's very useful
to be able to identify which commands are privilege-related
(T_GrantStmt, T_GrantRoleStmt, T_CreatePolicyStmt, etc) vs. things like
Vacuum.
> > Also, we could allow more granularity by using the actual classes
> > which we already have for objects.
>
> Explain?
That thought was based on looking at ObjectTypeMap- we might be able to
provide a way for administrators to configure which objects they want to
be audited by naming them using the names provided by ObjectTypeMap.
> > > /*
> > > * This module collects AuditEvents from various sources (event
> > > * triggers, and executor/utility hooks) and passes them to the
> > > * log_audit_event() function.
> > > *
> > > * An AuditEvent represents an operation that potentially affects a
> > > * single object. If an underlying command affects multiple objects,
> > > * multiple AuditEvents must be created to represent it.
> > > */
> >
> > The above doesn't appear to be accurate (perhaps it once was?) as
> > log_audit_event() only takes a single AuditEvent structure at a time
> > and it's not a list. I'm not sure that needs to change, just pointing
> > out that the comment appears to be inaccurate.
>
> If multiple AuditEvents are created (as the ExecutorCheckPerms_hook may
> do), log_audit_event() will be called multiple times. The comment is
> correct, but I'll try to make it more clear.
I agree that log_audit_event() gets called multiple times, but there's
no 'collection' of AuditEvents, ever, is there? An AuditEvent is
created and then passed to log_audit_event which then issues an ereport-
and that's it. "collects" above implies queueing is happening, at least
to me. Perhaps if you said "This module collects audit events" then
it'd be a bit clearer but as it stands, it's referencing a specific
structure.
> > I'm inclined to suggest that the decision be made earlier on about if
> > a given action should be logged, as the earlier we do that the less
> > work we have to do and we could avoid having to pass in things like
> > 'granted' to the log_audit_event function at all.
>
> I considered that, but it makes it much harder to review all of the
> places where such decisions are made. It's partly because I wrote the
> code in this way that I was able to quickly change it to work the way
> you suggested (at least once I understood what you suggested).
>
> I prefer the one-line function and a few «if (pgaudit_configured())»
> checks (to avoid creating AuditEvents if they won't be needed at all)
> and centralising the decision-making rather than spreading the latter
> around the whole code.
I understand what you're getting at, but I'm not sure that 3 places
really rises to 'around the whole code'. I'm also hopeful that it might
be possible to reduce that down to 2 places- commands which go through
the main Executor and therefore call under ExecutorCheckPerms and those
which go through ProcessUtility and fall under the ProcessUtility hook.
> > > /*
> > > * We don't have access to the parsetree here, so we have to
> > > * generate the node type, object type, and command tag by
> > > * decoding rte->requiredPerms and rte->relkind.
> > > */
> >
> > This seems like it could be improved- have you considered suggesting a
> > hook which is, perhaps, in a better place and could pass in this
> > information instead of having to attempt to regenerate it?
>
> No. (I don't see how that could work.)
Well, there's a number of other Executor hooks (Start/Run/Finish/End)
which are all passed QueryDesc, as an example. I'm not sure that you'd
actually want to deal with using those specific hooks, but another hook
which is just run (instead of an instead-of type hook) and gets the
QueryDesc would certainly give you access to a lot more information
directly without having to back into it from the range table.
> > > /*
> > > * If a role named "audit" exists, we check if it has been
> > > * granted permission to perform the operation identified above.
> > > * If so, we must log the event regardless of the static pgaudit
> > > * settings.
> > > */
> >
> > This isn't quite what was intended. This isn't about what the audit
> > role could do but rather auditing specifically what the role has
> > access to.
>
> This is what you said earlier:
>
> «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.»
>
> As far as I can tell, that's exactly what the code does. In what way
> have I misunderstood your suggestion now?
The key part above is "any". We're using the grant system as a proxy
for saying "I want to audit column X". That's different from "I want to
audit commands which would be allowed if I *only* had access to column
X". If I want to audit access to column X, then:
select A, B, X from mytable;
Should be audited, even if the audit role doesn't have access to columns
A or B.
> > As an example, if an UPDATE with a WHERE clause is executed and the
> > audit user has SELECT rights on the table, then the UPDATE should be
> > logged due to the WHERE clause.
>
> Sorry, that makes my head hurt. I will think about it and respond later.
Ok, hopefully the above helps. :)
> > > /*
> > > * Create AuditEvents for certain kinds of CREATE and ALTER statements,
> > > * as detected by log_object_access() in lieu of event trigger support
> > > * for them.
> > > */
> >
> > Just to comment on this, the object access framework feels like it
> > should, perhaps, be reworked to be based on event triggers..
>
> The point of that code in the object access hook is to stand in when
> event triggers are not available.
Yeah- but are we always going to have to deal with a partial event
trigger set? Shouldn't it be possible to cover the object access hook
cases with event triggers?
Thanks!
Stephen
At 2015-01-19 08:26:59 -0500, sfrost@snowman.net wrote: > > I'm confused by this statement.. Let me see if I've understood your clarification: Suppose you have pgaudit.roles set to 'r0, r1', and that you have granted r0 to u0. Now, if you're connected to the database as r0 or r1, then everything you do is logged. But if you're connected as u0, then only those things are logged that can be derived (in a manner discussed later) from the permissions explicitly granted to r0 (but not u0)? So when I'm trying to decide what to audit, I have to: (a) check if the current user is mentioned in .roles; if so, audit. (b) check if the current user is a descendant of one of the roles mentioned in .roles; if not, no audit. (c) check for permissions granted to the "root" role and see if that tells us to audit. Is that right? If so, it would work fine. I don't look forward to trying to explain it to people, but yes, it would work (for anything you could grant permissions for). > You can't say that you want r1 to have X actions logged, with r2 > having Y actions logged. Right. But how do you do that for non-GRANT-able actions in your scheme? For example, what if I want to see all the tables created and dropped by a particular user? > Have you considered splitting pgaudit.c into multiple files, perhaps > along the pre/post deparse lines? If such a split were done properly, then could the backwards-compatible version be more acceptable for inclusion in contrib in 9.5? If so, I'll look into it. > One thought might be to provide the intersection between LOGSTMT and > ObjectTypes. Hm. OK. > The key part above is "any". We're using the grant system as a proxy > for saying "I want to audit column X". That's different from "I want > to audit commands which would be allowed if I *only* had access to > column X". If I want to audit access to column X, then: > > select A, B, X from mytable; > > Should be audited, even if the audit role doesn't have access to > columns A or B. Yes, that's what the code already does (modulo handling of the audit role's oid, which I tweaked to match the behaviour described earlier in this mail). I did the following: create table x(a int,b int,c int); insert into x(a,b,c) values (1,2,3); create user y; grant select on x to y; create role x; grant select (a) on table x to x; grant x to y; Then, as user y, I did «select a,b,c from x» and «select b,c from x». Only the former was logged: LOG: AUDIT,2015-01-20 11:19:02.156441+05:30,postgres,y,y,READ,SELECT,TABLE,public.x,select a,b,c from x; > Yeah- but are we always going to have to deal with a partial event > trigger set? As a practical matter, yes. I don't know if there are current plans to extend event trigger support to the commands and object types that are yet unsupported. -- Abhijit
On Tue, Jan 20, 2015 at 1:05 AM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote: > So when I'm trying to decide what to audit, I have to: > > (a) check if the current user is mentioned in .roles; if so, audit. > > (b) check if the current user is a descendant of one of the roles > mentioned in .roles; if not, no audit. > > (c) check for permissions granted to the "root" role and see if that > tells us to audit. > > Is that right? If so, it would work fine. I don't look forward to trying > to explain it to people, but yes, it would work (for anything you could > grant permissions for). I think this points to fundamental weakness in the idea of doing this through the GRANT system. Some people are going want to audit everything a particular user does, some people are going to want to audit all access to particular objects, and some people will have more complicated requirements. Some people will want to audit even super-users, others especially super-users, others only non super-users. None of this necessarily matches up to the usual permissions framework. >> Have you considered splitting pgaudit.c into multiple files, perhaps >> along the pre/post deparse lines? > > If such a split were done properly, then could the backwards-compatible > version be more acceptable for inclusion in contrib in 9.5? If so, I'll > look into it. We're not going to include code in contrib that has leftovers in it for compatibility with earlier source trees. That's been discussed on this mailing list many times and the policy is clear. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 1/20/15 2:20 PM, Robert Haas wrote: > On Tue, Jan 20, 2015 at 1:05 AM, Abhijit Menon-Sen<ams@2ndquadrant.com> wrote: >> >So when I'm trying to decide what to audit, I have to: >> > >> > (a) check if the current user is mentioned in .roles; if so, audit. >> > >> > (b) check if the current user is a descendant of one of the roles >> > mentioned in .roles; if not, no audit. >> > >> > (c) check for permissions granted to the "root" role and see if that >> > tells us to audit. >> > >> >Is that right? If so, it would work fine. I don't look forward to trying >> >to explain it to people, but yes, it would work (for anything you could >> >grant permissions for). > I think this points to fundamental weakness in the idea of doing this > through the GRANT system. Some people are going want to audit > everything a particular user does, some people are going to want to > audit all access to particular objects, and some people will have more > complicated requirements. Some people will want to audit even > super-users, others especially super-users, others only non > super-users. None of this necessarily matches up to the usual > permissions framework. +1. In particular I'm very concerned with the idea of doing this via roles, because that would make it trivial for any superuserto disable auditing. The only good option I could see to provide this kind of flexibility would be allowing theuser to provide a function that accepts role, object, etc and make return a boolean. The performance of that would presumablysuck with anything but a C function, but we could provide some C functions to handle simple cases. That said, I think the best idea at this stage is either log everything or nothing. We can always expand upon that later. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
On Tue, Jan 20, 2015 at 5:03 PM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote: > +1. In particular I'm very concerned with the idea of doing this via roles, > because that would make it trivial for any superuser to disable auditing. > The only good option I could see to provide this kind of flexibility would > be allowing the user to provide a function that accepts role, object, etc > and make return a boolean. The performance of that would presumably suck > with anything but a C function, but we could provide some C functions to > handle simple cases. > > That said, I think the best idea at this stage is either log everything or > nothing. We can always expand upon that later. I think this is throwing the baby out with the bathwater. Neither C functions nor all-or-nothing are going to be of any practical use. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Abhijit,
* Abhijit Menon-Sen (ams@2ndQuadrant.com) wrote:
> At 2015-01-19 08:26:59 -0500, sfrost@snowman.net wrote:
> > I'm confused by this statement..
>
> Let me see if I've understood your clarification:
Thanks much for the example use-case and for working this through with
me. I actually think I've come up with a further specification which
might allow us to make this extremely flexible, but also simple for
those who want to keep it simple.
Consider this:
Everything is single-level to the roles mentioned in the GUC.
Is the logged in role a member of one of the GUC roles? Yes -> Audit
Now to cover the "user X for table Y" case:
Did any of the GUC value roles grant SELECT rights for this table to the
current role? Yes -> Audit SELECT on the table by the current role
Did any of the GUC value roles grant INSERT rights for this table to the
current role? Yes -> Audit INSERT on the table by the current role
etc.
For the 'log all access to an object' case, under this scheme, I'm
afraid we'd need some special role to GRANT the access to. We wouldn't
want that to simply be 'public' since then it might actually be
granted access rights that we don't want to. We can't simply use the
same role because you need to grant that role whatever access 'with
grant option' in order for it to be able to re-grant the privilege.
With the special role, it becomes:
Does the special role have SELECT rights on the table? Yes -> Audit SELECTs on the table
Does the special role have INSERT rights on the table? Yes -> Audit INSERTs on the table
> Suppose you have pgaudit.roles set to 'r0, r1', and that you have
> granted r0 to u0.
Not quite- I wasn't thinking you'd grant r0 to u0 but rather the other
way around- u0 is granted to r0. If you granted r0 to u0, then u0 would
have all of r0's rights which could be quite a bit larger than you want
u0 to have. It only works in the other direction.
> Now, if you're connected to the database as r0 or r1, then everything
> you do is logged. But if you're connected as u0, then only those things
> are logged that can be derived (in a manner discussed later) from the
> permissions explicitly granted to r0 (but not u0)?
> So when I'm trying to decide what to audit, I have to:
>
> (a) check if the current user is mentioned in .roles; if so, audit.
>
> (b) check if the current user is a descendant of one of the roles
> mentioned in .roles; if not, no audit.
>
> (c) check for permissions granted to the "root" role and see if that
> tells us to audit.
>
> Is that right? If so, it would work fine. I don't look forward to trying
> to explain it to people, but yes, it would work (for anything you could
> grant permissions for).
This is pretty close- (a) and (b) are mostly correct, though I would
strongly discourage users from actually logging in as an audit role.
The one caveat with (b) is that 'if not, no audit' is not correct- all
cases are essentially OR'd together when it comes to auditing. Roles
can be audited even if they are not descendants of the roles mentioned
in .roles.
Review the opening of this email though and consider that we could look
at "what privileges has the audit role granted to the current role?" as
defining what is to be audited.
> > You can't say that you want r1 to have X actions logged, with r2
> > having Y actions logged.
>
> Right. But how do you do that for non-GRANT-able actions in your scheme?
> For example, what if I want to see all the tables created and dropped by
> a particular user?
I hadn't been intending to address that with this scheme, but I think we
have that by looking for privilege grants where the audit role is the
grantee and the role-to-be-audited the grantor.
> > Have you considered splitting pgaudit.c into multiple files, perhaps
> > along the pre/post deparse lines?
>
> If such a split were done properly, then could the backwards-compatible
> version be more acceptable for inclusion in contrib in 9.5? If so, I'll
> look into it.
As Robert says, the short answer is 'no'- but it might make it easier to
get the 9.5 bits into 9.5.. :)
> > The key part above is "any". We're using the grant system as a proxy
> > for saying "I want to audit column X". That's different from "I want
> > to audit commands which would be allowed if I *only* had access to
> > column X". If I want to audit access to column X, then:
> >
> > select A, B, X from mytable;
> >
> > Should be audited, even if the audit role doesn't have access to
> > columns A or B.
>
> Yes, that's what the code already does (modulo handling of the audit
> role's oid, which I tweaked to match the behaviour described earlier
> in this mail). I did the following:
>
> create table x(a int,b int,c int);
> insert into x(a,b,c) values (1,2,3);
>
> create user y;
> grant select on x to y;
>
> create role x;
> grant select (a) on table x to x;
> grant x to y;
>
> Then, as user y, I did «select a,b,c from x» and «select b,c from x».
> Only the former was logged:
>
> LOG: AUDIT,2015-01-20 11:19:02.156441+05:30,postgres,y,y,READ,SELECT,TABLE,public.x,select a,b,c from x;
Ok, I had tried something similar and it didn't work for me and so I
ended up assuming it wasn't operating the way I was expecting.
> > Yeah- but are we always going to have to deal with a partial event
> > trigger set?
>
> As a practical matter, yes. I don't know if there are current plans to
> extend event trigger support to the commands and object types that are
> yet unsupported.
Well, I was primairly digging for someone to say "yes, the object access
stuff will be reworked to be based on event triggers, thus removing the
need for the object access bits". Let's just say that I'm still hopeful
for that. That won't get us completely away from ProcessUtility, but at
least we would only have to deal with that..
Thanks!
Stephen
* Robert Haas (robertmhaas@gmail.com) wrote:
> On Tue, Jan 20, 2015 at 1:05 AM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote:
> > So when I'm trying to decide what to audit, I have to:
> >
> > (a) check if the current user is mentioned in .roles; if so, audit.
> >
> > (b) check if the current user is a descendant of one of the roles
> > mentioned in .roles; if not, no audit.
> >
> > (c) check for permissions granted to the "root" role and see if that
> > tells us to audit.
> >
> > Is that right? If so, it would work fine. I don't look forward to trying
> > to explain it to people, but yes, it would work (for anything you could
> > grant permissions for).
>
> I think this points to fundamental weakness in the idea of doing this
> through the GRANT system. Some people are going want to audit
> everything a particular user does, some people are going to want to
> audit all access to particular objects, and some people will have more
> complicated requirements. Some people will want to audit even
> super-users, others especially super-users, others only non
> super-users. None of this necessarily matches up to the usual
> permissions framework.
I'm hopeful that my email from a few minutes ago provides a way to cover
all of the above, while still making it easy for users who just want to
say "audit everything this user does" or "audit everything which touches
this column".
Any auditing of superusers is going to be circumventable by those same
superusers if it's happening in the context of the PG process, so I'm
not sure why they would be any different under this scheme vs. some
other scheme.
Don't get me wrong- I agree completely that using the GRANT system isn't
ideal, but I don't see anyone proposing another way for an extension to
store metadata on catalog tables with the same granularity the GRANT
system provides and its dependency handling and ability to have default
values. We've been over that ground a number of times and I certainly
don't feel like we're any closer to solving it and I'd hate to see that
block pgaudit.
Put another way, if the requirement for pgaudit is that it has to solve
the metadata-on-catalogs-for-extensions problem, I think I'd rather just
move pgaudit into core instead, give it catalog tables, make it part of
the dependency system, provide syntax for it, etc. I'm pretty sure
that'd be easier and happen a lot faster.
Thanks!
Stephen
Jim,
* Jim Nasby (Jim.Nasby@BlueTreble.com) wrote:
> +1. In particular I'm very concerned with the idea of doing this via roles, because that would make it trivial for
anysuperuser to disable auditing. The only good option I could see to provide this kind of flexibility would be
allowingthe user to provide a function that accepts role, object, etc and make return a boolean. The performance of
thatwould presumably suck with anything but a C function, but we could provide some C functions to handle simple
cases.
Superusers will be able to bypass, trivially, anything that's done in
the process space of PG. The only possible exception to that being an
SELinux or similar solution, but I don't think that's what you were
getting at.
I certainly don't think having the user provide a C function to specify
what should be audited as making any sense- if they can do that, they
can use the same hooks pgaudit is using and skip the middle-man. As for
the performance concern you raise, I actually don't buy into it at all.
It's not like we worry about the performance of checking permissions on
objects in general and, for my part, I like to think that's because it's
pretty darn quick already.
> That said, I think the best idea at this stage is either log everything or nothing. We can always expand upon that
later.
We've already got those options and they are, clearly, insufficient for
a large number of our users.
Thanks,
Stephen
At 2015-01-20 20:36:39 -0500, robertmhaas@gmail.com wrote: > > I think this is throwing the baby out with the bathwater. Neither C > functions nor all-or-nothing are going to be of any practical use. Do you see some approach that has a realistic chance of making 9.5 and would also actually be worth putting into 9.5? Suggestions very welcome. -- Abhijit
[After a discussion on IRC with Stephen…] At 2015-01-20 21:47:02 -0500, sfrost@snowman.net wrote: > > Review the opening of this email though and consider that we could > look at "what privileges has the audit role granted to the current > role?" as defining what is to be audited. Right, I understand now how that would work. I'll try to find time to (a) implement this, (b) remove the backwards-compatibility code, and (c) split up the USE_DEPARSE_FUNCTIONS stuff. > > For example, what if I want to see all the tables created and > > dropped by a particular user? > > I hadn't been intending to address that with this scheme, but I think > we have that by looking for privilege grants where the audit role is > the grantee and the role-to-be-audited the grantor. For CREATE, yes, with a bit of extra ACL-checking code in the utility hook; but I don't think we'll get very far without the ability to log ALTER/DROP too. :-) So there has to be some alternative mechanism for that, and I'm hoping Robert (or anyone!) has something in mind. > Well, I was primairly digging for someone to say "yes, the object > access stuff will be reworked to be based on event triggers, thus > removing the need for the object access bits". (This doesn't entirely make sense to me, but it's tangential anyway, so I won't comment on it for now.) -- Abhijit
* Abhijit Menon-Sen (ams@2ndQuadrant.com) wrote:
> At 2015-01-20 21:47:02 -0500, sfrost@snowman.net wrote:
> > Review the opening of this email though and consider that we could
> > look at "what privileges has the audit role granted to the current
> > role?" as defining what is to be audited.
>
> Right, I understand now how that would work. I'll try to find time to
> (a) implement this, (b) remove the backwards-compatibility code, and
> (c) split up the USE_DEPARSE_FUNCTIONS stuff.
Great! Thanks!
> > > For example, what if I want to see all the tables created and
> > > dropped by a particular user?
> >
> > I hadn't been intending to address that with this scheme, but I think
> > we have that by looking for privilege grants where the audit role is
> > the grantee and the role-to-be-audited the grantor.
>
> For CREATE, yes, with a bit of extra ACL-checking code in the utility
> hook; but I don't think we'll get very far without the ability to log
> ALTER/DROP too. :-) So there has to be some alternative mechanism for
> that, and I'm hoping Robert (or anyone!) has something in mind.
ALTER/DROP can be logged based on the USAGE privilege for the schema.
We can't differentiate those cases but we can at least handle them as a
group.
Thanks!
Stephen
On 1/20/15 9:01 PM, Stephen Frost wrote: > * Jim Nasby (Jim.Nasby@BlueTreble.com) wrote: >> >+1. In particular I'm very concerned with the idea of doing this via roles, because that would make it trivial for anysuperuser to disable auditing. The only good option I could see to provide this kind of flexibility would be allowingthe user to provide a function that accepts role, object, etc and make return a boolean. The performance of thatwould presumably suck with anything but a C function, but we could provide some C functions to handle simple cases. > Superusers will be able to bypass, trivially, anything that's done in > the process space of PG. The only possible exception to that being an > SELinux or similar solution, but I don't think that's what you were > getting at. Not if the GUC was startup-only. That would allow someone with OS access to the server to prevent a Postgres superuser fromdisabling it. > I certainly don't think having the user provide a C function to specify > what should be audited as making any sense- if they can do that, they > can use the same hooks pgaudit is using and skip the middle-man. As for > the performance concern you raise, I actually don't buy into it at all. > It's not like we worry about the performance of checking permissions on > objects in general and, for my part, I like to think that's because it's > pretty darn quick already. I was only mentioning C because of performance concerns. If SQL or plpgsql is fast enough then there's no need. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
* Jim Nasby (Jim.Nasby@BlueTreble.com) wrote:
> On 1/20/15 9:01 PM, Stephen Frost wrote:
> >* Jim Nasby (Jim.Nasby@BlueTreble.com) wrote:
> >>>+1. In particular I'm very concerned with the idea of doing this via roles, because that would make it trivial for
anysuperuser to disable auditing. The only good option I could see to provide this kind of flexibility would be
allowingthe user to provide a function that accepts role, object, etc and make return a boolean. The performance of
thatwould presumably suck with anything but a C function, but we could provide some C functions to handle simple cases.
> >Superusers will be able to bypass, trivially, anything that's done in
> >the process space of PG. The only possible exception to that being an
> >SELinux or similar solution, but I don't think that's what you were
> >getting at.
>
> Not if the GUC was startup-only. That would allow someone with OS access to the server to prevent a Postgres
superuserfrom disabling it.
That is not accurate.
Being startup-only won't help if the user is a superuser.
> >I certainly don't think having the user provide a C function to specify
> >what should be audited as making any sense- if they can do that, they
> >can use the same hooks pgaudit is using and skip the middle-man. As for
> >the performance concern you raise, I actually don't buy into it at all.
> >It's not like we worry about the performance of checking permissions on
> >objects in general and, for my part, I like to think that's because it's
> >pretty darn quick already.
>
> I was only mentioning C because of performance concerns. If SQL or plpgsql is fast enough then there's no need.
If this is being done for every execution of a query then I agree- SQL
or plpgsql probably wouldn't be fast enough. That doesn't mean it makes
sense to have pgaudit support calling a C function, it simply means that
we need to find another way to configure auditing (which is what I think
I've done...).
Thanks,
Stephen
On 1/21/15 5:38 PM, Stephen Frost wrote: > * Jim Nasby (Jim.Nasby@BlueTreble.com) wrote: >> On 1/20/15 9:01 PM, Stephen Frost wrote: >>> * Jim Nasby (Jim.Nasby@BlueTreble.com) wrote: >>>>> +1. In particular I'm very concerned with the idea of doing this via roles, because that would make it trivial forany superuser to disable auditing. The only good option I could see to provide this kind of flexibility would be allowingthe user to provide a function that accepts role, object, etc and make return a boolean. The performance of thatwould presumably suck with anything but a C function, but we could provide some C functions to handle simple cases. >>> Superusers will be able to bypass, trivially, anything that's done in >>> the process space of PG. The only possible exception to that being an >>> SELinux or similar solution, but I don't think that's what you were >>> getting at. >> >> Not if the GUC was startup-only. That would allow someone with OS access to the server to prevent a Postgres superuserfrom disabling it. > > That is not accurate. > > Being startup-only won't help if the user is a superuser. Crap, I thought postgresql.auto.conf was handled as an #include and therefore you could still preempt it via postgresql.conf >>> I certainly don't think having the user provide a C function to specify >>> what should be audited as making any sense- if they can do that, they >>> can use the same hooks pgaudit is using and skip the middle-man. As for >>> the performance concern you raise, I actually don't buy into it at all. >>> It's not like we worry about the performance of checking permissions on >>> objects in general and, for my part, I like to think that's because it's >>> pretty darn quick already. >> >> I was only mentioning C because of performance concerns. If SQL or plpgsql is fast enough then there's no need. > > If this is being done for every execution of a query then I agree- SQL > or plpgsql probably wouldn't be fast enough. That doesn't mean it makes > sense to have pgaudit support calling a C function, it simply means that > we need to find another way to configure auditing (which is what I think > I've done...). I'm still nervous about overloading this onto the roles system; I think it will end up being very easy to accidentally break.But if others think it'll work then I guess I'm just being paranoid. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
* Jim Nasby (Jim.Nasby@BlueTreble.com) wrote:
> On 1/21/15 5:38 PM, Stephen Frost wrote:
> >Being startup-only won't help if the user is a superuser.
>
> Crap, I thought postgresql.auto.conf was handled as an #include and therefore you could still preempt it via
postgresql.conf
It's not just that.. Having superuser access should really be
considered equivilant to having a shell as the unix user that postgres
is running as.
> >If this is being done for every execution of a query then I agree- SQL
> >or plpgsql probably wouldn't be fast enough. That doesn't mean it makes
> >sense to have pgaudit support calling a C function, it simply means that
> >we need to find another way to configure auditing (which is what I think
> >I've done...).
>
> I'm still nervous about overloading this onto the roles system; I think it will end up being very easy to
accidentallybreak. But if others think it'll work then I guess I'm just being paranoid.
Break in which way..? If you're saying "it'll be easy for a user to
misconfigure" then I might agree with you- but documentation and
examples can help to address that. If you're worried that future PG
hacking will break it, well, I tend to doubt the GRANT piece is the area
of concern there- the recent development work is really around event
triggers and adding new object classes; the GRANT components have been
reasonably stable for the past few years.
Thanks!
Stephen
On Wed, Jan 21, 2015 at 7:05 PM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote: > I'm still nervous about overloading this onto the roles system; I think it > will end up being very easy to accidentally break. But if others think it'll > work then I guess I'm just being paranoid. I agree with you. I don't hear anyone who actually likes that proposal except for Stephen. The list of people who don't like it seems to include the patch author, which strikes me as a rather significant point. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Wed, Jan 21, 2015 at 1:42 AM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote: > At 2015-01-20 20:36:39 -0500, robertmhaas@gmail.com wrote: >> I think this is throwing the baby out with the bathwater. Neither C >> functions nor all-or-nothing are going to be of any practical use. > > Do you see some approach that has a realistic chance of making 9.5 and > would also actually be worth putting into 9.5? Suggestions very welcome. Well, I'm still of the view that there's little to lose by having this remain out-of-core for a release or three. We don't really all agree on what we want, and non-core code can evolve a lot faster than core code, so what's the rush? I'm coming around to the view that we're going to need fairly deep integration to make this work nicely. It seems to me that the natural way of controlling auditing of a table is with table or column options on that table, but that requires either an extensible reloptions framework that we don't have today, or that it be in the core server. Similarly, the nice way of controlling options on a user is some property attached to the user: audit operations X, Y, Z when performed by this user. If you held a gun to my head and said, we must have something this release, I'd probably go for a GUC with a value that is a comma-separated list of role:operation:object triplets, like this: audit_stuff = 'alice:*:*, *:delete:*, bob:*:table secret' ...read as: - Audit everything alice does. - Audit all deletes by anyone. - Audit all access by bob to table secret. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 1/21/15 6:50 PM, Stephen Frost wrote: >> I'm still nervous about overloading this onto the roles system; I think it will end up being very easy to accidentallybreak. But if others think it'll work then I guess I'm just being paranoid. > Break in which way..? If you're saying "it'll be easy for a user to > misconfigure" then I might agree with you- but documentation and > examples can help to address that. I'm worried about user misconfiguration. Setting up a good system of roles (as in, distinguishing between application accounts,users, account(s) used to deploy code changes, DBAs, etc) is already tricky because of all the different use casesto consider. I fear that adding auditing to that matrix is just going to make it worse. I do like Robert's idea of role:action:object triplets more, though I'm not sure it's enough. For example, what happens ifyou CREATE ROLE su SUPERUSER NOINHERIT NOLOGIN; CREATE ROLE su_role IN ROLE su NOLOGIN; GRANT su_role TO bob; and have su_role:*:* Does bob get audited all the time then? Only when he does SET ROLE su? For that matter, how does SET ROLE affect auditing? -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
* Robert Haas (robertmhaas@gmail.com) wrote:
> On Wed, Jan 21, 2015 at 7:05 PM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:
> > I'm still nervous about overloading this onto the roles system; I think it
> > will end up being very easy to accidentally break. But if others think it'll
> > work then I guess I'm just being paranoid.
>
> I agree with you. I don't hear anyone who actually likes that
> proposal except for Stephen. The list of people who don't like it
> seems to include the patch author, which strikes me as a rather
> significant point.
Just to clarify- this concept isn't actually mine but was suggested by a
pretty sizable PG user who has a great deal of familiarity with other
databases. I don't mean to try and invoke the 'silent majority' but
rather to make sure folks don't think this is my idea alone or that it's
only me who thinks it makes sense. :) Simon had weighed in earlier
with, iirc, a comment that he thought it was a good approach also,
though that was a while ago and things have changed.
I happen to like the idea specifically because it would allow regular
roles to change the auditing settings (no need to be a superuser or to
be able to modify postgresql.conf/postgresql.auto.conf), it would
provide the level of granularity desired, would follow table, column,
role changes, renames, drops, recreations, the dependency system would
function as expected, etc, while keeping it as an extension, which I
understood to be pretty desirable, especially initially.
* Robert Haas (robertmhaas@gmail.com) wrote:
> On Wed, Jan 21, 2015 at 1:42 AM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote:
> > At 2015-01-20 20:36:39 -0500, robertmhaas@gmail.com wrote:
> >> I think this is throwing the baby out with the bathwater. Neither C
> >> functions nor all-or-nothing are going to be of any practical use.
> >
> > Do you see some approach that has a realistic chance of making 9.5 and
> > would also actually be worth putting into 9.5? Suggestions very welcome.
>
> Well, I'm still of the view that there's little to lose by having this
> remain out-of-core for a release or three. We don't really all agree
> on what we want, and non-core code can evolve a lot faster than core
> code, so what's the rush?
Being out of core makes it unusable in production environments for a
large number of organizations. :/
> I'm coming around to the view that we're going to need fairly deep
> integration to make this work nicely. It seems to me that the natural
> way of controlling auditing of a table is with table or column options
> on that table, but that requires either an extensible reloptions
> framework that we don't have today, or that it be in the core server.
> Similarly, the nice way of controlling options on a user is some
> property attached to the user: audit operations X, Y, Z when performed
> by this user.
This is basically the same position which I held about a year ago when
we were discussing this, but there was quite a bit of push-back on
having an in-core solution, from the additional catalog tables to making
the grammar larger and slowing things down. For my 2c, auditing is more
than valuable enough to warrant those compromises, but I'm not anxious
to spend time developing an in-core solution only to get it shot down
after all the work has been put into it.
Still, if folks have come around to feeling like an in-core solution
makes sense, I'm certainly willing to work towards making that happen;
it's, after all, what I've been interested in for years. :)
> If you held a gun to my head and said, we must have something this
> release, I'd probably go for a GUC with a value that is a
> comma-separated list of role:operation:object triplets, like this:
>
> audit_stuff = 'alice:*:*, *:delete:*, bob:*:table secret'
>
> ...read as:
>
> - Audit everything alice does.
> - Audit all deletes by anyone.
> - Audit all access by bob to table secret.
And this approach doesn't address any of the issues mentioned above,
unfortunately, which would make it pretty difficult to really use. I
think I'd rather deal with pgaudit being outside of the tree than pursue
an approach which has so many issues.
Thanks!
Stephen
* Jim Nasby (Jim.Nasby@BlueTreble.com) wrote:
> On 1/21/15 6:50 PM, Stephen Frost wrote:
> >>I'm still nervous about overloading this onto the roles system; I think it will end up being very easy to
accidentallybreak. But if others think it'll work then I guess I'm just being paranoid.
> >Break in which way..? If you're saying "it'll be easy for a user to
> >misconfigure" then I might agree with you- but documentation and
> >examples can help to address that.
>
> I'm worried about user misconfiguration. Setting up a good system of roles (as in, distinguishing between application
accounts,users, account(s) used to deploy code changes, DBAs, etc) is already tricky because of all the different use
casesto consider. I fear that adding auditing to that matrix is just going to make it worse.
Even with an in-core solution, users would need to work out who should
be able to configure auditing.. I agree that seeing the permission
grants to the auditing roles might be confusing for folks who have not
seen it before, but I think that'll quickly resolve itself since the
only people who would see that are those who want to use pgaudit...
> I do like Robert's idea of role:action:object triplets more, though I'm not sure it's enough. For example, what
happensif you
I'd suggest considering what happens if you:
ALTER ROLE su_role RENAME TO new_su_role;
Or if you want to grant a non-superuser the ability to modify the
auditing rules..
Thanks,
Stephen
On 1/23/15 12:16 PM, Stephen Frost wrote: > Just to clarify- this concept isn't actually mine but was suggested by a > pretty sizable PG user who has a great deal of familiarity with other > databases. I don't mean to try and invoke the 'silent majority' but > rather to make sure folks don't think this is my idea alone or that it's > only me who thinks it makes sense.:) Simon had weighed in earlier > with, iirc, a comment that he thought it was a good approach also, > though that was a while ago and things have changed. I know there's definitely demand for auditing. I'd love to see us support it. > I happen to like the idea specifically because it would allow regular > roles to change the auditing settings (no need to be a superuser or to > be able to modify postgresql.conf/postgresql.auto.conf) Is there really a use case for non-superusers to be able to change auditing config? That seems like a bad idea. Also, was there a solution to how to configure auditing on specific objects with a role-based mechanism? I think we reallydo need something akin to role:action:object tuples, and I don't see how to do that with roles alone. BTW, I'm starting to feel like this needs a wiki page to get the design pulled together. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
* Jim Nasby (Jim.Nasby@BlueTreble.com) wrote:
> On 1/23/15 12:16 PM, Stephen Frost wrote:
> >Just to clarify- this concept isn't actually mine but was suggested by a
> >pretty sizable PG user who has a great deal of familiarity with other
> >databases. I don't mean to try and invoke the 'silent majority' but
> >rather to make sure folks don't think this is my idea alone or that it's
> >only me who thinks it makes sense.:) Simon had weighed in earlier
> >with, iirc, a comment that he thought it was a good approach also,
> >though that was a while ago and things have changed.
>
> I know there's definitely demand for auditing. I'd love to see us support it.
>
> >I happen to like the idea specifically because it would allow regular
> >roles to change the auditing settings (no need to be a superuser or to
> >be able to modify postgresql.conf/postgresql.auto.conf)
>
> Is there really a use case for non-superusers to be able to change auditing config? That seems like a bad idea.
What's a bad idea is having every auditor on the system running around
as superuser..
> Also, was there a solution to how to configure auditing on specific objects with a role-based mechanism? I think we
reallydo need something akin to role:action:object tuples, and I don't see how to do that with roles alone.
That is supported with the grant-based proposal.
> BTW, I'm starting to feel like this needs a wiki page to get the design pulled together.
I agree with that and was planning to offer help with the documentation
and building of such a wiki with examples, etc, once the implementation
was far enough along to demonstrate that the design will actually work..
Thanks!
Stephen
On Fri, Jan 23, 2015 at 1:16 PM, Stephen Frost <sfrost@snowman.net> wrote: >> Well, I'm still of the view that there's little to lose by having this >> remain out-of-core for a release or three. We don't really all agree >> on what we want, and non-core code can evolve a lot faster than core >> code, so what's the rush? > > Being out of core makes it unusable in production environments for a > large number of organizations. :/ Tough. Once we accept something into core, we're stuck maintaining it forever. We shouldn't do that unless we're absolutely confident the design is solid. We are not close to having consensus on this. If somebody has a reason for accepting only core PostgreSQL and not add-ons, it's either a stupid reason, or it's because they believe that the core stuff will be better thought-out than the add-ons. If it's the latter, we shouldn't disappoint. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 1/23/15 2:15 PM, Stephen Frost wrote: >>> > >I happen to like the idea specifically because it would allow regular >>> > >roles to change the auditing settings (no need to be a superuser or to >>> > >be able to modify postgresql.conf/postgresql.auto.conf) >> > >> >Is there really a use case for non-superusers to be able to change auditing config? That seems like a bad idea. > What's a bad idea is having every auditor on the system running around > as superuser.. When it comes to looking at auditing data, I agree. When it comes to changing auditing settings, I think that needs to be very restrictive. Really, it should be more (or differently)restrictive than SU, so that you can effectively audit your superusers with minimal worries about superuserstampering with auditing. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
Robert,
* Robert Haas (robertmhaas@gmail.com) wrote:
> On Fri, Jan 23, 2015 at 1:16 PM, Stephen Frost <sfrost@snowman.net> wrote:
> >> Well, I'm still of the view that there's little to lose by having this
> >> remain out-of-core for a release or three. We don't really all agree
> >> on what we want, and non-core code can evolve a lot faster than core
> >> code, so what's the rush?
> >
> > Being out of core makes it unusable in production environments for a
> > large number of organizations. :/
>
> Tough. Once we accept something into core, we're stuck maintaining it
> forever. We shouldn't do that unless we're absolutely confident the
> design is solid. We are not close to having consensus on this. If
> somebody has a reason for accepting only core PostgreSQL and not
> add-ons, it's either a stupid reason, or it's because they believe
> that the core stuff will be better thought-out than the add-ons. If
> it's the latter, we shouldn't disappoint.
I don't disagree with you about any of that. I don't think you disagree
with my comment either. What I'm not entirely clear on is how consensus
could be reached. Leaving it dormant for the better part of a year
certainly doesn't appear to have helped that situation. We've discussed
having it be part of the main server and having it be a contrib module
and until about a week ago, I had understood that having it in contrib
would be preferrable. Based on the recent emails, it appears there's
been a shift of preference to having it be in-core, but clearly there's
no time left to do that in this release cycle.
While I appreciate that it doesn't quite feel right to use the GRANT
system in the way I'm suggesting, I'm of the opinion that it's mostly
due to a lack of implementation with documentation and examples about
how it would work. Using the GRANT system gives us nearly the best of
both worlds- an SQL-based syntax, a very fine-grained configuration
method, dependency handling, rename handling, and means you don't need
to be a superuser or have to restart the server to change the config,
nor is there any need to deal with CSV configuration via GUCs.
For my part, I'd still prefer an in-core system with top-level syntax
(eg: AUDIT), but that could clearly come later even if we included
pgaudit today (as Tom and others pointed out to me early this past
summer). Auditing should definitely be a top-level capability in PG and
shouldn't be relegated to external modules or commercial solutions.
I've come around to feel that perhaps the first step should be a contrib
module rather than going in-core from the beginning as we'd get the
feedback which could lead to a better in-core solution. I don't think
we're going to get it perfectly right the first time, either way, and
having it be completely outside the tree will mean we won't get any real
feedback on it.
Thanks,
Stephen
Jim,
* Jim Nasby (Jim.Nasby@BlueTreble.com) wrote:
> When it comes to changing auditing settings, I think that needs to be very restrictive. Really, it should be more (or
differently)restrictive than SU, so that you can effectively audit your superusers with minimal worries about
superuserstampering with auditing.
I continue to be of the opinion that you're not going to be able to
effectively audit your superusers with any mechanism that resides inside
of the process space which superusers control. If you want to audit
superusers, you need something that operates outside of the postgres
process space. I'm certainly interested in that, but it's an orthogonal
discussion to anything we're talking about here.
Thanks,
Stephen
On Mon, Jan 26, 2015 at 5:21 PM, Stephen Frost <sfrost@snowman.net> wrote: > I don't disagree with you about any of that. I don't think you disagree > with my comment either. What I'm not entirely clear on is how consensus > could be reached. Leaving it dormant for the better part of a year > certainly doesn't appear to have helped that situation. We've discussed > having it be part of the main server and having it be a contrib module > and until about a week ago, I had understood that having it in contrib > would be preferrable. Based on the recent emails, it appears there's > been a shift of preference to having it be in-core, but clearly there's > no time left to do that in this release cycle. Well, I'm not sure that anyone else here agreed with me on that, and one person does not a consensus make no matter who it is. The basic problem here is that we don't seem to have even two people here who agree on how this ought to be done. The basic dynamic here seems to be you asking for changes and Abhijit making them but without any real confidence, and I don't feel good about that. I'm willing to defer to an emerging consensus here when there is one, but what Abhijit likes best is not a consensus, and neither is what you like, and neither is what I like. What we need is some people agreeing with each other. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
At 2015-01-26 17:45:52 -0500, robertmhaas@gmail.com wrote: > > > Based on the recent emails, it appears there's been a shift of > > preference to having it be in-core […] > > Well, I'm not sure that anyone else here agreed with me on that Sure, an in-core AUDIT command would be great. Stephen has always said that would be the most preferable solution; and if we had the code to implement it, I doubt anyone would prefer the contrib module instead. But we don't have that code now, and we won't have it in time for 9.5. We had an opportunity to work on pgaudit in its current form, and I like to think that the result is useful. To me, the question has always been whether some variant of that code would be acceptable for 9.5's contrib. If so, I had some time to work on that. If not… well, hard luck. But the option to implement AUDIT was not available to me, which is why I have not commented much on it recently. > The basic dynamic here seems to be you asking for changes and Abhijit > making them but without any real confidence, and I don't feel good > about that. I understand how I might have given you that impression, but I didn't mean to, and I don't really feel that way. I appreciate Stephen's suggestions and, although it took me some time to understand them fully, I think the use of GRANT to provide finer-grained auditing configuration has improved pgaudit. I am slightly concerned by the resulting complexity, but I think that can be addressed by examples and so on. I wouldn't be unhappy if this code were to go into contrib. (I should point out that it is also not the case that I do not hold any opinions and would be happy with anything pgaudit-shaped being included. For example, I strongly prefer GRANT to the 'alice:*:*' approach.) Anyway, I think it's reasonably clear now that pgaudit is unlikely to make it into 9.5 in any form, so I'll find something else to do. -- Abhijit
On 1/26/15 4:45 PM, Robert Haas wrote: > On Mon, Jan 26, 2015 at 5:21 PM, Stephen Frost <sfrost@snowman.net> wrote: >> I don't disagree with you about any of that. I don't think you disagree >> with my comment either. What I'm not entirely clear on is how consensus >> could be reached. Leaving it dormant for the better part of a year >> certainly doesn't appear to have helped that situation. We've discussed >> having it be part of the main server and having it be a contrib module >> and until about a week ago, I had understood that having it in contrib >> would be preferrable. Based on the recent emails, it appears there's >> been a shift of preference to having it be in-core, but clearly there's >> no time left to do that in this release cycle. > > Well, I'm not sure that anyone else here agreed with me on that, and > one person does not a consensus make no matter who it is. The basic > problem here is that we don't seem to have even two people here who > agree on how this ought to be done. The basic dynamic here seems to > be you asking for changes and Abhijit making them but without any real > confidence, and I don't feel good about that. I'm willing to defer to > an emerging consensus here when there is one, but what Abhijit likes > best is not a consensus, and neither is what you like, and neither is > what I like. What we need is some people agreeing with each other. BTW, I know that at least earlier versions of EnterpriseDB's version of Postgres (circa 2007) had an auditing feature. Inever dealt with any customers who were using it when I was there, but perhaps some other folks could shed some light onwhat customers wanted to see an an auditing feature... (I'm looking at you, Jimbo!) -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
On Tue, Jan 27, 2015 at 5:55 PM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote: > BTW, I know that at least earlier versions of EnterpriseDB's version of > Postgres (circa 2007) had an auditing feature. I never dealt with any > customers who were using it when I was there, but perhaps some other folks > could shed some light on what customers wanted to see an an auditing > feature... (I'm looking at you, Jimbo!) It's still there, but it's nothing like pgaudit. What I hear is that our customers are looking for something that has the capabilities of DBMS_FGA. I haven't researched either that or pgaudit enough to know how similar they are. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 1/27/15 5:06 PM, Robert Haas wrote: > On Tue, Jan 27, 2015 at 5:55 PM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote: >> BTW, I know that at least earlier versions of EnterpriseDB's version of >> Postgres (circa 2007) had an auditing feature. I never dealt with any >> customers who were using it when I was there, but perhaps some other folks >> could shed some light on what customers wanted to see an an auditing >> feature... (I'm looking at you, Jimbo!) > > It's still there, but it's nothing like pgaudit. What I hear is that > our customers are looking for something that has the capabilities of > DBMS_FGA. I haven't researched either that or pgaudit enough to know > how similar they are. Do they really need the full capabilities of DBMS_FGA? At first glance, it looks even more sophisticated than anything that'sbeen discussed so far. To wit (from http://docs.oracle.com/cd/B19306_01/appdev.102/b14258/d_fga.htm#CDEIECAG): DBMS_FGA.ADD_POLICY( object_schema VARCHAR2, object_name VARCHAR2, policy_name VARCHAR2, audit_condition VARCHAR2, audit_column VARCHAR2, handler_schema VARCHAR2, handler_module VARCHAR2, enable BOOLEAN, statement_types VARCHAR2, audit_trail BINARY_INTEGER IN DEFAULT, audit_column_opts BINARY_INTEGER IN DEFAULT); -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
On Tue, Jan 27, 2015 at 6:54 PM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote: > On 1/27/15 5:06 PM, Robert Haas wrote: >> >> On Tue, Jan 27, 2015 at 5:55 PM, Jim Nasby <Jim.Nasby@bluetreble.com> >> wrote: >>> >>> BTW, I know that at least earlier versions of EnterpriseDB's version of >>> Postgres (circa 2007) had an auditing feature. I never dealt with any >>> customers who were using it when I was there, but perhaps some other >>> folks >>> could shed some light on what customers wanted to see an an auditing >>> feature... (I'm looking at you, Jimbo!) >> >> >> It's still there, but it's nothing like pgaudit. What I hear is that >> our customers are looking for something that has the capabilities of >> DBMS_FGA. I haven't researched either that or pgaudit enough to know >> how similar they are. > > > Do they really need the full capabilities of DBMS_FGA? At first glance, it > looks even more sophisticated than anything that's been discussed so far. To > wit (from > http://docs.oracle.com/cd/B19306_01/appdev.102/b14258/d_fga.htm#CDEIECAG): > > DBMS_FGA.ADD_POLICY( > object_schema VARCHAR2, > object_name VARCHAR2, > policy_name VARCHAR2, > audit_condition VARCHAR2, > audit_column VARCHAR2, > handler_schema VARCHAR2, > handler_module VARCHAR2, > enable BOOLEAN, > statement_types VARCHAR2, > audit_trail BINARY_INTEGER IN DEFAULT, > audit_column_opts BINARY_INTEGER IN DEFAULT); I don't know. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 1/27/15 4:08 AM, Abhijit Menon-Sen wrote: > Anyway, I think it's reasonably clear now that pgaudit is unlikely to > make it into 9.5 in any form, so I'll find something else to do. That's unfortunate. I've been following this thread for a while with some interest (and anticipation). The role-base approach being considered may strike some as a misuse of the role system, but to my eye it is syntactically very close to how Oracle does auditing prior to 12c. Say you wanted to audit selects on the table hr.employee: Oracle: AUDIT SELECT ON hr.employee; pgaudit: GRANT SELECT ON hr.employee TO audit; (assuming audit is the role defined by pgaudit.roles) Object-based auditing in Oracle would be very easy to migrate to the grants needed for pgaudit. In addition, if an AUDIT command were introduced later in core, it would be easy to migrate from pgaudit to AUDIT assuming the syntax was similar to grant, which seems plausible. Unified auditing in 12c brings together the AUDIT command and DBMS_FGA under the concept of audit polices. That type of granularity might be something to shoot for eventually, but I think having a way to do auditing similar to what is available in pre-12c covers most use cases and would certainly be a big step forward for Postgres. -- - David Steele david@pgmasters.net
On 2/2/15 3:49 PM, David Steele wrote:
The role-base approach being considered may strike some as a misuse of the role system, but to my eye it is syntactically very close to how Oracle does auditing prior to 12c. Say you wanted to audit selects on the table hr.employee: Oracle: AUDIT SELECT ON hr.employee; pgaudit: GRANT SELECT ON hr.employee TO audit; (assuming audit is the role defined by pgaudit.roles) Object-based auditing in Oracle would be very easy to migrate to the grants needed for pgaudit. In addition, if an AUDIT command were introduced later in core, it would be easy to migrate from pgaudit to AUDIT assuming the syntax was similar to grant, which seems plausible.
I decided to take a whack at this and see what I could come up with, starting with the code in master at https://github.com/2ndQuadrant/pgaudit. I modified pgaudit.log to work similarly to Oracle's session-level logging, meaning user statements are logged instead of tables which are accessed. pgaudit.log still has the various classes of commands and only those commands which match one of the classes are logged. Note that the pgaudit.log GUC is SUSET but can be set at the cluster, database, or user level. An example - log all statements that create an object or read data: DB: connect user postgres, database postgres SQL: set pgaudit.log = 'DEFINITION, READ' SQL: create user user1 DB: connect user user1, database postgres SQL: create table account ( id int, name text, password text, description text ); AUDIT: SESSION,DEFINITION,CREATE TABLE,TABLE,public.account,<sql> SQL: select * from account; AUDIT: SESSION,READ,SELECT,,,<statement> SQL: insert into account (id, name, password, description) values (1, 'user1', 'HASH1', 'blah, blah'); AUDIT: <nothing logged> Object auditing is done via the grant system (similar to Oracle object auditing), but now there is now a single audit role (defined by the pgaudit.role GUC which can also be set at the cluster, database, or user level). An example - using column-level grants since they are more interesting: DB: connect user postgres, database postgres SQL: set pgaudit.log = 'NONE' SQL: create role audit SQL: set pgaudit.role = 'audit' DB: connect user user1, database postgres SQL: grant select (password) on public.account to audit; AUDIT: <nothing logged> SQL: select id, name from account; AUDIT: <nothing logged> SQL: select password from account; AUDIT: OBJECT,READ,SELECT,TABLE,public.account,<sql> SQL: grant update (name, password) on public.account to audit; AUDIT: <nothing logged> SQL: update account set description = 'yada, yada'; AUDIT: <nothing logged> SQL: update account set password = 'HASH2'; AUDIT: OBJECT,WRITE,UPDATE,TABLE,public.account,<sql> Session and object auditing can be used together so a statement that does not match on an object may still be session logged depending on the settings. An example - in this case the pgaudit.log GUC will be set on the user and grants persist from the previous example. Another table is added to show how that affects logging: DB: connect user postgres, database postgres SQL: alter role user1 set pgaudit.log = 'READ,WRITE'; AUDIT: <nothing logged> DB: connect user user1, database postgres SQL: create table account_role_map ( account_id int, role_id int ); AUDIT: <nothing logged> SQL: grant select on public.account_role_map to audit; AUDIT: <nothing logged> SQL: select account.password, account_role_map.role_id from account inner join account_role_map on account.id = account_role_map.account_id AUDIT: SESSION,READ,SELECT,,,<sql> AUDIT: OBJECT,READ,SELECT,TABLE,public.account,<sql> AUDIT: OBJECT,READ,SELECT,TABLE,public.account_role_map,<sql> SQL: update account set description = 'yada, yada'; AUDIT: SESSION,WRITE,UPDATE,,,<sql> SQL: update account set description = 'yada, yada' where password = 'HASH2'; AUDIT: SESSION,WRITE,UPDATE,,,<sql> AUDIT: OBJECT,WRITE,UPDATE,TABLE,public.account,<sql> That about covers it. I'd be happy to create a pull request to contribute the code back to 2ndQuadrant. There are some things I'm still planning to do, but I think this draft looks promising. pgaudit.c is attached. Thoughts and suggestions are welcome.-- - David Steele david@pgmasters.net
Attachment
<div dir="ltr"><br /><div class="gmail_extra"><br /><div class="gmail_quote">On Tue, Jan 27, 2015 at 6:08 PM, Abhijit Menon-Sen<span dir="ltr"><<a href="mailto:ams@2ndquadrant.com" target="_blank">ams@2ndquadrant.com</a>></span> wrote:<br/><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Anyway,I think it's reasonably clear now that pgaudit is unlikely to<br /> make it into 9.5 in anyform, so I'll find something else to do.<span class="HOEnZb"><font color="#888888"><br /></font></span></blockquote></div><br/></div><div class="gmail_extra">Well, I am marking this patch as rejected then...Let's in any case the discussion continue. Perhaps we could reach a clearer spec about what we want and how to shapeit.<br />-- <br /><div class="gmail_signature">Michael<br /></div></div></div>
Hi list, On 20/01/15 23:03, Jim Nasby wrote:> On 1/20/15 2:20 PM, Robert Haas wrote: >> On Tue, Jan 20, 2015 at 1:05 AM, Abhijit >> Menon-Sen<ams@2ndquadrant.com> wrote: >>> >So when I'm trying to decide what to audit, I have to: >>> > >>> > (a) check if the current user is mentioned in .roles; if so, >>> audit. >>> > >>> > (b) check if the current user is a descendant of one of the roles >>> > mentioned in .roles; if not, no audit. >>> > >>> > (c) check for permissions granted to the "root" role and see if >>> that >>> > tells us to audit. >>> > >>> >Is that right? If so, it would work fine. I don't look forward to >>> trying >>> >to explain it to people, but yes, it would work (for anything you could >>> >grant permissions for). >> I think this points to fundamental weakness in the idea of doing this >> through the GRANT system. Some people are going want to audit >> everything a particular user does, some people are going to want to >> audit all access to particular objects, and some people will have more >> complicated requirements. Some people will want to audit even >> super-users, others especially super-users, others only non >> super-users. None of this necessarily matches up to the usual >> permissions framework. > > +1. In particular I'm very concerned with the idea of doing this via > roles, because that would make it trivial for any superuser to disable > auditing. Rejecting the audit administration through the GRANT system, on the grounds that it easy for the superuser to disable it, seems unreasonable to me, since superusers are different from non-superusers in a fundamental way. The superuser operates on the administration level of the database, in contrast with users that need access to the actual information in the data to perform their job. An organization that wants to implement an auditing strategy needs to think in different terms to audit user/application level actions, and administrator/superuser actions. Consequently it should be no surprise when PostgreSQL mechanisms for auditing behave different for superusers vs non superusers. The patch as it is, is targeted at auditing user/application level access to the database, and as such it matches the use case of auditing user actions. Auditing superuser access means auditing beyond the running database. The superuser can dump a table, and pipe this data everywhere outside of the auditing domain. I cannot begin to imagine the kind of security restrictions you'd need to audit what happens with data after libpq has produced the results. My best guess would be to incorporate some kind of separation of duty mechanism; only allow certain superuser operations with two people involved. regards, Yeb Havinga
On 2/17/15 4:40 AM, Yeb Havinga wrote: > On 20/01/15 23:03, Jim Nasby wrote:> On 1/20/15 2:20 PM, Robert Haas wrote: >>> On Tue, Jan 20, 2015 at 1:05 AM, Abhijit >>> Menon-Sen<ams@2ndquadrant.com> wrote: >>>>> So when I'm trying to decide what to audit, I have to: >>>>> >>>>> (a) check if the current user is mentioned in .roles; if so, >>>> audit. >>>>> >>>>> (b) check if the current user is a descendant of one of the roles >>>>> mentioned in .roles; if not, no audit. >>>>> >>>>> (c) check for permissions granted to the "root" role and see if >>>> that >>>>> tells us to audit. >>>>> >>>>> Is that right? If so, it would work fine. I don't look forward to >>>> trying >>>>> to explain it to people, but yes, it would work (for anything you could >>>>> grant permissions for). >>> I think this points to fundamental weakness in the idea of doing this >>> through the GRANT system. Some people are going want to audit >>> everything a particular user does, some people are going to want to >>> audit all access to particular objects, and some people will have more >>> complicated requirements. Some people will want to audit even >>> super-users, others especially super-users, others only non >>> super-users. None of this necessarily matches up to the usual >>> permissions framework. >> >> +1. In particular I'm very concerned with the idea of doing this via >> roles, because that would make it trivial for any superuser to disable >> auditing. > > Rejecting the audit administration through the GRANT system, on the > grounds that it easy for the superuser to disable it, seems unreasonable > to me, since superusers are different from non-superusers in a > fundamental way. Agreed. This logic could be applied to any database object: why have tables when a superuser can so easily drop them and cause data loss? > The patch as it is, is targeted at auditing user/application level > access to the database, and as such it matches the use case of auditing > user actions. Exactly. This patch (and my rework) are focused entirely on auditing the actions of normal users in the database. While auditing can be enabled for superusers, there's no guarantee that it's reliable since it would be easy for a superuser to disable. Normal users can be configured to not have that capability, so auditing them is reliable. -- - David Steele david@pgmasters.net
Yeb,
* Yeb Havinga (yebhavinga@gmail.com) wrote:
> On 20/01/15 23:03, Jim Nasby wrote:> On 1/20/15 2:20 PM, Robert Haas wrote:
> > +1. In particular I'm very concerned with the idea of doing this via
> > roles, because that would make it trivial for any superuser to disable
> > auditing.
>
> Rejecting the audit administration through the GRANT system, on the
> grounds that it easy for the superuser to disable it, seems unreasonable
> to me, since superusers are different from non-superusers in a
> fundamental way.
Agreed.
> The patch as it is, is targeted at auditing user/application level
> access to the database, and as such it matches the use case of auditing
> user actions.
Right, and that's a *very* worthwhile use-case.
> Auditing superuser access means auditing beyond the running database.
Exactly! :)
Thanks!
Stephen
On 17 February 2015 at 14:44, Stephen Frost <sfrost@snowman.net> wrote: >> The patch as it is, is targeted at auditing user/application level >> access to the database, and as such it matches the use case of auditing >> user actions. > > Right, and that's a *very* worthwhile use-case. Agreed. So, we are still at the same place we were at 7-8 months ago: Some people would like an AUDIT command, but this isn't it. We have neither a design nor a developer willing to implement it (or funding to do so). That may change in the future, but if it does, we will not have auditing in production version of Postgres before September 2016, earliest if we wait for that. I vote to include pgaudit in 9.5, albeit with any changes. In particular, David may have some changes to recommend, but I haven't seen a spec or a patch, just a new version of code (which isn't how we do things...). In my understanding, the following people are in favour of pgaudit, in some form: Simon, Yeb, David, Stephen and other developers have spoken earlier in favour of including it. Abhijit, Jim and Robert have voiced recent doubts of various kinds, but there seems to be no outstanding objection to including pgaudit, only a wish that we had something better. (Please correct me). I'm happy to do final review and commit. Assuming we are in agreement, what changes are needed prior to commit? -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, RemoteDBA, Training & Services
* Simon Riggs (simon@2ndQuadrant.com) wrote:
> I vote to include pgaudit in 9.5, albeit with any changes. In
> particular, David may have some changes to recommend, but I haven't
> seen a spec or a patch, just a new version of code (which isn't how we
> do things...).
Hrm. I thought David's new patch actually looked quite good and it's
certainly quite a bit different from the initial patch (which didn't
seem like it was moving forward..). Guess I'm confused how a new patch
is different from a 'new version of code' and I didn't see a spec for
either patch. From the old thread, David had offered to submit a pull
request if there was interest and I didn't see any response...
> I'm happy to do final review and commit. Assuming we are in agreement,
> what changes are needed prior to commit?
I'm all about getting something done here for 9.5 also and would
certainly prefer to focus on that.
The recent discussion has all moved towards the approach that I was
advocating where we use GRANT simimlar to how AUDIT exists in other
RDBMS's. Both the latest version of the code from Abhijit and David's
code do that and I found what David did quite easy to follow- no big
#ifdef blocks (something I complained about earlier but didn't see any
progress on..) and no big switch statements that would likely get
out-dated very quickly. I'm not against going back to the code
submitted by Abhijit, if it's cleaned up and has the #ifdef blocks and
whatnot removed that were discussed previously. I don't fault David for
moving forward though, given the lack of feedback.
Perhaps there's an issue where the classes provided by David's approach
aren't granular enough but it's certainly better than what we have
today. The event-trigger based approach depends on as-yet-uncommitted
code, as I understand it. I'd certainly rather have fewer audit classes
which cover everything than more audit classes which end up not covering
everything because we don't have all the deparse code or event triggers
we need completed and committed yet.
Thanks!
Stephen
On 17 February 2015 at 15:52, Stephen Frost <sfrost@snowman.net> wrote: > * Simon Riggs (simon@2ndQuadrant.com) wrote: >> I vote to include pgaudit in 9.5, albeit with any changes. In >> particular, David may have some changes to recommend, but I haven't >> seen a spec or a patch, just a new version of code (which isn't how we >> do things...). > > Hrm. I thought David's new patch actually looked quite good and it's > certainly quite a bit different from the initial patch (which didn't > seem like it was moving forward..). Guess I'm confused how a new patch > is different from a 'new version of code' and I didn't see a spec for > either patch. From the old thread, David had offered to submit a pull > request if there was interest and I didn't see any response... My comment was that the cycle of development is discuss then develop. David's work is potentially useful, but having two versions of a feature slows things down. Since he is new to development here, I have made those comments so he understands, not so you would pick up on that. "didn't seem to be moving forwards" is strange comment. We usually wait for patches to stabilise, not for them to keep changing as evidence of worth. David, please submit your work to pgsql-hackers as a patch on Abhijit's last version. There is no need for a pull request to 2ndQuadrant. Thanks. -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, RemoteDBA, Training & Services
On 2/17/15 10:23 AM, Simon Riggs wrote: > I vote to include pgaudit in 9.5, albeit with any changes. In > particular, David may have some changes to recommend, but I haven't > seen a spec or a patch, just a new version of code (which isn't how we > do things...). I submitted the new patch in my name under a separate thread "Auditing extension for PostgreSQL (Take 2)" (54E005CC.1060605@pgmasters.net) because I was not getting any response from the original authors and I wanted to make sure the patch got in for the 2015-02 CF. Of course credit should be given where it is due - to that end I sent email to Ian and Abhijit over the weekend but haven't heard back from them yet. I appreciate that 2ndQuadrant spearheaded this effort and though I made many changes, the resulting code follows the same general design. > I'm happy to do final review and commit. Assuming we are in agreement, > what changes are needed prior to commit? I've incorporated most of Stephen's changes but I won't have time to get another patch out today. However, since most of his comments were about comments, I'd be happy to have your review as is and I appreciate your feedback. -- - David Steele david@pgmasters.net
At 2015-02-17 10:52:55 -0500, sfrost@snowman.net wrote: > > From the old thread, David had offered to submit a pull request if > there was interest and I didn't see any response... For whatever it's worth, that's because I've been away from work, and only just returned. I had it on my list to look at the code tomorrow. -- Abhijit
> On Feb 17, 2015, at 3:40 AM, Yeb Havinga <yebhavinga@gmail.com> wrote: > > Hi list, > . . . . . > Auditing superuser access means auditing beyond the running database. > The superuser can dump a table, and pipe this data everywhere outside of > the auditing domain. I cannot begin to imagine the kind of security > restrictions you'd need to audit what happens with data after libpq has > produced the results. My best guess would be to incorporate some kind of > separation of duty mechanism; only allow certain superuser operations > with two people involved. My views are from working with FDA validated environments, and I don’t really understand the above. It is not db auditing’sjob to stop or control the access to data or to log what happens to data outside of PostgreSQL. To audit a dbsuperuser is very simple, keep a log of everything a super user does and to write that log to a write append, no read filesystemor location. Since the db superuser can do anything there is no value in configuring what to log. This shouldbe an option that once set, cannot be changed without reinstalling the PostgreSQL binary. The responsibility for auditing/controllingany binary replacement is the operating system’s, not PostgreSQL. In this environment the db superuserwill not have authorized root access for the os. The use case examples, that I am familiar with, are that procedural policies control what the db superuser can do. If thepolicy says that the db superuser cannot dump a table and pipe this data someplace without being supervised by a thirdparty auditor (building on the above), then what you want in the log is that the data was dumped by whom with a dateand time. Thats it. Its up to policies, audit review, management, and third party audit tools, to pick up the violation. Auditing’s job is to keep a complete report, not prevent. Prevention is the role of security. Neil
* Simon Riggs (simon@2ndQuadrant.com) wrote:
> David's work is potentially useful, but having two versions of a
> feature slows things down. Since he is new to development here, I have
> made those comments so he understands, not so you would pick up on
> that.
I have a bad tendency of replying to email which is replying to comments
which I made. :)
> "didn't seem to be moving forwards" is strange comment. We usually
> wait for patches to stabilise, not for them to keep changing as
> evidence of worth.
I have to admit that I'm confused by this. Patches don't stabilise
through sitting in the archives, they stabilise when the comments are
being addressed, the patch updated, and further comments are addressing
less important issues. The issues which Robert and I had both commented
on didn't appear to be getting addressed. That seems to be due to
Abhijit being out, which is certainly fine, but that wasn't clear, at
least to me.
> David, please submit your work to pgsql-hackers as a patch on
> Abhijit's last version. There is no need for a pull request to
> 2ndQuadrant. Thanks.
Ugh. For my part, at least, having patches on top of patches does *not*
make things easier to work with or review. I'm very glad to hear that
Abhijit is back and has time to work on this, but as it relates to
submitting patches for review to the list or to the commitfest, I'd
really like those to be complete patches and not just .c files or
patches on top of other patches. To the extent that it helps move this
along, I'm not going to object if such patches are posted, but I would
object to patches-on-patches being included in the commmitfest. I've
not spent much time with the new commitfest app yet, but hopefully it
won't be hard to note which patches are the complete ones.
Thanks!
Stephen
Neil,
* Neil Tiffin (neilt@neiltiffin.com) wrote:
> > On Feb 17, 2015, at 3:40 AM, Yeb Havinga <yebhavinga@gmail.com> wrote:
> > Auditing superuser access means auditing beyond the running database.
> > The superuser can dump a table, and pipe this data everywhere outside of
> > the auditing domain. I cannot begin to imagine the kind of security
> > restrictions you'd need to audit what happens with data after libpq has
> > produced the results. My best guess would be to incorporate some kind of
> > separation of duty mechanism; only allow certain superuser operations
> > with two people involved.
>
> My views are from working with FDA validated environments, and I don’t really understand the above. It is not db
auditing’sjob to stop or control the access to data or to log what happens to data outside of PostgreSQL. To audit a
dbsuperuser is very simple, keep a log of everything a super user does and to write that log to a write append, no read
filesystemor location. Since the db superuser can do anything there is no value in configuring what to log. This
shouldbe an option that once set, cannot be changed without reinstalling the PostgreSQL binary. The responsibility for
auditing/controllingany binary replacement is the operating system’s, not PostgreSQL. In this environment the db
superuserwill not have authorized root access for the os.
I agree that it's not the auditing job to stop or control access to
data, but it's not so simple to audit the superuser completely. The
issue is that even if you have a hard-coded bit in the binary which says
"audit everything", a superuser can change the running code to twiddle
that bit off, redirect the output of whatever auditing is happening,
gain OS-level (eg: shell) access to the system and then make changes to
the files under PG directly, etc. Setting a bit in a binary and then
not allowing that binary to be unchanged does not actually solve the
issue.
> The use case examples, that I am familiar with, are that procedural policies control what the db superuser can do.
Ifthe policy says that the db superuser cannot dump a table and pipe this data someplace without being supervised by a
thirdparty auditor (building on the above), then what you want in the log is that the data was dumped by whom with a
dateand time. Thats it. Its up to policies, audit review, management, and third party audit tools, to pick up the
violation. Auditing’s job is to keep a complete report, not prevent. Prevention is the role of security.
Agreed, policy is how to handle superuser-level access and auditing can
assist with that but without having an independent process (one which
the PG superuser can't control, which means it needs to be a different
OS-level user), it's not possible to guarantee that the audit is
complete when the superuser is involved.
Thanks!
Stephen
On 2/17/15 12:07 PM, Stephen Frost wrote: >> My views are from working with FDA validated environments, and I don’t really understand the above. It is not db auditing’sjob to stop or control the access to data or to log what happens to data outside of PostgreSQL. To audit a dbsuperuser is very simple, keep a log of everything a super user does and to write that log to a write append, no read filesystemor location. Since the db superuser can do anything there is no value in configuring what to log. This shouldbe an option that once set, cannot be changed without reinstalling the PostgreSQL binary. The responsibility for auditing/controllingany binary replacement is the operating system’s, not PostgreSQL. In this environment the db superuserwill not have authorized root access for the os. > I agree that it's not the auditing job to stop or control access to > data, but it's not so simple to audit the superuser completely. The > issue is that even if you have a hard-coded bit in the binary which says > "audit everything", a superuser can change the running code to twiddle > that bit off, redirect the output of whatever auditing is happening, > gain OS-level (eg: shell) access to the system and then make changes to > the files under PG directly, etc. Setting a bit in a binary and then > not allowing that binary to be unchanged does not actually solve the > issue. If we've allowed a superuser *in the database* that kind of power at the OS level then we have a problem. There needs to be *something* that a database SU can't do at the OS level, otherwise we'll never be able to audit database SU activity. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
* Jim Nasby (Jim.Nasby@BlueTreble.com) wrote:
> On 2/17/15 12:07 PM, Stephen Frost wrote:
> >I agree that it's not the auditing job to stop or control access to
> >data, but it's not so simple to audit the superuser completely. The
> >issue is that even if you have a hard-coded bit in the binary which says
> >"audit everything", a superuser can change the running code to twiddle
> >that bit off, redirect the output of whatever auditing is happening,
> >gain OS-level (eg: shell) access to the system and then make changes to
> >the files under PG directly, etc. Setting a bit in a binary and then
> >not allowing that binary to be unchanged does not actually solve the
> >issue.
>
> If we've allowed a superuser *in the database* that kind of power at
> the OS level then we have a problem. There needs to be *something*
> that a database SU can't do at the OS level, otherwise we'll never
> be able to audit database SU activity.
This isn't a question. The database superuser has essentially OS-level
privileges as the user which PG runs as.
I'm all for coming up with a less powerful superuser and the work I've
been involved in around adding more role attributes is along the lines
to get us there, but I don't think we're ever going to really reduce the
power that the PG superuser has, for a variety of reasons.
Improving the documentation of what a superuser can do and how granting
such access is the same as giving OS shell-level access to the system as
the user that PG runs as would certainly be good.
Thanks,
Stephen
On 2/17/15 12:23 PM, Stephen Frost wrote: > * Jim Nasby (Jim.Nasby@BlueTreble.com) wrote: >> On 2/17/15 12:07 PM, Stephen Frost wrote: >>> I agree that it's not the auditing job to stop or control access to >>> data, but it's not so simple to audit the superuser completely. The >>> issue is that even if you have a hard-coded bit in the binary which says >>> "audit everything", a superuser can change the running code to twiddle >>> that bit off, redirect the output of whatever auditing is happening, >>> gain OS-level (eg: shell) access to the system and then make changes to >>> the files under PG directly, etc. Setting a bit in a binary and then >>> not allowing that binary to be unchanged does not actually solve the >>> issue. >> >> If we've allowed a superuser *in the database* that kind of power at >> the OS level then we have a problem. There needs to be *something* >> that a database SU can't do at the OS level, otherwise we'll never >> be able to audit database SU activity. > > This isn't a question. The database superuser has essentially OS-level > privileges as the user which PG runs as. > > I'm all for coming up with a less powerful superuser and the work I've > been involved in around adding more role attributes is along the lines > to get us there, but I don't think we're ever going to really reduce the > power that the PG superuser has, for a variety of reasons. > > Improving the documentation of what a superuser can do and how granting > such access is the same as giving OS shell-level access to the system as > the user that PG runs as would certainly be good. It certainly would. I'm honestly not totally clear on what all the holes are. We may need to bite the bullet and allow changing the user that the postgres process runs under so it doesn't match who owns the files. Maybe there's a way to allow that other than having the process start as root. Or maybe there's some other way we could restrict what a DB superuser can do in the shell. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
Jim,
* Jim Nasby (Jim.Nasby@BlueTreble.com) wrote:
> We may need to bite the bullet and allow changing the user that the
> postgres process runs under so it doesn't match who owns the files.
> Maybe there's a way to allow that other than having the process
> start as root.
That's an interesting thought but it doesn't seem too likely to work out
for us. The process still has to be able to read and write the files,
create new files in the PGDATA directories, etc.
> Or maybe there's some other way we could restrict what a DB
> superuser can do in the shell.
This could be done with SELinux and similar tools, but at the end of the
day the answer, in my view really, is to have fewer superusers and for
those superusers to be understood to have OS-level shell access. We
don't want to deal with all of the security implications of trying to
provide a "trusted" superuser when that user can create functions in
untrusted languages, modify the catalog directly, etc, it really just
doesn't make sense.
Thanks,
Stephen
On 2/17/15 12:50 PM, Stephen Frost wrote: > Jim, > > * Jim Nasby (Jim.Nasby@BlueTreble.com) wrote: >> We may need to bite the bullet and allow changing the user that the >> postgres process runs under so it doesn't match who owns the files. >> Maybe there's a way to allow that other than having the process >> start as root. > > That's an interesting thought but it doesn't seem too likely to work out > for us. The process still has to be able to read and write the files, > create new files in the PGDATA directories, etc. Right, but if we don't allow things like loading C functions from wherever you please then it's a lot less likely that a DB SU could disable auditing. >> Or maybe there's some other way we could restrict what a DB >> superuser can do in the shell. > > This could be done with SELinux and similar tools, but at the end of the > day the answer, in my view really, is to have fewer superusers and for > those superusers to be understood to have OS-level shell access. We > don't want to deal with all of the security implications of trying to > provide a "trusted" superuser when that user can create functions in > untrusted languages, modify the catalog directly, etc, it really just > doesn't make sense. The part I don't like about that is then you still have this highly trusted account that can also run SQL. The big issue with that is that no OS-level auditing is going to catch what happens inside the database itself (well, I guess short of a key logger...) What I'd prefer to see is a way to decouple the OS account from any DB account. Clearly that doesn't protect us from the OS user doing something bad, but at least then there's no way for them to just silently run SQL commands. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
* Jim Nasby (Jim.Nasby@BlueTreble.com) wrote:
> On 2/17/15 12:50 PM, Stephen Frost wrote:
> >* Jim Nasby (Jim.Nasby@BlueTreble.com) wrote:
> >>We may need to bite the bullet and allow changing the user that the
> >>postgres process runs under so it doesn't match who owns the files.
> >>Maybe there's a way to allow that other than having the process
> >>start as root.
> >
> >That's an interesting thought but it doesn't seem too likely to work out
> >for us. The process still has to be able to read and write the files,
> >create new files in the PGDATA directories, etc.
>
> Right, but if we don't allow things like loading C functions from
> wherever you please then it's a lot less likely that a DB SU could
> disable auditing.
It's not just C functions, there's also a whole slew of untrusted
languages, and a superuser can modify the catalog directly. They could
change the relfileno for a relation to some other relation that they're
really interested in, or use pageinspect, etc.
> >>Or maybe there's some other way we could restrict what a DB
> >>superuser can do in the shell.
> >
> >This could be done with SELinux and similar tools, but at the end of the
> >day the answer, in my view really, is to have fewer superusers and for
> >those superusers to be understood to have OS-level shell access. We
> >don't want to deal with all of the security implications of trying to
> >provide a "trusted" superuser when that user can create functions in
> >untrusted languages, modify the catalog directly, etc, it really just
> >doesn't make sense.
>
> The part I don't like about that is then you still have this highly
> trusted account that can also run SQL. The big issue with that is
> that no OS-level auditing is going to catch what happens inside the
> database itself (well, I guess short of a key logger...)
Right- you would need an independent process which acts as an
intermediary between the database and the account connecting which
simply logs everything. That's really not all *that* hard to do, but
it's clearly outside of the scope of PG.
> What I'd prefer to see is a way to decouple the OS account from any
> DB account. Clearly that doesn't protect us from the OS user doing
> something bad, but at least then there's no way for them to just
> silently run SQL commands.
If the DB account isn't a superuser then everything changes. There's no
point fighting with the OS user- they can run some other PG binary which
they've copied over locally and run SQL with that, or copy all the files
over to another server which they have complete access to. The fact
that they can also connect to the DB and run SQL isn't really an issue.
Thanks,
Stephen
On 2/17/15 1:10 PM, Stephen Frost wrote: >> What I'd prefer to see is a way to decouple the OS account from any >> >DB account. Clearly that doesn't protect us from the OS user doing >> >something bad, but at least then there's no way for them to just >> >silently run SQL commands. > If the DB account isn't a superuser then everything changes. There's no > point fighting with the OS user- they can run some other PG binary which > they've copied over locally and run SQL with that, or copy all the files > over to another server which they have complete access to. The fact > that they can also connect to the DB and run SQL isn't really an issue. I disagree. The difference here is that the OS can audit whatever commands they're running, but not what happens inside something like psql. Even if you did run a keylogger, trying to use that to interpret a psql session would be a real pain, if not impossible. So I don't think we can rely on the OS to audit SQL at all. But obviously if they did something like copy the files somewhere else, or bring in a new binary, the OS would at least have record that that happened. Though... I wonder if there's some way we could disallow *all* superuser access to the database, and instead create a special non-interactive CLI. That would allow us to throw the problem over the wall to the OS. In any case, I think it's clear that there's a lot of value in at least handling the non-SU case, so we should try and do that now. Even if it's only in contrib. One thing that does occur to me though... perhaps we should specifically disable auditing of SU activities, so we're not providing a false sense of security. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
* Jim Nasby (Jim.Nasby@BlueTreble.com) wrote:
> On 2/17/15 1:10 PM, Stephen Frost wrote:
> >>What I'd prefer to see is a way to decouple the OS account from any
> >>>DB account. Clearly that doesn't protect us from the OS user doing
> >>>something bad, but at least then there's no way for them to just
> >>>silently run SQL commands.
> >If the DB account isn't a superuser then everything changes. There's no
> >point fighting with the OS user- they can run some other PG binary which
> >they've copied over locally and run SQL with that, or copy all the files
> >over to another server which they have complete access to. The fact
> >that they can also connect to the DB and run SQL isn't really an issue.
>
> I disagree. The difference here is that the OS can audit whatever
> commands they're running, but not what happens inside something like
> psql. Even if you did run a keylogger, trying to use that to
> interpret a psql session would be a real pain, if not impossible. So
> I don't think we can rely on the OS to audit SQL at all. But
> obviously if they did something like copy the files somewhere else,
> or bring in a new binary, the OS would at least have record that
> that happened.
From my experience, logging the commands is no where near enough.. psql
is hardly the only complex CLI process one can run. Further, I'm not
suggesting that we rely on the OS to audit SQL, merely stating that
anything which deals with auditing the connection to PG needs to be
outside of the PG process space (and that of processes owned by the user
which PG is running as).
> Though... I wonder if there's some way we could disallow *all*
> superuser access to the database, and instead create a special
> non-interactive CLI. That would allow us to throw the problem over
> the wall to the OS.
That sounds like a horrible approach. A non-interactive CLI would be
terrible and it's not clear to me how that'd really help. What happens
when they run emacs or vim?
> In any case, I think it's clear that there's a lot of value in at
> least handling the non-SU case, so we should try and do that now.
> Even if it's only in contrib.
Absolutely. Glad we agree on that. :)
> One thing that does occur to me though... perhaps we should
> specifically disable auditing of SU activities, so we're not
> providing a false sense of security.
I don't agree with this. Done properly (auditing enabled on startup,
using a remote auditing target, etc), it might be possible for such
auditing to catch odd superuser behavior. What we don't want to do is
claim that we provide full superuser auditing, as that's something we
can't provide. Appropriate and clear documentation is absolutely
critical, of course.
Thanks!
Stephen
At 2015-02-17 13:01:46 -0500, sfrost@snowman.net wrote: > > I have to admit that I'm confused by this. Patches don't stabilise > through sitting in the archives, they stabilise when the comments are > being addressed, the patch updated, and further comments are > addressing less important issues. The issues which Robert and I had > both commented on didn't appear to be getting addressed. I'm confused and unhappy about your characterisation of the state of this patch. You make it seem as though there was broad consensus about the changes that were needed, and that I left the patch sitting in the archives for a long time without addressing important issues. You revised and refined your GRANT proposal in stages, and I tried to adapt the code to suit. I'm sorry that my efforts were not fast enough or responsive enough to make you feel that progress was being made. But nobody else commented in detail on the GRANT changes except to express general misgivings, and nobody else even disagreed when I inferred, in light of the lack of consensus that Robert pointed out, that the code was unlikely to make it into 9.5. Given that I've maintained the code over the past year despite its being rejected in an earlier CF, and given the circumstances outlined above, I do not think it's reasonable to conclude after a couple of weeks without a new version that it was abandoned. As I had mentioned earlier, there are people who already use pgaudit as-is, and complain if I break it. Anyway, I guess there is no such thing as a constructive history discussion, so I'll drop it. -- Abhijit
On Wed, Feb 18, 2015 at 1:26 AM, David Steele <david@pgmasters.net> wrote: > On 2/17/15 10:23 AM, Simon Riggs wrote: >> I vote to include pgaudit in 9.5, albeit with any changes. In >> particular, David may have some changes to recommend, but I haven't >> seen a spec or a patch, just a new version of code (which isn't how we >> do things...). > > I submitted the new patch in my name under a separate thread "Auditing > extension for PostgreSQL (Take 2)" (54E005CC.1060605@pgmasters.net) I played the patch version of pg_audit a bit and have basic comments about its spec. The pg_audit doesn't log BIND parameter values when prepared statement is used. Seems this is an oversight of the patch. Or is this intentional? The pg_audit cannot log the statement like "SELECT 1" which doesn't access to the database object. Is this intentional? I think that there are many users who want to audit even such statement. Imagine the case where you call the user-defined function which executes many nested statements. In this case, pg_audit logs only top-level statement (i.e., issued directly by client) every time nested statement is executed. In fact, one call of such UDF can cause lots of *same* log messages. I think this is problematic. Regards, -- Fujii Masao
Stephen, I meant it to go to the list, but hit the wrong button. > On Feb 17, 2015, at 7:01 PM, Stephen Frost <sfrost@snowman.net> wrote: > > Neil, > > I noticed that you email'd me directly on this reply. Not sure if you > intended to or not, but I'm fine with my response going to the list. > > * Neil Tiffin (neilt@neiltiffin.com) wrote: >>> On Feb 17, 2015, at 1:10 PM, Stephen Frost <sfrost@snowman.net> wrote: >>> If the DB account isn't a superuser then everything changes. There's no >>> point fighting with the OS user- they can run some other PG binary which >>> they've copied over locally and run SQL with that, or copy all the files >>> over to another server which they have complete access to. The fact >>> that they can also connect to the DB and run SQL isn’t really an issue. >> >> Thats the point. If this environment matters then the db superuser would not be an authorized os superuser (with rootor even close privileges). And no, they could not be running some other binary. >> >> One way to do pg superuser auditing is to utilize a file (outside of the pg data directory, which probably violates somethingin pg) like /etc/pg_su_audit that has os root rw and r for all others (or the equivalent for other os’s) containinga URL. If this file is present, send all db superuser usage to the URL. If this file is present with the wrongprivileges, then don’t start pg. Done. No configuration in pg config files, no GUCs, no nothing for the pg superuserto mess around with, not tables, no ability for any pg superuser to configure or control. > > This approach doesn't violate anything in PG and can be used with any of > the pgaudit approaches being discussed. The fact that it's > postgresql.conf, which represents GUCs, doesn't change anything > regarding what you’re getting it. > It changes everything, pg superusers have complete control of files in the pg data directory. If set up correctly pg superusershave no control in /etc. >> If they can copy or install a PG binary, then the OS auditing and security failed. PG code need not be concerned. >> >> Sure someone could still break access to the URL, but again, not PG’s concern. Other security and auditing would haveresponsibility to pick that up. >> >> It is a really simple use case, record everything any db superuser does and send it to the audit system. Done. Thena designated role can control what gets audited in production. As long as everything the db superuser does can be writtento an audit log, then it no longer matters technically if the db superuser can change the rest of the auditing. Ifthey do and it violates policy, then when the audit picks it up, they lose their job plus depending on the environment. > > The issue is really around what we claim to provide with this auditing. > We can't claim to provide *full* superuser auditing with any of these > approaches since the superuser can disable auditing. We can, however, > provide auditing up until that happens, which is likely to be sufficient > in many environments. For those environments where full superuser > auditing is required, an independent system must be used. > > Of course, it's important that any auditing mechanism which is used to > audit superusers be impossible for the superuser to modify after the > fact, meaning that syslog or similar needs to be used. > I’m still confused since you do do not differentiate between db superuser and os superuser and what you mean by an independentsystem? With the scheme I described above, how does the db superuser disable auditing without os root privileges? If they can, thenpg security is fundamentally broken, and I don’t believe it is. How can an independent system monitor what commands are issued inside the database? I understand my comments do not cover what is being proposed or worked on and that is fine. But saying it does not havevalue because the superuser could disable any system in pg, is wrong IMO. Being able to reliability log db superuserswithout their ability to change the logging would be a fundamentally good security tool as companies become moreserious about internal security. This is, and will be more, important since a lot of people consider insider breachesthe biggest security challenge today. Neil
Neil,
* Neil Tiffin (neilt@neiltiffin.com) wrote:
> I meant it to go to the list, but hit the wrong button.
No problem.
> > On Feb 17, 2015, at 7:01 PM, Stephen Frost <sfrost@snowman.net> wrote:
> > I noticed that you email'd me directly on this reply. Not sure if you
> > intended to or not, but I'm fine with my response going to the list.
> >
> > This approach doesn't violate anything in PG and can be used with any of
> > the pgaudit approaches being discussed. The fact that it's
> > postgresql.conf, which represents GUCs, doesn't change anything
> > regarding what you’re getting it.
> >
>
> It changes everything, pg superusers have complete control of files in the pg data directory. If set up correctly pg
superusershave no control in /etc.
If set up correctly, postgresql.conf is in /etc. :) That's distribution
specific though. However, postgresql.auto.conf ends up causing problems
since it can't be disabled and it's forced to live in the PG data
directory. Even so though, your argument was that, as long as the
system is set up to be auditing initially and whatever action the
superuser takes to disable auditing will be audited, and disabling
auditing is against policy, then it's sufficient to meet the
requirement. That's true in either case.
> > The issue is really around what we claim to provide with this auditing.
> > We can't claim to provide *full* superuser auditing with any of these
> > approaches since the superuser can disable auditing. We can, however,
> > provide auditing up until that happens, which is likely to be sufficient
> > in many environments. For those environments where full superuser
> > auditing is required, an independent system must be used.
> >
> > Of course, it's important that any auditing mechanism which is used to
> > audit superusers be impossible for the superuser to modify after the
> > fact, meaning that syslog or similar needs to be used.
>
> I’m still confused since you do do not differentiate between db superuser and os superuser and what you mean by an
independentsystem?
When I'm talking about 'superuser' here, it's the PG superuser, not the
OS superuser. What I mean by independent system is a process which is
not owned by the same user that the database is running as, and isn't
owned by the user who is connecting, that facilitates the connection
from the user to PG, but which logs everything that happens on that
connection.
> With the scheme I described above, how does the db superuser disable auditing without os root privileges? If they
can,then pg security is fundamentally broken, and I don’t believe it is.
The DB superuser can modify the running process, through mechanisms as
simple as changing GUCs to creating functions in untrusted languages
(including C) and then using them to change or break more-or-less
anything that's happening in the backend.
> How can an independent system monitor what commands are issued inside the database?
It can log everything which happens on the connection between the user
and the database because it's in the middle.
> I understand my comments do not cover what is being proposed or worked on and that is fine. But saying it does not
havevalue because the superuser could disable any system in pg, is wrong IMO. Being able to reliability log db
superuserswithout their ability to change the logging would be a fundamentally good security tool as companies become
moreserious about internal security. This is, and will be more, important since a lot of people consider insider
breachesthe biggest security challenge today.
It'd be a great tool, certainly, but it's not actually possible to do
completely inside the DB process given the capabilities the PG superuser
has. Being able to create and run functions in untrusted languages
means that the superuser can completely hijack the process, that's why
those languages are untrusted.
Thanks,
Stephen
Abhijit,
* Abhijit Menon-Sen (ams@2ndQuadrant.com) wrote:
> At 2015-02-17 13:01:46 -0500, sfrost@snowman.net wrote:
> > I have to admit that I'm confused by this. Patches don't stabilise
> > through sitting in the archives, they stabilise when the comments are
> > being addressed, the patch updated, and further comments are
> > addressing less important issues. The issues which Robert and I had
> > both commented on didn't appear to be getting addressed.
>
> I'm confused and unhappy about your characterisation of the state of
> this patch. You make it seem as though there was broad consensus about
> the changes that were needed, and that I left the patch sitting in the
> archives for a long time without addressing important issues.
The specific issue which I was referring to there was the #ifdef's for
the deparse branch. I thought it was pretty clear, based on the
feedback from multiple people, that all of that really needed to be
taken out as we don't commit code to the main repo which has such
external dependencies. That wasn't meant as a characterization of the
patch itself but rather a comment on the state of the process and that
I, at least, had been hoping for a new version which addressed those
bits.
> You revised and refined your GRANT proposal in stages, and I tried to
> adapt the code to suit. I'm sorry that my efforts were not fast enough
> or responsive enough to make you feel that progress was being made. But
> nobody else commented in detail on the GRANT changes except to express
> general misgivings, and nobody else even disagreed when I inferred, in
> light of the lack of consensus that Robert pointed out, that the code
> was unlikely to make it into 9.5.
Progress was being made and I certainly appreciate your efforts. I
don't think anyone would want to stand up and say it's going to be in
9.5 or not be in 9.5 which is likely why you didn't get any reaction
from your comment about it being unlikely. I'm certainly hopeful that
something gets in along these lines as it's a pretty big capability that
we're currently missing.
> Given that I've maintained the code over the past year despite its being
> rejected in an earlier CF, and given the circumstances outlined above, I
> do not think it's reasonable to conclude after a couple of weeks without
> a new version that it was abandoned. As I had mentioned earlier, there
> are people who already use pgaudit as-is, and complain if I break it.
For my part, at least, I didn't intend to characterize it as abandoned
but rather that it simply didn't seem to be moving forward, perhaps due
to a lack of time to work on it or other priorities; I didn't mean to
imply that you wouldn't be continuing to maintain it. As for the
comment about people depending on it, I'm not sure what that's getting
at, but I don't think that's really a consideration for us as it relates
to having a contrib module to provide this capability. We certainly
want to consider what capabilities users are looking for and make sure
we cover those cases, if possible, but this is at a development stage
with regard to core and therefore things may break for existing users.
If that's an issue for your users then it would probably be good to have
a clean distinction between the stable code you're providing to them for
auditing and what's being submitted for inclusion in core, with an
expectation that users will need to make some changes when they switch
to the version which is included in core.
> Anyway, I guess there is no such thing as a constructive history
> discussion, so I'll drop it.
It's not my intent to continue this as I certainly agree that it seems
unlikely to be a constructive use of time, but I wanted to reply and
clarify that it wasn't my intent to characterize pgaudit as abandoned
and to apologize for my comments coming across that way.
Thanks!
Stephen
Stephen Frost wrote: > Abhijit, > > * Abhijit Menon-Sen (ams@2ndQuadrant.com) wrote: > > I'm confused and unhappy about your characterisation of the state of > > this patch. You make it seem as though there was broad consensus about > > the changes that were needed, and that I left the patch sitting in the > > archives for a long time without addressing important issues. > > The specific issue which I was referring to there was the #ifdef's for > the deparse branch. I thought it was pretty clear, based on the > feedback from multiple people, that all of that really needed to be > taken out as we don't commit code to the main repo which has such > external dependencies. We can remove the #ifdef lines as soon as DDL deparse gets committed, of course. There is no external dependency here, only a dependency on a patch that's being submitted in parallel. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Hi Fujii, Thanks for taking a look at the patch. Comments below: On 2/18/15 6:11 AM, Fujii Masao wrote: > On Wed, Feb 18, 2015 at 1:26 AM, David Steele <david@pgmasters.net> wrote: >> On 2/17/15 10:23 AM, Simon Riggs wrote: >>> I vote to include pgaudit in 9.5, albeit with any changes. In >>> particular, David may have some changes to recommend, but I haven't >>> seen a spec or a patch, just a new version of code (which isn't how we >>> do things...). >> >> I submitted the new patch in my name under a separate thread "Auditing >> extension for PostgreSQL (Take 2)" (54E005CC.1060605@pgmasters.net) > > I played the patch version of pg_audit a bit and have basic comments about > its spec. > > The pg_audit doesn't log BIND parameter values when prepared statement is used. > Seems this is an oversight of the patch. Or is this intentional? It's actually intentional - following the model I talked about in my earlier emails, the idea is to log statements only. This also follows on 2ndQuadrant's implementation. Logging values is interesting, but I'm sure the user would want to specify which columns to log, which I felt was beyond the scope of the patch. > The pg_audit cannot log the statement like "SELECT 1" which doesn't access to > the database object. Is this intentional? I think that there are many users who > want to audit even such statement. I think I see how to make this work. I'll work on it for the next version of the patch. > > Imagine the case where you call the user-defined function which executes > many nested statements. In this case, pg_audit logs only top-level statement > (i.e., issued directly by client) every time nested statement is executed. > In fact, one call of such UDF can cause lots of *same* log messages. I think > this is problematic. I agree - not sure how to go about addressing it, though. I've tried to cut down on the verbosity of the logging in general, but of course it can still be a problem. Using security definer and a different logging GUC for the defining role might work. I'll add that to my unit tests and see what happens. I appreciate your feedback! -- - David Steele david@pgmasters.net
On Thu, Feb 19, 2015 at 12:25 AM, David Steele <david@pgmasters.net> wrote: > Hi Fujii, > > Thanks for taking a look at the patch. Comments below: > > On 2/18/15 6:11 AM, Fujii Masao wrote: >> On Wed, Feb 18, 2015 at 1:26 AM, David Steele <david@pgmasters.net> wrote: >>> On 2/17/15 10:23 AM, Simon Riggs wrote: >>>> I vote to include pgaudit in 9.5, albeit with any changes. In >>>> particular, David may have some changes to recommend, but I haven't >>>> seen a spec or a patch, just a new version of code (which isn't how we >>>> do things...). >>> >>> I submitted the new patch in my name under a separate thread "Auditing >>> extension for PostgreSQL (Take 2)" (54E005CC.1060605@pgmasters.net) >> >> I played the patch version of pg_audit a bit and have basic comments about >> its spec. >> >> The pg_audit doesn't log BIND parameter values when prepared statement is used. >> Seems this is an oversight of the patch. Or is this intentional? > > It's actually intentional - following the model I talked about in my > earlier emails, the idea is to log statements only. Is this acceptable for audit purpose in many cases? Without the values, I'm afraid that it's hard to analyze what table records are affected by the statements from the audit logs. I was thinking that identifying the data affected is one of important thing for the audit. If I'm malicious DBA, I will always use the extended protocol to prevent the values from being audited when I execute the statement. Regards, -- Fujii Masao
On 2/18/15 10:29 PM, Fujii Masao wrote: > On Thu, Feb 19, 2015 at 12:25 AM, David Steele <david@pgmasters.net> wrote: >>> The pg_audit doesn't log BIND parameter values when prepared statement is used. >>> Seems this is an oversight of the patch. Or is this intentional? >> >> It's actually intentional - following the model I talked about in my >> earlier emails, the idea is to log statements only. > > Is this acceptable for audit purpose in many cases? Without the values, > I'm afraid that it's hard to analyze what table records are affected by > the statements from the audit logs. I was thinking that identifying the > data affected is one of important thing for the audit. If I'm malicious DBA, > I will always use the extended protocol to prevent the values from being > audited when I execute the statement. I agree with you, but I wonder how much is practical at this stage. Let me think about it and see what I can come up with. -- - David Steele david@pgmasters.net
On 2/18/15 10:25 AM, David Steele wrote: > On 2/18/15 6:11 AM, Fujii Masao wrote: >> The pg_audit doesn't log BIND parameter values when prepared statement is used. >> Seems this is an oversight of the patch. Or is this intentional? > > It's actually intentional - following the model I talked about in my > earlier emails, the idea is to log statements only. This also follows > on 2ndQuadrant's implementation. Unfortunately, I think it's beyond the scope of this module to log bind variables. I'm following not only 2ndQuadrant's implementation, but Oracle's as well. > Logging values is interesting, but I'm sure the user would want to > specify which columns to log, which I felt was beyond the scope of the > patch. > >> The pg_audit cannot log the statement like "SELECT 1" which doesn't access to >> the database object. Is this intentional? I think that there are many users who >> want to audit even such statement. > > I think I see how to make this work. I'll work on it for the next > version of the patch. This has been fixed in the v2 patch. >> Imagine the case where you call the user-defined function which executes >> many nested statements. In this case, pg_audit logs only top-level statement >> (i.e., issued directly by client) every time nested statement is executed. >> In fact, one call of such UDF can cause lots of *same* log messages. I think >> this is problematic. > > I agree - not sure how to go about addressing it, though. I've tried to > cut down on the verbosity of the logging in general, but of course it > can still be a problem. > > Using security definer and a different logging GUC for the defining role > might work. I'll add that to my unit tests and see what happens. That didn't work - but I didn't really expect it to. Here are two options I thought of: 1) Follow Oracle's "as session" option and only log each statement type against an object the first time it happens in a session. This would greatly reduce logging, but might be too little detail. It would increase the memory footprint of the module to add the tracking. 2) Only log once per call to the backend. Essentially, we would only log the statement you see in pg_stat_activity. This could be a good option because it logs what the user accesses directly, rather than functions, views, etc. which hopefully are already going through a review process and can be audited that way. Would either of those address your concerns? -- - David Steele david@pgmasters.net
On Tue, Feb 24, 2015 at 1:29 AM, David Steele <david@pgmasters.net> wrote: > On 2/18/15 10:25 AM, David Steele wrote: >> On 2/18/15 6:11 AM, Fujii Masao wrote: >>> The pg_audit doesn't log BIND parameter values when prepared statement is used. >>> Seems this is an oversight of the patch. Or is this intentional? >> >> It's actually intentional - following the model I talked about in my >> earlier emails, the idea is to log statements only. This also follows >> on 2ndQuadrant's implementation. > > Unfortunately, I think it's beyond the scope of this module to log bind > variables. Maybe I can live with that at least in the first version. > I'm following not only 2ndQuadrant's implementation, but > Oracle's as well. Oracle's audit_trail (e.g., = db, extended) can log even bind values. Also log_statement=on in PostgreSQL also can log bind values. Maybe we can reuse the same technique that log_statement uses. >> Logging values is interesting, but I'm sure the user would want to >> specify which columns to log, which I felt was beyond the scope of the >> patch. >> >>> The pg_audit cannot log the statement like "SELECT 1" which doesn't access to >>> the database object. Is this intentional? I think that there are many users who >>> want to audit even such statement. >> >> I think I see how to make this work. I'll work on it for the next >> version of the patch. > > This has been fixed in the v2 patch. Thanks! >>> Imagine the case where you call the user-defined function which executes >>> many nested statements. In this case, pg_audit logs only top-level statement >>> (i.e., issued directly by client) every time nested statement is executed. >>> In fact, one call of such UDF can cause lots of *same* log messages. I think >>> this is problematic. >> >> I agree - not sure how to go about addressing it, though. I've tried to >> cut down on the verbosity of the logging in general, but of course it >> can still be a problem. >> >> Using security definer and a different logging GUC for the defining role >> might work. I'll add that to my unit tests and see what happens. > > That didn't work - but I didn't really expect it to. > > Here are two options I thought of: > > 1) Follow Oracle's "as session" option and only log each statement type > against an object the first time it happens in a session. This would > greatly reduce logging, but might be too little detail. It would > increase the memory footprint of the module to add the tracking. > > 2) Only log once per call to the backend. Essentially, we would only > log the statement you see in pg_stat_activity. This could be a good > option because it logs what the user accesses directly, rather than > functions, views, etc. which hopefully are already going through a > review process and can be audited that way. > > Would either of those address your concerns? Before discussing how to implement, probably we need to consider the spec about this. For example, should we log even nested statements for the audit purpose? If yes, how should we treat the case where the user changes the setting so that only DDL is logged, and then the user-defined function which internally executes DDL is called? Since the top-level SQL (calling the function) is not the target of audit, we should not log even the nested DDL? Regards, -- Fujii Masao
Fujii Masao wrote: > On Tue, Feb 24, 2015 at 1:29 AM, David Steele <david@pgmasters.net> wrote: > > 1) Follow Oracle's "as session" option and only log each statement type > > against an object the first time it happens in a session. This would > > greatly reduce logging, but might be too little detail. It would > > increase the memory footprint of the module to add the tracking. Doesn't this mean that a user could conceal malicious activity simply by doing a innocuous query that silences all further activity? > > 2) Only log once per call to the backend. Essentially, we would only > > log the statement you see in pg_stat_activity. This could be a good > > option because it logs what the user accesses directly, rather than > > functions, views, etc. which hopefully are already going through a > > review process and can be audited that way. ... assuming the user does not create stuff on the fly behind your back. > > Would either of those address your concerns? > > Before discussing how to implement, probably we need to consider the > spec about this. For example, should we log even nested statements for > the audit purpose? If yes, how should we treat the case where > the user changes the setting so that only DDL is logged, and then > the user-defined function which internally executes DDL is called? > Since the top-level SQL (calling the function) is not the target of > audit, we should not log even the nested DDL? Clearly if you log only DROP TABLE, and then the malicious user hides one such call inside a function that executes the drop (which is called via a SELECT top-level SQL), you're not going to be happy. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Feb 26, 2015 at 1:40 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > Fujii Masao wrote: >> On Tue, Feb 24, 2015 at 1:29 AM, David Steele <david@pgmasters.net> wrote: > >> > 1) Follow Oracle's "as session" option and only log each statement type >> > against an object the first time it happens in a session. This would >> > greatly reduce logging, but might be too little detail. It would >> > increase the memory footprint of the module to add the tracking. > > Doesn't this mean that a user could conceal malicious activity simply by > doing a innocuous query that silences all further activity? > >> > 2) Only log once per call to the backend. Essentially, we would only >> > log the statement you see in pg_stat_activity. This could be a good >> > option because it logs what the user accesses directly, rather than >> > functions, views, etc. which hopefully are already going through a >> > review process and can be audited that way. > > ... assuming the user does not create stuff on the fly behind your back. > >> > Would either of those address your concerns? >> >> Before discussing how to implement, probably we need to consider the >> spec about this. For example, should we log even nested statements for >> the audit purpose? If yes, how should we treat the case where >> the user changes the setting so that only DDL is logged, and then >> the user-defined function which internally executes DDL is called? >> Since the top-level SQL (calling the function) is not the target of >> audit, we should not log even the nested DDL? > > Clearly if you log only DROP TABLE, and then the malicious user hides > one such call inside a function that executes the drop (which is called > via a SELECT top-level SQL), you're not going to be happy. Yep, so what SQL should be logged in this case? Only "targeted" nested DDL? Both top and nested ones? Seems the later is better to me. What about the case where the function A calls the function B executing DDL? Every ancestor SQLs of the "targeted" DDL should be logged? Probably yes. Regards, -- Fujii Masao
On 2/25/15 10:42 PM, Fujii Masao wrote: > On Tue, Feb 24, 2015 at 1:29 AM, David Steele <david@pgmasters.net> wrote: >> On 2/18/15 10:25 AM, David Steele wrote: >>> On 2/18/15 6:11 AM, Fujii Masao wrote: >>>> The pg_audit doesn't log BIND parameter values when prepared statement is used. >>>> Seems this is an oversight of the patch. Or is this intentional? >>> >>> It's actually intentional - following the model I talked about in my >>> earlier emails, the idea is to log statements only. This also follows >>> on 2ndQuadrant's implementation. >> >> Unfortunately, I think it's beyond the scope of this module to log bind >> variables. > > Maybe I can live with that at least in the first version. > >> I'm following not only 2ndQuadrant's implementation, but >> Oracle's as well. > > Oracle's audit_trail (e.g., = db, extended) can log even bind values. > Also log_statement=on in PostgreSQL also can log bind values. > Maybe we can reuse the same technique that log_statement uses. I'll look at how this is done in the logging code and see if it can be used in pg_audit. >>>> Imagine the case where you call the user-defined function which executes >>>> many nested statements. In this case, pg_audit logs only top-level statement >>>> (i.e., issued directly by client) every time nested statement is executed. >>>> In fact, one call of such UDF can cause lots of *same* log messages. I think >>>> this is problematic. >>> >>> I agree - not sure how to go about addressing it, though. I've tried to >>> cut down on the verbosity of the logging in general, but of course it >>> can still be a problem. >>> >>> Using security definer and a different logging GUC for the defining role >>> might work. I'll add that to my unit tests and see what happens. >> >> That didn't work - but I didn't really expect it to. >> >> Here are two options I thought of: >> >> 1) Follow Oracle's "as session" option and only log each statement type >> against an object the first time it happens in a session. This would >> greatly reduce logging, but might be too little detail. It would >> increase the memory footprint of the module to add the tracking. >> >> 2) Only log once per call to the backend. Essentially, we would only >> log the statement you see in pg_stat_activity. This could be a good >> option because it logs what the user accesses directly, rather than >> functions, views, etc. which hopefully are already going through a >> review process and can be audited that way. >> >> Would either of those address your concerns? > > Before discussing how to implement, probably we need to consider the > spec about this. For example, should we log even nested statements for > the audit purpose? If yes, how should we treat the case where > the user changes the setting so that only DDL is logged, and then > the user-defined function which internally executes DDL is called? > Since the top-level SQL (calling the function) is not the target of > audit, we should not log even the nested DDL? I think logging nested statements should at least be an option. And yes, I think that nested statements should be logged even if the top-level SQL is not (depending on configuration). The main case for this would be DO blocks which can be run by anybody. -- - David Steele david@pgmasters.net
On 2/25/15 11:40 PM, Alvaro Herrera wrote: > Fujii Masao wrote: >> On Tue, Feb 24, 2015 at 1:29 AM, David Steele <david@pgmasters.net> wrote: > >>> 1) Follow Oracle's "as session" option and only log each statement type >>> against an object the first time it happens in a session. This would >>> greatly reduce logging, but might be too little detail. It would >>> increase the memory footprint of the module to add the tracking. > > Doesn't this mean that a user could conceal malicious activity simply by > doing a innocuous query that silences all further activity? True enough, but I thought it might be useful as an option. I tend to lock down users pretty tightly so there's not much they can do without somehow getting superuser access, at which point all bets are off anyway. In the case where users are highly constrained, the idea here would be to just keeps tabs on the sort of things they are reading/writing for financial audits and ISO compliance. >>> 2) Only log once per call to the backend. Essentially, we would only >>> log the statement you see in pg_stat_activity. This could be a good >>> option because it logs what the user accesses directly, rather than >>> functions, views, etc. which hopefully are already going through a >>> review process and can be audited that way. > > ... assuming the user does not create stuff on the fly behind your back. Sure, but then the thing they created/modified would also be logged somewhere earlier (assuming pg_audit.log = 'all'). It would require some analysis to figure out what they did but I think that would be true in many cases. >>> Would either of those address your concerns? >> >> Before discussing how to implement, probably we need to consider the >> spec about this. For example, should we log even nested statements for >> the audit purpose? If yes, how should we treat the case where >> the user changes the setting so that only DDL is logged, and then >> the user-defined function which internally executes DDL is called? >> Since the top-level SQL (calling the function) is not the target of >> audit, we should not log even the nested DDL? > > Clearly if you log only DROP TABLE, and then the malicious user hides > one such call inside a function that executes the drop (which is called > via a SELECT top-level SQL), you're not going to be happy. If the purpose of the auditing it to catch malicious activity then all statements would need to be logged. If only top-level statements are logged then it might be harder to detect, but it would still be logged. -- - David Steele david@pgmasters.net
On 2/26/15 1:00 AM, Fujii Masao wrote: > On Thu, Feb 26, 2015 at 1:40 PM, Alvaro Herrera >> Clearly if you log only DROP TABLE, and then the malicious user hides >> one such call inside a function that executes the drop (which is called >> via a SELECT top-level SQL), you're not going to be happy. > > Yep, so what SQL should be logged in this case? Only "targeted" nested DDL? > Both top and nested ones? Seems the later is better to me. > > What about the case where the function A calls the function B executing DDL? > Every ancestor SQLs of the "targeted" DDL should be logged? Probably yes. Currently only the targeted nested DDL would be logged. However, it would log the top-level statement as well as the object that was dropped. Here's an example from the unit tests: do $$ begin create table test_block (id int); drop table test_block; end; $$ When pg_audit.log = 'function, ddl' the output will be: AUDIT: SESSION,FUNCTION,DO,,,do $$ begin create table test_block (id int); drop table test_block; end; $$ AUDIT: SESSION,DDL,CREATE TABLE,TABLE,public.test_block,do $$ begin create table test_block (id int); drop table test_block; end; $$ AUDIT: SESSION,DDL,DROP TABLE,TABLE,public.test_block,do $$ begin create table test_block (id int); drop table test_block; end; $$ You can see that in the create and drop audit entries the fully-qualified name is given. The statement comes from debug_query_string so it shows the top-level statement, even though more detail is given in the other fields when possible. If pg_audit.log = 'ddl' then the DO entry would not be logged even though statements under it were logged. At least, that's the way it works currently. -- - David Steele david@pgmasters.net
On Thu, Feb 19, 2015 at 12:29:18PM +0900, Fujii Masao wrote: > >> The pg_audit doesn't log BIND parameter values when prepared statement is used. > >> Seems this is an oversight of the patch. Or is this intentional? > > > > It's actually intentional - following the model I talked about in my > > earlier emails, the idea is to log statements only. > > Is this acceptable for audit purpose in many cases? Without the values, > I'm afraid that it's hard to analyze what table records are affected by > the statements from the audit logs. I was thinking that identifying the > data affected is one of important thing for the audit. If I'm malicious DBA, > I will always use the extended protocol to prevent the values from being > audited when I execute the statement. I added protocol-level bind() value logging for log_statement, and there were many requests for this functionality before I implemented it. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +