diff --git a/rust/connlib/shared/Cargo.toml b/rust/connlib/shared/Cargo.toml index b3e5b8368..96bcaac2a 100644 --- a/rust/connlib/shared/Cargo.toml +++ b/rust/connlib/shared/Cargo.toml @@ -39,7 +39,6 @@ proptest = { version = "1.4.0", optional = true } log = "0.4" [dev-dependencies] -anyhow = "1.0" itertools = "0.12" tempfile = "3.10.1" mutants = "0.0.3" # Needed to mark functions as exempt from `cargo-mutants` testing diff --git a/rust/connlib/shared/src/lib.rs b/rust/connlib/shared/src/lib.rs index 04a102fa1..87185dd2e 100644 --- a/rust/connlib/shared/src/lib.rs +++ b/rust/connlib/shared/src/lib.rs @@ -51,6 +51,19 @@ pub const BUNDLE_ID: &str = "dev.firezone.client"; const VERSION: &str = env!("CARGO_PKG_VERSION"); const LIB_NAME: &str = "connlib"; +/// Deactivates DNS control on Windows +#[cfg(target_os = "windows")] +pub fn deactivate_dns_control() -> anyhow::Result<()> { + windows::dns::deactivate() +} + +/// Deactivates DNS control on Windows +#[cfg(not(target_os = "windows"))] +#[allow(clippy::unnecessary_wraps)] +pub fn deactivate_dns_control() -> anyhow::Result<()> { + Ok(()) +} + pub fn keypair() -> (StaticSecret, PublicKey) { let private_key = StaticSecret::random_from_rng(OsRng); let public_key = PublicKey::from(&private_key); diff --git a/rust/connlib/shared/src/windows.rs b/rust/connlib/shared/src/windows.rs index 535b5bafe..ea71ccf2b 100644 --- a/rust/connlib/shared/src/windows.rs +++ b/rust/connlib/shared/src/windows.rs @@ -16,6 +16,113 @@ pub fn app_local_data_dir() -> Result { Ok(path) } +pub mod dns { + //! Gives Firezone DNS privilege over other DNS resolvers on the system + //! + //! This uses NRPT and claims all domains, similar to the `systemd-resolved` control method + //! on Linux. + //! This allows us to "shadow" DNS resolvers that are configured by the user or DHCP on + //! physical interfaces, as long as they don't have any NRPT rules that outrank us. + //! + //! If Firezone crashes, restarting Firezone and closing it gracefully will resume + //! normal DNS operation. The Powershell command to remove the NRPT rule can also be run + //! by hand. + //! + //! The system default resolvers don't need to be reverted because they're never deleted. + //! + //! + + use anyhow::Result; + use std::{net::IpAddr, os::windows::process::CommandExt, process::Command}; + + /// Hides Powershell's console on Windows + /// + /// + /// Also used for self-elevation + const CREATE_NO_WINDOW: u32 = 0x08000000; + + // Unique magic number that we can use to delete our well-known NRPT rule. + // Copied from the deep link schema + const FZ_MAGIC: &str = "firezone-fd0020211111"; + + /// Tells Windows to send all DNS queries to our sentinels + /// + /// Parameters: + /// - `dns_config_string`: Comma-separated IP addresses of DNS servers, e.g. "1.1.1.1,8.8.8.8" + pub fn activate(dns_config: &[IpAddr], iface_idx: u32) -> Result<()> { + let dns_config_string = dns_config + .iter() + .map(|ip| format!("\"{ip}\"")) + .collect::>() + .join(","); + + // Set our DNS IP as the DNS server for our interface + // TODO: Known issue where web browsers will keep a connection open to a site, + // using QUIC, HTTP/2, or even HTTP/1.1, and so they won't resolve the DNS + // again unless you let that connection time out: + // + // TODO: If we have a Windows gateway, it shouldn't configure DNS, right? + Command::new("powershell") + .creation_flags(CREATE_NO_WINDOW) + .arg("-Command") + .arg(format!("Set-DnsClientServerAddress -InterfaceIndex {iface_idx} -ServerAddresses({dns_config_string})")) + .status()?; + + tracing::info!("Activating DNS control"); + Command::new("powershell") + .creation_flags(CREATE_NO_WINDOW) + .args([ + "-Command", + "Add-DnsClientNrptRule", + "-Namespace", + ".", + "-Comment", + FZ_MAGIC, + "-NameServers", + &dns_config_string, + ]) + .status()?; + Ok(()) + } + + /// Tells Windows to send all DNS queries to this new set of sentinels + /// + /// Currently implemented as just removing the rule and re-adding it, which + /// creates a gap but doesn't require us to parse Powershell output to figure + /// out the rule's UUID. + /// + /// Parameters: + /// - `dns_config_string` - Passed verbatim to [`activate`] + pub fn change(dns_config: &[IpAddr], iface_idx: u32) -> Result<()> { + deactivate()?; + activate(dns_config, iface_idx)?; + Ok(()) + } + + pub fn deactivate() -> Result<()> { + Command::new("powershell") + .creation_flags(CREATE_NO_WINDOW) + .args(["-Command", "Get-DnsClientNrptRule", "|"]) + .args(["where", "Comment", "-eq", FZ_MAGIC, "|"]) + .args(["foreach", "{"]) + .args(["Remove-DnsClientNrptRule", "-Name", "$_.Name", "-Force"]) + .args(["}"]) + .status()?; + tracing::info!("Deactivated DNS control"); + Ok(()) + } + + /// Flush Windows' system-wide DNS cache + pub fn flush() -> Result<()> { + tracing::info!("Flushing Windows DNS cache"); + Command::new("powershell") + .creation_flags(CREATE_NO_WINDOW) + .args(["-Command", "Clear-DnsClientCache"]) + .status()?; + Ok(()) + } +} + /// Returns the absolute path for installing and loading `wintun.dll` /// /// e.g. `C:\Users\User\AppData\Local\dev.firezone.client\data\wintun.dll` diff --git a/rust/connlib/tunnel/src/device_channel/tun_windows.rs b/rust/connlib/tunnel/src/device_channel/tun_windows.rs index bf89b7a76..b1ab69318 100644 --- a/rust/connlib/tunnel/src/device_channel/tun_windows.rs +++ b/rust/connlib/tunnel/src/device_channel/tun_windows.rs @@ -39,6 +39,9 @@ pub struct Tun { impl Drop for Tun { fn drop(&mut self) { + if let Err(error) = connlib_shared::windows::dns::deactivate() { + tracing::error!(?error, "Failed to deactivate DNS control"); + } if let Err(e) = self.session.shutdown() { tracing::error!("wintun::Session::shutdown: {e:#?}"); } @@ -137,25 +140,8 @@ impl Tun { let iface_idx = self.adapter.get_adapter_index()?; - // Set our DNS IP as the DNS server for our interface - // TODO: Known issue where web browsers will keep a connection open to a site, - // using QUIC, HTTP/2, or even HTTP/1.1, and so they won't resolve the DNS - // again unless you let that connection time out: - // - // TODO: If we have a Windows gateway, it shouldn't configure DNS, right? - Command::new("powershell") - .creation_flags(CREATE_NO_WINDOW) - .arg("-Command") - .arg(format!( - "Set-DnsClientServerAddress -InterfaceIndex {iface_idx} -ServerAddresses({})", - dns_config - .iter() - .map(|ip| format!("\"{ip}\"")) - .collect::>() - .join(",") - )) - .stdout(Stdio::null()) - .status()?; + connlib_shared::windows::dns::change(dns_config, iface_idx) + .expect("Should be able to control DNS"); Ok(()) } @@ -176,6 +162,9 @@ impl Tun { self.routes = new_routes; + // TODO: Might be calling this more often than it needs + connlib_shared::windows::dns::flush().expect("Should be able to flush Windows' DNS cache"); + Ok(()) } diff --git a/rust/gui-client/src-tauri/src/client/elevation.rs b/rust/gui-client/src-tauri/src/client/elevation.rs index ee5ef76fb..010aeaa50 100644 --- a/rust/gui-client/src-tauri/src/client/elevation.rs +++ b/rust/gui-client/src-tauri/src/client/elevation.rs @@ -71,8 +71,9 @@ mod imp { } pub(crate) fn elevate() -> Result<()> { - // Hides Powershell's console on Windows - // + /// Hides Powershell's console on Windows + /// + /// const CREATE_NO_WINDOW: u32 = 0x08000000; let current_exe = tauri_utils::platform::current_exe()?; diff --git a/rust/gui-client/src-tauri/src/client/gui.rs b/rust/gui-client/src-tauri/src/client/gui.rs index 7e6f5bc47..843063e62 100644 --- a/rust/gui-client/src-tauri/src/client/gui.rs +++ b/rust/gui-client/src-tauri/src/client/gui.rs @@ -280,20 +280,21 @@ pub(crate) fn run(cli: client::Cli) -> Result<(), Error> { // This seems to be a platform limitation that Tauri is unable to hide // from us. It was the source of much consternation at time of writing. - match task.await { + let exit_code = match task.await { Err(error) => { tracing::error!(?error, "run_controller panicked"); - app_handle.exit(1); + 1 } Ok(Err(error)) => { tracing::error!(?error, "run_controller returned an error"); - app_handle.exit(1); + 1 } - Ok(Ok(_)) => { - tracing::info!("GUI controller task exited cleanly. Exiting process"); - app_handle.exit(0); - } - } + Ok(Ok(_)) => 0, + }; + + cleanup(); + tracing::info!(?exit_code); + app_handle.exit(exit_code); }); Ok(()) }; @@ -334,6 +335,13 @@ pub(crate) fn run(cli: client::Cli) -> Result<(), Error> { Ok(()) } +/// Best-effort cleanup of things like DNS control before graceful exit +fn cleanup() { + // Do this redundant deactivation because `Tun` will not automatically Drop before + // the process exits + connlib_shared::deactivate_dns_control().ok(); +} + /// Runs a smoke test and then asks Controller to exit gracefully /// /// You can purposely fail this test by deleting the exported zip file during diff --git a/rust/gui-client/src-tauri/src/client/tunnel-wrapper/in_proc.rs b/rust/gui-client/src-tauri/src/client/tunnel-wrapper/in_proc.rs index 7ee2b1ee5..d645d8421 100644 --- a/rust/gui-client/src-tauri/src/client/tunnel-wrapper/in_proc.rs +++ b/rust/gui-client/src-tauri/src/client/tunnel-wrapper/in_proc.rs @@ -76,6 +76,12 @@ pub fn connect( let login = LoginUrl::client(api_url, &token, device_id.id, None, public_key.to_bytes())?; + // Deactivate DNS control since that can prevent us from bootstrapping a connection + // to the portal. Maybe we could bring up a sentinel resolver before + // connecting to the portal, but right now the portal seems to need system DNS + // for the first connection. + connlib_shared::deactivate_dns_control()?; + // All direct calls into connlib must be in the tunnel process let session = connlib_client_shared::Session::connect( login,