From 15361c575bb04235bb17128d4297e6412b312aa4 Mon Sep 17 00:00:00 2001 From: Srikar Date: Fri, 17 Jul 2026 12:43:05 -0700 Subject: [PATCH] fix(preingestion): create unified BFB with 0o600 perms --- .../src/bfb_rshim_copier.rs | 121 ++++++++++++++---- 1 file changed, 97 insertions(+), 24 deletions(-) diff --git a/crates/preingestion-manager/src/bfb_rshim_copier.rs b/crates/preingestion-manager/src/bfb_rshim_copier.rs index d76636b365..e84acf5763 100644 --- a/crates/preingestion-manager/src/bfb_rshim_copier.rs +++ b/crates/preingestion-manager/src/bfb_rshim_copier.rs @@ -19,7 +19,7 @@ use std::sync::Arc; use std::time::Duration; use carbide_secrets::credentials::{CredentialKey, CredentialReader, Credentials}; -use tokio::fs::{self, File}; +use tokio::fs::{self, File, OpenOptions}; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::sync::Mutex; @@ -159,31 +159,56 @@ impl BfbRshimCopier { ) -> Result<(), BfbRshimCopyError> { let _lock = self.bfb_file_lock.lock().await; - if fs::metadata(UNIFIED_PREINGESTION_BFB_PATH).await.is_err() { - tracing::info!( - path = UNIFIED_PREINGESTION_BFB_PATH, - "Writing unified preingestion BFB" - ); + Self::write_unified_preingestion_bfb( + PREINGESTION_BFB_PATH, + UNIFIED_PREINGESTION_BFB_PATH, + username, + password, + ) + .await + } + + /// Assemble the unified pre-ingestion BFB by concatenating the base BFB with + /// a `bf.cfg` blob carrying the BMC credentials. + /// + /// The `bf.cfg` blob embeds the BMC root password in cleartext, so the + /// artifact is created with `0o600` (owner read/write only). The mode is + /// applied atomically by `open` at creation time rather than being left to + /// the process umask, so the password is never momentarily readable by other + /// local users/processes. + async fn write_unified_preingestion_bfb( + source_bfb_path: &str, + unified_bfb_path: &str, + username: &str, + password: &str, + ) -> Result<(), BfbRshimCopyError> { + if fs::metadata(unified_bfb_path).await.is_err() { + tracing::info!(path = unified_bfb_path, "Writing unified preingestion BFB"); let bf_cfg_contents = format!( "BMC_USER=\"{username}\"\nBMC_PASSWORD=\"{password}\"\nBMC_REBOOT=\"yes\"\nCEC_REBOOT=\"yes\"\n" ); - let mut preingestion_bfb = File::open(PREINGESTION_BFB_PATH).await.map_err(|err| { - BfbRshimCopyError::Other { - details: format!("failed to open {PREINGESTION_BFB_PATH}: {err}"), - } - })?; - - let mut unified_bfb = - File::create(UNIFIED_PREINGESTION_BFB_PATH) + let mut preingestion_bfb = + File::open(source_bfb_path) .await .map_err(|err| BfbRshimCopyError::Other { - details: format!("failed to create {UNIFIED_PREINGESTION_BFB_PATH}: {err}"), + details: format!("failed to open {source_bfb_path}: {err}"), })?; + let mut unified_bfb = OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .mode(0o600) + .open(unified_bfb_path) + .await + .map_err(|err| BfbRshimCopyError::Other { + details: format!("failed to create {unified_bfb_path}: {err}"), + })?; + let mut buffer = vec![0; 1024 * 1024].into_boxed_slice(); // 1 MB buffer - tracing::info!(path = UNIFIED_PREINGESTION_BFB_PATH, "Writing BFB payload"); + tracing::info!(path = unified_bfb_path, "Writing BFB payload"); loop { let n = preingestion_bfb.read(&mut buffer).await.map_err(|err| { BfbRshimCopyError::Other { @@ -197,17 +222,12 @@ impl BfbRshimCopier { unified_bfb.write_all(&buffer[..n]).await.map_err(|err| { BfbRshimCopyError::Other { - details: format!( - "failed to write BFB to {UNIFIED_PREINGESTION_BFB_PATH}: {err}" - ), + details: format!("failed to write BFB to {unified_bfb_path}: {err}"), } })?; } - tracing::info!( - path = UNIFIED_PREINGESTION_BFB_PATH, - "Writing bf.cfg payload" - ); + tracing::info!(path = unified_bfb_path, "Writing bf.cfg payload"); unified_bfb .write_all(bf_cfg_contents.as_bytes()) @@ -220,7 +240,7 @@ impl BfbRshimCopier { .sync_all() .await .map_err(|err| BfbRshimCopyError::Other { - details: format!("failed to flush {UNIFIED_PREINGESTION_BFB_PATH}: {err}"), + details: format!("failed to flush {unified_bfb_path}: {err}"), })?; } @@ -257,3 +277,56 @@ impl BfbRshimCopier { }) } } + +#[cfg(test)] +mod tests { + use std::os::unix::fs::PermissionsExt; + + use super::*; + + /// The unified BFB embeds the BMC root password in cleartext, so the + /// assembled artifact must be readable only by its owner (`0o600`), + /// independent of the process umask. + #[tokio::test] + async fn write_unified_preingestion_bfb_creates_owner_only_artifact() { + let dir = tempfile::tempdir().expect("create tempdir"); + let source_path = dir.path().join("preingestion.bfb"); + let unified_path = dir.path().join("preingestion_unified_update.bfb"); + + let base_payload = b"base-bfb-payload"; + fs::write(&source_path, base_payload) + .await + .expect("write source bfb"); + + BfbRshimCopier::write_unified_preingestion_bfb( + source_path.to_str().expect("utf-8 source path"), + unified_path.to_str().expect("utf-8 unified path"), + "root", + "s3cr3t-p@ssw0rd", + ) + .await + .expect("write unified bfb"); + + let mode = fs::metadata(&unified_path) + .await + .expect("stat unified bfb") + .permissions() + .mode(); + assert_eq!( + mode & 0o777, + 0o600, + "unified BFB must be owner-only, got {:o}", + mode & 0o777 + ); + + // The base BFB is concatenated with the bf.cfg blob carrying the creds. + let written = fs::read(&unified_path).await.expect("read unified bfb"); + assert!( + written.starts_with(base_payload), + "unified BFB must start with the base BFB payload" + ); + let bf_cfg = String::from_utf8_lossy(&written[base_payload.len()..]); + assert!(bf_cfg.contains("BMC_USER=\"root\"")); + assert!(bf_cfg.contains("BMC_PASSWORD=\"s3cr3t-p@ssw0rd\"")); + } +}