From e532c859dc2f04e6c180936f3656d0838df4ced4 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 11:45:37 +0200 Subject: [PATCH 01/35] F-9367: document the real cyw43_connect return contract 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. --- src/port/rp2350_cyw43439/cyw43439_driver.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/port/rp2350_cyw43439/cyw43439_driver.h b/src/port/rp2350_cyw43439/cyw43439_driver.h index 50205112..5d294abd 100644 --- a/src/port/rp2350_cyw43439/cyw43439_driver.h +++ b/src/port/rp2350_cyw43439/cyw43439_driver.h @@ -72,8 +72,11 @@ int cyw43_set_powersave(uint32_t pm); /* Initiate association to the named SSID. open_auth = 1 for an open * (non-RSN) network; for WPA2/WPA3 the call kicks off MLME and the * 4-way / SAE handshake runs in the wolfIP supplicant. Returns 0 once - * the (Re)Assoc Response arrives with a success code; the supplicant - * is responsible for finishing the handshake before traffic flows. + * the WLC_SET_SSID command is accepted - the (Re)Association completes + * asynchronously; poll cyw43_assoc_up() (or the latched + * cyw43_assoc_seen()) for the later WLC_E_ASSOC / WLC_E_LINK event, + * and the supplicant is responsible for finishing the handshake before + * traffic flows. * * bssid may be NULL (any matching SSID). channel = 0 means scan all. */ int cyw43_connect(const uint8_t *ssid, size_t ssid_len, From 3c021c6b50cdf9010a1f567b06eb427309833723 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 11:45:53 +0200 Subject: [PATCH 02/35] F-9368: stop claiming WPA3/SAE in cyw43_connect docs 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. --- src/port/rp2350_cyw43439/cyw43439_driver.h | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/port/rp2350_cyw43439/cyw43439_driver.h b/src/port/rp2350_cyw43439/cyw43439_driver.h index 5d294abd..96c77609 100644 --- a/src/port/rp2350_cyw43439/cyw43439_driver.h +++ b/src/port/rp2350_cyw43439/cyw43439_driver.h @@ -19,9 +19,10 @@ * channel registration. * * cyw43_connect() = WLC_SET_SSID + SET_KEY plumbing for an open or - * pre-shared assoc; the WPA{2,3} 4-way / SAE - * handshake itself runs in the wolfIP supplicant - * and we just shuttle EAPOL frames in/out. + * WPA2-PSK assoc only (no SAE/WPA3 on this path); + * the 4-way handshake itself runs in the wolfIP + * supplicant and we just shuttle EAPOL frames + * in/out. * * cyw43_tx_eapol() = push one EAPOL frame onto the F2 data channel * (BDC encapsulation, type 0x888E). @@ -70,8 +71,9 @@ int cyw43_wifi_up(const char *country); int cyw43_set_powersave(uint32_t pm); /* Initiate association to the named SSID. open_auth = 1 for an open - * (non-RSN) network; for WPA2/WPA3 the call kicks off MLME and the - * 4-way / SAE handshake runs in the wolfIP supplicant. Returns 0 once + * (non-RSN) network; for WPA2-PSK the call kicks off MLME (WPA2_AUTH_PSK + * + WPA2-PSK RSN IE, no AKM selector) and the 4-way handshake runs in the + * wolfIP supplicant. Returns 0 once * the WLC_SET_SSID command is accepted - the (Re)Association completes * asynchronously; poll cyw43_assoc_up() (or the latched * cyw43_assoc_seen()) for the later WLC_E_ASSOC / WLC_E_LINK event, From da02bdbb2165ec62ec90a0a1c7587d94da558bf2 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 11:46:17 +0200 Subject: [PATCH 03/35] F-9369: document when the cyw43_get_mac cache is valid 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. --- src/port/rp2350_cyw43439/cyw43439_driver.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/port/rp2350_cyw43439/cyw43439_driver.h b/src/port/rp2350_cyw43439/cyw43439_driver.h index 96c77609..b68e4e73 100644 --- a/src/port/rp2350_cyw43439/cyw43439_driver.h +++ b/src/port/rp2350_cyw43439/cyw43439_driver.h @@ -124,8 +124,11 @@ void cyw43_set_rx_callbacks(cyw43_eapol_cb_t eapol_cb, int cyw43_poll(void); -/* Read the radio's permanent MAC address (set during firmware load - * from OTP). out is 6 bytes. Returns 0 on success. */ +/* Read the radio's STA MAC address. The 6-byte cache is populated from + * the cur_etheraddr iovar read inside cyw43_wifi_up(), so the value is + * only valid after that call; before it the cache is zero-filled. out + * is 6 bytes. Returns 0 once firmware is up (cyw43_init done), -1 + * before that. */ int cyw43_get_mac(uint8_t out[6]); /* Read the associated AP's BSSID (learned during assoc). out is 6 From 9556686d15438cb5c98ad375196d0e143889415c Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 11:46:22 +0200 Subject: [PATCH 04/35] F-9370: correct rp2350_pio_xfer32 bit-count contract 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. --- src/port/rp2350_cyw43439/rp2350_pio.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/port/rp2350_cyw43439/rp2350_pio.h b/src/port/rp2350_cyw43439/rp2350_pio.h index 5ed1286f..7d59785a 100644 --- a/src/port/rp2350_cyw43439/rp2350_pio.h +++ b/src/port/rp2350_cyw43439/rp2350_pio.h @@ -30,7 +30,9 @@ void rp2350_pio_init(void); * (MSB-first, already byte-permuted for the bus mode) are clocked out * (out_bits total), then in_bits are clocked in and returned in `rx` * (one 32-bit word, MSB-first). CS is asserted/deasserted by the - * caller. out_bits and in_bits are each 1..32 for register access. */ + * caller. out_bits and in_bits must both be exactly 32: the PIO OSR + * is drained per full word, so sub-word counts desynchronize the state + * machine (see rp2350_pio_xfer). */ uint32_t rp2350_pio_xfer32(uint32_t cmd_word, uint32_t out_bits, uint32_t in_bits); From 8f3b60e3f77b5bf47aaae021724178db81d463b8 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 11:46:35 +0200 Subject: [PATCH 05/35] F-9371: describe what rp2350_spi_init actually configures 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. --- src/port/rp2350_cyw43439/rp2350_spi.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/port/rp2350_cyw43439/rp2350_spi.h b/src/port/rp2350_cyw43439/rp2350_spi.h index 6f2522ce..57fc0d73 100644 --- a/src/port/rp2350_cyw43439/rp2350_spi.h +++ b/src/port/rp2350_cyw43439/rp2350_spi.h @@ -20,10 +20,12 @@ extern "C" { #endif -/* Initialise the GPIO and SPI peripheral for the CYW43439 gSPI bus. - * Pin assignment lives in board.h (CYW43_PIN_SPI_*); this function - * configures pad strength, function-mux, clock divider, and brings the - * SPI controller out of reset. Safe to call multiple times. */ +/* Initialise the CPU-driven control lines for the CYW43439 gSPI bus: + * CS deasserted and WL_REG_ON driven low (radio held off). Pin + * assignment lives in board.h (CYW43_PIN_*). CLK and DATA are owned by + * the PIO transport (rp2350_pio_init), not this function. Call once, + * before power-up: repeating it while the radio is running drives + * WL_REG_ON low and powers the CYW43439 down. */ void rp2350_spi_init(void); /* Drive WL_REG_ON high to power the CYW43439. Caller should wait From c148fe3acf2180e69be76450480fcdee045ed5a2 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 11:48:00 +0200 Subject: [PATCH 06/35] F-9372: document that the HAL port forces RMII 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. --- src/port/stm32_hal/stm32_hal_eth.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/port/stm32_hal/stm32_hal_eth.h b/src/port/stm32_hal/stm32_hal_eth.h index afe3a3d4..2fe24168 100644 --- a/src/port/stm32_hal/stm32_hal_eth.h +++ b/src/port/stm32_hal/stm32_hal_eth.h @@ -59,7 +59,7 @@ * * // Initialize wolfIP (same code on ALL STM32 boards) * wolfIP_init_static(&ipstack); - * stm32_hal_eth_init(wolfIP_getdev(ipstack)); // Auto-configures RMII/MII! + * stm32_hal_eth_init(wolfIP_getdev(ipstack)); // Auto-configures RMII! * wolfIP_ipconfig_set(ipstack, * atoip4("192.168.1.100"), * atoip4("255.255.255.0"), @@ -88,11 +88,12 @@ * * The driver automatically: * 1. Detects your STM32 family at compile time - * 2. Configures RMII/MII interface (SBS/SYSCFG) + * 2. Configures the RMII interface (SBS/SYSCFG) - MII boards must be + * configured manually in HAL_ETH_MspInit(); there is no mode option * 3. Reinitializes ETH with correct settings * 4. Starts the MAC in interrupt mode * - * No manual MspInit changes required for supported families! + * No manual MspInit changes required for supported RMII families! * * ## Implementation Notes * From 9c92b47f36dff7b19d15005a15c9a1d174a71476 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 11:48:11 +0200 Subject: [PATCH 07/35] F-9373: soften the ETH NVIC prerequisite in the HAL docs 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. --- src/port/stm32_hal/stm32_hal_eth.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/port/stm32_hal/stm32_hal_eth.h b/src/port/stm32_hal/stm32_hal_eth.h index 2fe24168..ac0916fd 100644 --- a/src/port/stm32_hal/stm32_hal_eth.h +++ b/src/port/stm32_hal/stm32_hal_eth.h @@ -78,8 +78,9 @@ * - Mode: RMII (or MII depending on your board's PHY) * * 2. **NVIC Settings** (System Core -> NVIC) - CRITICAL - * - ETH global interrupt: **ENABLED** - * - Without this, received frames will not be detected + * - ETH global interrupt: **ENABLED** for prompt frame notification + * - Without it, frames are still detected by the periodic RX poll + * (every 100th poll) but with added latency * * 3. **GPIO Configuration** * - CubeMX auto-configures correct pins for NUCLEO boards From 26a64d397d7c9b87e62ca9233250f78b8b413ade Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 11:50:10 +0200 Subject: [PATCH 08/35] F-9375: correct the ap_rsn_ie NULL-fallback comment 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. --- src/supplicant/supplicant.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/supplicant/supplicant.h b/src/supplicant/supplicant.h index c1a97ee4..3b5c5ff1 100644 --- a/src/supplicant/supplicant.h +++ b/src/supplicant/supplicant.h @@ -193,10 +193,11 @@ struct wolfip_supplicant_cfg { * M3 to detect downgrade attacks (IEEE 802.11-2020 12.7.6.4). * * If ap_rsn_ie is NULL, the supplicant falls back to using its own - * default WPA2-PSK RSN IE for the comparison. This is acceptable - * for a closed PSK deployment where supplicant and AP agree on - * cipher choices by configuration, but real hardware ports should - * pass the IE from the chip's scan results. + * generated RSN IE for the comparison - the AKM suite patched to + * the configured mode (PSK, SAE or 802.1X), not always WPA2-PSK. + * This is acceptable for a closed deployment where supplicant and + * AP agree on cipher choices by configuration, but real hardware + * ports should pass the IE from the chip's scan results. */ const uint8_t *ap_rsn_ie; size_t ap_rsn_ie_len; From bf889f8beaa25cf7ff65ec166893fe41cb377564 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 11:50:20 +0200 Subject: [PATCH 09/35] F-9376: require zero-init of the context before first supplicant init 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. --- src/supplicant/supplicant.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/supplicant/supplicant.h b/src/supplicant/supplicant.h index 3b5c5ff1..f7100ceb 100644 --- a/src/supplicant/supplicant.h +++ b/src/supplicant/supplicant.h @@ -269,8 +269,12 @@ struct wolfip_supplicant { * context zero but is wiped by _deinit() and _pmksa_clear(). * * Reuse requires BOTH pmksa_magic == WOLFIP_PMKSA_MAGIC AND an exact - * SSID match, so an uninitialized (garbage) context on the very first - * init cannot be mistaken for a valid cache. */ + * SSID match, so a garbage value cannot be mistaken for a valid + * cache hit. The first init still READS these fields (PMKSA + * snapshot) before zeroing the context, so caller-owned (stack / + * static / pool) contexts MUST be zero-initialized (memset / + * "= {0}") before their very first wolfip_supplicant_init(); only + * re-inits on a previously valid context may carry PMKSA state. */ uint32_t pmksa_magic; uint8_t pmksa_pmk[WPA_PMK_LEN]; uint8_t pmksa_pmkid[16]; /* PMKID of the cached PMKSA (for reconnect) */ From 6e176da9ae62059e64846517a87f11605d9ff83b Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 11:50:30 +0200 Subject: [PATCH 10/35] F-9377: document which init failures zero the context 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. --- src/supplicant/supplicant.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/supplicant/supplicant.h b/src/supplicant/supplicant.h index f7100ceb..4f828c94 100644 --- a/src/supplicant/supplicant.h +++ b/src/supplicant/supplicant.h @@ -306,8 +306,11 @@ extern "C" { /* Caller-allocated init. `out` is a struct provided by the caller (stack, * static, or pool) and is fully populated from cfg on success. Returns 0 - * on success, negative on bad args / crypto failure. On failure, the - * struct is left zeroed; caller does not need to call _deinit. + * on success, negative on bad args / crypto failure. On failure after + * initialization has begun, the struct is left zeroed and the caller + * does not need to call _deinit; bad-argument and cfg-validation + * failures return before any write and leave the supplied context + * unchanged. */ int wolfip_supplicant_init(struct wolfip_supplicant *out, const struct wolfip_supplicant_cfg *cfg); From b36b1a0be683b6d4fd2dd5259e30d4a54e0ccaf8 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 11:50:42 +0200 Subject: [PATCH 11/35] F-9378: document the mode-specific wolfip_supplicant_kick transitions 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. --- src/supplicant/supplicant.h | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/supplicant/supplicant.h b/src/supplicant/supplicant.h index 4f828c94..624b72e4 100644 --- a/src/supplicant/supplicant.h +++ b/src/supplicant/supplicant.h @@ -330,9 +330,16 @@ struct wolfip_supplicant *wolfip_supplicant_new( void wolfip_supplicant_free(struct wolfip_supplicant *s); -/* Signal that the radio reports "associated" - supplicant moves from - * IDLE to 4WAY_M1_WAIT. (On real hardware, called by the driver after - * the FullMAC chip completes auth+assoc.) `now_ms` is the current +/* Start the handshake from IDLE. The transition depends on the auth + * mode: + * PSK (or SAE with a pre-installed FullMAC PMK): IDLE -> 4WAY_M1_WAIT. + * EAP (TLS / PEAP): sends EAPOL-Start, IDLE -> EAP_IDENTITY_WAIT. + * Software SAE: sends the SAE Commit, IDLE -> SAE_COMMIT_SENT. SAE + * authentication runs BEFORE radio association, so software-SAE + * callers must kick the supplicant before the driver associates - + * waiting for the association inverts the required sequencing. + * (On FullMAC hardware where the chip did auth itself, the driver calls + * this after the chip completes auth+assoc.) `now_ms` is the current * monotonic timestamp; the supplicant uses it as the handshake start. */ int wolfip_supplicant_kick(struct wolfip_supplicant *s, uint64_t now_ms); From 1f00ca3b1a5e6559b41e2baab54e7a647997cdf7 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 11:50:54 +0200 Subject: [PATCH 12/35] F-9379: document that get_pmk reports PMK availability, not success 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. --- src/supplicant/supplicant.h | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/supplicant/supplicant.h b/src/supplicant/supplicant.h index 624b72e4..877ce0ed 100644 --- a/src/supplicant/supplicant.h +++ b/src/supplicant/supplicant.h @@ -383,10 +383,16 @@ const uint8_t *wolfip_supplicant_kck(const struct wolfip_supplicant *s); const uint8_t *wolfip_supplicant_tk (const struct wolfip_supplicant *s); const uint8_t *wolfip_supplicant_snonce(const struct wolfip_supplicant *s); -/* Export the current PMK (32 bytes). Returns 0 on success, -1 if no - * PMK is available (state == IDLE / FAILED, or auth_mode never derived - * a PSK-grade PMK). Caller can persist the PMK and pass it back via - * cfg.psk_pmk on the next wolfip_supplicant_init() to skip PBKDF2. */ +/* Export the current PMK (32 bytes). Reports PMK-material availability, + * NOT authentication success: PSK mode holds a PMK right after init, and + * a context with a PTK or an installed (pmk_installed) SAE PMK exports + * it from every state, FAILED included - in FAILED state a PMK that was + * never derived is exported as all zeros with rc 0. Returns -1 only + * while IDLE with no derived/installed PMK (EAP / software SAE before + * the handshake). Callers that need "did authentication complete" must + * check wolfip_supplicant_state() separately. Caller can persist the PMK + * and pass it back via cfg.psk_pmk on the next wolfip_supplicant_init() + * to skip PBKDF2. */ int wolfip_supplicant_get_pmk(const struct wolfip_supplicant *s, uint8_t out_pmk[WPA_PMK_LEN]); From 2f1b3995e205f9aa8c4ebe8b6f0b6d17e2447742 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 12:00:06 +0200 Subject: [PATCH 13/35] F-10264: fix the gmac enum name in the sa_new_gcm comment 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. --- src/wolfesp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wolfesp.c b/src/wolfesp.c index 4cf10550..1d36d8bb 100644 --- a/src/wolfesp.c +++ b/src/wolfesp.c @@ -196,7 +196,7 @@ esp_spi_valid(const uint8_t * spi, int log_it) } /* Configure a new Security Association based on either - * enc = ESP_ENC_GCM_RFC4106 (gcm), or enc = ESP_AUTH_GCM_RFC4543 (gmac). + * enc = ESP_ENC_GCM_RFC4106 (gcm), or enc = ESP_ENC_GCM_RFC4543 (gmac). * */ int wolfIP_esp_sa_new_gcm(int in, uint8_t * spi, ip4 src, ip4 dst, esp_enc_t enc, uint8_t * enc_key, From a15ea21e47ec21ebee5ba162fc86472946b1ef99 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 12:11:08 +0200 Subject: [PATCH 14/35] F-8572: skip anti-replay for SAs without integrity protection 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. --- src/test/unit/unit_esp.c | 61 ++++++++++++++++++++++++++++++++++++++++ src/wolfesp.c | 23 +++++++++++---- 2 files changed, 78 insertions(+), 6 deletions(-) diff --git a/src/test/unit/unit_esp.c b/src/test/unit/unit_esp.c index d36f3395..f0f17e6e 100644 --- a/src/test/unit/unit_esp.c +++ b/src/test/unit/unit_esp.c @@ -715,6 +715,66 @@ START_TEST(test_replay_aead_verify) } END_TEST +/* RFC 4303 s3.4.3: anti-replay must not be enabled for SAs without + * integrity. For a cipher-only SA (icv_len == 0) unwrap must neither + * consult nor update the replay window: even a window state that would + * reject the incoming seq must not drop the packet. */ +START_TEST(test_unwrap_cipher_only_sa_skips_replay_window) +{ + static uint8_t buf[LINK_MTU + 256]; + uint8_t ref[64]; + uint32_t frame_len; + uint16_t ip_len; + struct wolfIP_ip_packet *ip = (struct wolfIP_ip_packet *)buf; + wolfIP_esp_sa * esp_sa = NULL; + int ret; + uint32_t i; + + for (i = 0U; i < sizeof(ref); i++) { + ref[i] = (uint8_t)(i & 0xFFU); + } + + esp_setup(); + + /* Cipher-only SAs: no integrity, icv_len == 0. */ + ret = wolfIP_esp_sa_new_cbc_hmac(0, (uint8_t *)spi_rt, + atoip4(T_SRC), atoip4(T_DST), + (uint8_t *)k_aes128, sizeof(k_aes128), + ESP_AUTH_NONE, NULL, 0, 0); + ck_assert_int_eq(ret, 0); + ret = wolfIP_esp_sa_new_cbc_hmac(1, (uint8_t *)spi_rt, + atoip4(T_SRC), atoip4(T_DST), + (uint8_t *)k_aes128, sizeof(k_aes128), + ESP_AUTH_NONE, NULL, 0, 0); + ck_assert_int_eq(ret, 0); + + esp_sa = esp_sa_get(1, (uint8_t *)spi_rt); + ck_assert_ptr_nonnull(esp_sa); + + /* Poison the inbound window: with hi_seq = 1000 the seq about to + * arrive (1) is below the window floor, so a replay check would + * reject it. */ + esp_sa->replay.hi_seq = 1000U; + + frame_len = build_ip_packet(buf, sizeof(buf), WI_IPPROTO_UDP, + ref, sizeof(ref)); + ip_len = (uint16_t)(frame_len - ETH_HEADER_LEN); + + ret = esp_transport_wrap(ip, &ip_len); + ck_assert_int_eq(ret, 0); + frame_len = (uint32_t)ip_len + ETH_HEADER_LEN; + + /* Unwrap must succeed despite the poisoned window: no integrity + * means no anti-replay (RFC 4303 s3.4.3). */ + ret = esp_transport_unwrap(ip, &frame_len); + ck_assert_int_eq(ret, 0); + + /* The window must be left untouched (no commit for ICV-less SAs). */ + ck_assert_uint_eq(esp_sa->replay.hi_seq, 1000U); + ck_assert_uint_eq(esp_sa->replay.bitmap, 0U); +} +END_TEST + /* * esp_transport_unwrap error paths */ @@ -2452,6 +2512,7 @@ static Suite *esp_suite(void) tcase_add_test(tc, test_replay_overflow); tcase_add_test(tc, test_replay_aead_verify); tcase_add_test(tc, test_regression_replay_window_not_updated_before_icv); + tcase_add_test(tc, test_unwrap_cipher_only_sa_skips_replay_window); suite_add_tcase(s, tc); /* Unwrap error paths */ diff --git a/src/wolfesp.c b/src/wolfesp.c index 1d36d8bb..ce242308 100644 --- a/src/wolfesp.c +++ b/src/wolfesp.c @@ -1488,9 +1488,17 @@ esp_transport_unwrap(struct wolfIP_ip_packet *ip, uint32_t * frame_len) return -1; } - err = esp_replay_check(&esp_sa->replay, seq); - if (err) { - return -1; + /* RFC 4303 s3.4.3: anti-replay MUST NOT be enabled unless the SA also + * provides integrity. A cipher-only SA (icv_len == 0) leaves the + * Sequence Number field unprotected, so the window must neither be + * consulted nor advanced for it: it would protect nothing, and an + * attacker able to rewrite the sequence could advance the window and + * drop legitimate packets. */ + if (esp_sa->icv_len) { + err = esp_replay_check(&esp_sa->replay, seq); + if (err) { + return -1; + } } iv_len = esp_iv_len_from_enc(esp_sa->enc); @@ -1575,9 +1583,12 @@ esp_transport_unwrap(struct wolfIP_ip_packet *ip, uint32_t * frame_len) } /* icv verified for hmacs and aeads at this point. now safe to commit the - * sequence to the replay window (RFC 4303 s3.4.3). */ - esp_replay_commit(&esp_sa->replay, seq); - esp_state_save(esp_sa); + * sequence to the replay window (RFC 4303 s3.4.3). Cipher-only SAs + * (icv_len == 0) keep the window untouched, matching the check above. */ + if (esp_sa->icv_len) { + esp_replay_commit(&esp_sa->replay, seq); + esp_state_save(esp_sa); + } /* Payload is now verified and decrypted. We can now parse * the ESP trailer for next header and pad_len. */ From 78ba53b3ec36d4166d740612c24fda77d208ccac Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 12:16:11 +0200 Subject: [PATCH 15/35] F-8573: widen the ESP anti-replay window to 64 packets 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. --- src/test/unit/unit_esp.c | 30 +++++++++++++++--------------- src/wolfesp.c | 16 ++++++++-------- wolfesp.h | 9 +++++---- 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/src/test/unit/unit_esp.c b/src/test/unit/unit_esp.c index f0f17e6e..f381583a 100644 --- a/src/test/unit/unit_esp.c +++ b/src/test/unit/unit_esp.c @@ -481,7 +481,7 @@ END_TEST START_TEST(test_replay_first_packet_accepted) { replay_t r; - esp_replay_init(r); /* hi_seq=32, seq_low=1 */ + esp_replay_init(r); /* hi_seq=64, seq_low=1 */ ck_assert_int_eq(esp_replay_check(&r, 1U), 0); } END_TEST @@ -503,7 +503,7 @@ START_TEST(test_replay_multiple_in_window) { replay_t r; uint32_t i; - esp_replay_init(r); /* window [1..32] */ + esp_replay_init(r); /* window [1..64] */ for (i = 1U; i <= 31U; i++) { ck_assert_int_eq(esp_replay_check(&r, i), 0); esp_replay_commit(&r, i); @@ -517,8 +517,8 @@ START_TEST(test_replay_below_window_rejected) replay_t r; esp_replay_init(r); /* Advance the window by receiving a high sequence number. */ - ck_assert_int_eq(esp_replay_check(&r, 64U), 0); - esp_replay_commit(&r, 64U); /* hi_seq=64, seq_low=34 */ + ck_assert_int_eq(esp_replay_check(&r, 100U), 0); + esp_replay_commit(&r, 100U); /* hi_seq=100, seq_low=37 */ /* seq=1 is now below the window floor. */ ck_assert_int_ne(esp_replay_check(&r, 1U), 0); } @@ -528,10 +528,10 @@ END_TEST START_TEST(test_replay_advance_hi_seq) { replay_t r; - esp_replay_init(r); /* hi_seq=32 */ - ck_assert_int_eq(esp_replay_check(&r, 33U), 0); - esp_replay_commit(&r, 33U); - ck_assert_uint_eq(r.hi_seq, 33U); + esp_replay_init(r); /* hi_seq=64 */ + ck_assert_int_eq(esp_replay_check(&r, 65U), 0); + esp_replay_commit(&r, 65U); + ck_assert_uint_eq(r.hi_seq, 65U); } END_TEST @@ -539,10 +539,10 @@ END_TEST START_TEST(test_replay_advanced_hi_seq_duplicate_rejected) { replay_t r; - esp_replay_init(r); /* hi_seq=32 */ - ck_assert_int_eq(esp_replay_check(&r, 33U), 0); - esp_replay_commit(&r, 33U); - ck_assert_int_ne(esp_replay_check(&r, 33U), 0); + esp_replay_init(r); /* hi_seq=64 */ + ck_assert_int_eq(esp_replay_check(&r, 65U), 0); + esp_replay_commit(&r, 65U); + ck_assert_int_ne(esp_replay_check(&r, 65U), 0); } END_TEST @@ -567,7 +567,7 @@ START_TEST(test_replay_jump_resets_bitmap) esp_replay_commit(&r, 1U); ck_assert_int_eq(esp_replay_check(&r, 2U), 0); esp_replay_commit(&r, 2U); - /* Jump more than ESP_REPLAY_WIN (32) ahead. */ + /* Jump more than ESP_REPLAY_WIN (64) ahead. */ ck_assert_int_eq(esp_replay_check(&r, 1000U), 0); esp_replay_commit(&r, 1000U); ck_assert_uint_eq(r.hi_seq, 1000U); @@ -585,8 +585,8 @@ START_TEST(test_replay_old_seqs_after_jump) ck_assert_int_eq(esp_replay_check(&r, 10U), 0); esp_replay_commit(&r, 10U); ck_assert_int_eq(esp_replay_check(&r, 500U), 0); - esp_replay_commit(&r, 500U); /* jump > 32 */ - /* 10 is now well below the new window floor (500-31=469). */ + esp_replay_commit(&r, 500U); /* jump > 64 */ + /* 10 is now well below the new window floor (500-63=437). */ ck_assert_int_ne(esp_replay_check(&r, 10U), 0); } END_TEST diff --git a/src/wolfesp.c b/src/wolfesp.c index ce242308..35a05e85 100644 --- a/src/wolfesp.c +++ b/src/wolfesp.c @@ -1311,7 +1311,7 @@ esp_replay_check(const struct replay_t * replay, uint32_t seq) (void)replay; (void)seq; #else - uint32_t bitn = 0; + uint64_t bitn = 0; uint32_t seq_low = 1U; if (seq == 0) { @@ -1328,16 +1328,16 @@ esp_replay_check(const struct replay_t * replay, uint32_t seq) return -1; } - /* Simple 32 bit replay window: + /* Sliding replay window: * seq_low - - - - - - - seq - - - - - - hi_seq * |<----------- ESP_REPLAY_WIN --------------| * */ if (seq <= replay->hi_seq) { /* seq number within window. */ - bitn = 1U << (replay->hi_seq - seq); + bitn = 1ULL << (replay->hi_seq - seq); - if ((replay->bitmap & bitn) != 0U) { - ESP_LOG("error: seq replayed: %u, %d\n", bitn, seq); + if ((replay->bitmap & bitn) != 0ULL) { + ESP_LOG("error: seq replayed: %d\n", seq); return -1; } } @@ -1361,16 +1361,16 @@ esp_replay_commit(struct replay_t * replay, uint32_t seq) if (seq <= replay->hi_seq) { /* Within window: mark the bit. */ - replay->bitmap |= 1U << (replay->hi_seq - seq); + replay->bitmap |= 1ULL << (replay->hi_seq - seq); } else { /* Above window: slide up. */ diff = seq - replay->hi_seq; if (diff < ESP_REPLAY_WIN) { - replay->bitmap = (replay->bitmap << diff) | 1U; + replay->bitmap = (replay->bitmap << diff) | 1ULL; } else { - replay->bitmap = 1; + replay->bitmap = 1ULL; } replay->hi_seq = seq; } diff --git a/wolfesp.h b/wolfesp.h index 1d316715..b8e8c9ee 100644 --- a/wolfesp.h +++ b/wolfesp.h @@ -53,12 +53,13 @@ typedef enum { ESP_AUTH_GCM_RFC4543 /* rfc4543 gmac */ } esp_auth_t; -/* simple static 32 bit replay window */ +/* simple static sliding replay window; RFC 4301 s3.3.1 recommends a + * window of at least 64 packets */ #define ESP_MAX_32_SEQ 0xffffffffUL -#define ESP_REPLAY_WIN 32U +#define ESP_REPLAY_WIN 64U struct replay_t { - uint32_t bitmap; /* inbound sequence bitmap */ + uint64_t bitmap; /* inbound sequence bitmap */ uint32_t hi_seq; /* inbound high sequence number */ uint32_t oseq; /* outbound sequence number */ }; @@ -67,7 +68,7 @@ typedef struct replay_t replay_t; #define esp_replay_init(r) \ do { \ - (r).bitmap = 0U; \ + (r).bitmap = 0ULL; \ (r).hi_seq = ESP_REPLAY_WIN; \ (r).oseq = 0U; \ } while (0) From f0b4aa9ff75ee6e36d8da37894a6f3f99071fbe8 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 12:16:30 +0200 Subject: [PATCH 16/35] F-8519: do not free the Hmac when wc_HmacInit failed 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. --- src/wolfesp.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/wolfesp.c b/src/wolfesp.c index 35a05e85..205cb757 100644 --- a/src/wolfesp.c +++ b/src/wolfesp.c @@ -652,6 +652,7 @@ esp_calc_icv_hmac(uint8_t * hash, const wolfIP_esp_sa * esp_sa, int err = 0; int type = 0; uint32_t auth_len = esp_len; + uint8_t inited = 0; switch (esp_sa->auth) { case ESP_AUTH_MD5_RFC2403: @@ -674,9 +675,10 @@ esp_calc_icv_hmac(uint8_t * hash, const wolfIP_esp_sa * esp_sa, err = wc_HmacInit(&hmac, NULL, INVALID_DEVID); if (err) { - ESP_LOG("error: wc_HmacSetKey: %d\n", err); - goto calc_icv_hmac_end; + ESP_LOG("error: wc_HmacInit: %d\n", err); + return err; } + inited = 1; err = wc_HmacSetKey(&hmac, type, esp_sa->auth_key, esp_sa->auth_key_len); if (err) { @@ -699,7 +701,11 @@ esp_calc_icv_hmac(uint8_t * hash, const wolfIP_esp_sa * esp_sa, } calc_icv_hmac_end: - wc_HmacFree(&hmac); + /* Free only after a successful init: freeing an Hmac whose + * wc_HmacInit() failed would release state that was never set up. */ + if (inited) { + wc_HmacFree(&hmac); + } return err; } From 7a5edaf05f896d9082a3a2ca8eba0749f72b391f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 12:18:05 +0200 Subject: [PATCH 17/35] F-10262: zeroize the full GCM nonce in the four ESP helpers 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. --- src/wolfesp.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/wolfesp.c b/src/wolfesp.c index 205cb757..c3f7d842 100644 --- a/src/wolfesp.c +++ b/src/wolfesp.c @@ -1077,7 +1077,7 @@ esp_aes_rfc4106_dec(const wolfIP_esp_sa * esp_sa, uint8_t * esp_data, } rfc4106_dec_out: - wc_ForceZero(nonce, salt_len); + wc_ForceZero(nonce, sizeof(nonce)); if (inited) { wc_AesFree(&gcm_dec); inited = 0; @@ -1150,7 +1150,7 @@ esp_aes_rfc4106_enc(const wolfIP_esp_sa * esp_sa, uint8_t * esp_data, } rfc4106_enc_out: - wc_ForceZero(nonce, salt_len); + wc_ForceZero(nonce, sizeof(nonce)); if (inited) { wc_AesFree(&gcm_enc); inited = 0; @@ -1204,7 +1204,7 @@ esp_aes_rfc4543_dec(const wolfIP_esp_sa * esp_sa, uint8_t * esp_data, } rfc4543_dec_out: - wc_ForceZero(nonce, salt_len); + wc_ForceZero(nonce, sizeof(nonce)); return err; } @@ -1260,7 +1260,7 @@ esp_aes_rfc4543_enc(const wolfIP_esp_sa * esp_sa, uint8_t * esp_data, } rfc4543_enc_out: - wc_ForceZero(nonce, salt_len); + wc_ForceZero(nonce, sizeof(nonce)); if (inited) { wc_AesFree(&gmac_enc.aes); inited = 0; From 764b0bb31a7ca91798047e3ecbddcbb5cab4bb67 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 12:19:10 +0200 Subject: [PATCH 18/35] F-10263: drop the redundant second AES-GCM key set 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. --- src/wolfesp.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/wolfesp.c b/src/wolfesp.c index c3f7d842..c1a7a3f6 100644 --- a/src/wolfesp.c +++ b/src/wolfesp.c @@ -1055,7 +1055,9 @@ esp_aes_rfc4106_dec(const wolfIP_esp_sa * esp_sa, uint8_t * esp_data, } inited = 1; - /* subtract 4 byte salt from enc_key_len */ + /* subtract 4 byte salt from enc_key_len. wc_AesGcmInit installs the + * key itself (it calls wc_AesGcmSetKey internally), so no second + * key set is needed before the one-shot decrypt. */ err = wc_AesGcmInit(&gcm_dec, esp_sa->enc_key, esp_sa->enc_key_len - 4, nonce, sizeof(nonce)); if (err != 0) { @@ -1063,12 +1065,6 @@ esp_aes_rfc4106_dec(const wolfIP_esp_sa * esp_sa, uint8_t * esp_data, goto rfc4106_dec_out; } - err = wc_AesGcmSetKey(&gcm_dec, esp_sa->enc_key, esp_sa->enc_key_len - 4); - if (err != 0) { - ESP_LOG("error: wc_AesGcmSetKey: %d\n", err); - goto rfc4106_dec_out; - } - err = wc_AesGcmDecrypt(&gcm_dec, enc_payload, enc_payload, enc_len, nonce, sizeof(nonce), icv, icv_len, aad, aad_len); if (err != 0) { @@ -1128,7 +1124,9 @@ esp_aes_rfc4106_enc(const wolfIP_esp_sa * esp_sa, uint8_t * esp_data, } inited = 1; - /* subtract 4 byte salt from enc_key_len */ + /* subtract 4 byte salt from enc_key_len. wc_AesGcmInit installs the + * key itself (it calls wc_AesGcmSetKey internally), so no second + * key set is needed before the one-shot encrypt. */ err = wc_AesGcmInit(&gcm_enc, esp_sa->enc_key, esp_sa->enc_key_len - 4, nonce, sizeof(nonce)); if (err != 0) { @@ -1136,12 +1134,6 @@ esp_aes_rfc4106_enc(const wolfIP_esp_sa * esp_sa, uint8_t * esp_data, goto rfc4106_enc_out; } - err = wc_AesGcmSetKey(&gcm_enc, esp_sa->enc_key, esp_sa->enc_key_len - 4); - if (err != 0) { - ESP_LOG("error: wc_AesGcmSetKey: %d\n", err); - goto rfc4106_enc_out; - } - err = wc_AesGcmEncrypt(&gcm_enc, enc_payload, enc_payload, enc_len, nonce, sizeof(nonce), icv, icv_len, aad, aad_len); if (err != 0) { From 920af46056fe15401f4c2dee057861060a3f649f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 13:07:29 +0200 Subject: [PATCH 19/35] F-6939: zero echo reply code per RFC 792 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. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_dns_dhcp.c | 36 +++++++++++++++++++++++++++++ src/wolfip.c | 3 +++ 3 files changed, 40 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 3b6d5d76..776aa4aa 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -908,6 +908,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_icmp_input_echo_reply_wrong_dst_dropped); tcase_add_test(tc_proto, test_icmp_input_echo_request_reply_sent); tcase_add_test(tc_proto, test_icmp_input_echo_reply_sets_df); + tcase_add_test(tc_proto, test_icmp_echo_reply_code_zeroed); tcase_add_test(tc_proto, test_icmp_input_echo_request_bad_checksum_dropped); tcase_add_test(tc_proto, test_icmp_input_echo_request_odd_len_reply_checksum); tcase_add_test(tc_proto, test_icmp_input_echo_request_dhcp_running_no_reply); diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index 0e67b59a..20afe39c 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -2041,6 +2041,42 @@ START_TEST(test_icmp_input_echo_reply_sets_df) } END_TEST +START_TEST(test_icmp_echo_reply_code_zeroed) +{ + struct wolfIP s; + struct wolfIP_icmp_packet icmp; + struct wolfIP_icmp_packet *reply; + uint32_t frame_len; + + wolfIP_init(&s); + mock_link_init(&s); + s.dhcp_state = DHCP_OFF; + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + wolfIP_filter_set_callback(NULL, NULL); + last_frame_sent_size = 0; + + /* a non-zero code on the request is malformed per RFC 792, but the + * reply must not propagate it - echo reply code is always 0 */ + memset(&icmp, 0, sizeof(icmp)); + icmp.ip.src = ee32(0x0A000002U); + icmp.ip.dst = ee32(0x0A000001U); + icmp.ip.ttl = 64; + icmp.ip.len = ee16(IP_HEADER_LEN + ICMP_HEADER_LEN); + icmp.type = ICMP_ECHO_REQUEST; + icmp.code = 5; + icmp.csum = ee16(icmp_checksum(&icmp, ICMP_HEADER_LEN)); + frame_len = (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + ICMP_HEADER_LEN); + + icmp_input(&s, TEST_PRIMARY_IF, (struct wolfIP_ip_packet *)&icmp, frame_len); + ck_assert_uint_gt(last_frame_sent_size, 0); + reply = (struct wolfIP_icmp_packet *)last_frame_sent; + ck_assert_uint_eq(reply->type, ICMP_ECHO_REPLY); + ck_assert_uint_eq(reply->code, 0U); + /* reply checksum must cover the corrected type/code pair */ + ck_assert_uint_eq(icmp_checksum(reply, ICMP_HEADER_LEN), 0U); +} +END_TEST + START_TEST(test_icmp_input_echo_request_bad_checksum_dropped) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 9be2492c..1d7666d8 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -8060,6 +8060,9 @@ static void icmp_input(struct wolfIP *s, unsigned int if_idx, struct wolfIP_ip_p if (!dst_match) return; icmp->type = ICMP_ECHO_REPLY; + /* RFC 792: echo reply code is 0 (do not propagate the request's + * code field) */ + icmp->code = 0; /* Recompute full ICMP checksum for portability */ icmp->csum = 0; icmp->csum = ee16(icmp_checksum(icmp, ee16(ip->len) - IP_HEADER_LEN)); From 0ab2e4735cd52bf6240ae45864f731a428fe837a Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 14:25:45 +0200 Subject: [PATCH 20/35] F-10271: apply RFC 9293 acceptability in SYN_RCVD instead of exact seq 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. --- src/test/unit/unit.c | 4 +- src/test/unit/unit_tests_tcp_flow.c | 161 ++++++++++++++++++++++++++- src/test/unit/unit_tests_tcp_state.c | 1 + src/wolfip.c | 40 +++++-- 4 files changed, 194 insertions(+), 12 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 776aa4aa..8c7d7eab 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -604,7 +604,9 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_tcp_input_syn_listen_mismatch); tcase_add_test(tc_utils, test_tcp_input_syn_rcvd_ack_established); tcase_add_test(tc_utils, test_tcp_input_syn_rcvd_ack_invalid_ack_rejected); - tcase_add_test(tc_utils, test_tcp_input_syn_rcvd_ack_invalid_seq_rejected); + tcase_add_test(tc_utils, test_tcp_input_syn_rcvd_high_seq_valid_ack_establishes); + tcase_add_test(tc_utils, test_tcp_input_syn_rcvd_high_seq_data_held_ooo); + tcase_add_test(tc_utils, test_tcp_input_syn_rcvd_out_of_window_ack_drop); tcase_add_test(tc_utils, test_tcp_input_syn_rcvd_ack_fin_transitions_to_close_wait); tcase_add_test(tc_utils, test_tcp_input_filter_drop); tcase_add_test(tc_utils, test_tcp_input_port_mismatch_skips_socket); diff --git a/src/test/unit/unit_tests_tcp_flow.c b/src/test/unit/unit_tests_tcp_flow.c index 76b9e4c4..d3223878 100644 --- a/src/test/unit/unit_tests_tcp_flow.c +++ b/src/test/unit/unit_tests_tcp_flow.c @@ -3248,13 +3248,14 @@ START_TEST(test_tcp_input_syn_rcvd_ack_invalid_ack_rejected) } END_TEST -START_TEST(test_tcp_input_syn_rcvd_ack_invalid_seq_rejected) +START_TEST(test_tcp_input_syn_rcvd_high_seq_valid_ack_establishes) { struct wolfIP s; int listen_sd; struct tsocket *ts; struct wolfIP_sockaddr_in sin; struct wolfIP_tcp_seg ackseg; + uint32_t rcv_nxt; wolfIP_init(&s); mock_link_init(&s); @@ -3272,7 +3273,12 @@ START_TEST(test_tcp_input_syn_rcvd_ack_invalid_seq_rejected) inject_tcp_syn(&s, TEST_PRIMARY_IF, 0x0A000001U, 1234); ts = &s.tcpsockets[SOCKET_UNMARK(listen_sd)]; ck_assert_int_eq(ts->sock.tcp.state, TCP_SYN_RCVD); + rcv_nxt = ts->sock.tcp.ack; + /* ACK with a valid acknowledgment number and a sequence one above + * RCV.NXT (inside the receive window): per RFC 9293 this completes + * the handshake and the segment is held for later processing - the + * old exact-match check reset the connection here. */ memset(&ackseg, 0, sizeof(ackseg)); ackseg.ip.ver_ihl = 0x45; ackseg.ip.proto = WI_IPPROTO_TCP; @@ -3282,14 +3288,165 @@ START_TEST(test_tcp_input_syn_rcvd_ack_invalid_seq_rejected) ackseg.ip.dst = ee32(ts->local_ip); ackseg.dst_port = ee16(ts->src_port); ackseg.src_port = ee16(ts->dst_port); - ackseg.seq = ee32(ts->sock.tcp.ack + 1); + ackseg.seq = ee32(rcv_nxt + 1); ackseg.ack = ee32(ts->sock.tcp.seq + 1); ackseg.hlen = TCP_HEADER_LEN << 2; ackseg.flags = TCP_FLAG_ACK; fix_tcp_checksums(&ackseg); tcp_input(&s, TEST_PRIMARY_IF, &ackseg, (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN)); + ck_assert_int_eq(ts->sock.tcp.state, TCP_ESTABLISHED); + /* Nothing contiguous arrived: RCV.NXT must not skip the hole. */ + ck_assert_uint_eq(ts->sock.tcp.ack, rcv_nxt); +} +END_TEST + +START_TEST(test_tcp_input_syn_rcvd_high_seq_data_held_ooo) +{ + struct wolfIP s; + int listen_sd; + struct tsocket *ts; + struct wolfIP_sockaddr_in sin; + uint8_t seg_buf[sizeof(struct wolfIP_tcp_seg) + 10]; + struct wolfIP_tcp_seg *dataseg = (struct wolfIP_tcp_seg *)seg_buf; + uint32_t rcv_nxt; + uint8_t out[32]; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + listen_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, WI_IPPROTO_TCP); + ck_assert_int_gt(listen_sd, 0); + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_port = ee16(1234); + sin.sin_addr.s_addr = ee32(0x0A000001U); + ck_assert_int_eq(wolfIP_sock_bind(&s, listen_sd, (struct wolfIP_sockaddr *)&sin, sizeof(sin)), 0); + ck_assert_int_eq(wolfIP_sock_listen(&s, listen_sd, 1), 0); + + inject_tcp_syn(&s, TEST_PRIMARY_IF, 0x0A000001U, 1234); + ts = &s.tcpsockets[SOCKET_UNMARK(listen_sd)]; + ck_assert_int_eq(ts->sock.tcp.state, TCP_SYN_RCVD); + rcv_nxt = ts->sock.tcp.ack; + + /* The peer's first data segment (8 bytes) reorders ahead of the final + * ACK: seq = RCV.NXT + 10, valid acknowledgment number. */ + memset(seg_buf, 0, sizeof(seg_buf)); + dataseg->ip.ver_ihl = 0x45; + dataseg->ip.proto = WI_IPPROTO_TCP; + dataseg->ip.ttl = 64; + dataseg->ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN + 8); + dataseg->ip.src = ee32(ts->remote_ip); + dataseg->ip.dst = ee32(ts->local_ip); + dataseg->dst_port = ee16(ts->src_port); + dataseg->src_port = ee16(ts->dst_port); + dataseg->seq = ee32(rcv_nxt + 10); + dataseg->ack = ee32(tcp_seq_inc(ts->sock.tcp.snd_una, 1)); + dataseg->hlen = TCP_HEADER_LEN << 2; + dataseg->flags = TCP_FLAG_ACK; + memcpy(dataseg->data, "ABCDEFGH", 8); + fix_tcp_checksums(dataseg); + tcp_input(&s, TEST_PRIMARY_IF, dataseg, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN + 8)); + /* Handshake completes on the valid ACK; the data is OOO-cached and + * RCV.NXT must not skip the 10-byte hole. */ + ck_assert_int_eq(ts->sock.tcp.state, TCP_ESTABLISHED); + ck_assert_uint_eq(ts->sock.tcp.ack, rcv_nxt); + ck_assert_uint_eq(ts->sock.tcp.rx_sack_count, 1); + ck_assert_uint_eq(ts->sock.tcp.rx_sack[0].left, rcv_nxt + 10); + ck_assert_uint_eq(ts->sock.tcp.rx_sack[0].right, rcv_nxt + 18); + + /* The missing 10 bytes arrive in order and pull in the cached tail. */ + memset(seg_buf, 0, sizeof(seg_buf)); + dataseg->ip.ver_ihl = 0x45; + dataseg->ip.proto = WI_IPPROTO_TCP; + dataseg->ip.ttl = 64; + dataseg->ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN + 10); + dataseg->ip.src = ee32(ts->remote_ip); + dataseg->ip.dst = ee32(ts->local_ip); + dataseg->dst_port = ee16(ts->src_port); + dataseg->src_port = ee16(ts->dst_port); + dataseg->seq = ee32(rcv_nxt); + dataseg->ack = ee32(tcp_seq_inc(ts->sock.tcp.snd_una, 1)); + dataseg->hlen = TCP_HEADER_LEN << 2; + dataseg->flags = TCP_FLAG_ACK; + memcpy(dataseg->data, "0123456789", 10); + fix_tcp_checksums(dataseg); + tcp_input(&s, TEST_PRIMARY_IF, dataseg, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN + 10)); + ck_assert_uint_eq(ts->sock.tcp.ack, rcv_nxt + 18); + ck_assert_int_eq(queue_pop(&ts->sock.tcp.rxbuf, out, sizeof(out)), 18); + ck_assert_mem_eq(out, "0123456789ABCDEFGH", 18); +} +END_TEST + +START_TEST(test_tcp_input_syn_rcvd_out_of_window_ack_drop) +{ + struct wolfIP s; + int listen_sd; + struct tsocket *ts; + struct wolfIP_sockaddr_in sin; + struct wolfIP_tcp_seg ackseg; + uint32_t rcv_nxt; + uint32_t rcv_wnd; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + listen_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, WI_IPPROTO_TCP); + ck_assert_int_gt(listen_sd, 0); + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_port = ee16(1234); + sin.sin_addr.s_addr = ee32(0x0A000001U); + ck_assert_int_eq(wolfIP_sock_bind(&s, listen_sd, (struct wolfIP_sockaddr *)&sin, sizeof(sin)), 0); + ck_assert_int_eq(wolfIP_sock_listen(&s, listen_sd, 1), 0); + + inject_tcp_syn(&s, TEST_PRIMARY_IF, 0x0A000001U, 1234); + ts = &s.tcpsockets[SOCKET_UNMARK(listen_sd)]; ck_assert_int_eq(ts->sock.tcp.state, TCP_SYN_RCVD); + rcv_nxt = ts->sock.tcp.ack; + rcv_wnd = queue_space((struct queue *)&ts->sock.tcp.rxbuf); + + /* ACK-only segment whose sequence is beyond the receive window: + * RFC 9293 says acknowledge and drop, do not reset. */ + memset(&ackseg, 0, sizeof(ackseg)); + ackseg.ip.ver_ihl = 0x45; + ackseg.ip.proto = WI_IPPROTO_TCP; + ackseg.ip.ttl = 64; + ackseg.ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN); + ackseg.ip.src = ee32(ts->remote_ip); + ackseg.ip.dst = ee32(ts->local_ip); + ackseg.dst_port = ee16(ts->src_port); + ackseg.src_port = ee16(ts->dst_port); + ackseg.seq = ee32(rcv_nxt + rcv_wnd + 100); + ackseg.ack = ee32(ts->sock.tcp.seq + 1); + ackseg.hlen = TCP_HEADER_LEN << 2; + ackseg.flags = TCP_FLAG_ACK; + fix_tcp_checksums(&ackseg); + last_frame_sent_size = 0; + tcp_input(&s, TEST_PRIMARY_IF, &ackseg, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN)); + /* Out of window: ACK and drop, no state change. RSTs are sent + * immediately (they bypass the TX queue), so a clean wire right after + * tcp_input proves no reset was generated. */ + ck_assert_int_eq(ts->sock.tcp.state, TCP_SYN_RCVD); + ck_assert_uint_eq(ts->sock.tcp.ack, rcv_nxt); + ck_assert_uint_eq(last_frame_sent_size, 0U); + /* The acknowledgment is queued for the TX flush, behind the parked + * SYN-ACK: verify the second entry is an ACK, not a RST. */ + { + struct pkt_desc *pd = fifo_peek(&ts->sock.tcp.txbuf); + struct pkt_desc *pd2 = fifo_next(&ts->sock.tcp.txbuf, pd); + const struct wolfIP_tcp_seg *f; + ck_assert_ptr_nonnull(pd); + ck_assert_ptr_nonnull(pd2); + f = (const struct wolfIP_tcp_seg *)(pd2 + 1); + ck_assert_uint_eq(f->flags & (TCP_FLAG_ACK | TCP_FLAG_RST), + TCP_FLAG_ACK); + } } END_TEST diff --git a/src/test/unit/unit_tests_tcp_state.c b/src/test/unit/unit_tests_tcp_state.c index 5489f6a0..226629ef 100644 --- a/src/test/unit/unit_tests_tcp_state.c +++ b/src/test/unit/unit_tests_tcp_state.c @@ -964,6 +964,7 @@ START_TEST(test_tcp_input_syn_rcvd_ack_with_fin_enters_close_wait) ts->if_idx = TEST_PRIMARY_IF; ts->sock.tcp.peer_rwnd = 32768; ts->sock.tcp.tmr_rto = NO_TIMER; + queue_init(&ts->sock.tcp.rxbuf, ts->rxmem, RXBUF_SIZE, 2); fifo_init(&ts->sock.tcp.txbuf, ts->txmem, TXBUF_SIZE); /* ACK + FIN completing the handshake: expected_ack = snd_una+1 = 2 */ diff --git a/src/wolfip.c b/src/wolfip.c index 1d7666d8..61a51409 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -5497,10 +5497,20 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, } if (tcp->flags & TCP_FLAG_ACK) { uint32_t expected_ack = tcp_seq_inc(t->sock.tcp.snd_una, 1); - uint32_t expected_seq = t->sock.tcp.ack; - if (ee32(tcp->ack) != expected_ack || ee32(tcp->seq) != expected_seq) { - /* RFC 9293 section 3.10.7.4: unacceptable ACK in - * SYN_RCVD - send RST to peer. */ + /* RFC 9293 section 3.10.7.4: the sequence acceptability + * test (Table 6) applies before the final-ACK transition. + * 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 - + * ACK it and hold it for later processing (SHLD-31) + * instead of resetting the connection. */ + if (!tcp_segment_acceptable(t, tcp, tcplen)) { + tcp_send_ack(t); + continue; + } + if (ee32(tcp->ack) != expected_ack) { + /* RFC 9293 section 3.10.7.4: acceptable sequence but + * unacceptable ACK in SYN_RCVD - send RST to peer. */ tcp_send_reset_reply(S, if_idx, tcp); continue; } @@ -5508,7 +5518,10 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, tcp_ctrl_rto_stop(t); if (t->sock.tcp.is_listener) tcp_preaccept_timeout_start(t, t->S->last_tick); - t->sock.tcp.ack = ee32(tcp->seq); + /* t->sock.tcp.ack (RCV.NXT) is left as-is: when the + * accepted segment begins above RCV.NXT, tcp_recv caches + * it as OOO and advances RCV.NXT only once the hole is + * filled. */ t->sock.tcp.seq = ee32(tcp->ack); t->sock.tcp.snd_una = t->sock.tcp.seq; t->sock.tcp.cwnd = tcp_initial_cwnd(t->sock.tcp.peer_rwnd, tcp_cc_mss(t)); @@ -5520,10 +5533,19 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, /* RFC 9293 section 3.10.7.4: process FIN if present in the * same segment that completed the handshake. */ if (tcp->flags & TCP_FLAG_FIN) { - t->sock.tcp.ack = tcp_seq_inc(t->sock.tcp.ack, 1); - t->sock.tcp.state = TCP_CLOSE_WAIT; - t->events |= CB_EVENT_READABLE; - tcp_send_ack(t); + uint32_t fin_seq_end = + tcp_seq_inc(ee32(tcp->seq), tcplen); + if (t->sock.tcp.ack == fin_seq_end) { + t->sock.tcp.ack = tcp_seq_inc(fin_seq_end, 1); + t->sock.tcp.state = TCP_CLOSE_WAIT; + t->events |= CB_EVENT_READABLE; + tcp_send_ack(t); + } else { + /* FIN sits above a receive hole: the data was + * OOO-cached by tcp_recv; process the FIN once + * the hole fills. */ + tcp_send_ack(t); + } } } } else if (t->sock.tcp.state == TCP_TIME_WAIT) { From a95ff61622af29e0a9f5fdd289e8386a7fc347bf Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 14:47:28 +0200 Subject: [PATCH 21/35] F-6473: gate TS.Recent update on the RFC 7323 4.3 preconditions 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). --- src/test/unit/unit.c | 2 + src/test/unit/unit_tests_tcp_ack.c | 140 +++++++++++++++++++++++++++++ src/wolfip.c | 12 ++- 3 files changed, 153 insertions(+), 1 deletion(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 8c7d7eab..50d38ea6 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -683,6 +683,8 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_tcp_process_ts_nop_then_ts); tcase_add_test(tc_utils, test_tcp_process_ts_skips_unknown_option); tcase_add_test(tc_utils, test_tcp_process_ts_no_ecr); + tcase_add_test(tc_utils, test_tcp_process_ts_ooo_segment_keeps_recent); + tcase_add_test(tc_utils, test_tcp_input_paws_ooo_does_not_poison_hole_fill); tcase_add_test(tc_utils, test_tcp_process_ts_updates_rtt_when_set); tcase_add_test(tc_utils, test_tcp_send_syn_advertises_sack_permitted); tcase_add_test(tc_utils, test_tcp_build_ack_options_does_not_write_past_returned_len); diff --git a/src/test/unit/unit_tests_tcp_ack.c b/src/test/unit/unit_tests_tcp_ack.c index 0804538a..486454d2 100644 --- a/src/test/unit/unit_tests_tcp_ack.c +++ b/src/test/unit/unit_tests_tcp_ack.c @@ -4329,6 +4329,146 @@ START_TEST(test_tcp_process_ts_no_ecr) } END_TEST +START_TEST(test_tcp_process_ts_ooo_segment_keeps_recent) +{ + struct wolfIP s; + struct tsocket *ts; + uint8_t buf[sizeof(struct wolfIP_tcp_seg) + TCP_OPTIONS_LEN]; + struct wolfIP_tcp_seg *tcp = (struct wolfIP_tcp_seg *)buf; + struct tcp_opt_ts *opt = (struct tcp_opt_ts *)tcp->data; + + wolfIP_init(&s); + ts = &s.tcpsockets[0]; + memset(ts, 0, sizeof(*ts)); + ts->proto = WI_IPPROTO_TCP; + ts->S = &s; + ts->sock.tcp.ack = 1000; /* RCV.NXT == Last.ACK.sent (host order) */ + ts->sock.tcp.last_ts = ee32(100); /* TS.Recent (stored network order) */ + + memset(buf, 0, sizeof(buf)); + tcp->hlen = (TCP_HEADER_LEN + TCP_OPTIONS_LEN) << 2; + opt->opt = TCP_OPTION_TS; + opt->len = TCP_OPTION_TS_LEN; + opt->pad = TCP_OPTION_NOP; + opt->eoo = TCP_OPTION_EOO; + + /* Out-of-order segment (SEQ above the left edge): RFC 7323 4.3 + * rule (2) - TS.Recent must NOT be replaced. */ + tcp->seq = ee32(1100); + opt->val = ee32(5000); + tcp_process_ts(ts, tcp, sizeof(buf)); + ck_assert_uint_eq(ts->sock.tcp.last_ts, ee32(100)); + + /* In-order segment (SEQ == RCV.NXT) with a newer TSval: replace. */ + tcp->seq = ee32(1000); + opt->val = ee32(200); + tcp_process_ts(ts, tcp, sizeof(buf)); + ck_assert_uint_eq(ts->sock.tcp.last_ts, ee32(200)); + + /* Retransmission with an older TSval: never roll back. */ + opt->val = ee32(150); + tcp_process_ts(ts, tcp, sizeof(buf)); + ck_assert_uint_eq(ts->sock.tcp.last_ts, ee32(200)); +} +END_TEST + +START_TEST(test_tcp_input_paws_ooo_does_not_poison_hole_fill) +{ + struct wolfIP s; + struct tsocket *ts; + uint8_t seg_buf[sizeof(struct wolfIP_tcp_seg) + TCP_OPTIONS_LEN + 10]; + struct wolfIP_tcp_seg *seg = (struct wolfIP_tcp_seg *)seg_buf; + struct tcp_opt_ts *opt = (struct tcp_opt_ts *)seg->data; + uint32_t frame_len; + uint8_t out[32]; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + ts = &s.tcpsockets[0]; + memset(ts, 0, sizeof(*ts)); + ts->proto = WI_IPPROTO_TCP; + ts->S = &s; + ts->sock.tcp.state = TCP_ESTABLISHED; + ts->sock.tcp.ack = 1000; + ts->sock.tcp.seq = 1; + ts->sock.tcp.snd_una = 1; + ts->sock.tcp.ts_enabled = 1; + ts->sock.tcp.sack_permitted = 1; + ts->sock.tcp.last_ts = ee32(100); + ts->local_ip = 0x0A000001U; + ts->remote_ip = 0x0A000002U; + ts->src_port = 1234; + ts->dst_port = 4321; + ts->if_idx = TEST_PRIMARY_IF; + queue_init(&ts->sock.tcp.rxbuf, ts->rxmem, RXBUF_SIZE, 1000); + fifo_init(&ts->sock.tcp.txbuf, ts->txmem, TXBUF_SIZE); + + frame_len = (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + + TCP_HEADER_LEN + TCP_OPTIONS_LEN + 10); + + /* Out-of-order segment: seq 1010 (above RCV.NXT 1000), 10 bytes, + * TSval 5000. */ + memset(seg_buf, 0, sizeof(seg_buf)); + seg->ip.ver_ihl = 0x45; + seg->ip.proto = WI_IPPROTO_TCP; + seg->ip.ttl = 64; + seg->ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN + TCP_OPTIONS_LEN + 10); + seg->ip.src = ee32(ts->remote_ip); + seg->ip.dst = ee32(ts->local_ip); + seg->src_port = ee16(ts->dst_port); + seg->dst_port = ee16(ts->src_port); + seg->seq = ee32(1010); + seg->ack = ee32(ts->sock.tcp.snd_una); + seg->hlen = (TCP_HEADER_LEN + TCP_OPTIONS_LEN) << 2; + seg->flags = TCP_FLAG_ACK; + opt->opt = TCP_OPTION_TS; + opt->len = TCP_OPTION_TS_LEN; + opt->val = ee32(5000); + opt->ecr = 0; + opt->pad = TCP_OPTION_NOP; + opt->eoo = TCP_OPTION_EOO; + memcpy(seg->data + TCP_OPTIONS_LEN, "ABCDEFGHIJ", 10); + fix_tcp_checksums(seg); + tcp_input(&s, TEST_PRIMARY_IF, seg, frame_len); + /* OOO: cached above the hole; TS.Recent must not be poisoned. */ + ck_assert_uint_eq(ts->sock.tcp.ack, 1000); + ck_assert_uint_eq(ts->sock.tcp.last_ts, ee32(100)); + + /* The in-order hole-filling segment carries a LOWER TSval than the + * OOO one. If TS.Recent had been advanced to 5000 by the OOO + * segment, tcp_paws_check would drop this segment and the hole + * could never fill. */ + memset(seg_buf, 0, sizeof(seg_buf)); + seg->ip.ver_ihl = 0x45; + seg->ip.proto = WI_IPPROTO_TCP; + seg->ip.ttl = 64; + seg->ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN + TCP_OPTIONS_LEN + 10); + seg->ip.src = ee32(ts->remote_ip); + seg->ip.dst = ee32(ts->local_ip); + seg->src_port = ee16(ts->dst_port); + seg->dst_port = ee16(ts->src_port); + seg->seq = ee32(1000); + seg->ack = ee32(ts->sock.tcp.snd_una); + seg->hlen = (TCP_HEADER_LEN + TCP_OPTIONS_LEN) << 2; + seg->flags = TCP_FLAG_ACK; + opt->opt = TCP_OPTION_TS; + opt->len = TCP_OPTION_TS_LEN; + opt->val = ee32(200); + opt->ecr = 0; + opt->pad = TCP_OPTION_NOP; + opt->eoo = TCP_OPTION_EOO; + memcpy(seg->data + TCP_OPTIONS_LEN, "0123456789", 10); + fix_tcp_checksums(seg); + tcp_input(&s, TEST_PRIMARY_IF, seg, frame_len); + ck_assert_uint_eq(ts->sock.tcp.ack, 1020); + ck_assert_uint_eq(ts->sock.tcp.last_ts, ee32(200)); + ck_assert_int_eq(queue_pop(&ts->sock.tcp.rxbuf, out, sizeof(out)), 20); + ck_assert_mem_eq(out, "0123456789ABCDEFGHIJ", 20); +} +END_TEST + START_TEST(test_tcp_process_ts_updates_rtt_when_set) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 61a51409..86af5372 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -4687,7 +4687,17 @@ static int tcp_process_ts(struct tsocket *t, const struct wolfIP_tcp_seg *tcp, return -1; if (!t->S) return -1; /* Socket was closed; ignore. */ - t->sock.tcp.last_ts = ee32(po.ts_val); + /* RFC 7323 section 4.3 rule (2): TS.Recent is replaced only when the + * segment's TSval is not older than the stored one and the segment's + * sequence is at or below the ACK field of the last segment we sent + * (RCV.NXT here - this stack ACKs immediately, so no delayed-ACK skew). + * An out-of-order segment (SEQ above the left edge) must not advance + * TS.Recent: it would corrupt the TSecr echoed per rule (3) and make + * tcp_paws_check drop the in-order segment that later fills the hole, + * whose TSval is necessarily lower than the OOO segment's. */ + if (!tcp_seq_lt(ee32(po.ts_val), t->sock.tcp.last_ts) && + tcp_seq_leq(ee32(tcp->seq), t->sock.tcp.ack)) + t->sock.tcp.last_ts = ee32(po.ts_val); if (po.ts_ecr == 0) return -1; /* No echoed timestamp; fall back to coarse RTT. */ if (po.ts_ecr > t->S->last_tick) From 7ef3453d115acc1283ba61cca025cc895c54a97b Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 14:56:58 +0200 Subject: [PATCH 22/35] F-8520: drop dead IP length re-check in the tcp_input socket loop 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). --- src/wolfip.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/wolfip.c b/src/wolfip.c index 86af5372..18d04a47 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -5259,12 +5259,9 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, if (t->sock.tcp.state == TCP_CLOSED && (t->events & CB_EVENT_CLOSED)) continue; if (t->src_port == ee16(tcp->dst_port)) { - /* TCP segment sanity checks */ + /* TCP segment sanity checks (the ip.len vs frame_len bound is + * already enforced by the prologue above). */ iplen = ee16(tcp->ip.len); - if (iplen > frame_len - ETH_HEADER_LEN) { - return; /* discard */ - } - if (t->sock.tcp.state > TCP_LISTEN) { if (t->dst_port != ee16(tcp->src_port)) { /* Not the right socket */ From d9f119c1f4e8435c3dde09afee07f1d9ad8978f6 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 14:57:03 +0200 Subject: [PATCH 23/35] F-6944: test the future-TSecr guard in tcp_process_ts 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). --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_tcp_ack.c | 35 ++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 50d38ea6..d25e4a95 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -683,6 +683,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_tcp_process_ts_nop_then_ts); tcase_add_test(tc_utils, test_tcp_process_ts_skips_unknown_option); tcase_add_test(tc_utils, test_tcp_process_ts_no_ecr); + tcase_add_test(tc_utils, test_tcp_process_ts_future_ecr_rejected); tcase_add_test(tc_utils, test_tcp_process_ts_ooo_segment_keeps_recent); tcase_add_test(tc_utils, test_tcp_input_paws_ooo_does_not_poison_hole_fill); tcase_add_test(tc_utils, test_tcp_process_ts_updates_rtt_when_set); diff --git a/src/test/unit/unit_tests_tcp_ack.c b/src/test/unit/unit_tests_tcp_ack.c index 486454d2..fec76441 100644 --- a/src/test/unit/unit_tests_tcp_ack.c +++ b/src/test/unit/unit_tests_tcp_ack.c @@ -4329,6 +4329,41 @@ START_TEST(test_tcp_process_ts_no_ecr) } END_TEST +START_TEST(test_tcp_process_ts_future_ecr_rejected) +{ + struct wolfIP s; + struct tsocket *ts; + uint8_t buf[sizeof(struct wolfIP_tcp_seg) + TCP_OPTIONS_LEN]; + struct wolfIP_tcp_seg *tcp = (struct wolfIP_tcp_seg *)buf; + struct tcp_opt_ts *opt = (struct tcp_opt_ts *)tcp->data; + + wolfIP_init(&s); + ts = &s.tcpsockets[0]; + memset(ts, 0, sizeof(*ts)); + ts->proto = WI_IPPROTO_TCP; + ts->S = &s; + + memset(tcp, 0, sizeof(buf)); + tcp->hlen = (TCP_HEADER_LEN + TCP_OPTIONS_LEN) << 2; + opt->opt = TCP_OPTION_TS; + opt->len = TCP_OPTION_TS_LEN; + opt->val = ee32(1000); + /* ECR in the future of the local clock: the guard must reject it + * before the sample is computed, or (last_tick - ecr) underflows + * into a huge RTT sample that wrecks the RTO. */ + opt->ecr = ee32(5000); + opt->pad = TCP_OPTION_NOP; + opt->eoo = TCP_OPTION_EOO; + + s.last_tick = 1000; + ck_assert_int_eq(tcp_process_ts(ts, tcp, sizeof(buf)), -1); + /* No sample taken: RTO state is untouched. */ + ck_assert_uint_eq(ts->sock.tcp.rto_initialized, 0); + ck_assert_uint_eq(ts->sock.tcp.rtt, 0); + ck_assert_uint_eq(ts->sock.tcp.rto, 0); +} +END_TEST + START_TEST(test_tcp_process_ts_ooo_segment_keeps_recent) { struct wolfIP s; From e612852c4c83c9c546f17a7c8ee2f7493a993923 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 15:04:47 +0200 Subject: [PATCH 24/35] F-6474: RST on unacceptable ACK number in SYN_SENT (bare ACKs too) 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 (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). --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_api.c | 43 ++++++++++++++++++++++++++++++++++ src/wolfip.c | 25 +++++++++++++++----- 3 files changed, 63 insertions(+), 6 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index d25e4a95..ed3d1c01 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -296,6 +296,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_dhcp_renewing_transitions_to_rebinding); tcase_add_test(tc_utils, test_arp_recv_rejects_wrong_htype); tcase_add_test(tc_utils, test_syn_sent_bad_ack_synack_sends_rst); + tcase_add_test(tc_utils, test_syn_sent_bad_ack_bare_ack_sends_rst); tcase_add_test(tc_utils, test_syn_rcvd_bad_ack_sends_rst); tcase_add_test(tc_utils, test_established_fin_without_ack_dropped); tcase_add_test(tc_utils, test_ip_recv_drops_source_routed_packet); diff --git a/src/test/unit/unit_tests_api.c b/src/test/unit/unit_tests_api.c index 91e53461..54a9b0e1 100644 --- a/src/test/unit/unit_tests_api.c +++ b/src/test/unit/unit_tests_api.c @@ -4655,6 +4655,49 @@ START_TEST(test_syn_sent_bad_ack_synack_sends_rst) } END_TEST +/* Regression: per RFC 9293 §3.10.7.3, a bare ACK (no SYN) with an + * unacceptable ACK number in SYN_SENT must trigger a RST + * (). The code only matched the SYN|ACK flag + * combination, so a stray bare ACK was silently dropped instead of + * eliciting the required reset. */ +START_TEST(test_syn_sent_bad_ack_bare_ack_sends_rst) +{ + struct wolfIP s; + int sd; + struct tsocket *ts; + struct wolfIP_sockaddr_in sin; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, WI_IPPROTO_TCP); + ck_assert_int_gt(sd, 0); + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_port = ee16(80); + sin.sin_addr.s_addr = ee32(0x0A000002U); + ck_assert_int_eq(wolfIP_sock_connect(&s, sd, (struct wolfIP_sockaddr *)&sin, sizeof(sin)), + -WOLFIP_EAGAIN); + + ts = &s.tcpsockets[SOCKET_UNMARK(sd)]; + ck_assert_int_eq(ts->sock.tcp.state, TCP_SYN_SENT); + + /* Inject a bare ACK (no SYN) with an unacceptable ACK number. */ + last_frame_sent_size = 0; + inject_tcp_segment(&s, TEST_PRIMARY_IF, + 0x0A000002U, 0x0A000001U, + 80, ts->src_port, + 1000, 99, + TCP_FLAG_ACK); + + /* A RST must be sent */ + ck_assert_uint_gt(last_frame_sent_size, 0); + /* Socket must remain in SYN_SENT */ + ck_assert_int_eq(ts->sock.tcp.state, TCP_SYN_SENT); +} +END_TEST + /* Regression: per RFC 9293 §3.10.7.4, an ACK with invalid ack value in * SYN_RCVD must trigger a RST. The code silently dropped it. */ START_TEST(test_syn_rcvd_bad_ack_sends_rst) diff --git a/src/wolfip.c b/src/wolfip.c index 18d04a47..5da82cd1 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -5412,6 +5412,23 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, continue; } + /* RFC 9293 3.10.7.3: in SYN_SENT the ACK bit is examined + * first, for every segment (SYN or not). An ACK number outside + * (SND.UNA, SND.NXT] is unacceptable - in SYN_SENT that is any + * value other than ISS+1 (no data rides on the SYN). A segment + * carrying the ACK bit with such a number - a stray bare ACK, + * or a SYN-ACK with a bad ACK - gets + * and is dropped. RST-bearing segments never reach here: the + * RST handler above consumes them first, per the RFC's "unless + * the RST bit is set" caveat. */ + if (t->sock.tcp.state == TCP_SYN_SENT && + (tcp->flags & TCP_FLAG_ACK) && + (!tcp_seq_lt(t->sock.tcp.snd_una, ee32(tcp->ack)) || + tcp_seq_lt(tcp_seq_inc(t->sock.tcp.seq, 1), + ee32(tcp->ack)))) { + tcp_send_reset_reply(S, if_idx, tcp); + continue; + } /* Check if SYN */ if (tcp->flags & TCP_FLAG_SYN) { if (t->sock.tcp.state == TCP_LISTEN) { @@ -5465,13 +5482,9 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, tcp_ctrl_rto_start(t, S->last_tick); break; } else if (t->sock.tcp.state == TCP_SYN_SENT) { + /* Only reached for a SYN-ACK whose ACK number was + * acceptable (the check above RSTs the bad ones). */ if (tcp->flags == (TCP_FLAG_SYN | TCP_FLAG_ACK)) { - if (ee32(tcp->ack) != tcp_seq_inc(t->sock.tcp.seq, 1)) { - /* RFC 9293: invalid ACK in SYN_SENT - send RST - * to help peer clean up stale half-open state. */ - tcp_send_reset_reply(S, if_idx, tcp); - continue; - } t->sock.tcp.state = TCP_ESTABLISHED; tcp_ctrl_rto_stop(t); t->sock.tcp.ack = tcp_seq_inc(ee32(tcp->seq), 1); From 9bd32172f7239f70fbb9a1794663856e21b4f36b Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 15:13:27 +0200 Subject: [PATCH 25/35] F-10265: RST replies carry a Timestamps option when the trigger had one 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). --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_api.c | 80 ++++++++++++++++++++++++ src/wolfip.c | 111 ++++++++++++++++++++++++--------- 3 files changed, 161 insertions(+), 31 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index ed3d1c01..74f9c6b9 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -297,6 +297,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_arp_recv_rejects_wrong_htype); tcase_add_test(tc_utils, test_syn_sent_bad_ack_synack_sends_rst); tcase_add_test(tc_utils, test_syn_sent_bad_ack_bare_ack_sends_rst); + tcase_add_test(tc_utils, test_rst_reply_carries_timestamp_when_incoming_had_one); tcase_add_test(tc_utils, test_syn_rcvd_bad_ack_sends_rst); tcase_add_test(tc_utils, test_established_fin_without_ack_dropped); tcase_add_test(tc_utils, test_ip_recv_drops_source_routed_packet); diff --git a/src/test/unit/unit_tests_api.c b/src/test/unit/unit_tests_api.c index 54a9b0e1..331486b2 100644 --- a/src/test/unit/unit_tests_api.c +++ b/src/test/unit/unit_tests_api.c @@ -4698,6 +4698,86 @@ START_TEST(test_syn_sent_bad_ack_bare_ack_sends_rst) } END_TEST +/* Regression: per RFC 7323, a generated RST must carry a Timestamps + * option (TSecr = incoming TSval, TSval = 0) whenever the segment that + * triggered it carried one, so PAWS-aware peers can apply stricter RST + * acceptance checks. tcp_send_reset_reply built a bare 20-byte header + * and dropped the option. */ +START_TEST(test_rst_reply_carries_timestamp_when_incoming_had_one) +{ + struct wolfIP s; + int sd; + struct tsocket *ts; + struct wolfIP_sockaddr_in sin; + uint8_t seg_buf[sizeof(struct wolfIP_tcp_seg) + TCP_OPTIONS_LEN]; + struct wolfIP_tcp_seg *seg = (struct wolfIP_tcp_seg *)seg_buf; + struct wolfIP_tcp_seg *rst; + struct tcp_opt_ts *opt = (struct tcp_opt_ts *)seg->data; + struct tcp_opt_ts *rts; + uint32_t frame_len; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, WI_IPPROTO_TCP); + ck_assert_int_gt(sd, 0); + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_port = ee16(80); + sin.sin_addr.s_addr = ee32(0x0A000002U); + ck_assert_int_eq(wolfIP_sock_connect(&s, sd, (struct wolfIP_sockaddr *)&sin, sizeof(sin)), + -WOLFIP_EAGAIN); + + ts = &s.tcpsockets[SOCKET_UNMARK(sd)]; + ck_assert_int_eq(ts->sock.tcp.state, TCP_SYN_SENT); + + /* Build a bare ACK with an unacceptable ACK number AND a Timestamps + * option (TSval = 43981). In SYN_SENT this elicits a RST. */ + memset(seg_buf, 0, sizeof(seg_buf)); + seg->ip.eth.type = ee16(ETH_TYPE_IP); + seg->ip.ver_ihl = 0x45; + seg->ip.ttl = 64; + seg->ip.proto = WI_IPPROTO_TCP; + seg->ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN + TCP_OPTIONS_LEN); + seg->ip.src = ee32(0x0A000002U); + seg->ip.dst = ee32(ts->local_ip); + seg->src_port = ee16(80); + seg->dst_port = ee16(ts->src_port); + seg->seq = ee32(1000); + seg->ack = ee32(99); /* unacceptable in SYN_SENT */ + seg->hlen = (uint8_t)((TCP_HEADER_LEN + TCP_OPTIONS_LEN) << 2); + seg->flags = TCP_FLAG_ACK; + seg->win = ee16(65535); + opt->opt = TCP_OPTION_TS; + opt->len = TCP_OPTION_TS_LEN; + opt->val = ee32(43981); /* incoming TSval */ + opt->ecr = 0; + opt->pad = TCP_OPTION_NOP; + opt->eoo = TCP_OPTION_EOO; + fix_tcp_checksums(seg); + + frame_len = (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + + TCP_HEADER_LEN + TCP_OPTIONS_LEN); + last_frame_sent_size = 0; + tcp_input(&s, TEST_PRIMARY_IF, seg, frame_len); + + /* A RST must be sent and the socket must stay in SYN_SENT. */ + ck_assert_uint_gt(last_frame_sent_size, 0); + ck_assert_int_eq(ts->sock.tcp.state, TCP_SYN_SENT); + + /* The RST must carry the Timestamps option: TSval = 0, TSecr = the + * incoming TSval (43981). */ + rst = (struct wolfIP_tcp_seg *)last_frame_sent; + ck_assert_uint_eq(rst->flags & (TCP_FLAG_RST | TCP_FLAG_ACK), TCP_FLAG_RST); + rts = (struct tcp_opt_ts *)rst->data; + ck_assert_uint_eq(rts->opt, TCP_OPTION_TS); + ck_assert_uint_eq(rts->len, TCP_OPTION_TS_LEN); + ck_assert_uint_eq(ee32(rts->val), 0); + ck_assert_uint_eq(ee32(rts->ecr), 43981); +} +END_TEST + /* Regression: per RFC 9293 §3.10.7.4, an ACK with invalid ack value in * SYN_RCVD must trigger a RST. The code silently dropped it. */ START_TEST(test_syn_rcvd_bad_ack_sends_rst) diff --git a/src/wolfip.c b/src/wolfip.c index 5da82cd1..610f1329 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -3568,11 +3568,16 @@ static void tcp_send_ack(struct tsocket *t) static void tcp_send_reset_reply(struct wolfIP *s, unsigned int if_idx, const struct wolfIP_tcp_seg *in) { - struct wolfIP_tcp_seg out; + uint8_t buffer[sizeof(struct wolfIP_tcp_seg) + sizeof(struct tcp_opt_ts)]; + struct wolfIP_tcp_seg *out = (struct wolfIP_tcp_seg *)buffer; union transport_pseudo_header ph; uint16_t ip_len; uint32_t tcp_hlen; uint32_t seg_ack; + uint32_t opt_len; + uint32_t out_len; + uint8_t have_ts = 0; + uint32_t in_tsval = 0; if (in->flags & TCP_FLAG_RST) return; @@ -3584,14 +3589,48 @@ static void tcp_send_reset_reply(struct wolfIP *s, unsigned int if_idx, if (ip_len < (uint16_t)(IP_HEADER_LEN + tcp_hlen)) return; - memset(&out, 0, sizeof(out)); - out.src_port = in->dst_port; - out.dst_port = in->src_port; - out.hlen = TCP_HEADER_LEN << 2; + /* RFC 7323: when the segment that caused this RST carried a Timestamps + * option, the RST SHOULD carry one too, with TSecr set to the incoming + * TSval and TSval set to zero, so PAWS-aware peers can apply stricter + * RST acceptance checks. (A RST we receive is never fed into PAWS state: + * the RST handler consumes it before any timestamp processing.) */ + if (tcp_hlen > TCP_HEADER_LEN) { + const uint8_t *opt = in->data; + const uint8_t *opt_end = opt + (tcp_hlen - TCP_HEADER_LEN); + while (opt < opt_end) { + uint8_t kind = *opt; + uint8_t olen; + if (kind == TCP_OPTION_EOO) + break; + if (kind == TCP_OPTION_NOP) { + opt++; + continue; + } + if (opt + 2 > opt_end) + break; + olen = opt[1]; + if (olen < 2 || opt + olen > opt_end) + break; + if (kind == TCP_OPTION_TS && olen == TCP_OPTION_TS_LEN) { + memcpy(&in_tsval, opt + 2, sizeof(in_tsval)); + in_tsval = ee32(in_tsval); + have_ts = 1; + } + opt += olen; + } + } + + opt_len = have_ts ? (uint32_t)sizeof(struct tcp_opt_ts) : 0; + out_len = sizeof(struct wolfIP_tcp_seg) + opt_len; + + memset(buffer, 0, sizeof(buffer)); + out->src_port = in->dst_port; + out->dst_port = in->src_port; + out->hlen = (uint8_t)((TCP_HEADER_LEN + opt_len) << 2); if (in->flags & TCP_FLAG_ACK) { - out.seq = in->ack; - out.flags = TCP_FLAG_RST; + out->seq = in->ack; + out->flags = TCP_FLAG_RST; } else { seg_ack = ee32(in->seq); seg_ack = tcp_seq_inc(seg_ack, ip_len - (uint16_t)(IP_HEADER_LEN + tcp_hlen)); @@ -3599,40 +3638,50 @@ static void tcp_send_reset_reply(struct wolfIP *s, unsigned int if_idx, seg_ack = tcp_seq_inc(seg_ack, 1); if (in->flags & TCP_FLAG_FIN) seg_ack = tcp_seq_inc(seg_ack, 1); - out.ack = ee32(seg_ack); - out.flags = TCP_FLAG_RST | TCP_FLAG_ACK; + out->ack = ee32(seg_ack); + out->flags = TCP_FLAG_RST | TCP_FLAG_ACK; + } + + if (have_ts) { + struct tcp_opt_ts *ts = (struct tcp_opt_ts *)out->data; + ts->opt = TCP_OPTION_TS; + ts->len = TCP_OPTION_TS_LEN; + ts->val = 0; /* TSval = 0 */ + ts->ecr = ee32(in_tsval); /* TSecr = incoming TSval */ + ts->pad = TCP_OPTION_NOP; + ts->eoo = TCP_OPTION_EOO; } - out.ip.src = in->ip.dst; - out.ip.dst = in->ip.src; - out.ip.ver_ihl = 0x45; - out.ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN); - out.ip.flags_fo = ee16(0x4000U); - out.ip.ttl = 64; - out.ip.proto = WI_IPPROTO_TCP; - out.ip.id = ipcounter_next(s); - iphdr_set_checksum(&out.ip); + out->ip.src = in->ip.dst; + out->ip.dst = in->ip.src; + out->ip.ver_ihl = 0x45; + out->ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN + opt_len); + out->ip.flags_fo = ee16(0x4000U); + out->ip.ttl = 64; + out->ip.proto = WI_IPPROTO_TCP; + out->ip.id = ipcounter_next(s); + iphdr_set_checksum(&out->ip); memset(&ph, 0, sizeof(ph)); - ph.ph.src = out.ip.src; - ph.ph.dst = out.ip.dst; + ph.ph.src = out->ip.src; + ph.ph.dst = out->ip.dst; ph.ph.proto = WI_IPPROTO_TCP; - ph.ph.len = ee16(TCP_HEADER_LEN); - out.csum = ee16(transport_checksum(&ph, &out.src_port)); + ph.ph.len = ee16(TCP_HEADER_LEN + opt_len); + out->csum = ee16(transport_checksum(&ph, &out->src_port)); #ifdef ETHERNET if (!wolfIP_ll_is_non_ethernet(s, if_idx)) { - if (eth_output_add_header(s, if_idx, in->ip.eth.src, &out.ip.eth, ETH_TYPE_IP) != 0) + if (eth_output_add_header(s, if_idx, in->ip.eth.src, &out->ip.eth, ETH_TYPE_IP) != 0) return; } #endif - if (wolfIP_filter_notify_tcp(WOLFIP_FILT_SENDING, s, if_idx, &out, sizeof(out)) != 0) + if (wolfIP_filter_notify_tcp(WOLFIP_FILT_SENDING, s, if_idx, out, out_len) != 0) return; - if (wolfIP_filter_notify_ip(WOLFIP_FILT_SENDING, s, if_idx, &out.ip, sizeof(out)) != 0) + if (wolfIP_filter_notify_ip(WOLFIP_FILT_SENDING, s, if_idx, &out->ip, out_len) != 0) return; #ifdef ETHERNET if (!wolfIP_ll_is_non_ethernet(s, if_idx)) { - if (wolfIP_filter_notify_eth(WOLFIP_FILT_SENDING, s, if_idx, &out.ip.eth, sizeof(out)) != 0) + if (wolfIP_filter_notify_eth(WOLFIP_FILT_SENDING, s, if_idx, &out->ip.eth, out_len) != 0) return; } #endif @@ -3640,16 +3689,16 @@ static void tcp_send_reset_reply(struct wolfIP *s, unsigned int if_idx, #ifdef WOLFIP_ESP if (!wolfIP_ll_is_non_ethernet(s, if_idx)) { struct wolfIP_ll_dev *ll_esp = wolfIP_ll_at(s, if_idx); - int esp_err = esp_send(ll_esp, &out.ip, - (uint16_t)(sizeof(out) - ETH_HEADER_LEN)); + int esp_err = esp_send(ll_esp, &out->ip, + (uint16_t)(out_len - ETH_HEADER_LEN)); if (esp_err == 1) { - wolfIP_ll_send_frame(s, if_idx, &out.ip, sizeof(out)); + wolfIP_ll_send_frame(s, if_idx, &out->ip, out_len); } } else { - wolfIP_ll_send_frame(s, if_idx, &out.ip, sizeof(out)); + wolfIP_ll_send_frame(s, if_idx, &out->ip, out_len); } #else - wolfIP_ll_send_frame(s, if_idx, &out.ip, sizeof(out)); + wolfIP_ll_send_frame(s, if_idx, &out->ip, out_len); #endif } } From 0ee8f61b8f6014a15810dc385161ab21d4b06fbf Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 15:21:50 +0200 Subject: [PATCH 26/35] F-6943: pin the PMTU never-increase guard on peer_mss 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. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_dns_dhcp.c | 63 +++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 74f9c6b9..51aeb95c 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -929,6 +929,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_icmp_input_dest_unreach_port_unreachable_keeps_established_tcp_socket); tcase_add_test(tc_proto, test_icmp_input_dest_unreach_frag_needed_reduces_tcp_peer_mss); tcase_add_test(tc_proto, test_icmp_input_dest_unreach_frag_needed_below_floor_preserves_peer_mss); + tcase_add_test(tc_proto, test_icmp_input_dest_unreach_frag_needed_larger_mtu_does_not_raise_peer_mss); tcase_add_test(tc_proto, test_icmp_input_dest_unreach_port_unreachable_closes_syn_sent_tcp_socket); tcase_add_test(tc_proto, test_icmp_input_dest_unreach_port_unreachable_quoted_ip_options_keep_established_tcp_socket); tcase_add_test(tc_proto, test_icmp_input_dest_unreach_port_unreachable_mismatched_orig_src_ip_ignored); diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index 20afe39c..f64daba1 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -2552,6 +2552,69 @@ START_TEST(test_icmp_input_dest_unreach_frag_needed_below_floor_preserves_peer_m } END_TEST +/* Regression: the PMTU peer_mss guard is monotonic-decrease (never + * increase). A spoofed FRAG_NEEDED advertising a LARGER next-hop MTU must + * not re-inflate peer_mss back up after a legitimate prior PMTU reduction. + * This pins the `new_mss < peer_mss` direction: deleting/inverting that + * clause lets a forged message restore peer_mss to the local MSS, cancelling + * the reduction and re-creating a Path-MTU black hole (DF is set). */ +START_TEST(test_icmp_input_dest_unreach_frag_needed_larger_mtu_does_not_raise_peer_mss) +{ + struct wolfIP s; + struct tsocket *ts; + struct wolfIP_icmp_dest_unreachable_packet icmp; + struct wolfIP_tcp_wire_prefix *orig; + uint32_t frame_len; + uint16_t next_hop_mtu; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + ts = &s.tcpsockets[0]; + memset(ts, 0, sizeof(*ts)); + ts->proto = WI_IPPROTO_TCP; + ts->S = &s; + ts->sock.tcp.state = TCP_ESTABLISHED; + ts->local_ip = 0x0A000001U; + ts->remote_ip = 0x0A000002U; + ts->src_port = 1234; + ts->dst_port = 4321; + /* Simulate a prior legitimate PMTU reduction. */ + ts->sock.tcp.peer_mss = 536U; + + memset(&icmp, 0, sizeof(icmp)); + icmp.ip.src = ee32(0x0A0000FEU); + icmp.ip.dst = ee32(ts->local_ip); + icmp.ip.ttl = 64; + icmp.ip.proto = WI_IPPROTO_ICMP; + icmp.ip.len = ee16(IP_HEADER_LEN + ICMP_DEST_UNREACH_SIZE); + icmp.type = ICMP_DEST_UNREACH; + icmp.code = ICMP_FRAG_NEEDED; + /* Large next-hop MTU: derived MSS (1460) is ABOVE the current 536. */ + next_hop_mtu = ee16(1500U); + memcpy(&icmp.unused[2], &next_hop_mtu, sizeof(next_hop_mtu)); + + orig = (struct wolfIP_tcp_wire_prefix *)icmp.orig_packet; + orig->ip.ver_ihl = 0x45; + orig->ip.proto = WI_IPPROTO_TCP; + orig->ip.src = ee32(ts->local_ip); + orig->ip.dst = ee32(ts->remote_ip); + orig->ip.len = ee16(IP_HEADER_LEN + 8U); + orig->src_port = ee16(ts->src_port); + orig->dst_port = ee16(ts->dst_port); + + icmp.csum = ee16(icmp_checksum((struct wolfIP_icmp_packet *)&icmp, + ICMP_DEST_UNREACH_SIZE)); + frame_len = (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + ICMP_DEST_UNREACH_SIZE); + + icmp_input(&s, TEST_PRIMARY_IF, (struct wolfIP_ip_packet *)&icmp, frame_len); + + /* peer_mss must be unchanged: the guard never raises it. */ + ck_assert_uint_eq(ts->sock.tcp.peer_mss, 536U); +} +END_TEST + START_TEST(test_icmp_input_dest_unreach_port_unreachable_closes_syn_sent_tcp_socket) { struct wolfIP s; From 3f63c5502f6f9b12eb698a14d8f3594b4e2cad8a Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 15:33:03 +0200 Subject: [PATCH 27/35] F-10276: RENEWING/REBINDING DHCPREQUEST retries use half-remaining-time 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)., --- src/test/unit/unit.c | 4 ++ src/test/unit/unit_tests_dns_dhcp.c | 102 ++++++++++++++++++++++++++++ src/wolfip.c | 31 ++++++++- 3 files changed, 135 insertions(+), 2 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 51aeb95c..12cdd228 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -420,6 +420,10 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_dhcp_timer_cb_paths); tcase_add_test(tc_utils, test_dhcp_discover_retransmit_backoff); tcase_add_test(tc_utils, test_dhcp_request_retransmit_backoff); + tcase_add_test(tc_utils, test_dhcp_renew_rebind_delay_ms); + tcase_add_test(tc_utils, test_dhcp_schedule_renew_rebind_retry); + tcase_add_test(tc_utils, test_dhcp_renewing_retry_half_remaining_to_t2); + tcase_add_test(tc_utils, test_dhcp_rebinding_retry_half_remaining_to_lease); tcase_add_test(tc_utils, test_regression_dhcp_lease_expiry_deconfigures_address); tcase_add_test(tc_utils, test_dhcp_request_retry_exhaustion_deconfigures_lease); tcase_add_test(tc_utils, test_dhcp_timer_cb_send_failure_does_not_consume_retry_budget); diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index f64daba1..5586b879 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -5021,6 +5021,108 @@ START_TEST(test_dhcp_request_retransmit_backoff) } END_TEST +/* RFC 2131: in RENEWING/REBINDING a DHCPREQUEST with no response is + * retransmitted after one-half the remaining time (to T2 / to lease expiry), + * floored at 60 s. The pure delay function is pinned here, including the + * floor and the cap at the remaining time (a retry must never land past the + * deadline, where the state transition fires). */ +START_TEST(test_dhcp_renew_rebind_delay_ms) +{ + /* one-half remaining, above the 60 s floor */ + ck_assert_uint_eq(dhcp_renew_rebind_delay_ms(200000), 100000); + ck_assert_uint_eq(dhcp_renew_rebind_delay_ms(120000), 60000); /* exactly the floor */ + /* half below the floor -> floored to 60 s */ + ck_assert_uint_eq(dhcp_renew_rebind_delay_ms(100000), 60000); + ck_assert_uint_eq(dhcp_renew_rebind_delay_ms(90000), 60000); + /* remaining at/below the floor -> capped at remaining */ + ck_assert_uint_eq(dhcp_renew_rebind_delay_ms(60000), 60000); + ck_assert_uint_eq(dhcp_renew_rebind_delay_ms(30000), 30000); + ck_assert_uint_eq(dhcp_renew_rebind_delay_ms(1000), 1000); + /* zero remaining -> 1 ms, never an indefinite/past-now schedule */ + ck_assert_uint_eq(dhcp_renew_rebind_delay_ms(0), 1); +} +END_TEST + +/* The RENEWING/REBINDING scheduler must place the retry at + * last_tick + max(60 s, remaining/2), capped at the deadline. */ +START_TEST(test_dhcp_schedule_renew_rebind_retry) +{ + struct wolfIP s; + const uint64_t now = 100000U; + + wolfIP_init(&s); + mock_link_init(&s); + + /* remaining 200 s -> half = 100 s (above the floor) */ + s.last_tick = now; + dhcp_schedule_renew_rebind_retry(&s, now + 200000); + ck_assert_uint_eq(find_timer_expiry(&s, s.dhcp_timer) - now, 100000); + + /* remaining 100 s -> half = 50 s, floored to 60 s */ + s.last_tick = now; + dhcp_schedule_renew_rebind_retry(&s, now + 100000); + ck_assert_uint_eq(find_timer_expiry(&s, s.dhcp_timer) - now, 60000); + + /* remaining 30 s -> floored to 60 s, capped to 30 s (the deadline) */ + s.last_tick = now; + dhcp_schedule_renew_rebind_retry(&s, now + 30000); + ck_assert_uint_eq(find_timer_expiry(&s, s.dhcp_timer) - now, 30000); +} +END_TEST + +/* End-to-end: a RENEWING DHCPREQUEST with no response must be retransmitted + * at one-half the remaining time to T2 (RFC 2131), not on the generic 2 s + * exponential backoff. With T2 200 s out the retry lands at +100 s. Pre-fix + * this scheduled ~2 s (the backoff), so this assertion fails without the fix. */ +START_TEST(test_dhcp_renewing_retry_half_remaining_to_t2) +{ + struct wolfIP s; + const uint64_t now = 100000U; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + s.dhcp_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_gt(s.dhcp_udp_sd, 0); + s.dhcp_xid = 1; + s.dhcp_server_ip = 0x0A000064U; /* on-link, so the renewal unicast routes */ + + s.dhcp_state = DHCP_RENEWING; + s.last_tick = now; + s.dhcp_timeout_count = 0; + s.dhcp_rebind_at = now + 200000; /* T2 is 200 s ahead */ + dhcp_timer_cb(&s); + + ck_assert_uint_eq(find_timer_expiry(&s, s.dhcp_timer) - now, 100000); +} +END_TEST + +/* End-to-end: a REBINDING DHCPREQUEST with no response must be retransmitted + * at one-half the remaining lease time (RFC 2131). With the lease 200 s from + * expiry the retry lands at +100 s. Pre-fix this scheduled ~2 s backoff. */ +START_TEST(test_dhcp_rebinding_retry_half_remaining_to_lease) +{ + struct wolfIP s; + const uint64_t now = 100000U; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + s.dhcp_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_gt(s.dhcp_udp_sd, 0); + s.dhcp_xid = 1; + s.dhcp_server_ip = 0x0A000064U; + + s.dhcp_state = DHCP_REBINDING; + s.last_tick = now; + s.dhcp_timeout_count = 0; + s.dhcp_lease_expires = now + 200000; /* lease expires in 200 s */ + dhcp_timer_cb(&s); + + ck_assert_uint_eq(find_timer_expiry(&s, s.dhcp_timer) - now, 100000); +} +END_TEST + START_TEST(test_regression_dhcp_lease_expiry_deconfigures_address) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 610f1329..36096c96 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -8270,6 +8270,33 @@ static void dhcp_schedule_retry_timer(struct wolfIP *s, uint64_t deadline) dhcp_schedule_timer_at(s, next); } +/* RFC 2131 retransmission delay for RENEWING (to T2) and REBINDING (to + * lease expiry): one-half the remaining time, floored at 60 s. Capped at + * the remaining time so the retry never lands past the deadline - the + * T2 / lease-expiry state transition (handled in the timer callback) must + * still fire on time. */ +static uint64_t dhcp_renew_rebind_delay_ms(uint64_t remaining_ms) +{ + uint64_t delay = remaining_ms / 2U; + + if (delay < 60000U) + delay = 60000U; + if (delay > remaining_ms) + delay = remaining_ms; + return (delay == 0) ? 1U : delay; +} + +static void dhcp_schedule_renew_rebind_retry(struct wolfIP *s, uint64_t deadline) +{ + uint64_t remaining; + + if (!s || deadline == 0) + return; + remaining = (deadline > s->last_tick) ? (deadline - s->last_tick) : 0; + dhcp_schedule_timer_at(s, + s->last_tick + dhcp_renew_rebind_delay_ms(remaining)); +} + static uint16_t dhcp_elapsed_secs(const struct wolfIP *s) { uint64_t elapsed_ms; @@ -9075,9 +9102,9 @@ static int dhcp_send_request(struct wolfIP *s) if (!renewing && !rebinding) { dhcp_schedule_retry_timer(s, 0); } else if (renewing) { - dhcp_schedule_retry_timer(s, s->dhcp_rebind_at); + dhcp_schedule_renew_rebind_retry(s, s->dhcp_rebind_at); } else { - dhcp_schedule_retry_timer(s, s->dhcp_lease_expires); + dhcp_schedule_renew_rebind_retry(s, s->dhcp_lease_expires); } return 0; } From 7f9d3b96efbe9a2ddda461787f820e60669db7a3 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 15:47:01 +0200 Subject: [PATCH 28/35] F-6940: set the DHCP BROADCAST bit while the client has no bound IP 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)., --- src/test/unit/unit.c | 2 + src/test/unit/unit_tests_dns_dhcp.c | 84 +++++++++++++++++++++++++++++ src/wolfip.c | 12 ++++- 3 files changed, 97 insertions(+), 1 deletion(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 12cdd228..4f17a6ee 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -538,6 +538,8 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_dhcp_parse_offer_option_split_across_region_boundary); tcase_add_test(tc_utils, test_dhcp_discover_first_retry_delay_rfc2131); + tcase_add_test(tc_utils, test_dhcp_discover_sets_broadcast_flag); + tcase_add_test(tc_utils, test_dhcp_request_broadcast_flag_by_state); tcase_add_test(tc_utils, test_dhcp_discover_retry_delay_small_base_no_underflow); tcase_add_test(tc_utils, test_dns_wrapper_apis); tcase_add_test(tc_utils, test_wolfip_static_instance_apis); diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index 5586b879..aea5ef58 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -1187,6 +1187,90 @@ START_TEST(test_dhcp_discover_retry_delay_small_base_no_underflow) } END_TEST +/* RFC 2131 4.4.1: a discovering client has no IP address and cannot + * receive a unicast reply, so DHCPDISCOVER must carry the BROADCAST bit + * (bit 15 of the flags field) so the server broadcasts the DHCPOFFER. */ +START_TEST(test_dhcp_discover_sets_broadcast_flag) +{ + struct wolfIP s; + struct dhcp_msg *msg; + + wolfIP_init(&s); + mock_link_init(&s); + s.dhcp_xid = 1U; + s.last_tick = 1000U; + s.dhcp_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, + WI_IPPROTO_UDP); + ck_assert_int_gt(s.dhcp_udp_sd, 0); + + last_frame_sent_size = 0; + ck_assert_int_eq(dhcp_send_discover(&s), 0); + (void)wolfIP_poll(&s, 10); + ck_assert_uint_gt(last_frame_sent_size, 0); + + msg = (struct dhcp_msg *)(last_frame_sent + + ETH_HEADER_LEN + IP_HEADER_LEN + UDP_HEADER_LEN); + ck_assert_uint_eq(msg->flags, ee16(0x8000)); +} +END_TEST + +/* RFC 2131 4.4.1: the BROADCAST bit is set only while the client has no + * bound IP (the initial DHCPREQUEST). In RENEWING and REBINDING the client + * holds a bound IP and expects a unicast reply, so the bit stays clear. */ +START_TEST(test_dhcp_request_broadcast_flag_by_state) +{ + struct wolfIP s; + struct dhcp_msg *msg; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + s.dhcp_xid = 1U; + s.last_tick = 1000U; + s.dhcp_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, + WI_IPPROTO_UDP); + ck_assert_int_gt(s.dhcp_udp_sd, 0); + s.dhcp_server_ip = 0x0A000064U; + s.dhcp_ip = 0x0A000065U; + + /* Pre-seed ARP for the renewal server so the RENEWING unicast flushes + * (otherwise the frame waits on ARP and the capture stays stale). */ + s.arp.neighbors[0].ip = 0x0A000064U; + s.arp.neighbors[0].if_idx = TEST_PRIMARY_IF; + memset(s.arp.neighbors[0].mac, 0x02, 6); + + /* Initial REQUEST (no bound IP): BROADCAST bit set. */ + s.dhcp_state = DHCP_REQUEST_SENT; + last_frame_sent_size = 0; + ck_assert_int_eq(dhcp_send_request(&s), 0); + (void)wolfIP_poll(&s, 10); + ck_assert_uint_gt(last_frame_sent_size, 0); + msg = (struct dhcp_msg *)(last_frame_sent + + ETH_HEADER_LEN + IP_HEADER_LEN + UDP_HEADER_LEN); + ck_assert_uint_eq(msg->flags, ee16(0x8000)); + + /* RENEWING (bound IP): BROADCAST bit clear. */ + s.dhcp_state = DHCP_RENEWING; + last_frame_sent_size = 0; + ck_assert_int_eq(dhcp_send_request(&s), 0); + (void)wolfIP_poll(&s, 10); + ck_assert_uint_gt(last_frame_sent_size, 0); + msg = (struct dhcp_msg *)(last_frame_sent + + ETH_HEADER_LEN + IP_HEADER_LEN + UDP_HEADER_LEN); + ck_assert_uint_eq(msg->flags, 0); + + /* REBINDING (bound IP): BROADCAST bit clear. */ + s.dhcp_state = DHCP_REBINDING; + last_frame_sent_size = 0; + ck_assert_int_eq(dhcp_send_request(&s), 0); + (void)wolfIP_poll(&s, 10); + ck_assert_uint_gt(last_frame_sent_size, 0); + msg = (struct dhcp_msg *)(last_frame_sent + + ETH_HEADER_LEN + IP_HEADER_LEN + UDP_HEADER_LEN); + ck_assert_uint_eq(msg->flags, 0); +} +END_TEST + START_TEST(test_sock_connect_tcp_src_port_low) { diff --git a/src/wolfip.c b/src/wolfip.c index 36096c96..2b6f98ed 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -9034,8 +9034,14 @@ static int dhcp_send_request(struct wolfIP *s) /* Prepare DHCP request */ memset(&req, 0, sizeof(struct dhcp_msg)); req.op = BOOT_REQUEST; - if (!renewing && !rebinding) + if (!renewing && !rebinding) { s->dhcp_state = DHCP_REQUEST_SENT; + /* RFC 2131 4.4.1: with no bound IP the client cannot receive a + * unicast reply, so set the BROADCAST bit and ask the server to + * broadcast the DHCPACK. RENEWING/REBINDING hold a bound IP and + * leave the bit clear for a unicast reply. */ + req.flags = ee16(0x8000); + } req.htype = 1; /* Ethernet */ req.hlen = 6; /* MAC */ req.xid = ee32(s->dhcp_xid); @@ -9138,6 +9144,10 @@ static int dhcp_send_discover(struct wolfIP *s) /* Prepare DHCP discover */ memset(&disc, 0, sizeof(struct dhcp_msg)); disc.op = BOOT_REQUEST; + /* RFC 2131 4.4.1: a discovering client has no IP address and cannot + * receive a unicast reply, so set the BROADCAST bit and ask the server + * to broadcast the DHCPOFFER. */ + disc.flags = ee16(0x8000); disc.htype = 1; /* Ethernet */ disc.hlen = 6; /* MAC */ disc.xid = ee32(s->dhcp_xid); From e58a377418af3574bc6e7bea34c0e9a308482c84 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 16:07:13 +0200 Subject: [PATCH 29/35] F-8526: make DHCPACK config commit atomic 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)., --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_dns_dhcp.c | 70 +++++++++++++++++++++++++++++ src/wolfip.c | 58 ++++++++++++++++-------- 3 files changed, 110 insertions(+), 19 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 4f17a6ee..2283ae1c 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -493,6 +493,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_dhcp_parse_ack_ignores_zero_len_unknown_option); tcase_add_test(tc_utils, test_dhcp_parse_ack_missing_server_id_rejected); tcase_add_test(tc_utils, test_dhcp_parse_ack_missing_end_rejected); + tcase_add_test(tc_utils, test_dhcp_parse_ack_reject_preserves_config); tcase_add_test(tc_utils, test_dhcp_parse_offer_bad_magic_rejected); tcase_add_test(tc_utils, test_dhcp_parse_ack_bad_magic_rejected); tcase_add_test(tc_utils, test_dhcp_parse_offer_rejects_boot_request_op); diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index aea5ef58..2c5e7be4 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -1272,6 +1272,76 @@ START_TEST(test_dhcp_request_broadcast_flag_by_state) END_TEST +/* Atomicity: a rejected DHCPACK must not leave a partial network + * configuration behind. This ACK carries valid IP/mask/gw/DNS options + * (all different from the live values) but omits the mandatory + * lease-time option (51), so dhcp_parse_ack() rejects it. The live + * ipconf and dns_server must be left exactly as they were. Pre-fix, the + * inline option writes committed the new values before the lease-time + * check failed, so the rejected ACK corrupted the running config. */ +START_TEST(test_dhcp_parse_ack_reject_preserves_config) +{ + struct wolfIP s; + struct ipconf *primary; + struct dhcp_msg msg; + struct dhcp_option *opt; + uint32_t ip_before, mask_before, gw_before, dns_before, srv_before; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0x0A000064U); + primary = wolfIP_primary_ipconf(&s); + ck_assert_ptr_nonnull(primary); + + /* Live configuration to preserve. */ + ip_before = primary->ip; /* 0x0A000001 */ + mask_before = primary->mask; /* 0xFFFFFF00 */ + gw_before = primary->gw; /* 0x0A000064 */ + s.dns_server = 0x08080808U; + dns_before = s.dns_server; + s.dhcp_server_ip = 0x0A000064U; + srv_before = s.dhcp_server_ip; + s.dhcp_xid = 0x12345678U; + + /* Build an ACK offering DIFFERENT config values, with a matching + * server-id (so the identity check passes) but no lease time. */ + memset(&msg, 0, sizeof(msg)); + msg.op = BOOT_REPLY; + msg.xid = ee32(0x12345678U); + msg.magic = ee32(DHCP_MAGIC); + opt = (struct dhcp_option *)msg.options; + opt->code = DHCP_OPTION_MSG_TYPE; opt->len = 1; opt->data[0] = DHCP_ACK; + opt = (struct dhcp_option *)((uint8_t *)opt + 3); + opt->code = DHCP_OPTION_SERVER_ID; opt->len = 4; + DHCP_OPT_u32_to_data(opt, 0x0A000064U); + opt = (struct dhcp_option *)((uint8_t *)opt + 6); + opt->code = DHCP_OPTION_OFFER_IP; opt->len = 4; + DHCP_OPT_u32_to_data(opt, 0x0A0000AAU); /* != ip_before */ + opt = (struct dhcp_option *)((uint8_t *)opt + 6); + opt->code = DHCP_OPTION_SUBNET_MASK; opt->len = 4; + DHCP_OPT_u32_to_data(opt, 0xFFFFFE00U); /* != mask_before */ + opt = (struct dhcp_option *)((uint8_t *)opt + 6); + opt->code = DHCP_OPTION_ROUTER; opt->len = 4; + DHCP_OPT_u32_to_data(opt, 0x0A000099U); /* != gw_before */ + opt = (struct dhcp_option *)((uint8_t *)opt + 6); + opt->code = DHCP_OPTION_DNS; opt->len = 4; + DHCP_OPT_u32_to_data(opt, 0x01010101U); /* != dns_before */ + opt = (struct dhcp_option *)((uint8_t *)opt + 6); + opt->code = DHCP_OPTION_END; opt->len = 0; + + /* No lease time -> rejected. */ + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), -1); + + /* Live config must be untouched. */ + ck_assert_uint_eq(primary->ip, ip_before); + ck_assert_uint_eq(primary->mask, mask_before); + ck_assert_uint_eq(primary->gw, gw_before); + ck_assert_uint_eq(s.dns_server, dns_before); + ck_assert_uint_eq(s.dhcp_server_ip, srv_before); +} +END_TEST + + START_TEST(test_sock_connect_tcp_src_port_low) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 2b6f98ed..e82f0192 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -8815,12 +8815,24 @@ static int dhcp_parse_ack(struct wolfIP *s, struct dhcp_msg *msg, uint32_t msg_l struct dhcp_opt_stream st; int saw_end = 0; int saw_server_id = 0; - int saw_offer_ip = 0; struct ipconf *primary = wolfIP_primary_ipconf(s); uint32_t lease_ip = 0; + uint32_t lease_mask = 0; uint32_t lease_s = 0; uint32_t renew_s = 0; uint32_t rebind_s = 0; + /* Candidate configuration values: the option stream is parsed into + * these first and committed atomically only on full success, so a + * rejected (malformed or incomplete) ACK never leaves a partial + * network configuration behind. */ + uint32_t cand_ip = 0; + uint32_t cand_mask = 0; + uint32_t cand_gw = 0; + uint32_t cand_dns = 0; + uint32_t cand_server_ip = 0; + int have_ip = 0; + int have_mask = 0; + int have_gw = 0; if (msg_len < DHCP_HEADER_LEN) return -1; if (msg->op != BOOT_REPLY) @@ -8867,29 +8879,28 @@ static int dhcp_parse_ack(struct wolfIP *s, struct dhcp_msg *msg, uint32_t msg_l * we committed to during the OFFER phase. */ if (s->dhcp_server_ip != 0 && val != s->dhcp_server_ip) return -1; - s->dhcp_server_ip = val; + cand_server_ip = val; saw_server_id = 1; } else if (code == DHCP_OPTION_OFFER_IP) { if (len < 4) return -1; - val = DHCP_OPT_data_to_u32((struct dhcp_option *)idata); - lease_ip = val; - saw_offer_ip = 1; + cand_ip = DHCP_OPT_data_to_u32((struct dhcp_option *)idata); + have_ip = 1; } else if (primary && code == DHCP_OPTION_SUBNET_MASK) { if (len < 4) return -1; - val = DHCP_OPT_data_to_u32((struct dhcp_option *)idata); - primary->mask = val; + cand_mask = DHCP_OPT_data_to_u32((struct dhcp_option *)idata); + have_mask = 1; } else if (primary && code == DHCP_OPTION_ROUTER) { if (len < 4) return -1; - val = DHCP_OPT_data_to_u32((struct dhcp_option *)idata); - primary->gw = val; + cand_gw = DHCP_OPT_data_to_u32((struct dhcp_option *)idata); + have_gw = 1; } else if ((code == DHCP_OPTION_DNS) && (s->dns_server == 0)) { if (len < 4) return -1; - val = DHCP_OPT_data_to_u32((struct dhcp_option *)idata); - s->dns_server = val; + if (cand_dns == 0) + cand_dns = DHCP_OPT_data_to_u32((struct dhcp_option *)idata); } else if (code == DHCP_OPTION_LEASE_TIME) { if (len < 4) return -1; @@ -8909,21 +8920,30 @@ static int dhcp_parse_ack(struct wolfIP *s, struct dhcp_msg *msg, uint32_t msg_l /* The lease address is option 50 (the requested IP) when the * server echoes it, otherwise the yiaddr it committed; either * way it must be a usable unicast address before it is applied - * to the interface. The offered netmask applies when the ACK - * carries none. */ - if (!saw_offer_ip) - lease_ip = ee32(msg->yiaddr); - if (primary && primary->mask == 0) - primary->mask = s->dhcp_offered_mask; + * to the interface. The netmask is the ACK's when it carries + * one, else the interface's current mask, else the one + * recorded during the OFFER phase. Both are effective values + * computed here, never written until the commit below. */ + lease_ip = have_ip ? cand_ip : ee32(msg->yiaddr); + lease_mask = have_mask ? cand_mask + : ((primary && primary->mask != 0) + ? primary->mask : s->dhcp_offered_mask); /* RFC 2131: the IP-address-lease-time option (51) is mandatory * in a DHCPACK. lease_s is only ever set by that option (and a * short option already returns -1 above), so lease_s != 0 means * it was present with a valid nonzero duration. Without it the * lease would be bound with no expiry/renewal timer. */ if (primary && saw_server_id && lease_s != 0 && - (primary->mask != 0) && - dhcp_lease_ip_sane(lease_ip, primary->mask)) { + (lease_mask != 0) && + dhcp_lease_ip_sane(lease_ip, lease_mask)) { + /* Commit the validated configuration atomically. */ + s->dhcp_server_ip = cand_server_ip; primary->ip = lease_ip; + primary->mask = lease_mask; + if (have_gw) + primary->gw = cand_gw; + if (s->dns_server == 0 && cand_dns != 0) + s->dns_server = cand_dns; dhcp_cancel_timer(s); s->dhcp_ip = primary->ip; #ifdef ETHERNET From 44c7204b7a32dad9dcc467c6d46f743122569b73 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 16:26:20 +0200 Subject: [PATCH 30/35] F-10279: make the forwarding filter notification IHL-aware 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)., --- src/test/unit/unit.c | 2 + src/test/unit/unit_tests_api.c | 98 +++++++++++++++++++++++++++++++- src/wolfip.c | 100 ++++++++++++++++++++++++--------- 3 files changed, 174 insertions(+), 26 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 2283ae1c..57e132b0 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -176,6 +176,8 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_wolfip_poll_preserves_tcp_events_raised_during_callback); tcase_add_test(tc_utils, test_wolfip_poll_limits_device_drain_to_poll_budget); tcase_add_test(tc_utils, test_filter_notify_tcp_metadata); + tcase_add_test(tc_utils, test_filter_notify_udp_ihl_options_metadata); + tcase_add_test(tc_utils, test_filter_notify_udp_ihl_truncated_no_overread); tcase_add_test(tc_utils, test_filter_dispatch_no_callback); tcase_add_test(tc_utils, test_filter_dispatch_mask_not_set); tcase_add_test(tc_utils, test_filter_fresh_callback_consulted_before_mask_configured); diff --git a/src/test/unit/unit_tests_api.c b/src/test/unit/unit_tests_api.c index 331486b2..d681edf5 100644 --- a/src/test/unit/unit_tests_api.c +++ b/src/test/unit/unit_tests_api.c @@ -186,7 +186,8 @@ START_TEST(test_filter_notify_tcp_metadata) wolfIP_filter_set_callback(test_filter_cb, NULL); wolfIP_filter_set_mask(WOLFIP_FILT_MASK(WOLFIP_FILT_SENDING)); - (void)wolfIP_filter_notify_tcp(WOLFIP_FILT_SENDING, &s, 1, &tcp, sizeof(tcp)); + (void)wolfIP_filter_notify_tcp(WOLFIP_FILT_SENDING, &s, 1, &tcp, sizeof(tcp), + IP_HEADER_LEN); ck_assert_int_eq(filter_cb_calls, 1); ck_assert_uint_eq(filter_last_event.meta.ip_proto, WOLFIP_FILTER_PROTO_TCP); ck_assert_uint_eq(filter_last_event.meta.l4.tcp.src_port, tcp.src_port); @@ -197,6 +198,101 @@ START_TEST(test_filter_notify_tcp_metadata) } END_TEST +/* IHL-aware filter metadata: a forwarded datagram keeps its IP options + * (IHL > 5) and the filter notify must read the transport header at the + * actual IHL, not at a fixed 20-byte offset. This UDP datagram carries a + * 24-byte IP header (IHL=6) with four option bytes; the UDP header (the + * ports) follows the options. Pre-fix, the notify sampled the option + * bytes as the ports, misrepresenting them to any L4 filter and + * (for a header-only datagram) over-reading the received bytes. */ +START_TEST(test_filter_notify_udp_ihl_options_metadata) +{ + struct wolfIP s; + uint8_t buf[ETH_HEADER_LEN + 24 + UDP_HEADER_LEN + 4]; + uint8_t *ip; + uint8_t *udp; + uint16_t src_port = 1234; + uint16_t dst_port = 5678; + + memset(&s, 0, sizeof(s)); + memset(buf, 0, sizeof(buf)); + + /* IP header at offset ETH_HEADER_LEN, IHL=6 (24 bytes). */ + ip = buf + ETH_HEADER_LEN; + ip[0] = 0x46; /* ver=4, IHL=6 */ + ip[2] = 0; ip[3] = 36; /* total IP length: 24 + 8 + 4 */ + ip[8] = 64; /* ttl */ + ip[9] = WI_IPPROTO_UDP; /* proto */ + ip[12] = 0x0A; ip[13] = 0; ip[14] = 0; ip[15] = 1; /* src 10.0.0.1 */ + ip[16] = 0x0A; ip[17] = 0; ip[18] = 0; ip[19] = 2; /* dst 10.0.0.2 */ + /* Four option bytes (NOPs) at IP-header offset 20. A fixed-20-byte + * transport offset would read these as the UDP ports. */ + ip[20] = 0x01; ip[21] = 0x01; ip[22] = 0x01; ip[23] = 0x01; + + /* UDP header at IP-header offset 24 (after the options). */ + udp = buf + ETH_HEADER_LEN + 24; + udp[0] = (uint8_t)(src_port >> 8); udp[1] = (uint8_t)(src_port & 0xFF); + udp[2] = (uint8_t)(dst_port >> 8); udp[3] = (uint8_t)(dst_port & 0xFF); + udp[4] = 0; udp[5] = (uint8_t)(UDP_HEADER_LEN + 4); /* udp len */ + udp[6] = 0; udp[7] = 0; /* csum */ + buf[ETH_HEADER_LEN + 24 + UDP_HEADER_LEN] = 0xAA; /* 4 payload bytes */ + + filter_cb_calls = 0; + memset(&filter_last_event, 0, sizeof(filter_last_event)); + wolfIP_filter_set_callback(test_filter_cb, NULL); + wolfIP_filter_set_mask(WOLFIP_FILT_MASK(WOLFIP_FILT_SENDING)); + + (void)wolfIP_filter_notify_udp(WOLFIP_FILT_SENDING, &s, 1, + (struct wolfIP_udp_datagram *)buf, + sizeof(buf), 24); + ck_assert_int_eq(filter_cb_calls, 1); + ck_assert_uint_eq(filter_last_event.meta.ip_proto, WOLFIP_FILTER_PROTO_UDP); + /* The ports must come from the UDP header after the options, not from + * the option bytes. */ + ck_assert_uint_eq(filter_last_event.meta.l4.udp.src_port, ee16(src_port)); + ck_assert_uint_eq(filter_last_event.meta.l4.udp.dst_port, ee16(dst_port)); + + wolfIP_filter_set_callback(NULL, NULL); +} +END_TEST + + +/* The IHL-aware guard: a header-only (or truncated) datagram must not + * over-read the received bytes when the transport header is not fully + * present after the actual IHL. The notify must skip the L4 metadata + * (no callback) rather than read past the buffer. */ +START_TEST(test_filter_notify_udp_ihl_truncated_no_overread) +{ + struct wolfIP s; + uint8_t buf[ETH_HEADER_LEN + 24]; /* eth + 24-byte IP header only */ + uint8_t *ip; + + memset(&s, 0, sizeof(s)); + memset(buf, 0, sizeof(buf)); + + ip = buf + ETH_HEADER_LEN; + ip[0] = 0x46; /* ver=4, IHL=6 */ + ip[2] = 0; ip[3] = 24; /* total IP length: just the header */ + ip[8] = 64; + ip[9] = WI_IPPROTO_UDP; + + filter_cb_calls = 0; + memset(&filter_last_event, 0, sizeof(filter_last_event)); + wolfIP_filter_set_callback(test_filter_cb, NULL); + wolfIP_filter_set_mask(WOLFIP_FILT_MASK(WOLFIP_FILT_SENDING)); + + /* Only the IP header is present; the UDP header is not. The notify + * must not fire (no transport bytes to read). */ + (void)wolfIP_filter_notify_udp(WOLFIP_FILT_SENDING, &s, 1, + (struct wolfIP_udp_datagram *)buf, + sizeof(buf), 24); + ck_assert_int_eq(filter_cb_calls, 0); + + wolfIP_filter_set_callback(NULL, NULL); +} +END_TEST + + START_TEST(test_filter_dispatch_no_callback) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index e82f0192..4e1b4e57 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -1014,46 +1014,73 @@ static int wolfIP_filter_notify_ip(enum wolfIP_filter_reason reason, static int wolfIP_filter_notify_tcp(enum wolfIP_filter_reason reason, struct wolfIP *s, unsigned int if_idx, - const struct wolfIP_tcp_seg *tcp, uint32_t len) + const struct wolfIP_tcp_seg *tcp, uint32_t len, + uint32_t ip_hlen) { struct wolfIP_filter_metadata meta; + const uint8_t *tp; + uint16_t v; wolfIP_filter_init_metadata(&meta); wolfIP_filter_fill_ip_metadata(&meta, &tcp->ip); meta.ip_proto = WOLFIP_FILTER_PROTO_TCP; - meta.l4.tcp.src_port = tcp->src_port; - meta.l4.tcp.dst_port = tcp->dst_port; - meta.l4.tcp.flags = tcp->flags; + /* The transport header follows the IP header at its actual length + * (ip_hlen), not at a fixed 20-byte offset: a forwarded datagram may + * carry IP options (IHL > 5) that are not stripped before the filter + * is notified, so reading the struct members would sample the option + * bytes. The guard keeps a header-only or truncated datagram from + * over-reading the received bytes. */ + if (len < (uint32_t)(ETH_HEADER_LEN + ip_hlen + TCP_HEADER_LEN)) + return 0; + tp = (const uint8_t *)tcp + ETH_HEADER_LEN + ip_hlen; + memcpy(&v, tp + 0, sizeof(v)); + meta.l4.tcp.src_port = v; + memcpy(&v, tp + 2, sizeof(v)); + meta.l4.tcp.dst_port = v; + meta.l4.tcp.flags = tp[13]; return wolfIP_filter_dispatch(reason, s, if_idx, tcp, len, &meta); } static int wolfIP_filter_notify_udp(enum wolfIP_filter_reason reason, struct wolfIP *s, unsigned int if_idx, - const struct wolfIP_udp_datagram *udp, uint32_t len) + const struct wolfIP_udp_datagram *udp, uint32_t len, + uint32_t ip_hlen) { struct wolfIP_filter_metadata meta; + const uint8_t *tp; + uint16_t v; wolfIP_filter_init_metadata(&meta); wolfIP_filter_fill_ip_metadata(&meta, &udp->ip); meta.ip_proto = WOLFIP_FILTER_PROTO_UDP; - meta.l4.udp.src_port = udp->src_port; - meta.l4.udp.dst_port = udp->dst_port; + if (len < (uint32_t)(ETH_HEADER_LEN + ip_hlen + UDP_HEADER_LEN)) + return 0; + tp = (const uint8_t *)udp + ETH_HEADER_LEN + ip_hlen; + memcpy(&v, tp + 0, sizeof(v)); + meta.l4.udp.src_port = v; + memcpy(&v, tp + 2, sizeof(v)); + meta.l4.udp.dst_port = v; return wolfIP_filter_dispatch(reason, s, if_idx, udp, len, &meta); } static int wolfIP_filter_notify_icmp(enum wolfIP_filter_reason reason, struct wolfIP *s, unsigned int if_idx, - const struct wolfIP_icmp_packet *icmp, uint32_t len) + const struct wolfIP_icmp_packet *icmp, uint32_t len, + uint32_t ip_hlen) { struct wolfIP_filter_metadata meta; + const uint8_t *tp; wolfIP_filter_init_metadata(&meta); wolfIP_filter_fill_ip_metadata(&meta, &icmp->ip); meta.ip_proto = WOLFIP_FILTER_PROTO_ICMP; - meta.l4.icmp.type = icmp->type; - meta.l4.icmp.code = icmp->code; + if (len < (uint32_t)(ETH_HEADER_LEN + ip_hlen + ICMP_HEADER_LEN)) + return 0; + tp = (const uint8_t *)icmp + ETH_HEADER_LEN + ip_hlen; + meta.l4.icmp.type = tp[0]; + meta.l4.icmp.code = tp[1]; return wolfIP_filter_dispatch(reason, s, if_idx, icmp, len, &meta); } @@ -2309,7 +2336,8 @@ static void wolfIP_send_ttl_exceeded(struct wolfIP *s, unsigned int if_idx, if (!wolfIP_ll_is_non_ethernet(s, if_idx)) { eth_output_add_header(s, if_idx, orig->eth.src, &icmp.ip.eth, ETH_TYPE_IP); } - if (wolfIP_filter_notify_icmp(WOLFIP_FILT_SENDING, s, if_idx, icmp_pkt, frame_len) != 0) + if (wolfIP_filter_notify_icmp(WOLFIP_FILT_SENDING, s, if_idx, icmp_pkt, + frame_len, IP_HEADER_LEN) != 0) return; if (wolfIP_filter_notify_ip(WOLFIP_FILT_SENDING, s, if_idx, &icmp.ip, frame_len) != 0) return; @@ -2396,7 +2424,8 @@ static void wolfIP_send_port_unreachable(struct wolfIP *s, unsigned int if_idx, if (!wolfIP_ll_is_non_ethernet(s, if_idx)) { eth_output_add_header(s, if_idx, orig->eth.src, &icmp.ip.eth, ETH_TYPE_IP); } - if (wolfIP_filter_notify_icmp(WOLFIP_FILT_SENDING, s, if_idx, icmp_pkt, frame_len) != 0) + if (wolfIP_filter_notify_icmp(WOLFIP_FILT_SENDING, s, if_idx, icmp_pkt, + frame_len, IP_HEADER_LEN) != 0) return; if (wolfIP_filter_notify_ip(WOLFIP_FILT_SENDING, s, if_idx, &icmp.ip, frame_len) != 0) return; @@ -2621,7 +2650,8 @@ static void udp_try_recv(struct wolfIP *s, unsigned int if_idx, dst_ip = ee32(udp->ip.dst); src_ip = ee32(udp->ip.src); - if (wolfIP_filter_notify_udp(WOLFIP_FILT_RECEIVING, s, if_idx, udp, frame_len) != 0) + if (wolfIP_filter_notify_udp(WOLFIP_FILT_RECEIVING, s, if_idx, udp, frame_len, + IP_HEADER_LEN) != 0) return; for (i = 0; i < MAX_UDPSOCKETS; i++) { struct tsocket *t = &s->udpsockets[i]; @@ -3489,7 +3519,8 @@ static int tcp_send_empty_immediate(struct tsocket *t, struct wolfIP_tcp_seg *tc eth_output_add_header(t->S, tx_if, t->nexthop_mac, &tcp->ip.eth, ETH_TYPE_IP); #endif - if (wolfIP_filter_notify_tcp(WOLFIP_FILT_SENDING, t->S, tx_if, tcp, frame_len) != 0) + if (wolfIP_filter_notify_tcp(WOLFIP_FILT_SENDING, t->S, tx_if, tcp, frame_len, + IP_HEADER_LEN) != 0) return -1; if (wolfIP_filter_notify_ip(WOLFIP_FILT_SENDING, t->S, tx_if, &tcp->ip, frame_len) != 0) return -1; @@ -3675,7 +3706,8 @@ static void tcp_send_reset_reply(struct wolfIP *s, unsigned int if_idx, return; } #endif - if (wolfIP_filter_notify_tcp(WOLFIP_FILT_SENDING, s, if_idx, out, out_len) != 0) + if (wolfIP_filter_notify_tcp(WOLFIP_FILT_SENDING, s, if_idx, out, out_len, + IP_HEADER_LEN) != 0) return; if (wolfIP_filter_notify_ip(WOLFIP_FILT_SENDING, s, if_idx, &out->ip, out_len) != 0) return; @@ -4156,7 +4188,8 @@ static int tcp_send_zero_wnd_probe(struct tsocket *t) eth_output_add_header(t->S, tx_if, t->nexthop_mac, &probe->ip.eth, ETH_TYPE_IP); #endif - if (wolfIP_filter_notify_tcp(WOLFIP_FILT_SENDING, t->S, tx_if, probe, frame_len) != 0) + if (wolfIP_filter_notify_tcp(WOLFIP_FILT_SENDING, t->S, tx_if, probe, frame_len, + IP_HEADER_LEN) != 0) return -1; if (wolfIP_filter_notify_ip(WOLFIP_FILT_SENDING, t->S, tx_if, &probe->ip, frame_len) != 0) return -1; @@ -4619,6 +4652,14 @@ static void wolfIP_forward_packet(struct wolfIP *s, unsigned int out_if, { #ifdef ETHERNET int drop = 0; + /* The forwarded datagram keeps its IP options (a router relays the + * packet as-is; stripping them here would alter the relayed packet), + * so the filter notify below must read the transport header at the + * actual IHL, not a fixed 20-byte offset. */ + uint32_t ip_hlen = (uint32_t)(ip->ver_ihl & 0x0fU) << 2; + + if (ip_hlen < IP_HEADER_LEN) + ip_hlen = IP_HEADER_LEN; if (!wolfIP_ll_is_non_ethernet(s, out_if)) { if (broadcast) eth_output_add_header(s, out_if, NULL, &ip->eth, ETH_TYPE_IP); @@ -4627,13 +4668,16 @@ static void wolfIP_forward_packet(struct wolfIP *s, unsigned int out_if, } if (ip->proto == WI_IPPROTO_TCP) drop = wolfIP_filter_notify_tcp(WOLFIP_FILT_SENDING, s, out_if, - (struct wolfIP_tcp_seg *)ip, len); + (struct wolfIP_tcp_seg *)ip, len, + ip_hlen); else if (ip->proto == WI_IPPROTO_UDP) drop = wolfIP_filter_notify_udp(WOLFIP_FILT_SENDING, s, out_if, - (struct wolfIP_udp_datagram *)ip, len); + (struct wolfIP_udp_datagram *)ip, len, + ip_hlen); else if (ip->proto == WI_IPPROTO_ICMP) drop = wolfIP_filter_notify_icmp(WOLFIP_FILT_SENDING, s, out_if, - (struct wolfIP_icmp_packet *)ip, len); + (struct wolfIP_icmp_packet *)ip, len, + ip_hlen); if (drop != 0) return; if (wolfIP_filter_notify_ip(WOLFIP_FILT_SENDING, s, out_if, ip, len) != 0) @@ -5293,7 +5337,8 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, return; } - if (wolfIP_filter_notify_tcp(WOLFIP_FILT_RECEIVING, S, if_idx, tcp, frame_len) != 0) + if (wolfIP_filter_notify_tcp(WOLFIP_FILT_RECEIVING, S, if_idx, tcp, frame_len, + IP_HEADER_LEN) != 0) return; for (i = 0; i < MAX_TCPSOCKETS; i++) { uint32_t tcplen; @@ -8117,7 +8162,8 @@ static void icmp_input(struct wolfIP *s, unsigned int if_idx, struct wolfIP_ip_p if (icmp_checksum(icmp, (uint16_t)(ee16(ip->len) - IP_HEADER_LEN)) != 0) return; - if (wolfIP_filter_notify_icmp(WOLFIP_FILT_RECEIVING, s, if_idx, icmp, len) != 0) + if (wolfIP_filter_notify_icmp(WOLFIP_FILT_RECEIVING, s, if_idx, icmp, len, + IP_HEADER_LEN) != 0) return; if (icmp->type == ICMP_ECHO_REPLY) { ip4 dst = ee32(ip->dst); @@ -8170,7 +8216,8 @@ static void icmp_input(struct wolfIP *s, unsigned int if_idx, struct wolfIP_ip_p eth_output_add_header(s, if_idx, ip->eth.src, &ip->eth, ETH_TYPE_IP); } #endif - if (wolfIP_filter_notify_icmp(WOLFIP_FILT_SENDING, s, if_idx, icmp, len) != 0) + if (wolfIP_filter_notify_icmp(WOLFIP_FILT_SENDING, s, if_idx, icmp, len, + IP_HEADER_LEN) != 0) return; if (wolfIP_filter_notify_ip(WOLFIP_FILT_SENDING, s, if_idx, ip, len) != 0) return; @@ -11245,7 +11292,8 @@ static void flush_tcp_tx(struct wolfIP *s, uint64_t now) if (!wolfIP_ll_is_non_ethernet(ts->S, tx_if)) eth_output_add_header(ts->S, tx_if, ts->nexthop_mac, &tcp->ip.eth, ETH_TYPE_IP); #endif - if (wolfIP_filter_notify_tcp(WOLFIP_FILT_SENDING, ts->S, tx_if, tcp, desc->len) != 0) { + if (wolfIP_filter_notify_tcp(WOLFIP_FILT_SENDING, ts->S, tx_if, tcp, desc->len, + IP_HEADER_LEN) != 0) { break; } if (wolfIP_filter_notify_ip(WOLFIP_FILT_SENDING, ts->S, tx_if, &tcp->ip, desc->len) != 0) { @@ -11388,11 +11436,13 @@ static void flush_datagram_tx(struct wolfIP *s, struct tsocket *socks, if (is_udp) { if (wolfIP_filter_notify_udp(WOLFIP_FILT_SENDING, s, tx_if, - (struct wolfIP_udp_datagram *)ip, desc->len) != 0) + (struct wolfIP_udp_datagram *)ip, desc->len, + IP_HEADER_LEN) != 0) break; } else { if (wolfIP_filter_notify_icmp(WOLFIP_FILT_SENDING, s, tx_if, - (struct wolfIP_icmp_packet *)ip, desc->len) != 0) + (struct wolfIP_icmp_packet *)ip, desc->len, + IP_HEADER_LEN) != 0) break; } if (wolfIP_filter_notify_ip(WOLFIP_FILT_SENDING, s, tx_if, ip, desc->len) != 0) From e30d7dca1b3c70502fe033e33c41537760d31d62 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 16:35:31 +0200 Subject: [PATCH 31/35] F-8559: correct the source-route drop rationale comment 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., --- src/wolfip.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/wolfip.c b/src/wolfip.c index 4e1b4e57..b26b6e13 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -10129,9 +10129,15 @@ static inline void ip_recv(struct wolfIP *s, unsigned int if_idx, * so tapping ahead of the LSRR/SSRR drop cannot make the stack act on one. */ raw_try_recv(s, if_idx, ip, len); #endif - /* RFC 7126 section 3.8: drop source-routed (LSRR/SSRR) packets before either - * forwarding or local-delivery dispatch. - * */ + /* Drop source-routed (LSRR/SSRR) packets before either forwarding or + * local-delivery dispatch. RFC 7126 (IP Security Considerations) 4.3.5 + * (LSRR) and 4.4.5 (SSRR) recommend an option-specific configuration + * knob whose DEFAULT setting SHOULD be "drop"; RFC 6274 notes the + * security implications (firewall bypass, stealthy connections, network + * topology disclosure, bandwidth-exhaustion attacks) outweigh any + * legitimate use. wolfIP implements that secure default unconditionally: + * source-routed packets are always dropped. This is deliberate + * hardening for a minimal embedded stack, not a parsing limitation. */ if (ip_hlen > IP_HEADER_LEN) { uint8_t *opt = ((uint8_t *)ip) + ETH_HEADER_LEN + IP_HEADER_LEN; uint8_t *opt_end = opt + (ip_hlen - IP_HEADER_LEN); From a4197a59afa15ac9566e00ee812f0eef0c65b8a6 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 17:24:03 +0200 Subject: [PATCH 32/35] F-9808: make timer_binheap_cancel eager to stop dead-slot heap exhaustion 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)., --- src/test/unit/unit.c | 3 +- src/test/unit/unit_tests_branches.c | 4 +- src/test/unit/unit_tests_dhcp_edges.c | 7 ++- src/test/unit/unit_tests_dns_dhcp.c | 13 ++--- src/test/unit/unit_tests_poll_dispatcher.c | 11 ++-- src/test/unit/unit_tests_proto.c | 65 ++++++++++++++++------ src/test/unit/unit_tests_tcp_ack.c | 13 ++--- src/wolfip.c | 51 ++++++++++++++++- 8 files changed, 125 insertions(+), 42 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 57e132b0..1145b677 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -132,7 +132,8 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_pop_timer); tcase_add_test(tc_utils, test_is_timer_expired); tcase_add_test(tc_utils, test_cancel_timer); - tcase_add_test(tc_utils, test_timer_pop_removes_zero_head_first); + tcase_add_test(tc_utils, test_timer_heap_no_dead_slot_accumulation); + tcase_add_test(tc_utils, test_timer_pop_skips_zero_expires); tcase_add_test(tc_utils, test_timer_pop_reorders_heap); tcase_add_test(tc_utils, test_timer_pop_right_child_swap); tcase_add_test(tc_utils, test_timer_pop_break_when_root_small); diff --git a/src/test/unit/unit_tests_branches.c b/src/test/unit/unit_tests_branches.c index 935fd96e..64f616cb 100644 --- a/src/test/unit/unit_tests_branches.c +++ b/src/test/unit/unit_tests_branches.c @@ -1137,11 +1137,11 @@ START_TEST(test_timer_binheap_drain_keeps_live_timer) ck_assert_int_gt(ida, 0); ck_assert_int_gt(idb, 0); - /* Cancel the earliest timer, leaving a tombstone at the heap root. */ + /* Cancel the earliest timer (eager: slot removed physically). */ timer_binheap_cancel(&heap, (uint32_t)ida); ck_assert_int_eq(is_timer_expired(&heap, 50), 0); - /* Draining the tombstone must leave the next live timer in place. */ + /* The surviving live timer must still be in place. */ ck_assert_int_eq((int)heap.size, 1); got = timers_binheap_pop(&heap); ck_assert_uint_eq(got.id, (uint32_t)idb); diff --git a/src/test/unit/unit_tests_dhcp_edges.c b/src/test/unit/unit_tests_dhcp_edges.c index 810ccff6..3099dc2b 100644 --- a/src/test/unit/unit_tests_dhcp_edges.c +++ b/src/test/unit/unit_tests_dhcp_edges.c @@ -1301,9 +1301,12 @@ START_TEST(test_dhcp_lease_expiry_relearns_dns_server) ck_assert_uint_eq(s.dns_server, first_dns); ck_assert_uint_ne(s.dhcp_lease_expires, 0U); - /* The lease expires: every parameter it carried is released. */ + /* The lease expires: every parameter it carried is released. Drive it + * through handle_timers so the renew timer is popped before the + * callback runs (exactly as in production), leaving no orphaned timer + * behind. */ s.last_tick = s.dhcp_lease_expires; - dhcp_timer_cb(&s); + handle_timers(&s, s.last_tick); ck_assert_int_eq(s.dhcp_state, DHCP_DISCOVER_SENT); ck_assert_uint_eq(primary->ip, 0U); ck_assert_uint_eq(primary->gw, 0U); diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index 2c5e7be4..a2455c52 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -5652,7 +5652,7 @@ START_TEST(test_sock_close_tcp_cancels_rto_timer) int sd; uint32_t rto_id; uint32_t i; - int found_canceled = 0; + int found = 0; wolfIP_init(&s); mock_link_init(&s); @@ -5672,14 +5672,13 @@ START_TEST(test_sock_close_tcp_cancels_rto_timer) ck_assert_int_eq(wolfIP_sock_close(&s, sd), 0); ck_assert_int_eq(ts->proto, 0); + /* Eager cancel removes the RTO timer physically; it must no longer be + * present in the heap. */ for (i = 0; i < s.timers.size; i++) { - if (s.timers.timers[i].id == rto_id) { - found_canceled = 1; - ck_assert_uint_eq(s.timers.timers[i].expires, 0); - break; - } + if (s.timers.timers[i].id == rto_id) + found = 1; } - ck_assert_int_eq(found_canceled, 1); + ck_assert_int_eq(found, 0); } END_TEST diff --git a/src/test/unit/unit_tests_poll_dispatcher.c b/src/test/unit/unit_tests_poll_dispatcher.c index dc169bd8..00b9be4f 100644 --- a/src/test/unit/unit_tests_poll_dispatcher.c +++ b/src/test/unit/unit_tests_poll_dispatcher.c @@ -264,13 +264,13 @@ START_TEST(test_poll_timer_cancelled_tombstone_drained_before_live_timer) mock_link_init(&s); timer_cb_calls = 0; - /* Insert one timer and cancel it immediately — tombstone only */ + /* Insert one timer and cancel it immediately (eager: gone physically) */ memset(&tmr, 0, sizeof(tmr)); tmr.cb = test_timer_cb; tmr.expires = 50; handle = timers_binheap_insert(&s.timers, tmr); - /* Cancel to create a tombstone */ + /* Cancel removes the slot */ timer_binheap_cancel(&s.timers, handle); /* Insert a second live timer after the cancelled one */ @@ -279,11 +279,10 @@ START_TEST(test_poll_timer_cancelled_tombstone_drained_before_live_timer) tmr.expires = 60; timers_binheap_insert(&s.timers, tmr); - /* Poll at t=100: the tombstone is at the heap head; is_timer_expired - * drains it via timers_binheap_pop which also consumes the next timer. - * Verify poll does not crash and heap is empty after draining. */ + /* Poll at t=100: the live timer is due and is consumed. + * Verify poll does not crash and the heap is empty afterwards. */ (void)wolfIP_poll(&s, 100); - /* Heap must be empty after tombstone draining */ + /* Heap must be empty after the timer fired */ ck_assert_uint_eq(s.timers.size, 0U); } END_TEST diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index 690f2266..7bc96687 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -2371,9 +2371,12 @@ START_TEST(test_timer_cancel_existing_and_missing) ck_assert_uint_eq(local.timers[0].expires, 5); timer_binheap_cancel(&local, id); - ck_assert_uint_eq(local.timers[0].expires, 0); + /* Eager cancel removes the slot physically; the heap is empty. */ + ck_assert_uint_eq(local.size, 0); timer_binheap_cancel(&local, id + 1); + /* Cancelling a missing id is a no-op. */ + ck_assert_uint_eq(local.size, 0); } END_TEST @@ -2422,17 +2425,51 @@ START_TEST(test_cancel_timer) { (void)id2; timer_binheap_cancel(&heap, id1); - ck_assert_int_eq(heap.timers[0].expires, 0); // tmr1 canceled + /* Eager cancel removes tmr1 physically; only tmr2 remains. */ + ck_assert_int_eq(heap.size, 1); + ck_assert_int_eq(heap.timers[0].expires, 200); popped = timers_binheap_pop(&heap); - ck_assert_uint_eq(popped.expires, 0); /* the tombstone itself */ - popped = timers_binheap_pop(&heap); - ck_assert_uint_eq(popped.expires, 200); /* tmr2 survived the drain */ + ck_assert_int_eq(popped.expires, 200); // Only tmr2 should remain ck_assert_int_eq(heap.size, 0); } END_TEST -START_TEST(test_timer_pop_removes_zero_head_first) +/* F-9808: the timer heap must not clog with lazy-cancelled dead slots. 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). Pre-fix, each cancel left a dead + * slot (expires==0) that still counted toward size and broke the min-heap + * invariant, so enough re-arms filled the heap and made + * timers_binheap_insert return NO_TIMER, silently dropping control-state RTO + * arms. Eager cancel removes the slot physically, so the size tracks live + * timers and the heap stays usable. */ +START_TEST(test_timer_heap_no_dead_slot_accumulation) +{ + struct timers_binheap local = {0}; + struct wolfIP_timer tmr = {0}; + int id; + int i; + + tmr.expires = 100; + tmr.cb = test_timer_cb; + + /* Many RTO re-arms: insert then cancel, over and over - the exact + * pattern a multi-segment send drives through the shared heap. */ + for (i = 0; i < (MAX_TIMERS * 2); i++) { + id = timers_binheap_insert(&local, tmr); + ck_assert_int_ne(id, NO_TIMER); + timer_binheap_cancel(&local, id); + } + + /* The heap must not be clogged with dead slots: it is empty, and a + * fresh insert still succeeds (pre-fix it returned NO_TIMER). */ + ck_assert_uint_eq(local.size, 0); + id = timers_binheap_insert(&local, tmr); + ck_assert_int_ne(id, NO_TIMER); +} +END_TEST + +START_TEST(test_timer_pop_skips_zero_expires) { struct timers_binheap h; struct wolfIP_timer tmr1 = { .expires = 50 }; @@ -2444,10 +2481,10 @@ START_TEST(test_timer_pop_removes_zero_head_first) tmr2.id = timers_binheap_insert(&h, tmr2); timer_binheap_cancel(&h, tmr2.id); - /* One root per pop: the tombstone first, then the live timer. */ - popped = timers_binheap_pop(&h); - ck_assert_uint_eq(popped.expires, 0); + /* Eager cancel removed tmr2 physically: the pop returns the live + * timer, never a zero slot. */ popped = timers_binheap_pop(&h); + ck_assert_uint_ne(popped.expires, 0); ck_assert_uint_eq(popped.expires, 50); } END_TEST @@ -2544,16 +2581,12 @@ START_TEST(test_timer_pop_siftdown_resets_after_cancelled) timers_binheap_insert(&h, (struct wolfIP_timer){ .expires = 100 }); timers_binheap_insert(&h, (struct wolfIP_timer){ .expires = 200 }); - /* Cancel the two smallest */ + /* Cancel the two smallest (eager: slots removed physically) */ timer_binheap_cancel(&h, id1); timer_binheap_cancel(&h, id2); - /* Pops remove one root each: both tombstones, then the live timers - * in order -- verifies the heap invariant held through the run. */ - popped = timers_binheap_pop(&h); - ck_assert_uint_eq(popped.expires, 0); - popped = timers_binheap_pop(&h); - ck_assert_uint_eq(popped.expires, 0); + /* Pops return the live timers in order -- verifies the heap + * invariant held through the eager removals. */ popped = timers_binheap_pop(&h); ck_assert_uint_eq(popped.expires, 50); popped = timers_binheap_pop(&h); diff --git a/src/test/unit/unit_tests_tcp_ack.c b/src/test/unit/unit_tests_tcp_ack.c index fec76441..955ce976 100644 --- a/src/test/unit/unit_tests_tcp_ack.c +++ b/src/test/unit/unit_tests_tcp_ack.c @@ -3932,7 +3932,7 @@ START_TEST(test_tcp_last_ack_closes_socket) uint16_t remote_port = 7777; uint32_t ctrl_rto_id; uint32_t i; - int found_canceled = 0; + int found = 0; wolfIP_init(&s); mock_link_init(&s); @@ -3961,14 +3961,13 @@ START_TEST(test_tcp_last_ack_closes_socket) inject_tcp_segment(&s, TEST_PRIMARY_IF, remote_ip, local_ip, remote_port, local_port, 10, 10, TCP_FLAG_ACK); ck_assert_int_eq(ts->proto, 0); + /* Eager cancel removes the control RTO timer physically; it must no + * longer be present in the heap. */ for (i = 0; i < s.timers.size; i++) { - if (s.timers.timers[i].id == ctrl_rto_id) { - found_canceled = 1; - ck_assert_uint_eq(s.timers.timers[i].expires, 0); - break; - } + if (s.timers.timers[i].id == ctrl_rto_id) + found = 1; } - ck_assert_int_eq(found_canceled, 1); + ck_assert_int_eq(found, 0); } END_TEST diff --git a/src/wolfip.c b/src/wolfip.c index b26b6e13..ac5e6f00 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -2565,12 +2565,61 @@ static int is_timer_expired(struct timers_binheap *heap, uint64_t now) return (heap->timers[0].expires <= now)?1:0; } +/* Restore the min-heap property after the element at index i was replaced + * (by the former last slot during a cancel). Sifts the element down toward + * its children, then up toward its parent; whichever direction is needed is + * taken and the other becomes a no-op. */ +static void timers_heapify(struct timers_binheap *heap, uint32_t i) +{ + uint32_t n = heap->size; + + if (i >= n) + return; /* the last slot was removed; nothing left to order */ + /* Sift down. */ + while (2 * i + 1 < n) { + uint32_t j = 2 * i + 1; + struct wolfIP_timer tmp; + if (j + 1 < n && heap->timers[j + 1].expires < heap->timers[j].expires) + j++; + if (heap->timers[i].expires <= heap->timers[j].expires) + break; + tmp = heap->timers[i]; + heap->timers[i] = heap->timers[j]; + heap->timers[j] = tmp; + i = j; + } + /* Sift up. */ + while (i > 0) { + uint32_t p = (i - 1) / 2; + struct wolfIP_timer tmp; + if (heap->timers[i].expires >= heap->timers[p].expires) + break; + tmp = heap->timers[i]; + heap->timers[i] = heap->timers[p]; + heap->timers[p] = tmp; + i = p; + } +} + static void timer_binheap_cancel(struct timers_binheap *heap, uint32_t id) { uint32_t i; for (i = 0; i < heap->size; i++) { if (heap->timers[i].id == id) { - heap->timers[i].expires = 0; + /* Remove the slot physically so heap->size tracks live timers. + * The old lazy in-place expires==0 mark left a dead slot that + * still counted toward size and broke the min-heap invariant + * (a zeroed slot sorts below its parent). Only dead slots that + * happened to sit at the root were ever popped, so the victim's + * own multi-segment TX re-arms (which cancel and re-insert the + * data RTO on every segment) accumulated dead slots until the + * heap filled and timers_binheap_insert returned NO_TIMER, + * silently dropping control-state RTO arms and sticking sockets + * in SYN_RCVD/FIN_WAIT_2 until the socket table exhausted + * (F-9808). */ + heap->timers[i] = heap->timers[heap->size - 1]; + heap->size--; + timers_heapify(heap, i); break; } } From 13e95d6f915b7af07e0a63068796a4ea63e86e4e Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 20:17:58 +0200 Subject: [PATCH 33/35] F-6473: seed TS.Recent from the first peer timestamp 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. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_api.c | 5 ++ src/test/unit/unit_tests_proto.c | 16 ++-- src/test/unit/unit_tests_tcp_ack.c | 4 +- src/test/unit/unit_tests_tcp_flow.c | 124 ++++++++++++++++++++++++++++ src/wolfip.c | 24 +++++- 6 files changed, 164 insertions(+), 10 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 1145b677..bc15884d 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -619,6 +619,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_tcp_input_syn_rcvd_high_seq_valid_ack_establishes); tcase_add_test(tc_utils, test_tcp_input_syn_rcvd_high_seq_data_held_ooo); tcase_add_test(tc_utils, test_tcp_input_syn_rcvd_out_of_window_ack_drop); + tcase_add_test(tc_utils, test_tcp_input_paws_upper_half_tsval_flows); tcase_add_test(tc_utils, test_tcp_input_syn_rcvd_ack_fin_transitions_to_close_wait); tcase_add_test(tc_utils, test_tcp_input_filter_drop); tcase_add_test(tc_utils, test_tcp_input_port_mismatch_skips_socket); diff --git a/src/test/unit/unit_tests_api.c b/src/test/unit/unit_tests_api.c index d681edf5..621643b9 100644 --- a/src/test/unit/unit_tests_api.c +++ b/src/test/unit/unit_tests_api.c @@ -2599,6 +2599,7 @@ START_TEST(test_sock_accept_clones_half_open_state_and_queues_synack) uint32_t pre_accept_seq; uint32_t pre_accept_ack; uint32_t pre_accept_last_ts; + uint8_t pre_accept_ts_recent_valid; uint32_t pre_accept_local_ip; uint32_t pre_accept_remote_ip; uint32_t pre_accept_peer_rwnd; @@ -2637,6 +2638,7 @@ START_TEST(test_sock_accept_clones_half_open_state_and_queues_synack) /* Seed half-open negotiation state so accept() must clone it into the child socket. */ listener->sock.tcp.last_ts = 0x11223344U; + listener->sock.tcp.ts_recent_valid = 1; listener->sock.tcp.peer_rwnd = 4096; listener->sock.tcp.peer_mss = 1200; listener->sock.tcp.snd_wscale = 4; @@ -2651,6 +2653,7 @@ START_TEST(test_sock_accept_clones_half_open_state_and_queues_synack) pre_accept_seq = listener->sock.tcp.seq; pre_accept_ack = listener->sock.tcp.ack; pre_accept_last_ts = listener->sock.tcp.last_ts; + pre_accept_ts_recent_valid = listener->sock.tcp.ts_recent_valid; pre_accept_local_ip = listener->local_ip; pre_accept_remote_ip = listener->remote_ip; pre_accept_peer_rwnd = listener->sock.tcp.peer_rwnd; @@ -2684,6 +2687,8 @@ START_TEST(test_sock_accept_clones_half_open_state_and_queues_synack) ck_assert_uint_eq(accepted->sock.tcp.ack, pre_accept_ack); ck_assert_uint_eq(accepted->sock.tcp.snd_una, pre_accept_seq); ck_assert_uint_eq(accepted->sock.tcp.last_ts, pre_accept_last_ts); + ck_assert_uint_eq(accepted->sock.tcp.ts_recent_valid, + pre_accept_ts_recent_valid); ck_assert_uint_eq(accepted->sock.tcp.peer_rwnd, pre_accept_peer_rwnd); ck_assert_uint_eq(accepted->sock.tcp.peer_mss, pre_accept_peer_mss); ck_assert_uint_eq(accepted->sock.tcp.snd_wscale, pre_accept_snd_wscale); diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index 7bc96687..ee4ed934 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -6608,7 +6608,8 @@ START_TEST(test_regression_paws_rejects_stale_timestamp) ts->sock.tcp.cwnd = TCP_MSS; ts->sock.tcp.peer_rwnd = TCP_MSS; ts->sock.tcp.ts_enabled = 1; - ts->sock.tcp.last_ts = ee32(5000); /* TS.Recent = 5000 */ + ts->sock.tcp.last_ts = ee32(5000); + ts->sock.tcp.ts_recent_valid = 1; /* TS.Recent = 5000 */ ts->src_port = 1234; ts->dst_port = 4321; ts->local_ip = 0x0A000001U; @@ -6698,7 +6699,8 @@ START_TEST(test_regression_paws_accepts_wrapped_newer_timestamp) ts->sock.tcp.cwnd = TCP_MSS; ts->sock.tcp.peer_rwnd = TCP_MSS; ts->sock.tcp.ts_enabled = 1; - ts->sock.tcp.last_ts = ee32(0xFFFFFFF0U); /* TS.Recent just before wrap */ + ts->sock.tcp.last_ts = ee32(0xFFFFFFF0U); + ts->sock.tcp.ts_recent_valid = 1; /* TS.Recent just before wrap */ ts->src_port = 1234; ts->dst_port = 4321; ts->local_ip = 0x0A000001U; @@ -6782,7 +6784,8 @@ START_TEST(test_regression_paws_drops_segment_without_timestamp_option) ts->sock.tcp.cwnd = TCP_MSS; ts->sock.tcp.peer_rwnd = TCP_MSS; ts->sock.tcp.ts_enabled = 1; - ts->sock.tcp.last_ts = ee32(5000); /* TS.Recent = 5000 */ + ts->sock.tcp.last_ts = ee32(5000); + ts->sock.tcp.ts_recent_valid = 1; /* TS.Recent = 5000 */ ts->src_port = 1234; ts->dst_port = 4321; ts->local_ip = 0x0A000001U; @@ -6861,6 +6864,7 @@ START_TEST(test_regression_paws_preempts_acceptability_for_replayed_segment) ts->sock.tcp.peer_rwnd = TCP_MSS; ts->sock.tcp.ts_enabled = 1; ts->sock.tcp.last_ts = ee32(5000); + ts->sock.tcp.ts_recent_valid = 1; ts->src_port = 1234; ts->dst_port = 4321; ts->local_ip = 0x0A000001U; @@ -6934,7 +6938,8 @@ START_TEST(test_regression_paws_drops_last_ack_segment_without_timestamp_option) ts->sock.tcp.cwnd = TCP_MSS; ts->sock.tcp.peer_rwnd = TCP_MSS; ts->sock.tcp.ts_enabled = 1; - ts->sock.tcp.last_ts = ee32(5000); /* TS.Recent = 5000 */ + ts->sock.tcp.last_ts = ee32(5000); + ts->sock.tcp.ts_recent_valid = 1; /* TS.Recent = 5000 */ ts->src_port = 1234; ts->dst_port = 4321; ts->local_ip = 0x0A000001U; @@ -7005,7 +7010,8 @@ START_TEST(test_regression_paws_drops_time_wait_segment_without_timestamp_option ts->sock.tcp.cwnd = TCP_MSS; ts->sock.tcp.peer_rwnd = TCP_MSS; ts->sock.tcp.ts_enabled = 1; - ts->sock.tcp.last_ts = ee32(5000); /* TS.Recent = 5000 */ + ts->sock.tcp.last_ts = ee32(5000); + ts->sock.tcp.ts_recent_valid = 1; /* TS.Recent = 5000 */ ts->src_port = 1234; ts->dst_port = 4321; ts->local_ip = 0x0A000001U; diff --git a/src/test/unit/unit_tests_tcp_ack.c b/src/test/unit/unit_tests_tcp_ack.c index 955ce976..1e749ec5 100644 --- a/src/test/unit/unit_tests_tcp_ack.c +++ b/src/test/unit/unit_tests_tcp_ack.c @@ -4377,7 +4377,8 @@ START_TEST(test_tcp_process_ts_ooo_segment_keeps_recent) ts->proto = WI_IPPROTO_TCP; ts->S = &s; ts->sock.tcp.ack = 1000; /* RCV.NXT == Last.ACK.sent (host order) */ - ts->sock.tcp.last_ts = ee32(100); /* TS.Recent (stored network order) */ + ts->sock.tcp.last_ts = ee32(100); + ts->sock.tcp.ts_recent_valid = 1; /* TS.Recent (stored network order) */ memset(buf, 0, sizeof(buf)); tcp->hlen = (TCP_HEADER_LEN + TCP_OPTIONS_LEN) << 2; @@ -4431,6 +4432,7 @@ START_TEST(test_tcp_input_paws_ooo_does_not_poison_hole_fill) ts->sock.tcp.ts_enabled = 1; ts->sock.tcp.sack_permitted = 1; ts->sock.tcp.last_ts = ee32(100); + ts->sock.tcp.ts_recent_valid = 1; ts->local_ip = 0x0A000001U; ts->remote_ip = 0x0A000002U; ts->src_port = 1234; diff --git a/src/test/unit/unit_tests_tcp_flow.c b/src/test/unit/unit_tests_tcp_flow.c index d3223878..7f514d76 100644 --- a/src/test/unit/unit_tests_tcp_flow.c +++ b/src/test/unit/unit_tests_tcp_flow.c @@ -3450,6 +3450,130 @@ START_TEST(test_tcp_input_syn_rcvd_out_of_window_ack_drop) } END_TEST +/* Regression (F-6473 follow-up): the first peer timestamp must seed + * TS.Recent unconditionally. last_ts starts zeroed, which is not a + * timestamp: the RFC 7323 4.3 update gate compares the SYN's TSval + * against it, and a TSval in the upper half of the 32-bit space (the + * common case for real kernel clocks) compares as "older" than zero, + * so TS.Recent never got seeded. Every later segment then failed + * tcp_paws_check as an old duplicate and was dropped with a bare ACK - + * the handshake completed but no data ever flowed (dup-ACK loop). + * Values mirror a captured failure: SYN TSval 0xE0600D2C. */ +START_TEST(test_tcp_input_paws_upper_half_tsval_flows) +{ + struct wolfIP s; + int listen_sd; + struct tsocket *ts; + struct wolfIP_sockaddr_in sin; + uint8_t seg_buf[sizeof(struct wolfIP_tcp_seg) + TCP_OPTIONS_LEN + 10]; + struct wolfIP_tcp_seg *seg = (struct wolfIP_tcp_seg *)seg_buf; + struct tcp_opt_ts *opt = (struct tcp_opt_ts *)seg->data; + uint32_t rcv_nxt; + uint8_t out[32]; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + listen_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, WI_IPPROTO_TCP); + ck_assert_int_gt(listen_sd, 0); + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_port = ee16(1234); + sin.sin_addr.s_addr = ee32(0x0A000001U); + ck_assert_int_eq(wolfIP_sock_bind(&s, listen_sd, (struct wolfIP_sockaddr *)&sin, sizeof(sin)), 0); + ck_assert_int_eq(wolfIP_sock_listen(&s, listen_sd, 1), 0); + + /* SYN with a Timestamps option in the upper half of the 32-bit + * space - compares as "older" than the zeroed last_ts. */ + memset(seg_buf, 0, sizeof(seg_buf)); + seg->ip.ver_ihl = 0x45; + seg->ip.proto = WI_IPPROTO_TCP; + seg->ip.ttl = 64; + seg->ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN + TCP_OPTIONS_LEN); + seg->ip.src = ee32(0x0A000002U); + seg->ip.dst = ee32(0x0A000001U); + seg->src_port = ee16(52798); + seg->dst_port = ee16(1234); + seg->seq = ee32(0); + seg->hlen = (TCP_HEADER_LEN + TCP_OPTIONS_LEN) << 2; + seg->flags = TCP_FLAG_SYN; + opt->opt = TCP_OPTION_TS; + opt->len = TCP_OPTION_TS_LEN; + opt->val = ee32(0xE0600D2CU); + opt->ecr = 0; + opt->pad = TCP_OPTION_NOP; + opt->eoo = TCP_OPTION_EOO; + fix_tcp_checksums(seg); + tcp_input(&s, TEST_PRIMARY_IF, seg, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + + TCP_HEADER_LEN + TCP_OPTIONS_LEN)); + ts = &s.tcpsockets[SOCKET_UNMARK(listen_sd)]; + ck_assert_int_eq(ts->sock.tcp.state, TCP_SYN_RCVD); + /* The SYN must have seeded TS.Recent despite the upper-half value. */ + ck_assert_uint_eq(ts->sock.tcp.ts_recent_valid, 1); + ck_assert_uint_eq(ts->sock.tcp.last_ts, ee32(0xE0600D2CU)); + rcv_nxt = ts->sock.tcp.ack; + + /* Final ACK, same clock (still upper half). */ + memset(seg_buf, 0, sizeof(seg_buf)); + seg->ip.ver_ihl = 0x45; + seg->ip.proto = WI_IPPROTO_TCP; + seg->ip.ttl = 64; + seg->ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN + TCP_OPTIONS_LEN); + seg->ip.src = ee32(0x0A000002U); + seg->ip.dst = ee32(0x0A000001U); + seg->src_port = ee16(52798); + seg->dst_port = ee16(1234); + seg->seq = ee32(rcv_nxt); + seg->ack = ee32(tcp_seq_inc(ts->sock.tcp.snd_una, 1)); + seg->hlen = (TCP_HEADER_LEN + TCP_OPTIONS_LEN) << 2; + seg->flags = TCP_FLAG_ACK; + opt->opt = TCP_OPTION_TS; + opt->len = TCP_OPTION_TS_LEN; + opt->val = ee32(0xE0600D2EU); + opt->ecr = 0; + opt->pad = TCP_OPTION_NOP; + opt->eoo = TCP_OPTION_EOO; + fix_tcp_checksums(seg); + tcp_input(&s, TEST_PRIMARY_IF, seg, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + + TCP_HEADER_LEN + TCP_OPTIONS_LEN)); + ck_assert_int_eq(ts->sock.tcp.state, TCP_ESTABLISHED); + + /* First data segment: pre-fix this was dropped by tcp_paws_check as + * "older than TS.Recent" (zeroed) and the connection stalled in a + * dup-ACK loop. */ + memset(seg_buf, 0, sizeof(seg_buf)); + seg->ip.ver_ihl = 0x45; + seg->ip.proto = WI_IPPROTO_TCP; + seg->ip.ttl = 64; + seg->ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN + TCP_OPTIONS_LEN + 10); + seg->ip.src = ee32(0x0A000002U); + seg->ip.dst = ee32(0x0A000001U); + seg->src_port = ee16(52798); + seg->dst_port = ee16(1234); + seg->seq = ee32(rcv_nxt); + seg->ack = ee32(tcp_seq_inc(ts->sock.tcp.snd_una, 1)); + seg->hlen = (TCP_HEADER_LEN + TCP_OPTIONS_LEN) << 2; + seg->flags = TCP_FLAG_ACK; + opt->opt = TCP_OPTION_TS; + opt->len = TCP_OPTION_TS_LEN; + opt->val = ee32(0xE0600D32U); + opt->ecr = 0; + opt->pad = TCP_OPTION_NOP; + opt->eoo = TCP_OPTION_EOO; + memcpy(seg->data + TCP_OPTIONS_LEN, "0123456789", 10); + fix_tcp_checksums(seg); + tcp_input(&s, TEST_PRIMARY_IF, seg, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + + TCP_HEADER_LEN + TCP_OPTIONS_LEN + 10)); + ck_assert_uint_eq(ts->sock.tcp.ack, rcv_nxt + 10); + ck_assert_int_eq(queue_pop(&ts->sock.tcp.rxbuf, out, sizeof(out)), 10); + ck_assert_mem_eq(out, "0123456789", 10); +} +END_TEST + /* Regression: an ACK+FIN segment in SYN_RCVD must not be silently discarded. * The ACK should complete the handshake (to ESTABLISHED) and the FIN should * be processed in the same pass (to CLOSE_WAIT). Per RFC 9293 section 3.10.7.4 diff --git a/src/wolfip.c b/src/wolfip.c index ac5e6f00..6ccf4991 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -1212,6 +1212,9 @@ struct tcpsocket { uint16_t last_peer_win; uint8_t snd_wscale, rcv_wscale, ws_enabled, ws_offer; uint8_t ts_enabled, ts_offer; + /* Set once the first peer timestamp has seeded TS.Recent; until then + * last_ts is an uninitialized sentinel, not a timestamp. */ + uint8_t ts_recent_valid; uint8_t sack_offer, sack_permitted; uint8_t rx_sack_count, peer_sack_count; struct tcp_sack_block rx_sack[TCP_SACK_MAX_BLOCKS]; @@ -3090,6 +3093,7 @@ static struct tsocket *tcp_new_socket(struct wolfIP *s) t->sock.tcp.snd_wscale = 0; t->sock.tcp.ws_enabled = 0; t->sock.tcp.ts_enabled = 0; + t->sock.tcp.ts_recent_valid = 0; t->sock.tcp.sack_offer = 1; t->sock.tcp.sack_permitted = 0; t->sock.tcp.rx_sack_count = 0; @@ -4836,10 +4840,18 @@ static int tcp_process_ts(struct tsocket *t, const struct wolfIP_tcp_seg *tcp, * An out-of-order segment (SEQ above the left edge) must not advance * TS.Recent: it would corrupt the TSecr echoed per rule (3) and make * tcp_paws_check drop the in-order segment that later fills the hole, - * whose TSval is necessarily lower than the OOO segment's. */ - if (!tcp_seq_lt(ee32(po.ts_val), t->sock.tcp.last_ts) && - tcp_seq_leq(ee32(tcp->seq), t->sock.tcp.ack)) + * whose TSval is necessarily lower than the OOO segment's. + * The first timestamp after negotiation seeds TS.Recent unconditionally: + * until then there is no reference to compare against. A zeroed last_ts + * is not a timestamp - treating it as one makes every segment whose + * TSval sits in the upper half of the 32-bit space look "older" and + * tcp_paws_check drops the whole data flow after the handshake. */ + if (!t->sock.tcp.ts_recent_valid || + (!tcp_seq_lt(ee32(po.ts_val), t->sock.tcp.last_ts) && + tcp_seq_leq(ee32(tcp->seq), t->sock.tcp.ack))) { t->sock.tcp.last_ts = ee32(po.ts_val); + t->sock.tcp.ts_recent_valid = 1; + } if (po.ts_ecr == 0) return -1; /* No echoed timestamp; fall back to coarse RTT. */ if (po.ts_ecr > t->S->last_tick) @@ -4868,7 +4880,10 @@ static int tcp_paws_check(const struct tsocket *t, * dropped silently. */ if (!po.ts_found) return TCP_PAWS_DROP; - if (tcp_seq_lt(po.ts_val, ee32(t->sock.tcp.last_ts))) + /* Without a stored reference (no peer timestamp seen yet) there is + * nothing to compare against: accept. */ + if (t->sock.tcp.ts_recent_valid && + tcp_seq_lt(po.ts_val, ee32(t->sock.tcp.last_ts))) return TCP_PAWS_ACK_DROP; return TCP_PAWS_OK; } @@ -6543,6 +6558,7 @@ int wolfIP_sock_accept(struct wolfIP *s, int sockfd, struct wolfIP_sockaddr *add newts->sock.tcp.ws_offer = ts->sock.tcp.ws_offer; newts->sock.tcp.ts_enabled = ts->sock.tcp.ts_enabled; newts->sock.tcp.ts_offer = ts->sock.tcp.ts_offer; + newts->sock.tcp.ts_recent_valid = ts->sock.tcp.ts_recent_valid; newts->sock.tcp.sack_offer = ts->sock.tcp.sack_offer; newts->sock.tcp.sack_permitted = ts->sock.tcp.sack_permitted; newts->sock.tcp.state = TCP_SYN_RCVD; From 99c73315c87bfc3bfd6d1d86afccae2eae2ccb22 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 20:18:02 +0200 Subject: [PATCH 34/35] wolfesp: log replay sequence numbers as unsigned 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. --- src/wolfesp.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/wolfesp.c b/src/wolfesp.c index c1a7a3f6..c1b66842 100644 --- a/src/wolfesp.c +++ b/src/wolfesp.c @@ -1322,7 +1322,8 @@ esp_replay_check(const struct replay_t * replay, uint32_t seq) } if (seq < seq_low) { - ESP_LOG("error: seq (%d) below window (%d)\n", seq, seq_low); + ESP_LOG("error: seq (%lu) below window (%lu)\n", + (unsigned long)seq, (unsigned long)seq_low); return -1; } @@ -1335,7 +1336,7 @@ esp_replay_check(const struct replay_t * replay, uint32_t seq) bitn = 1ULL << (replay->hi_seq - seq); if ((replay->bitmap & bitn) != 0ULL) { - ESP_LOG("error: seq replayed: %d\n", seq); + ESP_LOG("error: seq replayed: %lu\n", (unsigned long)seq); return -1; } } From 8a5132423b20d565019fcd97d732c29f1e4c5cc4 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 21:39:08 +0200 Subject: [PATCH 35/35] F-10271: test the SYN_RCVD FIN-above-hole deferral --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_tcp_flow.c | 113 ++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index bc15884d..b4c767bb 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -618,6 +618,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_tcp_input_syn_rcvd_ack_invalid_ack_rejected); tcase_add_test(tc_utils, test_tcp_input_syn_rcvd_high_seq_valid_ack_establishes); tcase_add_test(tc_utils, test_tcp_input_syn_rcvd_high_seq_data_held_ooo); + tcase_add_test(tc_utils, test_tcp_input_syn_rcvd_fin_above_hole_deferred); tcase_add_test(tc_utils, test_tcp_input_syn_rcvd_out_of_window_ack_drop); tcase_add_test(tc_utils, test_tcp_input_paws_upper_half_tsval_flows); tcase_add_test(tc_utils, test_tcp_input_syn_rcvd_ack_fin_transitions_to_close_wait); diff --git a/src/test/unit/unit_tests_tcp_flow.c b/src/test/unit/unit_tests_tcp_flow.c index 7f514d76..f95f7d68 100644 --- a/src/test/unit/unit_tests_tcp_flow.c +++ b/src/test/unit/unit_tests_tcp_flow.c @@ -3381,6 +3381,119 @@ START_TEST(test_tcp_input_syn_rcvd_high_seq_data_held_ooo) } END_TEST +/* A data+FIN segment that reorders ahead of the final ACK completes the + * handshake but must NOT enter CLOSE_WAIT: the FIN sits above the receive + * hole, so it is deferred (ACK only). When the hole fills, RCV.NXT reaches + * the FIN's sequence number and the cumulative ACK makes the peer + * retransmit the segment; the retransmit is then accepted and the socket + * moves to CLOSE_WAIT. */ +START_TEST(test_tcp_input_syn_rcvd_fin_above_hole_deferred) +{ + struct wolfIP s; + int listen_sd; + struct tsocket *ts; + struct wolfIP_sockaddr_in sin; + uint8_t seg_buf[sizeof(struct wolfIP_tcp_seg) + 10]; + struct wolfIP_tcp_seg *dataseg = (struct wolfIP_tcp_seg *)seg_buf; + uint32_t rcv_nxt; + uint8_t out[32]; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + listen_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, WI_IPPROTO_TCP); + ck_assert_int_gt(listen_sd, 0); + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_port = ee16(1234); + sin.sin_addr.s_addr = ee32(0x0A000001U); + ck_assert_int_eq(wolfIP_sock_bind(&s, listen_sd, (struct wolfIP_sockaddr *)&sin, sizeof(sin)), 0); + ck_assert_int_eq(wolfIP_sock_listen(&s, listen_sd, 1), 0); + + inject_tcp_syn(&s, TEST_PRIMARY_IF, 0x0A000001U, 1234); + ts = &s.tcpsockets[SOCKET_UNMARK(listen_sd)]; + ck_assert_int_eq(ts->sock.tcp.state, TCP_SYN_RCVD); + rcv_nxt = ts->sock.tcp.ack; + + /* data+FIN (8 bytes) reorders ahead of the final ACK: seq = RCV.NXT + 10, + * valid acknowledgment number. */ + memset(seg_buf, 0, sizeof(seg_buf)); + dataseg->ip.ver_ihl = 0x45; + dataseg->ip.proto = WI_IPPROTO_TCP; + dataseg->ip.ttl = 64; + dataseg->ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN + 8); + dataseg->ip.src = ee32(ts->remote_ip); + dataseg->ip.dst = ee32(ts->local_ip); + dataseg->dst_port = ee16(ts->src_port); + dataseg->src_port = ee16(ts->dst_port); + dataseg->seq = ee32(rcv_nxt + 10); + dataseg->ack = ee32(tcp_seq_inc(ts->sock.tcp.snd_una, 1)); + dataseg->hlen = TCP_HEADER_LEN << 2; + dataseg->flags = TCP_FLAG_ACK | TCP_FLAG_FIN; + memcpy(dataseg->data, "ABCDEFGH", 8); + fix_tcp_checksums(dataseg); + tcp_input(&s, TEST_PRIMARY_IF, dataseg, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN + 8)); + /* Handshake completes on the valid ACK; the data is OOO-cached and the + * FIN above the hole must be deferred: ESTABLISHED, not CLOSE_WAIT. */ + ck_assert_int_eq(ts->sock.tcp.state, TCP_ESTABLISHED); + ck_assert_uint_eq(ts->sock.tcp.ack, rcv_nxt); + ck_assert_uint_eq(ts->sock.tcp.rx_sack_count, 1); + ck_assert_uint_eq(ts->sock.tcp.rx_sack[0].left, rcv_nxt + 10); + ck_assert_uint_eq(ts->sock.tcp.rx_sack[0].right, rcv_nxt + 18); + + /* The missing 10 bytes arrive in order and pull in the cached tail. + * RCV.NXT reaches the FIN's sequence number, but the FIN bit was not + * recorded: the socket must stay ESTABLISHED. */ + memset(seg_buf, 0, sizeof(seg_buf)); + dataseg->ip.ver_ihl = 0x45; + dataseg->ip.proto = WI_IPPROTO_TCP; + dataseg->ip.ttl = 64; + dataseg->ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN + 10); + dataseg->ip.src = ee32(ts->remote_ip); + dataseg->ip.dst = ee32(ts->local_ip); + dataseg->dst_port = ee16(ts->src_port); + dataseg->src_port = ee16(ts->dst_port); + dataseg->seq = ee32(rcv_nxt); + dataseg->ack = ee32(tcp_seq_inc(ts->sock.tcp.snd_una, 1)); + dataseg->hlen = TCP_HEADER_LEN << 2; + dataseg->flags = TCP_FLAG_ACK; + memcpy(dataseg->data, "0123456789", 10); + fix_tcp_checksums(dataseg); + tcp_input(&s, TEST_PRIMARY_IF, dataseg, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN + 10)); + ck_assert_uint_eq(ts->sock.tcp.ack, rcv_nxt + 18); + ck_assert_int_eq(ts->sock.tcp.state, TCP_ESTABLISHED); + ck_assert_uint_eq(ts->sock.tcp.rx_sack_count, 0); + ck_assert_int_eq(queue_pop(&ts->sock.tcp.rxbuf, out, sizeof(out)), 18); + ck_assert_mem_eq(out, "0123456789ABCDEFGH", 18); + + /* The peer retransmits the data+FIN (its cumulative ACK shows the FIN + * unacknowledged). The payload is a pure duplicate; the FIN now sits + * exactly at RCV.NXT and completes the close. */ + memset(seg_buf, 0, sizeof(seg_buf)); + dataseg->ip.ver_ihl = 0x45; + dataseg->ip.proto = WI_IPPROTO_TCP; + dataseg->ip.ttl = 64; + dataseg->ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN + 8); + dataseg->ip.src = ee32(ts->remote_ip); + dataseg->ip.dst = ee32(ts->local_ip); + dataseg->dst_port = ee16(ts->src_port); + dataseg->src_port = ee16(ts->dst_port); + dataseg->seq = ee32(rcv_nxt + 10); + dataseg->ack = ee32(tcp_seq_inc(ts->sock.tcp.snd_una, 1)); + dataseg->hlen = TCP_HEADER_LEN << 2; + dataseg->flags = TCP_FLAG_ACK | TCP_FLAG_FIN; + memcpy(dataseg->data, "ABCDEFGH", 8); + fix_tcp_checksums(dataseg); + tcp_input(&s, TEST_PRIMARY_IF, dataseg, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN + 8)); + ck_assert_int_eq(ts->sock.tcp.state, TCP_CLOSE_WAIT); + ck_assert_uint_eq(ts->sock.tcp.ack, rcv_nxt + 19); +} +END_TEST + START_TEST(test_tcp_input_syn_rcvd_out_of_window_ack_drop) { struct wolfIP s;