v2transport: restore responder handshake progress
What changed, and why it matters
This commit fixes a deadlock risk in btcd's new Bitcoin v2 transport handshake. Previously, the responder waited until it had received the initiator's full 64-byte key before doing any work, which could cause both sides to sit waiting for each other. The change makes the responder send its own key earlier, after the first byte that differs from the old v1 protocol, and splits expensive cryptographic work into two separately rate-limited phases so the CPU lock is not held across network reads or writes. It is a protocol correctness/DoS-resilience fix rather than a direct theft-of-funds bug.
Treat as a recommended update for operators running btcd with BIP324 v2 transport enabled, primarily for liveness/DoS-resilience. Review admission policy implementations that depend on the old single-lease behavior, as the patch now invokes Acquire twice per responder handshake. No immediate emergency response is indicated absent evidence of active exploitation.
Security signals we found
BIP324 handshake deadlock avoidance
CPU admission lease split to prevent resource exhaustion / lock holding across network I/O
Responder now sends key material before full initiator key is received
Wrong-network v1 path now generates and sends responder key before detecting mismatch, changing admission/release counts
New regression test enforces progress after first v1-prefix mismatch
Evidence from the diff
The patch refactors RespondV2Handshake in v2transport/transport.go. The responder now: (1) acquires a CPU admission lease, generates its ElligatorSwift key and garbage, releases the lease, and sends the data as soon as the initiator’s prefix diverges from the v1 prefix; (2) reads the remainder of the initiator key; (3) acquires a second lease and performs key agreement/cipher initialization. This avoids the BIP324 deadlock where both peers block on a read because neither has written. Tests are updated to require that the responder makes progress after a partial prefix, that two admission leases are used, and that a rejected second lease does not leak resources or initialize ciphers.
Changed components
v2transport/transport.gov2transport/handshake_test.goPeer.RespondV2HandshakePeer.prepareResponderHandshakePeer.generateResponderKeyPeer.initializeResponderPeer.acquireResponderAdmissionInspect captured patch +231 / −86
diff --git a/v2transport/handshake_test.go b/v2transport/handshake_test.go
index 23c3820..1a086a4 100644
--- a/v2transport/handshake_test.go
+++ b/v2transport/handshake_test.go
@@ -3,8 +3,8 @@ package v2transport
import (
"bytes"
"errors"
- "fmt"
"io"
+ "net"
"testing"
"time"
)
@@ -42,6 +42,13 @@ func (rw *splitReadWriter) Write(p []byte) (int, error) {
return rw.writes.Write(p)
}
+// prefixedReadWriter reads already-read bytes before reading from its
+// underlying connection.
+type prefixedReadWriter struct {
+ io.Reader
+ io.Writer
+}
+
// bufferedReadWriter is one endpoint of a buffered in-memory duplex pipe.
type bufferedReadWriter struct {
recv <-chan []byte
@@ -105,55 +112,116 @@ func TestResponderV1Fallback(t *testing.T) {
}
}
-// TestResponderIncompleteCandidate verifies incomplete v2 candidates do not
-// consume CPU admission or generate responder key material.
-func TestResponderIncompleteCandidate(t *testing.T) {
- for _, length := range []int{1, 15, 16, 63} {
- t.Run(fmt.Sprintf("length_%d", length), func(t *testing.T) {
- var admissions int
- peer := NewPeerWithOptions(WithResponderHandshakeAdmission(
- testHandshakeAdmission(func() (func(), error) {
- admissions++
- return func() {}, nil
- }),
- ))
- candidate := make([]byte, length)
- candidate[0] = 1
- rw := newSplitReadWriter(candidate)
- peer.UseReadWriter(rw)
-
- err := peer.RespondV2Handshake(0, BitcoinNet(0xd9b4bef9))
- if err == nil {
- t.Fatal("incomplete candidate unexpectedly succeeded")
- }
- if admissions != 0 {
- t.Fatalf("incomplete candidate consumed %d admissions",
- admissions)
- }
- if peer.privkeyOurs != nil {
- t.Fatal("incomplete candidate generated responder key")
- }
- if rw.writes.Len() != 0 {
- t.Fatalf("incomplete candidate wrote %d bytes",
- rw.writes.Len())
- }
- })
+// TestResponderHandshakeProgress verifies the responder sends after the first
+// v1-prefix mismatch instead of waiting for the complete initiator key. The
+// peers then complete key agreement and exchange an encrypted packet.
+func TestResponderHandshakeProgress(t *testing.T) {
+ const testNet = BitcoinNet(0xd9b4bef9)
+
+ initiator := NewPeer()
+ capture := newSplitReadWriter(nil)
+ initiator.UseReadWriter(capture)
+ if err := initiator.InitiateV2Handshake(0); err != nil {
+ t.Fatalf("initiator key generation failed: %v", err)
+ }
+ initiatorKey := append([]byte(nil), capture.writes.Bytes()...)
+ v1Prefix := createV1Prefix(testNet)
+ mismatchLen := 0
+ for mismatchLen < len(v1Prefix) &&
+ initiatorKey[mismatchLen] == v1Prefix[mismatchLen] {
+
+ mismatchLen++
+ }
+ if mismatchLen == len(v1Prefix) {
+ t.Fatal("initiator key matched the complete v1 prefix")
+ }
+ mismatchLen++
+
+ initiatorConn, responderConn := net.Pipe()
+ defer initiatorConn.Close()
+ defer responderConn.Close()
+
+ deadline := time.Now().Add(5 * time.Second)
+ if err := initiatorConn.SetDeadline(deadline); err != nil {
+ t.Fatalf("initiator deadline failed: %v", err)
+ }
+ if err := responderConn.SetDeadline(deadline); err != nil {
+ t.Fatalf("responder deadline failed: %v", err)
+ }
+
+ responder := NewPeer()
+ responder.UseReadWriter(responderConn)
+ responderErr := make(chan error, 1)
+ go func() {
+ responderErr <- responder.RespondV2Handshake(0, testNet)
+ }()
+
+ if _, err := initiatorConn.Write(initiatorKey[:mismatchLen]); err != nil {
+ t.Fatalf("initiator mismatch prefix failed: %v", err)
+ }
+
+ var responderPrefix [1]byte
+ if _, err := io.ReadFull(initiatorConn, responderPrefix[:]); err != nil {
+ t.Fatalf("responder made no progress after mismatch: %v", err)
+ }
+
+ initiatorWriteErr := make(chan error, 1)
+ go func() {
+ _, err := initiatorConn.Write(initiatorKey[mismatchLen:])
+ initiatorWriteErr <- err
+ }()
+
+ initiator.UseReadWriter(&prefixedReadWriter{
+ Reader: io.MultiReader(
+ bytes.NewReader(responderPrefix[:]), initiatorConn,
+ ),
+ Writer: initiatorConn,
+ })
+ if err := initiator.completeKeyExchange(true, testNet); err != nil {
+ t.Fatalf("initiator key exchange failed: %v", err)
+ }
+ if err := <-initiatorWriteErr; err != nil {
+ t.Fatalf("remaining initiator key failed: %v", err)
+ }
+ if err := <-responderErr; err != nil {
+ t.Fatalf("responder handshake failed: %v", err)
+ }
+
+ payload := []byte("post-handshake payload")
+ sendErr := make(chan error, 1)
+ go func() {
+ _, _, err := initiator.V2EncPacket(payload, nil, false)
+ sendErr <- err
+ }()
+
+ got, err := responder.V2ReceivePacket(nil)
+ if err != nil {
+ t.Fatalf("packet receive failed: %v", err)
+ }
+ if err := <-sendErr; err != nil {
+ t.Fatalf("packet send failed: %v", err)
+ }
+ if !bytes.Equal(got, payload) {
+ t.Fatalf("packet mismatch: got %x, want %x", got, payload)
}
}
-// TestResponderWrongNetworkV1 verifies wrong-network v1 is rejected before
-// CPU admission and key generation.
+// TestResponderWrongNetworkV1 verifies wrong-network v1 is rejected after the
+// responder sends at the first prefix mismatch but before key agreement.
func TestResponderWrongNetworkV1(t *testing.T) {
const expectedNet = BitcoinNet(0xd9b4bef9)
candidate := make([]byte, 64)
copy(candidate, createV1Prefix(BitcoinNet(0x0709110b)))
- var admissions int
+ var (
+ admissions int
+ releases int
+ )
peer := NewPeerWithOptions(WithResponderHandshakeAdmission(
testHandshakeAdmission(func() (func(), error) {
admissions++
- return func() {}, nil
+ return func() { releases++ }, nil
}),
))
rw := newSplitReadWriter(candidate)
@@ -163,11 +231,15 @@ func TestResponderWrongNetworkV1(t *testing.T) {
if !errors.Is(err, errWrongNetV1Peer) {
t.Fatalf("unexpected wrong-network error: %v", err)
}
- if admissions != 0 {
- t.Fatalf("wrong-network v1 consumed %d admissions", admissions)
+ if admissions != 1 || releases != 1 {
+ t.Fatalf("unexpected lease counts: admissions=%d releases=%d",
+ admissions, releases)
}
- if peer.privkeyOurs != nil {
- t.Fatal("wrong-network v1 generated responder key")
+ if peer.privkeyOurs == nil {
+ t.Fatal("wrong-network v1 did not generate responder key")
+ }
+ if rw.writes.Len() != 64 {
+ t.Fatalf("responder wrote %d bytes, want 64", rw.writes.Len())
}
}
@@ -205,8 +277,8 @@ func TestResponderAdmissionRejected(t *testing.T) {
}
}
-// TestResponderAdmissionLease verifies the admission lease covers responder
-// cryptography and is released before the response is written.
+// TestResponderAdmissionLease verifies separate admission leases cover key
+// generation and key agreement without spanning network I/O.
func TestResponderAdmissionLease(t *testing.T) {
candidate := make([]byte, 64)
for i := range candidate {
@@ -234,7 +306,7 @@ func TestResponderAdmissionLease(t *testing.T) {
if err := peer.RespondV2Handshake(0, BitcoinNet(0xd9b4bef9)); err != nil {
t.Fatalf("responder handshake failed: %v", err)
}
- if admissions != 1 || releases != 1 {
+ if admissions != 2 || releases != 2 {
t.Fatalf("unexpected lease counts: admissions=%d releases=%d",
admissions, releases)
}
@@ -246,6 +318,51 @@ func TestResponderAdmissionLease(t *testing.T) {
}
}
+// TestResponderSecondAdmissionRejected verifies a rejection after the
+// responder write does not start key agreement or leak the first lease.
+func TestResponderSecondAdmissionRejected(t *testing.T) {
+ errRejected := errors.New("rejected")
+ candidate := make([]byte, 64)
+ for i := range candidate {
+ candidate[i] = byte(i)
+ }
+
+ var (
+ admissions int
+ releases int
+ )
+ peer := NewPeerWithOptions(WithResponderHandshakeAdmission(
+ testHandshakeAdmission(func() (func(), error) {
+ admissions++
+ if admissions == 2 {
+ return nil, errRejected
+ }
+
+ return func() { releases++ }, nil
+ }),
+ ))
+ rw := newSplitReadWriter(candidate)
+ peer.UseReadWriter(rw)
+
+ err := peer.RespondV2Handshake(0, BitcoinNet(0xd9b4bef9))
+ if !errors.Is(err, errRejected) {
+ t.Fatalf("unexpected admission error: %v", err)
+ }
+ if admissions != 2 || releases != 1 {
+ t.Fatalf("unexpected lease counts: admissions=%d releases=%d",
+ admissions, releases)
+ }
+ if peer.privkeyOurs == nil {
+ t.Fatal("first responder phase did not generate key material")
+ }
+ if peer.responderReady {
+ t.Fatal("rejected second phase initialized responder ciphers")
+ }
+ if rw.writes.Len() != 64 {
+ t.Fatalf("responder wrote %d bytes, want 64", rw.writes.Len())
+ }
+}
+
// TestResponderHandshakeInteroperability verifies the refactored responder
// ordering preserves the complete v2 wire transcript.
func TestResponderHandshakeInteroperability(t *testing.T) {
diff --git a/v2transport/transport.go b/v2transport/transport.go
index 6dfd74e..aecdd9e 100644
--- a/v2transport/transport.go
+++ b/v2transport/transport.go
@@ -106,12 +106,12 @@ var (
ErrShouldDowngradeToV1 = fmt.Errorf("should downgrade to v1")
)
-// HandshakeAdmission controls access to the CPU-bound portion of an inbound
-// v2 handshake.
+// HandshakeAdmission controls access to the CPU-bound phases of an inbound v2
+// handshake.
type HandshakeAdmission interface {
- // Acquire reserves the resources needed to generate the responder key
- // and perform key agreement. The returned function releases the
- // reservation once those operations are complete.
+ // Acquire reserves the resources needed for one CPU-bound responder
+ // phase. Responders acquire once for key generation and again for key
+ // agreement. The returned function releases the phase reservation.
Acquire() (release func(), err error)
}
@@ -119,8 +119,7 @@ type HandshakeAdmission interface {
type PeerOption func(*Peer)
// WithResponderHandshakeAdmission installs an admission policy that runs
-// after a responder has received a complete v2 key candidate and before it
-// performs key generation or key agreement.
+// before each CPU-bound responder phase.
func WithResponderHandshakeAdmission(
admission HandshakeAdmission,
) PeerOption {
@@ -193,7 +192,7 @@ type Peer struct {
rw io.ReadWriter
// responderAdmission optionally reserves resources for the CPU-bound
- // portion of an inbound handshake.
+ // phases of an inbound handshake.
responderAdmission HandshakeAdmission
// responderReady is set after the responder has completed key agreement
@@ -421,10 +420,12 @@ func (p *Peer) InitiateV2Handshake(garbageLen int) error {
}
// RespondV2Handshake determines whether the initiator wants to use the v2
-// protocol. For a v2 initiator, it receives the complete key candidate before
-// reserving CPU resources, initializes the responder ciphers, and sends the
-// responder's ElligatorSwift-encoded public key followed by garbage. For a v1
-// initiator, it returns ErrUseV1Protocol without performing v2 cryptography.
+// protocol. For a v2 initiator, it sends the responder's ElligatorSwift key
+// after the first v1-prefix mismatch, receives the rest of the initiator key,
+// and initializes the responder ciphers. Key generation and key agreement use
+// separate CPU admissions, and neither admission is held during network I/O.
+// For a v1 initiator, it returns ErrUseV1Protocol without performing v2
+// cryptography.
func (p *Peer) RespondV2Handshake(garbageLen int, net BitcoinNet) error {
v1Prefix := createV1Prefix(net)
@@ -470,10 +471,27 @@ func (p *Peer) RespondV2Handshake(garbageLen int, net BitcoinNet) error {
return ErrUseV1Protocol
}
-// prepareResponderHandshake receives the rest of the initiator's key and
-// performs all CPU-bound responder setup without holding a resource admission
-// across a network read or write.
+// prepareResponderHandshake generates and sends the responder key, receives
+// the rest of the initiator key, then initializes the responder ciphers. The
+// two CPU-bound phases use separate admissions so neither lease spans network
+// I/O.
func (p *Peer) prepareResponderHandshake(garbageLen int, net BitcoinNet) error {
+ release, err := p.acquireResponderAdmission()
+ if err != nil {
+ return err
+ }
+
+ data, err := p.generateResponderKey(garbageLen, release)
+ if err != nil {
+ return err
+ }
+
+ log.Debugf("Sending ellswift pubkey and garbage (total_len=%d)",
+ len(data))
+ if _, err := p.Send(data); err != nil {
+ return err
+ }
+
remaining := 64 - len(p.receivedPrefix)
recvData, _, err := p.Receive(remaining)
if err != nil {
@@ -492,49 +510,58 @@ func (p *Peer) prepareResponderHandshake(garbageLen int, net BitcoinNet) error {
return errWrongNetV1Peer
}
- release := func() {}
- if p.responderAdmission != nil {
- release, err = p.responderAdmission.Acquire()
- if err != nil {
- return err
- }
- if release == nil {
- release = func() {}
- }
- }
-
- err = p.initializeResponder(net, release)
+ release, err = p.acquireResponderAdmission()
if err != nil {
return err
}
- data, err := p.generateKeyAndGarbage(garbageLen)
- if err != nil {
- return err
+ return p.initializeResponder(net, release)
+}
+
+// acquireResponderAdmission reserves one CPU-bound responder phase.
+func (p *Peer) acquireResponderAdmission() (func(), error) {
+ if p.responderAdmission == nil {
+ return func() {}, nil
}
- log.Debugf("Sending ellswift pubkey and garbage (total_len=%d)",
- len(data))
- _, err = p.Send(data)
+ release, err := p.responderAdmission.Acquire()
+ if err != nil {
+ return nil, err
+ }
+ if release == nil {
+ release = func() {}
+ }
- return err
+ return release, nil
}
-// initializeResponder performs the responder's CPU-bound key generation, key
-// agreement, and cipher setup. The admission is released before this method
-// returns so callers never hold it across a network write.
-func (p *Peer) initializeResponder(net BitcoinNet, release func()) error {
+// generateResponderKey performs the responder's CPU-bound key generation. The
+// admission is released before this method returns so callers never hold it
+// across a network write.
+func (p *Peer) generateResponderKey(
+ garbageLen int, release func(),
+) ([]byte, error) {
+
defer release()
var err error
p.privkeyOurs, p.ellswiftOurs, err = ellswift.EllswiftCreate()
if err != nil {
log.Errorf("Failed to create ellswift keypair: %v", err)
- return err
+ return nil, err
}
log.Tracef("Created ellswift keypair, pubkey=%x", p.ellswiftOurs)
+ return p.generateKeyAndGarbage(garbageLen)
+}
+
+// initializeResponder performs the responder's CPU-bound key agreement and
+// cipher setup. The admission is released before this method returns so
+// callers never hold it across further network I/O.
+func (p *Peer) initializeResponder(net BitcoinNet, release func()) error {
+ defer release()
+
var ellswiftTheirs [64]byte
copy(ellswiftTheirs[:], p.receivedPrefix)
@@ -603,8 +630,9 @@ func createV1Prefix(net BitcoinNet) []byte {
}
// completeKeyExchange receives the remote key and initializes the packet
-// ciphers. RespondV2Handshake performs this phase early for responders so its
-// CPU admission can be released before any response is written.
+// ciphers. RespondV2Handshake performs this phase early for responders so its
+// CPU admission is released before CompleteHandshake writes the garbage
+// terminator and encrypted version packet.
func (p *Peer) completeKeyExchange(initiating bool, net BitcoinNet) error {
if p.responderReady {
return nil
Why this scored 48/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.