On Tue, Jul 26, 2016 at 01:56:38PM +0200, Jakub Hrozek wrote:
> On Tue, Jul 26, 2016 at 01:51:56PM +0200, Sumit Bose wrote:
> > On Mon, Jul 25, 2016 at 01:45:13PM +0200, Jakub Hrozek wrote:
> > > On Thu, Jul 21, 2016 at 02:13:40PM +0200, Sumit Bose wrote:
> > > > Hi,
> > > >
> > > > this is my suggestion to solve https://fedorahosted.org/sssd/ticket/2948
> > > > "Handle overriden name of members in the memberUid attribute".
> > >
> > > So far I read them to get a grasp of what they do, but didn't do a full
> > > review. See some comments below.
> > >
> > > >
> > > > The first two patches are for the IPA provider and make sure that all
> > > > ghost members in a group get resolved because otherwise we cannot
> > > > determine if the name is overridden or not. This adds an overhead to
> > > > group lookups, especially for larger groups but I think it is an
> > > > requirement which cannot be skipped.
> > >
> > > Right. But I wonder if we could skip it on-demand. Could we maybe check
> > > if any views exist in the IPA domain at all and not resolve the members
> > > if there are no idview data? I would guess that since this is SSSD on
> > > the IPA server, then an additiona LDAP lookup might be cheaper than
> > > resolving a full large group.
> >
> > Yes, that would be possible. But since this is about AD users I would
> > prefer a different change.
> >
> > Since AD used RFC2307bis (member attribute with the DN of the member) we
> > already read the user object from the AD GC when resolving the group but
> > only
> > ask for objectclass and samAccountName to be able to fill the ghost
> > entry. If we would switch to the LDAP port, read the whole entry and
> > save it to the chance there wouldn't be any ghost entries in the first
> > place. But I wanted to avoid regressions in the common SDAP code at this
> > time so I resolved the ghosts in a second step.
> >
> > But I guess it is worth to open a ticket to add either or both kinds of
> > optimization.
>
> OK, I hope with the timestamp cache, the performance impact would only
> be during the first lookup.
>
> Should I open that ticket or would you?
Please open a ticket.
New version of the patches attached.
bye,
Sumit
> _______________________________________________
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
From 5c89f2985fd1aaf8d7b8bd4c00a1e5f2e898398c Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Tue, 12 Jul 2016 17:09:00 +0200
Subject: [PATCH 1/4] IPA: make ipa_resolve_user_list_{send|recv} public and
allow AD users
---
src/providers/ipa/ipa_id.c | 20 ++++++++++++++++----
src/providers/ipa/ipa_id.h | 8 ++++++++
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/src/providers/ipa/ipa_id.c b/src/providers/ipa/ipa_id.c
index
cfc25b9b322d78bbc64de96227a03ed0604d3391..7f0cd5689dbfde390c162eef340e1120247b703c
100644
--- a/src/providers/ipa/ipa_id.c
+++ b/src/providers/ipa/ipa_id.c
@@ -71,7 +71,7 @@ struct ipa_resolve_user_list_state {
static errno_t ipa_resolve_user_list_get_user_step(struct tevent_req *req);
static void ipa_resolve_user_list_get_user_done(struct tevent_req *subreq);
-static struct tevent_req *
+struct tevent_req *
ipa_resolve_user_list_send(TALLOC_CTX *memctx, struct tevent_context *ev,
struct ipa_id_ctx *ipa_ctx,
const char *domain_name,
@@ -132,7 +132,14 @@ static errno_t ipa_resolve_user_list_get_user_step(struct
tevent_req *req)
DEBUG(SSSDBG_TRACE_ALL, "Trying to resolve user [%s].\n",
ar->filter_value);
- subreq = ipa_id_get_account_info_send(state, state->ev, state->ipa_ctx,
ar);
+ if (strcasecmp(state->domain_name,
+ state->ipa_ctx->sdap_id_ctx->be->domain->name) != 0) {
+ subreq = ipa_subdomain_account_send(state, state->ev, state->ipa_ctx,
+ ar);
+ } else {
+ subreq = ipa_id_get_account_info_send(state, state->ev, state->ipa_ctx,
+ ar);
+ }
if (subreq == NULL) {
DEBUG(SSSDBG_OP_FAILURE, "sdap_handle_acct_req_send failed.\n");
return ENOMEM;
@@ -151,7 +158,12 @@ static void ipa_resolve_user_list_get_user_done(struct
tevent_req *subreq)
struct
ipa_resolve_user_list_state);
int ret;
- ret = ipa_id_get_account_info_recv(subreq, &state->dp_error);
+ if (strcasecmp(state->domain_name,
+ state->ipa_ctx->sdap_id_ctx->be->domain->name) != 0) {
+ ret = ipa_subdomain_account_recv(subreq, &state->dp_error);
+ } else {
+ ret = ipa_id_get_account_info_recv(subreq, &state->dp_error);
+ }
talloc_zfree(subreq);
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, "sdap_handle_acct request failed: %d\n", ret);
@@ -182,7 +194,7 @@ done:
return;
}
-static int ipa_resolve_user_list_recv(struct tevent_req *req, int *dp_error)
+int ipa_resolve_user_list_recv(struct tevent_req *req, int *dp_error)
{
struct ipa_resolve_user_list_state *state = tevent_req_data(req,
struct
ipa_resolve_user_list_state);
diff --git a/src/providers/ipa/ipa_id.h b/src/providers/ipa/ipa_id.h
index
410c2086833d811571195e05a7cab0e2c82fa5b9..4b25498821c6cd0d574c4ce78e4ece29f60459f7
100644
--- a/src/providers/ipa/ipa_id.h
+++ b/src/providers/ipa/ipa_id.h
@@ -135,4 +135,12 @@ struct tevent_req
*ipa_get_subdom_acct_process_pac_send(TALLOC_CTX *mem_ctx,
struct ldb_message
*user_msg);
errno_t ipa_get_subdom_acct_process_pac_recv(struct tevent_req *req);
+
+struct tevent_req *
+ipa_resolve_user_list_send(TALLOC_CTX *memctx, struct tevent_context *ev,
+ struct ipa_id_ctx *ipa_ctx,
+ const char *domain_name,
+ struct ldb_message_element *users);
+int ipa_resolve_user_list_recv(struct tevent_req *req, int *dp_error);
+
#endif
--
2.1.0
From a11b11cb55cf52a7f8f44665547bae141da4c939 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Tue, 12 Jul 2016 17:09:40 +0200
Subject: [PATCH 2/4] IPA: expand ghost members of AD groups in server-mode
---
src/providers/ipa/ipa_subdomains_id.c | 79 ++++++++++++++++++++++++++++++++++-
1 file changed, 78 insertions(+), 1 deletion(-)
diff --git a/src/providers/ipa/ipa_subdomains_id.c
b/src/providers/ipa/ipa_subdomains_id.c
index
0a89ef5a68802fb7d5f9faeb9ef1d9f9cf8d14f2..7c5deab34f257040a128f09b6617458b8b109b4c
100644
--- a/src/providers/ipa/ipa_subdomains_id.c
+++ b/src/providers/ipa/ipa_subdomains_id.c
@@ -1201,6 +1201,67 @@ fail:
return;
}
+static void ipa_check_ghost_members_done(struct tevent_req *subreq);
+static errno_t ipa_check_ghost_members(struct tevent_req *req)
+{
+ struct ipa_get_ad_acct_state *state = tevent_req_data(req,
+ struct ipa_get_ad_acct_state);
+ errno_t ret;
+ struct tevent_req *subreq;
+ struct ldb_message_element *ghosts = NULL;
+
+
+ if (state->obj_msg == NULL) {
+ ret = get_object_from_cache(state, state->obj_dom, state->ar,
+ &state->obj_msg);
+ if (ret == ENOENT) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ "Object not found, ending request\n");
+ return EOK;
+ } else if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, "get_object_from_cache failed.\n");
+ return ret;
+ }
+ }
+
+ ghosts = ldb_msg_find_element(state->obj_msg, SYSDB_GHOST);
+
+ if (ghosts != NULL) {
+ /* Resolve ghost members */
+ subreq = ipa_resolve_user_list_send(state, state->ev,
+ state->ipa_ctx,
+ state->obj_dom->name,
+ ghosts);
+ if (subreq == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "ipa_resolve_user_list_send failed.\n");
+ return ENOMEM;
+ }
+ tevent_req_set_callback(subreq, ipa_check_ghost_members_done, req);
+ return EAGAIN;
+ }
+
+ return EOK;
+}
+
+static void ipa_check_ghost_members_done(struct tevent_req *subreq)
+{
+ struct tevent_req *req = tevent_req_callback_data(subreq,
+ struct tevent_req);
+ int ret;
+
+ ret = ipa_resolve_user_list_recv(subreq, NULL);
+ talloc_zfree(subreq);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, "ipa_resolve_user_list request failed [%d]\n",
+ ret);
+ tevent_req_error(req, ret);
+ return;
+ }
+
+ tevent_req_done(req);
+ return;
+}
+
static errno_t ipa_get_ad_apply_override_step(struct tevent_req *req)
{
struct ipa_get_ad_acct_state *state = tevent_req_data(req,
@@ -1228,11 +1289,27 @@ static errno_t ipa_get_ad_apply_override_step(struct
tevent_req *req)
entry_type = (state->ar->entry_type & BE_REQ_TYPE_MASK);
if (entry_type != BE_REQ_INITGROUPS
&& entry_type != BE_REQ_USER
- && entry_type != BE_REQ_BY_SECID) {
+ && entry_type != BE_REQ_BY_SECID
+ && entry_type != BE_REQ_GROUP) {
tevent_req_done(req);
return EOK;
}
+ /* expand ghost members, if any, to get group members with overrides
+ * right. */
+ if (entry_type == BE_REQ_GROUP) {
+ ret = ipa_check_ghost_members(req);
+ if (ret == EOK) {
+ tevent_req_done(req);
+ return EOK;
+ } else if (ret == EAGAIN) {
+ return EOK;
+ } else {
+ DEBUG(SSSDBG_OP_FAILURE, "ipa_check_ghost_members failed.\n");
+ return ret;
+ }
+ }
+
/* Replace ID with name in search filter */
if ((entry_type == BE_REQ_USER && state->ar->filter_type ==
BE_FILTER_IDNUM)
|| (entry_type == BE_REQ_INITGROUPS
--
2.1.0
From 5596a3cb36c9f0d0b8ecbcd68f82bc3d7b040b55 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Wed, 20 Jul 2016 18:42:27 +0200
Subject: [PATCH 3/4] sysdb: add sysdb_get_user_members_recursively()
---
src/db/sysdb.h | 5 ++
src/db/sysdb_ops.c | 206 +++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 211 insertions(+)
diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index
0cc550a4c389b4a1a2b78aff760f4b5cbf94e17f..405f89e2f1ac6fabc06e77c345de8693845f9d92
100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -1257,6 +1257,11 @@ errno_t sysdb_get_sids_of_members(TALLOC_CTX *mem_ctx,
const char ***_dns,
size_t *_n);
+errno_t sysdb_get_user_members_recursively(TALLOC_CTX *mem_ctx,
+ struct sss_domain_info *dom,
+ struct ldb_dn *group_dn,
+ struct ldb_result **members);
+
errno_t sysdb_handle_original_uuid(const char *orig_name,
struct sysdb_attrs *src_attrs,
const char *src_name,
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index
19d6be03ede1bcec3bc7a4ed777e326460d80591..b7c04cb4ab64c2767e211d6ead1463a3a3024e68
100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -4711,6 +4711,212 @@ done:
return ret;
}
+static
+errno_t sysdb_get_user_members_recursively_int(TALLOC_CTX *mem_ctx,
+ struct sss_domain_info *dom,
+ struct ldb_dn *group_dn,
+ hash_table_t *users,
+ hash_table_t *groups)
+{
+ errno_t ret;
+ size_t c;
+ size_t m_count;
+ struct ldb_message **members;
+ const char *oc;
+ hash_key_t key;
+ hash_value_t value;
+ int hret;
+
+ /* Get all elemets pointed to by group members */
+ ret = sysdb_asq_search(mem_ctx, dom, group_dn, NULL, SYSDB_MEMBER,
+ NULL /* requesting all attributes */,
+ &m_count, &members);
+ if (ret != EOK) {
+ if (ret != ENOENT) {
+ DEBUG(SSSDBG_OP_FAILURE, "sysdb_asq_search failed.\n");
+ }
+ goto done;
+ }
+
+ key.type = HASH_KEY_STRING;
+
+ for (c = 0; c < m_count; c++) {
+
+ oc = ldb_msg_find_attr_as_string(members[c], SYSDB_OBJECTCLASS, NULL);
+ if (oc == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Missing objectclass in object [%s].\n",
+ ldb_dn_get_linearized(members[c]->dn));
+ ret = EINVAL;
+ goto done;
+ }
+
+ if (strcmp(oc, SYSDB_USER_CLASS) == 0) {
+ key.str = discard_const(ldb_dn_get_linearized(members[c]->dn));
+ if (key.str == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "ldb_dn_get_linearized failed.\n");
+ ret = EFAULT;
+ goto done;
+ }
+
+ value.type = HASH_VALUE_PTR;
+ value.ptr = members[c];
+
+ hret = hash_enter(users, &key, &value);
+ if (hret != HASH_SUCCESS) {
+ DEBUG(SSSDBG_OP_FAILURE, "hash_enter failed.\n");
+ ret = ENOMEM;
+ goto done;
+ }
+
+ } else if (strcmp(oc, SYSDB_GROUP_CLASS) == 0) {
+ key.str = discard_const(ldb_dn_get_linearized(members[c]->dn));
+ if (key.str == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "ldb_dn_get_linearized failed.\n");
+ ret = EFAULT;
+ goto done;
+ }
+
+ hret = hash_lookup(groups, &key, &value);
+ if (hret == HASH_SUCCESS) {
+ /* Already seen this group, skipping */
+ continue;
+ } else if (hret != HASH_ERROR_KEY_NOT_FOUND) {
+ DEBUG(SSSDBG_OP_FAILURE, "hash_lookup failed.\n");
+ ret = EFAULT;
+ goto done;
+ }
+
+ /* group was not found in the hash, adding and processing it */
+
+ value.type = HASH_VALUE_PTR;
+ value.ptr = members[c];
+
+ hret = hash_enter(groups, &key, &value);
+ if (hret != HASH_SUCCESS) {
+ DEBUG(SSSDBG_OP_FAILURE, "hash_enter failed.\n");
+ ret = ENOMEM;
+ goto done;
+ }
+
+ ret = sysdb_get_user_members_recursively_int(mem_ctx, dom,
+ members[c]->dn,
+ users, groups);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "sysdb_get_user_members_recursively_int failed.\n");
+ goto done;
+ }
+
+ } else {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Unexpected objectclass [%s] in object [%s].\n", oc,
+ ldb_dn_get_linearized(members[c]->dn));
+ ret = EINVAL;
+ goto done;
+ }
+ }
+
+ ret = EOK;
+
+done:
+
+ return ret;
+}
+
+errno_t sysdb_get_user_members_recursively(TALLOC_CTX *mem_ctx,
+ struct sss_domain_info *dom,
+ struct ldb_dn *group_dn,
+ struct ldb_result **members)
+{
+ TALLOC_CTX *tmp_ctx;
+ hash_table_t *users;
+ hash_table_t *groups;
+ int ret;
+ int hret;
+ size_t c;
+ unsigned long count;
+ struct ldb_result *res;
+ hash_value_t *values;
+
+ tmp_ctx = talloc_new(NULL);
+ if (tmp_ctx == NULL) {
+ return ENOMEM;
+ }
+
+ ret = sss_hash_create(tmp_ctx, 10, &users);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, "sss_hash_create failed.\n");
+ goto done;
+ }
+
+ ret = sss_hash_create(tmp_ctx, 10, &groups);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, "sss_hash_create failed.\n");
+ goto done;
+ }
+
+ ret = sysdb_get_user_members_recursively_int(tmp_ctx, dom, group_dn, users,
+ groups);
+ if (ret != EOK) {
+ if (ret != ENOENT) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "sysdb_get_user_members_recursively_int failed.\n");
+ }
+ goto done;
+ }
+
+ res = talloc_zero(tmp_ctx, struct ldb_result);
+ if (res == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "talloc_zero failed.\n");
+ ret = ENOMEM;
+ goto done;
+ }
+
+ hret = hash_values(users, &count, &values);
+ if (hret != HASH_SUCCESS) {
+ DEBUG(SSSDBG_OP_FAILURE, "hash_values failed.\n");
+ ret = ENOMEM;
+ goto done;
+ }
+ if (count <= UINTMAX_MAX) {
+ res->count = count;
+ } else {
+ DEBUG(SSSDBG_CRIT_FAILURE, "More then [%"PRIuMAX"] results.\n",
+ UINTMAX_MAX);
+ ret = EINVAL;
+ goto done;
+ }
+ if (res->count == 0) {
+ ret = EOK;
+ goto done;
+ }
+
+ res->msgs = talloc_array(res, struct ldb_message *, res->count);
+ if (res->msgs == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "talloc_array failed.\n");
+ ret = ENOMEM;
+ goto done;
+ }
+
+ for (c = 0; c < res->count; c++) {
+ res->msgs[c] = (struct ldb_message *) talloc_steal(res->msgs,
+ values[c].ptr);
+ }
+
+ ret = EOK;
+
+done:
+ if (ret == EOK) {
+ *members = talloc_steal(mem_ctx, res);
+ } else if (ret == ENOENT) {
+ DEBUG(SSSDBG_TRACE_FUNC, "No such entry\n");
+ } else {
+ DEBUG(SSSDBG_OP_FAILURE, "Error: %d (%s)\n", ret, strerror(ret));
+ }
+ talloc_free(tmp_ctx);
+ return ret;
+}
+
errno_t sysdb_handle_original_uuid(const char *orig_name,
struct sysdb_attrs *src_attrs,
const char *src_name,
--
2.1.0
From 57ec193fd6a4d5014ef47a720dae74fdf6df2207 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
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
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..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
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
