From 36ff9bb92de558201f5136a7de8bdbf0937036d8 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 16:45:59 +0200 Subject: [PATCH 01/20] F-11434: consume SYN-ACK after handshake completion in tcp_input The SYN_SENT handler sent the handshake ACK and fell through into the synchronized-state branch, which saw the consumed SYN-ACK as out-of-window and queued a second, redundant ACK on every active open. Continue past it; add a unit test asserting exactly one ACK is queued for the handshake. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_tcp_flow.c | 41 +++++++++++++++++++++++++++++ src/wolfip.c | 5 ++++ 3 files changed, 47 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 70b5f099..f7ad0688 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -606,6 +606,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_tcp_syn_sent_to_established); tcase_add_test(tc_utils, test_tcp_input_syn_sent_unexpected_flags); tcase_add_test(tc_utils, test_tcp_input_syn_sent_synack_transitions); + tcase_add_test(tc_utils, test_tcp_input_syn_sent_synack_queues_single_ack); tcase_add_test(tc_utils, test_tcp_input_syn_sent_synack_invalid_ack_rejected); tcase_add_test(tc_utils, test_tcp_input_syn_listen_does_not_scale_syn_window); tcase_add_test(tc_utils, test_tcp_input_syn_sent_does_not_scale_synack_window); diff --git a/src/test/unit/unit_tests_tcp_flow.c b/src/test/unit/unit_tests_tcp_flow.c index dd7e99ef..9c5f6210 100644 --- a/src/test/unit/unit_tests_tcp_flow.c +++ b/src/test/unit/unit_tests_tcp_flow.c @@ -661,6 +661,47 @@ START_TEST(test_tcp_input_syn_sent_synack_transitions) } END_TEST +START_TEST(test_tcp_input_syn_sent_synack_queues_single_ack) +{ + struct wolfIP s; + struct tsocket *ts; + struct pkt_desc *ack_desc; + struct wolfIP_tcp_seg *out; + int tcp_sd; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + wolfIP_filter_set_callback(NULL, NULL); + wolfIP_filter_set_mask(0); + wolfIP_filter_set_tcp_mask(0); + + tcp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, WI_IPPROTO_TCP); + ck_assert_int_gt(tcp_sd, 0); + ts = &s.tcpsockets[SOCKET_UNMARK(tcp_sd)]; + ts->sock.tcp.state = TCP_SYN_SENT; + ts->sock.tcp.seq = 100; + ts->src_port = 1234; + ts->dst_port = 4321; + ts->local_ip = 0x0A000001U; + ts->remote_ip = 0x0A000002U; + + inject_tcp_segment(&s, TEST_PRIMARY_IF, 0x0A000002U, 0x0A000001U, 4321, 1234, 10, 101, (TCP_FLAG_SYN | TCP_FLAG_ACK)); + ck_assert_int_eq(ts->sock.tcp.state, TCP_ESTABLISHED); + /* The consumed SYN-ACK must not fall through into the + * synchronized-state branch, which would see its sequence as + * old and queue a second, redundant ACK: exactly one ACK + * (the handshake completion) may be queued. */ + ack_desc = fifo_pop(&ts->sock.tcp.txbuf); + ck_assert_ptr_nonnull(ack_desc); + out = (struct wolfIP_tcp_seg *)(ts->txmem + ack_desc->pos + + sizeof(*ack_desc)); + ck_assert_uint_eq(out->flags & (TCP_FLAG_SYN | TCP_FLAG_ACK), TCP_FLAG_ACK); + ck_assert_uint_eq(ee32(out->ack), 11); + ck_assert_ptr_null(fifo_pop(&ts->sock.tcp.txbuf)); +} +END_TEST + START_TEST(test_tcp_input_syn_sent_synack_invalid_ack_rejected) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index d6e86a78..5c218f75 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -5942,6 +5942,11 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, t->events |= CB_EVENT_WRITABLE; tcp_process_ts(t, tcp, frame_len); tcp_send_ack(t); + /* The SYN-ACK is consumed: keep it out of the + * synchronized-state branch below, which would + * see its sequence as old and queue a redundant + * second ACK. */ + continue; } } } From 4d0c3cf2bffbd79450a4caeec6053395c64d17dd Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 17:01:16 +0200 Subject: [PATCH 02/20] F-13765: gate tcp_ack dup-ACK arm on forward progress A forward ACK can advance snd_una while the marking loop counts zero descriptors (retransmit-marked or partially covered head), so the ack_count gate miscounted it as a duplicate, triggering fast retransmit one ACK early. Reject forward ACKs in the dup arm, keeping the stale-ACK rejection; add a unit test. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_tcp_ack.c | 72 ++++++++++++++++++++++++++++++ src/wolfip.c | 7 ++- 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index f7ad0688..bf13b4cb 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -745,6 +745,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_tcp_consume_ooo_wrap_drop_fully_acked); tcase_add_test(tc_utils, test_tcp_store_ooo_overlap_does_not_exhaust_cache); tcase_add_test(tc_utils, test_tcp_ack_sack_early_retransmit_before_three_dupack); + tcase_add_test(tc_utils, test_tcp_ack_forward_ack_after_retransmit_not_duplicate); tcase_add_test(tc_utils, test_tcp_input_listen_syn_without_sack_disables_sack); tcase_add_test(tc_utils, test_tcp_input_listen_syn_arms_control_rto); tcase_add_test(tc_utils, test_tcp_input_listen_syn_sends_synack_immediately); diff --git a/src/test/unit/unit_tests_tcp_ack.c b/src/test/unit/unit_tests_tcp_ack.c index 0d8379c2..a5794d5e 100644 --- a/src/test/unit/unit_tests_tcp_ack.c +++ b/src/test/unit/unit_tests_tcp_ack.c @@ -5065,6 +5065,78 @@ START_TEST(test_tcp_ack_sack_early_retransmit_before_three_dupack) } END_TEST +/* F-13765: a forward ACK that fully acknowledges a segment whose + * PKT_FLAG_SENT was cleared by the retransmit marker advances + * snd_una, but the marking loop counts zero descriptors, so the + * ack_count gate sends it into the duplicate-ACK arm. RFC 5681 s2: + * a duplicate ACK must not advance SND.UNA, so dup_acks must stay 0. */ +START_TEST(test_tcp_ack_forward_ack_after_retransmit_not_duplicate) +{ + struct wolfIP s; + struct tsocket *ts; + struct tcp_seg_buf segbuf1; + struct tcp_seg_buf segbuf2; + struct wolfIP_tcp_seg *seg1; + struct wolfIP_tcp_seg *seg2; + uint8_t ackbuf[sizeof(struct wolfIP_tcp_seg)]; + struct wolfIP_tcp_seg *ackseg = (struct wolfIP_tcp_seg *)ackbuf; + struct pkt_desc *desc; + + wolfIP_init(&s); + 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.seq = 102; + ts->sock.tcp.snd_una = 100; + ts->sock.tcp.bytes_in_flight = 2; + ts->sock.tcp.cwnd = TCP_MSS * 4; + ts->sock.tcp.peer_rwnd = TCP_MSS * 4; + fifo_init(&ts->sock.tcp.txbuf, ts->txmem, TXBUF_SIZE); + + memset(&segbuf1, 0, sizeof(segbuf1)); + seg1 = &segbuf1.seg; + seg1->ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN + 1); + seg1->hlen = TCP_HEADER_LEN << 2; + seg1->seq = ee32(100); + ck_assert_int_eq(fifo_push(&ts->sock.tcp.txbuf, &segbuf1, sizeof(segbuf1)), 0); + desc = fifo_peek(&ts->sock.tcp.txbuf); + ck_assert_ptr_nonnull(desc); + desc->flags |= PKT_FLAG_SENT; + + memset(&segbuf2, 0, sizeof(segbuf2)); + seg2 = &segbuf2.seg; + seg2->ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN + 1); + seg2->hlen = TCP_HEADER_LEN << 2; + seg2->seq = ee32(101); + ck_assert_int_eq(fifo_push(&ts->sock.tcp.txbuf, &segbuf2, sizeof(segbuf2)), 0); + desc = fifo_next(&ts->sock.tcp.txbuf, desc); + ck_assert_ptr_nonnull(desc); + desc->flags |= PKT_FLAG_SENT; + + /* Drive the head segment into the retransmit state the marker + * leaves it in: PKT_FLAG_SENT cleared, PKT_FLAG_RETRANS set. */ + ck_assert_int_eq(tcp_mark_unsacked_for_retransmit(ts, 100), 1); + desc = fifo_peek(&ts->sock.tcp.txbuf); + ck_assert_ptr_nonnull(desc); + ck_assert_int_eq(desc->flags & PKT_FLAG_SENT, 0); + ck_assert_int_ne(desc->flags & PKT_FLAG_RETRANS, 0); + + /* Pure ACK (no data, unchanged window) that fully acknowledges + * the retransmitted head segment: snd_una moves 100 -> 101. */ + memset(ackbuf, 0, sizeof(ackbuf)); + ackseg->ack = ee32(101); + ackseg->hlen = TCP_HEADER_LEN << 2; + ackseg->flags = TCP_FLAG_ACK; + + tcp_ack(ts, ackseg); + ck_assert_uint_eq(ts->sock.tcp.snd_una, 101); + /* The ACK advanced snd_una, so it is not a duplicate. */ + ck_assert_uint_eq(ts->sock.tcp.dup_acks, 0); +} +END_TEST + START_TEST(test_tcp_input_listen_syn_without_sack_disables_sack) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 5c218f75..d6218b8e 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -5566,7 +5566,12 @@ static void tcp_ack(struct tsocket *t, const struct wolfIP_tcp_seg *tcp) * trigger fast retransmit. */ uint32_t ip_len = ee16(tcp->ip.len); uint32_t hdr_len = IP_HEADER_LEN + tcp_data_offset_bytes(tcp->hlen); - if (ack != t->sock.tcp.snd_una) + /* RFC 5681 s2: a duplicate ACK equals the greatest ACK + * received. A forward ACK is not a duplicate even when the + * marking loop counted zero descriptors (retransmit-marked or + * partially covered head descriptor); a stale one is not. + * Both must stay out of the counter. */ + if (ack_advanced || ack != t->sock.tcp.snd_una) return; if (inflight_pre == 0) return; From ff18e66a8ff4b9b803ebfbcd879e3f3f665f67f1 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 17:04:39 +0200 Subject: [PATCH 03/20] F-13767: remove dead tcpsocket field last_ack The field was written on two transmit paths (tcp_send_empty_immediate, flush_tcp_tx) and never read anywhere; one unit test asserted its refresh. Remove the field, both assignments, and the test references. No behaviour change. --- src/test/unit/unit_tests_dns_dhcp.c | 2 -- src/test/unit/unit_tests_proto.c | 3 --- src/wolfip.c | 5 +---- 3 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index 52e0edc5..e630787a 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -4266,7 +4266,6 @@ START_TEST(test_poll_tcp_ack_only_skips_send) ts->src_port = 1111; ts->dst_port = 2222; ts->sock.tcp.ack = 10; - ts->sock.tcp.last_ack = 10; ts->sock.tcp.rto = 100; /* Ensure send window allows processing of the queued ACK-only segment. */ ts->sock.tcp.cwnd = TCP_MSS; @@ -4314,7 +4313,6 @@ START_TEST(test_poll_tcp_send_on_arp_hit) ts->src_port = 1111; ts->dst_port = 2222; ts->sock.tcp.ack = 20; - ts->sock.tcp.last_ack = 0; ts->sock.tcp.rto = 100; /* Ensure send window allows emitting the queued data segment. */ ts->sock.tcp.cwnd = TCP_MSS * 4; diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index 024ea2cf..7f758f18 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -1410,7 +1410,6 @@ START_TEST(test_poll_tcp_residual_window_gates_data_segment) ts->src_port = 1111; ts->dst_port = 2222; ts->sock.tcp.ack = 20; - ts->sock.tcp.last_ack = 0; ts->sock.tcp.rto = 100; ts->sock.tcp.cwnd = 32; ts->sock.tcp.peer_rwnd = 20; @@ -1467,7 +1466,6 @@ START_TEST(test_poll_tcp_residual_window_allows_exact_fit) ts->src_port = 1111; ts->dst_port = 2222; ts->sock.tcp.ack = 20; - ts->sock.tcp.last_ack = 0; ts->sock.tcp.rto = 100; ts->sock.tcp.cwnd = 32; ts->sock.tcp.peer_rwnd = 20; @@ -6066,7 +6064,6 @@ START_TEST(test_regression_loopback_pure_ack_uses_deferred_buffer_until_poll) ck_assert_int_eq(tcp_send_empty_immediate(ts, &seg, (uint32_t)sizeof(seg)), 0); - ck_assert_uint_eq(ts->sock.tcp.last_ack, ts->sock.tcp.ack); ck_assert_uint_eq(last_frame_sent_size, 0U); ck_assert_uint_eq(s.loopback_count, 1U); ck_assert_uint_eq(s.loopback_pending_len[s.loopback_head], diff --git a/src/wolfip.c b/src/wolfip.c index d6218b8e..cea8d2be 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -1213,7 +1213,7 @@ enum tcp_state { struct tcpsocket { enum tcp_state state; uint32_t last_ts, rtt, rto, cwnd, cwnd_count, ssthresh, tmr_rto, rto_backoff, - tmr_persist, seq, ack, last_ack, last, bytes_in_flight, snd_una, + tmr_persist, seq, ack, last, bytes_in_flight, snd_una, recovery_point; uint32_t srtt, rttvar; uint32_t last_early_rexmit_ack; @@ -3773,7 +3773,6 @@ static int tcp_send_empty_immediate(struct tsocket *t, struct wolfIP_tcp_seg *tc } #endif - t->sock.tcp.last_ack = t->sock.tcp.ack; tcp->ack = ee32(t->sock.tcp.ack); tcp->win = ee16(tcp_adv_win(t, 1)); ip_output_add_header(t, (struct wolfIP_ip_packet *)tcp, WI_IPPROTO_TCP, @@ -11978,8 +11977,6 @@ static void flush_tcp_tx(struct wolfIP *s, uint64_t now) (in_flight < snd_wnd && seg_payload_len <= (snd_wnd - in_flight))) { struct wolfIP_timer new_tmr = {}; size = seg_ip_len; - /* Refresh ack counter */ - ts->sock.tcp.last_ack = ts->sock.tcp.ack; tcp->ack = ee32(ts->sock.tcp.ack); tcp->win = ee16(tcp_adv_win(ts, 1)); ip_output_add_header(ts, (struct wolfIP_ip_packet *)tcp, WI_IPPROTO_TCP, From a3e502f8151625055d06be8640cee5f7b56d8a6e Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 17:38:21 +0200 Subject: [PATCH 04/20] F-13206: bound DHCP lease timer deadlines to the heap horizon The heap compares deadlines in the 32-bit tick domain, so a renewal past INT32_MAX ticks (lease > 24.8 days) reads as already due and the BOUND callback drops it, leaving the lease timers disarmed and the lease to expire silently. Long deadlines are now driven by bounded checkpoints (now + INT32_MAX, re-armed by the callback until representable); RENEWING/REBINDING retry schedules are bounded the same way. Also lands F-12388: dhcp_lease_ip_sane rejects host-bit-set and network-broadcast addresses. Verification: make build/wolfip.o && make unit -> 1559/1559. Pre-fix, test_dhcp_long_lease_renewal_checkpoint_rearm fails on s.dhcp_timer == 0 after DAD (fix stashed); with the fix the checkpoint re-arms through T1 and the renewal starts exactly at T1. --- src/test/unit/unit.c | 2 + src/test/unit/unit_tests_dns_dhcp.c | 121 +++++++++++++++++++++++++++- src/wolfip.c | 51 +++++++++--- 3 files changed, 161 insertions(+), 13 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index bf13b4cb..7acb3a7e 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -992,6 +992,8 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_dns_query_and_callback_a); tcase_add_test(tc_proto, test_dhcp_option_u32_macros_round_trip_wire_order); tcase_add_test(tc_proto, test_dhcp_parse_offer_and_ack); + tcase_add_test(tc_proto, test_dhcp_lease_ip_sane_rejects_bad_mask_and_network_addr); + tcase_add_test(tc_proto, test_dhcp_long_lease_renewal_checkpoint_rearm); tcase_add_test(tc_proto, test_dhcp_schedule_lease_timer_defaults_t1_t2); tcase_add_test(tc_proto, test_dhcp_schedule_lease_timer_small_lease_clamps_t1_t2); tcase_add_test(tc_proto, test_dhcp_parse_offer_defaults_mask_when_missing); diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index e630787a..9eb9d22d 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -233,6 +233,125 @@ START_TEST(test_dhcp_parse_offer_and_ack) } END_TEST +START_TEST(test_dhcp_lease_ip_sane_rejects_bad_mask_and_network_addr) +{ + /* Usable host addresses must be accepted. */ + ck_assert_int_eq(dhcp_lease_ip_sane(0xC0A80105U, 0xFFFFFF00U), 1); + /* A non-contiguous mask must be rejected: the old + * (ip | mask) == 0xFFFFFFFF check could not see it, and a + * malformed mask must not be installed on the interface. */ + ck_assert_int_eq(dhcp_lease_ip_sane(0xC0A80105U, 0xFF00FF00U), 0); + /* A network address (all-zero host portion) is not a usable + * host on a conventional prefix. */ + ck_assert_int_eq(dhcp_lease_ip_sane(0xC0A80100U, 0xFFFFFF00U), 0); + /* The subnet broadcast stays rejected. */ + ck_assert_int_eq(dhcp_lease_ip_sane(0xC0A801FFU, 0xFFFFFF00U), 0); + /* /31 (RFC 3021 point-to-point): both addresses are usable + * host addresses; the old broadcast check rejected the second. */ + ck_assert_int_eq(dhcp_lease_ip_sane(0x0A000000U, 0xFFFFFFFEU), 1); + ck_assert_int_eq(dhcp_lease_ip_sane(0x0A000001U, 0xFFFFFFFEU), 1); + /* /32: the address is the host; the old check rejected every + * /32 lease. */ + ck_assert_int_eq(dhcp_lease_ip_sane(0x0A000007U, 0xFFFFFFFFU), 1); + /* The existing rejections stay: zero, limited broadcast, + * multicast. */ + ck_assert_int_eq(dhcp_lease_ip_sane(0U, 0xFFFFFF00U), 0); + ck_assert_int_eq(dhcp_lease_ip_sane(0xFFFFFFFFU, 0xFFFFFF00U), 0); + ck_assert_int_eq(dhcp_lease_ip_sane(0xE0000001U, 0xFFFFFF00U), 0); +} +END_TEST + +START_TEST(test_dhcp_long_lease_renewal_checkpoint_rearm) +{ + struct wolfIP s; + struct dhcp_msg msg; + struct dhcp_option *opt; + uint32_t server_ip = 0x0A000001U; + uint32_t offer_ip = 0xC0A80164U; + uint32_t mask = 0xFFFFFF00U; + uint32_t lease_s = 5184000U; /* 60 days: default T1 (50%) exceeds the + * timer heap's 2^31 tick horizon */ + + wolfIP_init(&s); + mock_link_init(&s); + s.last_tick = 1000U; + s.dhcp_xid = 0x1234U; + + memset(&msg, 0, sizeof(msg)); + msg.op = BOOT_REPLY; + msg.magic = ee32(DHCP_MAGIC); + msg.xid = ee32(s.dhcp_xid); + msg.yiaddr = ee32(offer_ip); + opt = (struct dhcp_option *)msg.options; + opt->code = DHCP_OPTION_MSG_TYPE; + opt->len = 1; + opt->data[0] = DHCP_OFFER; + opt = (struct dhcp_option *)((uint8_t *)opt + 3); + opt->code = DHCP_OPTION_SERVER_ID; + opt->len = 4; + DHCP_OPT_u32_to_data(opt, server_ip); + opt = (struct dhcp_option *)((uint8_t *)opt + 6); + opt->code = DHCP_OPTION_SUBNET_MASK; + opt->len = 4; + DHCP_OPT_u32_to_data(opt, mask); + opt = (struct dhcp_option *)((uint8_t *)opt + 6); + opt->code = DHCP_OPTION_END; + opt->len = 0; + ck_assert_int_eq(dhcp_parse_offer(&s, &msg, sizeof(msg)), 0); + + /* ACK with the long lease and no renew/rebind options: the client + * defaults give T1 = 50% of the lease, beyond the 32-bit horizon. */ + memset(&msg, 0, sizeof(msg)); + msg.op = BOOT_REPLY; + msg.magic = ee32(DHCP_MAGIC); + msg.xid = ee32(s.dhcp_xid); + msg.yiaddr = ee32(offer_ip); + 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, server_ip); + opt = (struct dhcp_option *)((uint8_t *)opt + 6); + opt->code = DHCP_OPTION_SUBNET_MASK; + opt->len = 4; + DHCP_OPT_u32_to_data(opt, mask); + opt = (struct dhcp_option *)((uint8_t *)opt + 6); + opt->code = DHCP_OPTION_LEASE_TIME; + opt->len = 4; + DHCP_OPT_u32_to_data(opt, lease_s); + opt = (struct dhcp_option *)((uint8_t *)opt + 6); + opt->code = DHCP_OPTION_END; + opt->len = 0; + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), 0); + + /* Bug precondition: T1 sits beyond the 32-bit tick horizon. */ + ck_assert_uint_gt(s.dhcp_renew_at - s.last_tick, (uint64_t)INT32_MAX); + + dhcp_test_complete_dad(&s); + ck_assert_int_eq(s.dhcp_state, DHCP_BOUND); + /* The heap compares deadlines in the 32-bit tick domain, so the long + * T1 reads as already due and fires during the final DAD poll. The + * callback must re-arm a bounded checkpoint instead of leaving the + * lease timers disarmed. */ + ck_assert_int_ne(s.dhcp_timer, NO_TIMER); + ck_assert_uint_eq(find_timer_expiry(&s, s.dhcp_timer), + s.last_tick + (uint64_t)INT32_MAX); + + /* The checkpoint re-arms itself until T1 becomes representable, then + * the renewal starts exactly at T1. */ + s.last_tick = find_timer_expiry(&s, s.dhcp_timer); + handle_timers(&s, s.last_tick); + ck_assert_int_eq(s.dhcp_state, DHCP_BOUND); + ck_assert_uint_eq(find_timer_expiry(&s, s.dhcp_timer), s.dhcp_renew_at); + s.last_tick = s.dhcp_renew_at; + handle_timers(&s, s.last_tick); + ck_assert_int_eq(s.dhcp_state, DHCP_RENEWING); +} +END_TEST + START_TEST(test_dhcp_schedule_lease_timer_defaults_t1_t2) { struct wolfIP s; @@ -6440,7 +6559,7 @@ END_TEST * any source, even after sendto() set the socket's dst_port to * something specific. Prior wolfIP behaviour conflated "destination * of last send" with "RX filter" and rejected the reply with ICMP - * port-unreachable when the peer answered from a different port — + * port-unreachable when the peer answered from a different port - * which is exactly what RFC 1350 TFTP does on the first DATA/OACK. */ START_TEST(test_udp_try_recv_unconnected_accepts_any_peer_port) { diff --git a/src/wolfip.c b/src/wolfip.c index cea8d2be..463380cd 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -8859,6 +8859,21 @@ static void dhcp_schedule_retry_timer(struct wolfIP *s, uint64_t deadline) dhcp_schedule_timer_at(s, next); } +/* The timer heap compares deadlines in the tick source's 32-bit domain + * (see tick_expired), so a deadline more than INT32_MAX ticks ahead is + * misread as already due. Long DHCP deadlines stay full-width in the + * lease absolutes and are driven by bounded checkpoints: return the + * deadline when it is representable, else the farthest representable + * point; the timer callback re-arms the next checkpoint until the + * deadline itself is representable. */ +static uint64_t dhcp_bounded_at(const struct wolfIP *s, uint64_t deadline) +{ + if (deadline > s->last_tick && + deadline - s->last_tick > (uint64_t)INT32_MAX) + return s->last_tick + (uint64_t)INT32_MAX; + return deadline; +} + /* 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 @@ -8882,8 +8897,8 @@ static void dhcp_schedule_renew_rebind_retry(struct wolfIP *s, uint64_t deadline 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)); + dhcp_schedule_timer_at(s, dhcp_bounded_at(s, + s->last_tick + dhcp_renew_rebind_delay_ms(remaining))); } static uint16_t dhcp_elapsed_secs(const struct wolfIP *s) @@ -8985,12 +9000,14 @@ static void dhcp_timer_cb(void *arg) break; } if (s->dhcp_renew_at != 0 && s->last_tick < s->dhcp_renew_at) { - /* A stale timer from an earlier lease cycle fired early - * (e.g. a renewal timer left pending across a lease drop - * and re-DORA). The current lease's renew time is still - * ahead, so its timer is the one that should drive the - * renewal; stay BOUND instead of starting a spurious - * RENEWING transaction. */ + /* A timer fired before the renewal deadline: either a + * bounded checkpoint for a deadline beyond the heap's + * 32-bit horizon, or a stale timer from an earlier lease + * cycle. Re-arm the nearest representable checkpoint so + * the renewal still reaches its deadline instead of + * leaving the lease timers disarmed. */ + dhcp_schedule_timer_at(s, + dhcp_bounded_at(s, s->dhcp_renew_at)); break; } s->dhcp_state = DHCP_RENEWING; @@ -9242,17 +9259,27 @@ static int dhcp_opt_stream_next(struct dhcp_opt_stream *st, uint8_t *code, } } -/* A lease address must be a usable unicast host address: not 0.0.0.0, - * not the limited broadcast, not multicast, and not the broadcast of - * its own subnet. */ +/* A lease address must be a usable unicast host address: not + * 0.0.0.0, not the limited broadcast, not multicast, on a + * contiguous mask, and not the network or broadcast address of + * its own conventional subnet. /31 (RFC 3021) and /32 leases use + * every address as a host address. */ static int dhcp_lease_ip_sane(uint32_t ip, uint32_t mask) { + uint8_t prefix_len; + if (ip == 0U || ip == 0xFFFFFFFFU) return 0; if (wolfIP_ip_is_multicast(ip)) return 0; - if (mask != 0U && ((ip | mask) == 0xFFFFFFFFU)) + if (wolfIP_mask_prefix_len(mask, &prefix_len) < 0) return 0; + if (prefix_len < 31U) { + if ((ip & ~mask) == 0U) + return 0; + if ((ip & ~mask) == ~mask) + return 0; + } return 1; } From 60fe11226ecf3e1d35a57765f697c7d7fa78bf53 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 17:52:25 +0200 Subject: [PATCH 05/20] F-13764: collect the DHCP server ID like the other offer fields dhcp_parse_offer wrote s->dhcp_server_ip inside the option loop, before the end-of-stream validation, so a rejected OFFER (bad msg type, missing END, unsane lease IP) committed the attacker's server identifier into stack state. No reader is reachable from DISCOVER_SENT today - every consumer (ACK cross-check, REQUEST, DECLINE) only runs after a successful parse re-wrote the field - but the write breaks the function's own collect-then-validate-then-commit invariant. Collect into a local and commit with the rest. Verification: make build/wolfip.o && touch src/test/unit/unit.c && sleep 1 && make unit && ./build/test/unit -> 1560/1560. Pre-fix, test_dhcp_parse_offer_reject_does_not_commit_server_id fails with s.dhcp_server_ip == 0x0A000001 (fix stashed). --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_dns_dhcp.c | 41 +++++++++++++++++++++++++++++ src/wolfip.c | 4 ++- 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 7acb3a7e..017d6adb 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -994,6 +994,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_dhcp_parse_offer_and_ack); tcase_add_test(tc_proto, test_dhcp_lease_ip_sane_rejects_bad_mask_and_network_addr); tcase_add_test(tc_proto, test_dhcp_long_lease_renewal_checkpoint_rearm); + tcase_add_test(tc_proto, test_dhcp_parse_offer_reject_does_not_commit_server_id); tcase_add_test(tc_proto, test_dhcp_schedule_lease_timer_defaults_t1_t2); tcase_add_test(tc_proto, test_dhcp_schedule_lease_timer_small_lease_clamps_t1_t2); tcase_add_test(tc_proto, test_dhcp_parse_offer_defaults_mask_when_missing); diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index 9eb9d22d..fe663a16 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -352,6 +352,47 @@ START_TEST(test_dhcp_long_lease_renewal_checkpoint_rearm) } END_TEST +START_TEST(test_dhcp_parse_offer_reject_does_not_commit_server_id) +{ + struct wolfIP s; + struct dhcp_msg msg; + struct dhcp_option *opt; + uint32_t server_ip = 0x0A000001U; + + wolfIP_init(&s); + s.last_tick = 1000U; + s.dhcp_xid = 0x1234U; + s.dhcp_state = DHCP_DISCOVER_SENT; + + /* OFFER with a server ID but a network-broadcast yiaddr: the parse + * must reject the message without committing the server identifier + * to the stack state. */ + memset(&msg, 0, sizeof(msg)); + msg.op = BOOT_REPLY; + msg.magic = ee32(DHCP_MAGIC); + msg.xid = ee32(s.dhcp_xid); + msg.yiaddr = ee32(0xC0A80100U); + opt = (struct dhcp_option *)msg.options; + opt->code = DHCP_OPTION_MSG_TYPE; + opt->len = 1; + opt->data[0] = DHCP_OFFER; + opt = (struct dhcp_option *)((uint8_t *)opt + 3); + opt->code = DHCP_OPTION_SERVER_ID; + opt->len = 4; + DHCP_OPT_u32_to_data(opt, server_ip); + opt = (struct dhcp_option *)((uint8_t *)opt + 6); + opt->code = DHCP_OPTION_SUBNET_MASK; + opt->len = 4; + DHCP_OPT_u32_to_data(opt, 0xFFFFFF00U); + opt = (struct dhcp_option *)((uint8_t *)opt + 6); + opt->code = DHCP_OPTION_END; + opt->len = 0; + ck_assert_int_eq(dhcp_parse_offer(&s, &msg, sizeof(msg)), -1); + ck_assert_uint_eq(s.dhcp_server_ip, 0); + ck_assert_int_eq(s.dhcp_state, DHCP_DISCOVER_SENT); +} +END_TEST + START_TEST(test_dhcp_schedule_lease_timer_defaults_t1_t2) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 463380cd..9fa0ff4d 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -9290,6 +9290,7 @@ static int dhcp_parse_offer(struct wolfIP *s, struct dhcp_msg *msg, uint32_t msg int saw_server_id = 0; int msg_type = 0; uint32_t ip; + uint32_t server_ip = 0; uint32_t netmask = DHCP_DEFAULT_24BIT_NETMASK; if (msg_len < DHCP_HEADER_LEN) return -1; @@ -9324,7 +9325,7 @@ static int dhcp_parse_offer(struct wolfIP *s, struct dhcp_msg *msg, uint32_t msg else if (code == DHCP_OPTION_SERVER_ID) { if (len < 4) return -1; - s->dhcp_server_ip = + server_ip = DHCP_OPT_data_to_u32((struct dhcp_option *)data); saw_server_id = 1; } @@ -9348,6 +9349,7 @@ static int dhcp_parse_offer(struct wolfIP *s, struct dhcp_msg *msg, uint32_t msg * until the server's ACK confirms the lease. */ s->dhcp_ip = ip; s->dhcp_offered_mask = netmask; + s->dhcp_server_ip = server_ip; dhcp_cancel_timer(s); s->dhcp_state = DHCP_REQUEST_SENT; return 0; From 151c6f5686aee830ef7ca3050b725c0d87a885b5 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 18:22:57 +0200 Subject: [PATCH 06/20] DHCP: wait 10 s after DECLINE before re-DISCOVER (F-13776) RFC 2131 4.4.2: after declining a conflicted address the client waits 10 s before a new DISCOVER so the server can stop leasing the address. The DAD-conflict path sent DECLINE and re-DISCOVER in the same tick, so the client re-acquired the same address in a conflict loop. The wait is armed as the DHCP timer and a new DHCP_OFF case in the timer callback drives the re-DISCOVER; the conflict wait is the only path that arms a timer while OFF (NAK and lease-expiry restarts call dhcp_send_discover directly). --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_dhcp_edges.c | 29 +++++++---- src/test/unit/unit_tests_dns_dhcp.c | 73 ++++++++++++++++++++++++++- src/wolfip.c | 14 ++++- 4 files changed, 105 insertions(+), 12 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 017d6adb..de23baf0 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -995,6 +995,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_dhcp_lease_ip_sane_rejects_bad_mask_and_network_addr); tcase_add_test(tc_proto, test_dhcp_long_lease_renewal_checkpoint_rearm); tcase_add_test(tc_proto, test_dhcp_parse_offer_reject_does_not_commit_server_id); + tcase_add_test(tc_proto, test_dhcp_dad_conflict_waits_10s_before_rediscover); tcase_add_test(tc_proto, test_dhcp_schedule_lease_timer_defaults_t1_t2); tcase_add_test(tc_proto, test_dhcp_schedule_lease_timer_small_lease_clamps_t1_t2); tcase_add_test(tc_proto, test_dhcp_parse_offer_defaults_mask_when_missing); diff --git a/src/test/unit/unit_tests_dhcp_edges.c b/src/test/unit/unit_tests_dhcp_edges.c index 37c1c411..bfad992c 100644 --- a/src/test/unit/unit_tests_dhcp_edges.c +++ b/src/test/unit/unit_tests_dhcp_edges.c @@ -1325,14 +1325,14 @@ START_TEST(test_dhcp_timer_cb_default_state_noop) wolfIP_init(&s); mock_link_init(&s); s.dhcp_xid = 0x5678U; - s.dhcp_state = DHCP_OFF; /* unhandled in switch */ + s.dhcp_state = 99; /* not a real state: default branch */ s.dhcp_timeout_count = 0; dhcp_timer_cb(&s); - /* Timer must reset to NO_TIMER and state must stay DHCP_OFF */ + /* Timer must reset to NO_TIMER and state must be untouched */ ck_assert_int_eq(s.dhcp_timer, NO_TIMER); - ck_assert_int_eq(s.dhcp_state, DHCP_OFF); + ck_assert_int_eq(s.dhcp_state, 99); } END_TEST @@ -1716,9 +1716,15 @@ START_TEST(test_dhcp_dad_conflict_releases_and_rediscover) arp_recv(&s, TEST_PRIMARY_IF, &reply, sizeof(reply)); - /* Address released, DAD aborted, back to discovery. */ + /* Address released, DAD aborted. RFC 2131 4.4.2: the re-DISCOVER + * is deferred 10 s behind the DECLINE. */ ck_assert_uint_eq(s.dhcp_dad_probes, 0U); ck_assert_uint_eq(primary->ip, 0U); + ck_assert_int_eq(s.dhcp_state, DHCP_OFF); + + /* The wait expires: the re-DISCOVER goes out. */ + s.last_tick += 10000U; + handle_timers(&s, s.last_tick); ck_assert_int_eq(s.dhcp_state, DHCP_DISCOVER_SENT); } END_TEST @@ -1771,8 +1777,9 @@ START_TEST(test_dhcp_dad_request_claiming_candidate_conflict) 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); + /* Conflict detected: DAD aborted, lease released. The re-DISCOVER + * waits 10 s behind the DECLINE (RFC 2131 4.4.2). */ + ck_assert_int_eq(s.dhcp_state, DHCP_OFF); ck_assert_uint_eq(s.dhcp_dad_probes, 0U); ck_assert_uint_eq(primary->ip, 0U); } @@ -1824,8 +1831,9 @@ START_TEST(test_dhcp_dad_probe_for_candidate_conflict) 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); + /* Conflict detected: DAD aborted, lease released. The re-DISCOVER + * waits 10 s behind the DECLINE (RFC 2131 4.4.2). */ + ck_assert_int_eq(s.dhcp_state, DHCP_OFF); ck_assert_uint_eq(s.dhcp_dad_probes, 0U); ck_assert_uint_eq(primary->ip, 0U); } @@ -1878,8 +1886,9 @@ START_TEST(test_dhcp_dad_garp_announcement_conflict) 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); + /* Conflict detected: DAD aborted, lease released. The re-DISCOVER + * waits 10 s behind the DECLINE (RFC 2131 4.4.2). */ + ck_assert_int_eq(s.dhcp_state, DHCP_OFF); ck_assert_uint_eq(s.dhcp_dad_probes, 0U); ck_assert_uint_eq(primary->ip, 0U); } diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index fe663a16..3fa4270e 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -270,7 +270,7 @@ START_TEST(test_dhcp_long_lease_renewal_checkpoint_rearm) uint32_t offer_ip = 0xC0A80164U; uint32_t mask = 0xFFFFFF00U; uint32_t lease_s = 5184000U; /* 60 days: default T1 (50%) exceeds the - * timer heap's 2^31 tick horizon */ + * timer heap's 2^31 tick horizon */ wolfIP_init(&s); mock_link_init(&s); @@ -393,6 +393,77 @@ START_TEST(test_dhcp_parse_offer_reject_does_not_commit_server_id) } END_TEST +START_TEST(test_dhcp_dad_conflict_waits_10s_before_rediscover) +{ + struct wolfIP s; + uint8_t frame[sizeof(struct arp_packet)]; + struct arp_packet *arp = (struct arp_packet *)frame; + struct wolfIP_udp_datagram *udp; + struct dhcp_msg *msg; + struct dhcp_option *opt; + const uint8_t att_mac[6] = {0x66, 0x55, 0x44, 0x33, 0x22, 0x11}; + uint32_t client_ip = 0x0A000064U; + uint32_t server_ip = 0x0A000001U; + uint64_t t0; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + ck_assert_int_eq(dhcp_client_init(&s), 0); + + /* The ACK offered client_ip; DAD is probing it. */ + s.dhcp_ip = client_ip; + s.dhcp_server_ip = server_ip; + s.dhcp_state = DHCP_DAD; + s.dhcp_dad_if = WOLFIP_PRIMARY_IF_IDX; + + /* A foreign host claims the candidate address during DAD: an ARP + * request carrying the candidate as sender IP. */ + memset(frame, 0, sizeof(frame)); + memset(arp->eth.dst, 0xFF, 6); + memcpy(arp->eth.src, att_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, att_mac, 6); + arp->sip = ee32(client_ip); + memset(arp->tma, 0xFF, 6); + wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, sizeof(frame)); + + /* RFC 2131 4.4.2: the DECLINE goes out immediately, but the + * re-DISCOVER waits 10 s so the server can stop leasing the + * address. */ + t0 = s.last_tick; + wolfIP_poll(&s, t0); + ck_assert_int_eq(s.dhcp_state, DHCP_OFF); + ck_assert_int_ne(s.dhcp_timer, NO_TIMER); + ck_assert_uint_eq(find_timer_expiry(&s, s.dhcp_timer), t0 + 10000U); + udp = (struct wolfIP_udp_datagram *)last_frame_sent; + msg = (struct dhcp_msg *)udp->data; + ck_assert_uint_eq(msg->op, BOOT_REQUEST); + opt = (struct dhcp_option *)msg->options; + ck_assert_uint_eq(opt->code, DHCP_OPTION_MSG_TYPE); + ck_assert_uint_eq(opt->data[0], DHCP_DECLINE); + + /* The wait is not over: nothing re-enters discovery. */ + wolfIP_poll(&s, t0 + 9999U); + ck_assert_int_eq(s.dhcp_state, DHCP_OFF); + + /* After the wait, the DISCOVER goes out. */ + wolfIP_poll(&s, t0 + 10000U); + ck_assert_int_eq(s.dhcp_state, DHCP_DISCOVER_SENT); + udp = (struct wolfIP_udp_datagram *)last_frame_sent; + msg = (struct dhcp_msg *)udp->data; + ck_assert_uint_eq(msg->op, BOOT_REQUEST); + opt = (struct dhcp_option *)msg->options; + ck_assert_uint_eq(opt->code, DHCP_OPTION_MSG_TYPE); + ck_assert_uint_eq(opt->data[0], DHCP_DISCOVER); +} +END_TEST + START_TEST(test_dhcp_schedule_lease_timer_defaults_t1_t2) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 9fa0ff4d..6abfdc44 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -1163,6 +1163,10 @@ static int wolfIP_filter_notify_icmp(enum wolfIP_filter_reason reason, /* RFC 4331 / RFC 5227: probe the address after a DHCPACK before using it. */ #define DHCP_DAD_PROBES 3 #define DHCP_DAD_INTERVAL_MS 1000U +/* RFC 2131 4.4.2: after declining a conflicted address, wait 10 s + * before sending a new DISCOVER so the server has time to process the + * DECLINE and stop leasing the address. */ +#define DHCP_DECLINE_WAIT_MS 10000U enum dhcp_state { DHCP_OFF = 0, @@ -9066,6 +9070,12 @@ static void dhcp_timer_cb(void *arg) } break; #endif + case DHCP_OFF: + /* Only the post-DECLINE wait timer arms while OFF + * (dhcp_dad_conflict); the NAK and lease-expiry restarts + * call dhcp_send_discover directly. */ + dhcp_send_discover(s); + break; default: break; } @@ -9923,7 +9933,9 @@ static void dhcp_dad_conflict(struct wolfIP *s) dhcp_deconfigure_lease(s); s->dhcp_state = DHCP_OFF; s->dhcp_timeout_count = 0; - dhcp_send_discover(s); + /* RFC 2131 4.4.2: the re-DISCOVER is deferred behind the 10 s + * post-DECLINE wait; the timer callback drives it. */ + dhcp_schedule_timer_at(s, s->last_tick + DHCP_DECLINE_WAIT_MS); } #endif From 2481b004887d3e4d33d82c86aa043478618edbf9 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 18:33:54 +0200 Subject: [PATCH 07/20] F-11436: restore GCM SA state only on successful construction The GCM constructor is the only one with a fallible step (pre-iv RNG) after the slot is filled; its state restore ran even after the failure wipe, querying the read callback with the wiped zero SPI. Return -1 right after the wipe so restore runs on success only. Adds test_esp_state_restore_gcm pinning the GCM success-path restore (persisted window applied, read callback called once with the SPI). --- src/test/unit/unit_esp.c | 75 ++++++++++++++++++++++++++++++++++++++++ src/wolfesp.c | 4 +-- 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/src/test/unit/unit_esp.c b/src/test/unit/unit_esp.c index 0d410ac5..e9e85ae3 100644 --- a/src/test/unit/unit_esp.c +++ b/src/test/unit/unit_esp.c @@ -2412,6 +2412,80 @@ START_TEST(test_esp_state_persistence_keeps_64bit_bitmap) } END_TEST +/* F-11436: the GCM constructor is the only SA constructor with a + * fallible step (pre-iv RNG) after the slot is filled. Its restore must + * run on the success path only; on the RNG failure path the SA is wiped + * and -1 returned, without querying the read callback with the wiped + * zero SPI. This test pins the success path: a GCM SA recreated with a + * known SPI picks up its persisted window. */ +static uint8_t state_gcm_spi[ESP_SPI_LEN] = {0x11, 0x22, 0x33, 0x44}; +static uint32_t state_gcm_oseq = 0; +static uint32_t state_gcm_hi_seq = 0; +static uint64_t state_gcm_bitmap = 0; +static int state_gcm_read_calls = 0; + +static int state_gcm_read_cb(const uint8_t *spi, uint32_t *oseq, + uint32_t *hi_seq, uint64_t *bitmap) +{ + state_gcm_read_calls++; + if (memcmp(spi, state_gcm_spi, ESP_SPI_LEN) == 0) { + *oseq = state_gcm_oseq; + *hi_seq = state_gcm_hi_seq; + *bitmap = state_gcm_bitmap; + return 0; + } + return -1; /* unknown SPI: start fresh */ +} + +static int state_gcm_write_cb(const uint8_t *spi, uint32_t oseq, + uint32_t hi_seq, uint64_t bitmap) +{ + if (memcmp(spi, state_gcm_spi, ESP_SPI_LEN) == 0) { + state_gcm_oseq = oseq; + state_gcm_hi_seq = hi_seq; + state_gcm_bitmap = bitmap; + } + return 0; +} + +START_TEST(test_esp_state_restore_gcm) +{ + int ret; + wolfIP_esp_sa *esp_sa; + + esp_setup(); + state_gcm_oseq = 5; + state_gcm_hi_seq = 40; + state_gcm_bitmap = (1ULL << 10) | 1ULL; + state_gcm_read_calls = 0; + + ret = wolfIP_esp_state_set_cbs(state_gcm_write_cb, state_gcm_read_cb); + ck_assert_int_eq(ret, 0); + + ret = wolfIP_esp_sa_new_gcm(1, (uint8_t *)state_gcm_spi, + atoip4(T_SRC), atoip4(T_DST), + ESP_ENC_GCM_RFC4543, + (uint8_t *)k_aes256_gcm, + sizeof(k_aes256_gcm)); + ck_assert_int_eq(ret, 0); + ck_assert_int_eq(state_gcm_read_calls, 1); + esp_sa = esp_sa_get(1, (uint8_t *)state_gcm_spi); + ck_assert_ptr_nonnull(esp_sa); + ck_assert_uint_eq(esp_sa->replay.oseq, 5U); + ck_assert_uint_eq(esp_sa->replay.hi_seq, 40U); + ck_assert_uint_eq(esp_sa->replay.bitmap, (1ULL << 10) | 1ULL); + + /* The restored window rejects the duplicate (seq 30, bit 10) and + * accepts a new sequence (41). */ + ck_assert_int_ne(esp_replay_check(&esp_sa->replay, 30U), 0); + ck_assert_int_eq(esp_replay_check(&esp_sa->replay, 41U), 0); + + wolfIP_esp_sa_del_all(); + ret = wolfIP_esp_state_set_cbs(NULL, NULL); + ck_assert_int_eq(ret, 0); +} +END_TEST + START_TEST(test_esp_state_persistence_callbacks) { static uint8_t buf[LINK_MTU + 256]; @@ -2589,6 +2663,7 @@ static Suite *esp_suite(void) tcase_add_test(tc, test_esp_state_persistence_callbacks); tcase_add_test(tc, test_esp_state_restore_failed_read_keeps_fresh_state); tcase_add_test(tc, test_esp_state_persistence_keeps_64bit_bitmap); + tcase_add_test(tc, test_esp_state_restore_gcm); suite_add_tcase(s, tc); /* Replay window */ diff --git a/src/wolfesp.c b/src/wolfesp.c index 79d8a404..bd16ed6b 100644 --- a/src/wolfesp.c +++ b/src/wolfesp.c @@ -260,13 +260,13 @@ int wolfIP_esp_sa_new_gcm(int in, uint8_t * spi, ip4 src, ip4 dst, if (err) { ESP_LOG("error: wc_RNG_GenerateBlock: %d\n", err); wc_ForceZero(new_sa, sizeof(*new_sa)); - err = -1; + return -1; } esp_state_restore(new_sa); ESP_DEBUG("info: esp_sa_new_gcm: %s\n", in == 1 ? "in" : "out"); - return err; + return 0; } /* Check if valid hmac auth config: From 3bd8d7aa0611ce43aaf7b28784ec83e55380332a Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 18:52:30 +0200 Subject: [PATCH 08/20] F-11435: raise CB_EVENT_WRITABLE when the ICMP txbuf drains flush_datagram_tx serves both UDP and ICMP sockets, but the post-drain writable event was gated on is_udp. An ICMP sender that got -WOLFIP_EAGAIN from a full txbuf stayed blocked after the drain until some unrelated event reached the socket. Raise the event for both protocols; the loopback path already handles itself via wolfIP_notify_loopback_space_available(). --- src/test/unit/unit.c | 3 +- src/test/unit/unit_tests_poll_dispatcher.c | 52 ++++++++++++++++++++++ src/wolfip.c | 9 ++-- 3 files changed, 59 insertions(+), 5 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index de23baf0..7fc438e6 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1394,7 +1394,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_sock_close_close_wait_disarms_callback); tcase_add_test(tc_core, test_rst_in_fin_wait_1_delivers_close_event); tcase_add_test(tc_core, test_last_ack_final_ack_delivers_close_event); - /* --- unit_tests_poll_dispatcher.c (47 tests) --- */ + /* --- unit_tests_poll_dispatcher.c (48 tests) --- */ tcase_add_test(tc_core, test_poll_device_poll_returns_zero_exits_loop); tcase_add_test(tc_core, test_poll_device_poll_returns_negative_exits_loop); tcase_add_test(tc_core, test_poll_device_non_ethernet_path_receives); @@ -1427,6 +1427,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_poll_tx_udp_filter_ip_blocks_send); tcase_add_test(tc_core, test_poll_tx_udp_eagain_retains_queue); tcase_add_test(tc_core, test_poll_tx_udp_drain_sets_writable); + tcase_add_test(tc_core, test_poll_tx_icmp_drain_sets_writable); tcase_add_test(tc_core, test_poll_tx_udp_broadcast_sets_ff_mac); tcase_add_test(tc_core, test_poll_tx_udp_loopback_path_no_crash); tcase_add_test(tc_core, test_poll_tx_icmp_sends_on_arp_hit); diff --git a/src/test/unit/unit_tests_poll_dispatcher.c b/src/test/unit/unit_tests_poll_dispatcher.c index 7583b5be..8099656d 100644 --- a/src/test/unit/unit_tests_poll_dispatcher.c +++ b/src/test/unit/unit_tests_poll_dispatcher.c @@ -1029,6 +1029,58 @@ START_TEST(test_poll_tx_udp_drain_sets_writable) } END_TEST +START_TEST(test_poll_tx_icmp_drain_sets_writable) +{ + struct wolfIP s; + int icmp_sd; + struct tsocket *ts; + struct wolfIP_sockaddr_in sin; + uint8_t payload[ICMP_HEADER_LEN + 56]; + uint8_t peer_mac[6] = {0x11, 0x22, 0x33, 0x44, 0x55, 0xA3}; + int rc; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + wolfIP_filter_set_callback(NULL, NULL); + + s.arp.neighbors[0].ip = 0x0A000002U; + s.arp.neighbors[0].if_idx = TEST_PRIMARY_IF; + memcpy(s.arp.neighbors[0].mac, peer_mac, 6); + + memset(payload, 0, sizeof(payload)); + payload[0] = ICMP_ECHO_REQUEST; + icmp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, + WI_IPPROTO_ICMP); + ck_assert_int_gt(icmp_sd, 0); + ts = &s.icmpsockets[SOCKET_UNMARK(icmp_sd)]; + + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_addr.s_addr = ee32(0x0A000002U); + + /* Fill the ICMP txbuf until sendto() reports the buffer full. */ + do { + rc = wolfIP_sock_sendto(&s, icmp_sd, payload, sizeof(payload), 0, + (struct wolfIP_sockaddr *)&sin, sizeof(sin)); + } while (rc > 0); + ck_assert_int_eq(rc, -WOLFIP_EAGAIN); + ts->if_idx = TEST_PRIMARY_IF; + + /* A blocked sendto() would now be waiting for CB_EVENT_WRITABLE. */ + ts->events = 0; + + /* Poll drains the queue over the wire (mock_send succeeds). */ + (void)wolfIP_poll(&s, 200); + + /* Draining freed txbuf space, so the drain must raise CB_EVENT_WRITABLE + * to wake a blocked ICMP sender (F-11435). Before the fix the event was + * raised for UDP only and an ICMP sender blocked on a full buffer stayed + * blocked until some unrelated event reached the socket. */ + ck_assert_uint_ne((unsigned)(ts->events & CB_EVENT_WRITABLE), 0U); +} +END_TEST + START_TEST(test_poll_tx_udp_broadcast_sets_ff_mac) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 6abfdc44..0ef1b16d 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -12217,11 +12217,12 @@ static void flush_datagram_tx(struct wolfIP *s, struct tsocket *socks, tx_drained = 1; desc = fifo_peek(&t->sock.udp.txbuf); } - /* UDP: Draining the txbuf frees space; raise CB_EVENT_WRITABLE so a sender - * blocked on a full buffer (e.g. the FreeRTOS BSD shim's sendto()) is - * woken. The loopback path is handled separately via + /* UDP and ICMP sockets share this flush: draining the txbuf frees + * space, so raise CB_EVENT_WRITABLE for either protocol to wake a + * sender blocked on a full buffer (e.g. the FreeRTOS BSD shim's + * sendto()). The loopback path is handled separately via * wolfIP_notify_loopback_space_available(). */ - if (is_udp && tx_drained && tx_has_writable_space(t)) + if (tx_drained && tx_has_writable_space(t)) t->events |= CB_EVENT_WRITABLE; } } From f8715936b4864a45ad399173658cdc073051dd08 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 18:58:09 +0200 Subject: [PATCH 09/20] F-12391: clear stale local_ip on ICMP bind to ANY The ICMP bind arm had no final fallback: re-binding IPADDR_ANY while no interface holds a configured address retained the previous local_ip, so the socket kept sending with an address the stack no longer owned. Reset to IPADDR_ANY like the TCP and UDP arms. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_api.c | 31 +++++++++++++++++++++++++++++++ src/wolfip.c | 2 ++ 3 files changed, 34 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 7fc438e6..1d95f4b0 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -216,6 +216,7 @@ Suite *wolf_suite(void) 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_bind_icmp_any_resets_stale_local_ip); tcase_add_test(tc_utils, test_sock_connect_wrong_family); tcase_add_test(tc_utils, test_sock_accept_error_paths); tcase_add_test(tc_utils, test_sock_accept_non_tcp_socket_sets_addrlen); diff --git a/src/test/unit/unit_tests_api.c b/src/test/unit/unit_tests_api.c index 47a559fe..26bafa8b 100644 --- a/src/test/unit/unit_tests_api.c +++ b/src/test/unit/unit_tests_api.c @@ -1709,6 +1709,37 @@ START_TEST(test_sock_bind_icmp_success) } END_TEST +START_TEST(test_sock_bind_icmp_any_resets_stale_local_ip) +{ + struct wolfIP s; + int icmp_sd; + struct tsocket *ts; + struct wolfIP_sockaddr_in sin; + + wolfIP_init(&s); + mock_link_init(&s); + /* No ipconfig: no interface holds a configured address. */ + + icmp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_ICMP); + ck_assert_int_gt(icmp_sd, 0); + ts = &s.icmpsockets[SOCKET_UNMARK(icmp_sd)]; + /* Stale local address from an earlier bind/send while the interface + * still held 10.0.0.1 (F-12391). */ + ts->local_ip = 0x0A000001U; + + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_port = ee16(7); + sin.sin_addr.s_addr = ee32(IPADDR_ANY); + + ck_assert_int_eq(wolfIP_sock_bind(&s, icmp_sd, (struct wolfIP_sockaddr *)&sin, sizeof(sin)), 0); + /* Re-binding ANY with no configured address must clear the stale + * local_ip, as the UDP and TCP bind arms do. */ + ck_assert_uint_eq(ts->local_ip, IPADDR_ANY); + ck_assert_uint_eq(ts->src_port, 7U); +} +END_TEST + START_TEST(test_sock_connect_wrong_family) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 0ef1b16d..47ced272 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -8576,6 +8576,8 @@ int wolfIP_sock_bind(struct wolfIP *s, int sockfd, const struct wolfIP_sockaddr struct ipconf *primary = wolfIP_primary_ipconf(s); if (primary && primary->ip != IPADDR_ANY) ts->local_ip = primary->ip; + else + ts->local_ip = IPADDR_ANY; } if (bind_port_in_use(s->icmpsockets, MAX_ICMPSOCKETS, ts, ts->local_ip, new_id)) { From 2daf3f1d91be6735ecb293acd223c7db9036f601 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 19:26:05 +0200 Subject: [PATCH 10/20] F-13189: send ICMP Parameter Problem for malformed forwarded IP options The option walker in ip_recv silently dropped transit datagrams whose IP options failed length validation. RFC 1122 3.2.2.4 requires a Parameter Problem (type 12) with the pointer at the offending option byte. Model the reply on the existing ttl-exceeded constructor, including the error-in-response-to-error suppression; multicast destinations are exempt per RFC 1812 4.3.2.4. Locally addressed packets are still dropped silently. --- src/test/unit/unit.c | 3 + src/test/unit/unit_tests_ip_arp_recv.c | 172 +++++++++++++++++++++++++ src/test/unit/unit_tests_proto.c | 25 +++- src/wolfip.c | 151 +++++++++++++++++++++- 4 files changed, 344 insertions(+), 7 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 1d95f4b0..8958ef6e 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1556,6 +1556,9 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_ip_recv_forward_ttl1_partial_payload_quoted); tcase_add_test(tc_core, test_forward_ttl_exceeded_copies_orig_tos); tcase_add_test(tc_core, test_ip_recv_forward_df_oversize_sends_frag_needed); + tcase_add_test(tc_core, test_ip_recv_forward_bad_option_sends_param_problem); + tcase_add_test(tc_core, test_ip_recv_local_bad_option_silent_drop); + tcase_add_test(tc_core, test_ip_recv_forward_bad_option_icmp_error_suppressed); tcase_add_test(tc_core, test_ip_recv_forward_nodf_oversize_dropped); tcase_add_test(tc_core, test_ip_recv_forward_df_at_mtu_forwarded); tcase_add_test(tc_core, test_ip_recv_forward_directed_bcast_ingress_net_not_forwarded); diff --git a/src/test/unit/unit_tests_ip_arp_recv.c b/src/test/unit/unit_tests_ip_arp_recv.c index ba28c9b7..5bf29cbb 100644 --- a/src/test/unit/unit_tests_ip_arp_recv.c +++ b/src/test/unit/unit_tests_ip_arp_recv.c @@ -1468,6 +1468,178 @@ START_TEST(test_ip_recv_forward_df_oversize_sends_frag_needed) } END_TEST +/* ========================================================================= + * ip_recv: transit datagram with a malformed IP option - Parameter Problem + * ========================================================================= + * RFC 1122 3.2.2.4: an option whose length runs past the end of the header + * must produce an ICMP Parameter Problem (type 12, code 0) with the pointer + * at the offending option byte; the datagram itself is not relayed. + */ +START_TEST(test_ip_recv_forward_bad_option_sends_param_problem) +{ + struct wolfIP s; + uint8_t frame[ETH_HEADER_LEN + 24 + 8]; + struct wolfIP_ip_packet *ip = (struct wolfIP_ip_packet *)frame; + ip4 primary_ip = 0x0A000001U; + ip4 secondary_ip = 0xC0A80101U; + ip4 dest_ip = 0xC0A80155U; + ip4 src_ip = 0x0A000002U; + static const uint8_t dest_mac[6] = {0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF}; + struct wolfIP_icmp_packet *ic; + uint8_t *opt; + + setup_stack_with_two_ifaces(&s, primary_ip, secondary_ip); + wolfIP_filter_set_callback(NULL, NULL); + + arp_store_neighbor(&s, TEST_SECOND_IF, dest_ip, dest_mac); + last_frame_sent_size = 0; + + memset(frame, 0, sizeof(frame)); + memcpy(ip->eth.dst, s.ll_dev[TEST_PRIMARY_IF].mac, 6); + memcpy(ip->eth.src, "\x01\x02\x03\x04\x05\x06", 6); + ip->eth.type = ee16(ETH_TYPE_IP); + ip->ver_ihl = 0x46; /* IHL 6: 20-byte header + 4 option bytes */ + ip->flags_fo = 0; + ip->ttl = 64; + ip->proto = WI_IPPROTO_UDP; + ip->len = ee16(24 + 8); + ip->src = ee32(src_ip); + ip->dst = ee32(dest_ip); + /* Record Route option at offset 20 with length 100: runs past the end + * of the 4-byte option area. */ + opt = frame + ETH_HEADER_LEN + IP_HEADER_LEN; + opt[0] = 0x44; + opt[1] = 100; + fix_ip_checksum(ip); + + ip_recv(&s, TEST_PRIMARY_IF, ip, (uint32_t)sizeof(frame)); + + /* Only the Parameter Problem reply is transmitted; the datagram itself + * is not relayed. */ + ck_assert_uint_eq(last_frame_sent_count, 1); + /* 14 ETH + 20 IP + 8 ICMP + 32 quoted (24 header + 8 payload). */ + ck_assert_uint_eq(last_frame_sent_size, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + 8 + 24 + 8)); + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + IP_HEADER_LEN], + ICMP_PARAM_PROBLEM); + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + IP_HEADER_LEN + 1], 0); + /* Pointer: the offending option type byte, offset 20 from the IP + * header start. */ + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + IP_HEADER_LEN + 4], 20); + /* Reply carries DF, TTL 64, from the ingress interface to the + * datagram's source. */ + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + 6], 0x40); + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + 8], 64); + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + 12], (primary_ip >> 24) & 0xFF); + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + 16], (src_ip >> 24) & 0xFF); + /* Quoted original: version/IHL 0x46 and source address. */ + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + IP_HEADER_LEN + 8], 0x46); + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + IP_HEADER_LEN + 8 + 12], + (src_ip >> 24) & 0xFF); + ic = (struct wolfIP_icmp_packet *)(last_frame_sent + + ETH_HEADER_LEN + IP_HEADER_LEN); + ck_assert_uint_eq(ic->csum, ee16(icmp_checksum( + (struct wolfIP_icmp_packet *)last_frame_sent, + (uint16_t)(8 + 24 + 8)))); +} +END_TEST + +/* ========================================================================= + * ip_recv: malformed IP option on a locally addressed datagram - silent + * drop + * ========================================================================= + * The Parameter Problem reply is for the transit case (RFC 1122 3.2.2.4); + * a packet addressed to one of our own addresses is dropped without an + * error and not delivered locally. + */ +START_TEST(test_ip_recv_local_bad_option_silent_drop) +{ + struct wolfIP s; + uint8_t frame[ETH_HEADER_LEN + 24 + 8]; + struct wolfIP_ip_packet *ip = (struct wolfIP_ip_packet *)frame; + ip4 primary_ip = 0x0A000001U; + ip4 secondary_ip = 0xC0A80101U; + ip4 src_ip = 0x0A000002U; + uint8_t *opt; + + setup_stack_with_two_ifaces(&s, primary_ip, secondary_ip); + wolfIP_filter_set_callback(NULL, NULL); + last_frame_sent_size = 0; + + memset(frame, 0, sizeof(frame)); + memcpy(ip->eth.dst, s.ll_dev[TEST_PRIMARY_IF].mac, 6); + memcpy(ip->eth.src, "\x01\x02\x03\x04\x05\x06", 6); + ip->eth.type = ee16(ETH_TYPE_IP); + ip->ver_ihl = 0x46; + ip->flags_fo = 0; + ip->ttl = 64; + ip->proto = WI_IPPROTO_UDP; + ip->len = ee16(24 + 8); + ip->src = ee32(src_ip); + ip->dst = ee32(primary_ip); /* addressed to us: not a transit packet */ + opt = frame + ETH_HEADER_LEN + IP_HEADER_LEN; + opt[0] = 0x44; + opt[1] = 100; + fix_ip_checksum(ip); + + ip_recv(&s, TEST_PRIMARY_IF, ip, (uint32_t)sizeof(frame)); + + /* No ICMP of any kind; the datagram is not delivered locally either. */ + ck_assert_uint_eq(last_frame_sent_count, 0); +} +END_TEST + +/* ========================================================================= + * ip_recv: malformed IP option on an ICMP error - suppressed + * ========================================================================= + * RFC 1812 4.3.2.7: no ICMP error is originated in response to another + * ICMP error, even when the option is malformed. + */ +START_TEST(test_ip_recv_forward_bad_option_icmp_error_suppressed) +{ + struct wolfIP s; + uint8_t frame[ETH_HEADER_LEN + 24 + 8]; + struct wolfIP_ip_packet *ip = (struct wolfIP_ip_packet *)frame; + ip4 primary_ip = 0x0A000001U; + ip4 secondary_ip = 0xC0A80101U; + ip4 dest_ip = 0xC0A80155U; + ip4 src_ip = 0x0A000002U; + static const uint8_t dest_mac[6] = {0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF}; + uint8_t *opt; + + setup_stack_with_two_ifaces(&s, primary_ip, secondary_ip); + wolfIP_filter_set_callback(NULL, NULL); + + arp_store_neighbor(&s, TEST_SECOND_IF, dest_ip, dest_mac); + last_frame_sent_size = 0; + + memset(frame, 0, sizeof(frame)); + memcpy(ip->eth.dst, s.ll_dev[TEST_PRIMARY_IF].mac, 6); + memcpy(ip->eth.src, "\x01\x02\x03\x04\x05\x06", 6); + ip->eth.type = ee16(ETH_TYPE_IP); + ip->ver_ihl = 0x46; + ip->flags_fo = 0; + ip->ttl = 64; + ip->proto = WI_IPPROTO_ICMP; + ip->len = ee16(24 + 8); + ip->src = ee32(src_ip); + ip->dst = ee32(dest_ip); + opt = frame + ETH_HEADER_LEN + IP_HEADER_LEN; + opt[0] = 0x44; + opt[1] = 100; + /* ICMP destination unreachable payload: the error-in-response-to-an- + * error suppression applies. */ + frame[ETH_HEADER_LEN + 24] = ICMP_DEST_UNREACH; + frame[ETH_HEADER_LEN + 25] = 0; + fix_ip_checksum(ip); + + ip_recv(&s, TEST_PRIMARY_IF, ip, (uint32_t)sizeof(frame)); + + /* Suppressed: no ICMP of any kind. */ + ck_assert_uint_eq(last_frame_sent_count, 0); +} +END_TEST + /* ========================================================================= * ip_recv: DF-clear datagram exceeding the egress MTU - silent drop * ========================================================================= diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index 7f758f18..5047a71b 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -4148,7 +4148,9 @@ START_TEST(test_regression_forwarding_drops_source_routed_packet) END_TEST /* A source route hidden behind an option with an illegal length byte (< 2) must - * still be caught, so the packet is never relayed with its options intact. */ + * still be caught, so the packet is never relayed with its options intact. The + * router answers the source with a Parameter Problem pointing at the + * malformed option (RFC 1122 3.2.2.4) instead of a silent drop. */ START_TEST(test_regression_forwarding_drops_source_route_behind_undersized_option) { static const uint8_t opt_types[] = { 0x83U, 0x89U }; /* LSRR, SSRR */ @@ -4156,6 +4158,7 @@ START_TEST(test_regression_forwarding_drops_source_route_behind_undersized_optio static const uint8_t iface1_mac[6] = {0x02, 0x00, 0x00, 0x00, 0x00, 0x02}; static const uint8_t next_hop_mac[6] = {0x02, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE}; static const uint32_t dest_ip = 0xC0A80164U; /* 192.168.1.100 on TEST_SECOND_IF */ + static const uint32_t src_ip = 0xC0A800AAU; /* in TEST_PRIMARY_IF subnet, passes RPF */ unsigned int i; for (i = 0; i < sizeof(opt_types) / sizeof(opt_types[0]); i++) { @@ -4181,7 +4184,7 @@ START_TEST(test_regression_forwarding_drops_source_route_behind_undersized_optio frame->ttl = 64; frame->proto = WI_IPPROTO_UDP; frame->len = ee16(IP_HEADER_LEN + 12); - frame->src = ee32(0xC0A800AAU); /* in TEST_PRIMARY_IF subnet, passes RPF */ + frame->src = ee32(src_ip); frame->dst = ee32(dest_ip); opts[0] = 0x44; /* Timestamp */ @@ -4201,7 +4204,23 @@ START_TEST(test_regression_forwarding_drops_source_route_behind_undersized_optio wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + 12)); - ck_assert_uint_eq(last_frame_sent_size, 0); + /* Never relayed: the single frame out is the Parameter Problem reply + * to the datagram's source, not a copy of the datagram. */ + ck_assert_uint_eq(last_frame_sent_size, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + 8 + 32)); + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + 9], WI_IPPROTO_ICMP); + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + IP_HEADER_LEN], + ICMP_PARAM_PROBLEM); + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + IP_HEADER_LEN + 1], 0); + /* Pointer: the malformed option type byte, offset 20 from the IP + * header start. */ + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + IP_HEADER_LEN + 4], + IP_HEADER_LEN); + /* Addressed to the datagram's source. */ + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + 16], (src_ip >> 24) & 0xFF); + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + 17], (src_ip >> 16) & 0xFF); + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + 18], (src_ip >> 8) & 0xFF); + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + 19], src_ip & 0xFF); } } END_TEST diff --git a/src/wolfip.c b/src/wolfip.c index 47ced272..d2dbb230 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -84,6 +84,7 @@ struct wolfIP_icmp_packet; #define ICMP_PROT_UNREACH 2 #define ICMP_PORT_UNREACH 3 #define ICMP_FRAG_NEEDED 4 +#define ICMP_PARAM_PROBLEM 12 #define WI_IPPROTO_ICMP 0x01 #define WI_IPPROTO_IGMP 0x02 @@ -822,6 +823,15 @@ struct PACKED wolfIP_icmp_dest_unreachable_packet { uint8_t orig_packet[TTL_EXCEEDED_ORIG_PACKET_SIZE_MAX]; }; +struct PACKED wolfIP_icmp_param_problem_packet { + struct wolfIP_ip_packet ip; + uint8_t type, code; + uint16_t csum; + uint8_t pointer; /* offending octet, offset from the IP header start */ + uint8_t reserved[3]; + uint8_t orig_packet[TTL_EXCEEDED_ORIG_PACKET_SIZE_MAX]; +}; + #ifdef IP_MULTICAST struct udp_mcast_join { ip4 group; @@ -2401,6 +2411,121 @@ static void wolfIP_send_ttl_exceeded(struct wolfIP *s, unsigned int if_idx, } #endif +#if WOLFIP_ENABLE_FORWARDING && defined(ETHERNET) +/* RFC 1122 3.2.2.4: a router that cannot forward a datagram because of a + * malformed IP option (missing or too-short option length, or an option + * running past the end of the header) answers the source with Parameter + * Problem (type 12, code 0); the pointer octet marks the offending + * option's type byte, offset from the IP header start. The reply goes out + * the interface the datagram arrived on, addressed to its source. */ +static void wolfIP_send_param_problem(struct wolfIP *s, unsigned int if_idx, + struct wolfIP_ip_packet *orig, + uint8_t pointer) +{ + struct wolfIP_ll_dev *ll = wolfIP_ll_at(s, if_idx); + struct wolfIP_icmp_param_problem_packet icmp = {0}; + struct wolfIP_icmp_packet *icmp_pkt = (struct wolfIP_icmp_packet *)&icmp; + uint32_t orig_ihl = (orig->ver_ihl & 0x0F) * 4; + uint32_t orig_total; + uint32_t orig_copy; + uint32_t icmp_data_len; +#if !CONFIG_IPFILTER + (void)icmp_pkt; +#endif + if (!ll) + return; +#if WOLFIP_VLAN + /* Same interface-validity rule as wolfIP_ll_send_frame: an active VLAN + * sub-iface has a NULL send and delegates to its parent. */ + if (ll->vlan_active) { + if (!ll->vlan_parent) + return; + } else if (!ll->send) { + return; + } +#else + if (!ll->send) + return; +#endif + if (orig_ihl < IP_HEADER_LEN) + orig_ihl = IP_HEADER_LEN; + /* RFC 1812 4.3.2.7: an ICMP error MUST NOT be originated in response to + * another ICMP error (type 3, 4, 5, 11, 12). A zero-payload ICMP cannot + * be an error, so it is never suppressed. */ + if (orig->proto == WI_IPPROTO_ICMP && ee16(orig->len) > orig_ihl) { + uint8_t orig_type = *(((uint8_t *)orig) + ETH_HEADER_LEN + orig_ihl); + if (orig_type == ICMP_DEST_UNREACH || orig_type == ICMP_FRAG_NEEDED || + orig_type == 5 /* Redirect */ || orig_type == ICMP_TTL_EXCEEDED || + orig_type == ICMP_PARAM_PROBLEM) + return; + } + /* Quote the original header plus up to 8 payload bytes, or as much of + * the datagram as exists. */ + orig_total = ee16(orig->len); + if (orig_total < orig_ihl) + orig_total = orig_ihl; + orig_copy = orig_ihl + 8; + if (orig_copy > orig_total) + orig_copy = orig_total; + if (orig_copy > TTL_EXCEEDED_ORIG_PACKET_SIZE_MAX) + orig_copy = TTL_EXCEEDED_ORIG_PACKET_SIZE_MAX; + icmp_data_len = 8 + orig_copy; /* ICMP header + quoted packet */ + icmp.type = ICMP_PARAM_PROBLEM; + icmp.pointer = pointer; + /* RFC 1812 4.3.2.5: the error carries the triggering packet's TOS. */ + icmp.ip.tos = orig->tos; + memcpy(icmp.orig_packet, ((uint8_t *)orig) + ETH_HEADER_LEN, orig_copy); + icmp.csum = ee16(icmp_checksum((struct wolfIP_icmp_packet *)&icmp, + icmp_data_len)); + icmp.ip.ver_ihl = 0x45; + icmp.ip.flags_fo = ee16(0x4000U); + icmp.ip.ttl = 64; + icmp.ip.proto = WI_IPPROTO_ICMP; + icmp.ip.id = ipcounter_next(s); + icmp.ip.len = ee16((uint16_t)(IP_HEADER_LEN + icmp_data_len)); + icmp.ip.src = ee32(wolfIP_ipconf_at(s, if_idx)->ip); + icmp.ip.dst = orig->src; + icmp.ip.csum = 0; + iphdr_set_checksum(&icmp.ip); + { + uint32_t frame_len = ETH_HEADER_LEN + IP_HEADER_LEN + icmp_data_len; + 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, IP_HEADER_LEN) != 0) + return; + if (wolfIP_filter_notify_ip(WOLFIP_FILT_SENDING, s, if_idx, &icmp.ip, frame_len) != 0) + return; + if (!wolfIP_ll_is_non_ethernet(s, if_idx)) { + if (wolfIP_filter_notify_eth(WOLFIP_FILT_SENDING, s, if_idx, &icmp.ip.eth, frame_len) != 0) + return; + } +#ifdef WOLFIP_ESP + if (!wolfIP_ll_is_non_ethernet(s, if_idx)) { + if (esp_send(ll, &icmp.ip, (uint16_t)(frame_len - ETH_HEADER_LEN)) == 1) { + wolfIP_ll_send_frame(s, if_idx, &icmp, frame_len); + } + } else { + wolfIP_ll_send_frame(s, if_idx, &icmp, frame_len); + } +#else + wolfIP_ll_send_frame(s, if_idx, &icmp, frame_len); +#endif + } +} +#elif WOLFIP_ENABLE_FORWARDING +static void wolfIP_send_param_problem(struct wolfIP *s, unsigned int if_idx, + struct wolfIP_ip_packet *orig, + uint8_t pointer) +{ + (void)s; + (void)if_idx; + (void)orig; + (void)pointer; +} +#endif + #if WOLFIP_ENABLE_FORWARDING && defined(ETHERNET) /* RFC 1812 4.3.2.4: a router that cannot relay a DF-set datagram because it * exceeds the egress MTU answers the source with Fragmentation Needed @@ -10676,6 +10801,7 @@ static inline void ip_recv(struct wolfIP *s, unsigned int if_idx, { uint8_t version; uint32_t ip_hlen; + uint16_t bad_opt_off = 0; /* malformed option offset, 0 = none */ #if WOLFIP_ENABLE_FORWARDING unsigned int i; #endif @@ -10760,10 +10886,15 @@ static inline void ip_recv(struct wolfIP *s, unsigned int if_idx, } if (type == 0x83 || type == 0x89) /* LSRR or SSRR */ return; - if (opt + 1 >= opt_end || opt[1] < 2) - return; - if (opt[1] > (uint8_t)(opt_end - opt)) - return; + if ((opt + 1 >= opt_end || opt[1] < 2) || + opt[1] > (uint8_t)(opt_end - opt)) { + /* Malformed option: record the offending type byte (offset + * from the IP header start) so the transit path can answer + * with a Parameter Problem; the packet is dropped either + * way. */ + bad_opt_off = (uint16_t)(opt - (uint8_t *)ip - ETH_HEADER_LEN); + break; + } opt += opt[1]; } } @@ -10891,6 +11022,16 @@ static inline void ip_recv(struct wolfIP *s, unsigned int if_idx, uint8_t mac[6]; int broadcast = 0; + if (bad_opt_off != 0) { + /* RFC 1122 3.2.2.4: a transit datagram with a malformed + * IP option gets a Parameter Problem pointing at the + * offending option byte, not a silent drop. Multicast + * destinations are exempt (RFC 1812 4.3.2.4). */ + if (!wolfIP_ip_is_multicast(dest)) + wolfIP_send_param_problem(s, if_idx, ip, + (uint8_t)bad_opt_off); + return; + } if (ip->ttl <= 1) { wolfIP_send_ttl_exceeded(s, if_idx, ip); return; @@ -10960,6 +11101,8 @@ static inline void ip_recv(struct wolfIP *s, unsigned int if_idx, } } #endif /* WOLFIP_ENABLE_FORWARDING */ + if (bad_opt_off != 0) + return; /* malformed IP options: never deliver locally */ #ifdef DEBUG_IP wolfIP_print_ip(ip); #endif /* DEBUG_IP*/ From 285d6f26712a445f6544c6684a874c1fcf3fe9b3 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 19:30:05 +0200 Subject: [PATCH 11/20] F-13190: document the RFC 1256 router discovery deviation The router portion of ICMP Router Discovery (solicited/periodic advertisements, solicitation handling, RFC 1256 configuration variables) is intentionally not implemented: wolfIP hosts are configured via DHCP or static config, routing is connected plus static only, and there is no host-side consumer in the ecosystem. Record the deviation in the forwarding how-to next to the fragmentation deviation, mark the icmp_input fall-through, and correct the stale Fragmentation Needed sentence the code superseded. --- docs/advanced_ipv4_howto.md | 36 +++++++++++++++++++++++++++--------- src/wolfip.c | 4 ++++ 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/docs/advanced_ipv4_howto.md b/docs/advanced_ipv4_howto.md index e7302883..218d9bbe 100644 --- a/docs/advanced_ipv4_howto.md +++ b/docs/advanced_ipv4_howto.md @@ -255,13 +255,12 @@ wolfIP is an endpoint stack first; the forwarding path deliberately omits IPv4 fragmentation and reassembly. These are documented, intended deviations, not bugs: -- **No egress fragmentation.** A forwarded datagram is handed to the egress - interface at its declared IP total length, with no comparison against the - egress IP MTU and no fragment generation. If the frame exceeds the link MTU, - `wolfIP_ll_send_frame()` rejects it and the datagram is **dropped silently** — - no ICMP Destination Unreachable (Fragmentation Needed, type 3 code 4) is - sent, regardless of the DF bit. Datagrams that fit the egress MTU are - forwarded normally. +- **No egress fragmentation.** A forwarded datagram is never split into + fragments. A DF-set datagram larger than the egress IP MTU is dropped with + an ICMP Destination Unreachable (Fragmentation Needed, type 3 code 4) + carrying the egress next-hop MTU (RFC 1812 4.3.2.4); a DF-clear datagram + that does not fit is **dropped silently** on transmit. Datagrams that fit + the egress MTU are forwarded normally. - **No reassembly.** The IP input path drops every fragment (MF set or non-zero fragment offset); the stack never reassembles fragmented datagrams. - **Locally generated UDP.** `wolfIP_sock_sendto()` fails with `-1` when the @@ -274,8 +273,27 @@ The practical consequence for a router build: keep every link's MTU at or above the largest datagram that traverses it (the usual 1500-byte Ethernet baseline). A higher-MTU upstream (e.g. jumbo frames) that injects datagrams larger than a downstream link's IP MTU will see them dropped at the egress -with no diagnostic ICMP. If your topology cannot guarantee that, IPv4 -fragmentation is out of scope for wolfIP and a different stack is needed. +(no Fragmentation Needed reply when the DF bit is clear). If your topology +cannot guarantee that, IPv4 fragmentation is out of scope for wolfIP and a +different stack is needed. + +### Deviations from RFC 1256: no ICMP Router Discovery + +wolfIP's forwarding path does not implement the router portion of ICMP +Router Discovery (RFC 1256): it neither sends Router Advertisements +(type 9), solicited or periodic, nor answers Router Solicitations (type 10). +This is a documented, intended deviation, not a bug: + +- wolfIP hosts are configured by DHCP or static configuration; the stack has + no host-side router-discovery consumer that would need the router side. +- Routing is connected-subnet plus static routes only (no dynamic protocol), + so there is no route information to advertise beyond what DHCP or a static + default gateway already provides. +- Periodic advertisement would add per-interface timer and scheduling state + to the forwarding path for no in-ecosystem consumer. + +Hosts attached to a wolfIP router must obtain their default gateway from +DHCP or be configured with the router's address statically. ### Wiring a router diff --git a/src/wolfip.c b/src/wolfip.c index d2dbb230..a9e5ccf1 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -8918,6 +8918,10 @@ static void icmp_input(struct wolfIP *s, unsigned int if_idx, struct wolfIP_ip_p #endif return; } + /* Router Advertisement (9) / Router Solicitation (10) are intentionally + * not handled: wolfIP documents a deliberate RFC 1256 deviation - no + * router discovery, hosts use DHCP or a static gateway. See + * docs/advanced_ipv4_howto.md (F-13190). */ icmp_try_deliver_tcp_error(s, icmp); icmp_try_recv(s, if_idx, icmp, len); } From 46a18a2e5397fce6b9e6c4a339140831b06aabcd Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 19:44:29 +0200 Subject: [PATCH 12/20] F-12390: plain teardown for aborted accept clones Accept clones carry the listener's callback, so closing one on an error path (SYN-ACK send failure, accepting-filter rejection, and the pre-accept struct-copy path) deferred a CB_EVENT_CLOSED for a descriptor accept() never returned. Route the abort through abort_accept_clone(), which clears the callback first. --- src/test/unit/unit_tests_dns_dhcp.c | 12 ++++++++++++ src/wolfip.c | 16 +++++++++++++--- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index 3fa4270e..6b251426 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -4393,6 +4393,7 @@ START_TEST(test_sock_accept_filtered_out) struct wolfIP s; int listen_sd; int new_sd; + int i; struct wolfIP_sockaddr_in sin; socklen_t alen = sizeof(sin); struct tsocket *listen_ts; @@ -4409,12 +4410,14 @@ START_TEST(test_sock_accept_filtered_out) 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); + wolfIP_register_callback(&s, listen_sd, test_socket_cb, NULL); filter_block_reason = WOLFIP_FILT_ACCEPTING; filter_block_calls = 0; wolfIP_filter_set_callback(test_filter_cb_block, NULL); wolfIP_filter_set_mask(WOLFIP_FILT_MASK(WOLFIP_FILT_ACCEPTING)); + socket_cb_calls = 0; inject_tcp_syn(&s, TEST_PRIMARY_IF, 0x0A000001U, 1234); new_sd = wolfIP_sock_accept(&s, listen_sd, (struct wolfIP_sockaddr *)&sin, &alen); ck_assert_int_eq(new_sd, -1); @@ -4422,6 +4425,15 @@ START_TEST(test_sock_accept_filtered_out) listen_ts = &s.tcpsockets[SOCKET_UNMARK(listen_sd)]; ck_assert_int_eq(listen_ts->sock.tcp.state, TCP_LISTEN); + /* The aborted clone takes the plain teardown path: the listener's + * callback must not be deferred as a CB_EVENT_CLOSED for a + * descriptor accept() never returned. */ + wolfIP_poll(&s, 1000); + ck_assert_int_eq(socket_cb_calls, 0); + for (i = 0; i < MAX_TCPSOCKETS; i++) { + ck_assert_uint_eq(s.tcpsockets[i].close_notify_pending, 0); + } + ck_assert_ptr_eq(listen_ts->callback, test_socket_cb); wolfIP_filter_set_callback(NULL, NULL); } diff --git a/src/wolfip.c b/src/wolfip.c index a9e5ccf1..b216849d 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -6976,6 +6976,16 @@ int wolfIP_sock_connect(struct wolfIP *s, int sockfd, const struct wolfIP_sockad return -WOLFIP_EINVAL; } +/* Aborted accept clone: clear the listener's callback before teardown so + * close_socket() takes the plain path instead of deferring a + * CB_EVENT_CLOSED for a descriptor accept() never returned. */ +static void abort_accept_clone(struct tsocket *ts) +{ + ts->callback = NULL; + ts->callback_arg = NULL; + close_socket(ts); +} + int wolfIP_sock_accept(struct wolfIP *s, int sockfd, struct wolfIP_sockaddr *addr, socklen_t *addrlen) { struct tsocket *ts; @@ -7062,7 +7072,7 @@ int wolfIP_sock_accept(struct wolfIP *s, int sockfd, struct wolfIP_sockaddr *add WOLFIP_FILT_ACCEPTING, s, newts, newts->local_ip, newts->src_port, newts->remote_ip, newts->dst_port) != 0) { - close_socket(newts); + abort_accept_clone(newts); return -1; } return (newts - s->tcpsockets) | MARK_TCP_SOCKET; @@ -7110,7 +7120,7 @@ int wolfIP_sock_accept(struct wolfIP *s, int sockfd, struct wolfIP_sockaddr *add * while we're still accepting. */ if (tcp_send_syn(newts, TCP_FLAG_SYN | TCP_FLAG_ACK) < 0) { - close_socket(newts); + abort_accept_clone(newts); return -WOLFIP_EAGAIN; } ts->events &= ~CB_EVENT_READABLE; @@ -7138,7 +7148,7 @@ int wolfIP_sock_accept(struct wolfIP *s, int sockfd, struct wolfIP_sockaddr *add if (wolfIP_filter_notify_socket_event( WOLFIP_FILT_ACCEPTING, s, newts, newts->local_ip, newts->src_port, newts->remote_ip, newts->dst_port) != 0) { - close_socket(newts); + abort_accept_clone(newts); return -1; } return (newts - s->tcpsockets) | MARK_TCP_SOCKET; From 9c6ff95358ff940e11163af068ca6821addf486d Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 19:46:54 +0200 Subject: [PATCH 13/20] F-13167: getpeername for connected UDP sockets UDP connect stores dst_port/remote_ip but getpeername only handled TCP (and raw), so a connected UDP socket returned an error instead of its peer. Add the UDP branch: connected sockets report the peer, unconnected ones return -1 like the raw no-remote-ip case. --- src/test/unit/unit.c | 3 ++ src/test/unit/unit_tests_socket_api_arms.c | 63 ++++++++++++++++++++++ src/wolfip.c | 15 ++++++ 3 files changed, 81 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 8958ef6e..4e0b8a97 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1298,6 +1298,9 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_sock_getpeername_tcp_success); tcase_add_test(tc_core, test_sock_getpeername_tcp_invalid_fd); tcase_add_test(tc_core, test_sock_getpeername_tcp_null_addr); + tcase_add_test(tc_core, test_sock_getpeername_udp_connected); + tcase_add_test(tc_core, test_sock_getpeername_udp_unconnected); + tcase_add_test(tc_core, test_sock_getpeername_udp_invalid_fd); #if WOLFIP_RAWSOCKETS tcase_add_test(tc_core, test_sock_getpeername_raw_success); tcase_add_test(tc_core, test_sock_getpeername_raw_no_remote_ip); diff --git a/src/test/unit/unit_tests_socket_api_arms.c b/src/test/unit/unit_tests_socket_api_arms.c index 720fea67..ce5166fb 100644 --- a/src/test/unit/unit_tests_socket_api_arms.c +++ b/src/test/unit/unit_tests_socket_api_arms.c @@ -1679,6 +1679,69 @@ START_TEST(test_sock_getpeername_tcp_null_addr) } END_TEST +START_TEST(test_sock_getpeername_udp_connected) +{ + struct wolfIP s; + int sd; + struct wolfIP_sockaddr_in peer; + struct wolfIP_sockaddr_in out; + socklen_t outlen = sizeof(out); + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_ge(sd, 0); + memset(&peer, 0, sizeof(peer)); + peer.sin_family = AF_INET; + peer.sin_port = ee16(5353); + peer.sin_addr.s_addr = ee32(0x0A000002U); + ck_assert_int_eq(wolfIP_sock_connect(&s, sd, (struct wolfIP_sockaddr *)&peer, + sizeof(peer)), 0); + + ck_assert_int_eq(wolfIP_sock_getpeername(&s, sd, (struct wolfIP_sockaddr *)&out, + &outlen), 0); + ck_assert_uint_eq(out.sin_family, AF_INET); + ck_assert_uint_eq(ee32(out.sin_addr.s_addr), 0x0A000002U); + ck_assert_uint_eq(ee16(out.sin_port), 5353); +} +END_TEST + +START_TEST(test_sock_getpeername_udp_unconnected) +{ + struct wolfIP s; + int sd; + struct wolfIP_sockaddr_in out; + socklen_t outlen = sizeof(out); + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_ge(sd, 0); + + /* No connect(): no peer to report */ + ck_assert_int_eq(wolfIP_sock_getpeername(&s, sd, (struct wolfIP_sockaddr *)&out, + &outlen), -1); +} +END_TEST + +START_TEST(test_sock_getpeername_udp_invalid_fd) +{ + struct wolfIP s; + struct wolfIP_sockaddr_in out; + socklen_t outlen = sizeof(out); + + wolfIP_init(&s); + mock_link_init(&s); + ck_assert_int_eq(wolfIP_sock_getpeername(&s, MARK_UDP_SOCKET | MAX_UDPSOCKETS, + (struct wolfIP_sockaddr *)&out, + &outlen), -WOLFIP_EINVAL); +} +END_TEST + #if WOLFIP_RAWSOCKETS START_TEST(test_sock_getpeername_raw_success) { diff --git a/src/wolfip.c b/src/wolfip.c index b216849d..38464a3b 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -8806,6 +8806,21 @@ int wolfIP_sock_getpeername(struct wolfIP *s, int sockfd, struct wolfIP_sockaddr sin->sin_addr.s_addr = ee32(ts->remote_ip); return 0; } + if (IS_SOCKET_UDP(sockfd)) { + if (SOCKET_UNMARK(sockfd) >= MAX_UDPSOCKETS) + return -WOLFIP_EINVAL; + ts = &s->udpsockets[SOCKET_UNMARK(sockfd)]; + /* An unconnected UDP socket has no peer: connect() is what + * stores dst_port/remote_ip, so only report them when set. */ + if (ts->sock.udp.connected == 0) + return -1; + if (!sin || !addrlen || *addrlen < sizeof(struct wolfIP_sockaddr_in)) + return -1; + sin->sin_family = AF_INET; + sin->sin_port = ee16(ts->dst_port); + sin->sin_addr.s_addr = ee32(ts->remote_ip); + return 0; + } #if WOLFIP_RAWSOCKETS if (IS_SOCKET_RAW(sockfd)) { struct rawsocket *rs = wolfIP_rawsocket_from_fd(s, sockfd); From 1bfb48020d97c591edff1bf6f244598ed9ce4272 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 19:53:48 +0200 Subject: [PATCH 14/20] F-13207: validate caller IHL in the IP_HDRINCL sendto path A raw-socket header declaring more header bytes than were supplied made the dst-override checksum recompute iterate past the initialized frame, folding stale stack bytes into the checksum. Require IPv4 and IHL*4 in [20, len] before trusting the header; the FIFO push already bounds the wire bytes to what was supplied. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_socket_api_arms.c | 58 ++++++++++++++++++++++ src/wolfip.c | 9 ++++ 3 files changed, 68 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 4e0b8a97..b6d482d7 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1257,6 +1257,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_sock_sendto_raw_null_dest_no_remote_ip); tcase_add_test(tc_core, test_sock_sendto_raw_hdrincl_dst_from_buf); tcase_add_test(tc_core, test_raw_hdrincl_dst_override_recomputes_ip_checksum); + tcase_add_test(tc_core, test_raw_hdrincl_oversized_ihl_rejected); tcase_add_test(tc_core, test_sock_sendto_raw_invalid_fd); tcase_add_test(tc_core, test_sock_sendto_raw_fifo_full_returns_eagain); tcase_add_test(tc_core, test_sock_setsockopt_raw_hdrincl); diff --git a/src/test/unit/unit_tests_socket_api_arms.c b/src/test/unit/unit_tests_socket_api_arms.c index ce5166fb..2a518733 100644 --- a/src/test/unit/unit_tests_socket_api_arms.c +++ b/src/test/unit/unit_tests_socket_api_arms.c @@ -1071,6 +1071,64 @@ START_TEST(test_raw_hdrincl_dst_override_recomputes_ip_checksum) } END_TEST +/* IP_HDRINCL: the caller's IHL is untrusted. A header that declares more + * bytes than were supplied must be rejected before the checksum recompute + * iterates past the initialized buffer (F-13207). */ +START_TEST(test_raw_hdrincl_oversized_ihl_rejected) +{ + struct wolfIP s; + int sd; + int one = 1; + uint8_t ip_buf[ETH_HEADER_LEN + 60]; + struct wolfIP_ip_packet *ip; + struct wolfIP_sockaddr_in sin; + int ret; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_RAW, WI_IPPROTO_UDP); + ck_assert_int_ge(sd, 0); + ck_assert_int_eq(wolfIP_sock_setsockopt(&s, sd, WOLFIP_SOL_IP, + WOLFIP_IP_HDRINCL, &one, sizeof(one)), 0); + + /* Declare a 60-byte header but supply only 20. */ + memset(ip_buf, 0, sizeof(ip_buf)); + ip = (struct wolfIP_ip_packet *)ip_buf; + ip->ver_ihl = 0x4F; + ip->ttl = 64; + ip->src = ee32(0x0A000001U); + ip->dst = ee32(0x0A000002U); + + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_addr.s_addr = ee32(0x0A000003U); + ret = wolfIP_sock_sendto(&s, sd, + (uint8_t *)ip + ETH_HEADER_LEN, + IP_HEADER_LEN, + 0, (const struct wolfIP_sockaddr *)&sin, + sizeof(sin)); + ck_assert_int_eq(ret, -WOLFIP_EINVAL); + + /* Non-IPv4 version and IHL below 20 bytes are rejected the same way. */ + ip->ver_ihl = 0x65; + ret = wolfIP_sock_sendto(&s, sd, + (uint8_t *)ip + ETH_HEADER_LEN, + IP_HEADER_LEN, + 0, (const struct wolfIP_sockaddr *)&sin, + sizeof(sin)); + ck_assert_int_eq(ret, -WOLFIP_EINVAL); + ip->ver_ihl = 0x41; + ret = wolfIP_sock_sendto(&s, sd, + (uint8_t *)ip + ETH_HEADER_LEN, + IP_HEADER_LEN, + 0, (const struct wolfIP_sockaddr *)&sin, + sizeof(sin)); + ck_assert_int_eq(ret, -WOLFIP_EINVAL); +} +END_TEST + START_TEST(test_sock_sendto_raw_invalid_fd) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 38464a3b..4125ffde 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -7465,8 +7465,17 @@ int wolfIP_sock_sendto(struct wolfIP *s, int sockfd, const void *buf, size_t len return -WOLFIP_EINVAL; if (rs->ipheader_include) { + uint32_t ip_hlen; if (len < IP_HEADER_LEN) return -WOLFIP_EINVAL; + /* The header is caller-supplied and its IHL is untrusted: + * the checksum recompute below iterates IHL*4 bytes, so the + * declared header length must fit in the supplied data. */ + if (((const uint8_t *)buf)[0] >> 4 != 4) + return -WOLFIP_EINVAL; + ip_hlen = (uint32_t)(((const uint8_t *)buf)[0] & 0x0fU) << 2; + if (ip_hlen < IP_HEADER_LEN || ip_hlen > len) + return -WOLFIP_EINVAL; #ifdef ETHERNET memset(rip, 0, ETH_HEADER_LEN); #endif From 0af8f24cb301f6eaac5f0f8267d8c936ae7755aa Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 20:01:43 +0200 Subject: [PATCH 15/20] F-13766: extend the VLAN delete EBUSY scan to the ICMP and AF_PACKET socket tables wolfIP_vlan_delete() refuses to free a slot while interface-indexed state still references it, because wolfIP_vlan_create reuses freed slots. The scan covered routes, multicast, TCP, UDP and raw sockets but missed icmpsockets[] and packetsockets[], which bind an if_idx the same way: a socket left on the deleted index would silently transmit and receive through the next VLAN created in that slot. Add both scans using each table's own liveness field (proto != 0 for the tsocket ICMP table, used for packetsockets, under the same WOLFIP_RAWSOCKETS/WOLFIP_PACKET_SOCKETS nesting as the struct definitions). Test test_vlan_delete_rejected_with_icmp_socket: an ICMP socket on the VLAN sub-iface makes delete return -WOLFIP_EBUSY, the socket survives, close releases the dependency and delete then succeeds. Verified in the unit-vlan build (make unit-vlan): pre-fix the test fails with ret == 0 (delete succeeds), post-fix 1615/1615; plain make unit 1570/1570. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_vlan.c | 32 ++++++++++++++++++++++++++++++++ src/wolfip.c | 12 ++++++++++++ 3 files changed, 45 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index b6d482d7..225e3849 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1746,6 +1746,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_vlan_delete_rejected_with_route); #endif tcase_add_test(tc_proto, test_vlan_delete_rejected_with_socket); + tcase_add_test(tc_proto, test_vlan_delete_rejected_with_icmp_socket); tcase_add_test(tc_proto, test_vlan_api_get_null_args_rejected); tcase_add_test(tc_proto, test_vlan_api_get_dangling_parent_pointer_rejected); tcase_add_test(tc_proto, test_vlan_tx_active_without_parent_rejected); diff --git a/src/test/unit/unit_tests_vlan.c b/src/test/unit/unit_tests_vlan.c index 8926b478..76a28c70 100644 --- a/src/test/unit/unit_tests_vlan.c +++ b/src/test/unit/unit_tests_vlan.c @@ -694,6 +694,38 @@ START_TEST(test_vlan_delete_rejected_with_socket) } END_TEST +/* Regression (F-13766): the EBUSY dependency scan must also cover the + * ICMP socket table; an ICMP socket bound to the VLAN keeps it busy. */ +START_TEST(test_vlan_delete_rejected_with_icmp_socket) +{ + struct wolfIP s; + unsigned int sub_idx = 0; + int sd; + struct tsocket *ts; + int ret; + + setup_vlan_stack(&s); + + ret = wolfIP_vlan_create(&s, TEST_PRIMARY_IF, 100, 0, 0, &sub_idx); + ck_assert_int_eq(ret, 0); + + sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_ICMP); + ck_assert_int_gt(sd, 0); + ts = &s.icmpsockets[SOCKET_UNMARK(sd)]; + ts->if_idx = (uint8_t)sub_idx; + + ret = wolfIP_vlan_delete(&s, sub_idx); + ck_assert_int_eq(ret, -WOLFIP_EBUSY); + + /* The socket survives a rejected delete (it belongs to the app). */ + ck_assert_uint_eq(ts->if_idx, (uint8_t)sub_idx); + + ck_assert_int_eq(wolfIP_sock_close(&s, sd), 0); + ret = wolfIP_vlan_delete(&s, sub_idx); + ck_assert_int_eq(ret, 0); +} +END_TEST + /* Regression: wolfIP_vlan_get used to default *parent_if_idx to 0 if the * parent pointer didn't match any slot in ll_dev[], silently reporting the * wrong parent. After the fix it must return -WOLFIP_EINVAL and leave the diff --git a/src/wolfip.c b/src/wolfip.c index 4125ffde..1e12e0bf 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -10692,12 +10692,24 @@ int wolfIP_vlan_delete(struct wolfIP *s, unsigned int if_idx) s->udpsockets[i].if_idx == (uint8_t)if_idx) return -WOLFIP_EBUSY; } + for (i = 0; i < MAX_ICMPSOCKETS; i++) { + if (s->icmpsockets[i].proto != 0 && + s->icmpsockets[i].if_idx == (uint8_t)if_idx) + return -WOLFIP_EBUSY; + } #if WOLFIP_RAWSOCKETS for (i = 0; i < WOLFIP_MAX_RAWSOCKETS; i++) { if (s->rawsockets[i].used && s->rawsockets[i].if_idx == (uint8_t)if_idx) return -WOLFIP_EBUSY; } +#if WOLFIP_PACKET_SOCKETS + for (i = 0; i < WOLFIP_MAX_PACKETSOCKETS; i++) { + if (s->packetsockets[i].used && + s->packetsockets[i].if_idx == (uint8_t)if_idx) + return -WOLFIP_EBUSY; + } +#endif #endif /* Wipe the slot so it can be reused. s->if_count is not changed to avoid * renumbering active sub-ifaces. */ From 81a04d2036d6f8aa6ee5b52dbfa1559e18edbb9c Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 20:04:16 +0200 Subject: [PATCH 16/20] F-12389: print real version and header length in the IP debug dump wolfIP_print_ip labeled the columns (ipv, hdr_len) but printed a hard-coded 0x04 and the packed ver_ihl byte, misreporting packets with IP options or malformed versions. Print ver_ihl >> 4 for the version and (ver_ihl & 0x0f) * 4 for the header length in bytes. --- src/wolfip_debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wolfip_debug.c b/src/wolfip_debug.c index 5ce9f703..b7a13b06 100644 --- a/src/wolfip_debug.c +++ b/src/wolfip_debug.c @@ -52,7 +52,7 @@ static void wolfIP_print_ip(struct wolfIP_ip_packet * ip) LOG("ip hdr:\n"); LOG("+-----------------------------+\n"); LOG("| 0x%02x | 0x%02x | 0x%02x | %4d | (ipv, hdr_len, tos, ip_len)\n", - 0x04, ip->ver_ihl, ip->tos, ee16(ip->len)); + ip->ver_ihl >> 4, (ip->ver_ihl & 0x0fU) * 4, ip->tos, ee16(ip->len)); LOG("+-----------------------------+\n"); LOG("| 0x%04x | 0x%04x | (id, flags_fo)\n", ee16(ip->id), ee16(ip->flags_fo)); From 01641383107d1cbc6a8f34cb54c4ddfea509e87d Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 20:06:49 +0200 Subject: [PATCH 17/20] F-12393: fix the UDP debug dump for empty payloads wolfIP_print_udp early-returned before the closing rows when the datagram had no payload; guard only the payload preview on len > UDP_HEADER_LEN so the dump always completes. Same pass: hoist the preview locals out of the anonymous block and use UDP_HEADER_LEN instead of the magic 8. --- src/wolfip_debug.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/wolfip_debug.c b/src/wolfip_debug.c index b7a13b06..47ba3219 100644 --- a/src/wolfip_debug.c +++ b/src/wolfip_debug.c @@ -76,6 +76,9 @@ static inline int wolfip_isprint(int c) static void wolfIP_print_udp(struct wolfIP_udp_datagram * udp) { uint16_t len = ee16(udp->len); + uint16_t max_len = 16; + size_t i; + size_t print_len = 0; char payload_str[32]; LOG("udp hdr:\n"); LOG("+-------------------+\n"); @@ -86,18 +89,15 @@ static void wolfIP_print_udp(struct wolfIP_udp_datagram * udp) len, ee16(udp->csum)); LOG("+-------------------+\n"); memset(payload_str, '\0', sizeof(payload_str)); - { + if (len > UDP_HEADER_LEN) { /* show first 16 printable chars of payload */ - uint16_t max_len = 16; - size_t i = 0; - size_t print_len = 0; - if (len <= UDP_HEADER_LEN) - return; - print_len = (len - 8) < max_len ? (len - 8): max_len; - memset(payload_str, '\0', sizeof(payload_str)); + print_len = (len - UDP_HEADER_LEN) < max_len ? + (len - UDP_HEADER_LEN) : max_len; memcpy(payload_str, udp->data, print_len); for (i = 0; i < print_len; i++) { - if (!wolfip_isprint(payload_str[i])) { payload_str[i] = '.'; } + if (!wolfip_isprint(payload_str[i])) { + payload_str[i] = '.'; + } } } LOG("| %17s | (payload first 16 bytes)\n", payload_str); From 6fde1df92072f171e7b08d82902605810cd49f88 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 20:06:49 +0200 Subject: [PATCH 18/20] F-12394: correct the PMKSA fast reconnect contract The doc promised the cached PMK skips SAE/EAP authentication entirely, but pmksa_reconnect rejects any cache without a PMKID (supplicant.c:1762) and the PMKID is only stored for SAE auth (supplicant.c:944-947). State the PMKID precondition and that EAP/PEAP sessions never satisfy it. --- src/supplicant/supplicant.h | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/supplicant/supplicant.h b/src/supplicant/supplicant.h index 877ce0ed..cf0c3525 100644 --- a/src/supplicant/supplicant.h +++ b/src/supplicant/supplicant.h @@ -409,11 +409,15 @@ int wolfip_supplicant_get_pmkid(const struct wolfip_supplicant *s, /* Set up a PMKSA "fast reconnect": when a valid PMKSA is cached for the * current SSID + BSSID (from a prior AUTHENTICATED session on this context), - * reuse the cached PMK + PMKID, skip the SAE/EAP authentication entirely, - * include the PMKID in the (Re)Assoc RSN IE, and go straight to the 4-way - * handshake on the next wolfip_supplicant_kick(). Call after _init() (which - * preserves the cache) and before kick(). Returns 0 if a usable PMKSA was - * found and armed, -1 otherwise (caller should fall back to a full auth). */ + * reuse the cached PMK + PMKID, skip the authentication exchange, include + * the PMKID in the (Re)Assoc RSN IE, and go straight to the 4-way handshake + * on the next wolfip_supplicant_kick(). Call after _init() (which preserves + * the cache) and before kick(). Returns 0 if a usable PMKSA was found and + * armed, -1 otherwise (caller should fall back to a full auth). + * + * Note: the cached entry must carry a PMKID, which is only stored for SAE + * authentication; EAP/PEAP sessions do not produce one, so they never + * satisfy this API and always perform a full EAP authentication. */ int wolfip_supplicant_pmksa_reconnect(struct wolfip_supplicant *s); #ifdef __cplusplus From 8ecce389eda8d0edc58ae696806c9a46c99b8d9e Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 23:21:46 +0200 Subject: [PATCH 19/20] F-13189: validate IP option length before source-route drop A malformed LSRR/SSRR option hit the source-route drop before the length check ran, so it was silently dropped instead of getting a Parameter Problem. Validate the option length first; the source-route drop now only applies to well-formed source routes. Adds a forwarding regression test for the malformed-source-route Parameter Problem. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_proto.c | 84 ++++++++++++++++++++++++++++++++ src/wolfip.c | 8 ++- 3 files changed, 91 insertions(+), 2 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 225e3849..2ada98fe 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -948,6 +948,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_regression_forwarding_rpf_drops_spoofed_source); tcase_add_test(tc_proto, test_regression_forwarding_drops_source_routed_packet); tcase_add_test(tc_proto, test_regression_forwarding_drops_source_route_behind_undersized_option); + tcase_add_test(tc_proto, test_regression_forwarding_malformed_source_route_param_problem); tcase_add_test(tc_proto, test_regression_loopback_source_dropped_on_non_loopback_iface); tcase_add_test(tc_proto, test_regression_icmp_echo_request_non_local_dst_no_reply); tcase_add_test(tc_proto, test_tcp_listen_rejects_wrong_interface); diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index 5047a71b..78584294 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -4225,6 +4225,90 @@ START_TEST(test_regression_forwarding_drops_source_route_behind_undersized_optio } END_TEST +/* A malformed source-route option (LSRR/SSRR with an illegal length byte) + * must be reported with a Parameter Problem on the transit path, not silently + * dropped by the source-route policy. The source-route drop only applies to + * well-formed source routes; a malformed option is malformed regardless of + * type, so the length is validated before the type is acted on. */ +START_TEST(test_regression_forwarding_malformed_source_route_param_problem) +{ + static const uint8_t opt_types[] = { 0x83U, 0x89U }; /* LSRR, SSRR */ + static const uint8_t src_mac[6] = {0x52, 0x54, 0x00, 0x12, 0x34, 0x56}; + static const uint8_t iface1_mac[6] = {0x02, 0x00, 0x00, 0x00, 0x00, 0x02}; + static const uint8_t next_hop_mac[6] = {0x02, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE}; + static const uint32_t dest_ip = 0xC0A80164U; /* 192.168.1.100 on TEST_SECOND_IF */ + static const uint32_t src_ip = 0xC0A800AAU; /* in TEST_PRIMARY_IF subnet, passes RPF */ + unsigned int i; + + for (i = 0; i < sizeof(opt_types) / sizeof(opt_types[0]); i++) { + struct wolfIP s; + uint8_t frame_buf[ETH_HEADER_LEN + IP_HEADER_LEN + 12]; + struct wolfIP_ip_packet *frame = (struct wolfIP_ip_packet *)frame_buf; + uint8_t *opts = frame_buf + ETH_HEADER_LEN + IP_HEADER_LEN; + + wolfIP_init(&s); + mock_link_init(&s); + mock_link_init_idx(&s, TEST_SECOND_IF, iface1_mac); + wolfIP_ipconfig_set(&s, 0xC0A80001U, 0xFFFFFF00U, 0); + wolfIP_ipconfig_set_ex(&s, TEST_SECOND_IF, 0xC0A80101U, 0xFFFFFF00U, 0); + s.arp.neighbors[0].ip = dest_ip; + s.arp.neighbors[0].if_idx = TEST_SECOND_IF; + memcpy(s.arp.neighbors[0].mac, next_hop_mac, 6); + + memset(frame_buf, 0, sizeof(frame_buf)); + memcpy(frame->eth.dst, s.ll_dev[TEST_PRIMARY_IF].mac, 6); + memcpy(frame->eth.src, src_mac, 6); + frame->eth.type = ee16(ETH_TYPE_IP); + frame->ver_ihl = 0x48; /* IHL=8, 32-byte header */ + frame->ttl = 64; + frame->proto = WI_IPPROTO_UDP; + frame->len = ee16(IP_HEADER_LEN + 12); + frame->src = ee32(src_ip); + frame->dst = ee32(dest_ip); + + /* A source-route option with an illegal length byte (< 2): the type + * says LSRR/SSRR but the option is malformed. */ + opts[0] = opt_types[i]; /* LSRR or SSRR */ + opts[1] = 1; /* illegal: an option is at least type + length */ + opts[2] = 0x00; /* end-of-options padding */ + opts[3] = 0x00; + opts[4] = 0x00; + opts[5] = 0x00; + opts[6] = 0x00; + opts[7] = 0x00; + opts[8] = 0x00; + opts[9] = 0x00; + opts[10] = 0x00; + opts[11] = 0x00; + fix_ip_checksum_with_hlen(frame, (uint16_t)(IP_HEADER_LEN + 12)); + + memset(last_frame_sent, 0, sizeof(last_frame_sent)); + last_frame_sent_size = 0; + + wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + 12)); + + /* The malformed source route is reported with a Parameter Problem, + * not silently dropped by the source-route policy. */ + ck_assert_uint_eq(last_frame_sent_size, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + 8 + 32)); + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + 9], WI_IPPROTO_ICMP); + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + IP_HEADER_LEN], + ICMP_PARAM_PROBLEM); + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + IP_HEADER_LEN + 1], 0); + /* Pointer: the malformed source-route type byte, offset 20 from the + * IP header start. */ + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + IP_HEADER_LEN + 4], + IP_HEADER_LEN); + /* Addressed to the datagram's source. */ + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + 16], (src_ip >> 24) & 0xFF); + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + 17], (src_ip >> 16) & 0xFF); + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + 18], (src_ip >> 8) & 0xFF); + ck_assert_uint_eq(last_frame_sent[ETH_HEADER_LEN + 19], src_ip & 0xFF); + } +} +END_TEST + START_TEST(test_regression_loopback_source_dropped_on_non_loopback_iface) { static const ip4 spoofed_loopback_sources[] = { diff --git a/src/wolfip.c b/src/wolfip.c index 1e12e0bf..a4b4dc7b 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -10934,8 +10934,10 @@ static inline void ip_recv(struct wolfIP *s, unsigned int if_idx, opt++; continue; } - if (type == 0x83 || type == 0x89) /* LSRR or SSRR */ - return; + /* Validate the option length before acting on the type, so a + * malformed option (of any type, including a malformed source + * route) is reported with a Parameter Problem rather than + * silently dropped. */ if ((opt + 1 >= opt_end || opt[1] < 2) || opt[1] > (uint8_t)(opt_end - opt)) { /* Malformed option: record the offending type byte (offset @@ -10945,6 +10947,8 @@ static inline void ip_recv(struct wolfIP *s, unsigned int if_idx, bad_opt_off = (uint16_t)(opt - (uint8_t *)ip - ETH_HEADER_LEN); break; } + if (type == 0x83 || type == 0x89) /* LSRR or SSRR, well-formed */ + return; opt += opt[1]; } } From 90bae3b5c7761d6ccbd4ea96a0a1ca59403caadc Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 16 Sep 2026 23:56:16 +0200 Subject: [PATCH 20/20] Fix NULL ll->send deref in Parameter Problem ESP path esp_send() was passed the VLAN child ll, whose send function is intentionally NULL (tx delegates to vlan_parent), crashing whenever ESP is enabled and a malformed transit packet arrives on a VLAN sub-interface. Resolve the VLAN parent for the ESP call, matching the existing Fragmentation Needed path. Found by Copilot review on PR 174 (follow-up to F-13189). --- src/wolfip.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/wolfip.c b/src/wolfip.c index a4b4dc7b..766f5cb6 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -2503,7 +2503,14 @@ static void wolfIP_send_param_problem(struct wolfIP *s, unsigned int if_idx, } #ifdef WOLFIP_ESP if (!wolfIP_ll_is_non_ethernet(s, if_idx)) { - if (esp_send(ll, &icmp.ip, (uint16_t)(frame_len - ETH_HEADER_LEN)) == 1) { + struct wolfIP_ll_dev *esp_ll = ll; +#if WOLFIP_VLAN + /* A VLAN sub-iface has no send function of its own; esp_send needs + * the physical device's send path. */ + if (ll->vlan_active && ll->vlan_parent) + esp_ll = ll->vlan_parent; +#endif + if (esp_send(esp_ll, &icmp.ip, (uint16_t)(frame_len - ETH_HEADER_LEN)) == 1) { wolfIP_ll_send_frame(s, if_idx, &icmp, frame_len); } } else {