diff --git a/v2transport/handshake_test.go b/v2transport/handshake_test.go index 23c3820f..1a086a47 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) +// 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) { diff --git a/v2transport/transport.go b/v2transport/transport.go index 6dfd74e4..aecdd9e3 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 - } - - 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