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