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@subdom
 ghost: someuser
 ghost: anotheruser@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@domain

2 files changed, 239 insertions, 9 deletions

diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index b785d916b..aa5dee903 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -2159,6 +2159,52 @@ 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
+         * of the parent domain
+         */
+        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,
@@ -2182,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) {
@@ -2217,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) {
@@ -2228,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 ? */
@@ -2243,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 a410d049b..e2e81a65f 100644
--- a/src/tests/cmocka/test_nss_srv.c
+++ b/src/tests/cmocka/test_nss_srv.c
@@ -892,6 +892,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[] = {
@@ -988,6 +1157,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. */