diff --git a/hive-c0re/src/gateway_nginx.rs b/hive-c0re/src/gateway_nginx.rs index b759dd07..1fe2bd8b 100644 --- a/hive-c0re/src/gateway_nginx.rs +++ b/hive-c0re/src/gateway_nginx.rs @@ -317,7 +317,10 @@ fn htpasswd_read() -> Result> { /// Write lines back atomically (`.tmp` then rename), with a trailing /// newline. fn htpasswd_write(lines: &[String]) -> Result<()> { - let path = htpasswd_path(); + htpasswd_write_at(&htpasswd_path(), lines) +} + +fn htpasswd_write_at(path: &std::path::Path, lines: &[String]) -> Result<()> { let tmp = path.with_extension("htpasswd.tmp"); let content = if lines.is_empty() { String::new() @@ -326,9 +329,16 @@ fn htpasswd_write(lines: &[String]) -> Result<()> { s.push('\n'); s }; - std::fs::write(&tmp, &content) + let mut f = std::fs::File::create(&tmp) + .with_context(|| format!("create htpasswd tmp {}", tmp.display()))?; + std::io::Write::write_all(&mut f, content.as_bytes()) + .and_then(|()| f.sync_all()) .with_context(|| format!("write htpasswd tmp {}", tmp.display()))?; - std::fs::rename(&tmp, &path) + // Mode set on the TMP file for the same reason as in `write()`: the + // rename carries it, and nginx reads this file as a different user. + std::fs::set_permissions(&tmp, std::fs::Permissions::from_mode(0o644)) + .with_context(|| format!("chmod htpasswd tmp {}", tmp.display()))?; + std::fs::rename(&tmp, path) .with_context(|| format!("rename {} → {}", tmp.display(), path.display()))?; Ok(()) } @@ -396,6 +406,27 @@ mod tests { use super::*; use crate::lifecycle::MANAGER_NAME; + /// The htpasswd file is published `0644` whatever the umask, holds the + /// lines newline-terminated, and leaves no temp file beside it. + #[test] + fn htpasswd_is_written_0644_with_no_temp_left() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("htpasswd"); + std::fs::write(&path, "stale\n").expect("seed"); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)).expect("chmod"); + + htpasswd_write_at(&path, &["a:x".to_owned(), "b:y".to_owned()]).expect("write"); + + let mode = std::fs::metadata(&path).expect("stat").permissions().mode() & 0o777; + assert_eq!(mode, 0o644); + assert_eq!(std::fs::read_to_string(&path).expect("read"), "a:x\nb:y\n"); + let entries: Vec<_> = std::fs::read_dir(dir.path()) + .expect("readdir") + .map(|e| e.expect("entry").file_name()) + .collect(); + assert_eq!(entries, [std::ffi::OsString::from("htpasswd")]); + } + // ── legacy mode (frontend_dir = None) ────────────────────────────── #[test] diff --git a/hivectl/src/wg.rs b/hivectl/src/wg.rs index 4bf9f193..61c436c6 100644 --- a/hivectl/src/wg.rs +++ b/hivectl/src/wg.rs @@ -94,10 +94,7 @@ pub(crate) async fn wg_init(socket: &Path, address: Option<&str>) -> Result<()> String::from_utf8_lossy(&out.stderr).trim() ); } - std::fs::write(key_path, &out.stdout) - .with_context(|| format!("write {}", key_path.display()))?; - std::fs::set_permissions(key_path, std::fs::Permissions::from_mode(0o400)) - .with_context(|| format!("chmod 400 {}", key_path.display()))?; + write_key_0400(key_path, &out.stdout)?; println!("Generated WireGuard private key at {WG_KEY_PATH} (0400)."); } @@ -125,6 +122,33 @@ pub(crate) async fn wg_init(socket: &Path, address: Option<&str>) -> Result<()> Ok(()) } +/// Write `key` to a sibling temp file created `0400` and rename it over +/// `path`, so the key is never readable by anyone else and a crash never +/// leaves half of it at `path`. Errors carry paths, never the key. +fn write_key_0400(path: &Path, key: &[u8]) -> Result<()> { + use std::io::Write as _; + use std::os::unix::fs::OpenOptionsExt as _; + + let mut tmp_name = path + .file_name() + .with_context(|| format!("{} has no file name", path.display()))? + .to_owned(); + tmp_name.push(".tmp"); + let tmp = path.with_file_name(tmp_name); + // A temp file left by a crashed earlier write would fail `create_new`. + let _ = std::fs::remove_file(&tmp); + let mut f = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o400) + .open(&tmp) + .with_context(|| format!("create {}", tmp.display()))?; + f.write_all(key) + .and_then(|()| f.sync_all()) + .with_context(|| format!("write {}", tmp.display()))?; + std::fs::rename(&tmp, path).with_context(|| format!("rename onto {}", path.display())) +} + /// Derive a WireGuard public key from a private key by piping it through /// `wg pubkey`. fn wg_pubkey(privkey: &[u8]) -> Result { @@ -252,7 +276,7 @@ pub(crate) fn wg_status() -> Result<()> { #[cfg(test)] mod tests { - use super::{STORE_PKI_ROOT_PATH, store_root_step}; + use super::{STORE_PKI_ROOT_PATH, store_root_step, write_key_0400}; const PEM: &str = "-----BEGIN CERTIFICATE-----\nMIIB\n-----END CERTIFICATE-----"; @@ -275,4 +299,29 @@ mod tests { assert!(!s.contains("BEGIN CERTIFICATE"), "{s}"); } } + + /// The key lands at `0400` with its bytes intact, replacing whatever was + /// there, and no temp file is left beside it. + #[test] + fn key_is_written_0400_with_no_temp_left() { + use std::os::unix::fs::PermissionsExt as _; + + let dir = std::env::temp_dir().join(format!("hivectl-wg-key-{}", std::process::id())); + std::fs::create_dir_all(&dir).expect("mkdir"); + let path = dir.join("hive.key"); + std::fs::write(&path, "stale").expect("seed"); + + write_key_0400(&path, b"key-bytes\n").expect("write"); + let mode = std::fs::metadata(&path).expect("stat").permissions().mode() & 0o777; + let content = std::fs::read(&path).expect("read"); + let entries: Vec<_> = std::fs::read_dir(&dir) + .expect("readdir") + .map(|e| e.expect("entry").file_name()) + .collect(); + std::fs::remove_dir_all(&dir).ok(); + + assert_eq!(mode, 0o400); + assert_eq!(content, b"key-bytes\n"); + assert_eq!(entries, [std::ffi::OsString::from("hive.key")]); + } } diff --git a/nix/host-modules/hive-forge/default.nix b/nix/host-modules/hive-forge/default.nix index 1dfe06db..f12912c7 100644 --- a/nix/host-modules/hive-forge/default.nix +++ b/nix/host-modules/hive-forge/default.nix @@ -65,6 +65,7 @@ let baoCfg = config.services.hyperhive.swarm.bao; storeRetry = import ../lib/store-retry.nix { }; + atomicWriteSecret = import ../lib/atomic-write-secret.nix { }; havePublisherIdentity = deployCfg.bao.forgePublisherClientCertFile != null && deployCfg.bao.forgePublisherClientKeyFile != null; @@ -916,6 +917,8 @@ in script = '' set -euo pipefail + ${atomicWriteSecret} + # Idempotent by query, same reasoning as the SSO unit: assert # the effect, not the command's exit code. `--admin`: the # controller creates repos in the agents org on the swarm's @@ -982,8 +985,7 @@ in echo "no token-shaped word in forgejo's generate-access-token output" >&2 exit 1 fi - umask 0177 - printf '%s' "$token" > "$token_path" + atomic_write_secret 0600 "" "$token_path" "$token" echo "minted forge access token for ${swarmControllerForgeUser}" fi ''; diff --git a/nix/host-modules/swarm-authelia.nix b/nix/host-modules/swarm-authelia.nix index dfdc5ce6..52324885 100644 --- a/nix/host-modules/swarm-authelia.nix +++ b/nix/host-modules/swarm-authelia.nix @@ -91,6 +91,7 @@ let unitName = lib.removeSuffix ".service" cfg.unit; instance = lib.removePrefix "authelia-" unitName; stateDir = "/var/lib/${unitName}"; + atomicWriteSecret = import ./lib/atomic-write-secret.nix { }; caTrust = import ./lib/hive-ca-trust.nix { inherit lib gatewayCfg; @@ -837,6 +838,8 @@ in script = '' set -euo pipefail + ${atomicWriteSecret} + # Each is generated once and never rotated here: the # session and storage keys are load-bearing for data # already written (sessions, the encrypted store), so @@ -844,7 +847,8 @@ in for f in ${lib.concatStringsSep " " randomKeys}; do p=${lib.escapeShellArg stateDir}/"$f".key if [ ! -s "$p" ]; then - head -c 64 /dev/urandom | od -An -tx1 | tr -d ' \n' > "$p" + value=$(head -c 64 /dev/urandom | od -An -tx1 | tr -d ' \n') + atomic_write_secret 0600 "" "$p" "$value" echo "generated $p" fi chmod 0600 "$p" diff --git a/nix/host-modules/swarm-bao.nix b/nix/host-modules/swarm-bao.nix index a2498c03..519f8e1a 100644 --- a/nix/host-modules/swarm-bao.nix +++ b/nix/host-modules/swarm-bao.nix @@ -3526,8 +3526,8 @@ in # expression, which would render it world-readable into the store. for p in so-pin user-pin; do if [ ! -e ${tokenStoreDir}/$p ]; then - ( umask 077; openssl rand -hex 16 > ${tokenStoreDir}/$p ) - chmod 0400 ${tokenStoreDir}/$p + pin=$(openssl rand -hex 16) + atomic_write_secret 0400 "" ${tokenStoreDir}/$p "$pin" fi done diff --git a/nix/host-modules/swarm-grafana.nix b/nix/host-modules/swarm-grafana.nix index 71b8de85..28291551 100644 --- a/nix/host-modules/swarm-grafana.nix +++ b/nix/host-modules/swarm-grafana.nix @@ -735,10 +735,13 @@ in ]; script = '' set -euo pipefail + + ${atomicWriteSecret} + key=${lib.escapeShellArg secretKeyPath} if [ ! -s "$key" ]; then - umask 077 - openssl rand -hex 32 > "$key" + value=$(openssl rand -hex 32) + atomic_write_secret 0600 "" "$key" "$value" echo "generated Grafana's datasource encryption key" fi ''; diff --git a/swarm-controller/src/webhook.rs b/swarm-controller/src/webhook.rs index e0ad142a..1803d22d 100644 --- a/swarm-controller/src/webhook.rs +++ b/swarm-controller/src/webhook.rs @@ -120,12 +120,39 @@ fn load_or_generate_at(path: &std::path::Path) -> Result { let secret = generate_hex_secret()?; std::fs::create_dir_all(path.parent().unwrap_or(path)) .with_context(|| format!("create dir for {}", path.display()))?; - std::fs::write(path, format!("{secret}\n")) + write_secret_0600(path, format!("{secret}\n").as_bytes()) .with_context(|| format!("write webhook secret to {}", path.display()))?; tracing::info!(path = %path.display(), "webhook secret generated and persisted"); Ok(secret) } +/// Write `contents` to a sibling temp file opened `0600` and rename it over +/// `path`, so the secret is never readable by anyone else and a crash never +/// leaves half of it at `path`. Errors carry paths, never `contents`. +fn write_secret_0600(path: &std::path::Path, contents: &[u8]) -> Result<()> { + use std::io::Write as _; + use std::os::unix::fs::OpenOptionsExt as _; + + let mut tmp_name = path + .file_name() + .with_context(|| format!("{} has no file name", path.display()))? + .to_owned(); + tmp_name.push(".tmp"); + let tmp = path.with_file_name(tmp_name); + // A temp file left by a crashed earlier write would fail `create_new`. + let _ = std::fs::remove_file(&tmp); + let mut f = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .open(&tmp) + .with_context(|| format!("create {}", tmp.display()))?; + f.write_all(contents) + .and_then(|()| f.sync_all()) + .with_context(|| format!("write {}", tmp.display()))?; + std::fs::rename(&tmp, path).with_context(|| format!("rename onto {}", path.display())) +} + /// Read 32 random bytes from `/dev/urandom` and hex-encode them. /// /// `pub(super)` rather than private: `forge::Client::ensure_agent_user` @@ -600,6 +627,30 @@ mod tests { ); } + /// A generated secret lands at `0600` whatever the umask, holds exactly + /// what was returned, and leaves no temp file beside it. + #[test] + fn a_generated_secret_is_0600_with_no_temp_left() { + use std::os::unix::fs::PermissionsExt as _; + + let dir = std::env::temp_dir().join(format!("swarm-hook-mode-{}", std::process::id())); + std::fs::create_dir_all(&dir).expect("mkdir"); + let path = dir.join("webhook-secret"); + + let secret = load_or_generate_at(&path).expect("generates"); + let mode = std::fs::metadata(&path).expect("stat").permissions().mode() & 0o777; + let content = std::fs::read_to_string(&path).expect("read"); + let entries: Vec<_> = std::fs::read_dir(&dir) + .expect("readdir") + .map(|e| e.expect("entry").file_name()) + .collect(); + std::fs::remove_dir_all(&dir).ok(); + + assert_eq!(mode, 0o600); + assert_eq!(content, format!("{secret}\n")); + assert_eq!(entries, [std::ffi::OsString::from("webhook-secret")]); + } + /// A path that exists but cannot be read is an error, and nothing is /// written over it. A directory stands in for the unreadable file because /// its read fails the same way for root and non-root test runners.