vinzenz/hyperhive
vinzenz/hyperhive
Watch
0
0
Fork
You've already forked hyperhive
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

hive-priv: refuse symlinks when writing the bridge-DNS marker

Browse source 
The marker lives in the container's own /etc, which root inside the
container controls, and hive-priv writes it as host root. The write
followed a symlink at the leaf and at etc, so an absolute symlink
planted in the container redirected a host-root truncate-and-write to a
host path.

Open etc with O_DIRECTORY|O_NOFOLLOW and the leaf relative to that fd
with O_NOFOLLOW, and require a regular file (opened O_NONBLOCK so a
FIFO cannot hang the helper). The marker keeps mode 0644.

Closes #4667
This commit is contained in:
atlas 2026-09-24 11:13:34 +02:00 • committed by mara
commit 11cd2038c9
1 changed files with 174 additions and 17 deletions
Download patch file Download diff file Expand all files Collapse all files

189
hive-priv/src/main.rs
View file

@ -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)
}
std::fs::write(&path, format!("{}\n", isolation.gateway_ip))
.with_context(|| format!("write bridge-DNS marker {path}"))?;
/// Write `<rootfs>/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()));
}
}
// 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