From 5d60d9c88b3aa858d4dee9bbb9ea618577f63e31 Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Sat, 10 Oct 2026 19:10:15 -0400 Subject: [PATCH] refactor(http): internalAuth route option and a shared bodyRecord EventsController and PeerController each checked x-puter-internal-auth by hand with different compares; an internalAuth RouteOption now picks the secret from config and one constant-time gate checks it. ShareController and EventsController share bodyRecord instead of private copies. --- .../controllers/events/EventsController.ts | 48 ++++------------ .../controllers/peer/PeerController.test.ts | 50 ++++------------ .../controllers/peer/PeerController.ts | 31 +--------- .../controllers/share/ShareController.ts | 19 ++----- .../core/http/middleware/internalAuth.test.ts | 57 +++++++++++++++++++ .../core/http/middleware/internalAuth.ts | 53 +++++++++++++++++ src/backend/core/http/requestBody.test.ts | 42 ++++++++++++++ src/backend/core/http/requestBody.ts | 32 +++++++++++ src/backend/core/http/types.ts | 10 +++- src/backend/server.ts | 9 ++- 10 files changed, 228 insertions(+), 123 deletions(-) create mode 100644 src/backend/core/http/middleware/internalAuth.test.ts create mode 100644 src/backend/core/http/middleware/internalAuth.ts create mode 100644 src/backend/core/http/requestBody.test.ts create mode 100644 src/backend/core/http/requestBody.ts diff --git a/src/backend/controllers/events/EventsController.ts b/src/backend/controllers/events/EventsController.ts index 238e2e84c..d5480cac1 100644 --- a/src/backend/controllers/events/EventsController.ts +++ b/src/backend/controllers/events/EventsController.ts @@ -17,11 +17,11 @@ * along with this program. If not, see . */ -import { timingSafeEqual } from 'node:crypto'; import type { Request, Response } from 'express'; import type { Actor } from '../../core/actor.js'; import { Controller, Delete, Get, Post } from '../../core/http/decorators.js'; import { HttpError } from '../../core/http/HttpError.js'; +import { bodyRecord } from '../../core/http/requestBody.js'; import { DURABLE_LIST_LIMIT_CAP } from '../../stores/events/DurableSubscriptionStore.js'; import { EVENTS_WORKERS_LIST_LIMIT_CAP } from '../../stores/events/EventHandlerStore.js'; import { KV_HANDLE_LIST_LIMIT_CAP } from '../../stores/events/KvShareHandleStore.js'; @@ -75,7 +75,7 @@ export class EventsController extends PuterController { const actor = this.#requireActor(req); const { sub } = await this.services.events.subscribeDurable( actor, - this.#body(req), + bodyRecord(req), ); res.json(sub); } @@ -147,7 +147,7 @@ export class EventsController extends PuterController { }) async unsubscribe(req: Request, res: Response): Promise { const actor = this.#requireActor(req); - await this.services.events.unsubscribeDurable(actor, this.#body(req)); + await this.services.events.unsubscribeDurable(actor, bodyRecord(req)); res.json({}); } @@ -171,7 +171,7 @@ export class EventsController extends PuterController { async mintKvHandle(req: Request, res: Response): Promise { const actor = this.#requireActor(req); res.json( - await this.services.events.mintKvHandle(actor, this.#body(req)), + await this.services.events.mintKvHandle(actor, bodyRecord(req)), ); } @@ -236,7 +236,7 @@ export class EventsController extends PuterController { async publishHandler(req: Request, res: Response): Promise { const actor = this.#requireActor(req); res.json( - await this.services.events.publishHandler(actor, this.#body(req)), + await this.services.events.publishHandler(actor, bodyRecord(req)), ); } @@ -251,7 +251,7 @@ export class EventsController extends PuterController { res.json({ handlers: await this.services.events.publishHandlers( actor, - this.#body(req), + bodyRecord(req), ), }); } @@ -283,7 +283,7 @@ export class EventsController extends PuterController { async removeHandler(req: Request, res: Response): Promise { const actor = this.#requireActor(req); res.json( - await this.services.events.removeHandler(actor, this.#body(req)), + await this.services.events.removeHandler(actor, bodyRecord(req)), ); } @@ -335,7 +335,7 @@ export class EventsController extends PuterController { res.json( await this.services.events.destroyEventsWorker( actor, - this.#body(req), + bodyRecord(req), ), ); } @@ -348,6 +348,7 @@ export class EventsController extends PuterController { */ @Post('/worker/rehydrate', { subdomain: 'api', + internalAuth: (config) => config.events?.internalSecret, // The shared secret is what gates this route, so the limit is only a // guessing-rate bound — hence per-IP and deliberately high. The caller // fans in from every edge location behind a small set of egress @@ -360,8 +361,7 @@ export class EventsController extends PuterController { }, }) async rehydrateWorker(req: Request, res: Response): Promise { - this.#requireInternalAuth(req); - const body = this.#body(req); + const body = bodyRecord(req); const script = String(body.script ?? ''); const appUid = String(body.appUid ?? ''); if (!EVENTS_SCRIPT_REGEX.test(script) || !APP_UID_REGEX.test(appUid)) @@ -428,25 +428,6 @@ export class EventsController extends PuterController { return this.#deployer; } - /** - * Constant-time, so the secret cannot be recovered a byte at a time. - * Compares Buffer byte lengths, not string lengths — a string length - * compares UTF-16 code units, which can differ from the byte length - * `timingSafeEqual` actually requires to match. - */ - #requireInternalAuth(req: Request): void { - const expected = Buffer.from(this.config.events?.internalSecret ?? ''); - const offered = Buffer.from( - String(req.headers['x-puter-internal-auth'] ?? ''), - ); - const ok = - expected.length > 0 && - expected.length === offered.length && - timingSafeEqual(expected, offered); - if (!ok) - throw new HttpError(403, 'Forbidden', { legacyCode: 'forbidden' }); - } - #requireActor(req: Request): Actor { const actor = req.actor; if (!actor?.user) @@ -455,13 +436,4 @@ export class EventsController extends PuterController { }); return actor; } - - #body(req: Request): Record { - const body = req.body; - if (!body || typeof body !== 'object' || Array.isArray(body)) - throw new HttpError(400, 'body must be an object', { - legacyCode: 'bad_request', - }); - return body as Record; - } } diff --git a/src/backend/controllers/peer/PeerController.test.ts b/src/backend/controllers/peer/PeerController.test.ts index afa93def0..64f6c53f6 100644 --- a/src/backend/controllers/peer/PeerController.test.ts +++ b/src/backend/controllers/peer/PeerController.test.ts @@ -172,21 +172,17 @@ describe('PeerController', () => { ingestHandler = route!.handler; }); - it('rejects requests without valid internal auth secret', async () => { - const req = makeReq({ - body: { records: [] }, - headers: { 'x-puter-internal-auth': 'wrong-secret' }, - }); - await expect( - ingestHandler(req, makeRes().res), - ).rejects.toMatchObject({ statusCode: 403 }); - }); - - it('rejects requests with missing auth header', async () => { - const req = makeReq({ body: { records: [] } }); - await expect( - ingestHandler(req, makeRes().res), - ).rejects.toMatchObject({ statusCode: 403 }); + it('is gated on the configured internal auth secret', () => { + const router = new PuterRouter(); + controller.registerRoutes(router); + const route = router.routes.find( + (r) => r.path === '/turn/ingest-usage', + )!; + expect( + route.options.internalAuth?.({ + peers: { internal_auth_secret: 'test-secret' }, + } as never), + ).toBe('test-secret'); }); it('rejects when records is not an array', async () => { @@ -537,30 +533,6 @@ describe('PeerController TURN', () => { warnSpy.mockRestore(); } }); - - it('rejects usage ingest when no internal secret is configured', async () => { - const openServer = await setupTestServer(); - try { - const router = new PuterRouter(); - ( - openServer.controllers.peer as unknown as PeerController - ).registerRoutes(router); - const handler = router.routes.find( - (r) => r.path === '/turn/ingest-usage', - )!.handler; - await expect( - handler( - makeReq({ - body: { records: [] }, - headers: { 'x-puter-internal-auth': 'anything' }, - }), - makeRes().res, - ), - ).rejects.toMatchObject({ statusCode: 403 }); - } finally { - await openServer.shutdown(); - } - }); }); // -- Guest TURN access ------------------------------------------------- diff --git a/src/backend/controllers/peer/PeerController.ts b/src/backend/controllers/peer/PeerController.ts index 5683d678f..329c81e4f 100644 --- a/src/backend/controllers/peer/PeerController.ts +++ b/src/backend/controllers/peer/PeerController.ts @@ -17,7 +17,6 @@ * along with this program. If not, see . */ -import { createHmac, randomBytes, timingSafeEqual } from 'node:crypto'; import type { Request, Response } from 'express'; import { makeActor, type Actor } from '../../core/actor.js'; import { HttpError } from '../../core/http/HttpError.js'; @@ -46,21 +45,6 @@ const DEFAULT_GRANT_TTL = 3600; */ const DEFAULT_GUEST_CREDENTIAL_TTL = 3600; -/** - * Constant-time secret comparison for the internal-auth header. HMAC both sides - * under a random per-process key to a fixed 32-byte digest first: this avoids - * leaking length via an early-return and sidesteps `timingSafeEqual`'s - * equal-length requirement for arbitrary-length inputs. The key need not - * persist — it only has to be unknown to the attacker for the duration of the - * comparison. - */ -const COMPARE_KEY = randomBytes(32); -const secretsEqual = (a: string, b: string): boolean => { - const ha = createHmac('sha256', COMPARE_KEY).update(a).digest(); - const hb = createHmac('sha256', COMPARE_KEY).update(b).digest(); - return timingSafeEqual(ha, hb); -}; - /** * Encode a UUID (or `app-` UID) as base64url with no padding. Strips an * `app-` prefix and dashes, then reinterprets the hex bytes. @@ -230,6 +214,7 @@ export class PeerController extends PuterController { '/turn/ingest-usage', { subdomain: 'api', + internalAuth: (config) => config.peers?.internal_auth_secret, // Shared-secret authenticated, so this only bounds how // fast someone can guess the secret. rateLimit: { @@ -415,20 +400,6 @@ export class PeerController extends PuterController { * `services.metering.incrementUsage` multiplied by turn:egress-bytes cost. */ #ingestUsage = async (req: Request, res: Response): Promise => { - const cfg = this.config.peers; - if (!cfg || !cfg.internal_auth_secret) { - throw new HttpError(403, 'Forbidden', { legacyCode: 'forbidden' }); - } - const expectedSecret = cfg.internal_auth_secret; - const header = req.headers['x-puter-internal-auth']; - if ( - !expectedSecret || - typeof header !== 'string' || - !secretsEqual(header, expectedSecret) - ) { - throw new HttpError(403, 'Forbidden', { legacyCode: 'forbidden' }); - } - const { records } = req.body ?? {}; if (!Array.isArray(records)) { throw new HttpError(400, 'Missing `records` array', { diff --git a/src/backend/controllers/share/ShareController.ts b/src/backend/controllers/share/ShareController.ts index 515346050..47746e3d3 100644 --- a/src/backend/controllers/share/ShareController.ts +++ b/src/backend/controllers/share/ShareController.ts @@ -21,6 +21,7 @@ import type { Request, Response } from 'express'; import type { Actor } from '../../core/actor.js'; import { Controller, Delete, Get, Post } from '../../core/http/decorators.js'; import { HttpError, isHttpError } from '../../core/http/HttpError.js'; +import { bodyRecord } from '../../core/http/requestBody.js'; import type { ResolvedShare, ShareRecipient, @@ -96,7 +97,7 @@ export class ShareController extends PuterController { }) async createShares(req: Request, res: Response): Promise { const actor = this.#requireActor(req); - const body = this.#body(req); + const body = bodyRecord(req); const recipients = this.#recipients(body); const items = this.#items(body, actor); const mode = typeof body.mode === 'string' ? body.mode : 'read'; @@ -183,7 +184,7 @@ export class ShareController extends PuterController { }) async revokeShare(req: Request, res: Response): Promise { const actor = this.#requireActor(req); - const body = this.#body(req); + const body = bodyRecord(req); const recipients = this.#recipients(body); const items = this.#items(body, actor); @@ -484,7 +485,7 @@ export class ShareController extends PuterController { }) async createBlock(req: Request, res: Response): Promise { const actor = this.#requireActor(req); - const body = this.#body(req); + const body = bodyRecord(req); if (this.#isBlockAll(body)) { const { all } = await this.services.share.setBlockAllSenders( actor, @@ -512,7 +513,7 @@ export class ShareController extends PuterController { }) async deleteBlock(req: Request, res: Response): Promise { const actor = this.#requireActor(req); - const body = this.#body(req); + const body = bodyRecord(req); if (this.#isBlockAll(body)) { const { all } = await this.services.share.setBlockAllSenders( actor, @@ -596,16 +597,6 @@ export class ShareController extends PuterController { return actor; } - #body(req: Request): Record { - const body = req.body; - if (!body || typeof body !== 'object' || Array.isArray(body)) { - throw new HttpError(400, 'body must be an object', { - legacyCode: 'bad_request', - }); - } - return body as Record; - } - #query(req: Request): Record { return (req.query ?? {}) as Record; } diff --git a/src/backend/core/http/middleware/internalAuth.test.ts b/src/backend/core/http/middleware/internalAuth.test.ts new file mode 100644 index 000000000..4cc7b4afa --- /dev/null +++ b/src/backend/core/http/middleware/internalAuth.test.ts @@ -0,0 +1,57 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import type { Request, Response } from 'express'; +import { describe, expect, it } from 'vitest'; +import { internalAuthGate } from './internalAuth'; + +const run = (secret: string | undefined, header?: string): unknown => { + let outcome: unknown = 'not called'; + internalAuthGate(() => secret)( + { + headers: + header === undefined ? {} : { 'x-puter-internal-auth': header }, + } as unknown as Request, + {} as Response, + (err?: unknown) => { + outcome = err; + }, + ); + return outcome; +}; + +describe('internalAuthGate', () => { + it('passes the configured secret through', () => { + expect(run('s3cret', 's3cret')).toBeUndefined(); + }); + + it('refuses a wrong, shorter or missing secret with 403', () => { + for (const header of ['wrong!', 's3cre', 's3cret-longer', undefined]) { + expect(run('s3cret', header)).toMatchObject({ + statusCode: 403, + legacyCode: 'forbidden', + }); + } + }); + + it('refuses everything when no secret is configured', () => { + expect(run(undefined, '')).toMatchObject({ statusCode: 403 }); + expect(run('', '')).toMatchObject({ statusCode: 403 }); + }); +}); diff --git a/src/backend/core/http/middleware/internalAuth.ts b/src/backend/core/http/middleware/internalAuth.ts new file mode 100644 index 000000000..175cea6d7 --- /dev/null +++ b/src/backend/core/http/middleware/internalAuth.ts @@ -0,0 +1,53 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import type { RequestHandler } from 'express'; +import { createHmac, randomBytes, timingSafeEqual } from 'node:crypto'; +import { HttpError } from '../HttpError'; + +export const INTERNAL_AUTH_HEADER = 'x-puter-internal-auth'; + +// Both sides are HMACed to a fixed-size digest under a per-process key, so the +// comparison is constant-time and leaks no length. +const COMPARE_KEY = randomBytes(32); +const secretsEqual = (a: string, b: string): boolean => + timingSafeEqual( + createHmac('sha256', COMPARE_KEY).update(a).digest(), + createHmac('sha256', COMPARE_KEY).update(b).digest(), + ); + +/** + * 403 unless the request carries the shared secret in `x-puter-internal-auth`. + * Read per request; an unset secret refuses everything. + */ +export const internalAuthGate = + (secret: () => string | undefined): RequestHandler => + (req, _res, next) => { + const expected = secret(); + const offered = req.headers[INTERNAL_AUTH_HEADER]; + if ( + !expected || + typeof offered !== 'string' || + !secretsEqual(offered, expected) + ) { + next(new HttpError(403, 'Forbidden', { legacyCode: 'forbidden' })); + return; + } + next(); + }; diff --git a/src/backend/core/http/requestBody.test.ts b/src/backend/core/http/requestBody.test.ts new file mode 100644 index 000000000..6fa5fadd8 --- /dev/null +++ b/src/backend/core/http/requestBody.test.ts @@ -0,0 +1,42 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import type { Request } from 'express'; +import { describe, expect, it } from 'vitest'; +import { bodyRecord } from './requestBody'; + +const req = (body: unknown) => ({ body }) as Request; + +describe('bodyRecord', () => { + it('returns an object body as-is', () => { + const body = { a: 1 }; + expect(bodyRecord(req(body))).toBe(body); + }); + + it('refuses a missing, array or primitive body with 400', () => { + for (const body of [undefined, null, [], 'text', 3]) { + expect(() => bodyRecord(req(body))).toThrow( + expect.objectContaining({ + statusCode: 400, + message: 'body must be an object', + }), + ); + } + }); +}); diff --git a/src/backend/core/http/requestBody.ts b/src/backend/core/http/requestBody.ts new file mode 100644 index 000000000..e8155905a --- /dev/null +++ b/src/backend/core/http/requestBody.ts @@ -0,0 +1,32 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import type { Request } from 'express'; +import { HttpError } from './HttpError'; + +/** The parsed body as a plain object; 400 when it is missing or not one. */ +export const bodyRecord = (req: Request): Record => { + const body = req.body; + if (!body || typeof body !== 'object' || Array.isArray(body)) { + throw new HttpError(400, 'body must be an object', { + legacyCode: 'bad_request', + }); + } + return body as Record; +}; diff --git a/src/backend/core/http/types.ts b/src/backend/core/http/types.ts index 84f04024d..f837609ae 100644 --- a/src/backend/core/http/types.ts +++ b/src/backend/core/http/types.ts @@ -19,6 +19,7 @@ import type { NextFunction, Request, RequestHandler, Response } from 'express'; import type { UserRow } from '../../stores/user/UserStore'; +import type { IConfig } from '../../types'; import type { Actor } from '../actor'; /** @@ -115,7 +116,7 @@ export interface RouteRateLimit { * subdomain → requireAuth → emailConfirmed → requireUserActor → adminOnly → * allowedAppIds → phoneVerified → cardVerified → anyVerified → * requireReputation → requireSubscription → rateLimit → requireCredits → - * concurrent → userProtected → `middleware` → handler + * concurrent → internalAuth → userProtected → `middleware` → handler * * Options that imply `requireAuth` are deduped to a single auth gate. */ @@ -229,6 +230,13 @@ export interface RouteOptions { */ antiCsrf?: boolean; + /** + * Service-to-service routes: 403 unless `x-puter-internal-auth` matches the + * secret this picks from config (read per request; unset refuses). Runs + * after the rate limit, which then only bounds guessing. + */ + internalAuth?: (config: IConfig) => string | undefined; + /** * Security-critical account routes: session cookie only, a fresh * not-suspended user row on `req.userProtected`, and the password (or an diff --git a/src/backend/server.ts b/src/backend/server.ts index 80dcbd336..075d5f085 100644 --- a/src/backend/server.ts +++ b/src/backend/server.ts @@ -63,6 +63,7 @@ import { } from './services/metering/enforcement'; import { createStepUpGate } from './core/http/middleware/stepUpSession'; import { createUserProtectedGate } from './core/http/middleware/userProtected'; +import { internalAuthGate } from './core/http/middleware/internalAuth'; import { createNotFoundHandler } from './core/http/middleware/notFoundHandler'; import { cardFallbackDepsFrom } from './util/cardFallback'; import { installProcessGuards } from './util/processGuards'; @@ -1322,7 +1323,13 @@ export class PuterServer { ); } - // 3b. Password (or OIDC revalidation) re-check on account routes. + // 3b. Shared-secret check on service-to-service routes. + if (opts.internalAuth) { + const { internalAuth } = opts; + mwChain.push(internalAuthGate(() => internalAuth(this.#config))); + } + + // 3c. Password (or OIDC revalidation) re-check on account routes. if (opts.userProtected) { mwChain.push( ...createUserProtectedGate(