diff --git a/helpers/wkd.js b/helpers/wkd.js index 0eb4c9007..fed4564e1 100644 --- a/helpers/wkd.js +++ b/helpers/wkd.js @@ -115,31 +115,47 @@ function WKD(resolver, client) { : { connect: { lookup(hostname, options, fn) { + // Node 20+ asks for every address (`all: true`, for + // autoSelectFamily) and the resolver then returns an array, + // so all addresses are requested and each one is validated resolver - .lookup(hostname, options) - .then(async (result) => { + .lookup(hostname, { ...options, all: true }) + .then(async (results) => { + const addresses = ( + Array.isArray(results) ? results : [results] + ).filter((result) => result?.address); + if (addresses.length === 0) { + const err = new Error(`No address found for ${hostname}`); + err.code = 'ENOTFOUND'; + fn(err); + return; + } + // FWD-01-006: Validate resolved IP at connection time // to prevent TOCTOU DNS rebinding attacks - if ( - result?.address && - (await WKD.isPrivateTarget(result.address, resolver)) - ) { - fn( - new Error( - `Resolved IP ${result.address} is a private address` - ) - ); - return; + for (const { address } of addresses) { + if (await WKD.isPrivateTarget(address, resolver)) { + fn( + new Error( + `Resolved IP ${address} is a private address` + ) + ); + return; + } } // Handle both Node 18 (address, family) and Node 20+ // (all:true expects [{address, family}] array) formats if (options.all) { - fn(null, [ - { address: result?.address, family: result?.family } - ]); + fn( + null, + addresses.map(({ address, family }) => ({ + address, + family + })) + ); } else { - fn(null, result?.address, result?.family); + fn(null, addresses[0].address, addresses[0].family); } }) .catch((err) => fn(err)); diff --git a/test/helpers/wkd-connect-lookup.js b/test/helpers/wkd-connect-lookup.js new file mode 100644 index 000000000..a664d658e --- /dev/null +++ b/test/helpers/wkd-connect-lookup.js @@ -0,0 +1,120 @@ +/** + * Copyright (c) Forward Email LLC + * SPDX-License-Identifier: BUSL-1.1 + */ + +const http = require('node:http'); +const { once } = require('node:events'); + +const test = require('ava'); +const sinon = require('sinon'); + +const WKD = require('#helpers/wkd'); + +// +// Resolver that answers like Tangerine: an array of addresses when it is +// asked for all of them (as Node 20+ does for autoSelectFamily), otherwise +// the first address. +// +function createResolver(addresses) { + const resolver = { + calls: [], + async lookup(hostname, options = {}) { + resolver.calls.push({ hostname, options }); + if (options.all) return addresses; + return addresses[0]; + } + }; + return resolver; +} + +async function listen() { + const hits = []; + const server = http.createServer((req, res) => { + hits.push(req.url); + res.writeHead(200, { 'content-type': 'application/octet-stream' }); + res.end('key'); + }); + server.listen(0, '127.0.0.1'); + await once(server, 'listening'); + return { server, hits, port: server.address().port }; +} + +// The connect-time lookup is only used for hostnames, not for IP literals +async function fetchLocalhost(t, resolver) { + const origin = await listen(); + t.teardown(() => origin.server.close()); + const wkd = new WKD(resolver, null); + const promise = wkd._fetch(`http://localhost:${origin.port}/key`); + return { origin, promise }; +} + +test.afterEach.always(() => { + sinon.restore(); +}); + +test.serial( + 'a hostname is fetched when the resolver returns every address', + async (t) => { + sinon.stub(WKD, 'isPrivateTarget').resolves(false); + const resolver = createResolver([{ address: '127.0.0.1', family: 4 }]); + + const { origin, promise } = await fetchLocalhost(t, resolver); + const response = await promise; + t.is(response.status, 200); + t.is(await response.text(), 'key'); + t.deepEqual(origin.hits, ['/key']); + t.true(resolver.calls.some(({ options }) => options.all === true)); + } +); + +test.serial( + 'a hostname is fetched when the resolver returns a single address', + async (t) => { + sinon.stub(WKD, 'isPrivateTarget').resolves(false); + const resolver = { + async lookup() { + return { address: '127.0.0.1', family: 4 }; + } + }; + + const { origin, promise } = await fetchLocalhost(t, resolver); + const response = await promise; + t.is(response.status, 200); + t.deepEqual(origin.hits, ['/key']); + } +); + +test.serial( + 'a hostname with any private address is not connected to', + async (t) => { + // The hostname passes the check made before connecting; one of the + // addresses it resolves to at connection time is private. + const stub = sinon + .stub(WKD, 'isPrivateTarget') + .callsFake(async (target) => target === '10.0.0.1'); + const resolver = createResolver([ + { address: '127.0.0.1', family: 4 }, + { address: '10.0.0.1', family: 4 } + ]); + + const { origin, promise } = await fetchLocalhost(t, resolver); + const err = await t.throwsAsync(promise); + t.regex( + `${err.message} ${err.cause?.message}`, + /10\.0\.0\.1 is a private address/ + ); + t.is(origin.hits.length, 0, 'private address must not be requested'); + t.true(stub.calledWith('10.0.0.1')); + } +); + +test.serial('a hostname without addresses is not connected to', async (t) => { + sinon.stub(WKD, 'isPrivateTarget').resolves(false); + const resolver = createResolver([]); + + const { origin, promise } = await fetchLocalhost(t, resolver); + const err = await t.throwsAsync(promise); + t.regex(`${err.message} ${err.cause?.message}`, /No address found/); + t.is(origin.hits.length, 0); +});