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..1637b19c7db 100644 --- a/tests/api/test_tls.c +++ b/tests/api/test_tls.c @@ -730,6 +730,95 @@ 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. */ + 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); + 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); + 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); + 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 */