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

[Martin Kosek]

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?

>> 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.

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).

>> 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?

Martin