From 39ba4bd0c0b9016cab7e5be0c2e123c3ee0b9421 Mon Sep 17 00:00:00 2001 From: unohee Date: Thu, 10 Sep 2026 19:55:06 +0900 Subject: [PATCH] fix(web): make the Tailscale trust path usable, and stop it trusting carrier NAT (AGT-4294) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects on the same trust boundary. One made the path unusable; the other trusted clients that should never have been trusted. A trusted request reaches `POST /api/exec`, so the second is a security defect. **Nobody could register.** `isAuthorizedTailscalePeer` accepted only the IPv6 ULA form, but `tailscale status` prints the CGNAT IPv4 address and MagicDNS is commonly off — and `authorizedTailscalePeers`' own doc comment has always given a CGNAT address as a valid entry. Measured live today: browsing the daemon's CGNAT URL returns the page shell and 403 on every `/api/*` call, so the dashboard loads empty and asks for a token. Three more fail-closed spellings had the same symptom: `detectTailscaleIP` scanned IPv4 interfaces and then asked a predicate that only accepts IPv6, so it always returned undefined and the startup banner — the one place an operator reads how to connect — fell through to "token required"; the banner printed only the ULA; and `100.x:3847`, `::ffff:100.x`, `[addr]` and `[addr]:port` were all dropped silently. **Range membership is not identity.** `100.64.0.0/10` is also carrier-grade NAT (LTE, Starlink, many fixed ISPs) and a stock Kubernetes pod range (100.96/12). A daemon on a tethered uplink or in a pod would trust a listed peer address that reached it over the carrier network. The tailnet interface is now identified by the ULA it carries — that prefix is assigned by exactly one thing — and only Tailscale's own addresses on that interface count as a local end. Taking the whole interface would have let an on-link attacker land a rogue `fd7a:115c:a1e0::/48` router advertisement on `en0` and thereby promote that machine's plain LAN address to a trusted local end, reachable with no Origin header at all. With no ULA anywhere the check fails closed. That condition is not "IPv6 is disabled", which is what an earlier draft assumed — it is "no ULA right now", which `tailscale down` or a tailscaled restart satisfies on an ordinary dual-stack host, and the automatic fallback would then have degraded silently and permanently thirty seconds later. A host that genuinely cannot run IPv6 opts in with OPENSWARM_TAILSCALE_ALLOW_RANGE_LOCAL_END=true. Address spellings are canonicalised in one place, because the shape predicates strip `::ffff:` before matching and an identity compare that did not strip it would pass the shape check and then fail the allowlist — which is the same split that made the path unusable to begin with. Review: layer 2 (independent subagent), four rounds. R1 found the range MAJOR, R2 the silent fallback, R3 the interface-wide enrolment plus five smaller items; R4 returned APPROVE with no findings. Each fix is pinned by a mutant that kills tests. tsc --noEmit exit 0 · src/support/ 777 passed / 3 skipped, exit 0 · build exit 0 · oxlint 0 warnings on the changed files. Co-Authored-By: Claude Opus 5 (1M context) --- src/support/tailscaleNetwork.test.ts | 222 ++++++++++++++++++++++++++- src/support/tailscaleNetwork.ts | 196 +++++++++++++++++++++-- src/support/web.ts | 29 ++-- src/support/webAuth.test.ts | 126 ++++++++++++++- src/support/webAuth.ts | 20 ++- 5 files changed, 558 insertions(+), 35 deletions(-) diff --git a/src/support/tailscaleNetwork.test.ts b/src/support/tailscaleNetwork.test.ts index d843e274..92284d7b 100644 --- a/src/support/tailscaleNetwork.test.ts +++ b/src/support/tailscaleNetwork.test.ts @@ -1,5 +1,32 @@ -import { describe, expect, it } from 'vitest'; -import { isAuthorizedTailscalePeer, isTailscaleAddress } from './tailscaleNetwork.js'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +/** + * The interface list `isTailscaleLocalEnd` reads. + * + * It takes no injection point — it is the live-host lookup, which is the + * property under test — so the host is mocked instead. Assigning to this + * between assertions is how an outage is staged. + */ +let interfaces: Record = {}; +vi.mock('node:os', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, networkInterfaces: () => interfaces }; +}); + +import { + isAuthorizedTailscalePeer, isTailscaleAddress, isTailscaleCgnatAddress, + isTailscaleLocalEnd, isTailscaleShapedAddress, resetTailscaleInterfaceCacheForTests, + tailscaleInterfaceAddresses, +} from './tailscaleNetwork.js'; + +const ULA = 'fd7a:115c:a1e0::bc01:c823'; +const CGNAT = '100.95.200.28'; +/** One interface carrying both, exactly as tailscaled presents it. */ +const TAILNET_UP = { + lo0: [{ address: '127.0.0.1' }], + en0: [{ address: '192.168.50.43' }], + utun2: [{ address: CGNAT }, { address: ULA }, { address: 'fe80::9e76:eff:fe49:b36f' }], +}; describe('isTailscaleAddress', () => { it('should reject CGNAT addresses without explicit identity proof', () => { @@ -56,14 +83,201 @@ describe('isAuthorizedTailscalePeer', () => { delete process.env.OPENSWARM_TAILSCALE_PEERS; }); - it('never authorizes CGNAT addresses, even when allowlisted', () => { - process.env.OPENSWARM_TAILSCALE_PEERS = '100.64.0.1'; + it('accepts the bracketed IPv6 spelling this daemon prints in its own banner', () => { + // `web.ts` logs `http://[fd7a:...]:3847`. An operator copying the line + // they were just shown must land somewhere; dropping it silently is the + // same "still asked for a token" failure this path exists to remove. + for (const entry of [`[${PEER}]:3847`, `[${PEER}]`, PEER.toUpperCase(), `${PEER}%utun2`]) { + process.env.OPENSWARM_TAILSCALE_PEERS = entry; + expect(isAuthorizedTailscalePeer(PEER), entry).toBe(true); + } + delete process.env.OPENSWARM_TAILSCALE_PEERS; + }); + + it('refuses a CGNAT address that is not in the list — the range is never enough', () => { + // This is the property the old "never authorizes CGNAT" case was really + // protecting, and it is unchanged: 100.64.0.0/10 is shared with carrier + // NAT, so holding such an address proves nothing on its own. + process.env.OPENSWARM_TAILSCALE_PEERS = 'fd7a:115c:a1e0::b601:f469'; expect(isAuthorizedTailscalePeer('100.64.0.1')).toBe(false); delete process.env.OPENSWARM_TAILSCALE_PEERS; }); + it('authorizes a CGNAT address the operator named explicitly', () => { + // `tailscale status` prints the CGNAT address and MagicDNS is commonly + // off, so this is the address an operator actually has to hand. Refusing + // it outright left the trust path working and unusable: the machine that + // could reach the ULA was not the machine the browser was on (AGT-4294). + // The doc comment on `authorizedTailscalePeers` has always given a CGNAT + // address as an example of a valid entry. + process.env.OPENSWARM_TAILSCALE_PEERS = '100.126.196.94'; + expect(isAuthorizedTailscalePeer('100.126.196.94')).toBe(true); + expect(isAuthorizedTailscalePeer('100.126.196.95')).toBe(false); + delete process.env.OPENSWARM_TAILSCALE_PEERS; + }); + + it('accepts an entry copied from a URL bar or a log line', () => { + // Fail-closed either way, but both present as "still asked for a token", + // and both get likelier with CGNAT. + for (const entry of ['100.126.196.94:3847', '::ffff:100.126.196.94', '[100.126.196.94]', ' 100.126.196.94 ', '[100.126.196.94]:3847']) { + process.env.OPENSWARM_TAILSCALE_PEERS = entry; + expect(isAuthorizedTailscalePeer('100.126.196.94'), entry).toBe(true); + } + delete process.env.OPENSWARM_TAILSCALE_PEERS; + }); + + it('still refuses an address outside both Tailscale ranges, listed or not', () => { + // The list is named TAILSCALE_PEERS. Without a shape gate it would quietly + // become a general allowlist, and a LAN address would read as a tailnet + // peer. + for (const addr of ['192.168.50.99', '10.0.0.5', '100.63.255.255', '100.128.0.1', 'fd00::1']) { + process.env.OPENSWARM_TAILSCALE_PEERS = addr; + expect(isAuthorizedTailscalePeer(addr), addr).toBe(false); + } + delete process.env.OPENSWARM_TAILSCALE_PEERS; + }); + it('rejects empty and undefined addresses', () => { expect(isAuthorizedTailscalePeer(undefined)).toBe(false); expect(isAuthorizedTailscalePeer('')).toBe(false); }); }); + + +describe('isTailscaleCgnatAddress', () => { + it('accepts the range Tailscale hands out, and only its second octet 64-127', () => { + expect(isTailscaleCgnatAddress('100.64.0.1')).toBe(true); + expect(isTailscaleCgnatAddress('100.127.255.255')).toBe(true); + expect(isTailscaleCgnatAddress('100.63.255.255')).toBe(false); + expect(isTailscaleCgnatAddress('100.128.0.1')).toBe(false); + }); + + it('accepts the IPv4-mapped form, which is how a dual-stack bind delivers it', () => { + expect(isTailscaleCgnatAddress('::ffff:100.126.196.94')).toBe(true); + }); + + it('requires a whole dotted quad, not a prefix of one', () => { + // Unanchored, all of these passed. Nothing reaches it with a non-address + // today, but it is exported and the next caller may pass a Host header. + for (const bad of [ + '100.64.0.1.evil', '100.64.', '100.99.evil', '100.64.0.1:3847', + '100.64.257.1', '100.65.0.0/10', ' 100.64.0.1', '100.64.0.1\n', + ]) { + expect(isTailscaleCgnatAddress(bad), bad).toBe(false); + } + }); + + it('rejects nothing-like inputs', () => { + expect(isTailscaleCgnatAddress(undefined)).toBe(false); + expect(isTailscaleCgnatAddress('')).toBe(false); + expect(isTailscaleCgnatAddress('fd7a:115c:a1e0::1')).toBe(false); + }); +}); + +describe('isTailscaleShapedAddress', () => { + it('accepts either form Tailscale hands out, and nothing else', () => { + expect(isTailscaleShapedAddress('fd7a:115c:a1e0::1')).toBe(true); + expect(isTailscaleShapedAddress('100.64.0.1')).toBe(true); + expect(isTailscaleShapedAddress('192.168.1.1')).toBe(false); + expect(isTailscaleShapedAddress('fd00::1')).toBe(false); + }); +}); + +describe('tailscaleInterfaceAddresses', () => { + it('takes every address on the interface that carries the ULA, and no others', () => { + // The ULA prefix is assigned by exactly one thing, so the interface + // holding one IS the tailnet interface — and its CGNAT address is then + // known to be Tailscale's rather than merely shaped like it. + const got = tailscaleInterfaceAddresses({ + lo0: [{ address: '127.0.0.1' }], + en0: [{ address: '192.168.50.43' }], + // A carrier-NAT uplink: 100.x, but no ULA, so not the tailnet. + pdp_ip0: [{ address: '100.71.3.9' }], + // The link-local is on the tailnet interface and is NOT a Tailscale + // address. A real `utun` always has one, so a fixture without it cannot + // show what "every address on the interface" would have admitted. + utun2: [{ address: CGNAT }, { address: ULA }, { address: 'fe80::9e76:eff:fe49:b36f' }], + }); + + expect([...got].sort()).toEqual([CGNAT, ULA]); + expect(got.has('100.71.3.9')).toBe(false); + expect(got.has('fe80::9e76:eff:fe49:b36f')).toBe(false); + }); + + it('does not enrol an interface\'s other addresses when a rogue RA lands a ULA on it', () => { + // An on-link attacker can advertise `fd7a:115c:a1e0:dead::/64`; SLAAC then + // auto-configures a matching address on `en0` with no privilege on this + // host. Scoping to the interface must not promote that interface's plain + // LAN address to a trusted local end — a request to it carries no Origin, + // which `isTrustedLocalOrigin` allows by design, so the next hop would be + // `POST /api/exec`. + const got = tailscaleInterfaceAddresses({ + en0: [{ address: '192.168.50.196' }, { address: 'fd7a:115c:a1e0:dead::1' }], + }); + + expect(got.has('192.168.50.196')).toBe(false); + expect(got.has('fd7a:115c:a1e0:dead::1')).toBe(true); + }); + + it('finds nothing when no interface carries a ULA', () => { + expect(tailscaleInterfaceAddresses({ pdp_ip0: [{ address: '100.71.3.9' }] }).size).toBe(0); + }); +}); + +describe('isTailscaleLocalEnd', () => { + afterEach(() => { + interfaces = {}; + resetTailscaleInterfaceCacheForTests(); + vi.restoreAllMocks(); + }); + + it('accepts this host\'s tailnet addresses and nothing else on that interface', () => { + interfaces = TAILNET_UP; + resetTailscaleInterfaceCacheForTests(); + + expect(isTailscaleLocalEnd(CGNAT)).toBe(true); + expect(isTailscaleLocalEnd(ULA)).toBe(true); + // Same interface, not Tailscale's — see the rogue-RA case above. + expect(isTailscaleLocalEnd('fe80::9e76:eff:fe49:b36f')).toBe(false); + expect(isTailscaleLocalEnd('192.168.50.43')).toBe(false); + expect(isTailscaleLocalEnd(undefined)).toBe(false); + expect(isTailscaleLocalEnd('')).toBe(false); + }); + + it('expires the cache when the clock steps backwards', () => { + // A pinned entry is not merely stale here: pinning an empty set keeps the + // refusal engaged after the tailnet is back, and pinning a full one keeps + // trust alive after it is gone. `Math.abs` is what makes a backward NTP + // step expire rather than pin, and without this the mutant survives. + vi.spyOn(console, 'warn').mockImplementation(() => {}); + interfaces = {}; + resetTailscaleInterfaceCacheForTests(); + const t0 = 1_000_000; + expect(isTailscaleLocalEnd(CGNAT, t0)).toBe(false); + + interfaces = TAILNET_UP; + // Inside the TTL going forward: still the cached empty set. + expect(isTailscaleLocalEnd(CGNAT, t0 + 1_000)).toBe(false); + // A step backwards is |delta| > TTL, so the entry expires and is re-read. + expect(isTailscaleLocalEnd(CGNAT, t0 - 60_000)).toBe(true); + }); + + it('warns once per outage, not once per process', () => { + // The doc comment names a `tailscaled` restart as the case this branch is + // for, and a restart happens more than once. Latching the flag for the + // process lifetime would make every outage after the first one silent. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + interfaces = {}; + resetTailscaleInterfaceCacheForTests(); + expect(isTailscaleLocalEnd(CGNAT, 0)).toBe(false); + expect(isTailscaleLocalEnd(CGNAT, 1_000)).toBe(false); + expect(warn).toHaveBeenCalledTimes(1); + + interfaces = TAILNET_UP; + expect(isTailscaleLocalEnd(CGNAT, 60_000)).toBe(true); + + interfaces = {}; + expect(isTailscaleLocalEnd(CGNAT, 120_000)).toBe(false); + expect(warn).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/support/tailscaleNetwork.ts b/src/support/tailscaleNetwork.ts index a94ff2cf..5bdbbbf0 100644 --- a/src/support/tailscaleNetwork.ts +++ b/src/support/tailscaleNetwork.ts @@ -36,9 +36,25 @@ export function authorizedTailscalePeers(): ReadonlySet { export function canonicalIpv6(value: string): string { let text = value.trim().toLowerCase(); if (!text) return ''; - // A URL-bar copy keeps the brackets; a zone suffix names a local interface - // and is not part of the address identity. - if (text.startsWith('[') && text.endsWith(']')) text = text.slice(1, -1); + // A URL-bar copy keeps the brackets and the port. '[fd7a:...]:3847' is the + // exact spelling this daemon's own startup banner prints, so an operator who + // copies the line they were just shown has to land somewhere. + const bracketPort = /^\[([^\]]+)\](?::\d{1,5})?$/.exec(text); + if (bracketPort) text = bracketPort[1]; + // '100.64.0.1:3847' is what you copy from a URL bar and '::ffff:100.64.0.1' + // is what you copy from a log line on a dual-stack bind. Both used to fail + // closed and silently, which presents as "still asked for a token" — the + // symptom this path exists to remove. (AGT-4294) + const quadPort = /^(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}):\d{1,5}$/.exec(text); + if (quadPort) text = quadPort[1]; + // The shape predicates below strip a '::ffff:' prefix before matching, so the + // identity compare has to strip it too: otherwise a spelling passes the shape + // check and then fails the allowlist, which is the same split that made the + // trust path unusable in the first place (AGT-4290). + if (text.startsWith('::ffff:')) { + const rest = text.slice(7); + if (/^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.test(rest) || isIPv6(rest)) text = rest; + } const zone = text.indexOf('%'); if (zone !== -1) text = text.slice(0, zone); if (!isIPv6(text)) return text; @@ -60,7 +76,8 @@ export function canonicalIpv6(value: string): string { * `isAuthorizedTailscalePeer` (operator-configured allowlist) or the Tailscale * control plane (node key / capability check). * - * Only ULA addresses are accepted. CGNAT (100.64.0.0/10) is not trusted. + * Only ULA addresses match here. A CGNAT address is Tailscale-shaped too, but + * proving that takes the operator's allowlist — see `isAuthorizedTailscalePeer`. */ export function isTailscaleAddress(address: string | undefined): boolean { if (!address) return false; @@ -69,6 +86,145 @@ export function isTailscaleAddress(address: string | undefined): boolean { return normalized.toLowerCase().startsWith('fd7a:115c:a1e0:'); } +/** + * Tailscale's CGNAT range, 100.64.0.0/10. + * + * Shape only, and deliberately NOT trust: the range is shared with carrier- + * grade NAT, so a client behind one can hold such an address without being on + * any tailnet. It is meaningful only in combination with the operator's + * explicit peer allowlist, which is how `isAuthorizedTailscalePeer` uses it. + */ +export function isTailscaleCgnatAddress(address: string | undefined): boolean { + if (!address) return false; + const normalized = address.startsWith('::ffff:') ? address.slice(7) : address; + // Anchored and fully quad-shaped, like `isAllowedOrigin`. Unanchored, this + // accepted '100.64.0.1.evil', '100.64.0.1:3847' and '100.99.evil'. Every + // caller today passes a socket address, so nothing was reachable — but this + // is an exported "is this Tailscale" predicate and the next caller may hand + // it a Host header. + const m = /^100\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/.exec(normalized); + if (!m) return false; + if (m.slice(2).some(part => Number(part) > 255)) return false; + const octet = Number(m[1]); + return octet >= 64 && octet <= 127; +} + +/** Either shape Tailscale hands out. Shape is not trust — see the callers. */ +export function isTailscaleShapedAddress(address: string | undefined): boolean { + return isTailscaleAddress(address) || isTailscaleCgnatAddress(address); +} + +/** + * Every address on the interface that carries this host's Tailscale ULA. + * + * The ULA prefix `fd7a:115c:a1e0::/48` is assigned by exactly one thing, so + * the interface holding one IS the tailnet interface — and its CGNAT address + * is then known to be Tailscale's rather than merely shaped like it. + * + * That distinction is the whole point. `100.64.0.0/10` is also handed out by + * carrier-grade NAT (LTE, Starlink, many fixed ISPs) and by cloud CNIs + * (100.96/12 is a stock Kubernetes pod range), so "the connection arrived on a + * 100.x address of ours" does not mean "it arrived over Tailscale". A daemon on + * a tethered uplink or in a pod would otherwise trust a listed peer address + * reaching it over the carrier network. (AGT-4294) + * + * Empty when this host has no Tailscale ULA — see `isTailscaleLocalEnd`. + */ +export function tailscaleInterfaceAddresses( + interfaces: NodeJS.Dict<{ address: string }[]> = networkInterfaces(), +): ReadonlySet { + const found = new Set(); + for (const addresses of Object.values(interfaces)) { + const entries = addresses ?? []; + if (!entries.some(a => isTailscaleAddress(a.address))) continue; + // The interface identifies the tailnet; the addresses that count as a + // local end are still only Tailscale's own. Taking the whole interface + // would enrol its LAN IPv4 and its link-local, and an on-link attacker + // who lands a rogue `fd7a:115c:a1e0::/48` router advertisement on `en0` + // would thereby turn that machine's ordinary LAN address into a trusted + // local end — a request to it carries no Origin, which + // `isTrustedLocalOrigin` allows by design. + for (const a of entries) { + if (isTailscaleShapedAddress(a.address)) found.add(canonicalIpv6(a.address)); + } + } + return found; +} + +/** + * Cached interface lookup. + * + * This runs on every authorized request, and `networkInterfaces()` is a + * syscall. The TTL is short enough that a tailnet coming up mid-run is picked + * up without a restart, and long enough that a burst of requests costs one + * lookup. + */ +const INTERFACE_CACHE_MS = 30_000; +let cachedAddresses: ReadonlySet | undefined; +let cachedAt = 0; + +function currentTailscaleAddresses(now: number): ReadonlySet { + // abs, so an NTP step backwards expires the entry instead of pinning it — + // and a pinned empty set is what engages the refusal above. + if (cachedAddresses && Math.abs(now - cachedAt) < INTERFACE_CACHE_MS) return cachedAddresses; + cachedAddresses = tailscaleInterfaceAddresses(); + cachedAt = now; + // Re-arm the one-shot warning whenever the tailnet is back, so a second + // outage is reported too. Latching it for the process lifetime would make + // the `tailscale down` this branch exists for silent after the first time. + if (cachedAddresses.size > 0) warnedNoUla = false; + return cachedAddresses; +} + +/** + * Whether a connection arrived on this host's Tailscale interface. + * + * Fails closed when no ULA is present anywhere. Without one there is nothing + * to identify the tailnet interface by, so the only rule left is the range + * test — and that is exactly the rule interface scoping replaced, because + * 100.64.0.0/10 is also carrier-grade NAT and a stock Kubernetes pod range. + * + * An earlier version fell back to it automatically, which was wrong twice + * over. The condition is not "IPv6 is disabled", the case it was written for — + * it is "no ULA right now", which `tailscale down` or a `tailscaled` restart + * satisfies on an ordinary dual-stack host. Thirty seconds later the check + * would have silently degraded, permanently, on a daemon that was fine a + * moment earlier. And the degraded rule grants `POST /api/exec`. + * + * A host that genuinely cannot run IPv6 can opt in with + * OPENSWARM_TAILSCALE_ALLOW_RANGE_LOCAL_END=true, which is a choice an + * operator makes rather than a state they fall into. + */ +export function isTailscaleLocalEnd( + address: string | undefined, + now: number = Date.now(), +): boolean { + if (!address) return false; + const normalized = canonicalIpv6(address); + const onInterface = currentTailscaleAddresses(now); + if (onInterface.size > 0) return onInterface.has(normalized); + if (process.env.OPENSWARM_TAILSCALE_ALLOW_RANGE_LOCAL_END !== 'true') { + if (!warnedNoUla) { + warnedNoUla = true; + console.warn('[Tailscale] No Tailscale ULA on any interface, so the tailnet interface ' + + 'cannot be identified; refusing Tailscale trust. If this host cannot run IPv6, set ' + + 'OPENSWARM_TAILSCALE_ALLOW_RANGE_LOCAL_END=true — that accepts any 100.64/10 address ' + + 'of this host, which carrier NAT and Kubernetes pod networks also use.'); + } + return false; + } + return isTailscaleShapedAddress(normalized); +} + +let warnedNoUla = false; + +/** Drop the interface cache and the one-shot warning. */ +export function resetTailscaleInterfaceCacheForTests(): void { + warnedNoUla = false; + cachedAddresses = undefined; + cachedAt = 0; +} + /** * True only when the address is a Tailscale-shaped address AND appears in the * operator's explicit peer allowlist. This is the trust decision; range @@ -76,8 +232,13 @@ export function isTailscaleAddress(address: string | undefined): boolean { */ export function isAuthorizedTailscalePeer(address: string | undefined): boolean { if (!address) return false; - const normalized = canonicalIpv6(address.startsWith('::ffff:') ? address.slice(7) : address); - if (!isTailscaleAddress(normalized)) return false; + const normalized = canonicalIpv6(address); + // ULA or CGNAT: `tailscale status` prints the CGNAT address and MagicDNS is + // commonly off, so CGNAT is the address an operator actually has to hand. + // Refusing it outright meant the trust path existed and nobody could use it + // (AGT-4294). The range alone still proves nothing — membership in the + // operator's explicit list is what authorizes, exactly as before. + if (!isTailscaleShapedAddress(normalized)) return false; return authorizedTailscalePeers().has(normalized); } @@ -91,11 +252,26 @@ export function isAuthorizedTailscalePeer(address: string | undefined): boolean * daemon. That banner is what an operator reads to find out how to connect. */ export function detectTailscaleIP(): string | undefined { + const found = detectTailscaleAddresses(); + return found.ula ?? found.cgnat; +} + +/** + * Both addresses this host answers on, for the startup banner. + * + * The CGNAT one is what `tailscale status` prints and what an operator reaches + * for, so printing only the ULA made a trusted address undiscoverable — half + * of what AGT-4290 was reported as. (AGT-4294) + */ +export function detectTailscaleAddresses(): { ula?: string; cgnat?: string } { + const out: { ula?: string; cgnat?: string } = {}; for (const addresses of Object.values(networkInterfaces())) { - for (const address of addresses ?? []) { - if (address.internal) continue; - if (isTailscaleAddress(address.address)) return address.address; + const entries = (addresses ?? []).filter(a => !a.internal); + if (!entries.some(a => isTailscaleAddress(a.address))) continue; + for (const a of entries) { + if (isTailscaleAddress(a.address)) out.ula ??= a.address; + else if (isTailscaleCgnatAddress(a.address)) out.cgnat ??= a.address; } } - return undefined; + return out; } diff --git a/src/support/web.ts b/src/support/web.ts index ee3ee6bb..0e709d08 100644 --- a/src/support/web.ts +++ b/src/support/web.ts @@ -25,7 +25,7 @@ import { getAllProcesses, killProcess, startHealthChecker, stopHealthChecker } f import { setDefaultAdapter, isKnownAdapter, listAdapterNames } from '../adapters/index.js'; import { writeProviderOverride } from '../core/providerOverride.js'; import * as memory from '../memory/index.js'; -import { detectTailscaleIP, isTailscaleAddress } from './tailscaleNetwork.js'; +import { detectTailscaleAddresses } from './tailscaleNetwork.js'; export { detectTailscaleIP, isTailscaleAddress, isAuthorizedTailscalePeer } from './tailscaleNetwork.js'; import { runChatCompletion, getDefaultChatModel } from './chatBackend.js'; import { handleGraphQL, isGraphQLRequest } from '../issues/graphql/server.js'; @@ -1412,13 +1412,11 @@ export async function startWebServer(port: number = 3847): Promise { }); const trustTailscale = process.env.OPENSWARM_TRUST_TAILSCALE === 'true'; - // '::' rather than '0.0.0.0', and the difference decides whether the - // Tailscale trust path is reachable at all. `isTailscaleAddress` trusts - // ONLY the IPv6 ULA prefix — CGNAT is refused on purpose, because - // 100.64.0.0/10 is shared with carriers and proves no identity. Binding - // IPv4-only left that the one trusted address shape nothing could connect - // to, so an operator who had allowlisted their peer exactly was still - // asked for a token on every remote request (AGT-4290). + // '::' rather than '0.0.0.0'. The Tailscale ULA is IPv6, and binding + // IPv4-only left it unreachable — an operator who had allowlisted their + // peer exactly was still asked for a token on every remote request + // (AGT-4290). CGNAT peers are also trusted now, when explicitly listed + // (AGT-4294), but the ULA remains how the tailnet interface is identified. // // Node defaults to dual-stack (ipv6Only false), so IPv4 clients keep // working and arrive as '::ffff:…'. The auth layer already expects that @@ -1443,7 +1441,9 @@ export async function startWebServer(port: number = 3847): Promise { } else if (!triedIpv4Fallback && listenHost === ALL_INTERFACES && IPV6_UNAVAILABLE.has(err.code ?? '')) { triedIpv4Fallback = true; console.warn(`[Web] IPv6 unavailable (${err.code}); falling back to 0.0.0.0. ` - + 'Tailscale trust requires IPv6 and will not work on this host.'); + + 'Tailscale trust still works over CGNAT, but without IPv6 there is no ULA to ' + + 'identify the tailnet interface by, so it is refused unless ' + + 'OPENSWARM_TAILSCALE_ALLOW_RANGE_LOCAL_END=true. See tailscaleNetwork.ts.'); server?.listen(port, '0.0.0.0'); } else { reject(err); @@ -1451,7 +1451,7 @@ export async function startWebServer(port: number = 3847): Promise { }); server.listen(port, listenHost, () => { - const tailscaleIP = detectTailscaleIP(); + const tailscaleAddrs = detectTailscaleAddresses(); console.log(`Web interface running at:`); console.log(` - http://127.0.0.1:${port} (localhost)`); if (listenHost === ALL_INTERFACES) { @@ -1460,12 +1460,17 @@ export async function startWebServer(port: number = 3847): Promise { : 'token required'; // The ULA is IPv6, so it needs brackets to be a usable URL — this line // is what an operator copies into a browser. - if (tailscaleIP) console.log(` - http://[${tailscaleIP}]:${port} (${access})`); + // Both, and the CGNAT one first: it is what `tailscale status` prints, + // so it is the address an operator reaches for. (AGT-4294) + if (tailscaleAddrs.cgnat) console.log(` - http://${tailscaleAddrs.cgnat}:${port} (${access})`); + if (tailscaleAddrs.ula) console.log(` - http://[${tailscaleAddrs.ula}]:${port} (${access})`); // No Tailscale address found. Say what auth actually applies rather // than always claiming a token: with trust on and no token configured // there is no token to present, and that misdirection is what // AGT-4290 was reported as. - else console.log(` - http://:${port} (${access})`); + if (!tailscaleAddrs.cgnat && !tailscaleAddrs.ula) { + console.log(` - http://:${port} (${access})`); + } } gitStatusPoller = startGitStatusPoller(() => Array.from(pinnedProjects)); startHealthCache(); diff --git a/src/support/webAuth.test.ts b/src/support/webAuth.test.ts index 571fab9e..632e30f6 100644 --- a/src/support/webAuth.test.ts +++ b/src/support/webAuth.test.ts @@ -15,7 +15,25 @@ // form was refused here, because a bracketed IPv6 hostname matched none of the // allowed shapes. A test on either function alone would have passed. -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, onTestFinished, vi } from 'vitest'; + +// The Tailscale path now identifies its interface by the ULA that interface +// carries (AGT-4294), so these tests must name the host's addresses rather +// than inherit whatever the machine running them happens to have. +const DAEMON_ULA = 'fd7a:115c:a1e0::bc01:c823'; +const DAEMON_CGNAT = '100.95.200.28'; +vi.mock('node:os', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + networkInterfaces: () => ({ + lo0: [{ address: '127.0.0.1' }], + en0: [{ address: '192.168.50.43' }], + // One interface carrying both, exactly as tailscaled presents it. + utun2: [{ address: DAEMON_CGNAT }, { address: DAEMON_ULA }], + }), + }; +}); import type { IncomingMessage } from 'node:http'; import { @@ -44,7 +62,8 @@ function req(opts: { const ORIGINAL = process.env.OPENSWARM_WEB_TOKEN; const ORIGINAL_TRUST = process.env.OPENSWARM_TRUST_TAILSCALE; const ORIGINAL_PEERS = process.env.OPENSWARM_TAILSCALE_PEERS; -beforeEach(() => { +beforeEach(async () => { + (await import('./tailscaleNetwork.js')).resetTailscaleInterfaceCacheForTests(); delete process.env.OPENSWARM_WEB_TOKEN; delete process.env.OPENSWARM_TRUST_TAILSCALE; delete process.env.OPENSWARM_TAILSCALE_PEERS; @@ -295,13 +314,112 @@ describe('isAuthorizedMutation / isAuthorizedLocalRead', () => { } }); - it('still refuses CGNAT, even allowlisted — reaching IPv6 must not widen trust', () => { + it('lets an allowlisted CGNAT peer read, arriving on our CGNAT address', () => { + // The address `tailscale status` prints, which is the one an operator + // types. Both ends are CGNAT here: the browser's source and the daemon's + // own Tailscale address it connected to (AGT-4294). + process.env.OPENSWARM_TRUST_TAILSCALE = 'true'; + process.env.OPENSWARM_TAILSCALE_PEERS = '100.126.196.94'; + const r = req({ + remote: '100.126.196.94', + local: '100.95.200.28', + origin: 'http://100.95.200.28:3847', + host: '100.95.200.28:3847', + }); + expect(isAuthorizedLocalRead(r)).toBe(true); + }); + + it('refuses Tailscale trust when no ULA identifies the tailnet interface', async () => { + // Reached by `tailscale down` or a tailscaled restart, not only by IPv6 + // being disabled. Falling back to the range test there would trust a + // listed peer arriving on a pod IP or a carrier-NAT uplink — the exact + // widening interface scoping exists to prevent. + const os = await import('node:os'); + const spy = vi.spyOn(os, 'networkInterfaces').mockReturnValue({ + eth0: [{ address: '100.96.4.17' }], tailscale0: [{ address: '100.95.200.28' }], + } as never); + (await import('./tailscaleNetwork.js')).resetTailscaleInterfaceCacheForTests(); + vi.spyOn(console, 'warn').mockImplementation(() => {}); + onTestFinished(async () => { + spy.mockRestore(); + (await import('./tailscaleNetwork.js')).resetTailscaleInterfaceCacheForTests(); + }); + process.env.OPENSWARM_TRUST_TAILSCALE = 'true'; + process.env.OPENSWARM_TAILSCALE_PEERS = '100.126.196.94'; + + const r = req({ remote: '100.126.196.94', local: '100.96.4.17', host: '100.96.4.17:3847' }); + expect(isAuthorizedLocalRead(r)).toBe(false); + expect(isAuthorizedMutation(r)).toBe(false); + + // ...unless the operator explicitly accepts the weaker rule. + process.env.OPENSWARM_TAILSCALE_ALLOW_RANGE_LOCAL_END = 'true'; + onTestFinished(() => { delete process.env.OPENSWARM_TAILSCALE_ALLOW_RANGE_LOCAL_END; }); + expect(isAuthorizedLocalRead(r)).toBe(true); + }); + + it('accepts the IPv4-mapped form both ends actually arrive in', () => { + // The daemon binds '::' (AGT-4290), so an IPv4 client's remoteAddress AND + // localAddress both arrive '::ffff:'-prefixed — verified against a real + // dual-stack socket. Every other CGNAT case here uses the bare form, so + // deleting the strip that makes production work left the suite green. + process.env.OPENSWARM_TRUST_TAILSCALE = 'true'; + process.env.OPENSWARM_TAILSCALE_PEERS = '100.126.196.94'; + const r = req({ + remote: '::ffff:100.126.196.94', + local: '::ffff:100.95.200.28', + host: '100.95.200.28:3847', + }); + expect(isAuthorizedLocalRead(r)).toBe(true); + expect(isAuthorizedMutation(r)).toBe(true); + }); + + it('refuses a listed peer that reached a 100.x address which is not ours', () => { + // The reason the local end is matched against the interface rather than + // the range: 100.64.0.0/10 is also carrier-grade NAT and a stock k8s pod + // range, so a tethered or containerised daemon would otherwise trust a + // listed peer arriving over the carrier network. + process.env.OPENSWARM_TRUST_TAILSCALE = 'true'; + process.env.OPENSWARM_TAILSCALE_PEERS = '100.126.196.94'; + for (const local of ['100.71.3.9', '100.96.4.17']) { + const r = req({ remote: '100.126.196.94', local, host: `${local}:3847` }); + expect(isAuthorizedLocalRead(r), local).toBe(false); + expect(isAuthorizedMutation(r), local).toBe(false); + } + }); + + it('refuses a spoofed peer that reached us on a LAN address with no Origin', () => { + // The case the local-end check actually exists for. A browser cannot + // reach it — a LAN Origin is not on the allowlist and a cross-host Origin + // fails the CSRF match — but a non-browser client sends no Origin at all, + // and `isTrustedLocalOrigin` allows that by design (same-origin GETs do + // not send one). So with a self-assigned allowlisted source aimed at our + // LAN address, nothing else in the chain says no. + // + // 100.64.0.0/10 is not internet-routable, so arriving on our Tailscale + // address is what says the packet came through the tailnet. + process.env.OPENSWARM_TRUST_TAILSCALE = 'true'; + process.env.OPENSWARM_TAILSCALE_PEERS = '100.126.196.94'; + const spoofed = req({ remote: '100.126.196.94', local: '192.168.50.43', host: '192.168.50.43:3847' }); + expect(isAuthorizedLocalRead(spoofed)).toBe(false); + expect(isAuthorizedMutation(spoofed)).toBe(false); + + // Same request, arriving on the daemon's own Tailscale address: allowed. + const viaTailnet = req({ remote: '100.126.196.94', local: '100.95.200.28', host: '100.95.200.28:3847' }); + expect(isAuthorizedLocalRead(viaTailnet)).toBe(true); + }); + + it('still refuses CGNAT that the operator never listed', () => { // 100.64.0.0/10 is shared with carriers, so the address proves no // identity. Binding dual-stack must not turn that judgement over. process.env.OPENSWARM_TRUST_TAILSCALE = 'true'; - process.env.OPENSWARM_TAILSCALE_PEERS = '100.123.244.103'; + process.env.OPENSWARM_TAILSCALE_PEERS = 'fd7a:115c:a1e0::b601:f469'; const r = req({ remote: '100.123.244.103', + // Arriving on our own Tailscale address, so the local-end check passes + // and the allowlist is the only thing that can refuse this. Without + // this line the default '127.0.0.1' refused it for the wrong reason and + // deleting the allowlist check left the test green. + local: '100.95.200.28', origin: 'http://100.95.200.28:3847', host: '100.95.200.28:3847', }); diff --git a/src/support/webAuth.ts b/src/support/webAuth.ts index 5ecd1b17..23388257 100644 --- a/src/support/webAuth.ts +++ b/src/support/webAuth.ts @@ -9,7 +9,7 @@ // handling in a 1650-line file, and the surface most worth reading on its own. import type { IncomingMessage, ServerResponse } from 'node:http'; -import { isAuthorizedTailscalePeer, isLoopbackAddress, isTailscaleAddress } from './tailscaleNetwork.js'; +import { isAuthorizedTailscalePeer, isLoopbackAddress, isTailscaleLocalEnd } from './tailscaleNetwork.js'; import { isGraphQLRequest } from '../issues/graphql/server.js'; // CORS origin allowlist — hostname-strict match (no substring/prefix pitfalls) @@ -58,10 +58,20 @@ export function isTrustedTailscaleRequest(req: IncomingMessage): boolean { // means the packet was addressed to us through the tailnet, not to our LAN // address with a forged source. // - // This is defence in depth, not proof: an on-link attacker who can also - // route our ULA prefix defeats it. Real proof needs the Tailscale control - // plane (node key / capability check), which this process does not talk to. - && isTailscaleAddress(req.socket.localAddress) + // ...on the Tailscale interface, identified by the ULA it carries rather + // than by address range. "Not globally routable" is not "same adjacency": + // 100.64.0.0/10 is also carrier-grade NAT and a stock Kubernetes pod + // range, so a range test would trust a listed peer reaching a tethered or + // containerised daemon over the carrier network. (AGT-4294) + // + // Still defence in depth, not proof: an on-link attacker who can deliver a + // frame to this host's MAC defeats it either way. Real proof needs the + // Tailscale control plane — tailscaled's local API answers `whois` for a + // source address — which this process does not talk to yet. + // + // The bar matters because trust here reaches POST /api/exec, which hands + // an arbitrary prompt to a coding agent with shell access on this host. + && isTailscaleLocalEnd(req.socket.localAddress) && isTrustedLocalOrigin(req); }