Harden peer handshake — strict pstrlen + magic check + state machine#4
Merged
Conversation
…chine
Closes the last LangSec gap in the BitTorrent client: the BEP 3 handshake
previously read pstrlen bytes blindly without validating the protocol magic
string, and PeerConn had no explicit state guarding which methods were valid
when. Now the recognizer enforces the full handshake grammar before any
state mutation and a typed state field gates downstream calls.
- internal/client/peer.go:
- protoMagic constant + handshakeLen = 68 (BEP 3 mandates exactly this).
- Read the peer's full 68-byte handshake in one ReadFull, then validate:
pstrlen == 19, pstr == "BitTorrent protocol", info_hash matches. Only
then promote state to stateHandshook (and stateExtended after BEP 10).
- peerState enum {Init, Handshook, Extended}. Handshake requires Init.
ReadMessage/WriteMessage require >= Handshook. RequestMetadata requires
Extended. Each precondition returns a typed error on misuse.
- internal/client/peer_test.go: new tests for bad pstrlen, wrong magic,
ReadMessage/WriteMessage before handshake, RequestMetadata before
extended handshake. fakePeer helper for handshake-rejection cases.
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.
Summary
Closes the last LangSec gap in the BitTorrent client. The BEP 3 handshake previously read
pstrlenbytes blindly without validating the protocol magic string, andPeerConnhad no explicit state guarding which methods were valid when. Now the recognizer enforces the full 68-byte handshake grammar before any state mutation, and a typed state field gates downstream calls.This is Priority 3 of 3 from the LangSec analysis. P1 (announce recognizer, #2) and P2 (bencode validator, #3) are independent.
What's new
Strict handshake recognition (
Handshakeininternal/client/peer.go):io.ReadFull, then validate the entire structure before promoting any state.pstrlenmust be exactly19(length of"BitTorrent protocol"per BEP 3). The previous code rejected onlypstrlen == 0and accepted any other value."BitTorrent protocol". Previously the bytes were read and indexed past without a single byte of comparison — a peer speaking a 19-byte non-BitTorrent protocol would have passed.Typed state machine (
peerStateenum):stateInitConnectHandshakestateHandshookHandshake(post-validation)ReadMessage,WriteMessagestateExtendedHandshake(after BEP 10 ext handshake)RequestMetadataEach precondition returns a typed error on misuse — calling
RequestMetadatabefore the BEP 10 handshake orReadMessageon a fresh connection now fails withRequestMetadata called in state N (expected extended)instead of writing/reading garbage on the wire.Test plan
go test ./...— all packages greengo vet ./...— cleanTestHandshakeandTestHandshakeMismatchcontinue to pass (they always sent valid magic + pstrlen=19).TestHandshakeRejectsBadPstrlen— pstrlen=18 rejected with typed reasonTestHandshakeRejectsBadMagic— pstr="WrongTorrent magic!"(correct length, wrong content) rejectedTestReadMessageBeforeHandshakeRejected—ReadMessage/WriteMessageinstateInitrejectedTestRequestMetadataBeforeExtendedRejected—RequestMetadatainstateHandshookrejectedpeerStateis the right shape — single int with<comparisons, no exposed transition methods. Could be replaced with an interface-based state pattern, but felt over-engineered for 3 states.🤖 Generated with Claude Code