Refactor Docker proxy host routing to avoid ExtraHosts

pimterry · pimterry · commit 40c7c57f71f6 · 2022-11-15T15:42:43.000+01:00
ExtraHosts doesn't work with any client that relies exclusively on DNS
(one good example so far - Mastodon - but this is also easy to do in
Node.js and elsewhere). This therefore breaks all Docker connections to
the proxy on Linux, where host.docker.internal was injected this way.
Instead, we now use the raw IP on Linux, avoiding DNS/hostname issues
entirely.
diff --git a/src/interceptors/docker/docker-build-injection.ts b/src/interceptors/docker/docker-build-injection.ts
@@ -13,7 +13,7 @@ import {
     OVERRIDES_DIR
 } from '../terminal/terminal-env-overrides';
 import { getDeferred } from '../../util/promise';
-import { DOCKER_HOST_HOSTNAME } from './docker-commands';
+import { getDockerHostAddress } from './docker-commands';
 
 const HTTP_TOOLKIT_INJECTED_PATH = '/http-toolkit-injections';
 const HTTP_TOOLKIT_INJECTED_OVERRIDES_PATH = path.posix.join(HTTP_TOOLKIT_INJECTED_PATH, 'overrides');
@@ -44,7 +44,7 @@ export function injectIntoBuildStream(
         { certPath: HTTP_TOOLKIT_INJECTED_CA_PATH },
         'posix-runtime-inherit', // Dockerfile commands can reference vars directly
         {
-            httpToolkitIp: DOCKER_HOST_HOSTNAME,
+            httpToolkitHost: getDockerHostAddress(process.platform),
             overridePath: HTTP_TOOLKIT_INJECTED_OVERRIDES_PATH,
             targetPlatform: 'linux'
         }
diff --git a/src/interceptors/docker/docker-commands.ts b/src/interceptors/docker/docker-commands.ts
@@ -22,51 +22,28 @@ const HTTP_TOOLKIT_INJECTED_OVERRIDES_PATH = path.posix.join(HTTP_TOOLKIT_INJECT
 const HTTP_TOOLKIT_INJECTED_CA_PATH = path.posix.join(HTTP_TOOLKIT_INJECTED_PATH, 'ca.pem');
 
 /**
- * The hostname that resolves to the host OS (i.e. generally: where HTTP Toolkit is running)
+ * Get the hostname that resolves to the host OS (i.e. generally: where HTTP Toolkit is running)
  * from inside containers.
  *
  * In Docker for Windows & Mac, host.docker.internal is supported automatically:
  * https://docs.docker.com/docker-for-windows/networking/#use-cases-and-workarounds
  * https://docs.docker.com/docker-for-mac/networking/#use-cases-and-workarounds
  *
- * On Linux this is _not_ supported, so we add it ourselves with (--add-host).
+ * On Linux this is _not_ supported, and we need to be more clever.
  */
-export const DOCKER_HOST_HOSTNAME = "host.docker.internal";
-
-/**
- * To make the above hostname work on Linux, where it's not supported by default, we need to map it to the
- * host ip. This method works out the host IP to use to do so.
- */
-export const getDockerHostIp = (
+export function getDockerHostAddress(
     platform: typeof process.platform,
-    dockerVersion: { apiVersion: string } | { engineVersion: string },
     containerMetadata?: Docker.ContainerInspectInfo
-) => {
-    const semverVersion = semver.coerce(
-        'apiVersion' in dockerVersion
-        ? dockerVersion.apiVersion
-        : dockerVersion.engineVersion
-    );
-
-    if (platform !== 'linux') {
-        // On non-linux platforms this method isn't necessary - host.docker.internal is always supported
-        // so we can just use that.
-        return DOCKER_HOST_HOSTNAME;
-    } else if (
-        semver.satisfies(
-            semverVersion ?? '0.0.0',
-            'apiVersion' in dockerVersion ? '>=1.41' : '>=20.10'
-        )
-    ) {
-        // This is supported in Docker Engine 20.10, so always supported at least in API 1.41+
-        // Special name defined in new Docker versions, that refers to the host gateway
-        return 'host-gateway';
-    } else if (containerMetadata) {
-        // Old/Unknown Linux with known container: query the metadata, and if _that_ fails, use the default gateway IP.
-        return containerMetadata.NetworkSettings.Gateway || "172.17.0.1";
+) {
+    if (platform === 'win32' || platform === 'darwin') {
+        // On Docker Desktop, this alias always points to the host (outside the VM) IP:
+        return 'host.docker.internal';
     } else {
-        // Old/Unknown Linux without a container (e.g. during a build). Always use the default gateway IP:
-        return "172.17.0.1";
+        // Elsewhere (Linux) we should be able to always use the gateway address. We avoid
+        // using ExtraHosts with host-gateway, because that uses /etc/hosts, and not all
+        // clients use that for resolution (some use _only_ DNS lookups). IPs avoid this.
+        return containerMetadata?.NetworkSettings.Gateway
+            || "172.17.0.1";
     }
 }
 
@@ -134,7 +111,7 @@ export function transformContainerCreationConfig(
         { certPath: HTTP_TOOLKIT_INJECTED_CA_PATH },
         envArrayToObject(currentConfig.Env),
         {
-            httpToolkitIp: DOCKER_HOST_HOSTNAME,
+            httpToolkitHost: getDockerHostAddress(process.platform),
             overridePath: HTTP_TOOLKIT_INJECTED_OVERRIDES_PATH,
             targetPlatform: 'linux'
         }
@@ -160,18 +137,7 @@ export function transformContainerCreationConfig(
             // Bind-mount the overrides directory into the container:
             `${OVERRIDES_DIR}:${HTTP_TOOLKIT_INJECTED_OVERRIDES_PATH}:ro`
             // ^ Both 'ro' - untrusted containers must not be able to mess with these!
-        ],
-        ...(process.platform === 'linux'
-            // On Linux only, we need to add an explicit host to make host.docker.internal work:
-            ? {
-                ExtraHosts: [
-                    `${DOCKER_HOST_HOSTNAME}:${proxyHost}`,
-                    // Seems that first host wins conflicts, so we go before existing values
-                    ...(currentConfig.HostConfig?.ExtraHosts ?? [])
-                ]
-            }
-            : {}
-        )
+        ]
     };
 
     // Extend that config, injecting our custom overrides:
@@ -256,11 +222,7 @@ export async function restartAndInjectContainer(
         }
     });
 
-    const proxyHost = getDockerHostIp(
-        process.platform,
-        { engineVersion: (await docker.version()).Version },
-        containerDetails
-    );
+    const proxyHost = getDockerHostAddress(process.platform, containerDetails);
 
     // First we clone the continer, injecting our custom settings:
     const newContainer = await docker.createContainer(
diff --git a/src/interceptors/docker/docker-networking.ts b/src/interceptors/docker/docker-networking.ts
@@ -6,7 +6,7 @@ import * as mobx from 'mobx';
 
 import { reportError } from '../../error-tracking';
 
-import { DOCKER_HOST_HOSTNAME, isInterceptedContainer } from './docker-commands';
+import { isInterceptedContainer } from './docker-commands';
 import { isDockerAvailable } from './docker-interception-services';
 import { updateDockerTunnelledNetworks } from './docker-tunnel-proxy';
 import { getDnsServer } from '../../dns-server';
diff --git a/src/interceptors/docker/docker-proxy.ts b/src/interceptors/docker/docker-proxy.ts
@@ -15,10 +15,9 @@ import { reportError } from '../../error-tracking';
 import { addShutdownHandler } from '../../shutdown';
 
 import {
+    getDockerHostAddress,
     isInterceptedContainer,
-    transformContainerCreationConfig,
-    DOCKER_HOST_HOSTNAME,
-    getDockerHostIp
+    transformContainerCreationConfig
 } from './docker-commands';
 import { injectIntoBuildStream, getBuildOutputPipeline } from './docker-build-injection';
 import { ensureDockerServicesRunning, isDockerAvailable } from './docker-interception-services';
@@ -127,10 +126,7 @@ async function createDockerProxy(proxyPort: number, httpsConfig: { certPath: str
                 // create will fail, and will be re-run after the image is pulled in a minute.
                 .catch(() => undefined);
 
-            const proxyHost = getDockerHostIp(
-                process.platform,
-                { apiVersion: dockerApiVersion! },
-            );
+            const proxyHost = getDockerHostAddress(process.platform);
 
             const transformedConfig = transformContainerCreationConfig(
                 config,
@@ -182,18 +178,6 @@ async function createDockerProxy(proxyPort: number, httpsConfig: { certPath: str
 
             requestBodyStream = streamInjection.injectedStream;
             extraDockerCommandCount = streamInjection.totalCommandsAddedPromise;
-
-            // Make sure that host.docker.internal resolves on Linux too:
-            if (process.platform === 'linux') {
-                reqUrl.searchParams.append(
-                    'extrahosts',
-                    `${DOCKER_HOST_HOSTNAME}:${getDockerHostIp(
-                        process.platform,
-                        { apiVersion: dockerApiVersion! }
-                    )}`
-                );
-                req.url = reqUrl.toString();
-            }
         }
 
         const dockerReq = sendToDocker(req, requestBodyStream);
diff --git a/src/interceptors/docker/docker-tunnel-proxy.ts b/src/interceptors/docker/docker-tunnel-proxy.ts
@@ -3,7 +3,7 @@ import * as Docker from 'dockerode';
 import * as semver from 'semver';
 import { Mutex } from 'async-mutex';
 
-import { DOCKER_HOST_HOSTNAME, isImageAvailable } from './docker-commands';
+import { getDockerHostAddress, isImageAvailable } from './docker-commands';
 import { isDockerAvailable } from './docker-interception-services';
 import { delay } from '../../util/promise';
 import { reportError } from '../../error-tracking';
@@ -73,21 +73,6 @@ export function ensureDockerTunnelRunning(proxyPort: number) {
                 },
                 HostConfig: {
                     AutoRemove: true,
-                    ...(process.platform === 'linux' ? {
-                        ExtraHosts: [
-                            // Make sure the host hostname is defined (not set by default on Linux).
-                            // We use the host-gateway address on engines where that's possible, or
-                            // the default Docker bridge host IP when it's not, because we're always
-                            // connected to that network.
-                            `${DOCKER_HOST_HOSTNAME}:${
-                                semver.satisfies(engineVersion, '>= 20.10')
-                                    ? 'host-gateway'
-                                    : defaultBridgeGateway || '172.17.0.1'
-                            }`
-                            // (This doesn't reuse getDockerHostIp, since the logic is slightly
-                            // simpler  and we never have container metadata/network state).
-                        ]
-                    } : {}),
                     PortBindings: {
                         '1080/tcp': [{
                             // Bind host-locally only: we don't want to let remote clients
diff --git a/src/interceptors/terminal/terminal-env-overrides.ts b/src/interceptors/terminal/terminal-env-overrides.ts
@@ -26,13 +26,13 @@ export function getTerminalEnvVars(
         | 'posix-runtime-inherit'
         | 'powershell-runtime-inherit',
     targetEnvConfig: {
-        httpToolkitIp?: string,
+        httpToolkitHost?: string,
         overridePath?: string,
         targetPlatform?: NodeJS.Platform
     } = {}
 ): { [key: string]: string } {
-    const { overridePath, targetPlatform, httpToolkitIp } = {
-        httpToolkitIp: '127.0.0.1',
+    const { overridePath, targetPlatform, httpToolkitHost } = {
+        httpToolkitHost: '127.0.0.1',
         overridePath: OVERRIDES_DIR,
         targetPlatform: process.platform,
         ...targetEnvConfig
@@ -52,7 +52,7 @@ export function getTerminalEnvVars(
     const pathVarSeparator = targetPlatform === 'win32' ? ';' : ':';
     const joinPath = targetPlatform === 'win32' ? path.win32.join : path.posix.join;
 
-    const proxyUrl = `http://${httpToolkitIp}:${proxyPort}`;
+    const proxyUrl = `http://${httpToolkitHost}:${proxyPort}`;
 
     const dockerHost = `${
         targetPlatform === 'win32'
@@ -73,7 +73,7 @@ export function getTerminalEnvVars(
 
     const javaAgentOption = `-javaagent:"${
         joinPath(overridePath, JAVA_AGENT_JAR)
-    }"=${httpToolkitIp}|${proxyPort}|${httpsConfig.certPath}`;
+    }"=${httpToolkitHost}|${proxyPort}|${httpsConfig.certPath}`;
 
     const binPath = joinPath(overridePath, BIN_OVERRIDE_DIR);
 
