From c8c8772787f1f422aeb1fbf77cb8b9934e3024fb Mon Sep 17 00:00:00 2001 From: Lucas Kent Date: Wed, 9 Nov 2022 11:25:13 +1100 Subject: [PATCH] TLS: Use destination as hostname instead of hardcoded localhost (#899) --- .../cassandra-cluster-tls/topology.yaml | 2 +- .../cassandra-tls/topology-with-key.yaml | 2 +- .../cassandra-tls/topology.yaml | 4 +- .../redis-cluster-tls/topology-with-key.yaml | 2 +- .../redis-cluster-tls/topology.yaml | 2 +- .../example-configs/redis-tls/topology.yaml | 4 +- shotover-proxy/src/tls.rs | 76 ++++++++++++++++++- .../src/transforms/cassandra/connection.rs | 9 +-- .../transforms/cassandra/sink_cluster/node.rs | 4 +- .../src/transforms/redis/sink_single.rs | 8 +- .../util/cluster_connection_pool.rs | 9 +-- .../tests/cassandra_int_tests/cluster/mod.rs | 8 +- shotover-proxy/tests/helpers/mod.rs | 6 +- shotover-proxy/tests/redis_int_tests/mod.rs | 2 +- 14 files changed, 105 insertions(+), 33 deletions(-) diff --git a/shotover-proxy/example-configs/cassandra-cluster-tls/topology.yaml b/shotover-proxy/example-configs/cassandra-cluster-tls/topology.yaml index 2add093c1..d67de694c 100644 --- a/shotover-proxy/example-configs/cassandra-cluster-tls/topology.yaml +++ b/shotover-proxy/example-configs/cassandra-cluster-tls/topology.yaml @@ -22,6 +22,6 @@ chain_config: certificate_authority_path: "example-configs/docker-images/cassandra-tls-4.0.6/certs/localhost_CA.crt" certificate_path: "example-configs/docker-images/cassandra-tls-4.0.6/certs/localhost.crt" private_key_path: "example-configs/docker-images/cassandra-tls-4.0.6/certs/localhost.key" - verify_hostname: true + verify_hostname: false source_to_chain_mapping: cassandra_prod: main_chain diff --git a/shotover-proxy/example-configs/cassandra-tls/topology-with-key.yaml b/shotover-proxy/example-configs/cassandra-tls/topology-with-key.yaml index 6b1f5b06f..ba92e93b8 100644 --- a/shotover-proxy/example-configs/cassandra-tls/topology-with-key.yaml +++ b/shotover-proxy/example-configs/cassandra-tls/topology-with-key.yaml @@ -10,7 +10,7 @@ sources: chain_config: main_chain: - CassandraSinkSingle: - remote_address: "127.0.0.1:9042" + remote_address: "localhost:9042" connect_timeout_ms: 3000 tls: certificate_authority_path: "example-configs/docker-images/cassandra-tls-4.0.6/certs/localhost_CA.crt" diff --git a/shotover-proxy/example-configs/cassandra-tls/topology.yaml b/shotover-proxy/example-configs/cassandra-tls/topology.yaml index 67765e95c..a629d63dd 100644 --- a/shotover-proxy/example-configs/cassandra-tls/topology.yaml +++ b/shotover-proxy/example-configs/cassandra-tls/topology.yaml @@ -10,10 +10,10 @@ sources: chain_config: main_chain: - CassandraSinkSingle: - remote_address: "127.0.0.1:9042" + remote_address: "localhost:9042" connect_timeout_ms: 3000 tls: certificate_authority_path: "example-configs/docker-images/cassandra-tls-4.0.6/certs/localhost_CA.crt" - verify_hostname: false + verify_hostname: true source_to_chain_mapping: cassandra_prod: main_chain diff --git a/shotover-proxy/example-configs/redis-cluster-tls/topology-with-key.yaml b/shotover-proxy/example-configs/redis-cluster-tls/topology-with-key.yaml index eeb255b0e..810ebb03e 100644 --- a/shotover-proxy/example-configs/redis-cluster-tls/topology-with-key.yaml +++ b/shotover-proxy/example-configs/redis-cluster-tls/topology-with-key.yaml @@ -12,6 +12,6 @@ chain_config: certificate_authority_path: "example-configs/redis-tls/certs/ca.crt" certificate_path: "example-configs/redis-tls/certs/redis.crt" private_key_path: "example-configs/redis-tls/certs/redis.key" - verify_hostname: true + verify_hostname: false source_to_chain_mapping: redis_prod: redis_chain diff --git a/shotover-proxy/example-configs/redis-cluster-tls/topology.yaml b/shotover-proxy/example-configs/redis-cluster-tls/topology.yaml index b3c63c114..f0260af87 100644 --- a/shotover-proxy/example-configs/redis-cluster-tls/topology.yaml +++ b/shotover-proxy/example-configs/redis-cluster-tls/topology.yaml @@ -10,6 +10,6 @@ chain_config: connect_timeout_ms: 3000 tls: certificate_authority_path: "example-configs/redis-tls/certs/ca.crt" - verify_hostname: true + verify_hostname: false source_to_chain_mapping: redis_prod: redis_chain diff --git a/shotover-proxy/example-configs/redis-tls/topology.yaml b/shotover-proxy/example-configs/redis-tls/topology.yaml index fa628c258..aed9f5d3c 100644 --- a/shotover-proxy/example-configs/redis-tls/topology.yaml +++ b/shotover-proxy/example-configs/redis-tls/topology.yaml @@ -13,13 +13,13 @@ sources: chain_config: redis_chain_tls: - RedisSinkSingle: - remote_address: "127.0.0.1:1111" + remote_address: "localhost:1111" connect_timeout_ms: 3000 tls: certificate_authority_path: "example-configs/redis-tls/certs/ca.crt" certificate_path: "example-configs/redis-tls/certs/redis.crt" private_key_path: "example-configs/redis-tls/certs/redis.key" - verify_hostname: false + verify_hostname: true source_to_chain_mapping: redis_prod: redis_chain_tls redis_prod_tls: redis_chain_tls diff --git a/shotover-proxy/src/tls.rs b/shotover-proxy/src/tls.rs index 8a55de5ff..cfc8e1fbe 100644 --- a/shotover-proxy/src/tls.rs +++ b/shotover-proxy/src/tls.rs @@ -1,13 +1,16 @@ +use crate::tcp; use anyhow::{anyhow, Result}; use openssl::ssl::Ssl; use openssl::ssl::{SslAcceptor, SslConnector, SslFiletype, SslMethod}; use serde::{Deserialize, Serialize}; use std::fmt::Write; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; use std::path::Path; use std::pin::Pin; use std::sync::Arc; +use std::time::Duration; use tokio::io::{AsyncRead, AsyncWrite}; -use tokio::net::TcpStream; +use tokio::net::{TcpStream, ToSocketAddrs}; use tokio_openssl::SslStream; #[derive(Serialize, Deserialize, Debug, Clone)] @@ -127,15 +130,20 @@ impl TlsConnector { }) } - pub async fn connect(&self, tcp_stream: TcpStream) -> Result> { + pub async fn connect( + &self, + connect_timeout: Duration, + address: A, + ) -> Result> { let ssl = self .connector .configure() .map_err(openssl_stack_error_to_anyhow)? .verify_hostname(self.verify_hostname) - .into_ssl("localhost") + .into_ssl(&address.to_hostname()) .map_err(openssl_stack_error_to_anyhow)?; + let tcp_stream = tcp::tcp_stream(connect_timeout, address).await?; let mut ssl_stream = SslStream::new(ssl, tcp_stream).map_err(openssl_stack_error_to_anyhow)?; Pin::new(&mut ssl_stream).connect().await.map_err(|e| { @@ -147,7 +155,7 @@ impl TlsConnector { } } -// Always use these openssl_* conversion methods instead of directly directly converting to anyhow +// Always use these openssl_* conversion methods instead of directly converting to anyhow fn openssl_ssl_error_to_anyhow(error: openssl::ssl::Error) -> anyhow::Error { if let Some(stack) = error.ssl_error() { @@ -205,3 +213,63 @@ pub trait AsyncStream: AsyncRead + AsyncWrite {} /// We need to tell rust that these types implement AsyncStream even though they already implement AsyncRead and AsyncWrite impl AsyncStream for tokio_openssl::SslStream {} impl AsyncStream for TcpStream {} + +/// Allows retrieving the hostname from any ToSocketAddrs type +pub trait ToHostname { + fn to_hostname(&self) -> String; +} + +/// Implement for all reference types +impl ToHostname for &T { + fn to_hostname(&self) -> String { + (**self).to_hostname() + } +} + +impl ToHostname for String { + fn to_hostname(&self) -> String { + self.split(':').next().unwrap_or("").to_owned() + } +} + +impl ToHostname for &str { + fn to_hostname(&self) -> String { + self.split(':').next().unwrap_or("").to_owned() + } +} + +impl ToHostname for (&str, u16) { + fn to_hostname(&self) -> String { + self.0.to_string() + } +} + +impl ToHostname for (String, u16) { + fn to_hostname(&self) -> String { + self.0.to_string() + } +} + +impl ToHostname for (IpAddr, u16) { + fn to_hostname(&self) -> String { + self.0.to_string() + } +} + +impl ToHostname for (Ipv4Addr, u16) { + fn to_hostname(&self) -> String { + self.0.to_string() + } +} + +impl ToHostname for (Ipv6Addr, u16) { + fn to_hostname(&self) -> String { + self.0.to_string() + } +} + +impl ToHostname for SocketAddr { + fn to_hostname(&self) -> String { + self.ip().to_string() + } +} diff --git a/shotover-proxy/src/transforms/cassandra/connection.rs b/shotover-proxy/src/transforms/cassandra/connection.rs index 185c24579..5cb1312de 100644 --- a/shotover-proxy/src/transforms/cassandra/connection.rs +++ b/shotover-proxy/src/transforms/cassandra/connection.rs @@ -3,7 +3,7 @@ use crate::frame::cassandra::CassandraMetadata; use crate::message::{Message, Metadata}; use crate::server::CodecReadError; use crate::tcp; -use crate::tls::TlsConnector; +use crate::tls::{TlsConnector, ToHostname}; use crate::transforms::util::Response; use crate::transforms::Messages; use anyhow::{anyhow, Result}; @@ -35,21 +35,19 @@ pub struct CassandraConnection { } impl CassandraConnection { - pub async fn new( + pub async fn new( connect_timeout: Duration, host: A, codec: CassandraCodec, mut tls: Option, pushed_messages_tx: Option>, ) -> Result { - let tcp_stream = tcp::tcp_stream(connect_timeout, host).await?; - let (out_tx, out_rx) = mpsc::unbounded_channel::(); let (return_tx, return_rx) = mpsc::unbounded_channel::(); let (rx_process_has_shutdown_tx, rx_process_has_shutdown_rx) = oneshot::channel::<()>(); if let Some(tls) = tls.as_mut() { - let tls_stream = tls.connect(tcp_stream).await?; + let tls_stream = tls.connect(connect_timeout, host).await?; let (read, write) = split(tls_stream); tokio::spawn( tx_process( @@ -72,6 +70,7 @@ impl CassandraConnection { .in_current_span(), ); } else { + let tcp_stream = tcp::tcp_stream(connect_timeout, host).await?; let (read, write) = split(tcp_stream); tokio::spawn( tx_process( diff --git a/shotover-proxy/src/transforms/cassandra/sink_cluster/node.rs b/shotover-proxy/src/transforms/cassandra/sink_cluster/node.rs index 20a0a4373..d05e6da46 100644 --- a/shotover-proxy/src/transforms/cassandra/sink_cluster/node.rs +++ b/shotover-proxy/src/transforms/cassandra/sink_cluster/node.rs @@ -1,7 +1,7 @@ use crate::codec::cassandra::CassandraCodec; use crate::frame::Frame; use crate::message::{Message, Messages}; -use crate::tls::TlsConnector; +use crate::tls::{TlsConnector, ToHostname}; use crate::transforms::cassandra::connection::CassandraConnection; use anyhow::{anyhow, Result}; use cassandra_protocol::frame::Version; @@ -100,7 +100,7 @@ impl ConnectionFactory { } } - pub async fn new_connection( + pub async fn new_connection( &self, address: A, ) -> Result { diff --git a/shotover-proxy/src/transforms/redis/sink_single.rs b/shotover-proxy/src/transforms/redis/sink_single.rs index d835d8dcc..cd31bfcec 100644 --- a/shotover-proxy/src/transforms/redis/sink_single.rs +++ b/shotover-proxy/src/transforms/redis/sink_single.rs @@ -108,12 +108,14 @@ impl Transform for RedisSinkSingle { } if self.connection.is_none() { - let tcp_stream = tcp::tcp_stream(self.connect_timeout, self.address.clone()).await?; - let generic_stream = if let Some(tls) = self.tls.as_mut() { - let tls_stream = tls.connect(tcp_stream).await?; + let tls_stream = tls + .connect(self.connect_timeout, self.address.clone()) + .await?; Box::pin(tls_stream) as Pin> } else { + let tcp_stream = + tcp::tcp_stream(self.connect_timeout, self.address.clone()).await?; Box::pin(tcp_stream) as Pin> }; diff --git a/shotover-proxy/src/transforms/util/cluster_connection_pool.rs b/shotover-proxy/src/transforms/util/cluster_connection_pool.rs index 0def17daa..1191e5eea 100644 --- a/shotover-proxy/src/transforms/util/cluster_connection_pool.rs +++ b/shotover-proxy/src/transforms/util/cluster_connection_pool.rs @@ -159,18 +159,17 @@ impl, T: Token> ConnectionPool address: &str, token: &Option, ) -> Result> { - let tcp_stream = tcp::tcp_stream(self.connect_timeout, address) - .await - .map_err(ConnectionError::IO)?; - let mut connection = if let Some(tls) = &self.tls { let tls_stream = tls - .connect(tcp_stream) + .connect(self.connect_timeout, address) .await .map_err(ConnectionError::TLS)?; let (rx, tx) = tokio::io::split(tls_stream); spawn_read_write_tasks(&self.codec, rx, tx) } else { + let tcp_stream = tcp::tcp_stream(self.connect_timeout, address) + .await + .map_err(ConnectionError::IO)?; let (rx, tx) = tcp_stream.into_split(); spawn_read_write_tasks(&self.codec, rx, tx) }; diff --git a/shotover-proxy/tests/cassandra_int_tests/cluster/mod.rs b/shotover-proxy/tests/cassandra_int_tests/cluster/mod.rs index 19c107ffb..cd978cebc 100644 --- a/shotover-proxy/tests/cassandra_int_tests/cluster/mod.rs +++ b/shotover-proxy/tests/cassandra_int_tests/cluster/mod.rs @@ -8,6 +8,7 @@ use shotover_proxy::transforms::cassandra::sink_cluster::{ }; use std::time::Duration; use tokio::sync::{mpsc, watch}; +use tokio::time::timeout; pub mod multi_rack; pub mod single_rack_v3; @@ -22,7 +23,7 @@ pub async fn run_topology_task(ca_path: Option<&str>, port: Option) -> Vec< certificate_authority_path: ca_path.into(), certificate_path: None, private_key_path: None, - verify_hostname: true, + verify_hostname: false, }) .unwrap() }); @@ -44,7 +45,10 @@ pub async fn run_topology_task(ca_path: Option<&str>, port: Option) -> Vec< .await .unwrap(); - nodes_rx.changed().await.unwrap(); + timeout(Duration::from_secs(30), nodes_rx.changed()) + .await + .unwrap() + .unwrap(); let nodes = nodes_rx.borrow().clone(); nodes } diff --git a/shotover-proxy/tests/helpers/mod.rs b/shotover-proxy/tests/helpers/mod.rs index 01446e48d..2df2bd20a 100644 --- a/shotover-proxy/tests/helpers/mod.rs +++ b/shotover-proxy/tests/helpers/mod.rs @@ -117,11 +117,11 @@ impl ShotoverManager { let address = "127.0.0.1"; test_helpers::wait_for_socket_to_open(address, port); - let tcp_stream = tokio::net::TcpStream::connect((address, port)) + let connector = TlsConnector::new(config).unwrap(); + let tls_stream = connector + .connect(Duration::from_secs(3), (address, port)) .await .unwrap(); - let connector = TlsConnector::new(config).unwrap(); - let tls_stream = connector.connect(tcp_stream).await.unwrap(); ShotoverManager::redis_connection_async_inner( Box::pin(tls_stream) as Pin> ) diff --git a/shotover-proxy/tests/redis_int_tests/mod.rs b/shotover-proxy/tests/redis_int_tests/mod.rs index bdd28671d..badb51984 100644 --- a/shotover-proxy/tests/redis_int_tests/mod.rs +++ b/shotover-proxy/tests/redis_int_tests/mod.rs @@ -81,7 +81,7 @@ async fn source_tls_and_single_tls() { certificate_authority_path: "example-configs/redis-tls/certs/ca.crt".into(), certificate_path: Some("example-configs/redis-tls/certs/redis.crt".into()), private_key_path: Some("example-configs/redis-tls/certs/redis.key".into()), - verify_hostname: true, + verify_hostname: false, }; let mut connection = shotover_manager