From b0372e31b81321a820204450a35c7633caf1b7dd Mon Sep 17 00:00:00 2001 From: Greg Hudson Date: Fri, 14 Jan 2022 02:05:58 -0500 Subject: [PATCH] Factor out PAC checksum verification Reduce code repetition in PAC checksum handling by adding a helper function. Remove the unnecessary prefix on several function names. --- src/lib/krb5/krb/pac.c | 173 +++++++++++++---------------------------- 1 file changed, 55 insertions(+), 118 deletions(-) diff --git a/src/lib/krb5/krb/pac.c b/src/lib/krb5/krb/pac.c index 6eb23d8090..2f6ad4e1df 100644 --- a/src/lib/krb5/krb/pac.c +++ b/src/lib/krb5/krb/pac.c @@ -493,10 +493,8 @@ k5_pac_validate_client(krb5_context context, } static krb5_error_code -k5_pac_zero_signature(krb5_context context, - const krb5_pac pac, - krb5_ui_4 type, - krb5_data *data) +zero_signature(krb5_context context, const krb5_pac pac, krb5_ui_4 type, + krb5_data *data) { PAC_INFO_BUFFER *buffer = NULL; size_t i; @@ -530,151 +528,89 @@ k5_pac_zero_signature(krb5_context context, } static krb5_error_code -k5_pac_verify_server_checksum(krb5_context context, - const krb5_pac pac, - const krb5_keyblock *server) +verify_checksum(krb5_context context, const krb5_pac pac, uint32_t buffer_type, + const krb5_keyblock *key, krb5_keyusage usage, + const krb5_data *data) { krb5_error_code ret; - krb5_data pac_data; /* PAC with zeroed checksums */ + krb5_data buffer; + krb5_cksumtype cksumtype; krb5_checksum checksum; - krb5_data checksum_data; krb5_boolean valid; - krb5_octet *p; + size_t cksumlen; - ret = k5_pac_locate_buffer(context, pac, KRB5_PAC_SERVER_CHECKSUM, - &checksum_data); + ret = k5_pac_locate_buffer(context, pac, buffer_type, &buffer); if (ret != 0) return ret; - - if (checksum_data.length < PAC_SIGNATURE_DATA_LENGTH) + if (buffer.length < PAC_SIGNATURE_DATA_LENGTH) return KRB5_BAD_MSIZE; - p = (krb5_octet *)checksum_data.data; - checksum.checksum_type = load_32_le(p); - checksum.length = checksum_data.length - PAC_SIGNATURE_DATA_LENGTH; - checksum.contents = p + PAC_SIGNATURE_DATA_LENGTH; - if (checksum.checksum_type == CKSUMTYPE_SHA1) + cksumtype = load_32_le(buffer.data); + if (buffer_type == KRB5_PAC_SERVER_CHECKSUM && cksumtype == CKSUMTYPE_SHA1) return KRB5KDC_ERR_SUMTYPE_NOSUPP; - if (!krb5_c_is_keyed_cksum(checksum.checksum_type)) + if (!krb5_c_is_keyed_cksum(cksumtype)) return KRB5KRB_ERR_GENERIC; - pac_data.length = pac->data.length; - pac_data.data = k5memdup(pac->data.data, pac->data.length, &ret); - if (pac_data.data == NULL) - return ret; - - /* Zero out both checksum buffers */ - ret = k5_pac_zero_signature(context, pac, KRB5_PAC_SERVER_CHECKSUM, - &pac_data); - if (ret != 0) { - free(pac_data.data); - return ret; - } - - ret = k5_pac_zero_signature(context, pac, KRB5_PAC_PRIVSVR_CHECKSUM, - &pac_data); - if (ret != 0) { - free(pac_data.data); + /* There may be an RODCIdentifier trailer (see [MS-PAC] 2.8), so look up + * the length of the checksum by its type. */ + ret = krb5_c_checksum_length(context, cksumtype, &cksumlen); + if (ret) return ret; - } + if (cksumlen > buffer.length - PAC_SIGNATURE_DATA_LENGTH) + return KRB5_BAD_MSIZE; + checksum.checksum_type = cksumtype; + checksum.length = cksumlen; + checksum.contents = (uint8_t *)buffer.data + PAC_SIGNATURE_DATA_LENGTH; - ret = krb5_c_verify_checksum(context, server, - KRB5_KEYUSAGE_APP_DATA_CKSUM, - &pac_data, &checksum, &valid); + ret = krb5_c_verify_checksum(context, key, usage, data, &checksum, &valid); + return ret ? ret : (valid ? 0 : KRB5KRB_AP_ERR_MODIFIED); +} - free(pac_data.data); +static krb5_error_code +verify_server_checksum(krb5_context context, const krb5_pac pac, + const krb5_keyblock *server) +{ + krb5_error_code ret; + krb5_data copy; /* PAC with zeroed checksums */ - if (ret != 0) { + ret = krb5int_copy_data_contents(context, &pac->data, ©); + if (ret) return ret; - } - if (valid == FALSE) - ret = KRB5KRB_AP_ERR_MODIFIED; + /* Zero out both checksum buffers */ + ret = zero_signature(context, pac, KRB5_PAC_SERVER_CHECKSUM, ©); + if (ret) + goto cleanup; + ret = zero_signature(context, pac, KRB5_PAC_PRIVSVR_CHECKSUM, ©); + if (ret) + goto cleanup; + + ret = verify_checksum(context, pac, KRB5_PAC_SERVER_CHECKSUM, server, + KRB5_KEYUSAGE_APP_DATA_CKSUM, ©); +cleanup: + free(copy.data); return ret; } static krb5_error_code -k5_pac_verify_kdc_checksum(krb5_context context, - const krb5_pac pac, - const krb5_keyblock *privsvr) +verify_kdc_checksum(krb5_context context, const krb5_pac pac, + const krb5_keyblock *privsvr) { krb5_error_code ret; - krb5_data server_checksum, privsvr_checksum; - krb5_checksum checksum; - krb5_boolean valid; - krb5_octet *p; - - ret = k5_pac_locate_buffer(context, pac, KRB5_PAC_PRIVSVR_CHECKSUM, - &privsvr_checksum); - if (ret != 0) - return ret; - - if (privsvr_checksum.length < PAC_SIGNATURE_DATA_LENGTH) - return KRB5_BAD_MSIZE; + krb5_data server_checksum; ret = k5_pac_locate_buffer(context, pac, KRB5_PAC_SERVER_CHECKSUM, &server_checksum); - if (ret != 0) + if (ret) return ret; - if (server_checksum.length < PAC_SIGNATURE_DATA_LENGTH) return KRB5_BAD_MSIZE; - - p = (krb5_octet *)privsvr_checksum.data; - checksum.checksum_type = load_32_le(p); - checksum.length = privsvr_checksum.length - PAC_SIGNATURE_DATA_LENGTH; - checksum.contents = p + PAC_SIGNATURE_DATA_LENGTH; - if (!krb5_c_is_keyed_cksum(checksum.checksum_type)) - return KRB5KRB_ERR_GENERIC; - server_checksum.data += PAC_SIGNATURE_DATA_LENGTH; server_checksum.length -= PAC_SIGNATURE_DATA_LENGTH; - ret = krb5_c_verify_checksum(context, privsvr, - KRB5_KEYUSAGE_APP_DATA_CKSUM, - &server_checksum, &checksum, &valid); - if (ret != 0) - return ret; - - if (valid == FALSE) - ret = KRB5KRB_AP_ERR_MODIFIED; - - return ret; -} - -static krb5_error_code -verify_ticket_checksum(krb5_context context, const krb5_pac pac, - const krb5_data *ticket, const krb5_keyblock *privsvr) -{ - krb5_error_code ret; - krb5_checksum checksum; - krb5_data checksum_data; - krb5_boolean valid; - krb5_octet *p; - - ret = k5_pac_locate_buffer(context, pac, KRB5_PAC_TICKET_CHECKSUM, - &checksum_data); - if (ret != 0) - return KRB5KRB_AP_ERR_MODIFIED; - - if (checksum_data.length < PAC_SIGNATURE_DATA_LENGTH) - return KRB5_BAD_MSIZE; - - p = (krb5_octet *)checksum_data.data; - checksum.checksum_type = load_32_le(p); - checksum.length = checksum_data.length - PAC_SIGNATURE_DATA_LENGTH; - checksum.contents = p + PAC_SIGNATURE_DATA_LENGTH; - if (!krb5_c_is_keyed_cksum(checksum.checksum_type)) - return KRB5KRB_ERR_GENERIC; - - ret = krb5_c_verify_checksum(context, privsvr, - KRB5_KEYUSAGE_APP_DATA_CKSUM, ticket, - &checksum, &valid); - if (ret != 0) - return ret; - - return valid ? 0 : KRB5KRB_AP_ERR_MODIFIED; + return verify_checksum(context, pac, KRB5_PAC_PRIVSVR_CHECKSUM, privsvr, + KRB5_KEYUSAGE_APP_DATA_CKSUM, &server_checksum); } /* Per MS-PAC 2.8.3, tickets encrypted to TGS and password change principals @@ -761,7 +697,8 @@ krb5_kdc_verify_ticket(krb5_context context, const krb5_enc_tkt_part *enc_tkt, if (ret) goto cleanup; - ret = verify_ticket_checksum(context, pac, recoded_tkt, privsvr); + ret = verify_checksum(context, pac, KRB5_PAC_TICKET_CHECKSUM, privsvr, + KRB5_KEYUSAGE_APP_DATA_CKSUM, recoded_tkt); if (ret) goto cleanup; } @@ -804,13 +741,13 @@ krb5_pac_verify_ext(krb5_context context, krb5_error_code ret; if (server != NULL) { - ret = k5_pac_verify_server_checksum(context, pac, server); + ret = verify_server_checksum(context, pac, server); if (ret != 0) return ret; } if (privsvr != NULL) { - ret = k5_pac_verify_kdc_checksum(context, pac, privsvr); + ret = verify_kdc_checksum(context, pac, privsvr); if (ret != 0) return ret; } -- 2.39.2