From 6acc2b4795615ca482caa9ec33c13499676d5885 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 26 Jun 2023 19:02:24 +0200 Subject: [PATCH] tcp: consider segments partially overlapping the window as acceptable rfc9293 says this should be an OR, not an AND: https://www.rfc-editor.org/rfc/rfc9293.html#name-segment-acceptability-tests --- src/socket/tcp.rs | 141 +++++++++++++++++++++++++++++++++++----------- src/wire/tcp.rs | 18 ++++++ 2 files changed, 125 insertions(+), 34 deletions(-) diff --git a/src/socket/tcp.rs b/src/socket/tcp.rs index 0384d865a..a5ced968d 100644 --- a/src/socket/tcp.rs +++ b/src/socket/tcp.rs @@ -1475,41 +1475,77 @@ impl<'a> Socket<'a> { let segment_start = repr.seq_number; let segment_end = repr.seq_number + repr.segment_len(); - let payload_offset; - match self.state { + let (payload, payload_offset) = match self.state { // In LISTEN and SYN-SENT states, we have not yet synchronized with the remote end. - State::Listen | State::SynSent => payload_offset = 0, - // In all other states, segments must occupy a valid portion of the receive window. + State::Listen | State::SynSent => (&[][..], 0), _ => { - let mut segment_in_window = true; + // https://www.rfc-editor.org/rfc/rfc9293.html#name-segment-acceptability-tests + let segment_in_window = match ( + segment_start == segment_end, + window_start == window_end, + ) { + (true, _) if segment_end == window_start - 1 => { + net_debug!( + "received a keep-alive or window probe packet, will send an ACK" + ); + false + } + (true, true) => { + if window_start == segment_start { + true + } else { + net_debug!( + "zero-length segment not inside zero-length window, will send an ACK." + ); + false + } + } + (true, false) => { + if window_start <= segment_start && segment_start < window_end { + true + } else { + net_debug!("zero-length segment not inside window, will send an ACK."); + false + } + } + (false, true) => { + net_debug!( + "non-zero-length segment with zero receive window, will only send an ACK" + ); + false + } + (false, false) => { + if (window_start <= segment_start && segment_start < window_end) + || (window_start < segment_end && segment_end <= window_end) + { + true + } else { + net_debug!( + "segment not in receive window ({}..{} not intersecting {}..{}), will send challenge ACK", + segment_start, + segment_end, + window_start, + window_end + ); + false + } + } + }; - if window_start == window_end && segment_start != segment_end { - net_debug!( - "non-zero-length segment with zero receive window, will only send an ACK" - ); - segment_in_window = false; - } + if segment_in_window { + let segment_data_end = repr.seq_number + repr.payload.len(); + let overlap_start = window_start.max(segment_start); + let overlap_end = window_end.min(segment_data_end); - if segment_start == segment_end && segment_end == window_start - 1 { - net_debug!("received a keep-alive or window probe packet, will send an ACK"); - segment_in_window = false; - } else if !((window_start <= segment_start && segment_start <= window_end) - && (window_start <= segment_end && segment_end <= window_end)) - { - net_debug!( - "segment not in receive window ({}..{} not intersecting {}..{}), will send challenge ACK", - segment_start, - segment_end, - window_start, - window_end - ); - segment_in_window = false; - } + // the checks done above imply this. + debug_assert!(overlap_start <= overlap_end); - if segment_in_window { - // We've checked that segment_start >= window_start above. - payload_offset = segment_start - window_start; self.local_rx_last_seq = Some(repr.seq_number); + + ( + &repr.payload[overlap_start - segment_start..overlap_end - segment_start], + overlap_start - window_start, + ) } else { // If we're in the TIME-WAIT state, restart the TIME-WAIT timeout, since // the remote end may not have realized we've closed the connection. @@ -1520,7 +1556,7 @@ impl<'a> Socket<'a> { return self.challenge_ack_reply(cx, ip_repr, repr); } } - } + }; // Compute the amount of acknowledged octets, removing the SYN and FIN bits // from the sequence space. @@ -1824,7 +1860,7 @@ impl<'a> Socket<'a> { } } - let payload_len = repr.payload.len(); + let payload_len = payload.len(); if payload_len == 0 { return None; } @@ -1847,9 +1883,7 @@ impl<'a> Socket<'a> { payload_len, payload_offset ); - let len_written = self - .rx_buffer - .write_unallocated(payload_offset, repr.payload); + let len_written = self.rx_buffer.write_unallocated(payload_offset, payload); debug_assert!(len_written == payload_len); if contig_len != 0 { @@ -3915,6 +3949,45 @@ mod test { ); } + #[test] + fn test_established_receive_partially_outside_window() { + let mut s = socket_established(); + + send!( + s, + TcpRepr { + seq_number: REMOTE_SEQ + 1, + ack_number: Some(LOCAL_SEQ + 1), + payload: &b"abc"[..], + ..SEND_TEMPL + } + ); + + s.recv(|data| { + assert_eq!(data, b"abc"); + (3, ()) + }) + .unwrap(); + + // Peer decides to retransmit (perhaps because the ACK was lost) + // and also pushed data. + send!( + s, + TcpRepr { + seq_number: REMOTE_SEQ + 1, + ack_number: Some(LOCAL_SEQ + 1), + payload: &b"abcdef"[..], + ..SEND_TEMPL + } + ); + + s.recv(|data| { + assert_eq!(data, b"def"); + (3, ()) + }) + .unwrap(); + } + #[test] fn test_established_send_wrap() { let mut s = socket_established(); diff --git a/src/wire/tcp.rs b/src/wire/tcp.rs index bcc6fbc59..060f95891 100644 --- a/src/wire/tcp.rs +++ b/src/wire/tcp.rs @@ -13,6 +13,24 @@ use crate::wire::{IpAddress, IpProtocol}; #[derive(Debug, PartialEq, Eq, Clone, Copy, Default)] pub struct SeqNumber(pub i32); +impl SeqNumber { + pub fn max(self, rhs: Self) -> Self { + if self > rhs { + self + } else { + rhs + } + } + + pub fn min(self, rhs: Self) -> Self { + if self < rhs { + self + } else { + rhs + } + } +} + impl fmt::Display for SeqNumber { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{}", self.0 as u32)