mirror of
https://github.com/outbackdingo/firezone.git
synced 2026-01-27 10:18:54 +00:00
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>
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -1006,18 +1006,25 @@ impl ClientState {
|
||||
})
|
||||
}
|
||||
|
||||
pub fn update_system_resolvers(&mut self, new_dns: Vec<IpAddr>) {
|
||||
/// 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<IpAddr>) -> Vec<IpAddr> {
|
||||
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) {
|
||||
|
||||
@@ -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<IpAddr>) -> 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<IpAddr>) -> 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<IpAddr> {
|
||||
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<IpAddr>,
|
||||
default_resolvers: Vec<IpAddr>,
|
||||
) -> Vec<SocketAddr> {
|
||||
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<IpAddr>) -> DnsMapping {
|
||||
@@ -170,6 +176,10 @@ fn sentinel_dns_mapping(dns: &[SocketAddr], old_sentinels: Vec<IpAddr>) -> DnsMa
|
||||
DnsMapping { inner: mapping }
|
||||
}
|
||||
|
||||
fn without_sentinel_ips(servers: &[IpAddr]) -> Vec<IpAddr> {
|
||||
servers.iter().copied().filter_map(not_sentinel).collect()
|
||||
}
|
||||
|
||||
fn not_sentinel(srv: IpAddr) -> Option<IpAddr> {
|
||||
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<IpAddr> {
|
||||
|
||||
#[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::<IpAddr>::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<IpNetwork> {
|
||||
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<SocketAddr> {
|
||||
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()
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user