Resolves: RHEL-28161 - Passkey cannot fall back to password

This commit is contained in:
Alexey Tikhonov 2024-03-21 15:43:15 +01:00
parent ea62250f41
commit 4f70d2204e
5 changed files with 449 additions and 1 deletions

View File

@ -0,0 +1,185 @@
From 5b9bc0a1a6116e6fb001c7dce7497854fcdd40c4 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose@redhat.com>
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 <allopez@redhat.com>
Reviewed-by: Justin Stephenson <jstephen@redhat.com>
(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

View File

@ -0,0 +1,119 @@
From c3725a13ef694c2c34813953153f33ebfbaf1c27 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose@redhat.com>
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 <allopez@redhat.com>
Reviewed-by: Justin Stephenson <jstephen@redhat.com>
(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

View File

@ -0,0 +1,67 @@
From 87b54bd8448760241e7071a585f95b3e2604355a Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose@redhat.com>
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 <allopez@redhat.com>
Reviewed-by: Justin Stephenson <jstephen@redhat.com>
(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

View File

@ -0,0 +1,70 @@
From d06b4a3eda612d1a54b6bdb3c3b779543bc23b0f Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose@redhat.com>
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 <allopez@redhat.com>
Reviewed-by: Justin Stephenson <jstephen@redhat.com>
(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

View File

@ -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 <atikhono@redhat.com> - 2.9.4-4
- Resolves: RHEL-28161 - Passkey cannot fall back to password
* Wed Mar 13 2024 Alexey Tikhonov <atikhono@redhat.com> - 2.9.4-3
- Resolves: RHEL-22340 - socket leak
- Resolves: RHEL-28161 - Passkey cannot fall back to password