From 9e12012a95b8884dc7180d4f3f04ccfbdf050a98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?m=C3=BCde?= Date: Tue, 2 Jun 2026 23:43:02 +0200 Subject: [PATCH] fix(#702): route container journal reads through hive-priv The privsep drop to the hive-core user left four journalctl -M call sites shelling out directly. -M enters the container namespace via the machine bus, which needs root, so all container-journal reads failed with Permission denied. Add a ReadContainerJournal verb to hive-priv and route dashboard get_journal, manager get_logs, the rebuild-failure journal tail, and the agent host-journal -M path through it. Host-journal reads (no -M) stay direct via systemd-journal group membership. --- hive-c0re/src/agent_server.rs | 34 +++++++++++-- hive-c0re/src/dashboard.rs | 69 ++++++++++++++------------ hive-c0re/src/lifecycle.rs | 24 ++++++--- hive-c0re/src/manager_server.rs | 35 ++++++------- hive-c0re/src/priv_client.rs | 33 ++++++++++++- hive-priv/src/main.rs | 87 ++++++++++++++++++++++++++++++++- hive-sh4re/src/priv_proto.rs | 62 +++++++++++++++++++++++ 7 files changed, 279 insertions(+), 65 deletions(-) diff --git a/hive-c0re/src/agent_server.rs b/hive-c0re/src/agent_server.rs index b9964468..93c7d81a 100644 --- a/hive-c0re/src/agent_server.rs +++ b/hive-c0re/src/agent_server.rs @@ -378,6 +378,36 @@ pub async fn dispatch_host_journal( }; } let n = lines.unwrap_or(30).min(100); + + // A container (`-M`) read enters the container namespace and needs + // root, so it's delegated to hive-priv. A host read (no container) + // the unprivileged hive-core user can do directly via its + // systemd-journal group membership. + if let Some(c) = container { + tracing::info!(%agent, machine = %c, %n, "get_host_journal (container)"); + return match crate::priv_client::read_container_journal( + c, + n, + false, + hive_sh4re::priv_proto::JournalOutput::Short, + unit.clone(), + priority.as_ref().map(|p| p.as_str().to_owned()), + grep.clone(), + since.clone(), + until.clone(), + ) + .await + { + Ok((stdout, stderr)) => { + let content = if !stdout.is_empty() { stdout } else { stderr }; + AgentResponse::HostJournal { content } + } + Err(e) => AgentResponse::Err { + message: format!("journal read: {e:#}"), + }, + }; + } + let mut args: Vec = vec![ "--no-pager".to_owned(), "--output=short".to_owned(), @@ -388,10 +418,6 @@ pub async fn dispatch_host_journal( args.push("-u".to_owned()); args.push(u.clone()); } - if let Some(c) = container { - args.push("-M".to_owned()); - args.push(c.clone()); - } if let Some(p) = priority { args.push("-p".to_owned()); args.push(p.as_str().to_owned()); diff --git a/hive-c0re/src/dashboard.rs b/hive-c0re/src/dashboard.rs index 84e59668..07ec47ce 100644 --- a/hive-c0re/src/dashboard.rs +++ b/hive-c0re/src/dashboard.rs @@ -1160,10 +1160,10 @@ struct JournalQuery { lines: Option, } -/// Shell out to `journalctl -M -b` and return its text -/// output. Operator-only by virtue of the dashboard being host-bound; -/// hive-c0re already runs as root in its systemd unit so journalctl -/// has the access it needs. +/// Read `journalctl -M -b` and return its text output. +/// Operator-only by virtue of the dashboard being host-bound. hive-c0re +/// runs unprivileged (privsep), so the `-M` read — which enters the +/// container namespace and needs root — is delegated to hive-priv. async fn get_journal( AxumPath(name): AxumPath, axum::extract::Query(q): axum::extract::Query, @@ -1184,40 +1184,45 @@ async fn get_journal( return error_response(&format!("journal: no managed container {prefixed:?}")); } let lines = q.lines.unwrap_or(500).min(5000); - let mut cmd = tokio::process::Command::new("journalctl"); - cmd.args([ - "-M", - &prefixed, - "-b", - "--no-pager", - "--output=short-iso", - "--lines", - ]) - .arg(lines.to_string()); - if let Some(u) = q.unit.as_deref().filter(|s| !s.is_empty()) { - // accept hive-ag3nt[.service] — anything else refused. - let allowed = ["hive-ag3nt.service"]; - let unit = if u.ends_with(".service") { - u.to_owned() - } else { - format!("{u}.service") - }; - if !allowed.contains(&unit.as_str()) { - return error_response(&format!("journal: unknown unit {unit:?}")); + let unit = match q.unit.as_deref().filter(|s| !s.is_empty()) { + Some(u) => { + // accept hive-ag3nt[.service] — anything else refused. + let allowed = ["hive-ag3nt.service"]; + let unit = if u.ends_with(".service") { + u.to_owned() + } else { + format!("{u}.service") + }; + if !allowed.contains(&unit.as_str()) { + return error_response(&format!("journal: unknown unit {unit:?}")); + } + Some(unit) } - cmd.args(["-u", &unit]); - } - match cmd.output().await { - Ok(out) => { + None => None, + }; + match crate::priv_client::read_container_journal( + &prefixed, + lines, + true, + hive_sh4re::priv_proto::JournalOutput::ShortIso, + unit, + None, + None, + None, + None, + ) + .await + { + Ok((stdout, stderr)) => { // Combine stdout + stderr — journalctl emits to both on errors. - let mut body = String::from_utf8_lossy(&out.stdout).into_owned(); - if !out.status.success() { + let mut body = stdout; + if !stderr.is_empty() { body.push_str("\n--- stderr ---\n"); - body.push_str(&String::from_utf8_lossy(&out.stderr)); + body.push_str(&stderr); } ([("content-type", "text/plain; charset=utf-8")], body).into_response() } - Err(e) => error_response(&format!("journalctl spawn: {e}")), + Err(e) => error_response(&format!("journal read: {e:#}")), } } diff --git a/hive-c0re/src/lifecycle.rs b/hive-c0re/src/lifecycle.rs index 8ff68199..77f17d0e 100644 --- a/hive-c0re/src/lifecycle.rs +++ b/hive-c0re/src/lifecycle.rs @@ -1195,14 +1195,24 @@ async fn priv_run(kind: &str, name: &str) -> Result<()> { /// or when the journal can't be read (machine gone, journalctl /// missing); it never produces an error of its own. async fn container_journal_tail(container: &str) -> String { - let out = Command::new("journalctl") - .args(["-M", container, "-n", "40", "--no-pager", "--output=short"]) - .output() - .await; - match out { - Ok(o) if !o.stdout.is_empty() => format!( + // `-M` enters the container namespace and needs root, so the read + // is delegated to hive-priv (hive-c0re itself runs unprivileged). + let res = crate::priv_client::read_container_journal( + container, + 40, + false, + hive_sh4re::priv_proto::JournalOutput::Short, + None, + None, + None, + None, + None, + ) + .await; + match res { + Ok((stdout, _)) if !stdout.is_empty() => format!( "\n--- last 40 journal lines from container '{container}' ---\n{}", - String::from_utf8_lossy(&o.stdout).trim_end() + stdout.trim_end() ), _ => String::new(), } diff --git a/hive-c0re/src/manager_server.rs b/hive-c0re/src/manager_server.rs index 246c3106..4d0fd629 100644 --- a/hive-c0re/src/manager_server.rs +++ b/hive-c0re/src/manager_server.rs @@ -254,31 +254,28 @@ async fn dispatch(req: &ManagerRequest, coord: &Arc) -> ManagerResp let n = lines.unwrap_or(50); // `journalctl -M` wants the container name (`h-`), // not the logical agent name. `container_name` adds the prefix. + // The `-M` read needs root, so it goes through hive-priv. let machine = crate::lifecycle::container_name(agent); tracing::info!(%agent, %machine, %n, "manager: get_logs"); - match tokio::process::Command::new("journalctl") - .args([ - "-M", - &machine, - "-n", - &n.to_string(), - "--no-pager", - "--output=short", - ]) - .output() - .await + match crate::priv_client::read_container_journal( + &machine, + n, + false, + hive_sh4re::priv_proto::JournalOutput::Short, + None, + None, + None, + None, + None, + ) + .await { - Ok(out) => { - let content = if out.status.success() || !out.stdout.is_empty() { - String::from_utf8_lossy(&out.stdout).into_owned() - } else { - let stderr = String::from_utf8_lossy(&out.stderr); - format!("journalctl exited {}: {stderr}", out.status) - }; + Ok((stdout, stderr)) => { + let content = if !stdout.is_empty() { stdout } else { stderr }; ManagerResponse::Logs { content } } Err(e) => ManagerResponse::Err { - message: format!("journalctl spawn failed: {e:#}"), + message: format!("get_logs: {e:#}"), }, } } diff --git a/hive-c0re/src/priv_client.rs b/hive-c0re/src/priv_client.rs index 81ef6b44..34b52e6f 100644 --- a/hive-c0re/src/priv_client.rs +++ b/hive-c0re/src/priv_client.rs @@ -7,7 +7,7 @@ //! a persistent connection. use anyhow::{Context as _, Result, bail}; -use hive_sh4re::priv_proto::{BindMount, PRIV_SOCK, PrivRequest, PrivResponse}; +use hive_sh4re::priv_proto::{BindMount, JournalOutput, PRIV_SOCK, PrivRequest, PrivResponse}; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio::net::UnixStream; @@ -81,6 +81,37 @@ pub async fn list_containers() -> Result { Ok(stdout) } +/// Read a container's journal via the root helper (`journalctl -M`). +/// Returns `(stdout, stderr)`; a non-zero journalctl exit is reported in +/// `stderr` rather than as an `Err`, so callers can surface either. +#[allow(clippy::too_many_arguments)] +pub async fn read_container_journal( + container: &str, + lines: u32, + boot: bool, + output: JournalOutput, + unit: Option, + priority: Option, + grep: Option, + since: Option, + until: Option, +) -> Result<(String, String)> { + check( + call(&PrivRequest::ReadContainerJournal { + container: container.to_owned(), + lines, + boot, + output, + unit, + priority, + grep, + since, + until, + }) + .await?, + ) +} + pub async fn write_nspawn_flags(container: &str, binds: &[BindMount]) -> Result<()> { ok(call(&PrivRequest::WriteNspawnFlags { container: container.to_owned(), diff --git a/hive-priv/src/main.rs b/hive-priv/src/main.rs index deb2b4a2..ef8316b6 100644 --- a/hive-priv/src/main.rs +++ b/hive-priv/src/main.rs @@ -21,8 +21,8 @@ use std::path::{Path, PathBuf}; use anyhow::{Context as _, Result, bail}; use hive_sh4re::priv_proto::{ - AGENT_PREFIX, BindMount, MANAGER_NAME, META_DIR, PRIV_SOCK, PrivRequest, PrivResponse, - SIBLING_CONTAINERS, + AGENT_PREFIX, BindMount, JournalOutput, MANAGER_NAME, META_DIR, PRIV_SOCK, PrivRequest, + PrivResponse, SIBLING_CONTAINERS, }; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio::net::{UnixListener, UnixStream}; @@ -186,6 +186,24 @@ async fn exec(req: PrivRequest) -> Result<(String, String)> { PrivRequest::ListContainers => container_run(&["list"]).await, + PrivRequest::ReadContainerJournal { + ref container, + lines, + boot, + output, + ref unit, + ref priority, + ref grep, + ref since, + ref until, + } => { + validate_container_system_name(container)?; + read_container_journal( + container, lines, boot, output, unit, priority, grep, since, until, + ) + .await + } + PrivRequest::WriteNspawnFlags { ref container, ref binds, @@ -313,6 +331,71 @@ async fn container_run(args: &[&str]) -> Result<(String, String)> { Ok((stdout, stderr)) } +/// Read a container's journal as root via `journalctl -M`. Returns +/// `(stdout, stderr)`. Unlike `container_run` a non-zero exit is *not* a +/// hard error — journalctl's own diagnostic (folded into `stderr` with +/// the exit status) is what the caller surfaces to the operator, so the +/// helper never bails. +#[allow(clippy::too_many_arguments)] +async fn read_container_journal( + container: &str, + lines: u32, + boot: bool, + output: JournalOutput, + unit: &Option, + priority: &Option, + grep: &Option, + since: &Option, + until: &Option, +) -> Result<(String, String)> { + let mut args: Vec = vec![ + "-M".to_owned(), + container.to_owned(), + "--no-pager".to_owned(), + format!("--output={}", output.as_journalctl()), + "-n".to_owned(), + lines.to_string(), + ]; + if boot { + args.push("-b".to_owned()); + } + if let Some(u) = unit { + args.push("-u".to_owned()); + args.push(u.clone()); + } + if let Some(p) = priority { + args.push("-p".to_owned()); + args.push(p.clone()); + } + // `--grep=`/`--since=`/`--until=` use the `=`-joined form so a value + // can never be parsed as a separate journalctl flag. + if let Some(g) = grep { + args.push(format!("--grep={g}")); + } + if let Some(s) = since { + args.push(format!("--since={s}")); + } + if let Some(u) = until { + args.push(format!("--until={u}")); + } + let out = Command::new("journalctl") + .args(&args) + .output() + .await + .context("invoke journalctl -M")?; + let stdout = String::from_utf8_lossy(&out.stdout).into_owned(); + let stderr = if out.status.success() { + String::from_utf8_lossy(&out.stderr).into_owned() + } else { + format!( + "journalctl -M {container} exited {}: {}", + out.status, + String::from_utf8_lossy(&out.stderr).trim() + ) + }; + Ok((stdout, stderr)) +} + /// Return the system container name for a logical agent name. /// All agents (including the manager) use the `h-` prefix. fn container_system_name(name: &str) -> String { diff --git a/hive-sh4re/src/priv_proto.rs b/hive-sh4re/src/priv_proto.rs index 82e99783..718781fe 100644 --- a/hive-sh4re/src/priv_proto.rs +++ b/hive-sh4re/src/priv_proto.rs @@ -22,6 +22,29 @@ pub const SIBLING_CONTAINERS: &[&str] = &["hive-forge", "hive-matrix", "hive-gat /// `{META_DIR}#{name}`, derived by `hive-priv` — never passed over the wire. pub const META_DIR: &str = "/var/lib/hyperhive/meta"; +/// Output format for `ReadContainerJournal`. Maps to journalctl +/// `--output=<...>`. Restricted to the two formats hive callers use so +/// the wire type can't smuggle an arbitrary `--output` value. +#[derive(Debug, Clone, Copy, Default, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum JournalOutput { + /// `short` — the journalctl default (syslog-style timestamps). + #[default] + Short, + /// `short-iso` — ISO 8601 timestamps. + ShortIso, +} + +impl JournalOutput { + /// The string journalctl expects after `--output=`. + pub fn as_journalctl(self) -> &'static str { + match self { + JournalOutput::Short => "short", + JournalOutput::ShortIso => "short-iso", + } + } +} + /// One bind-mount entry for `WriteNspawnFlags`. /// hive-priv constructs `--bind=:` (or `--bind-ro=`) /// and validates both paths before writing the conf file. @@ -65,6 +88,45 @@ pub enum PrivRequest { /// `nixos-container list` ListContainers, + // --- Container journal reads --- + /// Read a container's journal via `journalctl -M `. + /// Requires root: the machine-bus transport enters the container's + /// namespace, so this can't run from the unprivileged hive-c0re + /// process. hive-priv validates `container` against the managed- + /// container allowlist, then runs journalctl and returns its output. + /// + /// The filters (`unit` / `priority` / `grep` / `since` / `until`) + /// are applied within the already-authorized machine and passed to + /// journalctl as plain argument values; they can't widen access + /// beyond the validated `container`. + ReadContainerJournal { + /// System container name (`h-` or a sibling service). + container: String, + /// `-n `. + lines: u32, + /// `-b` — restrict to the current boot. + #[serde(default)] + boot: bool, + /// `--output=<...>`. + #[serde(default)] + output: JournalOutput, + /// `-u `. + #[serde(default)] + unit: Option, + /// `-p `. + #[serde(default)] + priority: Option, + /// `--grep=`. + #[serde(default)] + grep: Option, + /// `--since=`. + #[serde(default)] + since: Option, + /// `--until=`. + #[serde(default)] + until: Option, + }, + // --- Config file writes --- /// Update `/etc/nixos-containers/.conf`: strip network-isolation /// vars, force `PRIVATE_NETWORK=0`, and set `EXTRA_NSPAWN_FLAGS` from the