From 5d2acc73402cd78db89096d8bc026c7eb3988c74 Mon Sep 17 00:00:00 2001 From: 21pages Date: Tue, 3 Feb 2026 14:55:38 +0800 Subject: [PATCH] fix: improve machine_uid handling and add pk decryption fallback - Cache machine_uid in get_uuid with retry logic for macOS - Fallback to pk decryption when machine_uid decryption fails - Add is_encrypted check to prevent duplicate encryption - Add get_existing_key_pair to get key pair without generating new one Signed-off-by: 21pages --- src/config.rs | 23 +++++ src/lib.rs | 59 ++++++++++++- src/password_security.rs | 182 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 256 insertions(+), 8 deletions(-) diff --git a/src/config.rs b/src/config.rs index 20230110e..41db73811 100644 --- a/src/config.rs +++ b/src/config.rs @@ -588,6 +588,7 @@ impl Config { store = true; } if !id_valid { + log::warn!("ID is invalid, generating new one"); for _ in 0..3 { if let Some(id) = Config::gen_id() { config.id = id; @@ -918,6 +919,7 @@ impl Config { id = (id << 8) | (*x as u32); } id &= 0x1FFFFFFF; + log::info!("Generated id {}", id); Some(id.to_string()) } else { None @@ -996,6 +998,27 @@ impl Config { KEY_PAIR.lock().unwrap().clone().map(|k| k.1) } + /// Get existing key pair without generating a new one. + /// Returns None if no key pair exists in cache or config file. + pub fn get_existing_key_pair() -> Option { + let mut lock = KEY_PAIR.lock().unwrap(); + if let Some(p) = lock.as_ref() { + return Some(p.clone()); + } + + // IMPORTANT: this path is called while holding KEY_PAIR lock. + // Config::load_ must remain a raw conf load/deserialize path and must never + // call decrypt_* / symmetric_crypt (directly or indirectly), otherwise this + // can re-enter key loading and deadlock. + let config = Config::load_::(""); + if !config.key_pair.0.is_empty() { + *lock = Some(config.key_pair.clone()); + Some(config.key_pair) + } else { + None + } + } + pub fn no_register_device() -> bool { BUILTIN_SETTINGS .read() diff --git a/src/lib.rs b/src/lib.rs index d9713b962..2b3564219 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -312,10 +312,65 @@ pub fn get_exe_time() -> SystemTime { }) } +/// Known cases where machine_uid::get() may fail: +/// - Windows shutdown: "The media is write protected. (os error 19)" +/// - macOS (hard to reproduce, reproduced at login screen): "No matching IOPlatformUUID in `ioreg -rd1 -c IOPlatformExpertDevice` command" pub fn get_uuid() -> Vec { #[cfg(not(any(target_os = "android", target_os = "ios")))] - if let Ok(id) = machine_uid::get() { - return id.into(); + { + use std::sync::atomic::{AtomicUsize, Ordering}; + + static CACHED_MACHINE_UID: std::sync::OnceLock> = std::sync::OnceLock::new(); + // Throttle only applies to the fallback machine_uid::get() log below, not the Once::call_once retry logs. + static LOG_COUNT: AtomicUsize = AtomicUsize::new(0); + + // Only macOS needs retry logic here because: + // - macOS: in testing, only one failure occurred when reading at 50ms intervals, so retry helps + // - Windows: failures during shutdown are persistent, retrying is pointless + #[cfg(target_os = "macos")] + { + static INIT: std::sync::Once = std::sync::Once::new(); + INIT.call_once(|| { + // Keep in sync with upstream handling: + // https://github.com/rustdesk/rustdesk/blob/85db6779828349b23ca3eba91cc7cd36c5337797/src/common.rs#L822 + let username = whoami::username().trim_end_matches('\0').to_owned(); + let max_retries = if username == "root" { 16 } else { 8 }; + for i in 0..max_retries { + match machine_uid::get() { + Ok(id) => { + let _ = CACHED_MACHINE_UID.set(id.into()); + return; + } + Err(e) => { + log::error!("Failed to get machine uid in macOS retry #{i}: {e}"); + } + } + std::thread::sleep(std::time::Duration::from_millis(50)); + } + }); + } + + if let Some(uid) = CACHED_MACHINE_UID.get() { + return uid.clone(); + } + + match machine_uid::get() { + Ok(id) => { + let uid: Vec = id.into(); + let _ = CACHED_MACHINE_UID.set(uid.clone()); + return uid; + } + Err(e) => { + if LOG_COUNT + .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |count| { + (count < 30).then_some(count + 1) + }) + .is_ok() + { + log::error!("Failed to get machine uid: {e}"); + } + } + } } Config::get_key_pair().1 } diff --git a/src/password_security.rs b/src/password_security.rs index 739c85f0b..3d532b772 100644 --- a/src/password_security.rs +++ b/src/password_security.rs @@ -93,8 +93,27 @@ pub fn hide_cm() -> bool { const VERSION_LEN: usize = 2; +// Check if data is already encrypted by verifying: +// 1) version prefix "00" +// 2) valid base64 payload +// 3) decoded payload length >= secretbox::MACBYTES +// +// We intentionally avoid trying to decrypt here because key mismatch would cause +// false negatives. +// 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 { + if v.len() <= VERSION_LEN || !v.starts_with(b"00") { + return false; + } + match base64::decode(&v[VERSION_LEN..], base64::Variant::Original) { + Ok(decoded) => decoded.len() >= sodiumoxide::crypto::secretbox::MACBYTES, + Err(_) => false, + } +} + pub fn encrypt_str_or_original(s: &str, version: &str, max_len: usize) -> String { - if decrypt_str_or_original(s, version).1 { + if is_encrypted(s.as_bytes()) { log::error!("Duplicate encryption!"); return s.to_owned(); } @@ -127,11 +146,17 @@ pub fn decrypt_str_or_original(s: &str, current_version: &str) -> (String, bool, } } - (s.to_owned(), false, !s.is_empty()) + // For values that already look encrypted (version prefix + base64), avoid + // repeated store on each load when decryption fails. + ( + s.to_owned(), + false, + !s.is_empty() && !is_encrypted(s.as_bytes()), + ) } pub fn encrypt_vec_or_original(v: &[u8], version: &str, max_len: usize) -> Vec { - if decrypt_vec_or_original(v, version).1 { + if is_encrypted(v) { log::error!("Duplicate encryption!"); return v.to_owned(); } @@ -161,7 +186,9 @@ pub fn decrypt_vec_or_original(v: &[u8], current_version: &str) -> (Vec, boo } } - (v.to_owned(), false, !v.is_empty()) + // For values that already look encrypted (version prefix + base64), avoid + // repeated store on each load when decryption fails. + (v.to_owned(), false, !v.is_empty() && !is_encrypted(v)) } fn encrypt(v: &[u8]) -> Result { @@ -184,7 +211,8 @@ pub fn symmetric_crypt(data: &[u8], encrypt: bool) -> Result, ()> { use sodiumoxide::crypto::secretbox; use std::convert::TryInto; - let mut keybuf = crate::get_uuid(); + let uuid = crate::get_uuid(); + 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]); @@ -192,7 +220,21 @@ pub fn symmetric_crypt(data: &[u8], encrypt: bool) -> Result, ()> { if encrypt { Ok(secretbox::seal(data, &nonce, &key)) } else { - secretbox::open(data, &nonce, &key) + let res = secretbox::open(data, &nonce, &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) + if let Some(key_pair) = Config::get_existing_key_pair() { + let pk = key_pair.1; + if pk != uuid { + 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); + } + } + } + res } } @@ -268,6 +310,33 @@ mod test { let data: Vec = "1ΓΌ1111".as_bytes().to_vec(); assert_eq!(decrypt_vec_or_original(&data, version).0, data); + // Base64-shaped "00" prefixed values shorter than MACBYTES are treated + // as original/plain values and should be stored. + let data = "00YWJjZA=="; + let (decrypted, succ, store) = decrypt_str_or_original(data, version); + assert_eq!(decrypted, data); + assert!(!succ); + assert!(store); + let data = b"00YWJjZA==".to_vec(); + let (decrypted, succ, store) = decrypt_vec_or_original(&data, version); + assert_eq!(decrypted, data); + assert!(!succ); + assert!(store); + + // When decoded length reaches MACBYTES, it is treated as encrypted-like + // and should not trigger repeated store. + let exact_mac = vec![0u8; sodiumoxide::crypto::secretbox::MACBYTES]; + let exact_mac_b64 = + sodiumoxide::base64::encode(&exact_mac, sodiumoxide::base64::Variant::Original); + let data = format!("00{exact_mac_b64}"); + let (_, succ, store) = decrypt_str_or_original(&data, version); + assert!(!succ); + assert!(!store); + let data = data.into_bytes(); + let (_, succ, store) = decrypt_vec_or_original(&data, version); + assert!(!succ); + assert!(!store); + println!("test speed"); let test_speed = |len: usize, name: &str| { let mut data: Vec = vec![]; @@ -301,4 +370,105 @@ mod test { test_speed(10 * 1024 * 1024, "10M"); test_speed(100 * 1024 * 1024, "100M"); } + + #[test] + fn test_is_encrypted() { + use super::*; + use sodiumoxide::base64::{encode, Variant}; + use sodiumoxide::crypto::secretbox; + + // Empty data should not be considered encrypted + assert!(!is_encrypted(b"")); + assert!(!is_encrypted(b"0")); + assert!(!is_encrypted(b"00")); + + // Data without "00" prefix should not be considered encrypted + assert!(!is_encrypted(b"01abcd")); + assert!(!is_encrypted(b"99abcd")); + assert!(!is_encrypted(b"hello world")); + + // Data with "00" prefix but invalid base64 should not be considered encrypted + assert!(!is_encrypted(b"00!!!invalid base64!!!")); + assert!(!is_encrypted(b"00@#$%")); + + // Data with "00" prefix and valid base64 but shorter than MACBYTES is not encrypted + assert!(!is_encrypted(b"00YWJjZA==")); // "abcd" in base64 + assert!(!is_encrypted(b"00SGVsbG8gV29ybGQ=")); // "Hello World" in base64 + + // Data with "00" prefix and valid base64 with decoded len == MACBYTES is considered encrypted + let exact_mac = vec![0u8; secretbox::MACBYTES]; + let exact_mac_b64 = encode(&exact_mac, Variant::Original); + let exact_mac_candidate = format!("00{exact_mac_b64}"); + assert!(is_encrypted(exact_mac_candidate.as_bytes())); + + // Real encrypted data should be detected + let version = "00"; + let max_len = 128; + let encrypted_str = encrypt_str_or_original("1", version, max_len); + assert!(is_encrypted(encrypted_str.as_bytes())); + let encrypted_vec = encrypt_vec_or_original(b"1", version, max_len); + assert!(is_encrypted(&encrypted_vec)); + + // Original unencrypted data should not be detected as encrypted + assert!(!is_encrypted(b"1")); + assert!(!is_encrypted("1".as_bytes())); + } + + #[test] + fn test_encrypted_payload_min_len_macbytes() { + use super::*; + use sodiumoxide::base64::{decode, Variant}; + use sodiumoxide::crypto::secretbox; + + let version = "00"; + let max_len = 128; + + let encrypted_str = encrypt_str_or_original("1", version, max_len); + let decoded = decode(&encrypted_str.as_bytes()[VERSION_LEN..], Variant::Original).unwrap(); + assert!( + decoded.len() >= secretbox::MACBYTES, + "decoded encrypted payload must be at least MACBYTES" + ); + + let encrypted_vec = encrypt_vec_or_original(b"1", version, max_len); + let decoded = decode(&encrypted_vec[VERSION_LEN..], Variant::Original).unwrap(); + assert!( + decoded.len() >= secretbox::MACBYTES, + "decoded encrypted payload must be at least MACBYTES" + ); + } + + // 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")))] + fn test_decrypt_with_pk_fallback() { + use sodiumoxide::crypto::secretbox; + use std::convert::TryInto; + + let uuid = crate::get_uuid(); + let pk = crate::config::Config::get_key_pair().1; + + // Ensure uuid != pk, otherwise fallback branch won't be tested + if uuid == pk { + eprintln!("skip: uuid == pk, fallback branch won't be tested"); + return; + } + + let data = b"test password 123"; + let nonce = secretbox::Nonce([0; secretbox::NONCEBYTES]); + + // Encrypt with pk (simulating machine_uid failure during encryption) + let mut pk_keybuf = pk; + pk_keybuf.resize(secretbox::KEYBYTES, 0); + let pk_key = secretbox::Key(pk_keybuf.try_into().unwrap()); + let encrypted = secretbox::seal(data, &nonce, &pk_key); + + // Decrypt using symmetric_crypt (should fallback to pk since uuid differs) + let decrypted = super::symmetric_crypt(&encrypted, false); + assert!( + decrypted.is_ok(), + "Decryption with pk fallback should succeed" + ); + assert_eq!(decrypted.unwrap(), data); + } }