Merge branch 'main' into test-streams-bench

neqo-transport/src/crypto.rs

@@ -36,7 +36,7 @@ use crate::{
     recovery::RecoveryToken,
     recv_stream::RxStreamOrderer,
     send_stream::TxBuffer,
-    shuffle::find_sni,
+    sni::find_sni,
     stats::FrameStats,
     tparams::{TpZeroRttChecker, TransportParameters, TransportParametersHandler},
     tracking::PacketNumberSpace,
@@ -1554,51 +1554,52 @@ impl CryptoStreams {
                 (left, _) = left.split_at(limit - right.len());
             } else {
                 // Both chunks are too long to fit into one packet. Just send a part of each.
-                let half_limit = limit / 2;
-                (left, _) = left.split_at(half_limit);
-                (right, _) = right.split_at(half_limit);
+                (left, _) = left.split_at(limit / 2);
+                (right, _) = right.split_at(limit / 2);
             }
             ((left_offset, left), (right_offset, right))
         }
 
         let Some(cs) = self.get_mut(space) else {
             return;
         };
-        let Some((offset, data)) = cs.tx.next_bytes() else {
-            return;
-        };
-        let written = if sni_slicing {
-            if let Some(sni) = find_sni(data) {
-                // Cut the crypto data in two at the midpoint of the SNI and swap the chunks.
-                let mid = sni.start + (sni.end - sni.start) / 2;
-                let (left, right) = data.split_at(mid);
-
-                // Truncate the chunks so we can fit them into roughly evenly-filled packets.
-                let packets_needed = data.len().div_ceil(builder.limit());
-                let limit = data.len() / packets_needed;
-                let ((left_offset, left), (right_offset, right)) =
-                    limit_chunks((offset, left), (offset + mid as u64, right), limit);
-                (
-                    write_chunk(right_offset, right, builder),
-                    write_chunk(left_offset, left, builder),
-                )
+        while let Some((offset, data)) = cs.tx.next_bytes() {
+            let written = if sni_slicing && offset == 0 {
+                if let Some(sni) = find_sni(data) {
+                    // Cut the crypto data in two at the midpoint of the SNI and swap the chunks.
+                    let mid = sni.start + (sni.end - sni.start) / 2;
+                    let (left, right) = data.split_at(mid);
+
+                    // Truncate the chunks so we can fit them into roughly evenly-filled packets.
+                    let packets_needed = data.len().div_ceil(builder.limit());
+                    let limit = data.len() / packets_needed;
+                    let ((left_offset, left), (right_offset, right)) =
+                        limit_chunks((offset, left), (offset + mid as u64, right), limit);
+                    (
+                        write_chunk(right_offset, right, builder),
+                        write_chunk(left_offset, left, builder),
+                    )
+                } else {
+                    // No SNI found, write the entire data.
+                    (write_chunk(offset, data, builder), None)
+                }
             } else {
-                // No SNI found, write the entire data.
+                // SNI slicing disabled or data not at offset 0, write the entire data.
                 (write_chunk(offset, data, builder), None)
-            }
-        } else {
-            // SNI slicing disabled, write the entire data.
-            (write_chunk(offset, data, builder), None)
-        };
+            };
 
-        match written {
-            (None, None) => (),
-            (None, Some((offset, len))) | (Some((offset, len)), None) => {
-                mark_as_sent(cs, space, tokens, offset, len, stats);
-            }
-            (Some((offset1, len1)), Some((offset2, len2))) => {
-                mark_as_sent(cs, space, tokens, offset1, len1, stats);
-                mark_as_sent(cs, space, tokens, offset2, len2, stats);
+            match written {
+                (None, None) => break,
+                (None, Some((offset, len))) | (Some((offset, len)), None) => {
+                    mark_as_sent(cs, space, tokens, offset, len, stats);
+                }
+                (Some((offset1, len1)), Some((offset2, len2))) => {
+                    mark_as_sent(cs, space, tokens, offset1, len1, stats);
+                    mark_as_sent(cs, space, tokens, offset2, len2, stats);
+                    // We only end up in this arm if we successfully sliced above. In that case,
+                    // don't try and fit more crypto data into this packet.
+                    break;
+                }
             }
         }
     }

neqo-transport/src/lib.rs

@@ -44,7 +44,7 @@ pub mod send_stream;
 mod send_stream;
 mod sender;
 pub mod server;
-mod shuffle;
+mod sni;
 mod stats;
 pub mod stream_id;
 pub mod streams;
@@ -69,7 +69,7 @@ pub use self::{
     quic_datagrams::DatagramTracking,
     recv_stream::{RecvStreamStats, RECV_BUFFER_SIZE},
     send_stream::{SendStreamStats, SEND_BUFFER_SIZE},
-    shuffle::find_sni,
+    sni::find_sni,
     stats::Stats,
     stream_id::{StreamId, StreamType},
     version::Version,

neqo-transport/src/shuffle.rs → neqo-transport/src/sni.rs

@@ -47,7 +47,10 @@ pub fn find_sni(buf: &[u8]) -> Option<Range<usize>> {
         let ext_len: u16 = dec.decode_uint()?;
         if ext_type == 0 {
             // SNI!
-            let sni_len: u16 = dec.decode_uint()?;
+            let sni_len = dec.decode_uint::<u16>()?;
+            if sni_len < 3 {
+                return None;
+            }
             skip(&mut dec, 3)?; // Skip name_type and host_name length
             let start = dec.offset();
             let end = start + usize::from(sni_len) - 3;
@@ -107,10 +110,8 @@ mod tests {
         // ClientHello without SNI extension
         let mut buf = Vec::from(&BUF_WITH_SNI[..BUF_WITH_SNI.len() - 39]);
         let len = buf.len();
-        assert!(buf[len - 2] == 0x01 && buf[len - 1] == 0xcb); // Check extensions length
-                                                               // Set extensions length to 0
-        buf[len - 2] = 0x00;
-        buf[len - 1] = 0x00;
+        assert_eq!(&buf[len - 2..len], &[0x01, 0xcb], "check extensions length");
+        buf[len - 2..len].copy_from_slice(&[0x00, 0x00]); // Set extensions length to 0
         assert!(super::find_sni(&buf).is_none());
     }
 
@@ -121,6 +122,16 @@ mod tests {
         assert!(super::find_sni(truncated).is_none());
     }
 
+    #[test]
+    fn find_sni_invalid_sni_length() {
+        // ClientHello with an SNI extension with an invalid length
+        let mut buf = Vec::from(BUF_WITH_SNI);
+        let len = buf.len();
+        assert_eq!(&buf[len - 23..len - 21], &[0x00, 0x0c], "check SNI length");
+        buf[len - 23..len - 21].copy_from_slice(&[0x00, 0x02]); // Set Server Name List length to 2
+        assert!(super::find_sni(&buf).is_none());
+    }
+
     #[test]
     fn find_sni_no_ci() {
         // Not a ClientHello (msg_type != 1)

neqo-transport/tests/sni.rs

@@ -0,0 +1,44 @@
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use neqo_transport::ConnectionParameters;
+use test_fixture::{default_client, default_server, new_client, now};
+
+#[test]
+fn sni_no_slicing_at_nonzero_offset() {
+    let mut client = default_client();
+    let mut server = default_server();
+    let mut now = now();
+
+    // This packet will have two CRPYTO frames [y..end] and [0..x], where x < y.
+    let ch1 = client.process_output(now).dgram();
+    assert_eq!(client.stats().frame_tx.crypto, 2);
+    // This packet will have one CRYPTO frame [x..y].
+    let _ch2 = client.process_output(now).dgram();
+    assert_eq!(client.stats().frame_tx.crypto, 3);
+    // We are dropping the second packet and only deliver the first.
+    let ack = server.process(ch1, now).dgram();
+    // Client will now RTX the second packet.
+    now += client.process(ack, now).callback();
+    // Make sure it only has one CRYPTO frame.
+    _ = client.process_output(now).dgram();
+    assert_eq!(client.stats().frame_tx.crypto, 4);
+}
+
+#[test]
+fn sni_no_slicing_at_nonzero_offset_no_mlkem() {
+    let mut client = new_client(ConnectionParameters::default().mlkem(false));
+    let mut now = now();
+
+    // This packet will have two CRPYTO frames [x..end] and [0..x].
+    _ = client.process_output(now).dgram();
+    assert_eq!(client.stats().frame_tx.crypto, 2);
+    // Client will now RTX the packet.
+    now += client.process_output(now).callback();
+    // Make sure it has two CRYPTO frames.
+    _ = client.process_output(now).dgram();
+    assert_eq!(client.stats().frame_tx.crypto, 4);
+}

qns/Dockerfile

@@ -1,4 +1,4 @@
-FROM lukemathwalker/cargo-chef@sha256:66b16e2261871e522c3c8b2bc2da4222843e03b3581f4f2a98ac8fc59fa3d3ae AS chef
+FROM lukemathwalker/cargo-chef@sha256:2c9b2693a474b223a20f1e2bf850816d51848920bd34794fdf7c09afcaaa0fa6 AS chef
 
 WORKDIR /app