Merge branch 'main' into ci-bench-multiple-mtu

.github/actions/nss/action.yml

@@ -120,7 +120,7 @@ runs:
  if: env.USE_SYSTEM_NSS == '0'
  shell: bash
  run: |
- if [ "${{ runner.environment }}" != "github-hosted" ] || [ "${{ steps.cache.outputs.cache-hit }}" == "false" ]; then
+ if [ "${{ runner.environment }}" != "github-hosted" ] || [ "${{ steps.cache.outputs.cache-hit }}" == "" ]; then
  echo "Building NSS from source"
  echo "BUILD_NSS=1" >> "$GITHUB_ENV"
  else

neqo-bin/src/client/mod.rs

@@ -457,7 +457,7 @@ impl<'a, H: Handler> Runner<'a, H> {
 
  async fn process_multiple_input(&mut self) -> Res<()> {
  loop {
- let dgrams = self.socket.recv(&self.local_addr)?;
+ let dgrams = self.socket.recv(self.local_addr)?;
  if dgrams.is_empty() {
  break;
  }

neqo-bin/src/server/mod.rs

@@ -289,7 +289,7 @@ impl ServerRunner {
  match self.ready().await? {
  Ready::Socket(inx) => loop {
  let (host, socket) = self.sockets.get_mut(inx).unwrap();
- let dgrams = socket.recv(host)?;
+ let dgrams = socket.recv(*host)?;
  if dgrams.is_empty() {
  break;
  }

neqo-bin/src/udp.rs

@@ -55,7 +55,7 @@ impl Socket {
 
  /// Receive a batch of [`Datagram`]s on the given [`Socket`], each set with
  /// the provided local address.
- pub fn recv(&self, local_address: &SocketAddr) -> Result<Vec<Datagram>, io::Error> {
+ pub fn recv(&self, local_address: SocketAddr) -> Result<Vec<Datagram>, io::Error> {
  self.inner
  .try_io(tokio::io::Interest::READABLE, || {
  neqo_udp::recv_inner(local_address, &self.state, &self.inner)

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

@@ -13,7 +13,7 @@ use super::{
  ack_bytes, connect_rtt_idle, default_client, default_server, fill_cwnd, increase_cwnd,
  induce_persistent_congestion, new_client, new_server, send_something, DEFAULT_RTT,
 };
-use crate::stream_id::StreamType;
+use crate::{connection::tests::assert_path_challenge_min_len, stream_id::StreamType};
 
 /// With the default RTT here (100ms) and default ratio (4), endpoints won't send
 /// `ACK_FREQUENCY` as the ACK delay isn't different enough from the default.
@@ -169,6 +169,7 @@ fn migrate_ack_delay() {
 
  let client1 = send_something(&mut client, now);
  assertions::assert_v4_path(&client1, true); // Contains PATH_CHALLENGE.
+ assert_path_challenge_min_len(&client, &client1, now);
  let client2 = send_something(&mut client, now);
  assertions::assert_v4_path(&client2, false); // Doesn't. Is dropped.
  now += DEFAULT_RTT / 2;

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

@@ -14,9 +14,10 @@ use test_fixture::{
 
 use crate::{
  connection::tests::{
- connect_force_idle, connect_force_idle_with_modifier, default_client, default_server,
- handshake_with_modifier, migration::get_cid, new_client, new_server, send_and_receive,
- send_something, send_something_with_modifier, send_with_modifier_and_receive, DEFAULT_RTT,
+ assert_path_challenge_min_len, connect_force_idle, connect_force_idle_with_modifier,
+ default_client, default_server, handshake_with_modifier, migration::get_cid, new_client,
+ new_server, send_and_receive, send_something, send_something_with_modifier,
+ send_with_modifier_and_receive, DEFAULT_RTT,
  },
  ecn::ECN_TEST_COUNT,
  path::MAX_PATH_PROBES,
@@ -120,6 +121,7 @@ fn migration_delay_to_ecn_blackhole() {
  // This should be a PATH_CHALLENGE.
  probes += 1;
  assert_eq!(client.stats().frame_tx.path_challenge, probes);
+ assert_path_challenge_min_len(&client, &d, now);
  if probes <= MAX_PATH_PROBES {
  // The first probes should be sent with ECN.
  assert_ecn_enabled(d.tos());
@@ -247,13 +249,15 @@ pub fn migration_with_modifiers(
  let probe = new_path_modifier(client.process_output(now).dgram().unwrap());
  if let Some(probe) = probe {
  assert_v4_path(&probe, true); // Contains PATH_CHALLENGE.
+ assert_path_challenge_min_len(&client, &probe, now);
  assert_eq!(client.stats().frame_tx.path_challenge, 1);
  let probe_cid = ConnectionId::from(get_cid(&probe));
 
  let resp = new_path_modifier(server.process(Some(&probe), now).dgram().unwrap()).unwrap();
  assert_v4_path(&resp, true);
  assert_eq!(server.stats().frame_tx.path_response, 1);
  assert_eq!(server.stats().frame_tx.path_challenge, 1);
+ assert_path_challenge_min_len(&server, &resp, now);
 
  // Data continues to be exchanged on the old path.
  let client_data = send_something_with_modifier(&mut client, now, orig_path_modifier);
@@ -287,6 +291,10 @@ pub fn migration_with_modifiers(
  server.stats().frame_tx.path_challenge,
  if migrated { 2 } else { 0 }
  );
+ if migrated {
+ assert_path_challenge_min_len(&server, &probe_old_server, now);
+ }
+
  assert_eq!(
  server.stats().frame_tx.stream,
  if migrated { stream_before } else { 1 }

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

@@ -25,7 +25,7 @@ use super::{
 };
 use crate::{
  cid::LOCAL_ACTIVE_CID_LIMIT,
- connection::tests::{send_something_paced, send_with_extra},
+ connection::tests::{assert_path_challenge_min_len, send_something_paced, send_with_extra},
  frame::FRAME_TYPE_NEW_CONNECTION_ID,
  packet::PacketBuilder,
  path::MAX_PATH_PROBES,
@@ -102,10 +102,12 @@ fn path_forwarding_attack() {
  // The server now probes the new (primary) path.
  let new_probe = server.process_output(now).dgram().unwrap();
  assert_eq!(server.stats().frame_tx.path_challenge, 1);
+ assert_path_challenge_min_len(&server, &new_probe, now);
  assert_v4_path(&new_probe, false); // Can't be padded.
 
  // The server also probes the old path.
  let old_probe = server.process_output(now).dgram().unwrap();
+ assert_path_challenge_min_len(&server, &old_probe, now);
  assert_eq!(server.stats().frame_tx.path_challenge, 2);
  assert_v6_path(&old_probe, true);
 
@@ -140,6 +142,7 @@ fn path_forwarding_attack() {
  let server_data1 = server.process(Some(&new_resp), now).dgram().unwrap();
  assert_v4_path(&server_data1, true);
  assert_eq!(server.stats().frame_tx.path_challenge, 3);
+ assert_path_challenge_min_len(&server, &server_data1, now);
 
  // The client responds to this probe on the new path.
  client.process_input(&server_data1, now);
@@ -179,6 +182,8 @@ fn migrate_immediate() {
 
  let client1 = send_something(&mut client, now);
  assert_v4_path(&client1, true); // Contains PATH_CHALLENGE.
+ assert_path_challenge_min_len(&client, &client1, now);
+
  let client2 = send_something(&mut client, now);
  assert_v4_path(&client2, false); // Doesn't.
 
@@ -236,6 +241,7 @@ fn migrate_immediate_fail() {
 
  let probe = client.process_output(now).dgram().unwrap();
  assert_v4_path(&probe, true); // Contains PATH_CHALLENGE.
+ assert_path_challenge_min_len(&client, &probe, now);
 
  // -1 because first PATH_CHALLENGE already sent above
  for _ in 0..MAX_PATH_PROBES * 2 - 1 {
@@ -246,15 +252,17 @@ fn migrate_immediate_fail() {
  let before = client.stats().frame_tx;
  let probe = client.process_output(now).dgram().unwrap();
  assert_v4_path(&probe, true); // Contains PATH_CHALLENGE.
+ assert_path_challenge_min_len(&client, &probe, now);
  let after = client.stats().frame_tx;
  assert_eq!(after.path_challenge, before.path_challenge + 1);
  assert_eq!(after.padding, before.padding + 1);
  assert_eq!(after.all(), before.all() + 2);
 
  // This might be a PTO, which will result in sending a probe.
  if let Some(probe) = client.process_output(now).dgram() {
- assert_v4_path(&probe, false); // Contains PATH_CHALLENGE.
+ assert_v4_path(&probe, false); // Contains PING.
  let after = client.stats().frame_tx;
+ assert_eq!(after.path_challenge, before.path_challenge + 1);
  assert_eq!(after.ping, before.ping + 1);
  assert_eq!(after.all(), before.all() + 3);
  }
@@ -286,6 +294,7 @@ fn migrate_same() {
  let probe = client.process_output(now).dgram().unwrap();
  assert_v6_path(&probe, true); // Contains PATH_CHALLENGE.
  assert_eq!(client.stats().frame_tx.path_challenge, 1);
+ assert_path_challenge_min_len(&client, &probe, now);
 
  let resp = server.process(Some(&probe), now).dgram().unwrap();
  assert_v6_path(&resp, true);
@@ -312,6 +321,7 @@ fn migrate_same_fail() {
 
  let probe = client.process_output(now).dgram().unwrap();
  assert_v6_path(&probe, true); // Contains PATH_CHALLENGE.
+ assert_path_challenge_min_len(&client, &probe, now);
 
  // -1 because first PATH_CHALLENGE already sent above
  for _ in 0..MAX_PATH_PROBES * 2 - 1 {
@@ -322,15 +332,17 @@ fn migrate_same_fail() {
  let before = client.stats().frame_tx;
  let probe = client.process_output(now).dgram().unwrap();
  assert_v6_path(&probe, true); // Contains PATH_CHALLENGE.
+ assert_path_challenge_min_len(&client, &probe, now);
  let after = client.stats().frame_tx;
  assert_eq!(after.path_challenge, before.path_challenge + 1);
  assert_eq!(after.padding, before.padding + 1);
  assert_eq!(after.all(), before.all() + 2);
 
  // This might be a PTO, which will result in sending a probe.
  if let Some(probe) = client.process_output(now).dgram() {
- assert_v6_path(&probe, false); // Contains PATH_CHALLENGE.
+ assert_v6_path(&probe, false); // Contains PING.
  let after = client.stats().frame_tx;
+ assert_eq!(after.path_challenge, before.path_challenge + 1);
  assert_eq!(after.ping, before.ping + 1);
  assert_eq!(after.all(), before.all() + 3);
  }
@@ -368,11 +380,13 @@ fn migration(mut client: Connection) {
 
  let probe = client.process_output(now).dgram().unwrap();
  assert_v4_path(&probe, true); // Contains PATH_CHALLENGE.
+ assert_path_challenge_min_len(&client, &probe, now);
  assert_eq!(client.stats().frame_tx.path_challenge, 1);
  let probe_cid = ConnectionId::from(get_cid(&probe));
 
  let resp = server.process(Some(&probe), now).dgram().unwrap();
  assert_v4_path(&resp, true);
+ assert_path_challenge_min_len(&server, &resp, now);
  assert_eq!(server.stats().frame_tx.path_response, 1);
  assert_eq!(server.stats().frame_tx.path_challenge, 1);
 
@@ -399,6 +413,7 @@ fn migration(mut client: Connection) {
  let probe_old_server = send_something(&mut server, now);
  // This is just the double-check probe; no STREAM frames.
  assert_v6_path(&probe_old_server, true);
+ assert_path_challenge_min_len(&server, &probe_old_server, now);
  assert_eq!(server.stats().frame_tx.path_challenge, 2);
  assert_eq!(server.stats().frame_tx.stream, stream_before);
 
@@ -515,6 +530,7 @@ fn preferred_address(hs_client: SocketAddr, hs_server: SocketAddr, preferred: So
  let probe = client.process(dgram.as_ref(), now()).dgram().unwrap();
  assert_toward_spa(&probe, true);
  assert_eq!(client.stats().frame_tx.path_challenge, 1);
+ assert_path_challenge_min_len(&client, &probe, now());
  assert_ne!(client.process_output(now()).callback(), Duration::new(0, 0));
 
  // Data continues on the main path for the client.
@@ -525,6 +541,7 @@ fn preferred_address(hs_client: SocketAddr, hs_server: SocketAddr, preferred: So
  let resp = server.process(Some(&probe), now()).dgram().unwrap();
  assert_from_spa(&resp, true);
  assert_eq!(server.stats().frame_tx.path_challenge, 1);
+ assert_path_challenge_min_len(&server, &resp, now());
  assert_eq!(server.stats().frame_tx.path_response, 1);
 
  // Data continues on the main path for the server.
@@ -544,6 +561,7 @@ fn preferred_address(hs_client: SocketAddr, hs_server: SocketAddr, preferred: So
  let probe = server.process(Some(&data), now()).dgram().unwrap();
  assert_orig_path(&probe, true);
  assert_eq!(server.stats().frame_tx.path_challenge, 2);
+ assert_path_challenge_min_len(&server, &probe, now());
 
  // But data now goes on the new path.
  let data = send_something(&mut server, now());
@@ -854,6 +872,7 @@ fn retire_prior_to_migration_failure() {
  let probe = client.process_output(now()).dgram().unwrap();
  assert_v4_path(&probe, true);
  assert_eq!(client.stats().frame_tx.path_challenge, 1);
+ assert_path_challenge_min_len(&client, &probe, now());
  let probe_cid = ConnectionId::from(get_cid(&probe));
  assert_ne!(original_cid, probe_cid);
 
@@ -865,6 +884,7 @@ fn retire_prior_to_migration_failure() {
  assert_v4_path(&resp, true);
  assert_eq!(server.stats().frame_tx.path_response, 1);
  assert_eq!(server.stats().frame_tx.path_challenge, 1);
+ assert_path_challenge_min_len(&server, &resp, now());
 
  // Have the client receive the NEW_CONNECTION_ID with Retire Prior To.
  client.process_input(&retire_all, now());
@@ -907,6 +927,7 @@ fn retire_prior_to_migration_success() {
  let probe = client.process_output(now()).dgram().unwrap();
  assert_v4_path(&probe, true);
  assert_eq!(client.stats().frame_tx.path_challenge, 1);
+ assert_path_challenge_min_len(&client, &probe, now());
  let probe_cid = ConnectionId::from(get_cid(&probe));
  assert_ne!(original_cid, probe_cid);
 
@@ -918,6 +939,7 @@ fn retire_prior_to_migration_success() {
  assert_v4_path(&resp, true);
  assert_eq!(server.stats().frame_tx.path_response, 1);
  assert_eq!(server.stats().frame_tx.path_challenge, 1);
+ assert_path_challenge_min_len(&server, &resp, now());
 
  // Have the client receive the NEW_CONNECTION_ID with Retire Prior To second.
  // As this occurs in a very specific order, migration succeeds.

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

@@ -30,7 +30,7 @@ use crate::{
  stats::{FrameStats, Stats, MAX_PTO_COUNTS},
  tparams::{DISABLE_MIGRATION, GREASE_QUIC_BIT},
  ConnectionIdDecoder, ConnectionIdGenerator, ConnectionParameters, Error, StreamId, StreamType,
- Version,
+ Version, MIN_INITIAL_PACKET_SIZE,
 };
 
 // All the tests.
@@ -669,6 +669,27 @@ fn assert_default_stats(stats: &Stats) {
  assert_eq!(stats.frame_tx, dflt_frames);
 }
 
+fn assert_path_challenge_min_len(c: &Connection, d: &Datagram, now: Instant) {
+ let path = c.paths.find_path(
+ d.source(),
+ d.destination(),
+ c.conn_params.get_cc_algorithm(),
+ c.conn_params.pacing_enabled(),
+ now,
+ );
+ if path.borrow().amplification_limit() < path.borrow().plpmtu() {
+ // If the amplification limit is less than the PLPMTU, then the path
+ // challenge will not have been padded.
+ return;
+ }
+ assert!(
+ d.len() >= MIN_INITIAL_PACKET_SIZE,
+ "{} < {}",
+ d.len(),
+ MIN_INITIAL_PACKET_SIZE
+ );
+}
+
 #[test]
 fn create_client() {
  let client = default_client();

neqo-udp/src/lib.rs

@@ -58,7 +58,7 @@ use std::os::fd::AsFd as SocketRef;
 use std::os::windows::io::AsSocket as SocketRef;
 
 pub fn recv_inner(
- local_address: &SocketAddr,
+ local_address: SocketAddr,
  state: &UdpSocketState,
  socket: impl SocketRef,
 ) -> Result<Vec<Datagram>, io::Error> {
@@ -99,7 +99,7 @@ pub fn recv_inner(
  );
  Datagram::new(
  meta.addr,
- *local_address,
+ local_address,
  meta.ecn.map(|n| IpTos::from(n as u8)).unwrap_or_default(),
  d,
  )
@@ -138,7 +138,7 @@ impl<S: SocketRef> Socket<S> {
 
  /// Receive a batch of [`Datagram`]s on the given [`Socket`], each
  /// set with the provided local address.
- pub fn recv(&self, local_address: &SocketAddr) -> Result<Vec<Datagram>, io::Error> {
+ pub fn recv(&self, local_address: SocketAddr) -> Result<Vec<Datagram>, io::Error> {
  recv_inner(local_address, &self.state, &self.inner)
  }
 }
@@ -170,7 +170,7 @@ mod tests {
  );
 
  sender.send(&datagram)?;
- let res = receiver.recv(&receiver_addr);
+ let res = receiver.recv(receiver_addr);
  assert_eq!(res.unwrap_err().kind(), std::io::ErrorKind::WouldBlock);
 
  Ok(())
@@ -192,7 +192,7 @@ mod tests {
  sender.send(&datagram)?;
 
  let received_datagram = receiver
- .recv(&receiver_addr)
+ .recv(receiver_addr)
  .expect("receive to succeed")
  .into_iter()
  .next()
@@ -238,7 +238,7 @@ mod tests {
  let mut num_received = 0;
  while num_received < max_gso_segments {
  receiver
- .recv(&receiver_addr)
+ .recv(receiver_addr)
  .expect("receive to succeed")
  .into_iter()
  .for_each(|d| {

qns/Dockerfile

@@ -33,7 +33,7 @@ RUN cargo chef cook --release --recipe-path recipe.json
 ADD . /neqo
 RUN set -eux; \
  cd /neqo; \
- cargo build --release --bin neqo-client --bin neqo-server
+ CARGO_PROFILE_RELEASE_DEBUG=true cargo build --release --bin neqo-client --bin neqo-server
 
 # Copy only binaries to the final image to keep it small.