From 02de9935648c307098fb69da26f74424da8dde64 Mon Sep 17 00:00:00 2001 From: Greg Hudson Date: Thu, 5 Jun 2014 12:03:16 -0400 Subject: Simplify ticket retrieval from AP-REQs After krb5_rd_req_decoded or krb5_rd_req_decoded_anyflag, the ticket (with enc_part2 if we could decrypt it) is accessible via request->ticket; there is no need to copy it. Stop using the ticket parameter of those functions. Where we need to save the ticket beyond the lifetime of the krb5_ap_req, steal the pointer before freeing the request. --- src/kdc/kdc_util.c | 48 +++++++++++++++----------------- src/lib/gssapi/krb5/accept_sec_context.c | 7 ++--- src/lib/krb5/krb/rd_req.c | 7 ++++- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c index cd276e4a9..48be1ae2c 100644 --- a/src/kdc/kdc_util.c +++ b/src/kdc/kdc_util.c @@ -72,8 +72,7 @@ static krb5_error_code kdc_rd_ap_req(kdc_realm_t *kdc_active_realm, krb5_ap_req *apreq, krb5_auth_context auth_context, krb5_db_entry **server, - krb5_keyblock **tgskey, - krb5_ticket **ticket); + krb5_keyblock **tgskey); static krb5_error_code find_server_key(krb5_context, krb5_db_entry *, krb5_enctype, krb5_kvno, krb5_keyblock **, @@ -194,10 +193,11 @@ comp_cksum(krb5_context kcontext, krb5_data *source, krb5_ticket *ticket, return(0); } +/* If a header ticket is decrypted, *ticket_out is filled in even on error. */ krb5_error_code kdc_process_tgs_req(kdc_realm_t *kdc_active_realm, krb5_kdc_req *request, const krb5_fulladdr *from, - krb5_data *pkt, krb5_ticket **ticket, + krb5_data *pkt, krb5_ticket **ticket_out, krb5_db_entry **krbtgt_ptr, krb5_keyblock **tgskey, krb5_keyblock **subkey, @@ -214,7 +214,9 @@ kdc_process_tgs_req(kdc_realm_t *kdc_active_realm, krb5_authenticator * authenticator = NULL; krb5_checksum * his_cksum = NULL; krb5_db_entry * krbtgt = NULL; + krb5_ticket * ticket; + *ticket_out = NULL; *krbtgt_ptr = NULL; *tgskey = NULL; @@ -227,6 +229,7 @@ kdc_process_tgs_req(kdc_realm_t *kdc_active_realm, scratch1.data = (char *)tmppa->contents; if ((retval = decode_krb5_ap_req(&scratch1, &apreq))) return retval; + ticket = apreq->ticket; if (isflagset(apreq->ap_options, AP_OPTS_USE_SESSION_KEY) || isflagset(apreq->ap_options, AP_OPTS_MUTUAL_REQUIRED)) { @@ -260,13 +263,13 @@ kdc_process_tgs_req(kdc_realm_t *kdc_active_realm, goto cleanup_auth_context; retval = kdc_rd_ap_req(kdc_active_realm, - apreq, auth_context, &krbtgt, tgskey, ticket); + apreq, auth_context, &krbtgt, tgskey); if (retval) goto cleanup_auth_context; /* "invalid flag" tickets can must be used to validate */ - if (isflagset((*ticket)->enc_part2->flags, TKT_FLG_INVALID) - && !isflagset(request->kdc_options, KDC_OPT_VALIDATE)) { + if (isflagset(ticket->enc_part2->flags, TKT_FLG_INVALID) && + !isflagset(request->kdc_options, KDC_OPT_VALIDATE)) { retval = KRB5KRB_AP_ERR_TKT_INVALID; goto cleanup_auth_context; } @@ -280,7 +283,7 @@ kdc_process_tgs_req(kdc_realm_t *kdc_active_realm, goto cleanup_auth_context; retval = krb5_find_authdata(kdc_context, - (*ticket)->enc_part2->authorization_data, + ticket->enc_part2->authorization_data, authenticator->authorization_data, KRB5_AUTHDATA_FX_ARMOR, &authdata); if (retval != 0) @@ -306,7 +309,7 @@ kdc_process_tgs_req(kdc_realm_t *kdc_active_realm, !krb5int_find_pa_data(kdc_context, request->padata, KRB5_PADATA_FOR_USER)) { if (is_local_principal(kdc_active_realm, - (*ticket)->enc_part2->client)) { + ticket->enc_part2->client)) { /* someone in a foreign realm claiming to be local */ krb5_klog_syslog(LOG_INFO, _("PROCESS_TGS: failed lineage check")); retval = KRB5KDC_ERR_POLICY; @@ -324,9 +327,9 @@ kdc_process_tgs_req(kdc_realm_t *kdc_active_realm, */ if (pkt && (fetch_asn1_field((unsigned char *) pkt->data, 1, 4, &scratch1) >= 0)) { - if (comp_cksum(kdc_context, &scratch1, *ticket, his_cksum)) { + if (comp_cksum(kdc_context, &scratch1, ticket, his_cksum)) { if (!(retval = encode_krb5_kdc_req_body(request, &scratch))) - retval = comp_cksum(kdc_context, scratch, *ticket, his_cksum); + retval = comp_cksum(kdc_context, scratch, ticket, his_cksum); krb5_free_data(kdc_context, scratch); if (retval) goto cleanup_authenticator; @@ -348,6 +351,11 @@ cleanup: krb5_free_keyblock(kdc_context, *tgskey); *tgskey = NULL; } + if (apreq->ticket->enc_part2 != NULL) { + /* Steal the decrypted ticket pointer, even on error. */ + *ticket_out = apreq->ticket; + apreq->ticket = NULL; + } krb5_free_ap_req(kdc_context, apreq); krb5_db_free_principal(kdc_context, krbtgt); return retval; @@ -363,26 +371,19 @@ cleanup: * * This function also implements key rollover support for kvno 0 cross-realm * TGTs issued by AD. - * - * If the ticket was successfully decrypted, it will be returned in *ticket - * even if we return an error because the ticket was invalid (e.g. if it was - * expired). */ static krb5_error_code kdc_rd_ap_req(kdc_realm_t *kdc_active_realm, krb5_ap_req *apreq, krb5_auth_context auth_context, - krb5_db_entry **server, krb5_keyblock **tgskey, - krb5_ticket **ticket) + krb5_db_entry **server, krb5_keyblock **tgskey) { - krb5_error_code retval, ret2; + krb5_error_code retval; krb5_enctype search_enctype = apreq->ticket->enc_part.enctype; krb5_boolean match_enctype = 1; krb5_kvno kvno; size_t tries = 3; - *ticket = NULL; - /* * When we issue tickets we use the first key in the principals' highest * kvno keyset. For non-cross-realm krbtgt principals we want to only @@ -421,14 +422,9 @@ kdc_rd_ap_req(kdc_realm_t *kdc_active_realm, kdc_active_realm->realm_keytab, NULL, NULL); - /* If the ticket was decrypted, save it even if it didn't validate, and - * don't try any more keys. */ - if (apreq->ticket->enc_part2 != NULL) { - ret2 = krb5_copy_ticket(kdc_context, apreq->ticket, ticket); - if (!retval) - retval = ret2; + /* If the ticket was decrypted, don't try any more keys. */ + if (apreq->ticket->enc_part2 != NULL) break; - } } while (retval && apreq->ticket->enc_part.kvno == 0 && kvno-- > 1 && --tries > 0); diff --git a/src/lib/gssapi/krb5/accept_sec_context.c b/src/lib/gssapi/krb5/accept_sec_context.c index af7f0dcd5..b8086509e 100644 --- a/src/lib/gssapi/krb5/accept_sec_context.c +++ b/src/lib/gssapi/krb5/accept_sec_context.c @@ -607,6 +607,7 @@ kg_accept_krb5(minor_status, context_handle, major_status = GSS_S_FAILURE; goto done; } + ticket = request->ticket; /* decode the message */ @@ -644,7 +645,7 @@ kg_accept_krb5(minor_status, context_handle, } code = krb5_rd_req_decoded(context, &auth_context, request, accprinc, - cred->keytab, &ap_req_options, &ticket); + cred->keytab, &ap_req_options, NULL); krb5_free_principal(context, accprinc); if (code) { @@ -968,8 +969,6 @@ kg_accept_krb5(minor_status, context_handle, ctx->gss_flags |= GSS_C_DELEG_FLAG; } - krb5_free_ticket(context, ticket); /* Done with ticket */ - { krb5_int32 seq_temp; krb5_auth_con_getremoteseqnumber(context, auth_context, &seq_temp); @@ -1234,7 +1233,7 @@ fail: (void) krb5_us_timeofday(context, &krb_error_data.stime, &krb_error_data.susec); - krb_error_data.server = request->ticket->server; + krb_error_data.server = ticket->server; code = krb5_mk_error(context, &krb_error_data, &scratch); if (code) goto done; diff --git a/src/lib/krb5/krb/rd_req.c b/src/lib/krb5/krb/rd_req.c index 5ad77c106..c0fc97932 100644 --- a/src/lib/krb5/krb/rd_req.c +++ b/src/lib/krb5/krb/rd_req.c @@ -85,7 +85,12 @@ krb5_rd_req(krb5_context context, krb5_auth_context *auth_context, #endif /* LEAN_CLIENT */ retval = krb5_rd_req_decoded(context, auth_context, request, server, - keytab, ap_req_options, ticket); + keytab, ap_req_options, NULL); + if (!retval && ticket != NULL) { + /* Steal the ticket pointer for the caller. */ + *ticket = request->ticket; + request->ticket = NULL; + } #ifndef LEAN_CLIENT if (new_keytab != NULL) -- cgit