[SSSD] [PATCHES] Handle multi-value names for users and groups

Stephen Gallagher sgallagh at redhat.com
Wed Mar 23 08:59:47 UTC 2011


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/22/2011 04:28 PM, Sumit Bose wrote:
> On Tue, Mar 22, 2011 at 02:09:27PM -0400, Stephen Gallagher wrote:
> On 03/21/2011 04:24 PM, Sumit Bose wrote:
>>>> On Thu, Mar 17, 2011 at 05:25:55PM -0400, Stephen Gallagher wrote:
>>>> On 03/16/2011 02:07 PM, Simo Sorce wrote:
>>>>>>> On Wed, 16 Mar 2011 13:28:20 -0400
>>>>>>> Stephen Gallagher <sgallagh at redhat.com> wrote:
>>>>>>>
>>>>>>>> -----BEGIN PGP SIGNED MESSAGE-----
>>>>>>>> Hash: SHA1
>>>>>>>>
>>>>>>>> Fixes https://fedorahosted.org/sssd/ticket/818
>>>>>>>>
>>>>>>>> Patch 0001: Create sysdb_get_rdn() function
>>>>>>>> This function takes a DN formatted string and returns the RDN
>>>>>>>> value from it.
>>>>>>>
>>>>>>> Nack, please return both the value and the attribute name
>>>>>>>
>>>>>>>> Patch 0002: Add sysdb_attrs_primary_name()
>>>>>>>> This function will check a sysdb_attrs struct for the primary name
>>>>>>>> of the entity it represents. If there are multiple entries, it
>>>>>>>> will pick the one that matches the RDN. If none match, it will
>>>>>>>> throw an error.
>>>>>>>
>>>>>>> Nack please the attribute used in the RDN matches before checking the
>>>>>>> values.
>>>>>>>
>>>>>>>> Patch 0003: Handle multi-valued usernames correctly
>>>>>>>> Users in ldap with multiple values for their username attribute
>>>>>>>> will now be compared against the RDN of the entry to determine the
>>>>>>>> "primary" username. We will save all of the alternate names to the
>>>>>>>> ldb cache as well, so a lookup for any of them will return the
>>>>>>>> values for the primary name.
>>>>>>>> e.g.
>>>>>>>> getent passwd altusername
>>>>>>>> primaryuser:*:800014:800014:primaryuser:/home/primaryuser:/bin/sh
>>>>>>>
>>>>>>> Nack, turning SYSDB_NAME form a single valued attribute (as used
>>>>>>> throughout the code) to a multivalued attribute is a very dangerous
>>>>>>> proposition IMO.
>>>>>>>
>>>>>>> Although the current version of LDB always ends up returning values in
>>>>>>> the same order, fill_pwent() and other functions seem to be working
>>>>>>> only by accident now.
>>>>>>>
>>>>>>> I would rather use a different attribute name for aliases.
>>>>>>> This assuming we really want to allow aliases, I am personally still
>>>>>>> unconvinced it is a good idea. I would rather throw aliases away and
>>>>>>> allow only the canonical name to be stored in our cache.
>>>>>>>
>>>>>>>> Patch 0004: RFC2307: Handle multi-valued group names correctly
>>>>>>>> Groups in ldap with multiple values for their groupname attribute
>>>>>>>> will now be compared against the RDN of the entry to determine the
>>>>>>>> "primary" group name. We will save all of the alternate names to the
>>>>>>>> ldb cache as well, so a lookup for any of them will return the
>>>>>>>> values for the primary name.
>>>>>>>> e.g.
>>>>>>>> getent group altgroup
>>>>>>>> primarygroup:*:800014:member1,member2
>>>>>>>>
>>>>>>>> Patch 0005: RFC2307bis: Handle multi-valued group names correctly
>>>>>>>> Groups in ldap with multiple values for their groupname attribute
>>>>>>>> will now be compared against the RDN of the entry to determine the
>>>>>>>> "primary" group name. We will save all of the alternate names to the
>>>>>>>> ldb cache as well, so a lookup for any of them will return the
>>>>>>>> values for the primary name.
>>>>>>>> e.g.
>>>>>>>> getent group altgroup
>>>>>>>> primarygroup:*:800014:member1,member2
>>>>>>>>
>>>>>>>>
>>>>>>>> I tested with RFC2307, RFC2307bis and FreeIPA v2 data.
>>>>>>>
>>>>>>> I think the fact you are getting back the "right" name is happening by
>>>>>>> accident.
>>>>>>> Change the code to explicitly save the aliases first and add the
>>>>>>> canonical name last, and I think you will see the code to fail
>>>>>>> consistently to give you back the canonical name.
>>>>>>>
>>>>>>> Simo.
>>>>>>>
>>>>
>>>>
>>>> I have made the following changes to the patches.
>>>>
>>>>  * sysdb_get_rdn() will return both the attribute name and value of the RDN.
>>>>
>>>>  * sysdb_attrs_primary_name() will now validate that the attribute name
>>>> matches the ldap_[user|group]_name value.
>>>>
>>>>  * We will no longer attempt to save the aliases at all. Only the
>>>> primary name will be stored in our database.
>>>>
>>>>
>>>> There is a side-effect here that I think is probably okay, but I thought
>>>> I should mention it all the same. A direct request for a user or group
>>>> alias will always cause a cache refresh of the primary group. This is
>>>> because a check of our cache will return no values for this alias, so we
>>>> need to go to the backend to validate it.
>>>>
>>>> This is not really an exploitable issue because we have the grouping
>>>> optimization that ensures that we'd only have a single lookup for this
>>>> alias going on at once (so the worst-case scenario is that a user could
>>>> force us to keep running lookups after the previous one completed).
>>>>
>>>> A potential future solution to this would be to store the alternate
>>>> names in the cache with a different attribute name as a sort of
>>>> secondary negative cache.
>>>>
>>>>> Patch works good, but please put the reoccurring sequence of
>>>>> sysdb_attrs_get_el()-sysdb_attrs_primary_name() calls into a call of its
>>>>> onw.
> 
> 
> 
> Thanks for the review. New patches attached makes the requested change.
> I also switched to having the sysdb_attrs_get_el() use SYSDB_NAME
> always, as that was the net effect anyway (since
> user_map[LDAP_USER_NAME].sys_name always maps to this anyway).
> 
>> Still works as expected.
> 
>> ACK
> 

Pushed all five to master and sssd-1-5.


- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk2JtoMACgkQeiVVYja6o6O66gCeMbCXdMTLr9zSAgJa8/dE5CZZ
99QAoI5JPwFl0bKPg3BmxXD21lpmt/Id
=oFVe
-----END PGP SIGNATURE-----



More information about the sssd-devel mailing list