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