diff --git a/hive-priv/src/main.rs b/hive-priv/src/main.rs index b3c6ee54..817deba2 100644 --- a/hive-priv/src/main.rs +++ b/hive-priv/src/main.rs @@ -2967,28 +2967,104 @@ fn write_nspawn_flags( Ok(()) } -/// Path to the in-container DNS marker (the container's own `/etc`). -fn bridge_dns_marker_path(container: &str) -> String { - format!("/var/lib/nixos-containers/{container}/etc/hyperhive-bridge-dns") -} +/// Filename of the in-container DNS marker, in the container's own `/etc`. +const BRIDGE_DNS_MARKER: &std::ffi::CStr = c"hyperhive-bridge-dns"; /// Write the bridge-DNS marker the `hyperhive-isolated-dns` oneshot keys /// off. The marker file contains just the gateway IP. Always written: /// every container is isolated, so there is no host-netns case that /// wants the marker absent. fn write_bridge_dns_marker(container: &str, isolation: &NetworkIsolation) -> Result<()> { - let path = bridge_dns_marker_path(container); - // On a fresh install the container's `/etc` may not exist yet - // (rootfs not fully materialised before the first start), so - // `write` would fail with ENOENT. Create the parent dir first - // — it's the container's own `/etc`, which nixos-container - // populates on start; a pre-created dir + our marker persist. - if let Some(parent) = std::path::Path::new(&path).parent() { - std::fs::create_dir_all(parent) - .with_context(|| format!("create bridge-DNS marker dir {}", parent.display()))?; + let rootfs = PathBuf::from(format!("/var/lib/nixos-containers/{container}")); + write_bridge_dns_marker_in(&rootfs, &isolation.gateway_ip) +} + +/// Write `/etc/hyperhive-bridge-dns` without following a symlink. +/// +/// Root inside the container owns its `/etc`, and this write runs as host +/// root, where an absolute symlink planted in the container resolves against +/// the host's `/`. So this opens `etc` with `O_DIRECTORY|O_NOFOLLOW` and the +/// leaf relative to that fd with `O_NOFOLLOW`: a symlink at either step fails +/// the write (`ELOOP`) instead of redirecting it. It also opens the leaf +/// `O_NONBLOCK` and requires a regular file, so a FIFO cannot hang the helper +/// and a device node cannot take the write. +/// +/// On a fresh install the container's `/etc` may not exist yet (nixos-container +/// materialises the rootfs on the first start), so this creates it first; the +/// dir and the marker persist through that start. Mode 0644: the gateway IP +/// is not a secret, and 0644 matches what the default 0022 umask gave the +/// earlier plain write. +fn write_bridge_dns_marker_in(rootfs: &Path, gateway_ip: &str) -> Result<()> { + use std::io::Write as _; + use std::os::unix::ffi::OsStrExt as _; + use std::os::unix::fs::{OpenOptionsExt as _, PermissionsExt as _}; + + let etc_path = rootfs.join("etc"); + let path = etc_path.join(std::ffi::OsStr::from_bytes(BRIDGE_DNS_MARKER.to_bytes())); + // The rootfs dir is the container's `/`, which the container cannot + // replace, so creating it by path is safe. + std::fs::create_dir_all(rootfs) + .with_context(|| format!("create container rootfs {}", rootfs.display()))?; + let root = std::fs::OpenOptions::new() + .read(true) + .custom_flags(libc::O_DIRECTORY | libc::O_NOFOLLOW) + .open(rootfs) + .with_context(|| format!("open container rootfs {}", rootfs.display()))?; + // SAFETY: `root` is an open directory fd and the name a NUL-terminated + // literal. + if unsafe { libc::mkdirat(root.as_raw_fd(), c"etc".as_ptr(), 0o755) } != 0 { + let e = std::io::Error::last_os_error(); + if e.kind() != std::io::ErrorKind::AlreadyExists { + return Err(e).with_context(|| format!("create {}", etc_path.display())); + } } - std::fs::write(&path, format!("{}\n", isolation.gateway_ip)) - .with_context(|| format!("write bridge-DNS marker {path}"))?; + // SAFETY: as above; this checks the result before wrapping it. + let etc_fd = unsafe { + libc::openat( + root.as_raw_fd(), + c"etc".as_ptr(), + libc::O_RDONLY | libc::O_DIRECTORY | libc::O_NOFOLLOW | libc::O_CLOEXEC, + ) + }; + if etc_fd < 0 { + return Err(std::io::Error::last_os_error()) + .with_context(|| format!("open (no-follow) {}", etc_path.display())); + } + // SAFETY: `etc_fd` is a fresh, valid fd that nothing else owns. + let etc = unsafe { OwnedFd::from_raw_fd(etc_fd) }; + // SAFETY: `etc` is an open directory fd and the name a NUL-terminated + // literal; this checks the result before wrapping it. + let leaf_fd = unsafe { + libc::openat( + etc.as_raw_fd(), + BRIDGE_DNS_MARKER.as_ptr(), + libc::O_WRONLY + | libc::O_CREAT + | libc::O_TRUNC + | libc::O_NOFOLLOW + | libc::O_NONBLOCK + | libc::O_CLOEXEC, + 0o644 as libc::c_uint, + ) + }; + if leaf_fd < 0 { + return Err(std::io::Error::last_os_error()) + .with_context(|| format!("open (no-follow) {}", path.display())); + } + // SAFETY: `leaf_fd` is a fresh, valid fd that nothing else owns. + let mut file = std::fs::File::from(unsafe { OwnedFd::from_raw_fd(leaf_fd) }); + let is_file = file + .metadata() + .with_context(|| format!("stat {}", path.display()))? + .file_type() + .is_file(); + if !is_file { + bail!("{} is not a regular file", path.display()); + } + file.write_all(format!("{gateway_ip}\n").as_bytes()) + .with_context(|| format!("write bridge-DNS marker {}", path.display()))?; + file.set_permissions(std::fs::Permissions::from_mode(0o644)) + .with_context(|| format!("chmod 644 {}", path.display()))?; Ok(()) } @@ -3117,7 +3193,8 @@ mod tests { contains_secret_shaped_run, ensure_plain_filename, git_overlay_flags, limits_dropin_body, matrix_token_filename, partial_name, redact_secret_line, remove_marker_in, single_output_path, toplevel_attr, validate_account_name, validate_credential_name, - validate_snapshot_name, write_agent_dir_file, write_state_file_nofollow, + validate_snapshot_name, write_agent_dir_file, write_bridge_dns_marker_in, + write_state_file_nofollow, }; use std::path::PathBuf; use std::sync::atomic::{AtomicU32, Ordering}; @@ -3636,6 +3713,86 @@ mod tests { std::fs::remove_dir_all(&dir).ok(); } + /// The container plants a symlink where the bridge-DNS marker goes. + #[test] + fn bridge_dns_marker_refuses_symlink_leaf() { + let dir = scratch(); + let rootfs = dir.join("rootfs"); + std::fs::create_dir_all(rootfs.join("etc")).unwrap(); + let target = dir.join("target"); + std::fs::write(&target, "original").unwrap(); + std::os::unix::fs::symlink(&target, rootfs.join("etc/hyperhive-bridge-dns")).unwrap(); + + let res = write_bridge_dns_marker_in(&rootfs, "10.0.0.1"); + assert!(res.is_err(), "O_NOFOLLOW must refuse a symlink leaf"); + assert_eq!( + std::fs::read_to_string(&target).unwrap(), + "original", + "symlink target must be untouched" + ); + std::fs::remove_dir_all(&dir).ok(); + } + + /// The container replaces its whole `/etc` with a symlink, so the leaf + /// name resolves inside a directory of its choosing. + #[test] + fn bridge_dns_marker_refuses_symlink_etc() { + let dir = scratch(); + let rootfs = dir.join("rootfs"); + std::fs::create_dir_all(&rootfs).unwrap(); + let elsewhere = dir.join("elsewhere"); + std::fs::create_dir_all(&elsewhere).unwrap(); + let target = elsewhere.join("hyperhive-bridge-dns"); + std::fs::write(&target, "original").unwrap(); + std::os::unix::fs::symlink(&elsewhere, rootfs.join("etc")).unwrap(); + + let res = write_bridge_dns_marker_in(&rootfs, "10.0.0.1"); + assert!(res.is_err(), "O_NOFOLLOW must refuse a symlinked etc"); + assert_eq!( + std::fs::read_to_string(&target).unwrap(), + "original", + "file behind the symlinked etc must be untouched" + ); + std::fs::remove_dir_all(&dir).ok(); + } + + /// A FIFO at the leaf fails the write rather than blocking the helper. + #[test] + fn bridge_dns_marker_refuses_fifo_leaf() { + use std::os::unix::ffi::OsStrExt as _; + let dir = scratch(); + let rootfs = dir.join("rootfs"); + std::fs::create_dir_all(rootfs.join("etc")).unwrap(); + let fifo = rootfs.join("etc/hyperhive-bridge-dns"); + let c_fifo = std::ffi::CString::new(fifo.as_os_str().as_bytes()).unwrap(); + // SAFETY: `c_fifo` is a valid NUL-terminated path. + assert_eq!(unsafe { libc::mkfifo(c_fifo.as_ptr(), 0o644) }, 0); + + let res = write_bridge_dns_marker_in(&rootfs, "10.0.0.1"); + assert!(res.is_err(), "a FIFO leaf must be refused"); + std::fs::remove_dir_all(&dir).ok(); + } + + /// The control: a plain rootfs with no `etc` yet (a fresh install) gets + /// the dir and a 0644 marker, and a second write truncates. + #[test] + fn bridge_dns_marker_written_into_plain_etc() { + use std::os::unix::fs::PermissionsExt as _; + let dir = scratch(); + let rootfs = dir.join("rootfs"); + std::fs::create_dir_all(&rootfs).unwrap(); + + write_bridge_dns_marker_in(&rootfs, "10.0.0.1").unwrap(); + let path = rootfs.join("etc/hyperhive-bridge-dns"); + assert_eq!(std::fs::read_to_string(&path).unwrap(), "10.0.0.1\n"); + let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o644, "marker must stay readable in the container"); + + write_bridge_dns_marker_in(&rootfs, "10.9.8.7").unwrap(); + assert_eq!(std::fs::read_to_string(&path).unwrap(), "10.9.8.7\n"); + std::fs::remove_dir_all(&dir).ok(); + } + /// The runner-credential clear, in all three states that matter. The /// PRESENCE arm is the load-bearing one: an implementation that did nothing /// at all would pass the "absent is fine" arm perfectly, and the whole point