URL: https://github.com/SSSD/sssd/pull/462
Author: lslebodn
 Title: #462: confdb: Do not start implicit_files with proxy domain
Action: opened

PR body:
"""
id_provider = proxy + proxy_lib_name = files is equivalent
to id_provider = files. But requests to user hit implicit_files
domain instead of proxy domain and therefore it broke usage
of proxy domain with auth_provider = krb5.
    
Resolves:
https://pagure.io/SSSD/sssd/issue/3590
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/462/head:pr462
git checkout pr462
From 67ff670c9f81c2c569da788fbf9c18df3451bca6 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Wed, 29 Nov 2017 17:57:56 +0100
Subject: [PATCH 1/4] confdb: Move detection files to separate function

---
 src/confdb/confdb.c | 47 ++++++++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
index a02822481..c41bd5087 100644
--- a/src/confdb/confdb.c
+++ b/src/confdb/confdb.c
@@ -1718,15 +1718,39 @@ int confdb_get_sub_sections(TALLOC_CTX *mem_ctx,
     return ret;
 }
 
+static bool need_implicit_files_domain(TALLOC_CTX *tmp_ctx,
+                                       struct ldb_result *doms)
+{
+    const char *id_provider = NULL;
+    unsigned int i;
+
+    for (i = 0; i < doms->count; i++) {
+        id_provider = ldb_msg_find_attr_as_string(doms->msgs[i],
+                                                  CONFDB_DOMAIN_ID_PROVIDER,
+                                                  NULL);
+        if (id_provider == NULL) {
+            DEBUG(SSSDBG_OP_FAILURE,
+                  "The object [%s] doesn't have a id_provider\n",
+                  ldb_dn_get_linearized(doms->msgs[i]->dn));
+            continue;
+        }
+
+        if (strcasecmp(id_provider, "files") == 0) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
 static int confdb_has_files_domain(struct confdb_ctx *cdb)
 {
     TALLOC_CTX *tmp_ctx = NULL;
     struct ldb_dn *dn = NULL;
     struct ldb_result *res = NULL;
     static const char *attrs[] = { CONFDB_DOMAIN_ID_PROVIDER, NULL };
-    const char *id_provider = NULL;
     int ret;
-    unsigned int i;
+    bool need_files_dom;
 
     tmp_ctx = talloc_new(NULL);
     if (tmp_ctx == NULL) {
@@ -1746,24 +1770,9 @@ static int confdb_has_files_domain(struct confdb_ctx *cdb)
         goto done;
     }
 
-    for (i = 0; i < res->count; i++) {
-        id_provider = ldb_msg_find_attr_as_string(res->msgs[i],
-                                                  CONFDB_DOMAIN_ID_PROVIDER,
-                                                  NULL);
-        if (id_provider == NULL) {
-            DEBUG(SSSDBG_CRIT_FAILURE,
-                  "The object [%s] doesn't have a id_provider\n",
-                  ldb_dn_get_linearized(res->msgs[i]->dn));
-            ret = EINVAL;
-            goto done;
-        }
-
-        if (strcasecmp(id_provider, "files") == 0) {
-            break;
-        }
-    }
+    need_files_dom = need_implicit_files_domain(tmp_ctx, res);
 
-    ret = i < res->count ? EOK : ENOENT;
+    ret = need_files_dom ? ENOENT : EOK;
 done:
     talloc_free(tmp_ctx);
     return ret;

From 0110427eaf3386c3ed970feedb0cc4adc35c473d Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Wed, 29 Nov 2017 20:02:35 +0100
Subject: [PATCH 2/4] confdb: Fix starting of implicit files domain

We did not start implicit_files domain when sssd configuration
contains files domain which was disabled.
---
 src/confdb/confdb.c                   | 36 +++++++++++++++++++++++++++++++++--
 src/tests/intg/test_files_provider.py |  3 +++
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
index c41bd5087..ef1be4a6e 100644
--- a/src/confdb/confdb.c
+++ b/src/confdb/confdb.c
@@ -1719,12 +1719,43 @@ int confdb_get_sub_sections(TALLOC_CTX *mem_ctx,
 }
 
 static bool need_implicit_files_domain(TALLOC_CTX *tmp_ctx,
+                                       struct confdb_ctx *cdb,
                                        struct ldb_result *doms)
 {
     const char *id_provider = NULL;
     unsigned int i;
+    errno_t ret;
+    char **domlist;
+    const char *val;
+
+    ret = confdb_get_string_as_list(cdb, tmp_ctx,
+                                    CONFDB_MONITOR_CONF_ENTRY,
+                                    CONFDB_MONITOR_ACTIVE_DOMAINS,
+                                    &domlist);
+    if (ret == ENOENT) {
+        return true;
+    } else if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              "Cannot get active domains %d[%s]\n",
+              ret, sss_strerror(ret));
+        return false;
+    }
 
     for (i = 0; i < doms->count; i++) {
+        val = ldb_msg_find_attr_as_string(doms->msgs[i], CONFDB_DOMAIN_ATTR,
+                                          NULL);
+        if (val == NULL) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  "The object [%s] doesn't have a name\n",
+                  ldb_dn_get_linearized(doms->msgs[i]->dn));
+            continue;
+        }
+
+        /* skip disabled domain */
+        if (!string_in_list(val, domlist, false)) {
+            continue;
+        }
+
         id_provider = ldb_msg_find_attr_as_string(doms->msgs[i],
                                                   CONFDB_DOMAIN_ID_PROVIDER,
                                                   NULL);
@@ -1748,7 +1779,8 @@ static int confdb_has_files_domain(struct confdb_ctx *cdb)
     TALLOC_CTX *tmp_ctx = NULL;
     struct ldb_dn *dn = NULL;
     struct ldb_result *res = NULL;
-    static const char *attrs[] = { CONFDB_DOMAIN_ID_PROVIDER, NULL };
+    static const char *attrs[] = { CONFDB_DOMAIN_ID_PROVIDER,
+                                   CONFDB_DOMAIN_ATTR, NULL };
     int ret;
     bool need_files_dom;
 
@@ -1770,7 +1802,7 @@ static int confdb_has_files_domain(struct confdb_ctx *cdb)
         goto done;
     }
 
-    need_files_dom = need_implicit_files_domain(tmp_ctx, res);
+    need_files_dom = need_implicit_files_domain(tmp_ctx, cdb, res);
 
     ret = need_files_dom ? ENOENT : EOK;
 done:
diff --git a/src/tests/intg/test_files_provider.py b/src/tests/intg/test_files_provider.py
index e507ea10d..169da7137 100644
--- a/src/tests/intg/test_files_provider.py
+++ b/src/tests/intg/test_files_provider.py
@@ -167,6 +167,9 @@ def no_files_domain(request):
 
         [domain/local]
         id_provider = local
+
+        [domain/disabled.files]
+        id_provider = files
     """).format(**locals())
     create_conf_fixture(request, conf)
     create_sssd_fixture(request)

From b0154c2905983fd058a41a0e881230818a8bc0cc Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Thu, 30 Nov 2017 07:59:33 +0100
Subject: [PATCH 3/4] confdb: Do not start implicit_files with proxy domain

id_provider = proxy + proxy_lib_name = files is equivalent
to id_provider = files. But requests to user hit implicit_files
domain instead of proxy domain and therefore it broke usage
of proxy domain with auth_provider = krb5.

Resolves:
https://pagure.io/SSSD/sssd/issue/3590
---
 src/confdb/confdb.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
index ef1be4a6e..0a4be57e0 100644
--- a/src/confdb/confdb.c
+++ b/src/confdb/confdb.c
@@ -1769,6 +1769,25 @@ static bool need_implicit_files_domain(TALLOC_CTX *tmp_ctx,
         if (strcasecmp(id_provider, "files") == 0) {
             return false;
         }
+
+        if (strcasecmp(id_provider, "proxy") == 0) {
+            val = ldb_msg_find_attr_as_string(doms->msgs[i],
+                                              CONFDB_PROXY_LIBNAME, NULL);
+            if (val == NULL) {
+                DEBUG(SSSDBG_OP_FAILURE,
+                      "The object [%s] doesn't have proxy_lib_name with "
+                      "id_provider proxy\n",
+                      ldb_dn_get_linearized(doms->msgs[i]->dn));
+                continue;
+            }
+
+            /* id_provider = proxy + proxy_lib_name = files is equivalent
+             * to id_provider = files
+             */
+            if (strcmp(val, "files") == 0) {
+                return false;
+            }
+        }
     }
 
     return true;
@@ -1780,7 +1799,8 @@ static int confdb_has_files_domain(struct confdb_ctx *cdb)
     struct ldb_dn *dn = NULL;
     struct ldb_result *res = NULL;
     static const char *attrs[] = { CONFDB_DOMAIN_ID_PROVIDER,
-                                   CONFDB_DOMAIN_ATTR, NULL };
+                                   CONFDB_DOMAIN_ATTR,
+                                   CONFDB_PROXY_LIBNAME, NULL };
     int ret;
     bool need_files_dom;
 

From 0ad9a8dbc59de835766bc2d8d6c42542a4f782a0 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Thu, 30 Nov 2017 10:21:17 +0100
Subject: [PATCH 4/4] test_files_provider: Regression test for implicit_files +
 proxy

Related to:
https://pagure.io/SSSD/sssd/issue/3590
---
 src/tests/intg/test_files_provider.py | 40 +++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/src/tests/intg/test_files_provider.py b/src/tests/intg/test_files_provider.py
index 169da7137..ea4e5b70a 100644
--- a/src/tests/intg/test_files_provider.py
+++ b/src/tests/intg/test_files_provider.py
@@ -145,6 +145,26 @@ def files_domain_only(request):
     return None
 
 
+@pytest.fixture
+def proxy_to_files_domain_only(request):
+    conf = unindent("""\
+        [sssd]
+        domains             = proxy, local
+        services            = nss
+
+        [domain/local]
+        id_provider = local
+
+        [domain/proxy]
+        id_provider = proxy
+        proxy_lib_name = files
+        auth_provider = none
+    """).format(**locals())
+    create_conf_fixture(request, conf)
+    create_sssd_fixture(request)
+    return None
+
+
 @pytest.fixture
 def no_sssd_domain(request):
     conf = unindent("""\
@@ -980,6 +1000,26 @@ def test_no_sssd_domain(add_user_with_canary, no_sssd_domain):
     assert user == USER1
 
 
+def test_proxy_to_files_domain_only(add_user_with_canary,
+                                    proxy_to_files_domain_only):
+    """
+    Test that implicit_files domain is not started together with proxy to files
+    """
+    local_user1 = dict(name='user1', passwd='*', uid=10009, gid=10009,
+                       gecos='user1', dir='/home/user1', shell='/bin/bash')
+
+    # Add a user with a different UID than the one in files
+    subprocess.check_call(
+        ["sss_useradd", "-u", "10009", "-M", USER1["name"]])
+
+    res, user = call_sssd_getpwnam(USER1["name"])
+    assert res == NssReturnCode.SUCCESS
+    assert user == local_user1
+
+    res, _ = call_sssd_getpwnam("{0}@implicit_files".format(USER1["name"]))
+    assert res == NssReturnCode.NOTFOUND
+
+
 def test_no_files_domain(add_user_with_canary, no_files_domain):
     """
     Test that if no files domain is configured, sssd will add the implicit one
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Reply via email to