From: Greg Hudson Date: Mon, 21 Nov 2011 21:14:39 +0000 (+0000) Subject: Clean up client-side preauth error data handling X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=3fe47057c7535f4603825a01fb84262b7bfa4c55;p=krb5.git Clean up client-side preauth error data handling Change the clpreauth tryagain method to accept a list of pa-data, taken either from the FAST response or from decoding the e_data as either pa-data or typed-data. Also change the in_padata argument to contain just the type of the request padata rather than the whole element, since modules generally shouldn't care about the contents of their request padata (or they can remember it). In krb5int_fast_process_error, no longer re-encode FAST pa-data as typed-data for the inner error e_data, but decode traditional error e_data for all error types, and try both pa-data and typed-data encoding. In PKINIT, try all elements of the new pa-data list, since it may contain FAST elements as well as the actual PKINIT array. (Fixes an outstanding bug in FAST PKINIT.) ticket: 7023 target_version: 1.10 tags: pullup git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@25483 dc483132-0cff-0310-8789-dd5450dbe970 --- diff --git a/src/include/k5-int.h b/src/include/k5-int.h index 21519b096..514e2ea6d 100644 --- a/src/include/k5-int.h +++ b/src/include/k5-int.h @@ -1105,8 +1105,9 @@ krb5_do_preauth_tryagain(krb5_context context, krb5_kdc_req *request, krb5_data *encoded_request_body, krb5_data *encoded_previous_request, krb5_pa_data **in_padata, krb5_pa_data ***out_padata, - krb5_error *err_reply, krb5_prompter_fct prompter, - void *prompter_data, krb5_clpreauth_rock preauth_rock, + krb5_error *err_reply, krb5_pa_data **err_padata, + krb5_prompter_fct prompter, void *prompter_data, + krb5_clpreauth_rock preauth_rock, krb5_gic_opt_ext *opte); void KRB5_CALLCONV krb5_init_preauth_context(krb5_context); diff --git a/src/include/krb5/preauth_plugin.h b/src/include/krb5/preauth_plugin.h index a0b15a810..f732b947d 100644 --- a/src/include/krb5/preauth_plugin.h +++ b/src/include/krb5/preauth_plugin.h @@ -242,10 +242,14 @@ typedef krb5_error_code krb5_pa_data ***pa_data_out); /* - * Optional: Attempt to use e-data in the error response to try to recover from - * the given error. If this function is provided, and it stores data in - * pa_data_out which is different data from the contents of pa_data_in, then - * the client library will retransmit the request. + * Optional: Attempt to use error and error_padata to try to recover from the + * given error. To work with both FAST and non-FAST errors, an implementation + * should generally consult error_padata rather than decoding error->e_data. + * For non-FAST errors, it contains the e_data decoded as either pa-data or + * typed-data. + * + * If this function is provided, and it returns 0 and stores data in + * pa_data_out, then the client library will retransmit the request. */ typedef krb5_error_code (*krb5_clpreauth_tryagain_fn)(krb5_context context, @@ -257,8 +261,9 @@ typedef krb5_error_code krb5_kdc_req *request, krb5_data *encoded_request_body, krb5_data *encoded_previous_request, - krb5_pa_data *pa_data_in, + 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); diff --git a/src/lib/krb5/krb/fast.c b/src/lib/krb5/krb/fast.c index 2290d4874..226cd0665 100644 --- a/src/lib/krb5/krb/fast.c +++ b/src/lib/krb5/krb/fast.c @@ -346,17 +346,12 @@ decrypt_fast_reply(krb5_context context, } /* - * FAST separates two concepts: the set of padata we're using to - * decide what pre-auth mechanisms to use and the set of padata we're - * making available to mechanisms in order for them to respond to an - * error. The plugin interface in March 2009 does not permit - * separating these concepts for the plugins. This function makes - * both available for future revisions to the plugin interface. It - * also re-encodes the padata from the current error as a encoded - * typed-data and puts that in the e_data field. That will allow - * existing plugins with the old interface to find the error data. - * The output parameter out_padata contains the padata from the error - * whenever padata is available (all the time with fast). + * If state contains an armor key and *err_replyptr contains a FAST error, + * decode it and set *err_replyptr to the inner error and *out_padata to the + * padata in the FAST response. Otherwise, leave *err_replyptr alone and set + * *out_padata to the error e_data decoded as pa-data or typed-data, or to NULL + * if it doesn't decode as either. In either case, set *retry to indicate + * whether the client should try to make a follow-up request. */ krb5_error_code krb5int_fast_process_error(krb5_context context, @@ -373,7 +368,7 @@ krb5int_fast_process_error(krb5_context context, if (state->armor_key) { krb5_pa_data *fx_error_pa; krb5_pa_data **result = NULL; - krb5_data scratch, *encoded_td = NULL; + krb5_data scratch; krb5_error *fx_error = NULL; krb5_fast_response *fast_response = NULL; @@ -408,20 +403,7 @@ krb5int_fast_process_error(krb5_context context, scratch.length = fx_error_pa->length; retval = decode_krb5_error(&scratch, &fx_error); } - /* - * krb5_pa_data and krb5_typed_data are safe to cast between: - * they have the same type fields in the same order. - * (krb5_preauthtype is a krb5_int32). If krb5_typed_data is - * ever changed then this will need to be a copy not a cast. - */ - if (retval == 0) - retval = encode_krb5_typed_data((const krb5_typed_data **) - fast_response->padata, - &encoded_td); if (retval == 0) { - fx_error->e_data = *encoded_td; - free(encoded_td); /*contents owned by fx_error*/ - encoded_td = NULL; krb5_free_error(context, err_reply); *err_replyptr = fx_error; fx_error = NULL; @@ -440,21 +422,18 @@ krb5int_fast_process_error(krb5_context context, krb5_free_error(context, fx_error); krb5_free_fast_response(context, fast_response); } else { /*not FAST*/ + /* Possibly retry if there's any e_data to process. */ *retry = (err_reply->e_data.length > 0); - if ((err_reply->error == KDC_ERR_PREAUTH_REQUIRED || - err_reply->error == KDC_ERR_PREAUTH_FAILED) && - err_reply->e_data.length) { - krb5_pa_data **result = NULL; - retval = decode_krb5_padata_sequence(&err_reply->e_data, &result); - if (retval == 0) { - *out_padata = result; - return 0; - } - krb5_free_pa_data(context, result); - krb5_set_error_message(context, retval, - _("Error decoding padata in error reply")); - return retval; + /* Try to decode e_data as pa-data or typed-data for out_padata. */ + retval = decode_krb5_padata_sequence(&err_reply->e_data, out_padata); + if (retval != 0) { + krb5_typed_data **tdata; + /* krb5_typed data and krb5_pa_data are compatible structures. */ + if (decode_krb5_typed_data(&err_reply->e_data, &tdata) == 0) + *out_padata = (krb5_pa_data **)tdata; + retval = 0; } + *out_padata = padata; } return retval; } diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c index f39f2184e..4237915d8 100644 --- a/src/lib/krb5/krb/get_in_tkt.c +++ b/src/lib/krb5/krb/get_in_tkt.c @@ -526,6 +526,7 @@ krb5_init_creds_free(krb5_context context, zap(ctx->password.data, ctx->password.length); krb5_free_data_contents(context, &ctx->password); krb5_free_error(context, ctx->err_reply); + krb5_free_pa_data(context, ctx->err_padata); krb5_free_cred_contents(context, &ctx->cred); krb5_free_kdc_req(context, ctx->request); krb5_free_kdc_rep(context, ctx->reply); @@ -1130,7 +1131,7 @@ init_creds_step_request(krb5_context context, if (ctx->preauth_to_use != NULL) { /* * Retry after an error other than PREAUTH_NEEDED, - * using e-data to figure out what to change. + * using ctx->err_padata to figure out what to change. */ code = krb5_do_preauth_tryagain(context, ctx->request, @@ -1139,6 +1140,7 @@ init_creds_step_request(krb5_context context, ctx->preauth_to_use, &ctx->request->padata, ctx->err_reply, + ctx->err_padata, ctx->prompter, ctx->prompter_data, &ctx->preauth_rock, @@ -1255,7 +1257,6 @@ init_creds_step_reply(krb5_context context, krb5_data *in) { krb5_error_code code; - krb5_pa_data **padata = NULL; krb5_pa_data **kdc_padata = NULL; krb5_boolean retry = FALSE; int canon_flag = 0; @@ -1277,23 +1278,26 @@ init_creds_step_reply(krb5_context context, if (ctx->err_reply != NULL) { code = krb5int_fast_process_error(context, ctx->fast_state, - &ctx->err_reply, &padata, &retry); + &ctx->err_reply, &ctx->err_padata, + &retry); if (code != 0) goto cleanup; - if (negotiation_requests_restart(context, ctx, padata)) { + if (negotiation_requests_restart(context, ctx, ctx->err_padata)) { ctx->have_restarted = 1; krb5_preauth_request_context_fini(context); if ((ctx->fast_state->fast_state_flags & KRB5INT_FAST_DO_FAST) ==0) ctx->enc_pa_rep_permitted = 0; - code = restart_init_creds_loop(context, ctx, padata); + code = restart_init_creds_loop(context, ctx, ctx->err_padata); krb5_free_error(context, ctx->err_reply); ctx->err_reply = NULL; + krb5_free_pa_data(context, ctx->err_padata); + ctx->err_padata = NULL; } else if (ctx->err_reply->error == KDC_ERR_PREAUTH_REQUIRED && retry) { /* reset the list of preauth types to try */ krb5_free_pa_data(context, ctx->preauth_to_use); - ctx->preauth_to_use = padata; - padata = NULL; + ctx->preauth_to_use = ctx->err_padata; + ctx->err_padata = NULL; /* this will trigger a new call to krb5_do_preauth() */ krb5_free_error(context, ctx->err_reply); ctx->err_reply = NULL; @@ -1489,7 +1493,6 @@ init_creds_step_reply(krb5_context context, ctx->complete = TRUE; cleanup: - krb5_free_pa_data(context, padata); krb5_free_pa_data(context, kdc_padata); krb5_free_keyblock(context, strengthen_key); krb5_free_keyblock_contents(context, &encrypting_key); diff --git a/src/lib/krb5/krb/init_creds_ctx.h b/src/lib/krb5/krb/init_creds_ctx.h index aa3129d7a..604f3f89a 100644 --- a/src/lib/krb5/krb/init_creds_ctx.h +++ b/src/lib/krb5/krb/init_creds_ctx.h @@ -18,6 +18,7 @@ struct _krb5_init_creds_context { unsigned int loopcount; krb5_data password; krb5_error *err_reply; + krb5_pa_data **err_padata; krb5_creds cred; krb5_kdc_req *request; krb5_kdc_rep *reply; diff --git a/src/lib/krb5/krb/preauth2.c b/src/lib/krb5/krb/preauth2.c index 810096a09..0c8ead5fe 100644 --- a/src/lib/krb5/krb/preauth2.c +++ b/src/lib/krb5/krb/preauth2.c @@ -1370,6 +1370,7 @@ krb5_do_preauth_tryagain(krb5_context kcontext, krb5_pa_data **padata, krb5_pa_data ***return_padata, krb5_error *err_reply, + krb5_pa_data **err_padata, krb5_prompter_fct prompter, void *prompter_data, krb5_clpreauth_rock preauth_rock, krb5_gic_opt_ext *opte) @@ -1409,8 +1410,8 @@ krb5_do_preauth_tryagain(krb5_context kcontext, request, encoded_request_body, encoded_previous_request, - padata[i], - err_reply, + padata[i]->pa_type, + err_reply, err_padata, prompter, prompter_data, &out_padata) == 0) { if (out_padata != NULL) { diff --git a/src/plugins/preauth/pkinit/pkinit_clnt.c b/src/plugins/preauth/pkinit/pkinit_clnt.c index 2e5afef75..ad354cf0b 100644 --- a/src/plugins/preauth/pkinit/pkinit_clnt.c +++ b/src/plugins/preauth/pkinit/pkinit_clnt.c @@ -93,7 +93,7 @@ pa_pkinit_gen_req(krb5_context context, pkinit_context plgctx, pkinit_req_context reqctx, krb5_kdc_req * request, - krb5_pa_data * in_padata, + krb5_preauthtype pa_type, krb5_pa_data *** out_padata, krb5_prompter_fct prompter, void *prompter_data, @@ -110,7 +110,7 @@ pa_pkinit_gen_req(krb5_context context, krb5_pa_data **return_pa_data = NULL; cksum.contents = NULL; - reqctx->pa_type = in_padata->pa_type; + reqctx->pa_type = pa_type; pkiDebug("kdc_options = 0x%x till = %d\n", request->kdc_options, request->till); @@ -183,10 +183,10 @@ pa_pkinit_gen_req(krb5_context context, return_pa_data[0]->magic = KV5M_PA_DATA; - if (in_padata->pa_type == KRB5_PADATA_PK_AS_REQ_OLD) + if (pa_type == KRB5_PADATA_PK_AS_REQ_OLD) return_pa_data[0]->pa_type = KRB5_PADATA_PK_AS_REP_OLD; else - return_pa_data[0]->pa_type = in_padata->pa_type; + return_pa_data[0]->pa_type = pa_type; return_pa_data[0]->length = out_data->length; return_pa_data[0]->contents = (krb5_octet *) out_data->data; @@ -1084,7 +1084,7 @@ pkinit_client_process(krb5_context context, krb5_clpreauth_moddata moddata, return retval; } retval = pa_pkinit_gen_req(context, plgctx, reqctx, request, - in_padata, out_padata, prompter, + in_padata->pa_type, out_padata, prompter, prompter_data, gic_opt); } else { /* @@ -1110,85 +1110,76 @@ pkinit_client_tryagain(krb5_context context, krb5_clpreauth_moddata moddata, krb5_clpreauth_callbacks cb, krb5_clpreauth_rock rock, krb5_kdc_req *request, krb5_data *encoded_request_body, krb5_data *encoded_previous_request, - krb5_pa_data *in_padata, krb5_error *err_reply, - krb5_prompter_fct prompter, void *prompter_data, - krb5_pa_data ***out_padata) + krb5_preauthtype pa_type, krb5_error *err_reply, + krb5_pa_data **err_padata, krb5_prompter_fct prompter, + void *prompter_data, krb5_pa_data ***out_padata) { krb5_error_code retval = KRB5KDC_ERR_PREAUTH_FAILED; pkinit_context plgctx = (pkinit_context)moddata; pkinit_req_context reqctx = (pkinit_req_context)modreq; - krb5_typed_data **typed_data = NULL; + krb5_pa_data *pa; krb5_data scratch; - krb5_external_principal_identifier **krb5_trusted_certifiers = NULL; + krb5_external_principal_identifier **certifiers = NULL; krb5_algorithm_identifier **algId = NULL; int do_again = 0; pkiDebug("pkinit_client_tryagain %p %p %p %p\n", context, plgctx, reqctx, request); - if (reqctx->pa_type != in_padata->pa_type) + if (reqctx->pa_type != pa_type || err_padata == NULL) return retval; -#ifdef DEBUG_ASN1 - print_buffer_bin((unsigned char *)err_reply->e_data.data, - err_reply->e_data.length, "/tmp/client_edata"); -#endif - retval = k5int_decode_krb5_typed_data(&err_reply->e_data, &typed_data); - if (retval) { - pkiDebug("decode_krb5_typed_data failed\n"); - goto cleanup; - } -#ifdef DEBUG_ASN1 - print_buffer_bin(typed_data[0]->data, typed_data[0]->length, - "/tmp/client_typed_data"); -#endif - OCTETDATA_TO_KRB5DATA(typed_data[0], &scratch); - - switch(typed_data[0]->type) { - case TD_TRUSTED_CERTIFIERS: - case TD_INVALID_CERTIFICATES: - retval = k5int_decode_krb5_td_trusted_certifiers(&scratch, - &krb5_trusted_certifiers); - if (retval) { - pkiDebug("failed to decode sequence of trusted certifiers\n"); - goto cleanup; - } - retval = pkinit_process_td_trusted_certifiers(context, - plgctx->cryptoctx, reqctx->cryptoctx, reqctx->idctx, - krb5_trusted_certifiers, typed_data[0]->type); - if (!retval) - do_again = 1; - break; - case TD_DH_PARAMETERS: - retval = k5int_decode_krb5_td_dh_parameters(&scratch, &algId); - if (retval) { - pkiDebug("failed to decode td_dh_parameters\n"); - goto cleanup; + for (; *err_padata != NULL && !do_again; err_padata++) { + pa = *err_padata; + PADATA_TO_KRB5DATA(pa, &scratch); + switch (pa->pa_type) { + case TD_TRUSTED_CERTIFIERS: + case TD_INVALID_CERTIFICATES: + retval = k5int_decode_krb5_td_trusted_certifiers(&scratch, + &certifiers); + if (retval) { + pkiDebug("failed to decode sequence of trusted certifiers\n"); + goto cleanup; + } + retval = pkinit_process_td_trusted_certifiers(context, + plgctx->cryptoctx, + reqctx->cryptoctx, + reqctx->idctx, + certifiers, + pa->pa_type); + if (!retval) + do_again = 1; + break; + case TD_DH_PARAMETERS: + retval = k5int_decode_krb5_td_dh_parameters(&scratch, &algId); + if (retval) { + pkiDebug("failed to decode td_dh_parameters\n"); + goto cleanup; + } + retval = pkinit_process_td_dh_params(context, plgctx->cryptoctx, + reqctx->cryptoctx, + reqctx->idctx, algId, + &reqctx->opts->dh_size); + if (!retval) + do_again = 1; + break; + default: + break; } - retval = pkinit_process_td_dh_params(context, plgctx->cryptoctx, - reqctx->cryptoctx, reqctx->idctx, algId, - &reqctx->opts->dh_size); - if (!retval) - do_again = 1; - break; - default: - break; } if (do_again) { - retval = pa_pkinit_gen_req(context, plgctx, reqctx, request, in_padata, - out_padata, prompter, prompter_data, gic_opt); + retval = pa_pkinit_gen_req(context, plgctx, reqctx, request, pa_type, + out_padata, prompter, prompter_data, + gic_opt); if (retval) goto cleanup; } retval = 0; cleanup: - if (krb5_trusted_certifiers != NULL) - free_krb5_external_principal_identifier(&krb5_trusted_certifiers); - - if (typed_data != NULL) - free_krb5_typed_data(&typed_data); + if (certifiers != NULL) + free_krb5_external_principal_identifier(&certifiers); if (algId != NULL) free_krb5_algorithm_identifiers(&algId);