[SSSD] [PATCH] AD: use GC for SID requests as well

Sumit Bose sbose at redhat.com
Fri Jan 30 13:24:34 UTC 2015


On Fri, Jan 30, 2015 at 02:23:43PM +0100, Sumit Bose wrote:
> On Fri, Jan 30, 2015 at 01:27:34PM +0100, Jakub Hrozek wrote:
> > On Wed, Jan 28, 2015 at 08:18:04PM +0100, Sumit Bose wrote:
> > > On Wed, Jan 28, 2015 at 04:21:53PM +0100, Sumit Bose wrote:
> > > > On Wed, Jan 28, 2015 at 03:27:44PM +0100, Jakub Hrozek wrote:
> > > > > On Wed, Jan 28, 2015 at 03:05:00PM +0100, Sumit Bose wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > another issue found by Steeve during testing. To reproduce this you need
> > > > > > a universal group with members from different domains. Then either look
> > > > > > up the group by SID e.g. with
> > > > > > 
> > > > > > python -c "import pysss_nss_idmap; print pysss_nss_idmap.getnamebysid('S-1-5-21-3456664713-2053453454-4165325232-1234')"
> > > > > > 
> > > > > > and then with getent group groupname.
> > > > > > 
> > > > > > Or use IPA views, override the group name in the 'default trust view'
> > > > > > on the IPA server and look up the group by the overridden name. In both
> > > > > > case the group should not already be in the cache. Only members from the
> > > > > > domain of the group should be show without the patch.
> > > > > > 
> > > > > > bye,
> > > > > > Sumit
> > > > > 
> > > > > The patch is correct, but I'm worried about the implications. What kind
> > > > > of requests by SID does the server receive? Do we also resolve requests
> > > > > for users by SID? In that case, we might be surprised that some POSIX
> > > > > attributes are not available in GC..
> > > > 
> > > > good point, I'm working on a group only alternative.
> > > 
> > > Please find attached a slightly extended version. It tries to switch to
> > > the LDAP connection in the common LDAP code. We already do similar
> > > things during group requests in the common LDAP code so I hope this is
> > > ok. This switch only happens during user_and_group request where it is
> > > not clear if the match will be a user or a group, e.g. a search by SID.
> > > I have to admit that there is a side effect because the AD provider uses
> > > GC lookups with LDAP fallback for these types of requests and will be
> > > affected by this change as well. But since the more detailed user
> > > information will be available via LDAP this might even be a benefit.
> > > 
> > > bye,
> > > Sumit
> > 
> > [...]
> > 
> > > -    if (state->sdap_ret == ENOENT) {
> > > +    if (state->sdap_ret == ENOENT && state->noexist_delete == true) {
> > 
> > I don't think this change is correct, the noexist_delete check should be
> > in its own nested if, otherwise looking up a SID that matches neither user
> > nor group fails with EIO:
> 
> ah sorry, of course you are right new version attached.

and now with a patch.

> 
> bye,
> Sumit
> 
> > Breakpoint 3, get_user_and_group_users_done (subreq=0x18beca0) at src/providers/ldap/ldap_id.c:1799
> > 1799        struct tevent_req *req = tevent_req_callback_data(subreq,
> > (gdb) n
> > 1801        struct get_user_and_group_state *state = tevent_req_data(req,
> > (gdb) 
> > 1805        ret = users_get_recv(subreq, &state->dp_error, &state->sdap_ret);
> > (gdb) 
> > 1806        talloc_zfree(subreq);
> > (gdb) 
> > 1808        if (ret != EOK) {
> > (gdb) 
> > 1812        if (state->sdap_ret == ENOENT && state->noexist_delete == true) {
> > (gdb) 
> > 1822        } else if (state->sdap_ret != EOK) {
> > (gdb) 
> > 1823            tevent_req_error(req, EIO);
> > (gdb) 
> > 1824            return;
> > (gdb) p state->sdap_ret
> > $1 = 2
> > (gdb) p state->noexist_delete 
> > $2 = false
> > (gdb) quit
> > _______________________________________________
> > sssd-devel mailing list
> > sssd-devel at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
-------------- next part --------------
From 44d917806f8b8ce0f2fe536a901ef278aadf40c4 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 28 Jan 2015 14:04:45 +0100
Subject: [PATCH] AD: use GC for SID requests as well

If a universal group is looked up by SID the cross-domain members must
be resolved with the help of the Global Catalog.

Related to https://fedorahosted.org/sssd/ticket/2514
---
 src/providers/ipa/ipa_subdomains_id.c |  1 +
 src/providers/ldap/ldap_id.c          | 38 ++++++++++++++++++++++++++---------
 2 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/src/providers/ipa/ipa_subdomains_id.c b/src/providers/ipa/ipa_subdomains_id.c
index c8714a216daff7506f00248e25c281529d0479c4..0508e14b690c144f4bace9ed14a326ac724eb910 100644
--- a/src/providers/ipa/ipa_subdomains_id.c
+++ b/src/providers/ipa/ipa_subdomains_id.c
@@ -603,6 +603,7 @@ ipa_get_ad_acct_send(TALLOC_CTX *mem_ctx,
      */
     switch (state->ar->entry_type & BE_REQ_TYPE_MASK) {
     case BE_REQ_INITGROUPS:
+    case BE_REQ_BY_SECID:
     case BE_REQ_GROUP:
         clist = ad_gc_conn_list(req, ad_id_ctx, state->obj_dom);
         if (clist == NULL) {
diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index 2e58f4e49eb33a85cbb8b4144c69004c6b5b312b..5ce462d77867f115fe5c0214fcb95b72a4370472 100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -33,6 +33,7 @@
 #include "providers/ldap/sdap_async.h"
 #include "providers/ldap/sdap_idmap.h"
 #include "providers/ldap/sdap_users.h"
+#include "providers/ad/ad_common.h"
 
 /* =Users-Related-Functions-(by-name,by-uid)============================== */
 
@@ -1745,6 +1746,8 @@ static void get_user_and_group_groups_done(struct tevent_req *subreq)
     struct get_user_and_group_state *state = tevent_req_data(req,
                                                struct get_user_and_group_state);
     int ret;
+    struct ad_id_ctx *ad_id_ctx;
+    struct sdap_id_conn_ctx *user_conn;
 
     ret = groups_get_recv(subreq, &state->dp_error, &state->sdap_ret);
     talloc_zfree(subreq);
@@ -1764,8 +1767,22 @@ static void get_user_and_group_groups_done(struct tevent_req *subreq)
 
     /* Now the search finished fine but did not find an entry.
      * Retry with users. */
+
+    user_conn = state->conn;
+    /* Prefer LDAP over GC for users */
+    if (state->id_ctx->opts->schema_type == SDAP_SCHEMA_AD
+            && state->sdom->pvt != NULL) {
+        ad_id_ctx = talloc_get_type(state->sdom->pvt, struct ad_id_ctx);
+        if (ad_id_ctx != NULL &&  ad_id_ctx->ldap_ctx != NULL
+                && state->conn == ad_id_ctx->gc_ctx) {
+            DEBUG(SSSDBG_TRACE_ALL,
+                  "Switching to LDAP connection for user lookup.\n");
+            user_conn = ad_id_ctx->ldap_ctx;
+        }
+    }
+
     subreq = users_get_send(req, state->ev, state->id_ctx,
-                            state->sdom, state->conn,
+                            state->sdom, user_conn,
                             state->filter_val, state->filter_type, NULL,
                             state->attrs_type, state->noexist_delete);
     if (subreq == NULL) {
@@ -1792,16 +1809,17 @@ static void get_user_and_group_users_done(struct tevent_req *subreq)
         tevent_req_error(req, ret);
         return;
     }
-
     if (state->sdap_ret == ENOENT) {
-        /* The search ran to completion, but nothing was found.
-         * Delete the existing entry, if any. */
-        ret = sysdb_delete_by_sid(state->sysdb, state->domain,
-                                  state->filter_val);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE, "Could not delete entry by SID!\n");
-            tevent_req_error(req, ret);
-            return;
+        if (state->noexist_delete == true) {
+            /* The search ran to completion, but nothing was found.
+             * Delete the existing entry, if any. */
+            ret = sysdb_delete_by_sid(state->sysdb, state->domain,
+                                      state->filter_val);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_OP_FAILURE, "Could not delete entry by SID!\n");
+                tevent_req_error(req, ret);
+                return;
+            }
         }
     } else if (state->sdap_ret != EOK) {
         tevent_req_error(req, EIO);
-- 
2.1.0



More information about the sssd-devel mailing list