From 2bff792f1e10d5c277d93d4e0828a51b4caabeca Mon Sep 17 00:00:00 2001 From: Benjamin Saunders Date: Sat, 31 Oct 2020 11:39:45 -0700 Subject: [PATCH 1/5] Draft 32 packet protection limits --- quinn-proto/src/crypto/rustls.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/quinn-proto/src/crypto/rustls.rs b/quinn-proto/src/crypto/rustls.rs index 4061c7181..2a5e9043d 100644 --- a/quinn-proto/src/crypto/rustls.rs +++ b/quinn-proto/src/crypto/rustls.rs @@ -378,7 +378,7 @@ impl crypto::PacketKey for PacketKey { fn confidentiality_limit(&self) -> u64 { let cipher = self.key.algorithm(); if cipher == &aead::AES_128_GCM || cipher == &aead::AES_256_GCM { - 2u64.pow(25) + 2u64.pow(23) } else if cipher == &aead::CHACHA20_POLY1305 { u64::MAX } else { @@ -389,7 +389,7 @@ impl crypto::PacketKey for PacketKey { fn integrity_limit(&self) -> u64 { let cipher = self.key.algorithm(); if cipher == &aead::AES_128_GCM || cipher == &aead::AES_256_GCM { - 2u64.pow(54) + 2u64.pow(52) } else if cipher == &aead::CHACHA20_POLY1305 { 2u64.pow(36) } else { From 18467b85ad2f4c05518252cafc98dfa6a5936432 Mon Sep 17 00:00:00 2001 From: Benjamin Saunders Date: Sat, 31 Oct 2020 12:18:04 -0700 Subject: [PATCH 2/5] Move max_streams transport parameter sanity checks out of Connection --- quinn-proto/src/connection/mod.rs | 7 ------- quinn-proto/src/transport_parameters.rs | 4 +++- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index 4a6a59210..ebf098d05 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -2746,13 +2746,6 @@ where "CID authentication failure", )); } - if params.initial_max_streams_bidi.0 > MAX_STREAM_COUNT - || params.initial_max_streams_uni.0 > MAX_STREAM_COUNT - { - return Err(TransportError::STREAM_LIMIT_ERROR( - "unrepresentable initial stream limit", - )); - } Ok(()) } diff --git a/quinn-proto/src/transport_parameters.rs b/quinn-proto/src/transport_parameters.rs index f8d3ba3a7..74c0c1cc9 100644 --- a/quinn-proto/src/transport_parameters.rs +++ b/quinn-proto/src/transport_parameters.rs @@ -21,7 +21,7 @@ use crate::{ config::{EndpointConfig, ServerConfig, TransportConfig}, crypto, shared::ConnectionId, - ResetToken, Side, TransportError, VarInt, MAX_CID_SIZE, RESET_TOKEN_SIZE, + ResetToken, Side, TransportError, VarInt, MAX_CID_SIZE, MAX_STREAM_COUNT, RESET_TOKEN_SIZE, }; // Apply a given macro to a list of all the transport parameters having integer types, along with @@ -406,6 +406,8 @@ impl TransportParameters { || params.max_ack_delay.0 >= 1 << 14 || params.active_connection_id_limit.0 < 2 || params.max_udp_payload_size.0 < 1200 + || params.initial_max_streams_bidi.0 > MAX_STREAM_COUNT + || params.initial_max_streams_uni.0 > MAX_STREAM_COUNT || (side.is_server() && (params.stateless_reset_token.is_some() || params.preferred_address.is_some())) { From 14c68ee818eba8342d7929edf0b0e3d9330d806a Mon Sep 17 00:00:00 2001 From: Benjamin Saunders Date: Sat, 31 Oct 2020 12:46:40 -0700 Subject: [PATCH 3/5] Pad packets containing PATH_CHALLENGE and PATH_RESPONSE --- quinn-proto/src/connection/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index ebf098d05..34c7705ed 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -378,7 +378,10 @@ where let mut buf = Vec::with_capacity(self.mtu as usize); let mut coalesce = spaces.len() > 1; - let pad_space = if self.side.is_client() && spaces.first() == Some(&SpaceId::Initial) { + let pad_space = if self.side.is_client() && spaces.first() == Some(&SpaceId::Initial) + || self.path.challenge.is_some() + || self.path_response.is_some() + { spaces.last().cloned() } else { None From a9672a81ae0fa221c6467e5a5ac4e4041af3facd Mon Sep 17 00:00:00 2001 From: Benjamin Saunders Date: Sat, 31 Oct 2020 13:02:36 -0700 Subject: [PATCH 4/5] Style tweak --- quinn-proto/src/connection/mod.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index 34c7705ed..28eb590d8 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -378,14 +378,11 @@ where let mut buf = Vec::with_capacity(self.mtu as usize); let mut coalesce = spaces.len() > 1; - let pad_space = if self.side.is_client() && spaces.first() == Some(&SpaceId::Initial) - || self.path.challenge.is_some() - || self.path_response.is_some() - { - spaces.last().cloned() - } else { - None - }; + let pad_space = spaces.last().cloned().filter(|_| { + self.side.is_client() && spaces.first() == Some(&SpaceId::Initial) + || self.path.challenge.is_some() + || self.path_response.is_some() + }); for space_id in spaces { let buf_start = buf.len(); From 831e676b17432ad0a2a19c2fb39e53c1bbb6fdf6 Mon Sep 17 00:00:00 2001 From: Benjamin Saunders Date: Sat, 31 Oct 2020 13:50:40 -0700 Subject: [PATCH 5/5] Support peers on drafts 29 through 32 inclusive --- README.md | 2 +- quinn-proto/src/connection/mod.rs | 10 +++------- quinn-proto/src/endpoint.rs | 2 +- quinn-proto/src/lib.rs | 10 ++++++++-- quinn-proto/src/packet.rs | 8 ++++---- quinn-proto/src/tests/mod.rs | 5 +---- 6 files changed, 18 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 2980fd07c..61e509d4a 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,7 @@ Quinn was created and is maintained by Dirkjan Ochtman and Benjamin Saunders. ## Status -- [x] QUIC draft 27 with TLS 1.3 +- [x] QUIC draft 32 with TLS 1.3 - [x] Cryptographic handshake - [x] Stream data w/ flow control and congestion control - [x] Connection close diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index 28eb590d8..036c12f1f 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -1,7 +1,6 @@ use std::{ cmp, collections::{BTreeMap, VecDeque}, - convert::TryInto, fmt, io, mem, net::SocketAddr, sync::Arc, @@ -20,6 +19,7 @@ use crate::{ crypto::{self, HeaderKey, KeyPair, Keys, PacketKey}, frame, frame::{Close, Datagram, FrameStruct}, + is_supported_version, packet::{Header, LongType, Packet, PacketNumber, PartialDecode, PartialEncode, SpaceId}, range_set::RangeSet, shared::{ @@ -29,7 +29,7 @@ use crate::{ transport_parameters::TransportParameters, Dir, Frame, Side, StreamId, Transmit, TransportError, TransportErrorCode, VarInt, LOC_CID_COUNT, MAX_STREAM_COUNT, MIN_INITIAL_SIZE, MIN_MTU, RESET_TOKEN_SIZE, - TIMER_GRANULARITY, VERSION, + TIMER_GRANULARITY, }; mod assembler; @@ -2092,11 +2092,7 @@ where if self.total_authed_packets > 1 { return Ok(()); } - if packet - .payload - .chunks(4) - .any(|chunk| u32::from_be_bytes(chunk.try_into().unwrap()) == VERSION) - { + if packet.payload.chunks(4).any(is_supported_version) { return Ok(()); } debug!("remote doesn't support our version"); diff --git a/quinn-proto/src/endpoint.rs b/quinn-proto/src/endpoint.rs index 2b5f842de..97b7f039d 100644 --- a/quinn-proto/src/endpoint.rs +++ b/quinn-proto/src/endpoint.rs @@ -182,7 +182,7 @@ where } else { buf.write::(0x0a1a_2a4a); } - buf.write(VERSION); // supported version + buf.write(*VERSION.start()); // supported version self.transmits.push_back(Transmit { destination: remote, ecn: None, diff --git a/quinn-proto/src/lib.rs b/quinn-proto/src/lib.rs index 4436d36c3..c6d5803e9 100644 --- a/quinn-proto/src/lib.rs +++ b/quinn-proto/src/lib.rs @@ -18,7 +18,7 @@ #![allow(clippy::cognitive_complexity)] #![allow(clippy::too_many_arguments)] -use std::{fmt, net::SocketAddr, ops, time::Duration}; +use std::{convert::TryInto, fmt, net::SocketAddr, ops, time::Duration}; mod cid_queue; #[doc(hidden)] @@ -116,8 +116,14 @@ pub mod fuzzing { } } } + /// The QUIC protocol version implemented -const VERSION: u32 = 0xff00_001d; +const VERSION: std::ops::RangeInclusive = 0xff00_001d..=0xff00_0020; + +/// Whether a 4-byte slice represents a supported version number +fn is_supported_version(x: &[u8]) -> bool { + VERSION.contains(&u32::from_be_bytes(x.try_into().unwrap())) +} /// Whether an endpoint was the initiator of a connection #[cfg_attr(feature = "arbitrary", derive(Arbitrary))] diff --git a/quinn-proto/src/packet.rs b/quinn-proto/src/packet.rs index 56b13f0ee..89e355784 100644 --- a/quinn-proto/src/packet.rs +++ b/quinn-proto/src/packet.rs @@ -254,7 +254,7 @@ impl Header { number, } => { w.write(u8::from(LongHeaderType::Initial) | number.tag()); - w.write(VERSION); + w.write(*VERSION.start()); dst_cid.encode_long(w); src_cid.encode_long(w); w.write_var(token.len() as u64); @@ -274,7 +274,7 @@ impl Header { number, } => { w.write(u8::from(LongHeaderType::Standard(ty)) | number.tag()); - w.write(VERSION); + w.write(*VERSION.start()); dst_cid.encode_long(w); src_cid.encode_long(w); w.write::(0); // Placeholder for payload length; see `set_payload_length` @@ -290,7 +290,7 @@ impl Header { ref src_cid, } => { w.write(u8::from(LongHeaderType::Retry)); - w.write(VERSION); + w.write(*VERSION.start()); dst_cid.encode_long(w); src_cid.encode_long(w); PartialEncode { @@ -528,7 +528,7 @@ impl PlainHeader { }); } - if version != VERSION { + if !VERSION.contains(&version) { return Err(PacketDecodeError::UnsupportedVersion { src_cid, dst_cid, diff --git a/quinn-proto/src/tests/mod.rs b/quinn-proto/src/tests/mod.rs index 27a3bb555..2c824c851 100644 --- a/quinn-proto/src/tests/mod.rs +++ b/quinn-proto/src/tests/mod.rs @@ -1,5 +1,4 @@ use std::{ - convert::TryInto, net::{Ipv4Addr, Ipv6Addr, SocketAddr}, sync::Arc, time::{Duration, Instant}, @@ -39,9 +38,7 @@ fn version_negotiate_server() { if let Some(Transmit { contents, .. }) = io { assert_ne!(contents[0] & 0x80, 0); assert_eq!(&contents[1..15], hex!("00000000 04 00000000 04 00000000")); - assert!(contents[15..] - .chunks(4) - .any(|x| u32::from_be_bytes(x.try_into().unwrap()) == VERSION)); + assert!(contents[15..].chunks(4).any(is_supported_version)); } assert_matches!(server.poll_transmit(), None); }