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); +});