p2p/discover: from address in ping is IPv6 unspecified in some cases

[axic]

While working on an independent peer implementation in ethereumjs, I've came across this weird issue. This is a decoded ping message in DHT (redacted some fields and received a similar from many nodes):

{ version: <Buffer 04>,
  address: '::',
  udpPort: 30303,
  tcpPort: 30303,
  dstAddress: '42.42.42.42', // changed this, obviously
  dstUdpPort: 30304,
  dstTcpPort: 0,
  expiration: 1452565204,
  id: <Buffer a9 79 fb 57 54 95 b8 d6 db ... >,
  port: 30303 }

The address field (from address) is actually the '::' (any/unspecified) encoded in IPv6 over the wire.

The relevant pieces of code in geth are:
https://github.com/ethereum/go-ethereum/blob/master/p2p/discover/udp.go#L216-L227
and
https://github.com/ethereum/go-ethereum/blob/master/p2p/discover/udp.go#L246

The above code does a NAT discovery for the public IP.

Is it possible the node had NAT discovery turned off? Is there some other bug there stopping the lookup from working? Shouldn't such an address (unspecified IPv4 or IPv6) be rejected in the protocol? Should IPv4 and IPv6 addresses be mixed in one ping?

[karalabe]

@fjl Could you take a look at this?

[fjl]

Is it possible the node had NAT discovery turned off? ...
Shouldn't such an address (unspecified IPv4 or IPv6) be rejected in the protocol?

Yes. This can happen when NAT punching is off or doesn't work. In this case, it will send the unspecified address because it simply does not know what its externally reachable address is. I mean, we could send 127.0.0.1, but it wouldn't help you either.

It is better to use the UDP envelope address rather than relying on the address field.

Should IPv4 and IPv6 addresses be mixed in one ping?

You are getting a 16-element address because of the internal representation in Go.
IMHO it shouldn't matter because it is unspecified either way.

[axic]

It is better to use the UDP envelope address rather than relying on the address field.

I've resorted to doing that in case unspecified is received. Maybe it would make sense to use the PONG response as an alternative to NAT traversal? e.g. update the internal representation if the sender in PING was unspecified and the PONG received has an external address.

At least documenting that an implementor should use the UDP sender address in case the PING fromAddress is unspecified.

And you are right, having unspecified is much better, than localhost if we take the rules above as a result.

You are getting a 16-element address because of the internal representation in Go.
IMHO it shouldn't matter because it is unspecified either way.

I still find a bit weird to have it mixed as they are different protocols. Would it be hard to change it?

[fjl]

UDP endpoint exchange like you suggest is planned (and the protocol supports it) but I haven't found the time to implement it. It's a bit non-trivial because it needs to look at multiple packets, otherwise this could be an attack vector.

I'll look into sending IPv4 unspecified address next time I look into discovery protocol stuff.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[axic]

@holgerd77 since you are implementing the new ethereumjs version of this, please take a look above

[holgerd77]

@axic Ok, will keep this thread in mind. I think I already stumbled about something similar - some IPv4/IPv6 switching - but couldn't make much out of it yet.

[fjl]

Fixed in #17753