Clean up client-side preauth error data handling
authorGreg Hudson <ghudson@mit.edu>
Mon, 21 Nov 2011 21:14:39 +0000 (21:14 +0000)
committerGreg Hudson <ghudson@mit.edu>
Mon, 21 Nov 2011 21:14:39 +0000 (21:14 +0000)
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

src/include/k5-int.h
src/include/krb5/preauth_plugin.h
src/lib/krb5/krb/fast.c
src/lib/krb5/krb/get_in_tkt.c
src/lib/krb5/krb/init_creds_ctx.h
src/lib/krb5/krb/preauth2.c
src/plugins/preauth/pkinit/pkinit_clnt.c

index 21519b096c4dcfe5a816f7cb621bcf106c0db339..514e2ea6db4e2d0a492b4b5a065eaf6b6c1abe4d 100644 (file)
@@ -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);
index a0b15a810e6bf93cd6531b46bdcf94214b92a1e3..f732b947d2b57179b9ef318f312982684e8ad34f 100644 (file)
@@ -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);
 
index 2290d487407759af88251c2164463b91addd632c..226cd0665dd95b2c4f1e25e9e7539ab1bd6922d2 100644 (file)
@@ -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;
 }
index f39f2184e84c9c878036526881caaeb6dd3c947a..4237915d8453e2a4e9bd5c665aad816a112dad5d 100644 (file)
@@ -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);
index aa3129d7af474d9972100d91c23a91e0966ee4bd..604f3f89a580d2a674f61183538743c50a9f2064 100644 (file)
@@ -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;
index 810096a09cae82ccf73096d7066a9a1ed3a43f64..0c8ead5fe70766bbd7ecd626d04f01dd008958bb 100644 (file)
@@ -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) {
index 2e5afef75fde01594ceac834d40e9b77961c6433..ad354cf0bda29262d72b520785bc16b97c753263 100644 (file)
@@ -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);