Skip to content

Commit

Permalink
feat(NODE-3924)!: read tls files async (#3776)
Browse files Browse the repository at this point in the history
Co-authored-by: Bailey Pearson <bailey.pearson@mongodb.com>
  • Loading branch information
W-A-James and baileympearson committed Aug 1, 2023
1 parent b16ef9e commit 68adaf1
Show file tree
Hide file tree
Showing 8 changed files with 155 additions and 66 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@
"check:oidc-azure": "mocha --config test/mocha_mongodb.json test/integration/auth/mongodb_oidc_azure.prose.test.ts",
"check:ocsp": "mocha --config test/manual/mocharc.json test/manual/ocsp_support.test.js",
"check:kerberos": "nyc mocha --config test/manual/mocharc.json test/manual/kerberos.test.ts",
"check:tls": "mocha --config test/manual/mocharc.json test/manual/tls_support.test.js",
"check:tls": "mocha --config test/manual/mocharc.json test/manual/tls_support.test.ts",
"check:ldap": "nyc mocha --config test/manual/mocharc.json test/manual/ldap.test.js",
"check:socks5": "mocha --config test/manual/mocharc.json test/manual/socks5.test.ts",
"check:csfle": "mocha --config test/mocha_mongodb.json test/integration/client-side-encryption",
Expand Down
11 changes: 2 additions & 9 deletions src/connection_string.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as dns from 'dns';
import * as fs from 'fs';
import ConnectionString from 'mongodb-connection-string-url';
import { URLSearchParams } from 'url';

Expand Down Expand Up @@ -1097,16 +1096,10 @@ export const OPTIONS = {
}
},
tlsCAFile: {
target: 'ca',
transform({ values: [value] }) {
return fs.readFileSync(String(value), { encoding: 'ascii' });
}
type: 'string'
},
tlsCertificateKeyFile: {
target: 'key',
transform({ values: [value] }) {
return fs.readFileSync(String(value), { encoding: 'ascii' });
}
type: 'string'
},
tlsCertificateKeyFilePassword: {
target: 'passphrase',
Expand Down
22 changes: 21 additions & 1 deletion src/mongo_client.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { promises as fs } from 'fs';
import type { TcpNetConnectOpts } from 'net';
import type { ConnectionOptions as TLSConnectionOptions, TLSSocketOptions } from 'tls';
import { promisify } from 'util';
Expand Down Expand Up @@ -433,6 +434,14 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> {

const options = this[kOptions];

if (options.tls) {
if (typeof options.tlsCAFile === 'string') {
options.ca ??= await fs.readFile(options.tlsCAFile, { encoding: 'utf8' });
}
if (typeof options.tlsCertificateKeyFile === 'string') {
options.key ??= await fs.readFile(options.tlsCertificateKeyFile, { encoding: 'utf8' });
}
}
if (typeof options.srvHost === 'string') {
const hosts = await resolveSRVRecord(options);

Expand Down Expand Up @@ -768,7 +777,7 @@ export interface MongoOptions
*
* ### Additional options:
*
* | nodejs native option | driver spec compliant option name | driver option type |
* | nodejs native option | driver spec equivalent option name | driver option type |
* |:----------------------|:----------------------------------------------|:-------------------|
* | `ca` | `tlsCAFile` | `string` |
* | `crl` | N/A | `string` |
Expand All @@ -784,9 +793,20 @@ export interface MongoOptions
* If `tlsInsecure` is set to `false`, then it will set the node native options `checkServerIdentity`
* to a no-op and `rejectUnauthorized` to the inverse value of `tlsAllowInvalidCertificates`. If
* `tlsAllowInvalidCertificates` is not set, then `rejectUnauthorized` will be set to `true`.
*
* ### Note on `tlsCAFile` and `tlsCertificateKeyFile`
*
* The files specified by the paths passed in to the `tlsCAFile` and `tlsCertificateKeyFile` fields
* are read lazily on the first call to `MongoClient.connect`. Once these files have been read and
* the `ca` and `key` fields are populated, they will not be read again on subsequent calls to
* `MongoClient.connect`. As a result, until the first call to `MongoClient.connect`, the `ca`
* and `key` fields will be undefined.
*/
tls: boolean;

tlsCAFile?: string;
tlsCertificateKeyFile?: string;

/** @internal */
[featureFlag: symbol]: any;

Expand Down
3 changes: 2 additions & 1 deletion test/manual/mocharc.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@
"require": "ts-node/register",
"reporter": "test/tools/reporter/mongodb_reporter.js",
"failZero": true,
"color": true
"color": true,
"timeout": 10000
}
44 changes: 0 additions & 44 deletions test/manual/tls_support.test.js

This file was deleted.

120 changes: 120 additions & 0 deletions test/manual/tls_support.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import { expect } from 'chai';
import { promises as fs } from 'fs';

import { LEGACY_HELLO_COMMAND, MongoClient, type MongoClientOptions } from '../mongodb';

const REQUIRED_ENV = ['MONGODB_URI', 'SSL_KEY_FILE', 'SSL_CA_FILE'];

describe('TLS Support', function () {
for (const key of REQUIRED_ENV) {
if (process.env[key] == null) {
throw new Error(`skipping SSL tests, ${key} environment variable is not defined`);
}
}

const CONNECTION_STRING = process.env.MONGODB_URI as string;
const TLS_CERT_KEY_FILE = process.env.SSL_KEY_FILE as string;
const TLS_CA_FILE = process.env.SSL_CA_FILE as string;
const tlsSettings = {
tls: true,
tlsCertificateKeyFile: TLS_CERT_KEY_FILE,
tlsCAFile: TLS_CA_FILE
};

it(
'should connect with tls via client options',
makeConnectionTest(CONNECTION_STRING, tlsSettings)
);

it(
'should connect with tls via url options',
makeConnectionTest(
`${CONNECTION_STRING}?${Object.keys(tlsSettings)
.map(key => `${key}=${tlsSettings[key]}`)
.join('&')}`
)
);

context('when tls filepaths are provided', () => {
let client: MongoClient;
afterEach(async () => {
if (client) await client.close();
});

context('when tls filepaths have length > 0', () => {
beforeEach(async () => {
client = new MongoClient(CONNECTION_STRING, tlsSettings);
});

it('should read in files async at connect time', async () => {
expect(client.options).property('tlsCAFile', TLS_CA_FILE);
expect(client.options).property('tlsCertificateKeyFile', TLS_CERT_KEY_FILE);
expect(client.options).not.have.property('ca');
expect(client.options).not.have.property('key');

await client.connect();

expect(client.options).property('ca').to.exist;
expect(client.options).property('key').to.exist;
});

context('when client has been opened and closed more than once', function () {
it('should only read files once', async () => {
await client.connect();
await client.close();

const caFileAccessTime = (await fs.stat(TLS_CA_FILE)).atime;
const certKeyFileAccessTime = (await fs.stat(TLS_CERT_KEY_FILE)).atime;

await client.connect();

expect((await fs.stat(TLS_CA_FILE)).atime).to.deep.equal(caFileAccessTime);
expect((await fs.stat(TLS_CERT_KEY_FILE)).atime).to.deep.equal(certKeyFileAccessTime);
});
});
});

context('when tlsCAFile has length === 0', () => {
beforeEach(() => {
client = new MongoClient(CONNECTION_STRING, {
tls: true,
tlsCAFile: '',
tlsCertificateKeyFile: TLS_CERT_KEY_FILE
});
});

it('should throw an error at connect time', async () => {
const err = await client.connect().catch(e => e);

expect(err).to.be.instanceof(Error);
});
});

context('when tlsCertificateKeyFile has length === 0', () => {
beforeEach(() => {
client = new MongoClient(CONNECTION_STRING, {
tls: true,
tlsCAFile: TLS_CA_FILE,
tlsCertificateKeyFile: ''
});
});

it('should throw an error at connect time', async () => {
const err = await client.connect().catch(e => e);

expect(err).to.be.instanceof(Error);
});
});
});
});

function makeConnectionTest(connectionString: string, clientOptions?: MongoClientOptions) {
return async function () {
const client = new MongoClient(connectionString, clientOptions);

await client.connect();
await client.db('admin').command({ [LEGACY_HELLO_COMMAND]: 1 });
await client.db('test').collection('test').findOne({});
return await client.close();
};
}
4 changes: 2 additions & 2 deletions test/tools/uri_spec_runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,13 +314,13 @@ export function executeUriValidationTest(
.equal(optionValue);
break;
case 'tlsCertificateKeyFile':
expectedProp = 'key';
expectedProp = 'tlsCertificateKeyFile';
expect(options, `${errorMessage} ${optionKey} -> ${expectedProp}`)
.to.have.property(expectedProp)
.equal(optionValue);
break;
case 'tlsCAFile':
expectedProp = 'ca';
expectedProp = 'tlsCAFile';
expect(options, `${errorMessage} ${optionKey} -> ${expectedProp}`)
.to.have.property(expectedProp)
.equal(optionValue);
Expand Down
15 changes: 7 additions & 8 deletions test/unit/mongo_client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe('MongoOptions', function () {
*
* ### Additional options:
*
* | nodejs native option | driver spec compliant option name | driver option type |
* | nodejs native option | driver spec equivalent option name | driver option type |
* |:----------------------|:----------------------------------------------|:-------------------|
* | `ca` | `tlsCAFile` | `string` |
* | `crl` | N/A | `string` |
Expand All @@ -55,12 +55,11 @@ describe('MongoOptions', function () {
* | see note below | `tlsInsecure` | `boolean` |
*
*/
expect(options).to.not.have.property('tlsCertificateKeyFile');
expect(options).to.not.have.property('tlsCAFile');
expect(options).to.not.have.property('tlsCertificateKeyFilePassword');
expect(options).has.property('ca', '');
expect(options).has.property('key');
expect(options.key).has.length(0);
expect(options).to.not.have.property('key');
expect(options).to.not.have.property('ca');
expect(options).to.have.property('tlsCertificateKeyFile', filename);
expect(options).to.have.property('tlsCAFile', filename);
expect(options).has.property('passphrase', 'tlsCertificateKeyFilePassword');
expect(options).has.property('tls', true);
});
Expand Down Expand Up @@ -394,10 +393,10 @@ describe('MongoOptions', function () {
const optsFromObject = parseOptions('mongodb://localhost/', {
tlsCertificateKeyFile: 'testCertKey.pem'
});
expect(optsFromObject).to.have.property('key', 'cert key');
expect(optsFromObject).to.have.property('tlsCertificateKeyFile', 'testCertKey.pem');

const optsFromUri = parseOptions('mongodb://localhost?tlsCertificateKeyFile=testCertKey.pem');
expect(optsFromUri).to.have.property('key', 'cert key');
expect(optsFromUri).to.have.property('tlsCertificateKeyFile', 'testCertKey.pem');
});
});

Expand Down

0 comments on commit 68adaf1

Please sign in to comment.