Watch
0
0
Fork
You've already forked hyperhive
0

write secret and key files atomically with their final mode

Each of these wrote its file in place (or set the mode after the
write), so a write cut short by a kill or a full disk left a partial
file behind, and some secrets existed for a moment at umask mode:

- swarm-authelia/swarm-grafana/hive-forge/swarm-bao generators: a
  partial file passed the `-s`/`-e` guard and was kept forever. They now
  capture the value first and land it with `atomic_write_secret`.
- swarm-controller webhook secret: a truncated file failed the 64-hex
  check and the secret was regenerated, out of step with the forge
  hooks. It is now written 0600 to a sibling temp, fsynced, renamed.
- hive-c0re htpasswd: the temp is now fsynced and set 0644 before the
  rename, like the sibling agents.conf writer, instead of relying on
  c0re's umask.
- hivectl wg init: the private key is created 0400 in a temp and
  renamed, instead of written at umask mode and chmod'ed after.

Closes #5068
This commit is contained in:
atlas 2026-10-09 15:53:39 +02:00 • committed by mara
commit 01b8387ab8
7 changed files with 156 additions and 16 deletions

View file

@ -317,7 +317,10 @@ fn htpasswd_read() -> Result<Vec<String>> {
/// Write lines back atomically (`<path>.tmp` then rename), with a trailing /// Write lines back atomically (`<path>.tmp` then rename), with a trailing
/// newline. /// newline.
fn htpasswd_write(lines: &[String]) -> Result<()> { 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 tmp = path.with_extension("htpasswd.tmp");
let content = if lines.is_empty() { let content = if lines.is_empty() {
String::new() String::new()
@ -326,9 +329,16 @@ fn htpasswd_write(lines: &[String]) -> Result<()> {
s.push('\n'); s.push('\n');
s 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()))?; .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()))?; .with_context(|| format!("rename {} → {}", tmp.display(), path.display()))?;
Ok(()) Ok(())
} }
@ -396,6 +406,27 @@ mod tests {
use super::*; use super::*;
use crate::lifecycle::MANAGER_NAME; 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) ────────────────────────────── // ── legacy mode (frontend_dir = None) ──────────────────────────────
#[test] #[test]

View file

@ -94,10 +94,7 @@ pub(crate) async fn wg_init(socket: &Path, address: Option<&str>) -> Result<()>
String::from_utf8_lossy(&out.stderr).trim() String::from_utf8_lossy(&out.stderr).trim()
); );
} }
std::fs::write(key_path, &out.stdout) write_key_0400(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()))?;
println!("Generated WireGuard private key at {WG_KEY_PATH} (0400)."); 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(()) 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 /// Derive a WireGuard public key from a private key by piping it through
/// `wg pubkey`. /// `wg pubkey`.
fn wg_pubkey(privkey: &[u8]) -> Result<String> { fn wg_pubkey(privkey: &[u8]) -> Result<String> {
@ -252,7 +276,7 @@ pub(crate) fn wg_status() -> Result<()> {
#[cfg(test)] #[cfg(test)]
mod tests { 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-----"; const PEM: &str = "-----BEGIN CERTIFICATE-----\nMIIB\n-----END CERTIFICATE-----";
@ -275,4 +299,29 @@ mod tests {
assert!(!s.contains("BEGIN CERTIFICATE"), "{s}"); 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")]);
}
} }

View file

@ -65,6 +65,7 @@ let
baoCfg = config.services.hyperhive.swarm.bao; baoCfg = config.services.hyperhive.swarm.bao;
storeRetry = import ../lib/store-retry.nix { }; storeRetry = import ../lib/store-retry.nix { };
atomicWriteSecret = import ../lib/atomic-write-secret.nix { };
havePublisherIdentity = havePublisherIdentity =
deployCfg.bao.forgePublisherClientCertFile != null deployCfg.bao.forgePublisherClientCertFile != null
&& deployCfg.bao.forgePublisherClientKeyFile != null; && deployCfg.bao.forgePublisherClientKeyFile != null;
@ -916,6 +917,8 @@ in
script = '' script = ''
set -euo pipefail set -euo pipefail
${atomicWriteSecret}
# Idempotent by query, same reasoning as the SSO unit: assert # Idempotent by query, same reasoning as the SSO unit: assert
# the effect, not the command's exit code. `--admin`: the # the effect, not the command's exit code. `--admin`: the
# controller creates repos in the agents org on the swarm's # 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 echo "no token-shaped word in forgejo's generate-access-token output" >&2
exit 1 exit 1
fi fi
umask 0177 atomic_write_secret 0600 "" "$token_path" "$token"
printf '%s' "$token" > "$token_path"
echo "minted forge access token for ${swarmControllerForgeUser}" echo "minted forge access token for ${swarmControllerForgeUser}"
fi fi
''; '';

View file

@ -91,6 +91,7 @@ let
unitName = lib.removeSuffix ".service" cfg.unit; unitName = lib.removeSuffix ".service" cfg.unit;
instance = lib.removePrefix "authelia-" unitName; instance = lib.removePrefix "authelia-" unitName;
stateDir = "/var/lib/${unitName}"; stateDir = "/var/lib/${unitName}";
atomicWriteSecret = import ./lib/atomic-write-secret.nix { };
caTrust = import ./lib/hive-ca-trust.nix { caTrust = import ./lib/hive-ca-trust.nix {
inherit lib gatewayCfg; inherit lib gatewayCfg;
@ -837,6 +838,8 @@ in
script = '' script = ''
set -euo pipefail set -euo pipefail
${atomicWriteSecret}
# Each is generated once and never rotated here: the # Each is generated once and never rotated here: the
# session and storage keys are load-bearing for data # session and storage keys are load-bearing for data
# already written (sessions, the encrypted store), so # already written (sessions, the encrypted store), so
@ -844,7 +847,8 @@ in
for f in ${lib.concatStringsSep " " randomKeys}; do for f in ${lib.concatStringsSep " " randomKeys}; do
p=${lib.escapeShellArg stateDir}/"$f".key p=${lib.escapeShellArg stateDir}/"$f".key
if [ ! -s "$p" ]; then 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" echo "generated $p"
fi fi
chmod 0600 "$p" chmod 0600 "$p"

View file

@ -3526,8 +3526,8 @@ in
# expression, which would render it world-readable into the store. # expression, which would render it world-readable into the store.
for p in so-pin user-pin; do for p in so-pin user-pin; do
if [ ! -e ${tokenStoreDir}/$p ]; then if [ ! -e ${tokenStoreDir}/$p ]; then
( umask 077; openssl rand -hex 16 > ${tokenStoreDir}/$p ) pin=$(openssl rand -hex 16)
chmod 0400 ${tokenStoreDir}/$p atomic_write_secret 0400 "" ${tokenStoreDir}/$p "$pin"
fi fi
done done

View file

@ -735,10 +735,13 @@ in
]; ];
script = '' script = ''
set -euo pipefail set -euo pipefail
${atomicWriteSecret}
key=${lib.escapeShellArg secretKeyPath} key=${lib.escapeShellArg secretKeyPath}
if [ ! -s "$key" ]; then if [ ! -s "$key" ]; then
umask 077 value=$(openssl rand -hex 32)
openssl rand -hex 32 > "$key" atomic_write_secret 0600 "" "$key" "$value"
echo "generated Grafana's datasource encryption key" echo "generated Grafana's datasource encryption key"
fi fi
''; '';

View file

@ -120,12 +120,39 @@ fn load_or_generate_at(path: &std::path::Path) -> Result<String> {
let secret = generate_hex_secret()?; let secret = generate_hex_secret()?;
std::fs::create_dir_all(path.parent().unwrap_or(path)) std::fs::create_dir_all(path.parent().unwrap_or(path))
.with_context(|| format!("create dir for {}", path.display()))?; .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()))?; .with_context(|| format!("write webhook secret to {}", path.display()))?;
tracing::info!(path = %path.display(), "webhook secret generated and persisted"); tracing::info!(path = %path.display(), "webhook secret generated and persisted");
Ok(secret) 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. /// Read 32 random bytes from `/dev/urandom` and hex-encode them.
/// ///
/// `pub(super)` rather than private: `forge::Client::ensure_agent_user` /// `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 /// 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 /// 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. /// its read fails the same way for root and non-root test runners.