>From 20266af076cb55662ae71b6a7864ec6f3d2f0847 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek 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 | 3 + 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, 306 insertions(+), 20 deletions(-) diff --git a/Makefile.am b/Makefile.am index dc0670a5c720ab58a47e7da356578256b4659695..958d3bcad84b011efdcd3b0082e8ba95d15e7fb5 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 = \ @@ -2222,6 +2223,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 +2245,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