On (13/04/16 12:55), Lukas Slebodnik wrote: >On (13/04/16 12:43), Sumit Bose wrote: >>On Wed, Apr 13, 2016 at 10:39:44AM +0200, Lukas Slebodnik wrote: >>> On (12/04/16 11:03), Sumit Bose wrote: >>> >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 <[email protected]> >>> >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 >>> >+ >>> >[email protected] >>> >+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 <[email protected]> >>> >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 <[email protected]> >>> >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 <[email protected]> >>> >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; >>> >+ } >>> I think it would be better to check ret for EOK >>> rather then checking dummy_domain. The variable dummy_domain >>> will be everytime set if ret is EOK. >> >>Are you sure, I thought if the name does not contain a domain part >>sss_parse_name() returns EOK and set the returned domain name to NULL? >>At least this is how I understand the parse_name_plain() name test. >> >Ahh, >you are right. >I was looking into unit tests for sss_parse_name >and I didn't notice different re_expression. >sss_override should work with any regex. > >ACK > >http://sssd-ci.duckdns.org/logs/job/41/16/summary.html > master: * 32dd0dd34193a7566d83adf6845f5194decc3304 * e45096aead1d2e2b8f8b2b386b420c5f62ad07d3 * 3a8b5ccf7c27b72054e1d8b3ab355cb1e28efda9 * e6e2d1575ac7feb3494649f94ef51ef13cbdce48
LS _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
