[Freeipa-devel] [PATCHES] 0536-0537 Add ACI for read-only admin attributes

Petr Viktorin pviktori at redhat.com
Thu Apr 24 13:28:28 UTC 2014


On 04/24/2014 03:18 PM, 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?
>
>>> 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?
>
>>> krbPrincipalAliases - same as krbPrincipalName - we need this removed if we
>>> want to set aliases anyway
>
> Allow this one?
>
>>> krbPasswordExpiration - OK, generated by DS plugin
>>> krbLastPwdChange - OK, generated by DS plugin
>>> krbUPEnabled - not used, can we remove it?
>
> What about this one?
>
>>> krbTicketPolicyReference - why cannot admin set it?
>>
>> Unclear why, probably should be able to.
>
> We may want to remove it from blacklist then.
>
>>
>>> krbPwdPolicyReference - why cannot admin set it?
>>
>> We assign password policy based on groups now, right ?
>
> Yup.
>
>>
>>> krbPrincipalType - why cannot admin set it?
>>
>> Unused.
>
> So... allow this one?
>
>>
>>> krbLastSuccessfulAuth - OK, generated by DS plugin
>>> krbLastFailedAuth - OK, generated by DS plugin
>>> krbLoginFailedCount - OK, generated by DS plugin
>>>
>>> It seems to me that some attributes can be indeed removed from the backlist
>>> (and thus from the admin whitelist too).
>>>
>>> Besides that, the patch looked OK to me.
>>>
>>> 537: ACK (tests pass)
>
>
> Petr, regarding the "Admin can manage any entry" update - you removed 4 related
>   "remove:aci" definitions, but added just 3. Is that intentional?

The fourth was exactly the same as the one being added.


-- 
Petr³




More information about the Freeipa-devel mailing list