From db5b26325d1fb6da2d72dd7bc8cbf89b9d77bdfb Mon Sep 17 00:00:00 2001 From: Daria Pardue <81593090+dariakp@users.noreply.github.com> Date: Fri, 10 Sep 2021 16:36:00 -0400 Subject: [PATCH 1/5] fix(NODE-3591): tlsCertificateKeyFile options does not default cert --- src/connection_string.ts | 4 +++ src/mongo_client.ts | 2 +- test/unit/mongo_client_options.test.js | 41 ++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index 0ed6cbd3de..c98228fdce 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -335,6 +335,10 @@ export function parseOptions( allOptions.set(key, values); } + if (allOptions.has('tlsCertificateKeyFile') && !allOptions.has('tlsCertificateFile')) { + allOptions.set('tlsCertificateFile', allOptions.get('tlsCertificateKeyFile')); + } + const unsupportedOptions = setDifference( allKeys, Array.from(Object.keys(OPTIONS)).map(s => s.toLowerCase()) diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 19192aa020..b57563f51d 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -112,7 +112,7 @@ export interface MongoClientOptions extends BSONSerializeOptions, SupportedNodeC ssl?: boolean; /** Specifies the location of a local TLS Certificate */ tlsCertificateFile?: string; - /** Specifies the location of a local .pem file that contains either the client’s TLS/SSL certificate or the client’s TLS/SSL certificate and key. */ + /** Specifies the location of a local .pem file that contains either the client's TLS/SSL certificate and key or only the client's TLS/SSL key when tlsCertificateFile is used to provide the certificate. */ tlsCertificateKeyFile?: string; /** Specifies the password to de-crypt the tlsCertificateKeyFile. */ tlsCertificateKeyFilePassword?: string; diff --git a/test/unit/mongo_client_options.test.js b/test/unit/mongo_client_options.test.js index 083908f291..dab418c3ec 100644 --- a/test/unit/mongo_client_options.test.js +++ b/test/unit/mongo_client_options.test.js @@ -366,6 +366,47 @@ describe('MongoOptions', function () { expect(optionsUndefined.checkServerIdentity).to.equal(undefined); }); + describe('[tls certificate handling]', () => { + before(() => { + fs.writeFileSync('testCertKey.pem', 'cert key'); + fs.writeFileSync('testKey.pem', 'test key'); + fs.writeFileSync('testCert.pem', 'test cert'); + }); + + after(() => { + fs.unlinkSync('testCertKey.pem'); + fs.unlinkSync('testKey.pem'); + fs.unlinkSync('testCert.pem'); + }); + + it('correctly sets the cert and key if only tlsCertificateKeyFile is provided', function () { + const optsFromObject = parseOptions('mongodb://localhost/', { + tlsCertificateKeyFile: 'testCertKey.pem' + }); + expect(optsFromObject).to.have.property('cert', 'cert key'); + expect(optsFromObject).to.have.property('key', 'cert key'); + + const optsFromUri = parseOptions('mongodb://localhost?tlsCertificateKeyFile=testCertKey.pem'); + expect(optsFromUri).to.have.property('cert', 'cert key'); + expect(optsFromUri).to.have.property('key', 'cert key'); + }); + + it('correctly sets the cert and key if both tlsCertificateKeyFile and tlsCertificateFile is provided', function () { + const optsFromObject = parseOptions('mongodb://localhost/', { + tlsCertificateKeyFile: 'testKey.pem', + tlsCertificateFile: 'testCert.pem' + }); + expect(optsFromObject).to.have.property('cert', 'test cert'); + expect(optsFromObject).to.have.property('key', 'test key'); + + const optsFromUri = parseOptions( + 'mongodb://localhost?tlsCertificateKeyFile=testKey.pem&tlsCertificateFile=testCert.pem' + ); + expect(optsFromUri).to.have.property('cert', 'test cert'); + expect(optsFromUri).to.have.property('key', 'test key'); + }); + }); + it('transforms tlsInsecure correctly', function () { const optionsTrue = parseOptions('mongodb://localhost/', { tlsInsecure: true From 98464d9b2441625e13abef8ca8a72f0c896b80db Mon Sep 17 00:00:00 2001 From: Daria Pardue <81593090+dariakp@users.noreply.github.com> Date: Fri, 10 Sep 2021 17:32:01 -0400 Subject: [PATCH 2/5] fix: multiple tls/ssl parameter validation --- src/connection_string.ts | 23 +++++++--------- test/unit/mongo_client_options.test.js | 36 ++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 13 deletions(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index c98228fdce..c188e2df85 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -176,7 +176,7 @@ function getBoolean(name: string, value: unknown): boolean { const valueString = String(value).toLowerCase(); if (TRUTHS.has(valueString)) return true; if (FALSEHOODS.has(valueString)) return false; - throw new MongoParseError(`For ${name} Expected stringified boolean value, got: ${value}`); + throw new MongoParseError(`Expected ${name} to be stringified boolean value, got: ${value}`); } function getInt(name: string, value: unknown): number { @@ -339,6 +339,15 @@ export function parseOptions( allOptions.set('tlsCertificateFile', allOptions.get('tlsCertificateKeyFile')); } + if (allOptions.has('tls') || allOptions.has('ssl')) { + const tlsAndSslOpts = (allOptions.get('tls') || []) + .concat(allOptions.get('ssl') || []) + .map(getBoolean.bind(null, 'tls/ssl')); + if (new Set(tlsAndSslOpts).size !== 1) { + throw new MongoParseError('All values of tls/ssl must be the same.'); + } + } + const unsupportedOptions = setDifference( allKeys, Array.from(Object.keys(OPTIONS)).map(s => s.toLowerCase()) @@ -388,18 +397,6 @@ export function parseOptions( mongoOptions.dbName = 'test'; } - if (allOptions.has('tls')) { - if (new Set(allOptions.get('tls')?.map(getBoolean)).size !== 1) { - throw new MongoParseError('All values of tls must be the same.'); - } - } - - if (allOptions.has('ssl')) { - if (new Set(allOptions.get('ssl')?.map(getBoolean)).size !== 1) { - throw new MongoParseError('All values of ssl must be the same.'); - } - } - checkTLSOptions(mongoOptions); if (options.promiseLibrary) PromiseProvider.set(options.promiseLibrary); diff --git a/test/unit/mongo_client_options.test.js b/test/unit/mongo_client_options.test.js index dab418c3ec..0afbdc886f 100644 --- a/test/unit/mongo_client_options.test.js +++ b/test/unit/mongo_client_options.test.js @@ -407,6 +407,42 @@ describe('MongoOptions', function () { }); }); + it('throws an error if multiple tls parameters are not all set to the same value', () => { + expect(() => parseOptions('mongodb://localhost?tls=true&tls=false')).to.throw( + 'All values of tls/ssl must be the same.' + ); + }); + + it('throws an error if multiple ssl parameters are not all set to the same value', () => { + expect(() => parseOptions('mongodb://localhost?ssl=true&ssl=false')).to.throw( + 'All values of tls/ssl must be the same.' + ); + }); + + it('throws an error if tls and ssl parameters are not all set to the same value', () => { + expect(() => parseOptions('mongodb://localhost?tls=true&ssl=false')).to.throw( + 'All values of tls/ssl must be the same.' + ); + expect(() => parseOptions('mongodb://localhost?tls=false&ssl=true')).to.throw( + 'All values of tls/ssl must be the same.' + ); + }); + + it('correctly sets tls if multiple tls parameters are all set to the same value', () => { + expect(parseOptions('mongodb://localhost?tls=true&tls=true')).to.have.property('tls', true); + expect(parseOptions('mongodb://localhost?tls=false&tls=false')).to.have.property('tls', false); + }); + + it('correctly sets tls if multiple ssl parameters are all set to the same value', () => { + expect(parseOptions('mongodb://localhost?ssl=true&ssl=true')).to.have.property('tls', true); + expect(parseOptions('mongodb://localhost?ssl=false&ssl=false')).to.have.property('tls', false); + }); + + it('correctly sets tls if tls and ssl parameters are all set to the same value', () => { + expect(parseOptions('mongodb://localhost?ssl=true&tls=true')).to.have.property('tls', true); + expect(parseOptions('mongodb://localhost?ssl=false&tls=false')).to.have.property('tls', false); + }); + it('transforms tlsInsecure correctly', function () { const optionsTrue = parseOptions('mongodb://localhost/', { tlsInsecure: true From 3ba104aabf00169132728f4703db886ab3eb1a14 Mon Sep 17 00:00:00 2001 From: Daria Pardue <81593090+dariakp@users.noreply.github.com> Date: Fri, 10 Sep 2021 17:34:08 -0400 Subject: [PATCH 3/5] test: single tls/ssl options tests --- test/unit/mongo_client_options.test.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/unit/mongo_client_options.test.js b/test/unit/mongo_client_options.test.js index 0afbdc886f..b1ff2f1494 100644 --- a/test/unit/mongo_client_options.test.js +++ b/test/unit/mongo_client_options.test.js @@ -245,6 +245,18 @@ describe('MongoOptions', function () { expect(options).has.property('tls', false); }); + it('ssl= can be used to set tls=true', function () { + const options = parseOptions('mongodb+srv://server.example.com/?ssl=true'); + expect(options).has.property('srvHost', 'server.example.com'); + expect(options).has.property('tls', true); + }); + + it('tls= can be used to set tls=true', function () { + const options = parseOptions('mongodb+srv://server.example.com/?tls=true'); + expect(options).has.property('srvHost', 'server.example.com'); + expect(options).has.property('tls', true); + }); + it('supports ReadPreference option in url', function () { const options = parseOptions('mongodb://localhost/?readPreference=nearest'); expect(options.readPreference).to.be.an.instanceof(ReadPreference); From 450ad493dd74dfbb4b2fdf27f3627aabb53f2bc6 Mon Sep 17 00:00:00 2001 From: Daria Pardue <81593090+dariakp@users.noreply.github.com> Date: Fri, 10 Sep 2021 17:52:45 -0400 Subject: [PATCH 4/5] test: update expected error message --- test/unit/core/connection_string.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/core/connection_string.test.js b/test/unit/core/connection_string.test.js index 41f0f227ba..fd8bc17344 100644 --- a/test/unit/core/connection_string.test.js +++ b/test/unit/core/connection_string.test.js @@ -136,7 +136,7 @@ describe('Connection String', function () { it('should validate non-equal tls values', function () { expect(() => parseOptions('mongodb://localhost/?tls=true&tls=false')).to.throw( MongoParseError, - 'All values of tls must be the same.' + 'All values of tls/ssl must be the same.' ); }); }); From 8daa5ee8dcd5b2444419f441983027dbedde949c Mon Sep 17 00:00:00 2001 From: Daria Pardue <81593090+dariakp@users.noreply.github.com> Date: Fri, 10 Sep 2021 17:54:28 -0400 Subject: [PATCH 5/5] test: update all options test to pass in valid input --- test/unit/mongo_client_options.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/mongo_client_options.test.js b/test/unit/mongo_client_options.test.js index b1ff2f1494..47a7785c95 100644 --- a/test/unit/mongo_client_options.test.js +++ b/test/unit/mongo_client_options.test.js @@ -128,7 +128,7 @@ describe('MongoOptions', function () { ssl: true, sslPass: 'pass', sslValidate: true, - tls: false, + tls: true, tlsAllowInvalidCertificates: true, tlsAllowInvalidHostnames: true, tlsCertificateKeyFilePassword: 'tls-pass',