v2transport: restore responder handshake progress

In this commit, we send the responder key as soon as the initiator diverges
from the v1 prefix. This avoids the BIP324 deadlock where both sides can wait
for the other to write before the initiator has sent its complete 64-byte
key.

We split key generation and key agreement into separate admission leases,
and release each lease before the intervening network I/O. The regression
test stops after the first mismatch, requires the responder to make
progress, then completes key agreement and verifies the resulting packet
ciphers interoperate.
This commit is contained in:
Olaoluwa Osuntokun 2026-07-20 17:18:10 -05:00
parent 95c11c560a
commit 097178713e
2 changed files with 233 additions and 88 deletions

View file

@ -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)
// 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)
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())
}
})
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) {

View file

@ -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
}
log.Debugf("Sending ellswift pubkey and garbage (total_len=%d)",
len(data))
_, err = p.Send(data)
return err
return p.initializeResponder(net, release)
}
// 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 {
// acquireResponderAdmission reserves one CPU-bound responder phase.
func (p *Peer) acquireResponderAdmission() (func(), error) {
if p.responderAdmission == nil {
return func() {}, nil
}
release, err := p.responderAdmission.Acquire()
if err != nil {
return nil, err
}
if release == nil {
release = func() {}
}
return release, nil
}
// 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