From 4630edaeb40d67dff7801c99dd1806bd0ed2373b Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Wed, 30 Apr 2025 17:53:52 -0400 Subject: [PATCH] tpm2: Fix potential out-of-bound access & abort due to HMAC signing issue Fix an HMAC signing issue that may causes an out-of-bounds access in a TPM2B that in turn was running into an assert() in libtpms causing an abort. The signing issue was due to an inconsistent pairing of the signKey and signScheme parameters, where the signKey is ALG_KEYEDHASH key and inScheme is an ECC or RSA scheme. This fixes CVE-2025-49133. Signed-off-by: Stefan Berger --- src/tpm2/CryptUtil.c | 119 +++++++++++++++++++++++----- src/tpm2/SigningCommands.c | 10 ++- src/tpm2/crypto/CryptHash_fp.h | 2 +- src/tpm2/crypto/openssl/CryptHash.c | 6 +- 4 files changed, 109 insertions(+), 28 deletions(-) diff --git a/src/tpm2/CryptUtil.c b/src/tpm2/CryptUtil.c index cf7d4b9f..febef733 100644 --- a/src/tpm2/CryptUtil.c +++ b/src/tpm2/CryptUtil.c @@ -67,7 +67,7 @@ #include "Tpm.h" /* 10.2.6.3 Hash/HMAC Functions */ /* 10.2.6.3.1 CryptHmacSign() */ -/* Sign a digest using an HMAC key. This an HMAC of a digest, not an HMAC of a message. */ +/* Sign a digest using an HMAC key. This is an HMAC of a digest, not an HMAC of a message. */ /* Error Returns Meaning */ /* TPM_RC_HASH not a valid hash */ static TPM_RC @@ -79,12 +79,18 @@ CryptHmacSign( { HMAC_STATE hmacState; UINT32 digestSize; - digestSize = CryptHmacStart2B(&hmacState, signature->signature.any.hashAlg, - &signKey->sensitive.sensitive.bits.b); - CryptDigestUpdate2B(&hmacState.hashState, &hashData->b); - CryptHmacEnd(&hmacState, digestSize, - (BYTE *)&signature->signature.hmac.digest); - return TPM_RC_SUCCESS; + + if(signature->sigAlg == TPM_ALG_HMAC) + { + digestSize = CryptHmacStart2B(&hmacState, + signature->signature.any.hashAlg, + &signKey->sensitive.sensitive.bits.b); + CryptDigestUpdate2B(&hmacState.hashState, &hashData->b); + CryptHmacEnd(&hmacState, digestSize, + (BYTE *)&signature->signature.hmac.digest); + return TPM_RC_SUCCESS; + } + return TPM_RC_SCHEME; } /* 10.2.6.3.2 CryptHMACVerifySignature() */ /* This function will verify a signature signed by a HMAC key. Note that a caller needs to prepare @@ -1091,7 +1097,7 @@ CryptIsSplitSign( } } /* 10.2.6.6.11 CryptIsAsymSignScheme() */ -/* This function indicates if a scheme algorithm is a sign algorithm. */ +/* This function indicates if a scheme algorithm is a sign algorithm valid for the public key type. */ BOOL CryptIsAsymSignScheme( TPMI_ALG_PUBLIC publicType, // IN: Type of the object @@ -1120,9 +1126,11 @@ CryptIsAsymSignScheme( #if ALG_ECC // If ECC is implemented ECDSA is required case TPM_ALG_ECC: +# if !ALG_ECDSA +# error "ECDSA required if ECC enabled." +# endif switch(scheme) { - // Support for ECDSA is required for ECC case TPM_ALG_ECDSA: #if ALG_ECDAA // ECDAA is optional case TPM_ALG_ECDAA: @@ -1146,6 +1154,58 @@ CryptIsAsymSignScheme( } return isSignScheme; } +//*** CryptIsValidSignScheme() +// This function checks that a signing scheme is valid. This includes verifying +// that the scheme signing algorithm is compatible with the signing object type +// and that the scheme specifies a valid hash algorithm. +static BOOL CryptIsValidSignScheme(TPMI_ALG_PUBLIC publicType, // IN: Type of the object + TPMT_SIG_SCHEME* scheme // IN: the signing scheme +) +{ + BOOL isValidSignScheme = TRUE; + + switch(publicType) + { +#if ALG_RSA + case TPM_ALG_RSA: + isValidSignScheme = CryptIsAsymSignScheme(publicType, scheme->scheme); + break; +#endif // ALG_RSA + +#if ALG_ECC + case TPM_ALG_ECC: + isValidSignScheme = CryptIsAsymSignScheme(publicType, scheme->scheme); + break; +#endif // ALG_ECC + + case TPM_ALG_KEYEDHASH: + if(scheme->scheme != TPM_ALG_HMAC) + { + isValidSignScheme = FALSE; + } + break; + + default: + isValidSignScheme = FALSE; + break; + } + + // Ensure that a valid hash algorithm is specified. Pass 'flag' = FALSE to + // indicate that TPM_ALG_NULL should not be treated as valid. + // + // NOTE: 'details' is of type TPMU_SIG_SCHEME which is a union of many + // different signature scheme types. In all these types (including the type + // of 'any'), the very first member is of type TPMI_ALG_HASH. Therefore, + // when 'any.hashAlg' is set to a valid hash algorithm ID, the hash for any + // signature scheme type will also be a valid hash algorithm ID. (All valid + // hash algorithm IDs are the same for all signature scheme types.) + if(!CryptHashIsValidAlg(scheme->details.any.hashAlg, /* flag = */ FALSE)) + { + isValidSignScheme = FALSE; + } + + return isValidSignScheme; +} /* 10.2.6.6.12 CryptIsAsymDecryptScheme() */ /* This function indicate if a scheme algorithm is a decrypt algorithm. */ BOOL @@ -1200,8 +1260,9 @@ CryptIsAsymDecryptScheme( } /* 10.2.6.6.13 CryptSelectSignScheme() */ /* This function is used by the attestation and signing commands. It implements the rules for - selecting the signature scheme to use in signing. This function requires that the signing key - either be TPM_RH_NULL or be loaded. */ + selecting the signature scheme to use in signing and validates that the selected scheme is + compatible with the key type. It also ensures the selected scheme specifies a valid hash + algorithm. This function requires that the signing key either be TPM_RH_NULL or be loaded. */ /* If a default scheme is defined in object, the default scheme should be chosen, otherwise, the input scheme should be chosen. In the case that both object and input scheme has a non-NULL scheme algorithm, if the schemes are compatible, the input scheme will be chosen. */ @@ -1232,25 +1293,32 @@ CryptSelectSignScheme( { // assignment to save typing. publicArea = &signObject->publicArea; - // A symmetric cipher can be used to encrypt and decrypt but it can't - // be used for signing - if(publicArea->type == TPM_ALG_SYMCIPHER) - return FALSE; - // Point to the scheme object + + // Get a point to the scheme object if(CryptIsAsymAlgorithm(publicArea->type)) - objectScheme = - (TPMT_SIG_SCHEME *)&publicArea->parameters.asymDetail.scheme; + { + objectScheme = + (TPMT_SIG_SCHEME *)&publicArea->parameters.asymDetail.scheme; + } + else if(publicArea->type == TPM_ALG_KEYEDHASH) + { + objectScheme = + (TPMT_SIG_SCHEME *)&publicArea->parameters.keyedHashDetail.scheme; + } else - objectScheme = - (TPMT_SIG_SCHEME *)&publicArea->parameters.keyedHashDetail.scheme; + { + // Only asymmetric key types (RSA, ECC) and keyed hashes can be + // used for signing. A symmetric cipher can be used to encrypt and + // decrypt but can't be used for signing. + return FALSE; + } + // If the object doesn't have a default scheme, then use the // input scheme. if(objectScheme->scheme == TPM_ALG_NULL) { // Input and default can't both be NULL OK = (scheme->scheme != TPM_ALG_NULL); - // Assume that the scheme is compatible with the key. If not, - // an error will be generated in the signing operation. } else if(scheme->scheme == TPM_ALG_NULL) { @@ -1277,6 +1345,13 @@ CryptSelectSignScheme( && (objectScheme->details.any.hashAlg == scheme->details.any.hashAlg); } + + if(OK) + { + // Check that the scheme is compatible with the key type and has a + // valid hash algorithm specified. + OK = CryptIsValidSignScheme(publicArea->type, scheme); + } } return OK; } diff --git a/src/tpm2/SigningCommands.c b/src/tpm2/SigningCommands.c index b26d34f9..a315f750 100644 --- a/src/tpm2/SigningCommands.c +++ b/src/tpm2/SigningCommands.c @@ -116,11 +116,17 @@ TPM2_Sign( // // Input Validation if(!IsSigningObject(signObject)) + { return TPM_RCS_KEY + RC_Sign_keyHandle; - // pick a scheme for sign. If the input sign scheme is not compatible with - // the default scheme, return an error. + } + + // Pick a scheme for signing. If the input signing scheme is not compatible + // with the default scheme or the signing key type, return an error. If a + // valid hash algorithm is not specified, return an error. if(!CryptSelectSignScheme(signObject, &in->inScheme)) + { return TPM_RCS_SCHEME + RC_Sign_inScheme; + } // If validation is provided, or the key is restricted, check the ticket if(in->validation.digest.t.size != 0 || IS_ATTRIBUTE(signObject->publicArea.objectAttributes, TPMA_OBJECT, restricted)) diff --git a/src/tpm2/crypto/CryptHash_fp.h b/src/tpm2/crypto/CryptHash_fp.h index 93d147aa..1149b1a3 100644 --- a/src/tpm2/crypto/CryptHash_fp.h +++ b/src/tpm2/crypto/CryptHash_fp.h @@ -77,7 +77,7 @@ CryptGetHashDef( BOOL CryptHashIsValidAlg( TPM_ALG_ID hashAlg, - BOOL flag + BOOL isAlgNullValid ); LIB_EXPORT TPM_ALG_ID CryptHashGetAlgByIndex( diff --git a/src/tpm2/crypto/openssl/CryptHash.c b/src/tpm2/crypto/openssl/CryptHash.c index fdf66b57..699ce689 100644 --- a/src/tpm2/crypto/openssl/CryptHash.c +++ b/src/tpm2/crypto/openssl/CryptHash.c @@ -149,8 +149,8 @@ CryptGetHashDef( /* FALSE hashAlg is not valid for this TPM */ BOOL CryptHashIsValidAlg( - TPM_ALG_ID hashAlg, - BOOL flag + TPM_ALG_ID hashAlg, // IN: the algorithm to check + BOOL isAlgNullValid // IN: TRUE if TPM_ALG_NULL is to be treated ) { switch(hashAlg) @@ -173,7 +173,7 @@ CryptHashIsValidAlg( return TRUE; break; case ALG_NULL_VALUE: - return flag; + return isAlgNullValid; break; default: break;