From 1ebdd37552bac2ce05519efaf932946f79640a04 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Thu, 7 Jul 2016 18:54:02 +0200 Subject: [PATCH 4/4] views: properly override group member names Resolves https://fedorahosted.org/sssd/ticket/2948 --- src/db/sysdb.h | 3 +- src/db/sysdb_search.c | 99 ++++++++++++++++------------- src/db/sysdb_views.c | 136 ++++++++++++++++++---------------------- src/responder/nss/nsssrv_cmd.c | 7 ++- src/tests/cmocka/test_nss_srv.c | 26 ++++---- 5 files changed, 138 insertions(+), 133 deletions(-) diff --git a/src/db/sysdb.h b/src/db/sysdb.h index 489cb4d1b1af81711d93ed1eb0cb734955c3f8c0..e10cf1bb26d31a0bb598a85ac38a931017c52600 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -575,7 +575,8 @@ errno_t sysdb_add_overrides_to_object(struct sss_domain_info *domain, const char **req_attrs); errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain, - struct ldb_message *obj); + struct ldb_message *obj, + bool expect_override_dn); errno_t sysdb_getpwnam_with_views(TALLOC_CTX *mem_ctx, struct sss_domain_info *domain, diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c index e40b36c38e28992e185447497d1bf69cabc09821..cfee5784dbadd692f30d0758e7e5c3c9fb2814cb 100644 --- a/src/db/sysdb_search.c +++ b/src/db/sysdb_search.c @@ -771,28 +771,33 @@ int sysdb_getgrnam_with_views(TALLOC_CTX *mem_ctx, /* If there are views we have to check if override values must be added to * the original object. */ - if (DOM_HAS_VIEWS(domain) && orig_obj->count == 1) { - if (!is_local_view(domain->view_name)) { - el = ldb_msg_find_element(orig_obj->msgs[0], SYSDB_GHOST); - if (el != NULL && el->num_values != 0) { - DEBUG(SSSDBG_TRACE_ALL, "Group object [%s], contains ghost " - "entries which must be resolved before overrides can be " - "applied.\n", - ldb_dn_get_linearized(orig_obj->msgs[0]->dn)); - ret = ENOENT; + if (orig_obj->count == 1) { + if (DOM_HAS_VIEWS(domain)) { + if (!is_local_view(domain->view_name)) { + el = ldb_msg_find_element(orig_obj->msgs[0], SYSDB_GHOST); + if (el != NULL && el->num_values != 0) { + DEBUG(SSSDBG_TRACE_ALL, "Group object [%s], contains ghost " + "entries which must be resolved before overrides can be " + "applied.\n", + ldb_dn_get_linearized(orig_obj->msgs[0]->dn)); + ret = ENOENT; + goto done; + } + } + + ret = sysdb_add_overrides_to_object(domain, orig_obj->msgs[0], + override_obj == NULL ? NULL : override_obj ->msgs[0], + NULL); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "sysdb_add_overrides_to_object failed.\n"); goto done; } } - ret = sysdb_add_overrides_to_object(domain, orig_obj->msgs[0], - override_obj == NULL ? NULL : override_obj ->msgs[0], - NULL); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, "sysdb_add_overrides_to_object failed.\n"); - goto done; - } - - ret = sysdb_add_group_member_overrides(domain, orig_obj->msgs[0]); + /* Must be called even without views to check to + * SYSDB_DEFAULT_OVERRIDE_NAME */ + ret = sysdb_add_group_member_overrides(domain, orig_obj->msgs[0], + DOM_HAS_VIEWS(domain)); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "sysdb_add_group_member_overrides failed.\n"); @@ -922,28 +927,33 @@ int sysdb_getgrgid_with_views(TALLOC_CTX *mem_ctx, /* If there are views we have to check if override values must be added to * the original object. */ - if (DOM_HAS_VIEWS(domain) && orig_obj->count == 1) { - if (!is_local_view(domain->view_name)) { - el = ldb_msg_find_element(orig_obj->msgs[0], SYSDB_GHOST); - if (el != NULL && el->num_values != 0) { - DEBUG(SSSDBG_TRACE_ALL, "Group object [%s], contains ghost " - "entries which must be resolved before overrides can be " - "applied.\n", - ldb_dn_get_linearized(orig_obj->msgs[0]->dn)); - ret = ENOENT; + if (orig_obj->count == 1) { + if (DOM_HAS_VIEWS(domain)) { + if (!is_local_view(domain->view_name)) { + el = ldb_msg_find_element(orig_obj->msgs[0], SYSDB_GHOST); + if (el != NULL && el->num_values != 0) { + DEBUG(SSSDBG_TRACE_ALL, "Group object [%s], contains ghost " + "entries which must be resolved before overrides can be " + "applied.\n", + ldb_dn_get_linearized(orig_obj->msgs[0]->dn)); + ret = ENOENT; + goto done; + } + } + + ret = sysdb_add_overrides_to_object(domain, orig_obj->msgs[0], + override_obj == NULL ? NULL : override_obj ->msgs[0], + NULL); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "sysdb_add_overrides_to_object failed.\n"); goto done; } } - ret = sysdb_add_overrides_to_object(domain, orig_obj->msgs[0], - override_obj == NULL ? NULL : override_obj ->msgs[0], - NULL); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, "sysdb_add_overrides_to_object failed.\n"); - goto done; - } - - ret = sysdb_add_group_member_overrides(domain, orig_obj->msgs[0]); + /* Must be called even without views to check to + * SYSDB_DEFAULT_OVERRIDE_NAME */ + ret = sysdb_add_group_member_overrides(domain, orig_obj->msgs[0], + DOM_HAS_VIEWS(domain)); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "sysdb_add_group_member_overrides failed.\n"); @@ -1157,8 +1167,8 @@ int sysdb_enumgrent_filter_with_views(TALLOC_CTX *mem_ctx, goto done; } - if (DOM_HAS_VIEWS(domain)) { - for (c = 0; c < res->count; c++) { + for (c = 0; c < res->count; c++) { + if (DOM_HAS_VIEWS(domain)) { ret = sysdb_add_overrides_to_object(domain, res->msgs[c], NULL, NULL); /* enumeration assumes that the cache is up-to-date, hence we do not @@ -1167,13 +1177,14 @@ int sysdb_enumgrent_filter_with_views(TALLOC_CTX *mem_ctx, DEBUG(SSSDBG_OP_FAILURE, "sysdb_add_overrides_to_object failed.\n"); goto done; } + } - ret = sysdb_add_group_member_overrides(domain, res->msgs[c]); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, - "sysdb_add_group_member_overrides failed.\n"); - goto done; - } + ret = sysdb_add_group_member_overrides(domain, res->msgs[c], + DOM_HAS_VIEWS(domain)); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "sysdb_add_group_member_overrides failed.\n"); + goto done; } } diff --git a/src/db/sysdb_views.c b/src/db/sysdb_views.c index 2b89e5ca41f719e1217ef3b9e0fd683656e05d42..79f513d13ba41212a6cd84e1d9e609df6acba29c 100644 --- a/src/db/sysdb_views.c +++ b/src/db/sysdb_views.c @@ -1348,14 +1348,13 @@ done: } errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain, - struct ldb_message *obj) + struct ldb_message *obj, + bool expect_override_dn) { int ret; size_t c; - struct ldb_message_element *members; + struct ldb_result *res_members; TALLOC_CTX *tmp_ctx; - struct ldb_dn *member_dn; - struct ldb_result *member_obj; struct ldb_result *override_obj; static const char *member_attrs[] = SYSDB_PW_ATTRS; const char *override_dn_str; @@ -1366,12 +1365,6 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain, char *val; struct sss_domain_info *orig_dom; - members = ldb_msg_find_element(obj, SYSDB_MEMBER); - if (members == NULL || members->num_values == 0) { - DEBUG(SSSDBG_TRACE_ALL, "Group has no members.\n"); - return EOK; - } - tmp_ctx = talloc_new(NULL); if (tmp_ctx == NULL) { DEBUG(SSSDBG_OP_FAILURE, "talloc_new failed.\n"); @@ -1379,38 +1372,30 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain, goto done; } - for (c = 0; c < members->num_values; c++) { - member_dn = ldb_dn_from_ldb_val(tmp_ctx, domain->sysdb->ldb, - &members->values[c]); - if (member_dn == NULL) { - DEBUG(SSSDBG_OP_FAILURE, "ldb_dn_from_ldb_val failed.\n"); - ret = ENOMEM; - goto done; - } + ret = sysdb_get_user_members_recursively(tmp_ctx, domain, obj->dn, + &res_members); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "sysdb_get_user_members_recursively failed.\n"); + goto done; + } - ret = ldb_search(domain->sysdb->ldb, member_dn, &member_obj, member_dn, - LDB_SCOPE_BASE, member_attrs, NULL); - if (ret != LDB_SUCCESS) { - ret = sysdb_error_to_errno(ret); - goto done; - } + for (c = 0; c < res_members->count; c++) { - if (member_obj->count != 1) { - DEBUG(SSSDBG_CRIT_FAILURE, - "Base search for member object returned [%d] results.\n", - member_obj->count); - ret = EINVAL; - goto done; - } - - if (ldb_msg_find_attr_as_uint64(member_obj->msgs[0], + if (ldb_msg_find_attr_as_uint64(res_members->msgs[c], SYSDB_UIDNUM, 0) == 0) { /* Skip non-POSIX-user members i.e. groups and non-POSIX users */ continue; } - override_dn_str = ldb_msg_find_attr_as_string(member_obj->msgs[0], - SYSDB_OVERRIDE_DN, NULL); + if (expect_override_dn) { + override_dn_str = ldb_msg_find_attr_as_string(res_members->msgs[c], + SYSDB_OVERRIDE_DN, + NULL); + } else { + override_dn_str = ldb_dn_get_linearized(res_members->msgs[c]->dn); + } + if (override_dn_str == NULL) { if (is_local_view(domain->view_name)) { /* LOCAL view doesn't have to have overrideDN specified. */ @@ -1420,12 +1405,12 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain, DEBUG(SSSDBG_CRIT_FAILURE, "Missing override DN for object [%s].\n", - ldb_dn_get_linearized(member_obj->msgs[0]->dn)); + ldb_dn_get_linearized(res_members->msgs[c]->dn)); ret = ENOENT; goto done; } - override_dn = ldb_dn_new(member_obj, domain->sysdb->ldb, + override_dn = ldb_dn_new(res_members, domain->sysdb->ldb, override_dn_str); if (override_dn == NULL) { DEBUG(SSSDBG_OP_FAILURE, "ldb_dn_new failed.\n"); @@ -1433,22 +1418,27 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain, goto done; } - orig_name = ldb_msg_find_attr_as_string(member_obj->msgs[0], + orig_name = ldb_msg_find_attr_as_string(res_members->msgs[c], SYSDB_NAME, NULL); if (orig_name == NULL) { DEBUG(SSSDBG_CRIT_FAILURE, "Object [%s] has no name.\n", - ldb_dn_get_linearized(member_obj->msgs[0]->dn)); + ldb_dn_get_linearized(res_members->msgs[c]->dn)); ret = EINVAL; goto done; } - memberuid = NULL; - if (ldb_dn_compare(member_obj->msgs[0]->dn, override_dn) != 0) { + /* start with default view name, if it exists or use NULL */ + memberuid = ldb_msg_find_attr_as_string(res_members->msgs[c], + SYSDB_DEFAULT_OVERRIDE_NAME, + NULL); + + /* If there is an override object, check if the name is overridden */ + if (ldb_dn_compare(res_members->msgs[c]->dn, override_dn) != 0) { DEBUG(SSSDBG_TRACE_ALL, "Checking override for object [%s].\n", - ldb_dn_get_linearized(member_obj->msgs[0]->dn)); + ldb_dn_get_linearized(res_members->msgs[c]->dn)); - ret = ldb_search(domain->sysdb->ldb, member_obj, &override_obj, + ret = ldb_search(domain->sysdb->ldb, res_members, &override_obj, override_dn, LDB_SCOPE_BASE, member_attrs, NULL); if (ret != LDB_SUCCESS) { ret = sysdb_error_to_errno(ret); @@ -1458,43 +1448,44 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain, if (override_obj->count != 1) { DEBUG(SSSDBG_CRIT_FAILURE, "Base search for override object returned [%d] results.\n", - member_obj->count); + override_obj->count); ret = EINVAL; goto done; } memberuid = ldb_msg_find_attr_as_string(override_obj->msgs[0], SYSDB_NAME, - NULL); + memberuid); + } - if (memberuid != NULL) { - ret = sss_parse_internal_fqname(tmp_ctx, orig_name, - NULL, &orig_domain); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, - "sss_parse_internal_fqname failed to split [%s].\n", - orig_name); + /* add domain name if memberuid is a short name */ + if (memberuid != NULL && strchr(memberuid, '@') == NULL) { + ret = sss_parse_internal_fqname(tmp_ctx, orig_name, + NULL, &orig_domain); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "sss_parse_internal_fqname failed to split [%s].\n", + orig_name); + goto done; + } + + if (orig_domain != NULL) { + orig_dom = find_domain_by_name(get_domains_head(domain), + orig_domain, true); + if (orig_dom == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Cannot find domain with name [%s].\n", + orig_domain); + ret = ERR_DOMAIN_NOT_FOUND; goto done; } - - if (orig_domain != NULL) { - orig_dom = find_domain_by_name(get_domains_head(domain), - orig_domain, true); - if (orig_dom == NULL) { - DEBUG(SSSDBG_CRIT_FAILURE, - "Cannot find domain with name [%s].\n", - orig_domain); - ret = ERR_DOMAIN_NOT_FOUND; - goto done; - } - memberuid = sss_create_internal_fqname(tmp_ctx, memberuid, - orig_dom->name); - if (memberuid == NULL) { - DEBUG(SSSDBG_OP_FAILURE, - "sss_create_internal_fqname failed.\n"); - ret = ENOMEM; - goto done; - } + memberuid = sss_create_internal_fqname(tmp_ctx, memberuid, + orig_dom->name); + if (memberuid == NULL) { + DEBUG(SSSDBG_OP_FAILURE, + "sss_create_internal_fqname failed.\n"); + ret = ENOMEM; + goto done; } } } @@ -1521,9 +1512,6 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain, DEBUG(SSSDBG_TRACE_ALL, "Added [%s] to [%s].\n", memberuid, OVERRIDE_PREFIX SYSDB_MEMBERUID); - /* Free all temporary data of the current member to avoid memory usage - * spikes. All temporary data should be allocated below member_dn. */ - talloc_free(member_dn); } ret = EOK; diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c index 1ae17969688fa29734ca14fd2b152decef1fdbca..4e84b3202cbf367e70a47a3c7edb06e357657538 100644 --- a/src/responder/nss/nsssrv_cmd.c +++ b/src/responder/nss/nsssrv_cmd.c @@ -2976,7 +2976,12 @@ static int fill_grent(struct sss_packet *packet, memnum = 0; if (!dom->ignore_group_members) { - el = sss_view_ldb_msg_find_element(dom, msg, SYSDB_MEMBERUID); + /* unconditionally prefer OVERRIDE_PREFIX SYSDB_MEMBERUID, it + * might contain override names from the default view */ + el = ldb_msg_find_element(msg, OVERRIDE_PREFIX SYSDB_MEMBERUID); + if (el == NULL) { + el = ldb_msg_find_element(msg, SYSDB_MEMBERUID); + } if (el) { ret = fill_members(packet, nctx->rctx, dom, nctx, el, &rzero, &rsize, &memnum); diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c index 82a304feed864b09168d0f3e06a4e1bb120df7e4..892bebe63816a0dac0317dd1fd0135523f0e64d7 100644 --- a/src/tests/cmocka/test_nss_srv.c +++ b/src/tests/cmocka/test_nss_srv.c @@ -1342,8 +1342,8 @@ static int test_nss_getgrnam_members_check(uint32_t status, int ret; uint32_t nmem; struct group gr; - const char *exp_members[] = { testmember1.pw_name, - testmember2.pw_name }; + const char *exp_members[] = { testmember2.pw_name, + testmember1.pw_name }; struct group expected = { .gr_gid = testgroup_members.gr_gid, .gr_name = testgroup_members.gr_name, @@ -1429,10 +1429,10 @@ static int test_nss_getgrnam_members_check_fqdn(uint32_t status, assert_non_null(tmp_ctx); exp_members[0] = sss_tc_fqname(tmp_ctx, nss_test_ctx->tctx->dom->names, - nss_test_ctx->tctx->dom, testmember1.pw_name); + nss_test_ctx->tctx->dom, testmember2.pw_name); assert_non_null(exp_members[0]); exp_members[1] = sss_tc_fqname(tmp_ctx, nss_test_ctx->tctx->dom->names, - nss_test_ctx->tctx->dom, testmember2.pw_name); + nss_test_ctx->tctx->dom, testmember1.pw_name); assert_non_null(exp_members[1]); expected.gr_name = sss_tc_fqname(tmp_ctx, @@ -1619,8 +1619,8 @@ static int test_nss_getgrnam_check_mix_dom(uint32_t status, tmp_ctx = talloc_new(nss_test_ctx); assert_non_null(tmp_ctx); - exp_members[0] = testmember1.pw_name; - exp_members[1] = testmember2.pw_name; + exp_members[0] = testmember2.pw_name; + exp_members[1] = testmember1.pw_name; exp_members[2] = sss_tc_fqname(tmp_ctx, nss_test_ctx->subdom->names, nss_test_ctx->subdom, submember1.pw_name); assert_non_null(exp_members[2]); @@ -1683,10 +1683,10 @@ static int test_nss_getgrnam_check_mix_dom_fqdn(uint32_t status, assert_non_null(tmp_ctx); exp_members[0] = sss_tc_fqname(tmp_ctx, nss_test_ctx->tctx->dom->names, - nss_test_ctx->tctx->dom, testmember1.pw_name); + nss_test_ctx->tctx->dom, testmember2.pw_name); assert_non_null(exp_members[0]); exp_members[1] = sss_tc_fqname(tmp_ctx, nss_test_ctx->tctx->dom->names, - nss_test_ctx->tctx->dom, testmember2.pw_name); + nss_test_ctx->tctx->dom, testmember1.pw_name); assert_non_null(exp_members[1]); exp_members[2] = sss_tc_fqname(tmp_ctx, nss_test_ctx->subdom->names, nss_test_ctx->subdom, submember1.pw_name); @@ -1752,16 +1752,16 @@ static int test_nss_getgrnam_check_mix_subdom(uint32_t status, tmp_ctx = talloc_new(nss_test_ctx); assert_non_null(tmp_ctx); - exp_members[0] = sss_tc_fqname(tmp_ctx, nss_test_ctx->subdom->names, - nss_test_ctx->subdom, submember1.pw_name); - assert_non_null(exp_members[0]); exp_members[1] = sss_tc_fqname(tmp_ctx, nss_test_ctx->subdom->names, - nss_test_ctx->subdom, submember2.pw_name); + nss_test_ctx->subdom, submember1.pw_name); assert_non_null(exp_members[1]); + exp_members[2] = sss_tc_fqname(tmp_ctx, nss_test_ctx->subdom->names, + nss_test_ctx->subdom, submember2.pw_name); + assert_non_null(exp_members[2]); /* Important: this member is from a non-qualified domain, so his name will * not be qualified either */ - exp_members[2] = testmember1.pw_name; + exp_members[0] = testmember1.pw_name; expected.gr_name = sss_tc_fqname(tmp_ctx, nss_test_ctx->subdom->names, nss_test_ctx->subdom, testsubdomgroup.gr_name); -- 2.1.0