[SSSD] [PATCH] NSS: Handle ENOENT when doing initgroups by UPN

Jakub Hrozek jhrozek at redhat.com
Wed Mar 11 14:46:01 UTC 2015


On Mon, Mar 09, 2015 at 06:17:50PM +0100, Jakub Hrozek wrote:
> Hi,
> 
> the attached patches fix the crash as described in
> https://fedorahosted.org/sssd/ticket/2598 and add a unit test. I hope
> the patch descriptions are fine.
> 
> Thanks for review.

attached are patches rebased on top of my cmocka test changes.
-------------- next part --------------
>From c14978d23957b57b33a223ebc3f103151c0cd39f Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 9 Mar 2015 17:26:54 +0100
Subject: [PATCH 1/4] tests: ncache_hit must be an int to test UPNs

In order to detect faulty cases where negcache would be checked twice,
we need to convert the ncache_hit to integer and check exact amounts of
hits.
---
 src/tests/cmocka/test_nss_srv.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c
index 4dbda3bd605a9559658cec58009c762c77dd1b1a..bf8bddb263f1f0b8eae1293c021fa958f329f1e0 100644
--- a/src/tests/cmocka/test_nss_srv.c
+++ b/src/tests/cmocka/test_nss_srv.c
@@ -48,7 +48,7 @@ struct nss_test_ctx {
     struct sss_cmd_table *nss_cmds;
     struct nss_ctx *nctx;
 
-    bool ncache_hit;
+    int ncache_hits;
 };
 
 const char *global_extra_attrs[] = {"phone", "mobile", NULL};
@@ -159,7 +159,7 @@ int __wrap_sss_ncache_check_user(struct sss_nc_ctx *ctx, int ttl,
 
     ret = __real_sss_ncache_check_user(ctx, ttl, dom, name);
     if (ret == EEXIST) {
-        nss_test_ctx->ncache_hit = true;
+        nss_test_ctx->ncache_hits++;
     }
     return ret;
 }
@@ -172,7 +172,7 @@ int __wrap_sss_ncache_check_uid(struct sss_nc_ctx *ctx, int ttl, uid_t uid)
 
     ret = __real_sss_ncache_check_uid(ctx, ttl, uid);
     if (ret == EEXIST) {
-        nss_test_ctx->ncache_hit = true;
+        nss_test_ctx->ncache_hits++;
     }
     return ret;
 }
@@ -347,7 +347,7 @@ void test_nss_getpwnam_neg(void **state)
     mock_input_user_or_group("testuser_neg");
     mock_account_recv_simple();
 
-    assert_true(nss_test_ctx->ncache_hit == false);
+    assert_int_equal(nss_test_ctx->ncache_hits, 0);
 
     ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWNAM,
                           nss_test_ctx->nss_cmds);
@@ -356,7 +356,7 @@ void test_nss_getpwnam_neg(void **state)
     /* Wait until the test finishes with ENOENT */
     ret = test_ev_loop(nss_test_ctx->tctx);
     assert_int_equal(ret, ENOENT);
-    assert_true(nss_test_ctx->ncache_hit == false);
+    assert_int_equal(nss_test_ctx->ncache_hits, 0);
 
     /* Test that subsequent search for a nonexistent user yields
      * ENOENT and Account callback is not called, on the other hand
@@ -373,7 +373,7 @@ void test_nss_getpwnam_neg(void **state)
     ret = test_ev_loop(nss_test_ctx->tctx);
     assert_int_equal(ret, ENOENT);
     /* Negative cache was hit this time */
-    assert_true(nss_test_ctx->ncache_hit == true);
+    assert_int_equal(nss_test_ctx->ncache_hits, 1);
 }
 
 static int test_nss_getpwnam_search_acct_cb(void *pvt)
@@ -791,7 +791,7 @@ void test_nss_getpwuid_neg(void **state)
     mock_input_id(nss_test_ctx, id);
     mock_account_recv_simple();
 
-    assert_true(nss_test_ctx->ncache_hit == false);
+    assert_int_equal(nss_test_ctx->ncache_hits, 0);
 
     ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWUID,
                           nss_test_ctx->nss_cmds);
@@ -800,7 +800,7 @@ void test_nss_getpwuid_neg(void **state)
     /* Wait until the test finishes with ENOENT */
     ret = test_ev_loop(nss_test_ctx->tctx);
     assert_int_equal(ret, ENOENT);
-    assert_true(nss_test_ctx->ncache_hit == false);
+    assert_int_equal(nss_test_ctx->ncache_hits, 0);
 
     /* Test that subsequent search for a nonexistent id yields
      * ENOENT and Account callback is not called, on the other hand
@@ -817,7 +817,7 @@ void test_nss_getpwuid_neg(void **state)
     ret = test_ev_loop(nss_test_ctx->tctx);
     assert_int_equal(ret, ENOENT);
     /* Negative cache was hit this time */
-    assert_true(nss_test_ctx->ncache_hit == true);
+    assert_int_equal(nss_test_ctx->ncache_hits, 1);
 }
 
 static int test_nss_getpwuid_search_acct_cb(void *pvt)
-- 
2.1.0

-------------- next part --------------
>From 4cbd5783da8cd4173ac4c7dd33a0a8af023de8bd Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 9 Mar 2015 15:41:56 +0100
Subject: [PATCH 2/4] tests: Add a getpwnam-by-UPN test

---
 src/tests/cmocka/test_nss_srv.c | 97 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c
index bf8bddb263f1f0b8eae1293c021fa958f329f1e0..2e95bc0f9a92e702a37bc21f029254691a706025 100644
--- a/src/tests/cmocka/test_nss_srv.c
+++ b/src/tests/cmocka/test_nss_srv.c
@@ -2071,6 +2071,99 @@ void test_nss_getorigbyname_multi_value_attrs(void **state)
     assert_int_equal(ret, EOK);
 }
 
+static int test_nss_getpwnam_upn_check(uint32_t status,
+                                       uint8_t *body,
+                                       size_t blen)
+{
+    struct passwd pwd;
+    errno_t ret;
+
+    assert_int_equal(status, EOK);
+
+    ret = parse_user_packet(body, blen, &pwd);
+    assert_int_equal(ret, EOK);
+
+    assert_int_equal(pwd.pw_uid, 34567);
+    assert_int_equal(pwd.pw_gid, 45678);
+    assert_string_equal(pwd.pw_name, "upnuser");
+    assert_string_equal(pwd.pw_shell, "/bin/sh");
+    assert_string_equal(pwd.pw_passwd, "*");
+    return EOK;
+}
+
+void test_nss_getpwnam_upn(void **state)
+{
+    errno_t ret;
+    struct sysdb_attrs *attrs;
+
+    attrs = sysdb_new_attrs(nss_test_ctx);
+    assert_non_null(attrs);
+
+    ret = sysdb_attrs_add_string(attrs, SYSDB_UPN, "upnuser at upndomain.test");
+    assert_int_equal(ret, EOK);
+
+    /* Prime the cache with a valid user */
+    ret = sysdb_add_user(nss_test_ctx->tctx->dom,
+                         "upnuser", 34567, 45678, "up user",
+                         "/home/upnuser", "/bin/sh", NULL,
+                         attrs, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    mock_input_user_or_group("upnuser at upndomain.test");
+    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETPWNAM);
+    mock_fill_user();
+
+    /* Query for that user, call a callback when command finishes */
+    set_cmd_cb(test_nss_getpwnam_upn_check);
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWNAM,
+                          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);
+}
+
+/* Test that searching for a nonexistant user yields ENOENT.
+ * Account callback will be called
+ */
+void test_nss_getpwnam_upn_neg(void **state)
+{
+    errno_t ret;
+
+    mock_input_user_or_group("nosuchupnuser at upndomain.test");
+    mock_account_recv_simple();
+
+    assert_int_equal(nss_test_ctx->ncache_hits, 0);
+
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWNAM,
+                          nss_test_ctx->nss_cmds);
+    assert_int_equal(ret, EOK);
+
+    /* Wait until the test finishes with ENOENT */
+    ret = test_ev_loop(nss_test_ctx->tctx);
+    assert_int_equal(ret, ENOENT);
+    assert_int_equal(nss_test_ctx->ncache_hits, 1);
+
+    /* Test that subsequent search for a nonexistent user yields
+     * ENOENT and Account callback is not called, on the other hand
+     * the ncache functions will be called
+     */
+    nss_test_ctx->tctx->done = false;
+    nss_test_ctx->ncache_hits = 0;
+
+    mock_input_user_or_group("nosuchupnuser at upndomain.test");
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWNAM,
+                          nss_test_ctx->nss_cmds);
+    assert_int_equal(ret, EOK);
+
+    /* Wait until the test finishes with ENOENT */
+    ret = test_ev_loop(nss_test_ctx->tctx);
+    assert_int_equal(ret, ENOENT);
+    /* Negative cache was hit this time */
+    assert_int_equal(nss_test_ctx->ncache_hits, 1);
+}
+
 static int nss_test_setup(void **state)
 {
     struct sss_test_conf_param params[] = {
@@ -2239,6 +2332,10 @@ int main(int argc, const char *argv[])
         cmocka_unit_test_setup_teardown(test_nss_getorigbyname_multi_value_attrs,
                                         nss_test_setup_extra_attr,
                                         nss_test_teardown),
+        cmocka_unit_test_setup_teardown(test_nss_getpwnam_upn,
+                                        nss_test_setup, nss_test_teardown),
+        cmocka_unit_test_setup_teardown(test_nss_getpwnam_upn_neg,
+                                        nss_test_setup, nss_test_teardown),
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */
-- 
2.1.0

-------------- next part --------------
>From c0daf41e1c9ca1e8331dc9e5e7098e7862fd8b7e Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 9 Mar 2015 16:59:39 +0100
Subject: [PATCH 3/4] Add unit tests for initgroups

---
 src/tests/cmocka/test_nss_srv.c | 346 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 346 insertions(+)

diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c
index 2e95bc0f9a92e702a37bc21f029254691a706025..480d65aec502cd60e11a7a23911c7c7afb59833b 100644
--- a/src/tests/cmocka/test_nss_srv.c
+++ b/src/tests/cmocka/test_nss_srv.c
@@ -39,6 +39,9 @@
 #define TEST_SUBDOM_NAME "test.subdomain"
 #define TEST_ID_PROVIDER "ldap"
 
+#define N_ELEMENTS(arr) \
+    (sizeof(arr) / sizeof(arr[0]))
+
 struct nss_test_ctx {
     struct sss_test_ctx *tctx;
     struct sss_domain_info *subdom;
@@ -205,6 +208,11 @@ static void mock_fill_user(void)
     will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
 }
 
+static void mock_fill_initgr_user(void)
+{
+    will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
+}
+
 static void mock_fill_group_with_members(unsigned members)
 {
     unsigned i;
@@ -287,6 +295,26 @@ static int parse_group_packet(uint8_t *body, size_t blen, struct group *gr, uint
     return EOK;
 }
 
+static void check_initgr_packet(uint8_t *body, size_t blen,
+                                gid_t *gids, size_t num_gids)
+{
+    size_t rp;
+    unsigned i;
+    gid_t cur_gid;
+    uint32_t num_ret;
+
+    rp = 0;
+    SAFEALIGN_COPY_UINT32(&num_ret, body, NULL);
+    assert_int_equal(num_ret, num_gids);
+
+    rp = 2 * sizeof(uint32_t); /* Len and reserved */
+
+    for (i = 0; i < num_gids; i++) {
+        SAFEALIGN_COPY_UINT32(&cur_gid, body + rp, &rp);
+        assert_int_equal(cur_gid, gids[i]);
+    }
+}
+
 /* ====================== The tests =============================== */
 
 /* Check getting cached and valid user from cache. Account callback will
@@ -2164,6 +2192,312 @@ void test_nss_getpwnam_upn_neg(void **state)
     assert_int_equal(nss_test_ctx->ncache_hits, 1);
 }
 
+static int test_nss_initgr_check(uint32_t status, uint8_t *body, size_t blen)
+{
+    gid_t expected_gids[] = { 3211, 3212 };
+
+    assert_int_equal(status, EOK);
+    check_initgr_packet(body, blen, expected_gids, N_ELEMENTS(expected_gids));
+    return EOK;
+}
+
+void test_nss_initgroups(void **state)
+{
+    errno_t ret;
+    struct sysdb_attrs *attrs;
+
+    attrs = sysdb_new_attrs(nss_test_ctx);
+    assert_non_null(attrs);
+
+    ret = sysdb_attrs_add_uint32(attrs, SYSDB_INITGR_EXPIRE,
+                                 time(NULL) + 300);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_attrs_add_string(attrs, SYSDB_UPN, "upninitgr at upndomain.test");
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_user(nss_test_ctx->tctx->dom,
+                         "testinitgr", 321, 654, "test initgroups",
+                         "/home/testinitgr", "/bin/sh", NULL,
+                         attrs, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group(nss_test_ctx->tctx->dom,
+                          "testinitgr_gr1", 3211,
+                          NULL, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group(nss_test_ctx->tctx->dom,
+                          "testinitgr_gr2", 3212,
+                          NULL, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group_member(nss_test_ctx->tctx->dom,
+                                 "testinitgr_gr1", "testinitgr",
+                                 SYSDB_MEMBER_USER, false);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group_member(nss_test_ctx->tctx->dom,
+                                 "testinitgr_gr2", "testinitgr",
+                                 SYSDB_MEMBER_USER, false);
+    assert_int_equal(ret, EOK);
+
+    mock_input_user_or_group("testinitgr");
+    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_INITGR);
+    mock_fill_initgr_user();
+
+    /* Query for that user, call a callback when command finishes */
+    set_cmd_cb(test_nss_initgr_check);
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_INITGR,
+                          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);
+}
+
+/* Test that searching for a nonexistant user yields ENOENT.
+ * Account callback will be called
+ */
+void test_initgr_neg_by_name(const char *name, bool is_upn)
+{
+    errno_t ret;
+
+    mock_input_user_or_group(name);
+    mock_account_recv_simple();
+
+    assert_int_equal(nss_test_ctx->ncache_hits, 0);
+
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_INITGR,
+                          nss_test_ctx->nss_cmds);
+    assert_int_equal(ret, EOK);
+
+    /* Wait until the test finishes with ENOENT */
+    ret = test_ev_loop(nss_test_ctx->tctx);
+    assert_int_equal(ret, ENOENT);
+    /* UPN lookup will first hit negcache with the username */
+    assert_int_equal(nss_test_ctx->ncache_hits, is_upn ? 1 : 0);
+
+    /* Test that subsequent search for a nonexistent user yields
+     * ENOENT and Account callback is not called, on the other hand
+     * the ncache functions will be called
+     */
+    nss_test_ctx->tctx->done = false;
+    nss_test_ctx->ncache_hits = 0;
+
+    mock_input_user_or_group(name);
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_INITGR,
+                          nss_test_ctx->nss_cmds);
+    assert_int_equal(ret, EOK);
+
+    /* Wait until the test finishes with ENOENT */
+    ret = test_ev_loop(nss_test_ctx->tctx);
+    assert_int_equal(ret, ENOENT);
+    /* Negative cache was hit this time */
+    assert_int_equal(nss_test_ctx->ncache_hits, 1);
+}
+
+void test_nss_initgr_neg(void **state)
+{
+    test_initgr_neg_by_name("testinitgr_neg", false);
+}
+
+static int test_nss_initgr_search_acct_cb(void *pvt)
+{
+    errno_t ret;
+    struct sysdb_attrs *attrs;
+
+    attrs = sysdb_new_attrs(nss_test_ctx);
+    assert_non_null(attrs);
+
+    ret = sysdb_attrs_add_uint32(attrs, SYSDB_INITGR_EXPIRE,
+                                 time(NULL) + 300);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_user(nss_test_ctx->tctx->dom,
+                         "testinitgr_srch", 421, 654, "test initgroups",
+                         "/home/testinitgr", "/bin/sh", NULL,
+                         attrs, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group(nss_test_ctx->tctx->dom,
+                          "testinitgr_srch_gr1", 4211,
+                          NULL, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group(nss_test_ctx->tctx->dom,
+                          "testinitgr_srch_gr2", 4212,
+                          NULL, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group_member(nss_test_ctx->tctx->dom,
+                                 "testinitgr_srch_gr1", "testinitgr_srch",
+                                 SYSDB_MEMBER_USER, false);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group_member(nss_test_ctx->tctx->dom,
+                                 "testinitgr_srch_gr2", "testinitgr_srch",
+                                 SYSDB_MEMBER_USER, false);
+    assert_int_equal(ret, EOK);
+
+
+    return EOK;
+}
+
+static int test_nss_initgr_search_check(uint32_t status,
+                                        uint8_t *body, size_t blen)
+{
+    gid_t expected_gids[] = { 4211, 4212 };
+
+    assert_int_equal(status, EOK);
+    check_initgr_packet(body, blen, expected_gids, N_ELEMENTS(expected_gids));
+    return EOK;
+}
+
+void test_nss_initgr_search(void **state)
+{
+    errno_t ret;
+    struct ldb_result *res;
+
+    mock_input_user_or_group("testinitgr_srch");
+    mock_account_recv(0, 0, NULL, test_nss_initgr_search_acct_cb, nss_test_ctx);
+    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_INITGR);
+    mock_fill_initgr_user();
+    set_cmd_cb(test_nss_initgr_search_check);
+
+    ret = sysdb_getpwnam(nss_test_ctx, nss_test_ctx->tctx->dom,
+                         "testinitgr_srch", &res);
+    assert_int_equal(ret, EOK);
+    assert_int_equal(res->count, 0);
+
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_INITGR,
+                          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);
+
+    /* test_nss_getpwnam_search_check will check the user attributes */
+    ret = sysdb_getpwnam(nss_test_ctx, nss_test_ctx->tctx->dom,
+                         "testinitgr_srch", &res);
+    assert_int_equal(ret, EOK);
+    assert_int_equal(res->count, 1);
+}
+
+static int test_nss_initgr_update_acct_cb(void *pvt)
+{
+    errno_t ret;
+    struct sysdb_attrs *attrs;
+
+    attrs = sysdb_new_attrs(nss_test_ctx);
+    assert_non_null(attrs);
+
+    ret = sysdb_attrs_add_uint32(attrs, SYSDB_INITGR_EXPIRE,
+                                 time(NULL) + 300);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_set_user_attr(nss_test_ctx->tctx->dom,
+                              "testinitgr_update",
+                              attrs, SYSDB_MOD_REP);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group(nss_test_ctx->tctx->dom,
+                          "testinitgr_check_gr2", 5212,
+                          NULL, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group_member(nss_test_ctx->tctx->dom,
+                                 "testinitgr_check_gr2",
+                                 "testinitgr_update",
+                                 SYSDB_MEMBER_USER, false);
+    assert_int_equal(ret, EOK);
+
+    return EOK;
+}
+
+static int test_nss_initgr_update_check(uint32_t status, uint8_t *body, size_t blen)
+{
+    gid_t expected_gids[] = { 5211, 5212 };
+
+    assert_int_equal(status, EOK);
+    check_initgr_packet(body, blen, expected_gids, N_ELEMENTS(expected_gids));
+    return EOK;
+}
+
+void test_nss_initgr_update(void **state)
+{
+    errno_t ret;
+    struct sysdb_attrs *attrs;
+
+    attrs = sysdb_new_attrs(nss_test_ctx);
+    assert_non_null(attrs);
+
+    ret = sysdb_attrs_add_uint32(attrs, SYSDB_INITGR_EXPIRE,
+                                 time(NULL) - 1);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_user(nss_test_ctx->tctx->dom,
+                         "testinitgr_update", 521, 654, "test initgroups",
+                         "/home/testinitgr", "/bin/sh", NULL,
+                         attrs, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group(nss_test_ctx->tctx->dom,
+                          "testinitgr_update_gr1", 5211,
+                          NULL, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group_member(nss_test_ctx->tctx->dom,
+                                 "testinitgr_update_gr1", "testinitgr_update",
+                                 SYSDB_MEMBER_USER, false);
+    assert_int_equal(ret, EOK);
+
+    mock_input_user_or_group("testinitgr_update");
+    mock_account_recv(0, 0, NULL, test_nss_initgr_update_acct_cb, nss_test_ctx);
+    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_INITGR);
+    mock_fill_initgr_user();
+    set_cmd_cb(test_nss_initgr_update_check);
+
+
+    /* Query for that user, call a callback when command finishes */
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_INITGR,
+                          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);
+}
+
+void test_nss_initgroups_upn(void **state)
+{
+    errno_t ret;
+
+    mock_input_user_or_group("upninitgr at upndomain.test");
+    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_INITGR);
+    mock_fill_initgr_user();
+
+    /* Query for that user, call a callback when command finishes */
+    set_cmd_cb(test_nss_initgr_check);
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_INITGR,
+                          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);
+}
+
+/* Test that searching for a nonexistant user yields ENOENT.
+ * Account callback will be called
+ */
+void test_nss_initgr_neg_upn(void **state)
+{
+    test_initgr_neg_by_name("upninitgr_neg at upndomain.test", true);
+}
+
 static int nss_test_setup(void **state)
 {
     struct sss_test_conf_param params[] = {
@@ -2336,6 +2670,18 @@ int main(int argc, const char *argv[])
                                         nss_test_setup, nss_test_teardown),
         cmocka_unit_test_setup_teardown(test_nss_getpwnam_upn_neg,
                                         nss_test_setup, nss_test_teardown),
+        cmocka_unit_test_setup_teardown(test_nss_initgroups,
+                                        nss_test_setup, nss_test_teardown),
+        cmocka_unit_test_setup_teardown(test_nss_initgr_neg,
+                                        nss_test_setup, nss_test_teardown),
+        cmocka_unit_test_setup_teardown(test_nss_initgr_search,
+                                        nss_test_setup, nss_test_teardown),
+        cmocka_unit_test_setup_teardown(test_nss_initgr_update,
+                                        nss_test_setup, nss_test_teardown),
+        cmocka_unit_test_setup_teardown(test_nss_initgroups_upn,
+                                        nss_test_setup, nss_test_teardown),
+        cmocka_unit_test_setup_teardown(test_nss_initgr_neg_upn,
+                                        nss_test_setup, nss_test_teardown),
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */
-- 
2.1.0

-------------- next part --------------
>From 09b9f129cc87d9065702a8a49e894815ede6eb97 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 9 Mar 2015 17:25:48 +0100
Subject: [PATCH 4/4] NSS: Handle ENOENT when doing initgroups by UPN

https://fedorahosted.org/sssd/ticket/2598

We need to return an empty result in cases an initgroups lookup by UPN
doesn't return anything. Please note testing with "id user" is not
sufficient as id calls a getpwnam first.
---
 src/responder/nss/nsssrv_cmd.c | 50 +++++++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 20 deletions(-)

diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index f9056590a369b8cea358d12192357bf51599e7b6..aaf393f1c2c5ddd026412aaed1635cbe0e871680 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -4062,29 +4062,39 @@ static int nss_cmd_initgroups_search(struct nss_dom_ctx *dctx)
 
         if (cmdctx->name_is_upn) {
             ret = sysdb_search_user_by_upn(cmdctx, dom, name, user_attrs, &msg);
-            if (ret != EOK && ret != ENOENT) {
+            if (ret == EOK) {
+                sysdb_name = ldb_msg_find_attr_as_string(msg, SYSDB_NAME, NULL);
+                if (sysdb_name == NULL) {
+                    DEBUG(SSSDBG_OP_FAILURE,
+                        "Sysdb entry does not have a name.\n");
+                    return EINVAL;
+                }
+
+                ret = sysdb_initgroups(cmdctx, dom, sysdb_name, &dctx->res);
+                if (ret == EOK && DOM_HAS_VIEWS(dom)) {
+                    for (c = 0; c < dctx->res->count; c++) {
+                        ret = sysdb_add_overrides_to_object(dom, dctx->res->msgs[c],
+                                                            NULL, NULL);
+                        if (ret != EOK) {
+                            DEBUG(SSSDBG_OP_FAILURE,
+                                "sysdb_add_overrides_to_object failed.\n");
+                            return ret;
+                        }
+                    }
+                }
+            } else if (ret != ENOENT) {
                 DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_user_by_upn failed.\n");
                 return ret;
-            }
-
-            sysdb_name = ldb_msg_find_attr_as_string(msg, SYSDB_NAME, NULL);
-            if (sysdb_name == NULL) {
-                DEBUG(SSSDBG_OP_FAILURE,
-                      "Sysdb entry does not have a name.\n");
-                return EINVAL;
-            }
-
-            ret = sysdb_initgroups(cmdctx, dom, sysdb_name, &dctx->res);
-            if (ret == EOK && DOM_HAS_VIEWS(dom)) {
-                for (c = 0; c < dctx->res->count; c++) {
-                    ret = sysdb_add_overrides_to_object(dom, dctx->res->msgs[c],
-                                                        NULL, NULL);
-                    if (ret != EOK) {
-                        DEBUG(SSSDBG_OP_FAILURE,
-                              "sysdb_add_overrides_to_object failed.\n");
-                        return ret;
-                    }
+            } else {
+                dctx->res = talloc_zero(cmdctx, struct ldb_result);
+                if (dctx->res == NULL) {
+                    DEBUG(SSSDBG_OP_FAILURE, "talloc_zero failed.\n");
+                    return ENOMEM;
                 }
+
+                dctx->res->count = 0;
+                dctx->res->msgs = NULL;
+                ret = EOK;
             }
         } else {
             ret = sysdb_initgroups_with_views(cmdctx, dom, name, &dctx->res);
-- 
2.1.0



More information about the sssd-devel mailing list