Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
59 changes: 57 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
182 changes: 176 additions & 6 deletions src/password_security.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
Comment on lines +96 to +113
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

The is_encrypted heuristic may have false positives with certain plaintext values.

While the MACBYTES length check significantly reduces false positive risk, a plaintext value that:

  1. Starts with "00"
  2. Has valid base64 characters after the prefix
  3. Decodes to >= 16 bytes

...would be misclassified as already encrypted and skip encryption, storing plaintext.

For example, "00QUFBQUFBQUFBQUFBQUFBQUFBQQ==" (decodes to 16 'A' characters) would pass all checks.

This is a low-probability edge case for typical passwords, but consider one of these mitigations:

  • Use a longer/stronger magic prefix (e.g., a unique sentinel)
  • Document this limitation as a known constraint for password values


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();
}
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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, ()> {
Expand All @@ -184,15 +211,30 @@ 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]);

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
}
}

Expand Down Expand Up @@ -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![];
Expand Down Expand Up @@ -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);
}
}
Loading