Preserve DTLS association on invalid record headers during handshake

[yosuke-wolfssl]

Preserve DTLS association on invalid record headers during the handshake

Summary

While a DTLS handshake is in progress (handShakeDone == 0), a single
injected or corrupted datagram whose record header fails validation
(UNKNOWN_RECORD_TYPE or LENGTH_ERROR) aborted the handshake instead of
being silently discarded. This let an off-path attacker tear down an
in-progress DTLS handshake with one spoofed packet. This PR makes those
record-header errors droppable during the handshake window, matching the
already-correct post-handshake behavior.

Root cause

In DoProcessReplyEx, after GetRecordHeader runs, the record is only
discarded when DtlsShouldDrop() returns 1; otherwise the negative error is
returned up the stack and the connection is torn down. DtlsShouldDrop()
dropped SEQUENCE_ERROR, DTLS_CID_ERROR, DTLS_PARTIAL_RECORD_READ, and the
post-handshake catch-all (handShakeDone && retcode != 0), but not
UNKNOWN_RECORD_TYPE (unrecognized ContentType) or LENGTH_ERROR
(oversized/zero-length/malformed records) while handShakeDone == 0.

The result was an asymmetry: an established DTLS connection already shrugged
off such a datagram, but a connection mid-handshake — the more
attacker-interesting window — was torn down by it.

Fix

Add UNKNOWN_RECORD_TYPE and LENGTH_ERROR to the silent-drop set in
DtlsShouldDrop(). The change is version-agnostic (applies to DTLS 1.2 and
1.3) and is scoped to DTLS only — DtlsShouldDrop() is called from the reply
path solely when ssl->options.dtls is set.

This aligns wolfSSL with RFC 6347 §4.1.2.7 / RFC 9147 §4.5.2, which
recommend (SHOULD) silently discarding invalid records — including bad length
and unknown type — to preserve the association, and explicitly warn that the
alert/teardown alternative is NOT RECOMMENDED over UDP (DoS).

Behavior change

Under HAVE_MAX_FRAGMENT, an over-length record during a DTLS handshake
previously reached the record_overflow fatal-alert path in DoProcessReplyEx;
it is now silently dropped before that switch. This is intentional and
RFC-preferred for DTLS. The alert path is unchanged for TLS — the call site is
gated on ssl->options.dtls, so TLS still sends record_overflow.

VERSION_ERROR is deliberately not added, so the legitimate version-alert path
for a downgrading client is untouched.

Testing

Added test_dtls_drop_invalid_record_during_handshake (tests/api/test_dtls.c),
built on a small helper parameterized by side (client/server drop) and
corruption mode (unknown ContentType / over-length). It corrupts a genuine
handshake-flight record the peer is about to read, asserts the peer returns
WANT_READ (no fatal error) and sends nothing back, then re-delivers the
genuine record and confirms the handshake completes and data flows.

Matrix — all four (side x corruption) combinations for both DTLS 1.2 and
DTLS 1.3 (8 cases).

Verification:

• Negative control: with the fix reverted, the discriminator cases fail
  (handshake torn down); with the fix, all pass.
• Non-DTLS / non-memio builds: #else TEST_SKIPPED stub keeps the unit test
  linking; the test reports skipped.

Compatibility

No public API changes. No behavior change for TLS. For DTLS, the handshake
window now behaves like an established connection for these record-header
errors.

Addressed by f_6539.

[Copilot]

Pull request overview

This PR hardens DTLS handshake robustness by ensuring invalid DTLS record headers received during an in-progress handshake are silently dropped (instead of aborting the association), aligning handshake behavior with the already-droppable post-handshake path and RFC DoS guidance.

Changes:

• Extend DtlsShouldDrop() to treat UNKNOWN_RECORD_TYPE and LENGTH_ERROR as droppable DTLS errors during the handshake window.
• Add a DTLS unit test that corrupts a real handshake record header (unknown ContentType / over-length) and verifies the peer returns WANT_READ, sends nothing back, then successfully completes the handshake when the genuine record is re-delivered.
• Register the new unit test in the DTLS test header and test list.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
tests/api/test_dtls.h	Adds prototype + registers the new DTLS handshake drop test in the DTLS test group.
tests/api/test_dtls.c	Adds a new memio-based DTLS test that corrupts handshake record headers and asserts silent-drop + handshake completion.
src/internal.c	Updates DTLS drop policy to include UNKNOWN_RECORD_TYPE and LENGTH_ERROR during handshake processing.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

MemBrowse Memory Report

No memory changes detected for:

• gcc-arm-cortex-m0plus
• gcc-arm-cortex-m3
• gcc-arm-cortex-m4
• gcc-arm-cortex-m4-baremetal
• gcc-arm-cortex-m4-crypto-only
• gcc-arm-cortex-m4-dtls13
• gcc-arm-cortex-m4-min-ecc
• gcc-arm-cortex-m4-openssl-compat
• gcc-arm-cortex-m4-pkcs7
• gcc-arm-cortex-m4-pq
• gcc-arm-cortex-m4-rsa-only
• gcc-arm-cortex-m4-sp-math
• gcc-arm-cortex-m4-tls12
• gcc-arm-cortex-m4-tls13
• gcc-arm-cortex-m7
• gcc-arm-cortex-m7-pq
• gcc-arm-cortex-m7-tls13
• linuxkm-pie
• linuxkm-standard
• stm32-sim-stm32h753

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #10826

Scan targets checked: wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src

No new issues found in the changed files. ✅

[Frauschi]

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.