From 95fdb7f62a252d4092bcf98a991f1aa22cd01e53 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Sat, 15 Nov 2025 16:47:43 +1100 Subject: [PATCH] fix(connlib): sanitize resolvers before re-resolving portal URL (#10880) In #10817, connlib gained the ability to re-resolve the portal's hostname on WebSocket connection hiccups. The list of upstream servers used for that may contain sentinel DNS server IPs on certain systems if connlib's DNS control is currently active. Connlib filters these servers internally before computing the effective list of upstream servers. The DNS client used by the event-loop contacts all servers in the list but waits for at most 2s before merging all received records together. If there are upstream DNS servers defined in the portal and those are also resources which we are currently not connected to, querying these servers would trigger a message to the portal, forming a circular dependency. This circular dependency is only broken by the 2s timeout. Whilst not fatal for connlib's functionality, it means that in such a situation, reconnecting to the portal always has to wait for this timeout. To fix this, we first apply the system DNS resolvers to connlib and only pass the now returned sanitized list on to the DNS client. Related: #10854 --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: thomaseizinger <5486389+thomaseizinger@users.noreply.github.com> --- rust/client-shared/src/eventloop.rs | 6 +- rust/connlib/tunnel/src/client.rs | 15 +- rust/connlib/tunnel/src/client/dns_config.rs | 138 ++++++++++++------- 3 files changed, 103 insertions(+), 56 deletions(-) diff --git a/rust/client-shared/src/eventloop.rs b/rust/client-shared/src/eventloop.rs index e4432ca51..3d0379bdb 100644 --- a/rust/client-shared/src/eventloop.rs +++ b/rust/client-shared/src/eventloop.rs @@ -200,12 +200,12 @@ impl Eventloop { return Ok(ControlFlow::Continue(())); }; + let dns = tunnel.state_mut().update_system_resolvers(dns); + self.portal_cmd_tx - .send(PortalCommand::UpdateDnsServers(dns.clone())) + .send(PortalCommand::UpdateDnsServers(dns)) .await .context("Failed to send message to portal")?; - - tunnel.state_mut().update_system_resolvers(dns); } Command::SetInternetResourceState(active) => { let Some(tunnel) = self.tunnel.as_mut() else { diff --git a/rust/connlib/tunnel/src/client.rs b/rust/connlib/tunnel/src/client.rs index a6d987a12..3c51ed798 100644 --- a/rust/connlib/tunnel/src/client.rs +++ b/rust/connlib/tunnel/src/client.rs @@ -1006,18 +1006,25 @@ impl ClientState { }) } - pub fn update_system_resolvers(&mut self, new_dns: Vec) { + /// Update our list of known system DNS resolvers. + /// + /// Returns back the list of resolvers, sanitized from all unusable servers, + /// i.e. all servers within the sentinel DNS range. + /// + /// Note: The returned list is not necessarily the list of DNS resolvers that is active. + /// If DNS servers are defined in the portal, those will be preferred over the system defined ones. + pub fn update_system_resolvers(&mut self, new_dns: Vec) -> Vec { let changed = self.dns_config.update_system_resolvers(new_dns); if !changed { - return; + return self.dns_config.system_dns_resolvers(); } self.dns_cache.flush("DNS servers changed"); let Some(config) = self.tun_config.clone() else { tracing::debug!("Unable to update DNS servers without interface configuration"); - return; + return self.dns_config.system_dns_resolvers(); }; let dns_by_sentinel = self.dns_config.mapping(); @@ -1026,6 +1033,8 @@ impl ClientState { dns_by_sentinel, ..config }); + + self.dns_config.system_dns_resolvers() } pub fn update_interface_config(&mut self, config: InterfaceConfig) { diff --git a/rust/connlib/tunnel/src/client/dns_config.rs b/rust/connlib/tunnel/src/client/dns_config.rs index 209a3e153..bfde03cae 100644 --- a/rust/connlib/tunnel/src/client/dns_config.rs +++ b/rust/connlib/tunnel/src/client/dns_config.rs @@ -70,18 +70,26 @@ impl DnsMapping { impl DnsConfig { #[must_use = "Check if the DNS mapping has changed"] pub(crate) fn update_system_resolvers(&mut self, servers: Vec) -> bool { - tracing::debug!(?servers, "Received system-defined DNS servers"); + let sanitized = without_sentinel_ips(&servers); - self.system_resolvers = servers; + tracing::debug!(?servers, ?sanitized, "Received system-defined DNS servers"); + + self.system_resolvers = sanitized; self.update_dns_mapping() } #[must_use = "Check if the DNS mapping has changed"] pub(crate) fn update_upstream_do53_resolvers(&mut self, servers: Vec) -> bool { - tracing::debug!(?servers, "Received upstream-defined Do53 servers"); + let sanitized = without_sentinel_ips(&servers); - self.upstream_do53 = servers; + tracing::debug!( + ?servers, + ?sanitized, + "Received upstream-defined DNS servers" + ); + + self.upstream_do53 = sanitized; self.update_dns_mapping() } @@ -103,6 +111,10 @@ impl DnsConfig { self.mapping.clone() } + pub(crate) fn system_dns_resolvers(&self) -> Vec { + self.system_resolvers.clone() + } + fn update_dns_mapping(&mut self) -> bool { let effective_dns_servers = effective_dns_servers(self.upstream_do53.clone(), self.system_resolvers.clone()); @@ -125,30 +137,24 @@ fn effective_dns_servers( upstream_do53: Vec, default_resolvers: Vec, ) -> Vec { - let mut upstream_dns = upstream_do53 - .into_iter() - .filter_map(not_sentinel) - .peekable(); - if upstream_dns.peek().is_some() { - return upstream_dns + if !upstream_do53.is_empty() { + return upstream_do53 + .into_iter() .map(|ip| SocketAddr::new(ip, DNS_PORT)) .collect(); } - let mut dns_servers = default_resolvers - .into_iter() - .filter_map(not_sentinel) - .map(|ip| SocketAddr::new(ip, DNS_PORT)) - .peekable(); - - if dns_servers.peek().is_none() { + if default_resolvers.is_empty() { tracing::info!( "No system default DNS servers available! Can't initialize resolver. DNS resources won't work." ); return Vec::new(); } - dns_servers.collect() + default_resolvers + .into_iter() + .map(|ip| SocketAddr::new(ip, DNS_PORT)) + .collect() } fn sentinel_dns_mapping(dns: &[SocketAddr], old_sentinels: Vec) -> DnsMapping { @@ -170,6 +176,10 @@ fn sentinel_dns_mapping(dns: &[SocketAddr], old_sentinels: Vec) -> DnsMa DnsMapping { inner: mapping } } +fn without_sentinel_ips(servers: &[IpAddr]) -> Vec { + servers.iter().copied().filter_map(not_sentinel).collect() +} + fn not_sentinel(srv: IpAddr) -> Option { let is_v4_dns = IpNetwork::V4(DNS_SENTINELS_V4).contains(srv); let is_v6_dns = IpNetwork::V6(DNS_SENTINELS_V6).contains(srv); @@ -179,52 +189,80 @@ fn not_sentinel(srv: IpAddr) -> Option { #[cfg(test)] mod tests { - use std::collections::HashSet; - use super::*; #[test] - fn sentinel_dns_works() { - let servers = dns_list(); - let sentinel_dns = sentinel_dns_mapping(&servers, vec![]); + fn maps_system_resolvers_to_sentinel_ips() { + let mut config = DnsConfig::default(); - for server in servers { - assert!( - sentinel_dns - .sentinel_by_upstream(server) - .is_some_and(|ip| sentinel_ranges().iter().any(|e| e.contains(ip))) - ) - } + let changed = config.update_system_resolvers(vec![ + ip("1.1.1.1"), + ip("1.0.0.1"), + ip("2606:4700:4700::1111"), + ]); + assert!(changed); + + assert_eq!(config.mapping().sentinel_ips().len(), 3); + assert_eq!( + config.mapping().upstream_sockets(), + vec![ + socket("1.1.1.1:53"), + socket("1.0.0.1:53"), + socket("[2606:4700:4700::1111]:53"), + ] + ); } #[test] - fn sentinel_dns_excludes_old_ones() { - let servers = dns_list(); - let sentinel_dns_old = sentinel_dns_mapping(&servers, vec![]); - let sentinel_dns_new = sentinel_dns_mapping(&servers, sentinel_dns_old.sentinel_ips()); + fn prefers_upstream_do53_over_system_resolvers() { + let mut config = DnsConfig::default(); - assert!( - HashSet::::from_iter(sentinel_dns_old.sentinel_ips()) - .is_disjoint(&HashSet::from_iter(sentinel_dns_new.sentinel_ips())) - ) + let changed = config.update_system_resolvers(vec![ip("1.1.1.1")]); + assert!(changed); + let changed = config.update_upstream_do53_resolvers(vec![ip("1.0.0.1")]); + assert!(changed); + + assert_eq!(config.mapping().sentinel_ips().len(), 1); + assert_eq!( + config.mapping().upstream_sockets(), + vec![socket("1.0.0.1:53"),] + ); } - fn sentinel_ranges() -> Vec { - vec![ - IpNetwork::V4(DNS_SENTINELS_V4), - IpNetwork::V6(DNS_SENTINELS_V6), - ] + #[test] + fn filters_sentinel_ips_from_system() { + let mut config = DnsConfig::default(); + + let changed = config.update_system_resolvers(vec![ip("1.1.1.1"), ip("100.100.111.1")]); + assert!(changed); + + assert_eq!(config.mapping().sentinel_ips().len(), 1); + assert_eq!( + config.mapping().upstream_sockets(), + vec![socket("1.1.1.1:53"),] + ); } - fn dns_list() -> Vec { - vec![ - dns("1.1.1.1:53"), - dns("1.0.0.1:53"), - dns("[2606:4700:4700::1111]:53"), - ] + #[test] + fn filters_sentinel_ips_from_upstream() { + let mut config = DnsConfig::default(); + + let changed = + config.update_upstream_do53_resolvers(vec![ip("1.1.1.1"), ip("100.100.111.1")]); + assert!(changed); + + assert_eq!(config.mapping().sentinel_ips().len(), 1); + assert_eq!( + config.mapping().upstream_sockets(), + vec![socket("1.1.1.1:53"),] + ); } - fn dns(address: &str) -> SocketAddr { + fn ip(address: &str) -> IpAddr { address.parse().unwrap() } + + fn socket(socket: &str) -> SocketAddr { + socket.parse().unwrap() + } }