Blob Blame History Raw
From b7d2310e9ddd79bfdea2bc334bd11d4df9be37a2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio@redhat.com>
Date: Wed, 12 Apr 2017 10:43:25 +0200
Subject: [PATCH 105/110] 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 <fidencio@redhat.com>

Reviewed-by: Pavel Březina <pbrezina@redhat.com>
(cherry picked from commit a3faad0e4dc1ca4473746c3822ecfc5aed876e6d)
---
 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           | 108 +++++++++++++---------
 3 files changed, 74 insertions(+), 53 deletions(-)

diff --git a/src/responder/common/cache_req/cache_req_domain.c b/src/responder/common/cache_req/cache_req_domain.c
index bbabd695f1c6b6c29b7e61f571382ab9adfb0ea2..86a88efd54ca0f4a0748b44ece1b8515438d4628 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 @@ done:
     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 41c50e8c293d7b032cb2f05482c40e93e4f723dc..000087e5ca2074f22169a4af627810f4f287e430 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 ac6320b08de09bc6c7e8dd1af72e0a493a449f7a..62b71b5104fdbb585d086d44d2ca2ab9717dd788 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -1486,10 +1486,11 @@ fail:
 }
 
 /* ====== 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;
@@ -1498,7 +1499,7 @@ sss_resp_new_cr_domains_from_ipa_id_view(TALLOC_CTX *mem_ctx,
 
     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 +1511,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 +1525,31 @@ sss_resp_new_cr_domains_from_ipa_id_view(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
+    *_cr_domains = cr_domains;
+
+    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 +1562,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 +1576,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 +1590,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 +1610,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 +1620,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;
 
-- 
2.9.3