kdc: remove outpadata from astgs_request_t

Remove the outpadata field from astgs_request_t, because it's not something we
wish to expose publically (yet it is something that Samba needs in the
client_access plugin API, to add Windows error information).

Instead, allocate rep->padata at the start of AS/TGS request handling, and
ensure it is valid for the lifetime of the request until it is encoded (at
which point it will be freed and set to NULL if zero length, to avoid sending a
zero length METHOD-DATA to the client).

(The previous approach of setting rep->padata to point to &r->outpadata was
fragile, because it required clearing the pointer before freeing the KDC-REP.)
This commit is contained in:
Luke Howard
2021-12-24 10:25:11 +11:00
committed by Nico Williams
parent 64dad876a4
commit b6be850e0d
5 changed files with 35 additions and 32 deletions

View File

@@ -816,12 +816,12 @@ _kdc_gss_mk_pa_reply(astgs_request_t r,
/* only return padata in error case if we have an error token */
if (!GSS_ERROR(gcp->major) || gcp->output_token.length) {
ret = krb5_padata_add(r->context, &r->outpadata, KRB5_PADATA_GSS,
ret = krb5_padata_add(r->context, r->rep.padata, KRB5_PADATA_GSS,
gcp->output_token.value, gcp->output_token.length);
if (ret)
return ret;
/* token is now owned by outpadata */
/* token is now owned by r->rep.padata */
gcp->output_token.length = 0;
gcp->output_token.value = NULL;
}

View File

@@ -70,8 +70,6 @@ struct kdc_patypes;
struct astgs_request_desc {
ASTGS_REQUEST_DESC_COMMON_ELEMENTS;
METHOD_DATA outpadata;
/* Only AS */
const struct kdc_patypes *pa_used;
struct as_request_pa_state *pa_state;

View File

@@ -767,12 +767,12 @@ pa_enc_chal_validate(astgs_request_t r,
ret = _krb5_make_pa_enc_challenge(r->context, challengecrypto,
KRB5_KU_ENC_CHALLENGE_KDC,
&r->outpadata);
r->rep.padata);
krb5_crypto_destroy(r->context, challengecrypto);
if (ret)
goto out;
set_salt_padata(&r->outpadata, k->salt);
set_salt_padata(r->rep.padata, k->salt);
/*
* Success
@@ -949,7 +949,7 @@ pa_enc_ts_validate(astgs_request_t r,
}
free_PA_ENC_TS_ENC(&p);
set_salt_padata(&r->outpadata, pa_key->salt);
set_salt_padata(r->rep.padata, pa_key->salt);
ret = krb5_copy_keyblock_contents(r->context, &pa_key->key, &r->reply_key);
if (ret)
@@ -1098,6 +1098,8 @@ _kdc_encode_reply(krb5_context context,
EncTicketPart *et = &r->et;
EncKDCRepPart *ek = &r->ek;
heim_assert(rep->padata != NULL, "reply padata uninitialized");
ASN1_MALLOC_ENCODE(EncTicketPart, buf, buf_size, et, &len, ret);
if(ret) {
const char *msg = krb5_get_error_message(context, ret);
@@ -1169,8 +1171,7 @@ _kdc_encode_reply(krb5_context context,
if (ret)
return ret;
free_METHOD_DATA(&r->outpadata);
rep->padata = &r->outpadata;
free_METHOD_DATA(r->rep.padata);
ret = krb5_padata_add(context, rep->padata,
KRB5_PADATA_FX_FAST,
@@ -1195,6 +1196,12 @@ _kdc_encode_reply(krb5_context context,
}
}
if (rep->padata->len == 0) {
free_METHOD_DATA(rep->padata);
free(rep->padata);
rep->padata = NULL;
}
if(rep->msg_type == krb_as_rep && !config->encode_as_rep_as_tgs_rep)
ASN1_MALLOC_ENCODE(EncASRepPart, buf, buf_size, ek, &len, ret);
else
@@ -2095,7 +2102,12 @@ _kdc_as_rep(astgs_request_t r)
memset(rep, 0, sizeof(*rep));
rep->padata = &r->outpadata; /* so we don't need to make this public */
ALLOC(rep->padata);
if (rep->padata == NULL) {
ret = ENOMEM;
krb5_set_error_message(r->context, ret, N_("malloc: out of memory", ""));
goto out;
}
/*
* Look for FAST armor and unwrap
@@ -2179,7 +2191,7 @@ _kdc_as_rep(astgs_request_t r)
r->cname, fixed_client_name);
free(fixed_client_name);
ret = _kdc_fast_mk_error(r, &r->outpadata, r->armor_crypto,
ret = _kdc_fast_mk_error(r, r->rep.padata, r->armor_crypto,
&req->req_body, KRB5_KDC_ERR_WRONG_REALM,
NULL,
r->client->entry.principal, r->server_princ,
@@ -2285,7 +2297,7 @@ _kdc_as_rep(astgs_request_t r)
NULL, &ckey, &default_salt);
if (ret2 == 0) {
ret2 = get_pa_etype_info_both(r->context, config, &b->etype,
&r->outpadata, ckey, !default_salt);
r->rep.padata, ckey, !default_salt);
if (ret2 != 0)
ret = ret2;
}
@@ -2331,7 +2343,7 @@ _kdc_as_rep(astgs_request_t r)
continue;
}
ret = krb5_padata_add(r->context, &r->outpadata,
ret = krb5_padata_add(r->context, r->rep.padata,
pat[n].type, NULL, 0);
if (ret)
goto out;
@@ -2345,7 +2357,7 @@ _kdc_as_rep(astgs_request_t r)
NULL, &ckey, &default_salt);
if (ret == 0) {
ret = get_pa_etype_info_both(r->context, config, &b->etype,
&r->outpadata, ckey, !default_salt);
r->rep.padata, ckey, !default_salt);
if (ret)
goto out;
}
@@ -2435,7 +2447,6 @@ _kdc_as_rep(astgs_request_t r)
/*
* Build reply
*/
rep->pvno = 5;
rep->msg_type = krb_as_rep;
@@ -2680,9 +2691,6 @@ _kdc_as_rep(astgs_request_t r)
if (ret)
goto out;
if (r->outpadata.len == 0)
rep->padata = NULL;
/* Add the PAC */
if (!r->et.flags.anonymous) {
generate_pac(r, skey, krbtgt_key, is_tgs);
@@ -2761,15 +2769,12 @@ _kdc_as_rep(astgs_request_t r)
}
out:
r->rep.padata = NULL; /* may point to outpadata */
free_AS_REP(&r->rep);
/*
* In case of a non proxy error, build an error message.
*/
if (ret != 0 && ret != HDB_ERR_NOT_FOUND_HERE && r->reply->length == 0)
ret = _kdc_fast_mk_error(r,
&r->outpadata,
r->rep.padata,
r->armor_crypto,
&req->req_body,
ret, r->e_text,
@@ -2781,12 +2786,11 @@ out:
if (r->pa_used && r->pa_used->cleanup)
r->pa_used->cleanup(r);
free_AS_REP(&r->rep);
free_EncTicketPart(&r->et);
free_EncKDCRepPart(&r->ek);
_kdc_free_fast_state(&r->fast);
if (r->outpadata.len)
free_METHOD_DATA(&r->outpadata);
if (r->client_princ) {
krb5_free_principal(r->context, r->client_princ);
r->client_princ = NULL;

View File

@@ -785,8 +785,6 @@ tgs_make_reply(astgs_request_t r,
_kdc_log_timestamp(r, "TGS-REQ", et->authtime, et->starttime,
et->endtime, et->renew_till);
rep->padata = r->outpadata.len ? &r->outpadata : NULL;
if (krb5_enctype_valid(r->context, serverkey->keytype) != 0
&& _kdc_is_weak_exception(server->entry.principal, serverkey->keytype))
{
@@ -2389,7 +2387,7 @@ server_lookup:
}
pa.padata_type = KRB5_PADATA_SERVER_REFERRAL;
ret = add_METHOD_DATA(&priv->outpadata, &pa);
ret = add_METHOD_DATA(priv->rep.padata, &pa);
krb5_data_free(&pa.padata_value);
if (ret) {
kdc_log(context, config, 4,
@@ -2554,6 +2552,13 @@ _kdc_tgs_rep(astgs_request_t r)
if (ret)
goto out;
ALLOC(r->rep.padata);
if (r->rep.padata == NULL) {
ret = ENOMEM;
krb5_set_error_message(r->context, ret, N_("malloc: out of memory", ""));
goto out;
}
ret = tgs_build_reply(r,
krbtgt,
krbtgt_etype,
@@ -2593,7 +2598,6 @@ out:
free(csec);
free(cusec);
r->rep.padata = NULL; /* may point to outpadata */
free_TGS_REP(&r->rep);
free_TransitedEncoding(&r->et.transited);
free(r->et.starttime);
@@ -2632,9 +2636,6 @@ out:
_kdc_free_fast_state(&r->fast);
krb5_pac_free(r->context, r->pac);
if (r->outpadata.len)
free_METHOD_DATA(&r->outpadata);
if (auth_data) {
free_AuthorizationData(auth_data);
free(auth_data);

View File

@@ -1137,7 +1137,7 @@ _kdc_pk_mk_pa_reply(astgs_request_t r, pk_client_params *cp)
const krb5_data *req_buffer = &r->request;
krb5_keyblock *reply_key = &r->reply_key;
krb5_keyblock *sessionkey = &r->session_key;
METHOD_DATA *md = &r->outpadata;
METHOD_DATA *md = r->rep.padata;
krb5_error_code ret;
void *buf = NULL;
size_t len = 0, size = 0;