URL: https://github.com/SSSD/sssd/pull/311 Author: fidencio Title: #311: RESPONDER: Use fqnames as output when needed Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/311/head:pr311 git checkout pr311
From 440cd890eb835e020488ce17597e1321cde9a1c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 19 Jun 2017 09:05:00 +0200 Subject: [PATCH 1/2] RESPONDER: Use fqnames as output when needed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As some regressions have been caused by not handling properly naming conflicts when using shortnames, last explicitly use fully qualified names as output in the following situations: - domain resolution order is set; - a trusted domain has been using `use_fully_qualified_name = false` In both cases we want to ensure that even handling shortnames as input, the output will always be fully qualified. As part of this patch, our tests ended up being modified to reflect the changes done. In other words, the tests related to shortnames now return expect as return a fully qualified name for trusted domains. Resolves: https://pagure.io/SSSD/sssd/issue/3403 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/confdb/confdb.h | 1 + src/db/sysdb_subdomains.c | 7 ++ src/responder/common/cache_req/cache_req_domain.c | 14 +++ src/tests/cmocka/test_nss_srv.c | 108 +++++++++------------- src/util/usertools.c | 2 +- 5 files changed, 66 insertions(+), 66 deletions(-) diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h index 797353141..32a422155 100644 --- a/src/confdb/confdb.h +++ b/src/confdb/confdb.h @@ -291,6 +291,7 @@ struct sss_domain_info { bool enumerate; char **sd_enumerate; bool fqnames; + bool output_fqnames; bool mpg; bool ignore_group_members; uint32_t id_min; diff --git a/src/db/sysdb_subdomains.c b/src/db/sysdb_subdomains.c index e2a4f7bb1..2789cc494 100644 --- a/src/db/sysdb_subdomains.c +++ b/src/db/sysdb_subdomains.c @@ -129,6 +129,13 @@ struct sss_domain_info *new_subdomain(TALLOC_CTX *mem_ctx, dom->mpg = mpg; dom->state = DOM_ACTIVE; + /* use fully qualified names as output in order to avoid causing + * conflicts with users who have the same name and either the + * shortname user resolution is enabled or the trusted domain has + * been explicitly set to use non-fully qualified names as input. + */ + dom->output_fqnames = true; + /* If the parent domain filters out group members, the subdomain should * as well if configured */ inherit_option = string_in_list(CONFDB_DOMAIN_IGNORE_GROUP_MEMBERS, diff --git a/src/responder/common/cache_req/cache_req_domain.c b/src/responder/common/cache_req/cache_req_domain.c index 8bf7fc6dc..bad4bf9a6 100644 --- a/src/responder/common/cache_req/cache_req_domain.c +++ b/src/responder/common/cache_req/cache_req_domain.c @@ -132,6 +132,12 @@ cache_req_domain_new_list_from_string_list(TALLOC_CTX *mem_ctx, cr_domain->fqnames = cache_req_domain_use_fqnames(dom, enforce_non_fqnames); + /* when using the domain resolution order, using shortnames as + * input is allowed by default. However, we really want to use + * the fully qualified name as output in order to avoid + * conflicts whith users who have the very same name. */ + cr_domain->domain->output_fqnames = true; + DLIST_ADD_END(cr_domains, cr_domain, struct cache_req_domain *); break; @@ -155,6 +161,14 @@ cache_req_domain_new_list_from_string_list(TALLOC_CTX *mem_ctx, cr_domain->fqnames = cache_req_domain_use_fqnames(dom, enforce_non_fqnames); + /* when using the domain resolution order, using shortnames as input + * is allowed by default. However, we really want to use the fully + * qualified name as output in order to avoid conflicts whith users + * who have the very same name. */ + if (resolution_order != NULL) { + cr_domain->domain->output_fqnames = true; + } + DLIST_ADD_END(cr_domains, cr_domain, struct cache_req_domain *); } diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c index 03b5bcc30..ccedf96be 100644 --- a/src/tests/cmocka/test_nss_srv.c +++ b/src/tests/cmocka/test_nss_srv.c @@ -1648,29 +1648,23 @@ static int test_nss_getgrnam_members_check_subdom(uint32_t status, tmp_ctx = talloc_new(nss_test_ctx); assert_non_null(tmp_ctx); - if (nss_test_ctx->subdom->fqnames) { - 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); - assert_non_null(exp_members[1]); + 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]); - expected.gr_name = sss_tc_fqname(tmp_ctx, - nss_test_ctx->subdom->names, - nss_test_ctx->subdom, - testsubdomgroup.gr_name); - assert_non_null(expected.gr_name); - } else { - exp_members[0] = submember1.pw_name; - exp_members[1] = submember2.pw_name; - expected.gr_name = testsubdomgroup.gr_name; - } + exp_members[1] = sss_tc_fqname(tmp_ctx, + nss_test_ctx->subdom->names, + nss_test_ctx->subdom, + submember2.pw_name); + assert_non_null(exp_members[1]); + + expected.gr_name = sss_tc_fqname(tmp_ctx, + nss_test_ctx->subdom->names, + nss_test_ctx->subdom, + testsubdomgroup.gr_name); + assert_non_null(expected.gr_name); assert_int_equal(status, EOK); @@ -1744,15 +1738,11 @@ static int test_nss_getgrnam_check_mix_dom(uint32_t status, tmp_ctx = talloc_new(nss_test_ctx); assert_non_null(tmp_ctx); - if (nss_test_ctx->subdom->fqnames) { - 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]); - } else { - exp_members[0] = submember1.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] = testmember1.pw_name; exp_members[2] = testmember2.pw_name; @@ -1840,15 +1830,12 @@ static int test_nss_getgrnam_check_mix_dom_fqdn(uint32_t status, tmp_ctx = talloc_new(nss_test_ctx); assert_non_null(tmp_ctx); - if (nss_test_ctx->subdom->fqnames) { - 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]); - } else { - exp_members[0] = submember1.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]); + if (nss_test_ctx->tctx->dom->fqnames) { exp_members[1] = sss_tc_fqname(tmp_ctx, nss_test_ctx->tctx->dom->names, nss_test_ctx->tctx->dom, testmember1.pw_name); @@ -1961,37 +1948,28 @@ static int test_nss_getgrnam_check_mix_subdom(uint32_t status, tmp_ctx = talloc_new(nss_test_ctx); assert_non_null(tmp_ctx); - if (nss_test_ctx->subdom->fqnames) { - 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); - assert_non_null(exp_members[1]); - } else { - exp_members[0] = submember1.pw_name; - exp_members[1] = submember2.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->subdom->names, + nss_test_ctx->subdom, + submember2.pw_name); + assert_non_null(exp_members[1]); /* Important: this member is from a non-qualified domain, so his name will * not be qualified either */ exp_members[2] = testmember1.pw_name; - if (nss_test_ctx->subdom->fqnames) { - expected.gr_name = sss_tc_fqname(tmp_ctx, - nss_test_ctx->subdom->names, - nss_test_ctx->subdom, - testsubdomgroup.gr_name); - assert_non_null(expected.gr_name); - } else { - expected.gr_name = testsubdomgroup.gr_name; - } + expected.gr_name = sss_tc_fqname(tmp_ctx, + nss_test_ctx->subdom->names, + nss_test_ctx->subdom, + testsubdomgroup.gr_name); + assert_non_null(expected.gr_name); assert_int_equal(status, EOK); diff --git a/src/util/usertools.c b/src/util/usertools.c index 5dfe6d776..83131da1c 100644 --- a/src/util/usertools.c +++ b/src/util/usertools.c @@ -867,7 +867,7 @@ int sss_output_fqname(TALLOC_CTX *mem_ctx, goto done; } - if (domain->fqnames) { + if (domain->output_fqnames || domain->fqnames) { output_name = sss_tc_fqname(tmp_ctx, domain->names, domain, output_name); if (output_name == NULL) { From 3824fa6e1965a1e981b9af06162fddad045ee4e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Tue, 20 Jun 2017 14:22:48 +0200 Subject: [PATCH 2/2] DOMAIN: Add sss_domain_info_set_output_fqnames() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let's avoid setting a domain's property directly from cr_domain code. In order to do so, let's introduce a setter, which may help us in the future whenever we decide to make sss_domain_info an opaque structure. Related: https://pagure.io/SSSD/sssd/issue/3403 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/responder/common/cache_req/cache_req_domain.c | 4 ++-- src/util/domain_info_utils.c | 6 ++++++ src/util/util.h | 3 +++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/responder/common/cache_req/cache_req_domain.c b/src/responder/common/cache_req/cache_req_domain.c index bad4bf9a6..7b58f7c94 100644 --- a/src/responder/common/cache_req/cache_req_domain.c +++ b/src/responder/common/cache_req/cache_req_domain.c @@ -136,7 +136,7 @@ cache_req_domain_new_list_from_string_list(TALLOC_CTX *mem_ctx, * input is allowed by default. However, we really want to use * the fully qualified name as output in order to avoid * conflicts whith users who have the very same name. */ - cr_domain->domain->output_fqnames = true; + sss_domain_info_set_output_fqnames(cr_domain->domain, true); DLIST_ADD_END(cr_domains, cr_domain, struct cache_req_domain *); @@ -166,7 +166,7 @@ cache_req_domain_new_list_from_string_list(TALLOC_CTX *mem_ctx, * qualified name as output in order to avoid conflicts whith users * who have the very same name. */ if (resolution_order != NULL) { - cr_domain->domain->output_fqnames = true; + sss_domain_info_set_output_fqnames(cr_domain->domain, true); } DLIST_ADD_END(cr_domains, cr_domain, struct cache_req_domain *); diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c index 46375656c..c06490102 100644 --- a/src/util/domain_info_utils.c +++ b/src/util/domain_info_utils.c @@ -905,3 +905,9 @@ const char *sss_domain_type_str(struct sss_domain_info *dom) } return "Unknown"; } + +void sss_domain_info_set_output_fqnames(struct sss_domain_info *domain, + bool output_fqnames) +{ + domain->output_fqnames = output_fqnames; +} diff --git a/src/util/util.h b/src/util/util.h index 2434d9b3c..c4f6c755a 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -562,6 +562,9 @@ errno_t sssd_domain_init(TALLOC_CTX *mem_ctx, const char *db_path, struct sss_domain_info **_domain); +void sss_domain_info_set_output_fqnames(struct sss_domain_info *domain, + bool output_fqname); + #define IS_SUBDOMAIN(dom) ((dom)->parent != NULL) #define DOM_HAS_VIEWS(dom) ((dom)->has_views)
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
