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. > > The third patch adds a sysdb call to recursively resolve all > user-members of a group. Since the groups in SSSD's cache are > hierarchically organized the member attribute only contains direct > user and group members. To get all users the group members must be > resolved recursively. Would dereferencing memberof:top-level-group yield different results? > > Finally the forth patch applies the code-path which is already used for > non-default views to the default case as well and adds a new list of > members, with correctly overridden names (hopefully :-) which is then > used in fill_grmem(). This adds some overhead to the overall group > processing in the NSS responder (as can be seen in the test changes > because the members are returned in different order in some cases). But > I think because the of memory cache this is acceptable and might even > help to remove the memberuid attribute in future and make the memberof > plugin simpler. > > I worked on an alternative approach as well which tried to make the > memberof plugin aware of the defaultOverrideName attribute. My wip tree > is at > https://fedorapeople.org/cgit/sbose/public_git/sssd.git/log/?h=memberof_default_view > but so far it does not work properly. Additionally I would prefer to not > touch the memberof plugin. > > bye, > Sumit > From 3a4ce5d188c2c97372aa92bb1e1596f8eca8c6e5 Mon Sep 17 00:00:00 2001 > From: Sumit Bose <[email protected]> > 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 a47bc1d1acd0e8eae2ccdab0cb285c965cd1189d Mon Sep 17 00:00:00 2001 > From: Sumit Bose <[email protected]> > 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 | 81 > ++++++++++++++++++++++++++++++++++- > 1 file changed, 80 insertions(+), 1 deletion(-) > > diff --git a/src/providers/ipa/ipa_subdomains_id.c > b/src/providers/ipa/ipa_subdomains_id.c > index > 0a89ef5a68802fb7d5f9faeb9ef1d9f9cf8d14f2..a6cf3a9072c4cc6995a5813dd52397b90dc245c8 > 100644 > --- a/src/providers/ipa/ipa_subdomains_id.c > +++ b/src/providers/ipa/ipa_subdomains_id.c > @@ -1201,6 +1201,69 @@ 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); > + struct ipa_get_ad_acct_state *state = tevent_req_data(req, > + struct ipa_get_ad_acct_state); > + 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 +1291,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 a2f8440cf9848488353b320cf355a90529840879 Mon Sep 17 00:00:00 2001 > From: Sumit Bose <[email protected]> > 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 | 205 > +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 210 insertions(+) > > diff --git a/src/db/sysdb.h b/src/db/sysdb.h > index > 407ce3c18a7077e8fe45c3c9c7576ae626105122..489cb4d1b1af81711d93ed1eb0cb734955c3f8c0 > 100644 > --- a/src/db/sysdb.h > +++ b/src/db/sysdb.h > @@ -1260,6 +1260,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 > 4755ea3427b99a51d73b7b9134e357cf2b987613..358e0bb6d493d313ff2e2280b92506f266d3e722 > 100644 > --- a/src/db/sysdb_ops.c > +++ b/src/db/sysdb_ops.c > @@ -4706,6 +4706,211 @@ 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 [%llu] 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 1ebdd37552bac2ce05519efaf932946f79640a04 Mon Sep 17 00:00:00 2001 > From: Sumit Bose <[email protected]> > 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 > > _______________________________________________ > sssd-devel mailing list > [email protected] > https://lists.fedorahosted.org/admin/lists/[email protected] _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
