From c6f0e6f25dc4ef6392ea5b6cb86d0b16a832835e Mon Sep 17 00:00:00 2001 From: Jannis Braun <151788261+TheZwiss@users.noreply.github.com> Date: Thu, 2 Jul 2026 12:56:26 +0200 Subject: [PATCH] fix(federation): initiator handles 409 + verifies handshake before activating (BUG-1b/BUG-2) --- .../src/routes/federation.peerAccept.test.ts | 41 ++++--- .../federation.peerInitiateOutbound.test.ts | 112 +++++++++++++++++- packages/server/src/routes/federation.ts | 56 ++++++--- .../utils/federationEpochHandshake.test.ts | 60 +++++++--- 4 files changed, 220 insertions(+), 49 deletions(-) diff --git a/packages/server/src/routes/federation.peerAccept.test.ts b/packages/server/src/routes/federation.peerAccept.test.ts index d26433dd..41880af9 100644 --- a/packages/server/src/routes/federation.peerAccept.test.ts +++ b/packages/server/src/routes/federation.peerAccept.test.ts @@ -8,9 +8,31 @@ import path from 'node:path'; import { fileURLToPath } from 'node:url'; import * as schema from '../db/schema.js'; import { setWorkerId } from '../utils/snowflake.js'; +import { buildFederationHeaders } from '../utils/federationAuth.js'; setWorkerId(1); +// A URL-aware outbound fetch stub for /peer/initiate: /peer/accept returns +// `acceptBody` (200); /api/federation/epoch signs its response with the secret +// the initiator just sent in /peer/accept, so fetchPeerEpoch's real signature +// check passes and the handshake verifies (status → active). +function initiateFetchStub(acceptBody: Record): typeof globalThis.fetch { + let capturedSecret = ''; + return (async (url: string | URL | Request, init?: RequestInit): Promise => { + const u = String(url); + if (u.endsWith('/api/federation/peer/accept')) { + capturedSecret = (JSON.parse(String(init?.body)) as { hmacSecret: string }).hmacSecret; + return new Response(JSON.stringify(acceptBody), { status: 200, headers: { 'content-type': 'application/json' } }); + } + if (u.endsWith('/api/federation/epoch')) { + const body = JSON.stringify({ instanceId: 'remote-epoch' }); + const headers = buildFederationHeaders(body, capturedSecret, 'https://remote.example'); + return new Response(body, { status: 200, headers }); + } + throw new Error(`unexpected fetch ${u}`); + }) as typeof globalThis.fetch; +} + const __dirname = path.dirname(fileURLToPath(import.meta.url)); type TestDb = ReturnType>; @@ -473,13 +495,8 @@ describe('POST /api/federation/peer/initiate — persists remote instanceName fr vi.unstubAllGlobals(); }); - it('writes remote.instanceName when remote /peer/accept succeeds', async () => { - vi.stubGlobal('fetch', vi.fn(async () => - new Response(JSON.stringify({ accepted: true, instanceName: 'Remote Backspace' }), { - status: 200, - headers: { 'content-type': 'application/json' }, - }), - )); + it('writes remote.instanceName when remote /peer/accept succeeds (and epoch verifies)', async () => { + vi.stubGlobal('fetch', initiateFetchStub({ accepted: true, instanceName: 'Remote Backspace', instanceId: 'remote-epoch' })); const response = await app.inject({ method: 'POST', @@ -494,13 +511,8 @@ describe('POST /api/federation/peer/initiate — persists remote instanceName fr expect(row?.instanceName).toBe('Remote Backspace'); }); - it('writes null instanceName when remote response omits the field', async () => { - vi.stubGlobal('fetch', vi.fn(async () => - new Response(JSON.stringify({ accepted: true }), { - status: 200, - headers: { 'content-type': 'application/json' }, - }), - )); + it('writes null instanceName when remote response omits the field (and epoch verifies)', async () => { + vi.stubGlobal('fetch', initiateFetchStub({ accepted: true, instanceId: 'remote-epoch' })); const response = await app.inject({ method: 'POST', @@ -511,6 +523,7 @@ describe('POST /api/federation/peer/initiate — persists remote instanceName fr expect(response.statusCode).toBe(200); const row = testDb.select().from(schema.federationPeers) .where(eq(schema.federationPeers.origin, 'https://remote.example')).get(); + expect(row?.status).toBe('active'); expect(row?.instanceName).toBeNull(); }); }); diff --git a/packages/server/src/routes/federation.peerInitiateOutbound.test.ts b/packages/server/src/routes/federation.peerInitiateOutbound.test.ts index e23930b6..74925525 100644 --- a/packages/server/src/routes/federation.peerInitiateOutbound.test.ts +++ b/packages/server/src/routes/federation.peerInitiateOutbound.test.ts @@ -8,9 +8,36 @@ import path from 'node:path'; import { fileURLToPath } from 'node:url'; import * as schema from '../db/schema.js'; import { setWorkerId } from '../utils/snowflake.js'; +import { buildFederationHeaders } from '../utils/federationAuth.js'; setWorkerId(1); +// The pending row's secret is deterministic in this suite (generateHmacSecret is +// mocked below to return this exact value). fetchPeerEpoch signs its /epoch +// request with the pending row's secret and verifies the response with the same +// secret, so a valid signed epoch response must be signed with THIS secret. +const PENDING_SECRET = 'mock-generated-secret'; + +/** + * URL-aware outbound fetch stub. `/peer/accept` returns `acceptResponse`; the + * subsequent `/api/federation/epoch` call returns a valid HMAC-signed + * `{ instanceId }` (signed with the pending row's secret so fetchPeerEpoch's real + * signature check passes) when `epochToEcho` is a string, or `401` when null. + */ +function makeUrlAwareFetch(acceptResponse: Response, epochToEcho: string | null): typeof globalThis.fetch { + return (async (url: string | URL | Request): Promise => { + const u = String(url); + if (u.endsWith('/api/federation/peer/accept')) return acceptResponse.clone(); + if (u.endsWith('/api/federation/epoch')) { + if (epochToEcho === null) return new Response(JSON.stringify({ error: 'Invalid signature' }), { status: 401 }); + const body = JSON.stringify({ instanceId: epochToEcho }); + const headers = buildFederationHeaders(body, PENDING_SECRET, 'https://remote.example'); + return new Response(body, { status: 200, headers }); + } + throw new Error(`unexpected fetch ${u}`); + }) as typeof globalThis.fetch; +} + const __dirname = path.dirname(fileURLToPath(import.meta.url)); type TestDb = ReturnType>; @@ -173,12 +200,13 @@ describe('POST /api/federation/peer/initiate — 202 token capture & 200 clear', }); it('on 200 from remote, activates and clears approvalToken', async () => { - vi.spyOn(globalThis, 'fetch').mockResolvedValue( + vi.spyOn(globalThis, 'fetch').mockImplementation(makeUrlAwareFetch( new Response( - JSON.stringify({ accepted: true, instanceName: 'Remote' }), + JSON.stringify({ accepted: true, instanceName: 'Remote', instanceId: 'remote-epoch' }), { status: 200, headers: { 'Content-Type': 'application/json' } }, ), - ); + 'remote-epoch', + )); const response = await app.inject({ method: 'POST', @@ -192,4 +220,82 @@ describe('POST /api/federation/peer/initiate — 202 token capture & 200 clear', expect(peer?.status).toBe('active'); expect(peer?.approvalToken).toBeNull(); }); + + // ─── Task 6: 409 honest-refusal + verify-before-activate (BUG-1b/BUG-2) ───── + + it('(a) on 409 PEER_EXISTS_RESET_REQUIRED, returns 409 and DELETES the pending row', async () => { + vi.spyOn(globalThis, 'fetch').mockImplementation(makeUrlAwareFetch( + new Response( + JSON.stringify({ accepted: false, code: 'PEER_EXISTS_RESET_REQUIRED', error: 'reset required' }), + { status: 409, headers: { 'Content-Type': 'application/json' } }, + ), + // No epoch call is expected on this path; guard with null anyway. + null, + )); + + const response = await app.inject({ + method: 'POST', + url: '/api/federation/peer/initiate', + payload: { remoteOrigin: 'https://remote.example' }, + }); + + expect(response.statusCode).toBe(409); + expect(response.json().code).toBe('PEER_EXISTS_RESET_REQUIRED'); + + // The pending row must be gone — no false-active, no lingering slot. + const peer = testDb.select().from(schema.federationPeers) + .where(eq(schema.federationPeers.origin, 'https://remote.example')).get(); + expect(peer).toBeUndefined(); + }); + + it('(b) on 200 but failed epoch verification, parks in needs_attention (verified:false), NOT active', async () => { + vi.spyOn(globalThis, 'fetch').mockImplementation(makeUrlAwareFetch( + new Response( + JSON.stringify({ accepted: true, instanceName: 'Remote', instanceId: 'remote-epoch' }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ), + // Epoch endpoint responds 401 → fetchPeerEpoch returns null. + null, + )); + + const response = await app.inject({ + method: 'POST', + url: '/api/federation/peer/initiate', + payload: { remoteOrigin: 'https://remote.example' }, + }); + + expect(response.statusCode).toBe(200); + expect(response.json().verified).toBe(false); + + const peer = testDb.select().from(schema.federationPeers) + .where(eq(schema.federationPeers.origin, 'https://remote.example')).get(); + expect(peer?.status).toBe('needs_attention'); + expect(peer?.needsAttentionReason).toBe('repeer_incomplete'); + expect(peer?.status).not.toBe('active'); + }); + + it('(c) on 200 with a valid signed epoch, activates (verified:true) and clears needsAttentionReason', async () => { + vi.spyOn(globalThis, 'fetch').mockImplementation(makeUrlAwareFetch( + new Response( + JSON.stringify({ accepted: true, instanceName: 'Remote', instanceId: 'remote-epoch' }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ), + 'remote-epoch', + )); + + const response = await app.inject({ + method: 'POST', + url: '/api/federation/peer/initiate', + payload: { remoteOrigin: 'https://remote.example' }, + }); + + expect(response.statusCode).toBe(200); + expect(response.json().verified).toBe(true); + + const peer = testDb.select().from(schema.federationPeers) + .where(eq(schema.federationPeers.origin, 'https://remote.example')).get(); + expect(peer?.status).toBe('active'); + expect(peer?.needsAttentionReason).toBeNull(); + expect(peer?.peerInstanceId).toBe('remote-epoch'); + }); }); diff --git a/packages/server/src/routes/federation.ts b/packages/server/src/routes/federation.ts index 1d4b0144..cb10f66d 100644 --- a/packages/server/src/routes/federation.ts +++ b/packages/server/src/routes/federation.ts @@ -19,7 +19,7 @@ import { deleteAttachmentFiles, deleteUploadFile } from '../utils/fileCleanup.js import { tombstoneUser, collectDeletionBroadcastTargets, collectProfileBroadcastTargetIds } from '../utils/userDeletion.js'; import { computeFederatedId, getDmParticipants, sendCallRelay } from '../utils/federationOutbox.js'; import { onPeerActivated, onPeerDeactivated } from '../utils/federationPeerActivation.js'; -import { getInstanceId } from '../utils/federationEpoch.js'; +import { getInstanceId, fetchPeerEpoch } from '../utils/federationEpoch.js'; import { probePeerReachable, recoverOrDetectReset } from '../utils/federationRecovery.js'; import { markPeerReset, homeInstanceMatch } from '../utils/federationReset.js'; import { getDmMessageWithUser } from './dm.js'; @@ -939,16 +939,27 @@ export async function federationRoutes(app: FastifyInstance): Promise { } if (!response.ok) { - let errorMessage = `Remote instance rejected peering (HTTP ${response.status})`; - try { - const body = await response.json() as { error?: string }; - if (body.error) { - errorMessage = body.error; - } - } catch { - // Ignore parse failures — use the default error message + // Read the body exactly ONCE here — response.json()/text() consumes the + // stream, so both the honest-409 branch and the generic branch below + // share this single parse (no double-read of the same Response). + const rawBody = await response.text().catch(() => ''); + let parsed: { error?: string; code?: string } = {}; + try { parsed = JSON.parse(rawBody) as { error?: string; code?: string }; } catch { /* non-JSON body */ } + + // Responder honestly refused: it already holds peering for us and will + // not rekey (anti-hijack). Do NOT create a conflicting row — delete the + // pending row so our slot stays clean and the remote's own later Re-peer + // can land on a fresh responder slot. Surface an actionable reason. + if (response.status === 409 && parsed.code === 'PEER_EXISTS_RESET_REQUIRED') { + db.delete(schema.federationPeers).where(eq(schema.federationPeers.id, peerId)).run(); + return reply.code(409).send({ + error: 'The remote instance still holds stale peering for you. Ask its admin to reset (or Re-peer) their side, then try again.', + code: 'PEER_EXISTS_RESET_REQUIRED', + statusCode: 409, + }); } + const errorMessage = parsed.error || `Remote instance rejected peering (HTTP ${response.status})`; // Clean up the pending peer db.delete(schema.federationPeers).where(eq(schema.federationPeers.id, peerId)).run(); return reply.code(502).send({ error: errorMessage, statusCode: 502 }); @@ -972,8 +983,25 @@ export async function federationRoutes(app: FastifyInstance): Promise { // Non-JSON body — leave null. } + // The responder returned 200 → it claims it adopted our secret. PROVE it + // with a signed round-trip before trusting the peering (catches BUG-1: a + // responder that reported success without adopting, and any residual + // desync). fetchPeerEpoch signs with the just-negotiated secret; a desync + // → 401/403 → null. Park the peer in needs_attention instead of falsely + // activating so the admin sees "re-peer incomplete", not a dead-active row. + const verifiedEpoch = await fetchPeerEpoch({ origin: remoteOrigin, hmacSecret }); + if (!verifiedEpoch) { + db.update(schema.federationPeers) + .set({ status: 'needs_attention', needsAttentionReason: 'repeer_incomplete', lastSeenAt: Date.now() }) + .where(eq(schema.federationPeers.id, peerId)) + .run(); + connectionManager.sendToAdmins({ type: 'federation_peers_changed' as const }); + const parked = db.select().from(schema.federationPeers).where(eq(schema.federationPeers.id, peerId)).get(); + return reply.code(200).send({ peer: parked ? sanitizePeer(parked) : null, verified: false }); + } + db.update(schema.federationPeers) - .set({ status: 'active', lastSeenAt: Date.now(), instanceName: remoteInstanceName, peerInstanceId: remoteInstanceId, approvalToken: null }) + .set({ status: 'active', lastSeenAt: Date.now(), instanceName: remoteInstanceName, peerInstanceId: remoteInstanceId, needsAttentionReason: null, approvalToken: null }) .where(eq(schema.federationPeers.id, peerId)) .run(); connectionManager.sendToAdmins({ type: 'federation_peers_changed' as const }); @@ -991,7 +1019,7 @@ export async function federationRoutes(app: FastifyInstance): Promise { return reply.code(500).send({ error: 'Failed to read peer after activation', statusCode: 500 }); } - return reply.code(200).send({ peer: sanitizePeer(peer) }); + return reply.code(200).send({ peer: sanitizePeer(peer), verified: true }); } catch (err: unknown) { // Clean up the pending peer on network/timeout errors db.delete(schema.federationPeers).where(eq(schema.federationPeers.id, peerId)).run(); @@ -1124,9 +1152,9 @@ export async function federationRoutes(app: FastifyInstance): Promise { // Detection-only: if the inbound epoch differs from our trusted // baseline, the peer is a NEW incarnation on the same domain (a // wipe-and-reinstall). Route it to needs_attention + snapshot + - // journal — but STILL return 200 and STILL do not rekey. The - // anti-hijack guard above is preserved verbatim; detection never - // grants capability. + // journal — but STILL return 409 (PEER_EXISTS_RESET_REQUIRED) and + // STILL do not rekey. The anti-hijack guard above is preserved + // verbatim; detection never grants capability. if (reqInstanceId && existing.peerInstanceId && reqInstanceId !== existing.peerInstanceId) { markPeerReset(existing.id, sourceOrigin, existing.peerInstanceId, reqInstanceId); } diff --git a/packages/server/src/utils/federationEpochHandshake.test.ts b/packages/server/src/utils/federationEpochHandshake.test.ts index d51054af..df9f63c0 100644 --- a/packages/server/src/utils/federationEpochHandshake.test.ts +++ b/packages/server/src/utils/federationEpochHandshake.test.ts @@ -8,6 +8,7 @@ import path from 'node:path'; import { fileURLToPath } from 'node:url'; import * as schema from '../db/schema.js'; import { setWorkerId } from './snowflake.js'; +import { buildFederationHeaders } from './federationAuth.js'; setWorkerId(1); @@ -232,7 +233,7 @@ describe('POST /api/federation/peer/accept — peer_instance_id (epoch) persiste }); }); -describe('POST /api/federation/peer/initiate — persists remote epoch from handshake response', () => { +describe('POST /api/federation/peer/initiate — verifies the handshake before persisting remote epoch', () => { let app: FastifyInstance; beforeEach(async () => { @@ -251,13 +252,26 @@ describe('POST /api/federation/peer/initiate — persists remote epoch from hand sqlite.close(); }); - it('writes peer_instance_id from the remote /peer/accept response body', async () => { - const fetchMock = vi.fn(async () => - new Response(JSON.stringify({ accepted: true, instanceName: 'Remote', instanceId: 'remote-epoch-1' }), { - status: 200, - headers: { 'content-type': 'application/json' }, - }), - ); + it('activates and persists peer_instance_id after a verified /epoch round-trip', async () => { + // The responder signs the /epoch response with the SAME secret the initiator + // sent it in /peer/accept — mirroring a real responder that adopted the secret. + let capturedSecret = ''; + const fetchMock = vi.fn(async (url: string | URL, init?: RequestInit) => { + const u = String(url); + if (u.endsWith('/api/federation/peer/accept')) { + capturedSecret = (JSON.parse(String(init?.body)) as { hmacSecret: string }).hmacSecret; + return new Response(JSON.stringify({ accepted: true, instanceName: 'Remote', instanceId: 'remote-epoch-1' }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (u.endsWith('/api/federation/epoch')) { + const body = JSON.stringify({ instanceId: 'remote-epoch-1' }); + const headers = buildFederationHeaders(body, capturedSecret, 'https://remote.example'); + return new Response(body, { status: 200, headers }); + } + throw new Error(`unexpected fetch ${u}`); + }); vi.stubGlobal('fetch', fetchMock); const response = await app.inject({ @@ -267,24 +281,32 @@ describe('POST /api/federation/peer/initiate — persists remote epoch from hand }); expect(response.statusCode).toBe(200); + expect((response.json() as { verified?: boolean }).verified).toBe(true); const row = testDb.select().from(schema.federationPeers) .where(eq(schema.federationPeers.origin, 'https://remote.example')).get(); expect(row?.status).toBe('active'); expect(row?.peerInstanceId).toBe('remote-epoch-1'); // Our epoch must be sent in the outbound handshake body. - const call = fetchMock.mock.calls[0] as unknown as [string, RequestInit]; - const sentBody = JSON.parse(call[1].body as string) as { instanceId?: string }; + const acceptCall = fetchMock.mock.calls.find(([u]) => String(u).endsWith('/api/federation/peer/accept')) as unknown as [string, RequestInit]; + const sentBody = JSON.parse(acceptCall[1].body as string) as { instanceId?: string }; expect(sentBody.instanceId).toBe(LOCAL_EPOCH); }); - it('writes null peer_instance_id when the remote response omits instanceId', async () => { - vi.stubGlobal('fetch', vi.fn(async () => - new Response(JSON.stringify({ accepted: true, instanceName: 'Remote' }), { - status: 200, - headers: { 'content-type': 'application/json' }, - }), - )); + it('parks in needs_attention when the /epoch verification cannot complete (legacy/unverifiable peer)', async () => { + // Remote returns 200 on /peer/accept but has no verifiable /epoch endpoint + // (404 → legacy). Without a signed round-trip we refuse to false-activate. + vi.stubGlobal('fetch', vi.fn(async (url: string | URL) => { + const u = String(url); + if (u.endsWith('/api/federation/peer/accept')) { + return new Response(JSON.stringify({ accepted: true, instanceName: 'Remote' }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (u.endsWith('/api/federation/epoch')) return new Response('not found', { status: 404 }); + throw new Error(`unexpected fetch ${u}`); + })); const response = await app.inject({ method: 'POST', @@ -293,9 +315,11 @@ describe('POST /api/federation/peer/initiate — persists remote epoch from hand }); expect(response.statusCode).toBe(200); + expect((response.json() as { verified?: boolean }).verified).toBe(false); const row = testDb.select().from(schema.federationPeers) .where(eq(schema.federationPeers.origin, 'https://remote.example')).get(); - expect(row?.status).toBe('active'); + expect(row?.status).toBe('needs_attention'); + expect(row?.needsAttentionReason).toBe('repeer_incomplete'); expect(row?.peerInstanceId).toBeNull(); }); });