feat: racenonce for deduplicating connection attempts

Author: Frando

quinn-proto/src/config/mod.rs

@@ -525,6 +525,9 @@ impl fmt::Debug for ValidationTokenConfig {
     }
 }
 
+/// Racenonce
+pub(crate) type Racenonce = [u8; 32];
+
 /// Configuration for outgoing connections
 ///
 /// Default values should be suitable for most internet applications.
@@ -545,6 +548,8 @@ pub struct ClientConfig {
 
     /// QUIC protocol version to use
     pub(crate) version: u32,
+
+    pub(crate) racenonce: Option<Racenonce>,
 }
 
 impl ClientConfig {
@@ -558,6 +563,7 @@ impl ClientConfig {
                 RandomConnectionIdGenerator::new(MAX_CID_SIZE).generate_cid()
             }),
             version: 1,
+            racenonce: None,
         }
     }
 
@@ -597,6 +603,12 @@ impl ClientConfig {
         self.version = version;
         self
     }
+
+    /// Set the racenonce.
+    pub fn racenonce(&mut self, racenonce: [u8; 32]) -> &mut Self {
+        self.racenonce = Some(racenonce);
+        self
+    }
 }
 
 #[cfg(any(feature = "rustls-aws-lc-rs", feature = "rustls-ring"))]

quinn-proto/src/connection/mod.rs

@@ -21,7 +21,7 @@ use crate::{
     cid_generator::ConnectionIdGenerator,
     cid_queue::CidQueue,
     coding::BufMutExt,
-    config::{ServerConfig, TransportConfig},
+    config::{Racenonce, ServerConfig, TransportConfig},
     congestion::Controller,
     crypto::{self, KeyPair, Keys, PacketKey},
     frame::{self, Close, Datagram, FrameStruct, NewToken, ObservedAddr},
@@ -4278,6 +4278,10 @@ impl Connection {
             new_tokens.push(remote_addr);
         }
     }
+
+    pub(crate) fn racenonce(&self) -> Option<Racenonce> {
+        self.peer_params.racenonce
+    }
 }
 
 impl fmt::Debug for Connection {
@@ -4414,6 +4418,9 @@ pub enum ConnectionError {
     /// Try using longer connection IDs.
     #[error("CIDs exhausted")]
     CidsExhausted,
+    // /// Duplicate racenonce
+    // #[error("Duplicate racenonce")]
+    // DuplicateRacenonce,
 }
 
 impl From<Close> for ConnectionError {

quinn-proto/src/endpoint.rs

@@ -19,7 +19,7 @@ use crate::{
     ResetToken, Side, Transmit, TransportConfig, TransportError,
     cid_generator::ConnectionIdGenerator,
     coding::BufMutExt,
-    config::{ClientConfig, EndpointConfig, ServerConfig},
+    config::{ClientConfig, EndpointConfig, Racenonce, ServerConfig},
     connection::{Connection, ConnectionError, SideArgs},
     crypto::{self, Keys, UnsupportedVersion},
     frame,
@@ -356,6 +356,7 @@ impl Endpoint {
             &self.config,
             self.local_cid_generator.as_ref(),
             loc_cid,
+            config.racenonce,
             None,
             &mut self.rng,
         );
@@ -612,6 +613,7 @@ impl Endpoint {
             &self.config,
             self.local_cid_generator.as_ref(),
             loc_cid,
+            None,
             Some(&server_config),
             &mut self.rng,
         );
@@ -663,6 +665,33 @@ impl Endpoint {
             incoming.rest,
         ) {
             Ok(()) => {
+                if let Some(nonce) = conn.racenonce() {
+                    if self
+                        .connections
+                        .iter()
+                        .any(|(_idx, conn)| conn.racenonce == Some(nonce))
+                    {
+                        debug!("handshake failed: duplicate racenonce");
+                        self.handle_event(ch, EndpointEvent(EndpointEventInner::Drained));
+                        // TODO(frando): Proper error variant.
+                        let e = TransportError::CONNECTION_REFUSED("duplicate racenonce");
+                        // let response = None;
+                        let response = self.initial_close(
+                            version,
+                            incoming.addresses,
+                            &incoming.crypto,
+                            &src_cid,
+                            e.clone(),
+                            buf,
+                        );
+                        return Err(AcceptError {
+                            cause: e.into(),
+                            response: Some(response),
+                        });
+                    } else {
+                        self.connections[ch].racenonce = Some(nonce);
+                    }
+                }
                 trace!(id = ch.0, icid = %dst_cid, "new connection");
 
                 for event in incoming_buffer.datagrams {
@@ -853,6 +882,7 @@ impl Endpoint {
             addresses,
             side,
             reset_token: None,
+            racenonce: None,
         });
         debug_assert_eq!(id, ch.0, "connection handle allocation out of sync");
 
@@ -1138,6 +1168,7 @@ pub(crate) struct ConnectionMeta {
     /// Reset token provided by the peer for the CID we're currently sending to, and the address
     /// being sent to
     reset_token: Option<(SocketAddr, ResetToken)>,
+    racenonce: Option<Racenonce>,
 }
 
 /// Local connection IDs for a single path

quinn-proto/src/tests/mod.rs

@@ -3569,3 +3569,52 @@ fn reject_short_idcid() {
         panic!("expected an initial close");
     };
 }
+
+#[test]
+fn racenonce() {
+    let _guard = subscribe();
+    let mut pair = Pair::default();
+
+    println!("connect1");
+    // first connect with a racenonce: succeeds
+    {
+        let mut cfg = client_config();
+        cfg.racenonce([1u8; 32]);
+        pair.connect_with(cfg);
+    }
+
+    println!("connect2");
+    // second connect with the same racenonce: fails
+    {
+        let mut cfg = client_config();
+        cfg.racenonce([1u8; 32]);
+
+        let client_ch = pair.begin_connect(cfg);
+        pair.drive();
+
+        assert_matches!(
+            pair.server.assert_accept_error(),
+            ConnectionError::TransportError(TransportError {
+                code,
+                reason,
+                ..
+            })
+            if code == TransportErrorCode::CONNECTION_REFUSED && &reason == "duplicate racenonce"
+        );
+
+        let client_conn = pair.client_conn_mut(client_ch);
+        assert_matches!(
+            client_conn.poll(),
+            Some(Event::ConnectionLost { reason: ConnectionError::ConnectionClosed(err) }) if err.error_code == TransportErrorCode::CONNECTION_REFUSED && err.reason.as_ref() == b"duplicate racenonce"
+        );
+        assert_matches!(client_conn.poll(), None);
+    }
+
+    // third connect with a different racenonce: succeeds
+    println!("connect3");
+    {
+        let mut cfg = client_config();
+        cfg.racenonce([2u8; 32]);
+        pair.connect_with(cfg);
+    }
+}

quinn-proto/src/transport_parameters.rs

@@ -21,7 +21,7 @@ use crate::{
     cid_generator::ConnectionIdGenerator,
     cid_queue::CidQueue,
     coding::{BufExt, BufMutExt, UnexpectedEnd},
-    config::{EndpointConfig, ServerConfig, TransportConfig},
+    config::{EndpointConfig, Racenonce, ServerConfig, TransportConfig},
     connection::PathId,
     shared::ConnectionId,
 };
@@ -118,6 +118,9 @@ macro_rules! make_struct {
 
             // Multipath extension
             pub(crate) initial_max_path_id: Option<PathId>,
+
+            // Racenonce
+            pub(crate) racenonce: Option<Racenonce>,
         }
 
         // We deliberately don't implement the `Default` trait, since that would be public, and
@@ -143,6 +146,7 @@ macro_rules! make_struct {
                     write_order: None,
                     address_discovery_role: address_discovery::Role::Disabled,
                     initial_max_path_id: None,
+                    racenonce: None
                 }
             }
         }
@@ -157,6 +161,7 @@ impl TransportParameters {
         endpoint_config: &EndpointConfig,
         cid_gen: &dyn ConnectionIdGenerator,
         initial_src_cid: ConnectionId,
+        racenonce: Option<Racenonce>,
         server_config: Option<&ServerConfig>,
         rng: &mut impl RngCore,
     ) -> Self {
@@ -193,6 +198,7 @@ impl TransportParameters {
             address_discovery_role: config.address_discovery_role,
             // TODO(@divma): TransportConfig or..?
             initial_max_path_id: config.initial_max_path_id.map(PathId::from),
+            racenonce,
             ..Self::default()
         }
     }
@@ -409,6 +415,13 @@ impl TransportParameters {
                         w.write(val);
                     }
                 }
+                TransportParameterId::Racenonce => {
+                    if let Some(val) = self.racenonce {
+                        w.write_var(id as u64);
+                        w.write_var(val.len() as u64);
+                        w.put_slice(&val);
+                    }
+                }
                 id => {
                     macro_rules! write_params {
                         {$($(#[$doc:meta])* $name:ident ($id:ident) = $default:expr,)*} => {
@@ -535,6 +548,14 @@ impl TransportParameters {
                     params.initial_max_path_id = Some(value);
                     tracing::debug!(initial_max_path_id=%value, "multipath enabled");
                 }
+                TransportParameterId::Racenonce => {
+                    if len != 32 || params.racenonce.is_some() {
+                        return Err(Error::Malformed);
+                    }
+                    let mut val = [0; 32];
+                    r.copy_to_slice(&mut val);
+                    params.racenonce = Some(val);
+                }
                 _ => {
                     macro_rules! parse {
                         {$($(#[$doc:meta])* $name:ident ($id:ident) = $default:expr,)*} => {
@@ -703,11 +724,13 @@ pub(crate) enum TransportParameterId {
 
     // https://datatracker.ietf.org/doc/html/draft-ietf-quic-multipath
     InitialMaxPathId = 0x0f739bbc1b666d0c,
+
+    Racenonce = 0x0f138193fac,
 }
 
 impl TransportParameterId {
     /// Array with all supported transport parameter IDs
-    const SUPPORTED: [Self; 23] = [
+    const SUPPORTED: [Self; 24] = [
         Self::MaxIdleTimeout,
         Self::MaxUdpPayloadSize,
         Self::InitialMaxData,
@@ -731,6 +754,7 @@ impl TransportParameterId {
         Self::MinAckDelayDraft07,
         Self::ObservedAddr,
         Self::InitialMaxPathId,
+        Self::Racenonce,
     ];
 }
 
@@ -772,6 +796,7 @@ impl TryFrom<u64> for TransportParameterId {
             id if Self::MinAckDelayDraft07 == id => Self::MinAckDelayDraft07,
             id if Self::ObservedAddr == id => Self::ObservedAddr,
             id if Self::InitialMaxPathId == id => Self::InitialMaxPathId,
+            id if Self::Racenonce == id => Self::Racenonce,
             _ => return Err(()),
         };
         Ok(param)
@@ -812,6 +837,7 @@ mod test {
             min_ack_delay: Some(2_000u32.into()),
             address_discovery_role: address_discovery::Role::SendOnly,
             initial_max_path_id: Some(PathId::MAX),
+            racenonce: Some([42u8; 32]),
             ..TransportParameters::default()
         };
         params.write(&mut buf);

quinn/src/tests.rs

@@ -16,7 +16,10 @@ use std::{
 use crate::runtime::TokioRuntime;
 use crate::{Duration, Instant};
 use bytes::Bytes;
-use proto::{RandomConnectionIdGenerator, crypto::rustls::QuicClientConfig};
+use proto::{
+    ConnectionError, RandomConnectionIdGenerator, TransportErrorCode,
+    crypto::rustls::QuicClientConfig,
+};
 use rand::{RngCore, SeedableRng, rngs::StdRng};
 use rustls::{
     RootCertStore,
@@ -310,6 +313,13 @@ impl EndpointFactory {
 
         endpoint
     }
+
+    fn client_config(&self) -> ClientConfig {
+        let mut roots = rustls::RootCertStore::empty();
+        roots.add(self.cert.cert.der().clone()).unwrap();
+        let client_config = ClientConfig::with_root_certificates(Arc::new(roots)).unwrap();
+        client_config
+    }
 }
 
 #[tokio::test]
@@ -892,3 +902,56 @@ async fn test_multipath_negotiated() {
 
     tokio::join!(server_task, client_task);
 }
+
+#[tokio::test]
+async fn racenonce() {
+    let _guard = subscribe();
+    let factory = EndpointFactory::new();
+    let server = factory.endpoint("server");
+    let server_addr = server.local_addr().unwrap();
+
+    let client = factory.endpoint("client1");
+
+    let client_task = async move {
+        let mut config = factory.client_config();
+        config.racenonce([1u8; 32]);
+        let conn1 = client
+            .connect_with(config, server_addr, "localhost")
+            .unwrap()
+            .await
+            .unwrap();
+        let mut config = factory.client_config();
+        config.racenonce([1u8; 32]);
+        let conn2 = client
+            .connect_with(config, server_addr, "localhost")
+            .unwrap()
+            .await;
+        assert!(matches!(conn2,
+            Err(ConnectionError::ConnectionClosed(ref frame))
+            if frame.error_code == TransportErrorCode::CONNECTION_REFUSED && frame.reason.as_ref() == b"duplicate racenonce"
+        ));
+        let mut config = factory.client_config();
+        config.racenonce([2u8; 32]);
+        let conn3 = client
+            .connect_with(config, server_addr, "localhost")
+            .unwrap()
+            .await
+            .unwrap();
+        drop(conn1);
+        drop(conn2);
+        drop(conn3);
+    };
+    let server_task = async move {
+        let _client1 = server.accept().await.unwrap().await.unwrap();
+        let incoming2 = server.accept().await.unwrap();
+        let res = incoming2.accept();
+        assert!(matches!(
+            res,
+            Err(ConnectionError::TransportError(err))
+            if err.code == TransportErrorCode::CONNECTION_REFUSED && &err.reason == "duplicate racenonce"
+        ));
+        let _client1 = server.accept().await.unwrap().await.unwrap();
+    }
+    .instrument(error_span!("server"));
+    tokio::join!(client_task, server_task);
+}