Hi, this patch aims to fix https://fedorahosted.org/sssd/ticket/2811. It occurs if the client of sssd_pam does not provide a user name during pre-auth and certificate/Smartcard authentication is not enabled.
I relaxed the user name check when adding the Smartcard support and added tests as well but unfortunately only for the case when Smartcard authentication is enabled. This patch adds a check for the other case as well. Besides the check itself I tried to send a more suitable error code then PAM_SYSTEM_ERR. For this I had to make a small change in pam_forwarder() to not unconditionally overwrite the return code with EINVAL. I think the change is safe because pam_forwarder_parse_data() does not return ENOENT which would be the only case when a misleading error would be send back to the caller. bye, Sumit
From 170c8c970c9ee2bfd10a6f3d988d7cd5fd33b322 Mon Sep 17 00:00:00 2001 From: Sumit Bose <sb...@redhat.com> Date: Thu, 1 Oct 2015 10:10:22 +0200 Subject: [PATCH] PAM: only allow missing user name for certificate authentication Resolves https://fedorahosted.org/sssd/ticket/2811 --- src/responder/pam/pamsrv_cmd.c | 12 +++++++++--- src/tests/cmocka/test_pam_srv.c | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c index 27dddcf43c1ff6eb465e1cb58d6dddf21413dcc4..978c637e22b03d3be1e07e8dc713aa01c4bb22e5 100644 --- a/src/responder/pam/pamsrv_cmd.c +++ b/src/responder/pam/pamsrv_cmd.c @@ -957,11 +957,13 @@ static errno_t pam_forwarder_parse_data(struct cli_ctx *cctx, struct pam_data *p } else { /* Only SSS_PAM_PREAUTH request may have a missing name, e.g. if the * name is determined with the help of a certificate */ - if (pd->cmd == SSS_PAM_PREAUTH) { + if (pd->cmd == SSS_PAM_PREAUTH + && may_do_cert_auth(talloc_get_type(cctx->rctx->pvt_ctx, + struct pam_ctx),pd)) { ret = EOK; } else { DEBUG(SSSDBG_CRIT_FAILURE, "Missing logon name in PAM request.\n"); - ret = EINVAL; + ret = ENODATA; goto done; } } @@ -1104,7 +1106,6 @@ static int pam_forwarder(struct cli_ctx *cctx, int pam_cmd) } goto done; } else if (ret != EOK) { - ret = EINVAL; goto done; } @@ -1610,6 +1611,11 @@ static int pam_check_user_done(struct pam_auth_req *preq, int ret) pam_reply(preq); break; + case ENODATA: + preq->pd->pam_status = PAM_CRED_INSUFFICIENT; + pam_reply(preq); + break; + default: preq->pd->pam_status = PAM_SYSTEM_ERR; pam_reply(preq); diff --git a/src/tests/cmocka/test_pam_srv.c b/src/tests/cmocka/test_pam_srv.c index ab33433fdce8d6030331a57e2b7cbd97ce5637df..dd20a19be5ca7fd57376b3d639a36b1b4f5158f4 100644 --- a/src/tests/cmocka/test_pam_srv.c +++ b/src/tests/cmocka/test_pam_srv.c @@ -623,6 +623,23 @@ static int test_pam_wrong_pw_offline_auth_check(uint32_t status, return test_pam_simple_check(status, body, blen); } +static int test_pam_creds_insufficient_check(uint32_t status, + uint8_t *body, size_t blen) +{ + size_t rp = 0; + uint32_t val; + + assert_int_equal(status, 0); + + SAFEALIGN_COPY_UINT32(&val, body + rp, &rp); + assert_int_equal(val, PAM_CRED_INSUFFICIENT); + + SAFEALIGN_COPY_UINT32(&val, body + rp, &rp); + assert_int_equal(val, 0); + + return EOK; +} + static int test_pam_user_unknown_check(uint32_t status, uint8_t *body, size_t blen) { @@ -1127,6 +1144,25 @@ void test_pam_offline_chauthtok(void **state) assert_int_equal(ret, EOK); } +void test_pam_preauth_no_logon_name(void **state) +{ + int ret; + + mock_input_pam_cert(pam_test_ctx, NULL, NULL); + + will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); + will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); + + set_cmd_cb(test_pam_creds_insufficient_check); + ret = sss_cmd_execute(pam_test_ctx->cctx, SSS_PAM_PREAUTH, + pam_test_ctx->pam_cmds); + assert_int_equal(ret, EOK); + + /* Wait until the test finishes with EOK */ + ret = test_ev_loop(pam_test_ctx->tctx); + assert_int_equal(ret, EOK); +} + static void set_cert_auth_param(struct pam_ctx *pctx, const char *dbpath) { pam_test_ctx->pctx->cert_auth = true; @@ -1432,6 +1468,8 @@ int main(int argc, const char *argv[]) pam_test_setup, pam_test_teardown), cmocka_unit_test_setup_teardown(test_pam_offline_chauthtok, pam_test_setup, pam_test_teardown), + cmocka_unit_test_setup_teardown(test_pam_preauth_no_logon_name, + pam_test_setup, pam_test_teardown), /* p11_child is not built without NSS */ #ifdef HAVE_NSS cmocka_unit_test_setup_teardown(test_pam_preauth_cert_nocert, -- 2.1.0
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
