search

n0-computer / iroh

A toolkit for building distributed applications
https://iroh.computer
Apache License 2.0
2.57k stars 161 forks source link

Specify `src_ip` correctly in `Transmit`s in `iroh-net` #2632

Open matheus23 opened 2 months ago

matheus23 commented 2 months ago

Currently, the src_ip values for quinn::Transmits are indirectly set by us via our rewriting of dst_ip (and choice of dst_ip in the relay case) in the RecvMeta of poll_recv (in MagicSock).

This can cause issues, e.g. when unspecified IP addrs (0.0.0.0 or ::) make it to src_ip. Windows seems more pedantic about this and errors out.

We return unspecified IP addrs, because we set dst_ip to MagicSock::normalized_local_addr.

As an interim solution, we've set it to None on windows. I suspect this can cause issues when there's multiple IP interfaces connected to the device and the OS ends up choosing the wrong one.

Instead, we should probably figure out the interface we want to send from via netcheck.

More information in this PR comment.

Ralith commented 2 months ago

I suspect this can cause issues when there's multiple IP interfaces connected to the device and the OS ends up choosing the wrong one.

The problem also arises when you have multiple addresses on the same subnet on the same interface. IIRC this is the default on Windows, and a supported configuration on Linux, due to RFC 4941 privacy extensions: a host may have both a stable EUI-64 address, a primary temporary address, and one or more old temporary addresses being turned down, all on the same WAN subnet. An IPv6-based protocol is unlikely to be able to establish a connection correctly in this environment without correctly specifying source addresses.

As a general rule of thumb, you should send from whatever address you received on, for a given path.

flub commented 2 months ago

(I previously wrote this on discord somewhere, repeating here)

We need to store the dst_ip of the RecvMeta in the PathState and when we return the "addr_for_send" it should be returned with it so that the send path can put it in src_ip.

I'm not sure if this should be blocking for the transition to 0.11, what do you think @matheus23 ?

I do think we should do the same on all platforms regardless.

matheus23 commented 2 months ago

We need to store the dst_ip of the RecvMeta in the PathState and when we return the "addr_for_send" it should be returned with it so that the send path can put it in src_ip.

Yeah agreed, we should do this & do it like this.

I'm not sure if this should be blocking for the transition to 0.11, what do you think @matheus23 ?

I don't think this should be blocking the transition.