fix: Enable MLKEM768X25519 by default (#2389)

* fix: Enable MLKEM768X25519 by default WIP * Improve SNI slicing for large CIs * WIP * Finalize * clippy * Fixes * MLKEM off in simulator * Update neqo-transport/src/connection/mod.rs Co-authored-by: Martin Thomson <[email protected]> Signed-off-by: Lars Eggert <[email protected]> * Update neqo-transport/src/crypto.rs Co-authored-by: Martin Thomson <[email protected]> Signed-off-by: Lars Eggert <[email protected]> * Update neqo-transport/src/connection/mod.rs Co-authored-by: Martin Thomson <[email protected]> Signed-off-by: Lars Eggert <[email protected]> * Fixes * SNI slicing revamp * clippy * Minimize diff * Minimize * clippy * Comments * Fix merge --------- Signed-off-by: Lars Eggert <[email protected]> Co-authored-by: Martin Thomson <[email protected]>
mozilla · Feb 3, 2025 · ac8c81e · ac8c81e
1 parent 384b4bc
commit ac8c81e
Show file tree

Hide file tree

Showing 30 changed files with 764 additions and 310 deletions.
diff --git a/neqo-http3/src/connection_client.rs b/neqo-http3/src/connection_client.rs
@@ -1626,12 +1626,16 @@ mod tests {
     fn handshake_only(client: &mut Http3Client, server: &mut TestServer) -> Output {
         assert_eq!(client.state(), Http3State::Initializing);
         let out = client.process_output(now());
+        let out2 = client.process_output(now());
         assert_eq!(client.state(), Http3State::Initializing);
 
         assert_eq!(*server.conn.state(), State::Init);
-        let out = server.conn.process(out.dgram(), now());
+        server.conn.process_input(out.dgram().unwrap(), now());
+        let out = server.conn.process(out2.dgram(), now());
         assert_eq!(*server.conn.state(), State::Handshaking);
 
+        let out = client.process(out.dgram(), now());
+        let out = server.conn.process(out.dgram(), now());
         let out = client.process(out.dgram(), now());
         let out = server.conn.process(out.dgram(), now());
         assert!(out.as_dgram_ref().is_none());
@@ -4159,10 +4163,12 @@ mod tests {
         let (mut client, mut server) = start_with_0rtt();
 
         let out = client.process_output(now());
+        let out2 = client.process_output(now());
 
         assert_eq!(client.state(), Http3State::ZeroRtt);
         assert_eq!(*server.conn.state(), State::Init);
-        let out = server.conn.process(out.dgram(), now());
+        server.conn.process_input(out.dgram().unwrap(), now());
+        let out = server.conn.process(out2.dgram(), now());
 
         // Check that control and qpack streams are received and a
         // SETTINGS frame has been received.
@@ -4176,6 +4182,8 @@ mod tests {
 
         assert_eq!(*server.conn.state(), State::Handshaking);
         let out = client.process(out.dgram(), now());
+        let out = server.conn.process(out.dgram(), now());
+        let out = client.process(out.dgram(), now());
         assert_eq!(client.state(), Http3State::Connected);
 
         drop(server.conn.process(out.dgram(), now()));
@@ -4194,10 +4202,12 @@ mod tests {
         assert_eq!(request_stream_id, 0);
 
         let out = client.process_output(now());
+        let out2 = client.process_output(now());
 
         assert_eq!(client.state(), Http3State::ZeroRtt);
         assert_eq!(*server.conn.state(), State::Init);
-        let out = server.conn.process(out.dgram(), now());
+        server.conn.process_input(out.dgram().unwrap(), now());
+        let out = server.conn.process(out2.dgram(), now());
 
         // Check that control and qpack streams are received and a
         // SETTINGS frame has been received.
@@ -4211,6 +4221,8 @@ mod tests {
 
         assert_eq!(*server.conn.state(), State::Handshaking);
         let out = client.process(out.dgram(), now());
+        let out = server.conn.process(out.dgram(), now());
+        let out = client.process(out.dgram(), now());
         assert_eq!(client.state(), Http3State::Connected);
         let out = server.conn.process(out.dgram(), now());
         assert!(server.conn.state().connected());
@@ -4281,17 +4293,19 @@ mod tests {
         let client_0rtt = client.process_output(now());
         assert!(client_0rtt.as_dgram_ref().is_some());
 
-        let server_hs = server.process(client_hs.dgram(), now());
+        server.process_input(client_hs.dgram().unwrap(), now());
+        let server_hs = server.process(client_0rtt.dgram(), now());
         assert!(server_hs.as_dgram_ref().is_some()); // Should produce ServerHello etc...
-        let server_ignored = server.process(client_0rtt.dgram(), now());
-        assert!(server_ignored.as_dgram_ref().is_none());
+
+        let dgram = client.process(server_hs.dgram(), now()).dgram();
+        let dgram = server.process(dgram, now());
 
         // The server shouldn't receive that 0-RTT data.
         let recvd_stream_evt = |e| matches!(e, ConnectionEvent::NewStream { .. });
         assert!(!server.events().any(recvd_stream_evt));
 
         // Client should get a rejection.
-        let client_out = client.process(server_hs.dgram(), now());
+        let client_out = client.process(dgram.dgram(), now());
         assert!(client_out.as_dgram_ref().is_some());
         let recvd_0rtt_reject = |e| e == Http3ClientEvent::ZeroRttRejected;
         assert!(client.events().any(recvd_0rtt_reject));
@@ -4330,10 +4344,12 @@ mod tests {
             .expect("Set resumption token");
         assert_eq!(client.state(), Http3State::ZeroRtt);
         let out = client.process_output(now());
+        let out2 = client.process_output(now());
 
         assert_eq!(client.state(), Http3State::ZeroRtt);
         assert_eq!(*server.conn.state(), State::Init);
-        let out = server.conn.process(out.dgram(), now());
+        server.conn.process_input(out.dgram().unwrap(), now());
+        let out = server.conn.process(out2.dgram(), now());
 
         // Check that control and qpack streams and a SETTINGS frame are received.
         // Also qpack encoder stream will send "change capacity" instruction because it has
@@ -4346,6 +4362,8 @@ mod tests {
 
         assert_eq!(*server.conn.state(), State::Handshaking);
         let out = client.process(out.dgram(), now());
+        let out = server.conn.process(out.dgram(), now());
+        let out = client.process(out.dgram(), now());
         assert_eq!(client.state(), Http3State::Connected);
 
         drop(server.conn.process(out.dgram(), now()));

diff --git a/neqo-http3/src/features/extended_connect/tests/webtransport/mod.rs b/neqo-http3/src/features/extended_connect/tests/webtransport/mod.rs
@@ -82,8 +82,12 @@ fn exchange_packets(client: &mut Http3Client, server: &mut Http3Server) {
 fn connect_with(client: &mut Http3Client, server: &mut Http3Server) {
     assert_eq!(client.state(), Http3State::Initializing);
     let out = client.process_output(now());
+    let out2 = client.process_output(now());
     assert_eq!(client.state(), Http3State::Initializing);
 
+    _ = server.process(out.dgram(), now());
+    let out = server.process(out2.dgram(), now());
+    let out = client.process(out.dgram(), now());
     let out = server.process(out.dgram(), now());
     let out = client.process(out.dgram(), now());
     let out = server.process(out.dgram(), now());

diff --git a/neqo-http3/src/frames/tests/mod.rs b/neqo-http3/src/frames/tests/mod.rs
@@ -21,6 +21,10 @@ pub fn enc_dec<T: FrameDecoder<T>>(d: &Encoder, st: &str, remaining: usize) -> T
     let mut conn_c = default_client();
     let mut conn_s = default_server();
     let out = conn_c.process_output(now());
+    let out2 = conn_c.process_output(now());
+    _ = conn_s.process(out.dgram(), now());
+    let out = conn_s.process(out2.dgram(), now());
+    let out = conn_c.process(out.dgram(), now());
     let out = conn_s.process(out.dgram(), now());
     let out = conn_c.process(out.dgram(), now());
     drop(conn_s.process(out.dgram(), now()));

diff --git a/neqo-http3/src/server.rs b/neqo-http3/src/server.rs
@@ -409,8 +409,12 @@ mod tests {
 
     fn connect_transport(server: &mut Http3Server, client: &mut Connection, resume: bool) {
         let c1 = client.process_output(now());
-        let s1 = server.process(c1.dgram(), now());
+        let c11 = client.process_output(now());
+        _ = server.process(c1.dgram(), now());
+        let s1 = server.process(c11.dgram(), now());
         let c2 = client.process(s1.dgram(), now());
+        let s2 = server.process(c2.dgram(), now());
+        let c2 = client.process(s2.dgram(), now());
         let needs_auth = client
             .events()
             .any(|e| e == ConnectionEvent::AuthenticationNeeded);
@@ -429,8 +433,7 @@ mod tests {
         assert!(client.state().connected());
         let s2 = server.process(c2.dgram(), now());
         assert_connected(server);
-        let c3 = client.process(s2.dgram(), now());
-        assert!(c3.dgram().is_none());
+        _ = client.process(s2.dgram(), now());
     }
 
     // Start a client/server and check setting frame.

diff --git a/neqo-http3/tests/httpconn.rs b/neqo-http3/tests/httpconn.rs
@@ -94,8 +94,12 @@ fn process_client_events(conn: &mut Http3Client) {
 fn connect_peers(hconn_c: &mut Http3Client, hconn_s: &mut Http3Server) -> Option<Datagram> {
     assert_eq!(hconn_c.state(), Http3State::Initializing);
     let out = hconn_c.process_output(now()); // Initial
-    let out = hconn_s.process(out.dgram(), now()); // Initial + Handshake
-    let out = hconn_c.process(out.dgram(), now()); // ACK
+    let out2 = hconn_c.process_output(now()); // Initial
+    _ = hconn_s.process(out.dgram(), now()); // ACK
+    let out = hconn_s.process(out2.dgram(), now()); // Initial + Handshake
+    let out = hconn_c.process(out.dgram(), now());
+    let out = hconn_s.process(out.dgram(), now());
+    let out = hconn_c.process(out.dgram(), now());
     drop(hconn_s.process(out.dgram(), now())); // consume ACK
     let authentication_needed = |e| matches!(e, Http3ClientEvent::AuthenticationNeeded);
     assert!(hconn_c.events().any(authentication_needed));
@@ -121,8 +125,14 @@ fn connect_peers_with_network_propagation_delay(
     assert_eq!(hconn_c.state(), Http3State::Initializing);
     let mut now = now();
     let out = hconn_c.process_output(now); // Initial
+    let out2 = hconn_c.process_output(now); // Initial
+    now += net_delay;
+    _ = hconn_s.process(out.dgram(), now); // ACK
+    let out = hconn_s.process(out2.dgram(), now);
     now += net_delay;
-    let out = hconn_s.process(out.dgram(), now); // Initial + Handshake
+    let out = hconn_c.process(out.dgram(), now);
+    now += net_delay;
+    let out = hconn_s.process(out.dgram(), now);
     now += net_delay;
     let out = hconn_c.process(out.dgram(), now); // ACK
     now += net_delay;
@@ -448,6 +458,11 @@ fn zerortt() {
     hconn_c.stream_close_send(req).unwrap();
 
     let out = hconn_c.process(dgram, now());
+    let out2 = hconn_c.process_output(now());
+    _ = hconn_s.process(out.dgram(), now());
+    let out = hconn_s.process(out2.dgram(), now());
+
+    let out = hconn_c.process(out.dgram(), now());
     let out = hconn_s.process(out.dgram(), now());
 
     let mut request_stream = None;

diff --git a/neqo-http3/tests/priority.rs b/neqo-http3/tests/priority.rs
@@ -29,8 +29,12 @@ fn exchange_packets(client: &mut Http3Client, server: &mut Http3Server) {
 fn connect_with(client: &mut Http3Client, server: &mut Http3Server) {
     assert_eq!(client.state(), Http3State::Initializing);
     let out = client.process_output(now());
+    let out2 = client.process_output(now());
     assert_eq!(client.state(), Http3State::Initializing);
 
+    _ = server.process(out.dgram(), now());
+    let out = server.process(out2.dgram(), now());
+    let out = client.process(out.dgram(), now());
     let out = server.process(out.dgram(), now());
     let out = client.process(out.dgram(), now());
     let out = server.process(out.dgram(), now());

diff --git a/neqo-http3/tests/webtransport.rs b/neqo-http3/tests/webtransport.rs
@@ -44,8 +44,12 @@ fn connect() -> (Http3Client, Http3Server) {
     .expect("create a server");
     assert_eq!(client.state(), Http3State::Initializing);
     let out = client.process_output(now());
+    let out2 = client.process_output(now());
     assert_eq!(client.state(), Http3State::Initializing);
 
+    _ = server.process(out.dgram(), now());
+    let out = server.process(out2.dgram(), now());
+    let out = client.process(out.dgram(), now());
     let out = server.process(out.dgram(), now());
     let out = client.process(out.dgram(), now());
     let out = server.process(out.dgram(), now());

diff --git a/neqo-transport/benches/transfer.rs b/neqo-transport/benches/transfer.rs
@@ -33,14 +33,20 @@ fn benchmark_transfer(c: &mut Criterion, label: &str, seed: Option<impl AsRef<st
                 || {
                     let nodes = boxed![
                         ConnectionNode::new_client(
-                            ConnectionParameters::default().pmtud(true).pacing(pacing),
+                            ConnectionParameters::default()
+                                .pmtud(true)
+                                .pacing(pacing)
+                                .mlkem(false),
                             boxed![ReachState::new(State::Confirmed)],
                             boxed![SendData::new(TRANSFER_AMOUNT)]
                         ),
                         TailDrop::dsl_uplink(),
                         Delay::new(ZERO..JITTER),
                         ConnectionNode::new_server(
-                            ConnectionParameters::default().pmtud(true).pacing(pacing),
+                            ConnectionParameters::default()
+                                .pmtud(true)
+                                .pacing(pacing)
+                                .mlkem(false),
                             boxed![ReachState::new(State::Confirmed)],
                             boxed![ReceiveData::new(TRANSFER_AMOUNT)]
                         ),

diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs
@@ -381,6 +381,7 @@ impl Connection {
         let tphandler = Rc::new(RefCell::new(tps));
         let crypto = Crypto::new(
             conn_params.get_versions().initial(),
+            &conn_params,
             agent,
             protocols.iter().map(P::as_ref).map(String::from).collect(),
             Rc::clone(&tphandler),
@@ -913,7 +914,7 @@ impl Connection {
                     // error. This may happen when client_start fails.
                     self.set_state(State::Closed(error), now);
                 }
-                State::WaitInitial => {
+                State::WaitInitial | State::WaitVersion => {
                     // We don't have any state yet, so don't bother with
                     // the closing state, just send one CONNECTION_CLOSE.
                     if let Some(path) = path.or_else(|| self.paths.primary()) {
@@ -1517,6 +1518,24 @@ impl Connection {
             self.start_handshake(path, packet, now);
         }
 
+        if matches!(self.state, State::WaitInitial | State::WaitVersion) {
+            let new_state = if self.has_version() {
+                State::Handshaking
+            } else {
+                State::WaitVersion
+            };
+            self.set_state(new_state, now);
+            if self.role == Role::Server && self.state == State::Handshaking {
+                self.zero_rtt_state =
+                    if self.crypto.enable_0rtt(self.version, self.role) == Ok(true) {
+                        qdebug!("[{self}] Accepted 0-RTT");
+                        ZeroRttState::AcceptedServer
+                    } else {
+                        ZeroRttState::Rejected
+                    };
+            }
+        }
+
         if self.state.connected() {
             self.handle_migration(path, d, migrate, now);
         } else if self.role != Role::Client
@@ -1807,39 +1826,28 @@ impl Connection {
         debug_assert_eq!(packet.packet_type(), PacketType::Initial);
         self.remote_initial_source_cid = Some(ConnectionId::from(packet.scid()));
 
-        let got_version = if self.role == Role::Server {
-            if let Some(original_destination_cid) = self.original_destination_cid.as_ref() {
-                self.cid_manager.add_odcid(original_destination_cid.clone());
-                // Make a path on which to run the handshake.
-                self.setup_handshake_path(path, now);
-
-                self.zero_rtt_state =
-                    if self.crypto.enable_0rtt(self.version, self.role) == Ok(true) {
-                        qdebug!("[{self}] Accepted 0-RTT");
-                        ZeroRttState::AcceptedServer
-                    } else {
-                        qdebug!("[{self}] Rejected 0-RTT");
-                        ZeroRttState::Rejected
-                    };
-
-                // The server knows the final version if it has remote transport parameters.
-                self.tps.borrow().remote.is_some()
-            } else {
+        if self.role == Role::Server {
+            let Some(original_destination_cid) = self.original_destination_cid.as_ref() else {
                 qdebug!("[{self}] No original destination DCID");
-                false
-            }
+                return;
+            };
+            self.cid_manager.add_odcid(original_destination_cid.clone());
+            // Make a path on which to run the handshake.
+            self.setup_handshake_path(path, now);
         } else {
             qdebug!("[{self}] Changing to use Server CID={}", packet.scid());
             debug_assert!(path.borrow().is_primary());
             path.borrow_mut().set_remote_cid(packet.scid());
+        };
+    }
 
+    fn has_version(&self) -> bool {
+        if self.role == Role::Server {
+            // The server knows the final version if it has remote transport parameters.
+            self.tps.borrow().remote.is_some()
+        } else {
             // The client knows the final version if it processed a CRYPTO frame.
             self.stats.borrow().frame_rx.crypto > 0
-        };
-        if got_version {
-            self.set_state(State::Handshaking, now);
-        } else {
-            self.set_state(State::WaitVersion, now);
         }
     }
 
@@ -2490,6 +2498,18 @@ impl Connection {
                 // but wait until after sending an ACK.
                 self.discard_keys(PacketNumberSpace::Handshake, now);
             }
+
+            // If the client has more CRYPTO data queued up, do not coalesce if
+            // this packet is an Initial. Without this, 0-RTT packets could be
+            // coalesced with the first Initial, which some server (e.g., ours)
+            // do not support, because may do not save packets they can't
+            // decrypt yet.
+            if self.role == Role::Client
+                && *space == PacketNumberSpace::Initial
+                && !self.crypto.streams.is_empty(*space)
+            {
+                break;
+            }
         }
 
         if encoder.is_empty() {
@@ -2499,7 +2519,7 @@ impl Connection {
             // Perform additional padding for Initial packets as necessary.
             let mut packets: Vec<u8> = encoder.into();
             if let Some(mut initial) = initial_sent.take() {
-                if needs_padding {
+                if needs_padding && packets.len() < profile.limit() {
                     qdebug!(
                         "[{self}] pad Initial from {} to PLPMTU {}",
                         packets.len(),