diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index 90f50f1ac8..2349482bcd 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -33,7 +33,7 @@ use crate::{ token::ResetToken, transport_parameters::TransportParameters, Dir, EndpointConfig, Frame, Side, StreamId, Transmit, TransportError, TransportErrorCode, - VarInt, MAX_STREAM_COUNT, MIN_INITIAL_SIZE, TIMER_GRANULARITY, + VarInt, MAX_CID_SIZE, MAX_STREAM_COUNT, MIN_INITIAL_SIZE, TIMER_GRANULARITY, }; mod ack_frequency; @@ -608,7 +608,16 @@ impl Connection { buf.len() }; - if !coalesce || buf_capacity - buf_end < MIN_PACKET_SPACE { + let tag_len = if let Some(ref crypto) = self.spaces[space_id].crypto { + crypto.packet.local.tag_len() + } else if space_id == SpaceId::Data { + self.zero_rtt_crypto.as_ref().expect( + "sending packets in the application data space requires known 0-RTT or 1-RTT keys", + ).packet.tag_len() + } else { + unreachable!("tried to send {:?} packet without keys", space_id) + }; + if !coalesce || buf_capacity - buf_end < MIN_PACKET_SPACE + tag_len { // We need to send 1 more datagram and extend the buffer for that. // Is 1 more datagram allowed? @@ -3739,8 +3748,24 @@ fn get_max_ack_delay(params: &TransportParameters) -> Duration { // Prevents overflow and improves behavior in extreme circumstances const MAX_BACKOFF_EXPONENT: u32 = 16; -// Minimal remaining size to allow packet coalescing -const MIN_PACKET_SPACE: usize = 40; + +/// Largest amount of space that could be occupied by a Handshake or 0-RTT packet's header +/// +/// Excludes packet-type-specific fields such as packet number or Initial token +// https://www.rfc-editor.org/rfc/rfc9000.html#name-0-rtt: flags + version + dcid len + dcid + +// scid len + scid + length + pn +const MAX_HANDSHAKE_OR_0RTT_HEADER_SIZE: usize = + 1 + 4 + 1 + MAX_CID_SIZE + 1 + MAX_CID_SIZE + VarInt::from_u32(u16::MAX as u32).size() + 4; + +/// Minimal remaining size to allow packet coalescing, excluding cryptographic tag +/// +/// This must be at least as large as the header for a well-formed empty packet to be coalesced, +/// plus some space for frames. We only care about handshake headers because we short header packets +/// necessarily have smaller headers, and initial packets are only ever the first packet in a +/// datagram (because we coalesce in ascending packet space order and the only reason to split a +/// packet is when packet space changes). +const MIN_PACKET_SPACE: usize = MAX_HANDSHAKE_OR_0RTT_HEADER_SIZE + 32; + /// The maximum amount of datagrams that are sent in a single transmit /// /// This can be lower than the maximum platform capabilities, to avoid excessive diff --git a/quinn-proto/src/connection/packet_builder.rs b/quinn-proto/src/connection/packet_builder.rs index a29ff448ef..70a9867f2e 100644 --- a/quinn-proto/src/connection/packet_builder.rs +++ b/quinn-proto/src/connection/packet_builder.rs @@ -138,12 +138,10 @@ impl PacketBuilder { crypto.packet.local.tag_len(), ) } else if space_id == SpaceId::Data { - let zero_rtt = conn.zero_rtt_crypto.as_ref().expect( - "sending packets in the application data space requires known 0-RTT or 1-RTT keys", - ); + let zero_rtt = conn.zero_rtt_crypto.as_ref().unwrap(); (zero_rtt.header.sample_size(), zero_rtt.packet.tag_len()) } else { - unreachable!("tried to send {:?} packet without keys", space_id); + unreachable!(); }; // Each packet must be large enough for header protection sampling, i.e. the combined @@ -159,6 +157,7 @@ impl PacketBuilder { partial_encode.start + dst_cid.len() + 6, ); let max_size = buffer_capacity - tag_len; + debug_assert!(max_size >= min_size); Some(Self { datagram_start,