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

[21pages]

• Cache machine uid after successful retrieval
• Retry when getting machine uid fails
• Fallback to pk decryption when failed to decrypt with machine uid

pk decryption

I found that decrypting with my password failed, but decrypting with the PK worked. So it’s possible that the machine UID failed to be retrieved during a previous encryption, and the PK was used for encryption instead.

The PK is only attempted during the second decryption. In the first attempt, the machine UID is given priority for decryption. If the PK were also used in the first attempt, then the second attempt to retrieve the machine UID would most likely fail as well.

Reproducing the machine UID read failure

On Windows, reading the machine UID may fail when the system is close to shutdown

Retry Test

diff --git a/src/lib.rs b/src/lib.rs
index c24c16d..5180327 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -312,6 +312,29 @@ pub fn get_exe_time() -> SystemTime {
     })
 }
 
+#[cfg(test)]
+#[cfg(not(any(target_os = "android", target_os = "ios")))]
+pub mod machine_uid_mock {
+    use std::sync::atomic::{AtomicUsize, Ordering};
+
+    /// Which call should succeed (0 = first call succeeds, usize::MAX = always fail)
+    pub static SUCCESS_AT_CALL: AtomicUsize = AtomicUsize::new(0);
+    static CALL_COUNT: AtomicUsize = AtomicUsize::new(0);
+
+    pub fn reset() {
+        CALL_COUNT.store(0, Ordering::SeqCst);
+    }
+
+    pub fn get() -> Result<String, &'static str> {
+        let count = CALL_COUNT.fetch_add(1, Ordering::SeqCst);
+        if count >= SUCCESS_AT_CALL.load(Ordering::SeqCst) {
+            Ok(machine_uid::get().unwrap())
+        } else {
+            Err("mock failure")
+        }
+    }
+}
+
 pub fn get_uuid() -> Vec<u8> {
     #[cfg(not(any(target_os = "android", target_os = "ios")))]
     {
@@ -335,7 +358,11 @@ pub fn get_uuid() -> Vec<u8> {
             6
         };
         for i in 0..retries_per_round {
-            match machine_uid::get() {
+            #[cfg(test)]
+            let result = machine_uid_mock::get();
+            #[cfg(not(test))]
+            let result = machine_uid::get();
+            match result {
                 Ok(id) => {
                     let uid: Vec<u8> = id.into();
                     let _ = CACHED_MACHINE_UID.set(uid.clone());
@@ -613,4 +640,113 @@ mod test {
         assert_eq!(get_version_number("1.1.11-1"), 1001111);
         assert_eq!(get_version_number("1.2.3"), 1002030);
     }
+
+    /// Test machine_uid retry logic with FAILED_ROUNDS accumulation.
+    ///
+    /// Test scenarios (modify SUCCESS_AT_CALL, loops, threshold):
+    ///
+    /// Scenario 1: SUCCESS_AT_CALL = usize::MAX, loops = 12, threshold = 9
+    ///   - Always fail, test FAILED_ROUNDS accumulation
+    ///   - round 0-8: returns pk, ~150ms
+    ///   - round 9+: returns pk, <50ms
+    ///
+    /// Scenario 2: SUCCESS_AT_CALL = 0, loops = 3, threshold = 0
+    ///   - First call succeeds, subsequent calls return from cache
+    ///   - round 0-2: returns machine_uid, <50ms
+    ///
+    /// Scenario 3: SUCCESS_AT_CALL = 3, loops = 1, threshold = 0
+    ///   - 4th call succeeds (first 3 fail)
+    ///   - round 0: returns machine_uid, ~90ms (3 × 30ms)
+    ///
+    /// Scenario 4: SUCCESS_AT_CALL = 5, loops = 1, threshold = 0
+    ///   - 6th call succeeds (first 5 fail)
+    ///   - round 0: returns machine_uid, ~150ms (5 × 30ms)
+    ///
+    /// Scenario 5: SUCCESS_AT_CALL = 6, loops = 2, threshold = 1
+    ///   - 1st round: all 6 calls fail, returns pk
+    ///   - 2nd round: 1st call succeeds (cumulative 7th call), returns machine_uid
+    ///   - round 0: returns pk, ~150ms
+    ///   - round 1: returns machine_uid, <50ms
+    ///
+    /// Scenario 6: SUCCESS_AT_CALL = 8, loops = 2, threshold = 1
+    ///   - 1st round: all 6 calls fail, returns pk
+    ///   - 2nd round: first 2 fail, 3rd succeeds (cumulative 9th call), returns machine_uid
+    ///   - round 0: returns pk, ~150ms
+    ///   - round 1: returns machine_uid, ~60ms (2 × 30ms)
+    #[test]
+    #[cfg(not(any(target_os = "android", target_os = "ios")))]
+    fn test_machine_uid_retry() {
+        use std::sync::atomic::Ordering;
+        use std::time::Instant;
+
+        // (success_at_call, loops, threshold, expect_machine_uid_at, min_ms)
+        // - success_at_call: which call succeeds (usize::MAX = always fail)
+        // - loops: number of get_uuid() calls
+        // - threshold: round < threshold expects long time (>= min_ms)
+        // - expect_machine_uid_at: from which round expects machine_uid (usize::MAX = always pk)
+        // - min_ms: minimum expected time for rounds < threshold
+        //
+        // Scenario 0: Always fail, test FAILED_ROUNDS accumulation (MAX_FAILED_ROUNDS=9)
+        // Scenario 1: First call succeeds, subsequent from cache
+        // Scenario 2: 4th call succeeds (3 failures, ~90ms)
+        // Scenario 3: 6th call succeeds (5 failures, ~150ms)
+        // Scenario 4: 1st round fails (6 calls), 2nd round 1st call succeeds
+        // Scenario 5: 1st round fails, 2nd round 3rd call succeeds (~60ms)
+        let scenarios: [(usize, usize, usize, usize, u128); 6] = [
+            (usize::MAX, 12, 9, usize::MAX, 120), // 0: always fail, 5 sleeps
+            (0, 3, 0, 0, 0),                       // 1: first call succeeds
+            (3, 1, 1, 0, 60),                      // 2: 4th call succeeds, 3 sleeps ~90ms
+            (5, 1, 1, 0, 120),                     // 3: 6th call succeeds, 5 sleeps ~150ms
+            (6, 2, 1, 1, 120),                     // 4: 2nd round succeeds immediately
+            (8, 2, 2, 1, 30),                      // 5: 2nd round 3rd call succeeds, 2 sleeps ~60ms
+        ];
+
+        // Change this index to test different scenarios
+        let idx = 5;
+        let (success_at_call, loops, threshold, expect_machine_uid_at, min_ms) = scenarios[idx];
+
+        machine_uid_mock::SUCCESS_AT_CALL.store(success_at_call, Ordering::SeqCst);
+        machine_uid_mock::reset();
+
+        let pk = config::Config::get_key_pair().1;
+        let machine_uid: Vec<u8> = machine_uid::get().unwrap().into();
+
+        for round in 0..loops {
+            let start = Instant::now();
+            let uuid = get_uuid();
+            let elapsed = start.elapsed();
+
+            let is_machine_uid = uuid == machine_uid;
+            let is_pk = uuid == pk;
+            println!(
+                "round {}: elapsed {:?}, machine_uid={}, pk={}",
+                round, elapsed, is_machine_uid, is_pk
+            );
+
+            // Assert return value
+            if round >= expect_machine_uid_at {
+                assert!(is_machine_uid, "round {} should return machine_uid", round);
+            } else {
+                assert!(is_pk, "round {} should return pk", round);
+            }
+
+            // Assert timing
+            if round < threshold {
+                assert!(
+                    elapsed.as_millis() >= min_ms,
+                    "round {} elapsed {:?} should >= {}ms",
+                    round,
+                    elapsed,
+                    min_ms
+                );
+            } else {
+                assert!(
+                    elapsed.as_millis() < 100,
+                    "round {} elapsed {:?} should < 100ms",
+                    round,
+                    elapsed
+                );
+            }
+        }
+    }
 }

[Copilot]

Pull request overview

Improves resilience of the password/config encryption/decryption path when machine-UID retrieval is flaky (e.g., during shutdown) and when historical encryptions may have used the config key-pair instead of the machine UID.

Changes:

• Cache machine UID after successful retrieval and add retry/backoff behavior when retrieval fails.
• Add a decryption fallback path: if decryption with machine UID fails, retry with the key-pair-derived key.
• Add a unit test intended to cover the pk fallback behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/lib.rs	Adds machine-UID caching plus retry logic in get_uuid() before falling back to key-pair bytes.
src/password_security.rs	Updates symmetric_crypt to retry decryption with pk-derived key on failure; adds a new test for the fallback behavior.

Comments suppressed due to low confidence (1)

src/password_security.rs:192

• secretbox is used with a constant all-zero nonce. Reusing a nonce with the same key in XSalsa20-Poly1305 breaks confidentiality and can allow plaintext recovery across multiple encrypted values. Consider generating a random nonce per encryption (and storing it alongside the ciphertext, e.g., prefixing it) so decrypt can read it back safely; this can be rolled out via a new version tag.

    let nonce = secretbox::Nonce([0; secretbox::NONCEBYTES]);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[rustdesk]

Have you really tested this? or can be tested anv verified?

[21pages]

The logic has been verified by the test code above. Real-world failure scenarios are not tested.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

get_existing_key_pair holds the KEY_PAIR mutex while doing config file I/O (Config::load_). This can block other threads that need the keypair (including encryption/decryption paths) for the duration of disk access. Consider releasing the lock before loading from disk (double-checked locking pattern: check cache under lock, drop lock, load, then re-lock and set if still empty).

Suggested change

	let mut lock = KEY_PAIR.lock().unwrap();
	if let Some(p) = lock.as_ref() {
	return Some(p.clone());
	}
	let config = Config::load_::<Config>("");
	if !config.key_pair.0.is_empty() {
	*lock = Some(config.key_pair.clone());
	Some(config.key_pair)
	} else {
	None
	}
	// First, check the cache under the mutex.
	{
	let lock = KEY_PAIR.lock().unwrap();
	if let Some(p) = lock.as_ref() {
	return Some(p.clone());
	}
	}
	// Load config from disk without holding the mutex.
	let config = Config::load_::<Config>("");
	if config.key_pair.0.is_empty() {
	return None;
	}
	// Re-lock and perform a second check in case another thread
	// populated the cache while we were loading the config.
	let mut lock = KEY_PAIR.lock().unwrap();
	if let Some(p) = lock.as_ref() {
	return Some(p.clone());
	}
	*lock = Some(config.key_pair.clone());
	Some(config.key_pair)

[Copilot]

This test assumes crate::get_uuid() returns a machine UID distinct from the config public key. If machine_uid::get() fails in the test environment (CI, containers, permissions), get_uuid() can fall back to Config::get_key_pair().1, making uuid == pk and failing the test (or making the fallback path untestable). To make this deterministic, consider refactoring symmetric_crypt to call an internal helper that accepts the primary key material (uuid) and optional fallback key, so the test can inject a non-pk uuid without depending on the host machine UID behavior.