consomme: avoid injecting guest packets with a localhost source address#3407
consomme: avoid injecting guest packets with a localhost source address#3407Brian-Perkins wants to merge 1 commit intomicrosoft:mainfrom
Conversation
…o route back to the virtual network device.
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent Consomme from injecting packets into the guest with a loopback (localhost) source address by rewriting such sources to the gateway address, so guest replies route back through the virtual NIC instead of the guest’s loopback interface.
Changes:
- UDP: Rewrite injected packet source IP from loopback to
gateway_ip/gateway_link_local_ipv6in both connection receive and listener forwarding paths. - TCP: Rewrite accepted peer
SocketAddrIP from loopback to the gateway address before constructing the connection tuple.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| vm/devices/net/net_consomme/consomme/src/udp.rs | Rewrites loopback source IPs to gateway for injected UDP packets (both connection RX and listener forwarding). |
| vm/devices/net/net_consomme/consomme/src/tcp.rs | Rewrites loopback peer IP to gateway for accepted inbound TCP connections. |
| // Replace the loopback IP with the gateway IP so | ||
| // the guest's reply routes back through the virtual | ||
| // adapter instead of its own loopback interface. | ||
| match &mut other_addr { | ||
| SocketAddr::V4(v4) => { | ||
| v4.set_ip(self.inner.state.params.gateway_ip); | ||
| } | ||
| SocketAddr::V6(v6) => { | ||
| v6.set_ip(self.inner.state.params.gateway_link_local_ipv6); | ||
| } | ||
| } |
There was a problem hiding this comment.
There are existing TCP listener forwarding tests, but none appear to assert the new loopback-to-gateway source-IP rewrite. Consider updating test_tcp_bind_port_forward (or adding a dedicated test) to parse the forwarded SYN and confirm its source IP is the gateway when the host connector is loopback.
| // Replace loopback source IPs with the gateway IP so | ||
| // the guest's reply routes back through the virtual | ||
| // adapter instead of its own loopback interface. | ||
| let (packet_len, checksum_state) = match (dst_addr, src_addr.ip()) { | ||
| (SocketAddr::V4(dst), IpAddr::V4(src_ip)) => { | ||
| let effective_src_ip: IpAddress = if src_ip.is_loopback() { | ||
| state.params.gateway_ip.into() | ||
| } else { | ||
| src_ip.into() | ||
| }; | ||
| let len = build_udp_packet( | ||
| &mut eth, | ||
| src_ip.into(), | ||
| effective_src_ip, |
There was a problem hiding this comment.
Replacing a loopback source IP with gateway_ip changes the remote address the guest sees. For UDP, replies will typically be sent to the packet’s source IP; that means the guest will reply to gateway_ip, and handle_udp() will then send the response to gateway_ip:<port> (no special-case remap exists beyond DHCP/DNS). This likely breaks bidirectional UDP flows involving host loopback senders. Consider adding a reverse mapping on egress (e.g., when dst is gateway_ip/gateway_link_local_ipv6 and port matches a forwarded flow) or preserving the original loopback IP while fixing routing another way.
| @@ -269,9 +290,14 @@ impl UdpListener { | |||
| Some((len, ChecksumState::UDP4)) | |||
| } | |||
| (IpAddr::V6(dst_ip), IpAddr::V6(src_ip)) => { | |||
| let effective_src_ip: IpAddress = if src_ip.is_loopback() { | |||
| state.params.gateway_link_local_ipv6.into() | |||
| } else { | |||
| src_ip.into() | |||
| }; | |||
| let len = build_udp_packet( | |||
| &mut eth, | |||
| src_ip.into(), | |||
| effective_src_ip, | |||
| dst_ip.into(), | |||
| src_addr.port(), | |||
| self.guest_port, | |||
There was a problem hiding this comment.
Same concern as above for inbound UDP port-forwarding: rewriting a loopback src_ip to the gateway makes the guest’s response target the gateway address, but the egress path will send to that gateway address rather than the original host loopback sender (no tracking/remap is performed). If the intent is to treat gateway as an alias for host loopback, the egress path needs an explicit rewrite/mapping for these flows.
| // Replace loopback source IPs with the gateway IP so | ||
| // the guest's reply routes back through the virtual | ||
| // adapter instead of its own loopback interface. | ||
| let (packet_len, checksum_state) = match (dst_addr, src_addr.ip()) { | ||
| (SocketAddr::V4(dst), IpAddr::V4(src_ip)) => { | ||
| let effective_src_ip: IpAddress = if src_ip.is_loopback() { | ||
| state.params.gateway_ip.into() | ||
| } else { | ||
| src_ip.into() | ||
| }; | ||
| let len = build_udp_packet( | ||
| &mut eth, | ||
| src_ip.into(), | ||
| effective_src_ip, |
There was a problem hiding this comment.
This behavior change (rewriting loopback source IPs to the gateway) isn’t currently asserted by the existing UDP forwarding tests. Consider extending test_udp_bind_port_forward to validate the forwarded packet’s IP source address is rewritten as expected (and ideally add a regression test that a guest reply can be forwarded back to the host sender).
When a guest tries to respond to a localhost address, that request will not typically route back through the interface it arrived on. Replace any localhost address source with the gateway address, as that should route properly.