search

ethereum / devp2p

Ethereum peer-to-peer networking specifications
979 stars 275 forks source link

discv5: DNS support and potential issues with IP-based connections #138

Closed AgeManning closed 4 years ago

AgeManning commented 4 years ago

Description

Discv5 currently uses the ip/ip6 fields in an ENR's in order to send UDP traffic.

The PING and PONG packets allow external IP discovery for nodes. Nodes receiving a PONG can update their ENR based new IP information received from it's peers.

It was pointed out to me that in some circumstances, (e.g AWS), traffic may be outgoing on a particular gateway which refuses incoming traffic. The IP-based discovery suggested in this protocol does not accommodate such setups.

One idea would be to optionally use a dns field inside the ENR. Node's receiving ENR's with a dns field would connect via the dns field rather than the ip field. Potentially, we could allow static ip's in the dns field also, or make a static_ip field separately.

Curious about thoughts on this / other ideas of handling such scenarios.

I wonder if there are possible ddos attack vectors where users can spam the DHT with records which have dns entries that point to a victim IP.

fjl commented 4 years ago

The way we handle it in go-ethereum is like this: "ip" and "ip6" fields are determined in multiple ways depending on configuration. The local IP discovery based on PONG happens by default, but the predicted address is overridden if a UPnP or NAT-PMP capable gateway is found or if the user specifies an explicit address (i.e. using geth -nat=extip:<IP>).

I think ENR shouldn't care too much about where the IP comes from. Implementations should just try to find the real IP. I can make this more obvious in the spec by saying that the IP discovery through PONG is optional.

Regarding the "dns" field, it does sound like a DoS vector to me because we can't verify it properly in the client. That said, we do want to support Tor onion addresses eventually, which has a similar problem. But these will never be supported in the discovery. The discovery protocol should always require the IP.

AgeManning commented 4 years ago

Sure, that ks felix. This makes sense to me and I can adjust my implementation to mirror some of the go logic

fjl commented 4 years ago

See https://github.com/ethereum/go-ethereum/blob/01d92531ee0993c0e6e5efe877a1242bfd808626/p2p/enode/localnode.go#L155 for the code that does this.

fjl commented 4 years ago

Closing because I think this is resolved as far as the specs go. We won't add "dns" in ENR, but you can determine the "ip" field however you like in your implementation.