Merge pull request from GHSA-2vxv-pv3m-3wvj

* fix: normalize incoming signed activities * Tweak style * Update CHANGELOG.md * Log compacted activity as well --------- Co-authored-by: syuilo <4439005+syuilo@users.noreply.github.com>
2024-11-21 22:35:11 +00:00 · 2024-05-01 07:33:58 +00:00 · 2024-05-01 07:33:58 +00:00 · d2a5bb39e3
commit d2a5bb39e3
parent 9f66f22953
7 changed files with 146 additions and 70 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -2,6 +2,7 @@

 ### Note
 - コントロールパネル内にあるサマリープロキシの設定個所がセキュリティから全般へ変更となります。
+- 悪意のある第三者がリモートユーザーになりすましたアクティビティを受け取れてしまう問題を修正しました。詳しくは[GitHub security advisory](https://github.com/misskey-dev/misskey/security/advisories/GHSA-2vxv-pv3m-3wvj)をご覧ください。

 ### General
 - Enhance: URLプレビューの有効化・無効化を設定できるように #13569
@ -61,6 +62,7 @@
 ### Server
 - Enhance: エンドポイント`antennas/update`の必須項目を`antennaId`のみに
 - Enhance: misskey-dev/summaly@5.1.0の取り込み（プレビュー生成処理の効率化）
+- Fix: リモートから配送されたアクティビティにJSON-LD compactionをかける
 - Fix: フォローリクエストを作成する際に既存のものは削除するように  
  (Cherry-picked from https://activitypub.software/TransFem-org/Sharkey/-/merge_requests/440)
 - Fix: エンドポイント`notes/translate`のエラーを改善
--- a/packages/backend/src/core/CoreModule.ts
+++ b/packages/backend/src/core/CoreModule.ts
@ -127,7 +127,7 @@ import { ApMfmService } from './activitypub/ApMfmService.js';
 import { ApRendererService } from './activitypub/ApRendererService.js';
 import { ApRequestService } from './activitypub/ApRequestService.js';
 import { ApResolverService } from './activitypub/ApResolverService.js';
-import { LdSignatureService } from './activitypub/LdSignatureService.js';
+import { JsonLdService } from './activitypub/JsonLdService.js';
 import { RemoteLoggerService } from './RemoteLoggerService.js';
 import { RemoteUserResolveService } from './RemoteUserResolveService.js';
 import { WebfingerService } from './WebfingerService.js';
@ -266,7 +266,7 @@ const $ApMfmService: Provider = { provide: 'ApMfmService', useExisting: ApMfmSer
 const $ApRendererService: Provider = { provide: 'ApRendererService', useExisting: ApRendererService };
 const $ApRequestService: Provider = { provide: 'ApRequestService', useExisting: ApRequestService };
 const $ApResolverService: Provider = { provide: 'ApResolverService', useExisting: ApResolverService };
-const $LdSignatureService: Provider = { provide: 'LdSignatureService', useExisting: LdSignatureService };
+const $JsonLdService: Provider = { provide: 'JsonLdService', useExisting: JsonLdService };
 const $RemoteLoggerService: Provider = { provide: 'RemoteLoggerService', useExisting: RemoteLoggerService };
 const $RemoteUserResolveService: Provider = { provide: 'RemoteUserResolveService', useExisting: RemoteUserResolveService };
 const $WebfingerService: Provider = { provide: 'WebfingerService', useExisting: WebfingerService };
@ -406,7 +406,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting
 		ApRendererService,
 		ApRequestService,
 		ApResolverService,
-		LdSignatureService,
+		JsonLdService,
 		RemoteLoggerService,
 		RemoteUserResolveService,
 		WebfingerService,
@ -542,7 +542,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting
 		$ApRendererService,
 		$ApRequestService,
 		$ApResolverService,
-		$LdSignatureService,
+		$JsonLdService,
 		$RemoteLoggerService,
 		$RemoteUserResolveService,
 		$WebfingerService,
@ -678,7 +678,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting
 		ApRendererService,
 		ApRequestService,
 		ApResolverService,
-		LdSignatureService,
+		JsonLdService,
 		RemoteLoggerService,
 		RemoteUserResolveService,
 		WebfingerService,
@ -813,7 +813,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting
 		$ApRendererService,
 		$ApRequestService,
 		$ApResolverService,
-		$LdSignatureService,
+		$JsonLdService,
 		$RemoteLoggerService,
 		$RemoteUserResolveService,
 		$WebfingerService,
--- a/packages/backend/src/core/activitypub/ApRendererService.ts
+++ b/packages/backend/src/core/activitypub/ApRendererService.ts
@ -28,8 +28,9 @@ import { bindThis } from '@/decorators.js';
 import { CustomEmojiService } from '@/core/CustomEmojiService.js';
 import { isNotNull } from '@/misc/is-not-null.js';
 import { IdService } from '@/core/IdService.js';
-import { LdSignatureService } from './LdSignatureService.js';
+import { JsonLdService } from './JsonLdService.js';
 import { ApMfmService } from './ApMfmService.js';
+import { CONTEXT } from './misc/contexts.js';
 import type { IAccept, IActivity, IAdd, IAnnounce, IApDocument, IApEmoji, IApHashtag, IApImage, IApMention, IBlock, ICreate, IDelete, IFlag, IFollow, IKey, ILike, IMove, IObject, IPost, IQuestion, IReject, IRemove, ITombstone, IUndo, IUpdate } from './type.js';

@Injectable()
@ -56,7 +57,7 @@ export class ApRendererService {
 		private customEmojiService: CustomEmojiService,
 		private userEntityService: UserEntityService,
 		private driveFileEntityService: DriveFileEntityService,
-		private ldSignatureService: LdSignatureService,
+		private jsonLdService: JsonLdService,
 		private userKeypairService: UserKeypairService,
 		private apMfmService: ApMfmService,
 		private mfmService: MfmService,
@ -617,48 +618,16 @@ export class ApRendererService {
 			x.id = `${this.config.url}/${randomUUID()}`;
 		}

-		return Object.assign({
-			'@context': [
-				'https://www.w3.org/ns/activitystreams',
-				'https://w3id.org/security/v1',
-				{
-					Key: 'sec:Key',
-					// as non-standards
-					manuallyApprovesFollowers: 'as:manuallyApprovesFollowers',
-					sensitive: 'as:sensitive',
-					Hashtag: 'as:Hashtag',
-					quoteUrl: 'as:quoteUrl',
-					// Mastodon
-					toot: 'http://joinmastodon.org/ns#',
-					Emoji: 'toot:Emoji',
-					featured: 'toot:featured',
-					discoverable: 'toot:discoverable',
-					// schema
-					schema: 'http://schema.org#',
-					PropertyValue: 'schema:PropertyValue',
-					value: 'schema:value',
-					// Misskey
-					misskey: 'https://misskey-hub.net/ns#',
-					'_misskey_content': 'misskey:_misskey_content',
-					'_misskey_quote': 'misskey:_misskey_quote',
-					'_misskey_reaction': 'misskey:_misskey_reaction',
-					'_misskey_votes': 'misskey:_misskey_votes',
-					'_misskey_summary': 'misskey:_misskey_summary',
-					'isCat': 'misskey:isCat',
-					// vcard
-					vcard: 'http://www.w3.org/2006/vcard/ns#',
-				},
-			],
-		}, x as T & { id: string });
+		return Object.assign({ '@context': CONTEXT }, x as T & { id: string });
 	}

 	@bindThis
 	public async attachLdSignature(activity: any, user: { id: MiUser['id']; host: null; }): Promise<IActivity> {
 		const keypair = await this.userKeypairService.getUserKeypair(user.id);

-		const ldSignature = this.ldSignatureService.use();
-		ldSignature.debug = false;
-		activity = await ldSignature.signRsaSignature2017(activity, keypair.privateKey, `${this.config.url}/users/${user.id}#main-key`);
+		const jsonLd = this.jsonLdService.use();
+		jsonLd.debug = false;
+		activity = await jsonLd.signRsaSignature2017(activity, keypair.privateKey, `${this.config.url}/users/${user.id}#main-key`);

 		return activity;
 	}
--- a/packages/backend/src/core/activitypub/LdSignatureService.ts
+++ b/packages/backend/src/core/activitypub/LdSignatureService.ts
@ -7,14 +7,14 @@ import * as crypto from 'node:crypto';
 import { Injectable } from '@nestjs/common';
 import { HttpRequestService } from '@/core/HttpRequestService.js';
 import { bindThis } from '@/decorators.js';
-import { CONTEXTS } from './misc/contexts.js';
+import { CONTEXT, PRELOADED_CONTEXTS } from './misc/contexts.js';
 import { validateContentTypeSetAsJsonLD } from './misc/validator.js';
 import type { JsonLdDocument } from 'jsonld';
-import type { JsonLd, RemoteDocument } from 'jsonld/jsonld-spec.js';
+import type { JsonLd as JsonLdObject, RemoteDocument } from 'jsonld/jsonld-spec.js';

-// RsaSignature2017 based from https://github.com/transmute-industries/RsaSignature2017
+// RsaSignature2017 implementation is based on https://github.com/transmute-industries/RsaSignature2017

-class LdSignature {
+class JsonLd {
 	public debug = false;
 	public preLoad = true;
 	public loderTimeout = 5000;
@ -89,10 +89,18 @@ class LdSignature {
 	}

 	@bindThis
-	public async normalize(data: JsonLdDocument): Promise<string> {
+	public async compact(data: any, context: any = CONTEXT): Promise<JsonLdDocument> {
 		const customLoader = this.getLoader();
 		// XXX: Importing jsonld dynamically since Jest frequently fails to import it statically
 		// https://github.com/misskey-dev/misskey/pull/9894#discussion_r1103753595
+		return (await import('jsonld')).default.compact(data, context, {
+			documentLoader: customLoader,
+		});
+	}
+
+	@bindThis
+	public async normalize(data: JsonLdDocument): Promise<string> {
+		const customLoader = this.getLoader();
 		return (await import('jsonld')).default.normalize(data, {
 			documentLoader: customLoader,
 		});
@ -104,11 +112,11 @@ class LdSignature {
 			if (!/^https?:\/\//.test(url)) throw new Error(`Invalid URL ${url}`);

 			if (this.preLoad) {
-				if (url in CONTEXTS) {
+				if (url in PRELOADED_CONTEXTS) {
 					if (this.debug) console.debug(`HIT: ${url}`);
 					return {
 						contextUrl: undefined,
-						document: CONTEXTS[url],
+						document: PRELOADED_CONTEXTS[url],
 						documentUrl: url,
 					};
 				}
@ -125,7 +133,7 @@ class LdSignature {
 	}

 	@bindThis
-	private async fetchDocument(url: string): Promise<JsonLd> {
+	private async fetchDocument(url: string): Promise<JsonLdObject> {
 		const json = await this.httpRequestService.send(
 			url,
 			{
@ -146,7 +154,7 @@ class LdSignature {
 			}
 		});

-		return json as JsonLd;
+		return json as JsonLdObject;
 	}

 	@bindThis
@ -158,14 +166,14 @@ class LdSignature {
 }

@Injectable()
-export class LdSignatureService {
+export class JsonLdService {
 	constructor(
 		private httpRequestService: HttpRequestService,
 	) {
 	}

 	@bindThis
-	public use(): LdSignature {
-		return new LdSignature(this.httpRequestService);
+	public use(): JsonLd {
+		return new JsonLd(this.httpRequestService);
 	}
 }
--- a/packages/backend/src/core/activitypub/misc/contexts.ts
+++ b/packages/backend/src/core/activitypub/misc/contexts.ts
@ -3,7 +3,7 @@
 * SPDX-License-Identifier: AGPL-3.0-only
 */

-import type { JsonLd } from 'jsonld/jsonld-spec.js';
+import type { Context, JsonLd } from 'jsonld/jsonld-spec.js';

 /* eslint:disable:quotemark indent */
 const id_v1 = {
@ -526,7 +526,42 @@ const activitystreams = {
 	},
 } satisfies JsonLd;

-export const CONTEXTS: Record<string, JsonLd> = {
+const context_iris = [
+	'https://www.w3.org/ns/activitystreams',
+	'https://w3id.org/security/v1',
+];
+
+const extension_context_definition = {
+	Key: 'sec:Key',
+	// as non-standards
+	manuallyApprovesFollowers: 'as:manuallyApprovesFollowers',
+	sensitive: 'as:sensitive',
+	Hashtag: 'as:Hashtag',
+	quoteUrl: 'as:quoteUrl',
+	// Mastodon
+	toot: 'http://joinmastodon.org/ns#',
+	Emoji: 'toot:Emoji',
+	featured: 'toot:featured',
+	discoverable: 'toot:discoverable',
+	// schema
+	schema: 'http://schema.org#',
+	PropertyValue: 'schema:PropertyValue',
+	value: 'schema:value',
+	// Misskey
+	misskey: 'https://misskey-hub.net/ns#',
+	'_misskey_content': 'misskey:_misskey_content',
+	'_misskey_quote': 'misskey:_misskey_quote',
+	'_misskey_reaction': 'misskey:_misskey_reaction',
+	'_misskey_votes': 'misskey:_misskey_votes',
+	'_misskey_summary': 'misskey:_misskey_summary',
+	'isCat': 'misskey:isCat',
+	// vcard
+	vcard: 'http://www.w3.org/2006/vcard/ns#',
+} satisfies Context;
+
+export const CONTEXT: (string | Context)[] = [...context_iris, extension_context_definition];
+
+export const PRELOADED_CONTEXTS: Record<string, JsonLd> = {
 	'https://w3id.org/identity/v1': id_v1,
 	'https://w3id.org/security/v1': security_v1,
 	'https://www.w3.org/ns/activitystreams': activitystreams,
--- a/packages/backend/src/queue/processors/InboxProcessorService.ts
+++ b/packages/backend/src/queue/processors/InboxProcessorService.ts
@ -15,13 +15,14 @@ import InstanceChart from '@/core/chart/charts/instance.js';
 import ApRequestChart from '@/core/chart/charts/ap-request.js';
 import FederationChart from '@/core/chart/charts/federation.js';
 import { getApId } from '@/core/activitypub/type.js';
+import type { IActivity } from '@/core/activitypub/type.js';
 import type { MiRemoteUser } from '@/models/User.js';
 import type { MiUserPublickey } from '@/models/UserPublickey.js';
 import { ApDbResolverService } from '@/core/activitypub/ApDbResolverService.js';
 import { StatusError } from '@/misc/status-error.js';
 import { UtilityService } from '@/core/UtilityService.js';
 import { ApPersonService } from '@/core/activitypub/models/ApPersonService.js';
-import { LdSignatureService } from '@/core/activitypub/LdSignatureService.js';
+import { JsonLdService } from '@/core/activitypub/JsonLdService.js';
 import { ApInboxService } from '@/core/activitypub/ApInboxService.js';
 import { bindThis } from '@/decorators.js';
 import { IdentifiableError } from '@/misc/identifiable-error.js';
@ -38,7 +39,7 @@ export class InboxProcessorService {
 		private apInboxService: ApInboxService,
 		private federatedInstanceService: FederatedInstanceService,
 		private fetchInstanceMetadataService: FetchInstanceMetadataService,
-		private ldSignatureService: LdSignatureService,
+		private jsonLdService: JsonLdService,
 		private apPersonService: ApPersonService,
 		private apDbResolverService: ApDbResolverService,
 		private instanceChart: InstanceChart,
@ -52,7 +53,7 @@ export class InboxProcessorService {
 	@bindThis
 	public async process(job: Bull.Job<InboxJobData>): Promise<string> {
 		const signature = job.data.signature;	// HTTP-signature
-		const activity = job.data.activity;
+		let activity = job.data.activity;

 		//#region Log
 		const info = Object.assign({}, activity);
@ -110,20 +111,21 @@ export class InboxProcessorService {
 		// また、signatureのsignerは、activity.actorと一致する必要がある
 		if (!httpSignatureValidated || authUser.user.uri !== activity.actor) {
 			// 一致しなくても、でもLD-Signatureがありそうならそっちも見る
-			if (activity.signature) {
-				if (activity.signature.type !== 'RsaSignature2017') {
-					throw new Bull.UnrecoverableError(`skip: unsupported LD-signature type ${activity.signature.type}`);
+			const ldSignature = activity.signature;
+			if (ldSignature) {
+				if (ldSignature.type !== 'RsaSignature2017') {
+					throw new Bull.UnrecoverableError(`skip: unsupported LD-signature type ${ldSignature.type}`);
 				}

-				// activity.signature.creator: https://example.oom/users/user#main-key
+				// ldSignature.creator: https://example.oom/users/user#main-key
 				// みたいになっててUserを引っ張れば公開キーも入ることを期待する
-				if (activity.signature.creator) {
-					const candicate = activity.signature.creator.replace(/#.*/, '');
+				if (ldSignature.creator) {
+					const candicate = ldSignature.creator.replace(/#.*/, '');
 					await this.apPersonService.resolvePerson(candicate).catch(() => null);
 				}

 				// keyIdからLD-Signatureのユーザーを取得
-				authUser = await this.apDbResolverService.getAuthUserFromKeyId(activity.signature.creator);
+				authUser = await this.apDbResolverService.getAuthUserFromKeyId(ldSignature.creator);
 				if (authUser == null) {
 					throw new Bull.UnrecoverableError('skip: LD-Signatureのユーザーが取得できませんでした');
 				}
@ -132,13 +134,31 @@ export class InboxProcessorService {
 					throw new Bull.UnrecoverableError('skip: LD-SignatureのユーザーはpublicKeyを持っていませんでした');
 				}

+				const jsonLd = this.jsonLdService.use();
+
 				// LD-Signature検証
-				const ldSignature = this.ldSignatureService.use();
-				const verified = await ldSignature.verifyRsaSignature2017(activity, authUser.key.keyPem).catch(() => false);
+				const verified = await jsonLd.verifyRsaSignature2017(activity, authUser.key.keyPem).catch(() => false);
 				if (!verified) {
 					throw new Bull.UnrecoverableError('skip: LD-Signatureの検証に失敗しました');
 				}

+				// アクティビティを正規化
+				delete activity.signature;
+				try {
+					activity = await jsonLd.compact(activity) as IActivity;
+				} catch (e) {
+					throw new Bull.UnrecoverableError(`skip: failed to compact activity: ${e}`);
+				}
+				// TODO: 元のアクティビティと非互換な形に正規化される場合は転送をスキップする
+				// https://github.com/mastodon/mastodon/blob/664b0ca/app/services/activitypub/process_collection_service.rb#L24-L29
+				activity.signature = ldSignature;
+
+				//#region Log
+				const compactedInfo = Object.assign({}, activity);
+				delete compactedInfo['@context'];
+				this.logger.debug(`compacted: ${JSON.stringify(compactedInfo, null, 2)}`);
+				//#endregion
+
 				// もう一度actorチェック
 				if (authUser.user.uri !== activity.actor) {
 					throw new Bull.UnrecoverableError(`skip: LD-Signature user(${authUser.user.uri}) !== activity.actor(${activity.actor})`);
--- a/packages/backend/test/unit/activitypub.ts
+++ b/packages/backend/test/unit/activitypub.ts
@ -13,6 +13,8 @@ import { ApImageService } from '@/core/activitypub/models/ApImageService.js';
 import { ApNoteService } from '@/core/activitypub/models/ApNoteService.js';
 import { ApPersonService } from '@/core/activitypub/models/ApPersonService.js';
 import { ApRendererService } from '@/core/activitypub/ApRendererService.js';
+import { JsonLdService } from '@/core/activitypub/JsonLdService.js';
+import { CONTEXT } from '@/core/activitypub/misc/contexts.js';
 import { GlobalModule } from '@/GlobalModule.js';
 import { CoreModule } from '@/core/CoreModule.js';
 import { FederatedInstanceService } from '@/core/FederatedInstanceService.js';
@ -88,6 +90,7 @@ describe('ActivityPub', () => {
 	let noteService: ApNoteService;
 	let personService: ApPersonService;
 	let rendererService: ApRendererService;
+	let jsonLdService: JsonLdService;
 	let resolver: MockResolver;

 	const metaInitial = {
@ -128,6 +131,7 @@ describe('ActivityPub', () => {
 		personService = app.get<ApPersonService>(ApPersonService);
 		rendererService = app.get<ApRendererService>(ApRendererService);
 		imageService = app.get<ApImageService>(ApImageService);
+		jsonLdService = app.get<JsonLdService>(JsonLdService);
 		resolver = new MockResolver(await app.resolve<LoggerService>(LoggerService));

 		// Prevent ApPersonService from fetching instance, as it causes Jest import-after-test error
@ -381,4 +385,42 @@ describe('ActivityPub', () => {
 			assert.strictEqual(driveFile, null);
 		});
 	});
+
+	describe('JSON-LD', () =>{
+		test('Compaction', async () => {
+			const jsonLd = jsonLdService.use();
+
+			const object = {
+				'@context': [
+					'https://www.w3.org/ns/activitystreams',
+					{
+						_misskey_quote: 'https://misskey-hub.net/ns#_misskey_quote',
+						unknown: 'https://example.org/ns#unknown',
+						undefined: null,
+					},
+				],
+				id: 'https://example.com/notes/42',
+				type: 'Note',
+				attributedTo: 'https://example.com/users/1',
+				to: ['https://www.w3.org/ns/activitystreams#Public'],
+				content: 'test test foo',
+				_misskey_quote: 'https://example.com/notes/1',
+				unknown: 'test test bar',
+				undefined: 'test test baz',
+			};
+			const compacted = await jsonLd.compact(object);
+
+			assert.deepStrictEqual(compacted, {
+				'@context': CONTEXT,
+				id: 'https://example.com/notes/42',
+				type: 'Note',
+				attributedTo: 'https://example.com/users/1',
+				to: 'as:Public',
+				content: 'test test foo',
+				_misskey_quote: 'https://example.com/notes/1',
+				'https://example.org/ns#unknown': 'test test bar',
+				// undefined: 'test test baz',
+			});
+		});
+	});
 });