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

[Rob Crittenden]

Petr Viktorin wrote:
> 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.

There needs to be some mechanism for us for force-replace existing ACIs 
in the case of a security issue.

>
>>> 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.
>
>