diff --git a/src/config.rs b/src/config.rs index 1afc43ee8..496d6bae3 100644 --- a/src/config.rs +++ b/src/config.rs @@ -521,8 +521,9 @@ impl Config2 { encrypt_str_or_original(&socks.password, PASSWORD_ENC_VERSION, ENCRYPT_MAX_LEN); config.socks = Some(socks); } + let stored = Config::load_::("2"); config.unlock_pin = - encrypt_str_or_original(&config.unlock_pin, PASSWORD_ENC_VERSION, ENCRYPT_MAX_LEN); + keep_encrypted_storage_if_plaintext_unchanged(&config.unlock_pin, &stored.unlock_pin); Config::store_(&config, "2"); } @@ -541,6 +542,14 @@ impl Config2 { } } +fn keep_encrypted_storage_if_plaintext_unchanged(plain: &str, stored: &str) -> String { + let (stored_plain, encrypted, _) = decrypt_str_or_original(stored, PASSWORD_ENC_VERSION); + if encrypted && stored_plain == plain { + return stored.to_owned(); + } + encrypt_str_or_original(plain, PASSWORD_ENC_VERSION, ENCRYPT_MAX_LEN) +} + pub fn load_path( file: PathBuf, ) -> T { @@ -710,7 +719,12 @@ impl Config { config.password = encrypt_str_or_original(&config.password, PASSWORD_ENC_VERSION, ENCRYPT_MAX_LEN); } - config.enc_id = encrypt_str_or_original(&config.id, PASSWORD_ENC_VERSION, ENCRYPT_MAX_LEN); + let (stored_id, encrypted, _) = + decrypt_str_or_original(&config.enc_id, PASSWORD_ENC_VERSION); + if !encrypted || stored_id != config.id { + config.enc_id = + encrypt_str_or_original(&config.id, PASSWORD_ENC_VERSION, ENCRYPT_MAX_LEN); + } config.id = "".to_owned(); Config::store_(&config, ""); } @@ -3263,6 +3277,11 @@ mod tests { original_hard_settings: HashMap, } + struct ConfigFileRestoreGuard { + path: PathBuf, + original_content: Option>, + } + impl ConfigStateTestGuard { fn new(config: Config, hard_settings: HashMap) -> Self { let original_config = Config::get(); @@ -3283,6 +3302,29 @@ mod tests { } } + impl ConfigFileRestoreGuard { + fn new(path: PathBuf) -> Self { + let original_content = fs::read(&path).ok(); + Self { + path, + original_content, + } + } + } + + impl Drop for ConfigFileRestoreGuard { + fn drop(&mut self) { + if let Some(content) = &self.original_content { + if let Some(parent) = self.path.parent() { + fs::create_dir_all(parent).ok(); + } + fs::write(&self.path, content).ok(); + } else { + fs::remove_file(&self.path).ok(); + } + } + } + fn with_config_and_hard_settings( config: Config, hard_settings: HashMap, @@ -3472,6 +3514,67 @@ mod tests { }); } + #[test] + fn test_store_keeps_existing_enc_id_when_id_is_unchanged() { + let mut cfg = Config::default(); + cfg.id = "123456789".to_owned(); + cfg.enc_id = encrypt_str_or_original(&cfg.id, PASSWORD_ENC_VERSION, ENCRYPT_MAX_LEN); + let original_enc_id = cfg.enc_id.clone(); + + with_config_and_hard_settings(Config::default(), HashMap::new(), || { + assert!(Config::set(cfg)); + + assert_eq!(Config::load().enc_id, original_enc_id); + assert_eq!(Config::get().id, "123456789"); + }); + } + + #[test] + fn test_store_rewrites_enc_id_when_id_changes() { + let original_id = "123456789"; + let updated_id = "987654321"; + let mut cfg = Config::default(); + cfg.id = updated_id.to_owned(); + let original_enc_id = + encrypt_str_or_original(original_id, PASSWORD_ENC_VERSION, ENCRYPT_MAX_LEN); + cfg.enc_id = original_enc_id.clone(); + + with_config_and_hard_settings(Config::default(), HashMap::new(), || { + assert!(Config::set(cfg)); + + let stored = Config::load().enc_id; + let (stored_id, encrypted, _) = decrypt_str_or_original(&stored, PASSWORD_ENC_VERSION); + assert_ne!(stored, original_enc_id); + assert!(encrypted); + assert_eq!(stored_id, updated_id); + assert_eq!(Config::get().id, updated_id); + }); + } + + #[test] + fn test_config2_store_keeps_existing_unlock_pin_when_pin_is_unchanged() { + let _guard = CONFIG_STATE_TEST_LOCK.lock().unwrap(); + let _file_guard = ConfigFileRestoreGuard::new(Config::file_("2")); + let pin = "123456"; + let original_unlock_pin = + encrypt_str_or_original(pin, PASSWORD_ENC_VERSION, ENCRYPT_MAX_LEN); + let mut cfg = Config2 { + unlock_pin: original_unlock_pin.clone(), + ..Default::default() + }; + Config::store_(&cfg, "2"); + let (unlock_pin, decrypted, _) = + decrypt_str_or_original(&cfg.unlock_pin, PASSWORD_ENC_VERSION); + assert!(decrypted); + cfg.unlock_pin = unlock_pin; + cfg.nat_type = 1; + + cfg.store(); + + let stored = Config::load_::("2"); + assert_eq!(stored.unlock_pin, original_unlock_pin); + } + #[test] fn test_set_does_not_convert_plaintext_permanent_password_to_storage_format_in_memory() { let mut cfg = Config::default(); diff --git a/src/password_security.rs b/src/password_security.rs index 6471d7135..3f5f32430 100644 --- a/src/password_security.rs +++ b/src/password_security.rs @@ -1,5 +1,5 @@ use crate::config::Config; -use sodiumoxide::base64; +use sodiumoxide::{base64, crypto::secretbox}; use std::sync::{Arc, RwLock}; lazy_static::lazy_static! { @@ -92,6 +92,7 @@ pub fn hide_cm() -> bool { } const VERSION_LEN: usize = 2; +const FORMAT_V1: u8 = 1; // Check if data is already encrypted by verifying: // 1) version prefix "00" @@ -100,6 +101,7 @@ const VERSION_LEN: usize = 2; // // We intentionally avoid trying to decrypt here because key mismatch would cause // false negatives. +// The decoded payload may be either legacy ciphertext or FORMAT_V1 || nonce || ciphertext. // Reference: secretbox::seal returns ciphertext length = plaintext length + MACBYTES // https://github.com/sodiumoxide/sodiumoxide/blob/3057acb1a030ad86ed8892a223d64036ab5e8523/src/crypto/secretbox/xsalsa20poly1305.rs#L67 fn is_encrypted(v: &[u8]) -> bool { @@ -215,12 +217,17 @@ pub fn symmetric_crypt(data: &[u8], encrypt: bool) -> Result, ()> { let mut keybuf = uuid.clone(); keybuf.resize(secretbox::KEYBYTES, 0); let key = secretbox::Key(keybuf.try_into().map_err(|_| ())?); - let nonce = secretbox::Nonce([0; secretbox::NONCEBYTES]); if encrypt { - Ok(secretbox::seal(data, &nonce, &key)) + let nonce = secretbox::gen_nonce(); + let encrypted = secretbox::seal(data, &nonce, &key); + let mut output = Vec::with_capacity(1 + nonce.0.len() + encrypted.len()); + output.push(FORMAT_V1); + output.extend(nonce.0); + output.extend(encrypted); + Ok(output) } else { - let res = secretbox::open(data, &nonce, &key); + let res = open_secretbox_payload(data, &key); #[cfg(not(any(target_os = "android", target_os = "ios")))] if res.is_err() { // Fallback: try pk if uuid decryption failed (in case encryption used pk due to machine_uid failure) @@ -230,7 +237,7 @@ pub fn symmetric_crypt(data: &[u8], encrypt: bool) -> Result, ()> { let mut keybuf = pk; keybuf.resize(secretbox::KEYBYTES, 0); let pk_key = secretbox::Key(keybuf.try_into().map_err(|_| ())?); - return secretbox::open(data, &nonce, &pk_key); + return open_secretbox_payload(data, &pk_key); } } } @@ -238,6 +245,22 @@ pub fn symmetric_crypt(data: &[u8], encrypt: bool) -> Result, ()> { } } +fn open_secretbox_payload(data: &[u8], key: &secretbox::Key) -> Result, ()> { + if data.first() == Some(&FORMAT_V1) + && data.len() >= 1 + secretbox::NONCEBYTES + secretbox::MACBYTES + { + let mut nonce = [0u8; secretbox::NONCEBYTES]; + nonce.copy_from_slice(&data[1..1 + secretbox::NONCEBYTES]); + let nonce = secretbox::Nonce(nonce); + if let Ok(decrypted) = secretbox::open(&data[1 + secretbox::NONCEBYTES..], &nonce, key) { + return Ok(decrypted); + } + } + + let legacy_nonce = secretbox::Nonce([0; secretbox::NONCEBYTES]); + secretbox::open(data, &legacy_nonce, key) +} + mod test { #[test] @@ -438,6 +461,116 @@ mod test { ); } + #[test] + fn test_encryption_uses_random_nonce() { + use super::*; + + let data = b"test password 123"; + let encrypted1 = symmetric_crypt(data, true).unwrap(); + let encrypted2 = symmetric_crypt(data, true).unwrap(); + + assert_eq!(encrypted1.first(), Some(&FORMAT_V1)); + assert_eq!(encrypted2.first(), Some(&FORMAT_V1)); + assert_eq!( + encrypted1.len(), + 1 + secretbox::NONCEBYTES + data.len() + secretbox::MACBYTES + ); + assert_ne!(encrypted1, encrypted2); + assert_eq!(symmetric_crypt(&encrypted1, false).unwrap(), data); + assert_eq!(symmetric_crypt(&encrypted2, false).unwrap(), data); + } + + #[test] + fn test_decrypt_legacy_zero_nonce_payload() { + use super::*; + use std::convert::TryInto; + + let data = b"test password 123"; + let uuid = crate::get_uuid(); + let mut keybuf = uuid.clone(); + keybuf.resize(secretbox::KEYBYTES, 0); + let key = secretbox::Key(keybuf.try_into().unwrap()); + let nonce = secretbox::Nonce([0; secretbox::NONCEBYTES]); + let encrypted = secretbox::seal(data, &nonce, &key); + + assert_eq!(symmetric_crypt(&encrypted, false).unwrap(), data); + } + + #[test] + fn test_decrypt_legacy_payload_starting_with_v1_marker() { + use super::*; + use std::convert::TryInto; + + let mut keybuf = crate::get_uuid(); + keybuf.resize(secretbox::KEYBYTES, 0); + let key = secretbox::Key(keybuf.try_into().unwrap()); + let nonce = secretbox::Nonce([0; secretbox::NONCEBYTES]); + + for i in 0..=u16::MAX { + let data = format!("legacy collision payload {i:05}"); + let encrypted = secretbox::seal(data.as_bytes(), &nonce, &key); + if encrypted.first() == Some(&FORMAT_V1) { + assert_eq!(symmetric_crypt(&encrypted, false).unwrap(), data.as_bytes()); + return; + } + } + + panic!("failed to find legacy payload starting with FORMAT_V1"); + } + + #[test] + fn test_invalid_short_v1_payload_returns_error() { + use super::*; + + let encrypted = vec![FORMAT_V1]; + + assert!(symmetric_crypt(&encrypted, false).is_err()); + } + + #[test] + fn test_decrypt_legacy_string_does_not_request_store() { + use super::*; + use sodiumoxide::base64::{encode, Variant}; + use std::convert::TryInto; + + let data = "test password 123"; + let uuid = crate::get_uuid(); + let mut keybuf = uuid.clone(); + keybuf.resize(secretbox::KEYBYTES, 0); + let key = secretbox::Key(keybuf.try_into().unwrap()); + let nonce = secretbox::Nonce([0; secretbox::NONCEBYTES]); + let encrypted = secretbox::seal(data.as_bytes(), &nonce, &key); + let encrypted = "00".to_owned() + &encode(encrypted, Variant::Original); + + let (decrypted, success, store) = decrypt_str_or_original(&encrypted, "00"); + + assert_eq!(decrypted, data); + assert!(success); + assert!(!store); + } + + #[test] + fn test_decrypt_legacy_vec_does_not_request_store() { + use super::*; + use sodiumoxide::base64::{encode, Variant}; + use std::convert::TryInto; + + let data = b"test password 123"; + let uuid = crate::get_uuid(); + let mut keybuf = uuid.clone(); + keybuf.resize(secretbox::KEYBYTES, 0); + let key = secretbox::Key(keybuf.try_into().unwrap()); + let nonce = secretbox::Nonce([0; secretbox::NONCEBYTES]); + let encrypted = secretbox::seal(data, &nonce, &key); + let encrypted = ("00".to_owned() + &encode(encrypted, Variant::Original)).into_bytes(); + + let (decrypted, success, store) = decrypt_vec_or_original(&encrypted, "00"); + + assert_eq!(decrypted, data); + assert!(success); + assert!(!store); + } + // Test decryption fallback when data was encrypted with key_pair but decryption tries machine_uid first #[test] #[cfg(not(any(target_os = "android", target_os = "ios")))] @@ -471,4 +604,48 @@ mod test { ); assert_eq!(decrypted.unwrap(), data); } + + #[test] + #[cfg(not(any(target_os = "android", target_os = "ios")))] + fn test_decrypt_v1_with_pk_fallback() { + use super::*; + use sodiumoxide::base64::{encode, Variant}; + use sodiumoxide::crypto::secretbox; + use std::convert::TryInto; + + let uuid = crate::get_uuid(); + let pk = crate::config::Config::get_key_pair().1; + + if uuid == pk { + eprintln!("skip: uuid == pk, fallback branch won't be tested"); + return; + } + + let data = b"test password 123"; + let nonce = secretbox::gen_nonce(); + + let mut pk_keybuf = pk; + pk_keybuf.resize(secretbox::KEYBYTES, 0); + let pk_key = secretbox::Key(pk_keybuf.try_into().unwrap()); + let ciphertext = secretbox::seal(data, &nonce, &pk_key); + + let mut encrypted = Vec::with_capacity(1 + secretbox::NONCEBYTES + ciphertext.len()); + encrypted.push(FORMAT_V1); + encrypted.extend(nonce.0); + encrypted.extend(ciphertext); + + assert_eq!(super::symmetric_crypt(&encrypted, false).unwrap(), data); + + let encrypted_str = "00".to_owned() + &encode(&encrypted, Variant::Original); + let (decrypted, success, store) = decrypt_str_or_original(&encrypted_str, "00"); + assert_eq!(decrypted.as_bytes(), data); + assert!(success); + assert!(!store); + + let encrypted_vec = encrypted_str.into_bytes(); + let (decrypted, success, store) = decrypt_vec_or_original(&encrypted_vec, "00"); + assert_eq!(decrypted, data); + assert!(success); + assert!(!store); + } }