Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 126 additions & 0 deletions .github/audit/canaries/local-networks-mixed-family-blocklist.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
Domain: remote
Rule: S272 — only against IPv4 ranges
Expect: lib/src/host/remote/network-interfaces.ts
Source: reverses #1240 (one BlockList held both families, so an IPv6 range over ::ffff:0:0/96 admitted IPv4)

diff --git a/lib/src/host/remote/local-networks.test.ts b/lib/src/host/remote/local-networks.test.ts
index 621427f53..e31f4609e 100644
--- a/lib/src/host/remote/local-networks.test.ts
+++ b/lib/src/host/remote/local-networks.test.ts
@@ -295,27 +295,6 @@ describe('localNetworksPath', () => {
expect(path.refusal({ local: '100.97.229.25', remote: '100.101.7.8' })?.end).toBe('local');
});

- it('lets an IPv6 range over the IPv4-mapped block admit no IPv4 end, in the strip or the pair', () => {
- const offer = [
- 'v=0',
- 'a=candidate:1 1 udp 2113937151 203.0.113.7 51236 typ host',
- 'a=candidate:2 1 udp 2113937151 ::ffff:203.0.113.7 51237 typ host',
- 'a=candidate:3 1 udp 2113937151 2001:db8::7 51238 typ host',
- '',
- ].join('\r\n');
- for (const range of ['::/0', '::ffff:0:0/96']) {
- const path = localNetworksPath([range]);
- expect(candidatesIn(path.acceptRemote(offer)), range).toEqual(
- range === '::/0' ? ['a=candidate:3 1 UDP 2113937151 2001:db8::7 51238 typ host'] : [],
- );
- expect(candidatesIn(path.describe(ANSWER) ?? '').filter((line) => /\d+\.\d+\.\d+\.\d+ /.test(line)), range).toEqual([]);
- expect(path.refusal({ local: '192.168.86.160', remote: '203.0.113.7' })?.end, range).toBe('local');
- expect(path.refusal({ local: '::ffff:192.168.86.160', remote: '::ffff:203.0.113.7' })?.end, range).toBe('local');
- }
- // An IPv4 range beside it still decides IPv4 ends alone.
- expect(localNetworksPath(['::/0', ...LAN]).refusal({ local: '192.168.86.160', remote: '203.0.113.7' })?.end).toBe('remote');
- });
-
it('refuses a name, an mDNS name, a missing end, or no pair at all', () => {
const path = localNetworksPath(LAN);
for (const remote of ['phone.local', '0f1e2d3c-aaaa-bbbb-cccc-000000000000.local', null]) {
diff --git a/lib/src/host/remote/network-interfaces.test.ts b/lib/src/host/remote/network-interfaces.test.ts
index 69c2b30cc..575dde448 100644
--- a/lib/src/host/remote/network-interfaces.test.ts
+++ b/lib/src/host/remote/network-interfaces.test.ts
@@ -87,24 +87,12 @@ describe('allowedAddressTest', () => {
});

it('matches an IPv4-mapped IPv6 address against its IPv4 range', () => {
+ // `BlockList` compares it as IPv4 (Node 24, 2026-09), so no unwrap is needed.
expect(addressAllowed('::ffff:192.168.1.5', allowed)).toBe(true);
expect(addressAllowed('::ffff:c0a8:105', allowed)).toBe(true);
expect(addressAllowed('::ffff:192.168.2.5', allowed)).toBe(false);
});

- it('never admits an IPv4 address, mapped or not, through an IPv6 range', () => {
- // Node's `BlockList` matches IPv4 against an IPv6 subnet over `::ffff:0:0/96`.
- for (const range of ['::/0', '::/1', '::/64', '::ffff:0:0/96', '::ffff:808:0/120']) {
- for (const address of ['8.8.8.8', '::ffff:8.8.8.8', '::ffff:808:808', '192.168.1.5']) {
- expect(addressAllowed(address, [range]), `${address} in ${range}`).toBe(false);
- }
- }
- // The IPv6 range still admits the IPv6 it covers, and an IPv4 range beside it still holds.
- expect(addressAllowed('2001:db8::5', ['::/0'])).toBe(true);
- expect(addressAllowed('::ffff:192.168.1.5', ['::/0', ...allowed])).toBe(true);
- expect(addressAllowed('8.8.8.8', ['::/0', ...allowed])).toBe(false);
- });
-
it('allows no hostname, mDNS name, zoned address, or empty list', () => {
for (const address of ['router.local', '4f1d2c3e-aaaa-bbbb-cccc-000000000000.local', 'fe80::1%en0', '', 'localhost']) {
expect(addressAllowed(address, [...allowed, 'fe80::/10']), address).toBe(false);
diff --git a/lib/src/host/remote/network-interfaces.ts b/lib/src/host/remote/network-interfaces.ts
index d99b26365..b898820d5 100644
--- a/lib/src/host/remote/network-interfaces.ts
+++ b/lib/src/host/remote/network-interfaces.ts
@@ -87,45 +87,28 @@ export function canonicalCidr(text: string): string | null {
return cidr && `${cidr.network}/${cidr.prefix}`;
}

-/**
- * One `BlockList` per family. Node's `BlockList` matches an IPv4 address
- * against an IPv6 subnet covering `::ffff:0:0/96`, even when asked to check
- * it as `'ipv4'`, so one list holding both families would let `::/0` (or any
- * IPv6 range over the mapped block) admit every IPv4 address.
- */
-function blockListsOf(cidrs: readonly string[]): { ipv4: BlockList; ipv6: BlockList } {
- const lists = { ipv4: new BlockList(), ipv6: new BlockList() };
+function blockListOf(cidrs: readonly string[]): BlockList {
+ const list = new BlockList();
for (const text of cidrs) {
const cidr = parseCidr(text);
- if (cidr) lists[cidr.family].addSubnet(cidr.network, cidr.prefix, cidr.family);
+ if (cidr) list.addSubnet(cidr.network, cidr.prefix, cidr.family);
}
- return lists;
-}
-
-/** The dotted quad an IPv4-mapped IPv6 address (`::ffff:a.b.c.d`) carries, or `null`. */
-function mappedIpv4(address: string): string | null {
- const serialized = serializeIpv6(address);
- if (serialized === null) return null;
- const groups = ipv6Groups(serialized);
- if (groups.slice(0, 5).some((group) => group !== 0) || groups[5] !== 0xffff) return null;
- return [groups[6]! >> 8, groups[6]! & 0xff, groups[7]! >> 8, groups[7]! & 0xff].join('.');
+ return list;
}

/**
* Whether an address is an IP literal inside one of `cidrs`, which are parsed
* once for every address tested. A hostname, an mDNS name, or a zoned address
- * is in none; a CIDR that does not parse matches nothing. Each family is
- * checked only against its own ranges, and an IPv4-mapped IPv6 address is
- * checked as the IPv4 address it carries, so an IPv6 range never admits IPv4.
+ * is in none; a CIDR that does not parse matches nothing. An IPv4-mapped IPv6
+ * address matches its IPv4 range: `BlockList` compares it as IPv4, which
+ * `network-interfaces.test.ts` pins.
*/
export function allowedAddressTest(cidrs: readonly string[]): (address: string) => boolean {
- const lists = blockListsOf(cidrs);
+ const list = blockListOf(cidrs);
return (address) => {
const family = isIP(address);
if (family === 0 || address.includes('%')) return false;
- if (family === 4) return lists.ipv4.check(address, 'ipv4');
- const ipv4 = mappedIpv4(address);
- return ipv4 === null ? lists.ipv6.check(address, 'ipv6') : lists.ipv4.check(ipv4, 'ipv4');
+ return list.check(address, family === 4 ? 'ipv4' : 'ipv6');
};
}

2 changes: 1 addition & 1 deletion docs/specs/remote-network.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ Under `local` each runtime — a one-time link, or the persistent Burrow — is
- **Must strip every candidate outside the allowed networks from the Burrow's answer**, and send a default address outside them as `0.0.0.0`. **An answer left with no candidate refuses the attempt.**
- **Must strip the phone's offer the same way before the Burrow applies it**, a hostname, mDNS name, or unreadable candidate included, so its ICE agent sends no check and makes no lookup toward an address the level does not hold. **An offer left with no candidate is still answered**: the phone's checks reach the answer's candidates, and the pair forms peer-reflexive (rationale).
- **Must hand the ICE agent each candidate either strip keeps rebuilt from the fields it read**, its address in one spelling and its extensions dropped; a candidate line in any other spelling is unreadable (`docs/specs/security-remote.md` -> "Direct path"; rationale).
- **Must check the selected candidate pair on the Burrow before its channel reports open**, and again while it is open and `connected` — on every ICE or connection state change, and every `DIRECT_PATH_RECHECK_MS` (rationale): both ends parse as IP addresses — IPv4-mapped IPv6 matching its IPv4 range — each inside an allowed CIDR. **A hostname, an mDNS name, or a pair the stack cannot read refuses**, open or not, and a frame arriving before the open is checked first. **A reading of no pair refuses before the open; once open it is left to the connection's own state** (rationale).
- **Must check the selected candidate pair on the Burrow before its channel reports open**, and again while it is open and `connected` — on every ICE or connection state change, and every `DIRECT_PATH_RECHECK_MS` (rationale): both ends parse as IP addresses, each inside an allowed CIDR of its own family — IPv4-mapped IPv6 read as its IPv4 address, and never matched by an IPv6 range (rationale). **A hostname, an mDNS name, or a pair the stack cannot read refuses**, open or not, and a frame arriving before the open is checked first. **A reading of no pair refuses before the open; once open it is left to the connection's own state** (rationale).
- **Never trust SDP candidates, Hosted-observed addresses, or Client claims** as path evidence; only the Burrow's own ICE agent answers (rationale).
- **A refusal is a violation**: it ends the session `network-not-allowed` (`docs/specs/one-time.md` -> "Burrow runtime"), switched or not.
- **The check gates terminal traffic, not approval** (rationale): an off-network phone holding a link can reach the two-digit prompt and still receives no terminal byte.
Expand Down
2 changes: 2 additions & 0 deletions docs/specs/remote-network.rationale.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ Evidence for `docs/specs/remote-network.md`, keyed by its headings.

**Why each kept candidate is rebuilt.** libdatachannel reads a candidate line's fields by C++ stream extraction, not a whitespace split, so a line can divide into different fields there than in the strip. A line rebuilt from the fields the strip read, in the one spelling both read alike, makes the address checked the address the ICE agent uses, whatever either parser makes of other text. TCP candidates go too: libjuice, the addon's ICE agent, checks UDP alone. Probed on `node-datachannel` 0.33.4 (macOS 27, 2026-10-20): a line opening with whitespace or `A=` is no candidate to the addon, a non-literal address such as `127.1` resolves to `127.0.0.1`, and a leading-zero port reads as decimal.

**Why each family is checked against its own ranges** *(measured on Node 24.22, 2026-10)*. Node's `BlockList` matches an IPv4 address against an IPv6 subnet covering `::ffff:0:0/96` even when asked to check it as IPv4, so one list holding both families let an allowed `::/0`, `::/64`, or `::ffff:0:0/96` admit `8.8.8.8` in both the strip and the pair check. `allowedAddressTest` keeps one list per family and unwraps a mapped address first, as the Relay's push-endpoint check already did.

**Why bind only the single-address case.** `bindAddress` takes one address, and a policy may allow several networks, so binding narrows the listener only where exactly one allowed address is present; elsewhere the listener stays as the security model states it and the path check alone holds the level. The single case is the common one for the users who care: a person who allows only their VPN has one interface, and IPv4 is preferred there because a VPN interface with both families still needs one address, and both shipped Clients reach IPv4. Probed on `node-datachannel` 0.33.4 (macOS, 2026-09-30): the polyfill spreads its configuration into the native `PeerConnection`, so `bindAddress` reaches it; with it the peer binds exactly that address and advertises one host candidate, without it one socket on `*:port` advertises every interface in both families.

**The bound path against a real browser** *(2026-09-30, HeadlessChrome 150 on macOS 27 via `dor agent-browser`, `node-datachannel` 0.33.4 over libdatachannel 0.24.5)*. `scripts/direct-interop/run.mjs --allow` with the LAN's two prefixes bound `192.168.86.160`, answered with that one candidate, and carried the three frames intact. Chrome offered one mDNS host candidate (`<uuid>.local`), and the addon's selected pair reported `192.168.86.160` → `192.168.86.160`, the remote end `prflx`: the addon learns the browser's real address from its connectivity check, so the pair is an IP literal even when every candidate the browser sent was a name. With the ULA prefix and the tailnet allowed — two interfaces, unbound — the stripped answer carried two candidates and `c=IN IP4 0.0.0.0`, Chrome accepted it, and the pair was the ULA address at both ends, again `prflx`.
Expand Down
2 changes: 1 addition & 1 deletion docs/specs/security-remote.md
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ The HTTPS origin may be public. Tailnet-only Serve is the installer default and
- **FAIL IF** `S238` either Burrow's native peer addon is loaded at host startup rather than at the first offer, or its absence changes anything but a decline. `createNativeDirectPeerFactory` in `lib/src/host/remote/` must reach `node-datachannel` — declared in `standalone/sidecar/package.json` and `vscode-ext/package.json` — only through a bare `require` inside an authorized session's first offer, and a load failure answers `direct-decline` and leaves that session relayed rather than failing the Burrow's start.
- **FAIL IF** `S239` a switched end waits on its peer without a deadline, or a channel this protocol did not ask for is adopted. Before reporting the open, `DirectPeer` in `lib/src/remote/direct/direct-peer.ts` must refuse a channel not labelled `DIRECT_CHANNEL_LABEL`, one reported unordered or partially reliable, and one whose association's per-message limit is below `NOISE_MAX_MESSAGE_LENGTH`, so each abandons the attempt while the relay still carries the session. **The reliability half is defence in depth against a paired Client, not a boundary control** (`docs/specs/remote-api.md` -> "Direct path"); `lib/src/host/remote/native-direct-peer.test.ts` pins that neither Burrow's implementation reports those flags. `DirectEndpoint` must arm `DIRECT_HANDOFF_TIMEOUT_MS` on its own switch, since from there it sends only on the channel.
- **FAIL IF** `S240` under Local networks a paired phone's application message is read off the Relay, or its session outlives a given-up attempt or a missed `DIRECT_ONLY_DEADLINE_MS` (`docs/specs/remote-network.md` -> "Local networks"). `BurrowRuntime.#promoteConnection` in `lib/src/remote/burrow/burrow-runtime.ts` must derive `directOnly` from the path policy alone, say so in `ConnectionOutcomeV1.directOnly`, and hand it to `EstablishedE2eSession`, and `BurrowService` in `lib/src/host/remote/service.ts` must start a `local` Burrow on `localNetworksPath` over the policy's `allowed` and restart it on any change `samePaths` sees. `scripts/e2e-lint.mjs` holds the derivation and its hand-off textually; pinned by `lib/src/remote/burrow/burrow-direct-only.test.ts`.
- **FAIL IF** `S272` under Local networks the Burrow's ICE agent is handed a candidate other than the one the strip checked: `localNetworksPath` in `lib/src/host/remote/local-networks.ts` must pass a kept candidate on only as the line it rebuilt from the fields it checked, stripping every other line the addon could read as one — two parsers can read two addresses in one line. Pinned on the addon by `lib/src/host/remote/native-direct-peer.test.ts`.
- **FAIL IF** `S272` under Local networks the Burrow's ICE agent is handed a candidate other than the one the strip checked, or the strip or the pair check admits an address through a range of the other family: `localNetworksPath` in `lib/src/host/remote/local-networks.ts` must pass a kept candidate on only as the line it rebuilt from the fields it checked, stripping every other line the addon could read as one — two parsers can read two addresses in one line — and `allowedAddressTest` in `lib/src/host/remote/network-interfaces.ts` must check IPv4, an IPv4-mapped address included, only against IPv4 ranges. Pinned on the addon by `lib/src/host/remote/native-direct-peer.test.ts`, and the families by `lib/src/host/remote/network-interfaces.test.ts`.
- **FAIL IF** `S241` a direct path survives `client-gone`, `burrow-gone`, or a lost relay socket: the Relay stays the lifecycle authority on both paths of any session it carries ([One-time connection](#one-time-connection) is the one carve-out), and every Burrow bound holds whichever path carried a frame.

### One-time connection
Expand Down
21 changes: 21 additions & 0 deletions lib/src/host/remote/local-networks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,27 @@ describe('localNetworksPath', () => {
expect(path.refusal({ local: '100.97.229.25', remote: '100.101.7.8' })?.end).toBe('local');
});

it('lets an IPv6 range over the IPv4-mapped block admit no IPv4 end, in the strip or the pair', () => {
const offer = [
'v=0',
'a=candidate:1 1 udp 2113937151 203.0.113.7 51236 typ host',
'a=candidate:2 1 udp 2113937151 ::ffff:203.0.113.7 51237 typ host',
'a=candidate:3 1 udp 2113937151 2001:db8::7 51238 typ host',
'',
].join('\r\n');
for (const range of ['::/0', '::ffff:0:0/96']) {
const path = localNetworksPath([range]);
expect(candidatesIn(path.acceptRemote(offer)), range).toEqual(
range === '::/0' ? ['a=candidate:3 1 UDP 2113937151 2001:db8::7 51238 typ host'] : [],
);
expect(candidatesIn(path.describe(ANSWER) ?? '').filter((line) => /\d+\.\d+\.\d+\.\d+ /.test(line)), range).toEqual([]);
expect(path.refusal({ local: '192.168.86.160', remote: '203.0.113.7' })?.end, range).toBe('local');
expect(path.refusal({ local: '::ffff:192.168.86.160', remote: '::ffff:203.0.113.7' })?.end, range).toBe('local');
}
// An IPv4 range beside it still decides IPv4 ends alone.
expect(localNetworksPath(['::/0', ...LAN]).refusal({ local: '192.168.86.160', remote: '203.0.113.7' })?.end).toBe('remote');
});

it('refuses a name, an mDNS name, a missing end, or no pair at all', () => {
const path = localNetworksPath(LAN);
for (const remote of ['phone.local', '0f1e2d3c-aaaa-bbbb-cccc-000000000000.local', null]) {
Expand Down
14 changes: 13 additions & 1 deletion lib/src/host/remote/network-interfaces.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,24 @@ describe('allowedAddressTest', () => {
});

it('matches an IPv4-mapped IPv6 address against its IPv4 range', () => {
// `BlockList` compares it as IPv4 (Node 24, 2026-09), so no unwrap is needed.
expect(addressAllowed('::ffff:192.168.1.5', allowed)).toBe(true);
expect(addressAllowed('::ffff:c0a8:105', allowed)).toBe(true);
expect(addressAllowed('::ffff:192.168.2.5', allowed)).toBe(false);
});

it('never admits an IPv4 address, mapped or not, through an IPv6 range', () => {
// Node's `BlockList` matches IPv4 against an IPv6 subnet over `::ffff:0:0/96`.
for (const range of ['::/0', '::/1', '::/64', '::ffff:0:0/96', '::ffff:808:0/120']) {
for (const address of ['8.8.8.8', '::ffff:8.8.8.8', '::ffff:808:808', '192.168.1.5']) {
expect(addressAllowed(address, [range]), `${address} in ${range}`).toBe(false);
}
}
// The IPv6 range still admits the IPv6 it covers, and an IPv4 range beside it still holds.
expect(addressAllowed('2001:db8::5', ['::/0'])).toBe(true);
expect(addressAllowed('::ffff:192.168.1.5', ['::/0', ...allowed])).toBe(true);
expect(addressAllowed('8.8.8.8', ['::/0', ...allowed])).toBe(false);
});

it('allows no hostname, mDNS name, zoned address, or empty list', () => {
for (const address of ['router.local', '4f1d2c3e-aaaa-bbbb-cccc-000000000000.local', 'fe80::1%en0', '', 'localhost']) {
expect(addressAllowed(address, [...allowed, 'fe80::/10']), address).toBe(false);
Expand Down
Loading
Loading