F-3015 - https://fenrir.wolfssl.com/finding/3015 - Remove short-circuit OR in FwVerifySignatureCore RSA-PKCS1v1.5 check

pull/488/head
Aidan 2026-04-17 16:18:53 -07:00
parent 61077220e7
commit 6c7d2e6af0
5 changed files with 53 additions and 16 deletions

View File

@ -194,10 +194,16 @@ static int PolicySign(TPM_ALG_ID alg, const char* keyFile, const char* password,
if (rc == 0) {
word32 keySz = key.ecc.dp->size;
*sigSz = keySz * 2;
/* constant-time fixed-width export of r and s avoids
* leaking the leading-zero count of each component */
/* fixed-width export of r and s avoids leaking the
* leading-zero count of each component via data-dependent
* offsets */
#ifdef WOLFSSL_HAVE_SP_ECC
mp_to_unsigned_bin_len_ct(&r, &sig[0], keySz);
mp_to_unsigned_bin_len_ct(&s, &sig[keySz], keySz);
#else
mp_to_unsigned_bin_len(&r, &sig[0], keySz);
mp_to_unsigned_bin_len(&s, &sig[keySz], keySz);
#endif
mp_clear(&r);
mp_clear(&s);
}

View File

@ -1835,9 +1835,12 @@ TPM_RC FwImportVerifyAndDecrypt(
}
}
if (rc == 0) {
/* Always run TPM2_ConstantCompare so timing doesn't leak size match */
/* Always run TPM2_ConstantCompare over min(sizes) so timing doesn't
* leak size match and we don't read past integrity[integritySize] */
word32 cmpSz = (integritySize < (UINT16)digestSz) ?
(word32)integritySize : (word32)digestSz;
sizeMismatch = (integritySize != (UINT16)digestSz);
hmacDiff = TPM2_ConstantCompare(integrity, hmacCalc, (word32)digestSz);
hmacDiff = TPM2_ConstantCompare(integrity, hmacCalc, cmpSz);
if (sizeMismatch | hmacDiff) {
rc = TPM_RC_INTEGRITY;
}
@ -2543,18 +2546,29 @@ TPM_RC FwVerifySignatureCore(FWTPM_Object* obj,
(enum wc_HashType)wcHash);
int expSz = wc_EncodeSignature(expDI,
digest, digestSz, oid);
int sizeMismatch;
int sigDiff;
word32 cmpSz;
FWTPM_ALLOC_BUF(decSig, FWTPM_MAX_PUB_BUF);
wcRc = wc_RsaSSL_Verify(
sig->signature.rsassa.sig.buffer,
sig->signature.rsassa.sig.size,
decSig, (word32)FWTPM_MAX_PUB_BUF, rsaKey);
if (wcRc >= 0) {
if (wcRc != expSz || expSz <= 0 ||
TPM2_ConstantCompare(decSig, expDI, expSz) != 0) {
if (wcRc >= 0 && expSz > 0) {
/* Always run TPM2_ConstantCompare so timing doesn't
* leak decoded-length vs expected-length match */
sizeMismatch = (wcRc != expSz);
cmpSz = (wcRc < expSz) ? (word32)wcRc :
(word32)expSz;
sigDiff = TPM2_ConstantCompare(decSig, expDI, cmpSz);
if (sizeMismatch | sigDiff) {
wcRc = -1;
}
}
else if (wcRc >= 0) {
wcRc = -1;
}
FWTPM_FREE_BUF(decSig);
}
if (wcRc < 0)
@ -2876,6 +2890,9 @@ TPM_RC FwCredentialUnwrap(
FWTPM_DECLARE_VAR(hmac, Hmac);
FWTPM_DECLARE_BUF(decBuf, FWTPM_MAX_NV_DATA + 2);
/* Zero-init so tail bytes are deterministic when integrityHmacSz < 32 */
TPM2_ForceZero(integrityHmac, sizeof(integrityHmac));
FWTPM_ALLOC_VAR(aes, Aes);
FWTPM_ALLOC_VAR(hmac, Hmac);
FWTPM_ALLOC_BUF(decBuf, FWTPM_MAX_NV_DATA + 2);

View File

@ -386,9 +386,11 @@ static int TPM2_ResponseProcess(TPM2_CTX* ctx, TPM2_Packet* packet,
return rc;
}
/* Verify HMAC using constant-time comparison — always run
* TPM2_ConstantCompare so code path timing doesn't depend on
* whether the size matched. */
/* Verify HMAC using constant-time comparison. The wire-format
* size was already validated above; this branch-free pattern
* hardens the tail check in case a future refactor removes
* that gate (hmac.size and authRsp.hmac.size are both
* algorithm-derived and non-secret at this point). */
sizeMismatch = (hmac.size != authRsp.hmac.size);
diff = TPM2_ConstantCompare(hmac.buffer, authRsp.hmac.buffer,
expectedHmacSz);

View File

@ -833,6 +833,11 @@ void TPM2_Packet_ParseSensitive(TPM2_Packet* packet, TPM2B_SENSITIVE* sensitive)
if (sensitive->size == 0) {
return;
}
/* Clamp outer size to remaining packet bytes so inner parses are bounded */
if (packet != NULL && packet->pos < packet->size &&
sensitive->size > (UINT16)(packet->size - packet->pos)) {
sensitive->size = (UINT16)(packet->size - packet->pos);
}
TPM2_Packet_ParseU16(packet, &sensitive->sensitiveArea.sensitiveType);

View File

@ -1158,8 +1158,9 @@ int wolfTPM2_SpdmEnable(WOLFTPM2_DEV* dev)
if (rc == 0) {
rc = wolfTPM2_SPDM_Enable(dev->spdmCtx);
}
/* Restore previous session[0] state */
/* Restore previous session[0] state and clear the stack copy */
XMEMCPY(&dev->session[0], &saveSess, sizeof(dev->session[0]));
TPM2_ForceZero(&saveSess, sizeof(saveSess));
return rc;
}
@ -1177,8 +1178,9 @@ int wolfTPM2_SpdmDisable(WOLFTPM2_DEV* dev)
if (rc == 0) {
rc = wolfTPM2_SPDM_Disable(dev->spdmCtx);
}
/* Restore previous session[0] state */
/* Restore previous session[0] state and clear the stack copy */
XMEMCPY(&dev->session[0], &saveSess, sizeof(dev->session[0]));
TPM2_ForceZero(&saveSess, sizeof(saveSess));
return rc;
}
@ -1586,8 +1588,9 @@ int wolfTPM2_SpdmNationsIdentityKeySet(WOLFTPM2_DEV* dev, int set)
}
}
/* Restore previous session[0] state */
/* Restore previous session[0] state and clear the stack copy */
XMEMCPY(&dev->session[0], &saveSess, sizeof(dev->session[0]));
TPM2_ForceZero(&saveSess, sizeof(saveSess));
return rc;
}
@ -2133,10 +2136,14 @@ static int wolfTPM2_EncryptSecret_ECC(WOLFTPM2_DEV* dev, const WOLFTPM2_KEY* tpm
r, &a, &prime, 1);
}
if (rc == 0) {
/* export shared secret x - constant-time fixed-size export avoids
* leaking the leading-zero count of the ECDH shared secret via
* data-dependent offsets */
/* export shared secret x - fixed-size export avoids leaking the
* leading-zero count of the ECDH shared secret via data-dependent
* offsets */
#ifdef WOLFSSL_HAVE_SP_ECC
rc = mp_to_unsigned_bin_len_ct(r->x, secretPoint.point.x.buffer, keySz);
#else
rc = mp_to_unsigned_bin_len(r->x, secretPoint.point.x.buffer, keySz);
#endif
secretPoint.point.x.size = keySz;
}
if (rc == 0) {