Merge pull request #4258 from nymtech/jon/exit-policy-for-portless

Apply exit policy check on destination ips without port
This commit is contained in:
Tommy Verrall
2024-01-03 09:55:09 +00:00
committed by GitHub
4 changed files with 116 additions and 25 deletions
+63
View File
@@ -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,
+37 -14
View File
@@ -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<IpAddr, PolicyError> {
})
}
/// Helper: try to parse a port making sure it's non-zero
fn parse_port(s: &str) -> Result<u16, PolicyError> {
let port = s
.parse::<u16>()
.map_err(|_| PolicyError::InvalidPort { raw: s.to_string() })?;
if port == 0 {
Err(PolicyError::InvalidPort {
raw: port.to_string(),
})
} else {
Ok(port)
}
s.parse::<u16>()
.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::<PortRange>().unwrap(), PortRange::new_all());
assert!("hello".parse::<PortRange>().is_err());
assert!("0".parse::<PortRange>().is_err());
assert!("65536".parse::<PortRange>().is_err());
assert!("65537".parse::<PortRange>().is_err());
assert!("1-2-3".parse::<PortRange>().is_err());
@@ -680,6 +700,9 @@ mod test {
assert!("1-".parse::<PortRange>().is_err());
assert!("-2".parse::<PortRange>().is_err());
assert!("-".parse::<PortRange>().is_err());
assert_eq!("0".parse::<PortRange>().unwrap(), PortRange::new_zero(),);
assert!("0-1".parse::<PortRange>().is_err());
}
#[test]
@@ -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]
@@ -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<u8> to bytes::Bytes?