fix(rust/client/windows): set our DNS resolvers on our interface (#6931)

Closes #6777
This commit is contained in:
Reactor Scram
2024-10-07 10:03:22 -05:00
committed by GitHub
parent 01530aa934
commit 9b93fc2a2c
6 changed files with 104 additions and 58 deletions

1
rust/Cargo.lock generated
View File

@@ -2344,6 +2344,7 @@ dependencies = [
"ip-packet",
"ip_network",
"ipconfig",
"itertools 0.13.0",
"known-folders",
"libc",
"mutants",

View File

@@ -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<Self> {
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) }?;

View File

@@ -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.

View File

@@ -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"] }

View File

@@ -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<Vec<IpAddr>>
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::<Vec<_>>()
.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();
}
}

View File

@@ -145,55 +145,9 @@ pub fn setup_stdout_logging() -> Result<LogFilterReloader> {
#[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::<ConnlibMsg>(), 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());
}
}