[Freeipa-devel] [PATCH] 0015-16 Allow multiple krbprincipalnames + test

[Simo Sorce]

On Mon, 22 Sep 2014 15:36:01 +0200
David Kupka <dkupka at redhat.com> wrote:

> On 09/18/2014 09:42 PM, Martin Kosek wrote:
> > On 09/18/2014 09:11 PM, Simo Sorce wrote:
> >> On Thu, 18 Sep 2014 14:57:45 -0400
> >> Rob Crittenden <rcritten at redhat.com> wrote:
> >>
> >>> Martin Kosek wrote:
> >>>> On 09/18/2014 04:06 PM, David Kupka wrote:
> >>>>> On 09/18/2014 03:44 PM, Rob Crittenden wrote:
> >>>>>> David Kupka wrote:
> >>>>>>> https://fedorahosted.org/freeipa/ticket/4421
> >>>>>>
> >>>>>> You are removing an ACI in this patch. It is always possible it
> >>>>>> is no longer needed. Did you test all the client enrollment
> >>>>>> scenarios?
> >>>>>>
> >>>>>> rob
> >>>>>>
> >>>>>
> >>>>> As far as I'm aware I'm not removing any ACI. I'm modifying ACI
> >>>>> so it is possible to add krbPrincipalName to host even when
> >>>>> there is already one (or more). And adding one ACI to allow
> >>>>> writing krbCanonicalName to host. But I'm still not really
> >>>>> familiar with ACI so please correct me if I'm wrong.
> >>>>>
> >>>>
> >>>> What refers to is probably the update in ACI.txt - the ACI
> >>>> alternative to API.txt. David updated an ACI, not removed it.
> >>>>
> >>>> On that note, what is the reason for this permission change:
> >>>>
> >>>> -            'ipapermtargetfilter': [
> >>>> -                '(objectclass=ipahost)',
> >>>> -                '(!(krbprincipalname=*))',
> >>>> -            ],
> >>>>
> >>>> ?
> >>>
> >>> Yes, this is exactly the change I was referring to. Permission
> >>> changes within a plugin now translate automatically to ACI
> >>> changes. Sorry I wasn't clearer.
> >>>
> >>> This ACI gets replaced with a much simpler one and I'm not 100%
> >>> sure it will work with all enrollments:
> >>>
> >>> -aci: (targetattr = "krbprincipalname")(targetfilter =
> >>> "(&(!(krbprincipalname=*))(objectclass=ipahost))")(version 3.0;acl
> >>> "permission:System: Add krbPrincipalName to a Host";allow (write)
> >>> groupdn = "ldap:///cn=System: Add krbPrincipalName to a
> >>> Host,cn=permissions,cn=pbac,dc=ipa,dc=example";)
> >>>
> >>> +aci: (targetattr = "krbprincipalname")(targetfilter =
> >>> "(objectclass=ipahost)")(version 3.0;acl "permission:System: Add
> >>> krbPrincipalName to a Host";allow (write) groupdn =
> >>> "ldap:///cn=System: Add krbPrincipalName to a
> >>> Host,cn=permissions,cn=pbac,dc=ipa,dc=example";)
> >>>
> >>> The first one restricts writing the attribute only if it isn't
> >>> already set. The second lets it be changed unconditionally.
> >>
> >> Yeah this is wrong indeed, the point of the ACI is to allow
> >> setting the principal only when it is not already set, which is
> >> the OTP enrollment case. But if krbprincipal is set then this
> >> specific permission should not grant rights to change it.
> >>
> >> At least this was my understanding.
> >>
> >> Simo.
> >
> > Right. It seems to me we should add keep this permission intact and
> > add a new permission allowing adding krbPrincipalName aliases. This
> > would allow writing both krbPrincipalName and krbCanonicalName.
> >
> > Martin
> >
> 
> Thank you all for explanation and help. I rewrote the patch so it
> should work as requested now. Also I added tests to reassure the
> behavior is correct.


Sorry for not catching this earlier, but should we also add new IPA
MODRDN configuration ?
We currently change krbPrincipalName if the user uid changes.

However perhaps what we should do is instead to allows aliases only for
service/host principals but not for ipa users, at least for now.

Should we also change permissions so that host/service entries
*cannot* be renamed ? Otherwise we'd need to add again IAP MODRDN
configuration so that if a service/host krbprincipalname is changed
then krbcanonicalname is too.

Last but not least, and here I regret we may have a BIG issue, I just
realize we use krbprincipalname in the Services DN, if we add
additional values there does it mean we end up with a multivalued RDN ?
That may be a problem, in such case should we rename services to use
krbcanonicalname as the rnd instead ? We can do this in a compatible
manner I think by renaming on the fly old entries if the still use
krbprincipalname and changing our code to start always setting
krbcanonicalname on new entries and set both canmonical and principal
name on every new entry.

Opinions ?



-- 
Simo Sorce * Red Hat, Inc * New York