From aa10bd12473da604e282b05e2ca150243d3e4784 Mon Sep 17 00:00:00 2001 From: archipelago Date: Tue, 6 Oct 2026 01:30:04 -0400 Subject: [PATCH] Fail closed when persistent session signing material is unavailable --- core/archipelago/src/api/rpc/middleware.rs | 6 +- core/archipelago/src/api/rpc/mod.rs | 37 ++-- core/archipelago/src/appgate/mod.rs | 35 +++- core/archipelago/src/session.rs | 43 ++-- core/archipelago/src/session_secret.rs | 219 +++++++++++++++++++++ docs/session-recovery-followup.md | 26 +++ 6 files changed, 316 insertions(+), 50 deletions(-) create mode 100644 core/archipelago/src/session_secret.rs diff --git a/core/archipelago/src/api/rpc/middleware.rs b/core/archipelago/src/api/rpc/middleware.rs index 643d351b..541c27da 100644 --- a/core/archipelago/src/api/rpc/middleware.rs +++ b/core/archipelago/src/api/rpc/middleware.rs @@ -317,14 +317,14 @@ mod sanitize_tests { /// Deterministic: same session token always produces the same CSRF token. /// Survives backend restarts because it depends only on the session token /// and the on-disk remember secret (not ephemeral state). -pub(crate) async fn derive_csrf_token(session_token: &str) -> String { +pub(crate) async fn derive_csrf_token(session_token: &str) -> std::io::Result { use hmac::{Hmac, Mac}; use sha2::Sha256; type HmacSha256 = Hmac; - let secret = SessionStore::load_or_create_remember_secret().await; + let secret = SessionStore::load_or_create_remember_secret().await?; let mut mac = HmacSha256::new_from_slice(&secret).expect("HMAC key"); mac.update(format!("csrf:{}", session_token).as_bytes()); - hex::encode(mac.finalize().into_bytes()) + Ok(hex::encode(mac.finalize().into_bytes())) } /// Extract a named cookie value from headers. diff --git a/core/archipelago/src/api/rpc/mod.rs b/core/archipelago/src/api/rpc/mod.rs index a926d54e..ac6a2452 100644 --- a/core/archipelago/src/api/rpc/mod.rs +++ b/core/archipelago/src/api/rpc/mod.rs @@ -347,6 +347,16 @@ impl RpcHandler { // Enforce authentication for non-allowlisted methods let is_unauthenticated = UNAUTHENTICATED_METHODS.contains(&rpc_req.method.as_str()); + if !is_unauthenticated || rpc_req.method.starts_with("auth.") { + if let Err(error) = SessionStore::load_or_create_remember_secret().await { + tracing::error!(%error, "Persistent session signing key unavailable"); + return Ok(self.error_response( + 503, + "Sign-in temporarily unavailable. Check server session storage.", + StatusCode::SERVICE_UNAVAILABLE, + )); + } + } let mut new_session_cookies: Option<(String, String)> = None; if !is_unauthenticated { let mut authenticated = match &session_token { @@ -359,7 +369,7 @@ impl RpcHandler { if let Some(remember) = extract_cookie(&parts.headers, "remember") { if crate::session::SessionStore::validate_remember_token(&remember).await { let new_token = self.session_store.create().await; - let new_csrf = derive_csrf_token(&new_token).await; + let new_csrf = derive_csrf_token(&new_token).await?; tracing::info!("Auto-restored session from remember-me token"); new_session_cookies = Some((new_token, new_csrf)); authenticated = true; @@ -407,7 +417,7 @@ impl RpcHandler { use hmac::{Hmac, Mac}; use sha2::Sha256; type HmacSha256 = Hmac; - let secret = SessionStore::load_or_create_remember_secret().await; + let secret = SessionStore::load_or_create_remember_secret().await?; let mut mac = match HmacSha256::new_from_slice(&secret) { Ok(m) => m, Err(_) => { @@ -438,7 +448,7 @@ impl RpcHandler { if let Some(token) = &session_token { self.set_csrf_cookie( &mut response, - &derive_csrf_token(token).await, + &derive_csrf_token(token).await?, secure_suffix, ); } @@ -550,7 +560,7 @@ impl RpcHandler { client_ip, secure_suffix, ) - .await; + .await?; Ok(response) } @@ -602,7 +612,7 @@ impl RpcHandler { new_session_cookies: &Option<(String, String)>, client_ip: std::net::IpAddr, secure_suffix: &str, - ) { + ) -> Result<()> { // Track failed login attempts for rate limiting if method == "auth.login" && rpc_resp.error.is_some() { self.login_rate_limiter.record_failure(client_ip).await; @@ -642,7 +652,7 @@ impl RpcHandler { if let Ok(Some(totp_data)) = self.auth_manager.get_totp_data().await { if let Ok(secret) = crate::totp::decrypt_secret(&totp_data, password) { let token = self.session_store.create_pending(secret).await; - let csrf_token = derive_csrf_token(&token).await; + let csrf_token = derive_csrf_token(&token).await?; self.set_session_cookie(response, &token, secure_suffix); self.set_csrf_cookie(response, &csrf_token, secure_suffix); let totp_body = serde_json::json!({ @@ -655,8 +665,8 @@ impl RpcHandler { } } else { let token = self.session_store.create().await; - let csrf_token = derive_csrf_token(&token).await; - let remember_token = self.session_store.create_remember_token().await; + let csrf_token = derive_csrf_token(&token).await?; + let remember_token = self.session_store.create_remember_token().await?; self.set_session_cookie(response, &token, secure_suffix); self.set_csrf_cookie(response, &csrf_token, secure_suffix); self.set_remember_cookie(response, &remember_token, secure_suffix); @@ -675,8 +685,8 @@ impl RpcHandler { .map(|s| s.to_string()); if let Some(new_token) = new_token_opt { - let csrf_token = derive_csrf_token(&new_token).await; - let remember_token = self.session_store.create_remember_token().await; + let csrf_token = derive_csrf_token(&new_token).await?; + let remember_token = self.session_store.create_remember_token().await?; self.set_session_cookie(response, &new_token, secure_suffix); self.set_csrf_cookie(response, &csrf_token, secure_suffix); self.set_remember_cookie(response, &remember_token, secure_suffix); @@ -695,7 +705,7 @@ impl RpcHandler { if method == "auth.changePassword" && rpc_resp.error.is_none() { if let Some(token) = session_token { let new_token = self.session_store.rotate(token).await; - let csrf_token = derive_csrf_token(&new_token).await; + let csrf_token = derive_csrf_token(&new_token).await?; self.set_session_cookie(response, &new_token, secure_suffix); self.set_csrf_cookie(response, &csrf_token, secure_suffix); } @@ -727,6 +737,7 @@ impl RpcHandler { self.set_session_cookie(response, new_session, secure_suffix); self.set_csrf_cookie(response, new_csrf, secure_suffix); } + Ok(()) } fn set_session_cookie( @@ -891,7 +902,7 @@ mod csrf_recovery_tests { .map(|v| v.to_str().unwrap()) .collect(); assert_eq!(cookies.len(), 1); - let expected = derive_csrf_token(&token).await; + let expected = derive_csrf_token(&token).await.unwrap(); assert_eq!( cookies[0], format!("csrf_token={expected}; SameSite=Lax; Path=/; Secure") @@ -906,7 +917,7 @@ mod csrf_recovery_tests { assert_eq!(stranger.status(), StatusCode::UNAUTHORIZED); assert!(!stranger.headers().contains_key("set-cookie")); assert!(!settings.exists()); - let valid = derive_csrf_token(&token).await; + let valid = derive_csrf_token(&token).await.unwrap(); let response = handler .handle(request(&token, Some(&valid), false)) .await diff --git a/core/archipelago/src/appgate/mod.rs b/core/archipelago/src/appgate/mod.rs index 76ba2268..d82fef6d 100644 --- a/core/archipelago/src/appgate/mod.rs +++ b/core/archipelago/src/appgate/mod.rs @@ -700,6 +700,14 @@ fn strip_gate_cookies(headers: &mut hyper::HeaderMap) { } async fn set_session_cookie(resp: &mut Response, token: &str) { + // The dashboard RPC layer requires a readable CSRF cookie as well as the + // HttpOnly session cookie. An app-gate login is a complete node login, so + // it must establish the same pair as auth.login; otherwise a fresh browser + // can open the signer broker but every identity/signing RPC is rejected + // with `has_session=true, has_header=false`. + if !set_csrf_cookie(resp, token).await { + return; + } // No Domain attribute, so the cookie is host-only. Cookies ignore port, // which is what makes one sign-in cover the dashboard and every app port // on the same host — and equally why an app on a *different* host (its @@ -709,22 +717,29 @@ async fn set_session_cookie(resp: &mut Response, token: &str) { { resp.headers_mut().append(header::SET_COOKIE, value); } - - // The dashboard RPC layer requires a readable CSRF cookie as well as the - // HttpOnly session cookie. An app-gate login is a complete node login, so - // it must establish the same pair as auth.login; otherwise a fresh browser - // can open the signer broker but every identity/signing RPC is rejected - // with `has_session=true, has_header=false`. - set_csrf_cookie(resp, token).await; } -async fn set_csrf_cookie(resp: &mut Response, token: &str) { - let csrf = crate::api::rpc::derive_csrf_token(token).await; +async fn set_csrf_cookie(resp: &mut Response, token: &str) -> bool { + let csrf = match crate::api::rpc::derive_csrf_token(token).await { + Ok(csrf) => csrf, + Err(error) => { + tracing::error!(%error, "App login could not load persistent session signing key"); + *resp = Response::builder() + .status(StatusCode::SERVICE_UNAVAILABLE) + .header(header::CACHE_CONTROL, "no-store") + .body(Body::from( + "Sign-in temporarily unavailable. Check server session storage.", + )) + .expect("static error response"); + return false; + } + }; if let Ok(value) = header::HeaderValue::from_str(&format!("csrf_token={csrf}; SameSite=Lax; Path=/")) { resp.headers_mut().append(header::SET_COOKIE, value); } + true } fn cookie_value(headers: &HeaderMap, name: &str) -> Option { @@ -1691,7 +1706,7 @@ mod tests { .iter() .filter_map(|value| value.to_str().ok()) .collect(); - let expected_csrf = crate::api::rpc::derive_csrf_token(token).await; + let expected_csrf = crate::api::rpc::derive_csrf_token(token).await.unwrap(); assert!(cookies .iter() .any(|cookie| cookie.starts_with(&format!("session={token};")))); diff --git a/core/archipelago/src/session.rs b/core/archipelago/src/session.rs index 13b44e4f..b94cec3d 100644 --- a/core/archipelago/src/session.rs +++ b/core/archipelago/src/session.rs @@ -1,5 +1,6 @@ +#[path = "session_secret.rs"] +mod secret_file; use hmac::{Hmac, Mac}; -use rand::RngCore; use sha2::{Digest, Sha256}; use std::collections::HashMap; use std::path::{Path, PathBuf}; @@ -398,8 +399,8 @@ impl SessionStore { // Format: "timestamp_hex:hmac_hex" /// Create a remember-me token. Returns the cookie value. - pub async fn create_remember_token(&self) -> String { - let secret = Self::load_or_create_remember_secret().await; + pub async fn create_remember_token(&self) -> std::io::Result { + let secret = Self::load_or_create_remember_secret().await?; let now = SystemTime::now() .duration_since(UNIX_EPOCH) .unwrap_or_default() @@ -408,13 +409,17 @@ impl SessionStore { let mut mac = HmacSha256::new_from_slice(&secret).expect("HMAC key"); mac.update(format!("remember:{}", ts_hex).as_bytes()); let sig = hex::encode(mac.finalize().into_bytes()); - format!("{}:{}", ts_hex, sig) + Ok(format!("{}:{}", ts_hex, sig)) } /// Validate a remember-me token. Returns true if valid and not expired. pub async fn validate_remember_token(token: &str) -> bool { - let secret = match tokio::fs::read(REMEMBER_SECRET_FILE).await { - Ok(s) if s.len() == 32 => s, + let secret = match tokio::task::spawn_blocking(|| { + secret_file::read_existing(Path::new(REMEMBER_SECRET_FILE)) + }) + .await + { + Ok(Ok(s)) => s, _ => return false, }; let parts: Vec<&str> = token.splitn(2, ':').collect(); @@ -454,27 +459,17 @@ impl SessionStore { now.saturating_sub(ts_bytes) < REMEMBER_TTL } - pub async fn load_or_create_remember_secret() -> Vec { + pub async fn load_or_create_remember_secret() -> std::io::Result> { REMEMBER_SECRET - .get_or_init(|| async { - // Try existing secret file first - if let Ok(secret) = tokio::fs::read(REMEMBER_SECRET_FILE).await { - if secret.len() == 32 { - return secret; - } - } - // Generate a cryptographically random 32-byte secret on first boot - let mut secret = [0u8; 32]; - rand::rngs::OsRng.fill_bytes(&mut secret); - // Ensure parent directory exists - if let Some(parent) = std::path::Path::new(REMEMBER_SECRET_FILE).parent() { - let _ = tokio::fs::create_dir_all(parent).await; - } - let _ = tokio::fs::write(REMEMBER_SECRET_FILE, &secret).await; - secret.to_vec() + .get_or_try_init(|| async { + tokio::task::spawn_blocking(|| { + secret_file::load_or_create(Path::new(REMEMBER_SECRET_FILE)) + }) + .await + .map_err(std::io::Error::other)? }) .await - .clone() + .cloned() } } diff --git a/core/archipelago/src/session_secret.rs b/core/archipelago/src/session_secret.rs new file mode 100644 index 00000000..f9377f2a --- /dev/null +++ b/core/archipelago/src/session_secret.rs @@ -0,0 +1,219 @@ +//! Persistent signing material must never fall back to an ephemeral key. +use std::fs::{self, File, OpenOptions}; +use std::io::{self, Read, Write}; +use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; +use std::path::{Path, PathBuf}; + +pub(super) fn read_existing(path: &Path) -> io::Result> { + let file = OpenOptions::new() + .read(true) + .custom_flags(libc::O_NOFOLLOW | libc::O_NONBLOCK) + .open(path)?; + let metadata = file.metadata()?; + if !metadata.is_file() || metadata.len() != 32 { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "Session key must be a regular 32-byte file; existing data was preserved", + )); + } + // Tighten legacy modes on the opened inode. Permission failures are errors, + // never permission to replace a valid key or start with a temporary one. + if metadata.permissions().mode() & 0o777 != 0o600 { + file.set_permissions(fs::Permissions::from_mode(0o600))?; + file.sync_all()?; + } + let mut bytes = Vec::with_capacity(32); + file.take(33).read_to_end(&mut bytes)?; + if bytes.len() != 32 { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "Session key changed while reading", + )); + } + Ok(bytes) +} + +struct TemporaryKey(PathBuf); +impl Drop for TemporaryKey { + fn drop(&mut self) { + let _ = fs::remove_file(&self.0); + } +} + +pub(super) fn load_or_create(path: &Path) -> io::Result> { + match read_existing(path) { + Ok(key) => return Ok(key), + Err(error) if error.kind() == io::ErrorKind::NotFound => {} + Err(error) => return Err(error), + } + let parent = path.parent().ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "Session key has no parent directory", + ) + })?; + fs::create_dir_all(parent)?; + let mut key = [0u8; 32]; + crate::entropy::draw_key_bytes(&mut rand::rngs::OsRng, &mut key) + .map_err(|_| io::Error::other("Session key entropy unavailable"))?; + // Publish only a fully written, synced private inode. A hard link provides + // no-replace semantics: concurrent creators all read the one winning key. + // The temporary filename is independent of the secret. + let temporary_path = parent.join(format!(".session-key-{}", uuid::Uuid::new_v4())); + let mut file = OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .open(&temporary_path)?; + let temporary = TemporaryKey(temporary_path); + file.write_all(&key)?; + file.sync_all()?; + match fs::hard_link(&temporary.0, path) { + Ok(()) => {} + Err(error) if error.kind() == io::ErrorKind::AlreadyExists => {} + Err(error) => return Err(error), + } + drop(temporary); + File::open(parent)?.sync_all()?; + read_existing(path) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::os::unix::fs::symlink; + + #[test] + fn durable_private_key_survives_reload_without_rotation() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("key"); + let key = load_or_create(&path).unwrap(); + assert_eq!(key.len(), 32); + assert_eq!(key, load_or_create(&path).unwrap()); + assert_eq!( + fs::metadata(path).unwrap().permissions().mode() & 0o777, + 0o600 + ); + assert_eq!(fs::read_dir(dir.path()).unwrap().count(), 1); + } + + #[test] + fn malformed_existing_key_is_preserved_and_rejected() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("key"); + fs::write(&path, b"partial key").unwrap(); + assert_eq!( + load_or_create(&path).unwrap_err().kind(), + io::ErrorKind::InvalidData + ); + assert_eq!(fs::read(path).unwrap(), b"partial key"); + } + + #[test] + fn legacy_mode_is_tightened_without_changing_key() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("key"); + fs::write(&path, [42; 32]).unwrap(); + fs::set_permissions(&path, fs::Permissions::from_mode(0o644)).unwrap(); + assert_eq!(load_or_create(&path).unwrap(), vec![42; 32]); + assert_eq!( + fs::metadata(path).unwrap().permissions().mode() & 0o777, + 0o600 + ); + } + + #[test] + fn symlinks_and_non_files_are_rejected_without_replacement() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("target"); + fs::write(&target, [42; 32]).unwrap(); + let link = dir.path().join("link"); + symlink(&target, &link).unwrap(); + assert!(load_or_create(&link).is_err()); + assert!(load_or_create(dir.path()).is_err()); + assert_eq!(fs::read(target).unwrap(), vec![42; 32]); + assert!(fs::symlink_metadata(link).unwrap().file_type().is_symlink()); + } + + #[test] + fn failed_storage_never_returns_an_ephemeral_key() { + let dir = tempfile::tempdir().unwrap(); + let parent = dir.path().join("not-a-directory"); + fs::write(&parent, b"preserve").unwrap(); + assert!(load_or_create(&parent.join("key")).is_err()); + assert_eq!(fs::read(parent).unwrap(), b"preserve"); + } + + #[test] + fn unreadable_existing_key_is_not_replaced() { + use std::os::unix::process::CommandExt; + let dir = tempfile::tempdir().unwrap(); + fs::set_permissions(dir.path(), fs::Permissions::from_mode(0o755)).unwrap(); + let path = dir.path().join("key"); + fs::write(&path, [42; 32]).unwrap(); + fs::set_permissions(&path, fs::Permissions::from_mode(0o600)).unwrap(); + if unsafe { libc::geteuid() } == 0 { + // The isolated runner is root. Probe as an unprivileged child so + // DAC_OVERRIDE cannot hide the exact production failure. + let status = std::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "--ignored", + "--exact", + "session::secret_file::tests::permission_denied_child_probe", + ]) + .env("ARCHY_SESSION_KEY_PERMISSION_PROBE", &path) + .uid(65534) + .gid(65534) + .status() + .unwrap(); + assert!(status.success()); + } else { + fs::set_permissions(&path, fs::Permissions::from_mode(0o000)).unwrap(); + assert_eq!( + load_or_create(&path).unwrap_err().kind(), + io::ErrorKind::PermissionDenied + ); + fs::set_permissions(&path, fs::Permissions::from_mode(0o600)).unwrap(); + } + assert_eq!(fs::read(path).unwrap(), vec![42; 32]); + } + + #[test] + #[ignore = "Executed by unreadable_existing_key_is_not_replaced as an unprivileged child"] + fn permission_denied_child_probe() { + let path = PathBuf::from( + std::env::var_os("ARCHY_SESSION_KEY_PERMISSION_PROBE") + .expect("parent provides private fixture path"), + ); + assert_eq!( + load_or_create(&path).unwrap_err().kind(), + io::ErrorKind::PermissionDenied + ); + } + + #[test] + fn concurrent_first_boot_creators_agree_on_one_key() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("key"); + let barrier = std::sync::Arc::new(std::sync::Barrier::new(8)); + let workers: Vec<_> = (0..8) + .map(|_| { + let path = path.clone(); + let barrier = barrier.clone(); + std::thread::spawn(move || { + barrier.wait(); + load_or_create(&path).unwrap() + }) + }) + .collect(); + let keys: Vec<_> = workers + .into_iter() + .map(|worker| worker.join().unwrap()) + .collect(); + for key in &keys { + assert_eq!(key, &keys[0]); + } + assert_eq!(fs::read(path).unwrap(), keys[0]); + assert_eq!(fs::read_dir(dir.path()).unwrap().count(), 1); + } +} diff --git a/docs/session-recovery-followup.md b/docs/session-recovery-followup.md index 6d2a1e16..ed9b1d4a 100644 --- a/docs/session-recovery-followup.md +++ b/docs/session-recovery-followup.md @@ -51,3 +51,29 @@ Retain follow-ups: durable remember-secret creation must be private and atomic; unreadable/unpersistable keys must not silently become ephemeral successful configuration; logout must expire/revoke remember credentials as intended. Those broader hardening changes are not claimed by this recovery patch. + +## Persistent key follow-up (qualification pending) + +The previous loader silently generated an in-memory signing key after any read +failure and ignored write failures. It now returns storage errors, so RPC auth +fails before dispatch and app login returns an uncached service-unavailable +response rather than issuing mismatched cookies. Existing valid bytes are never +rotated as a permissions repair. Remember-token validation still reads the +current on-disk key, preserving the existing rotation/revocation behavior. + +Creation writes and syncs a private temporary inode, publishes it without +replacement, syncs the directory, and reads the winning key. Concurrent creators +therefore agree. Existing symlinks, non-files, malformed files and unreadable +files are rejected. Legacy readable files have their permissions tightened on +the opened inode. Storage failures are not cached as successful initialization. + +Regression cases cover reload, permissions, malformed keys, links/non-files, +failed storage, concurrent creation, and the actual root-owned 0600 failure +using an unprivileged child inside the isolated backend runner. This section is +implementation scope, not a claim that compilation or live acceptance passed. + +Candidate frontend browser checks passed at 390 and 1440 pixels for stale-CSRF +recovery and interface-fetch failure/retry, with real Yaya interfaces on retry. +The initial failures were injected. Evidence: +`/tmp/archy-session-recovery-browser.log`. Physical companion and deployed +backend recovery acceptance remain separate gates.