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

[21pages]

• 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

Test

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.

encrypted judgement

Encrypt the local password in str form and the peer password in vec form as usual. Modify get_uuid so that it only returns pk, preventing str and vec from being encrypted again. Then revert the change — both the local password and the peer password can work normally.

[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.

[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]

Pull request overview

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

---

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

[rustdesk]

codex review

• Findings:

1. High: possible self-deadlock in get_existing_key_pair() due to lock re-entry through config decryption fallback.

• src/config.rs:1004 acquires KEY_PAIR mutex and keeps it held while calling Config::load_::("") at src/config.rs:1009.
• During Config::load_, decryption can call symmetric_crypt(), whose fallback path calls Config::get_existing_key_pair() again (src/password_security.rs:208), attempting to lock KEY_PAIR recursively on the
  same thread.
• This can deadlock exactly in the mismatch/error case this fallback is intended to handle.

2. Medium: plaintext can be falsely treated as “already encrypted”, skipping encryption.

• is_encrypted() only checks "00" prefix + base64 parse (src/password_security.rs:98), and encryption entrypoints now rely on it (src/password_security.rs:105, src/password_security.rs:142).
• Any plaintext like 00YWJjZA== will be treated as encrypted and returned unchanged, causing secret-at-rest not encrypted behavior.

3. Low: sensitive identifier source logged at info level.

• src/config.rs:922 logs MAC address and derived ID (Generated id ... from mac address ...), which can leak hardware identifier data into normal logs.

If you want, I can propose a minimal patch for #1 and #2 (deadlock-safe key loading + stronger duplicate-encryption check).

[21pages]

High: possible self-deadlock in get_existing_key_pair() due to lock re-entry through config decryption fallback.

Config::get_existing_key_pair() does hold KEY_PAIR while calling Config::load_::("") (src/config.rs:1004, src/config.rs:1009), but load_ is only a raw deserialize
wrapper (src/config.rs:546-555). It does not run Config::load() and does not call decrypt_* / symmetric_crypt.

The fallback in symmetric_crypt (src/password_security.rs:208) is only reachable from actual decryption flows, not from load_. So there is no recursive re-entry into KEY_PAIR
through that specific chain in the current implementation.

[rustdesk]

⚠️ Concerns

1. is_encrypted() false positives — Medium Risk
   The is_encrypted heuristic (prefix "00" + valid base64) is weak. Any plaintext password that happens to start with "00" followed by a base64-valid string (e.g., "00YWJjZA==") will be treated as "already encrypted" and never actually encrypted. This was flagged by the codex review and is a real concern.

WARNING

The is_encrypted() check is a heuristic, not cryptographic proof. A user-chosen password like 00SGVsbG8= would be silently stored in plaintext. Consider adding a stronger magic/sentinel or a length check — real encrypted output from secretbox::seal has a fixed overhead (MAC tag), so checking decoded_len >= secretbox::MACBYTES would significantly reduce false positives.

2. Deadlock concern — Likely Safe (but worth documenting)
   The codex review flagged a potential deadlock: get_existing_key_pair() holds KEY_PAIR mutex while calling Config::load_::(""). The author correctly responded that load_ is a raw confy::load / deserialize wrapper that does not go through decrypt_* / symmetric_crypt. This appears correct, but:

NOTE

It would be prudent to add a code comment in get_existing_key_pair() explicitly stating that Config::load_ must never call symmetric_crypt / decrypt_* to avoid deadlock, so future refactors don't accidentally introduce re-entry.

3. LOG_COUNT uses Relaxed ordering
   The LOG_COUNT atomic uses Relaxed ordering for both load and fetch-add. This means the count could slightly exceed 30 under contention (two threads both read 29, both increment). This is benign for a log-throttle, but worth noting.

4. MAC address logging at info level
   log::info!("Generated id {} from mac", id) leaks a hardware-derived identifier into normal logs. Consider log::debug! or at minimum redacting part of the value.

5. whoami::username() trimming null bytes
   rust
   let username = whoami::username().trim_end_matches('\0').to_owned();
   This suggests whoami may return null-terminated strings on macOS. This is defensive but unusual — it might be worth a comment explaining why, or filing upstream.

[Copilot]

Pull request overview

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

---

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