Skip to content

Reject foreign DHCP replies before policy fallback #196

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

Merged
edi-oai merged 1 commit into from
Aug 17, 2026
Merged

Reject foreign DHCP replies before policy fallback #196

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
39 changes: 26 additions & 13 deletions lib/proxy/host.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ use dhcproto::v4::Opcode;
use smoltcp::phy::ChecksumCapabilities;
use smoltcp::wire::{EthernetFrame, EthernetProtocol, Ipv4Packet, Ipv4Repr, UdpPacket};

/// DhcpResponseDisposition distinguishes non-DHCP traffic from allowed and rejected DHCP replies.
enum DhcpResponseDisposition {
NotDhcp,
Allow,
Reject,
}

impl Proxy<'_> {
pub(crate) fn process_frame_from_host(&mut self, frame: &EthernetFrame<&[u8]>) -> Result<()> {
if self.allowed_from_host(frame).is_none() {
Expand Down Expand Up @@ -55,13 +62,14 @@ impl Proxy<'_> {
}

pub(super) fn allowed_from_host_ipv4(&mut self, ipv4_pkt: &Ipv4Packet<&[u8]>) -> Option<()> {
// Backwards compatibility with Softnet consumers that only use stateless rules
if self.flows.is_none() {
return Some(());
match self.dhcp_response_disposition(ipv4_pkt) {
DhcpResponseDisposition::NotDhcp => { /* Fall through to generic policy */ }
DhcpResponseDisposition::Allow => return Some(()),
DhcpResponseDisposition::Reject => return None,
}

// DHCP is required to maintain the VM's lease and must bypass user-specified rules
if self.is_allowed_dhcp_response(ipv4_pkt) {
// Backwards compatibility with Softnet consumers that only use stateless rules
if self.flows.is_none() {
return Some(());
}

Expand Down Expand Up @@ -119,8 +127,9 @@ impl Proxy<'_> {
_ => return,
};

if !self.is_allowed_dhcp_response(&ipv4_pkt) {
return;
match self.dhcp_response_disposition(&ipv4_pkt) {
DhcpResponseDisposition::Allow => { /* Continue snooping */ }
DhcpResponseDisposition::NotDhcp | DhcpResponseDisposition::Reject => return,
}

let udp_pkt = match UdpPacket::new_checked(ipv4_pkt.payload()) {
Expand All @@ -137,30 +146,34 @@ impl Proxy<'_> {
}
}

fn is_allowed_dhcp_response(&self, ipv4_pkt: &Ipv4Packet<&[u8]>) -> bool {
fn dhcp_response_disposition(&self, ipv4_pkt: &Ipv4Packet<&[u8]>) -> DhcpResponseDisposition {
if ipv4_pkt.src_addr() != self.host.gateway_ip
|| ipv4_pkt.next_header() != smoltcp::wire::IpProtocol::Udp
{
return false;
return DhcpResponseDisposition::NotDhcp;
}

let Ok(udp_pkt) = UdpPacket::new_checked(ipv4_pkt.payload()) else {
return false;
return DhcpResponseDisposition::NotDhcp;

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Reject fragmented DHCP replies instead of falling through

When a BOOTP reply is IPv4-fragmented, the first fragment's UDP length describes the complete datagram, so UdpPacket::new_checked rejects the shorter fragment; later fragments contain no UDP header. Returning NotDhcp here sends every fragment through generic policy, which unconditionally permits them when flows is None, allowing the guest to reassemble a foreign DHCP reply and bypass the new client-MAC check. Fragmented gateway UDP traffic must be rejected or reassembled before deciding that it is not DHCP.

Useful? React with 👍 / 👎.

@edi-oai edi-oai Aug 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Let's handle this separately in #197, seems out-of-scope because this PR doesn't introduce this issue.

};

// Require the standard DHCP server and client ports
if !udp_pkt.is_dhcp_response() {
return false;
return DhcpResponseDisposition::NotDhcp;
}

// Require the BOOTP client hardware address to match this VM
// (symmetric with is_allowed_dhcp_request / #191 on the VM→host path)
let mut decoder = dhcproto::v4::Decoder::new(udp_pkt.payload());
let Ok(message) = dhcproto::v4::Message::decode(&mut decoder) else {
return false;
return DhcpResponseDisposition::Reject;
};

message_matches_bootp_client(&message, Opcode::BootReply, self.vm_mac_address.0)
if message_matches_bootp_client(&message, Opcode::BootReply, self.vm_mac_address.0) {
DhcpResponseDisposition::Allow
} else {
DhcpResponseDisposition::Reject
}
}
}

Expand Down