feat: Randomize CI packet number

neqo-transport/src/connection/mod.rs

@@ -323,6 +323,7 @@ impl Connection {
         now: Instant,
     ) -> Res<Self> {
         let dcid = ConnectionId::generate_initial();
+        let randomize_ci_pn = conn_params.randomize_ci_pn_enabled();
         let mut c = Self::new(
             Role::Client,
             Agent::from(Client::new(server_name.into(), conn_params.is_greasing())?),
@@ -334,6 +335,7 @@ impl Connection {
             c.conn_params.get_versions().compatible(),
             Role::Client,
             &dcid,
+            randomize_ci_pn,
         )?;
         c.original_destination_cid = Some(dcid);
         let path = Path::temporary(
@@ -1233,6 +1235,7 @@ impl Connection {
             self.conn_params.get_versions().compatible(),
             self.role,
             &retry_scid,
+            false,
         )?;
         self.address_validation = AddressValidationInfo::Retry {
             token: packet.token().to_vec(),
@@ -2472,6 +2475,9 @@ impl Connection {
             );
 
             self.stats.borrow_mut().packets_tx += 1;
+            if pt == PacketType::Initial && self.stats.borrow().first_initial_pn.is_none() {
+                self.stats.borrow_mut().first_initial_pn = Some(pn);
+            }
             let tx = self
                 .crypto
                 .states

neqo-transport/src/connection/params.rs

@@ -98,6 +98,8 @@ pub struct ConnectionParameters {
     sni_slicing: bool,
     /// Whether to enable mlkem768nistp256-sha256.
     mlkem: bool,
+    /// Whether to randomize the initial packet number.
+    randomize_ci_pn: bool,
 }
 
 impl Default for ConnectionParameters {
@@ -126,6 +128,7 @@ impl Default for ConnectionParameters {
             pmtud_iface_mtu: true,
             sni_slicing: true,
             mlkem: true,
+            randomize_ci_pn: true,
         }
     }
 }
@@ -419,6 +422,17 @@ impl ConnectionParameters {
         self
     }
 
+    #[must_use]
+    pub const fn randomize_ci_pn_enabled(&self) -> bool {
+        self.randomize_ci_pn
+    }
+
+    #[must_use]
+    pub const fn randomize_ci_pn(mut self, randomize_ci_pn: bool) -> Self {
+        self.randomize_ci_pn = randomize_ci_pn;
+        self
+    }
+
     /// # Errors
     /// When a connection ID cannot be obtained.
     /// # Panics

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

@@ -58,7 +58,9 @@ enum CongestionSignal {
 }
 
 fn cc_slow_start_to_cong_avoidance_recovery_period(congestion_signal: CongestionSignal) {
-    let mut client = default_client();
+    // This test needs to calculate largest_acked, which is difficult if packet number
+    // randomization is enabled.
+    let mut client = new_client(ConnectionParameters::default().randomize_ci_pn(false));
     let mut server = default_server();
     let now = connect_rtt_idle(&mut client, &mut server, DEFAULT_RTT);
 

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

@@ -1329,7 +1329,9 @@ fn client_initial_retransmits_identical() {
 #[test]
 fn server_initial_retransmits_identical() {
     let mut now = now();
-    let mut client = default_client();
+    // We need to calculate largest_acked below, which is difficult is packet number randomization
+    // in enabled.
+    let mut client = new_client(ConnectionParameters::default().randomize_ci_pn(false));
     let mut ci = client.process_output(now).dgram();
     let mut ci2 = client.process_output(now).dgram();
 

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

@@ -90,11 +90,12 @@ 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.
+    // This test needs to decrypt the CI, so turn off MLKEM and packet number randomization.
     let mut client = new_client(
         ConnectionParameters::default()
             .versions(Version::Version1, vec![Version::Version1])
-            .mlkem(false),
+            .mlkem(false)
+            .randomize_ci_pn(false),
     );
     let mut server = default_server();
     let mut now = now();

neqo-transport/src/crypto.rs

@@ -4,6 +4,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+use core::panic;
 use std::{
     cell::RefCell,
     cmp::{max, min},
@@ -17,7 +18,7 @@ use enum_map::EnumMap;
 use neqo_common::{hex, hex_snip_middle, qdebug, qinfo, qtrace, Encoder, Role};
 pub use neqo_crypto::Epoch;
 use neqo_crypto::{
-    hkdf, hp::HpKey, Aead, Agent, AntiReplay, Cipher, Error as CryptoError, HandshakeState,
+    hkdf, hp::HpKey, random, Aead, Agent, AntiReplay, Cipher, Error as CryptoError, HandshakeState,
     PrivateKey, PublicKey, Record, RecordList, ResumptionToken, SymKey, ZeroRttChecker,
     TLS_AES_128_GCM_SHA256, TLS_AES_256_GCM_SHA384, TLS_CHACHA20_POLY1305_SHA256, TLS_CT_HANDSHAKE,
     TLS_GRP_EC_SECP256R1, TLS_GRP_EC_SECP384R1, TLS_GRP_EC_SECP521R1, TLS_GRP_EC_X25519,
@@ -243,6 +244,7 @@ impl Crypto {
 
     /// Lock in a compatible upgrade.
     pub fn confirm_version(&mut self, confirmed: Version) {
+        // FIXME: Should this error be suppressed here?
         _ = self.states.confirm_version(self.version, confirmed);
         self.version = confirmed;
     }
@@ -444,17 +446,23 @@ impl CryptoDxState {
         epoch: Epoch,
         secret: &SymKey,
         cipher: Cipher,
+        randomize_ci_pn: bool,
     ) -> Res<Self> {
-        qdebug!("Making {direction:?} {epoch:?} CryptoDxState, v={version:?} cipher={cipher}",);
+        let min_pn = if randomize_ci_pn {
+            PacketNumber::from((random::<1>()[0] >> 4) + 1)
+        } else {
+            0
+        };
+        qdebug!("Making {direction:?} {epoch:?} CryptoDxState, v={version:?} cipher={cipher} min_pn={min_pn}",);
         let hplabel = String::from(version.label_prefix()) + "hp";
         Ok(Self {
             version,
             direction,
             epoch: usize::from(epoch),
             aead: Aead::new(TLS_VERSION_1_3, cipher, secret, version.label_prefix())?,
             hpkey: HpKey::extract(TLS_VERSION_1_3, cipher, secret, &hplabel)?,
-            used_pn: 0..0,
-            min_pn: 0,
+            used_pn: 0..min_pn,
+            min_pn,
             invocations: Self::limit(direction, cipher),
             largest_packet_len: INITIAL_LARGEST_PACKET_LEN,
         })
@@ -465,6 +473,7 @@ impl CryptoDxState {
         direction: CryptoDxDirection,
         label: &str,
         dcid: &[u8],
+        randomize_ci_pn: bool,
     ) -> Res<Self> {
         qtrace!("new_initial {version:?} {}", ConnectionIdRef::from(dcid));
         let salt = version.initial_salt();
@@ -478,7 +487,14 @@ impl CryptoDxState {
 
         let secret = hkdf::expand_label(TLS_VERSION_1_3, cipher, &initial_secret, &[], label)?;
 
-        Self::new(version, direction, Epoch::Initial, &secret, cipher)
+        Self::new(
+            version,
+            direction,
+            Epoch::Initial,
+            &secret,
+            cipher,
+            randomize_ci_pn,
+        )
     }
 
     /// Determine the confidentiality and integrity limits for the cipher.
@@ -569,17 +585,17 @@ impl CryptoDxState {
         let next = prev.next_pn();
         self.min_pn = next;
         if self.used_pn.is_empty() {
-            self.used_pn = next..next;
+            self.used_pn = prev.used_pn.clone();
             Ok(())
-        } else if prev.used_pn.end > self.used_pn.start {
+        } else if prev.used_pn.end > self.min_pn {
             qdebug!(
                 "[{self}] Found packet with too new packet number {} > {}, compared to {prev}",
-                self.used_pn.start,
+                self.min_pn,
                 prev.used_pn.end,
             );
             Err(Error::PacketNumberOverlap)
         } else {
-            self.used_pn.start = next;
+            self.used_pn = next..max(next, self.used_pn.end);
             Ok(())
         }
     }
@@ -699,6 +715,7 @@ impl CryptoDxState {
             CryptoDxDirection::Write,
             "server in",
             CLIENT_CID,
+            false,
         )
         .unwrap()
     }
@@ -761,7 +778,7 @@ impl CryptoDxAppData {
         cipher: Cipher,
     ) -> Res<Self> {
         Ok(Self {
-            dx: CryptoDxState::new(version, dir, Epoch::ApplicationData, secret, cipher)?,
+            dx: CryptoDxState::new(version, dir, Epoch::ApplicationData, secret, cipher, false)?,
             cipher,
             next_secret: Self::update_secret(cipher, secret)?,
         })
@@ -940,7 +957,13 @@ impl CryptoStates {
 
     /// Create the initial crypto state.
     /// Note that the version here can change and that's OK.
-    pub fn init<'v, V>(&mut self, versions: V, role: Role, dcid: &[u8]) -> Res<()>
+    pub fn init<'v, V>(
+        &mut self,
+        versions: V,
+        role: Role,
+        dcid: &[u8],
+        randomize_ci_pn: bool,
+    ) -> Res<()>
     where
         V: IntoIterator<Item = &'v Version>,
     {
@@ -959,8 +982,14 @@ impl CryptoStates {
             );
 
             let mut initial = CryptoState {
-                tx: CryptoDxState::new_initial(*v, CryptoDxDirection::Write, write, dcid)?,
-                rx: CryptoDxState::new_initial(*v, CryptoDxDirection::Read, read, dcid)?,
+                tx: CryptoDxState::new_initial(
+                    *v,
+                    CryptoDxDirection::Write,
+                    write,
+                    dcid,
+                    randomize_ci_pn,
+                )?,
+                rx: CryptoDxState::new_initial(*v, CryptoDxDirection::Read, read, dcid, false)?,
             };
             if let Some(prev) = &self.initials[*v] {
                 qinfo!(
@@ -982,7 +1011,7 @@ impl CryptoStates {
     /// the overall effort is small enough to write off.
     pub fn init_server(&mut self, version: Version, dcid: &[u8]) -> Res<()> {
         if self.initials[version].is_none() {
-            self.init(&[version], Role::Server, dcid)?;
+            self.init(&[version], Role::Server, dcid, false)?;
         }
         Ok(())
     }
@@ -1018,6 +1047,7 @@ impl CryptoStates {
             Epoch::ZeroRtt,
             secret,
             cipher,
+            false,
         )?);
         Ok(())
     }
@@ -1059,13 +1089,15 @@ impl CryptoStates {
                 Epoch::Handshake,
                 write_secret,
                 cipher,
+                false,
             )?,
             rx: CryptoDxState::new(
                 version,
                 CryptoDxDirection::Read,
                 Epoch::Handshake,
                 read_secret,
                 cipher,
+                false,
             )?,
         });
         Ok(())

neqo-transport/src/stats.rs

@@ -174,6 +174,9 @@ pub struct Stats {
     /// Whether the connection was resumed successfully.
     pub resumed: bool,
 
+    /// The packet number of the first Initial packet sent.
+    pub first_initial_pn: Option<PacketNumber>,
+
     /// The current, estimated round-trip time on the primary path.
     pub rtt: Duration,
     /// The current, estimated round-trip time variation on the primary path.
@@ -270,6 +273,7 @@ impl Debug for Stats {
             self.pmtud_pmtu
         )?;
         writeln!(f, "  resumed: {}", self.resumed)?;
+        writeln!(f, "  first initial pn: {:?}", self.first_initial_pn)?;
         writeln!(f, "  frames rx:")?;
         self.frame_rx.fmt(f)?;
         writeln!(f, "  frames tx:")?;

neqo-transport/tests/connection.rs

@@ -70,11 +70,12 @@ 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.
+    // This test needs to decrypt the CI, so turn off MLKEM and packet number randomization.
     let mut client = new_client(
         ConnectionParameters::default()
             .versions(Version::Version1, vec![Version::Version1])
-            .mlkem(false),
+            .mlkem(false)
+            .randomize_ci_pn(false),
     );
     let mut server = default_server();
 

neqo-transport/tests/retry.rs

@@ -385,8 +385,13 @@ fn vn_after_retry() {
 // long enough connection ID.
 #[test]
 fn mitm_retry() {
-    // This test decrypts packets and hence does not work with MLKEM enabled.
-    let mut client = test_fixture::new_client(ConnectionParameters::default().mlkem(false));
+    // This test decrypts packets and hence does not work with MLKEM and packet number randomization
+    // enabled.
+    let mut client = test_fixture::new_client(
+        ConnectionParameters::default()
+            .mlkem(false)
+            .randomize_ci_pn(false),
+    );
     let mut retry_server = default_server();
     retry_server.set_validation(ValidateAddress::Always);
     let mut server = default_server();