Skip to content

DTLS 1.3: don't echo legacy_session_id in ServerHello #10007

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
julek-wolfssl wants to merge 5 commits into
base: master
Choose a base branch
from
+227 −20
Open

DTLS 1.3: don't echo legacy_session_id in ServerHello #10007

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6c972ad
DTLS 1.3: don't echo legacy_session_id in ServerHello
julek-wolfssl Mar 18, 2026
7d4865c
Address review
julek-wolfssl Mar 19, 2026
bf9686e
Add a test that does a session resumption with session ID with downgrade
julek-wolfssl Mar 19, 2026
6f6af91
fixup! DTLS 1.3: don't echo legacy_session_id in ServerHello
julek-wolfssl Mar 20, 2026
9d11f91
fixup! DTLS 1.3: don't echo legacy_session_id in ServerHello
julek-wolfssl Mar 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/dtls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -857,9 +857,9 @@ static int SendStatelessReplyDtls13(const WOLFSSL* ssl, WolfSSL_CH* ch)
nonConstSSL->options.tls1_1 = 1;
nonConstSSL->options.tls1_3 = 1;

XMEMCPY(nonConstSSL->session->sessionID, ch->sessionId.elements,
ch->sessionId.size);
nonConstSSL->session->sessionIDSz = (byte)ch->sessionId.size;
/* RFC 9147 Section 5.3: DTLS 1.3 ServerHello must have empty
* legacy_session_id_echo. Don't copy the client's session ID. */
nonConstSSL->session->sessionIDSz = 0;
nonConstSSL->options.cipherSuite0 = cs.cipherSuite0;
nonConstSSL->options.cipherSuite = cs.cipherSuite;
nonConstSSL->extensions = parsedExts;
Expand Down
75 changes: 60 additions & 15 deletions src/tls13.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4653,6 +4653,13 @@ int SendTls13ClientHello(WOLFSSL* ssl)
ssl->session->sessionIDSz = 0;
ssl->options.tls13MiddleBoxCompat = 0;
}
#endif
#ifdef WOLFSSL_DTLS13
if (ssl->options.dtls) {
/* RFC 9147 Section 5: DTLS implementations do not use the
* TLS 1.3 "compatibility mode" */
ssl->options.tls13MiddleBoxCompat = 0;
}
#endif
GetTls13SessionId(ssl, NULL, &sessIdSz);
args->length += (word16)sessIdSz;
Expand Down Expand Up @@ -5596,16 +5603,25 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
}
else
#endif /* WOLFSSL_TLS13_MIDDLEBOX_COMPAT */
#if defined(WOLFSSL_QUIC) || defined(WOLFSSL_DTLS13)
if (0
#ifdef WOLFSSL_QUIC
if (WOLFSSL_IS_QUIC(ssl)) {
|| WOLFSSL_IS_QUIC(ssl)
#endif
#ifdef WOLFSSL_DTLS13
|| ssl->options.dtls
#endif
) {
/* RFC 9147 Section 5.3 / RFC 9001 Section 8.4: DTLS 1.3 and QUIC
* ServerHello must have empty legacy_session_id_echo. */
if (args->sessIdSz != 0) {
WOLFSSL_MSG("args->sessIdSz != 0");
WOLFSSL_ERROR_VERBOSE(INVALID_PARAMETER);
return INVALID_PARAMETER;
}
}
else
#endif /* WOLFSSL_QUIC */
#endif /* WOLFSSL_QUIC || WOLFSSL_DTLS13 */
if (args->sessIdSz != ssl->session->sessionIDSz || (args->sessIdSz > 0 &&
XMEMCMP(ssl->session->sessionID, args->sessId, args->sessIdSz) != 0))
{
Expand Down Expand Up @@ -6568,6 +6584,7 @@ static int RestartHandshakeHashWithCookie(WOLFSSL* ssl, Cookie* cookie)
word16 length;
int keyShareExt = 0;
int ret;
byte sessIdSz;

ret = TlsCheckCookie(ssl, cookie->data, (byte)cookie->len);
if (ret < 0)
Expand All @@ -6592,7 +6609,13 @@ static int RestartHandshakeHashWithCookie(WOLFSSL* ssl, Cookie* cookie)
return ret;

/* Reconstruct the HelloRetryMessage for handshake hash. */
length = HRR_BODY_SZ - ID_LEN + ssl->session->sessionIDSz +
sessIdSz = ssl->session->sessionIDSz;
#ifdef WOLFSSL_DTLS13
Copy link
Contributor

@rizlik rizlik Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't gather single if without the body as it makes the code very hard to read.
options.dtls is not guarded.
Prefer using an extra sessionIDSz variable set to zero when options.dtls is true

Copy link
Member Author

@julek-wolfssl julek-wolfssl Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using var

/* RFC 9147 Section 5.3: DTLS 1.3 must use empty legacy_session_id. */
if (ssl->options.dtls)
sessIdSz = 0;
#endif
length = HRR_BODY_SZ - ID_LEN + sessIdSz +
HRR_COOKIE_HDR_SZ + cookie->len;
length += HRR_VERSIONS_SZ;
/* HashSz (1 byte) + Hash (HashSz bytes) + CipherSuite (2 bytes) */
Expand All @@ -6619,10 +6642,10 @@ static int RestartHandshakeHashWithCookie(WOLFSSL* ssl, Cookie* cookie)
XMEMCPY(hrr + hrrIdx, helloRetryRequestRandom, RAN_LEN);
hrrIdx += RAN_LEN;

hrr[hrrIdx++] = ssl->session->sessionIDSz;
if (ssl->session->sessionIDSz > 0) {
XMEMCPY(hrr + hrrIdx, ssl->session->sessionID, ssl->session->sessionIDSz);
hrrIdx += ssl->session->sessionIDSz;
hrr[hrrIdx++] = sessIdSz;
if (sessIdSz > 0) {
XMEMCPY(hrr + hrrIdx, ssl->session->sessionID, sessIdSz);
hrrIdx += sessIdSz;
}

/* Cipher Suite */
Expand All @@ -6633,7 +6656,7 @@ static int RestartHandshakeHashWithCookie(WOLFSSL* ssl, Cookie* cookie)
hrr[hrrIdx++] = 0;

/* Extensions' length */
length -= HRR_BODY_SZ - ID_LEN + ssl->session->sessionIDSz;
length -= HRR_BODY_SZ - ID_LEN + sessIdSz;
c16toa(length, hrr + hrrIdx);
hrrIdx += 2;

Expand Down Expand Up @@ -7058,9 +7081,20 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
if (sessIdSz + args->idx > helloSz)
ERROR_OUT(BUFFER_ERROR, exit_dch);

ssl->session->sessionIDSz = sessIdSz;
if (sessIdSz > 0)
XMEMCPY(ssl->session->sessionID, input + args->idx, sessIdSz);
#ifdef WOLFSSL_DTLS13
Copy link
Contributor

@rizlik rizlik Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar to comments above

Copy link
Member Author

@julek-wolfssl julek-wolfssl Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using var

/* RFC 9147 Section 5.3: DTLS 1.3 ServerHello must have empty
* legacy_session_id_echo. Don't store the client's value so it
* won't be echoed in SendTls13ServerHello. */
if (ssl->options.dtls) {
ssl->session->sessionIDSz = 0;
}
else
#endif
{
ssl->session->sessionIDSz = sessIdSz;
if (sessIdSz > 0)
XMEMCPY(ssl->session->sessionID, input + args->idx, sessIdSz);
}
args->idx += sessIdSz;

#ifdef WOLFSSL_TLS13_MIDDLEBOX_COMPAT
Expand Down Expand Up @@ -7618,10 +7652,21 @@ int SendTls13ServerHello(WOLFSSL* ssl, byte extMsgType)
WOLFSSL_BUFFER(ssl->arrays->serverRandom, RAN_LEN);
#endif

output[idx++] = ssl->session->sessionIDSz;
if (ssl->session->sessionIDSz > 0) {
XMEMCPY(output + idx, ssl->session->sessionID, ssl->session->sessionIDSz);
idx += ssl->session->sessionIDSz;
#ifdef WOLFSSL_DTLS13
if (ssl->options.dtls) {
/* RFC 9147 Section 5.3: DTLS 1.3 ServerHello must have empty
* legacy_session_id_echo. */
output[idx++] = 0;
}
else
#endif
{
output[idx++] = ssl->session->sessionIDSz;
if (ssl->session->sessionIDSz > 0) {
XMEMCPY(output + idx, ssl->session->sessionID,
ssl->session->sessionIDSz);
idx += ssl->session->sessionIDSz;
}
}

/* Chosen cipher suite */
Expand Down
86 changes: 86 additions & 0 deletions tests/api/test_dtls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2609,3 +2609,89 @@ int test_dtls13_min_rtx_interval(void)
#endif
return EXPECT_RESULT();
}

/* RFC 9147 Section 5.3: DTLS 1.3 ServerHello must have empty
* legacy_session_id_echo, even if the ClientHello had a non-empty
* legacy_session_id. */
int test_dtls13_no_session_id_echo(void)
{
EXPECT_DECLS;
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS13)
struct test_memio_ctx test_ctx;
WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
WOLFSSL_SESSION *sess = NULL;
char readBuf[1];
/* Use traditional groups to avoid HRR from PQ key share mismatch */
int groups[] = {
WOLFSSL_ECC_SECP256R1,
WOLFSSL_ECC_SECP384R1,
};

/* First connection: complete a DTLS 1.3 handshake to get a session */
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method), 0);
ExpectIntEQ(wolfSSL_set_groups(ssl_c, groups, 2), WOLFSSL_SUCCESS);
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);

/* Read to process any NewSessionTicket */
ExpectIntEQ(wolfSSL_read(ssl_c, readBuf, sizeof(readBuf)), -1);
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);

ExpectNotNull(sess = wolfSSL_get1_session(ssl_c));

/* Ensure the session has a non-empty session ID so the ClientHello
* will have a populated legacy_session_id field (which is legal per
* RFC 9147). */
if (sess->sessionIDSz == 0) {
sess->sessionIDSz = ID_LEN;
XMEMSET(sess->sessionID, 0x42, ID_LEN);
}

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 connection: set the session on the client so the ClientHello
* contains a non-empty legacy_session_id. Verify the server does NOT
* echo it in the ServerHello. */
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method), 0);
ExpectIntEQ(wolfSSL_set_session(ssl_c, sess), WOLFSSL_SUCCESS);
/* Use traditional groups to avoid HRR from key share mismatch */
ExpectIntEQ(wolfSSL_set_groups(ssl_c, groups, 2), WOLFSSL_SUCCESS);
/* Disable HRR cookie so the server directly sends a ServerHello */
ExpectIntEQ(wolfSSL_disable_hrr_cookie(ssl_s), WOLFSSL_SUCCESS);

/* Client sends ClientHello (with non-empty legacy_session_id) */
ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1);
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);

/* Server processes ClientHello and sends ServerHello + flight */
ExpectIntEQ(wolfSSL_negotiate(ssl_s), -1);
ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ);

/* Verify the ServerHello on the wire.
* Layout: DTLS Record Header (13) + DTLS Handshake Header (12) +
* ProtocolVersion (2) + Random (32) = offset 59 for
* legacy_session_id_echo length byte. */
ExpectIntGE(test_ctx.c_len, 60);
ExpectIntEQ(test_ctx.c_buff[0], handshake);
ExpectIntEQ(test_ctx.c_buff[DTLS_RECORD_HEADER_SZ], server_hello);
ExpectIntEQ(test_ctx.c_buff[DTLS_RECORD_HEADER_SZ +
DTLS_HANDSHAKE_HEADER_SZ + OPAQUE16_LEN + RAN_LEN], 0);

/* Complete the handshake */
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);

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();
}
4 changes: 3 additions & 1 deletion tests/api/test_dtls.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ int test_dtls_memio_wolfio_stateless(void);
int test_dtls_mtu_fragment_headroom(void);
int test_dtls_mtu_split_messages(void);
int test_dtls13_min_rtx_interval(void);
int test_dtls13_no_session_id_echo(void);

#define TEST_DTLS_DECLS \
TEST_DECL_GROUP("dtls", test_dtls12_basic_connection_id), \
Expand Down Expand Up @@ -79,5 +80,6 @@ int test_dtls13_min_rtx_interval(void);
TEST_DECL_GROUP("dtls", test_dtls_mtu_fragment_headroom), \
TEST_DECL_GROUP("dtls", test_dtls_mtu_split_messages), \
TEST_DECL_GROUP("dtls", test_dtls_memio_wolfio_stateless), \
TEST_DECL_GROUP("dtls", test_dtls13_min_rtx_interval)
TEST_DECL_GROUP("dtls", test_dtls13_min_rtx_interval), \
TEST_DECL_GROUP("dtls", test_dtls13_no_session_id_echo)
#endif /* TESTS_API_DTLS_H */
72 changes: 72 additions & 0 deletions tests/api/test_tls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -768,3 +768,75 @@ int test_tls_set_curves_list_ecc_fallback(void)
return EXPECT_RESULT();
}

int test_tls_session_id_resume_downgrade(void)
{
EXPECT_DECLS;
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
!defined(WOLFSSL_NO_TLS12) && !defined(NO_SESSION_CACHE)
struct {
method_provider client_meth_v12;
method_provider server_meth;
method_provider client_meth;
int expected_version;
} params[] = {
#ifdef WOLFSSL_TLS13
/* TLS 1.2 client → server, then client resumes at 1.2 */
{ wolfTLSv1_2_client_method, wolfTLS_server_method,
wolfTLS_client_method, TLS1_2_VERSION },
#endif
#if defined(WOLFSSL_DTLS) && defined(WOLFSSL_DTLS13)
/* DTLS 1.2 client → server, then client resumes at 1.2 */
{ wolfDTLSv1_2_client_method, wolfDTLS_server_method,
wolfDTLS_client_method, DTLS1_2_VERSION },
#endif
};
size_t i;

for (i = 0; i < sizeof(params)/sizeof(*params) && !EXPECT_FAIL(); i++) {
struct test_memio_ctx test_ctx;
WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
WOLFSSL_SESSION *sess = NULL;

/* --- first connection: v1.2-only client to server --- */
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c,
&ssl_s, params[i].client_meth_v12,
params[i].server_meth), 0);
/* Disable tickets so resumption must use session IDs */
if (EXPECT_SUCCESS())
wolfSSL_CTX_set_options(ctx_s, WOLFSSL_OP_NO_TICKET);
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
ExpectIntEQ(wolfSSL_version(ssl_c), params[i].expected_version);

ExpectNotNull(sess = wolfSSL_get1_session(ssl_c));
#ifdef HAVE_SESSION_TICKET
ExpectIntEQ(sess->ticketLen, 0);
#endif

wolfSSL_free(ssl_c); ssl_c = NULL;
wolfSSL_free(ssl_s); ssl_s = NULL;
wolfSSL_CTX_free(ctx_c); ctx_c = NULL;
/* keep ctx_s so the server session cache is available */

/* --- second connection: client resumes the v1.2 session --- */
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c,
&ssl_s, params[i].client_meth,
params[i].server_meth), 0);
ExpectIntEQ(wolfSSL_set_session(ssl_c, sess), WOLFSSL_SUCCESS);
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);

ExpectTrue(wolfSSL_session_reused(ssl_c));
ExpectIntEQ(wolfSSL_version(ssl_c), params[i].expected_version);

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();
}

4 changes: 3 additions & 1 deletion tests/api/test_tls.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ int test_tls_certreq_order(void);
int test_tls12_bad_cv_sig_alg(void);
int test_tls12_no_null_compression(void);
int test_tls_set_curves_list_ecc_fallback(void);
int test_tls_session_id_resume_downgrade(void);

#define TEST_TLS_DECLS \
TEST_DECL_GROUP("tls", test_utils_memio_move_message), \
Expand All @@ -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_tls_set_curves_list_ecc_fallback)
TEST_DECL_GROUP("tls", test_tls_set_curves_list_ecc_fallback), \
TEST_DECL_GROUP("tls", test_tls_session_id_resume_downgrade)

#endif /* TESTS_API_TEST_TLS_H */
Loading