Skip to content

Preserve DTLS association on invalid record headers during handshake #10826

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
yosuke-wolfssl wants to merge 1 commit into
base: master
Choose a base branch
from
+141 −1
Open

Preserve DTLS association on invalid record headers during handshake #10826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22920,7 +22920,9 @@ static int DtlsShouldDrop(WOLFSSL* ssl, int retcode)
if ((ssl->options.handShakeDone && retcode != 0)
|| retcode == WC_NO_ERR_TRACE(SEQUENCE_ERROR)
|| retcode == WC_NO_ERR_TRACE(DTLS_CID_ERROR)
|| retcode == WC_NO_ERR_TRACE(DTLS_PARTIAL_RECORD_READ)) {
|| retcode == WC_NO_ERR_TRACE(DTLS_PARTIAL_RECORD_READ)
|| retcode == WC_NO_ERR_TRACE(UNKNOWN_RECORD_TYPE)
|| retcode == WC_NO_ERR_TRACE(LENGTH_ERROR)) {
WOLFSSL_MSG_EX("Silently dropping DTLS message: %d", retcode);
return 1;
}
Expand Down
135 changes: 135 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 @@ -675,6 +675,137 @@ static int test_dtls_communication(WOLFSSL *s, WOLFSSL *c)
return EXPECT_RESULT();
}

/* Drive a DTLS handshake until the dropping peer reads a genuine flight record,
* corrupt its header, confirm the peer silently discards it, then re-deliver
* the record and confirm the handshake still completes. */
static int test_dtls_drop_invalid_record(method_provider method_c,
method_provider method_s, int dropServer, int unknownType)
{
EXPECT_DECLS;
WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
struct test_memio_ctx test_ctx;
/* Sized for the small cookie-exchange/HRR/ClientHello records this test
* corrupts; a PQC-default build with a large key_share may need more, and
* the ExpectIntLE(recSz) guard below fails loudly rather than overflowing. */
unsigned char rec[2048];

@Frauschi Frauschi Jul 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Test is build-sensitive to a single-datagram first flight

The corrupt/re-inject model here assumes the peer's flight is a single datagram — enforced by ExpectIntEQ(... c_msg_count : s_msg_count, 1) and the fixed rec[2048] size. That holds for default builds, but a PQC-default build with a large key_share fragments the first flight across multiple datagrams. When that happens c_msg_count/s_msg_count != 1 and this case fails (loudly, not unsafely — the ExpectIntLE(recSz) guard prevents any overflow) rather than exercising the drop path.

Rather than only documenting the assumption, consider pinning a non-PQC group on both CTXs right after test_memio_setup so the single-datagram invariant holds regardless of build defaults. Something like a file-scope static const int secp256r1_group[] = { WOLFSSL_ECC_SECP256R1 }; and then:

ExpectIntEQ(wolfSSL_CTX_set_groups(ctx_c, (int*)secp256r1_group, 1), WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_CTX_set_groups(ctx_s, (int*)secp256r1_group, 1), WOLFSSL_SUCCESS);

This keeps the clean corrupt/re-inject logic and makes the test deterministic across CI matrices that change DTLS defaults. Non-blocking — the drop-path validation is header-level and doesn't depend on whether the flight was fragmented.

unsigned char* recBuf;
int recSz = 0;
int readIsClient = dropServer ? 0 : 1;

XMEMSET(&test_ctx, 0, sizeof(test_ctx));
recBuf = readIsClient ? test_ctx.c_buff : test_ctx.s_buff;

ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
method_c, method_s), 0);

/* Client sends the ClientHello into the server's buffer. */
wolfSSL_SetLoggingPrefix("client");
ExpectIntEQ(wolfSSL_connect(ssl_c), WOLFSSL_FATAL_ERROR);
ExpectIntEQ(wolfSSL_get_error(ssl_c, WOLFSSL_FATAL_ERROR),
WOLFSSL_ERROR_WANT_READ);

/* When dropping at the client, let the server produce its first flight into
* the client's buffer. */
if (!dropServer) {
wolfSSL_SetLoggingPrefix("server");
ExpectIntEQ(wolfSSL_accept(ssl_s), WOLFSSL_FATAL_ERROR);
ExpectIntEQ(wolfSSL_get_error(ssl_s, WOLFSSL_FATAL_ERROR),
WOLFSSL_ERROR_WANT_READ);
}

/* Corrupt then replay the datagram the dropping peer is about to read. This
* assumes a single-datagram flight (asserted below) so re-injecting cannot
* merge record boundaries or drop other queued datagrams. */
ExpectIntEQ(readIsClient ? test_ctx.c_msg_count : test_ctx.s_msg_count, 1);
recSz = readIsClient ? test_ctx.c_len : test_ctx.s_len;
ExpectIntGT(recSz, DTLS_RECORD_HEADER_SZ);
ExpectIntLE(recSz, (int)sizeof(rec));
if (EXPECT_SUCCESS()) {
XMEMCPY(rec, recBuf, (size_t)recSz);
if (unknownType) {
/* Not a valid ContentType, and not a DTLS 1.3 unified-header byte
* (those have top bits 001, i.e. 0x20-0x3F). */
recBuf[0] = 0x63;
}
else {
recBuf[DTLS_RECORD_HEADER_SZ - 2] = 0xff;
recBuf[DTLS_RECORD_HEADER_SZ - 1] = 0xff;
}
}

/* The dropping peer must silently discard the datagram: no fatal error and
* nothing sent back to the other peer. */
if (dropServer) {
wolfSSL_SetLoggingPrefix("server");
ExpectIntEQ(wolfSSL_accept(ssl_s), WOLFSSL_FATAL_ERROR);
ExpectIntEQ(wolfSSL_get_error(ssl_s, WOLFSSL_FATAL_ERROR),
WOLFSSL_ERROR_WANT_READ);
ExpectIntEQ(test_ctx.c_len, 0);
}
else {
wolfSSL_SetLoggingPrefix("client");
ExpectIntEQ(wolfSSL_connect(ssl_c), WOLFSSL_FATAL_ERROR);
ExpectIntEQ(wolfSSL_get_error(ssl_c, WOLFSSL_FATAL_ERROR),
WOLFSSL_ERROR_WANT_READ);
ExpectIntEQ(test_ctx.s_len, 0);
}

/* Re-deliver the genuine record and finish the handshake to prove the
* association was preserved. */
test_memio_clear_buffer(&test_ctx, readIsClient);
ExpectIntEQ(test_memio_inject_message(&test_ctx, readIsClient,
(const char*)rec, recSz),
0);
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);

ExpectIntEQ(test_dtls_communication(ssl_s, ssl_c), TEST_SUCCESS);

wolfSSL_SetLoggingPrefix(NULL);
wolfSSL_free(ssl_c);
wolfSSL_free(ssl_s);
wolfSSL_CTX_free(ctx_c);
wolfSSL_CTX_free(ctx_s);

return EXPECT_RESULT();
}

/* A DTLS peer must silently discard a handshake record whose header fails
* validation (UNKNOWN_RECORD_TYPE, LENGTH_ERROR) and still finish the handshake.
* Cover client and server paths for DTLS 1.2 and 1.3. */
int test_dtls_drop_invalid_record_during_handshake(void)
{
EXPECT_DECLS;

/* Client drops a corrupted server flight: unknown type, then over-length. */
ExpectIntEQ(test_dtls_drop_invalid_record(wolfDTLSv1_2_client_method,
wolfDTLSv1_2_server_method, 0, 1), TEST_SUCCESS);
ExpectIntEQ(test_dtls_drop_invalid_record(wolfDTLSv1_2_client_method,
wolfDTLSv1_2_server_method, 0, 0), TEST_SUCCESS);
/* Server drops a ClientHello with a bad record header: over-length, then
* unknown ContentType (dropped by the new UNKNOWN_RECORD_TYPE clause, which
* precedes the server-side non-stateful drop branch). */
ExpectIntEQ(test_dtls_drop_invalid_record(wolfDTLSv1_2_client_method,
wolfDTLSv1_2_server_method, 1, 0), TEST_SUCCESS);
ExpectIntEQ(test_dtls_drop_invalid_record(wolfDTLSv1_2_client_method,
wolfDTLSv1_2_server_method, 1, 1), TEST_SUCCESS);

#ifdef WOLFSSL_DTLS13
/* Same silent-drop behavior on the DTLS 1.3 receive path (all four
* side/corruption combinations). */
ExpectIntEQ(test_dtls_drop_invalid_record(wolfDTLSv1_3_client_method,
wolfDTLSv1_3_server_method, 0, 1), TEST_SUCCESS);
ExpectIntEQ(test_dtls_drop_invalid_record(wolfDTLSv1_3_client_method,
wolfDTLSv1_3_server_method, 0, 0), TEST_SUCCESS);
ExpectIntEQ(test_dtls_drop_invalid_record(wolfDTLSv1_3_client_method,
wolfDTLSv1_3_server_method, 1, 0), TEST_SUCCESS);
ExpectIntEQ(test_dtls_drop_invalid_record(wolfDTLSv1_3_client_method,
wolfDTLSv1_3_server_method, 1, 1), TEST_SUCCESS);
#endif

return EXPECT_RESULT();
}

#if defined(WOLFSSL_DTLS13) && !defined(WOLFSSL_DTLS_RECORDS_CAN_SPAN_DATAGRAMS)
int test_dtls13_longer_length(void)
{
Expand Down Expand Up @@ -1007,6 +1138,10 @@ int test_dtls_short_ciphertext(void)
return EXPECT_RESULT();
}
#else
int test_dtls_drop_invalid_record_during_handshake(void)
{
return TEST_SKIPPED;
}
int test_dtls_short_ciphertext(void)
{
return TEST_SKIPPED;
Expand Down
3 changes: 3 additions & 0 deletions tests/api/test_dtls.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ int test_dtls12_basic_connection_id(void);
int test_wolfSSL_dtls_cid_parse(void);
int test_wolfSSL_dtls_set_pending_peer(void);
int test_dtls_version_checking(void);
int test_dtls_drop_invalid_record_during_handshake(void);
int test_dtls_short_ciphertext(void);
int test_dtls12_record_length_mismatch(void);
int test_dtls12_short_read(void);
Expand Down Expand Up @@ -106,6 +107,8 @@ int test_WOLFSSL_dtls_version_alert(void);
TEST_DECL_GROUP("dtls", test_wolfSSL_dtls_cid_parse), \
TEST_DECL_GROUP("dtls", test_wolfSSL_dtls_set_pending_peer), \
TEST_DECL_GROUP("dtls", test_dtls_version_checking), \
TEST_DECL_GROUP("dtls", \
test_dtls_drop_invalid_record_during_handshake), \
TEST_DECL_GROUP("dtls", test_dtls_short_ciphertext), \
TEST_DECL_GROUP("dtls", test_dtls12_record_length_mismatch), \
TEST_DECL_GROUP("dtls", test_dtls12_short_read), \
Expand Down
Loading