From 90f34d9eb0060a90d15347ec4736cb00e288443e Mon Sep 17 00:00:00 2001 From: Bruce Arctor <5032356+brucearctor@users.noreply.github.com> Date: Wed, 13 May 2026 20:18:42 -0700 Subject: [PATCH 1/4] feat(client): support custom ServerCertVerifier in TlsOptions Add an optional `server_cert_verifier` field to `TlsOptions` that accepts an `Arc`. When set, the client uses tonic's `tls_config_with_verifier()` to replace the default WebPKI certificate verification. This enables: - Certificate pinning - Custom trust-domain validation (e.g., SAN-URI extraction) - Federated root certificate stores When a custom verifier is set, `server_root_ca_cert` is ignored (since tonic enforces mutual exclusion), while `domain` is still respected for the `:authority` header / origin override. The `ServerCertVerifier` trait is re-exported from the crate root for convenience, so users don't need to depend on `tokio-rustls` directly. Note: This does not cover `ResolvesClientCert` (dynamic client cert rotation), which would require either bypassing tonic's TLS config or upstream changes to tonic. See discussion in the linked issue. Closes #1248 --- crates/client/Cargo.toml | 1 + crates/client/src/envconfig.rs | 1 + crates/client/src/lib.rs | 174 ++++++++++++++++++++++++- crates/client/src/options_structs.rs | 37 +++++- crates/sdk-core-c-bridge/src/client.rs | 1 + crates/sdk-core/tests/common/mod.rs | 1 + 6 files changed, 207 insertions(+), 8 deletions(-) diff --git a/crates/client/Cargo.toml b/crates/client/Cargo.toml index 067bcfa1c..bf460cec9 100644 --- a/crates/client/Cargo.toml +++ b/crates/client/Cargo.toml @@ -45,6 +45,7 @@ tokio = { version = "1.47", default-features = false, features = [ "time", ] } tonic = { workspace = true, default-features = false, features = ["tls-native-roots", "channel"] } +tokio-rustls = { version = "0.26", default-features = false } tower = { version = "0.5", features = ["util"] } tracing = "0.1" url = "2.5" diff --git a/crates/client/src/envconfig.rs b/crates/client/src/envconfig.rs index 08d8ebb76..1b3a3fc50 100644 --- a/crates/client/src/envconfig.rs +++ b/crates/client/src/envconfig.rs @@ -89,6 +89,7 @@ fn build_tls_options(tls: ClientConfigTLS) -> Result { server_root_ca_cert, domain: tls.server_name, client_tls_options, + server_cert_verifier: None, }) } diff --git a/crates/client/src/lib.rs b/crates/client/src/lib.rs index 1f1f6ef05..1f13b4840 100644 --- a/crates/client/src/lib.rs +++ b/crates/client/src/lib.rs @@ -40,6 +40,9 @@ pub use metrics::{LONG_REQUEST_LATENCY_HISTOGRAM_NAME, REQUEST_LATENCY_HISTOGRAM pub use options_structs::*; pub use replaceable::SharedReplaceableClient; pub use retry::RetryOptions; +/// Re-export the `ServerCertVerifier` trait so that users can implement custom TLS +/// server certificate verification without depending on `tokio-rustls` directly. +pub use tokio_rustls::rustls::client::danger::ServerCertVerifier; pub use tonic; pub use workflow_handle::{ UntypedQuery, UntypedSignal, UntypedUpdate, UntypedWorkflow, UntypedWorkflowHandle, @@ -410,11 +413,15 @@ async fn add_tls_to_channel( if let Some(tls_cfg) = tls_options { let mut tls = tonic::transport::ClientTlsConfig::new(); - if let Some(root_cert) = &tls_cfg.server_root_ca_cert { - let server_root_ca_cert = Certificate::from_pem(root_cert); - tls = tls.ca_certificate(server_root_ca_cert); - } else { - tls = tls.with_native_roots(); + // When a custom verifier is set, server_root_ca_cert is ignored because + // tonic's tls_config_with_verifier replaces the default WebPKI verifier entirely. + if tls_cfg.server_cert_verifier.is_none() { + if let Some(root_cert) = &tls_cfg.server_root_ca_cert { + let server_root_ca_cert = Certificate::from_pem(root_cert); + tls = tls.ca_certificate(server_root_ca_cert); + } else { + tls = tls.with_native_roots(); + } } if let Some(domain) = &tls_cfg.domain { @@ -434,7 +441,13 @@ async fn add_tls_to_channel( tls = tls.identity(client_identity); } - return channel.tls_config(tls).map_err(Into::into); + return if let Some(verifier) = &tls_cfg.server_cert_verifier { + channel + .tls_config_with_verifier(tls, verifier.clone()) + .map_err(Into::into) + } else { + channel.tls_config(tls).map_err(Into::into) + }; } Ok(channel) } @@ -1485,6 +1498,155 @@ mod tests { assert!(opts.keep_alive.is_none()); } + mod tls_custom_verifier_tests { + use super::*; + use tokio_rustls::rustls::{ + client::danger::{HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier}, + pki_types::{CertificateDer, ServerName, UnixTime}, + DigitallySignedStruct, Error as RustlsError, SignatureScheme, + }; + + /// A minimal mock verifier for testing. In production, users would + /// implement real certificate pinning or custom validation here. + #[derive(Debug)] + struct MockVerifier; + + impl ServerCertVerifier for MockVerifier { + fn verify_server_cert( + &self, + _end_entity: &CertificateDer<'_>, + _intermediates: &[CertificateDer<'_>], + _server_name: &ServerName<'_>, + _ocsp_response: &[u8], + _now: UnixTime, + ) -> Result { + Ok(ServerCertVerified::assertion()) + } + + fn verify_tls12_signature( + &self, + _message: &[u8], + _cert: &CertificateDer<'_>, + _dss: &DigitallySignedStruct, + ) -> Result { + Ok(HandshakeSignatureValid::assertion()) + } + + fn verify_tls13_signature( + &self, + _message: &[u8], + _cert: &CertificateDer<'_>, + _dss: &DigitallySignedStruct, + ) -> Result { + Ok(HandshakeSignatureValid::assertion()) + } + + fn supported_verify_schemes(&self) -> Vec { + vec![ + SignatureScheme::ECDSA_NISTP256_SHA256, + SignatureScheme::RSA_PSS_SHA256, + ] + } + } + + #[test] + fn tls_options_with_verifier_default_is_none() { + let opts = TlsOptions::default(); + assert!(opts.server_cert_verifier.is_none()); + } + + #[test] + fn tls_options_with_verifier_clone() { + let opts = TlsOptions { + server_cert_verifier: Some(Arc::new(MockVerifier)), + ..Default::default() + }; + let cloned = opts.clone(); + assert!(cloned.server_cert_verifier.is_some()); + } + + #[test] + fn tls_options_debug_shows_custom_when_verifier_set() { + let opts = TlsOptions { + server_cert_verifier: Some(Arc::new(MockVerifier)), + domain: Some("test.example.com".to_string()), + ..Default::default() + }; + let debug_str = format!("{opts:?}"); + assert!( + debug_str.contains(""), + "Debug output should show for verifier, got: {debug_str}" + ); + assert!( + debug_str.contains("test.example.com"), + "Debug output should show domain, got: {debug_str}" + ); + } + + #[test] + fn tls_options_debug_shows_none_when_no_verifier() { + let opts = TlsOptions::default(); + let debug_str = format!("{opts:?}"); + assert!( + debug_str.contains("server_cert_verifier: None"), + "Debug output should show None for verifier, got: {debug_str}" + ); + } + + #[tokio::test] + async fn add_tls_to_channel_with_custom_verifier() { + let tls_opts = TlsOptions { + server_cert_verifier: Some(Arc::new(MockVerifier)), + domain: Some("test.temporal.io".to_string()), + ..Default::default() + }; + let endpoint = tonic::transport::Channel::from_static("https://test.temporal.io:7233"); + let result = add_tls_to_channel(Some(&tls_opts), endpoint).await; + assert!( + result.is_ok(), + "add_tls_to_channel should succeed with a custom verifier: {:?}", + result.err() + ); + } + + #[tokio::test] + async fn add_tls_to_channel_with_verifier_ignores_ca_cert() { + // When both server_cert_verifier and server_root_ca_cert are set, + // the CA cert should be ignored (not passed to tonic) to avoid + // tonic's VerifierConflict error. + let tls_opts = TlsOptions { + server_root_ca_cert: Some(b"some-ca-cert-bytes".to_vec()), + server_cert_verifier: Some(Arc::new(MockVerifier)), + domain: Some("test.temporal.io".to_string()), + ..Default::default() + }; + let endpoint = tonic::transport::Channel::from_static("https://test.temporal.io:7233"); + let result = add_tls_to_channel(Some(&tls_opts), endpoint).await; + assert!( + result.is_ok(), + "add_tls_to_channel should succeed even when server_root_ca_cert is set \ + alongside a custom verifier (CA cert should be ignored): {:?}", + result.err() + ); + } + + #[tokio::test] + async fn add_tls_to_channel_without_verifier_still_works() { + // Regression test: the original PEM path must still work. + let tls_opts = TlsOptions { + domain: Some("test.temporal.io".to_string()), + ..Default::default() + }; + let endpoint = tonic::transport::Channel::from_static("https://test.temporal.io:7233"); + let result = add_tls_to_channel(Some(&tls_opts), endpoint).await; + assert!( + result.is_ok(), + "add_tls_to_channel should succeed without a verifier (native roots): {:?}", + result.err() + ); + } + } + mod list_workflows_tests { use super::*; use futures_util::{FutureExt, StreamExt}; diff --git a/crates/client/src/options_structs.rs b/crates/client/src/options_structs.rs index a05923c98..2eb799b03 100644 --- a/crates/client/src/options_structs.rs +++ b/crates/client/src/options_structs.rs @@ -1,6 +1,6 @@ use crate::{HttpConnectProxyOptions, RetryOptions, VERSION, callback_based}; use http::Uri; -use std::{collections::HashMap, time::Duration}; +use std::{collections::HashMap, sync::Arc, time::Duration}; use temporalio_common::{ data_converters::DataConverter, protos::temporal::api::{ @@ -17,6 +17,7 @@ use temporalio_common::{ }, telemetry::metrics::TemporalMeter, }; +use tokio_rustls::rustls::client::danger::ServerCertVerifier; use url::Url; /// Options for [crate::Connection::connect]. @@ -132,7 +133,7 @@ pub struct ClientOptions { } /// Configuration options for TLS -#[derive(Clone, Debug, Default)] +#[derive(Clone, Default)] pub struct TlsOptions { /// Bytes representing the root CA certificate used by the server. If not set, and the server's /// cert is issued by someone the operating system trusts, verification will still work (ex: @@ -143,6 +144,38 @@ pub struct TlsOptions { pub domain: Option, /// TLS info for the client. If specified, core will attempt to use mTLS. pub client_tls_options: Option, + /// Optional custom server certificate verifier. When set, this replaces the default + /// certificate verification and `server_root_ca_cert` is ignored. + /// + /// This is useful for: + /// - Certificate pinning + /// - Custom trust-domain validation (e.g., SAN-URI extraction) + /// - Federated root certificate stores + /// + /// The verifier must implement [`ServerCertVerifier`] from the `rustls` crate. + /// Note that `domain` is still respected for the `:authority` header / origin override + /// even when a custom verifier is set. + pub server_cert_verifier: Option>, +} + +impl std::fmt::Debug for TlsOptions { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("TlsOptions") + .field( + "server_root_ca_cert", + &self + .server_root_ca_cert + .as_ref() + .map(|c| format!("{} bytes", c.len())), + ) + .field("domain", &self.domain) + .field("client_tls_options", &self.client_tls_options) + .field( + "server_cert_verifier", + &self.server_cert_verifier.as_ref().map(|_| ""), + ) + .finish() + } } /// If using mTLS, both the client cert and private key must be specified, this contains them. diff --git a/crates/sdk-core-c-bridge/src/client.rs b/crates/sdk-core-c-bridge/src/client.rs index a050533bd..674a85ad3 100644 --- a/crates/sdk-core-c-bridge/src/client.rs +++ b/crates/sdk-core-c-bridge/src/client.rs @@ -1432,6 +1432,7 @@ impl TryFrom<&ClientTlsOptions> for temporalio_client::TlsOptions { )); } }, + server_cert_verifier: None, }) } } diff --git a/crates/sdk-core/tests/common/mod.rs b/crates/sdk-core/tests/common/mod.rs index 7afe1c0e8..e3ff5e901 100644 --- a/crates/sdk-core/tests/common/mod.rs +++ b/crates/sdk-core/tests/common/mod.rs @@ -824,6 +824,7 @@ pub(crate) fn get_integ_tls_config() -> Option { client_cert, client_private_key, }), + server_cert_verifier: None, }) } else { None From ef49309967448ebef7b3f89fccba4352b67ef975 Mon Sep 17 00:00:00 2001 From: Bruce Arctor <5032356+brucearctor@users.noreply.github.com> Date: Tue, 19 May 2026 17:56:09 -0700 Subject: [PATCH 2/4] fix: address PR review feedback for custom ServerCertVerifier - Return InvalidConfig error when both server_root_ca_cert and server_cert_verifier are set (instead of silently ignoring) - Add WARNING doc comment about insecure connections - Move ServerCertVerifier re-export to pub mod danger to preserve the danger signal in the import path - Drop trivial tests per reviewer request --- crates/client/src/lib.rs | 74 +++++++--------------------- crates/client/src/options_structs.rs | 5 ++ 2 files changed, 23 insertions(+), 56 deletions(-) diff --git a/crates/client/src/lib.rs b/crates/client/src/lib.rs index 1f13b4840..d81e2f946 100644 --- a/crates/client/src/lib.rs +++ b/crates/client/src/lib.rs @@ -40,9 +40,13 @@ pub use metrics::{LONG_REQUEST_LATENCY_HISTOGRAM_NAME, REQUEST_LATENCY_HISTOGRAM pub use options_structs::*; pub use replaceable::SharedReplaceableClient; pub use retry::RetryOptions; -/// Re-export the `ServerCertVerifier` trait so that users can implement custom TLS -/// server certificate verification without depending on `tokio-rustls` directly. -pub use tokio_rustls::rustls::client::danger::ServerCertVerifier; +/// Danger-related TLS modules. +pub mod danger { + /// Re-export the `ServerCertVerifier` trait so that users can implement custom TLS + /// server certificate verification without depending on `tokio-rustls` directly, + /// while explicitly acknowledging the danger in the import path. + pub use tokio_rustls::rustls::client::danger::ServerCertVerifier; +} pub use tonic; pub use workflow_handle::{ UntypedQuery, UntypedSignal, UntypedUpdate, UntypedWorkflow, UntypedWorkflowHandle, @@ -411,10 +415,14 @@ async fn add_tls_to_channel( mut channel: Endpoint, ) -> Result { if let Some(tls_cfg) = tls_options { + if tls_cfg.server_cert_verifier.is_some() && tls_cfg.server_root_ca_cert.is_some() { + return Err(ClientConnectError::InvalidConfig( + "Cannot set both `server_root_ca_cert` and `server_cert_verifier`".to_owned(), + )); + } + let mut tls = tonic::transport::ClientTlsConfig::new(); - // When a custom verifier is set, server_root_ca_cert is ignored because - // tonic's tls_config_with_verifier replaces the default WebPKI verifier entirely. if tls_cfg.server_cert_verifier.is_none() { if let Some(root_cert) = &tls_cfg.server_root_ca_cert { let server_root_ca_cert = Certificate::from_pem(root_cert); @@ -1549,50 +1557,6 @@ mod tests { } } - #[test] - fn tls_options_with_verifier_default_is_none() { - let opts = TlsOptions::default(); - assert!(opts.server_cert_verifier.is_none()); - } - - #[test] - fn tls_options_with_verifier_clone() { - let opts = TlsOptions { - server_cert_verifier: Some(Arc::new(MockVerifier)), - ..Default::default() - }; - let cloned = opts.clone(); - assert!(cloned.server_cert_verifier.is_some()); - } - - #[test] - fn tls_options_debug_shows_custom_when_verifier_set() { - let opts = TlsOptions { - server_cert_verifier: Some(Arc::new(MockVerifier)), - domain: Some("test.example.com".to_string()), - ..Default::default() - }; - let debug_str = format!("{opts:?}"); - assert!( - debug_str.contains(""), - "Debug output should show for verifier, got: {debug_str}" - ); - assert!( - debug_str.contains("test.example.com"), - "Debug output should show domain, got: {debug_str}" - ); - } - - #[test] - fn tls_options_debug_shows_none_when_no_verifier() { - let opts = TlsOptions::default(); - let debug_str = format!("{opts:?}"); - assert!( - debug_str.contains("server_cert_verifier: None"), - "Debug output should show None for verifier, got: {debug_str}" - ); - } - #[tokio::test] async fn add_tls_to_channel_with_custom_verifier() { let tls_opts = TlsOptions { @@ -1610,10 +1574,9 @@ mod tests { } #[tokio::test] - async fn add_tls_to_channel_with_verifier_ignores_ca_cert() { + async fn add_tls_to_channel_with_verifier_and_ca_cert_fails() { // When both server_cert_verifier and server_root_ca_cert are set, - // the CA cert should be ignored (not passed to tonic) to avoid - // tonic's VerifierConflict error. + // add_tls_to_channel should fail with InvalidConfig. let tls_opts = TlsOptions { server_root_ca_cert: Some(b"some-ca-cert-bytes".to_vec()), server_cert_verifier: Some(Arc::new(MockVerifier)), @@ -1623,10 +1586,9 @@ mod tests { let endpoint = tonic::transport::Channel::from_static("https://test.temporal.io:7233"); let result = add_tls_to_channel(Some(&tls_opts), endpoint).await; assert!( - result.is_ok(), - "add_tls_to_channel should succeed even when server_root_ca_cert is set \ - alongside a custom verifier (CA cert should be ignored): {:?}", - result.err() + matches!(result, Err(ClientConnectError::InvalidConfig(_))), + "add_tls_to_channel should fail with InvalidConfig when both CA cert and verifier are set: {:?}", + result ); } diff --git a/crates/client/src/options_structs.rs b/crates/client/src/options_structs.rs index 2eb799b03..558fff79b 100644 --- a/crates/client/src/options_structs.rs +++ b/crates/client/src/options_structs.rs @@ -152,6 +152,11 @@ pub struct TlsOptions { /// - Custom trust-domain validation (e.g., SAN-URI extraction) /// - Federated root certificate stores /// + /// # WARNING + /// Implementing a custom `ServerCertVerifier` can lead to severely insecure TLS connections + /// (e.g., disabling all validation or allowing man-in-the-middle attacks) if not done carefully. + /// Only use this if you know exactly what you are doing. + /// /// The verifier must implement [`ServerCertVerifier`] from the `rustls` crate. /// Note that `domain` is still respected for the `:authority` header / origin override /// even when a custom verifier is set. From 6e766f535b124622ece9b82d9a7d46806272bb3d Mon Sep 17 00:00:00 2001 From: Spencer Judge Date: Wed, 20 May 2026 09:32:23 -0700 Subject: [PATCH 3/4] Change module docstring --- crates/client/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/client/src/lib.rs b/crates/client/src/lib.rs index d81e2f946..6bc0ac9f7 100644 --- a/crates/client/src/lib.rs +++ b/crates/client/src/lib.rs @@ -40,7 +40,7 @@ pub use metrics::{LONG_REQUEST_LATENCY_HISTOGRAM_NAME, REQUEST_LATENCY_HISTOGRAM pub use options_structs::*; pub use replaceable::SharedReplaceableClient; pub use retry::RetryOptions; -/// Danger-related TLS modules. +/// Potentially dangerous TLS related functionality. pub mod danger { /// Re-export the `ServerCertVerifier` trait so that users can implement custom TLS /// server certificate verification without depending on `tokio-rustls` directly, From 6566946c587d9a4620b8e9b1f6b07a966420cfd7 Mon Sep 17 00:00:00 2001 From: Bruce Arctor <5032356+brucearctor@users.noreply.github.com> Date: Thu, 21 May 2026 18:26:34 -0700 Subject: [PATCH 4/4] style: cargo fmt --all --- crates/client/src/lib.rs | 2 +- crates/sdk/src/workflows.rs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/client/src/lib.rs b/crates/client/src/lib.rs index 6bc0ac9f7..0b0022e9b 100644 --- a/crates/client/src/lib.rs +++ b/crates/client/src/lib.rs @@ -1509,9 +1509,9 @@ mod tests { mod tls_custom_verifier_tests { use super::*; use tokio_rustls::rustls::{ + DigitallySignedStruct, Error as RustlsError, SignatureScheme, client::danger::{HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier}, pki_types::{CertificateDer, ServerName, UnixTime}, - DigitallySignedStruct, Error as RustlsError, SignatureScheme, }; /// A minimal mock verifier for testing. In production, users would diff --git a/crates/sdk/src/workflows.rs b/crates/sdk/src/workflows.rs index 4e290314e..7ddae60d2 100644 --- a/crates/sdk/src/workflows.rs +++ b/crates/sdk/src/workflows.rs @@ -56,8 +56,7 @@ /// /// ```no_run /// use std::time::Duration; -/// use temporalio_sdk::workflows::select; -/// use temporalio_sdk::WorkflowContext; +/// use temporalio_sdk::{WorkflowContext, workflows::select}; /// /// # async fn hidden(ctx: &mut WorkflowContext<()>) { /// select! {