diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 42f096421..405a4a9b8 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -2344,6 +2344,7 @@ dependencies = [ "ip-packet", "ip_network", "ipconfig", + "itertools 0.13.0", "known-folders", "libc", "mutants", diff --git a/rust/bin-shared/src/tun_device_manager/windows.rs b/rust/bin-shared/src/tun_device_manager/windows.rs index cbd43368c..a82871131 100644 --- a/rust/bin-shared/src/tun_device_manager/windows.rs +++ b/rust/bin-shared/src/tun_device_manager/windows.rs @@ -1,4 +1,4 @@ -use crate::windows::CREATE_NO_WINDOW; +use crate::windows::{CREATE_NO_WINDOW, TUNNEL_UUID}; use crate::TUNNEL_NAME; use anyhow::{Context as _, Result}; use ip_network::{IpNetwork, Ipv4Network, Ipv6Network}; @@ -219,8 +219,6 @@ impl Drop for Tun { impl Tun { #[tracing::instrument(level = "debug")] pub fn new(mtu: u32) -> Result { - const TUNNEL_UUID: &str = "e9245bc1-b8c1-44ca-ab1d-c6aad4f13b9c"; - let path = ensure_dll()?; // SAFETY: we're loading a DLL from disk and it has arbitrary C code in it. There's no perfect way to prove it's safe. let wintun = unsafe { wintun::load_from_path(path) }?; diff --git a/rust/bin-shared/src/windows.rs b/rust/bin-shared/src/windows.rs index 801cccecc..02959d746 100644 --- a/rust/bin-shared/src/windows.rs +++ b/rust/bin-shared/src/windows.rs @@ -29,6 +29,11 @@ use windows::Win32::{ /// Also used for self-elevation pub const CREATE_NO_WINDOW: u32 = 0x08000000; +/// A UUID we generated at dev time for our tunnel. +/// +/// This ends up in registry keys and tunnel management. +pub const TUNNEL_UUID: &str = "e9245bc1-b8c1-44ca-ab1d-c6aad4f13b9c"; + #[derive(clap::ValueEnum, Clone, Copy, Debug)] pub enum DnsControlMethod { /// Explicitly disable DNS control. diff --git a/rust/headless-client/Cargo.toml b/rust/headless-client/Cargo.toml index 443a2720b..47d6b5f27 100644 --- a/rust/headless-client/Cargo.toml +++ b/rust/headless-client/Cargo.toml @@ -57,6 +57,7 @@ dirs = "5.0.1" [target.'cfg(target_os = "windows")'.dependencies] ipconfig = "0.3.2" +itertools = "0.13.0" known-folders = "1.2.0" thiserror = { version = "1.0", default-features = false } tracing-subscriber = { version = "0.3.17", features = ["env-filter"] } diff --git a/rust/headless-client/src/dns_control/windows.rs b/rust/headless-client/src/dns_control/windows.rs index 2ace81306..93f2aa5f8 100644 --- a/rust/headless-client/src/dns_control/windows.rs +++ b/rust/headless-client/src/dns_control/windows.rs @@ -15,7 +15,7 @@ use super::DnsController; use anyhow::{Context as _, Result}; -use firezone_bin_shared::platform::{DnsControlMethod, CREATE_NO_WINDOW}; +use firezone_bin_shared::platform::{DnsControlMethod, CREATE_NO_WINDOW, TUNNEL_UUID}; use std::{ io::ErrorKind, net::IpAddr, os::windows::process::CommandExt, path::Path, process::Command, }; @@ -102,9 +102,6 @@ pub(crate) fn system_resolvers(_method: DnsControlMethod) -> Result> const NRPT_REG_KEY: &str = "{6C0507CB-C884-4A78-BC55-0ACEE21227F6}"; /// 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" fn activate(dns_config: &[IpAddr]) -> Result<()> { // 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 @@ -114,11 +111,10 @@ fn activate(dns_config: &[IpAddr]) -> Result<()> { let hklm = winreg::RegKey::predef(winreg::enums::HKEY_LOCAL_MACHINE); - let dns_config_string = dns_config - .iter() - .map(|ip| ip.to_string()) - .collect::>() - .join(";"); + set_nameservers_on_interface(dns_config)?; + + // e.g. [100.100.111.1, 100.100.111.2] -> "100.100.111.1;100.100.111.2" + let dns_config_string = itertools::join(dns_config, ";"); // It's safe to always set the local rule. let (key, _) = hklm.create_subkey(local_nrpt_path().join(NRPT_REG_KEY))?; @@ -140,6 +136,36 @@ fn activate(dns_config: &[IpAddr]) -> Result<()> { Ok(()) } +/// Sets our DNS servers in the registry so `ipconfig` and WSL will notice them +/// Fixes #6777 +fn set_nameservers_on_interface(dns_config: &[IpAddr]) -> Result<()> { + let hklm = winreg::RegKey::predef(winreg::enums::HKEY_LOCAL_MACHINE); + + let key = hklm.open_subkey_with_flags( + Path::new(&format!( + r"SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Interfaces\{{{TUNNEL_UUID}}}" + )), + winreg::enums::KEY_WRITE, + )?; + key.set_value( + "NameServer", + &itertools::join(dns_config.iter().filter(|addr| addr.is_ipv4()), ";"), + )?; + + let key = hklm.open_subkey_with_flags( + Path::new(&format!( + r"SYSTEM\CurrentControlSet\Services\Tcpip6\Parameters\Interfaces\{{{TUNNEL_UUID}}}" + )), + winreg::enums::KEY_WRITE, + )?; + key.set_value( + "NameServer", + &itertools::join(dns_config.iter().filter(|addr| addr.is_ipv6()), ";"), + )?; + + Ok(()) +} + /// Returns the registry path we can use to set NRPT rules when Group Policy is not in effect. fn local_nrpt_path() -> &'static Path { // Must be backslashes. @@ -171,3 +197,64 @@ 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::Runtime::new().unwrap(); + + let mut tun_dev_manager = firezone_bin_shared::TunDeviceManager::new(1280).unwrap(); + 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()) + .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(); + } +} diff --git a/rust/headless-client/src/lib.rs b/rust/headless-client/src/lib.rs index 2ccfcb132..7fbeb3702 100644 --- a/rust/headless-client/src/lib.rs +++ b/rust/headless-client/src/lib.rs @@ -145,55 +145,9 @@ pub fn setup_stdout_logging() -> Result { #[cfg(test)] mod tests { use super::*; - use clap::Parser; - - const EXE_NAME: &str = "firezone-client-ipc"; - // Make sure it's okay to store a bunch of these to mitigate #5880 #[test] fn callback_msg_size() { assert_eq!(std::mem::size_of::(), 56) } - - #[test] - #[cfg(target_os = "linux")] - fn dns_control() { - let actual = CliCommon::parse_from([EXE_NAME]); - assert!(matches!( - actual.dns_control, - DnsControlMethod::SystemdResolved - )); - - let actual = CliCommon::parse_from([EXE_NAME, "--dns-control", "disabled"]); - assert!(matches!(actual.dns_control, DnsControlMethod::Disabled)); - - let actual = CliCommon::parse_from([EXE_NAME, "--dns-control", "etc-resolv-conf"]); - assert!(matches!( - actual.dns_control, - DnsControlMethod::EtcResolvConf - )); - - let actual = CliCommon::parse_from([EXE_NAME, "--dns-control", "systemd-resolved"]); - assert!(matches!( - actual.dns_control, - DnsControlMethod::SystemdResolved - )); - - assert!(CliCommon::try_parse_from([EXE_NAME, "--dns-control", "invalid"]).is_err()); - } - - #[test] - #[cfg(target_os = "windows")] - fn dns_control() { - let actual = CliCommon::parse_from([EXE_NAME]); - assert!(matches!(actual.dns_control, DnsControlMethod::Nrpt)); - - let actual = CliCommon::parse_from([EXE_NAME, "--dns-control", "disabled"]); - assert!(matches!(actual.dns_control, DnsControlMethod::Disabled)); - - let actual = CliCommon::parse_from([EXE_NAME, "--dns-control", "nrpt"]); - assert!(matches!(actual.dns_control, DnsControlMethod::Nrpt)); - - assert!(CliCommon::try_parse_from([EXE_NAME, "--dns-control", "invalid"]).is_err()); - } }