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

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>