From b87d9d3c376c2623ae9eb0cfc8da50985c7bb592 Mon Sep 17 00:00:00 2001 From: Greg Hudson Date: Mon, 14 Nov 2011 21:45:33 +0000 Subject: [PATCH] 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 --- src/include/krb5/preauth_plugin.h | 13 +++++------- src/kdc/do_as_req.c | 16 +++++++++++++-- src/kdc/do_tgs_req.c | 2 +- src/kdc/fast_util.c | 20 +++++++++++++++++-- src/kdc/kdc_preauth.c | 7 +++---- src/kdc/kdc_util.h | 12 ++++++++++- .../preauth/cksum_body/cksum_body_main.c | 13 +----------- 7 files changed, 53 insertions(+), 30 deletions(-) diff --git a/src/include/krb5/preauth_plugin.h b/src/include/krb5/preauth_plugin.h index 869ebd5cd..cbc57f147 100644 --- 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 0d5cbe515..a9f31fbea 100644 --- 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 2ed73349f..0f001d67c 100644 --- 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 96c8c1394..f3e037d53 100644 --- 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 c106027c6..4c5ef88de 100644 --- 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 f4773ae4c..9e123ee8e 100644 --- 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 c0a438f75..ed2b5b4e0 100644 --- 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. */ -- 2.26.2