From 164c929b27a17fb3eab274e1d53245a1e3c697dc Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Thu, 12 Jan 2023 15:24:02 -0800 Subject: [PATCH] Cleanup QUIC single signed client cert code (#29686) --- client/src/connection_cache.rs | 16 +++++----- quic-client/src/lib.rs | 16 +++++----- quic-client/src/nonblocking/quic_client.rs | 17 +++++------ quic-client/tests/quic_client.rs | 16 +++++----- streamer/src/nonblocking/quic.rs | 33 ++++++++++++--------- streamer/src/quic.rs | 20 +++++-------- streamer/src/tls_certificates.rs | 34 +++++++++------------- 7 files changed, 69 insertions(+), 83 deletions(-) diff --git a/client/src/connection_cache.rs b/client/src/connection_cache.rs index c66dda1fb..586e09b08 100644 --- a/client/src/connection_cache.rs +++ b/client/src/connection_cache.rs @@ -18,7 +18,7 @@ use { solana_streamer::{ nonblocking::quic::{compute_max_allowed_uni_streams, ConnectionPeerType}, streamer::StakedNodes, - tls_certificates::new_self_signed_tls_certificate_chain, + tls_certificates::new_self_signed_tls_certificate, }, solana_tpu_client::{ connection_cache_stats::{ConnectionCacheStats, CONNECTION_STAT_SUBMISSION_INTERVAL}, @@ -98,9 +98,9 @@ impl ConnectionCache { keypair: &Keypair, ipaddr: IpAddr, ) -> Result<(), Box> { - let (certs, priv_key) = new_self_signed_tls_certificate_chain(keypair, ipaddr)?; + let (cert, priv_key) = new_self_signed_tls_certificate(keypair, ipaddr)?; self.client_certificate = Arc::new(QuicClientCertificate { - certificates: certs, + certificate: cert, key: priv_key, }); Ok(()) @@ -371,11 +371,9 @@ impl ConnectionCache { impl Default for ConnectionCache { fn default() -> Self { - let (certs, priv_key) = new_self_signed_tls_certificate_chain( - &Keypair::new(), - IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), - ) - .expect("Failed to initialize QUIC client certificates"); + let (cert, priv_key) = + new_self_signed_tls_certificate(&Keypair::new(), IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0))) + .expect("Failed to initialize QUIC client certificates"); Self { map: RwLock::new(IndexMap::with_capacity(MAX_CONNECTIONS)), stats: Arc::new(ConnectionCacheStats::default()), @@ -386,7 +384,7 @@ impl Default for ConnectionCache { .expect("Unable to bind to UDP socket"), ), client_certificate: Arc::new(QuicClientCertificate { - certificates: certs, + certificate: cert, key: priv_key, }), use_quic: DEFAULT_TPU_USE_QUIC, diff --git a/quic-client/src/lib.rs b/quic-client/src/lib.rs index 73da3b0cd..85e38e06f 100644 --- a/quic-client/src/lib.rs +++ b/quic-client/src/lib.rs @@ -18,7 +18,7 @@ use { solana_streamer::{ nonblocking::quic::{compute_max_allowed_uni_streams, ConnectionPeerType}, streamer::StakedNodes, - tls_certificates::new_self_signed_tls_certificate_chain, + tls_certificates::new_self_signed_tls_certificate, }, solana_tpu_client::{ connection_cache_stats::ConnectionCacheStats, @@ -97,14 +97,12 @@ impl NewTpuConfig for QuicConfig { type ClientError = QuicClientError; fn new() -> Result { - let (certs, priv_key) = new_self_signed_tls_certificate_chain( - &Keypair::new(), - IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), - ) - .map_err(|err| QuicClientError::CertificateError(err.to_string()))?; + let (cert, priv_key) = + new_self_signed_tls_certificate(&Keypair::new(), IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0))) + .map_err(|err| QuicClientError::CertificateError(err.to_string()))?; Ok(Self { client_certificate: Arc::new(QuicClientCertificate { - certificates: certs, + certificate: cert, key: priv_key, }), maybe_staked_nodes: None, @@ -144,9 +142,9 @@ impl QuicConfig { keypair: &Keypair, ipaddr: IpAddr, ) -> Result<(), Box> { - let (certs, priv_key) = new_self_signed_tls_certificate_chain(keypair, ipaddr)?; + let (cert, priv_key) = new_self_signed_tls_certificate(keypair, ipaddr)?; self.client_certificate = Arc::new(QuicClientCertificate { - certificates: certs, + certificate: cert, key: priv_key, }); Ok(()) diff --git a/quic-client/src/nonblocking/quic_client.rs b/quic-client/src/nonblocking/quic_client.rs index 612dc414c..6bb3c50b2 100644 --- a/quic-client/src/nonblocking/quic_client.rs +++ b/quic-client/src/nonblocking/quic_client.rs @@ -23,8 +23,7 @@ use { transport::Result as TransportResult, }, solana_streamer::{ - nonblocking::quic::ALPN_TPU_PROTOCOL_ID, - tls_certificates::new_self_signed_tls_certificate_chain, + nonblocking::quic::ALPN_TPU_PROTOCOL_ID, tls_certificates::new_self_signed_tls_certificate, }, solana_tpu_client::{ connection_cache_stats::ConnectionCacheStats, nonblocking::tpu_connection::TpuConnection, @@ -63,7 +62,7 @@ impl rustls::client::ServerCertVerifier for SkipServerVerification { } pub struct QuicClientCertificate { - pub certificates: Vec, + pub certificate: rustls::Certificate, pub key: rustls::PrivateKey, } @@ -120,7 +119,7 @@ impl QuicLazyInitializedEndpoint { .with_safe_defaults() .with_custom_certificate_verifier(SkipServerVerification::new()) .with_single_cert( - self.client_certificate.certificates.clone(), + vec![self.client_certificate.certificate.clone()], self.client_certificate.key.clone(), ) .expect("Failed to set QUIC client certificates"); @@ -166,14 +165,12 @@ impl QuicLazyInitializedEndpoint { impl Default for QuicLazyInitializedEndpoint { fn default() -> Self { - let (certs, priv_key) = new_self_signed_tls_certificate_chain( - &Keypair::new(), - IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), - ) - .expect("Failed to create QUIC client certificate"); + let (cert, priv_key) = + new_self_signed_tls_certificate(&Keypair::new(), IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0))) + .expect("Failed to create QUIC client certificate"); Self::new( Arc::new(QuicClientCertificate { - certificates: certs, + certificate: cert, key: priv_key, }), None, diff --git a/quic-client/tests/quic_client.rs b/quic-client/tests/quic_client.rs index 946cd0bab..0f2907062 100644 --- a/quic-client/tests/quic_client.rs +++ b/quic-client/tests/quic_client.rs @@ -10,7 +10,7 @@ mod tests { solana_sdk::{packet::PACKET_DATA_SIZE, signature::Keypair}, solana_streamer::{ nonblocking::quic::DEFAULT_WAIT_FOR_CHUNK_TIMEOUT_MS, quic::StreamStats, - streamer::StakedNodes, tls_certificates::new_self_signed_tls_certificate_chain, + streamer::StakedNodes, tls_certificates::new_self_signed_tls_certificate, }, solana_tpu_client::connection_cache_stats::ConnectionCacheStats, std::{ @@ -228,13 +228,11 @@ mod tests { let tpu_addr = SocketAddr::new(addr, port); let connection_cache_stats = Arc::new(ConnectionCacheStats::default()); - let (certs, priv_key) = new_self_signed_tls_certificate_chain( - &Keypair::new(), - IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), - ) - .expect("Failed to initialize QUIC client certificates"); + let (cert, priv_key) = + new_self_signed_tls_certificate(&Keypair::new(), IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0))) + .expect("Failed to initialize QUIC client certificates"); let client_certificate = Arc::new(QuicClientCertificate { - certificates: certs, + certificate: cert, key: priv_key, }); @@ -254,14 +252,14 @@ mod tests { info!("Received requests!"); // Response sender - let (certs, priv_key) = new_self_signed_tls_certificate_chain( + let (cert, priv_key) = new_self_signed_tls_certificate( &Keypair::new(), IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), ) .expect("Failed to initialize QUIC client certificates"); let client_certificate2 = Arc::new(QuicClientCertificate { - certificates: certs, + certificate: cert, key: priv_key, }); diff --git a/streamer/src/nonblocking/quic.rs b/streamer/src/nonblocking/quic.rs index e7ddf44d5..772ff2d0e 100644 --- a/streamer/src/nonblocking/quic.rs +++ b/streamer/src/nonblocking/quic.rs @@ -166,18 +166,23 @@ fn get_connection_stake( .peer_identity() .and_then(|der_cert_any| der_cert_any.downcast::>().ok()) .and_then(|der_certs| { - get_pubkey_from_tls_certificate(&der_certs).and_then(|pubkey| { - debug!("Peer public key is {:?}", pubkey); + if der_certs.len() == 1 { + // Use the client cert only if it is self signed and the chain length is 1 + get_pubkey_from_tls_certificate(&der_certs[0]).and_then(|pubkey| { + debug!("Peer public key is {:?}", pubkey); - let staked_nodes = staked_nodes.read().unwrap(); - let total_stake = staked_nodes.total_stake; - let max_stake = staked_nodes.max_stake; - let min_stake = staked_nodes.min_stake; - staked_nodes - .pubkey_stake_map - .get(&pubkey) - .map(|stake| (pubkey, *stake, total_stake, max_stake, min_stake)) - }) + let staked_nodes = staked_nodes.read().unwrap(); + let total_stake = staked_nodes.total_stake; + let max_stake = staked_nodes.max_stake; + let min_stake = staked_nodes.min_stake; + staked_nodes + .pubkey_stake_map + .get(&pubkey) + .map(|stake| (pubkey, *stake, total_stake, max_stake, min_stake)) + }) + } else { + None + } }) } @@ -926,7 +931,7 @@ pub mod test { crate::{ nonblocking::quic::compute_max_allowed_uni_streams, quic::{MAX_STAKED_CONNECTIONS, MAX_UNSTAKED_CONNECTIONS}, - tls_certificates::new_self_signed_tls_certificate_chain, + tls_certificates::new_self_signed_tls_certificate, }, crossbeam_channel::{unbounded, Receiver}, quinn::{ClientConfig, IdleTimeout, TransportConfig, VarInt}, @@ -963,13 +968,13 @@ pub mod test { pub fn get_client_config(keypair: &Keypair) -> ClientConfig { let ipaddr = IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)); - let (certs, key) = new_self_signed_tls_certificate_chain(keypair, ipaddr) + let (cert, key) = new_self_signed_tls_certificate(keypair, ipaddr) .expect("Failed to generate client certificate"); let mut crypto = rustls::ClientConfig::builder() .with_safe_defaults() .with_custom_certificate_verifier(SkipServerVerification::new()) - .with_single_cert(certs, key) + .with_single_cert(vec![cert], key) .expect("Failed to use client certificate"); crypto.enable_early_data = true; diff --git a/streamer/src/quic.rs b/streamer/src/quic.rs index 8e23f1e02..8ddf26a29 100644 --- a/streamer/src/quic.rs +++ b/streamer/src/quic.rs @@ -1,7 +1,7 @@ use { crate::{ nonblocking::quic::ALPN_TPU_PROTOCOL_ID, streamer::StakedNodes, - tls_certificates::new_self_signed_tls_certificate_chain, + tls_certificates::new_self_signed_tls_certificate, }, crossbeam_channel::Sender, pem::Pem, @@ -58,22 +58,18 @@ pub(crate) fn configure_server( identity_keypair: &Keypair, gossip_host: IpAddr, ) -> Result<(ServerConfig, String), QuicServerError> { - let (cert_chain, priv_key) = - new_self_signed_tls_certificate_chain(identity_keypair, gossip_host) - .map_err(|_e| QuicServerError::ConfigureFailed)?; - let cert_chain_pem_parts: Vec = cert_chain - .iter() - .map(|cert| Pem { - tag: "CERTIFICATE".to_string(), - contents: cert.0.clone(), - }) - .collect(); + let (cert, priv_key) = new_self_signed_tls_certificate(identity_keypair, gossip_host) + .map_err(|_e| QuicServerError::ConfigureFailed)?; + let cert_chain_pem_parts = vec![Pem { + tag: "CERTIFICATE".to_string(), + contents: cert.0.clone(), + }]; let cert_chain_pem = pem::encode_many(&cert_chain_pem_parts); let mut server_tls_config = rustls::ServerConfig::builder() .with_safe_defaults() .with_client_cert_verifier(SkipClientVerification::new()) - .with_single_cert(cert_chain, priv_key) + .with_single_cert(vec![cert], priv_key) .map_err(|_e| QuicServerError::ConfigureFailed)?; server_tls_config.alpn_protocols = vec![ALPN_TPU_PROTOCOL_ID.to_vec()]; diff --git a/streamer/src/tls_certificates.rs b/streamer/src/tls_certificates.rs index 9e6bf35bc..f7a1c4c4a 100644 --- a/streamer/src/tls_certificates.rs +++ b/streamer/src/tls_certificates.rs @@ -6,10 +6,10 @@ use { x509_parser::{prelude::*, public_key::PublicKey}, }; -pub fn new_self_signed_tls_certificate_chain( +pub fn new_self_signed_tls_certificate( keypair: &Keypair, san: IpAddr, -) -> Result<(Vec, rustls::PrivateKey), Box> { +) -> Result<(rustls::Certificate, rustls::PrivateKey), Box> { // TODO(terorie): Is it safe to sign the TLS cert with the identity private key? // Unfortunately, rcgen does not accept a "raw" Ed25519 key. @@ -52,24 +52,18 @@ pub fn new_self_signed_tls_certificate_chain( let cert_der = cert.serialize_der().unwrap(); let priv_key = cert.serialize_private_key_der(); let priv_key = rustls::PrivateKey(priv_key); - let cert_chain = vec![rustls::Certificate(cert_der)]; - Ok((cert_chain, priv_key)) + Ok((rustls::Certificate(cert_der), priv_key)) } -pub fn get_pubkey_from_tls_certificate(certificates: &[rustls::Certificate]) -> Option { - if certificates.len() == 1 { - let der_cert = &certificates[0]; - X509Certificate::from_der(der_cert.as_ref()) - .ok() - .and_then(|(_, cert)| { - cert.public_key().parsed().ok().and_then(|key| match key { - PublicKey::Unknown(inner_key) => Some(Pubkey::new(inner_key)), - _ => None, - }) +pub fn get_pubkey_from_tls_certificate(der_cert: &rustls::Certificate) -> Option { + X509Certificate::from_der(der_cert.as_ref()) + .ok() + .and_then(|(_, cert)| { + cert.public_key().parsed().ok().and_then(|key| match key { + PublicKey::Unknown(inner_key) => Some(Pubkey::new(inner_key)), + _ => None, }) - } else { - None - } + }) } #[cfg(test)] @@ -80,10 +74,10 @@ mod tests { fn test_generate_tls_certificate() { let keypair = Keypair::new(); - if let Ok((certs, _)) = - new_self_signed_tls_certificate_chain(&keypair, IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1))) + if let Ok((cert, _)) = + new_self_signed_tls_certificate(&keypair, IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1))) { - if let Some(pubkey) = get_pubkey_from_tls_certificate(&certs) { + if let Some(pubkey) = get_pubkey_from_tls_certificate(&cert) { assert_eq!(pubkey, keypair.pubkey()); } else { panic!("Failed to get certificate pubkey");