diff --git a/src/connection_string.ts b/src/connection_string.ts index 2bf1f6dd193..f0b497ddf40 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -34,11 +34,11 @@ import { ReadPreference, type ReadPreferenceMode } from './read_preference'; import { ServerMonitoringMode } from './sdam/monitor'; import type { TagSet } from './sdam/server_description'; import { + checkParentDomainMatch, DEFAULT_PK_FACTORY, emitWarning, HostAddress, isRecord, - matchesParentDomain, parseInteger, setDifference, squashError @@ -64,11 +64,6 @@ export async function resolveSRVRecord(options: MongoOptions): Promise HostAddress.fromString(`${r.name}:${r.port ?? 27017}`)); diff --git a/src/sdam/srv_polling.ts b/src/sdam/srv_polling.ts index 39d393b7830..c95c386cfa7 100644 --- a/src/sdam/srv_polling.ts +++ b/src/sdam/srv_polling.ts @@ -3,7 +3,7 @@ import { clearTimeout, setTimeout } from 'timers'; import { MongoRuntimeError } from '../error'; import { TypedEventEmitter } from '../mongo_types'; -import { HostAddress, matchesParentDomain, squashError } from '../utils'; +import { checkParentDomainMatch, HostAddress, squashError } from '../utils'; /** * @internal @@ -127,8 +127,11 @@ export class SrvPoller extends TypedEventEmitter { const finalAddresses: dns.SrvRecord[] = []; for (const record of srvRecords) { - if (matchesParentDomain(record.name, this.srvHost)) { + try { + checkParentDomainMatch(record.name, this.srvHost); finalAddresses.push(record); + } catch (error) { + squashError(error); } } diff --git a/src/utils.ts b/src/utils.ts index 5ad754c9321..6d5a67d3ab9 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -18,6 +18,7 @@ import type { FindCursor } from './cursor/find_cursor'; import type { Db } from './db'; import { type AnyError, + MongoAPIError, MongoCompatibilityError, MongoInvalidArgumentError, MongoNetworkTimeoutError, @@ -1142,29 +1143,47 @@ export function parseUnsignedInteger(value: unknown): number | null { } /** - * Determines whether a provided address matches the provided parent domain. + * This function throws a MongoAPIError in the event that either of the following is true: + * * If the provided address domain does not match the provided parent domain + * * If the parent domain contains less than three `.` separated parts and the provided address does not contain at least one more domain level than its parent * * If a DNS server were to become compromised SRV records would still need to * advertise addresses that are under the same domain as the srvHost. * * @param address - The address to check against a domain * @param srvHost - The domain to check the provided address against - * @returns Whether the provided address matches the parent domain + * @returns void */ -export function matchesParentDomain(address: string, srvHost: string): boolean { +export function checkParentDomainMatch(address: string, srvHost: string): void { // Remove trailing dot if exists on either the resolved address or the srv hostname const normalizedAddress = address.endsWith('.') ? address.slice(0, address.length - 1) : address; const normalizedSrvHost = srvHost.endsWith('.') ? srvHost.slice(0, srvHost.length - 1) : srvHost; const allCharacterBeforeFirstDot = /^.*?\./; + const srvIsLessThanThreeParts = normalizedSrvHost.split('.').length < 3; // Remove all characters before first dot // Add leading dot back to string so // an srvHostDomain = '.trusted.site' // will not satisfy an addressDomain that endsWith '.fake-trusted.site' const addressDomain = `.${normalizedAddress.replace(allCharacterBeforeFirstDot, '')}`; - const srvHostDomain = `.${normalizedSrvHost.replace(allCharacterBeforeFirstDot, '')}`; + let srvHostDomain = srvIsLessThanThreeParts + ? normalizedSrvHost + : `.${normalizedSrvHost.replace(allCharacterBeforeFirstDot, '')}`; - return addressDomain.endsWith(srvHostDomain); + if (!srvHostDomain.startsWith('.')) { + srvHostDomain = '.' + srvHostDomain; + } + if ( + srvIsLessThanThreeParts && + normalizedAddress.split('.').length <= normalizedSrvHost.split('.').length + ) { + throw new MongoAPIError( + 'Server record does not have at least one more domain level than parent URI' + ); + } + if (!addressDomain.endsWith(srvHostDomain)) { + throw new MongoAPIError('Server record does not share hostname with parent URI'); + } } interface RequestOptions { diff --git a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts new file mode 100644 index 00000000000..a4fba77f068 --- /dev/null +++ b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts @@ -0,0 +1,293 @@ +import { expect } from 'chai'; +import * as dns from 'dns'; +import * as sinon from 'sinon'; + +import { MongoAPIError, Server, ServerDescription, Topology } from '../../mongodb'; +import { topologyWithPlaceholderClient } from '../../tools/utils'; + +describe('Initial DNS Seedlist Discovery (Prose Tests)', () => { + describe('1. Allow SRVs with fewer than 3 . separated parts', function () { + context('when running validation on an SRV string before DNS resolution', function () { + /** + * When running validation on an SRV string before DNS resolution, do not throw a error due to number of SRV parts. + * - mongodb+srv://localhost + * - mongodb+srv://mongo.localhost + */ + + let client; + + beforeEach(async function () { + // this fn stubs DNS resolution to always pass - so we are only checking pre-DNS validation + + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'resolved.mongo.localhost', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + + sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { + throw { code: 'ENODATA' }; + }); + + sinon.stub(Topology.prototype, 'selectServer').callsFake(async () => { + return new Server( + topologyWithPlaceholderClient([], {} as any), + new ServerDescription('a:1'), + {} as any + ); + }); + }); + + afterEach(async function () { + sinon.restore(); + await client.close(); + }); + + it('does not error on an SRV because it has one domain level', async function () { + client = await this.configuration.newClient('mongodb+srv://localhost', {}); + await client.connect(); + }); + + it('does not error on an SRV because it has two domain levels', async function () { + client = await this.configuration.newClient('mongodb+srv://mongo.localhost', {}); + await client.connect(); + }); + }); + }); + + describe('2. Throw when return address does not end with SRV domain', function () { + context( + 'when given a host from DNS resolution that does NOT end with the original SRVs domain name', + function () { + /** + * When given a returned address that does NOT end with the original SRV's domain name, throw a runtime error. + * For this test, run each of the following cases: + * - the SRV mongodb+srv://localhost resolving to localhost.mongodb + * - the SRV mongodb+srv://mongo.local resolving to test_1.evil.local + * - the SRV mongodb+srv://blogs.mongodb.com resolving to blogs.evil.com + * Remember, the domain of an SRV with one or two . separated parts is the SRVs entire hostname. + */ + + beforeEach(async function () { + sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { + throw { code: 'ENODATA' }; + }); + }); + + afterEach(async function () { + sinon.restore(); + }); + + it('an SRV with one domain level causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'localhost.mongodb', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://localhost', {}) + .connect() + .catch((e: any) => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); + }); + + it('an SRV with two domain levels causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'test_1.evil.local', // this string only ends with part of the domain, not all of it! + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://mongo.local', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); + }); + + it('an SRV with three or more domain levels causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'blogs.evil.com', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://blogs.mongodb.com', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); + }); + } + ); + }); + + describe('3. Throw when return address is identical to SRV hostname', function () { + /** + * When given a returned address that is identical to the SRV hostname and the SRV hostname has fewer than three . separated parts, throw a runtime error. + * For this test, run each of the following cases: + * - the SRV mongodb+srv://localhost resolving to localhost + * - the SRV mongodb+srv://mongo.local resolving to mongo.local + */ + + context( + 'when given a host from DNS resolution that is identical to the original SRVs hostname', + function () { + beforeEach(async function () { + sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { + throw { code: 'ENODATA' }; + }); + }); + + afterEach(async function () { + sinon.restore(); + }); + + it('an SRV with one domain level causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'localhost', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://localhost', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal( + 'Server record does not have at least one more domain level than parent URI' + ); + }); + + it('an SRV with two domain levels causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'mongo.local', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://mongo.local', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal( + 'Server record does not have at least one more domain level than parent URI' + ); + }); + } + ); + }); + + describe('4. Throw when return address does not contain . separating shared part of domain', function () { + /** + * When given a returned address that does NOT share the domain name of the SRV record because it's missing a ., throw a runtime error. + * For this test, run each of the following cases: + * - the SRV mongodb+srv://localhost resolving to test_1.cluster_1localhost + * - the SRV mongodb+srv://mongo.local resolving to test_1.my_hostmongo.local + * - the SRV mongodb+srv://blogs.mongodb.com resolving to cluster.testmongodb.com + */ + + context( + 'when given a returned address that does NOT share the domain name of the SRV record because its missing a `.`', + function () { + beforeEach(async function () { + sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { + throw { code: 'ENODATA' }; + }); + }); + + afterEach(async function () { + sinon.restore(); + }); + + it('an SRV with one domain level causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'test_1.cluster_1localhost', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://localhost', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); + }); + + it('an SRV with two domain levels causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'test_1.my_hostmongo.local', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://mongo.local', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); + }); + + it('an SRV with three domain levels causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'cluster.testmongodb.com', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://blogs.mongodb.com', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); + }); + } + ); + }); +}); diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts index 7494fb080f2..b15dbdf1f16 100644 --- a/test/unit/utils.test.ts +++ b/test/unit/utils.test.ts @@ -3,6 +3,7 @@ import { expect } from 'chai'; import { BufferPool, ByteUtils, + checkParentDomainMatch, compareObjectId, decorateWithExplain, Explain, @@ -12,7 +13,6 @@ import { isUint8Array, LEGACY_HELLO_COMMAND, List, - matchesParentDomain, MongoDBCollectionNamespace, MongoDBNamespace, MongoRuntimeError, @@ -939,46 +939,94 @@ describe('driver utils', function () { }); }); - describe('matchesParentDomain()', () => { - const exampleSrvName = 'i-love-javascript.mongodb.io'; - const exampleSrvNameWithDot = 'i-love-javascript.mongodb.io.'; - const exampleHostNameWithoutDot = 'i-love-javascript-00.mongodb.io'; - const exampleHostNamesWithDot = exampleHostNameWithoutDot + '.'; - const exampleHostNamThatDoNotMatchParent = 'i-love-javascript-00.evil-mongodb.io'; - const exampleHostNamThatDoNotMatchParentWithDot = 'i-love-javascript-00.evil-mongodb.io.'; + describe('checkParentDomainMatch()', () => { + const exampleSrvName = ['i-love-js', 'i-love-js.mongodb', 'i-love-javascript.mongodb.io']; + const exampleSrvNameWithDot = [ + 'i-love-js.', + 'i-love-js.mongodb.', + 'i-love-javascript.mongodb.io.' + ]; + const exampleHostNameWithoutDot = [ + 'js-00.i-love-js', + 'js-00.i-love-js.mongodb', + 'i-love-javascript-00.mongodb.io' + ]; + const exampleHostNamesWithDot = [ + 'js-00.i-love-js.', + 'js-00.i-love-js.mongodb.', + 'i-love-javascript-00.mongodb.io.' + ]; + const exampleHostNameThatDoNotMatchParent = [ + 'js-00.i-love-js-a-little', + 'js-00.i-love-js-a-little.mongodb', + 'i-love-javascript-00.evil-mongodb.io' + ]; + const exampleHostNameThatDoNotMatchParentWithDot = [ + 'i-love-js', + '', + 'i-love-javascript-00.evil-mongodb.io.' + ]; - context('when address does not match parent domain', () => { - it('without a trailing dot returns false', () => { - expect(matchesParentDomain(exampleHostNamThatDoNotMatchParent, exampleSrvName)).to.be.false; - }); + for (let num = 0; num < 3; num += 1) { + context(`when srvName has ${num + 1} part${num !== 0 ? 's' : ''}`, () => { + context('when address does not match parent domain', () => { + it('without a trailing dot throws', () => { + expect(() => + checkParentDomainMatch(exampleHostNameThatDoNotMatchParent[num], exampleSrvName[num]) + ).to.throw('Server record does not share hostname with parent URI'); + }); - it('with a trailing dot returns false', () => { - expect(matchesParentDomain(exampleHostNamThatDoNotMatchParentWithDot, exampleSrvName)).to.be - .false; - }); - }); + it('with a trailing dot throws', () => { + expect(() => + checkParentDomainMatch( + exampleHostNameThatDoNotMatchParentWithDot[num], + exampleSrvName[num] + ) + ).to.throw(); + }); + }); - context('when addresses in SRV record end with a dot', () => { - it('accepts address since it is considered to still match the parent domain', () => { - expect(matchesParentDomain(exampleHostNamesWithDot, exampleSrvName)).to.be.true; - }); - }); + context('when addresses in SRV record end with a dot', () => { + it('accepts address since it is considered to still match the parent domain', () => { + expect(() => + checkParentDomainMatch(exampleHostNamesWithDot[num], exampleSrvName[num]) + ).to.not.throw(); + }); + }); - context('when SRV host ends with a dot', () => { - it('accepts address if it ends with a dot', () => { - expect(matchesParentDomain(exampleHostNamesWithDot, exampleSrvNameWithDot)).to.be.true; - }); + context('when SRV host ends with a dot', () => { + it('accepts address if it ends with a dot', () => { + expect(() => + checkParentDomainMatch(exampleHostNamesWithDot[num], exampleSrvNameWithDot[num]) + ).to.not.throw(); + }); - it('accepts address if it does not end with a dot', () => { - expect(matchesParentDomain(exampleHostNameWithoutDot, exampleSrvName)).to.be.true; - }); - }); + it('accepts address if it does not end with a dot', () => { + expect(() => + checkParentDomainMatch(exampleHostNameWithoutDot[num], exampleSrvNameWithDot[num]) + ).to.not.throw(); + }); - context('when addresses in SRV record end without dots', () => { - it('accepts address since it matches the parent domain', () => { - expect(matchesParentDomain(exampleHostNamesWithDot, exampleSrvName)).to.be.true; + if (num < 2) { + it('does not accept address if it does not contain an extra domain level', () => { + expect(() => + checkParentDomainMatch(exampleSrvNameWithDot[num], exampleSrvNameWithDot[num]) + ).to.throw( + 'Server record does not have at least one more domain level than parent URI' + ); + }); + } + }); + + context('when addresses in SRV record end without dots', () => { + it('accepts address since it matches the parent domain', () => { + expect(() => + checkParentDomainMatch(exampleHostNamesWithDot[num], exampleSrvName[num]) + ).to.not.throw(); + }); + }); }); - }); + } }); describe('isUint8Array()', () => {