libnetwork
libnetwork copied to clipboard
moby
ensure docker-proxy sends UDP responses from correct source IP
When docker-proxy listens for UDP connections on 0.0.0.0 or :: it is not garuanteed to respond to a packet from the same IP the packet was sent to. This affects machines running docker-proxy that are multi-homed (multiple default gateways), or have multiple IPs assigned to the same interface. The impact is that packets may not be routed correctly back to the client due to the change of source-ip.
Currently the only workaround is to explicitly bind docker-proxy to each local IP address (e.g. -p 1.1.1.1:1234/1234, -p 1.1.1.2:1234/1234), which is not possible in situations where IPs change (for instance, VPN tunnels coming up and down).
This PR uses the IP_PKTINFO socket option to retrieve the destination IP of the packet received by the host side, and ensures responses for the given connection are sent from the same IP. This should close https://github.com/moby/libnetwork/issues/1729
I've added a basic unit test which simulate a multi-homed host, specifically:
- EchoServer binds to
127.0.0.1 - Proxy binds to
0.0.0.0on the host side, so it will accept connections to any IP assigned to the host - A test connection to
127.0.0.2(simulating a connection to a secondary IP address on a host) is initiated
Prior to this patch, docker-proxy would accept the packet on the 127.0.0.2 address, but respond from the 127.0.0.1 address. After the patch docker-proxy responds from the same IP the initial packet was sent to.
This is my first contribution to libnetwork/moby/docker, and I'm not a Go developer, so I appreciate any feedback or changes that may be required, and am happy to discuss/further advocate for this change.
Anything additional changes/justification needed my end to progress this PR?
@cpuguy83 I've removed the marshal and allocation from the reply loop (there's copy remaining but hopefully this will be lower impact performance wise) and tests are passing again. Want to take another look when you get a chance?
@cpuguy83 any time to review this? the underlying issue hit us hard in prod today 😢
@arkodg @euanh @cpuguy83 @theJeztah any unresolved issues in the PR as stands?
Also I think we only need to incur this overhead when the IP is 0.0.0.0 (or ipv6 equiv)?
Note we have migrated this codebase over to github.com/moby/moby/libnetwork. We are not accepting PR's on this repo anymore except for backports to be included in moby 20.10