Skip to content
3 changes: 3 additions & 0 deletions neqo-transport/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,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())?),
Expand All @@ -398,6 +399,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(
Expand Down Expand Up @@ -1342,6 +1344,7 @@ impl Connection {
self.conn_params.get_versions().compatible(),
self.role,
&retry_scid,
false,
)?;
self.address_validation = AddressValidationInfo::Retry {
token: packet.token().to_vec(),
Expand Down
14 changes: 14 additions & 0 deletions neqo-transport/src/connection/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,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 {
Expand Down Expand Up @@ -130,6 +132,7 @@ impl Default for ConnectionParameters {
pmtud_iface_mtu: true,
sni_slicing: true,
mlkem: true,
randomize_ci_pn: true,
}
}
}
Expand Down Expand Up @@ -423,6 +426,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
Expand Down
4 changes: 3 additions & 1 deletion neqo-transport/src/connection/tests/cc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
11 changes: 9 additions & 2 deletions neqo-transport/src/connection/tests/handshake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,9 @@ fn coalesce_05rtt() {
#[test]
fn reorder_handshake() {
const RTT: Duration = Duration::from_millis(100);
let mut client = default_client();
// TODO: Figure out why this test is failing when randomized packet number require two bytes of
// space. See below.
let mut client = new_client(ConnectionParameters::default().randomize_ci_pn(false));
let mut server = default_server();
let mut now = now();

Expand Down Expand Up @@ -561,6 +563,9 @@ fn reorder_handshake() {
assert_eq!(client.stats().saved_datagrams, 2);
assert_eq!(client.stats().packets_rx, 0);

// TODO: After this call, with single-byte packet numbers, the client has Handshake keys and can
// process saved packets. With two-byte packet numbers, it somehow doesn't have Handshake keys
// yet.
client.process_input(s_initial_2, now);
// Each saved packet should now be "received" again.
assert_eq!(client.stats().packets_rx, 3);
Expand Down Expand Up @@ -1325,7 +1330,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();

Expand Down
5 changes: 3 additions & 2 deletions neqo-transport/src/connection/tests/resumption.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,12 @@ fn ticket_rtt(rtt: Duration) -> Duration {
// A simple ACK frame for a single packet with packet number 0.
const ACK_FRAME_1: &[u8] = &[0x02, 0x00, 0x00, 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();
Expand Down
50 changes: 36 additions & 14 deletions neqo-transport/src/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@
use neqo_common::{hex, hex_snip_middle, qdebug, qinfo, qtrace, Buffer, Encoder, Role};
pub use neqo_crypto::Epoch;
use neqo_crypto::{
hkdf, hp, 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, TLS_GRP_KEM_MLKEM768X25519,
TLS_VERSION_1_3,
hkdf, hp, 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,
TLS_GRP_KEM_MLKEM768X25519, TLS_VERSION_1_3,
};

use crate::{
Expand Down Expand Up @@ -471,17 +471,18 @@
epoch: Epoch,
secret: &SymKey,
cipher: Cipher,
min_pn: packet::Number,
) -> Res<Self> {
qdebug!("Making {direction:?} {epoch:?} CryptoDxState, v={version:?} cipher={cipher}",);
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: hp::Key::extract(TLS_VERSION_1_3, cipher, secret, &hplabel)?,
used_pn: 0..0,
min_pn: 0,
used_pn: min_pn..min_pn,
min_pn,
invocations: Self::limit(direction, cipher),
largest_packet_len: INITIAL_LARGEST_PACKET_LEN,
})
Expand All @@ -492,6 +493,7 @@
direction: CryptoDxDirection,
label: &str,
dcid: &[u8],
min_pn: packet::Number,
) -> Res<Self> {
qtrace!("new_initial {version:?} {}", ConnectionIdRef::from(dcid));
let salt = version.initial_salt();
Expand All @@ -505,7 +507,7 @@

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, min_pn)
}

/// Determine the confidentiality and integrity limits for the cipher.
Expand Down Expand Up @@ -726,6 +728,7 @@
CryptoDxDirection::Write,
"server in",
CLIENT_CID,
0,
)
.unwrap()
}
Expand Down Expand Up @@ -788,7 +791,7 @@
cipher: Cipher,
) -> Res<Self> {
Ok(Self {
dx: CryptoDxState::new(version, dir, Epoch::ApplicationData, secret, cipher)?,
dx: CryptoDxState::new(version, dir, Epoch::ApplicationData, secret, cipher, 0)?,
cipher,
next_secret: Self::update_secret(cipher, secret)?,
})
Expand Down Expand Up @@ -967,7 +970,13 @@

/// 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>,
{
Expand All @@ -979,15 +988,22 @@
Role::Server => (SERVER_INITIAL_LABEL, CLIENT_INITIAL_LABEL),
};

let min_pn = if randomize_ci_pn {
let r = random::<2>();
packet::Number::from(r[0] & r[1]) + 1

Check warning on line 993 in neqo-transport/src/crypto.rs

View workflow job for this annotation

GitHub Actions / Find mutants

Missed mutant

replace & with ^ in CryptoStates::init
} else {
0
};

for v in versions {
qdebug!(
"[{self}] Creating initial cipher state v={v:?}, role={role:?} dcid={}",
hex(dcid)
);

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, min_pn)?,
rx: CryptoDxState::new_initial(*v, CryptoDxDirection::Read, read, dcid, 0)?,
};
if let Some(prev) = &self.initials[*v] {
qinfo!(
Expand All @@ -1009,7 +1025,10 @@
/// 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)?;
// We are not randomizing the server initial packet number, since we don't ship the
// server as a product, and doing so means more changes to the current tests to handle
// that (or disable it there).
self.init(&[version], Role::Server, dcid, false)?;
}
Ok(())
}
Expand Down Expand Up @@ -1045,6 +1064,7 @@
Epoch::ZeroRtt,
secret,
cipher,
0,
)?);
Ok(())
}
Expand Down Expand Up @@ -1086,13 +1106,15 @@
Epoch::Handshake,
write_secret,
cipher,
0,
)?,
rx: CryptoDxState::new(
version,
CryptoDxDirection::Read,
Epoch::Handshake,
read_secret,
cipher,
0,
)?,
});
Ok(())
Expand Down
4 changes: 2 additions & 2 deletions neqo-transport/src/saved.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@
pub fn save(&mut self, epoch: Epoch, d: Datagram, t: Instant) {
let store = self.store(epoch);

if store.len() < MAX_SAVED_DATAGRAMS {

Check warning on line 43 in neqo-transport/src/saved.rs

View workflow job for this annotation

GitHub Actions / Find mutants

Missed mutant

replace < with <= in SavedDatagrams::save
qdebug!("saving datagram of {} bytes", d.len());
qdebug!("saving {epoch:?} datagram of {} bytes", d.len());
store.push(SavedDatagram { d, t });
} else {
qinfo!("not saving datagram of {} bytes", d.len());
qinfo!("not saving {epoch:?} datagram of {} bytes", d.len());
}
}

Expand Down
27 changes: 25 additions & 2 deletions neqo-transport/tests/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,12 @@ fn reorder_server_initial() {
// A simple ACK frame for a single packet with packet number 0.
const ACK_FRAME: &[u8] = &[0x02, 0x00, 0x00, 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();

Expand Down Expand Up @@ -321,3 +322,25 @@ fn handshake_mlkem768x25519() {
neqo_crypto::TLS_GRP_KEM_MLKEM768X25519
);
}

#[test]
fn client_initial_packet_number() {
// Check that the initial packet number is randomized (i.e, > 0) if the `randomize_ci_pn`
// connection parameter is set, and that it is zero when not.
for randomize_ci_pn in [true, false] {
// This test needs to decrypt the CI, so turn off MLKEM.
let mut client = new_client(
ConnectionParameters::default()
.versions(Version::Version1, vec![Version::Version1])
.mlkem(false)
.randomize_ci_pn(randomize_ci_pn),
);

let client_initial = client.process_output(now());
let (protected_header, client_dcid, _, payload) =
decode_initial_header(client_initial.as_dgram_ref().unwrap(), Role::Client).unwrap();
let (_, hp) = initial_aead_and_hp(client_dcid, Role::Client);
let (_, pn) = header_protection::remove(&hp, protected_header, payload);
assert!(randomize_ci_pn && pn > 0 || !randomize_ci_pn && pn == 0);
}
}
9 changes: 7 additions & 2 deletions neqo-transport/tests/retry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,8 +438,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();
Expand Down
Loading