Skip to content

Commit

Permalink
fix(iroh-net): race ipv4 and ipv6 dns resolution (n0-computer#2026)
Browse files Browse the repository at this point in the history
This now races the resolution, with a fixed timeout on each, returning
as many results as found during that time.

Ref n0-computer#2006
  • Loading branch information
dignifiedquire authored and fubuloubu committed Feb 21, 2024
1 parent 8dc5a3e commit 77bd0cf
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 10 deletions.
9 changes: 4 additions & 5 deletions iroh-net/src/derp/http/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use crate::derp::{
client::ClientReceiver as DerpClientReceiver, metrics::Metrics, server::PacketForwarderHandler,
MeshKey, PacketForwarder, ReceivedMessage,
};
use crate::dns::DNS_RESOLVER;
use crate::dns::lookup_ipv4_ipv6;
use crate::key::{PublicKey, SecretKey};
use crate::util::AbortingJoinHandle;

Expand Down Expand Up @@ -104,7 +104,7 @@ pub enum ClientError {
InvalidUrl(String),
/// There was an error with DNS resolution
#[error("dns: {0:?}")]
Dns(Option<trust_dns_resolver::error::ResolveError>),
Dns(Option<anyhow::Error>),
/// There was a timeout resolving DNS.
#[error("dns timeout")]
DnsTimeout,
Expand Down Expand Up @@ -1019,14 +1019,13 @@ async fn resolve_host(url: &Url, prefer_ipv6: bool) -> Result<IpAddr, ClientErro
match host {
url::Host::Domain(domain) => {
// Need to do a DNS lookup
let addrs = tokio::time::timeout(DNS_TIMEOUT, DNS_RESOLVER.lookup_ip(domain))
let addrs = lookup_ipv4_ipv6(domain, DNS_TIMEOUT)
.await
.map_err(|_| ClientError::DnsTimeout)?
.map_err(|e| ClientError::Dns(Some(e)))?;

if prefer_ipv6 {
if let Some(addr) = addrs.iter().find(|addr| addr.is_ipv6()) {
return Ok(addr);
return Ok(*addr);
}
}
addrs
Expand Down
73 changes: 70 additions & 3 deletions iroh-net/src/dns.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use std::net::IpAddr;
use std::time::Duration;

use anyhow::Result;
use once_cell::sync::Lazy;
use trust_dns_resolver::{AsyncResolver, TokioAsyncResolver};
use trust_dns_resolver::{AsyncResolver, IntoName, TokioAsyncResolver, TryParseIp};

pub static DNS_RESOLVER: Lazy<TokioAsyncResolver> =
Lazy::new(|| get_resolver().expect("unable to create DNS resolver"));
Expand All @@ -14,23 +17,87 @@ fn get_resolver() -> Result<TokioAsyncResolver> {
let (config, mut options) =
trust_dns_resolver::system_conf::read_system_conf().unwrap_or_default();
// lookup IPv4 and IPv6 in parallel
options.ip_strategy = trust_dns_resolver::config::LookupIpStrategy::Ipv4AndIpv6;
options.ip_strategy = trust_dns_resolver::config::LookupIpStrategy::Ipv4thenIpv6;

let resolver = AsyncResolver::tokio(config, options);
Ok(resolver)
}

/// Resolve IPv4 and IPv6 in parallel.
///
/// `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
/// individually.
pub(crate) async fn lookup_ipv4_ipv6<N: IntoName + TryParseIp + Clone>(
host: N,
timeout: Duration,
) -> Result<Vec<IpAddr>> {
let ipv4 = DNS_RESOLVER.ipv4_lookup(host.clone());
let ipv6 = DNS_RESOLVER.ipv6_lookup(host);
let ipv4 = tokio::time::timeout(timeout, ipv4);
let ipv6 = tokio::time::timeout(timeout, ipv6);

let res = futures::future::join(ipv4, ipv6).await;
match res {
(Ok(Ok(ipv4)), Ok(Ok(ipv6))) => {
let res = ipv4
.into_iter()
.map(|ip| IpAddr::V4(ip.0))
.chain(ipv6.into_iter().map(|ip| IpAddr::V6(ip.0)))
.collect();
Ok(res)
}
(Ok(Ok(ipv4)), Err(_timeout)) => {
let res = ipv4.into_iter().map(|ip| IpAddr::V4(ip.0)).collect();
Ok(res)
}
(Ok(Ok(ipv4)), Ok(Err(_err))) => {
let res = ipv4.into_iter().map(|ip| IpAddr::V4(ip.0)).collect();
Ok(res)
}
(Ok(Err(_err)), Ok(Ok(ipv6))) => {
let res = ipv6.into_iter().map(|ip| IpAddr::V6(ip.0)).collect();
Ok(res)
}
(Ok(Err(err1)), Ok(Err(err2))) => {
anyhow::bail!("Ipv4: {:?}, Ipv6: {:?}", err1, err2);
}
(Ok(Err(err1)), Err(err2)) => {
anyhow::bail!("Ipv4: {:?}, Ipv6: {:?}", err1, err2);
}
(Err(_timeout), Ok(Ok(ipv6))) => {
let res = ipv6.into_iter().map(|ip| IpAddr::V6(ip.0)).collect();
Ok(res)
}
(Err(err1), Ok(Err(err2))) => {
anyhow::bail!("Ipv4: {:?}, Ipv6: {:?}", err1, err2);
}
(Err(timeout1), Err(timeout2)) => {
anyhow::bail!("Ipv4: {:?}, Ipv6: {:?}", timeout1, timeout2);
}
}
}

#[cfg(test)]
mod tests {
use crate::defaults::NA_DERP_HOSTNAME;

use super::*;

#[tokio::test]
async fn test_dns_lookup() {
async fn test_dns_lookup_basic() {
let res = DNS_RESOLVER.lookup_ip(NA_DERP_HOSTNAME).await.unwrap();
let res: Vec<_> = res.iter().collect();
assert!(!res.is_empty());
dbg!(res);
}

#[tokio::test]
async fn test_dns_lookup_ipv4_ipv6() {
let res = lookup_ipv4_ipv6(NA_DERP_HOSTNAME, Duration::from_secs(1))
.await
.unwrap();
assert!(!res.is_empty());
dbg!(res);
}
}
6 changes: 4 additions & 2 deletions iroh-net/src/netcheck/reportgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use tracing::{debug, debug_span, error, info_span, instrument, trace, warn, Inst
use super::NetcheckMetrics;
use crate::defaults::DEFAULT_DERP_STUN_PORT;
use crate::derp::{DerpMap, DerpNode, DerpUrl};
use crate::dns::DNS_RESOLVER;
use crate::dns::lookup_ipv4_ipv6;
use crate::net::interfaces;
use crate::net::ip;
use crate::net::UdpSocket;
Expand Down Expand Up @@ -65,6 +65,8 @@ const CAPTIVE_PORTAL_TIMEOUT: Duration = Duration::from_secs(2);

const ENOUGH_NODES: usize = 3;

const DNS_TIMEOUT: Duration = Duration::from_secs(1);

/// Holds the state for a single invocation of [`netcheck::Client::get_report`].
///
/// Dropping this will cancel the actor and stop the report generation.
Expand Down Expand Up @@ -958,7 +960,7 @@ async fn get_derp_addr(n: &DerpNode, proto: ProbeProto) -> Result<SocketAddr> {
async move {
debug!(?proto, %hostname, "Performing DNS lookup for derp addr");

if let Ok(addrs) = DNS_RESOLVER.lookup_ip(hostname).await {
if let Ok(addrs) = lookup_ipv4_ipv6(hostname, DNS_TIMEOUT).await {
for addr in addrs {
let addr = ip::to_canonical(addr);
if addr.is_ipv4()
Expand Down

0 comments on commit 77bd0cf

Please sign in to comment.