From acc6022ba7ca0a47a4b4c0c9d4682b3c5a73b6c3 Mon Sep 17 00:00:00 2001 From: Richard Patel Date: Wed, 4 Dec 2024 21:47:08 +0000 Subject: [PATCH] Only permit X25519 based QUIC-TLS key exchanges --- Cargo.toml | 2 +- core/src/repair/quic_endpoint.rs | 11 +++------- quic-client/src/nonblocking/quic_client.rs | 8 +++---- streamer/src/nonblocking/testing_utilities.rs | 6 ++---- streamer/src/quic.rs | 7 +++---- tls-utils/src/config.rs | 21 +++++++++++++++++++ tls-utils/src/crypto_provider.rs | 10 +++++++++ tls-utils/src/lib.rs | 6 ++++++ tls-utils/src/skip_client_verification.rs | 3 ++- tls-utils/src/skip_server_verification.rs | 5 +++-- tpu-client-next/src/quic_networking.rs | 6 ++---- turbine/src/quic_endpoint.rs | 11 +++------- 12 files changed, 60 insertions(+), 36 deletions(-) create mode 100644 tls-utils/src/config.rs create mode 100644 tls-utils/src/crypto_provider.rs diff --git a/Cargo.toml b/Cargo.toml index c62c59226c78d2..eaed0bf9115e6f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -397,7 +397,7 @@ reqwest = { version = "0.11.27", default-features = false } reqwest-middleware = "0.2.5" rolling-file = "0.2.0" rpassword = "7.3" -rustls = { version = "0.23.19", default-features = false } +rustls = { version = "0.23.19", features = ["std"], default-features = false } scopeguard = "1.2.0" semver = "1.0.23" seqlock = "0.2.0" diff --git a/core/src/repair/quic_endpoint.rs b/core/src/repair/quic_endpoint.rs index e8085c522c8c2a..0c8cc54dffe629 100644 --- a/core/src/repair/quic_endpoint.rs +++ b/core/src/repair/quic_endpoint.rs @@ -17,7 +17,7 @@ use { solana_runtime::bank_forks::BankForks, solana_sdk::{pubkey::Pubkey, signature::Keypair}, solana_tls_utils::{ - new_dummy_x509_certificate, SkipClientVerification, SkipServerVerification, + new_dummy_x509_certificate, tls_client_config_builder, tls_server_config_builder, }, std::{ cmp::Reverse, @@ -300,9 +300,7 @@ fn new_server_config( cert: CertificateDer<'static>, key: PrivateKeyDer<'static>, ) -> Result { - let mut config = rustls::ServerConfig::builder() - .with_client_cert_verifier(SkipClientVerification::new()) - .with_single_cert(vec![cert], key)?; + let mut config = tls_server_config_builder().with_single_cert(vec![cert], key)?; config.alpn_protocols = vec![ALPN_REPAIR_PROTOCOL_ID.to_vec()]; config.key_log = Arc::new(KeyLogFile::new()); let Ok(config) = QuicServerConfig::try_from(config) else { @@ -321,10 +319,7 @@ fn new_client_config( cert: CertificateDer<'static>, key: PrivateKeyDer<'static>, ) -> Result { - let mut config = rustls::ClientConfig::builder() - .dangerous() - .with_custom_certificate_verifier(SkipServerVerification::new()) - .with_client_auth_cert(vec![cert], key)?; + let mut config = tls_client_config_builder().with_client_auth_cert(vec![cert], key)?; config.enable_early_data = true; config.alpn_protocols = vec![ALPN_REPAIR_PROTOCOL_ID.to_vec()]; let mut config = ClientConfig::new(Arc::new(QuicClientConfig::try_from(config).unwrap())); diff --git a/quic-client/src/nonblocking/quic_client.rs b/quic-client/src/nonblocking/quic_client.rs index 2c9020bf846835..d8775aae7b4d32 100644 --- a/quic-client/src/nonblocking/quic_client.rs +++ b/quic-client/src/nonblocking/quic_client.rs @@ -23,7 +23,9 @@ use { }, solana_rpc_client_api::client_error::ErrorKind as ClientErrorKind, solana_streamer::nonblocking::quic::ALPN_TPU_PROTOCOL_ID, - solana_tls_utils::{new_dummy_x509_certificate, QuicClientCertificate, SkipServerVerification}, + solana_tls_utils::{ + new_dummy_x509_certificate, tls_client_config_builder, QuicClientCertificate, + }, solana_transaction_error::TransportResult, std::{ net::{IpAddr, Ipv4Addr, SocketAddr, UdpSocket}, @@ -85,9 +87,7 @@ impl QuicLazyInitializedEndpoint { QuicNewConnection::create_endpoint(EndpointConfig::default(), client_socket) }; - let mut crypto = rustls::ClientConfig::builder() - .dangerous() - .with_custom_certificate_verifier(SkipServerVerification::new()) + let mut crypto = tls_client_config_builder() .with_client_auth_cert( vec![self.client_certificate.certificate.clone()], self.client_certificate.key.clone_key(), diff --git a/streamer/src/nonblocking/testing_utilities.rs b/streamer/src/nonblocking/testing_utilities.rs index 80c758ca8957af..fc0c3a801784e0 100644 --- a/streamer/src/nonblocking/testing_utilities.rs +++ b/streamer/src/nonblocking/testing_utilities.rs @@ -21,7 +21,7 @@ use { solana_net_utils::bind_to_localhost, solana_perf::packet::PacketBatch, solana_quic_definitions::{QUIC_KEEP_ALIVE, QUIC_MAX_TIMEOUT}, - solana_tls_utils::{new_dummy_x509_certificate, SkipServerVerification}, + solana_tls_utils::{new_dummy_x509_certificate, tls_client_config_builder}, std::{ net::{SocketAddr, UdpSocket}, sync::{atomic::AtomicBool, Arc, RwLock}, @@ -32,9 +32,7 @@ use { pub fn get_client_config(keypair: &Keypair) -> ClientConfig { let (cert, key) = new_dummy_x509_certificate(keypair); - let mut crypto = rustls::ClientConfig::builder() - .dangerous() - .with_custom_certificate_verifier(SkipServerVerification::new()) + let mut crypto = tls_client_config_builder() .with_client_auth_cert(vec![cert], key) .expect("Failed to use client certificate"); diff --git a/streamer/src/quic.rs b/streamer/src/quic.rs index 1914861047dc43..85760f096f1932 100644 --- a/streamer/src/quic.rs +++ b/streamer/src/quic.rs @@ -19,7 +19,7 @@ use { solana_quic_definitions::{ NotifyKeyUpdate, QUIC_MAX_TIMEOUT, QUIC_MAX_UNSTAKED_CONCURRENT_STREAMS, }, - solana_tls_utils::{new_dummy_x509_certificate, SkipClientVerification}, + solana_tls_utils::{new_dummy_x509_certificate, tls_server_config_builder}, std::{ net::UdpSocket, sync::{ @@ -58,9 +58,8 @@ pub(crate) fn configure_server( }]; let cert_chain_pem = pem::encode_many(&cert_chain_pem_parts); - let mut server_tls_config = rustls::ServerConfig::builder() - .with_client_cert_verifier(SkipClientVerification::new()) - .with_single_cert(vec![cert], priv_key)?; + let mut server_tls_config = + tls_server_config_builder().with_single_cert(vec![cert], priv_key)?; server_tls_config.alpn_protocols = vec![ALPN_TPU_PROTOCOL_ID.to_vec()]; server_tls_config.key_log = Arc::new(KeyLogFile::new()); let quic_server_config = QuicServerConfig::try_from(server_tls_config)?; diff --git a/tls-utils/src/config.rs b/tls-utils/src/config.rs new file mode 100644 index 00000000000000..c0f038821547fd --- /dev/null +++ b/tls-utils/src/config.rs @@ -0,0 +1,21 @@ +use { + rustls::{ + client::WantsClientCert, server::WantsServerCert, ClientConfig, ConfigBuilder, ServerConfig, + }, + std::sync::Arc, +}; + +pub fn tls_client_config_builder() -> ConfigBuilder { + ClientConfig::builder_with_provider(Arc::new(crate::crypto_provider())) + .with_safe_default_protocol_versions() + .unwrap() + .dangerous() + .with_custom_certificate_verifier(crate::SkipServerVerification::new()) +} + +pub fn tls_server_config_builder() -> ConfigBuilder { + ServerConfig::builder_with_provider(Arc::new(crate::crypto_provider())) + .with_safe_default_protocol_versions() + .unwrap() + .with_client_cert_verifier(crate::SkipClientVerification::new()) +} diff --git a/tls-utils/src/crypto_provider.rs b/tls-utils/src/crypto_provider.rs new file mode 100644 index 00000000000000..1e1d754fda4de8 --- /dev/null +++ b/tls-utils/src/crypto_provider.rs @@ -0,0 +1,10 @@ +use rustls::{crypto::CryptoProvider, NamedGroup}; + +pub fn crypto_provider() -> CryptoProvider { + let mut provider = rustls::crypto::ring::default_provider(); + // Disable all key exchange algorithms except X25519 + provider + .kx_groups + .retain(|kx| kx.name() == NamedGroup::X25519); + provider +} diff --git a/tls-utils/src/lib.rs b/tls-utils/src/lib.rs index 2985d127fd379a..3f20152cc894a0 100644 --- a/tls-utils/src/lib.rs +++ b/tls-utils/src/lib.rs @@ -1,6 +1,12 @@ //! Collection of TLS related code fragments that end up popping up everywhere where quic is used. //! Aggregated here to avoid bugs due to conflicting implementations of the same functionality. +mod config; +pub use config::*; + +mod crypto_provider; +pub use crypto_provider::*; + mod tls_certificates; pub use tls_certificates::*; diff --git a/tls-utils/src/skip_client_verification.rs b/tls-utils/src/skip_client_verification.rs index 92be37ab8be657..17269ab0eff6f2 100644 --- a/tls-utils/src/skip_client_verification.rs +++ b/tls-utils/src/skip_client_verification.rs @@ -1,4 +1,5 @@ use { + crate::crypto_provider, rustls::{ pki_types::{CertificateDer, UnixTime}, server::danger::ClientCertVerified, @@ -14,7 +15,7 @@ pub struct SkipClientVerification(Arc); impl SkipClientVerification { pub fn new() -> Arc { - Arc::new(Self(Arc::new(rustls::crypto::ring::default_provider()))) + Arc::new(Self(Arc::new(crypto_provider()))) } } impl rustls::server::danger::ClientCertVerifier for SkipClientVerification { diff --git a/tls-utils/src/skip_server_verification.rs b/tls-utils/src/skip_server_verification.rs index cac5dd59410825..4fdef2c389679a 100644 --- a/tls-utils/src/skip_server_verification.rs +++ b/tls-utils/src/skip_server_verification.rs @@ -1,7 +1,8 @@ use { + crate::crypto_provider, rustls::{ client::danger::{HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier}, - crypto::{ring, verify_tls12_signature, verify_tls13_signature, CryptoProvider}, + crypto::{verify_tls12_signature, verify_tls13_signature, CryptoProvider}, pki_types::{CertificateDer, ServerName, UnixTime}, DigitallySignedStruct, Error, SignatureScheme, }, @@ -19,7 +20,7 @@ pub struct SkipServerVerification(Arc); impl SkipServerVerification { pub fn new() -> Arc { - Arc::new(Self(Arc::new(ring::default_provider()))) + Arc::new(Self(Arc::new(crypto_provider()))) } } diff --git a/tpu-client-next/src/quic_networking.rs b/tpu-client-next/src/quic_networking.rs index 65c3b07139f5f3..681c4a43138eb2 100644 --- a/tpu-client-next/src/quic_networking.rs +++ b/tpu-client-next/src/quic_networking.rs @@ -7,7 +7,7 @@ use { }, solana_sdk::quic::{QUIC_KEEP_ALIVE, QUIC_MAX_TIMEOUT}, solana_streamer::nonblocking::quic::ALPN_TPU_PROTOCOL_ID, - solana_tls_utils::SkipServerVerification, + solana_tls_utils::tls_client_config_builder, std::{net::SocketAddr, sync::Arc}, }; @@ -20,9 +20,7 @@ pub use { pub(crate) fn create_client_config(client_certificate: QuicClientCertificate) -> ClientConfig { // adapted from QuicLazyInitializedEndpoint::create_endpoint - let mut crypto = rustls::ClientConfig::builder() - .dangerous() - .with_custom_certificate_verifier(SkipServerVerification::new()) + let mut crypto = tls_client_config_builder() .with_client_auth_cert( vec![client_certificate.certificate.clone()], client_certificate.key.clone_key(), diff --git a/turbine/src/quic_endpoint.rs b/turbine/src/quic_endpoint.rs index 175663bfa9fe2a..59205201bab9b9 100644 --- a/turbine/src/quic_endpoint.rs +++ b/turbine/src/quic_endpoint.rs @@ -16,7 +16,7 @@ use { solana_runtime::bank_forks::BankForks, solana_sdk::{pubkey::Pubkey, signature::Keypair}, solana_tls_utils::{ - new_dummy_x509_certificate, SkipClientVerification, SkipServerVerification, + new_dummy_x509_certificate, tls_client_config_builder, tls_server_config_builder, }, std::{ cmp::Reverse, @@ -157,9 +157,7 @@ fn new_server_config( cert: CertificateDer<'static>, key: PrivateKeyDer<'static>, ) -> Result { - let mut config = rustls::ServerConfig::builder() - .with_client_cert_verifier(SkipClientVerification::new()) - .with_single_cert(vec![cert], key)?; + let mut config = tls_server_config_builder().with_single_cert(vec![cert], key)?; config.alpn_protocols = vec![ALPN_TURBINE_PROTOCOL_ID.to_vec()]; config.key_log = Arc::new(KeyLogFile::new()); let quic_server_config = QuicServerConfig::try_from(config) @@ -176,10 +174,7 @@ fn new_client_config( cert: CertificateDer<'static>, key: PrivateKeyDer<'static>, ) -> Result { - let mut config = rustls::ClientConfig::builder() - .dangerous() - .with_custom_certificate_verifier(SkipServerVerification::new()) - .with_client_auth_cert(vec![cert], key)?; + let mut config = tls_client_config_builder().with_client_auth_cert(vec![cert], key)?; config.enable_early_data = true; config.alpn_protocols = vec![ALPN_TURBINE_PROTOCOL_ID.to_vec()]; let mut config = ClientConfig::new(Arc::new(QuicClientConfig::try_from(config).unwrap()));