proto: validate ClientConfig crypto provider

Co-authored-by: gabrik <gabriele.baldoni@gmail.com>

bench/src/lib.rs

@@ -9,6 +9,7 @@ use std::{
 use anyhow::{Context, Result};
 use bytes::Bytes;
 use clap::Parser;
+use quinn::crypto::rustls::QuicClientConfig;
 use rustls::{
     pki_types::{CertificateDer, PrivateKeyDer},
     RootCertStore,
@@ -74,7 +75,7 @@ pub async fn connect_client(
         .with_root_certificates(roots)
         .with_no_client_auth();
 
-    let mut client_config = quinn::ClientConfig::new(Arc::new(crypto));
+    let mut client_config = quinn::ClientConfig::new(Arc::new(QuicClientConfig::try_from(crypto)?));
     client_config.transport_config(Arc::new(transport_config(&opt)));
 
     let connection = endpoint

perf/src/bin/perf_client.rs

@@ -7,7 +7,7 @@ use std::{
 use anyhow::{Context, Result};
 use bytes::Bytes;
 use clap::Parser;
-use quinn::TokioRuntime;
+use quinn::{crypto::rustls::QuicClientConfig, TokioRuntime};
 use rustls::pki_types::{CertificateDer, ServerName, UnixTime};
 use tokio::sync::Semaphore;
 use tracing::{debug, error, info};
@@ -137,7 +137,7 @@ async fn run(opt: Opt) -> Result<()> {
     let mut transport = quinn::TransportConfig::default();
     transport.initial_mtu(opt.initial_mtu);
 
-    let crypto = Arc::new(crypto);
+    let crypto = Arc::new(QuicClientConfig::try_from(crypto)?);
     let mut config = quinn::ClientConfig::new(match opt.no_protection {
         true => Arc::new(NoProtectionClientConfig::new(crypto)),
         false => crypto,

perf/src/noprotection.rs

@@ -3,7 +3,7 @@ use std::sync::Arc;
 use bytes::BytesMut;
 
 use quinn_proto::{
-    crypto::{self, CryptoError},
+    crypto::{self, rustls::QuicClientConfig, CryptoError},
     transport_parameters, ConnectionId, Side, TransportError,
 };
 
@@ -39,11 +39,11 @@ impl NoProtectionPacketKey {
 }
 
 pub struct NoProtectionClientConfig {
-    inner: Arc<rustls::ClientConfig>,
+    inner: Arc<QuicClientConfig>,
 }
 
 impl NoProtectionClientConfig {
-    pub fn new(config: Arc<rustls::ClientConfig>) -> Self {
+    pub fn new(config: Arc<QuicClientConfig>) -> Self {
         Self { inner: config }
     }
 }

quinn-proto/src/config.rs

@@ -927,7 +927,7 @@ impl ClientConfig {
     /// Create a client configuration that trusts the platform's native roots
     #[cfg(feature = "platform-verifier")]
     pub fn with_platform_verifier() -> Self {
-        Self::new(Arc::new(crypto::rustls::client_config(Arc::new(
+        Self::new(Arc::new(crypto::rustls::QuicClientConfig::new(Arc::new(
             rustls_platform_verifier::Verifier::new(),
         ))))
     }
@@ -936,7 +936,7 @@ impl ClientConfig {
     pub fn with_root_certificates(
         roots: Arc<rustls::RootCertStore>,
     ) -> Result<Self, rustls::client::VerifierBuilderError> {
-        Ok(Self::new(Arc::new(crypto::rustls::client_config(
+        Ok(Self::new(Arc::new(crypto::rustls::QuicClientConfig::new(
             WebPkiServerVerifier::builder(roots).build()?,
         ))))
     }

quinn-proto/src/crypto/rustls.rs

@@ -254,23 +254,64 @@ pub struct HandshakeData {
     pub server_name: Option<String>,
 }
 
-impl crypto::ClientConfig for rustls::ClientConfig {
+/// A QUIC-compatible TLS client configuration
+///
+/// Can be constructed via [`ClientConfig::with_root_certificates()`][root_certs],
+/// [`ClientConfig::with_platform_verifier()`][platform] or by using the [`TryFrom`] implementation with a
+/// custom [`rustls::ClientConfig`].
+///
+/// [root_certs]: crate::config::ClientConfig::with_root_certificates()
+/// [platform]: crate::config::ClientConfig::with_platform_verifier()
+pub struct QuicClientConfig {
+    pub(crate) inner: Arc<rustls::ClientConfig>,
+    initial: Suite,
+}
+
+impl QuicClientConfig {
+    /// Initialize a sane QUIC-compatible TLS client configuration
+    ///
+    /// QUIC requires that TLS 1.3 be enabled. Advanced users can use any [`rustls::ClientConfig`] that
+    /// satisfies this requirement.
+    pub(crate) fn new(verifier: Arc<dyn ServerCertVerifier>) -> Self {
+        let inner = Self::inner(verifier);
+        Self {
+            // We're confident that the *ring* default provider contains TLS13_AES_128_GCM_SHA256
+            initial: initial_suite_from_provider(inner.crypto_provider())
+                .expect("no initial cipher suite found"),
+            inner: Arc::new(inner),
+        }
+    }
+
+    pub(crate) fn inner(verifier: Arc<dyn ServerCertVerifier>) -> rustls::ClientConfig {
+        let mut config = rustls::ClientConfig::builder_with_provider(
+            rustls::crypto::ring::default_provider().into(),
+        )
+        .with_protocol_versions(&[&rustls::version::TLS13])
+        .unwrap() // The *ring* default provider supports TLS 1.3
+        .dangerous()
+        .with_custom_certificate_verifier(verifier)
+        .with_no_client_auth();
+
+        config.enable_early_data = true;
+        config
+    }
+}
+
+impl crypto::ClientConfig for QuicClientConfig {
     fn start_session(
         self: Arc<Self>,
         version: u32,
         server_name: &str,
         params: &TransportParameters,
     ) -> Result<Box<dyn crypto::Session>, ConnectError> {
         let version = interpret_version(version)?;
-        // TODO: validate the configuration before this point
-        let suite = suite_from_provider(self.crypto_provider()).unwrap();
         Ok(Box::new(TlsSession {
             version,
             got_handshake_data: false,
             next_secrets: None,
             inner: rustls::quic::Connection::Client(
                 rustls::quic::ClientConnection::new(
-                    self,
+                    self.inner.clone(),
                     version,
                     ServerName::try_from(server_name)
                         .map_err(|_| ConnectError::InvalidServerName(server_name.into()))?
@@ -279,11 +320,38 @@ impl crypto::ClientConfig for rustls::ClientConfig {
                 )
                 .unwrap(),
             ),
-            suite,
+            suite: Suite {
+                suite: self.initial.suite,
+                quic: self.initial.quic,
+            },
         }))
     }
 }
 
+impl TryFrom<rustls::ClientConfig> for QuicClientConfig {
+    type Error = NoInitialCipherSuite;
+
+    fn try_from(inner: rustls::ClientConfig) -> Result<Self, Self::Error> {
+        Ok(Self {
+            initial: initial_suite_from_provider(inner.crypto_provider())
+                .ok_or(NoInitialCipherSuite(()))?,
+            inner: Arc::new(inner),
+        })
+    }
+}
+
+/// The configured crypto provider does not support the QUIC initial cipher suite
+#[derive(Debug)]
+pub struct NoInitialCipherSuite(());
+
+impl std::fmt::Display for NoInitialCipherSuite {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        f.write_str("no initial cipher suite found")
+    }
+}
+
+impl std::error::Error for NoInitialCipherSuite {}
+
 impl crypto::ServerConfig for rustls::ServerConfig {
     fn start_session(
         self: Arc<Self>,
@@ -293,7 +361,7 @@ impl crypto::ServerConfig for rustls::ServerConfig {
         // Safe: `start_session()` is never called if `initial_keys()` rejected `version`
         let version = interpret_version(version).unwrap();
         // TODO: validate the configuration before this point
-        let suite = suite_from_provider(self.crypto_provider()).unwrap();
+        let suite = initial_suite_from_provider(self.crypto_provider()).unwrap();
         Box::new(TlsSession {
             version,
             got_handshake_data: false,
@@ -312,7 +380,7 @@ impl crypto::ServerConfig for rustls::ServerConfig {
     ) -> Result<Keys, UnsupportedVersion> {
         let version = interpret_version(version)?;
         // TODO: validate the configuration before this point
-        let suite = suite_from_provider(self.crypto_provider()).unwrap();
+        let suite = initial_suite_from_provider(self.crypto_provider()).unwrap();
         Ok(initial_keys(version, dst_cid, Side::Server, &suite))
     }
 
@@ -342,7 +410,9 @@ impl crypto::ServerConfig for rustls::ServerConfig {
     }
 }
 
-pub(crate) fn suite_from_provider(provider: &Arc<rustls::crypto::CryptoProvider>) -> Option<Suite> {
+pub(crate) fn initial_suite_from_provider(
+    provider: &Arc<rustls::crypto::CryptoProvider>,
+) -> Option<Suite> {
     provider
         .cipher_suites
         .iter()
@@ -415,24 +485,6 @@ impl crypto::PacketKey for Box<dyn PacketKey> {
     }
 }
 
-/// Initialize a sane QUIC-compatible TLS client configuration
-///
-/// QUIC requires that TLS 1.3 be enabled. Advanced users can use any [`rustls::ClientConfig`] that
-/// satisfies this requirement.
-pub(crate) fn client_config(verifier: Arc<dyn ServerCertVerifier>) -> rustls::ClientConfig {
-    let mut config = rustls::ClientConfig::builder_with_provider(
-        rustls::crypto::ring::default_provider().into(),
-    )
-    .with_protocol_versions(&[&rustls::version::TLS13])
-    .unwrap() // The *ring* default provider supports TLS 1.3
-    .dangerous()
-    .with_custom_certificate_verifier(verifier)
-    .with_no_client_auth();
-
-    config.enable_early_data = true;
-    config
-}
-
 /// Initialize a sane QUIC-compatible TLS server configuration
 ///
 /// QUIC requires that TLS 1.3 be enabled, and that the maximum early data size is either 0 or

quinn-proto/src/packet.rs

@@ -868,15 +868,15 @@ mod tests {
     #[cfg(feature = "rustls")]
     #[test]
     fn header_encoding() {
-        use crate::crypto::rustls::{initial_keys, suite_from_provider};
+        use crate::crypto::rustls::{initial_keys, initial_suite_from_provider};
         use crate::Side;
         use rustls::crypto::ring::default_provider;
         use rustls::quic::Version;
 
         let dcid = ConnectionId::new(&hex!("06b858ec6f80452b"));
         let provider = default_provider();
 
-        let suite = suite_from_provider(&std::sync::Arc::new(provider)).unwrap();
+        let suite = initial_suite_from_provider(&std::sync::Arc::new(provider)).unwrap();
         let client = initial_keys(Version::V1, &dcid, Side::Client, &suite);
         let mut buf = BytesMut::new();
         let header = Header::Initial(InitialHeader {

quinn-proto/src/tests/mod.rs

@@ -597,8 +597,8 @@ fn zero_rtt_rejection() {
         "bar".into(),
     ])));
     let mut pair = Pair::new(Arc::new(EndpointConfig::default()), server_config);
-    let mut client_crypto = client_crypto_with_alpn(vec!["foo".into()]);
-    let client_config = ClientConfig::new(Arc::new(client_crypto.clone()));
+    let mut client_crypto = Arc::new(client_crypto_with_alpn(vec!["foo".into()]));
+    let client_config = ClientConfig::new(client_crypto.clone());
 
     // Establish normal connection
     let client_ch = pair.begin_connect(client_config);
@@ -627,9 +627,15 @@ fn zero_rtt_rejection() {
     pair.client.connections.clear();
     pair.server.connections.clear();
 
+    // We want to have a TLS client config with the existing session cache (so resumption could
+    // happen), but with different ALPN protocols (so that the server must reject it). Reuse
+    // the existing `ClientConfig` and change the ALPN protocols to make that happen.
+    let this = Arc::get_mut(&mut client_crypto).expect("QuicClientConfig is shared");
+    let inner = Arc::get_mut(&mut this.inner).expect("QuicClientConfig.inner is shared");
+    inner.alpn_protocols = vec!["bar".into()];
+
     // Changing protocols invalidates 0-RTT
-    client_crypto.alpn_protocols = vec!["bar".into()];
-    let client_config = ClientConfig::new(Arc::new(client_crypto));
+    let client_config = ClientConfig::new(client_crypto);
     info!("resuming session");
     let client_ch = pair.begin_connect(client_config);
     assert!(pair.client_conn_mut(client_ch).has_0rtt());

quinn-proto/src/tests/util.rs

@@ -21,6 +21,7 @@ use rustls::{
 };
 use tracing::{info_span, trace};
 
+use super::crypto::rustls::QuicClientConfig;
 use super::*;
 
 pub(super) const DEFAULT_MTU: usize = 1452;
@@ -610,34 +611,34 @@ pub(super) fn client_config_with_certs(certs: Vec<CertificateDer<'static>>) -> C
     ClientConfig::new(Arc::new(client_crypto_inner(Some(certs), None)))
 }
 
-pub(super) fn client_crypto() -> rustls::ClientConfig {
+pub(super) fn client_crypto() -> QuicClientConfig {
     client_crypto_inner(None, None)
 }
 
-pub(super) fn client_crypto_with_alpn(protocols: Vec<Vec<u8>>) -> rustls::ClientConfig {
+pub(super) fn client_crypto_with_alpn(protocols: Vec<Vec<u8>>) -> QuicClientConfig {
     client_crypto_inner(None, Some(protocols))
 }
 
 fn client_crypto_inner(
     certs: Option<Vec<CertificateDer<'static>>>,
     alpn: Option<Vec<Vec<u8>>>,
-) -> rustls::ClientConfig {
+) -> QuicClientConfig {
     let mut roots = rustls::RootCertStore::empty();
     for cert in certs.unwrap_or_else(|| vec![CERTIFIED_KEY.cert.der().clone()]) {
         roots.add(cert).unwrap();
     }
 
-    let mut config = crate::crypto::rustls::client_config(
+    let mut inner = QuicClientConfig::inner(
         WebPkiServerVerifier::builder(Arc::new(roots))
             .build()
             .unwrap(),
     );
-    config.key_log = Arc::new(KeyLogFile::new());
+    inner.key_log = Arc::new(KeyLogFile::new());
     if let Some(alpn) = alpn {
-        config.alpn_protocols = alpn;
+        inner.alpn_protocols = alpn;
     }
 
-    config
+    inner.try_into().unwrap()
 }
 
 pub(super) fn min_opt<T: Ord>(x: Option<T>, y: Option<T>) -> Option<T> {

quinn/examples/client.rs

@@ -13,6 +13,7 @@ use std::{
 
 use anyhow::{anyhow, Result};
 use clap::Parser;
+use proto::crypto::rustls::QuicClientConfig;
 use rustls::pki_types::CertificateDer;
 use tracing::{error, info};
 use url::Url;
@@ -100,7 +101,8 @@ async fn run(options: Opt) -> Result<()> {
         client_crypto.key_log = Arc::new(rustls::KeyLogFile::new());
     }
 
-    let client_config = quinn::ClientConfig::new(Arc::new(client_crypto));
+    let client_config =
+        quinn::ClientConfig::new(Arc::new(QuicClientConfig::try_from(client_crypto)?));
     let mut endpoint = quinn::Endpoint::client(options.bind)?;
     endpoint.set_default_client_config(client_config);
 

quinn/examples/insecure_connection.rs

@@ -4,6 +4,7 @@
 
 use std::{error::Error, net::SocketAddr, sync::Arc};
 
+use proto::crypto::rustls::QuicClientConfig;
 use quinn::{ClientConfig, Endpoint};
 use rustls::pki_types::{CertificateDer, ServerName, UnixTime};
 
@@ -35,12 +36,12 @@ async fn run_client(server_addr: SocketAddr) -> Result<(), Box<dyn Error + Send
     let provider = rustls::crypto::CryptoProvider::get_default().unwrap();
     let mut endpoint = Endpoint::client("127.0.0.1:0".parse().unwrap())?;
 
-    endpoint.set_default_client_config(ClientConfig::new(Arc::new(
+    endpoint.set_default_client_config(ClientConfig::new(Arc::new(QuicClientConfig::try_from(
         rustls::ClientConfig::builder()
             .dangerous()
             .with_custom_certificate_verifier(SkipServerVerification::new(provider.clone()))
             .with_no_client_auth(),
-    )));
+    )?)));
 
     // connect to server
     let connection = endpoint

quinn/examples/server.rs

@@ -94,7 +94,7 @@ async fn run(options: Opt) -> Result<()> {
         let (cert, key) = match fs::read(&cert_path).and_then(|x| Ok((x, fs::read(&key_path)?))) {
             Ok((cert, key)) => (
                 CertificateDer::from(cert),
-                PrivateKeyDer::try_from(key).map_err(|err| anyhow::Error::msg(err))?,
+                PrivateKeyDer::try_from(key).map_err(anyhow::Error::msg)?,
             ),
             Err(ref e) if e.kind() == io::ErrorKind::NotFound => {
                 info!("generating self-signed certificate");