From c191975aabca82d075eaede2658e374b81e6a98a Mon Sep 17 00:00:00 2001 From: atlas Date: Wed, 28 Oct 2026 23:15:03 +0100 Subject: [PATCH] check every container's root before its start, through the container@ template --- docs/agent-lifecycle/persistence.md | 37 ++- docs/trust-boundary/security.md | 4 +- hive-priv-sock/src/lib.rs | 9 +- hive-priv/src/container_config.rs | 117 +--------- hive-priv/src/prestart.rs | 247 +++++--------------- hive-priv/src/shared.rs | 2 +- nix/host-modules/container-rootfs-check.nix | 44 ++++ nix/host-modules/default.nix | 1 + nix/module-eval/core-toggle.nix | 31 +++ nix/vm-tests/agent-ephemeral-rootfs.nix | 28 +-- 10 files changed, 180 insertions(+), 340 deletions(-) create mode 100644 nix/host-modules/container-rootfs-check.nix diff --git a/docs/agent-lifecycle/persistence.md b/docs/agent-lifecycle/persistence.md index 25be4475..75392956 100644 --- a/docs/agent-lifecycle/persistence.md +++ b/docs/agent-lifecycle/persistence.md @@ -385,35 +385,32 @@ never data. **Checked before every start.** nixpkgs' start script runs as host root on the root template, and its `mkdir -p`, `chmod`, `touch` and `cp --remove-destination` follow a link the container planted at the paths -they touch. The container's limits drop-in -(`container@h-.service.d/hyperhive-limits.conf`) therefore carries an -`ExecStartPre=` that runs `hive-priv check-rootfs h-` ahead of that -script, on systemd's own restarts too. It looks at each path with `lstat`, -never following a link, and fails the start with one line on stderr naming the -path and its type when: +they touch. The host's nix config therefore adds a drop-in to the `container@` +template (`container@.service.d/`, from +`nix/host-modules/container-rootfs-check.nix`) with an `ExecStartPre=` that +runs `hive-priv check-rootfs %i`. It applies to every container instance, the +agents and the swarm's service containers alike, and runs on systemd's own +restarts too. systemd reads a drop-in after the unit file and runs the +`ExecStartPre=` lines in that order, all of them before `ExecStart=`, so the +check runs before the start script touches the root. The unit file names the +`hive-priv` store path, so the system closure keeps that build alive. + +The check looks at each path with `lstat`, never following a link, and fails +the start with one line on stderr naming the path and its type when: - `usr`, `usr/bin`, `etc`, `var`, `var/lib`, `var/lib/private` or `root` exists and isn't a real directory; - `etc/machine-id` or `etc/resolv.conf` exists and isn't a regular file (`etc/os-release` may also be a link, which the script skips and a booted NixOS guest creates); -- the profile's `system` (`/nix/var/nix/profiles/per-container/h-/system`) +- the profile's `system` (`/nix/var/nix/profiles/per-container//system`) exists and isn't a link that resolves into `/nix/store`, or the `system` file inside the resolved directory isn't a regular file. -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 -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. -Destroying the container removes the drop-in and then its root; the build stays -until the next collection. A drop-in written by an older hive-priv has no -`ExecStartPre=` and no root, and the container starts as before. +The name `%i` selects both paths, so the check refuses a name that isn't +lowercase ascii, digits and hyphens. 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. **Disk use.** The copy shares blocks with the old root where it can: hive-priv clones each file (`FICLONE`), which on btrfs or xfs allocates metadata only, and diff --git a/docs/trust-boundary/security.md b/docs/trust-boundary/security.md index 3d7412e0..7f076c1d 100644 --- a/docs/trust-boundary/security.md +++ b/docs/trust-boundary/security.md @@ -263,8 +263,8 @@ 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 | | `EnsureAgentSocketDir` | `mkdirat` `/run/hive-agent/` `0751 root:root` under an `O_NOFOLLOW` parent fd; leaves an existing directory alone, refuses anything else | -| `WriteResourceLimits` | write `CPUQuota=`/`MemoryMax=`/`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 | -| `RemoveServiceDropin` | remove `container@.service.d/` drop-in on destroy, and the GC root of the hive-priv build it named | +| `WriteResourceLimits` | write `CPUQuota=`/`MemoryMax=`/`CPUWeight=`/`IOWeight=` systemd drop-in for agent container | +| `RemoveServiceDropin` | remove `container@.service.d/` drop-in on destroy | | `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) | | `SyncAgentTmpfiles` | unlink `/etc/tmpfiles.d/hyperhive-agents.conf` and return `Ok` | diff --git a/hive-priv-sock/src/lib.rs b/hive-priv-sock/src/lib.rs index 623fe470..d8e066fd 100644 --- a/hive-priv-sock/src/lib.rs +++ b/hive-priv-sock/src/lib.rs @@ -369,11 +369,7 @@ pub enum PrivRequest { /// Write `/run/systemd/system/container@.service.d/hyperhive-limits.conf` /// with `[Service]` carrying `MemoryMax=` / `CPUQuota=` (hard caps) and - /// `CPUWeight=` / `IOWeight=` (cgroup v2 relative shares, contention-only), - /// and an `ExecStartPre=` that runs `hive-priv check-rootfs ` - /// to refuse a start over a root with links where the start script - /// expects directories. The build that line names is rooted under - /// `/nix/var/nix/gcroots/hyperhive-check-rootfs/`. + /// `CPUWeight=` / `IOWeight=` (cgroup v2 relative shares, contention-only). /// Written by `lifecycle::set_resource_limits`. WriteResourceLimits { container: String, @@ -392,8 +388,7 @@ pub enum PrivRequest { io_weight: Option, }, - /// Remove `/run/systemd/system/container@.service.d/` if present, - /// and the GC root of the hive-priv build its `ExecStartPre=` named. + /// Remove `/run/systemd/system/container@.service.d/` if present. /// Called by `lifecycle::destroy` to clean up the resource-limits drop-in. RemoveServiceDropin { container: String }, diff --git a/hive-priv/src/container_config.rs b/hive-priv/src/container_config.rs index 197747c7..e5371e97 100644 --- a/hive-priv/src/container_config.rs +++ b/hive-priv/src/container_config.rs @@ -91,7 +91,7 @@ pub(crate) async fn handle_write_nspawn_flags_blocking( /// the manager included) and return the logical agent name after it. /// Anything else is refused, the swarm's forge and matrix containers /// included: they belong to the host that runs them, not to a hive. -pub(crate) fn validate_container_system_name(name: &str) -> Result<&str> { +fn validate_container_system_name(name: &str) -> Result<&str> { let Some(suffix) = name.strip_prefix(AGENT_PREFIX) else { bail!("container name {name:?} is not managed by hive"); }; @@ -100,14 +100,13 @@ pub(crate) fn validate_container_system_name(name: &str) -> Result<&str> { } /// `RemoveServiceDropin` — remove the container service's drop-in dir -/// if present (idempotent), then the GC root of the build it named. +/// if present (idempotent). pub(crate) fn remove_service_dropin(container: &str) -> Result<(String, String)> { validate_container_system_name(container)?; let dir = format!("/run/systemd/system/container@{container}.service.d"); if Path::new(&dir).exists() { std::fs::remove_dir_all(&dir).with_context(|| format!("remove {dir}"))?; } - crate::prestart::unpin_binary(container)?; Ok((String::new(), String::new())) } @@ -115,13 +114,6 @@ pub(crate) fn remove_service_dropin(container: &str) -> Result<(String, String)> /// container service's drop-in dir, together with a /// `ConditionPathIsDirectory=` guard on the agent's MCP runtime dir. /// -/// 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`]). -/// /// Two different kinds of setting land in the same file. `MemoryMax=` / /// `CPUQuota=` are hard caps that throttle even on an idle host; /// `CPUWeight=` / `IOWeight=` are cgroup v2 relative shares that only @@ -147,19 +139,7 @@ pub(crate) fn write_resource_limits( let dir = format!("/run/systemd/system/container@{container}.service.d"); std::fs::create_dir_all(&dir).with_context(|| format!("create {dir}"))?; let path = format!("{dir}/hyperhive-limits.conf"); - let exe = hive_priv_exe()?; - // Rooted before the drop-in names it, so the drop-in never names a build a - // collection can take. - crate::prestart::pin_binary(container, Path::new(&exe))?; - let rootfs_check = rootfs_check_command(&exe, container); - let content = limits_dropin_body( - &runtime_dir, - &rootfs_check, - memory_max, - cpu_quota, - cpu_weight, - io_weight, - ); + let content = limits_dropin_body(&runtime_dir, memory_max, cpu_quota, cpu_weight, io_weight); // Published whole: a `daemon-reload` between truncate and write would // otherwise load the unit with its limits and start guard missing. publish_file(Path::new(&path), content.as_bytes(), 0o644, None)?; @@ -195,10 +175,6 @@ const RESTART_SEC: u32 = 5; /// Render the body of `hyperhive-limits.conf`. /// -/// `rootfs_check` is the command line of the pre-start root check, the first -/// `[Service]` line: it runs before the nixpkgs start script and a non-zero -/// exit fails the start. -/// /// `[Unit]`: the condition is checked at start time — it skips (not fails) /// the unit when the MCP socket dir is absent, avoiding restart loops — /// plus the bounded start limit (see the constants above; `StartLimit*` are @@ -210,7 +186,6 @@ const RESTART_SEC: u32 = 5; /// existed — or one whose nix option is `null` — renders no weight lines. fn limits_dropin_body( runtime_dir: &str, - rootfs_check: &str, memory_max: &str, cpu_quota: &str, cpu_weight: Option, @@ -228,7 +203,6 @@ fn limits_dropin_body( StartLimitBurst={START_LIMIT_BURST}\n\ \n\ [Service]\n\ - ExecStartPre={rootfs_check}\n\ RestartSec={RESTART_SEC}\n\ MemoryMax={memory_max}\n\ CPUQuota={cpu_quota}\n\ @@ -236,36 +210,6 @@ 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() - ) - })?; - Ok(exe.to_owned()) -} - -/// 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 { - format!("{exe} {} {container}", crate::prestart::SUBCOMMAND) -} - /// `DaemonReload` — `systemctl daemon-reload` on the host. pub(crate) async fn daemon_reload() -> Result<(String, String)> { let out = Command::new("systemctl") @@ -624,15 +568,11 @@ pub(crate) fn write_container_etc_file_in(rootfs: &Path, name: &str, contents: & mod tests { use super::{ BindMount, HiveFacts, NetworkIsolation, git_overlay_flags, handle_write_nspawn_flags, - hive_priv_exe, limits_dropin_body, nspawn_flags, publish_file, remove_service_dropin, - render_hive_facts, rootfs_check_command, validate_container_system_name, - write_bridge_dns_marker_in, write_resource_limits, + limits_dropin_body, nspawn_flags, publish_file, remove_service_dropin, render_hive_facts, + validate_container_system_name, write_bridge_dns_marker_in, write_resource_limits, }; use crate::shared::test_support::{entries, scratch}; - /// A stand-in for the pre-start check's command line. - const CHECK: &str = "/store/hive-priv/bin/hive-priv check-rootfs h-iris"; - fn bind(container_path: &str) -> BindMount { BindMount { host_path: "/var/lib/hyperhive/whatever".to_owned(), @@ -690,21 +630,13 @@ mod tests { #[test] fn dropin_body_is_pinned_exactly() { assert_eq!( - limits_dropin_body( - "/run/hyperhive/agents/iris", - CHECK, - "4G", - "200%", - None, - None - ), + limits_dropin_body("/run/hyperhive/agents/iris", "4G", "200%", None, None), "[Unit]\n\ ConditionPathIsDirectory=/run/hyperhive/agents/iris\n\ StartLimitIntervalSec=600\n\ StartLimitBurst=3\n\ \n\ [Service]\n\ - ExecStartPre=/store/hive-priv/bin/hive-priv check-rootfs h-iris\n\ RestartSec=5\n\ MemoryMax=4G\n\ CPUQuota=200%\n" @@ -722,7 +654,7 @@ mod tests { ("512M", "50%", Some(10), Some(10)), ("infinity", "infinity", Some(10_000), None), ] { - let body = limits_dropin_body("/rt/x", CHECK, mem, cpu, cw, iw); + let body = limits_dropin_body("/rt/x", mem, cpu, cw, iw); let unit = body .split("[Service]") .next() @@ -742,55 +674,26 @@ mod tests { /// independently when `None`. #[test] fn weights_are_emitted_only_when_set() { - let both = limits_dropin_body("/rt/x", CHECK, "4G", "200%", Some(80), Some(80)); + let both = limits_dropin_body("/rt/x", "4G", "200%", Some(80), Some(80)); assert!( both.ends_with("CPUQuota=200%\nCPUWeight=80\nIOWeight=80\n"), "{both}" ); - let cpu_only = limits_dropin_body("/rt/x", CHECK, "4G", "200%", Some(80), None); + let cpu_only = limits_dropin_body("/rt/x", "4G", "200%", Some(80), None); assert!( cpu_only.ends_with("CPUQuota=200%\nCPUWeight=80\n"), "{cpu_only}" ); assert!(!cpu_only.contains("IOWeight"), "{cpu_only}"); - let io_only = limits_dropin_body("/rt/x", CHECK, "4G", "200%", None, Some(80)); + let io_only = limits_dropin_body("/rt/x", "4G", "200%", None, Some(80)); assert!( io_only.ends_with("CPUQuota=200%\nIOWeight=80\n"), "{io_only}" ); assert!(!io_only.contains("CPUWeight"), "{io_only}"); } - - /// The pre-start check is the first `[Service]` line, whatever else the - /// caller sets, and a drop-in without it is what an old hive-priv wrote. - #[test] - fn the_rootfs_check_leads_the_service_section_whatever_the_caller_passes() { - for (cw, iw) in [(None, None), (Some(80), Some(80))] { - let body = limits_dropin_body("/rt/x", CHECK, "4G", "200%", cw, iw); - let service = body - .split("[Service]\n") - .nth(1) - .expect("the body always has a [Service] section"); - assert_eq!( - service.lines().next(), - Some("ExecStartPre=/store/hive-priv/bin/hive-priv check-rootfs h-iris") - ); - assert_eq!(body.matches("ExecStartPre=").count(), 1, "{body}"); - } - } - - /// The command names an absolute path to this very binary, the - /// sub-command that binary implements and the container. - #[test] - fn the_check_command_names_this_binary_the_subcommand_and_the_container() { - let exe = std::env::current_exe().unwrap(); - let cmd = rootfs_check_command(&hive_priv_exe().unwrap(), "h-iris"); - assert_eq!(cmd, format!("{} check-rootfs h-iris", exe.display())); - assert!(cmd.starts_with('/'), "{cmd}"); - } - /// 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/prestart.rs b/hive-priv/src/prestart.rs index 5faccc75..5bfbfb8f 100644 --- a/hive-priv/src/prestart.rs +++ b/hive-priv/src/prestart.rs @@ -3,33 +3,28 @@ //! nixpkgs' `container@.service` start script runs as host root against the //! container's own root, which is persistent and writable by the container. //! `mkdir -p`, `chmod`, `touch` and `cp --remove-destination` there follow a -//! link the container planted at any of the names the script touches. The -//! per-container drop-in ([`crate::container_config`]) puts -//! `hive-priv check-rootfs ` ahead of that script as an -//! `ExecStartPre=`; a non-zero exit fails the start. The container is stopped +//! link the container planted at any of the names the script touches. A +//! drop-in of the `container@` template +//! (`nix/host-modules/container-rootfs-check.nix`) runs +//! `hive-priv check-rootfs %i` as an `ExecStartPre=` ahead of that script for +//! every instance; a non-zero exit fails the start. The container is stopped //! while this runs, so nothing changes under the walk. //! //! Every component is looked at with `symlink_metadata`: a link is reported, //! 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. -//! //! The entry point is [`run`]; [`check`] holds the rules. use std::fs::{FileType, Metadata}; -use std::os::unix::fs::{DirBuilderExt as _, symlink}; use std::path::{Path, PathBuf}; use anyhow::{Context as _, Result, bail}; -use crate::container_config::{container_rootfs, validate_container_system_name}; -use crate::shared::partial_name; +use crate::container_config::container_rootfs; +use crate::shared::validate_name_chars; -/// The sub-command the drop-in's `ExecStartPre=` names. +/// The sub-command the template drop-in's `ExecStartPre=` names. pub(crate) const SUBCOMMAND: &str = "check-rootfs"; /// The per-container nix profile directories on the host. @@ -64,74 +59,6 @@ const REGULAR_ONLY: [&str; 2] = ["etc/machine-id", "etc/resolv.conf"]; /// `/etc/static` once it has booted. const OS_RELEASE: &str = "etc/os-release"; -/// Where hive-priv roots the build named by each container's `ExecStartPre=`: -/// `` is a symlink to that build's store path. The directory is -/// under the host's `/nix/var/nix/gcroots`, not `per-container`, which is the -/// only part of it a container is given (`--bind=.../gcroots/per-container/` -/// in the nixpkgs start script), so a container can't reach it. -const GCROOTS: &str = "/nix/var/nix/gcroots/hyperhive-check-rootfs"; - -/// Root the store path of `exe` for `container`, replacing the root a previous -/// write left, so the build a drop-in names outlives a garbage collection for -/// as long as the drop-in does. One link per container, so they don't pile up. -pub(crate) fn pin_binary(container: &str, exe: &Path) -> Result<()> { - pin_binary_in(Path::new(GCROOTS), Path::new(STORE), container, exe) -} - -/// Drop the root [`pin_binary`] left for `container`, once its drop-in is gone. -/// A name that holds anything but a link is left alone and reported. -pub(crate) fn unpin_binary(container: &str) -> Result<()> { - unpin_binary_in(Path::new(GCROOTS), container) -} - -/// [`pin_binary`] with the roots directory and the store as parameters. -/// -/// A binary outside `store` can't be collected, so it gets no root. The link -/// is made under a temporary name and renamed over ``: `rename` -/// replaces whatever link is there without following it. -fn pin_binary_in(roots: &Path, store: &Path, container: &str, exe: &Path) -> Result<()> { - let Some(entry) = exe - .strip_prefix(store) - .ok() - .and_then(|rest| rest.components().next()) - else { - return Ok(()); - }; - let target = store.join(entry); - std::fs::DirBuilder::new() - .recursive(true) - .mode(0o755) - .create(roots) - .with_context(|| format!("create {}", roots.display()))?; - if !lstat(roots)?.is_some_and(|m| m.file_type().is_dir()) { - bail!("{} is not a real directory", roots.display()); - } - let tmp = roots.join(partial_name(container)); - symlink(&target, &tmp).with_context(|| format!("create {}", tmp.display()))?; - let root = roots.join(container); - std::fs::rename(&tmp, &root).map_err(|e| { - // Best effort: a leftover temp is a root too, but a harmless one. - let _ = std::fs::remove_file(&tmp); - anyhow::Error::new(e).context(format!("replace {}", root.display())) - }) -} - -/// [`unpin_binary`] with the roots directory as a parameter. -fn unpin_binary_in(roots: &Path, container: &str) -> Result<()> { - let root = roots.join(container); - match lstat(&root)? { - None => Ok(()), - Some(meta) if meta.file_type().is_symlink() => { - std::fs::remove_file(&root).with_context(|| format!("remove {}", root.display())) - } - Some(meta) => bail!( - "{} is {}, not a link: left alone", - root.display(), - kind(meta.file_type()) - ), - } -} - /// Handle `hive-priv ...` when the first argument names one. /// `None` means it is not a sub-command and the daemon starts. /// @@ -156,12 +83,21 @@ pub(crate) fn run(args: &[String]) -> Option { /// [`check`] on the host paths of `container`. fn check_container(container: &str) -> Result<()> { - validate_container_system_name(container)?; - check( - &container_rootfs(container), - &Path::new(PROFILES_ROOT).join(container).join("system"), - Path::new(STORE), - ) + let (root, system) = container_paths(container)?; + check(&root, &system, Path::new(STORE)) +} + +/// The container's root and its profile's `system` link on the host. +/// +/// The unit instance supplies the name and it selects both paths, so anything +/// but a plain container name is refused: lowercase ascii, digits and +/// hyphens. That holds no `/` and is never `.` or `..`. +fn container_paths(container: &str) -> Result<(PathBuf, PathBuf)> { + validate_name_chars(container)?; + Ok(( + container_rootfs(container), + Path::new(PROFILES_ROOT).join(container).join("system"), + )) } /// Check the paths the start script touches as root, returning an error that @@ -285,8 +221,8 @@ mod tests { use std::os::unix::fs::symlink; use std::path::PathBuf; - use super::{check, pin_binary_in, run, unpin_binary_in}; - use crate::shared::test_support::{entries, scratch}; + use super::{check, container_paths, run}; + use crate::shared::test_support::scratch; /// A scratch container: root, the profile link's path and a store with one /// toplevel in it. Nothing is linked yet. @@ -514,110 +450,53 @@ mod tests { assert!(target.is_dir()); } - /// A scratch store holding one build with a binary in it, a roots dir and - /// the binary's path. - fn pin_fixture() -> (PathBuf, PathBuf, PathBuf, PathBuf) { - let base = scratch(); - let store = base.join("store"); - let build = store.join("abc-hive-priv"); - std::fs::create_dir_all(build.join("bin")).unwrap(); - let exe = build.join("bin/hive-priv"); - std::fs::write(&exe, "").unwrap(); - (base.join("roots"), store, build, exe) + #[test] + fn a_plain_container_name_selects_its_root_and_profile() { + // Agent and non-agent names alike. + for name in ["h-iris", "hive-forge", "swarm-review-classifier"] { + let (root, system) = container_paths(name).unwrap(); + assert_eq!( + root, + PathBuf::from(format!("/var/lib/nixos-containers/{name}")) + ); + assert_eq!( + system, + PathBuf::from(format!("/nix/var/nix/profiles/per-container/{name}/system")) + ); + } } #[test] - fn the_root_points_at_the_store_path_the_drop_in_names() { - let (roots, store, build, exe) = pin_fixture(); - pin_binary_in(&roots, &store, "h-iris", &exe).unwrap(); - let root = roots.join("h-iris"); - assert!(std::fs::symlink_metadata(&root).unwrap().is_symlink()); - // The build, not the binary inside it. - assert_eq!(std::fs::read_link(&root).unwrap(), build); - assert_eq!(std::fs::read_dir(&roots).unwrap().count(), 1); + fn a_name_that_is_not_a_plain_container_name_is_refused() { + for name in [ + "", + ".", + "..", + "../h-iris", + "a/b", + "/etc", + "h-iris/", + "H-iris", + "a.b", + "a b", + ] { + assert!(container_paths(name).is_err(), "{name:?} was accepted"); + } } - #[test] - fn a_rewrite_replaces_the_root_and_leaves_no_temp() { - let (roots, store, build, exe) = pin_fixture(); - let newer = store.join("def-hive-priv"); - std::fs::create_dir_all(newer.join("bin")).unwrap(); - pin_binary_in(&roots, &store, "h-iris", &exe).unwrap(); - pin_binary_in(&roots, &store, "h-iris", &newer.join("bin/hive-priv")).unwrap(); - pin_binary_in(&roots, &store, "h-bob", &exe).unwrap(); - assert_eq!(std::fs::read_link(roots.join("h-iris")).unwrap(), newer); - assert_eq!(std::fs::read_link(roots.join("h-bob")).unwrap(), build); - assert_eq!(entries(&roots), ["h-bob", "h-iris"]); - } - - #[test] - fn a_link_planted_at_the_root_name_is_replaced_not_followed() { - let (roots, store, build, exe) = pin_fixture(); - let victim = victim(); - std::fs::write(victim.join("keep"), "data").unwrap(); - std::fs::create_dir_all(&roots).unwrap(); - symlink(&victim, roots.join("h-iris")).unwrap(); - pin_binary_in(&roots, &store, "h-iris", &exe).unwrap(); - assert_eq!(std::fs::read_link(roots.join("h-iris")).unwrap(), build); - assert_eq!( - std::fs::read_to_string(victim.join("keep")).unwrap(), - "data" - ); - assert_eq!(std::fs::read_dir(&victim).unwrap().count(), 1); - } - - #[test] - fn a_roots_dir_that_is_a_link_is_refused() { - let (roots, store, _, exe) = pin_fixture(); - let victim = victim(); - symlink(&victim, &roots).unwrap(); - let msg = format!( - "{:#}", - pin_binary_in(&roots, &store, "h-iris", &exe).unwrap_err() - ); - assert!(msg.contains("is not a real directory"), "{msg}"); - assert_eq!(std::fs::read_dir(&victim).unwrap().count(), 0); - } - - #[test] - fn a_binary_outside_the_store_gets_no_root() { - let (roots, store, _, _) = pin_fixture(); - let elsewhere = scratch().join("hive-priv"); - pin_binary_in(&roots, &store, "h-iris", &elsewhere).unwrap(); - assert!(!roots.exists()); - } - - #[test] - fn unpinning_removes_the_container_root_and_nothing_else() { - let (roots, store, build, exe) = pin_fixture(); - pin_binary_in(&roots, &store, "h-iris", &exe).unwrap(); - pin_binary_in(&roots, &store, "h-bob", &exe).unwrap(); - unpin_binary_in(&roots, "h-iris").unwrap(); - assert_eq!(entries(&roots), ["h-bob"]); - // Already gone: fine. The build it named is untouched. - unpin_binary_in(&roots, "h-iris").unwrap(); - assert!(build.join("bin/hive-priv").is_file()); - } - - #[test] - fn unpinning_leaves_a_name_that_is_not_a_link_alone() { - let (roots, _, _, _) = pin_fixture(); - std::fs::create_dir_all(roots.join("h-iris")).unwrap(); - std::fs::write(roots.join("h-iris/keep"), "").unwrap(); - let msg = format!("{:#}", unpin_binary_in(&roots, "h-iris").unwrap_err()); - assert!(msg.contains("left alone"), "{msg}"); - assert!(roots.join("h-iris/keep").exists()); - } #[test] fn only_the_check_rootfs_sub_command_is_taken() { assert_eq!(run(&[]), None); assert_eq!(run(&["serve".to_string()]), None); - // Wrong arity and a name that is not an agent container are refused, - // whatever is on disk. + // Wrong arity and a name that selects a path outside the containers + // directory are refused, whatever is on disk. assert_eq!(run(&["check-rootfs".to_string()]), Some(1)); - assert_eq!( - run(&["check-rootfs".to_string(), "not-hive".to_string()]), - Some(1) - ); + for name in ["", "..", "a/b"] { + assert_eq!( + run(&["check-rootfs".to_string(), name.to_string()]), + Some(1), + "{name:?}" + ); + } } } diff --git a/hive-priv/src/shared.rs b/hive-priv/src/shared.rs index a8ae7332..0c06f35e 100644 --- a/hive-priv/src/shared.rs +++ b/hive-priv/src/shared.rs @@ -75,7 +75,7 @@ pub(crate) fn ensure_plain_filename(who: &str, filename: &str) -> Result<()> { /// The leading dot and the `.partial` suffix are load-bearing: a reader /// globbing on a published name's prefix, or systemd reading `*.conf` out of /// `tmpfiles.d` and drop-in dirs, cannot pick up a half-written temp. -pub(crate) fn partial_name(filename: &str) -> String { +fn partial_name(filename: &str) -> String { use std::sync::atomic::{AtomicU64, Ordering}; static SEQ: AtomicU64 = AtomicU64::new(0); let n = SEQ.fetch_add(1, Ordering::Relaxed); diff --git a/nix/host-modules/container-rootfs-check.nix b/nix/host-modules/container-rootfs-check.nix new file mode 100644 index 00000000..3006df81 --- /dev/null +++ b/nix/host-modules/container-rootfs-check.nix @@ -0,0 +1,44 @@ +# The pre-start check of a container's root, for every `container@` instance. +# +# nixpkgs' start script runs as host root against the container's own root, +# which is persistent and writable by the container, and its `mkdir`, `chmod`, +# `touch` and `cp` follow links the container planted there. A drop-in on the +# `container@` template runs `hive-priv check-rootfs %i` as an `ExecStartPre=` +# before the script (`hive-priv/src/prestart.rs` has the rules). It covers the +# agents' imperative instances and the declarative ones (hive-forge, +# hive-matrix, the swarm services), each of which has a unit file of its own: +# systemd reads the template's drop-in directory for every instance. +# +# Order: systemd parses a drop-in after the unit file, so this line follows the +# unit file's own `ExecStartPre=` (nixpkgs' `preStart`, which touches no path +# under the root), and `ExecStart=` runs only after every `ExecStartPre=` +# succeeded. +# +# The drop-in is a unit-file package rather than `systemd.services."container@"` +# because that attribute renders the template's own unit file, which a +# declarative instance's unit file replaces. +{ + config, + lib, + pkgs, + ... +}: +let + dropin = pkgs.writeTextFile { + name = "hyperhive-container-rootfs-check"; + destination = "/lib/systemd/system/container@.service.d/10-hyperhive-rootfs-check.conf"; + text = '' + [Service] + ExecStartPre=${config.services.hyperhive.c0re.package}/bin/hive-priv check-rootfs %i + ''; + }; +in +{ + config = lib.mkIf config.boot.enableContainers { + systemd.packages = [ dropin ]; + + # The per-agent drop-ins this check replaced rooted a hive-priv build per + # agent here. Boot is the one time no such drop-in is loaded. + systemd.tmpfiles.rules = [ "R! /nix/var/nix/gcroots/hyperhive-check-rootfs - - - - -" ]; + }; +} diff --git a/nix/host-modules/default.nix b/nix/host-modules/default.nix index 17039af3..89f19214 100644 --- a/nix/host-modules/default.nix +++ b/nix/host-modules/default.nix @@ -22,6 +22,7 @@ ./hive-matrix-service.nix ./hive-matrix.nix ./hive-network.nix + ./container-rootfs-check.nix ./hive-priv.nix ./hive-priv-cache.nix ./hive-squid.nix diff --git a/nix/module-eval/core-toggle.nix b/nix/module-eval/core-toggle.nix index c2c99f8e..eed0f856 100644 --- a/nix/module-eval/core-toggle.nix +++ b/nix/module-eval/core-toggle.nix @@ -1497,6 +1497,37 @@ let && (sc.ProtectKernelLogs or false) && (sc.ProtectKernelModules or false); } + { + # Every `container@` instance starts through the template's drop-in, the + # agents' and the swarm services' alike, so a host with containers has it + # whether or not it runs a hive. A host with none has no `container@` + # unit and gets no drop-in. Only the rendered line is pinned: eval cannot + # show where systemd reads the file from. The misspelled name is the + # control that the lookup finds nothing when nothing is there. + name = "the container@ template drop-in runs the root check on every host with containers and on no other"; + ok = + let + dropinText = + name: m: + let + hits = lib.filter (p: (p.name or "") == name) m.systemd.packages; + in + if builtins.length hits == 1 then (builtins.head hits).text else ""; + runsCheck = + name: m: + let + want = "ExecStartPre=${m.services.hyperhive.c0re.package}/bin/hive-priv check-rootfs %i"; + lines = lib.splitString "\n" (dropinText name m); + in + lib.elem (builtins.unsafeDiscardStringContext want) (map builtins.unsafeDiscardStringContext lines); + check = "hyperhive-container-rootfs-check"; + in + runsCheck check allLocal + && runsCheck check shared.servicesHere + && !(runsCheck "${check}-misspelled" allLocal) + && !(bare.systemd.units ? "container@.service") + && !(runsCheck check bare); + } ] # Starting a unit that stays `active (exited)` is a no-op, so a timer aimed # at one fires and fetches nothing. diff --git a/nix/vm-tests/agent-ephemeral-rootfs.nix b/nix/vm-tests/agent-ephemeral-rootfs.nix index 74a651aa..a6c685e3 100644 --- a/nix/vm-tests/agent-ephemeral-rootfs.nix +++ b/nix/vm-tests/agent-ephemeral-rootfs.nix @@ -13,10 +13,10 @@ # forwarder (nix/agent-modules/otel.nix) reads it, and the host can read it; # - a package installed into the user's nix profile is still there, and on # PATH, after a restart; -# - a link planted at `root` in the template refuses the next start through -# the limits drop-in's `ExecStartPre=` check (whose build has a GC root), -# leaves the link's target untouched, and the start goes through once the -# link is gone. +# - a link planted at `root` in the root template refuses the next start through +# the `ExecStartPre=` check in the `container@` template drop-in +# (nix/host-modules/container-rootfs-check.nix), leaves the link's target +# untouched, and the start goes through once the link is gone. # # The agent's journal forwarder itself (an OTLP push to the hive's collector) is # not booted here; the module-eval suite covers its wiring. @@ -69,6 +69,7 @@ pkgs.testers.runNixOSTest { { imports = [ hiveStubs + ../host-modules/container-rootfs-check.nix ../host-modules/hive-priv.nix ]; virtualisation.memorySize = 2048; @@ -233,21 +234,10 @@ pkgs.testers.runNixOSTest { with subtest("a link planted in the root refuses the start and nothing is touched"): stop() - # What hive-c0re does besides WriteNspawnFlags: the limits drop-in, - # which carries the pre-start check, and the runtime dir its condition - # names. - host.succeed(f"mkdir -p /run/hyperhive/agents/{AGENT}") - for req in [ - {"op": "write_resource_limits", "container": MACHINE, "memory_max": "1G", "cpu_quota": "200%"}, - {"op": "daemon_reload"}, - ]: - out = host.succeed(f"echo '{json.dumps(req)}' | socat - UNIX-CONNECT:/run/hive/priv.sock") - assert json.loads(out.strip().splitlines()[-1])["ok"] is True, out - dropin = host.succeed(f"cat /run/systemd/system/container@{MACHINE}.service.d/hyperhive-limits.conf") - assert "ExecStartPre=/nix/store/" in dropin and f"check-rootfs {MACHINE}\n" in dropin, dropin - # The build the drop-in names is rooted against garbage collection. - build = "/".join(dropin.split("ExecStartPre=")[1].split()[0].split("/")[:4]) - assert host.succeed(f"readlink /nix/var/nix/gcroots/hyperhive-check-rootfs/{MACHINE}").strip() == build, build + # The pre-start check is a drop-in of the `container@` template, so the + # unit carries it with no per-container drop-in written. + unit = host.succeed(f"systemctl cat container@{MACHINE}") + assert "bin/hive-priv check-rootfs %i\n" in unit, unit # `root` is a link to a directory outside the root, which the start # script would `chmod 0700` as host root.