Re: pgaudit - an auditing extension for PostgreSQL

On 14 October 2014 13:57, Stephen Frost <sfrost(at)snowman(dot)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(at)2ndQuadrant(dot)com) wrote:
> On 14 October 2014 13:57, Stephen Frost <sfrost(at)snowman(dot)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(at)2ndQuadrant(dot)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

On 14 October 2014 20:33, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> wrote:
> At 2014-10-14 20:09:50 +0100, simon(at)2ndQuadrant(dot)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(at)2ndQuadrant(dot)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

Abhijit Menon-Sen <ams(at)2ndQuadrant(dot)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