diff --git a/apps/desktop/desktop_native/core/src/biometric/mod.rs b/apps/desktop/desktop_native/core/src/biometric/mod.rs index 79be43b1bfc..e4d51f5da9a 100644 --- a/apps/desktop/desktop_native/core/src/biometric/mod.rs +++ b/apps/desktop/desktop_native/core/src/biometric/mod.rs @@ -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::() + .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); + } +} diff --git a/apps/desktop/desktop_native/core/src/biometric/windows.rs b/apps/desktop/desktop_native/core/src/biometric/windows.rs index 4c2e2c8ae25..99bec132edb 100644 --- a/apps/desktop/desktop_native/core/src/biometric/windows.rs +++ b/apps/desktop/desktop_native/core/src/biometric/windows.rs @@ -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::()?; - let operation: IAsyncOperation = - unsafe { interop.RequestVerificationForWindowAsync(window, &HSTRING::from(message))? }; + let operation: IAsyncOperation = 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 { 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::::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 = ::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 = ::derive_key_material(None).unwrap(); let key = base64_engine.decode(result.key_b64).unwrap(); @@ -221,38 +169,8 @@ mod tests { assert!(::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::() - .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 = ::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); - } } diff --git a/apps/desktop/src/app/accounts/settings.component.html b/apps/desktop/src/app/accounts/settings.component.html index 46cd323b071..473cfa73f1d 100644 --- a/apps/desktop/src/app/accounts/settings.component.html +++ b/apps/desktop/src/app/accounts/settings.component.html @@ -126,13 +126,13 @@ {{ biometricText | i18n }} - {{ - additionalBiometricSettingsText | i18n + {{ + "additionalTouchIdSettings" | i18n }}
@@ -152,7 +152,7 @@ supportsBiometric && this.form.value.biometric && (userHasMasterPassword || (this.form.value.pin && userHasPinSet)) && - this.isWindows + false " >
@@ -170,9 +170,6 @@ }
- {{ - "recommendedForSecurity" | i18n - }} diff --git a/apps/desktop/src/app/accounts/settings.component.spec.ts b/apps/desktop/src/app/accounts/settings.component.spec.ts index 16ada3fbc07..819438eaa3b 100644 --- a/apps/desktop/src/app/accounts/settings.component.spec.ts +++ b/apps/desktop/src/app/accounts/settings.component.spec.ts @@ -271,74 +271,46 @@ describe("SettingsComponent", () => { vaultTimeoutSettingsService.isBiometricLockSet.mockResolvedValue(true); }); - it("require password or pin on app start message when RemoveUnlockWithPin policy is disabled and pin set and windows desktop", async () => { - const policy = new Policy(); - policy.type = PolicyType.RemoveUnlockWithPin; - policy.enabled = false; - policyService.policiesByType$.mockReturnValue(of([policy])); - platformUtilsService.getDevice.mockReturnValue(DeviceType.WindowsDesktop); - i18nService.t.mockImplementation((id: string) => { - if (id === "requirePasswordOnStart") { - return "Require password or pin on app start"; - } else if (id === "requirePasswordWithoutPinOnStart") { - return "Require password on app start"; - } - return ""; + describe("windows desktop", () => { + beforeEach(() => { + platformUtilsService.getDevice.mockReturnValue(DeviceType.WindowsDesktop); + + // Recreate component to apply the correct device + fixture = TestBed.createComponent(SettingsComponent); + component = fixture.componentInstance; }); - pinServiceAbstraction.isPinSet.mockResolvedValue(true); - await component.ngOnInit(); - fixture.detectChanges(); + it("require password or pin on app start not visible when RemoveUnlockWithPin policy is disabled and pin set and windows desktop", async () => { + const policy = new Policy(); + policy.type = PolicyType.RemoveUnlockWithPin; + policy.enabled = false; + policyService.policiesByType$.mockReturnValue(of([policy])); + pinServiceAbstraction.isPinSet.mockResolvedValue(true); - const requirePasswordOnStartLabelElement = fixture.debugElement.query( - By.css("label[for='requirePasswordOnStart']"), - ); - expect(requirePasswordOnStartLabelElement).not.toBeNull(); - expect(requirePasswordOnStartLabelElement.children).toHaveLength(1); - expect(requirePasswordOnStartLabelElement.children[0].name).toBe("input"); - expect(requirePasswordOnStartLabelElement.children[0].attributes).toMatchObject({ - id: "requirePasswordOnStart", - type: "checkbox", + await component.ngOnInit(); + fixture.detectChanges(); + + const requirePasswordOnStartLabelElement = fixture.debugElement.query( + By.css("label[for='requirePasswordOnStart']"), + ); + expect(requirePasswordOnStartLabelElement).toBeNull(); }); - const textNodes = requirePasswordOnStartLabelElement.childNodes - .filter((node) => node.nativeNode.nodeType === Node.TEXT_NODE) - .map((node) => node.nativeNode.wholeText?.trim()); - expect(textNodes).toContain("Require password or pin on app start"); - }); - it("require password on app start message when RemoveUnlockWithPin policy is enabled and pin set and windows desktop", async () => { - const policy = new Policy(); - policy.type = PolicyType.RemoveUnlockWithPin; - policy.enabled = true; - policyService.policiesByType$.mockReturnValue(of([policy])); - platformUtilsService.getDevice.mockReturnValue(DeviceType.WindowsDesktop); - i18nService.t.mockImplementation((id: string) => { - if (id === "requirePasswordOnStart") { - return "Require password or pin on app start"; - } else if (id === "requirePasswordWithoutPinOnStart") { - return "Require password on app start"; - } - return ""; + it("require password on app start not visible when RemoveUnlockWithPin policy is enabled and pin set and windows desktop", async () => { + const policy = new Policy(); + policy.type = PolicyType.RemoveUnlockWithPin; + policy.enabled = true; + policyService.policiesByType$.mockReturnValue(of([policy])); + pinServiceAbstraction.isPinSet.mockResolvedValue(true); + + await component.ngOnInit(); + fixture.detectChanges(); + + const requirePasswordOnStartLabelElement = fixture.debugElement.query( + By.css("label[for='requirePasswordOnStart']"), + ); + expect(requirePasswordOnStartLabelElement).toBeNull(); }); - pinServiceAbstraction.isPinSet.mockResolvedValue(true); - - await component.ngOnInit(); - fixture.detectChanges(); - - const requirePasswordOnStartLabelElement = fixture.debugElement.query( - By.css("label[for='requirePasswordOnStart']"), - ); - expect(requirePasswordOnStartLabelElement).not.toBeNull(); - expect(requirePasswordOnStartLabelElement.children).toHaveLength(1); - expect(requirePasswordOnStartLabelElement.children[0].name).toBe("input"); - expect(requirePasswordOnStartLabelElement.children[0].attributes).toMatchObject({ - id: "requirePasswordOnStart", - type: "checkbox", - }); - const textNodes = requirePasswordOnStartLabelElement.childNodes - .filter((node) => node.nativeNode.nodeType === Node.TEXT_NODE) - .map((node) => node.nativeNode.wholeText?.trim()); - expect(textNodes).toContain("Require password on app start"); }); }); diff --git a/apps/desktop/src/app/accounts/settings.component.ts b/apps/desktop/src/app/accounts/settings.component.ts index 74f02f8b619..bbe5fb60719 100644 --- a/apps/desktop/src/app/accounts/settings.component.ts +++ b/apps/desktop/src/app/accounts/settings.component.ts @@ -78,6 +78,7 @@ export class SettingsComponent implements OnInit, OnDestroy { showOpenAtLoginOption = false; isWindows: boolean; isLinux: boolean; + isMac: boolean; enableTrayText: string; enableTrayDescText: string; @@ -170,31 +171,33 @@ export class SettingsComponent implements OnInit, OnDestroy { private configService: ConfigService, private validationService: ValidationService, ) { - const isMac = this.platformUtilsService.getDevice() === DeviceType.MacOsDesktop; + this.isMac = this.platformUtilsService.getDevice() === DeviceType.MacOsDesktop; + this.isLinux = this.platformUtilsService.getDevice() === DeviceType.LinuxDesktop; + this.isWindows = this.platformUtilsService.getDevice() === DeviceType.WindowsDesktop; // Workaround to avoid ghosting trays https://github.com/electron/electron/issues/17622 this.requireEnableTray = this.platformUtilsService.getDevice() === DeviceType.LinuxDesktop; - const trayKey = isMac ? "enableMenuBar" : "enableTray"; + const trayKey = this.isMac ? "enableMenuBar" : "enableTray"; this.enableTrayText = this.i18nService.t(trayKey); this.enableTrayDescText = this.i18nService.t(trayKey + "Desc"); - const minToTrayKey = isMac ? "enableMinToMenuBar" : "enableMinToTray"; + const minToTrayKey = this.isMac ? "enableMinToMenuBar" : "enableMinToTray"; this.enableMinToTrayText = this.i18nService.t(minToTrayKey); this.enableMinToTrayDescText = this.i18nService.t(minToTrayKey + "Desc"); - const closeToTrayKey = isMac ? "enableCloseToMenuBar" : "enableCloseToTray"; + const closeToTrayKey = this.isMac ? "enableCloseToMenuBar" : "enableCloseToTray"; this.enableCloseToTrayText = this.i18nService.t(closeToTrayKey); this.enableCloseToTrayDescText = this.i18nService.t(closeToTrayKey + "Desc"); - const startToTrayKey = isMac ? "startToMenuBar" : "startToTray"; + const startToTrayKey = this.isMac ? "startToMenuBar" : "startToTray"; this.startToTrayText = this.i18nService.t(startToTrayKey); this.startToTrayDescText = this.i18nService.t(startToTrayKey + "Desc"); this.showOpenAtLoginOption = !ipc.platform.isWindowsStore; // DuckDuckGo browser is only for macos initially - this.showDuckDuckGoIntegrationOption = isMac; + this.showDuckDuckGoIntegrationOption = this.isMac; const localeOptions: any[] = []; this.i18nService.supportedTranslationLocales.forEach((locale) => { @@ -239,7 +242,6 @@ export class SettingsComponent implements OnInit, OnDestroy { async ngOnInit() { this.vaultTimeoutOptions = await this.generateVaultTimeoutOptions(); const activeAccount = await firstValueFrom(this.accountService.activeAccount$); - this.isLinux = (await this.platformUtilsService.getDevice()) === DeviceType.LinuxDesktop; // Autotype is for Windows initially const isWindows = this.platformUtilsService.getDevice() === DeviceType.WindowsDesktop; @@ -250,8 +252,6 @@ export class SettingsComponent implements OnInit, OnDestroy { this.userHasMasterPassword = await this.userVerificationService.hasMasterPassword(); - this.isWindows = this.platformUtilsService.getDevice() === DeviceType.WindowsDesktop; - this.currentUserEmail = activeAccount.email; this.currentUserId = activeAccount.id; @@ -911,28 +911,4 @@ export class SettingsComponent implements OnInit, OnDestroy { throw new Error("Unsupported platform"); } } - - get autoPromptBiometricsText() { - switch (this.platformUtilsService.getDevice()) { - case DeviceType.MacOsDesktop: - return "autoPromptTouchId"; - case DeviceType.WindowsDesktop: - return "autoPromptWindowsHello"; - case DeviceType.LinuxDesktop: - return "autoPromptPolkit"; - default: - throw new Error("Unsupported platform"); - } - } - - get additionalBiometricSettingsText() { - switch (this.platformUtilsService.getDevice()) { - case DeviceType.MacOsDesktop: - return "additionalTouchIdSettings"; - case DeviceType.WindowsDesktop: - return "additionalWindowsHelloSettings"; - default: - throw new Error("Unsupported platform"); - } - } } diff --git a/apps/desktop/src/key-management/biometrics/os-biometrics-linux.service.ts b/apps/desktop/src/key-management/biometrics/os-biometrics-linux.service.ts index c310f337182..26a8e949f38 100644 --- a/apps/desktop/src/key-management/biometrics/os-biometrics-linux.service.ts +++ b/apps/desktop/src/key-management/biometrics/os-biometrics-linux.service.ts @@ -34,6 +34,7 @@ const policyFileName = "com.bitwarden.Bitwarden.policy"; const policyPath = "/usr/share/polkit-1/actions/"; const SERVICE = "Bitwarden_biometric"; + function getLookupKeyForUser(userId: UserId): string { return `${userId}_user_biometric`; } @@ -45,16 +46,18 @@ export default class OsBiometricsServiceLinux implements OsBiometricService { private cryptoFunctionService: CryptoFunctionService, private logService: LogService, ) {} + private _iv: string | null = null; // Use getKeyMaterial helper instead of direct access private _osKeyHalf: string | null = null; private clientKeyHalves = new Map(); async setBiometricKey(userId: UserId, key: SymmetricCryptoKey): Promise { - const clientKeyPartB64 = Utils.fromBufferToB64( - await this.getOrCreateBiometricEncryptionClientKeyHalf(userId, key), - ); - const storageDetails = await this.getStorageDetails({ clientKeyHalfB64: clientKeyPartB64 }); + const clientKeyHalf = await this.getOrCreateBiometricEncryptionClientKeyHalf(userId, key); + + const storageDetails = await this.getStorageDetails({ + clientKeyHalfB64: clientKeyHalf ? Utils.fromBufferToB64(clientKeyHalf) : undefined, + }); await biometrics.setBiometricSecret( SERVICE, getLookupKeyForUser(userId), @@ -63,6 +66,7 @@ export default class OsBiometricsServiceLinux implements OsBiometricService { storageDetails.ivB64, ); } + async deleteBiometricKey(userId: UserId): Promise { try { await passwords.deletePassword(SERVICE, getLookupKeyForUser(userId)); @@ -91,11 +95,15 @@ export default class OsBiometricsServiceLinux implements OsBiometricService { if (value == null || value == "") { return null; } else { - const clientKeyHalf = this.clientKeyHalves.get(userId); - const clientKeyPartB64 = Utils.fromBufferToB64(clientKeyHalf); + let clientKeyPartB64: string | null = null; + if (this.clientKeyHalves.has(userId)) { + clientKeyPartB64 = Utils.fromBufferToB64(this.clientKeyHalves.get(userId)!); + } const encValue = new EncString(value); this.setIv(encValue.iv); - const storageDetails = await this.getStorageDetails({ clientKeyHalfB64: clientKeyPartB64 }); + const storageDetails = await this.getStorageDetails({ + clientKeyHalfB64: clientKeyPartB64 ?? undefined, + }); const storedValue = await biometrics.getBiometricSecret( SERVICE, getLookupKeyForUser(userId), @@ -169,7 +177,6 @@ export default class OsBiometricsServiceLinux implements OsBiometricService { }): Promise<{ key_material: biometrics.KeyMaterial; ivB64: string }> { if (this._osKeyHalf == null) { const keyMaterial = await biometrics.deriveKeyMaterial(this._iv); - // osKeyHalf is based on the iv and in contrast to windows is not locked behind user verification! this._osKeyHalf = keyMaterial.keyB64; this._iv = keyMaterial.ivB64; } @@ -209,8 +216,8 @@ export default class OsBiometricsServiceLinux implements OsBiometricService { } if (clientKeyHalf == null) { // Set a key half if it doesn't exist - const keyBytes = await this.cryptoFunctionService.randomBytes(32); - const encKey = await this.encryptService.encryptBytes(keyBytes, key); + clientKeyHalf = await this.cryptoFunctionService.randomBytes(32); + const encKey = await this.encryptService.encryptBytes(clientKeyHalf, key); await this.biometricStateService.setEncryptedClientKeyHalf(encKey, userId); } diff --git a/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.spec.ts b/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.spec.ts index 674d97bf696..f301efc70e7 100644 --- a/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.spec.ts +++ b/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.spec.ts @@ -1,51 +1,65 @@ +import { randomBytes } from "node:crypto"; + +import { BrowserWindow } from "electron"; import { mock } from "jest-mock-extended"; import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { Utils } from "@bitwarden/common/platform/misc/utils"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { UserId } from "@bitwarden/common/types/guid"; -import { passwords } from "@bitwarden/desktop-napi"; +import { biometrics, passwords } from "@bitwarden/desktop-napi"; import { BiometricsStatus, BiometricStateService } from "@bitwarden/key-management"; import { WindowMain } from "../../main/window.main"; import OsBiometricsServiceWindows from "./os-biometrics-windows.service"; -jest.mock("@bitwarden/desktop-napi", () => ({ - biometrics: { - available: jest.fn(), - setBiometricSecret: jest.fn(), - getBiometricSecret: jest.fn(), - deleteBiometricSecret: jest.fn(), - prompt: jest.fn(), - deriveKeyMaterial: jest.fn(), - }, - passwords: { - getPassword: jest.fn(), - deletePassword: jest.fn(), - isAvailable: jest.fn(), - PASSWORD_NOT_FOUND: "Password not found", - }, -})); +import OsDerivedKey = biometrics.OsDerivedKey; + +jest.mock("@bitwarden/desktop-napi", () => { + return { + biometrics: { + available: jest.fn().mockResolvedValue(true), + getBiometricSecret: jest.fn().mockResolvedValue(""), + setBiometricSecret: jest.fn().mockResolvedValue(""), + deleteBiometricSecret: jest.fn(), + deriveKeyMaterial: jest.fn().mockResolvedValue({ + keyB64: "", + ivB64: "", + }), + prompt: jest.fn().mockResolvedValue(true), + }, + passwords: { + getPassword: jest.fn().mockResolvedValue(null), + deletePassword: jest.fn().mockImplementation(() => {}), + isAvailable: jest.fn(), + PASSWORD_NOT_FOUND: "Password not found", + }, + }; +}); + +describe("OsBiometricsServiceWindows", function () { + const i18nService = mock(); + const windowMain = mock(); + const browserWindow = mock(); + const encryptionService: EncryptService = mock(); + const cryptoFunctionService: CryptoFunctionService = mock(); + const biometricStateService: BiometricStateService = mock(); + const logService = mock(); -describe("OsBiometricsServiceWindows", () => { let service: OsBiometricsServiceWindows; - let i18nService: I18nService; - let windowMain: WindowMain; - let logService: LogService; - let biometricStateService: BiometricStateService; - const mockUserId = "test-user-id" as UserId; + const key = new SymmetricCryptoKey(new Uint8Array(64)); + const userId = "test-user-id" as UserId; + const serviceKey = "Bitwarden_biometric"; + const storageKey = `${userId}_user_biometric`; beforeEach(() => { - i18nService = mock(); - windowMain = mock(); - logService = mock(); - biometricStateService = mock(); - const encryptionService = mock(); - const cryptoFunctionService = mock(); + windowMain.win = browserWindow; + service = new OsBiometricsServiceWindows( i18nService, windowMain, @@ -62,20 +76,13 @@ describe("OsBiometricsServiceWindows", () => { describe("getBiometricsFirstUnlockStatusForUser", () => { const userId = "test-user-id" as UserId; - it("should return Available when requirePasswordOnRestart is false", async () => { - biometricStateService.getRequirePasswordOnStart = jest.fn().mockResolvedValue(false); - const result = await service.getBiometricsFirstUnlockStatusForUser(userId); - expect(result).toBe(BiometricsStatus.Available); - }); - it("should return Available when requirePasswordOnRestart is true and client key half is set", async () => { - biometricStateService.getRequirePasswordOnStart = jest.fn().mockResolvedValue(true); + it("should return Available when client key half is set", async () => { (service as any).clientKeyHalves = new Map(); (service as any).clientKeyHalves.set(userId, new Uint8Array([1, 2, 3, 4])); const result = await service.getBiometricsFirstUnlockStatusForUser(userId); expect(result).toBe(BiometricsStatus.Available); }); - it("should return UnlockNeeded when requirePasswordOnRestart is true and client key half is not set", async () => { - biometricStateService.getRequirePasswordOnStart = jest.fn().mockResolvedValue(true); + it("should return UnlockNeeded when client key half is not set", async () => { (service as any).clientKeyHalves = new Map(); const result = await service.getBiometricsFirstUnlockStatusForUser(userId); expect(result).toBe(BiometricsStatus.UnlockNeeded); @@ -83,32 +90,7 @@ describe("OsBiometricsServiceWindows", () => { }); describe("getOrCreateBiometricEncryptionClientKeyHalf", () => { - const userId = "test-user-id" as UserId; - const key = new SymmetricCryptoKey(new Uint8Array(64)); - let encryptionService: EncryptService; - let cryptoFunctionService: CryptoFunctionService; - - beforeEach(() => { - encryptionService = mock(); - cryptoFunctionService = mock(); - service = new OsBiometricsServiceWindows( - mock(), - windowMain, - mock(), - biometricStateService, - encryptionService, - cryptoFunctionService, - ); - }); - - it("should return null if getRequirePasswordOnRestart is false", async () => { - biometricStateService.getRequirePasswordOnStart = jest.fn().mockResolvedValue(false); - const result = await service.getOrCreateBiometricEncryptionClientKeyHalf(userId, key); - expect(result).toBeNull(); - }); - it("should return cached key half if already present", async () => { - biometricStateService.getRequirePasswordOnStart = jest.fn().mockResolvedValue(true); const cachedKeyHalf = new Uint8Array([10, 20, 30]); (service as any).clientKeyHalves.set(userId.toString(), cachedKeyHalf); const result = await service.getOrCreateBiometricEncryptionClientKeyHalf(userId, key); @@ -116,7 +98,6 @@ describe("OsBiometricsServiceWindows", () => { }); it("should decrypt and return existing encrypted client key half", async () => { - biometricStateService.getRequirePasswordOnStart = jest.fn().mockResolvedValue(true); biometricStateService.getEncryptedClientKeyHalf = jest .fn() .mockResolvedValue(new Uint8Array([1, 2, 3])); @@ -132,7 +113,6 @@ describe("OsBiometricsServiceWindows", () => { }); it("should generate, encrypt, store, and cache a new key half if none exists", async () => { - biometricStateService.getRequirePasswordOnStart = jest.fn().mockResolvedValue(true); biometricStateService.getEncryptedClientKeyHalf = jest.fn().mockResolvedValue(null); const randomBytes = new Uint8Array([7, 8, 9]); cryptoFunctionService.randomBytes = jest.fn().mockResolvedValue(randomBytes); @@ -148,101 +128,251 @@ describe("OsBiometricsServiceWindows", () => { encrypted, userId, ); - expect(result).toBeNull(); - expect((service as any).clientKeyHalves.get(userId.toString())).toBeNull(); + expect(result).toEqual(randomBytes); + expect((service as any).clientKeyHalves.get(userId.toString())).toEqual(randomBytes); }); }); + describe("supportsBiometrics", () => { + it("should return true if biometrics are available", async () => { + biometrics.available = jest.fn().mockResolvedValue(true); + + const result = await service.supportsBiometrics(); + + expect(result).toBe(true); + }); + + it("should return false if biometrics are not available", async () => { + biometrics.available = jest.fn().mockResolvedValue(false); + + const result = await service.supportsBiometrics(); + + expect(result).toBe(false); + }); + }); + + describe("getBiometricKey", () => { + beforeEach(() => { + biometrics.prompt = jest.fn().mockResolvedValue(true); + }); + + it("should return null when unsuccessfully authenticated biometrics", async () => { + biometrics.prompt = jest.fn().mockResolvedValue(false); + + const result = await service.getBiometricKey(userId); + + expect(result).toBeNull(); + }); + + it.each([null, undefined, ""])( + "should throw error when no biometric key is found '%s'", + async (password) => { + passwords.getPassword = jest.fn().mockResolvedValue(password); + + await expect(service.getBiometricKey(userId)).rejects.toThrow( + "Biometric key not found for user", + ); + + expect(passwords.getPassword).toHaveBeenCalledWith(serviceKey, storageKey); + }, + ); + + it.each([[false], [true]])( + "should return the biometricKey and setBiometricSecret called if password is not encrypted and cached clientKeyHalves is %s", + async (haveClientKeyHalves) => { + const clientKeyHalveBytes = new Uint8Array([1, 2, 3]); + if (haveClientKeyHalves) { + service["clientKeyHalves"].set(userId, clientKeyHalveBytes); + } + const biometricKey = key.toBase64(); + passwords.getPassword = jest.fn().mockResolvedValue(biometricKey); + biometrics.deriveKeyMaterial = jest.fn().mockResolvedValue({ + keyB64: "testKeyB64", + ivB64: "testIvB64", + } satisfies OsDerivedKey); + + const result = await service.getBiometricKey(userId); + + expect(result.toBase64()).toBe(biometricKey); + expect(passwords.getPassword).toHaveBeenCalledWith(serviceKey, storageKey); + expect(biometrics.setBiometricSecret).toHaveBeenCalledWith( + serviceKey, + storageKey, + biometricKey, + { + osKeyPartB64: "testKeyB64", + clientKeyPartB64: haveClientKeyHalves + ? Utils.fromBufferToB64(clientKeyHalveBytes) + : undefined, + }, + "testIvB64", + ); + }, + ); + + it.each([[false], [true]])( + "should return the biometricKey if password is encrypted and cached clientKeyHalves is %s", + async (haveClientKeyHalves) => { + const clientKeyHalveBytes = new Uint8Array([1, 2, 3]); + if (haveClientKeyHalves) { + service["clientKeyHalves"].set(userId, clientKeyHalveBytes); + } + const biometricKey = key.toBase64(); + const biometricKeyEncrypted = "2.testId|data|mac"; + passwords.getPassword = jest.fn().mockResolvedValue(biometricKeyEncrypted); + biometrics.getBiometricSecret = jest.fn().mockResolvedValue(biometricKey); + biometrics.deriveKeyMaterial = jest.fn().mockResolvedValue({ + keyB64: "testKeyB64", + ivB64: "testIvB64", + } satisfies OsDerivedKey); + + const result = await service.getBiometricKey(userId); + + expect(result.toBase64()).toBe(biometricKey); + expect(passwords.getPassword).toHaveBeenCalledWith(serviceKey, storageKey); + expect(biometrics.setBiometricSecret).not.toHaveBeenCalled(); + expect(biometrics.getBiometricSecret).toHaveBeenCalledWith(serviceKey, storageKey, { + osKeyPartB64: "testKeyB64", + clientKeyPartB64: haveClientKeyHalves + ? Utils.fromBufferToB64(clientKeyHalveBytes) + : undefined, + }); + }, + ); + }); + describe("deleteBiometricKey", () => { const serviceName = "Bitwarden_biometric"; const keyName = "test-user-id_user_biometric"; - const witnessKeyName = "test-user-id_user_biometric_witness"; it("should delete biometric key successfully", async () => { - await service.deleteBiometricKey(mockUserId); + await service.deleteBiometricKey(userId); expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, keyName); - expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, witnessKeyName); }); - it.each([ - [false, false], - [false, true], - [true, false], - ])( - "should not throw error if key found: %s and witness key found: %s", - async (keyFound, witnessKeyFound) => { - passwords.deletePassword = jest.fn().mockImplementation((_, account) => { - if (account === keyName) { - if (!keyFound) { - throw new Error(passwords.PASSWORD_NOT_FOUND); - } - return Promise.resolve(); - } - if (account === witnessKeyName) { - if (!witnessKeyFound) { - throw new Error(passwords.PASSWORD_NOT_FOUND); - } - return Promise.resolve(); - } - throw new Error("Unexpected key"); - }); + it.each([[false], [true]])("should not throw error if key found: %s", async (keyFound) => { + if (!keyFound) { + passwords.deletePassword = jest + .fn() + .mockRejectedValue(new Error(passwords.PASSWORD_NOT_FOUND)); + } - await service.deleteBiometricKey(mockUserId); + await service.deleteBiometricKey(userId); - expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, keyName); - expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, witnessKeyName); - if (!keyFound) { - expect(logService.debug).toHaveBeenCalledWith( - "[OsBiometricService] Biometric key %s not found for service %s.", - keyName, - serviceName, - ); - } - if (!witnessKeyFound) { - expect(logService.debug).toHaveBeenCalledWith( - "[OsBiometricService] Biometric witness key %s not found for service %s.", - witnessKeyName, - serviceName, - ); - } - }, - ); + expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, keyName); + if (!keyFound) { + expect(logService.debug).toHaveBeenCalledWith( + "[OsBiometricService] Biometric key %s not found for service %s.", + keyName, + serviceName, + ); + } + }); it("should throw error when deletePassword for key throws unexpected errors", async () => { const error = new Error("Unexpected error"); - passwords.deletePassword = jest.fn().mockImplementation((_, account) => { - if (account === keyName) { - throw error; - } - if (account === witnessKeyName) { - return Promise.resolve(); - } - throw new Error("Unexpected key"); - }); + passwords.deletePassword = jest.fn().mockRejectedValue(error); - await expect(service.deleteBiometricKey(mockUserId)).rejects.toThrow(error); + await expect(service.deleteBiometricKey(userId)).rejects.toThrow(error); expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, keyName); - expect(passwords.deletePassword).not.toHaveBeenCalledWith(serviceName, witnessKeyName); + }); + }); + + describe("authenticateBiometric", () => { + const hwnd = randomBytes(32).buffer; + const consentMessage = "Test Windows Hello Consent Message"; + + beforeEach(() => { + windowMain.win.getNativeWindowHandle = jest.fn().mockReturnValue(hwnd); + i18nService.t.mockReturnValue(consentMessage); }); - it("should throw error when deletePassword for witness key throws unexpected errors", async () => { - const error = new Error("Unexpected error"); - passwords.deletePassword = jest.fn().mockImplementation((_, account) => { - if (account === keyName) { - return Promise.resolve(); - } - if (account === witnessKeyName) { - throw error; - } - throw new Error("Unexpected key"); - }); + it("should return true when biometric authentication is successful", async () => { + const result = await service.authenticateBiometric(); - await expect(service.deleteBiometricKey(mockUserId)).rejects.toThrow(error); + expect(result).toBe(true); + expect(biometrics.prompt).toHaveBeenCalledWith(hwnd, consentMessage); + }); - expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, keyName); - expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, witnessKeyName); + it("should return false when biometric authentication fails", async () => { + biometrics.prompt = jest.fn().mockResolvedValue(false); + + const result = await service.authenticateBiometric(); + + expect(result).toBe(false); + expect(biometrics.prompt).toHaveBeenCalledWith(hwnd, consentMessage); + }); + }); + + describe("getStorageDetails", () => { + it.each([ + ["testClientKeyHalfB64", "testIvB64"], + [undefined, "testIvB64"], + ["testClientKeyHalfB64", null], + [undefined, null], + ])( + "should derive key material and ivB64 and return it when os key half not saved yet", + async (clientKeyHalfB64, ivB64) => { + service["setIv"](ivB64); + + const derivedKeyMaterial = { + keyB64: "derivedKeyB64", + ivB64: "derivedIvB64", + }; + biometrics.deriveKeyMaterial = jest.fn().mockResolvedValue(derivedKeyMaterial); + + const result = await service["getStorageDetails"]({ clientKeyHalfB64 }); + + expect(result).toEqual({ + key_material: { + osKeyPartB64: derivedKeyMaterial.keyB64, + clientKeyPartB64: clientKeyHalfB64, + }, + ivB64: derivedKeyMaterial.ivB64, + }); + expect(biometrics.deriveKeyMaterial).toHaveBeenCalledWith(ivB64); + expect(service["_osKeyHalf"]).toEqual(derivedKeyMaterial.keyB64); + expect(service["_iv"]).toEqual(derivedKeyMaterial.ivB64); + }, + ); + + it("should throw an error when deriving key material and returned iv is null", async () => { + service["setIv"]("testIvB64"); + + const derivedKeyMaterial = { + keyB64: "derivedKeyB64", + ivB64: null as string | undefined | null, + }; + biometrics.deriveKeyMaterial = jest.fn().mockResolvedValue(derivedKeyMaterial); + + await expect( + service["getStorageDetails"]({ clientKeyHalfB64: "testClientKeyHalfB64" }), + ).rejects.toThrow("Initialization Vector is null"); + + expect(biometrics.deriveKeyMaterial).toHaveBeenCalledWith("testIvB64"); + }); + }); + + describe("setIv", () => { + it("should set the iv and reset the osKeyHalf", () => { + const iv = "testIv"; + service["_osKeyHalf"] = "testOsKeyHalf"; + + service["setIv"](iv); + + expect(service["_iv"]).toBe(iv); + expect(service["_osKeyHalf"]).toBeNull(); + }); + + it("should set the iv to null when iv is undefined and reset the osKeyHalf", () => { + service["_osKeyHalf"] = "testOsKeyHalf"; + + service["setIv"](undefined); + + expect(service["_iv"]).toBeNull(); + expect(service["_osKeyHalf"]).toBeNull(); }); }); }); diff --git a/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.ts b/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.ts index 65abced1526..897304c9f61 100644 --- a/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.ts +++ b/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.ts @@ -3,7 +3,6 @@ import { EncryptService } from "@bitwarden/common/key-management/crypto/abstract import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; -import { EncryptionType } from "@bitwarden/common/platform/enums"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { UserId } from "@bitwarden/common/types/guid"; @@ -14,10 +13,8 @@ import { WindowMain } from "../../main/window.main"; import { OsBiometricService } from "./os-biometrics.service"; -const KEY_WITNESS_SUFFIX = "_witness"; -const WITNESS_VALUE = "known key"; - const SERVICE = "Bitwarden_biometric"; + function getLookupKeyForUser(userId: UserId): string { return `${userId}_user_biometric`; } @@ -43,18 +40,25 @@ export default class OsBiometricsServiceWindows implements OsBiometricService { } async getBiometricKey(userId: UserId): Promise { - const value = await passwords.getPassword(SERVICE, getLookupKeyForUser(userId)); - let clientKeyHalfB64: string | null = null; - if (this.clientKeyHalves.has(userId)) { - clientKeyHalfB64 = Utils.fromBufferToB64(this.clientKeyHalves.get(userId)); + const success = await this.authenticateBiometric(); + if (!success) { + return null; } + const value = await passwords.getPassword(SERVICE, getLookupKeyForUser(userId)); if (value == null || value == "") { - return null; - } else if (!EncString.isSerializedEncString(value)) { + throw new Error("Biometric key not found for user"); + } + + let clientKeyHalfB64: string | null = null; + if (this.clientKeyHalves.has(userId)) { + clientKeyHalfB64 = Utils.fromBufferToB64(this.clientKeyHalves.get(userId)!); + } + + if (!EncString.isSerializedEncString(value)) { // Update to format encrypted with client key half const storageDetails = await this.getStorageDetails({ - clientKeyHalfB64: clientKeyHalfB64, + clientKeyHalfB64: clientKeyHalfB64 ?? undefined, }); await biometrics.setBiometricSecret( @@ -69,7 +73,7 @@ export default class OsBiometricsServiceWindows implements OsBiometricService { const encValue = new EncString(value); this.setIv(encValue.iv); const storageDetails = await this.getStorageDetails({ - clientKeyHalfB64: clientKeyHalfB64, + clientKeyHalfB64: clientKeyHalfB64 ?? undefined, }); return SymmetricCryptoKey.fromString( await biometrics.getBiometricSecret( @@ -84,35 +88,16 @@ export default class OsBiometricsServiceWindows implements OsBiometricService { async setBiometricKey(userId: UserId, key: SymmetricCryptoKey): Promise { const clientKeyHalf = await this.getOrCreateBiometricEncryptionClientKeyHalf(userId, key); - if ( - await this.valueUpToDate({ - value: key, - clientKeyPartB64: Utils.fromBufferToB64(clientKeyHalf), - service: SERVICE, - storageKey: getLookupKeyForUser(userId), - }) - ) { - return; - } - const storageDetails = await this.getStorageDetails({ clientKeyHalfB64: Utils.fromBufferToB64(clientKeyHalf), }); - const storedValue = await biometrics.setBiometricSecret( + await biometrics.setBiometricSecret( SERVICE, getLookupKeyForUser(userId), key.toBase64(), storageDetails.key_material, storageDetails.ivB64, ); - const parsedStoredValue = new EncString(storedValue); - await this.storeValueWitness( - key, - parsedStoredValue, - SERVICE, - getLookupKeyForUser(userId), - Utils.fromBufferToB64(clientKeyHalf), - ); } async deleteBiometricKey(userId: UserId): Promise { @@ -129,21 +114,11 @@ export default class OsBiometricsServiceWindows implements OsBiometricService { throw e; } } - try { - await passwords.deletePassword(SERVICE, getLookupKeyForUser(userId) + KEY_WITNESS_SUFFIX); - } catch (e) { - if (e instanceof Error && e.message === passwords.PASSWORD_NOT_FOUND) { - this.logService.debug( - "[OsBiometricService] Biometric witness key %s not found for service %s.", - getLookupKeyForUser(userId) + KEY_WITNESS_SUFFIX, - SERVICE, - ); - } else { - throw e; - } - } } + /** + * Prompts Windows Hello + */ async authenticateBiometric(): Promise { const hwnd = this.windowMain.win.getNativeWindowHandle(); return await biometrics.prompt(hwnd, this.i18nService.t("windowsHelloConsentMessage")); @@ -155,7 +130,6 @@ export default class OsBiometricsServiceWindows implements OsBiometricService { clientKeyHalfB64: string | undefined; }): Promise<{ key_material: biometrics.KeyMaterial; ivB64: string }> { if (this._osKeyHalf == null) { - // Prompts Windows Hello const keyMaterial = await biometrics.deriveKeyMaterial(this._iv); this._osKeyHalf = keyMaterial.keyB64; this._iv = keyMaterial.ivB64; @@ -187,118 +161,6 @@ export default class OsBiometricsServiceWindows implements OsBiometricService { this._osKeyHalf = null; } - /** - * Stores a witness key alongside the encrypted value. This is used to determine if the value is up to date. - * - * @param unencryptedValue The key to store - * @param encryptedValue The encrypted value of the key to store. Used to sync IV of the witness key with the stored key. - * @param service The service to store the witness key under - * @param storageKey The key to store the witness key under. The witness key will be stored under storageKey + {@link KEY_WITNESS_SUFFIX} - * @returns - */ - private async storeValueWitness( - unencryptedValue: SymmetricCryptoKey, - encryptedValue: EncString, - service: string, - storageKey: string, - clientKeyPartB64: string | undefined, - ) { - if (encryptedValue.iv == null) { - return; - } - - const storageDetails = { - keyMaterial: this.witnessKeyMaterial(unencryptedValue, clientKeyPartB64), - ivB64: encryptedValue.iv, - }; - await biometrics.setBiometricSecret( - service, - storageKey + KEY_WITNESS_SUFFIX, - WITNESS_VALUE, - storageDetails.keyMaterial, - storageDetails.ivB64, - ); - } - - /** - * Uses a witness key stored alongside the encrypted value to determine if the value is up to date. - * @param value The value being validated - * @param service The service the value is stored under - * @param storageKey The key the value is stored under. The witness key will be stored under storageKey + {@link KEY_WITNESS_SUFFIX} - * @returns Boolean indicating if the value is up to date. - */ - // Uses a witness key stored alongside the encrypted value to determine if the value is up to date. - private async valueUpToDate({ - value, - clientKeyPartB64, - service, - storageKey, - }: { - value: SymmetricCryptoKey; - clientKeyPartB64: string | undefined; - service: string; - storageKey: string; - }): Promise { - const witnessKeyMaterial = this.witnessKeyMaterial(value, clientKeyPartB64); - if (witnessKeyMaterial == null) { - return false; - } - - let witness = null; - try { - witness = await biometrics.getBiometricSecret( - service, - storageKey + KEY_WITNESS_SUFFIX, - witnessKeyMaterial, - ); - } catch (e) { - if (e instanceof Error && e.message === passwords.PASSWORD_NOT_FOUND) { - this.logService.debug( - "[OsBiometricService] Biometric witness key %s not found for service %s, value is not up to date.", - storageKey + KEY_WITNESS_SUFFIX, - service, - ); - } else { - this.logService.error( - "[OsBiometricService] Error retrieving witness key, assuming value is not up to date.", - e, - ); - } - return false; - } - - if (witness === WITNESS_VALUE) { - return true; - } - - return false; - } - - /** Derives a witness key from a symmetric key being stored for biometric protection */ - private witnessKeyMaterial( - symmetricKey: SymmetricCryptoKey, - clientKeyPartB64: string | undefined, - ): biometrics.KeyMaterial { - let key = null; - const innerKey = symmetricKey.inner(); - if (innerKey.type === EncryptionType.AesCbc256_HmacSha256_B64) { - key = Utils.fromBufferToB64(innerKey.authenticationKey); - } else { - key = Utils.fromBufferToB64(innerKey.encryptionKey); - } - - const result = { - osKeyPartB64: key, - clientKeyPartB64, - }; - - // napi-rs fails to convert null values - if (result.clientKeyPartB64 == null) { - delete result.clientKeyPartB64; - } - return result; - } - async needsSetup() { return false; } @@ -312,14 +174,9 @@ export default class OsBiometricsServiceWindows implements OsBiometricService { async getOrCreateBiometricEncryptionClientKeyHalf( userId: UserId, key: SymmetricCryptoKey, - ): Promise { - const requireClientKeyHalf = await this.biometricStateService.getRequirePasswordOnStart(userId); - if (!requireClientKeyHalf) { - return null; - } - + ): Promise { if (this.clientKeyHalves.has(userId)) { - return this.clientKeyHalves.get(userId); + return this.clientKeyHalves.get(userId)!; } // Retrieve existing key half if it exists @@ -331,8 +188,8 @@ export default class OsBiometricsServiceWindows implements OsBiometricService { } if (clientKeyHalf == null) { // Set a key half if it doesn't exist - const keyBytes = await this.cryptoFunctionService.randomBytes(32); - const encKey = await this.encryptService.encryptBytes(keyBytes, key); + clientKeyHalf = await this.cryptoFunctionService.randomBytes(32); + const encKey = await this.encryptService.encryptBytes(clientKeyHalf, key); await this.biometricStateService.setEncryptedClientKeyHalf(encKey, userId); } @@ -342,11 +199,6 @@ export default class OsBiometricsServiceWindows implements OsBiometricService { } async getBiometricsFirstUnlockStatusForUser(userId: UserId): Promise { - const requireClientKeyHalf = await this.biometricStateService.getRequirePasswordOnStart(userId); - if (!requireClientKeyHalf) { - return BiometricsStatus.Available; - } - if (this.clientKeyHalves.has(userId)) { return BiometricsStatus.Available; } else { diff --git a/apps/desktop/src/locales/en/messages.json b/apps/desktop/src/locales/en/messages.json index 1ad0dcf308f..e5e6dcb2882 100644 --- a/apps/desktop/src/locales/en/messages.json +++ b/apps/desktop/src/locales/en/messages.json @@ -1813,9 +1813,6 @@ "unlockWithWindowsHello": { "message": "Unlock with Windows Hello" }, - "additionalWindowsHelloSettings": { - "message": "Additional Windows Hello settings" - }, "unlockWithPolkit": { "message": "Unlock with system authentication" }, @@ -1831,12 +1828,6 @@ "touchIdConsentMessage": { "message": "unlock your vault" }, - "autoPromptWindowsHello": { - "message": "Ask for Windows Hello on app start" - }, - "autoPromptPolkit": { - "message": "Ask for system authentication on launch" - }, "autoPromptTouchId": { "message": "Ask for Touch ID on app start" }, @@ -1846,9 +1837,6 @@ "requirePasswordWithoutPinOnStart": { "message": "Require password on app start" }, - "recommendedForSecurity": { - "message": "Recommended for security." - }, "lockWithMasterPassOnRestart1": { "message": "Lock with master password on restart" }, diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index eee5e3c84f2..9b5aa0b31e9 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -201,6 +201,16 @@ export class Main { this.logService, true, ); + + this.windowMain = new WindowMain( + biometricStateService, + this.logService, + this.storageService, + this.desktopSettingsService, + (arg) => this.processDeepLink(arg), + (win) => this.trayMain.setupWindowListeners(win), + ); + this.biometricsService = new MainBiometricsService( this.i18nService, this.windowMain, @@ -211,14 +221,6 @@ export class Main { this.mainCryptoFunctionService, ); - this.windowMain = new WindowMain( - biometricStateService, - this.logService, - this.storageService, - this.desktopSettingsService, - (arg) => this.processDeepLink(arg), - (win) => this.trayMain.setupWindowListeners(win), - ); this.messagingMain = new MessagingMain(this, this.desktopSettingsService); this.updaterMain = new UpdaterMain(this.i18nService, this.windowMain); diff --git a/apps/desktop/tsconfig.json b/apps/desktop/tsconfig.json index 7db3e84e451..70a59ad164c 100644 --- a/apps/desktop/tsconfig.json +++ b/apps/desktop/tsconfig.json @@ -3,5 +3,6 @@ "angularCompilerOptions": { "strictTemplates": true }, - "include": ["src", "../../libs/common/src/key-management/crypto/services/encrypt.worker.ts"] + "include": ["src", "../../libs/common/src/key-management/crypto/services/encrypt.worker.ts"], + "exclude": ["src/**/*.spec.ts"] }