diff --git a/clients/rust/crates/mxgw-cli/src/main.rs b/clients/rust/crates/mxgw-cli/src/main.rs index 343081e..2f98d6f 100644 --- a/clients/rust/crates/mxgw-cli/src/main.rs +++ b/clients/rust/crates/mxgw-cli/src/main.rs @@ -426,6 +426,11 @@ struct ConnectionArgs { ca_file: Option, #[arg(long)] server_name_override: Option, + /// Verify the server certificate against the system trust roots even + /// without a pinned CA. The Rust client's default is to require a CA + /// file (see `--ca-file`); set this flag to use system roots instead. + #[arg(long)] + require_certificate_validation: bool, #[arg(long, default_value_t = 10)] connect_timeout_seconds: u64, #[arg(long, default_value_t = 30)] @@ -453,6 +458,9 @@ impl ConnectionArgs { if let Some(server_name_override) = &self.server_name_override { options = options.with_server_name_override(server_name_override); } + if self.require_certificate_validation { + options = options.with_require_certificate_validation(true); + } options } diff --git a/clients/rust/src/client.rs b/clients/rust/src/client.rs index cd4e358..bdac543 100644 --- a/clients/rust/src/client.rs +++ b/clients/rust/src/client.rs @@ -6,10 +6,8 @@ //! code should prefer [`GatewayClient::open_session`] and the [`Session`] //! handle it returns, rather than the `*_raw` methods. -use std::fs; - use tonic::codegen::InterceptedService; -use tonic::transport::{Certificate, Channel, ClientTlsConfig}; +use tonic::transport::Channel; use tonic::Request; use crate::auth::AuthInterceptor; @@ -21,7 +19,7 @@ use crate::generated::mxaccess_gateway::v1::{ OpenSessionReply, OpenSessionRequest, QueryActiveAlarmsRequest, StreamAlarmsRequest, StreamEventsRequest, }; -use crate::options::ClientOptions; +use crate::options::{build_tls_config, ClientOptions}; use crate::session::Session; /// Generated gateway client wrapped in the auth interceptor that @@ -78,38 +76,7 @@ impl GatewayClient { })?; endpoint = endpoint.connect_timeout(options.connect_timeout()); - if !options.plaintext() { - let mut tls = ClientTlsConfig::new(); - if let Some(server_name) = options.server_name_override() { - tls = tls.domain_name(server_name.to_owned()); - } - if let Some(ca_file) = options.ca_file() { - let certificate = fs::read(ca_file).map_err(|source| Error::InvalidEndpoint { - endpoint: options.endpoint().to_owned(), - detail: format!("failed to read CA file {}: {source}", ca_file.display()), - })?; - tls = tls.ca_certificate(Certificate::from_pem(certificate)); - } else if !options.require_certificate_validation() { - // Lenient-default fallback (Rust pin-only exception): tonic - // 0.13's `ClientTlsConfig` builds its rustls verifier inside a - // crate-private connector and exposes no hook for a custom - // `ServerCertVerifier`, so — unlike the other clients — the - // Rust client cannot accept an arbitrary self-signed cert. Pin - // the gateway's CA instead, or opt into strict verification - // against the system trust roots. We reject here rather than - // silently verifying against system roots (which would fail a - // self-signed gateway with a confusing handshake error). - return Err(Error::InvalidEndpoint { - endpoint: options.endpoint().to_owned(), - detail: "TLS requested without a pinned CA. The Rust client cannot accept an \ - arbitrary self-signed certificate (tonic 0.13 exposes no custom \ - rustls verifier). Pin the gateway certificate with \ - ClientOptions::with_ca_file, or call \ - ClientOptions::with_require_certificate_validation(true) to verify \ - against the system trust roots." - .to_owned(), - }); - } + if let Some(tls) = build_tls_config(&options)? { endpoint = endpoint.tls_config(tls)?; } diff --git a/clients/rust/src/galaxy.rs b/clients/rust/src/galaxy.rs index 8aee0dd..8ae97d7 100644 --- a/clients/rust/src/galaxy.rs +++ b/clients/rust/src/galaxy.rs @@ -6,13 +6,12 @@ //! re-exported through [`crate::generated::galaxy_repository::v1`]. use std::collections::HashSet; -use std::fs; use std::sync::Arc; use prost_types::Timestamp; use tokio::sync::Mutex as AsyncMutex; use tonic::codegen::InterceptedService; -use tonic::transport::{Certificate, Channel, ClientTlsConfig}; +use tonic::transport::Channel; use tonic::Request; use crate::auth::AuthInterceptor; @@ -23,7 +22,7 @@ use crate::generated::galaxy_repository::v1::{ DiscoverHierarchyRequest, GalaxyObject, GetLastDeployTimeRequest, TestConnectionRequest, WatchDeployEventsRequest, }; -use crate::options::ClientOptions; +use crate::options::{build_tls_config, ClientOptions}; const DISCOVER_HIERARCHY_PAGE_SIZE: i32 = 5000; const BROWSE_CHILDREN_PAGE_SIZE: i32 = 500; @@ -183,18 +182,7 @@ impl GalaxyClient { })?; endpoint = endpoint.connect_timeout(options.connect_timeout()); - if !options.plaintext() { - let mut tls = ClientTlsConfig::new(); - if let Some(server_name) = options.server_name_override() { - tls = tls.domain_name(server_name.to_owned()); - } - if let Some(ca_file) = options.ca_file() { - let certificate = fs::read(ca_file).map_err(|source| Error::InvalidEndpoint { - endpoint: options.endpoint().to_owned(), - detail: format!("failed to read CA file {}: {source}", ca_file.display()), - })?; - tls = tls.ca_certificate(Certificate::from_pem(certificate)); - } + if let Some(tls) = build_tls_config(&options)? { endpoint = endpoint.tls_config(tls)?; } diff --git a/clients/rust/src/options.rs b/clients/rust/src/options.rs index 6d4355b..4031809 100644 --- a/clients/rust/src/options.rs +++ b/clients/rust/src/options.rs @@ -3,10 +3,14 @@ //! chain of `with_*` setters; the `Debug` impl redacts the API key. use std::fmt; +use std::fs; use std::path::PathBuf; use std::time::Duration; +use tonic::transport::{Certificate, ClientTlsConfig}; + use crate::auth::ApiKey; +use crate::error::Error; const DEFAULT_MAX_GRPC_MESSAGE_BYTES: usize = 16 * 1024 * 1024; @@ -171,6 +175,68 @@ impl ClientOptions { } } +/// Build the [`ClientTlsConfig`] for a non-plaintext connection described by +/// `options`, applying the lenient-default guard that is the **Rust +/// pin-only exception**. +/// +/// Returns `Ok(None)` when `options.plaintext()` is `true` (no TLS needed). +/// Returns `Ok(Some(tls))` when a valid TLS config can be assembled. +/// Returns `Err(Error::InvalidEndpoint)` when TLS is requested but no pinned +/// CA was provided and `require_certificate_validation` is `false`. +/// +/// # Why this guard exists +/// +/// `tonic` 0.13's `ClientTlsConfig` builds its rustls verifier inside a +/// crate-private connector and exposes no hook for a custom +/// `ServerCertVerifier`. The Rust client therefore cannot accept an arbitrary +/// self-signed certificate the way the other language clients do. Rather than +/// silently falling back to system-root verification (which always fails +/// against a self-signed gateway certificate), we reject the configuration +/// early with an actionable error. +pub(crate) fn build_tls_config(options: &ClientOptions) -> Result, Error> { + if options.plaintext() { + return Ok(None); + } + + let mut tls = ClientTlsConfig::new(); + if let Some(server_name) = options.server_name_override() { + tls = tls.domain_name(server_name.to_owned()); + } + if let Some(ca_file) = options.ca_file() { + let certificate = fs::read(ca_file).map_err(|source| Error::InvalidEndpoint { + endpoint: options.endpoint().to_owned(), + detail: format!("failed to read CA file {}: {source}", ca_file.display()), + })?; + tls = tls.ca_certificate(Certificate::from_pem(certificate)); + } else if !options.require_certificate_validation() { + // Lenient-default fallback (Rust pin-only exception): tonic + // 0.13's `ClientTlsConfig` builds its rustls verifier inside a + // crate-private connector and exposes no hook for a custom + // `ServerCertVerifier`, so — unlike the other clients — the + // Rust client cannot accept an arbitrary self-signed cert. Pin + // the gateway's CA instead, or opt into strict verification + // against the system trust roots. We reject here rather than + // silently verifying against system roots (which would fail a + // self-signed gateway with a confusing handshake error). + // + // Note: a server-name override affects SNI (the hostname sent + // in the TLS ClientHello) but does NOT pin trust. Overriding + // the server name alone does not bypass certificate validation. + return Err(Error::InvalidEndpoint { + endpoint: options.endpoint().to_owned(), + detail: "TLS requested without a pinned CA. The Rust client cannot accept an \ + arbitrary self-signed certificate (tonic 0.13 exposes no custom \ + rustls verifier). Pin the gateway certificate with \ + ClientOptions::with_ca_file, or call \ + ClientOptions::with_require_certificate_validation(true) to verify \ + against the system trust roots. Note: a server-name override \ + affects SNI but does not pin trust." + .to_owned(), + }); + } + Ok(Some(tls)) +} + impl Default for ClientOptions { fn default() -> Self { Self::new("http://127.0.0.1:5000") diff --git a/clients/rust/tests/tls.rs b/clients/rust/tests/tls.rs index 0769f14..66b5d9b 100644 --- a/clients/rust/tests/tls.rs +++ b/clients/rust/tests/tls.rs @@ -10,7 +10,7 @@ use std::time::Duration; -use zb_mom_ww_mxgateway_client::{ClientOptions, Error, GatewayClient}; +use zb_mom_ww_mxgateway_client::{ClientOptions, Error, GalaxyClient, GatewayClient}; /// Drive `connect` to its error without requiring `GatewayClient: Debug` /// (the success arm is dropped explicitly so `unwrap_err` is unnecessary). @@ -87,6 +87,40 @@ async fn tls_with_ca_file_is_permitted_and_proceeds_past_the_guard() { ); } +/// Drive `GalaxyClient::connect` to its error (mirrors `connect_err` above). +async fn galaxy_connect_err(options: ClientOptions) -> Error { + match GalaxyClient::connect(options).await { + Ok(_client) => { + panic!("GalaxyClient::connect unexpectedly succeeded against a dead TLS address") + } + Err(error) => error, + } +} + +#[tokio::test] +async fn galaxy_tls_without_ca_is_rejected_with_actionable_error_by_default() { + // GalaxyClient::connect must apply the same TLS guard as GatewayClient — + // TLS without a pinned CA (and without require_certificate_validation) + // returns a clear, actionable InvalidEndpoint error. + let options = ClientOptions::new("https://127.0.0.1:1") + .with_plaintext(false) + .with_connect_timeout(Duration::from_millis(200)); + + let error = galaxy_connect_err(options).await; + + let Error::InvalidEndpoint { detail, .. } = error else { + panic!("expected InvalidEndpoint, got {error:?}"); + }; + assert!( + detail.contains("ca_file") || detail.contains("CA"), + "error should instruct the user to pass a CA file: {detail}" + ); + assert!( + detail.contains("require_certificate_validation"), + "error should mention the require_certificate_validation opt-in: {detail}" + ); +} + /// A throwaway self-signed CA certificate (PEM). Only needs to parse as a /// PEM trust root so the CA-pinning path is exercised past the guard. const SELF_SIGNED_CA_PEM: &str = "-----BEGIN CERTIFICATE-----