feat: Option to turn off SNI slicing (#2387)

* feat: Option to turn off SNI slicing

Wireshark can't reassemble sliced CRYPTO frames, which causes QNS tests to fail bcause it then can't parse all packets.

This PR adds an option to disable SNI slicing, and we do so by default when running in QNS.

* Leave SNI slicing on for some tests

neqo-bin/src/client/mod.rs

@@ -240,8 +240,12 @@ impl Args {
         self.shared.quic_parameters.quic_version = vec![Version::Version1];
         // This is the default for all tests except http3.
         self.shared.alpn = String::from("hq-interop");
+        // Wireshark can't reassemble sliced CRYPTO frames, which causes tests to fail.
+        // So let's turn that off by default, and only enable for some known-good QNS tests.
+        self.shared.quic_parameters.sni_slicing = false;
         match testcase.as_str() {
             "http3" => {
+                self.shared.quic_parameters.sni_slicing = true;
                 self.shared.alpn = String::from("h3");
                 if let Some(testcase) = &self.test {
                     if testcase.as_str() != "upload" {
@@ -265,6 +269,7 @@ impl Args {
                     qerror!("Warning: zerortt test won't work without >1 URL");
                     exit(127);
                 }
+                self.shared.quic_parameters.sni_slicing = true;
                 self.resume = true;
                 // PMTUD probes inflate what we sent in 1-RTT, causing QNS to fail the test.
                 self.shared.quic_parameters.no_pmtud = true;
@@ -285,6 +290,7 @@ impl Args {
                 self.key_update = true;
             }
             "v2" => {
+                self.shared.quic_parameters.sni_slicing = true;
                 // Use default version set for this test (which allows compatible vneg.)
                 self.shared.quic_parameters.quic_version.clear();
             }

neqo-bin/src/lib.rs

@@ -131,6 +131,10 @@ pub struct QuicParameters {
     #[arg(name = "preferred-address-v6", long)]
     /// An IPv6 address for the server preferred address.
     pub preferred_address_v6: Option<String>,
+
+    #[arg(long, default_value = "true")]
+    /// Whether to slice the SNI.
+    pub sni_slicing: bool,
 }
 
 #[cfg(any(test, feature = "bench"))]
@@ -146,6 +150,7 @@ impl Default for QuicParameters {
             no_pmtud: false,
             preferred_address_v4: None,
             preferred_address_v6: None,
+            sni_slicing: true,
         }
     }
 }
@@ -218,7 +223,8 @@ impl QuicParameters {
             .idle_timeout(Duration::from_secs(self.idle_timeout))
             .cc_algorithm(self.congestion_control)
             .pacing(!self.no_pacing)
-            .pmtud(!self.no_pmtud);
+            .pmtud(!self.no_pmtud)
+            .sni_slicing(self.sni_slicing);
         params = if let Some(pa) = self.preferred_address() {
             params.preferred_address(pa)
         } else {

neqo-transport/src/connection/mod.rs

@@ -2160,6 +2160,7 @@ impl Connection {
         let frame_stats = &mut stats.frame_tx;
         self.crypto.write_frame(
             PacketNumberSpace::ApplicationData,
+            self.conn_params.sni_slicing_enabled(),
             builder,
             tokens,
             frame_stats,
@@ -2294,7 +2295,13 @@ impl Connection {
                 self.write_appdata_frames(builder, &mut tokens);
             } else {
                 let stats = &mut self.stats.borrow_mut().frame_tx;
-                self.crypto.write_frame(space, builder, &mut tokens, stats);
+                self.crypto.write_frame(
+                    space,
+                    self.conn_params.sni_slicing_enabled(),
+                    builder,
+                    &mut tokens,
+                    stats,
+                );
             }
         }
 

neqo-transport/src/connection/params.rs

@@ -83,6 +83,8 @@ pub struct ConnectionParameters {
     pacing: bool,
     /// Whether the connection performs PLPMTUD.
     pmtud: bool,
+    /// Whether the connection should use SNI slicing.
+    sni_slicing: bool,
 }
 
 impl Default for ConnectionParameters {
@@ -107,6 +109,7 @@ impl Default for ConnectionParameters {
             disable_migration: false,
             pacing: true,
             pmtud: false,
+            sni_slicing: true,
         }
     }
 }
@@ -367,6 +370,17 @@ impl ConnectionParameters {
         self
     }
 
+    #[must_use]
+    pub const fn sni_slicing_enabled(&self) -> bool {
+        self.sni_slicing
+    }
+
+    #[must_use]
+    pub const fn sni_slicing(mut self, sni_slicing: bool) -> Self {
+        self.sni_slicing = sni_slicing;
+        self
+    }
+
     /// # Errors
     /// When a connection ID cannot be obtained.
     /// # Panics

neqo-transport/src/connection/tests/idle.rs

@@ -307,6 +307,7 @@ fn idle_caching() {
     let mut tokens = Vec::new();
     server.crypto.streams.write_frame(
         PacketNumberSpace::Initial,
+        server.conn_params.sni_slicing_enabled(),
         &mut builder,
         &mut tokens,
         &mut FrameStats::default(),
@@ -315,6 +316,7 @@ fn idle_caching() {
     tokens.clear();
     server.crypto.streams.write_frame(
         PacketNumberSpace::Initial,
+        server.conn_params.sni_slicing_enabled(),
         &mut builder,
         &mut tokens,
         &mut FrameStats::default(),

neqo-transport/src/crypto.rs

@@ -327,11 +327,13 @@ impl Crypto {
     pub fn write_frame(
         &mut self,
         space: PacketNumberSpace,
+        sni_slicing: bool,
         builder: &mut PacketBuilder,
         tokens: &mut Vec<RecoveryToken>,
         stats: &mut FrameStats,
     ) {
-        self.streams.write_frame(space, builder, tokens, stats);
+        self.streams
+            .write_frame(space, sni_slicing, builder, tokens, stats);
     }
 
     pub fn acked(&mut self, token: &CryptoRecoveryToken) {
@@ -1475,6 +1477,7 @@ impl CryptoStreams {
     pub fn write_frame(
         &mut self,
         space: PacketNumberSpace,
+        sni_slicing: bool,
         builder: &mut PacketBuilder,
         tokens: &mut Vec<RecoveryToken>,
         stats: &mut FrameStats,
@@ -1506,7 +1509,7 @@ impl CryptoStreams {
 
         let cs = self.get_mut(space).unwrap();
         if let Some((offset, data)) = cs.tx.next_bytes() {
-            let written = if offset == 0 {
+            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;