Re: security hook on table creation

(2010/10/07 6:02), Robert Haas wrote:
> 2010/10/5 KaiGai Kohei<kaigai@ak.jp.nec.com>:
>> I began to revise the security hooks, but I could find a few cases that does
>> not work with the approach of post-creation security hooks.
>> If we try to fix up the core PG routine to become suitable for the post-creation
>> security hooks, it shall need to put more CommandCounterIncrement() on various
>> places, so it made me doubtful whether this approach gives really minimum impact
>> to the core PG routine, or not.
> 
> We definitely don't want to add CCIs without a good reason.
> 
>> Some of object classes have CommandCounterIncrement() just after the routine
>> to create a new object. For example, DefineRelation() calls it just after the
>> heap_create_with_catalog(), so the new relation entry is visible for plugin
>> modules using SearchSysCache(), as long as we put the post-creation security
>> hook aftere the CommandCounterIncrement().
>>
>> However, we also have a few headache cases.
>> DefineType() creates a new type object and its array type, but it does not
>> call CommandCounterIncrement() by the end of this function, so the new type
>> entries are not visible from the plugin modules, even if we put a security
>> hook at tail of the DefineType().
>> DefineFunction() also has same matter. It create a new procedure object,
>> but it also does not call CommandCounterIncrement() by the end of this
>> function, except for the case when ProcedureCreate() invokes language
>> validator function.
> 
> So I guess the first question here is why it's important to be able to
> see the new entry.  I am thinking that you want it so that, for
> example, you can fetch the namespace OID to perform an SE-Linux type
> transition.  Is that right?
> 
We assumed that namespace OID can be fetched from the entry of pg_class,
so we thought the common InvokeObjectAccessHook() dose not need to take
many arguments, because we can pull out corresponding properties of new
object (such as namespace OID) from SysCache using OID of new object.

So, InvokeObjectAccessHook() must deliver OID of the namespace, rather
than OID of the new object, if it is not visible.

> Maybe we need a variant of InvokeObjectAccessHook that does a CCI only
> if a hook is present.  I can't see that we're going to want to pay for
> that unconditionally, but it shouldn't surprise anyone that loading an
> enhanced security provider comes with some overhead.
> 
The reason why we tried to move the object creation hooks into post
object creation was to reduce number of security hooks and burden of
code maintenance.

However, it seems to me paying attention for object visibility gives
us more burden rather than we have multiple creation hooks.

>> E.g, it may be possible to design creation of relation as follows:
>>
>> DefineRelation(...)
>> {
>>     /* DAC permission checks here */
>>         :
>>     /* MAC permission checks; it returns its private data */
>>     opaque = check_relation_create(...<relation parameters>...);
>>         :
>>     /* insertion into pg_class catalog */
>>     relationId = heap_create_with_catalog(...);
>>         :
>>     /* assign security labels on the new object */
>>     InvokeObjectAccessHook(OBJECT_TABLE, OAT_POST_CREATE,
>>                            relationId, 0, opaque);
>> }
> 
> I'd like to try to avoid that, if we can.  That's going to make this
> code far more complex to understand and maintain.
> 
Against our feeling, I consider the idea of two hooks help us to
understand and maintain security hooks in the future.
Because the place where we should put the prep-creation hook is
just after the default privilege checks, it is quite obvious.

If we would have only post-creation hook, is it obvious where we
should put the security hook on function creation, for example?

In the case when we have two hooks, obviously, the prep-creation
hook will be after the DAC checks, and the post-creation hook will
be after the insert/update of system catalogs.
It seems to me quite easy to understand and maintain.

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