[SSSD] [PATCH] NSS: Print FQDN for groups with mixed domain membership

Jakub Hrozek jhrozek at redhat.com
Mon Oct 28 15:55:17 UTC 2013


Hi,

The first patch adds a unit test for getgrnam. I wanted to make sure we
don't break anything this close to the 1.11.2 release.

The second patch is a workaround until
https://fedorahosted.org/sssd/ticket/2129 is fixed properly.

Consider a group entry such as:
 cn: subgroup at subdom
 ghost: someuser
 ghost: anotheruser at subdom

Currently in order to print all group members as FQDN (which is the default
for AD provider), the code needs to iterate over the ghost attributes and
parse them into (name,domain) and optionally re-add the domain.

The proper fix would be to store always just the FQDN in the hardcoded
form of user at domain
-------------- next part --------------
>From a02c7bd3ad8921bb6a7b93fcdb25b7eab867411a Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 28 Oct 2013 13:07:32 +0100
Subject: [PATCH 1/2] TEST: Test getgrnam with emphasis on members

---
 src/tests/cmocka/test_nss_srv.c | 400 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 392 insertions(+), 8 deletions(-)

diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c
index e8cc9aaea3d9f3452b4f2c77c9ba47c1de2d22db..a270391db19e321a48db23e7c08e9e4c8691fd14 100644
--- a/src/tests/cmocka/test_nss_srv.c
+++ b/src/tests/cmocka/test_nss_srv.c
@@ -35,10 +35,12 @@
 #define TEST_CONF_DB "test_nss_conf.ldb"
 #define TEST_SYSDB_FILE "cache_nss_test.ldb"
 #define TEST_DOM_NAME "nss_test"
+#define TEST_SUBDOM_NAME "test.sub"
 #define TEST_ID_PROVIDER "ldap"
 
 struct nss_test_ctx {
     struct sss_test_ctx *tctx;
+    struct sss_domain_info *subdom;
 
     struct resp_ctx *rctx;
     struct cli_ctx *cctx;
@@ -68,6 +70,7 @@ mock_nctx(TALLOC_CTX *mem_ctx)
         return NULL;
     }
     nctx->neg_timeout = 10;
+    nctx->pwfield = discard_const("*");
 
     return nctx;
 }
@@ -144,7 +147,7 @@ int __wrap_sss_ncache_check_user(struct sss_nc_ctx *ctx, int ttl,
 }
 
 /* Mock input from the client library */
-static void mock_input_user(const char *username)
+static void mock_input_user_or_group(const char *username)
 {
     will_return(__wrap_sss_packet_get_body, WRAP_CALL_WRAPPER);
     will_return(__wrap_sss_packet_get_body, username);
@@ -157,6 +160,25 @@ static void mock_fill_user(void)
     will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
 }
 
+static void mock_fill_group_with_members(unsigned members)
+{
+    unsigned i;
+
+    will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
+    will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
+
+    if (members == 0) return;
+
+    will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
+
+    /* Member header , one per member */
+    will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
+    for (i=0; i<members; i++) {
+        will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
+    }
+}
+
+
 static int parse_user_packet(uint8_t *body, size_t blen, struct passwd *pwd)
 {
     size_t rp = 2 * sizeof(uint32_t);
@@ -169,6 +191,10 @@ static int parse_user_packet(uint8_t *body, size_t blen, struct passwd *pwd)
     rp += strlen(pwd->pw_name) + 1;
     if (rp >= blen) return EINVAL;
 
+    pwd->pw_passwd = (char *) body+rp;
+    rp += strlen(pwd->pw_passwd) + 1;
+    if (rp >= blen) return EINVAL;
+
     pwd->pw_gecos = (char *) body+rp;
     rp += strlen(pwd->pw_gecos) + 1;
     if (rp >= blen) return EINVAL;
@@ -178,6 +204,41 @@ static int parse_user_packet(uint8_t *body, size_t blen, struct passwd *pwd)
     if (rp >= blen) return EINVAL;
 
     pwd->pw_shell = (char *) body+rp;
+    rp += strlen(pwd->pw_shell) + 1;
+    if (rp != blen) return EINVAL;
+
+    return EOK;
+}
+
+static int parse_group_packet(uint8_t *body, size_t blen, struct group *gr, uint32_t *nmem)
+{
+    size_t rp = 2 * sizeof(uint32_t); /* Len and reserved */
+    unsigned i;
+
+    SAFEALIGN_COPY_UINT32(&gr->gr_gid, body+rp, &rp);
+    SAFEALIGN_COPY_UINT32(nmem, body+rp, &rp);
+
+    gr->gr_name = (char *) body+rp;
+    rp += strlen(gr->gr_name) + 1;
+    if (rp >= blen) return EINVAL;
+
+    gr->gr_passwd = (char *) body+rp;
+    rp += strlen(gr->gr_passwd) + 1;
+
+    if (*nmem > 0) {
+        gr->gr_mem = talloc_zero_array(nss_test_ctx, char *, *nmem);
+        if (gr->gr_mem == NULL) return ENOMEM;
+
+        for (i = 0; i < *nmem; i++) {
+            if (rp >= blen) return EINVAL;
+
+            gr->gr_mem[i] = talloc_strdup(gr->gr_mem, (char *) body+rp);
+            rp += strlen(gr->gr_mem[i]) + 1;
+        }
+    }
+
+    /* Make sure we exactly matched the end of the packet */
+    if (rp != blen) return EINVAL;
     return EOK;
 }
 
@@ -199,6 +260,7 @@ static int test_nss_getpwnam_check(uint8_t *body, size_t blen)
     assert_int_equal(pwd.pw_gid, 456);
     assert_string_equal(pwd.pw_name, "testuser");
     assert_string_equal(pwd.pw_shell, "/bin/sh");
+    assert_string_equal(pwd.pw_passwd, "*");
     return EOK;
 }
 
@@ -214,7 +276,7 @@ void test_nss_getpwnam(void **state)
                          NULL, 300, 0);
     assert_int_equal(ret, EOK);
 
-    mock_input_user("testuser");
+    mock_input_user_or_group("testuser");
     will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETPWNAM);
     mock_fill_user();
 
@@ -236,7 +298,7 @@ void test_nss_getpwnam_neg(void **state)
 {
     errno_t ret;
 
-    mock_input_user("testuser_neg");
+    mock_input_user_or_group("testuser_neg");
     mock_account_recv_simple();
 
     assert_true(nss_test_ctx->ncache_hit == false);
@@ -256,7 +318,7 @@ void test_nss_getpwnam_neg(void **state)
      */
     nss_test_ctx->tctx->done = false;
 
-    mock_input_user("testuser_neg");
+    mock_input_user_or_group("testuser_neg");
     ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWNAM,
                           nss_test_ctx->nss_cmds);
     assert_int_equal(ret, EOK);
@@ -303,7 +365,7 @@ void test_nss_getpwnam_search(void **state)
     errno_t ret;
     struct ldb_result *res;
 
-    mock_input_user("testuser_search");
+    mock_input_user_or_group("testuser_search");
     mock_account_recv(0, 0, NULL, test_nss_getpwnam_search_acct_cb, nss_test_ctx);
     will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETPWNAM);
     mock_fill_user();
@@ -381,7 +443,7 @@ void test_nss_getpwnam_update(void **state)
     assert_int_equal(ret, EOK);
 
     /* Mock client input */
-    mock_input_user("testuser_update");
+    mock_input_user_or_group("testuser_update");
     /* Mock client command */
     will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETPWNAM);
     /* Call this function when user is updated by the mock DP request */
@@ -443,7 +505,7 @@ void test_nss_getpwnam_fqdn(void **state)
                          NULL, 300, 0);
     assert_int_equal(ret, EOK);
 
-    mock_input_user("testuser_fqdn@"TEST_DOM_NAME);
+    mock_input_user_or_group("testuser_fqdn@"TEST_DOM_NAME);
     will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETPWNAM);
     mock_fill_user();
 
@@ -492,7 +554,7 @@ void test_nss_getpwnam_fqdn_resize(void **state)
                          NULL, 300, 0);
     assert_int_equal(ret, EOK);
 
-    mock_input_user("testuser_fqdn_resize@"TEST_DOM_NAME);
+    mock_input_user_or_group("testuser_fqdn_resize@"TEST_DOM_NAME);
     will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETPWNAM);
     mock_fill_user();
     will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
@@ -546,6 +608,296 @@ void test_nss_setup(struct sss_test_conf_param params[],
     assert_non_null(nss_test_ctx->cctx);
 }
 
+static int test_nss_getgrnam_check(struct group *expected, struct group *gr, const int nmem)
+{
+    int i;
+
+    assert_int_equal(gr->gr_gid, expected->gr_gid);
+    assert_string_equal(gr->gr_name, expected->gr_name);
+    assert_string_equal(gr->gr_passwd, expected->gr_passwd);
+
+    if (expected->gr_mem == NULL || gr->gr_mem == NULL) {
+        /* If one group is empty, the other has to be empty, too */
+        assert_true(expected->gr_mem == gr->gr_mem);
+        return EOK;
+    }
+
+    for (i = 0; i < nmem; i++) {
+        assert_string_equal(gr->gr_mem[i], expected->gr_mem[i]);
+    }
+    return EOK;
+}
+
+static int test_nss_getgrnam_no_members_check(uint8_t *body, size_t blen)
+{
+    int ret;
+    uint32_t nmem;
+    struct group gr;
+    struct group expected = {
+        .gr_gid = 1123,
+        .gr_name = discard_const("testgroup"),
+        .gr_passwd = discard_const("*"),
+        .gr_mem = NULL,
+    };
+
+    ret = parse_group_packet(body, blen, &gr, &nmem);
+    assert_int_equal(ret, EOK);
+    assert_int_equal(nmem, 0);
+
+    ret = test_nss_getgrnam_check(&expected, &gr, nmem);
+    assert_int_equal(ret, EOK);
+
+    return EOK;
+}
+
+/* Test that requesting a valid, cached group with no members returns a valid
+ * group structure
+ */
+void test_nss_getgrnam_no_members(void **state)
+{
+    errno_t ret;
+
+    /* Prime the cache with a valid group */
+    ret = sysdb_add_group(nss_test_ctx->tctx->sysdb,
+                          nss_test_ctx->tctx->dom,
+                          "testgroup", 1123,
+                          NULL, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    mock_input_user_or_group("testgroup");
+    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETGRNAM);
+    mock_fill_group_with_members(0);
+
+    /* Query for that group, call a callback when command finishes */
+    set_cmd_cb(test_nss_getgrnam_no_members_check);
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETGRNAM,
+                          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);
+}
+
+static int test_nss_getgrnam_members_check(uint8_t *body, size_t blen)
+{
+    int ret;
+    uint32_t nmem;
+    struct group gr;
+    const char *exp_members[] = { "testmember1", "testmember2" };
+    struct group expected = {
+        .gr_gid = 1124,
+        .gr_name = discard_const("testgroup_members"),
+        .gr_passwd = discard_const("*"),
+        .gr_mem = discard_const(exp_members)
+    };
+
+    ret = parse_group_packet(body, blen, &gr, &nmem);
+    assert_int_equal(ret, EOK);
+    assert_int_equal(nmem, 2);
+
+    ret = test_nss_getgrnam_check(&expected, &gr, nmem);
+    assert_int_equal(ret, EOK);
+
+    return EOK;
+}
+
+/* Test that requesting a valid, cached group with some members returns a valid
+ * group structure with those members present
+ */
+void test_nss_getgrnam_members(void **state)
+{
+    errno_t ret;
+
+    /* Prime the cache with a valid group and some members */
+    ret = sysdb_add_group(nss_test_ctx->tctx->sysdb,
+                          nss_test_ctx->tctx->dom,
+                          "testgroup_members", 1124,
+                          NULL, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_user(nss_test_ctx->tctx->sysdb,
+                         nss_test_ctx->tctx->dom,
+                         "testmember1", 2001, 456, "test member1",
+                         "/home/testmember2", "/bin/sh", NULL,
+                         NULL, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_user(nss_test_ctx->tctx->sysdb,
+                         nss_test_ctx->tctx->dom,
+                         "testmember2", 2002, 456, "test member2",
+                         "/home/testmember2", "/bin/sh", NULL,
+                         NULL, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group_member(nss_test_ctx->tctx->sysdb,
+                                 nss_test_ctx->tctx->dom,
+                                 "testgroup_members", "testmember1",
+                                 SYSDB_MEMBER_USER, false);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group_member(nss_test_ctx->tctx->sysdb,
+                                 nss_test_ctx->tctx->dom,
+                                 "testgroup_members", "testmember2",
+                                 SYSDB_MEMBER_USER, false);
+    assert_int_equal(ret, EOK);
+
+    mock_input_user_or_group("testgroup_members");
+    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETGRNAM);
+    mock_fill_group_with_members(2);
+
+    /* Query for that group, call a callback when command finishes */
+    set_cmd_cb(test_nss_getgrnam_members_check);
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETGRNAM,
+                          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);
+}
+
+static int test_nss_getgrnam_members_check_fqdn(uint8_t *body, size_t blen)
+{
+    int ret;
+    uint32_t nmem;
+    struct group gr;
+    const char *exp_members[] = { "testmember1@"TEST_DOM_NAME,
+                                  "testmember2@"TEST_DOM_NAME };
+    struct group expected = {
+        .gr_gid = 1124,
+        .gr_name = discard_const("testgroup_members@"TEST_DOM_NAME),
+        .gr_passwd = discard_const("*"),
+        .gr_mem = discard_const(exp_members)
+    };
+
+    ret = parse_group_packet(body, blen, &gr, &nmem);
+    assert_int_equal(ret, EOK);
+    assert_int_equal(nmem, 2);
+
+    ret = test_nss_getgrnam_check(&expected, &gr, nmem);
+    assert_int_equal(ret, EOK);
+
+    return EOK;
+}
+
+/* Test that requesting a valid, cached group with some members returns a valid
+ * group structure with those members present as fully qualified names
+ */
+void test_nss_getgrnam_members_fqdn(void **state)
+{
+    errno_t ret;
+
+    nss_test_ctx->tctx->dom->fqnames = true;
+
+    mock_input_user_or_group("testgroup_members@"TEST_DOM_NAME);
+    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETGRNAM);
+    mock_fill_group_with_members(2);
+
+    /* Query for that group, call a callback when command finishes */
+    set_cmd_cb(test_nss_getgrnam_members_check_fqdn);
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETGRNAM,
+                          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);
+
+    /* Restore FQDN settings */
+    nss_test_ctx->tctx->dom->fqnames = false;
+    assert_int_equal(ret, EOK);
+}
+
+static int test_nss_getgrnam_members_check_subdom(uint8_t *body, size_t blen)
+{
+    int ret;
+    uint32_t nmem;
+    struct group gr;
+    const char *exp_members[] = { "submember1@"TEST_SUBDOM_NAME,
+                                  "submember2@"TEST_SUBDOM_NAME };
+    struct group expected = {
+        .gr_gid = 2124,
+        .gr_name = discard_const("testsubdomgroup@"TEST_SUBDOM_NAME),
+        .gr_passwd = discard_const("*"),
+        .gr_mem = discard_const(exp_members)
+    };
+
+    ret = parse_group_packet(body, blen, &gr, &nmem);
+    assert_int_equal(ret, EOK);
+    assert_int_equal(nmem, 2);
+
+    ret = test_nss_getgrnam_check(&expected, &gr, nmem);
+    assert_int_equal(ret, EOK);
+
+    return EOK;
+}
+
+/* Test that requesting a valid, cached group with some members returns a valid
+ * group structure with those members present as fully qualified names
+ */
+void test_nss_getgrnam_members_subdom(void **state)
+{
+    errno_t ret;
+
+    nss_test_ctx->tctx->dom->fqnames = true;
+
+    /* Add a group from a subdomain and two members from the same subdomain
+     */
+    ret = sysdb_add_group(nss_test_ctx->tctx->sysdb,
+                          nss_test_ctx->subdom,
+                          "testsubdomgroup@"TEST_SUBDOM_NAME,
+                          2124, NULL, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_user(nss_test_ctx->tctx->sysdb,
+                         nss_test_ctx->subdom,
+                         "submember1@"TEST_SUBDOM_NAME,
+                         4001, 456, "test subdomain member1",
+                         "/home/submember1", "/bin/sh", NULL,
+                         NULL, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_user(nss_test_ctx->tctx->sysdb,
+                         nss_test_ctx->subdom,
+                         "submember2@"TEST_SUBDOM_NAME,
+                         2002, 456, "test subdomain member2",
+                         "/home/submember2", "/bin/sh", NULL,
+                         NULL, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group_member(nss_test_ctx->tctx->sysdb,
+                                 nss_test_ctx->subdom,
+                                 "testsubdomgroup@"TEST_SUBDOM_NAME,
+                                 "submember1@"TEST_SUBDOM_NAME,
+                                 SYSDB_MEMBER_USER, false);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group_member(nss_test_ctx->tctx->sysdb,
+                                 nss_test_ctx->subdom,
+                                 "testsubdomgroup@"TEST_SUBDOM_NAME,
+                                 "submember2@"TEST_SUBDOM_NAME,
+                                 SYSDB_MEMBER_USER, false);
+    assert_int_equal(ret, EOK);
+
+
+    mock_input_user_or_group("testsubdomgroup@"TEST_SUBDOM_NAME);
+    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETGRNAM);
+    mock_fill_group_with_members(2);
+
+    /* Query for that group, call a callback when command finishes */
+    set_cmd_cb(test_nss_getgrnam_members_check_subdom);
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETGRNAM,
+                          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);
+
+    /* Restore FQDN settings */
+    nss_test_ctx->tctx->dom->fqnames = false;
+    assert_int_equal(ret, EOK);
+}
+
 void nss_test_setup(void **state)
 {
     struct sss_test_conf_param params[] = {
@@ -567,6 +919,30 @@ void nss_fqdn_test_setup(void **state)
     test_nss_setup(params, state);
 }
 
+void nss_subdom_test_setup(void **state)
+{
+    const char *const testdom[4] = { TEST_SUBDOM_NAME, "TEST.SUB", "test", "S-3" };
+    struct sss_domain_info *subdomain;
+    errno_t ret;
+
+    nss_test_setup(state);
+
+    subdomain = new_subdomain(nss_test_ctx, nss_test_ctx->tctx->dom,
+                              testdom[0], testdom[1], testdom[2], testdom[3],
+                              false, false, NULL);
+    assert_non_null(subdomain);
+
+    ret = sysdb_subdomain_store(nss_test_ctx->tctx->sysdb,
+                                testdom[0], testdom[1], testdom[2], testdom[3],
+                                false, false, NULL);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_update_subdomains(nss_test_ctx->tctx->dom);
+    assert_int_equal(ret, EOK);
+
+    nss_test_ctx->subdom = subdomain;
+}
+
 void nss_fqdn_resize_test_setup(void **state)
 {
     struct sss_test_conf_param params[] = {
@@ -610,6 +986,14 @@ int main(int argc, const char *argv[])
                                  nss_fqdn_test_setup, nss_test_teardown),
         unit_test_setup_teardown(test_nss_getpwnam_fqdn_resize,
                                  nss_fqdn_resize_test_setup, nss_test_teardown),
+        unit_test_setup_teardown(test_nss_getgrnam_no_members,
+                                 nss_test_setup, nss_test_teardown),
+        unit_test_setup_teardown(test_nss_getgrnam_members,
+                                 nss_test_setup, nss_test_teardown),
+        unit_test_setup_teardown(test_nss_getgrnam_members_fqdn,
+                                 nss_fqdn_test_setup, nss_test_teardown),
+        unit_test_setup_teardown(test_nss_getgrnam_members_subdom,
+                                 nss_subdom_test_setup, nss_test_teardown),
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */
-- 
1.8.3.1

-------------- next part --------------
>From a0646162d0879b5cc2be9adcee249403373e7cce Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 28 Oct 2013 16:13:17 +0100
Subject: [PATCH 2/2] NSS: Print FQDN for groups with mixed domain membership

This patch is a workaround until
https://fedorahosted.org/sssd/ticket/2129 is fixed properly.

Consider a group entry such as:
 cn: subgroup at subdom
 ghost: someuser
 ghost: anotheruser at subdom

Currently in order to print all group members as FQDN (which is the default
for AD provider), the code needs to iterate over the ghost attributes and
parse them into (name,domain) and optionally re-add the domain.

The proper fix would be to store always just the FQDN in the hardcoded
form of user at domain
---
 src/responder/nss/nsssrv_cmd.c  |  72 ++++++++++++++---
 src/tests/cmocka/test_nss_srv.c | 175 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 238 insertions(+), 9 deletions(-)

diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index b8db8dd1812a514c995aa553f810e843daa255bc..99f0f810ce028fddc48b7e2e1100848cd9c4a090 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -2160,6 +2160,51 @@ void nss_update_gr_memcache(struct nss_ctx *nctx)
 #define MNUM_ROFFSET sizeof(uint32_t)
 #define STRS_ROFFSET 2*sizeof(uint32_t)
 
+static int parse_member(TALLOC_CTX *mem_ctx, struct sss_domain_info *group_dom,
+                        const char *member, struct sss_domain_info **_member_dom,
+                        struct sized_string *_name, bool *_add_domain)
+{
+    errno_t ret;
+    char *username;
+    char *domname;
+    const char *use_member;
+    struct sss_domain_info *member_dom;
+    bool add_domain;
+
+    ret = sss_parse_name(mem_ctx, group_dom->names, member, &domname, &username);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_MINOR_FAILURE, ("Could not parse [%s] into "
+              "name-value components.\n", member));
+        return ret;
+    }
+
+    add_domain = (!IS_SUBDOMAIN(group_dom) && group_dom->fqnames);
+    use_member = member;
+    member_dom = group_dom;
+
+    if (IS_SUBDOMAIN(group_dom) == false && domname != NULL) {
+        /* The group is stored in the parent domain, but the member comes from.
+         * a subdomain. No need to add the domain component, it's already
+         * present in the memberuid/ghost attribute
+         */
+        add_domain = false;
+    }
+
+    if (IS_SUBDOMAIN(group_dom) == true && domname == NULL) {
+        /* The group is stored in a subdomain, but the member comes
+         * from the parent domain. Need to add the domain component
+         */
+        add_domain = true;
+        use_member = username;
+        member_dom = group_dom->parent;
+    }
+
+    to_sized_string(_name, use_member);
+    *_add_domain = add_domain;
+    *_member_dom = member_dom;
+    return EOK;
+}
+
 static int fill_members(struct sss_packet *packet,
                         struct sss_domain_info *dom,
                         struct nss_ctx *nctx,
@@ -2183,12 +2228,8 @@ static int fill_members(struct sss_packet *packet,
     size_t blen;
 
     const char *domain = dom->name;
-    bool add_domain = (!IS_SUBDOMAIN(dom) && dom->fqnames);
-
-    if (add_domain) {
-        delim = 1;
-        dom_len = sss_fqdom_len(dom->names, dom);
-    }
+    bool add_domain;
+    struct sss_domain_info *member_dom;
 
     tmp_ctx = talloc_new(NULL);
     if (tmp_ctx == NULL) {
@@ -2218,7 +2259,20 @@ static int fill_members(struct sss_packet *packet,
             }
         }
 
-        to_sized_string(&name, tmpstr);
+        delim = 0;
+        dom_len = 0;
+
+        ret = parse_member(tmp_ctx, dom, tmpstr, &member_dom, &name, &add_domain);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_MINOR_FAILURE,
+                  ("Could not process member %s, skipping\n", tmpstr));
+            continue;
+        }
+
+        if (add_domain) {
+            delim = 1;
+            dom_len = sss_fqdom_len(member_dom->names, member_dom);
+        }
 
         ret = sss_packet_grow(packet, name.len + delim + dom_len);
         if (ret != EOK) {
@@ -2229,7 +2283,7 @@ static int fill_members(struct sss_packet *packet,
         if (add_domain) {
             ret = sss_fqname((char *)&body[rzero + rsize],
                              name.len + delim + dom_len,
-                             dom->names, dom, name.str);
+                             member_dom->names, member_dom, name.str);
             if (ret >= (name.len + delim + dom_len)) {
                 /* need more space,
                  * got creative with the print format ? */
@@ -2244,7 +2298,7 @@ static int fill_members(struct sss_packet *packet,
                 /* retry */
                 ret = sss_fqname((char *)&body[rzero + rsize],
                                 name.len + delim + dom_len,
-                                dom->names, dom, name.str);
+                                member_dom->names, member_dom, name.str);
             }
 
             if (ret != name.len + delim + dom_len - 1) {
diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c
index a270391db19e321a48db23e7c08e9e4c8691fd14..46227289480825eb76753669efa6797a1294bc73 100644
--- a/src/tests/cmocka/test_nss_srv.c
+++ b/src/tests/cmocka/test_nss_srv.c
@@ -898,6 +898,175 @@ void test_nss_getgrnam_members_subdom(void **state)
     assert_int_equal(ret, EOK);
 }
 
+static int test_nss_getgrnam_check_mix_dom(uint8_t *body, size_t blen)
+{
+    int ret;
+    uint32_t nmem;
+    struct group gr;
+    const char *exp_members[] = { "testmember1",
+                                  "testmember2",
+                                  "submember1@"TEST_SUBDOM_NAME };
+    struct group expected = {
+        .gr_gid = 1124,
+        .gr_name = discard_const("testgroup_members"),
+        .gr_passwd = discard_const("*"),
+        .gr_mem = discard_const(exp_members)
+    };
+
+    ret = parse_group_packet(body, blen, &gr, &nmem);
+    assert_int_equal(ret, EOK);
+    assert_int_equal(nmem, 3);
+
+    ret = test_nss_getgrnam_check(&expected, &gr, nmem);
+    assert_int_equal(ret, EOK);
+
+    return EOK;
+}
+
+void test_nss_getgrnam_mix_dom(void **state)
+{
+    errno_t ret;
+    const char *group_strdn = NULL;
+    const char *add_groups[] = { NULL, NULL };
+
+    /* Add a subdomain user to a parent domain group */
+    group_strdn = sysdb_group_strdn(nss_test_ctx,
+                                    nss_test_ctx->tctx->dom->name,
+                                    "testgroup_members");
+    assert_non_null(group_strdn);
+    add_groups[0] = group_strdn;
+
+    ret = sysdb_update_members_dn(nss_test_ctx->tctx->sysdb,
+                                  nss_test_ctx->subdom,
+                                  "submember1@"TEST_SUBDOM_NAME,
+                                  SYSDB_MEMBER_USER,
+                                  add_groups, NULL);
+    assert_int_equal(ret, EOK);
+
+    mock_input_user_or_group("testgroup_members");
+    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETGRNAM);
+    mock_fill_group_with_members(3);
+
+    /* Query for that group, call a callback when command finishes */
+    set_cmd_cb(test_nss_getgrnam_check_mix_dom);
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETGRNAM,
+                          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);
+}
+
+static int test_nss_getgrnam_check_mix_dom_fqdn(uint8_t *body, size_t blen)
+{
+    int ret;
+    uint32_t nmem;
+    struct group gr;
+    const char *exp_members[] = { "testmember1@"TEST_DOM_NAME,
+                                  "testmember2@"TEST_DOM_NAME,
+                                  "submember1@"TEST_SUBDOM_NAME };
+    struct group expected = {
+        .gr_gid = 1124,
+        .gr_name = discard_const("testgroup_members@"TEST_DOM_NAME),
+        .gr_passwd = discard_const("*"),
+        .gr_mem = discard_const(exp_members)
+    };
+
+    ret = parse_group_packet(body, blen, &gr, &nmem);
+    assert_int_equal(ret, EOK);
+    assert_int_equal(nmem, 3);
+
+    ret = test_nss_getgrnam_check(&expected, &gr, nmem);
+    assert_int_equal(ret, EOK);
+
+    return EOK;
+}
+
+void test_nss_getgrnam_mix_dom_fqdn(void **state)
+{
+    errno_t ret;
+
+    nss_test_ctx->tctx->dom->fqnames = true;
+
+    mock_input_user_or_group("testgroup_members@"TEST_DOM_NAME);
+    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETGRNAM);
+    mock_fill_group_with_members(3);
+
+    /* Query for that group, call a callback when command finishes */
+    set_cmd_cb(test_nss_getgrnam_check_mix_dom_fqdn);
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETGRNAM,
+                          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);
+
+    /* Restore FQDN settings */
+    nss_test_ctx->tctx->dom->fqnames = false;
+    assert_int_equal(ret, EOK);
+}
+
+static int test_nss_getgrnam_check_mix_subdom(uint8_t *body, size_t blen)
+{
+    int ret;
+    uint32_t nmem;
+    struct group gr;
+    const char *exp_members[] = { "submember1@"TEST_SUBDOM_NAME,
+                                  "submember2@"TEST_SUBDOM_NAME,
+                                  "testmember1@"TEST_DOM_NAME };
+    struct group expected = {
+        .gr_gid = 2124,
+        .gr_name = discard_const("testsubdomgroup@"TEST_SUBDOM_NAME),
+        .gr_passwd = discard_const("*"),
+        .gr_mem = discard_const(exp_members)
+    };
+
+    ret = parse_group_packet(body, blen, &gr, &nmem);
+    assert_int_equal(ret, EOK);
+    assert_int_equal(nmem, 3);
+
+    ret = test_nss_getgrnam_check(&expected, &gr, nmem);
+    assert_int_equal(ret, EOK);
+
+    return EOK;
+}
+
+void test_nss_getgrnam_mix_subdom(void **state)
+{
+    errno_t ret;
+    const char *group_strdn = NULL;
+    const char *add_groups[] = { NULL, NULL };
+
+    /* Add a subdomain user to a parent domain group */
+    group_strdn = sysdb_group_strdn(nss_test_ctx,
+                                    nss_test_ctx->subdom->name,
+                                    "testsubdomgroup@"TEST_SUBDOM_NAME);
+    assert_non_null(group_strdn);
+    add_groups[0] = group_strdn;
+
+    ret = sysdb_update_members_dn(nss_test_ctx->tctx->sysdb,
+                                  nss_test_ctx->tctx->dom,
+                                  "testmember1",
+                                  SYSDB_MEMBER_USER,
+                                  add_groups, NULL);
+    assert_int_equal(ret, EOK);
+
+    mock_input_user_or_group("testsubdomgroup@"TEST_SUBDOM_NAME);
+    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETGRNAM);
+    mock_fill_group_with_members(3);
+
+    /* Query for that group, call a callback when command finishes */
+    set_cmd_cb(test_nss_getgrnam_check_mix_subdom);
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETGRNAM,
+                          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 nss_test_setup(void **state)
 {
     struct sss_test_conf_param params[] = {
@@ -994,6 +1163,12 @@ int main(int argc, const char *argv[])
                                  nss_fqdn_test_setup, nss_test_teardown),
         unit_test_setup_teardown(test_nss_getgrnam_members_subdom,
                                  nss_subdom_test_setup, nss_test_teardown),
+        unit_test_setup_teardown(test_nss_getgrnam_mix_dom,
+                                 nss_subdom_test_setup, nss_test_teardown),
+        unit_test_setup_teardown(test_nss_getgrnam_mix_dom_fqdn,
+                                 nss_subdom_test_setup, nss_test_teardown),
+        unit_test_setup_teardown(test_nss_getgrnam_mix_subdom,
+                                 nss_subdom_test_setup, nss_test_teardown),
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */
-- 
1.8.3.1



More information about the sssd-devel mailing list