[PM-14445] TS strict for Key Management Biometrics (#13039)

* PM-14445: TS strict for Key Management Biometrics

* formatting

* callbacks not null expectations

* state nullability expectations updates

* unit tests fix

* secure channel naming, explicit null check on messageId

* revert null for getUser, getGlobal in state.provider.ts

* revert null for getUser, getGlobal in state.provider.ts

apps/desktop/src/key-management/biometrics/main-biometrics-ipc.listener.ts

@@ -32,6 +32,9 @@ export class MainBiometricsIPCListener {
           case BiometricAction.GetStatusForUser:
             return await this.biometricService.getBiometricsStatusForUser(message.userId as UserId);
           case BiometricAction.SetKeyForUser:
+            if (message.key == null) {
+              return;
+            }
             return await this.biometricService.setBiometricProtectedUnlockKeyForUser(
               message.userId as UserId,
               message.key,
@@ -41,6 +44,9 @@ export class MainBiometricsIPCListener {
               message.userId as UserId,
             );
           case BiometricAction.SetClientKeyHalf:
+            if (message.key == null) {
+              return;
+            }
             return await this.biometricService.setClientKeyHalfForUser(
               message.userId as UserId,
               message.key,

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

@@ -25,10 +25,6 @@ export class MainBiometricsService extends DesktopBiometricsService {
     private biometricStateService: BiometricStateService,
   ) {
     super();
-    this.loadOsBiometricService(this.platform);
-  }
-
-  private loadOsBiometricService(platform: NodeJS.Platform) {
     if (platform === "win32") {
       // eslint-disable-next-line
       const OsBiometricsServiceWindows = require("./os-biometrics-windows.service").default;
@@ -117,13 +113,16 @@ export class MainBiometricsService extends DesktopBiometricsService {
   }
 
   async unlockWithBiometricsForUser(userId: UserId): Promise<UserKey | null> {
-    return SymmetricCryptoKey.fromString(
-      await this.osBiometricsService.getBiometricKey(
-        "Bitwarden_biometric",
-        `${userId}_user_biometric`,
-        this.clientKeyHalves.get(userId),
-      ),
-    ) as UserKey;
+    const biometricKey = await this.osBiometricsService.getBiometricKey(
+      "Bitwarden_biometric",
+      `${userId}_user_biometric`,
+      this.clientKeyHalves.get(userId),
+    );
+    if (biometricKey == null) {
+      return null;
+    }
+
+    return SymmetricCryptoKey.fromString(biometricKey) as UserKey;
   }
 
   async setBiometricProtectedUnlockKeyForUser(userId: UserId, value: string): Promise<void> {

apps/desktop/src/key-management/biometrics/os-biometrics-linux.service.ts

@@ -1,5 +1,3 @@
-// FIXME: Update this file to be type safe and remove this and next line
-// @ts-strict-ignore
 import { spawn } from "child_process";
 
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
@@ -138,23 +136,27 @@ export default class OsBiometricsServiceLinux implements OsBiometricService {
 
   // Nulls out key material in order to force a re-derive. This should only be used in getBiometricKey
   // when we want to force a re-derive of the key material.
-  private setIv(iv: string) {
-    this._iv = iv;
+  private setIv(iv?: string) {
+    this._iv = iv ?? null;
     this._osKeyHalf = null;
   }
 
   private async getStorageDetails({
     clientKeyHalfB64,
   }: {
-    clientKeyHalfB64: string;
+    clientKeyHalfB64: string | undefined;
   }): 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 verefication!
+      // 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;
     }
 
+    if (this._iv == null) {
+      throw new Error("Initialization Vector is null");
+    }
+
     return {
       key_material: {
         osKeyPartB64: this._osKeyHalf,

apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.ts

@@ -1,5 +1,3 @@
-// FIXME: Update this file to be type safe and remove this and next line
-// @ts-strict-ignore
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
 import { EncString } from "@bitwarden/common/platform/models/domain/enc-string";
@@ -104,7 +102,7 @@ export default class OsBiometricsServiceWindows implements OsBiometricService {
   private async getStorageDetails({
     clientKeyHalfB64,
   }: {
-    clientKeyHalfB64: string;
+    clientKeyHalfB64: string | undefined;
   }): Promise<{ key_material: biometrics.KeyMaterial; ivB64: string }> {
     if (this._osKeyHalf == null) {
       // Prompts Windows Hello
@@ -113,6 +111,10 @@ export default class OsBiometricsServiceWindows implements OsBiometricService {
       this._iv = keyMaterial.ivB64;
     }
 
+    if (this._iv == null) {
+      throw new Error("Initialization Vector is null");
+    }
+
     const result = {
       key_material: {
         osKeyPartB64: this._osKeyHalf,
@@ -130,8 +132,8 @@ export default class OsBiometricsServiceWindows implements OsBiometricService {
 
   // Nulls out key material in order to force a re-derive. This should only be used in getBiometricKey
   // when we want to force a re-derive of the key material.
-  private setIv(iv: string) {
-    this._iv = iv;
+  private setIv(iv?: string) {
+    this._iv = iv ?? null;
     this._osKeyHalf = null;
   }
 
@@ -149,9 +151,9 @@ export default class OsBiometricsServiceWindows implements OsBiometricService {
     encryptedValue: EncString,
     service: string,
     storageKey: string,
-    clientKeyPartB64: string,
+    clientKeyPartB64: string | undefined,
   ) {
-    if (encryptedValue.iv == null || encryptedValue == null) {
+    if (encryptedValue.iv == null) {
       return;
     }
 
@@ -183,7 +185,7 @@ export default class OsBiometricsServiceWindows implements OsBiometricService {
     storageKey,
   }: {
     value: SymmetricCryptoKey;
-    clientKeyPartB64: string;
+    clientKeyPartB64: string | undefined;
     service: string;
     storageKey: string;
   }): Promise<boolean> {
@@ -214,7 +216,7 @@ export default class OsBiometricsServiceWindows implements OsBiometricService {
   /** Derives a witness key from a symmetric key being stored for biometric protection */
   private witnessKeyMaterial(
     symmetricKey: SymmetricCryptoKey,
-    clientKeyPartB64: string,
+    clientKeyPartB64: string | undefined,
   ): biometrics.KeyMaterial {
     const key = symmetricKey?.macKeyB64 ?? symmetricKey?.keyB64;
 

apps/desktop/src/services/biometric-message-handler.service.spec.ts

@@ -2,7 +2,7 @@ import { NgZone } from "@angular/core";
 import { mock, MockProxy } from "jest-mock-extended";
 import { of } from "rxjs";
 
-import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
+import { AccountInfo, AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
 import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
 import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
@@ -27,7 +27,7 @@ import { BiometricMessageHandlerService } from "./biometric-message-handler.serv
 
 const SomeUser = "SomeUser" as UserId;
 const AnotherUser = "SomeOtherUser" as UserId;
-const accounts = {
+const accounts: Record<UserId, AccountInfo> = {
   [SomeUser]: {
     name: "some user",
     email: "some.user@example.com",
@@ -108,6 +108,30 @@ describe("BiometricMessageHandlerService", () => {
   });
 
   describe("setup encryption", () => {
+    it("should ignore when public key missing in message", async () => {
+      await service.handleMessage({
+        appId: "appId",
+        message: {
+          command: "setupEncryption",
+          messageId: 0,
+          userId: "unknownUser" as UserId,
+        },
+      });
+      expect((global as any).ipc.platform.nativeMessaging.sendMessage).not.toHaveBeenCalled();
+    });
+
+    it("should ignore when user id missing in message", async () => {
+      await service.handleMessage({
+        appId: "appId",
+        message: {
+          command: "setupEncryption",
+          messageId: 0,
+          publicKey: Utils.fromUtf8ToB64("publicKey"),
+        },
+      });
+      expect((global as any).ipc.platform.nativeMessaging.sendMessage).not.toHaveBeenCalled();
+    });
+
     it("should reject when user is not in app", async () => {
       await service.handleMessage({
         appId: "appId",
@@ -115,6 +139,7 @@ describe("BiometricMessageHandlerService", () => {
           command: "setupEncryption",
           messageId: 0,
           userId: "unknownUser" as UserId,
+          publicKey: Utils.fromUtf8ToB64("publicKey"),
         },
       });
       expect((global as any).ipc.platform.nativeMessaging.sendMessage).toHaveBeenCalledWith({
@@ -362,12 +387,15 @@ describe("BiometricMessageHandlerService", () => {
       // always reload when another user is active than the requested one
       [SomeUser, AuthenticationStatus.Unlocked, AnotherUser, false, true],
       [SomeUser, AuthenticationStatus.Locked, AnotherUser, false, true],
+      // don't reload when no active user
+      [null, AuthenticationStatus.Unlocked, AnotherUser, false, false],
 
       // don't reload in dev mode
       [SomeUser, AuthenticationStatus.Unlocked, SomeUser, true, false],
       [SomeUser, AuthenticationStatus.Locked, SomeUser, true, false],
       [SomeUser, AuthenticationStatus.Unlocked, AnotherUser, true, false],
       [SomeUser, AuthenticationStatus.Locked, AnotherUser, true, false],
+      [null, AuthenticationStatus.Unlocked, AnotherUser, true, false],
     ];
 
     it.each(testCases)(

apps/desktop/src/services/biometric-message-handler.service.ts

@@ -1,5 +1,3 @@
-// FIXME: Update this file to be type safe and remove this and next line
-// @ts-strict-ignore
 import { Injectable, NgZone } from "@angular/core";
 import { combineLatest, concatMap, firstValueFrom, map } from "rxjs";
 
@@ -25,8 +23,7 @@ import {
 } from "@bitwarden/key-management";
 
 import { BrowserSyncVerificationDialogComponent } from "../app/components/browser-sync-verification-dialog.component";
-import { LegacyMessage } from "../models/native-messaging/legacy-message";
-import { LegacyMessageWrapper } from "../models/native-messaging/legacy-message-wrapper";
+import { LegacyMessage, LegacyMessageWrapper } from "../models/native-messaging";
 import { DesktopSettingsService } from "../platform/services/desktop-settings.service";
 
 const MessageValidTimeout = 10 * 1000;
@@ -34,14 +31,14 @@ const HashAlgorithmForAsymmetricEncryption = "sha1";
 
 type ConnectedApp = {
   publicKey: string;
-  sessionSecret: string;
+  sessionSecret: string | null;
   trusted: boolean;
 };
 
 const ConnectedAppPrefix = "connectedApp_";
 
 class ConnectedApps {
-  async get(appId: string): Promise<ConnectedApp> {
+  async get(appId: string): Promise<ConnectedApp | null> {
     if (!(await this.has(appId))) {
       return null;
     }
@@ -112,6 +109,12 @@ export class BiometricMessageHandlerService {
 
     // Request to setup secure encryption
     if ("command" in rawMessage && rawMessage.command === "setupEncryption") {
+      if (rawMessage.publicKey == null || rawMessage.userId == null) {
+        this.logService.warning(
+          "[Native Messaging IPC] Received invalid setupEncryption message. Ignoring.",
+        );
+        return;
+      }
       const remotePublicKey = Utils.fromB64ToArray(rawMessage.publicKey);
 
       // Validate the UserId to ensure we are logged into the same account.
@@ -134,16 +137,18 @@ export class BiometricMessageHandlerService {
         );
       }
 
-      await this.connectedApps.set(appId, {
+      const connectedApp = {
         publicKey: Utils.fromBufferToB64(remotePublicKey),
         sessionSecret: null,
         trusted: false,
-      });
-      await this.secureCommunication(remotePublicKey, appId);
+      } as ConnectedApp;
+      await this.connectedApps.set(appId, connectedApp);
+      await this.secureCommunication(connectedApp, remotePublicKey, appId);
       return;
     }
 
-    if ((await this.connectedApps.get(appId))?.sessionSecret == null) {
+    const sessionSecret = (await this.connectedApps.get(appId))?.sessionSecret;
+    if (sessionSecret == null) {
       this.logService.info(
         "[Native Messaging IPC] Session secret for secure channel is missing. Invalidating encryption...",
       );
@@ -157,7 +162,7 @@ export class BiometricMessageHandlerService {
     const message: LegacyMessage = JSON.parse(
       await this.encryptService.decryptToUtf8(
         rawMessage as EncString,
-        SymmetricCryptoKey.fromString((await this.connectedApps.get(appId)).sessionSecret),
+        SymmetricCryptoKey.fromString(sessionSecret),
       ),
     );
 
@@ -173,7 +178,10 @@ export class BiometricMessageHandlerService {
       return;
     }
 
-    if (Math.abs(message.timestamp - Date.now()) > MessageValidTimeout) {
+    if (
+      message.timestamp == null ||
+      Math.abs(message.timestamp - Date.now()) > MessageValidTimeout
+    ) {
       this.logService.info("[Native Messaging IPC] Received a too old message. Ignoring.");
       return;
     }
@@ -277,11 +285,11 @@ export class BiometricMessageHandlerService {
           return this.send({ command: "biometricUnlock", response: "not unlocked" }, appId);
         }
 
-        const biometricUnlockPromise =
+        const biometricUnlock =
           message.userId == null
-            ? firstValueFrom(this.biometricStateService.biometricUnlockEnabled$)
-            : this.biometricStateService.getBiometricUnlockEnabled(message.userId as UserId);
-        if (!(await biometricUnlockPromise)) {
+            ? await firstValueFrom(this.biometricStateService.biometricUnlockEnabled$)
+            : await this.biometricStateService.getBiometricUnlockEnabled(message.userId as UserId);
+        if (!biometricUnlock) {
           await this.send({ command: "biometricUnlock", response: "not enabled" }, appId);
 
           return this.ngZone.run(() =>
@@ -310,13 +318,13 @@ export class BiometricMessageHandlerService {
 
             const currentlyActiveAccountId = (
               await firstValueFrom(this.accountService.activeAccount$)
-            ).id;
+            )?.id;
             const isCurrentlyActiveAccountUnlocked =
               (await this.authService.getAuthStatus(userId)) == AuthenticationStatus.Unlocked;
 
             // prevent proc reloading an active account, when it is the same as the browser
             if (currentlyActiveAccountId != message.userId || !isCurrentlyActiveAccountUnlocked) {
-              await ipc.platform.reloadProcess();
+              ipc.platform.reloadProcess();
             }
           } else {
             await this.send({ command: "biometricUnlock", response: "canceled" }, appId);
@@ -337,9 +345,14 @@ export class BiometricMessageHandlerService {
   private async send(message: any, appId: string) {
     message.timestamp = Date.now();
 
+    const sessionSecret = (await this.connectedApps.get(appId))?.sessionSecret;
+    if (sessionSecret == null) {
+      throw new Error("Session secret is missing");
+    }
+
     const encrypted = await this.encryptService.encrypt(
       JSON.stringify(message),
-      SymmetricCryptoKey.fromString((await this.connectedApps.get(appId)).sessionSecret),
+      SymmetricCryptoKey.fromString(sessionSecret),
     );
 
     ipc.platform.nativeMessaging.sendMessage({
@@ -349,9 +362,13 @@ export class BiometricMessageHandlerService {
     });
   }
 
-  private async secureCommunication(remotePublicKey: Uint8Array, appId: string) {
+  private async secureCommunication(
+    connectedApp: ConnectedApp,
+    remotePublicKey: Uint8Array,
+    appId: string,
+  ) {
     const secret = await this.cryptoFunctionService.randomBytes(64);
-    const connectedApp = await this.connectedApps.get(appId);
+
     connectedApp.sessionSecret = new SymmetricCryptoKey(secret).keyB64;
     await this.connectedApps.set(appId, connectedApp);
 
@@ -421,11 +438,15 @@ export class BiometricMessageHandlerService {
     }
   }
 
-  /** A process reload after a biometric unlock should happen if the userkey that was used for biometric unlock is for a different user than the
+  /**
+   * A process reload after a biometric unlock should happen if the userkey that was used for biometric unlock is for a different user than the
    * currently active account. The userkey for the active account was in memory anyways. Further, if the desktop app is locked, a reload should occur (since the userkey was not already in memory).
    */
   async processReloadWhenRequired(messageUserId: UserId) {
-    const currentlyActiveAccountId = (await firstValueFrom(this.accountService.activeAccount$)).id;
+    const currentlyActiveAccountId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
+    if (currentlyActiveAccountId == null) {
+      return;
+    }
     const isCurrentlyActiveAccountUnlocked =
       (await firstValueFrom(this.authService.authStatusFor$(currentlyActiveAccountId))) ==
       AuthenticationStatus.Unlocked;