Re: security hook on table creation

(2010/09/30 0:36), Robert Haas wrote:
> On Wed, Sep 29, 2010 at 9:59 AM, KaiGai Kohei<kaigai@kaigai.gr.jp>  wrote:
>> (2010/09/29 22:00), Robert Haas wrote:
>>>
>>> On Wed, Sep 29, 2010 at 6:38 AM, KaiGai Kohei<kaigai@kaigai.gr.jp>    wrote:
>>>>
>>>> I don't think it is an option to move the hook after the pollution
>>>> of system catalogs, although we can pull out any information about
>>>> the new relation from syscache.
>>>
>>> Why not?
>>>
>> All the existing security checks applies before modifying system catalogs.
>>
>> At least, I cannot find out any constructive reason why we try to apply
>> permission checks on object creation time with different manner towards
>> the existing privilege mechanism...
> 
> The reason would be so that you can apply security labels if you so
> desire.  If you choose to throw an error instead, transaction abort
> will clean everything up.  We could have two hooks - one earlier when
> we're checking DAC permisisons, and a second one later to apply
> labels, but I don't see that there's enough of a gain from that to be
> worth the additional complexity.  It's still simpler than your
> proposed design, though.
> 
Hmm. My scheme of consideration might be affected by kernel programming
which does not have any transaction rollback, so I though it needed all
the checks before doing anything.

>>> So what?  The patch you submitted doesn't provide the OID of the new
>>> schema when someone does ALTER TABLE SET SCHEMA, either.  I proposed a
>>> design which was much more general than what you submitted, and you're
>>> now complaining that it's not general enough.  It's unrealistic to
>>> think you're going to solve every problem with one patch.
>>
>> Sorry, I never said one patch with enough generic hook solves everything.
>>
>> By contraries, I think the proposed prototype of the hook cannot inform
>> the plugins anything except for OID and event type, even if necessary.
> 
> That is true.  But ISTM that it will handle a remarkably large number
> of cases very well.  We could of course do more later, either by
> adding additional hooks or by adding capabilities to this one.
> However, you'd first need to make a convincing argument that those
> capabilities are important.
> 
I now understand you are never suggesting a set of forever-fixed interfaces.
If we can fix up prototype of the security hooks later, I have less concern
for the approach.
It seems to me I misunderstood the intention of your proposition, sorry.

>> Some of permission checks needs its specific prototype to inform extra
>> information rather than OIDs; such as new label in SECURITY LABEL hook,
>> new schema in upcoming ALTER TABLE SET SCHEMA, and so on...
>>
>> Of course, we can implement some of permission checks with OID of the
>> target object and event type collectly. E,g. I cannot image any extra
>> information to check permission on COMMENT. I never deny it.
> 
> Why not?  If you're going to prohibit another plugin from relabeling
> an object based on the provider and label, why not allow or disallow
> comments based on the content of the comment?  A general problem with
> your designs from the very beginning is that they involve the enhanced
> security provider needing to know about absolutely everything that
> goes on in core, and visca versa.  That's unmaintainable and we're not
> doing it.
> 
Indeed, we can assume such a security module which also checks content
of the comment (aside from its effectivity).

> Incidentally, wanting to know the label that some other security
> provider might try to assign to an object is a crystal-clear example
> of moving the goal-posts.  You had a hook for that (which I removed)
> in the security label patch, and it didn't provide the label anyway.
> How can it be a requirement now if it wasn't two weeks ago?  You need
> to stay focused on coming up with simple, easy-to-understand hooks
> that ideally have use case cases that are broader than security, but
> at least that are broadly applicable to security rather than very
> narrowly tailored to extremely specific things which you want to do.
> 
The concern was also based on my misunderstanding.
I've agreed to the small-startup approach, so I believe this hook can
be eventually fixed up.

> I think that the remit of this patch should be to add hooks for CREATE
> and DROP to every single object type in the system that are generic
> and can be used for any purpose, whether security related or
> otherwise, with room for extension to additional operations in future
> patches.
> 
I agree.

>>> Moreover,
>>> it's far from obvious that you actually do need the details that
>>> you're proposing anyway.  Are you really going to write an SE-Linux
>>> policy that allows people to change the schema of table A to schema B
>>> but not schema C?  Or that allows a hypothetical smack plugin to label
>>> a given object with one label but not another?  And if so, are those
>>> absolutely must-have features for the first version or are those
>>> things that would be nice to have in version 3 or 4?
>>
>> In your proposition, prototype of the security hook has four arguments:
>> ObjectType, oid, subid and ObjectAccessType, doesn't it?
> 
> Yes.
> 
>> When user tries to change the schema of table from A to B, we can know
>> the current schema of the table using syscache, but we need to inform
>> the plugin that B is the new schema, because we have no way to pull out
>> what schema was provided by the user.
>>
>> As LookupCreationNamespace() checks CREATE permission on the new schema,
>> SELinux also want to check permission on the new schema, not only old one.
>> So, I concerned about the prototype does not inform about new schema that
>> user provided using ALTER TABLE SET SCHEMA statement.
> 
> You're not answering my question.  Are you going to write an SE-Linux
> policy that allows table A to be moved to schema B but not to schema
> C?  And if so, is that an essential feature for the first version or
> something that can be added later?
> 
Ah, Sorry.
Yes, I (eventually) want to provide this kind of the policy, but I think
its priority is not first, indeed.

> My understanding from the conversation at BWPUG is that this is not
> something that Josh Brindle and David Quigley are concerned about.
> Hooks on object creation are important for type transitions, so that
> you can automatically assign labels rather than forcing users to apply
> them by hand; the fact that we can also the entire CREATE operation to
> get bounced from the same hook is a bonus.
>
Yes, the hooks on object creation time are important, rather than others.

> But with that exception,
> they seemed to think that coarse-grained permissions would be fine for
> a basic implementation: perhaps even just install something in
> ProcessUtility_hook and bounce DDL across the board, so long as it's
> controlled by reference to the security policy rather than by DAC.  I
> think we can do better than that in a pretty short period of time if
> we avoid getting side-tracked, but the key is that we have to avoid
> getting side-tracked.
> 
In this approach, we eventually need to deploy the hooks on object creation
as we are currently working on. So, I don't think using ProcessUtility_hook
for coarse-grained permissions is a right direction.

Thanks,
-- 
KaiGai Kohei <kaigai@ak.jp.nec.com>