Re: [v9.2] sepgsql's DROP Permission checks

2012/1/17 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Tue, Jan 10, 2012 at 7:51 AM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>> The attached patch adds OAT_DROP object-access-hook around permission
>> checks of object deletion.
>> Due to the previous drop statement reworks, the number of places to
>> put this hook is limited to these six points: RemoveObjects,
>> RemoveRelations, ATExecDropColumn, dropdb, DropTableSpace and
>> shdepDropOwned().
>>
>> In sepgsql side, it checks {drop} permission for each object types,
>> and {remove_name} permission to the schema that owning the object
>> being removed. I'm still considering whether the drop permission
>> should be applied on objects being removed in cascade mode.
>> It is not difficult to implement. We can determine the bahavior on
>> object deletion with same manner of creation; that saves contextual
>> information using ProcessUtility hook.
>>
>> At this moment, the current proposed patch does not apply checks on
>> cascaded deletion, according to SQL behavior. However, my concern is
>> that user can automatically have right to remove all the objects
>> underlying a partidular schema being removable, even if individual
>> tables or functions are not able to be removed.
>>
>> So, my preference is sepgsql references dependency tables to check
>> {drop} permissions of involved objects, not only the target object.
>
> Hmm.  I kind of wonder if we shouldn't just have the OAT_DROP hook get
> invoked for every actual drop, rather than only for the top-level
> object.  It's somewhat appealing to have the hook more-or-less match
> up the permissions checks, as you have it here, but in general it
> seems more useful to have a callback for each thing dropped than to
> have a callback for each thing named to be dropped.  What is your
> opinion?
>
I think it is more ideal design.

If we could track object deletion only top-level, we have no option to
implement security features based on the object-access-hook, even
if security model is suitable to apply checks all the object deletion.
In addition, I believe it should be used to clean-up something set up
by external modules, not only permission checks.

Do I modify the patch to place object-access-hook on deleteOneObject
(probably, it is the best position to track actual deletion)?
One problem is case of deletion of columns by ALTER TABLE.
It just marks "attisdropped" flag; without removing catalog entry.
Do we ought to put this hook on ATExecDropColumn exceptionally?

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>