svn rev #25483: trunk/src/ include/ include/krb5/ lib/krb5/krb/ plugins/preauth/pkinit/

ghudson@MIT.EDU ghudson at MIT.EDU
Mon Nov 21 16:14:40 EST 2011


http://src.mit.edu/fisheye/changelog/krb5/?cs=25483
Commit By: ghudson
Log Message:
ticket: 7023
subject: Clean up client-side preauth error data handling
target_version: 1.10
tags: pullup

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.)


Changed Files:
U   trunk/src/include/k5-int.h
U   trunk/src/include/krb5/preauth_plugin.h
U   trunk/src/lib/krb5/krb/fast.c
U   trunk/src/lib/krb5/krb/get_in_tkt.c
U   trunk/src/lib/krb5/krb/init_creds_ctx.h
U   trunk/src/lib/krb5/krb/preauth2.c
U   trunk/src/plugins/preauth/pkinit/pkinit_clnt.c
Modified: trunk/src/include/k5-int.h
===================================================================
--- trunk/src/include/k5-int.h	2011-11-21 17:30:41 UTC (rev 25482)
+++ trunk/src/include/k5-int.h	2011-11-21 21:14:39 UTC (rev 25483)
@@ -1105,8 +1105,9 @@
                          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);

Modified: trunk/src/include/krb5/preauth_plugin.h
===================================================================
--- trunk/src/include/krb5/preauth_plugin.h	2011-11-21 17:30:41 UTC (rev 25482)
+++ trunk/src/include/krb5/preauth_plugin.h	2011-11-21 21:14:39 UTC (rev 25483)
@@ -242,10 +242,14 @@
                              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 @@
                               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);
 

Modified: trunk/src/lib/krb5/krb/fast.c
===================================================================
--- trunk/src/lib/krb5/krb/fast.c	2011-11-21 17:30:41 UTC (rev 25482)
+++ trunk/src/lib/krb5/krb/fast.c	2011-11-21 21:14:39 UTC (rev 25483)
@@ -346,17 +346,12 @@
 }
 
 /*
- * 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 @@
     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 @@
             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 @@
             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;
 }

Modified: trunk/src/lib/krb5/krb/get_in_tkt.c
===================================================================
--- trunk/src/lib/krb5/krb/get_in_tkt.c	2011-11-21 17:30:41 UTC (rev 25482)
+++ trunk/src/lib/krb5/krb/get_in_tkt.c	2011-11-21 21:14:39 UTC (rev 25483)
@@ -526,6 +526,7 @@
     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 @@
         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 @@
                                             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 @@
                       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 @@
 
     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 @@
     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);

Modified: trunk/src/lib/krb5/krb/init_creds_ctx.h
===================================================================
--- trunk/src/lib/krb5/krb/init_creds_ctx.h	2011-11-21 17:30:41 UTC (rev 25482)
+++ trunk/src/lib/krb5/krb/init_creds_ctx.h	2011-11-21 21:14:39 UTC (rev 25483)
@@ -18,6 +18,7 @@
     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;

Modified: trunk/src/lib/krb5/krb/preauth2.c
===================================================================
--- trunk/src/lib/krb5/krb/preauth2.c	2011-11-21 17:30:41 UTC (rev 25482)
+++ trunk/src/lib/krb5/krb/preauth2.c	2011-11-21 21:14:39 UTC (rev 25483)
@@ -1370,6 +1370,7 @@
                          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 @@
                                            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) {

Modified: trunk/src/plugins/preauth/pkinit/pkinit_clnt.c
===================================================================
--- trunk/src/plugins/preauth/pkinit/pkinit_clnt.c	2011-11-21 17:30:41 UTC (rev 25482)
+++ trunk/src/plugins/preauth/pkinit/pkinit_clnt.c	2011-11-21 21:14:39 UTC (rev 25483)
@@ -93,7 +93,7 @@
                   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 @@
     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 @@
 
     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 @@
             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,86 +1110,77 @@
                        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;
+    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_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;
-        }
-        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 (certifiers != NULL)
+        free_krb5_external_principal_identifier(&certifiers);
 
-    if (typed_data != NULL)
-        free_krb5_typed_data(&typed_data);
-
     if (algId != NULL)
         free_krb5_algorithm_identifiers(&algId);
 




More information about the cvs-krb5 mailing list