From 1b71d37438cce6a0b1148cf1c4e3790fcd914cc4 Mon Sep 17 00:00:00 2001 From: syuilo Date: Tue, 14 Nov 2023 17:09:45 +0900 Subject: [PATCH] Merge pull request from GHSA-3f39-6537-3cgc This commit implements HTTP header and body validation to fix [SIF-2023-002](https://advisory.silicon.moe/advisory/sif-2023-002/) Signed-off-by: perillamint Co-authored-by: perillamint Co-authored-by: yunochi --- packages/backend/package.json | 3 +- .../src/server/ActivityPubServerService.ts | 58 ++++++++++++++++++- packages/backend/src/server/ServerService.ts | 8 +++ pnpm-lock.yaml | 16 ++++- 4 files changed, 79 insertions(+), 6 deletions(-) diff --git a/packages/backend/package.json b/packages/backend/package.json index ca9a5f0f90..388a3d3c7a 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -60,7 +60,6 @@ "dependencies": { "@aws-sdk/client-s3": "3.412.0", "@aws-sdk/lib-storage": "3.412.0", - "@smithy/node-http-handler": "2.1.5", "@bull-board/api": "5.9.1", "@bull-board/fastify": "5.9.1", "@bull-board/ui": "5.9.1", @@ -81,6 +80,7 @@ "@peertube/http-signature": "1.7.0", "@simplewebauthn/server": "8.3.5", "@sinonjs/fake-timers": "11.2.2", + "@smithy/node-http-handler": "2.1.5", "@swc/cli": "0.1.62", "@swc/core": "1.3.95", "@vitalets/google-translate-api": "9.2.0", @@ -105,6 +105,7 @@ "date-fns": "2.30.0", "deep-email-validator": "0.1.21", "fastify": "4.24.3", + "fastify-raw-body": "^4.2.2", "feed": "4.2.2", "file-type": "18.6.0", "fluent-ffmpeg": "2.1.2", diff --git a/packages/backend/src/server/ActivityPubServerService.ts b/packages/backend/src/server/ActivityPubServerService.ts index cfc8576435..fa44ce0170 100644 --- a/packages/backend/src/server/ActivityPubServerService.ts +++ b/packages/backend/src/server/ActivityPubServerService.ts @@ -3,6 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-only */ +import * as crypto from 'node:crypto'; import { IncomingMessage } from 'node:http'; import { Inject, Injectable } from '@nestjs/common'; import fastifyAccepts from '@fastify/accepts'; @@ -108,7 +109,58 @@ export class ActivityPubServerService { return; } - // TODO: request.bodyのバリデーション？ + if (signature.params.headers.indexOf('host') === -1 + || request.headers.host !== this.config.host) { + // Host not specified or not match. + reply.code(401); + return; + } + + if (signature.params.headers.indexOf('digest') === -1) { + // Digest not found. + reply.code(401); + } else { + const digest = request.headers.digest; + + if (typeof digest !== 'string') { + // Huh? + reply.code(401); + return; + } + + const re = /^([a-zA-Z0-9\-]+)=(.+)$/; + const match = digest.match(re); + + if (match == null) { + // Invalid digest + reply.code(401); + return; + } + + const algo = match[1]; + const digestValue = match[2]; + + if (algo !== 'SHA-256') { + // Unsupported digest algorithm + reply.code(401); + return; + } + + if (request.rawBody == null) { + // Bad request + reply.code(400); + return; + } + + const hash = crypto.createHash('sha256').update(request.rawBody).digest('base64'); + + if (hash !== digestValue) { + // Invalid digest + reply.code(401); + return; + } + } + this.queueService.inbox(request.body as IActivity, signature); reply.code(202); @@ -474,8 +526,8 @@ export class ActivityPubServerService { //#region Routing // inbox (limit: 64kb) - fastify.post('/inbox', { bodyLimit: 1024 * 64 }, async (request, reply) => await this.inbox(request, reply)); - fastify.post('/users/:user/inbox', { bodyLimit: 1024 * 64 }, async (request, reply) => await this.inbox(request, reply)); + fastify.post('/inbox', { config: { rawBody: true }, bodyLimit: 1024 * 64 }, async (request, reply) => await this.inbox(request, reply)); + fastify.post('/users/:user/inbox', { config: { rawBody: true }, bodyLimit: 1024 * 64 }, async (request, reply) => await this.inbox(request, reply)); // note fastify.get<{ Params: { note: string; } }>('/notes/:note', { constraints: { apOrHtml: 'ap' } }, async (request, reply) => { diff --git a/packages/backend/src/server/ServerService.ts b/packages/backend/src/server/ServerService.ts index 4b4b91b402..e411184bc1 100644 --- a/packages/backend/src/server/ServerService.ts +++ b/packages/backend/src/server/ServerService.ts @@ -9,6 +9,7 @@ import { fileURLToPath } from 'node:url'; import { Inject, Injectable, OnApplicationShutdown } from '@nestjs/common'; import Fastify, { FastifyInstance } from 'fastify'; import fastifyStatic from '@fastify/static'; +import fastifyRawBody from 'fastify-raw-body'; import { IsNull } from 'typeorm'; import { GlobalEventService } from '@/core/GlobalEventService.js'; import type { Config } from '@/config.js'; @@ -86,6 +87,13 @@ export class ServerService implements OnApplicationShutdown { }); } + // Register raw-body parser for ActivityPub HTTP signature validation. + fastify.register(fastifyRawBody, { + global: false, + encoding: 'utf-8', + runFirst: true, + }); + // Register non-serving static server so that the child services can use reply.sendFile. // `root` here is just a placeholder and each call must use its own `rootPath`. fastify.register(fastifyStatic, { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b11dd62d37..34a8e94ea9 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1,4 +1,4 @@ -lockfileVersion: '6.0' +lockfileVersion: '6.1' settings: autoInstallPeers: true @@ -197,6 +197,9 @@ importers: fastify: specifier: 4.24.3 version: 4.24.3 + fastify-raw-body: + specifier: ^4.2.2 + version: 4.2.2 feed: specifier: 4.2.2 version: 4.2.2 @@ -7169,7 +7172,7 @@ packages: hasBin: true peerDependencies: '@swc/core': ^1.2.66 - chokidar: 3.5.3 + chokidar: ^3.5.1 peerDependenciesMeta: chokidar: optional: true @@ -12058,6 +12061,15 @@ packages: resolution: {integrity: sha512-79ak0JxddO0utAXAQ5ccKhvs6vX2MGyHHMMsmZkBANrq3hXc1CHzvNPHOcvTsVMEPl5I+NT+RO4YKMGehOfSIg==} dev: false + /fastify-raw-body@4.2.2: + resolution: {integrity: sha512-6l4fXtxNn7WOQiylu5fv9/JfUTvWCg1ED4gF44hqnVesgttOXEUMnNkdV8ZxwufCstRyUYaYSBIN4VuRHDbJkw==} + engines: {node: '>= 10'} + dependencies: + fastify-plugin: 4.5.0 + raw-body: 2.5.2 + secure-json-parse: 2.7.0 + dev: false + /fastify@4.24.3: resolution: {integrity: sha512-6HHJ+R2x2LS3y1PqxnwEIjOTZxFl+8h4kSC/TuDPXtA+v2JnV9yEtOsNSKK1RMD7sIR2y1ZsA4BEFaid/cK5pg==} dependencies: