diff --git a/lib/proxy/control.rs b/lib/proxy/control.rs index 4b6e79b..885f00e 100644 --- a/lib/proxy/control.rs +++ b/lib/proxy/control.rs @@ -29,7 +29,6 @@ const REVISION_CONFLICT: i32 = -32001; const BRIDGE_ISOLATION_CONFLICT: i32 = -32002; pub(super) struct Policy { - pub(super) rules: PrefixMap, allow: Vec, block: Vec, desired_revision: Option, @@ -50,10 +49,8 @@ impl Policy { let allow = normalize_targets(allow); let block = normalize_targets(block); let bridge_isolation = Self::bridge_isolation(&allow); - let rules = build_rules(gateway_ip, &allow, &block); Policy { - rules, allow, block, desired_revision: None, @@ -121,23 +118,23 @@ impl Policy { })) } - fn apply(&mut self, update: PolicyUpdate) { + fn apply(&mut self, update: PolicyUpdate) -> PrefixMap { // Build and validate everything before updating any active state. The packet filter // observes either the old PrefixMap or the complete new one. - self.rules = update.rules; self.allow = update.allow; self.block = update.block; self.applied_revisions .insert(update.desired_revision.clone()); self.desired_revision = Some(update.desired_revision); + update.rules } - fn result(&self) -> Value { + fn result(&self, rule_count: usize) -> Value { policy_result( &self.allow, &self.block, self.desired_revision.as_deref(), - self.rules.len(), + rule_count, self.bridge_isolation, ) } @@ -245,6 +242,7 @@ fn build_rules( } pub(super) struct Control { + policy: Policy, stream: UnixStream, input: Vec, output: Vec, @@ -254,7 +252,12 @@ pub(super) struct Control { } impl Control { - pub(super) fn new(control_fd: RawFd) -> Result { + pub(super) fn new( + control_fd: RawFd, + gateway_ip: Ipv4Address, + allow: Vec, + block: Vec, + ) -> Result { let control_fd = duplicate_control_fd(control_fd)?; // SAFETY: duplicate_control_fd returns an open Unix stream descriptor that it owns. @@ -262,6 +265,7 @@ impl Control { stream.set_nonblocking(true)?; Ok(Control { + policy: Policy::new(gateway_ip, allow, block), stream, input: Vec::new(), output: Vec::new(), @@ -271,7 +275,7 @@ impl Control { }) } - pub(super) fn service(&mut self, policy: &mut Policy) -> Result { + pub(super) fn service(&mut self, rules: &mut PrefixMap) -> Result { if !self.flush()? { return Ok(false); } @@ -296,7 +300,7 @@ impl Control { Ok(n) => { bytes_read += n; self.input.extend_from_slice(&buf[..n]); - self.process_input(policy)?; + self.process_input(rules)?; } Err(err) if err.kind() == ErrorKind::WouldBlock => break, Err(err) @@ -324,7 +328,7 @@ impl Control { .context("failed to shut down the control socket") } - fn process_input(&mut self, policy: &mut Policy) -> Result<()> { + fn process_input(&mut self, rules: &mut PrefixMap) -> Result<()> { loop { if self.discarding_input { if let Some(newline) = self.input.iter().position(|byte| *byte == b'\n') { @@ -362,11 +366,11 @@ impl Control { continue; } - let (response, update) = handle_request(policy, &line[..newline]); + let (response, update) = handle_request(&self.policy, rules.len(), &line[..newline]); self.enqueue(response)?; if let Some(update) = update { - policy.apply(update); + *rules = self.policy.apply(update); } } } @@ -428,7 +432,11 @@ struct SetParams { desired_revision: String, } -fn handle_request(policy: &Policy, line: &[u8]) -> (Value, Option) { +fn handle_request( + policy: &Policy, + rule_count: usize, + line: &[u8], +) -> (Value, Option) { let value = match serde_json::from_slice::(line) { Ok(value) => value, Err(_) => { @@ -465,7 +473,7 @@ fn handle_request(policy: &Policy, line: &[u8]) -> (Value, Option) "softnet.policy.get does not accept parameters", )) } else { - Ok(policy.result()) + Ok(policy.result(rule_count)) } } "softnet.policy.set" => { @@ -480,7 +488,7 @@ fn handle_request(policy: &Policy, line: &[u8]) -> (Value, Option) params.and_then(|params| { update = policy.set(params.allow, params.block, params.desired_revision)?; Ok(update.as_ref().map_or_else( - || policy.result(), + || policy.result(rule_count), |update| update.result(policy.bridge_isolation), )) }) @@ -613,36 +621,75 @@ mod tests { use super::{ BRIDGE_ISOLATION_CONFLICT, Control, INVALID_PARAMS, INVALID_REQUEST, MAX_PENDING_RESPONSE_BYTES, MAX_REQUEST_BYTES, MAX_TARGETS, METHOD_NOT_FOUND, PARSE_ERROR, - Policy, REVISION_CONFLICT, handle_request, + Policy, REVISION_CONFLICT, build_rules, handle_request, }; use crate::proxy::{Action, Target}; use ipnet::Ipv4Net; + use prefix_trie::PrefixMap; use serde_json::{Value, json}; use smoltcp::wire::Ipv4Address; use std::fs::File; use std::io::{Read, Write}; use std::net::{Shutdown, TcpListener}; - use std::os::fd::AsRawFd; + use std::os::fd::{AsRawFd, RawFd}; use std::os::unix::net::{UnixDatagram, UnixStream}; use std::str::FromStr; use std::time::Duration; - fn policy(allow: &[&str], block: &[&str]) -> Policy { - Policy::new( + struct TestPolicy { + state: Policy, + rules: PrefixMap, + } + + impl TestPolicy { + fn result(&self) -> Value { + self.state.result(self.rules.len()) + } + } + + impl std::ops::Deref for TestPolicy { + type Target = Policy; + + fn deref(&self) -> &Self::Target { + &self.state + } + } + + fn targets(targets: &[&str]) -> Vec { + targets + .iter() + .map(|target| target.parse().unwrap()) + .collect() + } + + fn policy(allow: &[&str], block: &[&str]) -> TestPolicy { + let gateway_ip = Ipv4Address::new(192, 168, 64, 1); + let allow = targets(allow); + let block = targets(block); + + TestPolicy { + rules: build_rules(gateway_ip, &allow, &block), + state: Policy::new(gateway_ip, allow, block), + } + } + + fn control(control_fd: RawFd) -> anyhow::Result { + Control::new( + control_fd, Ipv4Address::new(192, 168, 64, 1), - allow.iter().map(|target| target.parse().unwrap()).collect(), - block.iter().map(|target| target.parse().unwrap()).collect(), + Vec::new(), + Vec::new(), ) } - fn request(policy: &mut Policy, value: Value) -> Value { + fn request(policy: &mut TestPolicy, value: Value) -> Value { raw_request(policy, &serde_json::to_vec(&value).unwrap()) } - fn raw_request(policy: &mut Policy, line: &[u8]) -> Value { - let (response, update) = handle_request(policy, line); + fn raw_request(policy: &mut TestPolicy, line: &[u8]) -> Value { + let (response, update) = handle_request(&policy.state, policy.rules.len(), line); if let Some(update) = update { - policy.apply(update); + policy.rules = policy.state.apply(update); } response @@ -975,8 +1022,8 @@ mod tests { client .set_read_timeout(Some(Duration::from_secs(1))) .unwrap(); - let mut control = Control::new(server.as_raw_fd()).unwrap(); - let mut policy = policy(&[], &[]); + let mut control = control(server.as_raw_fd()).unwrap(); + let mut rules = PrefixMap::new(); client .write_all( @@ -988,7 +1035,7 @@ mod tests { ) .unwrap(); - assert!(control.service(&mut policy).unwrap()); + assert!(control.service(&mut rules).unwrap()); let mut response = [0; 2048]; let n = client.read(&mut response).unwrap(); let lines = std::str::from_utf8(&response[..n]) @@ -1000,12 +1047,12 @@ mod tests { assert_eq!(lines[0]["id"], 1); assert_eq!(lines[1]["id"], 2); assert_eq!(lines[1]["result"]["desiredRevision"], "11"); - assert_eq!(policy.allow, vec![Target::Host]); + assert_eq!(control.policy.allow, vec![Target::Host]); - let before = policy.result(); + let before = control.policy.result(rules.len()); drop(client); - assert!(!control.service(&mut policy).unwrap()); - assert_eq!(policy.result(), before); + assert!(!control.service(&mut rules).unwrap()); + assert_eq!(control.policy.result(rules.len()), before); } #[test] @@ -1014,15 +1061,15 @@ mod tests { client .set_read_timeout(Some(Duration::from_secs(1))) .unwrap(); - let mut control = Control::new(server.as_raw_fd()).unwrap(); - let mut policy = policy(&[], &[]); + let mut control = control(server.as_raw_fd()).unwrap(); + let mut rules = PrefixMap::new(); client .write_all(b"{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"softnet.policy.set\",\"params\":{\"allow\":[\"10.0.0.0/8\"],\"block\":[],\"desiredRevision\":\"final-rev\"}}\n") .unwrap(); client.shutdown(Shutdown::Write).unwrap(); - assert!(!control.service(&mut policy).unwrap()); + assert!(!control.service(&mut rules).unwrap()); let mut response = [0; 1024]; let n = client.read(&mut response).unwrap(); @@ -1035,17 +1082,17 @@ mod tests { #[test] fn oversized_frame_is_discarded_and_following_frame_is_processed() { let (_client, server) = UnixStream::pair().unwrap(); - let mut control = Control::new(server.as_raw_fd()).unwrap(); - let mut policy = policy(&[], &[]); + let mut control = control(server.as_raw_fd()).unwrap(); + let mut rules = PrefixMap::new(); control.input = vec![b'x'; MAX_REQUEST_BYTES + 1]; - control.process_input(&mut policy).unwrap(); + control.process_input(&mut rules).unwrap(); assert!(control.discarding_input); control.input.extend_from_slice( b"still-too-long\n{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"softnet.policy.get\"}\n", ); - control.process_input(&mut policy).unwrap(); + control.process_input(&mut rules).unwrap(); assert!(!control.discarding_input); let responses = std::str::from_utf8(&control.output) @@ -1064,36 +1111,36 @@ mod tests { client .set_read_timeout(Some(Duration::from_secs(1))) .unwrap(); - let mut control = Control::new(server.as_raw_fd()).unwrap(); - let mut policy = policy(&[], &[]); - let before = policy.result(); + let mut control = control(server.as_raw_fd()).unwrap(); + let mut rules = PrefixMap::new(); + let before = control.policy.result(rules.len()); client .write_all( b"{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"softnet.policy.set\",\"params\":{\"allow\":[\"@host\"],", ) .unwrap(); - assert!(control.service(&mut policy).unwrap()); - assert_eq!(policy.result(), before); + assert!(control.service(&mut rules).unwrap()); + assert_eq!(control.policy.result(rules.len()), before); assert!(control.output.is_empty()); client .write_all(b"\"block\":[],\"desiredRevision\":\"14\"}}\n") .unwrap(); - assert!(control.service(&mut policy).unwrap()); + assert!(control.service(&mut rules).unwrap()); let mut response = [0; 1024]; let n = client.read(&mut response).unwrap(); let response = serde_json::from_slice::(&response[..n - 1]).unwrap(); assert_eq!(response["id"], 1); assert_eq!(response["result"]["desiredRevision"], "14"); - assert_eq!(policy.allow, vec![Target::Host]); + assert_eq!(control.policy.allow, vec![Target::Host]); } #[test] fn response_backpressure_keeps_the_pending_queue_bounded() { let (_client, server) = UnixStream::pair().unwrap(); - let mut control = Control::new(server.as_raw_fd()).unwrap(); + let mut control = control(server.as_raw_fd()).unwrap(); let response = json!({"jsonrpc": "2.0", "id": 1, "result": "x".repeat(MAX_REQUEST_BYTES)}); let mut bounded = false; @@ -1119,29 +1166,29 @@ mod tests { #[test] fn response_queue_overflow_does_not_apply_a_policy_update() { let (_client, server) = UnixStream::pair().unwrap(); - let mut control = Control::new(server.as_raw_fd()).unwrap(); - let mut policy = policy(&[], &[]); - let before = policy.result(); + let mut control = control(server.as_raw_fd()).unwrap(); + let mut rules = PrefixMap::new(); + let before = control.policy.result(rules.len()); control.output = vec![b'x'; MAX_PENDING_RESPONSE_BYTES]; control.input = b"{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"softnet.policy.set\",\"params\":{\"allow\":[\"10.0.0.0/8\"],\"block\":[],\"desiredRevision\":\"overflow-rev\"}}\n".to_vec(); - let error = control.process_input(&mut policy).unwrap_err(); + let error = control.process_input(&mut rules).unwrap_err(); assert!( error .to_string() .contains("control response queue exceeded") ); - assert_eq!(policy.result(), before); - assert!(!policy.applied_revisions.contains("overflow-rev")); + assert_eq!(control.policy.result(rules.len()), before); + assert!(!control.policy.applied_revisions.contains("overflow-rev")); } #[test] fn response_backpressure_stops_consuming_policy_updates() { let (mut client, server) = UnixStream::pair().unwrap(); - let mut control = Control::new(server.as_raw_fd()).unwrap(); - let mut policy = policy(&[], &[]); - let before = policy.result(); + let mut control = control(server.as_raw_fd()).unwrap(); + let mut rules = PrefixMap::new(); + let before = control.policy.result(rules.len()); control.output = vec![b'x'; MAX_REQUEST_BYTES]; assert!(control.flush().unwrap()); @@ -1151,28 +1198,28 @@ mod tests { .write_all(b"{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"softnet.policy.set\",\"params\":{\"allow\":[\"10.0.0.0/8\"],\"block\":[],\"desiredRevision\":\"backpressure-rev\"}}\n") .unwrap(); - assert!(control.service(&mut policy).unwrap()); - assert_eq!(policy.result(), before); + assert!(control.service(&mut rules).unwrap()); + assert_eq!(control.policy.result(rules.len()), before); assert!(control.input.is_empty()); } #[test] fn validates_control_descriptor_without_taking_ownership() { let file = File::open("/dev/null").unwrap(); - let error = Control::new(file.as_raw_fd()).err().unwrap(); + let error = control(file.as_raw_fd()).err().unwrap(); assert!(error.to_string().contains("is not a socket")); assert!(file.metadata().is_ok()); let (datagram, _) = UnixDatagram::pair().unwrap(); - let error = Control::new(datagram.as_raw_fd()).err().unwrap(); + let error = control(datagram.as_raw_fd()).err().unwrap(); assert!(error.to_string().contains("not a Unix stream socket")); let listener = TcpListener::bind("127.0.0.1:0").unwrap(); - let error = Control::new(listener.as_raw_fd()).err().unwrap(); + let error = control(listener.as_raw_fd()).err().unwrap(); assert!(error.to_string().contains("not a Unix socket")); let (stream, _peer) = UnixStream::pair().unwrap(); - let control = Control::new(stream.as_raw_fd()).unwrap(); + let control = control(stream.as_raw_fd()).unwrap(); drop(control); assert!(unsafe { libc::fcntl(stream.as_raw_fd(), libc::F_GETFD) != -1 }); } @@ -1183,7 +1230,7 @@ mod tests { client .set_read_timeout(Some(Duration::from_secs(1))) .unwrap(); - let control = Control::new(server.as_raw_fd()).unwrap(); + let control = control(server.as_raw_fd()).unwrap(); control.shutdown().unwrap(); drop(control); diff --git a/lib/proxy/mod.rs b/lib/proxy/mod.rs index fb1d2ef..5c2fc6a 100644 --- a/lib/proxy/mod.rs +++ b/lib/proxy/mod.rs @@ -16,6 +16,7 @@ pub use exposed_port::ExposedPort; use ipnet::Ipv4Net; use mac_address::MacAddress; use port_forwarder::PortForwarder; +use prefix_trie::PrefixMap; use smoltcp::wire::EthernetFrame; use std::io::ErrorKind; use std::os::unix::io::{AsRawFd, RawFd}; @@ -29,7 +30,7 @@ pub struct Proxy<'proxy> { poller: Poller<'proxy>, vm_mac_address: smoltcp::wire::EthernetAddress, dhcp_snooper: DhcpSnooper, - policy: Policy, + rules: PrefixMap, control: Option, enobufs_encountered: bool, port_forwarder: PortForwarder, @@ -72,8 +73,11 @@ impl Proxy<'_> { let vm = VM::new(vm_fd)?; let host = Host::new(vm_net_type, Policy::bridge_isolation(&allow))?; let poller_timeout = Duration::from_millis(100); - let policy = Policy::new(host.gateway_ip, allow, block); - let control = control_fd.map(Control::new).transpose()?; + let control = control_fd + .map(|control_fd| { + Control::new(control_fd, host.gateway_ip, allow.clone(), block.clone()) + }) + .transpose()?; let poller = Poller::new( vm.as_raw_fd(), host.as_raw_fd(), @@ -81,13 +85,37 @@ impl Proxy<'_> { poller_timeout, )?; + // 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_target in allow { + let allow_prefix = match allow_target { + Target::Prefix(prefix) => prefix, + Target::Host => host.gateway_ip.into(), + }; + + rules.insert(allow_prefix, Action::Allow); + } + + for block_target in block { + let block_prefix = match block_target { + Target::Prefix(prefix) => prefix, + Target::Host => host.gateway_ip.into(), + }; + + rules.insert(block_prefix, Action::Block); + } + Ok(Proxy { vm, host, poller, vm_mac_address: smoltcp::wire::EthernetAddress(vm_mac_address.bytes()), dhcp_snooper: DhcpSnooper::new(poller_timeout), - policy, + rules, control, enobufs_encountered: false, port_forwarder: PortForwarder::new(exposed_ports), @@ -201,7 +229,7 @@ impl Proxy<'_> { return; }; - let keep_open = match control.service(&mut self.policy) { + let keep_open = match control.service(&mut self.rules) { Ok(keep_open) => keep_open, Err(err) => { log::warn!("disabling Softnet control socket: {err:#}"); @@ -248,7 +276,7 @@ mod tests { let proxy = create_proxy(vm_ip, vec!["66.66.0.0/16"], vec!["66.66.0.0/16"]); assert_eq!( - proxy.policy.rules, + proxy.rules, PrefixMap::::from_iter(vec![( Ipv4Net::from_str("66.66.0.0/16").unwrap(), Action::Block @@ -265,7 +293,7 @@ mod tests { let proxy = create_proxy(vm_ip, vec!["33.33.33.33/32"], vec!["33.33.33.0/24"]); assert_eq!( - proxy.policy.rules, + 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), @@ -284,7 +312,7 @@ mod tests { let proxy = create_proxy(vm_ip, vec!["@host"], vec!["0.0.0.0/0"]); assert_eq!( - proxy.policy.rules, + proxy.rules, PrefixMap::from_iter(vec![ (proxy.host.gateway_ip.into(), Action::Allow), (Ipv4Net::from_str("0.0.0.0/0").unwrap(), Action::Block), diff --git a/lib/proxy/vm.rs b/lib/proxy/vm.rs index c8f65b4..74946bf 100644 --- a/lib/proxy/vm.rs +++ b/lib/proxy/vm.rs @@ -52,10 +52,10 @@ impl Proxy<'_> { let dst_addr = ipv4_pkt.dst_addr(); // Filter traffic based on user-specified rules first - if !self.policy.rules.is_empty() { + if !self.rules.is_empty() { let dst_net = Ipv4Net::from(dst_addr); - if let Some((_, action)) = self.policy.rules.get_lpm(&dst_net) { + if let Some((_, action)) = self.rules.get_lpm(&dst_net) { return match action { Action::Allow => Some(()), Action::Block => None,