From 8702bd2a69d3b9bf7c1169b43f69c240b90269e9 Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Tue, 14 Jan 2025 11:29:55 -0500 Subject: [PATCH 01/29] double hbone (no inner encryption) working!! --- examples/inpodserver.rs | 3 + examples/localhost.yaml | 40 +++++----- src/copy.rs | 23 +++--- src/proxy/h2.rs | 77 +++++++++++++++++- src/proxy/h2/client.rs | 3 +- src/proxy/outbound.rs | 59 +++++++++++++- src/proxy/pool.rs | 14 ++-- src/state.rs | 1 + src/state/workload.rs | 1 + src/tls/certificate.rs | 50 +++++++++--- src/tls/workload.rs | 172 ++++++++++++++++++++++------------------ 11 files changed, 310 insertions(+), 133 deletions(-) diff --git a/examples/inpodserver.rs b/examples/inpodserver.rs index 67d6c1ddde..e7aedbe86a 100644 --- a/examples/inpodserver.rs +++ b/examples/inpodserver.rs @@ -25,6 +25,8 @@ const PROXY_WORKLOAD_INFO: &str = "PROXY_WORKLOAD_INFO"; #[cfg(target_os = "linux")] #[tokio::main] async fn main() { + use std::time::Duration; + let uds = std::env::var("INPOD_UDS").unwrap(); let pwi = match parse_proxy_workload_info() { Ok(pwi) => pwi, @@ -50,6 +52,7 @@ async fn main() { .await .unwrap(); sender.wait_forever().await.unwrap(); + tokio::time::sleep(Duration::from_secs(99999999)).await; } fn parse_proxy_workload_info() -> Result { diff --git a/examples/localhost.yaml b/examples/localhost.yaml index a75800680b..bc87dace2f 100644 --- a/examples/localhost.yaml +++ b/examples/localhost.yaml @@ -11,31 +11,22 @@ workloads: network: "" services: "default/example.com": - 80: 8080 - "default/example2.com": - 80: 8080 + 80: 8081 # Define another local address, but this one uses TCP. This allows testing HBONE and TCP with one config. - uid: cluster1//v1/Pod/default/local-tcp name: local-tcp namespace: default serviceAccount: default - workloadIps: ["127.0.0.2"] - protocol: TCP - node: local - network: "" + workloadIps: ["172.19.0.1"] + protocol: DOUBLEHBONE + node: remote + network: "remote" services: - "default/example.com": - 80: 8080 "default/example2.com": - 80: 8080 -policies: - - action: Allow - rules: - - - - notDestinationPorts: - - 9999 - name: deny-9999 - namespace: default - scope: Namespace + 80: 8081 + "default/example3.com": + 80: 8081 + services: - name: local namespace: default @@ -43,13 +34,20 @@ services: vips: - /127.10.0.1 ports: - 80: 8080 + 80: 8081 subjectAltNames: - spiffe://cluster.local/ns/default/sa/local - name: remote namespace: default hostname: example2.com vips: - - remote/127.10.0.2 + - /1.2.3.4 + ports: + 80: 8081 +- name: remote + namespace: default + hostname: example3.com + vips: + - /1.1.1.1 ports: - 80: 8080 + 80: 8081 diff --git a/src/copy.rs b/src/copy.rs index 7cc0e48756..328d4c752b 100644 --- a/src/copy.rs +++ b/src/copy.rs @@ -36,18 +36,17 @@ pub trait BufferedSplitter: Unpin { } // Generic BufferedSplitter for anything that can Read/Write. -impl BufferedSplitter for I -where - I: AsyncRead + AsyncWrite + Unpin, -{ - type R = BufReader>; - type W = WriteAdapter>; - fn split_into_buffered_reader(self) -> (Self::R, Self::W) { - let (rh, wh) = tokio::io::split(self); - let rb = BufReader::new(rh); - (rb, WriteAdapter(wh)) - } -} +// impl BufferedSplitter for I where +// I: AsyncRead + AsyncWrite + Unpin, +// { +// type R = BufReader>; +// type W = WriteAdapter>; +// fn split_into_buffered_reader(self) -> (Self::R, Self::W) { +// let (rh, wh) = tokio::io::split(self); +// let rb = BufReader::new(rh); +// (rb, WriteAdapter(wh)) +// } +// } // TcpStreamSplitter is a specialized BufferedSplitter for TcpStream, which is more efficient than the generic // `tokio::io::split`. The generic method involves locking to access the read and write halves diff --git a/src/proxy/h2.rs b/src/proxy/h2.rs index bcea3f7c0f..2235eaa7a1 100644 --- a/src/proxy/h2.rs +++ b/src/proxy/h2.rs @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::copy; -use bytes::Bytes; +use crate::copy::{self, AsyncWriteBuf}; +use bytes::{BufMut, Bytes}; use futures_core::ready; use h2::Reason; use std::io::Error; @@ -138,6 +138,56 @@ impl Drop for DropCounter { } } +impl tokio::io::AsyncRead for H2Stream { + fn poll_read( + mut self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: &mut tokio::io::ReadBuf<'_>, + ) -> Poll> { + let pinned = std::pin::pin!(&mut self.read); + tokio::io::AsyncRead::poll_read(pinned, cx, buf) + } +} + +impl tokio::io::AsyncWrite for H2Stream { + fn poll_write( + mut self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: &[u8], + ) -> Poll> { + let pinned = std::pin::pin!(&mut self.write); + tokio::io::AsyncWrite::poll_write(pinned, cx, buf) + } + + fn poll_flush(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + let pinned = std::pin::pin!(&mut self.write); + tokio::io::AsyncWrite::poll_flush(pinned, cx) + } + + fn poll_shutdown( + mut self: Pin<&mut Self>, + cx: &mut Context<'_>, + ) -> Poll> { + let pinned = std::pin::pin!(&mut self.write); + tokio::io::AsyncWrite::poll_shutdown(pinned, cx) + } +} + +impl tokio::io::AsyncRead for H2StreamReadHalf { + fn poll_read( + self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: &mut tokio::io::ReadBuf<'_>, + ) -> Poll> { + copy::ResizeBufRead::poll_bytes(self, cx).map_ok(|bytes| buf.put(bytes)) + + // match copy::ResizeBufRead::poll_bytes(self, cx) { + // Poll::Ready(Ok(bytes)) => {buf.put(bytes); Poll::Ready(Ok(()))}, + // e => {e.map_ok(|_| {})}, + // } + } +} + impl copy::ResizeBufRead for H2StreamReadHalf { fn poll_bytes(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { let this = self.get_mut(); @@ -173,6 +223,29 @@ impl copy::ResizeBufRead for H2StreamReadHalf { } } +impl tokio::io::AsyncWrite for H2StreamWriteHalf { + fn poll_write( + self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: &[u8], + ) -> Poll> { + // note this is pretty slow because it is a copy, but we can optimize later. + let buf = Bytes::copy_from_slice(buf); + return self.poll_write_buf(cx, buf); + } + + fn poll_flush(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + copy::AsyncWriteBuf::poll_flush(self, cx) + } + + fn poll_shutdown( + self: Pin<&mut Self>, + cx: &mut Context<'_>, + ) -> Poll> { + copy::AsyncWriteBuf::poll_shutdown(self, cx) + } +} + impl copy::AsyncWriteBuf for H2StreamWriteHalf { fn poll_write_buf( mut self: Pin<&mut Self>, diff --git a/src/proxy/h2/client.rs b/src/proxy/h2/client.rs index e981dc0730..76d8c7e0d7 100644 --- a/src/proxy/h2/client.rs +++ b/src/proxy/h2/client.rs @@ -109,11 +109,12 @@ impl H2ConnectClient { pub async fn spawn_connection( cfg: Arc, - s: TlsStream, + s: impl AsyncRead + AsyncWrite + Unpin + Send + 'static, driver_drain: Receiver, ) -> Result { let mut builder = h2::client::Builder::new(); builder + .header_table_size(0) .initial_window_size(cfg.window_size) .initial_connection_window_size(cfg.connection_window_size) .max_frame_size(cfg.frame_size) diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index 5c047ade71..1086addf1d 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -15,6 +15,7 @@ use std::net::{IpAddr, SocketAddr}; use std::sync::Arc; +use futures::AsyncWriteExt; use futures_util::TryFutureExt; use hyper::header::FORWARDED; use std::time::Instant; @@ -38,6 +39,8 @@ use crate::state::workload::{address::Address, NetworkAddress, Protocol, Workloa use crate::state::ServiceResolutionMode; use crate::{assertions, copy, proxy, socket}; +use super::h2::client::spawn_connection; + pub struct Outbound { pi: Arc, drain: DrainWatcher, @@ -107,7 +110,7 @@ impl Outbound { debug!(component="outbound", dur=?start.elapsed(), "connection completed"); }).instrument(span); - assertions::size_between_ref(1000, 1750, &serve_outbound_connection); + assertions::size_between_ref(0, 999999, &serve_outbound_connection); tokio::spawn(serve_outbound_connection); } Err(e) => { @@ -202,10 +205,55 @@ impl OutboundConnection { self.proxy_to_tcp(source_stream, &req, &result_tracker) .await } + Protocol::DOUBLEHBONE => { + self.proxy_to_double_hbone(source_stream, source_addr, &req, &result_tracker) + .await + } }; result_tracker.record(res) } + async fn proxy_to_double_hbone( + &mut self, + stream: TcpStream, + remote_addr: SocketAddr, + req: &Request, + connection_stats: &ConnectionResult, + ) -> Result<(), Error> { + let upgraded = Box::pin(self.send_hbone_request(remote_addr, req)).await?; + // Not too sure if this will work, but establish a http2 connection over the existing + // h2 tunnel + let mut client = spawn_connection( + self.pi.cfg.clone(), + upgraded, + self.pool.state.spawner.timeout_rx.clone(), + ) + .await?; + + let mut f = http_types::proxies::Forwarded::new(); + f.add_for(remote_addr.to_string()); + if let Some(svc) = &req.intended_destination_service { + f.set_host(svc.hostname.as_str()); + } + let request: http::Request<()> = http::Request::builder() + .uri( + req.hbone_target_destination + .expect("HBONE must have target") + .to_string(), + ) + .method(hyper::Method::CONNECT) + .version(hyper::Version::HTTP_2) + .header(BAGGAGE_HEADER, baggage(req, self.pi.cfg.cluster_id.clone())) + .header(FORWARDED, f.value().expect("Forwarded value is infallible")) + .header(TRACEPARENT_HEADER, self.id.header()) + .body(()) + .expect("builder with known status code should not fail"); + + //copy::copy_bidirectional(copy::TcpStreamSplitter(stream), upgraded, connection_stats).await + let double_upgraded = client.send_request(request).await?; + copy::copy_bidirectional(copy::TcpStreamSplitter(stream), double_upgraded, connection_stats).await + } + async fn proxy_to_hbone( &mut self, stream: TcpStream, @@ -367,6 +415,9 @@ impl OutboundConnection { }); }; + // Here, we have workload for service (or if we have picked a remote cluster, so it would + // pick the E/W gateway). + let from_waypoint = proxy::check_from_waypoint( state, &us.workload, @@ -404,11 +455,13 @@ impl OutboundConnection { // only change the port if we're sending HBONE let actual_destination = match us.workload.protocol { - Protocol::HBONE => SocketAddr::from((us.selected_workload_ip, self.hbone_port)), + Protocol::HBONE | Protocol::DOUBLEHBONE => { + SocketAddr::from((us.selected_workload_ip, self.hbone_port)) + } Protocol::TCP => us.workload_socket_addr(), }; let hbone_target_destination = match us.workload.protocol { - Protocol::HBONE => Some(us.workload_socket_addr()), + Protocol::HBONE | Protocol::DOUBLEHBONE => Some(us.workload_socket_addr()), Protocol::TCP => None, }; diff --git a/src/proxy/pool.rs b/src/proxy/pool.rs index ea09002433..d4469f50fa 100644 --- a/src/proxy/pool.rs +++ b/src/proxy/pool.rs @@ -54,12 +54,12 @@ use tokio::io; // by flow control throttling. #[derive(Clone)] pub struct WorkloadHBONEPool { - state: Arc, + pub state: Arc, pool_watcher: watch::Receiver, } // PoolState is effectively the gnarly inner state stuff that needs thread/task sync, and should be wrapped in a Mutex. -struct PoolState { +pub struct PoolState { pool_notifier: watch::Sender, // This is already impl clone? rustc complains that it isn't, tho timeout_tx: watch::Sender, // This is already impl clone? rustc complains that it isn't, tho // this is effectively just a convenience data type - a rwlocked hashmap with keying and LRU drops @@ -71,14 +71,14 @@ struct PoolState { // This is merely a counter to track the overall number of conns this pool spawns // to ensure we get unique poolkeys-per-new-conn, it is not a limit pool_global_conn_count: AtomicI32, - spawner: ConnSpawner, + pub spawner: ConnSpawner, } -struct ConnSpawner { +pub struct ConnSpawner { cfg: Arc, socket_factory: Arc, local_workload: Arc, - timeout_rx: watch::Receiver, + pub timeout_rx: watch::Receiver, } // Does nothing but spawn new conns when asked @@ -109,6 +109,10 @@ impl ConnSpawner { } impl PoolState { + + // pub fn spawner(&self) -> &ConnSpawner { + // &self.spawner + // } // This simply puts the connection back into the inner pool, // and sets up a timed popper, which will resolve // - when this reference is popped back out of the inner pool (doing nothing) diff --git a/src/state.rs b/src/state.rs index 879a12df87..f4ddccfe22 100644 --- a/src/state.rs +++ b/src/state.rs @@ -865,6 +865,7 @@ impl DemandProxyState { pub async fn fetch_address(&self, network_addr: &NetworkAddress) -> Option
{ // Wait for it on-demand, *if* needed debug!(%network_addr.address, "fetch address"); + debug!(%network_addr.network, "fetch network"); if let Some(address) = self.state.read().unwrap().find_address(network_addr) { return Some(address); } diff --git a/src/state/workload.rs b/src/state/workload.rs index 6229c47199..bdf91ffbc6 100644 --- a/src/state/workload.rs +++ b/src/state/workload.rs @@ -48,6 +48,7 @@ pub enum Protocol { #[default] TCP, HBONE, + DOUBLEHBONE, } impl From for Protocol { diff --git a/src/tls/certificate.rs b/src/tls/certificate.rs index 8fabac3b38..71c02fe138 100644 --- a/src/tls/certificate.rs +++ b/src/tls/certificate.rs @@ -19,10 +19,11 @@ use bytes::Bytes; use itertools::Itertools; use rustls::client::Resumption; +use rustls::pki_types::pem::PemObject; use rustls::pki_types::{CertificateDer, PrivateKeyDer}; use rustls::server::WebPkiClientVerifier; -use rustls::{server, ClientConfig, RootCertStore, ServerConfig}; +use rustls::{server, ClientConfig, KeyLogFile, RootCertStore, ServerConfig}; use rustls_pemfile::Item; use std::io::Cursor; use std::str::FromStr; @@ -30,9 +31,11 @@ use std::sync::Arc; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use tracing::warn; -use crate::tls; +// use crate::tls; use x509_parser::certificate::X509Certificate; +use super::TLS_VERSIONS; + #[derive(Clone, Debug)] pub struct Certificate { pub(in crate::tls) expiry: Expiration, @@ -248,22 +251,29 @@ impl WorkloadCertificate { } pub fn server_config(&self) -> Result { - let td = self.cert.identity().map(|i| match i { + let _td = self.cert.identity().map(|i| match i { Identity::Spiffe { trust_domain, .. } => trust_domain, }); - let raw_client_cert_verifier = WebPkiClientVerifier::builder_with_provider( + let _raw_client_cert_verifier = WebPkiClientVerifier::builder_with_provider( self.roots.clone(), crate::tls::lib::provider(), ) .build()?; - let client_cert_verifier = - crate::tls::workload::TrustDomainVerifier::new(raw_client_cert_verifier, td); - let mut sc = ServerConfig::builder_with_provider(crate::tls::lib::provider()) - .with_protocol_versions(tls::TLS_VERSIONS) - .expect("server config must be valid") - .with_client_cert_verifier(client_cert_verifier) - .with_single_cert(self.cert_and_intermediates(), self.private_key.clone_key())?; + let mut sc = ServerConfig::builder() + .with_no_client_auth() + .with_single_cert( + vec![ + CertificateDer::from_pem_file("/home/sj/learning/openssl/c/jimmy.crt").unwrap(), + ], + PrivateKeyDer::from_pem_file("/home/sj/learning/openssl/c/jimmy.key").unwrap(), + )?; + //let mut sc = ServerConfig::builder_with_provider(crate::tls::lib::provider()) + // .with_protocol_versions(TLS_VERSIONS) + // .expect("server config must be valid") + // .with_no_client_auth() + // .with_single_cert(self.cert_and_intermediates(), self.private_key.clone_key())?; + sc.key_log = Arc::new(KeyLogFile::new()); sc.alpn_protocols = vec![b"h2".into()]; Ok(sc) } @@ -271,15 +281,29 @@ impl WorkloadCertificate { pub fn outbound_connector(&self, identity: Vec) -> Result { let roots = self.roots.clone(); let verifier = IdentityVerifier { roots, identity }; + let mut root_cert_store = RootCertStore::empty(); + root_cert_store.add_parsable_certificates(vec![CertificateDer::from_pem_file( + "/home/sj/learning/openssl/c/root.crt", + ) + .unwrap()]); let mut cc = ClientConfig::builder_with_provider(crate::tls::lib::provider()) - .with_protocol_versions(tls::TLS_VERSIONS) + .with_protocol_versions(TLS_VERSIONS) .expect("client config must be valid") .dangerous() // Customer verifier is requires "dangerous" opt-in .with_custom_certificate_verifier(Arc::new(verifier)) - .with_client_auth_cert(self.cert_and_intermediates(), self.private_key.clone_key())?; + .with_no_client_auth(); + + // let mut cc = ClientConfig::builder_with_provider(crate::tls::lib::provider()) + // .with_protocol_versions(TLS_VERSIONS) + // .expect("client config must be valid") + // .dangerous() // Customer verifier is requires "dangerous" opt-in + // .with_custom_certificate_verifier(Arc::new(verifier)) + // .with_no_client_auth(); + // .with_client_auth_cert(self.cert_and_intermediates(), self.private_key.clone_key())?; cc.alpn_protocols = vec![b"h2".into()]; cc.resumption = Resumption::disabled(); cc.enable_sni = false; + cc.key_log = Arc::new(KeyLogFile::new()); Ok(OutboundConnector { client_config: Arc::new(cc), }) diff --git a/src/tls/workload.rs b/src/tls/workload.rs index 25a9cac771..b0536d1474 100644 --- a/src/tls/workload.rs +++ b/src/tls/workload.rs @@ -167,15 +167,16 @@ impl OutboundConnector { self, stream: TcpStream, ) -> Result, io::Error> { - let dest = ServerName::IpAddress( - stream - .peer_addr() - .expect("peer_addr must be set") - .ip() - .into(), - ); + let sn = ServerName::try_from("jimmy").unwrap(); + //let dest = ServerName::IpAddress( + // stream + // .peer_addr() + // .expect("peer_addr must be set") + // .ip() + // .into(), + //); let c = tokio_rustls::TlsConnector::from(self.client_config); - c.connect(dest, stream).await + c.connect(sn, stream).await } } @@ -186,31 +187,32 @@ pub struct IdentityVerifier { } impl IdentityVerifier { - fn verify_full_san(&self, server_cert: &CertificateDer<'_>) -> Result<(), rustls::Error> { - use x509_parser::prelude::*; - let (_, c) = X509Certificate::from_der(server_cert).map_err(|_e| { - rustls::Error::InvalidCertificate(rustls::CertificateError::BadEncoding) - })?; - let id = tls::certificate::identities(c).map_err(|_e| { - rustls::Error::InvalidCertificate( - rustls::CertificateError::ApplicationVerificationFailure, - ) - })?; - trace!( - "verifying server identities {id:?} against {:?}", - self.identity - ); - for ident in id.iter() { - if let Some(_i) = self.identity.iter().find(|id| id == &ident) { - return Ok(()); - } - } - debug!("identity mismatch {id:?} != {:?}", self.identity); - Err(rustls::Error::InvalidCertificate( - rustls::CertificateError::Other(rustls::OtherError(Arc::new(DebugAsDisplay( - TlsError::SanError(self.identity.clone(), id), - )))), - )) + fn verify_full_san(&self, _server_cert: &CertificateDer<'_>) -> Result<(), rustls::Error> { + Ok(()) + // use x509_parser::prelude::*; + // let (_, c) = X509Certificate::from_der(server_cert).map_err(|_e| { + // rustls::Error::InvalidCertificate(rustls::CertificateError::BadEncoding) + // })?; + // let id = tls::certificate::identities(c).map_err(|_e| { + // rustls::Error::InvalidCertificate( + // rustls::CertificateError::ApplicationVerificationFailure, + // ) + // })?; + // trace!( + // "verifying server identities {id:?} against {:?}", + // self.identity + // ); + // for ident in id.iter() { + // if let Some(_i) = self.identity.iter().find(|id| id == &ident) { + // return Ok(()); + // } + // } + // debug!("identity mismatch {id:?} != {:?}", self.identity); + // Err(rustls::Error::InvalidCertificate( + // rustls::CertificateError::Other(rustls::OtherError(Arc::new(DebugAsDisplay( + // TlsError::SanError(self.identity.clone(), id), + // )))), + // )) } } @@ -243,65 +245,83 @@ impl ServerCertVerifier for IdentityVerifier { /// - Not Expired fn verify_server_cert( &self, - end_entity: &CertificateDer<'_>, - intermediates: &[CertificateDer<'_>], - _sn: &ServerName, - ocsp_response: &[u8], - now: UnixTime, + _end_entity: &CertificateDer<'_>, + _intermediates: &[CertificateDer<'_>], + __sn: &ServerName, + _ocsp_response: &[u8], + _now: UnixTime, ) -> Result { - let cert = ParsedCertificate::try_from(end_entity)?; - - let algs = provider().signature_verification_algorithms; - rustls::client::verify_server_cert_signed_by_trust_anchor( - &cert, - &self.roots, - intermediates, - now, - algs.all, - )?; - - if !ocsp_response.is_empty() { - trace!("Unvalidated OCSP response: {ocsp_response:?}"); - } - - self.verify_full_san(end_entity)?; - Ok(ServerCertVerified::assertion()) + // let cert = ParsedCertificate::try_from(end_entity)?; + // + // let algs = provider().signature_verification_algorithms; + // rustls::client::verify_server_cert_signed_by_trust_anchor( + // &cert, + // &self.roots, + // intermediates, + // now, + // algs.all, + // )?; + // + // if !ocsp_response.is_empty() { + // trace!("Unvalidated OCSP response: {ocsp_response:?}"); + // } + // + // self.verify_full_san(end_entity)?; + // + // Ok(ServerCertVerified::assertion()) } // Rest use the default implementations fn verify_tls12_signature( &self, - message: &[u8], - cert: &CertificateDer<'_>, - dss: &DigitallySignedStruct, + _message: &[u8], + _cert: &CertificateDer<'_>, + _dss: &DigitallySignedStruct, ) -> Result { - rustls::crypto::verify_tls12_signature( - message, - cert, - dss, - &provider().signature_verification_algorithms, - ) + Ok(HandshakeSignatureValid::assertion()) + // rustls::crypto::verify_tls12_signature( + // message, + // cert, + // dss, + // &provider().signature_verification_algorithms, + // ) } fn verify_tls13_signature( &self, - message: &[u8], - cert: &CertificateDer<'_>, - dss: &DigitallySignedStruct, + _message: &[u8], + _cert: &CertificateDer<'_>, + _dss: &DigitallySignedStruct, ) -> Result { - rustls::crypto::verify_tls13_signature( - message, - cert, - dss, - &provider().signature_verification_algorithms, - ) + Ok(HandshakeSignatureValid::assertion()) + // rustls::crypto::verify_tls13_signature( + // message, + // cert, + // dss, + // &provider().signature_verification_algorithms, + // ) } fn supported_verify_schemes(&self) -> Vec { - provider() - .signature_verification_algorithms - .supported_schemes() + vec![ + SignatureScheme::RSA_PKCS1_SHA1, + SignatureScheme::ECDSA_SHA1_Legacy, + SignatureScheme::RSA_PKCS1_SHA256, + SignatureScheme::ECDSA_NISTP256_SHA256, + SignatureScheme::RSA_PKCS1_SHA384, + SignatureScheme::ECDSA_NISTP384_SHA384, + SignatureScheme::RSA_PKCS1_SHA512, + SignatureScheme::ECDSA_NISTP521_SHA512, + SignatureScheme::RSA_PSS_SHA256, + SignatureScheme::RSA_PSS_SHA384, + SignatureScheme::RSA_PSS_SHA512, + SignatureScheme::ED25519, + SignatureScheme::ED448, + ] + // provider() + // .signature_verification_algorithms + // .supported_schemes() } } From e340d8c80bea9bd16a6eef4f4bbecf95d77e3c2a Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Wed, 15 Jan 2025 16:31:15 -0500 Subject: [PATCH 02/29] hbone-ish --- examples/localhost.yaml | 2 +- local-dev-setup.sh | 43 ++++++++++++ src/copy.rs | 22 +++--- src/proxy/h2.rs | 76 +++++++-------------- src/proxy/h2/client.rs | 1 - src/proxy/outbound.rs | 63 +++++++++-------- src/proxy/pool.rs | 46 ++++++++++++- src/state.rs | 1 - src/tls/certificate.rs | 48 ++++++------- src/tls/workload.rs | 146 ++++++++++++++++++---------------------- tests/namespaced.rs | 22 +++++- 11 files changed, 259 insertions(+), 211 deletions(-) create mode 100755 local-dev-setup.sh diff --git a/examples/localhost.yaml b/examples/localhost.yaml index bc87dace2f..c089fe856c 100644 --- a/examples/localhost.yaml +++ b/examples/localhost.yaml @@ -20,7 +20,7 @@ workloads: workloadIps: ["172.19.0.1"] protocol: DOUBLEHBONE node: remote - network: "remote" + network: "" services: "default/example2.com": 80: 8081 diff --git a/local-dev-setup.sh b/local-dev-setup.sh new file mode 100755 index 0000000000..a2d04c41ae --- /dev/null +++ b/local-dev-setup.sh @@ -0,0 +1,43 @@ +#! /bin/bash + +set -eux + +ip netns delete pod1 || true +ip netns delete z || true +ip link delete pod1 || true +ip link delete z || true + +ip netns add pod1 +ip -n pod1 link set lo up +# veth device +ip link add pod1 type veth peer name pod1-eth0 +# move one end to the pod +ip link set pod1-eth0 netns pod1 +# configure the veth devices +ip link set pod1 up +ip -n pod1 link set pod1-eth0 up +ip addr add dev pod1 10.0.0.1/24 +ip -n pod1 addr add dev pod1-eth0 10.0.0.2/24 +ip netns exec pod1 ip route add default dev pod1-eth0 + +ip netns add z +ip -n z link set lo up +# veth device +ip link add z type veth peer name z-eth0 +# move one end to the pod +ip link set z-eth0 netns z +# configure the veth devices +ip link set z up +ip -n z link set z-eth0 up +ip addr add dev z 10.0.1.1/24 +ip -n z addr add dev z-eth0 10.0.1.2/24 +ip netns exec z ip route add default dev z-eth0 + + + +cat < BufferedSplitter for I where -// I: AsyncRead + AsyncWrite + Unpin, -// { -// type R = BufReader>; -// type W = WriteAdapter>; -// fn split_into_buffered_reader(self) -> (Self::R, Self::W) { -// let (rh, wh) = tokio::io::split(self); -// let rb = BufReader::new(rh); -// (rb, WriteAdapter(wh)) -// } -// } +impl BufferedSplitter for I where + I: AsyncRead + AsyncWrite + Unpin, +{ + type R = BufReader>; + type W = WriteAdapter>; + fn split_into_buffered_reader(self) -> (Self::R, Self::W) { + let (rh, wh) = tokio::io::split(self); + let rb = BufReader::new(rh); + (rb, WriteAdapter(wh)) + } +} // TcpStreamSplitter is a specialized BufferedSplitter for TcpStream, which is more efficient than the generic // `tokio::io::split`. The generic method involves locking to access the read and write halves diff --git a/src/proxy/h2.rs b/src/proxy/h2.rs index 2235eaa7a1..e447787bc9 100644 --- a/src/proxy/h2.rs +++ b/src/proxy/h2.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::copy::{self, AsyncWriteBuf}; +use crate::copy::{self}; use bytes::{BufMut, Bytes}; use futures_core::ready; use h2::Reason; @@ -85,6 +85,8 @@ pub struct H2StreamWriteHalf { _dropped: Option, } +pub struct TokioH2Stream(H2Stream); + struct DropCounter { // Whether the other end of this shared counter has already dropped. // We only decrement if they have, so we do not double count @@ -138,53 +140,50 @@ impl Drop for DropCounter { } } -impl tokio::io::AsyncRead for H2Stream { +// We can't directly implement tokio::io::{AsyncRead, AsyncWrite} for H2Stream because +// then the specific implementation will conflict with the generic one. +impl TokioH2Stream { + pub fn new(stream: H2Stream) -> Self { + Self(stream) + } +} + +impl tokio::io::AsyncRead for TokioH2Stream { fn poll_read( mut self: Pin<&mut Self>, cx: &mut Context<'_>, buf: &mut tokio::io::ReadBuf<'_>, ) -> Poll> { - let pinned = std::pin::pin!(&mut self.read); - tokio::io::AsyncRead::poll_read(pinned, cx, buf) + let pinned = std::pin::Pin::new(&mut self.0.read); + copy::ResizeBufRead::poll_bytes(pinned, cx).map_ok(|bytes| buf.put(bytes)) } } -impl tokio::io::AsyncWrite for H2Stream { +impl tokio::io::AsyncWrite for TokioH2Stream { fn poll_write( mut self: Pin<&mut Self>, cx: &mut Context<'_>, buf: &[u8], ) -> Poll> { - let pinned = std::pin::pin!(&mut self.write); - tokio::io::AsyncWrite::poll_write(pinned, cx, buf) - } - - fn poll_flush(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { - let pinned = std::pin::pin!(&mut self.write); - tokio::io::AsyncWrite::poll_flush(pinned, cx) + let pinned = std::pin::Pin::new(&mut self.0.write); + let buf = Bytes::copy_from_slice(buf); + copy::AsyncWriteBuf::poll_write_buf(pinned, cx, buf) } - fn poll_shutdown( + fn poll_flush( mut self: Pin<&mut Self>, cx: &mut Context<'_>, ) -> Poll> { - let pinned = std::pin::pin!(&mut self.write); - tokio::io::AsyncWrite::poll_shutdown(pinned, cx) + let pinned = std::pin::Pin::new(&mut self.0.write); + copy::AsyncWriteBuf::poll_flush(pinned, cx) } -} -impl tokio::io::AsyncRead for H2StreamReadHalf { - fn poll_read( - self: Pin<&mut Self>, + fn poll_shutdown( + mut self: Pin<&mut Self>, cx: &mut Context<'_>, - buf: &mut tokio::io::ReadBuf<'_>, - ) -> Poll> { - copy::ResizeBufRead::poll_bytes(self, cx).map_ok(|bytes| buf.put(bytes)) - - // match copy::ResizeBufRead::poll_bytes(self, cx) { - // Poll::Ready(Ok(bytes)) => {buf.put(bytes); Poll::Ready(Ok(()))}, - // e => {e.map_ok(|_| {})}, - // } + ) -> Poll> { + let pinned = std::pin::Pin::new(&mut self.0.write); + copy::AsyncWriteBuf::poll_shutdown(pinned, cx) } } @@ -223,29 +222,6 @@ impl copy::ResizeBufRead for H2StreamReadHalf { } } -impl tokio::io::AsyncWrite for H2StreamWriteHalf { - fn poll_write( - self: Pin<&mut Self>, - cx: &mut Context<'_>, - buf: &[u8], - ) -> Poll> { - // note this is pretty slow because it is a copy, but we can optimize later. - let buf = Bytes::copy_from_slice(buf); - return self.poll_write_buf(cx, buf); - } - - fn poll_flush(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { - copy::AsyncWriteBuf::poll_flush(self, cx) - } - - fn poll_shutdown( - self: Pin<&mut Self>, - cx: &mut Context<'_>, - ) -> Poll> { - copy::AsyncWriteBuf::poll_shutdown(self, cx) - } -} - impl copy::AsyncWriteBuf for H2StreamWriteHalf { fn poll_write_buf( mut self: Pin<&mut Self>, diff --git a/src/proxy/h2/client.rs b/src/proxy/h2/client.rs index 76d8c7e0d7..2396671930 100644 --- a/src/proxy/h2/client.rs +++ b/src/proxy/h2/client.rs @@ -114,7 +114,6 @@ pub async fn spawn_connection( ) -> Result { let mut builder = h2::client::Builder::new(); builder - .header_table_size(0) .initial_window_size(cfg.window_size) .initial_connection_window_size(cfg.connection_window_size) .max_frame_size(cfg.frame_size) diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index 1086addf1d..d3d0a9e9ed 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -19,6 +19,7 @@ use futures::AsyncWriteExt; use futures_util::TryFutureExt; use hyper::header::FORWARDED; use std::time::Instant; +use tokio::io::{AsyncRead, AsyncWrite}; use tokio::net::TcpStream; use tokio::sync::watch; @@ -40,6 +41,7 @@ use crate::state::ServiceResolutionMode; use crate::{assertions, copy, proxy, socket}; use super::h2::client::spawn_connection; +use super::h2::TokioH2Stream; pub struct Outbound { pi: Arc, @@ -221,37 +223,25 @@ impl OutboundConnection { connection_stats: &ConnectionResult, ) -> Result<(), Error> { let upgraded = Box::pin(self.send_hbone_request(remote_addr, req)).await?; - // Not too sure if this will work, but establish a http2 connection over the existing - // h2 tunnel - let mut client = spawn_connection( - self.pi.cfg.clone(), - upgraded, - self.pool.state.spawner.timeout_rx.clone(), - ) - .await?; - let mut f = http_types::proxies::Forwarded::new(); - f.add_for(remote_addr.to_string()); - if let Some(svc) = &req.intended_destination_service { - f.set_host(svc.hostname.as_str()); - } - let request: http::Request<()> = http::Request::builder() - .uri( - req.hbone_target_destination - .expect("HBONE must have target") - .to_string(), - ) - .method(hyper::Method::CONNECT) - .version(hyper::Version::HTTP_2) - .header(BAGGAGE_HEADER, baggage(req, self.pi.cfg.cluster_id.clone())) - .header(FORWARDED, f.value().expect("Forwarded value is infallible")) - .header(TRACEPARENT_HEADER, self.id.header()) - .body(()) - .expect("builder with known status code should not fail"); + let upgraded = TokioH2Stream::new(upgraded); + let inner_workload = pool::WorkloadKey { + src_id: req.source.identity(), + // Clone here shouldn't be needed ideally, we could just take ownership of Request. + // But that + dst_id: req.upstream_sans.clone(), + src: remote_addr.ip(), + dst: req.actual_destination, + }; + let request = self.create_hbone_request(remote_addr, req); - //copy::copy_bidirectional(copy::TcpStreamSplitter(stream), upgraded, connection_stats).await - let double_upgraded = client.send_request(request).await?; - copy::copy_bidirectional(copy::TcpStreamSplitter(stream), double_upgraded, connection_stats).await + let inner_upgraded = self.pool.send_request_unpooled(upgraded, &inner_workload, request).await?; + copy::copy_bidirectional( + copy::TcpStreamSplitter(stream), + inner_upgraded, + connection_stats, + ) + .await } async fn proxy_to_hbone( @@ -265,12 +255,12 @@ impl OutboundConnection { copy::copy_bidirectional(copy::TcpStreamSplitter(stream), upgraded, connection_stats).await } - async fn send_hbone_request( + fn create_hbone_request( &mut self, remote_addr: SocketAddr, req: &Request, - ) -> Result { - let request = http::Request::builder() + ) -> http::Request<()> { + http::Request::builder() .uri( req.hbone_target_destination .expect("HBONE must have target") @@ -285,8 +275,15 @@ impl OutboundConnection { ) .header(TRACEPARENT_HEADER, self.id.header()) .body(()) - .expect("builder with known status code should not fail"); + .expect("builder with known status code should not fail") + } + async fn send_hbone_request( + &mut self, + remote_addr: SocketAddr, + req: &Request, + ) -> Result { + let request = self.create_hbone_request(remote_addr, req); let pool_key = Box::new(pool::WorkloadKey { src_id: req.source.identity(), // Clone here shouldn't be needed ideally, we could just take ownership of Request. diff --git a/src/proxy/pool.rs b/src/proxy/pool.rs index d4469f50fa..db027a1210 100644 --- a/src/proxy/pool.rs +++ b/src/proxy/pool.rs @@ -77,12 +77,35 @@ pub struct PoolState { pub struct ConnSpawner { cfg: Arc, socket_factory: Arc, - local_workload: Arc, + pub local_workload: Arc, pub timeout_rx: watch::Receiver, } // Does nothing but spawn new conns when asked impl ConnSpawner { + // creates new HBONE connection over existing stream. + // Useful for when we want the lifetime of the connection to be tied to the connection pool, + // but we don't want to pool the connection itself. + async fn new_unpooled_conn( + &self, + key: WorkloadKey, + stream: impl tokio::io::AsyncRead + tokio::io::AsyncWrite + Send + Unpin + 'static, + ) -> Result { + let dest = rustls::pki_types::ServerName::IpAddress(key.dst.ip().into()); + + let cert = self.local_workload.fetch_certificate().await?; + let connector = cert.outbound_connector(vec![])?; + let tls_stream = connector.connect(stream, dest).await?; + let sender = + h2::client::spawn_connection(self.cfg.clone(), tls_stream, self.timeout_rx.clone()) + .await?; + let client = ConnClient { + sender, + wl_key: key, + }; + Ok(client) + } + async fn new_pool_conn(&self, key: WorkloadKey) -> Result { debug!("spawning new pool conn for {}", key); @@ -95,7 +118,14 @@ impl ConnSpawner { _ => e.into(), })?; - let tls_stream = connector.connect(tcp_stream).await?; + let dest = rustls::pki_types::ServerName::IpAddress( + tcp_stream + .peer_addr() + .expect("peer_addr must be set") + .ip() + .into(), + ); + let tls_stream = connector.connect(tcp_stream, dest).await?; trace!("connector connected, handshaking"); let sender = h2::client::spawn_connection(self.cfg.clone(), tls_stream, self.timeout_rx.clone()) @@ -109,7 +139,6 @@ impl ConnSpawner { } impl PoolState { - // pub fn spawner(&self) -> &ConnSpawner { // &self.spawner // } @@ -375,6 +404,17 @@ impl WorkloadHBONEPool { } } + pub async fn send_request_unpooled( + &mut self, + stream: impl tokio::io::AsyncWrite + tokio::io::AsyncRead + Send + Unpin + 'static, + workload_key: &WorkloadKey, + request: http::Request<()>, + ) -> Result { + let mut connection = self.state.spawner.new_unpooled_conn(workload_key.clone(), stream).await?; + + connection.sender.send_request(request).await + } + pub async fn send_request_pooled( &mut self, workload_key: &WorkloadKey, diff --git a/src/state.rs b/src/state.rs index f4ddccfe22..879a12df87 100644 --- a/src/state.rs +++ b/src/state.rs @@ -865,7 +865,6 @@ impl DemandProxyState { pub async fn fetch_address(&self, network_addr: &NetworkAddress) -> Option
{ // Wait for it on-demand, *if* needed debug!(%network_addr.address, "fetch address"); - debug!(%network_addr.network, "fetch network"); if let Some(address) = self.state.read().unwrap().find_address(network_addr) { return Some(address); } diff --git a/src/tls/certificate.rs b/src/tls/certificate.rs index 71c02fe138..43c6649bde 100644 --- a/src/tls/certificate.rs +++ b/src/tls/certificate.rs @@ -31,11 +31,9 @@ use std::sync::Arc; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use tracing::warn; -// use crate::tls; +use crate::tls; use x509_parser::certificate::X509Certificate; -use super::TLS_VERSIONS; - #[derive(Clone, Debug)] pub struct Certificate { pub(in crate::tls) expiry: Expiration, @@ -230,6 +228,10 @@ impl WorkloadCertificate { let mut roots = RootCertStore::empty(); roots.add_parsable_certificates(chain.iter().last().map(|c| c.der.clone())); + roots.add_parsable_certificates(vec![CertificateDer::from_pem_file( + "/home/sj/learning/openssl/c/root.crt", + ).unwrap()]); + Ok(WorkloadCertificate { cert, chain, @@ -251,28 +253,26 @@ impl WorkloadCertificate { } pub fn server_config(&self) -> Result { - let _td = self.cert.identity().map(|i| match i { + let td = self.cert.identity().map(|i| match i { Identity::Spiffe { trust_domain, .. } => trust_domain, }); - let _raw_client_cert_verifier = WebPkiClientVerifier::builder_with_provider( - self.roots.clone(), + let mut roots = (*self.roots).clone(); + roots.add_parsable_certificates(vec![CertificateDer::from_pem_file( + "/home/sj/learning/openssl/c/root.crt", + ).unwrap()]); + let raw_client_cert_verifier = WebPkiClientVerifier::builder_with_provider( + Arc::new(roots), crate::tls::lib::provider(), ) .build()?; - let mut sc = ServerConfig::builder() - .with_no_client_auth() - .with_single_cert( - vec![ - CertificateDer::from_pem_file("/home/sj/learning/openssl/c/jimmy.crt").unwrap(), - ], - PrivateKeyDer::from_pem_file("/home/sj/learning/openssl/c/jimmy.key").unwrap(), - )?; - //let mut sc = ServerConfig::builder_with_provider(crate::tls::lib::provider()) - // .with_protocol_versions(TLS_VERSIONS) - // .expect("server config must be valid") - // .with_no_client_auth() - // .with_single_cert(self.cert_and_intermediates(), self.private_key.clone_key())?; + let client_cert_verifier = + crate::tls::workload::TrustDomainVerifier::new(raw_client_cert_verifier, td); + let mut sc = ServerConfig::builder_with_provider(crate::tls::lib::provider()) + .with_protocol_versions(tls::TLS_VERSIONS) + .expect("server config must be valid") + .with_client_cert_verifier(client_cert_verifier) + .with_single_cert(self.cert_and_intermediates(), self.private_key.clone_key())?; sc.key_log = Arc::new(KeyLogFile::new()); sc.alpn_protocols = vec![b"h2".into()]; Ok(sc) @@ -287,19 +287,11 @@ impl WorkloadCertificate { ) .unwrap()]); let mut cc = ClientConfig::builder_with_provider(crate::tls::lib::provider()) - .with_protocol_versions(TLS_VERSIONS) + .with_protocol_versions(tls::TLS_VERSIONS) .expect("client config must be valid") .dangerous() // Customer verifier is requires "dangerous" opt-in .with_custom_certificate_verifier(Arc::new(verifier)) .with_no_client_auth(); - - // let mut cc = ClientConfig::builder_with_provider(crate::tls::lib::provider()) - // .with_protocol_versions(TLS_VERSIONS) - // .expect("client config must be valid") - // .dangerous() // Customer verifier is requires "dangerous" opt-in - // .with_custom_certificate_verifier(Arc::new(verifier)) - // .with_no_client_auth(); - // .with_client_auth_cert(self.cert_and_intermediates(), self.private_key.clone_key())?; cc.alpn_protocols = vec![b"h2".into()]; cc.resumption = Resumption::disabled(); cc.enable_sni = false; diff --git a/src/tls/workload.rs b/src/tls/workload.rs index b0536d1474..d573930557 100644 --- a/src/tls/workload.rs +++ b/src/tls/workload.rs @@ -31,6 +31,7 @@ use std::future::Future; use std::io; use std::pin::Pin; use std::sync::Arc; +use tokio::io::{AsyncRead, AsyncWrite}; use crate::strng::Strng; use crate::tls; @@ -107,11 +108,12 @@ impl ClientCertVerifier for TrustDomainVerifier { intermediates: &[CertificateDer<'_>], now: UnixTime, ) -> Result { - let res = self - .base - .verify_client_cert(end_entity, intermediates, now)?; - self.verify_trust_domain(end_entity)?; - Ok(res) + Ok(ClientCertVerified::assertion()) + // let res = self + // .base + // .verify_client_cert(end_entity, intermediates, now)?; + // self.verify_trust_domain(end_entity)?; + // Ok(res) } fn verify_tls12_signature( @@ -120,7 +122,8 @@ impl ClientCertVerifier for TrustDomainVerifier { cert: &CertificateDer<'_>, dss: &DigitallySignedStruct, ) -> Result { - self.base.verify_tls12_signature(message, cert, dss) + Ok(HandshakeSignatureValid::assertion()) + // self.base.verify_tls12_signature(message, cert, dss) } fn verify_tls13_signature( @@ -129,7 +132,8 @@ impl ClientCertVerifier for TrustDomainVerifier { cert: &CertificateDer<'_>, dss: &DigitallySignedStruct, ) -> Result { - self.base.verify_tls13_signature(message, cert, dss) + Ok(HandshakeSignatureValid::assertion()) + // self.base.verify_tls13_signature(message, cert, dss) } fn supported_verify_schemes(&self) -> Vec { @@ -163,20 +167,16 @@ pub struct OutboundConnector { } impl OutboundConnector { - pub async fn connect( + pub async fn connect( self, - stream: TcpStream, - ) -> Result, io::Error> { - let sn = ServerName::try_from("jimmy").unwrap(); - //let dest = ServerName::IpAddress( - // stream - // .peer_addr() - // .expect("peer_addr must be set") - // .ip() - // .into(), - //); + stream: IO, + domain: ServerName<'static>, + ) -> Result, io::Error> + where + IO: AsyncRead + AsyncWrite + Unpin, + { let c = tokio_rustls::TlsConnector::from(self.client_config); - c.connect(sn, stream).await + c.connect(domain, stream).await } } @@ -187,7 +187,7 @@ pub struct IdentityVerifier { } impl IdentityVerifier { - fn verify_full_san(&self, _server_cert: &CertificateDer<'_>) -> Result<(), rustls::Error> { + fn verify_full_san(&self, server_cert: &CertificateDer<'_>) -> Result<(), rustls::Error> { Ok(()) // use x509_parser::prelude::*; // let (_, c) = X509Certificate::from_der(server_cert).map_err(|_e| { @@ -245,83 +245,67 @@ impl ServerCertVerifier for IdentityVerifier { /// - Not Expired fn verify_server_cert( &self, - _end_entity: &CertificateDer<'_>, - _intermediates: &[CertificateDer<'_>], - __sn: &ServerName, - _ocsp_response: &[u8], - _now: UnixTime, + end_entity: &CertificateDer<'_>, + intermediates: &[CertificateDer<'_>], + _sn: &ServerName, + ocsp_response: &[u8], + now: UnixTime, ) -> Result { + let cert = ParsedCertificate::try_from(end_entity)?; + + let algs = provider().signature_verification_algorithms; + rustls::client::verify_server_cert_signed_by_trust_anchor( + &cert, + &self.roots, + intermediates, + now, + algs.all, + )?; + + if !ocsp_response.is_empty() { + trace!("Unvalidated OCSP response: {ocsp_response:?}"); + } + + self.verify_full_san(end_entity)?; + Ok(ServerCertVerified::assertion()) - // let cert = ParsedCertificate::try_from(end_entity)?; - // - // let algs = provider().signature_verification_algorithms; - // rustls::client::verify_server_cert_signed_by_trust_anchor( - // &cert, - // &self.roots, - // intermediates, - // now, - // algs.all, - // )?; - // - // if !ocsp_response.is_empty() { - // trace!("Unvalidated OCSP response: {ocsp_response:?}"); - // } - // - // self.verify_full_san(end_entity)?; - // - // Ok(ServerCertVerified::assertion()) } // Rest use the default implementations fn verify_tls12_signature( &self, - _message: &[u8], - _cert: &CertificateDer<'_>, - _dss: &DigitallySignedStruct, + message: &[u8], + cert: &CertificateDer<'_>, + dss: &DigitallySignedStruct, ) -> Result { - Ok(HandshakeSignatureValid::assertion()) - // rustls::crypto::verify_tls12_signature( - // message, - // cert, - // dss, - // &provider().signature_verification_algorithms, - // ) + // Ok(HandshakeSignatureValid::assertion()) + rustls::crypto::verify_tls12_signature( + message, + cert, + dss, + &provider().signature_verification_algorithms, + ) } fn verify_tls13_signature( &self, - _message: &[u8], - _cert: &CertificateDer<'_>, - _dss: &DigitallySignedStruct, + message: &[u8], + cert: &CertificateDer<'_>, + dss: &DigitallySignedStruct, ) -> Result { - Ok(HandshakeSignatureValid::assertion()) - // rustls::crypto::verify_tls13_signature( - // message, - // cert, - // dss, - // &provider().signature_verification_algorithms, - // ) + // Ok(HandshakeSignatureValid::assertion()) + rustls::crypto::verify_tls13_signature( + message, + cert, + dss, + &provider().signature_verification_algorithms, + ) } fn supported_verify_schemes(&self) -> Vec { - vec![ - SignatureScheme::RSA_PKCS1_SHA1, - SignatureScheme::ECDSA_SHA1_Legacy, - SignatureScheme::RSA_PKCS1_SHA256, - SignatureScheme::ECDSA_NISTP256_SHA256, - SignatureScheme::RSA_PKCS1_SHA384, - SignatureScheme::ECDSA_NISTP384_SHA384, - SignatureScheme::RSA_PKCS1_SHA512, - SignatureScheme::ECDSA_NISTP521_SHA512, - SignatureScheme::RSA_PSS_SHA256, - SignatureScheme::RSA_PSS_SHA384, - SignatureScheme::RSA_PSS_SHA512, - SignatureScheme::ED25519, - SignatureScheme::ED448, - ] - // provider() - // .signature_verification_algorithms - // .supported_schemes() + provider() + .signature_verification_algorithms + .supported_schemes() } } diff --git a/tests/namespaced.rs b/tests/namespaced.rs index b8f1ed1b17..9a2df1d856 100644 --- a/tests/namespaced.rs +++ b/tests/namespaced.rs @@ -800,7 +800,16 @@ mod namespaced { let connector = cert.outbound_connector(vec![dst_id]).unwrap(); let hbone = SocketAddr::new(srv.ip(), 15008); let tcp_stream = TcpStream::connect(hbone).await.unwrap(); - let tls_stream = connector.connect(tcp_stream).await.unwrap(); + + let dest = rustls::pki_types::ServerName::IpAddress( + tcp_stream + .peer_addr() + .expect("peer_addr must be set") + .ip() + .into(), + ); + let tls_stream = connector.connect(tcp_stream, dest).await.unwrap(); + let (mut request_sender, connection) = builder.handshake(TokioIo::new(tls_stream)).await.unwrap(); // spawn a task to poll the connection and drive the HTTP state @@ -863,7 +872,16 @@ mod namespaced { let tcp_stream = TcpStream::connect(SocketAddr::from((srv.ip(), 15008))) .await .unwrap(); - let tls_stream = connector.connect(tcp_stream).await.unwrap(); + + let dest = rustls::pki_types::ServerName::IpAddress( + tcp_stream + .peer_addr() + .expect("peer_addr must be set") + .ip() + .into(), + ); + let tls_stream = connector.connect(tcp_stream, dest).await.unwrap(); + let (mut request_sender, connection) = builder.handshake(TokioIo::new(tls_stream)).await.unwrap(); // spawn a task to poll the connection and drive the HTTP state From b381e3431854e57dd338e8f7d75ffc5368974ff4 Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Wed, 15 Jan 2025 17:19:20 -0500 Subject: [PATCH 03/29] cleanup --- src/copy.rs | 3 ++- src/proxy/h2.rs | 2 +- src/proxy/outbound.rs | 7 +------ src/proxy/pool.rs | 11 ++++------- 4 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/copy.rs b/src/copy.rs index 0ffc2b8c97..7cc0e48756 100644 --- a/src/copy.rs +++ b/src/copy.rs @@ -36,7 +36,8 @@ pub trait BufferedSplitter: Unpin { } // Generic BufferedSplitter for anything that can Read/Write. -impl BufferedSplitter for I where +impl BufferedSplitter for I +where I: AsyncRead + AsyncWrite + Unpin, { type R = BufReader>; diff --git a/src/proxy/h2.rs b/src/proxy/h2.rs index e447787bc9..afdf396143 100644 --- a/src/proxy/h2.rs +++ b/src/proxy/h2.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::copy::{self}; +use crate::copy; use bytes::{BufMut, Bytes}; use futures_core::ready; use h2::Reason; diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index d3d0a9e9ed..ce64cd68ee 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -112,7 +112,7 @@ impl Outbound { debug!(component="outbound", dur=?start.elapsed(), "connection completed"); }).instrument(span); - assertions::size_between_ref(0, 999999, &serve_outbound_connection); + assertions::size_between_ref(1000, 1750, &serve_outbound_connection); tokio::spawn(serve_outbound_connection); } Err(e) => { @@ -227,8 +227,6 @@ impl OutboundConnection { let upgraded = TokioH2Stream::new(upgraded); let inner_workload = pool::WorkloadKey { src_id: req.source.identity(), - // Clone here shouldn't be needed ideally, we could just take ownership of Request. - // But that dst_id: req.upstream_sans.clone(), src: remote_addr.ip(), dst: req.actual_destination, @@ -412,9 +410,6 @@ impl OutboundConnection { }); }; - // Here, we have workload for service (or if we have picked a remote cluster, so it would - // pick the E/W gateway). - let from_waypoint = proxy::check_from_waypoint( state, &us.workload, diff --git a/src/proxy/pool.rs b/src/proxy/pool.rs index db027a1210..f54c26cfd8 100644 --- a/src/proxy/pool.rs +++ b/src/proxy/pool.rs @@ -54,7 +54,7 @@ use tokio::io; // by flow control throttling. #[derive(Clone)] pub struct WorkloadHBONEPool { - pub state: Arc, + state: Arc, pool_watcher: watch::Receiver, } @@ -74,11 +74,11 @@ pub struct PoolState { pub spawner: ConnSpawner, } -pub struct ConnSpawner { +struct ConnSpawner { cfg: Arc, socket_factory: Arc, - pub local_workload: Arc, - pub timeout_rx: watch::Receiver, + local_workload: Arc, + timeout_rx: watch::Receiver, } // Does nothing but spawn new conns when asked @@ -139,9 +139,6 @@ impl ConnSpawner { } impl PoolState { - // pub fn spawner(&self) -> &ConnSpawner { - // &self.spawner - // } // This simply puts the connection back into the inner pool, // and sets up a timed popper, which will resolve // - when this reference is popped back out of the inner pool (doing nothing) From 2192d67342268a00db12da734ab4a7ed842bd5c3 Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Fri, 17 Jan 2025 11:50:40 -0500 Subject: [PATCH 04/29] graceful shutdowns --- src/config.rs | 2 +- src/proxy/h2/client.rs | 6 +++--- src/proxy/outbound.rs | 23 +++++++++++++++++++---- src/proxy/pool.rs | 38 ++++++++++++++++++++++++++++---------- 4 files changed, 51 insertions(+), 18 deletions(-) diff --git a/src/config.rs b/src/config.rs index e9b7f9cf5a..1f7b061c4a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -70,7 +70,7 @@ const IPV6_ENABLED: &str = "IPV6_ENABLED"; const UNSTABLE_ENABLE_SOCKS5: &str = "UNSTABLE_ENABLE_SOCKS5"; -const DEFAULT_WORKER_THREADS: u16 = 2; +const DEFAULT_WORKER_THREADS: u16 = 40; const DEFAULT_ADMIN_PORT: u16 = 15000; const DEFAULT_READINESS_PORT: u16 = 15021; const DEFAULT_STATS_PORT: u16 = 15020; diff --git a/src/proxy/h2/client.rs b/src/proxy/h2/client.rs index 2396671930..5f25dc9f67 100644 --- a/src/proxy/h2/client.rs +++ b/src/proxy/h2/client.rs @@ -111,7 +111,7 @@ pub async fn spawn_connection( cfg: Arc, s: impl AsyncRead + AsyncWrite + Unpin + Send + 'static, driver_drain: Receiver, -) -> Result { +) -> Result<(H2ConnectClient, tokio::task::JoinHandle<()>), Error> { let mut builder = h2::client::Builder::new(); builder .initial_window_size(cfg.window_size) @@ -139,7 +139,7 @@ pub async fn spawn_connection( // spawn a task to poll the connection and drive the HTTP state // if we got a drain for that connection, respect it in a race // it is important to have a drain here, or this connection will never terminate - tokio::spawn( + let driver_handle = tokio::spawn( async move { drive_connection(connection, driver_drain).await; } @@ -151,7 +151,7 @@ pub async fn spawn_connection( stream_count: Arc::new(AtomicU16::new(0)), max_allowed_streams, }; - Ok(c) + Ok((c, driver_handle)) } async fn drive_connection(mut conn: Connection, mut driver_drain: Receiver) diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index ce64cd68ee..7e0190863c 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -112,7 +112,7 @@ impl Outbound { debug!(component="outbound", dur=?start.elapsed(), "connection completed"); }).instrument(span); - assertions::size_between_ref(1000, 1750, &serve_outbound_connection); + assertions::size_between_ref(1000, 99999, &serve_outbound_connection); tokio::spawn(serve_outbound_connection); } Err(e) => { @@ -233,13 +233,28 @@ impl OutboundConnection { }; let request = self.create_hbone_request(remote_addr, req); - let inner_upgraded = self.pool.send_request_unpooled(upgraded, &inner_workload, request).await?; - copy::copy_bidirectional( + let (conn_client, inner_upgraded, drain_tx, driver_task) = self + .pool + .send_request_unpooled(upgraded, &inner_workload, request) + .await?; + let inner_upgraded = inner_upgraded?; + let res = copy::copy_bidirectional( copy::TcpStreamSplitter(stream), inner_upgraded, connection_stats, ) - .await + .await; + + // This always drops ungracefully + // drop(conn_client); + // tokio::time::sleep(std::time::Duration::from_secs(1)).await; + // drain_tx.send(true).unwrap(); + // tokio::time::sleep(std::time::Duration::from_secs(1)).await; + drain_tx.send(true).unwrap(); + driver_task.await; + // this sleep is important, so we have a race condition somewhere + // tokio::time::sleep(std::time::Duration::from_secs(1)).await; + res } async fn proxy_to_hbone( diff --git a/src/proxy/pool.rs b/src/proxy/pool.rs index f54c26cfd8..5f20f7a62e 100644 --- a/src/proxy/pool.rs +++ b/src/proxy/pool.rs @@ -90,20 +90,20 @@ impl ConnSpawner { &self, key: WorkloadKey, stream: impl tokio::io::AsyncRead + tokio::io::AsyncWrite + Send + Unpin + 'static, - ) -> Result { + driver_drain: tokio::sync::watch::Receiver, + ) -> Result<(ConnClient, tokio::task::JoinHandle<()>), Error> { let dest = rustls::pki_types::ServerName::IpAddress(key.dst.ip().into()); let cert = self.local_workload.fetch_certificate().await?; let connector = cert.outbound_connector(vec![])?; let tls_stream = connector.connect(stream, dest).await?; - let sender = - h2::client::spawn_connection(self.cfg.clone(), tls_stream, self.timeout_rx.clone()) - .await?; + let (sender, driver_drain) = + h2::client::spawn_connection(self.cfg.clone(), tls_stream, driver_drain).await?; let client = ConnClient { sender, wl_key: key, }; - Ok(client) + Ok((client, driver_drain)) } async fn new_pool_conn(&self, key: WorkloadKey) -> Result { @@ -127,7 +127,7 @@ impl ConnSpawner { ); let tls_stream = connector.connect(tcp_stream, dest).await?; trace!("connector connected, handshaking"); - let sender = + let (sender, _) = h2::client::spawn_connection(self.cfg.clone(), tls_stream, self.timeout_rx.clone()) .await?; let client = ConnClient { @@ -406,10 +406,28 @@ impl WorkloadHBONEPool { stream: impl tokio::io::AsyncWrite + tokio::io::AsyncRead + Send + Unpin + 'static, workload_key: &WorkloadKey, request: http::Request<()>, - ) -> Result { - let mut connection = self.state.spawner.new_unpooled_conn(workload_key.clone(), stream).await?; + ) -> Result< + ( + ConnClient, + Result, + tokio::sync::watch::Sender, + tokio::task::JoinHandle<()>, + ), + Error, + > { + let (tx, rx) = tokio::sync::watch::channel(false); + let (mut connection, driver_task) = self + .state + .spawner + .new_unpooled_conn(workload_key.clone(), stream, rx) + .await?; - connection.sender.send_request(request).await + Ok(( + connection.clone(), + connection.sender.send_request(request).await, + tx, + driver_task, + )) } pub async fn send_request_pooled( @@ -552,7 +570,7 @@ impl WorkloadHBONEPool { #[derive(Debug, Clone)] // A sort of faux-client, that represents a single checked-out 'request sender' which might // send requests over some underlying stream using some underlying http/2 client -struct ConnClient { +pub struct ConnClient { sender: H2ConnectClient, // A WL key may have many clients, but every client has no more than one WL key wl_key: WorkloadKey, // the WL key associated with this client. From dff6c92a386de1aecccb72f37f440199a8778e0d Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Fri, 17 Jan 2025 12:43:00 -0500 Subject: [PATCH 05/29] Some cleanup --- local-dev-setup.sh | 43 ------------------------------------------ src/proxy/h2/client.rs | 2 -- src/proxy/outbound.rs | 9 ++++----- src/proxy/pool.rs | 5 +---- 4 files changed, 5 insertions(+), 54 deletions(-) delete mode 100755 local-dev-setup.sh diff --git a/local-dev-setup.sh b/local-dev-setup.sh deleted file mode 100755 index a2d04c41ae..0000000000 --- a/local-dev-setup.sh +++ /dev/null @@ -1,43 +0,0 @@ -#! /bin/bash - -set -eux - -ip netns delete pod1 || true -ip netns delete z || true -ip link delete pod1 || true -ip link delete z || true - -ip netns add pod1 -ip -n pod1 link set lo up -# veth device -ip link add pod1 type veth peer name pod1-eth0 -# move one end to the pod -ip link set pod1-eth0 netns pod1 -# configure the veth devices -ip link set pod1 up -ip -n pod1 link set pod1-eth0 up -ip addr add dev pod1 10.0.0.1/24 -ip -n pod1 addr add dev pod1-eth0 10.0.0.2/24 -ip netns exec pod1 ip route add default dev pod1-eth0 - -ip netns add z -ip -n z link set lo up -# veth device -ip link add z type veth peer name z-eth0 -# move one end to the pod -ip link set z-eth0 netns z -# configure the veth devices -ip link set z up -ip -n z link set z-eth0 up -ip addr add dev z 10.0.1.1/24 -ip -n z addr add dev z-eth0 10.0.1.2/24 -ip netns exec z ip route add default dev z-eth0 - - - -cat < Result<(), Error> { + // Outer HBONE let upgraded = Box::pin(self.send_hbone_request(remote_addr, req)).await?; + // Inner HBONE let upgraded = TokioH2Stream::new(upgraded); let inner_workload = pool::WorkloadKey { src_id: req.source.identity(), @@ -233,7 +232,7 @@ impl OutboundConnection { }; let request = self.create_hbone_request(remote_addr, req); - let (conn_client, inner_upgraded, drain_tx, driver_task) = self + let (_conn_client, inner_upgraded, drain_tx, driver_task) = self .pool .send_request_unpooled(upgraded, &inner_workload, request) .await?; @@ -251,7 +250,7 @@ impl OutboundConnection { // drain_tx.send(true).unwrap(); // tokio::time::sleep(std::time::Duration::from_secs(1)).await; drain_tx.send(true).unwrap(); - driver_task.await; + let _ = driver_task.await; // this sleep is important, so we have a race condition somewhere // tokio::time::sleep(std::time::Duration::from_secs(1)).await; res diff --git a/src/proxy/pool.rs b/src/proxy/pool.rs index 5f20f7a62e..7ed14e1745 100644 --- a/src/proxy/pool.rs +++ b/src/proxy/pool.rs @@ -59,7 +59,7 @@ pub struct WorkloadHBONEPool { } // PoolState is effectively the gnarly inner state stuff that needs thread/task sync, and should be wrapped in a Mutex. -pub struct PoolState { +struct PoolState { pool_notifier: watch::Sender, // This is already impl clone? rustc complains that it isn't, tho timeout_tx: watch::Sender, // This is already impl clone? rustc complains that it isn't, tho // this is effectively just a convenience data type - a rwlocked hashmap with keying and LRU drops @@ -83,9 +83,6 @@ struct ConnSpawner { // Does nothing but spawn new conns when asked impl ConnSpawner { - // creates new HBONE connection over existing stream. - // Useful for when we want the lifetime of the connection to be tied to the connection pool, - // but we don't want to pool the connection itself. async fn new_unpooled_conn( &self, key: WorkloadKey, From f1cc535de919ece63b60ee6d7071b1ce1b97b39a Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Tue, 21 Jan 2025 12:03:00 -0500 Subject: [PATCH 06/29] Add some auth/tls logic back in --- src/tls/certificate.rs | 8 +++-- src/tls/workload.rs | 66 ++++++++++++++++++++---------------------- 2 files changed, 36 insertions(+), 38 deletions(-) diff --git a/src/tls/certificate.rs b/src/tls/certificate.rs index 43c6649bde..8177a50c55 100644 --- a/src/tls/certificate.rs +++ b/src/tls/certificate.rs @@ -230,7 +230,8 @@ impl WorkloadCertificate { roots.add_parsable_certificates(chain.iter().last().map(|c| c.der.clone())); roots.add_parsable_certificates(vec![CertificateDer::from_pem_file( "/home/sj/learning/openssl/c/root.crt", - ).unwrap()]); + ) + .unwrap()]); Ok(WorkloadCertificate { cert, @@ -259,7 +260,8 @@ impl WorkloadCertificate { let mut roots = (*self.roots).clone(); roots.add_parsable_certificates(vec![CertificateDer::from_pem_file( "/home/sj/learning/openssl/c/root.crt", - ).unwrap()]); + ) + .unwrap()]); let raw_client_cert_verifier = WebPkiClientVerifier::builder_with_provider( Arc::new(roots), crate::tls::lib::provider(), @@ -291,7 +293,7 @@ impl WorkloadCertificate { .expect("client config must be valid") .dangerous() // Customer verifier is requires "dangerous" opt-in .with_custom_certificate_verifier(Arc::new(verifier)) - .with_no_client_auth(); + .with_client_auth_cert(self.cert_and_intermediates(), self.private_key.clone_key())?; cc.alpn_protocols = vec![b"h2".into()]; cc.resumption = Resumption::disabled(); cc.enable_sni = false; diff --git a/src/tls/workload.rs b/src/tls/workload.rs index d573930557..05fe94bc0a 100644 --- a/src/tls/workload.rs +++ b/src/tls/workload.rs @@ -108,12 +108,11 @@ impl ClientCertVerifier for TrustDomainVerifier { intermediates: &[CertificateDer<'_>], now: UnixTime, ) -> Result { - Ok(ClientCertVerified::assertion()) - // let res = self - // .base - // .verify_client_cert(end_entity, intermediates, now)?; - // self.verify_trust_domain(end_entity)?; - // Ok(res) + let res = self + .base + .verify_client_cert(end_entity, intermediates, now)?; + self.verify_trust_domain(end_entity)?; + Ok(res) } fn verify_tls12_signature( @@ -122,8 +121,7 @@ impl ClientCertVerifier for TrustDomainVerifier { cert: &CertificateDer<'_>, dss: &DigitallySignedStruct, ) -> Result { - Ok(HandshakeSignatureValid::assertion()) - // self.base.verify_tls12_signature(message, cert, dss) + self.base.verify_tls12_signature(message, cert, dss) } fn verify_tls13_signature( @@ -132,8 +130,7 @@ impl ClientCertVerifier for TrustDomainVerifier { cert: &CertificateDer<'_>, dss: &DigitallySignedStruct, ) -> Result { - Ok(HandshakeSignatureValid::assertion()) - // self.base.verify_tls13_signature(message, cert, dss) + self.base.verify_tls13_signature(message, cert, dss) } fn supported_verify_schemes(&self) -> Vec { @@ -188,31 +185,30 @@ pub struct IdentityVerifier { impl IdentityVerifier { fn verify_full_san(&self, server_cert: &CertificateDer<'_>) -> Result<(), rustls::Error> { - Ok(()) - // use x509_parser::prelude::*; - // let (_, c) = X509Certificate::from_der(server_cert).map_err(|_e| { - // rustls::Error::InvalidCertificate(rustls::CertificateError::BadEncoding) - // })?; - // let id = tls::certificate::identities(c).map_err(|_e| { - // rustls::Error::InvalidCertificate( - // rustls::CertificateError::ApplicationVerificationFailure, - // ) - // })?; - // trace!( - // "verifying server identities {id:?} against {:?}", - // self.identity - // ); - // for ident in id.iter() { - // if let Some(_i) = self.identity.iter().find(|id| id == &ident) { - // return Ok(()); - // } - // } - // debug!("identity mismatch {id:?} != {:?}", self.identity); - // Err(rustls::Error::InvalidCertificate( - // rustls::CertificateError::Other(rustls::OtherError(Arc::new(DebugAsDisplay( - // TlsError::SanError(self.identity.clone(), id), - // )))), - // )) + use x509_parser::prelude::*; + let (_, c) = X509Certificate::from_der(server_cert).map_err(|_e| { + rustls::Error::InvalidCertificate(rustls::CertificateError::BadEncoding) + })?; + let id = tls::certificate::identities(c).map_err(|_e| { + rustls::Error::InvalidCertificate( + rustls::CertificateError::ApplicationVerificationFailure, + ) + })?; + trace!( + "verifying server identities {id:?} against {:?}", + self.identity + ); + for ident in id.iter() { + if let Some(_i) = self.identity.iter().find(|id| id == &ident) { + return Ok(()); + } + } + debug!("identity mismatch {id:?} != {:?}", self.identity); + Err(rustls::Error::InvalidCertificate( + rustls::CertificateError::Other(rustls::OtherError(Arc::new(DebugAsDisplay( + TlsError::SanError(self.identity.clone(), id), + )))), + )) } } From cab48493ad1606296306c80a07aa2513debec059 Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Tue, 21 Jan 2025 15:51:09 -0500 Subject: [PATCH 07/29] inline double hbone code --- src/proxy/outbound.rs | 50 ++++++++++++++++++++++++++++--------------- src/proxy/pool.rs | 23 ++++++++++++++------ 2 files changed, 50 insertions(+), 23 deletions(-) diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index 32e1907da2..dabb0e7349 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -219,11 +219,7 @@ impl OutboundConnection { req: &Request, connection_stats: &ConnectionResult, ) -> Result<(), Error> { - // Outer HBONE - let upgraded = Box::pin(self.send_hbone_request(remote_addr, req)).await?; - - // Inner HBONE - let upgraded = TokioH2Stream::new(upgraded); + // TODO: dst should take a hostname? and upstream_sans currently contains E/W Gateway certs let inner_workload = pool::WorkloadKey { src_id: req.source.identity(), dst_id: req.upstream_sans.clone(), @@ -231,12 +227,34 @@ impl OutboundConnection { dst: req.actual_destination, }; let request = self.create_hbone_request(remote_addr, req); + let workload_key = &inner_workload; - let (_conn_client, inner_upgraded, drain_tx, driver_task) = self - .pool - .send_request_unpooled(upgraded, &inner_workload, request) + // To shut down inner HTTP2 connection + let (drain_tx, drain_rx) = tokio::sync::watch::channel(false); + let key = workload_key.clone(); + let dest = rustls::pki_types::ServerName::IpAddress(key.dst.ip().into()); + let cert = self + .pi + .local_workload_information + .fetch_certificate() .await?; - let inner_upgraded = inner_upgraded?; + // FIXME The following isn't great because it will also contain the identity of the E/W gateways + let connector = cert.outbound_connector(req.upstream_sans.clone())?; + + // Do actual IO as late as possible + // Outer HBONE + let upgraded = Box::pin(self.send_hbone_request(remote_addr, req)).await?; + // Wrap upgraded to implement tokio's Async{Write,Read} + let upgraded = TokioH2Stream::new(upgraded); + let tls_stream = connector.connect(upgraded, dest).await?; + + let (sender, driver_task) = + super::h2::client::spawn_connection(self.pi.cfg.clone(), tls_stream, drain_rx).await?; + let mut connection = super::pool::ConnClient { + sender, + wl_key: key, + }; + let inner_upgraded = connection.sender.send_request(request).await?; let res = copy::copy_bidirectional( copy::TcpStreamSplitter(stream), inner_upgraded, @@ -244,15 +262,13 @@ impl OutboundConnection { ) .await; - // This always drops ungracefully - // drop(conn_client); - // tokio::time::sleep(std::time::Duration::from_secs(1)).await; - // drain_tx.send(true).unwrap(); - // tokio::time::sleep(std::time::Duration::from_secs(1)).await; - drain_tx.send(true).unwrap(); + let _ = drain_tx.send(true); let _ = driver_task.await; - // this sleep is important, so we have a race condition somewhere - // tokio::time::sleep(std::time::Duration::from_secs(1)).await; + // Here, there is an implicit, drop(conn_client). + // Its really important that this happens AFTER driver_task finishes. + // Otherwise, TLS connections do not terminate gracefully. + // + // drop(conn_client); res } diff --git a/src/proxy/pool.rs b/src/proxy/pool.rs index 7ed14e1745..ceb646f482 100644 --- a/src/proxy/pool.rs +++ b/src/proxy/pool.rs @@ -71,7 +71,7 @@ struct PoolState { // This is merely a counter to track the overall number of conns this pool spawns // to ensure we get unique poolkeys-per-new-conn, it is not a limit pool_global_conn_count: AtomicI32, - pub spawner: ConnSpawner, + spawner: ConnSpawner, } struct ConnSpawner { @@ -413,17 +413,28 @@ impl WorkloadHBONEPool { Error, > { let (tx, rx) = tokio::sync::watch::channel(false); - let (mut connection, driver_task) = self + let key = workload_key.clone(); + let dest = rustls::pki_types::ServerName::IpAddress(key.dst.ip().into()); + let cert = self .state .spawner - .new_unpooled_conn(workload_key.clone(), stream, rx) + .local_workload + .fetch_certificate() .await?; + let connector = cert.outbound_connector(vec![])?; + let tls_stream = connector.connect(stream, dest).await?; + let (sender, driver_drain) = + h2::client::spawn_connection(self.state.spawner.cfg.clone(), tls_stream, rx).await?; + let mut connection = ConnClient { + sender, + wl_key: key, + }; Ok(( connection.clone(), connection.sender.send_request(request).await, tx, - driver_task, + driver_drain, )) } @@ -568,9 +579,9 @@ impl WorkloadHBONEPool { // A sort of faux-client, that represents a single checked-out 'request sender' which might // send requests over some underlying stream using some underlying http/2 client pub struct ConnClient { - sender: H2ConnectClient, + pub sender: H2ConnectClient, // A WL key may have many clients, but every client has no more than one WL key - wl_key: WorkloadKey, // the WL key associated with this client. + pub wl_key: WorkloadKey, // the WL key associated with this client. } impl ConnClient { From 565f41f69897208887a79c938d3043a51b6b0fcd Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Tue, 21 Jan 2025 16:15:57 -0500 Subject: [PATCH 08/29] Use correct(?) identities --- src/proxy/outbound.rs | 17 ++++++++++++++--- src/state.rs | 13 +++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index dabb0e7349..0238eaf31c 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -238,8 +238,7 @@ impl OutboundConnection { .local_workload_information .fetch_certificate() .await?; - // FIXME The following isn't great because it will also contain the identity of the E/W gateways - let connector = cert.outbound_connector(req.upstream_sans.clone())?; + let connector = cert.outbound_connector(req.final_sans.clone())?; // Do actual IO as late as possible // Outer HBONE @@ -407,6 +406,7 @@ impl OutboundConnection { intended_destination_service: Some(ServiceDescription::from(&*target_service)), actual_destination, upstream_sans, + final_sans: vec![], }); } // this was service addressed but we did not find a waypoint @@ -437,6 +437,7 @@ impl OutboundConnection { intended_destination_service: None, actual_destination: target, upstream_sans: vec![], + final_sans: vec![], }); }; @@ -470,6 +471,7 @@ impl OutboundConnection { intended_destination_service: us.destination_service.clone(), actual_destination, upstream_sans, + final_sans: vec![], }); } // Workload doesn't have a waypoint; send directly @@ -486,9 +488,12 @@ impl OutboundConnection { Protocol::HBONE | Protocol::DOUBLEHBONE => Some(us.workload_socket_addr()), Protocol::TCP => None, }; + let (upstream_sans, final_sans) = match us.workload.protocol { + Protocol::DOUBLEHBONE => (vec![us.workload.identity()], us.service_sans()), + Protocol::TCP | Protocol::HBONE => (us.workload_and_services_san(), vec![]), + }; // For case no waypoint for both side and direct to remote node proxy - let upstream_sans = us.workload_and_services_san(); debug!("built request to workload"); Ok(Request { protocol: us.workload.protocol, @@ -498,6 +503,7 @@ impl OutboundConnection { intended_destination_service: us.destination_service.clone(), actual_destination, upstream_sans, + final_sans, }) } } @@ -546,6 +552,11 @@ struct Request { // The identity we will assert for the next hop; this may not be the same as actual_destination_workload // in the case of proxies along the path. upstream_sans: Vec, + + // The identity of workload that will ultimately process this request. + // This field only matters if we need to know both the identity of the next hop, as well as the + // final hop (currently, this is only double HBONE). + final_sans: Vec, } #[cfg(test)] diff --git a/src/state.rs b/src/state.rs index 879a12df87..f43ff30fc1 100644 --- a/src/state.rs +++ b/src/state.rs @@ -88,6 +88,19 @@ impl Upstream { .chain(std::iter::once(self.workload.identity())) .collect() } + + pub fn service_sans(&self) -> Vec { + self.service_sans + .iter() + .flat_map(|san| match Identity::from_str(san) { + Ok(id) => Some(id), + Err(err) => { + warn!("ignoring invalid SAN {}: {}", san, err); + None + } + }) + .collect() + } } // Workload information that a specific proxy instance represents. This is used to cross check From 9f5609dff657e6a6e8c4134fc86bfd09cd07195b Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Wed, 5 Mar 2025 13:08:29 -0500 Subject: [PATCH 09/29] checkpoint --- examples/localhost.yaml | 44 +++++--------- src/proxy/inbound.rs | 2 +- src/proxy/metrics.rs | 8 +-- src/proxy/outbound.rs | 128 +++++++++++++++++++++++++++++----------- src/proxy/pool.rs | 95 +++++------------------------ src/state.rs | 45 +++++++++++--- src/state/service.rs | 6 ++ src/state/workload.rs | 1 - 8 files changed, 168 insertions(+), 161 deletions(-) diff --git a/examples/localhost.yaml b/examples/localhost.yaml index c089fe856c..a4a73d1c98 100644 --- a/examples/localhost.yaml +++ b/examples/localhost.yaml @@ -2,52 +2,36 @@ # This allows local testing by sending requests through the local ztunnel to other servers running on localhost. workloads: - uid: cluster1//v1/Pod/default/local - name: local + name: local2 namespace: default serviceAccount: default workloadIps: ["127.0.0.1"] protocol: HBONE node: local network: "" - services: - "default/example.com": - 80: 8081 + services: {} # Define another local address, but this one uses TCP. This allows testing HBONE and TCP with one config. - uid: cluster1//v1/Pod/default/local-tcp - name: local-tcp + name: local namespace: default serviceAccount: default - workloadIps: ["172.19.0.1"] - protocol: DOUBLEHBONE - node: remote + protocol: HBONE + node: local network: "" + networkGateway: + destination: "/172.19.0.1" + hboneMtlsPort: 15008 + workloadIps: [] services: - "default/example2.com": - 80: 8081 - "default/example3.com": - 80: 8081 - + "default/google.com": + 80: 80 services: -- name: local +- name: google namespace: default - hostname: example.com - vips: - - /127.10.0.1 - ports: - 80: 8081 + hostname: google.com subjectAltNames: - - spiffe://cluster.local/ns/default/sa/local -- name: remote - namespace: default - hostname: example2.com + - spiffe:///ns/default/sa/default vips: - /1.2.3.4 ports: 80: 8081 -- name: remote - namespace: default - hostname: example3.com - vips: - - /1.1.1.1 - ports: - 80: 8081 diff --git a/src/proxy/inbound.rs b/src/proxy/inbound.rs index 596a4de2b0..ede947a030 100644 --- a/src/proxy/inbound.rs +++ b/src/proxy/inbound.rs @@ -346,7 +346,7 @@ impl Inbound { // For consistency with outbound logs, report the original destination (with 15008 port) // as dst.addr, and the target address as dst.hbone_addr original_dst, - Some(hbone_addr), + Some(hbone_addr.to_string()), start, ConnectionOpen { reporter: Reporter::destination, diff --git a/src/proxy/metrics.rs b/src/proxy/metrics.rs index 5f36783e40..0e378c7ae8 100644 --- a/src/proxy/metrics.rs +++ b/src/proxy/metrics.rs @@ -345,7 +345,7 @@ pub struct ConnectionResult { src: (SocketAddr, Option), // Dst address and name dst: (SocketAddr, Option), - hbone_target: Option, + hbone_target: Option, start: Instant, // TODO: storing CommonTrafficLabels adds ~600 bytes retained throughout a connection life time. @@ -427,7 +427,7 @@ impl ConnectionResult { dst: SocketAddr, // If using hbone, the inner HBONE address // That is, dst is the L4 address, while is the :authority. - hbone_target: Option, + hbone_target: Option, start: Instant, conn: ConnectionOpen, metrics: Arc, @@ -456,7 +456,7 @@ impl ConnectionResult { src.identity = tl.source_principal.as_ref().filter(|_| mtls).map(to_value_owned), dst.addr = %dst.0, - dst.hbone_addr = hbone_target.map(display), + dst.hbone_addr = hbone_target.as_ref().map(display), dst.service = tl.destination_service.to_value(), dst.workload = dst.1.as_deref().map(to_value), dst.namespace = tl.destination_workload_namespace.to_value(), @@ -550,7 +550,7 @@ impl ConnectionResult { src.identity = tl.source_principal.as_ref().filter(|_| mtls).map(to_value_owned), dst.addr = %self.dst.0, - dst.hbone_addr = self.hbone_target.map(display), + dst.hbone_addr = self.hbone_target, dst.service = tl.destination_service.to_value(), dst.workload = self.dst.1.as_deref().map(to_value), dst.namespace = tl.destination_workload_namespace.to_value(), diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index 23daa919a9..ae6bd1c5f3 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -177,6 +177,7 @@ impl OutboundConnection { return; } }; + // At this point, we want all the addresses resolved // TODO: should we use the original address or the actual address? Both seems nice! let _conn_guard = self.pi.connection_manager.track_outbound( source_addr, @@ -185,27 +186,32 @@ impl OutboundConnection { ); let metrics = self.pi.metrics.clone(); - let hbone_target = req.hbone_target_destination; + let hbone_target = &req.hbone_target_destination; let result_tracker = Box::new(ConnectionResult::new( source_addr, req.actual_destination, - hbone_target, + hbone_target.clone(), start, Self::conn_metrics_from_request(&req), metrics, )); - let res = match req.protocol { - Protocol::HBONE => { - self.proxy_to_hbone(source_stream, source_addr, &req, &result_tracker) + let res = match ( + req.protocol, + req.actual_destination_workload + .as_ref() + .map(|wl| &wl.network_gateway), + ) { + (_, Some(_)) => { + self.proxy_to_double_hbone(source_stream, source_addr, &req, &result_tracker) .await } - Protocol::TCP => { - self.proxy_to_tcp(source_stream, &req, &result_tracker) + (Protocol::HBONE, _) => { + self.proxy_to_hbone(source_stream, source_addr, &req, &result_tracker) .await } - Protocol::DOUBLEHBONE => { - self.proxy_to_double_hbone(source_stream, source_addr, &req, &result_tracker) + (Protocol::TCP, _) => { + self.proxy_to_tcp(source_stream, &req, &result_tracker) .await } }; @@ -220,17 +226,60 @@ impl OutboundConnection { connection_stats: &ConnectionResult, ) -> Result<(), Error> { // TODO: dst should take a hostname? and upstream_sans currently contains E/W Gateway certs - let inner_workload = pool::WorkloadKey { + + // we need two connection requests, one for the outer and one for the inner. + // The outer should have the network gateway as the destination. + // + let svc = req + .intended_destination_service + .as_ref() + .expect("Workloads with network gateways must be service addressed."); + let svc_uri = format!("{}.{}:{}", svc.namespace, svc.hostname, 80); + let mut outer_req = req.clone(); + let network_gateway = req + .actual_destination_workload + .clone() + .unwrap() + .network_gateway + .clone() + .unwrap(); + + outer_req.actual_destination = SocketAddr::new( + match network_gateway.destination { + crate::state::workload::gatewayaddress::Destination::Address(network_address) => { + network_address.address + } + crate::state::workload::gatewayaddress::Destination::Hostname( + _namespaced_hostname, + ) => todo!(), + }, + network_gateway.hbone_mtls_port, + ); + + let upgraded = + Box::pin(self.send_hbone_request(remote_addr, &outer_req, svc_uri.clone())).await?; + + // For the inner one, we do it manually to avoid connection pool shenanigans. + let inner_workload = WorkloadKey { src_id: req.source.identity(), dst_id: req.upstream_sans.clone(), src: remote_addr.ip(), dst: req.actual_destination, }; - let request = self.create_hbone_request(remote_addr, req); + + let http_request = self.create_hbone_request(remote_addr, req, svc_uri.clone()); + // FIXME + // *http_request.uri_mut() = Uri::from_str( + // req.intended_destination_service + // .as_ref() + // .unwrap() + // .hostname + // .as_str(), + // ) + // .unwrap(); let workload_key = &inner_workload; // To shut down inner HTTP2 connection - let (drain_tx, drain_rx) = tokio::sync::watch::channel(false); let key = workload_key.clone(); let dest = rustls::pki_types::ServerName::IpAddress(key.dst.ip().into()); let cert = self @@ -242,18 +291,18 @@ impl OutboundConnection { // Do actual IO as late as possible // Outer HBONE - let upgraded = Box::pin(self.send_hbone_request(remote_addr, req)).await?; + // + // + let (drain_tx, drain_rx) = tokio::sync::watch::channel(false); + // Wrap upgraded to implement tokio's Async{Write,Read} let upgraded = TokioH2Stream::new(upgraded); let tls_stream = connector.connect(upgraded, dest).await?; - let (sender, driver_task) = - super::h2::client::spawn_connection(self.pi.cfg.clone(), tls_stream, drain_rx).await?; - let mut connection = super::pool::ConnClient { - sender, - wl_key: key, - }; - let inner_upgraded = connection.sender.send_request(request).await?; + let (mut sender, driver_task) = + super::h2::client::spawn_connection(self.pi.cfg.clone(), tls_stream, drain_rx, key) + .await?; + let inner_upgraded = sender.send_request(http_request).await?; let res = copy::copy_bidirectional( copy::TcpStreamSplitter(stream), inner_upgraded, @@ -278,7 +327,12 @@ impl OutboundConnection { req: &Request, connection_stats: &ConnectionResult, ) -> Result<(), Error> { - let upgraded = Box::pin(self.send_hbone_request(remote_addr, req)).await?; + let uri = req + .hbone_target_destination + .as_ref() + .expect("HBONE must have target"); + + let upgraded = Box::pin(self.send_hbone_request(remote_addr, req, uri.clone())).await?; copy::copy_bidirectional(copy::TcpStreamSplitter(stream), upgraded, connection_stats).await } @@ -286,10 +340,11 @@ impl OutboundConnection { &mut self, remote_addr: SocketAddr, req: &Request, + uri: String, ) -> http::Request<()> { http::Request::builder() .uri( - req.hbone_target_destination + req.hbone_target_destination.as_ref() .expect("HBONE must have target") .to_string(), ) @@ -309,9 +364,10 @@ impl OutboundConnection { &mut self, remote_addr: SocketAddr, req: &Request, + uri: String, ) -> Result { - let request = self.create_hbone_request(remote_addr, req); - let pool_key = Box::new(pool::WorkloadKey { + let request = self.create_hbone_request(remote_addr, req, uri); + let pool_key = Box::new(WorkloadKey { src_id: req.source.identity(), // Clone here shouldn't be needed ideally, we could just take ownership of Request. dst_id: req.upstream_sans.clone(), @@ -400,7 +456,7 @@ impl OutboundConnection { return Ok(Request { protocol: Protocol::HBONE, source: source_workload, - hbone_target_destination: Some(target), + hbone_target_destination: Some(target.to_string()), actual_destination_workload: Some(waypoint.workload), intended_destination_service: Some(ServiceDescription::from(&*target_service)), actual_destination, @@ -465,7 +521,7 @@ impl OutboundConnection { protocol: Protocol::HBONE, source: source_workload, // Use the original VIP, not translated - hbone_target_destination: Some(target), + hbone_target_destination: Some(target.to_string()), actual_destination_workload: Some(waypoint.workload), intended_destination_service: us.destination_service.clone(), actual_destination, @@ -478,19 +534,18 @@ impl OutboundConnection { // only change the port if we're sending HBONE let actual_destination = match us.workload.protocol { - Protocol::HBONE | Protocol::DOUBLEHBONE => { - SocketAddr::from((us.selected_workload_ip, self.hbone_port)) - } + Protocol::HBONE => SocketAddr::from(( + us.selected_workload_ip, + us.proxy_protocol_port_override.unwrap_or(self.hbone_port), + )), Protocol::TCP => us.workload_socket_addr(), }; let hbone_target_destination = match us.workload.protocol { - Protocol::HBONE | Protocol::DOUBLEHBONE => Some(us.workload_socket_addr()), + Protocol::HBONE => Some(us.hbone_target()), Protocol::TCP => None, }; - let (upstream_sans, final_sans) = match us.workload.protocol { - Protocol::DOUBLEHBONE => (vec![us.workload.identity()], us.service_sans()), - Protocol::TCP | Protocol::HBONE => (us.workload_and_services_san(), vec![]), - }; + // FIXME this seems wrong + let (upstream_sans, final_sans) = (us.workload_and_services_san(), us.service_sans()); // For case no waypoint for both side and direct to remote node proxy debug!("built request to workload"); @@ -531,7 +586,7 @@ fn baggage(r: &Request, cluster: String) -> String { ) } -#[derive(Debug)] +#[derive(Debug, Clone)] struct Request { protocol: Protocol, // Source workload sending the request @@ -549,7 +604,7 @@ struct Request { // When using HBONE, the `hbone_target_destination` is the inner :authority and `actual_destination` is the TCP destination. actual_destination: SocketAddr, // If using HBONE, the inner (:authority) of the HBONE request. - hbone_target_destination: Option, + hbone_target_destination: Option, // The identity we will assert for the next hop; this may not be the same as actual_destination_workload // in the case of proxies along the path. @@ -558,6 +613,7 @@ struct Request { // The identity of workload that will ultimately process this request. // This field only matters if we need to know both the identity of the next hop, as well as the // final hop (currently, this is only double HBONE). + // fixme is this even necessary? final_sans: Vec, } diff --git a/src/proxy/pool.rs b/src/proxy/pool.rs index cfc6fad29c..710f4c9871 100644 --- a/src/proxy/pool.rs +++ b/src/proxy/pool.rs @@ -82,19 +82,15 @@ impl ConnSpawner { key: WorkloadKey, stream: impl tokio::io::AsyncRead + tokio::io::AsyncWrite + Send + Unpin + 'static, driver_drain: tokio::sync::watch::Receiver, - ) -> Result<(ConnClient, tokio::task::JoinHandle<()>), Error> { + ) -> Result<(H2ConnectClient, tokio::task::JoinHandle<()>), Error> { let dest = rustls::pki_types::ServerName::IpAddress(key.dst.ip().into()); let cert = self.local_workload.fetch_certificate().await?; let connector = cert.outbound_connector(vec![])?; let tls_stream = connector.connect(stream, dest).await?; let (sender, driver_drain) = - h2::client::spawn_connection(self.cfg.clone(), tls_stream, driver_drain).await?; - let client = ConnClient { - sender, - wl_key: key, - }; - Ok((client, driver_drain)) + h2::client::spawn_connection(self.cfg.clone(), tls_stream, driver_drain, key).await?; + Ok((sender, driver_drain)) } async fn new_pool_conn(&self, key: WorkloadKey) -> Result { @@ -118,17 +114,7 @@ impl ConnSpawner { ); let tls_stream = connector.connect(tcp_stream, dest).await?; trace!("connector connected, handshaking"); -<<<<<<< HEAD - let (sender, _) = - h2::client::spawn_connection(self.cfg.clone(), tls_stream, self.timeout_rx.clone()) - .await?; - let client = ConnClient { - sender, - wl_key: key, - }; - Ok(client) -======= - let sender = h2::client::spawn_connection( + let (sender, _) = h2::client::spawn_connection( self.cfg.clone(), tls_stream, self.timeout_rx.clone(), @@ -136,7 +122,6 @@ impl ConnSpawner { ) .await?; Ok(sender) ->>>>>>> master } } @@ -411,7 +396,7 @@ impl WorkloadHBONEPool { request: http::Request<()>, ) -> Result< ( - ConnClient, + H2ConnectClient, Result, tokio::sync::watch::Sender, tokio::task::JoinHandle<()>, @@ -429,16 +414,17 @@ impl WorkloadHBONEPool { .await?; let connector = cert.outbound_connector(vec![])?; let tls_stream = connector.connect(stream, dest).await?; - let (sender, driver_drain) = - h2::client::spawn_connection(self.state.spawner.cfg.clone(), tls_stream, rx).await?; - let mut connection = ConnClient { - sender, - wl_key: key, - }; + let (mut sender, driver_drain) = h2::client::spawn_connection( + self.state.spawner.cfg.clone(), + tls_stream, + rx, + workload_key.clone(), + ) + .await?; Ok(( - connection.clone(), - connection.sender.send_request(request).await, + sender.clone(), + sender.send_request(request).await, tx, driver_drain, )) @@ -584,59 +570,6 @@ impl WorkloadHBONEPool { } } -<<<<<<< HEAD -#[derive(Debug, Clone)] -// A sort of faux-client, that represents a single checked-out 'request sender' which might -// send requests over some underlying stream using some underlying http/2 client -pub struct ConnClient { - pub sender: H2ConnectClient, - // A WL key may have many clients, but every client has no more than one WL key - pub wl_key: WorkloadKey, // the WL key associated with this client. -} - -impl ConnClient { - pub fn is_for_workload(&self, wl_key: &WorkloadKey) -> Result<(), crate::proxy::Error> { - if !(self.wl_key == *wl_key) { - Err(crate::proxy::Error::Generic( - "fetched connection does not match workload key!".into(), - )) - } else { - Ok(()) - } - } -} - -// This is currently only for debugging -impl Drop for ConnClient { - fn drop(&mut self) { - trace!("dropping ConnClient for key {}", self.wl_key,) - } -} - -#[derive(PartialEq, Eq, Hash, Clone, Debug)] -pub struct WorkloadKey { - pub src_id: Identity, - pub dst_id: Vec, - // In theory we can just use src,dst,node. However, the dst has a check that - // the L3 destination IP matches the HBONE IP. This could be loosened to just assert they are the same identity maybe. - pub dst: SocketAddr, - // Because we spoof the source IP, we need to key on this as well. Note: for in-pod its already per-pod - // pools anyways. - pub src: IpAddr, -} - -impl Display for WorkloadKey { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!(f, "{}({})->{}[", self.src, &self.src_id, self.dst,)?; - for i in &self.dst_id { - write!(f, "{i}")?; - } - write!(f, "]") - } -} - -======= ->>>>>>> master #[cfg(test)] mod test { use std::convert::Infallible; diff --git a/src/state.rs b/src/state.rs index 4588e96724..543a1b6f83 100644 --- a/src/state.rs +++ b/src/state.rs @@ -70,9 +70,23 @@ pub struct Upstream { pub service_sans: Vec, /// If this was from a service, the service info. pub destination_service: Option, + /// Use this port instead of the default one for the proxy protocol. + pub proxy_protocol_port_override: Option, } impl Upstream { + /// If there is a network gateway, use :. + /// Otherwise, use : + /// Fortunately, for double-hbone, the authority/host is the same for inner and outer + /// Connect request. + pub fn hbone_target(&self) -> String { + if let Some(_) = self.workload.network_gateway.as_ref() { + let svc = self.destination_service.as_ref().expect("Workloads with network gateways must be service addressed."); + format!("{}.{}:{}", svc.namespace, svc.hostname, self.port) + } else { + self.workload_socket_addr().to_string() + } + } pub fn workload_socket_addr(&self) -> SocketAddr { SocketAddr::new(self.selected_workload_ip, self.port) } @@ -767,20 +781,35 @@ impl DemandProxyState { }; let svc_desc = svc.clone().map(|s| ServiceDescription::from(s.as_ref())); let ip_family_restriction = svc.as_ref().and_then(|s| s.ip_families); - let selected_workload_ip = self - .pick_workload_destination_or_resolve( - &wl, - source_workload, - original_target_address, - ip_family_restriction, - ) - .await?; // if we can't load balance just return the error + let (selected_workload_ip, proxy_protocol_port_override) = + if let Some(network_gateway) = wl.network_gateway.as_ref() { + match &network_gateway.destination { + Destination::Address(network_address) => ( + network_address.address, + Some(network_gateway.hbone_mtls_port), + ), + Destination::Hostname(_) => todo!(), + } + } else { + ( + self.pick_workload_destination_or_resolve( + &wl, + source_workload, + original_target_address, + ip_family_restriction, + ) + .await?, + None, + ) // if we can't load balance just return the error + }; + let res = Upstream { workload: wl, selected_workload_ip, port, service_sans: svc.map(|s| s.subject_alt_names.clone()).unwrap_or_default(), destination_service: svc_desc, + proxy_protocol_port_override, }; tracing::trace!(?res, "finalize_upstream"); Ok(Some(res)) diff --git a/src/state/service.rs b/src/state/service.rs index 77e9168449..e6eba989f9 100644 --- a/src/state/service.rs +++ b/src/state/service.rs @@ -249,6 +249,12 @@ pub struct ServiceDescription { pub namespace: Strng, } +impl ServiceDescription { + pub fn uri(&self) -> String { + format!("https://{}.{}:80", self.namespace, self.hostname) + } +} + impl From<&Service> for ServiceDescription { fn from(value: &Service) -> Self { ServiceDescription { diff --git a/src/state/workload.rs b/src/state/workload.rs index 9805c813d1..cadce057fb 100644 --- a/src/state/workload.rs +++ b/src/state/workload.rs @@ -48,7 +48,6 @@ pub enum Protocol { #[default] TCP, HBONE, - DOUBLEHBONE, } impl From for Protocol { From 54dd3e301037620cbe3d2fe822ce3b959e99656b Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Wed, 5 Mar 2025 14:10:32 -0500 Subject: [PATCH 10/29] another checkpoint --- examples/localhost.yaml | 2 +- src/proxy/outbound.rs | 106 +++++++++++----------------------------- src/state.rs | 9 ++-- 3 files changed, 35 insertions(+), 82 deletions(-) diff --git a/examples/localhost.yaml b/examples/localhost.yaml index a4a73d1c98..646bfc9c52 100644 --- a/examples/localhost.yaml +++ b/examples/localhost.yaml @@ -20,7 +20,7 @@ workloads: network: "" networkGateway: destination: "/172.19.0.1" - hboneMtlsPort: 15008 + hboneMtlsPort: 15009 workloadIps: [] services: "default/google.com": diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index ae6bd1c5f3..871c8fe40a 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -225,84 +225,44 @@ impl OutboundConnection { req: &Request, connection_stats: &ConnectionResult, ) -> Result<(), Error> { - // TODO: dst should take a hostname? and upstream_sans currently contains E/W Gateway certs - - // we need two connection requests, one for the outer and one for the inner. - // The outer should have the network gateway as the destination. - // - let svc = req - .intended_destination_service - .as_ref() - .expect("Workloads with network gateways must be service addressed."); - let svc_uri = format!("{}.{}:{}", svc.namespace, svc.hostname, 80); - let mut outer_req = req.clone(); - let network_gateway = req - .actual_destination_workload - .clone() - .unwrap() - .network_gateway - .clone() - .unwrap(); - - outer_req.actual_destination = SocketAddr::new( - match network_gateway.destination { - crate::state::workload::gatewayaddress::Destination::Address(network_address) => { - network_address.address - } - crate::state::workload::gatewayaddress::Destination::Hostname( - _namespaced_hostname, - ) => todo!(), - }, - network_gateway.hbone_mtls_port, - ); - - let upgraded = - Box::pin(self.send_hbone_request(remote_addr, &outer_req, svc_uri.clone())).await?; + // Create the outer HBONE stream + let upgraded = Box::pin(self.send_hbone_request(remote_addr, &req)).await?; + // Wrap upgraded to implement tokio's Async{Write,Read} + let upgraded = TokioH2Stream::new(upgraded); - // For the inner one, we do it manually to avoid connection pool shenanigans. - let inner_workload = WorkloadKey { + // For the inner one, we do it manually to avoid connection pooling. + // Otherwise, we would only ever reach one workload in the remote cluster. + // We also need to abort tasks the right way to get graceful terminations. + let wl_key = WorkloadKey { src_id: req.source.identity(), - dst_id: req.upstream_sans.clone(), + dst_id: req.final_sans.clone(), src: remote_addr.ip(), dst: req.actual_destination, }; - let http_request = self.create_hbone_request(remote_addr, req, svc_uri.clone()); - // FIXME - // *http_request.uri_mut() = Uri::from_str( - // req.intended_destination_service - // .as_ref() - // .unwrap() - // .hostname - // .as_str(), - // ) - // .unwrap(); - let workload_key = &inner_workload; - - // To shut down inner HTTP2 connection - let key = workload_key.clone(); - let dest = rustls::pki_types::ServerName::IpAddress(key.dst.ip().into()); + // Fetch certs and establish inner TLS connection. let cert = self .pi .local_workload_information .fetch_certificate() .await?; - let connector = cert.outbound_connector(req.final_sans.clone())?; + let connector = cert.outbound_connector(wl_key.dst_id.clone())?; + let tls_stream = connector + .connect( + upgraded, + rustls::pki_types::ServerName::IpAddress(wl_key.dst.ip().into()), + ) + .await?; - // Do actual IO as late as possible - // Outer HBONE - // - // + // Spawn inner CONNECT tunnel let (drain_tx, drain_rx) = tokio::sync::watch::channel(false); - - // Wrap upgraded to implement tokio's Async{Write,Read} - let upgraded = TokioH2Stream::new(upgraded); - let tls_stream = connector.connect(upgraded, dest).await?; - let (mut sender, driver_task) = - super::h2::client::spawn_connection(self.pi.cfg.clone(), tls_stream, drain_rx, key) + super::h2::client::spawn_connection(self.pi.cfg.clone(), tls_stream, drain_rx, wl_key) .await?; + let http_request = self.create_hbone_request(remote_addr, req); let inner_upgraded = sender.send_request(http_request).await?; + + // Proxy let res = copy::copy_bidirectional( copy::TcpStreamSplitter(stream), inner_upgraded, @@ -315,8 +275,7 @@ impl OutboundConnection { // Here, there is an implicit, drop(conn_client). // Its really important that this happens AFTER driver_task finishes. // Otherwise, TLS connections do not terminate gracefully. - // - // drop(conn_client); + res } @@ -327,12 +286,7 @@ impl OutboundConnection { req: &Request, connection_stats: &ConnectionResult, ) -> Result<(), Error> { - let uri = req - .hbone_target_destination - .as_ref() - .expect("HBONE must have target"); - - let upgraded = Box::pin(self.send_hbone_request(remote_addr, req, uri.clone())).await?; + let upgraded = Box::pin(self.send_hbone_request(remote_addr, req)).await?; copy::copy_bidirectional(copy::TcpStreamSplitter(stream), upgraded, connection_stats).await } @@ -340,11 +294,11 @@ impl OutboundConnection { &mut self, remote_addr: SocketAddr, req: &Request, - uri: String, ) -> http::Request<()> { http::Request::builder() .uri( - req.hbone_target_destination.as_ref() + req.hbone_target_destination + .as_ref() .expect("HBONE must have target") .to_string(), ) @@ -364,9 +318,8 @@ impl OutboundConnection { &mut self, remote_addr: SocketAddr, req: &Request, - uri: String, ) -> Result { - let request = self.create_hbone_request(remote_addr, req, uri); + let request = self.create_hbone_request(remote_addr, req); let pool_key = Box::new(WorkloadKey { src_id: req.source.identity(), // Clone here shouldn't be needed ideally, we could just take ownership of Request. @@ -747,9 +700,8 @@ mod tests { Some(ExpectedRequest { protocol: r.protocol, hbone_destination: &r - .hbone_target_destination - .map(|s| s.to_string()) - .unwrap_or_default(), + .hbone_target_destination.as_ref() + .unwrap_or(&String::new()), destination: &r.actual_destination.to_string(), }) ); diff --git a/src/state.rs b/src/state.rs index 543a1b6f83..f174bdf439 100644 --- a/src/state.rs +++ b/src/state.rs @@ -75,10 +75,10 @@ pub struct Upstream { } impl Upstream { - /// If there is a network gateway, use :. - /// Otherwise, use : - /// Fortunately, for double-hbone, the authority/host is the same for inner and outer - /// Connect request. + /// If there is a network gateway, use :. Otherwise, use + /// :. Fortunately, for double-hbone, the authority/host is the same + /// for inner and outer CONNECT request, so we can reuse this for inner double hbone, + /// outer double hbone, and normal hbone. pub fn hbone_target(&self) -> String { if let Some(_) = self.workload.network_gateway.as_ref() { let svc = self.destination_service.as_ref().expect("Workloads with network gateways must be service addressed."); @@ -87,6 +87,7 @@ impl Upstream { self.workload_socket_addr().to_string() } } + pub fn workload_socket_addr(&self) -> SocketAddr { SocketAddr::new(self.selected_workload_ip, self.port) } From 58399be2a18a16d00e390ff8546da3a906e567f9 Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Thu, 6 Mar 2025 22:07:51 -0500 Subject: [PATCH 11/29] Use new xds --- examples/localhost.yaml | 48 ++++++++++++++++------- src/config.rs | 2 +- src/proxy/metrics.rs | 2 +- src/proxy/outbound.rs | 3 +- src/proxy/pool.rs | 57 --------------------------- src/proxy/socks5.rs | 2 +- src/state.rs | 21 ++++++++-- src/state/service.rs | 6 --- src/test_helpers/linux.rs | 28 ++++++++++++- src/test_helpers/tcp.rs | 37 +++++++++++------- src/tls/certificate.rs | 19 +-------- src/tls/workload.rs | 2 - tests/namespaced.rs | 82 +++++++++++++++++++++++++++++++++++---- 13 files changed, 180 insertions(+), 129 deletions(-) diff --git a/examples/localhost.yaml b/examples/localhost.yaml index 646bfc9c52..a75800680b 100644 --- a/examples/localhost.yaml +++ b/examples/localhost.yaml @@ -2,36 +2,54 @@ # This allows local testing by sending requests through the local ztunnel to other servers running on localhost. workloads: - uid: cluster1//v1/Pod/default/local - name: local2 + name: local namespace: default serviceAccount: default workloadIps: ["127.0.0.1"] protocol: HBONE node: local network: "" - services: {} + services: + "default/example.com": + 80: 8080 + "default/example2.com": + 80: 8080 # Define another local address, but this one uses TCP. This allows testing HBONE and TCP with one config. - uid: cluster1//v1/Pod/default/local-tcp - name: local + name: local-tcp namespace: default serviceAccount: default - protocol: HBONE + workloadIps: ["127.0.0.2"] + protocol: TCP node: local network: "" - networkGateway: - destination: "/172.19.0.1" - hboneMtlsPort: 15009 - workloadIps: [] services: - "default/google.com": - 80: 80 + "default/example.com": + 80: 8080 + "default/example2.com": + 80: 8080 +policies: + - action: Allow + rules: + - - - notDestinationPorts: + - 9999 + name: deny-9999 + namespace: default + scope: Namespace services: -- name: google +- name: local namespace: default - hostname: google.com + hostname: example.com + vips: + - /127.10.0.1 + ports: + 80: 8080 subjectAltNames: - - spiffe:///ns/default/sa/default + - spiffe://cluster.local/ns/default/sa/local +- name: remote + namespace: default + hostname: example2.com vips: - - /1.2.3.4 + - remote/127.10.0.2 ports: - 80: 8081 + 80: 8080 diff --git a/src/config.rs b/src/config.rs index de8797607c..790a0a8077 100644 --- a/src/config.rs +++ b/src/config.rs @@ -72,7 +72,7 @@ const IPV6_ENABLED: &str = "IPV6_ENABLED"; const UNSTABLE_ENABLE_SOCKS5: &str = "UNSTABLE_ENABLE_SOCKS5"; -const DEFAULT_WORKER_THREADS: u16 = 40; +const DEFAULT_WORKER_THREADS: u16 = 2; const DEFAULT_ADMIN_PORT: u16 = 15000; const DEFAULT_READINESS_PORT: u16 = 15021; const DEFAULT_STATS_PORT: u16 = 15020; diff --git a/src/proxy/metrics.rs b/src/proxy/metrics.rs index 0e378c7ae8..187db39cef 100644 --- a/src/proxy/metrics.rs +++ b/src/proxy/metrics.rs @@ -456,7 +456,7 @@ impl ConnectionResult { src.identity = tl.source_principal.as_ref().filter(|_| mtls).map(to_value_owned), dst.addr = %dst.0, - dst.hbone_addr = hbone_target.as_ref().map(display), + dst.hbone_addr = hbone_target.as_ref(), dst.service = tl.destination_service.to_value(), dst.workload = dst.1.as_deref().map(to_value), dst.namespace = tl.destination_workload_namespace.to_value(), diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index 871c8fe40a..d3ea7bdd20 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -196,11 +196,12 @@ impl OutboundConnection { metrics, )); + let res = match ( req.protocol, req.actual_destination_workload .as_ref() - .map(|wl| &wl.network_gateway), + .and_then(|wl| wl.network_gateway.as_ref()), ) { (_, Some(_)) => { self.proxy_to_double_hbone(source_stream, source_addr, &req, &result_tracker) diff --git a/src/proxy/pool.rs b/src/proxy/pool.rs index 710f4c9871..87999d51e9 100644 --- a/src/proxy/pool.rs +++ b/src/proxy/pool.rs @@ -77,22 +77,6 @@ struct ConnSpawner { // Does nothing but spawn new conns when asked impl ConnSpawner { - async fn new_unpooled_conn( - &self, - key: WorkloadKey, - stream: impl tokio::io::AsyncRead + tokio::io::AsyncWrite + Send + Unpin + 'static, - driver_drain: tokio::sync::watch::Receiver, - ) -> Result<(H2ConnectClient, tokio::task::JoinHandle<()>), Error> { - let dest = rustls::pki_types::ServerName::IpAddress(key.dst.ip().into()); - - let cert = self.local_workload.fetch_certificate().await?; - let connector = cert.outbound_connector(vec![])?; - let tls_stream = connector.connect(stream, dest).await?; - let (sender, driver_drain) = - h2::client::spawn_connection(self.cfg.clone(), tls_stream, driver_drain, key).await?; - Ok((sender, driver_drain)) - } - async fn new_pool_conn(&self, key: WorkloadKey) -> Result { debug!("spawning new pool conn for {}", key); @@ -389,47 +373,6 @@ impl WorkloadHBONEPool { } } - pub async fn send_request_unpooled( - &mut self, - stream: impl tokio::io::AsyncWrite + tokio::io::AsyncRead + Send + Unpin + 'static, - workload_key: &WorkloadKey, - request: http::Request<()>, - ) -> Result< - ( - H2ConnectClient, - Result, - tokio::sync::watch::Sender, - tokio::task::JoinHandle<()>, - ), - Error, - > { - let (tx, rx) = tokio::sync::watch::channel(false); - let key = workload_key.clone(); - let dest = rustls::pki_types::ServerName::IpAddress(key.dst.ip().into()); - let cert = self - .state - .spawner - .local_workload - .fetch_certificate() - .await?; - let connector = cert.outbound_connector(vec![])?; - let tls_stream = connector.connect(stream, dest).await?; - let (mut sender, driver_drain) = h2::client::spawn_connection( - self.state.spawner.cfg.clone(), - tls_stream, - rx, - workload_key.clone(), - ) - .await?; - - Ok(( - sender.clone(), - sender.send_request(request).await, - tx, - driver_drain, - )) - } - pub async fn send_request_pooled( &mut self, workload_key: &WorkloadKey, diff --git a/src/proxy/socks5.rs b/src/proxy/socks5.rs index 905f6dfd07..3e7d8f4771 100644 --- a/src/proxy/socks5.rs +++ b/src/proxy/socks5.rs @@ -107,7 +107,7 @@ impl Socks5 { debug!(component="socks5", dur=?start.elapsed(), "connection completed"); }).instrument(span); - assertions::size_between_ref(1000, 2000, &serve); + assertions::size_between_ref(1000, 10000, &serve); tokio::spawn(serve); } Err(e) => { diff --git a/src/state.rs b/src/state.rs index f174bdf439..c47fca4901 100644 --- a/src/state.rs +++ b/src/state.rs @@ -80,8 +80,11 @@ impl Upstream { /// for inner and outer CONNECT request, so we can reuse this for inner double hbone, /// outer double hbone, and normal hbone. pub fn hbone_target(&self) -> String { - if let Some(_) = self.workload.network_gateway.as_ref() { - let svc = self.destination_service.as_ref().expect("Workloads with network gateways must be service addressed."); + if let Some(_) = self.workload.network_gateway.as_ref() { + let svc = self + .destination_service + .as_ref() + .expect("Workloads with network gateways must be service addressed."); format!("{}.{}:{}", svc.namespace, svc.hostname, self.port) } else { self.workload_socket_addr().to_string() @@ -650,7 +653,13 @@ impl DemandProxyState { original_target_address: SocketAddr, ) -> Result { let workload_uid = workload.uid.clone(); - let hostname = workload.hostname.clone(); + // FIXME(stevenjin8) Throw a error if this a network gateway without a hostname? + let hostname = match workload.network_gateway.as_ref().map(|g| g.destination.clone()) { + Some(Destination::Hostname(hostname)) => hostname.hostname.clone(), + _ => workload.hostname.clone(), + }; + + // = workload.hostname.clone(); trace!(%hostname, "starting DNS lookup"); let resp = match self.dns_resolver.lookup_ip(hostname.as_str()).await { @@ -789,7 +798,11 @@ impl DemandProxyState { network_address.address, Some(network_gateway.hbone_mtls_port), ), - Destination::Hostname(_) => todo!(), + Destination::Hostname(_) => ( + self.resolve_workload_address(&wl, source_workload, original_target_address) + .await?, + Some(network_gateway.hbone_mtls_port), + ), } } else { ( diff --git a/src/state/service.rs b/src/state/service.rs index e6eba989f9..77e9168449 100644 --- a/src/state/service.rs +++ b/src/state/service.rs @@ -249,12 +249,6 @@ pub struct ServiceDescription { pub namespace: Strng, } -impl ServiceDescription { - pub fn uri(&self) -> String { - format!("https://{}.{}:80", self.namespace, self.hostname) - } -} - impl From<&Service> for ServiceDescription { fn from(value: &Service) -> Self { ServiceDescription { diff --git a/src/test_helpers/linux.rs b/src/test_helpers/linux.rs index eb90d76d00..f9ef1624cb 100644 --- a/src/test_helpers/linux.rs +++ b/src/test_helpers/linux.rs @@ -26,6 +26,7 @@ use crate::inpod::istio::zds::WorkloadInfo; use crate::signal::ShutdownTrigger; use crate::test_helpers::inpod::start_ztunnel_server; use crate::test_helpers::linux::TestMode::{Dedicated, Shared}; +use arcstr::ArcStr; use itertools::Itertools; use nix::unistd::mkdtemp; use std::net::IpAddr; @@ -350,6 +351,11 @@ impl<'a> TestServiceBuilder<'a> { self } + pub fn subject_alt_names(mut self, mut sans: Vec) -> Self { + self.s.subject_alt_names.append(&mut sans); + self + } + /// Set the service waypoint pub fn waypoint(mut self, waypoint: IpAddr) -> Self { self.s.waypoint = Some(GatewayAddress { @@ -387,6 +393,7 @@ impl<'a> TestServiceBuilder<'a> { pub struct TestWorkloadBuilder<'a> { w: LocalWorkload, captured: bool, + is_network_gateway: bool, manager: &'a mut WorkloadManager, } @@ -394,6 +401,7 @@ impl<'a> TestWorkloadBuilder<'a> { pub fn new(name: &str, manager: &'a mut WorkloadManager) -> TestWorkloadBuilder<'a> { TestWorkloadBuilder { captured: false, + is_network_gateway: false, w: LocalWorkload { workload: Workload { name: name.into(), @@ -453,12 +461,17 @@ impl<'a> TestWorkloadBuilder<'a> { self } - /// Set a waypoint to the workload + /// Mutate the workload pub fn mutate_workload(mut self, f: impl FnOnce(&mut Workload)) -> Self { f(&mut self.w.workload); self } + pub fn network_gateway(mut self) -> Self { + self.is_network_gateway = true; + self + } + /// Append a service to the workload pub fn service(mut self, service: &str, server_port: u16, target_port: u16) -> Self { self.w @@ -505,7 +518,18 @@ impl<'a> TestWorkloadBuilder<'a> { .namespaces .child(&self.w.workload.node, &self.w.workload.name)? }; - self.w.workload.workload_ips = vec![network_namespace.ip()]; + if self.is_network_gateway { + self.w.workload.workload_ips = vec![]; + self.w.workload.network_gateway = Some(GatewayAddress { + destination: gatewayaddress::Destination::Address(NetworkAddress { + network: "".into(), + address: network_namespace.ip(), + }), + hbone_mtls_port: 15008, // FIXME + }) + } else { + self.w.workload.workload_ips = vec![network_namespace.ip()]; + } self.w.workload.uid = format!( "cluster1//v1/Pod/{}/{}", self.w.workload.namespace, self.w.workload.name, diff --git a/src/test_helpers/tcp.rs b/src/test_helpers/tcp.rs index d59317eb90..77f7cfc2e2 100644 --- a/src/test_helpers/tcp.rs +++ b/src/test_helpers/tcp.rs @@ -222,16 +222,19 @@ pub struct HboneTestServer { listener: TcpListener, mode: Mode, name: String, + /// Write this message when acting as waypoint to show that waypoint was hit. + waypoint_message: Vec, } impl HboneTestServer { - pub async fn new(mode: Mode, name: &str) -> Self { + pub async fn new(mode: Mode, name: &str, waypoint_message: Vec) -> Self { let addr = SocketAddr::new(IpAddr::V6(Ipv6Addr::UNSPECIFIED), 15008); let listener = TcpListener::bind(addr).await.unwrap(); Self { listener, mode, name: name.to_string(), + waypoint_message, } } @@ -254,24 +257,28 @@ impl HboneTestServer { let mut tls_stream = crate::hyper_util::tls_server(acceptor, self.listener); let mode = self.mode; while let Some(socket) = tls_stream.next().await { + let waypoint_message = self.waypoint_message.clone(); if let Err(err) = http2::Builder::new(TokioExecutor) .serve_connection( TokioIo::new(socket), - service_fn(move |req| async move { - info!("waypoint: received request"); - tokio::task::spawn(async move { - match hyper::upgrade::on(req).await { - Ok(upgraded) => { - let mut io = TokioIo::new(upgraded); - // let (mut ri, mut wi) = tokio::io::split(TokioIo::new(upgraded)); - // Signal we are the waypoint so tests can validate this - io.write_all(b"waypoint\n").await.unwrap(); - handle_stream(mode, &mut io).await; + service_fn(move |req| { + let waypoint_message = waypoint_message.clone(); + async move { + info!("waypoint: received request"); + tokio::task::spawn(async move { + match hyper::upgrade::on(req).await { + Ok(upgraded) => { + let mut io = TokioIo::new(upgraded); + // let (mut ri, mut wi) = tokio::io::split(TokioIo::new(upgraded)); + // Signal we are the waypoint so tests can validate this + io.write_all(&waypoint_message[..]).await.unwrap(); + handle_stream(mode, &mut io).await; + } + Err(e) => error!("No upgrade {e}"), } - Err(e) => error!("No upgrade {e}"), - } - }); - Ok::<_, Infallible>(Response::new(Full::::from("streaming..."))) + }); + Ok::<_, Infallible>(Response::new(Full::::from("streaming..."))) + } }), ) .await diff --git a/src/tls/certificate.rs b/src/tls/certificate.rs index babc685869..8a1466a933 100644 --- a/src/tls/certificate.rs +++ b/src/tls/certificate.rs @@ -19,11 +19,10 @@ use bytes::Bytes; use itertools::Itertools; use rustls::client::Resumption; -use rustls::pki_types::pem::PemObject; use rustls::pki_types::{CertificateDer, PrivateKeyDer}; use rustls::server::WebPkiClientVerifier; -use rustls::{ClientConfig, KeyLogFile, RootCertStore, ServerConfig, server}; +use rustls::{ClientConfig, RootCertStore, ServerConfig, server}; use rustls_pemfile::Item; use std::io::Cursor; use std::str::FromStr; @@ -228,10 +227,6 @@ impl WorkloadCertificate { let mut roots = RootCertStore::empty(); roots.add_parsable_certificates(chain.iter().last().map(|c| c.der.clone())); - roots.add_parsable_certificates(vec![ - CertificateDer::from_pem_file("/home/sj/learning/openssl/c/root.crt").unwrap(), - ]); - Ok(WorkloadCertificate { cert, chain, @@ -256,12 +251,8 @@ impl WorkloadCertificate { let td = self.cert.identity().map(|i| match i { Identity::Spiffe { trust_domain, .. } => trust_domain, }); - let mut roots = (*self.roots).clone(); - roots.add_parsable_certificates(vec![ - CertificateDer::from_pem_file("/home/sj/learning/openssl/c/root.crt").unwrap(), - ]); let raw_client_cert_verifier = WebPkiClientVerifier::builder_with_provider( - Arc::new(roots), + self.roots.clone(), crate::tls::lib::provider(), ) .build()?; @@ -273,7 +264,6 @@ impl WorkloadCertificate { .expect("server config must be valid") .with_client_cert_verifier(client_cert_verifier) .with_single_cert(self.cert_and_intermediates(), self.private_key.clone_key())?; - sc.key_log = Arc::new(KeyLogFile::new()); sc.alpn_protocols = vec![b"h2".into()]; Ok(sc) } @@ -281,10 +271,6 @@ impl WorkloadCertificate { pub fn outbound_connector(&self, identity: Vec) -> Result { let roots = self.roots.clone(); let verifier = IdentityVerifier { roots, identity }; - let mut root_cert_store = RootCertStore::empty(); - root_cert_store.add_parsable_certificates(vec![ - CertificateDer::from_pem_file("/home/sj/learning/openssl/c/root.crt").unwrap(), - ]); let mut cc = ClientConfig::builder_with_provider(crate::tls::lib::provider()) .with_protocol_versions(tls::TLS_VERSIONS) .expect("client config must be valid") @@ -294,7 +280,6 @@ impl WorkloadCertificate { cc.alpn_protocols = vec![b"h2".into()]; cc.resumption = Resumption::disabled(); cc.enable_sni = false; - cc.key_log = Arc::new(KeyLogFile::new()); Ok(OutboundConnector { client_config: Arc::new(cc), }) diff --git a/src/tls/workload.rs b/src/tls/workload.rs index 6d63d01972..bd708a5896 100644 --- a/src/tls/workload.rs +++ b/src/tls/workload.rs @@ -275,7 +275,6 @@ impl ServerCertVerifier for IdentityVerifier { cert: &CertificateDer<'_>, dss: &DigitallySignedStruct, ) -> Result { - // Ok(HandshakeSignatureValid::assertion()) rustls::crypto::verify_tls12_signature( message, cert, @@ -290,7 +289,6 @@ impl ServerCertVerifier for IdentityVerifier { cert: &CertificateDer<'_>, dss: &DigitallySignedStruct, ) -> Result { - // Ok(HandshakeSignatureValid::assertion()) rustls::crypto::verify_tls13_signature( message, cert, diff --git a/tests/namespaced.rs b/tests/namespaced.rs index 081debc674..92f332c957 100644 --- a/tests/namespaced.rs +++ b/tests/namespaced.rs @@ -50,6 +50,8 @@ mod namespaced { use ztunnel::test_helpers::netns::{Namespace, Resolver}; use ztunnel::test_helpers::*; + const WAYPOINT_MESSAGE: &[u8] = b"waypoint\n"; + /// initialize_namespace_tests sets up the namespace tests. #[ctor::ctor] fn initialize_namespace_tests() { @@ -137,7 +139,12 @@ mod namespaced { let waypoint = manager.register_waypoint("waypoint", DEFAULT_NODE).await?; let waypoint_ip = waypoint.ip(); - run_hbone_server(waypoint, "waypoint")?; + run_hbone_server( + waypoint, + "waypoint", + tcp::Mode::ReadWrite, + WAYPOINT_MESSAGE.into(), + )?; manager .workload_builder("server", DEFAULT_NODE) @@ -187,6 +194,53 @@ mod namespaced { Ok(()) } + #[tokio::test] + async fn double_hbone() -> anyhow::Result<()> { + let mut manager = setup_netns_test!(Shared); + + let _zt = manager.deploy_ztunnel(DEFAULT_NODE).await?; + + manager + .service_builder("remote") + .addresses(vec![NetworkAddress { + network: strng::EMPTY, + address: TEST_VIP.parse::()?, + }]) + .subject_alt_names(vec!["spiffe://cluster.local/ns/default/sa/echo".into()]) + .ports(HashMap::from([(15008u16, 15008u16)])) + .register() + .await?; + + let ew_gtw = manager + .workload_builder("ew-gtw", "remote-node") + .hbone() + .network_gateway() + .service( + "default/remote.default.svc.cluster.local", + 15008u16, + 15008u16, + ) + .register() + .await?; + + let echo = manager + .workload_builder("echo", "remote-node2") + .register() + .await?; + + let client = manager + .workload_builder("client", DEFAULT_NODE) + .register() + .await?; + let echo_addr = SocketAddr::new(echo.ip(), 15008); + run_hbone_server(echo, "echo", tcp::Mode::ReadWrite, WAYPOINT_MESSAGE.into())?; + run_hbone_server(ew_gtw, "ew-gtw", tcp::Mode::Forward(echo_addr), b"".into())?; + + run_tcp_to_hbone_client(client, manager.resolver(), &format!("{TEST_VIP}:15008"))?; + + Ok(()) + } + #[tokio::test] async fn service_waypoint() -> anyhow::Result<()> { let mut manager = setup_netns_test!(Shared); @@ -195,7 +249,12 @@ mod namespaced { let waypoint = manager.register_waypoint("waypoint", DEFAULT_NODE).await?; let waypoint_ip = waypoint.ip(); - run_hbone_server(waypoint, "waypoint")?; + run_hbone_server( + waypoint, + "waypoint", + tcp::Mode::ReadWrite, + WAYPOINT_MESSAGE.into(), + )?; let client = manager .workload_builder("client", DEFAULT_NODE) @@ -275,7 +334,12 @@ mod namespaced { ) .register() .await?; - run_hbone_server(waypoint, "waypoint")?; + run_hbone_server( + waypoint, + "waypoint", + tcp::Mode::ReadWrite, + WAYPOINT_MESSAGE.into(), + )?; manager .workload_builder("server", DEFAULT_NODE) @@ -338,7 +402,7 @@ mod namespaced { .mutate_workload(|w| w.hostname = "waypoint.example.com".into()) .register() .await?; - run_hbone_server(waypoint, "waypoint")?; + run_hbone_server(waypoint, "waypoint", tcp::Mode::ReadWrite, WAYPOINT_MESSAGE.into())?; manager .workload_builder("server", DEFAULT_NODE) @@ -1416,10 +1480,15 @@ mod namespaced { } /// run_hbone_server deploys a simple echo server, deployed over HBONE, in the provided namespace - fn run_hbone_server(server: Namespace, name: &str) -> anyhow::Result<()> { + fn run_hbone_server( + server: Namespace, + name: &str, + mode: tcp::Mode, + waypoint_message: Vec, + ) -> anyhow::Result<()> { let name = name.to_string(); server.run_ready(move |ready| async move { - let echo = tcp::HboneTestServer::new(tcp::Mode::ReadWrite, &name).await; + let echo = tcp::HboneTestServer::new(mode, &name, waypoint_message).await; info!("Running hbone echo server at {}", echo.address()); ready.set_ready(); echo.run().await; @@ -1439,7 +1508,6 @@ mod namespaced { async fn hbone_read_write_stream(stream: &mut TcpStream) { const BODY: &[u8] = b"hello world"; - const WAYPOINT_MESSAGE: &[u8] = b"waypoint\n"; stream.write_all(BODY).await.unwrap(); let mut buf = [0; BODY.len() + WAYPOINT_MESSAGE.len()]; stream.read_exact(&mut buf).await.unwrap(); From b2a21bd2150cb607770e9de5cf63cf1679716f1f Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Fri, 7 Mar 2025 12:36:54 -0500 Subject: [PATCH 12/29] lint --- src/proxy/h2/client.rs | 2 +- src/proxy/metrics.rs | 2 +- src/proxy/outbound.rs | 9 ++++--- src/state.rs | 57 +++++++++++++++++++++++------------------- tests/namespaced.rs | 7 +++++- 5 files changed, 44 insertions(+), 33 deletions(-) diff --git a/src/proxy/h2/client.rs b/src/proxy/h2/client.rs index 7b45ed32ae..e0cc4af928 100644 --- a/src/proxy/h2/client.rs +++ b/src/proxy/h2/client.rs @@ -29,7 +29,7 @@ use std::task::{Context, Poll}; use tokio::io::{AsyncRead, AsyncWrite}; use tokio::sync::oneshot; use tokio::sync::watch::Receiver; -use tracing::{debug, error, trace, warn, Instrument}; +use tracing::{Instrument, debug, error, trace, warn}; #[derive(Debug, Clone)] // H2ConnectClient is a wrapper abstracting h2 diff --git a/src/proxy/metrics.rs b/src/proxy/metrics.rs index 52ab54790d..f912c246a9 100644 --- a/src/proxy/metrics.rs +++ b/src/proxy/metrics.rs @@ -458,7 +458,7 @@ impl ConnectionResult { src.identity = tl.source_principal.as_ref().filter(|_| mtls).map(to_value_owned), dst.addr = %dst.0, - dst.hbone_addr = hbone_target.as_ref(), + dst.hbone_addr = hbone_target.as_ref().map(display), dst.service = tl.destination_service.to_value(), dst.workload = dst.1.as_deref().map(to_value), dst.namespace = tl.destination_workload_namespace.to_value(), diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index a3ff2658d7..6741b8162b 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -198,7 +198,6 @@ impl OutboundConnection { metrics, )); - let res = match ( req.protocol, req.actual_destination_workload @@ -229,7 +228,7 @@ impl OutboundConnection { connection_stats: &ConnectionResult, ) -> Result<(), Error> { // Create the outer HBONE stream - let upgraded = Box::pin(self.send_hbone_request(remote_addr, &req)).await?; + let upgraded = Box::pin(self.send_hbone_request(remote_addr, req)).await?; // Wrap upgraded to implement tokio's Async{Write,Read} let upgraded = TokioH2Stream::new(upgraded); @@ -703,8 +702,10 @@ mod tests { Some(ExpectedRequest { protocol: r.protocol, hbone_destination: &r - .hbone_target_destination.as_ref() - .unwrap_or(&String::new()), + .hbone_target_destination + .as_ref() + .map(|s| s.to_string()) + .unwrap_or_default(), destination: &r.actual_destination.to_string(), }) ); diff --git a/src/state.rs b/src/state.rs index 39eba4f950..e1456f2185 100644 --- a/src/state.rs +++ b/src/state.rs @@ -665,7 +665,11 @@ impl DemandProxyState { ) -> Result { let workload_uid = workload.uid.clone(); // FIXME(stevenjin8) Throw a error if this a network gateway without a hostname? - let hostname = match workload.network_gateway.as_ref().map(|g| g.destination.clone()) { + let hostname = match workload + .network_gateway + .as_ref() + .map(|g| g.destination.clone()) + { Some(Destination::Hostname(hostname)) => hostname.hostname.clone(), _ => workload.hostname.clone(), }; @@ -802,31 +806,32 @@ impl DemandProxyState { }; let svc_desc = svc.clone().map(|s| ServiceDescription::from(s.as_ref())); let ip_family_restriction = svc.as_ref().and_then(|s| s.ip_families); - let (selected_workload_ip, proxy_protocol_port_override) = - if let Some(network_gateway) = wl.network_gateway.as_ref() { - match &network_gateway.destination { - Destination::Address(network_address) => ( - network_address.address, - Some(network_gateway.hbone_mtls_port), - ), - Destination::Hostname(_) => ( - self.resolve_workload_address(&wl, source_workload, original_target_address) - .await?, - Some(network_gateway.hbone_mtls_port), - ), - } - } else { - ( - self.pick_workload_destination_or_resolve( - &wl, - source_workload, - original_target_address, - ip_family_restriction, - ) - .await?, - None, - ) // if we can't load balance just return the error - }; + let (selected_workload_ip, proxy_protocol_port_override) = if let Some(network_gateway) = + wl.network_gateway.as_ref() + { + match &network_gateway.destination { + Destination::Address(network_address) => ( + network_address.address, + Some(network_gateway.hbone_mtls_port), + ), + Destination::Hostname(_) => ( + self.resolve_workload_address(&wl, source_workload, original_target_address) + .await?, + Some(network_gateway.hbone_mtls_port), + ), + } + } else { + ( + self.pick_workload_destination_or_resolve( + &wl, + source_workload, + original_target_address, + ip_family_restriction, + ) + .await?, + None, + ) // if we can't load balance just return the error + }; let res = Upstream { workload: wl, diff --git a/tests/namespaced.rs b/tests/namespaced.rs index d63c66dcf0..97804d9c7c 100644 --- a/tests/namespaced.rs +++ b/tests/namespaced.rs @@ -402,7 +402,12 @@ mod namespaced { .mutate_workload(|w| w.hostname = "waypoint.example.com".into()) .register() .await?; - run_hbone_server(waypoint, "waypoint", tcp::Mode::ReadWrite, WAYPOINT_MESSAGE.into())?; + run_hbone_server( + waypoint, + "waypoint", + tcp::Mode::ReadWrite, + WAYPOINT_MESSAGE.into(), + )?; manager .workload_builder("server", DEFAULT_NODE) From 896cef534ca5f4554a0a4cc04232a622fa1df7f1 Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Tue, 11 Mar 2025 14:56:22 -0400 Subject: [PATCH 13/29] Fix type sizes --- src/proxy/outbound.rs | 9 +++++++-- src/proxy/socks5.rs | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index 6741b8162b..066180566f 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -205,8 +205,13 @@ impl OutboundConnection { .and_then(|wl| wl.network_gateway.as_ref()), ) { (_, Some(_)) => { - self.proxy_to_double_hbone(source_stream, source_addr, &req, &result_tracker) - .await + Box::pin(self.proxy_to_double_hbone( + source_stream, + source_addr, + &req, + &result_tracker, + )) + .await } (Protocol::HBONE, _) => { self.proxy_to_hbone(source_stream, source_addr, &req, &result_tracker) diff --git a/src/proxy/socks5.rs b/src/proxy/socks5.rs index 3e7d8f4771..905f6dfd07 100644 --- a/src/proxy/socks5.rs +++ b/src/proxy/socks5.rs @@ -107,7 +107,7 @@ impl Socks5 { debug!(component="socks5", dur=?start.elapsed(), "connection completed"); }).instrument(span); - assertions::size_between_ref(1000, 10000, &serve); + assertions::size_between_ref(1000, 2000, &serve); tokio::spawn(serve); } Err(e) => { From 522fd35da648ba38fa2d1ac60c2ec59924454ead Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Tue, 11 Mar 2025 17:15:41 -0400 Subject: [PATCH 14/29] Check workload potocol --- src/proxy/outbound.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index 066180566f..45e01d8f6f 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -15,6 +15,7 @@ use std::net::{IpAddr, SocketAddr}; use std::sync::Arc; +use anyhow::anyhow; use futures_util::TryFutureExt; use hyper::header::FORWARDED; use std::time::Instant; @@ -204,7 +205,7 @@ impl OutboundConnection { .as_ref() .and_then(|wl| wl.network_gateway.as_ref()), ) { - (_, Some(_)) => { + (Protocol::HBONE, Some(_)) => { Box::pin(self.proxy_to_double_hbone( source_stream, source_addr, @@ -213,14 +214,18 @@ impl OutboundConnection { )) .await } - (Protocol::HBONE, _) => { + (Protocol::HBONE, None) => { self.proxy_to_hbone(source_stream, source_addr, &req, &result_tracker) .await } - (Protocol::TCP, _) => { + (Protocol::TCP, None) => { self.proxy_to_tcp(source_stream, &req, &result_tracker) .await } + (_, Some(_)) => Err(Error::Anyhow(anyhow!( + "Cannot proxy to workload with protocol {:?} and network gateway", + req.protocol + ))), }; result_tracker.record(res) } From a56b3ca21287a34f847924104c6d3cafeddc0ec5 Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Fri, 14 Mar 2025 12:29:49 -0400 Subject: [PATCH 15/29] wip --- src/proxy/h2.rs | 16 ++++- src/proxy/outbound.rs | 3 +- src/proxy/pool.rs | 33 ++++++++++ src/state.rs | 145 ++++++++++++++++++++++++++++++------------ 4 files changed, 154 insertions(+), 43 deletions(-) diff --git a/src/proxy/h2.rs b/src/proxy/h2.rs index 70d25d35aa..2130932545 100644 --- a/src/proxy/h2.rs +++ b/src/proxy/h2.rs @@ -155,7 +155,20 @@ impl tokio::io::AsyncRead for TokioH2Stream { buf: &mut tokio::io::ReadBuf<'_>, ) -> Poll> { let pinned = std::pin::Pin::new(&mut self.0.read); - copy::ResizeBufRead::poll_bytes(pinned, cx).map_ok(|bytes| buf.put(bytes)) + copy::ResizeBufRead::poll_bytes(pinned, cx).map(|r| match r { + Ok(bytes) => { + if buf.remaining() < bytes.len() { + Err(Error::new( + std::io::ErrorKind::Other, + format!("Would overflow buffer of with {} remaining", buf.remaining()), + )) + } else { + buf.put(bytes); + Ok(()) + } + } + Err(e) => Err(e), + }) } } @@ -289,3 +302,4 @@ fn h2_to_io_error(e: h2::Error) -> std::io::Error { std::io::Error::new(std::io::ErrorKind::Other, e) } } + diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index 45e01d8f6f..d551db47e8 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -112,7 +112,7 @@ impl Outbound { debug!(component="outbound", dur=?start.elapsed(), "connection completed"); }).instrument(span); - assertions::size_between_ref(1000, 99999, &serve_outbound_connection); + assertions::size_between_ref(1000, 1750, &serve_outbound_connection); tokio::spawn(serve_outbound_connection); } Err(e) => { @@ -206,6 +206,7 @@ impl OutboundConnection { .and_then(|wl| wl.network_gateway.as_ref()), ) { (Protocol::HBONE, Some(_)) => { + // We box this since its not a common path and it would make the future really big. Box::pin(self.proxy_to_double_hbone( source_stream, source_addr, diff --git a/src/proxy/pool.rs b/src/proxy/pool.rs index 87999d51e9..6610b277fb 100644 --- a/src/proxy/pool.rs +++ b/src/proxy/pool.rs @@ -532,6 +532,7 @@ mod test { use std::sync::RwLock; use std::sync::atomic::AtomicU32; use std::time::Duration; + use tokio::io::AsyncReadExt; use tokio::io::AsyncWriteExt; use tokio::net::TcpListener; @@ -544,6 +545,8 @@ mod test { use crate::identity::Identity; + use self::h2::TokioH2Stream; + use super::*; use crate::drain::DrainWatcher; use crate::state::workload; @@ -597,6 +600,36 @@ mod test { assert_opens_drops!(srv, 1, 1); } + /// This is really a test for TokioH2Stream, but its nicer here because we have access to + /// streams + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn small_reads() { + let (mut pool, srv) = setup_test(3).await; + + let key = key(&srv, 2); + let req = || { + http::Request::builder() + .uri(srv.addr.to_string()) + .method(http::Method::CONNECT) + .version(http::Version::HTTP_2) + .body(()) + .unwrap() + }; + + let start = Instant::now(); + + let c = pool.send_request_pooled(&key.clone(), req()).await.unwrap(); + let mut c = TokioH2Stream::new(c); + c.write_all(b"abcde").await.unwrap(); + let mut b = [0u8; 0]; + // Crucially, this should error rather than panic. + if let Err(e) = c.read(&mut b).await { + assert_eq!(e.kind(), io::ErrorKind::Other); + } else { + panic!("Should have errored"); + } + } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn unique_keys_have_unique_connections() { let (pool, mut srv) = setup_test(3).await; diff --git a/src/state.rs b/src/state.rs index e1456f2185..70999654ca 100644 --- a/src/state.rs +++ b/src/state.rs @@ -601,13 +601,39 @@ impl DemandProxyState { src_workload: &Workload, original_target_address: SocketAddr, ip_family_restriction: Option, - ) -> Result { + ) -> Result<(IpAddr, Option), Error> { + // If the wl has a network gateway, use it + // if let Some(network_gateway) = dst_workload.network_gateway.as_ref() { + // return match &network_gateway.destination { + // Destination::Address(network_address) => Ok(( + // // Use network address if it exists + // network_address.address, + // Some(network_gateway.hbone_mtls_port), + // )), + // Destination::Hostname(hostname) => { + // // Look in the service registry for this. + // // HERE FIXME do an SVC lookup + // let svc = self.read().find_hostname(&hostname).ok_or(Error::NoHealthyUpstream(SocketAddr::from_str("1.1.1.1:50").unwrap()))?; + // Ok(( + // self.resolve_workload_address( + // &dst_workload, + // src_workload, + // original_target_address, + // ) + // .await?, + // Some(network_gateway.hbone_mtls_port), + // )) + // } + // }; + // } + // + // // If the user requested the pod by a specific IP, use that directly. if dst_workload .workload_ips .contains(&original_target_address.ip()) { - return Ok(original_target_address.ip()); + return Ok((original_target_address.ip(), None)); } // They may have 1 or 2 IPs (single/dual stack) // Ensure we are meeting the Service family restriction (if any is defined). @@ -622,7 +648,7 @@ impl DemandProxyState { }) .find_or_first(|ip| ip.is_ipv6() == original_target_address.is_ipv6()) { - return Ok(*ip); + return Ok((*ip, None)); } if dst_workload.hostname.is_empty() { debug!( @@ -637,7 +663,7 @@ impl DemandProxyState { original_target_address, )) .await?; - Ok(ip) + Ok((ip, None)) } async fn resolve_workload_address( @@ -664,17 +690,7 @@ impl DemandProxyState { original_target_address: SocketAddr, ) -> Result { let workload_uid = workload.uid.clone(); - // FIXME(stevenjin8) Throw a error if this a network gateway without a hostname? - let hostname = match workload - .network_gateway - .as_ref() - .map(|g| g.destination.clone()) - { - Some(Destination::Hostname(hostname)) => hostname.hostname.clone(), - _ => workload.hostname.clone(), - }; - - // = workload.hostname.clone(); + let hostname = workload.hostname.clone(); trace!(%hostname, "starting DNS lookup"); let resp = match self.dns_resolver.lookup_ip(hostname.as_str()).await { @@ -776,6 +792,7 @@ impl DemandProxyState { self.read().workloads.find_uid(uid) } + // Find/resolve all information about the target workload (or have it passthrough). pub async fn fetch_upstream( &self, network: Strng, @@ -783,18 +800,43 @@ impl DemandProxyState { addr: SocketAddr, resolution_mode: ServiceResolutionMode, ) -> Result, Error> { + // update cache? self.fetch_address(&network_addr(network.clone(), addr.ip())) .await; + // find upstream and select destination workload let upstream = { self.read() .find_upstream(network, source_workload, addr, resolution_mode) // Drop the lock }; + // deal with network gateway + // if let Some((wl, port, service)) = upstream.as_ref() { + // if let Some(ref network_gateway) = wl.network_gateway { + // self.handle_network_gateway(network_gateway).await; + // } + // } tracing::trace!(%addr, ?upstream, "fetch_upstream"); + // package into a finalize upstream and pick/resolve addresses self.finalize_upstream(source_workload, addr, upstream) .await } + // Here, we want to handle the case where there is a network gateway. + // This is only if we find an upstream workload and there is a network gateway. + // We want this to look at the workloads network gateway and parse out the workload ip + // and if there is an hbone port override. + // Also, we might have to do another svc/vip lookup if the network gateway is a hostname. + // We also have to be careful to not infinite loop if the network gateway workload itself has a + // network gateway. + // async fn handle_network_gateway(&self, network_gateway: &GatewayAddress) { + // let x = &network_gateway.destination; + // match x { + // Destination::Address(address) => todo!, + // Destination::Hostname(h) => todo!, + // + // } + // } + async fn finalize_upstream( &self, source_workload: &Workload, @@ -806,32 +848,53 @@ impl DemandProxyState { }; let svc_desc = svc.clone().map(|s| ServiceDescription::from(s.as_ref())); let ip_family_restriction = svc.as_ref().and_then(|s| s.ip_families); - let (selected_workload_ip, proxy_protocol_port_override) = if let Some(network_gateway) = - wl.network_gateway.as_ref() - { - match &network_gateway.destination { - Destination::Address(network_address) => ( - network_address.address, - Some(network_gateway.hbone_mtls_port), - ), - Destination::Hostname(_) => ( - self.resolve_workload_address(&wl, source_workload, original_target_address) - .await?, - Some(network_gateway.hbone_mtls_port), - ), - } - } else { - ( - self.pick_workload_destination_or_resolve( - &wl, - source_workload, - original_target_address, - ip_family_restriction, - ) - .await?, - None, - ) // if we can't load balance just return the error - }; + + let (selected_workload_ip, proxy_protocol_port_override) = self + .pick_workload_destination_or_resolve( + &wl, + source_workload, + original_target_address, + ip_family_restriction, + ) + .await?; // if we can't load balance just return the error + // if let Some(network_gateway) = + // wl.network_gateway.as_ref() + // { + // match &network_gateway.destination { + // Destination::Address(network_address) => ( + // // Use network address if it exists + // network_address.address, + // Some(network_gateway.hbone_mtls_port), + // ), + // Destination::Hostname(hostname) => { + // // Look in the service registry for this. + // // HERE FIXME do an SVC lookup + // let svc = self + // .read() + // .find_service_by_hostname(&hostname.hostname)?.first().ok_or(Error::NoHostname("Workloads referenced by Network Gateways destination hostnames must have IPs".into()))?; + // ( + // self.resolve_workload_address( + // &wl, + // source_workload, + // original_target_address, + // ) + // .await?, + // Some(network_gateway.hbone_mtls_port), + // ) + // } + // } + // } else { + // ( + // self.pick_workload_destination_or_resolve( + // &wl, + // source_workload, + // original_target_address, + // ip_family_restriction, + // ) + // .await?, + // None, + // ) // if we can't load balance just return the error + // }; let res = Upstream { workload: wl, From 08e7a5888a692623664f02c344008c76276b4d41 Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Wed, 26 Mar 2025 16:15:30 -0400 Subject: [PATCH 16/29] initial impl of hostname --- benches/throughput.rs | 6 +- src/cert_fetcher.rs | 4 +- src/proxy/inbound.rs | 4 +- src/proxy/outbound.rs | 161 ++++++++++++++++++++++++++++-------------- src/state.rs | 144 +++++++++++++++++++++---------------- src/state/workload.rs | 37 ++++++++-- src/test_helpers.rs | 6 +- tests/namespaced.rs | 2 +- 8 files changed, 231 insertions(+), 133 deletions(-) diff --git a/benches/throughput.rs b/benches/throughput.rs index c3c51dae72..9fea8102b2 100644 --- a/benches/throughput.rs +++ b/benches/throughput.rs @@ -35,7 +35,7 @@ use tokio::sync::Mutex; use tracing::info; use ztunnel::rbac::{Authorization, RbacMatch, StringMatch}; -use ztunnel::state::workload::{Protocol, Workload}; +use ztunnel::state::workload::{ServerProtocol, Workload}; use ztunnel::state::{DemandProxyState, ProxyRbacContext, ProxyState}; use ztunnel::test_helpers::app::{DestinationAddr, TestApp}; use ztunnel::test_helpers::linux::{TestMode, WorkloadManager}; @@ -457,7 +457,7 @@ fn hbone_connection_config() -> ztunnel::config::ConfigSource { let lwl = LocalWorkload { workload: Workload { workload_ips: vec![hbone_connection_ip(i)], - protocol: Protocol::HBONE, + protocol: ServerProtocol::HBONE, uid: strng::format!("cluster1//v1/Pod/default/remote{}", i), name: strng::format!("workload-{}", i), namespace: strng::format!("namespace-{}", i), @@ -471,7 +471,7 @@ fn hbone_connection_config() -> ztunnel::config::ConfigSource { let lwl = LocalWorkload { workload: Workload { workload_ips: vec![], - protocol: Protocol::HBONE, + protocol: ServerProtocol::HBONE, uid: "cluster1//v1/Pod/default/local-source".into(), name: "local-source".into(), namespace: "default".into(), diff --git a/src/cert_fetcher.rs b/src/cert_fetcher.rs index ab905b7d9d..02de419994 100644 --- a/src/cert_fetcher.rs +++ b/src/cert_fetcher.rs @@ -16,7 +16,7 @@ use crate::config; use crate::config::ProxyMode; use crate::identity::Priority::Warmup; use crate::identity::{Identity, Request, SecretManager}; -use crate::state::workload::{Protocol, Workload}; +use crate::state::workload::{ServerProtocol, Workload}; use std::sync::Arc; use tokio::sync::mpsc; use tracing::{debug, error, info}; @@ -96,7 +96,7 @@ impl CertFetcherImpl { // We only get certs for our own node Some(w.node.as_ref()) == self.local_node.as_deref() && // If it doesn't support HBONE it *probably* doesn't need a cert. - (w.native_tunnel || w.protocol == Protocol::HBONE) + (w.native_tunnel || w.protocol == ServerProtocol::HBONE) } } diff --git a/src/proxy/inbound.rs b/src/proxy/inbound.rs index 9f55a25f91..6992e60747 100644 --- a/src/proxy/inbound.rs +++ b/src/proxy/inbound.rs @@ -711,7 +711,7 @@ mod tests { self, DemandProxyState, service::{Endpoint, EndpointSet, Service}, workload::{ - ApplicationTunnel, GatewayAddress, NetworkAddress, Protocol, Workload, + ApplicationTunnel, GatewayAddress, NetworkAddress, ServerProtocol, Workload, application_tunnel::Protocol as AppProtocol, gatewayaddress::Destination, }, }, @@ -978,7 +978,7 @@ mod tests { .map(|(name, ip, waypoint, app_tunnel)| Workload { workload_ips: vec![ip.parse().unwrap()], waypoint: waypoint.workload_attached(), - protocol: Protocol::HBONE, + protocol: ServerProtocol::HBONE, uid: strng::format!("cluster1//v1/Pod/default/{name}"), name: strng::format!("workload-{name}"), namespace: "default".into(), diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index d551db47e8..870bc39757 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -15,7 +15,6 @@ use std::net::{IpAddr, SocketAddr}; use std::sync::Arc; -use anyhow::anyhow; use futures_util::TryFutureExt; use hyper::header::FORWARDED; use std::time::Instant; @@ -38,7 +37,9 @@ use crate::drain::run_with_drain; use crate::proxy::h2::{H2Stream, client::WorkloadKey}; use crate::state::ServiceResolutionMode; use crate::state::service::ServiceDescription; -use crate::state::workload::{NetworkAddress, Protocol, Workload, address::Address}; +use crate::state::workload::ClientProtocol; +use crate::state::workload::gatewayaddress::Destination; +use crate::state::workload::{NetworkAddress, ServerProtocol, Workload, address::Address}; use crate::{assertions, copy, proxy, socket}; use super::h2::TokioH2Stream; @@ -180,7 +181,6 @@ impl OutboundConnection { return; } }; - // At this point, we want all the addresses resolved // TODO: should we use the original address or the actual address? Both seems nice! let _conn_guard = self.pi.connection_manager.track_outbound( source_addr, @@ -199,13 +199,8 @@ impl OutboundConnection { metrics, )); - let res = match ( - req.protocol, - req.actual_destination_workload - .as_ref() - .and_then(|wl| wl.network_gateway.as_ref()), - ) { - (Protocol::HBONE, Some(_)) => { + let res = match req.protocol { + ClientProtocol::DOUBLEHBONE => { // We box this since its not a common path and it would make the future really big. Box::pin(self.proxy_to_double_hbone( source_stream, @@ -215,18 +210,14 @@ impl OutboundConnection { )) .await } - (Protocol::HBONE, None) => { + ClientProtocol::HBONE => { self.proxy_to_hbone(source_stream, source_addr, &req, &result_tracker) .await } - (Protocol::TCP, None) => { + ClientProtocol::TCP => { self.proxy_to_tcp(source_stream, &req, &result_tracker) .await } - (_, Some(_)) => Err(Error::Anyhow(anyhow!( - "Cannot proxy to workload with protocol {:?} and network gateway", - req.protocol - ))), }; result_tracker.record(res) } @@ -369,7 +360,7 @@ impl OutboundConnection { } fn conn_metrics_from_request(req: &Request) -> ConnectionOpen { - let derived_source = if req.protocol == Protocol::HBONE { + let derived_source = if req.protocol == ClientProtocol::HBONE { Some(DerivedWorkload { // We are going to do mTLS, so report our identity identity: Some(req.source.as_ref().identity()), @@ -383,7 +374,7 @@ impl OutboundConnection { derived_source, source: Some(req.source.clone()), destination: req.actual_destination_workload.clone(), - connection_security_policy: if req.protocol == Protocol::HBONE { + connection_security_policy: if req.protocol == ClientProtocol::HBONE { metrics::SecurityPolicy::mutual_tls } else { metrics::SecurityPolicy::unknown @@ -417,10 +408,15 @@ impl OutboundConnection { .await? { let upstream_sans = waypoint.workload_and_services_san(); - let actual_destination = waypoint.workload_socket_addr(); + let actual_destination = + waypoint + .workload_socket_addr() + .ok_or(Error::NoValidDestination(Box::new( + (*waypoint.workload).clone(), + )))?; debug!("built request to service waypoint proxy"); return Ok(Request { - protocol: Protocol::HBONE, + protocol: ClientProtocol::HBONE, source: source_workload, hbone_target_destination: Some(HboneAddress::SocketAddr(target)), actual_destination_workload: Some(waypoint.workload), @@ -451,7 +447,7 @@ impl OutboundConnection { } debug!("built request as passthrough; no upstream found"); return Ok(Request { - protocol: Protocol::TCP, + protocol: ClientProtocol::TCP, source: source_workload, hbone_target_destination: None, actual_destination_workload: None, @@ -462,6 +458,55 @@ impl OutboundConnection { }); }; + // Check whether we are using an E/W gateway + // FIXME: this should check if there is a network gateway AND we are in different networks + // since wl entries are absolute + if let Some(ew_gtw) = &us.workload.network_gateway { + let svc = us + .destination_service + .as_ref() + .expect("Workloads with network gateways must be service addressed."); + + let actual_destination = match &ew_gtw.destination { + // FIXME assume that this address is reachable and ignore the address.network + // field? + Destination::Address(address) => { + SocketAddr::from((address.address, ew_gtw.hbone_mtls_port)) + } + Destination::Hostname(host) => { + let us = self.pi.state.fetch_upstream_by_host( + &source_workload, + host, + 15008, //fixme + target, + ServiceResolutionMode::Standard + ).await?.unwrap(); + SocketAddr::from((us.selected_workload_ip.unwrap(), us.port)) + } + }; + + let hbone_target_destination = + Some(HboneAddress::SvcHostname(svc.hostname.clone(), us.port)); + + // TODO: if the gateway address is a hostname, we want to use the workload/service sans + // of the resolved workload. + let (upstream_sans, final_sans) = (us.workload_and_services_san(), us.service_sans()); + + return Ok(Request { + protocol: ClientProtocol::DOUBLEHBONE, + source: source_workload, + hbone_target_destination, + actual_destination_workload: Some(us.workload.clone()), + intended_destination_service: us.destination_service.clone(), + actual_destination, + upstream_sans, + final_sans, + }); + } + + // We are not using a network gateway and there is no workload address. + // + // let from_waypoint = proxy::check_from_waypoint( state, &us.workload, @@ -479,12 +524,17 @@ impl OutboundConnection { .fetch_workload_waypoint(&us.workload, &source_workload, target) .await?; if let Some(waypoint) = waypoint { - let actual_destination = waypoint.workload_socket_addr(); + let actual_destination = + waypoint + .workload_socket_addr() + .ok_or(Error::NoValidDestination(Box::new( + (*waypoint.workload).clone(), + )))?; let upstream_sans = waypoint.workload_and_services_san(); debug!("built request to workload waypoint proxy"); return Ok(Request { // Always use HBONE here - protocol: Protocol::HBONE, + protocol: ClientProtocol::HBONE, source: source_workload, // Use the original VIP, not translated hbone_target_destination: Some(HboneAddress::SocketAddr(target)), @@ -498,25 +548,30 @@ impl OutboundConnection { // Workload doesn't have a waypoint; send directly } + let selected_workload_ip = us + .selected_workload_ip + .ok_or(Error::NoValidDestination(Box::new((*us.workload).clone())))?; + // only change the port if we're sending HBONE let actual_destination = match us.workload.protocol { - Protocol::HBONE => SocketAddr::from(( - us.selected_workload_ip, - us.proxy_protocol_port_override.unwrap_or(self.hbone_port), - )), - Protocol::TCP => us.workload_socket_addr(), + ServerProtocol::HBONE => SocketAddr::from((selected_workload_ip, self.hbone_port)), + ServerProtocol::TCP => us + .workload_socket_addr() + .ok_or(Error::NoValidDestination(Box::new((*us.workload).clone())))?, }; let hbone_target_destination = match us.workload.protocol { - Protocol::HBONE => Some(us.hbone_target()), - Protocol::TCP => None, + ServerProtocol::HBONE => Some(HboneAddress::SocketAddr( + us.workload_socket_addr() + .ok_or(Error::NoValidDestination(Box::new((*us.workload).clone())))?, + )), + ServerProtocol::TCP => None, }; - // FIXME this seems wrong - let (upstream_sans, final_sans) = (us.workload_and_services_san(), us.service_sans()); // For case no waypoint for both side and direct to remote node proxy + let (upstream_sans, final_sans) = (us.workload_and_services_san(), vec![]); debug!("built request to workload"); Ok(Request { - protocol: us.workload.protocol, + protocol: ClientProtocol::from(us.workload.protocol), source: source_workload, hbone_target_destination, actual_destination_workload: Some(us.workload.clone()), @@ -554,7 +609,7 @@ fn baggage(r: &Request, cluster: String) -> String { #[derive(Debug, Clone)] struct Request { - protocol: Protocol, + protocol: ClientProtocol, // Source workload sending the request source: Arc, // The actual destination workload we are targeting. When proxying through a waypoint, this is the waypoint, @@ -737,7 +792,7 @@ mod tests { ..Default::default() }), Some(ExpectedRequest { - protocol: Protocol::TCP, + protocol: ClientProtocol::TCP, hbone_destination: "", destination: "1.2.3.4:80", }), @@ -760,7 +815,7 @@ mod tests { ..Default::default() }), Some(ExpectedRequest { - protocol: Protocol::TCP, + protocol: ClientProtocol::TCP, hbone_destination: "", destination: "127.0.0.2:80", }), @@ -783,7 +838,7 @@ mod tests { ..Default::default() }), Some(ExpectedRequest { - protocol: Protocol::HBONE, + protocol: ClientProtocol::HBONE, hbone_destination: "127.0.0.2:80", destination: "127.0.0.2:15008", }), @@ -806,7 +861,7 @@ mod tests { ..Default::default() }), Some(ExpectedRequest { - protocol: Protocol::TCP, + protocol: ClientProtocol::TCP, hbone_destination: "", destination: "127.0.0.2:80", }), @@ -829,7 +884,7 @@ mod tests { ..Default::default() }), Some(ExpectedRequest { - protocol: Protocol::HBONE, + protocol: ClientProtocol::HBONE, hbone_destination: "127.0.0.2:80", destination: "127.0.0.2:15008", }), @@ -858,7 +913,7 @@ mod tests { }), // Even though source has a waypoint, we don't use it Some(ExpectedRequest { - protocol: Protocol::TCP, + protocol: ClientProtocol::TCP, hbone_destination: "", destination: "127.0.0.1:80", }), @@ -887,7 +942,7 @@ mod tests { }), // Should use the waypoint Some(ExpectedRequest { - protocol: Protocol::HBONE, + protocol: ClientProtocol::HBONE, hbone_destination: "127.0.0.2:80", destination: "127.0.0.10:15008", }), @@ -921,7 +976,7 @@ mod tests { }), // Should use the waypoint Some(ExpectedRequest { - protocol: Protocol::HBONE, + protocol: ClientProtocol::HBONE, hbone_destination: "[ff06::c3]:80", destination: "127.0.0.11:15008", }), @@ -956,7 +1011,7 @@ mod tests { }), // Should use the waypoint Some(ExpectedRequest { - protocol: Protocol::HBONE, + protocol: ClientProtocol::HBONE, hbone_destination: "127.0.0.3:80", destination: "127.0.0.10:15008", }), @@ -1041,7 +1096,7 @@ mod tests { }), ], Some(ExpectedRequest { - protocol: Protocol::TCP, + protocol: ClientProtocol::TCP, hbone_destination: "", destination: "127.0.0.2:1234", }), @@ -1096,8 +1151,8 @@ mod tests { xds.clone(), // Traffic to the service should go to the pod in the service Some(ExpectedRequest { + protocol: ClientProtocol::TCP, destination: "127.0.0.2:80", - protocol: Protocol::TCP, hbone_destination: "", }), ) @@ -1117,8 +1172,8 @@ mod tests { xds.clone(), // Traffic to the service should go to the pod in the service Some(ExpectedRequest { + protocol: ClientProtocol::TCP, destination: "127.0.0.2:80", - protocol: Protocol::TCP, hbone_destination: "", }), ) @@ -1148,7 +1203,7 @@ mod tests { "127.0.0.2:80", workload.clone(), Some(ExpectedRequest { - protocol: Protocol::TCP, + protocol: ClientProtocol::TCP, hbone_destination: "", destination: "127.0.0.2:80", }), @@ -1160,7 +1215,7 @@ mod tests { "[ff06::c3]:80", workload.clone(), Some(ExpectedRequest { - protocol: Protocol::TCP, + protocol: ClientProtocol::TCP, hbone_destination: "", destination: "[ff06::c3]:80", }), @@ -1212,7 +1267,7 @@ mod tests { "127.0.0.3:80", vec![svc(IpFamilies::Ipv6Only), workload.clone()], Some(ExpectedRequest { - protocol: Protocol::HBONE, + protocol: ClientProtocol::HBONE, hbone_destination: "[ff06::c3]:80", destination: "[ff06::c3]:15008", }), @@ -1224,7 +1279,7 @@ mod tests { "127.0.0.3:80", vec![svc(IpFamilies::Ipv4Only), workload.clone()], Some(ExpectedRequest { - protocol: Protocol::HBONE, + protocol: ClientProtocol::HBONE, hbone_destination: "127.0.0.2:80", destination: "127.0.0.2:15008", }), @@ -1236,7 +1291,7 @@ mod tests { "127.0.0.3:80", vec![svc(IpFamilies::Dual), workload.clone()], Some(ExpectedRequest { - protocol: Protocol::HBONE, + protocol: ClientProtocol::HBONE, hbone_destination: "127.0.0.2:80", destination: "127.0.0.2:15008", }), @@ -1248,7 +1303,7 @@ mod tests { "[::3]:80", vec![svc(IpFamilies::Dual), workload.clone()], Some(ExpectedRequest { - protocol: Protocol::HBONE, + protocol: ClientProtocol::HBONE, hbone_destination: "[ff06::c3]:80", destination: "[ff06::c3]:15008", }), @@ -1281,7 +1336,7 @@ mod tests { #[derive(PartialEq, Debug)] struct ExpectedRequest<'a> { - protocol: Protocol, + protocol: ClientProtocol, hbone_destination: &'a str, destination: &'a str, } diff --git a/src/state.rs b/src/state.rs index 70999654ca..9cc4d232ed 100644 --- a/src/state.rs +++ b/src/state.rs @@ -13,7 +13,7 @@ // limitations under the License. use crate::identity::{Identity, SecretManager}; -use crate::proxy::{Error, HboneAddress, OnDemandDnsLabels}; +use crate::proxy::{Error, OnDemandDnsLabels}; use crate::rbac::Authorization; use crate::state::policy::PolicyStore; use crate::state::service::{ @@ -62,7 +62,8 @@ pub struct Upstream { pub workload: Arc, /// selected_workload_ip defines the IP address we should actually use to connect to this workload /// This handles multiple IPs (dual stack) or Hostname destinations (DNS resolution) - pub selected_workload_ip: IpAddr, + /// The workload IP might be empty if we have to go through a network gateway. + pub selected_workload_ip: Option, /// Port is the port we should connect to pub port: u16, /// Service SANs defines SANs defined at the service level *only*. A complete view of things requires @@ -75,27 +76,28 @@ pub struct Upstream { } impl Upstream { - /// If there is a network gateway, use :. Otherwise, use - /// :. Fortunately, for double-hbone, the authority/host is the same - /// for inner and outer CONNECT request, so we can reuse this for inner double hbone, - /// outer double hbone, and normal hbone. - pub fn hbone_target(&self) -> HboneAddress { - if self.workload.network_gateway.is_some() { - let svc = self - .destination_service - .as_ref() - .expect("Workloads with network gateways must be service addressed."); - HboneAddress::SvcHostname( - format!("{}.{}", svc.namespace, svc.hostname).into(), - self.port, - ) - } else { - HboneAddress::SocketAddr(self.workload_socket_addr()) - } - } - - pub fn workload_socket_addr(&self) -> SocketAddr { - SocketAddr::new(self.selected_workload_ip, self.port) + // /// If there is a network gateway, use :. Otherwise, use + // /// :. Fortunately, for double-hbone, the authority/host is the same + // /// for inner and outer CONNECT request, so we can reuse this for inner double hbone, + // /// outer double hbone, and normal hbone. + // pub fn hbone_target(&self) -> HboneAddress { + // if self.workload.network_gateway.is_some() { + // let svc = self + // .destination_service + // .as_ref() + // .expect("Workloads with network gateways must be service addressed."); + // HboneAddress::SvcHostname( + // format!("{}.{}", svc.namespace, svc.hostname).into(), + // self.port, + // ) + // } else { + // HboneAddress::SocketAddr(self.workload_socket_addr()) + // } + // } + + pub fn workload_socket_addr(&self) -> Option { + self.selected_workload_ip + .map(|ip| SocketAddr::new(ip, self.port)) } pub fn workload_and_services_san(&self) -> Vec { self.service_sans @@ -301,6 +303,23 @@ impl ProxyState { }) } + fn find_upstream_from_host( + &self, + source_workload: &Workload, + host: &NamespacedHostname, + port: u16, + resolution_mode: ServiceResolutionMode, + ) -> Option<(Arc, u16, Option>)> { + let address = self.find_hostname(host)?; + match address { + Address::Service(svc) => { + self.find_upstream_from_service(source_workload, port, resolution_mode, svc) + } + Address::Workload(wl) => Some((wl, port, None)), + }; + todo!() + } + fn find_upstream( &self, network: Strng, @@ -328,7 +347,7 @@ impl ProxyState { None } - fn find_upstream_from_service( + pub fn find_upstream_from_service( &self, source_workload: &Workload, svc_port: u16, @@ -601,39 +620,13 @@ impl DemandProxyState { src_workload: &Workload, original_target_address: SocketAddr, ip_family_restriction: Option, - ) -> Result<(IpAddr, Option), Error> { - // If the wl has a network gateway, use it - // if let Some(network_gateway) = dst_workload.network_gateway.as_ref() { - // return match &network_gateway.destination { - // Destination::Address(network_address) => Ok(( - // // Use network address if it exists - // network_address.address, - // Some(network_gateway.hbone_mtls_port), - // )), - // Destination::Hostname(hostname) => { - // // Look in the service registry for this. - // // HERE FIXME do an SVC lookup - // let svc = self.read().find_hostname(&hostname).ok_or(Error::NoHealthyUpstream(SocketAddr::from_str("1.1.1.1:50").unwrap()))?; - // Ok(( - // self.resolve_workload_address( - // &dst_workload, - // src_workload, - // original_target_address, - // ) - // .await?, - // Some(network_gateway.hbone_mtls_port), - // )) - // } - // }; - // } - // - // + ) -> Result<(Option, Option), Error> { // If the user requested the pod by a specific IP, use that directly. if dst_workload .workload_ips .contains(&original_target_address.ip()) { - return Ok((original_target_address.ip(), None)); + return Ok((Some(original_target_address.ip()), None)); } // They may have 1 or 2 IPs (single/dual stack) // Ensure we are meeting the Service family restriction (if any is defined). @@ -648,14 +641,19 @@ impl DemandProxyState { }) .find_or_first(|ip| ip.is_ipv6() == original_target_address.is_ipv6()) { - return Ok((*ip, None)); + return Ok((Some(*ip), None)); } if dst_workload.hostname.is_empty() { - debug!( - "workload {} has no suitable workload IPs for routing", - dst_workload.name - ); - return Err(Error::NoValidDestination(Box::new(dst_workload.clone()))); + if dst_workload.network_gateway.is_none() { + debug!( + "workload {} has no suitable workload IPs for routing", + dst_workload.name + ); + return Err(Error::NoValidDestination(Box::new(dst_workload.clone()))); + } else { + // We can route through network gateway + return Ok((None, None)); + } } let ip = Box::pin(self.resolve_workload_address( dst_workload, @@ -663,7 +661,7 @@ impl DemandProxyState { original_target_address, )) .await?; - Ok((ip, None)) + Ok((Some(ip), None)) } async fn resolve_workload_address( @@ -792,6 +790,28 @@ impl DemandProxyState { self.read().workloads.find_uid(uid) } + pub async fn fetch_upstream_by_host( + &self, + source_workload: &Workload, + host: &NamespacedHostname, + port: u16, + original_target_address: SocketAddr, + resolution_mode: ServiceResolutionMode, + ) -> Result, Error> { + let upstream = { + self.read().find_upstream_from_host( + source_workload, + host, + port, + resolution_mode, + ) + // Drop the lock + }; + // tracing::trace!(%addr, ?upstream, "fetch_upstream"); + self.finalize_upstream(source_workload, original_target_address, upstream) + .await + } + // Find/resolve all information about the target workload (or have it passthrough). pub async fn fetch_upstream( &self, @@ -837,7 +857,7 @@ impl DemandProxyState { // } // } - async fn finalize_upstream( + pub async fn finalize_upstream( &self, source_workload: &Workload, original_target_address: SocketAddr, @@ -849,7 +869,7 @@ impl DemandProxyState { let svc_desc = svc.clone().map(|s| ServiceDescription::from(s.as_ref())); let ip_family_restriction = svc.as_ref().and_then(|s| s.ip_families); - let (selected_workload_ip, proxy_protocol_port_override) = self + let (selected_workload_ip, _) = self .pick_workload_destination_or_resolve( &wl, source_workload, @@ -902,7 +922,7 @@ impl DemandProxyState { port, service_sans: svc.map(|s| s.subject_alt_names.clone()).unwrap_or_default(), destination_service: svc_desc, - proxy_protocol_port_override, + proxy_protocol_port_override: None, }; tracing::trace!(?res, "finalize_upstream"); Ok(Some(res)) diff --git a/src/state/workload.rs b/src/state/workload.rs index cadce057fb..e3fedb1945 100644 --- a/src/state/workload.rs +++ b/src/state/workload.rs @@ -41,24 +41,47 @@ use xds::istio::workload::ApplicationTunnel as XdsApplicationTunnel; use xds::istio::workload::GatewayAddress as XdsGatewayAddress; use xds::istio::workload::Workload as XdsWorkload; +// The protocol that the final workload expects #[derive( Default, Debug, Hash, Eq, PartialEq, Clone, Copy, serde::Serialize, serde::Deserialize, )] -pub enum Protocol { +pub enum ServerProtocol { #[default] TCP, HBONE, } -impl From for Protocol { +impl From for ServerProtocol { fn from(value: xds::istio::workload::TunnelProtocol) -> Self { match value { - xds::istio::workload::TunnelProtocol::Hbone => Protocol::HBONE, - xds::istio::workload::TunnelProtocol::None => Protocol::TCP, + xds::istio::workload::TunnelProtocol::Hbone => ServerProtocol::HBONE, + xds::istio::workload::TunnelProtocol::None => ServerProtocol::TCP, } } } +// The protocol that the sender should use to send data. Can be different from ServerProtocol when there is a +// proxy in the middle (e.g. e/w gateway with double hbone). +#[derive( + Default, Debug, Eq, PartialEq, Clone, Copy +)] +pub enum ClientProtocol { + #[default] + TCP, + HBONE, + DOUBLEHBONE, +} + +impl From for ClientProtocol { + fn from(value: ServerProtocol) -> Self { + match value { + ServerProtocol::HBONE => ClientProtocol::HBONE, + ServerProtocol::TCP => ClientProtocol::TCP, + } + } +} + + #[derive( Default, Debug, Hash, Eq, PartialEq, Clone, Copy, serde::Serialize, serde::Deserialize, )] @@ -179,7 +202,7 @@ pub struct Workload { pub network_gateway: Option, #[serde(default)] - pub protocol: Protocol, + pub protocol: ServerProtocol, #[serde(default)] pub network_mode: NetworkMode, @@ -399,7 +422,7 @@ impl TryFrom for (Workload, HashMap) { waypoint: wp, network_gateway: network_gw, - protocol: Protocol::from(xds::istio::workload::TunnelProtocol::try_from( + protocol: ServerProtocol::from(xds::istio::workload::TunnelProtocol::try_from( resource.tunnel_protocol, )?), network_mode: NetworkMode::from(xds::istio::workload::NetworkMode::try_from( @@ -704,7 +727,7 @@ impl WorkloadByAddr { let is_pod = w.uid.contains("//Pod/"); // We fallback to looking for HBONE -- a resource marked as in the mesh is likely // to have more useful context than one not in the mesh. - let is_hbone = w.protocol == Protocol::HBONE; + let is_hbone = w.protocol == ServerProtocol::HBONE; match (is_pod, is_hbone) { (true, true) => 3, (true, false) => 2, diff --git a/src/test_helpers.rs b/src/test_helpers.rs index b11b595756..426a3626be 100644 --- a/src/test_helpers.rs +++ b/src/test_helpers.rs @@ -15,11 +15,11 @@ use crate::config::ConfigSource; use crate::config::{self, RootCert}; use crate::state::service::{Endpoint, EndpointSet, Service}; -use crate::state::workload::Protocol::{HBONE, TCP}; +use crate::state::workload::ServerProtocol::{HBONE, TCP}; use crate::state::workload::{ GatewayAddress, NamespacedHostname, NetworkAddress, Workload, gatewayaddress, }; -use crate::state::workload::{HealthStatus, Protocol}; +use crate::state::workload::{HealthStatus, ServerProtocol}; use crate::state::{DemandProxyState, ProxyState}; use crate::xds::istio::security::Authorization as XdsAuthorization; use crate::xds::istio::workload::Address as XdsAddress; @@ -233,7 +233,7 @@ pub fn test_default_workload() -> Workload { fn test_custom_workload( ip_str: &str, name: &str, - protocol: Protocol, + protocol: ServerProtocol, echo_port: u16, services_vec: Vec<&Service>, hostname_only: bool, diff --git a/tests/namespaced.rs b/tests/namespaced.rs index 97804d9c7c..d54d4a843c 100644 --- a/tests/namespaced.rs +++ b/tests/namespaced.rs @@ -217,7 +217,7 @@ mod namespaced { .network_gateway() .service( "default/remote.default.svc.cluster.local", - 15008u16, + 15008u16, // These can't be right 15008u16, ) .register() From 4ef17cc1163d81f247590e16ae3f80459c5ed68a Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Fri, 28 Mar 2025 10:17:45 -0400 Subject: [PATCH 17/29] tests passing --- src/proxy/outbound.rs | 38 ++++++++++------- src/state.rs | 3 +- src/test_helpers/linux.rs | 24 +++++------ tests/namespaced.rs | 87 ++++++++++++++++++++++++++++++++++++--- 4 files changed, 117 insertions(+), 35 deletions(-) diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index 870bc39757..fea8b13f6a 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -467,30 +467,38 @@ impl OutboundConnection { .as_ref() .expect("Workloads with network gateways must be service addressed."); - let actual_destination = match &ew_gtw.destination { + let (actual_destination, upstream_sans, final_sans) = match &ew_gtw.destination { // FIXME assume that this address is reachable and ignore the address.network // field? - Destination::Address(address) => { - SocketAddr::from((address.address, ew_gtw.hbone_mtls_port)) - } + Destination::Address(address) => ( + SocketAddr::from((address.address, ew_gtw.hbone_mtls_port)), + us.workload_and_services_san(), + us.service_sans(), + ), Destination::Hostname(host) => { - let us = self.pi.state.fetch_upstream_by_host( - &source_workload, - host, - 15008, //fixme - target, - ServiceResolutionMode::Standard - ).await?.unwrap(); - SocketAddr::from((us.selected_workload_ip.unwrap(), us.port)) + let us_gtw = self + .pi + .state + .fetch_upstream_by_host( + &source_workload, + host, + 15008, //fixme + target, + ServiceResolutionMode::Standard, + ) + .await? + .unwrap(); + ( + SocketAddr::from((us_gtw.selected_workload_ip.unwrap(), us_gtw.port)), + us_gtw.workload_and_services_san(), + us.service_sans(), + ) } }; let hbone_target_destination = Some(HboneAddress::SvcHostname(svc.hostname.clone(), us.port)); - // TODO: if the gateway address is a hostname, we want to use the workload/service sans - // of the resolved workload. - let (upstream_sans, final_sans) = (us.workload_and_services_san(), us.service_sans()); return Ok(Request { protocol: ClientProtocol::DOUBLEHBONE, diff --git a/src/state.rs b/src/state.rs index cbd0d4a344..287671f4a3 100644 --- a/src/state.rs +++ b/src/state.rs @@ -316,8 +316,7 @@ impl ProxyState { self.find_upstream_from_service(source_workload, port, resolution_mode, svc) } Address::Workload(wl) => Some((wl, port, None)), - }; - todo!() + } } fn find_upstream( diff --git a/src/test_helpers/linux.rs b/src/test_helpers/linux.rs index f9ef1624cb..031f83c90a 100644 --- a/src/test_helpers/linux.rs +++ b/src/test_helpers/linux.rs @@ -393,7 +393,6 @@ impl<'a> TestServiceBuilder<'a> { pub struct TestWorkloadBuilder<'a> { w: LocalWorkload, captured: bool, - is_network_gateway: bool, manager: &'a mut WorkloadManager, } @@ -401,7 +400,6 @@ impl<'a> TestWorkloadBuilder<'a> { pub fn new(name: &str, manager: &'a mut WorkloadManager) -> TestWorkloadBuilder<'a> { TestWorkloadBuilder { captured: false, - is_network_gateway: false, w: LocalWorkload { workload: Workload { name: name.into(), @@ -467,8 +465,8 @@ impl<'a> TestWorkloadBuilder<'a> { self } - pub fn network_gateway(mut self) -> Self { - self.is_network_gateway = true; + pub fn network_gateway(mut self, network_gateway: GatewayAddress) -> Self { + self.w.workload.network_gateway = Some(network_gateway); self } @@ -518,15 +516,17 @@ impl<'a> TestWorkloadBuilder<'a> { .namespaces .child(&self.w.workload.node, &self.w.workload.name)? }; - if self.is_network_gateway { + if self.w.workload.network_gateway.is_some() { + // This is a little inefficient, because we create the + // namespace, but never actually use it. self.w.workload.workload_ips = vec![]; - self.w.workload.network_gateway = Some(GatewayAddress { - destination: gatewayaddress::Destination::Address(NetworkAddress { - network: "".into(), - address: network_namespace.ip(), - }), - hbone_mtls_port: 15008, // FIXME - }) + // self.w.workload.network_gateway = Some(GatewayAddress { + // destination: gatewayaddress::Destination::Address(NetworkAddress { + // network: "".into(), + // address: network_namespace.ip(), + // }), + // hbone_mtls_port: 15008, // FIXME + // }) } else { self.w.workload.workload_ips = vec![network_namespace.ip()]; } diff --git a/tests/namespaced.rs b/tests/namespaced.rs index d54d4a843c..44485d9283 100644 --- a/tests/namespaced.rs +++ b/tests/namespaced.rs @@ -18,6 +18,8 @@ mod namespaced { use futures::future::poll_fn; use http_body_util::Empty; use std::collections::HashMap; + use ztunnel::state::workload::{GatewayAddress, NamespacedHostname}; + use ztunnel::state::workload::gatewayaddress::Destination; use std::net::{IpAddr, SocketAddr}; @@ -200,6 +202,7 @@ mod namespaced { let _zt = manager.deploy_ztunnel(DEFAULT_NODE).await?; + // Service that resolves to workload with ew gateway manager .service_builder("remote") .addresses(vec![NetworkAddress { @@ -211,13 +214,76 @@ mod namespaced { .register() .await?; - let ew_gtw = manager - .workload_builder("ew-gtw", "remote-node") + // Service that resolves to workload with ew gateway that uses service addressing + manager + .service_builder("remote-svc-gtw") + .addresses(vec![NetworkAddress { + network: strng::EMPTY, + address: TEST_VIP2.parse::()?, + }]) + .subject_alt_names(vec!["spiffe://cluster.local/ns/default/sa/echo".into()]) + .ports(HashMap::from([(15008u16, 15008u16)])) + .register() + .await?; + + // Service that resolves to the ew gateway. + manager + .service_builder("ew-gtw-svc") + .addresses(vec![NetworkAddress { + network: strng::EMPTY, + address: TEST_VIP3.parse::()?, + }]) + .ports(HashMap::from([(15008u16, 15008u16)])) + .register() + .await?; + + // This is the e/w gateway that is supposed to be in the remote cluster/network. + let actual_ew_gtw = manager + .workload_builder("actual-ew-gtw", "remote-node") .hbone() - .network_gateway() + .service( + "default/ew-gtw-svc.default.svc.cluster.local", + 15008u16, // These can't be right (yet, or they might be...) + 15008u16, + ) + .register() + .await?; + + // This is the workload in the local cluster that represents the workloads in the remote cluster. + // Its local in the sense that the it shows up in the local cluster's xds, but it + // represents workloads in the remote cluster. + let local_remote_workload = manager + .workload_builder("local-remote-workload", "remote-node") + .hbone() + .network_gateway(GatewayAddress { + destination: Destination::Address(NetworkAddress { + network: "".into(), + address: actual_ew_gtw.ip(), + }), + hbone_mtls_port: 15008, // FIXME + }) .service( "default/remote.default.svc.cluster.local", - 15008u16, // These can't be right + 15008u16, // These can't be right (yet, or they might be...) + 15008u16, + ) + .register() + .await?; + + // Like local_remote_workload, but the network gateway is service addressed + let local_remote_workload_svc_gtw = manager + .workload_builder("local-remote-workload-svc-gtw", "remote-node") + .hbone() + .network_gateway(GatewayAddress { + destination: Destination::Hostname(NamespacedHostname { + namespace: "default".into(), + hostname: "ew-gtw-svc.default.svc.cluster.local".into(), + }), + hbone_mtls_port: 15008, // FIXME + }) + .service( + "default/remote-svc-gtw.default.svc.cluster.local", + 15008u16, // These can't be right (yet, or they might be...) 15008u16, ) .register() @@ -234,9 +300,16 @@ mod namespaced { .await?; let echo_addr = SocketAddr::new(echo.ip(), 15008); run_hbone_server(echo, "echo", tcp::Mode::ReadWrite, WAYPOINT_MESSAGE.into())?; - run_hbone_server(ew_gtw, "ew-gtw", tcp::Mode::Forward(echo_addr), b"".into())?; + run_hbone_server( + actual_ew_gtw, + "actual-ew-gtw", + tcp::Mode::Forward(echo_addr), + b"".into(), + )?; + // No need to run local_remote_workload, as it doesn't actually exist. - run_tcp_to_hbone_client(client, manager.resolver(), &format!("{TEST_VIP}:15008"))?; + // run_tcp_to_hbone_client(client.clone(), manager.resolver(), &format!("{TEST_VIP}:15008"))?; + run_tcp_to_hbone_client(client.clone(), manager.resolver(), &format!("{TEST_VIP2}:15008"))?; Ok(()) } @@ -1250,6 +1323,8 @@ mod namespaced { } const TEST_VIP: &str = "10.10.0.1"; + const TEST_VIP2: &str = "10.10.0.2"; + const TEST_VIP3: &str = "10.10.0.3"; const SERVER_PORT: u16 = 8080; const SERVER_HOSTNAME: &str = "server.default.svc.cluster.local"; From 11b4f872bde78b3df60b31b68a67e9ab09a1dc86 Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Fri, 28 Mar 2025 11:39:25 -0400 Subject: [PATCH 18/29] svc addressing --- src/proxy/outbound.rs | 2 +- tests/namespaced.rs | 175 ++++++++++++++++++++++++++++-------------- 2 files changed, 118 insertions(+), 59 deletions(-) diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index fea8b13f6a..6a787d476e 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -482,7 +482,7 @@ impl OutboundConnection { .fetch_upstream_by_host( &source_workload, host, - 15008, //fixme + ew_gtw.hbone_mtls_port, target, ServiceResolutionMode::Standard, ) diff --git a/tests/namespaced.rs b/tests/namespaced.rs index 44485d9283..54750ae7f8 100644 --- a/tests/namespaced.rs +++ b/tests/namespaced.rs @@ -197,7 +197,7 @@ mod namespaced { } #[tokio::test] - async fn double_hbone() -> anyhow::Result<()> { + async fn double_hbone1() -> anyhow::Result<()> { let mut manager = setup_netns_test!(Shared); let _zt = manager.deploy_ztunnel(DEFAULT_NODE).await?; @@ -214,17 +214,6 @@ mod namespaced { .register() .await?; - // Service that resolves to workload with ew gateway that uses service addressing - manager - .service_builder("remote-svc-gtw") - .addresses(vec![NetworkAddress { - network: strng::EMPTY, - address: TEST_VIP2.parse::()?, - }]) - .subject_alt_names(vec!["spiffe://cluster.local/ns/default/sa/echo".into()]) - .ports(HashMap::from([(15008u16, 15008u16)])) - .register() - .await?; // Service that resolves to the ew gateway. manager @@ -233,7 +222,7 @@ mod namespaced { network: strng::EMPTY, address: TEST_VIP3.parse::()?, }]) - .ports(HashMap::from([(15008u16, 15008u16)])) + .ports(HashMap::from([(15009u16, 15008u16)])) .register() .await?; @@ -243,7 +232,7 @@ mod namespaced { .hbone() .service( "default/ew-gtw-svc.default.svc.cluster.local", - 15008u16, // These can't be right (yet, or they might be...) + 15009u16, // These can't be right (yet, or they might be...) 15008u16, ) .register() @@ -262,6 +251,11 @@ mod namespaced { }), hbone_mtls_port: 15008, // FIXME }) + .identity(identity::Identity::Spiffe { + trust_domain: "cluster.local".into(), + namespace: "default".into(), + service_account: "actual-ew-gtw".into(), + }) .service( "default/remote.default.svc.cluster.local", 15008u16, // These can't be right (yet, or they might be...) @@ -269,6 +263,70 @@ mod namespaced { ) .register() .await?; + let echo = manager + .workload_builder("echo", "remote-node2") + .register() + .await?; + + let client = manager + .workload_builder("client", DEFAULT_NODE) + .register() + .await?; + let echo_addr = SocketAddr::new(echo.ip(), 15008); + // No need to run local_remote_workload, as it doesn't actually exist. + run_hbone_server(echo.clone(), "echo", tcp::Mode::ReadWrite, WAYPOINT_MESSAGE.into())?; + run_hbone_server( + actual_ew_gtw.clone(), + "actual-ew-gtw", + tcp::Mode::Forward(echo_addr.clone()), + b"".into(), + )?; + + run_tcp_to_hbone_client(client.clone(), manager.resolver(), &format!("{TEST_VIP}:15008"))?; + + Ok(()) + } + + #[tokio::test] + async fn double_hbone2() -> anyhow::Result<()> { + let mut manager = setup_netns_test!(Shared); + + let _zt = manager.deploy_ztunnel(DEFAULT_NODE).await?; + + // Service that resolves to workload with ew gateway that uses service addressing + manager + .service_builder("remote-svc-gtw") + .addresses(vec![NetworkAddress { + network: strng::EMPTY, + address: TEST_VIP2.parse::()?, + }]) + .subject_alt_names(vec!["spiffe://cluster.local/ns/default/sa/echo".into()]) + .ports(HashMap::from([(15008u16, 15008u16)])) + .register() + .await?; + + // Service that resolves to the ew gateway. + manager + .service_builder("ew-gtw-svc") + .addresses(vec![NetworkAddress { + network: strng::EMPTY, + address: TEST_VIP3.parse::()?, + }]) + .ports(HashMap::from([(15009u16, 15008u16)])) + .register() + .await?; + + // This is the e/w gateway that is supposed to be in the remote cluster/network. + let actual_ew_gtw = manager + .workload_builder("actual-ew-gtw", "remote-node") + .hbone() + .service( + "default/ew-gtw-svc.default.svc.cluster.local", + 15009u16, // These can't be right (yet, or they might be...) + 15008u16, + ) + .register() + .await?; // Like local_remote_workload, but the network gateway is service addressed let local_remote_workload_svc_gtw = manager @@ -279,7 +337,7 @@ mod namespaced { namespace: "default".into(), hostname: "ew-gtw-svc.default.svc.cluster.local".into(), }), - hbone_mtls_port: 15008, // FIXME + hbone_mtls_port: 15009, // FIXME }) .service( "default/remote-svc-gtw.default.svc.cluster.local", @@ -299,6 +357,8 @@ mod namespaced { .register() .await?; let echo_addr = SocketAddr::new(echo.ip(), 15008); + // No need to run local_remote_workload, as it doesn't actually exist. + run_hbone_server(echo, "echo", tcp::Mode::ReadWrite, WAYPOINT_MESSAGE.into())?; run_hbone_server( actual_ew_gtw, @@ -306,9 +366,8 @@ mod namespaced { tcp::Mode::Forward(echo_addr), b"".into(), )?; - // No need to run local_remote_workload, as it doesn't actually exist. - // run_tcp_to_hbone_client(client.clone(), manager.resolver(), &format!("{TEST_VIP}:15008"))?; + run_tcp_to_hbone_client(client.clone(), manager.resolver(), &format!("{TEST_VIP2}:15008"))?; Ok(()) @@ -1206,47 +1265,47 @@ mod namespaced { #[tokio::test] async fn trust_domain_mismatch_rejected() -> anyhow::Result<()> { - let mut manager = setup_netns_test!(Shared); - let id = identity::Identity::Spiffe { - trust_domain: "clusterset.local".into(), // change to mismatched trustdomain - service_account: "my-app".into(), - namespace: "default".into(), - }; - - let _ = manager.deploy_ztunnel(DEFAULT_NODE).await?; - run_tcp_server( - manager - .workload_builder("server", DEFAULT_NODE) - .register() - .await?, - )?; - - let client = manager - .workload_builder("client", DEFAULT_NODE) - .identity(id) - .register() - .await?; - - let srv = resolve_target(manager.resolver(), "server"); - - client - .run(move || async move { - let mut tcp_stream = TcpStream::connect(&srv.to_string()).await?; - tcp_stream.write_all(b"hello world!").await?; - let mut buf = [0; 10]; - let mut buf = ReadBuf::new(&mut buf); - - let result = poll_fn(|cx| tcp_stream.poll_peek(cx, &mut buf)).await; - assert!(result.is_err()); // expect a connection reset due to TLS SAN mismatch - assert_eq!( - result.err().unwrap().kind(), - std::io::ErrorKind::ConnectionReset - ); - - Ok(()) - })? - .join() - .unwrap()?; + // let mut manager = setup_netns_test!(Shared); + // let id = identity::Identity::Spiffe { + // trust_domain: "clusterset.local".into(), // change to mismatched trustdomain + // service_account: "my-app".into(), + // namespace: "default".into(), + // }; + // + // let _ = manager.deploy_ztunnel(DEFAULT_NODE).await?; + // run_tcp_server( + // manager + // .workload_builder("server", DEFAULT_NODE) + // .register() + // .await?, + // )?; + // + // let client = manager + // .workload_builder("client", DEFAULT_NODE) + // .identity(id) + // .register() + // .await?; + // + // let srv = resolve_target(manager.resolver(), "server"); + // + // client + // .run(move || async move { + // let mut tcp_stream = TcpStream::connect(&srv.to_string()).await?; + // tcp_stream.write_all(b"hello world!").await?; + // let mut buf = [0; 10]; + // let mut buf = ReadBuf::new(&mut buf); + // + // let result = poll_fn(|cx| tcp_stream.poll_peek(cx, &mut buf)).await; + // assert!(result.is_err()); // expect a connection reset due to TLS SAN mismatch + // assert_eq!( + // result.err().unwrap().kind(), + // std::io::ErrorKind::ConnectionReset + // ); + // + // Ok(()) + // })? + // .join() + // .unwrap()?; Ok(()) } From 8b506cbf124617a6df818cdbf288a34e01740d70 Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Fri, 28 Mar 2025 15:02:22 -0400 Subject: [PATCH 19/29] Check workload network when using network gateways --- src/proxy/outbound.rs | 96 +++++++++++------------ src/state.rs | 61 --------------- src/test_helpers/linux.rs | 14 ++-- tests/namespaced.rs | 157 +++++++++++++++++++------------------- 4 files changed, 131 insertions(+), 197 deletions(-) diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index 6a787d476e..d2b0ec4844 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -458,63 +458,58 @@ impl OutboundConnection { }); }; - // Check whether we are using an E/W gateway - // FIXME: this should check if there is a network gateway AND we are in different networks - // since wl entries are absolute + // Check whether we are using an E/W gateway and sending cross network traffic if let Some(ew_gtw) = &us.workload.network_gateway { - let svc = us - .destination_service - .as_ref() - .expect("Workloads with network gateways must be service addressed."); - - let (actual_destination, upstream_sans, final_sans) = match &ew_gtw.destination { - // FIXME assume that this address is reachable and ignore the address.network - // field? - Destination::Address(address) => ( - SocketAddr::from((address.address, ew_gtw.hbone_mtls_port)), - us.workload_and_services_san(), - us.service_sans(), - ), - Destination::Hostname(host) => { - let us_gtw = self - .pi - .state - .fetch_upstream_by_host( - &source_workload, - host, - ew_gtw.hbone_mtls_port, - target, - ServiceResolutionMode::Standard, - ) - .await? - .unwrap(); - ( - SocketAddr::from((us_gtw.selected_workload_ip.unwrap(), us_gtw.port)), - us_gtw.workload_and_services_san(), - us.service_sans(), - ) - } - }; + if us.workload.network != source_workload.network { + let svc = us + .destination_service + .as_ref() + .expect("Workloads with network gateways must be service addressed."); - let hbone_target_destination = - Some(HboneAddress::SvcHostname(svc.hostname.clone(), us.port)); + let (actual_destination, upstream_sans, final_sans) = match &ew_gtw.destination { + Destination::Address(address) => ( + SocketAddr::from((address.address, ew_gtw.hbone_mtls_port)), + us.workload_and_services_san(), + us.service_sans(), + ), + Destination::Hostname(host) => { + let us_gtw = self + .pi + .state + .fetch_upstream_by_host( + &source_workload, + host, + ew_gtw.hbone_mtls_port, + target, + ServiceResolutionMode::Standard, + ) + .await? + .unwrap(); + ( + SocketAddr::from((us_gtw.selected_workload_ip.unwrap(), us_gtw.port)), + us_gtw.workload_and_services_san(), + us.service_sans(), + ) + } + }; + let hbone_target_destination = + Some(HboneAddress::SvcHostname(svc.hostname.clone(), us.port)); - return Ok(Request { - protocol: ClientProtocol::DOUBLEHBONE, - source: source_workload, - hbone_target_destination, - actual_destination_workload: Some(us.workload.clone()), - intended_destination_service: us.destination_service.clone(), - actual_destination, - upstream_sans, - final_sans, - }); + return Ok(Request { + protocol: ClientProtocol::DOUBLEHBONE, + source: source_workload, + hbone_target_destination, + actual_destination_workload: Some(us.workload.clone()), + intended_destination_service: us.destination_service.clone(), + actual_destination, + upstream_sans, + final_sans, + }); + } } // We are not using a network gateway and there is no workload address. - // - // let from_waypoint = proxy::check_from_waypoint( state, &us.workload, @@ -642,7 +637,6 @@ struct Request { // The identity of workload that will ultimately process this request. // This field only matters if we need to know both the identity of the next hop, as well as the // final hop (currently, this is only double HBONE). - // fixme is this even necessary? final_sans: Vec, } diff --git a/src/state.rs b/src/state.rs index 287671f4a3..4c233bfde6 100644 --- a/src/state.rs +++ b/src/state.rs @@ -828,34 +828,12 @@ impl DemandProxyState { .find_upstream(network, source_workload, addr, resolution_mode) // Drop the lock }; - // deal with network gateway - // if let Some((wl, port, service)) = upstream.as_ref() { - // if let Some(ref network_gateway) = wl.network_gateway { - // self.handle_network_gateway(network_gateway).await; - // } - // } tracing::trace!(%addr, ?upstream, "fetch_upstream"); // package into a finalize upstream and pick/resolve addresses self.finalize_upstream(source_workload, addr, upstream) .await } - // Here, we want to handle the case where there is a network gateway. - // This is only if we find an upstream workload and there is a network gateway. - // We want this to look at the workloads network gateway and parse out the workload ip - // and if there is an hbone port override. - // Also, we might have to do another svc/vip lookup if the network gateway is a hostname. - // We also have to be careful to not infinite loop if the network gateway workload itself has a - // network gateway. - // async fn handle_network_gateway(&self, network_gateway: &GatewayAddress) { - // let x = &network_gateway.destination; - // match x { - // Destination::Address(address) => todo!, - // Destination::Hostname(h) => todo!, - // - // } - // } - pub async fn finalize_upstream( &self, source_workload: &Workload, @@ -876,45 +854,6 @@ impl DemandProxyState { ip_family_restriction, ) .await?; // if we can't load balance just return the error - // if let Some(network_gateway) = - // wl.network_gateway.as_ref() - // { - // match &network_gateway.destination { - // Destination::Address(network_address) => ( - // // Use network address if it exists - // network_address.address, - // Some(network_gateway.hbone_mtls_port), - // ), - // Destination::Hostname(hostname) => { - // // Look in the service registry for this. - // // HERE FIXME do an SVC lookup - // let svc = self - // .read() - // .find_service_by_hostname(&hostname.hostname)?.first().ok_or(Error::NoHostname("Workloads referenced by Network Gateways destination hostnames must have IPs".into()))?; - // ( - // self.resolve_workload_address( - // &wl, - // source_workload, - // original_target_address, - // ) - // .await?, - // Some(network_gateway.hbone_mtls_port), - // ) - // } - // } - // } else { - // ( - // self.pick_workload_destination_or_resolve( - // &wl, - // source_workload, - // original_target_address, - // ip_family_restriction, - // ) - // .await?, - // None, - // ) // if we can't load balance just return the error - // }; - let res = Upstream { workload: wl, selected_workload_ip, diff --git a/src/test_helpers/linux.rs b/src/test_helpers/linux.rs index 031f83c90a..bcbc409530 100644 --- a/src/test_helpers/linux.rs +++ b/src/test_helpers/linux.rs @@ -16,6 +16,7 @@ use crate::config::{ConfigSource, ProxyMode}; use crate::rbac::Authorization; use crate::state::service::{Endpoint, Service}; use crate::state::workload::{HealthStatus, Workload, gatewayaddress}; +use crate::strng::Strng; use crate::test_helpers::app::TestApp; use crate::test_helpers::netns::{Namespace, Resolver}; use crate::test_helpers::*; @@ -420,6 +421,12 @@ impl<'a> TestWorkloadBuilder<'a> { self } + /// + pub fn network (mut self, network: Strng) -> Self { + self.w.workload.network = network; + self + } + pub fn identity(mut self, identity: identity::Identity) -> Self { match identity { identity::Identity::Spiffe { @@ -520,13 +527,6 @@ impl<'a> TestWorkloadBuilder<'a> { // This is a little inefficient, because we create the // namespace, but never actually use it. self.w.workload.workload_ips = vec![]; - // self.w.workload.network_gateway = Some(GatewayAddress { - // destination: gatewayaddress::Destination::Address(NetworkAddress { - // network: "".into(), - // address: network_namespace.ip(), - // }), - // hbone_mtls_port: 15008, // FIXME - // }) } else { self.w.workload.workload_ips = vec![network_namespace.ip()]; } diff --git a/tests/namespaced.rs b/tests/namespaced.rs index 54750ae7f8..82d87e7cdd 100644 --- a/tests/namespaced.rs +++ b/tests/namespaced.rs @@ -18,8 +18,8 @@ mod namespaced { use futures::future::poll_fn; use http_body_util::Empty; use std::collections::HashMap; - use ztunnel::state::workload::{GatewayAddress, NamespacedHostname}; use ztunnel::state::workload::gatewayaddress::Destination; + use ztunnel::state::workload::{GatewayAddress, NamespacedHostname}; use std::net::{IpAddr, SocketAddr}; @@ -203,6 +203,9 @@ mod namespaced { let _zt = manager.deploy_ztunnel(DEFAULT_NODE).await?; // Service that resolves to workload with ew gateway + // The 8080 port mappings don't actually matter because the + // final ztunnel is actually an hbone echo server that doesn't + // forward anything. manager .service_builder("remote") .addresses(vec![NetworkAddress { @@ -210,19 +213,7 @@ mod namespaced { address: TEST_VIP.parse::()?, }]) .subject_alt_names(vec!["spiffe://cluster.local/ns/default/sa/echo".into()]) - .ports(HashMap::from([(15008u16, 15008u16)])) - .register() - .await?; - - - // Service that resolves to the ew gateway. - manager - .service_builder("ew-gtw-svc") - .addresses(vec![NetworkAddress { - network: strng::EMPTY, - address: TEST_VIP3.parse::()?, - }]) - .ports(HashMap::from([(15009u16, 15008u16)])) + .ports(HashMap::from([(8080, 8080)])) .register() .await?; @@ -230,37 +221,32 @@ mod namespaced { let actual_ew_gtw = manager .workload_builder("actual-ew-gtw", "remote-node") .hbone() - .service( - "default/ew-gtw-svc.default.svc.cluster.local", - 15009u16, // These can't be right (yet, or they might be...) - 15008u16, - ) + .network("remote".into()) .register() .await?; // This is the workload in the local cluster that represents the workloads in the remote cluster. // Its local in the sense that the it shows up in the local cluster's xds, but it // represents workloads in the remote cluster. + // Its a little weird because we do give it a namespaced/ip, + // but that's because of how the tests infra works. let local_remote_workload = manager .workload_builder("local-remote-workload", "remote-node") .hbone() + .network("remote".into()) .network_gateway(GatewayAddress { destination: Destination::Address(NetworkAddress { - network: "".into(), + network: "remote".into(), address: actual_ew_gtw.ip(), }), - hbone_mtls_port: 15008, // FIXME + hbone_mtls_port: 15008, }) .identity(identity::Identity::Spiffe { trust_domain: "cluster.local".into(), namespace: "default".into(), service_account: "actual-ew-gtw".into(), }) - .service( - "default/remote.default.svc.cluster.local", - 15008u16, // These can't be right (yet, or they might be...) - 15008u16, - ) + .service("default/remote.default.svc.cluster.local", 8080, 8080) .register() .await?; let echo = manager @@ -272,17 +258,27 @@ mod namespaced { .workload_builder("client", DEFAULT_NODE) .register() .await?; - let echo_addr = SocketAddr::new(echo.ip(), 15008); + let echo_hbone_addr = SocketAddr::new(echo.ip(), 15008); + // No need to run local_remote_workload, as it doesn't actually exist. - run_hbone_server(echo.clone(), "echo", tcp::Mode::ReadWrite, WAYPOINT_MESSAGE.into())?; + run_hbone_server( + echo.clone(), + "echo", + tcp::Mode::ReadWrite, + WAYPOINT_MESSAGE.into(), + )?; run_hbone_server( actual_ew_gtw.clone(), "actual-ew-gtw", - tcp::Mode::Forward(echo_addr.clone()), + tcp::Mode::Forward(echo_hbone_addr.clone()), b"".into(), )?; - run_tcp_to_hbone_client(client.clone(), manager.resolver(), &format!("{TEST_VIP}:15008"))?; + run_tcp_to_hbone_client( + client.clone(), + manager.resolver(), + &format!("{TEST_VIP}:8080"), + )?; Ok(()) } @@ -301,7 +297,7 @@ mod namespaced { address: TEST_VIP2.parse::()?, }]) .subject_alt_names(vec!["spiffe://cluster.local/ns/default/sa/echo".into()]) - .ports(HashMap::from([(15008u16, 15008u16)])) + .ports(HashMap::from([(8080, 8080)])) .register() .await?; @@ -322,9 +318,10 @@ mod namespaced { .hbone() .service( "default/ew-gtw-svc.default.svc.cluster.local", - 15009u16, // These can't be right (yet, or they might be...) + 15009u16, 15008u16, ) + .network("remote".into()) .register() .await?; @@ -332,17 +329,18 @@ mod namespaced { let local_remote_workload_svc_gtw = manager .workload_builder("local-remote-workload-svc-gtw", "remote-node") .hbone() + .network("remote".into()) .network_gateway(GatewayAddress { destination: Destination::Hostname(NamespacedHostname { namespace: "default".into(), hostname: "ew-gtw-svc.default.svc.cluster.local".into(), }), - hbone_mtls_port: 15009, // FIXME + hbone_mtls_port: 15009, }) .service( "default/remote-svc-gtw.default.svc.cluster.local", - 15008u16, // These can't be right (yet, or they might be...) - 15008u16, + 8080, + 8080, ) .register() .await?; @@ -367,8 +365,11 @@ mod namespaced { b"".into(), )?; - - run_tcp_to_hbone_client(client.clone(), manager.resolver(), &format!("{TEST_VIP2}:15008"))?; + run_tcp_to_hbone_client( + client.clone(), + manager.resolver(), + &format!("{TEST_VIP2}:8080"), + )?; Ok(()) } @@ -1265,47 +1266,47 @@ mod namespaced { #[tokio::test] async fn trust_domain_mismatch_rejected() -> anyhow::Result<()> { - // let mut manager = setup_netns_test!(Shared); - // let id = identity::Identity::Spiffe { - // trust_domain: "clusterset.local".into(), // change to mismatched trustdomain - // service_account: "my-app".into(), - // namespace: "default".into(), - // }; - // - // let _ = manager.deploy_ztunnel(DEFAULT_NODE).await?; - // run_tcp_server( - // manager - // .workload_builder("server", DEFAULT_NODE) - // .register() - // .await?, - // )?; - // - // let client = manager - // .workload_builder("client", DEFAULT_NODE) - // .identity(id) - // .register() - // .await?; - // - // let srv = resolve_target(manager.resolver(), "server"); - // - // client - // .run(move || async move { - // let mut tcp_stream = TcpStream::connect(&srv.to_string()).await?; - // tcp_stream.write_all(b"hello world!").await?; - // let mut buf = [0; 10]; - // let mut buf = ReadBuf::new(&mut buf); - // - // let result = poll_fn(|cx| tcp_stream.poll_peek(cx, &mut buf)).await; - // assert!(result.is_err()); // expect a connection reset due to TLS SAN mismatch - // assert_eq!( - // result.err().unwrap().kind(), - // std::io::ErrorKind::ConnectionReset - // ); - // - // Ok(()) - // })? - // .join() - // .unwrap()?; + let mut manager = setup_netns_test!(Shared); + let id = identity::Identity::Spiffe { + trust_domain: "clusterset.local".into(), // change to mismatched trustdomain + service_account: "my-app".into(), + namespace: "default".into(), + }; + + let _ = manager.deploy_ztunnel(DEFAULT_NODE).await?; + run_tcp_server( + manager + .workload_builder("server", DEFAULT_NODE) + .register() + .await?, + )?; + + let client = manager + .workload_builder("client", DEFAULT_NODE) + .identity(id) + .register() + .await?; + + let srv = resolve_target(manager.resolver(), "server"); + + client + .run(move || async move { + let mut tcp_stream = TcpStream::connect(&srv.to_string()).await?; + tcp_stream.write_all(b"hello world!").await?; + let mut buf = [0; 10]; + let mut buf = ReadBuf::new(&mut buf); + + let result = poll_fn(|cx| tcp_stream.poll_peek(cx, &mut buf)).await; + assert!(result.is_err()); // expect a connection reset due to TLS SAN mismatch + assert_eq!( + result.err().unwrap().kind(), + std::io::ErrorKind::ConnectionReset + ); + + Ok(()) + })? + .join() + .unwrap()?; Ok(()) } From 862c6f8d68534b3ab4e474ba50543636586403f9 Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Fri, 28 Mar 2025 15:56:30 -0400 Subject: [PATCH 20/29] Use dummy SNI --- src/proxy/outbound.rs | 4 ---- src/proxy/pool.rs | 10 ++-------- src/state.rs | 18 ------------------ src/tls/workload.rs | 12 +++++++++--- tests/namespaced.rs | 28 ++++------------------------ 5 files changed, 15 insertions(+), 57 deletions(-) diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index d2b0ec4844..a5b1a24b8c 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -254,7 +254,6 @@ impl OutboundConnection { let tls_stream = connector .connect( upgraded, - rustls::pki_types::ServerName::IpAddress(wl_key.dst.ip().into()), ) .await?; @@ -276,9 +275,6 @@ impl OutboundConnection { let _ = drain_tx.send(true); let _ = driver_task.await; - // Here, there is an implicit, drop(conn_client). - // Its really important that this happens AFTER driver_task finishes. - // Otherwise, TLS connections do not terminate gracefully. res } diff --git a/src/proxy/pool.rs b/src/proxy/pool.rs index 6610b277fb..9d128505cd 100644 --- a/src/proxy/pool.rs +++ b/src/proxy/pool.rs @@ -89,14 +89,8 @@ impl ConnSpawner { _ => e.into(), })?; - let dest = rustls::pki_types::ServerName::IpAddress( - tcp_stream - .peer_addr() - .expect("peer_addr must be set") - .ip() - .into(), - ); - let tls_stream = connector.connect(tcp_stream, dest).await?; + + let tls_stream = connector.connect(tcp_stream).await?; trace!("connector connected, handshaking"); let (sender, _) = h2::client::spawn_connection( self.cfg.clone(), diff --git a/src/state.rs b/src/state.rs index 4c233bfde6..cc65a1d099 100644 --- a/src/state.rs +++ b/src/state.rs @@ -76,24 +76,6 @@ pub struct Upstream { } impl Upstream { - // /// If there is a network gateway, use :. Otherwise, use - // /// :. Fortunately, for double-hbone, the authority/host is the same - // /// for inner and outer CONNECT request, so we can reuse this for inner double hbone, - // /// outer double hbone, and normal hbone. - // pub fn hbone_target(&self) -> HboneAddress { - // if self.workload.network_gateway.is_some() { - // let svc = self - // .destination_service - // .as_ref() - // .expect("Workloads with network gateways must be service addressed."); - // HboneAddress::SvcHostname( - // format!("{}.{}", svc.namespace, svc.hostname).into(), - // self.port, - // ) - // } else { - // HboneAddress::SocketAddr(self.workload_socket_addr()) - // } - // } pub fn workload_socket_addr(&self) -> Option { self.selected_workload_ip diff --git a/src/tls/workload.rs b/src/tls/workload.rs index bd708a5896..7bb19beb43 100644 --- a/src/tls/workload.rs +++ b/src/tls/workload.rs @@ -21,7 +21,7 @@ use crate::tls::{ServerCertProvider, TlsError}; use futures_util::TryFutureExt; use rustls::client::danger::{HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier}; -use rustls::pki_types::{CertificateDer, ServerName, UnixTime}; +use rustls::pki_types::{CertificateDer, DnsName, ServerName, UnixTime}; use rustls::server::ParsedCertificate; use rustls::server::danger::{ClientCertVerified, ClientCertVerifier}; use rustls::{ @@ -35,10 +35,17 @@ use tokio::io::{AsyncRead, AsyncWrite}; use crate::strng::Strng; use crate::tls; +use once_cell::sync::Lazy; use tokio::net::TcpStream; use tokio_rustls::client; use tracing::{debug, trace}; +// Dummy domain used for TLS SNI because the actual domain isn't relevant +// since we use SANs for identity verification. +// dummy.istio.io is a valid domain, so the unwrap should never fail. +static DUMMY_DOMAIN: Lazy> = + Lazy::new(|| ServerName::DnsName(DnsName::try_from_str("dummy.istio.io").unwrap())); + #[derive(Clone, Debug)] pub struct InboundAcceptor { provider: F, @@ -167,13 +174,12 @@ impl OutboundConnector { pub async fn connect( self, stream: IO, - domain: ServerName<'static>, ) -> Result, io::Error> where IO: AsyncRead + AsyncWrite + Unpin, { let c = tokio_rustls::TlsConnector::from(self.client_config); - c.connect(domain, stream).await + c.connect(DUMMY_DOMAIN.clone(), stream).await } } diff --git a/tests/namespaced.rs b/tests/namespaced.rs index 82d87e7cdd..68f7fd2497 100644 --- a/tests/namespaced.rs +++ b/tests/namespaced.rs @@ -1003,14 +1003,7 @@ mod namespaced { let hbone = SocketAddr::new(srv.ip(), 15008); let tcp_stream = TcpStream::connect(hbone).await.unwrap(); - let dest = rustls::pki_types::ServerName::IpAddress( - tcp_stream - .peer_addr() - .expect("peer_addr must be set") - .ip() - .into(), - ); - let tls_stream = connector.connect(tcp_stream, dest).await.unwrap(); + let tls_stream = connector.connect(tcp_stream).await.unwrap(); let (mut request_sender, connection) = builder.handshake(TokioIo::new(tls_stream)).await.unwrap(); @@ -1075,14 +1068,7 @@ mod namespaced { .await .unwrap(); - let dest = rustls::pki_types::ServerName::IpAddress( - tcp_stream - .peer_addr() - .expect("peer_addr must be set") - .ip() - .into(), - ); - let tls_stream = connector.connect(tcp_stream, dest).await.unwrap(); + let tls_stream = connector.connect(tcp_stream).await.unwrap(); let (mut request_sender, connection) = builder.handshake(TokioIo::new(tls_stream)).await.unwrap(); @@ -1165,14 +1151,8 @@ mod namespaced { let tcp_stream = TcpStream::connect(SocketAddr::from((srv.ip(), 15008))) .await .unwrap(); - let dest = rustls::pki_types::ServerName::IpAddress( - tcp_stream - .peer_addr() - .expect("peer_addr must be set") - .ip() - .into(), - ); - let tls_stream = connector.connect(tcp_stream, dest).await.unwrap(); + + let tls_stream = connector.connect(tcp_stream).await.unwrap(); let (mut request_sender, connection) = builder.handshake(TokioIo::new(tls_stream)).await.unwrap(); // spawn a task to poll the connection and drive the HTTP state From d54a820a61770b9f0cc854221dd5c2d063506363 Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Fri, 28 Mar 2025 16:00:30 -0400 Subject: [PATCH 21/29] remove extra derive --- src/proxy/outbound.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index a5b1a24b8c..b11c47fe82 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -606,7 +606,7 @@ fn baggage(r: &Request, cluster: String) -> String { ) } -#[derive(Debug, Clone)] +#[derive(Debug)] struct Request { protocol: ClientProtocol, // Source workload sending the request From 6d398efe5fa97ee445520beccfde20d2e01d3130 Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Mon, 31 Mar 2025 12:14:39 -0400 Subject: [PATCH 22/29] lint --- src/proxy/pool.rs | 2 -- tests/namespaced.rs | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/proxy/pool.rs b/src/proxy/pool.rs index 9d128505cd..62fef60f92 100644 --- a/src/proxy/pool.rs +++ b/src/proxy/pool.rs @@ -610,8 +610,6 @@ mod test { .unwrap() }; - let start = Instant::now(); - let c = pool.send_request_pooled(&key.clone(), req()).await.unwrap(); let mut c = TokioH2Stream::new(c); c.write_all(b"abcde").await.unwrap(); diff --git a/tests/namespaced.rs b/tests/namespaced.rs index 68f7fd2497..4da178237f 100644 --- a/tests/namespaced.rs +++ b/tests/namespaced.rs @@ -230,7 +230,7 @@ mod namespaced { // represents workloads in the remote cluster. // Its a little weird because we do give it a namespaced/ip, // but that's because of how the tests infra works. - let local_remote_workload = manager + let _local_remote_workload = manager .workload_builder("local-remote-workload", "remote-node") .hbone() .network("remote".into()) @@ -326,7 +326,7 @@ mod namespaced { .await?; // Like local_remote_workload, but the network gateway is service addressed - let local_remote_workload_svc_gtw = manager + let _local_remote_workload_svc_gtw = manager .workload_builder("local-remote-workload-svc-gtw", "remote-node") .hbone() .network("remote".into()) From 4874f080656bc407111bb83af0a5a6cf6b12dc6e Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Tue, 1 Apr 2025 11:16:23 -0400 Subject: [PATCH 23/29] more tests --- src/proxy/outbound.rs | 225 +++++++++++++++++++++++++++++++++--------- tests/namespaced.rs | 36 ++++++- 2 files changed, 210 insertions(+), 51 deletions(-) diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index afffd5c10d..01cdc11171 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -39,7 +39,7 @@ use crate::state::ServiceResolutionMode; use crate::state::service::ServiceDescription; use crate::state::workload::OutboundProtocol; use crate::state::workload::gatewayaddress::Destination; -use crate::state::workload::{NetworkAddress, InboundProtocol, Workload, address::Address}; +use crate::state::workload::{InboundProtocol, NetworkAddress, Workload, address::Address}; use crate::{assertions, copy, proxy, socket}; use super::h2::TokioH2Stream; @@ -249,11 +249,7 @@ impl OutboundConnection { .fetch_certificate() .await?; let connector = cert.outbound_connector(wl_key.dst_id.clone())?; - let tls_stream = connector - .connect( - upgraded, - ) - .await?; + let tls_stream = connector.connect(upgraded).await?; // Spawn inner CONNECT tunnel let (drain_tx, drain_rx) = tokio::sync::watch::channel(false); @@ -354,25 +350,23 @@ impl OutboundConnection { } fn conn_metrics_from_request(req: &Request) -> ConnectionOpen { - let derived_source = if req.protocol == OutboundProtocol::HBONE { - Some(DerivedWorkload { - // We are going to do mTLS, so report our identity - identity: Some(req.source.as_ref().identity()), - ..Default::default() - }) - } else { - None + let (derived_source, security_policy) = match req.protocol { + OutboundProtocol::HBONE | OutboundProtocol::DOUBLEHBONE => ( + Some(DerivedWorkload { + // We are going to do mTLS, so report our identity + identity: Some(req.source.as_ref().identity()), + ..Default::default() + }), + metrics::SecurityPolicy::mutual_tls, + ), + OutboundProtocol::TCP => (None, metrics::SecurityPolicy::unknown), }; ConnectionOpen { reporter: Reporter::source, derived_source, source: Some(req.source.clone()), destination: req.actual_destination_workload.clone(), - connection_security_policy: if req.protocol == OutboundProtocol::HBONE { - metrics::SecurityPolicy::mutual_tls - } else { - metrics::SecurityPolicy::unknown - }, + connection_security_policy: security_policy, destination_service: req.intended_destination_service.clone(), } } @@ -652,7 +646,9 @@ mod tests { use crate::xds::istio::workload::Workload as XdsWorkload; use crate::xds::istio::workload::address::Type as XdsAddressType; use crate::xds::istio::workload::{IpFamilies, Port}; - use crate::xds::istio::workload::{NetworkAddress as XdsNetworkAddress, PortList}; + use crate::xds::istio::workload::{ + NamespacedHostname as XdsNamespacedHostname, NetworkAddress as XdsNetworkAddress, PortList, + }; use crate::xds::istio::workload::{NetworkMode, Service as XdsService}; use crate::{identity, xds}; @@ -796,6 +792,137 @@ mod tests { .await; } + #[tokio::test] + async fn build_request_double_hbone() { + run_build_request_multi( + "127.0.0.1", + "127.0.0.3:80", + vec![ + XdsAddressType::Service(XdsService { + hostname: "example.com".to_string(), + addresses: vec![XdsNetworkAddress { + network: "".to_string(), + address: vec![127, 0, 0, 3], + }], + ports: vec![Port { + service_port: 80, + target_port: 8080, + }], + ..Default::default() + }), + XdsAddressType::Workload(XdsWorkload { + uid: "cluster1//v1/Pod/default/remote-pod".to_string(), + addresses: vec![], + network: "remote".to_string(), + network_gateway: Some(xds::istio::workload::GatewayAddress { + destination: Some( + xds::istio::workload::gateway_address::Destination::Address( + XdsNetworkAddress { + network: "".to_string(), + address: vec![1, 2, 3, 4], + }, + ), + ), + hbone_mtls_port: 15009, + }), + services: std::collections::HashMap::from([( + "/example.com".to_string(), + PortList { + ports: vec![Port { + service_port: 80, + target_port: 8080, + }], + }, + )]), + ..Default::default() + }), + ], + Some(ExpectedRequest { + protocol: OutboundProtocol::DOUBLEHBONE, + hbone_destination: "example.com:8080", + destination: "1.2.3.4:15009", + }), + ) + .await; + run_build_request_multi( + "127.0.0.1", + "127.0.0.3:80", + vec![ + XdsAddressType::Service(XdsService { + hostname: "example.com".to_string(), + addresses: vec![XdsNetworkAddress { + network: "".to_string(), + address: vec![127, 0, 0, 3], + }], + ports: vec![Port { + service_port: 80, + target_port: 8080, + }], + ..Default::default() + }), + XdsAddressType::Service(XdsService { + hostname: "ew-gtw".to_string(), + addresses: vec![XdsNetworkAddress { + network: "".to_string(), + address: vec![127, 0, 0, 4], + }], + ports: vec![Port { + service_port: 15009, + target_port: 15009, + }], + ..Default::default() + }), + XdsAddressType::Workload(XdsWorkload { + uid: "cluster1//v1/Pod/default/remote-pod".to_string(), + addresses: vec![Bytes::copy_from_slice(&[127, 0, 0, 6])], + network: "remote".to_string(), + network_gateway: Some(xds::istio::workload::GatewayAddress { + hbone_mtls_port: 15009, + destination: Some( + xds::istio::workload::gateway_address::Destination::Hostname( + XdsNamespacedHostname { + namespace: Default::default(), + hostname: "ew-gtw".into(), + }, + ), + ), + }), + services: std::collections::HashMap::from([( + "/example.com".to_string(), + PortList { + ports: vec![Port { + service_port: 80, + target_port: 8080, + }], + }, + )]), + ..Default::default() + }), + XdsAddressType::Workload(XdsWorkload { + uid: "cluster1//v1/Pod/default/ew-gtw".to_string(), + addresses: vec![Bytes::copy_from_slice(&[127, 0, 0, 5])], + network: "remote".to_string(), + services: std::collections::HashMap::from([( + "/ew-gtw".to_string(), + PortList { + ports: vec![Port { + service_port: 15009, + target_port: 15008, + }], + }, + )]), + ..Default::default() + }), + ], + Some(ExpectedRequest { + protocol: OutboundProtocol::DOUBLEHBONE, + hbone_destination: "example.com:8080", + destination: "127.0.0.5:15008", + }), + ) + .await; + } + #[tokio::test] async fn build_request_known_dest_remote_node_tcp() { run_build_request( @@ -888,35 +1015,35 @@ mod tests { .await; } - #[tokio::test] - async fn build_request_source_waypoint() { - run_build_request( - "127.0.0.2", - "127.0.0.1:80", - XdsAddressType::Workload(XdsWorkload { - uid: "cluster1//v1/Pod/default/my-pod".to_string(), - addresses: vec![Bytes::copy_from_slice(&[127, 0, 0, 2])], - waypoint: Some(xds::istio::workload::GatewayAddress { - destination: Some(xds::istio::workload::gateway_address::Destination::Address( - XdsNetworkAddress { - network: "".to_string(), - address: [127, 0, 0, 10].to_vec(), - }, - )), - hbone_mtls_port: 15008, - }), - ..Default::default() - }), - // Even though source has a waypoint, we don't use it - Some(ExpectedRequest { - protocol: OutboundProtocol::TCP, - hbone_destination: "", - destination: "127.0.0.1:80", - }), - ) - .await; - } - + // #[tokio::test] + // async fn build_request_source_waypoint() { + // run_build_request( + // "127.0.0.2", + // "127.0.0.1:80", + // XdsAddressType::Workload(XdsWorkload { + // uid: "cluster1//v1/Pod/default/my-pod".to_string(), + // addresses: vec![Bytes::copy_from_slice(&[127, 0, 0, 2])], + // waypoint: Some(xds::istio::workload::GatewayAddress { + // destination: Some(xds::istio::workload::gateway_address::Destination::( + // XdsNamespacedHostname { + // namespace: "".into(), + // hostname: "ew-gtw.default.svc.cluster.local".to_vec(), + // }, + // )), + // hbone_mtls_port: 15008, + // }), + // ..Default::default() + // }), + // // Even though source has a waypoint, we don't use it + // Some(ExpectedRequest { + // protocol: OutboundProtocol::TCP, + // hbone_destination: "", + // destination: "127.0.0.1:80", + // }), + // ) + // .await; + // } + // #[tokio::test] async fn build_request_destination_waypoint() { run_build_request( diff --git a/tests/namespaced.rs b/tests/namespaced.rs index 4da178237f..1913e398ec 100644 --- a/tests/namespaced.rs +++ b/tests/namespaced.rs @@ -200,7 +200,7 @@ mod namespaced { async fn double_hbone1() -> anyhow::Result<()> { let mut manager = setup_netns_test!(Shared); - let _zt = manager.deploy_ztunnel(DEFAULT_NODE).await?; + let zt = manager.deploy_ztunnel(DEFAULT_NODE).await?; // Service that resolves to workload with ew gateway // The 8080 port mappings don't actually matter because the @@ -258,6 +258,7 @@ mod namespaced { .workload_builder("client", DEFAULT_NODE) .register() .await?; + let echo_hbone_addr = SocketAddr::new(echo.ip(), 15008); // No need to run local_remote_workload, as it doesn't actually exist. @@ -280,6 +281,37 @@ mod namespaced { &format!("{TEST_VIP}:8080"), )?; + let metrics = [ + (CONNECTIONS_OPENED, 1), + (CONNECTIONS_CLOSED, 1), + (BYTES_RECV, REQ_SIZE), + (BYTES_SENT, HBONE_REQ_SIZE), + ]; + verify_metrics(&zt, &metrics, &source_labels()).await; + + let sent = format!("{REQ_SIZE}"); + let recv = format!("{HBONE_REQ_SIZE}"); + let dst_addr = format!("{}:15008", actual_ew_gtw.ip()); + let want = HashMap::from([ + ("scope", "access"), + ("src.workload", "client"), + ("dst.workload", "local-remote-workload"), + ("dst.hbone_addr", "remote.default.svc.cluster.local:8080"), + ("dst.addr", &dst_addr), + ("bytes_sent", &sent), + ("bytes_recv", &recv), + ("direction", "outbound"), + ("message", "connection complete"), + ( + "src.identity", + "spiffe://cluster.local/ns/default/sa/client", + ), + ( + "dst.identity", + "spiffe://cluster.local/ns/default/sa/actual-ew-gtw", + ), + ]); + telemetry::testing::assert_contains(want); Ok(()) } @@ -318,7 +350,7 @@ mod namespaced { .hbone() .service( "default/ew-gtw-svc.default.svc.cluster.local", - 15009u16, + 15009u16, 15008u16, ) .network("remote".into()) From d7cb313ba840f9fb021e06784ab3cb3c335d2132 Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Wed, 2 Apr 2025 15:41:55 -0400 Subject: [PATCH 24/29] Drop some uneeded changers --- examples/inpodserver.rs | 3 -- src/proxy/h2.rs | 4 ++- src/proxy/keylog | 0 src/proxy/outbound.rs | 62 +++++++++++++++++++-------------------- src/state.rs | 21 +++++-------- src/test_helpers/linux.rs | 1 - 6 files changed, 42 insertions(+), 49 deletions(-) create mode 100644 src/proxy/keylog diff --git a/examples/inpodserver.rs b/examples/inpodserver.rs index 4109db26c4..cf131ce976 100644 --- a/examples/inpodserver.rs +++ b/examples/inpodserver.rs @@ -25,8 +25,6 @@ const PROXY_WORKLOAD_INFO: &str = "PROXY_WORKLOAD_INFO"; #[cfg(target_os = "linux")] #[tokio::main] async fn main() { - use std::time::Duration; - let uds = std::env::var("INPOD_UDS").unwrap(); let pwi = match parse_proxy_workload_info() { Ok(pwi) => pwi, @@ -52,7 +50,6 @@ async fn main() { .await .unwrap(); sender.wait_forever().await.unwrap(); - tokio::time::sleep(Duration::from_secs(99999999)).await; } fn parse_proxy_workload_info() -> Result { diff --git a/src/proxy/h2.rs b/src/proxy/h2.rs index 2130932545..c1d1ab1efc 100644 --- a/src/proxy/h2.rs +++ b/src/proxy/h2.rs @@ -160,7 +160,7 @@ impl tokio::io::AsyncRead for TokioH2Stream { if buf.remaining() < bytes.len() { Err(Error::new( std::io::ErrorKind::Other, - format!("Would overflow buffer of with {} remaining", buf.remaining()), + format!("kould overflow buffer of with {} remaining", buf.remaining()), )) } else { buf.put(bytes); @@ -303,3 +303,5 @@ fn h2_to_io_error(e: h2::Error) -> std::io::Error { } } + + diff --git a/src/proxy/keylog b/src/proxy/keylog new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index 01cdc11171..2fd0054ba6 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -1015,35 +1015,35 @@ mod tests { .await; } - // #[tokio::test] - // async fn build_request_source_waypoint() { - // run_build_request( - // "127.0.0.2", - // "127.0.0.1:80", - // XdsAddressType::Workload(XdsWorkload { - // uid: "cluster1//v1/Pod/default/my-pod".to_string(), - // addresses: vec![Bytes::copy_from_slice(&[127, 0, 0, 2])], - // waypoint: Some(xds::istio::workload::GatewayAddress { - // destination: Some(xds::istio::workload::gateway_address::Destination::( - // XdsNamespacedHostname { - // namespace: "".into(), - // hostname: "ew-gtw.default.svc.cluster.local".to_vec(), - // }, - // )), - // hbone_mtls_port: 15008, - // }), - // ..Default::default() - // }), - // // Even though source has a waypoint, we don't use it - // Some(ExpectedRequest { - // protocol: OutboundProtocol::TCP, - // hbone_destination: "", - // destination: "127.0.0.1:80", - // }), - // ) - // .await; - // } - // + #[tokio::test] + async fn build_request_source_waypoint() { + run_build_request( + "127.0.0.2", + "127.0.0.1:80", + XdsAddressType::Workload(XdsWorkload { + uid: "cluster1//v1/Pod/default/my-pod".to_string(), + addresses: vec![Bytes::copy_from_slice(&[127, 0, 0, 2])], + waypoint: Some(xds::istio::workload::GatewayAddress { + destination: Some(xds::istio::workload::gateway_address::Destination::Address( + XdsNetworkAddress { + network: "".to_string(), + address: [127, 0, 0, 10].to_vec(), + }, + )), + hbone_mtls_port: 15008, + }), + ..Default::default() + }), + // Even though source has a waypoint, we don't use it + Some(ExpectedRequest { + protocol: OutboundProtocol::TCP, + hbone_destination: "", + destination: "127.0.0.1:80", + }), + ) + .await; + } + #[tokio::test] async fn build_request_destination_waypoint() { run_build_request( @@ -1274,8 +1274,8 @@ mod tests { xds.clone(), // Traffic to the service should go to the pod in the service Some(ExpectedRequest { - protocol: OutboundProtocol::TCP, destination: "127.0.0.2:80", + protocol: OutboundProtocol::TCP, hbone_destination: "", }), ) @@ -1295,8 +1295,8 @@ mod tests { xds.clone(), // Traffic to the service should go to the pod in the service Some(ExpectedRequest { - protocol: OutboundProtocol::TCP, destination: "127.0.0.2:80", + protocol: OutboundProtocol::TCP, hbone_destination: "", }), ) diff --git a/src/state.rs b/src/state.rs index cc65a1d099..c39079b2ed 100644 --- a/src/state.rs +++ b/src/state.rs @@ -328,7 +328,7 @@ impl ProxyState { None } - pub fn find_upstream_from_service( + fn find_upstream_from_service( &self, source_workload: &Workload, svc_port: u16, @@ -601,13 +601,13 @@ impl DemandProxyState { src_workload: &Workload, original_target_address: SocketAddr, ip_family_restriction: Option, - ) -> Result<(Option, Option), Error> { + ) -> Result, Error> { // If the user requested the pod by a specific IP, use that directly. if dst_workload .workload_ips .contains(&original_target_address.ip()) { - return Ok((Some(original_target_address.ip()), None)); + return Ok(Some(original_target_address.ip())); } // They may have 1 or 2 IPs (single/dual stack) // Ensure we are meeting the Service family restriction (if any is defined). @@ -622,7 +622,7 @@ impl DemandProxyState { }) .find_or_first(|ip| ip.is_ipv6() == original_target_address.is_ipv6()) { - return Ok((Some(*ip), None)); + return Ok(Some(*ip)); } if dst_workload.hostname.is_empty() { if dst_workload.network_gateway.is_none() { @@ -633,7 +633,7 @@ impl DemandProxyState { return Err(Error::NoValidDestination(Box::new(dst_workload.clone()))); } else { // We can route through network gateway - return Ok((None, None)); + return Ok(None); } } let ip = Box::pin(self.resolve_workload_address( @@ -642,7 +642,7 @@ impl DemandProxyState { original_target_address, )) .await?; - Ok((Some(ip), None)) + Ok(Some(ip)) } async fn resolve_workload_address( @@ -793,7 +793,6 @@ impl DemandProxyState { .await } - // Find/resolve all information about the target workload (or have it passthrough). pub async fn fetch_upstream( &self, network: Strng, @@ -801,22 +800,19 @@ impl DemandProxyState { addr: SocketAddr, resolution_mode: ServiceResolutionMode, ) -> Result, Error> { - // update cache? self.fetch_address(&network_addr(network.clone(), addr.ip())) .await; - // find upstream and select destination workload let upstream = { self.read() .find_upstream(network, source_workload, addr, resolution_mode) // Drop the lock }; tracing::trace!(%addr, ?upstream, "fetch_upstream"); - // package into a finalize upstream and pick/resolve addresses self.finalize_upstream(source_workload, addr, upstream) .await } - pub async fn finalize_upstream( + async fn finalize_upstream( &self, source_workload: &Workload, original_target_address: SocketAddr, @@ -827,8 +823,7 @@ impl DemandProxyState { }; let svc_desc = svc.clone().map(|s| ServiceDescription::from(s.as_ref())); let ip_family_restriction = svc.as_ref().and_then(|s| s.ip_families); - - let (selected_workload_ip, _) = self + let selected_workload_ip = self .pick_workload_destination_or_resolve( &wl, source_workload, diff --git a/src/test_helpers/linux.rs b/src/test_helpers/linux.rs index bcbc409530..ec406ba225 100644 --- a/src/test_helpers/linux.rs +++ b/src/test_helpers/linux.rs @@ -421,7 +421,6 @@ impl<'a> TestWorkloadBuilder<'a> { self } - /// pub fn network (mut self, network: Strng) -> Self { self.w.workload.network = network; self From 7cd4cf194638cf359111c3fde2d27bf6fe851a9d Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Wed, 2 Apr 2025 15:47:58 -0400 Subject: [PATCH 25/29] get rid of more stuff --- src/proxy/h2/client.rs | 6 +++--- src/proxy/keylog | 0 src/proxy/outbound.rs | 3 +-- src/proxy/pool.rs | 3 +-- src/state.rs | 3 --- tests/namespaced.rs | 5 ----- 6 files changed, 5 insertions(+), 15 deletions(-) delete mode 100644 src/proxy/keylog diff --git a/src/proxy/h2/client.rs b/src/proxy/h2/client.rs index e0cc4af928..4c768d537e 100644 --- a/src/proxy/h2/client.rs +++ b/src/proxy/h2/client.rs @@ -148,7 +148,7 @@ pub async fn spawn_connection( s: impl AsyncRead + AsyncWrite + Unpin + Send + 'static, driver_drain: Receiver, wl_key: WorkloadKey, -) -> Result<(H2ConnectClient, tokio::task::JoinHandle<()>), Error> { +) -> Result { let mut builder = h2::client::Builder::new(); builder .initial_window_size(cfg.window_size) @@ -176,7 +176,7 @@ pub async fn spawn_connection( // spawn a task to poll the connection and drive the HTTP state // if we got a drain for that connection, respect it in a race // it is important to have a drain here, or this connection will never terminate - let driver_handle = tokio::spawn( + tokio::spawn( async move { drive_connection(connection, driver_drain).await; } @@ -189,7 +189,7 @@ pub async fn spawn_connection( max_allowed_streams, wl_key, }; - Ok((c, driver_handle)) + Ok(c) } async fn drive_connection(mut conn: Connection, mut driver_drain: Receiver) diff --git a/src/proxy/keylog b/src/proxy/keylog deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index 2fd0054ba6..21e83c9056 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -253,7 +253,7 @@ impl OutboundConnection { // Spawn inner CONNECT tunnel let (drain_tx, drain_rx) = tokio::sync::watch::channel(false); - let (mut sender, driver_task) = + let mut sender = super::h2::client::spawn_connection(self.pi.cfg.clone(), tls_stream, drain_rx, wl_key) .await?; let http_request = self.create_hbone_request(remote_addr, req); @@ -268,7 +268,6 @@ impl OutboundConnection { .await; let _ = drain_tx.send(true); - let _ = driver_task.await; res } diff --git a/src/proxy/pool.rs b/src/proxy/pool.rs index 62fef60f92..4b55b9dd6a 100644 --- a/src/proxy/pool.rs +++ b/src/proxy/pool.rs @@ -89,10 +89,9 @@ impl ConnSpawner { _ => e.into(), })?; - let tls_stream = connector.connect(tcp_stream).await?; trace!("connector connected, handshaking"); - let (sender, _) = h2::client::spawn_connection( + let sender = h2::client::spawn_connection( self.cfg.clone(), tls_stream, self.timeout_rx.clone(), diff --git a/src/state.rs b/src/state.rs index c39079b2ed..db2a52ad48 100644 --- a/src/state.rs +++ b/src/state.rs @@ -71,8 +71,6 @@ pub struct Upstream { pub service_sans: Vec, /// If this was from a service, the service info. pub destination_service: Option, - /// Use this port instead of the default one for the proxy protocol. - pub proxy_protocol_port_override: Option, } impl Upstream { @@ -837,7 +835,6 @@ impl DemandProxyState { port, service_sans: svc.map(|s| s.subject_alt_names.clone()).unwrap_or_default(), destination_service: svc_desc, - proxy_protocol_port_override: None, }; tracing::trace!(?res, "finalize_upstream"); Ok(Some(res)) diff --git a/tests/namespaced.rs b/tests/namespaced.rs index 1913e398ec..87f0d8ff38 100644 --- a/tests/namespaced.rs +++ b/tests/namespaced.rs @@ -1034,9 +1034,7 @@ mod namespaced { let connector = cert.outbound_connector(vec![dst_id]).unwrap(); let hbone = SocketAddr::new(srv.ip(), 15008); let tcp_stream = TcpStream::connect(hbone).await.unwrap(); - let tls_stream = connector.connect(tcp_stream).await.unwrap(); - let (mut request_sender, connection) = builder.handshake(TokioIo::new(tls_stream)).await.unwrap(); // spawn a task to poll the connection and drive the HTTP state @@ -1099,9 +1097,7 @@ mod namespaced { let tcp_stream = TcpStream::connect(SocketAddr::from((srv.ip(), 15008))) .await .unwrap(); - let tls_stream = connector.connect(tcp_stream).await.unwrap(); - let (mut request_sender, connection) = builder.handshake(TokioIo::new(tls_stream)).await.unwrap(); // spawn a task to poll the connection and drive the HTTP state @@ -1183,7 +1179,6 @@ mod namespaced { let tcp_stream = TcpStream::connect(SocketAddr::from((srv.ip(), 15008))) .await .unwrap(); - let tls_stream = connector.connect(tcp_stream).await.unwrap(); let (mut request_sender, connection) = builder.handshake(TokioIo::new(tls_stream)).await.unwrap(); From fa46e460b14921fc5d323667306a0d2e9927cd90 Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Wed, 2 Apr 2025 16:08:41 -0400 Subject: [PATCH 26/29] lint --- tests/namespaced.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/namespaced.rs b/tests/namespaced.rs index 87f0d8ff38..3df6185d2c 100644 --- a/tests/namespaced.rs +++ b/tests/namespaced.rs @@ -271,7 +271,7 @@ mod namespaced { run_hbone_server( actual_ew_gtw.clone(), "actual-ew-gtw", - tcp::Mode::Forward(echo_hbone_addr.clone()), + tcp::Mode::Forward(echo_hbone_addr), b"".into(), )?; From 0638119511825b807ff9e6f1c3de71392766730e Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Wed, 2 Apr 2025 16:40:20 -0400 Subject: [PATCH 27/29] lint --- src/proxy/connection_manager.rs | 2 +- src/proxy/h2.rs | 8 ++++---- src/proxy/inbound.rs | 2 +- src/state.rs | 11 +++-------- src/state/workload.rs | 1 - src/test_helpers/linux.rs | 2 +- src/tls/workload.rs | 5 +---- 7 files changed, 11 insertions(+), 20 deletions(-) diff --git a/src/proxy/connection_manager.rs b/src/proxy/connection_manager.rs index a821ffc84b..a02cfa0322 100644 --- a/src/proxy/connection_manager.rs +++ b/src/proxy/connection_manager.rs @@ -24,7 +24,7 @@ use std::net::SocketAddr; use crate::drain; use crate::drain::{DrainTrigger, DrainWatcher}; -use crate::state::workload::{OutboundProtocol, InboundProtocol}; +use crate::state::workload::{InboundProtocol, OutboundProtocol}; use std::sync::Arc; use std::sync::RwLock; use tracing::{debug, error, info, warn}; diff --git a/src/proxy/h2.rs b/src/proxy/h2.rs index c1d1ab1efc..b86ac4df59 100644 --- a/src/proxy/h2.rs +++ b/src/proxy/h2.rs @@ -160,7 +160,10 @@ impl tokio::io::AsyncRead for TokioH2Stream { if buf.remaining() < bytes.len() { Err(Error::new( std::io::ErrorKind::Other, - format!("kould overflow buffer of with {} remaining", buf.remaining()), + format!( + "kould overflow buffer of with {} remaining", + buf.remaining() + ), )) } else { buf.put(bytes); @@ -302,6 +305,3 @@ fn h2_to_io_error(e: h2::Error) -> std::io::Error { std::io::Error::new(std::io::ErrorKind::Other, e) } } - - - diff --git a/src/proxy/inbound.rs b/src/proxy/inbound.rs index 5f5b2e38bb..0df26ffc67 100644 --- a/src/proxy/inbound.rs +++ b/src/proxy/inbound.rs @@ -665,7 +665,7 @@ mod tests { self, DemandProxyState, service::{Endpoint, EndpointSet, Service}, workload::{ - ApplicationTunnel, GatewayAddress, NetworkAddress, InboundProtocol, Workload, + ApplicationTunnel, GatewayAddress, InboundProtocol, NetworkAddress, Workload, application_tunnel::Protocol as AppProtocol, gatewayaddress::Destination, }, }, diff --git a/src/state.rs b/src/state.rs index db2a52ad48..7f730fb309 100644 --- a/src/state.rs +++ b/src/state.rs @@ -74,7 +74,6 @@ pub struct Upstream { } impl Upstream { - pub fn workload_socket_addr(&self) -> Option { self.selected_workload_ip .map(|ip| SocketAddr::new(ip, self.port)) @@ -778,12 +777,8 @@ impl DemandProxyState { resolution_mode: ServiceResolutionMode, ) -> Result, Error> { let upstream = { - self.read().find_upstream_from_host( - source_workload, - host, - port, - resolution_mode, - ) + self.read() + .find_upstream_from_host(source_workload, host, port, resolution_mode) // Drop the lock }; // tracing::trace!(%addr, ?upstream, "fetch_upstream"); @@ -821,7 +816,7 @@ impl DemandProxyState { }; let svc_desc = svc.clone().map(|s| ServiceDescription::from(s.as_ref())); let ip_family_restriction = svc.as_ref().and_then(|s| s.ip_families); - let selected_workload_ip = self + let selected_workload_ip = self .pick_workload_destination_or_resolve( &wl, source_workload, diff --git a/src/state/workload.rs b/src/state/workload.rs index f393b2fe85..f850357f2c 100644 --- a/src/state/workload.rs +++ b/src/state/workload.rs @@ -101,7 +101,6 @@ impl From for OutboundProtocol { } } - #[derive( Default, Debug, Hash, Eq, PartialEq, Clone, Copy, serde::Serialize, serde::Deserialize, )] diff --git a/src/test_helpers/linux.rs b/src/test_helpers/linux.rs index ec406ba225..ac0c4fd2c3 100644 --- a/src/test_helpers/linux.rs +++ b/src/test_helpers/linux.rs @@ -421,7 +421,7 @@ impl<'a> TestWorkloadBuilder<'a> { self } - pub fn network (mut self, network: Strng) -> Self { + pub fn network(mut self, network: Strng) -> Self { self.w.workload.network = network; self } diff --git a/src/tls/workload.rs b/src/tls/workload.rs index 7bb19beb43..4aa1873518 100644 --- a/src/tls/workload.rs +++ b/src/tls/workload.rs @@ -171,10 +171,7 @@ pub struct OutboundConnector { } impl OutboundConnector { - pub async fn connect( - self, - stream: IO, - ) -> Result, io::Error> + pub async fn connect(self, stream: IO) -> Result, io::Error> where IO: AsyncRead + AsyncWrite + Unpin, { From 04e7d303f75b9cb00c2207fee5be5a31383e8808 Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Mon, 7 Apr 2025 12:38:09 -0400 Subject: [PATCH 28/29] Review: * Code layout * Handle mismatched networks with no network gateway * Get rid of a panic --- src/proxy/outbound.rs | 82 ++++++++++++++++++++++++++---------------- src/state.rs | 84 +++++++++++++++++++++++++------------------ src/tls/workload.rs | 16 ++++----- 3 files changed, 108 insertions(+), 74 deletions(-) diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index 21e83c9056..6eb9997027 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -38,7 +38,6 @@ use crate::proxy::h2::{H2Stream, client::WorkloadKey}; use crate::state::ServiceResolutionMode; use crate::state::service::ServiceDescription; use crate::state::workload::OutboundProtocol; -use crate::state::workload::gatewayaddress::Destination; use crate::state::workload::{InboundProtocol, NetworkAddress, Workload, address::Address}; use crate::{assertions, copy, proxy, socket}; @@ -446,40 +445,19 @@ impl OutboundConnection { }; // Check whether we are using an E/W gateway and sending cross network traffic - if let Some(ew_gtw) = &us.workload.network_gateway { - if us.workload.network != source_workload.network { + if us.workload.network != source_workload.network { + if let Some(ew_gtw) = &us.workload.network_gateway { + let (actual_destination, upstream_sans, final_sans) = { + self.pi + .state + .fetch_network_gateway(ew_gtw, &source_workload, target, &us) + .await? + }; + let svc = us .destination_service .as_ref() .expect("Workloads with network gateways must be service addressed."); - - let (actual_destination, upstream_sans, final_sans) = match &ew_gtw.destination { - Destination::Address(address) => ( - SocketAddr::from((address.address, ew_gtw.hbone_mtls_port)), - us.workload_and_services_san(), - us.service_sans(), - ), - Destination::Hostname(host) => { - let us_gtw = self - .pi - .state - .fetch_upstream_by_host( - &source_workload, - host, - ew_gtw.hbone_mtls_port, - target, - ServiceResolutionMode::Standard, - ) - .await? - .unwrap(); - ( - SocketAddr::from((us_gtw.selected_workload_ip.unwrap(), us_gtw.port)), - us_gtw.workload_and_services_san(), - us.service_sans(), - ) - } - }; - let hbone_target_destination = Some(HboneAddress::SvcHostname(svc.hostname.clone(), us.port)); @@ -493,6 +471,9 @@ impl OutboundConnection { upstream_sans, final_sans, }); + } else { + // Do not try to send cross-network traffic without network gateway. + return Err(Error::NoValidDestination(Box::new((*us.workload).clone()))); } } @@ -791,6 +772,45 @@ mod tests { .await; } + #[tokio::test] + async fn build_request_wrong_network() { + run_build_request_multi( + "127.0.0.1", + "127.0.0.3:80", + vec![ + XdsAddressType::Service(XdsService { + hostname: "example.com".to_string(), + addresses: vec![XdsNetworkAddress { + network: "".to_string(), + address: vec![127, 0, 0, 3], + }], + ports: vec![Port { + service_port: 80, + target_port: 8080, + }], + ..Default::default() + }), + XdsAddressType::Workload(XdsWorkload { + uid: "cluster1//v1/Pod/default/remote-pod".to_string(), + addresses: vec![Bytes::copy_from_slice(&[10, 0, 0, 2])], + network: "remote".to_string(), + services: std::collections::HashMap::from([( + "/example.com".to_string(), + PortList { + ports: vec![Port { + service_port: 80, + target_port: 8080, + }], + }, + )]), + ..Default::default() + }), + ], + None, + ) + .await; + } + #[tokio::test] async fn build_request_double_hbone() { run_build_request_multi( diff --git a/src/state.rs b/src/state.rs index 7f730fb309..b3a2b05f62 100644 --- a/src/state.rs +++ b/src/state.rs @@ -282,22 +282,6 @@ impl ProxyState { .ok_or_else(|| Error::NoHostname(hostname.to_string())) } - fn find_upstream_from_host( - &self, - source_workload: &Workload, - host: &NamespacedHostname, - port: u16, - resolution_mode: ServiceResolutionMode, - ) -> Option<(Arc, u16, Option>)> { - let address = self.find_hostname(host)?; - match address { - Address::Service(svc) => { - self.find_upstream_from_service(source_workload, port, resolution_mode, svc) - } - Address::Workload(wl) => Some((wl, port, None)), - } - } - fn find_upstream( &self, network: Strng, @@ -768,24 +752,6 @@ impl DemandProxyState { self.read().workloads.find_uid(uid) } - pub async fn fetch_upstream_by_host( - &self, - source_workload: &Workload, - host: &NamespacedHostname, - port: u16, - original_target_address: SocketAddr, - resolution_mode: ServiceResolutionMode, - ) -> Result, Error> { - let upstream = { - self.read() - .find_upstream_from_host(source_workload, host, port, resolution_mode) - // Drop the lock - }; - // tracing::trace!(%addr, ?upstream, "fetch_upstream"); - self.finalize_upstream(source_workload, original_target_address, upstream) - .await - } - pub async fn fetch_upstream( &self, network: Strng, @@ -835,6 +801,56 @@ impl DemandProxyState { Ok(Some(res)) } + /// Returns destination address, upstream sans, and final sans, for + /// connecting to a remote workload through a gateway. + /// Would be nice to return this as an Upstream, but gateways don't necessarily + /// have workloads. That is, they could just be IPs without a corresponding workload. + pub async fn fetch_network_gateway( + &self, + gtw: &GatewayAddress, + source_workload: &Workload, + original_destination_address: SocketAddr, + original_us: &Upstream, + ) -> Result<(SocketAddr, Vec, Vec), Error> { + match >w.destination { + Destination::Address(address) => Ok(( + SocketAddr::from((address.address, gtw.hbone_mtls_port)), + original_us.workload_and_services_san(), + original_us.service_sans(), + )), + Destination::Hostname(host) => { + let us_gtw = match self.read().find_hostname(host) { + Some(Address::Service(svc)) => self.read().find_upstream_from_service( + source_workload, + gtw.hbone_mtls_port, + ServiceResolutionMode::Standard, + svc, + ), + Some(Address::Workload(wl)) => Some((wl, gtw.hbone_mtls_port, None)), + None => return Err(Error::NoHostname(host.to_string())), + }; + let us_gtw = self + .finalize_upstream(source_workload, original_destination_address, us_gtw) + .await? + .ok_or(Error::NoValidDestination(Box::new( + (*original_us.workload).clone(), + )))?; + Ok(( + SocketAddr::from(( + us_gtw + .selected_workload_ip + .ok_or(Error::NoValidDestination(Box::new( + (*us_gtw.workload).clone(), + )))?, + us_gtw.port, + )), + us_gtw.workload_and_services_san(), + original_us.service_sans(), + )) + } + } + } + async fn fetch_waypoint( &self, gw_address: &GatewayAddress, diff --git a/src/tls/workload.rs b/src/tls/workload.rs index 4aa1873518..92e05d40c8 100644 --- a/src/tls/workload.rs +++ b/src/tls/workload.rs @@ -21,7 +21,7 @@ use crate::tls::{ServerCertProvider, TlsError}; use futures_util::TryFutureExt; use rustls::client::danger::{HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier}; -use rustls::pki_types::{CertificateDer, DnsName, ServerName, UnixTime}; +use rustls::pki_types::{CertificateDer, ServerName, UnixTime}; use rustls::server::ParsedCertificate; use rustls::server::danger::{ClientCertVerified, ClientCertVerifier}; use rustls::{ @@ -35,17 +35,10 @@ use tokio::io::{AsyncRead, AsyncWrite}; use crate::strng::Strng; use crate::tls; -use once_cell::sync::Lazy; use tokio::net::TcpStream; use tokio_rustls::client; use tracing::{debug, trace}; -// Dummy domain used for TLS SNI because the actual domain isn't relevant -// since we use SANs for identity verification. -// dummy.istio.io is a valid domain, so the unwrap should never fail. -static DUMMY_DOMAIN: Lazy> = - Lazy::new(|| ServerName::DnsName(DnsName::try_from_str("dummy.istio.io").unwrap())); - #[derive(Clone, Debug)] pub struct InboundAcceptor { provider: F, @@ -176,7 +169,12 @@ impl OutboundConnector { IO: AsyncRead + AsyncWrite + Unpin, { let c = tokio_rustls::TlsConnector::from(self.client_config); - c.connect(DUMMY_DOMAIN.clone(), stream).await + // Use dummy value for domain because it doesn't matter. + c.connect( + ServerName::IpAddress(std::net::Ipv4Addr::new(0, 0, 0, 0).into()), + stream, + ) + .await } } From 0fbae22d871338dfc3220cc9869776347149039c Mon Sep 17 00:00:00 2001 From: Steven Jin Xuan Date: Mon, 7 Apr 2025 17:06:13 -0400 Subject: [PATCH 29/29] Assume existing workload --- src/proxy.rs | 3 ++ src/proxy/outbound.rs | 26 ++++++++----- src/state.rs | 85 ++++++++++++++++++++++++------------------- tests/namespaced.rs | 2 +- 4 files changed, 68 insertions(+), 48 deletions(-) diff --git a/src/proxy.rs b/src/proxy.rs index b6c6346e4e..5ccafb8515 100644 --- a/src/proxy.rs +++ b/src/proxy.rs @@ -446,6 +446,9 @@ pub enum Error { #[error("unknown waypoint: {0}")] UnknownWaypoint(String), + #[error("unknown network gateway: {0}")] + UnknownNetworkGateway(String), + #[error("no service or workload for hostname: {0}")] NoHostname(String), diff --git a/src/proxy/outbound.rs b/src/proxy/outbound.rs index 6eb9997027..91b1015ed5 100644 --- a/src/proxy/outbound.rs +++ b/src/proxy/outbound.rs @@ -447,10 +447,10 @@ impl OutboundConnection { // Check whether we are using an E/W gateway and sending cross network traffic if us.workload.network != source_workload.network { if let Some(ew_gtw) = &us.workload.network_gateway { - let (actual_destination, upstream_sans, final_sans) = { + let gtw_us = { self.pi .state - .fetch_network_gateway(ew_gtw, &source_workload, target, &us) + .fetch_network_gateway(ew_gtw, &source_workload, target) .await? }; @@ -465,11 +465,13 @@ impl OutboundConnection { protocol: OutboundProtocol::DOUBLEHBONE, source: source_workload, hbone_target_destination, - actual_destination_workload: Some(us.workload.clone()), + actual_destination_workload: Some(gtw_us.workload.clone()), intended_destination_service: us.destination_service.clone(), - actual_destination, - upstream_sans, - final_sans, + actual_destination: gtw_us.workload_socket_addr().ok_or( + Error::NoValidDestination(Box::new((*gtw_us.workload).clone())), + )?, + upstream_sans: gtw_us.workload_and_services_san(), + final_sans: us.service_sans(), }); } else { // Do not try to send cross-network traffic without network gateway. @@ -837,8 +839,8 @@ mod tests { destination: Some( xds::istio::workload::gateway_address::Destination::Address( XdsNetworkAddress { - network: "".to_string(), - address: vec![1, 2, 3, 4], + network: "remote".to_string(), + address: vec![10, 22, 1, 1], }, ), ), @@ -855,11 +857,17 @@ mod tests { )]), ..Default::default() }), + XdsAddressType::Workload(XdsWorkload { + uid: "cluster1//v1/Pod/default/ew-gtw".to_string(), + addresses: vec![Bytes::copy_from_slice(&[10, 22, 1, 1])], + network: "remote".to_string(), + ..Default::default() + }), ], Some(ExpectedRequest { protocol: OutboundProtocol::DOUBLEHBONE, hbone_destination: "example.com:8080", - destination: "1.2.3.4:15009", + destination: "10.22.1.1:15009", }), ) .await; diff --git a/src/state.rs b/src/state.rs index b3a2b05f62..756c13c08a 100644 --- a/src/state.rs +++ b/src/state.rs @@ -807,48 +807,57 @@ impl DemandProxyState { /// have workloads. That is, they could just be IPs without a corresponding workload. pub async fn fetch_network_gateway( &self, - gtw: &GatewayAddress, + gw_address: &GatewayAddress, source_workload: &Workload, original_destination_address: SocketAddr, - original_us: &Upstream, - ) -> Result<(SocketAddr, Vec, Vec), Error> { - match >w.destination { - Destination::Address(address) => Ok(( - SocketAddr::from((address.address, gtw.hbone_mtls_port)), - original_us.workload_and_services_san(), - original_us.service_sans(), - )), + ) -> Result { + let (res, target_address) = match &gw_address.destination { + Destination::Address(ip) => { + let addr = SocketAddr::new(ip.address, gw_address.hbone_mtls_port); + let us = self.state.read().unwrap().find_upstream( + ip.network.clone(), + source_workload, + addr, + ServiceResolutionMode::Standard, + ); + // If the workload references a network gateway by IP, use that IP as the destination. + // Note this means that an IPv6 call may be translated to IPv4 if the network + // gateway is specified as an IPv4 address. + // For this reason, the Hostname method is preferred which can adapt to the callers IP family. + (us, addr) + } Destination::Hostname(host) => { - let us_gtw = match self.read().find_hostname(host) { - Some(Address::Service(svc)) => self.read().find_upstream_from_service( - source_workload, - gtw.hbone_mtls_port, - ServiceResolutionMode::Standard, - svc, - ), - Some(Address::Workload(wl)) => Some((wl, gtw.hbone_mtls_port, None)), - None => return Err(Error::NoHostname(host.to_string())), - }; - let us_gtw = self - .finalize_upstream(source_workload, original_destination_address, us_gtw) - .await? - .ok_or(Error::NoValidDestination(Box::new( - (*original_us.workload).clone(), - )))?; - Ok(( - SocketAddr::from(( - us_gtw - .selected_workload_ip - .ok_or(Error::NoValidDestination(Box::new( - (*us_gtw.workload).clone(), - )))?, - us_gtw.port, - )), - us_gtw.workload_and_services_san(), - original_us.service_sans(), - )) + let state = self.read(); + match state.find_hostname(host) { + Some(Address::Service(s)) => { + let us = state.find_upstream_from_service( + source_workload, + gw_address.hbone_mtls_port, + ServiceResolutionMode::Standard, + s, + ); + // For hostname, use the original_destination_address as the target so we can + // adapt to the callers IP family. + (us, original_destination_address) + } + Some(Address::Workload(w)) => { + let us = Some((w, gw_address.hbone_mtls_port, None)); + (us, original_destination_address) + } + None => { + return Err(Error::UnknownNetworkGateway(format!( + "network gateway {} not found", + host.hostname + ))); + } + } } - } + }; + self.finalize_upstream(source_workload, target_address, res) + .await? + .ok_or_else(|| { + Error::UnknownNetworkGateway(format!("network gateway {:?} not found", gw_address)) + }) } async fn fetch_waypoint( diff --git a/tests/namespaced.rs b/tests/namespaced.rs index 3df6185d2c..7d7d0d4fe0 100644 --- a/tests/namespaced.rs +++ b/tests/namespaced.rs @@ -295,7 +295,7 @@ mod namespaced { let want = HashMap::from([ ("scope", "access"), ("src.workload", "client"), - ("dst.workload", "local-remote-workload"), + ("dst.workload", "actual-ew-gtw"), ("dst.hbone_addr", "remote.default.svc.cluster.local:8080"), ("dst.addr", &dst_addr), ("bytes_sent", &sent),