diff --git a/neqo-http3/src/connection_client.rs b/neqo-http3/src/connection_client.rs index 5641dc9401..8056887d46 100644 --- a/neqo-http3/src/connection_client.rs +++ b/neqo-http3/src/connection_client.rs @@ -1624,15 +1624,21 @@ 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 out1 = 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(out1.dgram(), 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()); let authentication_needed = |e| matches!(e, Http3ClientEvent::AuthenticationNeeded); @@ -4156,12 +4162,13 @@ mod tests { #[test] fn zero_rtt_negotiated() { let (mut client, mut server) = start_with_0rtt(); - - let out = client.process_output(now()); + let out1 = 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(out1.dgram(), now()); + let out = server.conn.process(out2.dgram(), now()); // Check that control and qpack streams are received and a // SETTINGS frame has been received. @@ -4175,6 +4182,9 @@ 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())); @@ -4192,11 +4202,13 @@ mod tests { make_request(&mut client, true, &[Header::new("myheaders", "myvalue")]); assert_eq!(request_stream_id, 0); - let out = client.process_output(now()); + let out1 = 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(out1.dgram(), now()); + let out = server.conn.process(out2.dgram(), now()); // Check that control and qpack streams are received and a // SETTINGS frame has been received. @@ -4210,6 +4222,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()); @@ -4269,28 +4283,30 @@ mod tests { let zerortt_event = |e| matches!(e, Http3ClientEvent::StateChange(Http3State::ZeroRtt)); assert!(client.events().any(zerortt_event)); - // Send ClientHello. - let client_hs = client.process_output(now()); - assert!(client_hs.as_dgram_ref().is_some()); - // Create a request let request_stream_id = make_request(&mut client, false, &[]); assert_eq!(request_stream_id, 0); + // Send ClientHello. + let client_hs = client.process_output(now()); + assert!(client_hs.as_dgram_ref().is_some()); + 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(client_hs.dgram(), 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)); @@ -4328,11 +4344,13 @@ mod tests { .enable_resumption(now(), &token) .expect("Set resumption token"); assert_eq!(client.state(), Http3State::ZeroRtt); - let out = client.process_output(now()); + let out1 = 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(out1.dgram(), 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 @@ -4345,6 +4363,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 index 15b6c343cb..cee26e4876 100644 --- a/neqo-http3/src/features/extended_connect/tests/webtransport/mod.rs +++ b/neqo-http3/src/features/extended_connect/tests/webtransport/mod.rs @@ -81,9 +81,13 @@ fn exchange_packets(client: &mut Http3Client, server: &mut Http3Server) { // Perform only QUIC transport handshake. fn connect_with(client: &mut Http3Client, server: &mut Http3Server) { assert_eq!(client.state(), Http3State::Initializing); - let out = client.process_output(now()); + let out1 = client.process_output(now()); + let out2 = client.process_output(now()); assert_eq!(client.state(), Http3State::Initializing); + _ = server.process(out1.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 index 3f93c11547..58fa022e8f 100644 --- a/neqo-http3/src/frames/tests/mod.rs +++ b/neqo-http3/src/frames/tests/mod.rs @@ -20,7 +20,11 @@ pub fn enc_dec>(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 out1 = conn_c.process_output(now()); + let out2 = conn_c.process_output(now()); + _ = conn_s.process(out1.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 index ad8025f55f..91f18134fc 100644 --- a/neqo-http3/src/server.rs +++ b/neqo-http3/src/server.rs @@ -410,8 +410,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); @@ -430,8 +434,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 index 90494361c5..18f6ede120 100644 --- a/neqo-http3/tests/httpconn.rs +++ b/neqo-http3/tests/httpconn.rs @@ -91,9 +91,13 @@ fn process_client_events(conn: &mut Http3Client) { fn connect_peers(hconn_c: &mut Http3Client, hconn_s: &mut Http3Server) -> Option { 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 out1 = hconn_c.process_output(now()); // Initial + let out2 = hconn_c.process_output(now()); // Initial + _ = hconn_s.process(out1.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)); @@ -118,9 +122,15 @@ fn connect_peers_with_network_propagation_delay( let net_delay = Duration::from_millis(net_delay); assert_eq!(hconn_c.state(), Http3State::Initializing); let mut now = now(); - let out = hconn_c.process_output(now); // Initial + let out1 = hconn_c.process_output(now); // Initial + let out2 = hconn_c.process_output(now); // Initial + now += net_delay; + _ = hconn_s.process(out1.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; @@ -445,7 +455,12 @@ fn zerortt() { .unwrap(); hconn_c.stream_close_send(req).unwrap(); - let out = hconn_c.process(dgram, now()); + let out1 = hconn_c.process(dgram, now()); + let out2 = hconn_c.process_output(now()); + _ = hconn_s.process(out1.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 index 7edb2d03b9..35bf1c453b 100644 --- a/neqo-http3/tests/priority.rs +++ b/neqo-http3/tests/priority.rs @@ -28,9 +28,13 @@ fn exchange_packets(client: &mut Http3Client, server: &mut Http3Server) { // Perform only QUIC transport handshake. fn connect_with(client: &mut Http3Client, server: &mut Http3Server) { assert_eq!(client.state(), Http3State::Initializing); - let out = client.process_output(now()); + let out1 = client.process_output(now()); + let out2 = client.process_output(now()); assert_eq!(client.state(), Http3State::Initializing); + _ = server.process(out1.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 index 6291020f58..9018c8f27f 100644 --- a/neqo-http3/tests/webtransport.rs +++ b/neqo-http3/tests/webtransport.rs @@ -41,9 +41,13 @@ fn connect() -> (Http3Client, Http3Server) { ) .expect("create a server"); assert_eq!(client.state(), Http3State::Initializing); - let out = client.process_output(now()); + let out1 = client.process_output(now()); + let out2 = client.process_output(now()); assert_eq!(client.state(), Http3State::Initializing); + _ = server.process(out1.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/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs index dcdbb7bebe..52f311c962 100644 --- a/neqo-transport/src/connection/mod.rs +++ b/neqo-transport/src/connection/mod.rs @@ -383,6 +383,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), @@ -915,7 +916,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()) { @@ -1448,6 +1449,7 @@ impl Connection { { self.crypto.resend_unacked(PacketNumberSpace::Initial); self.resend_0rtt(now); + qdebug!("XXX resend done"); } } (PacketType::VersionNegotiation | PacketType::Retry | PacketType::OtherVersion, ..) => { @@ -1520,6 +1522,26 @@ impl Connection { self.start_handshake(path, packet, now); } + if matches!(self.state, State::WaitInitial | State::WaitVersion) { + self.set_state( + if self.has_version() { + State::Handshaking + } else { + State::WaitVersion + }, + now, + ); + if self.role == Role::Server { + 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 @@ -1817,34 +1839,25 @@ 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 self.role == Role::Server { self.cid_manager .add_odcid(self.original_destination_cid.as_ref().unwrap().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 { - qtrace!("[{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 { 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); } } @@ -2483,7 +2496,7 @@ impl Connection { if padded { needs_padding = false; self.loss_recovery.on_packet_sent(path, sent, now); - } else if pt == PacketType::Initial && (self.role == Role::Client || ack_eliciting) { + } else if pt == PacketType::Initial { // Packets containing Initial packets might need padding, and we want to // track that padding along with the Initial packet. So defer tracking. initial_sent = Some(sent); @@ -2503,6 +2516,14 @@ impl Connection { // but wait until after sending an ACK. self.discard_keys(PacketNumberSpace::Handshake, now); } + + // If the client has more Initial CRYPTO data queued up, do not coalesce. + if self.role == Role::Client + && *space == PacketNumberSpace::Initial + && !self.crypto.streams.is_empty(*space) + { + break; + } } if encoder.is_empty() { @@ -2512,7 +2533,7 @@ impl Connection { // Perform additional padding for Initial packets as necessary. let mut packets: Vec = 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(), diff --git a/neqo-transport/src/connection/params.rs b/neqo-transport/src/connection/params.rs index 5260415b2d..2ec2142edf 100644 --- a/neqo-transport/src/connection/params.rs +++ b/neqo-transport/src/connection/params.rs @@ -85,6 +85,8 @@ pub struct ConnectionParameters { pmtud: bool, /// Whether the connection should use SNI slicing. sni_slicing: bool, + /// Whether to enable mlkem768nistp256-sha256. + mlkem: bool, } impl Default for ConnectionParameters { @@ -110,6 +112,7 @@ impl Default for ConnectionParameters { pacing: true, pmtud: false, sni_slicing: true, + mlkem: true, } } } @@ -381,6 +384,17 @@ impl ConnectionParameters { self } + #[must_use] + pub const fn mlkem_enabled(&self) -> bool { + self.mlkem + } + + #[must_use] + pub const fn mlkem(mut self, mlkem: bool) -> Self { + self.mlkem = mlkem; + self + } + /// # Errors /// When a connection ID cannot be obtained. /// # Panics diff --git a/neqo-transport/src/connection/tests/close.rs b/neqo-transport/src/connection/tests/close.rs index a69db15220..a9883b987b 100644 --- a/neqo-transport/src/connection/tests/close.rs +++ b/neqo-transport/src/connection/tests/close.rs @@ -109,9 +109,11 @@ fn bad_tls_version() { .unwrap(); let mut server = default_server(); - let dgram = client.process_output(now()).dgram(); - assert!(dgram.is_some()); - let dgram = server.process(dgram, now()).dgram(); + let dgram1 = client.process_output(now()).dgram(); + let dgram2 = client.process_output(now()).dgram(); + assert!(dgram1.is_some() && dgram2.is_some()); + _ = server.process(dgram1, now()).dgram(); + let dgram = server.process(dgram2, now()).dgram(); assert_eq!( *server.state(), State::Closed(CloseReason::Transport(Error::ProtocolViolation)) diff --git a/neqo-transport/src/connection/tests/handshake.rs b/neqo-transport/src/connection/tests/handshake.rs index c9ff3a5f41..56c3f34407 100644 --- a/neqo-transport/src/connection/tests/handshake.rs +++ b/neqo-transport/src/connection/tests/handshake.rs @@ -46,13 +46,16 @@ const ECH_PUBLIC_NAME: &str = "public.example"; fn full_handshake(pmtud: bool) { qdebug!("---- client: generate CH"); let mut client = new_client(ConnectionParameters::default().pmtud(pmtud)); - let out = client.process_output(now()); - assert!(out.as_dgram_ref().is_some()); - assert_eq!(out.as_dgram_ref().unwrap().len(), client.plpmtu()); + let out1 = client.process_output(now()); + let out2 = client.process_output(now()); + assert!(out1.as_dgram_ref().is_some() && out2.as_dgram_ref().is_some()); + assert_eq!(out1.as_dgram_ref().unwrap().len(), client.plpmtu()); + assert_eq!(out2.as_dgram_ref().unwrap().len(), client.plpmtu()); qdebug!("---- server: CH -> SH, EE, CERT, CV, FIN"); let mut server = new_server(ConnectionParameters::default().pmtud(pmtud)); - let out = server.process(out.dgram(), now()); + _ = server.process(out1.dgram(), now()); + let out = server.process(out2.dgram(), now()); assert!(out.as_dgram_ref().is_some()); assert_eq!(out.as_dgram_ref().unwrap().len(), server.plpmtu()); @@ -60,6 +63,9 @@ fn full_handshake(pmtud: bool) { let out = client.process(out.dgram(), now()); assert!(out.as_dgram_ref().is_some()); + let out = server.process(out.dgram(), now()); + let out = client.process(out.dgram(), now()); + let out = server.process(out.dgram(), now()); assert!(out.as_dgram_ref().is_none()); @@ -155,22 +161,33 @@ fn no_alpn() { } #[test] +#[allow(clippy::cognitive_complexity)] fn dup_server_flight1() { qdebug!("---- client: generate CH"); let mut client = default_client(); - let out = client.process_output(now()); - assert!(out.as_dgram_ref().is_some()); - assert_eq!(out.as_dgram_ref().unwrap().len(), client.plpmtu()); - qdebug!("Output={:0x?}", out.as_dgram_ref()); + let out1 = client.process_output(now()); + let out2 = client.process_output(now()); + assert!(out1.as_dgram_ref().is_some() && out2.as_dgram_ref().is_some()); + assert_eq!(out1.as_dgram_ref().unwrap().len(), client.plpmtu()); + assert_eq!(out2.as_dgram_ref().unwrap().len(), client.plpmtu()); + qdebug!( + "Output={:0x?} {:0x?}", + out1.as_dgram_ref(), + out2.as_dgram_ref() + ); qdebug!("---- server: CH -> SH, EE, CERT, CV, FIN"); let mut server = default_server(); - let out_to_rep = server.process(out.dgram(), now()); + _ = server.process(out1.dgram(), now()); + let out_to_rep = server.process(out2.dgram(), now()); assert!(out_to_rep.as_dgram_ref().is_some()); qdebug!("Output={:0x?}", out_to_rep.as_dgram_ref()); qdebug!("---- client: cert verification"); let out = client.process(Some(out_to_rep.as_dgram_ref().cloned().unwrap()), now()); + let out_to_rep2 = server.process(out.dgram(), now()); + let out = client.process(out_to_rep2.clone().dgram(), now()); + assert!(out.as_dgram_ref().is_some()); qdebug!("Output={:0x?}", out.as_dgram_ref()); @@ -184,27 +201,29 @@ fn dup_server_flight1() { assert!(out.as_dgram_ref().is_some()); qdebug!("Output={:0x?}", out.as_dgram_ref()); - assert_eq!(3, client.stats().packets_rx); + assert_eq!(4, client.stats().packets_rx); assert_eq!(0, client.stats().dups_rx); assert_eq!(1, client.stats().dropped_rx); qdebug!("---- Dup, ignored"); - let out = client.process(out_to_rep.dgram(), now()); + _ = client.process(out_to_rep.dgram(), now()); + let out = client.process(out_to_rep2.dgram(), now()); assert!(out.as_dgram_ref().is_none()); qdebug!("Output={:0x?}", out.as_dgram_ref()); // Four packets total received, 1 of them is a dup and one has been dropped because Initial keys // are dropped. Add 2 counts of the padding that the server adds to Initial packets. - assert_eq!(6, client.stats().packets_rx); + assert_eq!(8, client.stats().packets_rx); assert_eq!(1, client.stats().dups_rx); - assert_eq!(3, client.stats().dropped_rx); + assert_eq!(4, client.stats().dropped_rx); } // Test that we split crypto data if they cannot fit into one packet. // To test this we will use a long server certificate. #[test] fn crypto_frame_split() { - let mut client = default_client(); + // This test has its own logic for generating large CRYPTO frames, so turn off MLKEM. + let mut client = new_client(ConnectionParameters::default().mlkem(false)); let mut server = Connection::new_server( test_fixture::LONG_CERT_KEYS, @@ -425,9 +444,11 @@ fn coalesce_05rtt() { // The first exchange doesn't offer a chance for the server to send. // So drop the server flight and wait for the PTO. let c1 = client.process_output(now).dgram(); - assert!(c1.is_some()); + let c11 = client.process_output(now).dgram(); + assert!(c1.is_some() && c11.is_some()); now += RTT / 2; - let s1 = server.process(c1, now).dgram(); + _ = server.process(c1, now).dgram(); + let s1 = server.process(c11, now).dgram(); assert!(s1.is_some()); // Drop the server flight. Then send some data. @@ -440,9 +461,15 @@ fn coalesce_05rtt() { // including the application data, which it sends in a 1-RTT packet. now += AT_LEAST_PTO; let c2 = client.process_output(now).dgram(); - assert!(c2.is_some()); + let c21 = client.process_output(now).dgram(); + assert!(c2.is_some() && c21.is_some()); now += RTT / 2; + _ = server.process(c21, now).dgram(); let s2 = server.process(c2, now).dgram(); + + let dgram = client.process(s2, now).dgram(); + let s2 = server.process(dgram, now).dgram(); + // Even though there is a 1-RTT packet at the end of the datagram, the // flight should be padded to full size. assert_eq!(s2.as_ref().unwrap().len(), server.plpmtu()); @@ -454,7 +481,7 @@ fn coalesce_05rtt() { drop(client.process(s2, now).dgram()); // This packet will contain an ACK, but we can ignore it. assert_eq!(client.stats().dropped_rx, 0); - assert_eq!(client.stats().packets_rx, 3); + assert_eq!(client.stats().packets_rx, 4); assert_eq!(client.stats().saved_datagrams, 1); // After (successful) authentication, the packet is processed. @@ -462,7 +489,7 @@ fn coalesce_05rtt() { let c3 = client.process_output(now).dgram(); assert!(c3.is_some()); assert_eq!(client.stats().dropped_rx, 0); // No Initial padding. - assert_eq!(client.stats().packets_rx, 4); + assert_eq!(client.stats().packets_rx, 5); assert_eq!(client.stats().saved_datagrams, 1); assert!(client.stats().frame_rx.padding > 0); // Padding uses frames. @@ -662,13 +689,18 @@ fn extra_initial_hs() { let mut server = default_server(); let mut now = now(); - let c_init = client.process_output(now).dgram(); - assert!(c_init.is_some()); + let c_init1 = client.process_output(now).dgram(); + let c_init2 = client.process_output(now).dgram(); + assert!(c_init1.is_some() && c_init2.is_some()); now += DEFAULT_RTT / 2; - let s_init = server.process(c_init, now).dgram(); + _ = server.process(c_init1, now).dgram(); + let s_init = server.process(c_init2, now).dgram(); assert!(s_init.is_some()); now += DEFAULT_RTT / 2; + let dgram = client.process(s_init, now).dgram(); + let s_init = server.process(dgram, now).dgram(); + // Drop the Initial packet, keep only the Handshake. let (_, undecryptable) = split_datagram(&s_init.unwrap()); assert!(undecryptable.is_some()); @@ -677,7 +709,14 @@ fn extra_initial_hs() { // Do that EXTRA_INITIALS times and each time the client will emit // another Initial packet. for _ in 0..=super::super::EXTRA_INITIALS { - let c_init = client.process(undecryptable.clone(), now).dgram(); + let c_init = match client.process(undecryptable.clone(), now) { + Output::None => todo!(), + Output::Datagram(c_init) => Some(c_init), + Output::Callback(duration) => { + now += duration; + client.process_output(now).dgram() + } + }; assertions::assert_initial(c_init.as_ref().unwrap(), false); now += DEFAULT_RTT / 10; } @@ -687,7 +726,7 @@ fn extra_initial_hs() { assert!(nothing.is_none()); // Until PTO, where another Initial can be used to complete the handshake. - now += AT_LEAST_PTO; + now += client.process_output(now).callback(); let c_init = client.process_output(now).dgram(); assertions::assert_initial(c_init.as_ref().unwrap(), false); now += DEFAULT_RTT / 2; @@ -708,13 +747,18 @@ fn extra_initial_invalid_cid() { let mut server = default_server(); let mut now = now(); - let c_init = client.process_output(now).dgram(); - assert!(c_init.is_some()); + let c_init1 = client.process_output(now).dgram(); + let c_init2 = client.process_output(now).dgram(); + assert!(c_init1.is_some() && c_init2.is_some()); now += DEFAULT_RTT / 2; - let s_init = server.process(c_init, now).dgram(); + _ = server.process(c_init1, now).dgram(); + let s_init = server.process(c_init2, now).dgram(); assert!(s_init.is_some()); now += DEFAULT_RTT / 2; + let dgram = client.process(s_init, now).dgram(); + let s_init = server.process(dgram, now).dgram(); + // If the client receives a packet that contains the wrong connection // ID, it won't send another Initial. let (_, hs) = split_datagram(&s_init.unwrap()); @@ -761,7 +805,8 @@ fn connect_one_version() { #[test] fn anti_amplification() { - let mut client = default_client(); + // This test has its own logic for generating large CRYPTO frames, so turn off MLKEM. + let mut client = new_client(ConnectionParameters::default().mlkem(false)); let mut server = default_server(); let mut now = now(); @@ -828,11 +873,13 @@ fn garbage_initial() { #[test] fn drop_initial_packet_from_wrong_address() { let mut client = default_client(); - let out = client.process_output(now()); - assert!(out.as_dgram_ref().is_some()); + let out1 = client.process_output(now()); + let out2 = client.process_output(now()); + assert!(out1.as_dgram_ref().is_some() && out2.as_dgram_ref().is_some()); let mut server = default_server(); - let out = server.process(out.dgram(), now()); + _ = server.process(out1.dgram(), now()); + let out = server.process(out2.dgram(), now()); assert!(out.as_dgram_ref().is_some()); let p = out.dgram().unwrap(); @@ -918,7 +965,11 @@ fn ech_retry() { .client_enable_ech(damaged_ech_config(server.ech_config())) .unwrap(); - let dgram = client.process_output(now()).dgram(); + let dgram1 = client.process_output(now()).dgram(); + let dgram2 = client.process_output(now()).dgram(); + _ = server.process(dgram1, now()).dgram(); + let dgram = server.process(dgram2, now()).dgram(); + let dgram = client.process(dgram, now()).dgram(); let dgram = server.process(dgram, now()).dgram(); client.process_input(dgram.unwrap(), now()); let auth_event = ConnectionEvent::EchFallbackAuthenticationNeeded { @@ -973,7 +1024,11 @@ fn ech_retry_fallback_rejected() { .client_enable_ech(damaged_ech_config(server.ech_config())) .unwrap(); - let dgram = client.process_output(now()).dgram(); + let dgram1 = client.process_output(now()).dgram(); + let dgram2 = client.process_output(now()).dgram(); + _ = server.process(dgram1, now()).dgram(); + let dgram = server.process(dgram2, now()).dgram(); + let dgram = client.process(dgram, now()).dgram(); let dgram = server.process(dgram, now()).dgram(); client.process_input(dgram.unwrap(), now()); let auth_event = ConnectionEvent::EchFallbackAuthenticationNeeded { @@ -1006,7 +1061,11 @@ fn bad_min_ack_delay() { .unwrap(); let mut client = default_client(); - let dgram = client.process_output(now()).dgram(); + let dgram1 = client.process_output(now()).dgram(); + let dgram2 = client.process_output(now()).dgram(); + _ = server.process(dgram1, now()).dgram(); + let dgram = server.process(dgram2, now()).dgram(); + let dgram = client.process(dgram, now()).dgram(); let dgram = server.process(dgram, now()).dgram(); client.process_input(dgram.unwrap(), now()); client.authenticated(AuthenticationStatus::Ok, now()); @@ -1142,19 +1201,24 @@ fn emit_authentication_needed_once() { test_fixture::LONG_CERT_KEYS, test_fixture::DEFAULT_ALPN, Rc::new(RefCell::new(CountingConnectionIdGenerator::default())), - ConnectionParameters::default(), + // TODO: Why is this needed to avoind the 5ms pacing delay? + ConnectionParameters::default().pacing(false), ) .expect("create a server"); let client1 = client.process_output(now()); - assert!(client1.as_dgram_ref().is_some()); + let client2 = client.process_output(now()); + assert!(client1.as_dgram_ref().is_some() && client2.as_dgram_ref().is_some()); // The entire server flight doesn't fit in a single packet because the // certificate is large, therefore the server will produce 2 packets. - let server1 = server.process(client1.dgram(), now()); + _ = server.process(client1.dgram(), now()); + let server1 = server.process(client2.dgram(), now()); assert!(server1.as_dgram_ref().is_some()); let server2 = server.process_output(now()); assert!(server2.as_dgram_ref().is_some()); + let server3 = server.process_output(now()); + assert!(server3.as_dgram_ref().is_some()); let authentication_needed_count = |client: &mut Connection| { client @@ -1163,7 +1227,7 @@ fn emit_authentication_needed_once() { .count() }; - // Upon receiving the first packet, the client has the server certificate, + // Upon receiving the first two packet, the client has the server certificate, // but not yet all required handshake data. It moves to // `HandshakeState::AuthenticationPending` and emits a // `ConnectionEvent::AuthenticationNeeded` event. @@ -1174,6 +1238,7 @@ fn emit_authentication_needed_once() { // also fit in the same packet. Our default test setup achieves this, but // changes to the setup might invalidate this test. _ = client.process(server1.dgram(), now()); + _ = client.process(server2.dgram(), now()); assert_eq!(1, authentication_needed_count(&mut client)); assert!(client.peer_certificate().is_some()); @@ -1181,24 +1246,27 @@ fn emit_authentication_needed_once() { // `Connection::authenticated`. On receiving the second packet from the // server, the client must not emit a another // `ConnectionEvent::AuthenticationNeeded`. - _ = client.process(server2.dgram(), now()); + _ = client.process(server3.dgram(), now()); assert_eq!(0, authentication_needed_count(&mut client)); } #[test] fn client_initial_retransmits_identical() { let mut now = now(); - let mut client = default_client(); + // TODO: With pacing on, why does the delay callback return by 5ms and then PTO after 295ms? + let mut client = new_client(ConnectionParameters::default().pacing(false)); - // Force the client to retransmit its Initial packet a number of times and make sure the + // Force the client to retransmit its Initial flight a number of times and make sure the // retranmissions are identical to the original. Also, verify the PTO durations. for i in 1..=5 { - let ci = client.process_output(now).dgram().unwrap(); - assert_eq!(ci.len(), client.plpmtu()); + let ci1 = client.process_output(now).dgram().unwrap(); + assert_eq!(ci1.len(), client.plpmtu()); + let ci2 = client.process_output(now).dgram().unwrap(); + assert_eq!(ci2.len(), client.plpmtu()); assert_eq!( client.stats().frame_tx, FrameStats { - crypto: 2 * i, + crypto: 3 * i, ..Default::default() } ); @@ -1212,34 +1280,43 @@ fn client_initial_retransmits_identical() { fn server_initial_retransmits_identical() { let mut now = now(); let mut client = default_client(); - let mut ci = client.process_output(now).dgram(); + let mut ci1 = client.process_output(now).dgram(); + let mut ci2 = client.process_output(now).dgram(); - // Force the server to retransmit its Initial packet a number of times and make sure the + // Force the server to retransmit its Initial flight a number of times and make sure the // retranmissions are identical to the original. Also, verify the PTO durations. - let mut server = default_server(); + let mut server = new_server(ConnectionParameters::default().pacing(false)); let mut total_ptos = Duration::from_secs(0); - for i in 1..=3 { - let si = server.process(ci.take(), now).dgram().unwrap(); - assert_eq!(si.len(), server.plpmtu()); + for i in 1..=2 { + let si1 = server.process(ci1.take(), now).dgram().unwrap(); + assert_eq!(si1.len(), server.plpmtu()); + let si2 = server.process(ci2.take(), now).dgram().unwrap(); + assert_eq!(si2.len(), server.plpmtu()); + if i == 1 { + // On the first iteration, the server will want to send its entire flight. + // During later ones, we will have hit a PTO and can hence only send two packets. + _ = server.process(ci2.take(), now).dgram().unwrap(); + } assert_eq!( server.stats().frame_tx, FrameStats { - crypto: i * 2, - ack: i, + crypto: i * 3, + ack: i * 2 - usize::from(i != 1), + largest_acknowledged: i as u64 - u64::from(i != 1), ..Default::default() } ); let pto = server.process_output(now).callback(); - if i < 3 { - assert_eq!(pto, DEFAULT_RTT * 3 * (1 << (i - 1))); - } else { - // Server is amplification-limited after three (re)transmissions. - assert_eq!(pto, server.conn_params.get_idle_timeout() - total_ptos); - } now += pto; total_ptos += pto; } + + // Server is amplification-limited now. + let si1 = server.process(ci1.take(), now).dgram().unwrap(); + assert_eq!(si1.len(), server.plpmtu()); + let pto = server.process_output(now).callback(); + assert_eq!(pto, server.conn_params.get_idle_timeout() - total_ptos); } #[test] diff --git a/neqo-transport/src/connection/tests/idle.rs b/neqo-transport/src/connection/tests/idle.rs index 38f657b89b..78d1f848d8 100644 --- a/neqo-transport/src/connection/tests/idle.rs +++ b/neqo-transport/src/connection/tests/idle.rs @@ -289,8 +289,11 @@ fn idle_caching() { // Perform the first round trip, but drop the Initial from the server. // The client then caches the Handshake packet. - let dgram = client.process_output(start).dgram(); - let dgram = server.process(dgram, start).dgram(); + let dgram1 = client.process_output(start).dgram(); + let dgram2 = client.process_output(start).dgram(); + _ = server.process(dgram1, start).dgram(); + _ = server.process(dgram2, start).dgram(); + let dgram = server.process_output(start).dgram(); let (_, handshake) = split_datagram(&dgram.unwrap()); client.process_input(handshake.unwrap(), start); @@ -342,6 +345,8 @@ fn idle_caching() { maybe_authenticate(&mut client); let dgram = client.process_output(end).dgram(); let dgram = server.process(dgram, end).dgram(); + let dgram = client.process(dgram, end).dgram(); + let dgram = server.process(dgram, end).dgram(); client.process_input(dgram.unwrap(), end); assert_eq!(*client.state(), State::Confirmed); assert_eq!(*server.state(), State::Confirmed); diff --git a/neqo-transport/src/connection/tests/keys.rs b/neqo-transport/src/connection/tests/keys.rs index d8686b7943..314a7c7d84 100644 --- a/neqo-transport/src/connection/tests/keys.rs +++ b/neqo-transport/src/connection/tests/keys.rs @@ -55,25 +55,30 @@ fn overwrite_invocations(n: PacketNumber) { fn discarded_initial_keys() { qdebug!("---- client: generate CH"); let mut client = default_client(); - let init_pkt_c = client.process_output(now()).dgram(); - assert!(init_pkt_c.is_some()); - assert_eq!(init_pkt_c.as_ref().unwrap().len(), client.plpmtu()); + let init_pkt_c1 = client.process_output(now()).dgram(); + let init_pkt_c2 = client.process_output(now()).dgram(); + assert!(init_pkt_c1.is_some() && init_pkt_c2.is_some()); + assert_eq!(init_pkt_c1.as_ref().unwrap().len(), client.plpmtu()); + assert_eq!(init_pkt_c2.as_ref().unwrap().len(), client.plpmtu()); qdebug!("---- server: CH -> SH, EE, CERT, CV, FIN"); let mut server = default_server(); - let init_pkt_s = server.process(init_pkt_c.clone(), now()).dgram(); - assert!(init_pkt_s.is_some()); + _ = server.process(init_pkt_c1.clone(), now()).dgram(); + let s1 = server.process(init_pkt_c2, now()).dgram(); + let s2 = server.process_output(now()).dgram(); + assert!(s1.is_some() && s2.is_some()); qdebug!("---- client: cert verification"); - let out = client.process(init_pkt_s.clone(), now()).dgram(); + _ = client.process(s1, now()).dgram(); + let out = client.process(s2.clone(), now()).dgram(); assert!(out.is_some()); // The client has received a handshake packet. It will remove the Initial keys. - // We will check this by processing init_pkt_s a second time. + // We will check this by processing init_pkt_s2 a second time. // The initial packet should be dropped. The packet contains a Handshake packet as well, which // will be marked as dup. And it will contain padding, which will be "dropped". // The client will generate a Handshake packet here to avoid stalling. - check_discarded(&mut client, &init_pkt_s.unwrap(), true, 2, 1); + check_discarded(&mut client, &s2.unwrap(), true, 2, 1); assert!(maybe_authenticate(&mut client)); @@ -81,7 +86,7 @@ fn discarded_initial_keys() { // packet from the client. // We will check this by processing init_pkt_c a second time. // The dropped packet is padding. The Initial packet has been mark dup. - check_discarded(&mut server, &init_pkt_c.clone().unwrap(), false, 1, 1); + check_discarded(&mut server, &init_pkt_c1.clone().unwrap(), false, 1, 1); qdebug!("---- client: SH..FIN -> FIN"); let out = client.process_output(now()).dgram(); @@ -96,7 +101,7 @@ fn discarded_initial_keys() { // We will check this by processing init_pkt_c a third time. // The Initial packet has been dropped and padding that follows it. // There is no dups, everything has been dropped. - check_discarded(&mut server, &init_pkt_c.unwrap(), false, 1, 0); + check_discarded(&mut server, &init_pkt_c1.unwrap(), false, 1, 0); } #[test] diff --git a/neqo-transport/src/connection/tests/migration.rs b/neqo-transport/src/connection/tests/migration.rs index 949ed96134..567df7a989 100644 --- a/neqo-transport/src/connection/tests/migration.rs +++ b/neqo-transport/src/connection/tests/migration.rs @@ -572,7 +572,7 @@ fn migrate_same_fail() { assert_path_challenge_min_len(&client, &probe, now); // -1 because first PATH_CHALLENGE already sent above - for _ in 0..MAX_PATH_PROBES * 2 - 1 { + for _ in 0..MAX_PATH_PROBES - 1 { let cb = client.process_output(now).callback(); assert_ne!(cb, Duration::new(0, 0)); now += cb; @@ -703,7 +703,11 @@ fn migration_client_empty_cid() { /// Drive the handshake in the most expeditious fashion. /// Returns the packet containing `HANDSHAKE_DONE` from the server. fn fast_handshake(client: &mut Connection, server: &mut Connection) -> Option { - let dgram = client.process_output(now()).dgram(); + let dgram1 = client.process_output(now()).dgram(); + let dgram2 = client.process_output(now()).dgram(); + _ = server.process(dgram1, now()).dgram(); + let dgram = server.process(dgram2, now()).dgram(); + let dgram = client.process(dgram, now()).dgram(); let dgram = server.process(dgram, now()).dgram(); client.process_input(dgram.unwrap(), now()); assert!(maybe_authenticate(client)); diff --git a/neqo-transport/src/connection/tests/recovery.rs b/neqo-transport/src/connection/tests/recovery.rs index dcaabccbdf..66cc3049c8 100644 --- a/neqo-transport/src/connection/tests/recovery.rs +++ b/neqo-transport/src/connection/tests/recovery.rs @@ -166,17 +166,21 @@ fn pto_initial() { qdebug!("---- client: generate CH"); let mut client = default_client(); let pkt1 = client.process_output(now).dgram(); - assert!(pkt1.is_some()); - assert_eq!(pkt1.clone().unwrap().len(), client.plpmtu()); + let pkt2 = client.process_output(now).dgram(); + assert!(pkt1.is_some() && pkt2.is_some()); + assert_eq!(pkt1.unwrap().len(), client.plpmtu()); + assert_eq!(pkt2.unwrap().len(), client.plpmtu()); let delay = client.process_output(now).callback(); assert_eq!(delay, INITIAL_PTO); // Resend initial after PTO. now += delay; + let pkt1 = client.process_output(now).dgram(); let pkt2 = client.process_output(now).dgram(); - assert!(pkt2.is_some()); - assert_eq!(pkt2.unwrap().len(), client.plpmtu()); + assert!(pkt1.is_some() && pkt2.is_some()); + assert_eq!(pkt1.clone().unwrap().len(), client.plpmtu()); + assert_eq!(pkt2.clone().unwrap().len(), client.plpmtu()); let delay = client.process_output(now).callback(); // PTO has doubled. @@ -184,7 +188,8 @@ fn pto_initial() { // Server process the first initial pkt. let mut server = default_server(); - let out = server.process(pkt1, now).dgram(); + _ = server.process(pkt1, now).dgram(); + let out = server.process(pkt2, now).dgram(); assert!(out.is_some()); // Client receives ack for the first initial packet as well a Handshake packet. @@ -212,13 +217,16 @@ fn pto_handshake_complete() { let mut client = default_client(); let mut server = default_server(); - let pkt = client.process_output(now).dgram(); - assert_initial(pkt.as_ref().unwrap(), false); + let pkt1 = client.process_output(now).dgram(); + let pkt2 = client.process_output(now).dgram(); + assert_initial(pkt1.as_ref().unwrap(), false); + assert_initial(pkt2.as_ref().unwrap(), false); let cb = client.process_output(now).callback(); assert_eq!(cb, Duration::from_millis(300)); now += HALF_RTT; - let pkt = server.process(pkt, now).dgram(); + _ = server.process(pkt1, now).dgram(); + let pkt = server.process(pkt2, now).dgram(); assert_initial(pkt.as_ref().unwrap(), false); now += HALF_RTT; @@ -331,12 +339,14 @@ fn pto_handshake_frames() { let mut now = now(); qdebug!("---- client: generate CH"); let mut client = default_client(); - let pkt = client.process_output(now); + let pkt1 = client.process_output(now); + let pkt2 = client.process_output(now); now += Duration::from_millis(10); qdebug!("---- server: CH -> SH, EE, CERT, CV, FIN"); let mut server = default_server(); - let pkt = server.process(pkt.dgram(), now); + _ = server.process(pkt1.dgram(), now); + let pkt = server.process(pkt2.dgram(), now); now += Duration::from_millis(10); qdebug!("---- client: cert verification"); diff --git a/neqo-transport/src/connection/tests/resumption.rs b/neqo-transport/src/connection/tests/resumption.rs index 7b976712fe..1a3d650a85 100644 --- a/neqo-transport/src/connection/tests/resumption.rs +++ b/neqo-transport/src/connection/tests/resumption.rs @@ -90,8 +90,11 @@ fn ticket_rtt(rtt: Duration) -> Duration { // A simple ACK_ECN frame for a single packet with packet number 0 with a single ECT(0) mark. const ACK_FRAME_1: &[u8] = &[0x03, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00]; + // This test needs to decrypt the CI, so turn off MLKEM. let mut client = new_client( - ConnectionParameters::default().versions(Version::Version1, vec![Version::Version1]), + ConnectionParameters::default() + .versions(Version::Version1, vec![Version::Version1]) + .mlkem(false), ); let mut server = default_server(); let mut now = now(); diff --git a/neqo-transport/src/connection/tests/stream.rs b/neqo-transport/src/connection/tests/stream.rs index 2cb47b08c2..2848c0159b 100644 --- a/neqo-transport/src/connection/tests/stream.rs +++ b/neqo-transport/src/connection/tests/stream.rs @@ -32,10 +32,14 @@ use crate::{ fn stream_create() { let mut client = default_client(); - let out = client.process_output(now()); + let out1 = client.process_output(now()); + let out2 = client.process_output(now()); let mut server = default_server(); - let out = server.process(out.dgram(), now()); + _ = server.process(out1.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()); drop(server.process(out.dgram(), now())); assert!(maybe_authenticate(&mut client)); @@ -752,16 +756,21 @@ fn client_fin_reorder() { let mut server = default_server(); // Send ClientHello. - let client_hs = client.process_output(now()); - assert!(client_hs.as_dgram_ref().is_some()); + let client_hs1 = client.process_output(now()); + let client_hs2 = client.process_output(now()); + assert!(client_hs1.as_dgram_ref().is_some() && client_hs2.as_dgram_ref().is_some()); - let server_hs = server.process(client_hs.dgram(), now()); + _ = server.process(client_hs1.dgram(), now()); + let server_hs = server.process(client_hs2.dgram(), now()); assert!(server_hs.as_dgram_ref().is_some()); // ServerHello, etc... let client_ack = client.process(server_hs.dgram(), now()); assert!(client_ack.as_dgram_ref().is_some()); - let server_out = server.process(client_ack.dgram(), now()); + let dgram = server.process(client_ack.dgram(), now()); + let dgram = client.process(dgram.dgram(), now()); + + let server_out = server.process(dgram.dgram(), now()); assert!(server_out.as_dgram_ref().is_none()); assert!(maybe_authenticate(&mut client)); @@ -1272,14 +1281,18 @@ fn session_flow_control_affects_all_streams() { fn connect_w_different_limit(bidi_limit: u64, unidi_limit: u64) { let mut client = default_client(); - let out = client.process_output(now()); + let out1 = client.process_output(now()); + let out2 = client.process_output(now()); let mut server = new_server( ConnectionParameters::default() .max_streams(StreamType::BiDi, bidi_limit) .max_streams(StreamType::UniDi, unidi_limit), ); - let out = server.process(out.dgram(), now()); + _ = server.process(out1.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()); drop(server.process(out.dgram(), now())); diff --git a/neqo-transport/src/connection/tests/vn.rs b/neqo-transport/src/connection/tests/vn.rs index 60fafd56aa..e305af583d 100644 --- a/neqo-transport/src/connection/tests/vn.rs +++ b/neqo-transport/src/connection/tests/vn.rs @@ -76,6 +76,7 @@ fn version_negotiation_current_version() { .dgram() .expect("a datagram") .to_vec(); + _ = client.process_output(now()).dgram().expect("a datagram"); let vn = create_vn( &initial_pkt, @@ -83,7 +84,8 @@ fn version_negotiation_current_version() { ); let dgram = datagram(vn); - let delay = client.process(Some(dgram), now()).callback(); + let mut delay = client.process(Some(dgram), now()).callback(); + delay += client.process_output(now() + delay).callback(); // TODO: Why is there first a 5ms pacing delay before the PTO? assert_eq!(delay, INITIAL_PTO); assert_eq!(*client.state(), State::WaitInitial); assert_eq!(1, client.stats().dropped_rx); @@ -98,11 +100,13 @@ fn version_negotiation_version0() { .dgram() .expect("a datagram") .to_vec(); + _ = client.process_output(now()).dgram().expect("a datagram"); let vn = create_vn(&initial_pkt, &[0, 0x1a1a_1a1a]); let dgram = datagram(vn); - let delay = client.process(Some(dgram), now()).callback(); + let mut delay = client.process(Some(dgram), now()).callback(); + delay += client.process_output(now() + delay).callback(); // TODO: Why is there first a 5ms pacing delay before the PTO? assert_eq!(delay, INITIAL_PTO); assert_eq!(*client.state(), State::WaitInitial); assert_eq!(1, client.stats().dropped_rx); @@ -139,11 +143,13 @@ fn version_negotiation_corrupted() { .dgram() .expect("a datagram") .to_vec(); + _ = client.process_output(now()).dgram().expect("a datagram"); let vn = create_vn(&initial_pkt, &[0x1a1a_1a1a, 0x2a2a_2a2a]); let dgram = datagram(vn[..vn.len() - 1].to_vec()); - let delay = client.process(Some(dgram), now()).callback(); + let mut delay = client.process(Some(dgram), now()).callback(); + delay += client.process_output(now() + delay).callback(); // TODO: Why is there first a 5ms pacing delay before the PTO? assert_eq!(delay, INITIAL_PTO); assert_eq!(*client.state(), State::WaitInitial); assert_eq!(1, client.stats().dropped_rx); @@ -158,11 +164,13 @@ fn version_negotiation_empty() { .dgram() .expect("a datagram") .to_vec(); + _ = client.process_output(now()).dgram().expect("a datagram"); let vn = create_vn(&initial_pkt, &[]); let dgram = datagram(vn); - let delay = client.process(Some(dgram), now()).callback(); + let mut delay = client.process(Some(dgram), now()).callback(); + delay += client.process_output(now() + delay).callback(); // TODO: Why is there first a 5ms pacing delay before the PTO? assert_eq!(delay, INITIAL_PTO); assert_eq!(*client.state(), State::WaitInitial); assert_eq!(1, client.stats().dropped_rx); @@ -198,12 +206,14 @@ fn version_negotiation_bad_cid() { .dgram() .expect("a datagram") .to_vec(); + _ = client.process_output(now()).dgram().expect("a datagram"); initial_pkt[6] ^= 0xc4; let vn = create_vn(&initial_pkt, &[0x1a1a_1a1a, 0x2a2a_2a2a, 0xff00_0001]); let dgram = datagram(vn); - let delay = client.process(Some(dgram), now()).callback(); + let mut delay = client.process(Some(dgram), now()).callback(); + delay += client.process_output(now() + delay).callback(); // TODO: Why is there first a 5ms pacing delay before the PTO? assert_eq!(delay, INITIAL_PTO); assert_eq!(*client.state(), State::WaitInitial); assert_eq!(1, client.stats().dropped_rx); @@ -238,9 +248,11 @@ fn compatible_upgrade_large_initial() { // Client Initial should take 2 packets. // Each should elicit a Version 1 ACK from the server. - let dgram = client.process_output(now()).dgram(); - assert!(dgram.is_some()); - let dgram = server.process(dgram, now()).dgram(); + let dgram1 = client.process_output(now()).dgram(); + let dgram2 = client.process_output(now()).dgram(); + assert!(dgram1.is_some() && dgram2.is_some()); + _ = server.process(dgram1, now()).dgram(); + let dgram = server.process(dgram2, now()).dgram(); assert!(dgram.is_some()); // The following uses the Version from *outside* this crate. assertions::assert_version(dgram.as_ref().unwrap(), Version::Version1.wire_version()); @@ -250,8 +262,8 @@ fn compatible_upgrade_large_initial() { assert_eq!(client.version(), Version::Version2); assert_eq!(server.version(), Version::Version2); // Only handshake padding is "dropped". - assert_eq!(client.stats().dropped_rx, 1); - assert_eq!(server.stats().dropped_rx, 2); + assert_eq!(client.stats().dropped_rx, 3); + assert_eq!(server.stats().dropped_rx, 3); } /// A server that supports versions 1 and 2 might prefer version 1 and that's OK. @@ -327,13 +339,15 @@ fn invalid_server_version() { let mut server = new_server(ConnectionParameters::default().versions(Version::Version2, Version::all())); - let dgram = client.process_output(now()).dgram(); - server.process_input(dgram.unwrap(), now()); + let dgram1 = client.process_output(now()).dgram(); + let dgram2 = client.process_output(now()).dgram(); + server.process_input(dgram1.unwrap(), now()); + server.process_input(dgram2.unwrap(), now()); - // One packet received. - assert_eq!(server.stats().packets_rx, 1); - // None dropped; the server will have decrypted it successfully. - assert_eq!(server.stats().dropped_rx, 0); + // Three packets received (one is zero padding). + assert_eq!(server.stats().packets_rx, 3); + // One dropped (the zero padding). + assert_eq!(server.stats().dropped_rx, 1); assert_eq!(server.stats().saved_datagrams, 0); // The server effectively hasn't reacted here. match server.state() { @@ -456,10 +470,12 @@ fn compatible_upgrade_0rtt_rejected() { client.enable_resumption(now(), token).unwrap(); // Create a packet with 0-RTT from the client. - let initial = send_something(&mut client, now()); - assertions::assert_version(&initial, Version::Version1.wire_version()); - assertions::assert_coalesced_0rtt(&initial); - server.process_input(initial, now()); + let initial1 = send_something(&mut client, now()); + let initial2 = send_something(&mut client, now()); + assertions::assert_version(&initial1, Version::Version1.wire_version()); + assertions::assert_coalesced_0rtt(&initial2); + server.process_input(initial1, now()); + server.process_input(initial2, now()); assert!(!server .events() .any(|e| matches!(e, ConnectionEvent::NewStream { .. }))); @@ -467,6 +483,8 @@ fn compatible_upgrade_0rtt_rejected() { // Finalize the connection. Don't use connect() because it uses // maybe_authenticate() too liberally and that eats the events we want to check. let dgram = server.process_output(now()).dgram(); // ServerHello flight + let dgram = client.process(dgram, now()).dgram(); + let dgram = server.process(dgram, now()).dgram(); let dgram = client.process(dgram, now()).dgram(); // Client Finished (note: no authentication) let dgram = server.process(dgram, now()).dgram(); // HANDSHAKE_DONE client.process_input(dgram.unwrap(), now()); diff --git a/neqo-transport/src/connection/tests/zerortt.rs b/neqo-transport/src/connection/tests/zerortt.rs index 80929e5f72..ec7fc4ee10 100644 --- a/neqo-transport/src/connection/tests/zerortt.rs +++ b/neqo-transport/src/connection/tests/zerortt.rs @@ -52,18 +52,23 @@ fn zero_rtt_send_recv() { let mut server = resumed_server(&client); // Send ClientHello. - let client_hs = client.process_output(now()); - assert!(client_hs.as_dgram_ref().is_some()); + let client_hs1 = client.process_output(now()); + let client_hs2 = client.process_output(now()); + assert!(client_hs1.as_dgram_ref().is_some() && client_hs2.as_dgram_ref().is_some()); + + // Wait + let delay = client.process_output(now()).callback(); // Now send a 0-RTT packet. let client_stream_id = client.stream_create(StreamType::UniDi).unwrap(); client.stream_send(client_stream_id, &[1, 2, 3]).unwrap(); - let client_0rtt = client.process_output(now()); + let client_0rtt = client.process_output(now() + delay); assert!(client_0rtt.as_dgram_ref().is_some()); // 0-RTT packets on their own shouldn't be padded to MIN_INITIAL_PACKET_SIZE. assert!(client_0rtt.as_dgram_ref().unwrap().len() < MIN_INITIAL_PACKET_SIZE); - let server_hs = server.process(client_hs.dgram(), now()); + _ = server.process(client_hs1.dgram(), now()); + let server_hs = server.process(client_hs2.dgram(), now()); assert!(server_hs.as_dgram_ref().is_some()); // ServerHello, etc... let all_frames = server.stats().frame_tx.all(); @@ -100,11 +105,14 @@ fn zero_rtt_send_coalesce() { // This should result in a datagram that coalesces Initial and 0-RTT. let client_stream_id = client.stream_create(StreamType::UniDi).unwrap(); client.stream_send(client_stream_id, &[1, 2, 3]).unwrap(); + let client_init = client.process_output(now()); + assert!(client_init.as_dgram_ref().is_some()); let client_0rtt = client.process_output(now()); assert!(client_0rtt.as_dgram_ref().is_some()); assertions::assert_coalesced_0rtt(&client_0rtt.as_dgram_ref().unwrap()[..]); + _ = server.process(client_init.dgram(), now()); let server_hs = server.process(client_0rtt.dgram(), now()); assert!(server_hs.as_dgram_ref().is_some()); // Should produce ServerHello etc... @@ -152,27 +160,31 @@ fn zero_rtt_send_reject() { .server_enable_0rtt(&ar, AllowZeroRtt {}) .expect("enable 0-RTT"); + // Write some data on the client. + let stream_id = client.stream_create(StreamType::UniDi).unwrap(); + client.stream_send(stream_id, MESSAGE).unwrap(); + // Send ClientHello. let client_hs = client.process_output(now()); assert!(client_hs.as_dgram_ref().is_some()); - // Write some data on the client. - let stream_id = client.stream_create(StreamType::UniDi).unwrap(); - client.stream_send(stream_id, MESSAGE).unwrap(); let client_0rtt = client.process_output(now()); assert!(client_0rtt.as_dgram_ref().is_some()); let server_hs = server.process(client_hs.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 server_hs2 = server.process(client_0rtt.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.process(server_hs.dgram(), now()); + let dgram = client.process(server_hs2.dgram(), now()).dgram(); + let dgram = server.process(dgram, now()).dgram(); + // Client should get a rejection. - let client_fin = client.process(server_hs.dgram(), now()); + let client_fin = client.process(dgram, now()); let recvd_0rtt_reject = |e| e == ConnectionEvent::ZeroRttRejected; assert!(client.events().any(recvd_0rtt_reject)); @@ -227,15 +239,19 @@ fn zero_rtt_update_flow_control() { ); // Stream limits should be low for 0-RTT. - let client_hs = client.process_output(now()).dgram(); + let client_hs1 = client.process_output(now()).dgram(); + let client_hs2 = client.process_output(now()).dgram(); let uni_stream = client.stream_create(StreamType::UniDi).unwrap(); assert!(!client.stream_send_atomic(uni_stream, MESSAGE).unwrap()); let bidi_stream = client.stream_create(StreamType::BiDi).unwrap(); assert!(!client.stream_send_atomic(bidi_stream, MESSAGE).unwrap()); // Now get the server transport parameters. - let server_hs = server.process(client_hs, now()).dgram(); - client.process_input(server_hs.unwrap(), now()); + _ = server.process(client_hs1, now()).dgram(); + let server_hs = server.process(client_hs2, now()).dgram(); + let client_hs3 = client.process(server_hs, now()).dgram(); + let server_hs2 = server.process(client_hs3, now()).dgram(); + client.process_input(server_hs2.unwrap(), now()); // The streams should report a writeable event. let mut uni_stream_event = false; @@ -288,7 +304,9 @@ fn zero_rtt_loss_accepted() { client.stream_send(client_stream_id, &[1, 2, 3]).unwrap(); let mut ci = client.process_output(now); assert!(ci.as_dgram_ref().is_some()); - assertions::assert_coalesced_0rtt(&ci.as_dgram_ref().unwrap()[..]); + let mut c0rtt = client.process_output(now); + assert!(c0rtt.as_dgram_ref().is_some()); + assertions::assert_coalesced_0rtt(&c0rtt.as_dgram_ref().unwrap()[..]); // Drop CI/0-RTT a number of times qdebug!("Drop CI/0-RTT {i} extra times"); @@ -296,10 +314,13 @@ fn zero_rtt_loss_accepted() { now += client.process_output(now).callback(); ci = client.process_output(now); assert!(ci.as_dgram_ref().is_some()); + c0rtt = client.process_output(now); + assert!(c0rtt.as_dgram_ref().is_some()); } // Process CI/0-RTT - let si = server.process(ci.dgram(), now); + _ = server.process(ci.dgram(), now); + let si = server.process(c0rtt.dgram(), now); assert!(si.as_dgram_ref().is_some()); let server_stream_id = server diff --git a/neqo-transport/src/crypto.rs b/neqo-transport/src/crypto.rs index 120c270d10..d0e408de0e 100644 --- a/neqo-transport/src/crypto.rs +++ b/neqo-transport/src/crypto.rs @@ -37,7 +37,7 @@ use crate::{ tparams::{TpZeroRttChecker, TransportParameters, TransportParametersHandler}, tracking::PacketNumberSpace, version::Version, - Error, Res, + ConnectionParameters, Error, Res, }; const MAX_AUTH_TAG: usize = 32; @@ -69,6 +69,7 @@ type TpHandler = Rc>; impl Crypto { pub fn new( version: Version, + conn_params: &ConnectionParameters, mut agent: Agent, protocols: Vec, tphandler: TpHandler, @@ -79,17 +80,25 @@ impl Crypto { TLS_AES_256_GCM_SHA384, TLS_CHACHA20_POLY1305_SHA256, ])?; - agent.set_groups(&[ - TLS_GRP_KEM_MLKEM768X25519, - TLS_GRP_EC_X25519, - TLS_GRP_EC_SECP256R1, - TLS_GRP_EC_SECP384R1, - TLS_GRP_EC_SECP521R1, - ])?; + agent.set_groups(if conn_params.mlkem_enabled() { + &[ + TLS_GRP_KEM_MLKEM768X25519, + TLS_GRP_EC_X25519, + TLS_GRP_EC_SECP256R1, + TLS_GRP_EC_SECP384R1, + TLS_GRP_EC_SECP521R1, + ] + } else { + &[ + TLS_GRP_EC_X25519, + TLS_GRP_EC_SECP256R1, + TLS_GRP_EC_SECP384R1, + TLS_GRP_EC_SECP521R1, + ] + })?; if let Agent::Client(c) = &mut agent { - // Configure clients to send MLKEM768X25519, X25519 and P256 to - // reduce the rate of HRRs. - c.send_additional_key_shares(2)?; + // Configure clients to send additional key shares to reduce the rate of HRRs. + c.send_additional_key_shares(1)?; // Always enable 0-RTT on the client, but the server needs // more configuration passed to server_enable_0rtt. @@ -1421,6 +1430,10 @@ impl CryptoStreams { } } + pub fn is_empty(&mut self, space: PacketNumberSpace) -> bool { + self.get_mut(space).map_or(true, |cs| cs.tx.is_empty()) + } + const fn get(&self, space: PacketNumberSpace) -> Option<&CryptoStream> { let (initial, hs, app) = match self { Self::Initial { @@ -1530,7 +1543,9 @@ impl CryptoStreams { // some middleboxes. This is ignores CRYPTO frame headers so // undercounts a little, but that's fine. let packets_needed = data.len() / builder.limit() + 1; - builder.set_limit(data.len() / packets_needed); + if packets_needed > 1 { + builder.set_limit(data.len() / packets_needed); + } [ write_chunk(offset + mid as u64, right, builder), write_chunk(offset, left, builder), diff --git a/neqo-transport/src/send_stream.rs b/neqo-transport/src/send_stream.rs index 915997737a..347edc9d65 100644 --- a/neqo-transport/src/send_stream.rs +++ b/neqo-transport/src/send_stream.rs @@ -498,6 +498,11 @@ impl TxBuffer { can_buffer } + pub fn is_empty(&mut self) -> bool { + let (start, _) = self.ranges.first_unmarked_range(); + start == self.retired() + u64::try_from(self.buffered()).unwrap() + } + #[allow(clippy::missing_panics_doc)] // These are not possible. pub fn next_bytes(&mut self) -> Option<(u64, &[u8])> { let (start, maybe_len) = self.ranges.first_unmarked_range(); diff --git a/neqo-transport/tests/common/mod.rs b/neqo-transport/tests/common/mod.rs index f8b9c98d6a..c43800e662 100644 --- a/neqo-transport/tests/common/mod.rs +++ b/neqo-transport/tests/common/mod.rs @@ -58,15 +58,19 @@ pub fn connect(client: &mut Connection, server: &mut Server) -> ConnectionRef { server.set_validation(ValidateAddress::Never); assert_eq!(*client.state(), State::Init); - let out = client.process_output(now()); // ClientHello - assert!(out.as_dgram_ref().is_some()); - let out = server.process(out.dgram(), now()); // ServerHello... + let out1 = client.process_output(now()); // ClientHello + let out2 = client.process_output(now()); // ClientHello + assert!(out1.as_dgram_ref().is_some() && out2.as_dgram_ref().is_some()); + _ = server.process(out1.dgram(), now()); // ACK + let out = server.process(out2.dgram(), now()); // ServerHello... assert!(out.as_dgram_ref().is_some()); // Ingest the server Certificate. let out = client.process(out.dgram(), now()); assert!(out.as_dgram_ref().is_some()); // This should just be an ACK. let out = server.process(out.dgram(), now()); + let out = client.process(out.dgram(), now()); + let out = server.process(out.dgram(), now()); assert!(out.as_dgram_ref().is_none()); // So the server should have nothing to say. // Now mark the server as authenticated. diff --git a/neqo-transport/tests/connection.rs b/neqo-transport/tests/connection.rs index 12b4829732..6ef70f7ff0 100644 --- a/neqo-transport/tests/connection.rs +++ b/neqo-transport/tests/connection.rs @@ -29,10 +29,14 @@ fn truncate_long_packet() { let mut client = default_client(); let mut server = default_server(); - let out = client.process_output(now()); + let out1 = client.process_output(now()); + let out2 = client.process_output(now()); + assert!(out1.as_dgram_ref().is_some() && out2.as_dgram_ref().is_some()); + _ = server.process(out1.dgram(), now()); + let out = server.process(out2.dgram(), now()); assert!(out.as_dgram_ref().is_some()); + let out = client.process(out.dgram(), now()); let out = server.process(out.dgram(), now()); - assert!(out.as_dgram_ref().is_some()); // This will truncate the Handshake packet from the server. let dupe = out.as_dgram_ref().unwrap().clone(); @@ -66,8 +70,11 @@ fn reorder_server_initial() { // A simple ACK_ECN frame for a single packet with packet number 0 with a single ECT(0) mark. const ACK_FRAME: &[u8] = &[0x03, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00]; + // This test needs to decrypt the CI, so turn off MLKEM. let mut client = new_client( - ConnectionParameters::default().versions(Version::Version1, vec![Version::Version1]), + ConnectionParameters::default() + .versions(Version::Version1, vec![Version::Version1]) + .mlkem(false), ); let mut server = default_server(); diff --git a/neqo-transport/tests/retry.rs b/neqo-transport/tests/retry.rs index 99abfd96b9..b7470bb3c3 100644 --- a/neqo-transport/tests/retry.rs +++ b/neqo-transport/tests/retry.rs @@ -17,7 +17,8 @@ use common::{connected_server, default_server, generate_ticket}; use neqo_common::{hex_with_len, qdebug, qtrace, Datagram, Encoder, Role}; use neqo_crypto::AuthenticationStatus; use neqo_transport::{ - server::ValidateAddress, CloseReason, Error, State, StreamType, MIN_INITIAL_PACKET_SIZE, + server::ValidateAddress, CloseReason, ConnectionParameters, Error, State, StreamType, + MIN_INITIAL_PACKET_SIZE, }; use test_fixture::{ assertions, datagram, default_client, @@ -34,13 +35,19 @@ fn retry_basic() { server.set_validation(ValidateAddress::Always); let mut client = default_client(); - let dgram = client.process_output(now()).dgram(); // Initial - assert!(dgram.is_some()); - let dgram = server.process(dgram, now()).dgram().unwrap(); // Retry + let dgram1 = client.process_output(now()).dgram(); // Initial + let dgram2 = client.process_output(now()).dgram(); // Initial + assert!(dgram1.is_some() && dgram2.is_some()); + _ = server.process(dgram1, now()).dgram().unwrap(); // Retry + let dgram = server.process(dgram2, now()).dgram().unwrap(); // Retry assertions::assert_retry(&dgram); - let dgram = client.process(Some(dgram), now()).dgram(); // Initial w/token - assert!(dgram.is_some()); + let dgram1 = client.process(Some(dgram), now()).dgram(); // Initial w/token + let dgram2 = client.process_output(now()).dgram(); // Initial + assert!(dgram1.is_some() && dgram2.is_some()); + _ = server.process(dgram1, now()).dgram().unwrap(); + let dgram = server.process(dgram2, now()).dgram(); + let dgram = client.process(dgram, now()).dgram(); let dgram = server.process(dgram, now()).dgram(); // Initial, HS assert!(dgram.is_some()); drop(client.process(dgram, now()).dgram()); // Ingest, drop any ACK. @@ -108,21 +115,27 @@ fn retry_0rtt() { let client_stream = client.stream_create(StreamType::UniDi).unwrap(); client.stream_send(client_stream, &[1, 2, 3]).unwrap(); - let dgram = client.process_output(now()).dgram(); // Initial w/0-RTT - assert!(dgram.is_some()); - assertions::assert_coalesced_0rtt(dgram.as_ref().unwrap()); - let dgram = server.process(dgram, now()).dgram(); // Retry + let dgram1 = client.process_output(now()).dgram(); // Initial + let dgram2 = client.process_output(now()).dgram(); // Initial w/0-RTT + assert!(dgram1.is_some() && dgram2.is_some()); + assertions::assert_coalesced_0rtt(dgram2.as_ref().unwrap()); + _ = server.process(dgram1, now()).dgram(); // Retry + let dgram = server.process(dgram2, now()).dgram(); // Retry assert!(dgram.is_some()); assertions::assert_retry(dgram.as_ref().unwrap()); // After retry, there should be a token and still coalesced 0-RTT. - let dgram = client.process(dgram, now()).dgram(); - assert!(dgram.is_some()); - assertions::assert_coalesced_0rtt(dgram.as_ref().unwrap()); + let dgram1 = client.process(dgram, now()).dgram(); // Initial + let dgram2 = client.process_output(now()).dgram(); // Initial w/0-RTT + assert!(dgram1.is_some() && dgram2.is_some()); + assertions::assert_coalesced_0rtt(dgram2.as_ref().unwrap()); - let dgram = server.process(dgram, now()).dgram(); // Initial, HS + _ = server.process(dgram1, now()).dgram(); // ACK + let dgram = server.process(dgram2, now()).dgram(); // Initial, HS assert!(dgram.is_some()); let dgram = client.process(dgram, now()).dgram(); + let dgram = server.process(dgram, now()).dgram(); + let dgram = client.process(dgram, now()).dgram(); // Note: the client doesn't need to authenticate the server here // as there is no certificate; authentication is based on the ticket. assert!(dgram.is_some()); @@ -212,9 +225,11 @@ fn retry_after_initial() { retry_server.set_validation(ValidateAddress::Always); let mut client = default_client(); - let cinit = client.process_output(now()).dgram(); // Initial - assert!(cinit.is_some()); - let server_flight = server.process(cinit.clone(), now()).dgram(); // Initial + let cinit1 = client.process_output(now()).dgram(); // Initial + let cinit2 = client.process_output(now()).dgram(); // Initial + assert!(cinit1.is_some() && cinit2.is_some()); + _ = server.process(cinit1.clone(), now()).dgram(); // Initial + let server_flight = server.process(cinit2, now()).dgram(); // Initial assert!(server_flight.is_some()); // We need to have the client just process the Initial. @@ -223,7 +238,7 @@ fn retry_after_initial() { assert!(dgram.is_some()); assert!(*client.state() != State::Connected); - let retry = retry_server.process(cinit, now()).dgram(); // Retry! + let retry = retry_server.process(cinit1, now()).dgram(); // Retry! assert!(retry.is_some()); assertions::assert_retry(retry.as_ref().unwrap()); @@ -233,6 +248,8 @@ fn retry_after_initial() { // Either way, the client should still be able to process the server flight and connect. let dgram = client.process(server_flight, now()).dgram(); + let dgram = server.process(dgram, now()).dgram(); + let dgram = client.process(dgram, now()).dgram(); assert!(dgram.is_some()); // Drop this one. assert!(test_fixture::maybe_authenticate(&mut client)); let dgram = client.process_output(now()).dgram(); @@ -250,9 +267,11 @@ fn retry_bad_integrity() { server.set_validation(ValidateAddress::Always); let mut client = default_client(); - let dgram = client.process_output(now()).dgram(); // Initial - assert!(dgram.is_some()); - let dgram = server.process(dgram, now()).dgram(); // Retry + let dgram1 = client.process_output(now()).dgram(); // Initial + let dgram2 = client.process_output(now()).dgram(); // Initial + assert!(dgram1.is_some() && dgram2.is_some()); + _ = server.process(dgram1, now()).dgram(); // Retry + let dgram = server.process(dgram2, now()).dgram(); // Retry assert!(dgram.is_some()); let retry = &dgram.as_ref().unwrap(); @@ -298,17 +317,20 @@ fn retry_after_pto() { server.set_validation(ValidateAddress::Always); let mut now = now(); - let ci = client.process_output(now).dgram(); - assert!(ci.is_some()); // sit on this for a bit.RefCell + let ci1 = client.process_output(now).dgram(); + let ci2 = client.process_output(now).dgram(); + assert!(ci1.is_some() && ci2.is_some()); // sit on this for a bit // Let PTO fire on the client and then let it exhaust its PTO packets. now += Duration::from_secs(1); let pto = client.process_output(now).dgram(); assert!(pto.unwrap().len() >= MIN_INITIAL_PACKET_SIZE); + _ = client.process_output(now).dgram(); let cb = client.process_output(now).callback(); assert_ne!(cb, Duration::new(0, 0)); - let retry = server.process(ci, now).dgram(); + _ = server.process(ci1, now).dgram(); + let retry = server.process(ci2, now).dgram(); assertions::assert_retry(retry.as_ref().unwrap()); let ci2 = client.process(retry, now).dgram(); @@ -321,15 +343,19 @@ fn vn_after_retry() { server.set_validation(ValidateAddress::Always); let mut client = default_client(); - let dgram = client.process_output(now()).dgram(); // Initial - assert!(dgram.is_some()); - let dgram = server.process(dgram, now()).dgram(); // Retry + let dgram1 = client.process_output(now()).dgram(); // Initial + let dgram2 = client.process_output(now()).dgram(); // Initial + assert!(dgram1.is_some() && dgram2.is_some()); + _ = server.process(dgram1, now()).dgram(); // Retry + let dgram = server.process(dgram2, now()).dgram(); // Retry assert!(dgram.is_some()); assertions::assert_retry(dgram.as_ref().unwrap()); let dgram = client.process(dgram, now()).dgram(); // Initial w/token assert!(dgram.is_some()); + let dgram = server.process(dgram, now()).dgram(); + _ = client.process(dgram, now()).dgram(); let mut encoder = Encoder::default(); encoder.encode_byte(0x80); @@ -357,7 +383,8 @@ fn vn_after_retry() { // long enough connection ID. #[test] fn mitm_retry() { - let mut client = default_client(); + // This test decrypts packets and hence does not work with MLKEM enabled. + let mut client = test_fixture::new_client(ConnectionParameters::default().mlkem(false)); let mut retry_server = default_server(); retry_server.set_validation(ValidateAddress::Always); let mut server = default_server(); diff --git a/neqo-transport/tests/server.rs b/neqo-transport/tests/server.rs index c0a10b9afb..0f641c3080 100644 --- a/neqo-transport/tests/server.rs +++ b/neqo-transport/tests/server.rs @@ -311,6 +311,7 @@ fn zero_rtt() { }; // Now generate a bunch of 0-RTT packets... + let c0 = client_send(); let c1 = client_send(); assertions::assert_coalesced_0rtt(&c1); let c2 = client_send(); @@ -322,9 +323,10 @@ fn zero_rtt() { assert!(server.active_connections().is_empty()); // Now handshake and let another 0-RTT packet in. + _ = server.process(Some(c0), now); let shs = server.process(Some(c1), now); drop(server.process(Some(c3), now)); - // The server will have received two STREAM frames now if it processed both packets. + // The server will have received three STREAM frames now if it processed both packets. // `ActiveConnectionRef` `Hash` implementation doesn’t access any of the interior mutable types. #[allow(clippy::mutable_key_type)] let active = server.active_connections(); @@ -338,7 +340,7 @@ fn zero_rtt() { .stats() .frame_rx .stream, - 2 + 3 ); // Complete the handshake. As the client was pacing 0-RTT packets, extend the time @@ -362,7 +364,7 @@ fn zero_rtt() { .stats() .frame_rx .stream, - 2 + 4 ); } @@ -378,15 +380,19 @@ fn new_token_0rtt() { let client_stream = client.stream_create(StreamType::UniDi).unwrap(); client.stream_send(client_stream, &[1, 2, 3]).unwrap(); - let out = client.process_output(now()); // Initial w/0-RTT - assert!(out.as_dgram_ref().is_some()); - assertions::assert_initial(out.as_dgram_ref().unwrap(), true); - assertions::assert_coalesced_0rtt(out.as_dgram_ref().unwrap()); - let out = server.process(out.dgram(), now()); // Initial + let out1 = client.process_output(now()); + let out2 = client.process_output(now()); // Initial w/0-RTT + assert!(out1.as_dgram_ref().is_some() && out2.as_dgram_ref().is_some()); + assertions::assert_initial(out1.as_dgram_ref().unwrap(), true); + assertions::assert_coalesced_0rtt(out2.as_dgram_ref().unwrap()); + _ = server.process(out1.dgram(), now()); // Initial + let out = server.process(out2.dgram(), now()); // Initial assert!(out.as_dgram_ref().is_some()); assertions::assert_initial(out.as_dgram_ref().unwrap(), false); let dgram = client.process(out.as_dgram_ref().cloned(), now()); + let dgram = server.process(dgram.as_dgram_ref().cloned(), now()); + let dgram = client.process(dgram.as_dgram_ref().cloned(), now()); // Note: the client doesn't need to authenticate the server here // as there is no certificate; authentication is based on the ticket. assert!(out.as_dgram_ref().is_some()); @@ -421,7 +427,8 @@ fn new_token_different_port() { #[test] fn bad_client_initial() { - let mut client = default_client(); + // This test needs to decrypt the CI, so turn off MLKEM. + let mut client = new_client(ConnectionParameters::default().mlkem(false)); let mut server = default_server(); let dgram = client.process_output(now()).dgram().expect("a datagram"); @@ -567,6 +574,7 @@ fn version_negotiation_ignored() { // Any packet will do, but let's make something that looks real. let dgram = client.process_output(now()).dgram().expect("a datagram"); + _ = client.process_output(now()).dgram().expect("a datagram"); let mut input = dgram.to_vec(); input[1] ^= 0x12; let damaged = Datagram::new( @@ -649,18 +657,24 @@ fn version_negotiation_and_compatible() { // Run the full exchange so that we can observe the versions in use. // Version Negotiation - let dgram = client.process_output(now()).dgram(); - assert!(dgram.is_some()); - assertions::assert_version(dgram.as_ref().unwrap(), ORIG_VERSION.wire_version()); - let dgram = server.process(dgram, now()).dgram(); + let dgram1 = client.process_output(now()).dgram(); + let dgram2 = client.process_output(now()).dgram(); + assert!(dgram1.is_some() && dgram2.is_some()); + assertions::assert_version(dgram1.as_ref().unwrap(), ORIG_VERSION.wire_version()); + _ = server.process(dgram1, now()).dgram(); + let dgram = server.process(dgram2, now()).dgram(); assertions::assert_vn(dgram.as_ref().unwrap()); client.process_input(dgram.unwrap(), now()); - let dgram = client.process_output(now()).dgram(); // ClientHello - assertions::assert_version(dgram.as_ref().unwrap(), VN_VERSION.wire_version()); - let dgram = server.process(dgram, now()).dgram(); // ServerHello... + let dgram1 = client.process_output(now()).dgram(); // ClientHello + let dgram2 = client.process_output(now()).dgram(); // ClientHello + assertions::assert_version(dgram1.as_ref().unwrap(), VN_VERSION.wire_version()); + _ = server.process(dgram1, now()).dgram(); // ServerHello... + let dgram = server.process(dgram2, now()).dgram(); // ServerHello... assertions::assert_version(dgram.as_ref().unwrap(), COMPAT_VERSION.wire_version()); - client.process_input(dgram.unwrap(), now()); + let dgram = client.process(dgram, now()).dgram(); + let dgram = server.process(dgram, now()).dgram(); + _ = client.process(dgram, now()).dgram(); client.authenticated(AuthenticationStatus::Ok, now()); let dgram = client.process_output(now()).dgram(); @@ -825,7 +839,11 @@ fn max_streams_after_0rtt_rejection() { let mut client = default_client(); client.enable_resumption(now(), &token).unwrap(); _ = client.stream_create(StreamType::BiDi).unwrap(); - let dgram = client.process_output(now()).dgram(); + let dgram1 = client.process_output(now()).dgram(); + let dgram2 = client.process_output(now()).dgram(); + _ = server.process(dgram1, now()).dgram(); + let dgram = server.process(dgram2, now()).dgram(); + let dgram = client.process(dgram, now()).dgram(); let dgram = server.process(dgram, now()).dgram(); let dgram = client.process(dgram, now()).dgram(); assert!(dgram.is_some()); // We're far enough along to complete the test now.