fix(credentials): persist the fallback key instead of deriving an unstable one
The encrypted-file fallback derived its AES key from the hostname, a hardcoded salt and `$USER`. Two problems, and the second is the one users actually hit. It was never secret. Every input is readable by anyone who can read the ciphertext beside it, so the derivation bought nothing against the threat its name implies. Calling the result "AES-256-GCM encrypted" oversold it. And it was unstable. Renaming the machine, or launching from a context where `$USER` is unset — a systemd user service, some desktop launchers — changed the key and made every stored token undecryptable. `load_credentials_file` reports a failed decrypt as "no stored credentials", so this surfaced as being silently signed out with nothing to explain it. The key is now 32 random bytes persisted beside the credentials file, mode 0600, generated on first use. That is strictly better on both counts: higher entropy, and it does not move when the machine does. It is still obfuscation at rest rather than a secret — the key sits next to what it opens — and the module docs now say so plainly instead of implying otherwise. The keyring remains the only place a token is really protected. The old derivation is kept solely to read a file written by an earlier build; anything it opens is immediately rewritten under the persisted key, so no one is signed out by the upgrade. Verified against aarch64-linux-android as well as the host.
This commit is contained in:
+296
-50
@@ -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<String, CredentialError> {
|
||||
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<String, CredentialError> {
|
||||
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<String, CredentialError> {
|
||||
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<String, CredentialError> {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user