>From a0646162d0879b5cc2be9adcee249403373e7cce Mon Sep 17 00:00:00 2001 From: Jakub Hrozek 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@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 --- 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