From 85c087a88ac5d45b68bef609b5b098bf50e95d39 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Mon, 30 Oct 2017 17:11:56 +0100 Subject: [PATCH 47/79] PAM: allow missing logon_name during certificate authentication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If only one certificate is available and the logon_name is the user is not given the PAM responder already tried to find the name during the pre-auth step. With multiple certificates this might cause useless extra effort and the name should be determined after the certificate is selected in the authentication step. This might currently only happen with GDM because all other PAM clients will prompt for the user name unconditionally. New unit tests are added to cover this new case. Related to https://pagure.io/SSSD/sssd/issue/3560 Reviewed-by: Fabiano Fidêncio Tested-by: Scott Poore --- src/responder/pam/pamsrv_cmd.c | 63 ++++++++++++++++++++++++++----- src/tests/cmocka/test_pam_srv.c | 82 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 10 deletions(-) diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c index 8b2c086e206796ad4c977495be957c56b3255e7f..caf6c99489b8378d2e850473191223709920cd79 100644 --- a/src/responder/pam/pamsrv_cmd.c +++ b/src/responder/pam/pamsrv_cmd.c @@ -1151,6 +1151,7 @@ static errno_t pam_forwarder_parse_data(struct cli_ctx *cctx, struct pam_data *p size_t blen; errno_t ret; uint32_t terminator; + const char *key_id; prctx = talloc_get_type(cctx->protocol_ctx, struct cli_protocol); @@ -1191,9 +1192,33 @@ static errno_t pam_forwarder_parse_data(struct cli_ctx *cctx, struct pam_data *p pd->logon_name, &pd->domain, &pd->user); } 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 + /* SSS_PAM_PREAUTH request may have a missing name, e.g. if the + * name is determined with the help of a certificate. During + * SSS_PAM_AUTHENTICATE at least a key ID is needed to identify the + * selected certificate. */ + if (pd->cmd == SSS_PAM_AUTHENTICATE + && may_do_cert_auth(talloc_get_type(cctx->rctx->pvt_ctx, + struct pam_ctx), pd) + && (sss_authtok_get_type(pd->authtok) == SSS_AUTHTOK_TYPE_SC_PIN + || sss_authtok_get_type(pd->authtok) + == SSS_AUTHTOK_TYPE_SC_KEYPAD)) { + ret = sss_authtok_get_sc(pd->authtok, NULL, NULL, NULL, NULL, NULL, + NULL, &key_id, NULL); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "sss_authtok_get_sc failed.\n"); + goto done; + } + + if (key_id == NULL || *key_id == '\0') { + DEBUG(SSSDBG_CRIT_FAILURE, + "Missing logon and Smartcard key ID during " + "authentication.\n"); + ret = ERR_NO_CREDS; + goto done; + } + + ret = EOK; + } else if (pd->cmd == SSS_PAM_PREAUTH && may_do_cert_auth(talloc_get_type(cctx->rctx->pvt_ctx, struct pam_ctx), pd)) { ret = EOK; @@ -1375,9 +1400,12 @@ static int pam_forwarder(struct cli_ctx *cctx, int pam_cmd) /* Determine what domain type to contact */ preq->req_dom_type = get_domain_request_type(preq, pctx); - /* try backend first for authentication before doing local Smartcard - * authentication */ - if (pd->cmd != SSS_PAM_AUTHENTICATE && may_do_cert_auth(pctx, pd)) { + /* Try backend first for authentication before doing local Smartcard + * authentication if a logon name is available. Otherwise try to derive + * the logon name from the certificate first. */ + if ((pd->cmd != SSS_PAM_AUTHENTICATE + || (pd->cmd == SSS_PAM_AUTHENTICATE && pd->logon_name == NULL)) + && may_do_cert_auth(pctx, pd)) { ret = check_cert(cctx, cctx->ev, pctx, preq, pd); /* Finish here */ goto done; @@ -1577,9 +1605,10 @@ static void pam_forwarder_lookup_by_cert_done(struct tevent_req *req) } else { if (preq->pd->logon_name == NULL) { - if (preq->pd->cmd != SSS_PAM_PREAUTH) { + if (preq->pd->cmd != SSS_PAM_PREAUTH + && preq->pd->cmd != SSS_PAM_AUTHENTICATE) { DEBUG(SSSDBG_CRIT_FAILURE, - "Missing logon name only allowed during pre-auth.\n"); + "Missing logon name only allowed during (pre-)auth.\n"); ret = ENOENT; goto done; } @@ -1641,7 +1670,8 @@ static void pam_forwarder_lookup_by_cert_done(struct tevent_req *req) } } - if (preq->cctx->rctx->domains->user_name_hint) { + if (preq->cctx->rctx->domains->user_name_hint + && preq->pd->cmd == SSS_PAM_PREAUTH) { ret = add_pam_cert_response(preq->pd, cert_user, preq->cert_list, SSS_PAM_CERT_INFO_WITH_HINT); @@ -1664,6 +1694,20 @@ static void pam_forwarder_lookup_by_cert_done(struct tevent_req *req) goto done; } + /* If logon_name was not given during authentication add a + * SSS_PAM_CERT_INFO message to send the name to the caller. */ + if (preq->pd->cmd == SSS_PAM_AUTHENTICATE + && preq->pd->logon_name == NULL) { + ret = add_pam_cert_response(preq->pd, cert_user, + preq->cert_list, + SSS_PAM_CERT_INFO); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "add_pam_cert_response failed.\n"); + preq->pd->pam_status = PAM_AUTHINFO_UNAVAIL; + goto done; + } + } + /* cert_user will be returned to the PAM client as user name, so * we can use it here already e.g. to set in initgroups timeout */ preq->pd->logon_name = talloc_strdup(preq->pd, cert_user); @@ -2039,7 +2083,6 @@ static void pam_dom_forwarder(struct pam_auth_req *preq) "the backend.\n"); } - /* FIXME: use the right cert info */ ret = add_pam_cert_response(preq->pd, cert_user, preq->current_cert, SSS_PAM_CERT_INFO); diff --git a/src/tests/cmocka/test_pam_srv.c b/src/tests/cmocka/test_pam_srv.c index 02b7ddd16e6d30fec4c703c676066ae1eef1cf89..f5ba131357b536adac4b3466ebbc257a838d1420 100644 --- a/src/tests/cmocka/test_pam_srv.c +++ b/src/tests/cmocka/test_pam_srv.c @@ -967,6 +967,16 @@ static int test_pam_cert_check(uint32_t status, uint8_t *body, size_t blen) NULL); } +static int test_pam_cert_check_auth_success(uint32_t status, uint8_t *body, + size_t blen) +{ + assert_int_equal(pam_test_ctx->exp_pam_status, PAM_BAD_ITEM); + pam_test_ctx->exp_pam_status = PAM_SUCCESS; + return test_pam_cert_check_ex(status, body, blen, + SSS_PAM_CERT_INFO, "pamuser@"TEST_DOM_NAME, + NULL); +} + static int test_pam_cert_check_with_hint(uint32_t status, uint8_t *body, size_t blen) { @@ -2265,6 +2275,74 @@ void test_pam_cert_auth(void **state) assert_int_equal(ret, EOK); } +void test_pam_cert_auth_no_logon_name(void **state) +{ + int ret; + + set_cert_auth_param(pam_test_ctx->pctx, NSS_DB); + + /* Here the last option must be set to true because the backend is only + * connected once. During authentication the backend is connected first to + * see if it can handle Smartcard authentication, but before that the user + * is looked up. Since the first mocked reply already adds the certificate + * to the user entry the lookup by certificate will already find the user + * in the cache and no second request to the backend is needed. */ + mock_input_pam_cert(pam_test_ctx, NULL, "123456", "SSSD Test Token", + "NSS-Internal", + "A5EF7DEE625CA5996C8D1BA7D036708161FD49E7", NULL, + test_lookup_by_cert_cb, TEST_TOKEN_CERT, true); + + mock_account_recv_simple(); + mock_parse_inp("pamuser", NULL, EOK); + + will_return(__wrap_sss_packet_get_cmd, SSS_PAM_AUTHENTICATE); + will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); + + /* Assume backend cannot handle Smartcard credentials */ + pam_test_ctx->exp_pam_status = PAM_BAD_ITEM; + + set_cmd_cb(test_pam_cert_check_auth_success); + ret = sss_cmd_execute(pam_test_ctx->cctx, SSS_PAM_AUTHENTICATE, + 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); +} + +void test_pam_cert_auth_no_logon_name_no_key_id(void **state) +{ + int ret; + + set_cert_auth_param(pam_test_ctx->pctx, NSS_DB); + + /* Here the last option must be set to true because the backend is only + * connected once. During authentication the backend is connected first to + * see if it can handle Smartcard authentication, but before that the user + * is looked up. Since the first mocked reply already adds the certificate + * to the user entry the lookup by certificate will already find the user + * in the cache and no second request to the backend is needed. */ + mock_input_pam_cert(pam_test_ctx, NULL, "123456", "SSSD Test Token", + "NSS-Internal", NULL, NULL, + NULL, NULL, false); + + will_return(__wrap_sss_packet_get_cmd, SSS_PAM_AUTHENTICATE); + will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); + + /* Assume backend cannot handle Smartcard credentials */ + pam_test_ctx->exp_pam_status = PAM_BAD_ITEM; + + set_cmd_cb(test_pam_creds_insufficient_check); + ret = sss_cmd_execute(pam_test_ctx->cctx, SSS_PAM_AUTHENTICATE, + 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); +} + void test_pam_cert_auth_double_cert(void **state) { int ret; @@ -2721,6 +2799,10 @@ int main(int argc, const char *argv[]) pam_test_teardown), cmocka_unit_test_setup_teardown(test_pam_cert_auth_double_cert, pam_test_setup, pam_test_teardown), + cmocka_unit_test_setup_teardown(test_pam_cert_auth_no_logon_name, + pam_test_setup, pam_test_teardown), + cmocka_unit_test_setup_teardown(test_pam_cert_auth_no_logon_name_no_key_id, + pam_test_setup, pam_test_teardown), #endif /* HAVE_NSS */ cmocka_unit_test_setup_teardown(test_filter_response, -- 2.15.1