Hi,

I'm working on adding certificates to overrides and came across some
issues with the local overrides. The main cause was that instead you
searching the original object in the cache, the DN of the object was
constructed. This failed for users form sub-domain which have the
fully-qualified name in the DN and not only the short name. Additionally
users with mixed-case names where not found in case-insensitive domains
which can e.g. with the AD provider and the Administrator user from AD.

The first patch contains a new integration test which tests the
mixed-case user name. Unfortunately it is currently not possible to test
the sub-domain case.

The second patch replaces the DN based lookup with a proper search. Imo
calls like sysdb_user_dn() to construct a DN should only be used when
adding new entries but newer to look up entries. Here always proper
searches should be used. And as long as the search uses index attributes
it will not be much more expensive than the DN based lookup because
afaik the ldb index tables are kept in memory.

The third patch makes sure that the tools are aware of the flat name and
the SID of the master domain if this information is available in the
cache. This might currently not be needed explicitly but helps to avoid
some irritating debug messages about a missing flat name when generating
fully qualified names.

Finally the last patch fixes the output of sub-domain users which
already have a fully-qualified name where no additional domain component
must be added.

bye,
Sumit
From 4065b421e725118f3832d6f8ae71808aa57887c5 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Mon, 11 Apr 2016 14:25:18 +0200
Subject: [PATCH 1/4] intg: local override for user with mixed case name

Test for users with fully-qualified and mixed-cased names are added.
---
 src/tests/intg/ldap_local_override_test.py | 66 +++++++++++++++++++++++++++++-
 1 file changed, 65 insertions(+), 1 deletion(-)

diff --git a/src/tests/intg/ldap_local_override_test.py 
b/src/tests/intg/ldap_local_override_test.py
index 
d5f58fc7a5207413df70b2f103887c0a72a47aa7..542527180cf2c7ec5a2984f6b42655fb0c75f5d1
 100644
--- a/src/tests/intg/ldap_local_override_test.py
+++ b/src/tests/intg/ldap_local_override_test.py
@@ -139,7 +139,8 @@ def create_sssd_fixture(request):
 OVERRIDE_FILENAME = "export_file"
 
 
-def prepare_sssd(request, ldap_conn, use_fully_qualified_names=False):
+def prepare_sssd(request, ldap_conn, use_fully_qualified_names=False,
+                 case_sensitive=True):
     """Prepare SSSD with defaults"""
     conf = unindent("""\
         [sssd]
@@ -158,6 +159,7 @@ def prepare_sssd(request, ldap_conn, 
use_fully_qualified_names=False):
         ldap_uri            = {ldap_conn.ds_inst.ldap_url}
         ldap_search_base    = {ldap_conn.ds_inst.base_dn}
         use_fully_qualified_names = {use_fully_qualified_names}
+        case_sensitive      = {case_sensitive}
     """).format(**locals())
     create_conf_fixture(request, conf)
     create_sssd_fixture(request)
@@ -951,3 +953,65 @@ def test_regr_2790_override(ldap_conn, 
env_regr_2790_override):
     assert res == sssd_id.NssReturnCode.SUCCESS, \
         "Could not find groups for user2 %d" % errno
     assert sorted(grp_list) == sorted(["group1", "group2"])
+
+# Test fully qualified and case-insensitive names
+
+@pytest.fixture
+def env_mix_cased_name_override(request, ldap_conn):
+    """Setup test for mixed case names"""
+
+    prepare_sssd(request, ldap_conn, True, False)
+
+    # Add entries
+    ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+    ent_list.add_user("user1", 10001, 20001)
+    ent_list.add_user("uSeR2", 10002, 20002)
+
+    create_ldap_fixture(request, ldap_conn, ent_list)
+
+
+    pwd.getpwnam('user1@LDAP')
+    pwd.getpwnam('user2@LDAP')
+    with pytest.raises(KeyError):
+        pwd.getpwnam('ov_user1@LDAP')
+    with pytest.raises(KeyError):
+        pwd.getpwnam('ov_user2@LDAP')
+
+    # Override
+    subprocess.check_call(["sss_override", "user-add", "user1@LDAP",
+                           "-u", "10010",
+                           "-g", "20010",
+                           "-n", "ov_user1",
+                           "-c", "Overriden User 1",
+                           "-h", "/home/ov/user1",
+                           "-s", "/bin/ov_user1_shell"])
+
+    subprocess.check_call(["sss_override", "user-add", "user2@LDAP",
+                           "-u", "10020",
+                           "-g", "20020",
+                           "-n", "ov_user2",
+                           "-c", "Overriden User 2",
+                           "-h", "/home/ov/user2",
+                           "-s", "/bin/ov_user2_shell"])
+
+    restart_sssd()
+
+def test_mix_cased_name_override(ldap_conn, env_mix_cased_name_override):
+    """Test if names with upper and lower case letter are overridden"""
+
+    # Assert entries are overridden
+    user1 = dict(name='ov_user1@LDAP', passwd='*', uid=10010, gid=20010,
+                 gecos='Overriden User 1',
+                 dir='/home/ov/user1',
+                 shell='/bin/ov_user1_shell')
+
+    user2 = dict(name='ov_user2@LDAP', passwd='*', uid=10020, gid=20020,
+                 gecos='Overriden User 2',
+                 dir='/home/ov/user2',
+                 shell='/bin/ov_user2_shell')
+
+    ent.assert_passwd_by_name('user1@LDAP', user1)
+    ent.assert_passwd_by_name('ov_user1@LDAP', user1)
+
+    ent.assert_passwd_by_name('user2@LDAP', user2)
+    ent.assert_passwd_by_name('ov_user2@LDAP', user2)
-- 
2.1.0

From 4574149abea319b41cb6178fb5490339dc550527 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Fri, 8 Apr 2016 14:43:22 +0200
Subject: [PATCH 2/4] sss_override: do not generate DN, search object

DNs of existing objects can not be generate reliable because the use of
fully qualified names and upper and lower cases in names has to be
considered. The most reliable way to get the DN is to search the object
and take the DN from the result.
---
 src/tools/sss_override.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/src/tools/sss_override.c b/src/tools/sss_override.c
index 
3eb1191959653c6e4ad9b981c045956572fdc556..c8d3e55c11a19a21dea723dd634103eda5901f60
 100644
--- a/src/tools/sss_override.c
+++ b/src/tools/sss_override.c
@@ -584,6 +584,7 @@ static errno_t get_object_dn(TALLOC_CTX *mem_ctx,
     struct ldb_dn *ldb_dn;
     const char *str_dn;
     errno_t ret;
+    struct ldb_result *res;
 
     tmp_ctx = talloc_new(NULL);
     if (tmp_ctx == NULL) {
@@ -593,17 +594,36 @@ static errno_t get_object_dn(TALLOC_CTX *mem_ctx,
 
     switch (type) {
     case SYSDB_MEMBER_USER:
-       ldb_dn = sysdb_user_dn(tmp_ctx, domain, name);
-       break;
+        ret = sysdb_getpwnam(tmp_ctx, domain, name, &res);
+        break;
     case SYSDB_MEMBER_GROUP:
-       ldb_dn = sysdb_group_dn(tmp_ctx, domain, name);
-       break;
+        ret = sysdb_getgrnam(tmp_ctx, domain, name, &res);
+        break;
     default:
-       DEBUG(SSSDBG_CRIT_FAILURE, "Unsupported member type %d\n", type);
-       ret = ERR_INTERNAL;
-       goto done;
+        DEBUG(SSSDBG_CRIT_FAILURE, "Unsupported member type %d\n", type);
+        ret = ERR_INTERNAL;
+        goto done;
     }
 
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              "Failed to look up original object in cache.\n");
+        goto done;
+    }
+
+    if (res->count == 0) {
+        DEBUG(SSSDBG_MINOR_FAILURE, "Original object not found in cache.\n");
+        ret = ENOENT;
+        goto done;
+    } else if (res->count > 1) {
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              "There are multiple object with name [%s] in the cache.\n", 
name);
+        ret = EINVAL;
+        goto done;
+    }
+
+    ldb_dn = res->msgs[0]->dn;
+
     if (ldb_dn == NULL) {
         ret = ENOMEM;
         goto done;
-- 
2.1.0

From 08bcb4120c340078ec560d95f6ace7bd09bd8012 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Fri, 8 Apr 2016 18:40:48 +0200
Subject: [PATCH 3/4] tools: read additional data of the master domain

---
 src/tools/common/sss_tools.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/tools/common/sss_tools.c b/src/tools/common/sss_tools.c
index 
d3073a2e534fefa7a82a2e5c162eb22e1d433bc1..b916f772b7333e23119aa417a74a6897ba1f8e4d
 100644
--- a/src/tools/common/sss_tools.c
+++ b/src/tools/common/sss_tools.c
@@ -137,6 +137,14 @@ static errno_t sss_tool_domains_init(TALLOC_CTX *mem_ctx,
     for (dom = domains; dom != NULL;
             dom = get_next_domain(dom, SSS_GND_DESCEND)) {
         if (!IS_SUBDOMAIN(dom)) {
+            /* Get flat name and domain ID (SID) from the cache
+             * if available */
+            ret = sysdb_master_domain_update(dom);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_MINOR_FAILURE, "Failed to update domain %s.\n",
+                                            dom->name);
+            }
+
             /* Update list of subdomains for this domain */
             ret = sysdb_update_subdomains(dom);
             if (ret != EOK) {
-- 
2.1.0

From 8e5d0bec231b85af75566a6f6fc13bae1623440d Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Fri, 8 Apr 2016 18:43:57 +0200
Subject: [PATCH 4/4] sss_override: only add domain if name is not fully
 qualified

---
 src/tools/sss_override.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/tools/sss_override.c b/src/tools/sss_override.c
index 
c8d3e55c11a19a21dea723dd634103eda5901f60..7e63bdf6ecb3bb968f631e79f691942237f61793
 100644
--- a/src/tools/sss_override.c
+++ b/src/tools/sss_override.c
@@ -379,11 +379,37 @@ static char *get_fqname(TALLOC_CTX *mem_ctx,
     char *fqname;
     int fqlen;
     int check;
+    char *dummy_domain = NULL;
+    int ret;
 
-    if (domain == NULL) {
+    if (domain == NULL || domain->names == NULL) {
         return NULL;
     }
 
+    /* check if the name already contains domain part */
+    ret = sss_parse_name(mem_ctx, domain->names, name, &dummy_domain, NULL);
+    if (ret == ERR_REGEX_NOMATCH) {
+        DEBUG(SSSDBG_TRACE_FUNC,
+              "sss_parse_name could not parse domain from [%s]. "
+              "Assuming it is not FQDN.\n", name);
+    } else if (ret != EOK) {
+        DEBUG(SSSDBG_TRACE_FUNC,
+              "sss_parse_name failed [%d]: %s\n", ret, sss_strerror(ret));
+        return NULL;
+    }
+
+    if (dummy_domain != NULL) {
+        talloc_free(dummy_domain);
+        DEBUG(SSSDBG_TRACE_FUNC, "Name is already fully qualified.\n");
+        fqname = talloc_strdup(mem_ctx, name);
+        if (fqname == NULL) {
+            DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup failed.\n");
+            return NULL;
+        }
+
+        return fqname;
+    }
+
     /* Get length. */
     fqlen = sss_fqname(NULL, 0, domain->names, domain, name);
     if (fqlen > 0) {
-- 
2.1.0

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

Reply via email to