Skip to content

Commit b0ee2cc

Browse files
gasbytesdanielinux
authored andcommitted
Mask the reserved nibble on every receive-side read of tcp->hlen via a new
tcp_data_offset_bytes helper so segments carrying non-zero reserved bits are processed identically to those without, per RFC 9293 §3.1's MUST-ignore rule, with a new test_tcp_input_ignores_reserved_bits_in_hlen pinning the contract and four pre-existing option-parser tests re-aligned to legal 4-byte header encodings.
1 parent 3f46c79 commit b0ee2cc

3 files changed

Lines changed: 104 additions & 20 deletions

File tree

src/test/unit/unit.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,7 @@ Suite *wolf_suite(void)
624624
tcase_add_test(tc_utils, test_tcp_recv_ooo_capacity_limit);
625625
tcase_add_test(tc_utils, test_tcp_recv_overlapping_ooo_segments_coalesce_on_consume);
626626
tcase_add_test(tc_utils, test_tcp_input_syn_with_sack_option_enables_sack);
627+
tcase_add_test(tc_utils, test_tcp_input_ignores_reserved_bits_in_hlen);
627628
tcase_add_test(tc_utils, test_tcp_input_syn_with_sack_option_respects_local_sack_offer);
628629
tcase_add_test(tc_utils, test_tcp_input_iplen_too_big);
629630
tcase_add_test(tc_utils, test_tcp_checksum_valid_passes);

src/test/unit/unit_tests_tcp_flow.c

Lines changed: 88 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,68 @@ START_TEST(test_tcp_input_syn_with_sack_option_enables_sack)
431431
}
432432
END_TEST
433433

434+
START_TEST(test_tcp_input_ignores_reserved_bits_in_hlen)
435+
{
436+
struct wolfIP s;
437+
int listen_sd;
438+
struct tsocket *ts;
439+
struct wolfIP_sockaddr_in sin;
440+
struct {
441+
uint8_t frame[sizeof(struct wolfIP_tcp_seg) + 4];
442+
} pkt;
443+
struct wolfIP_tcp_seg *syn = (struct wolfIP_tcp_seg *)pkt.frame;
444+
struct wolfIP_ll_dev *ll;
445+
union transport_pseudo_header ph;
446+
447+
wolfIP_init(&s);
448+
mock_link_init(&s);
449+
wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0);
450+
listen_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, WI_IPPROTO_TCP);
451+
ck_assert_int_gt(listen_sd, 0);
452+
memset(&sin, 0, sizeof(sin));
453+
sin.sin_family = AF_INET;
454+
sin.sin_port = ee16(1234);
455+
sin.sin_addr.s_addr = ee32(0x0A000001U);
456+
ck_assert_int_eq(wolfIP_sock_bind(&s, listen_sd, (struct wolfIP_sockaddr *)&sin, sizeof(sin)), 0);
457+
ck_assert_int_eq(wolfIP_sock_listen(&s, listen_sd, 1), 0);
458+
ts = &s.tcpsockets[SOCKET_UNMARK(listen_sd)];
459+
ll = wolfIP_getdev_ex(&s, TEST_PRIMARY_IF);
460+
ck_assert_ptr_nonnull(ll);
461+
462+
memset(&pkt, 0, sizeof(pkt));
463+
memcpy(syn->ip.eth.dst, ll->mac, 6);
464+
syn->ip.eth.type = ee16(ETH_TYPE_IP);
465+
syn->ip.ver_ihl = 0x45;
466+
syn->ip.ttl = 64;
467+
syn->ip.proto = WI_IPPROTO_TCP;
468+
syn->ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN + 4);
469+
syn->ip.src = ee32(0x0A0000A1U);
470+
syn->ip.dst = ee32(0x0A000001U);
471+
iphdr_set_checksum(&syn->ip);
472+
syn->src_port = ee16(40000);
473+
syn->dst_port = ee16(1234);
474+
syn->seq = ee32(1);
475+
syn->hlen = (uint8_t)(((TCP_HEADER_LEN + 4) << 2) | 0x0C);
476+
syn->flags = TCP_FLAG_SYN;
477+
syn->win = ee16(65535);
478+
syn->data[0] = TCP_OPTION_SACK_PERMITTED;
479+
syn->data[1] = TCP_OPTION_SACK_PERMITTED_LEN;
480+
syn->data[2] = TCP_OPTION_NOP;
481+
syn->data[3] = TCP_OPTION_NOP;
482+
483+
memset(&ph, 0, sizeof(ph));
484+
ph.ph.src = syn->ip.src;
485+
ph.ph.dst = syn->ip.dst;
486+
ph.ph.proto = WI_IPPROTO_TCP;
487+
ph.ph.len = ee16(TCP_HEADER_LEN + 4);
488+
syn->csum = ee16(transport_checksum(&ph, &syn->src_port));
489+
490+
tcp_input(&s, TEST_PRIMARY_IF, syn,
491+
sizeof(struct wolfIP_eth_frame) + IP_HEADER_LEN + TCP_HEADER_LEN + 4);
492+
ck_assert_uint_eq(ts->sock.tcp.sack_permitted, 1);
493+
}
494+
END_TEST
495+
434496
START_TEST(test_tcp_input_syn_with_sack_option_respects_local_sack_offer)
435497
{
436498
struct wolfIP s;
@@ -722,7 +784,7 @@ END_TEST
722784

723785
START_TEST(test_tcp_parse_sack_wraparound_block_accepted)
724786
{
725-
uint8_t seg_buf[sizeof(struct wolfIP_tcp_seg) + 10];
787+
uint8_t seg_buf[sizeof(struct wolfIP_tcp_seg) + 12];
726788
struct wolfIP_tcp_seg *seg = (struct wolfIP_tcp_seg *)seg_buf;
727789
struct tcp_parsed_opts po;
728790
uint32_t left = 0xFFFFFFF0U;
@@ -731,7 +793,7 @@ START_TEST(test_tcp_parse_sack_wraparound_block_accepted)
731793
uint32_t frame_len;
732794

733795
memset(seg_buf, 0, sizeof(seg_buf));
734-
seg->hlen = (uint8_t)((TCP_HEADER_LEN + 10) << 2);
796+
seg->hlen = (uint8_t)((TCP_HEADER_LEN + 12) << 2);
735797
opt = seg->data;
736798
opt[0] = TCP_OPTION_SACK;
737799
opt[1] = 10;
@@ -741,8 +803,10 @@ START_TEST(test_tcp_parse_sack_wraparound_block_accepted)
741803
memcpy(opt + 2, &left_be, sizeof(left_be));
742804
memcpy(opt + 6, &right_be, sizeof(right_be));
743805
}
806+
opt[10] = TCP_OPTION_NOP;
807+
opt[11] = TCP_OPTION_NOP;
744808

745-
frame_len = ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN + 10;
809+
frame_len = ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN + 12;
746810
tcp_parse_options(seg, frame_len, &po);
747811

748812
ck_assert_int_eq(po.sack_count, 1);
@@ -808,7 +872,7 @@ START_TEST(test_tcp_parse_options_parses_and_clamps_mixed_options)
808872
uint32_t frame_len;
809873

810874
memset(seg_buf, 0, sizeof(seg_buf));
811-
seg->hlen = (uint8_t)((TCP_HEADER_LEN + 30) << 2);
875+
seg->hlen = (uint8_t)((TCP_HEADER_LEN + 32) << 2);
812876
opt = seg->data;
813877
opt[0] = TCP_OPTION_WS;
814878
opt[1] = TCP_OPTION_WS_LEN;
@@ -840,8 +904,10 @@ START_TEST(test_tcp_parse_options_parses_and_clamps_mixed_options)
840904
memcpy(opt + 6, &right, sizeof(right));
841905
}
842906
opt += 10;
843-
opt[0] = TCP_OPTION_EOO;
844-
frame_len = ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN + 30;
907+
opt[0] = TCP_OPTION_NOP;
908+
opt[1] = TCP_OPTION_NOP;
909+
opt[2] = TCP_OPTION_EOO;
910+
frame_len = ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN + 32;
845911

846912
tcp_parse_options(seg, frame_len, &po);
847913

@@ -867,7 +933,7 @@ START_TEST(test_tcp_parse_options_parses_mss_sack_permitted_timestamp_and_two_sa
867933
uint32_t frame_len;
868934

869935
memset(seg_buf, 0, sizeof(seg_buf));
870-
seg->hlen = (uint8_t)((TCP_HEADER_LEN + 34) << 2);
936+
seg->hlen = (uint8_t)((TCP_HEADER_LEN + 36) << 2);
871937
opt = seg->data;
872938
opt[0] = TCP_OPTION_MSS;
873939
opt[1] = TCP_OPTION_MSS_LEN;
@@ -897,7 +963,10 @@ START_TEST(test_tcp_parse_options_parses_mss_sack_permitted_timestamp_and_two_sa
897963
memcpy(opt + 10, &left, sizeof(left));
898964
memcpy(opt + 14, &right, sizeof(right));
899965
}
900-
frame_len = ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN + 34;
966+
opt += 18;
967+
opt[0] = TCP_OPTION_NOP;
968+
opt[1] = TCP_OPTION_NOP;
969+
frame_len = ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN + 36;
901970

902971
tcp_parse_options(seg, frame_len, &po);
903972

@@ -942,25 +1011,31 @@ END_TEST
9421011

9431012
START_TEST(test_tcp_parse_options_caps_sack_block_count)
9441013
{
945-
uint8_t seg_buf[sizeof(struct wolfIP_tcp_seg) + 48];
1014+
/* TCP_SACK_MAX_BLOCKS equals the natural ceiling of SACK blocks that fit
1015+
* in the 40-byte legal options budget (2-byte option header + 4*8 block
1016+
* bytes = 34 bytes), so we exercise the MAX path with a compliant SACK
1017+
* option carrying exactly TCP_SACK_MAX_BLOCKS blocks. */
1018+
uint8_t seg_buf[sizeof(struct wolfIP_tcp_seg) + 36];
9461019
struct wolfIP_tcp_seg *seg = (struct wolfIP_tcp_seg *)seg_buf;
9471020
struct tcp_parsed_opts po;
9481021
uint8_t *opt;
9491022
uint32_t frame_len;
9501023
int i;
9511024

9521025
memset(seg_buf, 0, sizeof(seg_buf));
953-
seg->hlen = (uint8_t)((TCP_HEADER_LEN + 42) << 2);
1026+
seg->hlen = (uint8_t)((TCP_HEADER_LEN + 36) << 2);
9541027
opt = seg->data;
9551028
opt[0] = TCP_OPTION_SACK;
956-
opt[1] = 42;
957-
for (i = 0; i < 5; i++) {
1029+
opt[1] = (uint8_t)(2 + (TCP_SACK_MAX_BLOCKS * 8));
1030+
for (i = 0; i < TCP_SACK_MAX_BLOCKS; i++) {
9581031
uint32_t left = ee32((uint32_t)(100 + (i * 20)));
9591032
uint32_t right = ee32((uint32_t)(110 + (i * 20)));
9601033
memcpy(opt + 2 + (i * 8), &left, sizeof(left));
9611034
memcpy(opt + 2 + (i * 8) + 4, &right, sizeof(right));
9621035
}
963-
frame_len = ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN + 42;
1036+
opt[2 + (TCP_SACK_MAX_BLOCKS * 8)] = TCP_OPTION_NOP;
1037+
opt[3 + (TCP_SACK_MAX_BLOCKS * 8)] = TCP_OPTION_NOP;
1038+
frame_len = ETH_HEADER_LEN + IP_HEADER_LEN + TCP_HEADER_LEN + 36;
9641039

9651040
tcp_parse_options(seg, frame_len, &po);
9661041

src/wolfip.c

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2691,11 +2691,19 @@ struct tcp_parsed_opts {
26912691
uint32_t ts_val, ts_ecr;
26922692
};
26932693

2694+
/* RFC 9293 3.1: the low nibble of the Data Offset byte is Rsrvd and MUST be
2695+
* ignored by receivers. Mask it off before translating the 4-bit word count
2696+
* into bytes. */
2697+
static inline uint32_t tcp_data_offset_bytes(uint8_t hlen)
2698+
{
2699+
return (uint32_t)((hlen & 0xF0U) >> 2);
2700+
}
2701+
26942702
static void tcp_parse_options(const struct wolfIP_tcp_seg *tcp, uint32_t frame_len,
26952703
struct tcp_parsed_opts *po)
26962704
{
26972705
const uint8_t *opt = tcp->data;
2698-
int claimed_opt_len = (tcp->hlen >> 2) - TCP_HEADER_LEN;
2706+
int claimed_opt_len = (int)tcp_data_offset_bytes(tcp->hlen) - TCP_HEADER_LEN;
26992707
int available_bytes = (int)frame_len - (int)sizeof(struct wolfIP_tcp_seg);
27002708
int opt_len;
27012709
const uint8_t *opt_end;
@@ -3116,7 +3124,7 @@ static void tcp_send_reset_reply(struct wolfIP *s, unsigned int if_idx,
31163124
return;
31173125

31183126
ip_len = ee16(in->ip.len);
3119-
tcp_hlen = (uint32_t)(in->hlen >> 2);
3127+
tcp_hlen = tcp_data_offset_bytes(in->hlen);
31203128
if (tcp_hlen < TCP_HEADER_LEN)
31213129
return;
31223130
if (ip_len < (uint16_t)(IP_HEADER_LEN + tcp_hlen))
@@ -4306,7 +4314,7 @@ static void tcp_ack(struct tsocket *t, const struct wolfIP_tcp_seg *tcp)
43064314
}
43074315

43084316
tcp_process_sack(t, tcp,
4309-
(uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + (tcp->hlen >> 2)));
4317+
(uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + tcp_data_offset_bytes(tcp->hlen)));
43104318
desc = fifo_peek(&t->sock.tcp.txbuf);
43114319
while ((desc) && (desc->flags & PKT_FLAG_SENT)) {
43124320
struct wolfIP_tcp_seg *seg = (struct wolfIP_tcp_seg *)(t->txmem + desc->pos + sizeof(*desc));
@@ -4394,7 +4402,7 @@ static void tcp_ack(struct tsocket *t, const struct wolfIP_tcp_seg *tcp)
43944402
if (ack_count > 0) {
43954403
struct pkt_desc *fresh_desc = NULL;
43964404
uint32_t ack_ip_len = ee16(tcp->ip.len);
4397-
uint32_t ack_hdr_len = IP_HEADER_LEN + (uint32_t)(tcp->hlen >> 2);
4405+
uint32_t ack_hdr_len = IP_HEADER_LEN + tcp_data_offset_bytes(tcp->hlen);
43984406
uint32_t ack_frame_len = 0;
43994407
/* This ACK ackwnowledged some data. */
44004408
desc = fifo_peek(&t->sock.tcp.txbuf);
@@ -4575,14 +4583,14 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx,
45754583
t->if_idx = (uint8_t)if_idx;
45764584
matched = 1;
45774585
/* Validate minimum TCP header length (data offset). */
4578-
if ((tcp->hlen >> 2) < TCP_HEADER_LEN) {
4586+
if (tcp_data_offset_bytes(tcp->hlen) < TCP_HEADER_LEN) {
45794587
return; /* malformed: TCP header below minimum length */
45804588
}
45814589
/* Validate TCP header length fits in IP payload */
4582-
if (iplen < (uint32_t)(IP_HEADER_LEN + (tcp->hlen >> 2))) {
4590+
if (iplen < (uint32_t)(IP_HEADER_LEN + tcp_data_offset_bytes(tcp->hlen))) {
45834591
return; /* malformed: TCP header exceeds IP length */
45844592
}
4585-
tcplen = iplen - (IP_HEADER_LEN + (tcp->hlen >> 2));
4593+
tcplen = iplen - (IP_HEADER_LEN + tcp_data_offset_bytes(tcp->hlen));
45864594
if (t->sock.tcp.state == TCP_LISTEN) {
45874595
/* RFC 9293 3.10.7.2: reject ACK-bearing segments before SYN handling. */
45884596
if (tcp->flags & TCP_FLAG_RST)

0 commit comments

Comments
 (0)