[SSSD] [PATCH] DB: sysdb_search_user_by_name: search by both name and alias

Jakub Hrozek jhrozek at redhat.com
Tue Jul 9 10:37:01 UTC 2013


On Mon, Jul 08, 2013 at 10:16:05PM +0200, Jakub Hrozek wrote:
> On Mon, Jul 08, 2013 at 04:01:26PM -0400, Stephen Gallagher wrote:
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> > 
> > On 07/08/2013 10:16 AM, Jakub Hrozek wrote:
> > > Hi,
> > > 
> > > Alexander found a long-standing bug that was revealed by the
> > > recent getsidbynam code. The getsidbynam code in the NSS responder
> > > searches users by name using sysdb_search_user_by_name() which in
> > > turn constructs the user DN based on username and sysdb domain
> > > name. But this is fragile when it comes to case-insensitive domains
> > > as the RDN contains the original name verbatim (think
> > > Administrator at AD.EXAMPLE.COM) but the input can be any combination
> > > of upper and lower case.
> > > 
> > > I think it it safest to just do a subtree search instead and search
> > > using both name and the lowercased alias. This would make the API
> > > more robust.
> > > 
> > > Also I realized that there is an ugly inconsistency when it comes
> > > to subdomain users which have a FQDN in the name attribute (and by 
> > > extension RDN). Some sysdb functions treat the name paramater in
> > > input as "plain name" and append the domain name in case the domain
> > > is a subdomain, some rely on the caller doing so. I think we
> > > should definitely standardize the API one way or another.
> > > 
> > 
> > Ack
> > 
> > Compiles cleanly and passes all sysdb tests. I'd prefer it if you
> > extended those tests to include alias lookups, but that doesn't have
> > to block this patch.
> 
> Yes, I will re-send another version but I wanted to unblock Alexander's
> testing first.
> 
> Thank you for the review.

Attached is a version of the patch that also adds a unit test. I also
attached just the unit test so you can quickly verify that the test
failed without the sysdb patch.
-------------- next part --------------
>From 4726e22e5cfc79ad3eec130032f62f3c93648fde Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 8 Jul 2013 16:04:24 +0200
Subject: [PATCH] DB: sysdb_search_user_by_name: search by both name and alias

---
 src/db/sysdb_ops.c      | 19 +++++++++++++++++--
 src/tests/sysdb-tests.c | 23 +++++++++++++++++++----
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 710a23b098398dd9c4ba9ec875e501fd6af6876d..ff8fb0085d356e5e4ca0062885284a998dfcdb0b 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -248,6 +248,8 @@ int sysdb_search_user_by_name(TALLOC_CTX *mem_ctx,
     struct ldb_message **msgs = NULL;
     struct ldb_dn *basedn;
     size_t msgs_count = 0;
+    char *sanitized_name;
+    char *filter;
     int ret;
 
     tmp_ctx = talloc_new(NULL);
@@ -255,13 +257,26 @@ int sysdb_search_user_by_name(TALLOC_CTX *mem_ctx,
         return ENOMEM;
     }
 
-    basedn = sysdb_user_dn(sysdb, tmp_ctx, domain, name);
+    basedn = ldb_dn_new_fmt(tmp_ctx, sysdb->ldb,
+                            SYSDB_TMPL_USER_BASE, domain->name);
     if (!basedn) {
         ret = ENOMEM;
         goto done;
     }
 
-    ret = sysdb_search_entry(tmp_ctx, sysdb, basedn, LDB_SCOPE_BASE, NULL,
+    ret = sss_filter_sanitize(tmp_ctx, name, &sanitized_name);
+    if (ret != EOK) {
+        goto done;
+    }
+
+    filter = talloc_asprintf(tmp_ctx, SYSDB_PWNAM_FILTER, sanitized_name,
+                             sanitized_name);
+    if (!filter) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    ret = sysdb_search_entry(tmp_ctx, sysdb, basedn, LDB_SCOPE_SUBTREE, filter,
                              attrs?attrs:def_attrs, &msgs_count, &msgs);
     if (ret) {
         goto done;
diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
index 308230f8ff1ac8d62cef6639f7baf5fcce6dd7ae..a4ca92a65de7e5e679afc8e367a99a2538f9d960 100644
--- a/src/tests/sysdb-tests.c
+++ b/src/tests/sysdb-tests.c
@@ -4535,6 +4535,9 @@ START_TEST(test_sysdb_subdomain_store_user)
     struct ldb_result *results = NULL;
     struct ldb_dn *base_dn = NULL;
     struct ldb_dn *check_dn = NULL;
+    const char *attrs[] = { SYSDB_NAME, SYSDB_NAME_ALIAS, NULL };
+    struct sysdb_attrs *user_attrs;
+    struct ldb_message *msg;
 
     ret = setup_sysdb_tests(&test_ctx);
     fail_if(ret != EOK, "Could not set up the test");
@@ -4552,21 +4555,27 @@ START_TEST(test_sysdb_subdomain_store_user)
     fail_unless(ret == EOK, "sysdb_update_subdomains failed with [%d][%s]",
                             ret, strerror(ret));
 
-    ret = sysdb_store_user(subdomain->sysdb, subdomain, "subdomuser",
+    user_attrs = sysdb_new_attrs(test_ctx);
+    fail_unless(user_attrs != NULL, "sysdb_new_attrs failed");
+
+    ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS, "subdomuser");
+    fail_unless(ret == EOK, "sysdb_store_user failed.");
+
+    ret = sysdb_store_user(subdomain->sysdb, subdomain, "SubDomUser",
                            NULL, 12345, 0, "Sub Domain User",
                            "/home/subdomuser", "/bin/bash",
-                           NULL, NULL, NULL, -1, 0);
+                           NULL, user_attrs, NULL, -1, 0);
     fail_unless(ret == EOK, "sysdb_store_user failed.");
 
     base_dn =ldb_dn_new(test_ctx, test_ctx->sysdb->ldb, "cn=sysdb");
     fail_unless(base_dn != NULL);
 
     check_dn = ldb_dn_new(test_ctx, test_ctx->sysdb->ldb,
-                          "name=subdomuser,cn=users,cn=test.sub,cn=sysdb");
+                          "name=SubDomUser,cn=users,cn=test.sub,cn=sysdb");
     fail_unless(check_dn != NULL);
 
     ret = ldb_search(test_ctx->sysdb->ldb, test_ctx, &results, base_dn,
-                     LDB_SCOPE_SUBTREE, NULL, "name=subdomuser");
+                     LDB_SCOPE_SUBTREE, NULL, "name=SubDomUser");
     fail_unless(ret == EOK, "ldb_search failed.");
     fail_unless(results->count == 1, "Unexpected number of results, "
                                      "expected [%d], got [%d]",
@@ -4574,6 +4583,12 @@ START_TEST(test_sysdb_subdomain_store_user)
     fail_unless(ldb_dn_compare(results->msgs[0]->dn, check_dn) == 0,
                 "Unexpedted DN returned");
 
+    /* Subdomains are case-insensitive. Test that the lowercased name
+     * can be found, too */
+    ret = sysdb_search_user_by_name(test_ctx, test_ctx->sysdb, subdomain,
+                                    "subdomuser", attrs, &msg);
+    fail_unless(ret == EOK, "sysdb_search_user_by_name failed.");
+
     ret = sysdb_delete_user(subdomain->sysdb, subdomain, "subdomuser", 0);
     fail_unless(ret == EOK, "sysdb_delete_user failed [%d][%s].",
                             ret, strerror(ret));
-- 
1.8.3.1

-------------- next part --------------
>From dbe26f53d95af1178cd9db8c01596ef66109680d Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 9 Jul 2013 12:33:34 +0200
Subject: [PATCH] Unit test to exercise searching users by alias

---
 src/tests/sysdb-tests.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
index 308230f8ff1ac8d62cef6639f7baf5fcce6dd7ae..a4ca92a65de7e5e679afc8e367a99a2538f9d960 100644
--- a/src/tests/sysdb-tests.c
+++ b/src/tests/sysdb-tests.c
@@ -4535,6 +4535,9 @@ START_TEST(test_sysdb_subdomain_store_user)
     struct ldb_result *results = NULL;
     struct ldb_dn *base_dn = NULL;
     struct ldb_dn *check_dn = NULL;
+    const char *attrs[] = { SYSDB_NAME, SYSDB_NAME_ALIAS, NULL };
+    struct sysdb_attrs *user_attrs;
+    struct ldb_message *msg;
 
     ret = setup_sysdb_tests(&test_ctx);
     fail_if(ret != EOK, "Could not set up the test");
@@ -4552,21 +4555,27 @@ START_TEST(test_sysdb_subdomain_store_user)
     fail_unless(ret == EOK, "sysdb_update_subdomains failed with [%d][%s]",
                             ret, strerror(ret));
 
-    ret = sysdb_store_user(subdomain->sysdb, subdomain, "subdomuser",
+    user_attrs = sysdb_new_attrs(test_ctx);
+    fail_unless(user_attrs != NULL, "sysdb_new_attrs failed");
+
+    ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS, "subdomuser");
+    fail_unless(ret == EOK, "sysdb_store_user failed.");
+
+    ret = sysdb_store_user(subdomain->sysdb, subdomain, "SubDomUser",
                            NULL, 12345, 0, "Sub Domain User",
                            "/home/subdomuser", "/bin/bash",
-                           NULL, NULL, NULL, -1, 0);
+                           NULL, user_attrs, NULL, -1, 0);
     fail_unless(ret == EOK, "sysdb_store_user failed.");
 
     base_dn =ldb_dn_new(test_ctx, test_ctx->sysdb->ldb, "cn=sysdb");
     fail_unless(base_dn != NULL);
 
     check_dn = ldb_dn_new(test_ctx, test_ctx->sysdb->ldb,
-                          "name=subdomuser,cn=users,cn=test.sub,cn=sysdb");
+                          "name=SubDomUser,cn=users,cn=test.sub,cn=sysdb");
     fail_unless(check_dn != NULL);
 
     ret = ldb_search(test_ctx->sysdb->ldb, test_ctx, &results, base_dn,
-                     LDB_SCOPE_SUBTREE, NULL, "name=subdomuser");
+                     LDB_SCOPE_SUBTREE, NULL, "name=SubDomUser");
     fail_unless(ret == EOK, "ldb_search failed.");
     fail_unless(results->count == 1, "Unexpected number of results, "
                                      "expected [%d], got [%d]",
@@ -4574,6 +4583,12 @@ START_TEST(test_sysdb_subdomain_store_user)
     fail_unless(ldb_dn_compare(results->msgs[0]->dn, check_dn) == 0,
                 "Unexpedted DN returned");
 
+    /* Subdomains are case-insensitive. Test that the lowercased name
+     * can be found, too */
+    ret = sysdb_search_user_by_name(test_ctx, test_ctx->sysdb, subdomain,
+                                    "subdomuser", attrs, &msg);
+    fail_unless(ret == EOK, "sysdb_search_user_by_name failed.");
+
     ret = sysdb_delete_user(subdomain->sysdb, subdomain, "subdomuser", 0);
     fail_unless(ret == EOK, "sysdb_delete_user failed [%d][%s].",
                             ret, strerror(ret));
-- 
1.8.3.1



More information about the sssd-devel mailing list