Reject foreign DHCP replies before policy fallback (#196)

Bumps the all-updates group with 2 updates: [clap](https://github.com/clap-rs/clap) and [system-configuration](https://github.com/mullvad/system-configuration-rs).


Updates `clap` from 4.6.5 to 4.6.6
- [Release notes](https://github.com/clap-rs/clap/releases)
- [Changelog](https://github.com/clap-rs/clap/blob/master/CHANGELOG.md)
- [Commits](https://github.com/clap-rs/clap/compare/clap_complete-v4.6.5...clap_complete-v4.6.6)

Updates `system-configuration` from 0.7.0 to 0.8.0
- [Changelog](https://github.com/mullvad/system-configuration-rs/blob/main/CHANGELOG.md)
- [Commits](https://github.com/mullvad/system-configuration-rs/compare/v0.7.0...v0.8.0)

---
updated-dependencies:
- dependency-name: clap
  dependency-version: 4.6.6
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: all-updates
- dependency-name: system-configuration
  dependency-version: 0.8.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: all-updates
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

.goreleaser.yml

@@ -5,6 +5,8 @@ project_name: softnet
 builds:
   - builder: rust
     command: build
+    env:
+      - SOFTNET_VERSION={{ .Version }}-{{ .ShortCommit }}
     targets:
       - aarch64-apple-darwin
       - x86_64-apple-darwin

Cargo.lock

@@ -368,9 +368,9 @@ dependencies = [
 
 [[package]]
 name = "clap"
-version = "4.6.5"
+version = "4.6.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "301b56658598e48f3648647ac6fc887be7e7108eddfa4e9b63fcf3ec58c0cadf"
+checksum = "473c7e07f409a8d772161724aa8db6a765a2532a70f9667eeb7b49d3d02fbdca"
 dependencies = [
  "clap_builder",
  "clap_derive",
@@ -378,9 +378,9 @@ dependencies = [
 
 [[package]]
 name = "clap_builder"
-version = "4.6.5"
+version = "4.6.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "94a65403d1a1bd28f7dc68eb8506e8874808ee5eecb59298de588e2e1407a078"
+checksum = "7b48fea5a88e9ae728a2dcbedbfc0e730f7d60da42e1cb049a83c9fb8b789889"
 dependencies = [
  "anstream",
  "anstyle",
@@ -458,6 +458,16 @@ dependencies = [
  "libc",
 ]
 
+[[package]]
+name = "core-foundation"
+version = "0.10.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b2a6cd9ae233e7f62ba4e9353e81a88df7fc8a5987b8d445b4d90c879bd156f6"
+dependencies = [
+ "core-foundation-sys",
+ "libc",
+]
+
 [[package]]
 name = "core-foundation-sys"
 version = "0.8.7"
@@ -2152,7 +2162,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "897b2245f0b511c87893af39b033e5ca9cce68824c4d7e7630b5a1d339658d02"
 dependencies = [
  "bitflags 2.9.4",
- "core-foundation",
+ "core-foundation 0.9.4",
  "core-foundation-sys",
  "libc",
  "security-framework-sys",
@@ -2596,12 +2606,12 @@ dependencies = [
 
 [[package]]
 name = "system-configuration"
-version = "0.7.0"
+version = "0.8.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a13f3d0daba03132c0aa9767f98351b3488edc2c100cda2d2ec2b04f3d8d3c8b"
+checksum = "501336eb7ba9e417300a6a0fa985721065467aa83a6dcf0422a8e43e4c0328fa"
 dependencies = [
  "bitflags 2.9.4",
- "core-foundation",
+ "core-foundation 0.10.1",
  "system-configuration-sys",
 ]
 

lib/proxy/host.rs

@@ -1,10 +1,20 @@
+use crate::dhcp_snooper::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, Result};
+use dhcproto::Decodable;
+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() {
@@ -52,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(());
         }
 
@@ -116,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()) {
@@ -134,15 +146,86 @@ 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;
         }
 
-        UdpPacket::new_checked(ipv4_pkt.payload())
-            .map(|udp_pkt| udp_pkt.is_dhcp_response())
-            .unwrap_or(false)
+        let Ok(udp_pkt) = UdpPacket::new_checked(ipv4_pkt.payload()) else {
+            return DhcpResponseDisposition::NotDhcp;
+        };
+
+        // Require the standard DHCP server and client ports
+        if !udp_pkt.is_dhcp_response() {
+            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 DhcpResponseDisposition::Reject;
+        };
+
+        if message_matches_bootp_client(&message, Opcode::BootReply, self.vm_mac_address.0) {
+            DhcpResponseDisposition::Allow
+        } else {
+            DhcpResponseDisposition::Reject
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::dhcp_snooper::message_matches_bootp_client;
+    use dhcproto::Decodable;
+    use dhcproto::v4::{DhcpOption, Message, MessageType, Opcode};
+    use dhcproto::{Encodable, Encoder};
+    use smoltcp::wire::Ipv4Address;
+
+    const VM_MAC: [u8; 6] = [0x02, 0x00, 0x00, 0x00, 0x00, 0x01];
+    const OTHER_MAC: [u8; 6] = [0x02, 0x00, 0x00, 0x00, 0x00, 0x02];
+
+    #[test]
+    fn dhcp_boot_reply_chaddr_must_match_vm() {
+        let own = encode_boot_reply(VM_MAC);
+        let foreign = encode_boot_reply(OTHER_MAC);
+
+        let mut dec = dhcproto::v4::Decoder::new(&own);
+        let own_msg = Message::decode(&mut dec).unwrap();
+        let mut dec = dhcproto::v4::Decoder::new(&foreign);
+        let foreign_msg = Message::decode(&mut dec).unwrap();
+
+        assert!(message_matches_bootp_client(
+            &own_msg,
+            Opcode::BootReply,
+            VM_MAC
+        ));
+        assert!(!message_matches_bootp_client(
+            &foreign_msg,
+            Opcode::BootReply,
+            VM_MAC
+        ));
+    }
+
+    fn encode_boot_reply(chaddr: [u8; 6]) -> Vec<u8> {
+        let mut message = Message::new(
+            Ipv4Address::UNSPECIFIED,
+            Ipv4Address::new(192, 168, 64, 2),
+            Ipv4Address::UNSPECIFIED,
+            Ipv4Address::UNSPECIFIED,
+            &chaddr,
+        );
+        message.set_opcode(Opcode::BootReply);
+        message
+            .opts_mut()
+            .insert(DhcpOption::MessageType(MessageType::Ack));
+        message.opts_mut().insert(DhcpOption::AddressLeaseTime(600));
+
+        let mut encoded = Vec::new();
+        message.encode(&mut Encoder::new(&mut encoded)).unwrap();
+        encoded
     }
 }

lib/proxy/mod.rs

@@ -108,6 +108,11 @@ impl Proxy<'_> {
         loop {
             let (vm_readable, host_readable, interrupt) = self.poller.wait()?;
 
+            // kqueue does not report peer disconnects for Unix datagram sockets.
+            if !self.vm.is_connected()? {
+                return Ok(());
+            }
+
             // Update coarse time for DHCP snooping and flows
             coarsetime::Instant::update();
 

lib/vm.rs

@@ -26,6 +26,14 @@ impl VM {
     pub fn read(&self, buf: &mut [u8]) -> std::io::Result<usize> {
         self.sock.recv(buf)
     }
+
+    pub fn is_connected(&self) -> io::Result<bool> {
+        match self.sock.peer_addr() {
+            Ok(_) => Ok(true),
+            Err(error) if error.kind() == io::ErrorKind::NotConnected => Ok(false),
+            Err(error) => Err(error),
+        }
+    }
 }
 
 fn duplicate_vm_fd(vm_fd: RawFd) -> Result<RawFd> {
@@ -115,10 +123,12 @@ impl AsRawFd for VM {
 #[cfg(test)]
 mod tests {
     use super::VM;
+    use polling::{Event, Events, PollMode, Poller};
     use std::fs::File;
     use std::net::UdpSocket;
     use std::os::fd::AsRawFd;
     use std::os::unix::net::{UnixDatagram, UnixStream};
+    use std::time::Duration;
 
     #[test]
     fn test_new_rejects_negative_fd() {
@@ -187,4 +197,62 @@ mod tests {
 
         assert!(socket_fd_is_open);
     }
+
+    #[test]
+    fn test_connected_socket_has_peer() {
+        let (socket, _peer) = UnixDatagram::pair().unwrap();
+        let vm = VM::new(socket.as_raw_fd()).unwrap();
+
+        assert!(vm.is_connected().unwrap());
+    }
+
+    #[test]
+    fn test_disconnected_peer_is_detected_without_kqueue_event() {
+        let (socket, peer) = UnixDatagram::pair().unwrap();
+        let vm = VM::new(socket.as_raw_fd()).unwrap();
+        let poller = Poller::new().unwrap();
+        let mut events = Events::new();
+
+        unsafe {
+            poller
+                .add_with_mode(vm.as_raw_fd(), Event::readable(0), PollMode::Edge)
+                .unwrap();
+        }
+        drop(peer);
+
+        poller
+            .wait(&mut events, Some(Duration::from_millis(20)))
+            .unwrap();
+
+        assert!(events.is_empty());
+        assert!(!vm.is_connected().unwrap());
+    }
+
+    #[test]
+    fn test_disconnected_peer_is_detected_when_another_socket_wakes_kqueue() {
+        let (socket, peer) = UnixDatagram::pair().unwrap();
+        let (host, host_peer) = UnixDatagram::pair().unwrap();
+        let vm = VM::new(socket.as_raw_fd()).unwrap();
+        let poller = Poller::new().unwrap();
+        let mut events = Events::new();
+
+        unsafe {
+            poller
+                .add_with_mode(vm.as_raw_fd(), Event::readable(0), PollMode::Edge)
+                .unwrap();
+            poller
+                .add_with_mode(host.as_raw_fd(), Event::readable(1), PollMode::Edge)
+                .unwrap();
+        }
+        drop(peer);
+        host_peer.send(&[1]).unwrap();
+
+        poller
+            .wait(&mut events, Some(Duration::from_millis(20)))
+            .unwrap();
+
+        assert!(events.iter().any(|event| event.key == 1));
+        assert!(!events.iter().any(|event| event.key == 0));
+        assert!(!vm.is_connected().unwrap());
+    }
 }

src/main.rs

@@ -25,7 +25,13 @@ use system_configuration::sys::preferences::{
 };
 use uzers::{get_current_groupname, get_current_username, get_effective_uid};
 
+const VERSION: &str = match option_env!("SOFTNET_VERSION") {
+    Some(version) => version,
+    None => "unknown-unknown",
+};
+
 #[derive(Parser, Debug)]
+#[command(version = VERSION)]
 struct Args {
     #[clap(
         long,