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

Jakub Hrozek jhrozek at redhat.com
Fri Oct 25 18:10:18 UTC 2013


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.
-------------- next part --------------
>From 36eec12c3e27197878e2bcee5f03fadf2fc00df7 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 | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/src/providers/ad/ad_id.c b/src/providers/ad/ad_id.c
index 20f9c23fa8d1d272f91c0b886bd1e2e3a2522320..eaec72887068b3cc3154cbd5a5a0f48d39eb5d82 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,61 @@ 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 (dp_opt_get_bool(idmap_ctx->id_ctx->opts->basic, SDAP_ID_MAPPING)) {
+        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 933da86d1fea3005c36a0b4b272b5f7e90d730f6 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 2/8] sdap: add sdap_domain_get_by_dn()

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

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

diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c
index b3a048c1cb7336cec0a5bbfebd3509c56637bed3..3a7751f0321df6c10bcb3ff6effa0a519eaa29ce 100644
--- a/src/providers/ldap/ldap_common.c
+++ b/src/providers/ldap/ldap_common.c
@@ -63,6 +63,35 @@ 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;
+    TALLOC_CTX *tmp_ctx = NULL;
+
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        return 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) ||
+            sss_ldap_dn_in_search_bases(tmp_ctx, dn, sditer->netgroup_search_bases, NULL) ||
+            sss_ldap_dn_in_search_bases(tmp_ctx, dn, sditer->sudo_search_bases, NULL) ||
+            sss_ldap_dn_in_search_bases(tmp_ctx, dn, sditer->service_search_bases, NULL) ||
+            sss_ldap_dn_in_search_bases(tmp_ctx, dn, sditer->autofs_search_bases, NULL)) {
+            break;
+        }
+    }
+
+    talloc_free(tmp_ctx);
+    return sditer;
+}
+
+
 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 fb9a34c6067521f8d28fb403b97ca6394918f303..b6a25cbdec67a6ee2d8700c0f2008a99b591510a 100644
--- a/src/providers/ldap/ldap_common.h
+++ b/src/providers/ldap/ldap_common.h
@@ -279,6 +279,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 5c5924051965e6c6930ae8f019b0b6fe5ab0ea9f 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 3/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 d4503d979ad2afd6d40d2b9bf9bdf17b83acf825 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 4/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 69681217b70bf1de133c8cab2ecb7d6453cad5ac 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 5/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 1b4b89c3bd34ceefce821c0107f9e33022df5393 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 6/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 26d4aec69d8d9ce0f43d63c056a7eabb217f17e2 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 7/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

-------------- next part --------------
>From 0503d2b416bd14f4d715986cf96097c92d950596 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 25 Oct 2013 16:52:10 +0200
Subject: [PATCH 8/8] NSS: Print FQDN for members of subdomain from the root
 domain

This patch is a workaround until
https://fedorahosted.org/sssd/ticket/2129 is fixed properly.

Consider a group entry such as:
 cn: subgroup at subdom
 ghost: someuser
 ghost: anotheruser at subdom

Currently in order to print all group members as FQDN (which is the default
for AD provider), the code needs to iterate over the ghost attributes and
parse them into (name,domain) and optionally re-add the domain..

The proper fix would be to store always just the FQDN in the hardcoded
form of user at domain
---
 src/responder/nss/nsssrv_cmd.c | 44 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 36 insertions(+), 8 deletions(-)

diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index b8db8dd1812a514c995aa553f810e843daa255bc..5f948c7b4952eb89dbe57f3f7b02da7bd5a59acf 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -2173,6 +2173,8 @@ static int fill_members(struct sss_packet *packet,
     size_t rzero= *_rzero;
     size_t rsize = *_rsize;
     char *tmpstr;
+    char *username;
+    char *domname;
     struct sized_string name;
     TALLOC_CTX *tmp_ctx = NULL;
 
@@ -2183,12 +2185,9 @@ static int fill_members(struct sss_packet *packet,
     size_t blen;
 
     const char *domain = dom->name;
-    bool add_domain = (!IS_SUBDOMAIN(dom) && dom->fqnames);
-
-    if (add_domain) {
-        delim = 1;
-        dom_len = sss_fqdom_len(dom->names, dom);
-    }
+    bool default_add_domain = (!IS_SUBDOMAIN(dom) && dom->fqnames);
+    bool add_domain;
+    struct sss_domain_info *member_dom;
 
     tmp_ctx = talloc_new(NULL);
     if (tmp_ctx == NULL) {
@@ -2218,8 +2217,34 @@ static int fill_members(struct sss_packet *packet,
             }
         }
 
+
+        add_domain = default_add_domain;
+        member_dom = dom;
+        delim = 0;
+        dom_len = 0;
         to_sized_string(&name, tmpstr);
 
+        if (IS_SUBDOMAIN(dom) == true) {
+            /* If the group is from a subdomain, but the user comes from the
+             * primary domain, the ghost or memberuid attribute will be just
+             * the name, so we need to add the domain component. This ugly
+             * hack is needed until #2129 is fixed.
+             */
+            ret = sss_parse_name(tmp_ctx, dom->names, tmpstr,
+                                 &domname, &username);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_MINOR_FAILURE, ("Could not parse [%s] into "
+                      "name-value components. Skipping\n", tmpstr));
+                continue;
+            }
+
+            if (domname == NULL) {
+                add_domain = true;
+                member_dom = dom->parent;
+                to_sized_string(&name, username);
+            }
+        }
+
         ret = sss_packet_grow(packet, name.len + delim + dom_len);
         if (ret != EOK) {
             goto done;
@@ -2227,9 +2252,12 @@ static int fill_members(struct sss_packet *packet,
         sss_packet_get_body(packet, &body, &blen);
 
         if (add_domain) {
+            delim = 1;
+            dom_len = sss_fqdom_len(member_dom->names, member_dom);
+
             ret = sss_fqname((char *)&body[rzero + rsize],
                              name.len + delim + dom_len,
-                             dom->names, dom, name.str);
+                             member_dom->names, member_dom, name.str);
             if (ret >= (name.len + delim + dom_len)) {
                 /* need more space,
                  * got creative with the print format ? */
@@ -2244,7 +2272,7 @@ static int fill_members(struct sss_packet *packet,
                 /* retry */
                 ret = sss_fqname((char *)&body[rzero + rsize],
                                 name.len + delim + dom_len,
-                                dom->names, dom, name.str);
+                                member_dom->names, member_dom, name.str);
             }
 
             if (ret != name.len + delim + dom_len - 1) {
-- 
1.8.3.1



More information about the sssd-devel mailing list