[382] in Kerberos-V5-bugs
bug in kdc_process_tgs_req (sort of)
daemon@ATHENA.MIT.EDU (Jim Miller)
Fri Oct 22 21:53:46 1993
From: jim@bilbo.suite.com (Jim Miller)
Date: Fri, 22 Oct 93 20:39:42 -0500
To: krb5-bugs@MIT.EDU
Cc: kerberos@MIT.EDU
Reply-To: Jim_Miller@suite.com
This is for Kerberos 5, pre-beta 3
This bug will take some explaining.
krb5kdc blows up inside of "prepare_error_tgs"(kdc/do_tgs_req.c) on the
following line:
errpkt.client = ticket ? ticket->enc_part2->client : 0; /* may not know
the name */
because "ticket->enc_part2" is NULL.
The reason it is null is what takes long to explain.
Let's back up a bit to see how we got into "prepare_error_tgs"...
In this particular case, "prepare_error_tgs" was called by "process_tgs_req".
Now "process_tgs_req" called it because "kdc_process_tgs_req" (kdc/kdc_util.c)
returned with an error after the point in which the parameter "req_authdat" was
set. Here's the "process_tgs_req" code I'm refering to:
errcode = kdc_process_tgs_req(request, from, pkt, &req_authdat);
if (req_authdat)
header_ticket = req_authdat->ticket;
if (errcode)
goto cleanup;
In other words, "kdc_process_tgs_req" returned an error (thus we'll goto
cleanup) AND "req_authdat" had been set (thus header_ticket gets set pointing
to req_authdat->ticket)
Ok, let's descend into "kdc_process_tgs_req" to see why it returned an error...
First, note that if "kdc_process_tgs_req" successfully decodes the imbedded
apreq, then "req_authdat" gets set: (NOTE: req_authdat == *ret_authdat)
if (retval = decode_krb5_ap_req(&scratch2, &apreq))
return retval;
if (!(authdat = (krb5_tkt_authent *)malloc(sizeof(*authdat)))) {
retval = ENOMEM;
goto cleanup;
}
memset((char *)authdat, 0, sizeof(*authdat));
authdat->ticket = apreq->ticket;
*ret_authdat = authdat;
Moving along, we eventually get to a call to "krb5_rd_req_decoded". Assume
that "krb5_rd_req_decoded" returns an error. (In the case where I discovered
this bus, "krb5_rd_req_decoded" returned with KRB5KRB_AP_ERR_BADADDR.) When
"krb5_rd_req_decoded" returns with an error, we goto cleanup:
retval = krb5_rd_req_decoded(apreq, apreq->ticket->server,
from->address,
0, /* no fetchfrom */
kdc_rdreq_keyproc,
(krb5_pointer)&who,
kdc_rcache,
&nauthdat);
krb5_free_keyblock(who.key);
if (retval) {
apreq->ticket = 0; /* Caller will free the ticket */
goto cleanup;
}
.
.
.
cleanup:
if (apreq)
krb5_free_ap_req(apreq);
if (scratch)
krb5_free_data(scratch);
return retval;
}
The code indicates that we don't want "cleanup" to free the ticket that was in
"apreq". The intent is to return to the caller with "*ret_authdat" still
pointing to a decoded and decrypted ticket that can later be used to form an
error message in "prepare_error_tgs".
However...
Looking inside of "krb5_rd_req_decoded" we see the following:
if (sender_addr && !krb5_address_search(sender_addr,
req->ticket->enc_part2->caddrs)) {
retval = KRB5KRB_AP_ERR_BADADDR;
goto cleanup;
}
.
.
.
cleanup:
if (tktauthent) {
if (retval) {
krb5_free_tkt_authent(tktauthent);
} else {
tktauthent->ap_options = req->ap_options;
*authdat = tktauthent;
}
}
if (retval && req->ticket->enc_part2) {
/* only free if we're erroring out...otherwise some
applications will need the output. */
krb5_free_enc_tkt_part(req->ticket->enc_part2); <--- *********
req->ticket->enc_part2 = NULL; <--- *********
}
if (tkt_key)
krb5_free_keyblock(tkt_key);
return retval;
}
Uh Oh! The ticket we thought we were saving for "prepare_error_tgs" was
actually cleaned up a bit. In particular, the "enc_part2" portion is freed and
set to NULL! And that's why the following line in "prepare_error_tgs" blows
up:
errpkt.client = ticket ? ticket->enc_part2->client : 0; /* may not know
the name */
The easy way to fix this is to do the following:
errpkt.client = (ticket && ticket->enc_part2) ? ticket->enc_part2->client
: 0; /* may not know
the name */
However, that doesn't really fix the real problem, which is the incorrect
assumption about the contents of "req_authdat" upon return from
"kdc_process_tgs_req"
I don't have a suggested fix for that at this time. I'll have to look at it
some more.
Jim_Miller@suite.com
