From a7d5b0db095849f759e46bbaf13cdeedecf4626d Mon Sep 17 00:00:00 2001 From: Kareem Date: Fri, 14 Aug 2026 16:40:43 -0700 Subject: [PATCH] EMS code review feedback: Disable peerAuthGood when rejecting ticket due to EMS being disabled Ensure that if EMS is disabled, sessions with EMS are not used Add client resumption test and extend server resumption test to optionally use session tickets --- src/internal.c | 2 + src/ssl_sess.c | 7 ++- tests/api.c | 1 + tests/api/test_tls_ext.c | 105 +++++++++++++++++++++++++++++++++++++-- tests/api/test_tls_ext.h | 1 + 5 files changed, 112 insertions(+), 4 deletions(-) diff --git a/src/internal.c b/src/internal.c index abe2b6e965..0ec2f1d412 100644 --- a/src/internal.c +++ b/src/internal.c @@ -41416,6 +41416,8 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) WOLFSSL_MSG("EMS disabled locally, declining resumption " "of an EMS session. Do full handshake."); ssl->options.resuming = 0; + /* A declined ticket must not satisfy client auth. */ + ssl->options.peerAuthGood = 0; } else #endif diff --git a/src/ssl_sess.c b/src/ssl_sess.c index a32a5bb23f..14d23f670d 100644 --- a/src/ssl_sess.c +++ b/src/ssl_sess.c @@ -1614,8 +1614,13 @@ int wolfSSL_SetSession(WOLFSSL* ssl, WOLFSSL_SESSION* session) /* A user EMS override takes precedence over the session's EMS state. */ if (ssl->options.requireEMS && ssl->options.side == WOLFSSL_CLIENT_END) ssl->options.haveEMS = 1; - else if (ssl->options.disableEMS) + else if (ssl->options.disableEMS) { ssl->options.haveEMS = 0; + /* An EMS session cannot be offered without the extension + * (RFC 7627 5.3): decline it and do a full handshake. */ + if (ssl->session->haveEMS) + ssl->options.resuming = 0; + } else #endif { diff --git a/tests/api.c b/tests/api.c index b72c11b8a3..b38ce275f9 100644 --- a/tests/api.c +++ b/tests/api.c @@ -42140,6 +42140,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_tls_ems_resumption_server_downgrade), TEST_DECL(test_tls_ems_server_disable), TEST_DECL(test_tls_ems_server_disable_resumption), + TEST_DECL(test_tls_ems_client_disable_resumption), TEST_DECL(test_tls_ems_disable_v23), TEST_DECL(test_tls_require_ems), TEST_DECL(test_tls_require_ems_resumption), diff --git a/tests/api/test_tls_ext.c b/tests/api/test_tls_ext.c index c61d3710ef..36a96e05ba 100644 --- a/tests/api/test_tls_ext.c +++ b/tests/api/test_tls_ext.c @@ -352,9 +352,106 @@ int test_tls_ems_server_disable(void) } +#if !defined(WOLFSSL_NO_TLS12) && defined(HAVE_EXTENDED_MASTER) && \ + !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \ + defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(NO_SESSION_CACHE) /* A server that disables EMS declines resumption of a session that used EMS - * from an EMS-offering client: full handshake instead of a fatal alert. */ + * from an EMS-offering client: full handshake instead of a fatal alert. + * useTicket selects session-ticket resumption instead of session-ID + * resumption. */ +static int test_tls_ems_server_disable_resumption_ex(int useTicket) +{ + EXPECT_DECLS; + struct test_memio_ctx test_ctx; + WOLFSSL_CTX *ctx_c = NULL; + WOLFSSL_CTX *ctx_s = NULL; + WOLFSSL *ssl_c = NULL; + WOLFSSL *ssl_s = NULL; + WOLFSSL_SESSION *session = NULL; + +#ifndef HAVE_SESSION_TICKET + (void)useTicket; +#endif + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + + /* Establish a session that uses EMS. */ + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0); +#ifdef HAVE_SESSION_TICKET + if (useTicket) + ExpectIntEQ(wolfSSL_UseSessionTicket(ssl_c), WOLFSSL_SUCCESS); +#endif + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + ExpectNotNull(session = wolfSSL_get1_session(ssl_c)); + ExpectTrue(session->haveEMS); +#ifdef HAVE_SESSION_TICKET + if (useTicket) + ExpectIntGT(session->ticketLen, 0); +#endif + + wolfSSL_free(ssl_c); + ssl_c = NULL; + wolfSSL_free(ssl_s); + ssl_s = NULL; + test_memio_clear_buffer(&test_ctx, 0); + test_memio_clear_buffer(&test_ctx, 1); + + 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_DisableExtendedMasterSecret(ssl_s), WOLFSSL_SUCCESS); + /* Verify the peer so the fallback's client auth cannot be satisfied by + * anything but the full handshake itself. */ + wolfSSL_set_verify(ssl_s, WOLFSSL_VERIFY_PEER, NULL); + ExpectIntEQ(wolfSSL_set_session(ssl_c, session), WOLFSSL_SUCCESS); + + /* ClientHello */ + ExpectIntEQ(wolfSSL_connect(ssl_c), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ); + /* Server flight declining the resumption: the declined session or ticket + * must not count as peer auth for the full handshake. */ + ExpectIntEQ(wolfSSL_accept(ssl_s), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ); + ExpectIntEQ(ssl_s->options.resuming, 0); + ExpectIntEQ(ssl_s->options.peerAuthGood, 0); + + /* The handshake completes as a full handshake without EMS. */ + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + ExpectIntEQ(ssl_c->options.haveEMS, 0); + ExpectIntEQ(ssl_s->options.haveEMS, 0); + + wolfSSL_SESSION_free(session); + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); + return EXPECT_RESULT(); +} +#endif + +/* Server-side EMS disable declines EMS-session resumption gracefully, on + * both session-ID and session-ticket resumption. */ int test_tls_ems_server_disable_resumption(void) +{ + EXPECT_DECLS; +#if !defined(WOLFSSL_NO_TLS12) && defined(HAVE_EXTENDED_MASTER) && \ + !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \ + defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(NO_SESSION_CACHE) + ExpectIntEQ(test_tls_ems_server_disable_resumption_ex(0), TEST_SUCCESS); +#if defined(HAVE_SESSION_TICKET) && !defined(WOLFSSL_NO_DEF_TICKET_ENC_CB) + ExpectIntEQ(test_tls_ems_server_disable_resumption_ex(1), TEST_SUCCESS); +#endif +#endif + return EXPECT_RESULT(); +} + + +/* A client that disables EMS must not offer an EMS-bound session: the session + * is declined at wolfSSL_set_session and a full handshake is done instead of + * an offer the server is required to reject (RFC 7627 5.3). */ +int test_tls_ems_client_disable_resumption(void) { EXPECT_DECLS; #if !defined(WOLFSSL_NO_TLS12) && defined(HAVE_EXTENDED_MASTER) && \ @@ -386,10 +483,12 @@ int test_tls_ems_server_disable_resumption(void) 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_DisableExtendedMasterSecret(ssl_s), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_DisableExtendedMasterSecret(ssl_c), WOLFSSL_SUCCESS); ExpectIntEQ(wolfSSL_set_session(ssl_c, session), WOLFSSL_SUCCESS); + /* The EMS session is declined rather than offered without EMS. */ + ExpectIntEQ(ssl_c->options.resuming, 0); - /* The handshake must complete as a full handshake without EMS. */ + /* The handshake completes as a full handshake without EMS. */ ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); ExpectIntEQ(ssl_s->options.resuming, 0); ExpectIntEQ(ssl_c->options.haveEMS, 0); diff --git a/tests/api/test_tls_ext.h b/tests/api/test_tls_ext.h index 061f489b2d..f1a6815bff 100644 --- a/tests/api/test_tls_ext.h +++ b/tests/api/test_tls_ext.h @@ -27,6 +27,7 @@ int test_tls_ems_resumption_downgrade(void); int test_tls_ems_resumption_server_downgrade(void); int test_tls_ems_server_disable(void); int test_tls_ems_server_disable_resumption(void); +int test_tls_ems_client_disable_resumption(void); int test_tls_ems_disable_v23(void); int test_tls_require_ems(void); int test_tls_require_ems_resumption(void);