sniffer: use the wire session id length in both hello parsers

[yosuke-wolfssl]

Problem

src/sniffer.c was the only TLS parser in the tree that ignored the wire session-ID length: both hello parsers validated, copied and recorded a hardcoded ID_LEN (32). RFC 5246 §7.4.1.2 permits 0–32.

• Legal short IDs are dropped. The guard asked whether 32 bytes fit (ID_LEN > *sslBytes), so a compact hello — 16-byte ID, one cipher suite, one compression method, 22 bytes remaining — is torn down with a fatal error.
• Non-session-ID bytes reach the cache key. For a short ID that is accepted, the 32-byte copy pulls in the cipher suite, compression and extension bytes that follow, and ProcessFinished() feeds that value to AddSession() as a session cache key.
• Over-long lengths are silently truncated to 32 instead of rejected.
• The sessionIDSz == ID_LEN tests on the resumption comparison are tautologies, since both sides are forced to ID_LEN.

Closes f-13334.

Fix (src/sniffer.c)

ProcessServerHello() and ProcessClientHello() reject a declared length above ID_LEN, copy exactly that many bytes, and record it as sessionIDSz. ProcessClientHello() gains a read-through check against the declared length, which it never had. This matches DoServerHello() and DoClientHello() in src/internal.c and both TLS 1.3 parsers.

The resumption comparison is unchanged from master; its two == ID_LEN tests only become meaningful now that sessionIDSz holds the wire length. A comment records why the full length is required: the session cache matches only full-length IDs (src/ssl_sess.c:1173), as do the client and server resumption decisions (src/internal.c:34543, src/internal.c:41480).

Not in scope: short-ID resumption. wolfSSL treats resumption as 32-only throughout, so supporting shorter IDs means changing the cache and both peers' resumption decisions — a separate change. Short IDs are now parsed and recorded correctly; they simply do not resume.

Tests (tests/api.c)

test_sniffer_hello_session_id_len drives ssl_DecodePacket() with hand-built IPv4/TCP packets — no pcap needed, as the sniffer verifies no checksums.

Scenario	Expected
ClientHello, length ID_LEN + 1	reject
ClientHello, length runs past the record	reject
ServerHello, length ID_LEN + 1, after a zero-ID ClientHello	reject
ClientHello + ServerHello, differing 16-byte IDs	accept
ClientHello + ServerHello, matching 16-byte IDs	accept, no resumption
ClientHello + ServerHello, matching 32-byte IDs	resumption attempted

Verification

Every guard was confirmed load-bearing by reverting it individually and checking the test fails — including a control that restores the length-based comparison, pinning the ID_LEN requirement. The bLen > *sslBytes bound is ASan-only: whenever it fires, the next check rejects the same packet, so the over-read is the sole observable, and the hello packet is allocated at its exact size to expose it.

Clean under -Werror with and without TLS 1.3. make check 6 passed, 0 failed, 5 skipped. scripts/sniffer-testsuite.test passes all 10 pcaps with all 9 resume decrypts intact. ASan + UBSan clean on both the unit test and the pcap suite.

[Copilot]

🟡 Changes recommended

haveSessionId/sessionIDSz are not cleared when a hello carries a zero-length Session ID, which can allow stale state to affect resumption/ticket logic if additional hellos are processed on the same sniffer session.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR fixes the TLS sniffer (src/sniffer.c) to honor the on-the-wire Session ID length (0–32) rather than forcing ID_LEN (32), preventing incorrect cache keys/truncation and making resumption detection respect the actual negotiated Session ID size. It also adds an API test that synthesizes minimal IPv4/TCP/TLS records to exercise these edge cases without pcap input.

Changes:

• Update ClientHello/ServerHello parsing to reject session_id_len > ID_LEN, copy exactly session_id_len bytes, and record sessionIDSz accordingly.
• Update resumption detection to compare both Session ID sizes and only compare the number of bytes actually present.
• Add a new sniffer-focused test that validates rejection/acceptance/resumption behavior for varying Session ID lengths and malformed lengths.

File summaries

File	Description
src/sniffer.c	Fixes sniffer hello parsing to respect the wire Session ID length and updates resumption comparison accordingly.
tests/api.c	Adds a targeted regression test that builds minimal packets to validate Session ID length handling in the sniffer.

Review details

Suppressed comments (1)

src/sniffer.c:4344

• haveSessionId/sessionIDSz are only set when bLen is non-zero; if another ClientHello is processed on the same sniffer session with session_id length 0, the previous session-id state can persist and affect later resumption decisions. Clear the session-id state before conditionally copying the new value.

    /* store session in case trying to resume */
    bLen = *input++;
    *sslBytes -= ENUM_LEN;
    if (bLen > ID_LEN) {
        SetError(CLIENT_HELLO_INPUT_STR, error, session, FATAL_ERROR_STATE);

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

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[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 #11390

Scan targets checked: wolfssl-bugs, wolfssl-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11390

Scan targets checked: wolfssl-bugs, wolfssl-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11390

Scan targets checked: wolfssl-bugs, wolfssl-src

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

Fenrir's latest completed scan found no issues; clearing the prior automated change request.