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

Jakub Hrozek jhrozek at redhat.com
Tue Aug 19 11:14:36 UTC 2014


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.



More information about the sssd-devel mailing list