[PM-21147] User key transferred over ipc within desktop app without its prototype (#15047)

* user key transferred over ipc within desktop app without its prototype.

`UserKey` object was transferred over IPC as regular `Object` type and not recreated as `SymmetricCryptoKey` type, losing its original functions and properties. As a result `inner` method did not exist and user key silently failed during decryption of encrypted client key halves during biometric unlock.

* ipc biometrics serializable user key type

* use encrypt service directly for decryption

* moving electron key service to KM

* log error when unlock via biometrics fails with exception in lock component

* bring back tech debt comment

* lock component logging prefix

.github/codecov.yml

@@ -44,6 +44,7 @@ component_management:
     - component_id: key-management-keys
       name: Key Management - Keys
       paths:
+        - apps/desktop/src/key-management/electron-key.service.ts
         - libs/key-management/src/kdf-config.service.ts
         - libs/key-management/src/key.service.ts
         - libs/common/src/key-management/master-password/**

apps/desktop/src/app/services/services.module.ts

@@ -114,10 +114,10 @@ import { DesktopAutofillService } from "../../autofill/services/desktop-autofill
 import { DesktopFido2UserInterfaceService } from "../../autofill/services/desktop-fido2-user-interface.service";
 import { DesktopBiometricsService } from "../../key-management/biometrics/desktop.biometrics.service";
 import { RendererBiometricsService } from "../../key-management/biometrics/renderer-biometrics.service";
+import { ElectronKeyService } from "../../key-management/electron-key.service";
 import { DesktopLockComponentService } from "../../key-management/lock/services/desktop-lock-component.service";
 import { flagEnabled } from "../../platform/flags";
 import { DesktopSettingsService } from "../../platform/services/desktop-settings.service";
-import { ElectronKeyService } from "../../platform/services/electron-key.service";
 import { ElectronLogRendererService } from "../../platform/services/electron-log.renderer.service";
 import {
   ELECTRON_SUPPORTS_SECURE_STORAGE,

apps/desktop/src/key-management/biometrics/renderer-biometrics.service.spec.ts

@@ -1,3 +1,7 @@
+import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
+import { CsprngArray } from "@bitwarden/common/types/csprng";
+import { UserId } from "@bitwarden/common/types/guid";
+import { UserKey } from "@bitwarden/common/types/key";
 import { BiometricsStatus } from "@bitwarden/key-management";
 
 import { RendererBiometricsService } from "./renderer-biometrics.service";
@@ -41,4 +45,34 @@ describe("renderer biometrics service tests", function () {
       expect(result).toBe(expected);
     });
   });
+
+  describe("unlockWithBiometricsForUser", () => {
+    const testUserId = "userId1" as UserId;
+    const service = new RendererBiometricsService();
+
+    it("should return null if no user key is returned", async () => {
+      (global as any).ipc.keyManagement.biometric.unlockWithBiometricsForUser.mockResolvedValue(
+        null,
+      );
+
+      const result = await service.unlockWithBiometricsForUser(testUserId);
+
+      expect(result).toBeNull();
+    });
+
+    it("should return a UserKey object when a user key is returned", async () => {
+      const mockRandomBytes = new Uint8Array(64) as CsprngArray;
+      const mockUserKey = new SymmetricCryptoKey(mockRandomBytes) as UserKey;
+      (global as any).ipc.keyManagement.biometric.unlockWithBiometricsForUser.mockResolvedValue(
+        mockUserKey.toJSON(),
+      );
+
+      const result = await service.unlockWithBiometricsForUser(testUserId);
+
+      expect(result).not.toBeNull();
+      expect(result).toBeInstanceOf(SymmetricCryptoKey);
+      expect(result!.keyB64).toEqual(mockUserKey.keyB64);
+      expect(result!.inner()).toEqual(mockUserKey.inner());
+    });
+  });
 });

apps/desktop/src/key-management/biometrics/renderer-biometrics.service.ts

@@ -1,5 +1,6 @@
 import { Injectable } from "@angular/core";
 
+import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
 import { UserId } from "@bitwarden/common/types/guid";
 import { UserKey } from "@bitwarden/common/types/key";
 import { BiometricsStatus } from "@bitwarden/key-management";
@@ -21,7 +22,12 @@ export class RendererBiometricsService extends DesktopBiometricsService {
   }
 
   async unlockWithBiometricsForUser(userId: UserId): Promise<UserKey | null> {
-    return await ipc.keyManagement.biometric.unlockWithBiometricsForUser(userId);
+    const userKey = await ipc.keyManagement.biometric.unlockWithBiometricsForUser(userId);
+    if (userKey == null) {
+      return null;
+    }
+    // Objects received over IPC lose their prototype, so they must be recreated to restore methods and properties.
+    return SymmetricCryptoKey.fromJSON(userKey) as UserKey;
   }
 
   async getBiometricsStatusForUser(id: UserId): Promise<BiometricsStatus> {

apps/desktop/src/key-management/electron-key.service.spec.ts

@@ -0,0 +1,188 @@
+import { mock } from "jest-mock-extended";
+
+import { PinServiceAbstraction } from "@bitwarden/auth/common";
+import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service";
+import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
+import { FakeMasterPasswordService } from "@bitwarden/common/key-management/master-password/services/fake-master-password.service";
+import { KeyGenerationService } from "@bitwarden/common/platform/abstractions/key-generation.service";
+import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
+import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
+import { Utils } from "@bitwarden/common/platform/misc/utils";
+import { CsprngArray } from "@bitwarden/common/types/csprng";
+import { UserId } from "@bitwarden/common/types/guid";
+import { UserKey } from "@bitwarden/common/types/key";
+import { BiometricStateService, KdfConfigService } from "@bitwarden/key-management";
+
+import {
+  makeEncString,
+  makeStaticByteArray,
+  makeSymmetricCryptoKey,
+  FakeAccountService,
+  mockAccountServiceWith,
+  FakeStateProvider,
+} from "../../../../libs/common/spec";
+
+import { DesktopBiometricsService } from "./biometrics/desktop.biometrics.service";
+import { ElectronKeyService } from "./electron-key.service";
+
+describe("ElectronKeyService", () => {
+  let keyService: ElectronKeyService;
+
+  const pinService = mock<PinServiceAbstraction>();
+  const keyGenerationService = mock<KeyGenerationService>();
+  const cryptoFunctionService = mock<CryptoFunctionService>();
+  const encryptService = mock<EncryptService>();
+  const platformUtilService = mock<PlatformUtilsService>();
+  const logService = mock<LogService>();
+  const stateService = mock<StateService>();
+  const kdfConfigService = mock<KdfConfigService>();
+  const biometricStateService = mock<BiometricStateService>();
+  const biometricService = mock<DesktopBiometricsService>();
+  let stateProvider: FakeStateProvider;
+
+  const mockUserId = Utils.newGuid() as UserId;
+  let accountService: FakeAccountService;
+  let masterPasswordService: FakeMasterPasswordService;
+
+  beforeEach(() => {
+    accountService = mockAccountServiceWith(mockUserId);
+    masterPasswordService = new FakeMasterPasswordService();
+    stateProvider = new FakeStateProvider(accountService);
+
+    keyService = new ElectronKeyService(
+      pinService,
+      masterPasswordService,
+      keyGenerationService,
+      cryptoFunctionService,
+      encryptService,
+      platformUtilService,
+      logService,
+      stateService,
+      accountService,
+      stateProvider,
+      biometricStateService,
+      kdfConfigService,
+      biometricService,
+    );
+  });
+
+  afterEach(() => {
+    jest.resetAllMocks();
+  });
+
+  describe("setUserKey", () => {
+    const userKey = makeSymmetricCryptoKey() as UserKey;
+
+    describe("store biometric key", () => {
+      it("does not set any biometric keys when biometric unlock disabled", async () => {
+        biometricStateService.getBiometricUnlockEnabled.mockResolvedValue(false);
+
+        await keyService.setUserKey(userKey, mockUserId);
+
+        expect(biometricService.setClientKeyHalfForUser).not.toHaveBeenCalled();
+        expect(biometricService.setBiometricProtectedUnlockKeyForUser).not.toHaveBeenCalled();
+        expect(biometricStateService.setEncryptedClientKeyHalf).not.toHaveBeenCalled();
+        expect(biometricStateService.getBiometricUnlockEnabled).toHaveBeenCalledWith(mockUserId);
+      });
+
+      describe("biometric unlock enabled", () => {
+        beforeEach(() => {
+          biometricStateService.getBiometricUnlockEnabled.mockResolvedValue(true);
+        });
+
+        it("sets null biometric client key half and biometric unlock key when require password on start disabled", async () => {
+          biometricStateService.getRequirePasswordOnStart.mockResolvedValue(false);
+
+          await keyService.setUserKey(userKey, mockUserId);
+
+          expect(biometricService.setClientKeyHalfForUser).toHaveBeenCalledWith(mockUserId, null);
+          expect(biometricService.setBiometricProtectedUnlockKeyForUser).toHaveBeenCalledWith(
+            mockUserId,
+            userKey.keyB64,
+          );
+          expect(biometricStateService.setEncryptedClientKeyHalf).not.toHaveBeenCalled();
+          expect(biometricStateService.getBiometricUnlockEnabled).toHaveBeenCalledWith(mockUserId);
+          expect(biometricStateService.getRequirePasswordOnStart).toHaveBeenCalledWith(mockUserId);
+        });
+
+        describe("require password on start enabled", () => {
+          beforeEach(() => {
+            biometricStateService.getRequirePasswordOnStart.mockResolvedValue(true);
+          });
+
+          it("sets new biometric client key half and biometric unlock key when no biometric client key half stored", async () => {
+            const clientKeyHalfBytes = makeStaticByteArray(32);
+            const clientKeyHalf = Utils.fromBufferToUtf8(clientKeyHalfBytes);
+            const encryptedClientKeyHalf = makeEncString();
+            biometricStateService.getEncryptedClientKeyHalf.mockResolvedValue(null);
+            cryptoFunctionService.randomBytes.mockResolvedValue(
+              clientKeyHalfBytes.buffer as CsprngArray,
+            );
+            encryptService.encryptString.mockResolvedValue(encryptedClientKeyHalf);
+
+            await keyService.setUserKey(userKey, mockUserId);
+
+            expect(biometricService.setClientKeyHalfForUser).toHaveBeenCalledWith(
+              mockUserId,
+              clientKeyHalf,
+            );
+            expect(biometricService.setBiometricProtectedUnlockKeyForUser).toHaveBeenCalledWith(
+              mockUserId,
+              userKey.keyB64,
+            );
+            expect(biometricStateService.setEncryptedClientKeyHalf).toHaveBeenCalledWith(
+              encryptedClientKeyHalf,
+              mockUserId,
+            );
+            expect(biometricStateService.getBiometricUnlockEnabled).toHaveBeenCalledWith(
+              mockUserId,
+            );
+            expect(biometricStateService.getRequirePasswordOnStart).toHaveBeenCalledWith(
+              mockUserId,
+            );
+            expect(biometricStateService.getEncryptedClientKeyHalf).toHaveBeenCalledWith(
+              mockUserId,
+            );
+            expect(cryptoFunctionService.randomBytes).toHaveBeenCalledWith(32);
+            expect(encryptService.encryptString).toHaveBeenCalledWith(clientKeyHalf, userKey);
+          });
+
+          it("sets decrypted biometric client key half and biometric unlock key when existing biometric client key half stored", async () => {
+            const encryptedClientKeyHalf = makeEncString();
+            const clientKeyHalf = Utils.fromBufferToUtf8(makeStaticByteArray(32));
+            biometricStateService.getEncryptedClientKeyHalf.mockResolvedValue(
+              encryptedClientKeyHalf,
+            );
+            encryptService.decryptString.mockResolvedValue(clientKeyHalf);
+
+            await keyService.setUserKey(userKey, mockUserId);
+
+            expect(biometricService.setClientKeyHalfForUser).toHaveBeenCalledWith(
+              mockUserId,
+              clientKeyHalf,
+            );
+            expect(biometricService.setBiometricProtectedUnlockKeyForUser).toHaveBeenCalledWith(
+              mockUserId,
+              userKey.keyB64,
+            );
+            expect(biometricStateService.setEncryptedClientKeyHalf).not.toHaveBeenCalled();
+            expect(biometricStateService.getBiometricUnlockEnabled).toHaveBeenCalledWith(
+              mockUserId,
+            );
+            expect(biometricStateService.getRequirePasswordOnStart).toHaveBeenCalledWith(
+              mockUserId,
+            );
+            expect(biometricStateService.getEncryptedClientKeyHalf).toHaveBeenCalledWith(
+              mockUserId,
+            );
+            expect(encryptService.decryptString).toHaveBeenCalledWith(
+              encryptedClientKeyHalf,
+              userKey,
+            );
+          });
+        });
+      });
+    });
+  });
+});

apps/desktop/src/key-management/electron-key.service.ts

@@ -19,8 +19,9 @@ import {
   BiometricStateService,
 } from "@bitwarden/key-management";
 
-import { DesktopBiometricsService } from "../../key-management/biometrics/desktop.biometrics.service";
+import { DesktopBiometricsService } from "./biometrics/desktop.biometrics.service";
 
+// TODO Remove this class once biometric client key half storage is moved https://bitwarden.atlassian.net/browse/PM-22342
 export class ElectronKeyService extends DefaultKeyService {
   constructor(
     pinService: PinServiceAbstraction,
@@ -77,7 +78,6 @@ export class ElectronKeyService extends DefaultKeyService {
 
   private async storeBiometricsProtectedUserKey(userKey: UserKey, userId: UserId): Promise<void> {
     // May resolve to null, in which case no client key have is required
-    // TODO: Move to windows implementation
     const clientEncKeyHalf = await this.getBiometricEncryptionClientKeyHalf(userKey, userId);
     await this.biometricService.setClientKeyHalfForUser(userId, clientEncKeyHalf);
     await this.biometricService.setBiometricProtectedUnlockKeyForUser(userId, userKey.keyB64);
@@ -102,11 +102,16 @@ export class ElectronKeyService extends DefaultKeyService {
     }
 
     // Retrieve existing key half if it exists
-    let clientKeyHalf = await this.biometricStateService
-      .getEncryptedClientKeyHalf(userId)
-      .then((result) => result?.decrypt(null /* user encrypted */, userKey))
-      .then((result) => result as CsprngString);
-    if (clientKeyHalf == null && userKey != null) {
+    let clientKeyHalf: CsprngString | null = null;
+    const encryptedClientKeyHalf =
+      await this.biometricStateService.getEncryptedClientKeyHalf(userId);
+    if (encryptedClientKeyHalf != null) {
+      clientKeyHalf = (await this.encryptService.decryptString(
+        encryptedClientKeyHalf,
+        userKey,
+      )) as CsprngString;
+    }
+    if (clientKeyHalf == null) {
       // Set a key half if it doesn't exist
       const keyBytes = await this.cryptoFunctionService.randomBytes(32);
       clientKeyHalf = Utils.fromBufferToUtf8(keyBytes) as CsprngString;

apps/desktop/src/key-management/preload.ts

@@ -1,4 +1,5 @@
 import { ipcRenderer } from "electron";
+import { Jsonify } from "type-fest";
 
 import { UserKey } from "@bitwarden/common/types/key";
 import { BiometricsStatus } from "@bitwarden/key-management";
@@ -14,7 +15,7 @@ const biometric = {
     ipcRenderer.invoke("biometric", {
       action: BiometricAction.GetStatus,
     } satisfies BiometricMessage),
-  unlockWithBiometricsForUser: (userId: string): Promise<UserKey | null> =>
+  unlockWithBiometricsForUser: (userId: string): Promise<Jsonify<UserKey> | null> =>
     ipcRenderer.invoke("biometric", {
       action: BiometricAction.UnlockForUser,
       userId: userId,

libs/key-management-ui/src/lock/components/lock.component.ts

@@ -388,6 +388,8 @@ export class LockComponent implements OnInit, OnDestroy {
         return;
       }
 
+      this.logService.error("[LockComponent] Failed to unlock via biometrics.", e);
+
       let biometricTranslatedErrorDesc;
 
       if (this.clientType === "browser") {