CSHARP-6005: CSFLE/QE support for HTTP Proxies - #2077
Conversation
| /// <param name="port">The KMS port (typically 443).</param> | ||
| /// <param name="cancellationToken">The cancellation token.</param> | ||
| /// <returns>A stream connected to the KMS host. The driver wraps this stream in TLS.</returns> | ||
| Stream Connect(string host, int port, CancellationToken cancellationToken); |
There was a problem hiding this comment.
Should we use Endpoint instead of host and port?
There was a problem hiding this comment.
Discussed offline as well but for visibility:
I'd keep host/port. EndPoint looks tidier but the base class doesn't actually expose Host/Port — they only exist on DnsEndPoint, so a Connect(EndPoint, ...) signature forces every user to downcast and just know it's always a DnsEndPoint. host/port is self-documenting, needs no cast, and maps 1:1 to the spec's callback.
| /// returned stream in TLS using the KMS provider's configured TLS options. | ||
| /// The primary use case is routing KMS traffic through an HTTP proxy via HTTPS CONNECT. | ||
| /// </summary> | ||
| public interface IKmsConnector |
There was a problem hiding this comment.
Why do we need this new abstraction? IStreamFactory looks very similar and it's public too. Can we let users provide the stream factory and do not introduce a new interface?
There was a problem hiding this comment.
Discussed offline but I added some reasoning in the PR description.
f46cc3a to
8dbcd87
Compare
There was a problem hiding this comment.
Pull request overview
Adds driver-side support for routing CSFLE/Queryable Encryption KMS traffic through an HTTP CONNECT proxy by introducing a connector callback (IKmsConnector) that supplies the underlying transport stream used for KMS TLS.
Changes:
- Introduces public
IKmsConnectorand wires it throughClientEncryptionOptions/AutoEncryptionOptions. - Updates the libmongocrypt KMS I/O path to use a connector-backed
IStreamFactorywhen configured (sync + async). - Adds prose test 28 (“KMS Connect Callback”) coverage for proxy tunneling, error propagation, and retry behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/MongoDB.Driver.Tests/Specifications/client-side-encryption/prose-tests/ClientEncryptionProseTests.cs | Adds prose tests + helper connector/proxy plumbing for KMS connect callback scenarios. |
| src/MongoDB.Driver/Encryption/IKmsConnector.cs | Defines the new public connector interface for opening KMS transport streams. |
| src/MongoDB.Driver/AutoEncryptionOptions.cs | Adds KmsConnector option plumbed through options and With(...). |
| src/MongoDB.Driver.Encryption/LibMongoCryptControllerBase.cs | Uses connector-backed base stream factory for KMS TLS when configured. |
| src/MongoDB.Driver.Encryption/KmsConnectorStreamFactory.cs | Adapts IKmsConnector to internal IStreamFactory used by TLS wrapping. |
| src/MongoDB.Driver.Encryption/ExplicitEncryptionLibMongoCryptController.cs | Passes connector from ClientEncryptionOptions into controller base. |
| src/MongoDB.Driver.Encryption/ClientEncryptionOptions.cs | Adds KmsConnector option plumbed through options and With(...). |
| src/MongoDB.Driver.Encryption/AutoEncryptionLibMongoController.cs | Passes connector from AutoEncryptionOptions into controller base. |
Comments suppressed due to low confidence (2)
src/MongoDB.Driver/AutoEncryptionOptions.cs:221
- The AutoEncryptionOptions.With method signature was changed by adding a new parameter, which is also a binary breaking change for compiled consumers. Please reintroduce the previous With overload (without kmsConnector) and have it delegate to the new overload to preserve compatibility.
public AutoEncryptionOptions With(
Optional<CollectionNamespace> keyVaultNamespace = default,
Optional<IReadOnlyDictionary<string, IReadOnlyDictionary<string, object>>> kmsProviders = default,
Optional<bool> bypassAutoEncryption = default,
Optional<bool?> bypassQueryAnalysis = default,
src/MongoDB.Driver.Encryption/ClientEncryptionOptions.cs:134
- The ClientEncryptionOptions.With method signature was changed by adding a new parameter, which is binary breaking for compiled consumers. Please reintroduce the previous overload (without kmsConnector) and delegate it to the new overload to preserve compatibility.
public ClientEncryptionOptions With(
Optional<IMongoClient> keyVaultClient = default,
Optional<CollectionNamespace> keyVaultNamespace = default,
Optional<IReadOnlyDictionary<string, IReadOnlyDictionary<string, object>>> kmsProviders = default,
Optional<IReadOnlyDictionary<string, SslSettings>> tlsOptions = default,
Optional<IKmsConnector> kmsConnector = default)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public ClientEncryptionOptions( | ||
| IMongoClient keyVaultClient, | ||
| CollectionNamespace keyVaultNamespace, | ||
| IReadOnlyDictionary<string, IReadOnlyDictionary<string, object>> kmsProviders, | ||
| Optional<IReadOnlyDictionary<string, SslSettings>> tlsOptions = default) | ||
| : this(keyVaultClient, keyVaultNamespace, kmsProviders, tlsOptions, keyExpiration: null) | ||
| Optional<IReadOnlyDictionary<string, SslSettings>> tlsOptions = default, | ||
| Optional<IKmsConnector> kmsConnector = default) | ||
| : this(keyVaultClient, keyVaultNamespace, kmsProviders, tlsOptions, kmsConnector, keyExpiration: null) |
There was a problem hiding this comment.
@adelinowona I guess we'll address this during backporting?
| public ClientEncryptionOptions( | ||
| IMongoClient keyVaultClient, | ||
| CollectionNamespace keyVaultNamespace, | ||
| IReadOnlyDictionary<string, IReadOnlyDictionary<string, object>> kmsProviders, | ||
| Optional<IReadOnlyDictionary<string, SslSettings>> tlsOptions = default) | ||
| : this(keyVaultClient, keyVaultNamespace, kmsProviders, tlsOptions, keyExpiration: null) | ||
| Optional<IReadOnlyDictionary<string, SslSettings>> tlsOptions = default, | ||
| Optional<IKmsConnector> kmsConnector = default) | ||
| : this(keyVaultClient, keyVaultNamespace, kmsProviders, tlsOptions, kmsConnector, keyExpiration: null) |
There was a problem hiding this comment.
@adelinowona I guess we'll address this during backporting?
|
|
||
| /// <summary> | ||
| /// Opens the transport connection used to reach a KMS host. When supplied via | ||
| /// ClientEncryptionOptions or <see cref="AutoEncryptionOptions"/>, the driver |
There was a problem hiding this comment.
ClientEncryptionOptions ?
There was a problem hiding this comment.
ClientEncryptionOptions type lives in the encryption package which isn't referenced by the driver package so the see cref won't resolve. I am assuming that's your question here.
There was a problem hiding this comment.
Sorry, I meant <c>ClientEncryptionOptions <c>
| { | ||
| var (host, port) = GetHostAndPort(endPoint); | ||
| var stream = _kmsConnector.Connect(host, port, cancellationToken); | ||
| return Ensure.IsNotNull(stream, $"{nameof(IKmsConnector)}.{nameof(IKmsConnector.Connect)}"); |
There was a problem hiding this comment.
minor: Consider a clearer message, like "connector returned null" or similar.
| /// Gets the KMS connector used to open connections to KMS hosts. | ||
| /// </summary> | ||
| /// <value> | ||
| /// The KMS connector to connect directly to KMS hosts. |
There was a problem hiding this comment.
I think technically it's not "directly to KMS hosts"?
https://jira.mongodb.org/browse/CSHARP-6005
Adds support for routing CSFLE/Queryable Encryption KMS traffic through an HTTP proxy (HTTP
CONNECTtunnel), implementing the spec proposed in specifications#1956 (DRIVERS-2920).Public API
A new optional
IKmsConnectorcan be set onClientEncryptionOptionsandAutoEncryptionOptions:When supplied, the driver invokes the connector to obtain the transport stream to a KMS host (
host/port) instead of opening a direct socket, then performs the KMS TLS handshake over the returned stream using the provider's configured TLS options. The typical implementation opens anHTTP CONNECTtunnel to a proxy and returns the tunnel stream. When no connector is set, behavior is unchanged (direct connection).How it works
The KMS state machine already builds
new SslStreamFactory(tlsOptions, baseStreamFactory)and runs TLS against the KMS endpoint. This change swaps the innerbaseStreamFactoryfor a connector-backedKmsConnectorStreamFactorywhen a connector is configured. Because the endpoint is unchanged, SNI and certificate/hostname verification still target the real KMS host, not the proxy — so the driver verifies KMS's identity end-to-end through the tunnel. This applies to both explicit encryption (ClientEncryption) and auto-encryption, and the existing KMS retry re-invokes the connector on transient network errors.No libmongocrypt change is required — this is entirely driver-side.
Why a new
IKmsConnectorrather than reusingIStreamFactory?IStreamFactoryis public and already models "create a stream to an endpoint," so it was a candidate for the callback type. We introduced a dedicated interface instead, for three reasons:IStreamFactoryoperates onEndPoint, which would force users to unpack aDnsEndPointand leak transport plumbing into their callback. The KMS-connect contract is conceptually(host, port) -> stream;IKmsConnectorexposes exactly that, matching the cross-driver spec'skmsConnectCallback(host, port)and the shapes used by the Node and C drivers.IStreamFactoryis the general-purpose transport abstraction used throughout SDAM, connection pools, TLS, and SOCKS5. Reusing it as the KMS-proxy hook would conflate a narrow, KMS-only extension point with a broad internal-transport contract, and would couple this public API to an interface that evolves for unrelated transport reasons. A purpose-built interface keeps the public surface minimal and decoupled.IKmsConnector.Connect(host, port)communicates what an implementer is expected to do;IStreamFactory.CreateStream(EndPoint)does not.Internally we still reuse the existing machinery rather than reimplementing it:
KmsConnectorStreamFactoryadaptsIKmsConnectortoIStreamFactory, so the TLS wrapping (SslStreamFactory) and the KMS state machine are unchanged — we only swap the base stream source.Notes
SendKmsRequestandSendKmsRequestAsync, so the connector is a two-method interface. Consumers that only use async encryption APIs may throw from the sync method.CancellationToken, not a deadline; CSOT is not available. The connect-callback timeout case is therefore not supported and its prose test is skipped.Testing
Adds prose test 28 (
KMS Connect Callback) — cases 1-4 and 6; case 5 (timeout) is skipped as above. The tests exercise the plain and TLS HTTP proxies, the full auto-encryption pipeline, error propagation, and retry-after-network-error. They require real AWS KMS credentials and thekms_http_proxy.pyservers (ports 9004/9005) started by drivers-evergreen-tools, so they gate onKMS_MOCK_SERVERS_ENABLEDand run in the mocked-KMS-TLS Evergreen variants.