Re: security label support, part.2

(2010/07/23 10:05), Robert Haas wrote:
> 2010/7/22 KaiGai Kohei<kaigai@ak.jp.nec.com>:
>> Thanks for your reviewing.
>>> 1. I am inclined to suggest the syntax SECURITY LABEL ON ... IS ...,
>>> following COMMENT ON (it's also somewhat similar to the GRANT syntax).
>>>    While the hook in ExecCheckRTPerms() will only allow meaningful
>>> permissions checks on the use of relations, there will presumably be
>>> ongoing demand to add additional hooks to cover other types of
>>> objects, and I'd rather add label support all at once rather than
>>> bit-by-bit.  I also think that the SECURITY LABEL syntax is more
>>> future-proof; we don't need to worry about conflicts in other parts of
>>> the grammar.
>>>
>> Hmm. Indeed, we cannot deny the possibility to conflict with other part
>> in the future, if we use ALTER xxx statement here.
>>
>> But, right now, we have no statement that starts in noun, rather than verb.
>> The "comment" is a noun, but "comment on" is a phrasal-verb, isn't it?
>>
>> How about RELABEL<object>  TO<label>, instead?
> 
> Well, I like SECURITY LABEL better because it's more clear about what
> kind of label we're talking about, but if there's consensus on some
> other option it's OK with me.  Actually, we need to work the security
> provider name in there too, I think, so perhaps SECURITY LABEL FOR
> provider ON object IS labeltext.  I realize it's slightly odd
> grammatically, but it's no worse than the COMMENT syntax.
> 
The "FOR <provider>" clause should be optional. I expect most use
cases installs only one security provider, rather than multiple.

If no explicit <provider> is specified, all the security providers
check the given security label. If two or more providers are here,
of course, either of them will raise an error, because they have
different label formats. It is right.

Anyway, I'd like to implement according to the idea.
 SECURITY LABEL [FOR <provider>] ON <object> IS <label>;

>>> 2. Similarly, the design of the hook in secabel.h is way too
>>> short-sighted and won't scale to any other object type.  We don't need
>>> or want one hook per object type here.  Use an ObjectAddress.
>>>
>> I think the relation type is an exceptional object class, because of
>> the recursion due to the table inheritances.
>> For other object classes, I also think one security hook which takes
>> ObjectAddress as an argument is enough to implement.
>>
>> So, I expect we need two hooks on relabeling eventually.
>> (One for relation, one for other object classes)
> 
> Please explain in more detail.
> 
For relations, one SECURITY LABEL statement may relabel multiple tables
when it has child tables, if ONLY clause was not given.
So, we need to obtain oids to be relabeled using find_all_inheritors(),
and need to ask providers whether it allows, or not.
But, obviously, it is specific for relations.

For other object class, the target object to be relabeled is identified
by <object> in SECURITY LABEL statement. It will be parsed by the upcoming
parse_object.c feature, then it solves the object name to ObjectAddress.
So, we can apply access controls after setting up the ObjectAddress with
common hooks for object classes except for relations, like:
 void check_object_relabel(ObjectAddress object, const char *new_label);

>>> 3. I am firmly of the view that we want to allow multiple security
>>> providers.  I think the way this should work here is that we should
>>> keep a list somewhere of security providers known to the system, which
>>> loadable modules should add themselves to.  Each such security
>>> provider should be represented by a struct containing, at least, a
>>> name and a function that gets called on relabel.  The labels should be
>>> stored in the catalog.  That way there is never any possibility of one
>>> security provider getting a label that was originally applied by some
>>> other security provider.  If we were storing these labels in pg_class
>>> or pg_attribute or similar, the storage cost for this might be worth
>>> worrying about, but as this is a separate catalog, it's not.
>>>
>> What I'm worrying about is that we cannot estimate amount of works when
>> we expand the concept to row-level security. We will need to revise the
>> implementation, if individual user tuples have its security label in the
>> future version.
>> If we don't support multiple labels right now, it will not be feature
>> degradation, even if it will be hard to implement multiple label support
>> for each user tuples. :(
> 
> I think it is 100% clear that row-level security will require
> completely separate infrastructure, and therefore I'm not even a tiny
> bit worried about this.  :-)
> 
Hmm. Are you saying we may degrade the feature when we switch to the
completely separate infrastructure? Is it preferable??

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