From 10ff165c187dd04180442fe126dceac2db39a644 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Mon, 18 Dec 2023 15:28:28 +0100 Subject: [PATCH] Apply exit policy check on portless packets --- common/exit-policy/src/lib.rs | 63 +++++++++++++++++++ .../exit-policy/src/policy/address_policy.rs | 51 ++++++++++----- .../ip-packet-router/src/error.rs | 8 ++- .../ip-packet-router/src/mixnet_listener.rs | 19 +++--- 4 files changed, 116 insertions(+), 25 deletions(-) diff --git a/common/exit-policy/src/lib.rs b/common/exit-policy/src/lib.rs index f1b41274bc..4c54e575b9 100644 --- a/common/exit-policy/src/lib.rs +++ b/common/exit-policy/src/lib.rs @@ -81,6 +81,15 @@ ExitPolicy accept6 *6:119 ExitPolicy accept *4:120 ExitPolicy reject6 [FC00::]/7:* +# Portless +ExitPolicy accept *:0 +ExitPolicy accept *4:0 +ExitPolicy accept *6:0 + +ExitPolicy reject *:0 +ExitPolicy reject *4:0 +ExitPolicy reject *6:0 + #ExitPolicy accept *:8080 #and another comment here ExitPolicy reject FE80:0000:0000:0000:0202:B3FF:FE1E:8329:* @@ -184,6 +193,60 @@ ExitPolicy reject *:* }, ); + // ExitPolicy accept *:0 + expected.push( + Accept, + AddressPortPattern { + ip_pattern: IpPattern::Star, + ports: PortRange::new_zero(), + }, + ); + + // ExitPolicy accept *4:0 + expected.push( + Accept, + AddressPortPattern { + ip_pattern: IpPattern::V4Star, + ports: PortRange::new_zero(), + }, + ); + + // ExitPolicy accept *6:0 + expected.push( + Accept, + AddressPortPattern { + ip_pattern: IpPattern::V6Star, + ports: PortRange::new_zero(), + }, + ); + + // ExitPolicy reject *:0 + expected.push( + Reject, + AddressPortPattern { + ip_pattern: IpPattern::Star, + ports: PortRange::new_zero(), + }, + ); + + // ExitPolicy reject *4:0 + expected.push( + Reject, + AddressPortPattern { + ip_pattern: IpPattern::V4Star, + ports: PortRange::new_zero(), + }, + ); + + // ExitPolicy reject *6:0 + expected.push( + Reject, + AddressPortPattern { + ip_pattern: IpPattern::V6Star, + ports: PortRange::new_zero(), + }, + ); + // ExitPolicy FE80:0000:0000:0000:0202:B3FF:FE1E:8329:* expected.push( Reject, diff --git a/common/exit-policy/src/policy/address_policy.rs b/common/exit-policy/src/policy/address_policy.rs index 09162a6e9b..621fd291e9 100644 --- a/common/exit-policy/src/policy/address_policy.rs +++ b/common/exit-policy/src/policy/address_policy.rs @@ -264,7 +264,13 @@ mod stringified_ip_pattern { impl AddressPortPattern { /// Return true iff this pattern matches a given address and port. pub fn matches(&self, addr: &IpAddr, port: u16) -> bool { - self.ip_pattern.matches(addr) && self.ports.contains(port) + // For backward compatibility, we treat port 0 as a wildcard until all gateways have + // upgraded, at which point we can add *:0 to the policy list. + if port == 0 { + self.ip_pattern.matches(addr) + } else { + self.ip_pattern.matches(addr) && self.ports.contains(port) + } } /// As matches, but accept a SocketAddr. @@ -395,19 +401,9 @@ fn parse_addr(s: &str) -> Result { }) } -/// Helper: try to parse a port making sure it's non-zero fn parse_port(s: &str) -> Result { - let port = s - .parse::() - .map_err(|_| PolicyError::InvalidPort { raw: s.to_string() })?; - - if port == 0 { - Err(PolicyError::InvalidPort { - raw: port.to_string(), - }) - } else { - Ok(port) - } + s.parse::() + .map_err(|_| PolicyError::InvalidPort { raw: s.to_string() }) } impl FromStr for IpPattern { @@ -494,6 +490,10 @@ impl PortRange { PortRange::new_unchecked(1, 65535) } + pub fn new_zero() -> Self { + PortRange { start: 0, end: 0 } + } + /// Create a new PortRange. /// /// The Portrange contains all ports between `start` and `end` inclusive. @@ -574,6 +574,7 @@ mod test { check("marzipan:80"); check("1.2.3.4:90-80"); + check("1.2.3.4:0-80"); check("1.2.3.4/100:8888"); check("[1.2.3.4]/16:80"); check("[::1]/130:8888"); @@ -612,6 +613,22 @@ mod test { check("0.0.0.0/0:*", &["127.0.0.1:80"], &["[f00b::]:80"]); check("[::]/0:*", &["[f00b::]:80"], &["127.0.0.1:80"]); + + check( + "*:0", + &["1.2.3.4:0", "[::1]:0", "9.0.0.0:0"], + &["1.2.3.4:443", "[::1]:500", "9.0.0.0:80", "[::1]:80"], + ); + check( + "*4:0", + &["1.2.3.4:0", "9.0.0.0:0"], + &["1.2.3.4:443", "9.0.0.0:80", "[::1]:0", "[::1]:80"], + ); + check( + "*6:0", + &["[::1]:0"], + &["[::1]:80", "1.2.3.4:0", "1.2.3.4:443"], + ); } #[test] @@ -620,6 +637,7 @@ mod test { policy.push(AddressPolicyAction::Accept, "*:443".parse()?); policy.push(AddressPolicyAction::Accept, "[::1]:80".parse()?); policy.push(AddressPolicyAction::Reject, "*:80".parse()?); + policy.push(AddressPolicyAction::Accept, "*:0".parse()?); let policy = policy; // drop mut assert!(policy @@ -640,6 +658,9 @@ mod test { assert!(policy .allows_sockaddr(&"127.0.0.1:66".parse().unwrap()) .is_none()); + assert!(policy + .allows_sockaddr(&"127.0.0.1:0".parse().unwrap()) + .unwrap()); Ok(()) } @@ -672,7 +693,6 @@ mod test { assert_eq!("*".parse::().unwrap(), PortRange::new_all()); assert!("hello".parse::().is_err()); - assert!("0".parse::().is_err()); assert!("65536".parse::().is_err()); assert!("65537".parse::().is_err()); assert!("1-2-3".parse::().is_err()); @@ -680,6 +700,9 @@ mod test { assert!("1-".parse::().is_err()); assert!("-2".parse::().is_err()); assert!("-".parse::().is_err()); + + assert_eq!("0".parse::().unwrap(), PortRange::new_zero(),); + assert!("0-1".parse::().is_err()); } #[test] diff --git a/service-providers/ip-packet-router/src/error.rs b/service-providers/ip-packet-router/src/error.rs index b71dac5ffc..77b2797c8e 100644 --- a/service-providers/ip-packet-router/src/error.rs +++ b/service-providers/ip-packet-router/src/error.rs @@ -1,4 +1,4 @@ -use std::net::SocketAddr; +use std::net::{IpAddr, SocketAddr}; pub use nym_client_core::error::ClientCoreError; use nym_exit_policy::PolicyError; @@ -57,9 +57,15 @@ pub enum IpPacketRouterError { #[error("the provided socket address, '{addr}' is not covered by the exit policy!")] AddressNotCoveredByExitPolicy { addr: SocketAddr }, + #[error("the provided ip address, '{ip}' is not covered by the exit policy!")] + IpNotCoveredByExitPolicy { ip: IpAddr }, + #[error("failed filter check: '{addr}'")] AddressFailedFilterCheck { addr: SocketAddr }, + #[error("failed filter check: '{ip}'")] + IpFailedFilterCheck { ip: IpAddr }, + #[error("failed to apply the exit policy: {source}")] ExitPolicyFailure { #[from] diff --git a/service-providers/ip-packet-router/src/mixnet_listener.rs b/service-providers/ip-packet-router/src/mixnet_listener.rs index 66911843c0..7eef17cec2 100644 --- a/service-providers/ip-packet-router/src/mixnet_listener.rs +++ b/service-providers/ip-packet-router/src/mixnet_listener.rs @@ -1,4 +1,7 @@ -use std::{collections::HashMap, net::IpAddr}; +use std::{ + collections::HashMap, + net::{IpAddr, SocketAddr}, +}; use futures::StreamExt; use nym_ip_packet_requests::{ @@ -320,15 +323,11 @@ impl MixnetListener { } // Filter check - if let Some(dst) = dst { - if !self.request_filter.check_address(&dst).await { - log::warn!("Failed filter check: {dst}"); - // TODO: we could consider sending back a response here - return Err(IpPacketRouterError::AddressFailedFilterCheck { addr: dst }); - } - } else { - // TODO: we should also filter packets without port number - log::warn!("Ignoring filter check for packet without port number! TODO!"); + let dst = dst.unwrap_or_else(|| SocketAddr::new(dst_addr, 0)); + if !self.request_filter.check_address(&dst).await { + log::info!("Denied filter check: {dst}"); + // TODO: we could consider sending back a response here + return Err(IpPacketRouterError::AddressFailedFilterCheck { addr: dst }); } // TODO: consider changing from Vec to bytes::Bytes?