Refactor tunnel state machine DNS configuration

mullvad-daemon/src/dns.rs

@@ -1,5 +1,6 @@
 use mullvad_types::settings::{DnsOptions, DnsState};
 use std::net::{IpAddr, Ipv4Addr};
+use talpid_core::{dns::DnsConfig, firewall::is_local_address};
 
 /// When we want to block certain contents with the help of DNS server side,
 /// we compute the resolver IP to use based on these constants. The last
@@ -12,9 +13,8 @@ const DNS_ADULT_BLOCKING_IP_BIT: u8 = 1 << 3; // 0b00001000
 const DNS_GAMBLING_BLOCKING_IP_BIT: u8 = 1 << 4; // 0b00010000
 const DNS_SOCIAL_MEDIA_BLOCKING_IP_BIT: u8 = 1 << 5; // 0b00100000
 
-/// Return the resolvers as a vector of `IpAddr`s. Returns `None` when no special resolvers
-/// are requested and the tunnel default gateway should be used.
-pub fn addresses_from_options(options: &DnsOptions) -> Option<Vec<IpAddr>> {
+/// Return the DNS resolvers to use
+pub fn addresses_from_options(options: &DnsOptions) -> DnsConfig {
     match options.state {
         DnsState::Default => {
             // Check if we should use a custom blocking DNS resolver.
@@ -43,17 +43,74 @@ pub fn addresses_from_options(options: &DnsOptions) -> Option<Vec<IpAddr>> {
             if last_byte != 0 {
                 let mut dns_ip = DNS_BLOCKING_IP_BASE.octets();
                 dns_ip[dns_ip.len() - 1] |= last_byte;
-                Some(vec![IpAddr::V4(Ipv4Addr::from(dns_ip))])
+                DnsConfig::from_addresses(&[IpAddr::V4(Ipv4Addr::from(dns_ip))], &[])
             } else {
-                None
+                DnsConfig::default()
             }
         }
+        DnsState::Custom if options.custom_options.addresses.is_empty() => DnsConfig::default(),
         DnsState::Custom => {
-            if options.custom_options.addresses.is_empty() {
-                None
-            } else {
-                Some(options.custom_options.addresses.clone())
-            }
+            let (non_tunnel_config, tunnel_config): (Vec<_>, Vec<_>) = options
+                .custom_options
+                .addresses
+                .iter()
+                // Private IP ranges should not be tunneled
+                .partition(|&addr| is_local_address(addr));
+            DnsConfig::from_addresses(&tunnel_config, &non_tunnel_config)
         }
     }
 }
+
+#[cfg(test)]
+mod test {
+    use crate::dns::addresses_from_options;
+    use mullvad_types::settings::{CustomDnsOptions, DefaultDnsOptions, DnsOptions, DnsState};
+    use talpid_core::dns::DnsConfig;
+
+    #[test]
+    fn test_default_dns() {
+        let public_cfg = DnsOptions {
+            state: DnsState::Default,
+            custom_options: CustomDnsOptions::default(),
+            default_options: DefaultDnsOptions::default(),
+        };
+
+        assert_eq!(addresses_from_options(&public_cfg), DnsConfig::default());
+    }
+
+    #[test]
+    fn test_content_blockers() {
+        let public_cfg = DnsOptions {
+            state: DnsState::Default,
+            custom_options: CustomDnsOptions::default(),
+            default_options: DefaultDnsOptions {
+                block_ads: true,
+                ..DefaultDnsOptions::default()
+            },
+        };
+
+        assert_eq!(
+            addresses_from_options(&public_cfg),
+            DnsConfig::from_addresses(&["100.64.0.1".parse().unwrap()], &[],)
+        );
+    }
+
+    // Public IPs should be tunneled, but most private IPs should not be
+    #[test]
+    fn test_custom_dns() {
+        let public_ip = "1.2.3.4".parse().unwrap();
+        let private_ip = "172.16.10.1".parse().unwrap();
+        let public_cfg = DnsOptions {
+            state: DnsState::Custom,
+            custom_options: CustomDnsOptions {
+                addresses: vec![public_ip, private_ip],
+            },
+            default_options: DefaultDnsOptions::default(),
+        };
+
+        assert_eq!(
+            addresses_from_options(&public_cfg),
+            DnsConfig::from_addresses(&[public_ip], &[private_ip],)
+        );
+    }
+}

mullvad-daemon/src/lib.rs

@@ -758,7 +758,7 @@ impl Daemon {
             tunnel_state_machine::InitialTunnelState {
                 allow_lan: settings.allow_lan,
                 block_when_disconnected: settings.block_when_disconnected,
-                dns_servers: dns::addresses_from_options(&settings.tunnel_options.dns_options),
+                dns_config: dns::addresses_from_options(&settings.tunnel_options.dns_options),
                 allowed_endpoint: access_mode_handler
                     .get_current()
                     .await

talpid-core/src/dns/android.rs

@@ -1,4 +1,4 @@
-use std::net::IpAddr;
+use crate::dns::ResolvedDnsConfig;
 
 /// Stub error type for DNS errors on Android.
 #[derive(Debug, thiserror::Error)]
@@ -14,7 +14,7 @@ impl super::DnsMonitorT for DnsMonitor {
         Ok(DnsMonitor)
     }
 
-    fn set(&mut self, _interface: &str, _servers: &[IpAddr]) -> Result<(), Self::Error> {
+    fn set(&mut self, _interface: &str, _servers: ResolvedDnsConfig) -> Result<(), Self::Error> {
         Ok(())
     }
 

talpid-core/src/dns/linux/mod.rs

@@ -10,6 +10,8 @@ use self::{
 use std::{env, fmt, net::IpAddr};
 use talpid_routing::RouteManagerHandle;
 
+use super::ResolvedDnsConfig;
+
 pub type Result<T> = std::result::Result<T, Error>;
 
 /// Errors that can happen in the Linux DNS monitor
@@ -53,7 +55,8 @@ impl super::DnsMonitorT for DnsMonitor {
         })
     }
 
-    fn set(&mut self, interface: &str, servers: &[IpAddr]) -> Result<()> {
+    fn set(&mut self, interface: &str, config: ResolvedDnsConfig) -> Result<()> {
+        let servers = config.tunnel_config();
         self.reset()?;
         // Creating a new DNS monitor for each set, in case the system changed how it manages DNS.
         let mut inner = DnsMonitorHolder::new()?;

talpid-core/src/dns/macos.rs

@@ -21,6 +21,8 @@ use system_configuration::{
 };
 use talpid_routing::debounce::BurstGuard;
 
+use super::ResolvedDnsConfig;
+
 pub type Result<T> = std::result::Result<T, Error>;
 
 /// Errors that can happen when setting/monitoring DNS on macOS.
@@ -357,9 +359,11 @@ impl super::DnsMonitorT for DnsMonitor {
         })
     }
 
-    fn set(&mut self, interface: &str, servers: &[IpAddr]) -> Result<()> {
+    fn set(&mut self, interface: &str, config: ResolvedDnsConfig) -> Result<()> {
+        let servers: Vec<_> = config.addresses().collect();
+
         let mut state = self.state.lock();
-        state.apply_new_config(&self.store, interface, servers)
+        state.apply_new_config(&self.store, interface, &servers)
     }
 
     fn reset(&mut self) -> Result<()> {

talpid-core/src/dns/mod.rs

@@ -1,4 +1,6 @@
+use std::fmt;
 use std::net::IpAddr;
+
 #[cfg(target_os = "linux")]
 use talpid_routing::RouteManagerHandle;
 
@@ -23,6 +25,116 @@ mod imp;
 
 pub use self::imp::Error;
 
+/// DNS configuration
+#[derive(Debug, Clone, PartialEq)]
+pub struct DnsConfig {
+    config: InnerDnsConfig,
+}
+
+impl Default for DnsConfig {
+    fn default() -> Self {
+        Self {
+            config: InnerDnsConfig::Default,
+        }
+    }
+}
+
+impl DnsConfig {
+    /// Use the specified addresses for DNS resolution
+    pub fn from_addresses(tunnel_config: &[IpAddr], non_tunnel_config: &[IpAddr]) -> Self {
+        DnsConfig {
+            config: InnerDnsConfig::Override {
+                tunnel_config: tunnel_config.to_owned(),
+                non_tunnel_config: non_tunnel_config.to_owned(),
+            },
+        }
+    }
+}
+
+#[derive(Debug, Clone, PartialEq)]
+enum InnerDnsConfig {
+    /// Use gateway addresses from the tunnel config
+    Default,
+    /// Use the specified addresses for DNS resolution
+    Override {
+        /// Addresses to configure on the tunnel interface
+        tunnel_config: Vec<IpAddr>,
+        /// Addresses to allow on non-tunnel interface.
+        /// For the most part, the tunnel state machine will not handle any of this configuration
+        /// on non-tunnel interface, only allow them in the firewall.
+        non_tunnel_config: Vec<IpAddr>,
+    },
+}
+
+impl DnsConfig {
+    pub(crate) fn resolve(&self, default_tun_config: &[IpAddr]) -> ResolvedDnsConfig {
+        match &self.config {
+            InnerDnsConfig::Default => ResolvedDnsConfig {
+                tunnel_config: default_tun_config.to_owned(),
+                non_tunnel_config: vec![],
+            },
+            InnerDnsConfig::Override {
+                tunnel_config,
+                non_tunnel_config,
+            } => ResolvedDnsConfig {
+                tunnel_config: tunnel_config.to_owned(),
+                non_tunnel_config: non_tunnel_config.to_owned(),
+            },
+        }
+    }
+}
+
+/// DNS configuration with `DnsConfig::Default` resolved
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct ResolvedDnsConfig {
+    /// Addresses to configure on the tunnel interface
+    tunnel_config: Vec<IpAddr>,
+    /// Addresses to allow on non-tunnel interface.
+    /// For the most part, the tunnel state machine will not handle any of this configuration
+    /// on non-tunnel interface, only allow them in the firewall.
+    non_tunnel_config: Vec<IpAddr>,
+}
+
+impl fmt::Display for ResolvedDnsConfig {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str("Tunnel DNS: ")?;
+        Self::fmt_addr_set(f, &self.tunnel_config)?;
+
+        f.write_str(" Non-tunnel DNS: ")?;
+        Self::fmt_addr_set(f, &self.non_tunnel_config)
+    }
+}
+
+impl ResolvedDnsConfig {
+    fn fmt_addr_set(f: &mut fmt::Formatter<'_>, addrs: &[IpAddr]) -> fmt::Result {
+        f.write_str("{")?;
+        for (i, addr) in addrs.iter().enumerate() {
+            if i > 0 {
+                f.write_str(", ")?;
+            }
+            write!(f, "{}", addr)?;
+        }
+        f.write_str("}")
+    }
+
+    /// Addresses to configure on the tunnel interface
+    pub fn tunnel_config(&self) -> &[IpAddr] {
+        &self.tunnel_config
+    }
+
+    /// Addresses to allow on non-tunnel interface.
+    /// For the most part, the tunnel state machine will not handle any of this configuration
+    /// on non-tunnel interface, only allow them in the firewall.
+    pub fn non_tunnel_config(&self) -> &[IpAddr] {
+        &self.non_tunnel_config
+    }
+
+    /// Consume `self` and return a vector of all addresses
+    pub fn addresses(self) -> impl Iterator<Item = IpAddr> {
+        self.non_tunnel_config.into_iter().chain(self.tunnel_config)
+    }
+}
+
 /// Sets and monitors system DNS settings. Makes sure the desired DNS servers are being used.
 pub struct DnsMonitor {
     inner: imp::DnsMonitor,
@@ -45,16 +157,9 @@ impl DnsMonitor {
     }
 
     /// Set DNS to the given servers. And start monitoring the system for changes.
-    pub fn set(&mut self, interface: &str, servers: &[IpAddr]) -> Result<(), Error> {
-        log::info!(
-            "Setting DNS servers to {}",
-            servers
-                .iter()
-                .map(|ip| ip.to_string())
-                .collect::<Vec<String>>()
-                .join(", ")
-        );
-        self.inner.set(interface, servers)
+    pub fn set(&mut self, interface: &str, config: ResolvedDnsConfig) -> Result<(), Error> {
+        log::info!("Setting DNS servers: {config}",);
+        self.inner.set(interface, config)
     }
 
     /// Reset system DNS settings to what it was before being set by this instance.
@@ -81,7 +186,7 @@ trait DnsMonitorT: Sized {
         #[cfg(target_os = "linux")] route_manager: RouteManagerHandle,
     ) -> Result<Self, Self::Error>;
 
-    fn set(&mut self, interface: &str, servers: &[IpAddr]) -> Result<(), Self::Error>;
+    fn set(&mut self, interface: &str, servers: ResolvedDnsConfig) -> Result<(), Self::Error>;
 
     fn reset(&mut self) -> Result<(), Self::Error>;
 

talpid-core/src/dns/windows/auto.rs

@@ -1,5 +1,5 @@
 use super::{iphlpapi, netsh, tcpip};
-use crate::dns::DnsMonitorT;
+use crate::dns::{DnsMonitorT, ResolvedDnsConfig};
 use windows_sys::Win32::System::Rpc::RPC_S_SERVER_UNAVAILABLE;
 
 pub struct DnsMonitor {
@@ -12,11 +12,11 @@ enum InnerMonitor {
 }
 
 impl InnerMonitor {
-    fn set(&mut self, interface: &str, servers: &[std::net::IpAddr]) -> Result<(), super::Error> {
+    fn set(&mut self, interface: &str, config: ResolvedDnsConfig) -> Result<(), super::Error> {
         match self {
-            InnerMonitor::Iphlpapi(monitor) => monitor.set(interface, servers)?,
-            InnerMonitor::Netsh(monitor) => monitor.set(interface, servers)?,
-            InnerMonitor::Tcpip(monitor) => monitor.set(interface, servers)?,
+            InnerMonitor::Iphlpapi(monitor) => monitor.set(interface, config)?,
+            InnerMonitor::Netsh(monitor) => monitor.set(interface, config)?,
+            InnerMonitor::Tcpip(monitor) => monitor.set(interface, config)?,
         }
         Ok(())
     }
@@ -53,10 +53,10 @@ impl DnsMonitorT for DnsMonitor {
         Ok(Self { current_monitor })
     }
 
-    fn set(&mut self, interface: &str, servers: &[std::net::IpAddr]) -> Result<(), Self::Error> {
-        let result = self.current_monitor.set(interface, servers);
+    fn set(&mut self, interface: &str, config: ResolvedDnsConfig) -> Result<(), Self::Error> {
+        let result = self.current_monitor.set(interface, config.clone());
         if self.fallback_due_to_dnscache(&result) {
-            return self.set(interface, servers);
+            return self.set(interface, config);
         }
         result
     }

talpid-core/src/dns/windows/iphlpapi.rs

@@ -3,7 +3,7 @@
 //! it requires at least Windows 10, build 19041. For that reason, use run-time linking and fall
 //! back on other methods if it is not available.
 
-use crate::dns::DnsMonitorT;
+use crate::dns::{DnsMonitorT, ResolvedDnsConfig};
 use once_cell::sync::OnceCell;
 use std::{
     ffi::OsString,
@@ -122,7 +122,8 @@ impl DnsMonitorT for DnsMonitor {
         Ok(DnsMonitor { current_guid: None })
     }
 
-    fn set(&mut self, interface: &str, servers: &[IpAddr]) -> Result<(), Error> {
+    fn set(&mut self, interface: &str, config: ResolvedDnsConfig) -> Result<(), Error> {
+        let servers = config.tunnel_config();
         let guid = guid_from_luid(&luid_from_alias(interface).map_err(Error::ObtainInterfaceLuid)?)
             .map_err(Error::ObtainInterfaceGuid)?;