-
Notifications
You must be signed in to change notification settings - Fork 5.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add BIP324: v2 P2P Transport Protocol #1378
Conversation
bip-0324.mediawiki
Outdated
|<code>GETCFHEADERS</code>||<code>CFHEADERS</code>||<code>GETCFCHECKPT</code>||<code>CFCHECKPT</code> | ||
|- | ||
!+44 | ||
|<code>WTXIDRELAY</code>||<code>ADDRV2</code>||<code>SENDADDRV2</code>||(undefined) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
VERSION
, VERACK
,SENDHEADERS
, WTXIDRELAY
, and SENDADDRV2
are only sent at most once per connection; wouldn't it make sense to reserve single byte message types for things that will give a meaningful benefit from compression? (EDIT: GETADDR
too, arguably)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are 210 additional message type IDs still available to allocate. In the future, if we did come close to using them up, a future upgrade could interpret 0xFF as "the next byte(or more than one) is also a part of the message type ID". Given that, it made sense to allocate an ID to all message types still in use.
|<code>GETDATA</code>||<code>GETHEADERS</code>||<code>HEADERS</code>||<code>INV</code> | ||
|- | ||
!+28 | ||
|<code>MEMPOOL</code>||<code>MERKLEBLOCK</code>||<code>NOTFOUND</code>||<code>PING</code> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|(8 byte string)||(9 byte string)||(10 byte string)||(11 byte string) | ||
|- | ||
!+12 | ||
|(12 byte string)||<code>ADDR</code>||<code>BLOCK</code>||<code>BLOCKTXN</code> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason to expect clients supporting this BIP to use ADDR
rather than ADDRV2
?
| <code>message_payload</code> || <code>message_length</code> || message payload | ||
|} | ||
|
||
If the first byte of <code>message_type</code> is in the range ''1..12'', it is interpreted as the number of ASCII bytes that follow for the message type. If it is in the range ''13..255'', it is interpreted as a message type ID. This structure results in smaller messages than the v1 protocol as most messages sent/received will have a message type ID.<ref name="smaller_messages">'''How do the length between v1 and v2 compare?''' For messages that use the 1-byte short message type ID, v2 packets use 3 bytes less per message than v1.</ref> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updating this table seems like it might be annoying (both if proposals made at the same time both choose the same byte, and if a proposal using just an ascii string wants to add support for a one byte id, while remaining compatible with nodes that aren't aware of the new id) -- it seems like it would serialize adding features in a similar way to having to bump the protocol version number to indicate support for a feature.
Did you consider a more generic approach, like just running the protocol through a compression stage prior to encryption, and letting that automatically reduce commonly used message types into a shorter byte stream?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the time that I worked on it, we did not consider such a generic approach. However, I imagine such an approach might come with new dependencies and an increased surface area?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we really considered that possibility in this context.
Generic compression may be useful, but for it to be useful for tiny messages, I think we'll need some kind of training data or dictionary approach; generic compression without that really only is beneficial after sufficient amounts of data.
For example zstd compression have standard tooling for constructing dictionary files based on training data. I could imagine an extension that uses that, either with a single dictionary, or a small per-message-type one. But that doesn't help us around the message coordination (either because the training data will become outdated with new messages, and switching to another dictionary is a protocol break; or, because new per-message dicts need some form of signalling... like this table).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree a custom (compression) dictionary doesn't seem workable; I was thinking of having a compression stream over the entire connection lifetime, and invoking flush
after each message, or perhaps when the uncompressed send buffer is empty.
I guess updating the mapping as a whole (this bip specifies table version 0, then later define table version 1 mapping 13..255 to some particular set of messages, then version 2 later provides some other mapping), and listing out the table versions you both support either using the VERSION message, or via some new message prior to VERACK, and then using the highest version that you both support once VERACK is completed would work. Perhaps message type 0 could be used for VERSION, since there's presumably no way it can be changed while retaining compatibility?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alternative is adding a feature later where the sender just announces their mapping once ("I will use byte <byte>
for message type <string>
), possibly so that any byte not listed retains its original assignment. That means a one-time up-to-3KiB cost for a fully custom set, but removes all coordination issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're mapping 44=WTXIDRELAY
then the receiver presumably either knows how to interpret WTXIDRELAY
, and can map it to a one or two byte enum MSG_WTXIDRELAY
, or doesn't know how to interpret WTXIDRELAY
and can map it to the same one/two byte enum as MSG_INVALID
or MSG_IGNORE
. So really this would be an up to 3kB message to announce your mappings but only 250 to 500 bytes per peer to actually track the mapping.
Since TLS isn't being used is there any way to recover "virtual host" functionality? This is necessary for addressing multiple nodes behind the same NAT which is increasingly common these days. |
The Transport layer specification says:
What does "typically TCP/IP" mean? Will UDP based connections be supported by BIP-324 as well? I'm asking, because I stumbled over this
Any thoughts? |
@yojoe The encrypted protocol we are proposing requires state and the order of packets is important. So we require a stream interface and can't use UDP. |
@dhruv @yojoe We should probably clarify that BIP324 assumes a reliable stream interface to run on top of, as the statefulnes of the Bitcoin P2P needs the same, so that's what we're targetting. The text says "typically" TCP/IP, because it can also run over whatever else provides that (in particular, Tor or I2P also do, and are used as layers to run the P2P protocol over). |
So BIP324 "could" run on QUIC and similar protocols, which use UDP instead of TCP to provide reliable streams over multiplexed connections? With quite a lot of P2P applications moving to QUIC (UDP) right now (at least in my perception) and keeping TCP only as a fallback, I think two or three sentences on "Why not QUIC?" would help the BIP324 spec. |
You can forward different port numbers to different machines on the NAT, if that's what you want. The P2P protocol already relays full IP/port combinations, and since version 23.0 Bitcoin Core also can keep track of different ports on the same IP as separate entities. If you're referring to some kind of (v)host functionality, as it exists in HTTP(S), where the client sends the server an actual hostname it's trying to connect to, so that multiple hostnames can be served from a single IP: no; I think that would be a significant complication for little/no gain. The P2P protocol has no notion of hostnames right now, and adding something like that would require changes not just to the transport protocol but also to IP address relay to convey these. It'd also require running a trusted service on the NAT to properly dispatch to what's behind it, so you'd have to wonder if that same entity couldn't just run a single node for all users behind it (who could run their own nodes just connected to that one). |
In theory, yes. I don't think there is that much to gain from QUIC, as we don't care about latency of connection set-up (we have very long-lived connections in general), integrated TLS encryption, or multiple parallel datastreams. I could imagine making use of the latter, but that'd need a much deeper redesign of the whole P2P protocol to make use of. In BIP324 we're just looking at improving the transport layer of the protocol. |
bip-0324.mediawiki
Outdated
TRANSPORT_VERSION = b'' | ||
NETWORK_MAGIC = b'\xf9\xbe\xb4\xd9' # Mainnet network magic; differs on other networks. | ||
V1_PREFIX = NETWORK_MAGIC + b'version\x00' | ||
|
||
def respond_v2_handshake(peer, garbage_len): | ||
peer.received_prefix = b"" | ||
while len(prefix) < 12: | ||
peer.received_prefix += receive(peer, 1) | ||
if peer.received_prefix[-1] != V1_PREFIX[len(peer.received_prefix) - 1]: | ||
peer.privkey_ours, peer.ellswift_ours = ellswift_create(initiating=False) | ||
peer.sent_garbage = rand_bytes(garbage_len) | ||
send(peer, ellswift_Y + peer.sent_garbage) | ||
return | ||
use_v1_protocol() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I correct in assuming that this inbound downgrade logic only exists because service flags are additive and v1 clients will make connection to addresses that have the v2 service bit?
Could it make more sense to introduce a new BIP155 network type instead of using the service flags? All the current network types essentially define a separate transport/network protocol (IPv{4,6}, Tor, I2P, CJDNS) while the current service flags are more about what type of application data a node serves (blocks, witnesses, compact filters, bloom filters).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So your proposal is something like:
- Add new BIP155 network types TRANSV2_IPV4, TRANSV2_IPV6 (and perhaps TRANSV2_TOR_V3, etc), corresponding to existing V1 transports on those networks (with similar IP/hash encoding).
- Allow nodes to support just v1, just v2, or both if done on separate ports.
- Drop the ability to create a v2 listener that can detect inbound v1 connections on the same port.
I do agree that's somewhat cleaner as service flags aren't a perfect match, but using separate networks has downsides too (roughly in decreasing order of severity):
- To avoid creating a heightened partition risk between v1 and v2, I think we need to expect that most nodes support both, at least on inbound, probably for a long time. And if that is the case, the introduction of new networks means effectively a doubling of addrv2 data rumoured on the network, as well as a duplication of addrman entries (or equivalent in other implementations).
- V2 address propagation will initially be hampered by being treated as unknown network by old nodes (a bootstrapping problem every new network has).
- Requiring every node to listen on two ports if they want to support v2 also means that in some instances, upgrading needs system administrator intervention to open ports.
- Special logic is needed so that if a node knows about a peer's IP in both V1 and V2 networks, it can prefer the V2 one.
- Anti-DOS logic would need to be special cased to treat V1 and corresponding V2 networks as the same (as clearly control over a netrange in one implies control over the same netrange in the other).
So I think that using service flags is a nice workaround for these issues, as it naturally implies the V1 and V2 "entry" to a node will be thought of as a single entity for the purpose of address relay, even to old nodes. It does mean listeners for now need auto-detection logic to be able to serve both V1 and V2 on the same port, but I think that's mostly an implementation issue.
The way to think about the service flags is that it is a hint to try V2 connections to a node. If the service flags incorrectly contains V2, the responder will almost certainly immediately disconnect, which the initiator interprets as a "retry as V1" per BIP324. Once a connection is established, using either V1 or V2, the initiator's knowledge of the responder's service flags will be corrected using the VERSION message, so even if the flags incorrectly say V1, the initiator can then still decide to reconnect as V2 (and if not, they will the next time a connection is established).
So it's just a hint, and not a disaster if it's wrong in either direction. It's an optimistic optimization over just not having any service flag at all, and make every V2 initiator always try V2 first and fall back to V1 on every connection. Given a very effective P2P attacker which can "or in" the V2 flag to every rumoured service flag, the service flag approach effectively becomes identical to not having the flags in the first place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bitcoin P2P connections are asymmetric: one peer connects to the other peer, and that other peer must be known by some id. For now, the connection happens by the network address (which serves that id). The proposed wire protocol doesn't provide any means of authenticating other peer, which opens possibilities for MiTs, and argues that this increases privacy. I see two contradictions:
- the connecting peer knows the peer it connects to; no anonymity here is gained;
- not authenticating the remote peer keeps the possibility for MiT attacks from the current wire protocol.
I think that "exchange-known" key scheme (like in Noise_XK) is more adequate for bitcoin P2P than "exchange-exchange", as in the current proposal (and like in Noise_XX).
@dr-orlovsky The proposal isn't to not authenticate at all; it's to leave authentication for a future improvement, in cases where it is desired. As the text argues, in a Bitcoin P2P setting, authentication is only relevant in cases where the initiator makes a deliberate connection to a peer with whom they have some out-of-band trust relation, and doesn't matter for automatic connections. That optionality makes it undesirable to integrate in the key exchange (as that would make it mandatory), but nothing prevents it from being done separately, in a post-key-exchange, optional step when desired (that's easy to do, but comes at the cost of an extra roundtrip, which many transport protocols prioritize; our P2P protocol connections are so long-lived this doesn't matter). And this isn't just a matter of avoiding a bit of complexity: using something like Noise_XK would require all listening nodes to have a known public key. I think that's something we actively don't want, as it would introduce a notion of identities to the protocol that must be known for it to operate at all. FWIW, we're also working on a protocol for an optional authentication scheme without discoverable identities, and with the property that a MitM can't learn anything about the desired/provided public/private keys. That will take time, as we'd want academic scrutiny before actually proposing it. There are already tangible benefits to deploying opportunistic encryption without authentication at all, so we don't feel like waiting for that is desirable either. It's also only one of a possible number of approaches that could be taken w.r.t. authentication. |
That is exactly the point I am trying to make: I think that even for automatic connections it might be desirable to know the identity of the peer. The requirement to always have a remote pubkey is not that hard to meet: the remote peers are listed either by the config or via P2P messages, and both of them can require to provide the remote identities alongside network address, as it is done in LN. At the same time I agree that the abstraction of authentication from encryption is a good thing to have. What is not good is to build a new P2P stack for bitcoin without paying attention to authentication. |
It's not hard; it's just a bad idea. Bitcoin nodes don't, and shouldn't, have identities. |
Isn't IP address an identity? Or Onion address? Or any other networking address? Having public key next to it is just a second factor: it doesn't add to the leak of a privacy or doesn't make the node "more identifiable" at all; it is just a second factor ensuring that the existing identity is not stolen by a MitM |
An IP can be used as an identity, and in some cases it is, and that's a bad idea. It's that scenario where adding authentication makes sense, e.g. people configuring manual IP addresses of their own nodes or people they know. But in the case of automatic connections, the IP addresses are just endpoints. They're not identities in the sense that they're not trusted (nodes verify all incoming data), there is no reputation, and there is no cost to creating a new one. Effectively, every node is equally (dis)trusted, and adding cryptographic identities for that is just pointless. Furthermore, it's risky; e.g. you need to make sure that nodes use separate identities for each IP they are reachable on (which the node might not even be aware of) or you're instantly adding a fingerprinting vector. And when restricted to just the manual connection use case, non-discoverable identities suffice: all you need is a way to verify you're connected to the party you intended to talk to; it doesn't require telling others who you are, greatly reducing those risks. |
I think there is terminological (actually semantic) difference. There is an identity for a reputation (state that persists before connections) -- and there is an "identity" as a "second factor" to ensure that there is no forgery of some network addresses controlled by authorities (DNS, NATs etc). For sure I am not arguing about the first case (reputation, permanent identity). The point I am trying to address is that bitcoin P2P protocol probably shouldn't allow connecting to remote peers by DNS or IP addresses without ensuring that the peer behind that address is the peer which was advertised via P2P. And static public key servers that purpose (and probably it is incorrect to call it "permanent identity", but its validation against an expected value is still an authentication). |
I disagree. There is no point in ensuring anything, because there is nothing to ensure: for automatic connections, every node is equally good. In any case, I believe I understand your argument, but we fundamentally disagree about what qualities the P2P network should have. |
I'd like to understand better why you think that P2P network should have a quality that for automatic connections, every node is equally good (i.e. the rational behind that). I am not trying to persuade you, I am asking for better rationale - I think others may have similar questions and it will be nice to clarify the approach better (including the text of the document). From my understanding, Bitcoin P2P network has a quality of eclipse attack. Connecting to peers which are guaranteed to be the peers known to be well-connected - or outside of the scope of a vulnerable network segment (for instance being in US for a Chinese node) is a way to mitigate that risk. If these nodes are not authenticated to be the nodes a peer tried to connect to (i.e. proving their network address was not stolen and they are just fakes run by a Chinese government) would make eclipse attacks more probable/simple. Or you mean that such nodes would be authenticated as a layer above this spec, while all other nodes ("automatically connected") can be fakes without increasing chances of eclipse? |
Bitcoin is special among P2P networks in regards to eclipse attacks as they can be easily/cheaply detected here. All you need to do is periodically check the chain tip hash via other low-bandwidth, out-of-band channel(s). Either manually or you script it. It would be really hard for an adversary to eclipse all your information channels and stop you from learning that another (and potentially heavier) chain tip exists. Alternatively, you can already manually and explicitly add a few trusted "nodes" to the mix, e.g. Tor, I2P or CJDNS, whose address schemes are all "public key" based and don't rely on centralised authorities like DNS. So if you worry about eclipse attacks, you can already detect and somewhat counter them. But the main point is that "public key based identities" and "authenticated connections" are not the silver bullet against eclipse attacks in TRUSTLESS, PUBLIC, OPEN NETWORKS.
That would be a "web of trust" and not a "trustless" topology.
AFAIK that's exactly what the clearnet (IP4/6) peer selection algorithm already considers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've got all answers to my questions, thank you. ACK
I don't see any un-addressed concerns regarding this pull request and will merge this soon unless someone pipes up. Thanks. |
@kallewoof we have a couple small changes coming up |
It's helpful if you change to draft when you don't want it merged so maintainers know to skip over it. |
@kallewoof I'm sorry about that -- I was unclear about the workflow on the bips repo but I understand now. I have pushed up the latest changes and the working group (@sipa, @real-or-random and I) agrees it's rfm. Latest push:
|
No worries! Maybe mark BIP-151 as "Replaced" in |
@kallewoof Done. |
Supersedes #1024
Link to rendered version for your convenience: https://github.com/dhruv/bips/blob/bip324/bip-0324.mediawiki
Open questions: