From ca51bb9d54617c8bc82ef7a0651efb54b1cb82ab Mon Sep 17 00:00:00 2001 From: atlas Date: Sun, 15 Nov 2026 16:34:25 +0100 Subject: [PATCH] hive-priv: write a configured stable exe path into unit drop-ins HIVE_PRIV_EXE, set by the module to the /run/hive-rootfs-check link, is checked at start and used for the drop-in's ExecStartPre. Without it the running binary's path is used as before, and an unlinked one gets an error that names the cause. --- .../agent-state/persistence.md | 15 +- .../trust-boundary/hive-c0re-privilege.md | 2 +- hive-priv/src/container_config.rs | 178 +++++++++++++++--- hive-priv/src/main.rs | 1 + hive-priv/src/prestart.rs | 8 +- nix/host-modules/hive-priv.nix | 5 + nix/module-eval/core-toggle.nix | 4 +- 7 files changed, 172 insertions(+), 41 deletions(-) diff --git a/docs/4-implementation/agent-state/persistence.md b/docs/4-implementation/agent-state/persistence.md index 648c4ca81..d03445961 100644 --- a/docs/4-implementation/agent-state/persistence.md +++ b/docs/4-implementation/agent-state/persistence.md @@ -354,10 +354,17 @@ path and its type when: Absent paths pass, so a first start works. The check neither deletes nor repairs: the operator removes the offending entry, and the next start goes through. -The drop-in names the `hive-priv` binary that wrote it, so a drop-in never -points at a build without the sub-command. A drop-in written by an older -points at a build without the sub-command. That build's store path has a GC -root, `/nix/var/nix/gcroots/hyperhive-check-rootfs/h-`, written before the +The drop-in runs the binary named by `HIVE_PRIV_EXE`, which `hive-priv.nix` sets +to `/run/hive-rootfs-check`, the link to the current build. That path doesn't +change and lies outside the store, so a garbage collection, or a switch that +leaves `hive-priv.service` running, can't strand a drop-in on a missing binary. +`hive-priv` checks the value when it starts (an absolute path of letters, +digits and `/._+-` that exists) and refuses to start otherwise. Without +`HIVE_PRIV_EXE`, as under an older module, the drop-in names the running +binary's own path; once someone unlinks that file the write fails with an error +that says so, until `hive-priv.service` restarts. A store path named that way +has a GC root, `/nix/var/nix/gcroots/hyperhive-check-rootfs/h-`, written +before the drop-in and replaced on each write, so a garbage collection can't remove the binary a restart would run. The directory sits beside `gcroots/per-container`, which is the only gcroots path bound into a container, so only root writes it. diff --git a/docs/4-implementation/trust-boundary/hive-c0re-privilege.md b/docs/4-implementation/trust-boundary/hive-c0re-privilege.md index 2c2c4cd6d..c449cefb8 100644 --- a/docs/4-implementation/trust-boundary/hive-c0re-privilege.md +++ b/docs/4-implementation/trust-boundary/hive-c0re-privilege.md @@ -41,7 +41,7 @@ known operations; there is no arbitrary command pass-through: | `ReloadGatewayNginx` | `systemctl reload/start/reset-failed nginx` (host unit; the unit name is hard-coded, not a parameter) | | `WriteNspawnFlags` | write `/etc/nixos-containers/.conf` (bind-mount list, network isolation vars, and `--ephemeral` after the root's salvage). hive-priv first copies a stopped container's root to the agent's `work/`, then renames it aside (it deletes nothing), following no link the container could have planted; for an agent whose closure record sets `private_users` it also adds `--private-users=pick`, mounts every bind `owneridmap`, and once chowns the agent's `state`, `harness`, `work` and `claude` dirs (never `config`, following no link) to the data owner | | `EnsureAgentSocketDir` | `mkdirat` `/run/hive-agent/` `0751 root:root` under an `O_NOFOLLOW` parent fd; leaves an existing directory alone, refuses anything else; for an agent whose closure record sets `private_users` it then chowns the dir to the data owner | -| `WriteResourceLimits` | write `CPUQuota=`/`MemoryMax=`/`MemoryHigh=`/`MemorySwapMax=`/`CPUWeight=`/`IOWeight=` systemd drop-in for agent container, with an `ExecStartPre=` that runs `hive-priv check-rootfs ` (no-follow check of the root paths the start script touches; see persistence.md), and a GC root on the build it names | +| `WriteResourceLimits` | write `CPUQuota=`/`MemoryMax=`/`MemoryHigh=`/`MemorySwapMax=`/`CPUWeight=`/`IOWeight=` systemd drop-in for agent container, with an `ExecStartPre=` that runs `hive-priv check-rootfs ` (no-follow check of the root paths the start script touches; see persistence.md), and, when the binary it runs is a store path, a GC root on that build | | `RemoveServiceDropin` | remove `container@.service.d/` drop-in on destroy, and the GC root of the hive-priv build it named | | `DaemonReload` | `systemctl daemon-reload` | | `ControlInfraContainer` | `systemctl nginx.service` for the gateway — the `InfraContainer` enum is the allowlist, and serde rejects unknown names at the wire boundary (`hive-c0re`, the forge and the matrix have no variant, so no request can name them) | diff --git a/hive-priv/src/container_config.rs b/hive-priv/src/container_config.rs index 4132b7707..19c290e6a 100644 --- a/hive-priv/src/container_config.rs +++ b/hive-priv/src/container_config.rs @@ -7,8 +7,10 @@ //! Every op that names a container takes its system name and refuses anything //! [`validate_container_system_name`] does not accept. +use std::ffi::OsStr; use std::os::fd::{AsRawFd as _, FromRawFd as _, OwnedFd}; use std::path::{Path, PathBuf}; +use std::sync::OnceLock; use anyhow::{Context as _, Result, bail}; use hive_priv_sock::{ @@ -132,10 +134,11 @@ pub(crate) fn remove_service_dropin(container: &str) -> Result<(String, String)> /// /// The drop-in also carries the `ExecStartPre=` that runs /// `hive-priv check-rootfs ` ([`crate::prestart`]) ahead of -/// nixpkgs' start script. It names this process's own binary, so the drop-in -/// and the binary that implements the check are always the same build. The -/// build is rooted against garbage collection for as long as the drop-in -/// names it ([`crate::prestart::pin_binary`]). +/// nixpkgs' start script. It names the binary [`hive_priv_exe`] picks: the +/// configured link to the current build when [`EXE_ENV`] is set, else this +/// process's own binary. A store path there is rooted against garbage +/// collection for as long as the drop-in names it +/// ([`crate::prestart::pin_binary`]); the link needs no root. /// /// Two different kinds of setting land in the same file. `MemoryMax=` / /// `CPUQuota=` are hard caps that throttle even on an idle host; @@ -272,30 +275,76 @@ fn limits_dropin_body( ) } -/// This binary's absolute path, for the drop-in's `ExecStartPre=`. -/// -/// The path is the running binary's, not a fixed one, so a drop-in never names -/// a build without the sub-command. systemd expands `%` and `$` in command -/// lines and splits on spaces, so a path with any of them is refused rather -/// than escaped; store paths never have them. -fn hive_priv_exe() -> Result { - let exe = std::env::current_exe().context("resolve the hive-priv binary path")?; - let exe = exe - .to_str() - .filter(|p| { - p.starts_with('/') - && p.bytes() - .all(|b| b.is_ascii_alphanumeric() || b"/._+-".contains(&b)) - }) - .with_context(|| { - format!( - "hive-priv binary path {} is not safe in a unit file", - exe.display() - ) - })?; +/// The environment variable naming the binary the drop-in's `ExecStartPre=` +/// runs. The NixOS module sets it to a path that outlives garbage collection +/// and switches (`/run/hive-rootfs-check`, a link to the current package). +pub(crate) const EXE_ENV: &str = "HIVE_PRIV_EXE"; + +/// The binary [`EXE_ENV`] named at start, once [`init_exe`] has checked it. +static CONFIGURED_EXE: OnceLock = OnceLock::new(); + +/// Read and check [`EXE_ENV`] once at start, so a missing or unsafe value stops +/// the daemon instead of failing the first drop-in write. With the variable +/// unset, [`hive_priv_exe`] resolves the running binary's path at each write. +pub(crate) fn init_exe() -> Result<()> { + if let Some(value) = std::env::var_os(EXE_ENV) { + let exe = configured_exe(&value)?; + // `init_exe` runs once, so the cell is empty. + let _ = CONFIGURED_EXE.set(exe); + } + Ok(()) +} + +/// `value` of [`EXE_ENV`], when it is safe in a unit file and exists. +fn configured_exe(value: &OsStr) -> Result { + let exe = safe_unit_path(value.to_str().unwrap_or_default()) + .with_context(|| format!("{EXE_ENV}={} is not safe in a unit file", value.display()))?; + if !Path::new(exe).exists() { + bail!("{EXE_ENV}={exe} does not exist"); + } Ok(exe.to_owned()) } +/// `path` when it can be written into a command line as is. systemd expands +/// `%` and `$` in command lines and splits on spaces, so a path with any of +/// them is refused rather than escaped; store paths never have them. +fn safe_unit_path(path: &str) -> Option<&str> { + (path.starts_with('/') + && path + .bytes() + .all(|b| b.is_ascii_alphanumeric() || b"/._+-".contains(&b))) + .then_some(path) +} + +/// The binary the drop-in's `ExecStartPre=` runs: the one [`EXE_ENV`] named at +/// start, else this process's own. +fn hive_priv_exe() -> Result { + pick_exe(CONFIGURED_EXE.get(), std::env::current_exe()) +} + +/// [`hive_priv_exe`] with both sources as parameters; the running binary is +/// not looked at when `configured` is set. +fn pick_exe(configured: Option<&String>, current: std::io::Result) -> Result { + match configured { + Some(exe) => Ok(exe.clone()), + None => running_exe(current), + } +} + +/// The path of the running binary, as [`std::env::current_exe`] reported it. +/// Linux appends ` (deleted)` once the file is unlinked, which a unit file +/// can't carry, so that case gets its own error. +fn running_exe(current: std::io::Result) -> Result { + let exe = current.context("resolve the hive-priv binary path")?; + let shown = exe.display().to_string(); + if shown.ends_with(" (deleted)") { + bail!("hive-priv's own binary {shown} was unlinked; restart hive-priv.service"); + } + safe_unit_path(&shown) + .map(str::to_owned) + .with_context(|| format!("hive-priv binary path {shown} is not safe in a unit file")) +} + /// The `ExecStartPre=` command of the pre-start root check for `container`: /// the binary `exe`, then the sub-command and the name. fn rootfs_check_command(exe: &str, container: &str) -> String { @@ -806,12 +855,12 @@ pub(crate) fn write_container_etc_file_in(rootfs: &Path, name: &str, contents: & mod tests { use super::{ BindMount, CredentialMount, HiveFacts, NIX_DAEMON_SHADOW_FLAG, NetworkIsolation, NixBuilds, - Path, chosen_nix_builds, git_overlay_flags, handle_write_nspawn_flags, hive_priv_exe, - is_owned_var, limits_dropin_body, nspawn_flags, previous_nix_builds, private_users_line, - publish_file, remove_service_dropin, render_hive_facts, rootfs_check_command, - validate_bind_path, validate_bridge, validate_container_system_name, validate_cpu_quota, - validate_gateway_ip, validate_memory_max, write_bridge_dns_marker_in, - write_resource_limits, + Path, chosen_nix_builds, configured_exe, git_overlay_flags, handle_write_nspawn_flags, + hive_priv_exe, is_owned_var, limits_dropin_body, nspawn_flags, pick_exe, + previous_nix_builds, private_users_line, publish_file, remove_service_dropin, + render_hive_facts, rootfs_check_command, running_exe, validate_bind_path, validate_bridge, + validate_container_system_name, validate_cpu_quota, validate_gateway_ip, + validate_memory_max, write_bridge_dns_marker_in, write_resource_limits, }; use crate::shared::test_support::{entries, scratch}; @@ -1038,6 +1087,73 @@ mod tests { assert!(cmd.starts_with('/'), "{cmd}"); } + /// A configured link is used as written, even though it is not this + /// process's own binary, and the process's own path is never consulted. + #[test] + fn a_configured_exe_is_used_as_written() { + let dir = scratch(); + let link = dir.join("hive-rootfs-check"); + std::os::unix::fs::symlink(std::env::current_exe().unwrap(), &link).unwrap(); + let got = configured_exe(link.as_os_str()).unwrap(); + assert_eq!(got, link.to_str().unwrap()); + + let broken = Err(std::io::Error::other("no /proc")); + assert_eq!(pick_exe(Some(&got), broken).unwrap(), got); + let own = std::env::current_exe().unwrap(); + assert_eq!( + pick_exe(None, Ok(own.clone())).unwrap(), + own.to_str().unwrap() + ); + } + + /// A configured value that is unsafe in a unit file, relative, empty or + /// missing stops the start, each for its own reason; the existing link is + /// the control that a good value passes. + #[test] + fn a_bad_configured_exe_is_refused_at_start() { + use std::ffi::OsStr; + let dir = scratch(); + let good = dir.join("good"); + std::fs::write(&good, b"").unwrap(); + assert!(configured_exe(good.as_os_str()).is_ok()); + + for bad in [ + "/store/hive priv/bin/hive-priv", + "/s/hive-priv%h", + "/s/$x", + "relative/hive-priv", + "", + ] { + let err = configured_exe(OsStr::new(bad)).unwrap_err().to_string(); + assert!(err.contains("is not safe in a unit file"), "{bad:?}: {err}"); + } + let missing = dir.join("absent"); + let err = configured_exe(missing.as_os_str()).unwrap_err().to_string(); + assert!(err.contains("does not exist"), "{err}"); + } + + /// Without a configured value the running binary's path is used; once the + /// kernel marks it unlinked the error names the cause and the fix, and a + /// path unsafe for another reason keeps the generic message. + #[test] + fn the_running_exe_is_the_fallback_and_an_unlinked_one_is_named() { + use std::path::PathBuf; + let ok = PathBuf::from("/nix/store/aaa-hive-priv/bin/hive-priv"); + assert_eq!( + running_exe(Ok(ok)).unwrap(), + "/nix/store/aaa-hive-priv/bin/hive-priv" + ); + + let gone = PathBuf::from("/nix/store/aaa-hive-priv/bin/hive-priv (deleted)"); + let err = running_exe(Ok(gone)).unwrap_err().to_string(); + assert!(err.contains("was unlinked"), "{err}"); + assert!(err.contains("restart hive-priv.service"), "{err}"); + + let spaced = PathBuf::from("/nix/store/a b/hive-priv"); + let err = running_exe(Ok(spaced)).unwrap_err().to_string(); + assert!(err.contains("is not safe in a unit file"), "{err}"); + } + /// The mode lands exactly as asked, whatever the umask would have made /// of it (0666 under the usual 022 would otherwise come out 0644), and /// the requested owner is on the file. diff --git a/hive-priv/src/main.rs b/hive-priv/src/main.rs index 52381289f..fa878dc68 100644 --- a/hive-priv/src/main.rs +++ b/hive-priv/src/main.rs @@ -76,6 +76,7 @@ async fn main() -> Result<()> { // they land in victorialogs as raw byte-array spam otherwise. .with_ansi(false) .init(); + container_config::init_exe()?; let listener = socket_listener()?; if let Err(e) = files::remove_legacy_tmpfiles() { diff --git a/hive-priv/src/prestart.rs b/hive-priv/src/prestart.rs index b0a0683e2..0ac5d55ac 100644 --- a/hive-priv/src/prestart.rs +++ b/hive-priv/src/prestart.rs @@ -18,10 +18,10 @@ //! never followed. Nothing is deleted or repaired, so whatever is there stays //! for the operator. //! -//! The drop-in names the running hive-priv binary, which nothing else roots -//! once the system has moved on to a newer build. [`pin_binary`] keeps a GC -//! root on it for as long as the drop-in exists, and [`unpin_binary`] drops -//! the root with the drop-in. +//! With no configured binary the drop-in names the running hive-priv binary, +//! which nothing else roots once the system has moved on to a newer build. +//! [`pin_binary`] keeps a GC root on it for as long as the drop-in exists, and +//! [`unpin_binary`] drops the root with the drop-in. //! //! The entry point is [`run`]; [`check`] holds the rules. diff --git a/nix/host-modules/hive-priv.nix b/nix/host-modules/hive-priv.nix index e441599a2..f2ac6c937 100644 --- a/nix/host-modules/hive-priv.nix +++ b/nix/host-modules/hive-priv.nix @@ -137,6 +137,11 @@ in # The owner a namespaced agent's data moves to. Which agents are # namespaced is not set here: hive-c0re sends it with each start. HIVE_PRIV_USERNS_DATA_OWNER = toString userns.dataOwner; + # The binary an agent's drop-in `ExecStartPre=` runs. The tmpfiles link + # above points at the current package and never goes away, so a garbage + # collection or a switch that leaves this process running cannot leave a + # drop-in naming a binary that is gone. + HIVE_PRIV_EXE = rootfsCheck.hivePriv; }; serviceConfig = { ExecStart = "${controllerCfg.package}/bin/hive-priv"; diff --git a/nix/module-eval/core-toggle.nix b/nix/module-eval/core-toggle.nix index 69426c48d..029ce1f09 100644 --- a/nix/module-eval/core-toggle.nix +++ b/nix/module-eval/core-toggle.nix @@ -253,13 +253,15 @@ let { # Read back from the rendered unit and assertion list: a misspelt # variable or a dropped assertion fails here. - name = "hive-priv carries the userns data owner, no agent list, and the owner assertion passes"; + name = "hive-priv carries the userns data owner, no agent list, the stable exe link its tmpfiles rule creates, and the owner assertion passes"; ok = let e = allLocal.systemd.services.hive-priv.environment; in !(e ? HIVE_PRIV_USERNS_AGENTS) && e.HIVE_PRIV_USERNS_DATA_OWNER == "60000" + && e.HIVE_PRIV_EXE == "/run/hive-rootfs-check" + && lib.any (r: lib.hasPrefix "L+ /run/hive-rootfs-check " r) allLocal.systemd.tmpfiles.rules && lib.any ( a: lib.hasInfix "hive-controller.userns.dataOwner" a.message && a.assertion ) allLocal.assertions;