From db1a9bc34e73d90cbbee80a59dbe33ba51b665f7 Mon Sep 17 00:00:00 2001 From: Jun Kurihara Date: Sat, 21 Dec 2024 16:02:23 +0900 Subject: [PATCH] Revert "Feat/not forwarded arpa" --- CHANGELOG.md | 10 +- Cargo.toml | 2 +- proxy-lib/src/constants.rs | 4 - proxy-lib/src/doh_client/dns_message.rs | 8 - proxy-lib/src/doh_client/doh_client_main.rs | 45 ++--- .../doh_client/manipulation/default_rule.rs | 159 ------------------ .../doh_client/manipulation/domain_block.rs | 18 +- .../manipulation/domain_override.rs | 18 +- proxy-lib/src/doh_client/manipulation/mod.rs | 42 +---- proxy-lib/src/doh_client/mod.rs | 6 - proxy-lib/src/log.rs | 2 - 11 files changed, 51 insertions(+), 263 deletions(-) delete mode 100644 proxy-lib/src/doh_client/manipulation/default_rule.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 2967045..824f705 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,15 +13,7 @@ You should also include the user name that made the change. --> -## 0.4.2 (Unreleased) - -## 0.4.1 - -- Feat: support handling not-forwarded domains and local domains by default. For example, `resolver.arpa` is not forwarded to the upstream resolver, and `localhost` is always resolved to `127.0.0.1` or `::1`. -- Refactor: Various minor improvements -- Deps. - -## 0.4.0 +## 0.4.0 (Unreleased) - Feat: Support anonymous token based on blind RSA signatures. - Feat: DNS query logging (`qrlog` feature) diff --git a/Cargo.toml b/Cargo.toml index d9386fc..c31e560 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ members = ["proxy-bin", "proxy-lib"] resolver = "2" [workspace.package] -version = "0.4.2" +version = "0.4.1" authors = ["Jun Kurihara"] homepage = "https://github.com/junkurihara/doh-auth-proxy" repository = "https://github.com/junkurihara/doh-auth-proxy" diff --git a/proxy-lib/src/constants.rs b/proxy-lib/src/constants.rs index 725f89e..f79500f 100644 --- a/proxy-lib/src/constants.rs +++ b/proxy-lib/src/constants.rs @@ -96,10 +96,6 @@ pub const HEALTHCHECK_TARGET_ADDR: &str = "8.8.8.8"; pub const BLOCK_MESSAGE_HINFO_CPU: &str = "BLOCKED"; /// Block message for query manipulation (HINFO OS field) pub const BLOCK_MESSAGE_HINFO_OS: &str = "POWERED-BY-DOH-AUTH-PROXY"; -/// Not-forwarded message for query manipulation (HINFO CPU field) -pub const NOT_FORWARDED_MESSAGE_HINFO_CPU: &str = "NOT-FORWARDED-BY-DEFAULT"; -/// Not-forwarded message for query manipulation (HINFO OS field) -pub const NOT_FORWARDED_MESSAGE_HINFO_OS: &str = "POWERED-BY-DOH-AUTH-PROXY"; // Logging /// Query log channel size diff --git a/proxy-lib/src/doh_client/dns_message.rs b/proxy-lib/src/doh_client/dns_message.rs index 1b0c3cc..d3acbbe 100644 --- a/proxy-lib/src/doh_client/dns_message.rs +++ b/proxy-lib/src/doh_client/dns_message.rs @@ -120,14 +120,6 @@ pub fn build_response_nx(msg: &Message) -> Message { res } -/// Build a DNS response message with REFUSED -pub fn build_response_refused(msg: &Message) -> Message { - let mut res = msg.clone(); - res.set_message_type(hickory_proto::op::MessageType::Response); - res.set_response_code(hickory_proto::op::ResponseCode::Refused); - res -} - /// Build a DNS response message for given QueryKey and IP address pub fn build_response_given_ipaddr(msg: &Message, q_key: &QueryKey, ipaddr: &IpAddr, min_ttl: u32) -> anyhow::Result { let mut res = msg.clone(); diff --git a/proxy-lib/src/doh_client/doh_client_main.rs b/proxy-lib/src/doh_client/doh_client_main.rs index 1ef1af6..16226b2 100644 --- a/proxy-lib/src/doh_client/doh_client_main.rs +++ b/proxy-lib/src/doh_client/doh_client_main.rs @@ -45,7 +45,7 @@ pub struct DoHClient { /// health check interval pub(super) healthcheck_period_sec: tokio::time::Duration, /// Query manipulation pulugins - query_manipulators: QueryManipulators, + query_manipulators: Option, /// Query logging sender query_log_tx: crossbeam_channel::Sender, } @@ -119,10 +119,10 @@ impl DoHClient { let healthcheck_period_sec = globals.proxy_config.healthcheck_period_sec; // query manipulators - let query_manipulators: QueryManipulators = if let Some(q) = &globals.proxy_config.query_manipulation_config { - q.as_ref().try_into().unwrap_or_default() + let query_manipulators: Option = if let Some(q) = &globals.proxy_config.query_manipulation_config { + q.as_ref().try_into().ok() } else { - QueryManipulators::default() + None }; Ok(Self { @@ -186,29 +186,20 @@ impl DoHClient { })?; // Process query plugins from the beginning of vec, e.g., domain filtering, cloaking, etc. - - let execution_result = self.query_manipulators.apply(&query_msg, &req.0[0]).await?; - match execution_result { - QueryManipulationResult::PassThrough => (), - QueryManipulationResult::SyntheticResponseBlocked(response_msg) => { - let res = dns_message::encode(&response_msg)?; - self.log_dns_message(&res, proto, src, DoHResponseType::Blocked, None, start); - return Ok(res); - } - QueryManipulationResult::SyntheticResponseOverridden(response_msg) => { - let res = dns_message::encode(&response_msg)?; - self.log_dns_message(&res, proto, src, DoHResponseType::Overridden, None, start); - return Ok(res); - } - QueryManipulationResult::SyntheticResponseNotForwarded(response_msg) => { - let res = dns_message::encode(&response_msg)?; - self.log_dns_message(&res, proto, src, DoHResponseType::NotForwarded, None, start); - return Ok(res); - } - QueryManipulationResult::SyntheticResponseDefaultHost(response_msg) => { - let res = dns_message::encode(&response_msg)?; - self.log_dns_message(&res, proto, src, DoHResponseType::DefaultHost, None, start); - return Ok(res); + if let Some(manipulators) = &self.query_manipulators { + let execution_result = manipulators.apply(&query_msg, &req.0[0]).await?; + match execution_result { + QueryManipulationResult::PassThrough => (), + QueryManipulationResult::SyntheticResponseBlocked(response_msg) => { + let res = dns_message::encode(&response_msg)?; + self.log_dns_message(&res, proto, src, DoHResponseType::Blocked, None, start); + return Ok(res); + } + QueryManipulationResult::SyntheticResponseOverridden(response_msg) => { + let res = dns_message::encode(&response_msg)?; + self.log_dns_message(&res, proto, src, DoHResponseType::Overridden, None, start); + return Ok(res); + } } } diff --git a/proxy-lib/src/doh_client/manipulation/default_rule.rs b/proxy-lib/src/doh_client/manipulation/default_rule.rs deleted file mode 100644 index 5fed6b0..0000000 --- a/proxy-lib/src/doh_client/manipulation/default_rule.rs +++ /dev/null @@ -1,159 +0,0 @@ -use super::{ - super::{ - dns_message::{build_response_given_ipaddr, build_response_refused, QueryKey}, - error::DohClientError, - }, - inspect_query_name, QueryManipulation, QueryManipulationResult, -}; -use crate::{ - constants::{NOT_FORWARDED_MESSAGE_HINFO_CPU, NOT_FORWARDED_MESSAGE_HINFO_OS}, - log::*, -}; -use async_trait::async_trait; -use hickory_proto::{op::Message, rr}; -use match_domain::DomainMatchingRule; -use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; - -/* -------------------------------------------------------- */ -/// Default not-forwarded domains -const DEFAULT_NOT_FORWARDED_DOMAINS: &[&str] = &[ - // https://www.rfc-editor.org/rfc/rfc9462.html#name-caching-forwarders - "resolver.arpa", -]; -/// Default localhost -const DEFAULT_LOCAL_DOMAINS: &[&str] = &["localhost", "localhost.localdomain"]; -/// Default broadcast -const DEFAULT_BROADCAST_DOMAINS: &[&str] = &["broadcasthost"]; - -#[inline] -fn build_local_v4_response(query_message: &Message, query_key: &QueryKey) -> anyhow::Result { - let addr = IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)); - build_response_given_ipaddr(query_message, query_key, &addr, 0) -} -#[inline] -fn build_local_v6_response(query_message: &Message, query_key: &QueryKey) -> anyhow::Result { - let addr = IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1)); - build_response_given_ipaddr(query_message, query_key, &addr, 0) -} -#[inline] -// only v4 -fn build_broadcast_response(query_message: &Message, query_key: &QueryKey) -> anyhow::Result { - let addr = IpAddr::V4(Ipv4Addr::new(255, 255, 255, 255)); - build_response_given_ipaddr(query_message, query_key, &addr, 0) -} -/* -------------------------------------------------------- */ - -#[async_trait] -impl QueryManipulation for DefaultRule { - type Error = DohClientError; - - /// Apply query plugin - async fn apply(&self, query_message: &Message, query_key: &QueryKey) -> Result { - let q_name = inspect_query_name(&query_key.query_name)?; - - // Check if the query is for not-forwarded domains - if self.is_not_forwarded(q_name.as_str())? { - debug!( - "[Not-Forwarded] {} {:?} {:?}", - query_key.query_name, query_key.query_type, query_key.query_class - ); - let response_msg = build_response_not_forwarded(query_message); - return Ok(QueryManipulationResult::SyntheticResponseNotForwarded(response_msg)); - } - - // Check if the query is for localhost - if self.is_localhost(q_name.as_str()) { - debug!( - "[LocalHost] {} {:?} {:?}", - query_key.query_name, query_key.query_type, query_key.query_class - ); - - let response_msg = match query_key.query_type { - rr::RecordType::A => build_local_v4_response(query_message, query_key)?, - rr::RecordType::AAAA => build_local_v6_response(query_message, query_key)?, - _ => build_response_refused(query_message), - }; - return Ok(QueryManipulationResult::SyntheticResponseDefaultHost(response_msg)); - } - - // Check if the query is for broadcast - if self.is_broadcast(q_name.as_str()) { - debug!( - "[Broadcast] {} {:?} {:?}", - query_key.query_name, query_key.query_type, query_key.query_class - ); - - let response_msg = match query_key.query_type { - rr::RecordType::A => build_broadcast_response(query_message, query_key)?, - _ => build_response_refused(query_message), - }; - return Ok(QueryManipulationResult::SyntheticResponseDefaultHost(response_msg)); - } - - return Ok(QueryManipulationResult::PassThrough); - } -} - -/// Build a synthetic response message for default not-forwarded domains -fn build_response_not_forwarded(query_message: &Message) -> Message { - let mut msg = build_response_refused(query_message); - let hinfo = rr::rdata::HINFO::new( - NOT_FORWARDED_MESSAGE_HINFO_CPU.to_string(), - NOT_FORWARDED_MESSAGE_HINFO_OS.to_string(), - ); - msg.add_answer(rr::Record::from_rdata( - query_message.queries()[0].name().clone(), - 0, - rr::RData::HINFO(hinfo), - )); - msg -} - -#[derive(Debug, Clone)] -/// NotForwardedRule is a query manipulation rule that refuses queries based on domain matching -/// This is a default rule, handling the regulations of IETF RFC -pub struct DefaultRule { - /// inner domain matching rule - not_forwarded: DomainMatchingRule, -} - -impl DefaultRule { - /// Create a new NotForwardedRule - pub fn new() -> Self { - let not_forwarded = DomainMatchingRule::try_from(DEFAULT_NOT_FORWARDED_DOMAINS).unwrap(); - DefaultRule { not_forwarded } - } - - /// Check if the query key is in blocklist - fn is_not_forwarded(&self, q_name: &str) -> anyhow::Result { - Ok(self.not_forwarded.is_matched(q_name)) - } - - /// Check if the query key is for localhost - fn is_localhost(&self, q_name: &str) -> bool { - DEFAULT_LOCAL_DOMAINS.iter().any(|&d| d.eq(q_name)) - } - /// Check if the query key is for broadcast - fn is_broadcast(&self, q_name: &str) -> bool { - DEFAULT_BROADCAST_DOMAINS.iter().any(|&d| d.eq(q_name)) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn default_works() { - let default_rule = DefaultRule::new(); - - assert!(default_rule.is_not_forwarded("resolver.arpa").unwrap()); - assert!(default_rule.is_not_forwarded("_dns.resolver.arpa").unwrap()); - assert!(default_rule.is_localhost("localhost")); - assert!(default_rule.is_localhost("localhost.localdomain")); - assert!(!default_rule.is_localhost("localhost.localdomain.com")); - assert!(!default_rule.is_localhost("x.localhost.localdomain")); - assert!(default_rule.is_broadcast("broadcasthost")); - assert!(!default_rule.is_broadcast("broadcasthost.com")); - } -} diff --git a/proxy-lib/src/doh_client/manipulation/domain_block.rs b/proxy-lib/src/doh_client/manipulation/domain_block.rs index a9f64f3..1250316 100644 --- a/proxy-lib/src/doh_client/manipulation/domain_block.rs +++ b/proxy-lib/src/doh_client/manipulation/domain_block.rs @@ -3,13 +3,14 @@ use super::{ dns_message::{build_response_nx, QueryKey}, error::DohClientError, }, - inspect_query_name, QueryManipulation, QueryManipulationResult, + QueryManipulation, QueryManipulationResult, }; use crate::{ constants::{BLOCK_MESSAGE_HINFO_CPU, BLOCK_MESSAGE_HINFO_OS}, log::*, QueryManipulationConfig, }; +use anyhow::bail; use async_trait::async_trait; use hickory_proto::{op::Message, rr}; use match_domain::DomainMatchingRule; @@ -66,8 +67,19 @@ impl TryFrom<&QueryManipulationConfig> for Option { impl DomainBlockRule { /// Check if the query key is in blocklist pub fn in_blocklist(&self, q_key: &QueryKey) -> anyhow::Result { - // remove final dot and convert to lowercase - let nn = inspect_query_name(q_key.query_name.as_str())?; + // remove final dot + let mut nn = q_key.clone().query_name.to_ascii_lowercase(); + match nn.pop() { + Some(dot) => { + if dot != '.' { + bail!("Invalid query name as fqdn (missing final dot): {}", nn); + } + } + None => { + bail!("Missing query name"); + } + } + Ok(self.inner.is_matched(&nn)) } } diff --git a/proxy-lib/src/doh_client/manipulation/domain_override.rs b/proxy-lib/src/doh_client/manipulation/domain_override.rs index a8dcc71..2cafad2 100644 --- a/proxy-lib/src/doh_client/manipulation/domain_override.rs +++ b/proxy-lib/src/doh_client/manipulation/domain_override.rs @@ -3,7 +3,6 @@ use super::{ dns_message::{build_response_given_ipaddr, QueryKey}, error::DohClientError, }, - inspect_query_name, regexp_vals::*, QueryManipulation, QueryManipulationResult, }; @@ -96,10 +95,19 @@ impl TryFrom<&QueryManipulationConfig> for Option { impl DomainOverrideRule { pub fn find_mapping(&self, q_key: &QueryKey) -> Option<&MapsTo> { let q_type = q_key.query_type; - - // remove final dot and convert to lowercase - let nn = inspect_query_name(q_key.query_name.as_str()).ok()?; - + // remove final dot + let mut nn = q_key.clone().query_name.to_ascii_lowercase(); + match nn.pop() { + Some(dot) => { + if dot != '.' { + return None; + } + } + None => { + warn!("Null request!"); + return None; + } + } // find matches if let Some(targets) = self.inner.get(&nn) { targets.iter().find(|x| match x { diff --git a/proxy-lib/src/doh_client/manipulation/mod.rs b/proxy-lib/src/doh_client/manipulation/mod.rs index 72e3bd0..bdc70d7 100644 --- a/proxy-lib/src/doh_client/manipulation/mod.rs +++ b/proxy-lib/src/doh_client/manipulation/mod.rs @@ -1,31 +1,14 @@ -mod default_rule; mod domain_block; mod domain_override; mod regexp_vals; -use self::{default_rule::DefaultRule, domain_block::DomainBlockRule, domain_override::DomainOverrideRule}; use super::{dns_message::QueryKey, error::DohClientError}; use crate::QueryManipulationConfig; use async_trait::async_trait; +use domain_block::DomainBlockRule; +use domain_override::DomainOverrideRule; use hickory_proto::op::Message; -/* -------------------------------------------------------- */ -fn inspect_query_name(query_name: &str) -> anyhow::Result { - let mut nn = query_name.to_ascii_lowercase(); - match nn.pop() { - Some(dot) => { - if dot != '.' { - anyhow::bail!("Invalid query name as fqdn (missing final dot): {}", nn); - } - } - None => { - anyhow::bail!("Missing query name"); - } - } - Ok(nn) -} -/* -------------------------------------------------------- */ - /// Result of application of query manipulation for a given query pub enum QueryManipulationResult { /// Pass the query with no manipulator application @@ -36,12 +19,6 @@ pub enum QueryManipulationResult { /// By the query manipulation, synthetic response is generated /// Overridden response message SyntheticResponseOverridden(Message), - /// By the query manipulation, synthetic response is generated - /// Not-Forwarded response message, this is a default rule due to the nature of DNS forwarder - SyntheticResponseNotForwarded(Message), - /// By the query manipulation, synthetic response is generated - /// Response message for localhost.localdomain, this is a default rule due to the nature of DNS forwarder - SyntheticResponseDefaultHost(Message), } #[async_trait] @@ -73,15 +50,6 @@ impl QueryManipulators { } } -impl Default for QueryManipulators { - fn default() -> Self { - let default_rule = DefaultRule::new(); - QueryManipulators { - manipulators: vec![Box::new(default_rule) as Box + Send + Sync>], - } - } -} - impl TryFrom<&QueryManipulationConfig> for QueryManipulators { type Error = anyhow::Error; fn try_from(config: &QueryManipulationConfig) -> std::result::Result { @@ -97,10 +65,6 @@ impl TryFrom<&QueryManipulationConfig> for QueryManipulators { manipulators.push(Box::new(domain_block) as Box + Send + Sync>); } - // Default rule is the last one - let default_rule = DefaultRule::new(); - manipulators.push(Box::new(default_rule) as Box + Send + Sync>); - Ok(QueryManipulators { manipulators }) } } @@ -132,6 +96,6 @@ mod tests { assert!(manipulators.is_ok()); let manipulators = manipulators.unwrap(); - assert_eq!(manipulators.len(), 3); + assert_eq!(manipulators.len(), 2); } } diff --git a/proxy-lib/src/doh_client/mod.rs b/proxy-lib/src/doh_client/mod.rs index 73074ae..180ad02 100644 --- a/proxy-lib/src/doh_client/mod.rs +++ b/proxy-lib/src/doh_client/mod.rs @@ -18,10 +18,6 @@ pub enum DoHResponseType { Blocked, /// Overridden response Overridden, - /// Not forwarded due to the nature of dns forwarding (like resolver.arpa) - NotForwarded, - /// Overridden response for the localhost and broadcast addresses - DefaultHost, /// Cached response Cached, /// Standard response fetched from upstream @@ -33,8 +29,6 @@ impl std::fmt::Display for DoHResponseType { match self { DoHResponseType::Blocked => write!(f, "Blocked"), DoHResponseType::Overridden => write!(f, "Overridden"), - DoHResponseType::NotForwarded => write!(f, "NotForwarded"), - DoHResponseType::DefaultHost => write!(f, "DefaultHost"), DoHResponseType::Cached => write!(f, "Cached"), DoHResponseType::Normal => write!(f, "Normal"), } diff --git a/proxy-lib/src/log.rs b/proxy-lib/src/log.rs index 581fcf6..2b18d98 100644 --- a/proxy-lib/src/log.rs +++ b/proxy-lib/src/log.rs @@ -70,8 +70,6 @@ impl QueryLoggingBase { let dst = match self.res_type { DoHResponseType::Blocked => "blocked".to_owned(), DoHResponseType::Overridden => "overridden".to_owned(), - DoHResponseType::NotForwarded => "not_forwarded".to_owned(), - DoHResponseType::DefaultHost => "default_host".to_owned(), DoHResponseType::Cached => "cached".to_owned(), DoHResponseType::Normal => { if let Some(dst_url) = &self.dst_url {