diff --git a/README.md b/README.md index a3c7e61..e5a524f 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,12 @@ in-memory database. Set `DESTROYER_DB_PATH` to retain data elsewhere. Production requires `DESTROYER_JWT_PRIVATE_KEY` containing an RSA private JWK; `DESTROYER_JWT_KID`, `HOST`, and `PORT` are configurable. +Contact and authentication rate limits use the TCP peer address authenticated by `@askrjs/node`. +Client-supplied `X-Forwarded-For` and `x-askr-client-address` values are never trusted; the Node +adapter overwrites its reserved header from the socket. A deployment behind a reverse proxy is +therefore limited by the proxy peer unless a separate, explicit trusted-proxy boundary is added. +Do not enable original-client forwarding by reading `X-Forwarded-For` directly in application code. + ## Askr packages Destroyer installs ranged releases from the npm registry for `@askrjs/askr`, auth, charts, lucide, diff --git a/package-lock.json b/package-lock.json index 3e0a163..a7e1169 100644 --- a/package-lock.json +++ b/package-lock.json @@ -94,9 +94,9 @@ } }, "node_modules/@askrjs/node": { - "version": "0.0.9", - "resolved": "https://registry.npmjs.org/@askrjs/node/-/node-0.0.9.tgz", - "integrity": "sha512-2sVn1crf0We/q/h7LVm3llFrkgcx/LqbtT7HRpZlHZkGMFG7JNxYBloLCAbGU5nE9AcaqZpac29w1VwWTsYE8g==", + "version": "0.0.11", + "resolved": "https://registry.npmjs.org/@askrjs/node/-/node-0.0.11.tgz", + "integrity": "sha512-2hRK6lC//JBo7hfsoemHXhMFXiHs9EpSWTU6Q+tnQ99xelgxKpNTplDZoiI+DMZWxq6HgrjEz+vIQLBj5S55mg==", "license": "Apache-2.0", "dependencies": { "@askrjs/auth": ">=0.0.8 <0.1.0", diff --git a/src/server/api.ts b/src/server/api.ts index 7919531..b3f2e3d 100644 --- a/src/server/api.ts +++ b/src/server/api.ts @@ -4,6 +4,7 @@ import type { AskrAppApi } from "@askrjs/server/askr"; import { security } from "@askrjs/server/openapi"; import type { AppDependencies } from "./contracts"; import { RepositoryConflictError } from "./contracts"; +import { clientAddress } from "./client-address"; export function defineOperationsApi(api: AskrAppApi) { const Summary = api.schema( @@ -196,7 +197,7 @@ export function defineOperationsApi(api: AskrAppApi) { input: { body: { schema: ContactInput, mediaTypes: ["application/json"] } }, documentation: { body: { required: true } }, async handler(ctx, input, deps) { - const address = ctx.headers.get("x-forwarded-for")?.split(",")[0]?.trim() ?? "local"; + const address = clientAddress(ctx.headers); const limit = await deps.rateLimits.consume( `contact:${address}:${input.body.email.toLowerCase()}`, 3, diff --git a/src/server/app.ts b/src/server/app.ts index 848d6e7..eb1d7b2 100644 --- a/src/server/app.ts +++ b/src/server/app.ts @@ -12,6 +12,7 @@ import { SESSION_COOKIE } from "./dependencies"; import { RepositoryConflictError } from "./contracts"; import { createQueryRegistry } from "./queries"; import { settingsActionHandlers } from "./actions"; +import { clientAddress } from "./client-address"; export function createApp(deps: AppDependencies, issuer: JwtIssuer) { const principalSchema: Schema = { @@ -59,7 +60,7 @@ export function createApp(deps: AppDependencies, issuer: JwtIssuer) { allowAttempt: async (ctx, operation, email) => ( await deps.rateLimits.consume( - `auth:${operation}:${email}:${ctx.headers.get("x-forwarded-for")?.split(",")[0]?.trim() ?? "local"}`, + `auth:${operation}:${email}:${clientAddress(ctx.headers)}`, 5, 15 * 60_000, ) diff --git a/src/server/client-address.ts b/src/server/client-address.ts new file mode 100644 index 0000000..d408f33 --- /dev/null +++ b/src/server/client-address.ts @@ -0,0 +1,6 @@ +import { CLIENT_ADDRESS_HEADER } from "@askrjs/node"; + +/** Returns the Node-adapter-authenticated TCP peer used by IP-keyed security controls. */ +export function clientAddress(headers: Headers): string { + return headers.get(CLIENT_ADDRESS_HEADER) ?? "unknown"; +} diff --git a/tests/full-stack.test.ts b/tests/full-stack.test.ts index 76f713a..10ad2e3 100644 --- a/tests/full-stack.test.ts +++ b/tests/full-stack.test.ts @@ -1,4 +1,5 @@ import { createJwtIssuer } from "@askrjs/auth/jwt"; +import { CLIENT_ADDRESS_HEADER, listen } from "@askrjs/node"; import { generateKeyPairSync } from "node:crypto"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; @@ -150,6 +151,82 @@ describe("Destroyer full stack", () => { expect(await deps.contacts.count()).toBe(3); }); + it("should hold contact and authentication limits against spoofed forwarding addresses", async () => { + const deps = dependencies(); + const app = testApp(deps); + const server = await listen(app, { host: "127.0.0.1" }); + const address = server.address(); + if (!address || typeof address === "string") throw new Error("Expected TCP address"); + const origin = `http://127.0.0.1:${address.port}`; + const post = (path: string, body: unknown, spoofed: string) => + fetch(`${origin}${path}`, { + method: "POST", + redirect: "manual", + headers: { + "content-type": "application/json", + origin, + [CLIENT_ADDRESS_HEADER]: spoofed, + "x-forwarded-for": spoofed, + }, + body: JSON.stringify(body), + }); + + try { + const contactStatuses: number[] = []; + for (let index = 0; index < 4; index += 1) { + contactStatuses.push( + ( + await post( + "/api/contact", + { + email: "spoof-proof@example.test", + subject: "Need help", + message: "A spoof-boundary request from the integration suite.", + }, + `198.51.100.${index + 1}`, + ) + ).status, + ); + } + expect( + ( + await post( + "/auth/v1/accounts", + { email: "rate-limited@example.test", password: "destroyer" }, + "203.0.113.1", + ) + ).status, + ).toBe(303); + const loginStatuses: number[] = []; + const emailVariants = [ + "rate-limited@example.test", + "Rate-Limited@example.test", + "RATE-LIMITED@example.test", + "rate-limited@EXAMPLE.test", + "rate-limited@example.TEST", + "RATE-limited@EXAMPLE.TEST", + ]; + for (let index = 0; index < 6; index += 1) { + loginStatuses.push( + ( + await post( + "/auth/v1/session", + { email: emailVariants[index], password: "incorrect" }, + `203.0.113.${index + 10}`, + ) + ).status, + ); + } + expect({ contactStatuses, loginStatuses, persisted: await deps.contacts.count() }).toEqual({ + contactStatuses: [201, 201, 201, 429], + loginStatuses: [401, 401, 401, 401, 401, 429], + persisted: 3, + }); + } finally { + await new Promise((resolve) => server.close(() => resolve())); + } + }); + it("should reject malformed support payloads given invalid contact input", async () => { const response = await testApp(dependencies()).fetch( new Request("http://destroyer.test/api/contact", {