Opaque config structs

interop/src/main.rs

@@ -291,12 +291,13 @@ impl State {
         if keylog {
             tls_config.key_log = Arc::new(rustls::KeyLogFile::new());
         }
+        let mut transport = quinn::TransportConfig::default();
+        transport
+            .idle_timeout(Some(Duration::from_secs(1)))
+            .unwrap();
         let client_config = quinn::ClientConfig {
             crypto: Arc::new(tls_config),
-            transport: Arc::new(quinn::TransportConfig {
-                idle_timeout: 1_000,
-                ..Default::default()
-            }),
+            transport: Arc::new(transport),
         };
 
         let mut endpoint = quinn::Endpoint::builder();

quinn-proto/src/connection.rs

@@ -42,7 +42,7 @@ pub struct Connection<S>
 where
     S: crypto::Session,
 {
-    endpoint_config: Arc<EndpointConfig>,
+    endpoint_config: Arc<EndpointConfig<S>>,
     server_config: Option<Arc<ServerConfig<S>>>,
     config: Arc<TransportConfig>,
     rng: StdRng,
@@ -109,7 +109,7 @@ where
     /// Whether the idle timer should be reset the next time an ack-eliciting packet is transmitted.
     permit_idle_reset: bool,
     /// Negotiated idle timeout
-    idle_timeout: u64,
+    idle_timeout: Option<Duration>,
     /// Number of the first 1-RTT packet transmitted
     first_1rtt_sent: Option<u64>,
     /// Sequence number of the first remote CID that we haven't been asked to retire
@@ -161,7 +161,7 @@ where
     S: crypto::Session,
 {
     pub(crate) fn new(
-        endpoint_config: Arc<EndpointConfig>,
+        endpoint_config: Arc<EndpointConfig<S>>,
         server_config: Option<Arc<ServerConfig<S>>>,
         config: Arc<TransportConfig>,
         init_cid: ConnectionId,
@@ -652,10 +652,7 @@ where
             .rtt
             .smoothed
             .map_or(self.path.rtt.latest, |x| cmp::max(x, self.path.rtt.latest));
-        let loss_delay = cmp::max(
-            rtt + ((rtt * u32::from(self.config.time_threshold)) / 65536),
-            TIMER_GRANULARITY,
-        );
+        let loss_delay = cmp::max(rtt.mul_f32(self.config.time_threshold), TIMER_GRANULARITY);
 
         // Packets sent before this time are deemed lost.
         let lost_send_time = now - loss_delay;
@@ -718,9 +715,10 @@ where
             return;
         }
         self.recovery_start_time = now;
-        // *= factor
+        // Converting a u64 to f32 risks some precision loss, but a modest amount of error in
+        // congestion window reductions is harmless.
         self.path.congestion_window =
-            (self.path.congestion_window * u64::from(self.config.loss_reduction_factor)) >> 16;
+            (self.path.congestion_window as f32 * self.config.loss_reduction_factor) as u64;
         self.path.congestion_window =
             cmp::max(self.path.congestion_window, self.config.minimum_window);
         self.path.ssthresh = self.path.congestion_window;
@@ -776,7 +774,7 @@ where
     /// Probe Timeout
     fn pto(&self) -> Duration {
         match self.path.rtt.smoothed {
-            None => 2 * Duration::from_micros(self.config.initial_rtt),
+            None => 2 * self.config.initial_rtt,
             Some(srtt) => {
                 srtt + cmp::max(4 * self.path.rtt.var, TIMER_GRANULARITY) + self.max_ack_delay()
             }
@@ -831,25 +829,24 @@ where
     }
 
     fn reset_idle_timeout(&mut self, now: Instant) {
-        if self.idle_timeout == 0 {
-            return;
-        }
+        let timeout = match self.idle_timeout {
+            None => return,
+            Some(x) => x,
+        };
         if self.state.is_closed() {
             self.timers.stop(Timer::Idle);
             return;
         }
-        let dt = cmp::max(Duration::from_millis(self.idle_timeout), 3 * self.pto());
+        let dt = cmp::max(timeout, 3 * self.pto());
         self.timers.set(Timer::Idle, now + dt);
     }
 
     fn reset_keep_alive(&mut self, now: Instant) {
-        if self.config.keep_alive_interval == 0 || !self.state.is_established() {
-            return;
-        }
-        self.timers.set(
-            Timer::KeepAlive,
-            now + Duration::from_millis(u64::from(self.config.keep_alive_interval)),
-        );
+        let interval = match self.config.keep_alive_interval {
+            Some(x) if self.state.is_established() => x,
+            _ => return,
+        };
+        self.timers.set(Timer::KeepAlive, now + interval);
     }
 
     fn queue_stream_data(&mut self, stream: StreamId, data: Bytes) -> Result<(), WriteError> {
@@ -2041,10 +2038,7 @@ where
         // Initiate path validation
         self.timers.set(
             Timer::PathValidation,
-            now + 3 * cmp::max(
-                self.pto(),
-                Duration::from_micros(2 * self.config.initial_rtt),
-            ),
+            now + 3 * cmp::max(self.pto(), 2 * self.config.initial_rtt),
         );
         self.path_challenge = Some(self.rng.gen());
         self.path_challenge_pending = true;
@@ -2650,10 +2644,11 @@ where
             self.streams.send_mut(id).unwrap().max_data =
                 params.initial_max_stream_data_bidi_local as u64;
         }
-        self.idle_timeout = if self.config.idle_timeout == 0 || params.idle_timeout == 0 {
-            cmp::max(self.config.idle_timeout, params.idle_timeout)
-        } else {
-            cmp::min(self.config.idle_timeout, params.idle_timeout)
+        self.idle_timeout = match (self.config.idle_timeout, params.idle_timeout) {
+            (None, 0) => None,
+            (None, x) => Some(Duration::from_millis(x)),
+            (Some(x), 0) => Some(x),
+            (Some(x), y) => Some(cmp::min(x, Duration::from_millis(y))),
         };
         self.params = params;
     }

quinn-proto/src/crypto/ring.rs

@@ -262,7 +262,7 @@ impl crypto::HmacKey for hmac::Key {
         if key.len() == Self::KEY_LEN {
             Ok(hmac::Key::new(hmac::HMAC_SHA256, key))
         } else {
-            Err(ConfigError::IllegalValue("key length must be 64 bytes"))
+            Err(ConfigError::OutOfBounds)
         }
     }
 

quinn-proto/src/endpoint.rs

@@ -16,9 +16,7 @@ use tracing::{debug, trace, warn};
 use crate::{
     coding::BufMutExt,
     connection::{initial_close, Connection, ConnectionError},
-    crypto::{
-        self, ClientConfig as ClientCryptoConfig, HmacKey, Keys, ServerConfig as ServerCryptoConfig,
-    },
+    crypto::{self, ClientConfig as ClientCryptoConfig, Keys, ServerConfig as ServerCryptoConfig},
     packet::{Header, Packet, PacketDecodeError, PartialDecode},
     shared::{
         ClientConfig, ConfigError, ConnectionEvent, ConnectionEventInner, ConnectionId,
@@ -51,15 +49,13 @@ where
     /// recipient, if any.
     connection_reset_tokens: ResetTokenTable,
     connections: Slab<ConnectionMeta>,
-    config: Arc<EndpointConfig>,
+    config: Arc<EndpointConfig<S>>,
     server_config: Option<Arc<ServerConfig<S>>>,
     incoming_handshakes: usize,
     /// Whether incoming connections should be unconditionally rejected by a server
     ///
     /// Equivalent to a `ServerConfig.accept_buffer` of `0`, but can be changed after the endpoint is constructed.
     reject_new_connections: bool,
-    reset_key: S::HmacKey,
-    token_key: Option<S::HmacKey>, // only available when server_config.is_some()
 }
 
 impl<S> Endpoint<S>
@@ -70,11 +66,10 @@ where
     ///
     /// Returns `Err` if the configuration is invalid.
     pub fn new(
-        config: Arc<EndpointConfig>,
+        config: Arc<EndpointConfig<S>>,
         server_config: Option<Arc<ServerConfig<S>>>,
-    ) -> Result<Self, ConfigError> {
-        config.validate()?;
-        Ok(Self {
+    ) -> Self {
+        Self {
             rng: StdRng::from_entropy(),
             transmits: VecDeque::new(),
             connection_ids_initial: HashMap::new(),
@@ -84,14 +79,9 @@ where
             connections: Slab::new(),
             incoming_handshakes: 0,
             reject_new_connections: false,
-            reset_key: S::HmacKey::new(&config.reset_key)?,
-            token_key: server_config
-                .as_ref()
-                .map(|c| S::HmacKey::new(&c.token_key))
-                .transpose()?,
             config,
             server_config,
-        })
+        }
     }
 
     fn is_server(&self) -> bool {
@@ -333,7 +323,7 @@ where
         buf.resize(padding_len, 0);
         self.rng.fill_bytes(&mut buf[0..padding_len]);
         buf[0] = 0b0100_0000 | buf[0] >> 2;
-        buf.extend_from_slice(&reset_token_for(&self.reset_key, dst_cid));
+        buf.extend_from_slice(&reset_token_for(&*self.config.reset_key, dst_cid));
 
         debug_assert!(buf.len() < inciting_dgram_len);
 
@@ -354,7 +344,6 @@ where
         if self.is_full() {
             return Err(ConnectError::TooManyConnections);
         }
-        config.transport.validate()?;
         let remote_id = ConnectionId::random(&mut self.rng, MAX_CID_SIZE);
         trace!(initial_dcid = %remote_id);
         let (ch, conn) = self.add_connection(
@@ -382,7 +371,7 @@ where
             ids.push(IssuedCid {
                 sequence,
                 id,
-                reset_token: reset_token_for(&self.reset_key, &id),
+                reset_token: reset_token_for(&*self.config.reset_key, &id),
             });
         }
         ConnectionEvent(ConnectionEventInner::NewIdentifiers(ids))
@@ -423,7 +412,7 @@ where
                 let config = self.server_config.as_ref().unwrap();
                 let params = TransportParameters::new(&config.transport, Some(config));
                 let server_params = TransportParameters {
-                    stateless_reset_token: Some(reset_token_for(&self.reset_key, &loc_cid)),
+                    stateless_reset_token: Some(reset_token_for(&*self.config.reset_key, &loc_cid)),
                     original_connection_id: orig_dst_cid,
                     ..params
                 };
@@ -550,7 +539,7 @@ where
         let mut retry_cid = None;
         if server_config.use_stateless_retry {
             if let Some((token_dst_cid, token_issued)) =
-                token::check(self.token_key.as_ref().unwrap(), &remote, &token)
+                token::check(&*server_config.token_key, &remote, &token)
             {
                 let expires = token_issued
                     + Duration::from_micros(
@@ -566,7 +555,7 @@ where
             }
             if retry_cid.is_none() {
                 let token = token::generate(
-                    self.token_key.as_ref().unwrap(),
+                    &*server_config.token_key,
                     &remote,
                     &dst_cid,
                     SystemTime::now(),

quinn-proto/src/lib.rs

@@ -48,7 +48,7 @@ pub use crate::endpoint::{ConnectError, ConnectionHandle, DatagramEvent};
 
 mod shared;
 pub use crate::shared::{
-    ConfigError, ConnectionEvent, EcnCodepoint, EndpointConfig, EndpointEvent, TransportConfig,
+    ConfigError, ConnectionEvent, EcnCodepoint, EndpointEvent, TransportConfig,
 };
 
 mod streams;
@@ -62,7 +62,7 @@ pub mod generic {
     pub use crate::{
         connection::Connection,
         endpoint::Endpoint,
-        shared::{ClientConfig, ServerConfig},
+        shared::{ClientConfig, EndpointConfig, ServerConfig},
     };
 }
 
@@ -78,6 +78,8 @@ mod rustls_impls {
     pub type Endpoint = generic::Endpoint<crypto::rustls::TlsSession>;
     /// A `ServerConfig` containing server-side rustls configuration
     pub type ServerConfig = generic::ServerConfig<crypto::rustls::TlsSession>;
+    /// A `EndpointConfig` using rustls keys
+    pub type EndpointConfig = generic::EndpointConfig<crypto::rustls::TlsSession>;
 }
 
 #[cfg(feature = "rustls")]