diff --git a/docs/backends/bwrap/bubblewrap-backend.md b/docs/backends/bwrap/bubblewrap-backend.md index 02d9584e1..04f6242c5 100644 --- a/docs/backends/bwrap/bubblewrap-backend.md +++ b/docs/backends/bwrap/bubblewrap-backend.md @@ -179,7 +179,16 @@ Common consequences of this default: compat symlink, and WSL's `/mnt/wsl/resolv.conf` is bound directly. Neither exposes host `/var` or `/mnt` contents. Hosts that point `/etc/resolv.conf` at some other custom location still need that target - listed in `readonlyPaths`. + listed in `readonlyPaths`. Bubblewrap retains a mount-configuration hint + for SDK callers, which `lxc-exec` also prints to stderr, when a direct, + absolute resolver symlink target has no overlapping known mount + destination. The check reads the link but does not resolve or stat its + target or policy paths; a missing file or symlink alias can make the hint + inapplicable. Check the link and consider listing its containing directory + in `readonlyPaths`. Relative targets, uncertain mounts, and synthetic + destinations such as `/tmp` and `/var/run` suppress the hint. Silence + does not prove DNS is available; the hint never changes mounts or the + run's outcome. Files in `/etc` that contain secrets (`/etc/shadow`, `/etc/sudoers`, `/etc/ssh/ssh_host_*_key`) are mode `0400` / `0640` `root` and remain diff --git a/src/mxc-sdk/src/backends/bubblewrap/common/bwrap_command.rs b/src/mxc-sdk/src/backends/bubblewrap/common/bwrap_command.rs index 34a023565..b5898895b 100644 --- a/src/mxc-sdk/src/backends/bubblewrap/common/bwrap_command.rs +++ b/src/mxc-sdk/src/backends/bubblewrap/common/bwrap_command.rs @@ -10,6 +10,8 @@ use std::collections::HashSet; use crate::mxc_common::filesystem_resolve::FsIntent; +#[cfg(any(target_os = "linux", test))] +use crate::mxc_common::models::ContainerPolicy; use crate::mxc_common::models::{ExecutionRequest, NetworkAction, ProxyAddress}; use crate::mxc_common::proxy_env::{is_managed_proxy_key, PROXY_SET_KEYS}; @@ -94,6 +96,52 @@ const BASELINE_RO_BIND_PATHS: &[&str] = &[ "/mnt/wsl/resolv.conf", ]; +/// An absolute resolver target with no overlapping known mount destination. +/// This lexical check does not resolve host or sandbox symlinks. +#[cfg(any(target_os = "linux", test))] +pub(crate) fn resolver_target_without_matching_mount( + policy: &ContainerPolicy, + target: &str, +) -> bool { + if target == "/" + || !target.starts_with('/') + || target.split('/').any(|part| part == "." || part == "..") + { + return false; + } + + let mut policy_mounts = policy + .readwrite_paths + .iter() + .chain(&policy.readonly_paths) + .chain(&policy.denied_paths) + .map(String::as_str); + for mount in BASELINE_RO_BIND_PATHS + .iter() + .copied() + .chain(["/dev", "/proc", "/tmp", "/var/run"]) + .chain(policy_mounts.clone()) + { + if !mount.starts_with('/') + || mount.split('/').any(|part| part == "." || part == "..") + || paths_overlap(target, mount) + { + return false; + } + } + + // A policy mount at the link (or an ancestor) replaces the host /etc + // entry, so the presence of its target alone would not explain DNS. + !policy_mounts.any(|mount| paths_overlap("/etc/resolv.conf", mount)) +} + +#[cfg(any(target_os = "linux", test))] +fn paths_overlap(a: &str, b: &str) -> bool { + let a = std::path::Path::new(a); + let b = std::path::Path::new(b); + a.starts_with(b) || b.starts_with(a) +} + /// The networking behavior Bubblewrap applies for one execution. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum ResolvedNetworkMode { @@ -445,6 +493,107 @@ mod tests { } } + #[test] + fn resolver_target_hint_requires_no_mount_overlap() { + let mut policy = ContainerPolicy::default(); + assert!(resolver_target_without_matching_mount( + &policy, + "/opt/private/resolv.conf" + )); + for no_warning in [ + "/run/systemd/resolve/resolv.conf", + "/var/run/NetworkManager/resolv.conf", + "/mnt/wsl/resolv.conf", + "/etc/resolv.conf", + "/tmp/resolv.conf", + ] { + assert!( + !resolver_target_without_matching_mount(&policy, no_warning), + "unexpected advisory for {no_warning}" + ); + } + policy.readonly_paths = vec!["/opt/private".into()]; + assert!(!resolver_target_without_matching_mount( + &policy, + "/opt/private/resolv.conf" + )); + for mount in ["/opt/private/", "/opt//private/"] { + policy.readonly_paths = vec![mount.into()]; + assert!( + !resolver_target_without_matching_mount(&policy, "/opt/private/resolv.conf"), + "{mount}" + ); + } + policy.readonly_paths = vec!["/opt/private/resolv.conf/child".into()]; + assert!(!resolver_target_without_matching_mount( + &policy, + "/opt/private/resolv.conf" + )); + policy.readonly_paths = vec!["/opt/private2".into()]; + assert!(resolver_target_without_matching_mount( + &policy, + "/opt/private/resolv.conf" + )); + policy.readonly_paths = vec!["/".into()]; + assert!(!resolver_target_without_matching_mount( + &policy, + "/opt/private/resolv.conf" + )); + } + + #[test] + fn resolver_diagnostic_recognizes_every_emitted_mount_destination() { + let mut with_policy = base_request(); + with_policy.policy.readwrite_paths = vec!["/home/user/data".into()]; + with_policy.policy.readonly_paths = vec!["/opt/private".into()]; + with_policy.policy.denied_paths = vec!["/srv/blocked".into()]; + for request in [base_request(), with_policy] { + let args = build_args(&request, None); + for triple in args.windows(3) { + if matches!( + triple[0].as_str(), + "--ro-bind-try" | "--ro-bind" | "--bind" | "--symlink" + ) { + assert!( + !resolver_target_without_matching_mount(&request.policy, &triple[2]), + "unrecognized mount destination: {triple:?}" + ); + } + } + for pair in args.windows(2) { + if matches!(pair[0].as_str(), "--dev" | "--proc" | "--tmpfs") { + assert!( + !resolver_target_without_matching_mount(&request.policy, &pair[1]), + "unrecognized virtual mount destination: {pair:?}" + ); + } + } + } + } + + #[test] + fn ambiguous_resolver_paths_and_shadowed_link_do_not_warn() { + let mut policy = ContainerPolicy::default(); + for target in ["relative/resolv.conf", "/opt/../secret/resolv.conf", "/"] { + assert!(!resolver_target_without_matching_mount(&policy, target)); + } + policy.denied_paths = vec!["/etc".into()]; + assert!(!resolver_target_without_matching_mount( + &policy, + "/opt/private/resolv.conf" + )); + policy.denied_paths = vec!["/etc//".into()]; + assert!(!resolver_target_without_matching_mount( + &policy, + "/opt/private/resolv.conf" + )); + policy.denied_paths = vec!["/alias/../etc".into()]; + assert!(!resolver_target_without_matching_mount( + &policy, + "/opt/private/resolv.conf" + )); + } + /// `process.env` resolution, which schema 0.9 gave a default block. mod env { use super::*; diff --git a/src/mxc-sdk/src/backends/bubblewrap/common/bwrap_runner.rs b/src/mxc-sdk/src/backends/bubblewrap/common/bwrap_runner.rs index edbcfaccb..ac262f081 100644 --- a/src/mxc-sdk/src/backends/bubblewrap/common/bwrap_runner.rs +++ b/src/mxc-sdk/src/backends/bubblewrap/common/bwrap_runner.rs @@ -145,6 +145,7 @@ impl SandboxBackend for BubblewrapScriptRunner { if let Err(msg) = check_pin_against_denied_hosts(request) { return Err(ScriptResponse::error(&msg)); } + warn_resolver_mount_hint(request, logger); let child = self.spawn_bwrap(request, &plan.files, egress_plan, logger, stdio)?; Ok(Box::new(BubblewrapSandboxProcess::new(child))) } @@ -350,6 +351,32 @@ fn warn_unreachable_v6_targets(plan: &network_rules::EgressPlan, logger: &mut Lo )); } +fn warn_resolver_mount_hint(request: &ExecutionRequest, logger: &mut Logger) { + warn_resolver_mount_hint_with(request, logger, |path| std::fs::read_link(path).ok()); +} + +fn warn_resolver_mount_hint_with( + request: &ExecutionRequest, + logger: &mut Logger, + read_link: impl FnOnce(&Path) -> Option, +) { + let Some(target) = read_link(Path::new("/etc/resolv.conf")) else { + return; + }; + let Some(target_str) = target.to_str() else { + return; + }; + if !bwrap_command::resolver_target_without_matching_mount(&request.policy, target_str) { + return; + } + logger.warning_line(&format!( + "WARNING: Bubblewrap: /etc/resolv.conf points to {target_str:?}, but no known \ + mount destination overlaps that path. DNS may fail if the link does not \ + resolve inside the sandbox; check the link and consider listing its \ + containing directory in filesystem.readonlyPaths." + )); +} + impl BubblewrapScriptRunner { /// Set up networking and spawn `bwrap`, returning a [`BwrapChild`] wrapped /// by the [`SandboxProcess`] handle. With [`StdioMode::Pipes`] the child's @@ -1176,6 +1203,97 @@ mod tests { } } + #[test] + fn resolver_hint_is_retained_for_unmatched_absolute_targets() { + use crate::mxc_common::logger::Mode; + + let mut request = base_request(); + let mut logger = Logger::new(Mode::Buffer); + let target = || Some(PathBuf::from("/opt/private/resolv.conf")); + warn_resolver_mount_hint_with(&request, &mut logger, |_| target()); + assert_eq!(logger.warnings().len(), 1); + assert!(logger.warnings()[0].contains("no known mount destination")); + assert!(logger.warnings()[0].contains("filesystem.readonlyPaths")); + assert!(logger.get_buffer().is_empty()); + + let mut logger = Logger::new(Mode::Buffer); + warn_resolver_mount_hint_with(&request, &mut logger, |_| None); + warn_resolver_mount_hint_with(&request, &mut logger, |_| { + Some(PathBuf::from("relative/resolv.conf")) + }); + warn_resolver_mount_hint_with(&request, &mut logger, |_| { + Some(PathBuf::from("/run/systemd/resolve/resolv.conf")) + }); + assert!(logger.warnings().is_empty()); + + request.policy.readonly_paths = vec!["/other".into()]; + warn_resolver_mount_hint_with(&request, &mut logger, |_| target()); + assert_eq!(logger.warnings().len(), 1); + let mut logger = Logger::new(Mode::Buffer); + request.policy.denied_paths = vec!["/etc".into()]; + warn_resolver_mount_hint_with(&request, &mut logger, |_| target()); + assert!(logger.warnings().is_empty()); + } + + #[test] + fn resolver_hint_checks_real_link_shapes_without_changing_host_etc() { + use crate::mxc_common::logger::Mode; + use std::os::unix::fs::symlink; + + let home = PathBuf::from(std::env::var_os("HOME").expect("HOME is required for this test")); + assert!(home.is_absolute()); + let dir = tempfile::tempdir_in(home).unwrap(); + let target = dir.path().join("resolver-target"); + std::fs::write(&target, "nameserver 127.0.0.1\n").unwrap(); + let link = dir.path().join("resolv.conf"); + symlink(&target, &link).unwrap(); + let warn = |request: &ExecutionRequest, logger: &mut Logger| { + warn_resolver_mount_hint_with(request, logger, |_| std::fs::read_link(&link).ok()); + }; + let mut request = base_request(); + let mut logger = Logger::new(Mode::Buffer); + warn(&request, &mut logger); + assert_eq!(logger.warnings().len(), 1); + assert!(logger.warnings()[0].contains(&target.display().to_string())); + assert!(logger.warnings()[0].contains("filesystem.readonlyPaths")); + assert!(logger.get_buffer().is_empty()); + + request.policy.readonly_paths = vec!["/other".into()]; + let mut unrelated_logger = Logger::new(Mode::Buffer); + warn(&request, &mut unrelated_logger); + assert_eq!(unrelated_logger.warnings().len(), 1); + + request.policy.readonly_paths = vec![dir.path().display().to_string()]; + let mut covered_logger = Logger::new(Mode::Buffer); + warn(&request, &mut covered_logger); + assert!(covered_logger.warnings().is_empty()); + + request.policy.readonly_paths.clear(); + std::fs::remove_file(&link).unwrap(); + std::fs::write(&link, "nameserver 127.0.0.1\n").unwrap(); + let mut regular_logger = Logger::new(Mode::Buffer); + warn(&request, &mut regular_logger); + assert!(regular_logger.warnings().is_empty()); + + std::fs::remove_file(&link).unwrap(); + let chained = dir.path().join("chained"); + symlink(&target, &chained).unwrap(); + let directory = dir.path().join("directory"); + std::fs::create_dir(&directory).unwrap(); + for (name, link_target, expected_hints) in [ + ("relative", PathBuf::from("resolver-target"), 0), + ("chained", chained, 1), + ("dangling", dir.path().join("missing"), 1), + ("directory", directory, 1), + ] { + symlink(link_target, &link).unwrap(); + let mut hint_logger = Logger::new(Mode::Buffer); + warn(&request, &mut hint_logger); + assert_eq!(hint_logger.warnings().len(), expected_hints, "{name}"); + std::fs::remove_file(&link).unwrap(); + } + } + /// A request carrying `alice:hunter2@` in its proxy URL, built the way a /// programmatic caller would rather than through the JSON parser. fn request_with_a_credential_bearing_proxy() -> ExecutionRequest {