diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a1a92a95..a33b1e9f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,13 @@ to include examples, links to docs, or any other relevant information. ### Added +- Added `TLSConfig.verification_server_name` to verify the server certificate against a fixed name + instead of the connection's server name. Unlike `domain`, it does not change the TLS SNI or + HTTP/2 authority values, which keep following the connected host, so it can be used when the + server's certificate does not carry the dialed name but on-path infrastructure (e.g. an + SNI-inspecting egress proxy) needs the SNI to remain resolvable. Requires + `server_root_ca_cert`. + ### Changed ### Deprecated diff --git a/pyproject.toml b/pyproject.toml index bcad8903c..7a8904b51 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -53,6 +53,7 @@ Documentation = "https://docs.temporal.io/docs/python" dev = [ "basedpyright==1.34.0", "cibuildwheel>=2.22.0,<3", + "cryptography>=46", "grpcio-tools>=1.48.2,<2", "mypy==1.18.2", "mypy-protobuf>=3.3.0,<4", diff --git a/temporalio/bridge/Cargo.lock b/temporalio/bridge/Cargo.lock index 1d45b94de..a92196ee3 100644 --- a/temporalio/bridge/Cargo.lock +++ b/temporalio/bridge/Cargo.lock @@ -2520,6 +2520,7 @@ dependencies = [ "temporalio-common", "temporalio-sdk-core", "tokio", + "tokio-rustls", "tokio-stream", "tonic", "tracing", diff --git a/temporalio/bridge/Cargo.toml b/temporalio/bridge/Cargo.toml index a1c6b5e3f..5984d9546 100644 --- a/temporalio/bridge/Cargo.toml +++ b/temporalio/bridge/Cargo.toml @@ -36,6 +36,9 @@ temporalio-sdk-core = { version = "0.5", path = "./sdk-core/crates/sdk-core", fe "ephemeral-server", ] } tokio = "1.26" +# Matches the sdk-core client crate's rustls stack; used to build the custom +# server certificate verifier for ClientTlsConfig.verification_server_name. +tokio-rustls = { version = "0.26", default-features = false, features = ["ring"] } tokio-stream = "0.1" tonic = "0.14" tracing = "0.1" diff --git a/temporalio/bridge/client.py b/temporalio/bridge/client.py index c2c5bef6e..ab1536e90 100644 --- a/temporalio/bridge/client.py +++ b/temporalio/bridge/client.py @@ -27,6 +27,7 @@ class ClientTlsConfig: domain: str | None client_cert: bytes | None client_private_key: bytes | None + verification_server_name: str | None @dataclass diff --git a/temporalio/bridge/src/client.rs b/temporalio/bridge/src/client.rs index 85dedef94..9440b01b7 100644 --- a/temporalio/bridge/src/client.rs +++ b/temporalio/bridge/src/client.rs @@ -2,6 +2,7 @@ use pyo3::exceptions::{PyException, PyRuntimeError, PyValueError}; use pyo3::prelude::*; use std::collections::HashMap; use std::str::FromStr; +use std::sync::Arc; use std::time::Duration; use temporalio_client::tonic::{ self, @@ -11,6 +12,14 @@ use temporalio_client::{ ClientKeepAliveOptions as CoreClientKeepAliveConfig, Connection, ConnectionOptions, DnsLoadBalancingOptions, GrpcCompression, HttpConnectProxyOptions, RetryOptions, }; +use tokio_rustls::rustls::client::danger::{ + HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier, +}; +use tokio_rustls::rustls::client::WebPkiServerVerifier; +use tokio_rustls::rustls::crypto::CryptoProvider; +use tokio_rustls::rustls::pki_types::pem::PemObject; +use tokio_rustls::rustls::pki_types::{CertificateDer, ServerName, UnixTime}; +use tokio_rustls::rustls::{self, DigitallySignedStruct, RootCertStore, SignatureScheme}; use tracing::warn; use url::Url; @@ -46,6 +55,7 @@ struct ClientTlsConfig { domain: Option, client_cert: Option>, client_private_key: Option>, + verification_server_name: Option, } #[derive(FromPyObject)] @@ -295,8 +305,22 @@ impl TryFrom for temporalio_client::TlsOptions { type Error = PyErr; fn try_from(conf: ClientTlsConfig) -> PyResult { + let mut server_root_ca_cert = conf.server_root_ca_cert; + let server_cert_verifier = match conf.verification_server_name { + None => None, + Some(name) => { + // The CA bundle is consumed by the verifier's own root store; a + // custom verifier cannot be combined with roots on the connection. + let ca_cert = server_root_ca_cert.take().ok_or_else(|| { + PyValueError::new_err( + "Must have server root CA cert when verification server name is set", + ) + })?; + Some(fixed_server_name_verifier(&name, &ca_cert)?) + } + }; Ok(temporalio_client::TlsOptions { - server_root_ca_cert: conf.server_root_ca_cert, + server_root_ca_cert, domain: conf.domain, client_tls_options: match (conf.client_cert, conf.client_private_key) { (None, None) => None, @@ -312,11 +336,92 @@ impl TryFrom for temporalio_client::TlsOptions { )) } }, - server_cert_verifier: None, + server_cert_verifier, }) } } +/// Builds a standard WebPKI verifier over the given root CA bundle that +/// checks the certificate against `verification_server_name` rather than the +/// connection's server name, leaving SNI/`:authority` to follow the +/// connected host (or `domain` when set). +fn fixed_server_name_verifier( + verification_server_name: &str, + ca_cert_pem: &[u8], +) -> PyResult> { + let certs = CertificateDer::pem_slice_iter(ca_cert_pem) + .collect::, _>>() + .map_err(|err| { + PyValueError::new_err(format!("Invalid server root CA cert PEM: {err:?}")) + })?; + // Root loading and provider selection mirror tonic's default (no custom + // verifier) client path: unparsable certificates in the bundle are + // skipped, and the provider is the process default if one is installed, + // else ring, as with the connection's `tls-ring` feature. + let mut roots = RootCertStore::empty(); + roots.add_parsable_certificates(certs); + let provider = CryptoProvider::get_default() + .cloned() + .unwrap_or_else(|| Arc::new(rustls::crypto::ring::default_provider())); + let inner = WebPkiServerVerifier::builder_with_provider(roots.into(), provider) + .build() + .map_err(|err| { + PyValueError::new_err(format!("Failed building certificate verifier: {err}")) + })?; + let server_name = ServerName::try_from(verification_server_name.to_owned()) + .map_err(|err| PyValueError::new_err(format!("Invalid verification server name: {err}")))?; + Ok(Arc::new(FixedServerNameVerifier { inner, server_name })) +} + +/// Delegates to the standard WebPKI verifier, but verifies the certificate +/// against a fixed server name instead of the connection's server name. +#[derive(Debug)] +struct FixedServerNameVerifier { + inner: Arc, + server_name: ServerName<'static>, +} + +impl ServerCertVerifier for FixedServerNameVerifier { + fn verify_server_cert( + &self, + end_entity: &CertificateDer<'_>, + intermediates: &[CertificateDer<'_>], + _server_name: &ServerName<'_>, + ocsp_response: &[u8], + now: UnixTime, + ) -> Result { + self.inner.verify_server_cert( + end_entity, + intermediates, + &self.server_name, + ocsp_response, + now, + ) + } + + fn verify_tls12_signature( + &self, + message: &[u8], + cert: &CertificateDer<'_>, + dss: &DigitallySignedStruct, + ) -> Result { + self.inner.verify_tls12_signature(message, cert, dss) + } + + fn verify_tls13_signature( + &self, + message: &[u8], + cert: &CertificateDer<'_>, + dss: &DigitallySignedStruct, + ) -> Result { + self.inner.verify_tls13_signature(message, cert, dss) + } + + fn supported_verify_schemes(&self) -> Vec { + self.inner.supported_verify_schemes() + } +} + impl From for RetryOptions { fn from(conf: ClientRetryConfig) -> Self { RetryOptions { diff --git a/temporalio/service.py b/temporalio/service.py index d4cb79720..be46485e6 100644 --- a/temporalio/service.py +++ b/temporalio/service.py @@ -43,7 +43,9 @@ class TLSConfig: """Root CA to validate the server certificate against.""" domain: str | None = None - """TLS domain.""" + """SNI host and HTTP/2 authority override, and the default name the server + certificate is verified against (see + :py:attr:`verification_server_name`).""" client_cert: bytes | None = None """Client certificate for mTLS. @@ -55,12 +57,26 @@ class TLSConfig: This must be combined with :py:attr:`client_cert`.""" + verification_server_name: str | None = None + """Name to verify the server certificate against, instead of the + :py:attr:`domain` / connected host. + + Unlike :py:attr:`domain`, this does not change the TLS SNI or HTTP/2 + authority values, which continue to follow the connected host (or + :py:attr:`domain` if set). Use this when the server's certificate does not + carry the name being dialed, e.g. when the connection traverses an + SNI-inspecting proxy that must be able to resolve the SNI value. + + Requires :py:attr:`server_root_ca_cert`; the system root store is not + consulted when this is set.""" + def _to_bridge_config(self) -> temporalio.bridge.client.ClientTlsConfig: return temporalio.bridge.client.ClientTlsConfig( server_root_ca_cert=self.server_root_ca_cert, domain=self.domain, client_cert=self.client_cert, client_private_key=self.client_private_key, + verification_server_name=self.verification_server_name, ) diff --git a/tests/test_tls.py b/tests/test_tls.py new file mode 100644 index 000000000..cc0eddde9 --- /dev/null +++ b/tests/test_tls.py @@ -0,0 +1,227 @@ +"""Tests for :py:attr:`temporalio.service.TLSConfig.verification_server_name`. + +Each test runs an in-process TLS server whose certificate is valid only for +``pinned.test`` while the client always dials ``localhost``, so no Temporal +server is needed. The server records each handshake's outcome and the SNI it +received. +""" + +from __future__ import annotations + +import asyncio +import datetime +import socket +import ssl +import threading +from collections.abc import AsyncIterator +from dataclasses import dataclass +from pathlib import Path + +import pytest +import pytest_asyncio +from cryptography import x509 +from cryptography.hazmat.primitives import hashes, serialization +from cryptography.hazmat.primitives.asymmetric import ec +from cryptography.x509.oid import NameOID + +import temporalio.service + +PINNED_NAME = "pinned.test" + + +def test_tls_config_verification_server_name_reaches_bridge(): + config = temporalio.service.TLSConfig(verification_server_name=PINNED_NAME) + assert config._to_bridge_config().verification_server_name == PINNED_NAME + default = temporalio.service.TLSConfig() + assert default._to_bridge_config().verification_server_name is None + + +async def test_tls_verification_server_name_requires_root_ca(): + # The check is enforced when building connection options, before any dial. + with pytest.raises(ValueError, match="server root CA cert"): + await temporalio.service.ServiceClient.connect( + temporalio.service.ConnectConfig( + target_host="localhost:1", + tls=temporalio.service.TLSConfig(verification_server_name=PINNED_NAME), + ) + ) + + +async def test_tls_default_verification_rejects_unmatched_name(tls_server: _TlsServer): + # Baseline for the tests below: the certificate is only valid for the + # pinned name, so verifying against the dialed host rejects it. + handshake = await _handshake( + tls_server, temporalio.service.TLSConfig(server_root_ca_cert=tls_server.ca_pem) + ) + assert not handshake.ok + + +async def test_tls_verification_server_name_decouples_verification_from_sni( + tls_server: _TlsServer, +): + # Verification against the pinned name succeeds, while the SNI the server + # sees is still the dialed host rather than the pinned name. + handshake = await _handshake( + tls_server, + temporalio.service.TLSConfig( + server_root_ca_cert=tls_server.ca_pem, + verification_server_name=PINNED_NAME, + ), + ) + assert handshake.ok + assert handshake.sni == "localhost" + + +async def test_tls_verification_server_name_is_enforced(tls_server: _TlsServer): + # A pinned name the certificate does not carry is still rejected; the + # option redirects verification rather than disabling it. + handshake = await _handshake( + tls_server, + temporalio.service.TLSConfig( + server_root_ca_cert=tls_server.ca_pem, + verification_server_name="wrong.test", + ), + ) + assert not handshake.ok + + +@dataclass +class _Handshake: + ok: bool + sni: str | None + + +async def _handshake( + server: _TlsServer, tls: temporalio.service.TLSConfig +) -> _Handshake: + """Attempt a connection and return the server's view of the handshake.""" + config = temporalio.service.ConnectConfig( + target_host=f"localhost:{server.port}", tls=tls + ) + # The connect always fails since this server speaks no gRPC; only whether + # the TLS handshake completed matters. + try: + await asyncio.wait_for(temporalio.service.ServiceClient.connect(config), 20) + except asyncio.TimeoutError: + raise + except Exception: + pass + else: + pytest.fail("connect unexpectedly succeeded") + return await server.next_handshake() + + +class _TlsServer: + """TLS server that records handshake outcomes and the SNI it receives.""" + + def __init__(self, certs: Path) -> None: + self.ca_pem = (certs / "ca.pem").read_bytes() + self._ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + self._ctx.load_cert_chain(certs / "srv.pem", certs / "srv.key") + self._ctx.set_alpn_protocols(["h2"]) + # Populated by the SNI callback for the connection currently + # handshaking; the accept loop is single-threaded. + self._sni: str | None = None + + def on_sni( + _sock: ssl.SSLObject, name: str | None, _ctx: ssl.SSLContext + ) -> None: + self._sni = name + + self._ctx.sni_callback = on_sni + self._handshakes: list[_Handshake] = [] + self._lock = threading.Lock() + self._stop = threading.Event() + self._sock = socket.create_server(("127.0.0.1", 0)) + self._sock.settimeout(0.1) + self.port: int = self._sock.getsockname()[1] + self._thread = threading.Thread(target=self._serve, daemon=True) + self._thread.start() + + def _serve(self) -> None: + while not self._stop.is_set(): + try: + conn, _ = self._sock.accept() + except TimeoutError: + continue + conn.settimeout(10) # A stalled handshake must not wedge this loop. + self._sni = None + try: + tls = self._ctx.wrap_socket(conn, server_side=True) + except (ssl.SSLError, OSError): + conn.close() + self._record(_Handshake(ok=False, sni=self._sni)) + continue + self._record(_Handshake(ok=True, sni=self._sni)) + tls.close() + self._sock.close() + + def _record(self, handshake: _Handshake) -> None: + with self._lock: + self._handshakes.append(handshake) + + async def next_handshake(self) -> _Handshake: + """Wait for and consume the next recorded handshake.""" + # The client can report its result before this thread has recorded + # the outcome, so wait for the observation to land. + for _ in range(200): + with self._lock: + if self._handshakes: + return self._handshakes.pop(0) + await asyncio.sleep(0.05) + raise AssertionError("server observed no TLS handshake") + + def close(self) -> None: + self._stop.set() + self._thread.join(timeout=2) + + +@pytest_asyncio.fixture # type: ignore[reportUntypedFunctionDecorator] +async def tls_server(tmp_path: Path) -> AsyncIterator[_TlsServer]: + _write_pinned_certs(tmp_path) + server = _TlsServer(tmp_path) + try: + yield server + finally: + server.close() + + +def _write_pinned_certs(path: Path) -> None: + """Write a CA and a server cert (chained to it) valid only for ``pinned.test``.""" + now = datetime.datetime.now(datetime.timezone.utc) + ca_key = ec.generate_private_key(ec.SECP256R1()) + ca_name = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, "test-ca")]) + ca_cert = ( + x509.CertificateBuilder() + .subject_name(ca_name) + .issuer_name(ca_name) + .public_key(ca_key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(now - datetime.timedelta(days=1)) + .not_valid_after(now + datetime.timedelta(days=7)) + .add_extension(x509.BasicConstraints(ca=True, path_length=None), critical=True) + .sign(ca_key, hashes.SHA256()) + ) + server_key = ec.generate_private_key(ec.SECP256R1()) + server_cert = ( + x509.CertificateBuilder() + .subject_name(x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, PINNED_NAME)])) + .issuer_name(ca_name) + .public_key(server_key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(now - datetime.timedelta(days=1)) + .not_valid_after(now + datetime.timedelta(days=7)) + .add_extension( + x509.SubjectAlternativeName([x509.DNSName(PINNED_NAME)]), critical=False + ) + .sign(ca_key, hashes.SHA256()) + ) + (path / "ca.pem").write_bytes(ca_cert.public_bytes(serialization.Encoding.PEM)) + (path / "srv.pem").write_bytes(server_cert.public_bytes(serialization.Encoding.PEM)) + (path / "srv.key").write_bytes( + server_key.private_bytes( + serialization.Encoding.PEM, + serialization.PrivateFormat.PKCS8, + serialization.NoEncryption(), + ) + ) diff --git a/uv.lock b/uv.lock index 0543cf0ed..3633a0a96 100644 --- a/uv.lock +++ b/uv.lock @@ -4721,6 +4721,7 @@ dev = [ { name = "async-timeout", marker = "python_full_version < '3.11'" }, { name = "basedpyright" }, { name = "cibuildwheel" }, + { name = "cryptography" }, { name = "googleapis-common-protos" }, { name = "grpcio-tools" }, { name = "httpx" }, @@ -4792,6 +4793,7 @@ dev = [ { name = "async-timeout", marker = "python_full_version < '3.11'", specifier = ">=4.0,<6" }, { name = "basedpyright", specifier = "==1.34.0" }, { name = "cibuildwheel", specifier = ">=2.22.0,<3" }, + { name = "cryptography", specifier = ">=46" }, { name = "googleapis-common-protos", specifier = ">=1.75.0,<2" }, { name = "grpcio-tools", specifier = ">=1.48.2,<2" }, { name = "httpx", specifier = ">=0.28.1" },