From e443dfe315a38607e7c9dcba219f73253d17032b Mon Sep 17 00:00:00 2001
From: Greg Hudson <ghudson@mit.edu>
Date: Fri, 13 Jan 2017 20:45:48 -0500
Subject: [PATCH] Simplify k5_preauth_tryagain()
When retrying pre-authentication for an error, try only the module for
the selected preauth type, not all preauth types in the original
method data. Pass the error and its padata to k5_preauth_tryagain()
explicitly, so that those fields of krb5_init_creds_context are only
referenced in get_in_tkt.c. Handle a degenerate case in
init_creds_step_reply() to simplify the code in
init_creds_step_request().
ticket: 8537
(cherry picked from commit 27628e5d9d5e6fcfa73276106edbd8149d134dc0)
---
src/include/k5-trace.h | 7 ++--
src/lib/krb5/krb/get_in_tkt.c | 20 ++++-------
src/lib/krb5/krb/int-proto.h | 3 +-
src/lib/krb5/krb/preauth2.c | 64 +++++++++++++++++++----------------
4 files changed, 48 insertions(+), 46 deletions(-)
diff --git a/src/include/k5-trace.h b/src/include/k5-trace.h
index f44f162d3..814da3195 100644
--- a/src/include/k5-trace.h
+++ b/src/include/k5-trace.h
@@ -287,8 +287,11 @@ void krb5int_trace(krb5_context context, const char *fmt, ...);
#define TRACE_PREAUTH_SKIP(c, name, patype) \
TRACE(c, "Skipping previously used preauth module {str} ({int})", \
name, (int) patype)
-#define TRACE_PREAUTH_TRYAGAIN_INPUT(c, padata) \
- TRACE(c, "Preauth tryagain input types: {patypes}", padata)
+#define TRACE_PREAUTH_TRYAGAIN_INPUT(c, patype, padata) \
+ TRACE(c, "Preauth tryagain input types ({int}): {patypes}", patype, padata)
+#define TRACE_PREAUTH_TRYAGAIN(c, name, patype, code) \
+ TRACE(c, "Preauth module {str} ({int}) tryagain returned: {kerr}", \
+ name, (int)patype, code)
#define TRACE_PREAUTH_TRYAGAIN_OUTPUT(c, padata) \
TRACE(c, "Followup preauth for next request: {patypes}", padata)
#define TRACE_PREAUTH_WRONG_CONTEXT(c) \
diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c
index da12204ac..988fca233 100644
--- a/src/lib/krb5/krb/get_in_tkt.c
+++ b/src/lib/krb5/krb/get_in_tkt.c
@@ -1340,17 +1340,11 @@ init_creds_step_request(krb5_context context,
if (code != 0)
goto cleanup;
} else {
- if (ctx->preauth_to_use != NULL) {
- /*
- * Retry after an error other than PREAUTH_NEEDED,
- * using ctx->err_padata to figure out what to change.
- */
- code = k5_preauth_tryagain(context, ctx, ctx->preauth_to_use,
- &ctx->request->padata);
- } else {
- /* No preauth supplied, so can't query the plugins. */
- code = KRB5KRB_ERR_GENERIC;
- }
+ /* Retry after an error other than PREAUTH_NEEDED, using error padata
+ * to figure out what to change. */
+ code = k5_preauth_tryagain(context, ctx, ctx->selected_preauth_type,
+ ctx->err_reply, ctx->err_padata,
+ &ctx->request->padata);
if (code != 0) {
/* couldn't come up with anything better */
code = ctx->err_reply->error + ERROR_TABLE_BASE_krb5;
@@ -1535,10 +1529,10 @@ init_creds_step_reply(krb5_context context,
ctx->enc_pa_rep_permitted = TRUE;
code = restart_init_creds_loop(context, ctx, FALSE);
} else {
- if (retry) {
+ if (retry && ctx->selected_preauth_type != KRB5_PADATA_NONE) {
code = 0;
} else {
- /* error + no hints = give up */
+ /* error + no hints (or no preauth mech) = give up */
code = (krb5_error_code)reply_code + ERROR_TABLE_BASE_krb5;
}
}
diff --git a/src/lib/krb5/krb/int-proto.h b/src/lib/krb5/krb/int-proto.h
index 628f0baa8..8903df232 100644
--- a/src/lib/krb5/krb/int-proto.h
+++ b/src/lib/krb5/krb/int-proto.h
@@ -185,7 +185,8 @@ k5_preauth(krb5_context context, krb5_init_creds_context ctx,
krb5_error_code
k5_preauth_tryagain(krb5_context context, krb5_init_creds_context ctx,
- krb5_pa_data **in_padata, krb5_pa_data ***padata_out);
+ krb5_preauthtype pa_type, krb5_error *err,
+ krb5_pa_data **err_padata, krb5_pa_data ***padata_out);
void
k5_init_preauth_context(krb5_context context);
diff --git a/src/lib/krb5/krb/preauth2.c b/src/lib/krb5/krb/preauth2.c
index cfe3dd5b0..354234a93 100644
--- a/src/lib/krb5/krb/preauth2.c
+++ b/src/lib/krb5/krb/preauth2.c
@@ -911,49 +911,53 @@ add_s4u_x509_user_padata(krb5_context context, krb5_s4u_userid *userid,
}
/*
- * If one of the modules can adjust its AS_REQ data using the contents of the
- * err_reply, return 0. If it's the sort of correction which requires that we
- * ask the user another question, we let the calling application deal with it.
+ * If the module for pa_type can adjust its AS_REQ data using the contents of
+ * err and err_padata, return 0 with *padata_out set to a padata list for the
+ * next request. If it's the sort of correction which requires that we ask the
+ * user another question, we let the calling application deal with it.
*/
krb5_error_code
k5_preauth_tryagain(krb5_context context, krb5_init_creds_context ctx,
- krb5_pa_data **in_padata, krb5_pa_data ***padata_out)
+ krb5_preauthtype pa_type, krb5_error *err,
+ krb5_pa_data **err_padata, krb5_pa_data ***padata_out)
{
krb5_error_code ret;
krb5_pa_data **mod_pa;
krb5_clpreauth_modreq modreq;
clpreauth_handle h;
- int i, count;
+ int count;
*padata_out = NULL;
- TRACE_PREAUTH_TRYAGAIN_INPUT(context, in_padata);
+ TRACE_PREAUTH_TRYAGAIN_INPUT(context, pa_type, err_padata);
- for (i = 0; in_padata[i] != NULL; i++) {
- h = find_module(context, ctx, in_padata[i]->pa_type, &modreq);
- if (h == NULL)
- continue;
- mod_pa = NULL;
- ret = clpreauth_tryagain(context, h, modreq, ctx->opt, &callbacks,
- (krb5_clpreauth_rock)ctx, ctx->request,
- ctx->inner_request_body,
- ctx->encoded_previous_request,
- in_padata[i]->pa_type,
- ctx->err_reply, ctx->err_padata,
- ctx->prompter, ctx->prompter_data, &mod_pa);
- if (ret == 0 && mod_pa != NULL) {
- for (count = 0; mod_pa[count] != NULL; count++);
- ret = copy_cookie(context, ctx->err_padata, &mod_pa, &count);
- if (ret) {
- krb5_free_pa_data(context, mod_pa);
- return ret;
- }
- TRACE_PREAUTH_TRYAGAIN_OUTPUT(context, mod_pa);
- *padata_out = mod_pa;
- return 0;
- }
+ h = find_module(context, ctx, pa_type, &modreq);
+ if (h == NULL)
+ return KRB5KRB_ERR_GENERIC;
+ mod_pa = NULL;
+ ret = clpreauth_tryagain(context, h, modreq, ctx->opt, &callbacks,
+ (krb5_clpreauth_rock)ctx, ctx->request,
+ ctx->inner_request_body,
+ ctx->encoded_previous_request, pa_type, err,
+ err_padata, ctx->prompter, ctx->prompter_data,
+ &mod_pa);
+ TRACE_PREAUTH_TRYAGAIN(context, h->vt.name, pa_type, ret);
+ if (!ret && mod_pa == NULL)
+ ret = KRB5KRB_ERR_GENERIC;
+ if (ret)
+ return ret;
+
+
+ for (count = 0; mod_pa[count] != NULL; count++);
+ ret = copy_cookie(context, err_padata, &mod_pa, &count);
+ if (ret) {
+ krb5_free_pa_data(context, mod_pa);
+ return ret;
}
- return KRB5KRB_ERR_GENERIC;
+
+ TRACE_PREAUTH_TRYAGAIN_OUTPUT(context, mod_pa);
+ *padata_out = mod_pa;
+ return 0;
}
/* Compile the set of response items for in_padata by invoke each module's