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 [email protected]'}}

>From 95ef335f17739c441bb6c9793a6b4d7c6185ca2b Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <[email protected]>
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 <[email protected]>
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 [email protected]'}}




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

_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to