bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2025-12-18 17:23:37 +00:00
Code Issues Releases Wiki Activity

[PM-18054] Chrome extension biometric unlock not functioning correctly with Windows Hello. (#14953)

Browse Source 
* Chrome extension biometric unlock not functioning correctly with Windows Hello.

When unlocking via Windows Hello prompt, the popup have to be in the foreground. If it is not, even for short amount of time (few seconds), if later prompt confirmed, it won't return success when returning signed os key half.

* unit test coverage

* unit test coverage

* exclude test files from build

* use electron `setAlwaysOnTop` instead of toggle

* remove Windows os key half created with derive_key_material biometric function, that prompted Windows Hello.

Moves Windows hello prompt into getBiometricKey.
Witness key no longer needed.

* windows crate formatting

* remove biometric on app start for windows

* failing os biometrics windows unit tests

* cleanup of os biometrics windows unit tests

* increased coverage of os biometrics windows unit tests

* open Windows Hello prompt in the currently focused window, instead of always desktop app

* conflict resolution after merge, typescript lint issues, increased test coverage.

* backwards compatibility when require password on start was disabled

* biometric unlock cancellation and error handling

* biometric settings simplifications
This commit is contained in:
Maciej Zieniuk
2025-07-21 19:35:31 +02:00
committed by GitHub
parent 8b5e6adc37
commit 167fa9a7ab
11 changed files with 481 additions and 603 deletions
Download Patch File Download Diff File Expand all files Collapse all files

90
apps/desktop/desktop_native/core/src/biometric/mod.rs
View File

@@ -83,3 +83,93 @@ impl KeyMaterial {
Ok(Sha256::digest(self.digest_material()))
}
}
#[cfg(test)]
mod tests {
use crate::biometric::{decrypt, encrypt, KeyMaterial};
use crate::crypto::CipherString;
use base64::{engine::general_purpose::STANDARD as base64_engine, Engine};
use std::str::FromStr;
fn key_material() -> KeyMaterial {
KeyMaterial {
os_key_part_b64: "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned(),
client_key_part_b64: Some("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned()),
}
}
#[test]
fn test_encrypt() {
let key_material = key_material();
let iv_b64 = "l9fhDUP/wDJcKwmEzcb/3w==".to_owned();
let secret = encrypt("secret", &key_material, &iv_b64)
.unwrap()
.parse::<CipherString>()
.unwrap();
match secret {
CipherString::AesCbc256_B64 { iv, data: _ } => {
assert_eq!(iv_b64, base64_engine.encode(iv));
}
_ => panic!("Invalid cipher string"),
}
}
#[test]
fn test_decrypt() {
let secret =
CipherString::from_str("0.l9fhDUP/wDJcKwmEzcb/3w==|uP4LcqoCCj5FxBDP77NV6Q==").unwrap(); // output from test_encrypt
let key_material = key_material();
assert_eq!(decrypt(&secret, &key_material).unwrap(), "secret")
}
#[test]
fn key_material_produces_valid_key() {
let result = key_material().derive_key().unwrap();
assert_eq!(result.len(), 32);
}
#[test]
fn key_material_uses_os_part() {
let mut key_material = key_material();
let result = key_material.derive_key().unwrap();
key_material.os_key_part_b64 = "BAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned();
let result2 = key_material.derive_key().unwrap();
assert_ne!(result, result2);
}
#[test]
fn key_material_uses_client_part() {
let mut key_material = key_material();
let result = key_material.derive_key().unwrap();
key_material.client_key_part_b64 =
Some("BAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned());
let result2 = key_material.derive_key().unwrap();
assert_ne!(result, result2);
}
#[test]
fn key_material_produces_consistent_os_only_key() {
let mut key_material = key_material();
key_material.client_key_part_b64 = None;
let result = key_material.derive_key().unwrap();
assert_eq!(
result,
[
81, 100, 62, 172, 151, 119, 182, 58, 123, 38, 129, 116, 209, 253, 66, 118, 218,
237, 236, 155, 201, 234, 11, 198, 229, 171, 246, 144, 71, 188, 84, 246
]
.into()
);
}
#[test]
fn key_material_produces_unique_os_only_key() {
let mut key_material = key_material();
key_material.client_key_part_b64 = None;
let result = key_material.derive_key().unwrap();
key_material.os_key_part_b64 = "BAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned();
let result2 = key_material.derive_key().unwrap();
assert_ne!(result, result2);
}
}

185
apps/desktop/desktop_native/core/src/biometric/windows.rs
View File

@@ -1,22 +1,18 @@
use std::{
ffi::c_void,
str::FromStr,
sync::{atomic::AtomicBool, Arc},
};
use std::{ffi::c_void, str::FromStr};
use anyhow::{anyhow, Result};
use base64::{engine::general_purpose::STANDARD as base64_engine, Engine};
use rand::RngCore;
use sha2::{Digest, Sha256};
use windows::{
core::{factory, h, HSTRING},
Security::{
Credentials::{
KeyCredentialCreationOption, KeyCredentialManager, KeyCredentialStatus, UI::*,
},
Cryptography::CryptographicBuffer,
core::{factory, HSTRING},
Security::Credentials::UI::{
UserConsentVerificationResult, UserConsentVerifier, UserConsentVerifierAvailability,
},
Win32::{
Foundation::HWND, System::WinRT::IUserConsentVerifierInterop,
UI::WindowsAndMessaging::GetForegroundWindow,
},
Win32::{Foundation::HWND, System::WinRT::IUserConsentVerifierInterop},
};
use windows_future::IAsyncOperation;
@@ -25,10 +21,7 @@ use crate::{
crypto::CipherString,
};
use super::{
decrypt, encrypt,
windows_focus::{focus_security_prompt, set_focus},
};
use super::{decrypt, encrypt, windows_focus::set_focus};
/// The Windows OS implementation of the biometric trait.
pub struct Biometric {}
@@ -44,9 +37,15 @@ impl super::BiometricTrait for Biometric {
// should set the window to the foreground and focus it.
set_focus(window);
// Windows Hello prompt must be in foreground, focused, otherwise the face or fingerprint
// unlock will not work. We get the current foreground window, which will either be the
// Bitwarden desktop app or the browser extension.
let foreground_window = unsafe { GetForegroundWindow() };
let interop = factory::<UserConsentVerifier, IUserConsentVerifierInterop>()?;
let operation: IAsyncOperation<UserConsentVerificationResult> =
unsafe { interop.RequestVerificationForWindowAsync(window, &HSTRING::from(message))? };
let operation: IAsyncOperation<UserConsentVerificationResult> = unsafe {
interop.RequestVerificationForWindowAsync(foreground_window, &HSTRING::from(message))?
};
let result = operation.get()?;
match result {
@@ -65,14 +64,6 @@ impl super::BiometricTrait for Biometric {
}
}
/// Derive the symmetric encryption key from the Windows Hello signature.
///
/// This works by signing a static challenge string with Windows Hello protected key store. The
/// signed challenge is then hashed using SHA-256 and used as the symmetric encryption key for the
/// Windows Hello protected keys.
///
/// Windows will only sign the challenge if the user has successfully authenticated with Windows,
/// ensuring user presence.
fn derive_key_material(challenge_str: Option<&str>) -> Result<OsDerivedKey> {
let challenge: [u8; 16] = match challenge_str {
Some(challenge_str) => base64_engine
@@ -81,51 +72,10 @@ impl super::BiometricTrait for Biometric {
.map_err(|e: Vec<_>| anyhow!("Expect length {}, got {}", 16, e.len()))?,
None => random_challenge(),
};
let bitwarden = h!("Bitwarden");
let result = KeyCredentialManager::RequestCreateAsync(
bitwarden,
KeyCredentialCreationOption::FailIfExists,
)?
.get()?;
let result = match result.Status()? {
KeyCredentialStatus::CredentialAlreadyExists => {
KeyCredentialManager::OpenAsync(bitwarden)?.get()?
}
KeyCredentialStatus::Success => result,
_ => return Err(anyhow!("Failed to create key credential")),
};
let challenge_buffer = CryptographicBuffer::CreateFromByteArray(&challenge)?;
let async_operation = result.Credential()?.RequestSignAsync(&challenge_buffer)?;
focus_security_prompt();
let done = Arc::new(AtomicBool::new(false));
let done_clone = done.clone();
let _ = std::thread::spawn(move || loop {
if !done_clone.load(std::sync::atomic::Ordering::Relaxed) {
focus_security_prompt();
std::thread::sleep(std::time::Duration::from_millis(500));
} else {
break;
}
});
let signature = async_operation.get();
done.store(true, std::sync::atomic::Ordering::Relaxed);
let signature = signature?;
if signature.Status()? != KeyCredentialStatus::Success {
return Err(anyhow!("Failed to sign data"));
}
let signature_buffer = signature.Result()?;
let mut signature_value =
windows::core::Array::<u8>::with_len(signature_buffer.Length().unwrap() as usize);
CryptographicBuffer::CopyToByteArray(&signature_buffer, &mut signature_value)?;
let key = Sha256::digest(&*signature_value);
// Uses a key derived from the iv. This key is not intended to add any security
// but only a place-holder
let key = Sha256::digest(challenge);
let key_b64 = base64_engine.encode(key);
let iv_b64 = base64_engine.encode(challenge);
Ok(OsDerivedKey { key_b64, iv_b64 })
@@ -182,10 +132,9 @@ fn random_challenge() -> [u8; 16] {
mod tests {
use super::*;
use crate::biometric::{encrypt, BiometricTrait};
use crate::biometric::BiometricTrait;
#[test]
#[cfg(feature = "manual_test")]
fn test_derive_key_material() {
let iv_input = "l9fhDUP/wDJcKwmEzcb/3w==";
let result = <Biometric as BiometricTrait>::derive_key_material(Some(iv_input)).unwrap();
@@ -195,7 +144,6 @@ mod tests {
}
#[test]
#[cfg(feature = "manual_test")]
fn test_derive_key_material_no_iv() {
let result = <Biometric as BiometricTrait>::derive_key_material(None).unwrap();
let key = base64_engine.decode(result.key_b64).unwrap();
@@ -221,38 +169,8 @@ mod tests {
assert!(<Biometric as BiometricTrait>::available().await.unwrap())
}
#[test]
fn test_encrypt() {
let key_material = KeyMaterial {
os_key_part_b64: "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned(),
client_key_part_b64: Some("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned()),
};
let iv_b64 = "l9fhDUP/wDJcKwmEzcb/3w==".to_owned();
let secret = encrypt("secret", &key_material, &iv_b64)
.unwrap()
.parse::<CipherString>()
.unwrap();
match secret {
CipherString::AesCbc256_B64 { iv, data: _ } => {
assert_eq!(iv_b64, base64_engine.encode(iv));
}
_ => panic!("Invalid cipher string"),
}
}
#[test]
fn test_decrypt() {
let secret =
CipherString::from_str("0.l9fhDUP/wDJcKwmEzcb/3w==|uP4LcqoCCj5FxBDP77NV6Q==").unwrap(); // output from test_encrypt
let key_material = KeyMaterial {
os_key_part_b64: "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned(),
client_key_part_b64: Some("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned()),
};
assert_eq!(decrypt(&secret, &key_material).unwrap(), "secret")
}
#[tokio::test]
#[cfg(feature = "manual_test")]
async fn get_biometric_secret_requires_key() {
let result = <Biometric as BiometricTrait>::get_biometric_secret("", "", None).await;
assert!(result.is_err());
@@ -263,6 +181,7 @@ mod tests {
}
#[tokio::test]
#[cfg(feature = "manual_test")]
async fn get_biometric_secret_handles_unencrypted_secret() {
let test = "test";
let secret = "password";
@@ -284,6 +203,7 @@ mod tests {
}
#[tokio::test]
#[cfg(feature = "manual_test")]
async fn get_biometric_secret_handles_encrypted_secret() {
let test = "test";
let secret =
@@ -316,61 +236,4 @@ mod tests {
"Key material is required for Windows Hello protected keys"
);
}
fn key_material() -> KeyMaterial {
KeyMaterial {
os_key_part_b64: "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned(),
client_key_part_b64: Some("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned()),
}
}
#[test]
fn key_material_produces_valid_key() {
let result = key_material().derive_key().unwrap();
assert_eq!(result.len(), 32);
}
#[test]
fn key_material_uses_os_part() {
let mut key_material = key_material();
let result = key_material.derive_key().unwrap();
key_material.os_key_part_b64 = "BAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned();
let result2 = key_material.derive_key().unwrap();
assert_ne!(result, result2);
}
#[test]
fn key_material_uses_client_part() {
let mut key_material = key_material();
let result = key_material.derive_key().unwrap();
key_material.client_key_part_b64 =
Some("BAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned());
let result2 = key_material.derive_key().unwrap();
assert_ne!(result, result2);
}
#[test]
fn key_material_produces_consistent_os_only_key() {
let mut key_material = key_material();
key_material.client_key_part_b64 = None;
let result = key_material.derive_key().unwrap();
assert_eq!(
result,
[
81, 100, 62, 172, 151, 119, 182, 58, 123, 38, 129, 116, 209, 253, 66, 118, 218,
237, 236, 155, 201, 234, 11, 198, 229, 171, 246, 144, 71, 188, 84, 246
]
.into()
);
}
#[test]
fn key_material_produces_unique_os_only_key() {
let mut key_material = key_material();
key_material.client_key_part_b64 = None;
let result = key_material.derive_key().unwrap();
key_material.os_key_part_b64 = "BAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned();
let result2 = key_material.derive_key().unwrap();
assert_ne!(result, result2);
}
}