From d21e370822c7fb53fb484903587c06d55640fe53 Mon Sep 17 00:00:00 2001 From: Chris Conlon Date: Thu, 27 Feb 2020 14:42:57 -0700 Subject: [PATCH 1/5] add support for PKCS7/CMS EnvelopedData with fragmented encrypted content --- tests/api.c | 2 +- wolfcrypt/src/pkcs7.c | 130 +++++++++++++++++++++++++++++--------- wolfssl/wolfcrypt/pkcs7.h | 4 ++ 3 files changed, 105 insertions(+), 31 deletions(-) diff --git a/tests/api.c b/tests/api.c index 39fccc391..97c2cb4fc 100644 --- a/tests/api.c +++ b/tests/api.c @@ -18416,7 +18416,7 @@ static void test_wc_PKCS7_BER(void) XFILE f; byte der[4096]; #ifndef NO_DES3 - byte decoded[1024]; + byte decoded[2048]; #endif word32 derSz; int ret; diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 3231ce661..12494d970 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -1172,6 +1172,11 @@ void wc_PKCS7_Free(PKCS7* pkcs7) pkcs7->pkcs7Digest = NULL; pkcs7->pkcs7DigestSz = 0; } + if (pkcs7->cachedEncryptedContent != NULL) { + XFREE(pkcs7->cachedEncryptedContent, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + pkcs7->cachedEncryptedContent = NULL; + pkcs7->cachedEncryptedContentSz = 0; + } if (pkcs7->isDynamic) { pkcs7->isDynamic = 0; @@ -9987,6 +9992,41 @@ WOLFSSL_API int wc_PKCS7_SetKey(PKCS7* pkcs7, byte* key, word32 keySz) } +/* append data to encrypted content cache in PKCS7 structure + * return 0 on success, negative on error */ +static int PKCS7_CacheEncryptedContent(PKCS7* pkcs7, byte* in, word32 inSz) +{ + byte* oldCache; + word32 oldCacheSz; + + if (pkcs7 == NULL || in == NULL) + return BAD_FUNC_ARG; + + /* save pointer to old cache */ + oldCache = pkcs7->cachedEncryptedContent; + oldCacheSz = pkcs7->cachedEncryptedContentSz; + + /* re-allocate new buffer to fit appended data */ + pkcs7->cachedEncryptedContent = (byte*)XMALLOC(oldCacheSz + inSz, + pkcs7->heap, DYNAMIC_TYPE_PKCS7); + if (pkcs7->cachedEncryptedContent == NULL) { + pkcs7->cachedEncryptedContentSz = 0; + XFREE(oldCache, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + return MEMORY_E; + } + + if (oldCache != NULL) { + XMEMCPY(pkcs7->cachedEncryptedContent, oldCache, oldCacheSz); + } + XMEMCPY(pkcs7->cachedEncryptedContent + oldCacheSz, in, inSz); + pkcs7->cachedEncryptedContentSz += inSz; + + XFREE(oldCache, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + + return 0; +} + + /* unwrap and decrypt PKCS#7 envelopedData object, return decoded size */ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, word32 inSz, byte* output, @@ -10009,6 +10049,7 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, byte* pkiMsg = in; word32 pkiMsgSz = inSz; byte* decryptedKey = NULL; + int encryptedContentTotalSz = 0; int encryptedContentSz = 0; byte padLen; byte* encryptedContent = NULL; @@ -10223,27 +10264,51 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, } idx++; - if (ret == 0 && GetLength(pkiMsg, &idx, &encryptedContentSz, + if (ret == 0 && GetLength(pkiMsg, &idx, &encryptedContentTotalSz, pkiMsgSz) <= 0) { ret = ASN_PARSE_E; } if (ret == 0 && explicitOctet) { - if (GetASNTag(pkiMsg, &idx, &tag, pkiMsgSz) < 0) { - ret = ASN_PARSE_E; - break; - } + /* encrypted content may be fragmented into multiple + * consecutive OCTET STRINGs, if so loop through + * collecting and caching encrypted content bytes */ + localIdx = idx; + while (idx < (localIdx + encryptedContentTotalSz)) { - if (tag != ASN_OCTET_STRING) { - ret = ASN_PARSE_E; - break; - } + if (GetASNTag(pkiMsg, &idx, &tag, pkiMsgSz) < 0) { + ret = ASN_PARSE_E; + break; + } - if (ret == 0 && GetLength(pkiMsg, &idx, &encryptedContentSz, - pkiMsgSz) <= 0) { - ret = ASN_PARSE_E; + if (tag != ASN_OCTET_STRING) { + ret = ASN_PARSE_E; + break; + } + + if (ret == 0 && GetLength(pkiMsg, &idx, + &encryptedContentSz, pkiMsgSz) <= 0) { + ret = ASN_PARSE_E; + break; + } + + ret = PKCS7_CacheEncryptedContent(pkcs7, &pkiMsg[idx], + encryptedContentSz); + if (ret != 0) { + break; + } + + /* advance idx past encrypted content */ + idx += encryptedContentSz; + } + } else { + /* cache encrypted content, no OCTET STRING */ + ret = PKCS7_CacheEncryptedContent(pkcs7, &pkiMsg[idx], + encryptedContentTotalSz); + if (ret != 0) { break; } + idx += encryptedContentTotalSz; } if (ret != 0) @@ -10253,10 +10318,9 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, if ((ret = wc_PKCS7_StreamEndCase(pkcs7, &tmpIdx, &idx)) != 0) { break; } - pkcs7->stream->expected = encryptedContentSz; + pkcs7->stream->expected = 0; wc_PKCS7_StreamGetVar(pkcs7, &encOID, &expBlockSz, 0); - wc_PKCS7_StreamStoreVar(pkcs7, encOID, expBlockSz, - encryptedContentSz); + wc_PKCS7_StreamStoreVar(pkcs7, encOID, expBlockSz, 0); #endif wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_ENV_5); FALL_THROUGH; @@ -10269,8 +10333,7 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, return ret; } - wc_PKCS7_StreamGetVar(pkcs7, &encOID, &expBlockSz, - &encryptedContentSz); + wc_PKCS7_StreamGetVar(pkcs7, &encOID, &expBlockSz, NULL); tmpIv = pkcs7->stream->tmpIv; /* restore decrypted key */ @@ -10280,23 +10343,16 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, #else ret = 0; #endif - encryptedContent = (byte*)XMALLOC(encryptedContentSz, pkcs7->heap, - DYNAMIC_TYPE_PKCS7); - if (encryptedContent == NULL) { - ret = MEMORY_E; - break; - } - if (ret == 0) { - XMEMCPY(encryptedContent, &pkiMsg[idx], encryptedContentSz); - } + /* use cached content */ + encryptedContent = pkcs7->cachedEncryptedContent; + encryptedContentSz = pkcs7->cachedEncryptedContentSz; /* decrypt encryptedContent */ ret = wc_PKCS7_DecryptContent(pkcs7, encOID, decryptedKey, blockKeySz, tmpIv, expBlockSz, NULL, 0, NULL, 0, encryptedContent, encryptedContentSz, encryptedContent); if (ret != 0) { - XFREE(encryptedContent, pkcs7->heap, DYNAMIC_TYPE_PKCS7); break; } @@ -10305,7 +10361,6 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, /* copy plaintext to output */ if (padLen > encryptedContentSz || (word32)(encryptedContentSz - padLen) > outputSz) { - XFREE(encryptedContent, pkcs7->heap, DYNAMIC_TYPE_PKCS7); ret = BUFFER_E; break; } @@ -10313,9 +10368,13 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, /* free memory, zero out keys */ ForceZero(decryptedKey, MAX_ENCRYPTED_KEY_SZ); - ForceZero(encryptedContent, encryptedContentSz); - XFREE(encryptedContent, pkcs7->heap, DYNAMIC_TYPE_PKCS7); XFREE(decryptedKey, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + if (pkcs7->cachedEncryptedContent != NULL) { + XFREE(pkcs7->cachedEncryptedContent, pkcs7->heap, + DYNAMIC_TYPE_PKCS7); + pkcs7->cachedEncryptedContent = NULL; + pkcs7->cachedEncryptedContentSz = 0; + } ret = encryptedContentSz - padLen; #ifndef NO_PKCS7_STREAM @@ -10335,12 +10394,23 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, if (ret < 0 && ret != WC_PKCS7_WANT_READ_E) { wc_PKCS7_ResetStream(pkcs7); wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_START); + if (pkcs7->cachedEncryptedContent != NULL) { + XFREE(pkcs7->cachedEncryptedContent, pkcs7->heap, + DYNAMIC_TYPE_PKCS7); + pkcs7->cachedEncryptedContent = NULL; + pkcs7->cachedEncryptedContentSz = 0; + } } #else if (decryptedKey != NULL && ret < 0) { ForceZero(decryptedKey, MAX_ENCRYPTED_KEY_SZ); XFREE(decryptedKey, pkcs7->heap, DYNAMIC_TYPE_PKCS7); } + if (pkcs7->cachedEncryptedContent != NULL && ret < 0) { + XFREE(pkcs7->cachedEncryptedContent, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + pkcs7->cachedEncryptedContent = NULL; + pkcs7->cachedEncryptedContentSz = 0; + } #endif return ret; } diff --git a/wolfssl/wolfcrypt/pkcs7.h b/wolfssl/wolfcrypt/pkcs7.h index 840d94281..0292d1d07 100644 --- a/wolfssl/wolfcrypt/pkcs7.h +++ b/wolfssl/wolfcrypt/pkcs7.h @@ -330,6 +330,10 @@ struct PKCS7 { #if defined(HAVE_PKCS7_RSA_RAW_SIGN_CALLBACK) && !defined(NO_RSA) CallbackRsaSignRawDigest rsaSignRawDigestCb; #endif + + /* used by DecodeEnvelopedData with multiple encrypted contents */ + byte* cachedEncryptedContent; + word32 cachedEncryptedContentSz; /* !! NEW DATA MEMBERS MUST BE ADDED AT END !! */ }; From debb79269017a647260c3a97560b467cf8837b35 Mon Sep 17 00:00:00 2001 From: Chris Conlon Date: Fri, 28 Feb 2020 17:55:19 -0700 Subject: [PATCH 2/5] fix PKCS7 encrypted content decoding for streaming API usage --- wolfcrypt/src/pkcs7.c | 117 ++++++++++++++++++++++-------------------- 1 file changed, 62 insertions(+), 55 deletions(-) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 12494d970..9142d7eb1 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -84,6 +84,7 @@ struct PKCS7State { word32 varOne; int varTwo; int varThree; + int varFour; word32 vers; word32 idx; /* index read into current input buffer */ @@ -373,23 +374,25 @@ static long wc_PKCS7_GetMaxStream(PKCS7* pkcs7, byte flag, byte* in, /* setter function for stored variables */ static void wc_PKCS7_StreamStoreVar(PKCS7* pkcs7, word32 var1, int var2, - int var3) + int var3, int var4) { if (pkcs7 != NULL && pkcs7->stream != NULL) { pkcs7->stream->varOne = var1; pkcs7->stream->varTwo = var2; pkcs7->stream->varThree = var3; + pkcs7->stream->varFour = var4; } } /* getter function for stored variables */ static void wc_PKCS7_StreamGetVar(PKCS7* pkcs7, word32* var1, int* var2, - int* var3) + int* var3, int* var4) { if (pkcs7 != NULL && pkcs7->stream != NULL) { if (var1 != NULL) *var1 = pkcs7->stream->varOne; if (var2 != NULL) *var2 = pkcs7->stream->varTwo; if (var3 != NULL) *var3 = pkcs7->stream->varThree; + if (var4 != NULL) *var4 = pkcs7->stream->varFour; } } @@ -4286,7 +4289,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, if (pkiMsg2 && pkiMsg2Sz > 0) { pkcs7->stream->maxLen += pkiMsg2Sz + pkcs7->contentSz; } - wc_PKCS7_StreamStoreVar(pkcs7, totalSz, 0, 0); + wc_PKCS7_StreamStoreVar(pkcs7, totalSz, 0, 0, 0); #endif wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_VERIFY_STAGE2); @@ -4300,7 +4303,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, break; } - wc_PKCS7_StreamGetVar(pkcs7, &totalSz, 0, 0); + wc_PKCS7_StreamGetVar(pkcs7, &totalSz, 0, 0, 0); if (pkcs7->stream->length > 0) pkiMsgSz = pkcs7->stream->length; #ifdef ASN_BER_TO_DER @@ -4458,7 +4461,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, if ((ret = wc_PKCS7_StreamEndCase(pkcs7, &stateIdx, &idx)) != 0) { break; } - wc_PKCS7_StreamStoreVar(pkcs7, pkiMsg2Sz, localIdx, length); + wc_PKCS7_StreamStoreVar(pkcs7, pkiMsg2Sz, localIdx, length, 0); /* content length is in multiple parts */ if (multiPart) { @@ -4489,7 +4492,8 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, else #endif pkiMsgSz = (word32)rc; - wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, (int*)&localIdx, &length); + wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, (int*)&localIdx, + &length, 0); if (pkcs7->stream->length > 0) { localIdx = 0; @@ -4649,7 +4653,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, if ((ret = wc_PKCS7_StreamEndCase(pkcs7, &stateIdx, &idx)) != 0) { break; } - wc_PKCS7_StreamStoreVar(pkcs7, pkiMsg2Sz, 0, length); + wc_PKCS7_StreamStoreVar(pkcs7, pkiMsg2Sz, 0, length, 0); if (length > 0) { pkcs7->stream->expected = length; } @@ -4672,7 +4676,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, break; } - wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, 0, &length); + wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, 0, &length, 0); if (pkcs7->stream->flagOne) { pkiMsg2 = pkiMsg; } @@ -4851,8 +4855,8 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, pkcs7->stream->expected = (pkcs7->stream->maxLen - pkcs7->stream->totalRd) + pkcs7->stream->length; - wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, 0, 0); - wc_PKCS7_StreamStoreVar(pkcs7, pkiMsg2Sz, 0, length); + wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, 0, 0, 0); + wc_PKCS7_StreamStoreVar(pkcs7, pkiMsg2Sz, 0, length, 0); #endif wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_VERIFY_STAGE5); FALL_THROUGH; @@ -4863,7 +4867,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, pkcs7->stream->expected, &pkiMsg, &idx)) != 0) { break; } - wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, 0, &length); + wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, 0, &length, 0); if (pkcs7->stream->flagOne) { pkiMsg2 = pkiMsg; } @@ -4914,7 +4918,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, if ((ret = wc_PKCS7_StreamEndCase(pkcs7, &stateIdx, &idx)) != 0) { break; } - wc_PKCS7_StreamStoreVar(pkcs7, pkiMsg2Sz, 0, length); + wc_PKCS7_StreamStoreVar(pkcs7, pkiMsg2Sz, 0, length, 0); if (in2 && in2Sz > 0 && hashBuf && hashSz > 0) { if (length > 0) { @@ -4941,7 +4945,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, break; } - wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, 0, &length); + wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, 0, &length, 0); if (pkcs7->stream->flagOne) { pkiMsg2 = pkiMsg; } @@ -8022,7 +8026,7 @@ static int wc_PKCS7_DecryptKtri(PKCS7* pkcs7, byte* in, word32 inSz, if ((ret = wc_PKCS7_StreamEndCase(pkcs7, &tmpIdx, idx)) != 0) { break; } - wc_PKCS7_StreamStoreVar(pkcs7, 0, sidType, version); + wc_PKCS7_StreamStoreVar(pkcs7, 0, sidType, version, 0); /* @TODO getting total amount left because of GetInt call later on * this could be optimized to stream better */ @@ -8048,7 +8052,7 @@ static int wc_PKCS7_DecryptKtri(PKCS7* pkcs7, byte* in, word32 inSz, } pkiMsgSz = (word32)rc; - wc_PKCS7_StreamGetVar(pkcs7, NULL, &sidType, &version); + wc_PKCS7_StreamGetVar(pkcs7, NULL, &sidType, &version, 0); /* @TODO get expected size for next part, does not account for * GetInt call well */ @@ -8165,7 +8169,7 @@ static int wc_PKCS7_DecryptKtri(PKCS7* pkcs7, byte* in, word32 inSz, if ((ret = wc_PKCS7_StreamEndCase(pkcs7, &tmpIdx, idx)) != 0) { break; } - wc_PKCS7_StreamStoreVar(pkcs7, encryptedKeySz, sidType, version); + wc_PKCS7_StreamStoreVar(pkcs7, encryptedKeySz, sidType, version, 0); pkcs7->stream->expected = encryptedKeySz; #endif wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_DECRYPT_KTRI_3); @@ -10212,7 +10216,7 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, if ((ret = wc_PKCS7_StreamEndCase(pkcs7, &tmpIdx, &idx)) != 0) { break; } - wc_PKCS7_StreamStoreVar(pkcs7, encOID, expBlockSz, length); + wc_PKCS7_StreamStoreVar(pkcs7, encOID, expBlockSz, length, 0); pkcs7->stream->contentSz = blockKeySz; pkcs7->stream->expected = length + MAX_LENGTH_SZ + MAX_LENGTH_SZ + ASN_TAG_SZ + ASN_TAG_SZ; @@ -10236,7 +10240,7 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, } pkiMsgSz = (word32)rc; - wc_PKCS7_StreamGetVar(pkcs7, 0, 0, &length); + wc_PKCS7_StreamGetVar(pkcs7, 0, 0, &length, 0); tmpIv = pkcs7->stream->tmpIv; if (tmpIv == NULL) { /* check added to help out static analysis tool */ @@ -10269,7 +10273,42 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, ret = ASN_PARSE_E; } - if (ret == 0 && explicitOctet) { + if (ret != 0) + break; + + #ifndef NO_PKCS7_STREAM + if ((ret = wc_PKCS7_StreamEndCase(pkcs7, &tmpIdx, &idx)) != 0) { + break; + } + pkcs7->stream->expected = encryptedContentTotalSz; + wc_PKCS7_StreamGetVar(pkcs7, &encOID, &expBlockSz, 0, 0); + wc_PKCS7_StreamStoreVar(pkcs7, encOID, expBlockSz, explicitOctet, + encryptedContentTotalSz); + #endif + wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_ENV_5); + FALL_THROUGH; + + case WC_PKCS7_ENV_5: + + #ifndef NO_PKCS7_STREAM + if ((ret = wc_PKCS7_AddDataToStream(pkcs7, in, inSz, + pkcs7->stream->expected, &pkiMsg, &idx)) != 0) { + return ret; + } + + wc_PKCS7_StreamGetVar(pkcs7, &encOID, &expBlockSz, &explicitOctet, + &encryptedContentTotalSz); + tmpIv = pkcs7->stream->tmpIv; + + /* restore decrypted key */ + decryptedKey = pkcs7->stream->aad; + decryptedKeySz = pkcs7->stream->aadSz; + blockKeySz = pkcs7->stream->contentSz; + #else + ret = 0; + #endif + + if (explicitOctet) { /* encrypted content may be fragmented into multiple * consecutive OCTET STRINGs, if so loop through * collecting and caching encrypted content bytes */ @@ -10311,39 +10350,6 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, idx += encryptedContentTotalSz; } - if (ret != 0) - break; - - #ifndef NO_PKCS7_STREAM - if ((ret = wc_PKCS7_StreamEndCase(pkcs7, &tmpIdx, &idx)) != 0) { - break; - } - pkcs7->stream->expected = 0; - wc_PKCS7_StreamGetVar(pkcs7, &encOID, &expBlockSz, 0); - wc_PKCS7_StreamStoreVar(pkcs7, encOID, expBlockSz, 0); - #endif - wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_ENV_5); - FALL_THROUGH; - - case WC_PKCS7_ENV_5: - - #ifndef NO_PKCS7_STREAM - if ((ret = wc_PKCS7_AddDataToStream(pkcs7, in, inSz, - pkcs7->stream->expected, &pkiMsg, &idx)) != 0) { - return ret; - } - - wc_PKCS7_StreamGetVar(pkcs7, &encOID, &expBlockSz, NULL); - tmpIv = pkcs7->stream->tmpIv; - - /* restore decrypted key */ - decryptedKey = pkcs7->stream->aad; - decryptedKeySz = pkcs7->stream->aadSz; - blockKeySz = pkcs7->stream->contentSz; - #else - ret = 0; - #endif - /* use cached content */ encryptedContent = pkcs7->cachedEncryptedContent; encryptedContentSz = pkcs7->cachedEncryptedContentSz; @@ -11108,7 +11114,7 @@ WOLFSSL_API int wc_PKCS7_DecodeAuthEnvelopedData(PKCS7* pkcs7, byte* in, if ((ret = wc_PKCS7_StreamEndCase(pkcs7, &tmpIdx, &idx)) != 0) { break; } - wc_PKCS7_StreamStoreVar(pkcs7, encOID, blockKeySz, 0); + wc_PKCS7_StreamStoreVar(pkcs7, encOID, blockKeySz, 0, 0); #endif wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_AUTHENV_4); FALL_THROUGH; @@ -11211,7 +11217,7 @@ WOLFSSL_API int wc_PKCS7_DecodeAuthEnvelopedData(PKCS7* pkcs7, byte* in, pkcs7->stream->expected = encryptedContentSz; wc_PKCS7_StreamStoreVar(pkcs7, encOID, blockKeySz, - encryptedContentSz); + encryptedContentSz, 0); #endif wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_AUTHENV_5); @@ -11456,7 +11462,8 @@ authenv_atrbend: encodedAttribs = pkcs7->stream->aad; } - wc_PKCS7_StreamGetVar(pkcs7, &encOID, &blockKeySz, &encryptedContentSz); + wc_PKCS7_StreamGetVar(pkcs7, &encOID, &blockKeySz, + &encryptedContentSz, 0); encryptedContent = pkcs7->stream->bufferPt; #ifdef WOLFSSL_SMALL_STACK decryptedKey = pkcs7->stream->key; From d8eeefb4b7394fd43cd70d8b645ee698c8108e3b Mon Sep 17 00:00:00 2001 From: Chris Conlon Date: Mon, 2 Mar 2020 09:13:11 -0700 Subject: [PATCH 3/5] initialize explicitOctet to 0 in pwc_PKCS7_DecodeEnvelopedData() --- wolfcrypt/src/pkcs7.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 9142d7eb1..54c2eae18 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -10057,7 +10057,7 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, int encryptedContentSz = 0; byte padLen; byte* encryptedContent = NULL; - int explicitOctet; + int explicitOctet = 0; word32 localIdx; byte tag; From 44d2fc55e6bc4adcd1241146f396ff5600d4811f Mon Sep 17 00:00:00 2001 From: Chris Conlon Date: Tue, 3 Mar 2020 10:27:22 -0700 Subject: [PATCH 4/5] scan-build fixes for wc_PKCS7_DecodeEnvelopedData() --- wolfcrypt/src/pkcs7.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 54c2eae18..27fa23a60 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -10317,22 +10317,22 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, if (GetASNTag(pkiMsg, &idx, &tag, pkiMsgSz) < 0) { ret = ASN_PARSE_E; - break; } - if (tag != ASN_OCTET_STRING) { + if (ret == 0 && (tag != ASN_OCTET_STRING)) { ret = ASN_PARSE_E; - break; } if (ret == 0 && GetLength(pkiMsg, &idx, &encryptedContentSz, pkiMsgSz) <= 0) { ret = ASN_PARSE_E; - break; } - ret = PKCS7_CacheEncryptedContent(pkcs7, &pkiMsg[idx], - encryptedContentSz); + if (ret == 0) { + ret = PKCS7_CacheEncryptedContent(pkcs7, &pkiMsg[idx], + encryptedContentSz); + } + if (ret != 0) { break; } @@ -10340,6 +10340,11 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, /* advance idx past encrypted content */ idx += encryptedContentSz; } + + if (ret != 0) { + break; + } + } else { /* cache encrypted content, no OCTET STRING */ ret = PKCS7_CacheEncryptedContent(pkcs7, &pkiMsg[idx], From 4ad8a2bacb1a7e0914c57f9967aa2fece2000cfc Mon Sep 17 00:00:00 2001 From: Chris Conlon Date: Fri, 6 Mar 2020 10:50:00 -0700 Subject: [PATCH 5/5] store wc_PKCS7_DecodeEnvelopedData encryptedContentTotalSz in existing variable instead of adding another --- wolfcrypt/src/pkcs7.c | 55 ++++++++++++++++++++----------------------- 1 file changed, 25 insertions(+), 30 deletions(-) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 27fa23a60..dbebf2534 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -84,7 +84,6 @@ struct PKCS7State { word32 varOne; int varTwo; int varThree; - int varFour; word32 vers; word32 idx; /* index read into current input buffer */ @@ -374,25 +373,23 @@ static long wc_PKCS7_GetMaxStream(PKCS7* pkcs7, byte flag, byte* in, /* setter function for stored variables */ static void wc_PKCS7_StreamStoreVar(PKCS7* pkcs7, word32 var1, int var2, - int var3, int var4) + int var3) { if (pkcs7 != NULL && pkcs7->stream != NULL) { pkcs7->stream->varOne = var1; pkcs7->stream->varTwo = var2; pkcs7->stream->varThree = var3; - pkcs7->stream->varFour = var4; } } /* getter function for stored variables */ static void wc_PKCS7_StreamGetVar(PKCS7* pkcs7, word32* var1, int* var2, - int* var3, int* var4) + int* var3) { if (pkcs7 != NULL && pkcs7->stream != NULL) { if (var1 != NULL) *var1 = pkcs7->stream->varOne; if (var2 != NULL) *var2 = pkcs7->stream->varTwo; if (var3 != NULL) *var3 = pkcs7->stream->varThree; - if (var4 != NULL) *var4 = pkcs7->stream->varFour; } } @@ -4289,7 +4286,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, if (pkiMsg2 && pkiMsg2Sz > 0) { pkcs7->stream->maxLen += pkiMsg2Sz + pkcs7->contentSz; } - wc_PKCS7_StreamStoreVar(pkcs7, totalSz, 0, 0, 0); + wc_PKCS7_StreamStoreVar(pkcs7, totalSz, 0, 0); #endif wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_VERIFY_STAGE2); @@ -4303,7 +4300,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, break; } - wc_PKCS7_StreamGetVar(pkcs7, &totalSz, 0, 0, 0); + wc_PKCS7_StreamGetVar(pkcs7, &totalSz, 0, 0); if (pkcs7->stream->length > 0) pkiMsgSz = pkcs7->stream->length; #ifdef ASN_BER_TO_DER @@ -4461,7 +4458,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, if ((ret = wc_PKCS7_StreamEndCase(pkcs7, &stateIdx, &idx)) != 0) { break; } - wc_PKCS7_StreamStoreVar(pkcs7, pkiMsg2Sz, localIdx, length, 0); + wc_PKCS7_StreamStoreVar(pkcs7, pkiMsg2Sz, localIdx, length); /* content length is in multiple parts */ if (multiPart) { @@ -4492,8 +4489,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, else #endif pkiMsgSz = (word32)rc; - wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, (int*)&localIdx, - &length, 0); + wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, (int*)&localIdx, &length); if (pkcs7->stream->length > 0) { localIdx = 0; @@ -4653,7 +4649,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, if ((ret = wc_PKCS7_StreamEndCase(pkcs7, &stateIdx, &idx)) != 0) { break; } - wc_PKCS7_StreamStoreVar(pkcs7, pkiMsg2Sz, 0, length, 0); + wc_PKCS7_StreamStoreVar(pkcs7, pkiMsg2Sz, 0, length); if (length > 0) { pkcs7->stream->expected = length; } @@ -4676,7 +4672,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, break; } - wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, 0, &length, 0); + wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, 0, &length); if (pkcs7->stream->flagOne) { pkiMsg2 = pkiMsg; } @@ -4855,8 +4851,8 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, pkcs7->stream->expected = (pkcs7->stream->maxLen - pkcs7->stream->totalRd) + pkcs7->stream->length; - wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, 0, 0, 0); - wc_PKCS7_StreamStoreVar(pkcs7, pkiMsg2Sz, 0, length, 0); + wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, 0, 0); + wc_PKCS7_StreamStoreVar(pkcs7, pkiMsg2Sz, 0, length); #endif wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_VERIFY_STAGE5); FALL_THROUGH; @@ -4867,7 +4863,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, pkcs7->stream->expected, &pkiMsg, &idx)) != 0) { break; } - wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, 0, &length, 0); + wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, 0, &length); if (pkcs7->stream->flagOne) { pkiMsg2 = pkiMsg; } @@ -4918,7 +4914,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, if ((ret = wc_PKCS7_StreamEndCase(pkcs7, &stateIdx, &idx)) != 0) { break; } - wc_PKCS7_StreamStoreVar(pkcs7, pkiMsg2Sz, 0, length, 0); + wc_PKCS7_StreamStoreVar(pkcs7, pkiMsg2Sz, 0, length); if (in2 && in2Sz > 0 && hashBuf && hashSz > 0) { if (length > 0) { @@ -4945,7 +4941,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, break; } - wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, 0, &length, 0); + wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, 0, &length); if (pkcs7->stream->flagOne) { pkiMsg2 = pkiMsg; } @@ -8026,7 +8022,7 @@ static int wc_PKCS7_DecryptKtri(PKCS7* pkcs7, byte* in, word32 inSz, if ((ret = wc_PKCS7_StreamEndCase(pkcs7, &tmpIdx, idx)) != 0) { break; } - wc_PKCS7_StreamStoreVar(pkcs7, 0, sidType, version, 0); + wc_PKCS7_StreamStoreVar(pkcs7, 0, sidType, version); /* @TODO getting total amount left because of GetInt call later on * this could be optimized to stream better */ @@ -8052,7 +8048,7 @@ static int wc_PKCS7_DecryptKtri(PKCS7* pkcs7, byte* in, word32 inSz, } pkiMsgSz = (word32)rc; - wc_PKCS7_StreamGetVar(pkcs7, NULL, &sidType, &version, 0); + wc_PKCS7_StreamGetVar(pkcs7, NULL, &sidType, &version); /* @TODO get expected size for next part, does not account for * GetInt call well */ @@ -8169,7 +8165,7 @@ static int wc_PKCS7_DecryptKtri(PKCS7* pkcs7, byte* in, word32 inSz, if ((ret = wc_PKCS7_StreamEndCase(pkcs7, &tmpIdx, idx)) != 0) { break; } - wc_PKCS7_StreamStoreVar(pkcs7, encryptedKeySz, sidType, version, 0); + wc_PKCS7_StreamStoreVar(pkcs7, encryptedKeySz, sidType, version); pkcs7->stream->expected = encryptedKeySz; #endif wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_DECRYPT_KTRI_3); @@ -10216,7 +10212,7 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, if ((ret = wc_PKCS7_StreamEndCase(pkcs7, &tmpIdx, &idx)) != 0) { break; } - wc_PKCS7_StreamStoreVar(pkcs7, encOID, expBlockSz, length, 0); + wc_PKCS7_StreamStoreVar(pkcs7, encOID, expBlockSz, length); pkcs7->stream->contentSz = blockKeySz; pkcs7->stream->expected = length + MAX_LENGTH_SZ + MAX_LENGTH_SZ + ASN_TAG_SZ + ASN_TAG_SZ; @@ -10240,7 +10236,7 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, } pkiMsgSz = (word32)rc; - wc_PKCS7_StreamGetVar(pkcs7, 0, 0, &length, 0); + wc_PKCS7_StreamGetVar(pkcs7, 0, 0, &length); tmpIv = pkcs7->stream->tmpIv; if (tmpIv == NULL) { /* check added to help out static analysis tool */ @@ -10281,9 +10277,8 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, break; } pkcs7->stream->expected = encryptedContentTotalSz; - wc_PKCS7_StreamGetVar(pkcs7, &encOID, &expBlockSz, 0, 0); - wc_PKCS7_StreamStoreVar(pkcs7, encOID, expBlockSz, explicitOctet, - encryptedContentTotalSz); + wc_PKCS7_StreamGetVar(pkcs7, &encOID, &expBlockSz, 0); + wc_PKCS7_StreamStoreVar(pkcs7, encOID, expBlockSz, explicitOctet); #endif wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_ENV_5); FALL_THROUGH; @@ -10296,9 +10291,9 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, return ret; } - wc_PKCS7_StreamGetVar(pkcs7, &encOID, &expBlockSz, &explicitOctet, - &encryptedContentTotalSz); + wc_PKCS7_StreamGetVar(pkcs7, &encOID, &expBlockSz, &explicitOctet); tmpIv = pkcs7->stream->tmpIv; + encryptedContentTotalSz = pkcs7->stream->expected; /* restore decrypted key */ decryptedKey = pkcs7->stream->aad; @@ -11119,7 +11114,7 @@ WOLFSSL_API int wc_PKCS7_DecodeAuthEnvelopedData(PKCS7* pkcs7, byte* in, if ((ret = wc_PKCS7_StreamEndCase(pkcs7, &tmpIdx, &idx)) != 0) { break; } - wc_PKCS7_StreamStoreVar(pkcs7, encOID, blockKeySz, 0, 0); + wc_PKCS7_StreamStoreVar(pkcs7, encOID, blockKeySz, 0); #endif wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_AUTHENV_4); FALL_THROUGH; @@ -11222,7 +11217,7 @@ WOLFSSL_API int wc_PKCS7_DecodeAuthEnvelopedData(PKCS7* pkcs7, byte* in, pkcs7->stream->expected = encryptedContentSz; wc_PKCS7_StreamStoreVar(pkcs7, encOID, blockKeySz, - encryptedContentSz, 0); + encryptedContentSz); #endif wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_AUTHENV_5); @@ -11468,7 +11463,7 @@ authenv_atrbend: } wc_PKCS7_StreamGetVar(pkcs7, &encOID, &blockKeySz, - &encryptedContentSz, 0); + &encryptedContentSz); encryptedContent = pkcs7->stream->bufferPt; #ifdef WOLFSSL_SMALL_STACK decryptedKey = pkcs7->stream->key;