[SSSD] [PATCH] IPA: handle searches by SID in apply_subdomain_homedir

Michal Židek mzidek at redhat.com
Tue Aug 19 12:14:46 UTC 2014


On 08/19/2014 01:44 PM, Michal Židek wrote:
> On 08/19/2014 01:14 PM, Jakub Hrozek wrote:
>> On Mon, Aug 18, 2014 at 08:52:52PM +0200, Michal Židek wrote:
>>> On 08/18/2014 10:46 AM, Jakub Hrozek wrote:
>>>> On Tue, Aug 12, 2014 at 01:14:03PM +0200, Lukas Slebodnik wrote:
>>>>> On (12/08/14 10:54), Jakub Hrozek wrote:
>>>>>> Hi,
>>>>>>
>>>>>> the attached patch fixes https://fedorahosted.org/sssd/ticket/2391
>>>>>>
>>>>>> To reproduce, request a group by SID, like this:
>>>>>>
>>>>>> $ python
>>>>>>     import pysss_nss_idmap
>>>>>>
>>>>>> pysss_nss_idmap.getnamebysid('S-1-5-21-1104576142-166023250-1132398744-513')
>>>>>>
>>>>>>     {'S-1-5-21-1104576142-166023250-1132398744-513': {'type': 2,
>>>>>> 'name': u'domain users at win.example.com'}}
>>>>>
>>>>> >From 95ef335f17739c441bb6c9793a6b4d7c6185ca2b Mon Sep 17 00:00:00
>>>>> 2001
>>>>>> From: Jakub Hrozek <jhrozek at redhat.com>
>>>>>> Date: Tue, 12 Aug 2014 10:32:33 +0200
>>>>>> Subject: [PATCH] IPA: handle searches by SID in
>>>>>> apply_subdomain_homedir
>>>>>>
>>>>>> https://fedorahosted.org/sssd/ticket/2391
>>>>>>
>>>>>> apply_subdomain_homedir() didn't handle the situation where an entity
>>>>>> that doesn't match was requested from the cache. For user and group
>>>>>> lookups this wasn't a problem because the negative match was caught
>>>>>> sooner.
>>>>>>
>>>>>> But SID lookups can match either user or group. When a group SID was
>>>>>> requested, the preceding LDAP request matched the SID and stored the
>>>>>> group in the cache. Then apply_subdomain_homedir() only tried to
>>>>>> search
>>>>>> user by SID, didn't find the entry and accessed a NULL pointer.
>>>>>>
>>>>>> A simple reproducer is:
>>>>>> $ python
>>>>>>>>> import pysss_nss_idmap
>>>>>>>>> pysss_nss_idmap.getnamebysid(group_sid)
>>>>>>
>>>>>> The group_sid can be anything, including Domain Users (XXX-513)
>>>>>> ---
>>>>>> src/providers/ipa/ipa_subdomains_id.c | 18 +++++++++++-------
>>>>>> 1 file changed, 11 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/src/providers/ipa/ipa_subdomains_id.c
>>>>>> b/src/providers/ipa/ipa_subdomains_id.c
>>>>>> index
>>>>>> 113bc6c06f82bc631b3efa92b87a1cadc7f22605..7857cd566f7585663d2e0fc94b1265132f4af739
>>>>>> 100644
>>>>>> --- a/src/providers/ipa/ipa_subdomains_id.c
>>>>>> +++ b/src/providers/ipa/ipa_subdomains_id.c
>>>>>> @@ -516,18 +516,22 @@ apply_subdomain_homedir(TALLOC_CTX *mem_ctx,
>>>>>> struct sss_domain_info *dom,
>>>>>>          goto done;
>>>>>>      }
>>>>>>
>>>>>> -    if (ret != EOK && ret != ENOENT) {
>>>>>> -        DEBUG(SSSDBG_OP_FAILURE,
>>>>>> -              "Failed to make request to our cache: [%d]: [%s]\n",
>>>>>> -               ret, sss_strerror(ret));
>>>>>> -        goto done;
>>>>>> -    }
>>>>>> -
>>>>>>      if ((res && res->count == 0) || (msg && msg->num_elements ==
>>>>>> 0)) {
>>>>>>          ret = ENOENT;
>>>>>>          goto done;
>>>>>>      }
>>>>>>
>>>>>> +    if (ret != EOK && ret != ENOENT) {
>>>>>> +        DEBUG(SSSDBG_OP_FAILURE,
>>>>>> +              "Failed to make request to our cache: [%d]: [%s]\n",
>>>>>> +               ret, sss_strerror(ret));
>>>>>> +        goto done;
>>>>>> +    } else if (ret == ENOENT) {
>>>>>> +        DEBUG(SSSDBG_TRACE_FUNC, "Cannot find [%s] with search
>>>>>> type [%d]\n",
>>>>>> +              filter_value, filter_type);
>>>>>> +        goto done;
>>>>>> +    }
>>>>>> +
>>>>>>      if (res != NULL) {
>>>>>>          msg = res->msgs[0];
>>>>>>      }
>>>>>
>>>>> I admit it can fix the crash, but it is opposite of change PavelR
>>>>> did in
>>>>> different code.
>>>>>
>>>>> commit 09be002e58babda513b4b75d2b9eb9b2c351fa26
>>>>> Author: Pavel Reichl <preichl at redhat.com>
>>>>> Date:   Wed Feb 26 16:58:24 2014 +0000
>>>>>      NSS: sysdb_getnetgr check return value first
>>>>
>>>> OK, what about the attached version? I unrolled the changes so the code
>>>> only sets ENOENT and does not shortcut until the ENOENT check.
>>>>
>>>>>
>>>>> It could be also chance to improve problems decribed in ticket #1991.
>>>>
>>>> Yes, but is it in the scope of fixing this crash and minimal changes
>>>> towards downstream? Could we fix the crash with as small patch as
>>>> possible and add additional one of top? I'm fine with adding the
>>>> additional patch to master right away..
>>>>
>>>>>
>>>>> BTW I was not able to reproduce crash. Could you describe hierarchy
>>>>> of groups
>>>>> in your testing environment.
>>>>
>>>> You need to test in a scenario with AD-IPA trusts on the IPA server
>>>> itself. The hierarchy is not nested, the user is a member of two flat
>>>> groups that have POSIX IDs and Domain Users which don't have POSIX IDs.
>>>>
>>>> There it was enough for me to request a group by its SID:
>>>> $ python
>>>>      import pysss_nss_idmap
>>>>
>>>> pysss_nss_idmap.getnamebysid('S-1-5-21-1104576142-166023250-1132398744-513')
>>>>
>>>>      {'S-1-5-21-1104576142-166023250-1132398744-513': {'type': 2,
>>>> 'name': u'domain users at win.example.com'}}
>>>>
>>>>
>>>>
>>>
>>> I can not reproduce this with current master. What SSSD version
>>> did you use?
>>
>> git master.
>>
>> I can give you access to my test setup, I can reproduce the problem
>> there easily.
>
> Sorry, I tested it wrongly (used the user SID, not the group SID).
> I can reproduce it now, will send review soon.
>
> Michal
>

ACK for master. Please send also patch that applies on sssd-1-11.

Michal




More information about the sssd-devel mailing list