From 25e24b053a4eae357584ca596e4faa974d14138a Mon Sep 17 00:00:00 2001 From: tamaina Date: Tue, 28 May 2024 14:36:06 +0900 Subject: [PATCH 01/20] =?UTF-8?q?refactor(federation):=20Inbox=E3=81=AE?= =?UTF-8?q?=E3=82=A8=E3=83=A9=E3=83=BC=E3=83=8F=E3=83=B3=E3=83=89=E3=83=AA?= =?UTF-8?q?=E3=83=B3=E3=82=B0=E3=81=AE=E4=BB=95=E6=A7=98=E5=A4=89=E6=9B=B4?= =?UTF-8?q?=20(#13610)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry-picked from 89b27d8587221a321b6ff9cdae4b714bbedd151a Co-authored-by: tamaina --- .../src/core/activitypub/ApInboxService.ts | 127 ++++++++++-------- .../core/activitypub/models/ApNoteService.ts | 8 +- packages/backend/src/core/activitypub/type.ts | 1 + .../queue/processors/InboxProcessorService.ts | 7 +- 4 files changed, 84 insertions(+), 59 deletions(-) diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index fd9c7c2ba..f1283cc5e 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -38,7 +38,7 @@ import { ApAudienceService } from './ApAudienceService.js'; import { ApPersonService } from './models/ApPersonService.js'; import { ApQuestionService } from './models/ApQuestionService.js'; import type { Resolver } from './ApResolverService.js'; -import type { IAccept, IAdd, IAnnounce, IBlock, ICreate, IDelete, IFlag, IFollow, ILike, IObject, IReject, IRemove, IUndo, IUpdate, IMove } from './type.js'; +import type { IAccept, IAdd, IAnnounce, IBlock, ICreate, IDelete, IFlag, IFollow, ILike, IObject, IReject, IRemove, IUndo, IUpdate, IMove, IPost } from './type.js'; @Injectable() export class ApInboxService { @@ -90,13 +90,15 @@ export class ApInboxService { } @bindThis - public async performActivity(actor: MiRemoteUser, activity: IObject, additionalCc?: MiLocalUser['id']): Promise { + public async performActivity(actor: MiRemoteUser, activity: IObject, additionalCc?: MiLocalUser['id']): Promise { + let result = 'error'; 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)) { const act = await resolver.resolve(item); try { - await this.performOneActivity(actor, act, additionalCc); + results.push([getApId(item), await this.performOneActivity(actor, act, additionalCc)]); } catch (err) { if (err instanceof Error || typeof err === 'string') { this.logger.error(err); @@ -105,8 +107,13 @@ export class ApInboxService { } } } + + const hasReason = results.some(([, reason]) => (reason != null && !reason.startsWith('ok'))); + if (hasReason) { + result = results.map(([id, reason]) => `${id}: ${reason}`).join('\n'); + } } else { - await this.performOneActivity(actor, activity, additionalCc); + result = await this.performOneActivity(actor, activity, additionalCc); } // ついでにリモートユーザーの情報が古かったら更新しておく @@ -117,42 +124,44 @@ export class ApInboxService { }); } } + + return result; } @bindThis - public async performOneActivity(actor: MiRemoteUser, activity: IObject, additionalCc?: MiLocalUser['id']): Promise { - if (actor.isSuspended) return; + public async performOneActivity(actor: MiRemoteUser, activity: IObject, additionalCc?: MiLocalUser['id']): Promise { + if (actor.isSuspended) return 'skip: actor is suspended'; if (isCreate(activity)) { - await this.create(actor, activity, additionalCc); + return await this.create(actor, activity, additionalCc); } else if (isDelete(activity)) { - await this.delete(actor, activity); + return await this.delete(actor, activity); } else if (isUpdate(activity)) { - await this.update(actor, activity, additionalCc); + return await this.update(actor, activity, additionalCc); } else if (isFollow(activity)) { - await this.follow(actor, activity); + return await this.follow(actor, activity); } else if (isAccept(activity)) { - await this.accept(actor, activity); + return await this.accept(actor, activity); } else if (isReject(activity)) { - await this.reject(actor, activity); + return await this.reject(actor, activity); } else if (isAdd(activity)) { - await this.add(actor, activity).catch(err => this.logger.error(err)); + return await this.add(actor, activity).catch(err => { this.logger.error(err); return `error: ${err.message}`; }); } else if (isRemove(activity)) { - await this.remove(actor, activity).catch(err => this.logger.error(err)); + return await this.remove(actor, activity).catch(err => { this.logger.error(err); return `error: ${err.message}`; }); } else if (isAnnounce(activity)) { - await this.announce(actor, activity); + return await this.announce(actor, activity); } else if (isLike(activity)) { - await this.like(actor, activity); + return await this.like(actor, activity); } else if (isUndo(activity)) { - await this.undo(actor, activity); + return await this.undo(actor, activity); } else if (isBlock(activity)) { - await this.block(actor, activity); + return await this.block(actor, activity); } else if (isFlag(activity)) { - await this.flag(actor, activity); + return await this.flag(actor, activity); } else if (isMove(activity)) { - await this.move(actor, activity); + return await this.move(actor, activity); } else { - this.logger.warn(`unrecognized activity type: ${activity.type}`); + return `skip: unknown activity type ${activity.type}`; } } @@ -234,47 +243,56 @@ export class ApInboxService { } @bindThis - private async add(actor: MiRemoteUser, activity: IAdd): Promise { + private async add(actor: MiRemoteUser, activity: IAdd): Promise { if (actor.uri !== activity.actor) { - throw new Error('invalid actor'); + return 'skip: invalid actor'; } if (activity.target == null) { - throw new Error('target is null'); + return 'skip: target is null'; } if (activity.target === actor.featured) { const note = await this.apNoteService.resolveNote(activity.object); - if (note == null) throw new Error('note not found'); + if (note == null) return 'skip: note not found'; await this.notePiningService.addPinned(actor, note.id); - return; + return 'ok'; } - throw new Error(`unknown target: ${activity.target}`); + return `skip: unknown target ${activity.target}`; } @bindThis - private async announce(actor: MiRemoteUser, activity: IAnnounce): Promise { + private async announce(actor: MiRemoteUser, activity: IAnnounce): Promise { const uri = getApId(activity); this.logger.info(`Announce: ${uri}`); - const targetUri = getApId(activity.object); + const resolver = this.apResolverService.createResolver(); - await this.announceNote(actor, activity, targetUri); + if (!activity.object) return 'skip: activity has no object property'; + + const target = await resolver.resolve(activity.object).catch(err => { + this.logger.error(`Resolution failed: ${err}`, { error: err }); + return err; + }); + + if (isPost(target)) await this.announceNote(actor, activity, target); + + return `skip: unknown object type ${getApType(target)}`; } @bindThis - private async announceNote(actor: MiRemoteUser, activity: IAnnounce, targetUri: string): Promise { + private async announceNote(actor: MiRemoteUser, activity: IAnnounce, target: IPost): Promise { const uri = getApId(activity); if (actor.isSuspended) { - return; + return 'skip: actor is suspended'; } // アナウンス先をブロックしてたら中断 const meta = await this.metaService.fetch(); - if (this.utilityService.isBlockedHost(meta.blockedHosts, this.utilityService.extractDbHost(uri))) return; + if (this.utilityService.isBlockedHost(meta.blockedHosts, this.utilityService.extractDbHost(uri))) return 'skip: blocked host'; const unlock = await this.appLockService.getApLock(uri); @@ -282,30 +300,28 @@ export class ApInboxService { // 既に同じURIを持つものが登録されていないかチェック const exist = await this.apNoteService.fetchNote(uri); if (exist) { - return; + return 'skip: note exists'; } // Announce対象をresolve let renote; try { - renote = await this.apNoteService.resolveNote(targetUri); - if (renote == null) throw new Error('announce target is null'); + renote = await this.apNoteService.resolveNote(target); + if (renote == null) return 'skip: target note not found'; } catch (err) { // 対象が4xxならスキップ if (err instanceof StatusError) { if (!err.isRetryable) { - this.logger.warn(`Ignored announce target ${targetUri} - ${err.statusCode}`); - return; + return `skip: Ignored announce target ${target} - ${err.statusCode}`; } - this.logger.warn(`Error in announce target ${targetUri} - ${err.statusCode}`); + this.logger.warn(`Error in announce target ${target} - ${err.statusCode}`); } throw err; } if (!await this.noteEntityService.isVisibleForMe(renote, actor.id)) { - this.logger.warn('skip: invalid actor for this activity'); - return; + return 'skip: invalid actor for this activity'; } this.logger.info(`Creating the (Re)Note: ${uri}`); @@ -314,8 +330,7 @@ export class ApInboxService { const createdAt = activity.published ? new Date(activity.published) : null; if (createdAt && createdAt < this.idService.parse(renote.id).date) { - this.logger.warn('skip: malformed createdAt'); - return; + return 'skip: malformed createdAt'; } await this.noteCreateService.create(actor, { @@ -328,6 +343,8 @@ export class ApInboxService { } finally { unlock(); } + + return 'ok'; } @bindThis @@ -349,11 +366,13 @@ export class ApInboxService { } @bindThis - private async create(actor: MiRemoteUser, activity: ICreate, additionalCc?: MiLocalUser['id']): Promise { + private async create(actor: MiRemoteUser, activity: ICreate, additionalCc?: MiLocalUser['id']): Promise { const uri = getApId(activity); this.logger.info(`Create: ${uri}`); + if (!activity.object) return 'skip: activity has no object property'; + // copy audiences between activity <=> object. if (typeof activity.object === 'object') { const to = unique(concat([toArray(activity.to), toArray(activity.object.to)])); @@ -378,9 +397,9 @@ export class ApInboxService { }); if (isPost(object)) { - await this.createNote(resolver, actor, object, false, activity, additionalCc); + return await this.createNote(resolver, actor, object, false, activity, additionalCc); } else { - this.logger.warn(`Unknown type: ${getApType(object)}`); + return `skip: Unknown type ${getApType(object)}`; } } @@ -433,7 +452,7 @@ export class ApInboxService { @bindThis private async delete(actor: MiRemoteUser, activity: IDelete): Promise { if (actor.uri !== activity.actor) { - throw new Error('invalid actor'); + return 'skip: invalid actor'; } // 削除対象objectのtype @@ -595,29 +614,29 @@ export class ApInboxService { } @bindThis - private async remove(actor: MiRemoteUser, activity: IRemove): Promise { + private async remove(actor: MiRemoteUser, activity: IRemove): Promise { if (actor.uri !== activity.actor) { - throw new Error('invalid actor'); + return 'skip: invalid actor'; } if (activity.target == null) { - throw new Error('target is null'); + return 'skip: target is null'; } if (activity.target === actor.featured) { const note = await this.apNoteService.resolveNote(activity.object); - if (note == null) throw new Error('note not found'); + if (note == null) return 'skip: note not found'; await this.notePiningService.removePinned(actor, note.id); - return; + return 'ok'; } - throw new Error(`unknown target: ${activity.target}`); + return `skip: unknown target ${activity.target}`; } @bindThis private async undo(actor: MiRemoteUser, activity: IUndo): Promise { if (actor.uri !== activity.actor) { - throw new Error('invalid actor'); + return 'skip: invalid actor'; } const uri = activity.id ?? activity; diff --git a/packages/backend/src/core/activitypub/models/ApNoteService.ts b/packages/backend/src/core/activitypub/models/ApNoteService.ts index 3d72c502e..fefd53f72 100644 --- a/packages/backend/src/core/activitypub/models/ApNoteService.ts +++ b/packages/backend/src/core/activitypub/models/ApNoteService.ts @@ -84,20 +84,20 @@ export class ApNoteService { const expectHost = this.utilityService.extractDbHost(uri); if (!validPost.includes(getApType(object))) { - return new Error(`invalid Note: invalid object type ${getApType(object)}`); + return new IdentifiableError('d450b8a9-48e4-4dab-ae36-f4db763fda7c', `invalid Note: invalid object type ${getApType(object)}`); } if (object.id && this.utilityService.extractDbHost(object.id) !== expectHost) { - return new Error(`invalid Note: id has different host. expected: ${expectHost}, actual: ${this.utilityService.extractDbHost(object.id)}`); + return new IdentifiableError('d450b8a9-48e4-4dab-ae36-f4db763fda7c', `invalid Note: id has different host. expected: ${expectHost}, actual: ${this.utilityService.extractDbHost(object.id)}`); } const actualHost = object.attributedTo && this.utilityService.extractDbHost(getOneApId(object.attributedTo)); if (object.attributedTo && actualHost !== expectHost) { - return new Error(`invalid Note: attributedTo has different host. expected: ${expectHost}, actual: ${actualHost}`); + return new IdentifiableError('d450b8a9-48e4-4dab-ae36-f4db763fda7c', `invalid Note: attributedTo has different host. expected: ${expectHost}, actual: ${actualHost}`); } if (object.published && !this.idService.isSafeT(new Date(object.published).valueOf())) { - return new Error('invalid Note: published timestamp is malformed'); + return new IdentifiableError('d450b8a9-48e4-4dab-ae36-f4db763fda7c', 'invalid Note: published timestamp is malformed'); } return null; diff --git a/packages/backend/src/core/activitypub/type.ts b/packages/backend/src/core/activitypub/type.ts index 09322888d..5b6c6c8ca 100644 --- a/packages/backend/src/core/activitypub/type.ts +++ b/packages/backend/src/core/activitypub/type.ts @@ -328,3 +328,4 @@ export const isAnnounce = (object: IObject): object is IAnnounce => getApType(ob export const isBlock = (object: IObject): object is IBlock => getApType(object) === 'Block'; export const isFlag = (object: IObject): object is IFlag => getApType(object) === 'Flag'; export const isMove = (object: IObject): object is IMove => getApType(object) === 'Move'; +export const isNote = (object: IObject): object is IPost => getApType(object) === 'Note'; diff --git a/packages/backend/src/queue/processors/InboxProcessorService.ts b/packages/backend/src/queue/processors/InboxProcessorService.ts index 259fd06f6..0bb30d27f 100644 --- a/packages/backend/src/queue/processors/InboxProcessorService.ts +++ b/packages/backend/src/queue/processors/InboxProcessorService.ts @@ -198,7 +198,11 @@ export class InboxProcessorService implements OnApplicationShutdown { // アクティビティを処理 try { - await this.apInboxService.performActivity(authUser.user, activity, job.data.user?.id); + const result = await this.apInboxService.performActivity(authUser.user, activity, job.data.user?.id); + if (result && !result.startsWith('ok')) { + this.logger.warn(`inbox activity ignored (maybe): id=${activity.id} reason=${result}`); + return result; + } } catch (e) { if (e instanceof IdentifiableError) { if ([ @@ -206,6 +210,7 @@ export class InboxProcessorService implements OnApplicationShutdown { '689ee33f-f97c-479a-ac49-1b9f8140af99', '9f466dab-c856-48cd-9e65-ff90ff750580', '85ab9bd7-3a41-4530-959d-f07073900109', + 'd450b8a9-48e4-4dab-ae36-f4db763fda7c', ].includes(e.id)) return e.message; } throw e; From 21e3b043916d972ff3f4c296b69704a99f768f69 Mon Sep 17 00:00:00 2001 From: Daiki Mizukami Date: Fri, 9 Aug 2024 12:10:51 +0900 Subject: [PATCH 02/20] fix(backend): check visibility of following/followers of remote users / feat: moderators can see following/followers of all users (#14375) Cherry-picked from 0d508db8a7a36218d38231af4e718aff0e94d9bc Co-authored-by: Daiki Mizukami --- .../activitypub/models/ApPersonService.ts | 50 ++++++++++++++++- packages/backend/src/core/activitypub/type.ts | 6 ++- .../src/core/entities/UserEntityService.ts | 4 +- .../server/api/endpoints/users/followers.ts | 34 ++++++------ .../server/api/endpoints/users/following.ts | 34 ++++++------ packages/backend/test/unit/activitypub.ts | 53 ++++++++++++++++++- .../frontend/src/scripts/isFfVisibleForMe.ts | 4 +- 7 files changed, 147 insertions(+), 38 deletions(-) diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index 430ec714a..1e9b9a4c0 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -49,7 +49,7 @@ import type { ApResolverService, Resolver } from '../ApResolverService.js'; import type { ApLoggerService } from '../ApLoggerService.js'; // eslint-disable-next-line @typescript-eslint/consistent-type-imports import type { ApImageService } from './ApImageService.js'; -import type { IActor, IObject } from '../type.js'; +import type { IActor, ICollection, IObject, IOrderedCollection } from '../type.js'; const nameLength = 128; const summaryLength = 2048; @@ -307,6 +307,21 @@ export class ApPersonService implements OnModuleInit { const isBot = getApType(object) === 'Service' || getApType(object) === 'Application'; + const [followingVisibility, followersVisibility] = await Promise.all( + [ + this.isPublicCollection(person.following, resolver), + this.isPublicCollection(person.followers, resolver), + ].map((p): Promise<'public' | 'private'> => p + .then(isPublic => isPublic ? 'public' : 'private') + .catch(err => { + if (!(err instanceof StatusError) || err.isRetryable) { + this.logger.error('error occurred while fetching following/followers collection', { error: err }); + } + return 'private'; + }) + ) + ); + const bday = person['vcard:bday']?.match(/^\d{4}-\d{2}-\d{2}/); const url = getOneApHrefNullable(person.url); @@ -368,6 +383,8 @@ export class ApPersonService implements OnModuleInit { description: _description, url, fields, + followingVisibility, + followersVisibility, birthday: bday?.[0] ?? null, location: person['vcard:Address'] ?? null, userHost: host, @@ -475,6 +492,23 @@ export class ApPersonService implements OnModuleInit { const tags = extractApHashtags(person.tag).map(normalizeForSearch).splice(0, 32); + const [followingVisibility, followersVisibility] = await Promise.all( + [ + this.isPublicCollection(person.following, resolver), + this.isPublicCollection(person.followers, resolver), + ].map((p): Promise<'public' | 'private' | undefined> => p + .then(isPublic => isPublic ? 'public' : 'private') + .catch(err => { + if (!(err instanceof StatusError) || err.isRetryable) { + this.logger.error('error occurred while fetching following/followers collection', { error: err }); + // Do not update the visibiility on transient errors. + return undefined; + } + return 'private'; + }) + ) + ); + const bday = person['vcard:bday']?.match(/^\d{4}-\d{2}-\d{2}/); const url = getOneApHrefNullable(person.url); @@ -545,6 +579,8 @@ export class ApPersonService implements OnModuleInit { url, fields, description: _description, + followingVisibility, + followersVisibility, birthday: bday?.[0] ?? null, location: person['vcard:Address'] ?? null, }); @@ -715,4 +751,16 @@ export class ApPersonService implements OnModuleInit { return 'ok'; } + + @bindThis + private async isPublicCollection(collection: string | ICollection | IOrderedCollection | undefined, resolver: Resolver): Promise { + if (collection) { + const resolved = await resolver.resolveCollection(collection); + if (resolved.first || (resolved as ICollection).items || (resolved as IOrderedCollection).orderedItems) { + return true; + } + } + + return false; + } } diff --git a/packages/backend/src/core/activitypub/type.ts b/packages/backend/src/core/activitypub/type.ts index 5b6c6c8ca..131c518c0 100644 --- a/packages/backend/src/core/activitypub/type.ts +++ b/packages/backend/src/core/activitypub/type.ts @@ -97,13 +97,15 @@ export interface IActivity extends IObject { export interface ICollection extends IObject { type: 'Collection'; totalItems: number; - items: ApObject; + first?: IObject | string; + items?: ApObject; } export interface IOrderedCollection extends IObject { type: 'OrderedCollection'; totalItems: number; - orderedItems: ApObject; + first?: IObject | string; + orderedItems?: ApObject; } export const validPost = ['Note', 'Question', 'Article', 'Audio', 'Document', 'Image', 'Page', 'Video', 'Event']; diff --git a/packages/backend/src/core/entities/UserEntityService.ts b/packages/backend/src/core/entities/UserEntityService.ts index 66ae69ed9..e9bf5a397 100644 --- a/packages/backend/src/core/entities/UserEntityService.ts +++ b/packages/backend/src/core/entities/UserEntityService.ts @@ -456,12 +456,12 @@ export class UserEntityService implements OnModuleInit { } const followingCount = profile == null ? null : - (profile.followingVisibility === 'public') || isMe ? user.followingCount : + (profile.followingVisibility === 'public') || isMe || iAmModerator ? user.followingCount : (profile.followingVisibility === 'followers') && (relation && relation.isFollowing) ? user.followingCount : null; const followersCount = profile == null ? null : - (profile.followersVisibility === 'public') || isMe ? user.followersCount : + (profile.followersVisibility === 'public') || isMe || iAmModerator ? user.followersCount : (profile.followersVisibility === 'followers') && (relation && relation.isFollowing) ? user.followersCount : null; diff --git a/packages/backend/src/server/api/endpoints/users/followers.ts b/packages/backend/src/server/api/endpoints/users/followers.ts index 7ce7734f5..a8b4319a6 100644 --- a/packages/backend/src/server/api/endpoints/users/followers.ts +++ b/packages/backend/src/server/api/endpoints/users/followers.ts @@ -11,6 +11,7 @@ import { QueryService } from '@/core/QueryService.js'; import { FollowingEntityService } from '@/core/entities/FollowingEntityService.js'; import { UtilityService } from '@/core/UtilityService.js'; import { DI } from '@/di-symbols.js'; +import { RoleService } from '@/core/RoleService.js'; import { ApiError } from '../../error.js'; export const meta = { @@ -81,6 +82,7 @@ export default class extends Endpoint { // eslint- private utilityService: UtilityService, private followingEntityService: FollowingEntityService, private queryService: QueryService, + private roleService: RoleService, ) { super(meta, paramDef, async (ps, me) => { const user = await this.usersRepository.findOneBy(ps.userId != null @@ -93,23 +95,25 @@ export default class extends Endpoint { // eslint- const profile = await this.userProfilesRepository.findOneByOrFail({ userId: user.id }); - if (profile.followersVisibility === 'private') { - if (me == null || (me.id !== user.id)) { - throw new ApiError(meta.errors.forbidden); - } - } else if (profile.followersVisibility === 'followers') { - if (me == null) { - throw new ApiError(meta.errors.forbidden); - } else if (me.id !== user.id) { - const isFollowing = await this.followingsRepository.exists({ - where: { - followeeId: user.id, - followerId: me.id, - }, - }); - if (!isFollowing) { + if (profile.followersVisibility !== 'public' && !await this.roleService.isModerator(me)) { + if (profile.followersVisibility === 'private') { + if (me == null || (me.id !== user.id)) { throw new ApiError(meta.errors.forbidden); } + } else if (profile.followersVisibility === 'followers') { + if (me == null) { + throw new ApiError(meta.errors.forbidden); + } else if (me.id !== user.id) { + const isFollowing = await this.followingsRepository.exists({ + where: { + followeeId: user.id, + followerId: me.id, + }, + }); + if (!isFollowing) { + throw new ApiError(meta.errors.forbidden); + } + } } } diff --git a/packages/backend/src/server/api/endpoints/users/following.ts b/packages/backend/src/server/api/endpoints/users/following.ts index 084d27809..ee2e17a0e 100644 --- a/packages/backend/src/server/api/endpoints/users/following.ts +++ b/packages/backend/src/server/api/endpoints/users/following.ts @@ -12,6 +12,7 @@ import { QueryService } from '@/core/QueryService.js'; import { FollowingEntityService } from '@/core/entities/FollowingEntityService.js'; import { UtilityService } from '@/core/UtilityService.js'; import { DI } from '@/di-symbols.js'; +import { RoleService } from '@/core/RoleService.js'; import { ApiError } from '../../error.js'; export const meta = { @@ -93,6 +94,7 @@ export default class extends Endpoint { // eslint- private utilityService: UtilityService, private followingEntityService: FollowingEntityService, private queryService: QueryService, + private roleService: RoleService, ) { super(meta, paramDef, async (ps, me) => { const user = await this.usersRepository.findOneBy(ps.userId != null @@ -105,23 +107,25 @@ export default class extends Endpoint { // eslint- const profile = await this.userProfilesRepository.findOneByOrFail({ userId: user.id }); - if (profile.followingVisibility === 'private') { - if (me == null || (me.id !== user.id)) { - throw new ApiError(meta.errors.forbidden); - } - } else if (profile.followingVisibility === 'followers') { - if (me == null) { - throw new ApiError(meta.errors.forbidden); - } else if (me.id !== user.id) { - const isFollowing = await this.followingsRepository.exists({ - where: { - followeeId: user.id, - followerId: me.id, - }, - }); - if (!isFollowing) { + if (profile.followingVisibility !== 'public' && !await this.roleService.isModerator(me)) { + if (profile.followingVisibility === 'private') { + if (me == null || (me.id !== user.id)) { throw new ApiError(meta.errors.forbidden); } + } else if (profile.followingVisibility === 'followers') { + if (me == null) { + throw new ApiError(meta.errors.forbidden); + } else if (me.id !== user.id) { + const isFollowing = await this.followingsRepository.exists({ + where: { + followeeId: user.id, + followerId: me.id, + }, + }); + if (!isFollowing) { + throw new ApiError(meta.errors.forbidden); + } + } } } diff --git a/packages/backend/test/unit/activitypub.ts b/packages/backend/test/unit/activitypub.ts index 7f5e06b52..e32096ef5 100644 --- a/packages/backend/test/unit/activitypub.ts +++ b/packages/backend/test/unit/activitypub.ts @@ -20,7 +20,8 @@ import { CoreModule } from '@/core/CoreModule.js'; import { FederatedInstanceService } from '@/core/FederatedInstanceService.js'; import { LoggerService } from '@/core/LoggerService.js'; import type { IActor, IApDocument, ICollection, IObject, IPost } from '@/core/activitypub/type.js'; -import { MiMeta, MiNote } from '@/models/_.js'; +import { MiMeta, MiNote, UserProfilesRepository } from '@/models/_.js'; +import { DI } from '@/di-symbols.js'; import { secureRndstr } from '@/misc/secure-rndstr.js'; import { DownloadService } from '@/core/DownloadService.js'; import { MetaService } from '@/core/MetaService.js'; @@ -86,6 +87,7 @@ async function createRandomRemoteUser( } describe('ActivityPub', () => { + let userProfilesRepository: UserProfilesRepository; let imageService: ApImageService; let noteService: ApNoteService; let personService: ApPersonService; @@ -127,6 +129,8 @@ describe('ActivityPub', () => { await app.init(); app.enableShutdownHooks(); + userProfilesRepository = app.get(DI.userProfilesRepository); + noteService = app.get(ApNoteService); personService = app.get(ApPersonService); rendererService = app.get(ApRendererService); @@ -205,6 +209,53 @@ describe('ActivityPub', () => { }); }); + describe('Collection visibility', () => { + test('Public following/followers', async () => { + const actor = createRandomActor(); + actor.following = { + id: `${actor.id}/following`, + type: 'OrderedCollection', + totalItems: 0, + first: `${actor.id}/following?page=1`, + }; + actor.followers = `${actor.id}/followers`; + + resolver.register(actor.id, actor); + resolver.register(actor.followers, { + id: actor.followers, + type: 'OrderedCollection', + totalItems: 0, + first: `${actor.followers}?page=1`, + }); + + const user = await personService.createPerson(actor.id, resolver); + const userProfile = await userProfilesRepository.findOneByOrFail({ userId: user.id }); + + assert.deepStrictEqual(userProfile.followingVisibility, 'public'); + assert.deepStrictEqual(userProfile.followersVisibility, 'public'); + }); + + test('Private following/followers', async () => { + const actor = createRandomActor(); + actor.following = { + id: `${actor.id}/following`, + type: 'OrderedCollection', + totalItems: 0, + // first: … + }; + actor.followers = `${actor.id}/followers`; + + resolver.register(actor.id, actor); + //resolver.register(actor.followers, { … }); + + const user = await personService.createPerson(actor.id, resolver); + const userProfile = await userProfilesRepository.findOneByOrFail({ userId: user.id }); + + assert.deepStrictEqual(userProfile.followingVisibility, 'private'); + assert.deepStrictEqual(userProfile.followersVisibility, 'private'); + }); + }); + describe('Renderer', () => { test('Render an announce with visibility: followers', () => { rendererService.renderAnnounce('https://example.com/notes/00example', { diff --git a/packages/frontend/src/scripts/isFfVisibleForMe.ts b/packages/frontend/src/scripts/isFfVisibleForMe.ts index 406404c46..e28e5725b 100644 --- a/packages/frontend/src/scripts/isFfVisibleForMe.ts +++ b/packages/frontend/src/scripts/isFfVisibleForMe.ts @@ -7,7 +7,7 @@ import * as Misskey from 'misskey-js'; import { $i } from '@/account.js'; export function isFollowingVisibleForMe(user: Misskey.entities.UserDetailed): boolean { - if ($i && $i.id === user.id) return true; + if ($i && ($i.id === user.id || $i.isAdmin || $i.isModerator)) return true; if (user.followingVisibility === 'private') return false; if (user.followingVisibility === 'followers' && !user.isFollowing) return false; @@ -15,7 +15,7 @@ export function isFollowingVisibleForMe(user: Misskey.entities.UserDetailed): bo return true; } export function isFollowersVisibleForMe(user: Misskey.entities.UserDetailed): boolean { - if ($i && $i.id === user.id) return true; + if ($i && ($i.id === user.id || $i.isAdmin || $i.isModerator)) return true; if (user.followersVisibility === 'private') return false; if (user.followersVisibility === 'followers' && !user.isFollowing) return false; From c909c00920fc53b907818fd54b08c034feac0423 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=8B=E3=81=A3=E3=81=93=E3=81=8B=E3=82=8A?= <67428053+kakkokari-gtyih@users.noreply.github.com> Date: Sun, 11 Aug 2024 16:25:57 +0900 Subject: [PATCH 03/20] =?UTF-8?q?fix(backend):=20getApType=E3=81=A7?= =?UTF-8?q?=E3=82=A8=E3=83=A9=E3=83=BC=E3=82=92=E6=8A=95=E3=81=92=E3=81=AA?= =?UTF-8?q?=E3=81=84=E3=82=88=E3=81=86=E3=81=AB=20(#14361)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cherry picked from commit 93fc06d18b8520919cdf422675c4102b4851df18 Co-authored-by: かっこかり <67428053+kakkokari-gtyih@users.noreply.github.com> --- .../src/core/activitypub/ApInboxService.ts | 12 +++---- .../core/activitypub/models/ApNoteService.ts | 5 +-- packages/backend/src/core/activitypub/type.ts | 32 +++++++++++++------ 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index f1283cc5e..0b6de71ea 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -215,7 +215,7 @@ export class ApInboxService { if (isFollow(object)) return await this.acceptFollow(actor, object); - return `skip: Unknown Accept type: ${getApType(object)}`; + return `skip: Unknown Accept type: ${getApType(object) ?? 'undefined'}}`; } @bindThis @@ -279,7 +279,7 @@ export class ApInboxService { if (isPost(target)) await this.announceNote(actor, activity, target); - return `skip: unknown object type ${getApType(target)}`; + return `skip: unknown object type ${getApType(target) ?? 'undefined'}}`; } @bindThis @@ -399,7 +399,7 @@ export class ApInboxService { if (isPost(object)) { return await this.createNote(resolver, actor, object, false, activity, additionalCc); } else { - return `skip: Unknown type ${getApType(object)}`; + return `skip: Unknown type ${getApType(object) ?? 'undefined'}}`; } } @@ -586,7 +586,7 @@ export class ApInboxService { if (isFollow(object)) return await this.rejectFollow(actor, object); - return `skip: Unknown Reject type: ${getApType(object)}`; + return `skip: Unknown Reject type: ${getApType(object) ?? 'undefined'}}`; } @bindThis @@ -657,7 +657,7 @@ export class ApInboxService { if (isAnnounce(object)) return await this.undoAnnounce(actor, object); if (isAccept(object)) return await this.undoAccept(actor, object); - return `skip: unknown object type ${getApType(object)}`; + return `skip: unknown object type ${getApType(object) ?? 'undefined'}}`; } @bindThis @@ -809,7 +809,7 @@ export class ApInboxService { unlock(); } } else { - return `skip: Unknown type: ${getApType(object)}`; + return `skip: Unknown type: ${getApType(object) ?? 'undefined'}}`; } } diff --git a/packages/backend/src/core/activitypub/models/ApNoteService.ts b/packages/backend/src/core/activitypub/models/ApNoteService.ts index fefd53f72..60a14a4e7 100644 --- a/packages/backend/src/core/activitypub/models/ApNoteService.ts +++ b/packages/backend/src/core/activitypub/models/ApNoteService.ts @@ -82,9 +82,10 @@ export class ApNoteService { @bindThis public validateNote(object: IObject, uri: string): Error | null { const expectHost = this.utilityService.extractDbHost(uri); + const apType = getApType(object); - if (!validPost.includes(getApType(object))) { - return new IdentifiableError('d450b8a9-48e4-4dab-ae36-f4db763fda7c', `invalid Note: invalid object type ${getApType(object)}`); + if (apType == null || !validPost.includes(apType)) { + return new IdentifiableError('d450b8a9-48e4-4dab-ae36-f4db763fda7c', `invalid Note: invalid object type ${apType ?? 'undefined'}`); } if (object.id && this.utilityService.extractDbHost(object.id) !== expectHost) { diff --git a/packages/backend/src/core/activitypub/type.ts b/packages/backend/src/core/activitypub/type.ts index 131c518c0..16812b7a4 100644 --- a/packages/backend/src/core/activitypub/type.ts +++ b/packages/backend/src/core/activitypub/type.ts @@ -60,11 +60,14 @@ export function getApId(value: string | IObject): string { /** * Get ActivityStreams Object type + * + * タイプ判定ができなかった場合に、あえてエラーではなくnullを返すようにしている。 + * 詳細: https://github.com/misskey-dev/misskey/issues/14239 */ -export function getApType(value: IObject): string { +export function getApType(value: IObject): string | null { if (typeof value.type === 'string') return value.type; if (Array.isArray(value.type) && typeof value.type[0] === 'string') return value.type[0]; - throw new Error('cannot detect type'); + return null; } export function getOneApHrefNullable(value: ApObject | undefined): string | undefined { @@ -110,8 +113,10 @@ export interface IOrderedCollection extends IObject { export const validPost = ['Note', 'Question', 'Article', 'Audio', 'Document', 'Image', 'Page', 'Video', 'Event']; -export const isPost = (object: IObject): object is IPost => - validPost.includes(getApType(object)); +export const isPost = (object: IObject): object is IPost => { + const type = getApType(object); + return type != null && validPost.includes(type); +}; export interface IPost extends IObject { type: 'Note' | 'Question' | 'Article' | 'Audio' | 'Document' | 'Image' | 'Page' | 'Video' | 'Event'; @@ -158,8 +163,10 @@ export const isTombstone = (object: IObject): object is ITombstone => export const validActor = ['Person', 'Service', 'Group', 'Organization', 'Application']; -export const isActor = (object: IObject): object is IActor => - validActor.includes(getApType(object)); +export const isActor = (object: IObject): object is IActor => { + const type = getApType(object); + return type != null && validActor.includes(type); +}; export interface IActor extends IObject { type: 'Person' | 'Service' | 'Organization' | 'Group' | 'Application'; @@ -242,12 +249,16 @@ export interface IKey extends IObject { publicKeyPem: string | Buffer; } +export const validDocumentTypes = ['Audio', 'Document', 'Image', 'Page', 'Video']; + export interface IApDocument extends IObject { type: 'Audio' | 'Document' | 'Image' | 'Page' | 'Video'; } -export const isDocument = (object: IObject): object is IApDocument => - ['Audio', 'Document', 'Image', 'Page', 'Video'].includes(getApType(object)); +export const isDocument = (object: IObject): object is IApDocument => { + const type = getApType(object); + return type != null && validDocumentTypes.includes(type); +}; export interface IApImage extends IApDocument { type: 'Image'; @@ -325,7 +336,10 @@ export const isAccept = (object: IObject): object is IAccept => getApType(object export const isReject = (object: IObject): object is IReject => getApType(object) === 'Reject'; export const isAdd = (object: IObject): object is IAdd => getApType(object) === 'Add'; export const isRemove = (object: IObject): object is IRemove => getApType(object) === 'Remove'; -export const isLike = (object: IObject): object is ILike => getApType(object) === 'Like' || getApType(object) === 'EmojiReaction' || getApType(object) === 'EmojiReact'; +export const isLike = (object: IObject): object is ILike => { + const type = getApType(object); + return type != null && ['Like', 'EmojiReaction', 'EmojiReact'].includes(type); +}; export const isAnnounce = (object: IObject): object is IAnnounce => getApType(object) === 'Announce'; export const isBlock = (object: IObject): object is IBlock => getApType(object) === 'Block'; export const isFlag = (object: IObject): object is IFlag => getApType(object) === 'Flag'; From edf94b54520a7a793fe13953635447487bb97821 Mon Sep 17 00:00:00 2001 From: taichan <40626578+tai-cha@users.noreply.github.com> Date: Sat, 17 Aug 2024 15:12:23 +0900 Subject: [PATCH 04/20] =?UTF-8?q?enhance(backend):=20head=E3=82=BF?= =?UTF-8?q?=E3=82=B0=E5=86=85=E3=81=ABrel=3Dalternate=E3=81=AE=E6=8C=87?= =?UTF-8?q?=E5=AE=9A=E3=81=AE=E3=81=82=E3=82=8Blink=E3=82=BF=E3=82=B0?= =?UTF-8?q?=E3=81=8C=E3=81=82=E3=82=8B=E5=A0=B4=E5=90=88=E3=80=81=E8=A8=98?= =?UTF-8?q?=E8=BF=B0=E3=81=95=E3=82=8C=E3=81=9FURL=E3=82=92=E5=8F=82?= =?UTF-8?q?=E7=85=A7=E3=81=97=E3=81=A6=E7=85=A7=E4=BC=9A=E3=81=A7=E3=81=8D?= =?UTF-8?q?=E3=82=8B=E3=82=88=E3=81=86=E3=81=AB=20(#14371)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cherry picked from commit 9fbc1b7f7b71cf0eafadd728a6b66cb95a0c35d8 Co-authored-by: syuilo <4439005+syuilo@users.noreply.github.com> Co-authored-by: taichan <40626578+tai-cha@users.noreply.github.com> --- .../src/core/activitypub/ApRequestService.ts | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts index b008a3ec5..b553708a8 100644 --- a/packages/backend/src/core/activitypub/ApRequestService.ts +++ b/packages/backend/src/core/activitypub/ApRequestService.ts @@ -6,6 +6,7 @@ import * as crypto from 'node:crypto'; import { URL } from 'node:url'; import { Inject, Injectable } from '@nestjs/common'; +import { JSDOM } from 'jsdom'; import { DI } from '@/di-symbols.js'; import type { Config } from '@/config.js'; import type { MiUser } from '@/models/User.js'; @@ -179,7 +180,8 @@ export class ApRequestService { * @param url URL to fetch */ @bindThis - public async signedGet(url: string, user: { id: MiUser['id'] }): Promise { + public async signedGet(url: string, user: { id: MiUser['id'] }, followAlternate?: boolean): Promise { + const _followAlternate = followAlternate ?? true; const keypair = await this.userKeypairService.getUserKeypair(user.id); const req = ApRequestCreator.createSignedGet({ @@ -197,9 +199,25 @@ export class ApRequestService { headers: req.request.headers, }, { throwErrorWhenResponseNotOk: true, - validators: [validateContentTypeSetAsActivityPub], }); + //#region リクエスト先がhtmlかつactivity+jsonへのalternate linkタグがあるとき + if (res.headers.get('Content-type')?.startsWith('text/html;') && _followAlternate) { + const html = await res.text(); + const fragment = JSDOM.fragment(html); + + const alternate = fragment.querySelector('head > link[rel="alternate"][type="application/activity+json"]'); + if (alternate) { + const href = alternate.getAttribute('href'); + if (href) { + return await this.signedGet(href, user, false); + } + } + } + //#endregion + + validateContentTypeSetAsActivityPub(res); + return await res.json(); } } From b3d4f18175418504965dfc64c1d26c16c5696629 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=8B=E3=81=A3=E3=81=93=E3=81=8B=E3=82=8A?= <67428053+kakkokari-gtyih@users.noreply.github.com> Date: Sat, 17 Aug 2024 18:25:46 +0900 Subject: [PATCH 05/20] Update packages/backend/src/core/activitypub/ApRequestService.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cherry picked from commit 129af061989f535ab4c79f497ba55cc5f6bf0385 Co-authored-by: かっこかり <67428053+kakkokari-gtyih@users.noreply.github.com> --- packages/backend/src/core/activitypub/ApRequestService.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts index b553708a8..5d3ae0a9e 100644 --- a/packages/backend/src/core/activitypub/ApRequestService.ts +++ b/packages/backend/src/core/activitypub/ApRequestService.ts @@ -202,7 +202,9 @@ export class ApRequestService { }); //#region リクエスト先がhtmlかつactivity+jsonへのalternate linkタグがあるとき - if (res.headers.get('Content-type')?.startsWith('text/html;') && _followAlternate) { + const contentType = res.headers.get('content-type'); + + if ((contentType === 'text/html' || contentType?.startsWith('text/html;')) && _followAlternate) { const html = await res.text(); const fragment = JSDOM.fragment(html); From 6d4dc5ea206782333437fd41f472da955dca14b6 Mon Sep 17 00:00:00 2001 From: taichan <40626578+tai-cha@users.noreply.github.com> Date: Sat, 17 Aug 2024 19:51:56 +0900 Subject: [PATCH 06/20] Fix(beckend): html content-type detection on signedGet (#14424) cherry picked from commit bf8c42eecd3d645652ddd7e69b727ced2a15457d Co-authored-by: taichan <40626578+tai-cha@users.noreply.github.com> --- packages/backend/src/core/activitypub/ApRequestService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts index 5d3ae0a9e..3c988773b 100644 --- a/packages/backend/src/core/activitypub/ApRequestService.ts +++ b/packages/backend/src/core/activitypub/ApRequestService.ts @@ -204,7 +204,7 @@ export class ApRequestService { //#region リクエスト先がhtmlかつactivity+jsonへのalternate linkタグがあるとき const contentType = res.headers.get('content-type'); - if ((contentType === 'text/html' || contentType?.startsWith('text/html;')) && _followAlternate) { + if ((contentType ?? '').split(';')[0].trimEnd().toLowerCase() === 'text/html' && _followAlternate) { const html = await res.text(); const fragment = JSDOM.fragment(html); From 9e998cc10baf0779d57a100088f17fe1f4ec8aa0 Mon Sep 17 00:00:00 2001 From: Hazel K Date: Sun, 18 Aug 2024 00:34:01 -0400 Subject: [PATCH 07/20] fix(backend): memory leak in memory caches (#14363) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cherry picked from commit bf8c42eecd3d645652ddd7e69b727ced2a15457d Co-authored-by: かっこかり <67428053+kakkokari-gtyih@users.noreply.github.com> Co-authored-by: Hazel K --- .../src/core/AvatarDecorationService.ts | 2 +- packages/backend/src/core/CacheService.ts | 12 +- .../backend/src/core/CustomEmojiService.ts | 12 +- packages/backend/src/core/RelayService.ts | 2 +- packages/backend/src/core/RoleService.ts | 6 +- .../backend/src/core/UserKeypairService.ts | 2 +- .../core/activitypub/ApDbResolverService.ts | 4 +- packages/backend/src/misc/cache.ts | 139 ++++++++++-------- .../processors/DeliverProcessorService.ts | 2 +- .../src/server/NodeinfoServerService.ts | 2 +- .../src/server/api/AuthenticateService.ts | 2 +- 11 files changed, 99 insertions(+), 86 deletions(-) diff --git a/packages/backend/src/core/AvatarDecorationService.ts b/packages/backend/src/core/AvatarDecorationService.ts index 21e31d79a..fa3f63677 100644 --- a/packages/backend/src/core/AvatarDecorationService.ts +++ b/packages/backend/src/core/AvatarDecorationService.ts @@ -29,7 +29,7 @@ export class AvatarDecorationService implements OnApplicationShutdown { private moderationLogService: ModerationLogService, private globalEventService: GlobalEventService, ) { - this.cache = new MemorySingleCache(1000 * 60 * 30); + this.cache = new MemorySingleCache(1000 * 60 * 30); // 30s this.redisForSub.on('message', this.onMessage); } diff --git a/packages/backend/src/core/CacheService.ts b/packages/backend/src/core/CacheService.ts index d008e7ec5..6725ebe75 100644 --- a/packages/backend/src/core/CacheService.ts +++ b/packages/backend/src/core/CacheService.ts @@ -56,10 +56,10 @@ export class CacheService implements OnApplicationShutdown { ) { //this.onMessage = this.onMessage.bind(this); - this.userByIdCache = new MemoryKVCache(Infinity); - this.localUserByNativeTokenCache = new MemoryKVCache(Infinity); - this.localUserByIdCache = new MemoryKVCache(Infinity); - this.uriPersonCache = new MemoryKVCache(Infinity); + this.userByIdCache = new MemoryKVCache(1000 * 60 * 5); // 5m + this.localUserByNativeTokenCache = new MemoryKVCache(1000 * 60 * 5); // 5m + this.localUserByIdCache = new MemoryKVCache(1000 * 60 * 5); // 5m + this.uriPersonCache = new MemoryKVCache(1000 * 60 * 5); // 5m this.userProfileCache = new RedisKVCache(this.redisClient, 'userProfile', { lifetime: 1000 * 60 * 30, // 30m @@ -135,14 +135,14 @@ export class CacheService implements OnApplicationShutdown { if (user == null) { this.userByIdCache.delete(body.id); this.localUserByIdCache.delete(body.id); - for (const [k, v] of this.uriPersonCache.cache.entries()) { + for (const [k, v] of this.uriPersonCache.entries) { if (v.value?.id === body.id) { this.uriPersonCache.delete(k); } } } else { this.userByIdCache.set(user.id, user); - for (const [k, v] of this.uriPersonCache.cache.entries()) { + for (const [k, v] of this.uriPersonCache.entries) { if (v.value?.id === user.id) { this.uriPersonCache.set(k, user); } diff --git a/packages/backend/src/core/CustomEmojiService.ts b/packages/backend/src/core/CustomEmojiService.ts index feca0fe5e..27b85b10b 100644 --- a/packages/backend/src/core/CustomEmojiService.ts +++ b/packages/backend/src/core/CustomEmojiService.ts @@ -24,7 +24,7 @@ const parseEmojiStrRegexp = /^([-\w]+)(?:@([\w.-]+))?$/; @Injectable() export class CustomEmojiService implements OnApplicationShutdown { - private cache: MemoryKVCache; + private emojisCache: MemoryKVCache; public localEmojisCache: RedisSingleCache>; constructor( @@ -40,7 +40,7 @@ export class CustomEmojiService implements OnApplicationShutdown { private moderationLogService: ModerationLogService, private globalEventService: GlobalEventService, ) { - this.cache = new MemoryKVCache(1000 * 60 * 60 * 12); + this.emojisCache = new MemoryKVCache(1000 * 60 * 60 * 12); // 12h this.localEmojisCache = new RedisSingleCache>(this.redisClient, 'localEmojis', { lifetime: 1000 * 60 * 30, // 30m @@ -346,7 +346,7 @@ export class CustomEmojiService implements OnApplicationShutdown { host, })) ?? null; - const emoji = await this.cache.fetch(`${name} ${host}`, queryOrNull); + const emoji = await this.emojisCache.fetch(`${name} ${host}`, queryOrNull); if (emoji == null) return null; return emoji.publicUrl || emoji.originalUrl; // || emoji.originalUrl してるのは後方互換性のため(publicUrlはstringなので??はだめ) @@ -372,7 +372,7 @@ export class CustomEmojiService implements OnApplicationShutdown { */ @bindThis public async prefetchEmojis(emojis: { name: string; host: string | null; }[]): Promise { - const notCachedEmojis = emojis.filter(emoji => this.cache.get(`${emoji.name} ${emoji.host}`) == null); + const notCachedEmojis = emojis.filter(emoji => this.emojisCache.get(`${emoji.name} ${emoji.host}`) == null); const emojisQuery: any[] = []; const hosts = new Set(notCachedEmojis.map(e => e.host)); for (const host of hosts) { @@ -387,7 +387,7 @@ export class CustomEmojiService implements OnApplicationShutdown { select: ['name', 'host', 'originalUrl', 'publicUrl'], }) : []; for (const emoji of _emojis) { - this.cache.set(`${emoji.name} ${emoji.host}`, emoji); + this.emojisCache.set(`${emoji.name} ${emoji.host}`, emoji); } } @@ -412,7 +412,7 @@ export class CustomEmojiService implements OnApplicationShutdown { @bindThis public dispose(): void { - this.cache.dispose(); + this.emojisCache.dispose(); } @bindThis diff --git a/packages/backend/src/core/RelayService.ts b/packages/backend/src/core/RelayService.ts index e9dc9b57a..91857dc68 100644 --- a/packages/backend/src/core/RelayService.ts +++ b/packages/backend/src/core/RelayService.ts @@ -35,7 +35,7 @@ export class RelayService { private createSystemUserService: CreateSystemUserService, private apRendererService: ApRendererService, ) { - this.relaysCache = new MemorySingleCache(1000 * 60 * 10); + this.relaysCache = new MemorySingleCache(1000 * 60 * 10); // 10m } @bindThis diff --git a/packages/backend/src/core/RoleService.ts b/packages/backend/src/core/RoleService.ts index 45e8b641c..9089480f9 100644 --- a/packages/backend/src/core/RoleService.ts +++ b/packages/backend/src/core/RoleService.ts @@ -149,10 +149,8 @@ export class RoleService implements OnApplicationShutdown, OnModuleInit { private moderationLogService: ModerationLogService, private fanoutTimelineService: FanoutTimelineService, ) { - //this.onMessage = this.onMessage.bind(this); - - this.rolesCache = new MemorySingleCache(1000 * 60 * 60 * 1); - this.roleAssignmentByUserIdCache = new MemoryKVCache(1000 * 60 * 60 * 1); + this.rolesCache = new MemorySingleCache(1000 * 60 * 60); // 1h + this.roleAssignmentByUserIdCache = new MemoryKVCache(1000 * 60 * 5); // 5m this.redisForSub.on('message', this.onMessage); } diff --git a/packages/backend/src/core/UserKeypairService.ts b/packages/backend/src/core/UserKeypairService.ts index 51ac99179..92d61cd10 100644 --- a/packages/backend/src/core/UserKeypairService.ts +++ b/packages/backend/src/core/UserKeypairService.ts @@ -25,7 +25,7 @@ export class UserKeypairService implements OnApplicationShutdown { ) { this.cache = new RedisKVCache(this.redisClient, 'userKeypair', { lifetime: 1000 * 60 * 60 * 24, // 24h - memoryCacheLifetime: Infinity, + memoryCacheLifetime: 1000 * 60 * 60, // 1h fetcher: (key) => this.userKeypairsRepository.findOneByOrFail({ userId: key }), toRedisConverter: (value) => JSON.stringify(value), fromRedisConverter: (value) => JSON.parse(value), diff --git a/packages/backend/src/core/activitypub/ApDbResolverService.ts b/packages/backend/src/core/activitypub/ApDbResolverService.ts index f6b70ead4..4192e8659 100644 --- a/packages/backend/src/core/activitypub/ApDbResolverService.ts +++ b/packages/backend/src/core/activitypub/ApDbResolverService.ts @@ -54,8 +54,8 @@ export class ApDbResolverService implements OnApplicationShutdown { private cacheService: CacheService, private apPersonService: ApPersonService, ) { - this.publicKeyCache = new MemoryKVCache(Infinity); - this.publicKeyByUserIdCache = new MemoryKVCache(Infinity); + this.publicKeyCache = new MemoryKVCache(1000 * 60 * 60 * 12); // 12h + this.publicKeyByUserIdCache = new MemoryKVCache(1000 * 60 * 60 * 12); // 12h } @bindThis diff --git a/packages/backend/src/misc/cache.ts b/packages/backend/src/misc/cache.ts index 4d7809570..07d4be7f3 100644 --- a/packages/backend/src/misc/cache.ts +++ b/packages/backend/src/misc/cache.ts @@ -7,23 +7,23 @@ import * as Redis from 'ioredis'; import { bindThis } from '@/decorators.js'; export class RedisKVCache { - private redisClient: Redis.Redis; - private name: string; - private lifetime: number; - private memoryCache: MemoryKVCache; - private fetcher: (key: string) => Promise; - private toRedisConverter: (value: T) => string; - private fromRedisConverter: (value: string) => T | undefined; + private readonly lifetime: number; + private readonly memoryCache: MemoryKVCache; + private readonly fetcher: (key: string) => Promise; + private readonly toRedisConverter: (value: T) => string; + private readonly fromRedisConverter: (value: string) => T | undefined; - constructor(redisClient: RedisKVCache['redisClient'], name: RedisKVCache['name'], opts: { - lifetime: RedisKVCache['lifetime']; - memoryCacheLifetime: number; - fetcher: RedisKVCache['fetcher']; - toRedisConverter: RedisKVCache['toRedisConverter']; - fromRedisConverter: RedisKVCache['fromRedisConverter']; - }) { - this.redisClient = redisClient; - this.name = name; + constructor( + private redisClient: Redis.Redis, + private name: string, + opts: { + lifetime: RedisKVCache['lifetime']; + memoryCacheLifetime: number; + fetcher: RedisKVCache['fetcher']; + toRedisConverter: RedisKVCache['toRedisConverter']; + fromRedisConverter: RedisKVCache['fromRedisConverter']; + }, + ) { this.lifetime = opts.lifetime; this.memoryCache = new MemoryKVCache(opts.memoryCacheLifetime); this.fetcher = opts.fetcher; @@ -55,10 +55,13 @@ export class RedisKVCache { const cached = await this.redisClient.get(`kvcache:${this.name}:${key}`); if (cached == null) return undefined; - const parsed = this.fromRedisConverter(cached); - if (parsed == null) return undefined; - this.memoryCache.set(key, parsed); - return parsed; + + const value = this.fromRedisConverter(cached); + if (value !== undefined) { + this.memoryCache.set(key, value); + } + + return value; } @bindThis @@ -69,6 +72,10 @@ export class RedisKVCache { /** * キャッシュがあればそれを返し、無ければfetcherを呼び出して結果をキャッシュ&返します + * This awaits the call to Redis to ensure that the write succeeded, which is important for a few reasons: + * * Other code uses this to synchronize changes between worker processes. A failed write can internally de-sync the cluster. + * * Without an `await`, consecutive calls could race. An unlucky race could result in the older write overwriting the newer value. + * * Not awaiting here makes the entire cache non-consistent. The prevents many possible uses. */ @bindThis public async fetch(key: string): Promise { @@ -80,14 +87,14 @@ export class RedisKVCache { // Cache MISS const value = await this.fetcher(key); - this.set(key, value); + await this.set(key, value); return value; } @bindThis public async refresh(key: string) { const value = await this.fetcher(key); - this.set(key, value); + await this.set(key, value); // TODO: イベント発行して他プロセスのメモリキャッシュも更新できるようにする } @@ -104,23 +111,23 @@ export class RedisKVCache { } export class RedisSingleCache { - private redisClient: Redis.Redis; - private name: string; - private lifetime: number; - private memoryCache: MemorySingleCache; - private fetcher: () => Promise; - private toRedisConverter: (value: T) => string; - private fromRedisConverter: (value: string) => T | undefined; + private readonly lifetime: number; + private readonly memoryCache: MemorySingleCache; + private readonly fetcher: () => Promise; + private readonly toRedisConverter: (value: T) => string; + private readonly fromRedisConverter: (value: string) => T | undefined; - constructor(redisClient: RedisSingleCache['redisClient'], name: RedisSingleCache['name'], opts: { - lifetime: RedisSingleCache['lifetime']; - memoryCacheLifetime: number; - fetcher: RedisSingleCache['fetcher']; - toRedisConverter: RedisSingleCache['toRedisConverter']; - fromRedisConverter: RedisSingleCache['fromRedisConverter']; - }) { - this.redisClient = redisClient; - this.name = name; + constructor( + private redisClient: Redis.Redis, + private name: string, + opts: { + lifetime: number; + memoryCacheLifetime: number; + fetcher: RedisSingleCache['fetcher']; + toRedisConverter: RedisSingleCache['toRedisConverter']; + fromRedisConverter: RedisSingleCache['fromRedisConverter']; + }, + ) { this.lifetime = opts.lifetime; this.memoryCache = new MemorySingleCache(opts.memoryCacheLifetime); this.fetcher = opts.fetcher; @@ -152,10 +159,13 @@ export class RedisSingleCache { const cached = await this.redisClient.get(`singlecache:${this.name}`); if (cached == null) return undefined; - const parsed = this.fromRedisConverter(cached); - if (parsed == null) return undefined; - this.memoryCache.set(parsed); - return parsed; + + const value = this.fromRedisConverter(cached); + if (value !== undefined) { + this.memoryCache.set(value); + } + + return value; } @bindThis @@ -166,6 +176,10 @@ export class RedisSingleCache { /** * キャッシュがあればそれを返し、無ければfetcherを呼び出して結果をキャッシュ&返します + * This awaits the call to Redis to ensure that the write succeeded, which is important for a few reasons: + * * Other code uses this to synchronize changes between worker processes. A failed write can internally de-sync the cluster. + * * Without an `await`, consecutive calls could race. An unlucky race could result in the older write overwriting the newer value. + * * Not awaiting here makes the entire cache non-consistent. The prevents many possible uses. */ @bindThis public async fetch(): Promise { @@ -177,14 +191,14 @@ export class RedisSingleCache { // Cache MISS const value = await this.fetcher(); - this.set(value); + await this.set(value); return value; } @bindThis public async refresh() { const value = await this.fetcher(); - this.set(value); + await this.set(value); // TODO: イベント発行して他プロセスのメモリキャッシュも更新できるようにする } @@ -197,18 +211,12 @@ export class MemoryKVCache { * データを持つマップ * これを直接操作するべきではない */ - public cache: Map; - private lifetime: number; - private gcIntervalHandle: NodeJS.Timeout; + private readonly cache = new Map(); + private readonly gcIntervalHandle = setInterval(() => this.gc(), 1000 * 60 * 3); // 3m - constructor(lifetime: MemoryKVCache['lifetime']) { - this.cache = new Map(); - this.lifetime = lifetime; - - this.gcIntervalHandle = setInterval(() => { - this.gc(); - }, 1000 * 60 * 3); - } + constructor( + private readonly lifetime: number, + ) {} @bindThis /** @@ -293,10 +301,14 @@ export class MemoryKVCache { @bindThis public gc(): void { const now = Date.now(); + for (const [key, { date }] of this.cache.entries()) { - if ((now - date) > this.lifetime) { - this.cache.delete(key); - } + // The map is ordered from oldest to youngest. + // We can stop once we find an entry that's still active, because all following entries must *also* be active. + const age = now - date; + if (age < this.lifetime) break; + + this.cache.delete(key); } } @@ -304,16 +316,19 @@ export class MemoryKVCache { public dispose(): void { clearInterval(this.gcIntervalHandle); } + + public get entries() { + return this.cache.entries(); + } } export class MemorySingleCache { private cachedAt: number | null = null; private value: T | undefined; - private lifetime: number; - constructor(lifetime: MemorySingleCache['lifetime']) { - this.lifetime = lifetime; - } + constructor( + private lifetime: number, + ) {} @bindThis public set(value: T): void { diff --git a/packages/backend/src/queue/processors/DeliverProcessorService.ts b/packages/backend/src/queue/processors/DeliverProcessorService.ts index 5fed07092..6f0cd718d 100644 --- a/packages/backend/src/queue/processors/DeliverProcessorService.ts +++ b/packages/backend/src/queue/processors/DeliverProcessorService.ts @@ -44,7 +44,7 @@ export class DeliverProcessorService { private queueLoggerService: QueueLoggerService, ) { this.logger = this.queueLoggerService.logger.createSubLogger('deliver'); - this.suspendedHostsCache = new MemorySingleCache(1000 * 60 * 60); + this.suspendedHostsCache = new MemorySingleCache(1000 * 60 * 60); // 1h } @bindThis diff --git a/packages/backend/src/server/NodeinfoServerService.ts b/packages/backend/src/server/NodeinfoServerService.ts index cc49c5ffa..22d673cd2 100644 --- a/packages/backend/src/server/NodeinfoServerService.ts +++ b/packages/backend/src/server/NodeinfoServerService.ts @@ -133,7 +133,7 @@ export class NodeinfoServerService { return document; }; - const cache = new MemorySingleCache>>(1000 * 60 * 10); + const cache = new MemorySingleCache>>(1000 * 60 * 10); // 10m fastify.get(nodeinfo2_1path, async (request, reply) => { const base = await cache.fetch(() => nodeinfo2(21)); diff --git a/packages/backend/src/server/api/AuthenticateService.ts b/packages/backend/src/server/api/AuthenticateService.ts index ddef8db98..690ff2e02 100644 --- a/packages/backend/src/server/api/AuthenticateService.ts +++ b/packages/backend/src/server/api/AuthenticateService.ts @@ -37,7 +37,7 @@ export class AuthenticateService implements OnApplicationShutdown { private cacheService: CacheService, ) { - this.appCache = new MemoryKVCache(Infinity); + this.appCache = new MemoryKVCache(1000 * 60 * 60 * 24 * 7); // 1w } @bindThis From c441c4728f579c890a87eea2d0c8260e86b516b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=8B=E3=81=A3=E3=81=93=E3=81=8B=E3=82=8A?= <67428053+kakkokari-gtyih@users.noreply.github.com> Date: Sun, 15 Sep 2024 12:30:27 +0900 Subject: [PATCH 08/20] =?UTF-8?q?fix(backend):=20happy-dom=E3=81=A7?= =?UTF-8?q?=E5=A4=96=E9=83=A8HTML=E3=82=92=E3=83=91=E3=83=BC=E3=82=B9?= =?UTF-8?q?=E3=81=99=E3=82=8B=E9=9A=9B=E3=81=AB=E9=96=A2=E9=80=A3=E3=83=AA?= =?UTF-8?q?=E3=82=BD=E3=83=BC=E3=82=B9=E3=81=8C=E8=AA=AD=E3=81=BF=E8=BE=BC?= =?UTF-8?q?=E3=81=BE=E3=82=8C=E3=82=8B=E5=95=8F=E9=A1=8C=E3=82=92=E4=BF=AE?= =?UTF-8?q?=E6=AD=A3=20(#14521)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cherry picked from commit be0906a6c73726ed02a358bcbe904fa3d99713ea Co-authored-by: かっこかり <67428053+kakkokari-gtyih@users.noreply.github.com> --- .../src/core/activitypub/ApRequestService.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts index 3c988773b..4810fe9e4 100644 --- a/packages/backend/src/core/activitypub/ApRequestService.ts +++ b/packages/backend/src/core/activitypub/ApRequestService.ts @@ -206,14 +206,18 @@ export class ApRequestService { if ((contentType ?? '').split(';')[0].trimEnd().toLowerCase() === 'text/html' && _followAlternate) { const html = await res.text(); - const fragment = JSDOM.fragment(html); + try { + const fragment = JSDOM.fragment(html); - const alternate = fragment.querySelector('head > link[rel="alternate"][type="application/activity+json"]'); - if (alternate) { - const href = alternate.getAttribute('href'); - if (href) { - return await this.signedGet(href, user, false); + const alternate = fragment.querySelector('head > link[rel="alternate"][type="application/activity+json"]'); + if (alternate) { + const href = alternate.getAttribute('href'); + if (href) { + return await this.signedGet(href, user, false); + } } + } catch (e) { + // something went wrong parsing the HTML, ignore the whole thing } } //#endregion From f5c0430bc9b833561915035bee12c35bbd159010 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=82=E3=82=8F=E3=82=8F=E3=82=8F=E3=81=A8=E3=83=BC?= =?UTF-8?q?=E3=81=AB=E3=82=85?= <17376330+u1-liquid@users.noreply.github.com> Date: Wed, 25 Dec 2024 03:45:35 +0900 Subject: [PATCH 09/20] =?UTF-8?q?Fix:=20``?= =?UTF-8?q?=E3=82=92=E8=BF=BD=E3=81=A3=E3=81=A6=E7=85=A7=E4=BC=9A=E3=81=99?= =?UTF-8?q?=E3=82=8B=E3=81=AE=E3=81=AFOK=E3=83=AC=E3=82=B9=E3=83=9D?= =?UTF-8?q?=E3=83=B3=E3=82=B9=E3=81=8C=E8=BF=94=E5=8D=B4=E3=81=95=E3=82=8C?= =?UTF-8?q?=E3=81=9F=E5=A0=B4=E5=90=88=E3=81=AE=E3=81=BF=E3=81=AB=20(#1462?= =?UTF-8?q?7)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cherry picked from commit dd124a8aedb34a1112405fa68bd5daaa96fdc882 Co-authored-by: Julia Johannesen Co-authored-by: かっこかり <67428053+kakkokari-gtyih@users.noreply.github.com> --- packages/backend/src/core/activitypub/ApRequestService.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts index 4810fe9e4..fe58b42cf 100644 --- a/packages/backend/src/core/activitypub/ApRequestService.ts +++ b/packages/backend/src/core/activitypub/ApRequestService.ts @@ -204,7 +204,11 @@ export class ApRequestService { //#region リクエスト先がhtmlかつactivity+jsonへのalternate linkタグがあるとき const contentType = res.headers.get('content-type'); - if ((contentType ?? '').split(';')[0].trimEnd().toLowerCase() === 'text/html' && _followAlternate) { + if ( + res.ok && + (contentType ?? '').split(';')[0].trimEnd().toLowerCase() === 'text/html' && + _followAlternate + ) { const html = await res.text(); try { const fragment = JSDOM.fragment(html); From d1b5d562204e59653f16861e45d2c907fbce96c4 Mon Sep 17 00:00:00 2001 From: Tamme Schichler Date: Mon, 28 Oct 2024 13:06:16 +0100 Subject: [PATCH 10/20] fix(backend): Accept arrays in ActivityPub `icon` and `image` properties (#14825) This is allowed according to the Activity vocabulary: https://www.w3.org/TR/activitystreams-vocabulary/#dfn-icon The issue is noticeable in combination with Bridgy Fed: https://github.com/snarfed/bridgy-fed/issues/1408 (cherry picked from commit 8eb7749e448d912bdbe2c4eadc35f5d5f1becf61) --- .../backend/src/core/activitypub/models/ApPersonService.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index 1e9b9a4c0..fc238f595 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -248,6 +248,12 @@ export class ApPersonService implements OnModuleInit { if (user == null) throw new Error('failed to create user: user is null'); const [avatar, banner] = await Promise.all([icon, image].map(img => { + // icon and image may be arrays + // see https://www.w3.org/TR/activitystreams-vocabulary/#dfn-icon + if (Array.isArray(img)) { + img = img.find(item => item && item.url) ?? null; + } + // if we have an explicitly missing image, return an // explicitly-null set of values if ((img == null) || (typeof img === 'object' && img.url == null)) { From 09f518b41b93cd8a1c597a642843d9a484511726 Mon Sep 17 00:00:00 2001 From: CDN Date: Sat, 16 Nov 2024 17:53:28 +0800 Subject: [PATCH 11/20] fix(backend): fallback sharedInbox to null in ApPersonService (#14970) (cherry picked from commit b3c2de2b2643d777d360de0171ae573f39411c02) --- .../backend/src/core/activitypub/models/ApPersonService.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index fc238f595..cfb8d2797 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -366,7 +366,7 @@ export class ApPersonService implements OnModuleInit { usernameLower: person.preferredUsername?.toLowerCase(), host, inbox: person.inbox, - sharedInbox: person.sharedInbox ?? person.endpoints?.sharedInbox, + sharedInbox: person.sharedInbox ?? person.endpoints?.sharedInbox ?? null, followersUri: person.followers ? getApId(person.followers) : undefined, featured: person.featured ? getApId(person.featured) : undefined, uri: person.id, @@ -528,7 +528,7 @@ export class ApPersonService implements OnModuleInit { const updates = { lastFetchedAt: new Date(), inbox: person.inbox, - sharedInbox: person.sharedInbox ?? person.endpoints?.sharedInbox, + sharedInbox: person.sharedInbox ?? person.endpoints?.sharedInbox ?? null, followersUri: person.followers ? getApId(person.followers) : undefined, featured: person.featured, emojis: emojiNames, @@ -599,7 +599,7 @@ export class ApPersonService implements OnModuleInit { // 該当ユーザーが既にフォロワーになっていた場合はFollowingもアップデートする await this.followingsRepository.update( { followerId: exist.id }, - { followerSharedInbox: person.sharedInbox ?? person.endpoints?.sharedInbox }, + { followerSharedInbox: person.sharedInbox ?? person.endpoints?.sharedInbox ?? null }, ); await this.updateFeatured(exist.id, resolver).catch(err => this.logger.error(err)); From 85096e58b9dac5f0467cd4b849894ffdf5d2448d Mon Sep 17 00:00:00 2001 From: Julia Date: Wed, 20 Nov 2024 18:20:09 -0500 Subject: [PATCH 12/20] 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> (cherry picked from commit 5f675201f261d5db6a58d3099a190372bb2f09f0) --- .../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 | 77 +++++++++------ .../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, 222 insertions(+), 86 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 3eb2a8089..4c1accae4 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 38f45a03a..40cb41343 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 16c71e971..7d3a74cf5 100644 --- a/packages/backend/src/core/UtilityService.ts +++ b/packages/backend/src/core/UtilityService.ts @@ -30,6 +30,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; @@ -90,7 +95,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 @@ -103,4 +108,11 @@ export class UtilityService { if (host == null) return null; return punycode.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; + } } diff --git a/packages/backend/src/core/activitypub/ApDbResolverService.ts b/packages/backend/src/core/activitypub/ApDbResolverService.ts index 4192e8659..5c16744a7 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 0b6de71ea..568113a9b 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -90,15 +90,26 @@ export class ApInboxService { } @bindThis - public async performActivity(actor: MiRemoteUser, activity: IObject, additionalCc?: MiLocalUser['id']): Promise { + public async performActivity(actor: MiRemoteUser, activity: IObject, resolver?: Resolver, additionalCc?: MiLocalUser['id']): Promise { let result = 'error'; 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.warn('skipping activity: activity id is null or mismatching'); + continue; + } try { - results.push([getApId(item), await this.performOneActivity(actor, act, additionalCc)]); + results.push([getApId(item), await this.performOneActivity(actor, act, resolver, additionalCc)]); } catch (err) { if (err instanceof Error || typeof err === 'string') { this.logger.error(err); @@ -113,7 +124,7 @@ export class ApInboxService { result = results.map(([id, reason]) => `${id}: ${reason}`).join('\n'); } } else { - result = await this.performOneActivity(actor, activity, additionalCc); + result = await this.performOneActivity(actor, activity, resolver, additionalCc); } // ついでにリモートユーザーの情報が古かったら更新しておく @@ -129,37 +140,37 @@ export class ApInboxService { } @bindThis - public async performOneActivity(actor: MiRemoteUser, activity: IObject, additionalCc?: MiLocalUser['id']): Promise { + public async performOneActivity(actor: MiRemoteUser, activity: IObject, resolver?: Resolver, additionalCc?: MiLocalUser['id']): Promise { if (actor.isSuspended) return 'skip: actor is suspended'; if (isCreate(activity)) { - return await this.create(actor, activity, additionalCc); + return await this.create(actor, activity, resolver, additionalCc); } else if (isDelete(activity)) { return await this.delete(actor, activity); } else if (isUpdate(activity)) { - return await this.update(actor, activity, additionalCc); + return await this.update(actor, activity, resolver, additionalCc); } 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).catch(err => { this.logger.error(err); return `error: ${err.message}`; }); + return await this.add(actor, activity, resolver).catch(err => { this.logger.error(err); return `error: ${err.message}`; }); } else if (isRemove(activity)) { - return await this.remove(actor, activity).catch(err => { this.logger.error(err); return `error: ${err.message}`; }); + return await this.remove(actor, activity, resolver).catch(err => { this.logger.error(err); return `error: ${err.message}`; }); } 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 `skip: unknown activity type ${activity.type}`; } @@ -201,12 +212,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}`, { error: err }); @@ -243,7 +255,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 'skip: invalid actor'; } @@ -253,7 +265,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 'skip: note not found'; await this.notePiningService.addPinned(actor, note.id); return 'ok'; @@ -263,12 +275,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'; @@ -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) { @@ -306,7 +319,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 'skip: target note not found'; } catch (err) { // 対象が4xxならスキップ @@ -326,7 +339,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) { @@ -366,7 +379,7 @@ export class ApInboxService { } @bindThis - private async create(actor: MiRemoteUser, activity: ICreate, additionalCc?: MiLocalUser['id']): Promise { + private async create(actor: MiRemoteUser, activity: ICreate, resolver?: Resolver, additionalCc?: MiLocalUser['id']): Promise { const uri = getApId(activity); this.logger.info(`Create: ${uri}`); @@ -389,7 +402,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}`, { error: e }); @@ -416,6 +430,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'; } } @@ -431,7 +447,7 @@ export class ApInboxService { return 'skip: note exists'; } - const createdNote = await this.apNoteService.createNote(note, resolver, silent); + const createdNote = await this.apNoteService.createNote(note, actor, resolver, silent); if (createdNote && additionalCc && !await this.noteEntityService.isVisibleForMe(createdNote, additionalCc)) { await this.noteCreateService.appendNoteVisibleUser(actor, createdNote, additionalCc); return 'ok: note visible user appended'; @@ -572,12 +588,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}`, { error: e }); @@ -614,7 +631,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 'skip: invalid actor'; } @@ -624,7 +641,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 'skip: note not found'; await this.notePiningService.removePinned(actor, note.id); return 'ok'; @@ -634,7 +651,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 'skip: invalid actor'; } @@ -643,7 +660,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}`, { error: e }); @@ -767,14 +785,15 @@ export class ApInboxService { } @bindThis - private async update(actor: MiRemoteUser, activity: IUpdate, additionalCc?: MiLocalUser['id']): Promise { + private async update(actor: MiRemoteUser, activity: IUpdate, resolver?: Resolver, additionalCc?: MiLocalUser['id']): 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}`, { error: e }); @@ -814,11 +833,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 fe58b42cf..f59fabba3 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, ) { this.logger = this.loggerService.getLogger('ap:request'); } @@ -216,7 +220,7 @@ export class ApRequestService { const alternate = fragment.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); } } @@ -227,7 +231,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 fbe32ba18..000f9d59f 100644 --- a/packages/backend/src/core/activitypub/ApResolverService.ts +++ b/packages/backend/src/core/activitypub/ApResolverService.ts @@ -42,7 +42,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'); @@ -53,6 +53,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' @@ -115,6 +120,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 000000000..78ba891a2 --- /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 60a14a4e7..bd00529da 100644 --- a/packages/backend/src/core/activitypub/models/ApNoteService.ts +++ b/packages/backend/src/core/activitypub/models/ApNoteService.ts @@ -80,7 +80,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); @@ -101,6 +101,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; } @@ -118,7 +126,7 @@ 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(); @@ -127,7 +135,7 @@ export class ApNoteService { 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() }, @@ -142,14 +150,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}`); @@ -162,8 +180,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', `User ${cachedActor.id} has been suspended.`); } @@ -195,7 +214,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) { @@ -359,7 +379,7 @@ export class ApNoteService { if (exist) return exist; //#endregion - if (new URL(uri).origin === this.config.url) { + if (this.utilityService.isUriLocal(uri)) { throw new StatusError('cannot resolve local note', 400, 'cannot resolve local note'); } @@ -367,7 +387,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 cfb8d2797..634d3f25f 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -130,12 +130,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 @@ -143,7 +137,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}'`); @@ -157,18 +151,26 @@ export class ApPersonService implements OnModuleInit { throw new Error('invalid Actor: wrong inbox'); } - try { - new URL(x.inbox); - } catch { - throw new Error('invalid Actor: wrong inbox'); + if (this.utilityService.punyHost(x.inbox) !== expectHost) { + throw new Error('invalid Actor: inbox has different host'); } - const sharedInbox = x.sharedInbox ?? x.endpoints?.sharedInbox; - if (typeof sharedInbox === 'string') { - try { - new URL(sharedInbox); - } catch { - throw new Error('invalid Actor: wrong sharedInbox'); + 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}`); } } @@ -195,7 +197,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'); } @@ -205,7 +207,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'); } @@ -291,7 +293,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 (new URL(uri).origin === 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'); } @@ -305,8 +308,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); @@ -332,8 +333,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 @@ -470,7 +481,7 @@ export class ApPersonService implements OnModuleInit { if (typeof uri !== 'string') throw new Error('uri is not string'); // URIがこのサーバーを指しているならスキップ - if (new URL(uri).origin === this.config.url) return; + if (this.utilityService.isUriLocal(uri)) return; //#region このサーバーに既に登録されているか const exist = await this.fetchPerson(uri) as MiRemoteUser | null; @@ -519,8 +530,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 policy = await this.roleService.getUserPolicies(exist.id); @@ -731,7 +752,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 (new URL(src.movedToUri).origin === 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 4e3df00e0..ce08d4098 100644 --- a/packages/backend/src/core/activitypub/models/ApQuestionService.ts +++ b/packages/backend/src/core/activitypub/models/ApQuestionService.ts @@ -11,6 +11,7 @@ import type { IPoll } from '@/models/Poll.js'; import type Logger from '@/logger.js'; import { bindThis } from '@/decorators.js'; import { isNotNull } from '@/misc/is-not-null.js'; +import { UtilityService } from '@/core/UtilityService.js'; import { isQuestion } from '../type.js'; import { ApLoggerService } from '../ApLoggerService.js'; import { ApResolverService } from '../ApResolverService.js'; @@ -33,6 +34,7 @@ export class ApQuestionService { private apResolverService: ApResolverService, private apLoggerService: ApLoggerService, + private utilityService: UtilityService, ) { this.logger = this.apLoggerService.logger; } @@ -71,7 +73,7 @@ export class ApQuestionService { if (uri == null) throw new Error('uri is null'); // URIがこのサーバーを指しているならスキップ - if (new URL(uri).origin === 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 0bb30d27f..1b11b167b 100644 --- a/packages/backend/src/queue/processors/InboxProcessorService.ts +++ b/packages/backend/src/queue/processors/InboxProcessorService.ts @@ -180,6 +180,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'); } // Update stats diff --git a/packages/backend/src/server/ActivityPubServerService.ts b/packages/backend/src/server/ActivityPubServerService.ts index 8ba2daf79..0a6dc6370 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 d8c55de7e..14286bc23 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 d3c40dba5..7cfd398d4 100644 --- a/packages/backend/src/server/api/endpoints/ap/show.ts +++ b/packages/backend/src/server/api/endpoints/ap/show.ts @@ -122,6 +122,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; From f1b5708971177d4365b0bc8b79658ba55ae677c3 Mon Sep 17 00:00:00 2001 From: Julia Date: Wed, 20 Nov 2024 18:24:50 -0500 Subject: [PATCH 13/20] Merge commit from fork * Fix poll update spoofing * fix: Disallow negative poll counts --------- Co-authored-by: syuilo <4439005+syuilo@users.noreply.github.com> (cherry picked from commit b9cb949eb1f8c57eaa98fc5446d902cf8a5ea85c) --- .../src/core/activitypub/ApInboxService.ts | 2 +- .../activitypub/models/ApQuestionService.ts | 31 ++++++++++++++----- .../queue/processors/InboxProcessorService.ts | 2 +- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index 568113a9b..b6c7877a8 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -804,7 +804,7 @@ export class ApInboxService { await this.apPersonService.updatePerson(actor.uri, resolver, object); return 'ok: Person updated'; } else if (getApType(object) === 'Question') { - await this.apQuestionService.updateQuestion(object, resolver).catch(err => this.logger.error(`err: failed to update question: ${err}`, { error: err })); + await this.apQuestionService.updateQuestion(object, actor, resolver).catch(err => this.logger.error(`err: failed to update question: ${err}`, { error: err })); return 'ok: Question updated'; } else if (additionalCc && isPost(object)) { const uri = getApId(object); diff --git a/packages/backend/src/core/activitypub/models/ApQuestionService.ts b/packages/backend/src/core/activitypub/models/ApQuestionService.ts index ce08d4098..d34c1a76c 100644 --- a/packages/backend/src/core/activitypub/models/ApQuestionService.ts +++ b/packages/backend/src/core/activitypub/models/ApQuestionService.ts @@ -5,18 +5,19 @@ import { Inject, Injectable } from '@nestjs/common'; import { DI } from '@/di-symbols.js'; -import type { NotesRepository, PollsRepository } from '@/models/_.js'; +import type { UsersRepository, NotesRepository, PollsRepository } from '@/models/_.js'; import type { Config } from '@/config.js'; import type { IPoll } from '@/models/Poll.js'; +import type { MiRemoteUser } from '@/models/User.js'; import type Logger from '@/logger.js'; import { bindThis } from '@/decorators.js'; import { isNotNull } from '@/misc/is-not-null.js'; import { UtilityService } from '@/core/UtilityService.js'; -import { isQuestion } from '../type.js'; +import { getOneApId, isQuestion } from '../type.js'; import { ApLoggerService } from '../ApLoggerService.js'; import { ApResolverService } from '../ApResolverService.js'; import type { Resolver } from '../ApResolverService.js'; -import type { IObject, IQuestion } from '../type.js'; +import type { IObject } from '../type.js'; @Injectable() export class ApQuestionService { @@ -26,6 +27,9 @@ export class ApQuestionService { @Inject(DI.config) private config: Config, + @Inject(DI.usersRepository) + private usersRepository: UsersRepository, + @Inject(DI.notesRepository) private notesRepository: NotesRepository, @@ -68,7 +72,7 @@ export class ApQuestionService { * @returns true if updated */ @bindThis - public async updateQuestion(value: string | IObject, resolver?: Resolver): Promise { + public async updateQuestion(value: string | IObject, actor?: MiRemoteUser, resolver?: Resolver): Promise { const uri = typeof value === 'string' ? value : value.id; if (uri == null) throw new Error('uri is null'); @@ -80,16 +84,27 @@ export class ApQuestionService { if (note == null) throw new Error('Question is not registed'); const poll = await this.pollsRepository.findOneBy({ noteId: note.id }); - if (poll == null) throw new Error('Question is not registed'); + if (poll == null) throw new Error('Question is not registered'); + + const user = await this.usersRepository.findOneBy({ id: poll.userId }); + if (user == null) throw new Error('User is not registered'); //#endregion // resolve new Question object // eslint-disable-next-line no-param-reassign if (resolver == null) resolver = this.apResolverService.createResolver(); - const question = await resolver.resolve(value) as IQuestion; + const question = await resolver.resolve(value); this.logger.debug(`fetched question: ${JSON.stringify(question, null, 2)}`); - if (question.type !== 'Question') throw new Error('object is not a Question'); + if (!isQuestion(question)) throw new Error('object is not a Question'); + + const attribution = (question.attributedTo) ? getOneApId(question.attributedTo) : user.uri; + const attributionMatchesExisting = attribution === user.uri; + const actorMatchesAttribution = (actor) ? attribution === actor.uri : true; + + if (!attributionMatchesExisting || !actorMatchesAttribution) { + throw new Error('Refusing to ingest update for poll by different user'); + } const apChoices = question.oneOf ?? question.anyOf; if (apChoices == null) throw new Error('invalid apChoices: ' + apChoices); @@ -99,7 +114,7 @@ export class ApQuestionService { for (const choice of poll.choices) { const oldCount = poll.votes[poll.choices.indexOf(choice)]; const newCount = apChoices.filter(ap => ap.name === choice).at(0)?.replies?.totalItems; - if (newCount == null) throw new Error('invalid newCount: ' + newCount); + if (newCount == null || !(Number.isInteger(newCount) && newCount >= 0)) throw new Error('invalid newCount: ' + newCount); if (oldCount !== newCount) { changed = true; diff --git a/packages/backend/src/queue/processors/InboxProcessorService.ts b/packages/backend/src/queue/processors/InboxProcessorService.ts index 1b11b167b..70f3e30c5 100644 --- a/packages/backend/src/queue/processors/InboxProcessorService.ts +++ b/packages/backend/src/queue/processors/InboxProcessorService.ts @@ -200,7 +200,7 @@ export class InboxProcessorService implements OnApplicationShutdown { // アクティビティを処理 try { - const result = await this.apInboxService.performActivity(authUser.user, activity, job.data.user?.id); + const result = await this.apInboxService.performActivity(authUser.user, activity, undefined, job.data.user?.id); if (result && !result.startsWith('ok')) { this.logger.warn(`inbox activity ignored (maybe): id=${activity.id} reason=${result}`); return result; From 8c5a9c19d10971c8e01e86b6b0c7cc95cc9b97f4 Mon Sep 17 00:00:00 2001 From: rectcoordsystem <37621004+rectcoordsystem@users.noreply.github.com> Date: Thu, 21 Nov 2024 08:27:09 +0900 Subject: [PATCH 14/20] Merge commit from fork * fix(backend): check target IP before sending HTTP request * fix(backend): allow accessing private IP when testing * Apply suggestions from code review Co-authored-by: anatawa12 * fix(backend): lint and typecheck * fix(backend): add isLocalAddressAllowed option to getAgentByUrl and send (HttpRequestService) * fix(backend): allow fetchSummaryFromProxy, trueMail to access local addresses --------- Co-authored-by: anatawa12 Co-authored-by: syuilo <4439005+syuilo@users.noreply.github.com> (cherry picked from commit 090e9392cdb1f584af94a6fb727fba95de3b8504) --- packages/backend/src/core/DownloadService.ts | 22 --- packages/backend/src/core/EmailService.ts | 1 + .../backend/src/core/HttpRequestService.ts | 133 ++++++++++++++++-- .../src/server/web/UrlPreviewService.ts | 2 +- 4 files changed, 122 insertions(+), 36 deletions(-) diff --git a/packages/backend/src/core/DownloadService.ts b/packages/backend/src/core/DownloadService.ts index 21ae798f9..7543534d9 100644 --- a/packages/backend/src/core/DownloadService.ts +++ b/packages/backend/src/core/DownloadService.ts @@ -6,7 +6,6 @@ import * as fs from 'node:fs'; import * as stream from 'node:stream/promises'; import { Inject, Injectable } from '@nestjs/common'; -import ipaddr from 'ipaddr.js'; import chalk from 'chalk'; import got, * as Got from 'got'; import { parse } from 'content-disposition'; @@ -70,13 +69,6 @@ export class DownloadService { }, enableUnixSockets: false, }).on('response', (res: Got.Response) => { - if ((process.env.NODE_ENV === 'production' || process.env.NODE_ENV === 'test') && !this.config.proxy && res.ip) { - if (this.isPrivateIp(res.ip)) { - this.logger.warn(`Blocked address: ${res.ip}`); - req.destroy(); - } - } - const contentLength = res.headers['content-length']; if (contentLength != null) { const size = Number(contentLength); @@ -139,18 +131,4 @@ export class DownloadService { cleanup(); } } - - @bindThis - private isPrivateIp(ip: string): boolean { - const parsedIp = ipaddr.parse(ip); - - for (const net of this.config.allowedPrivateNetworks ?? []) { - const cidr = ipaddr.parseCIDR(net); - if (cidr[0].kind() === parsedIp.kind() && parsedIp.match(ipaddr.parseCIDR(net))) { - return false; - } - } - - return parsedIp.range() !== 'unicast'; - } } diff --git a/packages/backend/src/core/EmailService.ts b/packages/backend/src/core/EmailService.ts index 1852536a8..832c2d55e 100644 --- a/packages/backend/src/core/EmailService.ts +++ b/packages/backend/src/core/EmailService.ts @@ -330,6 +330,7 @@ export class EmailService { Accept: 'application/json', Authorization: truemailAuthKey, }, + isLocalAddressAllowed: true, }); const json = (await res.json()) as { diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 4c1accae4..2034889a2 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -6,6 +6,7 @@ import * as http from 'node:http'; import * as https from 'node:https'; import * as net from 'node:net'; +import ipaddr from 'ipaddr.js'; import CacheableLookup from 'cacheable-lookup'; import fetch from 'node-fetch'; import { HttpProxyAgent, HttpsProxyAgent } from 'hpagent'; @@ -25,8 +26,102 @@ export type HttpRequestSendOptions = { validators?: ((res: Response) => void)[]; }; +declare module 'node:http' { + interface Agent { + createConnection(options: net.NetConnectOpts, callback?: (err: unknown, stream: net.Socket) => void): net.Socket; + } +} + +class HttpRequestServiceAgent extends http.Agent { + constructor( + private config: Config, + options?: http.AgentOptions, + ) { + super(options); + } + + @bindThis + public createConnection(options: net.NetConnectOpts, callback?: (err: unknown, stream: net.Socket) => void): net.Socket { + const socket = super.createConnection(options, callback) + .on('connect', () => { + const address = socket.remoteAddress; + if (process.env.NODE_ENV === 'production') { + if (address && ipaddr.isValid(address)) { + if (this.isPrivateIp(address)) { + socket.destroy(new Error(`Blocked address: ${address}`)); + } + } + } + }); + return socket; + }; + + @bindThis + private isPrivateIp(ip: string): boolean { + const parsedIp = ipaddr.parse(ip); + + for (const net of this.config.allowedPrivateNetworks ?? []) { + const cidr = ipaddr.parseCIDR(net); + if (cidr[0].kind() === parsedIp.kind() && parsedIp.match(ipaddr.parseCIDR(net))) { + return false; + } + } + + return parsedIp.range() !== 'unicast'; + } +} + +class HttpsRequestServiceAgent extends https.Agent { + constructor( + private config: Config, + options?: https.AgentOptions, + ) { + super(options); + } + + @bindThis + public createConnection(options: net.NetConnectOpts, callback?: (err: unknown, stream: net.Socket) => void): net.Socket { + const socket = super.createConnection(options, callback) + .on('connect', () => { + const address = socket.remoteAddress; + if (process.env.NODE_ENV === 'production') { + if (address && ipaddr.isValid(address)) { + if (this.isPrivateIp(address)) { + socket.destroy(new Error(`Blocked address: ${address}`)); + } + } + } + }); + return socket; + }; + + @bindThis + private isPrivateIp(ip: string): boolean { + const parsedIp = ipaddr.parse(ip); + + for (const net of this.config.allowedPrivateNetworks ?? []) { + const cidr = ipaddr.parseCIDR(net); + if (cidr[0].kind() === parsedIp.kind() && parsedIp.match(ipaddr.parseCIDR(net))) { + return false; + } + } + + return parsedIp.range() !== 'unicast'; + } +} + @Injectable() export class HttpRequestService { + /** + * Get http non-proxy agent (without local address filtering) + */ + private httpNative: http.Agent; + + /** + * Get https non-proxy agent (without local address filtering) + */ + private httpsNative: https.Agent; + /** * Get http non-proxy agent */ @@ -57,19 +152,20 @@ export class HttpRequestService { lookup: false, // nativeのdns.lookupにfallbackしない }); - this.http = new http.Agent({ + const agentOption = { keepAlive: true, keepAliveMsecs: 30 * 1000, lookup: cache.lookup as unknown as net.LookupFunction, localAddress: config.outgoingAddress, - }); + }; - this.https = new https.Agent({ - keepAlive: true, - keepAliveMsecs: 30 * 1000, - lookup: cache.lookup as unknown as net.LookupFunction, - localAddress: config.outgoingAddress, - }); + this.httpNative = new http.Agent(agentOption); + + this.httpsNative = new https.Agent(agentOption); + + this.http = new HttpRequestServiceAgent(config, agentOption); + + this.https = new HttpsRequestServiceAgent(config, agentOption); const maxSockets = Math.max(256, config.deliverJobConcurrency ?? 128); @@ -104,16 +200,22 @@ export class HttpRequestService { * @param bypassProxy Allways bypass proxy */ @bindThis - public getAgentByUrl(url: URL, bypassProxy = false): http.Agent | https.Agent { + public getAgentByUrl(url: URL, bypassProxy = false, isLocalAddressAllowed = false): http.Agent | https.Agent { if (bypassProxy || (this.config.proxyBypassHosts ?? []).includes(url.hostname)) { + if (isLocalAddressAllowed) { + return url.protocol === 'http:' ? this.httpNative : this.httpsNative; + } return url.protocol === 'http:' ? this.http : this.https; } else { + if (isLocalAddressAllowed && (!this.config.proxy)) { + return url.protocol === 'http:' ? this.httpNative : this.httpsNative; + } return url.protocol === 'http:' ? this.httpAgent : this.httpsAgent; } } @bindThis - public async getActivityJson(url: string): Promise { + public async getActivityJson(url: string, isLocalAddressAllowed = false): Promise { const res = await this.send(url, { method: 'GET', headers: { @@ -121,6 +223,7 @@ export class HttpRequestService { }, timeout: 5000, size: 1024 * 256, + isLocalAddressAllowed: isLocalAddressAllowed, }, { throwErrorWhenResponseNotOk: true, validators: [validateContentTypeSetAsActivityPub], @@ -135,7 +238,7 @@ export class HttpRequestService { } @bindThis - public async getJson(url: string, accept = 'application/json, */*', headers?: Record): Promise { + public async getJson(url: string, accept = 'application/json, */*', headers?: Record, isLocalAddressAllowed = false): Promise { const res = await this.send(url, { method: 'GET', headers: Object.assign({ @@ -143,19 +246,21 @@ export class HttpRequestService { }, headers ?? {}), timeout: 5000, size: 1024 * 256, + isLocalAddressAllowed: isLocalAddressAllowed, }); return await res.json() as T; } @bindThis - public async getHtml(url: string, accept = 'text/html, */*', headers?: Record): Promise { + public async getHtml(url: string, accept = 'text/html, */*', headers?: Record, isLocalAddressAllowed = false): Promise { const res = await this.send(url, { method: 'GET', headers: Object.assign({ Accept: accept, }, headers ?? {}), timeout: 5000, + isLocalAddressAllowed: isLocalAddressAllowed, }); return await res.text(); @@ -170,6 +275,7 @@ export class HttpRequestService { headers?: Record, timeout?: number, size?: number, + isLocalAddressAllowed?: boolean, } = {}, extra: HttpRequestSendOptions = { throwErrorWhenResponseNotOk: true, @@ -184,6 +290,7 @@ export class HttpRequestService { }, timeout); const bearcaps = url.startsWith('bear:?') ? this.parseBearcaps(url) : undefined; + const isLocalAddressAllowed = args.isLocalAddressAllowed ?? false; const res = await fetch(bearcaps?.url ?? url, { method: args.method ?? 'GET', @@ -194,7 +301,7 @@ export class HttpRequestService { }, body: args.body, size: args.size ?? 10 * 1024 * 1024, - agent: (url) => this.getAgentByUrl(url), + agent: (url) => this.getAgentByUrl(url, false, isLocalAddressAllowed), signal: controller.signal, }); diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index 4d8f212d8..f2be28de0 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -164,6 +164,6 @@ export class UrlPreviewService { contentLengthRequired: meta.urlPreviewRequireContentLength, }); - return this.httpRequestService.getJson(`${proxy}?${queryStr}`); + return this.httpRequestService.getJson(`${proxy}?${queryStr}`, 'application/json, */*', undefined, true); } } From 710e719fc51ccc646efe58af1f01933ac9eb3e60 Mon Sep 17 00:00:00 2001 From: syuilo <4439005+syuilo@users.noreply.github.com> Date: Thu, 21 Nov 2024 09:25:18 +0900 Subject: [PATCH 15/20] fix ap/show (cherry picked from commit 0f59adc436f80c495b4404807b0bd645da2b1db8) --- packages/backend/src/server/api/endpoints/ap/show.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/server/api/endpoints/ap/show.ts b/packages/backend/src/server/api/endpoints/ap/show.ts index 7cfd398d4..1aa405b66 100644 --- a/packages/backend/src/server/api/endpoints/ap/show.ts +++ b/packages/backend/src/server/api/endpoints/ap/show.ts @@ -144,7 +144,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, resolver) : null, ); } From ed682451771f2487ef44498186ac73f6576c9578 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=8B=E3=81=A3=E3=81=93=E3=81=8B=E3=82=8A?= <67428053+kakkokari-gtyih@users.noreply.github.com> Date: Thu, 21 Nov 2024 10:30:30 +0900 Subject: [PATCH 16/20] fix(backend): fix security patches (#15008) (cherry picked from commit 53e827b18c46f786268278645206404ff2d95f72) --- packages/backend/src/core/activitypub/ApInboxService.ts | 2 +- packages/backend/src/server/api/endpoints/ap/show.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index b6c7877a8..2fbd91efc 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -131,7 +131,7 @@ export class ApInboxService { if (actor.uri) { if (actor.lastFetchedAt == null || Date.now() - actor.lastFetchedAt.getTime() > 1000 * 60 * 60 * 24) { setImmediate(() => { - this.apPersonService.updatePerson(actor.uri); + this.apPersonService.updatePerson(actor.uri, resolver); }); } } diff --git a/packages/backend/src/server/api/endpoints/ap/show.ts b/packages/backend/src/server/api/endpoints/ap/show.ts index 1aa405b66..d13994e6c 100644 --- a/packages/backend/src/server/api/endpoints/ap/show.ts +++ b/packages/backend/src/server/api/endpoints/ap/show.ts @@ -143,8 +143,8 @@ 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, resolver) : null, + isActor(object) ? await this.apPersonService.createPerson(getApId(object), resolver) : null, + isPost(object) ? await this.apNoteService.createNote(getApId(object), undefined, resolver, true) : null, ); } From d1b953b15c9b3e85f4845e66229dbfcbe2499f31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=8B=E3=81=A3=E3=81=93=E3=81=8B=E3=82=8A?= <67428053+kakkokari-gtyih@users.noreply.github.com> Date: Thu, 21 Nov 2024 12:10:02 +0900 Subject: [PATCH 17/20] fix(backend): fix type error(s) in security fixes (#15009) * Fix type error in security fixes (cherry picked from commit fa3cf6c2996741e642955c5e2fca8ad785e83205) * Fix error in test function calls (cherry picked from commit 1758f29364eca3cbd13dbb5c84909c93712b3b3b) * Fix style error (cherry picked from commit 23c4aa25714af145098baa7edd74c1d217e51c1a) * Fix another style error (cherry picked from commit 36af07abe28bec670aaebf9f5af5694bb582c29a) * Fix `.punyHost` misuse (cherry picked from commit 6027b516e1c82324d55d6e54d0e17cbd816feb42) * attempt to fix test: make yaml valid --------- Co-authored-by: Julia Johannesen (cherry picked from commit 3a6c2aa83563515b2ce02cda289b0271d992e84e) --- packages/backend/src/core/HttpRequestService.ts | 12 ++++++------ .../backend/src/core/RemoteUserResolveService.ts | 2 +- .../core/activitypub/models/ApPersonService.ts | 15 +++++++++------ packages/backend/test/unit/activitypub.ts | 4 ++-- 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 2034889a2..e3640216f 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -54,19 +54,19 @@ class HttpRequestServiceAgent extends http.Agent { } }); return socket; - }; + } @bindThis private isPrivateIp(ip: string): boolean { const parsedIp = ipaddr.parse(ip); - + for (const net of this.config.allowedPrivateNetworks ?? []) { const cidr = ipaddr.parseCIDR(net); if (cidr[0].kind() === parsedIp.kind() && parsedIp.match(ipaddr.parseCIDR(net))) { return false; } } - + return parsedIp.range() !== 'unicast'; } } @@ -93,19 +93,19 @@ class HttpsRequestServiceAgent extends https.Agent { } }); return socket; - }; + } @bindThis private isPrivateIp(ip: string): boolean { const parsedIp = ipaddr.parse(ip); - + for (const net of this.config.allowedPrivateNetworks ?? []) { const cidr = ipaddr.parseCIDR(net); if (cidr[0].kind() === parsedIp.kind() && parsedIp.match(ipaddr.parseCIDR(net))) { return false; } } - + return parsedIp.range() !== 'unicast'; } } diff --git a/packages/backend/src/core/RemoteUserResolveService.ts b/packages/backend/src/core/RemoteUserResolveService.ts index 40cb41343..c7f5effd2 100644 --- a/packages/backend/src/core/RemoteUserResolveService.ts +++ b/packages/backend/src/core/RemoteUserResolveService.ts @@ -54,7 +54,7 @@ export class RemoteUserResolveService { }) as MiLocalUser; } - host = this.utilityService.punyHost(host); + host = this.utilityService.toPuny(host); if (host === this.utilityService.toPuny(this.config.host)) { this.logger.info(`return local user: ${usernameLower}`); diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index 634d3f25f..88c2dc5d2 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -164,13 +164,16 @@ export class ApPersonService implements OnModuleInit { } 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`); + const xCollection = (x as IActor)[collection]; + if (xCollection != null) { + const collectionUri = getApId(xCollection); + 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}`); } - } else if (collectionUri != null) { - throw new Error(`invalid Actor: wrong ${collection}`); } } diff --git a/packages/backend/test/unit/activitypub.ts b/packages/backend/test/unit/activitypub.ts index e32096ef5..155ec671a 100644 --- a/packages/backend/test/unit/activitypub.ts +++ b/packages/backend/test/unit/activitypub.ts @@ -173,7 +173,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'); @@ -333,7 +333,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); }); }); From e3cad435b8c281414d48ab7a52ca10309e6cd293 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=8B=E3=81=A3=E3=81=93=E3=81=8B=E3=82=8A?= <67428053+kakkokari-gtyih@users.noreply.github.com> Date: Thu, 21 Nov 2024 14:36:24 +0900 Subject: [PATCH 18/20] fix(backend): fix apResolver (#15010) * fix(backend): fix apResolver * fix * add comments * tweak comment (cherry picked from commit c1f19fad1e7e1717898b37bbb4e863e0f26b306b) --- packages/backend/src/core/activitypub/ApInboxService.ts | 3 ++- packages/backend/src/server/api/endpoints/ap/show.ts | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index 2fbd91efc..cb06b7faa 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -131,7 +131,8 @@ export class ApInboxService { if (actor.uri) { if (actor.lastFetchedAt == null || Date.now() - actor.lastFetchedAt.getTime() > 1000 * 60 * 60 * 24) { setImmediate(() => { - this.apPersonService.updatePerson(actor.uri, resolver); + // 同一ユーザーの情報を再度処理するので、使用済みのresolverを再利用してはいけない + this.apPersonService.updatePerson(actor.uri); }); } } diff --git a/packages/backend/src/server/api/endpoints/ap/show.ts b/packages/backend/src/server/api/endpoints/ap/show.ts index d13994e6c..cc7e24515 100644 --- a/packages/backend/src/server/api/endpoints/ap/show.ts +++ b/packages/backend/src/server/api/endpoints/ap/show.ts @@ -141,10 +141,11 @@ export default class extends Endpoint { // eslint- if (local != null) return local; } + // 同一ユーザーの情報を再度処理するので、使用済みのresolverを再利用してはいけない return await this.mergePack( me, - isActor(object) ? await this.apPersonService.createPerson(getApId(object), resolver) : null, - isPost(object) ? await this.apNoteService.createNote(getApId(object), undefined, resolver, true) : null, + isActor(object) ? await this.apPersonService.createPerson(getApId(object)) : null, + isPost(object) ? await this.apNoteService.createNote(getApId(object), undefined, undefined, true) : null, ); } From c6b6aab90e4be6bb866bac1992bdb3633654439b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=8B=E3=81=A3=E3=81=93=E3=81=8B=E3=82=8A?= <67428053+kakkokari-gtyih@users.noreply.github.com> Date: Fri, 22 Nov 2024 12:14:41 +0900 Subject: [PATCH 19/20] =?UTF-8?q?fix(backend):=20Inbox=E3=81=AE=E3=82=A8?= =?UTF-8?q?=E3=83=A9=E3=83=BC=E3=82=92throw=E3=81=9B=E3=81=9Areturn?= =?UTF-8?q?=E3=81=97=E3=81=A6=E3=81=84=E3=82=8B=E5=95=8F=E9=A1=8C=E3=82=92?= =?UTF-8?q?=E4=BF=AE=E6=AD=A3=20(#15022)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix exception handling for Like activities (cherry picked from commit 8f42e8434eaebe3aba5d1980c57f49dd8ad0de91) * fix exception handling for Announce activities (cherry picked from commit cfc3ab4b045af0674122fa49176431860176358b) * fix exception handling for Undo activities * Update Changelog --------- Co-authored-by: Hazelnoot (cherry picked from commit f25fc5215bd03b9405b257fc8b8b1d7df7ea33b3) --- .../backend/src/core/activitypub/ApInboxService.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index cb06b7faa..4d0342752 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -29,6 +29,7 @@ import { bindThis } from '@/decorators.js'; import type { MiLocalUser, MiRemoteUser } from '@/models/User.js'; import { isNotNull } from '@/misc/is-not-null.js'; import { GlobalEventService } from '@/core/GlobalEventService.js'; +import { IdentifiableError } from '@/misc/identifiable-error.js'; import { getApHrefNullable, getApId, getApIds, getApType, isAccept, isActor, isAdd, isAnnounce, isBlock, isCollection, isCollectionOrOrderedCollection, isCreate, isDelete, isFlag, isFollow, isLike, isMove, isPost, isReject, isRemove, isTombstone, isUndo, isUpdate, validActor, validPost } from './type.js'; import { ApNoteService } from './models/ApNoteService.js'; import { ApLoggerService } from './ApLoggerService.js'; @@ -203,13 +204,16 @@ export class ApInboxService { await this.apNoteService.extractEmojis(activity.tag ?? [], actor.host).catch(() => null); - return await this.reactionService.create(actor, note, activity._misskey_reaction ?? activity.content ?? activity.name).catch(err => { - if (err.id === '51c42bb4-931a-456b-bff7-e5a8a70dd298') { + try { + await this.reactionService.create(actor, note, activity._misskey_reaction ?? activity.content ?? activity.name); + return 'ok'; + } catch (err) { + if (err instanceof IdentifiableError && err.id === '51c42bb4-931a-456b-bff7-e5a8a70dd298') { return 'skip: already reacted'; } else { throw err; } - }).then(() => 'ok'); + } } @bindThis From d9ed76384964dd2a9904d68a5b8cde584b4ed191 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=82=E3=82=8F=E3=82=8F=E3=82=8F=E3=81=A8=E3=83=BC?= =?UTF-8?q?=E3=81=AB=E3=82=85?= <17376330+u1-liquid@users.noreply.github.com> Date: Wed, 25 Dec 2024 04:34:04 +0900 Subject: [PATCH 20/20] fix merge failure --- packages/backend/src/core/activitypub/models/ApNoteService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/core/activitypub/models/ApNoteService.ts b/packages/backend/src/core/activitypub/models/ApNoteService.ts index bd00529da..3ce588e18 100644 --- a/packages/backend/src/core/activitypub/models/ApNoteService.ts +++ b/packages/backend/src/core/activitypub/models/ApNoteService.ts @@ -183,7 +183,7 @@ export class ApNoteService { // 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', `User ${cachedActor.id} has been suspended.`); + throw new IdentifiableError('85ab9bd7-3a41-4530-959d-f07073900109', `User ${actor.id} has been suspended.`); } const apMentions = await this.apMentionService.extractApMentions(note.tag, resolver);