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(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?
git master.
I can give you access to my test setup, I can reproduce the problem
there easily.