Prep object creation hooks, and related sepgsql updates - Mailing list pgsql-hackers

These attached patches were getting rebased to the latest master and cosmetic
changes towrad upcoming commit-fest Nov.
Please apply these patches in order of Part-1, Part-2, Part-3 then Part-4.

We still don't have clear direction of the way to implement external permission
checks on object creation time. So, please consider these patches are on the
proof-of-concept stage; using prep-creation-hook to permission checks.

The reasons why existing post-creation-hook is not perfect to
implement permission
check on object creation time are:

(A) post-creation-hook is also invoked when we don't want to apply
permission checks.
For example, make_new_heap() invokes heap_create_with_catalog() to create
a temporary table for internal stuff as a part of CLUSTER, VACUUME FULL or some
of ALTER TABLE statements. However, it is not an option to deliver a
flag to show
whether this context needs permission checks, because it depends on
the knowledge
of individual security modules.

(B) system catalog does not contain all the necessary information for
permission checks.
Only pg_database hits this case. Existing DAC checks createdb
privilege and ownership
of the source database (if not templace), however, pg_database does
not contain oid of
the source database, so it is unavailable to apply equivalent
permission checks on the
external security modules.

Thus, the current solution is put new hooks (prep-creation) around
existing permission
checks with arguments being used to existing DAC checks commonly.
Since the argument has a pointer of the private opaque to be delivered
to post-creation
hooks. In sepgsql case, it is used to inform post-creation hook
security label to be
assigned on the new object being constructed.

Even though this approach enables to solve the issue, I'm not sure
whether we should
have prep-creation hooks for each object types in an automatic manner.
We currently have 30 of post-creation hooks for each object types,
however, most of
them does not hit the case of above either (A) or (B).
I guess pg_database, pg_class and pg_trigger (it takes "isInternal"
flag to avoid
nonsense permission checks) are the only case that hits post-creation hook is
not enough to implement correct permission checks.
So, I'm not sure to add prep-creation hook on rest of (90% of ) object
types, although
here is few strong reason to split creation-hook into two parts.

Thanks,

2011/11/12 Kohei KaiGai <kaigai@kaigai.gr.jp>:
> 2011/11/8 Robert Haas <robertmhaas@gmail.com>:
>> On Mon, Nov 7, 2011 at 12:20 PM, Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
>>> If sepgsql would apply permission checks db_procedure:{install} on the
>>> OAT_POST_CREATE hook based on the funcion-oid within new entry of
>>> system catalog, we can relocate OAT_PREP_CREATE hook more conceptually
>>> right place, such as just after the pg_namespace_aclcheck() of DefineType().
>>> On the other hand, we may need to answer why these information are NOT
>>> delivered on the OAT_PREP_CREATE hook without knowledge of sepgsql
>>> internal.
>>
>> I'm having a hard time understanding this.
>>
>> Can you strip this patch down so it just applies to a single object
>> type (tables, maybe, or functions, or whatever you like) and then
>> submit the corresponding sepgsql changes with it?  Just as a demo
>> patch, so I can understand where you're trying to go with this.
>>
> I tried to strip the previous patch into small portion per object types.
> Part-1 for database, Part-2 for schema, Part-3 for relations, and
> Part-4 for functions.
>
> The basic idea is the prep-creation hook informs the sepgsql module
> properties of the new object being constructed. According to the
> previous discussion, all the arguments are commonly used to existing
> DAC checks also. Then, this hook allows to write back its private
> opaque to be delivered to the post-creation hook; likely used to deliver
> security label of the new object to be assigned.
>
> However, I become unsure whether it is a good idea to put prep-creation
> hook on all the object, because it takes many boring interface changes
> to deliver private datum, and we're probably able to implement similar
> stuff with post-creation hook except for a few object types.
>
> I guess the following (A) and (B) are the only case that needs prep-
> creation hooks for permission checks. Elsewhere, sepgsql will be
> able to make its decision based on the entry of system catalogs
> on post-creation hook.
>
> (A) In the case when we want to apply checks based on information
> that is not contained within system catalogs.
> E.g, Oid of source database on CREATE DATABASE. Existing DAC
> checks ownership of the database, if not template.
>
> (B) In the case when we want to distinguish code path between user's
> query and system internal stuff.
> E.g, heap_create_with_catalog() is also called by make_new_heap()
> as a part of ALTER TABLE, but it is quite internal stuff, so not suitable
> to apply permission checks here.
>
> It seems to me, using post-creation hooks makes the patch mode simple
> to implement permission checks; except for above two object types.
> So, I'd like to adopt approach to put prep-creation hooks on limited number
> of object types, not symmetric with post-creation hook.
> How about your opinion about this?
>
> Thanks,
> --
> KaiGai Kohei <kaigai@kaigai.gr.jp>
>
--
KaiGai Kohei <kaigai@kaigai.gr.jp>