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

[Simo Sorce]

On Wed, 24 Sep 2014 10:22:29 +0200
Martin Kosek <mkosek at redhat.com> wrote:

> On 09/22/2014 05:02 PM, Alexander Bokovoy wrote:
> > On Mon, 22 Sep 2014, Simo Sorce wrote:
> >> 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.
> > I think it is sensible approach to limit aliases for service/host
> > principals only.
> 
> Currently, David was only adding aliases for hosts - should we then
> expand the logic also for services?

I expect aliases will be used almost exclusively with services not
hosts.

> >> 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.
> > Yes, host/service shouldn't be renamed. Additionally, this would
> > make also their certificates "obsolete", in certain sense.
> 
> Ok, this requirement will make it simpler. Here is the logic as I
> would see it:
> 
> 1) Whenever an alias is added to existing service with the single
> krbPrincipalName, also krbCanonicalName is added targetting the
> original value, object is renamed to the CanonicalName.

1.1) change the framework to start creating new services using
krbCanonicalName as RDN.

> 2) Framework would prohibit changing the canonical name as that would
> always be the first name used when running host-add. (if someone does
> that via ldapmodify, that's their problem).

We should just prevent modrdn in cn=services and cn=accounts by
removing the ability to do so in existing permissions I think.

> >> 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 ?
> > Unfortunately, this is what happens right now if you add more than
> > one krbprincipalname already. Things seem to work fine except you
> > cannot address such objects in Web UI and CLI.
> > 
> >> 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.
> > Sounds reasonable to me.
> 
> BTW I now also hit
> 
> https://fedorahosted.org/freeipa/ticket/3864
> 
> Simo, should we then do these changes in 4.1 given the schema may not
> be OK yet?

Yes, I think we should resolve 3864 at the same time.

Simo.


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