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: opened

PR body:
"""
This patch set contains:
- One patch fixing a fallback issue related to domain resolution order;
- One patch adding the ability to use the "shortnames" feature without 
requiring any configuration from the client side;

Please, see the commit messages for detailed information.


"""

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 a0c38c25e777d286c27331d320a90b20d37b7a4b 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/2] 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 |  15 ++-
 src/responder/common/cache_req/cache_req_domain.h |   5 +-
 src/responder/common/responder_common.c           | 109 +++++++++++++---------
 3 files changed, 77 insertions(+), 52 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..f6f61bd 100644
--- a/src/responder/common/cache_req/cache_req_domain.c
+++ b/src/responder/common/cache_req/cache_req_domain.c
@@ -120,11 +120,12 @@ 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;
@@ -133,7 +134,7 @@ cache_req_domain_new_list_from_domain_resolution_order(
 
     tmp_ctx = talloc_new(NULL);
     if (tmp_ctx == NULL) {
-        return NULL;
+        return ENOMEM;
     }
 
     if (domain_resolution_order != NULL) {
@@ -149,7 +150,7 @@ cache_req_domain_new_list_from_domain_resolution_order(
         }
     }
 
-    cr_domains = cache_req_domain_new_list_from_string_list(mem_ctx, domains,
+    cr_domains = cache_req_domain_new_list_from_string_list(tmp_ctx, domains,
                                                             list);
     if (cr_domains == NULL) {
         ret = ENOMEM;
@@ -157,10 +158,14 @@ cache_req_domain_new_list_from_domain_resolution_order(
               "cache_req_domain_new_list_from_domain_resolution_order() "
               "failed [%d]: [%s].\n",
               ret, sss_strerror(ret));
+        *_cr_domains = NULL;
         goto done;
     }
 
+    *_cr_domains = talloc_steal(mem_ctx, 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..c9f738d 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -1486,19 +1486,22 @@ 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 +1513,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(
+                        tmp_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 +1527,31 @@ sss_resp_new_cr_domains_from_ipa_id_view(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
+    *_cr_domains = talloc_steal(mem_ctx, 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 +1564,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(
+                        tmp_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 +1578,12 @@ sss_resp_new_cr_domains_from_ipa_config(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
+    *_cr_domains = talloc_steal(mem_ctx, cr_domains);
+    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 +1593,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 +1613,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 +1623,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 2251f16ec07acc8f6bd21388453e61ff844815e1 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 2/2] 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 f6f61bd..7c9270a 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]

Reply via email to