[SSSD] [PATCH] LDAP: Do not impose sizelimit=1 for single-entry searches

Jakub Hrozek jhrozek at redhat.com
Thu Sep 17 07:38:29 UTC 2015


On Thu, Sep 17, 2015 at 09:35:11AM +0200, Jakub Hrozek wrote:
> > After just only a month since the nack, attached are updated patches. I
> > would prefer to only apply the first one to downstream, since the others
> > are just optimizations and tests.
> 
> CI runs revealed the tests didn't link correctly on Debian. New patches
> are attached.

Ugh, those were from a wrong branch. Sorry about the spam.
-------------- next part --------------
>From 42e0c9459818d51fa203aa9449d69834c439007e Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 21 Jul 2015 21:00:27 +0200
Subject: [PATCH 1/4] LDAP: imposing sizelimit=1 for single-entry searches
 breaks overlapping domains

https://fedorahosted.org/sssd/ticket/2723

In case there are overlapping sdap domains, a search for a single user
might match and return multiple entries. For instance, with AD domains
represented by search bases:
    DC=win,DC=trust,DC=test
    DC=child,DC=win,DC=trust,DC=test

A search for user from win.trust.test would be based at:
    DC=win,DC=trust,DC=test
but would match both search bases and return both users.

Instead of performing complex filtering, just save both users. The
responder would select the entry that matches the user's search.
---
 src/providers/ldap/sdap_async_groups.c | 10 ----------
 src/providers/ldap/sdap_async_users.c  |  3 ---
 2 files changed, 13 deletions(-)

diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index 525c6fa09553d8c0232ce2317751184f83632d86..57a53af3f4eb46e6f31af9ee7c4d4625239d2a54 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -1874,8 +1874,6 @@ static errno_t sdap_get_groups_next_base(struct tevent_req *req)
 
     switch (state->lookup_type) {
     case SDAP_LOOKUP_SINGLE:
-        sizelimit = 1;
-        need_paging = false;
         break;
     /* Only requests that can return multiple entries should require
      * the paging control
@@ -1885,7 +1883,6 @@ static errno_t sdap_get_groups_next_base(struct tevent_req *req)
         need_paging = true;
         break;
     case SDAP_LOOKUP_ENUMERATE:
-        sizelimit = 0;  /* unlimited */
         need_paging = true;
         break;
     }
@@ -1934,13 +1931,6 @@ static void sdap_get_groups_process(struct tevent_req *subreq)
     DEBUG(SSSDBG_TRACE_FUNC,
           "Search for groups, returned %zu results.\n", count);
 
-    if (state->lookup_type == SDAP_LOOKUP_SINGLE && count > 1) {
-        DEBUG(SSSDBG_MINOR_FAILURE,
-              "Individual group search returned multiple results\n");
-        tevent_req_error(req, EINVAL);
-        return;
-    }
-
     if (state->lookup_type == SDAP_LOOKUP_WILDCARD || \
             state->lookup_type == SDAP_LOOKUP_ENUMERATE || \
         count == 0) {
diff --git a/src/providers/ldap/sdap_async_users.c b/src/providers/ldap/sdap_async_users.c
index a864a8b2187de7972aa963b355856e97f7c692a9..e38f4cd1610e62aa2cf9f4add3a5f7ad5290e748 100644
--- a/src/providers/ldap/sdap_async_users.c
+++ b/src/providers/ldap/sdap_async_users.c
@@ -692,8 +692,6 @@ static errno_t sdap_search_user_next_base(struct tevent_req *req)
 
     switch (state->lookup_type) {
     case SDAP_LOOKUP_SINGLE:
-        sizelimit = 1;
-        need_paging = false;
         break;
     /* Only requests that can return multiple entries should require
      * the paging control
@@ -703,7 +701,6 @@ static errno_t sdap_search_user_next_base(struct tevent_req *req)
         need_paging = true;
         break;
     case SDAP_LOOKUP_ENUMERATE:
-        sizelimit = 0;  /* unlimited */
         need_paging = true;
         break;
     }
-- 
2.4.3

-------------- next part --------------
>From 9fa269ee45fb37b2c8946668aaf07a4343ce654f Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Wed, 16 Sep 2015 15:31:02 +0200
Subject: [PATCH 2/4] tests: Move named_domain from test_utils to common test
 code

This handy function should be reused by other parts of the code.
---
 src/tests/cmocka/test_utils.c | 17 -----------------
 src/tests/common.h            |  4 ++++
 src/tests/common_dom.c        | 22 ++++++++++++++++++++++
 3 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/src/tests/cmocka/test_utils.c b/src/tests/cmocka/test_utils.c
index c7ebe0997ec00197e8852bedbcf26ef1f6394fc3..6a10be31ccc89532d59b422ccec9e439b61a40ba 100644
--- a/src/tests/cmocka/test_utils.c
+++ b/src/tests/cmocka/test_utils.c
@@ -454,23 +454,6 @@ void test_find_domain_by_sid_disabled(void **state)
     }
 }
 
-static struct sss_domain_info *named_domain(TALLOC_CTX *mem_ctx,
-                                            const char *name,
-                                            struct sss_domain_info *parent)
-{
-    struct sss_domain_info *dom = NULL;
-
-    dom = talloc_zero(mem_ctx, struct sss_domain_info);
-    assert_non_null(dom);
-
-    dom->name = talloc_strdup(dom, name);
-    assert_non_null(dom->name);
-
-    dom->parent = parent;
-
-    return dom;
-}
-
 /*
  * dom1 -> sub1a
  *  |
diff --git a/src/tests/common.h b/src/tests/common.h
index 1c6de2c3d6eb924ba7306bd350f8546d61f30751..c0ceebfbf5d9277a422065fbe4ddfdd65c758de8 100644
--- a/src/tests/common.h
+++ b/src/tests/common.h
@@ -134,4 +134,8 @@ test_dbus_call_sync(DBusConnection *conn,
                     int first_arg_type,
                     ...);
 
+struct sss_domain_info *named_domain(TALLOC_CTX *mem_ctx,
+                                     const char *name,
+                                     struct sss_domain_info *parent);
+
 #endif /* !__TESTS_COMMON_H__ */
diff --git a/src/tests/common_dom.c b/src/tests/common_dom.c
index 41f876555352744c4f61215c1a88369918f5192c..5b87e7a3a0831619d654e2a5bd50bb6b9e03a8e6 100644
--- a/src/tests/common_dom.c
+++ b/src/tests/common_dom.c
@@ -384,3 +384,25 @@ void test_dom_suite_cleanup(const char *tests_path,
 
     test_multidom_suite_cleanup(tests_path, cdb_file, domains);
 }
+
+struct sss_domain_info *named_domain(TALLOC_CTX *mem_ctx,
+                                     const char *name,
+                                     struct sss_domain_info *parent)
+{
+    struct sss_domain_info *dom = NULL;
+
+    dom = talloc_zero(mem_ctx, struct sss_domain_info);
+    if (dom == NULL) {
+        return NULL;
+    }
+
+    dom->name = talloc_strdup(dom, name);
+    if (dom->name == NULL) {
+        talloc_free(dom);
+        return NULL;
+    }
+
+    dom->parent = parent;
+
+    return dom;
+}
-- 
2.4.3

-------------- next part --------------
>From e07541b9b61778fcd3377388fe105de15898cb16 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Wed, 16 Sep 2015 15:28:54 +0200
Subject: [PATCH 3/4] LDAP: Move sdap_create_search_base from ldap to sdap code

The function shouldn't be placed in the LDAP tree, but in the SDAP tree
to make it usable from tests without linking to libraries that are
normally linked from LDAP provider (such as confdb)
---
 src/providers/ldap/ldap_common.h  |  7 -----
 src/providers/ldap/ldap_options.c | 63 ---------------------------------------
 src/providers/ldap/sdap.c         | 63 +++++++++++++++++++++++++++++++++++++++
 src/providers/ldap/sdap.h         |  7 +++++
 4 files changed, 70 insertions(+), 70 deletions(-)

diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h
index 716aaa08ef83b8fca3779b830fdef20bbdb14937..f552520a0503908f82b845f8e813cf67306ec954 100644
--- a/src/providers/ldap/ldap_common.h
+++ b/src/providers/ldap/ldap_common.h
@@ -294,13 +294,6 @@ struct sdap_domain *sdap_domain_get(struct sdap_options *opts,
 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,
-                        int scope,
-                        const char *filter,
-                        struct sdap_search_base **_base);
-
 errno_t sdap_parse_search_base(TALLOC_CTX *mem_ctx,
                                struct dp_option *opts, int class,
                                struct sdap_search_base ***_search_bases);
diff --git a/src/providers/ldap/ldap_options.c b/src/providers/ldap/ldap_options.c
index eb00aab32a55c6841ebbf32d30f3a48722e8b165..7ad6071508d0abbb33984c697b833cf12f9e4df9 100644
--- a/src/providers/ldap/ldap_options.c
+++ b/src/providers/ldap/ldap_options.c
@@ -532,69 +532,6 @@ errno_t sdap_parse_search_base(TALLOC_CTX *mem_ctx,
                                     _search_bases);
 }
 
-errno_t
-sdap_create_search_base(TALLOC_CTX *mem_ctx,
-                        const char *unparsed_base,
-                        int scope,
-                        const char *filter,
-                        struct sdap_search_base **_base)
-{
-    struct sdap_search_base *base;
-    TALLOC_CTX *tmp_ctx;
-    errno_t ret;
-    struct ldb_dn *ldn;
-    struct ldb_context *ldb;
-
-    tmp_ctx = talloc_new(NULL);
-    if (!tmp_ctx) {
-        ret = ENOMEM;
-        goto done;
-    }
-
-    /* Create a throwaway LDB context for validating the DN */
-    ldb = ldb_init(tmp_ctx, NULL);
-    if (!ldb) {
-        ret = ENOMEM;
-        goto done;
-    }
-
-    base = talloc_zero(tmp_ctx, struct sdap_search_base);
-    if (base == NULL) {
-        ret = ENOMEM;
-        goto done;
-    }
-
-    base->basedn = talloc_strdup(base, unparsed_base);
-    if (base->basedn == NULL) {
-        ret = ENOMEM;
-        goto done;
-    }
-
-    /* Validate the basedn */
-    ldn = ldb_dn_new(tmp_ctx, ldb, unparsed_base);
-    if (!ldn) {
-        ret = ENOMEM;
-        goto done;
-    }
-
-    if (!ldb_dn_validate(ldn)) {
-        DEBUG(SSSDBG_CRIT_FAILURE,
-                "Invalid base DN [%s]\n",
-                 unparsed_base);
-        ret = EINVAL;
-        goto done;
-    }
-
-    base->scope = scope;
-    base->filter = filter;
-
-    *_base = talloc_steal(mem_ctx, base);
-    ret = EOK;
-done:
-    talloc_free(tmp_ctx);
-    return ret;
-}
-
 errno_t common_parse_search_base(TALLOC_CTX *mem_ctx,
                                  const char *unparsed_base,
                                  const char *class_name,
diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
index 97bc14b8730565b6d0d2c41e51f910f042357bc3..ca03bde67eded5955e865194dc41252941e0a8e5 100644
--- a/src/providers/ldap/sdap.c
+++ b/src/providers/ldap/sdap.c
@@ -1031,6 +1031,69 @@ static char *get_naming_context(TALLOC_CTX *mem_ctx,
     return naming_context;
 }
 
+errno_t
+sdap_create_search_base(TALLOC_CTX *mem_ctx,
+                        const char *unparsed_base,
+                        int scope,
+                        const char *filter,
+                        struct sdap_search_base **_base)
+{
+    struct sdap_search_base *base;
+    TALLOC_CTX *tmp_ctx;
+    errno_t ret;
+    struct ldb_dn *ldn;
+    struct ldb_context *ldb;
+
+    tmp_ctx = talloc_new(NULL);
+    if (!tmp_ctx) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    /* Create a throwaway LDB context for validating the DN */
+    ldb = ldb_init(tmp_ctx, NULL);
+    if (!ldb) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    base = talloc_zero(tmp_ctx, struct sdap_search_base);
+    if (base == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    base->basedn = talloc_strdup(base, unparsed_base);
+    if (base->basedn == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    /* Validate the basedn */
+    ldn = ldb_dn_new(tmp_ctx, ldb, unparsed_base);
+    if (!ldn) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    if (!ldb_dn_validate(ldn)) {
+        DEBUG(SSSDBG_CRIT_FAILURE,
+                "Invalid base DN [%s]\n",
+                 unparsed_base);
+        ret = EINVAL;
+        goto done;
+    }
+
+    base->scope = scope;
+    base->filter = filter;
+
+    *_base = talloc_steal(mem_ctx, base);
+    ret = EOK;
+done:
+    talloc_free(tmp_ctx);
+    return ret;
+}
+
 static errno_t sdap_set_search_base(struct sdap_options *opts,
                                     struct sdap_domain *sdom,
                                     enum sdap_basic_opt class,
diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
index b3321be48e1b24124d59fbfe88096a2109c920a6..0dc6f751a116f4e32579646e4d9a85470cb9d686 100644
--- a/src/providers/ldap/sdap.h
+++ b/src/providers/ldap/sdap.h
@@ -373,6 +373,13 @@ struct sdap_search_base {
     const char *filter;
 };
 
+errno_t
+sdap_create_search_base(TALLOC_CTX *mem_ctx,
+                        const char *unparsed_base,
+                        int scope,
+                        const char *filter,
+                        struct sdap_search_base **_base);
+
 /* Values from
  * http://msdn.microsoft.com/en-us/library/cc223272%28v=prot.13%29.aspx
  */
-- 
2.4.3

-------------- next part --------------
>From 07b62ed6210f25a6b826896c97c8e75e9a6a9867 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 4 Sep 2015 18:45:45 +0200
Subject: [PATCH 4/4] LDAP: Filter out multiple entries when searching
 overlapping domains

In case domain overlap, we might download multiple objects. To avoid
saving them all, we attempt to filter out the objects from foreign
domains.

We can only do this optimization for non-wildcard lookups.
---
 Makefile.am                                |   4 +
 src/providers/ldap/sdap.c                  |  59 +++++++++
 src/providers/ldap/sdap.h                  |   9 ++
 src/providers/ldap/sdap_async_groups.c     |  33 +++--
 src/providers/ldap/sdap_async_initgroups.c |   1 +
 src/providers/ldap/sdap_async_users.c      |  35 ++++--
 src/tests/cmocka/test_sdap.c               | 186 +++++++++++++++++++++++++++++
 7 files changed, 307 insertions(+), 20 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index dc0670a5c720ab58a47e7da356578256b4659695..3e69e145975349bc5e3e7b5180e2cd68dc537081 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1631,6 +1631,7 @@ ipa_ldap_opt_tests_SOURCES = \
     src/providers/data_provider_opts.c \
     src/providers/ldap/sdap.c \
     src/providers/ldap/sdap_range.c \
+    src/providers/ldap/sdap_domain.c \
     src/util/sss_ldap.c \
     src/tests/ipa_ldap_opt-tests.c
 ipa_ldap_opt_tests_CFLAGS = \
@@ -1639,6 +1640,7 @@ ipa_ldap_opt_tests_CFLAGS = \
 ipa_ldap_opt_tests_LDADD = \
     $(CHECK_LIBS) \
     $(TALLOC_LIBS) \
+    $(LDB_LIBS) \
     $(SSSD_INTERNAL_LTLIBS) \
     $(OPENLDAP_LIBS) \
     libsss_test_common.la
@@ -2222,6 +2224,7 @@ dp_opt_tests_LDADD = \
 
 sdap_tests_SOURCES = \
     src/providers/data_provider_opts.c \
+    src/providers/ldap/sdap_domain.c \
     src/providers/ldap/sdap.c \
     src/providers/ldap/sdap_range.c \
     src/util/sss_ldap.c \
@@ -2243,6 +2246,7 @@ sdap_tests_LDFLAGS = \
 sdap_tests_LDADD = \
     $(CMOCKA_LIBS) \
     $(TALLOC_LIBS) \
+    $(LDB_LIBS) \
     $(POPT_LIBS) \
     $(SSSD_INTERNAL_LTLIBS) \
     $(SSS_CRYPT_LIBS) \
diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
index ca03bde67eded5955e865194dc41252941e0a8e5..b007347c6d2de5e8d82ddad1874f4d0209ce31f8 100644
--- a/src/providers/ldap/sdap.c
+++ b/src/providers/ldap/sdap.c
@@ -1621,3 +1621,62 @@ char *sdap_make_oc_list(TALLOC_CTX *mem_ctx, struct sdap_attr_map *map)
                                map[SDAP_OC_GROUP_ALT].name);
     }
 }
+
+static bool sdap_object_in_domain(struct sdap_options *opts,
+                                  struct sysdb_attrs *obj,
+                                  struct sss_domain_info *dom)
+{
+    errno_t ret;
+    const char *original_dn = NULL;
+    struct sdap_domain *sdmatch = NULL;
+
+    ret = sysdb_attrs_get_string(obj, SYSDB_ORIG_DN, &original_dn);
+    if (ret) {
+        DEBUG(SSSDBG_FUNC_DATA,
+              "The group has no original DN, assuming our domain\n");
+        return true;
+    }
+
+    sdmatch = sdap_domain_get_by_dn(opts, original_dn);
+    if (sdmatch == NULL) {
+        DEBUG(SSSDBG_FUNC_DATA,
+              "The group has no original DN, assuming our domain\n");
+        return true;
+    }
+
+    return (sdmatch->dom == dom);
+}
+
+size_t sdap_copy_objects_in_dom(struct sdap_options *opts,
+                                struct sysdb_attrs **dom_objects,
+                                size_t offset,
+                                struct sss_domain_info *dom,
+                                struct sysdb_attrs **all_objects,
+                                size_t count,
+                                bool filter)
+{
+    size_t copied = 0;
+
+    /* Own objects from all_objects by dom_objects in case they belong
+     * to domain dom.
+     *
+     * Don't copy objects from other domains in case
+     * the search was for parent domain but a child domain would match,
+     * too, such as:
+     *  dc=example,dc=com
+     *  dc=child,dc=example,dc=com
+     * while searching for an object from dc=example.
+     */
+    for (size_t i = 0; i < count; i++) {
+        if (filter &&
+                sdap_object_in_domain(opts, all_objects[i], dom) == false) {
+            continue;
+        }
+
+        dom_objects[offset + copied] =
+            talloc_steal(dom_objects, all_objects[i]);
+        copied++;
+    }
+
+    return copied;
+}
diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
index 0dc6f751a116f4e32579646e4d9a85470cb9d686..e15c6f2002a3f62e019a290e49e6465bfa460b9f 100644
--- a/src/providers/ldap/sdap.h
+++ b/src/providers/ldap/sdap.h
@@ -580,4 +580,13 @@ void sdap_steal_server_opts(struct sdap_id_ctx *id_ctx,
                             struct sdap_server_opts **srv_opts);
 
 char *sdap_make_oc_list(TALLOC_CTX *mem_ctx, struct sdap_attr_map *map);
+
+size_t sdap_copy_objects_in_dom(struct sdap_options *opts,
+                                struct sysdb_attrs **dom_objects,
+                                size_t offset,
+                                struct sss_domain_info *dom,
+                                struct sysdb_attrs **all_objects,
+                                size_t count,
+                                bool filter);
+
 #endif /* _SDAP_H_ */
diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index 57a53af3f4eb46e6f31af9ee7c4d4625239d2a54..ecadf147c3c85a35db7b19788f3e0171e1ea030f 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -1905,6 +1905,9 @@ static errno_t sdap_get_groups_next_base(struct tevent_req *req)
 }
 
 static void sdap_nested_done(struct tevent_req *req);
+static void sdap_search_group_copy_batch(struct sdap_get_groups_state *state,
+                                         struct sysdb_attrs **groups,
+                                         size_t count);
 static void sdap_ad_match_rule_members_process(struct tevent_req *subreq);
 
 static void sdap_get_groups_process(struct tevent_req *subreq)
@@ -1950,15 +1953,7 @@ static void sdap_get_groups_process(struct tevent_req *subreq)
             return;
         }
 
-        /* Copy the new groups into the list
-         */
-        for (i = 0; i < count; i++) {
-            state->groups[state->count + i] =
-                talloc_steal(state->groups, groups[i]);
-        }
-
-        state->count += count;
-        state->groups[state->count] = NULL;
+        sdap_search_group_copy_batch(state, groups, count);
     }
 
     if (next_base) {
@@ -2093,6 +2088,26 @@ static void sdap_get_groups_process(struct tevent_req *subreq)
     }
 }
 
+static void sdap_search_group_copy_batch(struct sdap_get_groups_state *state,
+                                         struct sysdb_attrs **groups,
+                                         size_t count)
+{
+    size_t copied;
+    bool filter;
+
+    /* Always copy all objects for wildcard lookups. */
+    filter = state->lookup_type == SDAP_LOOKUP_SINGLE ? true : false;
+
+    copied = sdap_copy_objects_in_dom(state->opts,
+                                      state->groups,
+                                      state->count,
+                                      state->dom,
+                                      groups, count, filter);
+
+    state->count += copied;
+    state->groups[state->count] = NULL;
+}
+
 static void sdap_get_groups_done(struct tevent_req *subreq)
 {
     struct tevent_req *req =
diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c
index ffb8f7e1f5db5564f45c4f5b7aff46139e64537d..ed0d029f03e5299420ce98423c5abc2fee9b48de 100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -2900,6 +2900,7 @@ static void sdap_get_initgr_user(struct tevent_req *subreq)
         expected_basedn_len = strlen(expected_basedn);
 
         for (c = 0; c < count; c++) {
+            /* FIXME - convert to the new stuff?? */
             ret = sysdb_attrs_get_string(usr_attrs[c], SYSDB_ORIG_DN, &orig_dn);
             if (ret != EOK) {
                 DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_string failed.\n");
diff --git a/src/providers/ldap/sdap_async_users.c b/src/providers/ldap/sdap_async_users.c
index e38f4cd1610e62aa2cf9f4add3a5f7ad5290e748..1d8008731fb2cc8252e3f18d83e05e79c6b3e9bd 100644
--- a/src/providers/ldap/sdap_async_users.c
+++ b/src/providers/ldap/sdap_async_users.c
@@ -617,6 +617,9 @@ struct sdap_search_user_state {
 };
 
 static errno_t sdap_search_user_next_base(struct tevent_req *req);
+static void sdap_search_user_copy_batch(struct sdap_search_user_state *state,
+                                        struct sysdb_attrs **users,
+                                        size_t count);
 static void sdap_search_user_process(struct tevent_req *subreq);
 
 struct tevent_req *sdap_search_user_send(TALLOC_CTX *memctx,
@@ -728,7 +731,7 @@ static void sdap_search_user_process(struct tevent_req *subreq)
     struct sdap_search_user_state *state = tevent_req_data(req,
                                             struct sdap_search_user_state);
     int ret;
-    size_t count, i;
+    size_t count;
     struct sysdb_attrs **users;
     bool next_base = false;
 
@@ -762,16 +765,7 @@ static void sdap_search_user_process(struct tevent_req *subreq)
             return;
         }
 
-        /* Copy the new users into the list
-         * They're already allocated on 'state'
-         */
-        for (i = 0; i < count; i++) {
-            state->users[state->count + i] =
-                talloc_steal(state->users, users[i]);
-        }
-
-        state->count += count;
-        state->users[state->count] = NULL;
+        sdap_search_user_copy_batch(state, users, count);
     }
 
     if (next_base) {
@@ -798,6 +792,25 @@ static void sdap_search_user_process(struct tevent_req *subreq)
     tevent_req_done(req);
 }
 
+static void sdap_search_user_copy_batch(struct sdap_search_user_state *state,
+                                        struct sysdb_attrs **users,
+                                        size_t count)
+{
+    size_t copied;
+    bool filter;
+
+    /* Always copy all objects for wildcard lookups. */
+    filter = state->lookup_type == SDAP_LOOKUP_SINGLE ? true : false;
+
+    copied = sdap_copy_objects_in_dom(state->opts,
+                                      state->users,
+                                      state->count,
+                                      state->dom,
+                                      users, count, filter);
+
+    state->count += copied;
+    state->users[state->count] = NULL;
+}
 
 int sdap_search_user_recv(TALLOC_CTX *memctx, struct tevent_req *req,
                           char **higher_usn, struct sysdb_attrs ***users,
diff --git a/src/tests/cmocka/test_sdap.c b/src/tests/cmocka/test_sdap.c
index 75fc34504bc35e4e4a5fc11bc9d645e78bc2da72..2e11085df29769f970573ec47351951430cec1a8 100644
--- a/src/tests/cmocka/test_sdap.c
+++ b/src/tests/cmocka/test_sdap.c
@@ -941,6 +941,184 @@ static void test_sdap_inherit_option_user(void **state)
     talloc_free(test_ctx->child_sdap_opts->user_map[SDAP_AT_USER_PRINC].name);
 }
 
+struct copy_dom_obj_test_ctx {
+    struct sdap_options *opts;
+
+    struct sss_domain_info *parent;
+    struct sss_domain_info *child;
+
+    struct sdap_domain *parent_sd;
+    struct sdap_domain *child_sd;
+
+    struct sysdb_attrs **ldap_objects;
+    struct sysdb_attrs **dom_objects;
+};
+
+static struct sysdb_attrs *test_obj(TALLOC_CTX *mem_ctx,
+                                    const char *name,
+                                    const char *basedn)
+{
+    errno_t ret;
+    const char *orig_dn;
+    struct sysdb_attrs *obj;
+
+    obj = sysdb_new_attrs(mem_ctx);
+    assert_non_null(obj);
+
+    orig_dn = talloc_asprintf(obj, "CN=%s,%s", name, basedn);
+    assert_non_null(orig_dn);
+
+    ret = sysdb_attrs_add_string(obj, SYSDB_ORIG_DN, orig_dn);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_attrs_add_string(obj, SYSDB_NAME, name);
+    assert_int_equal(ret, EOK);
+
+    return obj;
+}
+
+static struct sdap_domain *create_sdap_domain(struct sdap_options *opts,
+                                              struct sss_domain_info *dom)
+{
+    errno_t ret;
+    struct sdap_domain *sdom;
+
+    ret = sdap_domain_add(opts, dom, &sdom);
+    assert_int_equal(ret, EOK);
+
+    sdom->search_bases = talloc_array(sdom, struct sdap_search_base *, 2);
+    assert_non_null(sdom->search_bases);
+    sdom->search_bases[1] = NULL;
+
+    ret = sdap_create_search_base(sdom, sdom->basedn,
+                                  LDAP_SCOPE_SUBTREE,
+                                  NULL,
+                                  &sdom->search_bases[0]);
+    assert_int_equal(ret, EOK);
+
+    return sdom;
+}
+
+static int sdap_copy_objects_in_dom_setup(void **state)
+{
+    struct copy_dom_obj_test_ctx *test_ctx;
+
+    test_ctx = talloc_zero(NULL,
+                           struct copy_dom_obj_test_ctx);
+    assert_non_null(test_ctx);
+
+    test_ctx->opts = talloc_zero(test_ctx, struct sdap_options);
+    assert_non_null(test_ctx->opts);
+
+    test_ctx->parent = named_domain(test_ctx, "win.trust.test", NULL);
+    assert_non_null(test_ctx->parent);
+
+    test_ctx->child = named_domain(test_ctx, "child.win.trust.test",
+                                   test_ctx->parent);
+    assert_non_null(test_ctx->child);
+
+    test_ctx->parent_sd = create_sdap_domain(test_ctx->opts,
+                                             test_ctx->parent);
+    assert_non_null(test_ctx->parent_sd);
+
+    test_ctx->child_sd = create_sdap_domain(test_ctx->opts,
+                                             test_ctx->child);
+    assert_non_null(test_ctx->child_sd);
+
+    /* These two objects were 'returned by LDAP' */
+    test_ctx->ldap_objects = talloc_zero_array(test_ctx,
+                                               struct sysdb_attrs *, 2);
+    assert_non_null(test_ctx->ldap_objects);
+
+    test_ctx->ldap_objects[0] = test_obj(test_ctx->ldap_objects, "parent",
+                                         test_ctx->parent_sd->basedn);
+    assert_non_null(test_ctx->ldap_objects[0]);
+
+    test_ctx->ldap_objects[1] = test_obj(test_ctx->ldap_objects, "child",
+                                         test_ctx->child_sd->basedn);
+    assert_non_null(test_ctx->ldap_objects[1]);
+
+    /* This is the array we'll filter to */
+    test_ctx->dom_objects = talloc_zero_array(test_ctx,
+                                              struct sysdb_attrs *, 2);
+    assert_non_null(test_ctx->dom_objects);
+
+    *state = test_ctx;
+    return 0;
+}
+
+static int sdap_copy_objects_in_dom_teardown(void **state)
+{
+    struct copy_dom_obj_test_ctx *test_ctx = talloc_get_type_abort(*state,
+                                                 struct copy_dom_obj_test_ctx);
+
+    talloc_free(test_ctx);
+    return 0;
+}
+
+static void test_sdap_copy_objects_in_dom(void **state)
+{
+    struct copy_dom_obj_test_ctx *test_ctx = talloc_get_type_abort(*state,
+                                                 struct copy_dom_obj_test_ctx);
+    size_t count;
+
+    assert_true(talloc_parent(test_ctx->ldap_objects[0]) == \
+                test_ctx->ldap_objects);
+    assert_true(talloc_parent(test_ctx->ldap_objects[1]) == \
+                test_ctx->ldap_objects);
+
+    assert_null(test_ctx->dom_objects[0]);
+    assert_null(test_ctx->dom_objects[1]);
+
+    count = sdap_copy_objects_in_dom(test_ctx->opts,
+                                     test_ctx->dom_objects,
+                                     0,
+                                     test_ctx->parent,
+                                     test_ctx->ldap_objects,
+                                     2, true);
+    assert_int_equal(count, 1);
+
+    assert_non_null(test_ctx->dom_objects[0]);
+    assert_non_null(test_ctx->dom_objects[0] == test_ctx->ldap_objects[0]);
+    assert_null(test_ctx->dom_objects[1]);
+
+    assert_true(talloc_parent(test_ctx->ldap_objects[0]) == \
+                test_ctx->dom_objects);
+
+    count = sdap_copy_objects_in_dom(test_ctx->opts,
+                                     test_ctx->dom_objects,
+                                     1,
+                                     test_ctx->child,
+                                     test_ctx->ldap_objects,
+                                     2, true);
+    assert_int_equal(count, 1);
+
+    assert_non_null(test_ctx->dom_objects[1]);
+    assert_non_null(test_ctx->dom_objects[1] == test_ctx->ldap_objects[1]);
+    assert_true(talloc_parent(test_ctx->ldap_objects[1]) == \
+                test_ctx->dom_objects);
+}
+
+static void test_sdap_copy_objects_in_dom_nofilter(void **state)
+{
+    struct copy_dom_obj_test_ctx *test_ctx = talloc_get_type_abort(*state,
+                                                 struct copy_dom_obj_test_ctx);
+    size_t count;
+
+    count = sdap_copy_objects_in_dom(test_ctx->opts,
+                                     test_ctx->dom_objects,
+                                     0,
+                                     test_ctx->parent,
+                                     test_ctx->ldap_objects,
+                                     2, false);
+    assert_int_equal(count, 2);
+
+    assert_true(talloc_parent(test_ctx->ldap_objects[0]) == \
+                test_ctx->dom_objects);
+    assert_true(talloc_parent(test_ctx->ldap_objects[1]) == \
+                test_ctx->dom_objects);
+}
+
 int main(int argc, const char *argv[])
 {
     poptContext pc;
@@ -1008,6 +1186,14 @@ int main(int argc, const char *argv[])
         cmocka_unit_test_setup_teardown(test_sdap_inherit_option_user,
                                         test_sdap_inherit_option_setup,
                                         test_sdap_inherit_option_teardown),
+
+        /* Per-domain object filter tests */
+        cmocka_unit_test_setup_teardown(test_sdap_copy_objects_in_dom,
+                                        sdap_copy_objects_in_dom_setup,
+                                        sdap_copy_objects_in_dom_teardown),
+        cmocka_unit_test_setup_teardown(test_sdap_copy_objects_in_dom_nofilter,
+                                        sdap_copy_objects_in_dom_setup,
+                                        sdap_copy_objects_in_dom_teardown),
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */
-- 
2.4.3



More information about the sssd-devel mailing list