diff --git a/docs/agent-lifecycle/persistence.md b/docs/agent-lifecycle/persistence.md index 3043a128..c3daa839 100644 --- a/docs/agent-lifecycle/persistence.md +++ b/docs/agent-lifecycle/persistence.md @@ -382,6 +382,32 @@ first. A failed salvage leaves the root where it was and the container starts without the flag (hive-priv logs the error), so a failure costs the switch, 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: + +- `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`) + 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 +hive-priv has no `ExecStartPre=` and the container starts as before. + The journal still ships. The agent's collector (`nix/agent-modules/otel.nix`) reads the guest's own `/var/log/journal` and pushes it over OTLP, so no journal link to the host takes part; nspawn skips `--link-journal=try-guest` diff --git a/docs/trust-boundary/security.md b/docs/trust-boundary/security.md index 7f076c1d..d8323985 100644 --- a/docs/trust-boundary/security.md +++ b/docs/trust-boundary/security.md @@ -263,7 +263,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 | | `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 | +| `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) | | `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) | diff --git a/hive-priv-sock/src/lib.rs b/hive-priv-sock/src/lib.rs index d8e066fd..68f57e31 100644 --- a/hive-priv-sock/src/lib.rs +++ b/hive-priv-sock/src/lib.rs @@ -369,7 +369,10 @@ 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). + /// `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. /// Written by `lifecycle::set_resource_limits`. WriteResourceLimits { container: String, diff --git a/hive-priv/src/container_config.rs b/hive-priv/src/container_config.rs index 2984ac1d..be2ed2ab 100644 --- a/hive-priv/src/container_config.rs +++ b/hive-priv/src/container_config.rs @@ -65,7 +65,7 @@ pub(crate) fn handle_write_nspawn_flags( /// 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. -fn validate_container_system_name(name: &str) -> Result<&str> { +pub(crate) 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"); }; @@ -88,6 +88,11 @@ 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. +/// /// 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 @@ -113,7 +118,15 @@ 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 content = limits_dropin_body(&runtime_dir, memory_max, cpu_quota, cpu_weight, io_weight); + let rootfs_check = rootfs_check_command(container)?; + let content = limits_dropin_body( + &runtime_dir, + &rootfs_check, + 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)?; @@ -149,6 +162,10 @@ 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 @@ -160,6 +177,7 @@ 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, @@ -177,6 +195,7 @@ 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\ @@ -184,6 +203,31 @@ fn limits_dropin_body( ) } +/// The `ExecStartPre=` command of the pre-start root check for `container`: +/// this binary's absolute path, then the sub-command and the name. +/// +/// 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 rootfs_check_command(container: &str) -> 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(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") @@ -543,10 +587,14 @@ mod tests { use super::{ BindMount, HiveFacts, NetworkIsolation, git_overlay_flags, handle_write_nspawn_flags, 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, + rootfs_check_command, 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(), @@ -604,13 +652,21 @@ mod tests { #[test] fn dropin_body_is_pinned_exactly() { assert_eq!( - limits_dropin_body("/run/hyperhive/agents/iris", "4G", "200%", None, None), + limits_dropin_body( + "/run/hyperhive/agents/iris", + CHECK, + "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" @@ -628,7 +684,7 @@ mod tests { ("512M", "50%", Some(10), Some(10)), ("infinity", "infinity", Some(10_000), None), ] { - let body = limits_dropin_body("/rt/x", mem, cpu, cw, iw); + let body = limits_dropin_body("/rt/x", CHECK, mem, cpu, cw, iw); let unit = body .split("[Service]") .next() @@ -648,20 +704,20 @@ mod tests { /// independently when `None`. #[test] fn weights_are_emitted_only_when_set() { - let both = limits_dropin_body("/rt/x", "4G", "200%", Some(80), Some(80)); + let both = limits_dropin_body("/rt/x", CHECK, "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", "4G", "200%", Some(80), None); + let cpu_only = limits_dropin_body("/rt/x", CHECK, "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", "4G", "200%", None, Some(80)); + let io_only = limits_dropin_body("/rt/x", CHECK, "4G", "200%", None, Some(80)); assert!( io_only.ends_with("CPUQuota=200%\nIOWeight=80\n"), "{io_only}" @@ -669,6 +725,34 @@ mod tests { 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("h-iris").unwrap(); + 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/main.rs b/hive-priv/src/main.rs index e09c4793..2fb357a8 100644 --- a/hive-priv/src/main.rs +++ b/hive-priv/src/main.rs @@ -20,7 +20,8 @@ //! dispatch table. Each op family's handlers and validation live in their //! own module: [`container`] (lifecycle), [`container_config`] (nspawn flags, //! systemd drop-ins), [`rootfs`] (the salvage of a container's root before it -//! turns ephemeral), [`storage`] (btrfs subvolumes, quotas, snapshots, disk +//! turns ephemeral), [`prestart`] (the check of a container's root that runs +//! before each start, as a sub-command), [`storage`] (btrfs subvolumes, quotas, snapshots, disk //! measurement), [`files`] (pause marker, socket dir, legacy tmpfiles), //! [`infra`] (gateway nginx, infra containers) and [`closure`] (downloads from //! the swarm cache). Helpers more than one family uses are in [`shared`]. @@ -30,6 +31,7 @@ mod container; mod container_config; mod files; mod infra; +mod prestart; mod rootfs; mod shared; mod storage; @@ -44,6 +46,13 @@ use tokio::net::{UnixListener, UnixStream}; #[tokio::main] async fn main() -> Result<()> { + // `hive-priv check-rootfs ` is a container unit's `ExecStartPre=`, + // not the daemon: it answers with an exit code and never touches the socket. + let args: Vec = std::env::args().skip(1).collect(); + if let Some(code) = prestart::run(&args) { + std::process::exit(code); + } + tracing_subscriber::fmt() .with_env_filter( tracing_subscriber::EnvFilter::try_from_default_env().unwrap_or_else(|_| "info".into()), diff --git a/hive-priv/src/prestart.rs b/hive-priv/src/prestart.rs new file mode 100644 index 00000000..f4428830 --- /dev/null +++ b/hive-priv/src/prestart.rs @@ -0,0 +1,454 @@ +//! The pre-start check of a container's root, run by systemd as root. +//! +//! 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 +//! 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 entry point is [`run`]; [`check`] holds the rules. + +use std::fs::{FileType, Metadata}; +use std::path::{Path, PathBuf}; + +use anyhow::{Context as _, Result, bail}; + +use crate::container_config::{container_rootfs, validate_container_system_name}; + +/// The sub-command the drop-in's `ExecStartPre=` names. +pub(crate) const SUBCOMMAND: &str = "check-rootfs"; + +/// The per-container nix profile directories on the host. +const PROFILES_ROOT: &str = "/nix/var/nix/profiles/per-container"; + +/// The nix store: where the profile's `system` link has to end up. +const STORE: &str = "/nix/store"; + +/// Directories the start script creates or `chmod`s under the root. Each, +/// with all its parents, has to be a real directory when it exists. +const DIRS: [&str; 7] = [ + "usr", + "usr/bin", + "etc", + "var", + "var/lib", + "var/lib/private", + "root", +]; + +/// Files under the root the start script `touch`es or replaces. +/// +/// `machine-id` and `resolv.conf` have to be regular files. A link at +/// `machine-id` that dangles makes `touch` create its target, and no guest +/// writes a link there. The script's `cp --remove-destination` replaces a link +/// at `resolv.conf` rather than following it, but the guest keeps a regular +/// file there too (`nix/agent-modules/network.nix`), so a link is not expected. +const REGULAR_ONLY: [&str; 2] = ["etc/machine-id", "etc/resolv.conf"]; + +/// The one file allowed to be a link as well: the script skips it when +/// `-h` says it is one, and a NixOS guest has `etc/os-release` as a link into +/// `/etc/static` once it has booted. +const OS_RELEASE: &str = "etc/os-release"; + +/// Handle `hive-priv ...` when the first argument names one. +/// `None` means it is not a sub-command and the daemon starts. +/// +/// Prints one line to stderr and returns exit code 1 on a violation, so the +/// reason lands in the journal next to the failed unit. +pub(crate) fn run(args: &[String]) -> Option { + if args.first().map(String::as_str) != Some(SUBCOMMAND) { + return None; + } + let result = match args { + [_, container] => check_container(container), + _ => Err(anyhow::anyhow!("usage: hive-priv {SUBCOMMAND} ")), + }; + Some(match result { + Ok(()) => 0, + Err(e) => { + eprintln!("hive-priv {SUBCOMMAND}: refusing to start: {e:#}"); + 1 + } + }) +} + +/// [`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), + ) +} + +/// Check the paths the start script touches as root, returning an error that +/// names the first path to hold something it must not. +/// +/// `root` is the container's root, `system` its profile link +/// (`//system`) and `store` the nix store. +/// +/// A path that does not exist passes: a first start has nothing yet, and the +/// script creates it. +fn check(root: &Path, system: &Path, store: &Path) -> Result<()> { + for dir in DIRS { + check_dir(root, dir)?; + } + for file in REGULAR_ONLY { + check_file(root, file, false)?; + } + check_file(root, OS_RELEASE, true)?; + check_system(system, store) +} + +/// `symlink_metadata`, with `None` for a path that does not exist. The one +/// place the walk touches the file system, so nothing in it can follow a link. +fn lstat(path: &Path) -> Result> { + match std::fs::symlink_metadata(path) { + Ok(meta) => Ok(Some(meta)), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(e) => Err(e).with_context(|| format!("stat {}", path.display())), + } +} + +/// What a file type is, for the error line. +fn kind(ty: FileType) -> &'static str { + if ty.is_symlink() { + "a symlink" + } else if ty.is_dir() { + "a directory" + } else if ty.is_file() { + "a regular file" + } else { + "not a regular file or directory" + } +} + +/// `rel` under `root` and every parent below `root` must be a real directory +/// or absent. A link or a file in the way is an error. +fn check_dir(root: &Path, rel: &str) -> Result<()> { + let mut path = root.to_path_buf(); + for part in rel.split('/') { + path.push(part); + let Some(meta) = lstat(&path)? else { + return Ok(()); + }; + if !meta.file_type().is_dir() { + bail!( + "{} is {}, not a directory", + path.display(), + kind(meta.file_type()) + ); + } + } + Ok(()) +} + +/// `rel` under `root` must be absent or a regular file, or a link when +/// `link_ok`. Its parents are covered by [`check_dir`] on `etc`. +fn check_file(root: &Path, rel: &str, link_ok: bool) -> Result<()> { + let path = root.join(rel); + let Some(meta) = lstat(&path)? else { + return Ok(()); + }; + let ty = meta.file_type(); + if ty.is_file() || (link_ok && ty.is_symlink()) { + return Ok(()); + } + bail!("{} is {}, not a regular file", path.display(), kind(ty)) +} + +/// The profile's `system` must be absent, or a link that resolves to a +/// directory in `store` whose `system` is a regular file (the script reads it +/// as root). The link is followed on purpose here: nix points it at a +/// generation link, which points at the store. +fn check_system(system: &Path, store: &Path) -> Result<()> { + let Some(meta) = lstat(system)? else { + return Ok(()); + }; + if !meta.file_type().is_symlink() { + bail!( + "{} is {}, not a symlink into {}", + system.display(), + kind(meta.file_type()), + store.display() + ); + } + let target: PathBuf = std::fs::canonicalize(system) + .with_context(|| format!("{} does not resolve", system.display()))?; + let store = + std::fs::canonicalize(store).with_context(|| format!("resolve {}", store.display()))?; + if !target.starts_with(&store) || target == store { + bail!( + "{} resolves to {}, outside {}", + system.display(), + target.display(), + store.display() + ); + } + let file = target.join("system"); + match lstat(&file)? { + Some(meta) if meta.file_type().is_file() => Ok(()), + Some(meta) => bail!( + "{} is {}, not a regular file", + file.display(), + kind(meta.file_type()) + ), + None => bail!("{} is missing", file.display()), + } +} + +#[cfg(test)] +mod tests { + use std::os::unix::fs::symlink; + use std::path::PathBuf; + + use super::{check, 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. + struct Fixture { + root: PathBuf, + system: PathBuf, + store: PathBuf, + toplevel: PathBuf, + } + + impl Fixture { + fn new() -> Self { + let base = scratch(); + let store = base.join("store"); + let toplevel = store.join("abc-nixos-system"); + std::fs::create_dir_all(&toplevel).unwrap(); + std::fs::write(toplevel.join("system"), "x86_64-linux").unwrap(); + let root = base.join("root"); + std::fs::create_dir_all(&root).unwrap(); + let profile = base.join("profile"); + std::fs::create_dir_all(&profile).unwrap(); + Self { + root, + system: profile.join("system"), + store, + toplevel, + } + } + + /// A root as a booted guest leaves it, and a profile that resolves + /// into the store through a generation link. + fn clean() -> Self { + let f = Self::new(); + for dir in ["usr/bin", "etc", "var/lib/private", "root"] { + std::fs::create_dir_all(f.root.join(dir)).unwrap(); + } + std::fs::write(f.root.join("etc/machine-id"), "").unwrap(); + std::fs::write(f.root.join("etc/resolv.conf"), "nameserver 1.1.1.1\n").unwrap(); + symlink("/etc/static/os-release", f.root.join("etc/os-release")).unwrap(); + let generation = f.system.with_file_name("system-1-link"); + symlink(&f.toplevel, &generation).unwrap(); + symlink("system-1-link", &f.system).unwrap(); + f + } + + fn check(&self) -> anyhow::Result<()> { + check(&self.root, &self.system, &self.store) + } + + /// The error text of a check that has to fail. + fn refused(&self) -> String { + format!("{:#}", self.check().unwrap_err()) + } + } + + /// A target outside every fixture dir, for planted links to point at. + fn victim() -> PathBuf { + let dir = scratch(); + let path = dir.join("victim"); + std::fs::create_dir_all(&path).unwrap(); + path + } + + #[test] + fn a_clean_root_passes() { + Fixture::clean().check().unwrap(); + } + + #[test] + fn a_first_start_with_nothing_there_passes() { + let f = Fixture::new(); + f.check().unwrap(); + // The other half of the control: an empty root with the profile + // missing is the first-create case, and the profile dir is empty. + assert!(!f.system.exists()); + } + + #[test] + fn a_link_at_any_directory_is_refused_and_named() { + for dir in [ + "usr", + "usr/bin", + "etc", + "var", + "var/lib", + "var/lib/private", + "root", + ] { + let f = Fixture::clean(); + let path = f.root.join(dir); + // Replace the real directory by a link to an outside one. Parents + // of nested ones are real, so the link sits exactly at `dir`. + std::fs::remove_dir_all(&path).unwrap(); + symlink(victim(), &path).unwrap(); + let msg = f.refused(); + assert!( + msg.contains(&format!("{} is a symlink", path.display())), + "{dir}: {msg}" + ); + } + } + + #[test] + fn a_file_where_a_directory_belongs_is_refused() { + let f = Fixture::clean(); + std::fs::remove_dir_all(f.root.join("root")).unwrap(); + std::fs::write(f.root.join("root"), "").unwrap(); + let msg = f.refused(); + assert!( + msg.contains("root is a regular file, not a directory"), + "{msg}" + ); + } + + #[test] + fn a_missing_directory_below_a_real_one_passes() { + let f = Fixture::new(); + std::fs::create_dir_all(f.root.join("var")).unwrap(); + f.check().unwrap(); + } + + #[test] + fn a_link_at_machine_id_is_refused_even_when_dangling() { + let f = Fixture::clean(); + let path = f.root.join("etc/machine-id"); + std::fs::remove_file(&path).unwrap(); + symlink(f.root.join("nowhere"), &path).unwrap(); + let msg = f.refused(); + assert!(msg.contains("machine-id is a symlink"), "{msg}"); + } + + #[test] + fn a_link_at_resolv_conf_is_refused() { + let f = Fixture::clean(); + let path = f.root.join("etc/resolv.conf"); + std::fs::remove_file(&path).unwrap(); + symlink("/etc/hosts", &path).unwrap(); + let msg = f.refused(); + assert!(msg.contains("resolv.conf is a symlink"), "{msg}"); + } + + #[test] + fn a_directory_at_a_file_name_is_refused() { + let f = Fixture::clean(); + let path = f.root.join("etc/resolv.conf"); + std::fs::remove_file(&path).unwrap(); + std::fs::create_dir(&path).unwrap(); + let msg = f.refused(); + assert!(msg.contains("resolv.conf is a directory"), "{msg}"); + } + + #[test] + fn os_release_may_be_a_link_a_file_or_absent_but_not_a_directory() { + // Link: what the clean fixture has. + let f = Fixture::clean(); + f.check().unwrap(); + let path = f.root.join("etc/os-release"); + std::fs::remove_file(&path).unwrap(); + f.check().unwrap(); + std::fs::write(&path, "").unwrap(); + f.check().unwrap(); + std::fs::remove_file(&path).unwrap(); + std::fs::create_dir(&path).unwrap(); + let msg = f.refused(); + assert!(msg.contains("os-release is a directory"), "{msg}"); + } + + #[test] + fn a_profile_system_that_is_a_directory_is_refused() { + let f = Fixture::clean(); + std::fs::remove_file(&f.system).unwrap(); + std::fs::create_dir_all(f.system.join("system")).unwrap(); + let msg = f.refused(); + assert!( + msg.contains("system is a directory, not a symlink"), + "{msg}" + ); + } + + #[test] + fn a_profile_system_pointing_outside_the_store_is_refused() { + let f = Fixture::clean(); + let outside = victim(); + std::fs::write(outside.join("system"), "x86_64-linux").unwrap(); + std::fs::remove_file(&f.system).unwrap(); + symlink(&outside, &f.system).unwrap(); + let msg = f.refused(); + assert!(msg.contains("outside"), "{msg}"); + } + + #[test] + fn a_dangling_profile_system_is_refused() { + let f = Fixture::clean(); + std::fs::remove_file(&f.system).unwrap(); + symlink(f.store.join("gone"), &f.system).unwrap(); + let msg = f.refused(); + assert!(msg.contains("does not resolve"), "{msg}"); + } + + #[test] + fn a_toplevel_whose_system_is_not_a_regular_file_is_refused() { + let f = Fixture::clean(); + std::fs::remove_file(f.toplevel.join("system")).unwrap(); + std::fs::create_dir(f.toplevel.join("system")).unwrap(); + let msg = f.refused(); + assert!( + msg.contains("system is a directory, not a regular file"), + "{msg}" + ); + + std::fs::remove_dir(f.toplevel.join("system")).unwrap(); + let msg = f.refused(); + assert!(msg.contains("is missing"), "{msg}"); + } + + #[test] + fn a_check_leaves_everything_as_it_found_it() { + let f = Fixture::clean(); + let planted = f.root.join("root"); + std::fs::remove_dir_all(&planted).unwrap(); + let target = victim(); + symlink(&target, &planted).unwrap(); + f.check().unwrap_err(); + assert!(std::fs::symlink_metadata(&planted).unwrap().is_symlink()); + assert!(target.is_dir()); + } + + #[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. + assert_eq!(run(&["check-rootfs".to_string()]), Some(1)); + assert_eq!( + run(&["check-rootfs".to_string(), "not-hive".to_string()]), + Some(1) + ); + } +} diff --git a/nix/vm-tests/agent-ephemeral-rootfs.nix b/nix/vm-tests/agent-ephemeral-rootfs.nix index ed81d94b..eba9fe94 100644 --- a/nix/vm-tests/agent-ephemeral-rootfs.nix +++ b/nix/vm-tests/agent-ephemeral-rootfs.nix @@ -12,7 +12,10 @@ # - the guest journal is written inside the container, where the agent's # 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. +# PATH, after a restart; +# - a link planted at `root` in the template refuses the next start through +# the limits drop-in's `ExecStartPre=` check, 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. @@ -209,5 +212,38 @@ pkgs.testers.runNixOSTest { assert len(salvages) == 1, salvages host.succeed(f"ls -d {ROOT}.pre-ephemeral-* | wc -l | grep -x 1") assert conf().count("--ephemeral") == 1, conf() + + 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 + + # `root` is a link to a directory outside the root, which the start + # script would `chmod 0700` as host root. + victim = "/var/lib/victim" + host.succeed(f"mkdir -m 0755 {victim}") + host.succeed(f"rm -rf {ROOT}/root && ln -s {victim} {ROOT}/root") + host.fail(f"nixos-container start {MACHINE}") + host.succeed( + f"journalctl -u container@{MACHINE} | grep 'check-rootfs: refusing to start: {ROOT}/root is a symlink'" + ) + assert host.succeed(f"stat -c %a {victim}").strip() == "755" + host.succeed(f"test -L {ROOT}/root") + + # Control: with the link gone the same unit starts. + host.succeed(f"systemctl stop container@{MACHINE}") + host.succeed(f"systemctl reset-failed container@{MACHINE}") + host.succeed(f"rm {ROOT}/root && mkdir -m 0700 {ROOT}/root") + boot() ''; }