Re: security hook on table creation

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?

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.

> 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.

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