Use DNS to make passthrough relative-localhost check more accurate

Previously we just checked for known localhost URLs. This doesn't help
for requests to other hostnames that will resolve to localhost though.

We now handle that too, by resolving hostnames to IPs and then checking
those instead, when necessary.

This PR also extracts all that logic, and DNS configuration generally,
out from req/WS handlers into shared methods.

Author: pimterry

src/rules/passthrough-handling.ts

@@ -2,16 +2,20 @@ import * as _ from 'lodash';
 import * as tls from 'tls';
 import url = require('url');
 import { oneLine } from 'common-tags';
+import CacheableLookup from 'cacheable-lookup';
 
 import { CompletedBody, Headers } from '../types';
 import { byteLength } from '../util/util';
 import { asBuffer } from '../util/buffer-utils';
+import { isLocalhostAddress } from '../util/socket-util';
+import { CachedDns, dnsLookup, DnsLookupFunction } from '../util/dns';
 import { isMockttpBody, encodeBodyBuffer } from '../util/request-utils';
 import { areFFDHECurvesSupported } from '../util/openssl-compat';
 
 import {
     CallbackRequestResult,
-    CallbackResponseMessageResult
+    CallbackResponseMessageResult,
+    PassThroughLookupOptions
 } from './requests/request-handler-definitions';
 
 // TLS settings for proxied connections, intended to avoid TLS fingerprint blocking
@@ -270,3 +274,55 @@ export function shouldUseStrictHttps(
     }
     return !skipHttpsErrors;
 }
+
+export const getDnsLookupFunction = _.memoize((lookupOptions: PassThroughLookupOptions | undefined) => {
+    if (!lookupOptions) {
+        // By default, use 10s caching of hostnames, just to reduce the delay from
+        // endlessly 10ms query delay for 'localhost' with every request.
+        return new CachedDns(10000).lookup;
+    } else {
+        // Or if options are provided, use those to configure advanced DNS cases:
+        const cacheableLookup = new CacheableLookup({
+            maxTtl: lookupOptions.maxTtl,
+            errorTtl: lookupOptions.errorTtl,
+            // As little caching of "use the fallback server" as possible:
+            fallbackDuration: 0
+        });
+
+        if (lookupOptions.servers) {
+            cacheableLookup.servers = lookupOptions.servers;
+        }
+
+        return cacheableLookup.lookup;
+    }
+});
+
+export async function getClientRelativeHostname(
+    hostname: string | null,
+    remoteIp: string | undefined,
+    lookupFn: DnsLookupFunction
+) {
+    if (!hostname || !remoteIp || isLocalhostAddress(remoteIp)) return hostname;
+
+    // Otherwise, we have a request from a different machine (or Docker container/VM/etc) and we need
+    // to make sure that 'localhost' means _that_ machine, not ourselves.
+
+    // This check must be run before req modifications. If a modification changes the address to localhost,
+    // then presumably it really does mean *this* localhost.
+
+    if (
+        // If the hostname is a known localhost address, we're done:
+        isLocalhostAddress(hostname) ||
+        // Otherwise, we look up the IP, so we can accurately check for localhost-bound requests. This adds a little
+        // delay, but since it's cached we save the equivalent delay in request lookup later, so it should be
+        // effectively free. We ignore errors to delegate unresolvable etc to request processing later.
+        isLocalhostAddress(await dnsLookup(lookupFn, hostname).catch(() => null))
+    ) {
+        return remoteIp;
+
+        // Note that we just redirect - we don't update the host header. From the POV of the target, it's still
+        // 'localhost' traffic that should appear identical to normal.
+    } else {
+        return hostname;
+    }
+}

src/rules/requests/request-handlers.ts

@@ -7,7 +7,6 @@ import http = require('http');
 import https = require('https');
 import * as fs from 'fs/promises';
 import * as h2Client from 'http2-wrapper';
-import CacheableLookup from 'cacheable-lookup';
 import { decode as decodeBase64 } from 'base64-arraybuffer';
 import { Transform } from 'stream';
 import { stripIndent, oneLine } from 'common-tags';
@@ -60,7 +59,6 @@ import {
     withDeserializedCallbackBuffers,
     WithSerializedCallbackBuffers
 } from '../../serialization/body-serialization';
-import { CachedDns, DnsLookupFunction } from '../../util/dns';
 import { ErrorLike, isErrorLike } from '../../util/error';
 
 import { assertParamDereferenced, RuleParameters } from '../rule-parameters';
@@ -74,7 +72,9 @@ import {
     OVERRIDABLE_REQUEST_PSEUDOHEADERS,
     buildOverriddenBody,
     UPSTREAM_TLS_OPTIONS,
-    shouldUseStrictHttps
+    shouldUseStrictHttps,
+    getClientRelativeHostname,
+    getDnsLookupFunction
 } from '../passthrough-handling';
 
 import {
@@ -380,33 +380,6 @@ export class PassThroughHandler extends PassThroughHandlerDefinition {
         return this._trustedCACertificates;
     }
 
-    private _cacheableLookupInstance: CacheableLookup | CachedDns | undefined;
-    private lookup(): DnsLookupFunction {
-        if (!this.lookupOptions) {
-            if (!this._cacheableLookupInstance) {
-                // By default, use 10s caching of hostnames, just to reduce the delay from
-                // endlessly 10ms query delay for 'localhost' with every request.
-                this._cacheableLookupInstance = new CachedDns(10000);
-            }
-            return this._cacheableLookupInstance.lookup;
-        } else {
-            if (!this._cacheableLookupInstance) {
-                this._cacheableLookupInstance = new CacheableLookup({
-                    maxTtl: this.lookupOptions.maxTtl,
-                    errorTtl: this.lookupOptions.errorTtl,
-                    // As little caching of "use the fallback server" as possible:
-                    fallbackDuration: 0
-                });
-
-                if (this.lookupOptions.servers) {
-                    this._cacheableLookupInstance.servers = this.lookupOptions.servers;
-                }
-            }
-
-            return this._cacheableLookupInstance.lookup;
-        }
-    }
-
     async handle(clientReq: OngoingRequest, clientRes: OngoingResponse) {
         // Don't let Node add any default standard headers - we want full control
         dropDefaultHeaders(clientRes);
@@ -434,14 +407,11 @@ export class PassThroughHandler extends PassThroughHandlerDefinition {
 
         const isH2Downstream = isHttp2(clientReq);
 
-        if (isLocalhostAddress(hostname) && clientReq.remoteIpAddress && !isLocalhostAddress(clientReq.remoteIpAddress)) {
-            // If we're proxying localhost traffic from another remote machine, then we should really be proxying
-            // back to that machine, not back to ourselves! Best example is docker containers: if we capture & inspect
-            // their localhost traffic, it should still be sent back into that docker container.
-            hostname = clientReq.remoteIpAddress;
-
-            // We don't update the host header - from the POV of the target, it's still localhost traffic.
-        }
+        hostname = await getClientRelativeHostname(
+            hostname,
+            clientReq.remoteIpAddress,
+            getDnsLookupFunction(this.lookupOptions)
+        );
 
         if (this.forwarding) {
             const { targetHost, updateHostHeader } = this.forwarding;
@@ -747,7 +717,7 @@ export class PassThroughHandler extends PassThroughHandlerDefinition {
                 headers: shouldTryH2Upstream
                     ? rawHeadersToObjectPreservingCase(rawHeaders)
                     : flattenPairedRawHeaders(rawHeaders) as any,
-                lookup: this.lookup() as typeof dns.lookup,
+                lookup: getDnsLookupFunction(this.lookupOptions) as typeof dns.lookup,
                 // ^ Cast required to handle __promisify__ type hack in the official Node types
                 agent,
                 // TLS options:

src/rules/websockets/websocket-handlers.ts

@@ -5,7 +5,6 @@ import * as tls from 'tls';
 import * as http from 'http';
 import * as fs from 'fs/promises';
 import * as WebSocket from 'ws';
-import CacheableLookup from 'cacheable-lookup';
 
 import {
     ClientServerChannel,
@@ -29,14 +28,15 @@ import {
     rawHeadersToObjectPreservingCase
 } from '../../util/header-utils';
 import { streamToBuffer } from '../../util/buffer-utils';
-import { isLocalhostAddress } from '../../util/socket-util';
 import { MaybePromise } from '../../util/type-utils';
 
 import { getAgent } from '../http-agents';
 import { ProxySettingSource } from '../proxy-config';
 import { assertParamDereferenced, RuleParameters } from '../rule-parameters';
 import {
     UPSTREAM_TLS_OPTIONS,
+    getClientRelativeHostname,
+    getDnsLookupFunction,
     shouldUseStrictHttps
 } from '../passthrough-handling';
 
@@ -212,26 +212,6 @@ export class PassThroughWebSocketHandler extends PassThroughWebSocketHandlerDefi
         return this._trustedCACertificates;
     }
 
-    private _cacheableLookupInstance: CacheableLookup | undefined;
-    private lookup() {
-        if (!this.lookupOptions) return undefined;
-
-        if (!this._cacheableLookupInstance) {
-            this._cacheableLookupInstance = new CacheableLookup({
-                maxTtl: this.lookupOptions.maxTtl,
-                errorTtl: this.lookupOptions.errorTtl,
-                // As little caching of "use the fallback server" as possible:
-                fallbackDuration: 0
-            });
-
-            if (this.lookupOptions.servers) {
-                this._cacheableLookupInstance.servers = this.lookupOptions.servers;
-            }
-        }
-
-        return this._cacheableLookupInstance.lookup;
-    }
-
     async handle(req: OngoingRequest, socket: net.Socket, head: Buffer) {
         this.initializeWsServer();
 
@@ -242,14 +222,11 @@ export class PassThroughWebSocketHandler extends PassThroughWebSocketHandlerDefi
         const isH2Downstream = isHttp2(req);
         const hostHeaderName = isH2Downstream ? ':authority' : 'host';
 
-        if (isLocalhostAddress(hostname) && req.remoteIpAddress && !isLocalhostAddress(req.remoteIpAddress)) {
-            // If we're proxying localhost traffic from another remote machine, then we should really be proxying
-            // back to that machine, not back to ourselves! Best example is docker containers: if we capture & inspect
-            // their localhost traffic, it should still be sent back into that docker container.
-            hostname = req.remoteIpAddress;
-
-            // We don't update the host header - from the POV of the target, it's still localhost traffic.
-        }
+        hostname = await getClientRelativeHostname(
+            hostname,
+            req.remoteIpAddress,
+            getDnsLookupFunction(this.lookupOptions)
+        );
 
         if (this.forwarding) {
             const { targetHost, updateHostHeader } = this.forwarding;
@@ -348,7 +325,7 @@ export class PassThroughWebSocketHandler extends PassThroughWebSocketHandlerDefi
         const upstreamWebSocket = new WebSocket(wsUrl, {
             maxPayload: 0,
             agent,
-            lookup: this.lookup(),
+            lookup: getDnsLookupFunction(this.lookupOptions),
             headers: _.omitBy(headers, (_v, headerName) =>
                 headerName.toLowerCase().startsWith('sec-websocket') ||
                 headerName.toLowerCase() === 'connection' ||

src/util/dns.ts

@@ -40,4 +40,13 @@ export class CachedDns {
         }
     }
 
+}
+
+export function dnsLookup(lookupFn: typeof dns.lookup | DnsLookupFunction, hostname: string) {
+    return new Promise<string>((resolve, reject) => {
+        (lookupFn as typeof dns.lookup)(hostname!, (err, address) => {
+            if (err) reject(err);
+            else resolve(address);
+        });
+    })
 }

src/util/socket-util.ts

@@ -45,10 +45,12 @@ const normalizeIp = (ip: string | null | undefined) =>
         : ip;
 
 export const isLocalhostAddress = (host: string | null | undefined) =>
-    host === 'localhost' || // Most common
-    host?.endsWith('.localhost') ||
-    host === '::1' || // IPv6
-    normalizeIp(host)?.match(/^127\.\d{1,3}\.\d{1,3}\.\d{1,3}$/); // 127.0.0.0/8 range
+    !!host && ( // Null/undef are something else weird, but not localhost
+        host === 'localhost' || // Most common
+        host.endsWith('.localhost') ||
+        host === '::1' || // IPv6
+        normalizeIp(host)!.match(/^127\.\d{1,3}\.\d{1,3}\.\d{1,3}$/) // 127.0.0.0/8 range
+    );
 
 
 // Check whether an incoming socket is the other end of one of our outgoing sockets: