From f57ab60527634c9a47f7e24cd741e1d6a3a9524e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 5 Aug 2026 16:55:14 +0200 Subject: [PATCH 1/3] Send protocol_version alert when no version can be negotiated DoClientHello has four exits that fail with VERSION_ERROR when runtime version restrictions leave nothing acceptable at or below the version the client offered. Three of them sent no alert at all, and the fourth sent one only when WOLFSSL_EXTRA_ALERTS was defined, so a default build simply dropped the connection. The client could not tell a version mismatch from a network failure. The generic fallback did not help. SendFatalAlertOnly is a no-op unless WOLFSSL_EXTRA_ALERTS is defined, and where it is defined it grouped VERSION_ERROR with MATCH_SUITE_ERROR and sent handshake_failure. That also disagreed with the TLS 1.3 mapping, which already resolves VERSION_ERROR to protocol_version. Send a fatal protocol_version alert from all four branches regardless of WOLFSSL_EXTRA_ALERTS, and give VERSION_ERROR its own case in SendFatalAlertOnly so the generic path agrees. Note that this is only observable on the TLS 1.2 message path. A TLS 1.3 capable server routes the ClientHello through DoTls13HandShakeMsgType, which already translates the error into the right alert. Fixes F-7568. --- src/internal.c | 48 ++++++- src/tls13.c | 10 +- tests/api/test_tls.c | 323 +++++++++++++++++++++++++++++++++++++++++++ tests/api/test_tls.h | 8 ++ 4 files changed, 384 insertions(+), 5 deletions(-) diff --git a/src/internal.c b/src/internal.c index 953b71ff17..93eb071bcc 100644 --- a/src/internal.c +++ b/src/internal.c @@ -20523,8 +20523,11 @@ int SendFatalAlertOnly(WOLFSSL *ssl, int error) case WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E): why = bad_record_mac; break; - case WC_NO_ERR_TRACE(MATCH_SUITE_ERROR): case WC_NO_ERR_TRACE(VERSION_ERROR): + why = wolfssl_alert_protocol_version; + break; + /* listed for symmetry with TranslateErrorToAlert(); default covers it */ + case WC_NO_ERR_TRACE(MATCH_SUITE_ERROR): default: why = handshake_failure; break; @@ -40681,6 +40684,7 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) int ret = 0; byte lesserVersion; byte maxMinor; + byte preMaskMinor; WOLFSSL_START(WC_FUNC_CLIENT_HELLO_DO); WOLFSSL_ENTER("DoClientHello"); @@ -40762,9 +40766,19 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) word16 havePSK = 0; int keySz = 0; + /* RFC 5246 7.2.2 and RFC 8446 6.2: a version that cannot be + * negotiated must be refused with a fatal protocol_version alert. + * SendFatalAlertOnly() in ProcessReply is compiled out unless + * WOLFSSL_EXTRA_ALERTS is defined, so alert here. */ if (!ssl->options.downgrade) { WOLFSSL_MSG("Client trying to connect with lesser version"); ret = VERSION_ERROR; + /* propagate socket errors to avoid re-calling send alert */ + if (SendAlert(ssl, alert_fatal, + wolfssl_alert_protocol_version) + == WC_NO_ERR_TRACE(SOCKET_ERROR_E)) { + ret = SOCKET_ERROR_E; + } goto out; } @@ -40778,6 +40792,12 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) if (belowMinDowngrade) { WOLFSSL_MSG("\tversion below minimum allowed, fatal error"); ret = VERSION_ERROR; + /* propagate socket errors to avoid re-calling send alert */ + if (SendAlert(ssl, alert_fatal, + wolfssl_alert_protocol_version) + == WC_NO_ERR_TRACE(SOCKET_ERROR_E)) { + ret = SOCKET_ERROR_E; + } goto out; } @@ -40835,6 +40855,13 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) TRUE, TRUE, TRUE, TRUE, ssl->options.side); } + /* Version the record layer is on before the mask walk below steps it + * down. It is one the client offered (or the server's own maximum + * when the client offered more), so the two alert exits inside that + * block restore it rather than alerting with the masked-off version + * the walk stopped at, which the peer may reject outright. */ + preMaskMinor = ssl->version.minor; + /* check if option is set to not allow the current version * set from either wolfSSL_set_options or wolfSSL_CTX_set_options */ if (!ssl->options.dtls && ssl->options.downgrade && @@ -40874,15 +40901,28 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) WOLFSSL_OP_NO_SSLv3) { WOLFSSL_MSG("\tError, option set to not allow SSLv3"); ret = VERSION_ERROR; -#ifdef WOLFSSL_EXTRA_ALERTS - SendAlert(ssl, alert_fatal, wolfssl_alert_protocol_version); -#endif + /* alert with a version the client will accept */ + ssl->version.minor = preMaskMinor; + /* propagate socket errors to avoid re-calling send alert */ + if (SendAlert(ssl, alert_fatal, + wolfssl_alert_protocol_version) + == WC_NO_ERR_TRACE(SOCKET_ERROR_E)) { + ret = SOCKET_ERROR_E; + } goto out; } if (ssl->version.minor < ssl->options.minDowngrade) { WOLFSSL_MSG("\tversion below minimum allowed, fatal error"); ret = VERSION_ERROR; + /* alert with a version the client will accept */ + ssl->version.minor = preMaskMinor; + /* propagate socket errors to avoid re-calling send alert */ + if (SendAlert(ssl, alert_fatal, + wolfssl_alert_protocol_version) + == WC_NO_ERR_TRACE(SOCKET_ERROR_E)) { + ret = SOCKET_ERROR_E; + } goto out; } diff --git a/src/tls13.c b/src/tls13.c index 83ac9bd39a..d09ab8e58c 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -14893,7 +14893,15 @@ int DoTls13HandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx, alertType = TranslateErrorToAlert(ret); - if (alertType != invalid_alert) { + /* Skip when a fatal alert already went out for this error. The message + * handlers reached above send their own, more specific alert before + * returning (a protocol_version from the version checks in DoClientHello, + * decode_error, illegal_parameter, inappropriate_fallback), and a second + * fatal alert on a connection that is already being torn down is not a + * message the peer can act on. Matches the guard in + * SendFatalAlertOnly(). */ + if (alertType != invalid_alert && + ssl->alert_history.last_tx.level != alert_fatal) { #ifdef WOLFSSL_DTLS13 if (type == client_hello && ssl->options.dtls) DtlsSetSeqNumForReply(ssl); diff --git a/tests/api/test_tls.c b/tests/api/test_tls.c index bd5e0259a0..0dea62dbb9 100644 --- a/tests/api/test_tls.c +++ b/tests/api/test_tls.c @@ -1645,6 +1645,329 @@ int test_tls_fallback_scsv_no_downgrade_runtime_max(void) return EXPECT_RESULT(); } +/* RFC 5246 Section 7.2.2 and RFC 8446 Section 4.1.3: a server that cannot + * negotiate any version at or below ClientHello.client_version MUST abort with + * a fatal protocol_version alert. + * + * The dead end comes from runtime restrictions: a downgrade-capable method + * with a minimum of TLS 1.2 but both TLS 1.3 and TLS 1.2 masked off. Masking + * TLS 1.3 also keeps the ClientHello on the TLS 1.2 message path, since the + * TLS 1.3 handler alerts on its own and would hide the defect. + * + * Built only without WOLFSSL_EXTRA_ALERTS; with it the SendFatalAlertOnly() + * fallback emits the same alert and the test would pass either way. See + * test_tls_version_error_alert_mapping() for that build. + */ +int test_tls_no_acceptable_version_alert(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ + !defined(WOLFSSL_NO_TLS12) && !defined(WOLFSSL_EXTRA_ALERTS) + const byte clientHello[] = { + /* record header: handshake, TLS 1.2, length 45 */ + 0x16, 0x03, 0x03, 0x00, 0x2d, + /* handshake header: ClientHello, length 41 */ + 0x01, 0x00, 0x00, 0x29, + /* client version: TLS 1.2 */ + 0x03, 0x03, + /* random: 32 bytes */ + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, + 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, + 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, + /* session id length: 0 */ + 0x00, + /* cipher suites length: 2, TLS_RSA_WITH_AES_128_CBC_SHA */ + 0x00, 0x02, 0x00, 0x2f, + /* compression methods: 1 entry, null */ + 0x01, 0x00, + }; + WOLFSSL_CTX *ctx_s = NULL; + WOLFSSL *ssl_s = NULL; + struct test_memio_ctx test_ctx; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_inject_message(&test_ctx, 0, + (const char*)clientHello, sizeof(clientHello)), 0); + ExpectIntEQ(test_memio_setup(&test_ctx, NULL, &ctx_s, NULL, &ssl_s, + NULL, wolfSSLv23_server_method), 0); + /* Minimum TLS 1.2, but both TLS 1.3 and TLS 1.2 are disabled, so the + * server has no version left at or below the offered TLS 1.2. */ + ExpectIntEQ(wolfSSL_SetMinVersion(ssl_s, WOLFSSL_TLSV1_2), WOLFSSL_SUCCESS); + if (ssl_s != NULL) { + wolfSSL_set_options(ssl_s, + WOLFSSL_OP_NO_TLSv1_3 | WOLFSSL_OP_NO_TLSv1_2); + ExpectIntNE(wolfSSL_get_options(ssl_s) & WOLFSSL_OP_NO_TLSv1_2, 0); + ExpectIntNE(wolfSSL_get_options(ssl_s) & WOLFSSL_OP_NO_TLSv1_3, 0); + } + ExpectIntEQ(wolfSSL_accept(ssl_s), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_get_error(ssl_s, WOLFSSL_FATAL_ERROR), + WC_NO_ERR_TRACE(VERSION_ERROR)); + /* A fatal protocol_version (70) alert must be on the wire. Check by + * offset rather than comparing the whole record: the version in the alert + * record header follows whatever the downgrade logic last settled on. */ + ExpectIntGE(test_ctx.c_len, 7); + ExpectIntEQ((byte)test_ctx.c_buff[0], 0x15); /* alert content type */ + ExpectIntEQ((byte)test_ctx.c_buff[5], 0x02); /* level: fatal */ + ExpectIntEQ((byte)test_ctx.c_buff[6], 0x46); /* protocol_version (70) */ + + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(WOLFSSL_NO_TLS12) && !defined(WOLFSSL_EXTRA_ALERTS) +/* Drive a raw ClientHello offering client_version TLS 1. at a + * server built from `method`, with `minVersion` as its minimum version (0 for + * the method default), and require a fatal protocol_version alert back. + * + * `noTls13` masks TLS 1.3 off. A TLS 1.3 capable server routes the + * ClientHello to the TLS 1.3 handler, which refuses it there and never gets to + * the version checks in DoClientHello; masking TLS 1.3 keeps the message on + * the TLS 1.2 path, like test_tls_no_acceptable_version_alert does. */ +static int test_tls_lesser_version_alert_case(byte clientMinor, + method_provider method, int minVersion, int noTls13) +{ + EXPECT_DECLS; + byte clientHello[] = { + /* record header: handshake, TLS 1.2, length 45. The record version + * is not what is under test; only client_version below is. */ + 0x16, 0x03, 0x03, 0x00, 0x2d, + /* handshake header: ClientHello, length 41 */ + 0x01, 0x00, 0x00, 0x29, + /* client version: minor patched below */ + 0x03, 0x03, + /* random: 32 bytes */ + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, + 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, + 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, + /* session id length: 0 */ + 0x00, + /* cipher suites length: 2, TLS_RSA_WITH_AES_128_CBC_SHA */ + 0x00, 0x02, 0x00, 0x2f, + /* compression methods: 1 entry, null */ + 0x01, 0x00, + }; + WOLFSSL_CTX *ctx_s = NULL; + WOLFSSL *ssl_s = NULL; + struct test_memio_ctx test_ctx; + + clientHello[10] = clientMinor; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_inject_message(&test_ctx, 0, + (const char*)clientHello, sizeof(clientHello)), 0); + ExpectIntEQ(test_memio_setup(&test_ctx, NULL, &ctx_s, NULL, &ssl_s, + NULL, method), 0); + if (minVersion != 0) { + ExpectIntEQ(wolfSSL_SetMinVersion(ssl_s, minVersion), + WOLFSSL_SUCCESS); + } + if (noTls13 && ssl_s != NULL) { + wolfSSL_set_options(ssl_s, WOLFSSL_OP_NO_TLSv1_3); + ExpectIntNE(wolfSSL_get_options(ssl_s) & WOLFSSL_OP_NO_TLSv1_3, 0); + } + ExpectIntEQ(wolfSSL_accept(ssl_s), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_get_error(ssl_s, WOLFSSL_FATAL_ERROR), + WC_NO_ERR_TRACE(VERSION_ERROR)); + /* A fatal protocol_version (70) alert must be on the wire. Check by + * offset rather than comparing the whole record: the version in the alert + * record header follows whatever the server settled on. */ + ExpectIntGE(test_ctx.c_len, 7); + ExpectIntEQ((byte)test_ctx.c_buff[0], 0x15); /* alert content type */ + ExpectIntEQ((byte)test_ctx.c_buff[5], 0x02); /* level: fatal */ + ExpectIntEQ((byte)test_ctx.c_buff[6], 0x46); /* protocol_version (70) */ + + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_s); + return EXPECT_RESULT(); +} +#endif + +/* DoClientHello refuses a client_version it cannot come down to at four + * points; each must alert with protocol_version (RFC 5246 7.2.2). This covers + * the two that reject the offered version outright - the server does not + * downgrade at all, and the offer is below its minimum - while + * test_tls_no_acceptable_version_alert covers the one where the version mask + * lowers the server below its own minimum. + * + * The remaining one, behind WOLFSSL_OP_NO_SSLv3, is left out on purpose: it is + * only reached once the downgrade logic has settled on SSLv3, which no build + * of the test suite negotiates. + * + * Built only without WOLFSSL_EXTRA_ALERTS, for the reason given on + * test_tls_no_acceptable_version_alert. + */ +int test_tls_lesser_version_alerts(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(WOLFSSL_NO_TLS12) && !defined(WOLFSSL_EXTRA_ALERTS) + /* A method fixed to TLS 1.2 does not downgrade, so a client offering + * TLS 1.1 has to be refused outright. */ + ExpectIntEQ(test_tls_lesser_version_alert_case(TLSv1_1_MINOR, + wolfTLSv1_2_server_method, 0, 0), TEST_SUCCESS); + /* A downgrade-capable server can come down, but not below its configured + * minimum of TLS 1.2. TLS 1.3 is masked off only where it exists, to + * keep the ClientHello on the TLS 1.2 message path. */ +#ifdef WOLFSSL_TLS13 + ExpectIntEQ(test_tls_lesser_version_alert_case(TLSv1_1_MINOR, + wolfSSLv23_server_method, WOLFSSL_TLSV1_2, 1), TEST_SUCCESS); +#else + ExpectIntEQ(test_tls_lesser_version_alert_case(TLSv1_1_MINOR, + wolfSSLv23_server_method, WOLFSSL_TLSV1_2, 0), TEST_SUCCESS); +#endif +#endif + return EXPECT_RESULT(); +} + +/* Two properties of the alert DoClientHello sends when the version mask walks + * the server below its own minimum, both invisible to the tests above: + * + * - The record carries the TLS 1.2 the client offered, not the TLS 1.1 the + * mask walk stepped down to before giving up. A peer is entitled to + * discard a record announcing a version it never proposed, which would + * leave it unable to tell a version mismatch from a dropped connection - + * the whole point of alerting here. + * + * - Exactly one fatal alert goes out. DoClientHello alerts itself, so the + * generic handler it returns through must not append a second one. + * + * The server is TLS 1.3 capable and only TLS 1.2 is masked off, so the TLS 1.3 + * handler hands the TLS 1.2 ClientHello down to DoClientHello - which is also + * what puts a second alert within reach. Masking TLS 1.3 as well, the way the + * tests above do, would keep the message off that path and cover neither. + */ +int test_tls_version_mask_alert_record(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ + !defined(WOLFSSL_NO_TLS12) + const byte clientHello[] = { + /* record header: handshake, TLS 1.2, length 45 */ + 0x16, 0x03, 0x03, 0x00, 0x2d, + /* handshake header: ClientHello, length 41 */ + 0x01, 0x00, 0x00, 0x29, + /* client version: TLS 1.2 */ + 0x03, 0x03, + /* random: 32 bytes */ + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, + 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, + 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, + /* session id length: 0 */ + 0x00, + /* cipher suites length: 2, TLS_RSA_WITH_AES_128_CBC_SHA */ + 0x00, 0x02, 0x00, 0x2f, + /* compression methods: 1 entry, null */ + 0x01, 0x00, + }; + WOLFSSL_CTX *ctx_s = NULL; + WOLFSSL *ssl_s = NULL; + struct test_memio_ctx test_ctx; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_inject_message(&test_ctx, 0, + (const char*)clientHello, sizeof(clientHello)), 0); + ExpectIntEQ(test_memio_setup(&test_ctx, NULL, &ctx_s, NULL, &ssl_s, + NULL, wolfSSLv23_server_method), 0); + /* Only TLS 1.2 is masked off. The server's version is still TLS 1.3 when + * the mask is applied, so nothing is lowered until DoClientHello has + * downgraded to the offered TLS 1.2 - which is what makes the mask walk + * run there and step down to TLS 1.1, below the TLS 1.2 minimum. */ + if (ssl_s != NULL) { + wolfSSL_set_options(ssl_s, WOLFSSL_OP_NO_TLSv1_2); + ExpectIntNE(wolfSSL_get_options(ssl_s) & WOLFSSL_OP_NO_TLSv1_2, 0); + } + ExpectIntEQ(wolfSSL_accept(ssl_s), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_get_error(ssl_s, WOLFSSL_FATAL_ERROR), + WC_NO_ERR_TRACE(VERSION_ERROR)); + /* One fatal alert record and nothing else: 5 byte header plus level and + * description. Nothing is encrypted this early, so a second alert would + * show up as another 7 bytes. */ + ExpectIntEQ(test_ctx.c_len, 7); + ExpectIntEQ((byte)test_ctx.c_buff[0], 0x15); /* alert content type */ + ExpectIntEQ((byte)test_ctx.c_buff[1], 0x03); /* record version: TLS 1.2, */ + ExpectIntEQ((byte)test_ctx.c_buff[2], 0x03); /* not the TLS 1.1 walked to */ + ExpectIntEQ((byte)test_ctx.c_buff[5], 0x02); /* level: fatal */ + ExpectIntEQ((byte)test_ctx.c_buff[6], 0x46); /* protocol_version (70) */ + + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + +/* Companion to test_tls_no_acceptable_version_alert covering the + * SendFatalAlertOnly() fallback (WOLFSSL_EXTRA_ALERTS builds only): a + * VERSION_ERROR must map to protocol_version, not handshake_failure. + * + * Reaching the mapping takes a rejection that does not alert on its own, and + * DTLS is where one is left. The TLS version checks all alert first, and + * SendFatalAlertOnly() bails out once a fatal alert has been sent. + * DoHelloVerifyRequest() does not alert: a HelloVerifyRequest carrying a + * non-DTLS version returns VERSION_ERROR straight up to ProcessReply. + */ +int test_tls_version_error_alert_mapping(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + defined(WOLFSSL_EXTRA_ALERTS) && defined(WOLFSSL_DTLS) && \ + !defined(WOLFSSL_NO_TLS12) + const byte helloVerifyRequest[] = { + /* record header: handshake, DTLS 1.0, epoch 0, sequence 0, length 15. + * A handshake record's version is not checked by GetRecordHeader(). */ + 0x16, 0xfe, 0xff, + 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x0f, + /* handshake header: HelloVerifyRequest, length 3, message_seq 0, + * fragment offset 0, fragment length 3 */ + 0x03, 0x00, 0x00, 0x03, + 0x00, 0x00, + 0x00, 0x00, 0x00, + 0x00, 0x00, 0x03, + /* server version: TLS 1.2, i.e. not a DTLS version at all */ + 0x03, 0x03, + /* cookie length: 0 */ + 0x00, + }; + WOLFSSL_CTX *ctx_c = NULL; + WOLFSSL *ssl_c = NULL; + struct test_memio_ctx test_ctx; + WOLFSSL_ALERT_HISTORY h; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, NULL, &ssl_c, NULL, + wolfDTLSv1_2_client_method, NULL), 0); + + /* First call sends the ClientHello and blocks with nothing to read. */ + ExpectIntEQ(wolfSSL_connect(ssl_c), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_get_error(ssl_c, WOLFSSL_FATAL_ERROR), + WOLFSSL_ERROR_WANT_READ); + + ExpectIntEQ(test_memio_inject_message(&test_ctx, 1, + (const char*)helloVerifyRequest, sizeof(helloVerifyRequest)), 0); + + ExpectIntEQ(wolfSSL_connect(ssl_c), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_get_error(ssl_c, WOLFSSL_FATAL_ERROR), + WC_NO_ERR_TRACE(VERSION_ERROR)); + + /* The mapping is what picks the description here. */ + XMEMSET(&h, 0, sizeof(h)); + ExpectIntEQ(wolfSSL_get_alert_history(ssl_c, &h), WOLFSSL_SUCCESS); + ExpectIntEQ(h.last_tx.level, alert_fatal); + ExpectIntEQ(h.last_tx.code, 70); /* protocol_version */ + + wolfSSL_free(ssl_c); + wolfSSL_CTX_free(ctx_c); +#endif + return EXPECT_RESULT(); +} + /* Test that set_curves_list correctly resolves ECC curve names that fall * through the kNistCurves table and reach the wc_ecc_get_curve_idx_from_name * fallback path. The kNistCurves lookup uses a case-sensitive XSTRNCMP, so diff --git a/tests/api/test_tls.h b/tests/api/test_tls.h index ab79c07890..6c3fda8996 100644 --- a/tests/api/test_tls.h +++ b/tests/api/test_tls.h @@ -42,6 +42,10 @@ int test_dtls_fallback_scsv(void); int test_dtls_fallback_scsv_no_downgrade(void); int test_tls_fallback_scsv_no_downgrade(void); int test_tls_fallback_scsv_no_downgrade_runtime_max(void); +int test_tls_no_acceptable_version_alert(void); +int test_tls_lesser_version_alerts(void); +int test_tls_version_mask_alert_record(void); +int test_tls_version_error_alert_mapping(void); int test_tls12_etm_failed_resumption(void); int test_tls12_resume_ticket_wrong_suite(void); int test_tls12_resume_ticket_decline_fallback(void); @@ -85,6 +89,10 @@ int test_wolfSSL_get_shared_ciphers(void); TEST_DECL_GROUP("tls", test_dtls_fallback_scsv_no_downgrade), \ TEST_DECL_GROUP("tls", test_tls_fallback_scsv_no_downgrade), \ TEST_DECL_GROUP("tls", test_tls_fallback_scsv_no_downgrade_runtime_max),\ + TEST_DECL_GROUP("tls", test_tls_no_acceptable_version_alert), \ + TEST_DECL_GROUP("tls", test_tls_lesser_version_alerts), \ + TEST_DECL_GROUP("tls", test_tls_version_mask_alert_record), \ + TEST_DECL_GROUP("tls", test_tls_version_error_alert_mapping), \ TEST_DECL_GROUP("tls", test_tls12_etm_failed_resumption), \ TEST_DECL_GROUP("tls", test_tls12_resume_ticket_wrong_suite), \ TEST_DECL_GROUP("tls", test_tls12_resume_ticket_decline_fallback), \ From 1614d2e5287ab1a10f496e681e1765f6e5480312 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 5 Aug 2026 17:05:22 +0200 Subject: [PATCH 2/3] Fix double free in wolfSSL_X509_EXTENSION_set_data The function freed the extension's dynamically allocated ASN.1 string buffer but left value.data and value.isDynamic pointing at it. The subsequent wolfSSL_ASN1_STRING_copy() call snapshots those fields before copying and frees the old buffer once the copy is complete, so the stale pointer was freed a second time. Any second call to wolfSSL_X509_EXTENSION_set_data() on an extension holding a value of at least CTC_NAME_SIZE bytes hit this, and passing the extension its own value made the copy read freed memory as well. wolfSSL_ASN1_STRING_set() already performs an alias safe replacement and disposes of the previous buffer itself, so drop the manual free. Add a regression test that replaces a dynamically allocated extension value and then sets the value from itself. Fixes F-7340. --- src/x509.c | 12 ++++----- tests/api/test_ossl_x509_ext.c | 46 ++++++++++++++++++++++++++++++++++ tests/api/test_ossl_x509_ext.h | 2 ++ 3 files changed, 53 insertions(+), 7 deletions(-) diff --git a/src/x509.c b/src/x509.c index 2c702989ae..555d402a58 100644 --- a/src/x509.c +++ b/src/x509.c @@ -3776,16 +3776,14 @@ WOLFSSL_ASN1_STRING* wolfSSL_X509_EXTENSION_get_data( int wolfSSL_X509_EXTENSION_set_data(WOLFSSL_X509_EXTENSION* ext, WOLFSSL_ASN1_STRING* data) { - WOLFSSL_ASN1_STRING* current; - if (ext == NULL || data == NULL) return WOLFSSL_FAILURE; - current = wolfSSL_X509_EXTENSION_get_data_internal(ext); - if (current->length > 0 && current->data != NULL && current->isDynamic) { - XFREE(current->data, NULL, DYNAMIC_TYPE_OPENSSL); - } - + /* wolfSSL_ASN1_STRING_copy() defers to wolfSSL_ASN1_STRING_set(), which + * owns the free of any existing dynamic buffer and only releases it once + * the new contents have been copied. Freeing it here as well would leave + * ext->value.data dangling for that copy, so leave the buffer alone; the + * self-aliased case (data == &ext->value) keeps working too. */ return wolfSSL_ASN1_STRING_copy(&ext->value, data); } diff --git a/tests/api/test_ossl_x509_ext.c b/tests/api/test_ossl_x509_ext.c index 18849b4fd6..616f798280 100644 --- a/tests/api/test_ossl_x509_ext.c +++ b/tests/api/test_ossl_x509_ext.c @@ -850,6 +850,52 @@ int test_wolfSSL_X509_EXTENSION_get_data(void) return EXPECT_RESULT(); } +int test_wolfSSL_X509_EXTENSION_set_data(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) || defined(OPENSSL_ALL) + WOLFSSL_X509_EXTENSION* ext = NULL; + WOLFSSL_ASN1_STRING* str = NULL; +#ifndef WOLFSSL_OLD_EXTDATA_FMT + WOLFSSL_ASN1_STRING* cur = NULL; +#endif + /* Long enough that the ASN.1 STRING data is dynamically allocated. */ + byte longData[CTC_NAME_SIZE * 2]; + + XMEMSET(longData, 'A', sizeof(longData)); + + ExpectNotNull(ext = wolfSSL_X509_EXTENSION_new()); + ExpectNotNull(str = wolfSSL_ASN1_STRING_new()); + ExpectIntEQ(wolfSSL_ASN1_STRING_set(str, longData, (int)sizeof(longData)), + 1); + + ExpectIntEQ(wolfSSL_X509_EXTENSION_set_data(NULL, NULL), + WC_NO_ERR_TRACE(WOLFSSL_FAILURE)); + ExpectIntEQ(wolfSSL_X509_EXTENSION_set_data(ext, NULL), + WC_NO_ERR_TRACE(WOLFSSL_FAILURE)); + ExpectIntEQ(wolfSSL_X509_EXTENSION_set_data(NULL, str), + WC_NO_ERR_TRACE(WOLFSSL_FAILURE)); + + /* Replace a dynamically allocated value with another one. */ + ExpectIntEQ(wolfSSL_X509_EXTENSION_set_data(ext, str), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_X509_EXTENSION_set_data(ext, str), WOLFSSL_SUCCESS); + +#ifndef WOLFSSL_OLD_EXTDATA_FMT + /* Set the value from itself. */ + ExpectNotNull(cur = wolfSSL_X509_EXTENSION_get_data(ext)); + ExpectIntEQ(wolfSSL_X509_EXTENSION_set_data(ext, cur), WOLFSSL_SUCCESS); + + ExpectNotNull(cur = wolfSSL_X509_EXTENSION_get_data(ext)); + ExpectIntEQ(cur->length, (int)sizeof(longData)); + ExpectBufEQ(cur->data, longData, sizeof(longData)); +#endif + + wolfSSL_ASN1_STRING_free(str); + wolfSSL_X509_EXTENSION_free(ext); +#endif + return EXPECT_RESULT(); +} + int test_wolfSSL_X509_EXTENSION_get_critical(void) { EXPECT_DECLS; diff --git a/tests/api/test_ossl_x509_ext.h b/tests/api/test_ossl_x509_ext.h index 13ab76484c..8b25a606c8 100644 --- a/tests/api/test_ossl_x509_ext.h +++ b/tests/api/test_ossl_x509_ext.h @@ -37,6 +37,7 @@ int test_wolfSSL_X509_EXTENSION_new(void); int test_wolfSSL_X509_EXTENSION_dup(void); int test_wolfSSL_X509_EXTENSION_get_object(void); int test_wolfSSL_X509_EXTENSION_get_data(void); +int test_wolfSSL_X509_EXTENSION_set_data(void); int test_wolfSSL_X509_EXTENSION_get_critical(void); int test_wolfSSL_X509_EXTENSION_create_by_OBJ(void); int test_wolfSSL_X509V3_set_ctx(void); @@ -73,6 +74,7 @@ int test_wolfSSL_NAME_CONSTRAINTS_excluded(void); TEST_DECL_GROUP("ossl_x509_ext", test_wolfSSL_X509_EXTENSION_dup), \ TEST_DECL_GROUP("ossl_x509_ext", test_wolfSSL_X509_EXTENSION_get_object), \ TEST_DECL_GROUP("ossl_x509_ext", test_wolfSSL_X509_EXTENSION_get_data), \ + TEST_DECL_GROUP("ossl_x509_ext", test_wolfSSL_X509_EXTENSION_set_data), \ TEST_DECL_GROUP("ossl_x509_ext", \ test_wolfSSL_X509_EXTENSION_get_critical), \ TEST_DECL_GROUP("ossl_x509_ext", \ From a3973044f64e2e8fee931d436336a9df083d3230 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 5 Aug 2026 17:21:02 +0200 Subject: [PATCH 3/3] Honor per-context X509_STORE_CTX verify callback wolfSSL_X509_STORE_CTX_set_verify_cb stored the application callback in ctx->verify_cb, but every verification site read ctx->store->verify_cb instead, so the field was never consulted. An application installing a restrictive callback on the store context, which is the OpenSSL documented way to enforce extra policy during verification, had it silently ignored, and wolfSSL_X509_verify_cert could report success on a chain the callback would have rejected. Add X509StoreGetVerifyCb, which prefers the context callback and falls back to the store one, and use it at all four call sites in X509StoreVerifyCert, X509StoreCheckPathLen and wolfSSL_X509_verify_cert. The store fallback keeps its OPENSSL_ALL or WOLFSSL_QT guard because the store field only exists there, while the call sites now follow the OPENSSL_EXTRA guard of the setter. Clear ctx->verify_cb in wolfSSL_X509_STORE_CTX_init along with the other per-verification state so a reused context does not carry a stale callback. A rejection also has to be reportable. When the certificate manager accepts a chain, ctx->error is X509_V_OK, so a callback that rejects it without recording an error of its own left wolfSSL_X509_verify_cert returning failure while X509_STORE_CTX_get_error still said the chain was fine. Record WOLFSSL_X509_V_ERR_UNSPECIFIED in that case, matching what OpenSSL reports, and only when the callback set no error itself. Add that value to the X509 error enum, where the openssl compatibility header already had the define. Feeding the rejection marker to SetupStoreCtxError is not an option there, since GetX509Error has no X509_V_ error for it and would pass the negative value through as the reported error. The OPENSSL_ALL date recheck did exactly that after a rejection, so skip that block once the callback has rejected, which also stops it from consulting the callback a second time. Add a regression test that verifies a good chain twice, once bare and once with a rejecting context callback, requires the second attempt to fail, and checks the reported error both when the callback records one and when it does not. Fixes F-7341. --- src/internal.c | 3 + src/x509_str.c | 145 ++++++++++++--- tests/api/test_ossl_x509_str.c | 315 +++++++++++++++++++++++++++++++-- tests/api/test_ossl_x509_str.h | 8 + wolfssl/ssl.h | 1 + 5 files changed, 439 insertions(+), 33 deletions(-) diff --git a/src/internal.c b/src/internal.c index 93eb071bcc..bf6f2a06cf 100644 --- a/src/internal.c +++ b/src/internal.c @@ -29386,6 +29386,9 @@ static const char* wolfSSL_ERR_reason_error_string_OpenSSL(unsigned long e) /* TODO: -WOLFSSL_X509_V_ERR_CERT_SIGNATURE_FAILURE. Conflicts with * -WOLFSSL_ERROR_WANT_CONNECT. */ + case WOLFSSL_X509_V_ERR_UNSPECIFIED: + return "unspecified certificate verification error"; + case WOLFSSL_X509_V_ERR_CRL_HAS_EXPIRED: return "CRL has expired"; diff --git a/src/x509_str.c b/src/x509_str.c index bfe27150e9..e9ee28e45b 100644 --- a/src/x509_str.c +++ b/src/x509_str.c @@ -218,6 +218,7 @@ int wolfSSL_X509_STORE_CTX_init(WOLFSSL_X509_STORE_CTX* ctx, XMEMSET(&ctx->ex_data, 0, sizeof(ctx->ex_data)); #endif ctx->userCtx = NULL; + ctx->verify_cb = NULL; ctx->error = 0; ctx->error_depth = 0; ctx->discardSessionCerts = 0; @@ -323,6 +324,10 @@ int GetX509Error(int e) return WOLFSSL_X509_V_ERR_CERT_REVOKED; case WC_NO_ERR_TRACE(CRL_MISSING): return WOLFSSL_X509_V_ERR_UNABLE_TO_GET_CRL; + /* is an internal wolfSSL return code, not an X509_V_* code, so 1 + * here is WOLFSSL_SUCCESS - it does not collide with + * WOLFSSL_X509_V_ERR_UNSPECIFIED, which shares the value but never + * reaches this function. */ case 0: case 1: return 0; @@ -508,11 +513,51 @@ static int X509StoreCheckCtxCrls(WOLFSSL_X509_STORE_CTX* ctx) } #endif /* HAVE_CRL */ -static int X509StoreVerifyCert(WOLFSSL_X509_STORE_CTX* ctx) +/* Get the verification callback that applies to this context, or NULL when + * none is installed. A callback set with wolfSSL_X509_STORE_CTX_set_verify_cb + * takes precedence over one set on the store, matching OpenSSL. + * + * The context callback is settable in every OPENSSL_EXTRA build, so the + * pathLen and INVALID_CA overrides driven from here are now reachable there + * too, not just under OPENSSL_ALL or WOLFSSL_QT. */ +static WOLFSSL_X509_STORE_CTX_verify_cb X509StoreGetVerifyCb( + WOLFSSL_X509_STORE_CTX* ctx) +{ + if (ctx == NULL) + return NULL; + + if (ctx->verify_cb != NULL) + return ctx->verify_cb; + +#if defined(OPENSSL_ALL) || defined(WOLFSSL_QT) + if (ctx->store != NULL) + return ctx->store->verify_cb; +#endif + + return NULL; +} + +/* Verify ctx->current_cert against the store. + * + * is an out-parameter, never NULL. It is set to 1 when the + * application's per-context verify callback explicitly rejected a certificate + * the verification itself accepted, and to 0 otherwise. The rejection is + * reported out of band rather than as a return value so that it cannot be + * confused with any of the internal error codes this function passes through. + * + * A caller must stop chain building when it is set: a veto must not be turned + * into a retry with another issuer, and must not be cleared by the + * partial-chain fallback in wolfSSL_X509_verify_cert(). */ +static int X509StoreVerifyCert(WOLFSSL_X509_STORE_CTX* ctx, int* cbRejected) { int ret = WC_NO_ERR_TRACE(WOLFSSL_FAILURE); + WOLFSSL_X509_STORE_CTX_verify_cb verifyCb; WOLFSSL_ENTER("X509StoreVerifyCert"); + *cbRejected = 0; + + verifyCb = X509StoreGetVerifyCb(ctx); + if (ctx->current_cert != NULL && ctx->current_cert->derCert != NULL) { ret = wolfSSL_CertManagerVerifyBuffer(ctx->store->cm, ctx->current_cert->derCert->buffer, @@ -540,14 +585,40 @@ static int X509StoreVerifyCert(WOLFSSL_X509_STORE_CTX* ctx) } #endif SetupStoreCtxError(ctx, ret); - #if defined(OPENSSL_ALL) || defined(WOLFSSL_QT) - if (ctx->store->verify_cb) - ret = ctx->store->verify_cb(ret >= 0 ? 1 : 0, ctx) == 1 ? - WOLFSSL_SUCCESS : ret; - #endif + if (verifyCb != NULL) { + /* Snapshot the error so the rejection below can tell one the + * callback recorded itself from one left over from an earlier + * certificate in the chain - SetupStoreCtxError() preserves the + * worst error seen so far, so ctx->error is not necessarily + * WOLFSSL_X509_V_OK on entry even when this certificate + * verified. */ + int preCbError = ctx->error; + + if (verifyCb(ret >= 0 ? 1 : 0, ctx) == 1) { + ret = WOLFSSL_SUCCESS; + } + else if (ret >= 0 && ctx->verify_cb != NULL) { + /* Returning 0 must reject a chain the cert manager accepted. + * Only for the per-context callback - a store callback has + * never been able to reject here, and widening it is a + * separate behavior change. */ + if (ctx->error == preCbError) { + /* Keep an error the callback recorded itself; otherwise + * the rejection has no error to report. */ + wolfSSL_X509_STORE_CTX_set_error(ctx, + WOLFSSL_X509_V_ERR_UNSPECIFIED); + } + *cbRejected = 1; + ret = WOLFSSL_FAILURE; + } + } } #if !defined(NO_ASN_TIME) && defined(OPENSSL_ALL) - if (ret != WC_NO_ERR_TRACE(ASN_BEFORE_DATE_E) && + /* Skipped once the callback has rejected: the decision is already made, + * and re-running the date check would consult the callback a second time, + * which could overturn the rejection. */ + if (*cbRejected == 0 && + ret != WC_NO_ERR_TRACE(ASN_BEFORE_DATE_E) && ret != WC_NO_ERR_TRACE(ASN_AFTER_DATE_E)) { /* With OpenSSL, we need to check the certificate's date * after certificate manager verification, @@ -556,8 +627,8 @@ static int X509StoreVerifyCert(WOLFSSL_X509_STORE_CTX* ctx) ret = X509StoreVerifyCertDate(ctx, ret); SetupStoreCtxError(ctx, ret); ret = ret == WOLFSSL_SUCCESS ? 1 : 0; - if (ctx->store->verify_cb) { - if (ctx->store->verify_cb(ret, ctx) == 1) { + if (verifyCb != NULL) { + if (verifyCb(ret, ctx) == 1) { ret = WOLFSSL_SUCCESS; } else { @@ -780,10 +851,13 @@ static int X509StoreCheckPathLen(WOLFSSL_X509_STORE_CTX* ctx) word32 maxPathLen = 0; byte haveConstraint = 0; WOLFSSL_X509* anchor; + WOLFSSL_X509_STORE_CTX_verify_cb verifyCb; if (ctx == NULL || ctx->chain == NULL) return WOLFSSL_SUCCESS; + verifyCb = X509StoreGetVerifyCb(ctx); + num = wolfSSL_sk_X509_num(ctx->chain); /* A pathLen violation requires at least one intermediate between the leaf * (index 0) and the trust anchor, i.e. a chain of three or more. */ @@ -823,16 +897,13 @@ static int X509StoreCheckPathLen(WOLFSSL_X509_STORE_CTX* ctx) if (maxPathLen == 0) { SetupStoreCtxError_ex(ctx, WOLFSSL_X509_V_ERR_PATH_LENGTH_EXCEEDED, i); - #if defined(OPENSSL_ALL) || defined(WOLFSSL_QT) /* Allow an application verify callback to override, matching * the INVALID_CA handling in wolfSSL_X509_verify_cert(). */ - if (ctx->store != NULL && ctx->store->verify_cb != NULL && - ctx->store->verify_cb(0, ctx) == 1) { + if (verifyCb != NULL && verifyCb(0, ctx) == 1) { /* Overridden: keep walking without decrementing (budget is * already exhausted). */ continue; } - #endif return WOLFSSL_FAILURE; } maxPathLen--; @@ -863,12 +934,16 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx) int numFailedCerts = 0; int depth = 0; int origDepth = 0; + int cbRejected = 0; WOLFSSL_X509 *issuer = NULL; WOLFSSL_X509 *orig = NULL; WOLF_STACK_OF(WOLFSSL_X509)* certs = NULL; WOLF_STACK_OF(WOLFSSL_X509)* certsToUse = NULL; WOLF_STACK_OF(WOLFSSL_X509)* failedCerts = NULL; WOLF_STACK_OF(WOLFSSL_X509)* origTrustedSk = NULL; +#ifndef WOLFSSL_X509_STORE_ALLOW_NON_CA_INTERMEDIATE + WOLFSSL_X509_STORE_CTX_verify_cb verifyCb; +#endif WOLFSSL_ENTER("wolfSSL_X509_verify_cert"); if (ctx == NULL || ctx->store == NULL || ctx->store->cm == NULL @@ -876,6 +951,10 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx) return WOLFSSL_FATAL_ERROR; } +#ifndef WOLFSSL_X509_STORE_ALLOW_NON_CA_INTERMEDIATE + verifyCb = X509StoreGetVerifyCb(ctx); +#endif + certs = ctx->store->certs; if (ctx->setTrustedSk != NULL) { @@ -975,17 +1054,14 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx) /* error depth is current depth + 1 */ SetupStoreCtxError_ex(ctx, WOLFSSL_X509_V_ERR_INVALID_CA, (ctx->chain) ? (int)(ctx->chain->num + 1) : 1); - #if defined(OPENSSL_ALL) || defined(WOLFSSL_QT) - if (ctx->store->verify_cb) { - ret = ctx->store->verify_cb(0, ctx); + if (verifyCb != NULL) { + ret = verifyCb(0, ctx); if (ret != WOLFSSL_SUCCESS) { ret = WOLFSSL_FAILURE; goto exit; } } - else - #endif - { + else { ret = WOLFSSL_FAILURE; goto exit; } @@ -998,7 +1074,14 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx) continue; } added = 1; - ret = X509StoreVerifyCert(ctx); + ret = X509StoreVerifyCert(ctx, &cbRejected); + if (cbRejected) { + /* The application vetoed this certificate. Stop instead of + * looking for another issuer: the decision is the + * application's and retrying would only ask it again. */ + ret = WOLFSSL_FAILURE; + goto exit; + } if (ret != WOLFSSL_SUCCESS) { if ((origDepth - depth) <= 1) added = 0; @@ -1030,7 +1113,14 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx) goto exit; } added = 0; - ret = X509StoreVerifyCert(ctx); + ret = X509StoreVerifyCert(ctx, &cbRejected); + if (cbRejected) { + /* An application veto is final. The partial-chain fallback + * below must not accept the chain here and clear ctx->error: + * the certificate verified, the application rejected it. */ + ret = WOLFSSL_FAILURE; + goto exit; + } if (ret != WOLFSSL_SUCCESS) { /* WOLFSSL_PARTIAL_CHAIN may only terminate the chain at a * certificate the caller actually trusts. The previous @@ -1195,6 +1285,19 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx) } } + /* Fail closed on the way out: every failure has to be reportable through + * X509_STORE_CTX_get_error(), or the application is told the chain was + * fine while this function reports failure. Not all of them record one - + * a verify callback that rejects a chain the verification accepted, an + * allocation failure, or a chain-building error that never reached + * SetupStoreCtxError() all leave ctx->error at X509_V_OK. This is a + * deliberate blanket fallback for that whole class; it is applied only + * when nothing more specific was recorded, so an error the callback or + * the verification did set survives untouched. */ + if (ret != WOLFSSL_SUCCESS && ctx->error == WOLFSSL_X509_V_OK) { + ctx->error = WOLFSSL_X509_V_ERR_UNSPECIFIED; + } + return ret == WOLFSSL_SUCCESS ? WOLFSSL_SUCCESS : WOLFSSL_FAILURE; } diff --git a/tests/api/test_ossl_x509_str.c b/tests/api/test_ossl_x509_str.c index 2257fa5ed3..b6bb0291ea 100644 --- a/tests/api/test_ossl_x509_str.c +++ b/tests/api/test_ossl_x509_str.c @@ -909,6 +909,79 @@ static int test_wolfSSL_X509_STORE_CTX_ex_partial_chain_untrusted_terminal( return EXPECT_RESULT(); } +/* Certificate the callback below vetoes, and whether it ever got to. */ +static X509* partialChainRejectCert; +static int partialChainRejectSeen; + +static int partial_chain_reject_cb(int ok, X509_STORE_CTX* store_ctx) +{ + if (ok && X509_STORE_CTX_get_current_cert(store_ctx) == + partialChainRejectCert) { + partialChainRejectSeen = 1; + /* Reject a certificate the verification itself accepted. */ + return 0; + } + return ok; +} + +static int test_wolfSSL_X509_STORE_CTX_ex_partial_chain_cb_reject( + X509_STORE_test_data *testData) +{ + EXPECT_DECLS; + X509_STORE* store = NULL; + X509_STORE_CTX* ctx = NULL; + STACK_OF(X509)* trusted = NULL; + + /* A per-context verify callback that rejects a certificate must not be + * overruled by the partial-chain terminus. The store trusts the root, so + * the intermediate the chain ends at verifies successfully - only the + * callback rejects it. The callback's veto is what makes + * X509StoreVerifyCert fail, and the WOLFSSL_PARTIAL_CHAIN fallback would + * then accept the chain at that same (caller-trusted) certificate and + * clear ctx->error, reporting X509_V_OK for a chain the application + * refused. */ + ExpectNotNull(store = X509_STORE_new()); + ExpectIntEQ(X509_STORE_add_cert(store, testData->x509Ca), 1); + + /* Trust the two intermediates through the ctx override instead of the + * store, so the chain runs out of issuers at x509CaInt while the root + * that signed it is still a trusted CA in the CertManager. That is what + * makes the last X509StoreVerifyCert() succeed, leaving the callback as + * the only reason for the failure. */ + ExpectNotNull(trusted = sk_X509_new_null()); + ExpectIntGT(sk_X509_push(trusted, testData->x509CaInt2), 0); + ExpectIntGT(sk_X509_push(trusted, testData->x509CaInt), 0); + + ExpectNotNull(ctx = X509_STORE_CTX_new()); + ExpectIntEQ(X509_STORE_CTX_init(ctx, store, testData->x509Leaf, NULL), 1); + X509_STORE_CTX_trusted_stack(ctx, trusted); + X509_STORE_CTX_set_flags(ctx, X509_V_FLAG_PARTIAL_CHAIN); + /* After init, which clears any callback already on the context. */ + partialChainRejectCert = testData->x509CaInt; + partialChainRejectSeen = 0; + X509_STORE_CTX_set_verify_cb(ctx, partial_chain_reject_cb); + + /* Sanity check that the setup reached the veto at all - without it the + * verification result below would prove nothing. */ + ExpectIntNE(X509_verify_cert(ctx), 1); + ExpectIntEQ(partialChainRejectSeen, 1); + /* The rejection has to be reportable, not X509_V_OK. */ + ExpectIntEQ(X509_STORE_CTX_get_error(ctx), X509_V_ERR_UNSPECIFIED); +#ifndef NO_ERROR_STRINGS + /* And it has to render as a verification error. Without a reason string + * of its own, X509_V_ERR_UNSPECIFIED (1) falls through to the negated + * lookup and comes back out as the unrelated TLS "fatal error". */ + ExpectStrEQ(X509_verify_cert_error_string(X509_STORE_CTX_get_error(ctx)), + "unspecified certificate verification error"); +#endif + + partialChainRejectCert = NULL; + X509_STORE_CTX_free(ctx); + X509_STORE_free(store); + sk_X509_free(trusted); + return EXPECT_RESULT(); +} + #ifdef HAVE_ECC static int test_wolfSSL_X509_STORE_CTX_ex12(void) { @@ -1071,7 +1144,7 @@ int test_wolfSSL_X509_verify_cert_pathlen_ok(void) return EXPECT_RESULT(); } -#if defined(OPENSSL_ALL) && !defined(NO_CERTS) && \ +#if defined(OPENSSL_EXTRA) && !defined(NO_CERTS) && \ !defined(NO_FILESYSTEM) && !defined(NO_RSA) /* Records whether the pathLen violation was surfaced to the verify callback, * then overrides it (returns 1) so verification continues - exercising the @@ -1084,18 +1157,13 @@ static int pathlen_override_cb(int ok, X509_STORE_CTX *ctx) pathlen_override_seen = 1; return 1; /* override: accept despite the error */ } -#endif -/* A verify callback that returns 1 must be able to override the pathLen - * violation, matching the INVALID_CA override handling in - * wolfSSL_X509_verify_cert(). Reuses the rejecting chainF: with the override - * callback installed the same chain must now verify, and the callback must have - * observed X509_V_ERR_PATH_LENGTH_EXCEEDED. */ -int test_wolfSSL_X509_verify_cert_pathlen_override(void) +/* Drives the rejecting chainF with pathlen_override_cb installed on the store + * context (onCtx) or on the store itself. Either way the callback must reach + * the override branch in X509StoreCheckPathLen() and the chain must verify. */ +static int test_pathlen_override_with_cb(int onCtx) { EXPECT_DECLS; -#if defined(OPENSSL_ALL) && !defined(NO_CERTS) && \ - !defined(NO_FILESYSTEM) && !defined(NO_RSA) X509* root = NULL; X509* ica2 = NULL; X509* ica1 = NULL; @@ -1117,13 +1185,21 @@ int test_wolfSSL_X509_verify_cert_pathlen_override(void) ExpectNotNull(store = X509_STORE_new()); ExpectIntEQ(X509_STORE_add_cert(store, root), 1); - X509_STORE_set_verify_cb(store, pathlen_override_cb); + if (!onCtx) { + #if defined(OPENSSL_ALL) || defined(WOLFSSL_QT) + X509_STORE_set_verify_cb(store, pathlen_override_cb); + #endif + } ExpectNotNull(inter = sk_X509_new_null()); ExpectIntGT(sk_X509_push(inter, ica2), 0); ExpectIntGT(sk_X509_push(inter, ica1), 0); ExpectNotNull(ctx = X509_STORE_CTX_new()); ExpectIntEQ(X509_STORE_CTX_init(ctx, store, leaf, inter), 1); + if (onCtx) { + /* After init, which clears any callback already on the context. */ + X509_STORE_CTX_set_verify_cb(ctx, pathlen_override_cb); + } /* The callback overrides the violation, so verification now succeeds... */ ExpectIntEQ(X509_verify_cert(ctx), 1); /* ...and the callback must actually have seen the pathLen error. */ @@ -1136,10 +1212,163 @@ int test_wolfSSL_X509_verify_cert_pathlen_override(void) X509_free(ica2); X509_free(ica1); X509_free(leaf); + return EXPECT_RESULT(); +} +#endif + +/* A verify callback that returns 1 must be able to override the pathLen + * violation, matching the INVALID_CA override handling in + * wolfSSL_X509_verify_cert(). Reuses the rejecting chainF: with the override + * callback installed the same chain must now verify, and the callback must have + * observed X509_V_ERR_PATH_LENGTH_EXCEEDED. */ +int test_wolfSSL_X509_verify_cert_pathlen_override(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_ALL) && !defined(NO_CERTS) && \ + !defined(NO_FILESYSTEM) && !defined(NO_RSA) + ExpectIntEQ(test_pathlen_override_with_cb(0), TEST_SUCCESS); #endif /* OPENSSL_ALL && !NO_CERTS && !NO_FILESYSTEM && !NO_RSA */ return EXPECT_RESULT(); } +/* Same override, but through the per-context callback, which every + * OPENSSL_EXTRA build can install - the store callback needs OPENSSL_ALL or + * WOLFSSL_QT. Without this the pathLen override branch goes untested in a + * plain --enable-opensslextra build. */ +int test_wolfSSL_X509_verify_cert_pathlen_override_ctx_cb(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) && !defined(NO_CERTS) && \ + !defined(NO_FILESYSTEM) && !defined(NO_RSA) + ExpectIntEQ(test_pathlen_override_with_cb(1), TEST_SUCCESS); +#endif /* OPENSSL_EXTRA && !NO_CERTS && !NO_FILESYSTEM && !NO_RSA */ + return EXPECT_RESULT(); +} + +#if defined(OPENSSL_EXTRA) && !defined(NO_CERTS) && !defined(NO_FILESYSTEM) && \ + !defined(NO_RSA) && !defined(NO_ASN_TIME) +/* Rejects whatever it is handed and records that it ran. Installed on the + * store ctx, to prove the per-context callback is consulted. */ +static int ctx_reject_seen = 0; +static int ctx_reject_cb(int ok, X509_STORE_CTX *ctx) +{ + (void)ok; + (void)ctx; + ctx_reject_seen = 1; + return 0; /* reject */ +} + +/* Rejects, but records an error of its own first, the way an application + * enforcing extra policy does. That error must survive. */ +static int ctx_reject_seterr_cb(int ok, X509_STORE_CTX *ctx) +{ + (void)ok; + X509_STORE_CTX_set_error(ctx, X509_V_ERR_APPLICATION_VERIFICATION); + return 0; /* reject */ +} + +#if defined(OPENSSL_ALL) || defined(WOLFSSL_QT) +/* Accepts whatever it is handed. Installed on the store, so the rejecting + * per-context callback has something to take precedence over. */ +static int store_accept_seen = 0; +static int store_accept_cb(int ok, X509_STORE_CTX *ctx) +{ + (void)ok; + (void)ctx; + store_accept_seen = 1; + return 1; /* accept */ +} +#endif +#endif + +/* A callback installed with X509_STORE_CTX_set_verify_cb must be honored by + * X509_verify_cert(). The chain verifies cleanly on its own, so a rejecting + * callback is the only thing that can fail it. Also pins that the + * per-context callback wins over the store's, and that + * X509_STORE_CTX_init() clears it. */ +int test_wolfSSL_X509_STORE_CTX_verify_cb(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) && !defined(NO_CERTS) && !defined(NO_FILESYSTEM) && \ + !defined(NO_RSA) && !defined(NO_ASN_TIME) + X509* ca = NULL; + X509* leaf = NULL; + X509_STORE* store = NULL; + X509_STORE_CTX* ctx = NULL; + + ctx_reject_seen = 0; + + ExpectNotNull(ca = test_wolfSSL_X509_STORE_CTX_ex_helper( + "./certs/ca-cert.pem")); + ExpectNotNull(leaf = test_wolfSSL_X509_STORE_CTX_ex_helper( + "./certs/server-cert.pem")); + ExpectNotNull(store = X509_STORE_new()); + ExpectIntEQ(X509_STORE_add_cert(store, ca), 1); + + /* Sanity check: the chain verifies with no callback installed. */ + ExpectNotNull(ctx = X509_STORE_CTX_new()); + ExpectIntEQ(X509_STORE_CTX_init(ctx, store, leaf, NULL), 1); + ExpectIntEQ(X509_verify_cert(ctx), 1); + X509_STORE_CTX_free(ctx); + ctx = NULL; + + /* Same chain, but now a per-context callback rejects it. */ + ExpectNotNull(ctx = X509_STORE_CTX_new()); + ExpectIntEQ(X509_STORE_CTX_init(ctx, store, leaf, NULL), 1); + X509_STORE_CTX_set_verify_cb(ctx, ctx_reject_cb); + ExpectIntNE(X509_verify_cert(ctx), 1); + ExpectIntEQ(ctx_reject_seen, 1); + /* A rejected verification must be reportable through get_error(). The + * callback set no error of its own, so the generic one stands in, as in + * OpenSSL. Leaving X509_V_OK here would tell the application the chain + * was fine. */ + ExpectIntEQ(X509_STORE_CTX_get_error(ctx), X509_V_ERR_UNSPECIFIED); + + /* Re-initializing the same context must drop that callback again. */ + ExpectIntEQ(X509_STORE_CTX_init(ctx, store, leaf, NULL), 1); + ctx_reject_seen = 0; + ExpectIntEQ(X509_verify_cert(ctx), 1); + ExpectIntEQ(ctx_reject_seen, 0); + ExpectIntEQ(X509_STORE_CTX_get_error(ctx), X509_V_OK); + + X509_STORE_CTX_free(ctx); + ctx = NULL; + + /* An error the callback records itself must not be replaced. */ + ExpectNotNull(ctx = X509_STORE_CTX_new()); + ExpectIntEQ(X509_STORE_CTX_init(ctx, store, leaf, NULL), 1); + X509_STORE_CTX_set_verify_cb(ctx, ctx_reject_seterr_cb); + ExpectIntNE(X509_verify_cert(ctx), 1); + ExpectIntEQ(X509_STORE_CTX_get_error(ctx), + X509_V_ERR_APPLICATION_VERIFICATION); + + X509_STORE_CTX_free(ctx); + ctx = NULL; + +#if defined(OPENSSL_ALL) || defined(WOLFSSL_QT) + /* The per-context callback takes precedence over the store's, so the + * rejecting one still decides even though the store accepts. */ + X509_STORE_set_verify_cb(store, store_accept_cb); + store_accept_seen = 0; + ctx_reject_seen = 0; + + ExpectNotNull(ctx = X509_STORE_CTX_new()); + ExpectIntEQ(X509_STORE_CTX_init(ctx, store, leaf, NULL), 1); + X509_STORE_CTX_set_verify_cb(ctx, ctx_reject_cb); + ExpectIntNE(X509_verify_cert(ctx), 1); + ExpectIntEQ(ctx_reject_seen, 1); + ExpectIntEQ(store_accept_seen, 0); +#endif + + X509_STORE_CTX_free(ctx); + X509_STORE_free(store); + X509_free(leaf); + X509_free(ca); +#endif /* OPENSSL_EXTRA && !NO_CERTS && !NO_FILESYSTEM && !NO_RSA && + * !NO_ASN_TIME */ + return EXPECT_RESULT(); +} + /* The trust anchor's own pathLenConstraint must bound the path (matching * OpenSSL's -partial_chain behavior and wolfSSL's native ParseCertRelative). * Trust chainF-ICA2 (pathlen:0) directly as a partial-chain anchor and verify @@ -1233,6 +1462,8 @@ int test_wolfSSL_X509_STORE_CTX_ex(void) ExpectIntEQ( test_wolfSSL_X509_STORE_CTX_ex_partial_chain_untrusted_terminal( &testData), 1); + ExpectIntEQ( + test_wolfSSL_X509_STORE_CTX_ex_partial_chain_cb_reject(&testData), 1); #ifdef HAVE_ECC ExpectIntEQ(test_wolfSSL_X509_STORE_CTX_ex12(), 1); #endif @@ -2041,7 +2272,7 @@ int test_X509_STORE_untrusted(void) return EXPECT_RESULT(); } -#if defined(OPENSSL_ALL) && !defined(NO_RSA) && !defined(NO_FILESYSTEM) && \ +#if defined(OPENSSL_EXTRA) && !defined(NO_RSA) && !defined(NO_FILESYSTEM) && \ !defined(WOLFSSL_X509_STORE_ALLOW_NON_CA_INTERMEDIATE) static int last_errcode; @@ -2117,6 +2348,66 @@ int test_X509_STORE_InvalidCa(void) return EXPECT_RESULT(); } +/* Same override as test_X509_STORE_InvalidCa, but through the per-context + * callback. It is settable in every OPENSSL_EXTRA build, so the INVALID_CA + * override in wolfSSL_X509_verify_cert() is reachable there too - the store + * callback the test above uses needs OPENSSL_ALL or WOLFSSL_QT. */ +int test_X509_STORE_InvalidCa_CtxCallback(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) && !defined(NO_RSA) && !defined(NO_FILESYSTEM) && \ + !defined(WOLFSSL_X509_STORE_ALLOW_NON_CA_INTERMEDIATE) + const char* filename = "./certs/intermediate/ca_false_intermediate/" + "test_int_not_cacert.pem"; + const char* srvfile = "./certs/intermediate/ca_false_intermediate/" + "test_sign_bynoca_srv.pem"; + X509_STORE_CTX* ctx = NULL; + X509_STORE* str = NULL; + XFILE fp = XBADFILE; + X509* cert = NULL; + STACK_OF(X509)* untrusted = NULL; + + last_errcode = 0; + last_errdepth = 0; + + ExpectTrue((fp = XFOPEN(srvfile, "rb")) + != XBADFILE); + ExpectNotNull(cert = PEM_read_X509(fp, 0, 0, 0 )); + if (fp != XBADFILE) { + XFCLOSE(fp); + fp = XBADFILE; + } + + ExpectNotNull(str = X509_STORE_new()); + ExpectNotNull(ctx = X509_STORE_CTX_new()); + ExpectNotNull(untrusted = sk_X509_new_null()); + + /* Create cert chain stack with an intermediate that is CA:FALSE. */ + ExpectIntEQ(test_X509_STORE_untrusted_load_cert_to_stack(filename, + untrusted), TEST_SUCCESS); + + ExpectIntEQ(X509_STORE_load_locations(str, + "./certs/intermediate/ca_false_intermediate/test_ca.pem", + NULL), 1); + + ExpectIntEQ(X509_STORE_CTX_init(ctx, str, cert, untrusted), 1); + /* After init, which clears any callback already on the context. */ + X509_STORE_CTX_set_verify_cb(ctx, X509Callback); + /* The callback overrides the CA:FALSE issuer, so verification succeeds... */ + ExpectIntEQ(X509_verify_cert(ctx), 1); + /* ...and it must actually have been handed the INVALID_CA error. */ + ExpectIntEQ(last_errcode, X509_V_ERR_INVALID_CA); + (void)last_errdepth; + ExpectIntEQ(X509_STORE_CTX_get_error(ctx), X509_V_ERR_INVALID_CA); + + X509_free(cert); + X509_STORE_free(str); + X509_STORE_CTX_free(ctx); + sk_X509_pop_free(untrusted, NULL); +#endif + return EXPECT_RESULT(); +} + int test_X509_STORE_InvalidCa_NoCallback(void) { EXPECT_DECLS; diff --git a/tests/api/test_ossl_x509_str.h b/tests/api/test_ossl_x509_str.h index d320133b62..62846ce47d 100644 --- a/tests/api/test_ossl_x509_str.h +++ b/tests/api/test_ossl_x509_str.h @@ -32,11 +32,14 @@ int test_wolfSSL_X509_STORE_CTX_ex(void); int test_wolfSSL_X509_verify_cert_pathlen(void); int test_wolfSSL_X509_verify_cert_pathlen_ok(void); int test_wolfSSL_X509_verify_cert_pathlen_override(void); +int test_wolfSSL_X509_verify_cert_pathlen_override_ctx_cb(void); int test_wolfSSL_X509_verify_cert_pathlen_anchor(void); +int test_wolfSSL_X509_STORE_CTX_verify_cb(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_CtxCallback(void); int test_X509_STORE_InvalidCa_NoCallback(void); int test_wolfSSL_X509_STORE_CTX_trusted_stack_cleanup(void); int test_wolfSSL_X509_STORE_CTX_get_issuer(void); @@ -65,13 +68,18 @@ int test_wolfSSL_CTX_set_cert_store(void); test_wolfSSL_X509_verify_cert_pathlen_ok), \ TEST_DECL_GROUP("ossl_x509_store", \ test_wolfSSL_X509_verify_cert_pathlen_override), \ + TEST_DECL_GROUP("ossl_x509_store", \ + test_wolfSSL_X509_verify_cert_pathlen_override_ctx_cb), \ TEST_DECL_GROUP("ossl_x509_store", \ test_wolfSSL_X509_verify_cert_pathlen_anchor), \ + TEST_DECL_GROUP("ossl_x509_store", \ + test_wolfSSL_X509_STORE_CTX_verify_cb), \ 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_CtxCallback), \ TEST_DECL_GROUP("ossl_x509_store", test_X509_STORE_InvalidCa_NoCallback), \ TEST_DECL_GROUP("ossl_x509_store", \ test_wolfSSL_X509_STORE_CTX_trusted_stack_cleanup), \ diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index 87b330d94c..96f6a8ef59 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -2798,6 +2798,7 @@ WOLFSSL_API long wolfSSL_get_default_read_buffer_len(const WOLFSSL* ssl); */ enum { WOLFSSL_X509_V_OK = 0, + WOLFSSL_X509_V_ERR_UNSPECIFIED = 1, WOLFSSL_X509_V_ERR_UNABLE_TO_GET_CRL = 3, WOLFSSL_X509_V_ERR_CERT_SIGNATURE_FAILURE = 7, WOLFSSL_X509_V_ERR_CERT_NOT_YET_VALID = 9,