From 635f6ebe7e6f1840a68581b5d87aacb975d8b243 Mon Sep 17 00:00:00 2001 From: Joseph Mearman Date: Wed, 9 Sep 2026 22:40:42 +0100 Subject: [PATCH] fix(core): verify a claimed peer ID against its presented certificate TlsTransport accepted introduce, pong, and connectToPeer identity claims purely from the self-reported peerId in the wire message, over a rejectUnauthorized: false TLS connection with no fingerprint check against the certificate actually presented. Any socket could claim any peerId regardless of the certificate it held, contradicting the transport's own doc comment claiming fingerprint verification already happened. Add verifyClaimedPeerId(), applied wherever a remote peer's identity is established from a self-reported wire message: the coordinator's introduce handler, the data server's pong handler, and the client's own connectToPeer dial (which already holds the peer's expected ID from the peer list). A mismatch, or no certificate at all, destroys the socket and reports an error rather than accepting the connection. --- package.json | 2 +- src/core/tls-transport.ts | 49 +++- .../peer-id-verification.integration.test.ts | 246 ++++++++++++++++++ 3 files changed, 294 insertions(+), 3 deletions(-) create mode 100644 src/test/peer-id-verification.integration.test.ts diff --git a/package.json b/package.json index b90ded6..793a244 100644 --- a/package.json +++ b/package.json @@ -52,7 +52,7 @@ "generate:server-json": "tsx scripts/sync-release-metadata.ts", "publish:mcp-registry": "tsx scripts/publish-mcp-registry.ts", "lint": "eslint .", - "test": "node --test --test-concurrency=1 dist/test/coordinator-socket-error.integration.test.js dist/test/identity-store.test.js dist/test/identity-cert.test.js dist/test/broadcast-window.integration.test.js dist/test/identity-restart.integration.test.js dist/test/mesh-e2e.integration.test.js dist/test/ws-broadcast-window.integration.test.js dist/test/tls-transport.integration.test.js dist/test/state-sync-convergence.test.js dist/test/downtime-replay.test.js dist/test/downtime-replay.integration.test.js dist/test/filestore.test.js", + "test": "node --test --test-concurrency=1 dist/test/coordinator-socket-error.integration.test.js dist/test/identity-store.test.js dist/test/identity-cert.test.js dist/test/broadcast-window.integration.test.js dist/test/identity-restart.integration.test.js dist/test/mesh-e2e.integration.test.js dist/test/ws-broadcast-window.integration.test.js dist/test/tls-transport.integration.test.js dist/test/peer-id-verification.integration.test.js dist/test/state-sync-convergence.test.js dist/test/downtime-replay.test.js dist/test/downtime-replay.integration.test.js dist/test/filestore.test.js", "test:visibility": "node --test dist/test/visibility.integration.test.js", "test:delivery": "node dist/test/delivery-receipt.runner.js", "test:federation": "node dist/test/federation.integration.test.js", diff --git a/src/core/tls-transport.ts b/src/core/tls-transport.ts index b17c755..3822bd2 100644 --- a/src/core/tls-transport.ts +++ b/src/core/tls-transport.ts @@ -28,6 +28,7 @@ import type { TransportEvents, } from "./transport.js"; import type { PeerIdentity } from "./identity.js"; +import { fingerprintDer } from "./identity.js"; import { nanoid } from "./nanoid.js"; // --------------------------------------------------------------------------- @@ -188,8 +189,7 @@ export class TlsTransport { return { key: this.identity.privateKey, cert: this.identity.certificate, - // Do not reject unauthorized — we do our own fingerprint verification - // after the TLS handshake completes. + // Do not reject unauthorized — we do our own fingerprint verification after the TLS handshake completes, in verifyClaimedPeerId(). rejectUnauthorized: false, requestCert: true, }; @@ -204,6 +204,43 @@ export class TlsTransport { }; } + // ----------------------------------------------------------------------- + // Peer identity verification + // ----------------------------------------------------------------------- + + /** + * Verify a connected socket's presented certificate fingerprint matches the peer ID it claims via `introduce`/`pong` in the wire protocol, destroying the socket and returning false on any mismatch (including no certificate presented at all). + * + * Peer IDs are minted as the fingerprint of the peer's own certificate (identity.ts's `generateIdentity()`, wired up by every bridge as `store.peerId = identity.fingerprint`), so a claimed peer ID that doesn't match the certificate actually presented on this connection means the socket is not who it says it is — regardless of what it typed into the wire message. + */ + private verifyClaimedPeerId( + socket: tls.TLSSocket, + claimedPeerId: string, + ): boolean { + const cert = socket.getPeerCertificate(); + // Node's types declare every PeerCertificate field non-optional, but the documented runtime behaviour when the peer presents no certificate at all is an empty object — not null/undefined, and not a Buffer-typed `raw`. Detect that real shape rather than trusting the declared type. + if (Object.keys(cert).length === 0) { + socket.destroy(); + this.events.onError?.( + new Error( + `Rejected connection claiming peer ID ${claimedPeerId}: no certificate presented`, + ), + ); + return false; + } + const actualFingerprint = fingerprintDer(cert.raw); + if (actualFingerprint !== claimedPeerId) { + socket.destroy(); + this.events.onError?.( + new Error( + `Rejected connection claiming peer ID ${claimedPeerId}: presented certificate fingerprint is ${actualFingerprint}`, + ), + ); + return false; + } + return true; + } + // ----------------------------------------------------------------------- // MeshTransport — Data server // ----------------------------------------------------------------------- @@ -471,6 +508,12 @@ export class TlsTransport { const socket = tls.connect( { ...this.connectOptions, host: COORDINATOR_HOST, port: peer.port }, () => { + if (!this.verifyClaimedPeerId(socket, peer.id)) { + this.pendingOutbound.delete(peer.id); + resolve(); + return; + } + const buffer = new MessageBuffer(); this.peerConnections.set(peer.id, { socket, buffer }); @@ -770,6 +813,7 @@ export class TlsTransport { if (!isMeshMessage(item)) continue; if (item.method === "introduce") { + if (!this.verifyClaimedPeerId(socket, item.peerId)) continue; const handle: ConnectionHandle = { id: item.peerId, policy }; this.introConnections.set(handle.id, socket); this.events.onIntroduction(handle, { @@ -820,6 +864,7 @@ export class TlsTransport { if (isMeshMessage(item)) { if (item.method === "pong") { const peerId = item.peerId; + if (!this.verifyClaimedPeerId(socket, peerId)) continue; remotePeerId = peerId; if (!this.peerConnections.has(peerId)) { this.peerConnections.set(peerId, { socket, buffer }); diff --git a/src/test/peer-id-verification.integration.test.ts b/src/test/peer-id-verification.integration.test.ts new file mode 100644 index 0000000..0b43431 --- /dev/null +++ b/src/test/peer-id-verification.integration.test.ts @@ -0,0 +1,246 @@ +/** + * Peer ID verification integration test (#40) — a socket claiming a peer ID that doesn't match the certificate it actually presents must be rejected, on every path where TlsTransport learns a remote peer's identity from a self-reported wire message: the coordinator's `introduce` handler, the data server's `pong` handler, and the client's own `connectToPeer` dial. + * + * Run: node dist/test/peer-id-verification.integration.test.js [test-name] With no argument, every scenario runs in order. + */ + +import * as net from "node:net"; +import * as tls from "node:tls"; +import * as assert from "node:assert/strict"; +import { TlsTransport } from "../core/tls-transport.js"; +import { generateIdentity } from "../core/identity.js"; +import { encode } from "../core/wire-protocol.js"; +import type { PeerInfo } from "../core/wire-protocol.js"; +import type { TransportEvents } from "../core/transport.js"; + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +function allocFreePort(): Promise { + return new Promise((resolve, reject) => { + const server = net.createServer(); + server.listen(0, "127.0.0.1", () => { + const addr = server.address(); + const port = addr && typeof addr === "object" ? addr.port : 0; + server.close(() => resolve(port)); + }); + server.on("error", reject); + }); +} + +function noopEvents(overrides: Partial = {}): TransportEvents { + return { + onMessage: () => undefined, + onPeerConnected: () => undefined, + onPeerDisconnected: () => undefined, + onIntroduction: () => undefined, + onConnectionRequest: () => undefined, + onPeerList: () => undefined, + onPeerJoined: () => undefined, + onBecomeCoordinator: () => undefined, + ...overrides, + }; +} + +async function testSpoofedIntroduceRejected(): Promise { + const identityCoordinator = generateIdentity(); + const identityAttacker = generateIdentity(); + + let introduced = false; + let sawError = false; + const transport = new TlsTransport( + noopEvents({ + onIntroduction: () => { + introduced = true; + }, + onError: () => { + sawError = true; + }, + }), + identityCoordinator, + ); + const port = await allocFreePort(); + await transport.becomeCoordinator("127.0.0.1", port); + + // The attacker connects with its own genuine certificate, but sends an `introduce` claiming the coordinator's own peer ID — self-reported identity that doesn't match the certificate on this connection. + const socket = tls.connect({ + key: identityAttacker.privateKey, + cert: identityAttacker.certificate, + host: "127.0.0.1", + port, + rejectUnauthorized: false, + }); + + await new Promise((resolve, reject) => { + socket.once("secureConnect", () => { + socket.write( + encode({ + method: "introduce", + peerId: identityCoordinator.fingerprint, + dataPort: 12345, + }), + ); + resolve(); + }); + socket.once("error", reject); + }); + + await sleep(300); + + assert.strictEqual( + introduced, + false, + "onIntroduction must not fire for a spoofed peer ID", + ); + assert.ok(sawError, "the rejection should be reported via onError"); + assert.ok(socket.destroyed, "the spoofing socket should be destroyed"); + + await transport.shutdown(); + console.log(" ✓ introduce with mismatched certificate is rejected"); +} + +async function testSpoofedPongRejected(): Promise { + const identityListener = generateIdentity(); + const identityAttacker = generateIdentity(); + + let connected = false; + let sawError = false; + const transport = new TlsTransport( + noopEvents({ + onPeerConnected: () => { + connected = true; + }, + onError: () => { + sawError = true; + }, + }), + identityListener, + ); + await transport.startDataServer(); + + // The attacker connects to the data server with its own certificate, but sends a `pong` claiming an arbitrary, unrelated peer ID. + const socket = tls.connect({ + key: identityAttacker.privateKey, + cert: identityAttacker.certificate, + host: "127.0.0.1", + port: transport.dataPort, + rejectUnauthorized: false, + }); + + await new Promise((resolve, reject) => { + socket.once("secureConnect", () => { + socket.write(encode({ method: "pong", peerId: "NOT-MY-CERTIFICATE" })); + resolve(); + }); + socket.once("error", reject); + }); + + await sleep(300); + + assert.strictEqual( + connected, + false, + "onPeerConnected must not fire for a spoofed peer ID", + ); + assert.ok(sawError, "the rejection should be reported via onError"); + assert.ok(socket.destroyed, "the spoofing socket should be destroyed"); + + await transport.shutdown(); + console.log(" ✓ pong with mismatched certificate is rejected"); +} + +async function testConnectToPeerCertMismatchRejected(): Promise { + // The real peer B is listening under its own genuine identity... + const identityB = generateIdentity(); + const transportB = new TlsTransport(noopEvents(), identityB); + await transportB.startDataServer(); + + // ...but the peer list entry a compromised or misbehaving coordinator could hand to a dialling client claims a completely different ID for that same host:port. + const claimedPeer: PeerInfo = { + id: "CLAIMED-BUT-WRONG-ID", + port: transportB.dataPort, + startedAt: new Date().toISOString(), + }; + + let sawError = false; + const identityDialer = generateIdentity(); + const transportDialer = new TlsTransport( + noopEvents({ + onError: () => { + sawError = true; + }, + }), + identityDialer, + ); + + await transportDialer.connectToPeer(claimedPeer, identityDialer.fingerprint); + await sleep(200); + + assert.ok( + sawError, + "the rejection should be reported via onError when the dialled peer's certificate doesn't match the claimed ID", + ); + await assert.rejects( + () => + transportDialer.send( + { id: claimedPeer.id }, + { method: "pong", peerId: identityDialer.fingerprint }, + ), + /No connection for handle/, + "no peer connection should have been registered under the falsely claimed ID", + ); + + await transportDialer.shutdown(); + await transportB.shutdown(); + console.log( + " ✓ connectToPeer rejects a certificate that doesn't match the claimed peer ID", + ); +} + +// --------------------------------------------------------------------------- +// Runner +// --------------------------------------------------------------------------- + +const testName = process.argv[2]; + +const tests: Record Promise> = { + "spoofed-introduce-rejected": testSpoofedIntroduceRejected, + "spoofed-pong-rejected": testSpoofedPongRejected, + "connect-to-peer-cert-mismatch-rejected": + testConnectToPeerCertMismatchRejected, +}; + +const selected = + testName === undefined + ? Object.entries(tests) + : Object.entries(tests).filter(([name]) => name === testName); +if (selected.length === 0) { + console.error(`Unknown test: ${testName}`); + console.error(`Available: ${Object.keys(tests).join(", ")}`); + process.exit(1); +} + +async function run(): Promise { + for (const [name, fn] of selected) { + console.log(`Running ${name}:`); + await fn(); + } + + const maxWait = 2000; + const start = Date.now(); + while ( + (( + process as unknown as { _getActiveHandles?: () => unknown[] } + )._getActiveHandles?.()?.length ?? 0) > 0 && + Date.now() - start < maxWait + ) { + await new Promise((resolve) => setTimeout(resolve, 50)); + } + process.exit(0); +} + +run().catch((err: unknown) => { + console.error(`FAIL [${testName ?? "all"}]:`, err); + process.exit(1); +});