fix(core): verify a claimed peer ID against its presented certificate - #41
Merged
Conversation
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.
Mearman
marked this pull request as ready for review
September 9, 2026 21:43
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
🎉 This PR is included in version 1.27.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #40
Ordinary peer connections accepted a peer's identity purely from the self-reported
peerIdinintroduce/pong/connectToPeer, over arejectUnauthorized: falseTLS connection with no fingerprint check against the certificate actually presented — despitetls-transport.ts's own doc comment claiming that check already existed.Every bridge sets
store.peerId = identity.fingerprint, so a peer's ID is always supposed to equal the fingerprint of its own certificate. This addsTlsTransport.verifyClaimedPeerId(), applied at the three points a remote peer's identity is established from a self-reported wire message:introducehandlerponghandlerconnectToPeerdial, which already holds the target peer's expected ID from the peer listA mismatch (or no certificate at all) destroys the socket and reports it via
onErrorinstead of accepting the connection.connect_request/connectToRemote(the invite-based remote-join flow) is untouched — it already requires explicit human approval viaacceptConnection, a different trust mechanism from the always-on mesh path this issue is scoped to.Added
src/test/peer-id-verification.integration.test.tscovering all three rejection paths, plus confirmed the existing TLS/mesh/federation integration suites still pass with legitimate (matching) identities.