diff --git a/src/config-writer-dns-isolation.test.ts b/src/config-writer-dns-isolation.test.ts index 7cfb6b814..fa6f1fa3b 100644 --- a/src/config-writer-dns-isolation.test.ts +++ b/src/config-writer-dns-isolation.test.ts @@ -1,20 +1,18 @@ /** - * Config-writer integration tests for DNS filtering in network-isolation mode. + * Config-writer integration tests for DNS preservation in network-isolation mode. * * Covers the gate at src/config-writer.ts lines 330-333: - * - Non-portable resolvers are filtered only when networkIsolation is enabled - * and dnsServersExplicit is false (auto-detected). - * - The filtered (effective) DNS list is passed to generateSquidConfig. - * - The filtered (effective) DNS list is also used in the policy-manifest audit + * - Auto-detected resolvers are preserved when networkIsolation is enabled + * and dnsServersExplicit is false. + * - The effective DNS list is passed to generateSquidConfig. + * - The effective DNS list is also used in the policy-manifest audit * artifact, not the raw config.dnsServers list. - * - Explicitly-supplied DNS servers are never filtered in isolation mode. + * - Explicitly-supplied DNS servers are also preserved in isolation mode. */ // 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, @@ -31,7 +29,7 @@ function getSquidConfigMock() { }; } -describe('writeConfigs — DNS filtering in network-isolation mode', () => { +describe('writeConfigs — DNS preservation in network-isolation mode', () => { let tempDir: string; beforeEach(() => { @@ -72,19 +70,8 @@ describe('writeConfigs — DNS filtering in network-isolation mode', () => { }); }); - 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; - }); - + describe('isolation mode + auto-detected DNS — runner resolvers are preserved', () => { + it('retains GKE NodeLocal DNS in the Squid config', async () => { await writeConfigs( buildWriteConfig(tempDir, { networkIsolation: true, @@ -97,7 +84,7 @@ describe('writeConfigs — DNS filtering in network-isolation mode', () => { expect(squidCall.dnsServers).toEqual(['169.254.20.10']); }); - it('filters Azure DHCP DNS from Squid config', async () => { + it('preserves Azure DHCP DNS in Squid config', async () => { await writeConfigs( buildWriteConfig(tempDir, { networkIsolation: true, @@ -105,13 +92,11 @@ describe('writeConfigs — DNS filtering in network-isolation mode', () => { dnsServersExplicit: false, }) ); - const squidCall = getSquidConfigMock().generateSquidConfig.mock.calls[0][0]; - // 168.63.129.16 is non-portable; fallback to default public DNS - expect(squidCall.dnsServers).toEqual(['8.8.8.8', '8.8.4.4']); + expect(squidCall.dnsServers).toEqual(['168.63.129.16']); }); - it('filters Tailscale Magic DNS from Squid config', async () => { + it('preserves Tailscale Magic DNS in Squid config', async () => { await writeConfigs( buildWriteConfig(tempDir, { networkIsolation: true, @@ -121,10 +106,10 @@ describe('writeConfigs — DNS filtering in network-isolation mode', () => { ); const squidCall = getSquidConfigMock().generateSquidConfig.mock.calls[0][0]; - expect(squidCall.dnsServers).toEqual(['8.8.8.8', '8.8.4.4']); + expect(squidCall.dnsServers).toEqual(['100.100.100.100']); }); - it('keeps portable servers from a mixed list and removes non-portable ones', async () => { + it('preserves mixed resolver lists without removing detected entries', async () => { await writeConfigs( buildWriteConfig(tempDir, { networkIsolation: true, @@ -134,10 +119,10 @@ describe('writeConfigs — DNS filtering in network-isolation mode', () => { ); const squidCall = getSquidConfigMock().generateSquidConfig.mock.calls[0][0]; - expect(squidCall.dnsServers).toEqual(['8.8.8.8', '1.1.1.1']); + expect(squidCall.dnsServers).toEqual(['168.63.129.16', '8.8.8.8', '1.1.1.1']); }); - it('passes the filtered list (not config.dnsServers) to the policy manifest', async () => { + it('passes the preserved list to the policy manifest', async () => { await writeConfigs( buildWriteConfig(tempDir, { networkIsolation: true, @@ -147,9 +132,7 @@ describe('writeConfigs — DNS filtering in network-isolation mode', () => { ); const manifestCall = getSquidConfigMock().generatePolicyManifest.mock.calls[0][0]; - // Policy manifest must reflect what Squid actually uses, not the raw detected list - expect(manifestCall.dnsServers).toEqual(['8.8.8.8']); - expect(manifestCall.dnsServers).not.toContain('168.63.129.16'); + expect(manifestCall.dnsServers).toEqual(['168.63.129.16', '8.8.8.8']); }); }); diff --git a/src/config-writer.ts b/src/config-writer.ts index 88cb3a6b9..06c683acd 100644 --- a/src/config-writer.ts +++ b/src/config-writer.ts @@ -317,18 +317,11 @@ export async function writeConfigs(config: WrapperConfig): Promise { logger.debug(`Parsed ${urlPatterns.length} URL pattern(s) for SSL Bump filtering`); } - // In network-isolation (topology) mode the Squid container is dual-homed: it - // has a static IP on the internal `awf-net` network and an auto-assigned IP on - // the external `awf-ext` Docker bridge. All DNS queries leave through `awf-ext`. - // When the host's routing is later modified by tools like Tailscale (e.g. an - // accepted exit-node or subnet route that captures 0.0.0.0/0 or the specific - // 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. 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. + // In network-isolation (topology) mode, preserve auto-detected DNS resolvers + // as operator-controlled runner network settings. Enterprise and cloud + // resolvers may be private or virtual-network-specific, and replacing them + // with public defaults can break environments where public DNS is blocked. + // Explicitly-specified servers are also trusted as-is. const resolvedDnsServers = config.dnsServers ?? DEFAULT_DNS_SERVERS; const squidDnsServers = config.networkIsolation && !config.dnsServersExplicit ? await filterForNetworkIsolation(resolvedDnsServers, logger) diff --git a/src/dns-resolver.test.ts b/src/dns-resolver.test.ts index 57ffc94f5..c181f7db5 100644 --- a/src/dns-resolver.test.ts +++ b/src/dns-resolver.test.ts @@ -123,82 +123,49 @@ describe('DEFAULT_DNS_SERVERS', () => { }); describe('filterForNetworkIsolation', () => { - 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); + const result = await filterForNetworkIsolation(['8.8.8.8', '8.8.4.4'], mockLogger as any); expect(result).toEqual(['8.8.8.8', '8.8.4.4']); expect(mockLogger.warn).not.toHaveBeenCalled(); - expect(unreachable).not.toHaveBeenCalled(); }); - 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('preserves Azure DHCP DNS in network-isolation mode', async () => { + const result = await filterForNetworkIsolation(['168.63.129.16'], mockLogger as any); + expect(result).toEqual(['168.63.129.16']); + expect(mockLogger.warn).not.toHaveBeenCalled(); }); - 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('preserves Tailscale Magic DNS in network-isolation mode', async () => { + const result = await filterForNetworkIsolation(['100.100.100.100'], mockLogger as any); + expect(result).toEqual(['100.100.100.100']); + expect(mockLogger.warn).not.toHaveBeenCalled(); }); - 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('preserves link-local DNS addresses', async () => { + const result = await filterForNetworkIsolation(['169.254.1.1'], mockLogger as any); + expect(result).toEqual(['169.254.1.1']); + expect(mockLogger.warn).not.toHaveBeenCalled(); }); - 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 - ); + it('preserves GKE NodeLocal DNS', async () => { + const result = await filterForNetworkIsolation(['169.254.20.10'], mockLogger as any); expect(result).toEqual(['169.254.20.10']); - expect(reachable).toHaveBeenCalledWith('169.254.20.10'); - expect(mockLogger.warn).toHaveBeenCalledWith( - expect.stringContaining('retaining reachable') - ); + expect(mockLogger.warn).not.toHaveBeenCalled(); }); - it('keeps portable servers when mixed with unreachable non-portable servers', async () => { + it('preserves mixed resolver lists without substituting public DNS', async () => { const result = await filterForNetworkIsolation( ['168.63.129.16', '8.8.8.8', '1.1.1.1'], - mockLogger as any, - unreachable - ); - expect(result).toEqual(['8.8.8.8', '1.1.1.1']); - expect(mockLogger.warn).toHaveBeenCalledWith( - expect.stringContaining('168.63.129.16') - ); - }); - - 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, - 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 reachable DNS servers remain') + mockLogger as any ); + expect(result).toEqual(['168.63.129.16', '8.8.8.8', '1.1.1.1']); + expect(mockLogger.warn).not.toHaveBeenCalled(); }); it('keeps RFC1918 corporate DNS servers intact', async () => { const result = await filterForNetworkIsolation( ['10.0.0.1', '192.168.1.1'], - mockLogger as any, - unreachable + mockLogger as any ); expect(result).toEqual(['10.0.0.1', '192.168.1.1']); expect(mockLogger.warn).not.toHaveBeenCalled(); diff --git a/src/dns-resolver.ts b/src/dns-resolver.ts index fee0d83d0..7fd7b2c22 100644 --- a/src/dns-resolver.ts +++ b/src/dns-resolver.ts @@ -1,5 +1,5 @@ import * as fs from 'fs'; -import { createConnection, isIP } from 'net'; +import { isIP } from 'net'; import { logger as defaultLogger } from './logger'; import { DEFAULT_DNS_SERVERS } from './config/network-policy'; @@ -13,122 +13,29 @@ type Logger = typeof defaultLogger; export { DEFAULT_DNS_SERVERS }; /** - * DNS servers that are reachable only via host-specific network paths which - * may be disrupted by VPN tools like Tailscale that modify host routing after - * AWF starts. + * Preserves DNS servers for use in network-isolation (topology) mode. * - * - Azure DHCP DNS (168.63.129.16): intercepted by Azure hypervisor; unreachable - * outside Azure VNet or when policy-routing routes 0.0.0.0/0 via a Tailscale - * exit node or accepted subnet route. - * - Tailscale Magic DNS (100.100.100.100): only reachable via the tailscale0 - * interface; Docker bridge containers cannot reach it. - */ -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 - * from Docker bridge containers when the host's routing is modified by tools - * like Tailscale (e.g. when an exit node or accepted subnet route captures the - * default route or the path to these servers). - * - * Non-portable servers include: - * - Azure DHCP DNS (168.63.129.16) - * - Tailscale Magic DNS (100.100.100.100) - * - Link-local addresses (169.254.x.x / RFC 3927) — not routable over bridges - */ -function isNonPortableDns(ip: string): boolean { - if (ip === AZURE_DHCP_DNS) return true; - if (ip === TAILSCALE_MAGIC_DNS) return true; - if (ip.startsWith('169.254.')) return true; - return false; -} - -/** - * Filters DNS servers for use in network-isolation (topology) mode. + * The runner's resolver list is an operator-controlled network setting. In + * enterprise and cloud environments those resolvers may be private, + * virtual-network-specific, or otherwise intentionally not globally routable. + * A resolver not being publicly portable does not make it invalid for the + * runner's Docker topology. * - * In isolation mode the Squid proxy container is dual-homed: it has a static IP - * on the internal `awf-net` network and an auto-assigned IP on the external - * `awf-ext` Docker bridge. All DNS queries and upstream TCP connections leave - * through `awf-ext`. When the host's routing is later modified by a VPN tool - * such as Tailscale (e.g. via an accepted exit-node or subnet-route that covers - * `0.0.0.0/0` or the specific DNS server address), DNS queries from the Docker - * bridge to host-specific servers like Azure DNS (168.63.129.16) or Tailscale - * 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 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). + * Do not silently replace detected resolvers with public defaults here. Fallback + * to DEFAULT_DNS_SERVERS only belongs in host DNS detection when no resolver can + * be detected at all. * * @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. + * @returns The DNS servers AWF should pass through to Squid and containers. */ export async function filterForNetworkIsolation( servers: string[], - logger?: Logger, - probe: DnsReachabilityProbe = isDnsServerReachable + logger?: Logger ): Promise { const log = logger ?? defaultLogger; - - const nonPortable = servers.filter(isNonPortableDns); - const portable = servers.filter(s => !isNonPortableDns(s)); - 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 ${unreachableNonPortable.length} unreachable non-portable DNS server(s): ` + - `${unreachableNonPortable.join(', ')}` - ); - } - - 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 reachable DNS servers remain after filtering; ` + - `falling back to ${DEFAULT_DNS_SERVERS.join(', ')}. ` + - `If your environment requires specific DNS, use --dns-servers to override.` - ); - return [...DEFAULT_DNS_SERVERS]; + log.debug(`Network-isolation: preserving DNS server(s): ${servers.join(', ')}`); + return [...servers]; } /**