From f89a374e5f33ead62106e7ef18f24c189a729f6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne?= Date: Fri, 23 Jun 2023 16:16:43 +0200 Subject: [PATCH] BREAKING: activitypub: validate fetch signatures Enforces HTTP signatures on object fetches, and rejects fetches from blocked instances. This should mean proper and full blocking of remote instances. This is now default behavior, which makes it a breaking change. To disable it (mostly for development purposes), "meta"."allowUnsignedFetches" can be set to true. It is not the default for development environments as it is important to have as close as possible behavior to real environments for ActivityPub development. Co-authored-by: nullobsi Co-authored-by: Norm Changelog: Added --- .../1687460719276-allow-unsigned-fetches.js | 11 ++ packages/backend/src/models/entities/meta.ts | 5 + .../src/remote/activitypub/misc/auth-user.ts | 9 +- packages/backend/src/server/activitypub.ts | 73 +++++++++++-- .../src/server/activitypub/featured.ts | 1 - .../src/server/activitypub/fetch-signature.ts | 102 ++++++++++++++++++ .../src/server/activitypub/followers.ts | 1 - .../src/server/activitypub/following.ts | 1 - .../backend/src/server/activitypub/outbox.ts | 1 - .../backend/src/services/instance-actor.ts | 6 +- 10 files changed, 194 insertions(+), 16 deletions(-) create mode 100644 packages/backend/migration/1687460719276-allow-unsigned-fetches.js create mode 100644 packages/backend/src/server/activitypub/fetch-signature.ts diff --git a/packages/backend/migration/1687460719276-allow-unsigned-fetches.js b/packages/backend/migration/1687460719276-allow-unsigned-fetches.js new file mode 100644 index 000000000..6ff0eba68 --- /dev/null +++ b/packages/backend/migration/1687460719276-allow-unsigned-fetches.js @@ -0,0 +1,11 @@ +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/models/entities/meta.ts b/packages/backend/src/models/entities/meta.ts index 28ba51195..cfaeb32a2 100644 --- a/packages/backend/src/models/entities/meta.ts +++ b/packages/backend/src/models/entities/meta.ts @@ -82,6 +82,11 @@ 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/remote/activitypub/misc/auth-user.ts b/packages/backend/src/remote/activitypub/misc/auth-user.ts index e140d2fa1..94c60ef00 100644 --- a/packages/backend/src/remote/activitypub/misc/auth-user.ts +++ b/packages/backend/src/remote/activitypub/misc/auth-user.ts @@ -4,6 +4,7 @@ import { IRemoteUser } from '@/models/entities/user.js'; import { UserPublickey } from '@/models/entities/user-publickey.js'; import { uriPersonCache, userByIdCache } from '@/services/user-cache.js'; import { createPerson } from '@/remote/activitypub/models/person.js'; +import { Resolver } from '@/remote/activitypub/resolver.js'; export type AuthUser = { user: IRemoteUser; @@ -29,8 +30,8 @@ function authUserFromApId(uri: string): Promise { }); } -export async function getAuthUser(keyId: string, actorUri: string, resolver: Resolver): Promise { - let authUser = await publicKeyCache.fetch(keyId) +export async function authUserFromKeyId(keyId: string): Promise { + return await publicKeyCache.fetch(keyId) .then(async key => { if (!key) return null; else return { @@ -38,6 +39,10 @@ export async function getAuthUser(keyId: string, actorUri: string, resolver: Res key, }; }); +} + +export async function getAuthUser(keyId: string, actorUri: string, resolver: Resolver): Promise { + let authUser = await authUserFromKeyId(keyId); if (authUser != null) return authUser; authUser = await authUserFromApId(actorUri); diff --git a/packages/backend/src/server/activitypub.ts b/packages/backend/src/server/activitypub.ts index 9ddda96d9..d7253c28f 100644 --- a/packages/backend/src/server/activitypub.ts +++ b/packages/backend/src/server/activitypub.ts @@ -20,6 +20,8 @@ import Outbox from './activitypub/outbox.js'; import Followers from './activitypub/followers.js'; import Following from './activitypub/following.js'; import Featured from './activitypub/featured.js'; +import { SignatureValidationResult, validateFetchSignature } from './activitypub/fetch-signature.js'; +import { isInstanceActor } from '@/services/instance-actor.js'; // Init router const router = new Router(); @@ -59,6 +61,30 @@ export function setResponseType(ctx: Router.RouterContext): void { } } +export function handleSignatureResult(ctx: Router.RouterContext, result: SignatureValidationResult): boolean { + switch (result) { + case 'always': + ctx.set('Cache-Control', 'public, max-age=180'); + return true; + 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', 'no-store'); + return false; +} + // inbox router.post('/inbox', json(), inbox); router.post('/users/:user/inbox', json(), inbox); @@ -66,6 +92,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; const note = await Notes.findOneBy({ id: ctx.params.note, @@ -89,7 +116,6 @@ router.get('/notes/:note', async (ctx, next) => { } ctx.body = renderActivity(await renderNote(note, false)); - ctx.set('Cache-Control', 'public, max-age=180'); setResponseType(ctx); }); @@ -103,6 +129,7 @@ router.get('/notes/:note/activity', async ctx => { ctx.redirect(`/notes/${ctx.params.note}`); return; } + if (!handleSignatureResult(ctx, await validateFetchSignature(ctx.req))) return; const note = await Notes.findOneBy({ id: ctx.params.note, @@ -117,21 +144,32 @@ router.get('/notes/:note/activity', async ctx => { } ctx.body = renderActivity(await renderNoteOrRenoteActivity(note)); - ctx.set('Cache-Control', 'public, max-age=180'); setResponseType(ctx); }); // outbox -router.get('/users/:user/outbox', Outbox); +router.get('/users/:user/outbox', async ctx => { + if (!handleSignatureResult(ctx, await validateFetchSignature(ctx.req))) return; + return await Outbox(ctx); +}); // followers -router.get('/users/:user/followers', Followers); +router.get('/users/:user/followers', async ctx => { + if (!handleSignatureResult(ctx, await validateFetchSignature(ctx.req))) return; + return await Followers(ctx); +}); // following -router.get('/users/:user/following', Following); +router.get('/users/:user/following', async ctx => { + if (!handleSignatureResult(ctx, await validateFetchSignature(ctx.req))) return; + return await Following(ctx); +}); // featured -router.get('/users/:user/collections/featured', Featured); +router.get('/users/:user/collections/featured', async ctx => { + if (!handleSignatureResult(ctx, await validateFetchSignature(ctx.req))) return; + return await Featured(ctx); +}); // publickey router.get('/users/:user/publickey', async ctx => { @@ -166,7 +204,6 @@ async function userInfo(ctx: Router.RouterContext, user: User | null): Promise { isSuspended: false, }); + // Allow fetching the instance actor without any HTTP signature. + // Only on this route, as it is the canonical route. + // If the user could not be resolved, or is not the instance actor, + // validate and enforce signatures. + if (user == null || !isInstanceActor(user)) + { + if (!handleSignatureResult(ctx, await validateFetchSignature(ctx.req))) return; + } + else if (isInstanceActor(user)) + { + // Set cache at all times for instance actors. + ctx.set('Cache-Control', 'public, max-age=180'); + } + await userInfo(ctx, user); }); router.get('/@:user', async (ctx, next) => { if (!isActivityPubReq(ctx)) return await next(); + if (!handleSignatureResult(ctx, await validateFetchSignature(ctx.req))) return; const user = await Users.findOneBy({ usernameLower: ctx.params.user.toLowerCase(), @@ -198,6 +250,9 @@ router.get('/@:user', async (ctx, next) => { // emoji router.get('/emojis/:emoji', async ctx => { + // Enforcing HTTP signatures on Emoji objects could cause problems for + // other software that might use those objects for copying custom emoji. + const emoji = await Emojis.findOneBy({ host: IsNull(), name: ctx.params.emoji, @@ -215,6 +270,7 @@ router.get('/emojis/:emoji', async ctx => { // like router.get('/likes/:like', async ctx => { + if (!handleSignatureResult(ctx, await validateFetchSignature(ctx.req))) return; const reaction = await NoteReactions.findOneBy({ id: ctx.params.like }); if (reaction == null) { @@ -233,12 +289,12 @@ router.get('/likes/:like', async ctx => { } ctx.body = renderActivity(await renderLike(reaction, note)); - ctx.set('Cache-Control', 'public, max-age=180'); setResponseType(ctx); }); // follow router.get('/follows/:follower/:followee', async ctx => { + if (!handleSignatureResult(ctx, await validateFetchSignature(ctx.req))) return; // This may be used before the follow is completed, so we do not // check if the following exists. @@ -259,7 +315,6 @@ router.get('/follows/:follower/:followee', async ctx => { } ctx.body = renderActivity(renderFollow(follower, followee)); - ctx.set('Cache-Control', 'public, max-age=180'); setResponseType(ctx); }); diff --git a/packages/backend/src/server/activitypub/featured.ts b/packages/backend/src/server/activitypub/featured.ts index 09906250f..e33d89cbf 100644 --- a/packages/backend/src/server/activitypub/featured.ts +++ b/packages/backend/src/server/activitypub/featured.ts @@ -36,6 +36,5 @@ export default async (ctx: Router.RouterContext) => { ); ctx.body = renderActivity(rendered); - ctx.set('Cache-Control', 'public, max-age=180'); setResponseType(ctx); }; diff --git a/packages/backend/src/server/activitypub/fetch-signature.ts b/packages/backend/src/server/activitypub/fetch-signature.ts new file mode 100644 index 000000000..3b16a4c62 --- /dev/null +++ b/packages/backend/src/server/activitypub/fetch-signature.ts @@ -0,0 +1,102 @@ +import httpSignature from '@peertube/http-signature'; +import { toPuny } from '@/misc/convert-host.js'; +import { fetchMeta } from '@/misc/fetch-meta.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'; + +export type SignatureValidationResult = 'missing' | 'invalid' | 'rejected' | 'valid' | 'always'; + +async function resolveKeyId(keyId: string, resolver: Resolver): Promise { + // Do we already know that keyId? + const authUser = await authUserFromKeyId(keyId); + if (authUser != null) return authUser; + + // If not, discover it. + const keyUrl = new URL(keyId); + keyUrl.hash = ''; // Fragment should not be part of the request. + + const keyObject = await resolver.resolve(keyUrl.toString()); + + // Does the keyId end up resolving to an Actor? + if (isActor(keyObject)) { + await createPerson(keyObject, resolver); + return await getAuthUser(keyId, getApId(keyObject), resolver); + } + + // Does the keyId end up resolving to a Key-like? + const keyData = keyObject as any; + if (keyData.owner != null && keyData.publicKeyPem != null) { + await createPerson(keyData.owner, resolver); + return await getAuthUser(keyId, getApId(keyData.owner), resolver); + } + + // Cannot be resolved. + return null; +} + +export async function validateFetchSignature(req: IncomingMessage): Promise { + const meta = await fetchMeta(); + let signature; + + if (meta.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'; + } + + // This old `keyId` format is no longer supported. + const keyIdLower = signature.keyId.toLowerCase(); + if (keyIdLower.startsWith('acct:')) + return 'invalid'; + + const host = toPuny(new URL(keyIdLower).hostname); + + // Reject if the host is blocked. + if (await shouldBlockInstance(host)) + return 'rejected'; + + const resolver = new Resolver(); + let authUser; + try { + authUser = await resolveKeyId(signature.keyId, resolver); + } catch (e) { + if (e instanceof StatusError) { + if (e.isClientError) { + // Actor is deleted. + return 'rejected'; + } else { + throw new Error(`Error in signature ${signature} - ${e.statusCode || e}`); + } + } + } + + if (authUser == null) { + // Key not found? Unacceptable! + return 'invalid'; + } else { + // Found key! + } + + // Make sure the resolved user matches the keyId host. + if (authUser.user.host !== host) + return 'rejected'; + + // Verify the HTTP Signature + const httpSignatureValidated = httpSignature.verifySignature(signature, authUser.key.keyPem); + if (httpSignatureValidated === true) + return 'valid'; + + // Otherwise, fail. + return 'invalid'; +} diff --git a/packages/backend/src/server/activitypub/followers.ts b/packages/backend/src/server/activitypub/followers.ts index 2c2b6cfb4..d6f42fba4 100644 --- a/packages/backend/src/server/activitypub/followers.ts +++ b/packages/backend/src/server/activitypub/followers.ts @@ -82,7 +82,6 @@ export default async (ctx: Router.RouterContext) => { // index page const rendered = renderOrderedCollection(partOf, user.followersCount, `${partOf}?page=true`); ctx.body = renderActivity(rendered); - ctx.set('Cache-Control', 'public, max-age=180'); setResponseType(ctx); } }; diff --git a/packages/backend/src/server/activitypub/following.ts b/packages/backend/src/server/activitypub/following.ts index 4e156a19f..252eaf504 100644 --- a/packages/backend/src/server/activitypub/following.ts +++ b/packages/backend/src/server/activitypub/following.ts @@ -82,7 +82,6 @@ export default async (ctx: Router.RouterContext) => { // index page const rendered = renderOrderedCollection(partOf, user.followingCount, `${partOf}?page=true`); ctx.body = renderActivity(rendered); - ctx.set('Cache-Control', 'public, max-age=180'); setResponseType(ctx); } }; diff --git a/packages/backend/src/server/activitypub/outbox.ts b/packages/backend/src/server/activitypub/outbox.ts index a0a6af011..a0b69f44f 100644 --- a/packages/backend/src/server/activitypub/outbox.ts +++ b/packages/backend/src/server/activitypub/outbox.ts @@ -90,7 +90,6 @@ export default async (ctx: Router.RouterContext) => { `${partOf}?page=true&since_id=000000000000000000000000`, ); ctx.body = renderActivity(rendered); - ctx.set('Cache-Control', 'public, max-age=180'); setResponseType(ctx); } }; diff --git a/packages/backend/src/services/instance-actor.ts b/packages/backend/src/services/instance-actor.ts index 15bdc674f..5e7be93f0 100644 --- a/packages/backend/src/services/instance-actor.ts +++ b/packages/backend/src/services/instance-actor.ts @@ -1,5 +1,5 @@ import { IsNull } from 'typeorm'; -import { ILocalUser } from '@/models/entities/user.js'; +import { ILocalUser, User } from '@/models/entities/user.js'; import { Users } from '@/models/index.js'; import { getSystemUser } from './system-user.js'; @@ -17,3 +17,7 @@ export async function getInstanceActor(): Promise { return instanceActor; } + +export function isInstanceActor(user: User): boolean { + return user.username === ACTOR_USERNAME && user.host === null; +}