Skip to content

Fenrir fixes 2026 08 20 - #157

Merged
gasbytes merged 35 commits into
wolfSSL:masterfrom
danielinux:fenrir-fixes-2026-08-20
Aug 21, 2026
Merged

Fenrir fixes 2026 08 20#157
gasbytes merged 35 commits into
wolfSSL:masterfrom
danielinux:fenrir-fixes-2026-08-20

Conversation

@danielinux

@danielinux danielinux commented Aug 20, 2026

Copy link
Copy Markdown
Member

13e95d6 F-6473: seed TS.Recent from the first peer timestamp
a4197a5 F-9808: make timer_binheap_cancel eager to stop dead-slot heap exhaustion
e30d7dc F-8559: correct the source-route drop rationale comment
44c7204 F-10279: make the forwarding filter notification IHL-aware
e58a377 F-8526: make DHCPACK config commit atomic
7f9d3b9 F-6940: set the DHCP BROADCAST bit while the client has no bound IP
3f63c55 F-10276: RENEWING/REBINDING DHCPREQUEST retries use half-remaining-time
0ee8f61 F-6943: pin the PMTU never-increase guard on peer_mss
9bd3217 F-10265: RST replies carry a Timestamps option when the trigger had one
e612852 F-6474: RST on unacceptable ACK number in SYN_SENT (bare ACKs too)
d9f119c F-6944: test the future-TSecr guard in tcp_process_ts
7ef3453 F-8520: drop dead IP length re-check in the tcp_input socket loop
a95ff61 F-6473: gate TS.Recent update on the RFC 7323 4.3 preconditions
0ab2e47 F-10271: apply RFC 9293 acceptability in SYN_RCVD instead of exact seq match
920af46 F-6939: zero echo reply code per RFC 792
764b0bb F-10263: drop the redundant second AES-GCM key set
7a5edaf F-10262: zeroize the full GCM nonce in the four ESP helpers
f0b4aa9 F-8519: do not free the Hmac when wc_HmacInit failed
78ba53b F-8573: widen the ESP anti-replay window to 64 packets
a15ea21 F-8572: skip anti-replay for SAs without integrity protection
2f1b399 F-10264: fix the gmac enum name in the sa_new_gcm comment
1f00ca3 F-9379: document that get_pmk reports PMK availability, not success
b36b1a0 F-9378: document the mode-specific wolfip_supplicant_kick transitions
6e176da F-9377: document which init failures zero the context
bf889f8 F-9376: require zero-init of the context before first supplicant init
26a64d3 F-9375: correct the ap_rsn_ie NULL-fallback comment
9c92b47 F-9373: soften the ETH NVIC prerequisite in the HAL docs
c148fe3 F-9372: document that the HAL port forces RMII
8f3b60e F-9371: describe what rp2350_spi_init actually configures
9556686 F-9370: correct rp2350_pio_xfer32 bit-count contract
da02bdb F-9369: document when the cyw43_get_mac cache is valid
3c021c6 F-9368: stop claiming WPA3/SAE in cyw43_connect docs
e532c85 F-9367: document the real cyw43_connect return contract

Copilot AI lite review requested due to automatic review settings August 20, 2026 16:05
@danielinux danielinux self-assigned this Aug 20, 2026

Copilot AI left a comment

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.

Pull request overview

This PR delivers a set of protocol-correctness and robustness fixes across the wolfIP IPv4/TCP/DHCP stack and wolfESP (IPsec/ESP), plus accompanying unit tests and clarifying documentation for several ports/APIs.

Changes:

  • Fix timer binheap cancellation to physically remove canceled timers and restore heap ordering, preventing dead-slot accumulation.
  • Make filter notifications IHL-aware for forwarded packets (avoid sampling IP option bytes as L4 headers) and add related tests.
  • Improve TCP/DHCP/ESP correctness (RFC-aligned TCP handshake/PAWS/timestamp/RST handling, DHCP broadcast/retry timing and ACK atomicity, ESP replay window sizing and cipher-only anti-replay disablement).

Reviewed changes

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

Show a summary per file
File Description
wolfesp.h Increase ESP anti-replay window to 64 and widen replay bitmap to 64-bit.
src/wolfip.c Core stack fixes: filter IHL handling, timer heap cancel/heapify, TCP RFC compliance fixes, DHCP behavior hardening.
src/wolfesp.c ESP correctness changes (HMAC init/free handling, nonce zeroing, replay window logic and integrity-gated anti-replay).
src/test/unit/unit.c Register new unit tests covering new/changed behaviors.
src/test/unit/unit_tests_tcp_state.c Test fixture initialization update for TCP state tests.
src/test/unit/unit_tests_tcp_flow.c New/updated TCP SYN_RCVD sequence/OOO handling tests.
src/test/unit/unit_tests_tcp_ack.c New PAWS/timestamp/RST tests and timer-cancel expectation updates.
src/test/unit/unit_tests_proto.c Timer cancellation semantics tests + regression test for dead-slot accumulation.
src/test/unit/unit_tests_dns_dhcp.c New DHCP broadcast flag, ACK atomicity, ICMP echo code, PMTU monotonicity, and DHCP retry timing tests.
src/test/unit/unit_tests_dhcp_edges.c Drive DHCP expiry path through timer handling to match production behavior.
src/test/unit/unit_tests_api.c Filter metadata tests for IHL options + truncated-header guard behavior.
src/test/unit/unit_esp.c Update ESP replay window tests + new cipher-only SA anti-replay skip test.
src/supplicant/supplicant.h Clarify API semantics, initialization requirements, and handshake kick sequencing.
src/port/stm32_hal/stm32_hal_eth.h Documentation clarifications for RMII and interrupt/poll behavior.
src/port/rp2350_cyw43439/rp2350_spi.h Clarify init semantics (control lines, WL_REG_ON behavior, call-once constraint).
src/port/rp2350_cyw43439/rp2350_pio.h Clarify xfer bit-count constraints for PIO transactions.
src/port/rp2350_cyw43439/cyw43439_driver.h Clarify WPA2-PSK-only scope and async association/MAC-read semantics.

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

Comment thread src/wolfesp.c
cyw43_connect() returns the WLC_SET_SSID ioctl result; association
completes later via the async WLC_E_ASSOC / WLC_E_LINK events. State
that the caller must poll cyw43_assoc_up()/cyw43_assoc_seen() instead
of promising a (Re)Assoc Response at return time.
The protected branch hard-codes WPA2_AUTH_PSK and the WPA2-PSK RSN IE
with no AKM selector, and the host supplicant build compiles SAE out.
Document the port as open or WPA2-PSK only in both the surface
contract block and the declaration comment.
g_cyw43.mac is filled from the cur_etheraddr iovar read in
cyw43_wifi_up(), not during firmware load; before that call the
cache is zero-filled while ready (and the return value) is already 1.
Say so in the header comment.
The wrapper forwards to rp2350_pio_xfer, whose OSR drain and ISR
autopush require out_bits and in_bits to be full 32-bit words; the
documented 1..32 range was wrong and a sub-word count would desync
the PIO. Both in-tree callers pass 32/32.
It only sets the CPU-driven CS (deasserted) and WL_REG_ON (radio held
off); CLK/DATA and the PIO live in rp2350_pio_init. The old comment
claimed pad strength, mux, divider, controller reset, and repeat
safety - re-calling it while the radio runs powers the CYW43439 down.
Every supported-family branch (H5 SBS, H7 SYSCFG PMCR, F4/F7 SYSCFG
PMC) unconditionally selects RMII; there is no board-mode input and no
MII branch. State that MII boards must be configured manually in
HAL_ETH_MspInit instead of promising RMII/MII auto-detection.
hal_eth_poll() runs a hybrid RX path: HAL_ETH_ReadData is called every
100th poll even when the RxCplt interrupt never flagged data, so a
disabled ETH NVIC delays reception rather than preventing it. Document
that the interrupt is for promptness, not a hard prerequisite.
supp_build_rsn_ies() patches own_rsn_ie's AKM suite to the configured
mode (SAE 00:0F:AC:08, 802.1X :01, PSK :02) before copying it into the
ap_rsn_ie fallback slot, so the fallback follows the configured AKM
rather than always being a WPA2-PSK IE.
wolfip_supplicant_init() snapshots the PMKSA fields (magic, lengths,
PMK, SSID, ...) before memset-ing the context, so the first init
performs reads on whatever the caller stored. The magic/SSID match
prevents a garbage hit, but the reads are only defined for
zero-initialized storage. Document the zero-init precondition for
caller-owned contexts.
wolfip_supplicant_init() returns for NULL/invalid-cfg before its
memset, so validation failures leave the caller's previous bytes (and
any prior secrets) untouched; only failures after initialization
begins go through wolfip_supplicant_deinit() and zero the context.
Split the promise accordingly.
kick() does not always go IDLE -> 4WAY_M1_WAIT: EAP modes send
EAPOL-Start and enter EAP_IDENTITY_WAIT, software SAE sends the SAE
Commit and enters SAE_COMMIT_SENT (authentication precedes radio
association - waiting for assoc inverts the sequencing), and only PSK
or SAE-with-preinstalled-PMK reaches 4WAY_M1_WAIT.
The rejection test is (state==IDLE && !have_ptk && !pmk_installed &&
auth_mode!=PSK): PSK mode exports a PMK from IDLE, and a PTK or
installed SAE PMK bypasses the state check entirely, so a FAILED
context can return 0 with an all-zero PMK. State that callers must
check wolfip_supplicant_state() for handshake completion.
The enc parameter is esp_enc_t and the switch only accepts
ESP_ENC_GCM_RFC4106 / ESP_ENC_GCM_RFC4543; the comment named the
esp_auth_t value ESP_AUTH_GCM_RFC4543, which falls through to the
default case and fails SA setup.
RFC 4303 s3.4.3: anti-replay MUST NOT be enabled unless the SA also
provides integrity. esp_transport_unwrap() consulted and advanced the
replay window unconditionally, so a cipher-only SA (icv_len == 0,
allowed per the cipher_null/digest_null design decision) still ran
replay processing on an unprotected Sequence Number field - protecting
nothing, while letting an attacker who can rewrite the sequence advance
the window and drop legitimate packets.

Gate both esp_replay_check() and esp_replay_commit() on icv_len != 0
(HMAC and AEAD SAs keep anti-replay; cipher-only SAs do not).
Proven by test_unwrap_cipher_only_sa_skips_replay_window: a poisoned
inbound window no longer rejects a valid packet for an ICV-less SA,
and the window is left untouched.
RFC 4301 s3.3.1 recommends a replay window of at least 64 packets;
the fixed 32-packet window dropped older out-of-order packets from
protection sooner than the recommendation. Widen ESP_REPLAY_WIN to 64
and the inbound bitmap to uint64_t (shifts move to 1ULL), and update
the window-dependent replay unit tests to the 64-wide semantics.
esp_calc_icv_hmac() jumped to the common cleanup on a failed
wc_HmacInit() and called wc_HmacFree() on a context that was never
initialised - releasing state that was never set up. Return directly
from the init failure and free only when the init succeeded (same
inited-flag pattern as the GCM/DES3 helpers). Also fix the init-failure
log, which misnamed the failing API as wc_HmacSetKey.

Defensive error path: a failed wc_HmacInit is not reproducible in the
unit environment, so this is verified by build + the existing HMAC
roundtrip/tamper tests rather than a failing test.
The helpers built a 12-byte nonce (4-byte salt + 8-byte IV) but only
force-zeroed the 4-byte salt on exit. The salt (trailing key bytes) is
the secret part and was covered, while the IV bytes are wire-visible
or wire-derived - so no secret residue was left, but the partial
zeroize made the nonce buffer's cleanup ambiguous. Zero the whole
nonce in esp_aes_rfc4106_enc/dec and esp_aes_rfc4543_enc/dec; the
scratch is gone either way and the cleanup no longer depends on which
part of the nonce holds key material.

Behavior-neutral; verified by the GCM/GMAC roundtrip unit tests.
esp_aes_rfc4106_enc/dec called wc_AesGcmInit() with the key (which
installs the key itself - it calls wc_AesGcmSetKey internally, see
wolfSSL wc_AesGcmInit) and then called wc_AesGcmSetKey() again,
re-expanding the key on every ESP packet. Remove the redundant second
key set; the wc_AesGcmInit + one-shot wc_AesGcmEncrypt/Decrypt
sequence is the standard pattern and now sets the key exactly once.

The finding's 'streaming init API mixed with one-shot' framing is not
the case - only the double key setup was real.

Behavior-neutral; verified by the GCM/GMAC roundtrip unit tests.
The echo reply path swapped type/src/dst and recomputed the ICMP
checksum but copied the request's code field in place. RFC 792
defines code 0 for echo messages, so a request carrying a non-zero
code produced a reply that violated the defined encoding.

Zero the code before recomputing the checksum and pin the behavior
with test_icmp_echo_reply_code_zeroed, which sends a request with
code 5 and asserts the reply carries code 0 with a valid checksum.
…q match

The final-ACK branch required SEG.SEQ to equal RCV.NXT exactly and
reset the connection on any mismatch. RFC 9293 section 3.10.7.4
applies the Table 6 acceptability test first: a segment that begins
above RCV.NXT but inside the receive window (e.g. the peer's first
data segment reordering ahead of its final ACK) is acceptable and
SHOULD be held for later processing (SHLD-31), and an out-of-window
segment gets an acknowledgment and is dropped, not a reset.

- Use tcp_segment_acceptable() before the final-ACK transition; an
  in-window segment above RCV.NXT now completes the handshake and
  tcp_recv() caches it as OOO (RCV.NXT is no longer advanced to the
  segment's sequence, so the hole is not skipped).
- An unacceptable sequence now sends an ACK and drops; only an
  acceptable sequence with an unacceptable ACK still resets, per the
  RFC's per-state processing.
- The FIN-in-handshake-completing-segment handling now requires the
  FIN to be contiguous with RCV.NXT, mirroring the ESTABLISHED path,
  so a FIN above a hole cannot skip the receive window.

Tests: repurpose test_tcp_input_syn_rcvd_ack_invalid_seq_rejected
(the injected sequence is in-window and is now accepted) as
test_tcp_input_syn_rcvd_high_seq_valid_ack_establishes; add
test_tcp_input_syn_rcvd_high_seq_data_held_ooo (reordered first data
segment completes the handshake, is OOO-cached, and is delivered in
order once the hole fills) and
test_tcp_input_syn_rcvd_out_of_window_ack_drop (out-of-window ACK is
acknowledged and dropped, no RST, no state change). The FIN test
socket now initializes its rxbuf queue like tcp_new_socket does, so
the acceptability test sees the real receive window.
tcp_process_ts() copied every segment's TSval into last_ts
(TS.Recent) unconditionally. RFC 7323 section 4.3 rule (2) allows
the replacement only when SEG.TSval >= TS.Recent AND
SEG.SEQ <= Last.ACK.sent; any other value is ignored.

An out-of-order segment (SEQ above the left edge) advancing
TS.Recent is actively harmful: it corrupts the TSecr echoed per
rule (3), and when the in-order segment that fills the hole later
arrives - necessarily carrying a lower TSval than the OOO segment -
tcp_paws_check() sees it as an old duplicate and drops it, so the
hole never fills without a retransmission.

Apply both preconditions before updating last_ts (this stack ACKs
immediately, so Last.ACK.sent == RCV.NXT == t->sock.tcp.ack).

Tests: test_tcp_process_ts_ooo_segment_keeps_recent (OOO segment
does not advance TS.Recent, in-order does, an older TSval never
rolls it back) and test_tcp_input_paws_ooo_does_not_poison_hole_fill
(end-to-end: an OOO segment with a high TSval followed by the
in-order hole-filling segment with a lower TSval - the hole now
fills and both segments are delivered in order).
The loop's 'iplen > frame_len - ETH_HEADER_LEN' check re-derives the
exact bound the prologue already enforces ('frame_len <
ETH_HEADER_LEN + ip_len' -> return) from the same unmodified values,
so it can never fire. Remove it; keep the iplen assignment (still
used by the tcplen computation below).
Add test_tcp_process_ts_future_ecr_rejected: a segment echoing a
TSecr ahead of the local clock (ecr 5000 vs last_tick 1000) must be
rejected before the sample is computed. Without the guard,
(last_tick - ecr) underflows into a huge RTT sample that wrecks the
RTO; with it, tcp_process_ts returns -1 and the RTO state is left
untouched (rto_initialized/rtt/rto all zero).

Verified the test bites: with the guard temporarily removed, the
test fails (tcp_process_ts returns 0 instead of -1).
RFC 9293 3.10.7.3 examines the ACK bit first in SYN_SENT, for every
segment: if SEG.ACK <= ISS or SEG.ACK > SND.NXT, send
<SEQ=SEG.ACK><CTL=RST> (unless RST is set) and discard. wolfIP only
performed that check inside the 'flags == (SYN|ACK)' branch, so a
stray bare ACK (ACK set, SYN clear) with an unacceptable ACK number
was silently dropped instead of eliciting the required reset.

Hoist the check to before the SYN-flag gate, covering both stray
bare ACKs and SYN-ACKs with a bad ACK number; the sequence-range
form mirrors the existing RST-handler validation (SND.UNA <
SEG.ACK <= SND.NXT, which in SYN_SENT is exactly ISS+1 since no
data rides on the SYN). RST-bearing segments never reach the check:
the RST handler consumes them first, per the RFC caveat. The now
redundant inner check in the SYN|ACK branch is dropped.

Test: test_syn_sent_bad_ack_bare_ack_sends_rst - a bare ACK with an
unacceptable ACK number in SYN_SENT now elicits a RST and the socket
stays in SYN_SENT. Proven pre-fix: with the original code the
segment was dropped with no frame sent (last_frame_sent_size == 0).
RFC 7323 recommends that when generating an RST, if the segment
causing it contained a Timestamps option, the RST should also contain
one, with TSecr set to the incoming TSval and TSval set to zero, so
PAWS-aware peers can apply stricter RST acceptance checks.
tcp_send_reset_reply() built a bare 20-byte header and dropped the
option.

Parse the triggering segment's options (bounded by its validated
header length) and, when a Timestamps option is present, emit the RST
with that option: TSval = 0, TSecr = incoming TSval. The sender
switches from a fixed-size stack struct (data[0] - no room for
options) to a sized buffer, and the TCP length/checksum/eth/IP
lengths all account for the option. RSTs we receive are unaffected:
the RST handler consumes them before any timestamp processing, per
the RFC's MUST-NOT-update-state rule.

Test: test_rst_reply_carries_timestamp_when_incoming_had_one - a bad
ACK carrying TSval 43981 in SYN_SENT now elicits a RST whose options
carry TSval 0 and TSecr 43981. Proven pre-fix: the RST had no option
(rts->opt == 0).
The FRAG_NEEDED handler only applies a derived MSS when it lowers
peer_mss (peer_mss == 0 || new_mss < peer_mss), so a spoofed
FRAG_NEEDED advertising a larger next-hop MTU cannot re-inflate
peer_mss after a legitimate PMTU reduction. The existing tests only
exercise the reducing and below-floor cases, so the monotonic
decrease direction was unpinned: making the assignment
unconditional survived every existing assertion.

Add test_icmp_input_dest_unreach_frag_needed_larger_mtu_does_not_raise_peer_mss:
with peer_mss pre-reduced to 536, a FRAG_NEEDED with next-hop MTU
1500 (derived MSS 1460) must leave peer_mss at 536.

Proven: with the guard removed (unconditional assignment) the new
test is the only failure (peer_mss == 1460); all pre-existing
FRAG_NEEDED tests pass, confirming the mutation it pins.
RFC 2131: in RENEWING and REBINDING, a DHCPREQUEST with no response is
retransmitted after one-half the remaining time to T2 (RENEWING) or to
lease expiry (REBINDING), down to a minimum of 60 seconds. wolfIP
applied the generic 2 s exponential backoff (capped at T2 / lease
expiry) to both states, so a silent server got hammered every few
seconds instead of one polite retry per half-interval.

Add dhcp_renew_rebind_delay_ms() (one-half remaining, floored at 60 s,
capped at the remaining time so the retry never lands past the deadline
and the T2 / lease-expiry state transition still fires on time) and
dhcp_schedule_renew_rebind_retry(); the RENEWING/REBINDING arms of
dhcp_send_request() now use them. The non-renew/rebind path keeps the
RFC 4.1 backoff.

Tests: test_dhcp_renew_rebind_delay_ms (the formula, incl. the 60 s
floor and the remaining-time cap), test_dhcp_schedule_renew_rebind_retry
(the scheduler), and two end-to-end tests driving the state machine
(test_dhcp_renewing_retry_half_remaining_to_t2,
test_dhcp_rebinding_retry_half_remaining_to_lease). Proven pre-fix:
with the original backoff wiring both end-to-end tests fail (retry
scheduled at ~2 s instead of half the 200 s remaining).,
RFC 2131 4.4.1: if the client is unable to receive a unicast IP
datagram until its protocol software has been configured with an IP
address, it SHOULD set the BROADCAST bit (bit 15 of the flags field)
in the DHCP DISCOVER and REQUEST messages, so the server broadcasts
the DHCPOFFER/DHCPACK.

Set the bit in dhcp_send_discover (always - a discovering client has
no IP) and in the initial DHCPREQUEST of dhcp_send_request (the
!renewing && !rebinding case). RENEWING and REBINDING hold a bound IP
and expect a unicast reply, so they leave the bit clear.

The receive side already accepts broadcast replies: broadcast-
destination datagrams are marked local (never forwarded) and the
local_ip==0 DHCP socket matches any destination while DHCP is
running, so a server that honours the bit still reaches the client.

Tests: test_dhcp_discover_sets_broadcast_flag and
test_dhcp_request_broadcast_flag_by_state capture the transmitted
frame (via wolfIP_poll to flush the queued datagram) and assert the
BROADCAST bit is set for DISCOVER and the initial REQUEST, and clear
for RENEWING/REBINDING. Proven pre-fix: with the assignments removed
both tests fail (flags == 0).,
dhcp_parse_ack() wrote dhcp_server_ip, primary->ip, primary->mask,
primary->gw and dns_server directly into the live network-configuration
state as it walked the option TLV stream. A later validation failure
(malformed option, missing END, or the mandatory lease-time option
absent) returned -1 without restoring the fields already written, so a
rejected DHCPACK could leave a partially-applied, inconsistent
configuration (e.g. a new IP with the old mask, or a new gateway with
the old IP).

Parse the options into candidate values and commit them atomically,
only on the success path. The effective ip/mask fall back to the value
recorded during the OFFER phase when the ACK does not repeat the option
(preserving the prior behaviour, since the ACK is not required to).
The server-identity check still validates against the committed
server_ip; the DNS value is only committed when dns_server was unset.

Test: test_dhcp_parse_ack_reject_preserves_config feeds an ACK carrying
valid (but different) ip/mask/gw/dns options and no lease time -
rejected - and asserts every live field is unchanged. Proven pre-fix:
with the inline writes restored the test fails (primary->ip corrupted
to the offered value).,
With WOLFIP_ENABLE_FORWARDING, wolfIP_forward_packet() cast the routed
datagram directly to struct wolfIP_tcp_seg / wolfIP_udp_datagram /
wolfIP_icmp_packet and called the wolfIP_filter_notify_* functions.
Those structs embed a fixed 20-byte IPv4 header, so the notify
functions read the transport fields (ports / flags / type / code) at a
fixed ETH_HEADER_LEN + IP_HEADER_LEN offset regardless of the packet's
actual IHL.

The forwarding branch in ip_recv() runs before the local-dispatch block
that strips IP options and rewrites ver_ihl to 0x05, and a router must
relay the datagram with its options intact (stripping them here would
alter the relayed packet). So a forwarded datagram with IHL > 5 had its
transport metadata sampled from the option bytes: the registered L4
filter callback saw the wrong destination port / flags / type, which
could bypass an L4 forwarding rule. A header-only datagram with a large
IHL also let the fixed struct offsets extend past the received bytes
(stack over-read).

The three notify functions now take the actual ip_hlen, read the
transport header at ETH_HEADER_LEN + ip_hlen, and guard the read so a
header-only or truncated datagram does not over-read. All locally-built
and option-stripped call sites pass IP_HEADER_LEN; the forwarding path
computes and passes the real ip_hlen (covering both the immediate
ip_recv path and the ARP-deferred arp_flush_pending path).

Tests: test_filter_notify_udp_ihl_options_metadata feeds a UDP datagram
with a 24-byte IP header (IHL=6) and four option bytes and asserts the
ports come from the UDP header after the options;
test_filter_notify_udp_ihl_truncated_no_overread feeds a header-only
IHL=6 datagram and asserts the notify does not fire. Proven pre-fix:
with the fixed 20-byte offset restored the options test fails
(src_port read as 0x0101 from the option bytes instead of the real
port).,
The comment cited 'RFC 7126 section 3.8', which does not cover source
routing (RFC 7126 section 3 is 'General Security Implications of IP
Options' and has only 3.1). The source-route guidance is RFC 7126
4.3.5 (LSRR) and 4.4.5 (SSRR): an option-specific configuration knob
whose DEFAULT setting SHOULD be 'drop'. RFC 6274 adds that the security
implications outweigh any legitimate use.

The behaviour is unchanged (source-routed packets are always dropped,
the secure default); only the rationale comment is corrected to cite
the actual sections and to state that the unconditional drop is
deliberate hardening for a minimal embedded stack.,
…tion

The shared timer binheap (MAX_TIMERS = MAX_TCPSOCKETS*3) was filled with
lazy-cancel dead slots. timer_binheap_cancel marked a cancelled slot
expires==0 in place, never removing it; the slot still counted toward
heap->size and, worse, broke the min-heap invariant (a zeroed slot sorts
below its parent, so only dead slots that happened to sit at the root
were ever popped by the insert/expiry cleanup).

The TCP TX path re-arms the data RTO on every segment of a multi-segment
send (cancel the old RTO, insert a new one). Under an attacker holding
3+ connections with unacked data and zero windows, the victim's own
ordinary send bursts alone accumulated dead slots until size == MAX_
TIMERS, at which point timers_binheap_insert returned NO_TIMER and the
control-state armers stored 0 while setting the active flag - silently
dropping the RTO arms and sticking sockets in SYN_RCVD/FIN_WAIT_2 with
no repair path, until the socket table exhausted and the stack could
neither accept nor initiate any TCP connection (total, self-sustaining
TCP DoS).

timer_binheap_cancel now removes the slot physically (swap with the last
slot, size--, re-heapify via a new timers_heapify helper that sifts the
replaced element down then up), so heap->size tracks live timers and the
min-heap invariant always holds.

Tests: test_timer_heap_no_dead_slot_accumulation drives MAX_TIMERS*2
insert/cancel cycles (the RTO re-arm pattern) and asserts the heap ends
empty and a fresh insert still succeeds. Proven pre-fix: with the lazy
in-place mark restored the test fails (a dead slot remains, size != 0).
The eager cancel also corrects the heap ordering the lazy cancel had
corrupted: test_dhcp_lease_expiry_relearns_dns_server now drives the
lease expiry through handle_timers (popping the renew timer before the
callback, as in production) rather than calling dhcp_timer_cb directly,
which left an orphaned renew timer that the valid heap now surfaces.
test_cancel_timer, test_timer_cancel_existing_and_missing,
test_sock_close_tcp_cancels_rto_timer and test_tcp_last_ack_closes_
socket are re-aligned to the eager behaviour (the cancelled timer is
physically removed rather than left as an expires==0 slot).,
The F-6473 update gate compares the incoming TSval against last_ts
before storing it, but last_ts starts zeroed - a sentinel, not a
timestamp. A TSval in the upper half of the 32-bit space (the common
case for real kernel clocks) compares as "older" than zero, so the
SYN never seeded TS.Recent and every later segment failed
tcp_paws_check as an old duplicate, dropped with a bare ACK: the
handshake completed, then the connection stalled in a dup-ACK loop
with no data flow.

Add ts_recent_valid: the first timestamp after negotiation seeds
TS.Recent unconditionally and the RFC 7323 4.3 gate applies from the
second on; paws_check accepts while no reference is stored. The flag
resets in tcp_new_socket (also stops a stale last_ts from a reused
slot leaking into the next connection) and is copied with the other
negotiated state in the accept() clone.

Tests: test_tcp_input_paws_upper_half_tsval_flows - full listener
handshake with an upper-half SYN TSval (0xE0600D2C, mirroring a
captured CI failure) then first-data-segment delivery; fails on the
seed assertion pre-fix. Existing PAWS tests pre-seeding last_ts now
set ts_recent_valid, and the accept-clone test pins the flag copy.
esp_replay_check logged the uint32_t seq (and seq_low) with %d - a
varargs type mismatch (undefined behavior) that also prints sequence
numbers above 2^31 as negative. Use %lu with an explicit unsigned
long cast.
@danielinux danielinux assigned gasbytes and unassigned danielinux Aug 20, 2026
@danielinux
danielinux requested a review from gasbytes August 20, 2026 20:15
@gasbytes
gasbytes merged commit ceaefd3 into wolfSSL:master Aug 21, 2026
48 of 49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants