diff --git a/fuzz/fuzz_targets/ipfilter.rs b/fuzz/fuzz_targets/ipfilter.rs index 223b64c02..1004f1d3c 100644 --- a/fuzz/fuzz_targets/ipfilter.rs +++ b/fuzz/fuzz_targets/ipfilter.rs @@ -22,8 +22,13 @@ impl<'a> Arbitrary<'a> for ASubnet { let addr = IpAddr::V4(Ipv4Addr::from(u.arbitrary::<[u8; 4]>()?)); Ok(ASubnet(IpSubnet { mask, addr })) } else { - let mask: u8 = u.int_in_range(0..=128)?; let addr = IpAddr::V6(Ipv6Addr::from(u.arbitrary::<[u8; 16]>()?)); + let addr = addr.to_canonical(); + let mask: u8 = if addr.is_ipv4() { + u.int_in_range(0..=32)? + } else { + u.int_in_range(0..=128)? + }; Ok(ASubnet(IpSubnet { mask, addr })) } } diff --git a/ntp-proto/src/ipfilter.rs b/ntp-proto/src/ipfilter.rs index 10cac3ef8..54defbacc 100644 --- a/ntp-proto/src/ipfilter.rs +++ b/ntp-proto/src/ipfilter.rs @@ -200,19 +200,20 @@ impl IpFilter { /// Check whether a given ip address is contained in the filter. /// Complexity: O(1) - pub fn is_in(&self, addr: &IpAddr) -> bool { + pub fn is_in(&self, addr: IpAddr) -> bool { + let addr = addr.to_canonical(); match addr { IpAddr::V4(addr) => self.is_in4(addr), IpAddr::V6(addr) => self.is_in6(addr), } } - fn is_in4(&self, addr: &Ipv4Addr) -> bool { + fn is_in4(&self, addr: Ipv4Addr) -> bool { self.ipv4_filter .lookup((u32::from_be_bytes(addr.octets()) as u128) << 96) } - fn is_in6(&self, addr: &Ipv6Addr) -> bool { + fn is_in6(&self, addr: Ipv6Addr) -> bool { self.ipv6_filter.lookup(u128::from_be_bytes(addr.octets())) } } @@ -229,7 +230,7 @@ pub mod fuzz { use super::*; fn contains(subnet: &IpSubnet, addr: &IpAddr) -> bool { - match (subnet.addr, addr) { + match (subnet.addr, addr.to_canonical()) { (IpAddr::V4(net), IpAddr::V4(addr)) => { let net = u32::from_be_bytes(net.octets()); let addr = u32::from_be_bytes(addr.octets()); @@ -263,13 +264,15 @@ pub mod fuzz { let filter = IpFilter::new(nets); for addr in addr { - assert_eq!(filter.is_in(addr), any_contains(nets, addr)); + assert_eq!(filter.is_in(*addr), any_contains(nets, addr)); } } } #[cfg(test)] mod tests { + use crate::SubnetParseError; + use super::*; #[test] @@ -295,31 +298,45 @@ mod tests { fn test_filter() { let filter = IpFilter::new(&[ "127.0.0.0/24".parse().unwrap(), - "::FFFF:0000:0000/96".parse().unwrap(), + "::FFFF:192.168.0.0/104".parse().unwrap(), ]); - assert!(filter.is_in(&"127.0.0.1".parse().unwrap())); - assert!(!filter.is_in(&"192.168.1.1".parse().unwrap())); - assert!(filter.is_in(&"::FFFF:ABCD:0123".parse().unwrap())); - assert!(!filter.is_in(&"::FEEF:ABCD:0123".parse().unwrap())); + assert!(filter.is_in("127.0.0.1".parse().unwrap())); + assert!(!filter.is_in("10.0.1.1".parse().unwrap())); + assert!(filter.is_in("::FFFF:192.168.1.1".parse().unwrap())); + assert!(!filter.is_in("::FFFF:10.0.0.5".parse().unwrap())); + assert!(!filter.is_in("::FEEF:ABCD:1234".parse().unwrap())); + } + + #[test] + fn test_subnet_mapped_ipv4_overlap() { + let subnet_err = "::FFFF:192.168.0.0/95".parse::().unwrap_err(); + assert_eq!(subnet_err, SubnetParseError::MaskV4Range); + } + + #[test] + fn test_subnet_mapped_ipv4() { + let subnet = "::FFFF:192.168.0.0/120".parse::().unwrap(); + assert_eq!(subnet.addr, "192.168.0.0".parse::().unwrap()); + assert_eq!(subnet.mask, 24); } #[test] fn test_subnet_edgecases() { let filter = IpFilter::new(&["0.0.0.0/0".parse().unwrap(), "::/0".parse().unwrap()]); - assert!(filter.is_in(&"0.0.0.0".parse().unwrap())); - assert!(filter.is_in(&"255.255.255.255".parse().unwrap())); - assert!(filter.is_in(&"::".parse().unwrap())); - assert!(filter.is_in(&"FFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF".parse().unwrap())); + assert!(filter.is_in("0.0.0.0".parse().unwrap())); + assert!(filter.is_in("255.255.255.255".parse().unwrap())); + assert!(filter.is_in("::".parse().unwrap())); + assert!(filter.is_in("FFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF".parse().unwrap())); let filter = IpFilter::new(&[ "1.2.3.4/32".parse().unwrap(), "10:32:54:76:98:BA:DC:FE/128".parse().unwrap(), ]); - assert!(filter.is_in(&"1.2.3.4".parse().unwrap())); - assert!(!filter.is_in(&"1.2.3.5".parse().unwrap())); - assert!(filter.is_in(&"10:32:54:76:98:BA:DC:FE".parse().unwrap())); - assert!(!filter.is_in(&"10:32:54:76:98:BA:DC:FF".parse().unwrap())); + assert!(filter.is_in("1.2.3.4".parse().unwrap())); + assert!(!filter.is_in("1.2.3.5".parse().unwrap())); + assert!(filter.is_in("10:32:54:76:98:BA:DC:FE".parse().unwrap())); + assert!(!filter.is_in("10:32:54:76:98:BA:DC:FF".parse().unwrap())); } } diff --git a/ntp-proto/src/server.rs b/ntp-proto/src/server.rs index 403dff35f..9a8ab0cc0 100644 --- a/ntp-proto/src/server.rs +++ b/ntp-proto/src/server.rs @@ -125,10 +125,10 @@ impl Server { } fn intended_action(&mut self, client_ip: IpAddr) -> (ServerResponse, ServerReason) { - if self.denyfilter.is_in(&client_ip) { + if self.denyfilter.is_in(client_ip) { // First apply denylist (self.config.denylist.action.into(), ServerReason::Policy) - } else if !self.allowfilter.is_in(&client_ip) { + } else if !self.allowfilter.is_in(client_ip) { // Then allowlist (self.config.allowlist.action.into(), ServerReason::Policy) } else if !self.client_cache.is_allowed( @@ -422,6 +422,7 @@ pub enum SubnetParseError { Subnet, Ip(AddrParseError), Mask, + MaskV4Range, } impl std::error::Error for SubnetParseError {} @@ -432,6 +433,10 @@ impl Display for SubnetParseError { Self::Subnet => write!(f, "Invalid subnet syntax"), Self::Ip(e) => write!(f, "{e} in subnet"), Self::Mask => write!(f, "Invalid subnet mask"), + Self::MaskV4Range => write!( + f, + "Subnet mask overflows the IPv4 range of an IPv4-mapped IPv6 address" + ), } } } @@ -449,6 +454,17 @@ impl std::str::FromStr for IpSubnet { let (addr, mask) = s.split_once('/').ok_or(SubnetParseError::Subnet)?; let addr: IpAddr = addr.parse()?; let mask: u8 = mask.parse().map_err(|_| SubnetParseError::Mask)?; + + // Canonicalize IPv4-mapped IPv6 addresses (e.g. `::ffff:192.168.0.0`) + // to their IPv4 form so they match against canonicalized filtered IPs. + let (addr, mask) = match (addr, addr.to_canonical()) { + (IpAddr::V6(_), canonical @ IpAddr::V4(_)) => { + let mask = mask.checked_sub(96).ok_or(SubnetParseError::MaskV4Range)?; + (canonical, mask) + } + _ => (addr, mask), + }; + let max_mask = match addr { IpAddr::V4(_) => 32, IpAddr::V6(_) => 128, @@ -456,6 +472,7 @@ impl std::str::FromStr for IpSubnet { if mask > max_mask { return Err(SubnetParseError::Mask); } + Ok(IpSubnet { addr, mask }) } }