diff --git a/src/commands/trust.rs b/src/commands/trust.rs index 6746edd0..ae2a8201 100644 --- a/src/commands/trust.rs +++ b/src/commands/trust.rs @@ -1,9 +1,11 @@ use std::collections::BTreeMap; use std::fs::{self, OpenOptions}; use std::io::Write; +use std::os::unix::fs::OpenOptionsExt; use std::path::{Path, PathBuf}; use anyhow::{Context, Result}; +use bootroot::fs_util; use bootroot::openbao::OpenBaoClient; use bootroot::trust_bootstrap::CA_BUNDLE_PEM_KEY; use serde::{Deserialize, Serialize}; @@ -16,6 +18,12 @@ use crate::state::ServiceEntry; pub(crate) const SERVICE_TRUST_KV_SUFFIX: &str = "trust"; const ROTATION_STATE_FILENAME: &str = "rotation-state.json"; +/// Mode for `rotation-state.json`. Only the rotation command writes +/// and reads it, so it is created owner-only rather than at whatever +/// the umask happens to be — and the same mode on both write paths, +/// so the file does not change permissions depending on which one ran. +const ROTATION_STATE_MODE: u32 = 0o600; + /// Describes which CA components are included in the rotation. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub(crate) enum RotationMode { @@ -58,10 +66,13 @@ pub(crate) fn create_rotation_state( let mut file = OpenOptions::new() .write(true) .create_new(true) + .mode(ROTATION_STATE_MODE) .open(&path) .with_context(|| messages.error_write_file_failed(&path.display().to_string()))?; file.write_all(json.as_bytes()) .with_context(|| messages.error_write_file_failed(&path.display().to_string()))?; + file.sync_all() + .with_context(|| messages.error_write_file_failed(&path.display().to_string()))?; Ok(()) } @@ -74,18 +85,8 @@ pub(crate) fn update_rotation_state( let path = rotation_state_path(state_dir); let json = serde_json::to_string_pretty(state) .with_context(|| messages.error_write_file_failed(&path.display().to_string()))?; - let temp_path = state_dir.join(format!( - "{ROTATION_STATE_FILENAME}.tmp.{}.{}", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::SystemTime::UNIX_EPOCH) - .map_or(0, |d| d.as_nanos()) - )); - fs::write(&temp_path, json.as_bytes()) - .with_context(|| messages.error_write_file_failed(&temp_path.display().to_string()))?; - fs::rename(&temp_path, &path) - .with_context(|| messages.error_write_file_failed(&path.display().to_string()))?; - Ok(()) + fs_util::atomic_write_blocking(&path, json.as_bytes(), ROTATION_STATE_MODE) + .with_context(|| messages.error_write_file_failed(&path.display().to_string())) } /// Loads `rotation-state.json` if it exists. Returns `None` if absent. diff --git a/src/fast_poll.rs b/src/fast_poll.rs index 526195a1..8796ab67 100644 --- a/src/fast_poll.rs +++ b/src/fast_poll.rs @@ -103,6 +103,11 @@ fn log_poll_outcomes(kind: &str, outcomes: &[PollApplyOutcome], needs_relogin: & /// renewals but failed to be acknowledged back to `OpenBao`. Missing /// entries in `last_reissue_seen_version` mean "never seen a reissue /// request". +/// Mode for the fast-poll state file. It is the agent's own resume +/// record — nobody else reads it — so it is created owner-only rather +/// than at whatever the umask happens to be. +const STATE_FILE_MODE: u32 = 0o600; + #[derive(Debug, Default, Serialize, Deserialize, Clone)] pub(crate) struct FastPollState { #[serde(default)] @@ -190,7 +195,9 @@ impl FastPollState { } } - /// Persists the state file atomically via a `.tmp` rename. + /// Persists the state file atomically, owner-only and flushed: it + /// is what the next tick resumes from, so it has to survive a + /// power loss rather than merely replace cleanly. pub(crate) async fn save(&self, path: &Path) -> Result<()> { if let Some(parent) = path.parent() && !parent.as_os_str().is_empty() @@ -202,27 +209,11 @@ impl FastPollState { ) })?; } - let tmp_path: PathBuf = { - let mut os = path.as_os_str().to_owned(); - os.push(".tmp"); - PathBuf::from(os) - }; let body = serde_json::to_string_pretty(self).context("Failed to serialize fast-poll state")?; - fs::write(&tmp_path, body).await.with_context(|| { - format!( - "Failed to write fast-poll state tmp file at {}", - tmp_path.display() - ) - })?; - fs::rename(&tmp_path, path).await.with_context(|| { - format!( - "Failed to rename {} -> {}", - tmp_path.display(), - path.display() - ) - })?; - Ok(()) + fs_util::atomic_write(path, body.as_bytes(), STATE_FILE_MODE) + .await + .with_context(|| format!("Failed to write fast-poll state at {}", path.display())) } } diff --git a/src/fs_util.rs b/src/fs_util.rs index 2f871d99..bcd2b23a 100644 --- a/src/fs_util.rs +++ b/src/fs_util.rs @@ -279,60 +279,73 @@ async fn write_owned_impl(path: &Path, contents: &[u8], mode: u32, replace: bool /// permissioned, chowned (when ownership preservation is needed), or /// renamed. pub async fn atomic_write(path: &Path, contents: &[u8], mode: u32) -> Result<()> { + // Owned once, to move into the blocking task. The blocking half + // borrows, so this is the only copy either path makes. + let dest = path.to_path_buf(); + let payload = contents.to_vec(); + tokio::task::spawn_blocking(move || atomic_write_blocking(&dest, &payload, mode)) + .await + .context("Atomic write task panicked")? +} + +/// The blocking half of [`atomic_write`], for callers that are not async. +/// +/// Same guarantees, same order: staged in the destination's directory, +/// written, `sync_all`ed, permissioned, ownership-preserved, renamed. +/// Callers in an async context use [`atomic_write`] instead, which runs +/// this on a blocking thread. +/// +/// # Errors +/// Returns an error under the same conditions as [`atomic_write`]. +pub fn atomic_write_blocking(path: &Path, contents: &[u8], mode: u32) -> Result<()> { let parent = path .parent() .filter(|p| !p.as_os_str().is_empty()) .map_or_else(|| std::path::PathBuf::from("."), Path::to_path_buf); - let dest = path.to_path_buf(); - let payload = contents.to_vec(); - tokio::task::spawn_blocking(move || -> Result<()> { - // Capture the existing destination's uid/gid (if any) so the - // rename does not strip operator-meaningful ownership. Missing - // file -> None; do not chown the staged file in that case so a - // fresh create keeps process default ownership. - let existing_owner = std::fs::metadata(&dest).ok().map(|m| (m.uid(), m.gid())); - - let mut tmp = tempfile::NamedTempFile::new_in(&parent) - .with_context(|| format!("Failed to create temp file in {}", parent.display()))?; - tmp.as_file_mut() - .write_all(&payload) - .with_context(|| format!("Failed to write temp file for {}", dest.display()))?; - tmp.as_file_mut() - .sync_all() - .with_context(|| format!("Failed to fsync temp file for {}", dest.display()))?; - std::fs::set_permissions(tmp.path(), std::fs::Permissions::from_mode(mode)).with_context( - || { - format!( - "Failed to set mode {mode:o} on temp file for {}", - dest.display() - ) - }, - )?; - if let Some((dest_uid, dest_gid)) = existing_owner { - let tmp_meta = std::fs::metadata(tmp.path()) - .with_context(|| format!("Failed to stat temp file for {}", dest.display()))?; - if tmp_meta.uid() != dest_uid || tmp_meta.gid() != dest_gid { - std::os::unix::fs::chown(tmp.path(), Some(dest_uid), Some(dest_gid)).with_context( - || { - format!( - "Failed to preserve existing uid={dest_uid} gid={dest_gid} on {}", - dest.display() - ) - }, - )?; - } - } - tmp.persist(&dest).map_err(|e| { - anyhow::anyhow!( - "Failed to rename temp file to {}: {}", - dest.display(), - e.error + // Capture the existing destination's uid/gid (if any) so the + // rename does not strip operator-meaningful ownership. Missing + // file -> None; do not chown the staged file in that case so a + // fresh create keeps process default ownership. + let existing_owner = std::fs::metadata(path).ok().map(|m| (m.uid(), m.gid())); + + let mut tmp = tempfile::NamedTempFile::new_in(&parent) + .with_context(|| format!("Failed to create temp file in {}", parent.display()))?; + tmp.as_file_mut() + .write_all(contents) + .with_context(|| format!("Failed to write temp file for {}", path.display()))?; + tmp.as_file_mut() + .sync_all() + .with_context(|| format!("Failed to fsync temp file for {}", path.display()))?; + std::fs::set_permissions(tmp.path(), std::fs::Permissions::from_mode(mode)).with_context( + || { + format!( + "Failed to set mode {mode:o} on temp file for {}", + path.display() ) - })?; - Ok(()) - }) - .await - .context("Atomic write task panicked")? + }, + )?; + if let Some((dest_uid, dest_gid)) = existing_owner { + let tmp_meta = std::fs::metadata(tmp.path()) + .with_context(|| format!("Failed to stat temp file for {}", path.display()))?; + if tmp_meta.uid() != dest_uid || tmp_meta.gid() != dest_gid { + std::os::unix::fs::chown(tmp.path(), Some(dest_uid), Some(dest_gid)).with_context( + || { + format!( + "Failed to preserve existing uid={dest_uid} gid={dest_gid} on {}", + path.display() + ) + }, + )?; + } + } + tmp.persist(path).map_err(|e| { + anyhow::anyhow!( + "Failed to rename temp file to {}: {}", + path.display(), + e.error + ) + })?; + Ok(()) } /// Ensures the secrets directory exists and has secure permissions.