From 9b525f2406da57eb7a064fc56398a41e2680999a Mon Sep 17 00:00:00 2001 From: Greg Hudson 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