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

Sumit Bose sbose at redhat.com
Mon Mar 21 20:24:40 UTC 2011


On Thu, Mar 17, 2011 at 05:25:55PM -0400, Stephen Gallagher wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> 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.

bye,
Sumit

> 
> - -- 
> 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/
> 
> iEYEARECAAYFAk2CfGMACgkQeiVVYja6o6PMDwCgsNqV9lYnLVkMh26XoVR36A56
> pZoAn12FAncgYIbPtVzoJB0Snu7eDLMi
> =LQKy
> -----END PGP SIGNATURE-----



More information about the sssd-devel mailing list