Declare QUIC record length from bytes remaining

[yosuke-wolfssl]

Problem

quic_record_transfer() synthesises TLS record headers around QUIC CRYPTO-stream data so the normal record layer can parse it. When opening a new record it derived the declared length from qr->len, the size of the whole handshake message, instead of the bytes still to be handed out. qr->start advances as data is transferred but qr->len never changes, so every record after the first still declared MAX_RECORD_SIZE (16384) no matter how few bytes remained.

Any handshake message above 16 KB triggers it: a Certificate with a long chain, or a ClientHello carrying large QUIC transport parameters. For a 20004-byte message the first record correctly declares 16384 bytes, then a second header declares 16384 again while only 3620 bytes follow. wolfSSL_quic_receive() frees the exhausted record and the record layer waits forever for body bytes that never arrive — a WANT_READ stall, or a stray 5-byte header written into the middle of a record body when another QuicRecord is queued behind it.

Fix (src/quic.c)

Records only reach ssl->quic.input_head once quic_record_complete() is true, so the bytes actually remaining are exactly the len already computed at the top of the function:

-        rlen = (qr->len <= (word32)MAX_RECORD_SIZE) ?
-                qr->len : (word32)MAX_RECORD_SIZE;
+        rlen = (len <= (word32)MAX_RECORD_SIZE) ?
+                len : (word32)MAX_RECORD_SIZE;

Closes f-7522.

Tests (tests/quic.c)

• test_quic_record_split() feeds a synthetic handshake message through wolfSSL_provide_quic_data() at three sizes and requires the client not to stall on WANT_READ:

qr->len	records	covers
MAX_RECORD_SIZE	1	single-record case, unchanged by the fix
MAX_RECORD_SIZE + 1	2	tail record declaring exactly 1 byte
MAX_RECORD_SIZE + 1028	2	larger tail record

• test_quic_big_client_hello() runs a full client/server handshake with 16000 bytes of client transport parameters, so the ClientHello itself spans two records. It asserts the ClientHello exceeded MAX_RECORD_SIZE, that the handshake completes, and that the server reads the blob back byte-identical — reassembly checked directly, not inferred from "did not stall".

Verification

• Clean build, no warnings.
• Full ./tests/unit.test passes in two configs: --enable-all --enable-quic (0 failed / 1770 passed) and --enable-quic --enable-session-ticket --enable-earlydata (0 failed / 941 passed).
• Negative controls: with the fix reverted, test_quic_record_split fails on the MAX_RECORD_SIZE + 1 case and test_quic_big_client_hello deadlocks with both peers in WANT_READ.

[Copilot]

🟢 Ready to approve

The fix is narrowly scoped, addresses a clear correctness bug, and is backed by targeted regression tests covering both synthetic and full-handshake scenarios.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Fixes QUIC-to-record-layer bridging in quic_record_transfer() so each synthesized TLS record header declares the number of bytes actually remaining to be transferred (not the original full handshake-message length), preventing WANT_READ stalls and malformed record framing when a handshake message spans multiple records.

Changes:

• Compute the declared TLS record length from the bytes remaining (len) instead of the total message length (qr->len) when starting a new record.
• Add unit/integration-style QUIC tests that exercise multi-record handshake-message splitting and validate a full handshake with an oversized ClientHello transport-parameters extension.

File summaries

File	Description
src/quic.c	Corrects TLS record header length calculation during QUIC CRYPTO-stream transfer to the record layer to prevent stalls/misframing on split records.
tests/quic.c	Adds coverage for handshake-message splitting across records and a large ClientHello transport-parameters scenario to prevent regressions.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11045

Scan targets checked: wolfssl-bugs, wolfssl-src

No new issues found in the changed files. ✅

[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

[dgarske]

Recommendation: Move the 205-207 sz < RECORD_HEADER_SZ guard inside the if (qr->rec_hdr_remain == 0) branch so it only applies when a header is actually about to be written.

[yosuke-wolfssl]

Done — the guard now sits inside the if (qr->rec_hdr_remain == 0) branch, so it only applies when a header is actually about to be written. A record body already in progress can now be handed out in pieces smaller than RECORD_HEADER_SZ instead of failing the whole wolfSSL_quic_receive() with WOLFSSL_FATAL_ERROR.

[dgarske]

I measured tclient.output.len at this commit in three configurations:
--enable-all --enable-quic -> 17754 (base 1754, margin +1370) pass
--enable-quic (default, ML-KEM on) -> 17401 (base 1401, margin +1017) pass
--enable-quic --disable-mlkem -> 16244 (base 244, margin -140) FAIL

Recommendation: Derive the transport-parameter size at runtime from a measured base ClientHello (test_quic_client_hello already demonstrates the connect-and-inspect-tctx.output pattern) instead of hard-coding 16000. Then re-verify against at least --enable-quic --disable-mlkem in addition to the two configs already listed in the PR description.

[yosuke-wolfssl]

Fixed as recommended. QUIC_BIG_TP_SZ is gone; quic_client_hello_len() does a probe connect with a 16-byte payload and the test solves for the size that lands the ClientHello at MAX_RECORD_SIZE + 1024:

tp_sz = MAX_RECORD_SIZE + QUIC_BIG_TP_MARGIN - (base_len - QUIC_TP_PROBE_SZ)

Guarded against underflow, against the 65535 limit in QuicTransportParam_new(), and against MAX_HANDSHAKE_SZ (config-dependent itself, so checked rather than assumed). Payload fill also changed from constant 0x42 to i & 0xff so the round-trip XMEMCMP catches mis-ordered reassembly, not just a wrong length.

Measured, reproducing your overheads exactly:

Config	probe base_len	overhead	derived tp_sz	ClientHello
--enable-all --enable-quic	1770	1754	15654	17408
--enable-quic (ML-KEM on)	1417	1401	16007	17408
--enable-quic --disable-mlkem	260	244	17164	17408

All three pass, plus the --enable-quic --enable-tls13 --enable-earlydata --enable-session-ticket config. Negative control: pinning tp_sz back to 16000 under --disable-mlkem fails on tclient.output.len > MAX_RECORD_SIZE with a 16244-byte ClientHello — exactly the case you reported.

[dgarske]

MEDIUM test_quic_record_split oracle is too weak and masks the 1-4 byte tail bug it was written to catch
Recommendation: Assert the concrete expected error (VERSION_ERROR) instead of merely != WANT_READ, so a spurious SOCKET_ERROR_E is caught. Also extend msg_sizes[] with the remaining tail sizes MAX_RECORD_SIZE - HANDSHAKE_HEADER_SZ + 2 / +3 / +4 -- with the tightened assertion those become a direct regression test for the header-guard bug.

[yosuke-wolfssl]

Agreed, and the diagnosis was right. Assertion tightened to ExpectIntEQ(wolfSSL_get_error(ssl, 0), WC_NO_ERR_TRACE(VERSION_ERROR)), and msg_sizes[] extended with MAX_RECORD_SIZE - HANDSHAKE_HEADER_SZ + 2 / +3 / +4.

The old != WANT_READ oracle was passing on the +1 case because of the header-guard bug — that path returned WOLFSSL_FATAL_ERROR, which is also "not WANT_READ". With the tightened assertion the +1 .. +4 cases fail if the guard move on the src/quic.c thread is reverted, so they are now a direct regression test for it.

[dgarske]

Recommendation: Add an early-data case that actually transfers a blob larger than MAX_RECORD_SIZE through the record layer (the existing WOLFSSL_EARLY_DATA conversation helpers in tests/quic.c can drive this), and assert the payload round-trips byte-identically, as test_quic_big_client_hello does for transport parameters.

[yosuke-wolfssl]

Added test_quic_big_early_data(): full handshake for a ticket, then resume and send MAX_RECORD_SIZE + 1024 bytes of early data patterned i & 0xff, with conv.accept_early_data = 1 so the server drives wolfSSL_read_early_data(). It asserts the blob really exceeded one record before the conversation runs (new output_level_len() helper), that early data was accepted, and that the server got all 17408 bytes back byte-identically.

QuicConversation.early_data grew from 16 KB to 32 KB to hold it. No change was needed to the forwarding path: ctx_add_handshake_data() already coalesces same-level writes into one OutputBuffer, so the whole blob reaches wolfSSL_provide_quic_data() in a single call and the server splits it on the application_data branch.

Negative control confirms it reaches the fixed code: with the rlen fix reverted, the test aborts in QuicConversation_do() with client_error=0, server_error=2 — the server stalled in WANT_READ waiting for body bytes that never arrive.