From d8bbc7858622b6d9c278469aab701ca0b609cddf Mon Sep 17 00:00:00 2001 From: Daniel Baumann Date: Wed, 15 May 2024 05:35:49 +0200 Subject: Merging upstream version 126.0. Signed-off-by: Daniel Baumann --- .../rust/neqo-transport/src/cc/classic_cc.rs | 27 +- .../rust/neqo-transport/src/connection/dump.rs | 16 +- .../rust/neqo-transport/src/connection/mod.rs | 160 ++++------ .../rust/neqo-transport/src/connection/params.rs | 13 - .../rust/neqo-transport/src/connection/state.rs | 5 +- .../neqo-transport/src/connection/tests/fuzzing.rs | 42 --- .../src/connection/tests/handshake.rs | 11 +- .../neqo-transport/src/connection/tests/mod.rs | 85 ++++- .../neqo-transport/src/connection/tests/null.rs | 42 +++ .../neqo-transport/src/connection/tests/stream.rs | 6 - third_party/rust/neqo-transport/src/crypto.rs | 98 +----- third_party/rust/neqo-transport/src/frame.rs | 86 +++-- third_party/rust/neqo-transport/src/lib.rs | 8 +- third_party/rust/neqo-transport/src/packet/mod.rs | 99 +++++- .../rust/neqo-transport/src/packet/retry.rs | 1 - third_party/rust/neqo-transport/src/path.rs | 2 +- third_party/rust/neqo-transport/src/qlog.rs | 347 +++++++++++---------- third_party/rust/neqo-transport/src/stats.rs | 6 +- 18 files changed, 552 insertions(+), 502 deletions(-) delete mode 100644 third_party/rust/neqo-transport/src/connection/tests/fuzzing.rs create mode 100644 third_party/rust/neqo-transport/src/connection/tests/null.rs (limited to 'third_party/rust/neqo-transport/src') diff --git a/third_party/rust/neqo-transport/src/cc/classic_cc.rs b/third_party/rust/neqo-transport/src/cc/classic_cc.rs index 89be6c4b0f..f8bcee6722 100644 --- a/third_party/rust/neqo-transport/src/cc/classic_cc.rs +++ b/third_party/rust/neqo-transport/src/cc/classic_cc.rs @@ -164,7 +164,7 @@ impl CongestionControl for ClassicCongestionControl { let mut is_app_limited = true; let mut new_acked = 0; for pkt in acked_pkts { - qinfo!( + qdebug!( "packet_acked this={:p}, pn={}, ps={}, ignored={}, lost={}, rtt_est={:?}", self, pkt.pn, @@ -179,8 +179,9 @@ impl CongestionControl for ClassicCongestionControl { if pkt.pn < self.first_app_limited { is_app_limited = false; } - assert!(self.bytes_in_flight >= pkt.size); - self.bytes_in_flight -= pkt.size; + // BIF is set to 0 on a path change, but in case that was because of a simple rebinding + // event, we may still get ACKs for packets sent before the rebinding. + self.bytes_in_flight = self.bytes_in_flight.saturating_sub(pkt.size); if !self.after_recovery_start(pkt) { // Do not increase congestion window for packets sent before @@ -198,7 +199,7 @@ impl CongestionControl for ClassicCongestionControl { if is_app_limited { self.cc_algorithm.on_app_limited(); - qinfo!("on_packets_acked this={:p}, limited=1, bytes_in_flight={}, cwnd={}, state={:?}, new_acked={}", self, self.bytes_in_flight, self.congestion_window, self.state, new_acked); + qdebug!("on_packets_acked this={:p}, limited=1, bytes_in_flight={}, cwnd={}, state={:?}, new_acked={}", self, self.bytes_in_flight, self.congestion_window, self.state, new_acked); return; } @@ -208,7 +209,7 @@ impl CongestionControl for ClassicCongestionControl { let increase = min(self.ssthresh - self.congestion_window, self.acked_bytes); self.congestion_window += increase; self.acked_bytes -= increase; - qinfo!([self], "slow start += {}", increase); + qdebug!([self], "slow start += {}", increase); if self.congestion_window == self.ssthresh { // This doesn't look like it is necessary, but it can happen // after persistent congestion. @@ -249,7 +250,7 @@ impl CongestionControl for ClassicCongestionControl { QlogMetric::BytesInFlight(self.bytes_in_flight), ], ); - qinfo!([self], "on_packets_acked this={:p}, limited=0, bytes_in_flight={}, cwnd={}, state={:?}, new_acked={}", self, self.bytes_in_flight, self.congestion_window, self.state, new_acked); + qdebug!([self], "on_packets_acked this={:p}, limited=0, bytes_in_flight={}, cwnd={}, state={:?}, new_acked={}", self, self.bytes_in_flight, self.congestion_window, self.state, new_acked); } /// Update congestion controller state based on lost packets. @@ -265,14 +266,15 @@ impl CongestionControl for ClassicCongestionControl { } for pkt in lost_packets.iter().filter(|pkt| pkt.cc_in_flight()) { - qinfo!( + qdebug!( "packet_lost this={:p}, pn={}, ps={}", self, pkt.pn, pkt.size ); - assert!(self.bytes_in_flight >= pkt.size); - self.bytes_in_flight -= pkt.size; + // BIF is set to 0 on a path change, but in case that was because of a simple rebinding + // event, we may still declare packets lost that were sent before the rebinding. + self.bytes_in_flight = self.bytes_in_flight.saturating_sub(pkt.size); } qlog::metrics_updated( &mut self.qlog, @@ -286,7 +288,7 @@ impl CongestionControl for ClassicCongestionControl { pto, lost_packets, ); - qinfo!( + qdebug!( "on_packets_lost this={:p}, bytes_in_flight={}, cwnd={}, state={:?}", self, self.bytes_in_flight, @@ -335,7 +337,7 @@ impl CongestionControl for ClassicCongestionControl { } self.bytes_in_flight += pkt.size; - qinfo!( + qdebug!( "packet_sent this={:p}, pn={}, ps={}", self, pkt.pn, @@ -498,7 +500,7 @@ impl ClassicCongestionControl { self.congestion_window = max(cwnd, CWND_MIN); self.acked_bytes = acked_bytes; self.ssthresh = self.congestion_window; - qinfo!( + qdebug!( [self], "Cong event -> recovery; cwnd {}, ssthresh {}", self.congestion_window, @@ -516,7 +518,6 @@ impl ClassicCongestionControl { true } - #[allow(clippy::unused_self)] fn app_limited(&self) -> bool { if self.bytes_in_flight >= self.congestion_window { false diff --git a/third_party/rust/neqo-transport/src/connection/dump.rs b/third_party/rust/neqo-transport/src/connection/dump.rs index 8a4f34dbb8..12d337c570 100644 --- a/third_party/rust/neqo-transport/src/connection/dump.rs +++ b/third_party/rust/neqo-transport/src/connection/dump.rs @@ -9,7 +9,7 @@ use std::fmt::Write; -use neqo_common::{qdebug, Decoder}; +use neqo_common::{qdebug, Decoder, IpTos}; use crate::{ connection::Connection, @@ -26,6 +26,7 @@ pub fn dump_packet( pt: PacketType, pn: PacketNumber, payload: &[u8], + tos: IpTos, ) { if log::STATIC_MAX_LEVEL == log::LevelFilter::Off || !log::log_enabled!(log::Level::Debug) { return; @@ -38,9 +39,18 @@ pub fn dump_packet( s.push_str(" [broken]..."); break; }; - if let Some(x) = f.dump() { + let x = f.dump(); + if !x.is_empty() { write!(&mut s, "\n {} {}", dir, &x).unwrap(); } } - qdebug!([conn], "pn={} type={:?} {}{}", pn, pt, path.borrow(), s); + qdebug!( + [conn], + "pn={} type={:?} {} {:?}{}", + pn, + pt, + path.borrow(), + tos, + s + ); } diff --git a/third_party/rust/neqo-transport/src/connection/mod.rs b/third_party/rust/neqo-transport/src/connection/mod.rs index c81a3727c6..8522507a69 100644 --- a/third_party/rust/neqo-transport/src/connection/mod.rs +++ b/third_party/rust/neqo-transport/src/connection/mod.rs @@ -10,7 +10,7 @@ use std::{ cell::RefCell, cmp::{max, min}, fmt::{self, Debug}, - mem, + iter, mem, net::{IpAddr, SocketAddr}, ops::RangeInclusive, rc::{Rc, Weak}, @@ -19,7 +19,7 @@ use std::{ use neqo_common::{ event::Provider as EventProvider, hex, hex_snip_middle, hrtime, qdebug, qerror, qinfo, - qlog::NeqoQlog, qtrace, qwarn, Datagram, Decoder, Encoder, Role, + qlog::NeqoQlog, qtrace, qwarn, Datagram, Decoder, Encoder, IpTos, Role, }; use neqo_crypto::{ agent::CertificateInfo, Agent, AntiReplay, AuthenticationStatus, Cipher, Client, Group, @@ -383,7 +383,6 @@ impl Connection { agent, protocols.iter().map(P::as_ref).map(String::from).collect(), Rc::clone(&tphandler), - conn_params.is_fuzzing(), )?; let stats = StatsCell::default(); @@ -461,7 +460,7 @@ impl Connection { } /// # Errors - /// When the operation fails. + /// When the operation fails. pub fn client_enable_ech(&mut self, ech_config_list: impl AsRef<[u8]>) -> Res<()> { self.crypto.client_enable_ech(ech_config_list) } @@ -778,7 +777,7 @@ impl Connection { }); enc.encode(extra); let records = s.send_ticket(now, enc.as_ref())?; - qinfo!([self], "send session ticket {}", hex(&enc)); + qdebug!([self], "send session ticket {}", hex(&enc)); self.crypto.buffer_records(records)?; } else { unreachable!(); @@ -824,7 +823,7 @@ impl Connection { /// the connection to fail. However, if no packets have been /// exchanged, it's not OK. pub fn authenticated(&mut self, status: AuthenticationStatus, now: Instant) { - qinfo!([self], "Authenticated {:?}", status); + qdebug!([self], "Authenticated {:?}", status); self.crypto.tls.authenticated(status); let res = self.handshake(now, self.version, PacketNumberSpace::Handshake, None); self.absorb_error(now, res); @@ -979,19 +978,16 @@ impl Connection { /// Process new input datagrams on the connection. pub fn process_input(&mut self, d: &Datagram, now: Instant) { - self.input(d, now, now); - self.process_saved(now); - self.streams.cleanup_closed_streams(); + self.process_multiple_input(iter::once(d), now); } /// Process new input datagrams on the connection. pub fn process_multiple_input<'a, I>(&mut self, dgrams: I, now: Instant) where I: IntoIterator, - I::IntoIter: ExactSizeIterator, { - let dgrams = dgrams.into_iter(); - if dgrams.len() == 0 { + let mut dgrams = dgrams.into_iter().peekable(); + if dgrams.peek().is_none() { return; } @@ -1154,7 +1150,7 @@ impl Connection { fn discard_keys(&mut self, space: PacketNumberSpace, now: Instant) { if self.crypto.discard(space) { - qinfo!([self], "Drop packet number space {}", space); + qdebug!([self], "Drop packet number space {}", space); let primary = self.paths.primary(); self.loss_recovery.discard(&primary, space, now); self.acks.drop_space(space); @@ -1492,6 +1488,7 @@ impl Connection { payload.packet_type(), payload.pn(), &payload[..], + d.tos(), ); qlog::packet_received(&mut self.qlog, &packet, &payload); @@ -1552,6 +1549,10 @@ impl Connection { packet: &DecryptedPacket, now: Instant, ) -> Res { + (!packet.is_empty()) + .then_some(()) + .ok_or(Error::ProtocolViolation)?; + // TODO(ekr@rtfm.com): Have the server blow away the initial // crypto state if this fails? Otherwise, we will get a panic // on the assert for doesn't exist. @@ -1560,24 +1561,8 @@ impl Connection { let mut ack_eliciting = false; let mut probing = true; let mut d = Decoder::from(&packet[..]); - let mut consecutive_padding = 0; while d.remaining() > 0 { - let mut f = Frame::decode(&mut d)?; - - // Skip padding - while f == Frame::Padding && d.remaining() > 0 { - consecutive_padding += 1; - f = Frame::decode(&mut d)?; - } - if consecutive_padding > 0 { - qdebug!( - [self], - "PADDING frame repeated {} times", - consecutive_padding - ); - consecutive_padding = 0; - } - + let f = Frame::decode(&mut d)?; ack_eliciting |= f.ack_eliciting(); probing &= f.path_probing(); let t = f.get_type(); @@ -1841,7 +1826,7 @@ impl Connection { | State::Connected | State::Confirmed => { if let Some(path) = self.paths.select_path() { - let res = self.output_path(&path, now); + let res = self.output_path(&path, now, &None); self.capture_error(Some(path), now, 0, res) } else { Ok(SendOption::default()) @@ -1850,7 +1835,16 @@ impl Connection { State::Closing { .. } | State::Draining { .. } | State::Closed(_) => { if let Some(details) = self.state_signaling.close_frame() { let path = Rc::clone(details.path()); - let res = self.output_close(&details); + // In some error cases, we will not be able to make a new, permanent path. + // For example, if we run out of connection IDs and the error results from + // a packet on a new path, we avoid sending (and the privacy risk) rather + // than reuse a connection ID. + let res = if path.borrow().is_temporary() { + assert!(!cfg!(test), "attempting to close with a temporary path"); + Err(Error::InternalError) + } else { + self.output_path(&path, now, &Some(details)) + }; self.capture_error(Some(path), now, 0, res) } else { Ok(SendOption::default()) @@ -1927,62 +1921,6 @@ impl Connection { } } - fn output_close(&mut self, close: &ClosingFrame) -> Res { - let mut encoder = Encoder::with_capacity(256); - let grease_quic_bit = self.can_grease_quic_bit(); - let version = self.version(); - for space in PacketNumberSpace::iter() { - let Some((cspace, tx)) = self.crypto.states.select_tx_mut(self.version, *space) else { - continue; - }; - - let path = close.path().borrow(); - // In some error cases, we will not be able to make a new, permanent path. - // For example, if we run out of connection IDs and the error results from - // a packet on a new path, we avoid sending (and the privacy risk) rather - // than reuse a connection ID. - if path.is_temporary() { - assert!(!cfg!(test), "attempting to close with a temporary path"); - return Err(Error::InternalError); - } - let (_, mut builder) = Self::build_packet_header( - &path, - cspace, - encoder, - tx, - &AddressValidationInfo::None, - version, - grease_quic_bit, - ); - _ = Self::add_packet_number( - &mut builder, - tx, - self.loss_recovery.largest_acknowledged_pn(*space), - ); - // The builder will set the limit to 0 if there isn't enough space for the header. - if builder.is_full() { - encoder = builder.abort(); - break; - } - builder.set_limit(min(path.amplification_limit(), path.mtu()) - tx.expansion()); - debug_assert!(builder.limit() <= 2048); - - // ConnectionError::Application is only allowed at 1RTT. - let sanitized = if *space == PacketNumberSpace::ApplicationData { - None - } else { - close.sanitize() - }; - sanitized - .as_ref() - .unwrap_or(close) - .write_frame(&mut builder); - encoder = builder.build(tx)?; - } - - Ok(SendOption::Yes(close.path().borrow().datagram(encoder))) - } - /// Write the frames that are exchanged in the application data space. /// The order of calls here determines the relative priority of frames. fn write_appdata_frames( @@ -2203,7 +2141,12 @@ impl Connection { /// Build a datagram, possibly from multiple packets (for different PN /// spaces) and each containing 1+ frames. #[allow(clippy::too_many_lines)] // Yeah, that's just the way it is. - fn output_path(&mut self, path: &PathRef, now: Instant) -> Res { + fn output_path( + &mut self, + path: &PathRef, + now: Instant, + closing_frame: &Option, + ) -> Res { let mut initial_sent = None; let mut needs_padding = false; let grease_quic_bit = self.can_grease_quic_bit(); @@ -2256,8 +2199,23 @@ impl Connection { // Add frames to the packet. let payload_start = builder.len(); - let (tokens, ack_eliciting, padded) = - self.write_frames(path, *space, &profile, &mut builder, now); + let (mut tokens, mut ack_eliciting, mut padded) = (Vec::new(), false, false); + if let Some(ref close) = closing_frame { + // ConnectionError::Application is only allowed at 1RTT. + let sanitized = if *space == PacketNumberSpace::ApplicationData { + None + } else { + close.sanitize() + }; + sanitized + .as_ref() + .unwrap_or(close) + .write_frame(&mut builder); + self.stats.borrow_mut().frame_tx.connection_close += 1; + } else { + (tokens, ack_eliciting, padded) = + self.write_frames(path, *space, &profile, &mut builder, now); + } if builder.packet_empty() { // Nothing to include in this packet. encoder = builder.abort(); @@ -2271,6 +2229,7 @@ impl Connection { pt, pn, &builder.as_ref()[payload_start..], + IpTos::default(), // TODO: set from path ); qlog::packet_sent( &mut self.qlog, @@ -2323,7 +2282,7 @@ impl Connection { } if encoder.is_empty() { - qinfo!("TX blocked, profile={:?} ", profile); + qdebug!("TX blocked, profile={:?} ", profile); Ok(SendOption::No(profile.paced())) } else { // Perform additional padding for Initial packets as necessary. @@ -2337,6 +2296,8 @@ impl Connection { mtu ); initial.size += mtu - packets.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(mtu, 0); } self.loss_recovery.on_packet_sent(path, initial); @@ -2367,7 +2328,7 @@ impl Connection { } fn client_start(&mut self, now: Instant) -> Res<()> { - qinfo!([self], "client_start"); + qdebug!([self], "client_start"); debug_assert_eq!(self.role, Role::Client); qlog::client_connection_started(&mut self.qlog, &self.paths.primary()); qlog::client_version_information_initiated(&mut self.qlog, self.conn_params.get_versions()); @@ -2599,7 +2560,7 @@ impl Connection { fn confirm_version(&mut self, v: Version) { if self.version != v { - qinfo!([self], "Compatible upgrade {:?} ==> {:?}", self.version, v); + qdebug!([self], "Compatible upgrade {:?} ==> {:?}", self.version, v); } self.crypto.confirm_version(v); self.version = v; @@ -2694,9 +2655,8 @@ impl Connection { .input_frame(&frame, &mut self.stats.borrow_mut().frame_rx); } match frame { - Frame::Padding => { - // Note: This counts contiguous padding as a single frame. - self.stats.borrow_mut().frame_rx.padding += 1; + Frame::Padding(length) => { + self.stats.borrow_mut().frame_rx.padding += usize::from(length); } Frame::Ping => { // If we get a PING and there are outstanding CRYPTO frames, @@ -2899,7 +2859,7 @@ impl Connection { R: IntoIterator> + Debug, R::IntoIter: ExactSizeIterator, { - qinfo!([self], "Rx ACK space={}, ranges={:?}", space, ack_ranges); + qdebug!([self], "Rx ACK space={}, ranges={:?}", space, ack_ranges); let (acked_packets, lost_packets) = self.loss_recovery.on_ack_received( &self.paths.primary(), @@ -2953,7 +2913,7 @@ impl Connection { } fn set_connected(&mut self, now: Instant) -> Res<()> { - qinfo!([self], "TLS connection complete"); + qdebug!([self], "TLS connection complete"); if self.crypto.tls.info().map(SecretAgentInfo::alpn).is_none() { qwarn!([self], "No ALPN. Closing connection."); // 120 = no_application_protocol @@ -2996,7 +2956,7 @@ impl Connection { fn set_state(&mut self, state: State) { if state > self.state { - qinfo!([self], "State change from {:?} -> {:?}", self.state, state); + qdebug!([self], "State change from {:?} -> {:?}", self.state, state); self.state = state.clone(); if self.state.closed() { self.streams.clear_streams(); diff --git a/third_party/rust/neqo-transport/src/connection/params.rs b/third_party/rust/neqo-transport/src/connection/params.rs index 72d1efa3ee..d8aa617024 100644 --- a/third_party/rust/neqo-transport/src/connection/params.rs +++ b/third_party/rust/neqo-transport/src/connection/params.rs @@ -77,7 +77,6 @@ pub struct ConnectionParameters { outgoing_datagram_queue: usize, incoming_datagram_queue: usize, fast_pto: u8, - fuzzing: bool, grease: bool, pacing: bool, } @@ -100,7 +99,6 @@ impl Default for ConnectionParameters { outgoing_datagram_queue: MAX_QUEUED_DATAGRAMS_DEFAULT, incoming_datagram_queue: MAX_QUEUED_DATAGRAMS_DEFAULT, fast_pto: FAST_PTO_SCALE, - fuzzing: false, grease: true, pacing: true, } @@ -324,17 +322,6 @@ impl ConnectionParameters { self } - #[must_use] - pub fn is_fuzzing(&self) -> bool { - self.fuzzing - } - - #[must_use] - pub fn fuzzing(mut self, enable: bool) -> Self { - self.fuzzing = enable; - self - } - #[must_use] pub fn is_greasing(&self) -> bool { self.grease diff --git a/third_party/rust/neqo-transport/src/connection/state.rs b/third_party/rust/neqo-transport/src/connection/state.rs index 9789151d3f..cc2f6e30d2 100644 --- a/third_party/rust/neqo-transport/src/connection/state.rs +++ b/third_party/rust/neqo-transport/src/connection/state.rs @@ -209,7 +209,10 @@ pub enum StateSignaling { impl StateSignaling { pub fn handshake_done(&mut self) { if !matches!(self, Self::Idle) { - debug_assert!(false, "StateSignaling must be in Idle state."); + debug_assert!( + false, + "StateSignaling must be in Idle state but is in {self:?} state.", + ); return; } *self = Self::HandshakeDone; diff --git a/third_party/rust/neqo-transport/src/connection/tests/fuzzing.rs b/third_party/rust/neqo-transport/src/connection/tests/fuzzing.rs deleted file mode 100644 index 9924c06fa4..0000000000 --- a/third_party/rust/neqo-transport/src/connection/tests/fuzzing.rs +++ /dev/null @@ -1,42 +0,0 @@ -// 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. - -#![cfg(feature = "fuzzing")] - -use neqo_crypto::FIXED_TAG_FUZZING; -use test_fixture::now; - -use super::{connect_force_idle, default_client, default_server}; -use crate::StreamType; - -#[test] -fn no_encryption() { - const DATA_CLIENT: &[u8] = &[2; 40]; - const DATA_SERVER: &[u8] = &[3; 50]; - let mut client = default_client(); - let mut server = default_server(); - connect_force_idle(&mut client, &mut server); - - let stream_id = client.stream_create(StreamType::BiDi).unwrap(); - - client.stream_send(stream_id, DATA_CLIENT).unwrap(); - let client_pkt = client.process_output(now()).dgram().unwrap(); - assert!(client_pkt[..client_pkt.len() - FIXED_TAG_FUZZING.len()].ends_with(DATA_CLIENT)); - - server.process_input(&client_pkt, now()); - let mut buf = vec![0; 100]; - let (len, _) = server.stream_recv(stream_id, &mut buf).unwrap(); - assert_eq!(len, DATA_CLIENT.len()); - assert_eq!(&buf[..len], DATA_CLIENT); - server.stream_send(stream_id, DATA_SERVER).unwrap(); - let server_pkt = server.process_output(now()).dgram().unwrap(); - assert!(server_pkt[..server_pkt.len() - FIXED_TAG_FUZZING.len()].ends_with(DATA_SERVER)); - - client.process_input(&server_pkt, now()); - let (len, _) = client.stream_recv(stream_id, &mut buf).unwrap(); - assert_eq!(len, DATA_SERVER.len()); - assert_eq!(&buf[..len], DATA_SERVER); -} diff --git a/third_party/rust/neqo-transport/src/connection/tests/handshake.rs b/third_party/rust/neqo-transport/src/connection/tests/handshake.rs index af0352ce90..f2103523ec 100644 --- a/third_party/rust/neqo-transport/src/connection/tests/handshake.rs +++ b/third_party/rust/neqo-transport/src/connection/tests/handshake.rs @@ -16,9 +16,10 @@ use neqo_common::{event::Provider, qdebug, Datagram}; use neqo_crypto::{ constants::TLS_CHACHA20_POLY1305_SHA256, generate_ech_keys, AuthenticationStatus, }; +#[cfg(not(feature = "disable-encryption"))] +use test_fixture::datagram; use test_fixture::{ - assertions, assertions::assert_coalesced_0rtt, datagram, fixture_init, now, split_datagram, - DEFAULT_ADDR, + assertions, assertions::assert_coalesced_0rtt, fixture_init, now, split_datagram, DEFAULT_ADDR, }; use super::{ @@ -458,7 +459,7 @@ fn coalesce_05rtt() { assert_eq!(client.stats().dropped_rx, 0); // No Initial padding. assert_eq!(client.stats().packets_rx, 4); assert_eq!(client.stats().saved_datagrams, 1); - assert_eq!(client.stats().frame_rx.padding, 1); // Padding uses frames. + assert!(client.stats().frame_rx.padding > 0); // Padding uses frames. // Allow the handshake to complete. now += RTT / 2; @@ -605,7 +606,7 @@ fn reorder_1rtt() { } } -#[cfg(not(feature = "fuzzing"))] +#[cfg(not(feature = "disable-encryption"))] #[test] fn corrupted_initial() { let mut client = default_client(); @@ -808,7 +809,7 @@ fn anti_amplification() { assert_eq!(*server.state(), State::Confirmed); } -#[cfg(not(feature = "fuzzing"))] +#[cfg(not(feature = "disable-encryption"))] #[test] fn garbage_initial() { let mut client = default_client(); diff --git a/third_party/rust/neqo-transport/src/connection/tests/mod.rs b/third_party/rust/neqo-transport/src/connection/tests/mod.rs index b6ce08f8d1..c8c87a0df0 100644 --- a/third_party/rust/neqo-transport/src/connection/tests/mod.rs +++ b/third_party/rust/neqo-transport/src/connection/tests/mod.rs @@ -37,11 +37,11 @@ mod ackrate; mod cc; mod close; mod datagram; -mod fuzzing; mod handshake; mod idle; mod keys; mod migration; +mod null; mod priority; mod recovery; mod resumption; @@ -170,12 +170,17 @@ impl crate::connection::test_internal::FrameWriter for PingWriter { } } +trait DatagramModifier: FnMut(Datagram) -> Option {} + +impl DatagramModifier for T where T: FnMut(Datagram) -> Option {} + /// Drive the handshake between the client and server. -fn handshake( +fn handshake_with_modifier( client: &mut Connection, server: &mut Connection, now: Instant, rtt: Duration, + mut modifier: impl DatagramModifier, ) -> Instant { let mut a = client; let mut b = server; @@ -212,7 +217,11 @@ fn handshake( did_ping[a.role()] = true; } assert!(had_input || output.is_some()); - input = output; + if let Some(d) = output { + input = modifier(d); + } else { + input = output; + } qtrace!("handshake: t += {:?}", rtt / 2); now += rtt / 2; mem::swap(&mut a, &mut b); @@ -223,6 +232,15 @@ fn handshake( now } +fn handshake( + client: &mut Connection, + server: &mut Connection, + now: Instant, + rtt: Duration, +) -> Instant { + handshake_with_modifier(client, server, now, rtt, Some) +} + fn connect_fail( client: &mut Connection, server: &mut Connection, @@ -234,11 +252,12 @@ fn connect_fail( assert_error(server, &ConnectionError::Transport(server_error)); } -fn connect_with_rtt( +fn connect_with_rtt_and_modifier( client: &mut Connection, server: &mut Connection, now: Instant, rtt: Duration, + modifier: impl DatagramModifier, ) -> Instant { fn check_rtt(stats: &Stats, rtt: Duration) { assert_eq!(stats.rtt, rtt); @@ -246,7 +265,7 @@ fn connect_with_rtt( let n = stats.frame_rx.ack + usize::from(stats.rtt_init_guess); assert_eq!(stats.rttvar, rttvar_after_n_updates(n, rtt)); } - let now = handshake(client, server, now, rtt); + let now = handshake_with_modifier(client, server, now, rtt, modifier); assert_eq!(*client.state(), State::Confirmed); assert_eq!(*server.state(), State::Confirmed); @@ -255,6 +274,15 @@ fn connect_with_rtt( now } +fn connect_with_rtt( + client: &mut Connection, + server: &mut Connection, + now: Instant, + rtt: Duration, +) -> Instant { + connect_with_rtt_and_modifier(client, server, now, rtt, Some) +} + fn connect(client: &mut Connection, server: &mut Connection) { connect_with_rtt(client, server, now(), Duration::new(0, 0)); } @@ -301,8 +329,13 @@ fn assert_idle(client: &mut Connection, server: &mut Connection, rtt: Duration, } /// Connect with an RTT and then force both peers to be idle. -fn connect_rtt_idle(client: &mut Connection, server: &mut Connection, rtt: Duration) -> Instant { - let now = connect_with_rtt(client, server, now(), rtt); +fn connect_rtt_idle_with_modifier( + client: &mut Connection, + server: &mut Connection, + rtt: Duration, + modifier: impl DatagramModifier, +) -> Instant { + let now = connect_with_rtt_and_modifier(client, server, now(), rtt, modifier); assert_idle(client, server, rtt, now); // Drain events from both as well. _ = client.events().count(); @@ -311,8 +344,20 @@ fn connect_rtt_idle(client: &mut Connection, server: &mut Connection, rtt: Durat now } +fn connect_rtt_idle(client: &mut Connection, server: &mut Connection, rtt: Duration) -> Instant { + connect_rtt_idle_with_modifier(client, server, rtt, Some) +} + +fn connect_force_idle_with_modifier( + client: &mut Connection, + server: &mut Connection, + modifier: impl DatagramModifier, +) { + connect_rtt_idle_with_modifier(client, server, Duration::new(0, 0), modifier); +} + fn connect_force_idle(client: &mut Connection, server: &mut Connection) { - connect_rtt_idle(client, server, Duration::new(0, 0)); + connect_force_idle_with_modifier(client, server, Some); } fn fill_stream(c: &mut Connection, stream: StreamId) { @@ -524,12 +569,14 @@ fn assert_full_cwnd(packets: &[Datagram], cwnd: usize) { } /// Send something on a stream from `sender` to `receiver`, maybe allowing for pacing. +/// Takes a modifier function that can be used to modify the datagram before it is sent. /// Return the resulting datagram and the new time. #[must_use] -fn send_something_paced( +fn send_something_paced_with_modifier( sender: &mut Connection, mut now: Instant, allow_pacing: bool, + mut modifier: impl DatagramModifier, ) -> (Datagram, Instant) { let stream_id = sender.stream_create(StreamType::UniDi).unwrap(); assert!(sender.stream_send(stream_id, DEFAULT_STREAM_DATA).is_ok()); @@ -544,16 +591,32 @@ fn send_something_paced( .dgram() .expect("send_something: should have something to send") } - Output::Datagram(d) => d, + Output::Datagram(d) => modifier(d).unwrap(), Output::None => panic!("send_something: got Output::None"), }; (dgram, now) } +fn send_something_paced( + sender: &mut Connection, + now: Instant, + allow_pacing: bool, +) -> (Datagram, Instant) { + send_something_paced_with_modifier(sender, now, allow_pacing, Some) +} + +fn send_something_with_modifier( + sender: &mut Connection, + now: Instant, + modifier: impl DatagramModifier, +) -> Datagram { + send_something_paced_with_modifier(sender, now, false, modifier).0 +} + /// Send something on a stream from `sender` to `receiver`. /// Return the resulting datagram. fn send_something(sender: &mut Connection, now: Instant) -> Datagram { - send_something_paced(sender, now, false).0 + send_something_with_modifier(sender, now, Some) } /// Send something on a stream from `sender` to `receiver`. diff --git a/third_party/rust/neqo-transport/src/connection/tests/null.rs b/third_party/rust/neqo-transport/src/connection/tests/null.rs new file mode 100644 index 0000000000..e4d60445c6 --- /dev/null +++ b/third_party/rust/neqo-transport/src/connection/tests/null.rs @@ -0,0 +1,42 @@ +// 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. + +#![cfg(feature = "disable-encryption")] + +use neqo_crypto::aead_null::AEAD_NULL_TAG; +use test_fixture::now; + +use super::{connect_force_idle, default_client, default_server}; +use crate::StreamType; + +#[test] +fn no_encryption() { + const DATA_CLIENT: &[u8] = &[2; 40]; + const DATA_SERVER: &[u8] = &[3; 50]; + let mut client = default_client(); + let mut server = default_server(); + connect_force_idle(&mut client, &mut server); + + let stream_id = client.stream_create(StreamType::BiDi).unwrap(); + + client.stream_send(stream_id, DATA_CLIENT).unwrap(); + let client_pkt = client.process_output(now()).dgram().unwrap(); + assert!(client_pkt[..client_pkt.len() - AEAD_NULL_TAG.len()].ends_with(DATA_CLIENT)); + + server.process_input(&client_pkt, now()); + let mut buf = vec![0; 100]; + let (len, _) = server.stream_recv(stream_id, &mut buf).unwrap(); + assert_eq!(len, DATA_CLIENT.len()); + assert_eq!(&buf[..len], DATA_CLIENT); + server.stream_send(stream_id, DATA_SERVER).unwrap(); + let server_pkt = server.process_output(now()).dgram().unwrap(); + assert!(server_pkt[..server_pkt.len() - AEAD_NULL_TAG.len()].ends_with(DATA_SERVER)); + + client.process_input(&server_pkt, now()); + let (len, _) = client.stream_recv(stream_id, &mut buf).unwrap(); + assert_eq!(len, DATA_SERVER.len()); + assert_eq!(&buf[..len], DATA_SERVER); +} diff --git a/third_party/rust/neqo-transport/src/connection/tests/stream.rs b/third_party/rust/neqo-transport/src/connection/tests/stream.rs index f469866d50..66d3bf32f3 100644 --- a/third_party/rust/neqo-transport/src/connection/tests/stream.rs +++ b/third_party/rust/neqo-transport/src/connection/tests/stream.rs @@ -116,12 +116,6 @@ fn transfer() { assert!(fin3); } -#[derive(PartialEq, Eq, PartialOrd, Ord)] -struct IdEntry { - sendorder: StreamOrder, - stream_id: StreamId, -} - // tests stream sendorder priorization fn sendorder_test(order_of_sendorder: &[Option]) { let mut client = default_client(); diff --git a/third_party/rust/neqo-transport/src/crypto.rs b/third_party/rust/neqo-transport/src/crypto.rs index 9840eaa1e1..60d056f2d2 100644 --- a/third_party/rust/neqo-transport/src/crypto.rs +++ b/third_party/rust/neqo-transport/src/crypto.rs @@ -69,7 +69,6 @@ impl Crypto { mut agent: Agent, protocols: Vec, tphandler: TpHandler, - fuzzing: bool, ) -> Res { agent.set_version_range(TLS_VERSION_1_3, TLS_VERSION_1_3)?; agent.set_ciphers(&[ @@ -102,7 +101,6 @@ impl Crypto { tls: agent, streams: CryptoStreams::default(), states: CryptoStates { - fuzzing, ..CryptoStates::default() }, }) @@ -317,7 +315,7 @@ impl Crypto { } pub fn acked(&mut self, token: &CryptoRecoveryToken) { - qinfo!( + qdebug!( "Acked crypto frame space={} offset={} length={}", token.space, token.offset, @@ -367,7 +365,7 @@ impl Crypto { }); enc.encode_vvec(new_token.unwrap_or(&[])); enc.encode(t.as_ref()); - qinfo!("resumption token {}", hex_snip_middle(enc.as_ref())); + qdebug!("resumption token {}", hex_snip_middle(enc.as_ref())); Some(ResumptionToken::new(enc.into(), t.expiration_time())) } else { None @@ -420,7 +418,6 @@ pub struct CryptoDxState { /// The total number of operations that are remaining before the keys /// become exhausted and can't be used any more. invocations: PacketNumber, - fuzzing: bool, } impl CryptoDxState { @@ -431,9 +428,8 @@ impl CryptoDxState { epoch: Epoch, secret: &SymKey, cipher: Cipher, - fuzzing: bool, ) -> Self { - qinfo!( + qdebug!( "Making {:?} {} CryptoDxState, v={:?} cipher={}", direction, epoch, @@ -445,19 +441,11 @@ impl CryptoDxState { version, direction, epoch: usize::from(epoch), - aead: Aead::new( - fuzzing, - TLS_VERSION_1_3, - cipher, - secret, - version.label_prefix(), - ) - .unwrap(), + aead: Aead::new(TLS_VERSION_1_3, cipher, secret, version.label_prefix()).unwrap(), hpkey: HpKey::extract(TLS_VERSION_1_3, cipher, secret, &hplabel).unwrap(), used_pn: 0..0, min_pn: 0, invocations: Self::limit(direction, cipher), - fuzzing, } } @@ -466,7 +454,6 @@ impl CryptoDxState { direction: CryptoDxDirection, label: &str, dcid: &[u8], - fuzzing: bool, ) -> Self { qtrace!("new_initial {:?} {}", version, ConnectionIdRef::from(dcid)); let salt = version.initial_salt(); @@ -482,14 +469,7 @@ impl CryptoDxState { let secret = hkdf::expand_label(TLS_VERSION_1_3, cipher, &initial_secret, &[], label).unwrap(); - Self::new( - version, - direction, - TLS_EPOCH_INITIAL, - &secret, - cipher, - fuzzing, - ) + Self::new(version, direction, TLS_EPOCH_INITIAL, &secret, cipher) } /// Determine the confidentiality and integrity limits for the cipher. @@ -549,7 +529,6 @@ impl CryptoDxState { direction: self.direction, epoch: self.epoch + 1, aead: Aead::new( - self.fuzzing, TLS_VERSION_1_3, cipher, next_secret, @@ -560,7 +539,6 @@ impl CryptoDxState { used_pn: pn..pn, min_pn: pn, invocations, - fuzzing: self.fuzzing, } } @@ -696,7 +674,7 @@ impl CryptoDxState { Ok(res.to_vec()) } - #[cfg(all(test, not(feature = "fuzzing")))] + #[cfg(all(test, not(feature = "disable-encryption")))] pub(crate) fn test_default() -> Self { // This matches the value in packet.rs const CLIENT_CID: &[u8] = &[0x83, 0x94, 0xc8, 0xf0, 0x3e, 0x51, 0x57, 0x08]; @@ -705,7 +683,6 @@ impl CryptoDxState { CryptoDxDirection::Write, "server in", CLIENT_CID, - false, ) } @@ -759,7 +736,6 @@ pub(crate) struct CryptoDxAppData { cipher: Cipher, // Not the secret used to create `self.dx`, but the one needed for the next iteration. next_secret: SymKey, - fuzzing: bool, } impl CryptoDxAppData { @@ -768,20 +744,11 @@ impl CryptoDxAppData { dir: CryptoDxDirection, secret: &SymKey, cipher: Cipher, - fuzzing: bool, ) -> Res { Ok(Self { - dx: CryptoDxState::new( - version, - dir, - TLS_EPOCH_APPLICATION_DATA, - secret, - cipher, - fuzzing, - ), + dx: CryptoDxState::new(version, dir, TLS_EPOCH_APPLICATION_DATA, secret, cipher), cipher, next_secret: Self::update_secret(cipher, secret)?, - fuzzing, }) } @@ -791,7 +758,7 @@ impl CryptoDxAppData { } pub fn next(&self) -> Res { - if self.dx.epoch == usize::max_value() { + if self.dx.epoch == usize::MAX { // Guard against too many key updates. return Err(Error::KeysExhausted); } @@ -800,7 +767,6 @@ impl CryptoDxAppData { dx: self.dx.next(&self.next_secret, self.cipher), cipher: self.cipher, next_secret, - fuzzing: self.fuzzing, }) } @@ -834,7 +800,6 @@ pub struct CryptoStates { // If this is set, then we have noticed a genuine update. // Once this time passes, we should switch in new keys. read_update_time: Option, - fuzzing: bool, } impl CryptoStates { @@ -980,7 +945,7 @@ impl CryptoStates { }; for v in versions { - qinfo!( + qdebug!( [self], "Creating initial cipher state v={:?}, role={:?} dcid={}", v, @@ -989,20 +954,8 @@ impl CryptoStates { ); let mut initial = CryptoState { - tx: CryptoDxState::new_initial( - *v, - CryptoDxDirection::Write, - write, - dcid, - self.fuzzing, - ), - rx: CryptoDxState::new_initial( - *v, - CryptoDxDirection::Read, - read, - dcid, - self.fuzzing, - ), + tx: CryptoDxState::new_initial(*v, CryptoDxDirection::Write, write, dcid), + rx: CryptoDxState::new_initial(*v, CryptoDxDirection::Read, read, dcid), }; if let Some(prev) = self.initials.get(v) { qinfo!( @@ -1056,7 +1009,6 @@ impl CryptoStates { TLS_EPOCH_ZERO_RTT, secret, cipher, - self.fuzzing, )); } @@ -1097,7 +1049,6 @@ impl CryptoStates { TLS_EPOCH_HANDSHAKE, write_secret, cipher, - self.fuzzing, ), rx: CryptoDxState::new( version, @@ -1105,7 +1056,6 @@ impl CryptoStates { TLS_EPOCH_HANDSHAKE, read_secret, cipher, - self.fuzzing, ), }); } @@ -1113,13 +1063,7 @@ impl CryptoStates { pub fn set_application_write_key(&mut self, version: Version, secret: &SymKey) -> Res<()> { debug_assert!(self.app_write.is_none()); debug_assert_ne!(self.cipher, 0); - let mut app = CryptoDxAppData::new( - version, - CryptoDxDirection::Write, - secret, - self.cipher, - self.fuzzing, - )?; + let mut app = CryptoDxAppData::new(version, CryptoDxDirection::Write, secret, self.cipher)?; if let Some(z) = &self.zero_rtt { if z.direction == CryptoDxDirection::Write { app.dx.continuation(z)?; @@ -1138,13 +1082,7 @@ impl CryptoStates { ) -> Res<()> { debug_assert!(self.app_write.is_some(), "should have write keys installed"); debug_assert!(self.app_read.is_none()); - let mut app = CryptoDxAppData::new( - version, - CryptoDxDirection::Read, - secret, - self.cipher, - self.fuzzing, - )?; + let mut app = CryptoDxAppData::new(version, CryptoDxDirection::Read, secret, self.cipher)?; if let Some(z) = &self.zero_rtt { if z.direction == CryptoDxDirection::Read { app.dx.continuation(z)?; @@ -1286,7 +1224,7 @@ impl CryptoStates { } /// Make some state for removing protection in tests. - #[cfg(not(feature = "fuzzing"))] + #[cfg(not(feature = "disable-encryption"))] #[cfg(test)] pub(crate) fn test_default() -> Self { let read = |epoch| { @@ -1299,7 +1237,6 @@ impl CryptoStates { dx: read(epoch), cipher: TLS_AES_128_GCM_SHA256, next_secret: hkdf::import_key(TLS_VERSION_1_3, &[0xaa; 32]).unwrap(), - fuzzing: false, }; let mut initials = HashMap::new(); initials.insert( @@ -1319,11 +1256,10 @@ impl CryptoStates { app_read: Some(app_read(3)), app_read_next: Some(app_read(4)), read_update_time: None, - fuzzing: false, } } - #[cfg(all(not(feature = "fuzzing"), test))] + #[cfg(all(not(feature = "disable-encryption"), test))] pub(crate) fn test_chacha() -> Self { const SECRET: &[u8] = &[ 0x9a, 0xc3, 0x12, 0xa7, 0xf8, 0x77, 0x46, 0x8e, 0xbe, 0x69, 0x42, 0x27, 0x48, 0xad, @@ -1337,7 +1273,6 @@ impl CryptoStates { direction: CryptoDxDirection::Read, epoch, aead: Aead::new( - false, TLS_VERSION_1_3, TLS_CHACHA20_POLY1305_SHA256, &secret, @@ -1354,11 +1289,9 @@ impl CryptoStates { used_pn: 0..645_971_972, min_pn: 0, invocations: 10, - fuzzing: false, }, cipher: TLS_CHACHA20_POLY1305_SHA256, next_secret: secret.clone(), - fuzzing: false, }; Self { initials: HashMap::new(), @@ -1369,7 +1302,6 @@ impl CryptoStates { app_read: Some(app_read(3)), app_read_next: Some(app_read(4)), read_update_time: None, - fuzzing: false, } } } diff --git a/third_party/rust/neqo-transport/src/frame.rs b/third_party/rust/neqo-transport/src/frame.rs index b3bb024a2c..d84eb61ce8 100644 --- a/third_party/rust/neqo-transport/src/frame.rs +++ b/third_party/rust/neqo-transport/src/frame.rs @@ -20,7 +20,7 @@ use crate::{ #[allow(clippy::module_name_repetitions)] pub type FrameType = u64; -const FRAME_TYPE_PADDING: FrameType = 0x0; +pub const FRAME_TYPE_PADDING: FrameType = 0x0; pub const FRAME_TYPE_PING: FrameType = 0x1; pub const FRAME_TYPE_ACK: FrameType = 0x2; const FRAME_TYPE_ACK_ECN: FrameType = 0x3; @@ -95,6 +95,12 @@ impl From for CloseError { } } +impl From for Error { + fn from(_err: std::array::TryFromSliceError) -> Self { + Self::FrameEncodingError + } +} + #[derive(PartialEq, Eq, Debug, Default, Clone)] pub struct AckRange { pub(crate) gap: u64, @@ -103,7 +109,7 @@ pub struct AckRange { #[derive(PartialEq, Eq, Debug, Clone)] pub enum Frame<'a> { - Padding, + Padding(u16), Ping, Ack { largest_acknowledged: u64, @@ -213,9 +219,10 @@ impl<'a> Frame<'a> { } } + #[must_use] pub fn get_type(&self) -> FrameType { match self { - Self::Padding => FRAME_TYPE_PADDING, + Self::Padding { .. } => FRAME_TYPE_PADDING, Self::Ping => FRAME_TYPE_PING, Self::Ack { .. } => FRAME_TYPE_ACK, // We don't do ACK ECN. Self::ResetStream { .. } => FRAME_TYPE_RESET_STREAM, @@ -254,6 +261,7 @@ impl<'a> Frame<'a> { } } + #[must_use] pub fn is_stream(&self) -> bool { matches!( self, @@ -269,6 +277,7 @@ impl<'a> Frame<'a> { ) } + #[must_use] pub fn stream_type(fin: bool, nonzero_offset: bool, fill: bool) -> u64 { let mut t = FRAME_TYPE_STREAM; if fin { @@ -285,19 +294,21 @@ impl<'a> Frame<'a> { /// If the frame causes a recipient to generate an ACK within its /// advertised maximum acknowledgement delay. + #[must_use] pub fn ack_eliciting(&self) -> bool { !matches!( self, - Self::Ack { .. } | Self::Padding | Self::ConnectionClose { .. } + Self::Ack { .. } | Self::Padding { .. } | Self::ConnectionClose { .. } ) } /// If the frame can be sent in a path probe /// without initiating migration to that path. + #[must_use] pub fn path_probing(&self) -> bool { matches!( self, - Self::Padding + Self::Padding { .. } | Self::NewConnectionId { .. } | Self::PathChallenge { .. } | Self::PathResponse { .. } @@ -307,6 +318,10 @@ impl<'a> Frame<'a> { /// Converts `AckRanges` as encoded in a ACK frame (see -transport /// 19.3.1) into ranges of acked packets (end, start), inclusive of /// start and end values. + /// + /// # Errors + /// + /// Returns an error if the ranges are invalid. pub fn decode_ack_frame( largest_acked: u64, first_ack_range: u64, @@ -347,36 +362,36 @@ impl<'a> Frame<'a> { Ok(acked_ranges) } - pub fn dump(&self) -> Option { + #[must_use] + pub fn dump(&self) -> String { match self { - Self::Crypto { offset, data } => Some(format!( - "Crypto {{ offset: {}, len: {} }}", - offset, - data.len() - )), + Self::Crypto { offset, data } => { + format!("Crypto {{ offset: {}, len: {} }}", offset, data.len()) + } Self::Stream { stream_id, offset, fill, data, fin, - } => Some(format!( + } => format!( "Stream {{ stream_id: {}, offset: {}, len: {}{}, fin: {} }}", stream_id.as_u64(), offset, if *fill { ">>" } else { "" }, data.len(), fin, - )), - Self::Padding => None, - Self::Datagram { data, .. } => Some(format!("Datagram {{ len: {} }}", data.len())), - _ => Some(format!("{self:?}")), + ), + Self::Padding(length) => format!("Padding {{ len: {length} }}"), + Self::Datagram { data, .. } => format!("Datagram {{ len: {} }}", data.len()), + _ => format!("{self:?}"), } } + #[must_use] pub fn is_allowed(&self, pt: PacketType) -> bool { match self { - Self::Padding | Self::Ping => true, + Self::Padding { .. } | Self::Ping => true, Self::Crypto { .. } | Self::Ack { .. } | Self::ConnectionClose { @@ -388,6 +403,9 @@ impl<'a> Frame<'a> { } } + /// # Errors + /// + /// Returns an error if the frame cannot be decoded. #[allow(clippy::too_many_lines)] // Yeah, but it's a nice match statement. pub fn decode(dec: &mut Decoder<'a>) -> Res { /// Maximum ACK Range Count in ACK Frame @@ -409,13 +427,23 @@ impl<'a> Frame<'a> { } // TODO(ekr@rtfm.com): check for minimal encoding - let t = d(dec.decode_varint())?; + let t = dv(dec)?; match t { - FRAME_TYPE_PADDING => Ok(Self::Padding), + FRAME_TYPE_PADDING => { + let mut length: u16 = 1; + while let Some(b) = dec.peek_byte() { + if u64::from(b) != FRAME_TYPE_PADDING { + break; + } + length += 1; + dec.skip(1); + } + Ok(Self::Padding(length)) + } FRAME_TYPE_PING => Ok(Self::Ping), FRAME_TYPE_RESET_STREAM => Ok(Self::ResetStream { stream_id: StreamId::from(dv(dec)?), - application_error_code: d(dec.decode_varint())?, + application_error_code: dv(dec)?, final_size: match dec.decode_varint() { Some(v) => v, _ => return Err(Error::NoMoreData), @@ -457,12 +485,12 @@ impl<'a> Frame<'a> { } FRAME_TYPE_STOP_SENDING => Ok(Self::StopSending { stream_id: StreamId::from(dv(dec)?), - application_error_code: d(dec.decode_varint())?, + application_error_code: dv(dec)?, }), FRAME_TYPE_CRYPTO => { let offset = dv(dec)?; let data = d(dec.decode_vvec())?; - if offset + u64::try_from(data.len()).unwrap() > ((1 << 62) - 1) { + if offset + u64::try_from(data.len())? > ((1 << 62) - 1) { return Err(Error::FrameEncodingError); } Ok(Self::Crypto { offset, data }) @@ -489,7 +517,7 @@ impl<'a> Frame<'a> { qtrace!("STREAM frame, with length"); d(dec.decode_vvec())? }; - if o + u64::try_from(data.len()).unwrap() > ((1 << 62) - 1) { + if o + u64::try_from(data.len())? > ((1 << 62) - 1) { return Err(Error::FrameEncodingError); } Ok(Self::Stream { @@ -538,7 +566,7 @@ impl<'a> Frame<'a> { return Err(Error::DecodingFrame); } let srt = d(dec.decode(16))?; - let stateless_reset_token = <&[_; 16]>::try_from(srt).unwrap(); + let stateless_reset_token = <&[_; 16]>::try_from(srt)?; Ok(Self::NewConnectionId { sequence_number, @@ -563,7 +591,7 @@ impl<'a> Frame<'a> { Ok(Self::PathResponse { data: datav }) } FRAME_TYPE_CONNECTION_CLOSE_TRANSPORT | FRAME_TYPE_CONNECTION_CLOSE_APPLICATION => { - let error_code = CloseError::from_type_bit(t, d(dec.decode_varint())?); + let error_code = CloseError::from_type_bit(t, dv(dec)?); let frame_type = if t == FRAME_TYPE_CONNECTION_CLOSE_TRANSPORT { dv(dec)? } else { @@ -631,8 +659,10 @@ mod tests { #[test] fn padding() { - let f = Frame::Padding; + let f = Frame::Padding(1); just_dec(&f, "00"); + let f = Frame::Padding(2); + just_dec(&f, "0000"); } #[test] @@ -888,8 +918,8 @@ mod tests { #[test] fn test_compare() { - let f1 = Frame::Padding; - let f2 = Frame::Padding; + let f1 = Frame::Padding(1); + let f2 = Frame::Padding(1); let f3 = Frame::Crypto { offset: 0, data: &[1, 2, 3], diff --git a/third_party/rust/neqo-transport/src/lib.rs b/third_party/rust/neqo-transport/src/lib.rs index be482c466f..5488472b58 100644 --- a/third_party/rust/neqo-transport/src/lib.rs +++ b/third_party/rust/neqo-transport/src/lib.rs @@ -6,7 +6,7 @@ #![allow(clippy::module_name_repetitions)] // This lint doesn't work here. -use neqo_common::qinfo; +use neqo_common::qwarn; use neqo_crypto::Error as CryptoError; mod ackrate; @@ -70,8 +70,8 @@ const ERROR_AEAD_LIMIT_REACHED: TransportError = 15; #[derive(Clone, Debug, PartialEq, PartialOrd, Ord, Eq)] pub enum Error { NoError, - // Each time tihe error is return a different parameter is supply. - // This will be use to distinguish each occurance of this error. + // Each time this error is returned a different parameter is supplied. + // This will be used to distinguish each occurance of this error. InternalError, ConnectionRefused, FlowControlError, @@ -165,7 +165,7 @@ impl Error { impl From for Error { fn from(err: CryptoError) -> Self { - qinfo!("Crypto operation failed {:?}", err); + qwarn!("Crypto operation failed {:?}", err); match err { CryptoError::EchRetry(config) => Self::EchRetry(config), _ => Self::CryptoError(err), diff --git a/third_party/rust/neqo-transport/src/packet/mod.rs b/third_party/rust/neqo-transport/src/packet/mod.rs index 8458f69779..ce611a9664 100644 --- a/third_party/rust/neqo-transport/src/packet/mod.rs +++ b/third_party/rust/neqo-transport/src/packet/mod.rs @@ -18,6 +18,7 @@ use neqo_crypto::random; use crate::{ cid::{ConnectionId, ConnectionIdDecoder, ConnectionIdRef, MAX_CONNECTION_ID_LEN}, crypto::{CryptoDxState, CryptoSpace, CryptoStates}, + frame::FRAME_TYPE_PADDING, version::{Version, WireVersion}, Error, Res, }; @@ -157,7 +158,7 @@ impl PacketBuilder { } Self { encoder, - pn: u64::max_value(), + pn: u64::MAX, header: header_start..header_start, offsets: PacketBuilderOffsets { first_byte_mask: PACKET_HP_MASK_SHORT, @@ -200,7 +201,7 @@ impl PacketBuilder { Self { encoder, - pn: u64::max_value(), + pn: u64::MAX, header: header_start..header_start, offsets: PacketBuilderOffsets { first_byte_mask: PACKET_HP_MASK_LONG, @@ -255,9 +256,14 @@ impl PacketBuilder { /// Maybe pad with "PADDING" frames. /// Only does so if padding was needed and this is a short packet. /// Returns true if padding was added. + /// + /// # Panics + /// + /// Cannot happen. pub fn pad(&mut self) -> bool { if self.padding && !self.is_long() { - self.encoder.pad_to(self.limit, 0); + self.encoder + .pad_to(self.limit, FRAME_TYPE_PADDING.try_into().unwrap()); true } else { false @@ -288,6 +294,10 @@ impl PacketBuilder { /// The length is filled in after calling `build`. /// Does nothing if there isn't 4 bytes available other than render this builder /// unusable; if `remaining()` returns 0 at any point, call `abort()`. + /// + /// # Panics + /// + /// This will panic if the packet number length is too large. pub fn pn(&mut self, pn: PacketNumber, pn_len: usize) { if self.remaining() < 4 { self.limit = 0; @@ -352,6 +362,10 @@ impl PacketBuilder { } /// Build the packet and return the encoder. + /// + /// # Errors + /// + /// This will return an error if the packet is too large. pub fn build(mut self, crypto: &mut CryptoDxState) -> Res { if self.len() > self.limit { qwarn!("Packet contents are more than the limit"); @@ -376,7 +390,9 @@ impl PacketBuilder { // Calculate the mask. let offset = SAMPLE_OFFSET - self.offsets.pn.len(); - assert!(offset + SAMPLE_SIZE <= ciphertext.len()); + if offset + SAMPLE_SIZE > ciphertext.len() { + return Err(Error::InternalError); + } let sample = &ciphertext[offset..offset + SAMPLE_SIZE]; let mask = crypto.compute_mask(sample)?; @@ -410,6 +426,10 @@ impl PacketBuilder { /// As this is a simple packet, this is just an associated function. /// As Retry is odd (it has to be constructed with leading bytes), /// this returns a [`Vec`] rather than building on an encoder. + /// + /// # Errors + /// + /// This will return an error if AEAD encrypt fails. #[allow(clippy::similar_names)] // scid and dcid are fine here. pub fn retry( version: Version, @@ -443,6 +463,7 @@ impl PacketBuilder { /// Make a Version Negotiation packet. #[allow(clippy::similar_names)] // scid and dcid are fine here. + #[must_use] pub fn version_negotiation( dcid: &[u8], scid: &[u8], @@ -534,7 +555,10 @@ impl<'a> PublicPacket<'a> { if packet_type == PacketType::Retry { let header_len = decoder.offset(); let expansion = retry::expansion(version); - let token = Self::opt(decoder.decode(decoder.remaining() - expansion))?; + let token = decoder + .remaining() + .checked_sub(expansion) + .map_or(Err(Error::InvalidPacket), |v| Self::opt(decoder.decode(v)))?; if token.is_empty() { return Err(Error::InvalidPacket); } @@ -554,6 +578,10 @@ impl<'a> PublicPacket<'a> { /// Decode the common parts of a packet. This provides minimal parsing and validation. /// Returns a tuple of a `PublicPacket` and a slice with any remainder from the datagram. + /// + /// # Errors + /// + /// This will return an error if the packet could not be decoded. #[allow(clippy::similar_names)] // For dcid and scid, which are fine. pub fn decode(data: &'a [u8], dcid_decoder: &dyn ConnectionIdDecoder) -> Res<(Self, &'a [u8])> { let mut decoder = Decoder::new(data); @@ -585,7 +613,7 @@ impl<'a> PublicPacket<'a> { } // Generic long header. - let version = WireVersion::try_from(Self::opt(decoder.decode_uint(4))?).unwrap(); + let version = WireVersion::try_from(Self::opt(decoder.decode_uint(4))?)?; let dcid = ConnectionIdRef::from(Self::opt(decoder.decode_vec(1))?); let scid = ConnectionIdRef::from(Self::opt(decoder.decode_vec(1))?); @@ -645,11 +673,14 @@ impl<'a> PublicPacket<'a> { } /// Validate the given packet as though it were a retry. + #[must_use] pub fn is_valid_retry(&self, odcid: &ConnectionId) -> bool { if self.packet_type != PacketType::Retry { return false; } - let version = self.version().unwrap(); + let Some(version) = self.version() else { + return false; + }; let expansion = retry::expansion(version); if self.data.len() <= expansion { return false; @@ -665,6 +696,7 @@ impl<'a> PublicPacket<'a> { .unwrap_or(false) } + #[must_use] pub fn is_valid_initial(&self) -> bool { // Packet has to be an initial, with a DCID of 8 bytes, or a token. // Note: the Server class validates the token and checks the length. @@ -672,32 +704,42 @@ impl<'a> PublicPacket<'a> { && (self.dcid().len() >= 8 || !self.token.is_empty()) } + #[must_use] pub fn packet_type(&self) -> PacketType { self.packet_type } + #[must_use] pub fn dcid(&self) -> ConnectionIdRef<'a> { self.dcid } + /// # Panics + /// + /// This will panic if called for a short header packet. + #[must_use] pub fn scid(&self) -> ConnectionIdRef<'a> { self.scid .expect("should only be called for long header packets") } + #[must_use] pub fn token(&self) -> &'a [u8] { self.token } + #[must_use] pub fn version(&self) -> Option { self.version.and_then(|v| Version::try_from(v).ok()) } + #[must_use] pub fn wire_version(&self) -> WireVersion { debug_assert!(self.version.is_some()); self.version.unwrap_or(0) } + #[must_use] pub fn len(&self) -> usize { self.data.len() } @@ -725,14 +767,10 @@ impl<'a> PublicPacket<'a> { assert_ne!(self.packet_type, PacketType::Retry); assert_ne!(self.packet_type, PacketType::VersionNegotiation); - qtrace!( - "unmask hdr={}", - hex(&self.data[..self.header_len + SAMPLE_OFFSET]) - ); - let sample_offset = self.header_len + SAMPLE_OFFSET; let mask = if let Some(sample) = self.data.get(sample_offset..(sample_offset + SAMPLE_SIZE)) { + qtrace!("unmask hdr={}", hex(&self.data[..sample_offset])); crypto.compute_mask(sample) } else { Err(Error::NoMoreData) @@ -776,6 +814,9 @@ impl<'a> PublicPacket<'a> { )) } + /// # Errors + /// + /// This will return an error if the packet cannot be decrypted. pub fn decrypt(&self, crypto: &mut CryptoStates, release_at: Instant) -> Res { let cspace: CryptoSpace = self.packet_type.into(); // When we don't have a version, the crypto code doesn't need a version @@ -790,7 +831,9 @@ impl<'a> PublicPacket<'a> { // too small (which is public information). let (key_phase, pn, header, body) = self.decrypt_header(rx)?; qtrace!([rx], "decoded header: {:?}", header); - let rx = crypto.rx(version, cspace, key_phase).unwrap(); + let Some(rx) = crypto.rx(version, cspace, key_phase) else { + return Err(Error::DecryptError); + }; let version = rx.version(); // Version fixup; see above. let d = rx.decrypt(pn, &header, body)?; // If this is the first packet ever successfully decrypted @@ -813,8 +856,14 @@ impl<'a> PublicPacket<'a> { } } + /// # Errors + /// + /// This will return an error if the packet is not a version negotiation packet + /// or if the versions cannot be decoded. pub fn supported_versions(&self) -> Res> { - assert_eq!(self.packet_type, PacketType::VersionNegotiation); + if self.packet_type != PacketType::VersionNegotiation { + return Err(Error::InvalidPacket); + } let mut decoder = Decoder::new(&self.data[self.header_len..]); let mut res = Vec::new(); while decoder.remaining() > 0 { @@ -845,14 +894,17 @@ pub struct DecryptedPacket { } impl DecryptedPacket { + #[must_use] pub fn version(&self) -> Version { self.version } + #[must_use] pub fn packet_type(&self) -> PacketType { self.pt } + #[must_use] pub fn pn(&self) -> PacketNumber { self.pn } @@ -866,7 +918,7 @@ impl Deref for DecryptedPacket { } } -#[cfg(all(test, not(feature = "fuzzing")))] +#[cfg(all(test, not(feature = "disable-encryption")))] mod tests { use neqo_common::Encoder; use test_fixture::{fixture_init, now}; @@ -1469,4 +1521,21 @@ mod tests { assert_eq!(decrypted.pn(), 654_360_564); assert_eq!(&decrypted[..], &[0x01]); } + + #[test] + fn decode_empty() { + neqo_crypto::init().unwrap(); + let res = PublicPacket::decode(&[], &EmptyConnectionIdGenerator::default()); + assert!(res.is_err()); + } + + #[test] + fn decode_too_short() { + neqo_crypto::init().unwrap(); + let res = PublicPacket::decode( + &[179, 255, 0, 0, 32, 0, 0], + &EmptyConnectionIdGenerator::default(), + ); + assert!(res.is_err()); + } } diff --git a/third_party/rust/neqo-transport/src/packet/retry.rs b/third_party/rust/neqo-transport/src/packet/retry.rs index 72036d3b49..71193b9100 100644 --- a/third_party/rust/neqo-transport/src/packet/retry.rs +++ b/third_party/rust/neqo-transport/src/packet/retry.rs @@ -18,7 +18,6 @@ fn make_aead(version: Version) -> Aead { let secret = hkdf::import_key(TLS_VERSION_1_3, version.retry_secret()).unwrap(); Aead::new( - false, TLS_VERSION_1_3, TLS_AES_128_GCM_SHA256, &secret, diff --git a/third_party/rust/neqo-transport/src/path.rs b/third_party/rust/neqo-transport/src/path.rs index 4e8d9958ab..50e458ff36 100644 --- a/third_party/rust/neqo-transport/src/path.rs +++ b/third_party/rust/neqo-transport/src/path.rs @@ -216,7 +216,7 @@ impl Paths { /// to a migration from a peer, in which case the old path needs to be probed. #[must_use] fn select_primary(&mut self, path: &PathRef) -> Option { - qinfo!([path.borrow()], "set as primary path"); + qdebug!([path.borrow()], "set as primary path"); let old_path = self.primary.replace(Rc::clone(path)).map(|old| { old.borrow_mut().set_primary(false); old diff --git a/third_party/rust/neqo-transport/src/qlog.rs b/third_party/rust/neqo-transport/src/qlog.rs index 2572966104..a8ad986d2a 100644 --- a/third_party/rust/neqo-transport/src/qlog.rs +++ b/third_party/rust/neqo-transport/src/qlog.rs @@ -195,7 +195,7 @@ pub fn packet_sent( ) { qlog.add_event_with_stream(|stream| { let mut d = Decoder::from(body); - let header = PacketHeader::with_type(to_qlog_pkt_type(pt), Some(pn), None, None, None); + let header = PacketHeader::with_type(pt.into(), Some(pn), None, None, None); let raw = RawInfo { length: Some(plen as u64), payload_length: None, @@ -205,7 +205,7 @@ pub fn packet_sent( let mut frames = SmallVec::new(); while d.remaining() > 0 { if let Ok(f) = Frame::decode(&mut d) { - frames.push(frame_to_qlogframe(&f)); + frames.push(QuicFrame::from(&f)); } else { qinfo!("qlog: invalid frame"); break; @@ -231,13 +231,8 @@ pub fn packet_sent( pub fn packet_dropped(qlog: &mut NeqoQlog, public_packet: &PublicPacket) { qlog.add_event_data(|| { - let header = PacketHeader::with_type( - to_qlog_pkt_type(public_packet.packet_type()), - None, - None, - None, - None, - ); + let header = + PacketHeader::with_type(public_packet.packet_type().into(), None, None, None, None); let raw = RawInfo { length: Some(public_packet.len() as u64), payload_length: None, @@ -259,8 +254,7 @@ pub fn packet_dropped(qlog: &mut NeqoQlog, public_packet: &PublicPacket) { pub fn packets_lost(qlog: &mut NeqoQlog, pkts: &[SentPacket]) { qlog.add_event_with_stream(|stream| { for pkt in pkts { - let header = - PacketHeader::with_type(to_qlog_pkt_type(pkt.pt), Some(pkt.pn), None, None, None); + let header = PacketHeader::with_type(pkt.pt.into(), Some(pkt.pn), None, None, None); let ev_data = EventData::PacketLost(PacketLost { header: Some(header), @@ -283,7 +277,7 @@ pub fn packet_received( let mut d = Decoder::from(&payload[..]); let header = PacketHeader::with_type( - to_qlog_pkt_type(public_packet.packet_type()), + public_packet.packet_type().into(), Some(payload.pn()), None, None, @@ -299,7 +293,7 @@ pub fn packet_received( while d.remaining() > 0 { if let Ok(f) = Frame::decode(&mut d) { - frames.push(frame_to_qlogframe(&f)); + frames.push(QuicFrame::from(&f)); } else { qinfo!("qlog: invalid frame"); break; @@ -393,173 +387,180 @@ pub fn metrics_updated(qlog: &mut NeqoQlog, updated_metrics: &[QlogMetric]) { #[allow(clippy::too_many_lines)] // Yeah, but it's a nice match. #[allow(clippy::cast_possible_truncation, clippy::cast_precision_loss)] // No choice here. -fn frame_to_qlogframe(frame: &Frame) -> QuicFrame { - match frame { - Frame::Padding => QuicFrame::Padding, - Frame::Ping => QuicFrame::Ping, - Frame::Ack { - largest_acknowledged, - ack_delay, - first_ack_range, - ack_ranges, - } => { - let ranges = - Frame::decode_ack_frame(*largest_acknowledged, *first_ack_range, ack_ranges).ok(); - - let acked_ranges = ranges.map(|all| { - AckedRanges::Double( - all.into_iter() - .map(RangeInclusive::into_inner) - .collect::>(), - ) - }); - - QuicFrame::Ack { - ack_delay: Some(*ack_delay as f32 / 1000.0), - acked_ranges, - ect1: None, - ect0: None, - ce: None, +impl From<&Frame<'_>> for QuicFrame { + fn from(frame: &Frame) -> Self { + match frame { + // TODO: Add payload length to `QuicFrame::Padding` once + // https://github.com/cloudflare/quiche/pull/1745 is available via the qlog crate. + Frame::Padding { .. } => QuicFrame::Padding, + Frame::Ping => QuicFrame::Ping, + Frame::Ack { + largest_acknowledged, + ack_delay, + first_ack_range, + ack_ranges, + } => { + let ranges = + Frame::decode_ack_frame(*largest_acknowledged, *first_ack_range, ack_ranges) + .ok(); + + let acked_ranges = ranges.map(|all| { + AckedRanges::Double( + all.into_iter() + .map(RangeInclusive::into_inner) + .collect::>(), + ) + }); + + QuicFrame::Ack { + ack_delay: Some(*ack_delay as f32 / 1000.0), + acked_ranges, + ect1: None, + ect0: None, + ce: None, + } } - } - Frame::ResetStream { - stream_id, - application_error_code, - final_size, - } => QuicFrame::ResetStream { - stream_id: stream_id.as_u64(), - error_code: *application_error_code, - final_size: *final_size, - }, - Frame::StopSending { - stream_id, - application_error_code, - } => QuicFrame::StopSending { - stream_id: stream_id.as_u64(), - error_code: *application_error_code, - }, - Frame::Crypto { offset, data } => QuicFrame::Crypto { - offset: *offset, - length: data.len() as u64, - }, - Frame::NewToken { token } => QuicFrame::NewToken { - token: qlog::Token { - ty: Some(qlog::TokenType::Retry), - details: None, - raw: Some(RawInfo { - data: Some(hex(token)), - length: Some(token.len() as u64), - payload_length: None, - }), + Frame::ResetStream { + stream_id, + application_error_code, + final_size, + } => QuicFrame::ResetStream { + stream_id: stream_id.as_u64(), + error_code: *application_error_code, + final_size: *final_size, + }, + Frame::StopSending { + stream_id, + application_error_code, + } => QuicFrame::StopSending { + stream_id: stream_id.as_u64(), + error_code: *application_error_code, + }, + Frame::Crypto { offset, data } => QuicFrame::Crypto { + offset: *offset, + length: data.len() as u64, + }, + Frame::NewToken { token } => QuicFrame::NewToken { + token: qlog::Token { + ty: Some(qlog::TokenType::Retry), + details: None, + raw: Some(RawInfo { + data: Some(hex(token)), + length: Some(token.len() as u64), + payload_length: None, + }), + }, }, - }, - Frame::Stream { - fin, - stream_id, - offset, - data, - .. - } => QuicFrame::Stream { - stream_id: stream_id.as_u64(), - offset: *offset, - length: data.len() as u64, - fin: Some(*fin), - raw: None, - }, - Frame::MaxData { maximum_data } => QuicFrame::MaxData { - maximum: *maximum_data, - }, - Frame::MaxStreamData { - stream_id, - maximum_stream_data, - } => QuicFrame::MaxStreamData { - stream_id: stream_id.as_u64(), - maximum: *maximum_stream_data, - }, - Frame::MaxStreams { - stream_type, - maximum_streams, - } => QuicFrame::MaxStreams { - stream_type: match stream_type { - NeqoStreamType::BiDi => StreamType::Bidirectional, - NeqoStreamType::UniDi => StreamType::Unidirectional, + Frame::Stream { + fin, + stream_id, + offset, + data, + .. + } => QuicFrame::Stream { + stream_id: stream_id.as_u64(), + offset: *offset, + length: data.len() as u64, + fin: Some(*fin), + raw: None, }, - maximum: *maximum_streams, - }, - Frame::DataBlocked { data_limit } => QuicFrame::DataBlocked { limit: *data_limit }, - Frame::StreamDataBlocked { - stream_id, - stream_data_limit, - } => QuicFrame::StreamDataBlocked { - stream_id: stream_id.as_u64(), - limit: *stream_data_limit, - }, - Frame::StreamsBlocked { - stream_type, - stream_limit, - } => QuicFrame::StreamsBlocked { - stream_type: match stream_type { - NeqoStreamType::BiDi => StreamType::Bidirectional, - NeqoStreamType::UniDi => StreamType::Unidirectional, + Frame::MaxData { maximum_data } => QuicFrame::MaxData { + maximum: *maximum_data, }, - limit: *stream_limit, - }, - Frame::NewConnectionId { - sequence_number, - retire_prior, - connection_id, - stateless_reset_token, - } => QuicFrame::NewConnectionId { - sequence_number: *sequence_number as u32, - retire_prior_to: *retire_prior as u32, - connection_id_length: Some(connection_id.len() as u8), - connection_id: hex(connection_id), - stateless_reset_token: Some(hex(stateless_reset_token)), - }, - Frame::RetireConnectionId { sequence_number } => QuicFrame::RetireConnectionId { - sequence_number: *sequence_number as u32, - }, - Frame::PathChallenge { data } => QuicFrame::PathChallenge { - data: Some(hex(data)), - }, - Frame::PathResponse { data } => QuicFrame::PathResponse { - data: Some(hex(data)), - }, - Frame::ConnectionClose { - error_code, - frame_type, - reason_phrase, - } => QuicFrame::ConnectionClose { - error_space: match error_code { - CloseError::Transport(_) => Some(ErrorSpace::TransportError), - CloseError::Application(_) => Some(ErrorSpace::ApplicationError), + Frame::MaxStreamData { + stream_id, + maximum_stream_data, + } => QuicFrame::MaxStreamData { + stream_id: stream_id.as_u64(), + maximum: *maximum_stream_data, }, - error_code: Some(error_code.code()), - error_code_value: Some(0), - reason: Some(String::from_utf8_lossy(reason_phrase).to_string()), - trigger_frame_type: Some(*frame_type), - }, - Frame::HandshakeDone => QuicFrame::HandshakeDone, - Frame::AckFrequency { .. } => QuicFrame::Unknown { - frame_type_value: None, - raw_frame_type: frame.get_type(), - raw: None, - }, - Frame::Datagram { data, .. } => QuicFrame::Datagram { - length: data.len() as u64, - raw: None, - }, + Frame::MaxStreams { + stream_type, + maximum_streams, + } => QuicFrame::MaxStreams { + stream_type: match stream_type { + NeqoStreamType::BiDi => StreamType::Bidirectional, + NeqoStreamType::UniDi => StreamType::Unidirectional, + }, + maximum: *maximum_streams, + }, + Frame::DataBlocked { data_limit } => QuicFrame::DataBlocked { limit: *data_limit }, + Frame::StreamDataBlocked { + stream_id, + stream_data_limit, + } => QuicFrame::StreamDataBlocked { + stream_id: stream_id.as_u64(), + limit: *stream_data_limit, + }, + Frame::StreamsBlocked { + stream_type, + stream_limit, + } => QuicFrame::StreamsBlocked { + stream_type: match stream_type { + NeqoStreamType::BiDi => StreamType::Bidirectional, + NeqoStreamType::UniDi => StreamType::Unidirectional, + }, + limit: *stream_limit, + }, + Frame::NewConnectionId { + sequence_number, + retire_prior, + connection_id, + stateless_reset_token, + } => QuicFrame::NewConnectionId { + sequence_number: *sequence_number as u32, + retire_prior_to: *retire_prior as u32, + connection_id_length: Some(connection_id.len() as u8), + connection_id: hex(connection_id), + stateless_reset_token: Some(hex(stateless_reset_token)), + }, + Frame::RetireConnectionId { sequence_number } => QuicFrame::RetireConnectionId { + sequence_number: *sequence_number as u32, + }, + Frame::PathChallenge { data } => QuicFrame::PathChallenge { + data: Some(hex(data)), + }, + Frame::PathResponse { data } => QuicFrame::PathResponse { + data: Some(hex(data)), + }, + Frame::ConnectionClose { + error_code, + frame_type, + reason_phrase, + } => QuicFrame::ConnectionClose { + error_space: match error_code { + CloseError::Transport(_) => Some(ErrorSpace::TransportError), + CloseError::Application(_) => Some(ErrorSpace::ApplicationError), + }, + error_code: Some(error_code.code()), + error_code_value: Some(0), + reason: Some(String::from_utf8_lossy(reason_phrase).to_string()), + trigger_frame_type: Some(*frame_type), + }, + Frame::HandshakeDone => QuicFrame::HandshakeDone, + Frame::AckFrequency { .. } => QuicFrame::Unknown { + frame_type_value: None, + raw_frame_type: frame.get_type(), + raw: None, + }, + Frame::Datagram { data, .. } => QuicFrame::Datagram { + length: data.len() as u64, + raw: None, + }, + } } } -fn to_qlog_pkt_type(ptype: PacketType) -> qlog::events::quic::PacketType { - match ptype { - PacketType::Initial => qlog::events::quic::PacketType::Initial, - PacketType::Handshake => qlog::events::quic::PacketType::Handshake, - PacketType::ZeroRtt => qlog::events::quic::PacketType::ZeroRtt, - PacketType::Short => qlog::events::quic::PacketType::OneRtt, - PacketType::Retry => qlog::events::quic::PacketType::Retry, - PacketType::VersionNegotiation => qlog::events::quic::PacketType::VersionNegotiation, - PacketType::OtherVersion => qlog::events::quic::PacketType::Unknown, +impl From for qlog::events::quic::PacketType { + fn from(value: PacketType) -> Self { + match value { + PacketType::Initial => qlog::events::quic::PacketType::Initial, + PacketType::Handshake => qlog::events::quic::PacketType::Handshake, + PacketType::ZeroRtt => qlog::events::quic::PacketType::ZeroRtt, + PacketType::Short => qlog::events::quic::PacketType::OneRtt, + PacketType::Retry => qlog::events::quic::PacketType::Retry, + PacketType::VersionNegotiation => qlog::events::quic::PacketType::VersionNegotiation, + PacketType::OtherVersion => qlog::events::quic::PacketType::Unknown, + } } } diff --git a/third_party/rust/neqo-transport/src/stats.rs b/third_party/rust/neqo-transport/src/stats.rs index 9eff503dcf..0a61097010 100644 --- a/third_party/rust/neqo-transport/src/stats.rs +++ b/third_party/rust/neqo-transport/src/stats.rs @@ -14,7 +14,7 @@ use std::{ time::Duration, }; -use neqo_common::qinfo; +use neqo_common::qwarn; use crate::packet::PacketNumber; @@ -168,7 +168,7 @@ impl Stats { pub fn pkt_dropped(&mut self, reason: impl AsRef) { self.dropped_rx += 1; - qinfo!( + qwarn!( [self.info], "Dropped received packet: {}; Total: {}", reason.as_ref(), @@ -206,7 +206,7 @@ impl Debug for Stats { " tx: {} lost {} lateack {} ptoack {}", self.packets_tx, self.lost, self.late_ack, self.pto_ack )?; - writeln!(f, " resumed: {} ", self.resumed)?; + writeln!(f, " resumed: {}", self.resumed)?; writeln!(f, " frames rx:")?; self.frame_rx.fmt(f)?; writeln!(f, " frames tx:")?; -- cgit v1.2.3