[inetstack] Bug Fix: Check for window sizes before creating socket

network_simulator/input/tcp/accept/accept-large-window-size.pkt

@@ -0,0 +1,17 @@
+// Test for blocking accept with a large window size.
+
+// Accept a connection.
+ +.0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 500
++.0 bind(500, ..., ...) = 0
++.0 listen(500, 1) = 0
++.2 accept(500, ..., ...) = 0
+
+// Receive SYN packet.
++.2 TCP < S seq 0(0) win 65535 <mss 1450,wscale 14>
+// Send SYN-ACK packet.
++.0 TCP > S. seq 0(0) ack 1 win 65535 <mss 1450,wscale 0>
+// Receive ACK on SYN-ACK packet.
++.2 TCP < . seq 1(0) ack 1 win 65535 <nop>
+
+// Succeed to accept connection.
++.0 wait(500, ...) = 0

src/rust/inetstack/protocols/layer4/tcp/active_open.rs

@@ -139,13 +139,13 @@ impl SharedActiveOpenSocket {
         self.layer3_endpoint
             .transmit_tcp_packet_nonblocking(dst_ipv4_addr, pkt)?;
 
-        let mut remote_window_scale = None;
+        let mut remote_window_scale_bits = None;
         let mut mss = FALLBACK_MSS;
         for option in header.iter_options() {
             match option {
                 TcpOptions2::WindowScale(w) => {
                     info!("Received window scale: {}", w);
-                    remote_window_scale = Some(*w);
+                    remote_window_scale_bits = Some(*w);
                 },
                 TcpOptions2::MaximumSegmentSize(m) => {
                     info!("Received advertised MSS: {}", m);
@@ -155,16 +155,16 @@ impl SharedActiveOpenSocket {
             }
         }
 
-        let (local_window_scale, remote_window_scale): (u8, u8) = match remote_window_scale {
-            Some(remote_window_scale) => {
-                let remote: u8 = if remote_window_scale as usize > MAX_WINDOW_SCALE {
+        let (local_window_scale_bits, remote_window_scale_bits): (u8, u8) = match remote_window_scale_bits {
+            Some(remote_window_scale_bits) => {
+                let remote: u8 = if remote_window_scale_bits as usize > MAX_WINDOW_SCALE {
                     warn!(
                         "remote windows scale larger than {:?} is incorrect, so setting to {:?}. See RFC 1323.",
                         MAX_WINDOW_SCALE, MAX_WINDOW_SCALE
                     );
                     MAX_WINDOW_SCALE as u8
                 } else {
-                    remote_window_scale
+                    remote_window_scale_bits
                 };
                 (self.tcp_config.get_window_scale() as u8, remote)
             },
@@ -173,22 +173,25 @@ impl SharedActiveOpenSocket {
 
         // Expect is safe here because the receive window size is a 16-bit unsigned integer and MAX_WINDOW_SCALE is 14,
         // so it is impossible to overflow the 32-bit unsigned int.
-        debug_assert!((local_window_scale as usize) <= MAX_WINDOW_SCALE);
-        let rx_window_size: u32 = expect_some!(
-            (self.tcp_config.get_receive_window_size() as u32).checked_shl(local_window_scale as u32),
+        debug_assert!((local_window_scale_bits as usize) <= MAX_WINDOW_SCALE);
+        let rx_window_size_bytes: u32 = expect_some!(
+            (self.tcp_config.get_receive_window_size() as u32).checked_shl(local_window_scale_bits as u32),
             "Window size overflow"
         );
         // Expect is safe here because the window size is a 16-bit unsigned integer and MAX_WINDOW_SCALE is 14, so it is impossible to overflow the 32-bit
-        debug_assert!((remote_window_scale as usize) <= MAX_WINDOW_SCALE);
-        let tx_window_size: u32 = expect_some!(
-            (header.window_size as u32).checked_shl(remote_window_scale as u32),
+        debug_assert!((remote_window_scale_bits as usize) <= MAX_WINDOW_SCALE);
+        let tx_window_size_bytes: u32 = expect_some!(
+            (header.window_size as u32).checked_shl(remote_window_scale_bits as u32),
             "Window size overflow"
         );
 
-        info!("Window sizes: local {}, remote {}", rx_window_size, tx_window_size);
+        info!(
+            "Window sizes: local {} bytes, remote {} bytes",
+            rx_window_size_bytes, tx_window_size_bytes
+        );
         info!(
             "Window scale: local {}, remote {}",
-            local_window_scale, remote_window_scale
+            local_window_scale_bits, remote_window_scale_bits
         );
         Ok(SharedEstablishedSocket::new(
             self.local,
@@ -200,11 +203,11 @@ impl SharedActiveOpenSocket {
             self.socket_options,
             remote_seq_num,
             self.tcp_config.get_ack_delay_timeout(),
-            rx_window_size,
-            local_window_scale,
+            rx_window_size_bytes,
+            local_window_scale_bits,
             expected_seq,
-            tx_window_size,
-            remote_window_scale,
+            tx_window_size_bytes,
+            remote_window_scale_bits,
             mss,
             congestion_control::None::new,
             None,

src/rust/inetstack/protocols/layer4/tcp/established/mod.rs

@@ -1,12 +1,20 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
+//======================================================================================================================
+// Exports
+//======================================================================================================================
+
 pub mod congestion_control;
 pub mod ctrlblk;
 mod receiver;
 mod rto;
 mod sender;
 
+//======================================================================================================================
+// Imports
+//======================================================================================================================
+
 use crate::{
     async_timer,
     collections::async_queue::SharedAsyncQueue,
@@ -37,6 +45,19 @@ use ::std::{
     time::Instant,
 };
 
+//======================================================================================================================
+// Constants
+//======================================================================================================================
+
+// The max possible window size without scaling is 64KB.
+const MAX_WINDOW_SIZE_WITHOUT_SCALING: u32 = 65535;
+// THe max possible window size with scaling is 1GB.
+const MAX_WINDOW_SIZE_WITH_SCALING: u32 = 1073741824;
+
+//======================================================================================================================
+// Structures
+//======================================================================================================================
+
 pub struct EstablishedSocket {
     // All shared state for this established TCP connection.
     cb: ControlBlock,
@@ -47,6 +68,10 @@ pub struct EstablishedSocket {
 #[derive(Clone)]
 pub struct SharedEstablishedSocket(SharedObject<EstablishedSocket>);
 
+//======================================================================================================================
+// Associated Functions
+//======================================================================================================================
+
 impl SharedEstablishedSocket {
     pub fn new(
         local: SocketAddrV4,
@@ -58,26 +83,68 @@ impl SharedEstablishedSocket {
         default_socket_options: TcpSocketOptions,
         receiver_seq_no: SeqNumber,
         ack_delay_timeout_secs: Duration,
-        receiver_window_size_frames: u32,
+        receiver_window_size_bytes: u32,
         receiver_window_scale_bits: u8,
         sender_seq_no: SeqNumber,
-        sender_window_size_frames: u32,
+        sender_window_size_bytes: u32,
         sender_window_scale_bits: u8,
         sender_mss: usize,
         cc_constructor: CongestionControlConstructor,
         congestion_control_options: Option<congestion_control::Options>,
     ) -> Result<Self, Fail> {
+        // Check that the send window size is not too large.
+        match sender_window_scale_bits {
+            0 if sender_window_size_bytes > MAX_WINDOW_SIZE_WITHOUT_SCALING => {
+                let cause = "Sender window too large";
+                warn!(
+                    "{}: scale={:?} window={:?}",
+                    cause, sender_window_scale_bits, sender_window_size_bytes
+                );
+                return Err(Fail::new(libc::EINVAL, &cause));
+            },
+            _ if sender_window_size_bytes > MAX_WINDOW_SIZE_WITH_SCALING => {
+                let cause = "Sender window too large";
+                warn!(
+                    "{}: scale={:?} window={:?}",
+                    cause, sender_window_scale_bits, sender_window_size_bytes
+                );
+                return Err(Fail::new(libc::EINVAL, &cause));
+            },
+            _ => (),
+        };
+
+        // Check that the receive window size is not too large.
+        match receiver_window_scale_bits {
+            0 if receiver_window_size_bytes > MAX_WINDOW_SIZE_WITHOUT_SCALING => {
+                let cause = "Receiver window too large";
+                warn!(
+                    "{}: scale={:?} window={:?}",
+                    cause, receiver_window_scale_bits, receiver_window_size_bytes
+                );
+                return Err(Fail::new(libc::EINVAL, &cause));
+            },
+            _ if receiver_window_size_bytes > MAX_WINDOW_SIZE_WITH_SCALING => {
+                let cause = "Receiver window too large";
+                warn!(
+                    "{}: scale={:?} window={:?}",
+                    cause, receiver_window_scale_bits, receiver_window_size_bytes
+                );
+                return Err(Fail::new(libc::EINVAL, &cause));
+            },
+            _ => (),
+        };
+
         let sender: Sender = Sender::new(
             sender_seq_no,
-            sender_window_size_frames,
+            sender_window_size_bytes,
             sender_window_scale_bits,
             sender_mss,
         );
         let receiver: Receiver = Receiver::new(
             receiver_seq_no,
             receiver_seq_no,
             ack_delay_timeout_secs,
-            receiver_window_size_frames,
+            receiver_window_size_bytes,
             receiver_window_scale_bits,
         );
 

src/rust/inetstack/protocols/layer4/tcp/established/receiver.rs

@@ -16,7 +16,9 @@ use crate::{
     inetstack::protocols::{
         layer3::SharedLayer3Endpoint,
         layer4::tcp::{
-            established::{ctrlblk::State, ControlBlock, Sender},
+            established::{
+                ctrlblk::State, ControlBlock, Sender, MAX_WINDOW_SIZE_WITHOUT_SCALING, MAX_WINDOW_SIZE_WITH_SCALING,
+            },
             header::TcpHeader,
             SeqNumber,
         },
@@ -73,7 +75,7 @@ pub struct Receiver {
 
     // This is our receive buffer size, which is also the maximum size of our receive window.
     // Note: The maximum possible advertised window is 1 GiB with window scaling and 64 KiB without.
-    buffer_size_frames: u32,
+    buffer_size_bytes: u32,
 
     // This is the number of bits to shift to convert to/from the scaled value, and has a maximum value of 14.
     window_scale_shift_bits: u8,
@@ -93,7 +95,7 @@ impl Receiver {
         reader_next_seq_no: SeqNumber,
         receive_next_seq_no: SeqNumber,
         ack_delay_timeout_secs: Duration,
-        window_size_frames: u32,
+        window_size_bytes: u32,
         window_scale_shift_bits: u8,
     ) -> Self {
         Self {
@@ -103,7 +105,7 @@ impl Receiver {
             pop_queue: AsyncQueue::with_capacity(1024),
             ack_delay_timeout_secs,
             ack_deadline_time_secs: SharedAsyncValue::new(None),
-            buffer_size_frames: window_size_frames,
+            buffer_size_bytes: window_size_bytes,
             window_scale_shift_bits,
             out_of_order_frames: VecDeque::with_capacity(64),
         }
@@ -122,7 +124,12 @@ impl Receiver {
 
     pub fn get_receive_window_size(&self) -> u32 {
         let bytes_unread: u32 = (self.receive_next_seq_no - self.reader_next_seq_no).into();
-        self.buffer_size_frames - bytes_unread
+        // The window should be less than 1GB or 64KB without scaling.
+        debug_assert!(
+            (self.window_scale_shift_bits == 0 && bytes_unread <= MAX_WINDOW_SIZE_WITHOUT_SCALING)
+                || bytes_unread <= MAX_WINDOW_SIZE_WITH_SCALING
+        );
+        self.buffer_size_bytes - bytes_unread
     }
 
     pub fn hdr_window_size(&self) -> u16 {

src/rust/inetstack/protocols/layer4/tcp/passive_open.rs

@@ -362,8 +362,8 @@ impl SharedPassiveSocket {
         remote: SocketAddrV4,
         local_isn: SeqNumber,
         remote_isn: SeqNumber,
-        header_window_size: u16,
-        remote_window_scale: Option<u8>,
+        remote_window_size_bytes: u16,
+        remote_window_scale_bits: Option<u8>,
         mss: usize,
     ) -> Result<SharedEstablishedSocket, Fail> {
         let (ipv4_hdr, tcp_hdr, buf) = recv_queue.pop(None).await?;
@@ -375,7 +375,7 @@ impl SharedPassiveSocket {
         }
 
         // Calculate the window.
-        let (local_window_scale, remote_window_scale): (u8, u8) = match remote_window_scale {
+        let (local_window_scale_bits, remote_window_scale_bits): (u8, u8) = match remote_window_scale_bits {
             Some(remote_window_scale) => {
                 if (remote_window_scale as usize) > MAX_WINDOW_SCALE {
                     warn!(
@@ -391,25 +391,25 @@ impl SharedPassiveSocket {
         };
 
         // Expect is safe here because the window size is a 16-bit unsigned integer and MAX_WINDOW_SCALE is 14, so it is impossible to overflow the 32-bit
-        debug_assert!((remote_window_scale as usize) <= MAX_WINDOW_SCALE);
-        let remote_window_size: u32 = expect_some!(
-            (header_window_size as u32).checked_shl(remote_window_scale as u32),
+        debug_assert!((remote_window_scale_bits as usize) <= MAX_WINDOW_SCALE);
+        let remote_window_size_bytes: u32 = expect_some!(
+            (remote_window_size_bytes as u32).checked_shl(remote_window_scale_bits as u32),
             "Window size overflow"
         );
         // Expect is safe here because the receive window size is a 16-bit unsigned integer and MAX_WINDOW_SCALE is 14,
         // so it is impossible to overflow the 32-bit unsigned int.
-        debug_assert!((local_window_scale as usize) <= MAX_WINDOW_SCALE);
-        let local_window_size: u32 = expect_some!(
-            (self.tcp_config.get_receive_window_size() as u32).checked_shl(local_window_scale as u32),
+        debug_assert!((local_window_scale_bits as usize) <= MAX_WINDOW_SCALE);
+        let local_window_size_bytes: u32 = expect_some!(
+            (self.tcp_config.get_receive_window_size() as u32).checked_shl(local_window_scale_bits as u32),
             "Window size overflow"
         );
         info!(
-            "Window sizes: local {}, remote {}",
-            local_window_size, remote_window_size
+            "Window sizes: local {} bytes, remote {} bytes",
+            local_window_size_bytes, remote_window_size_bytes
         );
         info!(
             "Window scale: local {}, remote {}",
-            local_window_scale, remote_window_scale
+            local_window_scale_bits, remote_window_scale_bits
         );
 
         // If there is data with the SYN+ACK, deliver it.
@@ -427,11 +427,11 @@ impl SharedPassiveSocket {
             self.socket_options,
             remote_isn + SeqNumber::from(1),
             self.tcp_config.get_ack_delay_timeout(),
-            local_window_size,
-            local_window_scale,
+            local_window_size_bytes,
+            local_window_scale_bits,
             local_isn + SeqNumber::from(1),
-            remote_window_size,
-            remote_window_scale,
+            remote_window_size_bytes,
+            remote_window_scale_bits,
             mss,
             congestion_control::None::new,
             None,
@@ -441,6 +441,7 @@ impl SharedPassiveSocket {
     }
 
     fn complete_handshake(&mut self, remote: SocketAddrV4, result: Result<SharedEstablishedSocket, Fail>) {
+        warn!("completing handshake");
         self.connections.remove(&remote);
         self.ready.push((remote, result));
     }