From 0fcd63dc5a513dd230babdd3fa4572c9b72ea8ec Mon Sep 17 00:00:00 2001 From: kerneyJ Date: Mon, 8 Jun 2026 11:49:23 -0700 Subject: [PATCH 1/4] Removed ephemeral Removes ephemeral from the VM tests 1. Causes test_basic_vm_smoke to fail(at least on my machine) 2. VMHarness cleans up instances after the tests --- tests/helpers/vm.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/helpers/vm.rs b/tests/helpers/vm.rs index 6bfebd2..ac17a6c 100644 --- a/tests/helpers/vm.rs +++ b/tests/helpers/vm.rs @@ -162,7 +162,6 @@ impl VmHarness { &image, &instance, "--vm", - "--ephemeral", "-c", "boot.autostart=false", "-c", From a9a93ab3bc76287759492f642e13ef1155bd0bcd Mon Sep 17 00:00:00 2001 From: kerneyJ Date: Fri, 5 Jun 2026 12:01:06 -0700 Subject: [PATCH 2/4] tests: e2e cases for short-name resolution under search domain Adds four #[ignore]'d tests pinning resolv.conf handling when libc's search-expansion would rewrite short mesh names before nftables can intercept them. Three docker e2e tests cover a pre-seeded `search` + `ndots:1` resolv.conf, the same via `docker run --dns-search`, and SIGTERM restoring the original. One incus vm test covers the systemd-resolved symlink layout, where the proxy must skip management and leave the symlink intact. All four fail against current main; the proxy fix lands in a follow-up commit. There are probably tests that I'm not thinking of(e.g. maybe a vm tests with Tailscale). Would love suggestions for more tests. --- tests/e2e.rs | 255 ++++++++++++++++++++++++++++++++++++++++ tests/helpers/docker.rs | 26 ++++ tests/vm.rs | 65 ++++++++++ 3 files changed, 346 insertions(+) diff --git a/tests/e2e.rs b/tests/e2e.rs index f1125a5..03d8fff 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -798,6 +798,261 @@ fn test_short_name_dns() { ); } +/// Regression test for the libc search-expansion bug. +/// +/// On a host with `search ` in /etc/resolv.conf and the default +/// `ndots:1`, libc rewrites a 0-dot query like `db` into `db.` inside +/// the calling process, before any packet is sent. nftables-level interception +/// cannot help — the packet has already been mangled. Without the proxy taking +/// ownership of /etc/resolv.conf, short mesh names never reach the mesh +/// resolver. +/// +/// This test pre-populates the container's /etc/resolv.conf with a search +/// directive before starting the proxy. If the fix works, the proxy snapshots +/// the original, installs a managed file with `ndots:0`, and the short name +/// resolves via mesh. +#[test] +#[ignore = "e2e test requiring docker"] +fn test_short_name_dns_with_search_domain() { + let mut section = TestSection::new(); + let harness = Harness::new(); + + // Inject a search directive into the container's /etc/resolv.conf before + // launching the proxy. Mirrors the campus-network case from the bug + // report. + let proxy_cmd = r#" + printf 'nameserver 1.1.1.1\nsearch example.com\noptions ndots:1\n' >/etc/resolv.conf + intermesh proxy -- caddy respond --listen :8080 "hostname=$(hostname)" + "#; + + let root = harness.launch("root", proxy_cmd).assert(); + let db = harness.launch("db", proxy_cmd).assert(); + + section.next("Wait for daemons to be ready"); + wait(|| root.exec(&["intermesh", "status"])).assert(); + wait(|| db.exec(&["intermesh", "status"])).assert(); + + section.next("Verify /etc/resolv.conf is managed by intermesh"); + let resolv = db.exec(&["cat", "/etc/resolv.conf"]).assert(); + assert!( + resolv.contains("Managed by intermesh-proxy"), + "resolv.conf is not managed: {resolv}" + ); + assert!( + resolv.contains("ndots:0"), + "managed resolv.conf missing ndots:0: {resolv}" + ); + assert!( + !resolv.contains("search example.com"), + "managed resolv.conf still has search directive: {resolv}" + ); + + section.next("Bootstrap mesh"); + let mesh = "test.mesh"; + let token = root + .exec(&[ + "intermesh", + "adhoc", + "init", + "--mesh", + mesh, + "--name", + &format!("root.{mesh}"), + "--quiet", + ]) + .assert(); + db.exec(&[ + "intermesh", + "adhoc", + "join", + &token, + "--name", + &format!("db.{mesh}"), + "--yes", + ]) + .assert(); + + section.next("Wait for gossip convergence"); + let root_imid = root.imid().assert(); + let db_imid = db.imid().assert(); + let expected_names = HashMap::from([ + ( + format!("root.{mesh}").parse().assert(), + HashSet::from([root_imid.clone()]), + ), + ( + format!("db.{mesh}").parse().assert(), + HashSet::from([db_imid.clone()]), + ), + ]); + let expected_ips = HashMap::from([ + (root_imid.clone(), HashSet::from([root.ip])), + (db_imid.clone(), HashSet::from([db.ip])), + ]); + wait_for_convergence(&[&root], &expected_names, &expected_ips); + + section.next("Short name resolves despite host search directive"); + let response = root.curl("http://db:8080/").assert(); + assert!( + response.contains("hostname="), + "short name failed with search directive present: {response}" + ); +} + +/// Same scenario as `test_short_name_dns_with_search_domain`, but the +/// search directive is injected via `docker run --dns-search` instead of +/// a shell `printf` inside the container. Docker writes its own resolv.conf +/// header and the `search` directive before our entrypoint runs, mirroring +/// production hosts on a corp VPN or campus network. +#[test] +#[ignore = "e2e test requiring docker"] +fn test_docker_dns_search_flag_is_handled() { + let mut section = TestSection::new(); + let harness = Harness::new(); + + let proxy_cmd = r#"intermesh proxy -- caddy respond --listen :8080 "hostname=$(hostname)""#; + let dns_search = ["--dns-search", "example.com"]; + let root = harness + .launch_with_run_args("root", proxy_cmd, &dns_search) + .assert(); + let db = harness + .launch_with_run_args("db", proxy_cmd, &dns_search) + .assert(); + + section.next("Wait for daemons to be ready"); + wait(|| root.exec(&["intermesh", "status"])).assert(); + wait(|| db.exec(&["intermesh", "status"])).assert(); + + section.next("Snapshot captured Docker's search directive"); + let snapshot = db + .exec(&["cat", "/var/lib/intermesh/resolv.conf.orig"]) + .assert(); + assert!( + snapshot.contains("search example.com"), + "snapshot should record Docker's search directive: {snapshot}" + ); + + section.next("/etc/resolv.conf is managed"); + let resolv = db.exec(&["cat", "/etc/resolv.conf"]).assert(); + assert!( + resolv.contains("Managed by intermesh-proxy"), + "resolv.conf not managed: {resolv}" + ); + assert!( + resolv.contains("ndots:0"), + "managed resolv.conf missing ndots:0: {resolv}" + ); + assert!( + !resolv.contains("search example.com"), + "managed resolv.conf still carries search directive: {resolv}" + ); + + section.next("Bootstrap mesh"); + let mesh = "test.mesh"; + let token = root + .exec(&[ + "intermesh", + "adhoc", + "init", + "--mesh", + mesh, + "--name", + &format!("root.{mesh}"), + "--quiet", + ]) + .assert(); + db.exec(&[ + "intermesh", + "adhoc", + "join", + &token, + "--name", + &format!("db.{mesh}"), + "--yes", + ]) + .assert(); + + section.next("Wait for gossip convergence"); + let root_imid = root.imid().assert(); + let db_imid = db.imid().assert(); + let expected_names = HashMap::from([ + ( + format!("root.{mesh}").parse().assert(), + HashSet::from([root_imid.clone()]), + ), + ( + format!("db.{mesh}").parse().assert(), + HashSet::from([db_imid.clone()]), + ), + ]); + let expected_ips = HashMap::from([ + (root_imid.clone(), HashSet::from([root.ip])), + (db_imid.clone(), HashSet::from([db.ip])), + ]); + wait_for_convergence(&[&root], &expected_names, &expected_ips); + + section.next("Short name resolves despite Docker's --dns-search"); + let response = root.curl("http://db:8080/").assert(); + assert!( + response.contains("hostname="), + "short name failed with --dns-search: {response}" + ); +} + +/// Verify that /etc/resolv.conf is restored to its original content on +/// graceful daemon shutdown. +#[test] +#[ignore = "e2e test requiring docker"] +fn test_resolv_conf_restored_on_shutdown() { + let mut section = TestSection::new(); + let harness = Harness::new(); + + // Pre-seed the container's resolv.conf, then run caddy as the long-lived + // main process. The daemon will be started in the background via + // start_daemon so the container survives daemon shutdown — letting us + // inspect the file after SIGTERM. + let original_resolv = "nameserver 1.1.1.1\nsearch example.com\noptions ndots:1\n"; + let setup_cmd = format!("printf '{original_resolv}' >/etc/resolv.conf && {CADDY}"); + let node = harness.launch("root", &setup_cmd).assert(); + + section.next("Start daemon with intercept enabled"); + node.start_daemon(&["--intercept"]).assert(); + + section.next("Confirm resolv.conf is managed mid-run"); + // start_daemon only waits for the admin server; the proxy task may not + // have reached install() yet. The "installed managed" log line fires + // after install() succeeds, so use it as the readiness signal. + wait(|| { + let log = node.exec(&["cat", "/tmp/intermesh-daemon.log"])?; + if !log.contains("installed managed /etc/resolv.conf") { + anyhow::bail!("resolv.conf not yet installed"); + } + Ok(()) + }) + .assert(); + let managed = node.exec(&["cat", "/etc/resolv.conf"]).assert(); + assert!( + managed.contains("Managed by intermesh-proxy"), + "expected managed file, got: {managed}" + ); + + section.next("Send SIGTERM to trigger graceful shutdown"); + node.exec(&["pkill", "-TERM", "intermesh"]).assert(); + + section.next("Wait for original resolv.conf to be restored"); + wait(|| { + let current = node.exec(&["cat", "/etc/resolv.conf"])?; + if current.contains("Managed by intermesh-proxy") { + anyhow::bail!("still managed: {current}"); + } + if !current.contains("search example.com") { + anyhow::bail!("search directive missing from restored file: {current}"); + } + Ok(()) + }) + .assert(); +} + /// Test that non-mesh DNS queries resolve through the upstream resolver. /// /// The client query is redirected to the proxy's DNS listener by nftables. The diff --git a/tests/helpers/docker.rs b/tests/helpers/docker.rs index 4914cdc..55e005a 100644 --- a/tests/helpers/docker.rs +++ b/tests/helpers/docker.rs @@ -69,6 +69,18 @@ impl Harness { } pub(crate) fn launch(&self, name: &str, command: &str) -> Result> { + self.launch_with_run_args(name, command, &[]) + } + + /// Launch with additional `docker run` flags inserted before the image. + /// Used by tests that need flags like `--dns-search` to influence what + /// Docker writes into the bind-mounted files (`/etc/resolv.conf`, etc.). + pub(crate) fn launch_with_run_args( + &self, + name: &str, + command: &str, + extra_run_args: &[&str], + ) -> Result> { let container = format!("{}-{name}", self.prefix); let e2e_image = e2e_image()?; @@ -95,6 +107,7 @@ impl Harness { "-e", "PATH=/usr/local/bin:/nix/bin", ]); + run.args(extra_run_args); let output = run.args([&e2e_image, "sh", "-c", command]).output()?; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr); @@ -220,6 +233,19 @@ impl Node<'_> { Ok(()) } + /// Restart the container. Docker re-runs the original entrypoint and + /// rebuilds `/etc/resolv.conf` / `/etc/hosts` / `/etc/hostname` fresh; + /// the writable layer (state files, snapshots) persists. + pub(crate) fn restart(&self) -> Result<()> { + let output = Command::new("docker") + .args(["restart", &self.container]) + .output()?; + if !output.status.success() { + bail!("{}", command_error(&output, &self.container)); + } + Ok(()) + } + pub(crate) fn kill_daemon(&self) -> Result<()> { let _ = self.exec(&["pkill", "-KILL", "intermesh"]); diff --git a/tests/vm.rs b/tests/vm.rs index 03a7f20..a674d64 100644 --- a/tests/vm.rs +++ b/tests/vm.rs @@ -36,6 +36,71 @@ fn test_basic_vm_smoke() { } } +/// systemd-resolved layout: `/etc/resolv.conf` is a symlink to +/// `/run/systemd/resolve/stub-resolv.conf`. The proxy must detect the +/// symlink in `install()` and skip management with a warning rather than +/// clobber it (or worse, write through the symlink into resolved's +/// runtime file). +/// +/// Pins the documented limitation: with the symlink in place, libc reads +/// search/ndots from resolved's stub file before nftables interception, +/// so short mesh names won't resolve. We assert the skip-and-warn +/// invariants; the short-name limitation is implied by the skip. +#[test] +#[ignore = "vm test requiring incus"] +fn test_systemd_resolved_active() { + let mut section = TestSection::new(); + let harness = VmHarness::new(); + let vm = harness.launch("resolved").assert(); + + section.next("Enable systemd-resolved with the standard stub symlink"); + vm.exec(&[ + "sh", + "-lc", + "systemctl enable --now systemd-resolved; \ + ln -sf /run/systemd/resolve/stub-resolv.conf /etc/resolv.conf; \ + test -L /etc/resolv.conf", + ]) + .assert(); + + section.next("Start intercepting daemon"); + vm.start_daemon(&["--intercept"]).assert(); + + section.next("Wait for the skip-on-symlink decision to be logged"); + // start_daemon only waits for the admin server; the proxy task may + // not have reached install() yet. Use the warn! line as the readiness + // signal for the symlink branch. + wait(|| { + let log = vm.daemon_logs()?; + if !log.contains("skipping resolv.conf management") { + anyhow::bail!("symlink skip warning not yet in log"); + } + Ok(()) + }) + .assert(); + + section.next("/etc/resolv.conf is still a symlink to the resolved stub"); + vm.exec(&["test", "-L", "/etc/resolv.conf"]).assert(); + let link_target = vm.exec(&["readlink", "/etc/resolv.conf"]).assert(); + assert_eq!(link_target, "/run/systemd/resolve/stub-resolv.conf"); + + section.next("Proxy did not write through the symlink"); + // Reading the symlink follows it; the sentinel would appear here if + // fs::write went through the link into stub-resolv.conf. + let content = vm.exec(&["cat", "/etc/resolv.conf"]).assert(); + assert!( + !content.contains("Managed by intermesh-proxy"), + "proxy wrote through the symlink: {content}" + ); + + section.next("No snapshot was taken"); + let result = vm.exec(&["test", "-e", "/var/lib/intermesh/resolv.conf.orig"]); + assert!( + result.is_err(), + "snapshot should not exist when target is a symlink" + ); +} + fn start_workload(vm: &Vm) -> Result { let vm_name = &vm.name; let name = format!("{vm_name}-workload"); From 013cc33417757e26497dd83d4491ffff58d7f6a6 Mon Sep 17 00:00:00 2001 From: kerneyJ Date: Fri, 5 Jun 2026 12:01:46 -0700 Subject: [PATCH 3/4] proxy: resolve short names by managing /etc/resolv.conf Snapshots the host's resolv.conf at startup and installs a managed file with ndots:0, so libc no longer rewrites short queries via the search directive before the mesh resolver sees them. --- Cargo.lock | 4 +- context/interfaces/src/proxy/mod.md | 3 + src/proxy/mod.rs | 36 +++- src/proxy/resolv_conf.rs | 290 ++++++++++++++++++++++++++++ 4 files changed, 330 insertions(+), 3 deletions(-) create mode 100644 src/proxy/resolv_conf.rs diff --git a/Cargo.lock b/Cargo.lock index 80a5067..c06b2b8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -252,9 +252,9 @@ checksum = "55248b47b0caf0546f7988906588779981c43bb1bc9d0c44087278f80cdb44ba" [[package]] name = "bitflags" -version = "2.10.0" +version = "2.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "812e12b5285cc515a9c72a5c1d3b6d46a19dac5acfef5265968c166106e31dd3" +checksum = "84d7ced0ae9557296835c32bf1b1e02b44c746701f898460fb000d7eaa84f00a" [[package]] name = "block-buffer" diff --git a/context/interfaces/src/proxy/mod.md b/context/interfaces/src/proxy/mod.md index 2958fcd..9cd7d6d 100644 --- a/context/interfaces/src/proxy/mod.md +++ b/context/interfaces/src/proxy/mod.md @@ -5,6 +5,9 @@ - Resolving mesh names from trusted derivation state. - Intercepting local traffic. - Proxying connections over Intermesh mTLS. +- Owning `/etc/resolv.conf` while running: snapshotting the original, + installing a managed file (`ndots:0`) so libc does not search-expand short + mesh names, and restoring on shutdown. ## Public interface ```rust diff --git a/src/proxy/mod.rs b/src/proxy/mod.rs index 9a4950c..fd1fb07 100644 --- a/src/proxy/mod.rs +++ b/src/proxy/mod.rs @@ -5,6 +5,7 @@ use std::convert::Infallible; use std::net::{IpAddr, Ipv4Addr}; +use std::path::PathBuf; use std::sync::Arc; use anyhow::{Context, Result}; @@ -38,12 +39,19 @@ use crate::verifier::IntermeshVerifier; mod dns; mod intercept; +mod resolv_conf; use dns::{create_resolver, Dns}; use intercept::{ bind_dns, bind_tcp, get_original_dst, nftables_clean, nftables_inject, EXTERNAL_PORT, PROXY_PORT, }; +use resolv_conf::ResolvConf; + +/// Default location of the host's resolver config. +const RESOLV_CONF_PATH: &str = "/etc/resolv.conf"; +/// Filename of the on-disk snapshot, placed alongside the daemon state file. +const RESOLV_CONF_SNAPSHOT: &str = "resolv.conf.orig"; // ============================================================================ // Proxy @@ -83,8 +91,9 @@ impl Handle { /// enabled, creates local service for host namespace interception and /// spawns HTTP CONNECT listener for incoming mTLS from remote peers. pub(crate) async fn run(self, cancel: CancellationToken) -> Result<()> { - // Clean up any stale rules from a previous run. + // Clean up any stale state from a previous run. nftables_clean(); + let resolv_conf = make_resolv_conf(&self.state)?; if !self.state.intercept { cancel.cancelled().await; @@ -96,7 +105,21 @@ impl Handle { let tcp_sock = bind_tcp(Ipv4Addr::LOCALHOST, PROXY_PORT).context("bind proxy socket")?; let ext_sock = bind_tcp(Ipv4Addr::UNSPECIFIED, EXTERNAL_PORT).context("bind external socket")?; + // Read upstream nameservers from /etc/resolv.conf *before* + // resolv_conf.install() overwrites the file. Otherwise we'd read back + // our own `nameserver 127.0.0.1` and forward queries to ourselves. let resolver = create_resolver(); + // Defer order matters: scopeguard fires LIFO. Register the resolv.conf + // restore first so nftables_clean (registered second) fires first on + // shutdown — global system rules drop ASAP, local file restore after. + resolv_conf + .install() + .context("install managed resolv.conf")?; + defer!({ + if let Err(e) = resolv_conf.restore() { + error!("restore resolv.conf failed: {e:#}"); + } + }); nftables_inject().context("inject nftables rules")?; defer!(nftables_clean()); @@ -322,6 +345,17 @@ impl Handle { // Utilities // ============================================================================ +/// Construct a `ResolvConf` for production use, with the snapshot placed in +/// the same directory as the daemon state file. +fn make_resolv_conf(state: &State) -> Result { + let snapshot = state + .state_file + .parent() + .context("state file has no parent directory")? + .join(RESOLV_CONF_SNAPSHOT); + Ok(ResolvConf::new(PathBuf::from(RESOLV_CONF_PATH), snapshot)) +} + /// Parse the target from an HTTP CONNECT request URI authority. fn parse_connect_target(uri: &Uri) -> Result<(Name, u16)> { let authority = uri.authority().context("missing authority")?.as_str(); diff --git a/src/proxy/resolv_conf.rs b/src/proxy/resolv_conf.rs new file mode 100644 index 0000000..c9839c1 --- /dev/null +++ b/src/proxy/resolv_conf.rs @@ -0,0 +1,290 @@ +//! Manage /etc/resolv.conf while the proxy runs. +//! +//! Apps use libc's resolver, which reads /etc/resolv.conf for the `search` and +//! `ndots` directives and applies them *inside the calling process* before any +//! packet is sent. nftables-level interception cannot affect that rewrite. On +//! a host with `search example.com` and the default `ndots:1`, libc expands a +//! short query like `db` into `db.example.com` before the proxy ever sees it, +//! and the mesh resolver never gets a chance to match the bare name. +//! +//! While the proxy runs, this module replaces /etc/resolv.conf with a minimal +//! file that disables search expansion. The original is snapshotted to disk so +//! it can be restored on shutdown (and on recovery after a crash). If the file +//! is a symlink (typically systemd-resolved managed), we skip management with +//! a warning rather than clobber it. + +use std::fs; +use std::io; +use std::os::unix::fs::PermissionsExt; +use std::path::{Path, PathBuf}; + +use anyhow::{Context, Result}; +use tracing::{info, warn}; + +/// Marker on the first line of our managed file. Used to detect whether +/// /etc/resolv.conf is still ours when it's time to restore, and to recover +/// from a prior run that crashed before restoring. +const SENTINEL: &str = "# Managed by intermesh-proxy. Do not edit; will be restored on stop."; + +pub(super) struct ResolvConf { + path: PathBuf, + snapshot: PathBuf, +} + +impl ResolvConf { + pub(super) fn new(path: PathBuf, snapshot: PathBuf) -> Self { + Self { path, snapshot } + } + + /// Snapshot the original file, then replace it with the managed content. + /// + /// If the current file is a symlink (systemd-resolved style), skip with a + /// warning rather than clobber it. If our sentinel is already present + /// (crashed prior run), restore first so we snapshot the true original + /// rather than our own managed content. + pub(super) fn install(&self) -> Result<()> { + if is_symlink(&self.path)? { + warn!( + "{} is a symlink (likely systemd-resolved); skipping resolv.conf \ + management - short mesh names may be search-expanded by libc", + self.path.display() + ); + return Ok(()); + } + + if has_sentinel(&self.path)? { + warn!("found managed resolv.conf from prior run; restoring before reinstall"); + self.restore()?; + } + + match fs::read(&self.path) { + Ok(content) => { + if let Some(parent) = self.snapshot.parent() { + fs::create_dir_all(parent) + .with_context(|| format!("create dir {}", parent.display()))?; + } + fs::write(&self.snapshot, content) + .with_context(|| format!("write snapshot {}", self.snapshot.display()))?; + } + Err(e) if e.kind() == io::ErrorKind::NotFound => { + // Original absent. Remove any stale snapshot so restore knows + // to delete the managed file rather than recreate from it. + let _ = fs::remove_file(&self.snapshot); + } + Err(e) => return Err(e).with_context(|| format!("read {}", self.path.display())), + } + + write_resolv(&self.path, managed_content().as_bytes())?; + info!("installed managed {}", self.path.display()); + Ok(()) + } + + /// Restore the original file. If the current file is no longer ours + /// (sentinel mismatch), leave it alone — something else has claimed it + /// and overwriting would lose the operator's intent. + pub(super) fn restore(&self) -> Result<()> { + let still_ours = match fs::read_to_string(&self.path) { + Ok(c) => c.starts_with(SENTINEL), + // File vanished mid-run; safe to recreate from snapshot. + Err(e) if e.kind() == io::ErrorKind::NotFound => true, + Err(e) => return Err(e).context("read resolv.conf during restore"), + }; + + if !still_ours { + warn!( + "{} no longer has the intermesh sentinel; leaving it alone", + self.path.display() + ); + let _ = fs::remove_file(&self.snapshot); + return Ok(()); + } + + match fs::read(&self.snapshot) { + Ok(content) => { + write_resolv(&self.path, &content)?; + fs::remove_file(&self.snapshot) + .with_context(|| format!("remove snapshot {}", self.snapshot.display()))?; + } + // No snapshot means the original was absent at install time. + Err(e) if e.kind() == io::ErrorKind::NotFound => match fs::remove_file(&self.path) { + Ok(()) => {} + Err(e) if e.kind() == io::ErrorKind::NotFound => {} + Err(e) => return Err(e).with_context(|| format!("remove {}", self.path.display())), + }, + Err(e) => { + return Err(e) + .with_context(|| format!("read snapshot {}", self.snapshot.display())); + } + } + info!("restored {}", self.path.display()); + Ok(()) + } +} + +fn is_symlink(path: &Path) -> Result { + match fs::symlink_metadata(path) { + Ok(md) => Ok(md.file_type().is_symlink()), + Err(e) if e.kind() == io::ErrorKind::NotFound => Ok(false), + Err(e) => Err(e).with_context(|| format!("stat {}", path.display())), + } +} + +fn has_sentinel(path: &Path) -> Result { + match fs::read_to_string(path) { + Ok(c) => Ok(c.starts_with(SENTINEL)), + Err(e) if e.kind() == io::ErrorKind::NotFound => Ok(false), + Err(e) => Err(e).with_context(|| format!("read {}", path.display())), + } +} + +/// Write contents to a resolv.conf-style file with 0644 mode. +/// +/// Non-atomic, matching what `NetworkManager` and dhclient do for this file. +/// Tempfile + rename would fail on Docker bind mounts, and a torn read of +/// /etc/resolv.conf during a millisecond window is not a real failure mode. +fn write_resolv(path: &Path, contents: &[u8]) -> Result<()> { + fs::write(path, contents).with_context(|| format!("write {}", path.display()))?; + fs::set_permissions(path, fs::Permissions::from_mode(0o644)) + .with_context(|| format!("chmod {}", path.display()))?; + Ok(()) +} + +fn managed_content() -> String { + // nftables redirects all port-53 traffic regardless of nameserver IP, so + // the nameserver value is cosmetic; ndots:0 is the load-bearing line. + format!("{SENTINEL}\nnameserver 127.0.0.1\noptions ndots:0\n") +} + +#[cfg(test)] +mod tests { + use super::*; + use std::os::unix::fs as unix_fs; + use tempfile::TempDir; + + struct Fixture { + _tmp: TempDir, + rc: ResolvConf, + path: PathBuf, + snapshot: PathBuf, + } + + fn fixture() -> Fixture { + let tmp = TempDir::new().unwrap(); + let path = tmp.path().join("resolv.conf"); + let snapshot = tmp.path().join("snap").join("resolv.conf.orig"); + let rc = ResolvConf::new(path.clone(), snapshot.clone()); + Fixture { + _tmp: tmp, + rc, + path, + snapshot, + } + } + + #[test] + fn install_then_restore_present_file() { + let f = fixture(); + let original = "nameserver 1.2.3.4\nsearch example.com\n"; + fs::write(&f.path, original).unwrap(); + + f.rc.install().unwrap(); + let installed = fs::read_to_string(&f.path).unwrap(); + assert!(installed.starts_with(SENTINEL)); + assert!(installed.contains("ndots:0")); + assert!(!installed.contains("search example.com")); + assert!(f.snapshot.exists()); + + f.rc.restore().unwrap(); + assert_eq!(fs::read_to_string(&f.path).unwrap(), original); + assert!(!f.snapshot.exists()); + } + + #[test] + fn install_then_restore_absent_file() { + let f = fixture(); + assert!(!f.path.exists()); + + f.rc.install().unwrap(); + assert!(fs::read_to_string(&f.path).unwrap().starts_with(SENTINEL)); + assert!(!f.snapshot.exists()); + + f.rc.restore().unwrap(); + assert!(!f.path.exists()); + } + + #[test] + fn restore_leaves_externally_replaced_file_alone() { + let f = fixture(); + fs::write(&f.path, "nameserver 1.2.3.4\n").unwrap(); + f.rc.install().unwrap(); + + let external = "nameserver 9.9.9.9\n"; + fs::write(&f.path, external).unwrap(); + + f.rc.restore().unwrap(); + assert_eq!(fs::read_to_string(&f.path).unwrap(), external); + assert!(!f.snapshot.exists()); + } + + #[test] + fn install_skips_symlink() { + let f = fixture(); + let target = f.path.parent().unwrap().join("real-resolv.conf"); + fs::write(&target, "nameserver 1.2.3.4\n").unwrap(); + unix_fs::symlink(&target, &f.path).unwrap(); + + f.rc.install().unwrap(); + assert!(!f.snapshot.exists()); + assert!(fs::symlink_metadata(&f.path) + .unwrap() + .file_type() + .is_symlink()); + assert_eq!(fs::read_to_string(&target).unwrap(), "nameserver 1.2.3.4\n"); + } + + #[test] + fn install_recovers_from_crashed_prior_run() { + let f = fixture(); + let original = "nameserver 1.2.3.4\n"; + fs::write(&f.path, original).unwrap(); + + // Simulate prior install + crash before restore. + f.rc.install().unwrap(); + + // Second install should restore the true original first, then snapshot + // it again — never snapshotting our own managed content. + f.rc.install().unwrap(); + f.rc.restore().unwrap(); + assert_eq!(fs::read_to_string(&f.path).unwrap(), original); + } + + #[test] + fn install_with_external_writer_during_crash() { + let f = fixture(); + fs::write(&f.path, "nameserver 1.2.3.4\n").unwrap(); + f.rc.install().unwrap(); + + // Crash, then someone fixes /etc/resolv.conf before we restart. + fs::write(&f.path, "nameserver 9.9.9.9\n").unwrap(); + + // No sentinel present, so install treats the new file as the original + // and restore returns it on shutdown. + f.rc.install().unwrap(); + f.rc.restore().unwrap(); + assert_eq!(fs::read_to_string(&f.path).unwrap(), "nameserver 9.9.9.9\n"); + } + + #[test] + fn restore_recreates_file_that_vanished() { + let f = fixture(); + let original = "nameserver 1.2.3.4\n"; + fs::write(&f.path, original).unwrap(); + f.rc.install().unwrap(); + + fs::remove_file(&f.path).unwrap(); + + f.rc.restore().unwrap(); + assert_eq!(fs::read_to_string(&f.path).unwrap(), original); + assert!(!f.snapshot.exists()); + } +} From 2fe175a8bbcbd3181a2695cbd73fd2790fbdedb1 Mon Sep 17 00:00:00 2001 From: kerneyJ Date: Thu, 11 Jun 2026 13:37:00 -0700 Subject: [PATCH 4/4] proxy: recover resolv.conf before resolver, roll back failed install Two fixes from review of the short-name resolution change: - Extract `recover()` and run it before `create_resolver()` (and before the intercept gate, so a stale managed file is cleaned up even when restarting without intercept). Previously recovery happened inside `install()`, after the resolver had already read our managed `nameserver 127.0.0.1` and would forward queries back to the proxy. - Make `install()` atomic with respect to host-global state: if writing the managed file fails partway (e.g. content lands but chmod fails), roll back to the snapshot before returning, since the caller registers the restore guard only after `install()` returns Ok. --- src/proxy/mod.rs | 6 ++++++ src/proxy/resolv_conf.rs | 42 ++++++++++++++++++++++++++++++++-------- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/proxy/mod.rs b/src/proxy/mod.rs index fd1fb07..9d73a65 100644 --- a/src/proxy/mod.rs +++ b/src/proxy/mod.rs @@ -94,6 +94,12 @@ impl Handle { // Clean up any stale state from a previous run. nftables_clean(); let resolv_conf = make_resolv_conf(&self.state)?; + // A prior run may have crashed with our managed file still installed. + // Restore the real upstream config before create_resolver() reads it, + // otherwise the resolver would forward queries to our own loopback + // nameserver. Runs regardless of intercept so a stale managed file + // never outlives the proxy. + resolv_conf.recover().context("recover resolv.conf")?; if !self.state.intercept { cancel.cancelled().await; diff --git a/src/proxy/resolv_conf.rs b/src/proxy/resolv_conf.rs index c9839c1..c790817 100644 --- a/src/proxy/resolv_conf.rs +++ b/src/proxy/resolv_conf.rs @@ -19,7 +19,7 @@ use std::os::unix::fs::PermissionsExt; use std::path::{Path, PathBuf}; use anyhow::{Context, Result}; -use tracing::{info, warn}; +use tracing::{error, info, warn}; /// Marker on the first line of our managed file. Used to detect whether /// /etc/resolv.conf is still ours when it's time to restore, and to recover @@ -36,12 +36,36 @@ impl ResolvConf { Self { path, snapshot } } + /// Recover from a prior run that crashed with our managed file still in + /// place: if the sentinel is present, restore the true original from the + /// snapshot. Idempotent; a no-op when no managed file is present. + /// + /// Must run before the upstream resolver is constructed. Otherwise the + /// resolver reads our managed `nameserver 127.0.0.1` and forwards queries + /// back to the proxy itself instead of the real upstream. + pub(super) fn recover(&self) -> Result<()> { + if is_symlink(&self.path)? { + return Ok(()); + } + if has_sentinel(&self.path)? { + warn!("found managed resolv.conf from prior run; restoring"); + self.restore()?; + } + Ok(()) + } + /// Snapshot the original file, then replace it with the managed content. /// /// If the current file is a symlink (systemd-resolved style), skip with a - /// warning rather than clobber it. If our sentinel is already present - /// (crashed prior run), restore first so we snapshot the true original + /// warning rather than clobber it. Recovers any managed file left by a + /// crashed prior run first (see `recover`) so we snapshot the true original /// rather than our own managed content. + /// + /// Atomic with respect to host-global state: if writing the managed file + /// fails partway (e.g. the content lands but the chmod fails), roll back to + /// the snapshot before returning, so a failed install never leaves a + /// managed file behind. The caller registers the restore guard only after + /// `install` returns `Ok`, so this guarantee must hold here. pub(super) fn install(&self) -> Result<()> { if is_symlink(&self.path)? { warn!( @@ -52,10 +76,7 @@ impl ResolvConf { return Ok(()); } - if has_sentinel(&self.path)? { - warn!("found managed resolv.conf from prior run; restoring before reinstall"); - self.restore()?; - } + self.recover()?; match fs::read(&self.path) { Ok(content) => { @@ -74,7 +95,12 @@ impl ResolvConf { Err(e) => return Err(e).with_context(|| format!("read {}", self.path.display())), } - write_resolv(&self.path, managed_content().as_bytes())?; + if let Err(e) = write_resolv(&self.path, managed_content().as_bytes()) { + if let Err(rollback) = self.restore() { + error!("rollback after failed resolv.conf install failed: {rollback:#}"); + } + return Err(e); + } info!("installed managed {}", self.path.display()); Ok(()) }