diff --git a/src/middleware/rate-limit.ts b/src/middleware/rate-limit.ts index 7042042..7c5cd51 100644 --- a/src/middleware/rate-limit.ts +++ b/src/middleware/rate-limit.ts @@ -2,6 +2,7 @@ import type { Context, Next } from "hono"; import { getConnInfo } from "@hono/node-server/conninfo"; import { createLogger } from "@percolator/shared"; import { getSharedStore } from "./shared-store.js"; +import { rateLimitKey } from "../utils/ip-key.js"; const logger = createLogger("api:rate-limit"); @@ -78,8 +79,10 @@ export function readRateLimit() { return c.json({ error: "Bad request" }, 400); } + // #205: key IPv6 clients by /64, not by the full /128. `ip` itself stays + // the exact address so logs and error paths keep full fidelity. const bucket = await getSharedStore().incrementRateBucket( - `read:${ip}`, + `read:${rateLimitKey(ip)}`, WINDOW_MS, MAX_RATE_LIMIT_ENTRIES ); @@ -114,8 +117,9 @@ export function writeRateLimit() { return c.json({ error: "Bad request" }, 400); } + // #205: see readRateLimit — IPv6 buckets are keyed by /64. const bucket = await getSharedStore().incrementRateBucket( - `write:${ip}`, + `write:${rateLimitKey(ip)}`, WINDOW_MS, MAX_RATE_LIMIT_ENTRIES ); diff --git a/src/utils/ip-key.ts b/src/utils/ip-key.ts new file mode 100644 index 0000000..4b05c19 --- /dev/null +++ b/src/utils/ip-key.ts @@ -0,0 +1,103 @@ +/** + * Shared client-IP bucket keys (#205). + * + * Rate limiting, the IP blocklist and the WS per-IP counters each had their own + * copy of `normalizeIp`, which only unwrapped `::ffff:`-mapped addresses and + * otherwise used the full 128-bit IPv6 address as the bucket key. A single host + * controlling a /64 — the standard cloud and residential allocation — could + * rotate source addresses to mint a fresh quota per request. + * + * IPv6 clients are therefore keyed by their /64 prefix. IPv4 behaviour is + * unchanged: the exact address stays the key. + * + * No new dependency: #205 suggested a vetted IP library, but adding one now + * would mean regenerating pnpm-lock.yaml, which is exactly the file broken by + * #232 (it pins @percolatorct/sdk to a local filesystem path). Instead the + * expansion below is validated in tests against Node's built-in + * `net.BlockList`, which does its own subnet masking and so acts as an + * independent oracle. + */ +import { isIPv4 } from "node:net"; + +/** A /64 covers the first 4 of the 8 hextets. */ +const IPV6_PREFIX_GROUPS = 4; + +/** Drop a zone/scope id (`fe80::1%eth0`). */ +function stripZone(ip: string): string { + const i = ip.indexOf("%"); + return i === -1 ? ip : ip.slice(0, i); +} + +/** + * Expand an IPv6 literal into exactly 8 numeric hextets. + * Returns null when the input does not parse as IPv6. + */ +function expandIpv6(ip: string): number[] | null { + let addr = stripZone(ip); + + // Embedded IPv4 tail (`::ffff:1.2.3.4`, `2001:db8::1.2.3.4`) → two hextets. + const lastColon = addr.lastIndexOf(":"); + if (lastColon === -1) return null; + const tail = addr.slice(lastColon + 1); + if (tail.includes(".")) { + if (!isIPv4(tail)) return null; + const o = tail.split(".").map(Number); + const hi = (((o[0] << 8) | o[1]) >>> 0).toString(16); + const lo = (((o[2] << 8) | o[3]) >>> 0).toString(16); + addr = `${addr.slice(0, lastColon + 1)}${hi}:${lo}`; + } + + const halves = addr.split("::"); + if (halves.length > 2) return null; + + const parseGroups = (s: string): number[] | null => { + if (s === "") return []; + const out: number[] = []; + for (const g of s.split(":")) { + if (g === "" || g.length > 4 || !/^[0-9a-fA-F]+$/.test(g)) return null; + out.push(parseInt(g, 16)); + } + return out; + }; + + const head = parseGroups(halves[0] ?? ""); + if (head === null) return null; + + if (halves.length === 2) { + const rest = parseGroups(halves[1] ?? ""); + if (rest === null) return null; + const fill = 8 - head.length - rest.length; + if (fill < 0) return null; + return [...head, ...new Array(fill).fill(0), ...rest]; + } + + return head.length === 8 ? head : null; +} + +/** + * Bucket key for a client IP. + * + * - IPv4 (incl. `::ffff:`-mapped): the exact address, unchanged. + * - IPv6: the `/64` prefix, so address rotation inside one allocation shares + * a single quota. + * - Unparseable input: the address verbatim. That is the pre-#205 behaviour — + * a per-address bucket. Collapsing unparseable input into one shared key + * would let a malformed value evict or throttle unrelated clients. + */ +export function rateLimitKey(ip: string): string { + const norm = ip.startsWith("::ffff:") ? ip.slice(7) : ip; + if (!norm.includes(":")) return norm; // IPv4 fast path + + const groups = expandIpv6(norm); + if (!groups) return norm; + + return ( + groups + .slice(0, IPV6_PREFIX_GROUPS) + .map((g) => g.toString(16)) + .join(":") + "::/64" + ); +} + +/** Exported for tests that cross-check expansion against `net.BlockList`. */ +export const __ipKeyInternals = { expandIpv6 }; diff --git a/tests/utils/ip-key.test.ts b/tests/utils/ip-key.test.ts new file mode 100644 index 0000000..4d43254 --- /dev/null +++ b/tests/utils/ip-key.test.ts @@ -0,0 +1,108 @@ +/** + * #205: IPv6 rate-limit buckets must key on /64, not /128. + * + * A host controlling a /64 (the standard cloud and residential allocation) + * could rotate source addresses to mint a fresh 100 req/min bucket per request. + * + * The expansion in ip-key.ts is hand-written (see the module header for why no + * new dependency), so it is cross-checked here against Node's built-in + * `net.BlockList`. BlockList masks host bits itself, so `addSubnet(a, 64)` + * followed by `check(b)` is an independent answer to "are a and b in the same + * /64?" — derived without reference to our implementation. + */ +import { describe, it, expect } from "vitest"; +import { BlockList, isIPv6 } from "node:net"; +import { rateLimitKey } from "../../src/utils/ip-key.js"; + +const V6 = [ + "2001:db8::1", + "2001:db8::dead:beef", + "2001:0db8:0000:0000:0000:0000:0000:0002", + "2001:db8:0:0:aaaa:bbbb:cccc:dddd", + "2001:db8:0:1::1", + "2001:db8:0:1:ffff::9", + "fe80::1", + "fe80:0:0:0::abcd", + "::1", + "::", + "2001:db8:85a3::8a2e:370:7334", + "2001:db8:85a3:0:0:8a2e:370:7335", + "2600:1f18:abcd:1234::5", + "2600:1f18:abcd:1234:9999::5", + "2001:db8::1.2.3.4", + "2001:db8::5.6.7.8", + "fd00::1", + "fd00:0:0:1::1", +]; + +/** Independent answer: are these two addresses in the same /64? */ +function sameSlash64(a: string, b: string): boolean { + const list = new BlockList(); + list.addSubnet(a, 64, "ipv6"); + return list.check(b, "ipv6"); +} + +describe("#205 rateLimitKey", () => { + it("agrees with net.BlockList on every address pair", () => { + const mismatches: string[] = []; + let checked = 0; + + for (const a of V6) { + for (const b of V6) { + if (!isIPv6(a) || !isIPv6(b)) continue; + checked++; + const oracle = sameSlash64(a, b); + const mine = rateLimitKey(a) === rateLimitKey(b); + if (oracle !== mine) { + mismatches.push(`${a} vs ${b}: oracle=${oracle} mine=${mine}`); + } + } + } + + expect(checked).toBeGreaterThan(300); + expect(mismatches).toEqual([]); + }); + + it("collapses rotation within one /64 to a single bucket (the bypass)", () => { + // What an attacker rotates through. Pre-fix each of these was its own + // /128 bucket and therefore its own full quota. + const rotated = [ + "2001:db8::1", + "2001:db8::2", + "2001:db8::dead:beef", + "2001:db8:0:0:ffff:ffff:ffff:ffff", + ]; + const keys = new Set(rotated.map(rateLimitKey)); + expect(keys.size).toBe(1); + }); + + it("keeps distinct /64s in distinct buckets", () => { + expect(rateLimitKey("2001:db8:0:1::1")).not.toBe(rateLimitKey("2001:db8:0:2::1")); + }); + + it("leaves IPv4 keys exactly as-is", () => { + for (const v4 of ["1.2.3.4", "203.0.113.9", "10.0.0.1", "255.255.255.255"]) { + expect(rateLimitKey(v4)).toBe(v4); + } + }); + + it("unwraps IPv4-mapped IPv6 to the bare IPv4 address", () => { + expect(rateLimitKey("::ffff:1.2.3.4")).toBe("1.2.3.4"); + }); + + it("keeps two different IPv4 addresses in different buckets", () => { + expect(rateLimitKey("1.2.3.4")).not.toBe(rateLimitKey("1.2.3.5")); + }); + + it("ignores a zone id when keying", () => { + expect(rateLimitKey("fe80::2%eth0")).toBe(rateLimitKey("fe80::2")); + }); + + it("falls back to the verbatim value for unparseable input", () => { + // Per-value bucket, matching pre-#205 behaviour. Collapsing malformed + // input into one shared key would let it throttle unrelated clients. + for (const junk of ["not-an-ip", "2001:db8:::1", "xyz::1"]) { + expect(rateLimitKey(junk)).toBe(junk); + } + }); +});