Add TLSConfig.verification_server_name to decouple certificate verification from SNI#1651
Open
kollektiv wants to merge 1 commit into
Open
Add TLSConfig.verification_server_name to decouple certificate verification from SNI#1651kollektiv wants to merge 1 commit into
kollektiv wants to merge 1 commit into
Conversation
…cation from SNI When set, the server certificate is verified against this fixed name (using server_root_ca_cert) via a custom rustls ServerCertVerifier built in the bridge, while SNI and the HTTP/2 authority keep following the connected host (or domain, if set). This supports servers whose certificate does not carry the dialed name without overriding SNI, e.g. when the connection traverses an SNI-inspecting proxy that must be able to resolve the SNI value.
|
|
tconley1428
reviewed
Jul 16, 2026
| "ephemeral-server", | ||
| ] } | ||
| tokio = "1.26" | ||
| # Matches the sdk-core client crate's rustls stack; used to build the custom |
Contributor
There was a problem hiding this comment.
I don't see anything like this. How is tokio-rustls used in the bridge?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was changed
Adds an optional
TLSConfig.verification_server_name. When set, the server certificate is verified against this fixed name instead of the connection's server name, while the TLS SNI and HTTP/2:authorityvalues keep following the connected host (ordomain, if set).Today the only way to verify a certificate that carries a different name than the one being dialed is
TLSConfig.domain, butdomaincouples three things: the name the certificate is checked against, the SNI, and the:authorityheader. So verifying against name X forces SNI to X too.This came up connecting through an SNI-inspecting egress proxy that re-resolves the SNI hostname: the server's certificate carries an internal-only name, so setting
domainto that name made the connection unresolvable at the proxy, and leavingdomainunset failed certificate verification. There's an earlier ask in the same family in #463.Implementation
The bundled sdk-core client crate already exposes
TlsOptions.server_cert_verifier; the Python bridge just hardcoded it toNone. This wires it up: whenverification_server_nameis set, the bridge builds a rustlsServerCertVerifierthat delegates to the standardWebPkiServerVerifier(full chain, validity, and configured revocation checks) but substitutes the fixed name for the connection's server name. Root loading and crypto-provider selection mirror tonic's default (non-verifier) client path, so trust and negotiation match an ordinary connection.Because a custom verifier can't be combined with connection roots, the CA bundle from
server_root_ca_certis consumed by the verifier's own root store;server_root_ca_certis therefore required whenverification_server_nameis set (validated up front with a clear error). Existing behavior when the field is unset is byte-for-byte unchanged.Deliberately not touched:
envconfigTOML profiles, since that schema mirrors the cross-SDK envconfig spec and a Python-only key would fork it. Happy to follow up if you'd rather that be added.Testing
tests/test_tls.pyruns an in-process TLS server whose certificate is valid only forpinned.testwhile the client dialslocalhost, and asserts on both the handshake outcome and the SNI the server observed:verification_server_name="pinned.test"the handshake succeeds and the observed SNI islocalhost, not the pinned name;Also probed the bridge with malformed/truncated/empty PEM and invalid names; all surface as
ValueError, no panics.Open design question for reviewers: this could instead live in sdk-core's
TlsOptionsso every language bridge gets it. Kept it in the bridge here to stay self-contained, but glad to move it upstream to core if that's preferred.