Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for customizing how KMS sockets are established for CSFLE/Queryable Encryption by introducing a kmsConnectCallback API (enabling HTTP CONNECT proxy tunneling) and wiring it through the CSFLE state machine, with accompanying unit + prose integration coverage.
Changes:
- Introduces
KMSConnectCallback(and related TLS/socket option types) in a newkms_optionsmodule and exports them from the public index. - Wires
kmsConnectCallbackthroughAutoEncrypter/ClientEncryptioninto theStateMachineKMS request path (including CSOT-aware behavior) and enforces mutual exclusivity withproxyOptions. - Adds unit tests and a new prose integration test (Prose test 28) validating callback invocation, proxy tunneling behavior, and timeout/error propagation.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/unit/client-side-encryption/state_machine.test.ts | Adds unit coverage for CSOT behavior and new kmsConnectCallback socket establishment + timeout semantics. |
| test/unit/client-side-encryption/client_encryption.test.ts | Adds unit coverage for proxyOptions vs kmsConnectCallback conflict validation in ClientEncryption. |
| test/unit/client-side-encryption/auto_encrypter.test.ts | Adds unit coverage for proxyOptions vs kmsConnectCallback conflict validation in AutoEncrypter. |
| test/mongodb_all.ts | Re-exports new kms_options types for tests. |
| test/integration/client-side-encryption/client_side_encryption.prose.28.kms_connect_callback.test.ts | Adds prose integration coverage for KMS connect callback through HTTP/HTTPS proxy. |
| src/index.ts | Publicly exports kms_options types and removes those type exports from state_machine. |
| src/client-side-encryption/state_machine.ts | Implements kmsConnectCallback support and revises CSOT timeout handling for KMS requests. |
| src/client-side-encryption/kms_options.ts | New module defining public KMS TLS/socket option types and KMSConnectCallback contract. |
| src/client-side-encryption/client_encryption.ts | Stores/validates kmsConnectCallback and passes it into StateMachine. |
| src/client-side-encryption/auto_encrypter.ts | Stores/validates kmsConnectCallback and passes it into StateMachine. |
| const timeoutMS = Number.isFinite(remainingTimeMS) ? remainingTimeMS : undefined; | ||
| kmsRequestTimeout = timeoutMS ? Timeout.expires(timeoutMS) : undefined; | ||
| await (kmsRequestTimeout | ||
| ? Promise.race([willResolveKmsRequest, kmsRequestTimeout]) |
There was a problem hiding this comment.
Note: .race() instead of .all() since the timeout is throwing on expire. This is fix for the pre-existing bug (test coverage also added).
| } | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
This is small refactoring, instead of adding one more Options type into StateMachine code I have extracted it into separate module kms_options.ts.
…k is going to be used
nbbeeken
left a comment
There was a problem hiding this comment.
apologies for the piecemeal review, noticed this on the second read. LGTM either way
There was a problem hiding this comment.
Note that this PR must wait for mongodb/specifications#1956 to merge before it can be fully reviewed and merged. (Will resolve this comment once this PR is unblocked).
| host: socketOptions.host, | ||
| port: socketOptions.port, | ||
| timeoutMS, | ||
| signal: controller.signal |
There was a problem hiding this comment.
Should the signal also include options.signal? That's an optional signal that is honored elsewhere (line 450), but we're ignore it here.
| const timeoutError = new MongoOperationTimeoutError('KMS request timed out'); | ||
| // Abort with the timeout error as the reason so the callback sees it on `signal.reason`. | ||
| controller.abort(timeoutError); | ||
| throw timeoutError; |
There was a problem hiding this comment.
If connectPromise eventually (after timeout) returns a socket, that socket will never be closed. Consider something like to close the socket:
connectPromise.then(
sock => { if (controller.signal.aborted) sock.destroy(); },
() => {} // nothing to cleanup
);
Description
Summary of Changes
Adds
kmsConnectCallbacktoAutoEncryptionOptionsandClientEncryptionOptions. When provided, the driver invokes the callback to establish the TCP connection to a KMS host instead of opening the socket itself, then wraps the returned socket with TLS using the KMS provider's configured TLS options. The primary use case is routing KMS requests through an HTTP proxy via the HTTP CONNECT method, which the existing SOCKS5proxyOptionsdoes not cover.Spec: mongodb/specifications#1956
Release Highlight
HTTP proxy support for KMS requests in CSFLE and Queryable Encryption
In-use encryption can now route KMS requests through an HTTP proxy. Set
kmsConnectCallbackon yourClientEncryptionor auto-encryption options to control how the driver connects to a KMS host. The callback receives the targethostandportand returns a connected socket (for example, a tunnelopened with HTTP CONNECT); the driver then performs the KMS TLS handshake over that socket using the provider's configured TLS options. This unblocks CSFLE and Queryable Encryption in environments that require an HTTP forward proxy for outbound KMS traffic, which the existing SOCKS5
proxyOptionsdoes not cover.Double check the following
npm run check:lint)type(NODE-xxxx)[!]: descriptionfeat(NODE-1234)!: rewriting everything in coffeescript