From b7f6e77a95bfeda306b22dd34394e49c0a6c8a8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 21 Apr 2026 13:31:38 +0200 Subject: [PATCH] Reject PKCS#7 SignedData signer-identity forgery --- src/ssl_p7p12.c | 28 ++- tests/api/test_ossl_p7p12.c | 454 ++++++++++++++++++++++++++++++++++++ tests/api/test_ossl_p7p12.h | 4 + wolfcrypt/src/pkcs7.c | 159 +++++++++++++ 4 files changed, 640 insertions(+), 5 deletions(-) diff --git a/src/ssl_p7p12.c b/src/ssl_p7p12.c index ee48d700aa..a07f018b2a 100644 --- a/src/ssl_p7p12.c +++ b/src/ssl_p7p12.c @@ -269,24 +269,42 @@ WOLFSSL_STACK* wolfSSL_PKCS7_get0_signers(PKCS7* pkcs7, WOLFSSL_STACK* certs, WOLFSSL_X509* x509 = NULL; WOLFSSL_STACK* signers = NULL; WOLFSSL_PKCS7* p7 = (WOLFSSL_PKCS7*)pkcs7; + byte* signerCert; + word32 signerCertSz; if (p7 == NULL) return NULL; - /* Only PKCS#7 messages with a single cert that is the verifying certificate - * is supported. - */ if (flags & PKCS7_NOINTERN) { WOLFSSL_MSG("PKCS7_NOINTERN flag not supported"); return NULL; } + /* Prefer the certificate that actually verified the signature. Falling + * back to singleCert (cert[0]) would let an attacker that bundles a + * trusted cert ahead of their own attacker cert have the trusted cert + * reported as the signer even though it did not produce the signature. + * + * Copy the chosen pointer into a local before passing its address to + * wolfSSL_d2i_X509; d2i_X509 advances *in by the DER length, and if + * we handed it the address of the struct field directly it would + * permanently corrupt the field, producing a heap-OOB read on the + * next use (pointer advanced, singleCertSz unchanged). */ + if (p7->pkcs7.verifyCert != NULL && p7->pkcs7.verifyCertSz > 0) { + signerCert = p7->pkcs7.verifyCert; + signerCertSz = p7->pkcs7.verifyCertSz; + } + else { + signerCert = p7->pkcs7.singleCert; + signerCertSz = p7->pkcs7.singleCertSz; + } + signers = wolfSSL_sk_X509_new_null(); if (signers == NULL) return NULL; - if (wolfSSL_d2i_X509(&x509, (const byte**)&p7->pkcs7.singleCert, - p7->pkcs7.singleCertSz) == NULL) { + if (wolfSSL_d2i_X509(&x509, (const byte**)&signerCert, + signerCertSz) == NULL) { wolfSSL_sk_X509_pop_free(signers, NULL); return NULL; } diff --git a/tests/api/test_ossl_p7p12.c b/tests/api/test_ossl_p7p12.c index c92b80ee5f..03590a3ad7 100644 --- a/tests/api/test_ossl_p7p12.c +++ b/tests/api/test_ossl_p7p12.c @@ -446,6 +446,460 @@ int test_wolfSSL_PKCS7_sign(void) return EXPECT_RESULT(); } +/* Regression test for CMS SignedData signer-identity forgery. + * + * The embedded DER is a CMS SignedData message crafted so that the + * certificates SET contains two certificates: + * cert[0] = certs/ca-cert.pem (trusted wolfSSL CA; attacker does NOT hold + * its private key) + * cert[1] = a self-signed "attacker" P-256 certificate (attacker holds + * the private key) + * The signerInfo sid names the attacker certificate, and the signature + * was produced with the attacker's key over "Hello World". + * + * The bug that was present: wolfSSL_PKCS7_verify() iterated all bundled + * certificates trying each public key against the signature. When the + * attacker's key verified, it still reported cert[0] (the trusted CA cert, + * via singleCert) as the signer, and chain validation therefore succeeded + * on an unrelated trusted cert - a full signer-identity forgery. + * + * The expected, correct behavior: the CMS message is rejected because the + * signer certificate named by the sid (the attacker cert) does not chain + * to any certificate in the trust store. */ +int test_wolfSSL_PKCS7_verify_signer_forgery(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_ALL) && defined(HAVE_PKCS7) && !defined(NO_BIO) && \ + !defined(NO_FILESYSTEM) && !defined(NO_RSA) && defined(HAVE_ECC) + static const byte forgedSignedData[] = { + 0x30, 0x82, 0x07, 0x9c, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, + 0x01, 0x07, 0x02, 0xa0, 0x82, 0x07, 0x8d, 0x30, 0x82, 0x07, 0x89, 0x02, + 0x01, 0x01, 0x31, 0x0d, 0x30, 0x0b, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, + 0x65, 0x03, 0x04, 0x02, 0x01, 0x30, 0x1b, 0x06, 0x09, 0x2a, 0x86, 0x48, + 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x01, 0xa0, 0x0e, 0x04, 0x0c, 0x48, 0x65, + 0x6c, 0x6c, 0x6f, 0x20, 0x57, 0x6f, 0x72, 0x6c, 0x64, 0x0a, 0xa0, 0x82, + 0x06, 0xab, 0x30, 0x82, 0x04, 0xff, 0x30, 0x82, 0x03, 0xe7, 0xa0, 0x03, + 0x02, 0x01, 0x02, 0x02, 0x14, 0x3f, 0x29, 0x11, 0x20, 0x57, 0x71, 0xe7, + 0x8e, 0xf9, 0x18, 0x0d, 0xca, 0x70, 0x4d, 0x5b, 0x15, 0x2a, 0x43, 0xd6, + 0x24, 0x30, 0x0d, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, + 0x01, 0x0b, 0x05, 0x00, 0x30, 0x81, 0x94, 0x31, 0x0b, 0x30, 0x09, 0x06, + 0x03, 0x55, 0x04, 0x06, 0x13, 0x02, 0x55, 0x53, 0x31, 0x10, 0x30, 0x0e, + 0x06, 0x03, 0x55, 0x04, 0x08, 0x0c, 0x07, 0x4d, 0x6f, 0x6e, 0x74, 0x61, + 0x6e, 0x61, 0x31, 0x10, 0x30, 0x0e, 0x06, 0x03, 0x55, 0x04, 0x07, 0x0c, + 0x07, 0x42, 0x6f, 0x7a, 0x65, 0x6d, 0x61, 0x6e, 0x31, 0x11, 0x30, 0x0f, + 0x06, 0x03, 0x55, 0x04, 0x0a, 0x0c, 0x08, 0x53, 0x61, 0x77, 0x74, 0x6f, + 0x6f, 0x74, 0x68, 0x31, 0x13, 0x30, 0x11, 0x06, 0x03, 0x55, 0x04, 0x0b, + 0x0c, 0x0a, 0x43, 0x6f, 0x6e, 0x73, 0x75, 0x6c, 0x74, 0x69, 0x6e, 0x67, + 0x31, 0x18, 0x30, 0x16, 0x06, 0x03, 0x55, 0x04, 0x03, 0x0c, 0x0f, 0x77, + 0x77, 0x77, 0x2e, 0x77, 0x6f, 0x6c, 0x66, 0x73, 0x73, 0x6c, 0x2e, 0x63, + 0x6f, 0x6d, 0x31, 0x1f, 0x30, 0x1d, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, + 0xf7, 0x0d, 0x01, 0x09, 0x01, 0x16, 0x10, 0x69, 0x6e, 0x66, 0x6f, 0x40, + 0x77, 0x6f, 0x6c, 0x66, 0x73, 0x73, 0x6c, 0x2e, 0x63, 0x6f, 0x6d, 0x30, + 0x1e, 0x17, 0x0d, 0x32, 0x35, 0x31, 0x31, 0x31, 0x33, 0x32, 0x30, 0x34, + 0x31, 0x31, 0x31, 0x5a, 0x17, 0x0d, 0x32, 0x38, 0x30, 0x38, 0x30, 0x39, + 0x32, 0x30, 0x34, 0x31, 0x31, 0x31, 0x5a, 0x30, 0x81, 0x94, 0x31, 0x0b, + 0x30, 0x09, 0x06, 0x03, 0x55, 0x04, 0x06, 0x13, 0x02, 0x55, 0x53, 0x31, + 0x10, 0x30, 0x0e, 0x06, 0x03, 0x55, 0x04, 0x08, 0x0c, 0x07, 0x4d, 0x6f, + 0x6e, 0x74, 0x61, 0x6e, 0x61, 0x31, 0x10, 0x30, 0x0e, 0x06, 0x03, 0x55, + 0x04, 0x07, 0x0c, 0x07, 0x42, 0x6f, 0x7a, 0x65, 0x6d, 0x61, 0x6e, 0x31, + 0x11, 0x30, 0x0f, 0x06, 0x03, 0x55, 0x04, 0x0a, 0x0c, 0x08, 0x53, 0x61, + 0x77, 0x74, 0x6f, 0x6f, 0x74, 0x68, 0x31, 0x13, 0x30, 0x11, 0x06, 0x03, + 0x55, 0x04, 0x0b, 0x0c, 0x0a, 0x43, 0x6f, 0x6e, 0x73, 0x75, 0x6c, 0x74, + 0x69, 0x6e, 0x67, 0x31, 0x18, 0x30, 0x16, 0x06, 0x03, 0x55, 0x04, 0x03, + 0x0c, 0x0f, 0x77, 0x77, 0x77, 0x2e, 0x77, 0x6f, 0x6c, 0x66, 0x73, 0x73, + 0x6c, 0x2e, 0x63, 0x6f, 0x6d, 0x31, 0x1f, 0x30, 0x1d, 0x06, 0x09, 0x2a, + 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x09, 0x01, 0x16, 0x10, 0x69, 0x6e, + 0x66, 0x6f, 0x40, 0x77, 0x6f, 0x6c, 0x66, 0x73, 0x73, 0x6c, 0x2e, 0x63, + 0x6f, 0x6d, 0x30, 0x82, 0x01, 0x22, 0x30, 0x0d, 0x06, 0x09, 0x2a, 0x86, + 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x01, 0x05, 0x00, 0x03, 0x82, 0x01, + 0x0f, 0x00, 0x30, 0x82, 0x01, 0x0a, 0x02, 0x82, 0x01, 0x01, 0x00, 0xbf, + 0x0c, 0xca, 0x2d, 0x14, 0xb2, 0x1e, 0x84, 0x42, 0x5b, 0xcd, 0x38, 0x1f, + 0x4a, 0xf2, 0x4d, 0x75, 0x10, 0xf1, 0xb6, 0x35, 0x9f, 0xdf, 0xca, 0x7d, + 0x03, 0x98, 0xd3, 0xac, 0xde, 0x03, 0x66, 0xee, 0x2a, 0xf1, 0xd8, 0xb0, + 0x7d, 0x6e, 0x07, 0x54, 0x0b, 0x10, 0x98, 0x21, 0x4d, 0x80, 0xcb, 0x12, + 0x20, 0xe7, 0xcc, 0x4f, 0xde, 0x45, 0x7d, 0xc9, 0x72, 0x77, 0x32, 0xea, + 0xca, 0x90, 0xbb, 0x69, 0x52, 0x10, 0x03, 0x2f, 0xa8, 0xf3, 0x95, 0xc5, + 0xf1, 0x8b, 0x62, 0x56, 0x1b, 0xef, 0x67, 0x6f, 0xa4, 0x10, 0x41, 0x95, + 0xad, 0x0a, 0x9b, 0xe3, 0xa5, 0xc0, 0xb0, 0xd2, 0x70, 0x76, 0x50, 0x30, + 0x5b, 0xa8, 0xe8, 0x08, 0x2c, 0x7c, 0xed, 0xa7, 0xa2, 0x7a, 0x8d, 0x38, + 0x29, 0x1c, 0xac, 0xc7, 0xed, 0xf2, 0x7c, 0x95, 0xb0, 0x95, 0x82, 0x7d, + 0x49, 0x5c, 0x38, 0xcd, 0x77, 0x25, 0xef, 0xbd, 0x80, 0x75, 0x53, 0x94, + 0x3c, 0x3d, 0xca, 0x63, 0x5b, 0x9f, 0x15, 0xb5, 0xd3, 0x1d, 0x13, 0x2f, + 0x19, 0xd1, 0x3c, 0xdb, 0x76, 0x3a, 0xcc, 0xb8, 0x7d, 0xc9, 0xe5, 0xc2, + 0xd7, 0xda, 0x40, 0x6f, 0xd8, 0x21, 0xdc, 0x73, 0x1b, 0x42, 0x2d, 0x53, + 0x9c, 0xfe, 0x1a, 0xfc, 0x7d, 0xab, 0x7a, 0x36, 0x3f, 0x98, 0xde, 0x84, + 0x7c, 0x05, 0x67, 0xce, 0x6a, 0x14, 0x38, 0x87, 0xa9, 0xf1, 0x8c, 0xb5, + 0x68, 0xcb, 0x68, 0x7f, 0x71, 0x20, 0x2b, 0xf5, 0xa0, 0x63, 0xf5, 0x56, + 0x2f, 0xa3, 0x26, 0xd2, 0xb7, 0x6f, 0xb1, 0x5a, 0x17, 0xd7, 0x38, 0x99, + 0x08, 0xfe, 0x93, 0x58, 0x6f, 0xfe, 0xc3, 0x13, 0x49, 0x08, 0x16, 0x0b, + 0xa7, 0x4d, 0x67, 0x00, 0x52, 0x31, 0x67, 0x23, 0x4e, 0x98, 0xed, 0x51, + 0x45, 0x1d, 0xb9, 0x04, 0xd9, 0x0b, 0xec, 0xd8, 0x28, 0xb3, 0x4b, 0xbd, + 0xed, 0x36, 0x79, 0x02, 0x03, 0x01, 0x00, 0x01, 0xa3, 0x82, 0x01, 0x45, + 0x30, 0x82, 0x01, 0x41, 0x30, 0x1d, 0x06, 0x03, 0x55, 0x1d, 0x0e, 0x04, + 0x16, 0x04, 0x14, 0x27, 0x8e, 0x67, 0x11, 0x74, 0xc3, 0x26, 0x1d, 0x3f, + 0xed, 0x33, 0x63, 0xb3, 0xa4, 0xd8, 0x1d, 0x30, 0xe5, 0xe8, 0xd5, 0x30, + 0x81, 0xd4, 0x06, 0x03, 0x55, 0x1d, 0x23, 0x04, 0x81, 0xcc, 0x30, 0x81, + 0xc9, 0x80, 0x14, 0x27, 0x8e, 0x67, 0x11, 0x74, 0xc3, 0x26, 0x1d, 0x3f, + 0xed, 0x33, 0x63, 0xb3, 0xa4, 0xd8, 0x1d, 0x30, 0xe5, 0xe8, 0xd5, 0xa1, + 0x81, 0x9a, 0xa4, 0x81, 0x97, 0x30, 0x81, 0x94, 0x31, 0x0b, 0x30, 0x09, + 0x06, 0x03, 0x55, 0x04, 0x06, 0x13, 0x02, 0x55, 0x53, 0x31, 0x10, 0x30, + 0x0e, 0x06, 0x03, 0x55, 0x04, 0x08, 0x0c, 0x07, 0x4d, 0x6f, 0x6e, 0x74, + 0x61, 0x6e, 0x61, 0x31, 0x10, 0x30, 0x0e, 0x06, 0x03, 0x55, 0x04, 0x07, + 0x0c, 0x07, 0x42, 0x6f, 0x7a, 0x65, 0x6d, 0x61, 0x6e, 0x31, 0x11, 0x30, + 0x0f, 0x06, 0x03, 0x55, 0x04, 0x0a, 0x0c, 0x08, 0x53, 0x61, 0x77, 0x74, + 0x6f, 0x6f, 0x74, 0x68, 0x31, 0x13, 0x30, 0x11, 0x06, 0x03, 0x55, 0x04, + 0x0b, 0x0c, 0x0a, 0x43, 0x6f, 0x6e, 0x73, 0x75, 0x6c, 0x74, 0x69, 0x6e, + 0x67, 0x31, 0x18, 0x30, 0x16, 0x06, 0x03, 0x55, 0x04, 0x03, 0x0c, 0x0f, + 0x77, 0x77, 0x77, 0x2e, 0x77, 0x6f, 0x6c, 0x66, 0x73, 0x73, 0x6c, 0x2e, + 0x63, 0x6f, 0x6d, 0x31, 0x1f, 0x30, 0x1d, 0x06, 0x09, 0x2a, 0x86, 0x48, + 0x86, 0xf7, 0x0d, 0x01, 0x09, 0x01, 0x16, 0x10, 0x69, 0x6e, 0x66, 0x6f, + 0x40, 0x77, 0x6f, 0x6c, 0x66, 0x73, 0x73, 0x6c, 0x2e, 0x63, 0x6f, 0x6d, + 0x82, 0x14, 0x3f, 0x29, 0x11, 0x20, 0x57, 0x71, 0xe7, 0x8e, 0xf9, 0x18, + 0x0d, 0xca, 0x70, 0x4d, 0x5b, 0x15, 0x2a, 0x43, 0xd6, 0x24, 0x30, 0x0c, + 0x06, 0x03, 0x55, 0x1d, 0x13, 0x04, 0x05, 0x30, 0x03, 0x01, 0x01, 0xff, + 0x30, 0x1c, 0x06, 0x03, 0x55, 0x1d, 0x11, 0x04, 0x15, 0x30, 0x13, 0x82, + 0x0b, 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f, 0x6d, + 0x87, 0x04, 0x7f, 0x00, 0x00, 0x01, 0x30, 0x1d, 0x06, 0x03, 0x55, 0x1d, + 0x25, 0x04, 0x16, 0x30, 0x14, 0x06, 0x08, 0x2b, 0x06, 0x01, 0x05, 0x05, + 0x07, 0x03, 0x01, 0x06, 0x08, 0x2b, 0x06, 0x01, 0x05, 0x05, 0x07, 0x03, + 0x02, 0x30, 0x0d, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, + 0x01, 0x0b, 0x05, 0x00, 0x03, 0x82, 0x01, 0x01, 0x00, 0x0f, 0xae, 0x89, + 0xd5, 0x68, 0xe4, 0x41, 0xf8, 0x9b, 0xe0, 0xc5, 0x61, 0x06, 0x57, 0xff, + 0xa0, 0x92, 0x0f, 0xb2, 0xed, 0xd3, 0x99, 0x5b, 0x99, 0x5e, 0x32, 0x7e, + 0x97, 0xc7, 0xaf, 0x6c, 0xfe, 0x8c, 0xa6, 0xae, 0x32, 0xa1, 0x0d, 0xca, + 0xcd, 0xfc, 0x18, 0xe5, 0xd1, 0xf8, 0x20, 0x5b, 0x5a, 0x38, 0x81, 0x46, + 0x5b, 0x48, 0x87, 0xa5, 0x3f, 0x3b, 0x7b, 0xc7, 0xea, 0xf5, 0x35, 0x29, + 0x31, 0x15, 0x39, 0x38, 0x5d, 0x48, 0xe6, 0x01, 0x81, 0x5c, 0x5e, 0x7c, + 0x10, 0xf5, 0x16, 0xe3, 0x59, 0xaf, 0x44, 0xc8, 0xb5, 0x8d, 0xc1, 0x32, + 0x23, 0xb3, 0xb8, 0x12, 0x6e, 0x5c, 0x8d, 0xe6, 0xc2, 0xd2, 0x41, 0x03, + 0xeb, 0x17, 0x42, 0xe2, 0x7f, 0xbc, 0x00, 0x5d, 0xa5, 0x31, 0xef, 0xc6, + 0x48, 0xee, 0xdb, 0xcc, 0xe0, 0xf1, 0x56, 0xf5, 0xd4, 0xca, 0x45, 0xa1, + 0x59, 0xb5, 0xe4, 0xd7, 0x60, 0x9c, 0x57, 0xe0, 0xa7, 0x5a, 0xf2, 0x35, + 0x1e, 0xa0, 0x22, 0xdb, 0x5e, 0x1c, 0x0c, 0x61, 0xbd, 0xa1, 0xc5, 0x7b, + 0x9f, 0x69, 0xf2, 0xd5, 0x95, 0xe2, 0xbc, 0x52, 0xb9, 0x1d, 0x9c, 0x2c, + 0xda, 0xb6, 0x73, 0x75, 0x4a, 0x84, 0xe5, 0x94, 0xb8, 0x19, 0x4d, 0xdd, + 0x70, 0xbd, 0x7f, 0x4c, 0xb9, 0x17, 0x6a, 0x58, 0x16, 0x89, 0x22, 0x44, + 0x37, 0x57, 0x55, 0x26, 0x42, 0xe3, 0xb7, 0xe5, 0xc7, 0x2b, 0x40, 0x0c, + 0xe9, 0xe4, 0x7f, 0x52, 0x75, 0xdf, 0x06, 0xc9, 0xfb, 0x01, 0x44, 0x34, + 0xac, 0x20, 0x3c, 0xb4, 0xbe, 0x2b, 0x3e, 0xef, 0x85, 0x38, 0x96, 0x5b, + 0x9b, 0x1e, 0x25, 0x86, 0x18, 0x4c, 0xa4, 0x06, 0x70, 0x06, 0x6a, 0xc8, + 0x4b, 0x6f, 0x5f, 0xc4, 0x05, 0x1f, 0x03, 0x62, 0x30, 0x11, 0x61, 0xbc, + 0xc1, 0x40, 0x31, 0x66, 0xdc, 0x64, 0xf0, 0x4f, 0x6b, 0xb9, 0xec, 0xc8, + 0x29, 0x30, 0x82, 0x01, 0xa4, 0x30, 0x82, 0x01, 0x49, 0xa0, 0x03, 0x02, + 0x01, 0x02, 0x02, 0x14, 0x62, 0x4d, 0x11, 0x9c, 0xcf, 0x5d, 0xe5, 0x71, + 0xa2, 0x82, 0xd9, 0x8f, 0xe0, 0x04, 0xb8, 0x5f, 0x0e, 0x4d, 0x07, 0xad, + 0x30, 0x0a, 0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x04, 0x03, 0x02, + 0x30, 0x27, 0x31, 0x11, 0x30, 0x0f, 0x06, 0x03, 0x55, 0x04, 0x03, 0x0c, + 0x08, 0x61, 0x74, 0x74, 0x61, 0x63, 0x6b, 0x65, 0x72, 0x31, 0x12, 0x30, + 0x10, 0x06, 0x03, 0x55, 0x04, 0x0a, 0x0c, 0x09, 0x75, 0x6e, 0x74, 0x72, + 0x75, 0x73, 0x74, 0x65, 0x64, 0x30, 0x1e, 0x17, 0x0d, 0x32, 0x36, 0x30, + 0x34, 0x32, 0x31, 0x31, 0x31, 0x31, 0x36, 0x32, 0x38, 0x5a, 0x17, 0x0d, + 0x33, 0x36, 0x30, 0x34, 0x31, 0x38, 0x31, 0x31, 0x31, 0x36, 0x32, 0x38, + 0x5a, 0x30, 0x27, 0x31, 0x11, 0x30, 0x0f, 0x06, 0x03, 0x55, 0x04, 0x03, + 0x0c, 0x08, 0x61, 0x74, 0x74, 0x61, 0x63, 0x6b, 0x65, 0x72, 0x31, 0x12, + 0x30, 0x10, 0x06, 0x03, 0x55, 0x04, 0x0a, 0x0c, 0x09, 0x75, 0x6e, 0x74, + 0x72, 0x75, 0x73, 0x74, 0x65, 0x64, 0x30, 0x59, 0x30, 0x13, 0x06, 0x07, + 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, 0x01, 0x06, 0x08, 0x2a, 0x86, 0x48, + 0xce, 0x3d, 0x03, 0x01, 0x07, 0x03, 0x42, 0x00, 0x04, 0xae, 0xdb, 0xf7, + 0x3b, 0x7e, 0x82, 0x88, 0xfc, 0x1a, 0xfb, 0x86, 0x56, 0x83, 0x03, 0xdd, + 0x05, 0x14, 0x79, 0x51, 0x0f, 0x3c, 0x86, 0x85, 0x2d, 0xeb, 0x18, 0x17, + 0x20, 0x3b, 0x37, 0x6f, 0x7f, 0x78, 0x19, 0x3b, 0xf6, 0x71, 0xad, 0xc9, + 0x65, 0x81, 0x7e, 0xe0, 0xa9, 0x29, 0xdd, 0xfd, 0xf0, 0xff, 0x04, 0x7d, + 0x5a, 0x59, 0xd6, 0x6c, 0xe2, 0xde, 0xc5, 0xd5, 0xb6, 0x1f, 0x69, 0xd9, + 0x33, 0xa3, 0x53, 0x30, 0x51, 0x30, 0x1d, 0x06, 0x03, 0x55, 0x1d, 0x0e, + 0x04, 0x16, 0x04, 0x14, 0xf7, 0xab, 0x3f, 0x49, 0xcf, 0x7d, 0x48, 0x9c, + 0x04, 0x49, 0x1a, 0xac, 0x8f, 0x26, 0x16, 0x09, 0xa8, 0x2a, 0x74, 0xf5, + 0x30, 0x1f, 0x06, 0x03, 0x55, 0x1d, 0x23, 0x04, 0x18, 0x30, 0x16, 0x80, + 0x14, 0xf7, 0xab, 0x3f, 0x49, 0xcf, 0x7d, 0x48, 0x9c, 0x04, 0x49, 0x1a, + 0xac, 0x8f, 0x26, 0x16, 0x09, 0xa8, 0x2a, 0x74, 0xf5, 0x30, 0x0f, 0x06, + 0x03, 0x55, 0x1d, 0x13, 0x01, 0x01, 0xff, 0x04, 0x05, 0x30, 0x03, 0x01, + 0x01, 0xff, 0x30, 0x0a, 0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x04, + 0x03, 0x02, 0x03, 0x49, 0x00, 0x30, 0x46, 0x02, 0x21, 0x00, 0x8d, 0xbf, + 0x36, 0xe5, 0x51, 0x9a, 0xde, 0xf4, 0x7f, 0xbf, 0xbd, 0x7f, 0x71, 0x66, + 0xc1, 0x67, 0xfa, 0x71, 0x0d, 0x79, 0xc6, 0x60, 0x3a, 0x6c, 0xeb, 0x43, + 0xc3, 0xf2, 0x5e, 0xe8, 0x74, 0xb6, 0x02, 0x21, 0x00, 0xfa, 0xdb, 0x40, + 0x47, 0x72, 0xf0, 0x15, 0x52, 0xc1, 0x78, 0x11, 0x6b, 0x76, 0xc5, 0x1f, + 0xcf, 0xb6, 0x09, 0x6d, 0x8f, 0xcb, 0x92, 0x2f, 0x1b, 0x3c, 0xc3, 0x28, + 0x48, 0x61, 0x0f, 0x60, 0x71, 0x31, 0x81, 0xa8, 0x30, 0x81, 0xa5, 0x02, + 0x01, 0x01, 0x30, 0x3f, 0x30, 0x27, 0x31, 0x11, 0x30, 0x0f, 0x06, 0x03, + 0x55, 0x04, 0x03, 0x0c, 0x08, 0x61, 0x74, 0x74, 0x61, 0x63, 0x6b, 0x65, + 0x72, 0x31, 0x12, 0x30, 0x10, 0x06, 0x03, 0x55, 0x04, 0x0a, 0x0c, 0x09, + 0x75, 0x6e, 0x74, 0x72, 0x75, 0x73, 0x74, 0x65, 0x64, 0x02, 0x14, 0x62, + 0x4d, 0x11, 0x9c, 0xcf, 0x5d, 0xe5, 0x71, 0xa2, 0x82, 0xd9, 0x8f, 0xe0, + 0x04, 0xb8, 0x5f, 0x0e, 0x4d, 0x07, 0xad, 0x30, 0x0b, 0x06, 0x09, 0x60, + 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01, 0x30, 0x0a, 0x06, 0x08, + 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x04, 0x03, 0x02, 0x04, 0x46, 0x30, 0x44, + 0x02, 0x20, 0x22, 0x4a, 0x99, 0xb1, 0xbc, 0xa9, 0xee, 0x24, 0x60, 0x81, + 0xb9, 0x64, 0xba, 0x86, 0x00, 0xae, 0xb5, 0xd7, 0xb8, 0x72, 0xb9, 0x8c, + 0xb3, 0xe7, 0x78, 0x29, 0xdb, 0xa8, 0x27, 0xf7, 0x30, 0xf0, 0x02, 0x20, + 0x19, 0x2d, 0xd3, 0x17, 0x9a, 0xc1, 0xf9, 0xd2, 0x63, 0x92, 0x8e, 0x78, + 0xcc, 0xa4, 0x0b, 0x91, 0x12, 0xa5, 0xb2, 0xbc, 0x35, 0x87, 0x8e, 0x33, + 0xa7, 0xe0, 0x5e, 0xab, 0x95, 0xb2, 0x2a, 0xf4 + }; + PKCS7* p7 = NULL; + X509_STORE* store = NULL; + X509* caCert = NULL; + WOLFSSL_BIO* caBio = NULL; + const byte* p = forgedSignedData; + const char* ca = "./certs/ca-cert.pem"; + + /* Load the same CA into the trust store that the attacker bundled at + * cert[0] in the forged message. */ + ExpectNotNull(caBio = BIO_new_file(ca, "r")); + ExpectNotNull(caCert = PEM_read_bio_X509(caBio, NULL, 0, NULL)); + ExpectNotNull(store = X509_STORE_new()); + ExpectIntEQ(X509_STORE_add_cert(store, caCert), 1); + + /* Parse the forged message. d2i_PKCS7 internally runs + * wc_PKCS7_VerifySignedData, which must NOT accept the attacker's + * signature under any bundled cert other than the one named by the + * signerInfo sid. Since the sid names the attacker cert (which does + * not chain to the trusted CA), the parse may succeed but verification + * against the trust store must fail. */ + ExpectNotNull(p7 = d2i_PKCS7(NULL, &p, (int)sizeof(forgedSignedData))); + + /* PKCS7_verify() MUST fail: the only certificate in the trust store + * is the wolfSSL CA - it is bundled at cert[0] but did NOT sign this + * message. The actual signer (the attacker's self-signed cert at + * cert[1]) cannot chain to any trust anchor. */ + ExpectIntEQ(PKCS7_verify(p7, NULL, store, NULL, NULL, 0), + WC_NO_ERR_TRACE(WOLFSSL_FAILURE)); + + PKCS7_free(p7); + X509_STORE_free(store); + X509_free(caCert); + BIO_free(caBio); +#endif + return EXPECT_RESULT(); +} + +/* Exercise the SignerInfo-sid binding enforcement end-to-end. + * + * For both supported sid encodings (v1 = IssuerAndSerialNumber, v3 = + * SubjectKeyIdentifier), this builds a valid CMS SignedData message with + * two certificates in the bundle: + * cert[0] = ca-cert (extra, non-signing) + * cert[1] = server-cert (actual signer) + * and checks that: + * - parsing + signature verification succeeds, + * - chain validation against a trust store containing ca-cert succeeds, + * - PKCS7_get0_signers() returns the *signer* (server-cert), not the + * extra cert at cert[0] - which would be the pre-fix behavior and the + * core of the signer-identity forgery bug. */ +int test_wolfSSL_PKCS7_verify_sid_binding(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 byte content[] = "sid-binding test content"; + /* Build both variants: default v1 (IssuerAndSerialNumber) and v3 + * (SubjectKeyIdentifier). */ + const int sidTypes[2] = { CMS_ISSUER_AND_SERIAL_NUMBER, CMS_SKID }; + int variant; + + BIO* signerCertBio = NULL; + BIO* caBio = NULL; + X509* signerCertX509 = NULL; + X509* caX509 = NULL; + byte* signerCertDer = NULL; + byte* caDer = NULL; + byte* signerKey = NULL; + int signerCertDerSz = 0; + int caDerSz = 0; + size_t signerKeySz = 0; + XFILE keyFile = XBADFILE; + WC_RNG rng; + int rngInited = 0; + + /* ---- Load signer cert + key and the CA cert. ---- */ + ExpectNotNull(signerCertBio = BIO_new_file(signerCertFile, "r")); + ExpectNotNull(signerCertX509 = PEM_read_bio_X509(signerCertBio, NULL, 0, + NULL)); + ExpectIntGT(signerCertDerSz = i2d_X509(signerCertX509, &signerCertDer), 0); + + ExpectNotNull(caBio = BIO_new_file(caFile, "r")); + ExpectNotNull(caX509 = PEM_read_bio_X509(caBio, NULL, 0, NULL)); + ExpectIntGT(caDerSz = i2d_X509(caX509, &caDer), 0); + + /* Slurp the DER private key straight from a PEM->DER round-trip via + * wc_KeyPemToDer. The test only needs the bytes in a form + * wc_PKCS7_EncodeSignedData can consume. */ + { + long filePemLen = 0; + byte* keyPem = NULL; + int derLen = 0; + + ExpectTrue((keyFile = XFOPEN(signerKeyFile, "rb")) != XBADFILE); + if (keyFile != XBADFILE) { + (void)XFSEEK(keyFile, 0, XSEEK_END); + filePemLen = XFTELL(keyFile); + (void)XFSEEK(keyFile, 0, XSEEK_SET); + ExpectIntGT(filePemLen, 0); + keyPem = (byte*)XMALLOC((size_t)filePemLen, NULL, + DYNAMIC_TYPE_TMP_BUFFER); + ExpectNotNull(keyPem); + if (keyPem != NULL) { + ExpectIntEQ(XFREAD(keyPem, 1, (size_t)filePemLen, keyFile), + (size_t)filePemLen); + /* First call sizes the output buffer. */ + derLen = wc_KeyPemToDer(keyPem, (word32)filePemLen, NULL, 0, + NULL); + ExpectIntGT(derLen, 0); + if (derLen > 0) { + signerKey = (byte*)XMALLOC((size_t)derLen, NULL, + DYNAMIC_TYPE_TMP_BUFFER); + ExpectNotNull(signerKey); + if (signerKey != NULL) { + derLen = wc_KeyPemToDer(keyPem, (word32)filePemLen, + signerKey, (word32)derLen, + NULL); + ExpectIntGT(derLen, 0); + signerKeySz = (size_t)derLen; + } + } + } + XFREE(keyPem, NULL, DYNAMIC_TYPE_TMP_BUFFER); + XFCLOSE(keyFile); + keyFile = XBADFILE; + } + } + + ExpectIntEQ(wc_InitRng(&rng), 0); + if (EXPECT_SUCCESS()) + rngInited = 1; + + for (variant = 0; variant < 2; variant++) { + wc_PKCS7* p7Enc = NULL; + byte encoded[4096]; + int encodedSz = 0; + PKCS7* p7Ver = NULL; + X509_STORE* store = NULL; + const byte* encodedPtr = NULL; + STACK_OF(X509)* signers = NULL; + X509* reportedSigner = NULL; + byte* reportedSignerDer = NULL; + int reportedSignerDerSz = 0; + X509* caForStore = NULL; + BIO* caForStoreBio = NULL; + + /* ---- Encode: signer=server-cert, extra bundle cert=ca. ---- */ + ExpectNotNull(p7Enc = wc_PKCS7_New(HEAP_HINT, INVALID_DEVID)); + ExpectIntEQ(wc_PKCS7_Init(p7Enc, HEAP_HINT, INVALID_DEVID), 0); + ExpectIntEQ(wc_PKCS7_InitWithCert(p7Enc, signerCertDer, + (word32)signerCertDerSz), 0); + /* wc_PKCS7_AddCertificate prepends to the cert list - the encoded + * SET therefore ends up [ca, signer], putting the actual signer + * at index 1 and exercising the sid-selection path (cert[0] is + * NOT the signer and must be skipped). */ + ExpectIntEQ(wc_PKCS7_AddCertificate(p7Enc, caDer, (word32)caDerSz), 0); + + if (p7Enc != NULL) { + p7Enc->content = (byte*)content; + p7Enc->contentSz = (word32)sizeof(content); + p7Enc->encryptOID = RSAk; + p7Enc->hashOID = SHA256h; + p7Enc->privateKey = signerKey; + p7Enc->privateKeySz = (word32)signerKeySz; + p7Enc->rng = &rng; + } + + ExpectIntEQ(wc_PKCS7_SetSignerIdentifierType(p7Enc, sidTypes[variant]), + 0); + + ExpectIntGT((encodedSz = wc_PKCS7_EncodeSignedData(p7Enc, encoded, + sizeof(encoded))), + 0); + wc_PKCS7_Free(p7Enc); + p7Enc = NULL; + + /* ---- Parse + verify through the OpenSSL compat layer. ---- */ + encodedPtr = encoded; + ExpectNotNull(p7Ver = d2i_PKCS7(NULL, &encodedPtr, encodedSz)); + + /* Trust store holds only ca-cert. Reload it rather than reusing + * caX509, since X509_STORE_free takes ownership-like semantics. */ + ExpectNotNull(caForStoreBio = BIO_new_file(caFile, "r")); + ExpectNotNull(caForStore = PEM_read_bio_X509(caForStoreBio, NULL, 0, + NULL)); + ExpectNotNull(store = X509_STORE_new()); + ExpectIntEQ(X509_STORE_add_cert(store, caForStore), 1); + + ExpectIntEQ(PKCS7_verify(p7Ver, NULL, store, NULL, NULL, 0), 1); + + /* Snapshot the singleCert / verifyCert pointers and sizes after + * PKCS7_verify has finished re-parsing the message. A buggy + * implementation that passes &p7->pkcs7.singleCert (or verifyCert) + * directly to wolfSSL_d2i_X509 permanently advances the struct + * field, which corrupts it for any subsequent use - producing a + * heap-OOB read on the next call since the size isn't advanced. + * These pointers must stay exactly where they are across repeated + * get0_signers calls. */ + if (p7Ver != NULL) { + wc_PKCS7* wcP7 = &((WOLFSSL_PKCS7*)p7Ver)->pkcs7; + byte* singleBefore = wcP7->singleCert; + word32 singleSzBefore = wcP7->singleCertSz; + byte* verifyBefore = wcP7->verifyCert; + word32 verifySzBefore = wcP7->verifyCertSz; + int i; + + /* Call get0_signers repeatedly. Each invocation must return + * the correct cert and must not mutate singleCert/verifyCert. + * Three iterations so the "second call reads past the end" + * pattern (the exact OOB the reporter hit) is exercised. */ + for (i = 0; i < 3; i++) { + ExpectNotNull(signers = PKCS7_get0_signers(p7Ver, NULL, 0)); + ExpectIntEQ(sk_X509_num(signers), 1); + ExpectNotNull(reportedSigner = sk_X509_value(signers, 0)); + ExpectIntGT(reportedSignerDerSz = i2d_X509(reportedSigner, + &reportedSignerDer), 0); + /* DER-compare: reportedSigner must equal server-cert and + * must NOT equal ca-cert (the pre-fix signer-confusion + * outcome). */ + ExpectIntEQ(reportedSignerDerSz, signerCertDerSz); + if (reportedSignerDer != NULL && signerCertDer != NULL) { + ExpectIntEQ(XMEMCMP(reportedSignerDer, signerCertDer, + (size_t)signerCertDerSz), 0); + if (reportedSignerDerSz == caDerSz) { + ExpectIntNE(XMEMCMP(reportedSignerDer, caDer, + (size_t)caDerSz), 0); + } + } + XFREE(reportedSignerDer, NULL, DYNAMIC_TYPE_OPENSSL); + reportedSignerDer = NULL; + sk_X509_pop_free(signers, NULL); + signers = NULL; + + /* Struct fields must survive every call unchanged. */ + ExpectPtrEq(wcP7->singleCert, singleBefore); + ExpectIntEQ(wcP7->singleCertSz, singleSzBefore); + ExpectPtrEq(wcP7->verifyCert, verifyBefore); + ExpectIntEQ(wcP7->verifyCertSz, verifySzBefore); + } + } + + PKCS7_free(p7Ver); + X509_STORE_free(store); + X509_free(caForStore); + BIO_free(caForStoreBio); + } + + if (rngInited) + wc_FreeRng(&rng); + + XFREE(signerKey, NULL, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(signerCertDer, NULL, DYNAMIC_TYPE_OPENSSL); + XFREE(caDer, NULL, DYNAMIC_TYPE_OPENSSL); + X509_free(signerCertX509); + X509_free(caX509); + BIO_free(signerCertBio); + BIO_free(caBio); +#endif + return EXPECT_RESULT(); +} + int test_wolfSSL_PKCS7_SIGNED_new(void) { EXPECT_DECLS; diff --git a/tests/api/test_ossl_p7p12.h b/tests/api/test_ossl_p7p12.h index 58aa9dd12a..6704ab51ed 100644 --- a/tests/api/test_ossl_p7p12.h +++ b/tests/api/test_ossl_p7p12.h @@ -27,6 +27,8 @@ int test_wolfssl_PKCS7(void); int test_wolfSSL_PKCS7_certs(void); 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_SIGNED_new(void); int test_wolfSSL_PEM_write_bio_PKCS7(void); int test_wolfSSL_PEM_write_bio_encryptedKey(void); @@ -38,6 +40,8 @@ int test_wolfSSL_PKCS12(void); TEST_DECL_GROUP("ossl_p7", test_wolfssl_PKCS7), \ TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PKCS7_certs), \ TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PKCS7_sign), \ + 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_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 5d1a25bb31..2c1c3a80f0 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -97,6 +97,7 @@ typedef enum { /* holds information about the signers */ struct PKCS7SignerInfo { int version; + int sidType; /* CMS_ISSUER_AND_SERIAL_NUMBER or CMS_SKID */ byte *sid; word32 sidSz; }; @@ -4292,6 +4293,94 @@ int wc_PKCS7_SetEccSignRawDigestCb(wc_PKCS7* pkcs7, CallbackEccSignRawDigest cb) #endif /* HAVE_ECC */ +#if !defined(NO_RSA) || defined(HAVE_ECC) +/* Check whether the given decoded certificate matches the SignerIdentifier + * (sid) field of the currently parsed SignerInfo. Per RFC 5652 Section 5.3, + * the sid selects which certificate's public key must be used to verify the + * signature. Returns 1 on match, 0 on no match or when the sid is not + * available for comparison. */ +static int wc_PKCS7_CertMatchesSignerInfo(wc_PKCS7* pkcs7, DecodedCert* dCert) +{ + PKCS7SignerInfo* signerInfo; + + if (pkcs7 == NULL || dCert == NULL) + return 0; + + signerInfo = pkcs7->signerInfo; + if (signerInfo == NULL || signerInfo->sid == NULL || + signerInfo->sidSz == 0) { + /* No SID parsed, cannot perform an identity binding check. */ + return 0; + } + + if (signerInfo->sidType == CMS_ISSUER_AND_SERIAL_NUMBER) { + /* IssuerAndSerialNumber: SID blob stores the content of the outer + * SEQUENCE (issuer Name followed by INTEGER serialNumber). */ + word32 idx = 0; + byte sidIssuerHash[KEYID_SIZE]; + WC_DECLARE_VAR(sidSerial, mp_int, 1, pkcs7->heap); + WC_DECLARE_VAR(certSerial, mp_int, 1, pkcs7->heap); + int cmp; + int match = 0; + + if (GetNameHash_ex(signerInfo->sid, &idx, sidIssuerHash, + (int)signerInfo->sidSz, dCert->signatureOID) < 0) { + return 0; + } + if (XMEMCMP(sidIssuerHash, dCert->issuerHash, KEYID_SIZE) != 0) + return 0; + + WC_ALLOC_VAR_EX(sidSerial, mp_int, 1, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER, + { return 0; }); + WC_ALLOC_VAR_EX(certSerial, mp_int, 1, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER, + { WC_FREE_VAR_EX(sidSerial, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + return 0; }); + + if (mp_init(sidSerial) != MP_OKAY) { + WC_FREE_VAR_EX(sidSerial, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + WC_FREE_VAR_EX(certSerial, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + return 0; + } + if (mp_init(certSerial) != MP_OKAY) { + mp_clear(sidSerial); + WC_FREE_VAR_EX(sidSerial, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + WC_FREE_VAR_EX(certSerial, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + return 0; + } + + if (GetInt(sidSerial, signerInfo->sid, &idx, signerInfo->sidSz) == 0 && + mp_read_unsigned_bin(certSerial, dCert->serial, + (word32)dCert->serialSz) == MP_OKAY) { + cmp = mp_cmp(sidSerial, certSerial); + if (cmp == MP_EQ) + match = 1; + } + + mp_clear(sidSerial); + mp_clear(certSerial); + WC_FREE_VAR_EX(sidSerial, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + WC_FREE_VAR_EX(certSerial, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + return match; + } + else if (signerInfo->sidType == CMS_SKID) { + /* SubjectKeyIdentifier: SID blob is the raw SKID octet string + * content. Normalize the same way the certificate side does so + * that comparisons between SHA-1 SKIDs and other lengths match. */ + byte sidKid[KEYID_SIZE]; + + if (GetHashId(signerInfo->sid, (int)signerInfo->sidSz, sidKid, + HashIdAlg(dCert->signatureOID)) != 0) { + return 0; + } + if (XMEMCMP(sidKid, dCert->extSubjKeyId, KEYID_SIZE) == 0) + return 1; + return 0; + } + + return 0; +} +#endif /* !NO_RSA || HAVE_ECC */ + #ifndef NO_RSA /* returns size of signature put into out, negative on error */ @@ -4371,6 +4460,31 @@ static int wc_PKCS7_RsaVerify(wc_PKCS7* pkcs7, byte* sig, int sigSz, continue; } + /* If the SignerInfo sid was parsed, only try the certificate whose + * identity matches it. This binds the verifying public key to the + * signer identity advertised in the CMS message and prevents signer + * confusion when multiple certificates are embedded. */ + if (pkcs7->signerInfo != NULL && pkcs7->signerInfo->sid != NULL && + !wc_PKCS7_CertMatchesSignerInfo(pkcs7, dCert)) { + FreeDecodedCert(dCert); + wc_FreeRsaKey(key); + continue; + } + + /* Defense in depth: the sid-matched cert must actually carry an + * RSA-family key before we feed its SPKI to the RSA key decoder. + * Rejecting here avoids depending on wc_RsaPublicKeyDecode to reject + * wrong-type SPKIs. */ + if (dCert->keyOID != RSAk + #ifdef WC_RSA_PSS + && dCert->keyOID != RSAPSSk + #endif + ) { + FreeDecodedCert(dCert); + wc_FreeRsaKey(key); + continue; + } + if (wc_RsaPublicKeyDecode(dCert->publicKey, &scratch, key, dCert->pubKeySize) < 0) { WOLFSSL_MSG("ASN RSA key decode error"); @@ -4481,6 +4595,24 @@ static int wc_PKCS7_RsaPssVerify(wc_PKCS7* pkcs7, byte* sig, int sigSz, continue; } + /* Only try the certificate identified by the SignerInfo sid (see + * matching comment in wc_PKCS7_RsaVerify). */ + if (pkcs7->signerInfo != NULL && pkcs7->signerInfo->sid != NULL && + !wc_PKCS7_CertMatchesSignerInfo(pkcs7, dCert)) { + FreeDecodedCert(dCert); + wc_FreeRsaKey(key); + continue; + } + + /* Defense in depth: reject non-RSA SPKIs before key decode. RSA + * rsaEncryption certs (keyOID=RSAk) are accepted for PSS signatures + * per RFC 8017 - a RSASSA-PSS cert is not required. */ + if (dCert->keyOID != RSAk && dCert->keyOID != RSAPSSk) { + FreeDecodedCert(dCert); + wc_FreeRsaKey(key); + continue; + } + pkSz = dCert->pubKeySize; if (pkSz > (MAX_RSA_INT_SZ + MAX_RSA_E_SZ)) pkSz = (MAX_RSA_INT_SZ + MAX_RSA_E_SZ); @@ -4646,6 +4778,22 @@ static int wc_PKCS7_EcdsaVerify(wc_PKCS7* pkcs7, byte* sig, int sigSz, continue; } + /* Only try the certificate identified by the SignerInfo sid (see + * matching comment in wc_PKCS7_RsaVerify). */ + if (pkcs7->signerInfo != NULL && pkcs7->signerInfo->sid != NULL && + !wc_PKCS7_CertMatchesSignerInfo(pkcs7, dCert)) { + FreeDecodedCert(dCert); + wc_ecc_free(key); + continue; + } + + /* Defense in depth: reject non-ECDSA SPKIs before key decode. */ + if (dCert->keyOID != ECDSAk) { + FreeDecodedCert(dCert); + wc_ecc_free(key); + continue; + } + if (wc_EccPublicKeyDecode(dCert->publicKey, &idx, key, dCert->pubKeySize) < 0) { WOLFSSL_MSG("ASN ECC key decode error"); @@ -5359,11 +5507,16 @@ static int wc_PKCS7_ParseSignerInfo(wc_PKCS7* pkcs7, byte* in, word32 inSz, ret = ASN_PARSE_E; if (ret == 0) { + pkcs7->signerInfo->sidType = CMS_ISSUER_AND_SERIAL_NUMBER; ret = wc_PKCS7_SignerInfoSetSID(pkcs7, in + idx, length); idx += (word32)length; } } else if (ret == 0 && version == 3) { + /* Default: SignerInfo version 3 carries SubjectKeyIdentifier. + * May be overridden below if the parser instead finds a + * SEQUENCE (IssuerAndSerialNumber fallback). */ + pkcs7->signerInfo->sidType = CMS_SKID; /* Get the sequence of SubjectKeyIdentifier */ if (idx + 1 > inSz) ret = BUFFER_E; @@ -5406,6 +5559,12 @@ static int wc_PKCS7_ParseSignerInfo(wc_PKCS7* pkcs7, byte* in, word32 inSz, if (ret == 0 && GetSequence(in, &idx, &length, inSz) < 0) ret = ASN_PARSE_E; + + if (ret == 0) { + /* v3 carrying IssuerAndSerialNumber fallback */ + pkcs7->signerInfo->sidType = + CMS_ISSUER_AND_SERIAL_NUMBER; + } } }