Re: [HACKERS] PostgreSQL Audit Extension

2016-02-19 Thread Bruce Momjian 
On Fri, Feb 19, 2016 at 08:20:31AM -0800, Andres Freund wrote:
> On 2016-02-19 10:14:47 -0500, Bruce Momjian wrote:
> > We have already hesitated to record DDL changes for
> > logical replication because of the code size, maintenance overhead, and
> > testing required.
> 
> I'm not sure what you're referring to here? It'd be some relatively
> minor code surgery to also pass catalog changes to output plugins. It's
> just questionable what'd that bring to the table.

The complaint we got about recording DDL changes for logical replication
was the requirement to track every new DDL option we add.  If that
overhead can be done for both logical replication and auditing, it is a
win.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-19 Thread Bruce Momjian 
On Fri, Feb 19, 2016 at 11:20:13AM -0500, David Steele wrote:
> On 2/19/16 10:54 AM, Alvaro Herrera wrote:
> > Bruce Momjian wrote:
> > 
> >> Understood.  My point is that there is a short list of read events, and
> >> many DDL events.  We have already hesitated to record DDL changes for
> >> logical replication because of the code size, maintenance overhead, and
> >> testing required.
> > 
> > DDL is already captured using the event triggers mechanism (which is
> > what it was invented for in the first place).  The only thing we don't
> > have is a hardcoded mechanism to transform it from C struct format to
> > SQL language.
> 
> Since DDL event triggers only cover database-level DDL they miss a lot
> that is very important to auditing, e.g. CREATE/ALTER/DROP ROLE,
> GRANT/REVOKE, CREATE/ALTER/DROP DATABASE, etc.

Well, we need to enhance them then.

> I would like to see a general mechanism that allows event triggers,
> logical replication, and audit to all get the information they need
> without them being tied to each other directly.

I think the reporting of DDL would be produced in a way that could be
used by auditing or logical replication, as I already stated.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-19 Thread Andres Freund 
On 2016-02-19 10:14:47 -0500, Bruce Momjian wrote:
> We have already hesitated to record DDL changes for
> logical replication because of the code size, maintenance overhead, and
> testing required.

I'm not sure what you're referring to here? It'd be some relatively
minor code surgery to also pass catalog changes to output plugins. It's
just questionable what'd that bring to the table.

Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-19 Thread David Steele 
On 2/19/16 10:54 AM, Alvaro Herrera wrote:
> Bruce Momjian wrote:
> 
>> Understood.  My point is that there is a short list of read events, and
>> many DDL events.  We have already hesitated to record DDL changes for
>> logical replication because of the code size, maintenance overhead, and
>> testing required.
> 
> DDL is already captured using the event triggers mechanism (which is
> what it was invented for in the first place).  The only thing we don't
> have is a hardcoded mechanism to transform it from C struct format to
> SQL language.

Since DDL event triggers only cover database-level DDL they miss a lot
that is very important to auditing, e.g. CREATE/ALTER/DROP ROLE,
GRANT/REVOKE, CREATE/ALTER/DROP DATABASE, etc.

I would like to see a general mechanism that allows event triggers,
logical replication, and audit to all get the information they need
without them being tied to each other directly.

-- 
-David
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-19 Thread Bruce Momjian 
On Fri, Feb 19, 2016 at 12:54:17PM -0300, Alvaro Herrera wrote:
> Bruce Momjian wrote:
> 
> > Understood.  My point is that there is a short list of read events, and
> > many DDL events.  We have already hesitated to record DDL changes for
> > logical replication because of the code size, maintenance overhead, and
> > testing required.
> 
> DDL is already captured using the event triggers mechanism (which is
> what it was invented for in the first place).  The only thing we don't
> have is a hardcoded mechanism to transform it from C struct format to
> SQL language.

Right, which is I think were the maintenance/testing overhead will come
from, which we are trying to avoid.  Having logical replication and
auditing share that burden would be a win.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-19 Thread Alvaro Herrera 
Bruce Momjian wrote:

> Understood.  My point is that there is a short list of read events, and
> many DDL events.  We have already hesitated to record DDL changes for
> logical replication because of the code size, maintenance overhead, and
> testing required.

DDL is already captured using the event triggers mechanism (which is
what it was invented for in the first place).  The only thing we don't
have is a hardcoded mechanism to transform it from C struct format to
SQL language.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-19 Thread David Steele 
On 2/19/16 10:14 AM, Bruce Momjian wrote:
> On Fri, Feb 19, 2016 at 09:19:58AM -0500, David Steele wrote:
>>> I was suggesting we could track write events via logical replication and
>>> then we only have to figure out auditing of read events, which would be
>>> easier.
>>
>> I agree that DDL/DML audit logging requires a lot of the same
>> information as logical replication but I don't think reading the logical
>> WAL stream is a good way to do audit logging even for writes.
>>
>> For instance there are some "writes" that are not WAL logged such as SET
>> or ALTER SYSTEM.  Determining attribution would be difficult or
>> impossible, such as which function called an update statement (or vice
>> versa).  Tying together the read and write information would be tricky
>> as well since the WAL stream contains information on transactions but
>> not user sessions, if I understand it correctly.
>>
>> The end result is that it would be very difficult to record a coherent,
>> attributed, and sequential audit log and in fact represent a step
>> backward from pgaudit's current capabilities.
> 
> Understood.  My point is that there is a short list of read events, and
> many DDL events.  We have already hesitated to record DDL changes for
> logical replication because of the code size, maintenance overhead, and
> testing required.  If we could use the same facility for both logical
> replication and auditing, the cost overhead is less per feature.  For
> example, we don't need to read the WAL to do the auditing, but the same
> facility could be used for both, e.g. output some JSON standard format
> that both logical replication and auditing could understand.

Agreed, and this was discussed up thread.  In my mind putting a more
generic structure on top of logical replication and DDL auditing is a
promising solution but I have not looked at it closely enough to know if
it will work as expected or address maintenance concerns.

-- 
-David
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-19 Thread Bruce Momjian 
On Fri, Feb 19, 2016 at 09:19:58AM -0500, David Steele wrote:
> > I was suggesting we could track write events via logical replication and
> > then we only have to figure out auditing of read events, which would be
> > easier.
> 
> I agree that DDL/DML audit logging requires a lot of the same
> information as logical replication but I don't think reading the logical
> WAL stream is a good way to do audit logging even for writes.
> 
> For instance there are some "writes" that are not WAL logged such as SET
> or ALTER SYSTEM.  Determining attribution would be difficult or
> impossible, such as which function called an update statement (or vice
> versa).  Tying together the read and write information would be tricky
> as well since the WAL stream contains information on transactions but
> not user sessions, if I understand it correctly.
> 
> The end result is that it would be very difficult to record a coherent,
> attributed, and sequential audit log and in fact represent a step
> backward from pgaudit's current capabilities.

Understood.  My point is that there is a short list of read events, and
many DDL events.  We have already hesitated to record DDL changes for
logical replication because of the code size, maintenance overhead, and
testing required.  If we could use the same facility for both logical
replication and auditing, the cost overhead is less per feature.  For
example, we don't need to read the WAL to do the auditing, but the same
facility could be used for both, e.g. output some JSON standard format
that both logical replication and auditing could understand.

The bottom line is we need to crack the record-DDL nut somehow, and at
least in this case, we have two use-cases for it.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-19 Thread David Steele 
On 2/17/16 10:25 PM, Bruce Momjian wrote:
> On Wed, Feb 17, 2016 at 01:59:09PM +0530, Robert Haas wrote:
>> On Wed, Feb 17, 2016 at 5:20 AM, Bruce Momjian  wrote:
>>> On Fri, Feb  5, 2016 at 01:16:25PM -0500, Stephen Frost wrote:
 Looking at pgaudit and the other approaches to auditing which have been
 developed (eg: applications which sit in front of PG and essentially
 have to reimplement large bits of PG to then audit the commands sent
 before passing them to PG, or hacks which try to make sense out of log
 files full of SQL statements) make it quite clear, in my view, that
 attempts to bolt-on auditing to PG result in a poorer solution, from a
 technical perspective, than what this project is known for and capable
 of.  To make true progress towards that, however, we need to get past
 the thinking that auditing doesn't need to be in-core or that it should
 be a second-class citizen feature or that we don't need it in PG.
>>>
>>> Coming in late here, but the discussion around how to maintain the
>>> auditing code seems very similar to how to handle the logical
>>> replication of DDL commands.  First, have we looked into hooking
>>> auditing into scanning logical replication contents, and second, how are
>>> we handling the logical replication of DDL and could we use the same
>>> approach for auditing?
>>
>> Auditing needs to trace read-only events, which aren't reflected in
>> logical replication in any way.  I think it's a good idea to try to
>> drive auditing off of existing machinery instead of inventing
>> something new - I suggested plan invalidation items upthread.  But I
>> doubt that logical replication is the thing to attach it to.
> 
> I was suggesting we could track write events via logical replication and
> then we only have to figure out auditing of read events, which would be
> easier.

I agree that DDL/DML audit logging requires a lot of the same
information as logical replication but I don't think reading the logical
WAL stream is a good way to do audit logging even for writes.

For instance there are some "writes" that are not WAL logged such as SET
or ALTER SYSTEM.  Determining attribution would be difficult or
impossible, such as which function called an update statement (or vice
versa).  Tying together the read and write information would be tricky
as well since the WAL stream contains information on transactions but
not user sessions, if I understand it correctly.

The end result is that it would be very difficult to record a coherent,
attributed, and sequential audit log and in fact represent a step
backward from pgaudit's current capabilities.

-- 
-David
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-17 Thread Bruce Momjian 
On Wed, Feb 17, 2016 at 01:59:09PM +0530, Robert Haas wrote:
> On Wed, Feb 17, 2016 at 5:20 AM, Bruce Momjian  wrote:
> > On Fri, Feb  5, 2016 at 01:16:25PM -0500, Stephen Frost wrote:
> >> Looking at pgaudit and the other approaches to auditing which have been
> >> developed (eg: applications which sit in front of PG and essentially
> >> have to reimplement large bits of PG to then audit the commands sent
> >> before passing them to PG, or hacks which try to make sense out of log
> >> files full of SQL statements) make it quite clear, in my view, that
> >> attempts to bolt-on auditing to PG result in a poorer solution, from a
> >> technical perspective, than what this project is known for and capable
> >> of.  To make true progress towards that, however, we need to get past
> >> the thinking that auditing doesn't need to be in-core or that it should
> >> be a second-class citizen feature or that we don't need it in PG.
> >
> > Coming in late here, but the discussion around how to maintain the
> > auditing code seems very similar to how to handle the logical
> > replication of DDL commands.  First, have we looked into hooking
> > auditing into scanning logical replication contents, and second, how are
> > we handling the logical replication of DDL and could we use the same
> > approach for auditing?
> 
> Auditing needs to trace read-only events, which aren't reflected in
> logical replication in any way.  I think it's a good idea to try to
> drive auditing off of existing machinery instead of inventing
> something new - I suggested plan invalidation items upthread.  But I
> doubt that logical replication is the thing to attach it to.

I was suggesting we could track write events via logical replication and
then we only have to figure out auditing of read events, which would be
easier.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-17 Thread Robert Haas 
On Wed, Feb 17, 2016 at 5:20 AM, Bruce Momjian  wrote:
> On Fri, Feb  5, 2016 at 01:16:25PM -0500, Stephen Frost wrote:
>> Looking at pgaudit and the other approaches to auditing which have been
>> developed (eg: applications which sit in front of PG and essentially
>> have to reimplement large bits of PG to then audit the commands sent
>> before passing them to PG, or hacks which try to make sense out of log
>> files full of SQL statements) make it quite clear, in my view, that
>> attempts to bolt-on auditing to PG result in a poorer solution, from a
>> technical perspective, than what this project is known for and capable
>> of.  To make true progress towards that, however, we need to get past
>> the thinking that auditing doesn't need to be in-core or that it should
>> be a second-class citizen feature or that we don't need it in PG.
>
> Coming in late here, but the discussion around how to maintain the
> auditing code seems very similar to how to handle the logical
> replication of DDL commands.  First, have we looked into hooking
> auditing into scanning logical replication contents, and second, how are
> we handling the logical replication of DDL and could we use the same
> approach for auditing?

Auditing needs to trace read-only events, which aren't reflected in
logical replication in any way.  I think it's a good idea to try to
drive auditing off of existing machinery instead of inventing
something new - I suggested plan invalidation items upthread.  But I
doubt that logical replication is the thing to attach it to.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-16 Thread Bruce Momjian 
On Fri, Feb  5, 2016 at 01:16:25PM -0500, Stephen Frost wrote:
> Looking at pgaudit and the other approaches to auditing which have been
> developed (eg: applications which sit in front of PG and essentially
> have to reimplement large bits of PG to then audit the commands sent
> before passing them to PG, or hacks which try to make sense out of log
> files full of SQL statements) make it quite clear, in my view, that
> attempts to bolt-on auditing to PG result in a poorer solution, from a
> technical perspective, than what this project is known for and capable
> of.  To make true progress towards that, however, we need to get past
> the thinking that auditing doesn't need to be in-core or that it should
> be a second-class citizen feature or that we don't need it in PG.

Coming in late here, but the discussion around how to maintain the
auditing code seems very similar to how to handle the logical
replication of DDL commands.  First, have we looked into hooking
auditing into scanning logical replication contents, and second, how are
we handling the logical replication of DDL and could we use the same
approach for auditing?

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-05 Thread Stephen Frost 
All,

* Robert Haas (robertmh...@gmail.com) wrote:
> OK, I'll bite: I'm worried that this patch will be a maintenance
> burden.  It's easy to imagine that changes to core will result in the
> necessity or at least desirability of changes to pgaudit, but I'm
> definitely not prepared to insist that future authors try to insist
> that future patch submitters have to understand this code and update
> it as things change.

I agree with this concern in general, and have since pgaudit was first
proposed for inclusion by Simon two years ago.  Having auditing as an
extension is what makes this into an issue though.  Were auditing baked
into core, it'd be far more straight-forward, much easier to maintain,
and would be updated as we add new core capabilities naturally.  David's
comments about how pgaudit has to work are entirely accurate- everything
ends up having to be reconstructed in a very painful way.

> The set of things that the patch can audit is pretty arbitrary and not
> well tied into the core code.  

This also speaks to the difficulties of having auditing implemented as
an extension.

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Yeah.  Auditing strikes me as a fine example of something for which there
> is no *technical* reason to need to put it in core.  It might need some
> more hooks than we have now, but that's no big deal.

I disagree with this, quite strongly.  There are very good, technical,
reasons why auditing should be a core capability.  Adding more hooks,
exposing various internal functions for use by extensions, etc, doesn't
change that any extension would have to be constantly updated as new
capabilities are added to core and auditing, as a capability, would
continue to be limited by virtue of being implemented as an extension.

I don't mean to detract anything from what 2ndQ and David have done with
pgaudit (which is the only auditing solution of its kind that I'm aware
of, so I don't understand the "competing implementations" argument which
has been brought up at all- there's really only one).  They've done just
as much as each version of PG has allowed them to do.  It's a much
better solution than what we have today (which is basically just
log_statement = 'all', which no one should ever consider to be a real
auditing solution).  I've already stated that I'd be willing to take on
resonsibility for maintaining it as an extension (included with PG or
not), but it's not the end-all, be-all of auditing for PG and we should
be continuing to look at implementing a full in-core auditing solution.

I'm still of the opinion that we should include pgaudit for the reason
that it's a good incremental step towards proper in-core auditing, but
there's clearly no consensus on that and doesn't appear that further
discussion is likely to change that.

An in-core auditing solution would provide us with proper grammar
support, ability to directly mark objects for auditing in the catalog,
allow us to much more easily maintain auditing capability over time as
a small incremental bit of work for each new feature (with proper
in-core infrastructure for it) and generally be a far better technical
solution.  Leveraging the GRANT system is quite cute, and does work, but
it's certainly non-intuitive and is only because we've got no better
way, due to it being implemented as an extension.

Looking at pgaudit and the other approaches to auditing which have been
developed (eg: applications which sit in front of PG and essentially
have to reimplement large bits of PG to then audit the commands sent
before passing them to PG, or hacks which try to make sense out of log
files full of SQL statements) make it quite clear, in my view, that
attempts to bolt-on auditing to PG result in a poorer solution, from a
technical perspective, than what this project is known for and capable
of.  To make true progress towards that, however, we need to get past
the thinking that auditing doesn't need to be in-core or that it should
be a second-class citizen feature or that we don't need it in PG.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-05 Thread Stephen Frost 
* Joe Conway (m...@joeconway.com) wrote:
> On 02/05/2016 10:16 AM, Stephen Frost wrote:
> > An in-core auditing solution would provide us with proper grammar
> > support, ability to directly mark objects for auditing in the catalog,
> > allow us to much more easily maintain auditing capability over time as
> > a small incremental bit of work for each new feature (with proper
> > in-core infrastructure for it) and generally be a far better technical
> > solution.  Leveraging the GRANT system is quite cute, and does work, but
> > it's certainly non-intuitive and is only because we've got no better
> > way, due to it being implemented as an extension.
> 
> I think one additional item needed would be the ability for the audit
> logs to be sent to a different location than the standard logs.

Indeed, reworking the logging to be supportive of multiple destinations
with tagging of the source, etc, has long been a desire of mine (and
others), though that's largely independent of auditing itself.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-05 Thread Joe Conway 
On 02/05/2016 10:16 AM, Stephen Frost wrote:
> An in-core auditing solution would provide us with proper grammar
> support, ability to directly mark objects for auditing in the catalog,
> allow us to much more easily maintain auditing capability over time as
> a small incremental bit of work for each new feature (with proper
> in-core infrastructure for it) and generally be a far better technical
> solution.  Leveraging the GRANT system is quite cute, and does work, but
> it's certainly non-intuitive and is only because we've got no better
> way, due to it being implemented as an extension.

I think one additional item needed would be the ability for the audit
logs to be sent to a different location than the standard logs.

> To make true progress towards that, however, we need to get past
> the thinking that auditing doesn't need to be in-core or that it should
> be a second-class citizen feature or that we don't need it in PG.

+1

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-03 Thread Jim Nasby 

On 2/3/16 10:36 AM, Robert Haas wrote:

People who are interested in audit are also understandably leery of
>downloading code from an untrusted source.  Both PGXN and GitHub are The
>Wild West as far as conservative auditors are concerned.

I hate to be rude here, but that's not my problem.  You can put it on
your corporate web site and let people download it from there.  I'm
sure that auditors are familiar with the idea of downloading software
from for-profit companies.  Do they really not use any software from
Microsoft or Apple, for example?  If the problem is that they will
trust the PostgreSQL open source project but not YOUR company, then I
respectfully suggest that you need to establish the necessary
credibility, not try to piggyback on someone else's.


Luckily pgaudit is it's own group on Github 
(https://github.com/pgaudit), so it doesn't even have to be controlled 
by a single company. If others care about auditing I would hope that 
they'd contribute code there and eventually become a formal member of 
the pgaudit project.


As for PGXN being an untrusted source, that's something that it's in the 
project's best interest to try and address somehow, perhaps by having 
formally audited extensions. Amazon already has to do this to some 
degree before an extension can be allowed in RDS, and so does Heroku, so 
maybe that would be a starting point.


I think a big reason Postgres got to where it is today is because of 
it's superior extensibility, and I think continuing to encourage that 
with formal support for things like PGXN is important.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-03 Thread Tom Lane 
Jim Nasby  writes:
> As for PGXN being an untrusted source, that's something that it's in the 
> project's best interest to try and address somehow, perhaps by having 
> formally audited extensions. Amazon already has to do this to some 
> degree before an extension can be allowed in RDS, and so does Heroku, so 
> maybe that would be a starting point.

> I think a big reason Postgres got to where it is today is because of 
> it's superior extensibility, and I think continuing to encourage that 
> with formal support for things like PGXN is important.

Yeah.  Auditing strikes me as a fine example of something for which there
is no *technical* reason to need to put it in core.  It might need some
more hooks than we have now, but that's no big deal.  In the long run,
we'll be a lot better off if we can address the non-technical factors
that make people want to push such things into the core distribution.

Exactly how we get there, I don't pretend to know.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-03 Thread David Steele 
Hi Robert,

Thank you for replying.

On 2/1/16 11:23 PM, Robert Haas wrote:
> OK, I'll bite: I'm worried that this patch will be a maintenance
> burden.  It's easy to imagine that changes to core will result in the
> necessity or at least desirability of changes to pgaudit, but I'm
> definitely not prepared to insist that future authors try to insist
> that future patch submitters have to understand this code and update
> it as things change.

I agree this is a concern.  It's similar to deparse or event triggers in
this regard with the notable exception that pgaudit is not in core.

However, if it becomes popular enough out of core as everyone insists is
preferable then people will still need to maintain it.  Just as PostGIS
has a close relationship with core, the pgaudit team would need to have
the same sort of relationship.  Patches would be submitted for review
and (hopefully) committed and core developer time would still be spent
on pgaudit, ableit indirectly.  Core developers would still have to be
careful not to break pgaudit if it became popular enough.

> The set of things that the patch can audit is pretty arbitrary and not
> well tied into the core code.  

Since the set of what it can audit is every command that can be run by a
user in Postgres I don't see how that's arbitrary.  The amount of
*additional* detail provided for each audit record is also not arbitrary
but a function of what information is available through various hooks
and event triggers.

I was able to improve on the amount of information provided over the
original 2Q code (mostly by abandoning support for Postgres < 9.5) but
the methodology remains the same.

> There is a list of string constants in
> the code that covers each type of relations plus functions, but not
> any other kind of SQL object.  If somebody adds a new relkind, this
> would probably need to updated - it would not just work.  If somebody
> adds a new type of SQL object, it won't be covered unless the user
> takes some explicit action, but there's no obvious guiding principle
> to say whether that would be appropriate in any particular case.

I think a lot of this could be mitigated by some changes in utility.c.
I'm planning patches that will allow mapping command strings back to
event tags and a general classifier function that could incidentally be
used to improve the granularity of log_statement.

> In
> saying that it's arbitrary, I'm not saying it isn't *useful*.  I'm
> saying there could be five extensions like this that make equally
> arbitrary decisions about what to do and how to do it, and they could
> all be useful to different people.

There *could* be five extensions but there are not.  To my knowledge
there are two and one is just a more evolved version of the other.

> I don't really want to bless any
> given one of those current or hypothetical future solutions.  We have
> hooks precisely so that people can write stuff like this and put it up
> on PGXN or github or wherever - and this code can be published there,
> and people who want to can use it.

People who are interested in audit are also understandably leery of
downloading code from an untrusted source.  Both PGXN and GitHub are The
Wild West as far as conservative auditors are concerned.

Your use of the phrase "or wherever" only reinforces the point in my
mind.  The implication is that it doesn't matter where the pgaudit
extension comes from but it does matter, a lot.

> It also appears to me that if we did want to do that, it would need
> quite a lot of additional cleanup.  I haven't dug in enough to have a
> list of specific issues, but it does look to me like there would be
> quite a bit.  Maybe that'd be worth doing if there were other
> advantages of having this be in core, but I don't see them.

I'll be the first to admit that the design is not the prettiest.  Trying
to figure out what Postgres is doing internally through a couple of
hooks is like trying to replicate the script of a play when all you have
is the program.  However, so far it has been performed well and been
reliable in field tests.

-- 
-David
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-03 Thread Robert Haas 
On Wed, Feb 3, 2016 at 10:37 AM, David Steele  wrote:
> On 2/1/16 11:23 PM, Robert Haas wrote:
>> OK, I'll bite: I'm worried that this patch will be a maintenance
>> burden.  It's easy to imagine that changes to core will result in the
>> necessity or at least desirability of changes to pgaudit, but I'm
>> definitely not prepared to insist that future authors try to insist
>> that future patch submitters have to understand this code and update
>> it as things change.
>
> I agree this is a concern.  It's similar to deparse or event triggers in
> this regard with the notable exception that pgaudit is not in core.

I don't see event triggers as having the same issues, actually.  DDL
deparse - which I assume is what you mean by deparse - does, and I
complained about that too, quite a lot, and some work was done to
address it - I would have liked more.  One of the things I argued for
forcefully with regard to DDL deparse is that it needed to be able to
deparse every DDL command we have, not just the ones the authors
thought were most important.  I would expect no less from an auditing
facility.

> However, if it becomes popular enough out of core as everyone insists is
> preferable then people will still need to maintain it.  Just as PostGIS
> has a close relationship with core, the pgaudit team would need to have
> the same sort of relationship.  Patches would be submitted for review
> and (hopefully) committed and core developer time would still be spent
> on pgaudit, ableit indirectly.  Core developers would still have to be
> careful not to break pgaudit if it became popular enough.

This just isn't how it works.  I have no idea what the PostGIS folks
are doing, and they generally don't need to know what we're doing.
Occasionally we interact with each other, but mostly those two
different pieces of software can be developed by different people, and
that's a good thing.  Migrating PostGIS into PostgreSQL's core would
not be good for either project IMHO.  It is neither necessary nor
desirable to have multiple software projects all merged together in a
single git repo.

>> The set of things that the patch can audit is pretty arbitrary and not
>> well tied into the core code.
>
> Since the set of what it can audit is every command that can be run by a
> user in Postgres I don't see how that's arbitrary.

That's not what I'm talking about.  You audit relation access and
function calls but not, say, creation of event triggers.  Yes, you can
log every statement that comes in, but that's not the secret sauce:
log_statement=all will do that much.  The secret sauce is figuring out
the set of events that a statement might perform which might cause
that statement to generate audit records.  And it does not seem to me
that what you've got there right now is particularly general - you've
got relation access and function calls and a couple of other things,
but it's far from comprehensive.

>> There is a list of string constants in
>> the code that covers each type of relations plus functions, but not
>> any other kind of SQL object.  If somebody adds a new relkind, this
>> would probably need to updated - it would not just work.  If somebody
>> adds a new type of SQL object, it won't be covered unless the user
>> takes some explicit action, but there's no obvious guiding principle
>> to say whether that would be appropriate in any particular case.
>
> I think a lot of this could be mitigated by some changes in utility.c.
> I'm planning patches that will allow mapping command strings back to
> event tags and a general classifier function that could incidentally be
> used to improve the granularity of log_statement.

So, *this* starts to smell like a reason for core changes.  "I can't
really do what I want in my extension, but with these changes I could"
is an excellent reason to change core.

>> In
>> saying that it's arbitrary, I'm not saying it isn't *useful*.  I'm
>> saying there could be five extensions like this that make equally
>> arbitrary decisions about what to do and how to do it, and they could
>> all be useful to different people.
>
> There *could* be five extensions but there are not.  To my knowledge
> there are two and one is just a more evolved version of the other.

Right now that may be true, although it wouldn't surprise me very much
to find out that other people have written such extensions and they
just didn't get as much press.  Also, consider the future.  It is
*possible* that your version of pgaudit will turn out to be the be-all
and the end-all, but it's equally possible that somebody will fork
your version in turn and evolve it some more.  I don't see how you can
look at the pgaudit facilities you've got here and say that this is
the last word on auditing and all PostgreSQL users should be content
with exactly that facility.  I find that ridiculous.  Look me in the
eye and tell me that nobody's going to fork your version and evolve it
a bunch more.

> People who are

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-03 Thread David Steele 
On 2/3/16 11:36 AM, Robert Haas wrote:

> That's good to hear, but again, it's not enough for a core submission.
> Code that goes into our main git repository needs to be "the
> prettiest".  I mean it's not all perfect of course, but it should be
> pretty darn good.

I still think it's pretty darn good given what 2ndQuadrant and I had to
work with but I also think it could be a lot better with some core changes.

> Also, understand this: when you get a core submission accepted, the
> core project is then responsible for maintaining that code even if you
> disappear.  It's entirely reasonable for the project to demand that
> this isn't going to be too much work.  It's entirely reasonable for
> the community to want the design to be very good and the code quality
> to be high.  It's entirely reasonable for the community NOT to want to
> privilege one implementation over another.  If you don't agree that
> those things are reasonable then we disagree pretty fundamentally on
> the role of the community.  The community is a group of people to whom
> I (or you) can give our time and my (or your) code, not a group of
> people who owe me (or you) anything.

I think our differences are in the details and not in the general idea.
 In no way do I want to circumvent the process or get preferential
treatment for me or for my company.  I believe in the community but I
also believe that we won't get anywhere as a community unless
individuals give voice to their ideas.

I appreciate you taking the time to voice your opinion.  From my
perspective there's little to be gained in continuing to beat this
horse.  If the errhidefromclient() patch is accepted then that will be a
good step for pgaudit and I'll be on the lookout for other ways I can
both contribute useful code to core and move the pgaudit project forward.

-- 
-David
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-03 Thread David G. Johnston 
On Wed, Feb 3, 2016 at 9:36 AM, Robert Haas  wrote:

> On Wed, Feb 3, 2016 at 10:37 AM, David Steele  wrote:
> > On 2/1/16 11:23 PM, Robert Haas wrote:
>
> >> In
> >> saying that it's arbitrary, I'm not saying it isn't *useful*.  I'm
> >> saying there could be five extensions like this that make equally
> >> arbitrary decisions about what to do and how to do it, and they could
> >> all be useful to different people.
> >
> > There *could* be five extensions but there are not.  To my knowledge
> > there are two and one is just a more evolved version of the other.
>
> Right now that may be true, although it wouldn't surprise me very much
> to find out that other people have written such extensions and they
> just didn't get as much press.  Also, consider the future.  It is
> *possible* that your version of pgaudit will turn out to be the be-all
> and the end-all, but it's equally possible that somebody will fork
> your version in turn and evolve it some more.  I don't see how you can
> look at the pgaudit facilities you've got here and say that this is
> the last word on auditing and all PostgreSQL users should be content
> with exactly that facility.  I find that ridiculous.  Look me in the
> eye and tell me that nobody's going to fork your version and evolve it
> a bunch more.
>

​This rings hollow to me.  JSON got included with an admittedly weak
feature set and then got significantly improved upon in subsequent
releases.  Those who would be incline to fork pgaudit, seeing it already
being in core, would more likely and to the benefit of the community put
that work into improving the existing work.​  My off-the-cuff understanding
is that some current big features (namely the parallel-related stuff) are
also taking this "lets commit smaller but still useful pieces" into core to
build up to this super-feature we want working two years from now.  I don't
see any fundamental reason auditing couldn't be given the same opportunity
to improve inside core.

The other major downside of having it in core is that now the feature
release cycle is tied to core.  Telling PostGIS they can only release new
features when new versions of PostgreSQL come out would be an unacceptable
situation.

The best of both worlds would be for core to have its own implementation
written as an extension and to readily allow for other implementations to
be plugged in as well.  As your alluded to above there are likely a number
of things core really needs to enable such functionality without providing
the entire UI - leaving that for extensions.


> > People who are interested in audit are also understandably leery of
> > downloading code from an untrusted source.  Both PGXN and GitHub are The
> > Wild West as far as conservative auditors are concerned.
>
> I hate to be rude here, but that's not my problem.  You can put it on
> your corporate web site and let people download it from there.  I'm
> sure that auditors are familiar with the idea of downloading software
> from for-profit companies.  Do they really not use any software from
> Microsoft or Apple, for example?  If the problem is that they will
> trust the PostgreSQL open source project but not YOUR company, then I
> respectfully suggest that you need to establish the necessary
> credibility, not try to piggyback on someone else's.
>

​A bit short-sighted maybe.  Endorsing and including such a feature could
open PostgreSQL up to a​ new market being supported by people who right now
are not readily able to join the PostgreSQL community because they cannot
invest the necessary resources to get the horses put before the cart.
Those people, if they could get their clients to more easily use
PostgreSQL, may just find it worth their while to then contribute back to
this new frontier that has been opened up to them.  This would ideally
increase the number of contributors and reviewers within the community
which is the main thing that is presently needed.


> > I'll be the first to admit that the design is not the prettiest.  Trying
> > to figure out what Postgres is doing internally through a couple of
> > hooks is like trying to replicate the script of a play when all you have
> > is the program.  However, so far it has been performed well and been
> > reliable in field tests.
>
> That's good to hear, but again, it's not enough for a core submission.
> Code that goes into our main git repository needs to be "the
> prettiest".  I mean it's not all perfect of course, but it should be
> pretty darn good.
>
> Also, understand this: when you get a core submission accepted, the
> core project is then responsible for maintaining that code even if you
> disappear.  It's entirely reasonable for the project to demand that
> this isn't going to be too much work.  It's entirely reasonable for
> the community to want the design to be very good and the code quality
> to be high.


​So far so good...​

It's entirely reasonable for the community NOT to want to

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-03 Thread Robert Haas 
On Wed, Feb 3, 2016 at 12:38 PM, David G. Johnston
 wrote:
>> Right now that may be true, although it wouldn't surprise me very much
>> to find out that other people have written such extensions and they
>> just didn't get as much press.  Also, consider the future.  It is
>> *possible* that your version of pgaudit will turn out to be the be-all
>> and the end-all, but it's equally possible that somebody will fork
>> your version in turn and evolve it some more.  I don't see how you can
>> look at the pgaudit facilities you've got here and say that this is
>> the last word on auditing and all PostgreSQL users should be content
>> with exactly that facility.  I find that ridiculous.  Look me in the
>> eye and tell me that nobody's going to fork your version and evolve it
>> a bunch more.
>
> This rings hollow to me.  JSON got included with an admittedly weak feature
> set and then got significantly improved upon in subsequent releases.

True.  But most of that was adding, not changing.  Look, fundamentally
this is an opinion question, and everybody's entitled to an opinion on
whether pgaudit should be in core.  I have given mine; other people
can think about it differently.

> Those
> who would be incline to fork pgaudit, seeing it already being in core, would
> more likely and to the benefit of the community put that work into improving
> the existing work.  My off-the-cuff understanding is that some current big
> features (namely the parallel-related stuff) are also taking this "lets
> commit smaller but still useful pieces" into core to build up to this
> super-feature we want working two years from now.  I don't see any
> fundamental reason auditing couldn't be given the same opportunity to
> improve inside core.

Well, it means that every change has to be dealt with by a PostgreSQL
committer, for one thing.  We don't have a ton of people who have the
skillset for that, the time to work on it, and the community's trust.

> The other major downside of having it in core is that now the feature
> release cycle is tied to core.  Telling PostGIS they can only release new
> features when new versions of PostgreSQL come out would be an unacceptable
> situation.

Yep.

> The best of both worlds would be for core to have its own implementation
> written as an extension and to readily allow for other implementations to be
> plugged in as well.  As your alluded to above there are likely a number of
> things core really needs to enable such functionality without providing the
> entire UI - leaving that for extensions.

I really think this is not for the best.  People who write non-core
extensions are often quite unhappy when core gets a feature that is
even somewhat similar, because they feel that this takes attention
away from their work in favor of the not-necessarily-better work that
went into core.

> A bit short-sighted maybe.  Endorsing and including such a feature could
> open PostgreSQL up to a new market being supported by people who right now
> are not readily able to join the PostgreSQL community because they cannot
> invest the necessary resources to get the horses put before the cart.  Those
> people, if they could get their clients to more easily use PostgreSQL, may
> just find it worth their while to then contribute back to this new frontier
> that has been opened up to them.  This would ideally increase the number of
> contributors and reviewers within the community which is the main thing that
> is presently needed.

This is based, though, on the idea that they must not only have the
feature but they must have it in the core distribution.  And I'm
simply not willing to endorse that as a reason to put things in core.
Maybe it would be good for PostgreSQL adoption, but if everything that
somebody won't use unless it's in core goes in core, core will become
a bloated, stinking mess.

>> > I'll be the first to admit that the design is not the prettiest.  Trying
>> It's entirely reasonable for the community NOT to want to
>> privilege one implementation over another.
>
> This, not so much.

No, this is ABSOLUTELY critical.  Suppose EnterpriseDB writes an
auditing solution, 2ndQuadrant writes an auditing solution, and Cruncy
Data writes an auditing solution, and the community then picks one of
those to put in core.  Do you not think that the other two companies
will feel like they got the fuzzy end of the lollipop?  The only time
this sort of thing doesn't provoke hard feelings is when everybody
agrees that the solution that was finally adopted was way better than
the competing things.  If you don't think this is a problem, I
respectfully suggest that you haven't seen enough of these situations
play out.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-03 Thread Joshua D. Drake 

On 02/03/2016 10:36 AM, Robert Haas wrote:


I'll be the first to admit that the design is not the prettiest.  Trying

It's entirely reasonable for the community NOT to want to
privilege one implementation over another.


This, not so much.


No, this is ABSOLUTELY critical.  Suppose EnterpriseDB writes an
auditing solution, 2ndQuadrant writes an auditing solution, and Cruncy
Data writes an auditing solution, and the community then picks one of
those to put in core.  Do you not think that the other two companies
will feel like they got the fuzzy end of the lollipop?  The only time
this sort of thing doesn't provoke hard feelings is when everybody
agrees that the solution that was finally adopted was way better than
the competing things.  If you don't think this is a problem, I
respectfully suggest that you haven't seen enough of these situations
play out.



I am on the fence with this one because we should not care about what a 
company feels, period. We are not a business, we are not an employer. We 
are a community of people not companies.


On the other hand, I do very much understand what you are saying here 
and it is a difficult line to walk.


Then on the third hand (for those of us that were cloned and have 
issues), those companies chose PostgreSQL as their base, that is *their* 
problem, not ours. We also have to be considerate of the fact that those 
companies to contribute a lot to the community.


In short, may the best solution for the community win. Period.

Sincerely,

JD

--
Command Prompt, Inc.  http://the.postgres.company/
+1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Everyone appreciates your honesty, until you are honest with them.


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-01 Thread David Steele 
On 1/31/16 8:07 AM, Alvaro Herrera wrote:
> David Steele wrote:
>> The attached patch implements audit logging for PostgreSQL as an
>> extension.  I believe I have addressed the concerns that were raised at
>> the end of the 9.5 development cycle.
> 
> This patch got no feedback at all during the commitfest.  I think there
> is some interest on auditing capabilities so it's annoying and
> surprising that this has no movement at all.

Yes, especially since I've had queries at conferences, by email, and
once on this list asking if it would be submitted again.  I would not
have taken the time to prepare it if I hadn't been getting positive
feedback.

> If the lack of activity means lack of interest, please can we all keep
> what we've been doing in this thread, which is to ignore it, so that we
> can just mark this as rejected.

If nobody comments or reviews then that would be the best course of action.

> I'm giving
> this patch some more time by moving it to next commitfest instead.

I was under the mistaken impression that the CF was two months long so I
appreciate getting more time.

Thanks,
-- 
-David
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-01 Thread Alvaro Herrera 
David Steele wrote:
> On 1/31/16 8:07 AM, Alvaro Herrera wrote:

> > I'm giving
> > this patch some more time by moving it to next commitfest instead.
> 
> I was under the mistaken impression that the CF was two months long so I
> appreciate getting a little more time.

Yeah, sorry to disappoint but my intention is to close shop as soon as
possible.  Commitfests are supposed to last one month only, leaving the
month before the start of the next one free so that committers and
reviewers can enjoy life.

Anyway, what would *you* do with any additional time?  It's reviewers
that need to chime in when a patch is in needs-review state.  You
already did your part by submitting.  Anyway I think the tests here are
massive and the code is not; perhaps people get the mistaken impression
that this is a huge amount of code which scares them.  Perhaps you could
split it up in (1) code and (2) tests, which wouldn't achieve any
technical benefit but would offer some psychological comfort to
potential reviewers.  You know it's all psychology in these parts.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-01 Thread Tom Lane 
Alvaro Herrera  writes:
>   Anyway I think the tests here are
> massive and the code is not; perhaps people get the mistaken impression
> that this is a huge amount of code which scares them.  Perhaps you could
> split it up in (1) code and (2) tests, which wouldn't achieve any
> technical benefit but would offer some psychological comfort to
> potential reviewers.  You know it's all psychology in these parts.

Perhaps the tests could be made less bulky.  We do not need massive
permanent regression tests for a single feature, IMO.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-01 Thread David Steele 
[I pulled this back in from the other thread that got started.  This
happened because my email to pgsql-hackers bounced but was also sent to
Alvaro who replied to that message not the later one that went through
to hackers.]

On 2/1/16 3:59 PM, Alvaro Herrera wrote:
> Anyway, what would *you* do with any additional time? It's reviewers
> that need to chime in when a patch is in needs-review state. You
> already did your part by submitting. Anyway I think the tests
> here are massive and the code is not; perhaps people get the mistaken
> impression that this is a huge amount of code which scares them.
> Perhaps you could split it up in (1) code and (2) tests, which
> wouldn't achieve any technical benefit but would offer some
> psychological comfort to potential reviewers.

Good suggestion, so that's exactly what I've done.  Attached are a set
of patches that apply cleanly against 1d0c3b3.

* 01: Add errhidefromclient() macro for ereport

This is the same patch that I submitted here:

https://commitfest.postgresql.org/9/431/

but since pgaudit requires it I added it to this set for convenience.

* 02: The pgaudit code

And make file, etc.  Everything needed to get it compiled and installed
as an extension.

* 03: Docs and regression tests

To run regression tests this patch will need to be applied.  They have
been separated to make the code patch not look so large and hopefully
encourage reviews.

-- 
-David
da...@pgmasters.net
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 9005b26..c53ef95 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -1091,6 +1091,23 @@ errhidecontext(bool hide_ctx)
return 0;   /* return value does 
not matter */
 }
 
+/*
+ * errhidefromclient --- optionally suppress output of message to client
+ *
+ * Only log levels below ERROR can be hidden from the client.
+ */
+int
+errhidefromclient(bool hide_from_client)
+{
+   ErrorData  *edata = [errordata_stack_depth];
+
+   /* we don't bother incrementing recursion_depth */
+   CHECK_STACK_DEPTH();
+
+   edata->hide_from_client = hide_from_client;
+
+   return 0;   /* return value does 
not matter */
+}
 
 /*
  * errfunction --- add reporting function name to the current error
@@ -1467,7 +1484,7 @@ EmitErrorReport(void)
send_message_to_server_log(edata);
 
/* Send to client, if enabled */
-   if (edata->output_to_client)
+   if (edata->output_to_client && !edata->hide_from_client)
send_message_to_frontend(edata);
 
MemoryContextSwitchTo(oldcontext);
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 326896f..14b87b7 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -179,6 +179,7 @@ extern int  errcontext_msg(const char *fmt,...) 
pg_attribute_printf(1, 2);
 
 extern int errhidestmt(bool hide_stmt);
 extern int errhidecontext(bool hide_ctx);
+extern int errhidefromclient(bool hide_from_client);
 
 extern int errfunction(const char *funcname);
 extern int errposition(int cursorpos);
@@ -343,6 +344,7 @@ typedef struct ErrorData
boolshow_funcname;  /* true to force funcname inclusion */
boolhide_stmt;  /* true to prevent STATEMENT: 
inclusion */
boolhide_ctx;   /* true to prevent CONTEXT: 
inclusion */
+   boolhide_from_client;   /* true to prevent client 
output */
const char *filename;   /* __FILE__ of ereport() call */
int lineno; /* __LINE__ of 
ereport() call */
const char *funcname;   /* __func__ of ereport() call */
diff --git a/contrib/Makefile b/contrib/Makefile
index bd251f6..0046610 100644
--- a/contrib/Makefile
+++ b/contrib/Makefile
@@ -34,6 +34,7 @@ SUBDIRS = \
pg_standby  \
pg_stat_statements \
pg_trgm \
+   pgaudit \
pgcrypto\
pgrowlocks  \
pgstattuple \
diff --git a/contrib/pgaudit/.gitignore b/contrib/pgaudit/.gitignore
new file mode 100644
index 000..e8d2612
--- /dev/null
+++ b/contrib/pgaudit/.gitignore
@@ -0,0 +1,8 @@
+log/
+results/
+tmp_check/
+regression.diffs
+regression.out
+*.o
+*.so
+.vagrant
diff --git a/contrib/pgaudit/LICENSE b/contrib/pgaudit/LICENSE
new file mode 100644
index 000..998f814
--- /dev/null
+++ b/contrib/pgaudit/LICENSE
@@ -0,0 +1,4 @@
+This code is released under the PostgreSQL licence, as given at
+http://www.postgresql.org/about/licence/
+
+Copyright is novated to the PostgreSQL Global Development Group.
diff --git a/contrib/pgaudit/Makefile b/contrib/pgaudit/Makefile
new file mode 100644
index 000..b3a488b
--- /dev/null
+++ b/contrib/pgaudit/Makefile
@@ -0,0 +1,22 @@
+#

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-01 Thread David Steele 
On 2/1/16 4:19 PM, Tom Lane wrote:
> Alvaro Herrera  writes:
>>   Anyway I think the tests here are
>> massive and the code is not; perhaps people get the mistaken impression
>> that this is a huge amount of code which scares them.  Perhaps you could
>> split it up in (1) code and (2) tests, which wouldn't achieve any
>> technical benefit but would offer some psychological comfort to
>> potential reviewers.  You know it's all psychology in these parts.
> 
> Perhaps the tests could be made less bulky.  We do not need massive
> permanent regression tests for a single feature, IMO.

I'd certainly like to but pgaudit uses a lot of different techniques to
log various commands and there are a number of GUCs.  Each test provides
coverage for a different code path.

I'm sure they could be reorganized and tightened up a but I don't think
by a whole lot.

-- 
-David
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-01 Thread Joshua D. Drake 

On 02/01/2016 08:23 PM, Robert Haas wrote:


It also appears to me that if we did want to do that, it would need
quite a lot of additional cleanup.  I haven't dug in enough to have a
list of specific issues, but it does look to me like there would be
quite a bit.  Maybe that'd be worth doing if there were other
advantages of having this be in core, but I don't see them.



Thus... it is a great extension and doesn't need to be in core.

JD


--
Command Prompt, Inc.  http://the.postgres.company/
+1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Everyone appreciates your honesty, until you are honest with them.


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-01 Thread Noah Misch 
On Mon, Feb 01, 2016 at 09:59:01PM +0100, Alvaro Herrera wrote:
> Anyway I think the tests here are massive

I would not want to see fewer tests.  When I reviewed a previous incarnation
of pgaudit, the tests saved me hours of writing my own.

> and the code is not;

Nah; the patch size category is the same regardless of how you arrange the
tests.  The tests constitute less than half of the overall change.

> perhaps people get the mistaken impression
> that this is a huge amount of code which scares them.  Perhaps you could
> split it up in (1) code and (2) tests, which wouldn't achieve any
> technical benefit but would offer some psychological comfort to
> potential reviewers.  You know it's all psychology in these parts.

That's harmless.  If it makes someone feel better, great.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-02-01 Thread Robert Haas 
On Sun, Jan 31, 2016 at 5:19 PM, Alvaro Herrera
 wrote:
> Joshua D. Drake wrote:
>> On 01/31/2016 05:07 AM, Alvaro Herrera wrote:
>> >David Steele wrote:
>> >>The attached patch implements audit logging for PostgreSQL as an
>> >>extension.  I believe I have addressed the concerns that were raised at
>> >>the end of the 9.5 development cycle.
>> >
>> >This patch got no feedback at all during the commitfest.  I think there
>> >is some interest on auditing capabilities so it's annoying and
>> >surprising that this has no movement at all.
>> >
>> >If the lack of activity means lack of interest, please can we all keep
>> >what we've been doing in this thread, which is to ignore it, so that we
>> >can just mark this as rejected.  Otherwise, please chime in.  I'm giving
>> >this patch some more time by moving it to next commitfest instead.
>>
>> From my perspective, lack of activity means since it doesn't have a
>> technical requirement to be in -core, it doesn't need to be.
>
> Well, from mine it doesn't mean that.  We kind of assume that "no answer
> means yes"; but here what it really means is that nobody had time to
> look at it and think about it, so it stalled (and so have many other
> patches BTW).  But if you or others think that this patch belongs into
> PGXN, it's good to have that opinion in an email, so that the author can
> think about your perspective and agree or disagree with it.  Just by
> expression that opinion, a thousand other hackers might vote +1 or -1 on
> your proposal -- either way a clear sign.  Silence doesn't let us move
> forward, and we punt the patch to the next CF and let inertia continue.
> That's not good.

OK, I'll bite: I'm worried that this patch will be a maintenance
burden.  It's easy to imagine that changes to core will result in the
necessity or at least desirability of changes to pgaudit, but I'm
definitely not prepared to insist that future authors try to insist
that future patch submitters have to understand this code and update
it as things change.

The set of things that the patch can audit is pretty arbitrary and not
well tied into the core code.  There is a list of string constants in
the code that covers each type of relations plus functions, but not
any other kind of SQL object.  If somebody adds a new relkind, this
would probably need to updated - it would not just work.  If somebody
adds a new type of SQL object, it won't be covered unless the user
takes some explicit action, but there's no obvious guiding principle
to say whether that would be appropriate in any particular case.  In
saying that it's arbitrary, I'm not saying it isn't *useful*.  I'm
saying there could be five extensions like this that make equally
arbitrary decisions about what to do and how to do it, and they could
all be useful to different people.  I don't really want to bless any
given one of those current or hypothetical future solutions.  We have
hooks precisely so that people can write stuff like this and put it up
on PGXN or github or wherever - and this code can be published there,
and people who want to can use it.

It also appears to me that if we did want to do that, it would need
quite a lot of additional cleanup.  I haven't dug in enough to have a
list of specific issues, but it does look to me like there would be
quite a bit.  Maybe that'd be worth doing if there were other
advantages of having this be in core, but I don't see them.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-01-31 Thread Alvaro Herrera 
Joshua D. Drake wrote:
> On 01/31/2016 05:07 AM, Alvaro Herrera wrote:
> >David Steele wrote:
> >>The attached patch implements audit logging for PostgreSQL as an
> >>extension.  I believe I have addressed the concerns that were raised at
> >>the end of the 9.5 development cycle.
> >
> >This patch got no feedback at all during the commitfest.  I think there
> >is some interest on auditing capabilities so it's annoying and
> >surprising that this has no movement at all.
> >
> >If the lack of activity means lack of interest, please can we all keep
> >what we've been doing in this thread, which is to ignore it, so that we
> >can just mark this as rejected.  Otherwise, please chime in.  I'm giving
> >this patch some more time by moving it to next commitfest instead.
> 
> From my perspective, lack of activity means since it doesn't have a
> technical requirement to be in -core, it doesn't need to be.

Well, from mine it doesn't mean that.  We kind of assume that "no answer
means yes"; but here what it really means is that nobody had time to
look at it and think about it, so it stalled (and so have many other
patches BTW).  But if you or others think that this patch belongs into
PGXN, it's good to have that opinion in an email, so that the author can
think about your perspective and agree or disagree with it.  Just by
expression that opinion, a thousand other hackers might vote +1 or -1 on
your proposal -- either way a clear sign.  Silence doesn't let us move
forward, and we punt the patch to the next CF and let inertia continue.
That's not good.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-01-31 Thread Joshua D. Drake 

On 01/31/2016 05:07 AM, Alvaro Herrera wrote:

David Steele wrote:

The attached patch implements audit logging for PostgreSQL as an
extension.  I believe I have addressed the concerns that were raised at
the end of the 9.5 development cycle.


This patch got no feedback at all during the commitfest.  I think there
is some interest on auditing capabilities so it's annoying and
surprising that this has no movement at all.

If the lack of activity means lack of interest, please can we all keep
what we've been doing in this thread, which is to ignore it, so that we
can just mark this as rejected.  Otherwise, please chime in.  I'm giving
this patch some more time by moving it to next commitfest instead.


From my perspective, lack of activity means since it doesn't have a 
technical requirement to be in -core, it doesn't need to be.


That said, it is certainly true that people express interest in auditing 
(all the time). So I think we need to either punt it as something that 
is going to be an external extension or someone needs to pick it up.


It used to be that there was a philosophy of we built what people need 
to build other things. This is why replication for the longest time 
wasn't incorporated. If that is still our philosophy I don't even know 
why the audit extension is still a question.


Sincerely,

JD








--
Command Prompt, Inc.  http://the.postgres.company/
+1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PostgreSQL Audit Extension

2016-01-31 Thread Alvaro Herrera 
David Steele wrote:
> The attached patch implements audit logging for PostgreSQL as an
> extension.  I believe I have addressed the concerns that were raised at
> the end of the 9.5 development cycle.

This patch got no feedback at all during the commitfest.  I think there
is some interest on auditing capabilities so it's annoying and
surprising that this has no movement at all.

If the lack of activity means lack of interest, please can we all keep
what we've been doing in this thread, which is to ignore it, so that we
can just mark this as rejected.  Otherwise, please chime in.  I'm giving
this patch some more time by moving it to next commitfest instead.


-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers