From b600efae0dbb6d786e4defa0df173b4eee6b9283 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), the configuration item `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 --- packages/backend/src/config/load.ts | 1 + packages/backend/src/config/types.ts | 2 + .../src/remote/activitypub/misc/auth-user.ts | 9 +- packages/backend/src/server/activitypub.ts | 76 +++++++++++-- .../src/server/activitypub/featured.ts | 1 - .../src/server/activitypub/fetch-signature.ts | 101 ++++++++++++++++++ .../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, 183 insertions(+), 16 deletions(-) create mode 100644 packages/backend/src/server/activitypub/fetch-signature.ts 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/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..24947d473 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,33 @@ export function setResponseType(ctx: Router.RouterContext): void { } } +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; + 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 +95,7 @@ router.post('/users/:user/inbox', json(), inbox); // note router.get('/notes/:note', async (ctx, next) => { if (!isActivityPubReq(ctx)) return await next(); + if (!(await handleSignature(ctx))) return; const note = await Notes.findOneBy({ id: ctx.params.note, @@ -89,7 +119,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 +132,7 @@ router.get('/notes/:note/activity', async ctx => { ctx.redirect(`/notes/${ctx.params.note}`); return; } + if (!(await handleSignature(ctx))) return; const note = await Notes.findOneBy({ id: ctx.params.note, @@ -117,21 +147,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 (!(await handleSignature(ctx))) return; + return await Outbox(ctx); +}); // followers -router.get('/users/:user/followers', Followers); +router.get('/users/:user/followers', async ctx => { + if (!(await handleSignature(ctx))) return; + return await Followers(ctx); +}); // following -router.get('/users/:user/following', Following); +router.get('/users/:user/following', async ctx => { + if (!(await handleSignature(ctx))) return; + return await Following(ctx); +}); // featured -router.get('/users/:user/collections/featured', Featured); +router.get('/users/:user/collections/featured', async ctx => { + if (!(await handleSignature(ctx))) return; + return await Featured(ctx); +}); // publickey router.get('/users/:user/publickey', async ctx => { @@ -166,7 +207,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 (!(await handleSignature(ctx))) 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 (!(await handleSignature(ctx))) return; const user = await Users.findOneBy({ usernameLower: ctx.params.user.toLowerCase(), @@ -198,6 +253,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 +273,7 @@ router.get('/emojis/:emoji', async ctx => { // like router.get('/likes/:like', async ctx => { + if (!(await handleSignature(ctx))) return; const reaction = await NoteReactions.findOneBy({ id: ctx.params.like }); if (reaction == null) { @@ -233,12 +292,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 (!(await handleSignature(ctx))) return; // This may be used before the follow is completed, so we do not // check if the following exists. @@ -259,7 +318,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..521b4c106 --- /dev/null +++ b/packages/backend/src/server/activitypub/fetch-signature.ts @@ -0,0 +1,101 @@ +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'; + +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 { + 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'; + } + + // This old `keyId` format is no longer supported. + const keyIdLower = signature.keyId.toLowerCase(); + if (keyIdLower.startsWith('acct:')) + return 'invalid'; + + const host = extractDbHost(keyIdLower); + + // 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; +}