TC-85: security review fixes -- body/frame size limits, connection rate limiting, ordered-pair headers

Addresses the round-1 review of the tunnel relay (PR #1):

1. Body/frame size limits: proxied request bodies are now read with a
   streaming cap (host-router.ts's readLimitedBody, 413 past the cap) and
   response bodies accumulated in proxy.ts are capped too (502 + an `error`
   frame back to the node past the cap). Default 25MB, overridable via
   TUNNEL_MAX_BODY_BYTES. The relay's WebSocketServer now sets `maxPayload`
   (1MB, protocol.ts's MAX_FRAME_PAYLOAD_BYTES) so no single WS frame can be
   unbounded; request bodies are chunked into <=256KB requestBody frames
   (protocol.ts's BODY_CHUNK_BYTES) to respect it. Also adds a 'ws' `error`
   listener in upgrade.ts -- without one, a maxPayload violation's unhandled
   'error' event would crash the whole process, not just the one tunnel.

2. Rate/connection limiting on the WS upgrade path (upgrade.ts, new
   rate-limit.ts): a per-IP connection-attempt limiter (30/min), a per-name
   churn limiter (10/min), and a global concurrent-tunnel cap (1000, env
   TUNNEL_MAX_CONCURRENT) all run in the raw HTTP `upgrade` handler, before
   the WS handshake and before authenticate()'s nameStore.get Postgres read.
   Deliberately in-memory (not the Postgres-backed cert rate limiter): these
   are short, per-minute windows checked on every upgrade attempt.

3. Frame headers are now ordered `[name, value]` pairs (protocol.ts's
   TunnelRequestFrame/TunnelResponseFrame) instead of a Record, so duplicate
   header names -- most importantly Set-Cookie -- survive the roundtrip
   instead of colliding on one object key. host-router.ts rebuilds the final
   client-facing Response via repeated Headers.append() calls so multiple
   Set-Cookie entries are preserved end to end.

Tests: 9 new (66 -> 75) covering each fix -- request/response body caps,
multi-chunk body reassembly, WS maxPayload enforcement, per-IP/per-name/
global connection limits, header-pair passthrough, and a two-Set-Cookie
roundtrip.

Author: samgbafa

.env.example

@@ -26,3 +26,10 @@ ATTESTATION_DOCUMENT=
 # else under *.tinycloud.link are routed through the matching tunnel (TC-85)
 # instead of the /v1 API. Defaults to api.tinycloud.link.
 API_HOSTNAME=api.tinycloud.link
+
+# Max bytes accepted for a single proxied tunnel request body. Defaults to
+# 26214400 (25MB).
+TUNNEL_MAX_BODY_BYTES=
+# Max concurrently registered (authenticated) tunnels; further connection
+# attempts are dropped once at the cap. Defaults to 1000.
+TUNNEL_MAX_CONCURRENT=

src/index.ts

@@ -15,6 +15,12 @@ const cloudflareZoneId = process.env.CLOUDFLARE_ZONE_ID;
 const acmeEmail = process.env.ACME_EMAIL;
 const acmeDirectory =
   process.env.ACME_DIRECTORY ?? "https://acme-staging-v02.api.letsencrypt.org/directory";
+const tunnelMaxBodyBytes = process.env.TUNNEL_MAX_BODY_BYTES
+  ? Number.parseInt(process.env.TUNNEL_MAX_BODY_BYTES, 10)
+  : undefined;
+const tunnelMaxConcurrent = process.env.TUNNEL_MAX_CONCURRENT
+  ? Number.parseInt(process.env.TUNNEL_MAX_CONCURRENT, 10)
+  : undefined;
 
 if (!databaseUrl) throw new Error("DATABASE_URL is required");
 if (!cloudflareApiToken) throw new Error("CLOUDFLARE_API_TOKEN is required");
@@ -59,10 +65,15 @@ const app = createServer({
   attestationDocument: process.env.ATTESTATION_DOCUMENT,
   tunnelRegistry,
   apiHostname,
+  tunnelMaxBodyBytes,
 });
 
 const server = serve({ fetch: app.fetch, port });
-attachTunnelUpgrade(server, { registry: tunnelRegistry, nameStore });
+attachTunnelUpgrade(server, {
+  registry: tunnelRegistry,
+  nameStore,
+  maxConcurrentTunnels: tunnelMaxConcurrent,
+});
 console.log(`tinycloud-link listening on :${port}`);
 
 const shutdown = async () => {

src/server.ts

@@ -31,6 +31,8 @@ export interface ServerConfig {
   tunnelRegistry?: TunnelRegistry;
   /** The control-plane API's own hostname, exempted from tunnel routing. Defaults to "api.tinycloud.link". */
   apiHostname?: string;
+  /** Max bytes accepted for a proxied tunnel request body. Defaults to protocol.ts's DEFAULT_MAX_BODY_BYTES (25MB); overridable via the TUNNEL_MAX_BODY_BYTES env var (see index.ts). */
+  tunnelMaxBodyBytes?: number;
 }
 
 // Prefixes name-update entries so they share the cert rate-limit store without
@@ -49,6 +51,7 @@ export function createServer(config: ServerConfig): Hono {
       "*",
       createTunnelMiddleware(config.tunnelRegistry, {
         apiHostname: config.apiHostname ?? DEFAULT_API_HOSTNAME,
+        maxBodyBytes: config.tunnelMaxBodyBytes,
       })
     );
   }

src/tunnel.test.ts

@@ -11,19 +11,25 @@ import { createServer } from "./server.js";
 import { FakeAcmeClient } from "./test-support/fake-acme-client.js";
 import { InMemoryCertRateLimiter, InMemoryNameStore } from "./test-support/memory-stores.js";
 import { didKeySigner, type Signer } from "./test-support/signing.js";
-import { encodeFrame, parseFrame } from "./tunnel/protocol.js";
+import { MAX_FRAME_PAYLOAD_BYTES, encodeFrame, parseFrame } from "./tunnel/protocol.js";
 import { SUPERSEDED_CLOSE_CODE, TunnelRegistry } from "./tunnel/registry.js";
 import {
   CLOSE_INVALID_SIGNATURE,
   CLOSE_NAME_NOT_CLAIMED,
   CLOSE_NOT_OWNER,
   CLOSE_STALE_SEQUENCE,
+  type AttachTunnelUpgradeOptions,
   attachTunnelUpgrade,
 } from "./tunnel/upgrade.js";
 
 const API_HOSTNAME = "api.tinycloud.link";
 
-async function startTunnelServer() {
+type UpgradeOverrides = Partial<Omit<AttachTunnelUpgradeOptions, "registry" | "nameStore">>;
+
+async function startTunnelServer(
+  upgradeOverrides: UpgradeOverrides = {},
+  tunnelMaxBodyBytes?: number
+) {
   const nameStore = new InMemoryNameStore();
   const dnsProvider = new InMemoryDnsProvider();
   const rateLimiter = new InMemoryCertRateLimiter();
@@ -44,12 +50,13 @@ async function startTunnelServer() {
     rateLimiter,
     tunnelRegistry: registry,
     apiHostname: API_HOSTNAME,
+    tunnelMaxBodyBytes,
   });
 
   const server = await new Promise<ReturnType<typeof serve>>((resolve) => {
     const s = serve({ fetch: app.fetch, port: 0 }, () => resolve(s));
   });
-  attachTunnelUpgrade(server, { registry, nameStore, authTimeoutMs: 1000 });
+  attachTunnelUpgrade(server, { registry, nameStore, authTimeoutMs: 1000, ...upgradeOverrides });
   const port = (server.address() as AddressInfo).port;
 
   const sockets = new Set<WebSocket>();
@@ -78,15 +85,28 @@ async function startTunnelServer() {
 type TunnelTestHarness = Awaited<ReturnType<typeof startTunnelServer>>;
 
 /** Runs a test body against a fresh harness, guaranteeing the server (and any sockets it opened) is torn down even if the body throws. */
-async function withHarness(body: (harness: TunnelTestHarness) => Promise<void>): Promise<void> {
-  const harness = await startTunnelServer();
+async function withHarness(
+  body: (harness: TunnelTestHarness) => Promise<void>,
+  upgradeOverrides: UpgradeOverrides = {},
+  tunnelMaxBodyBytes?: number
+): Promise<void> {
+  const harness = await startTunnelServer(upgradeOverrides, tunnelMaxBodyBytes);
   try {
     await body(harness);
   } finally {
     await harness.close();
   }
 }
 
+/** Resolves true if the WS connection is rejected (errors or closes) before ever reaching 'open', false if it opens. Used to assert an upgrade-time limiter dropped the connection pre-handshake. */
+function connectionWasRejected(ws: WebSocket): Promise<boolean> {
+  return new Promise((resolve) => {
+    ws.once("open", () => resolve(false));
+    ws.once("error", () => resolve(true));
+    ws.once("close", () => resolve(true));
+  });
+}
+
 async function claim(app: Hono, name: string, signer: Signer, sequence: number): Promise<void> {
   const unsigned = {
     version: 1 as const,
@@ -153,7 +173,7 @@ function runEchoNode(ws: WebSocket): void {
         type: "response",
         id,
         status: 200,
-        headers: { "content-type": "text/plain" },
+        headers: [["content-type", "text/plain"]],
       })
     );
     ws.send(encodeFrame({ type: "responseBody", id, chunk: frame.chunk, done: true }));
@@ -291,3 +311,243 @@ test("newest tunnel connection wins: the older socket is evicted and the new one
     });
     assert.equal(res.status, 200);
   }));
+
+test("request headers travel to the node as an array of [name, value] pairs", () =>
+  withHarness(async (harness) => {
+    const signer = didKeySigner(111);
+    await claim(harness.app, "headerscheck", signer, 1);
+
+    const ws = harness.connect("headerscheck");
+    const ackPromise = waitForMessage(ws);
+    await sendAuth(ws, signer, "headerscheck", 2);
+    await ackPromise;
+
+    let capturedHeaders: Array<[string, string]> | undefined;
+    ws.on("message", (data) => {
+      const frame = parseFrame(data.toString());
+      if (frame.type === "request") {
+        capturedHeaders = frame.headers;
+      }
+    });
+    runEchoNode(ws);
+
+    const res = await harness.app.request("/x", {
+      headers: { host: "headerscheck.tinycloud.link", "x-custom": "hello" },
+    });
+    assert.equal(res.status, 200);
+    assert.ok(Array.isArray(capturedHeaders));
+    assert.ok(capturedHeaders?.some(([key, value]) => key.toLowerCase() === "x-custom" && value === "hello"));
+  }));
+
+test("duplicate Set-Cookie response headers survive the tunnel roundtrip", () =>
+  withHarness(async (harness) => {
+    const signer = didKeySigner(112);
+    await claim(harness.app, "cookienode", signer, 1);
+
+    const ws = harness.connect("cookienode");
+    const ackPromise = waitForMessage(ws);
+    await sendAuth(ws, signer, "cookienode", 2);
+    await ackPromise;
+
+    ws.on("message", (data) => {
+      const frame = parseFrame(data.toString());
+      if (frame.type !== "requestBody" || !frame.done) return;
+      const { id } = frame;
+      ws.send(
+        encodeFrame({
+          type: "response",
+          id,
+          status: 200,
+          headers: [
+            ["set-cookie", "a=1"],
+            ["set-cookie", "b=2"],
+          ],
+        })
+      );
+      ws.send(encodeFrame({ type: "responseBody", id, chunk: "", done: true }));
+    });
+
+    const res = await harness.app.request("/set-cookies", {
+      headers: { host: "cookienode.tinycloud.link" },
+    });
+    assert.equal(res.status, 200);
+    assert.deepEqual(res.headers.getSetCookie(), ["a=1", "b=2"]);
+  }));
+
+test("a request body larger than one body-frame chunk is split across multiple requestBody frames and reassembles correctly", () =>
+  withHarness(async (harness) => {
+    const signer = didKeySigner(113);
+    await claim(harness.app, "bigbody", signer, 1);
+
+    const ws = harness.connect("bigbody");
+    const ackPromise = waitForMessage(ws);
+    await sendAuth(ws, signer, "bigbody", 2);
+    await ackPromise;
+
+    // Larger than protocol.ts's BODY_CHUNK_BYTES (256KB), so the relay must
+    // split it across at least two requestBody frames.
+    const bigBody = "x".repeat(300 * 1024);
+
+    let requestBodyFrameCount = 0;
+    const chunks: string[] = [];
+    ws.on("message", (data) => {
+      const frame = parseFrame(data.toString());
+      if (frame.type !== "requestBody") return;
+      requestBodyFrameCount += 1;
+      chunks.push(frame.chunk);
+      if (!frame.done) return;
+      const id = frame.id;
+      const body = Buffer.concat(chunks.map((c) => Buffer.from(c, "base64"))).toString("utf8");
+      ws.send(encodeFrame({ type: "response", id, status: 200, headers: [["content-type", "text/plain"]] }));
+      ws.send(encodeFrame({ type: "responseBody", id, chunk: Buffer.from(body).toString("base64"), done: true }));
+    });
+
+    const res = await harness.app.request("/upload", {
+      method: "POST",
+      headers: { host: "bigbody.tinycloud.link" },
+      body: bigBody,
+    });
+    assert.equal(res.status, 200);
+    assert.equal(await res.text(), bigBody);
+    assert.ok(requestBodyFrameCount > 1, `expected multiple requestBody frames, got ${requestBodyFrameCount}`);
+  }));
+
+test("a request body over the configured limit is rejected with 413 before reaching the tunnel", () =>
+  withHarness(
+    async (harness) => {
+      const signer = didKeySigner(114);
+      await claim(harness.app, "toobig", signer, 1);
+
+      const ws = harness.connect("toobig");
+      const ackPromise = waitForMessage(ws);
+      await sendAuth(ws, signer, "toobig", 2);
+      await ackPromise;
+      runEchoNode(ws);
+
+      const res = await harness.app.request("/upload", {
+        method: "POST",
+        headers: { host: "toobig.tinycloud.link" },
+        body: "x".repeat(200),
+      });
+      assert.equal(res.status, 413);
+    },
+    {},
+    100 // TUNNEL_MAX_BODY_BYTES override for this test
+  ));
+
+test("a response body over the configured limit is aborted with 502 and the node is told via an error frame", () =>
+  withHarness(
+    async (harness) => {
+      const signer = didKeySigner(115);
+      await claim(harness.app, "hugeresponse", signer, 1);
+
+      const ws = harness.connect("hugeresponse");
+      const ackPromise = waitForMessage(ws);
+      await sendAuth(ws, signer, "hugeresponse", 2);
+      await ackPromise;
+
+      let sawErrorFrame = false;
+      ws.on("message", (data) => {
+        const frame = parseFrame(data.toString());
+        if (frame.type === "error") {
+          sawErrorFrame = true;
+          return;
+        }
+        if (frame.type !== "requestBody" || !frame.done) return;
+        const { id } = frame;
+        ws.send(encodeFrame({ type: "response", id, status: 200, headers: [] }));
+        // Stream a body well past the 100-byte test limit.
+        ws.send(encodeFrame({ type: "responseBody", id, chunk: Buffer.from("x".repeat(200)).toString("base64"), done: false }));
+        ws.send(encodeFrame({ type: "responseBody", id, chunk: "", done: true }));
+      });
+
+      const res = await harness.app.request("/download", {
+        headers: { host: "hugeresponse.tinycloud.link" },
+      });
+      assert.equal(res.status, 502);
+      // Give the node's message handler a tick to observe the error frame the relay sent back.
+      await new Promise((resolve) => setTimeout(resolve, 50));
+      assert.equal(sawErrorFrame, true);
+    },
+    {},
+    100 // TUNNEL_MAX_BODY_BYTES override for this test
+  ));
+
+test("the WebSocketServer enforces a max frame payload: an oversized single frame from the node closes the tunnel", () =>
+  withHarness(async (harness) => {
+    const signer = didKeySigner(116);
+    await claim(harness.app, "oversizedframe", signer, 1);
+
+    const ws = harness.connect("oversizedframe");
+    const ackPromise = waitForMessage(ws);
+    await sendAuth(ws, signer, "oversizedframe", 2);
+    await ackPromise;
+
+    const closed = waitForClose(ws);
+    const oversizedChunk = "a".repeat(MAX_FRAME_PAYLOAD_BYTES + 1024);
+    ws.send(encodeFrame({ type: "responseBody", id: "irrelevant", chunk: oversizedChunk, done: true }));
+
+    const { code } = await closed;
+    assert.equal(code, 1009); // RFC 6455 CLOSE_TOO_LARGE
+  }));
+
+test("per-IP connection attempts beyond the configured limit are dropped before the WS handshake completes", () =>
+  withHarness(
+    async (harness) => {
+      const signer = didKeySigner(117);
+      await claim(harness.app, "ratelimited", signer, 1);
+
+      const first = harness.connect("ratelimited");
+      assert.equal(await connectionWasRejected(first), false);
+      first.terminate();
+
+      const second = harness.connect("ratelimited");
+      assert.equal(await connectionWasRejected(second), false);
+      second.terminate();
+
+      // The limit is 2/minute; this third attempt within the window must be dropped pre-handshake.
+      const third = harness.connect("ratelimited");
+      assert.equal(await connectionWasRejected(third), true);
+    },
+    { ipConnectionLimitPerMinute: 2 }
+  ));
+
+test("per-name churn beyond the configured limit drops further connection attempts for that name", () =>
+  withHarness(
+    async (harness) => {
+      const signer = didKeySigner(118);
+      await claim(harness.app, "churny", signer, 1);
+
+      const first = harness.connect("churny");
+      assert.equal(await connectionWasRejected(first), false);
+      first.terminate();
+
+      const second = harness.connect("churny");
+      assert.equal(await connectionWasRejected(second), false);
+      second.terminate();
+
+      // The name-churn limit is 2/minute; this third attempt for the same name must be dropped.
+      const third = harness.connect("churny");
+      assert.equal(await connectionWasRejected(third), true);
+    },
+    { ipConnectionLimitPerMinute: 100, nameChurnLimitPerMinute: 2 }
+  ));
+
+test("a global concurrent-tunnel cap drops further connection attempts once reached", () =>
+  withHarness(
+    async (harness) => {
+      const ownerA = didKeySigner(119);
+      const ownerB = didKeySigner(120);
+      await claim(harness.app, "capped-a", ownerA, 1);
+      await claim(harness.app, "capped-b", ownerB, 1);
+
+      const first = harness.connect("capped-a");
+      const firstAck = waitForMessage(first);
+      await sendAuth(first, ownerA, "capped-a", 2);
+      await firstAck; // registry.size() === 1, at the configured cap.
+
+      const second = harness.connect("capped-b");
+      assert.equal(await connectionWasRejected(second), true);
+    },
+    { maxConcurrentTunnels: 1 }
+  ));