Simplify and fix kdcpreauth request_body callback

Alter the contract for the kdcpreauth request_body callback so that it
returns an alias to the encoded body instead of a fresh copy.  At the
beginning of AS request processing, save a copy of the encoded request
body, or the encoded inner request body for FAST requests.  Previously
the request_body callback would re-encode the request structure, which
in some cases has been modified by the AS request code.

No kdcpreauth modules currently use the request_body callback, but
PKINIT will need to start using it in order to handle FAST requests
correctly.

ticket: 7017
target_version: 1.10
tags: pullup

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@25473 dc483132-0cff-0310-8789-dd5450dbe970

diff --git a/src/include/krb5/preauth_plugin.h b/src/include/krb5/preauth_plugin.h
index 869ebd5cd49de4bbe97ca82a59d8c7dd349ad15c..cbc57f147bd509c661dfa221b668c0f74f4f0623 100644 (file)
--- a/src/include/krb5/preauth_plugin.h
+++ b/src/include/krb5/preauth_plugin.h
@@ -351,15 +351,12 @@ typedef struct krb5_kdcpreauth_callbacks_st {
                       krb5_keyblock *keys);
 
     /*
-     * Get the request structure, re-encoded using DER.  Unless the client
-     * implementation is the same as the server implementation, there's a good
-     * chance that the result will not match what the client sent, so don't
-     * create any fatal errors if it doesn't match up.  Free the resulting data
-     * object with krb5_free_data.
+     * Get the encoded request body, which is sometimes needed for checksums.
+     * For a FAST request this is the encoded inner request body.  The returned
+     * pointer is an alias and should not be freed.
      */
-    krb5_error_code (*request_body)(krb5_context context,
-                                    krb5_kdcpreauth_rock rock,
-                                    krb5_data **body_out);
+    krb5_data *(*request_body)(krb5_context context,
+                               krb5_kdcpreauth_rock rock);
 
     /* Get a pointer to the FAST armor key, or NULL if the request did not use
      * FAST.  The returned pointer is an alias and should not be freed. */

diff --git a/src/kdc/do_as_req.c b/src/kdc/do_as_req.c
index 0d5cbe51587f9b0aa92e81c66bc035d83bfa4fdc..a9f31fbea3ca96bba43a53ef833bf0e73c05d6c2 100644 (file)
--- a/src/kdc/do_as_req.c
+++ b/src/kdc/do_as_req.c
@@ -120,6 +120,7 @@ struct as_req_state {
     krb5_keyblock session_key;
     unsigned int c_flags;
     krb5_data *req_pkt;
+    krb5_data *inner_body;
     struct kdc_request_state *rstate;
     char *sname, *cname;
     void *pa_context;
@@ -396,6 +397,7 @@ egress:
     }
 
     krb5_free_pa_data(kdc_context, state->e_data);
+    krb5_free_data(kdc_context, state->inner_body);
     kdc_free_rstate(state->rstate);
     krb5_free_kdc_req(kdc_context, state->request);
     assert(did_log != 0);
@@ -492,13 +494,23 @@ process_as_req(krb5_kdc_req *request, krb5_data *req_pkt,
         state->status = "Finding req_body";
         goto errout;
     }
-    errcode = kdc_find_fast(&state->request, &encoded_req_body,
-                            NULL /*TGS key*/, NULL, state->rstate);
+    errcode = kdc_find_fast(&state->request, &encoded_req_body, NULL, NULL,
+                            state->rstate, &state->inner_body);
     if (errcode) {
         state->status = "error decoding FAST";
         goto errout;
     }
+    if (state->inner_body == NULL) {
+        /* Not a FAST request; copy the encoded request body. */
+        errcode = krb5_copy_data(kdc_context, &encoded_req_body,
+                                 &state->inner_body);
+        if (errcode) {
+            state->status = "storing req body";
+            goto errout;
+        }
+    }
     state->rock.request = state->request;
+    state->rock.inner_body = state->inner_body;
     state->rock.rstate = state->rstate;
     if (!state->request->client) {
         state->status = "NULL_CLIENT";

diff --git a/src/kdc/do_tgs_req.c b/src/kdc/do_tgs_req.c
index 2ed73349f284253ae218e2cd2757488ac647c965..0f001d67c328cacc27fff8aeb85d85aec1608f91 100644 (file)
--- a/src/kdc/do_tgs_req.c
+++ b/src/kdc/do_tgs_req.c
@@ -176,7 +176,7 @@ process_tgs_req(krb5_data *pkt, const krb5_fulladdr *from,
     scratch.length = pa_tgs_req->length;
     scratch.data = (char *) pa_tgs_req->contents;
     errcode = kdc_find_fast(&request, &scratch, subkey,
-                            header_ticket->enc_part2->session, state);
+                            header_ticket->enc_part2->session, state, NULL);
     if (errcode !=0) {
         status = "kdc_find_fast";
         goto cleanup;

diff --git a/src/kdc/fast_util.c b/src/kdc/fast_util.c
index 96c8c1394a9bb449391975253027640383afb5f5..f3e037d5351d79b2cd33f18d9b28a06a9473cd9f 100644 (file)
--- a/src/kdc/fast_util.c
+++ b/src/kdc/fast_util.c
@@ -126,11 +126,12 @@ kdc_find_fast(krb5_kdc_req **requestptr,
               krb5_data *checksummed_data,
               krb5_keyblock *tgs_subkey,
               krb5_keyblock *tgs_session,
-              struct kdc_request_state *state)
+              struct kdc_request_state *state,
+              krb5_data **inner_body_out)
 {
     krb5_error_code retval = 0;
     krb5_pa_data *fast_padata, *cookie_padata = NULL;
-    krb5_data scratch;
+    krb5_data scratch, *inner_body = NULL;
     krb5_fast_req * fast_req = NULL;
     krb5_kdc_req *request = *requestptr;
     krb5_fast_armored_req *fast_armored_req = NULL;
@@ -138,6 +139,8 @@ kdc_find_fast(krb5_kdc_req **requestptr,
     krb5_boolean cksum_valid;
     krb5_keyblock empty_keyblock;
 
+    if (inner_body_out != NULL)
+        *inner_body_out = NULL;
     scratch.data = NULL;
     krb5_clear_error_message(kdc_context);
     memset(&empty_keyblock, 0, sizeof(krb5_keyblock));
@@ -192,6 +195,14 @@ kdc_find_fast(krb5_kdc_req **requestptr,
                                     &plaintext);
             if (retval == 0)
                 retval = decode_krb5_fast_req(&plaintext, &fast_req);
+            if (retval == 0 && inner_body_out != NULL) {
+                retval = fetch_asn1_field((unsigned char *)plaintext.data,
+                                          1, 2, &scratch);
+                if (retval == 0) {
+                    retval = krb5_copy_data(kdc_context, &scratch,
+                                            &inner_body);
+                }
+            }
             if (plaintext.data)
                 free(plaintext.data);
         }
@@ -247,6 +258,11 @@ kdc_find_fast(krb5_kdc_req **requestptr,
             }
         }
     }
+    if (retval == 0 && inner_body_out != NULL) {
+        *inner_body_out = inner_body;
+        inner_body = NULL;
+    }
+    krb5_free_data(kdc_context, inner_body);
     if (fast_req)
         krb5_free_fast_req( kdc_context, fast_req);
     if (fast_armored_req)

diff --git a/src/kdc/kdc_preauth.c b/src/kdc/kdc_preauth.c
index c106027c6b397476c3f20f209b1db66775a397e0..4c5ef88de8ff5c177fec19785607da1b01d0b4b5 100644 (file)
--- a/src/kdc/kdc_preauth.c
+++ b/src/kdc/kdc_preauth.c
@@ -543,11 +543,10 @@ static void free_keys(krb5_context context, krb5_kdcpreauth_rock rock,
     free(keys);
 }
 
-static krb5_error_code
-request_body(krb5_context context, krb5_kdcpreauth_rock rock,
-             krb5_data **body_out)
+static krb5_data *
+request_body(krb5_context context, krb5_kdcpreauth_rock rock)
 {
-    return encode_krb5_kdc_req_body(rock->request, body_out);
+    return rock->inner_body;
 }
 
 static krb5_keyblock *

diff --git a/src/kdc/kdc_util.h b/src/kdc/kdc_util.h
index f4773ae4c4cc533fdd77c3de02f80e9cfcbf2ae9..9e123ee8e90bb54812ff8bb103cf9758219d277f 100644 (file)
--- a/src/kdc/kdc_util.h
+++ b/src/kdc/kdc_util.h
@@ -348,10 +348,19 @@ enum krb5_fast_kdc_flags {
     KRB5_FAST_REPLY_KEY_REPLACED = 0x02
 };
 
+/*
+ * If *requestptr contains FX_FAST padata, compute the armor key, verify the
+ * checksum over checksummed_data, decode the FAST request, and substitute
+ * *requestptr with the inner request.  Set the armor_key, cookie, and
+ * fast_options fields in state.  state->cookie will be set for a non-FAST
+ * request if it contains FX_COOKIE padata.  If inner_body_out is non-NULL, set
+ * *inner_body_out to a copy of the encoded inner body, or to NULL if the
+ * request is not a FAST request.
+ */
 krb5_error_code
 kdc_find_fast (krb5_kdc_req **requestptr,  krb5_data *checksummed_data,
                krb5_keyblock *tgs_subkey, krb5_keyblock *tgs_session,
-               struct kdc_request_state *state);
+               struct kdc_request_state *state, krb5_data **inner_body_out);
 
 krb5_error_code
 kdc_fast_response_handle_padata (struct kdc_request_state *state,
@@ -383,6 +392,7 @@ krb5int_get_domain_realm_mapping(krb5_context context,
 /* Information handle for kdcpreauth callbacks.  All pointers are aliases. */
 struct krb5_kdcpreauth_rock_st {
     krb5_kdc_req *request;
+    krb5_data *inner_body;
     krb5_db_entry *client;
     krb5_key_data *client_key;
     struct kdc_request_state *rstate;

diff --git a/src/plugins/preauth/cksum_body/cksum_body_main.c b/src/plugins/preauth/cksum_body/cksum_body_main.c
index c0a438f7585cdff224898e002db0c281fdf1da03..ed2b5b4e0106c581485232f8782bdfc998c17d38 100644 (file)
--- a/src/plugins/preauth/cksum_body/cksum_body_main.c
+++ b/src/plugins/preauth/cksum_body/cksum_body_main.c
@@ -403,17 +403,7 @@ server_verify(krb5_context kcontext,
     }
     cb->free_keys(kcontext, rock, keys);
 
-    /* Rebuild a copy of the client's request-body.  If we were serious
-     * about doing this with any chance of working interoperability, we'd
-     * extract the structure directly from the req_pkt structure.  This
-     * will probably work if it's us on both ends, though. */
-    req_body = NULL;
-    if (cb->request_body(kcontext, rock, &req_body) != 0) {
-        krb5_free_keyblock(kcontext, key);
-        stats->failures++;
-        (*respond)(arg, KRB5KDC_ERR_PREAUTH_FAILED, NULL, NULL, NULL);
-        return;
-    }
+    req_body = cb->request_body(kcontext, rock);
 
 #ifdef DEBUG
     fprintf(stderr, "AS key type %d, checksum type %d, %d bytes.\n",
@@ -428,7 +418,6 @@ server_verify(krb5_context kcontext,
                                     req_body, &checksum, &valid);
 
     /* Clean up. */
-    krb5_free_data(kcontext, req_body);
     krb5_free_keyblock(kcontext, key);
 
     /* Evaluate our results. */