Skip to content

consomme: avoid injecting guest packets with a localhost source address #3407

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Brian-Perkins wants to merge 2 commits into
base: main
Choose a base branch
from
+112 −40
Open

consomme: avoid injecting guest packets with a localhost source address #3407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
90c84cf
consomme: replace localhost src IP with gateway IP to allow replies t…
Apr 28, 2026
bc927c1
Treat the client IP as loopback.
May 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 47 additions & 29 deletions vm/devices/net/net_consomme/consomme/src/arp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use super::Access;
use super::Client;
use super::DropReason;
use crate::ChecksumState;
use crate::Ipv4Address;
use crate::MIN_MTU;
use smoltcp::wire::ArpOperation;
use smoltcp::wire::ArpPacket;
Expand All @@ -13,44 +14,61 @@ use smoltcp::wire::EthernetFrame;
use smoltcp::wire::EthernetProtocol;
use smoltcp::wire::EthernetRepr;

fn is_same_subnet(addr1: Ipv4Address, addr2: Ipv4Address, subnet_mask: Ipv4Address) -> bool {
let subnet_mask = subnet_mask.to_bits();
(addr1.to_bits() & subnet_mask) == (addr2.to_bits() & subnet_mask)
}

impl<T: Client> Access<'_, T> {
pub(crate) fn handle_arp(
&mut self,
frame: &EthernetRepr,
payload: &[u8],
) -> Result<(), DropReason> {
let arp = ArpRepr::parse(&ArpPacket::new_unchecked(payload))?;
match arp {
ArpRepr::EthernetIpv4 {
operation: ArpOperation::Request,
source_hardware_addr,
source_protocol_addr,
target_hardware_addr: _,
target_protocol_addr,
} if target_protocol_addr == self.inner.state.params.gateway_ip => {
let e_repr = EthernetRepr {
src_addr: self.inner.state.params.gateway_mac,
dst_addr: frame.src_addr,
ethertype: EthernetProtocol::Arp,
};
let arp_repr = ArpRepr::EthernetIpv4 {
operation: ArpOperation::Reply,
source_hardware_addr: self.inner.state.params.gateway_mac,
source_protocol_addr: target_protocol_addr,
target_hardware_addr: source_hardware_addr,
target_protocol_addr: source_protocol_addr,
};
let mut buffer = [0; MIN_MTU];
let mut response = EthernetFrame::new_unchecked(&mut buffer);
e_repr.emit(&mut response);
let mut arp_response = ArpPacket::new_unchecked(response.payload_mut());
arp_repr.emit(&mut arp_response);
let len = e_repr.buffer_len() + arp_repr.buffer_len();
self.client.recv(&buffer[..len], &ChecksumState::NONE);
}
let ArpRepr::EthernetIpv4 {
operation: ArpOperation::Request,
source_hardware_addr,
source_protocol_addr,
target_protocol_addr,
..
} = arp
else {
return Err(DropReason::UnsupportedArp);
};

_ => return Err(DropReason::UnsupportedArp),
// For any addresses in the subnet given to the guest, provide the gateway MAC address.
// This is the standard mechanism to indicate all traffic flows through the gateway, even
// local subnet traffic.
if !is_same_subnet(
self.inner.state.params.gateway_ip,
target_protocol_addr,
self.inner.state.params.net_mask,
) {
tracing::debug!(?target_protocol_addr, gateway = %self.inner.state.params.gateway_ip, subnet_mask = %self.inner.state.params.net_mask, "Ignoring ARP request");
return Ok(());
}

let mac_address = self.inner.state.params.gateway_mac;
let e_repr = EthernetRepr {
src_addr: mac_address,
dst_addr: frame.src_addr,
ethertype: EthernetProtocol::Arp,
};
let arp_repr = ArpRepr::EthernetIpv4 {
operation: ArpOperation::Reply,
source_hardware_addr: mac_address,
source_protocol_addr: target_protocol_addr,
target_hardware_addr: source_hardware_addr,
target_protocol_addr: source_protocol_addr,
};
let mut buffer = [0; MIN_MTU];
let mut response = EthernetFrame::new_unchecked(&mut buffer);
e_repr.emit(&mut response);
let mut arp_response = ArpPacket::new_unchecked(response.payload_mut());
arp_repr.emit(&mut arp_response);
let len = e_repr.buffer_len() + arp_repr.buffer_len();
self.client.recv(&buffer[..len], &ChecksumState::NONE);
Ok(())
}
}
25 changes: 19 additions & 6 deletions vm/devices/net/net_consomme/consomme/src/ndp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ fn rdnss_option_size(num_servers: usize) -> usize {
}
}

fn is_same_subnet(addr1: Ipv6Address, addr2: Ipv6Address, prefix_len: u8) -> bool {
if prefix_len == 0 {
return true;
}
let mask = u128::MAX << (128 - prefix_len);
(addr1.to_bits() & mask) == (addr2.to_bits() & mask)
}

#[derive(Debug)]
pub enum NdpMessageType {
RouterSolicit,
Expand Down Expand Up @@ -317,14 +325,19 @@ impl<T: Client> Access<'_, T> {
}
}

// Only respond if the target is our link-local address
// In a stateless NAT implementation, the gateway only responds for its own
// link-local address, not for global addresses that clients autoconfigure
if target_addr != self.inner.state.params.gateway_link_local_ipv6 {
// For any addresses in the subnet given to the guest, provide the gateway MAC address.
// This is the standard mechanism to indicate all traffic flows through the gateway, even
// local subnet traffic.
if !is_same_subnet(
self.inner.state.params.gateway_link_local_ipv6,
target_addr,
self.inner.state.params.prefix_len_ipv6,
) {
tracing::debug!(
target_addr = %target_addr,
our_link_local = %self.inner.state.params.gateway_link_local_ipv6,
"NS target is not our link-local address, ignoring"
gateway = %self.inner.state.params.gateway_link_local_ipv6,
prefix_len = %self.inner.state.params.prefix_len_ipv6,
"NS target is not local, ignoring"
);
return Ok(());
}
Expand Down
17 changes: 16 additions & 1 deletion vm/devices/net/net_consomme/consomme/src/tcp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,8 @@ impl<T: Client> Access<'_, T> {
if let Some((socket, mut other_addr)) = result {
// Check for loopback requests and replace the dest port.
// This supports a guest owning both the sending and receiving ports.
if other_addr.ip().is_loopback() {
let is_loopback = other_addr.ip().is_loopback();
if is_loopback {
for (other_ft, connection) in self.inner.tcp.connections.iter() {
if connection.inner.state == TcpState::Connecting && other_ft.dst.port() == *port {
if let LoopbackPortInfo::ProxyForGuestPort{sending_port, guest_port} = connection.inner.loopback_port {
Expand All @@ -258,6 +259,20 @@ impl<T: Client> Access<'_, T> {
}
}

// If the source IP is loopback or matches the client IP address, replace
// it with the gateway IP so that the guest's reply routes back through the
// virtual adapter instead of its own loopback interface.
let other_ip = other_addr.ip();
match &mut other_addr {
SocketAddr::V4(v4) if is_loopback || other_ip == self.inner.state.params.client_ip => {
v4.set_ip(self.inner.state.params.gateway_ip);
}
SocketAddr::V6(v6) if is_loopback || other_ip == self.inner.state.params.client_ip_ipv6.unwrap_or(::std::net::Ipv6Addr::UNSPECIFIED) => {
v6.set_ip(self.inner.state.params.gateway_link_local_ipv6);
}
_ => {}
}

let ft = match other_addr {
SocketAddr::V4(_) => FourTuple {
dst: other_addr,
Expand Down
34 changes: 30 additions & 4 deletions vm/devices/net/net_consomme/consomme/src/udp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,19 @@ impl UdpConnection {
},
) {
Poll::Ready(Ok((n, src_addr))) => {
// 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,
Comment on lines +165 to +177
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines +165 to +177
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copilot uses AI. Check for mistakes.
(*dst.ip()).into(),
src_addr.port(),
dst.port(),
Expand All @@ -177,9 +185,14 @@ impl UdpConnection {
(len, ChecksumState::UDP4)
}
(SocketAddr::V6(dst), 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(),
dst.port(),
Expand Down Expand Up @@ -254,11 +267,19 @@ impl UdpListener {
.recv_from(&mut eth.payload_mut()[header_offset..])
}) {
Poll::Ready(Ok((n, src_addr))) => {
// 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 Some((packet_len, checksum_state)) = (match (guest_dst_ip, src_addr.ip()) {
(IpAddr::V4(dst_ip), 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,
dst_ip.into(),
src_addr.port(),
self.guest_port,
Expand All @@ -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,
Comment on lines 270 to 303
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Expand Down
Loading