Skip to content

Commit b8e8923

Browse files
committed
Address review: keep extended_master_secret across a NewSessionTicket
TLSX_Parse() clears options.haveEMS for any non-request message that arrives without the extension. Now that every NewSessionTicket goes through TLSX_Parse(), a TLS 1.3 ticket cleared the flag, and wolfSSL_clear() does not restore it, so a reused object negotiated TLS 1.2 without EMS. NewSessionTicket is post-handshake and never carries the extension, so treat it like hello_retry_request and leave the flag alone.
1 parent a7734a9 commit b8e8923

3 files changed

Lines changed: 65 additions & 1 deletion

File tree

src/tls.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19342,9 +19342,12 @@ WOLFSSL_TEST_VIS int TLSX_Parse(WOLFSSL* ssl, const byte* input, word16 length,
1934219342

1934319343
#ifdef HAVE_EXTENDED_MASTER
1934419344
if (IsAtLeastTLSv1_3(ssl->version) &&
19345-
(msgType == hello_retry_request || msgType == hello_verify_request)) {
19345+
(msgType == hello_retry_request || msgType == hello_verify_request ||
19346+
msgType == session_ticket)) {
1934619347
/* Don't change EMS status until server_hello received.
1934719348
* Second ClientHello must have same extensions.
19349+
* NewSessionTicket is post-handshake and never carries the extension,
19350+
* so its absence there says nothing about what was negotiated.
1934819351
*/
1934919352
}
1935019353
else if (!isRequest && ssl->options.haveEMS && !pendingEMS)

tests/api/test_tls13.c

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10101,6 +10101,65 @@ int test_tls13_new_session_ticket_ext_framing(void)
1010110101
return EXPECT_RESULT();
1010210102
}
1010310103

10104+
/* Parsing a NewSessionTicket must not disturb the extended_master_secret
10105+
* state. TLSX_Parse() clears haveEMS for any non-request message that does
10106+
* not carry the extension, and wolfSSL_clear() never puts it back, so a
10107+
* cleared flag would leave a reused object negotiating TLS 1.2 without EMS. */
10108+
int test_tls13_new_session_ticket_keeps_ems(void)
10109+
{
10110+
EXPECT_DECLS;
10111+
#if defined(WOLFSSL_TLS13) && defined(HAVE_SESSION_TICKET) && \
10112+
defined(HAVE_EXTENDED_MASTER) && \
10113+
defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
10114+
!defined(WOLFSSL_NO_DEF_TICKET_ENC_CB) && \
10115+
!defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER)
10116+
WOLFSSL_CTX* ctx_c = NULL;
10117+
WOLFSSL_CTX* ctx_s = NULL;
10118+
WOLFSSL* ssl_c = NULL;
10119+
WOLFSSL* ssl_s = NULL;
10120+
struct test_memio_ctx test_ctx;
10121+
byte msg[64];
10122+
char buf[64];
10123+
int msgSz;
10124+
10125+
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
10126+
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
10127+
wolfTLSv1_3_client_method, wolfTLSv1_3_server_method), 0);
10128+
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
10129+
10130+
/* Consume the server's own NewSessionTicket. */
10131+
ExpectIntEQ(wolfSSL_read(ssl_c, buf, sizeof(buf)),
10132+
WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR));
10133+
ExpectIntEQ(wolfSSL_get_error(ssl_c, WOLFSSL_FATAL_ERROR),
10134+
WOLFSSL_ERROR_WANT_READ);
10135+
10136+
if (EXPECT_SUCCESS() && ssl_c != NULL)
10137+
ssl_c->options.haveEMS = 1;
10138+
10139+
/* A ticket with an empty extensions vector. */
10140+
msgSz = -1;
10141+
if (EXPECT_SUCCESS())
10142+
msgSz = test_tls13_make_nst(msg, (int)sizeof(msg), NULL, 0);
10143+
ExpectIntGT(msgSz, 0);
10144+
ExpectIntEQ(test_tls13_send_post_hs(&test_ctx, ssl_s, msg, msgSz),
10145+
TEST_SUCCESS);
10146+
10147+
ExpectIntEQ(wolfSSL_read(ssl_c, buf, sizeof(buf)),
10148+
WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR));
10149+
ExpectIntEQ(wolfSSL_get_error(ssl_c, WOLFSSL_FATAL_ERROR),
10150+
WOLFSSL_ERROR_WANT_READ);
10151+
10152+
if (EXPECT_SUCCESS() && ssl_c != NULL)
10153+
ExpectIntEQ(ssl_c->options.haveEMS, 1);
10154+
10155+
wolfSSL_free(ssl_c);
10156+
wolfSSL_free(ssl_s);
10157+
wolfSSL_CTX_free(ctx_c);
10158+
wolfSSL_CTX_free(ctx_s);
10159+
#endif
10160+
return EXPECT_RESULT();
10161+
}
10162+
1010410163
/* RFC 9846 Section 4.3.11: when the modes the client advertised leave no PSK
1010510164
* the server may use, the PSK is ignored and a certificate handshake runs.
1010610165
* Aborting instead would kill a connection that can still be completed. */

tests/api/test_tls13.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ int test_tls13_psk_mode_mismatch_falls_back(void);
123123
int test_tls13_ticket_psk_modes_uses_policy(void);
124124
int test_tls13_send_session_ticket_psk_modes(void);
125125
int test_tls13_new_session_ticket_ext_framing(void);
126+
int test_tls13_new_session_ticket_keeps_ems(void);
126127

127128
#define TEST_TLS13_DECLS \
128129
TEST_DECL_GROUP("tls13", test_tls13_apis), \
@@ -222,6 +223,7 @@ int test_tls13_new_session_ticket_ext_framing(void);
222223
TEST_DECL_GROUP("tls13", test_tls13_ticket_psk_modes), \
223224
TEST_DECL_GROUP("tls13", test_tls13_send_session_ticket_psk_modes), \
224225
TEST_DECL_GROUP("tls13", test_tls13_new_session_ticket_ext_framing), \
226+
TEST_DECL_GROUP("tls13", test_tls13_new_session_ticket_keeps_ems), \
225227
TEST_DECL_GROUP("tls13", test_tls13_psk_mode_mismatch_falls_back), \
226228
TEST_DECL_GROUP("tls13", test_tls13_ticket_psk_modes_uses_policy)
227229

0 commit comments

Comments
 (0)