diff --git a/examples/rustls-mtls/src/lib.rs b/examples/rustls-mtls/src/lib.rs index 947ae74456..97790ae199 100644 --- a/examples/rustls-mtls/src/lib.rs +++ b/examples/rustls-mtls/src/lib.rs @@ -4,7 +4,9 @@ use rustls::{ cipher_suite, ClientConfig, Error, RootCertStore, ServerConfig, SupportedCipherSuite, }; -use s2n_quic::provider::{tls, tls::rustls::rustls}; +use s2n_quic::provider::tls; +#[allow(deprecated)] +use s2n_quic::provider::tls::rustls::rustls; use std::{io::Cursor, path::Path, sync::Arc}; use tokio::{fs::File, io::AsyncReadExt}; use tracing::Level; diff --git a/quic/s2n-quic-qns/src/tls.rs b/quic/s2n-quic-qns/src/tls.rs index d81d198a8d..2ec8b184aa 100644 --- a/quic/s2n-quic-qns/src/tls.rs +++ b/quic/s2n-quic-qns/src/tls.rs @@ -116,8 +116,10 @@ impl Client { use ::rustls::{version, ClientConfig, KeyLogFile}; use std::sync::Arc; + #[allow(deprecated)] + let cipher_suites = rustls::DEFAULT_CIPHERSUITES; let mut config = ClientConfig::builder() - .with_cipher_suites(rustls::DEFAULT_CIPHERSUITES) + .with_cipher_suites(cipher_suites) .with_safe_default_kx_groups() .with_protocol_versions(&[&version::TLS13])? .with_custom_certificate_verifier(Arc::new(rustls::DisabledVerifier)) @@ -125,6 +127,7 @@ impl Client { config.max_fragment_size = None; config.alpn_protocols = alpns.iter().map(|p| p.as_bytes().to_vec()).collect(); config.key_log = Arc::new(KeyLogFile::new()); + #[allow(deprecated)] rustls::Client::new(config) } else { rustls::Client::builder() @@ -265,9 +268,11 @@ pub mod s2n_tls { pub mod rustls { use super::*; + #[allow(deprecated)] + pub use s2n_quic::provider::tls::rustls::DEFAULT_CIPHERSUITES; pub use s2n_quic::provider::tls::rustls::{ certificate::{Certificate, IntoCertificate, IntoPrivateKey, PrivateKey}, - Client, Server, DEFAULT_CIPHERSUITES, + Client, Server, }; pub fn ca(ca: Option<&PathBuf>) -> Result { diff --git a/quic/s2n-quic-rustls/src/certificate.rs b/quic/s2n-quic-rustls/src/certificate.rs index 47161d9690..abb7accbf2 100644 --- a/quic/s2n-quic-rustls/src/certificate.rs +++ b/quic/s2n-quic-rustls/src/certificate.rs @@ -1,9 +1,7 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -#![allow(dead_code)] - -use rustls::Error; +use crate::Error; macro_rules! cert_type { ($name:ident, $trait:ident, $method:ident, $inner:ty) => { @@ -57,13 +55,11 @@ macro_rules! cert_type { fn $method(self) -> Result<$name, Error> { match self.extension() { Some(ext) if ext == "der" => { - let pem = - std::fs::read(self).map_err(|err| Error::General(err.to_string()))?; + let pem = std::fs::read(self)?; pem.$method() } _ => { - let pem = std::fs::read_to_string(self) - .map_err(|err| Error::General(err.to_string()))?; + let pem = std::fs::read_to_string(self)?; pem.$method() } } @@ -91,8 +87,7 @@ mod pem { pub fn into_certificate(contents: &[u8]) -> Result, Error> { let mut cursor = std::io::Cursor::new(contents); let certs = rustls_pemfile::certs(&mut cursor) - .map(|certs| certs.into_iter().map(rustls::Certificate).collect()) - .map_err(|_| Error::General("Could not read certificate".to_string()))?; + .map(|certs| certs.into_iter().map(rustls::Certificate).collect())?; Ok(certs) } @@ -114,19 +109,18 @@ mod pem { return Ok(rustls::PrivateKey(keys.pop().unwrap())) } Ok(keys) => { - return Err(Error::General(format!( + return Err(rustls::Error::General(format!( "Unexpected number of keys: {} (only 1 supported)", keys.len() - ))); + )) + .into()); } // try the next parser Err(_) => continue, } } - Err(Error::General( - "could not load any valid private keys".to_string(), - )) + Err(rustls::Error::General("could not load any valid private keys".to_string()).into()) } } diff --git a/quic/s2n-quic-rustls/src/client.rs b/quic/s2n-quic-rustls/src/client.rs index a5dc35c541..3a7f08771b 100644 --- a/quic/s2n-quic-rustls/src/client.rs +++ b/quic/s2n-quic-rustls/src/client.rs @@ -1,7 +1,7 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -use crate::{certificate, session::Session}; +use crate::{certificate, session::Session, Error}; use core::convert::TryFrom; use rustls::ClientConfig; use s2n_codec::EncoderValue; @@ -14,6 +14,7 @@ pub struct Client { } impl Client { + #[deprecated = "client and server builders should be used instead"] pub fn new(config: ClientConfig) -> Self { Self { config: Arc::new(config), @@ -34,12 +35,14 @@ impl Default for Client { } } +// TODO this should be removed after removing deprecated re-exports impl From for Client { fn from(config: ClientConfig) -> Self { - Self::new(config) + Self::from(Arc::new(config)) } } +// TODO this should be removed after removing deprecated re-exports impl From> for Client { fn from(config: Arc) -> Self { Self { config } @@ -108,7 +111,7 @@ impl Builder { pub fn with_certificate( mut self, certificate: C, - ) -> Result { + ) -> Result { let certificates = certificate.into_certificate()?; let root_certificate = certificates.0.first().ok_or_else(|| { rustls::Error::General("Certificate chain needs to have at least one entry".to_string()) @@ -119,7 +122,7 @@ impl Builder { Ok(self) } - pub fn with_max_cert_chain_depth(self, len: u16) -> Result { + pub fn with_max_cert_chain_depth(self, len: u16) -> Result { // TODO is there a way to configure this? let _ = len; Ok(self) @@ -133,19 +136,19 @@ impl Builder { Ok(self) } - pub fn with_key_logging(mut self) -> Result { + pub fn with_key_logging(mut self) -> Result { self.key_log = Some(Arc::new(rustls::KeyLogFile::new())); Ok(self) } - pub fn build(self) -> Result { + pub fn build(self) -> Result { // TODO load system root store? if self.cert_store.is_empty() { //= https://www.rfc-editor.org/rfc/rfc9001#section-4.4 //# A client MUST authenticate the identity of the server. - return Err(rustls::Error::General( - "missing trusted root certificate(s)".to_string(), - )); + return Err( + rustls::Error::General("missing trusted root certificate(s)".to_string()).into(), + ); } let mut config = ClientConfig::builder() @@ -162,6 +165,7 @@ impl Builder { config.key_log = key_log; } + #[allow(deprecated)] Ok(Client::new(config)) } } diff --git a/quic/s2n-quic-rustls/src/lib.rs b/quic/s2n-quic-rustls/src/lib.rs index 8cc0d6886d..ba9ece96bb 100644 --- a/quic/s2n-quic-rustls/src/lib.rs +++ b/quic/s2n-quic-rustls/src/lib.rs @@ -3,7 +3,17 @@ #![forbid(unsafe_code)] -pub use rustls::{self, Certificate, PrivateKey}; +/// *WARNING*: These are deprecated and should not be used. +#[deprecated = "client and server builders should be used instead"] +pub use ::rustls::{Certificate, PrivateKey}; + +#[deprecated = "client and server builders should be used instead"] +pub mod rustls { + pub use ::rustls::*; +} + +/// Wrap error types in Box to avoid leaking rustls types +type Error = Box; mod cipher_suite; mod error; @@ -13,7 +23,10 @@ pub mod certificate; pub mod client; pub mod server; -pub use cipher_suite::DEFAULT_CIPHERSUITES; +#[deprecated = "client and server builders should be used instead"] +pub static DEFAULT_CIPHERSUITES: &[rustls::SupportedCipherSuite] = + cipher_suite::DEFAULT_CIPHERSUITES; + pub use client::Client; pub use server::Server; diff --git a/quic/s2n-quic-rustls/src/server.rs b/quic/s2n-quic-rustls/src/server.rs index 19ec1b9dc2..4bcb6c7bb7 100644 --- a/quic/s2n-quic-rustls/src/server.rs +++ b/quic/s2n-quic-rustls/src/server.rs @@ -1,7 +1,7 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -use crate::{certificate, session::Session}; +use crate::{certificate, session::Session, Error}; use rustls::ServerConfig; use s2n_codec::EncoderValue; use s2n_quic_core::{application::ServerName, crypto::tls}; @@ -13,6 +13,7 @@ pub struct Server { } impl Server { + #[deprecated = "client and server builders should be used instead"] pub fn new(config: ServerConfig) -> Self { Self { config: Arc::new(config), @@ -32,12 +33,14 @@ impl Default for Server { } } +// TODO this should be removed after removing deprecated re-exports impl From for Server { fn from(config: ServerConfig) -> Self { - Self::new(config) + Self::from(Arc::new(config)) } } +// TODO this should be removed after removing deprecated re-exports impl From> for Server { fn from(config: Arc) -> Self { Self { config } @@ -82,6 +85,7 @@ pub struct Builder { cert_resolver: Option>, application_protocols: Vec>, key_log: Option>, + prefer_server_cipher_suite_order: bool, } impl Default for Builder { @@ -96,25 +100,28 @@ impl Builder { cert_resolver: None, application_protocols: vec![b"h3".to_vec()], key_log: None, + prefer_server_cipher_suite_order: true, } } pub fn with_certificate( - self, + mut self, certificate: C, private_key: PK, - ) -> Result { + ) -> Result { let certificate = certificate.into_certificate()?; let private_key = private_key.into_private_key()?; let resolver = AlwaysResolvesChain::new(certificate, private_key)?; let resolver = Arc::new(resolver); - self.with_cert_resolver(resolver) + self.cert_resolver = Some(resolver); + Ok(self) } + #[deprecated = "client and server builders should be used instead"] pub fn with_cert_resolver( mut self, cert_resolver: Arc, - ) -> Result { + ) -> Result { self.cert_resolver = Some(cert_resolver); Ok(self) } @@ -122,17 +129,24 @@ impl Builder { pub fn with_application_protocols, I: AsRef<[u8]>>( mut self, protocols: P, - ) -> Result { + ) -> Result { self.application_protocols = protocols.map(|p| p.as_ref().to_vec()).collect(); Ok(self) } - pub fn with_key_logging(mut self) -> Result { + pub fn with_key_logging(mut self) -> Result { self.key_log = Some(Arc::new(rustls::KeyLogFile::new())); Ok(self) } - pub fn build(self) -> Result { + /// If enabled, the cipher suite order of the client is ignored, and the top cipher suite + /// in the server list that the client supports is chosen (default: true) + pub fn with_prefer_server_cipher_suite_order(mut self, enabled: bool) -> Result { + self.prefer_server_cipher_suite_order = enabled; + Ok(self) + } + + pub fn build(self) -> Result { let builder = ServerConfig::builder() .with_cipher_suites(crate::cipher_suite::DEFAULT_CIPHERSUITES) .with_safe_default_kx_groups() @@ -144,10 +158,11 @@ impl Builder { } else { return Err(rustls::Error::General( "Missing certificate or certificate resolver".to_string(), - )); + ) + .into()); }; - config.ignore_client_order = true; + config.ignore_client_order = self.prefer_server_cipher_suite_order; config.max_fragment_size = None; config.alpn_protocols = self.application_protocols; @@ -155,6 +170,7 @@ impl Builder { config.key_log = key_log; } + #[allow(deprecated)] Ok(Server::new(config)) } }