[Freeipa-devel] [PATCH 0179] Preserve user principal aliases during rename operation

[Martin Babinsky]

On 07/12/2016 02:00 PM, Martin Babinsky wrote:
> On 07/12/2016 01:05 PM, Alexander Bokovoy wrote:
>> On Mon, 11 Jul 2016, Martin Babinsky wrote:
>>> From 185bde00a76459430d95ff207bf1fb3fe31e811a Mon Sep 17 00:00:00 2001
>>> From: Martin Babinsky <mbabinsk at redhat.com>
>>> Date: Fri, 1 Jul 2016 18:09:04 +0200
>>> Subject: [PATCH] Preserve user principal aliases during rename operation
>>>
>>> When a MODRDN is performed on the user entry, the MODRDN plugin resets
>>> both
>>> krbPrincipalName and krbCanonicalName to the value constructed from
>>> uid. In
>>> doing so, hovewer, any principal aliases added to the krbPrincipalName
>>> are
>>> wiped clean. In this patch old aliases are fetched before the MODRDN
>>> operation
>>> takes place and inserted back after it is performed.
>>>
>>> This also preserves previous user logins which can be used further for
>>> authentication as aliases.
>>>
>>> https://fedorahosted.org/freeipa/ticket/6028
>>> ---
>>> ipaserver/plugins/baseuser.py | 46
>>> +++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 46 insertions(+)
>>>
>>> diff --git a/ipaserver/plugins/baseuser.py
>>> b/ipaserver/plugins/baseuser.py
>>> index
>>> 0052e718afe639bcc1c0a698ded39ea8407a0551..e4288a5a131157815ffb2452692a7edb342f6ac3
>>>
>>> 100644
>>> --- a/ipaserver/plugins/baseuser.py
>>> +++ b/ipaserver/plugins/baseuser.py
>>> @@ -498,6 +498,50 @@ class baseuser_mod(LDAPUpdate):
>>>                             len =
>>> int(config.get('ipamaxusernamelength')[0])
>>>                         )
>>>                     )
>>> +
>>> +    def preserve_krbprincipalname_pre(self, ldap, entry_attrs, *keys,
>>> **options):
>>> +        """
>>> +        preserve user principal aliases during rename operation. This
>>> is the
>>> +        pre-callback part of this. Another method called during
>>> post-callback
>>> +        shall insert the principals back
>>> +        """
>>> +        if options.get('rename', None) is None:
>>> +            return
>>> +
>>> +        try:
>>> +            old_entry = ldap.get_entry(
>>> +                entry_attrs.dn, attrs_list=(
>>> +                    'krbprincipalname', 'krbcanonicalname'))
>>> +
>>> +            if 'krbcanonicalname' not in old_entry:
>>> +                return
>>> +        except errors.NotFound:
>>> +            self.obj.handle_not_found(*keys)
>>> +
>>> +        self.context.krbprincipalname = old_entry.get(
>>> +            'krbprincipalname', [])
>>> +
>>> +    def preserve_krbprincipalname_post(self, ldap, entry_attrs,
>>> **options):
>>> +        """
>>> +        Insert the preserved aliases back to the user entry during
>>> rename
>>> +        operation
>>> +        """
>>> +        if options.get('rename', None) is None or not hasattr(
>>> +                self.context, 'krbprincipalname'):
>>> +            return
>>> +
>>> +        obj_pkey = self.obj.get_primary_key_from_dn(entry_attrs.dn)
>>> +        canonical_name = entry_attrs['krbcanonicalname'][0]
>>> +
>>> +        principals_to_add = tuple(p for p in
>>> self.context.krbprincipalname if
>>> +                                  p != canonical_name)
>>> +
>>> +        if principals_to_add:
>>> +            result = self.api.Command.user_add_principal(
>>> +                obj_pkey, principals_to_add)['result']
>>> +
>>> +            entry_attrs['krbprincipalname'] =
>>> result.get('krbprincipalname', [])
>>> +
>>>     def check_mail(self, entry_attrs):
>>>         if 'mail' in entry_attrs:
>>>             entry_attrs['mail'] =
>>> self.obj.normalize_and_validate_email(entry_attrs['mail'])
>>> @@ -557,9 +601,11 @@ class baseuser_mod(LDAPUpdate):
>>>
>>>         self.check_objectclass(ldap, dn, entry_attrs)
>>>         self.obj.convert_usercertificate_pre(entry_attrs)
>>> +        self.preserve_krbprincipalname_pre(ldap, entry_attrs, *keys,
>>> **options)
>>>
>>>     def post_common_callback(self, ldap, dn, entry_attrs, *keys,
>>> **options):
>>>         assert isinstance(dn, DN)
>>> +        self.preserve_krbprincipalname_post(ldap, entry_attrs,
>>> **options)
>>>         if options.get('random', False):
>>>             try:
>>>                 entry_attrs['randompassword'] =
>>> unicode(getattr(context, 'randompassword'))
>>> --
>>> 2.5.5
>>>
>> The approach looks good.
>>
>> For the record, we also support aliases for hosts and service kerberos
>> principals but we don't support rename options for them, so there is no
>> need to add similar logic there.
>>
>>
>
> That's right, I have updated the corresponding section of the design
> page [1] for future reference.
>
> [1]
> http://www.freeipa.org/page/V4/Kerberos_principal_aliases#Management_framework
>
>
Adding Simo to the loop since he is not convinced that this is the right 
behavior. As I see it, it seems to not be a security issue but more of a 
different expectations about the perceived correct behavior in this 
particular situation.

I can see the use case in keeping the old aliases, e.g. keeping the old 
credentials after legal name change, but I can also see the available 
space for user principal names piling up and cluttering quickly in large 
organizations.

-- 
Martin^3 Babinsky