Document and tighten CID length validation

quinn-rs · Mar 23, 2024 · f81dc7c · f81dc7c
1 parent 21e308a
commit f81dc7c
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 12 deletions.
diff --git a/quinn-proto/src/endpoint.rs b/quinn-proto/src/endpoint.rs
@@ -232,15 +232,9 @@ impl Endpoint {
                 }
             };
 
-            if let Some(response) = self.early_validate_first_packet(
-                version,
-                addresses,
-                &crypto,
-                // Necessarily `Some(_)` when `first_decode.initial_version()` is `Some(_)`
-                first_decode.src_cid().unwrap(),
-                first_decode.dst_cid(),
-                buf,
-            ) {
+            if let Some(response) =
+                self.early_validate_first_packet(version, addresses, &crypto, &first_decode, buf)
+            {
                 return Some(DatagramEvent::Response(response));
             }
 
@@ -415,11 +409,13 @@ impl Endpoint {
         version: u32,
         addresses: FourTuple,
         crypto: &Keys,
-        src_cid: &ConnectionId,
-        dst_cid: &ConnectionId,
+        first_decode: &PartialDecode,
         buf: &mut BytesMut,
     ) -> Option<Transmit> {
         let server_config = self.server_config.as_ref().unwrap();
+        // Necessarily `Some(_)` on initial packets
+        let src_cid = first_decode.src_cid().unwrap();
+        let dst_cid = first_decode.dst_cid();
         if self.connections.len() >= server_config.concurrent_connections as usize || self.is_full()
         {
             debug!("refusing connection");
@@ -433,8 +429,13 @@ impl Endpoint {
             ));
         }
 
+        // RFC9000 §7.2 dictates that initial (client-chosen) destination CIDs must be at least 8
+        // bytes. If this might be a Retry packet, then the length may instead match our usual CID
+        // length.
         if dst_cid.len() < 8
-            && (!server_config.use_retry || dst_cid.len() != self.local_cid_generator.cid_len())
+            && (!server_config.use_retry
+                || (first_decode.has_token()
+                    && dst_cid.len() != self.local_cid_generator.cid_len()))
         {
             debug!(
                 "rejecting connection due to invalid DCID length {}",

diff --git a/quinn-proto/src/packet.rs b/quinn-proto/src/packet.rs
@@ -107,6 +107,14 @@ impl PartialDecode {
         self.plain_header.dst_cid()
     }
 
+    /// Whether this is an Initial packet with a nonempty address validation token
+    pub(crate) fn has_token(&self) -> bool {
+        match &self.plain_header {
+            PlainHeader::Initial { token_pos, .. } => !token_pos.is_empty(),
+            _ => false,
+        }
+    }
+
     /// Length of QUIC packet being decoded
     #[allow(unreachable_pub)] // fuzzing only
     pub fn len(&self) -> usize {