krb5: rework PAC validation loop

Avoid allocating the PAC on error.

Closes: #836
This commit is contained in:
Isaac Boukris
2021-09-19 15:16:58 +03:00
committed by Luke Howard
parent b295167208
commit 6df8be5091

View File

@@ -1372,11 +1372,10 @@ _krb5_kdc_pac_ticket_parse(krb5_context context,
krb5_pac *ppac) krb5_pac *ppac)
{ {
AuthorizationData *ad = tkt->authorization_data; AuthorizationData *ad = tkt->authorization_data;
krb5_boolean pac_found = FALSE;
krb5_pac pac = NULL; krb5_pac pac = NULL;
unsigned i, j; unsigned i, j;
size_t len = 0; size_t len = 0;
krb5_error_code ret; krb5_error_code ret = 0;
*signedticket = FALSE; *signedticket = FALSE;
*ppac = NULL; *ppac = NULL;
@@ -1387,8 +1386,10 @@ _krb5_kdc_pac_ticket_parse(krb5_context context,
for (i = 0; i < ad->len; i++) { for (i = 0; i < ad->len; i++) {
AuthorizationData child; AuthorizationData child;
if (ad->val[i].ad_type == KRB5_AUTHDATA_WIN2K_PAC) if (ad->val[i].ad_type == KRB5_AUTHDATA_WIN2K_PAC) {
return KRB5KDC_ERR_BADOPTION; ret = KRB5KDC_ERR_BADOPTION;
goto out;
}
if (ad->val[i].ad_type != KRB5_AUTHDATA_IF_RELEVANT) if (ad->val[i].ad_type != KRB5_AUTHDATA_IF_RELEVANT)
continue; continue;
@@ -1400,20 +1401,22 @@ _krb5_kdc_pac_ticket_parse(krb5_context context,
if (ret) { if (ret) {
krb5_set_error_message(context, ret, "Failed to decode " krb5_set_error_message(context, ret, "Failed to decode "
"AD-IF-RELEVANT with %d", ret); "AD-IF-RELEVANT with %d", ret);
return ret; goto out;
} }
for (j = 0; j < child.len; j++) { for (j = 0; j < child.len; j++) {
if (child.val[j].ad_type == KRB5_AUTHDATA_WIN2K_PAC) {
krb5_data adifr_data = ad->val[i].ad_data; krb5_data adifr_data = ad->val[i].ad_data;
krb5_data pac_data = child.val[j].ad_data; krb5_data pac_data = child.val[j].ad_data;
krb5_data recoded_adifr; krb5_data recoded_adifr;
if (pac_found) { if (child.val[j].ad_type != KRB5_AUTHDATA_WIN2K_PAC)
continue;
if (pac != NULL) {
free_AuthorizationData(&child); free_AuthorizationData(&child);
return KRB5KDC_ERR_BADOPTION; ret = KRB5KDC_ERR_BADOPTION;
goto out;
} }
pac_found = TRUE;
ret = krb5_pac_parse(context, ret = krb5_pac_parse(context,
pac_data.data, pac_data.data,
@@ -1421,14 +1424,11 @@ _krb5_kdc_pac_ticket_parse(krb5_context context,
&pac); &pac);
if (ret) { if (ret) {
free_AuthorizationData(&child); free_AuthorizationData(&child);
return ret; goto out;
} }
if (pac->ticket_checksum == NULL) { if (pac->ticket_checksum == NULL)
free_AuthorizationData(&child);
*ppac = pac;
continue; continue;
}
/* /*
* Encode the ticket with the PAC replaced with a single zero * Encode the ticket with the PAC replaced with a single zero
@@ -1443,11 +1443,10 @@ _krb5_kdc_pac_ticket_parse(krb5_context context,
krb5_abortx(context, "Internal error in ASN.1 encoder"); krb5_abortx(context, "Internal error in ASN.1 encoder");
child.val[j].ad_data = pac_data; child.val[j].ad_data = pac_data;
free_AuthorizationData(&child);
if (ret) { if (ret) {
krb5_pac_free(context, pac); free_AuthorizationData(&child);
return ret; goto out;
} }
ad->val[i].ad_data = recoded_adifr; ad->val[i].ad_data = recoded_adifr;
@@ -1462,17 +1461,24 @@ _krb5_kdc_pac_ticket_parse(krb5_context context,
ad->val[i].ad_data = adifr_data; ad->val[i].ad_data = adifr_data;
krb5_data_free(&recoded_adifr); krb5_data_free(&recoded_adifr);
if (ret) {
free_AuthorizationData(&child);
goto out;
}
*signedticket = TRUE;
}
free_AuthorizationData(&child);
}
out:
if (ret) { if (ret) {
krb5_pac_free(context, pac); krb5_pac_free(context, pac);
return ret; return ret;
} }
*signedticket = TRUE;
*ppac = pac; *ppac = pac;
}
}
free_AuthorizationData(&child);
}
return 0; return 0;
} }