Skip to content

Validate DHCP client identity and source address #191

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
fkorotkov-oai merged 3 commits into from
Aug 11, 2026
Merged

Validate DHCP client identity and source address #191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8aeb89b
Validate DHCP client identity and source address
edi-oai Aug 10, 2026
30787c8
Use CFBoolean instead of CFNumber for dhcp_ignore_client_identifier
edi-oai Aug 11, 2026
6595a6c
Serialize bootpd preference updates
edi-oai Aug 11, 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
19 changes: 13 additions & 6 deletions lib/dhcp_snooper.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use dhcproto::Decodable;
use dhcproto::v4::{DhcpOption, HType, MessageType, Opcode, OptionCode};
use dhcproto::v4::{DhcpOption, HType, Message, MessageType, Opcode, OptionCode};
use smoltcp::wire::Ipv4Address;
use std::collections::HashSet;
use std::time::Duration;
Expand Down Expand Up @@ -32,11 +32,7 @@ impl DhcpSnooper {
// hardware address to avoid acting on another VM's lease transition
//
// [1]: https://datatracker.ietf.org/doc/html/rfc2131#section-4.1
if message.opcode() != Opcode::BootReply
|| message.htype() != HType::Eth
|| message.hlen() != self.vm_mac_address.len() as u8
|| message.chaddr() != self.vm_mac_address
{
if !message_matches_bootp_client(&message, Opcode::BootReply, self.vm_mac_address) {
return;
}

Expand Down Expand Up @@ -120,6 +116,17 @@ impl Lease {
}
}

pub(crate) fn message_matches_bootp_client(
message: &Message,
opcode: Opcode,
mac: [u8; 6],
) -> bool {
message.opcode() == opcode
&& message.htype() == HType::Eth
&& message.hlen() == mac.len() as u8
&& message.chaddr() == mac
}

#[cfg(test)]
mod tests {
use super::{DhcpSnooper, Lease};
Expand Down
125 changes: 106 additions & 19 deletions lib/proxy/vm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use crate::dhcp_snooper::Lease;
use crate::dhcp_snooper::{Lease, message_matches_bootp_client};
use crate::proxy::flows::{FlowDirection, FlowMatch};
use crate::proxy::udp_packet_helper::UdpPacketHelper;
use crate::proxy::{Direction, PolicyDecision, Proxy};
use anyhow::Context;
use anyhow::Result;
use dhcproto::Decodable;
use dhcproto::v4::Opcode;
use smoltcp::phy::ChecksumCapabilities;
use smoltcp::wire::{
ArpOperation, ArpPacket, ArpRepr, EthernetFrame, EthernetProtocol, IpProtocol, Ipv4Address,
Expand Down Expand Up @@ -61,7 +63,12 @@ impl Proxy<'_> {
{
// Unicast DHCP renewal is required to maintain the VM's lease
// and must bypass user-specified rules
if is_allowed_dhcp_request(&ipv4_pkt, Some(self.host.gateway_ip)) {
if is_allowed_dhcp_request(
&ipv4_pkt,
Some(self.host.gateway_ip),
self.vm_mac_address,
self.dhcp_snooper.lease(),
) {
return Some(());
}

Expand Down Expand Up @@ -123,7 +130,12 @@ impl Proxy<'_> {

// Allow outgoing DHCP requests to the bootpd(8) broadcast address,
// otherwise DHCP snooper will never be populated
if is_allowed_dhcp_request(&ipv4_pkt, None) {
if is_allowed_dhcp_request(
&ipv4_pkt,
None,
self.vm_mac_address,
self.dhcp_snooper.lease(),
) {
return Some(());
}

Expand All @@ -134,7 +146,20 @@ impl Proxy<'_> {
fn is_allowed_dhcp_request(
ipv4_pkt: &Ipv4Packet<&[u8]>,
unicast_target: Option<Ipv4Address>,
vm_mac_address: smoltcp::wire::EthernetAddress,
lease: &Option<Lease>,
) -> bool {
// Require the source address to be either:
// * covered by the VM's current lease
// * unspecified on the broadcast DHCP path
let src_addr = ipv4_pkt.src_addr();
let src_has_valid_lease = lease
.as_ref()
.is_some_and(|lease| lease.is_valid_for(src_addr));
if !src_has_valid_lease && !(unicast_target.is_none() && src_addr.is_unspecified()) {
return false;
}

let dst_addr = ipv4_pkt.dst_addr();

// Keep the common path cheap and inspect UDP only for a permitted DHCP target
Expand All @@ -150,7 +175,18 @@ fn is_allowed_dhcp_request(
return false;
};

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

// Require the BOOTP client hardware address to match this VM
let mut decoder = dhcproto::v4::Decoder::new(udp_pkt.payload());
let Ok(message) = dhcproto::v4::Message::decode(&mut decoder) else {
return false;
};

message_matches_bootp_client(&message, Opcode::BootRequest, vm_mac_address.0)
}

fn vm_arp_allowed(
Expand Down Expand Up @@ -191,22 +227,40 @@ fn vm_arp_allowed(
#[cfg(test)]
mod tests {
use crate::dhcp_snooper::Lease;
use dhcproto::v4::{DhcpOption, Message, MessageType};
use dhcproto::{Encodable, Encoder};
use smoltcp::wire::{
ArpHardware, ArpOperation, ArpPacket, EthernetAddress, EthernetProtocol, IpProtocol,
Ipv4Address, Ipv4Packet, UdpPacket,
};
use std::collections::HashSet;
use std::time::Duration;

const VM_MAC: EthernetAddress = EthernetAddress([0x02, 0x00, 0x00, 0x00, 0x00, 0x01]);

#[test]
fn test_allowed_dhcp_request_targets() {
fn test_allowed_dhcp_request_policy() {
let gateway = Ipv4Address::new(192, 168, 64, 1);
let other = Ipv4Address::new(192, 168, 64, 2);

assert!(allowed_dhcp_request(Ipv4Address::BROADCAST, None));
assert!(allowed_dhcp_request(gateway, Some(gateway)));
assert!(!allowed_dhcp_request(gateway, None));
assert!(!allowed_dhcp_request(other, Some(gateway)));
let lease_ip = Ipv4Address::new(192, 168, 64, 2);
let other = Ipv4Address::new(192, 168, 64, 3);
let no_lease = None;
let lease = Some(Lease::new(
lease_ip,
Duration::from_secs(600),
HashSet::new(),
));
let initial = |src, chaddr| {
allowed_dhcp_request(src, Ipv4Address::BROADCAST, None, chaddr, &no_lease)
};
let renewal = |src, dst| allowed_dhcp_request(src, dst, Some(gateway), VM_MAC.0, &lease);
let other_mac = [0x02, 0x00, 0x00, 0x00, 0x00, 0x02];

assert!(initial(Ipv4Address::UNSPECIFIED, VM_MAC.0));
assert!(renewal(lease_ip, gateway));
assert!(!renewal(other, gateway));
assert!(!renewal(Ipv4Address::UNSPECIFIED, gateway));
assert!(!renewal(lease_ip, other));
assert!(!initial(Ipv4Address::UNSPECIFIED, other_mac));
}

#[test]
Expand Down Expand Up @@ -308,21 +362,54 @@ mod tests {
buf
}

fn allowed_dhcp_request(dst_addr: Ipv4Address, unicast_target: Option<Ipv4Address>) -> bool {
let mut buf = vec![0; 28];
fn allowed_dhcp_request(
src_addr: Ipv4Address,
dst_addr: Ipv4Address,
unicast_target: Option<Ipv4Address>,
chaddr: [u8; 6],
lease: &Option<Lease>,
) -> bool {
let mut buf = dhcp_request(chaddr);
let mut ipv4_pkt = Ipv4Packet::new_unchecked(buf.as_mut_slice());
ipv4_pkt.set_src_addr(src_addr);
ipv4_pkt.set_dst_addr(dst_addr);

let ipv4_pkt = Ipv4Packet::new_checked(buf.as_slice()).unwrap();
super::is_allowed_dhcp_request(&ipv4_pkt, unicast_target, VM_MAC, lease)
}

fn dhcp_request(chaddr: [u8; 6]) -> Vec<u8> {
let mut message = Message::new(
Ipv4Address::UNSPECIFIED,
Ipv4Address::UNSPECIFIED,
Ipv4Address::UNSPECIFIED,
Ipv4Address::UNSPECIFIED,
&chaddr,
);
message
.opts_mut()
.insert(DhcpOption::MessageType(MessageType::Discover));

let mut dhcp_payload = Vec::new();
message
.encode(&mut Encoder::new(&mut dhcp_payload))
.unwrap();

let total_len = 20 + 8 + dhcp_payload.len();
let mut buf = vec![0; total_len];
let mut ipv4_pkt = Ipv4Packet::new_unchecked(buf.as_mut_slice());
ipv4_pkt.set_version(4);
ipv4_pkt.set_header_len(20);
ipv4_pkt.set_total_len(28);
ipv4_pkt.set_total_len(total_len as u16);
ipv4_pkt.set_next_header(IpProtocol::Udp);
ipv4_pkt.set_dst_addr(dst_addr);
ipv4_pkt.set_src_addr(Ipv4Address::UNSPECIFIED);
ipv4_pkt.set_dst_addr(Ipv4Address::BROADCAST);

let mut udp_pkt = UdpPacket::new_unchecked(ipv4_pkt.payload_mut());
udp_pkt.set_src_port(68);
udp_pkt.set_dst_port(67);
udp_pkt.set_len(8);

let ipv4_pkt = Ipv4Packet::new_checked(buf.as_slice()).unwrap();
super::is_allowed_dhcp_request(&ipv4_pkt, unicast_target)
udp_pkt.set_len((8 + dhcp_payload.len()) as u16);
udp_pkt.payload_mut().copy_from_slice(&dhcp_payload);
buf
}
}
63 changes: 50 additions & 13 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,15 @@ use std::os::unix::io::RawFd;
use std::os::unix::process::CommandExt;
use std::process::{Command, ExitCode};
use system_configuration::core_foundation::base::TCFType;
use system_configuration::core_foundation::boolean::CFBoolean;
use system_configuration::core_foundation::dictionary::CFDictionary;
use system_configuration::core_foundation::number::CFNumber;
use system_configuration::core_foundation::string::CFString;
use system_configuration::preferences::SCPreferences;
use system_configuration::sys::preferences::{SCPreferencesCommitChanges, SCPreferencesSetValue};
use system_configuration::sys::preferences::{
SCPreferencesApplyChanges, SCPreferencesCommitChanges, SCPreferencesLock,
SCPreferencesSetValue, SCPreferencesUnlock,
};
use uzers::{get_current_groupname, get_current_username, get_effective_uid};

#[derive(Parser, Debug)]
Expand Down Expand Up @@ -215,8 +219,8 @@ fn try_main() -> anyhow::Result<()> {
));
}

// Set bootpd(8) min/max lease time while still having the root privileges
set_bootpd_lease_time(args.bootpd_lease_time);
// Configure bootpd(8) while still having the root privileges
configure_bootpd(args.bootpd_lease_time)?;

// Initialize the proxy while still having the root privileges
let mut proxy = Proxy::new(
Expand Down Expand Up @@ -268,26 +272,59 @@ fn sudo_escalation_works() -> bool {
.unwrap_or(false)
}

fn set_bootpd_lease_time(lease_time: u32) {
fn configure_bootpd(lease_time: u32) -> anyhow::Result<()> {
let prefs = SCPreferences::group(
&CFString::new("softnet"),
&CFString::new("com.apple.InternetSharing.default.plist"),
);

let bootpd_dict = CFDictionary::from_CFType_pairs(&[(
CFString::new("DHCPLeaseTimeSecs"),
CFNumber::from(lease_time as i32),
)]);
let bootpd_dict = CFDictionary::from_CFType_pairs(&[
(
CFString::new("DHCPLeaseTimeSecs"),
CFNumber::from(lease_time as i32).as_CFType(),
),
(
CFString::new("dhcp_ignore_client_identifier"),
CFBoolean::true_value().as_CFType(),

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 11, 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 Store ignore-client-identifier as a CFNumber

The fresh evidence in the current diff is that this setting has now been changed to CFBoolean, whereas bootpd loads dhcp_ignore_client_identifier through SET_NUMBER_FROM_PLIST, which expects a CFNumber; Core Foundation booleans have a different type ID and are therefore ignored. When a client changes option 61, bootpd will continue allocating leases by client identifier, defeating the intended protection against exhausting the DHCP pool; encode 1 as a CFNumber instead.

Useful? React with 👍 / 👎.

@edi-oai edi-oai Aug 11, 2026

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.

SET_NUMBER_FROM_PLIST does not require a CFNumber, see #191 (comment) for a proof.

),
]);

unsafe {
SCPreferencesSetValue(
prefs.as_concrete_TypeRef(),
CFString::new("bootpd").as_concrete_TypeRef(),
bootpd_dict.as_concrete_TypeRef().cast(),
let prefs = prefs.as_concrete_TypeRef();
anyhow::ensure!(
SCPreferencesLock(prefs, 1) != 0,
"failed to lock bootpd preferences"
);

SCPreferencesCommitChanges(prefs.as_concrete_TypeRef());
let result = (|| -> anyhow::Result<()> {
anyhow::ensure!(
SCPreferencesSetValue(
prefs,
CFString::new("bootpd").as_concrete_TypeRef(),
bootpd_dict.as_concrete_TypeRef().cast(),
) != 0,
"failed to set bootpd preferences"
);

anyhow::ensure!(
SCPreferencesCommitChanges(prefs) != 0,
"failed to commit bootpd preferences"
);

anyhow::ensure!(
SCPreferencesApplyChanges(prefs) != 0,
"failed to apply bootpd preferences"
);

Ok(())
})();

let unlocked = SCPreferencesUnlock(prefs) != 0;
result?;
anyhow::ensure!(unlocked, "failed to unlock bootpd preferences");
}

Ok(())
}

#[cfg(test)]
Expand Down