From 5f675201f261d5db6a58d3099a190372bb2f09f0 Mon Sep 17 00:00:00 2001 From: Julia Date: Wed, 20 Nov 2024 18:20:09 -0500 Subject: [PATCH] Merge commit from fork * enhance: Add a few validation fixes from Sharkey See the original MR on the GitLab instance: https://activitypub.software/TransFem-org/Sharkey/-/merge_requests/484 Co-Authored-By: Dakkar * fix: primitive 2: acceptance of cross-origin alternate Co-Authored-By: Laura Hausmann * fix: primitive 3: validation of non-final url * fix: primitive 4: missing same-origin identifier validation of collection-wrapped activities * fix: primitives 5 & 8: reject activities with non string identifiers Co-Authored-By: Laura Hausmann * fix: primitive 6: reject anonymous objects that were fetched by their id * fix: primitives 9, 10 & 11: http signature validation doesn't enforce required headers or specify auth header name Co-Authored-By: Laura Hausmann * fix: primitive 14: improper validation of outbox, followers, following & shared inbox collections * fix: code style for primitive 14 * fix: primitive 15: improper same-origin validation for note uri and url Co-Authored-By: Laura Hausmann * fix: primitive 16: improper same-origin validation for user uri and url * fix: primitive 17: note same-origin identifier validation can be bypassed by wrapping the id in an array * fix: code style for primitive 17 * fix: check attribution against actor in notes While this isn't strictly required to fix the exploits at hand, this mirrors the fix in `ApQuestionService` for GHSA-5h8r-gq97-xv69, as a preemptive countermeasure. * fix: primitive 18: `ap/get` bypasses access checks One might argue that we could make this one actually preform access checks against the returned activity object, but I feel like that's a lot more work than just restricting it to administrators, since, to me at least, it seems more like a debugging tool than anything else. * fix: primitive 19 & 20: respect blocks and hide more Ideally, the user property should also be hidden (as leaving it in leaks information slightly), but given the schema of the note endpoint, I don't think that would be possible without introducing some kind of "ghost" user, who is attributed for posts by users who have you blocked. * fix: primitives 21, 22, and 23: reuse resolver This also increases the default `recursionLimit` for `Resolver`, as it theoretically will go higher that it previously would and could possibly fail on non-malicious collection activities. * fix: primitives 25-33: proper local instance checks * revert: fix: primitive 19 & 20 This reverts commit 465a9fe6591de90f78bd3d084e3c01e65dc3cf3c. --------- Co-authored-by: Dakkar Co-authored-by: Laura Hausmann Co-authored-by: syuilo <4439005+syuilo@users.noreply.github.com> --- .../backend/src/core/HttpRequestService.ts | 8 +- .../src/core/RemoteUserResolveService.ts | 4 +- packages/backend/src/core/UtilityService.ts | 14 ++- .../core/activitypub/ApDbResolverService.ts | 6 +- .../src/core/activitypub/ApInboxService.ts | 93 +++++++++++-------- .../src/core/activitypub/ApRequestService.ts | 12 ++- .../src/core/activitypub/ApResolverService.ts | 19 +++- .../activitypub/misc/check-against-url.ts | 19 ++++ .../core/activitypub/models/ApNoteService.ts | 42 ++++++--- .../activitypub/models/ApPersonService.ts | 72 ++++++++++---- .../activitypub/models/ApQuestionService.ts | 4 +- .../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 | 5 + 15 files changed, 227 insertions(+), 76 deletions(-) create mode 100644 packages/backend/src/core/activitypub/misc/check-against-url.ts diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 7f3cac7c58..08e9f46b2d 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -15,6 +15,7 @@ import type { Config } from '@/config.js'; import { StatusError } from '@/misc/status-error.js'; import { bindThis } from '@/decorators.js'; import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js'; +import { assertActivityMatchesUrls } from '@/core/activitypub/misc/check-against-url.js'; import type { IObject } from '@/core/activitypub/type.js'; import type { Response } from 'node-fetch'; import type { URL } from 'node:url'; @@ -125,7 +126,12 @@ export class HttpRequestService { validators: [validateContentTypeSetAsActivityPub], }); - return await res.json() as IObject; + const finalUrl = res.url; // redirects may have been involved + const activity = await res.json() as IObject; + + assertActivityMatchesUrls(activity, [finalUrl]); + + return activity; } @bindThis diff --git a/packages/backend/src/core/RemoteUserResolveService.ts b/packages/backend/src/core/RemoteUserResolveService.ts index f5a55eb8bc..678da0cfa6 100644 --- a/packages/backend/src/core/RemoteUserResolveService.ts +++ b/packages/backend/src/core/RemoteUserResolveService.ts @@ -54,9 +54,9 @@ export class RemoteUserResolveService { }) as MiLocalUser; } - host = this.utilityService.toPuny(host); + host = this.utilityService.punyHost(host); - if (this.config.host === host) { + if (host === this.utilityService.toPuny(this.config.host)) { this.logger.info(`return local user: ${usernameLower}`); return await this.usersRepository.findOneBy({ usernameLower, host: IsNull() }).then(u => { if (u == null) { diff --git a/packages/backend/src/core/UtilityService.ts b/packages/backend/src/core/UtilityService.ts index 86082ccdcd..9a2ba72ed3 100644 --- a/packages/backend/src/core/UtilityService.ts +++ b/packages/backend/src/core/UtilityService.ts @@ -34,6 +34,11 @@ export class UtilityService { return this.toPuny(this.config.host) === this.toPuny(host); } + @bindThis + public isUriLocal(uri: string): boolean { + return this.punyHost(uri) === this.toPuny(this.config.host); + } + @bindThis public isBlockedHost(blockedHosts: string[], host: string | null): boolean { if (host == null) return false; @@ -96,7 +101,7 @@ export class UtilityService { @bindThis public extractDbHost(uri: string): string { const url = new URL(uri); - return this.toPuny(url.hostname); + return this.toPuny(url.host); } @bindThis @@ -110,6 +115,13 @@ export class UtilityService { return toASCII(host.toLowerCase()); } + @bindThis + public punyHost(url: string): string { + const urlObj = new URL(url); + const host = `${this.toPuny(urlObj.hostname)}${urlObj.port.length > 0 ? ':' + urlObj.port : ''}`; + return host; + } + @bindThis public isFederationAllowedHost(host: string): boolean { if (this.meta.federation === 'none') return false; diff --git a/packages/backend/src/core/activitypub/ApDbResolverService.ts b/packages/backend/src/core/activitypub/ApDbResolverService.ts index 4192e8659a..5c16744a77 100644 --- a/packages/backend/src/core/activitypub/ApDbResolverService.ts +++ b/packages/backend/src/core/activitypub/ApDbResolverService.ts @@ -10,6 +10,7 @@ import type { Config } from '@/config.js'; import { MemoryKVCache } from '@/misc/cache.js'; import type { MiUserPublickey } from '@/models/UserPublickey.js'; import { CacheService } from '@/core/CacheService.js'; +import { UtilityService } from '@/core/UtilityService.js'; import type { MiNote } from '@/models/Note.js'; import { bindThis } from '@/decorators.js'; import { MiLocalUser, MiRemoteUser } from '@/models/User.js'; @@ -53,6 +54,7 @@ export class ApDbResolverService implements OnApplicationShutdown { private cacheService: CacheService, private apPersonService: ApPersonService, + private utilityService: UtilityService, ) { this.publicKeyCache = new MemoryKVCache(1000 * 60 * 60 * 12); // 12h this.publicKeyByUserIdCache = new MemoryKVCache(1000 * 60 * 60 * 12); // 12h @@ -63,7 +65,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 (this.utilityService.toPuny(uri.host) !== this.utilityService.toPuny(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 376c9c0151..8e8ec223cb 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -89,15 +89,26 @@ export class ApInboxService { } @bindThis - public async performActivity(actor: MiRemoteUser, activity: IObject): Promise { + public async performActivity(actor: MiRemoteUser, activity: IObject, resolver?: Resolver): Promise { let result = undefined as string | void; if (isCollectionOrOrderedCollection(activity)) { const results = [] as [string, string | void][]; - const resolver = this.apResolverService.createResolver(); - for (const item of toArray(isCollection(activity) ? activity.items : activity.orderedItems)) { + // eslint-disable-next-line no-param-reassign + resolver ??= this.apResolverService.createResolver(); + + const items = toArray(isCollection(activity) ? activity.items : activity.orderedItems); + if (items.length >= resolver.getRecursionLimit()) { + throw new Error(`skipping activity: collection would surpass recursion limit: ${this.utilityService.extractDbHost(actor.uri)}`); + } + + for (const item of items) { const act = await resolver.resolve(item); + if (act.id == null || this.utilityService.extractDbHost(act.id) !== this.utilityService.extractDbHost(actor.uri)) { + this.logger.debug('skipping activity: activity id is null or mismatching'); + continue; + } try { - results.push([getApId(item), await this.performOneActivity(actor, act)]); + results.push([getApId(item), await this.performOneActivity(actor, act, resolver)]); } catch (err) { if (err instanceof Error || typeof err === 'string') { this.logger.error(err); @@ -112,7 +123,7 @@ export class ApInboxService { result = results.map(([id, reason]) => `${id}: ${reason}`).join('\n'); } } else { - result = await this.performOneActivity(actor, activity); + result = await this.performOneActivity(actor, activity, resolver); } // ついでにリモートユーザーの情報が古かったら更新しておく @@ -127,37 +138,37 @@ export class ApInboxService { } @bindThis - public async performOneActivity(actor: MiRemoteUser, activity: IObject): Promise { + public async performOneActivity(actor: MiRemoteUser, activity: IObject, resolver?: Resolver): Promise { if (actor.isSuspended) return; if (isCreate(activity)) { - return await this.create(actor, activity); + return await this.create(actor, activity, resolver); } else if (isDelete(activity)) { return await this.delete(actor, activity); } else if (isUpdate(activity)) { - return await this.update(actor, activity); + return await this.update(actor, activity, resolver); } else if (isFollow(activity)) { return await this.follow(actor, activity); } else if (isAccept(activity)) { - return await this.accept(actor, activity); + return await this.accept(actor, activity, resolver); } else if (isReject(activity)) { - return await this.reject(actor, activity); + return await this.reject(actor, activity, resolver); } else if (isAdd(activity)) { - return await this.add(actor, activity); + return await this.add(actor, activity, resolver); } else if (isRemove(activity)) { - return await this.remove(actor, activity); + return await this.remove(actor, activity, resolver); } else if (isAnnounce(activity)) { - return await this.announce(actor, activity); + return await this.announce(actor, activity, resolver); } else if (isLike(activity)) { return await this.like(actor, activity); } else if (isUndo(activity)) { - return await this.undo(actor, activity); + return await this.undo(actor, activity, resolver); } else if (isBlock(activity)) { return await this.block(actor, activity); } else if (isFlag(activity)) { return await this.flag(actor, activity); } else if (isMove(activity)) { - return await this.move(actor, activity); + return await this.move(actor, activity, resolver); } else { return `unrecognized activity type: ${activity.type}`; } @@ -199,12 +210,13 @@ export class ApInboxService { } @bindThis - private async accept(actor: MiRemoteUser, activity: IAccept): Promise { + private async accept(actor: MiRemoteUser, activity: IAccept, resolver?: Resolver): Promise { const uri = activity.id ?? activity; this.logger.info(`Accept: ${uri}`); - const resolver = this.apResolverService.createResolver(); + // eslint-disable-next-line no-param-reassign + resolver ??= this.apResolverService.createResolver(); const object = await resolver.resolve(activity.object).catch(err => { this.logger.error(`Resolution failed: ${err}`); @@ -241,7 +253,7 @@ export class ApInboxService { } @bindThis - private async add(actor: MiRemoteUser, activity: IAdd): Promise { + private async add(actor: MiRemoteUser, activity: IAdd, resolver?: Resolver): Promise { if (actor.uri !== activity.actor) { return 'invalid actor'; } @@ -251,7 +263,7 @@ export class ApInboxService { } if (activity.target === actor.featured) { - const note = await this.apNoteService.resolveNote(activity.object); + const note = await this.apNoteService.resolveNote(activity.object, { resolver }); if (note == null) return 'note not found'; await this.notePiningService.addPinned(actor, note.id); return; @@ -261,12 +273,13 @@ export class ApInboxService { } @bindThis - private async announce(actor: MiRemoteUser, activity: IAnnounce): Promise { + private async announce(actor: MiRemoteUser, activity: IAnnounce, resolver?: Resolver): Promise { const uri = getApId(activity); this.logger.info(`Announce: ${uri}`); - const resolver = this.apResolverService.createResolver(); + // eslint-disable-next-line no-param-reassign + resolver ??= this.apResolverService.createResolver(); if (!activity.object) return 'skip: activity has no object property'; const targetUri = getApId(activity.object); @@ -283,7 +296,7 @@ export class ApInboxService { } @bindThis - private async announceNote(actor: MiRemoteUser, activity: IAnnounce, target: IPost): Promise { + private async announceNote(actor: MiRemoteUser, activity: IAnnounce, target: IPost, resolver?: Resolver): Promise { const uri = getApId(activity); if (actor.isSuspended) { @@ -305,7 +318,7 @@ export class ApInboxService { // Announce対象をresolve let renote; try { - renote = await this.apNoteService.resolveNote(target); + renote = await this.apNoteService.resolveNote(target, { resolver }); if (renote == null) return 'announce target is null'; } catch (err) { // 対象が4xxならスキップ @@ -324,7 +337,7 @@ export class ApInboxService { this.logger.info(`Creating the (Re)Note: ${uri}`); - const activityAudience = await this.apAudienceService.parseAudience(actor, activity.to, activity.cc); + const activityAudience = await this.apAudienceService.parseAudience(actor, activity.to, activity.cc, resolver); const createdAt = activity.published ? new Date(activity.published) : null; if (createdAt && createdAt < this.idService.parse(renote.id).date) { @@ -362,7 +375,7 @@ export class ApInboxService { } @bindThis - private async create(actor: MiRemoteUser, activity: ICreate): Promise { + private async create(actor: MiRemoteUser, activity: ICreate, resolver?: Resolver): Promise { const uri = getApId(activity); this.logger.info(`Create: ${uri}`); @@ -387,7 +400,8 @@ export class ApInboxService { activity.object.attributedTo = activity.actor; } - const resolver = this.apResolverService.createResolver(); + // eslint-disable-next-line no-param-reassign + resolver ??= this.apResolverService.createResolver(); const object = await resolver.resolve(activity.object).catch(e => { this.logger.error(`Resolution failed: ${e}`); @@ -414,6 +428,8 @@ export class ApInboxService { if (this.utilityService.extractDbHost(actor.uri) !== this.utilityService.extractDbHost(note.id)) { return 'skip: host in actor.uri !== note.id'; } + } else { + return 'skip: note.id is not a string'; } } @@ -423,7 +439,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) { @@ -555,12 +571,13 @@ export class ApInboxService { } @bindThis - private async reject(actor: MiRemoteUser, activity: IReject): Promise { + private async reject(actor: MiRemoteUser, activity: IReject, resolver?: Resolver): Promise { const uri = activity.id ?? activity; this.logger.info(`Reject: ${uri}`); - const resolver = this.apResolverService.createResolver(); + // eslint-disable-next-line no-param-reassign + resolver ??= this.apResolverService.createResolver(); const object = await resolver.resolve(activity.object).catch(e => { this.logger.error(`Resolution failed: ${e}`); @@ -597,7 +614,7 @@ export class ApInboxService { } @bindThis - private async remove(actor: MiRemoteUser, activity: IRemove): Promise { + private async remove(actor: MiRemoteUser, activity: IRemove, resolver?: Resolver): Promise { if (actor.uri !== activity.actor) { return 'invalid actor'; } @@ -607,7 +624,7 @@ export class ApInboxService { } if (activity.target === actor.featured) { - const note = await this.apNoteService.resolveNote(activity.object); + const note = await this.apNoteService.resolveNote(activity.object, { resolver }); if (note == null) return 'note not found'; await this.notePiningService.removePinned(actor, note.id); return; @@ -617,7 +634,7 @@ export class ApInboxService { } @bindThis - private async undo(actor: MiRemoteUser, activity: IUndo): Promise { + private async undo(actor: MiRemoteUser, activity: IUndo, resolver?: Resolver): Promise { if (actor.uri !== activity.actor) { return 'invalid actor'; } @@ -626,7 +643,8 @@ export class ApInboxService { this.logger.info(`Undo: ${uri}`); - const resolver = this.apResolverService.createResolver(); + // eslint-disable-next-line no-param-reassign + resolver ??= this.apResolverService.createResolver(); const object = await resolver.resolve(activity.object).catch(e => { this.logger.error(`Resolution failed: ${e}`); @@ -750,14 +768,15 @@ export class ApInboxService { } @bindThis - private async update(actor: MiRemoteUser, activity: IUpdate): Promise { + private async update(actor: MiRemoteUser, activity: IUpdate, resolver?: Resolver): Promise { if (actor.uri !== activity.actor) { return 'skip: invalid actor'; } this.logger.debug('Update'); - const resolver = this.apResolverService.createResolver(); + // eslint-disable-next-line no-param-reassign + resolver ??= this.apResolverService.createResolver(); const object = await resolver.resolve(activity.object).catch(e => { this.logger.error(`Resolution failed: ${e}`); @@ -776,11 +795,11 @@ export class ApInboxService { } @bindThis - private async move(actor: MiRemoteUser, activity: IMove): Promise { + private async move(actor: MiRemoteUser, activity: IMove, resolver?: Resolver): Promise { // fetch the new and old accounts const targetUri = getApHrefNullable(activity.target); if (!targetUri) return 'skip: invalid activity target'; - return await this.apPersonService.updatePerson(actor.uri) ?? 'skip: nothing to do'; + return await this.apPersonService.updatePerson(actor.uri, resolver) ?? 'skip: nothing to do'; } } diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts index c7d19adfd5..8c3b7295e4 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') と言われる @@ -238,7 +242,7 @@ export class ApRequestService { const alternate = document.querySelector('head > link[rel="alternate"][type="application/activity+json"]'); if (alternate) { const href = alternate.getAttribute('href'); - if (href) { + if (href && this.utilityService.punyHost(url) === this.utilityService.punyHost(href)) { return await this.signedGet(href, user, false); } } @@ -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, [finalUrl]); + + return activity; } } diff --git a/packages/backend/src/core/activitypub/ApResolverService.ts b/packages/backend/src/core/activitypub/ApResolverService.ts index ca35608d9b..b0b35274ea 100644 --- a/packages/backend/src/core/activitypub/ApResolverService.ts +++ b/packages/backend/src/core/activitypub/ApResolverService.ts @@ -41,7 +41,7 @@ export class Resolver { private apRendererService: ApRendererService, private apDbResolverService: ApDbResolverService, private loggerService: LoggerService, - private recursionLimit = 100, + private recursionLimit = 256, ) { this.history = new Set(); this.logger = this.loggerService.getLogger('ap-resolve'); @@ -52,6 +52,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' @@ -113,6 +118,18 @@ 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'); + } + + if (this.utilityService.punyHost(object.id) !== this.utilityService.punyHost(value)) { + throw new Error(`invalid AP object ${value}: id ${object.id} has different host`); + } + return object; } diff --git a/packages/backend/src/core/activitypub/misc/check-against-url.ts b/packages/backend/src/core/activitypub/misc/check-against-url.ts new file mode 100644 index 0000000000..78ba891a2e --- /dev/null +++ b/packages/backend/src/core/activitypub/misc/check-against-url.ts @@ -0,0 +1,19 @@ +/* + * SPDX-FileCopyrightText: dakkar and sharkey-project + * SPDX-License-Identifier: AGPL-3.0-only + */ +import type { IObject } from '../type.js'; + +export function assertActivityMatchesUrls(activity: IObject, urls: string[]) { + const idOk = activity.id !== undefined && urls.includes(activity.id); + + // technically `activity.url` could be an `ApObject = IObject | + // string | (IObject | string)[]`, but if it's a complicated thing + // and the `activity.id` doesn't match, I think we're fine + // rejecting the activity + const urlOk = typeof(activity.url) === 'string' && urls.includes(activity.url); + + if (!idOk && !urlOk) { + throw new Error(`bad Activity: neither id(${activity?.id}) nor url(${activity?.url}) match location(${urls})`); + } +} diff --git a/packages/backend/src/core/activitypub/models/ApNoteService.ts b/packages/backend/src/core/activitypub/models/ApNoteService.ts index 2d333b3634..eb2e771a38 100644 --- a/packages/backend/src/core/activitypub/models/ApNoteService.ts +++ b/packages/backend/src/core/activitypub/models/ApNoteService.ts @@ -77,7 +77,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 +98,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,14 +123,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() }, @@ -136,14 +144,24 @@ 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); + } + + if (this.utilityService.punyHost(url) !== this.utilityService.punyHost(note.id)) { + throw new Error(`note url & uri host mismatch: note url: ${url}, note uri: ${note.id}`); + } } this.logger.info(`Creating the Note: ${note.id}`); @@ -156,8 +174,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'); } @@ -189,7 +208,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) { @@ -348,7 +368,7 @@ export class ApNoteService { if (exist) return exist; //#endregion - if (uri.startsWith(this.config.url)) { + if (this.utilityService.isUriLocal(uri)) { throw new StatusError('cannot resolve local note', 400, 'cannot resolve local note'); } @@ -356,7 +376,7 @@ export class ApNoteService { // ここでuriの代わりに添付されてきたNote Objectが指定されていると、サーバーフェッチを経ずにノートが生成されるが // 添付されてきたNote Objectは偽装されている可能性があるため、常にuriを指定してサーバーフェッチを行う。 const createFrom = options.sentFrom?.origin === new URL(uri).origin ? value : uri; - return await this.createNote(createFrom, options.resolver, 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 8c4e40c561..026ddb6ece 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -129,12 +129,6 @@ export class ApPersonService implements OnModuleInit { this.logger = this.apLoggerService.logger; } - private punyHost(url: string): string { - const urlObj = new URL(url); - const host = `${this.utilityService.toPuny(urlObj.hostname)}${urlObj.port.length > 0 ? ':' + urlObj.port : ''}`; - return host; - } - /** * Validate and convert to actor object * @param x Fetched object @@ -142,7 +136,7 @@ export class ApPersonService implements OnModuleInit { */ @bindThis private validateActor(x: IObject, uri: string): IActor { - const expectHost = this.punyHost(uri); + const expectHost = this.utilityService.punyHost(uri); if (!isActor(x)) { throw new Error(`invalid Actor type '${x.type}'`); @@ -156,6 +150,29 @@ export class ApPersonService implements OnModuleInit { throw new Error('invalid Actor: wrong inbox'); } + if (this.utilityService.punyHost(x.inbox) !== expectHost) { + throw new Error('invalid Actor: inbox has different host'); + } + + const sharedInboxObject = x.sharedInbox ?? (x.endpoints ? x.endpoints.sharedInbox : undefined); + if (sharedInboxObject != null) { + const sharedInbox = getApId(sharedInboxObject); + if (!(typeof sharedInbox === 'string' && sharedInbox.length > 0 && this.utilityService.punyHost(sharedInbox) === expectHost)) { + throw new Error('invalid Actor: wrong shared inbox'); + } + } + + for (const collection of ['outbox', 'followers', 'following'] as (keyof IActor)[]) { + const collectionUri = getApId((x as IActor)[collection]); + if (typeof collectionUri === 'string' && collectionUri.length > 0) { + if (this.utilityService.punyHost(collectionUri) !== expectHost) { + throw new Error(`invalid Actor: ${collection} has different host`); + } + } else if (collectionUri != null) { + throw new Error(`invalid Actor: wrong ${collection}`); + } + } + if (!(typeof x.preferredUsername === 'string' && x.preferredUsername.length > 0 && x.preferredUsername.length <= 128 && /^\w([\w-.]*\w)?$/.test(x.preferredUsername))) { throw new Error('invalid Actor: wrong username'); } @@ -179,7 +196,7 @@ export class ApPersonService implements OnModuleInit { x.summary = truncate(x.summary, summaryLength); } - const idHost = this.punyHost(x.id); + const idHost = this.utilityService.punyHost(x.id); if (idHost !== expectHost) { throw new Error('invalid Actor: id has different host'); } @@ -189,7 +206,7 @@ export class ApPersonService implements OnModuleInit { throw new Error('invalid Actor: publicKey.id is not a string'); } - const publicKeyIdHost = this.punyHost(x.publicKey.id); + const publicKeyIdHost = this.utilityService.punyHost(x.publicKey.id); if (publicKeyIdHost !== expectHost) { throw new Error('invalid Actor: publicKey.id has different host'); } @@ -280,7 +297,8 @@ export class ApPersonService implements OnModuleInit { public async createPerson(uri: string, resolver?: Resolver): Promise { if (typeof uri !== 'string') throw new Error('uri is not string'); - if (uri.startsWith(this.config.url)) { + const host = this.utilityService.punyHost(uri); + if (host === this.utilityService.toPuny(this.config.host)) { throw new StatusError('cannot resolve local user', 400, 'cannot resolve local user'); } @@ -294,8 +312,6 @@ export class ApPersonService implements OnModuleInit { this.logger.info(`Creating the Person: ${person.id}`); - const host = this.punyHost(object.id); - const fields = this.analyzeAttachments(person.attachment ?? []); const tags = extractApHashtags(person.tag).map(normalizeForSearch).splice(0, 32); @@ -321,8 +337,18 @@ export class ApPersonService implements OnModuleInit { const url = getOneApHrefNullable(person.url); - if (url && !checkHttps(url)) { - throw new Error('unexpected schema of person url: ' + url); + if (person.id == null) { + throw new Error('Refusing to create person without id'); + } + + if (url != null) { + if (!checkHttps(url)) { + throw new Error('unexpected schema of person url: ' + url); + } + + if (this.utilityService.punyHost(url) !== this.utilityService.punyHost(person.id)) { + throw new Error(`person url <> uri host mismatch: ${url} <> ${person.id}`); + } } // Create user @@ -465,7 +491,7 @@ export class ApPersonService implements OnModuleInit { if (typeof uri !== 'string') throw new Error('uri is not string'); // URIがこのサーバーを指しているならスキップ - if (uri.startsWith(`${this.config.url}/`)) return; + if (this.utilityService.isUriLocal(uri)) return; //#region このサーバーに既に登録されているか const exist = await this.fetchPerson(uri) as MiRemoteUser | null; @@ -514,8 +540,18 @@ export class ApPersonService implements OnModuleInit { const url = getOneApHrefNullable(person.url); - if (url && !checkHttps(url)) { - throw new Error('unexpected schema of person url: ' + url); + if (person.id == null) { + throw new Error('Refusing to update person without id'); + } + + if (url != null) { + if (!checkHttps(url)) { + throw new Error('unexpected schema of person url: ' + url); + } + + if (this.utilityService.punyHost(url) !== this.utilityService.punyHost(person.id)) { + throw new Error(`person url <> uri host mismatch: ${url} <> ${person.id}`); + } } const updates = { @@ -728,7 +764,7 @@ export class ApPersonService implements OnModuleInit { await this.updatePerson(src.movedToUri, undefined, undefined, [...movePreventUris, src.uri]); dst = await this.fetchPerson(src.movedToUri) ?? dst; } else { - if (src.movedToUri.startsWith(`${this.config.url}/`)) { + if (this.utilityService.isUriLocal(src.movedToUri)) { // ローカルユーザーっぽいのにfetchPersonで見つからないということはmovedToUriが間違っている return 'failed: movedTo is local but not found'; } diff --git a/packages/backend/src/core/activitypub/models/ApQuestionService.ts b/packages/backend/src/core/activitypub/models/ApQuestionService.ts index 73004d10b0..1655a307c4 100644 --- a/packages/backend/src/core/activitypub/models/ApQuestionService.ts +++ b/packages/backend/src/core/activitypub/models/ApQuestionService.ts @@ -10,6 +10,7 @@ import type { Config } from '@/config.js'; import type { IPoll } from '@/models/Poll.js'; import type Logger from '@/logger.js'; import { bindThis } from '@/decorators.js'; +import { UtilityService } from '@/core/UtilityService.js'; import { isQuestion } from '../type.js'; import { ApLoggerService } from '../ApLoggerService.js'; import { ApResolverService } from '../ApResolverService.js'; @@ -32,6 +33,7 @@ export class ApQuestionService { private apResolverService: ApResolverService, private apLoggerService: ApLoggerService, + private utilityService: UtilityService, ) { this.logger = this.apLoggerService.logger; } @@ -70,7 +72,7 @@ export class ApQuestionService { if (uri == null) throw new Error('uri is null'); // URIがこのサーバーを指しているならスキップ - if (uri.startsWith(this.config.url + '/')) throw new Error('uri points local'); + if (this.utilityService.isUriLocal(uri)) throw new Error('uri points local'); //#region このサーバーに既に登録されているか const note = await this.notesRepository.findOneBy({ uri }); diff --git a/packages/backend/src/queue/processors/InboxProcessorService.ts b/packages/backend/src/queue/processors/InboxProcessorService.ts index 95d764e4d8..004fe1382d 100644 --- a/packages/backend/src/queue/processors/InboxProcessorService.ts +++ b/packages/backend/src/queue/processors/InboxProcessorService.ts @@ -190,6 +190,8 @@ export class InboxProcessorService implements OnApplicationShutdown { if (signerHost !== activityIdHost) { 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 c52608cefb..aca8258c08 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;