Re: Label switcher function

(2010/11/18 11:30), Robert Haas wrote:
> 2010/11/17 KaiGai Kohei<kaigai@ak.jp.nec.com>:
>> I revised my patch as I attached.
>>
>> The hook function is modified and consolidated as follows:
>>
>>   typedef enum FunctionCallEventType
>>   {
>>      FCET_BE_HOOKED,
>>      FCET_PREPARE,
>>      FCET_START,
>>      FCET_END,
>>      FCET_ABORT,
>>   } FunctionCallEventType;
>>
>>   typedef Datum (*function_call_event_type)(Oid functionId,
>>                                             FunctionCallEventType event,
>>                                             Datum event_arg);
>>   extern PGDLLIMPORT function_call_event_type function_call_event_hook;
>>
>> Unlike the subject of this e-mail, now it does not focus on only switching
>> security labels during execution of a certain functions.
>> For example, we may use this hook to track certain functions for security
>> auditing, performance tuning, and others.
>>
>> In the case of SE-PgSQL, it shall return BoolGetDatum(true), if the target
>> function is configured as a trusted procedure, then, this invocation will
>> be hooked by fmgr_security_definer. In the first call, it shall compute
>> the security context to be assigned during execution on FCET_PREPARE event.
>> Then, it switches to the computed label on the FCET_START event, and
>> restore it on the FCET_END or ECET_ABORT event.
> 
> This seems like it's a lot simpler than before, which is good.  It
> looks to me as though there should really be two separate hooks,
> though, one for what is now FCET_BE_HOOKED and one for everything
> else.  For FCET_BE_HOOKED, you want a function that takes an Oid and
> returns a bool.  For the other event types, the functionId and event
> arguments are OK, but I think you should forget about the save_datum
> stuff and just always pass fcache->flinfo and&fcache->private.  The
> plugin can get the effect of save_datum by passing around whatever
> state it needs to hold on to using fcache->private.  So:
> 
> bool (*needs_function_call_hook)(Oid fn_oid);
> void (*function_call_hook)(Oid fn_oid, FunctionCallEventType event,
> FmgrInfo flinfo, Datum *private);
> 
It seems to me a good idea. The characteristic of FCET_BE_HOOKED event
type was a bit different from other three event types.
Please wait for about a week to revise my patch.

> Another general comment is that you've not done a very complete job
> updating the comments; there are several of them in fmgr.c that are no
> longer accurate.  Also, please zap the unnecessary whitespace changes.
> 

Indeed, the comment at middle of the fmgr_info_cxt_security() and just
above definition of the fmgr_security_definer() are not correct.
Did you notice anything else?

Thanks,
-- 
KaiGai Kohei <kaigai@ak.jp.nec.com>