[Freeipa-devel] [PATCHES] 0289-0302 Managed Read permissions

[Martin Kosek]

On 10/18/2013 04:28 PM, Petr Viktorin wrote:
> On 10/03/2013 12:42 PM, Martin Kosek wrote:
>> On 10/02/2013 01:26 PM, Petr Viktorin wrote:
>>> On 10/02/2013 01:07 PM, Simo Sorce wrote:
>> ...
>>>>> To sum it up,  I would rather not build our permission system on this group.
>>>>>
>>>>> I think we need top base our ACIs on LDAP bind targets ldap:///all and
>>>>> ldap:///anyone to avoid performance issues and issues with ipausers not being
>>>>> complete:
>>>>>
>>>>> https://access.redhat.com/site/documentation/en-US/Red_Hat_Directory_Server/8.2/html/Administration_Guide/Managing_Access_Control-Bind_Rules.html
>>>>>
>>>>>
>>>>>
>>>>> I rather think we want to base the permissions on binddn. In the beginning,
>>>>> there would be 3 types of permissions based on binddn:
>>>>>
>>>>> * groupdn based: standard permissions that can be assigned privileges
>>>>> * ldap:///all permissions for all authenticated users. Cannot be assigned to
>>>>> privileges (would not make sense)
>>>>> * ldap:///anyone permissions for all, including anonymous users. Cannot be
>>>>> assigned to privileges (would not make sense)
>>>>>
>>>>> Just few examples:
>>>>> Read users - ldap:///anyone
>>>>> Read groups - ldap:///anyone
>>>>> Read sudo - ldap:///all
>>>>> Read hbac - ldap:///all
>>>>> ...
>>>>>
>>>>> Basing the permissions on these would allow us to get rid of all the awful
>>>>> deny permissions.
>>>>>
>>>>> To make the permission Bind part more user friendly, there should be an
>>>>> option
>>>>> in permission-find and a switch in Web UI to e.g. list permissions by bind
>>>>> type, i.e.:
>>>>> - anonymous permissions
>>>>> - authenticated users permissions
>>>>> - group based permissions
>>>>>
>>>>> If use would want to e.g restrict sudo only to specified group, I would see
>>>>> this workflow:
>>>>> 1) Change bind type from "authenticated users" to "group based"
>>>>> 2) Bind permission to chosen privilege
>>>>> 3) ...
>>>>>
>>>>> And the opposite, if he wants to make reading more open:
>>>>> 1) Unbind permission from privilege
>>>>> 2) Change bind type to "authenticated users" or "anonymous"
>>>>>
>>>>> Makes sense?
>>>
>>> Yes.
>>>
>>>> I agree with Martin's comments too.
>>>>
>>>> Simo.
>>>
>>> We use privileges to group related permissions. For example an "Automount
>>> Reader" privilege would contain "read automount keys" and "read automount maps"
>>> permissions.
>>> Wouldn't it make more sense (from the user's point of view at least) to have
>>> this setting at the privilege level?
>>>
>>> The selfservice plugin does pretty much the same thing. Should we (aim to) move
>>> selfservice functionality to this system?
>>
>> selfservice is not involved in privileges, neither is delegation, they are just
>> handling raw ACIs in a custom manner.
>>
>> With ldap:///all and ldap:///anyone we could theoretically add new permission
>> types and commands, like anonymousrbac-add or authenticatedrbac-add, but I
>> think it would be best to keep them with standard permissions for several
>> reasons:
>> 1) Can use the new cool features you are adding to permission entries
>> 2) With these ACIs, it makes sense to convert them to group-based permission
>> and assign to a privilege (does not make that much sense with selfservice ACIs).
>>
>> AFAIU, we all agree on that and the only part is how the
>> anonymous/authenticated-user permissions should be grouped.
>>
>> I still think that grouping them in privileges or roles does not make much
>> sense - anonymous/authenticated-user ACIs do not need to be grouped anywhere,
>> setting binddn is enough.
>>
>> It would be just our custom wiring in framework without much benefit to real
>> functionality in LDAP. If you have a privilege "Automount Reader" with setting
>> "Access: Anonymous: or similar, what if you add more non-anonymous permission
>> to the privilege? What would happen then? Or what if you remove permission from
>> the privilege, should it automagically become group-based ACI?
>>
>> Going with the workflow I described above seems to me as the most direct
>> approach with keeping the anonymous/authenticated users/group based information
>> in the single authoritative source - ACI/permission.
>>
>>>
>>> And, since these permissions are longer be compatible with old versions, I
>>> could move them out of $SUFFIX and onto the relevant containers. That should
>>> also help performance.
>>
>> +1
>>
>> Martin
>>
> 
> Alright, I'm crafting an updated design page with the above in mind. Here are
> the main differences.
> 
> 
> New permissions won't (necessarily) be in $SUFFIX, so old IPA servers will not
> be able to modify them.
> Extra attribute types needed in addition to ipaPerm*Attributes would be:
>   - ipaPermBindType (anonymous/any authenticated user/normal permission)
>   - ipaPermDN (container DN where the ACI is stored)
> 
> And objectclasses to group them:
> 
> 'ipaPermissionV2' SUP ipaPermission AUXILIARY MUST ( ipaPermBindType $ ipaPermDN )
> 'ipaManagedPermission' SUP ipaPermissionV2 AUXILIARY MAY ( ipaPermDefaultAttrs
> $ ipaPermAllowedAttrs $ ipaPermExcludedAttrs )
> 
> As for 'ipaPermissionV2', all non-SYSTEM permissions should be updated to it.
> Maybe a better name is needed.
> 
> 
> Another idea I had is to store all variable parts of the ACI in the permission
> entry. This would mean we'd not need to parse ACIs to read, search, or update
> them, which should make these operations faster and the code could be simplified.
> Doing this would require these new attribute types:
>   - ipaPermRight (add, update, read, delete, etc.)
>   - ipaPermObjectType
>   - ipaPermMemberof
>   - ipaPermFilter
>   - ipaPermSubtree
>   - ipaPermTargetgroup
> 
> Would that make sense?
> 

It partially makes sense - it would speed up permission-find commands. However,
it would also duplicate information and sets it in 2 places. Which smells like
a bucket of potential bugs to me.

What if somebody changes ipaPermObjectType, but ACI update fails or is
interrupted for some reason? It would create inconsistency between permission
entry and the ACI itself. Which should prevail?

Unless permission-find performance is not a problem (yet?), I would not add
these new attributes and only add ipaPermDN as this information is required.

I am thinking if we need to add a special objectClass for this new attribute,
it could be added as MAY to ipaPermission, along with ipaPermDefaultAttrs,
ipaPermAllowedAttrs, ipaPermExcludedAttrs. When the DN is not there, it would
simply default to SUFFIX DN.

Martin