Skip to content

Commit fa4a9ef

Browse files
FrauschiFrauschi
committed
pkcs7,aes: reject truncated GCM auth tags
wc_PKCS7_DecodeAuthEnvelopedData() accepted an attacker-controlled GCM tag length from the mac OCTET STRING and did not validate it against the parsed aes-ICVlen parameter. In parallel, wc_AesGcmDecrypt() accepted very short tags on decrypt while encrypt enforced WOLFSSL_MIN_AUTH_TAG_SZ. This made short-tag verification reachable through CMS AuthEnvelopedData and weakened integrity checks by allowing tag truncation. Fixes: - validate parsed macSz range in AuthEnvelopedData decode - require authTagSz to match parsed macSz - reject undersized GCM tags in PKCS7 decode - enforce WOLFSSL_MIN_AUTH_TAG_SZ in wc_AesGcmDecrypt() and wc_AesGcmDecryptFinal() Also add a regression test in pkcs7authenveloped vectors that truncates the final MAC OCTET STRING length from 16 to 1 and verifies decode fails. Reported by: Nicholas Carlini (Anthropic) & Thai Duong (Calif.io)
1 parent 929dd99 commit fa4a9ef

3 files changed

Lines changed: 67 additions & 6 deletions

File tree

‎wolfcrypt/src/aes.c‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10217,8 +10217,9 @@ int wc_AesGcmDecrypt(Aes* aes, byte* out, const byte* in, word32 sz,
1021710217
/* If the sz is non-zero, both in and out must be set. If sz is 0,
1021810218
* in and out are don't cares, as this is is the GMAC case. */
1021910219
if (aes == NULL || iv == NULL || (sz != 0 && (in == NULL || out == NULL)) ||
10220-
authTag == NULL || authTagSz > WC_AES_BLOCK_SIZE || authTagSz == 0 ||
10221-
ivSz == 0 || ((authInSz > 0) && (authIn == NULL)))
10220+
authTag == NULL || authTagSz > WC_AES_BLOCK_SIZE ||
10221+
authTagSz < WOLFSSL_MIN_AUTH_TAG_SZ || ivSz == 0 ||
10222+
((authInSz > 0) && (authIn == NULL)))
1022210223
{
1022310224
return BAD_FUNC_ARG;
1022410225
}
@@ -10781,8 +10782,8 @@ int wc_AesGcmDecrypt(Aes* aes, byte* out, const byte* in, word32 sz,
1078110782
/* If the sz is non-zero, both in and out must be set. If sz is 0,
1078210783
* in and out are don't cares, as this is is the GMAC case. */
1078310784
if (aes == NULL || iv == NULL || (sz != 0 && (in == NULL || out == NULL)) ||
10784-
authTag == NULL || authTagSz > WC_AES_BLOCK_SIZE || authTagSz == 0 ||
10785-
ivSz == 0) {
10785+
authTag == NULL || authTagSz > WC_AES_BLOCK_SIZE ||
10786+
authTagSz < WOLFSSL_MIN_AUTH_TAG_SZ || ivSz == 0) {
1078610787

1078710788
return BAD_FUNC_ARG;
1078810789
}
@@ -12473,7 +12474,7 @@ int wc_AesGcmEncryptFinal(Aes* aes, byte* authTag, word32 authTagSz)
1247312474

1247412475
/* Check validity of parameters. */
1247512476
if ((aes == NULL) || (authTag == NULL) || (authTagSz > WC_AES_BLOCK_SIZE) ||
12476-
(authTagSz == 0)) {
12477+
(authTagSz < WOLFSSL_MIN_AUTH_TAG_SZ)) {
1247712478
ret = BAD_FUNC_ARG;
1247812479
}
1247912480

‎wolfcrypt/src/pkcs7.c‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ struct PKCS7State {
140140
word32 nonceSz; /* size of nonce stored */
141141
word32 aadSz; /* size of additional AEAD data */
142142
word32 tagSz; /* size of tag for AEAD */
143+
word32 icvSz; /* expected ICV/MAC size from AlgoID parameter */
143144
word32 contentSz;
144145
word32 currContIdx; /* index of current content */
145146
word32 currContSz; /* size of current content */
@@ -14222,6 +14223,10 @@ int wc_PKCS7_DecodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* in,
1422214223
if (ret == 0 && GetMyVersion(pkiMsg, &idx, &macSz, pkiMsgSz) < 0) {
1422314224
ret = ASN_PARSE_E;
1422414225
}
14226+
if (ret == 0 && (macSz <= 0 || macSz > WC_AES_BLOCK_SIZE)) {
14227+
WOLFSSL_MSG("AuthEnvelopedData invalid MAC length");
14228+
ret = ASN_PARSE_E;
14229+
}
1422514230

1422614231
if (ret == 0) {
1422714232
explicitOctet = 0;
@@ -14267,7 +14272,8 @@ int wc_PKCS7_DecodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* in,
1426714272
break;
1426814273
}
1426914274

14270-
/* store nonce for later */
14275+
/* store nonce and macSz for later */
14276+
pkcs7->stream->icvSz = (word32)macSz;
1427114277
if (nonceSz > 0) {
1427214278
pkcs7->stream->nonceSz = (word32)nonceSz;
1427314279
pkcs7->stream->nonce = (byte*)XMALLOC((word32)nonceSz,
@@ -14458,6 +14464,7 @@ int wc_PKCS7_DecodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* in,
1445814464
encodedAttribSz = pkcs7->stream->aadSz;
1445914465
encodedAttribs = pkcs7->stream->aad;
1446014466
}
14467+
macSz = (int)pkcs7->stream->icvSz;
1446114468
#endif
1446214469

1446314470

@@ -14474,6 +14481,17 @@ int wc_PKCS7_DecodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* in,
1447414481
ret = ASN_PARSE_E;
1447514482
}
1447614483
authTagSz = (word32)length;
14484+
if (ret == 0 && authTagSz != (word32)macSz) {
14485+
WOLFSSL_MSG("AuthEnvelopedData authTag size mismatch");
14486+
ret = ASN_PARSE_E;
14487+
}
14488+
if (ret == 0 &&
14489+
(encOID == AES128GCMb || encOID == AES192GCMb ||
14490+
encOID == AES256GCMb) &&
14491+
authTagSz < WOLFSSL_MIN_AUTH_TAG_SZ) {
14492+
WOLFSSL_MSG("AuthEnvelopedData GCM authTag too small");
14493+
ret = ASN_PARSE_E;
14494+
}
1447714495

1447814496
#ifndef NO_PKCS7_STREAM
1447914497
/* there might not be enough data for the auth tag too */

‎wolfcrypt/test/test.c‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57660,6 +57660,9 @@ static wc_test_ret_t pkcs7authenveloped_run_vectors(byte* rsaCert, word32 rsaCer
5766057660
wc_test_ret_t ret = 0;
5766157661
int testSz = 0, i;
5766257662
int envelopedSz, decodedSz;
57663+
#ifdef HAVE_AESGCM
57664+
int tagTruncationChecked = 0;
57665+
#endif
5766357666

5766457667
byte *enveloped = NULL;
5766557668
byte *decoded = NULL;
@@ -58171,6 +58174,45 @@ static wc_test_ret_t pkcs7authenveloped_run_vectors(byte* rsaCert, word32 rsaCer
5817158174
ERROR_OUT(WC_TEST_RET_ENC_NC, out);
5817258175
}
5817358176

58177+
#ifdef HAVE_AESGCM
58178+
if (tagTruncationChecked == 0 &&
58179+
(testVectors[i].encryptOID == AES128GCMb ||
58180+
testVectors[i].encryptOID == AES192GCMb ||
58181+
testVectors[i].encryptOID == AES256GCMb) &&
58182+
testVectors[i].authAttribsSz == 0 &&
58183+
testVectors[i].unauthAttribsSz == 0 &&
58184+
envelopedSz > (WC_AES_BLOCK_SIZE + 2)) {
58185+
int macIdx = envelopedSz - (WC_AES_BLOCK_SIZE + 2);
58186+
byte* tampered = NULL;
58187+
58188+
/* For plain DER output without unauthenticated attributes, the
58189+
* MAC OCTET STRING is the final field. */
58190+
if (enveloped[macIdx] == ASN_OCTET_STRING &&
58191+
enveloped[macIdx + 1] == WC_AES_BLOCK_SIZE) {
58192+
tampered = (byte*)XMALLOC((word32)envelopedSz, HEAP_HINT,
58193+
DYNAMIC_TYPE_TMP_BUFFER);
58194+
if (tampered == NULL) {
58195+
wc_PKCS7_Free(pkcs7);
58196+
ERROR_OUT(WC_TEST_RET_ENC_ERRNO, out);
58197+
}
58198+
XMEMCPY(tampered, enveloped, (word32)envelopedSz);
58199+
tampered[macIdx + 1] = 1;
58200+
58201+
decodedSz = wc_PKCS7_DecodeAuthEnvelopedData(pkcs7, tampered,
58202+
(word32)envelopedSz, decoded, PKCS7_BUF_SIZE);
58203+
58204+
XFREE(tampered, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
58205+
tampered = NULL;
58206+
58207+
if (decodedSz > 0) {
58208+
wc_PKCS7_Free(pkcs7);
58209+
ERROR_OUT(WC_TEST_RET_ENC_NC, out);
58210+
}
58211+
tagTruncationChecked = 1;
58212+
}
58213+
}
58214+
#endif
58215+
5817458216
#ifdef PKCS7_OUTPUT_TEST_BUNDLES
5817558217
/* output pkcs7 envelopedData for external testing */
5817658218
pkcs7File = XFOPEN(testVectors[i].outFileName, "wb");

0 commit comments

Comments
 (0)