From 4f70d2204ec44d4eae3768bb8f9e29e295983307 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Thu, 21 Mar 2024 15:43:15 +0100 Subject: [PATCH] Resolves: RHEL-28161 - Passkey cannot fall back to password --- ...5-add-OTP-to-krb5-response-selection.patch | 185 ++++++++++++++++++ ...nswer_pkinit-use-matching-debug-mess.patch | 119 +++++++++++ ...er-and-pre-auth-debug-message-less-i.patch | 67 +++++++ ..._sss-prefer-Smartcard-authentication.patch | 70 +++++++ sssd.spec | 9 +- 5 files changed, 449 insertions(+), 1 deletion(-) create mode 100644 0009-krb5-add-OTP-to-krb5-response-selection.patch create mode 100644 0010-krb5-make-sure-answer_pkinit-use-matching-debug-mess.patch create mode 100644 0011-krb5-make-prompter-and-pre-auth-debug-message-less-i.patch create mode 100644 0012-pam_sss-prefer-Smartcard-authentication.patch diff --git a/0009-krb5-add-OTP-to-krb5-response-selection.patch b/0009-krb5-add-OTP-to-krb5-response-selection.patch new file mode 100644 index 0000000..87fbf45 --- /dev/null +++ b/0009-krb5-add-OTP-to-krb5-response-selection.patch @@ -0,0 +1,185 @@ +From 5b9bc0a1a6116e6fb001c7dce7497854fcdd40c4 Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Thu, 14 Mar 2024 09:18:45 +0100 +Subject: [PATCH 09/12] krb5: add OTP to krb5 response selection +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Originally where there was only password and OTP authentication we +checked for password authentication and used OTP as a fallback. This was +continued as other (pre)-authentication types were added. But so far +only one authentication type was returned. + +This changed recently to allow the user a better selection and as a +result OTP cannot be handled as a fallback anymore but has to be added +to the selection. In case there are no types (questions) available now +password is used as a fallback. + +Resolves: https://github.com/SSSD/sssd/issues/7152 + +Reviewed-by: Alejandro López +Reviewed-by: Justin Stephenson +(cherry picked from commit bf6cb6dcdd94d9f47e4e74acd51e30f86b488943) +--- + src/providers/krb5/krb5_child.c | 107 ++++++++++++++++++++++---------- + 1 file changed, 75 insertions(+), 32 deletions(-) + +diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c +index b8acae7d7..116f2adda 100644 +--- a/src/providers/krb5/krb5_child.c ++++ b/src/providers/krb5/krb5_child.c +@@ -1200,6 +1200,44 @@ done: + #endif /* BUILD_PASSKEY */ + } + ++static krb5_error_code answer_password(krb5_context kctx, ++ struct krb5_req *kr, ++ krb5_responder_context rctx) ++{ ++ krb5_error_code kerr; ++ int ret; ++ const char *pwd; ++ ++ kr->password_prompting = true; ++ ++ if ((kr->pd->cmd == SSS_PAM_AUTHENTICATE ++ || kr->pd->cmd == SSS_PAM_CHAUTHTOK_PRELIM ++ || kr->pd->cmd == SSS_PAM_CHAUTHTOK) ++ && sss_authtok_get_type(kr->pd->authtok) ++ == SSS_AUTHTOK_TYPE_PASSWORD) { ++ ret = sss_authtok_get_password(kr->pd->authtok, &pwd, NULL); ++ if (ret != EOK) { ++ DEBUG(SSSDBG_OP_FAILURE, ++ "sss_authtok_get_password failed.\n"); ++ return ret; ++ } ++ ++ kerr = krb5_responder_set_answer(kctx, rctx, ++ KRB5_RESPONDER_QUESTION_PASSWORD, ++ pwd); ++ if (kerr != 0) { ++ DEBUG(SSSDBG_OP_FAILURE, ++ "krb5_responder_set_answer failed.\n"); ++ } ++ ++ return kerr; ++ } ++ ++ /* For SSS_PAM_PREAUTH and the other remaining commands the caller should ++ * continue to iterate over the available authentication methods. */ ++ return EAGAIN; ++} ++ + static krb5_error_code sss_krb5_responder(krb5_context ctx, + void *data, + krb5_responder_context rctx) +@@ -1207,9 +1245,7 @@ static krb5_error_code sss_krb5_responder(krb5_context ctx, + struct krb5_req *kr = talloc_get_type(data, struct krb5_req); + const char * const *question_list; + size_t c; +- const char *pwd; +- int ret; +- krb5_error_code kerr; ++ krb5_error_code kerr = EINVAL; + + if (kr == NULL) { + return EINVAL; +@@ -1221,34 +1257,18 @@ static krb5_error_code sss_krb5_responder(krb5_context ctx, + for (c = 0; question_list[c] != NULL; c++) { + DEBUG(SSSDBG_TRACE_ALL, "Got question [%s].\n", question_list[c]); + ++ /* It is expected that the answer_*() functions only return EOK ++ * (success) if the authentication was successful, i.e. during ++ * SSS_PAM_AUTHENTICATE. In all other cases, e.g. during ++ * SSS_PAM_PREAUTH either EAGAIN should be returned to indicate ++ * that the other available authentication methods should be ++ * checked as well. Or some other error code to indicate a fatal ++ * error where no other methods should be tried. ++ * Especially if setting the answer failed neither EOK nor EAGAIN ++ * should be returned. */ + if (strcmp(question_list[c], + KRB5_RESPONDER_QUESTION_PASSWORD) == 0) { +- kr->password_prompting = true; +- +- if ((kr->pd->cmd == SSS_PAM_AUTHENTICATE +- || kr->pd->cmd == SSS_PAM_CHAUTHTOK_PRELIM +- || kr->pd->cmd == SSS_PAM_CHAUTHTOK) +- && sss_authtok_get_type(kr->pd->authtok) +- == SSS_AUTHTOK_TYPE_PASSWORD) { +- ret = sss_authtok_get_password(kr->pd->authtok, &pwd, NULL); +- if (ret != EOK) { +- DEBUG(SSSDBG_OP_FAILURE, +- "sss_authtok_get_password failed.\n"); +- return ret; +- } +- +- kerr = krb5_responder_set_answer(ctx, rctx, +- KRB5_RESPONDER_QUESTION_PASSWORD, +- pwd); +- if (kerr != 0) { +- DEBUG(SSSDBG_OP_FAILURE, +- "krb5_responder_set_answer failed.\n"); +- } +- +- return kerr; +- } +- +- kerr = EOK; ++ kerr = answer_password(ctx, kr, rctx); + } else if (strcmp(question_list[c], + KRB5_RESPONDER_QUESTION_PKINIT) == 0 + && (sss_authtok_get_type(kr->pd->authtok) +@@ -1265,6 +1285,8 @@ static krb5_error_code sss_krb5_responder(krb5_context ctx, + continue; + } + kerr = answer_passkey(ctx, kr, rctx); ++ } else if (strcmp(question_list[c], KRB5_RESPONDER_QUESTION_OTP) == 0) { ++ kerr = answer_otp(ctx, kr, rctx); + } else { + DEBUG(SSSDBG_MINOR_FAILURE, "Unknown question type [%s]\n", question_list[c]); + kerr = EINVAL; +@@ -1274,16 +1296,37 @@ static krb5_error_code sss_krb5_responder(krb5_context ctx, + * handled by the answer_* function. This allows fallback between auth + * types, such as passkey -> password. */ + if (kerr == EAGAIN) { +- DEBUG(SSSDBG_TRACE_ALL, "Auth type [%s] could not be handled by answer function, " +- "continuing to next question.\n", question_list[c]); ++ /* During pre-auth iterating over all authentication methods ++ * is expected and no message will be displayed. */ ++ if (kr->pd->cmd == SSS_PAM_AUTHENTICATE) { ++ DEBUG(SSSDBG_TRACE_ALL, ++ "Auth type [%s] could not be handled by answer " ++ "function, continuing to next question.\n", ++ question_list[c]); ++ } + continue; + } else { + return kerr; + } + } ++ } else { ++ kerr = answer_password(ctx, kr, rctx); + } + +- return answer_otp(ctx, kr, rctx); ++ /* During SSS_PAM_PREAUTH 'EAGAIN' is expected because we will run ++ * through all offered authentication methods and all are expect to return ++ * 'EAGAIN' in the positive case to indicate that the other methods should ++ * be checked as well. If all methods are checked we are done and should ++ * return success. ++ * In the other steps, especially SSS_PAM_AUTHENTICATE, having 'EAGAIN' at ++ * this stage would mean that no method feels responsible for the provided ++ * credentials i.e. authentication failed and we should return an error. ++ */ ++ if (kr->pd->cmd == SSS_PAM_PREAUTH) { ++ return kerr == EAGAIN ? 0 : kerr; ++ } else { ++ return kerr; ++ } + } + #endif /* HAVE_KRB5_GET_INIT_CREDS_OPT_SET_RESPONDER */ + +-- +2.42.0 + diff --git a/0010-krb5-make-sure-answer_pkinit-use-matching-debug-mess.patch b/0010-krb5-make-sure-answer_pkinit-use-matching-debug-mess.patch new file mode 100644 index 0000000..db83f42 --- /dev/null +++ b/0010-krb5-make-sure-answer_pkinit-use-matching-debug-mess.patch @@ -0,0 +1,119 @@ +From c3725a13ef694c2c34813953153f33ebfbaf1c27 Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Fri, 15 Mar 2024 11:29:47 +0100 +Subject: [PATCH 10/12] krb5: make sure answer_pkinit() use matching debug + messages +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Resolves: https://github.com/SSSD/sssd/issues/7152 + +Reviewed-by: Alejandro López +Reviewed-by: Justin Stephenson +(cherry picked from commit 7c33f9d57cebfff80778f930ff0cc3144a7cc261) +--- + src/providers/krb5/krb5_child.c | 77 ++++++++++++++++++--------------- + 1 file changed, 42 insertions(+), 35 deletions(-) + +diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c +index 116f2adda..926109588 100644 +--- a/src/providers/krb5/krb5_child.c ++++ b/src/providers/krb5/krb5_child.c +@@ -745,51 +745,58 @@ static krb5_error_code answer_pkinit(krb5_context ctx, + DEBUG(SSSDBG_TRACE_ALL, "Setting pkinit_prompting.\n"); + kr->pkinit_prompting = true; + +- if (kr->pd->cmd == SSS_PAM_AUTHENTICATE +- && (sss_authtok_get_type(kr->pd->authtok) ++ if (kr->pd->cmd == SSS_PAM_AUTHENTICATE) { ++ if ((sss_authtok_get_type(kr->pd->authtok) + == SSS_AUTHTOK_TYPE_SC_PIN + || sss_authtok_get_type(kr->pd->authtok) + == SSS_AUTHTOK_TYPE_SC_KEYPAD)) { +- kerr = sss_authtok_get_sc(kr->pd->authtok, &pin, NULL, +- &token_name, NULL, +- &module_name, NULL, +- NULL, NULL, NULL, NULL); +- if (kerr != EOK) { +- DEBUG(SSSDBG_OP_FAILURE, +- "sss_authtok_get_sc failed.\n"); +- goto done; +- } ++ kerr = sss_authtok_get_sc(kr->pd->authtok, &pin, NULL, ++ &token_name, NULL, ++ &module_name, NULL, ++ NULL, NULL, NULL, NULL); ++ if (kerr != EOK) { ++ DEBUG(SSSDBG_OP_FAILURE, ++ "sss_authtok_get_sc failed.\n"); ++ goto done; ++ } + +- for (c = 0; chl->identities[c] != NULL; c++) { +- if (chl->identities[c]->identity != NULL +- && pkinit_identity_matches(chl->identities[c]->identity, +- token_name, module_name)) { +- break; ++ for (c = 0; chl->identities[c] != NULL; c++) { ++ if (chl->identities[c]->identity != NULL ++ && pkinit_identity_matches(chl->identities[c]->identity, ++ token_name, module_name)) { ++ break; ++ } + } +- } + +- if (chl->identities[c] == NULL) { +- DEBUG(SSSDBG_CRIT_FAILURE, +- "No matching identity for [%s][%s] found in pkinit challenge.\n", +- token_name, module_name); +- kerr = EINVAL; +- goto done; +- } ++ if (chl->identities[c] == NULL) { ++ DEBUG(SSSDBG_CRIT_FAILURE, ++ "No matching identity for [%s][%s] found in pkinit " ++ "challenge.\n", token_name, module_name); ++ kerr = EINVAL; ++ goto done; ++ } + +- kerr = krb5_responder_pkinit_set_answer(ctx, rctx, +- chl->identities[c]->identity, +- pin); +- if (kerr != 0) { +- DEBUG(SSSDBG_OP_FAILURE, +- "krb5_responder_set_answer failed.\n"); +- } ++ kerr = krb5_responder_pkinit_set_answer(ctx, rctx, ++ chl->identities[c]->identity, ++ pin); ++ if (kerr != 0) { ++ DEBUG(SSSDBG_OP_FAILURE, ++ "krb5_responder_set_answer failed.\n"); ++ } + +- goto done; ++ goto done; ++ } else { ++ DEBUG(SSSDBG_MINOR_FAILURE, ++ "Unexpected authentication token type [%s]\n", ++ sss_authtok_type_to_str(sss_authtok_get_type(kr->pd->authtok))); ++ kerr = EAGAIN; ++ goto done; ++ } + } else { +- DEBUG(SSSDBG_MINOR_FAILURE, "Unexpected authentication token type [%s]\n", +- sss_authtok_type_to_str(sss_authtok_get_type(kr->pd->authtok))); ++ /* We only expect SSS_PAM_PREAUTH here, but also for all other ++ * commands the graceful solution would be to let the caller ++ * check other authentication methods as well. */ + kerr = EAGAIN; +- goto done; + } + + done: +-- +2.42.0 + diff --git a/0011-krb5-make-prompter-and-pre-auth-debug-message-less-i.patch b/0011-krb5-make-prompter-and-pre-auth-debug-message-less-i.patch new file mode 100644 index 0000000..5126106 --- /dev/null +++ b/0011-krb5-make-prompter-and-pre-auth-debug-message-less-i.patch @@ -0,0 +1,67 @@ +From 87b54bd8448760241e7071a585f95b3e2604355a Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Fri, 15 Mar 2024 12:35:00 +0100 +Subject: [PATCH 11/12] krb5: make prompter and pre-auth debug message less + irritating +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Resolves: https://github.com/SSSD/sssd/issues/7152 + +Reviewed-by: Alejandro López +Reviewed-by: Justin Stephenson +(cherry picked from commit e26cc69341bcfd2bbc758eca30df296431c70a28) +--- + src/providers/krb5/krb5_child.c | 15 +++++++++++---- + 1 file changed, 11 insertions(+), 4 deletions(-) + +diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c +index 926109588..494711de9 100644 +--- a/src/providers/krb5/krb5_child.c ++++ b/src/providers/krb5/krb5_child.c +@@ -1355,13 +1355,14 @@ static krb5_error_code sss_krb5_prompter(krb5_context context, void *data, + int ret; + size_t c; + struct krb5_req *kr = talloc_get_type(data, struct krb5_req); ++ const char *err_msg; + + if (kr == NULL) { + return EINVAL; + } + + DEBUG(SSSDBG_TRACE_ALL, +- "sss_krb5_prompter name [%s] banner [%s] num_prompts [%d] EINVAL.\n", ++ "sss_krb5_prompter name [%s] banner [%s] num_prompts [%d].\n", + name, banner, num_prompts); + + if (num_prompts != 0) { +@@ -1370,7 +1371,12 @@ static krb5_error_code sss_krb5_prompter(krb5_context context, void *data, + prompts[c].prompt); + } + +- DEBUG(SSSDBG_FUNC_DATA, "Prompter interface isn't used for password prompts by SSSD.\n"); ++ err_msg = krb5_get_error_message(context, KRB5_LIBOS_CANTREADPWD); ++ DEBUG(SSSDBG_FUNC_DATA, ++ "Prompter interface isn't used for prompting by SSSD." ++ "Returning the expected error [%ld/%s].\n", ++ KRB5_LIBOS_CANTREADPWD, err_msg); ++ krb5_free_error_message(context, err_msg); + return KRB5_LIBOS_CANTREADPWD; + } + +@@ -2839,8 +2845,9 @@ static errno_t tgt_req_child(struct krb5_req *kr) + * should now know which authentication methods are available to + * update the password. */ + DEBUG(SSSDBG_TRACE_FUNC, +- "krb5_get_init_creds_password returned [%d] during pre-auth, " +- "ignored.\n", kerr); ++ "krb5_get_init_creds_password returned [%d] while collecting " ++ "available authentication types, errors are expected " ++ "and ignored.\n", kerr); + ret = pam_add_prompting(kr); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, "pam_add_prompting failed.\n"); +-- +2.42.0 + diff --git a/0012-pam_sss-prefer-Smartcard-authentication.patch b/0012-pam_sss-prefer-Smartcard-authentication.patch new file mode 100644 index 0000000..90e27c9 --- /dev/null +++ b/0012-pam_sss-prefer-Smartcard-authentication.patch @@ -0,0 +1,70 @@ +From d06b4a3eda612d1a54b6bdb3c3b779543bc23b0f Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Wed, 20 Mar 2024 11:26:16 +0100 +Subject: [PATCH 12/12] pam_sss: prefer Smartcard authentication +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The current behavior is that Smartcard authentication is preferred if +possible, i.e. if a Smartcard is present. Since the Smartcard (or +equivalent) must be inserted manually the assumption is that if the user +has inserted it they most probably want to use it for authentication. + +With the latest patches pam_sss might receive multiple available +authentication methods. With this patch the checks for available +authentication types start Smartcard authentication to mimic the +existing behavior. + +Resolves: https://github.com/SSSD/sssd/issues/7152 + +Reviewed-by: Alejandro López +Reviewed-by: Justin Stephenson +(cherry picked from commit 0d5e8f11714e8e6cc0ad28e03fecf0f5732528b3) +--- + src/sss_client/pam_sss.c | 22 +++++++++++----------- + 1 file changed, 11 insertions(+), 11 deletions(-) + +diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c +index a1c353604..41a528dda 100644 +--- a/src/sss_client/pam_sss.c ++++ b/src/sss_client/pam_sss.c +@@ -2544,17 +2544,7 @@ static int get_authtok_for_authentication(pam_handle_t *pamh, + } else if (pi->pc != NULL) { + ret = prompt_by_config(pamh, pi); + } else { +- if (flags & PAM_CLI_FLAGS_USE_2FA +- || (pi->otp_vendor != NULL && pi->otp_token_id != NULL +- && pi->otp_challenge != NULL)) { +- if (pi->password_prompting) { +- ret = prompt_2fa(pamh, pi, _("First Factor: "), +- _("Second Factor (optional): ")); +- } else { +- ret = prompt_2fa(pamh, pi, _("First Factor: "), +- _("Second Factor: ")); +- } +- } else if (pi->cert_list != NULL) { ++ if (pi->cert_list != NULL) { + if (pi->cert_list->next == NULL) { + /* Only one certificate */ + pi->selected_cert = pi->cert_list; +@@ -2570,6 +2560,16 @@ static int get_authtok_for_authentication(pam_handle_t *pamh, + || (pi->flags & PAM_CLI_FLAGS_REQUIRE_CERT_AUTH)) { + /* Use pin prompt as fallback for gdm-smartcard */ + ret = prompt_sc_pin(pamh, pi); ++ } else if (flags & PAM_CLI_FLAGS_USE_2FA ++ || (pi->otp_vendor != NULL && pi->otp_token_id != NULL ++ && pi->otp_challenge != NULL)) { ++ if (pi->password_prompting) { ++ ret = prompt_2fa(pamh, pi, _("First Factor: "), ++ _("Second Factor (optional): ")); ++ } else { ++ ret = prompt_2fa(pamh, pi, _("First Factor: "), ++ _("Second Factor: ")); ++ } + } else if (pi->passkey_prompt_pin) { + ret = prompt_passkey(pamh, pi, + _("Insert your passkey device, then press ENTER."), +-- +2.42.0 + diff --git a/sssd.spec b/sssd.spec index 26f47fd..c420a5c 100644 --- a/sssd.spec +++ b/sssd.spec @@ -27,7 +27,7 @@ Name: sssd Version: 2.9.4 -Release: 3%{?dist} +Release: 4%{?dist} Summary: System Security Services Daemon License: GPLv3+ URL: https://github.com/SSSD/sssd/ @@ -42,6 +42,10 @@ Patch0005: 0005-sss-client-handle-key-value-in-destructor.patch Patch0006: 0006-krb5-Allow-fallback-between-responder-questions.patch Patch0007: 0007-krb5-Add-fallback-password-change-support.patch Patch0008: 0008-pam-fix-invalid-if-condition.patch +Patch0009: 0009-krb5-add-OTP-to-krb5-response-selection.patch +Patch0010: 0010-krb5-make-sure-answer_pkinit-use-matching-debug-mess.patch +Patch0011: 0011-krb5-make-prompter-and-pre-auth-debug-message-less-i.patch +Patch0012: 0012-pam_sss-prefer-Smartcard-authentication.patch ### Dependencies ### @@ -1091,6 +1095,9 @@ fi %systemd_postun_with_restart sssd.service %changelog +* Thu Mar 21 2024 Alexey Tikhonov - 2.9.4-4 +- Resolves: RHEL-28161 - Passkey cannot fall back to password + * Wed Mar 13 2024 Alexey Tikhonov - 2.9.4-3 - Resolves: RHEL-22340 - socket leak - Resolves: RHEL-28161 - Passkey cannot fall back to password