From a0535bfb3427a1c436b066e56acb7fdc1671745a Mon Sep 17 00:00:00 2001 From: Jun Kurihara Date: Fri, 8 Dec 2023 18:06:28 +0900 Subject: [PATCH 1/8] add stub --- modoh-lib/src/lib.rs | 1 + modoh-lib/src/request_filter/domain_filter.rs | 0 modoh-lib/src/request_filter/ip_filter.rs | 32 +++++++++++++++++++ modoh-lib/src/request_filter/mod.rs | 10 ++++++ 4 files changed, 43 insertions(+) create mode 100644 modoh-lib/src/request_filter/domain_filter.rs create mode 100644 modoh-lib/src/request_filter/ip_filter.rs create mode 100644 modoh-lib/src/request_filter/mod.rs diff --git a/modoh-lib/src/lib.rs b/modoh-lib/src/lib.rs index 87863e8..9d7ba70 100644 --- a/modoh-lib/src/lib.rs +++ b/modoh-lib/src/lib.rs @@ -8,6 +8,7 @@ mod hyper_executor; mod log; mod message_util; mod relay; +mod request_filter; mod router; mod target; mod validator; diff --git a/modoh-lib/src/request_filter/domain_filter.rs b/modoh-lib/src/request_filter/domain_filter.rs new file mode 100644 index 0000000..e69de29 diff --git a/modoh-lib/src/request_filter/ip_filter.rs b/modoh-lib/src/request_filter/ip_filter.rs new file mode 100644 index 0000000..81e88e9 --- /dev/null +++ b/modoh-lib/src/request_filter/ip_filter.rs @@ -0,0 +1,32 @@ +/// Header keys that may contain the client IP address by previous routers like CDNs. +const HEADER_IP_KEYS: &[&str] = &[ + "x-client-ip", + "x-forwarded-for", + "x-real-ip", + // load balancer like pulse secure + "x-cluster-client-ip", + // cloudflare v4 + "cf-connecting-ip", + // cloudflare v6 + "cf-connecting-ipv6", + // cloudflare enterprise + "true-client-ip", + // fastly + "fastly-client-ip", +]; + +// FastlyやCloudflareの場合、remote_addrがそれらCDNのアドレス帯に含まれているかチェックする。 +// remote_addrはTCP/QUIC handshakeが終わっているとして、xffとかはいくらでも偽装できるので一旦必ずCDNのIPチェックしないとダメ。CDN含め信頼できるIPを全部弾いた上で、一番過去のやつがsourceとみなすしかない。 +// 単純に一番過去のやつではない +// https://www.m3tech.blog/entry/x-forwarded-for +// https://christina04.hatenablog.com/entry/2016/10/25/190000 +// https://mrk21.hatenablog.com/entry/2020/08/06/214922 +// https://developers.cloudflare.com/fundamentals/reference/http-request-headers/ +// https://developer.fastly.com/reference/http/http-headers/Fastly-Client-IP/ +// Forwarded header +// https://www.nginx.com/resources/wiki/start/topics/examples/forwarded/ +// https://datatracker.ietf.org/doc/html/rfc7239 + +// cloudflare: https://developers.cloudflare.com/api/operations/cloudflare-i-ps-cloudflare-ip-details +// fastly: https://developer.fastly.com/reference/api/utils/public-ip-list/ +// https://github.com/femueller/cloud-ip-ranges githubのリストを使う手もある diff --git a/modoh-lib/src/request_filter/mod.rs b/modoh-lib/src/request_filter/mod.rs new file mode 100644 index 0000000..802ad5c --- /dev/null +++ b/modoh-lib/src/request_filter/mod.rs @@ -0,0 +1,10 @@ +mod domain_filter; +mod ip_filter; + +/// RequestFilter filtering inbound and outbound request. +pub struct RequestFilter { + /// outbound request filter mainly using for domain filtering + outbound_filter: Option<()>, + /// inbound request filter mainly using for ip filtering + inbound_filter: Option<()>, +} From 56871d2d48ea263afccdd75adf73fada852603ed Mon Sep 17 00:00:00 2001 From: Jun Kurihara Date: Fri, 8 Dec 2023 20:55:21 +0900 Subject: [PATCH 2/8] rewrite condition --- modoh-lib/src/hyper_client.rs | 4 ++-- modoh-lib/src/lib.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modoh-lib/src/hyper_client.rs b/modoh-lib/src/hyper_client.rs index 9f20f73..a5acbb5 100644 --- a/modoh-lib/src/hyper_client.rs +++ b/modoh-lib/src/hyper_client.rs @@ -64,7 +64,7 @@ Use this just for testing. Please enable native-tls or rustls feature to enable } } -#[cfg(feature = "native-tls")] +#[cfg(all(feature = "native-tls", not(feature = "rustls")))] impl HttpClient, B> where B: Body + Send + Unpin + 'static, @@ -102,7 +102,7 @@ where ::Error: Into>, { /// Build forwarder - pub async fn try_new(runtime_handle: tokio::runtime::Handle) -> Result { + pub fn try_new(runtime_handle: tokio::runtime::Handle) -> Result { todo!("Not implemented yet. Please use native-tls-backend feature for now."); // build hyper client with rustls and webpki, only https is allowed diff --git a/modoh-lib/src/lib.rs b/modoh-lib/src/lib.rs index 9d7ba70..753ae07 100644 --- a/modoh-lib/src/lib.rs +++ b/modoh-lib/src/lib.rs @@ -29,7 +29,7 @@ pub async fn entrypoint( term_notify: Option>, ) -> Result<()> { #[cfg(all(feature = "rustls", feature = "native-tls"))] - warn!("Both \"native-tls\" and feature \"rustls\" features are enabled. \"native-tls\" will be used."); + warn!("Both \"native-tls\" and feature \"rustls\" features are enabled. \"rustls\" will be used."); // build globals let globals = Arc::new(Globals { From 7694147f68321a7f90cfcadb1cd3efbcc8f00a91 Mon Sep 17 00:00:00 2001 From: Jun Kurihara Date: Tue, 12 Dec 2023 23:09:57 +0900 Subject: [PATCH 3/8] deps --- modoh-bin/Cargo.toml | 2 +- modoh-lib/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/modoh-bin/Cargo.toml b/modoh-bin/Cargo.toml index 0279d9f..c8a99fc 100644 --- a/modoh-bin/Cargo.toml +++ b/modoh-bin/Cargo.toml @@ -38,7 +38,7 @@ anyhow = "1.0.75" mimalloc = { version = "*", default-features = false } serde = { version = "1.0.193", default-features = false, features = ["derive"] } derive_builder = "0.12.0" -tokio = { version = "1.34.0", default-features = false, features = [ +tokio = { version = "1.35.0", default-features = false, features = [ "net", "rt-multi-thread", "time", diff --git a/modoh-lib/Cargo.toml b/modoh-lib/Cargo.toml index 5d679ae..53f7dc5 100644 --- a/modoh-lib/Cargo.toml +++ b/modoh-lib/Cargo.toml @@ -42,7 +42,7 @@ futures = { version = "0.3.29", default-features = false, features = [ "std", "async-await", ] } -tokio = { version = "1.34.0", features = [ +tokio = { version = "1.35.0", features = [ "net", "rt-multi-thread", "time", From eaffbc00ffa79ba6030963fc29d23ff6d85a1c1b Mon Sep 17 00:00:00 2001 From: Jun Kurihara Date: Wed, 13 Dec 2023 13:02:37 +0900 Subject: [PATCH 4/8] wip: use ipnet to express cidr --- modoh-bin/Cargo.toml | 3 ++ modoh-bin/src/config/target_config.rs | 54 +++++++++++++++++++++++++-- modoh-bin/src/config/toml.rs | 10 ++++- modoh-lib/Cargo.toml | 3 ++ modoh-lib/src/globals.rs | 7 +++- modoh-server.toml | 6 ++- 6 files changed, 74 insertions(+), 9 deletions(-) diff --git a/modoh-bin/Cargo.toml b/modoh-bin/Cargo.toml index c8a99fc..18b2e11 100644 --- a/modoh-bin/Cargo.toml +++ b/modoh-bin/Cargo.toml @@ -56,3 +56,6 @@ hot_reload = "0.1.4" # logging tracing = { version = "0.1.40" } tracing-subscriber = { version = "0.3.18", features = ["env-filter"] } + +# ip address +ipnet = { version = "2.9.0" } diff --git a/modoh-bin/src/config/target_config.rs b/modoh-bin/src/config/target_config.rs index 4a2d1d1..eefc449 100644 --- a/modoh-bin/src/config/target_config.rs +++ b/modoh-bin/src/config/target_config.rs @@ -2,8 +2,12 @@ use super::toml::ConfigToml; use crate::{error::*, log::*}; use async_trait::async_trait; use hot_reload::{Reload, ReloaderError}; +use ipnet::IpNet; use modoh_server_lib::{AccessConfig, ServiceConfig, ValidationConfig, ValidationConfigInner}; -use std::net::{IpAddr, SocketAddr}; +use std::{ + fs::read_to_string, + net::{IpAddr, SocketAddr}, +}; #[derive(PartialEq, Eq, Clone, Debug)] /// Wrapper of config toml and manipulation plugin settings @@ -154,15 +158,27 @@ impl TryInto for &TargetConfig { if let Some(access) = self.config_toml.access.as_ref() { let mut inner_ip = vec![]; - for ip in access.allowed_source_ip_addresses.iter() { - let ip = ip.parse::()?; + for ip in access.allowed_source_ips.as_ref().unwrap_or(&vec![]).iter() { + let ip = parse_ipnet(ip)?; info!("Set allowed source ip address: {}", ip); inner_ip.push(ip); } + let mut inner_cdn_ip = vec![]; + for ip in access.trusted_cdn_ips.as_ref().unwrap_or(&vec![]).iter() { + let ip = ip.parse::()?; + info!("Set trusted cdn ip address: {}", ip); + inner_cdn_ip.push(ip); + } + if let Some(cdn_ip_list_path) = access.trusted_cdn_ips_file.as_ref() { + let ip_list = read_ipnet_list_from_file(cdn_ip_list_path)?; + info!("Set trusted cdn ip address from file: {:#?}", ip_list); + inner_cdn_ip.extend(ip_list); + } + let mut inner_domain = vec![]; if service_conf.relay.is_some() { - for domain in access.allowed_destination_domains.iter() { + for domain in access.allowed_destination_domains.as_ref().unwrap_or(&vec![]).iter() { let domain = url::Url::parse(&format!("https://{domain}"))? .authority() .to_ascii_lowercase(); @@ -174,9 +190,39 @@ impl TryInto for &TargetConfig { service_conf.access = Some(AccessConfig { allowed_source_ip_addresses: inner_ip, allowed_destination_domains: inner_domain, + trusted_cdn_ip_addresses: inner_cdn_ip, }); }; Ok(service_conf) } } + +/// parse ipnet from string +fn parse_ipnet(ip: &str) -> anyhow::Result { + if ip.contains('/') { + let ip = ip.parse::()?; + return Ok(ip); + } + let ip = ip.parse::()?; + Ok(IpNet::from(ip)) +} + +/// read ipnet list from file +fn read_ipnet_list_from_file(path: &str) -> anyhow::Result> { + let list_lines = read_to_string(path)?; + + let ip_list = list_lines + .lines() + .filter_map(|line| { + let line_without_comment = line.trim().split('#').next().unwrap_or("").trim(); + if line_without_comment.is_empty() { + None + } else { + parse_ipnet(line_without_comment).ok() + } + }) + .collect::>(); + + Ok(ip_list) +} diff --git a/modoh-bin/src/config/toml.rs b/modoh-bin/src/config/toml.rs index 8ad9610..441a7ca 100644 --- a/modoh-bin/src/config/toml.rs +++ b/modoh-bin/src/config/toml.rs @@ -71,9 +71,15 @@ pub struct Token { /// Allowed source ip addresses and destination domains pub struct Access { /// Allowed source ip addresses - pub allowed_source_ip_addresses: Vec, + pub allowed_source_ips: Option>, + + /// Trusted cdn ip addresses + pub trusted_cdn_ips: Option>, + /// Trusted cdn ip addresses file + pub trusted_cdn_ips_file: Option, + /// Allowed destination domains - pub allowed_destination_domains: Vec, + pub allowed_destination_domains: Option>, } impl ConfigToml { diff --git a/modoh-lib/Cargo.toml b/modoh-lib/Cargo.toml index 53f7dc5..944580a 100644 --- a/modoh-lib/Cargo.toml +++ b/modoh-lib/Cargo.toml @@ -83,3 +83,6 @@ byteorder = "1.5.0" serde = { version = "1.0.193", default-features = false } auth-validator = { git = "https://github.com/junkurihara/rust-token-server", package = "rust-token-server-validator", branch = "develop" } serde_json = { version = "1.0.108" } + +# access control +ipnet = { version = "2.9.0" } diff --git a/modoh-lib/src/globals.rs b/modoh-lib/src/globals.rs index e86cb19..f9ac66a 100644 --- a/modoh-lib/src/globals.rs +++ b/modoh-lib/src/globals.rs @@ -1,7 +1,8 @@ use crate::{constants::*, count::RequestCount}; use auth_validator::ValidationConfig; +use ipnet::IpNet; use std::{ - net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV6}, + net::{Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV6}, sync::Arc, time::Duration, }; @@ -87,9 +88,11 @@ pub struct TargetConfig { /// Allowed source ip addresses and destination domains pub struct AccessConfig { /// Allowed source ip addresses - pub allowed_source_ip_addresses: Vec, + pub allowed_source_ip_addresses: Vec, /// Allowed destination domains pub allowed_destination_domains: Vec, + /// Trusted CDN ip addresses + pub trusted_cdn_ip_addresses: Vec, } impl Default for ServiceConfig { diff --git a/modoh-server.toml b/modoh-server.toml index 4f0b383..56efb92 100644 --- a/modoh-server.toml +++ b/modoh-server.toml @@ -64,7 +64,11 @@ client_ids = ["client_id_1", "client_id_2"] ## (TODO: from dns, should we fetch addrs associated with a given list of authorized relay domains? but such addresses may be compromised) ## This is evaluated when no authorization header is given in the request or no token validation is configured. ## This happens typcially for forwarded requests from a relay, not a client. -allowed_source_ip_addresses = ["192.168.1.1", "192.168.1.2"] +allowed_source_ips = ["192.168.1.1", "192.168.1.2", "192.168.11.0/24"] + +## Trusted CDNs and proxies ip ranges that will be written in X-Forwarded-For / Forwarded header +trusted_cdn_ips = ["192.168.123.0/24"] +trusted_cdn_ips_file = "/path/to/cdn_ips.txt" ## Allowed next destination target and relay domains allowed_destination_domains = ["example.com", "example.net"] From 4bab58ae5a24e0b074e9dc0afbb7411e91d128b3 Mon Sep 17 00:00:00 2001 From: Jun Kurihara Date: Wed, 13 Dec 2023 16:44:23 +0900 Subject: [PATCH 5/8] feat: implement initial ip filter --- modoh-bin/src/config/target_config.rs | 4 + modoh-bin/src/config/toml.rs | 2 + modoh-lib/src/error.rs | 9 + modoh-lib/src/globals.rs | 2 + modoh-lib/src/request_filter/ip_filter.rs | 224 ++++++++++++++++++++-- modoh-lib/src/request_filter/mod.rs | 22 ++- modoh-lib/src/router/router_main.rs | 13 +- modoh-lib/src/router/router_serve_req.rs | 21 +- modoh-server.toml | 12 +- 9 files changed, 286 insertions(+), 23 deletions(-) diff --git a/modoh-bin/src/config/target_config.rs b/modoh-bin/src/config/target_config.rs index eefc449..ef913a1 100644 --- a/modoh-bin/src/config/target_config.rs +++ b/modoh-bin/src/config/target_config.rs @@ -176,6 +176,9 @@ impl TryInto for &TargetConfig { inner_cdn_ip.extend(ip_list); } + let trust_previous_hop = access.trust_previous_hop.unwrap_or(true); + info!("Set trust previous hop: {}", trust_previous_hop); + let mut inner_domain = vec![]; if service_conf.relay.is_some() { for domain in access.allowed_destination_domains.as_ref().unwrap_or(&vec![]).iter() { @@ -191,6 +194,7 @@ impl TryInto for &TargetConfig { allowed_source_ip_addresses: inner_ip, allowed_destination_domains: inner_domain, trusted_cdn_ip_addresses: inner_cdn_ip, + trust_previous_hop, }); }; diff --git a/modoh-bin/src/config/toml.rs b/modoh-bin/src/config/toml.rs index 441a7ca..a73f954 100644 --- a/modoh-bin/src/config/toml.rs +++ b/modoh-bin/src/config/toml.rs @@ -77,6 +77,8 @@ pub struct Access { pub trusted_cdn_ips: Option>, /// Trusted cdn ip addresses file pub trusted_cdn_ips_file: Option, + /// Always trust previous proxy address retrieved from remote_addr + pub trust_previous_hop: Option, /// Allowed destination domains pub allowed_destination_domains: Option>, diff --git a/modoh-lib/src/error.rs b/modoh-lib/src/error.rs index 026ccd0..2d50714 100644 --- a/modoh-lib/src/error.rs +++ b/modoh-lib/src/error.rs @@ -98,6 +98,13 @@ pub enum HttpError { #[error("Invalid token")] InvalidToken, + #[error("Invalid forwarded header: {0}")] + InvalidForwardedHeader(String), + #[error("Invalid X-Forwarded-For header: {0}")] + InvalidXForwardedForHeader(String), + #[error("Forbidden source ip address: {0}")] + ForbiddenSourceAddress(String), + #[error(transparent)] Other(#[from] anyhow::Error), } @@ -143,6 +150,8 @@ impl From for StatusCode { HttpError::InvalidAuthorizationHeader => StatusCode::FORBIDDEN, HttpError::InvalidToken => StatusCode::UNAUTHORIZED, + HttpError::InvalidForwardedHeader(_) => StatusCode::BAD_REQUEST, + _ => StatusCode::INTERNAL_SERVER_ERROR, } } diff --git a/modoh-lib/src/globals.rs b/modoh-lib/src/globals.rs index f9ac66a..77b5f15 100644 --- a/modoh-lib/src/globals.rs +++ b/modoh-lib/src/globals.rs @@ -93,6 +93,8 @@ pub struct AccessConfig { pub allowed_destination_domains: Vec, /// Trusted CDN ip addresses pub trusted_cdn_ip_addresses: Vec, + /// Whether to trust previous hop reverse proxy + pub trust_previous_hop: bool, } impl Default for ServiceConfig { diff --git a/modoh-lib/src/request_filter/ip_filter.rs b/modoh-lib/src/request_filter/ip_filter.rs index 81e88e9..3ad71b1 100644 --- a/modoh-lib/src/request_filter/ip_filter.rs +++ b/modoh-lib/src/request_filter/ip_filter.rs @@ -1,19 +1,24 @@ -/// Header keys that may contain the client IP address by previous routers like CDNs. -const HEADER_IP_KEYS: &[&str] = &[ - "x-client-ip", - "x-forwarded-for", - "x-real-ip", - // load balancer like pulse secure - "x-cluster-client-ip", - // cloudflare v4 - "cf-connecting-ip", - // cloudflare v6 - "cf-connecting-ipv6", - // cloudflare enterprise - "true-client-ip", - // fastly - "fastly-client-ip", -]; +use crate::{error::*, log::*, AccessConfig}; +use http::{header, HeaderMap}; +use ipnet::IpNet; +use std::net::IpAddr; + +// /// Header keys that may contain the client IP address by previous routers like CDNs. +// const HEADER_IP_KEYS: &[&str] = &[ +// "x-client-ip", +// "x-forwarded-for", +// "x-real-ip", +// // load balancer like pulse secure +// "x-cluster-client-ip", +// // cloudflare v4 +// "cf-connecting-ip", +// // cloudflare v6 +// "cf-connecting-ipv6", +// // cloudflare enterprise +// "true-client-ip", +// // fastly +// "fastly-client-ip", +// ]; // FastlyやCloudflareの場合、remote_addrがそれらCDNのアドレス帯に含まれているかチェックする。 // remote_addrはTCP/QUIC handshakeが終わっているとして、xffとかはいくらでも偽装できるので一旦必ずCDNのIPチェックしないとダメ。CDN含め信頼できるIPを全部弾いた上で、一番過去のやつがsourceとみなすしかない。 @@ -30,3 +35,190 @@ const HEADER_IP_KEYS: &[&str] = &[ // cloudflare: https://developers.cloudflare.com/api/operations/cloudflare-i-ps-cloudflare-ip-details // fastly: https://developer.fastly.com/reference/api/utils/public-ip-list/ // https://github.com/femueller/cloud-ip-ranges githubのリストを使う手もある + +/// IpFilter filtering inbound request. +pub(crate) struct IpFilter { + /// allowed source ip addresses + allowed_source_ip_addresses: Vec, + /// trusted cdn ip addresses + trusted_cdn_ip_addresses: Vec, + /// whether to trust previous hop reverse proxy + trust_previous_hop: bool, +} + +impl IpFilter { + /// Create new IpFilter + pub(crate) fn new(access_config: &AccessConfig) -> Self { + let allowed_source_ip_addresses = access_config.allowed_source_ip_addresses.clone(); + let trusted_cdn_ip_addresses = access_config.trusted_cdn_ip_addresses.clone(); + Self { + allowed_source_ip_addresses, + trusted_cdn_ip_addresses, + trust_previous_hop: access_config.trust_previous_hop, + } + } + + /// Check if the incoming request is allowed to be forwarded by source ip filter + /// Note that `peer-addr` is always the address of the reverse proxy. + pub(crate) fn is_allowed_request(&self, peer_addr: &IpAddr, req_header: &HeaderMap) -> HttpResult<()> { + // Check if the source ip address is allowed if trust_previous_hop is false + if !self.trust_previous_hop && !self.is_allowed(peer_addr) { + return Err(HttpError::ForbiddenSourceAddress(format!( + "Previous hop ip address {} is not allowed", + peer_addr + ))); + } + + // First check the Forwarded header, and if exists (or invalid), capture this flow to assess the source ip address (skip x-forwarded-for, etc.) + let forwarded = retrieve_for_from_forwarded(req_header)?; + if !forwarded.is_empty() { + return self.is_origin_allowed(&forwarded); + } + + // Second check the X-Forwarded-For header and if exists (or invalid), capture this flow + let xff = retrieve_from_xff(req_header)?; + if !xff.is_empty() { + return self.is_origin_allowed(&xff); + } + + // In this case, we can see the origin is simply the peer_addr, then Ok(()) + debug!("No XFF and Forwarded header found. Pass: {}", peer_addr); + Ok(()) + } + + /// Remove proxy addresses from the given Ve + fn is_origin_allowed(&self, forwarded_addresses: &[IpAddr]) -> HttpResult<()> { + let filtered_proxies = forwarded_addresses + .iter() + .filter(|x| !self.trusted_cdn_ip_addresses.iter().any(|y| y.contains(*x))) + .collect::>(); + + let origin = if filtered_proxies.is_empty() { + // If all proxies are trusted, then the origin is supposed to be the first value + forwarded_addresses.first().unwrap() + } else { + // Otherwise, the first untrusted hop is the origin + filtered_proxies.last().unwrap() + }; + if self.is_allowed(origin) { + return Ok(()); + } + Err(HttpError::ForbiddenSourceAddress(format!( + "Origin ip address {} is not allowed", + origin + ))) + } + + /// Check if the source ip address is allowed + fn is_allowed(&self, source_ip: &IpAddr) -> bool { + self.allowed_source_ip_addresses.iter().any(|x| x.contains(source_ip)) + } +} + +/// Retrieved proxy addresses from X-Forwarded-For header +fn retrieve_from_xff(header: &HeaderMap) -> HttpResult> { + let xff = header::HeaderName::from_static("x-forwarded-for"); + let xff_view = header.get_all(xff).into_iter().collect::>(); + if xff_view.is_empty() { + return Ok(vec![]); + } + + let entries = xff_view + .iter() + .flat_map(|v| v.to_str().unwrap_or_default().split(',')) + .map(|v| v.trim()) + .collect::>(); + + let entries_extracted_for = entries + .iter() + .filter_map(|entry| entry.parse::().ok()) + .collect::>(); + if entries.len() != entries_extracted_for.len() { + return Err(HttpError::InvalidXForwardedForHeader( + "X-Forwarded-For header does not contain valid ip address".to_string(), + )); + } + Ok(entries_extracted_for) +} + +/// Retrieved proxy addresses from Forwarded header +/// [RFC7239](https://www.rfc-editor.org/rfc/rfc7239) +fn retrieve_for_from_forwarded(header: &HeaderMap) -> HttpResult> { + let forwarded_view = header.get_all(header::FORWARDED).into_iter().collect::>(); + if forwarded_view.is_empty() { + return Ok(vec![]); + } + + let entries = forwarded_view + .iter() + .flat_map(|v| v.to_str().unwrap_or_default().split(',')) + .collect::>(); + + if !entries.iter().all(|entry| entry.contains("for=")) { + return Err(HttpError::InvalidForwardedHeader( + "Forwarded header does not contain 'for='".to_string(), + )); + } + let entries_extracted_for = entries + .iter() + .filter_map(|entry| entry.split(';').find(|x| x.trim().starts_with("for="))) + .map(|v| v.split('=').last().unwrap_or_default().trim()) + .filter_map(|v| v.parse::().ok()) + .collect::>(); + if entries.len() != entries_extracted_for.len() { + return Err(HttpError::InvalidForwardedHeader( + "Forwarded header does not contain valid 'for='".to_string(), + )); + } + Ok(entries_extracted_for) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_forwarded_header() { + let mut req_header = HeaderMap::new(); + req_header.insert( + header::FORWARDED, + header::HeaderValue::from_str("proto=https;for=1.0.0.0;by=1.1.1.1").unwrap(), + ); + req_header.append( + header::FORWARDED, + header::HeaderValue::from_str("proto=https;for=2.0.0.0;by=1.1.1.1").unwrap(), + ); + req_header.append( + header::FORWARDED, + header::HeaderValue::from_str("proto=https;for=3.0.0.0;by=1.1.1.1, for=4.0.0.0").unwrap(), + ); + + let retrieved = retrieve_for_from_forwarded(&req_header).unwrap(); + assert_eq!(retrieved.len(), 4); + let mut iter = retrieved.iter(); + assert_eq!(iter.next(), Some(&IpAddr::from([1, 0, 0, 0]))); + assert_eq!(iter.next(), Some(&IpAddr::from([2, 0, 0, 0]))); + assert_eq!(iter.next(), Some(&IpAddr::from([3, 0, 0, 0]))); + assert_eq!(iter.next(), Some(&IpAddr::from([4, 0, 0, 0]))); + assert_eq!(iter.next(), None); + } + #[test] + fn test_xff_header() { + let xff = header::HeaderName::from_static("x-forwarded-for"); + let mut req_header = HeaderMap::new(); + req_header.insert( + xff.clone(), + header::HeaderValue::from_str("1.0.0.0, 2.0.0.0, 3.0.0.0").unwrap(), + ); + req_header.append(xff, header::HeaderValue::from_str("4.0.0.0").unwrap()); + + let retrieved = retrieve_from_xff(&req_header).unwrap(); + assert_eq!(retrieved.len(), 4); + let mut iter = retrieved.iter(); + assert_eq!(iter.next(), Some(&IpAddr::from([1, 0, 0, 0]))); + assert_eq!(iter.next(), Some(&IpAddr::from([2, 0, 0, 0]))); + assert_eq!(iter.next(), Some(&IpAddr::from([3, 0, 0, 0]))); + assert_eq!(iter.next(), Some(&IpAddr::from([4, 0, 0, 0]))); + assert_eq!(iter.next(), None); + } +} diff --git a/modoh-lib/src/request_filter/mod.rs b/modoh-lib/src/request_filter/mod.rs index 802ad5c..16d51e2 100644 --- a/modoh-lib/src/request_filter/mod.rs +++ b/modoh-lib/src/request_filter/mod.rs @@ -1,10 +1,28 @@ +use crate::AccessConfig; + +use self::ip_filter::IpFilter; mod domain_filter; mod ip_filter; /// RequestFilter filtering inbound and outbound request. pub struct RequestFilter { /// outbound request filter mainly using for domain filtering - outbound_filter: Option<()>, + pub outbound_filter: Option<()>, /// inbound request filter mainly using for ip filtering - inbound_filter: Option<()>, + pub inbound_filter: Option, +} + +impl RequestFilter { + /// Create new RequestFilter + pub(crate) fn new(access_config: &AccessConfig) -> Self { + let inbound_filter = if !access_config.allowed_source_ip_addresses.is_empty() { + Some(IpFilter::new(access_config)) + } else { + None + }; + Self { + outbound_filter: None, + inbound_filter, + } + } } diff --git a/modoh-lib/src/router/router_main.rs b/modoh-lib/src/router/router_main.rs index 53676b2..7df7148 100644 --- a/modoh-lib/src/router/router_main.rs +++ b/modoh-lib/src/router/router_main.rs @@ -1,7 +1,7 @@ use super::{router_serve_req::serve_request_with_validation, socket::bind_tcp_socket}; use crate::{ count::RequestCount, error::*, globals::Globals, hyper_client::HttpClient, hyper_executor::LocalExecutor, log::*, - relay::InnerRelay, target::InnerTarget, validator::Validator, + relay::InnerRelay, request_filter::RequestFilter, target::InnerTarget, validator::Validator, }; use hyper::{ body::Incoming, @@ -30,6 +30,8 @@ where inner_validator: Option>>, /// request count request_count: RequestCount, + /// request filter + request_filter: Option>, } impl Router @@ -53,6 +55,7 @@ where let relay_clone = self.inner_relay.clone(); let target_clone = self.inner_target.clone(); let validator_clone = self.inner_validator.clone(); + let request_filter_clone = self.request_filter.clone(); let timeout_sec = self.globals.service_config.timeout; self.globals.runtime_handle.clone().spawn(async move { timeout( @@ -69,6 +72,7 @@ where relay_clone.clone(), target_clone.clone(), validator_clone.clone(), + request_filter_clone.clone(), ) }), ), @@ -139,6 +143,11 @@ where Some(_) => Some(Validator::try_new(globals, http_client).await?), None => None, }; + let request_filter = globals + .service_config + .access + .as_ref() + .map(|_| Arc::new(RequestFilter::new(globals.service_config.access.as_ref().unwrap()))); Ok(Self { globals: globals.clone(), @@ -147,7 +156,7 @@ where inner_target, inner_validator, request_count, + request_filter, }) - // todo!() } } diff --git a/modoh-lib/src/router/router_serve_req.rs b/modoh-lib/src/router/router_serve_req.rs index c0de411..c4f3927 100644 --- a/modoh-lib/src/router/router_serve_req.rs +++ b/modoh-lib/src/router/router_serve_req.rs @@ -3,6 +3,7 @@ use crate::{ hyper_body::{passthrough_response, synthetic_error_response, synthetic_response, BoxBody, IncomingOr}, log::*, relay::InnerRelay, + request_filter::RequestFilter, target::InnerTarget, validator::Validator, }; @@ -14,10 +15,11 @@ use std::{net::SocketAddr, sync::Arc}; pub async fn serve_request_with_validation( req: Request, peer_addr: SocketAddr, - hostname: String, + _hostname: String, relay: Option>>, target: Option>, validator: Option>>, + request_filter: Option>, ) -> Result>> where C: Send + Sync + Connect + Clone + 'static, @@ -53,9 +55,24 @@ where }; } - // TODO: source ip access control here + // Source ip access control here // for authorized ip addresses, maintain blacklist (error metrics) at each relay for given requests // domain check should be done in forwarder. + let peer_ip_adder = peer_addr.ip(); + let req_header = req.headers(); + let filter_result = request_filter.as_ref().and_then(|filter| { + filter + .inbound_filter + .as_ref() + .map(|inbound| inbound.is_allowed_request(&peer_ip_adder, req_header)) + }); + if let Some(res) = filter_result { + if let Err(e) = res { + debug!("Source ip address is filtered: {}", e); + return synthetic_error_response(StatusCode::from(e)); + } + debug!("Passed source ip address access control"); + } // match modoh relay if relay.as_ref().map(|r| r.relay_path == path).unwrap_or(false) { diff --git a/modoh-server.toml b/modoh-server.toml index 56efb92..32e1430 100644 --- a/modoh-server.toml +++ b/modoh-server.toml @@ -64,11 +64,21 @@ client_ids = ["client_id_1", "client_id_2"] ## (TODO: from dns, should we fetch addrs associated with a given list of authorized relay domains? but such addresses may be compromised) ## This is evaluated when no authorization header is given in the request or no token validation is configured. ## This happens typcially for forwarded requests from a relay, not a client. -allowed_source_ips = ["192.168.1.1", "192.168.1.2", "192.168.11.0/24"] +allowed_source_ips = [ + "127.0.0.1", + "192.168.1.1", + "192.168.1.2", + "192.168.11.0/24", +] ## Trusted CDNs and proxies ip ranges that will be written in X-Forwarded-For / Forwarded header trusted_cdn_ips = ["192.168.123.0/24"] trusted_cdn_ips_file = "/path/to/cdn_ips.txt" +# Always trust previous hop ip address, retrieved from remote_addr. +# We set [default = true], since we assume that this application is always located internal network and exposed along with a TLS reverse proxy. +# (the previous hop is always trusted) +# If you set this to false, you should put your proxy address in trusted_cdn_ips or trusted_cdn_ips_file. +trust_previous_hop = true ## Allowed next destination target and relay domains allowed_destination_domains = ["example.com", "example.net"] From 3c6e0df1db6db3f502600d0d300a35efa591a877 Mon Sep 17 00:00:00 2001 From: Jun Kurihara Date: Wed, 13 Dec 2023 17:49:00 +0900 Subject: [PATCH 6/8] fix: support edge cases for xff --- modoh-lib/src/request_filter/domain_filter.rs | 3 ++ modoh-lib/src/request_filter/ip_filter.rs | 52 +++++++++++++++++-- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/modoh-lib/src/request_filter/domain_filter.rs b/modoh-lib/src/request_filter/domain_filter.rs index e69de29..cf4dcc8 100644 --- a/modoh-lib/src/request_filter/domain_filter.rs +++ b/modoh-lib/src/request_filter/domain_filter.rs @@ -0,0 +1,3 @@ +pub(crate) struct DomainFilter { + allowed_domains: Vec, +} diff --git a/modoh-lib/src/request_filter/ip_filter.rs b/modoh-lib/src/request_filter/ip_filter.rs index 3ad71b1..0252d6e 100644 --- a/modoh-lib/src/request_filter/ip_filter.rs +++ b/modoh-lib/src/request_filter/ip_filter.rs @@ -1,7 +1,7 @@ use crate::{error::*, log::*, AccessConfig}; use http::{header, HeaderMap}; use ipnet::IpNet; -use std::net::IpAddr; +use std::net::{IpAddr, SocketAddr}; // /// Header keys that may contain the client IP address by previous routers like CDNs. // const HEADER_IP_KEYS: &[&str] = &[ @@ -131,7 +131,7 @@ fn retrieve_from_xff(header: &HeaderMap) -> HttpResult> { let entries_extracted_for = entries .iter() - .filter_map(|entry| entry.parse::().ok()) + .filter_map(|entry| manipulate_ip_string(entry)) .collect::>(); if entries.len() != entries_extracted_for.len() { return Err(HttpError::InvalidXForwardedForHeader( @@ -152,6 +152,7 @@ fn retrieve_for_from_forwarded(header: &HeaderMap) -> HttpResult> { let entries = forwarded_view .iter() .flat_map(|v| v.to_str().unwrap_or_default().split(',')) + .map(|s| s.to_ascii_lowercase()) .collect::>(); if !entries.iter().all(|entry| entry.contains("for=")) { @@ -162,8 +163,8 @@ fn retrieve_for_from_forwarded(header: &HeaderMap) -> HttpResult> { let entries_extracted_for = entries .iter() .filter_map(|entry| entry.split(';').find(|x| x.trim().starts_with("for="))) - .map(|v| v.split('=').last().unwrap_or_default().trim()) - .filter_map(|v| v.parse::().ok()) + .map(|v| v.split('=').last().unwrap_or_default().trim().trim_matches('"')) + .filter_map(manipulate_ip_string) .collect::>(); if entries.len() != entries_extracted_for.len() { return Err(HttpError::InvalidForwardedHeader( @@ -173,6 +174,25 @@ fn retrieve_for_from_forwarded(header: &HeaderMap) -> HttpResult> { Ok(entries_extracted_for) } +fn manipulate_ip_string(ip: &str) -> Option { + if ip.starts_with('[') && ip.contains(']') { + // ipv6 case with bracket + // ip[1..ip.len() - 1].to_string() + return ip[1..].split(']').next().unwrap_or_default().parse::().ok(); + } + // ipv4 case, or ipv6 case without bracket, i.e., without port + if !ip.contains(':') { + // ipv4 case without port + return ip.parse::().ok(); + } + if ip.chars().filter(|v| v == &':').count() == 1 { + // ipv4 case with port + return ip.parse::().map(|v| v.ip()).ok(); + } + // ipv6 case without bracket + ip.parse::().ok() +} + #[cfg(test)] mod tests { use super::*; @@ -190,7 +210,7 @@ mod tests { ); req_header.append( header::FORWARDED, - header::HeaderValue::from_str("proto=https;for=3.0.0.0;by=1.1.1.1, for=4.0.0.0").unwrap(), + header::HeaderValue::from_str("proto=https;for=3.0.0.0;by=1.1.1.1, for=4.0.0.0:1234").unwrap(), ); let retrieved = retrieve_for_from_forwarded(&req_header).unwrap(); @@ -201,6 +221,28 @@ mod tests { assert_eq!(iter.next(), Some(&IpAddr::from([3, 0, 0, 0]))); assert_eq!(iter.next(), Some(&IpAddr::from([4, 0, 0, 0]))); assert_eq!(iter.next(), None); + + let mut req_header = HeaderMap::new(); + req_header.insert( + header::FORWARDED, + header::HeaderValue::from_str("proto=https;For=\"[2001:db8:cafe::17]\"").unwrap(), + ); + req_header.append( + header::FORWARDED, + header::HeaderValue::from_str("proto=https;For=\"[2001:db8:cafe::18]:1234\"").unwrap(), + ); + let retrieved = retrieve_for_from_forwarded(&req_header).unwrap(); + assert_eq!(retrieved.len(), 2); + let mut iter = retrieved.iter(); + assert_eq!( + iter.next(), + Some(&IpAddr::from([0x2001, 0xdb8, 0xcafe, 0, 0, 0, 0, 0x17])) + ); + assert_eq!( + iter.next(), + Some(&IpAddr::from([0x2001, 0xdb8, 0xcafe, 0, 0, 0, 0, 0x18])) + ); + assert_eq!(iter.next(), None); } #[test] fn test_xff_header() { From 96cf3b7d4c1bbfe6e8cb2f010d1a44e9e984e15f Mon Sep 17 00:00:00 2001 From: Jun Kurihara Date: Wed, 13 Dec 2023 18:27:59 +0900 Subject: [PATCH 7/8] wip: implement trie based domain filtering --- modoh-lib/Cargo.toml | 2 + modoh-lib/src/error.rs | 4 + modoh-lib/src/relay/relay_handle_url.rs | 1 + modoh-lib/src/relay/relay_main.rs | 27 ++- modoh-lib/src/request_filter/domain_filter.rs | 157 +++++++++++++++++- modoh-lib/src/request_filter/mod.rs | 15 +- modoh-lib/src/router/router_main.rs | 17 +- modoh-server.toml | 2 +- 8 files changed, 208 insertions(+), 17 deletions(-) diff --git a/modoh-lib/Cargo.toml b/modoh-lib/Cargo.toml index 944580a..19215fe 100644 --- a/modoh-lib/Cargo.toml +++ b/modoh-lib/Cargo.toml @@ -86,3 +86,5 @@ serde_json = { version = "1.0.108" } # access control ipnet = { version = "2.9.0" } +cedarwood = { version = "0.4.6" } +regex = { version = "1.10.2" } diff --git a/modoh-lib/src/error.rs b/modoh-lib/src/error.rs index 2d50714..fcefb0d 100644 --- a/modoh-lib/src/error.rs +++ b/modoh-lib/src/error.rs @@ -104,6 +104,8 @@ pub enum HttpError { InvalidXForwardedForHeader(String), #[error("Forbidden source ip address: {0}")] ForbiddenSourceAddress(String), + #[error("Forbidden destination domain: {0}")] + ForbiddenDomain(String), #[error(transparent)] Other(#[from] anyhow::Error), @@ -149,6 +151,8 @@ impl From for StatusCode { HttpError::NoAuthorizationHeader => StatusCode::FORBIDDEN, HttpError::InvalidAuthorizationHeader => StatusCode::FORBIDDEN, HttpError::InvalidToken => StatusCode::UNAUTHORIZED, + HttpError::ForbiddenSourceAddress(_) => StatusCode::FORBIDDEN, + HttpError::ForbiddenDomain(_) => StatusCode::FORBIDDEN, HttpError::InvalidForwardedHeader(_) => StatusCode::BAD_REQUEST, diff --git a/modoh-lib/src/relay/relay_handle_url.rs b/modoh-lib/src/relay/relay_handle_url.rs index d08c332..5d7fe9c 100644 --- a/modoh-lib/src/relay/relay_handle_url.rs +++ b/modoh-lib/src/relay/relay_handle_url.rs @@ -160,6 +160,7 @@ mod tests { relay_host: "example.com".to_string(), relay_path: "/proxy".to_string(), max_subseq_nodes: 3, + request_filter: None, }; let url = Url::parse("https://example.com/proxy?targethost=example1.com&targetpath=/dns-query").unwrap(); diff --git a/modoh-lib/src/relay/relay_main.rs b/modoh-lib/src/relay/relay_main.rs index 792d980..3ed6f76 100644 --- a/modoh-lib/src/relay/relay_main.rs +++ b/modoh-lib/src/relay/relay_main.rs @@ -6,6 +6,7 @@ use crate::{ hyper_client::HttpClient, log::*, message_util::{check_content_type, inspect_host, inspect_request_body, RequestType}, + request_filter::RequestFilter, }; use http::{ header::{self, HeaderMap, HeaderValue}, @@ -35,6 +36,8 @@ where pub(crate) relay_path: String, /// max number of subsequent nodes pub(super) max_subseq_nodes: usize, + /// request filter for destination domain name + pub(super) request_filter: Option>, } impl InnerRelay @@ -86,8 +89,23 @@ where }; debug!("(M)ODoH next hop url: {}", nexthop_url.as_str()); - // TODO: next hop domain name check here? + // next hop domain name check here // for authorized domains, maintain blacklist (error metrics) at each relay for given responses + let nexthop_domain = nexthop_url.host_str().ok_or(HttpError::InvalidUrl)?; + let filter_result = self.request_filter.as_ref().and_then(|filter| { + filter + .outbound_filter + .as_ref() + .map(|outbound| outbound.in_domain_list(nexthop_domain)) + }); + if let Some(res) = filter_result { + if !res { + debug!("Nexthop domain is filtered: {}", nexthop_domain); + return Err(HttpError::ForbiddenDomain(nexthop_domain.to_string())); + } + + debug!("Passed destination domain access control"); + } // split request into parts and body to manipulate them later let (mut parts, body) = req.into_parts(); @@ -158,7 +176,11 @@ where } /// Build inner relay - pub fn try_new(globals: &Arc, http_client: &Arc>) -> Result> { + pub fn try_new( + globals: &Arc, + http_client: &Arc>, + request_filter: Option>, + ) -> Result> { let relay_config = globals .service_config .relay @@ -185,6 +207,7 @@ where relay_host, relay_path, max_subseq_nodes, + request_filter: request_filter.clone(), })) } } diff --git a/modoh-lib/src/request_filter/domain_filter.rs b/modoh-lib/src/request_filter/domain_filter.rs index cf4dcc8..22ad89d 100644 --- a/modoh-lib/src/request_filter/domain_filter.rs +++ b/modoh-lib/src/request_filter/domain_filter.rs @@ -1,3 +1,158 @@ +use crate::log::*; +use cedarwood::Cedar; +use regex::Regex; + +/// Domain filter supporting prefix and suffix matching pub(crate) struct DomainFilter { - allowed_domains: Vec, + prefix_cedar: Cedar, + suffix_cedar: Cedar, + prefix_dict: Vec, + suffix_dict: Vec, +} + +/// Regex for domain or prefix matching +const REGEXP_DOMAIN_OR_PREFIX: &str = r"^([a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]*\.)+([a-zA-Z]{2,}|\*)"; + +/// Support function for reverse string +fn reverse_string(text: &str) -> String { + text.chars().rev().collect::() +} + +impl DomainFilter { + /// Create new domain filter + pub(crate) fn new(allowed_domains: Vec) -> Self { + let start_with_star = Regex::new(r"^\*\..+").unwrap(); + let end_with_star = Regex::new(r".+\.\*$").unwrap(); + // TODO: currently either one of prefix or suffix match with '*' is supported + let re = Regex::new(&format!("{}{}{}", r"^", REGEXP_DOMAIN_OR_PREFIX, r"$")).unwrap(); + let dict: Vec = allowed_domains + .iter() + .map(|d| if start_with_star.is_match(d) { &d[2..] } else { d }) + .filter(|x| re.is_match(x) || (x.split('.').count() == 1)) + .map(|y| y.to_ascii_lowercase()) + .collect(); + let prefix_dict: Vec = dict + .iter() + .filter(|d| end_with_star.is_match(d)) + .map(|d| d[..d.len() - 2].to_string()) + .collect(); + let suffix_dict: Vec = dict + .iter() + .filter(|d| !end_with_star.is_match(d)) + .map(|d| reverse_string(d)) + .collect(); + + let prefix_kv: Vec<(&str, i32)> = prefix_dict + .iter() + .map(AsRef::as_ref) + .enumerate() + .map(|(k, s)| (s, k as i32)) + .collect(); + let mut prefix_cedar = Cedar::new(); + prefix_cedar.build(&prefix_kv); + + let suffix_kv: Vec<(&str, i32)> = suffix_dict + .iter() + .map(AsRef::as_ref) + .enumerate() + .map(|(k, s)| (s, k as i32)) + .collect(); + let mut suffix_cedar = Cedar::new(); + suffix_cedar.build(&suffix_kv); + + Self { + prefix_cedar, + suffix_cedar, + prefix_dict, + suffix_dict, + } + } + + /// Check if the domain name is in the list by suffix/exact matching + fn find_suffix_match(&self, query_domain: &str) -> bool { + let rev_nn = reverse_string(query_domain); + let matched_items = self + .suffix_cedar + .common_prefix_iter(&rev_nn) + .map(|(x, _)| self.suffix_dict[x as usize].clone()); + + let mut matched_as_domain = matched_items.filter(|found| { + if found.len() == rev_nn.len() { + true + } else if let Some(nth) = rev_nn.chars().nth(found.chars().count()) { + nth.to_string() == "." + } else { + false + } + }); + matched_as_domain.next().is_some() + } + + /// Check if the domain name is in the list by prefix matching + fn find_prefix_match(&self, query_domain: &str) -> bool { + let matched_items = self + .prefix_cedar + .common_prefix_iter(query_domain) + .map(|(x, _)| self.prefix_dict[x as usize].clone()); + + let mut matched_as_domain = matched_items.filter(|found| { + if let Some(nth) = query_domain.chars().nth(found.chars().count()) { + nth.to_string() == "." + } else { + false + } + }); + matched_as_domain.next().is_some() + } + + pub fn in_domain_list(&self, domain_name: &str) -> bool { + // remove final dot + let nn = domain_name.to_ascii_lowercase(); + + if self.find_suffix_match(&nn) { + debug!("[with cw] suffix/exact match found: {}", nn); + return true; + } + + if self.find_prefix_match(&nn) { + debug!("[with cw] prefix match found: {}", nn); + return true; + } + + // TODO: other matching patterns + false + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn domain_filter_works() { + let allowed_domains = vec![ + "www.google.com".to_string(), + "www.yahoo.com".to_string(), + "*.google.com".to_string(), + "*.yahoo.com".to_string(), + "google.com".to_string(), + "yahoo.com".to_string(), + ]; + let domain_filter = DomainFilter::new(allowed_domains); + + let query = "www.google.com"; + assert!(domain_filter.in_domain_list(query)); + + let query = "www.yahoo.com"; + assert!(domain_filter.in_domain_list(query)); + + let query = "any.www.google.com"; + assert!(domain_filter.in_domain_list(query)); + + let query = "any.yahoo.com"; + assert!(domain_filter.in_domain_list(query)); + + let query = "googlee.com"; + assert!(!domain_filter.in_domain_list(query)); + } } diff --git a/modoh-lib/src/request_filter/mod.rs b/modoh-lib/src/request_filter/mod.rs index 16d51e2..7106d92 100644 --- a/modoh-lib/src/request_filter/mod.rs +++ b/modoh-lib/src/request_filter/mod.rs @@ -1,13 +1,13 @@ -use crate::AccessConfig; - -use self::ip_filter::IpFilter; mod domain_filter; mod ip_filter; +use self::{domain_filter::DomainFilter, ip_filter::IpFilter}; +use crate::AccessConfig; + /// RequestFilter filtering inbound and outbound request. pub struct RequestFilter { /// outbound request filter mainly using for domain filtering - pub outbound_filter: Option<()>, + pub outbound_filter: Option, /// inbound request filter mainly using for ip filtering pub inbound_filter: Option, } @@ -20,8 +20,13 @@ impl RequestFilter { } else { None }; + let outbound_filter = if !access_config.allowed_destination_domains.is_empty() { + Some(DomainFilter::new(access_config.allowed_destination_domains.clone())) + } else { + None + }; Self { - outbound_filter: None, + outbound_filter, inbound_filter, } } diff --git a/modoh-lib/src/router/router_main.rs b/modoh-lib/src/router/router_main.rs index 7df7148..a718e31 100644 --- a/modoh-lib/src/router/router_main.rs +++ b/modoh-lib/src/router/router_main.rs @@ -131,14 +131,6 @@ where ) -> Result { let request_count = globals.request_count.clone(); - let inner_relay = match &globals.service_config.relay { - Some(_) => Some(InnerRelay::try_new(globals, http_client)?), - None => None, - }; - let inner_target = match &globals.service_config.target { - Some(_) => Some(InnerTarget::try_new(globals)?), - None => None, - }; let inner_validator = match globals.service_config.validation.as_ref() { Some(_) => Some(Validator::try_new(globals, http_client).await?), None => None, @@ -149,6 +141,15 @@ where .as_ref() .map(|_| Arc::new(RequestFilter::new(globals.service_config.access.as_ref().unwrap()))); + let inner_relay = match &globals.service_config.relay { + Some(_) => Some(InnerRelay::try_new(globals, http_client, request_filter.clone())?), + None => None, + }; + let inner_target = match &globals.service_config.target { + Some(_) => Some(InnerTarget::try_new(globals)?), + None => None, + }; + Ok(Self { globals: globals.clone(), http_server: http_server.clone(), diff --git a/modoh-server.toml b/modoh-server.toml index 32e1430..d2889b1 100644 --- a/modoh-server.toml +++ b/modoh-server.toml @@ -81,4 +81,4 @@ trusted_cdn_ips_file = "/path/to/cdn_ips.txt" trust_previous_hop = true ## Allowed next destination target and relay domains -allowed_destination_domains = ["example.com", "example.net"] +allowed_destination_domains = ["example.com", "example.net", "*.example.org"] From 7de76ebe7a0ee5ba4d5e69b71956b8eb9ebf2167 Mon Sep 17 00:00:00 2001 From: Jun Kurihara Date: Wed, 13 Dec 2023 21:37:45 +0900 Subject: [PATCH 8/8] fix: allow skipping source ip filtering when token validated --- modoh-lib/src/router/router_serve_req.rs | 40 ++++++++++++++---------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/modoh-lib/src/router/router_serve_req.rs b/modoh-lib/src/router/router_serve_req.rs index c4f3927..ab0a22a 100644 --- a/modoh-lib/src/router/router_serve_req.rs +++ b/modoh-lib/src/router/router_serve_req.rs @@ -27,6 +27,7 @@ where //TODO: timeout for each services, which should be shorter than TIMEOUT_SEC in router_main.rs // validation with header + let mut token_validated = false; if let (Some(validator), true) = (validator, req.headers().contains_key(header::AUTHORIZATION)) { debug!("execute token validation"); let claims = match validator.validate_request(&req).await { @@ -40,7 +41,9 @@ where "token validation passed: subject {}", claims.custom.get("sub").and_then(|v| v.as_str()).unwrap_or("") ); + token_validated = true; } + // check path and route request let path = req.uri().path(); // match odoh config, without checking allowed ip address @@ -55,23 +58,28 @@ where }; } - // Source ip access control here - // for authorized ip addresses, maintain blacklist (error metrics) at each relay for given requests - // domain check should be done in forwarder. - let peer_ip_adder = peer_addr.ip(); - let req_header = req.headers(); - let filter_result = request_filter.as_ref().and_then(|filter| { - filter - .inbound_filter - .as_ref() - .map(|inbound| inbound.is_allowed_request(&peer_ip_adder, req_header)) - }); - if let Some(res) = filter_result { - if let Err(e) = res { - debug!("Source ip address is filtered: {}", e); - return synthetic_error_response(StatusCode::from(e)); + // Source ip access control here when we didn't have a chance to validate token. + // For authorized ip addresses, maintain blacklist (error metrics) at each relay for given requests. + // Domain check should be done in forwarder. + if !token_validated { + debug!("execute source ip access control"); + let peer_ip_adder = peer_addr.ip(); + let req_header = req.headers(); + let filter_result = request_filter.as_ref().and_then(|filter| { + filter + .inbound_filter + .as_ref() + .map(|inbound| inbound.is_allowed_request(&peer_ip_adder, req_header)) + }); + if let Some(res) = filter_result { + if let Err(e) = res { + debug!("Source ip address is filtered: {}", e); + return synthetic_error_response(StatusCode::from(e)); + } + debug!("Passed source ip address access control"); } - debug!("Passed source ip address access control"); + } else { + debug!("skip source ip access control since token was validated."); } // match modoh relay