Skip to content

feat(NODE-4179): allow secureContext in KMS TLS options #4578

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+224 −63
Open

feat(NODE-4179): allow secureContext in KMS TLS options #4578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
68309a1
feat(NODE-4179): allow secureContext in KMS TLS options
durran Jul 8, 2025
6617c9e
chore: comments
durran Jul 10, 2025
a833c73
chore: add note for API docs
durran Jul 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions src/client-side-encryption/state_machine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,12 @@ declare module 'mongodb-client-encryption' {
* - tlsInsecure
*
* These options are not included in the type, and are ignored if provided.
*
* Note that if a secureContext option is provided, all other TLS options will be ignored.
*/
export type ClientEncryptionTlsOptions = Pick<
MongoClientOptions,
'tlsCAFile' | 'tlsCertificateKeyFile' | 'tlsCertificateKeyFilePassword'
'tlsCAFile' | 'tlsCertificateKeyFile' | 'tlsCertificateKeyFilePassword' | 'secureContext'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't a blocker for us, but is there a reason not to accept other options that the driver passes through for regular connections (i.e. what's listed in LEGAL_TLS_SOCKET_OPTIONS)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scope of the ticket was for secureContext. There's no reason not to allow the others but I'd open a new ticket for that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@addaleax does that work for you?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, although I'll say that the intention when the ticket was first created was absolutely not just scoped to secureContext only. And I do feel like consistency between the KMS TLS handling and the main driver TLS handling should be an implicit goal in some form here

Copy link
Contributor

@dariakp dariakp Jul 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@addaleax fully agree - in retrospect we (the node team) should have been more diligent about stating our implicit assumptions in the ticket requirements; it's also a good reminder for us to always follow up with our stakeholders after refinement, whenever possible, to make sure we are all aligned; sorry about this, we'll try to do better next time :)

@durran do you want to go ahead and file that follow up?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created https://jira.mongodb.org/browse/NODE-7032

>;

/** @public */
Expand Down Expand Up @@ -526,15 +528,20 @@ export class StateMachine {
tlsOptions: ClientEncryptionTlsOptions,
options: tls.ConnectionOptions
): Promise<void> {
if (tlsOptions.tlsCertificateKeyFile) {
const cert = await fs.readFile(tlsOptions.tlsCertificateKeyFile);
options.cert = options.key = cert;
}
if (tlsOptions.tlsCAFile) {
options.ca = await fs.readFile(tlsOptions.tlsCAFile);
}
if (tlsOptions.tlsCertificateKeyFilePassword) {
options.passphrase = tlsOptions.tlsCertificateKeyFilePassword;
// If a secureContext is provided, it takes precedence over the other options.
if (tlsOptions.secureContext) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we be making an analogous change in the driver in, e.g., V7? we don't currently do any overrides for it in the main client logic (see _connect())

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that would make sense to me here for sure. I'd also consider the possibility of taking either an existing SecureContext object or the options object needed to create one via tls.createSecureContext() – keeping the overall MongoClient options object (including AutoEncryptionOptions) mostly (de)serializable through standard formats like JSON is generally a helpful best practice

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@durran Were you able to verify the behavior for what happens when we pass both secureContext and the standalone options to Node in these two cases: (1) secureContext has valid auth, standalone options have invalid auth and (2) secureContext has invalid auth, standalone options have valid auth? If in both cases the secureContext is used and the other options are ignored, we should be able to make the same change to the driver option handling right now for consistency. Otherwise please file the V7 ticket to do so.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll file a ticket for V7. In my testing when providing both a secure context and other TLS options, the connection just hangs and then times out (https://evergreen.mongodb.com/test_log/mongo_node_driver_next_rhel80_large_node_latest_test_tls_support_latest_patch_bff57ed87b236e7840a700989f3287f1be856fa9_68701e519bc80f00076e5617_25_07_10_20_11_06/0?test_name=b8c798e2ba619290e1fe4c562783bdc7#L0) so it doesn't seem like there's any real validation.

options.secureContext = tlsOptions.secureContext;
} else {
if (tlsOptions.tlsCertificateKeyFile) {
const cert = await fs.readFile(tlsOptions.tlsCertificateKeyFile);
options.cert = options.key = cert;
}
if (tlsOptions.tlsCAFile) {
options.ca = await fs.readFile(tlsOptions.tlsCAFile);
}
if (tlsOptions.tlsCertificateKeyFilePassword) {
options.passphrase = tlsOptions.tlsCertificateKeyFilePassword;
}
}
}

Expand Down
117 changes: 66 additions & 51 deletions ...tion/client_side_encryption.prose.test.js → ...tion/client_side_encryption.prose.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,24 @@
'use strict';
const BSON = require('bson');
const { expect } = require('chai');
const fs = require('fs');
const path = require('path');

const { dropCollection, APMEventCollector } = require('../shared');

const { EJSON } = BSON;
const { LEGACY_HELLO_COMMAND, MongoCryptError, MongoRuntimeError } = require('../../mongodb');
const { MongoServerError, MongoServerSelectionError, MongoClient } = require('../../mongodb');
const { getEncryptExtraOptions } = require('../../tools/utils');

const {
externalSchema
} = require('../../spec/client-side-encryption/external/external-schema.json');
/* eslint-disable no-restricted-modules */
const { ClientEncryption } = require('../../../src/client-side-encryption/client_encryption');
const { getCSFLEKMSProviders } = require('../../csfle-kms-providers');
const { AlpineTestConfiguration } = require('../../tools/runner/config');

const getKmsProviders = (localKey, kmipEndpoint, azureEndpoint, gcpEndpoint) => {
import { BSON, EJSON } from 'bson';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note there is no related change to this feature in this test. I needed to convert it to TS because it started breaking everything at module load with:

[2025/07/08 22:03:11.087]  Exception during run: ReferenceError: require is not defined in ES module scope, you can use import instead
[2025/07/08 22:03:11.087]     at file:///data/mci/f715752cc296c929fd848c3df13d321a/src/test/integration/client-side-encryption/client_side_encryption.prose.test.js:2:14
[2025/07/08 22:03:11.087]     at ModuleJob.run (node:internal/modules/esm/module_job:329:25)
[2025/07/08 22:03:11.087]     at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:644:26)
[2025/07/08 22:03:11.087]     at async formattedImport (/data/mci/f715752cc296c929fd848c3df13d321a/src/node_modules/mocha/lib/nodejs/esm-utils.js:9:14)
[2025/07/08 22:03:11.087]     at async Object.exports.requireOrImport (/data/mci/f715752cc296c929fd848c3df13d321a/src/node_modules/mocha/lib/nodejs/esm-utils.js:42:28)
[2025/07/08 22:03:11.087]     at async Object.exports.loadFilesAsync (/data/mci/f715752cc296c929fd848c3df13d321a/src/node_modules/mocha/lib/nodejs/esm-utils.js:100:20)
[2025/07/08 22:03:11.087]     at async singleRun (/data/mci/f715752cc296c929fd848c3df13d321a/src/node_modules/mocha/lib/cli/run-helpers.js:162:3)
[2025/07/08 22:03:11.087]     at async Object.exports.handler (/data/mci/f715752cc296c929fd848c3df13d321a/src/node_modules/mocha/lib/cli/run.js:375:5)

Copy link
Contributor

@dariakp dariakp Jul 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind the conversion, but what caused that? It would be good to at least understand where this is coming from and why now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All I could confirm is that my changes in driver.test.ts somehow caused this to happen, even though it doesn't reference the prose test at all. It happens on all Node versions. I didn't want to spend too much time getting down to the exact reason and just figured converting it now would be faster.

import { expect } from 'chai';
import * as fs from 'fs/promises';
import * as path from 'path';

// eslint-disable-next-line @typescript-eslint/no-restricted-imports
import { ClientEncryption } from '../../../src/client-side-encryption/client_encryption';
import { getCSFLEKMSProviders } from '../../csfle-kms-providers';
import {
LEGACY_HELLO_COMMAND,
MongoClient,
MongoCryptError,
MongoRuntimeError,
MongoServerError,
MongoServerSelectionError
} from '../../mongodb';
import { AlpineTestConfiguration } from '../../tools/runner/config';
import { getEncryptExtraOptions } from '../../tools/utils';
import { APMEventCollector, dropCollection } from '../shared';

export const getKmsProviders = (localKey, kmipEndpoint, azureEndpoint, gcpEndpoint) => {
const result = getCSFLEKMSProviders();
if (localKey) {
result.local = { key: localKey };
Expand All @@ -39,6 +38,7 @@ const getKmsProviders = (localKey, kmipEndpoint, azureEndpoint, gcpEndpoint) =>
return result;
};

// eslint-disable-next-line @typescript-eslint/no-empty-function
const noop = () => {};
const metadata = {
requires: {
Expand All @@ -55,6 +55,24 @@ const eeMetadata = {
}
};

async function loadExternal(file) {
return EJSON.parse(
await fs.readFile(
path.resolve(__dirname, '../../spec/client-side-encryption/external', file),
'utf8'
)
);
}

async function loadLimits(file) {
return EJSON.parse(
await fs.readFile(
path.resolve(__dirname, '../../spec/client-side-encryption/limits', file),
'utf8'
)
);
}

// Tests for the ClientEncryption type are not included as part of the YAML tests.

// In the prose tests LOCAL_MASTERKEY refers to the following base64:
Expand All @@ -63,6 +81,9 @@ const eeMetadata = {

// Mng0NCt4ZHVUYUJCa1kxNkVyNUR1QURhZ2h2UzR2d2RrZzh0cFBwM3R6NmdWMDFBMUN3YkQ5aXRRMkhGRGdQV09wOGVNYUMxT2k3NjZKelhaQmRCZGJkTXVyZG9uSjFk
describe('Client Side Encryption Prose Tests', metadata, function () {
let externalKey;
let externalSchema;

const dataDbName = 'db';
const dataCollName = 'coll';
const dataNamespace = `${dataDbName}.${dataCollName}`;
Expand All @@ -75,6 +96,11 @@ describe('Client Side Encryption Prose Tests', metadata, function () {
'base64'
);

before(async function () {
externalKey = await loadExternal('external-key.json');
externalSchema = await loadExternal('external-schema.json');
});

describe('Data key and double encryption', function () {
// Data key and double encryption
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down Expand Up @@ -350,18 +376,8 @@ describe('Client Side Encryption Prose Tests', metadata, function () {
// and confirming that the externalClient is firing off keyVault requests during
// encrypted operations
describe('External Key Vault Test', function () {
function loadExternal(file) {
return EJSON.parse(
fs.readFileSync(path.resolve(__dirname, '../../spec/client-side-encryption/external', file))
);
}

const externalKey = loadExternal('external-key.json');
const externalSchema = loadExternal('external-schema.json');

beforeEach(function () {
beforeEach(async function () {
this.client = this.configuration.newClient();

// 1. Create a MongoClient without encryption enabled (referred to as ``client``).
return (
this.client
Expand Down Expand Up @@ -551,15 +567,15 @@ describe('Client Side Encryption Prose Tests', metadata, function () {
});

describe('BSON size limits and batch splitting', function () {
function loadLimits(file) {
return EJSON.parse(
fs.readFileSync(path.resolve(__dirname, '../../spec/client-side-encryption/limits', file))
);
}

const limitsSchema = loadLimits('limits-schema.json');
const limitsKey = loadLimits('limits-key.json');
const limitsDoc = loadLimits('limits-doc.json');
let limitsSchema;
let limitsKey;
let limitsDoc;

before(async function () {
limitsSchema = await loadLimits('limits-schema.json');
limitsKey = await loadLimits('limits-key.json');
limitsDoc = await loadLimits('limits-doc.json');
});

let hasRunFirstTimeSetup = false;

Expand Down Expand Up @@ -826,9 +842,9 @@ describe('Client Side Encryption Prose Tests', metadata, function () {

describe('Corpus Test', function () {
it('runs in a separate suite', () => {
expect(() =>
fs.statSync(path.resolve(__dirname, './client_side_encryption.prose.06.corpus.test.ts'))
).not.to.throw();
expect(async () => {
await fs.stat(path.resolve(__dirname, './client_side_encryption.prose.06.corpus.test.ts'));
}).not.to.throw();
});
});

Expand Down Expand Up @@ -1691,6 +1707,7 @@ describe('Client Side Encryption Prose Tests', metadata, function () {
context(
'Case 5: `tlsDisableOCSPEndpointCheck` is permitted',
metadata,
// eslint-disable-next-line @typescript-eslint/no-empty-function
function () {}
).skipReason = 'TODO(NODE-4840): Node does not support any OCSP options';

Expand Down Expand Up @@ -1911,12 +1928,12 @@ describe('Client Side Encryption Prose Tests', metadata, function () {
beforeEach(async function () {
// Load the file encryptedFields.json as encryptedFields.
encryptedFields = EJSON.parse(
await fs.promises.readFile(path.join(data, 'encryptedFields.json')),
await fs.readFile(path.join(data, 'encryptedFields.json'), 'utf8'),
{ relaxed: false }
);
// Load the file key1-document.json as key1Document.
key1Document = EJSON.parse(
await fs.promises.readFile(path.join(data, 'keys', 'key1-document.json')),
await fs.readFile(path.join(data, 'keys', 'key1-document.json'), 'utf8'),
{ relaxed: false }
);
// Read the "_id" field of key1Document as key1ID.
Expand Down Expand Up @@ -2312,15 +2329,13 @@ describe('Client Side Encryption Prose Tests', metadata, function () {
kmip: {},
local: undefined
};
/** @type {import('../../mongodb').MongoClient} */
let client1;
/** @type {import('../../mongodb').MongoClient} */
let client2;

describe('Case 1: Rewrap with separate ClientEncryption', function () {
/**
* Run the following test case for each pair of KMS providers (referred to as ``srcProvider`` and ``dstProvider``).
* Include pairs where ``srcProvider`` equals ``dstProvider``.
* Run the following test case for each pair of KMS providers (referred to as `srcProvider` and `dstProvider`).
* Include pairs where `srcProvider` equals `dstProvider`.
*/
function* generateTestCombinations() {
const providers = Object.keys(masterKeys);
Expand Down
Loading