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