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

[Petr Viktorin]

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.
Martin, just to make sure: 0536-0537 are good to push, right?

-- 
Petr³
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0538-aci-update-Trim-the-admin-write-blacklist.patch
Type: text/x-patch
Size: 5153 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140425/86321029/attachment.bin>