From af5369636a938faf4a79d1f550d3b206c51fafec Mon Sep 17 00:00:00 2001 From: Eric Blankenhorn Date: Wed, 8 Apr 2026 15:06:11 -0500 Subject: [PATCH 1/4] Fix ETM on resumption --- src/internal.c | 20 +++++++---- tests/api/test_tls.c | 83 ++++++++++++++++++++++++++++++++++++++++++++ tests/api/test_tls.h | 2 ++ 3 files changed, 99 insertions(+), 6 deletions(-) diff --git a/src/internal.c b/src/internal.c index ad1587e0f63..267c75a5d65 100644 --- a/src/internal.c +++ b/src/internal.c @@ -38436,13 +38436,21 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) #if defined(HAVE_TLS_EXTENSIONS) && defined(HAVE_ENCRYPT_THEN_MAC) && \ !defined(WOLFSSL_AEAD_ONLY) - if (ssl->options.encThenMac && ssl->specs.cipher_type == block) { - ret = TLSX_EncryptThenMac_Respond(ssl); - if (ret != 0) - goto out; + /* Only respond to ETM here when resumption actually succeeded; + * HandleTlsResumption populates ssl->specs via SetCipherSpecs only + * on the success path. If resumption failed, resuming has been + * cleared and ssl->specs.cipher_type is still zero-initialized, + * so we must defer the ETM decision until after MatchSuite. */ + if (ssl->options.resuming) { + if (ssl->options.encThenMac && + ssl->specs.cipher_type == block) { + ret = TLSX_EncryptThenMac_Respond(ssl); + if (ret != 0) + goto out; + } + else + ssl->options.encThenMac = 0; } - else - ssl->options.encThenMac = 0; #endif if (ssl->options.clientState == CLIENT_KEYEXCHANGE_COMPLETE) { WOLFSSL_LEAVE("DoClientHello", ret); diff --git a/tests/api/test_tls.c b/tests/api/test_tls.c index 565006171bc..f6cbb6d3899 100644 --- a/tests/api/test_tls.c +++ b/tests/api/test_tls.c @@ -730,6 +730,89 @@ int test_tls12_no_null_compression(void) * uppercase names like "SECP384R1" do not match the lowercase "secp384r1" * entry; they fall through to the wolfCrypt ECC look-up which uses * XSTRCASECMP. */ +/* Regression test for the encrypt-then-MAC silent-disable bug. + * + * Before the fix, when a client sent a 32-byte session ID in its ClientHello + * (so the server set ssl->options.resuming = 1) but the server's session + * cache did not contain that session, DoClientHello would run an + * encrypt_then_mac decision *before* MatchSuite/SetCipherSpecs had populated + * ssl->specs.cipher_type. Because cipher_type was zero-initialized + * (== stream, not block), the ETM block cleared encThenMac to 0, and the + * post-MatchSuite block could not re-enable it. The connection then + * silently negotiated MAC-then-encrypt instead of encrypt-then-MAC. + * + * This test forces a stale-resumption ClientHello against a server with an + * empty session cache, using a CBC-mode cipher suite, and asserts that the + * server still negotiates encrypt-then-MAC. */ +int test_tls12_etm_failed_resumption(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(WOLFSSL_NO_TLS12) && defined(HAVE_ENCRYPT_THEN_MAC) && \ + !defined(WOLFSSL_AEAD_ONLY) && !defined(NO_RSA) && !defined(NO_AES) && \ + defined(HAVE_AES_CBC) && !defined(NO_SHA256) && \ + defined(HAVE_SESSION_TICKET) && defined(HAVE_ECC) + /* TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 — a CBC suite, where ETM applies. */ + const char* cbcSuite = "ECDHE-RSA-AES128-SHA256"; + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + WOLFSSL_SESSION *sess = NULL; + struct test_memio_ctx test_ctx; + + /* First handshake: establish a session-ID-based session on the client. + * Disable TLS 1.2 session tickets on both sides so resumption uses the + * session ID path (not tickets), which is the path the bug lives on. */ + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0); + ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_c), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_s), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_set_cipher_list(ssl_c, cbcSuite), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_set_cipher_list(ssl_s, cbcSuite), WOLFSSL_SUCCESS); + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + /* Sanity: the first handshake itself must use ETM. */ + ExpectIntEQ(ssl_s->options.encThenMac, 1); + ExpectNotNull(sess = wolfSSL_get1_session(ssl_c)); + + wolfSSL_free(ssl_c); ssl_c = NULL; + wolfSSL_free(ssl_s); ssl_s = NULL; + wolfSSL_CTX_free(ctx_c); ctx_c = NULL; + wolfSSL_CTX_free(ctx_s); ctx_s = NULL; + + /* Second handshake against a *fresh* server context (empty cache). The + * client offers the saved session, so the server's ClientHello parser + * sets options.resuming = 1, but HandleTlsResumption then fails to find + * the session and clears resuming. Pre-fix, ETM was silently dropped + * here. */ + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0); + /* The internal session cache is process-global, so the saved session is + * still findable via the cache. Disable lookups on this server SSL + * directly so that HandleTlsResumption hits its "session lookup failed" + * path — exactly the scenario the bug fix targets. */ + ssl_s->options.sessionCacheOff = 1; + ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_c), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_s), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_set_cipher_list(ssl_c, cbcSuite), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_set_cipher_list(ssl_s, cbcSuite), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_set_session(ssl_c, sess), WOLFSSL_SUCCESS); + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + /* The server should NOT have actually resumed (fresh ctx, empty cache). */ + ExpectIntEQ(ssl_s->options.resuming, 0); + /* And — the regression check — encrypt-then-MAC must still be active. */ + ExpectIntEQ(ssl_s->options.encThenMac, 1); + ExpectIntEQ(ssl_c->options.encThenMac, 1); + + wolfSSL_SESSION_free(sess); + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + int test_tls_set_curves_list_ecc_fallback(void) { EXPECT_DECLS; diff --git a/tests/api/test_tls.h b/tests/api/test_tls.h index 46d54043446..9d28a599ac7 100644 --- a/tests/api/test_tls.h +++ b/tests/api/test_tls.h @@ -30,6 +30,7 @@ int test_tls13_curve_intersection(void); int test_tls_certreq_order(void); int test_tls12_bad_cv_sig_alg(void); int test_tls12_no_null_compression(void); +int test_tls12_etm_failed_resumption(void); int test_tls_set_curves_list_ecc_fallback(void); #define TEST_TLS_DECLS \ @@ -41,6 +42,7 @@ int test_tls_set_curves_list_ecc_fallback(void); TEST_DECL_GROUP("tls", test_tls_certreq_order), \ TEST_DECL_GROUP("tls", test_tls12_bad_cv_sig_alg), \ TEST_DECL_GROUP("tls", test_tls12_no_null_compression), \ + TEST_DECL_GROUP("tls", test_tls12_etm_failed_resumption), \ TEST_DECL_GROUP("tls", test_tls_set_curves_list_ecc_fallback) #endif /* TESTS_API_TEST_TLS_H */ From b218b29403e5315af94110c3c512aea13fffc578 Mon Sep 17 00:00:00 2001 From: Eric Blankenhorn Date: Wed, 8 Apr 2026 16:13:23 -0500 Subject: [PATCH 2/4] Fix from review --- tests/api/test_tls.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/api/test_tls.c b/tests/api/test_tls.c index f6cbb6d3899..9e398f24745 100644 --- a/tests/api/test_tls.c +++ b/tests/api/test_tls.c @@ -791,7 +791,8 @@ int test_tls12_etm_failed_resumption(void) * still findable via the cache. Disable lookups on this server SSL * directly so that HandleTlsResumption hits its "session lookup failed" * path — exactly the scenario the bug fix targets. */ - ssl_s->options.sessionCacheOff = 1; + if (ssl_s != NULL) + ssl_s->options.sessionCacheOff = 1; ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_c), WOLFSSL_SUCCESS); ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_s), WOLFSSL_SUCCESS); ExpectIntEQ(wolfSSL_set_cipher_list(ssl_c, cbcSuite), WOLFSSL_SUCCESS); From 191b82757941bc273b5a54168aed22fbb18e0448 Mon Sep 17 00:00:00 2001 From: Eric Blankenhorn Date: Thu, 9 Apr 2026 10:14:45 -0500 Subject: [PATCH 3/4] Fix from review --- tests/api/test_tls.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/api/test_tls.c b/tests/api/test_tls.c index 9e398f24745..4a46c796f45 100644 --- a/tests/api/test_tls.c +++ b/tests/api/test_tls.c @@ -799,11 +799,16 @@ int test_tls12_etm_failed_resumption(void) ExpectIntEQ(wolfSSL_set_cipher_list(ssl_s, cbcSuite), WOLFSSL_SUCCESS); ExpectIntEQ(wolfSSL_set_session(ssl_c, sess), WOLFSSL_SUCCESS); ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); - /* The server should NOT have actually resumed (fresh ctx, empty cache). */ - ExpectIntEQ(ssl_s->options.resuming, 0); - /* And — the regression check — encrypt-then-MAC must still be active. */ - ExpectIntEQ(ssl_s->options.encThenMac, 1); - ExpectIntEQ(ssl_c->options.encThenMac, 1); + if (ssl_s != NULL) { + /* The server should NOT have actually resumed (fresh ctx, empty + * cache). */ + ExpectIntEQ(ssl_s->options.resuming, 0); + /* And - the regression check - encrypt-then-MAC must still be + * active. */ + ExpectIntEQ(ssl_s->options.encThenMac, 1); + } + if (ssl_c != NULL) + ExpectIntEQ(ssl_c->options.encThenMac, 1); wolfSSL_SESSION_free(sess); wolfSSL_free(ssl_c); From 31eb54f95091131a1161b91551f933e21eb3b840 Mon Sep 17 00:00:00 2001 From: Eric Blankenhorn Date: Thu, 9 Apr 2026 11:32:47 -0500 Subject: [PATCH 4/4] Fix from review --- tests/api/test_tls.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/api/test_tls.c b/tests/api/test_tls.c index 4a46c796f45..1637b19c7db 100644 --- a/tests/api/test_tls.c +++ b/tests/api/test_tls.c @@ -752,7 +752,7 @@ int test_tls12_etm_failed_resumption(void) !defined(WOLFSSL_AEAD_ONLY) && !defined(NO_RSA) && !defined(NO_AES) && \ defined(HAVE_AES_CBC) && !defined(NO_SHA256) && \ defined(HAVE_SESSION_TICKET) && defined(HAVE_ECC) - /* TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 — a CBC suite, where ETM applies. */ + /* TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 - a CBC suite, where ETM applies. */ const char* cbcSuite = "ECDHE-RSA-AES128-SHA256"; WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; WOLFSSL *ssl_c = NULL, *ssl_s = NULL; @@ -790,7 +790,7 @@ int test_tls12_etm_failed_resumption(void) /* The internal session cache is process-global, so the saved session is * still findable via the cache. Disable lookups on this server SSL * directly so that HandleTlsResumption hits its "session lookup failed" - * path — exactly the scenario the bug fix targets. */ + * path - exactly the scenario the bug fix targets. */ if (ssl_s != NULL) ssl_s->options.sessionCacheOff = 1; ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_c), WOLFSSL_SUCCESS);