[Freeipa-devel] [PATCH] 0538 aci-update: Trim the admin write blacklist

[Martin Kosek]

On 04/25/2014 01:01 PM, Petr Viktorin wrote:
> On 04/24/2014 05:15 PM, Simo Sorce wrote:
>> On Thu, 2014-04-24 at 16:47 +0200, Martin Kosek wrote:
>>> On 04/24/2014 03:42 PM, Simo Sorce wrote:
>>>> On Thu, 2014-04-24 at 15:18 +0200, Martin Kosek wrote:
>>>>> On 04/24/2014 02:28 PM, Simo Sorce wrote:
>>>>>> On Thu, 2014-04-24 at 14:17 +0200, Martin Kosek wrote:
>>>>>>> On 04/24/2014 09:41 AM, Petr Viktorin wrote:
>>>>>>>> On 04/23/2014 08:56 PM, Simo Sorce wrote:
>>>>>>>>> On Wed, 2014-04-23 at 20:37 +0200, Petr Viktorin wrote:
>>>>>>>>>> Admin access to read-only attributes such as ipaUniqueId, memberOf,
>>>>>>>>>> krbPrincipalName is provided by the anonymous read ACI, which will go
>>>>>>>>>> away. This patch adds a blanket read ACI for these.
>>>>>>>>>> I also moved some related ACIs to 20-aci.update.
>>>>>>>>>>
>>>>>>>>>> Previously krbPwdHistory was also readable by admins. I don't think we
>>>>>>>>>> want to include that.
>>>>>>>>>> Simo, should admins be allowed to read krbExtraData?
>>>>>>>>>
>>>>>>>>> Probably not necessary but there is nothing secret in it either.
>>>>>>>>>
>>>>>>>>> Simo.
>>>>>>>>
>>>>>>>> OK. I'm not a fan of hiding things from the admin, so no changes to the
>>>>>>>> patch
>>>>>>>> are necessary here.
>>>>>>>>
>>>>>>>
>>>>>>> 536:
>>>>>>> As we are touching these ACIs, may it is a time to see the blacklist of
>>>>>>> attributes that admin cannot write and check if this is still wanted:
>>>>>>>
>>>>>>> ipaUniqueId - OK, generated by DS plugin
>>>>>>> memberOf - OK, generated by DS plugin
>>>>>>> serverHostName - I did not even find a place where we manipulate it, except
>>>>>>> host.py -> remove from blacklist?
>>>>>
>>>>> What about this one?
>>>>
>>>> not sure, I did not work much on the hosts objects.
>>>
>>> Rob? Do you know?
>>>
>>>>>>> enrolledBy - OK, generated by DS plugin
>>>>>>> krbExtraData - OK, generated by DS plugin
>>>>>>> krbPrincipalName - why can't admin change it? It is filled by framework, I
>>>>>>> would not personally blacklist it
>>>>>>
>>>>>> It is changed by the ipa rename plugin when the user uid change, that's
>>>>>> why we prevent the admin from explicitly change it.
>>>>>>
>>>>>>> krbCanonicalName - same as krbPrincipalName
>>>>>
>>>>> Ok, in that case krbPrincipalName and krbCanonicalName should stay, right?
>>>>
>>>> They should be accessible by admin, yes.
>>>
>>> Note that we are now discussing write access. I.e. what attributes needs to
>>> stay in the admin's global write ACI blacklist and which can be removed ->
>>> admin can write in them.
>>>
>>> IIUC, these should be only readable by admin, not writeable.
>>>
>>>>
>>>>>>> krbPrincipalAliases - same as krbPrincipalName - we need this removed if we
>>>>>>> want to set aliases anyway
>>>>>
>>>>> Allow this one?
>>>>
>>>> This is one I wanted to eventually get rid of, Alexander seem to have
>>>> introduced it, but we discussed in the past of a way to kill it.
>>>> I forgot the details thouhg.
>>>> Alexander did we open a ticket for this ?
>>>
>>> Ok, we may eventually get rid of it, but does it need to be blacklisted in
>>> admins global write ACI?
>>>
>>>>
>>>>>>> krbPasswordExpiration - OK, generated by DS plugin
>>>>>>> krbLastPwdChange - OK, generated by DS plugin
>>>>>>> krbUPEnabled - not used, can we remove it?
>>>>>
>>>>> What about this one?
>>>>
>>>> We never use it.
>>>
>>> I read this as "remove it from global admin write ACI blacklist".
>>>
>>>>
>>>>>>> krbTicketPolicyReference - why cannot admin set it?
>>>>>>
>>>>>> Unclear why, probably should be able to.
>>>>>
>>>>> We may want to remove it from blacklist then.
>>>>
>>>> We never used it, but we should probably allow admin to modify
>>>
>>> Ok, let us remove it from global admin write ACI blacklist.
>>>
>>>>
>>>>>>> krbPwdPolicyReference - why cannot admin set it?
>>>>>>
>>>>>> We assign password policy based on groups now, right ?
>>>>>
>>>>> Yup.
>>>
>>> I see no complains, i.e. I read it as "remove it from global admin write ACI
>>> blacklist".
>>>
>>>>>
>>>>>>
>>>>>>> krbPrincipalType - why cannot admin set it?
>>>>>>
>>>>>> Unused.
>>>>>
>>>>> So... allow this one?
>>>>
>>>> we never use it so we do not need to allow admins by default.
>>>
>>> My point was to limit admin write ACI blacklist to the bare minimum. Only to
>>> attributes that really needs to be blacklisted as they are operated by DS
>>> plugins - like memberOf and others.
>>>
>>> My proposal is to remove it from global admin write ACI blacklist given it is
>>> not used.
>>
>> Ack, I agree with your intent.
>>
>> Simo.
>>
> 
> If I understand correctly, we want to allow:
> - krbPrincipalAliases
> - krbPrincipalType
> - krbPwdPolicyReference
> - krbTicketPolicyReference
> - krbUPEnabled
> - serverHostName
> 
> Here is the patch for that.

The list looks good to me.

> Martin, just to make sure: 0536-0537 are good to push, right?

One of the reasons why I wanted to prune the blacklist a bit was to make the
Admin read ACI (Admin read-only attributes) simpler (read - shorter). So please
also update this aci in 536.

IMO, 536 and 538 can be squashed, but that's up to you.

Martin