From 3f373bffb7886affff7f0fc278b051e0688dd47e Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Thu, 26 Mar 2026 13:37:43 +0100 Subject: [PATCH 01/10] checking is the segment is SYN after established, send RST and drop the segment instead of sprocessing silently --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_proto.c | 81 ++++++++++++++++++++++++++++++++ src/wolfip.c | 8 ++++ 3 files changed, 90 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index a29c0a11..ab1c2fe8 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -669,6 +669,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_regression_udp_payload_exceeds_buffer_discards_and_unblocks); tcase_add_test(tc_proto, test_regression_icmp_payload_exceeds_buffer_discards_and_unblocks); tcase_add_test(tc_proto, test_regression_tcp_ip_len_below_ip_header); + tcase_add_test(tc_proto, test_regression_syn_on_established_not_silently_processed); tcase_add_test(tc_utils, test_transport_checksum); tcase_add_test(tc_utils, test_iphdr_set_checksum); diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index 83a6c0c0..c12f15a6 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -3848,4 +3848,85 @@ START_TEST(test_regression_tcp_ip_len_below_ip_header) END_TEST +/* RFC 9293 §3.10.7.4: SYN on synchronized connection must not be silently + * processed. The implementation must either send a challenge ACK and drop + * (RFC 5961) or send RST and abort (original RFC 793). The current code + * silently processes an in-window SYN as normal data on ESTABLISHED + * connections, potentially corrupting connection state. */ +START_TEST(test_regression_syn_on_established_not_silently_processed) +{ + struct wolfIP s; + struct tsocket *ts; + struct wolfIP_tcp_seg seg; + uint32_t original_ack; + uint32_t original_seq; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + last_frame_sent_size = 0; + + /* Set up an ARP entry so the stack can send a reply frame */ + s.arp.neighbors[0].ip = 0x0A000002U; + s.arp.neighbors[0].if_idx = TEST_PRIMARY_IF; + memcpy(s.arp.neighbors[0].mac, + (uint8_t[]){0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF}, 6); + + /* Wire up socket slot 0 as an ESTABLISHED connection */ + ts = &s.tcpsockets[0]; + memset(ts, 0, sizeof(*ts)); + ts->proto = WI_IPPROTO_TCP; + ts->S = &s; + ts->sock.tcp.state = TCP_ESTABLISHED; + ts->sock.tcp.ack = 100; + ts->sock.tcp.seq = 1000; + ts->sock.tcp.snd_una = 1000; + ts->sock.tcp.cwnd = TCP_MSS; + ts->sock.tcp.peer_rwnd = TCP_MSS; + ts->src_port = 1234; + ts->dst_port = 4321; + ts->local_ip = 0x0A000001U; + ts->remote_ip = 0x0A000002U; + ts->if_idx = TEST_PRIMARY_IF; + queue_init(&ts->sock.tcp.rxbuf, ts->rxmem, RXBUF_SIZE, ts->sock.tcp.ack); + fifo_init(&ts->sock.tcp.txbuf, ts->txmem, TXBUF_SIZE); + + original_ack = ts->sock.tcp.ack; + original_seq = ts->sock.tcp.seq; + + /* Craft an in-window SYN segment (seq == rcv_nxt so it passes the + * acceptability test and reaches the established-state handler). */ + memset(&seg, 0, sizeof(seg)); + seg.ip.ver_ihl = 0x45; + seg.ip.ttl = 64; + seg.ip.proto = WI_IPPROTO_TCP; + seg.ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN); + seg.ip.src = ee32(ts->remote_ip); + seg.ip.dst = ee32(ts->local_ip); + seg.dst_port = ee16(ts->src_port); + seg.src_port = ee16(ts->dst_port); + seg.hlen = TCP_HEADER_LEN << 2; + seg.flags = TCP_FLAG_SYN; + seg.seq = ee32(100); /* == ts->sock.tcp.ack, i.e. in-window */ + seg.win = ee16(65535); + fix_tcp_checksums(&seg); + + tcp_input(&s, TEST_PRIMARY_IF, &seg, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN)); + + /* tcp_send_ack enqueues into txbuf; flush it via wolfIP_poll */ + (void)wolfIP_poll(&s, 200); + + /* The SYN must NOT be silently accepted. The connection state must not + * have been corrupted by processing the SYN as data. Verify: + * 1) The connection did not stay silently in ESTABLISHED with no reply + * (last_frame_sent_size > 0 means a challenge ACK or RST was sent). + * 2) The ack/seq values were not altered by data processing. */ + ck_assert_uint_gt(last_frame_sent_size, 0); + ck_assert_uint_eq(ts->sock.tcp.ack, original_ack); + ck_assert_uint_eq(ts->sock.tcp.seq, original_seq); +} +END_TEST + + /* ----------------------------------------------------------------------- */ diff --git a/src/wolfip.c b/src/wolfip.c index 3dfecfc8..19e7c2cc 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -3762,6 +3762,14 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, continue; } + /* RFC 9293 §3.10.7.4: if the SYN bit is set on a + * synchronized connection, send a challenge ACK and + * drop the segment (RFC 5961). */ + if (tcp->flags & TCP_FLAG_SYN) { + tcp_send_ack(t); + continue; + } + if (tcp->flags & TCP_FLAG_ACK) { tcp_ack(t, tcp); if (t->sock.tcp.state == TCP_CLOSED) From dee5bb31371af89cfb39ff1252d1feb6278fb706 Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Thu, 26 Mar 2026 13:45:39 +0100 Subject: [PATCH 02/10] send challenge ack, and drop segment for in window syn segments when on last_ack state --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_proto.c | 74 ++++++++++++++++++++++++++++++++ src/wolfip.c | 7 +++ 3 files changed, 82 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index ab1c2fe8..206dd010 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -670,6 +670,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_regression_icmp_payload_exceeds_buffer_discards_and_unblocks); tcase_add_test(tc_proto, test_regression_tcp_ip_len_below_ip_header); tcase_add_test(tc_proto, test_regression_syn_on_established_not_silently_processed); + tcase_add_test(tc_proto, test_regression_syn_on_last_ack_not_silently_processed); tcase_add_test(tc_utils, test_transport_checksum); tcase_add_test(tc_utils, test_iphdr_set_checksum); diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index c12f15a6..afe9d40a 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -3928,5 +3928,79 @@ START_TEST(test_regression_syn_on_established_not_silently_processed) } END_TEST +/* RFC 9293 §3.10.7.4: LAST-ACK is a synchronized state. A SYN arriving + * on a LAST_ACK socket must trigger a challenge ACK and be dropped, not + * silently ignored or processed as a normal ACK. */ +START_TEST(test_regression_syn_on_last_ack_not_silently_processed) +{ + struct wolfIP s; + struct tsocket *ts; + struct wolfIP_tcp_seg seg; + uint32_t original_ack; + uint32_t original_seq; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + last_frame_sent_size = 0; + + s.arp.neighbors[0].ip = 0x0A000002U; + s.arp.neighbors[0].if_idx = TEST_PRIMARY_IF; + memcpy(s.arp.neighbors[0].mac, + (uint8_t[]){0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF}, 6); + + ts = &s.tcpsockets[0]; + memset(ts, 0, sizeof(*ts)); + ts->proto = WI_IPPROTO_TCP; + ts->S = &s; + ts->sock.tcp.state = TCP_LAST_ACK; + ts->sock.tcp.ack = 100; + ts->sock.tcp.seq = 1000; + ts->sock.tcp.snd_una = 1000; + ts->sock.tcp.cwnd = TCP_MSS; + ts->sock.tcp.peer_rwnd = TCP_MSS; + ts->src_port = 1234; + ts->dst_port = 4321; + ts->local_ip = 0x0A000001U; + ts->remote_ip = 0x0A000002U; + ts->if_idx = TEST_PRIMARY_IF; + queue_init(&ts->sock.tcp.rxbuf, ts->rxmem, RXBUF_SIZE, ts->sock.tcp.ack); + fifo_init(&ts->sock.tcp.txbuf, ts->txmem, TXBUF_SIZE); + + original_ack = ts->sock.tcp.ack; + original_seq = ts->sock.tcp.seq; + + /* In-window SYN+ACK: without the fix tcp_ack() would process this + * as a normal acknowledgment in LAST_ACK. */ + memset(&seg, 0, sizeof(seg)); + seg.ip.ver_ihl = 0x45; + seg.ip.ttl = 64; + seg.ip.proto = WI_IPPROTO_TCP; + seg.ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN); + seg.ip.src = ee32(ts->remote_ip); + seg.ip.dst = ee32(ts->local_ip); + seg.dst_port = ee16(ts->src_port); + seg.src_port = ee16(ts->dst_port); + seg.hlen = TCP_HEADER_LEN << 2; + seg.flags = TCP_FLAG_SYN | TCP_FLAG_ACK; + seg.seq = ee32(100); + seg.ack = ee32(ts->sock.tcp.seq); + seg.win = ee16(65535); + fix_tcp_checksums(&seg); + + tcp_input(&s, TEST_PRIMARY_IF, &seg, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN)); + + (void)wolfIP_poll(&s, 200); + + /* The SYN must not be silently processed. A challenge ACK must be + * sent and the segment dropped without altering connection state. */ + ck_assert_uint_gt(last_frame_sent_size, 0); + ck_assert_uint_eq(ts->sock.tcp.state, TCP_LAST_ACK); + ck_assert_uint_eq(ts->sock.tcp.ack, original_ack); + ck_assert_uint_eq(ts->sock.tcp.seq, original_seq); +} +END_TEST + /* ----------------------------------------------------------------------- */ diff --git a/src/wolfip.c b/src/wolfip.c index 19e7c2cc..cb972121 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -3744,6 +3744,13 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, } } } else if (t->sock.tcp.state == TCP_LAST_ACK) { + /* RFC 9293 §3.10.7.4: if the SYN bit is set on a + * synchronized connection, send a challenge ACK and + * drop the segment (RFC 5961). */ + if (tcp->flags & TCP_FLAG_SYN) { + tcp_send_ack(t); + continue; + } if (tcp->flags & TCP_FLAG_ACK) { tcp_ack(t, tcp); /* tcp_ack may have closed the socket via close_socket(); From cc30b8556f1a5fa44b3d59a68112d16414abdcab Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Thu, 26 Mar 2026 14:36:35 +0100 Subject: [PATCH 03/10] =?UTF-8?q?Fix=20fast=20recovery=20to=20use=20Flight?= =?UTF-8?q?Size=20for=20ssthresh,=20inflate=20cwnd=20by=203*SMSS=20on=20en?= =?UTF-8?q?try=20and=20by=20SMSS=20on=20subsequent=20dup=20ACKs=20per=20RF?= =?UTF-8?q?C=205681=20=C2=A73.2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_proto.c | 120 ++++++++++++++++++++++++++++ src/test/unit/unit_tests_tcp_flow.c | 3 +- src/wolfip.c | 18 +++-- 4 files changed, 134 insertions(+), 8 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 206dd010..9d41ff14 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -671,6 +671,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_regression_tcp_ip_len_below_ip_header); tcase_add_test(tc_proto, test_regression_syn_on_established_not_silently_processed); tcase_add_test(tc_proto, test_regression_syn_on_last_ack_not_silently_processed); + tcase_add_test(tc_proto, test_regression_fast_recovery_cwnd_ssthresh_rfc5681); tcase_add_test(tc_utils, test_transport_checksum); tcase_add_test(tc_utils, test_iphdr_set_checksum); diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index afe9d40a..35c53238 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -4003,4 +4003,124 @@ START_TEST(test_regression_syn_on_last_ack_not_silently_processed) END_TEST +/* RFC 5681 §3.2: fast recovery deviates in multiple ways. + * (a) ssthresh uses cwnd/2 instead of max(FlightSize/2, 2*SMSS) + * (b) cwnd set to ssthresh + 1*SMSS instead of ssthresh + 3*SMSS + * (c) no cwnd inflation by SMSS for each dup ACK beyond the 3rd + * (d) no cwnd deflation to ssthresh on new-data ACK exiting recovery */ +START_TEST(test_regression_fast_recovery_cwnd_ssthresh_rfc5681) +{ + struct wolfIP s; + struct tsocket *ts; + struct wolfIP_tcp_seg seg; + uint32_t smss; + uint32_t flight_size; + uint32_t expected_ssthresh; + uint32_t expected_cwnd; + uint32_t cwnd_after_3rd; + int i; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + ts = &s.tcpsockets[0]; + memset(ts, 0, sizeof(*ts)); + ts->proto = WI_IPPROTO_TCP; + ts->S = &s; + ts->if_idx = TEST_PRIMARY_IF; + ts->sock.tcp.state = TCP_ESTABLISHED; + ts->src_port = 1234; + ts->dst_port = 4321; + ts->local_ip = 0x0A000001U; + ts->remote_ip = 0x0A000002U; + + smss = tcp_cc_mss(ts); + ck_assert_uint_gt(smss, 0); + + flight_size = 4 * smss; + + ts->sock.tcp.snd_una = 1000; + ts->sock.tcp.ack = 500; + ts->sock.tcp.cwnd = 10 * smss; /* App-limited: cwnd >> FlightSize */ + ts->sock.tcp.ssthresh = 20 * smss; + ts->sock.tcp.peer_rwnd = 20 * smss; + ts->sock.tcp.bytes_in_flight = flight_size; + + queue_init(&ts->sock.tcp.rxbuf, ts->rxmem, RXBUF_SIZE, ts->sock.tcp.ack); + fifo_init(&ts->sock.tcp.txbuf, ts->txmem, TXBUF_SIZE); + + /* Enqueue a sent segment at seq=snd_una so the retransmit path has + * something to work with (payload=8 bytes). */ + { + uint32_t saved_seq = ts->sock.tcp.seq; + struct pkt_desc *desc; + ts->sock.tcp.seq = ts->sock.tcp.snd_una; + enqueue_tcp_tx(ts, 8, TCP_FLAG_ACK | TCP_FLAG_PSH); + desc = fifo_peek(&ts->sock.tcp.txbuf); + ck_assert_ptr_nonnull(desc); + desc->flags |= PKT_FLAG_SENT; + ts->sock.tcp.seq = 1000 + flight_size; + (void)saved_seq; + } + + /* --- Phase 1: send 3 duplicate ACKs to enter fast recovery --- */ + for (i = 0; i < 3; i++) { + memset(&seg, 0, sizeof(seg)); + seg.ip.ver_ihl = 0x45; + seg.ip.ttl = 64; + seg.ip.proto = WI_IPPROTO_TCP; + seg.ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN); + seg.ip.src = ee32(ts->remote_ip); + seg.ip.dst = ee32(ts->local_ip); + seg.dst_port = ee16(ts->src_port); + seg.src_port = ee16(ts->dst_port); + seg.hlen = TCP_HEADER_LEN << 2; + seg.flags = TCP_FLAG_ACK; + seg.seq = ee32(ts->sock.tcp.ack); + seg.ack = ee32(1000); /* == snd_una, no advance */ + seg.win = ee16(65535); + fix_tcp_checksums(&seg); + + tcp_input(&s, TEST_PRIMARY_IF, &seg, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN)); + } + + /* (a) ssthresh = max(FlightSize/2, 2*SMSS) = max(2*SMSS, 2*SMSS) = 2*SMSS + * Bug: max(cwnd/2, 2*SMSS) = max(5*SMSS, 2*SMSS) = 5*SMSS + * (b) cwnd = ssthresh + 3*SMSS = 5*SMSS + * Bug: ssthresh + 1*SMSS = 6*SMSS */ + expected_ssthresh = 2 * smss; + expected_cwnd = expected_ssthresh + 3 * smss; + ck_assert_uint_eq(ts->sock.tcp.ssthresh, expected_ssthresh); + ck_assert_uint_eq(ts->sock.tcp.cwnd, expected_cwnd); + + /* --- Phase 2: 4th dup ACK should inflate cwnd by SMSS (c) --- */ + cwnd_after_3rd = ts->sock.tcp.cwnd; + + memset(&seg, 0, sizeof(seg)); + seg.ip.ver_ihl = 0x45; + seg.ip.ttl = 64; + seg.ip.proto = WI_IPPROTO_TCP; + seg.ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN); + seg.ip.src = ee32(ts->remote_ip); + seg.ip.dst = ee32(ts->local_ip); + seg.dst_port = ee16(ts->src_port); + seg.src_port = ee16(ts->dst_port); + seg.hlen = TCP_HEADER_LEN << 2; + seg.flags = TCP_FLAG_ACK; + seg.seq = ee32(ts->sock.tcp.ack); + seg.ack = ee32(1000); + seg.win = ee16(65535); + fix_tcp_checksums(&seg); + + tcp_input(&s, TEST_PRIMARY_IF, &seg, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN)); + + /* (c) cwnd should be inflated by exactly SMSS, not recomputed */ + ck_assert_uint_eq(ts->sock.tcp.cwnd, cwnd_after_3rd + smss); +} +END_TEST + + /* ----------------------------------------------------------------------- */ diff --git a/src/test/unit/unit_tests_tcp_flow.c b/src/test/unit/unit_tests_tcp_flow.c index 4ac8f956..01f4f1ff 100644 --- a/src/test/unit/unit_tests_tcp_flow.c +++ b/src/test/unit/unit_tests_tcp_flow.c @@ -1739,7 +1739,8 @@ START_TEST(test_tcp_ack_duplicate_zero_len_segment_large_ack) tcp_ack(ts, &ackseg); ck_assert_uint_le(fifo_len(&ts->sock.tcp.txbuf), TXBUF_SIZE); ck_assert_uint_eq(ts->sock.tcp.ssthresh, TCP_MSS * 2); - ck_assert_uint_eq(ts->sock.tcp.cwnd, TCP_MSS * 3); + /* RFC 5681 §3.2: cwnd = ssthresh + 3*SMSS on entering fast recovery */ + ck_assert_uint_eq(ts->sock.tcp.cwnd, TCP_MSS * 5); } END_TEST diff --git a/src/wolfip.c b/src/wolfip.c index cb972121..5c1586ed 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -3495,16 +3495,20 @@ static void tcp_ack(struct tsocket *t, const struct wolfIP_tcp_seg *tcp) } if (t->sock.tcp.dup_acks < 3) return; - { + if (t->sock.tcp.dup_acks == 3) { + /* RFC 5681 §3.2 step 2-3: enter fast recovery */ uint32_t smss = tcp_cc_mss(t); - t->sock.tcp.ssthresh = t->sock.tcp.cwnd / 2; - if (t->sock.tcp.ssthresh < 2 * smss) { + t->sock.tcp.ssthresh = inflight_pre / 2; + if (t->sock.tcp.ssthresh < 2 * smss) t->sock.tcp.ssthresh = 2 * smss; - } - t->sock.tcp.cwnd = t->sock.tcp.ssthresh + smss; + t->sock.tcp.cwnd = t->sock.tcp.ssthresh + 3 * smss; + t->sock.tcp.cwnd_count = 0; + (void)tcp_mark_unsacked_for_retransmit(t, ack); + } else { + /* RFC 5681 §3.2 step 4: inflate cwnd by SMSS for each + * additional duplicate ACK during fast recovery */ + t->sock.tcp.cwnd += tcp_cc_mss(t); } - t->sock.tcp.cwnd_count = 0; - (void)tcp_mark_unsacked_for_retransmit(t, ack); } } From db8380bc55bdda7169ae320a45d3e418efa879c6 Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Thu, 26 Mar 2026 14:47:09 +0100 Subject: [PATCH 04/10] paws check after timestamp implementation, send ack and discard --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_proto.c | 92 ++++++++++++++++++++++++++++++++ src/wolfip.c | 13 +++++ 3 files changed, 106 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 9d41ff14..3e799c8d 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -672,6 +672,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_regression_syn_on_established_not_silently_processed); tcase_add_test(tc_proto, test_regression_syn_on_last_ack_not_silently_processed); tcase_add_test(tc_proto, test_regression_fast_recovery_cwnd_ssthresh_rfc5681); + tcase_add_test(tc_proto, test_regression_paws_rejects_stale_timestamp); tcase_add_test(tc_utils, test_transport_checksum); tcase_add_test(tc_utils, test_iphdr_set_checksum); diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index 35c53238..0b50d20a 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -4123,4 +4123,96 @@ START_TEST(test_regression_fast_recovery_cwnd_ssthresh_rfc5681) END_TEST +/* RFC 7323 §3.2: when timestamps are negotiated, segments with + * SEG.TSval < TS.Recent (stale timestamp) must be rejected (send ACK + * and discard) to protect against wrapped sequence numbers (PAWS). + * The current code accepts such segments, risking data corruption on + * long-lived high-throughput connections. */ +START_TEST(test_regression_paws_rejects_stale_timestamp) +{ + struct wolfIP s; + struct tsocket *ts; + uint8_t buf[sizeof(struct wolfIP_tcp_seg) + TCP_OPTIONS_LEN + 4]; + struct wolfIP_tcp_seg *seg = (struct wolfIP_tcp_seg *)buf; + struct tcp_opt_ts *tsopt; + uint8_t payload[4] = {0xDE, 0xAD, 0xBE, 0xEF}; + uint32_t original_ack; + uint32_t tcp_hlen = TCP_HEADER_LEN + TCP_OPTIONS_LEN; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + last_frame_sent_size = 0; + + s.arp.neighbors[0].ip = 0x0A000002U; + s.arp.neighbors[0].if_idx = TEST_PRIMARY_IF; + memcpy(s.arp.neighbors[0].mac, + (uint8_t[]){0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF}, 6); + + ts = &s.tcpsockets[0]; + memset(ts, 0, sizeof(*ts)); + ts->proto = WI_IPPROTO_TCP; + ts->S = &s; + ts->if_idx = TEST_PRIMARY_IF; + ts->sock.tcp.state = TCP_ESTABLISHED; + ts->sock.tcp.ack = 100; + ts->sock.tcp.seq = 1000; + ts->sock.tcp.snd_una = 1000; + ts->sock.tcp.cwnd = TCP_MSS; + ts->sock.tcp.peer_rwnd = TCP_MSS; + ts->sock.tcp.ts_enabled = 1; + ts->sock.tcp.last_ts = ee32(5000); /* TS.Recent = 5000 */ + ts->src_port = 1234; + ts->dst_port = 4321; + ts->local_ip = 0x0A000001U; + ts->remote_ip = 0x0A000002U; + queue_init(&ts->sock.tcp.rxbuf, ts->rxmem, RXBUF_SIZE, ts->sock.tcp.ack); + fifo_init(&ts->sock.tcp.txbuf, ts->txmem, TXBUF_SIZE); + + original_ack = ts->sock.tcp.ack; + + /* Craft an in-window data segment with a stale timestamp (TSval=1000, + * which is less than TS.Recent=5000). This simulates an old duplicate + * with a wrapped sequence number. */ + memset(buf, 0, sizeof(buf)); + seg->ip.ver_ihl = 0x45; + seg->ip.ttl = 64; + seg->ip.proto = WI_IPPROTO_TCP; + seg->ip.len = ee16(IP_HEADER_LEN + tcp_hlen + sizeof(payload)); + seg->ip.src = ee32(ts->remote_ip); + seg->ip.dst = ee32(ts->local_ip); + seg->dst_port = ee16(ts->src_port); + seg->src_port = ee16(ts->dst_port); + seg->hlen = (uint8_t)(tcp_hlen << 2); + seg->flags = TCP_FLAG_ACK; + seg->seq = ee32(100); /* == rcv_nxt, in-window */ + seg->ack = ee32(ts->sock.tcp.seq); + seg->win = ee16(65535); + + /* Append timestamp option with stale TSval */ + tsopt = (struct tcp_opt_ts *)seg->data; + tsopt->opt = TCP_OPTION_TS; + tsopt->len = TCP_OPTION_TS_LEN; + tsopt->val = ee32(1000); /* SEG.TSval = 1000 < TS.Recent = 5000 */ + tsopt->ecr = ee32(500); + tsopt->pad = TCP_OPTION_NOP; + tsopt->eoo = TCP_OPTION_NOP; + + memcpy(seg->data + TCP_OPTIONS_LEN, payload, sizeof(payload)); + fix_tcp_checksums(seg); + + tcp_input(&s, TEST_PRIMARY_IF, seg, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + tcp_hlen + sizeof(payload))); + + (void)wolfIP_poll(&s, 200); + + /* PAWS: the stale-timestamp segment must be discarded without + * advancing rcv_nxt (no data accepted into rxbuf). An ACK should + * be sent in response. */ + ck_assert_uint_eq(ts->sock.tcp.ack, original_ack); + ck_assert_uint_gt(last_frame_sent_size, 0); +} +END_TEST + + /* ----------------------------------------------------------------------- */ diff --git a/src/wolfip.c b/src/wolfip.c index 5c1586ed..1f38253a 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -3773,6 +3773,19 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, continue; } + /* RFC 7323 §3.2 PAWS: if timestamps were negotiated, + * reject segments with stale TSval (send ACK, drop). */ + if (t->sock.tcp.ts_enabled && + !(tcp->flags & TCP_FLAG_RST)) { + struct tcp_parsed_opts po; + tcp_parse_options(tcp, frame_len, &po); + if (po.ts_found && + po.ts_val < ee32(t->sock.tcp.last_ts)) { + tcp_send_ack(t); + continue; + } + } + /* RFC 9293 §3.10.7.4: if the SYN bit is set on a * synchronized connection, send a challenge ACK and * drop the segment (RFC 5961). */ From 0e5d1c1013fdb5718b8d0278c4d16e748df622ed Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Thu, 26 Mar 2026 15:13:46 +0100 Subject: [PATCH 05/10] update anti-replay window in the essp code after icv verification of a packet --- src/test/unit/unit_esp.c | 49 ++++++++++++++++++++++++++++++++++++---- src/wolfesp.c | 45 ++++++++++++++++++++++++------------ 2 files changed, 76 insertions(+), 18 deletions(-) diff --git a/src/test/unit/unit_esp.c b/src/test/unit/unit_esp.c index 40f2c121..e05badec 100644 --- a/src/test/unit/unit_esp.c +++ b/src/test/unit/unit_esp.c @@ -424,8 +424,9 @@ START_TEST(test_replay_duplicate_rejected) { replay_t r; esp_replay_init(r); - ck_assert_int_eq(esp_check_replay(&r, 5U), 0); /* first time: ok */ - ck_assert_int_ne(esp_check_replay(&r, 5U), 0); /* second time: replayed */ + ck_assert_int_eq(esp_check_replay(&r, 5U), 0); + esp_replay_commit(&r, 5U); /* ICV passed */ + ck_assert_int_ne(esp_check_replay(&r, 5U), 0); /* second time: replayed */ } END_TEST @@ -437,6 +438,7 @@ START_TEST(test_replay_multiple_in_window) esp_replay_init(r); /* window [1..32] */ for (i = 1U; i <= 31U; i++) { ck_assert_int_eq(esp_check_replay(&r, i), 0); + esp_replay_commit(&r, i); } } END_TEST @@ -447,7 +449,8 @@ START_TEST(test_replay_below_window_rejected) replay_t r; esp_replay_init(r); /* Advance the window by receiving a high sequence number. */ - ck_assert_int_eq(esp_check_replay(&r, 64U), 0); /* hi_seq=64, seq_low=34 */ + ck_assert_int_eq(esp_check_replay(&r, 64U), 0); + esp_replay_commit(&r, 64U); /* hi_seq=64, seq_low=34 */ /* seq=1 is now below the window floor. */ ck_assert_int_ne(esp_check_replay(&r, 1U), 0); } @@ -459,6 +462,7 @@ START_TEST(test_replay_advance_hi_seq) replay_t r; esp_replay_init(r); /* hi_seq=32 */ ck_assert_int_eq(esp_check_replay(&r, 33U), 0); + esp_replay_commit(&r, 33U); ck_assert_uint_eq(r.hi_seq, 33U); } END_TEST @@ -469,6 +473,7 @@ START_TEST(test_replay_advanced_hi_seq_duplicate_rejected) replay_t r; esp_replay_init(r); /* hi_seq=32 */ ck_assert_int_eq(esp_check_replay(&r, 33U), 0); + esp_replay_commit(&r, 33U); ck_assert_int_ne(esp_check_replay(&r, 33U), 0); } END_TEST @@ -491,9 +496,12 @@ START_TEST(test_replay_jump_resets_bitmap) esp_replay_init(r); /* Accept some sequences so the bitmap has bits set. */ ck_assert_int_eq(esp_check_replay(&r, 1U), 0); + esp_replay_commit(&r, 1U); ck_assert_int_eq(esp_check_replay(&r, 2U), 0); + esp_replay_commit(&r, 2U); /* Jump more than ESP_REPLAY_WIN (32) ahead. */ ck_assert_int_eq(esp_check_replay(&r, 1000U), 0); + esp_replay_commit(&r, 1000U); ck_assert_uint_eq(r.hi_seq, 1000U); /* seq=1 is now far outside the window. */ ck_assert_int_ne(esp_check_replay(&r, 1U), 0); @@ -507,12 +515,44 @@ START_TEST(test_replay_old_seqs_after_jump) replay_t r; esp_replay_init(r); ck_assert_int_eq(esp_check_replay(&r, 10U), 0); - ck_assert_int_eq(esp_check_replay(&r, 500U), 0); /* jump > 32 */ + esp_replay_commit(&r, 10U); + ck_assert_int_eq(esp_check_replay(&r, 500U), 0); + esp_replay_commit(&r, 500U); /* jump > 32 */ /* 10 is now well below the new window floor (500-31=469). */ ck_assert_int_ne(esp_check_replay(&r, 10U), 0); } END_TEST +/* RFC 4303 s3.4.3: the replay window must not be updated until after + * ICV verification succeeds. esp_check_replay must be read-only; + * esp_replay_commit updates the window after ICV passes. */ +START_TEST(test_regression_replay_window_not_updated_before_icv) +{ + replay_t r; + replay_t saved; + + esp_replay_init(r); + + /* Accept a few packets to establish window state */ + ck_assert_int_eq(esp_check_replay(&r, 1U), 0); + esp_replay_commit(&r, 1U); + ck_assert_int_eq(esp_check_replay(&r, 2U), 0); + esp_replay_commit(&r, 2U); + + /* Save the replay state before the "unverified" packet arrives */ + memcpy(&saved, &r, sizeof(r)); + + /* Simulate receiving seq=10. This should only CHECK, not UPDATE. + * In the real flow, ICV verification would follow and might fail. */ + ck_assert_int_eq(esp_check_replay(&r, 10U), 0); + + /* esp_check_replay is now read-only (correct behavior), so the + * replay state must be unchanged. */ + ck_assert_uint_eq(r.bitmap, saved.bitmap); + ck_assert_uint_eq(r.hi_seq, saved.hi_seq); +} +END_TEST + /* The transmitted sequence number must never be allowed to overflow. */ START_TEST(test_replay_overflow) { @@ -1273,6 +1313,7 @@ static Suite *esp_suite(void) tcase_add_test(tc, test_replay_jump_resets_bitmap); tcase_add_test(tc, test_replay_old_seqs_after_jump); tcase_add_test(tc, test_replay_overflow); + tcase_add_test(tc, test_regression_replay_window_not_updated_before_icv); suite_add_tcase(s, tc); /* Unwrap error paths */ diff --git a/src/wolfesp.c b/src/wolfesp.c index e277105d..4df4b7ea 100644 --- a/src/wolfesp.c +++ b/src/wolfesp.c @@ -1159,19 +1159,20 @@ esp_check_icv_hmac(const wolfIP_esp_sa * esp_sa, uint8_t * esp_data, } /** - * Check sequence number against replay_t state. + * Check sequence number against replay_t state (read-only). + * RFC 4303 s3.4.3: the window must not be updated until after ICV + * verification succeeds. Call esp_replay_commit() after ICV passes. * * return 0 on success. * */ static int -esp_check_replay(struct replay_t * replay, uint32_t seq) +esp_check_replay(const struct replay_t * replay, uint32_t seq) { #if !defined(ESP_REPLAY_WIN) /* anti-replay service not enabled */ (void)replay; (void)seq; #else - uint32_t diff = 0; uint32_t bitn = 0; uint32_t seq_low = 1U; @@ -1201,29 +1202,41 @@ esp_check_replay(struct replay_t * replay, uint32_t seq) ESP_LOG("error: seq replayed: %u, %d\n", bitn, seq); return -1; } - else { - ESP_DEBUG("info: new seq : %d\n", seq); - replay->bitmap |= bitn; - } + } + #endif /* ESP_REPLAY_WIN */ + + return 0; +} + +/** + * Commit a verified sequence number to the replay window. + * Called only after ICV verification succeeds (RFC 4303 s3.4.3). + * */ +static void +esp_replay_commit(struct replay_t * replay, uint32_t seq) +{ + #if !defined(ESP_REPLAY_WIN) + (void)replay; + (void)seq; + #else + uint32_t diff; + + if (seq <= replay->hi_seq) { + /* Within window: mark the bit. */ + replay->bitmap |= 1U << (replay->hi_seq - seq); } else { - /* seq number above window. */ - ESP_DEBUG("info: new hi_seq : %d, %d\n", replay->hi_seq, seq); + /* Above window: slide up. */ diff = seq - replay->hi_seq; if (diff < ESP_REPLAY_WIN) { - /* within a window width, slide up. */ replay->bitmap = (replay->bitmap << diff) | 1U; } else { - /* reset window. */ replay->bitmap = 1; } - replay->hi_seq = seq; } #endif /* ESP_REPLAY_WIN */ - - return 0; } /** @@ -1349,6 +1362,10 @@ esp_transport_unwrap(struct wolfIP_ip_packet *ip, uint32_t * frame_len) } } + /* ICV verified; now safe to commit the sequence to the replay window + * (RFC 4303 s3.4.3). */ + esp_replay_commit(&esp_sa->replay, seq); + /* icv check good, now finish unwrapping esp packet. */ if (iv_len != 0) { /* Decrypt the payload in place. */ From 69746df6933c038c869e018b60fa9c6cda5a6fc8 Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Thu, 26 Mar 2026 15:32:11 +0100 Subject: [PATCH 06/10] - handle dhcpnak and to force the client to restart the dhcp process - added dhcp_msg_type to parse dhcp's message type --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_proto.c | 50 +++++++++++++++++++++++ src/wolfip.c | 70 ++++++++++++++++++++++++++------ 3 files changed, 109 insertions(+), 12 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 3e799c8d..520976f0 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -673,6 +673,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_regression_syn_on_last_ack_not_silently_processed); tcase_add_test(tc_proto, test_regression_fast_recovery_cwnd_ssthresh_rfc5681); tcase_add_test(tc_proto, test_regression_paws_rejects_stale_timestamp); + tcase_add_test(tc_proto, test_regression_dhcp_nak_restarts_configuration); tcase_add_test(tc_utils, test_transport_checksum); tcase_add_test(tc_utils, test_iphdr_set_checksum); diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index 0b50d20a..88e2c35b 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -4215,4 +4215,54 @@ START_TEST(test_regression_paws_rejects_stale_timestamp) END_TEST +/* RFC 2131 s4.4.1: if the client receives a DHCPNAK, it must restart + * the configuration process. The current code silently ignores NAKs + * during RENEWING/REBINDING because dhcp_parse_ack returns -1 and + * dhcp_poll treats that as a no-op. */ +START_TEST(test_regression_dhcp_nak_restarts_configuration) +{ + struct wolfIP s; + struct dhcp_msg msg; + struct dhcp_option *opt; + struct tsocket *ts; + struct ipconf *primary; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000064U, 0xFFFFFF00U, 0); + + s.dhcp_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_gt(s.dhcp_udp_sd, 0); + ts = &s.udpsockets[SOCKET_UNMARK(s.dhcp_udp_sd)]; + + /* Simulate a RENEWING client that receives a DHCPNAK */ + s.dhcp_state = DHCP_RENEWING; + s.dhcp_xid = 0x12345678U; + primary = wolfIP_primary_ipconf(&s); + ck_assert_ptr_nonnull(primary); + + /* Build a minimal DHCPNAK message (type 6) */ + memset(&msg, 0, sizeof(msg)); + msg.op = 2; /* BOOT_REPLY */ + msg.magic = ee32(DHCP_MAGIC); + msg.xid = ee32(0x12345678U); + opt = (struct dhcp_option *)msg.options; + opt->code = DHCP_OPTION_MSG_TYPE; + opt->len = 1; + opt->data[0] = 6; /* DHCPNAK */ + opt = (struct dhcp_option *)((uint8_t *)opt + 3); + opt->code = DHCP_OPTION_END; + + enqueue_udp_rx(ts, &msg, sizeof(msg), DHCP_SERVER_PORT); + (void)dhcp_poll(&s); + + /* After receiving NAK, the client must not remain in RENEWING. + * It should restart discovery (transition to DHCP_OFF or + * DHCP_DISCOVER_SENT). */ + ck_assert_int_ne(s.dhcp_state, DHCP_RENEWING); + ck_assert_int_ne(s.dhcp_state, DHCP_BOUND); +} +END_TEST + + /* ----------------------------------------------------------------------- */ diff --git a/src/wolfip.c b/src/wolfip.c index 1f38253a..415fed08 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -976,6 +976,7 @@ static int wolfIP_filter_notify_icmp(enum wolfIP_filter_reason reason, #define DHCP_OFFER 2 #define DHCP_REQUEST 3 #define DHCP_ACK 5 +#define DHCP_NAK 6 #define DHCP_MAGIC 0x63825363 #define DHCP_SERVER_PORT 67 @@ -5387,6 +5388,42 @@ static int dhcp_parse_offer(struct wolfIP *s, struct dhcp_msg *msg, uint32_t msg } +/* Return the DHCP message type from a validated message, or -1 on error. */ +static int dhcp_msg_type(struct wolfIP *s, struct dhcp_msg *msg, uint32_t msg_len) +{ + uint8_t *opt = (uint8_t *)msg->options; + uint8_t *opt_end; + if (msg_len < DHCP_HEADER_LEN) + return -1; + if (ee32(msg->magic) != DHCP_MAGIC) + return -1; + if (ee32(msg->xid) != s->dhcp_xid) + return -1; + if (msg_len - DHCP_HEADER_LEN > sizeof(msg->options)) + opt_end = (uint8_t *)msg->options + sizeof(msg->options); + else + opt_end = (uint8_t *)msg->options + (msg_len - DHCP_HEADER_LEN); + while (opt < opt_end) { + uint8_t code = opt[0]; + uint8_t len; + if (code == DHCP_OPTION_END) + break; + if (code == 0) { + opt++; + continue; + } + if (opt + 2 > opt_end) + break; + len = opt[1]; + if (opt + 2 + len > opt_end) + break; + if (code == DHCP_OPTION_MSG_TYPE && len == 1) + return opt[2]; + opt += 2 + len; + } + return -1; +} + static int dhcp_parse_ack(struct wolfIP *s, struct dhcp_msg *msg, uint32_t msg_len) { uint8_t *opt = (uint8_t *)msg->options; @@ -5520,19 +5557,28 @@ static int dhcp_poll(struct wolfIP *s) return -1; if ((s->dhcp_state == DHCP_DISCOVER_SENT) && (dhcp_parse_offer(s, &msg, (uint32_t)len) == 0)) dhcp_send_request(s); - else if ((s->dhcp_state == DHCP_REQUEST_SENT || + else if (s->dhcp_state == DHCP_REQUEST_SENT || s->dhcp_state == DHCP_RENEWING || - s->dhcp_state == DHCP_REBINDING) && - (dhcp_parse_ack(s, &msg, (uint32_t)len) == 0)) { - struct ipconf *primary = wolfIP_primary_ipconf(s); - LOG("DHCP configuration received.\n"); - if (primary) { - LOG("IP Address: %u.%u.%u.%u\n", (unsigned int)((primary->ip >> 24) & 0xFF), (unsigned int)((primary->ip >> 16) & 0xFF), (unsigned int)((primary->ip >> 8) & 0xFF), (unsigned int)((primary->ip >> 0) & 0xFF)); - LOG("Subnet Mask: %u.%u.%u.%u\n", (unsigned int)((primary->mask >> 24) & 0xFF), (unsigned int)((primary->mask >> 16) & 0xFF), (unsigned int)((primary->mask >> 8) & 0xFF), (unsigned int)((primary->mask >> 0) & 0xFF)); - LOG("Gateway: %u.%u.%u.%u\n", (unsigned int)((primary->gw >> 24) & 0xFF), (unsigned int)((primary->gw >> 16) & 0xFF), (unsigned int)((primary->gw >> 8) & 0xFF), (unsigned int)((primary->gw >> 0) & 0xFF)); - } - if (s->dns_server) - LOG("DNS Server: %u.%u.%u.%u\n", (unsigned int)((s->dns_server >> 24) & 0xFF), (unsigned int)((s->dns_server >> 16) & 0xFF), (unsigned int)((s->dns_server >> 8) & 0xFF), (unsigned int)((s->dns_server >> 0) & 0xFF)); + s->dhcp_state == DHCP_REBINDING) { + /* RFC 2131 s4.4.1: if the client receives a DHCPNAK, + * it must restart the configuration process. */ + if (dhcp_msg_type(s, &msg, (uint32_t)len) == DHCP_NAK) { + dhcp_cancel_timer(s); + s->dhcp_state = DHCP_OFF; + dhcp_send_discover(s); + return 0; + } + if (dhcp_parse_ack(s, &msg, (uint32_t)len) == 0) { + struct ipconf *primary = wolfIP_primary_ipconf(s); + LOG("DHCP configuration received.\n"); + if (primary) { + LOG("IP Address: %u.%u.%u.%u\n", (unsigned int)((primary->ip >> 24) & 0xFF), (unsigned int)((primary->ip >> 16) & 0xFF), (unsigned int)((primary->ip >> 8) & 0xFF), (unsigned int)((primary->ip >> 0) & 0xFF)); + LOG("Subnet Mask: %u.%u.%u.%u\n", (unsigned int)((primary->mask >> 24) & 0xFF), (unsigned int)((primary->mask >> 16) & 0xFF), (unsigned int)((primary->mask >> 8) & 0xFF), (unsigned int)((primary->mask >> 0) & 0xFF)); + LOG("Gateway: %u.%u.%u.%u\n", (unsigned int)((primary->gw >> 24) & 0xFF), (unsigned int)((primary->gw >> 16) & 0xFF), (unsigned int)((primary->gw >> 8) & 0xFF), (unsigned int)((primary->gw >> 0) & 0xFF)); + } + if (s->dns_server) + LOG("DNS Server: %u.%u.%u.%u\n", (unsigned int)((s->dns_server >> 24) & 0xFF), (unsigned int)((s->dns_server >> 16) & 0xFF), (unsigned int)((s->dns_server >> 8) & 0xFF), (unsigned int)((s->dns_server >> 0) & 0xFF)); + } } return 0; } From d66520191ca4bfc36c2fefa4abe4e103e1f0f5cf Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Thu, 26 Mar 2026 15:59:33 +0100 Subject: [PATCH 07/10] dhcp: extract the RCODE that was sent and abort query if non-zero and notify the caller instaed of waiting for the timer to expire --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_proto.c | 52 ++++++++++++++++++++++++++++++++ src/wolfip.c | 15 +++++++-- 3 files changed, 65 insertions(+), 3 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 520976f0..cfe1f2ff 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -674,6 +674,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_regression_fast_recovery_cwnd_ssthresh_rfc5681); tcase_add_test(tc_proto, test_regression_paws_rejects_stale_timestamp); tcase_add_test(tc_proto, test_regression_dhcp_nak_restarts_configuration); + tcase_add_test(tc_proto, test_regression_dns_rcode_error_aborts_query); tcase_add_test(tc_utils, test_transport_checksum); tcase_add_test(tc_utils, test_iphdr_set_checksum); diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index 88e2c35b..93158974 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -4264,5 +4264,57 @@ START_TEST(test_regression_dhcp_nak_restarts_configuration) } END_TEST +/* DNS response parser does not check the RCODE field. An error response + * such as NXDOMAIN (RCODE=3) passes the QR+RD check, the empty answer + * section is silently skipped, and the query stays active until the + * retry timer fires. RFC 1035 s4.1.1: RCODE != 0 is an error. */ +START_TEST(test_regression_dns_rcode_error_aborts_query) +{ + struct wolfIP s; + struct tsocket *ts; + uint8_t response[64]; + struct dns_header *hdr = (struct dns_header *)response; + struct dns_question *q; + int pos; + + wolfIP_init(&s); + mock_link_init(&s); + s.dns_server = 0x08080808U; + + /* Set up an active DNS query */ + s.dns_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_gt(s.dns_udp_sd, 0); + ts = &s.udpsockets[SOCKET_UNMARK(s.dns_udp_sd)]; + s.dns_id = 0xABCD; + s.dns_query_type = DNS_QUERY_TYPE_A; + s.dns_lookup_cb = test_dns_lookup_cb; + + /* Build a DNS response with RCODE=3 (NXDOMAIN). + * flags = 0x8183: QR=1, RD=1, RA=1, RCODE=3 + * ancount=0 (no answers, as expected for NXDOMAIN). */ + memset(response, 0, sizeof(response)); + hdr->id = ee16(0xABCD); + hdr->flags = ee16(0x8183); + hdr->qdcount = ee16(1); + hdr->ancount = ee16(0); + pos = sizeof(struct dns_header); + response[pos++] = 3; memcpy(&response[pos], "foo", 3); pos += 3; + response[pos++] = 3; memcpy(&response[pos], "com", 3); pos += 3; + response[pos++] = 0; + q = (struct dns_question *)(response + pos); + q->qtype = ee16(DNS_A); + q->qclass = ee16(1); + pos += sizeof(struct dns_question); + + enqueue_udp_rx(ts, response, (uint16_t)pos, DNS_PORT); + dns_callback(s.dns_udp_sd, CB_EVENT_READABLE, &s); + + /* The query must be aborted after receiving an authoritative error, + * not left active for the retry timer to fire. */ + ck_assert_uint_eq(s.dns_id, 0); + ck_assert_int_eq(s.dns_query_type, DNS_QUERY_TYPE_NONE); +} +END_TEST + /* ----------------------------------------------------------------------- */ diff --git a/src/wolfip.c b/src/wolfip.c index 415fed08..b2f2bcf4 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -6637,6 +6637,10 @@ void dns_callback(int dns_sd, uint16_t ev, void *arg) char buf[MAX_DNS_RESPONSE]; struct dns_header *hdr = (struct dns_header *)buf; int dns_len; + int pos; + int qcount; + int ancount; + if (!s) return; if (ev & CB_EVENT_READABLE) { @@ -6653,9 +6657,14 @@ void dns_callback(int dns_sd, uint16_t ev, void *arg) return; /* Parse DNS response */ if ((ee16(hdr->flags) & 0x8100) == 0x8100) { - int pos = sizeof(struct dns_header); - int qcount = ee16(hdr->qdcount); - int ancount = ee16(hdr->ancount); + /* RFC 1035 s4.1.1: RCODE != 0 is an error; abort query. */ + if ((ee16(hdr->flags) & 0x000F) != 0) { + dns_abort_query(s); + return; + } + pos = sizeof(struct dns_header); + qcount = ee16(hdr->qdcount); + ancount = ee16(hdr->ancount); while (qcount-- > 0) { pos = dns_skip_name((const uint8_t *)buf, dns_len, pos); if (pos < 0 || pos + (int)sizeof(struct dns_question) > dns_len) { From 88d73b73c7042eab34a0efdf483a640a44f15af8 Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Thu, 26 Mar 2026 16:17:35 +0100 Subject: [PATCH 08/10] replace zero UDP checksum with 0xFFF to avoid receivers interpretingt it as "no checksum was computer" --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_proto.c | 48 ++++++++++++++++++++++++++++++++ src/wolfip.c | 6 +++- 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index cfe1f2ff..1bf8d823 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -675,6 +675,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_regression_paws_rejects_stale_timestamp); tcase_add_test(tc_proto, test_regression_dhcp_nak_restarts_configuration); tcase_add_test(tc_proto, test_regression_dns_rcode_error_aborts_query); + tcase_add_test(tc_proto, test_regression_udp_checksum_zero_substituted_with_ffff); tcase_add_test(tc_utils, test_transport_checksum); tcase_add_test(tc_utils, test_iphdr_set_checksum); diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index 93158974..966ae601 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -4317,4 +4317,52 @@ START_TEST(test_regression_dns_rcode_error_aborts_query) END_TEST +/* RFC 768: if the computed UDP checksum is zero, it must be transmitted + * as 0xFFFF. A zero checksum means "no checksum computed" and the + * receiver would skip verification. The current code stores the raw + * transport_checksum result without zero-substitution. */ +START_TEST(test_regression_udp_checksum_zero_substituted_with_ffff) +{ + struct wolfIP s; + struct tsocket *ts; + struct wolfIP_udp_datagram udp; + + /* Craft a UDP datagram whose pseudo-header + data sums to 0xFFFF + * in one's complement, causing transport_checksum to return 0. + * + * Pseudo-header: src=0, dst=0, proto=0x11, len=8 + * sum = 0x0011 + 0x0008 = 0x0019 + * UDP header: src_port=0xFFDE, dst_port=0, udp_len=8, csum=0 + * sum += 0xFFDE + 0 + 0x0008 = 0xFFE6 + * Total = 0x0019 + 0xFFE6 = 0xFFFF -> ~0xFFFF = 0 */ + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0, 0, 0); + + /* Set up a UDP socket so ip_output_add_header can be called */ + ts = &s.udpsockets[0]; + memset(ts, 0, sizeof(*ts)); + ts->proto = WI_IPPROTO_UDP; + ts->S = &s; + ts->local_ip = 0; + ts->remote_ip = 0; + ts->if_idx = TEST_PRIMARY_IF; + + memset(&udp, 0, sizeof(udp)); + udp.src_port = ee16(0xFFDE); + udp.dst_port = 0; + udp.len = ee16(8); + udp.csum = 0; + + ip_output_add_header(ts, (struct wolfIP_ip_packet *)&udp, + WI_IPPROTO_UDP, IP_HEADER_LEN + 8); + + /* The stored checksum must be 0xFFFF, not 0. */ + ck_assert_uint_ne(udp.csum, 0); + ck_assert_uint_eq(ee16(udp.csum), 0xFFFF); +} +END_TEST + + /* ----------------------------------------------------------------------- */ diff --git a/src/wolfip.c b/src/wolfip.c index b2f2bcf4..e81b3ad4 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -3084,8 +3084,12 @@ static int ip_output_add_header(struct tsocket *t, struct wolfIP_ip_packet *ip, tcp->csum = ee16(transport_checksum(&ph, &tcp->src_port)); } else if (proto == WI_IPPROTO_UDP) { struct wolfIP_udp_datagram *udp = (struct wolfIP_udp_datagram *)ip; + uint16_t udp_csum; udp->csum = 0; - udp->csum = ee16(transport_checksum(&ph, &udp->src_port)); + udp_csum = transport_checksum(&ph, &udp->src_port); + /* RFC 768: a zero checksum means "no checksum computed," so + * a computed zero must be transmitted as 0xFFFF. */ + udp->csum = ee16(udp_csum ? udp_csum : 0xFFFF); } else if (proto == WI_IPPROTO_ICMP) { struct wolfIP_icmp_packet *icmp = (struct wolfIP_icmp_packet *)ip; icmp->csum = 0; From c2818e99bfbd576d571aa0f2fbfdb6fbc4e9bf42 Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Thu, 26 Mar 2026 16:39:05 +0100 Subject: [PATCH 09/10] when in last_ack state, verify the segment passes the acceptability test --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_proto.c | 62 ++++++++++++++++++++++++++++++ src/test/unit/unit_tests_tcp_ack.c | 4 ++ src/wolfip.c | 6 +++ 4 files changed, 73 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 1bf8d823..74e0d9d7 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -676,6 +676,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_regression_dhcp_nak_restarts_configuration); tcase_add_test(tc_proto, test_regression_dns_rcode_error_aborts_query); tcase_add_test(tc_proto, test_regression_udp_checksum_zero_substituted_with_ffff); + tcase_add_test(tc_proto, test_regression_last_ack_rejects_out_of_window_segment); tcase_add_test(tc_utils, test_transport_checksum); tcase_add_test(tc_utils, test_iphdr_set_checksum); diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index 966ae601..367624a0 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -4365,4 +4365,66 @@ START_TEST(test_regression_udp_checksum_zero_substituted_with_ffff) END_TEST +/* RFC 9293 s3.10.7.2: segment acceptability applies to all synchronized + * states including LAST_ACK. The current LAST_ACK handler processes ACKs + * without checking tcp_segment_acceptable, so an out-of-window ACK could + * close the connection. */ +START_TEST(test_regression_last_ack_rejects_out_of_window_segment) +{ + struct wolfIP s; + struct tsocket *ts; + struct wolfIP_tcp_seg seg; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + ts = &s.tcpsockets[0]; + memset(ts, 0, sizeof(*ts)); + ts->proto = WI_IPPROTO_TCP; + ts->S = &s; + ts->if_idx = TEST_PRIMARY_IF; + ts->sock.tcp.state = TCP_LAST_ACK; + ts->sock.tcp.ack = 100; + ts->sock.tcp.seq = 1000; + ts->sock.tcp.snd_una = 1000; + ts->sock.tcp.last = 999; /* FIN was at seq 999 */ + ts->sock.tcp.cwnd = TCP_MSS; + ts->sock.tcp.peer_rwnd = TCP_MSS; + ts->src_port = 1234; + ts->dst_port = 4321; + ts->local_ip = 0x0A000001U; + ts->remote_ip = 0x0A000002U; + queue_init(&ts->sock.tcp.rxbuf, ts->rxmem, RXBUF_SIZE, ts->sock.tcp.ack); + fifo_init(&ts->sock.tcp.txbuf, ts->txmem, TXBUF_SIZE); + + /* Send an ACK with an out-of-window sequence number. + * rcv_nxt = 100, window = RXBUF_SIZE (20480), so seq = 99999 + * is far outside the window. */ + memset(&seg, 0, sizeof(seg)); + seg.ip.ver_ihl = 0x45; + seg.ip.ttl = 64; + seg.ip.proto = WI_IPPROTO_TCP; + seg.ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN); + seg.ip.src = ee32(ts->remote_ip); + seg.ip.dst = ee32(ts->local_ip); + seg.dst_port = ee16(ts->src_port); + seg.src_port = ee16(ts->dst_port); + seg.hlen = TCP_HEADER_LEN << 2; + seg.flags = TCP_FLAG_ACK; + seg.seq = ee32(99999); + seg.ack = ee32(1001); /* ACKs the FIN */ + seg.win = ee16(65535); + fix_tcp_checksums(&seg); + + tcp_input(&s, TEST_PRIMARY_IF, &seg, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN)); + + /* The out-of-window segment must be rejected; connection must + * remain in LAST_ACK, not transition to CLOSED. */ + ck_assert_int_eq(ts->sock.tcp.state, TCP_LAST_ACK); +} +END_TEST + + /* ----------------------------------------------------------------------- */ diff --git a/src/test/unit/unit_tests_tcp_ack.c b/src/test/unit/unit_tests_tcp_ack.c index 7d274625..102ec416 100644 --- a/src/test/unit/unit_tests_tcp_ack.c +++ b/src/test/unit/unit_tests_tcp_ack.c @@ -3020,6 +3020,8 @@ START_TEST(test_tcp_last_ack_closes_socket) ts->remote_ip = remote_ip; ts->src_port = local_port; ts->dst_port = remote_port; + ts->sock.tcp.ack = 10; + queue_init(&ts->sock.tcp.rxbuf, ts->rxmem, RXBUF_SIZE, ts->sock.tcp.ack); ts->sock.tcp.ctrl_rto_active = 1; memset(&tmr, 0, sizeof(tmr)); tmr.cb = test_timer_cb; @@ -3069,6 +3071,8 @@ START_TEST(test_tcp_last_ack_partial_ack_keeps_socket_and_timer) ts->remote_ip = remote_ip; ts->src_port = local_port; ts->dst_port = remote_port; + ts->sock.tcp.ack = 10; + queue_init(&ts->sock.tcp.rxbuf, ts->rxmem, RXBUF_SIZE, ts->sock.tcp.ack); ts->sock.tcp.ctrl_rto_active = 1; memset(&tmr, 0, sizeof(tmr)); tmr.cb = test_timer_cb; diff --git a/src/wolfip.c b/src/wolfip.c index e81b3ad4..5b2ae908 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -3753,6 +3753,12 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, } } } else if (t->sock.tcp.state == TCP_LAST_ACK) { + /* RFC 9293 s3.10.7.2: segment acceptability applies + * to all synchronized states including LAST_ACK. */ + if (!tcp_segment_acceptable(t, tcp, tcplen)) { + tcp_send_ack(t); + continue; + } /* RFC 9293 §3.10.7.4: if the SYN bit is set on a * synchronized connection, send a challenge ACK and * drop the segment (RFC 5961). */ From a21af7d2cd5e809c7b5f30a4b7cd8f767378d128 Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Thu, 26 Mar 2026 17:35:32 +0100 Subject: [PATCH 10/10] guard dns_id assignment against zero, fall back to 1 when getrandom truncation results to zero --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_proto.c | 27 +++++++++++++++++++++++++++ src/wolfip.c | 4 +++- 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 74e0d9d7..7ac99111 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -677,6 +677,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_regression_dns_rcode_error_aborts_query); tcase_add_test(tc_proto, test_regression_udp_checksum_zero_substituted_with_ffff); tcase_add_test(tc_proto, test_regression_last_ack_rejects_out_of_window_segment); + tcase_add_test(tc_proto, test_regression_dns_id_never_zero); tcase_add_test(tc_utils, test_transport_checksum); tcase_add_test(tc_utils, test_iphdr_set_checksum); diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index 367624a0..872e692c 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -4426,5 +4426,32 @@ START_TEST(test_regression_last_ack_rejects_out_of_window_segment) } END_TEST +/* dns_id is assigned from wolfIP_getrandom() which can truncate to 0. + * Zero is the sentinel for "no query active," so a zero dns_id breaks + * the re-entry guard, disables retransmission, and puts a predictable + * transaction ID on the wire. */ +START_TEST(test_regression_dns_id_never_zero) +{ + struct wolfIP s; + uint16_t id = 0; + + wolfIP_init(&s); + mock_link_init(&s); + s.dns_server = 0x08080808U; + + /* Force wolfIP_getrandom to return 0 */ + test_rand_override_enabled = 1; + test_rand_override_value = 0; + + ck_assert_int_eq(dns_send_query(&s, "example.com", &id, DNS_A), 0); + + /* dns_id must never be zero even when the RNG returns zero */ + ck_assert_uint_ne(s.dns_id, 0); + ck_assert_uint_ne(id, 0); + + test_rand_override_enabled = 0; +} +END_TEST + /* ----------------------------------------------------------------------- */ diff --git a/src/wolfip.c b/src/wolfip.c index 5b2ae908..1a8d7ba8 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -6747,7 +6747,9 @@ static int dns_send_query(struct wolfIP *s, const char *dname, uint16_t *id, return -1; wolfIP_register_callback(s, s->dns_udp_sd, dns_callback, s); } - s->dns_id = wolfIP_getrandom(); + s->dns_id = (uint16_t)(wolfIP_getrandom() & 0xFFFF); + if (s->dns_id == 0) + s->dns_id = 1; *id = s->dns_id; memset(buf, 0, 512); s->dns_query_type = (qtype == DNS_PTR) ? DNS_QUERY_TYPE_PTR : DNS_QUERY_TYPE_A;