From 720a5de945fd95ac720d8121f4ebefc45c6b9735 Mon Sep 17 00:00:00 2001 From: Jannis Braun <151788261+TheZwiss@users.noreply.github.com> Date: Tue, 31 Mar 2026 19:16:48 +0200 Subject: [PATCH] fix(federation): add strict origin enforcement for user attribution (FED-010) Prevent malicious peers from forging events attributed to users on other instances. Every relay event processor now verifies the acting user's homeInstance (from payload) matches X-Federation-Origin (from HMAC-verified header) via verifyAttribution(), normalized to bare domain. - Add verifyAttribution() helper using extractDomain normalization - Guard all 13 event processors before any user resolution or DB writes - Add homeInstance to FederationRelayReaction type + outbound payloads - Replace unnormalized string equality in friend handlers - Log mismatched values on rejection for debugging --- docs/systems/federation.md | 7 +- packages/server/src/routes/federation.ts | 107 +++++++++++++++++++---- packages/server/src/ws/events.ts | 4 + packages/shared/src/types.ts | 1 + 4 files changed, 101 insertions(+), 18 deletions(-) diff --git a/docs/systems/federation.md b/docs/systems/federation.md index 633fb9c9..e6b94c4d 100644 --- a/docs/systems/federation.md +++ b/docs/systems/federation.md @@ -211,6 +211,11 @@ Locations where normalization is applied: - `dm.ts:655` -- `isLocalMember` broadcast filter checks both formats - `dm.ts:743` -- normalizes target homeInstance before peer origin comparison +**Attribution verification (`verifyAttribution`):** +- `verifyAttribution(actingUserHomeInstance, sourceInstance)` normalizes both via `extractDomain` and compares +- Applied as the FIRST check in every relay event processor (13 handlers) — before user resolution or DB writes +- Prevents malicious peers from forging events attributed to users on other instances (FED-010) + **Locations with potential mismatch (see Known Issues):** - `federation.ts:1278` -- `memberUser?.homeInstance === sourceInstance` -- compares stored homeInstance (possibly bare domain) against `sourceInstance` (full URL from relay request header) - `federationOutbox.ts:376-379` -- `getFriendEventTargets` compares `fromHomeInstance` against `ourOrigin` without normalization. The passed values come from `user.homeInstance || domainOrigin` where `domainOrigin = getOurOrigin()`. If `homeInstance` is a bare domain, `homeInstance !== ourOrigin` is true, so the bare domain gets added to targets, but `queueOutboxEvent` then fails to match it against `federation_peers.origin` @@ -848,7 +853,7 @@ The third case is the most dangerous -- it looks like the event was queued but n | Threat | Mitigation | Gap | |--------|-----------|-----| | Peer impersonation | `X-Federation-Origin` is verified against `federation_peers.origin` | An attacker who compromises the HMAC secret can impersonate the peer | -| User attribution fraud | Authority checks: e.g., `from.homeInstance !== sourceInstance` rejects events where the acting user doesn't belong to the source instance | The check is string equality on `homeInstance` from the payload, which the sender controls. A malicious peer could claim any user belongs to them by setting `homeInstance` to their own origin. | +| User attribution fraud | `verifyAttribution()` guard on every event processor: acting user's `homeInstance` (from payload) is normalized to bare domain via `extractDomain` and compared against `X-Federation-Origin` (from HMAC-verified header). Rejects with `attribution_mismatch` before any user resolution or DB writes. Reaction events include `homeInstance` in the payload for verification. | A compromised peer can still forge content from its own users, but cannot impersonate users from other instances. | | Event flooding | Outbox batches limited to 50 events. `/api/federation/peer/accept` rate-limited to 10/min/IP. `/api/federation/relay` rate-limited to 30/min/peer (sliding window, keyed by `peer.origin`). | A sustained attack from multiple compromised peers could still cause load, but individual peers are throttled. | | Replay attacks | 15-minute timestamp window + per-request nonce (UUID v4) in HMAC, in-memory dedup store with TTL, auto-ratchet enforcement | Nonces are in-memory only — a server restart clears the store, allowing replays of requests from the last 15 minutes of the previous session. Acceptable given the narrow window and idempotency of most events. | | Message content manipulation | None | A compromised peer can forge message content attributed to any user on their instance | diff --git a/packages/server/src/routes/federation.ts b/packages/server/src/routes/federation.ts index b9b47ef8..1d27ad9b 100644 --- a/packages/server/src/routes/federation.ts +++ b/packages/server/src/routes/federation.ts @@ -780,9 +780,9 @@ export async function federationRoutes(app: FastifyInstance): Promise { if (mutationType === 'reaction_add' || mutationType === 'reaction_remove') { // Use the stored payload from the mutation log if (mutation.payload) { - let reactionData: { userId: string; homeUserId: string; emoji: string; createdAt?: number } | null = null; + let reactionData: { userId: string; homeUserId: string; homeInstance?: string; emoji: string; createdAt?: number } | null = null; try { - reactionData = JSON.parse(mutation.payload) as { userId: string; homeUserId: string; emoji: string; createdAt?: number }; + reactionData = JSON.parse(mutation.payload) as { userId: string; homeUserId: string; homeInstance?: string; emoji: string; createdAt?: number }; } catch { // Skip malformed payload continue; @@ -797,6 +797,7 @@ export async function federationRoutes(app: FastifyInstance): Promise { reaction: { userId: reactionData.userId, homeUserId: reactionData.homeUserId, + homeInstance: reactionData.homeInstance || getOurOrigin(), emoji: reactionData.emoji, createdAt: reactionData.createdAt ?? mutation.mutated_at, }, @@ -992,6 +993,15 @@ export function extractDomain(homeInstance: string): string { } } +/** + * Verify that an acting user's homeInstance matches the source instance (X-Federation-Origin). + * In direct S2S federation, a peer should only send events for its own users. + * Both sides are normalized to bare domain before comparison. + */ +export function verifyAttribution(actingUserHomeInstance: string, sourceInstance: string): boolean { + return extractDomain(actingUserHomeInstance) === extractDomain(sourceInstance); +} + /** * Resolve a home user ID to a local user. * Matches users where home_user_id = homeUserId, or where @@ -1301,6 +1311,13 @@ function processCreateEvent( return; } + // Attribution: message author must belong to source instance (FED-010) + if (!verifyAttribution(event.message.homeInstance, sourceInstance)) { + console.warn(`[federation] Attribution mismatch in create: message homeInstance=${extractDomain(event.message.homeInstance)} source=${extractDomain(sourceInstance)}`); + rejected.push({ messageId: event.messageId, reason: 'attribution_mismatch' }); + return; + } + // Dedup: check for existing message with same source const existingMsg = db .select() @@ -1494,6 +1511,13 @@ function processUpdateEvent( accepted: string[], rejected: Array<{ messageId: string; reason: string }>, ): void { + // Attribution: if homeInstance present, verify it matches source (FED-010) + if (event.message?.homeInstance && !verifyAttribution(event.message.homeInstance, sourceInstance)) { + console.warn(`[federation] Attribution mismatch in update: message homeInstance=${extractDomain(event.message.homeInstance)} source=${extractDomain(sourceInstance)}`); + rejected.push({ messageId: event.messageId, reason: 'attribution_mismatch' }); + return; + } + const localMsg = db .select() .from(schema.dmMessages) @@ -1590,6 +1614,7 @@ function processDeleteEvent( accepted: string[], rejected: Array<{ messageId: string; reason: string }>, ): void { + // FED-010: delete is safe by design — lookup scoped to sourceInstance+sourceMessageId const localMsg = db .select() .from(schema.dmMessages) @@ -1689,6 +1714,13 @@ function processReactionAddEvent( return; } + // Attribution: reacting user must belong to source instance (FED-010) + if (!event.reaction.homeInstance || !verifyAttribution(event.reaction.homeInstance, sourceInstance)) { + console.warn(`[federation] Attribution mismatch in reaction_add: reaction homeInstance=${event.reaction.homeInstance ? extractDomain(event.reaction.homeInstance) : 'missing'} source=${extractDomain(sourceInstance)}`); + rejected.push({ messageId: event.messageId, reason: 'attribution_mismatch' }); + return; + } + const canonicalMessageId = event.reaction.messageId ?? event.messageId; const localMsg = resolveLocalDmMessage( canonicalMessageId, @@ -1770,6 +1802,13 @@ function processReactionRemoveEvent( return; } + // Attribution: reacting user must belong to source instance (FED-010) + if (!event.reaction.homeInstance || !verifyAttribution(event.reaction.homeInstance, sourceInstance)) { + console.warn(`[federation] Attribution mismatch in reaction_remove: reaction homeInstance=${event.reaction.homeInstance ? extractDomain(event.reaction.homeInstance) : 'missing'} source=${extractDomain(sourceInstance)}`); + rejected.push({ messageId: event.messageId, reason: 'attribution_mismatch' }); + return; + } + const canonicalMessageId = event.reaction.messageId ?? event.messageId; const localMsg = resolveLocalDmMessage( canonicalMessageId, @@ -1838,6 +1877,13 @@ function processMemberAddEvent( // Bootstrap: channel doesn't exist yet — create from group metadata if (!channel && event.group) { + // Attribution: only the owner's instance can bootstrap a group (FED-010) + if (event.group.owner && !verifyAttribution(event.group.owner.homeInstance, sourceInstance)) { + console.warn(`[federation] Attribution mismatch in member_add bootstrap: owner homeInstance=${extractDomain(event.group.owner.homeInstance)} source=${extractDomain(sourceInstance)}`); + rejected.push({ messageId: event.messageId, reason: 'attribution_mismatch' }); + return; + } + const channelId = generateSnowflake(); const now = Date.now(); @@ -1929,6 +1975,13 @@ function processMemberAddEvent( return; } + // Attribution: adder must belong to source instance (FED-010) + if (event.membership.addedBy && !verifyAttribution(event.membership.addedBy.homeInstance, sourceInstance)) { + console.warn(`[federation] Attribution mismatch in member_add: addedBy homeInstance=${extractDomain(event.membership.addedBy.homeInstance)} source=${extractDomain(sourceInstance)}`); + rejected.push({ messageId: event.messageId, reason: 'attribution_mismatch' }); + return; + } + // Cancel soft-delete if channel was pending GC if (channel.deletedAt) { db.update(schema.dmChannels) @@ -2035,6 +2088,13 @@ function processMemberRemoveEvent( return; } + // Attribution: for self-leave, user must belong to source instance (FED-010) + if (event.membership.reason === 'leave' && !verifyAttribution(event.membership.user.homeInstance, sourceInstance)) { + console.warn(`[federation] Attribution mismatch in member_remove: user homeInstance=${extractDomain(event.membership.user.homeInstance)} source=${extractDomain(sourceInstance)}`); + rejected.push({ messageId: event.messageId, reason: 'attribution_mismatch' }); + return; + } + const channel = db .select() .from(schema.dmChannels) @@ -2148,6 +2208,13 @@ function processOwnershipTransferEvent( return; } + // Attribution: previous owner must belong to source instance (FED-010) + if (event.ownership.previousOwner && !verifyAttribution(event.ownership.previousOwner.homeInstance, sourceInstance)) { + console.warn(`[federation] Attribution mismatch in ownership_transfer: previousOwner homeInstance=${extractDomain(event.ownership.previousOwner.homeInstance)} source=${extractDomain(sourceInstance)}`); + rejected.push({ messageId: event.messageId, reason: 'attribution_mismatch' }); + return; + } + const channel = db .select() .from(schema.dmChannels) @@ -2295,9 +2362,10 @@ function processFriendRequestCreateEvent( const { from, to } = event.friendship; - // Authority check: the sender's home instance must be the source - if (from.homeInstance !== sourceInstance) { - rejected.push({ messageId: event.messageId, reason: 'unauthorized_source' }); + // Attribution: sender must belong to source instance (FED-010) + if (!verifyAttribution(from.homeInstance, sourceInstance)) { + console.warn(`[federation] Attribution mismatch in friend_request_create: from homeInstance=${extractDomain(from.homeInstance)} source=${extractDomain(sourceInstance)}`); + rejected.push({ messageId: event.messageId, reason: 'attribution_mismatch' }); return; } @@ -2391,9 +2459,10 @@ function processFriendRequestUpdateEvent( const { from, to, status } = event.friendship; - // Authority check: the recipient's instance accepts/declines - if (to.homeInstance !== sourceInstance) { - rejected.push({ messageId: event.messageId, reason: 'unauthorized_source' }); + // Attribution: recipient (acceptor/decliner) must belong to source instance (FED-010) + if (!verifyAttribution(to.homeInstance, sourceInstance)) { + console.warn(`[federation] Attribution mismatch in friend_request_update: to homeInstance=${extractDomain(to.homeInstance)} source=${extractDomain(sourceInstance)}`); + rejected.push({ messageId: event.messageId, reason: 'attribution_mismatch' }); return; } @@ -2467,9 +2536,10 @@ function processFriendRequestCancelEvent( const { from, to } = event.friendship; - // Authority check: the sender cancels their own request - if (from.homeInstance !== sourceInstance) { - rejected.push({ messageId: event.messageId, reason: 'unauthorized_source' }); + // Attribution: sender must belong to source instance (FED-010) + if (!verifyAttribution(from.homeInstance, sourceInstance)) { + console.warn(`[federation] Attribution mismatch in friend_request_cancel: from homeInstance=${extractDomain(from.homeInstance)} source=${extractDomain(sourceInstance)}`); + rejected.push({ messageId: event.messageId, reason: 'attribution_mismatch' }); return; } @@ -2531,9 +2601,10 @@ function processFriendAddEvent( const { from, to } = event.friendship; - // Authority check: the recipient's instance creates the friendship - if (to.homeInstance !== sourceInstance) { - rejected.push({ messageId: event.messageId, reason: 'unauthorized_source' }); + // Attribution: acceptor must belong to source instance (FED-010) + if (!verifyAttribution(to.homeInstance, sourceInstance)) { + console.warn(`[federation] Attribution mismatch in friend_add: to homeInstance=${extractDomain(to.homeInstance)} source=${extractDomain(sourceInstance)}`); + rejected.push({ messageId: event.messageId, reason: 'attribution_mismatch' }); return; } @@ -2617,9 +2688,10 @@ function processFriendRemoveEvent( const { from, to } = event.friendship; - // Authority check: either side can unfriend - if (from.homeInstance !== sourceInstance && to.homeInstance !== sourceInstance) { - rejected.push({ messageId: event.messageId, reason: 'unauthorized_source' }); + // Attribution: at least one side must belong to source instance (FED-010) + if (!verifyAttribution(from.homeInstance, sourceInstance) && !verifyAttribution(to.homeInstance, sourceInstance)) { + console.warn(`[federation] Attribution mismatch in friend_remove: from homeInstance=${extractDomain(from.homeInstance)} to homeInstance=${extractDomain(to.homeInstance)} source=${extractDomain(sourceInstance)}`); + rejected.push({ messageId: event.messageId, reason: 'attribution_mismatch' }); return; } @@ -2664,6 +2736,7 @@ function processFileRejectedEvent( accepted: string[], rejected: Array<{ messageId: string; reason: string }>, ): void { + // FED-010: file_rejected is a system event from the rejecting peer — no user attribution to verify if (!event.attachmentId || !event.rejectionReason) { rejected.push({ messageId: event.messageId, reason: 'missing_file_rejected_payload' }); return; diff --git a/packages/server/src/ws/events.ts b/packages/server/src/ws/events.ts index edcc9816..768dda4e 100644 --- a/packages/server/src/ws/events.ts +++ b/packages/server/src/ws/events.ts @@ -1134,6 +1134,7 @@ function handleReactionAdd(event: Record, userId: string): void appendMutationLog(messageId, dmMsg.dmChannelId, 'reaction_add', JSON.stringify({ userId, homeUserId: reactionUser?.homeUserId || userId, + homeInstance: reactionUser?.homeInstance || getOurOrigin(), emoji, createdAt: now, })); @@ -1144,6 +1145,7 @@ function handleReactionAdd(event: Record, userId: string): void messageHomeInstance, userId, homeUserId: reactionUser?.homeUserId || userId, + homeInstance: reactionUser?.homeInstance || getOurOrigin(), emoji, createdAt: now, }, @@ -1213,6 +1215,7 @@ function handleReactionRemove(event: Record, userId: string): v appendMutationLog(messageId, dmMsg.dmChannelId, 'reaction_remove', JSON.stringify({ userId, homeUserId: removingUser?.homeUserId || userId, + homeInstance: removingUser?.homeInstance || getOurOrigin(), emoji, })); const reactionRemoveTargetOrigins = getGroupDmTargetOrigins(dmMsg.dmChannelId); @@ -1226,6 +1229,7 @@ function handleReactionRemove(event: Record, userId: string): v messageHomeInstance, userId, homeUserId: removingUser?.homeUserId || userId, + homeInstance: removingUser?.homeInstance || getOurOrigin(), emoji, }, }), diff --git a/packages/shared/src/types.ts b/packages/shared/src/types.ts index 4045d76c..e7a9e40e 100644 --- a/packages/shared/src/types.ts +++ b/packages/shared/src/types.ts @@ -819,6 +819,7 @@ export interface FederationRelayReaction { messageHomeInstance?: string; userId: string; homeUserId: string; + homeInstance: string; emoji: string; createdAt: number; }