From 0422deada118aeb656f48ccf0abf0bf8ee4ccfd6 Mon Sep 17 00:00:00 2001 From: Benjamin Saunders Date: Wed, 20 May 2020 21:09:25 -0700 Subject: [PATCH] Draft 28 transport parameters --- quinn-proto/src/connection/mod.rs | 38 +++++++++------ quinn-proto/src/endpoint.rs | 61 ++++++++++++++++------- quinn-proto/src/transport_parameters.rs | 64 +++++++++++++++++++++---- 3 files changed, 123 insertions(+), 40 deletions(-) diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index cc791be3f..abb5e9e1e 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -85,8 +85,13 @@ where key_phase: bool, /// Transport parameters set by the peer params: TransportParameters, - /// ConnectionId sent by this client on the first Initial, if a Retry was received. - orig_rem_cid: Option, + /// Source ConnectionId of the first packet received from the peer + orig_rem_cid: ConnectionId, + /// Destination ConnectionId sent by the client on the first Initial + initial_dst_cid: ConnectionId, + /// The value that the server included in the Source Connection ID field of a Retry packet, if + /// one was received + retry_src_cid: Option, /// Total number of outgoing packets that have been deemed lost lost_packets: u64, events: VecDeque, @@ -211,7 +216,9 @@ where zero_rtt_crypto: None, key_phase: false, params: TransportParameters::default(), - orig_rem_cid: None, + orig_rem_cid: rem_cid, + initial_dst_cid: init_cid, + retry_src_cid: None, lost_packets: 0, events: VecDeque::new(), endpoint_events: VecDeque::new(), @@ -1379,11 +1386,13 @@ where .expect("crypto layer didn't supply transport parameters with ticket"); // Certain values must not be cached let params = TransportParameters { - original_connection_id: None, + initial_source_connection_id: None, + original_destination_connection_id: None, preferred_address: None, + retry_source_connection_id: None, stateless_reset_token: None, ack_delay_exponent: TransportParameters::default().ack_delay_exponent, - active_connection_id_limit: 0, + max_ack_delay: TransportParameters::default().max_ack_delay, ..params }; self.set_params(params); @@ -1713,7 +1722,7 @@ where ); } - if self.orig_rem_cid.is_some() + if self.retry_src_cid.is_some() || packet.payload.len() <= 16 // token + 16 byte tag || !S::is_valid_retry( &self.rem_cid, @@ -1734,7 +1743,7 @@ where trace!("retrying with CID {}", rem_cid); let client_hello = state.client_hello.take().unwrap(); - self.orig_rem_cid = Some(self.rem_cid); + self.retry_src_cid = Some(rem_cid); self.rem_cid = rem_cid; self.rem_handshake_cid = rem_cid; @@ -1851,6 +1860,7 @@ where let mut state = state.clone(); self.rem_cid = rem_cid; self.rem_handshake_cid = rem_cid; + self.orig_rem_cid = rem_cid; state.rem_cid_set = true; self.state = State::Handshake(state); } else if rem_cid != self.rem_handshake_cid { @@ -2530,13 +2540,13 @@ where /// Validate transport parameters received from the peer fn validate_params(&mut self, params: &TransportParameters) -> Result<(), TransportError> { - if self.side.is_client() && self.orig_rem_cid != params.original_connection_id { - debug!( - "original connection ID mismatch: expected {:x?}, actual {:x?}", - self.orig_rem_cid, params.original_connection_id - ); - return Err(TransportError::TRANSPORT_PARAMETER_ERROR( - "original CID mismatch", + if Some(self.orig_rem_cid) != params.initial_source_connection_id + || (self.side.is_client() + && (Some(self.initial_dst_cid) != params.original_destination_connection_id + || self.retry_src_cid != params.retry_source_connection_id)) + { + return Err(TransportError::PROTOCOL_VIOLATION( + "CID authentication failure", )); } if params.initial_max_streams_bidi > MAX_STREAM_COUNT diff --git a/quinn-proto/src/endpoint.rs b/quinn-proto/src/endpoint.rs index ac9c2c7b8..f6244902e 100644 --- a/quinn-proto/src/endpoint.rs +++ b/quinn-proto/src/endpoint.rs @@ -390,20 +390,29 @@ where config, server_name, } => { - let params = TransportParameters::new::(&config.transport, &self.config, None); + let params = + TransportParameters::new::(&config.transport, &self.config, loc_cid, None); ( None, config.crypto.start_session(&server_name, ¶ms)?, config.transport, ) } - ConnectionOpts::Server { orig_dst_cid } => { + ConnectionOpts::Server { + orig_dst_cid, + retry_src_cid, + } => { let config = self.server_config.as_ref().unwrap(); - let params = - TransportParameters::new(&config.transport, &self.config, Some(config)); + let params = TransportParameters::new( + &config.transport, + &self.config, + loc_cid, + Some(config), + ); let server_params = TransportParameters { stateless_reset_token: Some(reset_token_for(&*self.config.reset_key, &loc_cid)), - original_connection_id: orig_dst_cid, + original_destination_connection_id: Some(orig_dst_cid), + retry_source_connection_id: retry_src_cid, ..params }; ( @@ -525,12 +534,13 @@ where return None; } - let retry_cid = if server_config.use_stateless_retry { + let (retry_src_cid, orig_dst_cid) = if server_config.use_stateless_retry { if token.is_empty() { // First Initial let token = token::generate( &*server_config.token_key, &remote, + &temp_loc_cid, &dst_cid, SystemTime::now(), ); @@ -553,14 +563,14 @@ where } match token::check(&*server_config.token_key, &remote, &token) { - Some((cid, issued)) + Some((src_cid, dst_cid, issued)) if issued + Duration::from_micros( self.server_config.as_ref().unwrap().retry_token_lifetime, ) > SystemTime::now() => { - Some(cid) + (Some(src_cid), dst_cid) } _ => { debug!("rejecting invalid stateless retry token"); @@ -579,7 +589,7 @@ where } } } else { - None + (None, dst_cid) }; let (ch, mut conn) = self @@ -588,7 +598,8 @@ where src_cid, remote, ConnectionOpts::Server { - orig_dst_cid: retry_cid, + retry_src_cid, + orig_dst_cid, }, now, ) @@ -738,6 +749,7 @@ mod token { pub fn generate( key: &K, address: &SocketAddr, + src_cid: &ConnectionId, dst_cid: &ConnectionId, issued: SystemTime, ) -> Vec @@ -745,8 +757,13 @@ mod token { K: HmacKey, { let mut buf = Vec::new(); + + buf.write(src_cid.len() as u8); + buf.put_slice(src_cid); + buf.write(dst_cid.len() as u8); buf.put_slice(dst_cid); + buf.write::( issued .duration_since(UNIX_EPOCH) @@ -770,17 +787,25 @@ mod token { key: &K, address: &SocketAddr, data: &[u8], - ) -> Option<(ConnectionId, SystemTime)> + ) -> Option<(ConnectionId, ConnectionId, SystemTime)> where K: HmacKey, { let mut reader = io::Cursor::new(data); + let src_cid_len = reader.get::().ok()? as usize; + if src_cid_len > reader.remaining() || src_cid_len > MAX_CID_SIZE { + return None; + } + let src_cid = ConnectionId::new(&reader.bytes()[..src_cid_len]); + reader.advance(src_cid_len); + let dst_cid_len = reader.get::().ok()? as usize; if dst_cid_len > reader.remaining() || dst_cid_len > MAX_CID_SIZE { return None; } - let dst_cid = ConnectionId::new(&data[1..=dst_cid_len]); + let dst_cid = ConnectionId::new(&reader.bytes()[..dst_cid_len]); reader.advance(dst_cid_len); + let issued = UNIX_EPOCH + Duration::new(reader.get::().ok()?, 0); let signature_start = reader.position() as usize; @@ -793,7 +818,7 @@ mod token { buf.write(address.port()); key.verify(&buf, &data[signature_start..]).ok()?; - Some((dst_cid, issued)) + Some((src_cid, dst_cid, issued)) } } @@ -837,7 +862,8 @@ enum ConnectionOpts { server_name: String, }, Server { - orig_dst_cid: Option, + retry_src_cid: Option, + orig_dst_cid: ConnectionId, }, } @@ -912,10 +938,13 @@ mod test { rand::thread_rng().fill_bytes(&mut key); let key = ::new(&key).unwrap(); let addr = SocketAddr::new(Ipv6Addr::LOCALHOST.into(), 4433); + let src_cid = ConnectionId::random(&mut rand::thread_rng(), MAX_CID_SIZE); let dst_cid = ConnectionId::random(&mut rand::thread_rng(), MAX_CID_SIZE); let issued = UNIX_EPOCH + Duration::new(42, 0); // Fractional seconds would be lost - let token = token::generate(&key, &addr, &dst_cid, issued); - let (dst_cid2, issued2) = token::check(&key, &addr, &token).expect("token didn't validate"); + let token = token::generate(&key, &addr, &src_cid, &dst_cid, issued); + let (src_cid2, dst_cid2, issued2) = + token::check(&key, &addr, &token).expect("token didn't validate"); + assert_eq!(src_cid, src_cid2); assert_eq!(dst_cid, dst_cid2); assert_eq!(issued, issued2); } diff --git a/quinn-proto/src/transport_parameters.rs b/quinn-proto/src/transport_parameters.rs index 72ca2660b..65cb84561 100644 --- a/quinn-proto/src/transport_parameters.rs +++ b/quinn-proto/src/transport_parameters.rs @@ -73,10 +73,17 @@ macro_rules! make_struct { pub(crate) disable_active_migration: bool, /// Maximum size for datagram frames pub(crate) max_datagram_frame_size: Option, + /// The value that the endpoint included in the Source Connection ID field of the first + /// Initial packet it sends for the connection + pub(crate) initial_source_connection_id: Option, // Server-only - /// The DCID from the first Initial packet; must be included if a Retry packet was sent - pub(crate) original_connection_id: Option, + /// The value of the Destination Connection ID field from the first Initial packet sent + /// by the client + pub(crate) original_destination_connection_id: Option, + /// The value that the server included in the Source Connection ID field of a Retry + /// packet + pub(crate) retry_source_connection_id: Option, /// Token used by the client to verify a stateless reset from the server pub(crate) stateless_reset_token: Option, /// The server's preferred address for communication after handshake completion @@ -91,8 +98,10 @@ macro_rules! make_struct { disable_active_migration: false, max_datagram_frame_size: None, + initial_source_connection_id: None, - original_connection_id: None, + original_destination_connection_id: None, + retry_source_connection_id: None, stateless_reset_token: None, preferred_address: None, } @@ -107,12 +116,14 @@ impl TransportParameters { pub(crate) fn new( config: &TransportConfig, endpoint_config: &EndpointConfig, + initial_source_connection_id: ConnectionId, server_config: Option<&ServerConfig>, ) -> Self where S: crypto::Session, { TransportParameters { + initial_source_connection_id: Some(initial_source_connection_id), initial_max_streams_bidi: config.stream_window_bidi, initial_max_streams_uni: config.stream_window_uni, initial_max_data: config.receive_window, @@ -143,7 +154,8 @@ impl TransportParameters { /// Check that these parameters are legal when resuming from /// certain cached parameters pub(crate) fn validate_0rtt(&self, cached: &TransportParameters) -> Result<(), TransportError> { - if cached.initial_max_data < self.initial_max_data + if cached.active_connection_id_limit < self.active_connection_id_limit + || cached.initial_max_data < self.initial_max_data || cached.initial_max_stream_data_bidi_local < self.initial_max_stream_data_bidi_local || cached.initial_max_stream_data_bidi_remote < self.initial_max_stream_data_bidi_remote || cached.initial_max_stream_data_uni < self.initial_max_stream_data_uni @@ -270,7 +282,7 @@ impl TransportParameters { w.write_var(31 * 5 + 27); w.write_var(0); - if let Some(ref x) = self.original_connection_id { + if let Some(ref x) = self.original_destination_connection_id { w.write_var(0x00); w.write_var(x.len() as u64); w.put_slice(x); @@ -298,6 +310,18 @@ impl TransportParameters { w.write_var(x.wire_size() as u64); x.write(w); } + + if let Some(ref x) = self.initial_source_connection_id { + w.write_var(0x0f); + w.write_var(x.len() as u64); + w.put_slice(x); + } + + if let Some(ref x) = self.retry_source_connection_id { + w.write_var(0x10); + w.write_var(x.len() as u64); + w.put_slice(x); + } } /// Decode `TransportParameters` from buffer @@ -328,12 +352,14 @@ impl TransportParameters { match id { 0x00 => { - if len > MAX_CID_SIZE as u64 || params.original_connection_id.is_some() { + if len > MAX_CID_SIZE as u64 + || params.original_destination_connection_id.is_some() + { return Err(Error::Malformed); } let mut staging = [0; MAX_CID_SIZE]; r.copy_to_slice(&mut staging[0..len as usize]); - params.original_connection_id = + params.original_destination_connection_id = Some(ConnectionId::new(&staging[0..len as usize])); } 0x02 => { @@ -357,6 +383,24 @@ impl TransportParameters { params.preferred_address = Some(PreferredAddress::read(&mut r.take(len as usize))?); } + 0x0f => { + if len > MAX_CID_SIZE as u64 || params.initial_source_connection_id.is_some() { + return Err(Error::Malformed); + } + let mut staging = [0; MAX_CID_SIZE]; + r.copy_to_slice(&mut staging[0..len as usize]); + params.initial_source_connection_id = + Some(ConnectionId::new(&staging[0..len as usize])); + } + 0x10 => { + if len > MAX_CID_SIZE as u64 || params.retry_source_connection_id.is_some() { + return Err(Error::Malformed); + } + let mut staging = [0; MAX_CID_SIZE]; + r.copy_to_slice(&mut staging[0..len as usize]); + params.retry_source_connection_id = + Some(ConnectionId::new(&staging[0..len as usize])); + } 0x20 => { if len > 8 || params.max_datagram_frame_size.is_some() { return Err(Error::Malformed); @@ -387,9 +431,7 @@ impl TransportParameters { || params.active_connection_id_limit < 2 || params.max_udp_payload_size < 1200 || (side.is_server() - && (params.original_connection_id.is_some() - || params.stateless_reset_token.is_some() - || params.preferred_address.is_some())) + && (params.stateless_reset_token.is_some() || params.preferred_address.is_some())) { return Err(Error::IllegalValue); } @@ -406,6 +448,8 @@ mod test { fn coding() { let mut buf = Vec::new(); let params = TransportParameters { + initial_source_connection_id: Some(ConnectionId::new(&[])), + original_destination_connection_id: Some(ConnectionId::new(&[])), initial_max_streams_bidi: 16, initial_max_streams_uni: 16, ack_delay_exponent: 2,