Merge pull request #486 from 21pages/try-fix-password-decrypt

fix: fallback to pk decryption when failed to decrypt with machine uid

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<KeyPair> {
+        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_::<Config>("");
+        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()

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<u8> {
     #[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<Vec<u8>> = 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<u8> = 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
 }

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<u8> {
-    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<u8>, 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<String, ()> {
@@ -184,7 +211,8 @@ pub fn symmetric_crypt(data: &[u8], encrypt: bool) -> Result<Vec<u8>, ()> {
     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<Vec<u8>, ()> {
     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<u8> = "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<u8> = 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);
+    }
 }