From 0254fd64126dbff2260d541833035c33779b96a2 Mon Sep 17 00:00:00 2001 From: Frando Date: Mon, 29 Sep 2025 12:27:13 +0200 Subject: [PATCH 1/5] feat: add a DNS resolver trait --- iroh-relay/src/dns.rs | 383 ++++++++++++++++++++++++++++-------- iroh-relay/src/node_info.rs | 161 +++------------ 2 files changed, 333 insertions(+), 211 deletions(-) diff --git a/iroh-relay/src/dns.rs b/iroh-relay/src/dns.rs index 3228aed1fd..9067cc7071 100644 --- a/iroh-relay/src/dns.rs +++ b/iroh-relay/src/dns.rs @@ -3,7 +3,7 @@ use std::{ fmt, future::Future, - net::{IpAddr, Ipv6Addr, SocketAddr}, + net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}, sync::Arc, }; @@ -11,16 +11,17 @@ use hickory_resolver::{TokioResolver, name_server::TokioConnectionProvider}; use iroh_base::NodeId; use n0_future::{ StreamExt, + boxed::BoxFuture, time::{self, Duration}, }; use nested_enum_utils::common_fields; -use snafu::{Backtrace, GenerateImplicitData, OptionExt, Snafu}; +use snafu::{Backtrace, GenerateImplicitData, OptionExt, ResultExt, Snafu}; use tokio::sync::RwLock; use url::Url; use crate::{ defaults::timeouts::DNS_TIMEOUT, - node_info::{LookupError, NodeInfo}, + node_info::{self, NodeInfo, ParseError}, }; /// The n0 testing DNS node origin, for production. @@ -31,6 +32,30 @@ pub const N0_DNS_NODE_ORIGIN_STAGING: &str = "staging-dns.iroh.link"; /// Percent of total delay to jitter. 20 means +/- 20% of delay. const MAX_JITTER_PERCENT: u64 = 20; +/// Trait for DNS resolvers used in iroh. +pub trait Resolver: fmt::Debug + Send + Sync + 'static { + /// Looks up an IPv4 address. + fn lookup_ipv4(&self, host: String) -> BoxFuture, DnsError>>; + + /// Looks up an IPv6 address. + fn lookup_ipv6(&self, host: String) -> BoxFuture, DnsError>>; + + /// Looks up TXT records. + fn lookup_txt(&self, host: String) -> BoxFuture, DnsError>>; + + /// Clears the internal cache. + fn clear_cache(&self); + + /// Completely resets the DNS resolver. + /// + /// This is called when the host's network changes majorly. Implementations should rebind all sockets + /// and refresh the nameserver configuration if read from the host system. + fn reset(&mut self); +} + +/// Boxed iterator alias. +pub type BoxIter = Box + Send + 'static>; + /// Potential errors related to dns. #[common_fields({ backtrace: Option, @@ -61,6 +86,29 @@ pub enum DnsError { InvalidResponse {}, } +#[cfg(not(wasm_browser))] +#[common_fields({ + backtrace: Option, + #[snafu(implicit)] + span_trace: n0_snafu::SpanTrace, +})] +#[allow(missing_docs)] +#[derive(Debug, Snafu)] +#[non_exhaustive] +#[snafu(visibility(pub(crate)))] +pub enum LookupError { + #[snafu(display("Malformed txt from lookup"))] + ParseError { + #[snafu(source(from(ParseError, Box::new)))] + source: Box, + }, + #[snafu(display("Failed to resolve TXT record"))] + LookupFailed { + #[snafu(source(from(DnsError, Box::new)))] + source: Box, + }, +} + /// Error returned when an input value is too long for [`crate::node_info::UserData`]. #[allow(missing_docs)] #[derive(Debug, Snafu)] @@ -85,23 +133,33 @@ impl StaggeredError { /// The DNS resolver used throughout `iroh`. #[derive(Debug, Clone)] -pub struct DnsResolver { - resolver: Arc>, - nameserver: Option, -} +pub struct DnsResolver(Arc>); impl DnsResolver { - /// Create a new DNS resolver with sensible cross-platform defaults. + /// Creates a new DNS resolver with sensible cross-platform defaults. /// /// We first try to read the system's resolver from `/etc/resolv.conf`. /// This does not work at least on some Androids, therefore we fallback /// to the default `ResolverConfig` which uses eg. to google's `8.8.8.8` or `8.8.4.4`. pub fn new() -> Self { let resolver = Self::new_inner(); - Self { - resolver: Arc::new(RwLock::new(resolver)), + Self(Arc::new(RwLock::new(DnsResolverInner::Hickory { + resolver, nameserver: None, - } + }))) + } + + /// Creates a new [`DnsResolver`] from a struct that implements [`Resolver`]. + /// + /// [`Resolver`] is implemented for [`hickory_resolver::TokioResolver`], so you can construct + /// a [`TokioResolver`] and pass that to this function. + /// + /// To use a different DNS resolver, you need to implement [`Resolver`] for your custom resolver + /// and then pass to this function. + pub fn custom(resolver: impl Resolver) -> Self { + Self(Arc::new(RwLock::new(DnsResolverInner::Custom(Box::new( + resolver, + ))))) } fn new_inner() -> TokioResolver { @@ -132,13 +190,13 @@ impl DnsResolver { builder.build() } - /// Create a new DNS resolver configured with a single UDP DNS nameserver. + /// Creates a new DNS resolver configured with a single UDP DNS nameserver. pub fn with_nameserver(nameserver: SocketAddr) -> Self { let resolver = Self::with_nameserver_inner(nameserver); - Self { - resolver: Arc::new(RwLock::new(resolver)), - nameserver: Some(nameserver), - } + Self(Arc::new(RwLock::new(DnsResolverInner::Hickory { + resolver, + nameserver: None, + }))) } fn with_nameserver_inner(nameserver: SocketAddr) -> TokioResolver { @@ -156,58 +214,67 @@ impl DnsResolver { /// Removes all entries from the cache. pub async fn clear_cache(&self) { - self.resolver.read().await.clear_cache(); + let this = self.0.read().await; + this.clear_cache(); } - /// Recreate the inner resolver + /// Recreates the inner resolver. pub async fn reset(&self) { - let mut this = self.resolver.write().await; - let resolver = if let Some(nameserver) = self.nameserver { - Self::with_nameserver_inner(nameserver) - } else { - Self::new_inner() - }; - - *this = resolver; + let mut this = self.0.write().await; + match &mut *this { + DnsResolverInner::Hickory { + resolver, + nameserver, + } => { + *resolver = if let Some(nameserver) = *nameserver { + Self::with_nameserver_inner(nameserver) + } else { + Self::new_inner() + }; + } + DnsResolverInner::Custom(resolver) => { + resolver.reset(); + } + } } - /// Lookup a TXT record. + /// Looks up a TXT record. pub async fn lookup_txt( &self, host: T, timeout: Duration, - ) -> Result { + ) -> Result, DnsError> { let host = host.to_string(); - let this = self.resolver.read().await; - let res = time::timeout(timeout, this.txt_lookup(host)).await??; - Ok(TxtLookup(res)) + let this = self.0.read().await; + let res = time::timeout(timeout, this.lookup_txt(host)).await??; + Ok(res) } - /// Perform an ipv4 lookup with a timeout. + /// Performs an IPv4 lookup with a timeout. pub async fn lookup_ipv4( &self, host: T, timeout: Duration, ) -> Result + use, DnsError> { let host = host.to_string(); - let this = self.resolver.read().await; - let addrs = time::timeout(timeout, this.ipv4_lookup(host)).await??; - Ok(addrs.into_iter().map(|ip| IpAddr::V4(ip.0))) + let this = self.0.read().await; + let addrs = time::timeout(timeout, this.lookup_ipv4(host)).await??; + Ok(addrs.into_iter().map(|ip| IpAddr::V4(ip))) } - /// Perform an ipv6 lookup with a timeout. + /// Performs an IPv6 lookup with a timeout. pub async fn lookup_ipv6( &self, host: T, timeout: Duration, ) -> Result + use, DnsError> { let host = host.to_string(); - let this = self.resolver.read().await; - let addrs = time::timeout(timeout, this.ipv6_lookup(host)).await??; - Ok(addrs.into_iter().map(|ip| IpAddr::V6(ip.0))) + let this = self.0.read().await; + let addrs = time::timeout(timeout, this.lookup_ipv6(host)).await??; + Ok(addrs.into_iter().map(|ip| IpAddr::V6(ip))) } - /// Resolve IPv4 and IPv6 in parallel with a timeout. + /// Resolves IPv4 and IPv6 in parallel with a timeout. /// /// `LookupIpStrategy::Ipv4AndIpv6` will wait for ipv6 resolution timeout, even if it is /// not usable on the stack, so we manually query both lookups concurrently and time them out @@ -235,7 +302,7 @@ impl DnsResolver { } } - /// Resolve a hostname from a URL to an IP address. + /// Resolves a hostname from a URL to an IP address. pub async fn resolve_host( &self, url: &Url, @@ -273,7 +340,7 @@ impl DnsResolver { } } - /// Perform an ipv4 lookup with a timeout in a staggered fashion. + /// Performs an IPv4 lookup with a timeout in a staggered fashion. /// /// From the moment this function is called, each lookup is scheduled after the delays in /// `delays_ms` with the first call being done immediately. `[200ms, 300ms]` results in calls @@ -290,7 +357,7 @@ impl DnsResolver { stagger_call(f, delays_ms).await } - /// Perform an ipv6 lookup with a timeout in a staggered fashion. + /// Performs an IPv6 lookup with a timeout in a staggered fashion. /// /// From the moment this function is called, each lookup is scheduled after the delays in /// `delays_ms` with the first call being done immediately. `[200ms, 300ms]` results in calls @@ -307,7 +374,7 @@ impl DnsResolver { stagger_call(f, delays_ms).await } - /// Race an ipv4 and ipv6 lookup with a timeout in a staggered fashion. + /// Races an IPv4 and IPv6 lookup with a timeout in a staggered fashion. /// /// From the moment this function is called, each lookup is scheduled after the delays in /// `delays_ms` with the first call being done immediately. `[200ms, 300ms]` results in calls @@ -334,20 +401,24 @@ impl DnsResolver { node_id: &NodeId, origin: &str, ) -> Result { - let attrs = crate::node_info::TxtAttrs::::lookup_by_id( - self, node_id, origin, - ) - .await?; - let info = attrs.into(); + let name = node_info::node_domain(node_id, origin); + let name = node_info::ensure_iroh_txt_label(name); + let lookup = self + .lookup_txt(name.clone(), DNS_TIMEOUT) + .await + .context(LookupFailedSnafu)?; + let info = NodeInfo::from_txt_lookup(name, lookup).context(ParseSnafu)?; Ok(info) } /// Looks up node info by DNS name. pub async fn lookup_node_by_domain_name(&self, name: &str) -> Result { - let attrs = - crate::node_info::TxtAttrs::::lookup_by_name(self, name) - .await?; - let info = attrs.into(); + let name = node_info::ensure_iroh_txt_label(name.to_string()); + let lookup = self + .lookup_txt(name.clone(), DNS_TIMEOUT) + .await + .context(LookupFailedSnafu)?; + let info = NodeInfo::from_txt_lookup(name, lookup).context(ParseSnafu)?; Ok(info) } @@ -389,15 +460,6 @@ impl Default for DnsResolver { } } -impl From for DnsResolver { - fn from(resolver: TokioResolver) -> Self { - DnsResolver { - resolver: Arc::new(RwLock::new(resolver)), - nameserver: None, - } - } -} - impl reqwest::dns::Resolve for DnsResolver { fn resolve(&self, name: reqwest::dns::Name) -> reqwest::dns::Resolving { let this = self.clone(); @@ -419,40 +481,145 @@ impl reqwest::dns::Resolve for DnsResolver { } } -/// TXT records returned from [`DnsResolver::lookup_txt`] -#[derive(Debug, Clone)] -pub struct TxtLookup(pub(crate) hickory_resolver::lookup::TxtLookup); +/// Wrapper enum that contains either a hickory resolver or a custom resolver. +/// +/// We do this to save the cost of boxing the futures and iterators when using +/// default hickory resolver. +#[derive(Debug)] +enum DnsResolverInner { + Hickory { + resolver: hickory_resolver::TokioResolver, + nameserver: Option, + }, + Custom(Box), +} -impl From for TxtLookup { - fn from(value: hickory_resolver::lookup::TxtLookup) -> Self { - Self(value) +impl DnsResolverInner { + /// Looks up an IPv4 address. + async fn lookup_ipv4( + &self, + host: String, + ) -> Result + use<>, DnsError> { + match self { + DnsResolverInner::Hickory { resolver, .. } => { + let addrs = resolver + .ipv4_lookup(host) + .await? + .into_iter() + .map(Ipv4Addr::from); + Ok(Either::Left(addrs)) + } + DnsResolverInner::Custom(resolver) => { + Ok(Either::Right(resolver.lookup_ipv4(host).await?)) + } + } } -} -impl IntoIterator for TxtLookup { - type Item = TXT; + /// Looks up an IPv6 address. + async fn lookup_ipv6( + &self, + host: String, + ) -> Result + use<>, DnsError> { + match self { + DnsResolverInner::Hickory { resolver, .. } => { + let addrs = resolver + .ipv6_lookup(host) + .await? + .into_iter() + .map(Ipv6Addr::from); + Ok(Either::Left(addrs)) + } + DnsResolverInner::Custom(resolver) => { + Ok(Either::Right(resolver.lookup_ipv6(host).await?)) + } + } + } - type IntoIter = Box>; + /// Looks up TXT records. + async fn lookup_txt( + &self, + host: String, + ) -> Result + use<>, DnsError> { + match self { + DnsResolverInner::Hickory { resolver, .. } => { + let lookup = resolver + .txt_lookup(host) + .await? + .into_iter() + .map(|txt| TxtRecordData::from_iter(txt.iter().cloned())); + Ok(Either::Left(lookup)) + } + DnsResolverInner::Custom(resolver) => { + Ok(Either::Right(resolver.lookup_txt(host).await?)) + } + } + } - fn into_iter(self) -> Self::IntoIter { - Box::new(self.0.into_iter().map(TXT)) + /// Clears the internal cache. + fn clear_cache(&self) { + match self { + DnsResolverInner::Hickory { resolver, .. } => resolver.clear_cache(), + DnsResolverInner::Custom(resolver) => resolver.clear_cache(), + } } } -/// Record data for a TXT record +/// Record data for a TXT record. +/// +/// This contains a list of character strings, as defined in [RFC 1035 Section 3.3.14]. +/// +/// [`TxtRecordData`] implements [`fmt::Display`], so you can call [`ToString::to_string`] to +/// convert the record data into a string. This will parse each character string with +/// [`String::from_utf8_lossy`] and then concatenate all strings without a separator. +/// +/// If you want to process each character string individually, use [`Self::iter`]. +/// +/// [RFC 1035 Section 3.3.14]: https://datatracker.ietf.org/doc/html/rfc1035#section-3.3.14 #[derive(Debug, Clone)] -pub struct TXT(hickory_resolver::proto::rr::rdata::TXT); +pub struct TxtRecordData(Box<[Box<[u8]>]>); -impl TXT { - /// Returns the raw character strings of this TXT record. - pub fn txt_data(&self) -> &[Box<[u8]>] { - self.0.txt_data() +impl TxtRecordData { + /// Returns an iterator over the character strings contained in this TXT record. + pub fn iter(&self) -> impl Iterator { + self.0.iter().map(|x| x.as_ref()) } } -impl fmt::Display for TXT { +impl fmt::Display for TxtRecordData { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.0) + for s in self.iter() { + write!(f, "{}", &String::from_utf8_lossy(s))? + } + Ok(()) + } +} + +impl FromIterator> for TxtRecordData { + fn from_iter>>(iter: T) -> Self { + Self(iter.into_iter().collect()) + } +} + +impl From>> for TxtRecordData { + fn from(value: Vec>) -> Self { + Self(value.into_boxed_slice()) + } +} + +/// Helper enum to give a unified type to either of two iterators +enum Either { + Left(A), + Right(B), +} + +impl, B: Iterator> Iterator for Either { + type Item = T; + + fn next(&mut self) -> Option { + match self { + Either::Left(iter) => iter.next(), + Either::Right(iter) => iter.next(), + } } } @@ -590,4 +757,56 @@ pub(crate) mod tests { assert!(add_jitter(&delay) < Duration::from_millis(delay * 12 / 10)); } } + + #[tokio::test] + #[traced_test] + async fn custom_resolver() { + #[derive(Debug)] + struct MyResolver; + impl Resolver for MyResolver { + fn lookup_ipv4(&self, host: String) -> BoxFuture, DnsError>> { + Box::pin(async move { + let addr = if host == "foo.example" { + Ipv4Addr::new(1, 1, 1, 1) + } else { + return Err(NoResponseSnafu.build()); + }; + let iter: BoxIter = Box::new(vec![addr].into_iter()); + Ok(iter) + }) + } + + fn lookup_ipv6(&self, _host: String) -> BoxFuture, DnsError>> { + todo!() + } + + fn lookup_txt( + &self, + _host: String, + ) -> BoxFuture, DnsError>> { + todo!() + } + + fn clear_cache(&self) { + todo!() + } + + fn reset(&mut self) { + todo!() + } + } + + let resolver = DnsResolver::custom(MyResolver); + let mut iter = resolver + .lookup_ipv4("foo.example", Duration::from_secs(1)) + .await + .expect("not to fail"); + let addr = iter.next().expect("one result"); + assert_eq!(addr, "1.1.1.1".parse::().unwrap()); + + let res = resolver + .lookup_ipv4("bar.example", Duration::from_secs(1)) + .await; + assert!(matches!(res, Err(DnsError::NoResponse { .. }))) + } } diff --git a/iroh-relay/src/node_info.rs b/iroh-relay/src/node_info.rs index acaa52f089..116c8ce320 100644 --- a/iroh-relay/src/node_info.rs +++ b/iroh-relay/src/node_info.rs @@ -40,21 +40,11 @@ use std::{ str::{FromStr, Utf8Error}, }; -#[cfg(not(wasm_browser))] -use hickory_resolver::{Name, proto::ProtoError}; use iroh_base::{NodeAddr, NodeId, RelayUrl, SecretKey, SignatureError}; use nested_enum_utils::common_fields; use snafu::{Backtrace, ResultExt, Snafu}; -#[cfg(not(wasm_browser))] -use tracing::warn; use url::Url; -#[cfg(not(wasm_browser))] -use crate::{ - defaults::timeouts::DNS_TIMEOUT, - dns::{DnsError, DnsResolver}, -}; - /// The DNS name for the iroh TXT record. pub const IROH_TXT_NAME: &str = "_iroh"; @@ -389,9 +379,12 @@ impl NodeInfo { #[cfg(not(wasm_browser))] /// Parses a [`NodeInfo`] from a TXT records lookup. - pub fn from_txt_lookup(lookup: crate::dns::TxtLookup) -> Result { - let attrs = TxtAttrs::from_txt_lookup(lookup)?; - Ok(attrs.into()) + pub(crate) fn from_txt_lookup( + name: String, + lookup: impl Iterator, + ) -> Result { + let attrs = TxtAttrs::from_txt_lookup(name, lookup)?; + Ok(Self::from(attrs)) } /// Parses a [`NodeInfo`] from a [`pkarr::SignedPacket`]. @@ -417,32 +410,6 @@ impl NodeInfo { } } -#[cfg(not(wasm_browser))] -#[common_fields({ - backtrace: Option, - #[snafu(implicit)] - span_trace: n0_snafu::SpanTrace, -})] -#[allow(missing_docs)] -#[derive(Debug, Snafu)] -#[non_exhaustive] -#[snafu(visibility(pub(crate)))] -pub enum LookupError { - #[snafu(display("Malformed txt from lookup"))] - ParseError { - #[snafu(source(from(ParseError, Box::new)))] - source: Box, - }, - #[snafu(display("Failed to resolve TXT record"))] - LookupFailed { - #[snafu(source(from(DnsError, Box::new)))] - source: Box, - }, - #[cfg(not(wasm_browser))] - #[snafu(display("Name is not a valid TXT label"))] - InvalidLabel { source: ProtoError }, -} - #[common_fields({ backtrace: Option, #[snafu(implicit)] @@ -489,23 +456,17 @@ impl std::ops::DerefMut for NodeInfo { /// [`IROH_TXT_NAME`] and the second label to be a z32 encoded [`NodeId`]. Ignores /// subsequent labels. #[cfg(not(wasm_browser))] -fn node_id_from_hickory_name( - name: &hickory_resolver::proto::rr::Name, -) -> Result { - if name.num_labels() < 2 { - return Err(NumLabelsSnafu { - num_labels: name.num_labels(), - } - .build()); +fn node_id_from_txt_name(name: &str) -> Result { + let num_labels = name.split(".").count(); + if num_labels < 2 { + return Err(NumLabelsSnafu { num_labels }.build()); } - let mut labels = name.iter(); - let label = - std::str::from_utf8(labels.next().expect("num_labels checked")).context(Utf8Snafu)?; + let mut labels = name.split("."); + let label = labels.next().expect("checked above"); if label != IROH_TXT_NAME { return Err(NotAnIrohRecordSnafu { label }.build()); } - let label = - std::str::from_utf8(labels.next().expect("num_labels checked")).context(Utf8Snafu)?; + let label = labels.next().expect("checked above"); let node_id = NodeId::from_z32(label)?; Ok(node_id) } @@ -580,38 +541,6 @@ impl TxtAttrs { Ok(Self { attrs, node_id }) } - #[cfg(not(wasm_browser))] - async fn lookup(resolver: &DnsResolver, name: Name) -> Result { - let name = ensure_iroh_txt_label(name)?; - let lookup = resolver - .lookup_txt(name, DNS_TIMEOUT) - .await - .context(LookupFailedSnafu)?; - let attrs = Self::from_txt_lookup(lookup).context(ParseSnafu)?; - Ok(attrs) - } - - /// Looks up attributes by [`NodeId`] and origin domain. - #[cfg(not(wasm_browser))] - pub(crate) async fn lookup_by_id( - resolver: &DnsResolver, - node_id: &NodeId, - origin: &str, - ) -> Result { - let name = node_domain(node_id, origin)?; - TxtAttrs::lookup(resolver, name).await - } - - /// Looks up attributes by DNS name. - #[cfg(not(wasm_browser))] - pub(crate) async fn lookup_by_name( - resolver: &DnsResolver, - name: &str, - ) -> Result { - let name = Name::from_str(name).context(InvalidLabelSnafu)?; - TxtAttrs::lookup(resolver, name).await - } - /// Returns the parsed attributes. pub(crate) fn attrs(&self) -> &BTreeMap> { &self.attrs @@ -650,43 +579,13 @@ impl TxtAttrs { /// Parses a TXT records lookup. #[cfg(not(wasm_browser))] - pub(crate) fn from_txt_lookup(lookup: crate::dns::TxtLookup) -> Result { - let queried_node_id = node_id_from_hickory_name(lookup.0.query().name())?; - - let strings = lookup.0.as_lookup().record_iter().filter_map(|record| { - match node_id_from_hickory_name(record.name()) { - // Filter out only TXT record answers that match the node_id we searched for. - Ok(n) if n == queried_node_id => match record.data().as_txt() { - Some(txt) => Some(txt.to_string()), - None => { - warn!( - ?queried_node_id, - data = ?record.data(), - "unexpected record type for DNS discovery query" - ); - None - } - }, - Ok(answered_node_id) => { - warn!( - ?queried_node_id, - ?answered_node_id, - "unexpected node ID answered for DNS query" - ); - None - } - Err(e) => { - warn!( - ?queried_node_id, - name = ?record.name(), - err = ?e, - "unexpected answer record name for DNS query" - ); - None - } - } - }); + pub(crate) fn from_txt_lookup( + name: String, + lookup: impl Iterator, + ) -> Result { + let queried_node_id = node_id_from_txt_name(&name)?; + let strings = lookup.map(|record| record.to_string()); Self::from_strings(queried_node_id, strings) } @@ -720,19 +619,18 @@ impl TxtAttrs { } #[cfg(not(wasm_browser))] -fn ensure_iroh_txt_label(name: Name) -> Result { - if name.iter().next() == Some(IROH_TXT_NAME.as_bytes()) { - Ok(name) +pub(crate) fn ensure_iroh_txt_label(name: String) -> String { + let mut parts = name.split("."); + if parts.next() == Some(IROH_TXT_NAME) { + name } else { - Name::parse(IROH_TXT_NAME, Some(&name)).context(InvalidLabelSnafu) + format!("{}.{}", IROH_TXT_NAME, name) } } #[cfg(not(wasm_browser))] -fn node_domain(node_id: &NodeId, origin: &str) -> Result { - let domain = format!("{}.{origin}", NodeId::to_z32(node_id)); - let domain = Name::from_str(&domain).context(InvalidLabelSnafu)?; - Ok(domain) +pub(crate) fn node_domain(node_id: &NodeId, origin: &str) -> String { + format!("{}.{}", NodeId::to_z32(node_id), origin) } #[cfg(test)] @@ -753,6 +651,8 @@ mod tests { use iroh_base::{NodeId, SecretKey}; use n0_snafu::{Result, ResultExt}; + use crate::dns::TxtRecordData; + use super::{NodeData, NodeIdExt, NodeInfo}; #[test] @@ -846,8 +746,11 @@ mod tests { ]; let lookup = Lookup::new_with_max_ttl(query, Arc::new(records)); let lookup = hickory_resolver::lookup::TxtLookup::from(lookup); + let lookup = lookup + .into_iter() + .map(|txt| TxtRecordData::from_iter(txt.iter().cloned())); - let node_info = NodeInfo::from_txt_lookup(lookup.into())?; + let node_info = NodeInfo::from_txt_lookup(name.to_string(), lookup)?; let expected_node_info = NodeInfo::new(NodeId::from_str( "1992d53c02cdc04566e5c0edb1ce83305cd550297953a047a445ea3264b54b18", From 627e5189cd8cc996d526cce4c293db8b28cd63bc Mon Sep 17 00:00:00 2001 From: Frando Date: Mon, 29 Sep 2025 12:51:29 +0200 Subject: [PATCH 2/5] refactor: improve code structure --- iroh-relay/src/dns.rs | 236 ++++++++++++++++++++++-------------------- 1 file changed, 125 insertions(+), 111 deletions(-) diff --git a/iroh-relay/src/dns.rs b/iroh-relay/src/dns.rs index 9067cc7071..ae6d8aa7e0 100644 --- a/iroh-relay/src/dns.rs +++ b/iroh-relay/src/dns.rs @@ -142,11 +142,8 @@ impl DnsResolver { /// This does not work at least on some Androids, therefore we fallback /// to the default `ResolverConfig` which uses eg. to google's `8.8.8.8` or `8.8.4.4`. pub fn new() -> Self { - let resolver = Self::new_inner(); - Self(Arc::new(RwLock::new(DnsResolverInner::Hickory { - resolver, - nameserver: None, - }))) + let resolver = HickoryResolver::new(HickoryResolverOpts::SystemDefaults); + Self(Arc::new(RwLock::new(DnsResolverInner::Hickory(resolver)))) } /// Creates a new [`DnsResolver`] from a struct that implements [`Resolver`]. @@ -162,79 +159,27 @@ impl DnsResolver { ))))) } - fn new_inner() -> TokioResolver { - let (system_config, mut options) = - hickory_resolver::system_conf::read_system_conf().unwrap_or_default(); - - // Copy all of the system config, but strip the bad windows nameservers. Unfortunately - // there is no easy way to do this. - let mut config = hickory_resolver::config::ResolverConfig::new(); - if let Some(name) = system_config.domain() { - config.set_domain(name.clone()); - } - for name in system_config.search() { - config.add_search(name.clone()); - } - for nameserver_cfg in system_config.name_servers() { - if !WINDOWS_BAD_SITE_LOCAL_DNS_SERVERS.contains(&nameserver_cfg.socket_addr.ip()) { - config.add_name_server(nameserver_cfg.clone()); - } - } - - // see [`DnsResolver::lookup_ipv4_ipv6`] for info on why we avoid `LookupIpStrategy::Ipv4AndIpv6` - options.ip_strategy = hickory_resolver::config::LookupIpStrategy::Ipv4thenIpv6; - - let mut builder = - TokioResolver::builder_with_config(config, TokioConnectionProvider::default()); - *builder.options_mut() = options; - builder.build() - } - /// Creates a new DNS resolver configured with a single UDP DNS nameserver. pub fn with_nameserver(nameserver: SocketAddr) -> Self { - let resolver = Self::with_nameserver_inner(nameserver); - Self(Arc::new(RwLock::new(DnsResolverInner::Hickory { - resolver, - nameserver: None, - }))) - } - - fn with_nameserver_inner(nameserver: SocketAddr) -> TokioResolver { - let mut config = hickory_resolver::config::ResolverConfig::new(); - let nameserver_config = hickory_resolver::config::NameServerConfig::new( - nameserver, - hickory_resolver::proto::xfer::Protocol::Udp, - ); - config.add_name_server(nameserver_config); - - let builder = - TokioResolver::builder_with_config(config, TokioConnectionProvider::default()); - builder.build() + let resolver = HickoryResolver::new(HickoryResolverOpts::SingleUdpNameserver(nameserver)); + Self(Arc::new(RwLock::new(DnsResolverInner::Hickory(resolver)))) } /// Removes all entries from the cache. pub async fn clear_cache(&self) { let this = self.0.read().await; - this.clear_cache(); + match &*this { + DnsResolverInner::Hickory(resolver) => resolver.clear_cache(), + DnsResolverInner::Custom(resolver) => resolver.clear_cache(), + } } /// Recreates the inner resolver. pub async fn reset(&self) { let mut this = self.0.write().await; match &mut *this { - DnsResolverInner::Hickory { - resolver, - nameserver, - } => { - *resolver = if let Some(nameserver) = *nameserver { - Self::with_nameserver_inner(nameserver) - } else { - Self::new_inner() - }; - } - DnsResolverInner::Custom(resolver) => { - resolver.reset(); - } + DnsResolverInner::Hickory(resolver) => resolver.reset(), + DnsResolverInner::Custom(resolver) => resolver.reset(), } } @@ -487,32 +432,114 @@ impl reqwest::dns::Resolve for DnsResolver { /// default hickory resolver. #[derive(Debug)] enum DnsResolverInner { - Hickory { - resolver: hickory_resolver::TokioResolver, - nameserver: Option, - }, + Hickory(HickoryResolver), Custom(Box), } impl DnsResolverInner { - /// Looks up an IPv4 address. async fn lookup_ipv4( &self, host: String, ) -> Result + use<>, DnsError> { - match self { - DnsResolverInner::Hickory { resolver, .. } => { - let addrs = resolver - .ipv4_lookup(host) - .await? - .into_iter() - .map(Ipv4Addr::from); - Ok(Either::Left(addrs)) - } - DnsResolverInner::Custom(resolver) => { - Ok(Either::Right(resolver.lookup_ipv4(host).await?)) + Ok(match self { + Self::Hickory(resolver) => Either::Left(resolver.lookup_ipv4(host).await?), + Self::Custom(resolver) => Either::Right(resolver.lookup_ipv4(host).await?), + }) + } + + async fn lookup_ipv6( + &self, + host: String, + ) -> Result + use<>, DnsError> { + Ok(match self { + Self::Hickory(resolver) => Either::Left(resolver.lookup_ipv6(host).await?), + Self::Custom(resolver) => Either::Right(resolver.lookup_ipv6(host).await?), + }) + } + + async fn lookup_txt( + &self, + host: String, + ) -> Result + use<>, DnsError> { + Ok(match self { + Self::Hickory(resolver) => Either::Left(resolver.lookup_txt(host).await?), + Self::Custom(resolver) => Either::Right(resolver.lookup_txt(host).await?), + }) + } +} + +#[derive(Debug, Clone)] +enum HickoryResolverOpts { + SystemDefaults, + SingleUdpNameserver(SocketAddr), +} + +#[derive(Debug)] +struct HickoryResolver { + resolver: TokioResolver, + opts: HickoryResolverOpts, +} + +impl HickoryResolver { + fn new(opts: HickoryResolverOpts) -> Self { + let resolver = match opts { + HickoryResolverOpts::SystemDefaults => Self::with_system_defaults(), + HickoryResolverOpts::SingleUdpNameserver(addr) => Self::with_single_nameserver(addr), + }; + Self { resolver, opts } + } + + fn with_single_nameserver(nameserver: SocketAddr) -> TokioResolver { + let mut config = hickory_resolver::config::ResolverConfig::new(); + let nameserver_config = hickory_resolver::config::NameServerConfig::new( + nameserver, + hickory_resolver::proto::xfer::Protocol::Udp, + ); + config.add_name_server(nameserver_config); + + let builder = + TokioResolver::builder_with_config(config, TokioConnectionProvider::default()); + builder.build() + } + + fn with_system_defaults() -> TokioResolver { + let (system_config, mut options) = + hickory_resolver::system_conf::read_system_conf().unwrap_or_default(); + + // Copy all of the system config, but strip the bad windows nameservers. Unfortunately + // there is no easy way to do this. + let mut config = hickory_resolver::config::ResolverConfig::new(); + if let Some(name) = system_config.domain() { + config.set_domain(name.clone()); + } + for name in system_config.search() { + config.add_search(name.clone()); + } + for nameserver_cfg in system_config.name_servers() { + if !WINDOWS_BAD_SITE_LOCAL_DNS_SERVERS.contains(&nameserver_cfg.socket_addr.ip()) { + config.add_name_server(nameserver_cfg.clone()); } } + + // see [`DnsResolver::lookup_ipv4_ipv6`] for info on why we avoid `LookupIpStrategy::Ipv4AndIpv6` + options.ip_strategy = hickory_resolver::config::LookupIpStrategy::Ipv4thenIpv6; + + let mut builder = + TokioResolver::builder_with_config(config, TokioConnectionProvider::default()); + *builder.options_mut() = options; + builder.build() + } + + async fn lookup_ipv4( + &self, + host: String, + ) -> Result + use<>, DnsError> { + Ok(self + .resolver + .ipv4_lookup(host) + .await? + .into_iter() + .map(Ipv4Addr::from)) } /// Looks up an IPv6 address. @@ -520,19 +547,12 @@ impl DnsResolverInner { &self, host: String, ) -> Result + use<>, DnsError> { - match self { - DnsResolverInner::Hickory { resolver, .. } => { - let addrs = resolver - .ipv6_lookup(host) - .await? - .into_iter() - .map(Ipv6Addr::from); - Ok(Either::Left(addrs)) - } - DnsResolverInner::Custom(resolver) => { - Ok(Either::Right(resolver.lookup_ipv6(host).await?)) - } - } + Ok(self + .resolver + .ipv6_lookup(host) + .await? + .into_iter() + .map(Ipv6Addr::from)) } /// Looks up TXT records. @@ -540,27 +560,21 @@ impl DnsResolverInner { &self, host: String, ) -> Result + use<>, DnsError> { - match self { - DnsResolverInner::Hickory { resolver, .. } => { - let lookup = resolver - .txt_lookup(host) - .await? - .into_iter() - .map(|txt| TxtRecordData::from_iter(txt.iter().cloned())); - Ok(Either::Left(lookup)) - } - DnsResolverInner::Custom(resolver) => { - Ok(Either::Right(resolver.lookup_txt(host).await?)) - } - } + Ok(self + .resolver + .txt_lookup(host) + .await? + .into_iter() + .map(|txt| TxtRecordData::from_iter(txt.iter().cloned()))) } /// Clears the internal cache. fn clear_cache(&self) { - match self { - DnsResolverInner::Hickory { resolver, .. } => resolver.clear_cache(), - DnsResolverInner::Custom(resolver) => resolver.clear_cache(), - } + self.resolver.clear_cache() + } + + fn reset(&mut self) { + *self = Self::new(self.opts.clone()); } } From 0c4346d505e040c0dacb974492e50ada1ee2d919 Mon Sep 17 00:00:00 2001 From: Frando Date: Mon, 29 Sep 2025 12:52:32 +0200 Subject: [PATCH 3/5] chore: fmt --- iroh-relay/src/node_info.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/iroh-relay/src/node_info.rs b/iroh-relay/src/node_info.rs index 116c8ce320..3017350095 100644 --- a/iroh-relay/src/node_info.rs +++ b/iroh-relay/src/node_info.rs @@ -651,9 +651,8 @@ mod tests { use iroh_base::{NodeId, SecretKey}; use n0_snafu::{Result, ResultExt}; - use crate::dns::TxtRecordData; - use super::{NodeData, NodeIdExt, NodeInfo}; + use crate::dns::TxtRecordData; #[test] fn txt_attr_roundtrip() { From 2b8fe478173176e4c6ba725560c0216217841195 Mon Sep 17 00:00:00 2001 From: Frando Date: Mon, 29 Sep 2025 13:00:19 +0200 Subject: [PATCH 4/5] refactor: move Arc into enum to avoid double boxing --- iroh-relay/src/dns.rs | 67 ++++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/iroh-relay/src/dns.rs b/iroh-relay/src/dns.rs index ae6d8aa7e0..e0c2ceb8c1 100644 --- a/iroh-relay/src/dns.rs +++ b/iroh-relay/src/dns.rs @@ -133,7 +133,7 @@ impl StaggeredError { /// The DNS resolver used throughout `iroh`. #[derive(Debug, Clone)] -pub struct DnsResolver(Arc>); +pub struct DnsResolver(DnsResolverInner); impl DnsResolver { /// Creates a new DNS resolver with sensible cross-platform defaults. @@ -143,7 +143,7 @@ impl DnsResolver { /// to the default `ResolverConfig` which uses eg. to google's `8.8.8.8` or `8.8.4.4`. pub fn new() -> Self { let resolver = HickoryResolver::new(HickoryResolverOpts::SystemDefaults); - Self(Arc::new(RwLock::new(DnsResolverInner::Hickory(resolver)))) + Self(DnsResolverInner::Hickory(Arc::new(RwLock::new(resolver)))) } /// Creates a new [`DnsResolver`] from a struct that implements [`Resolver`]. @@ -154,33 +154,23 @@ impl DnsResolver { /// To use a different DNS resolver, you need to implement [`Resolver`] for your custom resolver /// and then pass to this function. pub fn custom(resolver: impl Resolver) -> Self { - Self(Arc::new(RwLock::new(DnsResolverInner::Custom(Box::new( - resolver, - ))))) + Self(DnsResolverInner::Custom(Arc::new(RwLock::new(resolver)))) } /// Creates a new DNS resolver configured with a single UDP DNS nameserver. pub fn with_nameserver(nameserver: SocketAddr) -> Self { let resolver = HickoryResolver::new(HickoryResolverOpts::SingleUdpNameserver(nameserver)); - Self(Arc::new(RwLock::new(DnsResolverInner::Hickory(resolver)))) + Self(DnsResolverInner::Hickory(Arc::new(RwLock::new(resolver)))) } /// Removes all entries from the cache. pub async fn clear_cache(&self) { - let this = self.0.read().await; - match &*this { - DnsResolverInner::Hickory(resolver) => resolver.clear_cache(), - DnsResolverInner::Custom(resolver) => resolver.clear_cache(), - } + self.0.clear_cache().await } /// Recreates the inner resolver. pub async fn reset(&self) { - let mut this = self.0.write().await; - match &mut *this { - DnsResolverInner::Hickory(resolver) => resolver.reset(), - DnsResolverInner::Custom(resolver) => resolver.reset(), - } + self.0.reset().await } /// Looks up a TXT record. @@ -190,8 +180,7 @@ impl DnsResolver { timeout: Duration, ) -> Result, DnsError> { let host = host.to_string(); - let this = self.0.read().await; - let res = time::timeout(timeout, this.lookup_txt(host)).await??; + let res = time::timeout(timeout, self.0.lookup_txt(host)).await??; Ok(res) } @@ -202,9 +191,8 @@ impl DnsResolver { timeout: Duration, ) -> Result + use, DnsError> { let host = host.to_string(); - let this = self.0.read().await; - let addrs = time::timeout(timeout, this.lookup_ipv4(host)).await??; - Ok(addrs.into_iter().map(|ip| IpAddr::V4(ip))) + let addrs = time::timeout(timeout, self.0.lookup_ipv4(host)).await??; + Ok(addrs.into_iter().map(IpAddr::V4)) } /// Performs an IPv6 lookup with a timeout. @@ -214,9 +202,8 @@ impl DnsResolver { timeout: Duration, ) -> Result + use, DnsError> { let host = host.to_string(); - let this = self.0.read().await; - let addrs = time::timeout(timeout, this.lookup_ipv6(host)).await??; - Ok(addrs.into_iter().map(|ip| IpAddr::V6(ip))) + let addrs = time::timeout(timeout, self.0.lookup_ipv6(host)).await??; + Ok(addrs.into_iter().map(IpAddr::V6)) } /// Resolves IPv4 and IPv6 in parallel with a timeout. @@ -430,10 +417,10 @@ impl reqwest::dns::Resolve for DnsResolver { /// /// We do this to save the cost of boxing the futures and iterators when using /// default hickory resolver. -#[derive(Debug)] +#[derive(Debug, Clone)] enum DnsResolverInner { - Hickory(HickoryResolver), - Custom(Box), + Hickory(Arc>), + Custom(Arc>), } impl DnsResolverInner { @@ -442,8 +429,8 @@ impl DnsResolverInner { host: String, ) -> Result + use<>, DnsError> { Ok(match self { - Self::Hickory(resolver) => Either::Left(resolver.lookup_ipv4(host).await?), - Self::Custom(resolver) => Either::Right(resolver.lookup_ipv4(host).await?), + Self::Hickory(resolver) => Either::Left(resolver.read().await.lookup_ipv4(host).await?), + Self::Custom(resolver) => Either::Right(resolver.read().await.lookup_ipv4(host).await?), }) } @@ -452,8 +439,8 @@ impl DnsResolverInner { host: String, ) -> Result + use<>, DnsError> { Ok(match self { - Self::Hickory(resolver) => Either::Left(resolver.lookup_ipv6(host).await?), - Self::Custom(resolver) => Either::Right(resolver.lookup_ipv6(host).await?), + Self::Hickory(resolver) => Either::Left(resolver.read().await.lookup_ipv6(host).await?), + Self::Custom(resolver) => Either::Right(resolver.read().await.lookup_ipv6(host).await?), }) } @@ -462,10 +449,24 @@ impl DnsResolverInner { host: String, ) -> Result + use<>, DnsError> { Ok(match self { - Self::Hickory(resolver) => Either::Left(resolver.lookup_txt(host).await?), - Self::Custom(resolver) => Either::Right(resolver.lookup_txt(host).await?), + Self::Hickory(resolver) => Either::Left(resolver.read().await.lookup_txt(host).await?), + Self::Custom(resolver) => Either::Right(resolver.read().await.lookup_txt(host).await?), }) } + + async fn clear_cache(&self) { + match self { + Self::Hickory(resolver) => resolver.read().await.clear_cache(), + Self::Custom(resolver) => resolver.read().await.clear_cache(), + } + } + + async fn reset(&self) { + match self { + Self::Hickory(resolver) => resolver.write().await.reset(), + Self::Custom(resolver) => resolver.write().await.reset(), + } + } } #[derive(Debug, Clone)] From 695b530afbfe471eb75a369ce33ae1fd5cf5bf7a Mon Sep 17 00:00:00 2001 From: Frando Date: Tue, 30 Sep 2025 09:46:50 +0200 Subject: [PATCH 5/5] fixup: keep NodeInfo::from_txt_lookup public --- iroh-relay/src/node_info.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/iroh-relay/src/node_info.rs b/iroh-relay/src/node_info.rs index 3017350095..f81a175856 100644 --- a/iroh-relay/src/node_info.rs +++ b/iroh-relay/src/node_info.rs @@ -378,12 +378,12 @@ impl NodeInfo { } #[cfg(not(wasm_browser))] - /// Parses a [`NodeInfo`] from a TXT records lookup. - pub(crate) fn from_txt_lookup( - name: String, + /// Parses a [`NodeInfo`] from DNS TXT lookup. + pub fn from_txt_lookup( + domain_name: String, lookup: impl Iterator, ) -> Result { - let attrs = TxtAttrs::from_txt_lookup(name, lookup)?; + let attrs = TxtAttrs::from_txt_lookup(domain_name, lookup)?; Ok(Self::from(attrs)) }