Btcd disconnects upon receiving sendaddrv2

[Sjors]

Bitcoin Core master, soon to be released, introduces the new sendaddrv2 message (BIP 155). When such a Bitcoin Core node connects to a btcd node (tag v0.21.0-beta) it immediately disconnects:

2020-11-12 18:31:13.935 [ERR] PEER: Can't read message from x.x.x.x:42192 (inbound): ReadMessage: unhandled command [sendaddrv2]
2020-11-12 18:31:13.933 [INF] SYNC: New valid peer x.x.x.x:42192 (inbound) (/Satoshi:0.20.99/)
2020-11-12 18:31:13.946 [INF] SYNC: Syncing to block height 656631 from peer x.x.x.x:42192
2020-11-12 18:31:13.946 [INF] SYNC: Lost peer x.x.x.x:42192 (inbound)

On bitcoind side:

2020-11-12T18:31:13Z Added connection peer=6606
2020-11-12T18:31:13Z sending version (103 bytes) peer=6606
2020-11-12T18:31:13Z send version message: version 70016, blocks=656631, us=[::]:0, peer=6606
2020-11-12T18:31:13Z received: version (113 bytes) peer=6606
2020-11-12T18:31:13Z sending verack (0 bytes) peer=6606
2020-11-12T18:31:13Z sending sendaddrv2 (0 bytes) peer=6606
2020-11-12T18:31:13Z sending getaddr (0 bytes) peer=6606
2020-11-12T18:31:13Z receive version message: /btcwire:0.5.0/btcd:0.21.0/: version 70013, blocks=656631, us=x.x.x.x:42192, peer=6606
2020-11-12T18:31:13Z received: verack (0 bytes) peer=6606
2020-11-12T18:31:13Z New outbound peer connected: version: 70013, blocks=656631, peer=6606 (full-relay)
2020-11-12T18:31:13Z sending sendheaders (0 bytes) peer=6606
2020-11-12T18:31:13Z sending ping (8 bytes) peer=6606
2020-11-12T18:31:13Z sending addr (31 bytes) peer=6606
2020-11-12T18:31:13Z initial getheaders (656630) to peer=6606 (startheight:656631)
2020-11-12T18:31:13Z sending getheaders (1029 bytes) peer=6606
2020-11-12T18:31:13Z sending feefilter (8 bytes) peer=6606
2020-11-12T18:31:13Z received: reject (110 bytes) peer=6606
2020-11-12T18:31:13Z Unknown command "reject" from peer=6606
2020-11-12T18:31:13Z socket recv error for peer=6606: Connection reset by peer (104)
2020-11-12T18:31:13Z disconnecting peer=6606
2020-11-12T18:31:13Z Cleared nodestate for peer=6606

I tried with and without the new peerblockfilters feature, but that doesn't seem to matter.

This may lead to node loneliness. It also happens to Lnd nodes, see breez/breezmobile#408

This issue did not occur as of a slightly older Bitcoin Core commit, e.g. bitcoin/bitcoin@1d3ec2a

[Roasbeef]

Thanks for reporting this! The easiest way to resolve this is to permit unknown messages rather than disconnecting as we do now. However, what's puzzling to me is why a new message was introduced (sendaddrv2) in order to introduce a new message (addrv2), vs just bumping the p2p protocol version and only sending this new message to peers that understand it. In the past, we adopted this defensive measure (to D/C if we get an unknown message) as accepting unknown messages can possibly open up DoS vectors.

[TheBlueMatt]

However, what's puzzling to me is why a new message was introduced...vs just bumping the p2p protocol version

It depends on who you ask, but I think philosophically there are many folks (many of whom work in Core) who argue a p2p protocol with many implementations should never be dependent on a single version number to describe all the features supported. This is why Core has used empty messages to communicate protocol support for years.

accepting unknown messages can possibly open up DoS vectors.

Can you elaborate more on this? Throwing away bytes received should never be worse than receiving a lot of pings/other useless data, so it seems strange that there would be DoS issues here.

[vasild]

However, what's puzzling to me is why a new message was introduced (sendaddrv2) in order to introduce a new message (addrv2)

addrv2 is a replacement of the existent addr. Some method is needed to coordinate whether to send addr or addrv2 to a given peer. So, by sendaddrv2 a peer asks "Please send me addrv2 instead of addr".

[laanwj]

However, what's puzzling to me is why a new message was introduced...vs just bumping the p2p protocol version

FWIW bumping the protocol version was my initial proposal, but everyone preferred the solution to send a message for signalling instead for the sake of other implementations (e.g. having only part of the feature set but still wanting to use TorV3). For the record I agree with @TheBlueMatt's reasoning about permissionless in introducing features, but I am extremely disappointed that it turns out to be the method that is most problematic for other implementations.

[maflcko]

If bitcoin/bitcoin#20564 is merged, it will act as a temporary workaround to give btcd a bit of time to write and deploy a fix. It looks like there are already plans to implement a solution:

Thanks for reporting this! The easiest way to resolve this is to permit unknown messages rather than disconnecting as we do now.

[jakesylvestre]

Can you elaborate more on this? Throwing away bytes received should never be worse than receiving a lot of pings/other useless data, so it seems strange that there would be DoS issues here.

@Roasbeef feel free to correct me here, but I think the concern is you could have peer(s) that keep sending garbage commands that would otherwise be dropped

[sipa]

@jakesyl Of course, peers could be sending garbage. But if that's a problem, then peers spamming you with ping commands or tx messages would be even more of a problem. For that reason disconnecting on unknown messages doesn't protect against attackers - they'd just use an easier mechanism anyway - but it does make it harder to use send* style messages for feature negotiation.

[jakesylvestre]

got it- thanks for the clarification @sipa!