From fe0dde38c30730fc23617196a15eee7b0e0c70d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne?= Date: Fri, 23 Jun 2023 19:53:47 +0200 Subject: [PATCH] fixup! BREAKING: activitypub: validate fetch signatures --- .../1687460719276-allow-unsigned-fetches.js | 11 -------- packages/backend/src/config/load.ts | 1 + packages/backend/src/config/types.ts | 2 ++ packages/backend/src/models/entities/meta.ts | 5 ---- packages/backend/src/server/activitypub.ts | 25 +++++++++++-------- .../src/server/activitypub/fetch-signature.ts | 9 +++---- 6 files changed, 21 insertions(+), 32 deletions(-) delete mode 100644 packages/backend/migration/1687460719276-allow-unsigned-fetches.js diff --git a/packages/backend/migration/1687460719276-allow-unsigned-fetches.js b/packages/backend/migration/1687460719276-allow-unsigned-fetches.js deleted file mode 100644 index 6ff0eba68..000000000 --- a/packages/backend/migration/1687460719276-allow-unsigned-fetches.js +++ /dev/null @@ -1,11 +0,0 @@ -export class allowUnsignedFetches1687460719276 { - name = 'allowUnsignedFetches1687460719276' - - async up(queryRunner) { - await queryRunner.query(`ALTER TABLE "meta" ADD "allowUnsignedFetches" bool default false`); - } - - async down(queryRunner) { - await queryRunner.query(`ALTER TABLE "meta" DROP COLUMN "allowUnsignedFetches"`); - } -} diff --git a/packages/backend/src/config/load.ts b/packages/backend/src/config/load.ts index f16fb850b..d1b303f73 100644 --- a/packages/backend/src/config/load.ts +++ b/packages/backend/src/config/load.ts @@ -61,6 +61,7 @@ export function loadConfig(): Config { proxyRemoteFiles: false, maxFileSize: 262144000, // 250 MiB maxNoteTextLength: 3000, + allowUnsignedFetches: false, }, config); mixin.version = meta.version; diff --git a/packages/backend/src/config/types.ts b/packages/backend/src/config/types.ts index 686a8c242..2d163f91b 100644 --- a/packages/backend/src/config/types.ts +++ b/packages/backend/src/config/types.ts @@ -68,6 +68,8 @@ export type Source = { notFound?: string; error?: string; }; + + allowUnsignedFetches?: boolean; }; /** diff --git a/packages/backend/src/models/entities/meta.ts b/packages/backend/src/models/entities/meta.ts index cfaeb32a2..28ba51195 100644 --- a/packages/backend/src/models/entities/meta.ts +++ b/packages/backend/src/models/entities/meta.ts @@ -82,11 +82,6 @@ export class Meta { }) public blockedHosts: string[]; - @Column('boolean', { - default: false - }) - public allowUnsignedFetches: boolean; - @Column('varchar', { length: 512, nullable: true, diff --git a/packages/backend/src/server/activitypub.ts b/packages/backend/src/server/activitypub.ts index d7253c28f..24947d473 100644 --- a/packages/backend/src/server/activitypub.ts +++ b/packages/backend/src/server/activitypub.ts @@ -61,11 +61,14 @@ export function setResponseType(ctx: Router.RouterContext): void { } } -export function handleSignatureResult(ctx: Router.RouterContext, result: SignatureValidationResult): boolean { +async function handleSignature(ctx: Router.RouterContext): Promise { + const result = await validateFetchSignature(ctx.req); switch (result) { + // 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; @@ -92,7 +95,7 @@ router.post('/users/:user/inbox', json(), inbox); // note router.get('/notes/:note', async (ctx, next) => { if (!isActivityPubReq(ctx)) return await next(); - if (!handleSignatureResult(ctx, await validateFetchSignature(ctx.req))) return; + if (!(await handleSignature(ctx))) return; const note = await Notes.findOneBy({ id: ctx.params.note, @@ -129,7 +132,7 @@ router.get('/notes/:note/activity', async ctx => { ctx.redirect(`/notes/${ctx.params.note}`); return; } - if (!handleSignatureResult(ctx, await validateFetchSignature(ctx.req))) return; + if (!(await handleSignature(ctx))) return; const note = await Notes.findOneBy({ id: ctx.params.note, @@ -149,25 +152,25 @@ router.get('/notes/:note/activity', async ctx => { // outbox router.get('/users/:user/outbox', async ctx => { - if (!handleSignatureResult(ctx, await validateFetchSignature(ctx.req))) return; + if (!(await handleSignature(ctx))) return; return await Outbox(ctx); }); // followers router.get('/users/:user/followers', async ctx => { - if (!handleSignatureResult(ctx, await validateFetchSignature(ctx.req))) return; + if (!(await handleSignature(ctx))) return; return await Followers(ctx); }); // following router.get('/users/:user/following', async ctx => { - if (!handleSignatureResult(ctx, await validateFetchSignature(ctx.req))) return; + if (!(await handleSignature(ctx))) return; return await Following(ctx); }); // featured router.get('/users/:user/collections/featured', async ctx => { - if (!handleSignatureResult(ctx, await validateFetchSignature(ctx.req))) return; + if (!(await handleSignature(ctx))) return; return await Featured(ctx); }); @@ -224,7 +227,7 @@ router.get('/users/:user', async (ctx, next) => { // validate and enforce signatures. if (user == null || !isInstanceActor(user)) { - if (!handleSignatureResult(ctx, await validateFetchSignature(ctx.req))) return; + if (!(await handleSignature(ctx))) return; } else if (isInstanceActor(user)) { @@ -237,7 +240,7 @@ router.get('/users/:user', async (ctx, next) => { router.get('/@:user', async (ctx, next) => { if (!isActivityPubReq(ctx)) return await next(); - if (!handleSignatureResult(ctx, await validateFetchSignature(ctx.req))) return; + if (!(await handleSignature(ctx))) return; const user = await Users.findOneBy({ usernameLower: ctx.params.user.toLowerCase(), @@ -270,7 +273,7 @@ router.get('/emojis/:emoji', async ctx => { // like router.get('/likes/:like', async ctx => { - if (!handleSignatureResult(ctx, await validateFetchSignature(ctx.req))) return; + if (!(await handleSignature(ctx))) return; const reaction = await NoteReactions.findOneBy({ id: ctx.params.like }); if (reaction == null) { @@ -294,7 +297,7 @@ router.get('/likes/:like', async ctx => { // follow router.get('/follows/:follower/:followee', async ctx => { - if (!handleSignatureResult(ctx, await validateFetchSignature(ctx.req))) return; + if (!(await handleSignature(ctx))) return; // This may be used before the follow is completed, so we do not // check if the following exists. diff --git a/packages/backend/src/server/activitypub/fetch-signature.ts b/packages/backend/src/server/activitypub/fetch-signature.ts index 3b16a4c62..521b4c106 100644 --- a/packages/backend/src/server/activitypub/fetch-signature.ts +++ b/packages/backend/src/server/activitypub/fetch-signature.ts @@ -1,12 +1,12 @@ import httpSignature from '@peertube/http-signature'; -import { toPuny } from '@/misc/convert-host.js'; -import { fetchMeta } from '@/misc/fetch-meta.js'; +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'; @@ -39,10 +39,9 @@ async function resolveKeyId(keyId: string, resolver: Resolver): Promise { - const meta = await fetchMeta(); let signature; - if (meta.allowUnsignedFetches === true) + if (config.allowUnsignedFetches === true) return 'always'; try { @@ -60,7 +59,7 @@ export async function validateFetchSignature(req: IncomingMessage): Promise