[Pacemaker] Multi-level ACLs for the CIB

[Andrew Beekhof]

On Tue, Jan 12, 2010 at 1:06 PM, Yan Gao <ygao at novell.com> wrote:
>
> Andrew Beekhof wrote:
>> On Tue, Jan 12, 2010 at 10:41 AM, Dejan Muhamedagic <dejanmm at fastmail.fm> wrote:
>>> Hi,
>>>
>>> On Mon, Jan 11, 2010 at 09:01:30PM +0800, Yan Gao wrote:
>>
>> Ah, I was looking in the patch.
>>
>>>>     <acls>
>>>>       <role id="admin">
>>>>         <write id="admin-write-0" tag="configuration"/>
>>>>         <write id="admin-write-1" tag="status"/>
>>>>       </role>
>>>>       <role id="operator">
>>>>         <write id="operator-write-0" tag="nodes"/>
>>>>         <write id="operator-write-1" tag="status"/>
>>>>       </role>
>>>>       <role id="monitor">
>>>>         <read id="operator-read-0" tag="nodes"/>
>>>>         <read id="monitor-read-1" tag="status"/>
>>>>         <members>
>>>>           <uid id="ygao"/>
>>>>         </members>
>>>>       </role>
>>>>       <user id="ygao">
>>>>         <write id="ygao-write-0" ref="rsc0-meta_attributes-target-role"/>
>>>>         <deny id="gaoyan-deny-0" ref="rsc0-instance_attributes-password"/>
>>> I understand that the "ref" element references some "id", but
>>> note that the attributes's ids are sort of volatile.
>>
>> Agreed.
>> Not to mention that the admin wouldn't be able to remove the attribute
>> anymore (since its referenced in the acl).
>>
>>> In
>>> particular the meta attributes. Can we somehow reference the
>>> attributes themselves (in this case "password" and
>>> "target-role")? Would there be a big performance penalty?
>>
>> Probably simplest to just support xpath:
>>
>>    <deny id="gaoyan-deny-0" xpath="//[@id='rsc0']//[@name='password']"/>
> The only concern to introduce xpath was the complexity of the syntax.
> If we are OK with that, I can implement also :-)
>
>>
>>>>         <read id="ygao-read-0" ref="rsc0"/>
>>>>         <role_ref id="operator"/>
>>>>       </user>
>>>>     </acls>
>>>> ..
>>> Do you have a suggestion how to spell out acls in the crm shell
>>> syntax? We should also create a real-life acl configuration and
>>> then see how that can be represented.
>>>
>>>> The user "ygao" is a system account.
>>>> We could define several roles as we wish, such as "admin",
>>>> "operator" and "monitor", which could contain a member list
>>>> respectively if more than one user have the same permissions. A
>>>> role also could be referenced by a particular "<user ...>"
>>>> definition.
>>> I find this a bit confusing: roles have members and users can
>>> reference roles. Shouldn't one of the two suffice? The way it is
>>> now, it's also hard to follow.
>>
>> Agreed. I think I prefer the latter.
>>
>>>> [...]
>>>> The first command is achieved by requesting lrmd, which is running as root,
>>>> to do the cleanup for the client. So it could always bypass the ACL check.
>>> lrmd is not running as root, but as nobody. And it's not a
>>> client to the cib. But I guess that it is not relevant.
>>>
>>>> BTW, there're some changes comparing to the original design:
>>>> [...]
>>>> There could be an "attribute" for an ACL object in the original design :
>>>> <write id=... ref="rsc0-meta_attributes-target-role" attribute="value" />
>>>>
>>>> it was supposed to mean user could only write the "value" attribute of
>>>> "rsc0-meta_attributes-target-role" element.
>>>>
>>>> I didn't implement it because there's no good way for now for the ACL
>>>> checker to recognize if a modification would change/add/remove any
>>>> particular attributes of a XML element. And I'm thinking if it's
>>>> necessary to implement it... Your thoughts?
>>
>> Check the diff?
> It now does check the diff. Besides the "__crm_diff_marker__" set for add/remove,
> I also add a marker for "modified". But one modification could modify several of its
> attributes. So for recognizing those, we might need to add multiple markers for one element?

It doesn't make sense to me why you'd need to do that, it can be inferred.

If an attribute is only in diff-added - then it was created.
If an attribute is only in diff-removed - then it was deleted.
If an attribute is in both it was modified

te_callbacks.c has similar logic IIRC

If you see __crm_diff_marker__, then everything from there down was
affected (either created or deleted, its not set for modify).
And xpath will still work because the diff includes the path back to
the cib root.

>> If you do that after the operation but before the activation, you'd be
>> able to veto and invalid changes.
>> So you'd not even need to pre-filter the input cib or query.
> It does pre-filter only for a query operation.

Must have mis-read the logic. Sorry.

> If an user queries cib with a xpath,
> and the result is multiple objects listed under a "<xpath-query>", it could not
> accomplish the filter unless we pre-filter the input cib.

Yep.