From 8eeefd30486386c5ac97999be6289e23ac0b560c Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Wed, 15 Jan 2025 12:45:43 +0200 Subject: [PATCH 1/8] feat: Sock puppets for Changes This is a proof of concept for an idea that was discussed with @martinthomson a while ago. That idea is to prepend the actual CH with an invalid "sock puppet" CH that is invalid but contains an innocuous SNI. WIP --- neqo-transport/src/connection/mod.rs | 77 +++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 2 deletions(-) diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs index 023545354b..36ea35c875 100644 --- a/neqo-transport/src/connection/mod.rs +++ b/neqo-transport/src/connection/mod.rs @@ -278,6 +278,10 @@ pub struct Connection { release_resumption_token_timer: Option, conn_params: ConnectionParameters, hrtime: hrtime::Handle, + /// The "sock puppet" to use for this connection, i.e., a set of bytes making up an invalid CH + /// with an innocuous SNI that is prepended to the actual CH. Empty when a connection is only + /// used to generate a sock puppet for another (real) one. + sock_puppet: Vec, /// For testing purposes it is sometimes necessary to inject frames that wouldn't /// otherwise be sent, just to see how a connection handles them. Inserting them @@ -315,13 +319,65 @@ impl Connection { conn_params: ConnectionParameters, now: Instant, ) -> Res { + // Create a "sock puppet", i.e., an invalid CH with an innoccuous SNI. + // The sock puppet and the real connection need to use the same DCID. let dcid = ConnectionId::generate_initial(); + let sock_puppet = Self::new_client_internal( + "github.com", + protocols, + Rc::clone(&cid_generator), + dcid.clone(), + local_addr, + remote_addr, + conn_params.clone(), + now, + Vec::new(), + ) + .map_or(Vec::new(), |mut sock_puppet| { + sock_puppet + .process_output(now) + .dgram() + .map_or_else(Vec::new, |sp| { + // Invalidate the CH. + let mut sp = sp.to_vec(); + let pos = sp.len() - 1; + sp[pos] = sp[pos].wrapping_add(1); + qdebug!("Created sock puppet {:?}", sp); + sp + }) + }); + Self::new_client_internal( + server_name, + protocols, + cid_generator, + dcid, + local_addr, + remote_addr, + conn_params, + now, + sock_puppet, + ) + } + + #[allow(clippy::too_many_arguments)] // Yes, but we need them all. + fn new_client_internal( + server_name: impl Into, + protocols: &[impl AsRef], + cid_generator: Rc>, + dcid: ConnectionId, + local_addr: SocketAddr, + remote_addr: SocketAddr, + conn_params: ConnectionParameters, + now: Instant, + sock_puppet: Vec, + ) -> Res { let mut c = Self::new( Role::Client, Agent::from(Client::new(server_name.into(), conn_params.is_greasing())?), cid_generator, protocols, conn_params, + sock_puppet, )?; c.crypto.states.init( c.conn_params.get_versions().compatible(), @@ -357,6 +413,7 @@ impl Connection { cid_generator, protocols, conn_params, + Vec::new(), ) } @@ -366,6 +423,7 @@ impl Connection { cid_generator: Rc>, protocols: &[P], conn_params: ConnectionParameters, + sock_puppet: Vec, ) -> Res { // Setup the local connection ID. let local_initial_source_cid = cid_generator @@ -426,6 +484,7 @@ impl Connection { conn_params, hrtime: hrtime::Time::get(Self::LOOSE_TIMER_RESOLUTION), quic_datagrams, + sock_puppet, #[cfg(test)] test_frame_writer: None, }; @@ -2480,7 +2539,8 @@ impl Connection { // 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); - needs_padding = true; + // The sock puppet CH must not be padded, since it's being used as padding itself. + needs_padding = !self.sock_puppet.is_empty(); } else { if pt == PacketType::Handshake && self.role == Role::Client { needs_padding = false; @@ -2511,7 +2571,20 @@ impl Connection { packets.len(), profile.limit() ); - initial.track_padding(profile.limit() - packets.len()); + let padding_len = profile.limit() - packets.len(); + initial.track_padding(padding_len); + if !self.sock_puppet.is_empty() { + if self.sock_puppet.len() <= padding_len { + qdebug!("Prepending sock puppet CH, len={}", self.sock_puppet.len()); + packets.splice(0..0, self.sock_puppet.clone()); + } else { + qwarn!( + "Sock puppet CH too long ({} > {}), not prepending", + self.sock_puppet.len(), + padding_len + ); + } + } // These zeros aren't padding frames, they are an invalid all-zero coalesced // packet, which is why we don't increase `frame_tx.padding` count here. packets.resize(profile.limit(), 0); From d4c2c8fd2af9aae765b69da7ef150f259262f14e Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Wed, 15 Jan 2025 16:02:47 +0200 Subject: [PATCH 2/8] Tweaks --- neqo-transport/src/connection/mod.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs index 36ea35c875..f0d0be8679 100644 --- a/neqo-transport/src/connection/mod.rs +++ b/neqo-transport/src/connection/mod.rs @@ -322,7 +322,7 @@ impl Connection { // Create a "sock puppet", i.e., an invalid CH with an innoccuous SNI. // The sock puppet and the real connection need to use the same DCID. let dcid = ConnectionId::generate_initial(); - let sock_puppet = Self::new_client_internal( + let sock_puppet = Self::new_client_common( "github.com", protocols, Rc::clone(&cid_generator), @@ -338,7 +338,9 @@ impl Connection { .process_output(now) .dgram() .map_or_else(Vec::new, |sp| { - // Invalidate the CH. + // Invalidate the CH by incrementing the last byte, which breaks the checksum on + // the blob. TODO: Find a clever way to invalidate the CH, ideally based on + // something within the TLS data. let mut sp = sp.to_vec(); let pos = sp.len() - 1; sp[pos] = sp[pos].wrapping_add(1); @@ -346,7 +348,7 @@ impl Connection { sp }) }); - Self::new_client_internal( + Self::new_client_common( server_name, protocols, cid_generator, @@ -360,7 +362,7 @@ impl Connection { } #[allow(clippy::too_many_arguments)] // Yes, but we need them all. - fn new_client_internal( + fn new_client_common( server_name: impl Into, protocols: &[impl AsRef], cid_generator: Rc>, @@ -2583,6 +2585,7 @@ impl Connection { self.sock_puppet.len(), padding_len ); + // TODO: Is there a way to trim the size of the sock puppet CH? } } // These zeros aren't padding frames, they are an invalid all-zero coalesced From 9e32b476e1a5fc2c79b582b23389e0a0ea4034d4 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Thu, 16 Jan 2025 11:18:52 +0200 Subject: [PATCH 3/8] Progress --- neqo-transport/src/connection/mod.rs | 62 ++++++++++--------- neqo-transport/src/connection/params.rs | 14 +++++ .../src/connection/tests/handshake.rs | 17 ++--- neqo-transport/src/connection/tests/keys.rs | 4 +- neqo-transport/src/connection/tests/vn.rs | 8 +-- neqo-transport/tests/server.rs | 3 +- 6 files changed, 65 insertions(+), 43 deletions(-) diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs index f0d0be8679..594a1c5765 100644 --- a/neqo-transport/src/connection/mod.rs +++ b/neqo-transport/src/connection/mod.rs @@ -281,7 +281,8 @@ pub struct Connection { /// The "sock puppet" to use for this connection, i.e., a set of bytes making up an invalid CH /// with an innocuous SNI that is prepended to the actual CH. Empty when a connection is only /// used to generate a sock puppet for another (real) one. - sock_puppet: Vec, + sock_puppet: Vec, /* TODO: Should this be an Option>? Could it be a &[u8] or an + * Option<&[u8]>? */ /// For testing purposes it is sometimes necessary to inject frames that wouldn't /// otherwise be sent, just to see how a connection handles them. Inserting them @@ -322,32 +323,37 @@ impl Connection { // Create a "sock puppet", i.e., an invalid CH with an innoccuous SNI. // The sock puppet and the real connection need to use the same DCID. let dcid = ConnectionId::generate_initial(); - let sock_puppet = Self::new_client_common( - "github.com", - protocols, - Rc::clone(&cid_generator), - dcid.clone(), - local_addr, - remote_addr, - conn_params.clone(), - now, - Vec::new(), - ) - .map_or(Vec::new(), |mut sock_puppet| { - sock_puppet - .process_output(now) - .dgram() - .map_or_else(Vec::new, |sp| { - // Invalidate the CH by incrementing the last byte, which breaks the checksum on - // the blob. TODO: Find a clever way to invalidate the CH, ideally based on - // something within the TLS data. - let mut sp = sp.to_vec(); - let pos = sp.len() - 1; - sp[pos] = sp[pos].wrapping_add(1); - qdebug!("Created sock puppet {:?}", sp); - sp - }) - }); + let sock_puppet = if conn_params.sock_puppet_enabled() { + Self::new_client_common( + "github.com", + protocols, + Rc::clone(&cid_generator), + dcid.clone(), + local_addr, + remote_addr, + conn_params.clone(), + now, + Vec::new(), + ) + .map_or(Vec::new(), |mut sock_puppet| { + sock_puppet + .process_output(now) + .dgram() + .map_or_else(Vec::new, |sp| { + // Invalidate the CH by incrementing the last byte, which breaks the + // checksum on the blob. TODO: Find a clever way to + // invalidate the CH, ideally based on + // something within the TLS data. + let mut sp = sp.to_vec(); + let pos = sp.len() - 1; + sp[pos] = sp[pos].wrapping_add(1); + qdebug!("Created sock puppet CH of len {}", sp.len()); + sp + }) + }) + } else { + Vec::new() + }; Self::new_client_common( server_name, protocols, @@ -2542,7 +2548,7 @@ impl Connection { // track that padding along with the Initial packet. So defer tracking. initial_sent = Some(sent); // The sock puppet CH must not be padded, since it's being used as padding itself. - needs_padding = !self.sock_puppet.is_empty(); + needs_padding = self.role == Role::Server || !self.sock_puppet.is_empty(); } else { if pt == PacketType::Handshake && self.role == Role::Client { needs_padding = false; diff --git a/neqo-transport/src/connection/params.rs b/neqo-transport/src/connection/params.rs index ce27440543..9bba7f8727 100644 --- a/neqo-transport/src/connection/params.rs +++ b/neqo-transport/src/connection/params.rs @@ -83,6 +83,8 @@ pub struct ConnectionParameters { pacing: bool, /// Whether the connection performs PLPMTUD. pmtud: bool, + /// Whether the connection should use sock puppet CHs. + sock_puppet: bool, } impl Default for ConnectionParameters { @@ -107,6 +109,7 @@ impl Default for ConnectionParameters { disable_migration: false, pacing: true, pmtud: false, + sock_puppet: true, } } } @@ -367,6 +370,17 @@ impl ConnectionParameters { self } + #[must_use] + pub const fn sock_puppet_enabled(&self) -> bool { + self.sock_puppet + } + + #[must_use] + pub const fn sock_puppet(mut self, sock_puppet: bool) -> Self { + self.sock_puppet = sock_puppet; + self + } + /// # Errors /// When a connection ID cannot be obtained. /// # Panics diff --git a/neqo-transport/src/connection/tests/handshake.rs b/neqo-transport/src/connection/tests/handshake.rs index c9ff3a5f41..751925bdfc 100644 --- a/neqo-transport/src/connection/tests/handshake.rs +++ b/neqo-transport/src/connection/tests/handshake.rs @@ -572,18 +572,19 @@ fn reorder_1rtt() { server.process_input(d, now + RTT / 2); } // The server has now received those packets, and saved them. - // The two extra received are Initial + the junk we use for padding. - assert_eq!(server.stats().packets_rx, PACKETS + 2); + // The three extra received are sock puppet CH + Initial + the junk we use for padding. + assert_eq!(server.stats().packets_rx, PACKETS + 3); assert_eq!(server.stats().saved_datagrams, PACKETS); - assert_eq!(server.stats().dropped_rx, 1); + assert_eq!(server.stats().dropped_rx, 2); now += RTT / 2; let s2 = server.process(c2, now).dgram(); // The server has now received those packets, and saved them. - // The two additional are a Handshake and a 1-RTT (w/ NEW_CONNECTION_ID). - assert_eq!(server.stats().packets_rx, PACKETS * 2 + 4); + // The three additional are a Handshake and a 1-RTT (w/ NEW_CONNECTION_ID) and its sock puppet + // CH. + assert_eq!(server.stats().packets_rx, PACKETS * 2 + 5); assert_eq!(server.stats().saved_datagrams, PACKETS); - assert_eq!(server.stats().dropped_rx, 1); + assert_eq!(server.stats().dropped_rx, 2); assert_eq!(*server.state(), State::Confirmed); assert_eq!(server.paths.rtt(), RTT); @@ -630,8 +631,8 @@ fn corrupted_initial() { server.process_input(dgram, now()); // The server should have received two packets, // the first should be dropped, the second saved. - assert_eq!(server.stats().packets_rx, 2); - assert_eq!(server.stats().dropped_rx, 2); + assert_eq!(server.stats().packets_rx, 3); + assert_eq!(server.stats().dropped_rx, 3); assert_eq!(server.stats().saved_datagrams, 0); } diff --git a/neqo-transport/src/connection/tests/keys.rs b/neqo-transport/src/connection/tests/keys.rs index d8686b7943..9a52d50cec 100644 --- a/neqo-transport/src/connection/tests/keys.rs +++ b/neqo-transport/src/connection/tests/keys.rs @@ -80,8 +80,8 @@ fn discarded_initial_keys() { // The server has not removed the Initial keys yet, because it has not yet received a Handshake // 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); + // The dropped packets are the sock puppet CH and padding. The Initial packet has been mark dup. + check_discarded(&mut server, &init_pkt_c.clone().unwrap(), false, 2, 1); qdebug!("---- client: SH..FIN -> FIN"); let out = client.process_output(now()).dgram(); diff --git a/neqo-transport/src/connection/tests/vn.rs b/neqo-transport/src/connection/tests/vn.rs index 60fafd56aa..61c2c69eb9 100644 --- a/neqo-transport/src/connection/tests/vn.rs +++ b/neqo-transport/src/connection/tests/vn.rs @@ -330,10 +330,10 @@ fn invalid_server_version() { let dgram = client.process_output(now()).dgram(); server.process_input(dgram.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); + // Two packets received, one is the sock puppet CH. + assert_eq!(server.stats().packets_rx, 2); + // Sock puppet CH was dropped. + assert_eq!(server.stats().dropped_rx, 1); assert_eq!(server.stats().saved_datagrams, 0); // The server effectively hasn't reacted here. match server.state() { diff --git a/neqo-transport/tests/server.rs b/neqo-transport/tests/server.rs index c0a10b9afb..4c0aef629f 100644 --- a/neqo-transport/tests/server.rs +++ b/neqo-transport/tests/server.rs @@ -421,7 +421,8 @@ fn new_token_different_port() { #[test] fn bad_client_initial() { - let mut client = default_client(); + // Sock puppets don't work with the decryption this test uses. + let mut client = new_client(ConnectionParameters::default().sock_puppet(false)); let mut server = default_server(); let dgram = client.process_output(now()).dgram().expect("a datagram"); From 747d3d931750d70e5074b8c6265ea6623ea1999f Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Thu, 16 Jan 2025 12:46:31 +0200 Subject: [PATCH 4/8] Make SNI slicing controllable through a connection parameter --- neqo-transport/src/connection/mod.rs | 19 ++++++++++++++----- neqo-transport/src/connection/params.rs | 14 ++++++++++++++ neqo-transport/src/connection/tests/idle.rs | 2 ++ neqo-transport/src/crypto.rs | 7 +++++-- 4 files changed, 35 insertions(+), 7 deletions(-) diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs index 594a1c5765..726a2dcf24 100644 --- a/neqo-transport/src/connection/mod.rs +++ b/neqo-transport/src/connection/mod.rs @@ -331,7 +331,10 @@ impl Connection { dcid.clone(), local_addr, remote_addr, - conn_params.clone(), + conn_params + .clone() + .sni_slicing(false) // We want this SNI to be easily findable. + .grease(false), // We also don't need grease here. now, Vec::new(), ) @@ -341,9 +344,8 @@ impl Connection { .dgram() .map_or_else(Vec::new, |sp| { // Invalidate the CH by incrementing the last byte, which breaks the - // checksum on the blob. TODO: Find a clever way to - // invalidate the CH, ideally based on - // something within the TLS data. + // checksum on the blob. TODO: Find a clever way to invalidate the CH, + // ideally based on something within the TLS data. let mut sp = sp.to_vec(); let pos = sp.len() - 1; sp[pos] = sp[pos].wrapping_add(1); @@ -2227,6 +2229,7 @@ impl Connection { let frame_stats = &mut stats.frame_tx; self.crypto.write_frame( PacketNumberSpace::ApplicationData, + self.conn_params.sni_slicing_enabled(), builder, tokens, frame_stats, @@ -2361,7 +2364,13 @@ impl Connection { self.write_appdata_frames(builder, &mut tokens); } else { let stats = &mut self.stats.borrow_mut().frame_tx; - self.crypto.write_frame(space, builder, &mut tokens, stats); + self.crypto.write_frame( + space, + self.conn_params.sni_slicing_enabled(), + builder, + &mut tokens, + stats, + ); } } diff --git a/neqo-transport/src/connection/params.rs b/neqo-transport/src/connection/params.rs index 9bba7f8727..dcbe6fd66c 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 sock puppet CHs. sock_puppet: bool, + /// Whether the connection should use SNI slicing. + sni_slicing: bool, } impl Default for ConnectionParameters { @@ -110,6 +112,7 @@ impl Default for ConnectionParameters { pacing: true, pmtud: false, sock_puppet: true, + sni_slicing: true, } } } @@ -381,6 +384,17 @@ impl ConnectionParameters { self } + #[must_use] + pub const fn sni_slicing_enabled(&self) -> bool { + self.sni_slicing + } + + #[must_use] + pub const fn sni_slicing(mut self, sni_slicing: bool) -> Self { + self.sni_slicing = sni_slicing; + self + } + /// # Errors /// When a connection ID cannot be obtained. /// # Panics diff --git a/neqo-transport/src/connection/tests/idle.rs b/neqo-transport/src/connection/tests/idle.rs index b71a9d0ee0..38f657b89b 100644 --- a/neqo-transport/src/connection/tests/idle.rs +++ b/neqo-transport/src/connection/tests/idle.rs @@ -307,6 +307,7 @@ fn idle_caching() { let mut tokens = Vec::new(); server.crypto.streams.write_frame( PacketNumberSpace::Initial, + server.conn_params.sni_slicing_enabled(), &mut builder, &mut tokens, &mut FrameStats::default(), @@ -315,6 +316,7 @@ fn idle_caching() { tokens.clear(); server.crypto.streams.write_frame( PacketNumberSpace::Initial, + server.conn_params.sni_slicing_enabled(), &mut builder, &mut tokens, &mut FrameStats::default(), diff --git a/neqo-transport/src/crypto.rs b/neqo-transport/src/crypto.rs index 64d0e09a3e..ee0547a549 100644 --- a/neqo-transport/src/crypto.rs +++ b/neqo-transport/src/crypto.rs @@ -327,11 +327,13 @@ impl Crypto { pub fn write_frame( &mut self, space: PacketNumberSpace, + sni_slicing: bool, builder: &mut PacketBuilder, tokens: &mut Vec, stats: &mut FrameStats, ) { - self.streams.write_frame(space, builder, tokens, stats); + self.streams + .write_frame(space, sni_slicing, builder, tokens, stats); } pub fn acked(&mut self, token: &CryptoRecoveryToken) { @@ -1475,6 +1477,7 @@ impl CryptoStreams { pub fn write_frame( &mut self, space: PacketNumberSpace, + sni_slicing: bool, builder: &mut PacketBuilder, tokens: &mut Vec, stats: &mut FrameStats, @@ -1506,7 +1509,7 @@ impl CryptoStreams { let cs = self.get_mut(space).unwrap(); if let Some((offset, data)) = cs.tx.next_bytes() { - let written = if offset == 0 { + let written = if sni_slicing && offset == 0 { if let Some(sni) = find_sni(data) { // Cut the crypto data in two at the midpoint of the SNI and swap the chunks. let mid = sni.start + (sni.end - sni.start) / 2; From 587baffd5078203fcc50e1ad4c7777ec36888e9a Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Thu, 16 Jan 2025 15:56:05 +0200 Subject: [PATCH 5/8] Restructure --- neqo-transport/src/connection/mod.rs | 120 ++++-------- neqo-transport/src/connection/sockpuppet.rs | 197 ++++++++++++++++++++ neqo-transport/src/crypto.rs | 4 +- 3 files changed, 235 insertions(+), 86 deletions(-) create mode 100644 neqo-transport/src/connection/sockpuppet.rs diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs index 726a2dcf24..114fe30f5f 100644 --- a/neqo-transport/src/connection/mod.rs +++ b/neqo-transport/src/connection/mod.rs @@ -30,6 +30,7 @@ use neqo_crypto::{ Server, ZeroRttChecker, }; use smallvec::SmallVec; +use sockpuppet::sock_puppet; use crate::{ addr_valid::{AddressValidation, NewTokenState}, @@ -65,6 +66,7 @@ mod dump; mod idle; pub mod params; mod saved; +mod sockpuppet; mod state; #[cfg(test)] pub mod test_internal; @@ -278,11 +280,6 @@ pub struct Connection { release_resumption_token_timer: Option, conn_params: ConnectionParameters, hrtime: hrtime::Handle, - /// The "sock puppet" to use for this connection, i.e., a set of bytes making up an invalid CH - /// with an innocuous SNI that is prepended to the actual CH. Empty when a connection is only - /// used to generate a sock puppet for another (real) one. - sock_puppet: Vec, /* TODO: Should this be an Option>? Could it be a &[u8] or an - * Option<&[u8]>? */ /// For testing purposes it is sometimes necessary to inject frames that wouldn't /// otherwise be sent, just to see how a connection handles them. Inserting them @@ -320,74 +317,13 @@ impl Connection { conn_params: ConnectionParameters, now: Instant, ) -> Res { - // Create a "sock puppet", i.e., an invalid CH with an innoccuous SNI. - // The sock puppet and the real connection need to use the same DCID. let dcid = ConnectionId::generate_initial(); - let sock_puppet = if conn_params.sock_puppet_enabled() { - Self::new_client_common( - "github.com", - protocols, - Rc::clone(&cid_generator), - dcid.clone(), - local_addr, - remote_addr, - conn_params - .clone() - .sni_slicing(false) // We want this SNI to be easily findable. - .grease(false), // We also don't need grease here. - now, - Vec::new(), - ) - .map_or(Vec::new(), |mut sock_puppet| { - sock_puppet - .process_output(now) - .dgram() - .map_or_else(Vec::new, |sp| { - // Invalidate the CH by incrementing the last byte, which breaks the - // checksum on the blob. TODO: Find a clever way to invalidate the CH, - // ideally based on something within the TLS data. - let mut sp = sp.to_vec(); - let pos = sp.len() - 1; - sp[pos] = sp[pos].wrapping_add(1); - qdebug!("Created sock puppet CH of len {}", sp.len()); - sp - }) - }) - } else { - Vec::new() - }; - Self::new_client_common( - server_name, - protocols, - cid_generator, - dcid, - local_addr, - remote_addr, - conn_params, - now, - sock_puppet, - ) - } - - #[allow(clippy::too_many_arguments)] // Yes, but we need them all. - fn new_client_common( - server_name: impl Into, - protocols: &[impl AsRef], - cid_generator: Rc>, - dcid: ConnectionId, - local_addr: SocketAddr, - remote_addr: SocketAddr, - conn_params: ConnectionParameters, - now: Instant, - sock_puppet: Vec, - ) -> Res { let mut c = Self::new( Role::Client, Agent::from(Client::new(server_name.into(), conn_params.is_greasing())?), cid_generator, protocols, conn_params, - sock_puppet, )?; c.crypto.states.init( c.conn_params.get_versions().compatible(), @@ -423,7 +359,6 @@ impl Connection { cid_generator, protocols, conn_params, - Vec::new(), ) } @@ -433,7 +368,6 @@ impl Connection { cid_generator: Rc>, protocols: &[P], conn_params: ConnectionParameters, - sock_puppet: Vec, ) -> Res { // Setup the local connection ID. let local_initial_source_cid = cid_generator @@ -494,7 +428,6 @@ impl Connection { conn_params, hrtime: hrtime::Time::get(Self::LOOSE_TIMER_RESOLUTION), quic_datagrams, - sock_puppet, #[cfg(test)] test_frame_writer: None, }; @@ -2447,9 +2380,26 @@ impl Connection { let profile = self.loss_recovery.send_profile(&path.borrow(), now); qdebug!("[{self}] output_path send_profile {profile:?}"); + let sock_puppet = if let Some((_, tx)) = self + .crypto + .states + .select_tx_mut(self.version, PacketNumberSpace::Initial) + { + sock_puppet( + tx, + path, + &self.address_validation, + version, + &self.loss_recovery, + ) + } else { + Vec::new() + }; + // Frames for different epochs must go in different packets, but then these // packets can go in a single datagram let mut encoder = Encoder::with_capacity(profile.limit()); + let mut reserved = 0; for space in PacketNumberSpace::iter() { // Ensure we have tx crypto state for this epoch, or skip it. let Some((cspace, tx)) = self.crypto.states.select_tx_mut(self.version, *space) else { @@ -2479,9 +2429,14 @@ impl Connection { // Configure the limits and padding for this packet. let aead_expansion = tx.expansion(); + if pt == PacketType::Initial && self.role == Role::Client && pn == 0 { + qdebug!("Reserving {} space for sock puppet CH", sock_puppet.len()); + reserved += sock_puppet.len(); + }; + needs_padding |= builder.set_initial_limit( &profile, - aead_expansion, + aead_expansion + reserved, self.paths .primary() .ok_or(Error::InternalError)? @@ -2556,8 +2511,7 @@ impl Connection { // 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); - // The sock puppet CH must not be padded, since it's being used as padding itself. - needs_padding = self.role == Role::Server || !self.sock_puppet.is_empty(); + needs_padding = true; } else { if pt == PacketType::Handshake && self.role == Role::Client { needs_padding = false; @@ -2590,18 +2544,16 @@ impl Connection { ); let padding_len = profile.limit() - packets.len(); initial.track_padding(padding_len); - if !self.sock_puppet.is_empty() { - if self.sock_puppet.len() <= padding_len { - qdebug!("Prepending sock puppet CH, len={}", self.sock_puppet.len()); - packets.splice(0..0, self.sock_puppet.clone()); - } else { - qwarn!( - "Sock puppet CH too long ({} > {}), not prepending", - self.sock_puppet.len(), - padding_len - ); - // TODO: Is there a way to trim the size of the sock puppet CH? - } + if sock_puppet.len() <= padding_len { + qdebug!("Prepending sock puppet CH, len={}", sock_puppet.len()); + packets.splice(0..0, sock_puppet); + } else { + qwarn!( + "Sock puppet CH too long ({} > {}), not prepending", + sock_puppet.len(), + padding_len + ); + // TODO: Is there a way to trim the size of the sock puppet CH? } // These zeros aren't padding frames, they are an invalid all-zero coalesced // packet, which is why we don't increase `frame_tx.padding` count here. diff --git a/neqo-transport/src/connection/sockpuppet.rs b/neqo-transport/src/connection/sockpuppet.rs new file mode 100644 index 0000000000..ef74803d9b --- /dev/null +++ b/neqo-transport/src/connection/sockpuppet.rs @@ -0,0 +1,197 @@ +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use neqo_common::{qdebug, Encoder}; +use neqo_crypto::random; + +use super::AddressValidationInfo; +use crate::{ + crypto::{CryptoDxState, CryptoSpace, CryptoStreams}, + path::PathRef, + recovery::LossRecovery, + stats::FrameStats, + tracking::PacketNumberSpace, + Connection, Version, +}; + +/// See +#[expect(clippy::too_many_lines)] +fn generate_ch() -> Vec { + // Extensions + let mut extensions = Encoder::new(); + + let mut list_entry = Encoder::new(); + list_entry.encode_uint(1, 0x00_u8); // list entry is type 0x00 "DNS hostname" + list_entry.encode_vec(2, b"github.com"); + + let mut server_name_list = Encoder::new(); + server_name_list.encode_vec(2, list_entry.as_ref()); // first (and only) list entry + + let mut server_name_extension = Encoder::new(); + server_name_extension.encode_vec(2, server_name_list.as_ref()); // "server name" extension data + + extensions.encode_uint(2, 0x00_u8); // assigned value for extension "server name" + extensions.encode_vec(2, server_name_extension.as_ref()); // server name extension data + + let mut ec_points_formats = Encoder::new(); + ec_points_formats.encode_vec( + 1, + &[ + 0x00, // assigned value for format "uncompressed" + 0x01, // assigned value for format "ansiX962_compressed_prime" + 0x02, // assigned value for format "ansiX962_compressed_char2" + ], + ); + + extensions.encode_uint(2, 0x0b_u8); // assigned value for extension "ec point formats" + extensions.encode_vec(2, ec_points_formats.as_ref()); // ec point formats extension data + + let mut supported_groups = Encoder::new(); + supported_groups.encode_vec( + 2, + &[ + 0x00, 0x1d, // assigned value for the curve "x25519" + 0x00, 0x17, // assigned value for the curve "secp256r1" + 0x00, 0x1e, // assigned value for the curve "x448" + 0x00, 0x19, // assigned value for the curve "secp521r1" + 0x00, 0x18, // assigned value for the curve "secp384r1" + 0x01, 0x00, // assigned value for the curve "ffdhe2048" + 0x01, 0x01, // assigned value for the curve "ffdhe3072" + 0x01, 0x02, // assigned value for the curve "ffdhe4096" + 0x01, 0x03, // assigned value for the curve "ffdhe6144" + 0x01, 0x04, // assigned value for the curve "ffdhe8192" + ], + ); + + extensions.encode_uint(2, 0x0a_u8); // assigned value for extension "supported groups" + extensions.encode_vec(2, supported_groups.as_ref()); // supported groups extension data + + extensions.encode_uint(2, 0x23_u8); // assigned value for extension "Session Ticket" + extensions.encode_vec(2, &[]); // 0 bytes of "Session Ticket" extension data + + extensions.encode_uint(2, 0x17_u8); // assigned value for extension "Extended Master Secret" + extensions.encode_vec(2, &[]); // 0 bytes of "Extended Master Secret" extension data + + let mut signature_algorithms = Encoder::new(); + signature_algorithms.encode_vec( + 2, + &[ + 0x04, 0x03, // assigned value for ECDSA-SECP256r1-SHA256 + 0x05, 0x03, // assigned value for ECDSA-SECP384r1-SHA384 + 0x06, 0x03, // assigned value for ECDSA-SECP521r1-SHA512 + 0x08, 0x07, // assigned value for ED25519 + 0x08, 0x08, // assigned value for ED448 + 0x08, 0x09, // assigned value for RSA-PSS-PSS-SHA256 + 0x08, 0x0a, // assigned value for RSA-PSS-PSS-SHA384 + 0x08, 0x0b, // assigned value for RSA-PSS-PSS-SHA512 + 0x08, 0x04, // assigned value for RSA-PSS-RSAE-SHA256 + 0x08, 0x05, // assigned value for RSA-PSS-RSAE-SHA384 + 0x08, 0x06, // assigned value for RSA-PSS-RSAE-SHA512 + 0x04, 0x01, // assigned value for RSA-PKCS1-SHA256 + 0x05, 0x01, // assigned value for RSA-PKCS1-SHA384 + 0x06, 0x01, // assigned value for RSA-PKCS1-SHA512 + ], + ); + + extensions.encode_uint(2, 0x0d_u8); // assigned value for extension "Signature Algorithms" + extensions.encode_vec(2, signature_algorithms.as_ref()); // "Signature Algorithms" extension data + + let mut supported_versions = Encoder::new(); + supported_versions.encode_vec(1, &[0x03, 0x04]); // assigned value for TLS 1.3 + + extensions.encode_uint(2, 0x2b_u8); // assigned value for extension "Supported Versions" + extensions.encode_vec(2, supported_versions.as_ref()); // "Supported Versions" extension data + + let mut psk_key_exchange_modes = Encoder::new(); + psk_key_exchange_modes.encode_vec(1, &[0x01]); // assigned value for "PSK with (EC)DHE key establishment" + + extensions.encode_uint(2, 0x2d_u8); // assigned value for extension "PSK Key Exchange Modes" + extensions.encode_vec(2, psk_key_exchange_modes.as_ref()); // "PSK Key Exchange Modes" extension data + + let mut key_share = Encoder::new(); + key_share.encode_uint(2, 0x1d_u8); // assigned value for x25519 (key exchange via curve25519) + key_share.encode_vec(2, &random::<32>()); // 32 bytes of public key + + let mut key_share_list = Encoder::new(); + key_share_list.encode_vec(2, key_share.as_ref()); // first (and only) key share entry + + extensions.encode_uint(2, 0x33_u8); // assigned value for extension "Key Share" + extensions.encode_vec(2, key_share_list.as_ref()); // "Key Share" extension data + + // Handshake Header + let mut handshake_data = Encoder::new(); + handshake_data.encode(&[0x03, 0x03]); // Client Version + handshake_data.encode(&random::<32>()); // Client Random + handshake_data.encode_vec(1, &random::<32>()); // 32 bytes of session ID + handshake_data.encode_vec( + 2, + &[ + // Cipher Suites + 0x13, 0x02, // assigned value for TLS_AES_256_GCM_SHA384 + 0x13, 0x03, // assigned value for TLS_CHACHA20_POLY1305_SHA256 + 0x13, 0x01, // assigned value for TLS_AES_128_GCM_SHA256 + 0x00, 0xff, // assigned value for TLS_EMPTY_RENEGOTIATION_INFO_SCSV + ], + ); + handshake_data.encode_vec( + 1, + &[ + // Compression Methods + 0x00, // assigned value for "null" compression + ], + ); + handshake_data.encode_vec(2, extensions.as_ref()); // Extensions + + let mut handshake_message = Encoder::new(); + handshake_message.encode(&[0x01]); // handshake message type 0x01 (client hello) + handshake_message.encode_vec(3, handshake_data.as_ref()); // client hello data + + // Record Header + let mut record_header = Encoder::new(); + record_header.encode(&[ + 0x16, // type is 0x16 (handshake record) + 0x03, 0x01, // protocol version is "3,1" (also known as TLS 1.0) + ]); + record_header.encode_vec(2, handshake_message.as_ref()); // handshake message + + record_header.as_ref().to_vec() +} + +pub fn sock_puppet( + tx: &mut CryptoDxState, + path: &PathRef, + address_validation: &AddressValidationInfo, + version: Version, + loss_recovery: &LossRecovery, +) -> Vec { + let encoder = Encoder::new(); + let (_pt, mut builder) = Connection::build_packet_header( + &path.borrow(), + CryptoSpace::Initial, + encoder, + tx, + address_validation, + version, + false, + ); + let _pn = Connection::add_packet_number( + &mut builder, + tx, + loss_recovery.largest_acknowledged_pn(PacketNumberSpace::Initial), + ); + let sp = generate_ch(); + qdebug!("XXX SOCK PUPPET {:02x?}", sp); + let mut cstreams = CryptoStreams::default(); + cstreams.send(PacketNumberSpace::Initial, &sp).unwrap(); + cstreams.write_frame( + PacketNumberSpace::Initial, + false, + &mut builder, + &mut Vec::new(), + &mut FrameStats::default(), + ); + builder.build(tx).unwrap().into() +} diff --git a/neqo-transport/src/crypto.rs b/neqo-transport/src/crypto.rs index ee0547a549..6af69f3243 100644 --- a/neqo-transport/src/crypto.rs +++ b/neqo-transport/src/crypto.rs @@ -1326,8 +1326,8 @@ impl std::fmt::Display for CryptoStates { #[derive(Debug, Default)] pub struct CryptoStream { - tx: TxBuffer, - rx: RxStreamOrderer, + pub tx: TxBuffer, + pub rx: RxStreamOrderer, } #[derive(Debug)] From 05dba69eed274d6fe0064883a71632ab9f39283f Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Fri, 17 Jan 2025 10:24:42 +0200 Subject: [PATCH 6/8] Minimize diff --- neqo-transport/src/connection/mod.rs | 9 ++++----- neqo-transport/src/crypto.rs | 4 ++-- neqo-transport/src/packet/mod.rs | 6 +++--- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs index 114fe30f5f..d674b9ebe0 100644 --- a/neqo-transport/src/connection/mod.rs +++ b/neqo-transport/src/connection/mod.rs @@ -2399,7 +2399,6 @@ impl Connection { // Frames for different epochs must go in different packets, but then these // packets can go in a single datagram let mut encoder = Encoder::with_capacity(profile.limit()); - let mut reserved = 0; for space in PacketNumberSpace::iter() { // Ensure we have tx crypto state for this epoch, or skip it. let Some((cspace, tx)) = self.crypto.states.select_tx_mut(self.version, *space) else { @@ -2428,7 +2427,7 @@ impl Connection { } // Configure the limits and padding for this packet. - let aead_expansion = tx.expansion(); + let mut reserved = tx.expansion(); if pt == PacketType::Initial && self.role == Role::Client && pn == 0 { qdebug!("Reserving {} space for sock puppet CH", sock_puppet.len()); reserved += sock_puppet.len(); @@ -2436,7 +2435,7 @@ impl Connection { needs_padding |= builder.set_initial_limit( &profile, - aead_expansion + reserved, + reserved, self.paths .primary() .ok_or(Error::InternalError)? @@ -2472,13 +2471,13 @@ impl Connection { pn, &builder.as_ref()[payload_start..], path.borrow().tos(), - builder.len() + aead_expansion, + builder.len() + reserved, ); qlog::packet_sent( &self.qlog, pt, pn, - builder.len() - header_start + aead_expansion, + builder.len() - header_start + reserved, &builder.as_ref()[payload_start..], now, ); diff --git a/neqo-transport/src/crypto.rs b/neqo-transport/src/crypto.rs index 6af69f3243..ee0547a549 100644 --- a/neqo-transport/src/crypto.rs +++ b/neqo-transport/src/crypto.rs @@ -1326,8 +1326,8 @@ impl std::fmt::Display for CryptoStates { #[derive(Debug, Default)] pub struct CryptoStream { - pub tx: TxBuffer, - pub rx: RxStreamOrderer, + tx: TxBuffer, + rx: RxStreamOrderer, } #[derive(Debug)] diff --git a/neqo-transport/src/packet/mod.rs b/neqo-transport/src/packet/mod.rs index 0c11a4a869..0c7cb08a23 100644 --- a/neqo-transport/src/packet/mod.rs +++ b/neqo-transport/src/packet/mod.rs @@ -243,15 +243,15 @@ impl PacketBuilder { pub fn set_initial_limit( &mut self, profile: &SendProfile, - aead_expansion: usize, + reserved: usize, pmtud: &Pmtud, ) -> bool { if pmtud.needs_probe() { debug_assert!(pmtud.probe_size() > profile.limit()); - self.limit = pmtud.probe_size() - aead_expansion; + self.limit = pmtud.probe_size() - reserved; true } else { - self.limit = profile.limit() - aead_expansion; + self.limit = profile.limit() - reserved; false } } From 762ddfab5d7306209937b3b0e4531918bd0017a3 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Fri, 17 Jan 2025 17:11:20 +0200 Subject: [PATCH 7/8] Progress --- neqo-transport/src/connection/mod.rs | 52 ++++---- neqo-transport/src/connection/params.rs | 2 +- neqo-transport/src/connection/sockpuppet.rs | 129 +++++++++----------- test/test.sh | 11 +- 4 files changed, 87 insertions(+), 107 deletions(-) diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs index d674b9ebe0..135ba3b87a 100644 --- a/neqo-transport/src/connection/mod.rs +++ b/neqo-transport/src/connection/mod.rs @@ -30,7 +30,6 @@ use neqo_crypto::{ Server, ZeroRttChecker, }; use smallvec::SmallVec; -use sockpuppet::sock_puppet; use crate::{ addr_valid::{AddressValidation, NewTokenState}, @@ -2375,27 +2374,11 @@ impl Connection { let mut needs_padding = false; let grease_quic_bit = self.can_grease_quic_bit(); let version = self.version(); - + let mut sock_puppet = None; // Determine how we are sending packets (PTO, etc..). let profile = self.loss_recovery.send_profile(&path.borrow(), now); qdebug!("[{self}] output_path send_profile {profile:?}"); - let sock_puppet = if let Some((_, tx)) = self - .crypto - .states - .select_tx_mut(self.version, PacketNumberSpace::Initial) - { - sock_puppet( - tx, - path, - &self.address_validation, - version, - &self.loss_recovery, - ) - } else { - Vec::new() - }; - // Frames for different epochs must go in different packets, but then these // packets can go in a single datagram let mut encoder = Encoder::with_capacity(profile.limit()); @@ -2405,6 +2388,12 @@ impl Connection { continue; }; + sock_puppet = + (cspace == CryptoSpace::Initial && self.role == Role::Client).then(|| { + qdebug!("Creating sock puppet"); + sockpuppet::sock_puppet(tx, path, &self.loss_recovery, &self.tps.borrow().local) + }); + let header_start = encoder.len(); let (pt, mut builder) = Self::build_packet_header( &path.borrow(), @@ -2428,10 +2417,12 @@ impl Connection { // Configure the limits and padding for this packet. let mut reserved = tx.expansion(); - if pt == PacketType::Initial && self.role == Role::Client && pn == 0 { + if pn > 1 { + sock_puppet = None; + } else if let Some(sock_puppet) = &sock_puppet { qdebug!("Reserving {} space for sock puppet CH", sock_puppet.len()); reserved += sock_puppet.len(); - }; + } needs_padding |= builder.set_initial_limit( &profile, @@ -2543,16 +2534,17 @@ impl Connection { ); let padding_len = profile.limit() - packets.len(); initial.track_padding(padding_len); - if sock_puppet.len() <= padding_len { - qdebug!("Prepending sock puppet CH, len={}", sock_puppet.len()); - packets.splice(0..0, sock_puppet); - } else { - qwarn!( - "Sock puppet CH too long ({} > {}), not prepending", - sock_puppet.len(), - padding_len - ); - // TODO: Is there a way to trim the size of the sock puppet CH? + if let Some(sock_puppet) = sock_puppet { + if sock_puppet.len() <= padding_len { + qdebug!("Prepending sock puppet CH, len={}", sock_puppet.len()); + packets.splice(0..0, sock_puppet); + } else { + qwarn!( + "Sock puppet CH too long ({} > {}), not prepending", + sock_puppet.len(), + padding_len + ); + } } // These zeros aren't padding frames, they are an invalid all-zero coalesced // packet, which is why we don't increase `frame_tx.padding` count here. diff --git a/neqo-transport/src/connection/params.rs b/neqo-transport/src/connection/params.rs index dcbe6fd66c..3ac6f63d2c 100644 --- a/neqo-transport/src/connection/params.rs +++ b/neqo-transport/src/connection/params.rs @@ -112,7 +112,7 @@ impl Default for ConnectionParameters { pacing: true, pmtud: false, sock_puppet: true, - sni_slicing: true, + sni_slicing: false, // FIXME: This should be true by default. } } } diff --git a/neqo-transport/src/connection/sockpuppet.rs b/neqo-transport/src/connection/sockpuppet.rs index ef74803d9b..ee20bd4408 100644 --- a/neqo-transport/src/connection/sockpuppet.rs +++ b/neqo-transport/src/connection/sockpuppet.rs @@ -4,7 +4,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use neqo_common::{qdebug, Encoder}; +use neqo_common::Encoder; use neqo_crypto::random; use super::AddressValidationInfo; @@ -13,66 +13,40 @@ use crate::{ path::PathRef, recovery::LossRecovery, stats::FrameStats, + tparams::TransportParameters, tracking::PacketNumberSpace, Connection, Version, }; /// See -#[expect(clippy::too_many_lines)] -fn generate_ch() -> Vec { +fn generate_ch(sni: &[u8], tps: &TransportParameters) -> Vec { // Extensions let mut extensions = Encoder::new(); let mut list_entry = Encoder::new(); - list_entry.encode_uint(1, 0x00_u8); // list entry is type 0x00 "DNS hostname" - list_entry.encode_vec(2, b"github.com"); - - let mut server_name_list = Encoder::new(); - server_name_list.encode_vec(2, list_entry.as_ref()); // first (and only) list entry - + list_entry.encode_uint::(1, 0x00); // list entry is type 0x00 "DNS hostname" + list_entry.encode_vec(2, sni); let mut server_name_extension = Encoder::new(); - server_name_extension.encode_vec(2, server_name_list.as_ref()); // "server name" extension data + server_name_extension.encode_vec(2, list_entry.as_ref()); // "server name" extension data - extensions.encode_uint(2, 0x00_u8); // assigned value for extension "server name" + extensions.encode_uint::(2, 0x00); // assigned value for extension "server name" extensions.encode_vec(2, server_name_extension.as_ref()); // server name extension data - let mut ec_points_formats = Encoder::new(); - ec_points_formats.encode_vec( - 1, - &[ - 0x00, // assigned value for format "uncompressed" - 0x01, // assigned value for format "ansiX962_compressed_prime" - 0x02, // assigned value for format "ansiX962_compressed_char2" - ], - ); - - extensions.encode_uint(2, 0x0b_u8); // assigned value for extension "ec point formats" - extensions.encode_vec(2, ec_points_formats.as_ref()); // ec point formats extension data - let mut supported_groups = Encoder::new(); supported_groups.encode_vec( 2, &[ 0x00, 0x1d, // assigned value for the curve "x25519" 0x00, 0x17, // assigned value for the curve "secp256r1" - 0x00, 0x1e, // assigned value for the curve "x448" - 0x00, 0x19, // assigned value for the curve "secp521r1" 0x00, 0x18, // assigned value for the curve "secp384r1" - 0x01, 0x00, // assigned value for the curve "ffdhe2048" - 0x01, 0x01, // assigned value for the curve "ffdhe3072" - 0x01, 0x02, // assigned value for the curve "ffdhe4096" - 0x01, 0x03, // assigned value for the curve "ffdhe6144" - 0x01, 0x04, // assigned value for the curve "ffdhe8192" + 0x00, 0x19, // assigned value for the curve "secp521r1" ], ); - extensions.encode_uint(2, 0x0a_u8); // assigned value for extension "supported groups" + extensions.encode_uint::(2, 0x0a); // assigned value for extension "supported groups" extensions.encode_vec(2, supported_groups.as_ref()); // supported groups extension data - extensions.encode_uint(2, 0x23_u8); // assigned value for extension "Session Ticket" - extensions.encode_vec(2, &[]); // 0 bytes of "Session Ticket" extension data - - extensions.encode_uint(2, 0x17_u8); // assigned value for extension "Extended Master Secret" + extensions.encode_uint::(2, 0x17); // assigned value for extension "Extended Master Secret" extensions.encode_vec(2, &[]); // 0 bytes of "Extended Master Secret" extension data let mut signature_algorithms = Encoder::new(); @@ -82,45 +56,68 @@ fn generate_ch() -> Vec { 0x04, 0x03, // assigned value for ECDSA-SECP256r1-SHA256 0x05, 0x03, // assigned value for ECDSA-SECP384r1-SHA384 0x06, 0x03, // assigned value for ECDSA-SECP521r1-SHA512 - 0x08, 0x07, // assigned value for ED25519 - 0x08, 0x08, // assigned value for ED448 - 0x08, 0x09, // assigned value for RSA-PSS-PSS-SHA256 - 0x08, 0x0a, // assigned value for RSA-PSS-PSS-SHA384 - 0x08, 0x0b, // assigned value for RSA-PSS-PSS-SHA512 + 0x02, 0x03, // assigned value for ECDSA-SHA1 0x08, 0x04, // assigned value for RSA-PSS-RSAE-SHA256 0x08, 0x05, // assigned value for RSA-PSS-RSAE-SHA384 0x08, 0x06, // assigned value for RSA-PSS-RSAE-SHA512 + 0x08, 0x09, // assigned value for RSA-PSS-PSS-SHA256 0x04, 0x01, // assigned value for RSA-PKCS1-SHA256 0x05, 0x01, // assigned value for RSA-PKCS1-SHA384 0x06, 0x01, // assigned value for RSA-PKCS1-SHA512 + 0x02, 0x01, // assigned value for RSA-PKCS1-SHA1 ], ); - extensions.encode_uint(2, 0x0d_u8); // assigned value for extension "Signature Algorithms" + extensions.encode_uint::(2, 0x0d); // assigned value for extension "Signature Algorithms" extensions.encode_vec(2, signature_algorithms.as_ref()); // "Signature Algorithms" extension data let mut supported_versions = Encoder::new(); supported_versions.encode_vec(1, &[0x03, 0x04]); // assigned value for TLS 1.3 - extensions.encode_uint(2, 0x2b_u8); // assigned value for extension "Supported Versions" + extensions.encode_uint::(2, 0x2b); // assigned value for extension "Supported Versions" extensions.encode_vec(2, supported_versions.as_ref()); // "Supported Versions" extension data let mut psk_key_exchange_modes = Encoder::new(); psk_key_exchange_modes.encode_vec(1, &[0x01]); // assigned value for "PSK with (EC)DHE key establishment" - extensions.encode_uint(2, 0x2d_u8); // assigned value for extension "PSK Key Exchange Modes" + extensions.encode_uint::(2, 0x2d); // assigned value for extension "PSK Key Exchange Modes" extensions.encode_vec(2, psk_key_exchange_modes.as_ref()); // "PSK Key Exchange Modes" extension data let mut key_share = Encoder::new(); - key_share.encode_uint(2, 0x1d_u8); // assigned value for x25519 (key exchange via curve25519) + key_share.encode_uint::(2, 0x1d); // assigned value for x25519 (key exchange via curve25519) key_share.encode_vec(2, &random::<32>()); // 32 bytes of public key + key_share.encode_uint::(2, 0x17); // assigned value for secp256r1 (key exchange via NIST P-256) + key_share.encode_vec(2, &random::<65>()); // 32 bytes of public key let mut key_share_list = Encoder::new(); key_share_list.encode_vec(2, key_share.as_ref()); // first (and only) key share entry - extensions.encode_uint(2, 0x33_u8); // assigned value for extension "Key Share" + extensions.encode_uint::(2, 0x33); // assigned value for extension "Key Share" extensions.encode_vec(2, key_share_list.as_ref()); // "Key Share" extension data + let mut transport_parameters = Encoder::default(); + tps.encode(&mut transport_parameters); + + extensions.encode_uint::(2, 0x39); + extensions.encode_vec(2, transport_parameters.as_ref()); + + let mut alpn = Encoder::new(); + alpn.encode_vec(1, b"h3"); + let mut alpn_list = Encoder::new(); + alpn_list.encode_vec(2, alpn.as_ref()); + + extensions.encode_uint::(2, 0x10); + extensions.encode_vec(2, alpn_list.as_ref()); + + extensions.encode_uint::(2, 0x1c); // record size limit + extensions.encode_vec(2, &[0x40, 0x01]); + + extensions.encode_uint(2, 0xff01_u16); // renegotiation info + extensions.encode_vec(2, &[0x00]); + + extensions.encode_uint::(2, 0x05); // status request + extensions.encode_vec(2, &[0x01, 0x00, 0x00, 0x00, 0x00]); + // Handshake Header let mut handshake_data = Encoder::new(); handshake_data.encode(&[0x03, 0x03]); // Client Version @@ -130,42 +127,27 @@ fn generate_ch() -> Vec { 2, &[ // Cipher Suites - 0x13, 0x02, // assigned value for TLS_AES_256_GCM_SHA384 - 0x13, 0x03, // assigned value for TLS_CHACHA20_POLY1305_SHA256 0x13, 0x01, // assigned value for TLS_AES_128_GCM_SHA256 - 0x00, 0xff, // assigned value for TLS_EMPTY_RENEGOTIATION_INFO_SCSV - ], - ); - handshake_data.encode_vec( - 1, - &[ - // Compression Methods - 0x00, // assigned value for "null" compression + 0x13, 0x03, // assigned value for TLS_CHACHA20_POLY1305_SHA256 + 0x13, 0x02, // assigned value for TLS_AES_256_GCM_SHA384 ], ); + // Compression Methods + handshake_data.encode_vec(1, &[0x00]); // assigned value for "null" compression handshake_data.encode_vec(2, extensions.as_ref()); // Extensions let mut handshake_message = Encoder::new(); handshake_message.encode(&[0x01]); // handshake message type 0x01 (client hello) handshake_message.encode_vec(3, handshake_data.as_ref()); // client hello data - // Record Header - let mut record_header = Encoder::new(); - record_header.encode(&[ - 0x16, // type is 0x16 (handshake record) - 0x03, 0x01, // protocol version is "3,1" (also known as TLS 1.0) - ]); - record_header.encode_vec(2, handshake_message.as_ref()); // handshake message - - record_header.as_ref().to_vec() + handshake_message.as_ref().to_vec() } pub fn sock_puppet( tx: &mut CryptoDxState, path: &PathRef, - address_validation: &AddressValidationInfo, - version: Version, loss_recovery: &LossRecovery, + tps: &TransportParameters, ) -> Vec { let encoder = Encoder::new(); let (_pt, mut builder) = Connection::build_packet_header( @@ -173,19 +155,19 @@ pub fn sock_puppet( CryptoSpace::Initial, encoder, tx, - address_validation, - version, + &AddressValidationInfo::None, + Version::Version1, false, ); - let _pn = Connection::add_packet_number( + _ = Connection::add_packet_number( &mut builder, tx, loss_recovery.largest_acknowledged_pn(PacketNumberSpace::Initial), ); - let sp = generate_ch(); - qdebug!("XXX SOCK PUPPET {:02x?}", sp); let mut cstreams = CryptoStreams::default(); - cstreams.send(PacketNumberSpace::Initial, &sp).unwrap(); + cstreams + .send(PacketNumberSpace::Initial, &generate_ch(b"github.com", tps)) + .unwrap(); cstreams.write_frame( PacketNumberSpace::Initial, false, @@ -193,5 +175,6 @@ pub fn sock_puppet( &mut Vec::new(), &mut FrameStats::default(), ); + builder.build(tx).unwrap().into() } diff --git a/test/test.sh b/test/test.sh index f5fb5e1a7f..14a611f99a 100755 --- a/test/test.sh +++ b/test/test.sh @@ -16,7 +16,7 @@ cargo build --bin neqo-client --bin neqo-server addr=localhost port=4433 path=/20000 -flags="--verbose --verbose --verbose --qlog-dir $tmp --alpn hq-interop --quic-version 1" +flags="--verbose --verbose --verbose --verbose --qlog-dir $tmp --alpn hq-interop --quic-version 1" if [ "$(uname -s)" != "Linux" ]; then iface=lo0 else @@ -35,12 +35,17 @@ tcpdump -U -i "$iface" -w "$tmp/test.pcap" host $addr and port $port >/dev/null tcpdump_pid=$! trap 'kill $tcpdump_pid; rm -rf "$tmp"' EXIT +export SSLDEBUG=1 +export SSLTRACE=1 tmux -CC \ set-option -g default-shell "$(which bash)" \; \ new-session "$client; kill -USR2 $tcpdump_pid; touch $tmp/done" \; \ split-window -h "$server" \; \ split-window -v -f "\ + echo $tmp; \ until [ -e $tmp/done ]; do sleep 1; done; \ - echo $tmp; ls -l $tmp; echo; \ - tshark -r $tmp/test.pcap -o tls.keylog_file:$tmp/test.tlskey" \; \ + editcap --inject-secrets tls,$tmp/test.tlskey $tmp/test.pcap $tmp/test.pcap.injected; \ + mv $tmp/test.pcap.injected $tmp/test.pcap; \ + ls -l $tmp; echo; \ + tshark -r $tmp/test.pcap" \; \ set remain-on-exit on From f829b46f581420007a06c25b058324dc826e807a Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Mon, 20 Jan 2025 17:24:26 +0200 Subject: [PATCH 8/8] More --- neqo-transport/src/connection/mod.rs | 19 ++++++++++++++++--- .../{sockpuppet.rs => sock_puppet.rs} | 18 ++++++++++++------ test/test.sh | 6 ++---- 3 files changed, 30 insertions(+), 13 deletions(-) rename neqo-transport/src/connection/{sockpuppet.rs => sock_puppet.rs} (92%) diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs index 135ba3b87a..1a67f9aa7c 100644 --- a/neqo-transport/src/connection/mod.rs +++ b/neqo-transport/src/connection/mod.rs @@ -65,7 +65,7 @@ mod dump; mod idle; pub mod params; mod saved; -mod sockpuppet; +mod sock_puppet; mod state; #[cfg(test)] pub mod test_internal; @@ -279,6 +279,7 @@ pub struct Connection { release_resumption_token_timer: Option, conn_params: ConnectionParameters, hrtime: hrtime::Handle, + protocols: Vec, /// For testing purposes it is sometimes necessary to inject frames that wouldn't /// otherwise be sent, just to see how a connection handles them. Inserting them @@ -382,10 +383,15 @@ impl Connection { ); let tphandler = Rc::new(RefCell::new(tps)); + let protocols = protocols + .iter() + .map(P::as_ref) + .map(String::from) + .collect::>(); let crypto = Crypto::new( conn_params.get_versions().initial(), agent, - protocols.iter().map(P::as_ref).map(String::from).collect(), + protocols.clone(), Rc::clone(&tphandler), )?; @@ -427,6 +433,7 @@ impl Connection { conn_params, hrtime: hrtime::Time::get(Self::LOOSE_TIMER_RESOLUTION), quic_datagrams, + protocols, #[cfg(test)] test_frame_writer: None, }; @@ -2391,7 +2398,13 @@ impl Connection { sock_puppet = (cspace == CryptoSpace::Initial && self.role == Role::Client).then(|| { qdebug!("Creating sock puppet"); - sockpuppet::sock_puppet(tx, path, &self.loss_recovery, &self.tps.borrow().local) + sock_puppet::sock_puppet( + tx, + path, + &self.loss_recovery, + &self.tps.borrow().local, + &self.protocols, + ) }); let header_start = encoder.len(); diff --git a/neqo-transport/src/connection/sockpuppet.rs b/neqo-transport/src/connection/sock_puppet.rs similarity index 92% rename from neqo-transport/src/connection/sockpuppet.rs rename to neqo-transport/src/connection/sock_puppet.rs index ee20bd4408..1c5d8cbe46 100644 --- a/neqo-transport/src/connection/sockpuppet.rs +++ b/neqo-transport/src/connection/sock_puppet.rs @@ -19,7 +19,7 @@ use crate::{ }; /// See -fn generate_ch(sni: &[u8], tps: &TransportParameters) -> Vec { +fn generate_ch(sni: &[u8], protocols: &[String], tps: &TransportParameters) -> Vec { // Extensions let mut extensions = Encoder::new(); @@ -87,7 +87,7 @@ fn generate_ch(sni: &[u8], tps: &TransportParameters) -> Vec { key_share.encode_uint::(2, 0x1d); // assigned value for x25519 (key exchange via curve25519) key_share.encode_vec(2, &random::<32>()); // 32 bytes of public key key_share.encode_uint::(2, 0x17); // assigned value for secp256r1 (key exchange via NIST P-256) - key_share.encode_vec(2, &random::<65>()); // 32 bytes of public key + key_share.encode_vec(2, &random::<65>()); // 65 bytes of public key let mut key_share_list = Encoder::new(); key_share_list.encode_vec(2, key_share.as_ref()); // first (and only) key share entry @@ -102,7 +102,9 @@ fn generate_ch(sni: &[u8], tps: &TransportParameters) -> Vec { extensions.encode_vec(2, transport_parameters.as_ref()); let mut alpn = Encoder::new(); - alpn.encode_vec(1, b"h3"); + for protocol in protocols { + alpn.encode_vec(1, protocol.as_bytes()); + } let mut alpn_list = Encoder::new(); alpn_list.encode_vec(2, alpn.as_ref()); @@ -112,7 +114,7 @@ fn generate_ch(sni: &[u8], tps: &TransportParameters) -> Vec { extensions.encode_uint::(2, 0x1c); // record size limit extensions.encode_vec(2, &[0x40, 0x01]); - extensions.encode_uint(2, 0xff01_u16); // renegotiation info + extensions.encode_uint::(2, 0xff01); // renegotiation info extensions.encode_vec(2, &[0x00]); extensions.encode_uint::(2, 0x05); // status request @@ -122,7 +124,7 @@ fn generate_ch(sni: &[u8], tps: &TransportParameters) -> Vec { let mut handshake_data = Encoder::new(); handshake_data.encode(&[0x03, 0x03]); // Client Version handshake_data.encode(&random::<32>()); // Client Random - handshake_data.encode_vec(1, &random::<32>()); // 32 bytes of session ID + handshake_data.encode_uint::(1, 0); // 0 bytes of session ID handshake_data.encode_vec( 2, &[ @@ -148,6 +150,7 @@ pub fn sock_puppet( path: &PathRef, loss_recovery: &LossRecovery, tps: &TransportParameters, + protocols: &[String], ) -> Vec { let encoder = Encoder::new(); let (_pt, mut builder) = Connection::build_packet_header( @@ -166,7 +169,10 @@ pub fn sock_puppet( ); let mut cstreams = CryptoStreams::default(); cstreams - .send(PacketNumberSpace::Initial, &generate_ch(b"github.com", tps)) + .send( + PacketNumberSpace::Initial, + &generate_ch(b"github.com", protocols, tps), + ) .unwrap(); cstreams.write_frame( PacketNumberSpace::Initial, diff --git a/test/test.sh b/test/test.sh index 14a611f99a..0fdadf41aa 100755 --- a/test/test.sh +++ b/test/test.sh @@ -16,7 +16,7 @@ cargo build --bin neqo-client --bin neqo-server addr=localhost port=4433 path=/20000 -flags="--verbose --verbose --verbose --verbose --qlog-dir $tmp --alpn hq-interop --quic-version 1" +flags="--verbose --verbose --verbose --qlog-dir $tmp --alpn hq-interop --quic-version 1" if [ "$(uname -s)" != "Linux" ]; then iface=lo0 else @@ -35,11 +35,9 @@ tcpdump -U -i "$iface" -w "$tmp/test.pcap" host $addr and port $port >/dev/null tcpdump_pid=$! trap 'kill $tcpdump_pid; rm -rf "$tmp"' EXIT -export SSLDEBUG=1 -export SSLTRACE=1 tmux -CC \ set-option -g default-shell "$(which bash)" \; \ - new-session "$client; kill -USR2 $tcpdump_pid; touch $tmp/done" \; \ + new-session "sleep 1; $client; kill -USR2 $tcpdump_pid; touch $tmp/done" \; \ split-window -h "$server" \; \ split-window -v -f "\ echo $tmp; \