From d382439c7c63c07294cf4fcece0aa56a35399d15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 16 Jun 2026 13:03:59 +0200 Subject: [PATCH 1/7] PKCS7: tighten signature presence check in PKCS7_verify Ensure a signer signature is actually verified before reporting a PKCS7 SignedData object as verified, and add a regression test. --- src/ssl_p7p12.c | 13 +++++ tests/api/test_ossl_p7p12.c | 95 +++++++++++++++++++++++++++++++++++++ tests/api/test_ossl_p7p12.h | 2 + 3 files changed, 110 insertions(+) diff --git a/src/ssl_p7p12.c b/src/ssl_p7p12.c index 167083554d8..68a54ca54e5 100644 --- a/src/ssl_p7p12.c +++ b/src/ssl_p7p12.c @@ -816,6 +816,19 @@ int wolfSSL_PKCS7_verify(PKCS7* pkcs7, WOLFSSL_STACK* certs, if (ret != 0) return WOLFSSL_FAILURE; + /* Reject a degenerate (certs-only) PKCS#7 with no verified signer. Such an + * object has empty signerInfos, so wc_PKCS7_VerifySignedData() succeeds + * without authenticating the content. pkcs7.verifyCert is only set once a + * signer's signature has actually been verified, so a NULL value here means + * the content carries no valid signature and must not be reported as + * verified - regardless of PKCS7_NOVERIFY, which only suppresses signer + * certificate chain validation, not the requirement that a signature exist. + */ + if (p7->pkcs7.verifyCert == NULL) { + WOLFSSL_MSG("PKCS7 has no verified signer (degenerate/certs-only)"); + return WOLFSSL_FAILURE; + } + if ((flags & PKCS7_NOVERIFY) != PKCS7_NOVERIFY) { /* Verify signer certificates */ if (store == NULL || store->cm == NULL) { diff --git a/tests/api/test_ossl_p7p12.c b/tests/api/test_ossl_p7p12.c index 53f3d8749df..0c02f02659a 100644 --- a/tests/api/test_ossl_p7p12.c +++ b/tests/api/test_ossl_p7p12.c @@ -675,6 +675,101 @@ int test_wolfSSL_PKCS7_verify_signer_forgery(void) 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 + * PKCS7_NOVERIFY is set (which only suppresses signer-cert chain validation, + * not the requirement that a signature exist). */ +int test_wolfSSL_PKCS7_verify_degenerate(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* caFile = "./certs/ca-cert.pem"; + const byte attackerContent[] = "ATTACKER-CONTENT"; + PKCS7* encodeP7 = NULL; + PKCS7* p7 = NULL; + X509* signerCert = NULL; + X509* caCert = NULL; + STACK_OF(X509)* sk = NULL; + X509_STORE* store = NULL; + BIO* certBio = NULL; + BIO* caBio = NULL; + BIO* derBio = NULL; + BIO* inBio = NULL; + BIO* outBio = NULL; + const byte* der = NULL; + const byte* outData = NULL; + int derSz = 0; + + /* ---- Build a degenerate certs-only PKCS#7 wrapping server-cert. ---- */ + ExpectNotNull(certBio = BIO_new_file(signerCertFile, "r")); + ExpectNotNull(signerCert = PEM_read_bio_X509(certBio, NULL, 0, NULL)); + ExpectNotNull(sk = sk_X509_new_null()); + ExpectIntGT(sk_X509_push(sk, signerCert), 0); + if (EXPECT_SUCCESS()) + signerCert = NULL; /* now owned by sk */ + + ExpectNotNull(encodeP7 = PKCS7_new()); + if (encodeP7 != NULL) { + encodeP7->version = 1; + #ifdef NO_SHA + encodeP7->hashOID = SHA256h; + #else + encodeP7->hashOID = SHAh; + #endif + } + ExpectNotNull(derBio = BIO_new(BIO_s_mem())); + /* wolfSSL_PKCS7_encode_certs() takes ownership of sk (sets p7->certs, freed + * by PKCS7_free(encodeP7) below) whenever it is called with a valid PKCS7 + * and BIO - success or failure. It only declines ownership when encodeP7 or + * derBio is NULL, in which case the test still owns sk and frees it in + * cleanup. */ + ExpectIntEQ(wolfSSL_PKCS7_encode_certs(encodeP7, sk, derBio), 1); + if (encodeP7 != NULL && derBio != NULL) + sk = NULL; /* now owned by encodeP7 */ + ExpectIntGT((derSz = BIO_get_mem_data(derBio, &der)), 0); + + /* ---- Re-parse it; a degenerate bundle parses successfully. ---- */ + ExpectNotNull(p7 = d2i_PKCS7(NULL, &der, derSz)); + + /* ---- Trust store contains the embedded cert's issuer (ca-cert). ---- */ + ExpectNotNull(caBio = BIO_new_file(caFile, "r")); + ExpectNotNull(caCert = PEM_read_bio_X509(caBio, NULL, 0, NULL)); + ExpectNotNull(store = X509_STORE_new()); + ExpectIntEQ(X509_STORE_add_cert(store, caCert), 1); + + /* ---- Attacker-supplied detached content + capture of any output. ---- */ + ExpectNotNull(inBio = BIO_new_mem_buf(attackerContent, + (int)sizeof(attackerContent) - 1)); + ExpectNotNull(outBio = BIO_new(BIO_s_mem())); + + /* With a trust store (flags = 0): must be rejected, and the attacker + * content must NOT be emitted through the output BIO. */ + ExpectIntEQ(PKCS7_verify(p7, NULL, store, inBio, outBio, 0), + WC_NO_ERR_TRACE(WOLFSSL_FAILURE)); + ExpectIntLE(BIO_get_mem_data(outBio, &outData), 0); + + /* PKCS7_NOVERIFY must not turn a signature-less bundle into a pass. */ + ExpectIntEQ(PKCS7_verify(p7, NULL, store, inBio, NULL, PKCS7_NOVERIFY), + WC_NO_ERR_TRACE(WOLFSSL_FAILURE)); + + PKCS7_free(p7); + PKCS7_free(encodeP7); /* frees sk + signerCert once owned */ + X509_STORE_free(store); + X509_free(caCert); + sk_X509_pop_free(sk, X509_free); /* no-op if ownership passed to encodeP7 */ + X509_free(signerCert); /* no-op if pushed onto sk */ + BIO_free(certBio); + BIO_free(caBio); + BIO_free(derBio); + BIO_free(inBio); + BIO_free(outBio); +#endif + return EXPECT_RESULT(); +} + /* Exercise the SignerInfo-sid binding enforcement end-to-end. * * For both supported sid encodings (v1 = IssuerAndSerialNumber, v3 = diff --git a/tests/api/test_ossl_p7p12.h b/tests/api/test_ossl_p7p12.h index 6704ab51edd..8ab8fd79530 100644 --- a/tests/api/test_ossl_p7p12.h +++ b/tests/api/test_ossl_p7p12.h @@ -29,6 +29,7 @@ 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_verify_degenerate(void); int test_wolfSSL_PKCS7_SIGNED_new(void); int test_wolfSSL_PEM_write_bio_PKCS7(void); int test_wolfSSL_PEM_write_bio_encryptedKey(void); @@ -42,6 +43,7 @@ int test_wolfSSL_PKCS12(void); 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_verify_degenerate), \ 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), \ From 3e30e69c35b4aee3f8cf95449baae820c5175bbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 16 Jun 2026 14:02:00 +0200 Subject: [PATCH 2/7] certman: enforce keyCertSign usage on chain-supplied intermediate CAs Require the keyCertSign key usage on non-root intermediate CAs added during path building when a KeyUsage extension is present, per RFC 5280. Adds a regression test. --- src/ssl_certman.c | 11 +- tests/api/test_ossl_x509_str.c | 183 +++++++++++++++++++++++++++++++++ tests/api/test_ossl_x509_str.h | 3 + 3 files changed, 194 insertions(+), 3 deletions(-) diff --git a/src/ssl_certman.c b/src/ssl_certman.c index d10e79d56f3..094051d2908 100644 --- a/src/ssl_certman.c +++ b/src/ssl_certman.c @@ -3205,10 +3205,15 @@ int AddCA(WOLFSSL_CERT_MANAGER* cm, DerBuffer** pDer, int type, int verify) } #ifndef ALLOW_INVALID_CERTSIGN else if (ret == 0 && cert->isCA == 1 && type != WOLFSSL_USER_CA && - type != WOLFSSL_TEMP_CA && !cert->selfSigned && + !cert->selfSigned && cert->extKeyUsageSet && (cert->extKeyUsage & KEYUSE_KEY_CERT_SIGN) == 0) { - /* Intermediate CA certs are required to have the keyCertSign - * extension set. User loaded root certs are not. */ + /* Intermediate CA certs - including chain-supplied temporary CAs + * (WOLFSSL_TEMP_CA) added while building a path - are required to have + * the keyCertSign key usage when a Key Usage extension is present. + * Only operator-loaded root certs (WOLFSSL_USER_CA) and self-signed + * roots are exempt. Per RFC 5280 an absent Key Usage extension implies + * all usages, so only enforce this when the extension is actually + * present (extKeyUsageSet). */ WOLFSSL_MSG("\tDoesn't have key usage certificate signing"); ret = NOT_CA_ERROR; } diff --git a/tests/api/test_ossl_x509_str.c b/tests/api/test_ossl_x509_str.c index 964c862a6a3..2f1af0cc37f 100644 --- a/tests/api/test_ossl_x509_str.c +++ b/tests/api/test_ossl_x509_str.c @@ -1507,6 +1507,189 @@ int test_X509_verify_cert_untrusted_inter(void) return EXPECT_RESULT(); } +#if defined(OPENSSL_EXTRA) && !defined(NO_RSA) && !defined(NO_CERTS) && \ + defined(WOLFSSL_CERT_GEN) && defined(WOLFSSL_CERT_EXT) && \ + !defined(NO_SHA256) && defined(USE_CERT_BUFFERS_2048) && \ + !defined(NO_ASN_TIME) +/* Build a CA:TRUE intermediate signed by the 2048-bit test root + * (ca_cert_der_2048 / ca_key_der_2048). keyUsage == NULL omits the KeyUsage + * extension entirely. Returns the DER length, or <= 0 on failure. */ +static int gen_ca_int_keyusage(byte* out, int outMax, RsaKey* subjKey, + RsaKey* caKey, WC_RNG* rng, const char* cn, const char* keyUsage) +{ + Cert cert; + int sz; + + if (wc_InitCert(&cert) != 0) + return -1; + cert.isCA = 1; + cert.sigType = CTC_SHA256wRSA; + XSTRNCPY(cert.subject.country, "US", CTC_NAME_SIZE - 1); + XSTRNCPY(cert.subject.org, "wolfSSL_test", CTC_NAME_SIZE - 1); + XSTRNCPY(cert.subject.commonName, cn, CTC_NAME_SIZE - 1); + if (wc_SetSubjectKeyIdFromPublicKey(&cert, subjKey, NULL) != 0) + return -1; + if (wc_SetAuthKeyIdFromCert(&cert, ca_cert_der_2048, + (int)sizeof_ca_cert_der_2048) != 0) + return -1; + if (keyUsage != NULL && wc_SetKeyUsage(&cert, keyUsage) != 0) + return -1; + if (wc_SetIssuerBuffer(&cert, ca_cert_der_2048, + (int)sizeof_ca_cert_der_2048) != 0) + return -1; + if ((sz = wc_MakeCert(&cert, out, (word32)outMax, subjKey, NULL, rng)) < 0) + return -1; + return wc_SignCert(cert.bodySz, cert.sigType, out, (word32)outMax, caKey, + NULL, rng); +} + +/* Build a leaf signed by the given intermediate (its DER + private key). */ +static int gen_leaf_under_int(byte* out, int outMax, RsaKey* leafKey, + const byte* issuerDer, int issuerDerSz, RsaKey* issuerKey, WC_RNG* rng, + const char* cn) +{ + Cert cert; + int sz; + + if (wc_InitCert(&cert) != 0) + return -1; + cert.isCA = 0; + cert.sigType = CTC_SHA256wRSA; + XSTRNCPY(cert.subject.country, "US", CTC_NAME_SIZE - 1); + XSTRNCPY(cert.subject.org, "wolfSSL_test", CTC_NAME_SIZE - 1); + XSTRNCPY(cert.subject.commonName, cn, CTC_NAME_SIZE - 1); + if (wc_SetSubjectKeyIdFromPublicKey(&cert, leafKey, NULL) != 0) + return -1; + if (wc_SetAuthKeyIdFromCert(&cert, issuerDer, issuerDerSz) != 0) + return -1; + if (wc_SetKeyUsage(&cert, "digitalSignature") != 0) + return -1; + if (wc_SetIssuerBuffer(&cert, issuerDer, issuerDerSz) != 0) + return -1; + if ((sz = wc_MakeCert(&cert, out, (word32)outMax, leafKey, NULL, rng)) < 0) + return -1; + return wc_SignCert(cert.bodySz, cert.sigType, out, (word32)outMax, + issuerKey, NULL, rng); +} + +/* Verify "leaf <- intermediate <- root(ca-cert)" where the intermediate is + * supplied only as an untrusted candidate. Returns the X509_verify_cert() + * result (1 verified, 0 rejected) via *verifyRet. */ +static int run_int_keyusage_case(const byte* intDer, int intSz, + const byte* leafDer, int leafSz, X509* root, int* verifyRet) +{ + EXPECT_DECLS; + X509* inter = NULL; + X509* leaf = NULL; + X509_STORE* store = NULL; + X509_STORE_CTX* ctx = NULL; + STACK_OF(X509)* untrusted = NULL; + const byte* p; + + p = intDer; + ExpectNotNull(inter = d2i_X509(NULL, &p, intSz)); + p = leafDer; + ExpectNotNull(leaf = d2i_X509(NULL, &p, leafSz)); + ExpectNotNull(store = X509_STORE_new()); + ExpectIntEQ(X509_STORE_add_cert(store, root), 1); + ExpectNotNull(untrusted = sk_X509_new_null()); + ExpectIntGT(sk_X509_push(untrusted, inter), 0); + ExpectNotNull(ctx = X509_STORE_CTX_new()); + ExpectIntEQ(X509_STORE_CTX_init(ctx, store, leaf, untrusted), 1); + if (verifyRet != NULL) + *verifyRet = X509_verify_cert(ctx); + X509_STORE_CTX_free(ctx); + X509_STORE_free(store); + sk_X509_free(untrusted); + X509_free(leaf); + X509_free(inter); + return EXPECT_RESULT(); +} +#endif + +/* Regression: a chain-supplied (untrusted) intermediate that is CA:TRUE but + * whose KeyUsage extension does NOT assert keyCertSign must NOT be usable to + * sign the leaf - RFC 5280 4.2.1.3. Previously such an intermediate, added as + * a temporary CA during path building, was accepted as a signing CA. + * + * Conversely, an intermediate with NO KeyUsage extension implies all usages + * (including keyCertSign) and must still verify - the fix must not over-reject + * those. */ +int test_X509_verify_cert_ca_no_keycertsign(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) && !defined(NO_RSA) && !defined(NO_CERTS) && \ + defined(WOLFSSL_CERT_GEN) && defined(WOLFSSL_CERT_EXT) && \ + !defined(NO_SHA256) && defined(USE_CERT_BUFFERS_2048) && \ + !defined(NO_ASN_TIME) + WC_RNG rng; + RsaKey caKey, intKey, leafKey; + int rngI = 0, caI = 0, intI = 0, leafI = 0; + word32 idx; + byte* intDer = NULL; + byte* leafDer = NULL; + int intSz = 0, leafSz = 0; + int verifyRet = -1; + const byte* p; + X509* root = NULL; + + intDer = (byte*)XMALLOC(FOURK_BUF, NULL, DYNAMIC_TYPE_TMP_BUFFER); + leafDer = (byte*)XMALLOC(FOURK_BUF, NULL, DYNAMIC_TYPE_TMP_BUFFER); + ExpectNotNull(intDer); + ExpectNotNull(leafDer); + + ExpectIntEQ(wc_InitRng(&rng), 0); + if (EXPECT_SUCCESS()) rngI = 1; + ExpectIntEQ(wc_InitRsaKey(&caKey, NULL), 0); + if (EXPECT_SUCCESS()) caI = 1; + idx = 0; + ExpectIntEQ(wc_RsaPrivateKeyDecode(ca_key_der_2048, &idx, &caKey, + sizeof_ca_key_der_2048), 0); + ExpectIntEQ(wc_InitRsaKey(&intKey, NULL), 0); + if (EXPECT_SUCCESS()) intI = 1; + idx = 0; + ExpectIntEQ(wc_RsaPrivateKeyDecode(server_key_der_2048, &idx, &intKey, + sizeof_server_key_der_2048), 0); + ExpectIntEQ(wc_InitRsaKey(&leafKey, NULL), 0); + if (EXPECT_SUCCESS()) leafI = 1; + idx = 0; + ExpectIntEQ(wc_RsaPrivateKeyDecode(client_key_der_2048, &idx, &leafKey, + sizeof_client_key_der_2048), 0); + + p = ca_cert_der_2048; + ExpectNotNull(root = d2i_X509(NULL, &p, (int)sizeof_ca_cert_der_2048)); + + /* Case 1: intermediate CA WITHOUT keyCertSign -> verification must fail. */ + ExpectIntGT((intSz = gen_ca_int_keyusage(intDer, FOURK_BUF, &intKey, &caKey, + &rng, "No keyCertSign Intermediate", "digitalSignature")), 0); + ExpectIntGT((leafSz = gen_leaf_under_int(leafDer, FOURK_BUF, &leafKey, + intDer, intSz, &intKey, &rng, "Leaf under bad int")), 0); + verifyRet = -1; + ExpectIntEQ(run_int_keyusage_case(intDer, intSz, leafDer, leafSz, root, + &verifyRet), 1); + ExpectIntNE(verifyRet, 1); + + /* Case 2: intermediate CA with NO KeyUsage extension -> must verify. */ + ExpectIntGT((intSz = gen_ca_int_keyusage(intDer, FOURK_BUF, &intKey, &caKey, + &rng, "No KeyUsage Intermediate", NULL)), 0); + ExpectIntGT((leafSz = gen_leaf_under_int(leafDer, FOURK_BUF, &leafKey, + intDer, intSz, &intKey, &rng, "Leaf under noKU int")), 0); + verifyRet = -1; + ExpectIntEQ(run_int_keyusage_case(intDer, intSz, leafDer, leafSz, root, + &verifyRet), 1); + ExpectIntEQ(verifyRet, 1); + + X509_free(root); + if (rngI) wc_FreeRng(&rng); + if (caI) wc_FreeRsaKey(&caKey); + if (intI) wc_FreeRsaKey(&intKey); + if (leafI) wc_FreeRsaKey(&leafKey); + XFREE(intDer, NULL, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(leafDer, NULL, DYNAMIC_TYPE_TMP_BUFFER); +#endif + return EXPECT_RESULT(); +} + #if defined(OPENSSL_EXTRA) && !defined(NO_RSA) && !defined(NO_FILESYSTEM) static int test_X509_STORE_untrusted_load_cert_to_stack(const char* filename, STACK_OF(X509)* chain) diff --git a/tests/api/test_ossl_x509_str.h b/tests/api/test_ossl_x509_str.h index e1024deadd4..3b13fa13b76 100644 --- a/tests/api/test_ossl_x509_str.h +++ b/tests/api/test_ossl_x509_str.h @@ -30,6 +30,7 @@ int test_wolfSSL_X509_STORE_CTX_get0_store(void); int test_wolfSSL_X509_STORE_CTX(void); int test_wolfSSL_X509_STORE_CTX_ex(void); int test_X509_verify_cert_untrusted_inter(void); +int test_X509_verify_cert_ca_no_keycertsign(void); int test_X509_STORE_untrusted(void); int test_X509_STORE_InvalidCa(void); int test_X509_STORE_InvalidCa_NoCallback(void); @@ -53,6 +54,8 @@ int test_wolfSSL_CTX_set_cert_store(void); TEST_DECL_GROUP("ossl_x509_store", test_wolfSSL_X509_STORE_CTX), \ TEST_DECL_GROUP("ossl_x509_store", test_wolfSSL_X509_STORE_CTX_ex), \ TEST_DECL_GROUP("ossl_x509_store", test_X509_verify_cert_untrusted_inter), \ + TEST_DECL_GROUP("ossl_x509_store", \ + test_X509_verify_cert_ca_no_keycertsign), \ TEST_DECL_GROUP("ossl_x509_store", test_X509_STORE_untrusted), \ TEST_DECL_GROUP("ossl_x509_store", test_X509_STORE_InvalidCa), \ TEST_DECL_GROUP("ossl_x509_store", test_X509_STORE_InvalidCa_NoCallback), \ From c929798460656431bdeeffbf701a74521f403f36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 16 Jun 2026 15:30:49 +0200 Subject: [PATCH 3/7] TLS: validate negotiated certificate type for raw public keys Ensure a peer's certificate form (X.509 vs raw public key) matches the negotiated certificate type, defaulting to X.509 when none was negotiated, on both the client and server. Adds RPK regression tests covering both directions. --- src/internal.c | 40 ++++++++++++------------ tests/api/test_tls13.c | 69 ++++++++++++++++++++++++++++++++++++++++++ tests/api/test_tls13.h | 2 ++ 3 files changed, 90 insertions(+), 21 deletions(-) diff --git a/src/internal.c b/src/internal.c index ba1ff99467f..7cfb9cfb1a3 100644 --- a/src/internal.c +++ b/src/internal.c @@ -15690,31 +15690,29 @@ PRAGMA_GCC_DIAG_POP ret = ParseCertRelative(args->dCert, certType, verify, SSL_CM(ssl), extraSigners); #if defined(HAVE_RPK) - /* if cert type has negotiated with peer, confirm the cert received has - * the same type. - */ - if (ret == 0 ) { - if (ssl->options.side == WOLFSSL_CLIENT_END) { - if (ssl->options.rpkState.received_ServerCertTypeCnt == 1) { + /* Confirm the received certificate's form (X.509 vs raw public key) matches + * the type negotiated with the peer. A raw public key (RFC 7250) has no + * chain, so ParseCertRelative() accepts it without any trust verification; + * it must only be accepted when RPK was negotiated for this peer. When no + * type was negotiated the default is X.509 (RFC 7250/8446), so an + * un-negotiated bare key is rejected. The negotiated type is the received + * server cert type (client) or the selected client cert type (server). */ + if (ret == 0) { + cType = WOLFSSL_CERT_TYPE_X509; + if (ssl->options.side == WOLFSSL_CLIENT_END) { + if (ssl->options.rpkState.received_ServerCertTypeCnt == 1) cType = ssl->options.rpkState.received_ServerCertTypes[0]; - if ((cType == WOLFSSL_CERT_TYPE_RPK && !args->dCert->isRPK) || - (cType == WOLFSSL_CERT_TYPE_X509 && args->dCert->isRPK)) { - /* cert type mismatch */ - WOLFSSL_MSG("unsupported certificate type received"); - ret = UNSUPPORTED_CERTIFICATE; - } - } } else if (ssl->options.side == WOLFSSL_SERVER_END) { - if (ssl->options.rpkState.received_ClientCertTypeCnt == 1) { + if (ssl->options.rpkState.sending_ClientCertTypeCnt == 1) cType = ssl->options.rpkState.sending_ClientCertTypes[0]; - if ((cType == WOLFSSL_CERT_TYPE_RPK && !args->dCert->isRPK) || - (cType == WOLFSSL_CERT_TYPE_X509 && args->dCert->isRPK)) { - /* cert type mismatch */ - WOLFSSL_MSG("unsupported certificate type received"); - ret = UNSUPPORTED_CERTIFICATE; - } - } + } + + if ((cType == WOLFSSL_CERT_TYPE_RPK && !args->dCert->isRPK) || + (cType != WOLFSSL_CERT_TYPE_RPK && args->dCert->isRPK)) { + /* cert type mismatch - includes an un-negotiated raw public key */ + WOLFSSL_MSG("unsupported certificate type received"); + ret = UNSUPPORTED_CERTIFICATE; } } #endif /* HAVE_RPK */ diff --git a/tests/api/test_tls13.c b/tests/api/test_tls13.c index b3ec1ff0d96..d820c55de54 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -2751,6 +2751,75 @@ int test_tls13_rpk_handshake(void) return EXPECT_RESULT(); } +/* Regression: a peer must not present a raw public key (RPK) that was never + * negotiated. Neither side calls set_client/server_cert_type, so RPK is not + * negotiated and the default type is X.509; a received bare key must be + * rejected instead of accepted without any chain verification (auth bypass). + * Covers both directions: server presenting an RPK to the client, and client + * presenting an RPK to the server. */ +int test_tls13_rpk_handshake_no_negotiation(void) +{ + EXPECT_DECLS; +#if defined(HAVE_RPK) && defined(WOLFSSL_TLS13) && \ + !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + struct test_memio_ctx test_ctx; + int tp = 0; + + /* Direction 1: server loads an RPK cert, client is a plain X.509 client. */ + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ( + test_rpk_memio_setup( + &test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_3_client_method, wolfTLSv1_3_server_method, + cliCertFile, CERT_FILETYPE, + svrRpkCertFile, WOLFSSL_FILETYPE_ASN1, + cliKeyFile, CERT_FILETYPE, + svrKeyFile, CERT_FILETYPE) + , 0); + + /* Handshake must fail and the client must not record RPK as negotiated. */ + ExpectIntNE(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + ExpectIntEQ(wolfSSL_get_negotiated_server_cert_type(ssl_c, &tp), + WOLFSSL_SUCCESS); + ExpectIntNE(tp, WOLFSSL_CERT_TYPE_RPK); + + wolfSSL_free(ssl_c); + wolfSSL_CTX_free(ctx_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_s); + ssl_c = ssl_s = NULL; + ctx_c = ctx_s = NULL; + + /* Direction 2: client loads an RPK cert and the server requests client + * auth (VERIFY_PEER, set in the helper). The server expects X.509 and must + * reject the client's un-negotiated bare key. */ + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ( + test_rpk_memio_setup( + &test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_3_client_method, wolfTLSv1_3_server_method, + clntRpkCertFile, WOLFSSL_FILETYPE_ASN1, + svrCertFile, CERT_FILETYPE, + cliKeyFile, CERT_FILETYPE, + svrKeyFile, CERT_FILETYPE) + , 0); + + /* Handshake must fail and the server must not record RPK as negotiated. */ + ExpectIntNE(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + ExpectIntEQ(wolfSSL_get_negotiated_client_cert_type(ssl_s, &tp), + WOLFSSL_SUCCESS); + ExpectIntNE(tp, WOLFSSL_CERT_TYPE_RPK); + + wolfSSL_free(ssl_c); + wolfSSL_CTX_free(ctx_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + #if defined(HAVE_IO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ defined(WOLFSSL_HAVE_MLKEM) && !defined(WOLFSSL_MLKEM_NO_ENCAPSULATE) && \ diff --git a/tests/api/test_tls13.h b/tests/api/test_tls13.h index 984b89372d0..6e392f46d58 100644 --- a/tests/api/test_tls13.h +++ b/tests/api/test_tls13.h @@ -28,6 +28,7 @@ int test_tls13_apis(void); int test_tls13_cipher_suites(void); int test_tls13_bad_psk_binder(void); int test_tls13_rpk_handshake(void); +int test_tls13_rpk_handshake_no_negotiation(void); int test_tls13_pq_groups(void); int test_tls13_multi_pqc_key_share(void); int test_tls13_early_data(void); @@ -88,6 +89,7 @@ int test_tls13_AEAD_limit_KU_aes128_ccm_8_sha256(void); TEST_DECL_GROUP("tls13", test_tls13_cipher_suites), \ TEST_DECL_GROUP("tls13", test_tls13_bad_psk_binder), \ TEST_DECL_GROUP("tls13", test_tls13_rpk_handshake), \ + TEST_DECL_GROUP("tls13", test_tls13_rpk_handshake_no_negotiation), \ TEST_DECL_GROUP("tls13", test_tls13_pq_groups), \ TEST_DECL_GROUP("tls13", test_tls13_multi_pqc_key_share), \ TEST_DECL_GROUP("tls13", test_tls13_early_data), \ From f23544f09476d420becdfabd781eec0dd1a9c597 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 16 Jun 2026 15:59:54 +0200 Subject: [PATCH 4/7] TLS 1.3: fix for post-handshake authentication Only exempt the missing-certificate check during the initial handshake; once a post-handshake CertificateRequest is outstanding the server again requires the client certificate (and its CertificateVerify). Adds a post-handshake auth test. --- src/tls13.c | 19 ++++++++++-- tests/api/test_tls13.c | 68 ++++++++++++++++++++++++++++++++++++++++++ tests/api/test_tls13.h | 2 ++ 3 files changed, 87 insertions(+), 2 deletions(-) diff --git a/src/tls13.c b/src/tls13.c index 0314766310a..ecfd5946dd8 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -11835,7 +11835,10 @@ int DoTls13Finished(WOLFSSL* ssl, const byte* input, word32* inOutIdx, #endif if ( #ifdef WOLFSSL_POST_HANDSHAKE_AUTH - !ssl->options.verifyPostHandshake && + /* Exempt only the initial handshake; a pending post-handshake + * CertificateRequest (certReqCtx != NULL) still requires a peer + * certificate and a valid CertificateVerify. */ + (!ssl->options.verifyPostHandshake || ssl->certReqCtx != NULL) && #endif (!ssl->options.havePeerCert || !ssl->options.havePeerVerify)) { ret = NO_PEER_CERT; /* NO_PEER_VERIFY */ @@ -13507,7 +13510,19 @@ static int SanityCheckTls13MsgReceived(WOLFSSL* ssl, byte type) */ if (ssl->options.verifyPeer && #ifdef WOLFSSL_POST_HANDSHAKE_AUTH - !ssl->options.verifyPostHandshake && + /* The post-handshake-auth exemption is only valid during + * the initial handshake. On the server, once a + * post-handshake CertificateRequest is outstanding + * (certReqCtx != NULL), a Certificate is required again. + * Scoped to the server: certReqCtx means something + * different on the client (a received request) and the + * client does not process an inbound Finished in that + * state. Whether an empty Certificate is then accepted + * follows the verify mode (FAIL_IF_NO_PEER_CERT), exactly + * as for first-handshake client authentication. */ + (!ssl->options.verifyPostHandshake || + (ssl->options.side == WOLFSSL_SERVER_END && + ssl->certReqCtx != NULL)) && #endif !ssl->msgsReceived.got_certificate) { WOLFSSL_MSG("Finished received out of order - " diff --git a/tests/api/test_tls13.c b/tests/api/test_tls13.c index d820c55de54..e4d5c3821dc 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -2820,6 +2820,74 @@ int test_tls13_rpk_handshake_no_negotiation(void) return EXPECT_RESULT(); } +/* Post-handshake authentication (PHA) positive guard. A server configured with + * WOLFSSL_VERIFY_POST_HANDSHAKE must complete the initial handshake WITHOUT a + * client certificate (verification is deferred) and then be able to request one + * post-handshake. This guards the property the auth-bypass fix must preserve: + * the missing-certificate exemption stays in force while no post-handshake + * request is outstanding (certReqCtx == NULL). If the fix wrongly treated the + * initial handshake as a pending PHA request, the handshake below would fail. + * + * Scope: this is a positive guard only. + * - The bypass itself is NOT reproduced here: the + * SanityCheckTls13MsgReceived / DoTls13Finished branches the fix re-enables + * only fire when a client sends a Finished with NO Certificate message at all + * (got_certificate == 0). A conformant wolfSSL client always sends at least + * an empty Certificate, so reproducing the bypass needs a non-conformant + * client; the report used a state-machine model. The negative direction + * rests on code review. + * - Driving the full post-handshake certificate exchange to completion is + * intentionally not asserted: whether the peer chain ends up populated + * depends on build options unrelated to this fix (it differs between + * --enable-all and the user_settings_all header build), so it is not a + * portable assertion. */ +int test_tls13_pha(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ + defined(WOLFSSL_POST_HANDSHAKE_AUTH) && defined(SESSION_CERTS) && \ + !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \ + !defined(NO_RSA) && !defined(NO_CERTS) + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + struct test_memio_ctx test_ctx; + WOLFSSL_X509_CHAIN* chain = NULL; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_3_client_method, wolfTLSv1_3_server_method), 0); + + /* Server: trust the (self-signed) client certificate and defer + * verification to post-handshake. */ + ExpectIntEQ(wolfSSL_CTX_load_verify_locations(ctx_s, cliCertFile, NULL), + WOLFSSL_SUCCESS); + wolfSSL_set_verify(ssl_s, + WOLFSSL_VERIFY_PEER | WOLFSSL_VERIFY_POST_HANDSHAKE, NULL); + + /* Client: load a cert/key and advertise post-handshake auth. */ + ExpectIntEQ(wolfSSL_use_certificate_file(ssl_c, cliCertFile, + WOLFSSL_FILETYPE_PEM), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_use_PrivateKey_file(ssl_c, cliKeyFile, + WOLFSSL_FILETYPE_PEM), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_allow_post_handshake_auth(ssl_c), 0); + + /* Initial handshake must complete even though no client cert was sent - + * the verifyPostHandshake exemption (certReqCtx == NULL) is intact. */ + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + ExpectNotNull(chain = wolfSSL_get_peer_chain(ssl_s)); + ExpectIntEQ(wolfSSL_get_chain_count(chain), 0); + + /* And the server can issue a post-handshake certificate request. */ + ExpectIntEQ(wolfSSL_request_certificate(ssl_s), WOLFSSL_SUCCESS); + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + #if defined(HAVE_IO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ defined(WOLFSSL_HAVE_MLKEM) && !defined(WOLFSSL_MLKEM_NO_ENCAPSULATE) && \ diff --git a/tests/api/test_tls13.h b/tests/api/test_tls13.h index 6e392f46d58..e16c97e1656 100644 --- a/tests/api/test_tls13.h +++ b/tests/api/test_tls13.h @@ -29,6 +29,7 @@ int test_tls13_cipher_suites(void); int test_tls13_bad_psk_binder(void); int test_tls13_rpk_handshake(void); int test_tls13_rpk_handshake_no_negotiation(void); +int test_tls13_pha(void); int test_tls13_pq_groups(void); int test_tls13_multi_pqc_key_share(void); int test_tls13_early_data(void); @@ -90,6 +91,7 @@ int test_tls13_AEAD_limit_KU_aes128_ccm_8_sha256(void); TEST_DECL_GROUP("tls13", test_tls13_bad_psk_binder), \ TEST_DECL_GROUP("tls13", test_tls13_rpk_handshake), \ TEST_DECL_GROUP("tls13", test_tls13_rpk_handshake_no_negotiation), \ + TEST_DECL_GROUP("tls13", test_tls13_pha), \ TEST_DECL_GROUP("tls13", test_tls13_pq_groups), \ TEST_DECL_GROUP("tls13", test_tls13_multi_pqc_key_share), \ TEST_DECL_GROUP("tls13", test_tls13_early_data), \ From 8f55480a1d59e02192e79e8c8aa36f9b9cae61b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 16 Jun 2026 18:06:11 +0200 Subject: [PATCH 5/7] SP math: validate ECDH shared-secret output buffer against the field size Regenerate the SP backends so the ECDH secret generators check the caller's buffer against the number of bytes actually written. Adds a P-384/P-521 buffer-size regression test. --- tests/api/test_ecc.c | 104 ++++++++++++++++++++++++++++++++++++ tests/api/test_ecc.h | 2 + wolfcrypt/src/sp_arm32.c | 6 +-- wolfcrypt/src/sp_arm64.c | 6 +-- wolfcrypt/src/sp_armthumb.c | 6 +-- wolfcrypt/src/sp_c32.c | 6 +-- wolfcrypt/src/sp_c64.c | 6 +-- wolfcrypt/src/sp_cortexm.c | 6 +-- wolfcrypt/src/sp_x86_64.c | 6 +-- 9 files changed, 127 insertions(+), 21 deletions(-) diff --git a/tests/api/test_ecc.c b/tests/api/test_ecc.c index 90d5ced8822..c6f7cb9ecea 100644 --- a/tests/api/test_ecc.c +++ b/tests/api/test_ecc.c @@ -578,6 +578,110 @@ int test_wc_ecc_shared_secret(void) return EXPECT_RESULT(); } /* END tests_wc_ecc_shared_secret */ +#if defined(HAVE_ECC) && defined(HAVE_ECC_DHE) && !defined(WC_NO_RNG) && \ + (defined(HAVE_ECC384) || defined(HAVE_ECC521) || defined(HAVE_ALL_CURVES)) +/* Verify the output-buffer size contract of wc_ecc_shared_secret() at the + * field-size boundary. The single-precision (SP) math secret generators for + * P-384/P-521 historically validated the caller's buffer against the wrong + * length (e.g. P-521 checked 65 but writes 66), so a buffer declared one byte + * short of the field size slipped past the check and was overwritten. Assert + * that fieldSz-1 is rejected with BUFFER_E and fieldSz succeeds, for whichever + * math backend is built. + * + * Coverage note: this drives the blocking generators only (wc_ecc_shared_secret + * is synchronous). The fix also corrected the non-blocking (_nb) variants + * (sp_ecc_secret_gen_384_nb / _521_nb), which need WOLFSSL_SP_NONBLOCK plus the + * specialized SP build and are not exercised here. Of the blocking cases only + * P-521 (65->66) actually fails without the fix; P-384 already used 48, so its + * case is a guard against regression rather than a reproduction. */ +static int ecc_shared_secret_size_bound(WC_RNG* rng, int curveId, int fieldSz) +{ + EXPECT_DECLS; + ecc_key key; + ecc_key pub; + byte out[80]; /* >= P-521 field size (66) */ + word32 outlen; + int keyInit = 0, pubInit = 0; + int ret; + + XMEMSET(&key, 0, sizeof(key)); + XMEMSET(&pub, 0, sizeof(pub)); + + ExpectIntEQ(wc_ecc_init(&key), 0); + if (EXPECT_SUCCESS()) keyInit = 1; + ExpectIntEQ(wc_ecc_init(&pub), 0); + if (EXPECT_SUCCESS()) pubInit = 1; + + ret = wc_ecc_make_key_ex(rng, fieldSz, &key, curveId); +#if defined(WOLFSSL_ASYNC_CRYPT) + ret = wc_AsyncWait(ret, &key.asyncDev, WC_ASYNC_FLAG_NONE); +#endif + ExpectIntEQ(ret, 0); + + ret = wc_ecc_make_key_ex(rng, fieldSz, &pub, curveId); +#if defined(WOLFSSL_ASYNC_CRYPT) + ret = wc_AsyncWait(ret, &pub.asyncDev, WC_ASYNC_FLAG_NONE); +#endif + ExpectIntEQ(ret, 0); + +#if defined(ECC_TIMING_RESISTANT) && (!defined(HAVE_FIPS) || \ + (!defined(HAVE_FIPS_VERSION) || (HAVE_FIPS_VERSION != 2))) && \ + !defined(HAVE_SELFTEST) + ExpectIntEQ(wc_ecc_set_rng(&key, rng), 0); +#endif + + /* One byte short of the field size: must be rejected, not written past. */ + outlen = (word32)(fieldSz - 1); + ExpectIntEQ(wc_ecc_shared_secret(&key, &pub, out, &outlen), + WC_NO_ERR_TRACE(BUFFER_E)); + + /* Exactly the field size: must succeed and report the field size. */ + outlen = (word32)fieldSz; + ExpectIntEQ(wc_ecc_shared_secret(&key, &pub, out, &outlen), 0); + ExpectIntEQ(outlen, (word32)fieldSz); + + if (pubInit) + wc_ecc_free(&pub); + if (keyInit) + wc_ecc_free(&key); + return EXPECT_RESULT(); +} +#endif + +/* + * Testing wc_ecc_shared_secret() output buffer bounds at the field-size edge. + */ +int test_wc_ecc_shared_secret_size_bounds(void) +{ + EXPECT_DECLS; +#if defined(HAVE_ECC) && defined(HAVE_ECC_DHE) && !defined(WC_NO_RNG) && \ + (defined(HAVE_ECC384) || defined(HAVE_ECC521) || defined(HAVE_ALL_CURVES)) + WC_RNG rng; + int rngInit = 0; + + XMEMSET(&rng, 0, sizeof(rng)); + PRIVATE_KEY_UNLOCK(); + ExpectIntEQ(wc_InitRng(&rng), 0); + if (EXPECT_SUCCESS()) + rngInit = 1; + +#if defined(HAVE_ECC384) || defined(HAVE_ALL_CURVES) + ExpectIntEQ(ecc_shared_secret_size_bound(&rng, ECC_SECP384R1, 48), 1); +#endif +#if defined(HAVE_ECC521) || defined(HAVE_ALL_CURVES) + ExpectIntEQ(ecc_shared_secret_size_bound(&rng, ECC_SECP521R1, 66), 1); +#endif + + if (rngInit) + DoExpectIntEQ(wc_FreeRng(&rng), 0); +#ifdef FP_ECC + wc_ecc_fp_free(); +#endif + PRIVATE_KEY_LOCK(); +#endif + return EXPECT_RESULT(); +} + /* * testint wc_ecc_export_x963() */ diff --git a/tests/api/test_ecc.h b/tests/api/test_ecc.h index 95f6f8b93b0..bb3a7f58b03 100644 --- a/tests/api/test_ecc.h +++ b/tests/api/test_ecc.h @@ -36,6 +36,7 @@ int test_wc_ecc_size(void); int test_wc_ecc_params(void); int test_wc_ecc_signVerify_hash(void); int test_wc_ecc_shared_secret(void); +int test_wc_ecc_shared_secret_size_bounds(void); int test_wc_ecc_export_x963(void); int test_wc_ecc_export_x963_ex(void); int test_wc_ecc_import_x963(void); @@ -74,6 +75,7 @@ int test_wc_EccPrivateKeyToDer(void); TEST_DECL_GROUP("ecc", test_wc_ecc_params), \ TEST_DECL_GROUP("ecc", test_wc_ecc_signVerify_hash), \ TEST_DECL_GROUP("ecc", test_wc_ecc_shared_secret), \ + TEST_DECL_GROUP("ecc", test_wc_ecc_shared_secret_size_bounds), \ TEST_DECL_GROUP("ecc", test_wc_ecc_export_x963), \ TEST_DECL_GROUP("ecc", test_wc_ecc_export_x963_ex), \ TEST_DECL_GROUP("ecc", test_wc_ecc_import_x963), \ diff --git a/wolfcrypt/src/sp_arm32.c b/wolfcrypt/src/sp_arm32.c index 39ce504cd07..191fbce29ed 100644 --- a/wolfcrypt/src/sp_arm32.c +++ b/wolfcrypt/src/sp_arm32.c @@ -96378,7 +96378,7 @@ int sp_ecc_secret_gen_384_nb(sp_ecc_ctx_t* sp_ctx, const mp_int* priv, typedef char ctx_size_test[sizeof(sp_ecc_sec_gen_384_ctx) >= sizeof(*sp_ctx) ? -1 : 1]; (void)sizeof(ctx_size_test); - if (*outLen < 32U) { + if (*outLen < 48U) { err = BUFFER_E; } @@ -124117,7 +124117,7 @@ int sp_ecc_secret_gen_521(const mp_int* priv, const ecc_point* pub, byte* out, SP_DECL_VAR(sp_digit, k, 17); int err = MP_OKAY; - if (*outLen < 65U) { + if (*outLen < 66U) { err = BUFFER_E; } @@ -124158,7 +124158,7 @@ int sp_ecc_secret_gen_521_nb(sp_ecc_ctx_t* sp_ctx, const mp_int* priv, typedef char ctx_size_test[sizeof(sp_ecc_sec_gen_521_ctx) >= sizeof(*sp_ctx) ? -1 : 1]; (void)sizeof(ctx_size_test); - if (*outLen < 32U) { + if (*outLen < 66U) { err = BUFFER_E; } diff --git a/wolfcrypt/src/sp_arm64.c b/wolfcrypt/src/sp_arm64.c index 08d041a857d..be0ec3fc732 100644 --- a/wolfcrypt/src/sp_arm64.c +++ b/wolfcrypt/src/sp_arm64.c @@ -65945,7 +65945,7 @@ int sp_ecc_secret_gen_384_nb(sp_ecc_ctx_t* sp_ctx, const mp_int* priv, typedef char ctx_size_test[sizeof(sp_ecc_sec_gen_384_ctx) >= sizeof(*sp_ctx) ? -1 : 1]; (void)sizeof(ctx_size_test); - if (*outLen < 32U) { + if (*outLen < 48U) { err = BUFFER_E; } @@ -110680,7 +110680,7 @@ int sp_ecc_secret_gen_521(const mp_int* priv, const ecc_point* pub, byte* out, SP_DECL_VAR(sp_digit, k, 9); int err = MP_OKAY; - if (*outLen < 65U) { + if (*outLen < 66U) { err = BUFFER_E; } @@ -110721,7 +110721,7 @@ int sp_ecc_secret_gen_521_nb(sp_ecc_ctx_t* sp_ctx, const mp_int* priv, typedef char ctx_size_test[sizeof(sp_ecc_sec_gen_521_ctx) >= sizeof(*sp_ctx) ? -1 : 1]; (void)sizeof(ctx_size_test); - if (*outLen < 32U) { + if (*outLen < 66U) { err = BUFFER_E; } diff --git a/wolfcrypt/src/sp_armthumb.c b/wolfcrypt/src/sp_armthumb.c index dd4ac7d60a6..47bcafd122a 100644 --- a/wolfcrypt/src/sp_armthumb.c +++ b/wolfcrypt/src/sp_armthumb.c @@ -114511,7 +114511,7 @@ int sp_ecc_secret_gen_384_nb(sp_ecc_ctx_t* sp_ctx, const mp_int* priv, typedef char ctx_size_test[sizeof(sp_ecc_sec_gen_384_ctx) >= sizeof(*sp_ctx) ? -1 : 1]; (void)sizeof(ctx_size_test); - if (*outLen < 32U) { + if (*outLen < 48U) { err = BUFFER_E; } @@ -128390,7 +128390,7 @@ int sp_ecc_secret_gen_521(const mp_int* priv, const ecc_point* pub, byte* out, SP_DECL_VAR(sp_digit, k, 17); int err = MP_OKAY; - if (*outLen < 65U) { + if (*outLen < 66U) { err = BUFFER_E; } @@ -128431,7 +128431,7 @@ int sp_ecc_secret_gen_521_nb(sp_ecc_ctx_t* sp_ctx, const mp_int* priv, typedef char ctx_size_test[sizeof(sp_ecc_sec_gen_521_ctx) >= sizeof(*sp_ctx) ? -1 : 1]; (void)sizeof(ctx_size_test); - if (*outLen < 32U) { + if (*outLen < 66U) { err = BUFFER_E; } diff --git a/wolfcrypt/src/sp_c32.c b/wolfcrypt/src/sp_c32.c index 87e4e935545..443c5c2d49e 100644 --- a/wolfcrypt/src/sp_c32.c +++ b/wolfcrypt/src/sp_c32.c @@ -32973,7 +32973,7 @@ int sp_ecc_secret_gen_384_nb(sp_ecc_ctx_t* sp_ctx, const mp_int* priv, typedef char ctx_size_test[sizeof(sp_ecc_sec_gen_384_ctx) >= sizeof(*sp_ctx) ? -1 : 1]; (void)sizeof(ctx_size_test); - if (*outLen < 32U) { + if (*outLen < 48U) { err = BUFFER_E; } @@ -40644,7 +40644,7 @@ int sp_ecc_secret_gen_521(const mp_int* priv, const ecc_point* pub, byte* out, SP_DECL_VAR(sp_digit, k, 21); int err = MP_OKAY; - if (*outLen < 65U) { + if (*outLen < 66U) { err = BUFFER_E; } @@ -40685,7 +40685,7 @@ int sp_ecc_secret_gen_521_nb(sp_ecc_ctx_t* sp_ctx, const mp_int* priv, typedef char ctx_size_test[sizeof(sp_ecc_sec_gen_521_ctx) >= sizeof(*sp_ctx) ? -1 : 1]; (void)sizeof(ctx_size_test); - if (*outLen < 32U) { + if (*outLen < 66U) { err = BUFFER_E; } diff --git a/wolfcrypt/src/sp_c64.c b/wolfcrypt/src/sp_c64.c index 64c64144b6d..fe628261e23 100644 --- a/wolfcrypt/src/sp_c64.c +++ b/wolfcrypt/src/sp_c64.c @@ -33026,7 +33026,7 @@ int sp_ecc_secret_gen_384_nb(sp_ecc_ctx_t* sp_ctx, const mp_int* priv, typedef char ctx_size_test[sizeof(sp_ecc_sec_gen_384_ctx) >= sizeof(*sp_ctx) ? -1 : 1]; (void)sizeof(ctx_size_test); - if (*outLen < 32U) { + if (*outLen < 48U) { err = BUFFER_E; } @@ -40025,7 +40025,7 @@ int sp_ecc_secret_gen_521(const mp_int* priv, const ecc_point* pub, byte* out, SP_DECL_VAR(sp_digit, k, 9); int err = MP_OKAY; - if (*outLen < 65U) { + if (*outLen < 66U) { err = BUFFER_E; } @@ -40066,7 +40066,7 @@ int sp_ecc_secret_gen_521_nb(sp_ecc_ctx_t* sp_ctx, const mp_int* priv, typedef char ctx_size_test[sizeof(sp_ecc_sec_gen_521_ctx) >= sizeof(*sp_ctx) ? -1 : 1]; (void)sizeof(ctx_size_test); - if (*outLen < 32U) { + if (*outLen < 66U) { err = BUFFER_E; } diff --git a/wolfcrypt/src/sp_cortexm.c b/wolfcrypt/src/sp_cortexm.c index 29641a9d702..6755a9d5948 100644 --- a/wolfcrypt/src/sp_cortexm.c +++ b/wolfcrypt/src/sp_cortexm.c @@ -50559,7 +50559,7 @@ int sp_ecc_secret_gen_384_nb(sp_ecc_ctx_t* sp_ctx, const mp_int* priv, typedef char ctx_size_test[sizeof(sp_ecc_sec_gen_384_ctx) >= sizeof(*sp_ctx) ? -1 : 1]; (void)sizeof(ctx_size_test); - if (*outLen < 32U) { + if (*outLen < 48U) { err = BUFFER_E; } @@ -63136,7 +63136,7 @@ int sp_ecc_secret_gen_521(const mp_int* priv, const ecc_point* pub, byte* out, SP_DECL_VAR(sp_digit, k, 17); int err = MP_OKAY; - if (*outLen < 65U) { + if (*outLen < 66U) { err = BUFFER_E; } @@ -63177,7 +63177,7 @@ int sp_ecc_secret_gen_521_nb(sp_ecc_ctx_t* sp_ctx, const mp_int* priv, typedef char ctx_size_test[sizeof(sp_ecc_sec_gen_521_ctx) >= sizeof(*sp_ctx) ? -1 : 1]; (void)sizeof(ctx_size_test); - if (*outLen < 32U) { + if (*outLen < 66U) { err = BUFFER_E; } diff --git a/wolfcrypt/src/sp_x86_64.c b/wolfcrypt/src/sp_x86_64.c index a984ce6a86c..1a3c68fc88c 100644 --- a/wolfcrypt/src/sp_x86_64.c +++ b/wolfcrypt/src/sp_x86_64.c @@ -48834,7 +48834,7 @@ int sp_ecc_secret_gen_384_nb(sp_ecc_ctx_t* sp_ctx, const mp_int* priv, typedef char ctx_size_test[sizeof(sp_ecc_sec_gen_384_ctx) >= sizeof(*sp_ctx) ? -1 : 1]; (void)sizeof(ctx_size_test); - if (*outLen < 32U) { + if (*outLen < 48U) { err = BUFFER_E; } @@ -89503,7 +89503,7 @@ int sp_ecc_secret_gen_521(const mp_int* priv, const ecc_point* pub, byte* out, word32 cpuid_flags = cpuid_get_flags(); #endif - if (*outLen < 65U) { + if (*outLen < 66U) { err = BUFFER_E; } @@ -89552,7 +89552,7 @@ int sp_ecc_secret_gen_521_nb(sp_ecc_ctx_t* sp_ctx, const mp_int* priv, typedef char ctx_size_test[sizeof(sp_ecc_sec_gen_521_ctx) >= sizeof(*sp_ctx) ? -1 : 1]; (void)sizeof(ctx_size_test); - if (*outLen < 32U) { + if (*outLen < 66U) { err = BUFFER_E; } From eaa563419eb20fa5e57888ea4a5fdc09c85790d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 16 Jun 2026 18:38:13 +0200 Subject: [PATCH 6/7] BIO: reject negative length in memory BIO read Reject a negative read length in the memory BIO read path so it cannot bypass the signed bounds checks and reach a wild copy. Adds a regression test. --- src/bio.c | 10 ++++++++++ tests/api/test_ossl_bio.c | 39 +++++++++++++++++++++++++++++++++++++++ tests/api/test_ossl_bio.h | 2 ++ 3 files changed, 51 insertions(+) diff --git a/src/bio.c b/src/bio.c index 13fdef3a5df..b9dfe6b7dd5 100644 --- a/src/bio.c +++ b/src/bio.c @@ -120,6 +120,16 @@ static int wolfSSL_BIO_MEMORY_read(WOLFSSL_BIO* bio, void* buf, int len) WOLFSSL_ENTER("wolfSSL_BIO_MEMORY_read"); } + /* Reject a negative length up front. Callers are expected to validate, but + * guarding here too prevents a negative len from defeating the signed + * bounds checks below (sz/memSz comparisons) and reaching XMEMCPY with a + * length of (size_t)-1. Return the same error the public wolfSSL_BIO_read() + * does for a negative length rather than silently reporting 0 bytes read. A + * zero length is handled correctly by the logic below (copies nothing). */ + if (len < 0) { + return WOLFSSL_BIO_ERROR; + } + sz = wolfSSL_BIO_pending(bio); if (sz > 0) { int memSz; diff --git a/tests/api/test_ossl_bio.c b/tests/api/test_ossl_bio.c index 45c8e94086c..b111fd44681 100644 --- a/tests/api/test_ossl_bio.c +++ b/tests/api/test_ossl_bio.c @@ -980,6 +980,45 @@ int test_wolfSSL_BIO_write(void) return EXPECT_RESULT(); } +/* A negative length must never defeat the memory-BIO read bounds check and + * reach XMEMCPY with (size_t)-1. This exercises the public BIO_read() boundary + * (which rejects a negative length before dispatch); the matching guard in the + * static wolfSSL_BIO_MEMORY_read() sink is defense-in-depth and not separately + * reachable through the public API. Verify a negative length is rejected with + * an error without copying, a zero length reads nothing, and the pending data + * is left intact and still readable. */ +int test_wolfSSL_BIO_read_negative_len(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) + BIO* bio = NULL; + char msg[] = "negative length test"; + int msgLen = (int)XSTRLEN(msg); + char out[64]; + + ExpectNotNull(bio = BIO_new(BIO_s_mem())); + ExpectIntEQ(BIO_write(bio, msg, msgLen), msgLen); + + /* Negative length: must be rejected with an error, not a wild copy and not + * a silent 0-byte read. */ + XMEMSET(out, 0, sizeof(out)); + ExpectIntLT(BIO_read(bio, out, -1), 0); + /* Data must be untouched - still all pending. */ + ExpectIntEQ(BIO_pending(bio), msgLen); + + /* Zero length: nothing read, data still pending. */ + ExpectIntEQ(BIO_read(bio, out, 0), 0); + ExpectIntEQ(BIO_pending(bio), msgLen); + + /* A normal read still returns the intact message. */ + ExpectIntEQ(BIO_read(bio, out, (int)sizeof(out)), msgLen); + ExpectIntEQ(XMEMCMP(out, msg, msgLen), 0); + + BIO_free(bio); +#endif + return EXPECT_RESULT(); +} + int test_wolfSSL_BIO_printf(void) { diff --git a/tests/api/test_ossl_bio.h b/tests/api/test_ossl_bio.h index d401193b147..acf13fb7764 100644 --- a/tests/api/test_ossl_bio.h +++ b/tests/api/test_ossl_bio.h @@ -35,6 +35,7 @@ int test_wolfSSL_BIO_datagram(void); int test_wolfSSL_BIO_s_null(void); int test_wolfSSL_BIO_accept(void); int test_wolfSSL_BIO_write(void); +int test_wolfSSL_BIO_read_negative_len(void); int test_wolfSSL_BIO_printf(void); int test_wolfSSL_BIO_f_md(void); int test_wolfSSL_BIO_up_ref(void); @@ -55,6 +56,7 @@ int test_wolfSSL_BIO_get_init(void); TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_should_retry), \ TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_s_null), \ TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_write), \ + TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_read_negative_len), \ TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_printf), \ TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_f_md), \ TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_up_ref), \ From e6f02ecf4d0669c7f37a9f4b35490600fb1ba2f8 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Tue, 16 Jun 2026 16:09:42 -0600 Subject: [PATCH 7/7] fix for clang-tidy warning on variable not read --- tests/api/test_ossl_x509_str.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/api/test_ossl_x509_str.c b/tests/api/test_ossl_x509_str.c index 2f1af0cc37f..21f4d81d4e3 100644 --- a/tests/api/test_ossl_x509_str.c +++ b/tests/api/test_ossl_x509_str.c @@ -1518,7 +1518,6 @@ static int gen_ca_int_keyusage(byte* out, int outMax, RsaKey* subjKey, RsaKey* caKey, WC_RNG* rng, const char* cn, const char* keyUsage) { Cert cert; - int sz; if (wc_InitCert(&cert) != 0) return -1; @@ -1537,7 +1536,7 @@ static int gen_ca_int_keyusage(byte* out, int outMax, RsaKey* subjKey, if (wc_SetIssuerBuffer(&cert, ca_cert_der_2048, (int)sizeof_ca_cert_der_2048) != 0) return -1; - if ((sz = wc_MakeCert(&cert, out, (word32)outMax, subjKey, NULL, rng)) < 0) + if (wc_MakeCert(&cert, out, (word32)outMax, subjKey, NULL, rng) < 0) return -1; return wc_SignCert(cert.bodySz, cert.sigType, out, (word32)outMax, caKey, NULL, rng); @@ -1549,7 +1548,6 @@ static int gen_leaf_under_int(byte* out, int outMax, RsaKey* leafKey, const char* cn) { Cert cert; - int sz; if (wc_InitCert(&cert) != 0) return -1; @@ -1566,7 +1564,7 @@ static int gen_leaf_under_int(byte* out, int outMax, RsaKey* leafKey, return -1; if (wc_SetIssuerBuffer(&cert, issuerDer, issuerDerSz) != 0) return -1; - if ((sz = wc_MakeCert(&cert, out, (word32)outMax, leafKey, NULL, rng)) < 0) + if (wc_MakeCert(&cert, out, (word32)outMax, leafKey, NULL, rng) < 0) return -1; return wc_SignCert(cert.bodySz, cert.sigType, out, (word32)outMax, issuerKey, NULL, rng);