ethereum / devp2p

Ethereum peer-to-peer networking specifications
995 stars 277 forks source link

discv5: type confusion of WHOAREYOU and handshake attempt #131

Closed AgeManning closed 4 years ago

AgeManning commented 4 years ago

I have been spinning up new nodes regularly and connecting to a boot-node. As I spin up new nodes, I create new identities and connect to a boot-node via the same public ip.

The bootnode's DHT gradually fills with a number of nodes all of the same IP (mine). Eventually I have a node running, and when the bootnode searches for peers, it goes through it's DHT and tries to connect back to me using a number of different node-ids.

The issue is that I cannot determine if the node connecting to me, is expecting me to have a different node_id that what I actually have. In fact I see the request coming from a different node_id than what the boot-node has. I therefore treat this as a new peer and try and WHOAREYOU it.

Is there a smarter way of addressing this kind of thing? Have I missed some trick in my implementation to prevent this? Ideally with a large set of peers, older nodes would get kicked from the DHT and this could just be an artefact of a low-valued DHT

fjl commented 4 years ago

Oh, so what you mean is that you restarted your test node with a different node key every time, but used the same listening address?

AgeManning commented 4 years ago

Yep, so we're running a live network. The global DHT (due to small number of peers) remembers my old node_ids, then during searches attempts to connect to my IP with old node_id's.

Nothing fails, it just would be nice to drop packets if someone tried to connect to me with the wrong node id (but this would involve changing the spec, I believe)

fjl commented 4 years ago

I think the problem will go away when the DHT grows, but if you have a spec change in mind that would fix it, please don't hesitate to submit it!

fjl commented 4 years ago

It turns out this issue is worse than I thought. Clients cannot distinguish handshake packets and WHOAREYOU if the assumed node ID is wrong:

message-packet   = tag || auth-header || message
auth-header      = [ 5 elements ]
whoareyou-packet = magic || [token, id-nonce, enr-seq]
fjl commented 4 years ago

Idea for fixing this: we can use a cheap MAC function and key it with the destination node ID. Remove the XOR and send src-node-id in clear text followed by the MAC value.

Example using siphash MAC:

tag = src-node-id || siphash(dest-node-id[:16], "discv5" || src-node-id)

The length of tag here would be 40 bytes since it is the concatenation of the 32-byte source ID and 8 bytes of siphash output. The recipient would compute the MAC using its own node ID as the key. It can determine which node ID sent the message by simply reading the first 32 bytes of tag.

This achieves two things:

The downside is that src-node-id is sent clear text. One could argue it's not much of a secret with the XOR-based tag we currently use though.

fjl commented 4 years ago

Looking again, any checksum function would work for this purpose. It doesn't need to be a MAC specifically.

AgeManning commented 4 years ago

I agree. I like the MAC-based approach. Could we not keep the xor of the src-node-id instead of sending it in plaintext?

fjl commented 4 years ago

Just posted a proposal to fix this. Since it turned out longer than I originally expected, I made a new issue for it.