Re: Auditing extension for PostgreSQL (Take 2)

On 3/25/15 7:46 AM, Sawada Masahiko wrote:
> On Wed, Mar 25, 2015 at 12:23 PM, David Steele <david(at)pgmasters(dot)net> wrote:
>>> On Wed, Mar 25, 2015 at 12:38 AM, David Steele <david(at)pgmasters(dot)net> wrote:
>>>>> 2. OBJECT auditing does not work before adding acl info to pg_class.rel_acl.
>>>>> In following situation, pg_audit can not audit OBJECT log.
>>>>> $ cat postgresql.conf | grep audit
>>>>> shared_preload_libraries = 'pg_audit'
>>>>> pg_audit.role = 'hoge_user'
>>>>> pg_audit.log = 'read, write'
>>>>> $ psql -d postgres -U hoge_user
>>>>> =# create table hoge(col int);
>>>>> =# select * from hoge;
>>>>> LOG: AUDIT: SESSION,3,1,READ,SELECT,,,select * from hoge;
>>>>>
>>>>> OBJECT audit log is not logged here since pg_class.rel_acl is empty
>>>>> yet. (Only logged SESSION log)
>>>>> So after creating another unconcerned role and grant any privilege to that user,
>>>>> OBJECT audit is logged successfully.
>>>>
>>>> Yes, object auditing does not work until some grants have been made to
>>>> the audit role.
>>>>
>>>>> =# create role bar_user;
>>>>> =# grant select on hoge to bar_user;
>>>>> =# select * from hoge;
>>>>> LOG: AUDIT: SESSION,4,1,READ,SELECT,,,select * from hoge;
>>>>> LOG: AUDIT: OBJECT,4,1,READ,SELECT,TABLE,public.hoge,select * from hoge;
>>>>>
>>>>> The both OBJCET and SESSION log are logged.
>>>>
>>>> Looks right to me. If you don't want the session logging then disable
>>>> pg_audit.log.
>>>>
>>>> Session and object logging are completely independent from each other:
>>>> one or the other, or both, or neither can be enabled at any time.
>>>
>>> It means that OBJECT log is not logged just after creating table, even
>>> if that table is touched by its owner.
>>> To write OBJECT log, we need to grant privilege to role at least. right?
>>
>> Exactly. Privileges must be granted to the audit role in order for
>> object auditing to work.
>>
>
> The table owner always can touch its table.
> So does it lead that table owner can get its table information while
> hiding OBJECT logging?

Yes, the table owner would be able to access the table without object
logging if grants to that table were not made to the audit role. That
would also be true for any other user that had grants on the table.

The purpose of object auditing is to allow more fine-grained control and
is intended to be used in situations where you only want to audit some
things, rather than all things. Logging everything is better done with
the session logging.

However, object logging does yield more information since it lists every
table that was touched by the statement, so there may be cases where
you'd like to object log everything. In that case I'd recommend writing
a bit of plpgsql code to create the grants.

> Also I looked into latest patch again.
> Here are two review comment.
>
> 1.
>> typedef struct
>> {
>> int64 statementId;
>> int64 substatementId;
> Both statementId and substatementId could be negative number.
> I think these should be uint64 instead.

True. I did this because printf formatting for uint64 seems to be vary
across platforms. int64 formatting is more standard and still gives
more than enough IDs.

I could change it back to uint64 if you have a portable way to modify
the sprintf at line 507.

> 2.
> I got ERROR when executing function uses cursor.
>
> 1) create empty table (hoge table)
> 2) create test function as follows.
>
> create function test() returns int as $$
> declare
> cur1 cursor for select * from hoge;
> tmp int;
> begin
> open cur1;
> fetch cur1 into tmp;
> return tmp;
> end$$
> language plpgsql ;
>
> 3) execute test function (got ERROR)
> =# select test();
> LOG: AUDIT: SESSION,6,1,READ,SELECT,,,selecT test();
> LOG: AUDIT: SESSION,6,2,FUNCTION,EXECUTE,FUNCTION,public.test,selecT test();
> LOG: AUDIT: SESSION,6,3,READ,SELECT,,,select * from hoge
> CONTEXT: PL/pgSQL function test() line 6 at OPEN
> ERROR: pg_audit stack is already empty
> STATEMENT: selecT test();
>
> It seems like that the item in stack is already freed by deleting
> pg_audit memory context (in MemoryContextDelete()),
> before calling stack_pop in dropping of top-level Portal.

Good catch, I'll add this to my test cases and work on a fix. I think I
see a good way to approach it.

--
- David Steele
david(at)pgmasters(dot)net