From 9ab25ede28f4f04ac2ae48c947e7668a9a6012b2 Mon Sep 17 00:00:00 2001 From: Laura Hausmann Date: Thu, 24 Oct 2024 04:40:33 +0200 Subject: fix: primitives 9, 10 & 11: http signature validation doesn't enforce required headers or specify auth header name --- packages/backend/src/server/ActivityPubServerService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'packages/backend/src/server') diff --git a/packages/backend/src/server/ActivityPubServerService.ts b/packages/backend/src/server/ActivityPubServerService.ts index 52592c47c6..f955329fd1 100644 --- a/packages/backend/src/server/ActivityPubServerService.ts +++ b/packages/backend/src/server/ActivityPubServerService.ts @@ -152,7 +152,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) { // not signed, or malformed signature: refuse this.authlogger.warn(`${request.id} ${request.url} not signed, or malformed signature: refuse`); @@ -229,7 +229,7 @@ export class ActivityPubServerService { let signature; try { - signature = httpSignature.parseRequest(request.raw, { 'headers': [] }); + signature = httpSignature.parseRequest(request.raw, { 'headers': ['(request-target)', 'digest', 'host', 'date'], authorizationHeaderName: 'signature' }); } catch (e) { reply.code(401); return; -- cgit v1.2.3-freya From c04f34404991d69103703d781f52658fe3214d15 Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Thu, 14 Nov 2024 21:17:30 -0500 Subject: fix: primitive 13: check attribution against actor in notes --- .../backend/src/core/activitypub/ApInboxService.ts | 4 +- .../src/core/activitypub/models/ApNoteService.ts | 71 ++++++++++++++-------- .../backend/src/server/api/endpoints/ap/show.ts | 2 +- 3 files changed, 48 insertions(+), 29 deletions(-) (limited to 'packages/backend/src/server') diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index 42c2007799..0f56ad3f78 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -436,7 +436,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) { @@ -791,7 +791,7 @@ export class ApInboxService { await this.apQuestionService.updateQuestion(object, actor, resolver).catch(err => console.error(err)); return 'ok: Question updated'; } else if (getApType(object) === 'Note') { - await this.apNoteService.updateNote(object, resolver).catch(err => console.error(err)); + await this.apNoteService.updateNote(object, actor, resolver).catch(err => console.error(err)); return 'ok: Note updated'; } else { return `skip: Unknown type: ${getApType(object)}`; diff --git a/packages/backend/src/core/activitypub/models/ApNoteService.ts b/packages/backend/src/core/activitypub/models/ApNoteService.ts index 146ccb11a2..7857bcc28c 100644 --- a/packages/backend/src/core/activitypub/models/ApNoteService.ts +++ b/packages/backend/src/core/activitypub/models/ApNoteService.ts @@ -6,7 +6,7 @@ import { forwardRef, Inject, Injectable } from '@nestjs/common'; import { In } from 'typeorm'; import { DI } from '@/di-symbols.js'; -import type { PollsRepository, EmojisRepository, NotesRepository, MiMeta } from '@/models/_.js'; +import type { UsersRepository, PollsRepository, EmojisRepository, NotesRepository, MiMeta } from '@/models/_.js'; import type { Config } from '@/config.js'; import type { MiRemoteUser } from '@/models/User.js'; import type { MiNote } from '@/models/Note.js'; @@ -49,6 +49,9 @@ export class ApNoteService { @Inject(DI.meta) private meta: MiMeta, + @Inject(DI.usersRepository) + private usersRepository: UsersRepository, + @Inject(DI.pollsRepository) private pollsRepository: PollsRepository, @@ -82,7 +85,13 @@ export class ApNoteService { } @bindThis - public validateNote(object: IObject, uri: string): Error | null { + public validateNote( + object: IObject, + uri: string, + actor?: MiRemoteUser, + user?: MiRemoteUser, + note?: MiNote, + ): Error | null { const expectHost = this.utilityService.extractDbHost(uri); const apType = getApType(object); @@ -99,10 +108,27 @@ export class ApNoteService { return new IdentifiableError('d450b8a9-48e4-4dab-ae36-f4db763fda7c', `invalid Note: attributedTo has different host. expected: ${expectHost}, actual: ${actualHost}`); } + 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}`); + } + if (user && attribution !== user.uri) { + return new IdentifiableError('d450b8a9-48e4-4dab-ae36-f4db763fda7c', `invalid Note: updated attribution does not match original attribution. updated attribution: ${user.uri}, original attribution: ${attribution}`); + } + } + if (object.published && !this.idService.isSafeT(new Date(object.published).valueOf())) { return new IdentifiableError('d450b8a9-48e4-4dab-ae36-f4db763fda7c', 'invalid Note: published timestamp is malformed'); } + if (note) { + const url = (object.url) ? getOneApId(object.url) : note.url; + if (url && url !== note.url) { + return new IdentifiableError('d450b8a9-48e4-4dab-ae36-f4db763fda7c', `invalid Note: updated url does not match original url. updated url: ${url}, original url: ${note.url}`); + } + } + return null; } @@ -120,14 +146,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() }, @@ -171,8 +197,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'); } @@ -204,7 +231,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,7 +373,7 @@ export class ApNoteService { * Noteを作成します。 */ @bindThis - public async updateNote(value: string | IObject, resolver?: Resolver, silent = false): Promise { + public async updateNote(value: string | IObject, actor?: MiRemoteUser, resolver?: Resolver, silent = false): Promise { const noteUri = typeof value === 'string' ? value : value.id; if (noteUri == null) throw new Error('uri is null'); @@ -356,6 +384,9 @@ export class ApNoteService { const UpdatedNote = await this.notesRepository.findOneBy({ uri: noteUri }); if (UpdatedNote == null) throw new Error('Note is not registered'); + const user = await this.usersRepository.findOneBy({ id: UpdatedNote.userId }) as MiRemoteUser | null; + if (user == null) throw new Error('Note is not registered'); + // eslint-disable-next-line no-param-reassign if (resolver == null) resolver = this.apResolverService.createResolver(); @@ -372,6 +403,10 @@ export class ApNoteService { throw err; } + // `validateNote` checks that the actor and user are one and the same + // eslint-disable-next-line no-param-reassign + actor ??= user; + const note = object as IPost; this.logger.debug(`Note fetched: ${JSON.stringify(note, null, 2)}`); @@ -402,16 +437,7 @@ export class ApNoteService { this.logger.info(`Creating the Note: ${note.id}`); - // 投稿者をフェッチ - if (note.attributedTo == null) { - throw new Error('invalid note.attributedTo: ' + note.attributedTo); - } - - const uri = getOneApId(note.attributedTo); - - // ローカルで投稿者を検索し、もし凍結されていたらスキップ - const cachedActor = await this.apPersonService.fetchPerson(uri) as MiRemoteUser; - if (cachedActor && cachedActor.isSuspended) { + if (actor.isSuspended) { throw new IdentifiableError('85ab9bd7-3a41-4530-959d-f07073900109', 'actor has been suspended'); } @@ -443,13 +469,6 @@ export class ApNoteService { } //#endregion - const actor = cachedActor ?? await this.apPersonService.resolvePerson(uri, resolver) as MiRemoteUser; - - // 投稿者が凍結されていたらスキップ - if (actor.isSuspended) { - throw new IdentifiableError('85ab9bd7-3a41-4530-959d-f07073900109', 'actor has been suspended'); - } - const noteAudience = await this.apAudienceService.parseAudience(actor, note.to, note.cc, resolver); let visibility = noteAudience.visibility; const visibleUsers = noteAudience.visibleUsers; @@ -610,7 +629,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/server/api/endpoints/ap/show.ts b/packages/backend/src/server/api/endpoints/ap/show.ts index a877d1ce0d..4232bc6e39 100644 --- a/packages/backend/src/server/api/endpoints/ap/show.ts +++ b/packages/backend/src/server/api/endpoints/ap/show.ts @@ -140,7 +140,7 @@ export default class extends Endpoint { // eslint- return await this.mergePack( me, isActor(object) ? await this.apPersonService.createPerson(getApId(object)) : null, - isPost(object) ? await this.apNoteService.createNote(getApId(object), undefined, true) : null, + isPost(object) ? await this.apNoteService.createNote(getApId(object), undefined, undefined, true) : null, ); } -- cgit v1.2.3-freya From cbf8cc376e02e457a96d680dbbf0c110137d55f5 Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Thu, 14 Nov 2024 21:23:27 -0500 Subject: 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. --- packages/backend/src/server/api/endpoints/ap/get.ts | 1 + 1 file changed, 1 insertion(+) (limited to 'packages/backend/src/server') 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', -- cgit v1.2.3-freya From 776f6fd1f568fc36e1b6b84d9b8e56611be58c5d Mon Sep 17 00:00:00 2001 From: rectcoordsystem Date: Wed, 13 Nov 2024 15:27:17 +0900 Subject: fix(backend): allow fetchSummaryFromProxy, trueMail to access local addresses --- packages/backend/src/core/EmailService.ts | 1 + packages/backend/src/server/web/UrlPreviewService.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) (limited to 'packages/backend/src/server') diff --git a/packages/backend/src/core/EmailService.ts b/packages/backend/src/core/EmailService.ts index a176474b95..da198d0e42 100644 --- a/packages/backend/src/core/EmailService.ts +++ b/packages/backend/src/core/EmailService.ts @@ -312,6 +312,7 @@ export class EmailService { Accept: 'application/json', Authorization: truemailAuthKey, }, + isLocalAddressAllowed: true, }); const json = (await res.json()) as { diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index 981fbb4353..47cc09b067 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -170,6 +170,6 @@ export class UrlPreviewService { contentLengthRequired: meta.urlPreviewRequireContentLength, }); - return this.httpRequestService.getJson(`${proxy}?${queryStr}`); + return this.httpRequestService.getJson(`${proxy}?${queryStr}`, 'application/json, */*', undefined, true); } } -- cgit v1.2.3-freya