iroh icon indicating copy to clipboard operation
iroh copied to clipboard

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

Open matheus23 opened this issue 1 year ago • 3 comments

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.

matheus23 avatar Aug 15 '24 09:08 matheus23

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.

Ralith avatar Aug 23 '24 03:08 Ralith

(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.

flub avatar Aug 23 '24 07:08 flub

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.

  • With the current setup, iroh works on windows, both on CI and when I was testing it manually in my home network (it works on my machine (tm)).
  • There was a change in quinn 0.10 to quinn 0.11 behavior: Previously, quinn-udp didn't use transmit.src_ip information, in quinn 0.11 it does. The current state in the dig/quinn11 branch simply restores the quinn 0.10 behavior.
  • It's not ideal, but I want to minimize changes from the dig/quinn11 branch. Let's get that merged with the fewest bugs possible & the least changes. Then we can start "using newer features" of quinn 11, such as being able to supply src_ip in transmits on windows (and in general use the correct ones).

matheus23 avatar Aug 23 '24 08:08 matheus23

I'm running into a situation where having src_ip set would likely help: I'm running iroh-bench locally, and I have quite a few interfaces. Among those, there's my WiFi interface wlp2s0 at 192.168.0.99, as well as some docker0 interface at 172.17.* and some bridge interfaces (?) at 172.18.* and 172.20.*.

In this case, iroh-bench runs multipath and both client and server side are bound locally to 0.0.0.0 sockets on different ports. When the client connects, it might establish path 0 by using the remote address 192.* of the server. Datagrams sent this way from client to server come in at the server in quinn with RecvMeta::addr == 192.*:<client_port>. The server will answer the connection attempt and the client sees RecvMeta::addr == 192.*:<server port> for the packets.

Eventually, the client decides to open path 1 by using one of the other interfaces, some 172.*:<server port> address. It instructs the kernel to send it from any interface, and the kernel chooses the 192.* interface. The server now sees this packet coming in as RecvMeta::addr == 192.*:<client port>, but RecvMeta::dst_ip is not set, thus it sends it again, instructing the kernel to choose any interface, which again, chooses the 192.* interface. So on the client side, the response to the packet sent to 172.*:<server port> now comes in with address 192.*:<server port>. The client doesn't accept that, so it ignores the packet.

Essentially, we don't really track the whole 4-tuple. The (192.*, client port, 172.*, server port) 4-tuple should work, but doesn't, because we don't deal with the dst_ip we get, and thus don't set the src_ip.

matheus23 avatar Dec 11 '25 11:12 matheus23

This issue was fixed in #3770

matheus23 avatar Dec 12 '25 20:12 matheus23