fix: Use correct DNS caching for SNI to function properly

This removes the old (broken) DNS caching method for reqwest, and replaces
it with resolvematrix' `MatrixDnsResolver`, as it correctly implements
caching and overrides for correct SNI behavior.

Cargo.lock

@@ -4540,8 +4540,7 @@ checksum = "1e061d1b48cb8d38042de4ae0a7a6401009d6143dc80d2e2d6f31f0bdd6470c7"
 [[package]]
 name = "resolvematrix"
 version = "1.0.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3121df844d2f59c801c703af7200b8dab5d1c04c5032cd57fa9347440346fe87"
+source = "git+https://forgejo.ellis.link/continuwuation/resolvematrix?rev=397294dd7fbae7b43678140fec64ff50cc1f6ab6#397294dd7fbae7b43678140fec64ff50cc1f6ab6"
 dependencies = [
  "futures",
  "hickory-resolver",

Cargo.toml

@@ -570,7 +570,9 @@ features = ["std"]
 version = "0.3.0"
 
 [workspace.dependencies.resolvematrix]
-version = "1.0.0"
+#version = "1.0.0"
+git = "https://forgejo.ellis.link/continuwuation/resolvematrix"
+rev = "397294dd7fbae7b43678140fec64ff50cc1f6ab6"
 
 [workspace.dependencies.serde_urlencoded]
 version = "0.7.1"

src/admin/debug/commands.rs

@@ -1017,7 +1017,7 @@ impl crate::Context<'_> {
 				.http_client(self.services.client.default.clone())
 				.build()?
 		} else {
-			&self.services.client.resolver
+			&self.services.client.matrix_resolver
 		};
 
 		let actual = resolver.resolve_server(server_name.as_str()).await?;

src/admin/query/resolver.rs

@@ -47,7 +47,7 @@ impl crate::Context<'_> {
 		writeln!(self, "| Server Name | Destination | SNI | Override | Step | Expires |").await?;
 		writeln!(self, "| ----------- | ----------- | --- | -------- | ---- | ------- |").await?;
 
-		let entries = self.services.client.resolver.get_all_cache_entries();
+		let entries = self.services.client.matrix_resolver.get_all_cache_entries();
 
 		for (host, entry) in entries {
 			if let Some(ors) = overrides
@@ -81,13 +81,13 @@ impl crate::Context<'_> {
 
 	async fn flush_cache(&self, name: Option<OwnedServerName>, all: bool) -> Result {
 		if all {
-			self.services.client.resolver.clear_cache();
-			self.services.client.dns.clear_cache();
+			self.services.client.matrix_resolver.clear_cache();
+			self.services.client.dns_resolver.clear_cache();
 			writeln!(self, "Resolver and DNS caches cleared!").await
 		} else if let Some(name) = name {
 			self.services
 				.client
-				.resolver
+				.matrix_resolver
 				.remove_cache_entry(name.as_str());
 			self.write_str(&format!("Cleared {name} from resolver caches!"))
 				.await

src/service/client/dns.rs

@@ -1,108 +0,0 @@
-use std::{net::SocketAddr, sync::Arc, time::Duration};
-
-use conduwuit::{Result, Server, err};
-use futures::FutureExt;
-use hickory_resolver::{
-	TokioResolver, config::ConnectionConfig, lookup_ip::LookupIp,
-	net::runtime::TokioRuntimeProvider,
-};
-use reqwest::dns::{Addrs, Name, Resolve, Resolving};
-
-pub struct Resolver {
-	pub(crate) resolver: Arc<TokioResolver>,
-	server: Arc<Server>,
-}
-
-type ResolvingResult = Result<Addrs, Box<dyn std::error::Error + Send + Sync>>;
-
-impl Resolver {
-	#[allow(clippy::as_conversions, clippy::cast_sign_loss, clippy::cast_possible_truncation)]
-	pub(super) fn build(server: &Arc<Server>) -> Result<Arc<Self>> {
-		let config = &server.config;
-		let (sys_conf, mut opts) = hickory_resolver::system_conf::read_system_conf()
-			.map_err(|e| err!(error!("Failed to configure DNS resolver from system: {e}")))?;
-
-		let mut conf = hickory_resolver::config::ResolverConfig::default();
-
-		if let Some(domain) = sys_conf.domain() {
-			conf.set_domain(domain.clone());
-		}
-
-		for sys_conf in sys_conf.search() {
-			conf.add_search(sys_conf.clone());
-		}
-
-		for sys_conf in sys_conf.name_servers() {
-			let mut ns = sys_conf.clone();
-
-			if config.query_over_tcp_only {
-				ns.connections = vec![ConnectionConfig::tcp()];
-			}
-
-			ns.trust_negative_responses = !config.query_all_nameservers;
-
-			conf.add_name_server(ns);
-		}
-
-		opts.cache_size = u64::from(config.dns_cache_entries);
-		opts.preserve_intermediates = true;
-		opts.negative_min_ttl = Some(Duration::from_secs(config.dns_min_ttl_nxdomain));
-		opts.negative_max_ttl = Some(Duration::from_hours(720));
-		opts.positive_min_ttl = Some(Duration::from_secs(config.dns_min_ttl));
-		opts.positive_max_ttl = Some(Duration::from_hours(168));
-		opts.timeout = Duration::from_secs(config.dns_timeout);
-		opts.attempts = config.dns_attempts as usize;
-		opts.try_tcp_on_error = config.dns_tcp_fallback;
-		opts.num_concurrent_reqs = 1;
-		opts.edns0 = true;
-		opts.case_randomization = true;
-		opts.ip_strategy = match config.ip_lookup_strategy {
-			| 1 => hickory_resolver::config::LookupIpStrategy::Ipv4Only,
-			| 2 => hickory_resolver::config::LookupIpStrategy::Ipv6Only,
-			| 3 => hickory_resolver::config::LookupIpStrategy::Ipv4AndIpv6,
-			| 4 => hickory_resolver::config::LookupIpStrategy::Ipv6thenIpv4,
-			| _ => hickory_resolver::config::LookupIpStrategy::Ipv4thenIpv6,
-		};
-
-		let runtime_provider = TokioRuntimeProvider::new();
-		let mut builder = TokioResolver::builder_with_config(conf, runtime_provider);
-		*builder.options_mut() = opts;
-		let resolver = Arc::new(builder.build().expect("failed to build resolver :("));
-
-		Ok(Arc::new(Self { resolver, server: server.clone() }))
-	}
-
-	/// Clear the in-memory hickory-dns caches
-	#[inline]
-	pub fn clear_cache(&self) { self.resolver.clear_cache(); }
-}
-
-impl Resolve for Resolver {
-	fn resolve(&self, name: Name) -> Resolving {
-		resolve_to_reqwest(self.server.clone(), self.resolver.clone(), name).boxed()
-	}
-}
-
-async fn resolve_to_reqwest(
-	server: Arc<Server>,
-	resolver: Arc<TokioResolver>,
-	name: Name,
-) -> ResolvingResult {
-	use std::{io, io::ErrorKind::Interrupted};
-
-	let handle_shutdown = || Box::new(io::Error::new(Interrupted, "Server shutting down"));
-	let handle_results = |results: LookupIp| {
-		Box::new(
-			results
-				.iter()
-				.collect::<Vec<_>>()
-				.into_iter()
-				.map(|ip| SocketAddr::new(ip, 0)),
-		)
-	};
-
-	tokio::select! {
-		results = resolver.lookup_ip(name.as_str()) => Ok(handle_results(results?)),
-		() = server.until_shutdown() => Err(handle_shutdown()),
-	}
-}

src/service/client/mod.rs

@@ -1,19 +1,20 @@
-mod dns;
-
 use std::{sync::Arc, time::Duration};
 
 use async_trait::async_trait;
-use conduwuit::{Config, Result, err, trace};
+use conduwuit::{Config, Result, Server, err, trace};
 use either::Either;
+use hickory_resolver::{
+	TokioResolver, config::ConnectionConfig, net::runtime::TokioRuntimeProvider,
+};
 use ipaddress::IPAddress;
 use reqwest::redirect;
 use resolvematrix::server::{MatrixResolver, MatrixResolverBuilder};
 
-use crate::{client::dns::Resolver, service};
+use crate::service;
 
 pub struct Service {
-	pub resolver: MatrixResolver,
-	pub dns: Arc<Resolver>,
+	pub matrix_resolver: Arc<MatrixResolver>,
+	pub dns_resolver: Arc<TokioResolver>,
 
 	pub default: reqwest::Client,
 	pub url_preview: reqwest::Client,
@@ -31,8 +32,10 @@ pub struct Service {
 impl crate::Service for Service {
 	fn build(args: crate::Args<'_>) -> Result<Arc<Self>> {
 		let config = &args.server.config;
-		let dns = Resolver::build(args.server)?;
-		let resolver = MatrixResolverBuilder::new()
+
+		let dns_resolver = get_dns_resolver(args.server)?;
+
+		let matrix_resolver = Arc::new(MatrixResolverBuilder::new()
 			.dangerous_tls_accept_invalid_certs(args.server.config.allow_invalid_tls_certificates_yes_i_know_what_the_fuck_i_am_doing_with_this_and_i_know_this_is_insecure)
 			.http_client(
 				base(&args.server.config)?
@@ -43,8 +46,9 @@ impl crate::Service for Service {
 					.redirect(redirect::Policy::limited(4))
 					.build()?
 			)
-			.dns_resolver(dns.resolver.clone())
-			.build()?;
+			.dns_resolver(dns_resolver.clone())
+			.build()?);
+		let matrix_dns_resolver = matrix_resolver.create_dns_resolver();
 
 		let url_preview_bind_addr = config
 			.url_preview_bound_interface
@@ -62,29 +66,31 @@ impl crate::Service for Service {
 			.unwrap_or_else(|| conduwuit::user_agent_media().to_owned());
 
 		Ok(Arc::new(Self {
-			resolver,
-			dns: dns.clone(),
+			matrix_resolver,
+			dns_resolver,
 
-			default: base(config)?.dns_resolver(dns.clone()).build()?,
+			default: base(config)?
+				.dns_resolver(matrix_dns_resolver.clone())
+				.build()?,
 
 			url_preview: base(config)
 				.and_then(|builder| {
 					builder_interface(builder, url_preview_bind_iface.as_deref())
 				})?
 				.local_address(url_preview_bind_addr)
-				.dns_resolver(dns.clone())
+				.dns_resolver(matrix_dns_resolver.clone())
 				.timeout(Duration::from_secs(config.url_preview_timeout))
 				.redirect(redirect::Policy::limited(3))
 				.user_agent(url_preview_user_agent)
 				.build()?,
 
 			extern_media: base(config)?
-				.dns_resolver(dns.clone())
+				.dns_resolver(matrix_dns_resolver.clone())
 				.redirect(redirect::Policy::limited(3))
 				.build()?,
 
 			federation: base(config)?
-				.dns_resolver(dns.clone())
+				.dns_resolver(matrix_dns_resolver.clone())
 				.connect_timeout(Duration::from_secs(config.federation_conn_timeout))
 				.read_timeout(Duration::from_secs(config.federation_timeout))
 				.timeout(Duration::from_secs(
@@ -98,7 +104,7 @@ impl crate::Service for Service {
 				.build()?,
 
 			federation_slow: base(config)?
-				.dns_resolver(dns.clone())
+				.dns_resolver(matrix_dns_resolver.clone())
 				.connect_timeout(Duration::from_secs(config.federation_conn_timeout))
 				.read_timeout(Duration::from_secs(config.federation_timeout.saturating_mul(6)))
 				.timeout(Duration::from_secs(
@@ -112,7 +118,7 @@ impl crate::Service for Service {
 				.build()?,
 
 			sender: base(config)?
-				.dns_resolver(dns.clone())
+				.dns_resolver(matrix_dns_resolver.clone())
 				.connect_timeout(Duration::from_secs(config.federation_conn_timeout))
 				.read_timeout(Duration::from_secs(config.sender_timeout))
 				.timeout(Duration::from_secs(config.sender_timeout))
@@ -122,7 +128,7 @@ impl crate::Service for Service {
 				.build()?,
 
 			appservice: base(config)?
-				.dns_resolver(dns.clone())
+				.dns_resolver(matrix_dns_resolver.clone())
 				.connect_timeout(Duration::from_secs(5))
 				.read_timeout(Duration::from_secs(config.appservice_timeout))
 				.timeout(Duration::from_secs(config.appservice_timeout))
@@ -132,7 +138,7 @@ impl crate::Service for Service {
 				.build()?,
 
 			pusher: base(config)?
-				.dns_resolver(dns)
+				.dns_resolver(matrix_dns_resolver)
 				.connect_timeout(Duration::from_secs(config.pusher_conn_timeout))
 				.timeout(Duration::from_secs(config.pusher_timeout))
 				.pool_max_idle_per_host(1)
@@ -151,11 +157,11 @@ impl crate::Service for Service {
 	}
 
 	async fn clear_cache(&self) {
-		self.resolver.clear_cache();
-		self.dns.resolver.clear_cache();
+		self.matrix_resolver.clear_cache();
+		self.dns_resolver.clear_cache();
 	}
 
-	fn name(&self) -> &str { service::make_name(std::module_path!()) }
+	fn name(&self) -> &str { service::make_name(module_path!()) }
 }
 
 impl Service {
@@ -254,3 +260,58 @@ fn builder_interface(
 		Ok(builder)
 	}
 }
+
+fn get_dns_resolver(server: &Arc<Server>) -> Result<Arc<TokioResolver>> {
+	let config = &server.config;
+	let (sys_conf, mut opts) = hickory_resolver::system_conf::read_system_conf()
+		.map_err(|e| err!(error!("Failed to configure DNS resolver from system: {e}")))?;
+
+	let mut conf = hickory_resolver::config::ResolverConfig::default();
+
+	if let Some(domain) = sys_conf.domain() {
+		conf.set_domain(domain.clone());
+	}
+
+	for sys_conf in sys_conf.search() {
+		conf.add_search(sys_conf.clone());
+	}
+
+	for sys_conf in sys_conf.name_servers() {
+		let mut ns = sys_conf.clone();
+
+		if config.query_over_tcp_only {
+			ns.connections = vec![ConnectionConfig::tcp()];
+		}
+
+		ns.trust_negative_responses = !config.query_all_nameservers;
+
+		conf.add_name_server(ns);
+	}
+
+	opts.cache_size = u64::from(config.dns_cache_entries);
+	opts.preserve_intermediates = true;
+	opts.negative_min_ttl = Some(Duration::from_secs(config.dns_min_ttl_nxdomain));
+	opts.negative_max_ttl = Some(Duration::from_hours(720));
+	opts.positive_min_ttl = Some(Duration::from_secs(config.dns_min_ttl));
+	opts.positive_max_ttl = Some(Duration::from_hours(168));
+	opts.timeout = Duration::from_secs(config.dns_timeout);
+	opts.attempts = config.dns_attempts.into();
+	opts.try_tcp_on_error = config.dns_tcp_fallback;
+	opts.num_concurrent_reqs = 1;
+	opts.edns0 = true;
+	opts.case_randomization = true;
+	opts.ip_strategy = match config.ip_lookup_strategy {
+		| 1 => hickory_resolver::config::LookupIpStrategy::Ipv4Only,
+		| 2 => hickory_resolver::config::LookupIpStrategy::Ipv6Only,
+		| 3 => hickory_resolver::config::LookupIpStrategy::Ipv4AndIpv6,
+		| 4 => hickory_resolver::config::LookupIpStrategy::Ipv6thenIpv4,
+		| _ => hickory_resolver::config::LookupIpStrategy::Ipv4thenIpv6,
+	};
+
+	let runtime_provider = TokioRuntimeProvider::new();
+	let mut builder = TokioResolver::builder_with_config(conf, runtime_provider);
+	*builder.options_mut() = opts;
+	let resolver = Arc::new(builder.build().expect("failed to build resolver :("));
+
+	Ok(resolver)
+}

src/service/federation/execute.rs

@@ -134,7 +134,7 @@ impl super::Service {
 		let actual = self
 			.services
 			.client
-			.resolver
+			.matrix_resolver
 			.resolve_server(dest.as_str())
 			.await?;