diff --git a/docs/conventions.md b/docs/conventions.md index cbd3e46b..7b06bcc2 100644 --- a/docs/conventions.md +++ b/docs/conventions.md @@ -418,7 +418,7 @@ via `snapshotOpenDetails` / `restoreOpenDetails`. `job_queue::templates::rebuild` builds the DAG that reconciles a container to its wanted state: `write_dropins` (the nspawn-conf -rewrite — `PRIVATE_NETWORK=1`, `HOST_ADDRESS` = the bridge gateway IP, +rewrite — `PRIVATE_NETWORK=0`, clears `HOST_ADDRESS` / `LOCAL_ADDRESS`, sets `EXTRA_NSPAWN_FLAGS` — plus the systemd resource-limits drop-in) is folded into the `Swap` node, then `nixos-container update` + stop + start runs across the `StopForUpdate → Swap → RebuildBookkeeping` diff --git a/docs/network.md b/docs/network.md index 4bfeb99e..5ec8cd3b 100644 --- a/docs/network.md +++ b/docs/network.md @@ -209,7 +209,7 @@ the nix side sets up unconditionally: | Internet NAT | `networking.nat { enable = true; internalInterfaces = [ bridgeName ]; }` — MASQUERADE on packets leaving via any external NIC | | Loopback DROP | `networking.firewall.extraInputRules` — drops bridge-subnet → `127.0.0.0/8` traffic; defence-in-depth against routing table leaks | | Gateway access | `networking.firewall.interfaces..allowedTCPPorts = [ 80 443 ]` — lets isolated agents (private netns, veth on bridge) reach nginx on the host | -| c0re signal | `HIVE_NETWORK_BRIDGE`, `HIVE_NETWORK_SUBNET` in `systemd.services.hive-c0re.environment` — both **required**; `hive-c0re` refuses to start without them | +| c0re signal | `HIVE_NETWORK_ISOLATION=1`, `HIVE_NETWORK_BRIDGE`, `HIVE_NETWORK_SUBNET` in `systemd.services.hive-c0re.environment` | `HIVE_NETWORK_SUBNET` is the host-side bridge IP + prefix (e.g. `10.42.0.1/24`), **not** the canonical network address. The Rust side @@ -218,14 +218,10 @@ address arithmetic. ### What the Rust side does -`hive-c0re` reads `HIVE_NETWORK_BRIDGE` + `HIVE_NETWORK_SUBNET` and passes +`hive-c0re` reads `HIVE_NETWORK_ISOLATION` and passes `PRIVATE_NETWORK=1`, `LOCAL_ADDRESS=` (empty), `HOST_ADDRESS=`, and `HOST_BRIDGE=` via `lifecycle::set_nspawn_flags` when -creating or updating containers. Both variables are validated **once at -daemon startup**, not per container: they are process-global, so a -missing or malformed value is a misconfigured daemon rather than one bad -container, and failing at boot gives a single diagnostic instead of one -per agent. There is no non-isolated mode to fall back to. `LOCAL_ADDRESS` is left empty so the +creating or updating containers. `LOCAL_ADDRESS` is left empty so the container's dhcpcd acquires an address from the bridge dnsmasq pool (`networking.useDHCP = true` in `nix/agent-modules/network.nix`). This applies uniformly to all containers — agents and service containers alike. diff --git a/hive-c0re/src/lifecycle/host_config.rs b/hive-c0re/src/lifecycle/host_config.rs index 4768d493..3b572895 100644 --- a/hive-c0re/src/lifecycle/host_config.rs +++ b/hive-c0re/src/lifecycle/host_config.rs @@ -10,8 +10,8 @@ use hive_priv_sock::{BindMount, CredentialMount}; use crate::coordinator::{AgentPaths, HiveEnv}; use super::{ - AGENT_PREFIX, CONTAINER_RUNTIME_MOUNT, CONTAINER_SHARED_MOUNT, container_claude_mount, - container_name, validate, + AGENT_PREFIX, CONTAINER_RUNTIME_MOUNT, CONTAINER_SHARED_MOUNT, bridge_gateway_ip, + container_claude_mount, container_name, validate, }; /// Re-apply the per-container host-side config: nspawn flags (bind @@ -131,12 +131,9 @@ fn bind_child_agent_dirs(child: &str, binds: &mut Vec) { } /// Idempotently rewrite the lines in `/etc/nixos-containers/.conf` -/// that hive-c0re owns: `PRIVATE_NETWORK` (always 1 — every container runs in -/// its own network namespace with a veth on the host bridge, and the agent's -/// web UI is reached over that bridge rather than a host-shared netns), -/// `HOST_ADDRESS` (the bridge gateway IP, so the container installs a default -/// route before DHCP completes) and `EXTRA_NSPAWN_FLAGS` (the runtime-dir -/// bind). The start script expands `$EXTRA_NSPAWN_FLAGS` unquoted into the +/// that hive-c0re owns: `PRIVATE_NETWORK` (forced 0 so the agent's web UI port +/// is reachable on the host) and `EXTRA_NSPAWN_FLAGS` (the runtime-dir bind). +/// The start script expands `$EXTRA_NSPAWN_FLAGS` unquoted into the /// `systemd-nspawn` command. #[allow( clippy::too_many_lines, @@ -306,17 +303,40 @@ async fn set_nspawn_flags( read_only: false, }); - // Every container runs in a private network namespace with a veth - // pair attached to the host bridge — including the manager (all - // hive-c0re<->agent comms go through bind-mounted UDS, not TCP). - // There is no non-isolated mode to fall back to, and the settings - // are process-global, so anything wrong here was already fatal at - // daemon startup; this call cannot newly fail. - let isolation = super::network_isolation_from_env()?; - tracing::info!( - %agent_name, gateway_ip = %isolation.gateway_ip, bridge = %isolation.bridge, - "network isolation: PRIVATE_NETWORK=1 (DHCP)" - ); + // Network isolation: when HIVE_NETWORK_ISOLATION=1 is set (by the + // hive-network.nix module's `isolateContainers` option), flip the + // container to a private network namespace with a veth pair attached + // to the host bridge. Applies to all containers including the manager + // (all hive-c0re<->agent comms go through bind-mounted UDS, not TCP). + let isolation = { + let isolate = std::env::var("HIVE_NETWORK_ISOLATION").ok().as_deref() == Some("1"); + let bridge = std::env::var("HIVE_NETWORK_BRIDGE").unwrap_or_default(); + let subnet = std::env::var("HIVE_NETWORK_SUBNET").unwrap_or_default(); + if isolate && !bridge.is_empty() && !subnet.is_empty() { + let Some(gateway_ip) = bridge_gateway_ip(&subnet) else { + tracing::warn!( + %agent_name, %subnet, + "HIVE_NETWORK_SUBNET is set but the bridge gateway IP is unparseable; \ + skipping PRIVATE_NETWORK write to avoid an isolated container with no \ + default route or resolver" + ); + return crate::priv_client::write_nspawn_flags( + container, + &binds, + None, + &load_creds, + ) + .await; + }; + tracing::info!( + %agent_name, %gateway_ip, %bridge, + "network isolation: PRIVATE_NETWORK=1 (DHCP)" + ); + Some(hive_priv_sock::NetworkIsolation { bridge, gateway_ip }) + } else { + None + } + }; // Delegate the actual conf-file rewrite to hive-priv (runs as root). crate::priv_client::write_nspawn_flags(container, &binds, isolation, &load_creds).await diff --git a/hive-c0re/src/lifecycle/mod.rs b/hive-c0re/src/lifecycle/mod.rs index 44b8816e..36cd6cc1 100644 --- a/hive-c0re/src/lifecycle/mod.rs +++ b/hive-c0re/src/lifecycle/mod.rs @@ -125,56 +125,6 @@ pub fn bridge_gateway_ip(subnet_cidr: &str) -> Option { Some(ip_str.to_owned()) } -/// Build the network-isolation settings every container is configured -/// with, from the variables `hive-network.nix` sets on the `hive-c0re` -/// unit. -/// -/// Isolation is the only supported mode: the on/off toggle is gone, so -/// there is no non-isolated branch to fall back to and a missing or -/// malformed value means the daemon is misconfigured — not that a -/// container should quietly come up sharing the host's netns. Silently -/// degrading here dropped a security boundary with nothing in the log -/// to say so. -/// -/// Split from [`network_isolation_from_env`] so the parsing is testable -/// without touching process environment. -pub fn network_isolation_from_vars( - bridge: Option<&str>, - subnet: Option<&str>, -) -> Result { - let bridge = bridge.filter(|s| !s.is_empty()).context( - "HIVE_NETWORK_BRIDGE is unset or empty — hive-network.nix sets it on the \ - hive-c0re unit, so this means the daemon is running outside its unit or \ - with a broken module evaluation", - )?; - let subnet = subnet.filter(|s| !s.is_empty()).context( - "HIVE_NETWORK_SUBNET is unset or empty — hive-network.nix sets it on the \ - hive-c0re unit, so this means the daemon is running outside its unit or \ - with a broken module evaluation", - )?; - let gateway_ip = bridge_gateway_ip(subnet).with_context(|| { - format!( - "HIVE_NETWORK_SUBNET={subnet} is not a valid / pair; \ - it comes from services.hyperhive.network.bridgeIp + bridgePrefixLength" - ) - })?; - Ok(hive_priv_sock::NetworkIsolation { - bridge: bridge.to_owned(), - gateway_ip, - }) -} - -/// [`network_isolation_from_vars`] over the real process environment. -/// -/// Called once at daemon startup so a bad value fails the unit loudly, -/// and again per container — the variables are process-global, so the -/// second call cannot start failing once the first has passed. -pub fn network_isolation_from_env() -> Result { - let bridge = std::env::var("HIVE_NETWORK_BRIDGE").ok(); - let subnet = std::env::var("HIVE_NETWORK_SUBNET").ok(); - network_isolation_from_vars(bridge.as_deref(), subnet.as_deref()) -} - /// Read the agent user's `(uid, gid)` from the container's nixos-managed /// `/etc/passwd`. Returns `None` when the container hasn't been built /// yet, the passwd file is unparseable, or the agent user is missing diff --git a/hive-c0re/src/lifecycle/tests.rs b/hive-c0re/src/lifecycle/tests.rs index 81e925eb..fc699024 100644 --- a/hive-c0re/src/lifecycle/tests.rs +++ b/hive-c0re/src/lifecycle/tests.rs @@ -65,65 +65,6 @@ fn bridge_gateway_ip_rejects_bad_input() { assert!(bridge_gateway_ip("10.42.0/24").is_none()); // 3 octets } -/// The presence control for the two rejection tests below: with both -/// variables set and well-formed, the settings are built. Without this, -/// a `network_isolation_from_vars` that rejected *everything* would pass -/// every absence assertion and look like a working guard. -#[test] -fn network_isolation_accepts_a_well_formed_pair() { - let iso = network_isolation_from_vars(Some("hive0"), Some("10.42.0.1/24")) - .expect("well-formed bridge + subnet must be accepted"); - assert_eq!(iso.bridge, "hive0"); - // The gateway is the verbatim bridge address, prefix stripped. - assert_eq!(iso.gateway_ip, "10.42.0.1"); -} - -/// A missing or empty variable is fatal, not a fallback to the host -/// netns. Empty is tested alongside unset because `std::env::var` on a -/// variable set to `""` returns `Ok("")`, so treating only `None` as -/// missing would let an empty value through. -#[test] -fn network_isolation_rejects_missing_or_empty_vars() { - assert!(network_isolation_from_vars(None, Some("10.42.0.1/24")).is_err()); - assert!(network_isolation_from_vars(Some("hive0"), None).is_err()); - assert!(network_isolation_from_vars(None, None).is_err()); - assert!(network_isolation_from_vars(Some(""), Some("10.42.0.1/24")).is_err()); - assert!(network_isolation_from_vars(Some("hive0"), Some("")).is_err()); -} - -/// A malformed subnet is fatal too. Previously this logged a warning and -/// silently produced a container on the host netns — a dropped security -/// boundary with nothing in the journal saying so. -#[test] -fn network_isolation_rejects_a_malformed_subnet() { - assert!(network_isolation_from_vars(Some("hive0"), Some("notanip/24")).is_err()); - assert!(network_isolation_from_vars(Some("hive0"), Some("10.42.0.1")).is_err()); - assert!(network_isolation_from_vars(Some("hive0"), Some("10.42.0.1/33")).is_err()); - assert!(network_isolation_from_vars(Some("hive0"), Some("10.42.0.999/24")).is_err()); -} - -/// The error has to name the variable an operator must fix — these are -/// read at startup, so the message is the whole diagnostic. -#[test] -fn network_isolation_errors_name_the_offending_variable() { - let e = network_isolation_from_vars(None, Some("10.42.0.1/24")).unwrap_err(); - assert!( - format!("{e:#}").contains("HIVE_NETWORK_BRIDGE"), - "bridge error must name the variable, got: {e:#}" - ); - let e = network_isolation_from_vars(Some("hive0"), None).unwrap_err(); - assert!( - format!("{e:#}").contains("HIVE_NETWORK_SUBNET"), - "subnet error must name the variable, got: {e:#}" - ); - let e = network_isolation_from_vars(Some("hive0"), Some("nope/24")).unwrap_err(); - let msg = format!("{e:#}"); - assert!( - msg.contains("HIVE_NETWORK_SUBNET") && msg.contains("nope/24"), - "malformed-subnet error must name the variable and the bad value, got: {msg}" - ); -} - /// `setup_proposed` is idempotent: calling it on an existing repo is a /// no-op (the fresh guard skips all writes). #[tokio::test] diff --git a/hive-c0re/src/main.rs b/hive-c0re/src/main.rs index 1cfa654a..a2428fc0 100644 --- a/hive-c0re/src/main.rs +++ b/hive-c0re/src/main.rs @@ -217,15 +217,6 @@ async fn main() -> Result<()> { if let Some(v) = build_slots { sc.build_slots = v; } - // Network isolation is required and its settings are - // process-global, so a bad value breaks every container, not - // one. Validate once here: the unit then fails visibly at - // start with a single diagnostic naming the bad value, - // instead of coming up "healthy" and failing each container - // configure separately — possibly after some already - // succeeded. - lifecycle::network_isolation_from_env() - .context("network isolation settings are required at startup")?; cmd_serve(sc.env, sc.model_prices, sc.build_slots, db, &cli.socket).await } } diff --git a/hive-c0re/src/priv_client.rs b/hive-c0re/src/priv_client.rs index ec56567f..07a0f952 100644 --- a/hive-c0re/src/priv_client.rs +++ b/hive-c0re/src/priv_client.rs @@ -294,7 +294,7 @@ pub async fn read_container_journal( pub async fn write_nspawn_flags( container: &str, binds: &[BindMount], - isolation: NetworkIsolation, + isolation: Option, load_credentials: &[CredentialMount], ) -> Result<()> { ok(call(&PrivRequest::WriteNspawnFlags { diff --git a/hive-priv-sock/src/lib.rs b/hive-priv-sock/src/lib.rs index 74364e30..6407c357 100644 --- a/hive-priv-sock/src/lib.rs +++ b/hive-priv-sock/src/lib.rs @@ -291,10 +291,9 @@ pub struct CredentialMount { pub host_path: String, } -/// Network isolation parameters for `WriteNspawnFlags`. hive-priv writes -/// `PRIVATE_NETWORK=1` + veth bridge wiring from these; every container -/// is isolated, so they are required rather than a mode selector. -/// Containers receive their IP +/// Network isolation parameters for `WriteNspawnFlags`. When `Some`, +/// hive-priv writes `PRIVATE_NETWORK=1` + veth bridge wiring instead +/// of the default `PRIVATE_NETWORK=0`. Containers receive their IP /// dynamically via DHCP from the bridge dnsmasq pool (`networking.useDHCP` /// in `harness-base.nix`); no static address is pre-assigned here. #[derive(Debug, Clone, Serialize, Deserialize)] @@ -380,19 +379,15 @@ pub enum PrivRequest { /// Update `/etc/nixos-containers/.conf`: strip old network-isolation /// vars, write `PRIVATE_NETWORK` + bridge settings, and set `EXTRA_NSPAWN_FLAGS` /// from the provided bind-mount list. Written by `lifecycle::set_nspawn_flags`. - /// Always writes `PRIVATE_NETWORK=1` + veth wiring: isolation is the only - /// mode, so there is no request shape that yields a container sharing the - /// host's network namespace. + /// When `isolation` is `Some`, writes `PRIVATE_NETWORK=1` + veth wiring; + /// when `None`, writes `PRIVATE_NETWORK=0`. WriteNspawnFlags { container: String, binds: Vec, - /// Private netns with a veth on the given bridge - /// (`PRIVATE_NETWORK=1`). Required: isolation is the only - /// supported mode, so there is no value meaning "host netns". - /// Deliberately **not** `#[serde(default)]` — a request that - /// omits it is rejected rather than quietly configuring a - /// container that shares the host's network namespace. - isolation: NetworkIsolation, + /// `None` = host netns (`PRIVATE_NETWORK=0`). `Some` = private netns with + /// veth on the specified bridge (`PRIVATE_NETWORK=1`). + #[serde(default)] + isolation: Option, /// Host secrets forwarded into the container's credential store via /// nspawn `--load-credential=:`. Empty for agents /// with no credentials configured (the common case). `#[serde(default)]` diff --git a/hive-priv/src/main.rs b/hive-priv/src/main.rs index 57be923e..dbfb4ee6 100644 --- a/hive-priv/src/main.rs +++ b/hive-priv/src/main.rs @@ -376,7 +376,7 @@ async fn exec( ref binds, ref isolation, ref load_credentials, - } => handle_write_nspawn_flags(container, binds, isolation, load_credentials), + } => handle_write_nspawn_flags(container, binds, isolation.as_ref(), load_credentials), PrivRequest::WriteResourceLimits { ref container, @@ -923,7 +923,7 @@ fn single_output_path(stdout: &str) -> Result<&str, usize> { fn handle_write_nspawn_flags( container: &str, binds: &[BindMount], - isolation: &NetworkIsolation, + isolation: Option<&NetworkIsolation>, load_credentials: &[CredentialMount], ) -> Result<(String, String)> { validate_container_system_name(container)?; @@ -2718,13 +2718,13 @@ fn git_overlay_flags(binds: &[BindMount]) -> Vec { /// Update `/etc/nixos-containers/.conf`: strip old network vars /// (`PRIVATE_NETWORK`, `HOST_ADDRESS*`, `LOCAL_ADDRESS*`, `HOST_BRIDGE`), /// write the current network-isolation settings, then append -/// `EXTRA_NSPAWN_FLAGS`. Always writes `PRIVATE_NETWORK=1` + veth -/// wiring — isolation is the only mode, so there is no branch that -/// leaves a container on the host's network namespace. +/// `EXTRA_NSPAWN_FLAGS`. When `isolation` is `Some`, writes +/// `PRIVATE_NETWORK=1` + veth wiring; when `None`, writes +/// `PRIVATE_NETWORK=0`. fn write_nspawn_flags( container: &str, binds: &[BindMount], - isolation: &NetworkIsolation, + isolation: Option<&NetworkIsolation>, load_credentials: &[CredentialMount], ) -> Result<()> { use std::fmt::Write as _; @@ -2747,8 +2747,7 @@ fn write_nspawn_flags( if !out.is_empty() { out.push('\n'); } - { - let iso = isolation; + if let Some(iso) = isolation { out.push_str("PRIVATE_NETWORK=1\n"); // HOST_ADDRESS = the bridge gateway IP. nixos-container's // container-side setup only installs a default route @@ -2767,6 +2766,13 @@ fn write_nspawn_flags( out.push_str("HOST_ADDRESS6=\n"); out.push_str("LOCAL_ADDRESS6=\n"); let _ = writeln!(out, "HOST_BRIDGE={}", iso.bridge); + } else { + out.push_str("PRIVATE_NETWORK=0\n"); + out.push_str("HOST_ADDRESS=\n"); + out.push_str("LOCAL_ADDRESS=\n"); + out.push_str("HOST_ADDRESS6=\n"); + out.push_str("LOCAL_ADDRESS6=\n"); + out.push_str("HOST_BRIDGE=\n"); } let mut flags: Vec = binds .iter() @@ -2797,8 +2803,8 @@ fn write_nspawn_flags( // anyway). The `hyperhive-isolated-dns` oneshot in harness-base.nix // rewrites resolv.conf to point at the bridge resolver, but only when // this marker exists; it carries the gateway IP so the container - // doesn't have to re-derive it. Always written — every container is - // isolated, so there is no mode in which the marker should be absent. + // doesn't have to re-derive it. Written on isolate, removed otherwise, + // so the same shared container toplevel behaves correctly in both modes. write_bridge_dns_marker(container, isolation)?; Ok(()) } @@ -2808,23 +2814,32 @@ fn bridge_dns_marker_path(container: &str) -> String { format!("/var/lib/nixos-containers/{container}/etc/hyperhive-bridge-dns") } -/// Write the bridge-DNS marker the `hyperhive-isolated-dns` oneshot keys -/// off. The marker file contains just the gateway IP. Always written: -/// every container is isolated, so there is no host-netns case that -/// wants the marker absent. -fn write_bridge_dns_marker(container: &str, isolation: &NetworkIsolation) -> Result<()> { +/// Write (isolated) or remove (host-netns) the bridge-DNS marker the +/// `hyperhive-isolated-dns` oneshot keys off. The marker file contains +/// just the gateway IP. Best-effort on removal (absence is the goal). +fn write_bridge_dns_marker(container: &str, isolation: Option<&NetworkIsolation>) -> Result<()> { let path = bridge_dns_marker_path(container); - // On a fresh install the container's `/etc` may not exist yet - // (rootfs not fully materialised before the first start), so - // `write` would fail with ENOENT. Create the parent dir first - // — it's the container's own `/etc`, which nixos-container - // populates on start; a pre-created dir + our marker persist. - if let Some(parent) = std::path::Path::new(&path).parent() { - std::fs::create_dir_all(parent) - .with_context(|| format!("create bridge-DNS marker dir {}", parent.display()))?; + match isolation { + Some(iso) => { + // On a fresh install the container's `/etc` may not exist yet + // (rootfs not fully materialised before the first start), so + // `write` would fail with ENOENT. Create the parent dir first + // — it's the container's own `/etc`, which nixos-container + // populates on start; a pre-created dir + our marker persist. + if let Some(parent) = std::path::Path::new(&path).parent() { + std::fs::create_dir_all(parent).with_context(|| { + format!("create bridge-DNS marker dir {}", parent.display()) + })?; + } + std::fs::write(&path, format!("{}\n", iso.gateway_ip)) + .with_context(|| format!("write bridge-DNS marker {path}"))?; + } + None => match std::fs::remove_file(&path) { + Ok(()) => {} + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => return Err(e).with_context(|| format!("remove bridge-DNS marker {path}")), + }, } - std::fs::write(&path, format!("{}\n", isolation.gateway_ip)) - .with_context(|| format!("write bridge-DNS marker {path}"))?; Ok(()) } diff --git a/nix/host-modules/hive-network.nix b/nix/host-modules/hive-network.nix index c4eed090..957e5caf 100644 --- a/nix/host-modules/hive-network.nix +++ b/nix/host-modules/hive-network.nix @@ -264,6 +264,7 @@ in # container. HIVE_NETWORK_SUBNET is host-bridge IP/prefix, not canonical # network address — the Rust side normalises before subnet arithmetic. systemd.services.hive-c0re.environment = { + HIVE_NETWORK_ISOLATION = "1"; HIVE_NETWORK_BRIDGE = cfg.bridgeName; HIVE_NETWORK_SUBNET = "${cfg.bridgeIp}/${toString cfg.bridgePrefixLength}"; };