From 11f6fcedb0ac04528dd319fcf95d1fbaa4ea8bd1 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose@redhat.com>
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 <jhrozek@redhat.com>
(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