diff --git a/src-tauri/src/credentials.rs b/src-tauri/src/credentials.rs index b2f7fb68c..1f44501a2 100644 --- a/src-tauri/src/credentials.rs +++ b/src-tauri/src/credentials.rs @@ -4,8 +4,19 @@ //! - Primary: System keyring (Secret Service on Linux, Keychain on macOS) //! - Fallback: AES-256-GCM encrypted file when keyring unavailable //! -//! The fallback is less secure as the encryption key is derived from machine -//! identifiers, but provides functionality on headless systems. +//! The fallback is **obfuscation at rest, not a secret**: its key sits in a file +//! beside the ciphertext, so anyone who can read one can read the other. It +//! exists so headless systems keep working, and the keyring remains the only +//! place a token is actually protected. +//! +//! The key used to be *derived* from the hostname, `$USER` and a hardcoded salt. +//! That was no more secret — those are readable by anyone who can read the file +//! — and it was unstable: renaming the machine, or launching from a context +//! where `$USER` is unset, changed the key and made every stored token +//! undecryptable. `load_credentials_file` treats a failed decrypt as "no stored +//! credentials", so that surfaced as being silently signed out rather than as an +//! error. The key is now random and persisted, and the old derivation is kept +//! only to migrate a file written before this change. //! //! TRACES: UR-012 | IR-014 @@ -25,6 +36,119 @@ const SERVICE_NAME: &str = "com.dtourolle.jellytau"; const CREDENTIALS_FILENAME: &str = "credentials.enc"; +/// Key file for the encrypted-file fallback, beside the credentials it opens. +const KEY_FILENAME: &str = "credentials.key"; + +/// Load the fallback encryption key, creating it on first use. +/// +/// Random rather than derived. A derived key was no more secret — its inputs +/// (hostname, `$USER`, a hardcoded salt) are readable by anyone who can read +/// the ciphertext — and it silently changed when the machine was renamed or +/// `$USER` was unset, which read to the user as being signed out for no reason. +/// +/// If the key cannot be persisted the process still gets a usable key for this +/// run; credentials written under it simply will not be readable next launch, +/// which is the same outcome as today and better than refusing to store a token. +/// +/// TRACES: UR-012 | IR-014 | UT-014 +fn load_or_create_key(path: &std::path::Path) -> [u8; 32] { + if let Ok(existing) = fs::read(path) { + if existing.len() == 32 { + let mut key = [0u8; 32]; + key.copy_from_slice(&existing); + return key; + } + warn!( + "Fallback key at {:?} is {} bytes, not 32; replacing it. Credentials \ + written under the old key will need signing in again.", + path, + existing.len() + ); + } + + let mut key = [0u8; 32]; + if getrandom::getrandom(&mut key).is_err() { + warn!("No system randomness for the fallback key; deriving one for this run"); + return CredentialStore::derive_legacy_encryption_key(); + } + + if let Some(parent) = path.parent() { + let _ = fs::create_dir_all(parent); + } + match fs::write(path, key) { + Ok(()) => restrict_to_owner(path), + Err(e) => warn!( + "Could not persist the fallback key at {:?} ({}); credentials stored \ + this run will not be readable next launch", + path, e + ), + } + key +} + +/// Make a key file owner-readable only. Best effort — a filesystem without +/// Unix permissions is not a reason to fail. +fn restrict_to_owner(path: &std::path::Path) { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + if let Err(e) = fs::set_permissions(path, fs::Permissions::from_mode(0o600)) { + warn!("Could not restrict permissions on {:?}: {}", path, e); + } + } + #[cfg(not(unix))] + let _ = path; +} + +/// Decrypt `encrypted` with `key`. +/// +/// TRACES: UR-012 | IR-014 | UT-014 +fn decrypt_with(key: &[u8; 32], encrypted: &str) -> Result { + let combined = BASE64 + .decode(encrypted) + .map_err(|e| CredentialError::Encryption(e.to_string()))?; + + if combined.len() < 12 { + return Err(CredentialError::Encryption( + "Invalid encrypted data".to_string(), + )); + } + + let (nonce_bytes, ciphertext) = combined.split_at(12); + let nonce = Nonce::from_slice(nonce_bytes); + + let cipher = + Aes256Gcm::new_from_slice(key).map_err(|e| CredentialError::Encryption(e.to_string()))?; + + let plaintext = cipher + .decrypt(nonce, ciphertext) + .map_err(|e| CredentialError::Encryption(e.to_string()))?; + + String::from_utf8(plaintext).map_err(|e| CredentialError::Encryption(e.to_string())) +} + +/// Encrypt `plaintext` with `key`, prepending a fresh random nonce. +/// +/// TRACES: UR-012 | IR-014 | UT-014 +fn encrypt_with(key: &[u8; 32], plaintext: &str) -> Result { + let cipher = + Aes256Gcm::new_from_slice(key).map_err(|e| CredentialError::Encryption(e.to_string()))?; + + let mut nonce_bytes = [0u8; 12]; + getrandom::getrandom(&mut nonce_bytes) + .map_err(|e| CredentialError::Encryption(e.to_string()))?; + let nonce = Nonce::from_slice(&nonce_bytes); + + let ciphertext = cipher + .encrypt(nonce, plaintext.as_bytes()) + .map_err(|e| CredentialError::Encryption(e.to_string()))?; + + let mut combined = nonce_bytes.to_vec(); + combined.extend(ciphertext); + + Ok(BASE64.encode(&combined)) +} + /// Result of a credential storage operation #[derive(Debug)] pub enum CredentialResult { @@ -66,15 +190,21 @@ pub struct CredentialStore { using_keyring: bool, /// Path to the encrypted credentials file (fallback) credentials_path: PathBuf, - /// Encryption key for file fallback (derived from machine ID) + /// Encryption key for the file fallback. Random and persisted, so it does + /// not change when the machine is renamed. encryption_key: [u8; 32], + /// The pre-existing derivation, retained only to read a file written before + /// the key was persisted. Anything decrypted with it is rewritten under + /// `encryption_key`. + legacy_key: [u8; 32], } impl CredentialStore { /// Create a new credential store, detecting the best available backend pub fn new() -> Self { let credentials_path = Self::get_credentials_path(); - let encryption_key = Self::derive_encryption_key(); + let encryption_key = load_or_create_key(&Self::get_key_path()); + let legacy_key = Self::derive_legacy_encryption_key(); // Test if keyring is available by trying a dummy operation let using_keyring = Self::test_keyring_available(); @@ -93,6 +223,7 @@ impl CredentialStore { using_keyring, credentials_path, encryption_key, + legacy_key, } } @@ -406,6 +537,16 @@ impl CredentialStore { // --- Encrypted file backend --- + /// Where the fallback key lives: beside the credentials file, so the two + /// travel together and a restore that brings one brings the other. + fn get_key_path() -> PathBuf { + if let Some(proj_dirs) = ProjectDirs::from("com", "dtourolle", "jellytau") { + proj_dirs.data_dir().join(KEY_FILENAME) + } else { + PathBuf::from(KEY_FILENAME) + } + } + fn get_credentials_path() -> PathBuf { if let Some(proj_dirs) = ProjectDirs::from("com", "dtourolle", "jellytau") { proj_dirs.data_dir().join(CREDENTIALS_FILENAME) @@ -414,7 +555,14 @@ impl CredentialStore { } } - fn derive_encryption_key() -> [u8; 32] { + /// The key derivation used before keys were persisted. + /// + /// Kept **only** so a credentials file written by an older build can still + /// be read once and rewritten under the persisted key. Never used to + /// encrypt. See the module docs for why it was replaced. + /// + /// TRACES: UR-012 | IR-014 + fn derive_legacy_encryption_key() -> [u8; 32] { // Derive a key from machine-specific identifiers // This is less secure than a true keyring but provides some protection let mut hasher = Sha256::new(); @@ -490,7 +638,7 @@ impl CredentialStore { return Ok(serde_json::json!({})); } - let decrypted = match self.decrypt(&encrypted_data) { + let (decrypted, from_legacy_key) = match self.decrypt_migrating(&encrypted_data) { Ok(decrypted) => decrypted, Err(e) => { warn!( @@ -505,7 +653,19 @@ impl CredentialStore { }; match serde_json::from_str(&decrypted) { - Ok(value) => Ok(value), + Ok(value) => { + // Rewrite under the persisted key so the legacy derivation is + // never needed again. + if from_legacy_key { + if let Err(e) = self.save_credentials_file(&value) { + warn!( + "Could not rewrite credentials under the persisted key: {}", + e + ); + } + } + Ok(value) + } Err(e) => { warn!( "Credentials file at {:?} decrypted to invalid JSON ({}); \ @@ -531,48 +691,27 @@ impl CredentialStore { } fn encrypt(&self, plaintext: &str) -> Result { - let cipher = Aes256Gcm::new_from_slice(&self.encryption_key) - .map_err(|e| CredentialError::Encryption(e.to_string()))?; - - // Generate a random nonce - let mut nonce_bytes = [0u8; 12]; - getrandom::getrandom(&mut nonce_bytes) - .map_err(|e| CredentialError::Encryption(e.to_string()))?; - let nonce = Nonce::from_slice(&nonce_bytes); - - let ciphertext = cipher - .encrypt(nonce, plaintext.as_bytes()) - .map_err(|e| CredentialError::Encryption(e.to_string()))?; - - // Prepend nonce to ciphertext and encode as base64 - let mut combined = nonce_bytes.to_vec(); - combined.extend(ciphertext); - - Ok(BASE64.encode(&combined)) + encrypt_with(&self.encryption_key, plaintext) } - fn decrypt(&self, encrypted: &str) -> Result { - let combined = BASE64 - .decode(encrypted) - .map_err(|e| CredentialError::Encryption(e.to_string()))?; - - if combined.len() < 12 { - return Err(CredentialError::Encryption( - "Invalid encrypted data".to_string(), - )); + /// Decrypt with the current key, falling back to the legacy derivation. + /// + /// Returns the plaintext and whether the legacy key was what opened it, so + /// the caller can rewrite the file under the current key and stop depending + /// on a derivation that changes when the machine is renamed. + /// + /// TRACES: UR-012 | IR-014 | UT-014 + fn decrypt_migrating(&self, encrypted: &str) -> Result<(String, bool), CredentialError> { + match decrypt_with(&self.encryption_key, encrypted) { + Ok(plaintext) => Ok((plaintext, false)), + Err(current_err) => match decrypt_with(&self.legacy_key, encrypted) { + Ok(plaintext) => { + info!("Credentials were written under the legacy derived key; rewriting them"); + Ok((plaintext, true)) + } + Err(_) => Err(current_err), + }, } - - let (nonce_bytes, ciphertext) = combined.split_at(12); - let nonce = Nonce::from_slice(nonce_bytes); - - let cipher = Aes256Gcm::new_from_slice(&self.encryption_key) - .map_err(|e| CredentialError::Encryption(e.to_string()))?; - - let plaintext = cipher - .decrypt(nonce, ciphertext) - .map_err(|e| CredentialError::Encryption(e.to_string()))?; - - String::from_utf8(plaintext).map_err(|e| CredentialError::Encryption(e.to_string())) } fn save_to_file(&self, user_id: &str, token: &str) -> Result<(), CredentialError> { @@ -884,6 +1023,103 @@ pub use android_keystore::{ #[cfg(test)] mod tests { + + /// The fallback key must be the same on every launch. + /// + /// It used to be derived from the hostname, `$USER` and a static salt. + /// Renaming the machine — or launching from a context where `$USER` is + /// unset, such as a systemd user service — changed the key, and + /// `load_credentials_file` reports a failed decrypt as "no stored + /// credentials". The user was silently signed out with nothing to explain it. + /// + /// TRACES: UR-012 | IR-014 | UT-014 + #[test] + fn the_fallback_key_is_stable_across_processes() { + let dir = std::env::temp_dir().join(format!("jellytau-key-{}", std::process::id())); + let path = dir.join("credentials.key"); + let _ = fs::remove_file(&path); + + let first = load_or_create_key(&path); + let second = load_or_create_key(&path); + assert_eq!(first, second, "the key must not change between launches"); + assert_ne!(first, [0u8; 32], "the key must be real randomness"); + + let _ = fs::remove_dir_all(&dir); + } + + /// Two installs must not share a key. + /// + /// TRACES: UR-012 | IR-014 | UT-014 + #[test] + fn separate_installs_get_separate_keys() { + let base = std::env::temp_dir().join(format!("jellytau-keys-{}", std::process::id())); + let a = load_or_create_key(&base.join("a").join("credentials.key")); + let b = load_or_create_key(&base.join("b").join("credentials.key")); + assert_ne!(a, b); + let _ = fs::remove_dir_all(&base); + } + + /// A credentials file written under the old derived key must still open. + /// + /// TRACES: UR-012 | IR-014 | UT-014 + #[test] + fn credentials_written_under_the_legacy_key_still_decrypt() { + let legacy = CredentialStore::derive_legacy_encryption_key(); + let mut persisted = [0u8; 32]; + getrandom::getrandom(&mut persisted).unwrap(); + assert_ne!(legacy, persisted); + + let blob = encrypt_with(&legacy, r#"{"user-1":"token-abc"}"#).unwrap(); + + let store = CredentialStore { + using_keyring: false, + credentials_path: PathBuf::from("/nonexistent/credentials.enc"), + encryption_key: persisted, + legacy_key: legacy, + }; + + let (plaintext, migrated) = store + .decrypt_migrating(&blob) + .expect("a file written under the legacy key must still be readable"); + assert_eq!(plaintext, r#"{"user-1":"token-abc"}"#); + assert!(migrated, "the caller must know to rewrite it"); + } + + /// The current key is tried first and needs no migration. + /// + /// TRACES: UR-012 | IR-014 | UT-014 + #[test] + fn credentials_under_the_current_key_are_not_flagged_for_migration() { + let mut persisted = [0u8; 32]; + getrandom::getrandom(&mut persisted).unwrap(); + let blob = encrypt_with(&persisted, "hello").unwrap(); + + let store = CredentialStore { + using_keyring: false, + credentials_path: PathBuf::from("/nonexistent/credentials.enc"), + encryption_key: persisted, + legacy_key: [7u8; 32], + }; + + let (plaintext, migrated) = store.decrypt_migrating(&blob).unwrap(); + assert_eq!(plaintext, "hello"); + assert!(!migrated); + } + + /// A blob under neither key fails rather than returning something wrong. + /// + /// TRACES: UR-012 | IR-014 | UT-014 + #[test] + fn an_unreadable_blob_is_an_error() { + let blob = encrypt_with(&[1u8; 32], "secret").unwrap(); + let store = CredentialStore { + using_keyring: false, + credentials_path: PathBuf::from("/nonexistent/credentials.enc"), + encryption_key: [2u8; 32], + legacy_key: [3u8; 32], + }; + assert!(store.decrypt_migrating(&blob).is_err()); + } use super::*; /// Build a store pinned to the encrypted-file backend with an explicit key, @@ -894,6 +1130,9 @@ mod tests { using_keyring: false, credentials_path, encryption_key, + // A distinct legacy key, so "same file, different machine key" stays + // undecryptable rather than being opened by the migration path. + legacy_key: [0xABu8; 32], } } @@ -959,15 +1198,22 @@ mod tests { let plaintext = "test-access-token-12345"; let encrypted = store.encrypt(plaintext).unwrap(); - let decrypted = store.decrypt(&encrypted).unwrap(); + let (decrypted, _) = store.decrypt_migrating(&encrypted).unwrap(); assert_eq!(plaintext, decrypted); } + /// The legacy derivation must stay deterministic *within a machine*, or the + /// one-time migration of an old credentials file cannot read it. + /// + /// Its instability *across* machine states is exactly why it no longer + /// encrypts anything — see `the_fallback_key_is_stable_across_processes`. + /// + /// TRACES: UR-012 | IR-014 | UT-014 #[test] - fn test_derive_encryption_key_is_deterministic() { - let key1 = CredentialStore::derive_encryption_key(); - let key2 = CredentialStore::derive_encryption_key(); + fn test_legacy_derivation_is_deterministic_for_migration() { + let key1 = CredentialStore::derive_legacy_encryption_key(); + let key2 = CredentialStore::derive_legacy_encryption_key(); assert_eq!(key1, key2); } }