From 3a0afc35064ff3ce476f2f49490fe3793447d931 Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 31 Jan 2019 14:36:46 -0800 Subject: [PATCH 1/4] Fixes to handle degenerate PKCS 7 with BER encoding in `PKCS7_VerifySignedData`. Fix for PKCS7 API unit test with SHA512 disabled. ZD 4757. --- tests/api.c | 2 +- wolfcrypt/src/pkcs7.c | 43 ++++++++++++++++++++++++++++++++----------- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/tests/api.c b/tests/api.c index a12611441..6d10feb04 100644 --- a/tests/api.c +++ b/tests/api.c @@ -15886,7 +15886,7 @@ static void test_wc_PKCS7_EncodeDecodeEnvelopedData (void) AES256_WRAP, dhSinglePass_stdDH_sha256kdf_scheme, eccCert, eccCertSz, eccPrivKey, eccPrivKeySz}, #endif - #if !defined(WOLFSSL_SHA512) && !defined(NO_AES_256) + #if defined(WOLFSSL_SHA512) && !defined(NO_AES_256) {(byte*)input, (word32)(sizeof(input)/sizeof(char)), DATA, AES256CBCb, AES256_WRAP, dhSinglePass_stdDH_sha512kdf_scheme, eccCert, eccCertSz, eccPrivKey, eccPrivKeySz}, diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 6d2e599a5..e6d25c026 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -3366,7 +3366,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, byte* in2, word32 in2Sz) { word32 idx, outerContentType, hashOID = 0, sigOID, contentTypeSz = 0, totalSz = 0; - int length, version, ret = 0; + int length = 0, version, ret = 0; byte* content = NULL; byte* contentDynamic = NULL; byte* sig = NULL; @@ -3430,8 +3430,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, } { - long rc; - rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_SEQ_PEEK, in, inSz); + long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_SEQ_PEEK, in, inSz); if (rc < 0) { ret = (int)rc; break; @@ -3471,6 +3470,17 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, if (GetSequence_ex(pkiMsg, &idx, &length, pkiMsgSz, NO_USER_CHECK) < 0) return ASN_PARSE_E; + + #ifndef NO_PKCS7_STREAM + { + long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_SEQ_PEEK, + pkiMsg, pkiMsgSz); + if (rc < 0) { + ret = (int)rc; + break; + } + } + #endif #else ret = BER_INDEF_E; #endif @@ -3539,14 +3549,15 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, case WC_PKCS7_VERIFY_STAGE2: #ifndef NO_PKCS7_STREAM - if ((ret = wc_PKCS7_AddDataToStream(pkcs7, in, inSz + in2Sz, + if ((ret = wc_PKCS7_AddDataToStream(pkcs7, pkiMsg, inSz + in2Sz, MAX_SEQ_SZ + MAX_OID_SZ + ASN_TAG_SZ + MAX_LENGTH_SZ + ASN_TAG_SZ + MAX_LENGTH_SZ, &pkiMsg, &idx)) != 0) { break; } wc_PKCS7_StreamGetVar(pkcs7, &totalSz, 0, 0); - pkiMsgSz = (pkcs7->stream->length > 0)? pkcs7->stream->length :inSz; + if (pkcs7->stream->length > 0) + pkiMsgSz = pkcs7->stream->length; #endif /* Get the inner ContentInfo sequence */ if (GetSequence_ex(pkiMsg, &idx, &length, pkiMsgSz, @@ -3684,14 +3695,14 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, case WC_PKCS7_VERIFY_STAGE3: #ifndef NO_PKCS7_STREAM - if ((ret = wc_PKCS7_AddDataToStream(pkcs7, in, inSz + in2Sz, + if ((ret = wc_PKCS7_AddDataToStream(pkcs7, pkiMsg, inSz + in2Sz, pkcs7->stream->expected, &pkiMsg, &idx)) != 0) { break; } { long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, - in, inSz); + pkiMsg, pkiMsgSz); if (rc < 0) { ret = (int)rc; break; @@ -3709,7 +3720,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, /* Break out before content because it can be optional in degenerate * cases. */ - if (ret != 0 && !detached) + if (ret != 0 && !degenerate) break; /* get parts of content */ @@ -3860,7 +3871,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, case WC_PKCS7_VERIFY_STAGE4: #ifndef NO_PKCS7_STREAM - if ((ret = wc_PKCS7_AddDataToStream(pkcs7, in, inSz + in2Sz, + if ((ret = wc_PKCS7_AddDataToStream(pkcs7, pkiMsg, inSz + in2Sz, pkcs7->stream->expected, &pkiMsg, &idx)) != 0) { break; } @@ -4041,7 +4052,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, case WC_PKCS7_VERIFY_STAGE5: #ifndef NO_PKCS7_STREAM - if ((ret = wc_PKCS7_AddDataToStream(pkcs7, in, inSz, + if ((ret = wc_PKCS7_AddDataToStream(pkcs7, pkiMsg, inSz + in2Sz, pkcs7->stream->expected, &pkiMsg, &idx)) != 0) { break; } @@ -4101,7 +4112,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, case WC_PKCS7_VERIFY_STAGE6: #ifndef NO_PKCS7_STREAM - if ((ret = wc_PKCS7_AddDataToStream(pkcs7, in, inSz + in2Sz, + if ((ret = wc_PKCS7_AddDataToStream(pkcs7, pkiMsg, inSz + in2Sz, pkcs7->stream->expected, &pkiMsg, &idx)) != 0) { break; } @@ -9117,6 +9128,8 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, } pkiMsgSz = (word32)rc; } + #else + ret = 0; #endif /* remove EncryptedContentInfo */ @@ -9196,6 +9209,8 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, ret = MEMORY_E; break; } + #else + ret = 0; #endif XMEMCPY(tmpIv, &pkiMsg[idx], length); @@ -9260,6 +9275,8 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, decryptedKey = pkcs7->stream->aad; decryptedKeySz = pkcs7->stream->aadSz; blockKeySz = pkcs7->stream->contentSz; + #else + ret = 0; #endif encryptedContent = (byte*)XMALLOC(encryptedContentSz, pkcs7->heap, DYNAMIC_TYPE_PKCS7); @@ -10196,6 +10213,8 @@ WOLFSSL_API int wc_PKCS7_DecodeAuthEnvelopedData(PKCS7* pkcs7, byte* in, length = pkcs7->stream->expected; encodedAttribs = pkcs7->stream->aad; + #else + length = 0; #endif /* save pointer and length */ @@ -10990,6 +11009,8 @@ int wc_PKCS7_DecodeEncryptedData(PKCS7* pkcs7, byte* in, word32 inSz, encryptedContentSz = pkcs7->stream->varThree; version = pkcs7->stream->vers; tmpIv = pkcs7->stream->tmpIv; +#else + encOID = 0; #endif if (ret == 0 && (encryptedContent = (byte*)XMALLOC( encryptedContentSz, pkcs7->heap, DYNAMIC_TYPE_PKCS7)) == NULL) From c82d11f47dd07bce4a00539e2d08127c7fc3c684 Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 31 Jan 2019 14:37:25 -0800 Subject: [PATCH 2/4] Cleanup of the PKCS7 stream `long rc` and braces. --- wolfcrypt/src/pkcs7.c | 375 +++++++++++++++++++----------------------- 1 file changed, 169 insertions(+), 206 deletions(-) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index e6d25c026..2daf2f5d0 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -3387,6 +3387,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, word32 pkiMsgSz = inSz; #ifndef NO_PKCS7_STREAM word32 stateIdx = 0; + long rc; #endif byte* pkiMsg2 = in2; @@ -3429,12 +3430,10 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, break; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_SEQ_PEEK, in, inSz); - if (rc < 0) { - ret = (int)rc; - break; - } + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_SEQ_PEEK, in, inSz); + if (rc < 0) { + ret = (int)rc; + break; } pkiMsgSz = (pkcs7->stream->length > 0)? pkcs7->stream->length :inSz; #endif @@ -3472,13 +3471,11 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, return ASN_PARSE_E; #ifndef NO_PKCS7_STREAM - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_SEQ_PEEK, - pkiMsg, pkiMsgSz); - if (rc < 0) { - ret = (int)rc; - break; - } + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_SEQ_PEEK, + pkiMsg, pkiMsgSz); + if (rc < 0) { + ret = (int)rc; + break; } #endif #else @@ -3700,15 +3697,14 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, break; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, - pkiMsg, pkiMsgSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, + pkiMsg, pkiMsgSz); + if (rc < 0) { + ret = (int)rc; + break; } + if (pkcs7->stream->length > 0) + pkiMsgSz = (word32)rc; wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, (int*)&localIdx, &length); if (pkcs7->stream->length > 0) { @@ -7160,6 +7156,7 @@ static int wc_PKCS7_DecryptKtri(PKCS7* pkcs7, byte* in, word32 inSz, #ifndef NO_PKCS7_STREAM word32 tmpIdx = *idx; + long rc; #endif #ifdef WC_RSA_BLINDING WC_RNG rng; @@ -7183,15 +7180,13 @@ static int wc_PKCS7_DecryptKtri(PKCS7* pkcs7, byte* in, word32 inSz, return ret; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, - in, inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, + in, inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; #endif if (GetMyVersion(pkiMsg, idx, &version, pkiMsgSz) < 0) @@ -7227,15 +7222,14 @@ static int wc_PKCS7_DecryptKtri(PKCS7* pkcs7, byte* in, word32 inSz, return ret; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, - in, inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, + in, inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; + wc_PKCS7_StreamGetVar(pkcs7, NULL, &sidType, &version); /* @TODO get expected size for next part, does not account for @@ -7264,7 +7258,7 @@ static int wc_PKCS7_DecryptKtri(PKCS7* pkcs7, byte* in, word32 inSz, return WC_PKCS7_WANT_READ_E; } } - #endif + #endif /* !NO_PKCS7_STREAM */ if (sidType == CMS_ISSUER_AND_SERIAL_NUMBER) { @@ -7861,6 +7855,7 @@ static int wc_PKCS7_DecryptOri(PKCS7* pkcs7, byte* in, word32 inSz, word32 pkiMsgSz = inSz; #ifndef NO_PKCS7_STREAM word32 stateIdx = *idx; + long rc; #endif if (pkcs7->oriDecryptCb == NULL) { @@ -7879,15 +7874,13 @@ static int wc_PKCS7_DecryptOri(PKCS7* pkcs7, byte* in, word32 inSz, return ret; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, - inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, + inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; #endif /* get OtherRecipientInfo sequence length */ if (GetLength(pkiMsg, idx, &seqSz, pkiMsgSz) < 0) @@ -7960,6 +7953,7 @@ static int wc_PKCS7_DecryptPwri(PKCS7* pkcs7, byte* in, word32 inSz, word32 pkiMsgSz = inSz; #ifndef NO_PKCS7_STREAM word32 tmpIdx = *idx; + long rc; #endif switch (pkcs7->state) { @@ -7972,15 +7966,13 @@ static int wc_PKCS7_DecryptPwri(PKCS7* pkcs7, byte* in, word32 inSz, return ret; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, - inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, + inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; #endif /* remove KeyDerivationAlgorithmIdentifier */ if (pkiMsg[(*idx)++] != (ASN_CONSTRUCTED | ASN_CONTEXT_SPECIFIC | 0)) @@ -8175,6 +8167,7 @@ static int wc_PKCS7_DecryptKekri(PKCS7* pkcs7, byte* in, word32 inSz, word32 pkiMsgSz = inSz; #ifndef NO_PKCS7_STREAM word32 tmpIdx = *idx; + long rc; #endif switch (pkcs7->state) { @@ -8187,15 +8180,13 @@ static int wc_PKCS7_DecryptKekri(PKCS7* pkcs7, byte* in, word32 inSz, return ret; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, - inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, + inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; #endif /* remove KEKIdentifier */ if (GetSequence(pkiMsg, idx, &length, pkiMsgSz) < 0) @@ -8304,7 +8295,8 @@ static int wc_PKCS7_DecryptKari(PKCS7* pkcs7, byte* in, word32 inSz, byte* pkiMsg = in; word32 pkiMsgSz = inSz; #ifndef NO_PKCS7_STREAM - word32 tmpIdx = (idx)? *idx : 0; + word32 tmpIdx = (idx) ? *idx : 0; + long rc; #endif if (pkcs7 == NULL || pkcs7->singleCert == NULL || @@ -8323,15 +8315,13 @@ static int wc_PKCS7_DecryptKari(PKCS7* pkcs7, byte* in, word32 inSz, return ret; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, - inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, + inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; #endif WC_PKCS7_KARI* kari; @@ -8503,6 +8493,7 @@ static int wc_PKCS7_DecryptRecipientInfos(PKCS7* pkcs7, byte* in, word32 pkiMsgSz = inSz; #ifndef NO_PKCS7_STREAM word32 tmpIdx = *idx; + long rc; #endif if (pkcs7 == NULL || pkiMsg == NULL || idx == NULL || @@ -8559,14 +8550,13 @@ static int wc_PKCS7_DecryptRecipientInfos(PKCS7* pkcs7, byte* in, savedIdx = *idx; #ifndef NO_PKCS7_STREAM - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, inSz); - if (rc < 0) { - return (int)rc; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, inSz); + if (rc < 0) { + return (int)rc; } - if (pkcs7->stream->length > 0) pkiMsg = pkcs7->stream->buffer; + pkiMsgSz = (word32)rc; + if (pkcs7->stream->length > 0) + pkiMsg = pkcs7->stream->buffer; #endif /* when looking for next recipient, use first sequence and version to @@ -8733,6 +8723,7 @@ static int wc_PKCS7_ParseToRecipientInfoSet(PKCS7* pkcs7, byte* in, word32 pkiMsgSz = inSz; #ifndef NO_PKCS7_STREAM word32 tmpIdx = 0; + long rc; #endif if (pkcs7 == NULL || pkiMsg == NULL || pkiMsgSz == 0 || idx == NULL) @@ -8770,15 +8761,12 @@ static int wc_PKCS7_ParseToRecipientInfoSet(PKCS7* pkcs7, byte* in, return ret; } - { - long rc; - rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_SEQ_PEEK, in, inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_SEQ_PEEK, in, inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; #endif /* read past ContentInfo, verify type is envelopedData */ if (ret == 0 && GetSequence(pkiMsg, idx, &length, pkiMsgSz) < 0) @@ -8802,15 +8790,13 @@ static int wc_PKCS7_ParseToRecipientInfoSet(PKCS7* pkcs7, byte* in, return ret; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, - in, inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, + in, inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; #endif len = 0; @@ -8891,15 +8877,13 @@ static int wc_PKCS7_ParseToRecipientInfoSet(PKCS7* pkcs7, byte* in, return ret; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, - inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, + inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; #endif /* remove EnvelopedData and version */ if (ret == 0 && GetSequence(pkiMsg, idx, &length, pkiMsgSz) < 0) @@ -8928,15 +8912,13 @@ static int wc_PKCS7_ParseToRecipientInfoSet(PKCS7* pkcs7, byte* in, return ret; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, - inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, + inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; version = pkcs7->stream->varOne; #endif @@ -9014,6 +8996,7 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, word32 idx = 0; #ifndef NO_PKCS7_STREAM word32 tmpIdx = 0; + long rc; #endif word32 contentType, encOID = 0; word32 decryptedKeySz = MAX_ENCRYPTED_KEY_SZ; @@ -9119,15 +9102,13 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, return ret; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, - inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, + inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; #else ret = 0; #endif @@ -9193,15 +9174,14 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* in, return ret; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, - inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, + inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; + wc_PKCS7_StreamGetVar(pkcs7, 0, 0, &length); tmpIv = pkcs7->stream->tmpIv; if (tmpIv == NULL) { @@ -9854,6 +9834,7 @@ WOLFSSL_API int wc_PKCS7_DecodeAuthEnvelopedData(PKCS7* pkcs7, byte* in, word32 idx = 0; #ifndef NO_PKCS7_STREAM word32 tmpIdx = 0; + long rc; #endif word32 contentType, encOID = 0; word32 decryptedKeySz = 0; @@ -9973,15 +9954,13 @@ WOLFSSL_API int wc_PKCS7_DecodeAuthEnvelopedData(PKCS7* pkcs7, byte* in, break; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, - in, inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, + in, inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; #endif /* remove EncryptedContentInfo */ @@ -10035,15 +10014,13 @@ WOLFSSL_API int wc_PKCS7_DecodeAuthEnvelopedData(PKCS7* pkcs7, byte* in, break; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, - inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, + inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; #endif if (ret == 0 && GetLength(pkiMsg, &idx, &nonceSz, pkiMsgSz) < 0) { ret = ASN_PARSE_E; @@ -10130,15 +10107,14 @@ WOLFSSL_API int wc_PKCS7_DecodeAuthEnvelopedData(PKCS7* pkcs7, byte* in, break; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, - inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, + inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; + encryptedContentSz = pkcs7->stream->expected; #endif @@ -10250,15 +10226,14 @@ authenv_atrbend: return ret; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, - in, inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, + in, inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; + if (pkcs7->stream->aadSz > 0) { encodedAttribSz = pkcs7->stream->aadSz; encodedAttribs = pkcs7->stream->aad; @@ -10722,6 +10697,7 @@ int wc_PKCS7_DecodeEncryptedData(PKCS7* pkcs7, byte* in, word32 inSz, #ifndef NO_PKCS7_STREAM word32 tmpIdx = 0; + long rc; #endif word32 contentType, encOID; @@ -10761,15 +10737,12 @@ int wc_PKCS7_DecodeEncryptedData(PKCS7* pkcs7, byte* in, word32 inSz, return ret; } - { - long rc; - rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_SEQ_PEEK, in, inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_SEQ_PEEK, in, inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; #endif /* read past ContentInfo, verify type is encrypted-data */ @@ -10803,15 +10776,13 @@ int wc_PKCS7_DecodeEncryptedData(PKCS7* pkcs7, byte* in, word32 inSz, return ret; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, - inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, + inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; #endif if (ret == 0 && pkiMsg[idx++] != (ASN_CONSTRUCTED | ASN_CONTEXT_SPECIFIC | 0)) @@ -10842,15 +10813,13 @@ int wc_PKCS7_DecodeEncryptedData(PKCS7* pkcs7, byte* in, word32 inSz, return ret; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, - inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, + inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; #endif /* get version, check later */ haveAttribs = 0; @@ -10896,15 +10865,13 @@ int wc_PKCS7_DecodeEncryptedData(PKCS7* pkcs7, byte* in, word32 inSz, return ret; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, - inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, + inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; /* restore saved variables */ expBlockSz = pkcs7->stream->varOne; @@ -10941,15 +10908,13 @@ int wc_PKCS7_DecodeEncryptedData(PKCS7* pkcs7, byte* in, word32 inSz, return ret; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, - inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, + inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; /* use IV buffer from stream structure */ tmpIv = pkcs7->stream->tmpIv; @@ -10993,15 +10958,13 @@ int wc_PKCS7_DecodeEncryptedData(PKCS7* pkcs7, byte* in, word32 inSz, return ret; } - { - long rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, - inSz); - if (rc < 0) { - ret = (int)rc; - break; - } - pkiMsgSz = (word32)rc; + rc = wc_PKCS7_GetMaxStream(pkcs7, PKCS7_DEFAULT_PEEK, in, + inSz); + if (rc < 0) { + ret = (int)rc; + break; } + pkiMsgSz = (word32)rc; /* restore saved variables */ expBlockSz = pkcs7->stream->varOne; From ec28376e7f6f10e21b1515f24bfd433ea376ef8e Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Wed, 6 Feb 2019 11:05:15 -0700 Subject: [PATCH 3/4] add PKCS7 BER verify test and fix for streaming --- certs/include.am | 1 + tests/api.c | 32 ++++++++++++++++++++++++++++++++ wolfcrypt/src/pkcs7.c | 38 ++++++++++++++++++++++++++++++-------- wolfssl/wolfcrypt/pkcs7.h | 1 + 4 files changed, 64 insertions(+), 8 deletions(-) diff --git a/certs/include.am b/certs/include.am index 53bcb581c..c159b8e38 100644 --- a/certs/include.am +++ b/certs/include.am @@ -41,6 +41,7 @@ EXTRA_DIST += \ certs/server-revoked-key.pem \ certs/wolfssl-website-ca.pem \ certs/test-degenerate.p7b \ + certs/test-ber-exp02-05-2022.p7b \ certs/test-servercert.p12 \ certs/ecc-rsa-server.p12 \ certs/dsaparams.pem \ diff --git a/tests/api.c b/tests/api.c index 6d10feb04..98acf7456 100644 --- a/tests/api.c +++ b/tests/api.c @@ -16215,6 +16215,37 @@ static void test_wc_PKCS7_Degenerate(void) #endif } /* END test_wc_PKCS7_Degenerate() */ +/* + * Testing wc_PKCS7_BER() + */ +static void test_wc_PKCS7_BER(void) +{ +#if defined(HAVE_PKCS7) && !defined(NO_FILESYSTEM) && \ + defined(ASN_BER_TO_DER) + PKCS7* pkcs7; + char fName[] = "./certs/test-ber-exp02-05-2022.p7b"; + XFILE f; + byte der[4096]; + word32 derSz; + int ret; + + printf(testingFmt, "wc_PKCS7_BER()"); + + AssertNotNull(f = XFOPEN(fName, "rb")); + AssertIntGT((ret = (int)fread(der, 1, sizeof(der), f)), 0); + derSz = (word32)ret; + XFCLOSE(f); + + AssertNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, devId)); + AssertIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0); + AssertIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0); + AssertIntEQ(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), 0); + wc_PKCS7_Free(pkcs7); + + printf(resultFmt, passed); +#endif +} /* END test_wc_PKCS7_BER() */ + /* Testing wc_SignatureGetSize() for signature type ECC */ static int test_wc_SignatureGetSize_ecc(void) @@ -23587,6 +23618,7 @@ void ApiTest(void) test_wc_PKCS7_EncodeDecodeEnvelopedData(); test_wc_PKCS7_EncodeEncryptedData(); test_wc_PKCS7_Degenerate(); + test_wc_PKCS7_BER(); test_wolfSSL_CTX_LoadCRL(); diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 2daf2f5d0..ee14b38c2 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -265,6 +265,11 @@ static int wc_PKCS7_AddDataToStream(PKCS7* pkcs7, byte* in, word32 inSz, if (inSz - rdSz > 0 && pkcs7->stream->length < expected) { int len = min(inSz - rdSz, expected - pkcs7->stream->length); + /* sanity check that the input buffer is not internal buffer */ + if (in == pkcs7->stream->buffer) { + return WC_PKCS7_WANT_READ_E; + } + /* check if internal buffer size needs to be increased */ if (len + pkcs7->stream->length > pkcs7->stream->bufferSz) { int ret = wc_PKCS7_GrowStream(pkcs7, expected); @@ -3412,6 +3417,12 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, } idx = 0; +#ifdef ASN_BER_TO_DER + if (pkcs7->derSz > 0 && pkcs7->der) { + pkiMsg = in = pkcs7->der; + } +#endif + #ifndef NO_PKCS7_STREAM if (pkcs7->stream == NULL) { if ((ret = wc_PKCS7_CreateStream(pkcs7)) != 0) { @@ -3463,8 +3474,8 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, if (ret < 0) return ret; - pkiMsg = pkcs7->der; - pkiMsgSz = len; + pkiMsg = in = pkcs7->der; + pkiMsgSz = pkcs7->derSz = len; idx = 0; if (GetSequence_ex(pkiMsg, &idx, &length, pkiMsgSz, NO_USER_CHECK) < 0) @@ -3546,7 +3557,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, case WC_PKCS7_VERIFY_STAGE2: #ifndef NO_PKCS7_STREAM - if ((ret = wc_PKCS7_AddDataToStream(pkcs7, pkiMsg, inSz + in2Sz, + if ((ret = wc_PKCS7_AddDataToStream(pkcs7, in, inSz + in2Sz, MAX_SEQ_SZ + MAX_OID_SZ + ASN_TAG_SZ + MAX_LENGTH_SZ + ASN_TAG_SZ + MAX_LENGTH_SZ, &pkiMsg, &idx)) != 0) { break; @@ -3555,6 +3566,13 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, wc_PKCS7_StreamGetVar(pkcs7, &totalSz, 0, 0); if (pkcs7->stream->length > 0) pkiMsgSz = pkcs7->stream->length; + #ifdef ASN_BER_TO_DER + else if (pkcs7->der) + pkiMsgSz = pkcs7->derSz; + #endif + else + pkiMsgSz = inSz; + #endif /* Get the inner ContentInfo sequence */ if (GetSequence_ex(pkiMsg, &idx, &length, pkiMsgSz, @@ -3692,7 +3710,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, case WC_PKCS7_VERIFY_STAGE3: #ifndef NO_PKCS7_STREAM - if ((ret = wc_PKCS7_AddDataToStream(pkcs7, pkiMsg, inSz + in2Sz, + if ((ret = wc_PKCS7_AddDataToStream(pkcs7, in, inSz + in2Sz, pkcs7->stream->expected, &pkiMsg, &idx)) != 0) { break; } @@ -3703,7 +3721,11 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, ret = (int)rc; break; } - if (pkcs7->stream->length > 0) + #ifdef ASN_BER_TO_DER + if (pkcs7->derSz != 0) + pkiMsgSz = pkcs7->derSz; + else + #endif pkiMsgSz = (word32)rc; wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, (int*)&localIdx, &length); @@ -3867,7 +3889,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, case WC_PKCS7_VERIFY_STAGE4: #ifndef NO_PKCS7_STREAM - if ((ret = wc_PKCS7_AddDataToStream(pkcs7, pkiMsg, inSz + in2Sz, + if ((ret = wc_PKCS7_AddDataToStream(pkcs7, in, inSz + in2Sz, pkcs7->stream->expected, &pkiMsg, &idx)) != 0) { break; } @@ -4048,7 +4070,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, case WC_PKCS7_VERIFY_STAGE5: #ifndef NO_PKCS7_STREAM - if ((ret = wc_PKCS7_AddDataToStream(pkcs7, pkiMsg, inSz + in2Sz, + if ((ret = wc_PKCS7_AddDataToStream(pkcs7, in, inSz + in2Sz, pkcs7->stream->expected, &pkiMsg, &idx)) != 0) { break; } @@ -4108,7 +4130,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, case WC_PKCS7_VERIFY_STAGE6: #ifndef NO_PKCS7_STREAM - if ((ret = wc_PKCS7_AddDataToStream(pkcs7, pkiMsg, inSz + in2Sz, + if ((ret = wc_PKCS7_AddDataToStream(pkcs7, in, inSz + in2Sz, pkcs7->stream->expected, &pkiMsg, &idx)) != 0) { break; } diff --git a/wolfssl/wolfcrypt/pkcs7.h b/wolfssl/wolfcrypt/pkcs7.h index 316116241..15461dc32 100644 --- a/wolfssl/wolfcrypt/pkcs7.h +++ b/wolfssl/wolfcrypt/pkcs7.h @@ -214,6 +214,7 @@ struct PKCS7 { void* heap; /* heap hint for dynamic memory */ #ifdef ASN_BER_TO_DER byte* der; /* DER encoded version of message */ + word32 derSz; #endif byte* cert[MAX_PKCS7_CERTS]; From 8666b7de9a7bfe4b961c91079222845cd1761a5e Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Wed, 6 Feb 2019 11:11:27 -0700 Subject: [PATCH 4/4] add test-ber-exp02-05-2022.p7b file for test --- certs/test-ber-exp02-05-2022.p7b | Bin 0 -> 2544 bytes 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 certs/test-ber-exp02-05-2022.p7b diff --git a/certs/test-ber-exp02-05-2022.p7b b/certs/test-ber-exp02-05-2022.p7b new file mode 100644 index 0000000000000000000000000000000000000000..4fc8671e7c1d8a9507f2e585dbfd391d3a0a1da2 GIT binary patch literal 2544 zcmchZ`9IX_7r^K988Z_z_I-&+b(;~NF~*WhGGr9RjO^>Htd|*~i=<|TB!!w%;_@Ao z<hKzd4yL`aG*CR2FsdMw*PAFJ*EiT-}ICg1PT9e4FCw30F%K$Zj2qx_xDxc+Pt%^ zaT~RLpT2Nc$0aP_-sPpf+X%z*h;myO%WTY*$lKu;L<_EA70QM(&dJ$QN%pu#;6G)WL$?02?~tE61DE>)7M~8+(_;l8>{uy0v$U`kt2^! zROE#}l_uJu35hLR!vY{FzkzrR{w&69ZzDxI$A|xu2)ybn4`IUXO70 z7X?{t1Tc4`p*0NCMBKTAQ3f&NWgYG9e(*N{F}rK8-^dZmR>wj~+wIFKZ(HM?0&?9R z%1v&hB<;}YN3sl+*W2$?^`5Y%3Rm>sru+1Ix0)MOl*{7gkG-EX7tU^x#RfGc7&Ys2 zFa?^!0~spm@K)OcxjrtxYTxyiy}zzQ4M+Rp&W2O37tNxVKAEVPXhi|k6qRQisdBZs zi;_w~iM}z9B;!U(#{*tvM%s*W;OC~!PqkfvE4kDgPhHUztR;>3jCM74Omg({_a~mf zNQ(5s6SJI{@7zL5rjn7^1Ka!2jjGS-A|#!#h_QS)+2;AcOsC$~BQ z<0*%8rH7MK9+tHie}3uGIzthJyp(7)4q0T1(JSAaAVoHQ#(rJh{Wklnv063KUU@#X z_%OxN>kB!$w7jHj9@ta&d=x4Fiy?XiD>pq%$t~}FOi4j##*4&@(=aJ<4@c_)6!sudx4H zd}LX0YlsCBIdBjFK|p9Lj0isU0sn0Re1egZ;Bo4oIB&zCtm(8cFPhh%8WaX)N482}s+9+oWNw4s0V#j)1hORWkT8EA{}Wze{&d=ogQvs%=pp`L zk&v>iFkTf)4j8LKyZ2lr6`)n)B(T2{-8EBWXEr^5&#`Uay<4)+d+K-h&W8 zh-3`Jw&hrUEL611x~MPY(Ra2lbmDi?w30Su1mSUd)k`|4MZSbml2aY%y4h0|a;mAQ zC(}YtBX(8xcnR^iGqIgbFWKZ zodfxE6H6sY#I9Sydw0K~H4u&+r-XXn_!$2zw_l)+cP}Y)PjlykkeIc@Y^wxEtCiKB zc8S4zp(Zv9C?~f=1~VoZV)HP;ZU8iB3SHLu@{6e(%&vth%2>ng0pS1)*qR}JvG6kl zz>H%GjZTks*npP2Q)f4Q*kVKAi_5R_dt}ygMw-J`gUPQQGMQM6hW&=u&2B_&_pHTK zM{sqaf?oA}fDi{&?yxb@=V+x-l&h$z{Tgrnvus}A;6!=1dW|K~lLsWH6rU7CHPb>SPh_H~D5u9@iVjyd;< zQwuKXPnHK2u5*nVRyfrcEHX=RS4Uo@_BkfTSu>*)TcWFXg+^CA;zS+Ddbz#mGz^Ak zf$4mtZt*85ME@s?_8Y!`4QvkoH6U@MkY^kjh+8!W1VKDp5+M9d1^=dkAh{m{SP+g! z{}50Br1V1&MBqjFd>Z@@NIs#?(2gyqBk&ReVsmeB{%qF7$KNicU{}!Cmu9ANYcaW5{MANlz50hBARWj&~JYW765x%f1tie zs=!k7*j}iD_qlDnDB8lF!II8YN-8X~M=kB*OnWpCJQfaHsU2Cy2D7_6;}>f_%+l2A zceX)|7k%?@UOQM2A-xGUNT*=6h!K`io}}Q!qbL9%$~dbBs*;mQi8E-In8oOs@o=hG zf$N6rY7oxA-KhBW{#rdry6%+`KMNm&!u(}J*R+Q#W8+a&qR-$t&B~`$?Oo0B`MK1A ztklhTe?;I;h$vbmiW-Y($nj;wRPGOBC~}+j-HpsIafD;#O{Ilyu~yZJFEuzOWVOod wOR_$HC%D4J~=ly%&01UMe3+o+3