From db910d98f68621f4c745951abb62028a1bcf0ff0 Mon Sep 17 00:00:00 2001 From: Apoorv Kothari Date: Fri, 31 Mar 2023 22:25:48 -0700 Subject: [PATCH] fix: enforce max data limit size for stream and connection --- quic/s2n-quic-core/src/connection/limits.rs | 8 ++-- .../src/transport/parameters/mod.rs | 39 ++++++++++++++++--- .../src/transport/parameters/tests.rs | 8 ++-- quic/s2n-quic-qns/src/perf.rs | 14 +++---- 4 files changed, 48 insertions(+), 21 deletions(-) diff --git a/quic/s2n-quic-core/src/connection/limits.rs b/quic/s2n-quic-core/src/connection/limits.rs index 8c9a52d41b..83afa820cc 100644 --- a/quic/s2n-quic-core/src/connection/limits.rs +++ b/quic/s2n-quic-core/src/connection/limits.rs @@ -106,21 +106,21 @@ impl Limits { } setter!(with_max_idle_timeout, max_idle_timeout, Duration); - setter!(with_data_window, data_window, u64); + setter!(with_data_window, data_window, u32); setter!( with_bidirectional_local_data_window, bidirectional_local_data_window, - u64 + u32 ); setter!( with_bidirectional_remote_data_window, bidirectional_remote_data_window, - u64 + u32 ); setter!( with_unidirectional_data_window, unidirectional_data_window, - u64 + u32 ); /// Sets both the max local and remote limits for bidirectional streams. diff --git a/quic/s2n-quic-core/src/transport/parameters/mod.rs b/quic/s2n-quic-core/src/transport/parameters/mod.rs index c10007da52..028a3b908d 100644 --- a/quic/s2n-quic-core/src/transport/parameters/mod.rs +++ b/quic/s2n-quic-core/src/transport/parameters/mod.rs @@ -341,6 +341,16 @@ macro_rules! varint_transport_parameter { ($name:ident, $tag:expr $(, $default:expr)?) => { transport_parameter!($name(VarInt), $tag $(, $default)?); + impl $name { + pub const fn as_varint(self) -> VarInt { + self.0 + } + } + }; +} + +macro_rules! varint_from_u64 { + ($name:ident) => { impl TryFrom for $name { type Error = ValidationError; @@ -349,10 +359,17 @@ macro_rules! varint_transport_parameter { Self::try_from(value) } } + }; +} - impl $name { - pub const fn as_varint(self) -> VarInt { - self.0 +macro_rules! varint_from_u32 { + ($name:ident) => { + impl TryFrom for $name { + type Error = ValidationError; + + fn try_from(value: u32) -> Result { + let value = VarInt::from_u32(value); + Self::try_from(value) } } }; @@ -607,9 +624,10 @@ impl TryFrom for MaxUdpPayloadSize { //# immediately after completing the handshake. varint_transport_parameter!(InitialMaxData, 0x04); +varint_from_u32!(InitialMaxData); /// Computes a data window with the given configuration -pub const fn compute_data_window(mbps: u64, rtt: Duration, rtt_count: u64) -> VarInt { +pub const fn compute_data_window(mbps: u64, rtt: Duration, rtt_count: u64) -> u32 { // ideal throughput in Mbps let mut window = mbps; // Mbit/sec * 125 -> bytes/ms @@ -619,12 +637,15 @@ pub const fn compute_data_window(mbps: u64, rtt: Duration, rtt_count: u64) -> Va // bytes/RTT * rtt_count -> N * bytes/RTT window *= rtt_count; - VarInt::from_u32(window as u32) + window as u32 } impl InitialMaxData { /// Tuned for 150Mbps with a 100ms RTT - pub const RECOMMENDED: Self = Self(compute_data_window(150, Duration::from_millis(100), 2)); + pub const RECOMMENDED: Self = { + let limit = compute_data_window(150, Duration::from_millis(100), 2); + Self(VarInt::from_u32(limit)) + }; } impl TransportParameterValidator for InitialMaxData {} @@ -641,6 +662,7 @@ impl TransportParameterValidator for InitialMaxData {} //# 0x01. varint_transport_parameter!(InitialMaxStreamDataBidiLocal, 0x05); +varint_from_u32!(InitialMaxStreamDataBidiLocal); impl InitialMaxStreamDataBidiLocal { /// Tuned for 150Mbps throughput with a 100ms RTT @@ -660,6 +682,7 @@ impl TransportParameterValidator for InitialMaxStreamDataBidiLocal {} //# to streams with the least significant two bits set to 0x00. varint_transport_parameter!(InitialMaxStreamDataBidiRemote, 0x06); +varint_from_u32!(InitialMaxStreamDataBidiRemote); impl InitialMaxStreamDataBidiRemote { /// Tuned for 150Mbps throughput with a 100ms RTT @@ -679,6 +702,7 @@ impl TransportParameterValidator for InitialMaxStreamDataBidiRemote {} //# with the least significant two bits set to 0x02. varint_transport_parameter!(InitialMaxStreamDataUni, 0x07); +varint_from_u32!(InitialMaxStreamDataUni); impl InitialMaxStreamDataUni { /// Tuned for 150Mbps throughput with a 100ms RTT @@ -698,6 +722,7 @@ impl TransportParameterValidator for InitialMaxStreamDataUni {} //# corresponding type with the same value. varint_transport_parameter!(InitialMaxStreamsBidi, 0x08); +varint_from_u64!(InitialMaxStreamsBidi); impl InitialMaxStreamsBidi { /// Allow up to 100 concurrent streams at any time @@ -735,6 +760,7 @@ impl TransportParameterValidator for InitialMaxStreamsBidi { //# (Section 19.11) of the corresponding type with the same value. varint_transport_parameter!(InitialMaxStreamsUni, 0x09); +varint_from_u64!(InitialMaxStreamsUni); impl InitialMaxStreamsUni { /// Allow up to 100 concurrent streams at any time @@ -1018,6 +1044,7 @@ impl EncoderValue for PreferredAddress { //# active_connection_id_limit value received from its peer. varint_transport_parameter!(ActiveConnectionIdLimit, 0x0e, VarInt::from_u8(2)); +varint_from_u64!(ActiveConnectionIdLimit); impl ActiveConnectionIdLimit { /// The recommended value comes from the default of 2 diff --git a/quic/s2n-quic-core/src/transport/parameters/tests.rs b/quic/s2n-quic-core/src/transport/parameters/tests.rs index 90ed564e87..33230766e4 100644 --- a/quic/s2n-quic-core/src/transport/parameters/tests.rs +++ b/quic/s2n-quic-core/src/transport/parameters/tests.rs @@ -183,19 +183,19 @@ fn ignore_unknown_parameter() { #[test] fn compute_data_window_test() { assert_eq!( - *compute_data_window(150, Duration::from_millis(10), 1), + compute_data_window(150, Duration::from_millis(10), 1), 187_500 ); assert_eq!( - *compute_data_window(150, Duration::from_millis(10), 2), + compute_data_window(150, Duration::from_millis(10), 2), 375_000 ); assert_eq!( - *compute_data_window(150, Duration::from_millis(100), 2), + compute_data_window(150, Duration::from_millis(100), 2), 3_750_000 ); assert_eq!( - *compute_data_window(1500, Duration::from_millis(100), 2), + compute_data_window(1500, Duration::from_millis(100), 2), 37_500_000 ); } diff --git a/quic/s2n-quic-qns/src/perf.rs b/quic/s2n-quic-qns/src/perf.rs index f3ae1e81d5..8515b79409 100644 --- a/quic/s2n-quic-qns/src/perf.rs +++ b/quic/s2n-quic-qns/src/perf.rs @@ -111,27 +111,27 @@ pub struct Limits { impl Limits { pub fn limits(&self) -> s2n_quic::provider::limits::Limits { let data_window = self.data_window(); + let max_data_limit = u32::MAX; s2n_quic::provider::limits::Limits::default() - .with_data_window(data_window) + .with_max_send_buffer_size(data_window) .unwrap() - .with_max_send_buffer_size(data_window.min(u32::MAX as _) as _) + .with_data_window(max_data_limit) .unwrap() - .with_bidirectional_local_data_window(data_window) + .with_bidirectional_local_data_window(max_data_limit) .unwrap() - .with_bidirectional_remote_data_window(data_window) + .with_bidirectional_remote_data_window(max_data_limit) .unwrap() - .with_unidirectional_data_window(data_window) + .with_unidirectional_data_window(max_data_limit) .unwrap() } - fn data_window(&self) -> u64 { + fn data_window(&self) -> u32 { s2n_quic_core::transport::parameters::compute_data_window( self.max_throughput, core::time::Duration::from_millis(self.expected_rtt), 2, ) - .as_u64() } }