From 0713b92ec9f10b6dd913dc56cbc7845d1b025ccb Mon Sep 17 00:00:00 2001 From: Pavel Březina Date: Dec 19 2016 22:24:41 +0000 Subject: responders: unify usage of sss_cmd_send_empty and _error Originally sss_cmd_send_empty() called also sss_cmd_done() to send an empty reply packet to the cliant where as sss_cmd_send_error() did not invoke this call and required the caller to call it manually. For this reason, a possible error in users_find_by_cert_done() was not send to the caller. This patch unifies the usage of those two functions in a way that both of them only creates the reply packet but do not send it. Another sss_cmd_done() call is required to send the reply. Because sss_cmd_done() is now always called, unit tests needed to be changed to always mock a value for __wrap_sss_cmd_done. Resolves: https://fedorahosted.org/sssd/ticket/3151 Reviewed-by: Lukáš Slebodník --- diff --git a/src/responder/autofs/autofssrv_cmd.c b/src/responder/autofs/autofssrv_cmd.c index 68bef15..0878707 100644 --- a/src/responder/autofs/autofssrv_cmd.c +++ b/src/responder/autofs/autofssrv_cmd.c @@ -38,7 +38,7 @@ static int autofs_cmd_send_error(struct autofs_cmd_ctx *cmdctx, int err) static int autofs_cmd_send_empty(struct autofs_cmd_ctx *cmdctx) { - return sss_cmd_send_empty(cmdctx->cctx, cmdctx); + return sss_cmd_send_empty(cmdctx->cctx); } static int @@ -54,6 +54,7 @@ autofs_cmd_done(struct autofs_cmd_ctx *cmdctx, int ret) if (ret) { return EFAULT; } + sss_cmd_done(cmdctx->cctx, cmdctx); break; case EAGAIN: diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h index 9b69842..4d4d7f9 100644 --- a/src/responder/common/responder.h +++ b/src/responder/common/responder.h @@ -196,7 +196,7 @@ int activate_unix_sockets(struct resp_ctx *rctx, /* responder_cmd.c */ int sss_cmd_empty_packet(struct sss_packet *packet); -int sss_cmd_send_empty(struct cli_ctx *cctx, TALLOC_CTX *freectx); +int sss_cmd_send_empty(struct cli_ctx *cctx); int sss_cmd_send_error(struct cli_ctx *cctx, int err); void sss_cmd_done(struct cli_ctx *cctx, void *freectx); int sss_cmd_get_version(struct cli_ctx *cctx); diff --git a/src/responder/common/responder_cmd.c b/src/responder/common/responder_cmd.c index 104e377..bd05ca2 100644 --- a/src/responder/common/responder_cmd.c +++ b/src/responder/common/responder_cmd.c @@ -66,7 +66,7 @@ int sss_cmd_empty_packet(struct sss_packet *packet) return EOK; } -int sss_cmd_send_empty(struct cli_ctx *cctx, TALLOC_CTX *freectx) +int sss_cmd_send_empty(struct cli_ctx *cctx) { struct cli_protocol *pctx; int ret; @@ -88,7 +88,6 @@ int sss_cmd_send_empty(struct cli_ctx *cctx, TALLOC_CTX *freectx) } sss_packet_set_error(pctx->creq->out, EOK); - sss_cmd_done(cctx, freectx); return EOK; } diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c index 29c5cbf..0352ff3 100644 --- a/src/responder/nss/nsssrv_cmd.c +++ b/src/responder/nss/nsssrv_cmd.c @@ -45,7 +45,7 @@ static int nss_cmd_send_error(struct nss_cmd_ctx *cmdctx, int err) static int nss_cmd_send_empty(struct nss_cmd_ctx *cmdctx) { struct cli_ctx *cctx = cmdctx->cctx; - return sss_cmd_send_empty(cctx, cmdctx); + return sss_cmd_send_empty(cctx); } int nss_cmd_done(struct nss_cmd_ctx *cmdctx, int ret) @@ -60,6 +60,7 @@ int nss_cmd_done(struct nss_cmd_ctx *cmdctx, int ret) if (ret) { return EFAULT; } + sss_cmd_done(cmdctx->cctx, cmdctx); break; case EAGAIN: @@ -5453,13 +5454,14 @@ static void users_find_by_cert_done(struct tevent_req *req) done: if (ret == EOK) { sss_packet_set_error(pctx->creq->out, EOK); - sss_cmd_done(cctx, NULL); } else if (ret == ENOENT) { - sss_cmd_send_empty(cctx, NULL); + sss_cmd_send_empty(cctx); } else { sss_cmd_send_error(cctx, ret); } + sss_cmd_done(cctx, NULL); + return; } diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c index bc63c95..1e64a0b 100644 --- a/src/tests/cmocka/test_nss_srv.c +++ b/src/tests/cmocka/test_nss_srv.c @@ -127,13 +127,18 @@ void __wrap_sss_cmd_done(struct cli_ctx *cctx, void *freectx) check_cb = sss_mock_ptr_type(cmd_cb_fn_t); - pctx = talloc_get_type(cctx->protocol_ctx, struct cli_protocol); - packet = pctx->creq->out; + if (check_cb == NULL) { + nss_test_ctx->tctx->error = ENOENT; + } else { + pctx = talloc_get_type(cctx->protocol_ctx, struct cli_protocol); + packet = pctx->creq->out; + + __real_sss_packet_get_body(packet, &body, &blen); - __real_sss_packet_get_body(packet, &body, &blen); + nss_test_ctx->tctx->error = check_cb(sss_packet_get_status(packet), + body, blen); + } - nss_test_ctx->tctx->error = check_cb(sss_packet_get_status(packet), - body, blen); nss_test_ctx->tctx->done = true; talloc_free(freectx); } @@ -566,6 +571,7 @@ void test_nss_getpwnam_neg(void **state) assert_int_equal(nss_test_ctx->ncache_hits, 0); + set_cmd_cb(NULL); ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWNAM, nss_test_ctx->nss_cmds); assert_int_equal(ret, EOK); @@ -582,6 +588,7 @@ void test_nss_getpwnam_neg(void **state) nss_test_ctx->tctx->done = false; mock_input_user_or_group("testuser_neg"); + set_cmd_cb(NULL); ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWNAM, nss_test_ctx->nss_cmds); assert_int_equal(ret, EOK); @@ -1031,6 +1038,7 @@ void test_nss_getpwuid_neg(void **state) assert_int_equal(nss_test_ctx->ncache_hits, 0); + set_cmd_cb(NULL); ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWUID, nss_test_ctx->nss_cmds); assert_int_equal(ret, EOK); @@ -1047,6 +1055,7 @@ void test_nss_getpwuid_neg(void **state) nss_test_ctx->tctx->done = false; mock_input_id(nss_test_ctx, uid_neg); + set_cmd_cb(NULL); ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWUID, nss_test_ctx->nss_cmds); assert_int_equal(ret, EOK); @@ -2536,6 +2545,7 @@ void test_nss_getpwnam_upn_neg(void **state) assert_int_equal(nss_test_ctx->ncache_hits, 0); + set_cmd_cb(NULL); ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWNAM, nss_test_ctx->nss_cmds); assert_int_equal(ret, EOK); @@ -2553,6 +2563,7 @@ void test_nss_getpwnam_upn_neg(void **state) nss_test_ctx->ncache_hits = 0; mock_input_user_or_group("nosuchupnuser@upndomain.test"); + set_cmd_cb(NULL); ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWNAM, nss_test_ctx->nss_cmds); assert_int_equal(ret, EOK); @@ -2667,6 +2678,7 @@ void test_initgr_neg_by_name(const char *name, bool is_upn) assert_int_equal(nss_test_ctx->ncache_hits, 0); + set_cmd_cb(NULL); ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_INITGR, nss_test_ctx->nss_cmds); assert_int_equal(ret, EOK); @@ -2685,6 +2697,7 @@ void test_initgr_neg_by_name(const char *name, bool is_upn) nss_test_ctx->ncache_hits = 0; mock_input_user_or_group(name); + set_cmd_cb(NULL); ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_INITGR, nss_test_ctx->nss_cmds); assert_int_equal(ret, EOK); @@ -3227,6 +3240,7 @@ void test_nss_getnamebysid_neg(void **state) assert_int_equal(nss_test_ctx->ncache_hits, 0); + set_cmd_cb(NULL); ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETNAMEBYSID, nss_test_ctx->nss_cmds); assert_int_equal(ret, EOK); @@ -3243,6 +3257,7 @@ void test_nss_getnamebysid_neg(void **state) nss_test_ctx->tctx->done = false; mock_input_user_or_group(user_sid); + set_cmd_cb(NULL); ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETNAMEBYSID, nss_test_ctx->nss_cmds); assert_int_equal(ret, EOK); @@ -3449,6 +3464,7 @@ void test_nss_getnamebycert_neg(void **state) assert_int_equal(nss_test_ctx->ncache_hits, 0); + set_cmd_cb(NULL); ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETNAMEBYCERT, nss_test_ctx->nss_cmds); assert_int_equal(ret, EOK); @@ -3465,6 +3481,7 @@ void test_nss_getnamebycert_neg(void **state) nss_test_ctx->tctx->done = false; mock_input_user_or_group(TEST_TOKEN_CERT); + set_cmd_cb(NULL); ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETNAMEBYCERT, nss_test_ctx->nss_cmds); assert_int_equal(ret, EOK); @@ -3590,6 +3607,7 @@ void test_nss_getsidbyname_neg(void **state) mock_account_recv_simple(); /* Query for that user, call a callback when command finishes */ + set_cmd_cb(NULL); ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETSIDBYNAME, nss_test_ctx->nss_cmds); assert_int_equal(ret, EOK);