From 597de07465f4da2b01398c19f9d57d635500653e Mon Sep 17 00:00:00 2001 From: Johann150 Date: Mon, 26 Jun 2023 22:02:27 +0200 Subject: [PATCH] server: refactor HTTP signature validation --- .../backend/src/queue/processors/inbox.ts | 53 +++--------- .../http-signature.ts} | 74 ++++++++--------- packages/backend/src/server/activitypub.ts | 82 ++++++++++--------- 3 files changed, 89 insertions(+), 120 deletions(-) rename packages/backend/src/{server/activitypub/fetch-signature.ts => remote/http-signature.ts} (51%) diff --git a/packages/backend/src/queue/processors/inbox.ts b/packages/backend/src/queue/processors/inbox.ts index 87b1a421e..be239daa9 100644 --- a/packages/backend/src/queue/processors/inbox.ts +++ b/packages/backend/src/queue/processors/inbox.ts @@ -1,6 +1,5 @@ import { URL } from 'node:url'; import Bull from 'bull'; -import httpSignature from '@peertube/http-signature'; import { perform } from '@/remote/activitypub/perform.js'; import Logger from '@/services/logger.js'; import { registerOrFetchInstanceDoc } from '@/services/register-or-fetch-instance-doc.js'; @@ -11,17 +10,18 @@ import { getApId } from '@/remote/activitypub/type.js'; import { fetchInstanceMetadata } from '@/services/fetch-instance-metadata.js'; import { Resolver } from '@/remote/activitypub/resolver.js'; import { LdSignature } from '@/remote/activitypub/misc/ld-signature.js'; -import { getAuthUser } from '@/remote/activitypub/misc/auth-user.js'; -import { StatusError } from '@/misc/fetch.js'; +import { AuthUser, getAuthUser } from '@/remote/activitypub/misc/auth-user.js'; import { InboxJobData } from '@/queue/types.js'; import { shouldBlockInstance } from '@/misc/should-block-instance.js'; +import { verifyHttpSignature } from '@/remote/http-signature.js'; const logger = new Logger('inbox'); -// ユーザーのinboxにアクティビティが届いた時の処理 +// Processing when an activity arrives in the user's inbox export default async (job: Bull.Job): Promise => { const signature = job.data.signature; // HTTP-signature const activity = job.data.activity; + const resolver = new Resolver(); //#region Log const info = Object.assign({}, activity) as any; @@ -29,46 +29,12 @@ export default async (job: Bull.Job): Promise => { logger.debug(JSON.stringify(info, null, 2)); //#endregion - const keyIdLower = signature.keyId.toLowerCase(); - if (keyIdLower.startsWith('acct:')) { - return `Old keyId is no longer supported. ${keyIdLower}`; - } - - const host = extractDbHost(keyIdLower) - - // Stop if the host is blocked. - if (await shouldBlockInstance(host)) { - return `Blocked request: ${host}`; - } - - const resolver = new Resolver(); - - let authUser; - try { - authUser = await getAuthUser(signature.keyId, getApId(activity.actor), resolver); - } catch (e) { - if (e instanceof StatusError) { - if (e.isClientError) { - return `skip: Ignored deleted actors on both ends ${activity.actor} - ${e.statusCode}`; - } else { - throw new Error(`Error in actor ${activity.actor} - ${e.statusCode || e}`); - } - } - } - - if (authUser == null) { - // Key not found? Unacceptable! - return 'skip: failed to resolve user'; - } else { - // Found key! - } - - // verify the HTTP Signature - const httpSignatureValidated = httpSignature.verifySignature(signature, authUser.key.keyPem); + const validated = await verifyHttpSignature(signature, resolver, getApId(activity.actor)); + let authUser = validated.authUser; // The signature must be valid. // The signature must also match the actor otherwise anyone could sign any activity. - if (!httpSignatureValidated || authUser.user.uri !== activity.actor) { + if (validated.status !== 'valid' || validated.authUser.user.uri !== activity.actor) { // Last resort: LD-Signature if (activity.signature) { if (activity.signature.type !== 'RsaSignature2017') { @@ -107,6 +73,11 @@ export default async (job: Bull.Job): Promise => { } } + // authUser cannot be null at this point: + // either it was already not null because the HTTP signature was valid + // or, if the LD signature was not verified, this function will already have returned. + authUser = authUser as AuthUser; + // Verify that the actor's host is not blocked const signerHost = extractDbHost(authUser.user.uri!); if (await shouldBlockInstance(signerHost)) { diff --git a/packages/backend/src/server/activitypub/fetch-signature.ts b/packages/backend/src/remote/http-signature.ts similarity index 51% rename from packages/backend/src/server/activitypub/fetch-signature.ts rename to packages/backend/src/remote/http-signature.ts index 521b4c106..69e3cc798 100644 --- a/packages/backend/src/server/activitypub/fetch-signature.ts +++ b/packages/backend/src/remote/http-signature.ts @@ -1,14 +1,12 @@ -import httpSignature from '@peertube/http-signature'; -import { extractDbHost } from '@/misc/convert-host.js'; -import { shouldBlockInstance } from '@/misc/should-block-instance.js'; -import { authUserFromKeyId, getAuthUser } from '@/remote/activitypub/misc/auth-user.js'; -import { getApId, isActor } from '@/remote/activitypub/type.js'; -import { StatusError } from '@/misc/fetch.js'; -import { Resolver } from '@/remote/activitypub/resolver.js'; -import { createPerson } from '@/remote/activitypub/models/person.js'; -import config from '@/config/index.js'; - -export type SignatureValidationResult = 'missing' | 'invalid' | 'rejected' | 'valid' | 'always'; +import { URL } from 'node:url'; +import { extractDbHost } from "@/misc/convert-host.js"; +import { shouldBlockInstance } from "@/misc/should-block-instance.js"; +import httpSignature from "@peertube/http-signature"; +import { Resolver } from "./activitypub/resolver.js"; +import { StatusError } from "@/misc/fetch.js"; +import { AuthUser, authUserFromKeyId, getAuthUser } from "./activitypub/misc/auth-user.js"; +import { ApObject, getApId, isActor } from "./activitypub/type.js"; +import { createPerson } from "./activitypub/models/person.js"; async function resolveKeyId(keyId: string, resolver: Resolver): Promise { // Do we already know that keyId? @@ -18,7 +16,7 @@ async function resolveKeyId(keyId: string, resolver: Resolver): Promise { - let signature; - - if (config.allowUnsignedFetches === true) - return 'always'; - - try { - signature = httpSignature.parseRequest(req); - } catch (e) { - // TypeScript has wrong typings for Error, meaning I can't extract `name`. - // No typings for @peertube/http-signature's Errors either. - // This means we have to report it as missing instead of invalid in cases - // where the structure is incorrect. - return 'missing'; - } +export type SignatureValidationResult = { + status: 'missing' | 'invalid' | 'rejected'; + authUser: AuthUser | null; +} | { + status: 'valid'; + authUser: AuthUser; +}; +export async function verifyHttpSignature(signature: httpSignature.IParsedSignature, resolver: Resolver, actor?: ApObject): Promise { // This old `keyId` format is no longer supported. const keyIdLower = signature.keyId.toLowerCase(); - if (keyIdLower.startsWith('acct:')) - return 'invalid'; + if (keyIdLower.startsWith('acct:')) return { status: 'invalid', authUser: null }; const host = extractDbHost(keyIdLower); // Reject if the host is blocked. - if (await shouldBlockInstance(host)) - return 'rejected'; + if (await shouldBlockInstance(host)) return { status: 'rejected', authUser: null }; - const resolver = new Resolver(); - let authUser; + let authUser = null; try { - authUser = await resolveKeyId(signature.keyId, resolver); + if (actor != null) { + authUser = await getAuthUser(signature.keyId, getApId(actor), resolver); + } else { + authUser = await resolveKeyId(signature.keyId, resolver); + } } catch (e) { if (e instanceof StatusError) { if (e.isClientError) { // Actor is deleted. - return 'rejected'; + return { status: 'rejected', authUser }; } else { throw new Error(`Error in signature ${signature} - ${e.statusCode || e}`); } @@ -82,20 +74,22 @@ export async function validateFetchSignature(req: IncomingMessage): Promise { - const result = await validateFetchSignature(ctx.req); - switch (result) { + if (config.allowUnsignedFetches) { // Fetch signature verification is disabled. - case 'always': - ctx.set('Cache-Control', 'public, max-age=180'); - return true; - // Fetch signature verification succeeded. - case 'valid': - ctx.set('Cache-Control', 'no-store'); - return true; - case 'missing': - case 'invalid': - // This would leak information on blocks. Only use for debugging. - // ctx.status = 400; - // break; - // eslint-disable-next-line no-fallthrough - case 'rejected': - default: - ctx.status = 403; - break; - } + ctx.set('Cache-Control', 'public, max-age=180'); + return true; + } else { + let verified; + try { + let signature = httpSignature.parseRequest(ctx.req); + verified = await verifyHttpSignature(signature, new Resolver()); + } catch (e) { + verified = { status: 'missing' }; + } - ctx.set('Cache-Control', 'no-store'); - return false; + switch (verified.status) { + // Fetch signature verification succeeded. + case 'valid': + ctx.set('Cache-Control', 'no-store'); + return true; + case 'missing': + case 'invalid': + case 'rejected': + default: + ctx.status = 403; + ctx.set('Cache-Control', 'no-store'); + return false; + } + } } // inbox @@ -150,31 +156,29 @@ router.get('/notes/:note/activity', async ctx => { setResponseType(ctx); }); +async function requireHttpSignature(ctx: Router.Context, next: () => Promise) { + if (!(await handleSignature(ctx))) { + return; + } else { + await next(); + } +} + // outbox -router.get('/users/:user/outbox', async ctx => { - if (!(await handleSignature(ctx))) return; - return await Outbox(ctx); -}); +router.get('/users/:user/outbox', requireHttpSignature, Outbox); // followers -router.get('/users/:user/followers', async ctx => { - if (!(await handleSignature(ctx))) return; - return await Followers(ctx); -}); +router.get('/users/:user/followers', requireHttpSignature, Followers); // following -router.get('/users/:user/following', async ctx => { - if (!(await handleSignature(ctx))) return; - return await Following(ctx); -}); +router.get('/users/:user/following', requireHttpSignature, Following); // featured -router.get('/users/:user/collections/featured', async ctx => { - if (!(await handleSignature(ctx))) return; - return await Featured(ctx); -}); +router.get('/users/:user/collections/featured', requireHttpSignature, Featured); // publickey +// This does not require HTTP signatures in order for other instances +// to be able to verify our own signatures. router.get('/users/:user/publickey', async ctx => { const userId = ctx.params.user;