From d60c9c282df1a6f940ca42ec60fbbc48df313fe0 Mon Sep 17 00:00:00 2001 From: Benjamin Saunders Date: Sun, 10 Oct 2021 14:37:00 -0700 Subject: [PATCH] Erase crypto::Session::HmacKey EndpointConfig::default now depends on ring --- quinn-proto/src/config.rs | 51 +++++++++---------------- quinn-proto/src/crypto.rs | 19 +++------ quinn-proto/src/crypto/ring.rs | 20 +++------- quinn-proto/src/crypto/rustls.rs | 3 +- quinn-proto/src/endpoint.rs | 9 ++--- quinn-proto/src/lib.rs | 6 +-- quinn-proto/src/tests/mod.rs | 4 +- quinn-proto/src/token.rs | 7 ++-- quinn-proto/src/transport_parameters.rs | 2 +- quinn/src/builders.rs | 9 +++-- 10 files changed, 47 insertions(+), 83 deletions(-) diff --git a/quinn-proto/src/config.rs b/quinn-proto/src/config.rs index 49a2346d0..361c53ab3 100644 --- a/quinn-proto/src/config.rs +++ b/quinn-proto/src/config.rs @@ -8,7 +8,7 @@ use crate::crypto::types::{Certificate, CertificateChain, PrivateKey}; use crate::{ cid_generator::{ConnectionIdGenerator, RandomConnectionIdGenerator}, congestion, - crypto::{self, HandshakeTokenKey as _, HmacKey as _}, + crypto::{self, HandshakeTokenKey as _, HmacKey}, VarInt, VarIntBoundsExceeded, DEFAULT_SUPPORTED_VERSIONS, }; @@ -300,11 +300,9 @@ impl fmt::Debug for TransportConfig { /// Global configuration for the endpoint, affecting all connections /// /// Default values should be suitable for most internet applications. -pub struct EndpointConfig -where - S: crypto::Session, -{ - pub(crate) reset_key: Arc, +#[derive(Clone)] +pub struct EndpointConfig { + pub(crate) reset_key: Arc, pub(crate) max_udp_payload_size: VarInt, /// CID generator factory /// @@ -315,16 +313,13 @@ where pub(crate) initial_version: u32, } -impl EndpointConfig -where - S: crypto::Session, -{ +impl EndpointConfig { /// Create a default config with a particular `reset_key` - pub fn new(reset_key: S::HmacKey) -> Self { + pub fn new(reset_key: Arc) -> Self { let cid_factory: fn() -> Box = || Box::new(RandomConnectionIdGenerator::default()); Self { - reset_key: Arc::new(reset_key), + reset_key, max_udp_payload_size: 1480u32.into(), // Typical internet MTU minus IPv4 and UDP overhead, rounded up to a multiple of 8 connection_id_generator_factory: Arc::new(cid_factory), initial_version: DEFAULT_SUPPORTED_VERSIONS[0], @@ -352,9 +347,9 @@ where /// Private key used to send authenticated connection resets to peers who were /// communicating with a previous instance of this endpoint. - pub fn reset_key(&mut self, value: &[u8]) -> Result<&mut Self, ConfigError> { - self.reset_key = Arc::new(S::HmacKey::new(value)?); - Ok(self) + pub fn reset_key(&mut self, key: Arc) -> &mut Self { + self.reset_key = key; + self } /// Maximum UDP payload size accepted from peers. Excludes UDP and IP overhead. @@ -394,7 +389,7 @@ where } } -impl fmt::Debug for EndpointConfig { +impl fmt::Debug for EndpointConfig { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { fmt.debug_struct("EndpointConfig") .field("reset_key", &"[ elided ]") @@ -406,26 +401,16 @@ impl fmt::Debug for EndpointConfig { } } -impl Default for EndpointConfig { +#[cfg(feature = "ring")] +impl Default for EndpointConfig { fn default() -> Self { - let mut reset_key = vec![0; S::HmacKey::KEY_LEN]; + let mut reset_key = [0; 64]; rand::thread_rng().fill_bytes(&mut reset_key); - Self::new( - S::HmacKey::new(&reset_key) - .expect("HMAC key rejected random bytes; use EndpointConfig::new instead"), - ) - } -} -impl Clone for EndpointConfig { - fn clone(&self) -> Self { - Self { - reset_key: self.reset_key.clone(), - max_udp_payload_size: self.max_udp_payload_size, - connection_id_generator_factory: self.connection_id_generator_factory.clone(), - supported_versions: self.supported_versions.clone(), - initial_version: self.initial_version, - } + Self::new(Arc::new(ring::hmac::Key::new( + ring::hmac::HMAC_SHA256, + &reset_key, + ))) } } diff --git a/quinn-proto/src/crypto.rs b/quinn-proto/src/crypto.rs index 452e700a5..68b83aa20 100644 --- a/quinn-proto/src/crypto.rs +++ b/quinn-proto/src/crypto.rs @@ -13,8 +13,8 @@ use std::{any::Any, str}; use bytes::BytesMut; use crate::{ - config::ConfigError, shared::ConnectionId, transport_parameters::TransportParameters, - ConnectError, Side, TransportError, + shared::ConnectionId, transport_parameters::TransportParameters, ConnectError, Side, + TransportError, }; /// Cryptography interface based on *ring* @@ -31,8 +31,6 @@ pub(crate) mod types; pub trait Session: Send + Sized { /// Type used to hold configuration for client sessions type ClientConfig: ClientConfig; - /// Type used to sign various values - type HmacKey: HmacKey; /// Key used to generate one-time-use handshake token keys type HandshakeTokenKey: HandshakeTokenKey; /// Type of keys used to protect packet headers @@ -183,16 +181,11 @@ pub trait HeaderKey: Send { } /// A key for signing with HMAC-based algorithms -pub trait HmacKey: Send + Sized + Sync { - /// Length of the key input - const KEY_LEN: usize; - /// Type of the signatures created by `sign()` - type Signature: AsRef<[u8]>; - - /// Method for creating a key - fn new(key: &[u8]) -> Result; +pub trait HmacKey: Send + Sync { /// Method for signing a message - fn sign(&self, data: &[u8]) -> Self::Signature; + fn sign(&self, data: &[u8], signature_out: &mut [u8]); + /// Length of `sign`'s output + fn signature_len(&self) -> usize; /// Method for verifying a message fn verify(&self, data: &[u8], signature: &[u8]) -> Result<(), CryptoError>; } diff --git a/quinn-proto/src/crypto/ring.rs b/quinn-proto/src/crypto/ring.rs index 6d830aab9..c3ed16a11 100644 --- a/quinn-proto/src/crypto/ring.rs +++ b/quinn-proto/src/crypto/ring.rs @@ -1,24 +1,14 @@ use ring::{aead, hkdf, hmac}; -use crate::{ - config::ConfigError, - crypto::{self, CryptoError}, -}; +use crate::crypto::{self, CryptoError}; impl crypto::HmacKey for hmac::Key { - const KEY_LEN: usize = 64; - type Signature = hmac::Tag; - - fn new(key: &[u8]) -> Result { - if key.len() == Self::KEY_LEN { - Ok(hmac::Key::new(hmac::HMAC_SHA256, key)) - } else { - Err(ConfigError::OutOfBounds) - } + fn sign(&self, data: &[u8], out: &mut [u8]) { + out.copy_from_slice(hmac::sign(self, data).as_ref()); } - fn sign(&self, data: &[u8]) -> Self::Signature { - hmac::sign(self, data) + fn signature_len(&self) -> usize { + 32 } fn verify(&self, data: &[u8], signature: &[u8]) -> Result<(), CryptoError> { diff --git a/quinn-proto/src/crypto/rustls.rs b/quinn-proto/src/crypto/rustls.rs index 238c020c7..f2c248453 100644 --- a/quinn-proto/src/crypto/rustls.rs +++ b/quinn-proto/src/crypto/rustls.rs @@ -1,7 +1,7 @@ use std::{any::Any, convert::TryInto, io, str, sync::Arc}; use bytes::BytesMut; -use ring::{aead, hkdf, hmac}; +use ring::{aead, hkdf}; pub use rustls::Error; use rustls::{ self, @@ -37,7 +37,6 @@ impl TlsSession { impl crypto::Session for TlsSession { type ClientConfig = Arc; - type HmacKey = hmac::Key; type HandshakeTokenKey = hkdf::Prk; type PacketKey = PacketKey; type HeaderKey = HeaderProtectionKey; diff --git a/quinn-proto/src/endpoint.rs b/quinn-proto/src/endpoint.rs index fb6163b43..5b2e0aa01 100644 --- a/quinn-proto/src/endpoint.rs +++ b/quinn-proto/src/endpoint.rs @@ -65,7 +65,7 @@ where connection_reset_tokens: ResetTokenTable, connections: Slab, local_cid_generator: Box, - config: Arc>, + config: Arc, server_config: Option>>, /// Whether incoming connections should be unconditionally rejected by a server /// @@ -80,10 +80,7 @@ where /// Create a new endpoint /// /// Returns `Err` if the configuration is invalid. - pub fn new( - config: Arc>, - server_config: Option>>, - ) -> Self { + pub fn new(config: Arc, server_config: Option>>) -> Self { Self { rng: StdRng::from_entropy(), transmits: VecDeque::new(), @@ -720,7 +717,7 @@ where } /// Access the configuration used by this endpoint - pub fn config(&self) -> &EndpointConfig { + pub fn config(&self) -> &EndpointConfig { &self.config } diff --git a/quinn-proto/src/lib.rs b/quinn-proto/src/lib.rs index 51187bf52..bc3273d97 100644 --- a/quinn-proto/src/lib.rs +++ b/quinn-proto/src/lib.rs @@ -46,7 +46,7 @@ pub use crate::connection::{ }; mod config; -pub use config::{ConfigError, IdleTimeout, TransportConfig}; +pub use config::{ConfigError, EndpointConfig, IdleTimeout, TransportConfig}; pub mod crypto; #[cfg(feature = "rustls")] @@ -76,7 +76,7 @@ use token::{ResetToken, RetryToken}; /// Types that are generic over the crypto protocol implementation pub mod generic { pub use crate::{ - config::{ClientConfig, EndpointConfig, ServerConfig}, + config::{ClientConfig, ServerConfig}, connection::{Connection, Datagrams}, endpoint::Endpoint, }; @@ -96,8 +96,6 @@ mod rustls_impls { pub type Endpoint = generic::Endpoint; /// A `ServerConfig` containing server-side rustls configuration pub type ServerConfig = generic::ServerConfig; - /// A `EndpointConfig` using rustls keys - pub type EndpointConfig = generic::EndpointConfig; } #[cfg(feature = "rustls")] diff --git a/quinn-proto/src/tests/mod.rs b/quinn-proto/src/tests/mod.rs index 01a686d38..0dbba6be5 100644 --- a/quinn-proto/src/tests/mod.rs +++ b/quinn-proto/src/tests/mod.rs @@ -137,7 +137,7 @@ fn server_stateless_reset() { rng.fill_bytes(&mut reset_key); let reset_key = hmac::Key::new(hmac::HMAC_SHA256, &reset_key); - let endpoint_config = Arc::new(EndpointConfig::new(reset_key)); + let endpoint_config = Arc::new(EndpointConfig::new(Arc::new(reset_key))); let mut pair = Pair::new(endpoint_config.clone(), server_config()); let (client_ch, _) = pair.connect(); @@ -166,7 +166,7 @@ fn client_stateless_reset() { rng.fill_bytes(&mut reset_key); let reset_key = hmac::Key::new(hmac::HMAC_SHA256, &reset_key); - let endpoint_config = Arc::new(EndpointConfig::new(reset_key)); + let endpoint_config = Arc::new(EndpointConfig::new(Arc::new(reset_key))); let mut pair = Pair::new(endpoint_config.clone(), server_config()); let (_, server_ch) = pair.connect(); diff --git a/quinn-proto/src/token.rs b/quinn-proto/src/token.rs index d05d88c0b..4ce1d9e31 100644 --- a/quinn-proto/src/token.rs +++ b/quinn-proto/src/token.rs @@ -111,11 +111,12 @@ impl<'a> RetryToken<'a> { pub struct ResetToken([u8; RESET_TOKEN_SIZE]); impl ResetToken { - pub(crate) fn new(key: &impl HmacKey, id: &ConnectionId) -> Self { - let signature = key.sign(id); + pub(crate) fn new(key: &dyn HmacKey, id: &ConnectionId) -> Self { + let mut signature = vec![0; key.signature_len()]; + key.sign(id, &mut signature); // TODO: Server ID?? let mut result = [0; RESET_TOKEN_SIZE]; - result.copy_from_slice(&signature.as_ref()[..RESET_TOKEN_SIZE]); + result.copy_from_slice(&signature[..RESET_TOKEN_SIZE]); result.into() } } diff --git a/quinn-proto/src/transport_parameters.rs b/quinn-proto/src/transport_parameters.rs index b7d1e0216..ada0e5269 100644 --- a/quinn-proto/src/transport_parameters.rs +++ b/quinn-proto/src/transport_parameters.rs @@ -117,7 +117,7 @@ apply_params!(make_struct); impl TransportParameters { pub(crate) fn new( config: &TransportConfig, - endpoint_config: &EndpointConfig, + endpoint_config: &EndpointConfig, cid_gen: &dyn ConnectionIdGenerator, initial_src_cid: ConnectionId, server_config: Option<&ServerConfig>, diff --git a/quinn/src/builders.rs b/quinn/src/builders.rs index 84ca894c0..917d85ffe 100644 --- a/quinn/src/builders.rs +++ b/quinn/src/builders.rs @@ -1,8 +1,8 @@ use std::{io, net::SocketAddr, sync::Arc}; use proto::{ - generic::{ClientConfig, EndpointConfig, ServerConfig}, - ConnectionIdGenerator, + generic::{ClientConfig, ServerConfig}, + ConnectionIdGenerator, EndpointConfig, }; use thiserror::Error; use tracing::error; @@ -22,7 +22,7 @@ where S: proto::crypto::Session, { server_config: Option>, - config: EndpointConfig, + config: EndpointConfig, default_client_config: Option>, } @@ -32,7 +32,7 @@ where S: proto::crypto::Session + Send + 'static, { /// Start a builder with a specific initial low-level configuration - pub fn new(config: EndpointConfig, default_client_config: Option>) -> Self { + pub fn new(config: EndpointConfig, default_client_config: Option>) -> Self { Self { server_config: None, config, @@ -112,6 +112,7 @@ where } } +#[cfg(feature = "tls-rustls")] impl Default for EndpointBuilder where S: proto::crypto::Session,