URL: https://github.com/SSSD/sssd/pull/235 Author: fidencio Title: #235: Allow using the "shortnames" feature without requiring any configuration from the client side Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/235/head:pr235 git checkout pr235
From a8341b3968380156c94c76bfa50ca89bbd34ec79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]> Date: Wed, 12 Apr 2017 10:43:25 +0200 Subject: [PATCH 1/3] RESPONDER: Fallback to global domain resolution order in case the view doesn't have this option set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current code has been ignoring the domain resolution order set globally on IPA in case there's a view but this doesn't have any domain resolution order set. It happens because we haven't been checking whether the view attribute didn't exist and then we ended up populating the list cache_req domains' list assuming that no order has been set instead of falling back to the next preferred method. Related: https://pagure.io/SSSD/sssd/issue/3001 Signed-off-by: Fabiano Fidêncio <[email protected]> --- src/responder/common/cache_req/cache_req_domain.c | 14 ++- src/responder/common/cache_req/cache_req_domain.h | 5 +- src/responder/common/responder_common.c | 109 ++++++++++++---------- 3 files changed, 74 insertions(+), 54 deletions(-) diff --git a/src/responder/common/cache_req/cache_req_domain.c b/src/responder/common/cache_req/cache_req_domain.c index bbabd69..86a88ef 100644 --- a/src/responder/common/cache_req/cache_req_domain.c +++ b/src/responder/common/cache_req/cache_req_domain.c @@ -120,20 +120,21 @@ cache_req_domain_new_list_from_string_list(TALLOC_CTX *mem_ctx, return cr_domains; } -struct cache_req_domain * +errno_t cache_req_domain_new_list_from_domain_resolution_order( TALLOC_CTX *mem_ctx, struct sss_domain_info *domains, - const char *domain_resolution_order) + const char *domain_resolution_order, + struct cache_req_domain **_cr_domains) { TALLOC_CTX *tmp_ctx; - struct cache_req_domain *cr_domains = NULL; + struct cache_req_domain *cr_domains; char **list = NULL; errno_t ret; tmp_ctx = talloc_new(NULL); if (tmp_ctx == NULL) { - return NULL; + return ENOMEM; } if (domain_resolution_order != NULL) { @@ -160,7 +161,10 @@ cache_req_domain_new_list_from_domain_resolution_order( goto done; } + *_cr_domains = cr_domains; + ret = EOK; + done: talloc_free(tmp_ctx); - return cr_domains; + return ret; } diff --git a/src/responder/common/cache_req/cache_req_domain.h b/src/responder/common/cache_req/cache_req_domain.h index 41c50e8..000087e 100644 --- a/src/responder/common/cache_req/cache_req_domain.h +++ b/src/responder/common/cache_req/cache_req_domain.h @@ -34,11 +34,12 @@ struct cache_req_domain * cache_req_domain_get_domain_by_name(struct cache_req_domain *domains, const char *name); -struct cache_req_domain * +errno_t cache_req_domain_new_list_from_domain_resolution_order( TALLOC_CTX *mem_ctx, struct sss_domain_info *domains, - const char *domain_resolution_order); + const char *domain_resolution_order, + struct cache_req_domain **_cr_domains); void cache_req_domain_list_zfree(struct cache_req_domain **cr_domains); diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c index 67e1dee..c3d0bfb 100644 --- a/src/responder/common/responder_common.c +++ b/src/responder/common/responder_common.c @@ -1486,19 +1486,21 @@ errno_t responder_setup_idle_timeout_config(struct resp_ctx *rctx) } /* ====== Helper functions for the domain resolution order ======= */ -static struct cache_req_domain * +static errno_t sss_resp_new_cr_domains_from_ipa_id_view(TALLOC_CTX *mem_ctx, struct sss_domain_info *domains, - struct sysdb_ctx *sysdb) + struct sysdb_ctx *sysdb, + struct cache_req_domain **_cr_domains) { TALLOC_CTX *tmp_ctx; - struct cache_req_domain *cr_domains = NULL; const char *domain_resolution_order = NULL; errno_t ret; + *_cr_domains = NULL; + tmp_ctx = talloc_new(NULL); if (tmp_ctx == NULL) { - return NULL; + return ENOMEM; } ret = sysdb_get_view_domain_resolution_order(tmp_ctx, sysdb, @@ -1510,12 +1512,13 @@ sss_resp_new_cr_domains_from_ipa_id_view(TALLOC_CTX *mem_ctx, goto done; } - /* Using mem_ctx (which is rctx) directly here to avoid copying - * this memory around. */ - cr_domains = cache_req_domain_new_list_from_domain_resolution_order( - mem_ctx, domains, domain_resolution_order); - if (cr_domains == NULL) { - ret = ENOMEM; + if (ret == ENOENT) { + goto done; + } + + ret = cache_req_domain_new_list_from_domain_resolution_order( + mem_ctx, domains, domain_resolution_order, _cr_domains); + if (ret != EOK) { DEBUG(SSSDBG_DEFAULT, "cache_req_domain_new_list_from_domain_resolution_order() " "failed [%d]: [%s].\n", @@ -1523,25 +1526,29 @@ sss_resp_new_cr_domains_from_ipa_id_view(TALLOC_CTX *mem_ctx, goto done; } + ret = EOK; + done: talloc_free(tmp_ctx); - return cr_domains; + return ret; } -static struct cache_req_domain * +static errno_t sss_resp_new_cr_domains_from_ipa_config(TALLOC_CTX *mem_ctx, struct sss_domain_info *domains, struct sysdb_ctx *sysdb, - const char *domain) + const char *domain, + struct cache_req_domain **_cr_domains) { TALLOC_CTX *tmp_ctx; - struct cache_req_domain *cr_domains = NULL; const char *domain_resolution_order = NULL; errno_t ret; + *_cr_domains = NULL; + tmp_ctx = talloc_new(NULL); if (tmp_ctx == NULL) { - return NULL; + return ENOMEM; } ret = sysdb_domain_get_domain_resolution_order(tmp_ctx, sysdb, domain, @@ -1554,11 +1561,13 @@ sss_resp_new_cr_domains_from_ipa_config(TALLOC_CTX *mem_ctx, goto done; } - /* Using mem_ctx (which is rctx) directly here to avoid copying - * this memory around. */ - cr_domains = cache_req_domain_new_list_from_domain_resolution_order( - mem_ctx, domains, domain_resolution_order); - if (cr_domains == NULL) { + if (ret == ENOENT) { + goto done; + } + + ret = cache_req_domain_new_list_from_domain_resolution_order( + mem_ctx, domains, domain_resolution_order, _cr_domains); + if (ret != EOK) { DEBUG(SSSDBG_DEFAULT, "cache_req_domain_new_list_from_domain_resolution_order() " "failed [%d]: [%s].\n", @@ -1566,9 +1575,11 @@ sss_resp_new_cr_domains_from_ipa_config(TALLOC_CTX *mem_ctx, goto done; } + ret = EOK; + done: talloc_free(tmp_ctx); - return cr_domains; + return ret; } errno_t sss_resp_populate_cr_domains(struct resp_ctx *rctx) @@ -1578,16 +1589,16 @@ errno_t sss_resp_populate_cr_domains(struct resp_ctx *rctx) errno_t ret; if (rctx->domain_resolution_order != NULL) { - cr_domains = cache_req_domain_new_list_from_domain_resolution_order( - rctx, rctx->domains, rctx->domain_resolution_order); - - if (cr_domains == NULL) { + ret = cache_req_domain_new_list_from_domain_resolution_order( + rctx, rctx->domains, + rctx->domain_resolution_order, &cr_domains); + if (ret == EOK) { + goto done; + } else { DEBUG(SSSDBG_MINOR_FAILURE, "Failed to use domain_resolution_order set in the config file.\n" "Trying to fallback to use ipaDomainOrderResolution setup by " "IPA.\n"); - } else { - goto done; } } @@ -1598,9 +1609,9 @@ errno_t sss_resp_populate_cr_domains(struct resp_ctx *rctx) } if (dom == NULL) { - cr_domains = cache_req_domain_new_list_from_domain_resolution_order( - rctx, rctx->domains, NULL); - if (cr_domains == NULL) { + ret = cache_req_domain_new_list_from_domain_resolution_order( + rctx, rctx->domains, NULL, &cr_domains); + if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "Failed to flatten the list of domains.\n"); } @@ -1608,44 +1619,48 @@ errno_t sss_resp_populate_cr_domains(struct resp_ctx *rctx) } if (dom->has_views) { - cr_domains = sss_resp_new_cr_domains_from_ipa_id_view(rctx, - rctx->domains, - dom->sysdb); - if (cr_domains == NULL) { + ret = sss_resp_new_cr_domains_from_ipa_id_view(rctx, rctx->domains, + dom->sysdb, + &cr_domains); + if (ret == EOK) { + goto done; + } + + if (ret != ENOENT) { DEBUG(SSSDBG_MINOR_FAILURE, "Failed to use ipaDomainResolutionOrder set for the " "view \"%s\".\n" "Trying to fallback to use ipaDomainOrderResolution " "set in ipaConfig for the domain: %s.\n", dom->view_name, dom->name); - } else { - goto done; } } - cr_domains = sss_resp_new_cr_domains_from_ipa_config(rctx, rctx->domains, - dom->sysdb, - dom->name); - if (cr_domains == NULL) { + ret = sss_resp_new_cr_domains_from_ipa_config(rctx, rctx->domains, + dom->sysdb, dom->name, + &cr_domains); + if (ret == EOK) { + goto done; + } + + if (ret != ENOENT) { DEBUG(SSSDBG_MINOR_FAILURE, "Failed to use ipaDomainResolutionOrder set in ipaConfig " "for the domain: \"%s\".\n" "No ipaDomainResolutionOrder will be followed.\n", dom->name); - } else { - goto done; } - cr_domains = cache_req_domain_new_list_from_domain_resolution_order( - rctx, rctx->domains, NULL); - if (cr_domains == NULL) { + ret = cache_req_domain_new_list_from_domain_resolution_order( + rctx, rctx->domains, NULL, &cr_domains); + if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "Failed to flatten the list of domains.\n"); goto done; } -done: - ret = cr_domains != NULL ? EOK : ENOMEM; + ret = EOK; +done: cache_req_domain_list_zfree(&rctx->cr_domains); rctx->cr_domains = cr_domains; From 8ec4484e74fc084ecfc7023baa1a32b9b4ff21ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]> Date: Mon, 17 Apr 2017 09:32:39 +0200 Subject: [PATCH 2/3] NSS/TESTS: Improve non-fqnames tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the changes that are about to happen we have to have the subdomain's fqnames flag set by the time we populate the cr_domains list (as it actually occurs with the real code), as this list may set its own fqnames flag based on the subdomain's fqnames flag. Currently the flag is set to false only when running the tests itself so the cr_domains list doesn't get populate properly (although it still works with the current code). For the changes that are comming, let's introduce a new setup function that ensures that the subdomain's fqnames flag is set up in the right time. Related: https://pagure.io/SSSD/sssd/issue/3001 Signed-off-by: Fabiano Fidêncio <[email protected]> --- src/tests/cmocka/test_nss_srv.c | 45 +++++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c index 2f52666..8c72f44 100644 --- a/src/tests/cmocka/test_nss_srv.c +++ b/src/tests/cmocka/test_nss_srv.c @@ -1709,8 +1709,6 @@ void test_nss_getgrnam_members_subdom_nonfqnames(void **state) { errno_t ret; - nss_test_ctx->subdom->fqnames = false; - mock_input_user_or_group("testsubdomgroup"); mock_account_recv_simple(); will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETGRNAM); @@ -1802,8 +1800,6 @@ void test_nss_getgrnam_mix_dom_nonfqnames(void **state) { errno_t ret; - nss_test_ctx->subdom->fqnames = false; - ret = store_group_member(nss_test_ctx, testgroup_members.gr_name, nss_test_ctx->tctx->dom, @@ -1917,6 +1913,7 @@ void test_nss_getgrnam_mix_dom_fqdn(void **state) assert_int_equal(ret, EOK); } + void test_nss_getgrnam_mix_dom_fqdn_nonfqnames(void **state) { errno_t ret; @@ -1929,10 +1926,6 @@ void test_nss_getgrnam_mix_dom_fqdn_nonfqnames(void **state) SYSDB_MEMBER_USER); assert_int_equal(ret, EOK); - nss_test_ctx->tctx->dom->fqnames = false; - nss_test_ctx->subdom->fqnames = false; - - mock_input_user_or_group("testgroup_members"); will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETGRNAM); will_return_always(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2044,8 +2037,6 @@ void test_nss_getgrnam_mix_subdom_nonfqnames(void **state) { errno_t ret; - nss_test_ctx->subdom->fqnames = false; - ret = store_group_member(nss_test_ctx, testsubdomgroup.gr_name, nss_test_ctx->subdom, @@ -3417,9 +3408,11 @@ static int nss_test_setup_extra_attr(void **state) return 0; } -static int nss_subdom_test_setup(void **state) +static int nss_subdom_test_setup_common(void **state, bool nonfqnames) { const char *const testdom[4] = { TEST_SUBDOM_NAME, "TEST.SUB", "test", "S-3" }; + struct sss_domain_info *dom; + struct sss_domain_info *subdomain; errno_t ret; @@ -3440,6 +3433,17 @@ static int nss_subdom_test_setup(void **state) nss_test_ctx->tctx->confdb); assert_int_equal(ret, EOK); + if (nonfqnames) { + for (dom = nss_test_ctx->rctx->domains; + dom != NULL; + dom = get_next_domain(dom, SSS_GND_ALL_DOMAINS)) { + if (strcmp(dom->name, subdomain->name) == 0) { + dom->fqnames = false; + break; + } + } + } + ret = sss_resp_populate_cr_domains(nss_test_ctx->rctx); assert_int_equal(ret, EOK); assert_non_null(nss_test_ctx->rctx->cr_domains); @@ -3475,6 +3479,17 @@ static int nss_subdom_test_setup(void **state) assert_int_equal(ret, EOK); return 0; + +} + +static int nss_subdom_test_setup(void **state) +{ + return nss_subdom_test_setup_common(state, false); +} + +static int nss_subdom_test_setup_nonfqnames(void **state) +{ + return nss_subdom_test_setup_common(state, true); } static int nss_fqdn_fancy_test_setup(void **state) @@ -4192,25 +4207,25 @@ int main(int argc, const char *argv[]) nss_subdom_test_setup, nss_subdom_test_teardown), cmocka_unit_test_setup_teardown(test_nss_getgrnam_members_subdom_nonfqnames, - nss_subdom_test_setup, + nss_subdom_test_setup_nonfqnames, nss_subdom_test_teardown), cmocka_unit_test_setup_teardown(test_nss_getgrnam_mix_dom, nss_subdom_test_setup, nss_subdom_test_teardown), cmocka_unit_test_setup_teardown(test_nss_getgrnam_mix_dom_nonfqnames, - nss_subdom_test_setup, + nss_subdom_test_setup_nonfqnames, nss_subdom_test_teardown), cmocka_unit_test_setup_teardown(test_nss_getgrnam_mix_dom_fqdn, nss_subdom_test_setup, nss_subdom_test_teardown), cmocka_unit_test_setup_teardown(test_nss_getgrnam_mix_dom_fqdn_nonfqnames, - nss_subdom_test_setup, + nss_subdom_test_setup_nonfqnames, nss_subdom_test_teardown), cmocka_unit_test_setup_teardown(test_nss_getgrnam_mix_subdom, nss_subdom_test_setup, nss_subdom_test_teardown), cmocka_unit_test_setup_teardown(test_nss_getgrnam_mix_subdom_nonfqnames, - nss_subdom_test_setup, + nss_subdom_test_setup_nonfqnames, nss_subdom_test_teardown), cmocka_unit_test_setup_teardown(test_nss_getgrnam_space, nss_test_setup, nss_test_teardown), From 2b48c632357b1bf893ef68a5fd58353120501120 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]> Date: Tue, 11 Apr 2017 17:19:29 +0200 Subject: [PATCH 3/3] CACHE_REQ: Allow configurationless shortname lookups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Configurationless shortnames lookups must be allowed when a domains' resolution order is present and the (head) domain is not enforcing the usage of fully-qualified-names. With this patch SSSD does not require any kind of changes from client side for taking advantage of shortname lookups. Related: https://pagure.io/SSSD/sssd/issue/3001 Signed-off-by: Fabiano Fidêncio <[email protected]> --- src/responder/common/cache_req/cache_req.c | 2 +- src/responder/common/cache_req/cache_req_domain.c | 48 +++++++++++++++++++++++ src/responder/common/cache_req/cache_req_domain.h | 1 + 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/responder/common/cache_req/cache_req.c b/src/responder/common/cache_req/cache_req.c index 3a5fecf..797325a 100644 --- a/src/responder/common/cache_req/cache_req.c +++ b/src/responder/common/cache_req/cache_req.c @@ -480,7 +480,7 @@ static errno_t cache_req_search_domains_next(struct tevent_req *req) * qualified names on domain less search. We do not descend into * subdomains here since those are implicitly qualified. */ - if (state->check_next && !allow_no_fqn && domain->fqnames) { + if (state->check_next && !allow_no_fqn && state->cr_domain->fqnames) { state->cr_domain = state->cr_domain->next; continue; } diff --git a/src/responder/common/cache_req/cache_req_domain.c b/src/responder/common/cache_req/cache_req_domain.c index 86a88ef..bfdd2b7 100644 --- a/src/responder/common/cache_req/cache_req_domain.c +++ b/src/responder/common/cache_req/cache_req_domain.c @@ -60,6 +60,48 @@ void cache_req_domain_list_zfree(struct cache_req_domain **cr_domains) *cr_domains = NULL; } +static bool +cache_req_domain_use_fqnames(struct sss_domain_info *domain, + bool enforce_non_fqnames) +{ + struct sss_domain_info *head; + + head = get_domains_head(domain); + + /* + * In order to decide whether fully_qualified_names must be used on the + * lookups we have to take into consideration: + * - use_fully_qualified_name value of the head of the domains; + * (head->fqnames) + * - the presence of a domains' resolution order list; + * (non_fqnames_enforced) + * + * The relationship between those two can be described by: + * - head->fqnames: + * - true: in this case doesn't matter whether it's enforced or not, + * fully-qualified-names will _always_ be used + * - false: in this case (which is also the default case), the usage + * depends on it being enforced; + * + * - enforce_non_fqnames: + * - true: in this case, the usage of fully-qualified-names is not + * needed; + * - false: in this case, the usage of fully-qualified-names will be + * done accordingly to what's set for the domain itself. + */ + switch (head->fqnames) { + case true: + return true; + case false: + switch (enforce_non_fqnames) { + case true: + return false; + case false: + return domain->fqnames; + } + } +} + static struct cache_req_domain * cache_req_domain_new_list_from_string_list(TALLOC_CTX *mem_ctx, struct sss_domain_info *domains, @@ -71,9 +113,11 @@ cache_req_domain_new_list_from_string_list(TALLOC_CTX *mem_ctx, char *name; int flag = SSS_GND_ALL_DOMAINS; int i; + bool enforce_non_fqnames = false; errno_t ret; if (resolution_order != NULL) { + enforce_non_fqnames = true; for (i = 0; resolution_order[i] != NULL; i++) { name = resolution_order[i]; for (dom = domains; dom; dom = get_next_domain(dom, flag)) { @@ -87,6 +131,8 @@ cache_req_domain_new_list_from_string_list(TALLOC_CTX *mem_ctx, goto done; } cr_domain->domain = dom; + cr_domain->fqnames = + cache_req_domain_use_fqnames(dom, enforce_non_fqnames); DLIST_ADD_END(cr_domains, cr_domain, struct cache_req_domain *); @@ -106,6 +152,8 @@ cache_req_domain_new_list_from_string_list(TALLOC_CTX *mem_ctx, goto done; } cr_domain->domain = dom; + cr_domain->fqnames = + cache_req_domain_use_fqnames(dom, enforce_non_fqnames); DLIST_ADD_END(cr_domains, cr_domain, struct cache_req_domain *); } diff --git a/src/responder/common/cache_req/cache_req_domain.h b/src/responder/common/cache_req/cache_req_domain.h index 000087e..5bcbb9b 100644 --- a/src/responder/common/cache_req/cache_req_domain.h +++ b/src/responder/common/cache_req/cache_req_domain.h @@ -25,6 +25,7 @@ struct cache_req_domain { struct sss_domain_info *domain; + bool fqnames; struct cache_req_domain *prev; struct cache_req_domain *next;
_______________________________________________ sssd-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
