require network isolation, deleting the residual non-isolated branch
Per mara on #3725: the on/off toggle is removed, and required env vars unset lead to a crash. HIVE_NETWORK_ISOLATION is gone from hive-network.nix -- it was the toggle. Validation happens once at daemon startup rather than per container. The variables are process-global, so a bad value breaks every container rather than one: failing at boot gives a single diagnostic naming the bad value, and cannot reach a state where some containers were configured before it was noticed. Option<NetworkIsolation> collapses to NetworkIsolation through the wire type, client and helper, which deletes the branch instead of leaving it unreachable. serde(default) is dropped on that field deliberately: a request omitting isolation is now rejected rather than defaulting to a container sharing the host's network namespace. What this replaces was a silent security downgrade. Of the four ways into the old fallback, two logged nothing at all -- a container came up without isolation and the journal agreed it was fine. Doc comments that still described the removed branch are updated (argus's note on #3723 scoped that to this issue). The hive-priv one is a minimal edit inside the block #3723 rewrites; de-splicing is that PR's job.
This commit is contained in:
parent
16ac84ca63
commit
83c0e4b4bf
8 changed files with 166 additions and 83 deletions
|
|
@ -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, bridge_gateway_ip,
|
||||
container_claude_mount, container_name, validate,
|
||||
AGENT_PREFIX, CONTAINER_RUNTIME_MOUNT, CONTAINER_SHARED_MOUNT, container_claude_mount,
|
||||
container_name, validate,
|
||||
};
|
||||
|
||||
/// Re-apply the per-container host-side config: nspawn flags (bind
|
||||
|
|
@ -303,40 +303,17 @@ async fn set_nspawn_flags(
|
|||
read_only: false,
|
||||
});
|
||||
|
||||
// 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
|
||||
}
|
||||
};
|
||||
// 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)"
|
||||
);
|
||||
|
||||
// Delegate the actual conf-file rewrite to hive-priv (runs as root).
|
||||
crate::priv_client::write_nspawn_flags(container, &binds, isolation, &load_creds).await
|
||||
|
|
|
|||
|
|
@ -125,6 +125,56 @@ pub fn bridge_gateway_ip(subnet_cidr: &str) -> Option<String> {
|
|||
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<hive_priv_sock::NetworkIsolation> {
|
||||
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 <ipv4>/<prefix> 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<hive_priv_sock::NetworkIsolation> {
|
||||
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
|
||||
|
|
|
|||
|
|
@ -65,6 +65,65 @@ 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]
|
||||
|
|
|
|||
|
|
@ -217,6 +217,15 @@ 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
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -294,7 +294,7 @@ pub async fn read_container_journal(
|
|||
pub async fn write_nspawn_flags(
|
||||
container: &str,
|
||||
binds: &[BindMount],
|
||||
isolation: Option<NetworkIsolation>,
|
||||
isolation: NetworkIsolation,
|
||||
load_credentials: &[CredentialMount],
|
||||
) -> Result<()> {
|
||||
ok(call(&PrivRequest::WriteNspawnFlags {
|
||||
|
|
|
|||
|
|
@ -291,9 +291,10 @@ pub struct CredentialMount {
|
|||
pub host_path: String,
|
||||
}
|
||||
|
||||
/// 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
|
||||
/// 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
|
||||
/// 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)]
|
||||
|
|
@ -379,15 +380,19 @@ pub enum PrivRequest {
|
|||
/// Update `/etc/nixos-containers/<container>.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`.
|
||||
/// When `isolation` is `Some`, writes `PRIVATE_NETWORK=1` + veth wiring;
|
||||
/// when `None`, writes `PRIVATE_NETWORK=0`.
|
||||
/// 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.
|
||||
WriteNspawnFlags {
|
||||
container: String,
|
||||
binds: Vec<BindMount>,
|
||||
/// `None` = host netns (`PRIVATE_NETWORK=0`). `Some` = private netns with
|
||||
/// veth on the specified bridge (`PRIVATE_NETWORK=1`).
|
||||
#[serde(default)]
|
||||
isolation: Option<NetworkIsolation>,
|
||||
/// 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,
|
||||
/// Host secrets forwarded into the container's credential store via
|
||||
/// nspawn `--load-credential=<name>:<host_path>`. Empty for agents
|
||||
/// with no credentials configured (the common case). `#[serde(default)]`
|
||||
|
|
|
|||
|
|
@ -376,7 +376,7 @@ async fn exec(
|
|||
ref binds,
|
||||
ref isolation,
|
||||
ref load_credentials,
|
||||
} => handle_write_nspawn_flags(container, binds, isolation.as_ref(), load_credentials),
|
||||
} => handle_write_nspawn_flags(container, binds, isolation, 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: Option<&NetworkIsolation>,
|
||||
isolation: &NetworkIsolation,
|
||||
load_credentials: &[CredentialMount],
|
||||
) -> Result<(String, String)> {
|
||||
validate_container_system_name(container)?;
|
||||
|
|
@ -2718,13 +2718,13 @@ fn git_overlay_flags(binds: &[BindMount]) -> Vec<String> {
|
|||
/// Update `/etc/nixos-containers/<container>.conf`: strip old network vars
|
||||
/// (`PRIVATE_NETWORK`, `HOST_ADDRESS*`, `LOCAL_ADDRESS*`, `HOST_BRIDGE`),
|
||||
/// write the current network-isolation settings, then append
|
||||
/// `EXTRA_NSPAWN_FLAGS`. When `isolation` is `Some`, writes
|
||||
/// `PRIVATE_NETWORK=1` + veth wiring; when `None`, writes
|
||||
/// `PRIVATE_NETWORK=0`.
|
||||
/// `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.
|
||||
fn write_nspawn_flags(
|
||||
container: &str,
|
||||
binds: &[BindMount],
|
||||
isolation: Option<&NetworkIsolation>,
|
||||
isolation: &NetworkIsolation,
|
||||
load_credentials: &[CredentialMount],
|
||||
) -> Result<()> {
|
||||
use std::fmt::Write as _;
|
||||
|
|
@ -2747,7 +2747,8 @@ fn write_nspawn_flags(
|
|||
if !out.is_empty() {
|
||||
out.push('\n');
|
||||
}
|
||||
if let Some(iso) = isolation {
|
||||
{
|
||||
let 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
|
||||
|
|
@ -2766,13 +2767,6 @@ 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<String> = binds
|
||||
.iter()
|
||||
|
|
@ -2817,29 +2811,19 @@ fn bridge_dns_marker_path(container: &str) -> String {
|
|||
/// 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<()> {
|
||||
fn write_bridge_dns_marker(container: &str, isolation: &NetworkIsolation) -> Result<()> {
|
||||
let path = bridge_dns_marker_path(container);
|
||||
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}")),
|
||||
},
|
||||
// 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", isolation.gateway_ip))
|
||||
.with_context(|| format!("write bridge-DNS marker {path}"))?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -264,7 +264,6 @@ 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}";
|
||||
};
|
||||
|
|
|
|||
Loading…
Reference in a new issue