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(a)win.example.com'}}
>
> >From 95ef335f17739c441bb6c9793a6b4d7c6185ca2b Mon Sep 17 00:00:00 2001
>> From: Jakub Hrozek <jhrozek(a)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(a)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(a)win.example.com'}}
I can not reproduce this with current master. What SSSD version
did you use?
Michal