refactor(rust): move dns-control to bin-shared (#9023)

Currently, the platform-specific code for controlling DNS resolution on
a system sits in `firezone-headless-client`. This code is also used by
the GUI client. This creates a weird compile-time dependency from the
GUI client to the headless client.

For other components that have platform-specific implementations, we use
the `firezone-bin-shared` crate. As a first step of resolving the
compile-time dependency, we move the `dns_control` module to
`firezone-bin-shared`.

rust/Cargo.lock

@@ -2093,11 +2093,13 @@ name = "firezone-bin-shared"
 version = "0.1.0"
 dependencies = [
  "anyhow",
+ "atomicwrites",
  "axum",
  "bufferpool",
  "bytes",
  "clap",
  "dirs 5.0.1",
+ "dns-types",
  "firezone-logging",
  "flume",
  "futures",
@@ -2105,14 +2107,19 @@ dependencies = [
  "hex-literal",
  "ip-packet",
  "ip_network",
+ "ipconfig",
+ "itertools 0.13.0",
  "known-folders",
  "libc",
+ "mutants",
  "netlink-packet-core",
  "netlink-packet-route",
  "nix 0.29.0",
+ "resolv-conf",
  "ring",
  "rtnetlink",
  "socket-factory",
+ "tempfile",
  "thiserror 1.0.69",
  "tokio",
  "tokio-util",
@@ -2285,7 +2292,6 @@ dependencies = [
  "itertools 0.13.0",
  "known-folders",
  "libc",
- "mutants",
  "nix 0.29.0",
  "opentelemetry",
  "opentelemetry-stdout",

rust/bin-shared/Cargo.toml

@@ -10,6 +10,7 @@ license = { workspace = true }
 anyhow = { workspace = true }
 axum = { workspace = true, features = ["http1", "tokio"] }
 clap = { workspace = true, features = ["derive", "env"] }
+dns-types = { workspace = true }
 firezone-logging = { workspace = true }
 futures = { workspace = true, features = ["std", "async-await"] }
 gat-lending-iterator = { workspace = true }
@@ -18,7 +19,7 @@ ip-packet = { workspace = true }
 ip_network = { workspace = true, features = ["serde"] }
 socket-factory = { workspace = true }
 thiserror = { workspace = true }
-tokio = { workspace = true, features = ["io-util", "net", "rt", "sync"] }
+tokio = { workspace = true, features = ["io-util", "net", "rt", "sync", "process"] }
 tracing = { workspace = true }
 tun = { workspace = true }
 
@@ -29,11 +30,13 @@ tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
 
 [target.'cfg(target_os = "linux")'.dependencies]
 dirs = { workspace = true }
+atomicwrites = { workspace = true }
 flume = { workspace = true }
 libc = { workspace = true }
 netlink-packet-core = { version = "0.7" }
 netlink-packet-route = { version = "0.19" }
 nix = { workspace = true, features = ["socket"] }
+resolv-conf = { workspace = true }
 rtnetlink = { workspace = true }
 zbus = { workspace = true } # Can't use `zbus`'s `tokio` feature here, or it will break toast popups all the way over in `gui-client`.
 
@@ -46,6 +49,8 @@ windows-implement = { workspace = true }
 wintun = "0.5.1"
 winreg = { workspace = true }
 tokio-util = { workspace = true }
+ipconfig = "0.3.2"
+itertools = { workspace = true }
 
 [target.'cfg(windows)'.dependencies.windows]
 workspace = true
@@ -69,5 +74,9 @@ features = [
 ip-packet = { workspace = true }
 tokio = { workspace = true, features = ["net", "time"] }
 
+[target.'cfg(target_os = "linux")'.dev-dependencies]
+mutants = "0.0.3" # Needed to mark functions as exempt from `cargo-mutants` testing
+tempfile = { workspace = true }
+
 [lints]
 workspace = true

rust/bin-shared/src/dns_control.rs

@@ -6,7 +6,6 @@
 //! On Windows, we use NRPT by default. We can also explicitly not control DNS.
 
 use anyhow::Result;
-use firezone_bin_shared::platform::DnsControlMethod;
 use std::net::IpAddr;
 
 #[cfg(target_os = "linux")]
@@ -26,6 +25,8 @@ use macos as platform;
 
 use platform::system_resolvers;
 
+pub use platform::DnsControlMethod;
+
 /// Controls system-wide DNS.
 ///
 /// Always call `deactivate` when Firezone starts.

rust/bin-shared/src/dns_control/linux.rs

@@ -1,11 +1,36 @@
+use crate::TunDeviceManager;
+
 use super::DnsController;
 use anyhow::{Context as _, Result, bail};
 use dns_types::DomainName;
-use firezone_bin_shared::{TunDeviceManager, platform::DnsControlMethod};
 use std::{net::IpAddr, process::Command, str::FromStr};
 
 mod etc_resolv_conf;
 
+#[derive(clap::ValueEnum, Clone, Copy, Debug)]
+pub enum DnsControlMethod {
+    /// Explicitly disable DNS control.
+    ///
+    /// We don't use an `Option<Method>` because leaving out the CLI arg should
+    /// use Systemd, not disable DNS control.
+    Disabled,
+    /// Back up `/etc/resolv.conf` and replace it with our own
+    ///
+    /// Only suitable for the Alpine CI containers and maybe something like an
+    /// embedded system
+    EtcResolvConf,
+    /// Cooperate with `systemd-resolved`
+    ///
+    /// Suitable for most Ubuntu systems, probably
+    SystemdResolved,
+}
+
+impl Default for DnsControlMethod {
+    fn default() -> Self {
+        Self::SystemdResolved
+    }
+}
+
 impl DnsController {
     pub fn deactivate(&mut self) -> Result<()> {
         tracing::debug!("Deactivating DNS control...");

rust/bin-shared/src/dns_control/macos.rs

@@ -3,7 +3,12 @@ use std::net::IpAddr;
 use super::DnsController;
 use anyhow::{Result, bail};
 use dns_types::DomainName;
-use firezone_bin_shared::macos::DnsControlMethod;
+
+#[derive(clap::ValueEnum, Clone, Copy, Debug, Default)]
+pub enum DnsControlMethod {
+    #[default]
+    None,
+}
 
 impl DnsController {
     pub fn deactivate(&mut self) -> Result<()> {

rust/bin-shared/src/dns_control/windows.rs

@@ -13,11 +13,10 @@
 //!
 //! <https://superuser.com/a/1752670>
 
-use super::DnsController;
+use crate::DnsController;
+use crate::windows::{CREATE_NO_WINDOW, TUNNEL_UUID, error::EPT_S_NOT_REGISTERED};
 use anyhow::{Context as _, Result};
 use dns_types::DomainName;
-use firezone_bin_shared::platform::{CREATE_NO_WINDOW, DnsControlMethod, TUNNEL_UUID};
-use firezone_bin_shared::windows::error::EPT_S_NOT_REGISTERED;
 use std::{io, net::IpAddr, os::windows::process::CommandExt, path::Path, process::Command};
 use windows::Win32::System::GroupPolicy::{RP_FORCE, RefreshPolicyEx};
 
@@ -25,6 +24,23 @@ use windows::Win32::System::GroupPolicy::{RP_FORCE, RefreshPolicyEx};
 // Copied from the deep link schema
 const FZ_MAGIC: &str = "firezone-fd0020211111";
 
+#[derive(clap::ValueEnum, Clone, Copy, Debug)]
+pub enum DnsControlMethod {
+    /// Explicitly disable DNS control.
+    ///
+    /// We don't use an `Option<Method>` because leaving out the CLI arg should
+    /// use NRPT, not disable DNS control.
+    Disabled,
+    /// NRPT, the only DNS control method we use on Windows.
+    Nrpt,
+}
+
+impl Default for DnsControlMethod {
+    fn default() -> Self {
+        Self::Nrpt
+    }
+}
+
 impl DnsController {
     /// Deactivate any control Firezone has over the computer's DNS
     ///
@@ -280,66 +296,3 @@ fn set_nrpt_rule(key: &winreg::RegKey, dns_config_string: &str) -> Result<()> {
     key.set_value("Version", &0x2u32)?;
     Ok(())
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use std::collections::BTreeSet;
-
-    // Passes in CI but not locally. Maybe ReactorScram's dev system has IPv6 misconfigured. There it fails to pick up the IPv6 DNS servers.
-    #[ignore = "Needs admin, changes system state"]
-    #[test]
-    fn dns_control() {
-        let _guard = firezone_logging::test("debug");
-
-        let rt = tokio::runtime::Builder::new_current_thread()
-            .build()
-            .unwrap();
-
-        let mut tun_dev_manager = firezone_bin_shared::TunDeviceManager::new(1280, 1).unwrap(); // Note: num_threads (`1`) is unused on windows.
-        let _tun = tun_dev_manager.make_tun().unwrap();
-
-        rt.block_on(async {
-            tun_dev_manager
-                .set_ips(
-                    [100, 92, 193, 137].into(),
-                    [0xfd00, 0x2021, 0x1111, 0x0, 0x0, 0x0, 0xa, 0x9db5].into(),
-                )
-                .await
-        })
-        .unwrap();
-
-        let mut dns_controller = DnsController {
-            dns_control_method: DnsControlMethod::Nrpt,
-        };
-
-        let fz_dns_servers = vec![
-            IpAddr::from([100, 100, 111, 1]),
-            IpAddr::from([100, 100, 111, 2]),
-            IpAddr::from([
-                0xfd00, 0x2021, 0x1111, 0x8000, 0x0100, 0x0100, 0x0111, 0x0003,
-            ]),
-            IpAddr::from([
-                0xfd00, 0x2021, 0x1111, 0x8000, 0x0100, 0x0100, 0x0111, 0x0004,
-            ]),
-        ];
-        rt.block_on(async {
-            dns_controller
-                .set_dns(fz_dns_servers.clone(), None)
-                .await
-                .unwrap();
-        });
-
-        let adapter = ipconfig::get_adapters()
-            .unwrap()
-            .into_iter()
-            .find(|a| a.friendly_name() == "Firezone")
-            .unwrap();
-        assert_eq!(
-            BTreeSet::from_iter(adapter.dns_servers().iter().cloned()),
-            BTreeSet::from_iter(fz_dns_servers.into_iter())
-        );
-
-        dns_controller.deactivate().unwrap();
-    }
-}

rust/bin-shared/src/lib.rs

@@ -2,6 +2,7 @@
 
 pub mod http_health_check;
 
+mod dns_control;
 mod network_changes;
 mod tun_device_manager;
 
@@ -51,5 +52,6 @@ pub const BUNDLE_ID: &str = "dev.firezone.client";
 /// Mark for Firezone sockets to prevent routing loops on Linux.
 pub const FIREZONE_MARK: u32 = 0xfd002021;
 
+pub use dns_control::{DnsControlMethod, DnsController, system_resolvers_for_gui};
 pub use network_changes::{new_dns_notifier, new_network_notifier};
 pub use tun_device_manager::TunDeviceManager;

rust/bin-shared/src/linux.rs

@@ -4,30 +4,6 @@ use crate::FIREZONE_MARK;
 use nix::sys::socket::{setsockopt, sockopt};
 use socket_factory::{TcpSocket, UdpSocket};
 
-#[derive(clap::ValueEnum, Clone, Copy, Debug)]
-pub enum DnsControlMethod {
-    /// Explicitly disable DNS control.
-    ///
-    /// We don't use an `Option<Method>` because leaving out the CLI arg should
-    /// use Systemd, not disable DNS control.
-    Disabled,
-    /// Back up `/etc/resolv.conf` and replace it with our own
-    ///
-    /// Only suitable for the Alpine CI containers and maybe something like an
-    /// embedded system
-    EtcResolvConf,
-    /// Cooperate with `systemd-resolved`
-    ///
-    /// Suitable for most Ubuntu systems, probably
-    SystemdResolved,
-}
-
-impl Default for DnsControlMethod {
-    fn default() -> Self {
-        Self::SystemdResolved
-    }
-}
-
 pub fn tcp_socket_factory(socket_addr: &SocketAddr) -> io::Result<TcpSocket> {
     let socket = socket_factory::tcp(socket_addr)?;
     setsockopt(&socket, sockopt::Mark, &FIREZONE_MARK)?;

rust/bin-shared/src/macos.rs

@@ -1,8 +1,2 @@
-#[derive(clap::ValueEnum, Clone, Copy, Debug, Default)]
-pub enum DnsControlMethod {
-    #[default]
-    None,
-}
-
 pub use socket_factory::tcp as tcp_socket_factory;
 pub use socket_factory::udp as udp_socket_factory;

rust/bin-shared/src/network_changes/linux.rs

@@ -1,6 +1,6 @@
 //! Not implemented for Linux yet
 
-use crate::platform::DnsControlMethod;
+use crate::DnsControlMethod;
 use anyhow::Result;
 use futures::StreamExt as _;
 use std::time::Duration;

rust/bin-shared/src/network_changes/macos.rs

@@ -1,4 +1,4 @@
-use crate::platform::DnsControlMethod;
+use crate::DnsControlMethod;
 use anyhow::{Result, bail};
 
 pub async fn new_dns_notifier(

rust/bin-shared/src/network_changes/windows.rs

@@ -62,7 +62,7 @@
 //!
 //! Raymond Chen also explains it on his blog: <https://devblogs.microsoft.com/oldnewthing/20191125-00/?p=103135>
 
-use crate::platform::DnsControlMethod;
+use crate::DnsControlMethod;
 use anyhow::{Context as _, Result, anyhow};
 use std::collections::HashMap;
 use std::sync::Mutex;

rust/bin-shared/src/windows.rs

@@ -87,23 +87,6 @@ pub mod error {
     pub const EPT_S_NOT_REGISTERED: HRESULT = HRESULT::from_win32(0x06D9);
 }
 
-#[derive(clap::ValueEnum, Clone, Copy, Debug)]
-pub enum DnsControlMethod {
-    /// Explicitly disable DNS control.
-    ///
-    /// We don't use an `Option<Method>` because leaving out the CLI arg should
-    /// use NRPT, not disable DNS control.
-    Disabled,
-    /// NRPT, the only DNS control method we use on Windows.
-    Nrpt,
-}
-
-impl Default for DnsControlMethod {
-    fn default() -> Self {
-        Self::Nrpt
-    }
-}
-
 pub fn tcp_socket_factory(addr: &SocketAddr) -> io::Result<TcpSocket> {
     delete_all_routing_entries_matching(addr.ip())?;
 

rust/bin-shared/tests/dns_control_windows.rs

@@ -0,0 +1,62 @@
+#![cfg(target_os = "windows")]
+#![allow(clippy::unwrap_used)]
+
+use firezone_bin_shared::{DnsControlMethod, DnsController};
+use std::{collections::BTreeSet, net::IpAddr};
+
+// Passes in CI but not locally. Maybe ReactorScram's dev system has IPv6 misconfigured. There it fails to pick up the IPv6 DNS servers.
+#[ignore = "Needs admin, changes system state"]
+#[test]
+fn dns_control() {
+    let _guard = firezone_logging::test("debug");
+
+    let rt = tokio::runtime::Builder::new_current_thread()
+        .build()
+        .unwrap();
+
+    let mut tun_dev_manager = firezone_bin_shared::TunDeviceManager::new(1280, 1).unwrap(); // Note: num_threads (`1`) is unused on windows.
+    let _tun = tun_dev_manager.make_tun().unwrap();
+
+    rt.block_on(async {
+        tun_dev_manager
+            .set_ips(
+                [100, 92, 193, 137].into(),
+                [0xfd00, 0x2021, 0x1111, 0x0, 0x0, 0x0, 0xa, 0x9db5].into(),
+            )
+            .await
+    })
+    .unwrap();
+
+    let mut dns_controller = DnsController {
+        dns_control_method: DnsControlMethod::Nrpt,
+    };
+
+    let fz_dns_servers = vec![
+        IpAddr::from([100, 100, 111, 1]),
+        IpAddr::from([100, 100, 111, 2]),
+        IpAddr::from([
+            0xfd00, 0x2021, 0x1111, 0x8000, 0x0100, 0x0100, 0x0111, 0x0003,
+        ]),
+        IpAddr::from([
+            0xfd00, 0x2021, 0x1111, 0x8000, 0x0100, 0x0100, 0x0111, 0x0004,
+        ]),
+    ];
+    rt.block_on(async {
+        dns_controller
+            .set_dns(fz_dns_servers.clone(), None)
+            .await
+            .unwrap();
+    });
+
+    let adapter = ipconfig::get_adapters()
+        .unwrap()
+        .into_iter()
+        .find(|a| a.friendly_name() == "Firezone")
+        .unwrap();
+    assert_eq!(
+        BTreeSet::from_iter(adapter.dns_servers().iter().cloned()),
+        BTreeSet::from_iter(fz_dns_servers.into_iter())
+    );
+
+    dns_controller.deactivate().unwrap();
+}

rust/bin-shared/tests/network_notifiers.rs

@@ -1,6 +1,6 @@
 #![allow(clippy::unwrap_used)]
 
-use firezone_bin_shared::{new_dns_notifier, new_network_notifier, platform::DnsControlMethod};
+use firezone_bin_shared::{DnsControlMethod, new_dns_notifier, new_network_notifier};
 use futures::future::FutureExt as _;
 use std::time::Duration;
 use tokio::time::timeout;

rust/gui-client/src-common/src/controller.rs

@@ -6,7 +6,7 @@ use crate::{
 };
 use anyhow::{Context, Result, anyhow};
 use connlib_model::ResourceView;
-use firezone_bin_shared::platform::DnsControlMethod;
+use firezone_bin_shared::DnsControlMethod;
 use firezone_headless_client::{
     IpcClientMsg::{self, SetDisabledResources},
     IpcServerMsg, IpcServiceError,
@@ -246,8 +246,7 @@ impl<I: GuiIntegration> Controller<I> {
                 }
                 EventloopTick::DnsChanged(Ok(())) => {
                     if self.status.needs_network_changes() {
-                        let resolvers =
-                            firezone_headless_client::dns_control::system_resolvers_for_gui()?;
+                        let resolvers = firezone_bin_shared::system_resolvers_for_gui()?;
                         tracing::debug!(
                             ?resolvers,
                             "New DNS resolvers, calling `Session::set_dns`"

rust/headless-client/Cargo.toml

@@ -47,9 +47,6 @@ uuid = { workspace = true, features = ["std", "v4", "serde"] }
 [dev-dependencies]
 tempfile = { workspace = true }
 
-[target.'cfg(target_os = "linux")'.dev-dependencies]
-mutants = "0.0.3" # Needed to mark functions as exempt from `cargo-mutants` testing
-
 [target.'cfg(target_os = "linux")'.dependencies]
 libc = { workspace = true }
 nix = { workspace = true, features = ["fs", "user", "socket"] }

rust/headless-client/src/ipc_service.rs

@@ -1,11 +1,11 @@
-use crate::{CallbackHandler, CliCommon, ConnlibMsg, device_id, dns_control::DnsController};
+use crate::{CallbackHandler, CliCommon, ConnlibMsg, device_id};
 use anyhow::{Context as _, Result, bail};
 use atomicwrites::{AtomicFile, OverwriteBehavior};
 use clap::Parser;
 use connlib_model::ResourceView;
 use firezone_bin_shared::{
-    TOKEN_ENV_KEY, TunDeviceManager, known_dirs,
-    platform::{DnsControlMethod, tcp_socket_factory, udp_socket_factory},
+    DnsControlMethod, DnsController, TOKEN_ENV_KEY, TunDeviceManager, known_dirs,
+    platform::{tcp_socket_factory, udp_socket_factory},
     signals,
 };
 use firezone_logging::{FilterReloadHandle, err_with_src, sentry_layer, telemetry_span};

rust/headless-client/src/ipc_service/ipc.rs

@@ -6,8 +6,6 @@ use tokio_util::{
     codec::{FramedRead, FramedWrite, LengthDelimitedCodec},
 };
 
-// There is no special way to prevent `cargo-mutants` from throwing false
-// positives on code for other platforms.
 #[cfg(target_os = "linux")]
 #[path = "ipc/linux.rs"]
 mod platform;

rust/headless-client/src/ipc_service/windows.rs

@@ -1,6 +1,6 @@
 use crate::CliCommon;
 use anyhow::{Context as _, Result, bail};
-use firezone_bin_shared::platform::DnsControlMethod;
+use firezone_bin_shared::DnsControlMethod;
 use firezone_logging::FilterReloadHandle;
 use firezone_telemetry::Telemetry;
 use futures::channel::mpsc;

rust/headless-client/src/lib.rs

@@ -14,7 +14,7 @@ use anyhow::{Context as _, Result};
 use connlib_client_shared::Callbacks;
 use connlib_model::ResourceView;
 use dns_types::DomainName;
-use firezone_bin_shared::platform::DnsControlMethod;
+use firezone_bin_shared::DnsControlMethod;
 use firezone_logging::FilterReloadHandle;
 use std::{
     net::{IpAddr, Ipv4Addr, Ipv6Addr},
@@ -26,12 +26,9 @@ use tracing_subscriber::{EnvFilter, Layer as _, Registry, fmt, layer::Subscriber
 mod clear_logs;
 /// Generate a persistent device ID, stores it to disk, and reads it back.
 pub mod device_id;
-// Pub because the GUI reads the system resolvers
-pub mod dns_control;
 mod ipc_service;
 
 pub use clear_logs::clear_logs;
-pub use dns_control::DnsController;
 pub use ipc_service::{
     ClientMsg as IpcClientMsg, Error as IpcServiceError, ServerMsg as IpcServerMsg, ipc,
     run_only_ipc_service,

rust/headless-client/src/main.rs

@@ -7,11 +7,11 @@ use backoff::ExponentialBackoffBuilder;
 use clap::Parser;
 use connlib_client_shared::Session;
 use firezone_bin_shared::{
-    TOKEN_ENV_KEY, TunDeviceManager, new_dns_notifier, new_network_notifier,
+    DnsController, TOKEN_ENV_KEY, TunDeviceManager, new_dns_notifier, new_network_notifier,
     platform::{tcp_socket_factory, udp_socket_factory},
     signals,
 };
-use firezone_headless_client::{CallbackHandler, CliCommon, ConnlibMsg, DnsController, device_id};
+use firezone_headless_client::{CallbackHandler, CliCommon, ConnlibMsg, device_id};
 use firezone_logging::telemetry_span;
 use firezone_telemetry::Telemetry;
 use firezone_telemetry::otel;