server: refactor HTTP signature validation

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 { AuthUser, getAuthUser } from '@/remote/activitypub/misc/auth-user.js';
-import { StatusError } from '@/misc/fetch.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<InboxJobData>): Promise<string> => {
 	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<InboxJobData>): Promise<string> => {
 	logger.debug(JSON.stringify(info, null, 2));
 	//#endregion
 
-	const keyIdLower = signature.keyId.toLowerCase();
+	const validated = await verifyHttpSignature(signature, resolver, getApId(activity.actor));
-	if (keyIdLower.startsWith('acct:')) {
+	let authUser = validated.authUser;
-		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);
 
 	// 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<InboxJobData>): Promise<string> => {
 		}
 	}
 
+	// 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)) {

packages/backend/src/remote/http-signature.ts

@@ -1,14 +1,12 @@
-import httpSignature from '@peertube/http-signature';
+import { URL } from 'node:url';
-import { extractDbHost } from '@/misc/convert-host.js'; 
+import { extractDbHost } from "@/misc/convert-host.js";
-import { shouldBlockInstance } from '@/misc/should-block-instance.js';
+import { shouldBlockInstance } from "@/misc/should-block-instance.js";
-import { authUserFromKeyId, getAuthUser } from '@/remote/activitypub/misc/auth-user.js';
+import httpSignature from "@peertube/http-signature";
-import { getApId, isActor } from '@/remote/activitypub/type.js';
+import { Resolver } from "./activitypub/resolver.js";
-import { StatusError } from '@/misc/fetch.js';
+import { StatusError } from "@/misc/fetch.js";
-import { Resolver } from '@/remote/activitypub/resolver.js';
+import { AuthUser, authUserFromKeyId, getAuthUser } from "./activitypub/misc/auth-user.js";
-import { createPerson } from '@/remote/activitypub/models/person.js';
+import { ApObject, getApId, isActor } from "./activitypub/type.js";
-import config from '@/config/index.js';
+import { createPerson } from "./activitypub/models/person.js";
-
-export type SignatureValidationResult = 'missing' | 'invalid' | 'rejected' | 'valid' | 'always';
 
 async function resolveKeyId(keyId: string, resolver: Resolver): Promise<AuthUser | null> {
 	// Do we already know that keyId?
@@ -18,7 +16,7 @@ async function resolveKeyId(keyId: string, resolver: Resolver): Promise<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?
@@ -38,42 +36,36 @@ async function resolveKeyId(keyId: string, resolver: Resolver): Promise<AuthUser
 	return null;
 }
 
-export async function validateFetchSignature(req: IncomingMessage): Promise<SignatureValidationResult> {
+export type SignatureValidationResult = {
-	let signature;
+	status: 'missing' | 'invalid' | 'rejected';
-
+	authUser: AuthUser | null;
-	if (config.allowUnsignedFetches === true)
+} | {
-		return 'always';
+	status: 'valid';
-
+	authUser: AuthUser;
-	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 async function verifyHttpSignature(signature: httpSignature.IParsedSignature, resolver: Resolver, actor?: ApObject): Promise<SignatureValidationResult> {
 	// This old `keyId` format is no longer supported.
 	const keyIdLower = signature.keyId.toLowerCase();
-	if (keyIdLower.startsWith('acct:'))
+	if (keyIdLower.startsWith('acct:')) return { status: 'invalid', authUser: null };
-		return 'invalid';
 
 	const host = extractDbHost(keyIdLower);
 
 	// Reject if the host is blocked.
-	if (await shouldBlockInstance(host))
+	if (await shouldBlockInstance(host)) return { status: 'rejected', authUser: null };
-		return 'rejected';
 
-	const resolver = new Resolver();
+	let authUser = null;
-	let authUser;
 	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<Sign
 
 	if (authUser == null) {
 		// Key not found? Unacceptable!
-		return 'invalid';
+		return { status: 'invalid', authUser };
 	} else {
 		// Found key!
 	}
 
 	// Make sure the resolved user matches the keyId host.
-	if (authUser.user.host !== host)
+	if (authUser.user.host !== host) return { status: 'rejected', authUser };
-		return 'rejected';
 
 	// Verify the HTTP Signature
 	const httpSignatureValidated = httpSignature.verifySignature(signature, authUser.key.keyPem);
 	if (httpSignatureValidated === true)
-		return 'valid';
+		return {
+			status: 'valid',
+			authUser,
+		};
 
 	// Otherwise, fail.
-	return 'invalid';
+	return { status: 'invalid', authUser };
 }

packages/backend/src/server/activitypub.ts

@@ -20,8 +20,11 @@ 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';
+import { getUser } from './api/common/getters.js';
+import config from '@/config/index.js';
+import { verifyHttpSignature } from '@/remote/http-signature.js';
+import { Resolver } from '@/remote/activitypub/resolver.js';
 
 // Init router
 const router = new Router();
@@ -62,30 +65,33 @@ export function setResponseType(ctx: Router.RouterContext): void {
 }
 
 async function handleSignature(ctx: Router.RouterContext): Promise<boolean> {
-	const result = await validateFetchSignature(ctx.req);
+	if (config.allowUnsignedFetches) {
-	switch (result) {
 		// Fetch signature verification is disabled.
-		case 'always':
+		ctx.set('Cache-Control', 'public, max-age=180');
-			ctx.set('Cache-Control', 'public, max-age=180');
+		return true;
-			return true;
+	} else {
-		// Fetch signature verification succeeded.
+		let verified;
-		case 'valid':
+		try {
-			ctx.set('Cache-Control', 'no-store');
+			let signature = httpSignature.parseRequest(ctx.req);
-			return true;
+			verified = await verifyHttpSignature(signature, new Resolver());
-		case 'missing':
+		} catch (e) {
-		case 'invalid':
+			verified = { status: 'missing' };
-			// 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');
+		switch (verified.status) {
-	return false;
+			// 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<void>) {
+	if (!(await handleSignature(ctx))) {
+		return;
+	} else {
+		await next();
+	}
+}
+
 // outbox
-router.get('/users/:user/outbox', async ctx => {
+router.get('/users/:user/outbox', requireHttpSignature, Outbox);
-	if (!(await handleSignature(ctx))) return;
-	return await Outbox(ctx);
-});
 
 // followers
-router.get('/users/:user/followers', async ctx => {
+router.get('/users/:user/followers',  requireHttpSignature, Followers);
-	if (!(await handleSignature(ctx))) return;
-	return await Followers(ctx);
-});
 
 // following
-router.get('/users/:user/following', async ctx => {
+router.get('/users/:user/following', requireHttpSignature, Following);
-	if (!(await handleSignature(ctx))) return;
-	return await Following(ctx);
-});
 
 // featured
-router.get('/users/:user/collections/featured', async ctx => {
+router.get('/users/:user/collections/featured', requireHttpSignature, Featured);
-	if (!(await handleSignature(ctx))) return;
-	return await Featured(ctx);
-});
 
 // 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;