[SSSD] [PATCH] nss_check_name_of_well_known_sid() improve name splitting

Sumit Bose sbose at redhat.com
Tue Jul 14 13:02:07 UTC 2015


Hi,

this patch should fix https://fedorahosted.org/sssd/ticket/2717 .

As you can see I added a new entry ipa_ad_default_names to the global
nss context so that the regular expression string is only evaluated
once. Since it is currently only used in
nss_check_name_of_well_known_sid() I do the initialization here to avoid
initialization when it is not needed. If you think this is too risky in
future I'm fine with moving the initialization to the general
initialization of the nss context.

bye,
Sumit
-------------- next part --------------
From 1ff1e1f5fb57dd7e1fa85eb758b8769dfb1260d0 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 14 Jul 2015 14:41:34 +0200
Subject: [PATCH] nss_check_name_of_well_known_sid() improve name splitting

Currently in the default configuration
nss_check_name_of_well_known_sid() can only split fully-qualified names
in the user at domain.name style. DOM\user style names will cause an error
and terminate the whole request.

With this patch both styles can be handled by default, additionally if
the name could not be split nss_check_name_of_well_known_sid() returns
ENOENT which can be handled more gracefully by the caller.

Resolves https://fedorahosted.org/sssd/ticket/2717
---
 src/responder/nss/nsssrv.h      |  1 +
 src/responder/nss/nsssrv_cmd.c  | 32 ++++++++++++++-
 src/tests/cmocka/test_nss_srv.c | 90 ++++++++++++++++++++++++-----------------
 src/util/usertools.c            |  7 ++++
 src/util/util.h                 |  3 ++
 5 files changed, 94 insertions(+), 39 deletions(-)

diff --git a/src/responder/nss/nsssrv.h b/src/responder/nss/nsssrv.h
index e293e3b4d03582abf6abf07cce61d3b6fdebfcae..6a77f9f1179e0bb2353a01edf14a27e576eba481 100644
--- a/src/responder/nss/nsssrv.h
+++ b/src/responder/nss/nsssrv.h
@@ -76,6 +76,7 @@ struct nss_ctx {
 
     struct sss_idmap_ctx *idmap_ctx;
     struct sss_names_ctx *global_names;
+    struct sss_names_ctx *ipa_ad_default_names;
 
     const char **extra_attributes;
 };
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 0129467302f16af318bbbb0a5be47ff2e235da65..eb550cd007cfb6fac0271b7de211285b6cebfdef 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -1248,13 +1248,43 @@ static int nss_check_name_of_well_known_sid(struct nss_cmd_ctx *cmdctx,
     size_t pctr = 0;
 
     nss_ctx = talloc_get_type(cmdctx->cctx->rctx->pvt_ctx, struct nss_ctx);
-    ret = sss_parse_name(cmdctx, nss_ctx->global_names, full_name,
+
+    if (nss_ctx->ipa_ad_default_names == NULL) {
+        ret = get_default_ipa_ad_names(nss_ctx,
+                                       &nss_ctx->ipa_ad_default_names);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE,
+                  "get_default_ipa_ad_names failed, using global_names.\n");
+
+            /* Instead of failing try the next best. */
+            nss_ctx->ipa_ad_default_names = nss_ctx->global_names;
+        }
+    }
+
+    ret = sss_parse_name(cmdctx, nss_ctx->ipa_ad_default_names, full_name,
                          &wk_dom_name, &wk_name);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "sss_parse_name failed.\n");
         return ret;
     }
 
+    if (wk_dom_name == NULL || wk_name == NULL) {
+        ret = sss_parse_name(cmdctx, nss_ctx->global_names, full_name,
+                             &wk_dom_name, &wk_name);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, "sss_parse_name failed.\n");
+            return ret;
+        }
+
+        if (wk_dom_name == NULL || wk_name == NULL) {
+            DEBUG(SSSDBG_OP_FAILURE,
+                  "Unable to split [%s] in name and domain part. " \
+                  "Skipping check for well-known name.\n", full_name);
+
+            return ENOENT;
+        }
+    }
+
     ret = name_to_well_known_sid(wk_dom_name, wk_name, &wk_sid);
     talloc_free(wk_dom_name);
     talloc_free(wk_name);
diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c
index d1a4c16851427a36b123f04cecee5fe5ae2d333d..effd1973b3c1e629fbb215c3fac750b3a34d3e40 100644
--- a/src/tests/cmocka/test_nss_srv.c
+++ b/src/tests/cmocka/test_nss_srv.c
@@ -1737,63 +1737,77 @@ void test_nss_well_known_getidbysid_failure(void **state)
 void test_nss_well_known_getsidbyname(void **state)
 {
     errno_t ret;
+    const char *names[] = { "Cryptographic Operators at BUILTIN",
+                            "BUILTIN\\Cryptographic Operators", NULL};
+    size_t c;
 
-    will_return(__wrap_sss_packet_get_body, WRAP_CALL_WRAPPER);
-    will_return(__wrap_sss_packet_get_body, "Cryptographic Operators at BUILTIN");
-    will_return(__wrap_sss_packet_get_body, 0);
-    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETSIDBYNAME);
-    will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
-    will_return(test_nss_well_known_sid_check, "S-1-5-32-569");
+    for (c = 0; names[c] != NULL; c++) {
+        will_return(__wrap_sss_packet_get_body, WRAP_CALL_WRAPPER);
+        will_return(__wrap_sss_packet_get_body, names[c]);
+        will_return(__wrap_sss_packet_get_body, 0);
+        will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETSIDBYNAME);
+        will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
+        will_return(test_nss_well_known_sid_check, "S-1-5-32-569");
 
-    set_cmd_cb(test_nss_well_known_sid_check);
-    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETSIDBYNAME,
-                          nss_test_ctx->nss_cmds);
-    assert_int_equal(ret, EOK);
+        set_cmd_cb(test_nss_well_known_sid_check);
+        ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETSIDBYNAME,
+                              nss_test_ctx->nss_cmds);
+        assert_int_equal(ret, EOK);
 
-    /* Wait until the test finishes with EOK */
-    ret = test_ev_loop(nss_test_ctx->tctx);
-    assert_int_equal(ret, EOK);
+        /* Wait until the test finishes with EOK */
+        ret = test_ev_loop(nss_test_ctx->tctx);
+        assert_int_equal(ret, EOK);
+    }
 }
 
 void test_nss_well_known_getsidbyname_nonexisting(void **state)
 {
     errno_t ret;
+    const char *names[] = { "Abc at BUILTIN", "BUILTIN\\Abc", NULL };
+    size_t c;
 
-    will_return(__wrap_sss_packet_get_body, WRAP_CALL_WRAPPER);
-    will_return(__wrap_sss_packet_get_body, "Abc at BUILTIN");
-    will_return(__wrap_sss_packet_get_body, 0);
-    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETSIDBYNAME);
-    will_return(test_nss_well_known_sid_check, NULL);
+    for (c = 0; names[c] != NULL; c++) {
+        will_return(__wrap_sss_packet_get_body, WRAP_CALL_WRAPPER);
+        will_return(__wrap_sss_packet_get_body, names[c]);
+        will_return(__wrap_sss_packet_get_body, 0);
+        will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETSIDBYNAME);
+        will_return(test_nss_well_known_sid_check, NULL);
 
-    set_cmd_cb(test_nss_well_known_sid_check);
-    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETSIDBYNAME,
-                          nss_test_ctx->nss_cmds);
-    assert_int_equal(ret, EOK);
+        set_cmd_cb(test_nss_well_known_sid_check);
+        ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETSIDBYNAME,
+                              nss_test_ctx->nss_cmds);
+        assert_int_equal(ret, EOK);
 
-    /* Wait until the test finishes with EOK */
-    ret = test_ev_loop(nss_test_ctx->tctx);
-    assert_int_equal(ret, EOK);
+        /* Wait until the test finishes with EOK */
+        ret = test_ev_loop(nss_test_ctx->tctx);
+        assert_int_equal(ret, EOK);
+    }
 }
 
 void test_nss_well_known_getsidbyname_special(void **state)
 {
     errno_t ret;
+    const char *names[] = { "CREATOR OWNER at CREATOR AUTHORITY",
+                            "CREATOR AUTHORITY\\CREATOR OWNER", NULL };
+    size_t c;
 
-    will_return(__wrap_sss_packet_get_body, WRAP_CALL_WRAPPER);
-    will_return(__wrap_sss_packet_get_body, "CREATOR OWNER at CREATOR AUTHORITY");
-    will_return(__wrap_sss_packet_get_body, 0);
-    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETSIDBYNAME);
-    will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
-    will_return(test_nss_well_known_sid_check, "S-1-3-0");
+    for (c = 0; names[c] != NULL; c++) {
+        will_return(__wrap_sss_packet_get_body, WRAP_CALL_WRAPPER);
+        will_return(__wrap_sss_packet_get_body, names[c]);
+        will_return(__wrap_sss_packet_get_body, 0);
+        will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETSIDBYNAME);
+        will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
+        will_return(test_nss_well_known_sid_check, "S-1-3-0");
 
-    set_cmd_cb(test_nss_well_known_sid_check);
-    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETSIDBYNAME,
-                          nss_test_ctx->nss_cmds);
-    assert_int_equal(ret, EOK);
+        set_cmd_cb(test_nss_well_known_sid_check);
+        ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETSIDBYNAME,
+                              nss_test_ctx->nss_cmds);
+        assert_int_equal(ret, EOK);
 
-    /* Wait until the test finishes with EOK */
-    ret = test_ev_loop(nss_test_ctx->tctx);
-    assert_int_equal(ret, EOK);
+        /* Wait until the test finishes with EOK */
+        ret = test_ev_loop(nss_test_ctx->tctx);
+        assert_int_equal(ret, EOK);
+    }
 }
 
 static int test_nss_getorigbyname_check(uint32_t status, uint8_t *body,
diff --git a/src/util/usertools.c b/src/util/usertools.c
index c43d420e31c6c690628ef6179d932eaf99826fee..e4b76f6ac4727d9966e393a6a4103952bad23859 100644
--- a/src/util/usertools.c
+++ b/src/util/usertools.c
@@ -295,6 +295,13 @@ done:
     return ret;
 }
 
+errno_t get_default_ipa_ad_names(TALLOC_CTX *mem_ctx,
+                                 struct sss_names_ctx **out)
+{
+    return sss_names_init_from_args(mem_ctx, IPA_AD_DEFAULT_RE,
+                                    CONFDB_DEFAULT_FULL_NAME_FORMAT, out);
+}
+
 int sss_parse_name(TALLOC_CTX *memctx,
                    struct sss_names_ctx *snctx,
                    const char *orig, char **_domain, char **_name)
diff --git a/src/util/util.h b/src/util/util.h
index 3d90cf0d1024b93016987a4d3e8a515359fd974d..cf197969ff778cb2eea94ccb483e2c0dd4978971 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -298,6 +298,9 @@ int sss_names_init(TALLOC_CTX *mem_ctx,
                    const char *domain,
                    struct sss_names_ctx **out);
 
+errno_t get_default_ipa_ad_names(TALLOC_CTX *mem_ctx,
+                                 struct sss_names_ctx **out);
+
 int sss_parse_name(TALLOC_CTX *memctx,
                    struct sss_names_ctx *snctx,
                    const char *orig, char **_domain, char **_name);
-- 
2.1.0



More information about the sssd-devel mailing list