1dedfbb334
Resolves: upstream#3588 - sssd_nss consumes more memory until restarted or machine swaps Resolves: failure in glibc tests https://sourceware.org/bugzilla/show_bug.cgi?id=22530 Resolves: upstream#3451 - When sssd is configured with id_provider proxy and auth_provider ldap, login fails if the LDAP server is not allowing anonymous binds Resolves: upstream#3285 - SSSD needs restart after incorrect clock is corrected with AD Resolves: upstream#3586 - Give a more detailed debug and system-log message if krb5_init_context() failed Resolves: rhbz#1431153 - SSSD ships a drop-in configuration snippet in /etc/systemd/system Backport few upstream features from 1.16.1
256 lines
12 KiB
Diff
256 lines
12 KiB
Diff
From 85c087a88ac5d45b68bef609b5b098bf50e95d39 Mon Sep 17 00:00:00 2001
|
|
From: Sumit Bose <sbose@redhat.com>
|
|
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 <fidencio@redhat.com>
|
|
Tested-by: Scott Poore <spoore@redhat.com>
|
|
---
|
|
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
|
|
|