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

2011/10/18 Robert Haas <robertmhaas@gmail.com>:
>> In the example table creation, heap_create_with_catalog() is invoked
>> by 5 routines, however, 3 of them are just internal usages, so it is not
>> preferable to apply permission checks on table creation....
>
> Some wit once made the remark that if a function has 10 arguments, it
> has 11 arguments, meaning that functions with very large numbers of
> arguments typically indicate a poor choice of abstraction boundary.
> That's pretty much what I think is going on with
> heap_create_with_catalog().  I think maybe some refactoring is in
> order there, though I am not sure quite what.
>
Sorry, are you complained about the number of arguments in
heap_create_with_catalog? or hooks of security checks?

If we try to rework access control logic around DefineRelation and
OpenIntoRel to host both of DAC and MAC, it will takes a long
arguments list because an entry of pg_class catalog is not
constructed at the timing, so we need to inform the routine
all the parameters of new relation required to both DAC and MAC.
Thus, it takes a long argument list, however, number of arguments
of heap_create_with_catalog will not increased so much. (Maybe,
it additionally requires a private data to deliver security labels to be
assigned on the new relation.)

If we relocate logics of DAC into heap_create_with_catalog(), it will
need additional two arguments, though it has 19 arguments right now.
It is not a strange idea to inform routines that modified catalogs
whether this context needs permission checks, or not, because
CreateTrigger has a flag of "isInternal" to skip permission check.

>> BTW, it seems to me SELECT INTO should also check insert permission
>> on DAC level, because it become an incorrect assumption that owner of
>> table has insert permission on its creation time.  :
> You could make the argument that there's no real security hole there,
> because the user could give himself those same privileges right back.
> You could also make the argument that a SELECT .. INTO is not the same
> as an INSERT and therefore INSERT privileges shouldn't be checked.  I
> think there are valid arguments on both sides, but my main point is
> that we shouldn't have core do it one way and sepgsql do it the other
> way.  That's going to be impossible to maintain and doesn't really
> make any logical sense anyway.
>
My point is that we should minimize the code complexity, redundancy
or others between DAC and MAC implementation, however, it is not
possible to get their different to zero due to the differences of their
standpoint.

Yes, I never say SELECT INTO without DAC checks cause actual
security hole, because owner can change its access permissions by
himself, unlike MAC.
Please suppose that there are two security labels: confidential table
(TC) and public table (PT). Typical MAC policy (including selinux)
does not allow users who can read from tables with TC to write out
data into tables with PT, because PT is readable for public as literal,
so confidential data may be leaked to public domain in the result.

It is a significant characteristic of MAC; called as data-flow-control.
So, it damages significant part of its worth, if MAC could not distinct
CREATE TABLE from SELECT INTO in PostgreSQL, and it is the
reason why I strongly requires a flag of contextual information here.

Although you say it is not possible to maintain, doesn't the above
introduction help us to understand nothing why MAC needs to
distinct SELECT INTO from CREATE TABLE?, and why it needs
a flag to distinct two cases?

Thanks,
--
KaiGai Kohei <kaigai@kaigai.gr.jp>