From 44fdcedd2a61cd40fe68aef533c878b5f2f665a8 Mon Sep 17 00:00:00 2001 From: Greg Hudson Date: Tue, 20 Dec 2016 16:06:24 -0500 Subject: [PATCH] Properly scope per-request preauth data It should be possible to successfully use multiple initial credentials contexts with the same library context. Create a new internal type krb5_preauth_req_context containing per-request preauth state, including the clpreauth modreq handles and the list of preauth types already tried. Remove this state from clpreauth_handle and krb5_preauth_context. ticket: 7877 (cherry picked from commit b061f419cfc9653b7549b905e54fbbd78deea49e) --- src/include/k5-trace.h | 3 + src/lib/krb5/krb/get_in_tkt.c | 12 +- src/lib/krb5/krb/init_creds_ctx.h | 3 + src/lib/krb5/krb/int-proto.h | 8 +- src/lib/krb5/krb/preauth2.c | 190 +++++++++++++++++++----------- 5 files changed, 135 insertions(+), 81 deletions(-) diff --git a/src/include/k5-trace.h b/src/include/k5-trace.h index 2885408a2..f44f162d3 100644 --- a/src/include/k5-trace.h +++ b/src/include/k5-trace.h @@ -291,6 +291,9 @@ void krb5int_trace(krb5_context context, const char *fmt, ...); TRACE(c, "Preauth tryagain input types: {patypes}", padata) #define TRACE_PREAUTH_TRYAGAIN_OUTPUT(c, padata) \ TRACE(c, "Followup preauth for next request: {patypes}", padata) +#define TRACE_PREAUTH_WRONG_CONTEXT(c) \ + TRACE(c, "Wrong context passed to krb5_init_creds_free(); leaking " \ + "modreq objects") #define TRACE_PROFILE_ERR(c,subsection, section, retval) \ TRACE(c, "Bad value of {str} from [{str}] in conf file: {kerr}", \ diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c index ed15550f0..80f5e1870 100644 --- a/src/lib/krb5/krb/get_in_tkt.c +++ b/src/lib/krb5/krb/get_in_tkt.c @@ -565,7 +565,7 @@ krb5_init_creds_free(krb5_context context, k5_response_items_free(ctx->rctx.items); free(ctx->in_tkt_service); zapfree(ctx->gakpw.storage.data, ctx->gakpw.storage.length); - k5_preauth_request_context_fini(context); + k5_preauth_request_context_fini(context, ctx); krb5_free_error(context, ctx->err_reply); krb5_free_pa_data(context, ctx->err_padata); krb5_free_cred_contents(context, &ctx->cred); @@ -816,8 +816,8 @@ restart_init_creds_loop(krb5_context context, krb5_init_creds_context ctx, if (fast_upgrade) ctx->fast_state->fast_state_flags |= KRB5INT_FAST_DO_FAST; - k5_preauth_request_context_fini(context); - k5_preauth_request_context_init(context); + k5_preauth_request_context_fini(context, ctx); + k5_preauth_request_context_init(context, ctx); krb5_free_data(context, ctx->outer_request_body); ctx->outer_request_body = NULL; if (ctx->opt->flags & KRB5_GET_INIT_CREDS_OPT_PREAUTH_LIST) { @@ -1504,7 +1504,7 @@ init_creds_step_reply(krb5_context context, } else if ((reply_code == KDC_ERR_MORE_PREAUTH_DATA_REQUIRED || reply_code == KDC_ERR_PREAUTH_REQUIRED) && retry) { /* reset the list of preauth types to try */ - k5_reset_preauth_types_tried(context); + k5_reset_preauth_types_tried(ctx); krb5_free_pa_data(context, ctx->preauth_to_use); ctx->preauth_to_use = ctx->err_padata; ctx->err_padata = NULL; @@ -1555,7 +1555,7 @@ init_creds_step_reply(krb5_context context, goto cleanup; /* process any preauth data in the as_reply */ - k5_reset_preauth_types_tried(context); + k5_reset_preauth_types_tried(ctx); code = krb5int_fast_process_response(context, ctx->fast_state, ctx->reply, &strengthen_key); if (code != 0) @@ -1640,7 +1640,7 @@ init_creds_step_reply(krb5_context context, k5_prependmsg(context, code, _("Failed to store credentials")); } - k5_preauth_request_context_fini(context); + k5_preauth_request_context_fini(context, ctx); /* success */ ctx->complete = TRUE; diff --git a/src/lib/krb5/krb/init_creds_ctx.h b/src/lib/krb5/krb/init_creds_ctx.h index 38c01c775..a7cded942 100644 --- a/src/lib/krb5/krb/init_creds_ctx.h +++ b/src/lib/krb5/krb/init_creds_ctx.h @@ -6,6 +6,8 @@ #include "k5-json.h" #include "int-proto.h" +typedef struct krb5_preauth_req_context_st *krb5_preauth_req_context; + struct krb5_responder_context_st { k5_response_items *items; }; @@ -67,6 +69,7 @@ struct _krb5_init_creds_context { krb5_timestamp pa_offset; krb5_int32 pa_offset_usec; enum { NO_OFFSET = 0, UNAUTH_OFFSET, AUTH_OFFSET } pa_offset_state; + krb5_preauth_req_context preauth_reqctx; }; krb5_error_code diff --git a/src/lib/krb5/krb/int-proto.h b/src/lib/krb5/krb/int-proto.h index 9c746d05b..f1667c238 100644 --- a/src/lib/krb5/krb/int-proto.h +++ b/src/lib/krb5/krb/int-proto.h @@ -194,17 +194,19 @@ void k5_free_preauth_context(krb5_context context); void -k5_reset_preauth_types_tried(krb5_context context); +k5_reset_preauth_types_tried(krb5_init_creds_context ctx); void k5_preauth_prepare_request(krb5_context context, krb5_get_init_creds_opt *opt, krb5_kdc_req *request); void -k5_preauth_request_context_init(krb5_context context); +k5_preauth_request_context_init(krb5_context context, + krb5_init_creds_context ctx); void -k5_preauth_request_context_fini(krb5_context context); +k5_preauth_request_context_fini(krb5_context context, + krb5_init_creds_context ctx); krb5_error_code k5_response_items_new(k5_response_items **ri_out); diff --git a/src/lib/krb5/krb/preauth2.c b/src/lib/krb5/krb/preauth2.c index b04d14829..9a178f4e3 100644 --- a/src/lib/krb5/krb/preauth2.c +++ b/src/lib/krb5/krb/preauth2.c @@ -46,14 +46,18 @@ typedef struct { struct krb5_clpreauth_vtable_st vt; krb5_clpreauth_moddata data; - krb5_clpreauth_modreq req; } *clpreauth_handle; struct krb5_preauth_context_st { - krb5_preauthtype *tried; clpreauth_handle *handles; }; +struct krb5_preauth_req_context_st { + krb5_context orig_context; + krb5_preauthtype *tried; + krb5_clpreauth_modreq *modreqs; +}; + /* Release the memory used by a list of handles. */ static void free_handles(krb5_context context, clpreauth_handle *handles) @@ -71,21 +75,44 @@ free_handles(krb5_context context, clpreauth_handle *handles) free(handles); } -/* Find the handle in handles which can process pa_type. */ -static clpreauth_handle -find_module(clpreauth_handle *handles, krb5_preauthtype pa_type) +/* Return an index into handles which can process pa_type, or -1 if none is + * found found. */ +static int +search_module_list(clpreauth_handle *handles, krb5_preauthtype pa_type) { - clpreauth_handle *hp, h; - krb5_preauthtype *tp; + clpreauth_handle h; + int i, j; - for (hp = handles; *hp != NULL; hp++) { - h = *hp; - for (tp = h->vt.pa_type_list; *tp != 0; tp++) { - if (*tp == pa_type) - return h; + for (i = 0; handles[i] != NULL; i++) { + h = handles[i]; + for (j = 0; h->vt.pa_type_list[j] != 0; j++) { + if (h->vt.pa_type_list[j] == pa_type) + return i; } } - return FALSE; + return -1; +} + +/* Find the handle which can process pa_type, or NULL if none is found. On + * success, set *modreq_out to the corresponding per-request module data. */ +static clpreauth_handle +find_module(krb5_context context, krb5_init_creds_context ctx, + krb5_preauthtype pa_type, krb5_clpreauth_modreq *modreq_out) +{ + krb5_preauth_context pctx = context->preauth_context; + krb5_preauth_req_context reqctx = ctx->preauth_reqctx; + int i; + + *modreq_out = NULL; + if (pctx == NULL || reqctx == NULL) + return NULL; + + i = search_module_list(pctx->handles, pa_type); + if (i == -1) + return NULL; + + *modreq_out = reqctx->modreqs[i]; + return pctx->handles[i]; } /* Initialize the preauth state for a krb5 context. */ @@ -93,7 +120,8 @@ void k5_init_preauth_context(krb5_context context) { krb5_plugin_initvt_fn *modules = NULL, *mod; - clpreauth_handle *list = NULL, h, h2; + clpreauth_handle *list = NULL, h; + int i; size_t count; krb5_preauthtype *tp; @@ -140,9 +168,10 @@ k5_init_preauth_context(krb5_context context) /* Check for a preauth type conflict with an existing module. */ for (tp = h->vt.pa_type_list; *tp != 0; tp++) { - h2 = find_module(list, *tp); - if (h2 != NULL) { - TRACE_PREAUTH_CONFLICT(context, h->vt.name, h2->vt.name, *tp); + i = search_module_list(list, *tp); + if (i != -1) { + TRACE_PREAUTH_CONFLICT(context, h->vt.name, list[i]->vt.name, + *tp); break; } } @@ -164,7 +193,6 @@ k5_init_preauth_context(krb5_context context) context->preauth_context = malloc(sizeof(*context->preauth_context)); if (context->preauth_context == NULL) goto cleanup; - context->preauth_context->tried = NULL; context->preauth_context->handles = list; list = NULL; @@ -179,14 +207,14 @@ cleanup: * AS-REP). */ void -k5_reset_preauth_types_tried(krb5_context context) +k5_reset_preauth_types_tried(krb5_init_creds_context ctx) { - krb5_preauth_context pctx = context->preauth_context; + krb5_preauth_req_context reqctx = ctx->preauth_reqctx; - if (pctx == NULL) + if (reqctx == NULL) return; - free(pctx->tried); - pctx->tried = NULL; + free(reqctx->tried); + reqctx->tried = NULL; } @@ -200,7 +228,6 @@ k5_free_preauth_context(krb5_context context) if (pctx == NULL) return; - free(pctx->tried); free_handles(context, pctx->handles); free(pctx); context->preauth_context = NULL; @@ -209,10 +236,13 @@ k5_free_preauth_context(krb5_context context) /* Initialize the per-AS-REQ context. This means calling the client_req_init * function to give the plugin a chance to allocate a per-request context. */ void -k5_preauth_request_context_init(krb5_context context) +k5_preauth_request_context_init(krb5_context context, + krb5_init_creds_context ctx) { krb5_preauth_context pctx = context->preauth_context; - clpreauth_handle *hp, h; + clpreauth_handle h; + krb5_preauth_req_context reqctx; + size_t count, i; if (pctx == NULL) { k5_init_preauth_context(context); @@ -220,30 +250,50 @@ k5_preauth_request_context_init(krb5_context context) if (pctx == NULL) return; } - k5_reset_preauth_types_tried(context); - for (hp = pctx->handles; *hp != NULL; hp++) { - h = *hp; + + reqctx = calloc(1, sizeof(*reqctx)); + if (reqctx == NULL) + return; + reqctx->orig_context = context; + + /* Create an array of per-request module data objects corresponding to the + * preauth context's array of handles. */ + for (count = 0; pctx->handles[count] != NULL; count++); + reqctx->modreqs = calloc(count, sizeof(*reqctx->modreqs)); + for (i = 0; i < count; i++) { + h = pctx->handles[i]; if (h->vt.request_init != NULL) - h->vt.request_init(context, h->data, &h->req); + h->vt.request_init(context, h->data, &reqctx->modreqs[i]); } + ctx->preauth_reqctx = reqctx; } /* Free the per-AS-REQ context. This means clearing any request-specific * context which the plugin may have created. */ void -k5_preauth_request_context_fini(krb5_context context) +k5_preauth_request_context_fini(krb5_context context, + krb5_init_creds_context ctx) { krb5_preauth_context pctx = context->preauth_context; - clpreauth_handle *hp, h; + krb5_preauth_req_context reqctx = ctx->preauth_reqctx; + size_t i; + clpreauth_handle h; - if (pctx == NULL) + if (reqctx == NULL) return; - for (hp = pctx->handles; *hp != NULL; hp++) { - h = *hp; - if (h->req != NULL && h->vt.request_fini != NULL) - h->vt.request_fini(context, h->data, h->req); - h->req = NULL; + if (reqctx->orig_context == context && pctx != NULL) { + for (i = 0; pctx->handles[i] != NULL; i++) { + h = pctx->handles[i]; + if (reqctx->modreqs[i] != NULL && h->vt.request_fini != NULL) + h->vt.request_fini(context, h->data, reqctx->modreqs[i]); + } + } else { + TRACE_PREAUTH_WRONG_CONTEXT(context); } + free(reqctx->modreqs); + free(reqctx->tried); + free(reqctx); + ctx->preauth_reqctx = NULL; } /* Return 1 if pa_type is a real preauthentication mechanism according to the @@ -259,6 +309,7 @@ clpreauth_is_real(krb5_context context, clpreauth_handle h, static krb5_error_code clpreauth_prep_questions(krb5_context context, clpreauth_handle h, + krb5_clpreauth_modreq modreq, krb5_get_init_creds_opt *opt, krb5_clpreauth_callbacks cb, krb5_clpreauth_rock rock, krb5_kdc_req *req, krb5_data *req_body, @@ -266,35 +317,35 @@ clpreauth_prep_questions(krb5_context context, clpreauth_handle h, { if (h->vt.prep_questions == NULL) return 0; - return h->vt.prep_questions(context, h->data, h->req, opt, cb, rock, req, + return h->vt.prep_questions(context, h->data, modreq, opt, cb, rock, req, req_body, prev_req, pa_data); } static krb5_error_code clpreauth_process(krb5_context context, clpreauth_handle h, - krb5_get_init_creds_opt *opt, krb5_clpreauth_callbacks cb, - krb5_clpreauth_rock rock, krb5_kdc_req *req, - krb5_data *req_body, krb5_data *prev_req, + krb5_clpreauth_modreq modreq, krb5_get_init_creds_opt *opt, + krb5_clpreauth_callbacks cb, krb5_clpreauth_rock rock, + krb5_kdc_req *req, krb5_data *req_body, krb5_data *prev_req, krb5_pa_data *pa_data, krb5_prompter_fct prompter, void *prompter_data, krb5_pa_data ***pa_data_out) { - return h->vt.process(context, h->data, h->req, opt, cb, rock, req, + return h->vt.process(context, h->data, modreq, opt, cb, rock, req, req_body, prev_req, pa_data, prompter, prompter_data, pa_data_out); } static krb5_error_code clpreauth_tryagain(krb5_context context, clpreauth_handle h, - krb5_get_init_creds_opt *opt, krb5_clpreauth_callbacks cb, - krb5_clpreauth_rock rock, krb5_kdc_req *req, - krb5_data *req_body, krb5_data *prev_req, + krb5_clpreauth_modreq modreq, krb5_get_init_creds_opt *opt, + krb5_clpreauth_callbacks cb, krb5_clpreauth_rock rock, + krb5_kdc_req *req, krb5_data *req_body, krb5_data *prev_req, krb5_preauthtype pa_type, krb5_error *error, krb5_pa_data **error_padata, krb5_prompter_fct prompter, void *prompter_data, krb5_pa_data ***pa_data_out) { if (h->vt.tryagain == NULL) return 0; - return h->vt.tryagain(context, h->data, h->req, opt, cb, rock, req, + return h->vt.tryagain(context, h->data, modreq, opt, cb, rock, req, req_body, prev_req, pa_type, error, error_padata, prompter, prompter_data, pa_data_out); } @@ -554,22 +605,22 @@ pa_type_allowed(krb5_init_creds_context ctx, krb5_preauthtype pa_type) * types and return false. */ static krb5_boolean -already_tried(krb5_context context, krb5_preauthtype pa_type) +already_tried(krb5_init_creds_context ctx, krb5_preauthtype pa_type) { - krb5_preauth_context pctx = context->preauth_context; - size_t count; + krb5_preauth_req_context reqctx = ctx->preauth_reqctx; + size_t i; krb5_preauthtype *newptr; - for (count = 0; pctx->tried != NULL && pctx->tried[count] != 0; count++) { - if (pctx->tried[count] == pa_type) + for (i = 0; reqctx->tried != NULL && reqctx->tried[i] != 0; i++) { + if (reqctx->tried[i] == pa_type) return TRUE; } - newptr = realloc(pctx->tried, (count + 2) * sizeof(*newptr)); + newptr = realloc(reqctx->tried, (i + 2) * sizeof(*newptr)); if (newptr == NULL) return FALSE; - pctx->tried = newptr; - pctx->tried[count] = pa_type; - pctx->tried[count + 1] = ENCTYPE_NULL; + reqctx->tried = newptr; + reqctx->tried[i] = pa_type; + reqctx->tried[i + 1] = ENCTYPE_NULL; return FALSE; } @@ -580,16 +631,13 @@ process_pa_data(krb5_context context, krb5_init_creds_context ctx, krb5_pa_data ***out_pa_list, int *out_pa_list_size, krb5_preauthtype *out_type) { - krb5_preauth_context pctx = context->preauth_context; struct errinfo save = EMPTY_ERRINFO; krb5_pa_data *pa, **pa_ptr, **mod_pa; krb5_error_code ret = 0; + krb5_clpreauth_modreq modreq; clpreauth_handle h; int real, i; - if (pctx == NULL) - return ENOENT; - /* Process all informational padata types, then the first real preauth type * we succeed on. */ for (real = 0; real <= 1; real++) { @@ -598,17 +646,17 @@ process_pa_data(krb5_context context, krb5_init_creds_context ctx, /* Restrict real mechanisms to the chosen one if we have one. */ if (real && !pa_type_allowed(ctx, pa->pa_type)) continue; - h = find_module(pctx->handles, pa->pa_type); + h = find_module(context, ctx, pa->pa_type, &modreq); if (h == NULL) continue; /* Make sure this type is for the current pass. */ if (clpreauth_is_real(context, h, pa->pa_type) != real) continue; /* Only try a real mechanism once per authentication. */ - if (real && already_tried(context, pa->pa_type)) + if (real && already_tried(ctx, pa->pa_type)) continue; mod_pa = NULL; - ret = clpreauth_process(context, h, ctx->opt, &callbacks, + ret = clpreauth_process(context, h, modreq, ctx->opt, &callbacks, (krb5_clpreauth_rock)ctx, ctx->request, ctx->inner_request_body, ctx->encoded_previous_request, pa, @@ -858,24 +906,22 @@ 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_preauth_context pctx = context->preauth_context; krb5_error_code ret; krb5_pa_data **mod_pa; + krb5_clpreauth_modreq modreq; clpreauth_handle h; int i; *padata_out = NULL; - if (pctx == NULL) - return KRB5KRB_ERR_GENERIC; TRACE_PREAUTH_TRYAGAIN_INPUT(context, in_padata); for (i = 0; in_padata[i] != NULL; i++) { - h = find_module(pctx->handles, in_padata[i]->pa_type); + h = find_module(context, ctx, in_padata[i]->pa_type, &modreq); if (h == NULL) continue; mod_pa = NULL; - ret = clpreauth_tryagain(context, h, ctx->opt, &callbacks, + ret = clpreauth_tryagain(context, h, modreq, ctx->opt, &callbacks, (krb5_clpreauth_rock)ctx, ctx->request, ctx->inner_request_body, ctx->encoded_previous_request, @@ -897,9 +943,9 @@ static krb5_error_code fill_response_items(krb5_context context, krb5_init_creds_context ctx, krb5_pa_data **in_padata) { - krb5_preauth_context pctx = context->preauth_context; krb5_error_code ret; krb5_pa_data *pa; + krb5_clpreauth_modreq modreq; clpreauth_handle h; int i; @@ -908,11 +954,11 @@ fill_response_items(krb5_context context, krb5_init_creds_context ctx, pa = in_padata[i]; if (!pa_type_allowed(ctx, pa->pa_type)) continue; - h = find_module(pctx->handles, pa->pa_type); + h = find_module(context, ctx, pa->pa_type, &modreq); if (h == NULL) continue; - ret = clpreauth_prep_questions(context, h, ctx->opt, &callbacks, - (krb5_clpreauth_rock)ctx, + ret = clpreauth_prep_questions(context, h, modreq, ctx->opt, + &callbacks, (krb5_clpreauth_rock)ctx, ctx->request, ctx->inner_request_body, ctx->encoded_previous_request, pa); if (ret)