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

[Simo Sorce]

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.