From 504ead526af92fa27cd768955b867afb980eee31 Mon Sep 17 00:00:00 2001 From: Julia Date: Wed, 20 Nov 2024 18:20:09 -0500 Subject: [PATCH] Partial Merge of 5f675201f261d5db6a58d3099a190372bb2f09f0 Signed-off-by: eternal-flame-AD --- packages/backend/src/core/DownloadService.ts | 1 - .../backend/src/core/HttpRequestService.ts | 2 +- .../core/activitypub/ApDbResolverService.ts | 5 +- .../src/core/activitypub/ApInboxService.ts | 2 +- .../src/core/activitypub/ApRequestService.ts | 10 +++- .../src/core/activitypub/ApResolverService.ts | 22 ++++++++ .../src/core/activitypub/misc/validator.ts | 22 ++++++++ .../core/activitypub/models/ApNoteService.ts | 51 ++++++++++++++----- .../activitypub/models/ApPersonService.ts | 2 +- .../activitypub/models/ApQuestionService.ts | 12 +++-- .../queue/processors/InboxProcessorService.ts | 2 + .../src/server/ActivityPubServerService.ts | 2 +- .../src/server/api/endpoints/ap/get.ts | 1 + .../src/server/api/endpoints/ap/show.ts | 7 ++- .../src/server/web/UrlPreviewService.ts | 1 - packages/backend/test/unit/activitypub.ts | 4 +- 16 files changed, 117 insertions(+), 29 deletions(-) diff --git a/packages/backend/src/core/DownloadService.ts b/packages/backend/src/core/DownloadService.ts index 93f4a38246..6266e51496 100644 --- a/packages/backend/src/core/DownloadService.ts +++ b/packages/backend/src/core/DownloadService.ts @@ -61,7 +61,6 @@ export class DownloadService { request: operationTimeout, // whole operation timeout }, agent: { - http: this.httpRequestService.httpAgent, https: this.httpRequestService.httpsAgent, }, http2: false, // default diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 8a6d978232..0f39c0536b 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -107,7 +107,7 @@ export class HttpRequestService { const finalUrl = res.url; // redirects may have been involved const activity = await res.json() as IObject; - assertActivityMatchesUrls(activity, [url, finalUrl]); + assertActivityMatchesUrls(activity, [finalUrl]); return activity; } diff --git a/packages/backend/src/core/activitypub/ApDbResolverService.ts b/packages/backend/src/core/activitypub/ApDbResolverService.ts index 4192e8659a..6cc3ebab52 100644 --- a/packages/backend/src/core/activitypub/ApDbResolverService.ts +++ b/packages/backend/src/core/activitypub/ApDbResolverService.ts @@ -16,6 +16,7 @@ import { MiLocalUser, MiRemoteUser } from '@/models/User.js'; import { getApId } from './type.js'; import { ApPersonService } from './models/ApPersonService.js'; import type { IObject } from './type.js'; +import { toASCII } from 'node:punycode'; export type UriParseResult = { /** wether the URI was generated by us */ @@ -63,7 +64,9 @@ export class ApDbResolverService implements OnApplicationShutdown { const separator = '/'; const uri = new URL(getApId(value)); - if (uri.origin !== this.config.url) return { local: false, uri: uri.href }; + if (toASCII(uri.host) !== toASCII(this.config.host)) { + return { local: false, uri: uri.href }; + } const [, type, id, ...rest] = uri.pathname.split(separator); return { diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index 1b28b4b4a5..64ddd76bfa 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -487,7 +487,7 @@ export class ApInboxService { const exist = await this.apNoteService.fetchNote(note); if (exist) return 'skip: note exists'; - await this.apNoteService.createNote(note, resolver, silent); + await this.apNoteService.createNote(note, actor, resolver, silent); return 'ok'; } catch (err) { if (err instanceof StatusError && !err.isRetryable) { diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts index c7d19adfd5..500104d0f9 100644 --- a/packages/backend/src/core/activitypub/ApRequestService.ts +++ b/packages/backend/src/core/activitypub/ApRequestService.ts @@ -11,11 +11,14 @@ import { DI } from '@/di-symbols.js'; import type { Config } from '@/config.js'; import type { MiUser } from '@/models/User.js'; import { UserKeypairService } from '@/core/UserKeypairService.js'; +import { UtilityService } from '@/core/UtilityService.js'; import { HttpRequestService } from '@/core/HttpRequestService.js'; import { LoggerService } from '@/core/LoggerService.js'; import { bindThis } from '@/decorators.js'; import type Logger from '@/logger.js'; import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js'; +import { assertActivityMatchesUrls } from '@/core/activitypub/misc/check-against-url.js'; +import type { IObject } from './type.js'; type Request = { url: string; @@ -145,6 +148,7 @@ export class ApRequestService { private userKeypairService: UserKeypairService, private httpRequestService: HttpRequestService, private loggerService: LoggerService, + private utilityService: UtilityService, ) { // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition this.logger = this.loggerService?.getLogger('ap-request'); // なぜか TypeError: Cannot read properties of undefined (reading 'getLogger') と言われる @@ -251,7 +255,11 @@ export class ApRequestService { //#endregion validateContentTypeSetAsActivityPub(res); + const finalUrl = res.url; // redirects may have been involved + const activity = await res.json() as IObject; - return await res.json(); + assertActivityMatchesUrls(activity, [url, finalUrl]); + + return activity; } } diff --git a/packages/backend/src/core/activitypub/ApResolverService.ts b/packages/backend/src/core/activitypub/ApResolverService.ts index 5244de43de..b4fbaf24e4 100644 --- a/packages/backend/src/core/activitypub/ApResolverService.ts +++ b/packages/backend/src/core/activitypub/ApResolverService.ts @@ -21,6 +21,8 @@ import { ApDbResolverService } from './ApDbResolverService.js'; import { ApRendererService } from './ApRendererService.js'; import { ApRequestService } from './ApRequestService.js'; import type { IObject, ICollection, IOrderedCollection, IUnsanitizedObject } from './type.js'; +import { toASCII } from 'node:punycode'; +import { yumeAssertAcceptableURL } from './misc/validator.js'; export class Resolver { private history: Set; @@ -53,6 +55,11 @@ export class Resolver { return Array.from(this.history); } + @bindThis + public getRecursionLimit(): number { + return this.recursionLimit; + } + @bindThis public async resolveCollection(value: string | IObject): Promise { const collection = typeof value === 'string' @@ -114,6 +121,21 @@ export class Resolver { throw new Error('invalid response'); } + // HttpRequestService / ApRequestService have already checked that + // `object.id` or `object.url` matches the URL used to fetch the + // object after redirects; here we double-check that no redirects + // bounced between hosts + if (object.id == null) { + throw new Error('invalid AP object: missing id'); + } + + const idURL = yumeAssertAcceptableURL(object.id); + const valueURL = yumeAssertAcceptableURL(value); + + if (toASCII(idURL.host) !== toASCII(valueURL.host)) { + throw new Bull.UnrecoverableError(`invalid AP object ${value}: id ${object.id} has different host`); + } + return object; } diff --git a/packages/backend/src/core/activitypub/misc/validator.ts b/packages/backend/src/core/activitypub/misc/validator.ts index 690beeffef..8470d0c6ec 100644 --- a/packages/backend/src/core/activitypub/misc/validator.ts +++ b/packages/backend/src/core/activitypub/misc/validator.ts @@ -4,6 +4,28 @@ */ import type { Response } from 'node-fetch'; +import * as Bull from 'bullmq'; +import { toASCII } from 'node:punycode'; + +export function yumeAssertAcceptableURL(url: string | URL): URL { + const urlParsed = url instanceof URL ? url : new URL(url); + + if (urlParsed.search.length + urlParsed.pathname.length > 1024) { + throw new Bull.UnrecoverableError('URL is too long'); + } + + if (urlParsed.protocol !== 'https:') { + throw new Bull.UnrecoverableError('URL protocol is not https'); + } + + if (urlParsed.port && urlParsed.port !== '443') { + throw new Bull.UnrecoverableError('URL port is not 443'); + } + + urlParsed.hostname = toASCII(urlParsed.hostname); + + return urlParsed; +} export function validateContentTypeSetAsActivityPub(response: Response): void { const contentType = (response.headers.get('content-type') ?? '').toLowerCase(); diff --git a/packages/backend/src/core/activitypub/models/ApNoteService.ts b/packages/backend/src/core/activitypub/models/ApNoteService.ts index 43c4994706..d9d004a9cf 100644 --- a/packages/backend/src/core/activitypub/models/ApNoteService.ts +++ b/packages/backend/src/core/activitypub/models/ApNoteService.ts @@ -36,6 +36,7 @@ import { ApQuestionService } from './ApQuestionService.js'; import { ApImageService } from './ApImageService.js'; import type { Resolver } from '../ApResolverService.js'; import type { IObject, IPost } from '../type.js'; +import { yumeAssertAcceptableURL } from '../misc/validator.js'; @Injectable() export class ApNoteService { @@ -77,7 +78,7 @@ export class ApNoteService { } @bindThis - public validateNote(object: IObject, uri: string): Error | null { + public validateNote(object: IObject, uri: string, actor?: MiRemoteUser): Error | null { const expectHost = this.utilityService.extractDbHost(uri); const apType = getApType(object); @@ -98,6 +99,14 @@ export class ApNoteService { return new IdentifiableError('d450b8a9-48e4-4dab-ae36-f4db763fda7c', 'invalid Note: published timestamp is malformed'); } + if (actor) { + const attribution = (object.attributedTo) ? getOneApId(object.attributedTo) : actor.uri; + + if (attribution !== actor.uri) { + return new IdentifiableError('d450b8a9-48e4-4dab-ae36-f4db763fda7c', `invalid Note: attribution does not match the actor that send it. attribution: ${attribution}, actor: ${actor.uri}`); + } + } + return null; } @@ -115,11 +124,14 @@ export class ApNoteService { * Noteを作成します。 */ @bindThis - public async createNote(value: string | IObject, resolver: Resolver, silent = false): Promise { + public async createNote(value: string | IObject, actor?: MiRemoteUser, resolver?: Resolver, silent = false): Promise { + // eslint-disable-next-line no-param-reassign + if (resolver == null) resolver = this.apResolverService.createResolver(); + const object = await resolver.resolve(value); const entryUri = getApId(value); - const err = this.validateNote(object, entryUri); + const err = this.validateNote(object, entryUri, actor); if (err) { this.logger.error(err.message, { resolver: { history: resolver.getHistory() }, @@ -133,14 +145,27 @@ export class ApNoteService { this.logger.debug(`Note fetched: ${JSON.stringify(note, null, 2)}`); - if (note.id && !checkHttps(note.id)) { + if (note.id == null) { + throw new Error('Refusing to create note without id'); + } + + if (!checkHttps(note.id)) { throw new Error('unexpected schema of note.id: ' + note.id); } const url = getOneApHrefNullable(note.url); - if (url && !checkHttps(url)) { - throw new Error('unexpected schema of note url: ' + url); + if (url != null) { + if (!checkHttps(url)) { + throw new Error('unexpected schema of note url: ' + url); + } + + const actUrl = yumeAssertAcceptableURL(url); + const noteUrl = yumeAssertAcceptableURL(note.id); + + if (noteUrl.host !== actUrl.host) { + throw new Error(`note url & uri host mismatch: note url: ${url}, note uri: ${note.id}`); + } } this.logger.info(`Creating the Note: ${note.id}`); @@ -153,8 +178,9 @@ export class ApNoteService { const uri = getOneApId(note.attributedTo); // ローカルで投稿者を検索し、もし凍結されていたらスキップ - const cachedActor = await this.apPersonService.fetchPerson(uri) as MiRemoteUser; - if (cachedActor && cachedActor.isSuspended) { + // eslint-disable-next-line no-param-reassign + actor ??= await this.apPersonService.fetchPerson(uri) as MiRemoteUser | undefined; + if (actor && actor.isSuspended) { throw new IdentifiableError('85ab9bd7-3a41-4530-959d-f07073900109', 'actor has been suspended'); } @@ -186,7 +212,8 @@ export class ApNoteService { } //#endregion - const actor = cachedActor ?? await this.apPersonService.resolvePerson(uri, resolver) as MiRemoteUser; + // eslint-disable-next-line no-param-reassign + actor ??= await this.apPersonService.resolvePerson(uri, resolver) as MiRemoteUser; // 解決した投稿者が凍結されていたらスキップ if (actor.isSuspended) { @@ -345,15 +372,11 @@ export class ApNoteService { if (exist) return exist; //#endregion - if (uri.startsWith(this.config.url)) { - throw new StatusError('cannot resolve local note', 400, 'cannot resolve local note'); - } - // リモートサーバーからフェッチしてきて登録 // ここでuriの代わりに添付されてきたNote Objectが指定されていると、サーバーフェッチを経ずにノートが生成されるが // 添付されてきたNote Objectは偽装されている可能性があるため、常にuriを指定してサーバーフェッチを行う。 const createFrom = options.sentFrom?.origin === new URL(uri).origin ? value : uri; - return await this.createNote(createFrom, this.apResolverService.createResolver(), true); + return await this.createNote(createFrom, undefined, options.resolver, true); } finally { unlock(); } diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index d02701da52..00c9d1cf89 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -661,7 +661,7 @@ export class ApPersonService implements OnModuleInit { @bindThis public async updateFeatured(userId: MiUser['id'], resolver?: Resolver): Promise { - const user = await this.usersRepository.findOneByOrFail({ id: userId, isDeleted: false }); + const user = await this.usersRepository.findOneByOrFail({ id: userId }); if (!this.userEntityService.isRemoteUser(user)) return; if (!user.featured) return; diff --git a/packages/backend/src/core/activitypub/models/ApQuestionService.ts b/packages/backend/src/core/activitypub/models/ApQuestionService.ts index 73004d10b0..7a040aa7e8 100644 --- a/packages/backend/src/core/activitypub/models/ApQuestionService.ts +++ b/packages/backend/src/core/activitypub/models/ApQuestionService.ts @@ -15,6 +15,8 @@ import { ApLoggerService } from '../ApLoggerService.js'; import { ApResolverService } from '../ApResolverService.js'; import type { Resolver } from '../ApResolverService.js'; import type { IObject, IQuestion } from '../type.js'; +import { yumeAssertAcceptableURL } from '../misc/validator.js'; +import { toASCII } from 'punycode'; @Injectable() export class ApQuestionService { @@ -66,14 +68,16 @@ export class ApQuestionService { */ @bindThis public async updateQuestion(value: string | IObject, resolver?: Resolver): Promise { - const uri = typeof value === 'string' ? value : value.id; - if (uri == null) throw new Error('uri is null'); + const uriIn = typeof value === 'string' ? value : value.id; + if (uriIn == null) throw new Error('uri is null'); // URIがこのサーバーを指しているならスキップ - if (uri.startsWith(this.config.url + '/')) throw new Error('uri points local'); + const uri = yumeAssertAcceptableURL(uriIn); + + if (toASCII(this.config.host) === uri.host) throw new Error('uri points local'); //#region このサーバーに既に登録されているか - const note = await this.notesRepository.findOneBy({ uri }); + const note = await this.notesRepository.findOneBy({ uri: uriIn }); if (note == null) throw new Error('Question is not registered'); const poll = await this.pollsRepository.findOneBy({ noteId: note.id }); diff --git a/packages/backend/src/queue/processors/InboxProcessorService.ts b/packages/backend/src/queue/processors/InboxProcessorService.ts index 8dbba7f51e..c8d4bdb84a 100644 --- a/packages/backend/src/queue/processors/InboxProcessorService.ts +++ b/packages/backend/src/queue/processors/InboxProcessorService.ts @@ -255,6 +255,8 @@ export class InboxProcessorService implements OnApplicationShutdown { incCounter(mIncomingApReject, 'host_signature_mismatch'); throw new Bull.UnrecoverableError(`skip: signerHost(${signerHost}) !== activity.id host(${activityIdHost}`); } + } else { + throw new Bull.UnrecoverableError('skip: activity id is not a string'); } this.apRequestChart.inbox(); diff --git a/packages/backend/src/server/ActivityPubServerService.ts b/packages/backend/src/server/ActivityPubServerService.ts index ba2342b630..f34f6583d3 100644 --- a/packages/backend/src/server/ActivityPubServerService.ts +++ b/packages/backend/src/server/ActivityPubServerService.ts @@ -105,7 +105,7 @@ export class ActivityPubServerService { let signature; try { - signature = httpSignature.parseRequest(request.raw, { 'headers': [] }); + signature = httpSignature.parseRequest(request.raw, { 'headers': ['(request-target)', 'host', 'date'], authorizationHeaderName: 'signature' }); } catch (e) { reply.code(401); return; diff --git a/packages/backend/src/server/api/endpoints/ap/get.ts b/packages/backend/src/server/api/endpoints/ap/get.ts index d8c55de7ec..14286bc23e 100644 --- a/packages/backend/src/server/api/endpoints/ap/get.ts +++ b/packages/backend/src/server/api/endpoints/ap/get.ts @@ -11,6 +11,7 @@ import { ApResolverService } from '@/core/activitypub/ApResolverService.js'; export const meta = { tags: ['federation'], + requireAdmin: true, requireCredential: true, kind: 'read:federation', diff --git a/packages/backend/src/server/api/endpoints/ap/show.ts b/packages/backend/src/server/api/endpoints/ap/show.ts index 6127e104ce..bf99834c17 100644 --- a/packages/backend/src/server/api/endpoints/ap/show.ts +++ b/packages/backend/src/server/api/endpoints/ap/show.ts @@ -118,6 +118,11 @@ export default class extends Endpoint { // eslint- ])); if (local != null) return local; + const host = this.utilityService.extractDbHost(uri); + + // local object, not found in db? fail + if (this.utilityService.isSelfHost(host)) return null; + // リモートから一旦オブジェクトフェッチ const resolver = this.apResolverService.createResolver(); const object = await resolver.resolve(uri) as any; @@ -135,7 +140,7 @@ export default class extends Endpoint { // eslint- return await this.mergePack( me, isActor(object) ? await this.apPersonService.createPerson(getApId(object), resolver) : null, - isPost(object) ? await this.apNoteService.createNote(getApId(object), resolver, true) : null, + isPost(object) ? await this.apNoteService.createNote(getApId(object), undefined, resolver, true) : null, ); } diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index 5d493c2c46..94b4a9fd90 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -118,7 +118,6 @@ export class UrlPreviewService { private fetchSummary(url: string, meta: MiMeta, lang?: string): Promise { const agent = this.config.proxy ? { - http: this.httpRequestService.httpAgent, https: this.httpRequestService.httpsAgent, } : undefined; diff --git a/packages/backend/test/unit/activitypub.ts b/packages/backend/test/unit/activitypub.ts index fe336a0252..954bf8a62e 100644 --- a/packages/backend/test/unit/activitypub.ts +++ b/packages/backend/test/unit/activitypub.ts @@ -207,7 +207,7 @@ describe('ActivityPub', () => { resolver.register(actor.id, actor); resolver.register(post.id, post); - const note = await noteService.createNote(post.id, resolver, true); + const note = await noteService.createNote(post.id, undefined, resolver, true); assert.deepStrictEqual(note?.uri, post.id); assert.deepStrictEqual(note.visibility, 'public'); @@ -370,7 +370,7 @@ describe('ActivityPub', () => { resolver.register(actor.featured, featured); resolver.register(firstNote.id, firstNote); - const note = await noteService.createNote(firstNote.id as string, resolver); + const note = await noteService.createNote(firstNote.id as string, undefined, resolver); assert.strictEqual(note?.uri, firstNote.id); }); });