Re: security hook on table creation

(2010/10/01 11:23), Robert Haas wrote:
> On Sep 30, 2010, at 9:01 PM, KaiGai Kohei<kaigai@ak.jp.nec.com>  wrote:
>> (2010/10/01 3:09), Robert Haas wrote:
>>> 2010/9/29 KaiGai Kohei<kaigai@ak.jp.nec.com>:
>>>> In addition, I want to give these entrypoints its name which
>>>> represents an appropriate purpose of the hook, rather than
>>>> a uniformed one.
>>>
>>> It sounds like you're proposing to create a vast number of hooks
>>> rather than just one.  If we have ~20 object types in the system,
>>> that's 40 hooks just for create and drop, and then many more to handle
>>> comment, alter (perhaps in various flavors), etc.  I'm pretty
>>> unexcited about that.  The main hook function can always dispatch
>>> internally if it so desires, but I don't see any benefit to forcing
>>> people to write the code that way.
>>>
>> What I proposed is to create just one hook and wrapper functions
>> with appropriate name; that calls the hook with appropriate parameters,
>> such as SearchSysCache1, 2, 3 and 4.
> 
> Seems like you'd end up creating a lot of macros that were only used once.
> 

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.

See the following analysis:

Now we support to assign security label on the seven object classes enumerated
at ExecSecLabelStmt().

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.

During the discussion, we talked about which is more preferable design
(A) having both of prep/post creation hooks, or (B) having only post
creation hook. My original proposition is similar to the idea (A), but
we could not find out a good reason to justify (A) rather than simple
(B) a week ago.
However, again, it seems to me the idea (A) becomes better, because it
enables to avoid random injection of CommandCounterIncrement() in the
future.

So, I'd like to reconsider the idea with two hooks on creation time.

The issue of prep creation hook was that it needs per object class
definitions because we cannot pull-out information from SysCache
before the new database object being visible.
But it does not mean we eventually need (# of object classes) x (# of
access types) 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(...<relationparameters>...);       :   /* 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);
 
}

In this design, we can share the post-creation hook with any other object
classes, so all we need to provide for each object classes are just prep
creation hooks. It does not seem to me explosion of security hooks.

Could you give me your opinion to handle these problematic code paths?

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