From 2a488b0d0f21fe5ea5bd7cab02cb78367a6ceb9b Mon Sep 17 00:00:00 2001 From: Erik Johansen Date: Fri, 9 Oct 2026 19:41:38 +0200 Subject: [PATCH] fix: handle multi-address lookups for WKD on Node 20+ Node 20+ calls the connect-time lookup with `all: true` (autoSelectFamily), and the resolver then returns an array of addresses. The lookup read `.address` of that array, so every WKD fetch failed with ERR_INVALID_IP_ADDRESS and the private address check was skipped. All addresses are now requested from the resolver and each one is checked before any is used, then returned in the form Node asked for. Co-Authored-By: Claude Opus 5.5 --- helpers/wkd.js | 48 ++++++++---- test/helpers/wkd-connect-lookup.js | 120 +++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 16 deletions(-) create mode 100644 test/helpers/wkd-connect-lookup.js 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); +});