Cleanup and add docs

apps/desktop/desktop_native/core/src/biometric/mod.rs

@@ -17,13 +17,14 @@ pub trait BiometricTrait {
     async fn authenticate(&self, hwnd: Vec<u8>, message: String) -> Result<bool>;
     /// Check if biometric authentication is available
     async fn authenticate_available(&self) -> Result<bool>;
-    /// Enroll a key for persistent unlock
+    /// Enroll a key for persistent unlock. If the implementation does not support persistent enrollment,
+    /// this function should do nothing.
     async fn enroll_persistent(&self, user_id: &str, key: &[u8]) -> Result<()>;
     /// Clear the persistent and ephemeral keys
     async fn unenroll(&self, user_id: &str) -> Result<()>;
     /// Check if a persistent (survives app restarts and reboots) key is set for a user
     async fn has_persistent(&self, user_id: &str) -> Result<bool>;
-    /// On every unlock, the client provides a key to be held for subsequent biometric unlock
+    /// Provide a the key to be ephemerally held. This should be called on every unlock.
     async fn provide_key(&self, user_id: &str, key: &[u8]);
     /// Perform biometric unlock and return the key
     async fn unlock(&self, user_id: &str, hwnd: Vec<u8>) -> Result<Vec<u8>>;

apps/desktop/desktop_native/core/src/biometric/windows.rs

@@ -1,18 +1,23 @@
 //! This file implements Windows-Hello based biometric unlock.
 //!
+//! There are two paths implemented here.
+//! The former via UV + ephemerally (but protected) keys. This only works after first unlock.
+//! The latter via a signing API, that deterministically signs a challenge, from which a windows hello key is derived. This key
+//! is used to encrypt the protected key.
+//! 
 //! # Security
-//! Note: There are two scenarios to consider, with different security implications. This section
-//! describes the assumed security model and security guarantees achieved. In the required security
-//! guarantee is that a locked vault - a running app - cannot be unlocked when the device (user-space)
+//! The security goal is that a locked vault - a running app - cannot be unlocked when the device (user-space)
 //! is compromised in this state.
-//!
-//! ## Require master password on app restart
-//! In this scenario, when first unlocking the app, the app sends the user-key to this module, which holds it in secure memory,
+//! 
+//! ## UV path
+//! When first unlocking the app, the app sends the user-key to this module, which holds it in secure memory,
 //! protected by DPAPI. This makes it inaccessible to other processes, unless they compromise the system administrator, or kernel.
 //! While the app is running this key is held in memory, even if locked. When unlocking, the app will prompt the user via
 //! `windows_hello_authenticate` to get a yes/no decision on whether to release the key to the app.
-//!
-//! ## Do not require master password on app restart
+//! Note: Further process isolation is needed here so that code cannot be injected into the running process, which may
+//! circumvent DPAPI.
+//! 
+//! ## Sign path
 //! In this scenario, when enrolling, the app sends the user-key to this module, which derives the windows hello key
 //! with the Windows Hello prompt. This is done by signing a per-user challenge, which produces a deterministic
 //! signature which is hashed to obtain a key. This key is used to encrypt and persist the vault unlock key (user key).
@@ -97,10 +102,8 @@ impl super::BiometricTrait for BiometricLockSystem {
     }
 
     async fn unenroll(&self, user_id: &str) -> Result<()> {
-        let mut secure_memory = self.secure_memory.lock().await;
-        secure_memory.remove(user_id);
-        delete_keychain_entry(user_id).await?;
-        Ok(())
+        self.secure_memory.lock().await.remove(user_id);
+        delete_keychain_entry(user_id).await
     }
 
     async fn enroll_persistent(&self, user_id: &str, key: &[u8]) -> Result<()> {
@@ -116,15 +119,7 @@ impl super::BiometricTrait for BiometricLockSystem {
         // This key is unique to the challenge
         let windows_hello_key = windows_hello_authenticate_with_crypto(&challenge)?;
 
-        // Wrap key with xchacha20-poly1305
-        let nonce = {
-            let mut nonce_bytes = [0u8; 24];
-            rand::fill(&mut nonce_bytes);
-            XNonce::clone_from_slice(&nonce_bytes)
-        };
-        let wrapped_key = XChaCha20Poly1305::new(&windows_hello_key.into())
-            .encrypt(&nonce, key)
-            .map_err(|e| anyhow!(e))?;
+        let (wrapped_key, nonce) = encrypt_data(&windows_hello_key, key).unwrap();
 
         set_keychain_entry(
             user_id,
@@ -137,13 +132,11 @@ impl super::BiometricTrait for BiometricLockSystem {
                 wrapped_key,
             },
         )
-        .await?;
-        Ok(())
+        .await
     }
 
     async fn provide_key(&self, user_id: &str, key: &[u8]) {
-        let mut secure_memory = self.secure_memory.lock().await;
-        secure_memory.put(user_id.to_string(), key);
+        self.secure_memory.lock().await.put(user_id.to_string(), key);
     }
 
     async fn unlock(&self, user_id: &str, hwnd: Vec<u8>) -> Result<Vec<u8>> {
@@ -177,12 +170,11 @@ impl super::BiometricTrait for BiometricLockSystem {
             let keychain_entry = get_keychain_entry(user_id).await?;
             let windows_hello_key =
                 windows_hello_authenticate_with_crypto(&keychain_entry.challenge)?;
-            let decrypted_key = XChaCha20Poly1305::new(&windows_hello_key.into())
-                .decrypt(
-                    keychain_entry.nonce.as_slice().into(),
-                    keychain_entry.wrapped_key.as_slice(),
-                )
-                .map_err(|e| anyhow!(e))?;
+            let decrypted_key = decrypt_data(
+                &windows_hello_key,
+                &keychain_entry.wrapped_key,
+                &keychain_entry.nonce,
+            )?;
             // The first unlock already sets the key for subsequent unlocks. The key may again be set externally after unlock finishes.
             secure_memory.put(user_id.to_string(), &decrypted_key.clone());
             Ok(decrypted_key)
@@ -310,9 +302,38 @@ async fn has_keychain_entry(user_id: &str) -> Result<bool> {
         .is_empty())
 }
 
+/// Encrypt data with XChaCha20Poly1305
+fn encrypt_data(key: &[u8; 32], plaintext: &[u8]) -> Result<(Vec<u8>, [u8; 24])> {
+    let cipher = XChaCha20Poly1305::new(key.into());
+    let mut nonce = [0u8; 24];
+    rand::fill(&mut nonce);
+    let ciphertext = cipher
+        .encrypt(&XNonce::from_slice(&nonce), plaintext)
+        .map_err(|e| anyhow!(e))?;
+    Ok((ciphertext, nonce))
+}
+
+/// Decrypt data with XChaCha20Poly1305
+fn decrypt_data(key: &[u8; 32], ciphertext: &[u8], nonce: &[u8; 24]) -> Result<Vec<u8>> {
+    let cipher = XChaCha20Poly1305::new(key.into());
+    let plaintext = cipher
+        .decrypt(&XNonce::from_slice(nonce), ciphertext)
+        .map_err(|e| anyhow!(e))?;
+    Ok(plaintext)
+}
+
 #[cfg(test)]
 mod tests {
-    use crate::biometric::{biometric::{windows_hello_authenticate, windows_hello_authenticate_with_crypto}, BiometricLockSystem, BiometricTrait};
+    use crate::biometric::{biometric::{decrypt_data, encrypt_data, windows_hello_authenticate, windows_hello_authenticate_with_crypto}, BiometricLockSystem, BiometricTrait};
+
+    #[test]
+    fn test_encrypt_decrypt() {
+        let key = [0u8; 32];
+        let plaintext = b"Test data";
+        let (ciphertext, nonce) = encrypt_data(&key, plaintext).unwrap();
+        let decrypted = decrypt_data(&key, &ciphertext, &nonce).unwrap();
+        assert_eq!(plaintext.to_vec(), decrypted);
+    }
 
     // Note: These tests are ignored because they require manual intervention to run
 

apps/desktop/desktop_native/core/src/biometric/windows_focus.rs

@@ -25,14 +25,14 @@ pub(crate) fn get_active_window() -> Option<HwndHolder> {
 /// Only works when the process has permission to foreground, either by being in foreground
 /// Or by being given foreground permission https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-setforegroundwindow#remarks
 pub fn focus_security_prompt() {
-    let class_name = s!("Credential Dialog Xaml Host");
-    let hwnd = unsafe { FindWindowA(class_name, None) };
-    if let Ok(hwnd) = hwnd {
+    let hwnd_result = unsafe { FindWindowA(s!("Credential Dialog Xaml Host"), None) };
+    if let Ok(hwnd) = hwnd_result {
         set_focus(hwnd);
     }
 }
 
-pub(crate) fn set_focus(window: HWND) {
+/// Sets focus to a window using a few unstable methods
+pub(crate) fn set_focus(hwnd: HWND) {
     unsafe {
         // Windows REALLY does not like apps stealing focus, even if it is for fixing Windows-Hello bugs.
         // The windows hello signing prompt NEEDS to be focused instantly, or it will error, but it does
@@ -52,7 +52,6 @@ pub(crate) fn set_focus(window: HWND) {
         //   The calling process received the last input event.
         //   Either the foreground process or the calling process is being debugged.
 
-        // Attach to the foreground thread once attached, we can foregroud, even if in the background
         // Update the foreground lock timeout temporarily
         let mut old_timeout = 0;
         let _ = SystemParametersInfoW(
@@ -76,12 +75,11 @@ pub(crate) fn set_focus(window: HWND) {
             );
         });
 
-        // Attach to the active window's thread
+        // Attach to the foreground thread once attached, we can foregroud, even if in the background
         let dw_current_thread = GetCurrentThreadId();
         let dw_fg_thread = GetWindowThreadProcessId(GetForegroundWindow(), None);
 
         let _ = AttachThreadInput(dw_current_thread, dw_fg_thread, true);
-        let hwnd = window;
         let _ = SetForegroundWindow(hwnd);
         SetCapture(hwnd);
         let _ = SetFocus(Some(hwnd));