From 431ae9bbc908acca85ac96baf909be3aa15f8e3e Mon Sep 17 00:00:00 2001 From: Nikolay Edigaryev Date: Tue, 21 Oct 2025 15:14:28 +0200 Subject: [PATCH] Introduce --block in addition to --allow (#126) --- .cargo/config.toml | 2 + .cirrus.yml | 36 +++++++++-- Cargo.lock | 54 ++++++++++++++++ Cargo.toml | 5 +- lib/dhcp_snooper.rs | 10 ++- lib/host.rs | 8 ++- lib/poller.rs | 2 +- lib/proxy/exposed_port.rs | 2 +- lib/proxy/host.rs | 2 +- lib/proxy/mod.rs | 129 +++++++++++++++++++++++++++++++++++++- lib/proxy/vm.rs | 63 +++++++++++-------- src/main.rs | 31 +++++++-- 12 files changed, 296 insertions(+), 48 deletions(-) create mode 100644 .cargo/config.toml diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 0000000..c7f2528 --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,2 @@ +[target.aarch64-apple-darwin] +runner = 'sudo -E' diff --git a/.cirrus.yml b/.cirrus.yml index e695bd9..4433172 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -1,11 +1,38 @@ +use_compute_credits: true + +macos_instance: + image: ghcr.io/cirruslabs/macos-runner:tahoe + env: PATH: "$PATH:$HOME/.cargo/bin" +task: + name: Lint + install_rust_script: curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y + rustfmt_script: cargo fmt --check + clippy_script: cargo clippy --all-targets --all-features -- -D warnings + +task: + alias: Test + matrix: + - name: Test on macOS Sonoma + macos_instance: + image: ghcr.io/cirruslabs/macos-runner:sonoma + - name: Test on macOS Sequoia + macos_instance: + image: ghcr.io/cirruslabs/macos-runner:sequoia + - name: Test on macOS Tahoe + macos_instance: + image: ghcr.io/cirruslabs/macos-runner:tahoe + install_rust_script: curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y + test_script: cargo test + task: name: Release (Dry Run) only_if: $CIRRUS_TAG == '' - macos_instance: - image: ghcr.io/cirruslabs/macos-sequoia-xcode:latest + depends_on: + - Lint + - Test install_rust_script: curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y install_script: brew install go install_goreleaser_script: brew install --cask goreleaser/tap/goreleaser-pro @@ -16,8 +43,9 @@ task: task: name: Release only_if: $CIRRUS_TAG != '' - macos_instance: - image: ghcr.io/cirruslabs/macos-sequoia-xcode:latest + depends_on: + - Lint + - Test env: GITHUB_TOKEN: ENCRYPTED[!98ace8259c6024da912c14d5a3c5c6aac186890a8d4819fad78f3e0c41a4e0cd3a2537dd6e91493952fb056fa434be7c!] GORELEASER_KEY: ENCRYPTED[!9b80b6ef684ceaf40edd4c7af93014ee156c8aba7e6e5795f41c482729887b5c31f36b651491d790f1f668670888d9fd!] diff --git a/Cargo.lock b/Cargo.lock index cc9f86f..f266ba4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -761,6 +761,21 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "futures" +version = "0.3.30" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "645c6916888f6cb6350d2550b80fb63e734897a8498abe35cfb732b6487804b0" +dependencies = [ + "futures-channel", + "futures-core", + "futures-executor", + "futures-io", + "futures-sink", + "futures-task", + "futures-util", +] + [[package]] name = "futures-channel" version = "0.3.30" @@ -777,6 +792,17 @@ version = "0.3.30" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dfc6580bb841c5a68e9ef15c77ccc837b40a7504914d52e47b8b0e9bbda25a1d" +[[package]] +name = "futures-executor" +version = "0.3.30" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a576fc72ae164fca6b9db127eaa9a9dda0d61316034f33a0a0d4eda41f02b01d" +dependencies = [ + "futures-core", + "futures-task", + "futures-util", +] + [[package]] name = "futures-io" version = "0.3.30" @@ -801,6 +827,7 @@ version = "0.3.30" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3d6401deb83407ab3da39eba7e33987a73c3df0c82b4bb5813ee871c19c41d48" dependencies = [ + "futures-channel", "futures-core", "futures-io", "futures-sink", @@ -1420,6 +1447,7 @@ dependencies = [ "cfg-if", "cfg_aliases", "libc", + "memoffset", ] [[package]] @@ -2148,6 +2176,31 @@ dependencies = [ "serde", ] +[[package]] +name = "serial_test" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1c789ec87f4687d022a2405cf46e0cd6284889f1839de292cadeb6c6019506f2" +dependencies = [ + "dashmap", + "futures", + "lazy_static", + "log", + "parking_lot", + "serial_test_derive", +] + +[[package]] +name = "serial_test_derive" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b64f9e531ce97c88b4778aad0ceee079216071cffec6ac9b904277f8f92e7fe3" +dependencies = [ + "proc-macro2", + "quote", + "syn 1.0.109", +] + [[package]] name = "sha1" version = "0.10.6" @@ -2235,6 +2288,7 @@ dependencies = [ "privdrop", "sentry", "sentry-anyhow", + "serial_test", "smoltcp", "system-configuration", "uzers", diff --git a/Cargo.toml b/Cargo.toml index 3b3b222..bc36436 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ name = "softnet" version = "0.1.0" publish = false -edition = "2021" +edition = "2024" [lib] path = "lib/mod.rs" @@ -27,11 +27,12 @@ system-configuration = "0" num_enum = "0" sentry = { version = "0", features = ["debug-images"] } sentry-anyhow = { version = "0", features = ["backtrace"] } -nix = { version = "0", features = ["signal"] } +nix = { version = "0", features = ["signal", "socket"] } prefix-trie = "0" ipnet = "2" oslog = "0.2.0" log = "0.4.28" +serial_test = "0" [profile.release] debug = true diff --git a/lib/dhcp_snooper.rs b/lib/dhcp_snooper.rs index c6a3e8e..a47dc64 100644 --- a/lib/dhcp_snooper.rs +++ b/lib/dhcp_snooper.rs @@ -1,5 +1,5 @@ -use dhcproto::v4::{DhcpOption, MessageType, OptionCode}; use dhcproto::Decodable; +use dhcproto::v4::{DhcpOption, MessageType, OptionCode}; use smoltcp::wire::Ipv4Address; use std::collections::HashSet; use std::time::{Duration, Instant}; @@ -45,6 +45,11 @@ impl DhcpSnooper { }; } + #[cfg(test)] + pub(crate) fn set_lease(&mut self, vm_lease: Option) { + self.vm_lease = vm_lease + } + pub fn lease(&self) -> &Option { &self.vm_lease } @@ -58,6 +63,7 @@ impl DhcpSnooper { } } +#[derive(Debug)] pub struct Lease { address: Ipv4Address, valid_until: Instant, @@ -65,7 +71,7 @@ pub struct Lease { } impl Lease { - fn new(address: Ipv4Address, lease_time: Duration, dns_ips: HashSet) -> Lease { + pub fn new(address: Ipv4Address, lease_time: Duration, dns_ips: HashSet) -> Lease { Lease { address, valid_until: Instant::now() + lease_time, diff --git a/lib/host.rs b/lib/host.rs index 4115b81..c8fcb02 100644 --- a/lib/host.rs +++ b/lib/host.rs @@ -1,11 +1,11 @@ -use anyhow::{anyhow, Context, Result}; +use anyhow::{Context, Result, anyhow}; use clap::ValueEnum; use log::info; use std::net::Ipv4Addr; use std::os::unix::io::{AsRawFd, RawFd}; use std::os::unix::net::UnixDatagram; use std::str::FromStr; -use std::sync::mpsc::{sync_channel, SyncSender}; +use std::sync::mpsc::{SyncSender, sync_channel}; use vmnet::mode::Mode; use vmnet::parameters::{Parameter, ParameterKind}; use vmnet::port_forwarding::{AddressFamily, Protocol}; @@ -108,7 +108,9 @@ impl Host { internal_addr: Ipv4Addr, internal_port: u16, ) -> Result<()> { - let details = format!("external_port={external_port}, internal_addr={internal_addr}, internal_port={internal_port}"); + let details = format!( + "external_port={external_port}, internal_addr={internal_addr}, internal_port={internal_port}" + ); self.interface .port_forwarding_rule_add( diff --git a/lib/poller.rs b/lib/poller.rs index f2d61ca..24a657d 100644 --- a/lib/poller.rs +++ b/lib/poller.rs @@ -1,7 +1,7 @@ use anyhow::Result; use num_enum::IntoPrimitive; -use polling::os::kqueue::PollerKqueueExt; use polling::PollMode; +use polling::os::kqueue::PollerKqueueExt; use std::os::fd::{AsRawFd, BorrowedFd}; use std::os::unix::io::RawFd; use std::time::Duration; diff --git a/lib/proxy/exposed_port.rs b/lib/proxy/exposed_port.rs index 7a2e6a3..81cf08e 100644 --- a/lib/proxy/exposed_port.rs +++ b/lib/proxy/exposed_port.rs @@ -1,4 +1,4 @@ -use anyhow::{anyhow, Context, Error}; +use anyhow::{Context, Error, anyhow}; use std::str::FromStr; #[derive(Debug, Clone, Copy, Default, PartialEq)] diff --git a/lib/proxy/host.rs b/lib/proxy/host.rs index a02a2f0..efcc2bd 100644 --- a/lib/proxy/host.rs +++ b/lib/proxy/host.rs @@ -1,5 +1,5 @@ -use crate::proxy::udp_packet_helper::UdpPacketHelper; use crate::proxy::Proxy; +use crate::proxy::udp_packet_helper::UdpPacketHelper; use anyhow::{Context, Result}; use smoltcp::wire::{EthernetFrame, EthernetProtocol, Ipv4Packet, UdpPacket}; diff --git a/lib/proxy/mod.rs b/lib/proxy/mod.rs index 0542422..39f0347 100644 --- a/lib/proxy/mod.rs +++ b/lib/proxy/mod.rs @@ -14,7 +14,7 @@ pub use exposed_port::ExposedPort; use ipnet::Ipv4Net; use mac_address::MacAddress; use port_forwarder::PortForwarder; -use prefix_trie::{Prefix, PrefixSet}; +use prefix_trie::{Prefix, PrefixMap, PrefixSet}; use smoltcp::wire::EthernetFrame; use std::io::ErrorKind; use std::os::unix::io::{AsRawFd, RawFd}; @@ -25,30 +25,51 @@ pub struct Proxy<'proxy> { poller: Poller<'proxy>, vm_mac_address: smoltcp::wire::EthernetAddress, dhcp_snooper: DhcpSnooper, - allow: PrefixSet, + rules: PrefixMap, enobufs_encountered: bool, port_forwarder: PortForwarder, } +#[derive(Debug, Clone, PartialEq)] +pub(crate) enum Action { + Block, + Allow, +} + impl Proxy<'_> { pub fn new<'proxy>( vm_fd: RawFd, vm_mac_address: MacAddress, vm_net_type: NetType, allow: PrefixSet, + block: PrefixSet, exposed_ports: Vec, ) -> Result> { let vm = VM::new(vm_fd)?; let host = Host::new(vm_net_type, !allow.contains(&Ipv4Net::zero()))?; let poller = Poller::new(vm.as_raw_fd(), host.as_raw_fd())?; + // Craft packet filter rules + // + // SECURITY: blocking rules must always take precedence + // over allowing rules when prefixes are identical. + let mut rules = PrefixMap::new(); + + for allow_net in allow { + rules.insert(allow_net, Action::Allow); + } + + for block_net in block { + rules.insert(block_net, Action::Block); + } + Ok(Proxy { vm, host, poller, vm_mac_address: smoltcp::wire::EthernetAddress(vm_mac_address.bytes()), dhcp_snooper: Default::default(), - allow, + rules, enobufs_encountered: false, port_forwarder: PortForwarder::new(exposed_ports), }) @@ -123,3 +144,105 @@ impl Proxy<'_> { } } } + +#[cfg(test)] +mod tests { + use crate::NetType; + use crate::dhcp_snooper::Lease; + use crate::proxy::{Action, Proxy}; + use ipnet::Ipv4Net; + use mac_address::MacAddress; + use nix::sys::socket::{AddressFamily, SockFlag, SockType, socketpair}; + use prefix_trie::{PrefixMap, PrefixSet}; + use serial_test::serial; + use smoltcp::wire::{Ipv4Address, Ipv4Packet}; + use std::collections::HashSet; + use std::os::fd::AsRawFd; + use std::str::FromStr; + use std::time::Duration; + + #[test] + #[serial] + fn test_blocking_takes_precedence() { + let vm_ip = Ipv4Address::from_str("192.168.0.2").unwrap(); + let proxy = create_proxy(vm_ip, vec!["66.66.0.0/16"], vec!["66.66.0.0/16"]); + + assert_eq!( + proxy.rules, + PrefixMap::::from_iter(vec![( + Ipv4Net::from_str("66.66.0.0/16").unwrap(), + Action::Block + ),]) + ); + + assert!(allowed_from_vm_ipv4(&proxy, vm_ip, "66.66.66.66").is_none()); + } + + #[test] + #[serial] + fn test_longest_prefix_match_wins() { + let vm_ip = Ipv4Address::from_str("192.168.0.2").unwrap(); + let proxy = create_proxy(vm_ip, vec!["33.33.33.33/32"], vec!["33.33.33.0/24"]); + + assert_eq!( + proxy.rules, + PrefixMap::::from_iter(vec![ + (Ipv4Net::from_str("33.33.33.33/32").unwrap(), Action::Allow), + (Ipv4Net::from_str("33.33.33.0/24").unwrap(), Action::Block), + ]) + ); + + assert!(allowed_from_vm_ipv4(&proxy, vm_ip, "33.33.33.32").is_none()); + assert!(allowed_from_vm_ipv4(&proxy, vm_ip, "33.33.33.33").is_some()); + assert!(allowed_from_vm_ipv4(&proxy, vm_ip, "33.33.33.34").is_none()); + } + + fn create_proxy<'test>(vm_ip: Ipv4Address, allow: Vec<&str>, block: Vec<&str>) -> Proxy<'test> { + let (vm_fd, _) = socketpair( + AddressFamily::Unix, + SockType::Datagram, + None, + SockFlag::empty(), + ) + .unwrap(); + let vm_fd = Box::leak(Box::new(vm_fd)); + + let mut proxy = Proxy::new( + vm_fd.as_raw_fd(), + MacAddress::from_str("02:00:00:00:00:01").unwrap(), + NetType::Nat, + PrefixSet::from_iter( + allow + .into_iter() + .map(|cidr| Ipv4Net::from_str(cidr).unwrap()), + ), + PrefixSet::from_iter( + block + .into_iter() + .map(|cidr| Ipv4Net::from_str(cidr).unwrap()), + ), + Vec::default(), + ) + .unwrap(); + + proxy.dhcp_snooper.set_lease(Some(Lease::new( + vm_ip, + Duration::from_secs(600), + HashSet::new(), + ))); + + proxy + } + + fn allowed_from_vm_ipv4(proxy: &Proxy, src: Ipv4Address, dst: &str) -> Option<()> { + let mut buf = vec![0; 1500]; + + let mut ipv4_pkt_mut = Ipv4Packet::new_unchecked(&mut buf[..]); + ipv4_pkt_mut.set_src_addr(src); + ipv4_pkt_mut.set_dst_addr(Ipv4Address::from_str(dst).unwrap()); + + let ipv4_pkt = Ipv4Packet::new_unchecked(buf.as_slice()); + + proxy.allowed_from_vm_ipv4(ipv4_pkt) + } +} diff --git a/lib/proxy/vm.rs b/lib/proxy/vm.rs index 53c7530..d6d0e86 100644 --- a/lib/proxy/vm.rs +++ b/lib/proxy/vm.rs @@ -1,5 +1,5 @@ use crate::proxy::udp_packet_helper::UdpPacketHelper; -use crate::proxy::Proxy; +use crate::proxy::{Action, Proxy}; use anyhow::Context; use anyhow::Result; use ipnet::Ipv4Net; @@ -58,43 +58,54 @@ impl Proxy<'_> { None } - fn allowed_from_vm_ipv4(&self, ipv4_pkt: Ipv4Packet<&[u8]>) -> Option<()> { - // Have we learned the VM's IP from the DHCP snooping? - if let Some(lease) = &self.dhcp_snooper.lease() { - // If so, allow all global traffic + pub(crate) fn allowed_from_vm_ipv4(&self, ipv4_pkt: Ipv4Packet<&[u8]>) -> Option<()> { + // Is this packet coming from VM's IP address that we've learned from DHCP snooping? + if let Some(lease) = &self.dhcp_snooper.lease() + && lease.valid_ip_source(ipv4_pkt.src_addr()) + { let dst_addr = ipv4_pkt.dst_addr(); - let dst_is_global = ip_network::IpNetwork::from(dst_addr).is_global(); - if lease.valid_ip_source(ipv4_pkt.src_addr()) && dst_is_global { + // Filter traffic based on user-specified rules first + if !self.rules.is_empty() { + let dst_net = Ipv4Net::from(dst_addr); + + if let Some((_, action)) = self.rules.get_lpm(&dst_net) { + return match action { + Action::Allow => Some(()), + Action::Block => None, + }; + } + } + + // When no user-specified rules matched, simply allow all global traffic + if ip_network::IpNetwork::from(dst_addr).is_global() { return Some(()); } - // Also allow all traffic to the user-specified CIDRs - let dst_net = Ipv4Net::from(dst_addr); - - // Use get_lpm() instead of get_spm() to work around prefix-trie - // not handling prefixes like 0.0.0.0/0 correctly[1] - // - // [1]: https://github.com/tiborschneider/prefix-trie/issues/8 - if self.allow.get_lpm(&dst_net).is_some() { + // Additionally, allow communication with the host, + // otherwise things like SSH to a VM won't work + if ipv4_pkt.dst_addr() == self.host.gateway_ip { return Some(()); } + + // Additionally, allow DNS requests to DNS-servers + // provided to a VM by the host's DHCP server + if ipv4_pkt.next_header() == IpProtocol::Udp { + let udp_pkt = UdpPacket::new_checked(ipv4_pkt.payload()).ok()?; + + if udp_pkt.is_dns_request() + && self.dhcp_snooper.valid_dns_target(&ipv4_pkt.dst_addr()) + { + return Some(()); + } + } } - // Allow communication with host - if ipv4_pkt.dst_addr() == self.host.gateway_ip { - return Some(()); - } - + // Allow outgoing DHCP requests to broadcast addresses, + // otherwise DHCP snooper will never be populated if ipv4_pkt.next_header() == IpProtocol::Udp { let udp_pkt = UdpPacket::new_checked(ipv4_pkt.payload()).ok()?; - // Allow DNS communication with the DNS-servers provided by DHCP - if udp_pkt.is_dns_request() && self.dhcp_snooper.valid_dns_target(&ipv4_pkt.dst_addr()) - { - return Some(()); - } - // Allow DHCP communication with the bootpd(8) on host via broadcast address if udp_pkt.is_dhcp_request() && ipv4_pkt.dst_addr().is_broadcast() { return Some(()); diff --git a/src/main.rs b/src/main.rs index 492f793..49e84cd 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,14 +1,14 @@ -use anyhow::{anyhow, Context}; +use anyhow::{Context, anyhow}; use clap::Parser; use ipnet::Ipv4Net; use log::LevelFilter; -use nix::sys::signal::{signal, SigHandler, Signal}; +use nix::sys::signal::{SigHandler, Signal, signal}; use oslog::OsLogger; use prefix_trie::PrefixSet; use privdrop::PrivDrop; +use softnet::NetType; use softnet::proxy::ExposedPort; use softnet::proxy::Proxy; -use softnet::NetType; use std::borrow::Cow; use std::env; use std::os::raw::c_int; @@ -52,13 +52,31 @@ struct Args { #[clap( long, - help = "comma-separated list of CIDRs to allow the traffic to (e.g. --allow=192.168.0.0/24)", + help = "Comma-separated list of CIDRs to allow the traffic to \ + (e.g. --allow=192.168.0.0/24 may be used to allow a LAN access for a VM). \ + When used with --block, the longest prefix match always wins. \ + In case an identical prefix is both --allow'ed and --block'ed, \ + blocking will take precedence. --allow=0.0.0.0/0 is a special case, \ + it additionally disables bridge isolation (even when --block=0.0.0.0/0 is specified).", value_name = "comma-separated CIDRs", use_value_delimiter = true, action = clap::ArgAction::Set )] allow: Vec, + #[clap( + long, + help = "Comma-separated list of CIDRs to block the traffic to \ + (e.g. --block=0.0.0.0/0 may be used to establish a default deny policy \ + that is further relaxed with --allow). When used with --allow, \ + the longest prefix match always wins. In case the same prefix is both \ + --allow'ed and --block'ed, blocking takes precedence.", + value_name = "comma-separated CIDRs", + use_value_delimiter = true, + action = clap::ArgAction::Set + )] + block: Vec, + #[clap( long, help = "comma-separated list of TCP ports to expose (e.g. --expose 2222:22,8080:80)", @@ -78,7 +96,9 @@ struct Args { fn main() -> ExitCode { // Enable backtraces by default if env::var("RUST_BACKTRACE").is_err() { - env::set_var("RUST_BACKTRACE", "full"); + unsafe { + env::set_var("RUST_BACKTRACE", "full"); + } } // Initialize Sentry @@ -177,6 +197,7 @@ fn try_main() -> anyhow::Result<()> { args.vm_mac_address, args.vm_net_type, PrefixSet::from_iter(args.allow), + PrefixSet::from_iter(args.block), args.expose, ) .context("failed to initialize proxy")?;