Skip to content

Commit db09fd7

Browse files
committed
Adds connection limits config
1 parent f91fdb2 commit db09fd7

File tree

3 files changed

+34
-7
lines changed

3 files changed

+34
-7
lines changed

quic/s2n-quic-core/src/connection/limits.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ const MAX_KEEP_ALIVE_PERIOD_DEFAULT: Duration = Duration::from_secs(30);
3939
pub const ANTI_AMPLIFICATION_MULTIPLIER: u8 = 3;
4040

4141
pub const DEFAULT_STREAM_BATCH_SIZE: u8 = 1;
42+
pub const DEFAULT_STORED_PACKET_SIZE: usize = 0;
4243

4344
#[non_exhaustive]
4445
#[derive(Debug)]
@@ -104,6 +105,7 @@ pub struct Limits {
104105
pub(crate) migration_support: MigrationSupport,
105106
pub(crate) anti_amplification_multiplier: u8,
106107
pub(crate) stream_batch_size: u8,
108+
pub(crate) stored_packet_size: usize,
107109
}
108110

109111
impl Default for Limits {
@@ -151,6 +153,7 @@ impl Limits {
151153
migration_support: MigrationSupport::RECOMMENDED,
152154
anti_amplification_multiplier: ANTI_AMPLIFICATION_MULTIPLIER,
153155
stream_batch_size: DEFAULT_STREAM_BATCH_SIZE,
156+
stored_packet_size: DEFAULT_STORED_PACKET_SIZE,
154157
}
155158
}
156159

@@ -254,6 +257,7 @@ impl Limits {
254257
u64
255258
);
256259
setter!(with_stream_batch_size, stream_batch_size, u8);
260+
setter!(with_stored_packet_size, stored_packet_size, usize);
257261
setter!(with_ack_elicitation_interval, ack_elicitation_interval, u8);
258262
setter!(with_max_ack_ranges, ack_ranges_limit, u8);
259263
setter!(
@@ -422,6 +426,12 @@ impl Limits {
422426
pub fn stream_batch_size(&self) -> u8 {
423427
self.stream_batch_size
424428
}
429+
430+
#[doc(hidden)]
431+
#[inline]
432+
pub fn stored_packet_size(&self) -> usize {
433+
self.stored_packet_size
434+
}
425435
}
426436

427437
#[must_use]

quic/s2n-quic-transport/src/connection/connection_impl.rs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ impl<Config: endpoint::Config> ConnectionImpl<Config> {
364364
// use `from` instead of `into` so the location is correctly captured
365365
Poll::Ready(Err(err)) => return Err(connection::Error::from(err)),
366366
Poll::Pending => {
367-
// Process stored handshake packets if the handshake space was created during the last poll crypto call
367+
// Process stored handshake packets if the handshake space was recently created
368368
if self.space_manager.handshake().is_some()
369369
&& self.stored_packet_type == Some(PacketNumberSpace::Handshake)
370370
{
@@ -1620,8 +1620,12 @@ impl<Config: endpoint::Config> connection::Trait for ConnectionImpl<Config> {
16201620
//# The client MAY drop these packets, or it MAY buffer them in anticipation
16211621
//# of later packets that allow it to compute the key.
16221622

1623-
self.packet_storage = packet.get_wire_bytes();
1624-
self.stored_packet_type = Some(PacketNumberSpace::Handshake)
1623+
let packet_bytes = packet.get_wire_bytes();
1624+
if packet_bytes.len() + self.packet_storage.len() < self.limits.stored_packet_size()
1625+
{
1626+
self.packet_storage.extend(packet_bytes);
1627+
self.stored_packet_type = Some(PacketNumberSpace::Handshake)
1628+
}
16251629
} else {
16261630
let path = &self.path_manager[path_id];
16271631
publisher.on_packet_dropped(event::builder::PacketDropped {
@@ -1666,7 +1670,6 @@ impl<Config: endpoint::Config> connection::Trait for ConnectionImpl<Config> {
16661670
//# complete.
16671671

16681672
if !self.space_manager.is_handshake_complete() {
1669-
// We only store one packet of application data for now.
16701673
if self.stored_packet_type.is_none() {
16711674
//= https://www.rfc-editor.org/rfc/rfc9001#section-4.1.4
16721675
//# However, a TLS implementation could perform some of its processing
@@ -1684,8 +1687,14 @@ impl<Config: endpoint::Config> connection::Trait for ConnectionImpl<Config> {
16841687
//# The client MAY drop these packets, or it MAY buffer them in anticipation
16851688
//# of later packets that allow it to compute the key.
16861689

1687-
self.packet_storage = packet.get_wire_bytes();
1688-
self.stored_packet_type = Some(PacketNumberSpace::ApplicationData);
1690+
let packet_bytes = packet.get_wire_bytes();
1691+
if packet_bytes.len() < self.limits.stored_packet_size() {
1692+
// We only store one packet of application data for now. This is due to the fact that
1693+
// short packets do not contain a length prefix, therefore, we would have to store additional
1694+
// length info per packet to properly parse them once the application space is created.
1695+
self.packet_storage = packet_bytes;
1696+
self.stored_packet_type = Some(PacketNumberSpace::ApplicationData)
1697+
}
16891698
} else {
16901699
let path = &self.path_manager[path_id];
16911700
publisher.on_packet_dropped(event::builder::PacketDropped {

quic/s2n-quic/src/tests/slow_tls.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55
fn slow_tls() {
66
use super::*;
77
use crate::provider::tls::default;
8-
use s2n_quic_core::crypto::tls::testing::certificates::{CERT_PEM, KEY_PEM};
8+
use s2n_quic_core::{
9+
connection::limits::Limits,
10+
crypto::tls::testing::certificates::{CERT_PEM, KEY_PEM},
11+
};
912

1013
let model = Model::default();
1114

@@ -27,15 +30,20 @@ fn slow_tls() {
2730
endpoint: client_endpoint,
2831
};
2932

33+
// Connections will store up to 4000 bytes of packets that can't be processed yet
34+
let limits = Limits::default().with_stored_packet_size(4000).unwrap();
35+
3036
test(model, |handle| {
3137
let server = Server::builder()
3238
.with_io(handle.builder().build()?)?
39+
.with_limits(limits)?
3340
.with_tls(slow_server)?
3441
.start()?;
3542

3643
let client = Client::builder()
3744
.with_io(handle.builder().build().unwrap())?
3845
.with_tls(slow_client)?
46+
.with_limits(limits)?
3947
.with_event((tracing_events(), MyEvents))?
4048
.start()?;
4149
let addr = start_server(server)?;

0 commit comments

Comments
 (0)