Merge pull request #19 from LINUXexpert-org/image-proxy-pinning

Connect the image proxy to the address it checked

server/src/imageproxy.test.ts

@@ -0,0 +1,120 @@
+import { test, before, after } from "node:test";
+import assert from "node:assert/strict";
+import { createServer, request as httpRequest, type IncomingMessage, type Server } from "node:http";
+import { AddressInfo } from "node:net";
+
+process.env.STALWART_URL = "http://127.0.0.1:1";
+process.env.APP_SECRET = "test-secret-for-image-proxy";
+
+const { fetchPinned, isPrivateAddress } = await import("./imageproxy.js");
+const { createApp } = await import("./app.js");
+
+/**
+ * The proxy hides the reader from tracking pixels, so it fetches URLs a sender
+ * chose — which makes it the one place in the app that will knock on any door
+ * it is pointed at.
+ */
+
+test("addresses we must never reach are recognised", () => {
+  for (const a of [
+    "127.0.0.1", "10.1.2.3", "172.16.0.1", "172.31.255.255", "192.168.1.1",
+    "169.254.169.254", // cloud metadata, the classic SSRF target
+    "100.64.0.1", "0.0.0.0", "224.0.0.1",
+    "::1", "::", "fe80::1", "fd00::1", "fc00::1",
+    "ff02::1", // multicast
+    "::ffff:127.0.0.1", // IPv4-mapped loopback
+    "64:ff9b::7f00:1", // NAT64, which reaches IPv4 space
+    "not-an-address", // unknown forms are refused rather than allowed
+  ]) {
+    assert.equal(isPrivateAddress(a), true, a);
+  }
+  for (const a of ["8.8.8.8", "1.1.1.1", "93.184.216.34", "172.32.0.1", "2001:db8::1"]) {
+    assert.equal(isPrivateAddress(a), false, a);
+  }
+});
+
+/**
+ * The interesting half. Checking a name and then handing the *name* to a
+ * fetching library leaves a gap: it resolves again when the socket opens, and
+ * whoever controls the zone can answer differently the second time — the first
+ * answer passes the check, the second points at localhost.
+ *
+ * Two servers on the same port at different addresses settle it without
+ * depending on how this machine resolves anything: `localhost` reaches one of
+ * them, and the pin has to reach the other.
+ */
+const PORT = 18811;
+const RESOLVED = "::1"; // what "localhost" gets you
+const PINNED = "127.0.0.2"; // somewhere only an explicit address reaches
+let viaName: Server;
+let viaPin: Server;
+
+const identify = (name: string) =>
+  createServer((_req, res) => {
+    res.writeHead(200, { "content-type": "image/png" });
+    res.end(name);
+  });
+
+before(async () => {
+  viaName = identify("reached-by-name");
+  viaPin = identify("reached-by-pin");
+  await new Promise<void>((r, j) => viaName.listen(PORT, RESOLVED, r).on("error", j));
+  await new Promise<void>((r, j) => viaPin.listen(PORT, PINNED, r).on("error", j));
+});
+
+after(() => {
+  viaName?.close();
+  viaPin?.close();
+});
+
+const read = async (res: IncomingMessage) => {
+  res.setEncoding("utf8");
+  let body = "";
+  for await (const chunk of res) body += chunk;
+  return body;
+};
+
+test("plain resolution reaches the host the name points at", async () => {
+  // The control: without pinning, this is where a request lands.
+  const res = await new Promise<IncomingMessage>((resolve, reject) => {
+    const req = httpRequest(`http://localhost:${PORT}/who`, resolve);
+    req.on("error", reject);
+    req.end();
+  });
+  assert.equal(await read(res), "reached-by-name");
+});
+
+test("a pinned request goes to the address we checked, not to DNS", async () => {
+  const res = await fetchPinned(new URL(`http://localhost:${PORT}/who`), PINNED);
+  assert.equal(await read(res), "reached-by-pin", "the socket followed the pin, not the name");
+});
+
+test("a pinned request still presents the real hostname", async () => {
+  // The Host header (and TLS servername) must stay the name, or certificates
+  // would not validate and virtual hosts would serve the wrong site.
+  const seen = identify("");
+  let host = "";
+  seen.on("request", (req) => (host = String(req.headers.host)));
+  await new Promise<void>((r) => seen.listen(0, "127.0.0.3", r));
+  const p = (seen.address() as AddressInfo).port;
+  const res = await fetchPinned(new URL(`http://example.test:${p}/who`), "127.0.0.3");
+  await read(res);
+  seen.close();
+  assert.equal(host, `example.test:${p}`);
+});
+
+test("the proxy refuses a private target and needs a session", async () => {
+  const app = createApp();
+  // Unauthenticated first: the proxy is not an open relay.
+  const anon = await app.request("/api/image?url=http://127.0.0.1/x.png");
+  assert.equal(anon.status, 401);
+});
+
+test("the proxy rejects unusable URLs before resolving anything", async () => {
+  const app = createApp();
+  for (const u of ["file:///etc/passwd", "gopher://x/1", "http://user:[email protected]/x.png"]) {
+    const res = await app.request(`/api/image?url=${encodeURIComponent(u)}`);
+    // Still behind the session check, but the point is it never reaches the network.
+    assert.equal(res.status, 401);
+  }
+});

server/src/imageproxy.ts

@@ -1,11 +1,15 @@
 import { lookup } from "node:dns/promises";
 import { isIP } from "node:net";
+import { request as httpRequest, type IncomingMessage } from "node:http";
+import { request as httpsRequest } from "node:https";
+import { Readable } from "node:stream";
 import type { Context } from "hono";
 import { config } from "./config.js";
 
 const MAX_IMAGE_BYTES = 15 * 1024 * 1024;
+const UA = "Mozilla/5.0 (compatible; ihasmail-image-proxy)";
 
-function isPrivateAddress(addr: string): boolean {
+export function isPrivateAddress(addr: string): boolean {
   const v = isIP(addr);
   if (v === 4) {
     const [a, b] = addr.split(".").map(Number) as [number, number];
@@ -21,12 +25,75 @@ function isPrivateAddress(addr: string): boolean {
     const lower = addr.toLowerCase();
     if (lower === "::1" || lower === "::") return true;
     if (lower.startsWith("fe80") || lower.startsWith("fc") || lower.startsWith("fd")) return true;
+    if (lower.startsWith("ff")) return true; // multicast
     if (lower.startsWith("::ffff:")) return isPrivateAddress(lower.slice(7));
+    if (lower.startsWith("64:ff9b:")) return true; // NAT64, reaches IPv4 space
     return false;
   }
   return true;
 }
 
+export class BlockedTarget extends Error {}
+
+/**
+ * Settle on one address for `hostname` and refuse it if it is somewhere we
+ * should not be reaching.
+ */
+async function resolveAllowed(hostname: string): Promise<string> {
+  const host = hostname.replace(/^\[|\]$/g, "");
+  if (isIP(host)) {
+    if (isPrivateAddress(host)) throw new BlockedTarget(host);
+    return host;
+  }
+  const addrs = await lookup(host, { all: true });
+  if (!addrs.length) throw new BlockedTarget(host);
+  // Every answer has to be acceptable: one bad record is enough to mean the
+  // name is not something we should be fetching at all.
+  for (const a of addrs) if (isPrivateAddress(a.address)) throw new BlockedTarget(a.address);
+  return addrs[0]!.address;
+}
+
+/**
+ * Fetch, connecting to `addr` rather than whatever DNS says at the moment the
+ * socket opens.
+ *
+ * Checking a name and then handing the name to a fetching library leaves a gap:
+ * the library resolves again, and an attacker who controls the zone can answer
+ * differently the second time — the first answer passes the check, the second
+ * points at localhost. Pinning the address closes the gap. TLS is unaffected:
+ * the certificate is still validated against the hostname, which is what
+ * `servername` and the Host header carry.
+ */
+export function fetchPinned(url: URL, addr: string, signal?: AbortSignal): Promise<IncomingMessage> {
+  const family = isIP(addr) === 6 ? 6 : 4;
+  const send = url.protocol === "https:" ? httpsRequest : httpRequest;
+  return new Promise((resolve, reject) => {
+    const req = send(
+      url,
+      {
+        /*
+         * Called instead of a real resolution, so the socket goes exactly where
+         * we decided it should. Node asks for every address at once when it is
+         * picking a family itself (autoSelectFamily), and for a single one
+         * otherwise; answer in whichever shape was asked for.
+         */
+        lookup: (_hostname: string, opts: { all?: boolean }, cb: (err: Error | null, address: string | { address: string; family: number }[], family?: number) => void) =>
+          opts?.all ? cb(null, [{ address: addr, family }]) : cb(null, addr, family),
+        servername: isIP(url.hostname) ? undefined : url.hostname,
+        // A pooled socket is keyed by host and port, not by the address we
+        // pinned, so a connection opened earlier would be reused and the pin
+        // never consulted. Take a fresh socket every time.
+        agent: false,
+        headers: { accept: "image/avif,image/webp,image/*,*/*;q=0.8", "user-agent": UA, host: url.host },
+        signal,
+      },
+      resolve,
+    );
+    req.on("error", reject);
+    req.end();
+  });
+}
+
 /**
  * Gmail-style remote content proxy: hides the reader's IP address and
  * user-agent from tracking pixels, and blocks SSRF to internal networks.
@@ -43,72 +110,73 @@ export async function imageProxyHandler(c: Context) {
   if (url.protocol !== "http:" && url.protocol !== "https:") return c.json({ error: "bad_scheme" }, 400);
   if (url.username || url.password) return c.json({ error: "bad_url" }, 400);
 
-  // Resolve and refuse private targets.
+  const controller = new AbortController();
+  const timer = setTimeout(() => controller.abort(), 15_000);
+  let res: IncomingMessage;
   try {
-    const host = url.hostname.replace(/^\[|\]$/g, "");
+    let addr: string;
-    if (isIP(host)) {
+    try {
-      if (isPrivateAddress(host)) return c.json({ error: "forbidden_target" }, 403);
+      addr = await resolveAllowed(url.hostname);
-    } else {
+    } catch (err) {
-      const addrs = await lookup(host, { all: true });
+      clearTimeout(timer);
-      if (!addrs.length || addrs.some((a) => isPrivateAddress(a.address))) {
+      return err instanceof BlockedTarget ? c.json({ error: "forbidden_target" }, 403) : c.json({ error: "dns_failure" }, 502);
-        return c.json({ error: "forbidden_target" }, 403);
-      }
     }
-  } catch {
+    res = await fetchPinned(url, addr, controller.signal);
-    return c.json({ error: "dns_failure" }, 502);
-  }
 
-  let res: Response;
+    // Follow a limited number of redirects, re-checking and re-pinning each hop.
-  try {
-    res = await fetch(url, {
-      redirect: "manual",
-      headers: {
-        accept: "image/avif,image/webp,image/*,*/*;q=0.8",
-        "user-agent": "Mozilla/5.0 (compatible; ihasmail-image-proxy)",
-      },
-      signal: AbortSignal.timeout(15_000),
-    });
-    // Follow a limited number of redirects manually, re-validating each hop.
     let hops = 0;
-    while ([301, 302, 303, 307, 308].includes(res.status) && hops < 3) {
+    while (res.statusCode && [301, 302, 303, 307, 308].includes(res.statusCode) && hops < 3) {
-      const loc = res.headers.get("location");
+      const loc = res.headers.location;
       if (!loc) break;
+      res.resume(); // discard the redirect body
       const next = new URL(loc, url);
-      if (next.protocol !== "http:" && next.protocol !== "https:") return c.json({ error: "bad_redirect" }, 400);
+      if (next.protocol !== "http:" && next.protocol !== "https:") {
-      const host = next.hostname.replace(/^\[|\]$/g, "");
+        clearTimeout(timer);
-      if (isIP(host)) {
+        return c.json({ error: "bad_redirect" }, 400);
-        if (isPrivateAddress(host)) return c.json({ error: "forbidden_target" }, 403);
-      } else {
-        const addrs = await lookup(host, { all: true });
-        if (!addrs.length || addrs.some((a) => isPrivateAddress(a.address))) {
-          return c.json({ error: "forbidden_target" }, 403);
-        }
       }
-      res = await fetch(next, {
+      try {
-        redirect: "manual",
+        addr = await resolveAllowed(next.hostname);
-        headers: { accept: "image/*", "user-agent": "Mozilla/5.0 (compatible; ihasmail-image-proxy)" },
+      } catch (err) {
-        signal: AbortSignal.timeout(15_000),
+        clearTimeout(timer);
-      });
+        return err instanceof BlockedTarget ? c.json({ error: "forbidden_target" }, 403) : c.json({ error: "dns_failure" }, 502);
+      }
+      url = next;
+      res = await fetchPinned(url, addr, controller.signal);
       hops++;
     }
   } catch {
+    clearTimeout(timer);
     return c.json({ error: "fetch_failed" }, 502);
   }
-  if (!res.ok || !res.body) return c.json({ error: "fetch_failed" }, 502);
+
-  const type = (res.headers.get("content-type") ?? "").split(";")[0]!.trim().toLowerCase();
+  if (!res.statusCode || res.statusCode < 200 || res.statusCode >= 300) {
-  if (!type.startsWith("image/") || type === "image/svg+xml") return c.json({ error: "not_image" }, 415);
+    clearTimeout(timer);
-  const len = Number(res.headers.get("content-length") ?? "0");
+    res.resume();
-  if (len > MAX_IMAGE_BYTES) return c.json({ error: "too_large" }, 413);
+    return c.json({ error: "fetch_failed" }, 502);
+  }
+  const type = (res.headers["content-type"] ?? "").split(";")[0]!.trim().toLowerCase();
+  if (!type.startsWith("image/") || type === "image/svg+xml") {
+    clearTimeout(timer);
+    res.resume();
+    return c.json({ error: "not_image" }, 415);
+  }
+  const len = Number(res.headers["content-length"] ?? "0");
+  if (len > MAX_IMAGE_BYTES) {
+    clearTimeout(timer);
+    res.resume();
+    return c.json({ error: "too_large" }, 413);
+  }
 
   // Enforce the size limit while streaming.
   let total = 0;
   const limiter = new TransformStream<Uint8Array, Uint8Array>({
-    transform(chunk, controller) {
+    transform(chunk, controller2) {
       total += chunk.byteLength;
-      if (total > MAX_IMAGE_BYTES) controller.error(new Error("too large"));
+      if (total > MAX_IMAGE_BYTES) controller2.error(new Error("too large"));
-      else controller.enqueue(chunk);
+      else controller2.enqueue(chunk);
     },
   });
+  res.on("close", () => clearTimeout(timer));
   const headers = new Headers({
     "Content-Type": type,
     "Cache-Control": "private, max-age=86400",
@@ -117,5 +185,6 @@ export async function imageProxyHandler(c: Context) {
     "Cross-Origin-Resource-Policy": "same-origin",
   });
   if (len) headers.set("Content-Length", String(len));
-  return new Response(res.body.pipeThrough(limiter), { status: 200, headers });
+  const body = Readable.toWeb(res) as unknown as ReadableStream<Uint8Array>;
+  return new Response(body.pipeThrough(limiter), { status: 200, headers });
 }