From 11f6fcedb0ac04528dd319fcf95d1fbaa4ea8bd1 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Thu, 7 Jul 2016 18:54:02 +0200 Subject: [PATCH 48/62] views: properly override group member names Resolves https://fedorahosted.org/sssd/ticket/2948 Reviewed-by: Jakub Hrozek (cherry picked from commit 1594701fbdc341069e11cff9a85e7a795e52db3d) --- 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 | 18 +++--- 5 files changed, 134 insertions(+), 129 deletions(-) diff --git a/src/db/sysdb.h b/src/db/sysdb.h index 405f89e2f1ac6fabc06e77c345de8693845f9d92..a27552224bb40bd07c7dee4dfe35bfb7a0b4f2c3 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -572,7 +572,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..41425e76f3b76fafa917f33fcfef0946f2f71c7d 100644 --- a/src/tests/cmocka/test_nss_srv.c +++ b/src/tests/cmocka/test_nss_srv.c @@ -1619,11 +1619,11 @@ 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[2] = sss_tc_fqname(tmp_ctx, nss_test_ctx->subdom->names, + 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[2]); + assert_non_null(exp_members[0]); + exp_members[1] = testmember1.pw_name; + exp_members[2] = testmember2.pw_name; assert_int_equal(status, EOK); @@ -1682,14 +1682,14 @@ static int test_nss_getgrnam_check_mix_dom_fqdn(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->tctx->dom->names, - nss_test_ctx->tctx->dom, testmember1.pw_name); + 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->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); + exp_members[2] = sss_tc_fqname(tmp_ctx, nss_test_ctx->tctx->dom->names, + nss_test_ctx->tctx->dom, testmember2.pw_name); assert_non_null(exp_members[2]); expected.gr_name = sss_tc_fqname(tmp_ctx, -- 2.4.11