From e5ea882ed74b3cdd0e86f4dda5a8ce724f30d2ef Mon Sep 17 00:00:00 2001 From: dakkar Date: Wed, 20 Dec 2023 12:17:59 +0000 Subject: [PATCH] authorized fetch #217 the implementation is copied from the other places we already check HTTP signatures, and cross-checked with Firefish's implementation --- .config/example.yml | 2 + chart/files/default.yml | 2 + packages/backend/src/config.ts | 3 + .../src/server/ActivityPubServerService.ts | 129 ++++++++++++++++++ 4 files changed, 136 insertions(+) diff --git a/.config/example.yml b/.config/example.yml index 4b0d07ae85..28fe5b359d 100644 --- a/.config/example.yml +++ b/.config/example.yml @@ -212,6 +212,8 @@ proxyRemoteFiles: true # Sign to ActivityPub GET request (default: true) signToActivityPubGet: true +# check that inbound ActivityPub GET requests are signed ("authorized fetch") +checkActivityPubGetSignature: false # For security reasons, uploading attachments from the intranet is prohibited, # but exceptions can be made from the following settings. Default value is "undefined". diff --git a/chart/files/default.yml b/chart/files/default.yml index 4cc291e80a..9c81964736 100644 --- a/chart/files/default.yml +++ b/chart/files/default.yml @@ -194,6 +194,8 @@ id: "aidx" # Sign to ActivityPub GET request (default: true) signToActivityPubGet: true +# check that inbound ActivityPub GET requests are signed ("authorized fetch") +checkActivityPubGetSignature: false #allowedPrivateNetworks: [ # '127.0.0.1/32' diff --git a/packages/backend/src/config.ts b/packages/backend/src/config.ts index dceeac4691..4a4d7e8311 100644 --- a/packages/backend/src/config.ts +++ b/packages/backend/src/config.ts @@ -88,6 +88,7 @@ type Source = { customMOTD?: string[]; signToActivityPubGet?: boolean; + checkActivityPubGetSignature?: boolean; perChannelMaxNoteCacheCount?: number; perUserNotificationsMaxCount?: number; @@ -146,6 +147,7 @@ export type Config = { proxyRemoteFiles: boolean | undefined; customMOTD: string[] | undefined; signToActivityPubGet: boolean | undefined; + checkActivityPubGetSignature: boolean | undefined; version: string; host: string; @@ -253,6 +255,7 @@ export function loadConfig(): Config { proxyRemoteFiles: config.proxyRemoteFiles, customMOTD: config.customMOTD, signToActivityPubGet: config.signToActivityPubGet, + checkActivityPubGetSignature: config.checkActivityPubGetSignature, mediaProxy: externalMediaProxy ?? internalMediaProxy, externalMediaProxyEnabled: externalMediaProxy !== null && externalMediaProxy !== internalMediaProxy, videoThumbnailGenerator: config.videoThumbnailGenerator ? diff --git a/packages/backend/src/server/ActivityPubServerService.ts b/packages/backend/src/server/ActivityPubServerService.ts index 68e426b5bc..a0ad4a134f 100644 --- a/packages/backend/src/server/ActivityPubServerService.ts +++ b/packages/backend/src/server/ActivityPubServerService.ts @@ -5,6 +5,7 @@ import * as crypto from 'node:crypto'; import { IncomingMessage } from 'node:http'; +import { format as formatURL } from 'node:url'; import { Inject, Injectable } from '@nestjs/common'; import fastifyAccepts from '@fastify/accepts'; import httpSignature from '@peertube/http-signature'; @@ -17,9 +18,13 @@ import type { FollowingsRepository, NotesRepository, EmojisRepository, NoteReact import * as url from '@/misc/prelude/url.js'; import type { Config } from '@/config.js'; import { ApRendererService } from '@/core/activitypub/ApRendererService.js'; +import { ApDbResolverService } from '@/core/activitypub/ApDbResolverService.js'; import { QueueService } from '@/core/QueueService.js'; import type { MiLocalUser, MiRemoteUser, MiUser } from '@/models/User.js'; +import { MetaService } from '@/core/MetaService.js'; import { UserKeypairService } from '@/core/UserKeypairService.js'; +import { InstanceActorService } from '@/core/InstanceActorService.js'; +import type { MiUserPublickey } from '@/models/UserPublickey.js'; import type { MiFollowing } from '@/models/Following.js'; import { countIf } from '@/misc/prelude/array.js'; import type { MiNote } from '@/models/Note.js'; @@ -65,9 +70,12 @@ export class ActivityPubServerService { @Inject(DI.followRequestsRepository) private followRequestsRepository: FollowRequestsRepository, + private metaService: MetaService, private utilityService: UtilityService, private userEntityService: UserEntityService, + private instanceActorService: InstanceActorService, private apRendererService: ApRendererService, + private apDbResolverService: ApDbResolverService, private queueService: QueueService, private userKeypairService: UserKeypairService, private queryService: QueryService, @@ -99,6 +107,101 @@ export class ActivityPubServerService { return this.apRendererService.renderCreate(await this.apRendererService.renderNote(note, false), note); } + @bindThis + private async shouldRefuseGetRequest(request: FastifyRequest, reply: FastifyReply, userId: string | undefined = undefined): Promise { + if (!this.config.checkActivityPubGetSignature) return false; + + /* this code is inspired from the `inbox` function below, and + `queue/processors/InboxProcessorService` + + those pieces of code also check `digest`, and various bits from the + request body, but that only makes sense for requests with a body: + here we're validating GET requests + + this is also inspired by FireFish's `checkFetch` + */ + + /* we always allow requests about our instance actor, because when + a remote instance needs to check our signature on a request we + sent, it will need to fetch information about the user that + signed it (which is our instance actor), and if we try to check + their signature on *that* request, we'll fetch *their* instance + actor... leading to an infinite recursion */ + if (userId) { + const instanceActor = await this.instanceActorService.getInstanceActor(); + + if (userId === instanceActor.id) return false; + } + + let signature; + + try { + signature = httpSignature.parseRequest(request.raw, { 'headers': [] }); + } catch (e) { + // not signed, or malformed signature: refuse + reply.code(401); + return true; + } + + if (signature.params.headers.indexOf('host') === -1 + || request.headers.host !== this.config.host) { + // no destination host, or not us: refuse + reply.code(401); + return true; + } + + const keyId = new URL(signature.keyId); + const keyHost = this.utilityService.toPuny(keyId.hostname); + + const meta = await this.metaService.fetch(); + if (this.utilityService.isBlockedHost(meta.blockedHosts, keyHost)) { + /* blocked instance: refuse (we don't care if the signature is + good, if they even pretend to be from a blocked instance, + they're out) */ + reply.code(401); + return true; + } + + // do we know the signer already? + let authUser: { + user: MiRemoteUser; + key: MiUserPublickey | null; + } | null = await this.apDbResolverService.getAuthUserFromKeyId(signature.keyId); + + if (authUser == null) { + /* keyId is often in the shape `${user.uri}#${keyname}`, try + fetching information about the remote user */ + const candidate = formatURL(keyId, { fragment: false }); + authUser = await this.apDbResolverService.getAuthUserFromApId(candidate); + } + + if (authUser?.key == null) { + // we can't figure out who the signer is, or we can't get their key: refuse + reply.code(401); + return true; + } + + let httpSignatureValidated = httpSignature.verifySignature(signature, authUser.key.keyPem); + + if (!httpSignatureValidated) { + // maybe they changed their key? refetch it + authUser.key = await this.apDbResolverService.refetchPublicKeyForApId(authUser.user); + + if (authUser.key != null) { + httpSignatureValidated = httpSignature.verifySignature(signature, authUser.key.keyPem); + } + } + + if (!httpSignatureValidated) { + // bad signature: refuse + reply.code(401); + return true; + } + + // all good, don't refuse + return false; + } + @bindThis private inbox(request: FastifyRequest, reply: FastifyReply) { let signature; @@ -172,6 +275,8 @@ export class ActivityPubServerService { request: FastifyRequest<{ Params: { user: string; }; Querystring: { cursor?: string; page?: string; }; }>, reply: FastifyReply, ) { + if (await this.shouldRefuseGetRequest(request, reply, request.params.user)) return; + const userId = request.params.user; const cursor = request.query.cursor; @@ -264,6 +369,8 @@ export class ActivityPubServerService { request: FastifyRequest<{ Params: { user: string; }; Querystring: { cursor?: string; page?: string; }; }>, reply: FastifyReply, ) { + if (await this.shouldRefuseGetRequest(request, reply, request.params.user)) return; + const userId = request.params.user; const cursor = request.query.cursor; @@ -353,6 +460,8 @@ export class ActivityPubServerService { @bindThis private async featured(request: FastifyRequest<{ Params: { user: string; }; }>, reply: FastifyReply) { + if (await this.shouldRefuseGetRequest(request, reply, request.params.user)) return; + const userId = request.params.user; const user = await this.usersRepository.findOneBy({ @@ -397,6 +506,8 @@ export class ActivityPubServerService { }>, reply: FastifyReply, ) { + if (await this.shouldRefuseGetRequest(request, reply, request.params.user)) return; + const userId = request.params.user; const sinceId = request.query.since_id; @@ -551,6 +662,8 @@ export class ActivityPubServerService { // note fastify.get<{ Params: { note: string; } }>('/notes/:note', { constraints: { apOrHtml: 'ap' } }, async (request, reply) => { + if (await this.shouldRefuseGetRequest(request, reply)) return; + vary(reply.raw, 'Accept'); const note = await this.notesRepository.findOneBy({ @@ -581,6 +694,8 @@ export class ActivityPubServerService { // note activity fastify.get<{ Params: { note: string; } }>('/notes/:note/activity', async (request, reply) => { + if (await this.shouldRefuseGetRequest(request, reply)) return; + vary(reply.raw, 'Accept'); const note = await this.notesRepository.findOneBy({ @@ -623,6 +738,8 @@ export class ActivityPubServerService { // publickey fastify.get<{ Params: { user: string; } }>('/users/:user/publickey', async (request, reply) => { + if (await this.shouldRefuseGetRequest(request, reply, request.params.user)) return; + const userId = request.params.user; const user = await this.usersRepository.findOneBy({ @@ -648,6 +765,8 @@ export class ActivityPubServerService { }); fastify.get<{ Params: { user: string; } }>('/users/:user', { constraints: { apOrHtml: 'ap' } }, async (request, reply) => { + if (await this.shouldRefuseGetRequest(request, reply, request.params.user)) return; + const userId = request.params.user; const user = await this.usersRepository.findOneBy({ @@ -660,6 +779,8 @@ export class ActivityPubServerService { }); fastify.get<{ Params: { user: string; } }>('/@:user', { constraints: { apOrHtml: 'ap' } }, async (request, reply) => { + if (await this.shouldRefuseGetRequest(request, reply, request.params.user)) return; + const user = await this.usersRepository.findOneBy({ usernameLower: request.params.user.toLowerCase(), host: IsNull(), @@ -672,6 +793,8 @@ export class ActivityPubServerService { // emoji fastify.get<{ Params: { emoji: string; } }>('/emojis/:emoji', async (request, reply) => { + if (await this.shouldRefuseGetRequest(request, reply)) return; + const emoji = await this.emojisRepository.findOneBy({ host: IsNull(), name: request.params.emoji, @@ -689,6 +812,8 @@ export class ActivityPubServerService { // like fastify.get<{ Params: { like: string; } }>('/likes/:like', async (request, reply) => { + if (await this.shouldRefuseGetRequest(request, reply)) return; + const reaction = await this.noteReactionsRepository.findOneBy({ id: request.params.like }); if (reaction == null) { @@ -710,6 +835,8 @@ export class ActivityPubServerService { // follow fastify.get<{ Params: { follower: string; followee: string; } }>('/follows/:follower/:followee', async (request, reply) => { + if (await this.shouldRefuseGetRequest(request, reply)) return; + // This may be used before the follow is completed, so we do not // check if the following exists. @@ -736,6 +863,8 @@ export class ActivityPubServerService { // follow fastify.get<{ Params: { followRequestId: string ; } }>('/follows/:followRequestId', async (request, reply) => { + if (await this.shouldRefuseGetRequest(request, reply)) return; + // This may be used before the follow is completed, so we do not // check if the following exists and only check if the follow request exists.