Re: [v9.2] Object access hooks with arguments support (v1)

BTW, I remember that I was suggested the object-access-hooks to acquire
controls around changes of system catalogs are also useful to implement
clustering features, not only enhanced security features, when I had a talk
at PGcon2001.

It might be my mistake that I categorized this patch at the "security" topic.
If someone volunteers to review this patch from the different viewpoint, not
only security features, it is quite helpful for us.

Thanks,

2011/9/29 Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>:
> I noticed that the previous revision does not provide any way to inform
> the modules name of foreign server, even if foreign table was created,
> on the OAT_POST_CREATE hook.
> So, I modified the invocation at heap_create_with_catalog to deliver
> this information to the modules.
>
> Rest of parts were unchanged, except for rebasing to the latest git
> master.
>
> 2011/8/28 Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>:
>> The attached patch is a draft to support arguments in addition to
>> OAT_* enum and object identifiers.
>>
>> The existing object_access_hook enables loadable modules to acquire
>> control when objects are referenced. The first guest of this hook is
>> contrib/sepgsql for assignment of default security label on newly
>> created objects. Right now, OAT_POST_CREATE is the all supported
>> object access type. However, we plan to enhance this mechanism onto
>> other widespread purpose; such as comprehensive DDL permissions
>> supported by loadable modules.
>>
>> This patch is a groundwork to utilize this hook for object creation
>> permission checks, not only default labeling.
>> At the v9.1 development cycle, I proposed an idea that defines both
>> OAT_CREATE hook prior to system catalog modification and
>> OAT_POST_CREATE hook as currently we have. This design enables to
>> check permission next to the existing pg_xxx_aclcheck() or
>> pg_xxx_ownercheck(), and raise an error before system catalog updates.
>> However, it was painful to deliver private datum set on OAT_CREATE to
>> the OAT_POST_CREATE due to the code complexity.
>>
>> The other idea tried to do all the necessary things in OAT_POST_CREATE
>> hook, and it had been merged, because loadable modules can pull
>> properties of the new object from system catalogs by the supplied
>> object identifiers. Thus, contrib/sepgsql assigns a default security
>> label on new object using OAT_POST_CREATE hook.
>> However, I have two concern on the existing hook to implement
>> permission check for object creation. The first one is the entry of
>> system catalog is not visible using SnaphotNow, so we had to open and
>> scan system catalog again, instead of syscache mechanism. The second
>> one is more significant. A part of information to make access control
>> decision is not available within contents of the system catalog
>> entries. For example, we hope to skip permission check when
>> heap_create_with_catalog() was launched by make_new_heap() because the
>> new relation is just used to swap later.
>>
>> Thus, I'd like to propose to support argument of object_access_hook to
>> inform the loadable modules additional contextual information on its
>> invocations; to solve these concerns.
>>
>> Regarding to the first concern, fortunately, most of existing
>> OAT_POST_CREATE hook is deployed just after insert or update of system
>> catalogs, but before CCI. So, it is helpful for the loadable modules
>> to deliver Form_pg_xxxx data to reference properties of the new
>> object, instead of open and scan the catalog again.
>> In the draft patch, I enhanced OAT_POST_CREATE hook commonly to take
>> an argument that points to the Form_pg_xxxx data of the new object.
>>
>> Regarding to the second concern, I added a few contextual information
>> as second or later arguments in a part of object classes. Right now, I
>> hope the following contextual information shall be provided to
>> OAT_POST_CREATE hook to implement permission checks of object
>> creation.
>>
>> * pg_class - TupleDesc structure of the new relation
>> I want to reference of pg_attribute, not only pg_class.
>>
>> * pg_class - A flag to show whether the relation is defined for
>> rebuilding, or not.
>> I want not to apply permission check in the case when it is invoked
>> from make_new_heap(), because it just create a dummy table as a part
>> of internal process. All the necessary permission checks should be
>> done at ALTER TABLE or CLUSTER.
>>
> And, name of the foreign server being used by the foreign table
> being created just a moment before.
>
>> * pg_class - A flag to show whether the relation is created with
>> SELECT INTO, or not.
>> I want to check "insert" permission of the new table, created by
>> SELECT INTO, because DML hook is not available to check this case.
>>
>> * pg_type - A flag to show whether the type is defined as implicit
>> array, or not.
>> I want not to apply permission check on creation of implicit array type.
>>
>> * pg_database - Oid of the source (template) database.
>> I want to fetch security label of the source database to compute a
>> default label of the new database.
>>
>> * pg_trigger - A flag to show whether the trigger is used to FK
>> constraint, or not.
>> I want not to apply permission check on creation of FK constraint. It
>> should be done in ALTER TABLE level.
>>
>> Sorry for this long explanation. Right now, I tend to consider it is
>> the best way to implement permission checks on object creation with
>> least invasive way.
>>
>> Thanks, Any comments welcome.
>> --
>> KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>
>>
> --
> KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>
>

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