[Freeipa-devel] Notes and questions for fine-grained read permissions

[Petr Viktorin]

On 09/09/2013 03:46 PM, Simo Sorce wrote:
> On Mon, 2013-09-09 at 13:00 +0200, Petr Viktorin wrote:
>> On 09/07/2013 04:45 PM, Simo Sorce wrote:
>>> Sorry to come late to this thread.
>>>
>>> I think I like some of Petr plan, but not all of it.
>>>
[...]
>>>>>> It could get ugly real fast, and potentially cause a lot of extra processing. I
>>>>>> think the object(s) for each attribute should be considered so these wouldn't
>>>>>> have to be applied to every ACI but only those that are affected. We don't need
>>>>>> to worry about userPassword in groups, for example.
>>>>>
>>>>> I think that a decision that a param should not be included in default read
>>>>> rule should be included in the param object itself, see below.
>>>
>>> I really do not want to see the ACI templates/definition/call it how you
>>> want int the framework, because the framework will need to change with
>>> versions of IPA but the data may remain in LDAP for long. therefore any
>>> ACI template should be stored in LDAP, probably under cn=etc,$suffix.
>>>
>>> It may make sense to have cn=aci-templates,cn=etc,$suffix, and then
>>> within that container (accessible only by security admins) we have one
>>> template per object/container, that is used to generate ACIs.
>>>
>>> Something like:
>>>
>>> dn: cn=users,cn=aci-templates,cn=etc,$suffix
>>> objectclass: ipaAciTemplate
>>> anonReadAttributes: <multivalue list>
>>> authReadAttributes: <multivalue list>
>>> selfReadAttributes: <multivalue list>
>>> selfWriteAttributes: <multivalue list>
>>>
>>> and so on.
>>> These are the defaults, only attributes that must be disclosed are
>>> listed, *no* negative lists, the default is changed globaly to deny all,
>>> and if there is no allow ACI an attribute is inaccessible by default.
>>>
>>> This allows easy customization, if someone decides 'description' should
>>> be kept confidential, all he needs to do is to remove it from the right
>>> list (and perhaps add it to a privileged list), and just run a tool that
>>> regenerates and changes the default ACIs for the container.
>>> * No need to modify framework code anywhere. *
>>
>> +1 to storing the attribute lists in LDAP. I don't like the Template
>> idea though.
>
> The template is just an example, we can negotiate on it :)
>
>> First, on IPA upgrades, we need to be able to add attributes to the
>> ACIs. Automatically.
>> We cannot make the IPA-generated lists user-modifiable. If some past
>> upgrade added a readable 'description' attribute, but the admin decides
>> that it should not be readable, the next upgrade should not re-add it.
>> So we need to store the IPA defaults and user changes separately.
>
> We solved this problem in the past for LDAP updates, I agree it is a
> problem we need to address, and I agree the way we have done in the past
> with .update files may not be up to task for the ACI system, so let's
> talk.

How have we solved it? AFAIK, currently if the LDAP updater sees a state 
that used to be default in an old IPA version but changed, it will 
overwrite it with the new state.

So if the current IPA version adds an attribute that you don't want, 
you're out of luck: removing it reverts to the old state, which will get 
updated next time you upgrade.

Also, if you *add* an attribute, the resulting ACI no longer matches the 
default so the LDAP updater won't ever touch it again.

>> Second, we already have a place where users can customize ACIs: the
>> permissions. I don't see the point of another layer.
>
> Well, you want a 'blueprint' for the permissions, where you set the
> system defaults, I called it template.
>
>> The only thing we're adding here over regular permissions is the
>> aforementioned ability to auto-add new attributes on upgrades.
>> We just need a special "managed" permission for each object's "anon
>> read", "auth read", "self read", "self write", etc.
>>
>>
>> I propose a new Permission type with three lists:
>
>> attributes added by IPA,
> ^^^ this is equivalent to what I called 'template', only you make it
> read-only for admins
>
>>   attributes added by the admin, and attributes removed by the admin.
> ^^^ and this sounds like a read-write, second order version of a
> template.
>
> I am not entirely sure I like the complexity of layered templates, but I
> do understand the proposal, and will think a little bit more about it, I
> am not opposed in principle to your scheme.
>
>> On IPA updates, new items can be added to the first list, and the ACI
>> gets regenerated from all three. This ensures that an admin's changes
>> get preserved across updates.
>
> How do you handle a case where we add 'read-only by admin' for an
> attribute that was not in the default ACI list at all previously, but
> the admin already added 'read by everyone' in the custom ACI ?
> This is the kind of thing that makes me dislike the 2 separate sets, now
> you need intra-set conflict resolution.

There would be two permissions: "read by admin" and "read by everyone".
Both would now allow reading the attribute, so it would be readable by 
anyone -- as the admin wanted.

I don't see any need for intra-set conflict resolution. The algorithm is 
simple -- take IPA defaults, add admin-added attributes, filter out 
admin-excluded attributes, allow the resulting set. Recompute whenever 
any of the lists changes.

>> If we mark a Permission as SYSTEM, old IPA versions won't try to locate
>> or touch its ACI, but they'll still be able to add it to privileges and
>> roles using the existing API/CLI/UI.
>
> I do not understand the nuances of this SYSTEM permission, can you
> explain why we want it, and why it should't "locate or touch its ACI"
> whatever it really means ?

Permissions are IPA objects that encapsulate the underlying ACIs and 
make them easier to manage. Permissions are less flexible than raw ACIs.
Sometimes they need more power than what the Permission UI exposes, so 
we mark them as SYSTEM. The CLI/UI then doesn't read or update the 
underlying ACI.
If we add permissions that work differently than the existing ones, we 
can mark them SYSTEM so *old IPA* doesn't try to manipulate the 
underlying ACIs.
(And we need to add some kind of permision versioning, in case the 
functionality changes again.)

>> So we'd have something like:
>>
>> cn=Read Users,cn=permissions,cn=pbac,$SUFFIX
>> objectclass: ipaPermission
>> objectclass: ipaManagedPermission
>> ipaDefaultAttributes: <multivalue list>
>> ipaAdminAddedAttributes: <multivalue list>
>> ipaAdminExcludedAttributes: <multivalue list>
>> ipaPermissionType: SYSTEM
>>
>> Admins that don't want IPA updates messing with their ACIs can just
>> remove the permission from privileges and add custom plain permissions
>> instead.
>> (Or possibly even convert the default permission to non-managed, or
>> delete it -- if we write UI to do+undo these actions.)
>
> Sounds a bit fragile, I think you never want to remove system defaults,
> because you will always want to be able to go back to a known state if
> you mess up. Maybe we can mark something as DISABLED and preserve
> things.

Removing the permission from the privilege is like disabling, you can 
always add it back.
We just need to make sure IPA doesn't overwrite the admin's choice on 
updates.


-- 
Petr³