Skip to content

Commit

Permalink
Simplify ticket retrieval from AP-REQs
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@greghudson
greghudson committed Jun 11, 2014
1 parent 4799121 commit 02de993
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 31 deletions.
48 changes: 22 additions & 26 deletions src/kdc/kdc_util.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 **,
Expand Down Expand Up @@ -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,
Expand All @@ -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;

Expand All @@ -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)) {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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)
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -348,6 +351,11 @@ kdc_process_tgs_req(kdc_realm_t *kdc_active_realm,
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;
Expand All @@ -363,26 +371,19 @@ kdc_process_tgs_req(kdc_realm_t *kdc_active_realm,
*
* 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
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 3 additions & 4 deletions src/lib/gssapi/krb5/accept_sec_context.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,7 @@ kg_accept_krb5(minor_status, context_handle,
major_status = GSS_S_FAILURE;
goto done;
}
ticket = request->ticket;

/* decode the message */

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -1234,7 +1233,7 @@ kg_accept_krb5(minor_status, context_handle,
(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;
Expand Down
7 changes: 6 additions & 1 deletion src/lib/krb5/krb/rd_req.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 02de993

Please sign in to comment.