commit 460b23c9eb6aa826a7c8ca32d1bb28db01820a17 Author: tlyu tlyu@dc483132-0cff-0310-8789-dd5450dbe970 Date: Fri Feb 12 20:28:39 2010 +0000

ticket: 6656 version_fixed: 1.8 status: resolved

pull up r23712, r23714 from trunk

------------------------------------------------------------------------ r23714 | ghudson | 2010-02-09 20:55:36 -0500 (Tue, 09 Feb 2010) | 13 lines

ticket: 6656

Followon fixes to r23712: * A few formatting fixes. * Fix unlikely leak in kdc_handle_protected_negotiation: if add_pa_data_element with copy == FALSE fails, it's still the caller's responsibility to free pa.contents. * Fix pre-existing (since r23465) leak of reply_encpart.enc_padata in process_as_req. * Call add_pa_data_element with copy == TRUE in return_referral_enc_padata since we are passing memory owned by the database entry.

------------------------------------------------------------------------ r23712 | hartmans | 2010-02-09 14:15:07 -0500 (Tue, 09 Feb 2010) | 14 lines

subject: enc_padata can include empty sequence ticket: 6656 target_version: 1.8 tags: pullup

There are two issues with return_enc_padata. 1) It often will return an empty sequence of enc_padata rather than not including the field 2) FAST negotiation is double supported in the referral tgs path and not supported in the non-referral path

Rewrite the return_enc_padata logic to:

* Split out referral interactions with kdb into its own function * Use add_pa_data_element

git-svn-id: svn://anonsvn.mit.edu/krb5/branches/krb5-1-8@23718 dc483132-0cff-0310-8789-dd5450dbe970

src/kdc/do_as_req.c | 6 +++- src/kdc/do_tgs_req.c | 31 ++++++----------------- src/kdc/kdc_preauth.c | 66 ++++++++++++++++++++++++++----------------------- src/kdc/kdc_util.c | 44 ++++++++++++-------------------- src/kdc/kdc_util.h | 5 ++- 5 files changed, 67 insertions(+), 85 deletions(-) --- diff --git a/src/kdc/do_as_req.c b/src/kdc/do_as_req.c index 83d3101..b183dcf 100644 --- a/src/kdc/do_as_req.c +++ b/src/kdc/do_as_req.c @@ -133,6 +133,7 @@ process_as_req(krb5_kdc_req *request, krb5_data *req_pkt, server_keyblock.contents = NULL; client_keyblock.contents = NULL; reply.padata = 0; + reply_encpart.enc_padata = 0; memset(&reply, 0, sizeof(reply));

session_key.contents = 0; @@ -623,7 +624,8 @@ process_as_req(krb5_kdc_req *request, krb5_data *req_pkt, goto errout; } errcode = return_enc_padata(kdc_context, req_pkt, request, - as_encrypting_key, &server, &reply_encpart); + as_encrypting_key, &server, &reply_encpart, + FALSE); if (errcode) { status = "KDC_RETURN_ENC_PADATA"; goto errout; @@ -689,6 +691,8 @@ egress: krb5_free_keyblock_contents(kdc_context, &client_keyblock); if (reply.padata != NULL) krb5_free_pa_data(kdc_context, reply.padata); + if (reply_encpart.enc_padata) + krb5_free_pa_data(kdc_context, reply_encpart.enc_padata);

if (cname != NULL) free(cname); diff --git a/src/kdc/do_tgs_req.c b/src/kdc/do_tgs_req.c index 26fde1e..cb0496f 100644 --- a/src/kdc/do_tgs_req.c +++ b/src/kdc/do_tgs_req.c @@ -948,29 +948,14 @@ tgt_again: status = "generating reply key"; goto cleanup; } - if (is_referral && isflagset(s_flags, KRB5_KDB_FLAG_CANONICALIZE)) { - int idx = 0; - - errcode = return_enc_padata(kdc_context, pkt, request, - reply_key, &server, &reply_encpart); - if (errcode) { - status = "KDC_RETURN_ENC_PADATA"; - goto cleanup; - } - /* Not referral. */ - reply_encpart.enc_padata = calloc(3, sizeof(krb5_pa_data *)); - if (reply_encpart.enc_padata == NULL) { - errcode = ENOMEM; - status = "Allocating enc_padata"; - goto cleanup; - } - errcode = kdc_handle_protected_negotiation(pkt, request, reply_key, - reply_encpart.enc_padata, - &idx); - if (errcode != 0) { - status = "protected negotiation"; - goto cleanup; - } + errcode = return_enc_padata(kdc_context, pkt, request, + reply_key, &server, &reply_encpart, + is_referral && + isflagset(s_flags, + KRB5_KDB_FLAG_CANONICALIZE)); + if (errcode) { + status = "KDC_RETURN_ENC_PADATA"; + goto cleanup; }

errcode = krb5_encode_kdc_rep(kdc_context, KRB5_TGS_REP, &reply_encpart, diff --git a/src/kdc/kdc_preauth.c b/src/kdc/kdc_preauth.c index c5dfb1f..00800aa 100644 --- a/src/kdc/kdc_preauth.c +++ b/src/kdc/kdc_preauth.c @@ -3084,48 +3084,52 @@ include_pac_p(krb5_context context, krb5_kdc_req *request) return retval; }

-krb5_error_code -return_enc_padata(krb5_context context, krb5_data *req_pkt, - krb5_kdc_req *request, krb5_keyblock *reply_key, - krb5_db_entry *server, krb5_enc_kdc_rep_part *reply_encpart) +static krb5_error_code +return_referral_enc_padata( krb5_context context, + krb5_enc_kdc_rep_part *reply, + krb5_db_entry *server) { krb5_error_code code; krb5_tl_data tl_data; - krb5_pa_data *pa_data; - int idx = 0; + krb5_pa_data pa_data;

- /* This should be initialized and only used for Win2K compat and other - * specific standardized uses such as FAST negotiation. */ - assert(reply_encpart->enc_padata == NULL); - reply_encpart->enc_padata = calloc(4, sizeof(krb5_pa_data *)); - if (reply_encpart->enc_padata == NULL) - return ENOMEM; tl_data.tl_data_type = KRB5_TL_SVR_REFERRAL_DATA; code = krb5_dbe_lookup_tl_data(context, server, &tl_data); if (code || tl_data.tl_data_length == 0) - goto negotiate; /* no server referrals to return */ - - pa_data = (krb5_pa_data *)malloc(sizeof(*pa_data)); - if (pa_data == NULL) - return ENOMEM; - pa_data->magic = KV5M_PA_DATA; - pa_data->pa_type = KRB5_PADATA_SVR_REFERRAL_INFO; - pa_data->length = tl_data.tl_data_length; - pa_data->contents = malloc(pa_data->length); - if (pa_data->contents == NULL) { - free(pa_data); - return ENOMEM; - } - memcpy(pa_data->contents, tl_data.tl_data_contents, tl_data.tl_data_length); + return 0;

+ pa_data.magic = KV5M_PA_DATA; + pa_data.pa_type = KRB5_PADATA_SVR_REFERRAL_INFO; + pa_data.length = tl_data.tl_data_length; + pa_data.contents = tl_data.tl_data_contents; + return add_pa_data_element(context, &pa_data, &reply->enc_padata, TRUE); +}

- reply_encpart->enc_padata[idx++] = pa_data; - reply_encpart->enc_padata[1] = NULL; -negotiate: - return kdc_handle_protected_negotiation(req_pkt, request, reply_key, - reply_encpart->enc_padata, &idx); +krb5_error_code +return_enc_padata(krb5_context context, krb5_data *req_pkt, + krb5_kdc_req *request, krb5_keyblock *reply_key, + krb5_db_entry *server, krb5_enc_kdc_rep_part *reply_encpart, + krb5_boolean is_referral) +{ + krb5_error_code code = 0; + /* This should be initialized and only used for Win2K compat and other + * specific standardized uses such as FAST negotiation. */ + assert(reply_encpart->enc_padata == NULL); + if (is_referral) { + code = return_referral_enc_padata(context, reply_encpart, server); + if (code) + return code; + } + code = kdc_handle_protected_negotiation(req_pkt, request, reply_key, + &reply_encpart->enc_padata); + if (code) + goto cleanup; + /*Add potentially other enc_padata providers*/ +cleanup: + return code; }

+ #if 0 static krb5_error_code return_server_referral(krb5_context context, krb5_pa_data * padata, diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c index 281bcc8..d63bba2 100644 --- a/src/kdc/kdc_util.c +++ b/src/kdc/kdc_util.c @@ -2675,23 +2675,18 @@ kdc_get_ticket_endtime(krb5_context context, krb5_error_code kdc_handle_protected_negotiation(krb5_data *req_pkt, krb5_kdc_req *request, const krb5_keyblock *reply_key, - krb5_pa_data **out_enc_padata, int *idx) + krb5_pa_data ***out_enc_padata) { krb5_error_code retval = 0; krb5_checksum checksum; krb5_data *out = NULL; - krb5_pa_data *pa; - assert(out_enc_padata != NULL); - pa = krb5int_find_pa_data(kdc_context, request->padata, + krb5_pa_data pa, *pa_in; + pa_in = krb5int_find_pa_data(kdc_context, request->padata, KRB5_ENCPADATA_REQ_ENC_PA_REP); - if (pa == NULL) + if (pa_in == NULL) return 0; - checksum.contents = NULL; - pa = malloc(sizeof(krb5_pa_data)); - if (pa == NULL) - return ENOMEM; - pa->magic = KV5M_PA_DATA; - pa->pa_type = KRB5_ENCPADATA_REQ_ENC_PA_REP; + pa.magic = KV5M_PA_DATA; + pa.pa_type = KRB5_ENCPADATA_REQ_ENC_PA_REP; retval = krb5_c_make_checksum(kdc_context,0, reply_key, KRB5_KEYUSAGE_AS_REQ, req_pkt, &checksum); if (retval != 0) @@ -2699,29 +2694,22 @@ kdc_handle_protected_negotiation(krb5_data *req_pkt, krb5_kdc_req *request, retval = encode_krb5_checksum(&checksum, &out); if (retval != 0) goto cleanup; - pa->contents = (krb5_octet *) out->data; - pa->length = out->length; - out_enc_padata[(*idx)++] = pa; - pa = NULL; - out->data = NULL; - pa = malloc(sizeof(krb5_pa_data)); - if (pa == NULL) { - retval = ENOMEM; + pa.contents = (krb5_octet *) out->data; + pa.length = out->length; + retval = add_pa_data_element(kdc_context, &pa, out_enc_padata, FALSE); + if (retval) goto cleanup; - } - pa->magic = KV5M_PA_DATA; - pa->pa_type = KRB5_PADATA_FX_FAST; - pa->length = 0; - pa->contents = NULL; - out_enc_padata[(*idx)++] = pa; - pa = NULL; + out->data = NULL; + pa.magic = KV5M_PA_DATA; + pa.pa_type = KRB5_PADATA_FX_FAST; + pa.length = 0; + pa.contents = NULL; + retval = add_pa_data_element(kdc_context, &pa, out_enc_padata, FALSE); cleanup: if (checksum.contents) krb5_free_checksum_contents(kdc_context, &checksum); if (out != NULL) krb5_free_data(kdc_context, out); - if (pa != NULL) - free(pa); return retval; }

diff --git a/src/kdc/kdc_util.h b/src/kdc/kdc_util.h index 2583895..03ecaf7 100644 --- a/src/kdc/kdc_util.h +++ b/src/kdc/kdc_util.h @@ -258,7 +258,8 @@ return_enc_padata(krb5_context context, krb5_data *req_pkt, krb5_kdc_req *request, krb5_keyblock *reply_key, krb5_db_entry *server, - krb5_enc_kdc_rep_part *reply_encpart); + krb5_enc_kdc_rep_part *reply_encpart, + krb5_boolean is_referral);

krb5_error_code sign_db_authdata (krb5_context context, @@ -401,7 +402,7 @@ krb5_error_code kdc_preauth_get_cookie(struct kdc_request_state *state, krb5_error_code kdc_handle_protected_negotiation( krb5_data *req_pkt, krb5_kdc_req *request, const krb5_keyblock *reply_key, - krb5_pa_data **out_enc_padata, int *idx); + krb5_pa_data ***out_enc_padata); krb5_error_code krb5int_get_domain_realm_mapping(krb5_context context, const char *host, char ***realmsp);