diff --git a/src/ssl_p7p12.c b/src/ssl_p7p12.c index 51054006f41..9ca74905a66 100644 --- a/src/ssl_p7p12.c +++ b/src/ssl_p7p12.c @@ -783,7 +783,9 @@ int wolfSSL_PKCS7_verify(PKCS7* pkcs7, WOLFSSL_STACK* certs, WOLFSSL_X509_STORE* store, WOLFSSL_BIO* in, WOLFSSL_BIO* out, int flags) { int i, ret = 0; + int retVal = WOLFSSL_FAILURE; unsigned char* mem = NULL; + unsigned char* inBuf = NULL; int memSz = 0; WOLFSSL_PKCS7* p7 = (WOLFSSL_PKCS7*)pkcs7; int contTypeLen; @@ -798,8 +800,31 @@ int wolfSSL_PKCS7_verify(PKCS7* pkcs7, WOLFSSL_STACK* certs, return WOLFSSL_FAILURE; if (in != NULL) { - if ((memSz = wolfSSL_BIO_get_mem_data(in, &mem)) < 0) - return WOLFSSL_FAILURE; + /* Use a memory BIO in place; read any other BIO type into a + * temporary buffer, as wolfSSL_d2i_PKCS7_bio() does. */ + if (wolfSSL_BIO_method_type(in) == WOLFSSL_BIO_MEMORY) { + if ((memSz = wolfSSL_BIO_get_mem_data(in, &mem)) < 0) + goto cleanup; + } + else { + if ((memSz = wolfSSL_BIO_get_len(in)) <= 0) { + WOLFSSL_MSG("Error getting length of input BIO"); + goto cleanup; + } + + inBuf = (unsigned char*)XMALLOC((size_t)memSz, p7->pkcs7.heap, + DYNAMIC_TYPE_TMP_BUFFER); + if (inBuf == NULL) { + WOLFSSL_MSG("Error allocating memory for input BIO data"); + goto cleanup; + } + + if ((memSz = wolfSSL_BIO_read(in, inBuf, memSz)) <= 0) { + WOLFSSL_MSG("Error reading from input BIO"); + goto cleanup; + } + mem = inBuf; + } p7->pkcs7.content = mem; p7->pkcs7.contentSz = (word32)memSz; @@ -814,7 +839,7 @@ int wolfSSL_PKCS7_verify(PKCS7* pkcs7, WOLFSSL_STACK* certs, ret = wc_PKCS7_VerifySignedData(&p7->pkcs7, p7->data, p7->len); if (ret != 0) - return WOLFSSL_FAILURE; + goto cleanup; /* Reject a degenerate (certs-only) PKCS#7 with no verified signer. Such an * object has empty signerInfos, so wc_PKCS7_VerifySignedData() succeeds @@ -826,27 +851,27 @@ int wolfSSL_PKCS7_verify(PKCS7* pkcs7, WOLFSSL_STACK* certs, */ if (p7->pkcs7.verifyCert == NULL) { WOLFSSL_MSG("PKCS7 has no verified signer (degenerate/certs-only)"); - return WOLFSSL_FAILURE; + goto cleanup; } if ((flags & PKCS7_NOVERIFY) != PKCS7_NOVERIFY) { /* Verify signer certificates */ if (store == NULL || store->cm == NULL) { WOLFSSL_MSG("No store or store certs, but PKCS7_NOVERIFY not set"); - return WOLFSSL_FAILURE; + goto cleanup; } ctx = X509_STORE_CTX_new(); if (ctx == NULL) { WOLFSSL_MSG("Error allocating X509 Store Context"); - return WOLFSSL_FAILURE; + goto cleanup; } signers = wolfSSL_PKCS7_get0_signers(pkcs7, certs, flags); if (signers == NULL) { WOLFSSL_MSG("No signers found to verify"); wolfSSL_X509_STORE_CTX_free(ctx); - return WOLFSSL_FAILURE; + goto cleanup; } for (i = 0; i < wolfSSL_sk_X509_num(signers); i++) { @@ -856,13 +881,13 @@ int wolfSSL_PKCS7_verify(PKCS7* pkcs7, WOLFSSL_STACK* certs, WOLFSSL_MSG("Failed to initialize X509 STORE CTX"); wolfSSL_sk_X509_pop_free(signers, NULL); wolfSSL_X509_STORE_CTX_free(ctx); - return WOLFSSL_FAILURE; + goto cleanup; } if (wolfSSL_X509_verify_cert(ctx) != WOLFSSL_SUCCESS) { WOLFSSL_MSG("Failed to verify signer certificate"); wolfSSL_sk_X509_pop_free(signers, NULL); wolfSSL_X509_STORE_CTX_free(ctx); - return WOLFSSL_FAILURE; + goto cleanup; } } wolfSSL_sk_X509_pop_free(signers, NULL); @@ -875,7 +900,7 @@ int wolfSSL_PKCS7_verify(PKCS7* pkcs7, WOLFSSL_STACK* certs, if ((p7->pkcs7.contentSz < (word32)contTypeLen) || (XMEMCMP(p7->pkcs7.content, contTypeText, contTypeLen) != 0)) { WOLFSSL_MSG("Error PKCS7 Content-Type not found with PKCS7_TEXT"); - return WOLFSSL_FAILURE; + goto cleanup; } p7->pkcs7.content += contTypeLen; p7->pkcs7.contentSz -= contTypeLen; @@ -887,7 +912,17 @@ int wolfSSL_PKCS7_verify(PKCS7* pkcs7, WOLFSSL_STACK* certs, WOLFSSL_LEAVE("wolfSSL_PKCS7_verify", WOLFSSL_SUCCESS); - return WOLFSSL_SUCCESS; + retVal = WOLFSSL_SUCCESS; + +cleanup: + if (inBuf != NULL) { + /* content pointed into inBuf */ + p7->pkcs7.content = NULL; + p7->pkcs7.contentSz = 0; + XFREE(inBuf, p7->pkcs7.heap, DYNAMIC_TYPE_TMP_BUFFER); + } + + return retVal; } /** diff --git a/tests/api/test_ossl_p7p12.c b/tests/api/test_ossl_p7p12.c index b8a375503f6..835add0d5c1 100644 --- a/tests/api/test_ossl_p7p12.c +++ b/tests/api/test_ossl_p7p12.c @@ -675,6 +675,118 @@ int test_wolfSSL_PKCS7_verify_signer_forgery(void) return EXPECT_RESULT(); } +/* Detached content must be accepted through any BIO type, not just a + * memory BIO. */ +int test_wolfSSL_PKCS7_verify_detached_bio(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_ALL) && defined(HAVE_PKCS7) && !defined(NO_BIO) && \ + !defined(NO_FILESYSTEM) && !defined(NO_RSA) + const char* signerCertFile = "./certs/server-cert.pem"; + const char* signerKeyFile = "./certs/server-key.pem"; + const char* caFile = "./certs/ca-cert.pem"; + const char* contentFile = "test_pkcs7_detached_content.bin"; + byte content[] = "Detached content to verify."; + + WOLFSSL_BIO* certBio = NULL; + WOLFSSL_BIO* keyBio = NULL; + WOLFSSL_BIO* caBio = NULL; + WOLFSSL_BIO* signBio = NULL; + WOLFSSL_BIO* inBio = NULL; + X509* signCert = NULL; + EVP_PKEY* signKey = NULL; + X509* caCert = NULL; + X509_STORE* store = NULL; + PKCS7* p7 = NULL; + WOLFSSL_PKCS7* p7Ver = NULL; + byte* der = NULL; + int derSz = 0; + XFILE fp = XBADFILE; + + ExpectNotNull(certBio = BIO_new_file(signerCertFile, "r")); + ExpectNotNull(keyBio = BIO_new_file(signerKeyFile, "r")); + ExpectNotNull(caBio = BIO_new_file(caFile, "r")); + ExpectNotNull(signCert = PEM_read_bio_X509(certBio, NULL, 0, NULL)); + ExpectNotNull(signKey = PEM_read_bio_PrivateKey(keyBio, NULL, 0, NULL)); + ExpectNotNull(caCert = PEM_read_bio_X509(caBio, NULL, 0, NULL)); + ExpectNotNull(store = X509_STORE_new()); + ExpectIntEQ(X509_STORE_add_cert(store, caCert), 1); + + /* create a detached signature over 'content' */ + ExpectNotNull(signBio = BIO_new(BIO_s_mem())); + ExpectIntGT(BIO_write(signBio, content, sizeof(content)), 0); + ExpectNotNull(p7 = PKCS7_sign(signCert, signKey, NULL, signBio, + PKCS7_BINARY | PKCS7_DETACHED)); + ExpectIntGT((derSz = i2d_PKCS7(p7, &der)), 0); + ExpectNotNull(der); + + /* write the detached content out so it can be fed back as a file BIO */ + ExpectTrue((fp = XFOPEN(contentFile, "wb")) != XBADFILE); + ExpectIntEQ(XFWRITE(content, 1, sizeof(content), fp), sizeof(content)); + if (fp != XBADFILE) { + XFCLOSE(fp); + fp = XBADFILE; + } + + /* d2i_PKCS7() cannot be used here: it verifies while decoding and has no + * way to be handed the detached content, so load the bundle directly. */ + if (EXPECT_SUCCESS()) { + ExpectNotNull(p7Ver = (WOLFSSL_PKCS7*)PKCS7_new()); + if (p7Ver != NULL) { + ExpectNotNull(p7Ver->data = (byte*)XMALLOC((size_t)derSz, NULL, + DYNAMIC_TYPE_PKCS7)); + if (p7Ver->data != NULL) { + XMEMCPY(p7Ver->data, der, (size_t)derSz); + p7Ver->len = derSz; + } + } + } + + /* a file BIO must work just like a memory BIO */ + ExpectNotNull(inBio = BIO_new_file(contentFile, "rb")); + ExpectIntEQ(PKCS7_verify((PKCS7*)p7Ver, NULL, store, inBio, NULL, + PKCS7_BINARY), 1); + BIO_free(inBio); + inBio = NULL; + + /* the memory BIO path must keep working */ + ExpectNotNull(inBio = BIO_new_mem_buf(content, sizeof(content))); + ExpectIntEQ(PKCS7_verify((PKCS7*)p7Ver, NULL, store, inBio, NULL, + PKCS7_BINARY), 1); + BIO_free(inBio); + inBio = NULL; + + /* wrong content must still fail, from a file BIO too */ + ExpectTrue((fp = XFOPEN(contentFile, "wb")) != XBADFILE); + ExpectIntEQ(XFWRITE("bogus content", 1, 13, fp), 13); + if (fp != XBADFILE) { + XFCLOSE(fp); + fp = XBADFILE; + } + ExpectNotNull(inBio = BIO_new_file(contentFile, "rb")); + ExpectIntEQ(PKCS7_verify((PKCS7*)p7Ver, NULL, store, inBio, NULL, + PKCS7_BINARY), + WC_NO_ERR_TRACE(WOLFSSL_FAILURE)); + BIO_free(inBio); + inBio = NULL; + + (void)remove(contentFile); + + PKCS7_free((PKCS7*)p7Ver); + PKCS7_free(p7); + XFREE(der, NULL, DYNAMIC_TYPE_TMP_BUFFER); + X509_STORE_free(store); + X509_free(caCert); + X509_free(signCert); + EVP_PKEY_free(signKey); + BIO_free(signBio); + BIO_free(certBio); + BIO_free(keyBio); + BIO_free(caBio); +#endif + return EXPECT_RESULT(); +} + /* A degenerate (certs-only) PKCS#7 - one with an empty signerInfos SET and * therefore no signature at all - must NOT be reported as verified, even when * the embedded certificate chains to a trusted CA, and even when diff --git a/tests/api/test_ossl_p7p12.h b/tests/api/test_ossl_p7p12.h index 8ab8fd79530..962fb7ee4d0 100644 --- a/tests/api/test_ossl_p7p12.h +++ b/tests/api/test_ossl_p7p12.h @@ -30,6 +30,7 @@ int test_wolfSSL_PKCS7_sign(void); int test_wolfSSL_PKCS7_verify_signer_forgery(void); int test_wolfSSL_PKCS7_verify_sid_binding(void); int test_wolfSSL_PKCS7_verify_degenerate(void); +int test_wolfSSL_PKCS7_verify_detached_bio(void); int test_wolfSSL_PKCS7_SIGNED_new(void); int test_wolfSSL_PEM_write_bio_PKCS7(void); int test_wolfSSL_PEM_write_bio_encryptedKey(void); @@ -44,6 +45,7 @@ int test_wolfSSL_PKCS12(void); TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PKCS7_verify_signer_forgery), \ TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PKCS7_verify_sid_binding), \ TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PKCS7_verify_degenerate), \ + TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PKCS7_verify_detached_bio), \ TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PKCS7_SIGNED_new), \ TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PEM_write_bio_PKCS7), \ TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PEM_write_bio_encryptedKey), \ diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 6f63ba28354..d40193c859c 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -6654,6 +6654,9 @@ static int wc_PKCS7_HandleOctetStrings(wc_PKCS7* pkcs7, byte* in, word32 inSz, XMEMCPY(pkcs7->stream->content, pkcs7->content, pkcs7->contentSz); pkcs7->stream->contentSz = pkcs7->contentSz; } + /* drop the content copy left by a previous verify */ + XFREE(pkcs7->contentDynamic, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + pkcs7->contentDynamic = NULL; return 0; }