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.

Reviewed-by: Lukáš Slebodník <lslebodn@redhat.com>

6 files changed, 306 insertions, 20 deletions

diff --git a/Makefile.am b/Makefile.am
index 583ba9440..2d704fd39 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1634,6 +1634,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 = \
@@ -1642,6 +1643,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
@@ -2225,6 +2227,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 \
@@ -2246,6 +2249,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 5aa7ff7ca..fcdc4028e 100644
--- a/src/providers/ldap/sdap.c
+++ b/src/providers/ldap/sdap.c
@@ -1619,3 +1619,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_steal_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 0dc6f751a..edfbf229b 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_steal_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 57a53af3f..653187b3a 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_steal_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_users.c b/src/providers/ldap/sdap_async_users.c
index e38f4cd16..865439cad 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_steal_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 75fc34504..86c940c89 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_ptr_equal(talloc_parent(test_ctx->ldap_objects[0]),
+                     test_ctx->ldap_objects);
+    assert_ptr_equal(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_steal_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_ptr_equal(talloc_parent(test_ctx->ldap_objects[0]),
+                     test_ctx->dom_objects);
+
+    count = sdap_steal_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_ptr_equal(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_steal_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_ptr_equal(talloc_parent(test_ctx->ldap_objects[0]),
+                     test_ctx->dom_objects);
+    assert_ptr_equal(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. */