diff --git a/src/dns/builder.rs b/src/dns/builder.rs index 01b9a10a..203f3bcb 100644 --- a/src/dns/builder.rs +++ b/src/dns/builder.rs @@ -89,6 +89,7 @@ impl PoolState { pub struct DnsRegistry { limits: GlobalLimits, groups: FxHashMap>, + system_groups: std::collections::HashMap, /// Default resolver, created lazily only if needed. default_resolver: Option>, forbidden: Arc<[IpNetwork]>, @@ -96,8 +97,11 @@ pub struct DnsRegistry { } impl DnsRegistry { - pub(crate) fn rule_bootstrap(&self, url: &str) -> std::io::Result> { + pub(crate) async fn rule_bootstrap(&self, url: &str) -> std::io::Result> { if let Some(resolver) = self.get_by_name(url) { + if self.system_groups.get(url) == Some(&true) { + SystemResolver::check_native_configuration(self.forbidden.clone()).await?; + } return Ok(real_only(resolver, self.forbidden.clone())); } let parsed = ParsedDnsUrl::parse(url) @@ -111,7 +115,10 @@ impl DnsRegistry { .to_parsed_server(None) .map_err(std::io::Error::other)?; let endpoint = match &server { - ParsedDnsServer::System => None, + ParsedDnsServer::System => { + SystemResolver::check_native_configuration(self.forbidden.clone()).await?; + None + } ParsedDnsServer::Udp { addr } | ParsedDnsServer::Tcp { addr } | ParsedDnsServer::Tls { addr, .. } @@ -199,6 +206,14 @@ impl DnsRegistry { } } groups.retain(|group| required.contains(&group.name)); + let mut system_groups = std::collections::HashMap::new(); + for group in &mut groups { + let system = group_uses_system(group, &system_groups)?; + system_groups.insert(group.name.clone(), system); + } + if system_groups.values().any(|system| *system) { + SystemResolver::check_native_configuration(guard.forbidden.clone()).await?; + } for group in &mut groups { group.fake_ip = None; for spec in &mut group.specs { @@ -238,6 +253,7 @@ impl DnsRegistry { Self { limits: crate::resources::limits(), groups: FxHashMap::default(), + system_groups: Default::default(), default_resolver: None, forbidden: Arc::from([]), pools: FxHashMap::default(), @@ -341,6 +357,41 @@ impl DnsRegistry { } } +fn group_uses_system( + group: &mut ExpandedDnsGroup, + system_groups: &std::collections::HashMap, +) -> std::io::Result { + if group.use_default_resolver { + return Ok(true); + } + for spec in &mut group.specs { + if spec.url == "system" { + return Ok(true); + } + let mut native_proxy = false; + for chain in &mut spec.client_chains { + for hop in chain.hops.iter_mut() { + crate::config::client_visit::visit_hop(hop, &mut |client| { + native_proxy |= client.address.address().hostname().is_some(); + Ok(()) + })?; + } + } + if native_proxy + || (ParsedDnsUrl::parse(&spec.url).is_ok_and(|url| url.has_hostname()) + && spec.bootstrap_url.as_ref().is_none_or(|bootstrap| { + system_groups + .get(bootstrap) + .copied() + .unwrap_or(bootstrap == "system") + })) + { + return Ok(true); + } + } + Ok(false) +} + fn checkpoint_pools(pools: Vec>) -> std::io::Result<()> { let mut error = None; for pool in pools { @@ -881,7 +932,9 @@ pub async fn build_dns_registry_for_runtime( }; let mut bootstraps = FxHashMap::default(); - for group in groups { + for mut group in groups { + let system = group_uses_system(&mut group, ®istry.system_groups)?; + registry.system_groups.insert(group.name.clone(), system); for spec in &group.specs { for endpoint in std::iter::once(spec.url.as_str()).chain(spec.bootstrap_url.as_deref()) { diff --git a/src/dns/builder/tests.rs b/src/dns/builder/tests.rs index c487d5e5..b6a8d198 100644 --- a/src/dns/builder/tests.rs +++ b/src/dns/builder/tests.rs @@ -154,6 +154,71 @@ async fn native_cache_policy_distinguishes_implicit_and_explicit_system() { } } +#[tokio::test] +async fn rule_system_bootstrap_checks_current_and_retained_forbidden_upstreams() { + let dir = tempfile::tempdir().unwrap(); + let state = dir.path().join("fake.json"); + let forbidden: Arc<[IpNetwork]> = + Arc::from(["0.0.0.0/0".parse().unwrap(), "::/0".parse().unwrap()]); + let guard = DnsRegistry { + forbidden: forbidden.clone(), + ..DnsRegistry::new() + }; + let source: crate::rule_set::RuleSource = serde_json::from_value(serde_json::json!({ + "url": "https://remote.invalid/rules.json", "path": dir.path().join("cache.json"), + "download": {"client_chain": "direct", "bootstrap_url": "fake"} + })) + .unwrap(); + for url in ["system", "https://upstream.invalid/dns-query"] { + let mut groups = fake_groups(true); + groups[0].specs = vec![make_spec(url)]; + groups[0].fake_ip.as_mut().unwrap().state_file = Some(state.clone()); + let error = DnsRegistry::for_rule_downloads( + groups, + GlobalLimits::default(), + std::slice::from_ref(&source), + &[], + Some(&guard), + ) + .await + .err() + .unwrap(); + assert!(error.to_string().contains("system DNS"), "{url}: {error}"); + assert!(!state.exists()); + } + let mut cached = build_dns_registry(fake_groups(false)).await.unwrap(); + cached.forbidden = forbidden; + for bootstrap in ["system", "fake"] { + let error = cached.rule_bootstrap(bootstrap).await.err().unwrap(); + assert!( + error.to_string().contains("system DNS"), + "{bootstrap}: {error}" + ); + } +} + +#[test] +fn system_bootstrap_dependencies_preserve_named_group_precedence() { + let mut groups = fake_groups(false); + let group = &mut groups[0]; + let mut known = std::collections::HashMap::from([("system".into(), false)]); + group.specs[0] = make_spec("https://upstream.invalid/dns-query"); + assert!(group_uses_system(group, &known).unwrap()); + group.specs[0].bootstrap_url = Some("system".into()); + assert!(!group_uses_system(group, &known).unwrap()); + known.insert("system".into(), true); + assert!(group_uses_system(group, &known).unwrap()); + group.specs[0] = make_spec("udp://127.0.0.1:53"); + assert!(!group_uses_system(group, &known).unwrap()); + group.specs[0].client_chains = vec![ + serde_json::from_value(serde_json::json!({ + "address": "proxy.invalid:1080", "protocol": {"type": "socks"} + })) + .unwrap(), + ]; + assert!(group_uses_system(group, &known).unwrap()); +} + #[tokio::test] async fn omitted_servers_keeps_default_cache_and_retired_pools_freeze_on_shutdown() { let configs = crate::config::create_server_configs( diff --git a/src/dns/system.rs b/src/dns/system.rs index 6eec592a..16f04747 100644 --- a/src/dns/system.rs +++ b/src/dns/system.rs @@ -27,6 +27,29 @@ pub(super) struct SystemResolver { } impl SystemResolver { + pub(super) async fn check_native_configuration(forbidden: Arc<[IpNetwork]>) -> io::Result<()> { + if forbidden.is_empty() { + return Ok(()); + } + tokio::task::spawn_blocking(move || { + let (mut config, _) = discover()?; + // Native lookup uses the OS configuration, not our filtered copy. + if config.name_servers.iter().any(|server| { + forbidden + .iter() + .any(|range| range.contains(server.ip.to_canonical())) + }) { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "system DNS endpoint overlaps a synthetic endpoint", + )); + } + filter_endpoints(&mut config, &forbidden) + }) + .await + .map_err(io::Error::other)? + } + pub(super) async fn check_configuration(forbidden: Arc<[IpNetwork]>) -> io::Result<()> { if crate::socket_policy::bootstrap().is_some() { return Ok(()); diff --git a/src/rule_updater/http.rs b/src/rule_updater/http.rs index 4ad36a9c..a780758f 100644 --- a/src/rule_updater/http.rs +++ b/src/rule_updater/http.rs @@ -60,7 +60,7 @@ async fn connect( url::Host::Ipv4(ip) => ip.to_string(), url::Host::Ipv6(ip) => ip.to_string(), }; - let resolver = dns.rule_bootstrap(&download.bootstrap_url)?; + let resolver = dns.rule_bootstrap(&download.bootstrap_url).await?; let target = target(url)?; let targets = crate::resolver::resolve_addresses(&resolver, &target) .await diff --git a/src/rule_updater/tests.rs b/src/rule_updater/tests.rs index 77ff567c..c3e8f5f1 100644 --- a/src/rule_updater/tests.rs +++ b/src/rule_updater/tests.rs @@ -1679,8 +1679,8 @@ async fn cold_rule_bootstrap_rejects_synthetic_upstreams_before_lookup() { .unwrap(); let error = dns .rule_bootstrap(&bootstrap) - .err() - .expect("cold synthetic bootstrap endpoint accepted"); + .await + .expect_err("cold synthetic bootstrap endpoint accepted"); assert!( error.to_string().contains("synthetic endpoint"), "{bootstrap}: {error}" @@ -1754,7 +1754,7 @@ async fn remote_bootstrap_group_names_take_precedence_over_url_syntax() { ) .await .unwrap(); - dns.rule_bootstrap(name).unwrap(); + dns.rule_bootstrap(name).await.unwrap(); } }