radicle-dev / radicle-link

The second iteration of the Radicle code collaboration protocol.
Other
423 stars 39 forks source link

Localhost address advertisements processed #644

Open viraptor opened 3 years ago

viraptor commented 3 years ago

With the recent seed (bins commit 7780f036198a04a16d00ad706042501466f022e7) I'm seeing a lot of address advertisements for localhost which are then attempted. For example:

Apr 30 07:26:35 radicle-seed radicle-seed-node[20217]: Apr 30 07:26:35.216  WARN accept{peer_id=hyy4ebh8amk3ypyawgpjnfpbgkxqwdo5n1iif5s17yghwmewrf4d8a}:incoming:incoming{remote_id=hynjj6h373omfx5ss8qexishoy9tkc8c9r7trz1tuwokbmx9ow4xyg remote_addr=5.18.91.178:14446}:connect{remote_id=hynjj6h373omfx5ss8qexishoy9tkc8c9r7trz1tuwokbmx9ow4xyg}: librad::net::protocol::io::connections: could not connect err=Connection(TimedOut) remote_addr=127.0.0.1:49983
...
Apr 30 07:26:35 radicle-seed radicle-seed-node[20217]: Apr 30 07:26:35.216  WARN accept{peer_id=hyy4ebh8amk3ypyawgpjnfpbgkxqwdo5n1iif5s17yghwmewrf4d8a}:incoming:incoming{remote_id=hynjj6h373omfx5ss8qexishoy9tkc8c9r7trz1tuwokbmx9ow4xyg remote_addr=5.18.91.178:14446}: librad::net::protocol::tick: unreliable send error err=CouldNotConnect { to: GenericPeerInfo { peer_id: hynjj6h373omfx5ss8qexishoy9tkc8c9r7trz1tuwokbmx9ow4xyg, advertised_info: PeerAdvertisement { listen_addrs: Within { inner: [127.0.0.1:49983, 192.168.0.102:49983], _min: PhantomData, _max: PhantomData }, capabilities: {} }, seen_addrs: Within { inner: [5.18.91.178:14446], _min: PhantomData, _max: PhantomData } } }

These should be either not sent or not attempted (maybe unless seed is in some explicit debug mode?) since it won't be able to other nodes over loopback.

kim commented 3 years ago

This behaviour has not changed.

Without additional configuration, a peer cannot know if it should expect other peers to be reachable on the local network. Even if seen_addrs suggests that the remote is reachable at a public address, it could still be located in the local network (and the seen_addrs actually not routable due to hairpinning behaviour, resp. lack thereof).

The setting you introduced in #597 can be used to prevent a peer from advertising private addresses. We could say that, if Endpoint::listen_addrs does not contain any address for which IpAddr::is_global returns false, connection attempts are failed-fast.

viraptor commented 3 years ago

The private network addresses, sure - they're useful. But do you want to advertise the 127.0.0.1 as well? Are there scenarios outside of explicit local testing where loopback would be a valid option?

kim commented 3 years ago

explicit local testing

Probably not, but then we need a config option --enable-loopback =]