From f0354b4cbeef2df5397b177757b12de1165c0562 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Mon, 12 Jun 2023 16:39:00 -0600 Subject: [PATCH 1/4] parse ASN1 only with SMIME_read_PKCS7 --- src/ssl.c | 58 ++++++++++++++++++++++++++++++++++++++--------------- tests/api.c | 2 +- 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index c9720e96b4..e02a48723a 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -37036,19 +37036,14 @@ PKCS7* wolfSSL_d2i_PKCS7(PKCS7** p7, const unsigned char** in, int len) return wolfSSL_d2i_PKCS7_ex(p7, in, len, NULL, 0); } -/***************************************************************************** -* wolfSSL_d2i_PKCS7_ex - Converts the given unsigned char buffer of size len -* into a PKCS7 object. Optionally, accepts a byte buffer of content which -* is stored as the PKCS7 object's content, to support detached signatures. -* @param content The content which is signed, in case the signature is -* detached. Ignored if NULL. -* @param contentSz The size of the passed in content. +/* This internal function is only decoding and setting up the PKCS7 struct. It +* does not verify the PKCS7 signature. * * RETURNS: * returns pointer to a PKCS7 structure on success, otherwise returns NULL */ -PKCS7* wolfSSL_d2i_PKCS7_ex(PKCS7** p7, const unsigned char** in, int len, - byte* content, word32 contentSz) +static PKCS7* wolfSSL_d2i_PKCS7_only(PKCS7** p7, const unsigned char** in, + int len, byte* content, word32 contentSz) { WOLFSSL_PKCS7* pkcs7 = NULL; @@ -37072,12 +37067,6 @@ PKCS7* wolfSSL_d2i_PKCS7_ex(PKCS7** p7, const unsigned char** in, int len, pkcs7->pkcs7.content = content; pkcs7->pkcs7.contentSz = contentSz; } - if (wc_PKCS7_VerifySignedData(&pkcs7->pkcs7, pkcs7->data, pkcs7->len) - != 0) { - WOLFSSL_MSG("wc_PKCS7_VerifySignedData failed"); - wolfSSL_PKCS7_free((PKCS7*)pkcs7); - return NULL; - } if (p7 != NULL) *p7 = (PKCS7*)pkcs7; @@ -37085,6 +37074,43 @@ PKCS7* wolfSSL_d2i_PKCS7_ex(PKCS7** p7, const unsigned char** in, int len, return (PKCS7*)pkcs7; } + +/***************************************************************************** +* wolfSSL_d2i_PKCS7_ex - Converts the given unsigned char buffer of size len +* into a PKCS7 object. Optionally, accepts a byte buffer of content which +* is stored as the PKCS7 object's content, to support detached signatures. +* @param content The content which is signed, in case the signature is +* detached. Ignored if NULL. +* @param contentSz The size of the passed in content. +* +* RETURNS: +* returns pointer to a PKCS7 structure on success, otherwise returns NULL +*/ +PKCS7* wolfSSL_d2i_PKCS7_ex(PKCS7** p7, const unsigned char** in, int len, + byte* content, word32 contentSz) +{ + WOLFSSL_PKCS7* pkcs7 = NULL; + + WOLFSSL_ENTER("wolfSSL_d2i_PKCS7_ex"); + + if (in == NULL || *in == NULL || len < 0) + return NULL; + + pkcs7 = (WOLFSSL_PKCS7*)wolfSSL_d2i_PKCS7_only(p7, in, len, content, + contentSz); + if (pkcs7 != NULL) { + if (wc_PKCS7_VerifySignedData(&pkcs7->pkcs7, pkcs7->data, pkcs7->len) + != 0) { + WOLFSSL_MSG("wc_PKCS7_VerifySignedData failed"); + wolfSSL_PKCS7_free((PKCS7*)pkcs7); + return NULL; + } + } + + return (PKCS7*)pkcs7; +} + + /** * This API was added as a helper function for libest. It * extracts a stack of certificates from the pkcs7 object. @@ -38256,7 +38282,7 @@ PKCS7* wolfSSL_SMIME_read_PKCS7(WOLFSSL_BIO* in, WOLFSSL_MSG("Error base64 decoding S/MIME message."); goto error; } - pkcs7 = wolfSSL_d2i_PKCS7_ex(NULL, (const unsigned char**)&out, outLen, + pkcs7 = wolfSSL_d2i_PKCS7_only(NULL, (const unsigned char**)&out, outLen, bcontMem, bcontMemSz); wc_MIME_free_hdrs(allHdrs); diff --git a/tests/api.c b/tests/api.c index 9e9155186d..deb35356f2 100644 --- a/tests/api.c +++ b/tests/api.c @@ -48860,7 +48860,7 @@ static int test_wolfSSL_SMIME_read_PKCS7(void) smimeTestFile = XFOPEN("./certs/test/smime-test-multipart-badsig.p7s", "r"); ExpectIntEQ(wolfSSL_BIO_set_fp(bio, smimeTestFile, BIO_CLOSE), SSL_SUCCESS); pkcs7 = wolfSSL_SMIME_read_PKCS7(bio, &bcont); - ExpectNull(pkcs7); + ExpectNotNull(pkcs7); /* can read in the unverified smime bundle */ ExpectIntEQ(wolfSSL_PKCS7_verify(pkcs7, NULL, NULL, bcont, NULL, PKCS7_NOVERIFY), SSL_FAILURE); XFCLOSE(smimeTestFile); From 7866a40d06b874f7ae52e573a71a2f47b6409c4f Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Tue, 13 Jun 2023 17:15:22 -0600 Subject: [PATCH 2/4] resolve kari decode without recipient certificate --- tests/api.c | 4 +-- wolfcrypt/src/pkcs7.c | 73 ++++++++++++++++++++++++------------------- 2 files changed, 43 insertions(+), 34 deletions(-) diff --git a/tests/api.c b/tests/api.c index deb35356f2..e62752e960 100644 --- a/tests/api.c +++ b/tests/api.c @@ -25658,7 +25658,7 @@ static int test_wc_PKCS7_EncodeDecodeEnvelopedData(void) } ExpectIntEQ(wc_PKCS7_DecodeEnvelopedData(pkcs7, output, (word32)sizeof(output), decoded, (word32)sizeof(decoded)), - BAD_FUNC_ARG); + BUFFER_E); if (pkcs7 != NULL) { pkcs7->singleCertSz = tempWrd32; @@ -25667,7 +25667,7 @@ static int test_wc_PKCS7_EncodeDecodeEnvelopedData(void) } ExpectIntEQ(wc_PKCS7_DecodeEnvelopedData(pkcs7, output, (word32)sizeof(output), decoded, (word32)sizeof(decoded)), - BAD_FUNC_ARG); + ASN_PARSE_E); if (pkcs7 != NULL) { pkcs7->singleCert = tmpBytePtr; } diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index f469966c18..698ffd2324 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -5771,29 +5771,30 @@ static int wc_PKCS7_KariParseRecipCert(WC_PKCS7_KARI* kari, const byte* cert, int ret; word32 idx; - if (kari == NULL || kari->decoded == NULL || - cert == NULL || certSz == 0) + if (kari == NULL || kari->decoded == NULL) { return BAD_FUNC_ARG; + } /* decode certificate */ - InitDecodedCert(kari->decoded, (byte*)cert, certSz, kari->heap); - kari->decodedInit = 1; - ret = ParseCert(kari->decoded, CA_TYPE, NO_VERIFY, 0); - if (ret < 0) - return ret; + if (cert != NULL) { + InitDecodedCert(kari->decoded, (byte*)cert, certSz, kari->heap); + kari->decodedInit = 1; + ret = ParseCert(kari->decoded, CA_TYPE, NO_VERIFY, 0); + if (ret < 0) + return ret; - /* only supports ECDSA for now */ - if (kari->decoded->keyOID != ECDSAk) { - WOLFSSL_MSG("CMS KARI only supports ECDSA key types"); - return BAD_FUNC_ARG; - } + /* only supports ECDSA for now */ + if (kari->decoded->keyOID != ECDSAk) { + WOLFSSL_MSG("CMS KARI only supports ECDSA key types"); + return BAD_FUNC_ARG; + } - /* make sure subject key id was read from cert */ - if (kari->decoded->extSubjKeyIdSet == 0) { - WOLFSSL_MSG("Failed to read subject key ID from recipient cert"); - return BAD_FUNC_ARG; + /* make sure subject key id was read from cert */ + if (kari->decoded->extSubjKeyIdSet == 0) { + WOLFSSL_MSG("Failed to read subject key ID from recipient cert"); + return BAD_FUNC_ARG; + } } - ret = wc_ecc_init_ex(kari->recipKey, kari->heap, kari->devId); if (ret != 0) return ret; @@ -5802,6 +5803,10 @@ static int wc_PKCS7_KariParseRecipCert(WC_PKCS7_KARI* kari, const byte* cert, /* get recip public key */ if (kari->direction == WC_PKCS7_ENCODE) { + if (cert == NULL) { + WOLFSSL_MSG("Error recipient cert can not be null with encode"); + return BAD_FUNC_ARG; + } idx = 0; ret = wc_EccPublicKeyDecode(kari->decoded->publicKey, &idx, @@ -9270,7 +9275,14 @@ static int wc_PKCS7_KariGetIssuerAndSerialNumber(WC_PKCS7_KARI* kari, } /* if we found correct recipient, issuer hashes will match */ - if (XMEMCMP(rid, kari->decoded->issuerHash, keyIdSize) == 0) { + if (kari->decodedInit == 1) { + if (XMEMCMP(rid, kari->decoded->issuerHash, keyIdSize) == 0) { + *recipFound = 1; + } + } + else { + /* can not confirm recipient serial number with no cert provided */ + WOLFSSL_MSG("No recipient cert loaded to match with CMS serial number"); *recipFound = 1; } @@ -9308,7 +9320,8 @@ static int wc_PKCS7_KariGetIssuerAndSerialNumber(WC_PKCS7_KARI* kari, return ret; } - if (mp_cmp(recipSerial, serial) != MP_EQ) { + if (kari->decodedInit == 1 && + mp_cmp(recipSerial, serial) != MP_EQ) { mp_clear(serial); mp_clear(recipSerial); WOLFSSL_MSG("CMS serial number does not match recipient"); @@ -9944,8 +9957,6 @@ static int wc_PKCS7_DecryptKari(PKCS7* pkcs7, byte* in, word32 inSz, WOLFSSL_ENTER("wc_PKCS7_DecryptKari"); if (pkcs7 == NULL || pkiMsg == NULL || - ((pkcs7->singleCert == NULL || pkcs7->singleCertSz == 0) && - pkcs7->wrapCEKCb == NULL) || idx == NULL || decryptedKey == NULL || decryptedKeySz == NULL) { return BAD_FUNC_ARG; } @@ -9986,17 +9997,15 @@ static int wc_PKCS7_DecryptKari(PKCS7* pkcs7, byte* in, word32 inSz, encryptedKeySz = MAX_ENCRYPTED_KEY_SZ; /* parse cert and key */ - if (pkcs7->singleCert != NULL) { - ret = wc_PKCS7_KariParseRecipCert(kari, (byte*)pkcs7->singleCert, - pkcs7->singleCertSz, pkcs7->privateKey, - pkcs7->privateKeySz); - if (ret != 0) { - wc_PKCS7_KariFree(kari); - #ifdef WOLFSSL_SMALL_STACK - XFREE(encryptedKey, pkcs7->heap, DYNAMIC_TYPE_PKCS7); - #endif - return ret; - } + ret = wc_PKCS7_KariParseRecipCert(kari, (byte*)pkcs7->singleCert, + pkcs7->singleCertSz, pkcs7->privateKey, + pkcs7->privateKeySz); + if (ret != 0) { + wc_PKCS7_KariFree(kari); + #ifdef WOLFSSL_SMALL_STACK + XFREE(encryptedKey, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + #endif + return ret; } /* remove OriginatorIdentifierOrKey */ From 0e2749eeb419fb39bc6c74a8854820fa27aa8807 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Thu, 29 Jun 2023 09:34:41 -0700 Subject: [PATCH 3/4] adjust test case for asn=original --- tests/api.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/api.c b/tests/api.c index e62752e960..96f0f71745 100644 --- a/tests/api.c +++ b/tests/api.c @@ -25656,9 +25656,15 @@ static int test_wc_PKCS7_EncodeDecodeEnvelopedData(void) tempWrd32 = pkcs7->singleCertSz; pkcs7->singleCertSz = 0; } + #if defined(WOLFSSL_ASN_TEMPLATE) ExpectIntEQ(wc_PKCS7_DecodeEnvelopedData(pkcs7, output, (word32)sizeof(output), decoded, (word32)sizeof(decoded)), BUFFER_E); + #else + ExpectIntEQ(wc_PKCS7_DecodeEnvelopedData(pkcs7, output, + (word32)sizeof(output), decoded, (word32)sizeof(decoded)), + ASN_PARSE_E); + #endif if (pkcs7 != NULL) { pkcs7->singleCertSz = tempWrd32; From 9d3a95a2876bf3380b9543268cc19584f8874d55 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Sun, 9 Jul 2023 12:42:29 -0700 Subject: [PATCH 4/4] account for error return in test case when building without pkcs7 streaming --- tests/api.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/api.c b/tests/api.c index 96f0f71745..8f2d284e74 100644 --- a/tests/api.c +++ b/tests/api.c @@ -25671,9 +25671,17 @@ static int test_wc_PKCS7_EncodeDecodeEnvelopedData(void) tmpBytePtr = pkcs7->singleCert; pkcs7->singleCert = NULL; } + #if defined(NO_PKCS7_STREAM) + /* when none streaming mode is used and PKCS7 is in bad state buffer error + * is returned from kari parse which gets set to bad func arg */ + ExpectIntEQ(wc_PKCS7_DecodeEnvelopedData(pkcs7, output, + (word32)sizeof(output), decoded, (word32)sizeof(decoded)), + BAD_FUNC_ARG); + #else ExpectIntEQ(wc_PKCS7_DecodeEnvelopedData(pkcs7, output, (word32)sizeof(output), decoded, (word32)sizeof(decoded)), ASN_PARSE_E); + #endif if (pkcs7 != NULL) { pkcs7->singleCert = tmpBytePtr; }