[SSSD] [PATCHES] Fixes for sss_cache

Sumit Bose sbose at redhat.com
Mon Dec 16 14:49:01 UTC 2013


On Thu, Dec 05, 2013 at 01:56:41PM +0100, Jakub Hrozek wrote:
> On Wed, Dec 04, 2013 at 05:56:30PM +0100, Sumit Bose wrote:
> > On Wed, Dec 04, 2013 at 05:22:06PM +0100, Jakub Hrozek wrote:
> > > On Wed, Dec 04, 2013 at 10:42:52AM +0100, Sumit Bose wrote:
> > > > On Tue, Dec 03, 2013 at 02:01:27PM +0100, Jakub Hrozek wrote:
> > > > > On Thu, Nov 28, 2013 at 05:55:44PM +0100, Sumit Bose wrote:
> > > > > > On Wed, Nov 27, 2013 at 02:50:35PM +0100, Jakub Hrozek wrote:
> > > > > > > On Tue, Nov 26, 2013 at 11:51:41AM +0100, Sumit Bose wrote:
> > > > > > > > Hi,
> > > > > > > > 
> > > > > > > > Steeve found some issues when testing sss_cache with sub-domain users.
> > > > > > > > This was originally fixed in https://fedorahosted.org/sssd/ticket/1741
> > > > > > > > but I guess recent changes have broken it again.
> > > > > > > > 
> > > > > > > > I have tested the patches with users and groups. It would be nice is
> > > > > > > > someone with a suitable environment can test them for the other object
> > > > > > > > types as well.
> > > > > > > > 
> > > > > > > > bye,
> > > > > > > > Sumit
> > > > > > > 
> > > > > > > Hi,
> > > > > > > 
> > > > > > > during testing I found out that there is a difference in how we store
> > > > > > > the nameAlias attribute for subdomain users retrieved with the exended
> > > > > > > operation to IPA and subdomain users that are stored with the LDAP
> > > > > > > provider.
> > > > > > > 
> > > > > > > The IPA subdomain users have lowercase the whole alias (so typically
> > > > > > > user at windows.domain) while the LDAP users have only the name component
> > > > > > > lowercased (user at WINDOWS.DOMAIN). Currently sss_cache only works with he
> > > > > > > latter.
> > > > > > > 
> > > > > > > As discussed on IRC, we should pick on scheme and use it, ideally with
> > > > > > > some helper function.
> > > > > > 
> > > > > > I added two new patches to fix this. 0003 adds a new call to add a lower
> > > > > > case alias name to a sysdb_attrs struct and 0004 replace current code
> > > > > > with the new call.
> > > > > > 
> > > > > > bye,
> > > > > > Sumit
> > > > > 
> > > > > After testing the patches I think the issue is a bit more complex. I
> > > > > hope I can explain the problem clearly. We can only consider subdomain
> > > > > users and hence FQDN lookups for the problem.
> > > > > 
> > > > > In the responder code, the lookup will be performed for user at DOMAIN_NAME,
> > > > > where DOMAIN_NAME is exactly the same case as domain name in the confdb.
> > > > > So unfortunately lowercasing the whole alias won't work
> > > > > unless the responder FQDN lookups are lowercased as well.
> > > > > 
> > > > > I was wondering a bit why did the lookups for users on IPA clients
> > > > > (fetched with extop) work and it turns out we matched their name
> > > > > attribute, not alias. This is how the user entry looks now with git
> > > > > HEAD:
> > > > > 
> > > > > dn: name=administrator at WIN.EXAMPLE.COM,cn=users,cn=WIN.EXAMPLE.COM,cn=sysdb
> > > > > name: administrator at WIN.EXAMPLE.COM
> > > > > nameAlias: administrator at win.example.com
> > > > > 
> > > > > so the nameAlias can only be matched with lowercased sssd domain, in my
> > > > > case, the name attribute is matched. IIRC the reason even the name is
> > > > > lowercased and not original is a bug in winbind we tried to work
> > > > > around..
> > > > > 
> > > > > In IPA server mode the same user entry looks like this with git HEAD:
> > > > > 
> > > > > dn: name=Administrator at WIN.EXAMPLE.COM,cn=users,cn=WIN.EXAMPLE.COM,cn=sysdb
> > > > > name: Administrator at WIN.EXAMPLE.COM
> > > > > nameAlias: administrator at WIN.EXAMPLE.COM
> > > > > 
> > > > > So here the nameAlias is matched by the responder.
> > > > > 
> > > > > The patches changed that to:
> > > > > dn: name=Administrator at WIN.EXAMPLE.COM,cn=users,cn=WIN.EXAMPLE.COM,cn=sysdb
> > > > > name: Administrator at WIN.EXAMPLE.COM
> > > > > nameAlias: administrator at win.example.com
> > > > > 
> > > > > So neither name nor alias matched.
> > > > > 
> > > > > I'm not sure what the best way to fix the inconsistency would be. I
> > > > > think the way subdomain users are stored with AD backend makes more
> > > > > sense to me because the original name as stored on the server is kept in
> > > > > the name attribute and the alias is matched on lookups. But the way
> > > > > extop users are stored might be fine as well, we don't seem to be using
> > > > > the original version of name at the moment.
> > > > 
> > > > The users coming from extdom are always lower-cased because winbind will
> > > > return them in lower-case most of the time and winbind was feeding the
> > > > extdom plugin before we had ipa-server-mode. I say 'most of the time'
> > > > because there was a bug in some versions of winbind where the original
> > > > name was returned. Since the information from the extdom plugin have to
> > > > be extended with the group information from the PAC I thought the
> > > > easiest way to find the right user is to always use lower-case names
> > > > here which will always work because AD is case-insensitve. But if we can
> > > > make sure that nameAlias is handled consistently I do not have any
> > > > objections to use the name which is returned from the server without
> > > > making it lower-case explicitly.
> > > 
> > > OK, I'm fine with this, as I said above, the name is not really
> > > important but the way we treated the nameAlias with extdom users (where
> > > it never matched) was strange to me.
> > > 
> > > > 
> > > > About the case of the domain in nameAlias. As long as nameAlias is used
> > > > only inside the sssd process the case does not matter much because I
> > > > think the domain name is always taken from the name member of the
> > > > sss_domain_info struct (and if it is not the case it can easily
> > > > be converted). Nevertheless I find it a bit counter intuitive to have a
> > > > mixed cased name in nameAlias  to search for a case-insensitive name so
> > > > I would prefer to change the responder. Since
> > > > sss_parse_name_for_domains() make case-insensitive comparison of the
> > > > domain name as well we do not have a chance to support case-sensitive
> > > > domain names.
> > > 
> > > Thanks for the domain name investigation.
> > > 
> > > So the proposal is to change the responder searches to look for
> > > case-insensitive nameAlias? If so, don't we need some sysdb upgrade to
> > > lowercase the existing mixed case aliases?
> > 
> > good point, I think we should keep the old filter as a fallback to avoid
> > a sysdb upgrade for the time being. Maybe it can be folded into an other
> > major sysdb upgrade later and then the fallback can be removed.
> 
> Yes, Michal is working on sysdb upgrade anyway, that might be a good
> time to do two changes.

please find attached the patch set with an additional fifth patch which
adds an all-lower-cased string to the search filter where needed.

Please note that the original issue was that sss_cache was not able to
find some entries from sub-domains. If you think the current patch set
is too much for a minor release and the original issue should be fixed
differently, let me know. Nevertheless I think not having a
all-lower-cased version of the name in nameAlias for case-insensitive
searches is a bug which has to be fixed.

bye,
Sumit
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
-------------- next part --------------
From d0941349a41b0383bff7e40c9f76a17d464891a4 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Mon, 25 Nov 2013 17:54:06 +0100
Subject: [PATCH 1/5] sss_cache: initialize names member of sss_domain_info

sss_tc_fqname() called by sss_get_domain_name() requires that the names
member of the sss_domain_info struct is set to work properly. If the
names struct is properly initialized in sss_domain_info the separate one
in the tool context is not needed anymore.

Related to https://fedorahosted.org/sssd/ticket/1741
---
 src/tools/sss_cache.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/src/tools/sss_cache.c b/src/tools/sss_cache.c
index 0bf029e..6412d71 100644
--- a/src/tools/sss_cache.c
+++ b/src/tools/sss_cache.c
@@ -63,7 +63,6 @@ static errno_t search_autofsmaps(TALLOC_CTX *mem_ctx,
 struct cache_tool_ctx {
     struct confdb_ctx *confdb;
     struct sss_domain_info *domains;
-    struct sss_names_ctx *nctx;
 
     char *user_filter;
     char *group_filter;
@@ -208,7 +207,7 @@ static errno_t update_filter(struct cache_tool_ctx *tctx,
         return ENOMEM;
     }
 
-    ret = sss_parse_name(tmp_ctx, tctx->nctx, name,
+    ret = sss_parse_name(tmp_ctx, dinfo->names, name,
                          &parsed_domain, &parsed_name);
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, ("sss_parse_name failed\n"));
@@ -279,17 +278,6 @@ static errno_t update_all_filters(struct cache_tool_ctx *tctx,
 {
     errno_t ret;
 
-    if (IS_SUBDOMAIN(dinfo)) {
-        ret = sss_names_init(tctx, tctx->confdb, dinfo->parent->name,
-                             &tctx->nctx);
-    } else {
-        ret = sss_names_init(tctx, tctx->confdb, dinfo->name, &tctx->nctx);
-    }
-    if (ret != EOK) {
-        DEBUG(SSSDBG_CRIT_FAILURE, ("sss_names_init() failed\n"));
-        return ret;
-    }
-
     /* Update user filter */
     ret = update_filter(tctx, dinfo, tctx->user_name,
                         tctx->update_user_filter, "(%s=%s)", false,
@@ -464,6 +452,7 @@ errno_t init_domains(struct cache_tool_ctx *ctx, const char *domain)
 {
     char *confdb_path;
     int ret;
+    struct sss_domain_info *dinfo;
 
     confdb_path = talloc_asprintf(ctx, "%s/%s", DB_PATH, CONFDB_FILE);
     if (confdb_path == NULL) {
@@ -502,6 +491,14 @@ errno_t init_domains(struct cache_tool_ctx *ctx, const char *domain)
         }
     }
 
+    for (dinfo = ctx->domains; dinfo; dinfo = get_next_domain(dinfo, false)) {
+        ret = sss_names_init(ctx, ctx->confdb, dinfo->name, &dinfo->names);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("sss_names_init() failed\n"));
+            return ret;
+        }
+    }
+
     return EOK;
 }
 
-- 
1.8.3.1

-------------- next part --------------
From 9c2c912786188ab5b7b8f0c13e8ce946eaf9a9c0 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 26 Nov 2013 10:27:50 +0100
Subject: [PATCH 2/5] sss_cache: fix case-sensitivity issue

For case-insensitive domains the lower-case name for case-insensitive
searches is stored in SYSDB_NAME_ALIAS.

Related to https://fedorahosted.org/sssd/ticket/1741
---
 src/tools/sss_cache.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/tools/sss_cache.c b/src/tools/sss_cache.c
index 6412d71..91bfcee 100644
--- a/src/tools/sss_cache.c
+++ b/src/tools/sss_cache.c
@@ -235,7 +235,9 @@ static errno_t update_filter(struct cache_tool_ctx *tctx,
         if (!strcasecmp(dinfo->name, parsed_domain)) {
             if (fmt) {
                 filter = talloc_asprintf(tmp_ctx, fmt,
-                                         SYSDB_NAME, use_name);
+                                         dinfo->case_sensitive ?
+                                                SYSDB_NAME : SYSDB_NAME_ALIAS,
+                                         use_name);
             } else {
                 filter = talloc_strdup(tmp_ctx, use_name);
             }
@@ -251,7 +253,10 @@ static errno_t update_filter(struct cache_tool_ctx *tctx,
         }
     } else {
         if (fmt) {
-            filter = talloc_asprintf(tmp_ctx, fmt, SYSDB_NAME, name);
+            filter = talloc_asprintf(tmp_ctx, fmt,
+                                     dinfo->case_sensitive ?
+                                            SYSDB_NAME : SYSDB_NAME_ALIAS,
+                                     name);
         } else {
             filter = talloc_strdup(tmp_ctx, name);
         }
-- 
1.8.3.1

-------------- next part --------------
From 73c4075ec1aee012339d3cebf4f7d4989cbc6b5d Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Thu, 28 Nov 2013 11:28:39 +0100
Subject: [PATCH 3/5] Add sysdb_attrs_add_lc_name_alias

---
 src/db/sysdb.c          | 22 ++++++++++++++++++++++
 src/db/sysdb.h          |  2 ++
 src/tests/sysdb-tests.c | 29 +++++++++++++++++++++++++++++
 3 files changed, 53 insertions(+)

diff --git a/src/db/sysdb.c b/src/db/sysdb.c
index 283dabd..09a4b64 100644
--- a/src/db/sysdb.c
+++ b/src/db/sysdb.c
@@ -616,6 +616,28 @@ int sysdb_attrs_add_time_t(struct sysdb_attrs *attrs,
     return ret;
 }
 
+int sysdb_attrs_add_lc_name_alias(struct sysdb_attrs *attrs,
+                                  const char *value)
+{
+    char *lc_str;
+    int ret;
+
+    if (attrs == NULL || value == NULL) {
+        return EINVAL;
+    }
+
+    lc_str = sss_tc_utf8_str_tolower(attrs, value);
+    if (lc_str == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Cannot convert name to lowercase\n"));
+        return ENOMEM;
+    }
+
+    ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS, lc_str);
+    talloc_free(lc_str);
+
+    return ret;
+}
+
 int sysdb_attrs_copy_values(struct sysdb_attrs *src,
                             struct sysdb_attrs *dst,
                             const char *name)
diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index 63a1fab..cec8bdd 100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -274,6 +274,8 @@ int sysdb_attrs_add_uint32(struct sysdb_attrs *attrs,
                            const char *name, uint32_t value);
 int sysdb_attrs_add_time_t(struct sysdb_attrs *attrs,
                            const char *name, time_t value);
+int sysdb_attrs_add_lc_name_alias(struct sysdb_attrs *attrs,
+                                  const char *value);
 int sysdb_attrs_copy_values(struct sysdb_attrs *src,
                             struct sysdb_attrs *dst,
                             const char *name);
diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
index cc2d661..ee0030c 100644
--- a/src/tests/sysdb-tests.c
+++ b/src/tests/sysdb-tests.c
@@ -4332,6 +4332,33 @@ START_TEST(test_sysdb_svc_remove_alias)
 }
 END_TEST
 
+#define LC_NAME_ALIAS_TEST_VAL "TeSt VaLuE"
+#define LC_NAME_ALIAS_CHECK_VAL "test value"
+START_TEST(test_sysdb_attrs_add_lc_name_alias)
+{
+    int ret;
+    struct sysdb_attrs *attrs;
+    const char *str;
+
+    ret = sysdb_attrs_add_lc_name_alias(NULL, NULL);
+    fail_unless(ret == EINVAL, "EINVAL not returned for NULL input");
+
+    attrs = sysdb_new_attrs(NULL);
+    fail_unless(attrs != NULL, "sysdb_new_attrs failed");
+
+    ret = sysdb_attrs_add_lc_name_alias(attrs, LC_NAME_ALIAS_TEST_VAL);
+    fail_unless(ret == EOK, "sysdb_attrs_add_lc_name_alias failed");
+
+    ret = sysdb_attrs_get_string(attrs, SYSDB_NAME_ALIAS, &str);
+    fail_unless(ret == EOK, "sysdb_attrs_get_string failed");
+    fail_unless(strcmp(str, LC_NAME_ALIAS_CHECK_VAL) == 0,
+                "Unexpected value, expected [%s], got [%s]",
+                LC_NAME_ALIAS_CHECK_VAL, str);
+
+    talloc_free(attrs);
+}
+END_TEST
+
 START_TEST(test_sysdb_has_enumerated)
 {
     errno_t ret;
@@ -5090,6 +5117,8 @@ Suite *create_sysdb_suite(void)
     tcase_add_test(tc_sysdb, test_sysdb_store_services);
     tcase_add_test(tc_sysdb, test_sysdb_svc_remove_alias);
 
+    tcase_add_test(tc_sysdb, test_sysdb_attrs_add_lc_name_alias);
+
 /* Add all test cases to the test suite */
     suite_add_tcase(s, tc_sysdb);
 
-- 
1.8.3.1

-------------- next part --------------
From 24ee257e0c6de5b1240c5fea01f7f42fb43d1d9f Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Thu, 28 Nov 2013 12:31:24 +0100
Subject: [PATCH 4/5] Use sysdb_attrs_add_lc_name_alias to add case-insensitive
 alias

---
 src/providers/ipa/ipa_s2n_exop.c     | 27 ++++++---------------------
 src/providers/ldap/sdap_async.c      | 21 ++++++++++++++++-----
 src/providers/proxy/proxy_id.c       | 20 ++------------------
 src/providers/proxy/proxy_netgroup.c | 10 +---------
 src/responder/pac/pacsrv_utils.c     |  4 ++--
 5 files changed, 27 insertions(+), 55 deletions(-)

diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c
index 7379b99..628880f 100644
--- a/src/providers/ipa/ipa_s2n_exop.c
+++ b/src/providers/ipa/ipa_s2n_exop.c
@@ -652,7 +652,6 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq)
     struct sysdb_attrs *user_attrs = NULL;
     struct sysdb_attrs *group_attrs = NULL;
     char *name;
-    char *lc_name;
     char *realm;
     char *upn;
     struct berval *bv_req = NULL;
@@ -768,16 +767,10 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq)
                 goto done;
             }
 
-            lc_name = sss_tc_utf8_str_tolower(user_attrs, name);
-            if (lc_name == NULL) {
-                DEBUG(SSSDBG_CRIT_FAILURE, ("Cannot convert name to lowercase\n"));
-                ret =  ENOMEM;
-                goto done;
-            }
-
-            ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS, lc_name);
+            ret = sysdb_attrs_add_lc_name_alias(user_attrs, name);
             if (ret != EOK) {
-                DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n"));
+                DEBUG(SSSDBG_OP_FAILURE,
+                      ("sysdb_attrs_add_lc_name_alias failed.\n"));
                 goto done;
             }
 
@@ -853,18 +846,10 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq)
                 goto done;
             }
 
-            lc_name = sss_tc_utf8_str_tolower(group_attrs, name);
-            if (lc_name == NULL) {
-                DEBUG(SSSDBG_CRIT_FAILURE,
-                      ("Cannot convert name to lowercase\n"));
-                ret = ENOMEM;
-                goto done;
-            }
-
-            ret = sysdb_attrs_add_string(group_attrs, SYSDB_NAME_ALIAS,
-                                         lc_name);
+            ret = sysdb_attrs_add_lc_name_alias(group_attrs, name);
             if (ret != EOK) {
-                DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n"));
+                DEBUG(SSSDBG_OP_FAILURE,
+                      ("sysdb_attrs_add_lc_name_alias failed.\n"));
                 goto done;
             }
 
diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
index f5cc962..e905d2d 100644
--- a/src/providers/ldap/sdap_async.c
+++ b/src/providers/ldap/sdap_async.c
@@ -2318,12 +2318,23 @@ sdap_save_all_names(const char *name,
             goto done;
         }
 
-        ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS, domname);
-        if (ret) {
-            DEBUG(SSSDBG_OP_FAILURE, ("Failed to add alias [%s] into the "
-                                      "attribute list\n", aliases[i]));
-            goto done;
+        if (lowercase) {
+            ret = sysdb_attrs_add_lc_name_alias(attrs, domname);
+            if (ret) {
+                DEBUG(SSSDBG_OP_FAILURE, ("Failed to add lower-cased version "
+                                          "of alias [%s] into the "
+                                          "attribute list\n", aliases[i]));
+                goto done;
+            }
+        } else {
+            ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS, domname);
+            if (ret) {
+                DEBUG(SSSDBG_OP_FAILURE, ("Failed to add alias [%s] into the "
+                                          "attribute list\n", aliases[i]));
+                goto done;
+            }
         }
+
     }
 
     ret = EOK;
diff --git a/src/providers/proxy/proxy_id.c b/src/providers/proxy/proxy_id.c
index 6e2b175..4a6b28b 100644
--- a/src/providers/proxy/proxy_id.c
+++ b/src/providers/proxy/proxy_id.c
@@ -219,7 +219,6 @@ static int save_user(struct sss_domain_info *domain,
 {
     const char *shell;
     const char *gecos;
-    char *lower;
     struct sysdb_attrs *attrs = NULL;
     errno_t ret;
     const char *cased_alias;
@@ -245,14 +244,7 @@ static int save_user(struct sss_domain_info *domain,
     }
 
     if (lowercase) {
-        lower = sss_tc_utf8_str_tolower(attrs, pwd->pw_name);
-        if (!lower) {
-            DEBUG(SSSDBG_CRIT_FAILURE, ("Cannot convert name to lowercase\n"));
-            talloc_zfree(attrs);
-            return ENOMEM;
-        }
-
-        ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS, lower);
+        ret = sysdb_attrs_add_lc_name_alias(attrs, pwd->pw_name);
         if (ret) {
             DEBUG(SSSDBG_OP_FAILURE, ("Could not add name alias\n"));
             talloc_zfree(attrs);
@@ -534,7 +526,6 @@ static int save_group(struct sysdb_ctx *sysdb, struct sss_domain_info *dom,
 {
     errno_t ret, sret;
     struct sysdb_attrs *attrs = NULL;
-    char *lower;
     const char *cased_alias;
     TALLOC_CTX *tmp_ctx;
     time_t now = time(NULL);
@@ -589,14 +580,7 @@ static int save_group(struct sysdb_ctx *sysdb, struct sss_domain_info *dom,
         }
 
         if (dom->case_sensitive == false) {
-            lower = sss_tc_utf8_str_tolower(attrs, grp->gr_name);
-            if (!lower) {
-                DEBUG(SSSDBG_CRIT_FAILURE, ("Cannot convert name to lowercase\n"));
-                ret = ENOMEM;
-                goto done;
-            }
-
-            ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS, lower);
+            ret = sysdb_attrs_add_lc_name_alias(attrs, grp->gr_name);
             if (ret) {
                 DEBUG(SSSDBG_OP_FAILURE, ("Could not add name alias\n"));
                 ret = ENOMEM;
diff --git a/src/providers/proxy/proxy_netgroup.c b/src/providers/proxy/proxy_netgroup.c
index 0aeb7cf..6be889a 100644
--- a/src/providers/proxy/proxy_netgroup.c
+++ b/src/providers/proxy/proxy_netgroup.c
@@ -73,17 +73,9 @@ static errno_t save_netgroup(struct sss_domain_info *domain,
                              uint64_t cache_timeout)
 {
     errno_t ret;
-    char *lower;
 
     if (lowercase) {
-        lower = sss_tc_utf8_str_tolower(NULL, name);
-        if (!lower) {
-            DEBUG(SSSDBG_CRIT_FAILURE, ("Cannot convert name to lowercase\n"));
-            return ENOMEM;
-        }
-
-        ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS, lower);
-        talloc_free(lower);
+        ret = sysdb_attrs_add_lc_name_alias(attrs, name);
         if (ret) {
             DEBUG(SSSDBG_OP_FAILURE, ("Could not add name alias\n"));
             return ret;
diff --git a/src/responder/pac/pacsrv_utils.c b/src/responder/pac/pacsrv_utils.c
index d56fc19..0872e86 100644
--- a/src/responder/pac/pacsrv_utils.c
+++ b/src/responder/pac/pacsrv_utils.c
@@ -479,9 +479,9 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS, pwd->pw_name);
+    ret = sysdb_attrs_add_lc_name_alias(attrs, pwd->pw_name);
     if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n"));
+        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_lc_name_alias failed.\n"));
         goto done;
     }
 
-- 
1.8.3.1

-------------- next part --------------
From 2e8487164b327616ec32674d06722f06229bea53 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 13 Dec 2013 11:44:59 +0100
Subject: [PATCH 5/5] Use lower-case name for case-insensitive searches

The patch makes sure that a completely lower-cased version of a fully
qualified name is used for case insensitive searches. Currently there
are code paths where the domain name was used as configured and was not
lower-cased.

To make sure this patch does not break with old entries in the cache or
case sensitive domains a third template was added to the related filters
templates which is either filled with a completely lower-cased version or
with the old version. The other two template values are unchanged.
---
 src/db/sysdb.h                       | 10 +++++-----
 src/db/sysdb_ops.c                   |  8 +++++---
 src/db/sysdb_search.c                | 30 ++++++++++++++++++++--------
 src/responder/pam/pam_LOCAL_domain.c |  4 ++--
 src/tests/cmocka/test_utils.c        | 38 ++++++++++++++++++++++++++++++++++++
 src/util/sss_tc_utf8.c               | 30 ++++++++++++++++++++++++++++
 src/util/util.h                      |  6 ++++++
 7 files changed, 108 insertions(+), 18 deletions(-)

diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index cec8bdd..2230f2c 100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -144,23 +144,23 @@
 #define SYSDB_NC "objectclass="SYSDB_NETGROUP_CLASS
 #define SYSDB_MPGC "|("SYSDB_UC")("SYSDB_GC")"
 
-#define SYSDB_PWNAM_FILTER "(&("SYSDB_UC")(|("SYSDB_NAME_ALIAS"=%s)("SYSDB_NAME"=%s)))"
+#define SYSDB_PWNAM_FILTER "(&("SYSDB_UC")(|("SYSDB_NAME_ALIAS"=%s)("SYSDB_NAME_ALIAS"=%s)("SYSDB_NAME"=%s)))"
 #define SYSDB_PWUID_FILTER "(&("SYSDB_UC")("SYSDB_UIDNUM"=%lu))"
 #define SYSDB_PWSID_FILTER "(&("SYSDB_UC")("SYSDB_SID_STR"=%s))"
 #define SYSDB_PWENT_FILTER "("SYSDB_UC")"
 
-#define SYSDB_GRNAM_FILTER "(&("SYSDB_GC")(|("SYSDB_NAME_ALIAS"=%s)("SYSDB_NAME"=%s)))"
+#define SYSDB_GRNAM_FILTER "(&("SYSDB_GC")(|("SYSDB_NAME_ALIAS"=%s)("SYSDB_NAME_ALIAS"=%s)("SYSDB_NAME"=%s)))"
 #define SYSDB_GRGID_FILTER "(&("SYSDB_GC")("SYSDB_GIDNUM"=%lu))"
 #define SYSDB_GRSID_FILTER "(&("SYSDB_GC")("SYSDB_SID_STR"=%s))"
 #define SYSDB_GRENT_FILTER "("SYSDB_GC")"
-#define SYSDB_GRNAM_MPG_FILTER "(&("SYSDB_MPGC")(|("SYSDB_NAME_ALIAS"=%s)("SYSDB_NAME"=%s)))"
+#define SYSDB_GRNAM_MPG_FILTER "(&("SYSDB_MPGC")(|("SYSDB_NAME_ALIAS"=%s)("SYSDB_NAME_ALIAS"=%s)("SYSDB_NAME"=%s)))"
 #define SYSDB_GRGID_MPG_FILTER "(&("SYSDB_MPGC")("SYSDB_GIDNUM"=%lu))"
 #define SYSDB_GRENT_MPG_FILTER "("SYSDB_MPGC")"
 
 #define SYSDB_INITGR_FILTER "(&("SYSDB_GC")("SYSDB_GIDNUM"=*))"
 
-#define SYSDB_NETGR_FILTER "(&("SYSDB_NC")(|("SYSDB_NAME_ALIAS"=%s)("SYSDB_NAME"=%s)))"
-#define SYSDB_NETGR_TRIPLES_FILTER "(|("SYSDB_NAME"=%s)("SYSDB_NAME_ALIAS"=%s)("SYSDB_MEMBEROF"=%s))"
+#define SYSDB_NETGR_FILTER "(&("SYSDB_NC")(|("SYSDB_NAME_ALIAS"=%s)("SYSDB_NAME_ALIAS"=%s)("SYSDB_NAME"=%s)))"
+#define SYSDB_NETGR_TRIPLES_FILTER "(|("SYSDB_NAME_ALIAS"=%s)("SYSDB_NAME"=%s)("SYSDB_NAME_ALIAS"=%s)("SYSDB_MEMBEROF"=%s))"
 
 #define SYSDB_SID_FILTER "(&(|("SYSDB_UC")("SYSDB_GC"))("SYSDB_SID_STR"=%s))"
 
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 3273452..cafb1fe 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -304,6 +304,7 @@ int sysdb_search_user_by_name(TALLOC_CTX *mem_ctx,
     struct ldb_dn *basedn;
     size_t msgs_count = 0;
     char *sanitized_name;
+    char *lc_sanitized_name;
     char *filter;
     int ret;
 
@@ -319,13 +320,14 @@ int sysdb_search_user_by_name(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    ret = sss_filter_sanitize(tmp_ctx, name, &sanitized_name);
+    ret = sss_filter_sanitize_for_dom(tmp_ctx, name, domain, &sanitized_name,
+                                      &lc_sanitized_name);
     if (ret != EOK) {
         goto done;
     }
 
-    filter = talloc_asprintf(tmp_ctx, SYSDB_PWNAM_FILTER, sanitized_name,
-                             sanitized_name);
+    filter = talloc_asprintf(tmp_ctx, SYSDB_PWNAM_FILTER, lc_sanitized_name,
+                             sanitized_name, sanitized_name);
     if (!filter) {
         ret = ENOMEM;
         goto done;
diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c
index f2599a1..18fb2ff 100644
--- a/src/db/sysdb_search.c
+++ b/src/db/sysdb_search.c
@@ -37,6 +37,7 @@ int sysdb_getpwnam(TALLOC_CTX *mem_ctx,
     struct ldb_dn *base_dn;
     struct ldb_result *res;
     char *sanitized_name;
+    char *lc_sanitized_name;
     const char *src_name;
     int ret;
 
@@ -60,13 +61,15 @@ int sysdb_getpwnam(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    ret = sss_filter_sanitize(tmp_ctx, src_name, &sanitized_name);
+    ret = sss_filter_sanitize_for_dom(tmp_ctx, src_name, domain,
+                                      &sanitized_name, &lc_sanitized_name);
     if (ret != EOK) {
         goto done;
     }
 
     ret = ldb_search(domain->sysdb->ldb, tmp_ctx, &res, base_dn,
                      LDB_SCOPE_SUBTREE, attrs, SYSDB_PWNAM_FILTER,
+                     lc_sanitized_name,
                      sanitized_name, sanitized_name);
     if (ret) {
         ret = sysdb_error_to_errno(ret);
@@ -210,6 +213,7 @@ int sysdb_getgrnam(TALLOC_CTX *mem_ctx,
     struct ldb_dn *base_dn;
     struct ldb_result *res;
     const char *src_name;
+    char *lc_sanitized_name;
     int ret;
 
     tmp_ctx = talloc_new(NULL);
@@ -239,14 +243,15 @@ int sysdb_getgrnam(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    ret = sss_filter_sanitize(tmp_ctx, src_name, &sanitized_name);
+    ret = sss_filter_sanitize_for_dom(tmp_ctx, src_name, domain,
+                                      &sanitized_name, &lc_sanitized_name);
     if (ret != EOK) {
         goto done;
     }
 
     ret = ldb_search(domain->sysdb->ldb, tmp_ctx, &res, base_dn,
                      LDB_SCOPE_SUBTREE, attrs, fmt_filter,
-                     sanitized_name, sanitized_name);
+                     lc_sanitized_name, sanitized_name, sanitized_name);
     if (ret) {
         ret = sysdb_error_to_errno(ret);
         goto done;
@@ -473,6 +478,7 @@ int sysdb_get_user_attr(TALLOC_CTX *mem_ctx,
     struct ldb_dn *base_dn;
     struct ldb_result *res;
     char *sanitized_name;
+    char *lc_sanitized_name;
     int ret;
 
     tmp_ctx = talloc_new(NULL);
@@ -487,14 +493,15 @@ int sysdb_get_user_attr(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    ret = sss_filter_sanitize(tmp_ctx, name, &sanitized_name);
+    ret = sss_filter_sanitize_for_dom(tmp_ctx, name, domain, &sanitized_name,
+                                      &lc_sanitized_name);
     if (ret != EOK) {
         goto done;
     }
 
     ret = ldb_search(domain->sysdb->ldb, tmp_ctx, &res, base_dn,
                      LDB_SCOPE_SUBTREE, attributes,
-                     SYSDB_PWNAM_FILTER, sanitized_name,
+                     SYSDB_PWNAM_FILTER, lc_sanitized_name, sanitized_name,
                      sanitized_name);
     if (ret) {
         ret = sysdb_error_to_errno(ret);
@@ -776,6 +783,7 @@ errno_t sysdb_getnetgr(TALLOC_CTX *mem_ctx,
     struct ldb_dn *base_dn;
     struct ldb_result *result;
     char *sanitized_netgroup;
+    char *lc_sanitized_netgroup;
     char *netgroup_dn;
     int lret;
     errno_t ret;
@@ -793,7 +801,9 @@ errno_t sysdb_getnetgr(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    ret = sss_filter_sanitize(tmp_ctx, netgroup, &sanitized_netgroup);
+    ret = sss_filter_sanitize_for_dom(tmp_ctx, netgroup, domain,
+                                      &sanitized_netgroup,
+                                      &lc_sanitized_netgroup);
     if (ret != EOK) {
         goto done;
     }
@@ -807,7 +817,7 @@ errno_t sysdb_getnetgr(TALLOC_CTX *mem_ctx,
 
     lret = ldb_search(domain->sysdb->ldb, tmp_ctx, &result, base_dn,
                      LDB_SCOPE_SUBTREE, attrs,
-                     SYSDB_NETGR_TRIPLES_FILTER,
+                     SYSDB_NETGR_TRIPLES_FILTER, lc_sanitized_netgroup,
                      sanitized_netgroup, sanitized_netgroup,
                      netgroup_dn);
     ret = sysdb_error_to_errno(lret);
@@ -833,6 +843,7 @@ int sysdb_get_netgroup_attr(TALLOC_CTX *mem_ctx,
     struct ldb_dn *base_dn;
     struct ldb_result *result;
     char *sanitized_netgroup;
+    char *lc_sanitized_netgroup;
     int ret;
 
     tmp_ctx = talloc_new(NULL);
@@ -847,7 +858,9 @@ int sysdb_get_netgroup_attr(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    ret = sss_filter_sanitize(tmp_ctx, netgrname, &sanitized_netgroup);
+    ret = sss_filter_sanitize_for_dom(tmp_ctx, netgrname, domain,
+                                      &sanitized_netgroup,
+                                      &lc_sanitized_netgroup);
     if (ret != EOK) {
         goto done;
     }
@@ -855,6 +868,7 @@ int sysdb_get_netgroup_attr(TALLOC_CTX *mem_ctx,
     ret = ldb_search(domain->sysdb->ldb, tmp_ctx, &result, base_dn,
                      LDB_SCOPE_SUBTREE, attributes,
                      SYSDB_NETGR_FILTER,
+                     lc_sanitized_netgroup,
                      sanitized_netgroup,
                      sanitized_netgroup);
     if (ret) {
diff --git a/src/responder/pam/pam_LOCAL_domain.c b/src/responder/pam/pam_LOCAL_domain.c
index bb73785..036b47f 100644
--- a/src/responder/pam/pam_LOCAL_domain.c
+++ b/src/responder/pam/pam_LOCAL_domain.c
@@ -258,12 +258,12 @@ int LOCAL_pam_handler(struct pam_auth_req *preq)
 
     if (res->count < 1) {
         DEBUG(4, ("No user found with filter ["SYSDB_PWNAM_FILTER"]\n",
-                  pd->user, pd->user));
+                  pd->user, pd->user, pd->user));
         pd->pam_status = PAM_USER_UNKNOWN;
         goto done;
     } else if (res->count > 1) {
         DEBUG(4, ("More than one object found with filter ["SYSDB_PWNAM_FILTER"]\n",
-                  pd->user, pd->user));
+                  pd->user, pd->user, pd->user));
         lreq->error = EFAULT;
         goto done;
     }
diff --git a/src/tests/cmocka/test_utils.c b/src/tests/cmocka/test_utils.c
index f225127..eeef9ee 100644
--- a/src/tests/cmocka/test_utils.c
+++ b/src/tests/cmocka/test_utils.c
@@ -654,6 +654,41 @@ void test_name_to_well_known_sid(void **state)
     assert_string_equal(sid, "S-1-5-1");
 }
 
+#define TEST_SANITIZE_INPUT "TestUser at Test.Domain"
+#define TEST_SANITIZE_LC_INPUT "testuser at test.domain"
+
+void test_sss_filter_sanitize_for_dom(void **state)
+{
+    struct dom_list_test_ctx *test_ctx;
+    int ret;
+    char *sanitized;
+    char *lc_sanitized;
+    struct sss_domain_info *dom;
+
+    test_ctx = talloc_get_type(*state, struct dom_list_test_ctx);
+    dom = test_ctx->dom_list;
+
+    dom->case_sensitive = true;
+
+    ret = sss_filter_sanitize_for_dom(test_ctx, TEST_SANITIZE_INPUT, dom,
+                                      &sanitized, &lc_sanitized);
+    assert_int_equal(ret, EOK);
+    assert_string_equal(sanitized, TEST_SANITIZE_INPUT);
+    assert_string_equal(lc_sanitized, TEST_SANITIZE_INPUT);
+    talloc_free(sanitized);
+    talloc_free(lc_sanitized);
+
+    dom->case_sensitive = false;
+
+    ret = sss_filter_sanitize_for_dom(test_ctx, TEST_SANITIZE_INPUT, dom,
+                                      &sanitized, &lc_sanitized);
+    assert_int_equal(ret, EOK);
+    assert_string_equal(sanitized, TEST_SANITIZE_INPUT);
+    assert_string_equal(lc_sanitized, TEST_SANITIZE_LC_INPUT);
+    talloc_free(sanitized);
+    talloc_free(lc_sanitized);
+}
+
 int main(int argc, const char *argv[])
 {
     poptContext pc;
@@ -688,6 +723,9 @@ int main(int argc, const char *argv[])
 
         unit_test(test_well_known_sid_to_name),
         unit_test(test_name_to_well_known_sid),
+
+        unit_test_setup_teardown(test_sss_filter_sanitize_for_dom,
+                                 setup_dom_list, teardown_dom_list),
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */
diff --git a/src/util/sss_tc_utf8.c b/src/util/sss_tc_utf8.c
index 6a97621..e1426a4 100644
--- a/src/util/sss_tc_utf8.c
+++ b/src/util/sss_tc_utf8.c
@@ -55,3 +55,33 @@ sss_tc_utf8_tolower(TALLOC_CTX *mem_ctx, const uint8_t *s, size_t len, size_t *_
     return ret;
 }
 
+errno_t sss_filter_sanitize_for_dom(TALLOC_CTX *mem_ctx,
+                                    const char *input,
+                                    struct sss_domain_info *dom,
+                                    char **sanitized,
+                                    char **lc_sanitized)
+{
+    int ret;
+
+    ret = sss_filter_sanitize(mem_ctx, input, sanitized);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("sss_filter_sanitize failed.\n"));
+        return ret;
+    }
+
+    if (dom->case_sensitive) {
+        *lc_sanitized = talloc_strdup(mem_ctx, *sanitized);
+    } else {
+        *lc_sanitized = sss_tc_utf8_str_tolower(mem_ctx, *sanitized);
+    }
+
+    if (*lc_sanitized == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("%s failed.\n",
+                                              dom->case_sensitive ?
+                                                    "talloc_strdup" :
+                                                    "sss_tc_utf8_str_tolower"));
+        return ENOMEM;
+    }
+
+    return EOK;
+}
diff --git a/src/util/util.h b/src/util/util.h
index c249955..1012705 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -511,6 +511,12 @@ errno_t sss_filter_sanitize(TALLOC_CTX *mem_ctx,
                             const char *input,
                             char **sanitized);
 
+errno_t sss_filter_sanitize_for_dom(TALLOC_CTX *mem_ctx,
+                                    const char *input,
+                                    struct sss_domain_info *dom,
+                                    char **sanitized,
+                                    char **lc_sanitized);
+
 char *
 sss_escape_ip_address(TALLOC_CTX *mem_ctx, int family, const char *addr);
 
-- 
1.8.3.1



More information about the sssd-devel mailing list