From 462acebb41400dc80b916d517344a88238376090 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 11:31:58 +0200 Subject: [PATCH 01/17] F-11432: compare TS.Recent freshness in one byte-order domain tcp_process_ts() stored TS.Recent in wire order (last_ts is emitted verbatim as the ECR field) but compared the incoming TSval after swapping only that operand: tcp_seq_lt(ee32(po.ts_val), last_ts). po.ts_val is parsed into host order, so the test ordered byte-swapped numbers, which are not ordered. With TS.Recent = 1 a segment carrying TSval 256 is newer in host order (and tcp_paws_check accepted it), yet 0x00010000 < 0x01000000 in the swapped domain, so the stale TS.Recent was kept - and with TS.Recent = 256 the swapped comparison even rolled it back to 1, diverging from tcp_paws_check on the same segment. Compare both operands in host order, as tcp_paws_check already does: tcp_seq_lt(po.ts_val, ee32(last_ts)). The store stays wire order, so the ECR emission is untouched. Unit test (test_tcp_process_ts_recent_compare_host_order): TS.Recent 1 + in-order TSval 256 must advance TS.Recent; the mirror image (TS.Recent 256 + TSval 1) must not roll it back. Both assertions fail pre-fix (stale 0x01000000 kept / rollback to 1). --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_tcp_ack.c | 47 ++++++++++++++++++++++++++++++ src/wolfip.c | 7 +++-- 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index b61f3ce1..0fa01c32 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -710,6 +710,7 @@ Suite *wolf_suite(void) 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_process_ts_recent_compare_host_order); 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); diff --git a/src/test/unit/unit_tests_tcp_ack.c b/src/test/unit/unit_tests_tcp_ack.c index c4b529da..3c9dd682 100644 --- a/src/test/unit/unit_tests_tcp_ack.c +++ b/src/test/unit/unit_tests_tcp_ack.c @@ -4461,6 +4461,53 @@ START_TEST(test_tcp_process_ts_ooo_segment_keeps_recent) } END_TEST +/* The TS.Recent freshness test must compare both operands in + * the same byte-order domain. po.ts_val is host order while last_ts is + * stored in wire order (it is emitted verbatim as ECR); the old + * comparison swapped only the incoming value, so it ordered byte-swapped + * numbers. With TS.Recent = 1, TSval 256 is newer in host order (and + * tcp_paws_check accepts it), but 0x00010000 < 0x01000000 in the + * swapped domain, so the stale TS.Recent was kept; and with TS.Recent + * = 256 the swapped comparison even rolled it back to 1. */ +START_TEST(test_tcp_process_ts_recent_compare_host_order) +{ + 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(1); + ts->sock.tcp.ts_recent_valid = 1; + + 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; + + /* In-order segment with TSval 256 > TS.Recent 1: advance. */ + tcp->seq = ee32(1000); + opt->val = ee32(256); + tcp_process_ts(ts, tcp, sizeof(buf)); + ck_assert_uint_eq(ts->sock.tcp.last_ts, ee32(256)); + + /* Mirror image: TS.Recent 256, incoming TSval 1 is older in host + * order and must not roll TS.Recent back. */ + tcp->seq = ee32(1000); + opt->val = ee32(1); + tcp_process_ts(ts, tcp, sizeof(buf)); + ck_assert_uint_eq(ts->sock.tcp.last_ts, ee32(256)); +} +END_TEST + START_TEST(test_tcp_input_paws_ooo_does_not_poison_hole_fill) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 4408bc8c..dd96193e 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -4922,9 +4922,12 @@ static int tcp_process_ts(struct tsocket *t, const struct wolfIP_tcp_seg *tcp, * 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. */ + * tcp_paws_check drops the whole data flow after the handshake. + * last_ts is stored in wire order (it is emitted verbatim as ECR), + * so compare in host order as tcp_paws_check does: byte-swapped + * values are not ordered. */ if (!t->sock.tcp.ts_recent_valid || - (!tcp_seq_lt(ee32(po.ts_val), t->sock.tcp.last_ts) && + (!tcp_seq_lt(po.ts_val, ee32(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; From bae36794d3e7775b765e56d52a62ad44faf47002 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 11:58:59 +0200 Subject: [PATCH 02/17] F-12410: scan DHCP overloaded option stream file before sname RFC 2131 sec.4.4.1 defines the interpretation order for option 52 (Overload) values: after the standard options field, the 'file' field MUST be interpreted next, followed by the 'sname' field. The stream walker had the two reversed (sname first), so for overload values 2 and 3 the sname option list was consumed before the file list. The offer parser is last-wins for repeated option codes, so the wrong order changed which value won whenever a server placed the same option in both overloaded fields: the file field's value was silently discarded. Swap the region transitions in dhcp_opt_stream_next_region(): from the main field enter the file region (bit 1) before the sname region (bit 2), and after the file region continue to sname. The region numbers themselves (1 = sname, 2 = file) are unchanged. Test: test_dhcp_parse_offer_option_overload gains scenario 4, an overload=3 OFFER carrying the subnet mask in both the file and the sname option lists with different values; it asserts the sname value is the one kept. Scenario 3's sname list now ends with the end option, as required for the last-scanned field. Fails pre-fix: dhcp_offered_mask == 0xFFFFFF00 (file value) instead of 0xFFFFFFF0. --- src/test/unit/unit_tests_dns_dhcp.c | 61 +++++++++++++++++++++++++++-- src/wolfip.c | 25 ++++++------ 2 files changed, 70 insertions(+), 16 deletions(-) diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index f6bf0bfc..ebc9de5d 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -3572,8 +3572,8 @@ START_TEST(test_dhcp_parse_offer_option_overload) ck_assert_uint_eq(s.dhcp_server_ip, 0x0A000064U); /* --- Scenario 3: overload = both (value 3): the option lists run - * options (terminated by END) -> sname -> file; the mask is in - * sname, the server id in file. */ + * options (terminated by END) -> file -> sname (RFC 2131 + * sec.4.4.1); the mask is in sname, the server id in file. */ memset(&s, 0, sizeof(s)); wolfIP_init(&s); mock_link_init(&s); @@ -3597,7 +3597,9 @@ START_TEST(test_dhcp_parse_offer_option_overload) field_opt->data[0] = 0xFF; field_opt->data[1] = 0xFF; field_opt->data[2] = 0xFF; field_opt->data[3] = 0x00; field_opt = (struct dhcp_option *)((uint8_t *)field_opt + 6); - field_opt->code = 0; /* pad until the file field */ + /* sname is now the last scanned region (RFC 2131 sec.4.4.1), so its + * option list must carry the terminating end option. */ + field_opt->code = DHCP_OPTION_END; field_opt->len = 0; field_opt = (struct dhcp_option *)msg.file; field_opt->code = DHCP_OPTION_SERVER_ID; @@ -3611,7 +3613,58 @@ START_TEST(test_dhcp_parse_offer_option_overload) ret = dhcp_parse_offer(&s, &msg, DHCP_HEADER_LEN + opt_len); ck_assert_int_eq(ret, 0); ck_assert_uint_eq(s.dhcp_server_ip, 0x0A000064U); - ck_assert_uint_eq(s.ipconf[TEST_PRIMARY_IF].mask, 0xFFFFFF00U); + ck_assert_uint_eq(s.dhcp_offered_mask, 0xFFFFFF00U); + + /* --- Scenario 4: overload = both, the same option code present in + * both overloaded fields. RFC 2131 sec.4.4.1 interprets the file + * field before the sname field, and the offer parser is last-wins, + * so the sname value must be the one kept (the old sname-first scan + * let the file value win). --- */ + memset(&s, 0, sizeof(s)); + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + s.dhcp_xid = 0x1234; + memset(&msg, 0, sizeof(msg)); + msg.op = BOOT_REPLY; + msg.magic = ee32(DHCP_MAGIC); + msg.xid = ee32(s.dhcp_xid); + msg.yiaddr = ee32(0x0A00000AU); + opt = (uint8_t *)msg.options; + opt[0] = DHCP_OPTION_MSG_TYPE; opt[1] = 1; opt[2] = DHCP_OFFER; + opt += 3; + opt[0] = 52 /* DHCP_OPTION_OVERLOAD */; opt[1] = 1; opt[2] = 3; + opt += 3; + *opt++ = DHCP_OPTION_END; + opt_len = (uint32_t)(opt - (uint8_t *)msg.options); + field_opt = (struct dhcp_option *)msg.file; + field_opt->code = DHCP_OPTION_SUBNET_MASK; + field_opt->len = 4; + field_opt->data[0] = 0xFF; field_opt->data[1] = 0xFF; + field_opt->data[2] = 0xFF; field_opt->data[3] = 0x00; + field_opt = (struct dhcp_option *)((uint8_t *)field_opt + 6); + field_opt->code = DHCP_OPTION_END; + field_opt->len = 0; + field_opt = (struct dhcp_option *)msg.sname; + field_opt->code = DHCP_OPTION_SERVER_ID; + field_opt->len = 4; + field_opt->data[0] = 0x0A; field_opt->data[1] = 0x00; + field_opt->data[2] = 0x00; field_opt->data[3] = 0x64; + field_opt = (struct dhcp_option *)((uint8_t *)field_opt + 6); + field_opt->code = DHCP_OPTION_SUBNET_MASK; + field_opt->len = 4; + field_opt->data[0] = 0xFF; field_opt->data[1] = 0xFF; + field_opt->data[2] = 0xFF; field_opt->data[3] = 0xF0; + field_opt = (struct dhcp_option *)((uint8_t *)field_opt + 6); + field_opt->code = DHCP_OPTION_END; + field_opt->len = 0; + + ret = dhcp_parse_offer(&s, &msg, DHCP_HEADER_LEN + opt_len); + ck_assert_int_eq(ret, 0); + ck_assert_uint_eq(s.dhcp_server_ip, 0x0A000064U); + /* sname is scanned after file: its mask is the last one seen and + * must be the one kept. */ + ck_assert_uint_eq(s.dhcp_offered_mask, 0xFFFFFFF0U); } END_TEST diff --git a/src/wolfip.c b/src/wolfip.c index dd96193e..bf3a8857 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -8712,7 +8712,8 @@ static void dhcp_deconfigure_lease(struct wolfIP *s) * RFC 2132 ยง9.3: option 52 (Overload) tells the client that the reply's * sname (value bit 2) and/or file (value bit 1) fields carry additional * options, interpreted after the standard options field is exhausted. - * The stream is therefore scanned options -> sname -> file (wire order), + * The stream is therefore scanned options -> file -> sname (RFC 2131 + * sec.4.4.1: the file field is interpreted next, followed by sname), * each region with its own bounds, and ends at option 255 or when the * last active region runs out. Non-overloaded sname/file fields hold * plain strings (TFTP server, bootfile) and are never scanned. */ @@ -8751,24 +8752,24 @@ static void dhcp_opt_stream_init(struct dhcp_opt_stream *st, static int dhcp_opt_stream_next_region(struct dhcp_opt_stream *st) { if (st->region == 0) { - if (st->overload & 2) { - st->region = 1; - st->ptr = st->region_sname; - st->end = st->region_sname_end; - return 1; - } if (st->overload & 1) { st->region = 2; st->ptr = st->region_file; st->end = st->region_file_end; return 1; } + if (st->overload & 2) { + st->region = 1; + st->ptr = st->region_sname; + st->end = st->region_sname_end; + return 1; + } } - else if (st->region == 1) { - if (st->overload & 1) { - st->region = 2; - st->ptr = st->region_file; - st->end = st->region_file_end; + else if (st->region == 2) { + if (st->overload & 2) { + st->region = 1; + st->ptr = st->region_sname; + st->end = st->region_sname_end; return 1; } } From e932462aaed9c4ec877cc866083e9fe553b9ee62 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 11:59:28 +0200 Subject: [PATCH 03/17] F-11433: build the ethernet header before raw TX filter callbacks flush_raw_tx() notified the IP and ethernet filter callbacks before eth_output_add_header() filled the ethernet header, so a callback registered for WOLFIP_FILT_SENDING inspected an uninitialised destination/source address and ethertype for every raw-socket frame. Build the header first (the nexthop MAC is already resolved above), then run the IP and ethernet notifications. A failed header build now drops the frame at the FIFO head instead of notifying callbacks and the driver with a missing header. Test: test_filter_notify_raw_tx_eth_header_built sends a raw UDP frame to the limited broadcast (all-ones nexthop, no ARP lookup) with a SENDING filter callback installed, and asserts the eth event carries the all-ones destination and the interface MAC as source. Fails pre-fix: the callback saw an all-zero destination MAC. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_api.c | 47 ++++++++++++++++++++++++++++++++++ src/wolfip.c | 19 +++++++++++--- 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 0fa01c32..31a9312a 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -184,6 +184,7 @@ Suite *wolf_suite(void) 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_notify_raw_tx_eth_header_built); 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 16513a39..86ae3d5f 100644 --- a/src/test/unit/unit_tests_api.c +++ b/src/test/unit/unit_tests_api.c @@ -406,6 +406,53 @@ START_TEST(test_filter_notify_udp_ihl_truncated_no_overread) } END_TEST +START_TEST(test_filter_notify_raw_tx_eth_header_built) +{ + struct wolfIP s; + struct wolfIP_sockaddr_in dst; + uint8_t payload[4] = {0xDE, 0xAD, 0xBE, 0xEF}; + uint8_t bcast_mac[6] = {0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}; + int raw_sd; + int ret; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + 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)); + + raw_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_RAW, WI_IPPROTO_UDP); + ck_assert_int_ge(raw_sd, 0); + + /* Limited broadcast: the nexthop MAC is all-ones, so the flush + * path needs no ARP entry. */ + memset(&dst, 0, sizeof(dst)); + dst.sin_family = AF_INET; + dst.sin_addr.s_addr = ee32(0xFFFFFFFFU); + + ret = wolfIP_sock_sendto(&s, raw_sd, payload, sizeof(payload), 0, + (struct wolfIP_sockaddr *)&dst, sizeof(dst)); + ck_assert_int_eq(ret, (int)sizeof(payload)); + + (void)wolfIP_poll(&s, 0); + + wolfIP_filter_set_callback(NULL, NULL); + wolfIP_sock_close(&s, raw_sd); + + /* The eth event must carry the header the frame was actually sent + * with: all-ones destination, the interface MAC as source. */ + ck_assert_int_ge(filter_cb_calls, 1); + ck_assert_uint_eq(filter_last_event.meta.ip_proto, + WOLFIP_FILTER_PROTO_ETH); + ck_assert_int_eq(memcmp(filter_last_event.meta.dst_mac, bcast_mac, 6), 0); + ck_assert_int_eq(memcmp(filter_last_event.meta.src_mac, + wolfIP_ll_at(&s, TEST_PRIMARY_IF)->mac, 6), 0); +} +END_TEST + START_TEST(test_filter_dispatch_no_callback) { diff --git a/src/wolfip.c b/src/wolfip.c index bf3a8857..a50bf00d 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -11761,15 +11761,26 @@ static void flush_raw_tx(struct wolfIP *s) ip->csum = 0; iphdr_set_checksum(ip); } - if (wolfIP_filter_notify_ip(WOLFIP_FILT_SENDING, s, tx_if, ip, desc->len) != 0) +#ifdef ETHERNET + /* Build the ethernet header before the filter callbacks run: + * they may inspect the destination address the frame will + * actually carry. */ + if (!wolfIP_ll_is_non_ethernet(s, tx_if)) { + if (eth_output_add_header(s, tx_if, r->nexthop_mac, &ip->eth, + ETH_TYPE_IP) != 0) { + break; + } + } +#endif + if (wolfIP_filter_notify_ip(WOLFIP_FILT_SENDING, s, tx_if, ip, desc->len) != 0) { break; + } #ifdef ETHERNET if (!wolfIP_ll_is_non_ethernet(s, tx_if)) { if (wolfIP_filter_notify_eth(WOLFIP_FILT_SENDING, s, tx_if, - &ip->eth, desc->len) != 0) + &ip->eth, desc->len) != 0) { break; - eth_output_add_header(s, tx_if, r->nexthop_mac, &ip->eth, - ETH_TYPE_IP); + } } #endif /* Mirror flush_datagram_tx: on driver backpressure/hard error From fd967402a572e4d42a2161701a78fa2c562de5a9 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 12:18:19 +0200 Subject: [PATCH 04/17] F-12385: clear the pre-accept flag when the control RTO takes over the slot The control RTO, the FIN-WAIT-2 timeout and the pre-accept fast-fail timeout all share the single tmr_rto slot, and tcp_rto_cb dispatches on the matching *_timeout_active flags. tcp_ctrl_rto_start cleared the FIN-WAIT-2 flag when it took the slot over but not the pre-accept one. A listener whose handshake completed before accept() is ESTABLISHED with the pre-accept timer armed. Closing that socket moves it to FIN_WAIT_1 and arms the control RTO on the same slot. On the first RTO expiry, tcp_rto_cb saw the stale pre-accept flag, took the 'socket left the pinned condition' branch, disarmed the timer quietly and returned without retransmitting the FIN-ACK: the close stalled in FIN_WAIT_1 with no timer and a dead retry budget, pinning the port. Clear the pre-accept flag in tcp_ctrl_rto_start alongside the existing FIN-WAIT-2 clear, so the flags always describe the timer in the slot. Test: test_tcp_listener_preaccept_close_rto_retransmits_finack drives an un-accepted established listener through close() and the first RTO expiry, asserting the FIN-ACK is retransmitted and the backoff re-armed. Fails pre-fix: preaccept_timeout_active survives the close and the retransmit never happens. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_tcp_flow.c | 64 +++++++++++++++++++++++++++++ src/wolfip.c | 4 ++ 3 files changed, 69 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 31a9312a..b20d2f0d 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -609,6 +609,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_tcp_listener_preaccept_accept_reverts_port); tcase_add_test(tc_utils, test_tcp_listener_preaccept_timeout_reverts_port); tcase_add_test(tc_utils, test_tcp_listener_preaccept_revert_drains_connection_state); + tcase_add_test(tc_utils, test_tcp_listener_preaccept_close_rto_retransmits_finack); tcase_add_test(tc_utils, test_tcp_listener_revert_restores_option_baseline); tcase_add_test(tc_utils, test_tcp_parse_sack_wraparound_block_accepted); tcase_add_test(tc_utils, test_tcp_parse_options_stops_on_truncated_or_invalid_option_length); diff --git a/src/test/unit/unit_tests_tcp_flow.c b/src/test/unit/unit_tests_tcp_flow.c index f95f7d68..efd0600c 100644 --- a/src/test/unit/unit_tests_tcp_flow.c +++ b/src/test/unit/unit_tests_tcp_flow.c @@ -5766,3 +5766,67 @@ START_TEST(test_tcp_listener_revert_restores_option_baseline) ck_assert_uint_eq(lsn->sock.tcp.ts_offer, fresh->sock.tcp.ts_offer); } END_TEST + +/* A handshake that completes before accept() leaves the listener + * ESTABLISHED with the pre-accept fast-fail timer armed. If the + * application closes the socket instead of accepting, the control RTO + * takes over the shared timer slot and must keep retransmitting the + * FIN-ACK on every expiry. The pre-accept flag has to go with the timer + * it armed: left behind, tcp_rto_cb's pre-accept branch sees a socket + * that left the pinned condition, disarms quietly, and the FIN_WAIT_1 + * close stalls with no retransmit and no retry budget. */ +START_TEST(test_tcp_listener_preaccept_close_rto_retransmits_finack) +{ + struct wolfIP s; + int fd; + struct tsocket *lsn; + uint64_t t; + uint32_t frames_before; + const struct wolfIP_tcp_seg *out; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, LLK_LOCAL_IP, LLK_NET_MASK, 0); + fd = llk_open_listener(&s); + lsn = &s.tcpsockets[SOCKET_UNMARK(fd)]; + + /* Pending-only ARP policy: the peer is a known neighbor, so the + * FIN-ACK retransmits reach the wire. */ + llk_keep_arp_fresh(&s, LLK_ATT_IP); + + llk_attacker_syn(&s, LLK_ATT_IP, 41000, 1, 0); + llk_complete_handshake(&s, lsn, LLK_ATT_IP, 41000, 1); + ck_assert_int_eq(lsn->sock.tcp.state, TCP_ESTABLISHED); + ck_assert_int_eq(lsn->sock.tcp.preaccept_timeout_active, 1); + + /* Close the established-but-unaccepted connection: FIN_WAIT_1, the + * control RTO owns the timer slot, and the pre-accept flag must be + * gone with the timer it armed. */ + ck_assert_int_eq(wolfIP_sock_close(&s, fd), -WOLFIP_EAGAIN); + ck_assert_int_eq(lsn->sock.tcp.state, TCP_FIN_WAIT_1); + ck_assert_int_eq(lsn->sock.tcp.preaccept_timeout_active, 0); + ck_assert_int_eq(lsn->sock.tcp.ctrl_rto_active, 1); + ck_assert_uint_ne(lsn->sock.tcp.tmr_rto, NO_TIMER); + + /* First poll drains the queued FIN-ACK. */ + (void)wolfIP_poll(&s, 3); + out = llk_last_tcp(); + ck_assert_ptr_nonnull(out); + ck_assert(out->flags & (TCP_FLAG_FIN | TCP_FLAG_ACK)); + + /* The control RTO fires: the FIN-ACK is retransmitted and the + * backoff re-armed, not silently disarmed. */ + frames_before = last_frame_sent_count; + for (t = 4; t <= 2500; t += 100) { + (void)wolfIP_poll(&s, t); + } + ck_assert_uint_eq(last_frame_sent_count, frames_before + 1); + out = llk_last_tcp(); + ck_assert_ptr_nonnull(out); + ck_assert(out->flags & (TCP_FLAG_FIN | TCP_FLAG_ACK)); + ck_assert_uint_eq(ee16(out->src_port), LLK_LISTEN_PORT); + ck_assert_int_eq(lsn->sock.tcp.state, TCP_FIN_WAIT_1); + ck_assert_int_eq(lsn->sock.tcp.ctrl_rto_active, 1); + ck_assert_uint_ne(lsn->sock.tcp.tmr_rto, NO_TIMER); +} +END_TEST diff --git a/src/wolfip.c b/src/wolfip.c index a50bf00d..b3d7eae6 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -4011,7 +4011,11 @@ static void tcp_ctrl_rto_start(struct tsocket *t, uint64_t now) uint64_t shift_rto; if (!t || t->proto != WI_IPPROTO_TCP) return; + /* The control RTO takes over the shared timer slot: tcp_rto_cb + * dispatches on the timeout flags from that same slot, so every flag + * it replaces must be cleared with the timer it armed. */ t->sock.tcp.fin_wait_2_timeout_active = 0; + t->sock.tcp.preaccept_timeout_active = 0; if (t->sock.tcp.tmr_rto != NO_TIMER) { timer_binheap_cancel(&t->S->timers, t->sock.tcp.tmr_rto); t->sock.tcp.tmr_rto = NO_TIMER; From 7968c48f3cc07aff2fc41ae2e1aeb27e96b295b5 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 12:27:48 +0200 Subject: [PATCH 05/17] F-11425: document the PEAP success ack as the 2-byte form it builds The header described eap_peap_build_mschapv2_ack() as building a six-byte EAP Response carrying Code, id and Length. The implementation emits only the two compressed inner bytes [type=26, opcode=Success] and discards eap_id: PEAPv0 omits the inner EAP header. A reader following the header could allocate or serialize a full EAP packet and break the phase-2 framing. --- src/supplicant/eap_peap.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/supplicant/eap_peap.h b/src/supplicant/eap_peap.h index b4bb02a3..437699c3 100644 --- a/src/supplicant/eap_peap.h +++ b/src/supplicant/eap_peap.h @@ -63,9 +63,9 @@ int eap_peap_build_mschapv2_response(uint8_t *out, size_t out_cap, size_t username_len, size_t *out_len); -/* Build the trivial inner EAP-Response/MSCHAPv2 Success ack: 6 bytes, - * [Code=Resp, id, length=6 BE, type=26, opcode=Success] - * sent in reply to the server's "S=..." Success Request. +/* Build the 2-byte compressed inner MSCHAPv2 Success acknowledgment: + * [type=26, opcode=Success], sent in reply to the server's Success + * Request. eap_id is unused because PEAPv0 omits the inner EAP header. */ int eap_peap_build_mschapv2_ack(uint8_t *out, size_t out_cap, uint8_t eap_id, From e622e20ea571b231f8fb71eb54b0292f2c5c5a01 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 12:30:54 +0200 Subject: [PATCH 06/17] F-11426: document the MSK half order and key directions as built The header described the MSK as SendKey16 || RecvKey16 and mapped the client send direction to the MS-MPPE-Recv-Key. The implementation derives the send key with the client-to-server magic and the receive key with the server-to-client magic, then writes MSK = MS-MPPE-Recv-Key || MS-MPPE-Send-Key || 32 zero bytes, the RFC 3748 sec.7.10 order. Code mapping the documented halves to traffic directions would install the keys backwards. State the actual half order, the actual magic per half, and the client send/receive mapping. --- src/supplicant/mschapv2.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/supplicant/mschapv2.h b/src/supplicant/mschapv2.h index 25cf7cb8..58e0a647 100644 --- a/src/supplicant/mschapv2.h +++ b/src/supplicant/mschapv2.h @@ -95,13 +95,13 @@ int mschapv2_verify_authenticator_response( /* Derive the 64-byte EAP-MSCHAPv2 MSK per RFC 3079. * MasterKey = SHA1(PasswordHashHash || NTResponse || MagicConstant1) - * SendKey16 = GetAsymmetricStartKey(MasterKey, 16, server-to-client) - * RecvKey16 = GetAsymmetricStartKey(MasterKey, 16, client-to-server) - * MSK = SendKey16 || RecvKey16 || 32 zero bytes (per RFC 3748) + * SendKey16 = GetAsymmetricStartKey(MasterKey, 16, client-to-server) + * RecvKey16 = GetAsymmetricStartKey(MasterKey, 16, server-to-client) + * MSK = RecvKey16 || SendKey16 || 32 zero bytes (per RFC 3748) * - * Note RFC 3748 sec.7.10 specifies how the EAP MSK is built from - * MSCHAPv2 keys; we follow the "client" perspective: send = MS-MPPE- - * Recv-Key, recv = MS-MPPE-Send-Key, then 32 zero bytes. + * RFC 3748 sec.7.10 builds the EAP MSK from the MSCHAPv2 keys. From the + * client perspective, client send uses the client-to-server key and + * client receive uses the server-to-client key. */ int mschapv2_derive_msk(const char *password, size_t pw_len, const uint8_t nt_response[MSCHAPV2_NT_RESPONSE_LEN], From 42e37971a87646b81928b07e45c7be2ea8467339 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 12:37:23 +0200 Subject: [PATCH 07/17] F-12392: drop the redundant ACK-bit retest before processing The synchronized-connection path already continues when the ACK bit is off (RFC 9293 sec.3.10.7.4 step 5), so the immediately following 'if (ACK)' was always true. Unwrap it; the behavior is unchanged. --- src/wolfip.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/wolfip.c b/src/wolfip.c index b3d7eae6..b4be4744 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -5903,12 +5903,10 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, if (!(tcp->flags & TCP_FLAG_ACK)) continue; - if (tcp->flags & TCP_FLAG_ACK) { - tcp_ack(t, tcp); - if (t->sock.tcp.state == TCP_CLOSED) - continue; - tcp_process_ts(t, tcp, frame_len); - } + tcp_ack(t, tcp); + if (t->sock.tcp.state == TCP_CLOSED) + continue; + tcp_process_ts(t, tcp, frame_len); if (tcplen > 0) { if ((t->sock.tcp.state == TCP_LAST_ACK) || (t->sock.tcp.state == TCP_CLOSING) || (t->sock.tcp.state == TCP_CLOSED)) From e78d3d7d3354bbb6d70a3ce5f19041f6ca7853eb Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 12:37:23 +0200 Subject: [PATCH 08/17] F-12433: do not advance the receive ACK for a FIN that changes no state When an acceptable FIN arrives in a state with no outgoing transition (CLOSE_WAIT, CLOSING, TIME_WAIT, LAST_ACK), the old code still consumed the FIN's sequence number and advanced the receive ACK. A peer - or an attacker - that keeps sending FINs at RCV.NXT could therefore march the ACK forward indefinitely without any state change. Advance the receive ACK and raise the close events only when the FIN actually moved the state machine (ESTABLISHED, FIN_WAIT_1, FIN_WAIT_2); otherwise re-ACK and leave the receive window untouched. Test: test_tcp_fin_in_close_wait_does_not_advance_ack completes a handshake, closes from the peer (CLOSE_WAIT), then sends a second FIN at the new RCV.NXT and asserts the receive ACK did not move. Fails pre-fix: the ACK advances one sequence number per repeated FIN. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_tcp_flow.c | 58 +++++++++++++++++++++++++++++ src/wolfip.c | 20 +++++++--- 3 files changed, 74 insertions(+), 5 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index b20d2f0d..e0560526 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -610,6 +610,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_tcp_listener_preaccept_timeout_reverts_port); tcase_add_test(tc_utils, test_tcp_listener_preaccept_revert_drains_connection_state); tcase_add_test(tc_utils, test_tcp_listener_preaccept_close_rto_retransmits_finack); + tcase_add_test(tc_utils, test_tcp_fin_in_close_wait_does_not_advance_ack); tcase_add_test(tc_utils, test_tcp_listener_revert_restores_option_baseline); tcase_add_test(tc_utils, test_tcp_parse_sack_wraparound_block_accepted); tcase_add_test(tc_utils, test_tcp_parse_options_stops_on_truncated_or_invalid_option_length); diff --git a/src/test/unit/unit_tests_tcp_flow.c b/src/test/unit/unit_tests_tcp_flow.c index efd0600c..f42e3008 100644 --- a/src/test/unit/unit_tests_tcp_flow.c +++ b/src/test/unit/unit_tests_tcp_flow.c @@ -5830,3 +5830,61 @@ START_TEST(test_tcp_listener_preaccept_close_rto_retransmits_finack) ck_assert_uint_ne(lsn->sock.tcp.tmr_rto, NO_TIMER); } END_TEST + +/* An acceptable FIN that arrives in CLOSE_WAIT or CLOSING does not move + * the state machine: the peer's FIN was already consumed on the way in. + * It must not advance the receive ACK either, or a peer that keeps + * sending FINs at RCV.NXT would march the ACK forward with no state + * change. */ +START_TEST(test_tcp_fin_in_close_wait_does_not_advance_ack) +{ + struct wolfIP s; + int fd; + struct tsocket *lsn; + uint8_t seg_buf[sizeof(struct wolfIP_tcp_seg)]; + struct wolfIP_tcp_seg *fin = (struct wolfIP_tcp_seg *)seg_buf; + uint32_t rcv_nxt; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, LLK_LOCAL_IP, LLK_NET_MASK, 0); + fd = llk_open_listener(&s); + lsn = &s.tcpsockets[SOCKET_UNMARK(fd)]; + + llk_keep_arp_fresh(&s, LLK_ATT_IP); + + llk_attacker_syn(&s, LLK_ATT_IP, 41000, 1, 0); + llk_complete_handshake(&s, lsn, LLK_ATT_IP, 41000, 1); + ck_assert_int_eq(lsn->sock.tcp.state, TCP_ESTABLISHED); + rcv_nxt = lsn->sock.tcp.ack; + + /* The peer's FIN at RCV.NXT: ESTABLISHED -> CLOSE_WAIT, ACK advances. */ + memset(seg_buf, 0, sizeof(seg_buf)); + fin->ip.ver_ihl = 0x45; + fin->ip.proto = WI_IPPROTO_TCP; + fin->ip.ttl = 64; + fin->ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN); + fin->ip.src = ee32(lsn->remote_ip); + fin->ip.dst = ee32(lsn->local_ip); + fin->dst_port = ee16(lsn->src_port); + fin->src_port = ee16(lsn->dst_port); + fin->seq = ee32(rcv_nxt); + fin->ack = ee32(tcp_seq_inc(lsn->sock.tcp.snd_una, 1)); + fin->hlen = TCP_HEADER_LEN << 2; + fin->flags = TCP_FLAG_FIN | TCP_FLAG_ACK; + fix_tcp_checksums(fin); + tcp_input(&s, TEST_PRIMARY_IF, fin, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN)); + ck_assert_int_eq(lsn->sock.tcp.state, TCP_CLOSE_WAIT); + ck_assert_uint_eq(lsn->sock.tcp.ack, rcv_nxt + 1); + + /* A second FIN at the new RCV.NXT: no state change in CLOSE_WAIT, + * and the receive ACK must not advance. */ + fin->seq = ee32(rcv_nxt + 1); + fix_tcp_checksums(fin); + tcp_input(&s, TEST_PRIMARY_IF, fin, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN)); + ck_assert_int_eq(lsn->sock.tcp.state, TCP_CLOSE_WAIT); + ck_assert_uint_eq(lsn->sock.tcp.ack, rcv_nxt + 1); +} +END_TEST diff --git a/src/wolfip.c b/src/wolfip.c index b4be4744..edc069b9 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -5917,6 +5917,7 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, uint32_t seq = ee32(tcp->seq); uint32_t fin_seq_end = tcp_seq_inc(seq, tcplen); int accept_fin = 1; + int transitioned = 0; if ((tcplen == 0 && t->sock.tcp.ack != seq) || (tcplen > 0 && t->sock.tcp.ack != fin_seq_end)) { @@ -5929,18 +5930,27 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, (void)wolfIP_filter_notify_socket_event( WOLFIP_FILT_CLOSE_WAIT, S, t, t->local_ip, t->src_port, t->remote_ip, t->dst_port); + transitioned = 1; } else if (t->sock.tcp.state == TCP_FIN_WAIT_1) { t->sock.tcp.state = TCP_CLOSING; + transitioned = 1; } else if (t->sock.tcp.state == TCP_FIN_WAIT_2) { tcp_fin_wait_2_timeout_stop(t); t->sock.tcp.state = TCP_TIME_WAIT; + transitioned = 1; } - if (tcplen > 0) { - t->sock.tcp.ack = tcp_seq_inc(fin_seq_end, 1); - } else { - t->sock.tcp.ack = tcp_seq_inc(seq, 1); + /* Only a state transition consumes the FIN's + * sequence number: a FIN that re-arrives in + * CLOSE_WAIT, CLOSING or TIME_WAIT is re-ACKed + * without advancing the receive ACK. */ + if (transitioned) { + if (tcplen > 0) { + t->sock.tcp.ack = tcp_seq_inc(fin_seq_end, 1); + } else { + t->sock.tcp.ack = tcp_seq_inc(seq, 1); + } + t->events |= CB_EVENT_CLOSED | CB_EVENT_READABLE; } - t->events |= CB_EVENT_CLOSED | CB_EVENT_READABLE; tcp_send_ack(t); } else { tcp_send_ack(t); From 7c94890b998b5f92e5ccaae4a803f81a8c412eff Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 14:23:57 +0200 Subject: [PATCH 09/17] F-12432: don't answer ARP requests on an unconfigured interface arp_recv matched ARP_REQUEST against tip == conf->ip without checking the interface has an address. On an interface with no assigned address (conf->ip == IPADDR_ANY) a request for 0.0.0.0 matched (0 == 0) and the stack replied advertising 0.0.0.0 as the sender protocol address, leaking a bogus on-link binding into a peer's cache. Add conf->ip != IPADDR_ANY to the request match so only interfaces with a real configured address answer. Test: test_arp_recv_unconfigured_if_does_not_answer (two-interface stack) asserts a 0.0.0.0 request on the unconfigured secondary sends no frame, and a request for the primary's address on the configured primary is answered. Fails pre-fix: unconfigured secondary answers 0.0.0.0 (last_frame_sent_count 1 vs 0). --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_api.c | 65 ++++++++++++++++++++++++++++++++++ src/wolfip.c | 6 +++- 3 files changed, 71 insertions(+), 1 deletion(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index e0560526..87f805f0 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -280,6 +280,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_ip_recv_drops_zero_source); tcase_add_test(tc_utils, test_arp_recv_rejects_broadcast_sender); tcase_add_test(tc_utils, test_arp_recv_rejects_multicast_sender); + tcase_add_test(tc_utils, test_arp_recv_unconfigured_if_does_not_answer); tcase_add_test(tc_utils, test_dhcp_ack_rejects_mismatched_server_id); tcase_add_test(tc_utils, test_udp_no_icmp_unreachable_for_broadcast_src); tcase_add_test(tc_utils, test_udp_no_icmp_unreachable_for_multicast_src); diff --git a/src/test/unit/unit_tests_api.c b/src/test/unit/unit_tests_api.c index 86ae3d5f..b43a6ae2 100644 --- a/src/test/unit/unit_tests_api.c +++ b/src/test/unit/unit_tests_api.c @@ -4776,6 +4776,71 @@ START_TEST(test_arp_recv_rejects_multicast_sender) } END_TEST +/* An unconfigured interface (no assigned address) must not answer ARP + * requests: matching the target against a zero conf->ip let a request + * for 0.0.0.0 be answered by advertising 0.0.0.0 as the sender protocol + * address. */ +START_TEST(test_arp_recv_unconfigured_if_does_not_answer) +{ + struct wolfIP s; + struct arp_packet arp; + struct wolfIP_ll_dev *ll; + struct ipconf *conf; + static const uint8_t fake_mac[6] = {0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x03}; + uint32_t frames_before; + + wolfIP_init(&s); + mock_link_init(&s); + mock_link_init_idx(&s, TEST_SECOND_IF, NULL); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + conf = wolfIP_ipconf_at(&s, TEST_SECOND_IF); + ck_assert_uint_eq(conf->ip, IPADDR_ANY); + + /* A request for 0.0.0.0 on the unconfigured secondary: no reply. */ + ll = wolfIP_getdev_ex(&s, TEST_SECOND_IF); + memset(&arp, 0, sizeof(arp)); + memcpy(arp.eth.dst, ll->mac, 6); + memcpy(arp.eth.src, fake_mac, 6); + arp.eth.type = ee16(ETH_TYPE_ARP); + arp.htype = ee16(1); + arp.ptype = ee16(0x0800); + arp.hlen = 6; + arp.plen = 4; + arp.opcode = ee16(ARP_REQUEST); + memcpy(arp.sma, fake_mac, 6); + arp.sip = ee32(0x0A000002U); + memset(arp.tma, 0, 6); + arp.tip = ee32(IPADDR_ANY); + + frames_before = last_frame_sent_count; + arp_recv(&s, TEST_SECOND_IF, &arp, sizeof(arp)); + ck_assert_uint_eq(last_frame_sent_count, frames_before); + + /* Control: a request for the primary's IP on the configured primary + * is still answered. */ + ll = wolfIP_getdev_ex(&s, TEST_PRIMARY_IF); + conf = wolfIP_ipconf_at(&s, TEST_PRIMARY_IF); + memset(&arp, 0, sizeof(arp)); + memcpy(arp.eth.dst, ll->mac, 6); + memcpy(arp.eth.src, fake_mac, 6); + arp.eth.type = ee16(ETH_TYPE_ARP); + arp.htype = ee16(1); + arp.ptype = ee16(0x0800); + arp.hlen = 6; + arp.plen = 4; + arp.opcode = ee16(ARP_REQUEST); + memcpy(arp.sma, fake_mac, 6); + arp.sip = ee32(0x0A000002U); + memset(arp.tma, 0, 6); + arp.tip = ee32(conf->ip); + + frames_before = last_frame_sent_count; + arp_recv(&s, TEST_PRIMARY_IF, &arp, sizeof(arp)); + ck_assert_uint_eq(last_frame_sent_count, frames_before + 1); +} +END_TEST + /* Regression: arp_recv must reject ARP packets with incorrect hardware or * protocol type fields (htype != 1, ptype != 0x0800, hlen != 6, plen != 4). * Without validation, non-Ethernet/IPv4 ARP packets pollute the cache. */ diff --git a/src/wolfip.c b/src/wolfip.c index edc069b9..6458814f 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -9824,7 +9824,11 @@ static void arp_recv(struct wolfIP *s, unsigned int if_idx, void *buf, int len) if (arp->sma[0] & 0x01) return; - if (arp->opcode == ee16(ARP_REQUEST) && arp->tip == ee32(conf->ip)) { + /* An unconfigured interface (no assigned address) must not answer + * ARP requests: matching tip against a zero conf->ip would let a + * request for 0.0.0.0 be answered by advertising 0.0.0.0. */ + if (arp->opcode == ee16(ARP_REQUEST) && conf->ip != IPADDR_ANY && + arp->tip == ee32(conf->ip)) { uint32_t sender_ip = arp->sip; uint8_t sender_mac[6]; memcpy(sender_mac, arp->sma, 6); From add22bf00a960cabce523a59594ceb098cd236d6 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 15:22:51 +0200 Subject: [PATCH 10/17] F-12430: bind DAD conflict detection to the probing interface The DHCP DAD reply hook in arp_recv compared the reply's sender IP against the RECEIVING interface's conf->ip. DAD is a per-interface operation: the probes go out on the primary interface, so a conflict reply is only meaningful on that interface. Comparing against the receiving interface's address was wrong in two ways: on an unconfigured interface conf->ip is IPADDR_ANY, so a spoofed reply with a zero sender IP matched 0 == 0 and forced dhcp_dad_conflict() on the primary lease before the zero-address rejection further down could drop it; and a reply on any other interface cannot be a response to the probes sent on the primary. Record the DAD interface (s->dhcp_dad_if, set to the primary at DAD start) and match the reply on that interface against the recorded candidate (s->dhcp_ip) instead of the receiving interface's address. A reply now triggers a conflict only when it arrives on the probing interface and claims the candidate. Test: test_dhcp_dad_reply_on_unconfigured_secondary_ignored (two-interface stack, secondary left unconfigured) asserts a spoofed sip=0.0.0.0 reply from a foreign MAC on the secondary leaves DAD running and the primary lease intact. Fails pre-fix: the reply forced DHCP_DISCOVER_SENT and released the lease instead of staying in DHCP_DAD. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_dhcp_edges.c | 58 +++++++++++++++++++++++++++ src/wolfip.c | 12 ++++-- 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 87f805f0..1e57c954 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1478,6 +1478,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_dhcp_dad_conflict_releases_and_rediscover); tcase_add_test(tc_core, test_dhcp_dad_own_mac_reply_ignored); tcase_add_test(tc_core, test_dhcp_dad_reply_for_other_ip_ignored); + tcase_add_test(tc_core, test_dhcp_dad_reply_on_unconfigured_secondary_ignored); tcase_add_test(tc_core, test_dhcp_dad_single_dhcp_timer_in_heap); tcase_add_test(tc_core, test_dhcp_dad_probe_count_len_returning_driver); tcase_add_test(tc_core, test_dhcp_decline_wire_format); diff --git a/src/test/unit/unit_tests_dhcp_edges.c b/src/test/unit/unit_tests_dhcp_edges.c index 2a948b4d..b032fbff 100644 --- a/src/test/unit/unit_tests_dhcp_edges.c +++ b/src/test/unit/unit_tests_dhcp_edges.c @@ -1823,6 +1823,64 @@ START_TEST(test_dhcp_dad_reply_for_other_ip_ignored) } END_TEST +/* During DAD, a spoofed reply (sip = 0.0.0.0) arriving on an unconfigured + * secondary interface must not force a conflict on the primary lease: the + * DAD hook is bound to the probing interface and the recorded candidate, + * not the receiving interface's (zero) address. */ +START_TEST(test_dhcp_dad_reply_on_unconfigured_secondary_ignored) +{ + struct wolfIP s; + struct dhcp_msg msg; + struct arp_packet reply; + struct wolfIP_ll_dev *ll; + struct ipconf *primary; + uint32_t server_ip = 0x0A000001U; + uint32_t client_ip = 0x0A000064U; + uint8_t other_mac[6] = {0x02, 0xAA, 0xBB, 0xCC, 0xDD, 0x03}; + + /* Primary holds the candidate; the secondary is left unconfigured so + * its conf->ip is IPADDR_ANY (the trap the old check fell into). */ + setup_stack_with_two_ifaces(&s, client_ip, 0x0A010001U); + wolfIP_ipconf_at(&s, TEST_SECOND_IF)->ip = IPADDR_ANY; + s.dhcp_xid = 0xDA06U; + s.dhcp_state = DHCP_REQUEST_SENT; + s.last_tick = 1000U; + primary = wolfIP_primary_ipconf(&s); + ck_assert_ptr_nonnull(primary); + primary->ip = client_ip; + + build_full_ack(&s, &msg, server_ip, client_ip, 0xFFFFFF00U, + server_ip, 0x08080808U, 120U); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), 0); + ck_assert_int_eq(s.dhcp_state, DHCP_DAD); + + /* A spoofed reply with a zero sender IP on the unconfigured secondary: + * the old check (sip == conf->ip) saw 0 == 0 and forced a conflict on + * the primary lease. The DAD hook must stay on the probing interface. */ + ll = wolfIP_getdev_ex(&s, TEST_SECOND_IF); + ck_assert_ptr_nonnull(ll); + memset(&reply, 0, sizeof(reply)); + memcpy(reply.eth.dst, ll->mac, 6); + memcpy(reply.eth.src, other_mac, 6); + reply.eth.type = ee16(ETH_TYPE_ARP); + reply.htype = ee16(1); + reply.ptype = ee16(0x0800); + reply.hlen = 6; + reply.plen = 4; + reply.opcode = ee16(ARP_REPLY); + memcpy(reply.sma, other_mac, 6); + reply.sip = ee32(IPADDR_ANY); + reply.tip = ee32(client_ip); + + arp_recv(&s, TEST_SECOND_IF, &reply, sizeof(reply)); + + /* No conflict: DAD continues, the primary lease is intact. */ + ck_assert_int_eq(s.dhcp_state, DHCP_DAD); + ck_assert_uint_eq(s.dhcp_dad_probes, 1U); + ck_assert_uint_eq(primary->ip, client_ip); +} +END_TEST + /* Real ll drivers (stm32, lpc, fman, gem, tap) return the frame length on * send success, not 0. The DAD probe counter must treat any non-negative * return as "sent"; otherwise the first probe is miscounted, a fourth probe diff --git a/src/wolfip.c b/src/wolfip.c index 6458814f..b475564b 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -1464,6 +1464,7 @@ struct wolfIP { uint32_t dhcp_timer; /* Timer for DHCP */ uint32_t dhcp_timeout_count; /* DHCP timeout counter */ uint8_t dhcp_dad_probes; /* DAD probes sent (0 = DAD inactive) */ + uint8_t dhcp_dad_if; /* Interface DAD probes on (valid in DHCP_DAD) */ ip4 dhcp_server_ip; /* DHCP server IP */ ip4 dhcp_ip; /* IP address assigned by DHCP */ uint32_t dhcp_offered_mask; /* netmask from the accepted OFFER */ @@ -9138,6 +9139,7 @@ static int dhcp_parse_ack(struct wolfIP *s, struct dhcp_msg *msg, uint32_t msg_l * arp_recv (dhcp_dad_conflict). */ s->dhcp_state = DHCP_DAD; s->dhcp_dad_probes = 0; + s->dhcp_dad_if = WOLFIP_PRIMARY_IF_IDX; /* Arm the lease absolutes, then swap the renew timer for * the DAD timer: handle_timers() fires every expired * entry, so leaving both in the heap would double-fire @@ -9859,10 +9861,12 @@ static void arp_recv(struct wolfIP *s, unsigned int if_idx, void *buf, int len) else if (arp->opcode == ee16(ARP_REPLY)) { ip4 sip = ee32(arp->sip); int pending; - /* RFC 4331 DAD: a reply claiming the address being probed means - * it is in use, unless it came from our own MAC (looped probe). - * This is the one case where a reply for our own IP is acted on. */ - if (s->dhcp_state == DHCP_DAD && sip == conf->ip) { + /* RFC 4331 DAD: a reply on the probing interface claiming the + * candidate is a conflict, unless it is our own MAC (looped probe). + * Bound to the DAD interface + recorded candidate so a reply on + * another (e.g. unconfigured) interface cannot force one. */ + if (s->dhcp_state == DHCP_DAD && if_idx == s->dhcp_dad_if && + sip == s->dhcp_ip) { if (memcmp(arp->sma, ll->mac, 6) != 0) dhcp_dad_conflict(s); return; From a4a75e980d9c958f036ce2f77f4717a9e9ff3557 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 15:43:07 +0200 Subject: [PATCH 11/17] F-12431: detect DAD conflicts from ARP requests, not just replies A host that owns the candidate address can evade DAD by never answering our probe (a reply was the only conflict signal the reply-branch checked). It betrays itself by using the address: an ARP request that claims the candidate as its sender IP, or a probe for it (its own DAD), both prove the address is taken. On the probing interface, during DAD, a request from a foreign MAC that claims the candidate (sip == candidate, tip != candidate) or probes for it (sip == 0.0.0.0, tip == candidate) now aborts DAD via dhcp_dad_conflict() before the normal reply path, and no reply is sent. Bound to s->dhcp_dad_if + s->dhcp_ip (the F-12430 fields) so our own probe (own MAC) and non-DAD traffic are unaffected. Tests: test_dhcp_dad_request_claiming_candidate_conflict and test_dhcp_dad_probe_for_candidate_conflict. Fails pre-fix: both stay in DHCP_DAD (the request branch ignored them) instead of aborting to DHCP_DISCOVER_SENT with the lease released. --- src/test/unit/unit.c | 2 + src/test/unit/unit_tests_dhcp_edges.c | 108 ++++++++++++++++++++++++++ src/wolfip.c | 15 ++++ 3 files changed, 125 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 1e57c954..3e318852 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1476,6 +1476,8 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_dhcp_parse_ack_inner_pad_bytes_skipped); tcase_add_test(tc_core, test_dhcp_dad_probe_wire_format); tcase_add_test(tc_core, test_dhcp_dad_conflict_releases_and_rediscover); + tcase_add_test(tc_core, test_dhcp_dad_request_claiming_candidate_conflict); + tcase_add_test(tc_core, test_dhcp_dad_probe_for_candidate_conflict); tcase_add_test(tc_core, test_dhcp_dad_own_mac_reply_ignored); tcase_add_test(tc_core, test_dhcp_dad_reply_for_other_ip_ignored); tcase_add_test(tc_core, test_dhcp_dad_reply_on_unconfigured_secondary_ignored); diff --git a/src/test/unit/unit_tests_dhcp_edges.c b/src/test/unit/unit_tests_dhcp_edges.c index b032fbff..a9061cec 100644 --- a/src/test/unit/unit_tests_dhcp_edges.c +++ b/src/test/unit/unit_tests_dhcp_edges.c @@ -1689,6 +1689,114 @@ START_TEST(test_dhcp_dad_conflict_releases_and_rediscover) } END_TEST +/* During DAD, a foreign host that uses the candidate as its sender IP (an + * ARP request, not a reply) must be detected as a conflict: a host that + * owns the candidate can evade DAD by not answering our probe, but betrays + * itself by using the address. */ +START_TEST(test_dhcp_dad_request_claiming_candidate_conflict) +{ + struct wolfIP s; + struct dhcp_msg msg; + struct arp_packet req; + struct wolfIP_ll_dev *ll; + struct ipconf *primary; + uint32_t server_ip = 0x0A000001U; + uint32_t client_ip = 0x0A000064U; + uint8_t other_mac[6] = {0x02, 0xAA, 0xBB, 0xCC, 0xDD, 0x04}; + + wolfIP_init(&s); + mock_link_init(&s); + s.dhcp_xid = 0xDA07U; + s.dhcp_state = DHCP_REQUEST_SENT; + s.last_tick = 1000U; + primary = wolfIP_primary_ipconf(&s); + ck_assert_ptr_nonnull(primary); + primary->ip = client_ip; + + build_full_ack(&s, &msg, server_ip, client_ip, 0xFFFFFF00U, + server_ip, 0x08080808U, 120U); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), 0); + ck_assert_int_eq(s.dhcp_state, DHCP_DAD); + + /* A foreign host using the candidate as its sender IP (request, not + * reply): the DAD must treat it as a conflict, not answer it. */ + ll = wolfIP_getdev_ex(&s, TEST_PRIMARY_IF); + ck_assert_ptr_nonnull(ll); + memset(&req, 0, sizeof(req)); + memcpy(req.eth.dst, ll->mac, 6); + memcpy(req.eth.src, other_mac, 6); + req.eth.type = ee16(ETH_TYPE_ARP); + req.htype = ee16(1); + req.ptype = ee16(0x0800); + req.hlen = 6; + req.plen = 4; + req.opcode = ee16(ARP_REQUEST); + memcpy(req.sma, other_mac, 6); + req.sip = ee32(client_ip); + req.tip = ee32(0x0A000002U); + + arp_recv(&s, TEST_PRIMARY_IF, &req, sizeof(req)); + + /* Conflict detected: DAD aborted, lease released, back to DISCOVER. */ + ck_assert_int_eq(s.dhcp_state, DHCP_DISCOVER_SENT); + ck_assert_uint_eq(s.dhcp_dad_probes, 0U); + ck_assert_uint_eq(primary->ip, 0U); +} +END_TEST + +/* During DAD, a foreign host probing for the candidate (its own DAD: an ARP + * request with sip = 0.0.0.0, tip = candidate) is a conflict: two hosts + * cannot bind the same address. Our own probe is excluded by the MAC check. */ +START_TEST(test_dhcp_dad_probe_for_candidate_conflict) +{ + struct wolfIP s; + struct dhcp_msg msg; + struct arp_packet req; + struct wolfIP_ll_dev *ll; + struct ipconf *primary; + uint32_t server_ip = 0x0A000001U; + uint32_t client_ip = 0x0A000064U; + uint8_t other_mac[6] = {0x02, 0xAA, 0xBB, 0xCC, 0xDD, 0x05}; + + wolfIP_init(&s); + mock_link_init(&s); + s.dhcp_xid = 0xDA08U; + s.dhcp_state = DHCP_REQUEST_SENT; + s.last_tick = 1000U; + primary = wolfIP_primary_ipconf(&s); + ck_assert_ptr_nonnull(primary); + primary->ip = client_ip; + + build_full_ack(&s, &msg, server_ip, client_ip, 0xFFFFFF00U, + server_ip, 0x08080808U, 120U); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), 0); + ck_assert_int_eq(s.dhcp_state, DHCP_DAD); + + /* A foreign host probing for the candidate (its own DAD): conflict. */ + ll = wolfIP_getdev_ex(&s, TEST_PRIMARY_IF); + ck_assert_ptr_nonnull(ll); + memset(&req, 0, sizeof(req)); + memcpy(req.eth.dst, ll->mac, 6); + memcpy(req.eth.src, other_mac, 6); + req.eth.type = ee16(ETH_TYPE_ARP); + req.htype = ee16(1); + req.ptype = ee16(0x0800); + req.hlen = 6; + req.plen = 4; + req.opcode = ee16(ARP_REQUEST); + memcpy(req.sma, other_mac, 6); + req.sip = ee32(IPADDR_ANY); + req.tip = ee32(client_ip); + + arp_recv(&s, TEST_PRIMARY_IF, &req, sizeof(req)); + + /* Conflict detected: DAD aborted, lease released, back to DISCOVER. */ + ck_assert_int_eq(s.dhcp_state, DHCP_DISCOVER_SENT); + ck_assert_uint_eq(s.dhcp_dad_probes, 0U); + ck_assert_uint_eq(primary->ip, 0U); +} +END_TEST + /* A reply with our own MAC is our own probe looping back: ignored, DAD * continues. */ START_TEST(test_dhcp_dad_own_mac_reply_ignored) diff --git a/src/wolfip.c b/src/wolfip.c index b475564b..9808f300 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -9826,6 +9826,21 @@ static void arp_recv(struct wolfIP *s, unsigned int if_idx, void *buf, int len) if (arp->sma[0] & 0x01) return; + /* RFC 4331/5227 DAD: on the probing interface, a request from a + * foreign MAC that claims the candidate (sender IP) or probes for it + * is a conflict - a host that owns the candidate may not answer our + * probe (DAD evasion), but betrays itself by using/probing the IP. */ + if (arp->opcode == ee16(ARP_REQUEST) && s->dhcp_state == DHCP_DAD && + if_idx == s->dhcp_dad_if && memcmp(arp->sma, ll->mac, 6) != 0) { + ip4 sip = ee32(arp->sip); + ip4 tip = ee32(arp->tip); + if ((sip == s->dhcp_ip && tip != s->dhcp_ip) || + (sip == IPADDR_ANY && tip == s->dhcp_ip)) { + dhcp_dad_conflict(s); + return; + } + } + /* An unconfigured interface (no assigned address) must not answer * ARP requests: matching tip against a zero conf->ip would let a * request for 0.0.0.0 be answered by advertising 0.0.0.0. */ From 40e1a6cfba85d2d11993a97a76402550400b3af5 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 15:58:06 +0200 Subject: [PATCH 12/17] F-12387: abort the DNS query when the timeout timer cannot be scheduled dns_schedule_timer() ignored the timers_binheap_insert() result, so when the timer heap was full the DNS timeout was never armed while dns_id stayed set. A lost response then had no retry or abort path, and the busy guard (dns_id != 0) blocked every later lookup forever. Make dns_schedule_timer() report the insert result and abort the armed query on failure: the initial schedule in dns_send_query() rolls back the query state and returns an error; a retry rearm in dns_timeout_cb() aborts the query. Fails pre-fix: test_dns_send_query_timer_heap_full_aborts (heap filled to MAX_TIMERS, dns_send_query returned 0 and left dns_id set). --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_dns_dhcp.c | 31 +++++++++++++++++++++++++++++ src/wolfip.c | 21 +++++++++++++++---- 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 3e318852..c2da40cb 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -537,6 +537,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_dns_schedule_timer_initial_jitter_and_cancel); tcase_add_test(tc_utils, test_dns_schedule_timer_caps_large_retry_shift); tcase_add_test(tc_utils, test_dns_send_query_schedules_timeout); + tcase_add_test(tc_utils, test_dns_send_query_timer_heap_full_aborts); tcase_add_test(tc_utils, test_dns_send_query_send_failure_clears_outstanding_state); tcase_add_test(tc_utils, test_dns_resend_query_uses_stored_query_buffer); tcase_add_test(tc_utils, test_dns_resend_query_fails_without_valid_socket); diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index ebc9de5d..5cb942b7 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -3169,6 +3169,37 @@ START_TEST(test_dns_send_query_schedules_timeout) } END_TEST +/* When the timer heap is full, dns_send_query must abort the armed query + * (clearing dns_id) and return an error: a failed timeout insert would + * leave the DNS busy guard (dns_id != 0) set with no timer to clear it, + * wedging the resolver for every later lookup. */ +START_TEST(test_dns_send_query_timer_heap_full_aborts) +{ + struct wolfIP s; + struct wolfIP_timer t; + uint16_t id = 0; + int i; + + wolfIP_init(&s); + mock_link_init(&s); + s.dns_server = 0x08080808U; + s.last_tick = 100U; + + /* Fill the timer heap so the DNS timeout insert fails. */ + for (i = 0; i < MAX_TIMERS; i++) { + t.expires = s.last_tick + 1000U + (uint64_t)i; + t.arg = NULL; + t.cb = NULL; + (void)timers_binheap_insert(&s.timers, t); + } + + /* The query must abort (dns_id cleared) rather than wedge the resolver. */ + ck_assert_int_ne(dns_send_query(&s, "example.com", &id, DNS_A), 0); + ck_assert_uint_eq(s.dns_id, 0U); + ck_assert_uint_eq(s.dns_timer, NO_TIMER); +} +END_TEST + START_TEST(test_dns_send_query_send_failure_clears_outstanding_state) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 9808f300..ba1ba738 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -10994,14 +10994,14 @@ static void dns_cancel_timer(struct wolfIP *s) } } -static void dns_schedule_timer(struct wolfIP *s) +static int dns_schedule_timer(struct wolfIP *s) { struct wolfIP_timer tmr = { }; uint64_t interval = DNS_QUERY_TIMEOUT; uint8_t shift; if (!s) - return; + return -1; if (s->dns_retry_count == 0) { /* RFC 1035 recommends a 2s initial retransmission interval. On embedded * targets, add a small 0..390 ms random offset to 1800 ms so many @@ -11019,6 +11019,9 @@ static void dns_schedule_timer(struct wolfIP *s) tmr.arg = s; tmr.cb = dns_timeout_cb; s->dns_timer = timers_binheap_insert(&s->timers, tmr); + if (s->dns_timer == NO_TIMER) + return -1; + return 0; } static int dns_resend_query(struct wolfIP *s) @@ -11070,7 +11073,10 @@ static void dns_timeout_cb(void *arg) return; } s->dns_retry_count++; - dns_schedule_timer(s); + if (dns_schedule_timer(s) != 0) { + dns_abort_query(s); + return; + } } else { dns_abort_query(s); } @@ -11282,7 +11288,14 @@ static int dns_send_query(struct wolfIP *s, const char *dname, uint16_t *id, *id = DNS_ID_NONE; return ret; } - dns_schedule_timer(s); + if (dns_schedule_timer(s) != 0) { + /* Timer heap full: abort the armed query, or the busy guard + * (dns_id != 0) would block every later lookup with no timer to + * clear it. */ + dns_abort_query(s); + *id = DNS_ID_NONE; + return -1; + } return 0; } From e70617581ada6f692ff0687c2d425d101012e28c Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 16:28:09 +0200 Subject: [PATCH 13/17] F-12386: skip in-use ports in the auto source-port/ICMP-id allocators The inline auto-allocators for TCP connect, UDP sendto, and ICMP echo request picked a random port (or ICMP id) without checking whether it was already claimed by another socket. A collision with a bound port would make the new socket receive traffic intended for the existing one. port_alloc_random() picks a random value >= min_port and walks forward on a collision (wrapping to min_port, 16 tries), using the existing bind_port_in_use() scan. TCP passes its resolved local_ip; UDP and ICMP pass IPADDR_ANY because their route is resolved after the alloc (the scan then compares ports only). test_udp_auto_port_skips_in_use pins the RNG to a bound port (5000) and asserts the auto port walks to 5001. Fails pre-fix (auto port == 5000). --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_api.c | 48 ++++++++++++++++++++++++++++++++++ src/wolfip.c | 47 +++++++++++++++++++++++++-------- 3 files changed, 85 insertions(+), 11 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index c2da40cb..6a8dcf8d 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -211,6 +211,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_sock_bind_tcp_filter_blocks); tcase_add_test(tc_utils, test_sock_bind_tcp_port_collision_rejected); tcase_add_test(tc_utils, test_sock_bind_udp_src_port_nonzero); + tcase_add_test(tc_utils, test_udp_auto_port_skips_in_use); tcase_add_test(tc_utils, test_sock_bind_udp_filter_blocks); tcase_add_test(tc_utils, test_sock_bind_icmp_success); tcase_add_test(tc_utils, test_sock_connect_wrong_family); diff --git a/src/test/unit/unit_tests_api.c b/src/test/unit/unit_tests_api.c index b43a6ae2..315670a7 100644 --- a/src/test/unit/unit_tests_api.c +++ b/src/test/unit/unit_tests_api.c @@ -1507,6 +1507,54 @@ START_TEST(test_sock_bind_udp_src_port_nonzero) } END_TEST +/* An auto-assigned UDP source port must not collide with a port already + * bound by another socket: the allocator must skip in-use ports. With the + * RNG pinned to 5000 (the bound port), the fix walks forward to 5001. */ +START_TEST(test_udp_auto_port_skips_in_use) +{ + struct wolfIP s; + int udp_sd1, udp_sd2; + struct tsocket *ts2; + struct wolfIP_sockaddr_in sin; + const char payload[] = "test"; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + /* Bind the first UDP socket to port 5000. */ + udp_sd1 = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_gt(udp_sd1, 0); + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_port = ee16(5000); + sin.sin_addr.s_addr = ee32(0x0A000001U); + ck_assert_int_eq(wolfIP_sock_bind(&s, udp_sd1, + (struct wolfIP_sockaddr *)&sin, sizeof(sin)), 0); + + /* Pin the RNG to 5000 (the bound port); the auto allocator must walk + * forward to 5001 instead of colliding. */ + test_rand_override_enabled = 1; + test_rand_override_value = 5000U; + + /* Create a second UDP socket and sendto (auto-assigns a source port). */ + udp_sd2 = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_gt(udp_sd2, 0); + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_port = ee16(9999); + sin.sin_addr.s_addr = ee32(0x0A000002U); + ck_assert_int_ge(wolfIP_sock_sendto(&s, udp_sd2, payload, sizeof(payload), 0, + (const struct wolfIP_sockaddr *)&sin, + sizeof(sin)), 0); + test_rand_override_enabled = 0; + + /* The auto port must be 5001 (not the bound 5000). */ + ts2 = &s.udpsockets[SOCKET_UNMARK(udp_sd2)]; + ck_assert_uint_eq(ts2->src_port, 5001U); +} +END_TEST + START_TEST(test_sock_bind_udp_filter_blocks) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index ba1ba738..992f1e47 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -6409,6 +6409,34 @@ int wolfIP_sock_socket(struct wolfIP *s, int domain, int type, int protocol) return -1; } +/* Forward declaration: defined below, used by port_alloc_random. */ +static int bind_port_in_use(const struct tsocket *arr, int n, + const struct tsocket *self, + ip4 new_local_ip, uint16_t new_port); + +/* Pick a source port (or ICMP id) that no other socket in arr claims. + * Start from a random value >= min_port, then walk forward on a + * collision (wrapping to min_port). local_ip may be IPADDR_ANY when the + * route is not resolved yet; the check then compares ports only. */ +static uint16_t port_alloc_random(const struct tsocket *arr, int n, + const struct tsocket *self, + ip4 local_ip, uint16_t min_port) +{ + uint16_t port; + uint16_t tries; + port = (uint16_t)(wolfIP_getrandom() & 0xFFFF); + if (port < min_port) + port += min_port; + for (tries = 0; tries < 16; tries++) { + if (!bind_port_in_use(arr, n, self, local_ip, port)) + return port; + port++; + if (port < min_port) + port = min_port; + } + return port; +} + int wolfIP_sock_connect(struct wolfIP *s, int sockfd, const struct wolfIP_sockaddr *addr, socklen_t addrlen) { @@ -6581,7 +6609,8 @@ int wolfIP_sock_connect(struct wolfIP *s, int sockfd, const struct wolfIP_sockad ts->if_idx = new_if_idx; ts->local_ip = new_local_ip; if (!ts->src_port) - ts->src_port = (uint16_t)(wolfIP_getrandom() & 0xFFFF); + ts->src_port = port_alloc_random(s->tcpsockets, MAX_TCPSOCKETS, + ts, ts->local_ip, 1024); if (ts->src_port < 1024) ts->src_port += 1024; ts->dst_port = ee16(sin->sin_port); @@ -6840,11 +6869,9 @@ int wolfIP_sock_sendto(struct wolfIP *s, int sockfd, const void *buf, size_t len } if ((ts->dst_port==0) || (ts->remote_ip==0)) return -1; - if (ts->src_port == 0) { - ts->src_port = (uint16_t)(wolfIP_getrandom() & 0xFFFF); - if (ts->src_port < 1024) - ts->src_port += 1024; - } + if (ts->src_port == 0) + ts->src_port = port_alloc_random(s->udpsockets, MAX_UDPSOCKETS, + ts, IPADDR_ANY, 1024); if_idx = wolfIP_route_for_ip(s, ts->remote_ip); #ifdef IP_MULTICAST if (wolfIP_ip_is_multicast(ts->remote_ip) && ts->sock.udp.mcast_if_set) @@ -6900,11 +6927,9 @@ int wolfIP_sock_sendto(struct wolfIP *s, int sockfd, const void *buf, size_t len } if (ts->remote_ip == 0) return -1; - if (ts->src_port == 0) { - ts->src_port = (uint16_t)(wolfIP_getrandom() & 0xFFFF); - if (ts->src_port == 0) - ts->src_port = 1; - } + if (ts->src_port == 0) + ts->src_port = port_alloc_random(s->icmpsockets, MAX_ICMPSOCKETS, + ts, IPADDR_ANY, 1); if (ts->bound_local_ip != IPADDR_ANY) { int bound_match = 0; unsigned int bound_if = wolfIP_if_for_local_ip(s, ts->bound_local_ip, &bound_match); From c16646ab7d89fd80828484d4e7c348e02d739dfa Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 17:16:49 +0200 Subject: [PATCH 14/17] F-11445: do not mark the control RTO active when the timer insert fails tcp_ctrl_rto_start set ctrl_rto_active=1 even when timers_binheap_insert returned NO_TIMER (heap full). The active flag then pointed at no timer: the RTO could never fire and suppressed every other timeout on the socket until a later event cleared the flag. Return 0 on success, -1 when the insert fails, and only set ctrl_rto_active when the timer actually took a slot. Callers that were already returning an error propagate it; the two call sites that cannot (break; in a switch, mid-function) drop the return - the socket stays in a valid state and a later event re-arms the RTO. Fails pre-fix: with a full timer heap the old code set ctrl_rto_active=1 and tmr_rto=NO_TIMER; the new code leaves both at 0. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_tcp_flow.c | 31 +++++++++++++++++++++++++++++ src/wolfip.c | 13 +++++++++--- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 6a8dcf8d..8f2affa7 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1677,6 +1677,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_wolfip_packetsocket_from_fd_negative_fd); #endif /* WOLFIP_PACKET_SOCKETS */ tcase_add_test(tc_core, test_bind_port_in_use_different_ips_no_collision); + tcase_add_test(tc_core, test_tcp_ctrl_rto_start_no_timer_does_not_set_active); #if WOLFIP_VLAN /* --- unit_tests_vlan.c (30 tests for 802.1Q support) --- */ diff --git a/src/test/unit/unit_tests_tcp_flow.c b/src/test/unit/unit_tests_tcp_flow.c index f42e3008..7942e32e 100644 --- a/src/test/unit/unit_tests_tcp_flow.c +++ b/src/test/unit/unit_tests_tcp_flow.c @@ -5888,3 +5888,34 @@ START_TEST(test_tcp_fin_in_close_wait_does_not_advance_ack) ck_assert_uint_eq(lsn->sock.tcp.ack, rcv_nxt + 1); } END_TEST + +/* When the timer heap is full, tcp_ctrl_rto_start must not mark the + * control RTO active: an active flag with no timer behind it would never + * fire and would suppress every other timeout. */ +START_TEST(test_tcp_ctrl_rto_start_no_timer_does_not_set_active) +{ + struct wolfIP s; + struct tsocket *ts; + struct wolfIP_timer t = {0}; + int i; + + wolfIP_init(&s); + t.cb = tcp_rto_cb; + for (i = 0; i < MAX_TIMERS; i++) { + t.arg = (void *)(intptr_t)i; + t.expires = s.last_tick + 1000 + i; + timers_binheap_insert(&s.timers, t); + } + ts = &s.tcpsockets[0]; + ts->S = &s; + ts->proto = WI_IPPROTO_TCP; + ts->sock.tcp.rto = 1000; + ts->sock.tcp.ctrl_rto_retries = 0; + ts->sock.tcp.tmr_rto = NO_TIMER; + ts->sock.tcp.ctrl_rto_active = 0; + tcp_ctrl_rto_start(ts, s.last_tick); + /* Heap full: insert failed, so the control RTO must not be active. */ + ck_assert_int_eq(ts->sock.tcp.tmr_rto, NO_TIMER); + ck_assert_int_eq(ts->sock.tcp.ctrl_rto_active, 0); +} +END_TEST diff --git a/src/wolfip.c b/src/wolfip.c index 992f1e47..546b2f8a 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -1348,7 +1348,7 @@ static void tcp_persist_start(struct tsocket *t, uint64_t now); static void tcp_persist_stop(struct tsocket *t); static void tcp_rto_update_from_sample(struct tsocket *t, uint32_t sample_ms); static void tcp_rto_cb(void *arg); -static void tcp_ctrl_rto_start(struct tsocket *t, uint64_t now); +static int tcp_ctrl_rto_start(struct tsocket *t, uint64_t now); static void tcp_ctrl_rto_stop(struct tsocket *t); static void tcp_fin_wait_2_timeout_start(struct tsocket *t, uint64_t now); static void tcp_fin_wait_2_timeout_stop(struct tsocket *t); @@ -4006,12 +4006,12 @@ static uint32_t tcp_backoff_rto_ms(uint32_t rto_ms, uint32_t retries) /* Arm/re-arm control-RTO timer using exponential backoff over the current base RTO. * This path is dedicated to SYN/SYN-ACK/FIN reliability (not data-loss recovery). */ -static void tcp_ctrl_rto_start(struct tsocket *t, uint64_t now) +static int tcp_ctrl_rto_start(struct tsocket *t, uint64_t now) { struct wolfIP_timer tmr = {0}; uint64_t shift_rto; if (!t || t->proto != WI_IPPROTO_TCP) - return; + return 0; /* The control RTO takes over the shared timer slot: tcp_rto_cb * dispatches on the timeout flags from that same slot, so every flag * it replaces must be cleared with the timer it armed. */ @@ -4026,7 +4026,14 @@ static void tcp_ctrl_rto_start(struct tsocket *t, uint64_t now) tmr.arg = t; tmr.cb = tcp_rto_cb; t->sock.tcp.tmr_rto = timers_binheap_insert(&t->S->timers, tmr); + /* Only mark the control RTO active when the timer actually took a slot: + * an active flag with no timer behind it would never fire and would + * suppress every other timeout until a later event cleared it. */ + if (t->sock.tcp.tmr_rto == NO_TIMER) { + return -1; + } t->sock.tcp.ctrl_rto_active = 1; + return 0; } static void tcp_fin_wait_2_timeout_start(struct tsocket *t, uint64_t now) From 2437face2d372592083a3f14960cbb81f1e4a0fa Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 3 Sep 2026 10:21:45 +0200 Subject: [PATCH 15/17] tcp: clear the control-RTO active flag before the re-arm insert tcp_ctrl_rto_start() only set ctrl_rto_active on a successful insert, but never cleared it on entry. The retransmit path re-arms the control RTO on every timeout while the flag is still set by the prior arm, so a failed re-insert into a full heap left the flag set with no timer behind it: the exact active-without-timer wedge the F-11445 fix prevents for a first arm. Clear ctrl_rto_active up front, next to the other slot flags the control RTO takes over, and keep setting it only when the timer actually took a slot. A failed re-arm then just retries on the next event, as the state is still SYN_SENT/FIN_WAIT_1/LAST_ACK. test_tcp_ctrl_rto_start_rearm_failure_clears_active fills the timer heap, arms from the post-fire state (flag set, slot empty), and asserts the flag is cleared when the re-insert fails. Fails pre-fix (ctrl_rto_active stays 1). --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_tcp_flow.c | 34 +++++++++++++++++++++++++++++ src/wolfip.c | 3 +++ 3 files changed, 38 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 8f2affa7..6db7a781 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1678,6 +1678,7 @@ Suite *wolf_suite(void) #endif /* WOLFIP_PACKET_SOCKETS */ tcase_add_test(tc_core, test_bind_port_in_use_different_ips_no_collision); tcase_add_test(tc_core, test_tcp_ctrl_rto_start_no_timer_does_not_set_active); + tcase_add_test(tc_core, test_tcp_ctrl_rto_start_rearm_failure_clears_active); #if WOLFIP_VLAN /* --- unit_tests_vlan.c (30 tests for 802.1Q support) --- */ diff --git a/src/test/unit/unit_tests_tcp_flow.c b/src/test/unit/unit_tests_tcp_flow.c index 7942e32e..1a45c03e 100644 --- a/src/test/unit/unit_tests_tcp_flow.c +++ b/src/test/unit/unit_tests_tcp_flow.c @@ -5919,3 +5919,37 @@ START_TEST(test_tcp_ctrl_rto_start_no_timer_does_not_set_active) ck_assert_int_eq(ts->sock.tcp.ctrl_rto_active, 0); } END_TEST + +/* A re-arm (the normal retransmit path) enters with ctrl_rto_active set by + * the prior arm: if the re-insert into a full heap fails, the flag must not + * survive - an active flag with no timer behind it wedges the socket the + * same way a failed first arm does. */ +START_TEST(test_tcp_ctrl_rto_start_rearm_failure_clears_active) +{ + struct wolfIP s; + struct tsocket *ts; + struct wolfIP_timer t = {0}; + int i; + + wolfIP_init(&s); + t.cb = tcp_rto_cb; + for (i = 0; i < MAX_TIMERS; i++) { + t.arg = (void *)(intptr_t)i; + t.expires = s.last_tick + 1000 + i; + timers_binheap_insert(&s.timers, t); + } + ts = &s.tcpsockets[0]; + ts->S = &s; + ts->proto = WI_IPPROTO_TCP; + ts->sock.tcp.rto = 1000; + ts->sock.tcp.ctrl_rto_retries = 0; + ts->sock.tcp.tmr_rto = NO_TIMER; + /* State at the start of a retransmit: the prior arm succeeded and its + * timer has since fired, so the flag is set and the slot is empty. */ + ts->sock.tcp.ctrl_rto_active = 1; + tcp_ctrl_rto_start(ts, s.last_tick); + /* Heap full: the re-insert failed, so the flag must be cleared. */ + ck_assert_int_eq(ts->sock.tcp.tmr_rto, NO_TIMER); + ck_assert_int_eq(ts->sock.tcp.ctrl_rto_active, 0); +} +END_TEST diff --git a/src/wolfip.c b/src/wolfip.c index 546b2f8a..606a3990 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -4017,6 +4017,9 @@ static int tcp_ctrl_rto_start(struct tsocket *t, uint64_t now) * it replaces must be cleared with the timer it armed. */ t->sock.tcp.fin_wait_2_timeout_active = 0; t->sock.tcp.preaccept_timeout_active = 0; + /* Re-arms enter with the flag set by the prior arm: clear it up front + * so a failed insert cannot leave it set with no timer behind it. */ + t->sock.tcp.ctrl_rto_active = 0; if (t->sock.tcp.tmr_rto != NO_TIMER) { timer_binheap_cancel(&t->S->timers, t->sock.tcp.tmr_rto); t->sock.tcp.tmr_rto = NO_TIMER; From 08e1b8a2392412b8e47fab5207f91b03f249670d Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 3 Sep 2026 10:22:05 +0200 Subject: [PATCH 16/17] tcp: scan the full port range in the source-port allocator port_alloc_random() stopped walking after 16 collisions and returned the last collided value, breaking its "no other socket claims it" contract: with a long run of consecutive bound ports (possible once a port has 17+ sockets, e.g. the stm32h563 config) the allocator handed out an in-use port, and the new socket would demux traffic meant for the existing one. Walk the whole min_port..65535 range exactly once and return 0 when it holds no free port. The start normalization is made total at the same time (port % range instead of port += min_port, which wraps for min_port > 32768); for the existing min_port values (1, 1024) the start is bit-identical to before. The three call sites now treat 0 as allocation failure: TCP connect unwinds to TCP_CLOSED and returns -WOLFIP_EAGAIN (the same code as a failed SYN send, and before the < 1024 bump that would have turned a 0 into a colliding 1024), UDP sendto and the ICMP echo request return -WOLFIP_EAGAIN instead of emitting a zero source port / ICMP id. test_port_alloc_walks_past_long_collision_run pins the RNG to the start of a 17-port run and asserts the allocator lands on the first free port past it (the old loop returned an in-use port). test_port_alloc_returns_zero_when_range_exhausted claims the only candidate and asserts a 0 return. Both fail pre-fix. --- src/test/unit/unit.c | 2 ++ src/test/unit/unit_tests_api.c | 41 ++++++++++++++++++++++++++++++++++ src/wolfip.c | 38 ++++++++++++++++++++++--------- 3 files changed, 71 insertions(+), 10 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 6db7a781..71b62b6f 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -212,6 +212,8 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_sock_bind_tcp_port_collision_rejected); tcase_add_test(tc_utils, test_sock_bind_udp_src_port_nonzero); tcase_add_test(tc_utils, test_udp_auto_port_skips_in_use); + tcase_add_test(tc_utils, test_port_alloc_walks_past_long_collision_run); + tcase_add_test(tc_utils, test_port_alloc_returns_zero_when_range_exhausted); tcase_add_test(tc_utils, test_sock_bind_udp_filter_blocks); tcase_add_test(tc_utils, test_sock_bind_icmp_success); tcase_add_test(tc_utils, test_sock_connect_wrong_family); diff --git a/src/test/unit/unit_tests_api.c b/src/test/unit/unit_tests_api.c index 315670a7..8f6a894f 100644 --- a/src/test/unit/unit_tests_api.c +++ b/src/test/unit/unit_tests_api.c @@ -1555,6 +1555,47 @@ START_TEST(test_udp_auto_port_skips_in_use) } END_TEST +/* The allocator walks the whole min_port..65535 range: a collision run + * longer than the old 16-try limit must be skipped, not returned. With the + * RNG pinned to the run start, the old loop stopped after 16 tries and + * returned an in-use port. */ +START_TEST(test_port_alloc_walks_past_long_collision_run) +{ + static struct tsocket arr[18]; + uint16_t port; + int i; + + memset(arr, 0, sizeof(arr)); + /* 17 consecutive ports in use, starting at the pinned RNG start. */ + for (i = 0; i < 17; i++) + arr[i].src_port = (uint16_t)(1024 + i); + test_rand_override_enabled = 1; + test_rand_override_value = 1024U; + port = port_alloc_random(arr, 18, &arr[17], IPADDR_ANY, 1024); + test_rand_override_enabled = 0; + ck_assert_uint_eq(port, 1041U); + ck_assert_int_eq(bind_port_in_use(arr, 18, &arr[17], IPADDR_ANY, port), 0); +} +END_TEST + +/* When the candidate range holds no free port the allocator returns 0 + * instead of a collided value: callers treat 0 as allocation failure. */ +START_TEST(test_port_alloc_returns_zero_when_range_exhausted) +{ + static struct tsocket arr[2]; + uint16_t port; + + memset(arr, 0, sizeof(arr)); + /* The only candidate (min_port == 65535) is already claimed. */ + arr[0].src_port = 65535; + test_rand_override_enabled = 1; + test_rand_override_value = 65535U; + port = port_alloc_random(arr, 2, &arr[1], IPADDR_ANY, 65535); + test_rand_override_enabled = 0; + ck_assert_uint_eq(port, 0U); +} +END_TEST + START_TEST(test_sock_bind_udp_filter_blocks) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 606a3990..a136f344 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -6426,25 +6426,32 @@ static int bind_port_in_use(const struct tsocket *arr, int n, /* Pick a source port (or ICMP id) that no other socket in arr claims. * Start from a random value >= min_port, then walk forward on a - * collision (wrapping to min_port). local_ip may be IPADDR_ANY when the - * route is not resolved yet; the check then compares ports only. */ + * collision (wrapping to min_port) until every candidate in + * min_port..65535 has been tried. local_ip may be IPADDR_ANY when the + * route is not resolved yet; the check then compares ports only. + * Returns 0 when the range holds no free port: a collided value would + * break the "no other socket claims it" contract. */ static uint16_t port_alloc_random(const struct tsocket *arr, int n, const struct tsocket *self, ip4 local_ip, uint16_t min_port) { uint16_t port; - uint16_t tries; + uint16_t scanned; + uint16_t range; + range = (uint16_t)(0x10000 - min_port); port = (uint16_t)(wolfIP_getrandom() & 0xFFFF); if (port < min_port) - port += min_port; - for (tries = 0; tries < 16; tries++) { + port = (uint16_t)(min_port + (port % range)); + scanned = 0; + do { if (!bind_port_in_use(arr, n, self, local_ip, port)) return port; port++; if (port < min_port) port = min_port; - } - return port; + scanned++; + } while (scanned < range); + return 0; } int wolfIP_sock_connect(struct wolfIP *s, int sockfd, const struct wolfIP_sockaddr *addr, @@ -6618,9 +6625,14 @@ int wolfIP_sock_connect(struct wolfIP *s, int sockfd, const struct wolfIP_sockad ts->remote_ip = new_remote_ip; ts->if_idx = new_if_idx; ts->local_ip = new_local_ip; - if (!ts->src_port) + if (!ts->src_port) { ts->src_port = port_alloc_random(s->tcpsockets, MAX_TCPSOCKETS, ts, ts->local_ip, 1024); + if (ts->src_port == 0) { + ts->sock.tcp.state = TCP_CLOSED; + return -WOLFIP_EAGAIN; + } + } if (ts->src_port < 1024) ts->src_port += 1024; ts->dst_port = ee16(sin->sin_port); @@ -6879,9 +6891,12 @@ int wolfIP_sock_sendto(struct wolfIP *s, int sockfd, const void *buf, size_t len } if ((ts->dst_port==0) || (ts->remote_ip==0)) return -1; - if (ts->src_port == 0) + if (ts->src_port == 0) { ts->src_port = port_alloc_random(s->udpsockets, MAX_UDPSOCKETS, ts, IPADDR_ANY, 1024); + if (ts->src_port == 0) + return -WOLFIP_EAGAIN; + } if_idx = wolfIP_route_for_ip(s, ts->remote_ip); #ifdef IP_MULTICAST if (wolfIP_ip_is_multicast(ts->remote_ip) && ts->sock.udp.mcast_if_set) @@ -6937,9 +6952,12 @@ int wolfIP_sock_sendto(struct wolfIP *s, int sockfd, const void *buf, size_t len } if (ts->remote_ip == 0) return -1; - if (ts->src_port == 0) + if (ts->src_port == 0) { ts->src_port = port_alloc_random(s->icmpsockets, MAX_ICMPSOCKETS, ts, IPADDR_ANY, 1); + if (ts->src_port == 0) + return -WOLFIP_EAGAIN; + } if (ts->bound_local_ip != IPADDR_ANY) { int bound_match = 0; unsigned int bound_if = wolfIP_if_for_local_ip(s, ts->bound_local_ip, &bound_match); From 10d25825cc8cdfc761a5e03ff872411e988e523d Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 3 Sep 2026 10:22:22 +0200 Subject: [PATCH 17/17] dhcp: treat a GARP announcement as a DAD conflict The DAD request check flagged a foreign MAC claiming the candidate as its sender IP only when tip != candidate, which let a gratuitous ARP announcement (sip == tip == candidate) slip through: a host already using the candidate betrays itself by announcing it, the same way it does by using it in an ordinary request. Drop the tip exclusion so any request from a foreign MAC with sip == candidate is a conflict, next to the existing 0.0.0.0-probe case. A request that merely probes for the candidate from a real source IP is still not a conflict: wanting to reach an address is not owning it. test_dhcp_dad_garp_announcement_conflict injects a foreign sip==tip==candidate request on the probing interface and asserts the conflict path (lease released, back to DISCOVER). Fails pre-fix (the request was answered and DAD kept running). --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_dhcp_edges.c | 54 +++++++++++++++++++++++++++ src/wolfip.c | 8 ++-- 3 files changed, 59 insertions(+), 4 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 71b62b6f..c55060f2 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1482,6 +1482,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_dhcp_dad_conflict_releases_and_rediscover); tcase_add_test(tc_core, test_dhcp_dad_request_claiming_candidate_conflict); tcase_add_test(tc_core, test_dhcp_dad_probe_for_candidate_conflict); + tcase_add_test(tc_core, test_dhcp_dad_garp_announcement_conflict); tcase_add_test(tc_core, test_dhcp_dad_own_mac_reply_ignored); tcase_add_test(tc_core, test_dhcp_dad_reply_for_other_ip_ignored); tcase_add_test(tc_core, test_dhcp_dad_reply_on_unconfigured_secondary_ignored); diff --git a/src/test/unit/unit_tests_dhcp_edges.c b/src/test/unit/unit_tests_dhcp_edges.c index a9061cec..23623d66 100644 --- a/src/test/unit/unit_tests_dhcp_edges.c +++ b/src/test/unit/unit_tests_dhcp_edges.c @@ -1797,6 +1797,60 @@ START_TEST(test_dhcp_dad_probe_for_candidate_conflict) } END_TEST +/* During DAD, a foreign host announcing the candidate with a gratuitous + * ARP request (sip==tip==candidate) is a conflict: it is using the address, + * which is exactly what DAD must rule out. */ +START_TEST(test_dhcp_dad_garp_announcement_conflict) +{ + struct wolfIP s; + struct dhcp_msg msg; + struct arp_packet req; + struct wolfIP_ll_dev *ll; + struct ipconf *primary; + uint32_t server_ip = 0x0A000001U; + uint32_t client_ip = 0x0A000064U; + uint8_t other_mac[6] = {0x02, 0xAA, 0xBB, 0xCC, 0xDD, 0x06}; + + wolfIP_init(&s); + mock_link_init(&s); + s.dhcp_xid = 0xDA09U; + s.dhcp_state = DHCP_REQUEST_SENT; + s.last_tick = 1000U; + primary = wolfIP_primary_ipconf(&s); + ck_assert_ptr_nonnull(primary); + primary->ip = client_ip; + + build_full_ack(&s, &msg, server_ip, client_ip, 0xFFFFFF00U, + server_ip, 0x08080808U, 120U); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), 0); + ck_assert_int_eq(s.dhcp_state, DHCP_DAD); + + /* A foreign host announcing the candidate (gratuitous ARP: the request + * carries sip==tip==candidate): conflict. */ + ll = wolfIP_getdev_ex(&s, TEST_PRIMARY_IF); + ck_assert_ptr_nonnull(ll); + memset(&req, 0, sizeof(req)); + memcpy(req.eth.dst, ll->mac, 6); + memcpy(req.eth.src, other_mac, 6); + req.eth.type = ee16(ETH_TYPE_ARP); + req.htype = ee16(1); + req.ptype = ee16(0x0800); + req.hlen = 6; + req.plen = 4; + req.opcode = ee16(ARP_REQUEST); + memcpy(req.sma, other_mac, 6); + req.sip = ee32(client_ip); + req.tip = ee32(client_ip); + + arp_recv(&s, TEST_PRIMARY_IF, &req, sizeof(req)); + + /* Conflict detected: DAD aborted, lease released, back to DISCOVER. */ + ck_assert_int_eq(s.dhcp_state, DHCP_DISCOVER_SENT); + ck_assert_uint_eq(s.dhcp_dad_probes, 0U); + ck_assert_uint_eq(primary->ip, 0U); +} +END_TEST + /* A reply with our own MAC is our own probe looping back: ignored, DAD * continues. */ START_TEST(test_dhcp_dad_own_mac_reply_ignored) diff --git a/src/wolfip.c b/src/wolfip.c index a136f344..5e2b23c0 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -9880,15 +9880,15 @@ static void arp_recv(struct wolfIP *s, unsigned int if_idx, void *buf, int len) return; /* RFC 4331/5227 DAD: on the probing interface, a request from a - * foreign MAC that claims the candidate (sender IP) or probes for it - * is a conflict - a host that owns the candidate may not answer our + * foreign MAC that claims the candidate (sender IP, including a + * gratuitous announcement with sip==tip) or probes for it is a + * conflict - a host that owns the candidate may not answer our * probe (DAD evasion), but betrays itself by using/probing the IP. */ if (arp->opcode == ee16(ARP_REQUEST) && s->dhcp_state == DHCP_DAD && if_idx == s->dhcp_dad_if && memcmp(arp->sma, ll->mac, 6) != 0) { ip4 sip = ee32(arp->sip); ip4 tip = ee32(arp->tip); - if ((sip == s->dhcp_ip && tip != s->dhcp_ip) || - (sip == IPADDR_ANY && tip == s->dhcp_ip)) { + if (sip == s->dhcp_ip || (sip == IPADDR_ANY && tip == s->dhcp_ip)) { dhcp_dad_conflict(s); return; }