Blame SOURCES/0183-RESPONDER-Use-fqnames-as-output-when-needed.patch

bb7cd1
From 48b30d5a62e6af3d1f2b28eac3a2d39efa4349f1 Mon Sep 17 00:00:00 2001
bb7cd1
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio@redhat.com>
bb7cd1
Date: Mon, 19 Jun 2017 09:05:00 +0200
bb7cd1
Subject: [PATCH 183/186] RESPONDER: Use fqnames as output when needed
bb7cd1
MIME-Version: 1.0
bb7cd1
Content-Type: text/plain; charset=UTF-8
bb7cd1
Content-Transfer-Encoding: 8bit
bb7cd1
bb7cd1
As some regressions have been caused by not handling properly naming
bb7cd1
conflicts when using shortnames, last explicitly use fully qualified
bb7cd1
names as output in the following situations:
bb7cd1
- domain resolution order is set;
bb7cd1
- a trusted domain has been using `use_fully_qualified_name = false`
bb7cd1
bb7cd1
In both cases we want to ensure that even handling shortnames as input,
bb7cd1
the output will always be fully qualified.
bb7cd1
bb7cd1
As part of this patch, our tests ended up being modified to reflect the
bb7cd1
changes done. In other words, the tests related to shortnames now return
bb7cd1
expect as return a fully qualified name for trusted domains.
bb7cd1
bb7cd1
Resolves:
bb7cd1
https://pagure.io/SSSD/sssd/issue/3403
bb7cd1
bb7cd1
Signed-off-by: Fabiano FidĂȘncio <fidencio@redhat.com>
bb7cd1
bb7cd1
Reviewed-by: Jakub Hrozek <jhrozek@redhat.com>
bb7cd1
(cherry picked from commit 86526891366c4bc3e1ee861143b736d2670a6ba8)
bb7cd1
---
bb7cd1
 src/confdb/confdb.h                               |   1 +
bb7cd1
 src/db/sysdb_subdomains.c                         |   7 ++
bb7cd1
 src/responder/common/cache_req/cache_req_domain.c |  14 +++
bb7cd1
 src/responder/common/cache_req/cache_req_domain.h |   8 ++
bb7cd1
 src/tests/cmocka/test_nss_srv.c                   | 104 +++++++++-------------
bb7cd1
 src/util/usertools.c                              |   2 +-
bb7cd1
 6 files changed, 72 insertions(+), 64 deletions(-)
bb7cd1
bb7cd1
diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
bb7cd1
index 797353141edcccbf3341d161ca598c99492e54fe..32a422155abef428e8a75fc83a5fe14620c7028e 100644
bb7cd1
--- a/src/confdb/confdb.h
bb7cd1
+++ b/src/confdb/confdb.h
bb7cd1
@@ -291,6 +291,7 @@ struct sss_domain_info {
bb7cd1
     bool enumerate;
bb7cd1
     char **sd_enumerate;
bb7cd1
     bool fqnames;
bb7cd1
+    bool output_fqnames;
bb7cd1
     bool mpg;
bb7cd1
     bool ignore_group_members;
bb7cd1
     uint32_t id_min;
bb7cd1
diff --git a/src/db/sysdb_subdomains.c b/src/db/sysdb_subdomains.c
bb7cd1
index e2a4f7bb1fcdf20b6b7e04efc7f396d1c3d08f0f..2789cc4949fb7be9ad272d7613ed18a64fa8a20a 100644
bb7cd1
--- a/src/db/sysdb_subdomains.c
bb7cd1
+++ b/src/db/sysdb_subdomains.c
bb7cd1
@@ -129,6 +129,13 @@ struct sss_domain_info *new_subdomain(TALLOC_CTX *mem_ctx,
bb7cd1
     dom->mpg = mpg;
bb7cd1
     dom->state = DOM_ACTIVE;
bb7cd1
 
bb7cd1
+    /* use fully qualified names as output in order to avoid causing
bb7cd1
+     * conflicts with users who have the same name and either the
bb7cd1
+     * shortname user resolution is enabled or the trusted domain has
bb7cd1
+     * been explicitly set to use non-fully qualified names as input.
bb7cd1
+     */
bb7cd1
+    dom->output_fqnames = true;
bb7cd1
+
bb7cd1
     /* If the parent domain filters out group members, the subdomain should
bb7cd1
      * as well if configured */
bb7cd1
     inherit_option = string_in_list(CONFDB_DOMAIN_IGNORE_GROUP_MEMBERS,
bb7cd1
diff --git a/src/responder/common/cache_req/cache_req_domain.c b/src/responder/common/cache_req/cache_req_domain.c
bb7cd1
index 2c238c9966d322bb542fa2047313ee9e5144edee..b5f7f6c2ffabdbd92ee46b3020cee6ef7fec32d8 100644
bb7cd1
--- a/src/responder/common/cache_req/cache_req_domain.c
bb7cd1
+++ b/src/responder/common/cache_req/cache_req_domain.c
bb7cd1
@@ -136,6 +136,12 @@ cache_req_domain_new_list_from_string_list(TALLOC_CTX *mem_ctx,
bb7cd1
                 cr_domain->fqnames =
bb7cd1
                     cache_req_domain_use_fqnames(dom, enforce_non_fqnames);
bb7cd1
 
bb7cd1
+                /* when using the domain resolution order, using shortnames as
bb7cd1
+                 * input is allowed by default. However, we really want to use
bb7cd1
+                 * the fully qualified name as output in order to avoid
bb7cd1
+                 * conflicts whith users who have the very same name. */
bb7cd1
+                cr_domain->domain->output_fqnames = true;
bb7cd1
+
bb7cd1
                 DLIST_ADD_END(cr_domains, cr_domain,
bb7cd1
                               struct cache_req_domain *);
bb7cd1
                 break;
bb7cd1
@@ -159,6 +165,14 @@ cache_req_domain_new_list_from_string_list(TALLOC_CTX *mem_ctx,
bb7cd1
         cr_domain->fqnames =
bb7cd1
             cache_req_domain_use_fqnames(dom, enforce_non_fqnames);
bb7cd1
 
bb7cd1
+        /* when using the domain resolution order, using shortnames as input
bb7cd1
+         * is allowed by default. However, we really want to use the fully
bb7cd1
+         * qualified name as output in order to avoid conflicts whith users
bb7cd1
+         * who have the very same name. */
bb7cd1
+        if (resolution_order != NULL) {
bb7cd1
+            cr_domain->domain->output_fqnames = true;
bb7cd1
+        }
bb7cd1
+
bb7cd1
         DLIST_ADD_END(cr_domains, cr_domain, struct cache_req_domain *);
bb7cd1
     }
bb7cd1
 
bb7cd1
diff --git a/src/responder/common/cache_req/cache_req_domain.h b/src/responder/common/cache_req/cache_req_domain.h
bb7cd1
index 5bcbb9b493caf05bf71aac5cf7633ded91f22e73..3780a5d8d88d76e100738d28d1dd0e697edf5eae 100644
bb7cd1
--- a/src/responder/common/cache_req/cache_req_domain.h
bb7cd1
+++ b/src/responder/common/cache_req/cache_req_domain.h
bb7cd1
@@ -35,6 +35,14 @@ struct cache_req_domain *
bb7cd1
 cache_req_domain_get_domain_by_name(struct cache_req_domain *domains,
bb7cd1
                                     const char *name);
bb7cd1
 
bb7cd1
+/*
bb7cd1
+ * This function may have a side effect of setting the output_fqnames' domain
bb7cd1
+ * property when it's called.
bb7cd1
+ *
bb7cd1
+ * It happens as the output_fqnames' domain property must only be set depending
bb7cd1
+ * on whether a domain resolution order is set or not, and the saner place to
bb7cd1
+ * set it to all domains is when flattening those (thus, in this function).
bb7cd1
+ */
bb7cd1
 errno_t
bb7cd1
 cache_req_domain_new_list_from_domain_resolution_order(
bb7cd1
                                         TALLOC_CTX *mem_ctx,
bb7cd1
diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c
bb7cd1
index 03b5bcc302322551a32f5b8cfe4b7698947abbe7..ccedf96beaecfaa4232bbe456d5e5a8394098483 100644
bb7cd1
--- a/src/tests/cmocka/test_nss_srv.c
bb7cd1
+++ b/src/tests/cmocka/test_nss_srv.c
bb7cd1
@@ -1648,29 +1648,23 @@ static int test_nss_getgrnam_members_check_subdom(uint32_t status,
bb7cd1
     tmp_ctx = talloc_new(nss_test_ctx);
bb7cd1
     assert_non_null(tmp_ctx);
bb7cd1
 
bb7cd1
-    if (nss_test_ctx->subdom->fqnames) {
bb7cd1
-        exp_members[0] = sss_tc_fqname(tmp_ctx,
bb7cd1
-                                       nss_test_ctx->subdom->names,
bb7cd1
-                                       nss_test_ctx->subdom,
bb7cd1
-                                       submember1.pw_name);
bb7cd1
-        assert_non_null(exp_members[0]);
bb7cd1
+    exp_members[0] = sss_tc_fqname(tmp_ctx,
bb7cd1
+                                   nss_test_ctx->subdom->names,
bb7cd1
+                                   nss_test_ctx->subdom,
bb7cd1
+                                   submember1.pw_name);
bb7cd1
+    assert_non_null(exp_members[0]);
bb7cd1
 
bb7cd1
-        exp_members[1] = sss_tc_fqname(tmp_ctx,
bb7cd1
-                                       nss_test_ctx->subdom->names,
bb7cd1
-                                       nss_test_ctx->subdom,
bb7cd1
-                                       submember2.pw_name);
bb7cd1
-        assert_non_null(exp_members[1]);
bb7cd1
+    exp_members[1] = sss_tc_fqname(tmp_ctx,
bb7cd1
+                                   nss_test_ctx->subdom->names,
bb7cd1
+                                   nss_test_ctx->subdom,
bb7cd1
+                                   submember2.pw_name);
bb7cd1
+    assert_non_null(exp_members[1]);
bb7cd1
 
bb7cd1
-        expected.gr_name = sss_tc_fqname(tmp_ctx,
bb7cd1
-                                         nss_test_ctx->subdom->names,
bb7cd1
-                                         nss_test_ctx->subdom,
bb7cd1
-                                         testsubdomgroup.gr_name);
bb7cd1
-        assert_non_null(expected.gr_name);
bb7cd1
-    } else {
bb7cd1
-        exp_members[0] = submember1.pw_name;
bb7cd1
-        exp_members[1] = submember2.pw_name;
bb7cd1
-        expected.gr_name = testsubdomgroup.gr_name;
bb7cd1
-    }
bb7cd1
+    expected.gr_name = sss_tc_fqname(tmp_ctx,
bb7cd1
+                                     nss_test_ctx->subdom->names,
bb7cd1
+                                     nss_test_ctx->subdom,
bb7cd1
+                                     testsubdomgroup.gr_name);
bb7cd1
+    assert_non_null(expected.gr_name);
bb7cd1
 
bb7cd1
     assert_int_equal(status, EOK);
bb7cd1
 
bb7cd1
@@ -1744,15 +1738,11 @@ static int test_nss_getgrnam_check_mix_dom(uint32_t status,
bb7cd1
     tmp_ctx = talloc_new(nss_test_ctx);
bb7cd1
     assert_non_null(tmp_ctx);
bb7cd1
 
bb7cd1
-    if (nss_test_ctx->subdom->fqnames) {
bb7cd1
-        exp_members[0] = sss_tc_fqname(tmp_ctx,
bb7cd1
-                                       nss_test_ctx->subdom->names,
bb7cd1
-                                       nss_test_ctx->subdom,
bb7cd1
-                                       submember1.pw_name);
bb7cd1
-        assert_non_null(exp_members[0]);
bb7cd1
-    } else {
bb7cd1
-        exp_members[0] = submember1.pw_name;
bb7cd1
-    }
bb7cd1
+    exp_members[0] = sss_tc_fqname(tmp_ctx,
bb7cd1
+                                   nss_test_ctx->subdom->names,
bb7cd1
+                                   nss_test_ctx->subdom,
bb7cd1
+                                   submember1.pw_name);
bb7cd1
+    assert_non_null(exp_members[0]);
bb7cd1
     exp_members[1] = testmember1.pw_name;
bb7cd1
     exp_members[2] = testmember2.pw_name;
bb7cd1
 
bb7cd1
@@ -1840,15 +1830,12 @@ static int test_nss_getgrnam_check_mix_dom_fqdn(uint32_t status,
bb7cd1
     tmp_ctx = talloc_new(nss_test_ctx);
bb7cd1
     assert_non_null(tmp_ctx);
bb7cd1
 
bb7cd1
-    if (nss_test_ctx->subdom->fqnames) {
bb7cd1
-        exp_members[0] = sss_tc_fqname(tmp_ctx,
bb7cd1
-                                       nss_test_ctx->subdom->names,
bb7cd1
-                                       nss_test_ctx->subdom,
bb7cd1
-                                       submember1.pw_name);
bb7cd1
-        assert_non_null(exp_members[0]);
bb7cd1
-    } else {
bb7cd1
-        exp_members[0] = submember1.pw_name;
bb7cd1
-    }
bb7cd1
+    exp_members[0] = sss_tc_fqname(tmp_ctx,
bb7cd1
+                                   nss_test_ctx->subdom->names,
bb7cd1
+                                   nss_test_ctx->subdom,
bb7cd1
+                                   submember1.pw_name);
bb7cd1
+    assert_non_null(exp_members[0]);
bb7cd1
+
bb7cd1
     if (nss_test_ctx->tctx->dom->fqnames) {
bb7cd1
         exp_members[1] = sss_tc_fqname(tmp_ctx, nss_test_ctx->tctx->dom->names,
bb7cd1
                                        nss_test_ctx->tctx->dom, testmember1.pw_name);
bb7cd1
@@ -1961,37 +1948,28 @@ static int test_nss_getgrnam_check_mix_subdom(uint32_t status,
bb7cd1
     tmp_ctx = talloc_new(nss_test_ctx);
bb7cd1
     assert_non_null(tmp_ctx);
bb7cd1
 
bb7cd1
-    if (nss_test_ctx->subdom->fqnames) {
bb7cd1
-        exp_members[0] = sss_tc_fqname(tmp_ctx,
bb7cd1
-                                       nss_test_ctx->subdom->names,
bb7cd1
-                                       nss_test_ctx->subdom,
bb7cd1
-                                       submember1.pw_name);
bb7cd1
-        assert_non_null(exp_members[0]);
bb7cd1
+    exp_members[0] = sss_tc_fqname(tmp_ctx,
bb7cd1
+                                   nss_test_ctx->subdom->names,
bb7cd1
+                                   nss_test_ctx->subdom,
bb7cd1
+                                   submember1.pw_name);
bb7cd1
+    assert_non_null(exp_members[0]);
bb7cd1
 
bb7cd1
-        exp_members[1] = sss_tc_fqname(tmp_ctx,
bb7cd1
-                                       nss_test_ctx->subdom->names,
bb7cd1
-                                       nss_test_ctx->subdom,
bb7cd1
-                                       submember2.pw_name);
bb7cd1
-        assert_non_null(exp_members[1]);
bb7cd1
-    } else {
bb7cd1
-        exp_members[0] = submember1.pw_name;
bb7cd1
-        exp_members[1] = submember2.pw_name;
bb7cd1
-    }
bb7cd1
+    exp_members[1] = sss_tc_fqname(tmp_ctx,
bb7cd1
+                                   nss_test_ctx->subdom->names,
bb7cd1
+                                   nss_test_ctx->subdom,
bb7cd1
+                                   submember2.pw_name);
bb7cd1
+    assert_non_null(exp_members[1]);
bb7cd1
 
bb7cd1
     /* Important: this member is from a non-qualified domain, so his name will
bb7cd1
      * not be qualified either
bb7cd1
      */
bb7cd1
     exp_members[2] = testmember1.pw_name;
bb7cd1
 
bb7cd1
-    if (nss_test_ctx->subdom->fqnames) {
bb7cd1
-        expected.gr_name = sss_tc_fqname(tmp_ctx,
bb7cd1
-                                         nss_test_ctx->subdom->names,
bb7cd1
-                                         nss_test_ctx->subdom,
bb7cd1
-                                         testsubdomgroup.gr_name);
bb7cd1
-        assert_non_null(expected.gr_name);
bb7cd1
-    } else {
bb7cd1
-        expected.gr_name = testsubdomgroup.gr_name;
bb7cd1
-    }
bb7cd1
+    expected.gr_name = sss_tc_fqname(tmp_ctx,
bb7cd1
+                                     nss_test_ctx->subdom->names,
bb7cd1
+                                     nss_test_ctx->subdom,
bb7cd1
+                                     testsubdomgroup.gr_name);
bb7cd1
+    assert_non_null(expected.gr_name);
bb7cd1
 
bb7cd1
     assert_int_equal(status, EOK);
bb7cd1
 
bb7cd1
diff --git a/src/util/usertools.c b/src/util/usertools.c
bb7cd1
index 5dfe6d7765b8032c7447de75e10c6c2a1d4c49ec..83131da1cac25e60a5ec3fffa995a545673e53b9 100644
bb7cd1
--- a/src/util/usertools.c
bb7cd1
+++ b/src/util/usertools.c
bb7cd1
@@ -867,7 +867,7 @@ int sss_output_fqname(TALLOC_CTX *mem_ctx,
bb7cd1
         goto done;
bb7cd1
     }
bb7cd1
 
bb7cd1
-    if (domain->fqnames) {
bb7cd1
+    if (domain->output_fqnames || domain->fqnames) {
bb7cd1
         output_name = sss_tc_fqname(tmp_ctx, domain->names,
bb7cd1
                                     domain, output_name);
bb7cd1
         if (output_name == NULL) {
bb7cd1
-- 
bb7cd1
2.9.4
bb7cd1