[SSSD] [PATCH] ad: support cross domain membership

Jakub Hrozek jhrozek at redhat.com
Mon Oct 28 15:51:27 UTC 2013


On Fri, Oct 25, 2013 at 10:40:28PM +0200, Jakub Hrozek wrote:
> On Fri, Oct 25, 2013 at 10:33:42PM +0200, Pavel Březina wrote:
> > On 10/25/2013 10:24 PM, Pavel Březina wrote:
> > >On 10/25/2013 08:10 PM, Jakub Hrozek wrote:
> > >>On Fri, Oct 25, 2013 at 01:47:51PM -0400, Pavel Brezina wrote:
> > >>>
> > >>>
> > >>>----- Original Message -----
> > >>>>From: "Jakub Hrozek" <jhrozek at redhat.com>
> > >>>>To: sssd-devel at lists.fedorahosted.org
> > >>>>Sent: Friday, October 25, 2013 4:01:03 PM
> > >>>>Subject: Re: [SSSD] [PATCH] ad: support cross domain membership
> > >>>>
> > >>>>On Fri, Oct 25, 2013 at 03:27:40PM +0200, Pavel Březina wrote:
> > >>>>>On 10/25/2013 02:22 PM, Jakub Hrozek wrote:
> > >>>>>>On Fri, Oct 25, 2013 at 12:58:23PM +0200, Pavel Březina wrote:
> > >>>>>>>On 10/25/2013 10:55 AM, Pavel Březina wrote:
> > >>>>>>>>On 10/24/2013 08:40 PM, Jakub Hrozek wrote:
> > >>>>>>>>>On Wed, Oct 02, 2013 at 04:13:32PM +0200, Pavel Březina wrote:
> > >>>>>>>>>>On 10/01/2013 09:54 PM, Jakub Hrozek wrote:
> > >>>>>>>>>>>On Tue, Sep 24, 2013 at 03:17:47PM +0200, Pavel Březina wrote:
> > >>>>>>>>>>>>On 09/24/2013 01:32 PM, Jakub Hrozek wrote:
> > >>>>>>>>>>>>>On Wed, Sep 11, 2013 at 02:40:14PM +0200, Pavel Březina wrote:
> > >>>>>>>>>>>>>>https://fedorahosted.org/sssd/ticket/2064
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>These patch set depends on: [PATCH] ad: store group in
> > >>>>>>>>>>>>>>correct
> > >>>>>>>>>>>>>>tree on initgroups via tokenGroups
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>You can also pull it with all dependencies from my
> > >>>>>>>>>>>>>>repository:
> > >>>>>>>>>>>>>>fedorapeople.org:public_git/sssd.git #ad-groups
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>The fundamental changes in this patch set are: - lookup
> > >>>>>>>>>>>>>>groups
> > >>>>>>>>>>>>>>in global catalog - pick up member domain from its originalDN
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>>From 0273d17f24eac7b60dfc0515a9e3b97ad16d1199 Mon Sep 17
> > >>>>>>>>>>>>>>00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
> > >>>>>>>>>>>>>><pbrezina at redhat.com> Date: Mon, 9 Sep 2013 15:52:03 +0200
> > >>>>>>>>>>>>>>Subject: [PATCH 1/9] ad: shortcut if possible during get
> > >>>>>>>>>>>>>>object
> > >>>>>>>>>>>>>>by ID or SID
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>When getByID or getBySID comes from responder, the request
> > >>>>>>>>>>>>>>doesn't necessarily have to contain correct domain, since
> > >>>>>>>>>>>>>>responder iterates over all domains until it finds a match.
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>Every domain has its own ID range, so we can simply shortcut
> > >>>>>>>>>>>>>>if domain does not match and avoid LDAP round trip. Responder
> > >>>>>>>>>>>>>>will continue with next domain until it finds the correct
> > >>>>>>>>>>>>>>one.
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>This patch seems OK to me, but I'd like a second look from
> > >>>>>>>>>>>>>someone who understands the ranges better (which is probably
> > >>>>>>>>>>>>>Sumit)
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>>From f74d4637980438032649dfbf079fa6c839862586 Mon Sep 17
> > >>>>>>>>>>>>>>00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
> > >>>>>>>>>>>>>><pbrezina at redhat.com> Date: Tue, 10 Sep 2013 10:40:06 +0200
> > >>>>>>>>>>>>>>Subject: [PATCH 2/9] ad: simplify get_conn_list()
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>It was originally design to return list of connection
> > >>>>>>>>>>>>>>objects,
> > >>>>>>>>>>>>>>it really always work with only one connection.
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>I'd like to review this patch and the following along with my
> > >>>>>>>>>>>>>patches to look up POSIX IDs in GC, they touch the same code.
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>>From ad5dc9e7557ef605fc5d7fc759e5cb6c2f9a148c Mon Sep 17
> > >>>>>>>>>>>>>>00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
> > >>>>>>>>>>>>>><pbrezina at redhat.com> Date: Tue, 10 Sep 2013 14:45:50 +0200
> > >>>>>>>>>>>>>>Subject: [PATCH 4/9] sdap_domain_add(): fix possible memory
> > >>>>>>>>>>>>>>leak
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>ACK.
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>>From 9f2c212e01700289d70002c8c39b732ca6c11cee Mon Sep 17
> > >>>>>>>>>>>>>>00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
> > >>>>>>>>>>>>>><pbrezina at redhat.com> Date: Tue, 10 Sep 2013 14:45:52 +0200
> > >>>>>>>>>>>>>>Subject: [PATCH 5/9] sdap: store base dn in sdap_domain
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>Groups may contain members from different domains.
> > >>>>>>>>>>>>>>Remembering
> > >>>>>>>>>>>>>>base dn in domain object gives us the ability to simply
> > >>>>>>>>>>>>>>lookup
> > >>>>>>>>>>>>>>correct domain by comparing object dn with domain base dn.
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>Resolves: https://fedorahosted.org/sssd/ticket/2064
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>I haven't tested these patches yet.
> > >>>>>>>>>>>>
> > >>>>>>>>>>>>I'm sending rebased version of my patches.
> > >>>>>>>>>>>>
> > >>>>>>>>>>>>[PATCH 4/9] sdap_domain_add(): fix possible memory leak was
> > >>>>>>>>>>>>removed
> > >>>>>>>>>>>>from the patch set since recent Sumit's patch removed the
> > >>>>>>>>>>>code I
> > >>>>>>>>>>>>fixed :-)
> > >>>>>>>>>>>
> > >>>>>>>>>>>Hi,
> > >>>>>>>>>>>
> > >>>>>>>>>>>can you check if patches #6 and #7 still apply after the recent
> > >>>>>>>>>>>changes in 1.11 ? We actually do use the LDAP fallback now..
> > >>>>>>>>>>
> > >>>>>>>>>>They won't apply, but I think it is quite all right to just
> > >>>>>>>>>>skip them.
> > >>>>>>>>>>The purpose of these patches was to always contact GC for
> > >>>>>>>>>>get_group
> > >>>>>>>>>>and
> > >>>>>>>>>>initgroups.
> > >>>>>>>>>>
> > >>>>>>>>>>We always contact GC first at the moment and having LDAP as
> > >>>>>>>>>>fallback
> > >>>>>>>>>>is
> > >>>>>>>>>>fine for groups. If there will be a member from different
> > >>>>>>>>>>domain, we
> > >>>>>>>>>>will just fail - but if there won't be foreign member it will
> > >>>>>>>>>>work.
> > >>>>>>>>>
> > >>>>>>>>>Hi,
> > >>>>>>>>>
> > >>>>>>>>>In general the patches look good to me but I think the first
> > >>>>>>>>>two can be
> > >>>>>>>>>simplified.
> > >>>>>>>>>
> > >>>>>>>>>Instead of magically trying to find the base DN from the entry's
> > >>>>>>>>>original DN, I think we can simplify it to:
> > >>>>>>>>>
> > >>>>>>>>>struct sdap_domain *
> > >>>>>>>>>sdap_domain_get_by_dn(struct sdap_options *opts,
> > >>>>>>>>>                       const char *dn)
> > >>>>>>>>>{
> > >>>>>>>>>     struct sdap_domain *sditer = NULL;
> > >>>>>>>>>
> > >>>>>>>>>     DLIST_FOR_EACH(sditer, opts->sdom) {
> > >>>>>>>>>         if (sss_ldap_dn_in_search_bases(tmp_ctx, dn,
> > >>>>>>>>>sditer->search_bases, NULL) ||
> > >>>>>>>>>             sss_ldap_dn_in_search_bases(tmp_ctx, dn,
> > >>>>>>>>>sditer->user_search_bases, NULL) ||
> > >>>>>>>>>             sss_ldap_dn_in_search_bases(tmp_ctx, dn,
> > >>>>>>>>>sditer->group_search_bases, NULL)) {
> > >>>>>>>>>             return sditer;
> > >>>>>>>>>         }
> > >>>>>>>>>     }
> > >>>>>>>>>
> > >>>>>>>>>     return NULL;
> > >>>>>>>>>}
> > >>>>>>>>>
> > >>>>>>>>>Then you won't need to store the base DN, no need for the magic
> > >>>>>>>>>with
> > >>>>>>>>>strstr
> > >>>>>>>>>and the code will work even for custom search bases.
> > >>>>>>>>
> > >>>>>>>>Hi,
> > >>>>>>>>new patches are attached. I just added all of the search bases.
> > >>>>>>>
> > >>>>>>>And here's one more iteration. While working on token groups patches
> > >>>>>>>I found a bug in the first patch. I tried to shortcut using id
> > >>>>>>>mapping even if id mapping is disabled. This made the posix groups
> > >>>>>>>unresolvable since id mapping returned an error when trying to map
> > >>>>>>>GID to SID.
> > >>>>>>>
> > >>>>>>
> > >>>>>>Thanks. See also one more attached patch I applied on top of yours to
> > >>>>>>make cross-domain group resolution work. I'm still chasing one bug in
> > >>>>>>fill_members that might cause some members to be not qualified,
> > >>>>>>though.
> > >>>>>
> > >>>>>Hi,
> > >>>>>ack to this patch. The problem was when you have:
> > >>>>>group at subad : user at parentad
> > >>>>>it did not find it. Jakub's patch fixes it.
> > >>>>>
> > >>>>>There is still one problem that was introduce by the change of
> > >>>>>sdap_domain_get_by_dn to search bases instead of string comparison.
> > >>>>>
> > >>>>>We have domains:
> > >>>>>ad.pb -> sub.ad.pb
> > >>>>>
> > >>>>>Search bases:
> > >>>>>ad.pb: dc=ad,dc=pb
> > >>>>>sub.ad.pb: dc=sub,dc=ad,dc=pb
> > >>>>>
> > >>>>>User:
> > >>>>>cn=user,dc=sub,dc=ad,dc=pb
> > >>>>>
> > >>>>>This user belongs to sub.ad.pb but its dn match with dc=ad,dc=pb and
> > >>>>>as a result we will assign him to domain ad.pb.
> > >>>>>
> > >>>>>We can either swap back to my original patches or tune
> > >>>>>sss_ldap_dn_in_search_bases() so it returns expected result for this
> > >>>>>situation (it will require all dc parts to match).
> > >>>>
> > >>>>I would prefer the latter. Just remove the RDN and return true only if
> > >>>>the rest matches.
> > >>>
> > >>>Comparing only the part after RDN is not sufficient.
> > >>>
> > >>>dn: cn=Joe,cn=Manager,cn=Users,dc=example,dc=com
> > >>>rdn: cn=Joe
> > >>>search base: dc=example,dc=com
> > >>>
> > >>>strcmp(cn=Manager,cn=Users,dc=example,dc=com, dc=example,dc=com) != 0
> > >>>
> > >>>We would have to use similar "magic" to locate domain part of both dn
> > >>>that you did not like.
> > >>
> > >>Well, the main reason I didn't like the original patches was that they
> > >>were hardcoding "DC=". Sure, that works for AD, but I don't like these
> > >>hacks in the generic LDAP code.
> > >>
> > >>Could we modify sdap_domain_get_by_dn() so that it would try to match as
> > >>much as possible?
> > >>
> > >>btw latest patches based on your patchset are attached. I added two
> > >>fixes in the NSS responder that I needed to get getent reporting the
> > >>right members.
> > >
> > >We agreed with Jakub to go with original approach for the moment. Here
> > >is the current working patchset. The latest Jakub's patch contains a bug
> > >when no members are printed. We didn't find the root of this bug yet,
> > >but we need to leave it for a fresh day.
> > 
> > And one more time, now with Sumit's note addressed.
> 
> I have filed a ticket for fixing the nss responder -
> https://fedorahosted.org/sssd/ticket/2131
> 
> And one more to improve detecting the search bases - 
> https://fedorahosted.org/sssd/ticket/2132
> 
> I bypassed triage and put them directly to the 1.11.x bucket as they
> improve on existing feature.

The testing itself went fine, but patch #1 didn't even compile. Attached
are patches after I amended them, only patch #1 changed.
-------------- next part --------------
>From d8d7153c77dc0b43f5ecfd1a36f5b7e3a2e3fb0a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
Date: Mon, 9 Sep 2013 15:52:03 +0200
Subject: [PATCH 1/8] ad: shortcut if possible during get object by ID or SID

When getByID or getBySID comes from responder, the request doesn't
necessarily have to contain correct domain, since responder iterates
over all domains until it finds a match.

Every domain has its own ID range, so we can simply shortcut if
domain does not match and avoid LDAP round trip. Responder will
continue with next domain until it finds the correct one.
---
 src/providers/ad/ad_id.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/src/providers/ad/ad_id.c b/src/providers/ad/ad_id.c
index 20f9c23fa8d1d272f91c0b886bd1e2e3a2522320..986639404ccc1cac09b03ac3228f3c3cc3be94ca 100644
--- a/src/providers/ad/ad_id.c
+++ b/src/providers/ad/ad_id.c
@@ -20,10 +20,12 @@
     along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 #include "util/util.h"
+#include "util/strtonum.h"
 #include "providers/ad/ad_common.h"
 #include "providers/ad/ad_id.h"
 #include "providers/ad/ad_domain_info.h"
 #include "providers/ldap/sdap_async_enum.h"
+#include "providers/ldap/sdap_idmap.h"
 
 struct ad_handle_acct_info_state {
     struct be_req *breq;
@@ -235,17 +237,63 @@ ad_account_info_handler(struct be_req *be_req)
     struct sss_domain_info *dom;
     struct sdap_domain *sdom;
     struct sdap_id_conn_ctx **clist;
+    struct sdap_idmap_ctx *idmap_ctx;
+    enum idmap_error_code map_err;
+    uint32_t id;
+    char *sid = NULL;
     errno_t ret;
 
     ad_ctx = talloc_get_type(be_ctx->bet_info[BET_ID].pvt_bet_data,
                              struct ad_id_ctx);
     ar = talloc_get_type(be_req_get_data(be_req), struct be_acct_req);
     sdap_id_ctx = ad_ctx->sdap_id_ctx;
+    idmap_ctx = sdap_id_ctx->opts->idmap_ctx;
 
     if (be_is_offline(be_ctx)) {
         return be_req_terminate(be_req, DP_ERR_OFFLINE, EAGAIN, "Offline");
     }
 
+    /* Try to shortcut if this is ID or SID search and it belongs to
+     * other domain range than is in ar->domain. */
+    if (sdap_idmap_domain_has_algorithmic_mapping(idmap_ctx,
+                                                  be_ctx->domain->name,
+                                                  be_ctx->domain->domain_id)) {
+        if (ar->filter_type == BE_FILTER_IDNUM
+                || ar->filter_type == BE_FILTER_SECID) {
+            if (ar->filter_type == BE_FILTER_IDNUM) {
+                id = strtouint32(ar->filter_value, NULL, 10);
+                if (errno != EOK) {
+                    ret = EINVAL;
+                    goto fail;
+                }
+
+                /* Convert the ID to its objectSID */
+                map_err = sss_idmap_unix_to_sid(idmap_ctx->map, id, &sid);
+                if (map_err != IDMAP_SUCCESS) {
+                    DEBUG(SSSDBG_MINOR_FAILURE,
+                          ("Mapping ID [%s] to SID failed: [%s]\n",
+                           ar->filter_value, idmap_error_string(map_err)));
+                    ret = EIO;
+                    goto fail;
+                }
+            } else {
+                sid = ar->filter_value;
+            }
+
+            dom = find_subdomain_by_sid(be_ctx->domain, sid);
+            if (dom == NULL) {
+                DEBUG(SSSDBG_OP_FAILURE, ("Invalid domain\n"));
+                ret = EINVAL;
+                goto fail;
+            }
+
+            if (strcasecmp(dom->name, ar->domain) != 0) {
+                be_req_terminate(be_req, DP_ERR_OK, EOK, NULL);
+                return;
+            }
+        }
+    }
+
     dom = be_ctx->domain;
     if (strcasecmp(ar->domain, be_ctx->domain->name) != 0) {
         /* Subdomain request, verify subdomain */
-- 
1.8.3.1

-------------- next part --------------
>From 1424ed01e73add5742e5c6e0f6f415e773d7fee4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
Date: Tue, 10 Sep 2013 14:45:52 +0200
Subject: [PATCH 2/8] sdap: store base dn in sdap_domain

Groups may contain members from different domains. Remembering
base dn in domain object gives us the ability to simply lookup
correct domain by comparing object dn with domain base dn.

Resolves:
https://fedorahosted.org/sssd/ticket/2064
---
 src/providers/ldap/ldap_common.c | 35 ++++++++++++++++++++---------------
 src/providers/ldap/sdap.h        |  2 ++
 2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c
index fcb8c4884a2f31916b4fbf46e295d951942adfdc..16a1e4fe9fc870ab92103a674fe58c0844498dd5 100644
--- a/src/providers/ldap/ldap_common.c
+++ b/src/providers/ldap/ldap_common.c
@@ -69,6 +69,7 @@ sdap_domain_add(struct sdap_options *opts,
                 struct sdap_domain **_sdom)
 {
     struct sdap_domain *sdom;
+    errno_t ret;
 
     sdom = talloc_zero(opts, struct sdap_domain);
     if (sdom == NULL) {
@@ -77,11 +78,27 @@ sdap_domain_add(struct sdap_options *opts,
     sdom->dom = dom;
     sdom->head = &opts->sdom;
 
+    /* Convert the domain name into search base */
+    ret = domain_to_basedn(sdom, sdom->dom->name, &sdom->basedn);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE,
+            ("Cannot convert domain name [%s] to base DN [%d]: %s\n",
+            dom->name, ret, strerror(ret)));
+        goto done;
+    }
+
     talloc_set_destructor((TALLOC_CTX *)sdom, sdap_domain_destructor);
     DLIST_ADD_END(opts->sdom, sdom, struct sdap_domain *);
 
     if (_sdom) *_sdom = sdom;
-    return EOK;
+    ret = EOK;
+
+done:
+    if (ret != EOK) {
+        talloc_free(sdom);
+    }
+
+    return ret;
 }
 
 errno_t
@@ -91,7 +108,6 @@ sdap_domain_subdom_add(struct sdap_id_ctx *sdap_id_ctx,
 {
     struct sss_domain_info *dom;
     struct sdap_domain *sdom, *sditer;
-    char *basedn;
     errno_t ret;
 
     for (dom = get_next_domain(parent, true);
@@ -120,16 +136,6 @@ sdap_domain_subdom_add(struct sdap_id_ctx *sdap_id_ctx,
             sdom = sditer;
         }
 
-        /* Convert the domain name into search base */
-        ret = domain_to_basedn(sdom, sdom->dom->name, &basedn);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE,
-                ("Cannot convert domain name [%s] to base DN [%d]: %s\n",
-                dom->name, ret, strerror(ret)));
-            talloc_free(basedn);
-            return ret;
-        }
-
         /* Update search bases */
         talloc_zfree(sdom->search_bases);
         sdom->search_bases = talloc_array(sdom, struct sdap_search_base *, 2);
@@ -138,9 +144,8 @@ sdap_domain_subdom_add(struct sdap_id_ctx *sdap_id_ctx,
         }
         sdom->search_bases[1] = NULL;
 
-        ret = sdap_create_search_base(sdom, basedn, LDAP_SCOPE_SUBTREE, NULL,
-                                      &sdom->search_bases[0]);
-        talloc_free(basedn);
+        ret = sdap_create_search_base(sdom, sdom->basedn, LDAP_SCOPE_SUBTREE,
+                                      NULL, &sdom->search_bases[0]);
         if (ret) {
             DEBUG(SSSDBG_OP_FAILURE, ("Cannot create new sdap search base\n"));
             return ret;
diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
index abc9052f64ee93cf9cc279fcdab423e576a98424..c53471b9bd6d9b18f37a8bf4089c8700d4a1163d 100644
--- a/src/providers/ldap/sdap.h
+++ b/src/providers/ldap/sdap.h
@@ -372,6 +372,8 @@ enum dc_functional_level {
 struct sdap_domain {
     struct sss_domain_info *dom;
 
+    char *basedn;
+
     struct sdap_search_base **search_bases;
     struct sdap_search_base **user_search_bases;
     struct sdap_search_base **group_search_bases;
-- 
1.8.3.1

-------------- next part --------------
>From 54595459a833d41953c0b92644cb173d7521494b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
Date: Tue, 10 Sep 2013 14:58:20 +0200
Subject: [PATCH 3/8] sdap: add sdap_domain_get_by_dn()

This function will find sdap domain by comparing object dn
with domain base dn.

Resolves:
https://fedorahosted.org/sssd/ticket/2064
---
 src/providers/ldap/ldap_common.c | 24 ++++++++++++++++++++++++
 src/providers/ldap/ldap_common.h |  4 ++++
 2 files changed, 28 insertions(+)

diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c
index 16a1e4fe9fc870ab92103a674fe58c0844498dd5..0356cb37a89d983b72b3cbadc0e8ba9014d34e09 100644
--- a/src/providers/ldap/ldap_common.c
+++ b/src/providers/ldap/ldap_common.c
@@ -63,6 +63,30 @@ sdap_domain_get(struct sdap_options *opts,
     return sditer;
 }
 
+struct sdap_domain *
+sdap_domain_get_by_dn(struct sdap_options *opts,
+                      const char *dn)
+{
+    struct sdap_domain *sditer = NULL;
+    char *dc = NULL;
+
+    dc = strstr(dn, "dc=");
+    if (dc == NULL) {
+        dc = strstr(dn, "DC=");
+        if (dc == NULL) {
+            return NULL;
+        }
+    }
+
+    DLIST_FOR_EACH(sditer, opts->sdom) {
+        if (strcasecmp(sditer->basedn, dc) == 0) {
+            return sditer;
+        }
+    }
+
+    return NULL;
+}
+
 errno_t
 sdap_domain_add(struct sdap_options *opts,
                 struct sss_domain_info *dom,
diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h
index 8b2b1ea918725bf7adbe5d0b9872d8e91b0db3b0..b3bd950e1dca6df0f5668397d5e5a0796e519862 100644
--- a/src/providers/ldap/ldap_common.h
+++ b/src/providers/ldap/ldap_common.h
@@ -282,6 +282,10 @@ sdap_domain_remove(struct sdap_options *opts,
 
 struct sdap_domain *sdap_domain_get(struct sdap_options *opts,
                                     struct sss_domain_info *dom);
+
+struct sdap_domain *sdap_domain_get_by_dn(struct sdap_options *opts,
+                                          const char *dn);
+
 errno_t
 sdap_create_search_base(TALLOC_CTX *mem_ctx,
                         const char *unparsed_base,
-- 
1.8.3.1

-------------- next part --------------
>From 0381f43789c8773f96aa08f169d687e4a373ab94 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
Date: Wed, 11 Sep 2013 12:41:43 +0200
Subject: [PATCH 4/8] ghosts: pick correct domain for every member

Groups may contain members from different domains. We need
to make sure that we store subdomain users with correct
domain name.

Resolves:
https://fedorahosted.org/sssd/ticket/2064
---
 src/providers/ldap/sdap_async_groups.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index 00ac3e991710f227c07b411ddffb641748a32213..03a9110a6d4e210d9d6f67646fab87df2ca5d86e 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -2005,6 +2005,8 @@ static errno_t sdap_nested_group_populate_users(TALLOC_CTX *mem_ctx,
     const char *username;
     char *clean_orig_dn;
     const char *original_dn;
+    struct sss_domain_info *user_dom;
+    struct sdap_domain *sdap_dom;
 
     TALLOC_CTX *tmp_ctx;
     struct ldb_message **msgs;
@@ -2045,14 +2047,6 @@ static errno_t sdap_nested_group_populate_users(TALLOC_CTX *mem_ctx,
     in_transaction = true;
 
     for (i = 0; i < num_users; i++) {
-        ret = sdap_get_user_primary_name(tmp_ctx, opts, users[i],
-                                         domain, &username);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_MINOR_FAILURE,
-                  ("User entry %d has no name attribute. Skipping\n", i));
-            continue;
-        }
-
         ret = sysdb_attrs_get_el(users[i], SYSDB_ORIG_DN, &el);
         if (el->num_values == 0) {
             ret = EINVAL;
@@ -2072,6 +2066,17 @@ static errno_t sdap_nested_group_populate_users(TALLOC_CTX *mem_ctx,
             goto done;
         }
 
+        sdap_dom = sdap_domain_get_by_dn(opts, original_dn);
+        user_dom = sdap_dom == NULL ? domain : sdap_dom->dom;
+
+        ret = sdap_get_user_primary_name(tmp_ctx, opts, users[i],
+                                         user_dom, &username);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_MINOR_FAILURE,
+                  ("User entry %d has no name attribute. Skipping\n", i));
+            continue;
+        }
+
         /* Check for the specified origDN in the sysdb */
         filter = talloc_asprintf(tmp_ctx, "(%s=%s)",
                                  SYSDB_ORIG_DN,
@@ -2080,7 +2085,7 @@ static errno_t sdap_nested_group_populate_users(TALLOC_CTX *mem_ctx,
             ret = ENOMEM;
             goto done;
         }
-        ret = sysdb_search_users(tmp_ctx, sysdb, domain, filter,
+        ret = sysdb_search_users(tmp_ctx, user_dom->sysdb, user_dom, filter,
                                  search_attrs, &count, &msgs);
         talloc_zfree(filter);
         talloc_zfree(clean_orig_dn);
@@ -2109,7 +2114,7 @@ static errno_t sdap_nested_group_populate_users(TALLOC_CTX *mem_ctx,
 
             ret = sysdb_attrs_add_string(attrs, SYSDB_NAME, username);
             if (ret) goto done;
-            ret = sysdb_set_user_attr(sysdb, domain, sysdb_name,
+            ret = sysdb_set_user_attr(user_dom->sysdb, user_dom, sysdb_name,
                                       attrs, SYSDB_MOD_REP);
             if (ret != EOK) goto done;
         } else {
-- 
1.8.3.1

-------------- next part --------------
>From 6f57d501e13def83b00fcf25ef5579ae48461dfa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
Date: Tue, 10 Sep 2013 15:06:28 +0200
Subject: [PATCH 5/8] sdap_fill_memberships: pick correct domain for every
 member

Groups may contain members from different domains. We need
to make sure that we always choose correct domain for subdomain
users when looking up in sysdb.

Resolves:
https://fedorahosted.org/sssd/ticket/2064
---
 src/providers/ldap/sdap_async_groups.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index 03a9110a6d4e210d9d6f67646fab87df2ca5d86e..4abd3b71ca20417df6ac289fdd584d771ec4b610 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -175,7 +175,8 @@ sdap_dn_by_primary_gid(TALLOC_CTX *mem_ctx, struct sysdb_attrs *ldap_attrs,
     return EOK;
 }
 
-static int sdap_fill_memberships(struct sysdb_attrs *group_attrs,
+static int sdap_fill_memberships(struct sdap_options *opts,
+                                 struct sysdb_attrs *group_attrs,
                                  struct sysdb_ctx *ctx,
                                  struct sss_domain_info *domain,
                                  hash_table_t *ghosts,
@@ -190,6 +191,9 @@ static int sdap_fill_memberships(struct sysdb_attrs *group_attrs,
     errno_t hret;
     hash_key_t key;
     hash_value_t value;
+    struct sdap_domain *sdom;
+    struct sysdb_ctx *member_sysdb;
+    struct sss_domain_info *member_dom;
 
     ret = sysdb_attrs_get_el(group_attrs, SYSDB_MEMBER, &el);
     if (ret) {
@@ -215,9 +219,20 @@ static int sdap_fill_memberships(struct sysdb_attrs *group_attrs,
         }
 
         if (hret == HASH_ERROR_KEY_NOT_FOUND) {
+            sdom = sdap_domain_get_by_dn(opts, (char *)values[i].data);
+            if (sdom == NULL) {
+                DEBUG(SSSDBG_MINOR_FAILURE, ("Member [%s] is it out of domain "
+                      "scope?\n", (char *)values[i].data));
+                member_sysdb = ctx;
+                member_dom = domain;
+            } else {
+                member_sysdb = sdom->dom->sysdb;
+                member_dom = sdom->dom;
+            }
+
             /* sync search entry with this as origDN */
-            ret = sdap_find_entry_by_origDN(el->values, ctx, domain,
-                                            (char *)values[i].data,
+            ret = sdap_find_entry_by_origDN(el->values, member_sysdb,
+                                            member_dom, (char *)values[i].data,
                                             (char **)&el->values[j].data);
             if (ret == ENOENT) {
                 /* member may be outside of the configured search bases
@@ -709,7 +724,7 @@ static int sdap_save_grpmem(TALLOC_CTX *memctx,
             goto fail;
         }
 
-        ret = sdap_fill_memberships(group_attrs, ctx, dom, ghosts,
+        ret = sdap_fill_memberships(opts, group_attrs, ctx, dom, ghosts,
                                     el->values, el->num_values,
                                     userdns, nuserdns);
         if (ret) {
-- 
1.8.3.1

-------------- next part --------------
>From 174df5872665a5ffb0427fab87c41b7acfc09177 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
Date: Wed, 11 Sep 2013 14:01:31 +0200
Subject: [PATCH 6/8] nested groups: pick correct domain for cache lookups

Groups may contain members from different domains. We need
to make sure that we always choose correct domain for subdomain
users when looking up in sysdb.

Resolves:
https://fedorahosted.org/sssd/ticket/2064
---
 src/providers/ldap/sdap_async_nested_groups.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/providers/ldap/sdap_async_nested_groups.c b/src/providers/ldap/sdap_async_nested_groups.c
index 7040c6e9bae8a4765f3ffd50f62e7f321d723465..fafbb36ba2f918787c3a862ef2098e11da5552da 100644
--- a/src/providers/ldap/sdap_async_nested_groups.c
+++ b/src/providers/ldap/sdap_async_nested_groups.c
@@ -329,11 +329,14 @@ done:
 }
 
 static errno_t
-sdap_nested_group_check_cache(struct sss_domain_info *domain,
+sdap_nested_group_check_cache(struct sdap_options *opts,
+                              struct sss_domain_info *domain,
                               const char *member_dn,
                               enum sdap_nested_group_dn_type *_type)
 {
     TALLOC_CTX *tmp_ctx = NULL;
+    struct sdap_domain *sdap_domain = NULL;
+    struct sss_domain_info *member_domain = NULL;
     char *sanitized_dn = NULL;
     char *filter = NULL;
     errno_t ret;
@@ -355,8 +358,12 @@ sdap_nested_group_check_cache(struct sss_domain_info *domain,
         goto done;
     }
 
+    /* determine correct domain of this member */
+    sdap_domain = sdap_domain_get_by_dn(opts, member_dn);
+    member_domain = sdap_domain == NULL ? domain : sdap_domain->dom;
+
     /* search in users */
-    ret = sdap_nested_group_sysdb_search_users(domain, filter);
+    ret = sdap_nested_group_sysdb_search_users(member_domain, filter);
     if (ret == EOK || ret == EAGAIN) {
         /* user found */
         *_type = SDAP_NESTED_GROUP_DN_USER;
@@ -367,7 +374,7 @@ sdap_nested_group_check_cache(struct sss_domain_info *domain,
     }
 
     /* search in groups */
-    ret = sdap_nested_group_sysdb_search_groups(domain, filter);
+    ret = sdap_nested_group_sysdb_search_groups(member_domain, filter);
     if (ret == EOK || ret == EAGAIN) {
         /* group found */
         *_type = SDAP_NESTED_GROUP_DN_GROUP;
@@ -454,7 +461,8 @@ sdap_nested_group_split_members(TALLOC_CTX *mem_ctx,
         }
 
         /* check sysdb */
-        ret = sdap_nested_group_check_cache(group_ctx->domain, dn, &type);
+        ret = sdap_nested_group_check_cache(group_ctx->opts, group_ctx->domain,
+                                            dn, &type);
         if (ret == EOK) {
             /* found and valid */
             DEBUG(SSSDBG_TRACE_ALL, ("[%s] found in cache, skipping\n", dn));
-- 
1.8.3.1

-------------- next part --------------
>From e16c2720b28fa5efe0dea74a3b3c6294d4837147 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 25 Oct 2013 14:15:02 +0200
Subject: [PATCH 7/8] LDAP: Check all search bases during nested group
 processing

---
 src/providers/ldap/sdap_async_nested_groups.c | 55 ++++++++++++++++++++-------
 1 file changed, 42 insertions(+), 13 deletions(-)

diff --git a/src/providers/ldap/sdap_async_nested_groups.c b/src/providers/ldap/sdap_async_nested_groups.c
index fafbb36ba2f918787c3a862ef2098e11da5552da..01b3a002c374a32a48e749995ad58dc03196c6e6 100644
--- a/src/providers/ldap/sdap_async_nested_groups.c
+++ b/src/providers/ldap/sdap_async_nested_groups.c
@@ -392,6 +392,42 @@ done:
     return ret;
 }
 
+static bool
+sdap_nested_member_is_ent(struct sdap_nested_group_ctx *group_ctx,
+                          const char *dn, char **filter, bool is_user)
+{
+    struct sdap_domain *sditer = NULL;
+    bool ret;
+    struct sdap_search_base **search_bases;
+
+    DLIST_FOR_EACH(sditer, group_ctx->opts->sdom) {
+        search_bases = is_user ? sditer->user_search_bases : \
+                                 sditer->group_search_bases;
+
+        ret = sss_ldap_dn_in_search_bases(group_ctx, dn, search_bases,
+                                          filter);
+        if (ret == true) {
+            break;
+        }
+    }
+
+    return ret;
+}
+
+static inline bool
+sdap_nested_member_is_user(struct sdap_nested_group_ctx *group_ctx,
+                           const char *dn, char **filter)
+{
+    return sdap_nested_member_is_ent(group_ctx, dn, filter, true);
+}
+
+static inline bool
+sdap_nested_member_is_group(struct sdap_nested_group_ctx *group_ctx,
+                            const char *dn, char **filter)
+{
+    return sdap_nested_member_is_ent(group_ctx, dn, filter, false);
+}
+
 static errno_t
 sdap_nested_group_split_members(TALLOC_CTX *mem_ctx,
                                 struct sdap_nested_group_ctx *group_ctx,
@@ -475,13 +511,11 @@ sdap_nested_group_split_members(TALLOC_CTX *mem_ctx,
         /* try to determine type by dn */
         if (type == SDAP_NESTED_GROUP_DN_UNKNOWN) {
             /* user */
-            is_user = sss_ldap_dn_in_search_bases(tmp_ctx, dn,
-                           group_ctx->user_search_bases,
-                           &user_filter);
+            is_user = sdap_nested_member_is_user(group_ctx, dn,
+                                                 &user_filter);
 
-            is_group = sss_ldap_dn_in_search_bases(tmp_ctx, dn,
-                           group_ctx->group_search_bases,
-                           &group_filter);
+            is_group = sdap_nested_member_is_group(group_ctx, dn,
+                                                   &group_filter);
 
             if (is_user && is_group) {
                 /* search bases overlap */
@@ -2031,7 +2065,6 @@ sdap_nested_group_deref_direct_process(struct tevent_req *subreq)
     size_t num_entries = 0;
     size_t i, j;
     bool member_found;
-    bool bret;
     errno_t ret;
 
     req = tevent_req_callback_data(subreq, struct tevent_req);
@@ -2110,9 +2143,7 @@ sdap_nested_group_deref_direct_process(struct tevent_req *subreq)
             /* we found a user */
 
             /* skip the user if it is not amongst configured search bases */
-            bret = sss_ldap_dn_in_search_bases(state, orig_dn,
-                                               opts->sdom->user_search_bases, NULL);
-            if (!bret) {
+            if (!sdap_nested_member_is_user(state->group_ctx, orig_dn, NULL)) {
                 continue;
             }
 
@@ -2137,9 +2168,7 @@ sdap_nested_group_deref_direct_process(struct tevent_req *subreq)
             }
 
             /* skip the group if it is not amongst configured search bases */
-            bret = sss_ldap_dn_in_search_bases(state, orig_dn,
-                                               opts->sdom->group_search_bases, NULL);
-            if (!bret) {
+            if (!sdap_nested_member_is_group(state->group_ctx, orig_dn, NULL)) {
                 continue;
             }
 
-- 
1.8.3.1

-------------- next part --------------
>From f071448fa3355cd65a380c03b0301b48d8c3c1af Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 25 Oct 2013 18:23:04 +0200
Subject: [PATCH 8/8] NSS: Fix parenthesis

---
 src/responder/nss/nsssrv_cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index d37b4707cb734375011650632bca6d429042038c..b8db8dd1812a514c995aa553f810e843daa255bc 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -2233,7 +2233,7 @@ static int fill_members(struct sss_packet *packet,
             if (ret >= (name.len + delim + dom_len)) {
                 /* need more space,
                  * got creative with the print format ? */
-                int t = ret - name.len + delim + dom_len + 1;
+                int t = ret - (name.len + delim + dom_len) + 1;
                 ret = sss_packet_grow(packet, t);
                 if (ret != EOK) {
                     goto done;
-- 
1.8.3.1



More information about the sssd-devel mailing list