diff --git a/src/config-writer-dns-isolation.test.ts b/src/config-writer-dns-isolation.test.ts index 7a933947a..7cfb6b814 100644 --- a/src/config-writer-dns-isolation.test.ts +++ b/src/config-writer-dns-isolation.test.ts @@ -13,6 +13,8 @@ // Hoisted jest.mock() registrations live in the shared helper — must remain first. import './test-helpers/config-writer-dependency-mocks.test-utils'; +import { EventEmitter } from 'events'; +import * as net from 'net'; import { writeConfigs } from './config-writer'; import { buildWriteConfig, @@ -70,7 +72,31 @@ describe('writeConfigs — DNS filtering in network-isolation mode', () => { }); }); - describe('isolation mode + auto-detected DNS — non-portable servers are filtered', () => { + describe('isolation mode + auto-detected DNS — non-portable servers are checked', () => { + it('retains reachable GKE NodeLocal DNS in the Squid config', async () => { + const socket = new EventEmitter() as EventEmitter & { + destroy: jest.Mock; + setTimeout: jest.Mock; + }; + socket.destroy = jest.fn(); + socket.setTimeout = jest.fn(); + (net.createConnection as jest.Mock).mockImplementationOnce(() => { + process.nextTick(() => socket.emit('connect')); + return socket; + }); + + await writeConfigs( + buildWriteConfig(tempDir, { + networkIsolation: true, + dnsServers: ['169.254.20.10'], + dnsServersExplicit: false, + }) + ); + + const squidCall = getSquidConfigMock().generateSquidConfig.mock.calls[0][0]; + expect(squidCall.dnsServers).toEqual(['169.254.20.10']); + }); + it('filters Azure DHCP DNS from Squid config', async () => { await writeConfigs( buildWriteConfig(tempDir, { diff --git a/src/config-writer.ts b/src/config-writer.ts index 354404e73..88cb3a6b9 100644 --- a/src/config-writer.ts +++ b/src/config-writer.ts @@ -325,13 +325,13 @@ export async function writeConfigs(config: WrapperConfig): Promise { // DNS server address), DNS servers that depend on host-specific routing — such // as Azure DHCP DNS (168.63.129.16) or Tailscale Magic DNS (100.100.100.100) — // can become unreachable from the Docker bridge, causing every Squid DNS lookup - // to fail with TCP_TUNNEL:HIER_NONE 503. Filter them out in isolation mode when - // the DNS list was auto-detected (not explicitly supplied by the operator via - // --dns-servers), so Squid falls back to publicly-routable servers that are not - // affected by VPN route changes. Explicitly-specified servers are trusted as-is. + // to fail with TCP_TUNNEL:HIER_NONE 503. Probe them in isolation mode when the + // DNS list was auto-detected (not explicitly supplied by the operator via + // --dns-servers), retaining reachable resolvers and filtering unreachable ones. + // Explicitly-specified servers are trusted as-is. const resolvedDnsServers = config.dnsServers ?? DEFAULT_DNS_SERVERS; const squidDnsServers = config.networkIsolation && !config.dnsServersExplicit - ? filterForNetworkIsolation(resolvedDnsServers, logger) + ? await filterForNetworkIsolation(resolvedDnsServers, logger) : resolvedDnsServers; // Note: Use container path for SSL database since it's mounted at /var/spool/squid_ssl_db diff --git a/src/dns-resolver.test.ts b/src/dns-resolver.test.ts index f4f9a79a3..8ee06dbe9 100644 --- a/src/dns-resolver.test.ts +++ b/src/dns-resolver.test.ts @@ -157,38 +157,56 @@ describe('isNonPortableDns', () => { }); describe('filterForNetworkIsolation', () => { - it('returns public DNS servers unchanged', () => { - const result = filterForNetworkIsolation(['8.8.8.8', '8.8.4.4'], mockLogger as any); + const unreachable = jest.fn().mockResolvedValue(false); + + it('returns public DNS servers unchanged', async () => { + const result = await filterForNetworkIsolation(['8.8.8.8', '8.8.4.4'], mockLogger as any, unreachable); expect(result).toEqual(['8.8.8.8', '8.8.4.4']); expect(mockLogger.warn).not.toHaveBeenCalled(); + expect(unreachable).not.toHaveBeenCalled(); }); - it('removes Azure DHCP DNS and warns', () => { - const result = filterForNetworkIsolation(['168.63.129.16'], mockLogger as any); + it('removes unreachable Azure DHCP DNS and warns', async () => { + const result = await filterForNetworkIsolation(['168.63.129.16'], mockLogger as any, unreachable); expect(result).toEqual(DEFAULT_DNS_SERVERS); expect(mockLogger.warn).toHaveBeenCalledWith( expect.stringContaining('168.63.129.16') ); }); - it('removes Tailscale Magic DNS and warns', () => { - const result = filterForNetworkIsolation(['100.100.100.100'], mockLogger as any); + it('removes unreachable Tailscale Magic DNS and warns', async () => { + const result = await filterForNetworkIsolation(['100.100.100.100'], mockLogger as any, unreachable); expect(result).toEqual(DEFAULT_DNS_SERVERS); expect(mockLogger.warn).toHaveBeenCalledWith( expect.stringContaining('100.100.100.100') ); }); - it('removes link-local DNS addresses', () => { - const result = filterForNetworkIsolation(['169.254.1.1'], mockLogger as any); + it('removes unreachable link-local DNS addresses', async () => { + const result = await filterForNetworkIsolation(['169.254.1.1'], mockLogger as any, unreachable); expect(result).toEqual(DEFAULT_DNS_SERVERS); expect(mockLogger.warn).toHaveBeenCalled(); }); - it('keeps portable servers when mixed with non-portable', () => { - const result = filterForNetworkIsolation( + it('retains a reachable link-local DNS address', async () => { + const reachable = jest.fn().mockResolvedValue(true); + const result = await filterForNetworkIsolation( + ['169.254.20.10'], + mockLogger as any, + reachable + ); + expect(result).toEqual(['169.254.20.10']); + expect(reachable).toHaveBeenCalledWith('169.254.20.10'); + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('retaining reachable') + ); + }); + + it('keeps portable servers when mixed with unreachable non-portable servers', async () => { + const result = await filterForNetworkIsolation( ['168.63.129.16', '8.8.8.8', '1.1.1.1'], - mockLogger as any + mockLogger as any, + unreachable ); expect(result).toEqual(['8.8.8.8', '1.1.1.1']); expect(mockLogger.warn).toHaveBeenCalledWith( @@ -196,27 +214,32 @@ describe('filterForNetworkIsolation', () => { ); }); - it('falls back to DEFAULT_DNS_SERVERS when all servers are non-portable', () => { - const result = filterForNetworkIsolation( + it('falls back to DEFAULT_DNS_SERVERS when all servers are unreachable', async () => { + const result = await filterForNetworkIsolation( ['168.63.129.16', '100.100.100.100', '169.254.1.1'], - mockLogger as any + mockLogger as any, + unreachable ); expect(result).toEqual(DEFAULT_DNS_SERVERS); // Two separate warn calls: one for filtering, one for fallback expect(mockLogger.warn).toHaveBeenCalledTimes(2); expect(mockLogger.warn).toHaveBeenCalledWith( - expect.stringContaining('no portable DNS servers remain') + expect.stringContaining('no reachable DNS servers remain') ); }); - it('keeps RFC1918 corporate DNS servers intact', () => { - const result = filterForNetworkIsolation(['10.0.0.1', '192.168.1.1'], mockLogger as any); + it('keeps RFC1918 corporate DNS servers intact', async () => { + const result = await filterForNetworkIsolation( + ['10.0.0.1', '192.168.1.1'], + mockLogger as any, + unreachable + ); expect(result).toEqual(['10.0.0.1', '192.168.1.1']); expect(mockLogger.warn).not.toHaveBeenCalled(); }); - it('uses default logger when no logger is provided', () => { + it('uses default logger when no logger is provided', async () => { // Should not throw when called without a logger - expect(() => filterForNetworkIsolation(['8.8.8.8'])).not.toThrow(); + await expect(filterForNetworkIsolation(['8.8.8.8'])).resolves.toEqual(['8.8.8.8']); }); }); diff --git a/src/dns-resolver.ts b/src/dns-resolver.ts index f18bf9544..f6f06868e 100644 --- a/src/dns-resolver.ts +++ b/src/dns-resolver.ts @@ -1,5 +1,5 @@ import * as fs from 'fs'; -import { isIP } from 'net'; +import { createConnection, isIP } from 'net'; import { logger as defaultLogger } from './logger'; import { DEFAULT_DNS_SERVERS } from './config/network-policy'; @@ -25,6 +25,27 @@ export { DEFAULT_DNS_SERVERS }; */ const AZURE_DHCP_DNS = '168.63.129.16'; const TAILSCALE_MAGIC_DNS = '100.100.100.100'; +const DNS_REACHABILITY_TIMEOUT_MS = 1000; + +type DnsReachabilityProbe = (server: string) => Promise; + +function isDnsServerReachable(server: string): Promise { + return new Promise(resolve => { + const socket = createConnection({ host: server, port: 53 }); + let settled = false; + + const finish = (reachable: boolean) => { + if (settled) return; + settled = true; + socket.destroy(); + resolve(reachable); + }; + + socket.once('connect', () => finish(true)); + socket.once('error', () => finish(false)); + socket.setTimeout(DNS_REACHABILITY_TIMEOUT_MS, () => finish(false)); + }); +} /** * Returns true for DNS servers that are host-specific and may become unreachable @@ -57,35 +78,53 @@ export function isNonPortableDns(ip: string): boolean { * Magic DNS (100.100.100.100) can be black-holed, causing every Squid lookup to * fail with `TCP_TUNNEL:HIER_NONE 503`. * - * This function removes non-portable servers from the list. If no portable - * servers remain, it falls back to DEFAULT_DNS_SERVERS (8.8.8.8, 8.8.4.4), - * which are publicly routable and independent of host-specific network paths. + * This function removes non-portable servers only when a bounded TCP/53 probe + * confirms they are unreachable. If no usable servers remain, it falls back to + * DEFAULT_DNS_SERVERS (8.8.8.8, 8.8.4.4). * * @param servers - The resolved DNS server list (from --dns-servers or auto-detection). * @param logger - Optional logger for diagnostic output. + * @param probe - Optional reachability probe for tests. * @returns A filtered list of DNS servers safe for use from a Docker bridge. */ -export function filterForNetworkIsolation(servers: string[], logger?: Logger): string[] { +export async function filterForNetworkIsolation( + servers: string[], + logger?: Logger, + probe: DnsReachabilityProbe = isDnsServerReachable +): Promise { const log = logger ?? defaultLogger; const nonPortable = servers.filter(isNonPortableDns); const portable = servers.filter(s => !isNonPortableDns(s)); - - if (nonPortable.length > 0) { + const reachability = await Promise.all(nonPortable.map(async server => ({ + server, + reachable: await probe(server), + }))); + const reachableNonPortable = reachability.filter(result => result.reachable).map(result => result.server); + const unreachableNonPortable = reachability.filter(result => !result.reachable).map(result => result.server); + + if (unreachableNonPortable.length > 0) { log.warn( - `Network-isolation: removing ${nonPortable.length} non-portable DNS server(s) ` + - `that may become unreachable from Docker bridge containers when host routing ` + - `is modified by tools like Tailscale: ${nonPortable.join(', ')}` + `Network-isolation: removing ${unreachableNonPortable.length} unreachable non-portable DNS server(s): ` + + `${unreachableNonPortable.join(', ')}` ); } - if (portable.length > 0) { - return portable; + if (reachableNonPortable.length > 0) { + log.warn( + `Network-isolation: retaining reachable non-portable DNS server(s): ` + + `${reachableNonPortable.join(', ')}` + ); } + const usable = servers.filter(server => + portable.includes(server) || reachableNonPortable.includes(server) + ); + if (usable.length > 0) return usable; + // All detected servers are non-portable — fall back to public DNS. log.warn( - `Network-isolation: no portable DNS servers remain after filtering; ` + + `Network-isolation: no reachable DNS servers remain after filtering; ` + `falling back to ${DEFAULT_DNS_SERVERS.join(', ')}. ` + `If your environment requires specific DNS, use --dns-servers to override.` ); diff --git a/src/test-helpers/config-writer-dependency-mocks.test-utils.ts b/src/test-helpers/config-writer-dependency-mocks.test-utils.ts index cf1974942..f9262a79b 100644 --- a/src/test-helpers/config-writer-dependency-mocks.test-utils.ts +++ b/src/test-helpers/config-writer-dependency-mocks.test-utils.ts @@ -20,3 +20,18 @@ jest.mock('../squid-config', () => require('./config-writer-test-harness.test-ut // eslint-disable-next-line @typescript-eslint/no-require-imports jest.mock('../compose-generator', () => require('./config-writer-test-harness.test-utils').composeGeneratorMockFactory()); + +jest.mock('net', () => { + const actual = jest.requireActual('net'); + return { + ...actual, + createConnection: jest.fn(() => { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const socket = new (require('events').EventEmitter)(); + socket.destroy = jest.fn(); + socket.setTimeout = jest.fn(); + process.nextTick(() => socket.emit('error', new Error('unreachable'))); + return socket; + }), + }; +});