fix(client-rust): apply TLS guard to GalaxyClient and add CLI strict flag
Extract the TLS-without-CA guard into a shared `build_tls_config` helper in options.rs so both GatewayClient and GalaxyClient use identical logic. GalaxyClient previously had no guard, so TLS-without-CA produced a cryptic tonic handshake failure; it now returns the same actionable InvalidEndpoint error. The guard message notes that a server-name override affects SNI but does not pin trust. Add --require-certificate-validation to ConnectionArgs in the CLI binary. Add a mirror test for GalaxyClient in tests/tls.rs.
This commit is contained in:
@@ -426,6 +426,11 @@ struct ConnectionArgs {
|
|||||||
ca_file: Option<PathBuf>,
|
ca_file: Option<PathBuf>,
|
||||||
#[arg(long)]
|
#[arg(long)]
|
||||||
server_name_override: Option<String>,
|
server_name_override: Option<String>,
|
||||||
|
/// 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)]
|
#[arg(long, default_value_t = 10)]
|
||||||
connect_timeout_seconds: u64,
|
connect_timeout_seconds: u64,
|
||||||
#[arg(long, default_value_t = 30)]
|
#[arg(long, default_value_t = 30)]
|
||||||
@@ -453,6 +458,9 @@ impl ConnectionArgs {
|
|||||||
if let Some(server_name_override) = &self.server_name_override {
|
if let Some(server_name_override) = &self.server_name_override {
|
||||||
options = options.with_server_name_override(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
|
options
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -6,10 +6,8 @@
|
|||||||
//! code should prefer [`GatewayClient::open_session`] and the [`Session`]
|
//! code should prefer [`GatewayClient::open_session`] and the [`Session`]
|
||||||
//! handle it returns, rather than the `*_raw` methods.
|
//! handle it returns, rather than the `*_raw` methods.
|
||||||
|
|
||||||
use std::fs;
|
|
||||||
|
|
||||||
use tonic::codegen::InterceptedService;
|
use tonic::codegen::InterceptedService;
|
||||||
use tonic::transport::{Certificate, Channel, ClientTlsConfig};
|
use tonic::transport::Channel;
|
||||||
use tonic::Request;
|
use tonic::Request;
|
||||||
|
|
||||||
use crate::auth::AuthInterceptor;
|
use crate::auth::AuthInterceptor;
|
||||||
@@ -21,7 +19,7 @@ use crate::generated::mxaccess_gateway::v1::{
|
|||||||
OpenSessionReply, OpenSessionRequest, QueryActiveAlarmsRequest, StreamAlarmsRequest,
|
OpenSessionReply, OpenSessionRequest, QueryActiveAlarmsRequest, StreamAlarmsRequest,
|
||||||
StreamEventsRequest,
|
StreamEventsRequest,
|
||||||
};
|
};
|
||||||
use crate::options::ClientOptions;
|
use crate::options::{build_tls_config, ClientOptions};
|
||||||
use crate::session::Session;
|
use crate::session::Session;
|
||||||
|
|
||||||
/// Generated gateway client wrapped in the auth interceptor that
|
/// Generated gateway client wrapped in the auth interceptor that
|
||||||
@@ -78,38 +76,7 @@ impl GatewayClient {
|
|||||||
})?;
|
})?;
|
||||||
endpoint = endpoint.connect_timeout(options.connect_timeout());
|
endpoint = endpoint.connect_timeout(options.connect_timeout());
|
||||||
|
|
||||||
if !options.plaintext() {
|
if let Some(tls) = build_tls_config(&options)? {
|
||||||
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(),
|
|
||||||
});
|
|
||||||
}
|
|
||||||
endpoint = endpoint.tls_config(tls)?;
|
endpoint = endpoint.tls_config(tls)?;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -6,13 +6,12 @@
|
|||||||
//! re-exported through [`crate::generated::galaxy_repository::v1`].
|
//! re-exported through [`crate::generated::galaxy_repository::v1`].
|
||||||
|
|
||||||
use std::collections::HashSet;
|
use std::collections::HashSet;
|
||||||
use std::fs;
|
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
|
|
||||||
use prost_types::Timestamp;
|
use prost_types::Timestamp;
|
||||||
use tokio::sync::Mutex as AsyncMutex;
|
use tokio::sync::Mutex as AsyncMutex;
|
||||||
use tonic::codegen::InterceptedService;
|
use tonic::codegen::InterceptedService;
|
||||||
use tonic::transport::{Certificate, Channel, ClientTlsConfig};
|
use tonic::transport::Channel;
|
||||||
use tonic::Request;
|
use tonic::Request;
|
||||||
|
|
||||||
use crate::auth::AuthInterceptor;
|
use crate::auth::AuthInterceptor;
|
||||||
@@ -23,7 +22,7 @@ use crate::generated::galaxy_repository::v1::{
|
|||||||
DiscoverHierarchyRequest, GalaxyObject, GetLastDeployTimeRequest, TestConnectionRequest,
|
DiscoverHierarchyRequest, GalaxyObject, GetLastDeployTimeRequest, TestConnectionRequest,
|
||||||
WatchDeployEventsRequest,
|
WatchDeployEventsRequest,
|
||||||
};
|
};
|
||||||
use crate::options::ClientOptions;
|
use crate::options::{build_tls_config, ClientOptions};
|
||||||
|
|
||||||
const DISCOVER_HIERARCHY_PAGE_SIZE: i32 = 5000;
|
const DISCOVER_HIERARCHY_PAGE_SIZE: i32 = 5000;
|
||||||
const BROWSE_CHILDREN_PAGE_SIZE: i32 = 500;
|
const BROWSE_CHILDREN_PAGE_SIZE: i32 = 500;
|
||||||
@@ -183,18 +182,7 @@ impl GalaxyClient {
|
|||||||
})?;
|
})?;
|
||||||
endpoint = endpoint.connect_timeout(options.connect_timeout());
|
endpoint = endpoint.connect_timeout(options.connect_timeout());
|
||||||
|
|
||||||
if !options.plaintext() {
|
if let Some(tls) = build_tls_config(&options)? {
|
||||||
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));
|
|
||||||
}
|
|
||||||
endpoint = endpoint.tls_config(tls)?;
|
endpoint = endpoint.tls_config(tls)?;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -3,10 +3,14 @@
|
|||||||
//! chain of `with_*` setters; the `Debug` impl redacts the API key.
|
//! chain of `with_*` setters; the `Debug` impl redacts the API key.
|
||||||
|
|
||||||
use std::fmt;
|
use std::fmt;
|
||||||
|
use std::fs;
|
||||||
use std::path::PathBuf;
|
use std::path::PathBuf;
|
||||||
use std::time::Duration;
|
use std::time::Duration;
|
||||||
|
|
||||||
|
use tonic::transport::{Certificate, ClientTlsConfig};
|
||||||
|
|
||||||
use crate::auth::ApiKey;
|
use crate::auth::ApiKey;
|
||||||
|
use crate::error::Error;
|
||||||
|
|
||||||
const DEFAULT_MAX_GRPC_MESSAGE_BYTES: usize = 16 * 1024 * 1024;
|
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<Option<ClientTlsConfig>, 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 {
|
impl Default for ClientOptions {
|
||||||
fn default() -> Self {
|
fn default() -> Self {
|
||||||
Self::new("http://127.0.0.1:5000")
|
Self::new("http://127.0.0.1:5000")
|
||||||
|
|||||||
@@ -10,7 +10,7 @@
|
|||||||
|
|
||||||
use std::time::Duration;
|
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`
|
/// Drive `connect` to its error without requiring `GatewayClient: Debug`
|
||||||
/// (the success arm is dropped explicitly so `unwrap_err` is unnecessary).
|
/// (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
|
/// 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.
|
/// PEM trust root so the CA-pinning path is exercised past the guard.
|
||||||
const SELF_SIGNED_CA_PEM: &str = "-----BEGIN CERTIFICATE-----
|
const SELF_SIGNED_CA_PEM: &str = "-----BEGIN CERTIFICATE-----
|
||||||
|
|||||||
Reference in New Issue
Block a user