Merge pull request #377 from Coffey-Labs/fix/server-hardening

Bound what a request can make the server hold

.dockerignore

@@ -3,4 +3,7 @@ node_modules
 **/dist
 .git
 .env
+# deploy.example.sh keeps its settings in .env.production; any .env.* holds APP_SECRET.
+.env.*
+!.env.example
 server/data

.gitignore

@@ -1,6 +1,9 @@
 node_modules/
 dist/
 .env
+# deploy.example.sh keeps its settings in .env.production; any .env.* holds APP_SECRET.
+.env.*
+!.env.example
 *.log
 .DS_Store
 server/data/

docker-compose.yml

@@ -9,8 +9,21 @@ services:
         BASE_PATH: ${BASE_PATH:-}
     image: ihasmail:2
     restart: unless-stopped
+    # Loopback only: ihasmail expects a TLS reverse proxy in front of it. On
+    # every interface the app is reachable over plain HTTP, passwords and all,
+    # and with TRUST_PROXY any machine on a private network can set its own
+    # X-Forwarded-For. A proxy running in Docker can reach the service by name
+    # on the compose network and needs no published port at all.
     ports:
-      - "8080:8080"
+      - "127.0.0.1:8080:8080"
+    # The app needs no privileges and writes only to /data and /tmp.
+    read_only: true
+    tmpfs:
+      - /tmp
+    cap_drop:
+      - ALL
+    security_opt:
+      - no-new-privileges:true
     environment:
       STALWART_URL: ${STALWART_URL:?set STALWART_URL in .env}
       APP_SECRET: ${APP_SECRET:?set APP_SECRET in .env (openssl rand -base64 48)}

server/src/app.ts

@@ -1,6 +1,7 @@
 import { Hono } from "hono";
 import type { Context, MiddlewareHandler } from "hono";
 import { getCookie, setCookie, deleteCookie } from "hono/cookie";
+import { bodyLimit } from "hono/body-limit";
 import { compress } from "hono/compress";
 import { request as httpRequest } from "node:http";
 import { request as httpsRequest } from "node:https";
@@ -12,7 +13,7 @@ import { fetchPermissions } from "./permissionSchema.js";
 import { administrationAllowed, gateAdministration, grantsAdministration } from "./adminGate.js";
 import { SessionStore, type SessionBackend, type LiveSession } from "./sessions.js";
 import { RateLimiter } from "./ratelimit.js";
-import { resolveClientIp } from "./clientip.js";
+import { rateLimitKey, resolveClientIp } from "./clientip.js";
 import {
   type AccountInfo,
   UpstreamError,
@@ -209,6 +210,22 @@ const csrfGuard: MiddlewareHandler = async (c, next) => {
   await next();
 };
 
+/**
+ * The largest body an API route that reads JSON will take.
+ *
+ * Hono reads a JSON body whole, and before this nothing bounded it: a few
+ * unauthenticated sign-in attempts carrying hundreds of megabytes each could
+ * run the process out of memory, and a restart signs everybody out. What
+ * these routes actually receive is a username and password, or a code.
+ *
+ * JMAP and uploads carry real payloads and bound themselves as they stream;
+ * the push callback has its own limit ahead of this one.
+ */
+const MAX_SMALL_BODY = 64 * 1024;
+const LARGE_BODY_ROUTE = /\/api\/(jmap$|upload\/)/;
+const limitSmallBody = bodyLimit({ maxSize: MAX_SMALL_BODY, onError: (c) => c.json({ error: "too_large" }, 413) });
+const smallBodies: MiddlewareHandler = (c, next) => (LARGE_BODY_ROUTE.test(c.req.path) ? next() : limitSmallBody(c, next));
+
 const requireSession: MiddlewareHandler<Env> = async (c, next) => {
   const cookie = getCookie(c, config.cookieName);
   const session = sessions.resolve(cookie);
@@ -269,6 +286,7 @@ export function createApp(basePath = config.basePath): Hono<Env> {
 
   const api = new Hono<Env>();
   api.use("*", csrfGuard);
+  api.use("*", smallBodies);
 
   api.get("/health", (c) => c.json({ ok: true, name: config.appName, version: config.version, push: pushStatus() }));
 
@@ -303,6 +321,13 @@ export function createApp(basePath = config.basePath): Hono<Env> {
   // ---------- Auth ----------
   api.post("/auth/login", async (c) => {
     const ip = clientIp(c);
+    // What the limits count under: the address, or its /64 for IPv6.
+    const rateIp = rateLimitKey(ip);
+    // The flood ceiling needs nothing from the body, so it goes before reading one.
+    if (!loginFloodLimiter.check(rateIp)) {
+      c.header("Retry-After", String(loginFloodLimiter.retryAfterSeconds(rateIp)));
+      return c.json({ error: "rate_limited", message: "Too many login attempts. Please wait and try again." }, 429);
+    }
     let body: { username?: string; password?: string; totp?: string; remember?: boolean };
     try {
       body = await c.req.json();
@@ -318,7 +343,7 @@ export function createApp(basePath = config.basePath): Hono<Env> {
     /*
      * Three checks, answering different questions.
      *
-     * `limitKey` is this username from this address, and `ip` is any username
+     * `limitKey` is this username from this address, and `rateIp` is any username
      * from it -- both guard guessing, and both are given back when the upstream
      * never got as far as judging the password. Refunding only the first would
      * not fix #239: ten retries through an outage would still spend the address
@@ -328,12 +353,8 @@ export function createApp(basePath = config.basePath): Hono<Env> {
      * The flood ceiling is the one that is never refunded, and it is the reason
      * the other two safely can be.
      */
-    const limitKey = `${ip}|${username.toLowerCase()}`;
-    if (!loginFloodLimiter.check(ip)) {
-      c.header("Retry-After", String(loginFloodLimiter.retryAfterSeconds(ip)));
-      return c.json({ error: "rate_limited", message: "Too many login attempts. Please wait and try again." }, 429);
-    }
-    if (!loginLimiter.check(limitKey) || !loginLimiter.check(ip)) {
+    const limitKey = `${rateIp}|${username.toLowerCase()}`;
+    if (!loginLimiter.check(limitKey) || !loginLimiter.check(rateIp)) {
       c.header("Retry-After", String(loginLimiter.retryAfterSeconds(limitKey)));
       return c.json({ error: "rate_limited", message: "Too many login attempts. Please wait and try again." }, 429);
     }
@@ -351,7 +372,7 @@ export function createApp(basePath = config.basePath): Hono<Env> {
         // The credentials were accepted; only the server is too old. Not an
         // attempt worth counting against them.
         loginLimiter.refund(limitKey);
-        loginLimiter.refund(ip);
+        loginLimiter.refund(rateIp);
         return c.json(
           {
             error: "unsupported_server",
@@ -411,7 +432,7 @@ export function createApp(basePath = config.basePath): Hono<Env> {
        */
       if (!(err instanceof UpstreamError && err.status === 401)) {
         loginLimiter.refund(limitKey);
-        loginLimiter.refund(ip);
+        loginLimiter.refund(rateIp);
       }
       return upstreamFailure(c, err);
     }
@@ -648,12 +669,27 @@ export function createApp(basePath = config.basePath): Hono<Env> {
      */
     let body: ReadableStream<Uint8Array> | string | null = c.req.raw.body;
     if (!administrationAllowed(config.administration, session.remember)) {
+      const held = gatedReads.get(session.id) ?? 0;
+      if (held >= MAX_GATED_PER_SESSION) {
+        c.header("Retry-After", "1");
+        return c.json({ error: "rate_limited" }, 429);
+      }
+      gatedReads.set(session.id, held + 1);
       let raw: string;
       try {
+        if (Number(c.req.header("content-length") ?? "0") > MAX_GATED_REQUEST) return c.json({ error: "too_large" }, 413);
         // Counted as it arrives: a chunked body carries no length to refuse up front.
-        raw = c.req.raw.body ? await new Response(c.req.raw.body.pipeThrough(byteCap(MAX_GATED_REQUEST))).text() : "";
-      } catch {
+        raw = c.req.raw.body ? await readGated(c.req.raw.body) : "";
+      } catch (err) {
+        if (err instanceof GatedBudgetError) {
+          c.header("Retry-After", "1");
+          return c.json({ error: "busy" }, 503);
+        }
         return c.json({ error: "too_large" }, 413);
+      } finally {
+        const left = (gatedReads.get(session.id) ?? 1) - 1;
+        if (left > 0) gatedReads.set(session.id, left);
+        else gatedReads.delete(session.id);
       }
       const gate = gateAdministration(raw);
       if (!gate.ok) {
@@ -931,9 +967,50 @@ function sessionExtras(session: LiveSession, info: AccountInfo = { locale: null,
  */
 /**
  * The largest JMAP request read into memory for the administration check.
- * Stalwart's own default `maxSizeRequest` is 10 MB; uploads never come this way.
+ *
+ * Only sessions that may not administer come this way, and what the client
+ * sends is small: attachments and pasted images go through `/upload`, and the
+ * composer turns inline images into uploads before a draft is saved. Stalwart
+ * would take up to its `maxSizeRequest` (10 MB by default), but a request is
+ * held here as a string, parsed and serialized again, so each one costs
+ * several times its size; 4 MB is far past anything the client sends.
  */
-const MAX_GATED_REQUEST = 16 * 1024 * 1024;
+const MAX_GATED_REQUEST = 4 * 1024 * 1024;
+/**
+ * How many checked requests one session may have in flight at once. Matches
+ * the `maxConcurrentRequests` Stalwart advertises by default, which the client
+ * already stays within.
+ */
+const MAX_GATED_PER_SESSION = 4;
+/**
+ * The bytes all checked requests together may hold at once. Counted as they
+ * arrive rather than reserved up front, so a slow body that has sent little
+ * holds little, and a burst of large ones is turned away with a 503 instead of
+ * taking the process down.
+ */
+const GATED_BUDGET = 32 * 1024 * 1024;
+const gatedReads = new Map<string, number>();
+let gatedBytes = 0;
+
+class GatedBudgetError extends Error {}
+
+async function readGated(stream: ReadableStream<Uint8Array>): Promise<string> {
+  let mine = 0;
+  const counted = new TransformStream<Uint8Array, Uint8Array>({
+    transform(chunk, controller) {
+      mine += chunk.byteLength;
+      gatedBytes += chunk.byteLength;
+      if (mine > MAX_GATED_REQUEST) controller.error(new Error("request too large"));
+      else if (gatedBytes > GATED_BUDGET) controller.error(new GatedBudgetError("gated read budget spent"));
+      else controller.enqueue(chunk);
+    },
+  });
+  try {
+    return await new Response(stream.pipeThrough(counted)).text();
+  } finally {
+    gatedBytes -= mine;
+  }
+}
 
 const PASSTHROUGH_HEADERS = new Set(["content-type", "content-disposition", "content-language", "etag", "last-modified", "retry-after"]);
 

server/src/clientip.ts

@@ -97,3 +97,21 @@ export function resolveClientIp(peer: string, headers: ForwardHeaders, cfg: Trus
   const real = headers.realIp?.trim();
   return real && isIP(real) !== 0 ? real : peer;
 }
+
+/**
+ * The key a rate limit counts an address under.
+ *
+ * An IPv4 address is the key as it is. An IPv6 address is cut to its /64: that
+ * is the smallest block an ISP or a VPS hands out, so anyone who holds one
+ * address holds 2^64 of them, and a limit keyed on the full address is no
+ * limit. Everyone behind one /64 shares a budget, which is the same bargain an
+ * IPv4 NAT already makes.
+ */
+export function rateLimitKey(ip: string): string {
+  if (isIP(ip) !== 6) return ip;
+  const bits = toBits(ip);
+  if (!bits) return ip;
+  const prefix = bits.value >> 64n;
+  const groups = [48n, 32n, 16n, 0n].map((s) => ((prefix >> s) & 0xffffn).toString(16));
+  return `${groups.join(":")}::/64`;
+}

server/src/request-limits.test.ts

@@ -0,0 +1,115 @@
+import { test, before, after } from "node:test";
+import assert from "node:assert/strict";
+
+/**
+ * How much a request may make the proxy hold in memory.
+ *
+ * Routes that read JSON take a small body and no more, whether or not anyone
+ * is signed in. The JMAP route streams straight through for a session that may
+ * administer; for one that may not, it reads the body to check it, and that
+ * read is capped in size, in how many one session runs at once, and in bytes
+ * across everyone.
+ */
+
+const PORT = 18813;
+process.env.MOCK_PORT = String(PORT);
+process.env.MOCK_USER = "[email protected]";
+process.env.MOCK_PASS = "demo-password";
+process.env.STALWART_URL = `http://127.0.0.1:${PORT}`;
+process.env.APP_SECRET = "test-secret-for-request-limits";
+
+const mock = await import("./mock/index.js");
+const { createApp } = await import("./app.js");
+const { rateLimitKey } = await import("./clientip.js");
+
+const app = createApp();
+const HEADERS = { "content-type": "application/json", "x-requested-with": "ihasmail" };
+let cookie = "";
+
+/** A body that arrives in chunks with no content-length, as a chunked upload does. */
+function chunked(size: number, chunk = 256 * 1024): ReadableStream<Uint8Array> {
+  let sent = 0;
+  return new ReadableStream({
+    pull(controller) {
+      if (sent >= size) return controller.close();
+      const n = Math.min(chunk, size - sent);
+      controller.enqueue(new Uint8Array(n).fill(0x20));
+      sent += n;
+    },
+  });
+}
+
+const jmap = (body: BodyInit) =>
+  app.request("/api/jmap", { method: "POST", headers: { ...HEADERS, cookie }, body, duplex: "half" } as RequestInit);
+
+before(async () => {
+  // Not remembered: a device that is not the person's own, so JMAP is checked.
+  const res = await app.request("/api/auth/login", { method: "POST", headers: HEADERS, body: JSON.stringify({ username: "[email protected]", password: "demo-password" }) });
+  assert.equal(res.status, 200, "login should succeed against the mock");
+  cookie = res.headers.get("set-cookie")!.split(";")[0]!;
+});
+
+after(() => {
+  (mock as { server?: { close(): void } }).server?.close();
+});
+
+test("sign-in refuses a large body by its length, before reading it", async () => {
+  const res = await app.request("/api/auth/login", {
+    method: "POST",
+    headers: { ...HEADERS, "content-length": String(200 * 1024 * 1024) },
+    body: "{}",
+  });
+  assert.equal(res.status, 413);
+});
+
+test("sign-in refuses a large chunked body without holding all of it", async () => {
+  const res = await app.request("/api/auth/login", { method: "POST", headers: HEADERS, body: chunked(2 * 1024 * 1024), duplex: "half" } as RequestInit);
+  assert.equal(res.status, 413);
+});
+
+test("other JSON routes are limited too", async () => {
+  const res = await app.request("/api/account/password", { method: "POST", headers: { ...HEADERS, cookie }, body: chunked(1024 * 1024), duplex: "half" } as RequestInit);
+  assert.equal(res.status, 413);
+});
+
+test("an ordinary checked JMAP request still goes through", async () => {
+  const res = await jmap(JSON.stringify({ using: ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail"], methodCalls: [["Mailbox/get", { accountId: "a1", ids: [] }, "0"]] }));
+  assert.equal(res.status, 200);
+});
+
+test("a JMAP request larger than the check allows is refused", async () => {
+  assert.equal((await jmap(chunked(5 * 1024 * 1024))).status, 413);
+});
+
+test("a JMAP request larger than a sign-in body is not caught by the small-body limit", async () => {
+  // 200 KB of whitespace around a real request: valid JSON, well past 64 KB.
+  const body = `${" ".repeat(200 * 1024)}{"using":["urn:ietf:params:jmap:core"],"methodCalls":[["Core/echo",{},"0"]]}`;
+  assert.equal((await jmap(body)).status, 200);
+});
+
+test("one session cannot hold more than a few checked reads at once", async () => {
+  // Bodies that never finish: each holds its slot until its stream fails.
+  const controllers: ReadableStreamDefaultController<Uint8Array>[] = [];
+  const pending: Promise<Response>[] = [];
+  for (let i = 0; i < 4; i++) {
+    const s = new ReadableStream<Uint8Array>({ start(c) { controllers.push(c); c.enqueue(new TextEncoder().encode("{")); } });
+    pending.push(jmap(s));
+  }
+  await new Promise((r) => setTimeout(r, 50));
+  const fifth = await jmap("{}");
+  assert.equal(fifth.status, 429);
+  assert.ok(fifth.headers.get("retry-after"));
+  for (const c of controllers) c.error(new Error("client went away"));
+  await Promise.allSettled(pending);
+  // The slots are given back once those requests end.
+  const again = await jmap(JSON.stringify({ using: ["urn:ietf:params:jmap:core"], methodCalls: [["Core/echo", {}, "0"]] }));
+  assert.equal(again.status, 200);
+});
+
+test("IPv6 addresses share a rate-limit key across their /64", () => {
+  assert.equal(rateLimitKey("2001:db8:1:2:aaaa::1"), rateLimitKey("2001:db8:1:2:ffff:ffff:ffff:ffff"));
+  assert.notEqual(rateLimitKey("2001:db8:1:2::1"), rateLimitKey("2001:db8:1:3::1"));
+  assert.equal(rateLimitKey("2001:db8:1:2::1"), "2001:db8:1:2::/64");
+  assert.equal(rateLimitKey("198.51.100.7"), "198.51.100.7");
+  assert.equal(rateLimitKey("unknown"), "unknown");
+});