Skip to content
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

discv5: type confusion of WHOAREYOU and handshake attempt #131

Closed
AgeManning opened this issue Dec 7, 2019 · 8 comments · Fixed by #157
Closed

discv5: type confusion of WHOAREYOU and handshake attempt #131

AgeManning opened this issue Dec 7, 2019 · 8 comments · Fixed by #157

Comments

@AgeManning
Copy link
Contributor

AgeManning commented Dec 7, 2019

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 fjl changed the title Node-ID ambiguity discv5: Node-ID ambiguity with multiple nodes on same IP Dec 8, 2019
@fjl
Copy link
Collaborator

fjl commented Dec 8, 2019

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
Copy link
Contributor Author

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
Copy link
Collaborator

fjl commented Dec 10, 2019

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 fjl reopened this Mar 8, 2020
@fjl fjl changed the title discv5: Node-ID ambiguity with multiple nodes on same IP discv5: type confusion of WHOAREYOU and handshake attempt Mar 8, 2020
@fjl
Copy link
Collaborator

fjl commented Mar 8, 2020

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
Copy link
Collaborator

fjl commented Mar 9, 2020

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:

  • When the destination node ID is wrong, the recipient will compute the wrong MAC and ignore the packet because it doesn't match.
  • Adding the protocol name into the MAC means we can use this tag construction to reliably distinguish whether an incoming packet belongs to discv5 or to any other UDP protocol on the same socket.

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
Copy link
Collaborator

fjl commented Mar 11, 2020

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

@AgeManning
Copy link
Contributor Author

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
Copy link
Collaborator

fjl commented May 20, 2020

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

@fjl fjl mentioned this issue Jul 29, 2020
2 tasks
@fjl fjl added this to the Discovery v5 milestone Oct 1, 2020
@fjl fjl closed this as completed in #157 Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants