diff --git a/crates/nim-credentials/src/unix_impl.rs b/crates/nim-credentials/src/unix_impl.rs index ddbf9b4..c2ea5be 100644 --- a/crates/nim-credentials/src/unix_impl.rs +++ b/crates/nim-credentials/src/unix_impl.rs @@ -1,6 +1,12 @@ use crate::error::NimCredentialsError; use crate::CredentialStore; use keyring::Entry; +use std::collections::HashMap; +use std::sync::RwLock; + +lazy_static::lazy_static! { + static ref CREDENTIAL_CACHE: RwLock>> = RwLock::new(HashMap::new()); +} #[derive(Clone, Copy)] pub struct UnixCredentialStore; @@ -10,14 +16,28 @@ impl CredentialStore for UnixCredentialStore { &self, target: &str, ) -> Result>, NimCredentialsError> { + { + let cache = CREDENTIAL_CACHE.read().unwrap(); + if let Some(val) = cache.get(target) { + return Ok(val.clone().map(zeroize::Zeroizing::new)); + } + } + let entry = Entry::new("vessel", target) .map_err(|e| NimCredentialsError::SystemError(e.to_string()))?; - match entry.get_password() { - Ok(password) => Ok(Some(zeroize::Zeroizing::new(password))), + let result = match entry.get_password() { + Ok(password) => Ok(Some(password)), Err(keyring::Error::NoEntry) => Ok(None), Err(e) => Err(NimCredentialsError::SystemError(e.to_string())), + }; + + if let Ok(ref val) = result { + let mut cache = CREDENTIAL_CACHE.write().unwrap(); + cache.insert(target.to_string(), val.clone()); } + + result.map(|opt| opt.map(zeroize::Zeroizing::new)) } fn set_credential_by_target( @@ -37,6 +57,9 @@ impl CredentialStore for UnixCredentialStore { .set_password(trimmed) .map_err(|e| NimCredentialsError::SystemError(e.to_string()))?; + let mut cache = CREDENTIAL_CACHE.write().unwrap(); + cache.insert(target.to_string(), Some(trimmed.to_string())); + Ok(()) } @@ -44,11 +67,16 @@ impl CredentialStore for UnixCredentialStore { let entry = Entry::new("vessel", target) .map_err(|e| NimCredentialsError::SystemError(e.to_string()))?; - match entry.delete_credential() { - Ok(_) => Ok(true), - Err(keyring::Error::NoEntry) => Ok(false), - Err(e) => Err(NimCredentialsError::SystemError(e.to_string())), - } + let existed = match entry.delete_credential() { + Ok(_) => true, + Err(keyring::Error::NoEntry) => false, + Err(e) => return Err(NimCredentialsError::SystemError(e.to_string())), + }; + + let mut cache = CREDENTIAL_CACHE.write().unwrap(); + cache.insert(target.to_string(), None); + + Ok(existed) } fn purge_orphaned_credentials(&self) -> Result<(), NimCredentialsError> { diff --git a/crates/nim-credentials/src/windows_impl.rs b/crates/nim-credentials/src/windows_impl.rs index 432139e..ac53676 100644 --- a/crates/nim-credentials/src/windows_impl.rs +++ b/crates/nim-credentials/src/windows_impl.rs @@ -2,6 +2,12 @@ use crate::error::NimCredentialsError; use crate::CredentialStore; use std::ptr; +use std::collections::HashMap; +use std::sync::RwLock; + +lazy_static::lazy_static! { + static ref CREDENTIAL_CACHE: RwLock>> = RwLock::new(HashMap::new()); +} pub const DEFAULT_USERNAME: &str = "vessel"; pub const CRED_TYPE_GENERIC: u32 = 1; @@ -66,46 +72,57 @@ impl CredentialStore for NativeCredentialStore { &self, target: &str, ) -> Result>, NimCredentialsError> { + { + let cache = CREDENTIAL_CACHE.read().unwrap(); + if let Some(val) = cache.get(target) { + return Ok(val.clone().map(zeroize::Zeroizing::new)); + } + } + let target_wide = to_wide(target); let mut cred_ptr: *mut CREDENTIALW = ptr::null_mut(); // SAFETY: We pass a valid null-terminated wide string pointer to CredReadW. // We properly manage the returned credential pointer memory via the CredentialGuard wrapper. - unsafe { + let result = unsafe { let res = CredReadW(target_wide.as_ptr(), CRED_TYPE_GENERIC, 0, &mut cred_ptr); if res == 0 { let err = std::io::Error::last_os_error(); if err.raw_os_error() == Some(1168) || err.kind() == std::io::ErrorKind::NotFound { - return Ok(None); + Ok(None) + } else { + Err(NimCredentialsError::SystemError(err.to_string())) } - return Err(NimCredentialsError::SystemError(err.to_string())); - } - - let _guard = CredentialGuard(cred_ptr); - - let cred = &*cred_ptr; - if cred.credential_blob.is_null() - || cred - .credential_blob - .align_offset(std::mem::align_of::()) - != 0 - { - return Err(NimCredentialsError::CorruptedData); - } - - let blob_slice = std::slice::from_raw_parts( - cred.credential_blob, - cred.credential_blob_size as usize, - ); - let secret = match String::from_utf8(blob_slice.to_vec()) { - Ok(s) => s, - Err(_) => { - return Err(NimCredentialsError::CorruptedData); + } else { + let _guard = CredentialGuard(cred_ptr); + + let cred = &*cred_ptr; + if cred.credential_blob.is_null() + || cred + .credential_blob + .align_offset(std::mem::align_of::()) + != 0 + { + Err(NimCredentialsError::CorruptedData) + } else { + let blob_slice = std::slice::from_raw_parts( + cred.credential_blob, + cred.credential_blob_size as usize, + ); + match String::from_utf8(blob_slice.to_vec()) { + Ok(s) => Ok(Some(zeroize::Zeroizing::new(s))), + Err(_) => Err(NimCredentialsError::CorruptedData), + } } - }; + } + }; - Ok(Some(zeroize::Zeroizing::new(secret))) + if let Ok(ref val) = result { + let mut cache = CREDENTIAL_CACHE.write().unwrap(); + cache.insert(target.to_string(), val.as_ref().map(|z| z.to_string())); } + + result } fn set_credential_by_target( @@ -152,6 +169,9 @@ impl CredentialStore for NativeCredentialStore { )); } } + + let mut cache = CREDENTIAL_CACHE.write().unwrap(); + cache.insert(target.to_string(), Some(trimmed.to_string())); Ok(()) } @@ -159,17 +179,23 @@ impl CredentialStore for NativeCredentialStore { let target_wide = to_wide(target); // SAFETY: We pass a valid null-terminated wide string pointer to CredDeleteW. // No memory is allocated or returned that requires manual cleanup. - unsafe { + let existed = unsafe { let res = CredDeleteW(target_wide.as_ptr(), CRED_TYPE_GENERIC, 0); if res == 0 { let err = std::io::Error::last_os_error(); if err.raw_os_error() == Some(1168) || err.kind() == std::io::ErrorKind::NotFound { - return Ok(false); + false + } else { + return Err(NimCredentialsError::SystemError(err.to_string())); } - return Err(NimCredentialsError::SystemError(err.to_string())); + } else { + true } - } - Ok(true) + }; + + let mut cache = CREDENTIAL_CACHE.write().unwrap(); + cache.insert(target.to_string(), None); + Ok(existed) } fn purge_orphaned_credentials(&self) -> Result<(), NimCredentialsError> { diff --git a/crates/nim-credentials/tests/credentials_tests.rs b/crates/nim-credentials/tests/credentials_tests.rs index 1a096d1..79212b5 100644 --- a/crates/nim-credentials/tests/credentials_tests.rs +++ b/crates/nim-credentials/tests/credentials_tests.rs @@ -82,3 +82,72 @@ fn test_concurrency_isolation() { Some(zeroize::Zeroizing::new(secret_2.to_string())) ); } + +#[test] +fn test_hard_concurrency_cache_reliability() { + let _lock = TEST_MUTEX.lock().unwrap(); + global_setup(); + let store = PlatformCredentialStore {}; + if let Err(e) = store.set_credential_by_target("probe", "val") { + if e.to_string().contains("No default store has been set") { + println!("Skipping test: No native keyring available"); + return; + } + } + let _ = store.clear_credential_by_target("probe"); + + let num_threads = 10; + let ops_per_thread = 10; + + let mut handles = vec![]; + for i in 0..num_threads { + let handle = std::thread::spawn(move || { + let store = PlatformCredentialStore {}; + for j in 0..ops_per_thread { + let target = format!("test-target-{}-{}", i, j); + let secret = format!("secret-{}-{}", i, j); + + // Set credential + store.set_credential_by_target(&target, &secret).expect("Failed to set credential"); + + // Read credential immediately + let retrieved = store.get_credential_by_target(&target).expect("Failed to get credential"); + assert_eq!(retrieved, Some(zeroize::Zeroizing::new(secret.clone()))); + } + }); + handles.push(handle); + } + + // Wait for all threads to finish their set/get operations + for handle in handles { + handle.join().unwrap(); + } + + // Now spawn threads to verify the final state and clean up + let mut clean_handles = vec![]; + for i in 0..num_threads { + let handle = std::thread::spawn(move || { + let store = PlatformCredentialStore {}; + for j in 0..ops_per_thread { + let target = format!("test-target-{}-{}", i, j); + let secret = format!("secret-{}-{}", i, j); + + // Read credential to verify it survived concurrent writes + let retrieved = store.get_credential_by_target(&target).expect("Failed to get credential"); + assert_eq!(retrieved, Some(zeroize::Zeroizing::new(secret))); + + // Clear credential + store.clear_credential_by_target(&target).expect("Failed to clear credential"); + + // Verify it's cleared + let cleared = store.get_credential_by_target(&target).expect("Failed to get cleared credential"); + assert!(cleared.is_none()); + } + }); + clean_handles.push(handle); + } + + for handle in clean_handles { + handle.join().unwrap(); + } +} diff --git a/crates/security-engine/src/linux.rs b/crates/security-engine/src/linux.rs index 005db92..bc50e59 100644 --- a/crates/security-engine/src/linux.rs +++ b/crates/security-engine/src/linux.rs @@ -1,5 +1,6 @@ use anyhow::{anyhow, Result}; use std::process::Command; +use std::sync::OnceLock; pub fn get_vessel_user() -> String { "vesseluser".to_string() @@ -45,12 +46,19 @@ pub fn allow_folder_access(path: &str) -> Result<()> { Ok(()) } +#[derive(Clone, Copy)] pub struct Token { pub uid: u32, pub gid: u32, } +static TOKEN_CACHE: OnceLock = OnceLock::new(); + pub fn get_restricted_token() -> Result { + if let Some(token) = TOKEN_CACHE.get() { + return Ok(*token); + } + let output = Command::new("id").args(["-u", &get_vessel_user()]).output()?; let uid_str = String::from_utf8_lossy(&output.stdout).trim().to_string(); let uid = uid_str.parse::().unwrap_or(0); @@ -65,5 +73,7 @@ pub fn get_restricted_token() -> Result { return Err(anyhow!("Failed to get UID/GID for {}", get_vessel_user())); } - Ok(Token { uid, gid }) + let token = Token { uid, gid }; + let _ = TOKEN_CACHE.set(token); + Ok(token) } diff --git a/crates/security-engine/tests/windows_tests.rs b/crates/security-engine/tests/windows_tests.rs index 74c70c7..343f9d2 100644 --- a/crates/security-engine/tests/windows_tests.rs +++ b/crates/security-engine/tests/windows_tests.rs @@ -1,4 +1,3 @@ - #[cfg(windows)] #[test] fn test_get_vessel_credentials_consistency() { diff --git a/crates/tool-synthesis/src/config.rs b/crates/tool-synthesis/src/config.rs index 511d5ad..69be4b0 100644 --- a/crates/tool-synthesis/src/config.rs +++ b/crates/tool-synthesis/src/config.rs @@ -144,4 +144,3 @@ fn is_in_path(p: &Path) -> bool { } false } - diff --git a/crates/vessel/src/sanitation.rs b/crates/vessel/src/sanitation.rs index 2f87e07..710db09 100644 --- a/crates/vessel/src/sanitation.rs +++ b/crates/vessel/src/sanitation.rs @@ -120,9 +120,9 @@ pub fn sanitize_vessel_desktops() -> anyhow::Result<()> { fn home_dir() -> Option { #[cfg(windows)] { - use windows::Win32::UI::Shell::GetUserProfileDirectoryW; - use windows::Win32::Foundation::CloseHandle; use windows::core::PWSTR; + use windows::Win32::Foundation::CloseHandle; + use windows::Win32::UI::Shell::GetUserProfileDirectoryW; let token = match security_engine::get_restricted_token() { Ok(t) => t, @@ -153,7 +153,7 @@ fn home_dir() -> Option { let path_str = String::from_utf16_lossy(&buf) .trim_end_matches('\0') .to_string(); - + // Verify it is targeting the restricted user profile if !path_str.to_ascii_lowercase().ends_with(&security_engine::get_vessel_user().to_ascii_lowercase()) { log::error!("Sanitation: Resolved profile path '{}' does not match the restricted user '{}'", path_str, security_engine::get_vessel_user());