Skip to content

Commit a88dd07

Browse files
FrauschiJacobBarthelmeh
authored andcommitted
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 1faddd6 commit a88dd07

4 files changed

Lines changed: 69 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 */
@@ -14235,6 +14236,10 @@ int wc_PKCS7_DecodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* in,
1423514236
if (ret == 0 && GetMyVersion(pkiMsg, &idx, &macSz, pkiMsgSz) < 0) {
1423614237
ret = ASN_PARSE_E;
1423714238
}
14239+
if (ret == 0 && (macSz <= 0 || macSz > WC_AES_BLOCK_SIZE)) {
14240+
WOLFSSL_MSG("AuthEnvelopedData invalid MAC length");
14241+
ret = ASN_PARSE_E;
14242+
}
1423814243

1423914244
if (ret == 0) {
1424014245
explicitOctet = 0;
@@ -14280,7 +14285,8 @@ int wc_PKCS7_DecodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* in,
1428014285
break;
1428114286
}
1428214287

14283-
/* store nonce for later */
14288+
/* store nonce and macSz for later */
14289+
pkcs7->stream->icvSz = (word32)macSz;
1428414290
if (nonceSz > 0) {
1428514291
pkcs7->stream->nonceSz = (word32)nonceSz;
1428614292
pkcs7->stream->nonce = (byte*)XMALLOC((word32)nonceSz,
@@ -14471,6 +14477,7 @@ int wc_PKCS7_DecodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* in,
1447114477
encodedAttribSz = pkcs7->stream->aadSz;
1447214478
encodedAttribs = pkcs7->stream->aad;
1447314479
}
14480+
macSz = (int)pkcs7->stream->icvSz;
1447414481
#endif
1447514482

1447614483

@@ -14487,6 +14494,17 @@ int wc_PKCS7_DecodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* in,
1448714494
ret = ASN_PARSE_E;
1448814495
}
1448914496
authTagSz = (word32)length;
14497+
if (ret == 0 && authTagSz != (word32)macSz) {
14498+
WOLFSSL_MSG("AuthEnvelopedData authTag size mismatch");
14499+
ret = ASN_PARSE_E;
14500+
}
14501+
if (ret == 0 &&
14502+
(encOID == AES128GCMb || encOID == AES192GCMb ||
14503+
encOID == AES256GCMb) &&
14504+
authTagSz < WOLFSSL_MIN_AUTH_TAG_SZ) {
14505+
WOLFSSL_MSG("AuthEnvelopedData GCM authTag too small");
14506+
ret = ASN_PARSE_E;
14507+
}
1449014508

1449114509
#ifndef NO_PKCS7_STREAM
1449214510
/* 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
@@ -57721,6 +57721,9 @@ static wc_test_ret_t pkcs7authenveloped_run_vectors(byte* rsaCert, word32 rsaCer
5772157721
wc_test_ret_t ret = 0;
5772257722
int testSz = 0, i;
5772357723
int envelopedSz, decodedSz;
57724+
#ifdef HAVE_AESGCM
57725+
int tagTruncationChecked = 0;
57726+
#endif
5772457727

5772557728
byte *enveloped = NULL;
5772657729
byte *decoded = NULL;
@@ -58232,6 +58235,45 @@ static wc_test_ret_t pkcs7authenveloped_run_vectors(byte* rsaCert, word32 rsaCer
5823258235
ERROR_OUT(WC_TEST_RET_ENC_NC, out);
5823358236
}
5823458237

58238+
#ifdef HAVE_AESGCM
58239+
if (tagTruncationChecked == 0 &&
58240+
(testVectors[i].encryptOID == AES128GCMb ||
58241+
testVectors[i].encryptOID == AES192GCMb ||
58242+
testVectors[i].encryptOID == AES256GCMb) &&
58243+
testVectors[i].authAttribsSz == 0 &&
58244+
testVectors[i].unauthAttribsSz == 0 &&
58245+
envelopedSz > (WC_AES_BLOCK_SIZE + 2)) {
58246+
int macIdx = envelopedSz - (WC_AES_BLOCK_SIZE + 2);
58247+
byte* tampered = NULL;
58248+
58249+
/* For plain DER output without unauthenticated attributes, the
58250+
* MAC OCTET STRING is the final field. */
58251+
if (enveloped[macIdx] == ASN_OCTET_STRING &&
58252+
enveloped[macIdx + 1] == WC_AES_BLOCK_SIZE) {
58253+
tampered = (byte*)XMALLOC((word32)envelopedSz, HEAP_HINT,
58254+
DYNAMIC_TYPE_TMP_BUFFER);
58255+
if (tampered == NULL) {
58256+
wc_PKCS7_Free(pkcs7);
58257+
ERROR_OUT(WC_TEST_RET_ENC_ERRNO, out);
58258+
}
58259+
XMEMCPY(tampered, enveloped, (word32)envelopedSz);
58260+
tampered[macIdx + 1] = 1;
58261+
58262+
decodedSz = wc_PKCS7_DecodeAuthEnvelopedData(pkcs7, tampered,
58263+
(word32)envelopedSz, decoded, PKCS7_BUF_SIZE);
58264+
58265+
XFREE(tampered, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
58266+
tampered = NULL;
58267+
58268+
if (decodedSz > 0) {
58269+
wc_PKCS7_Free(pkcs7);
58270+
ERROR_OUT(WC_TEST_RET_ENC_NC, out);
58271+
}
58272+
tagTruncationChecked = 1;
58273+
}
58274+
}
58275+
#endif
58276+
5823558277
#ifdef PKCS7_OUTPUT_TEST_BUNDLES
5823658278
/* output pkcs7 envelopedData for external testing */
5823758279
pkcs7File = XFOPEN(testVectors[i].outFileName, "wb");

wolfssl/wolfcrypt/settings.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3429,6 +3429,8 @@ extern void uITRON4_free(void *p) ;
34293429
/* Default AES minimum auth tag sz, allow user to override */
34303430
#ifndef WOLFSSL_MIN_AUTH_TAG_SZ
34313431
#define WOLFSSL_MIN_AUTH_TAG_SZ 12
3432+
#elif WOLFSSL_MIN_AUTH_TAG_SZ < 1
3433+
#error WOLFSSL_MIN_AUTH_TAG_SZ must be at least 1
34323434
#endif
34333435

34343436

0 commit comments

Comments
 (0)