diff --git a/server/package.json b/server/package.json index 8137a69f7..063ab0129 100644 --- a/server/package.json +++ b/server/package.json @@ -24,15 +24,19 @@ "build": "tsc && shx cp -R static build", "start": "node build/index.js", "dev": "tsx watch --clear-screen=false src/index.ts", - "dev:windows": "tsx watch --clear-screen=false src/index.ts < NUL" + "dev:windows": "tsx watch --clear-screen=false src/index.ts < NUL", + "test": "vitest run", + "test:watch": "vitest" }, "devDependencies": { "@types/cors": "^2.8.19", "@types/express": "^5.0.0", + "@types/node": "^22.0.0", "@types/shell-quote": "^1.7.5", "@types/ws": "^8.5.12", "tsx": "^4.19.0", - "typescript": "^5.6.2" + "typescript": "^5.6.2", + "vitest": "^4.1.0" }, "dependencies": { "@modelcontextprotocol/sdk": "^1.25.2", diff --git a/server/src/__tests__/proxy-security.test.ts b/server/src/__tests__/proxy-security.test.ts new file mode 100644 index 000000000..3772f0bf2 --- /dev/null +++ b/server/src/__tests__/proxy-security.test.ts @@ -0,0 +1,265 @@ +/** + * Unit tests for proxy-security.ts + * + * Tests cover: + * - isBlockedProxyAddress: IPv4, IPv6, IPv4-mapped IPv6 (hex + dotted), edge cases + * - assertSafeProxyTarget: safe IPs, blocked IPs, literal-IP hosts, DNS errors + * - createPinnedAgent: correct agent type, lookup always returns pinned IP + * - TOCTOU guarantee: the pinned agent never invokes the OS resolver + */ + +import http from "node:http"; +import https from "node:https"; +import { vi, describe, it, expect, afterEach } from "vitest"; + +// Mock node:dns/promises before importing the module under test so that +// assertSafeProxyTarget's dnsLookup is replaceable in each test. +vi.mock("node:dns/promises", () => ({ + lookup: vi.fn(), +})); + +import * as dns from "node:dns/promises"; +import { + isBlockedProxyAddress, + assertSafeProxyTarget, + createPinnedAgent, + ProxyTargetError, +} from "../proxy-security.js"; + +// Convenience cast — vitest doesn't know the mock shape yet. +const mockLookup = dns.lookup as ReturnType; + +afterEach(() => { + vi.clearAllMocks(); +}); + +// --------------------------------------------------------------------------- +// isBlockedProxyAddress +// --------------------------------------------------------------------------- + +describe("isBlockedProxyAddress", () => { + describe("IPv4 link-local (169.254.0.0/16)", () => { + it("blocks 169.254.169.254 (AWS metadata)", () => { + expect(isBlockedProxyAddress("169.254.169.254")).toBe(true); + }); + + it("blocks 169.254.0.1 (first address in range)", () => { + expect(isBlockedProxyAddress("169.254.0.1")).toBe(true); + }); + + it("blocks 169.254.255.255 (last address in range)", () => { + expect(isBlockedProxyAddress("169.254.255.255")).toBe(true); + }); + + it("allows 169.253.0.1 (just outside the range)", () => { + expect(isBlockedProxyAddress("169.253.0.1")).toBe(false); + }); + + it("allows 170.254.0.1 (just outside the range)", () => { + expect(isBlockedProxyAddress("170.254.0.1")).toBe(false); + }); + + it("allows loopback 127.0.0.1", () => { + expect(isBlockedProxyAddress("127.0.0.1")).toBe(false); + }); + + it("allows a public IP", () => { + expect(isBlockedProxyAddress("93.184.216.34")).toBe(false); + }); + }); + + describe("IPv6 link-local (fe80::/10)", () => { + it("blocks fe80::1", () => { + expect(isBlockedProxyAddress("fe80::1")).toBe(true); + }); + + it("blocks fe80::aabb:ccdd (arbitrary link-local)", () => { + expect(isBlockedProxyAddress("fe80::aabb:ccdd")).toBe(true); + }); + + it("allows ::1 (loopback)", () => { + expect(isBlockedProxyAddress("::1")).toBe(false); + }); + + it("allows 2001:db8::1 (documentation range)", () => { + expect(isBlockedProxyAddress("2001:db8::1")).toBe(false); + }); + }); + + describe("AWS IPv6 IMDS (fd00:ec2::254)", () => { + it("blocks fd00:ec2::254 exactly", () => { + expect(isBlockedProxyAddress("fd00:ec2::254")).toBe(true); + }); + + it("allows fd00:ec2::255 (adjacent address)", () => { + expect(isBlockedProxyAddress("fd00:ec2::255")).toBe(false); + }); + }); + + describe("IPv4-mapped IPv6 variants of 169.254.169.254", () => { + it("blocks dotted form ::ffff:169.254.169.254", () => { + expect(isBlockedProxyAddress("::ffff:169.254.169.254")).toBe(true); + }); + + it("blocks hex form ::ffff:a9fe:a9fe (WHATWG URL serialization)", () => { + expect(isBlockedProxyAddress("::ffff:a9fe:a9fe")).toBe(true); + }); + + it("allows IPv4-mapped loopback ::ffff:127.0.0.1", () => { + expect(isBlockedProxyAddress("::ffff:127.0.0.1")).toBe(false); + }); + }); + + describe("non-IP strings", () => { + it("allows empty string (not an IP)", () => { + expect(isBlockedProxyAddress("")).toBe(false); + }); + + it("allows hostname string (not an IP)", () => { + expect(isBlockedProxyAddress("example.com")).toBe(false); + }); + }); +}); + +// --------------------------------------------------------------------------- +// assertSafeProxyTarget +// --------------------------------------------------------------------------- + +describe("assertSafeProxyTarget", () => { + it("resolves and allows a safe hostname", async () => { + mockLookup.mockResolvedValueOnce([{ address: "93.184.216.34", family: 4 }]); + + const addrs = await assertSafeProxyTarget(new URL("http://example.com/")); + expect(addrs).toEqual(["93.184.216.34"]); + expect(mockLookup).toHaveBeenCalledWith("example.com", { all: true }); + }); + + it("throws ProxyTargetError when host resolves to blocked IP", async () => { + mockLookup.mockResolvedValueOnce([ + { address: "169.254.169.254", family: 4 }, + ]); + + await expect( + assertSafeProxyTarget(new URL("http://evil.example.com/")), + ).rejects.toThrow(ProxyTargetError); + }); + + it("throws ProxyTargetError when any resolved IP is blocked (mixed results)", async () => { + mockLookup.mockResolvedValueOnce([ + { address: "93.184.216.34", family: 4 }, + { address: "169.254.169.254", family: 4 }, + ]); + + await expect( + assertSafeProxyTarget(new URL("http://dual.example.com/")), + ).rejects.toThrow(ProxyTargetError); + }); + + it("throws ProxyTargetError when DNS lookup fails", async () => { + mockLookup.mockRejectedValueOnce(new Error("ENOTFOUND")); + + await expect( + assertSafeProxyTarget(new URL("http://nonexistent.invalid/")), + ).rejects.toThrow(ProxyTargetError); + }); + + it("skips DNS lookup for literal IPv4 hosts", async () => { + const addrs = await assertSafeProxyTarget( + new URL("http://127.0.0.1/path"), + ); + expect(addrs).toEqual(["127.0.0.1"]); + expect(mockLookup).not.toHaveBeenCalled(); + }); + + it("throws ProxyTargetError for literal blocked IPv4", async () => { + await expect( + assertSafeProxyTarget(new URL("http://169.254.169.254/")), + ).rejects.toThrow(ProxyTargetError); + expect(mockLookup).not.toHaveBeenCalled(); + }); + + it("skips DNS lookup for literal IPv6 hosts", async () => { + const addrs = await assertSafeProxyTarget(new URL("http://[::1]/")); + expect(addrs).toEqual(["::1"]); + expect(mockLookup).not.toHaveBeenCalled(); + }); + + it("returns all validated addresses so caller can pick one for pinning", async () => { + mockLookup.mockResolvedValueOnce([ + { address: "192.0.2.1", family: 4 }, + { address: "192.0.2.2", family: 4 }, + ]); + + const addrs = await assertSafeProxyTarget(new URL("http://multi.example/")); + expect(addrs).toHaveLength(2); + expect(addrs).toContain("192.0.2.1"); + expect(addrs).toContain("192.0.2.2"); + }); +}); + +// --------------------------------------------------------------------------- +// createPinnedAgent +// --------------------------------------------------------------------------- + +describe("createPinnedAgent", () => { + it("returns an http.Agent for http: protocol", () => { + const agent = createPinnedAgent("http:", "127.0.0.1"); + expect(agent).toBeInstanceOf(http.Agent); + expect(agent).not.toBeInstanceOf(https.Agent); + }); + + it("returns an https.Agent for https: protocol", () => { + const agent = createPinnedAgent("https:", "127.0.0.1"); + expect(agent).toBeInstanceOf(https.Agent); + }); + + it("pinned lookup always returns the IPv4 address regardless of queried hostname", () => + new Promise((resolve) => { + const agent = createPinnedAgent("http:", "192.0.2.99"); + const lookup = (agent as http.Agent & { options: { lookup: Function } }) + .options.lookup; + + lookup( + "example.com", + {}, + (err: Error | null, address: string, family: number) => { + expect(err).toBeNull(); + expect(address).toBe("192.0.2.99"); + expect(family).toBe(4); + resolve(); + }, + ); + })); + + it("pinned lookup always returns the IPv6 address and family 6", () => + new Promise((resolve) => { + const agent = createPinnedAgent("http:", "2001:db8::1"); + const lookup = (agent as http.Agent & { options: { lookup: Function } }) + .options.lookup; + + lookup( + "example.com", + {}, + (err: Error | null, address: string, family: number) => { + expect(err).toBeNull(); + expect(address).toBe("2001:db8::1"); + expect(family).toBe(6); + resolve(); + }, + ); + })); + + it("TOCTOU guarantee: lookup never invokes the OS resolver", () => { + const agent = createPinnedAgent("http:", "10.0.0.1"); + const lookup = (agent as http.Agent & { options: { lookup: Function } }) + .options.lookup; + + const osDnsLookup = vi.fn(); + lookup("any-hostname.example", {}, osDnsLookup); + + // osDnsLookup was called as the callback, not as a resolver — + // the pinned implementation calls it synchronously with the fixed IP. + expect(osDnsLookup).toHaveBeenCalledTimes(1); + expect(osDnsLookup).toHaveBeenCalledWith(null, "10.0.0.1", 4); + }); +}); diff --git a/server/src/index.ts b/server/src/index.ts index 4c86bd791..b0b6774be 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -4,6 +4,11 @@ import cors from "cors"; import { parseArgs } from "node:util"; import { parse as shellParseArgs } from "shell-quote"; import nodeFetch, { Headers as NodeHeaders } from "node-fetch"; +import { + assertSafeProxyTarget, + createPinnedAgent, + ProxyTargetError, +} from "./proxy-security.js"; // Type-compatible wrappers for node-fetch to work with browser-style types const fetch = nodeFetch; @@ -29,8 +34,6 @@ import rateLimit from "express-rate-limit"; import { findActualExecutable } from "spawn-rx"; import mcpProxy, { type ProxyHeaderHolder } from "./mcpProxy.js"; import { randomUUID, randomBytes, timingSafeEqual } from "node:crypto"; -import { lookup as dnsLookup } from "node:dns/promises"; -import { isIP } from "node:net"; import { fileURLToPath } from "url"; import { dirname, join } from "path"; import { readFileSync } from "fs"; @@ -863,91 +866,17 @@ app.get("/health", (req, res) => { }); }); -// Raised when the /fetch proxy is asked to reach a disallowed target, so the -// route can distinguish an SSRF rejection (403) from an upstream failure (500). -class ProxyTargetError extends Error {} - const MAX_PROXY_REDIRECTS = 5; -// The /fetch proxy is used by the client to reach OAuth discovery/token -// endpoints on the connected MCP server (bypassing browser CORS). Connecting -// to loopback and RFC1918/private LAN hosts is a core use case (testing local -// or self-hosted MCP servers), so those stay reachable. We only block -// link-local addresses — 169.254.0.0/16 (the AWS/GCP/Azure/etc. cloud metadata -// endpoint 169.254.169.254 lives here) and IPv6 link-local / the AWS IPv6 IMDS -// address — which are never legitimate proxy targets and are the real SSRF -// exfiltration vector. -function isBlockedProxyAddress(ip: string): boolean { - const kind = isIP(ip); - if (kind === 4) { - const parts = ip.split(".").map(Number); - // 169.254.0.0/16 — IPv4 link-local (incl. cloud metadata 169.254.169.254) - return parts[0] === 169 && parts[1] === 254; - } - if (kind === 6) { - const addr = ip.toLowerCase(); - // IPv4-mapped IPv6 (::ffff:x). The WHATWG URL parser serializes the embedded - // IPv4 in hex (::ffff:a9fe:a9fe for 169.254.169.254), while DNS and other - // sources may use the dotted form (::ffff:169.254.169.254) — handle both by - // extracting the IPv4 and re-checking it. - const mapped = addr.match(/^::ffff:(.+)$/); - if (mapped) { - const tail = mapped[1]; - if (isIP(tail) === 4) { - return isBlockedProxyAddress(tail); - } - const hex = tail.match(/^([0-9a-f]{1,4}):([0-9a-f]{1,4})$/); - if (hex) { - const high = parseInt(hex[1], 16); - const low = parseInt(hex[2], 16); - return isBlockedProxyAddress( - `${high >> 8}.${high & 0xff}.${low >> 8}.${low & 0xff}`, - ); - } - } - // fe80::/10 — IPv6 link-local - if (/^fe[89ab]/.test(addr)) { - return true; - } - // fd00:ec2::254 — AWS IPv6 instance metadata endpoint - return addr === "fd00:ec2::254"; - } - return false; -} - -// Resolves the target host and rejects it if any resolved address is a blocked -// (link-local/metadata) address, mitigating SSRF including the DNS case where a -// hostname resolves to the cloud metadata IP. -async function assertSafeProxyTarget(parsedUrl: URL): Promise { - const host = parsedUrl.hostname.replace(/^\[|\]$/g, ""); // strip IPv6 brackets - - let addresses: string[]; - if (isIP(host)) { - addresses = [host]; - } else { - try { - addresses = (await dnsLookup(host, { all: true })).map((r) => r.address); - } catch { - throw new ProxyTargetError(`Could not resolve host: ${host}`); - } - } - - if (addresses.length === 0) { - throw new ProxyTargetError(`Could not resolve host: ${host}`); - } - - for (const address of addresses) { - if (isBlockedProxyAddress(address)) { - throw new ProxyTargetError( - `Refusing to proxy request to blocked address (${address})`, - ); - } - } -} - // Fetch that follows redirects manually so every hop's target is re-validated // against assertSafeProxyTarget — otherwise an allowed host could redirect the // proxy into a blocked internal/metadata address. +// +// On each hop we call assertSafeProxyTarget (which resolves DNS and validates +// every returned IP) and then pass a createPinnedAgent to node-fetch so it +// connects to the IP we just validated instead of re-resolving the hostname. +// This eliminates the TOCTOU window between the block-list check and the +// actual TCP connection that a DNS-rebinding attack would exploit. async function safeProxyFetch(initialUrl: string, init?: RequestInit) { let currentUrl = new URL(initialUrl); let method = init?.method ?? "GET"; @@ -958,13 +887,19 @@ async function safeProxyFetch(initialUrl: string, init?: RequestInit) { if (!["http:", "https:"].includes(currentUrl.protocol)) { throw new ProxyTargetError("Only http/https URLs are allowed"); } - await assertSafeProxyTarget(currentUrl); + + // Resolve DNS once, validate every address, and pin the connection to the + // first validated IP so node-fetch never does a second DNS lookup. + const validatedAddresses = await assertSafeProxyTarget(currentUrl); + const agent = createPinnedAgent(currentUrl.protocol, validatedAddresses[0]); const response = await fetch(currentUrl.toString(), { method, headers, body, redirect: "manual", + // eslint-disable-next-line @typescript-eslint/no-explicit-any + agent: agent as any, }); if (response.status >= 300 && response.status < 400) { diff --git a/server/src/proxy-security.ts b/server/src/proxy-security.ts new file mode 100644 index 000000000..d13365416 --- /dev/null +++ b/server/src/proxy-security.ts @@ -0,0 +1,157 @@ +/** + * SSRF-prevention helpers for the /fetch reverse proxy. + * + * Extracted from index.ts so they can be unit-tested independently. + * + * ## DNS-rebinding / TOCTOU threat model + * + * `assertSafeProxyTarget` resolves the target hostname and validates every + * returned IP against the block-list. The resolved IPs are returned to the + * caller so they can be passed to `createPinnedAgent`, which builds an + * http/https.Agent whose `lookup` callback unconditionally returns the + * pre-validated IP. The agent is then passed to `node-fetch`, which calls the + * lookup hook instead of doing its own DNS query. + * + * Without this pinning there is a TOCTOU race: an attacker who controls the + * target domain can flip its DNS record to 169.254.169.254 (AWS/GCP/Azure + * instance-metadata) in the window between `assertSafeProxyTarget` returning + * and `fetch` performing its own resolution. Pinning eliminates that window. + */ + +import http from "node:http"; +import https from "node:https"; +import { isIP } from "node:net"; +import { lookup as dnsLookup } from "node:dns/promises"; + +// --------------------------------------------------------------------------- +// Block-list +// --------------------------------------------------------------------------- + +/** + * Returns true if `ip` is a link-local or cloud-metadata address that must + * never be reachable through the proxy. + * + * Blocked ranges: + * - 169.254.0.0/16 (IPv4 link-local; includes 169.254.169.254 IMDS) + * - ::ffff:169.254.x.x (IPv4-mapped IPv6 variants of the above) + * - fe80::/10 (IPv6 link-local) + * - fd00:ec2::254 (AWS IPv6 instance-metadata) + */ +export function isBlockedProxyAddress(ip: string): boolean { + const kind = isIP(ip); + if (kind === 4) { + const parts = ip.split(".").map(Number); + return parts[0] === 169 && parts[1] === 254; + } + if (kind === 6) { + const addr = ip.toLowerCase(); + + // IPv4-mapped IPv6 — two serialization forms: + // dotted ::ffff:169.254.169.254 + // hex ::ffff:a9fe:a9fe + const mapped = addr.match(/^::ffff:(.+)$/); + if (mapped) { + const tail = mapped[1]; + if (isIP(tail) === 4) { + return isBlockedProxyAddress(tail); + } + const hex = tail.match(/^([0-9a-f]{1,4}):([0-9a-f]{1,4})$/); + if (hex) { + const high = parseInt(hex[1], 16); + const low = parseInt(hex[2], 16); + return isBlockedProxyAddress( + `${high >> 8}.${high & 0xff}.${low >> 8}.${low & 0xff}`, + ); + } + } + + if (/^fe[89ab]/.test(addr)) return true; // fe80::/10 + return addr === "fd00:ec2::254"; // AWS IPv6 IMDS + } + return false; +} + +// --------------------------------------------------------------------------- +// DNS validation +// --------------------------------------------------------------------------- + +/** Raised when the /fetch proxy is asked to reach a disallowed target. */ +export class ProxyTargetError extends Error {} + +/** + * Resolves `parsedUrl`'s hostname and asserts that every returned IP is + * outside the block-list. + * + * Returns the list of validated IP strings so the caller can pin one of them + * into an HTTP agent (see `createPinnedAgent`), eliminating the TOCTOU window + * between this check and the actual `fetch()` call. + * + * @throws {ProxyTargetError} if the host cannot be resolved or any resolved + * address falls inside the block-list. + */ +export async function assertSafeProxyTarget( + parsedUrl: URL, +): Promise { + const host = parsedUrl.hostname.replace(/^\[|\]$/g, ""); // strip IPv6 brackets + + let addresses: string[]; + if (isIP(host)) { + addresses = [host]; + } else { + try { + addresses = (await dnsLookup(host, { all: true })).map((r) => r.address); + } catch { + throw new ProxyTargetError(`Could not resolve host: ${host}`); + } + } + + if (addresses.length === 0) { + throw new ProxyTargetError(`Could not resolve host: ${host}`); + } + + for (const address of addresses) { + if (isBlockedProxyAddress(address)) { + throw new ProxyTargetError( + `Refusing to proxy request to blocked address (${address})`, + ); + } + } + + return addresses; +} + +// --------------------------------------------------------------------------- +// IP-pinned agent factory +// --------------------------------------------------------------------------- + +/** + * Creates an `http.Agent` or `https.Agent` whose internal `lookup` hook always + * returns `pinnedIp`, bypassing the OS resolver entirely. + * + * Pass this agent to `node-fetch` on every hop of `safeProxyFetch` so the TCP + * connection always goes to the IP that was validated by `assertSafeProxyTarget` + * — even if the domain's DNS record changes mid-flight. + * + * @param protocol - `"http:"` or `"https:"` (from `URL.protocol`) + * @param pinnedIp - A validated IPv4 or IPv6 address string + */ +export function createPinnedAgent( + protocol: string, + pinnedIp: string, +): http.Agent | https.Agent { + const family = isIP(pinnedIp) === 6 ? 6 : 4; + + // Node.js lookup signature: (hostname, options, callback) + const lookup = ( + _hostname: string, + _opts: unknown, + callback: (err: Error | null, address: string, family: number) => void, + ): void => { + callback(null, pinnedIp, family); + }; + + if (protocol === "https:") { + return new https.Agent({ lookup } as https.AgentOptions); + } + return new http.Agent({ lookup } as http.AgentOptions); +} diff --git a/server/vitest.config.ts b/server/vitest.config.ts new file mode 100644 index 000000000..9a3bc821f --- /dev/null +++ b/server/vitest.config.ts @@ -0,0 +1,9 @@ +import { defineConfig } from "vitest/config"; + +export default defineConfig({ + test: { + globals: true, + environment: "node", + include: ["src/__tests__/**/*.test.ts"], + }, +});