multi: introduce v2transport and implement BIP324 #2260

Crypt-iQ · 2024-10-02T16:10:32Z

Depends on #2219

This PR implements the BIP324 encrypted version 2 handshake. The service flag is added to the list of supported services so that btcd will optimistically try to make v2 connections.

The BIP324 ElligatorSwift test vectors are also included.

This package introduces FSChaCha20 and FSChaCha20Poly1305 primitives that are behind the v2 encrypted handshake. Handshake code is also introduced which allows a caller to initiate a v2 handshake.

This adds the v2 software flag to the list of services btcd supports and adds functionality to optionally use v2 transport if the peer supports it.

saubyk · 2024-10-07T23:41:30Z

cc: @kcalvinalvin for review

kcalvinalvin · 2024-10-12T14:03:44Z

Haven’t yet taken a look at other parts in details but seems like the PR is missing NODE_P2P_V2 signaling support. Not sure if this part of the code was out of scope for the PR but

Should add


SFNodeP2PV2 = 1 << 11

here:

btcd/wire/protocol.go

Lines 72 to 105 in 24eb815

    
           const ( 
        
           	// SFNodeNetwork is a flag used to indicate a peer is a full node. 
        
           	SFNodeNetwork ServiceFlag = 1 << iota 
        
           	// SFNodeGetUTXO is a flag used to indicate a peer supports the 
        
           	// getutxos and utxos commands (BIP0064). 
        
           	SFNodeGetUTXO 
        
           	// SFNodeBloom is a flag used to indicate a peer supports bloom 
        
           	// filtering. 
        
           	SFNodeBloom 
        
           	// SFNodeWitness is a flag used to indicate a peer supports blocks 
        
           	// and transactions including witness data (BIP0144). 
        
           	SFNodeWitness 
        
           	// SFNodeXthin is a flag used to indicate a peer supports xthin blocks. 
        
           	SFNodeXthin 
        
           	// SFNodeBit5 is a flag used to indicate a peer supports a service 
        
           	// defined by bit 5. 
        
           	SFNodeBit5 
        
           	// SFNodeCF is a flag used to indicate a peer supports committed 
        
           	// filters (CFs). 
        
           	SFNodeCF 
        
           	// SFNode2X is a flag used to indicate a peer is running the Segwit2X 
        
           	// software. 
        
           	SFNode2X 
        
           	// SFNodeNetWorkLimited is a flag used to indicate a peer supports serving 
        
           	// the last 288 blocks. 
        
           	SFNodeNetworkLimited = 1 << 10

And other necessary code at:

btcd/server.go

Lines 2723 to 2743 in 24eb815

    
           func newServer(listenAddrs, agentBlacklist, agentWhitelist []string, 
        
           	db database.DB, chainParams *chaincfg.Params, 
        
           	interrupt <-chan struct{}) (*server, error) { 
        
           	services := defaultServices 
        
           	if cfg.NoPeerBloomFilters { 
        
           		services &^= wire.SFNodeBloom 
        
           	} 
        
           	if cfg.NoCFilters { 
        
           		services &^= wire.SFNodeCF 
        
           	} 
        
           	if cfg.Prune != 0 { 
        
           		services &^= wire.SFNodeNetwork 
        
           	} 
        
           	amgr := addrmgr.New(cfg.DataDir, btcdLookup) 
        
           	var listeners []net.Listener 
        
           	var nat NAT 
        
           	if !cfg.DisableListen { 
        
           		var err error

kcalvinalvin

Initial round of review. Went through BIP324 and compared the flows and the pseudocode provided in the BIP with the code in transport.go and chacha.go

kcalvinalvin · 2024-10-12T14:33:40Z

v2transport/transport.go

+		if err != nil {
+			return err
+		}
+


Seems like we’re missing the check for the below which is stated in bip324.

If the first 4 received bytes do not match the network magic, but the 12 bytes after that do match the version message encoding above, implementations may interpret this as a v1 peer of a different network, and disconnect them.

Oh hmm seems like this is just the pseudocode specified in the BIP. Maybe it's the BIP text that I'm confusing.

kcalvinalvin · 2024-10-14T11:32:45Z

v2transport/transport.go

+		return err
+	}
+
+	var data []byte


We can allocate here instead of the append at the bottom doing the allocation for us.

data := make([]byte, 0, 64+garbageLen)

kcalvinalvin · 2024-10-14T11:35:16Z

v2transport/transport.go

+
+	versionBytes := []byte("version\x00\x00\x00\x00\x00")
+
+	v1Prefix = append(v1Prefix, magic[:]...)


Can pre-allocate v1Prefix

v1Prefix := make([]byte, 0, 4+12)

kcalvinalvin · 2024-10-14T11:47:12Z

v2transport/transport.go

+
+			// Send over our ElligatorSwift-encoded pubkey followed by our
+			// randomly generated garbage.
+			var data []byte


Same here. Can pre-allocate.

data := make([]byte, 0, 64+garbageLen)

Actually maybe refactor this out into a function since it's the same exact code.

kcalvinalvin · 2024-10-15T04:53:32Z

v2transport/transport.go

+
+	var receivedPrefix []byte
+	if initiating {
+		receivedPrefix = make([]byte, 0)


This doesn't do anything right? Maybe something like

receivedPrefix = make([]byte, 0, 64)

Since the length is still 0 but you've now pre-allocated.

kcalvinalvin · 2024-10-15T08:49:16Z

v2transport/chacha.go

+func NewFSChaCha20(initialKey []byte) (*FSChaCha20, error) {
+	var initialNonce [12]byte
+	numRekeys := 0
+	binary.LittleEndian.PutUint64(initialNonce[4:12], uint64(numRekeys))


s/uint64(numRekeys)/0? Since we're not using numRekeys anywhere else.

kcalvinalvin · 2024-10-15T08:55:20Z

v2transport/transport.go

+
+	header := []byte{byte(ignoreNum)}
+
+	plaintext := make([]byte, 0)


We can pre-allocate here:

// 1 for the header plaintext := make([]byte, 0, contentLen+1)

kcalvinalvin · 2024-10-15T08:58:54Z

v2transport/transport.go

+		return nil, 0, err
+	}
+
+	encPacket := make([]byte, 0)


Can pre-allocate here:

encPacket := make([]byte, 0, len(encContentsLen)+len(aeadCiphertext))

kcalvinalvin · 2024-10-15T09:07:08Z

v2transport/transport.go

+			return nil, err
+		}
+
+		plaintext, err := p.recvP.Decrypt(aad, aeadCiphertext)


In the BIP there's a check to see if the plaintext is nil or empty. Not sure if this is a requirement or implementation specific but brining it up just to sanity check

kcalvinalvin · 2024-10-15T15:14:47Z

v2transport/chacha.go

+	// XOR the text with the keystream to get either the cipher or plaintext.
+	textLen := len(text)
+	dst := make([]byte, textLen)
+	f.cipher.XORKeyStream(dst, text)


Do we not need th block_counter that's mentioned in the BIP because the standard library handles it all for us? Was a bit confused reading the code for Crypt since it was missing get_keystream_bytes and was wondering if the relevant code isn't needed for us.

litecomb · 2024-11-02T07:34:12Z

WriteV2MessageN should allow specify the garbage []byte, if nil default garbage is used
ReadV2MessageN should allow reading the garbage []byte

Rationale: this will permit custom nonstandard packets communication in the protocol, while still being compatible with bitcoin.

litecomb · 2024-11-09T12:57:34Z

It would be great if v2transport wasn't promoted to go module to ease reviewing

Crypt-iQ force-pushed the v2_transport branch from c3c14ea to 8097c4e Compare October 2, 2024 16:17

Crypt-iQ added 4 commits October 2, 2024 12:18

ellswift: introduce ElligatorSwift encoding and decoding funcs

379bc04

The BIP324 ElligatorSwift test vectors are also included.

v2transport: introduce FSChaCha20, FSChaCha20Poly1305 handshake

9460f4c

This package introduces FSChaCha20 and FSChaCha20Poly1305 primitives that are behind the v2 encrypted handshake. Handshake code is also introduced which allows a caller to initiate a v2 handshake.

wire: introduce encoding/decoding funcs for v2 messages

4f34144

server.go+peer: enable v2 functionality in peer code

b19bba0

This adds the v2 software flag to the list of services btcd supports and adds functionality to optionally use v2 transport if the peer supports it.

Crypt-iQ force-pushed the v2_transport branch from 8097c4e to b19bba0 Compare October 2, 2024 16:18

kcalvinalvin reviewed Oct 15, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

multi: introduce v2transport and implement BIP324 #2260

multi: introduce v2transport and implement BIP324 #2260

Crypt-iQ commented Oct 2, 2024

saubyk commented Oct 7, 2024

kcalvinalvin commented Oct 12, 2024

kcalvinalvin left a comment

kcalvinalvin Oct 12, 2024

kcalvinalvin Oct 15, 2024

kcalvinalvin Oct 14, 2024

kcalvinalvin Oct 14, 2024

kcalvinalvin Oct 14, 2024

kcalvinalvin Oct 14, 2024

kcalvinalvin Oct 15, 2024

kcalvinalvin Oct 15, 2024

kcalvinalvin Oct 15, 2024

kcalvinalvin Oct 15, 2024

kcalvinalvin Oct 15, 2024

kcalvinalvin Oct 15, 2024

litecomb commented Nov 2, 2024

litecomb commented Nov 9, 2024


		versionBytes := []byte("version\x00\x00\x00\x00\x00")

		v1Prefix = append(v1Prefix, magic[:]...)


		header := []byte{byte(ignoreNum)}

		plaintext := make([]byte, 0)

multi: introduce v2transport and implement BIP324 #2260

Are you sure you want to change the base?

multi: introduce v2transport and implement BIP324 #2260

Conversation

Crypt-iQ commented Oct 2, 2024

saubyk commented Oct 7, 2024

kcalvinalvin commented Oct 12, 2024

kcalvinalvin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

litecomb commented Nov 2, 2024

litecomb commented Nov 9, 2024