[BEEEP|PM-32521] Remove compare key hash and move to proof of decryption (#19101)

* Remove compare key hash and move to proof of decryption * Fix cli build * Fix mv2 * Fix provider * Prettier
2026-02-27 01:53:23 +00:00 · 2026-02-25 17:02:04 +01:00
parent 385ba70b5c
commit cf7f9cfc7e
9 changed files with 40 additions and 175 deletions
--- a/apps/browser/src/background/main.background.ts
+++ b/apps/browser/src/background/main.background.ts
@@ -111,7 +111,9 @@ import { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/
 import { DeviceTrustService } from "@bitwarden/common/key-management/device-trust/services/device-trust.service.implementation";
 import { KeyConnectorService as KeyConnectorServiceAbstraction } from "@bitwarden/common/key-management/key-connector/abstractions/key-connector.service";
 import { KeyConnectorService } from "@bitwarden/common/key-management/key-connector/services/key-connector.service";
+import { MasterPasswordUnlockService } from "@bitwarden/common/key-management/master-password/abstractions/master-password-unlock.service";
 import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction";
+import { DefaultMasterPasswordUnlockService } from "@bitwarden/common/key-management/master-password/services/default-master-password-unlock.service";
 import { MasterPasswordService } from "@bitwarden/common/key-management/master-password/services/master-password.service";
 import { PinStateService } from "@bitwarden/common/key-management/pin/pin-state.service.implementation";
 import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction";
@@ -373,6 +375,7 @@ export default class MainBackground {
  keyService: KeyServiceAbstraction;
  cryptoFunctionService: CryptoFunctionServiceAbstraction;
  masterPasswordService: InternalMasterPasswordServiceAbstraction;
+  masterPasswordUnlockService: MasterPasswordUnlockService;
  tokenService: TokenServiceAbstraction;
  appIdService: AppIdServiceAbstraction;
  apiService: ApiServiceAbstraction;
@@ -732,6 +735,12 @@ export default class MainBackground {
      this.accountCryptographicStateService,
    );

+    this.masterPasswordUnlockService = new DefaultMasterPasswordUnlockService(
+      this.masterPasswordService,
+      this.keyService,
+      this.logService,
+    );
+
    const pinStateService = new PinStateService(this.stateProvider);

    this.appIdService = new AppIdService(this.storageService, this.logService);
@@ -1022,6 +1031,7 @@ export default class MainBackground {
      this.pinService,
      this.kdfConfigService,
      this.biometricsService,
+      this.masterPasswordUnlockService,
    );

    this.vaultSettingsService = new VaultSettingsService(
--- a/apps/cli/src/service-container/service-container.ts
+++ b/apps/cli/src/service-container/service-container.ts
@@ -861,6 +861,7 @@ export class ServiceContainer {
      this.pinService,
      this.kdfConfigService,
      new CliBiometricsService(),
+      this.masterPasswordUnlockService,
    );

    const biometricService = new CliBiometricsService();
--- a/libs/angular/src/services/jslib-services.module.ts
+++ b/libs/angular/src/services/jslib-services.module.ts
@@ -1199,6 +1199,7 @@ const safeProviders: SafeProvider[] = [
      PinServiceAbstraction,
      KdfConfigService,
      BiometricsService,
+      MasterPasswordUnlockService,
    ],
  }),
  safeProvider({
--- a/libs/common/src/auth/services/user-verification/user-verification.service.spec.ts
+++ b/libs/common/src/auth/services/user-verification/user-verification.service.spec.ts
@@ -15,6 +15,7 @@ import {
 } from "@bitwarden/key-management";

 import { FakeAccountService, mockAccountServiceWith } from "../../../../spec";
+import { MasterPasswordUnlockService } from "../../../key-management/master-password/abstractions/master-password-unlock.service";
 import { InternalMasterPasswordServiceAbstraction } from "../../../key-management/master-password/abstractions/master-password.service.abstraction";
 import { PinLockType } from "../../../key-management/pin/pin-lock-type";
 import { PinServiceAbstraction } from "../../../key-management/pin/pin.service.abstraction";
@@ -43,6 +44,7 @@ describe("UserVerificationService", () => {
  const vaultTimeoutSettingsService = mock<VaultTimeoutSettingsService>();
  const kdfConfigService = mock<KdfConfigService>();
  const biometricsService = mock<BiometricsService>();
+  const masterPasswordUnlockService = mock<MasterPasswordUnlockService>();

  const mockUserId = Utils.newGuid() as UserId;
  let accountService: FakeAccountService;
@@ -61,6 +63,7 @@ describe("UserVerificationService", () => {
      pinService,
      kdfConfigService,
      biometricsService,
+      masterPasswordUnlockService,
    );
  });

@@ -328,11 +331,10 @@ describe("UserVerificationService", () => {
    describe("client-side verification", () => {
      beforeEach(() => {
        setMasterPasswordAvailability(true);
+        masterPasswordUnlockService.proofOfDecryption.mockResolvedValue(true);
      });

      it("returns if verification is successful", async () => {
-        keyService.compareKeyHash.mockResolvedValueOnce(true);
-
        const result = await sut.verifyUserByMasterPassword(
          {
            type: VerificationType.MasterPassword,
@@ -342,7 +344,10 @@ describe("UserVerificationService", () => {
          "email",
        );

-        expect(keyService.compareKeyHash).toHaveBeenCalled();
+        expect(masterPasswordUnlockService.proofOfDecryption).toHaveBeenCalledWith(
+          "password",
+          mockUserId,
+        );
        expect(masterPasswordService.setMasterKeyHash).toHaveBeenCalledWith(
          "localHash",
          mockUserId,
@@ -356,7 +361,7 @@ describe("UserVerificationService", () => {
      });

      it("throws if verification fails", async () => {
-        keyService.compareKeyHash.mockResolvedValueOnce(false);
+        masterPasswordUnlockService.proofOfDecryption.mockResolvedValueOnce(false);

        await expect(
          sut.verifyUserByMasterPassword(
@@ -369,7 +374,10 @@ describe("UserVerificationService", () => {
          ),
        ).rejects.toThrow("Invalid master password");

-        expect(keyService.compareKeyHash).toHaveBeenCalled();
+        expect(masterPasswordUnlockService.proofOfDecryption).toHaveBeenCalledWith(
+          "password",
+          mockUserId,
+        );
        expect(masterPasswordService.setMasterKeyHash).not.toHaveBeenCalledWith();
        expect(masterPasswordService.setMasterKey).not.toHaveBeenCalledWith();
      });
@@ -401,7 +409,7 @@ describe("UserVerificationService", () => {
          "email",
        );

-        expect(keyService.compareKeyHash).not.toHaveBeenCalled();
+        expect(masterPasswordUnlockService.proofOfDecryption).not.toHaveBeenCalled();
        expect(masterPasswordService.setMasterKeyHash).toHaveBeenCalledWith(
          "localHash",
          mockUserId,
@@ -435,7 +443,7 @@ describe("UserVerificationService", () => {
          ),
        ).rejects.toThrow("Invalid master password");

-        expect(keyService.compareKeyHash).not.toHaveBeenCalled();
+        expect(masterPasswordUnlockService.proofOfDecryption).not.toHaveBeenCalled();
        expect(masterPasswordService.setMasterKeyHash).not.toHaveBeenCalledWith();
        expect(masterPasswordService.setMasterKey).not.toHaveBeenCalledWith();
      });
--- a/libs/common/src/auth/services/user-verification/user-verification.service.ts
+++ b/libs/common/src/auth/services/user-verification/user-verification.service.ts
@@ -14,6 +14,7 @@ import {
  KeyService,
 } from "@bitwarden/key-management";

+import { MasterPasswordUnlockService } from "../../../key-management/master-password/abstractions/master-password-unlock.service";
 import { InternalMasterPasswordServiceAbstraction } from "../../../key-management/master-password/abstractions/master-password.service.abstraction";
 import { PinServiceAbstraction } from "../../../key-management/pin/pin.service.abstraction";
 import { I18nService } from "../../../platform/abstractions/i18n.service";
@@ -54,6 +55,7 @@ export class UserVerificationService implements UserVerificationServiceAbstracti
    private pinService: PinServiceAbstraction,
    private kdfConfigService: KdfConfigService,
    private biometricsService: BiometricsService,
+    private masterPasswordUnlockService: MasterPasswordUnlockService,
  ) {}

  async getAvailableVerificationOptions(
@@ -202,9 +204,8 @@ export class UserVerificationService implements UserVerificationServiceAbstracti
    let policyOptions: MasterPasswordPolicyResponse | null;
    // Client-side verification
    if (await this.hasMasterPasswordAndMasterKeyHash(userId)) {
-      const passwordValid = await this.keyService.compareKeyHash(
+      const passwordValid = await this.masterPasswordUnlockService.proofOfDecryption(
        verification.secret,
-        masterKey,
        userId,
      );
      if (!passwordValid) {
@@ -214,12 +215,13 @@ export class UserVerificationService implements UserVerificationServiceAbstracti
    } else {
      // Server-side verification
      const request = new SecretVerificationRequest();
-      const serverKeyHash = await this.keyService.hashMasterKey(
-        verification.secret,
-        masterKey,
-        HashPurpose.ServerAuthorization,
-      );
-      request.masterPasswordHash = serverKeyHash;
+      const authenticationData =
+        await this.masterPasswordService.makeMasterPasswordAuthenticationData(
+          verification.secret,
+          kdfConfig,
+          await firstValueFrom(this.masterPasswordService.saltForUser$(userId)),
+        );
+      request.authenticateWith(authenticationData);
      try {
        policyOptions = await this.userVerificationApiService.postAccountVerifyPassword(request);
        // FIXME: Remove when updating file. Eslint update
--- a/libs/key-management/src/abstractions/key.service.ts
+++ b/libs/key-management/src/abstractions/key.service.ts
@@ -174,21 +174,6 @@ export abstract class KeyService {
    key: MasterKey,
    hashPurpose?: HashPurpose,
  ): Promise<string>;
-  /**
-   * Compares the provided master password to the stored password hash.
-   * @deprecated Interacting with the master key directly is prohibited. Use a high level function from MasterPasswordService instead.
-   * @param masterPassword The user's master password
-   * @param masterKey The user's master key
-   * @param userId The id of the user to do the operation for.
-   * @throws Error when master key is null/undefined.
-   * @returns True if the derived master password hash matches the stored
-   * key hash, false otherwise.
-   */
-  abstract compareKeyHash(
-    masterPassword: string,
-    masterKey: MasterKey,
-    userId: UserId,
-  ): Promise<boolean>;
  /**
   * Stores the encrypted organization keys and clears any decrypted
   * organization keys currently in memory
--- a/libs/key-management/src/key.service.spec.ts
+++ b/libs/key-management/src/key.service.spec.ts
@@ -843,104 +843,6 @@ describe("keyService", () => {
    );
  });

-  describe("compareKeyHash", () => {
-    type TestCase = {
-      masterKey: MasterKey;
-      masterPassword: string;
-      storedMasterKeyHash: string | null;
-      mockReturnedHash: string;
-      expectedToMatch: boolean;
-    };
-
-    const data: TestCase[] = [
-      {
-        masterKey: makeSymmetricCryptoKey(32),
-        masterPassword: "my_master_password",
-        storedMasterKeyHash: "bXlfaGFzaA==",
-        mockReturnedHash: "bXlfaGFzaA==",
-        expectedToMatch: true,
-      },
-      {
-        masterKey: makeSymmetricCryptoKey(32),
-        masterPassword: null as unknown as string,
-        storedMasterKeyHash: "bXlfaGFzaA==",
-        mockReturnedHash: "bXlfaGFzaA==",
-        expectedToMatch: false,
-      },
-      {
-        masterKey: makeSymmetricCryptoKey(32),
-        masterPassword: null as unknown as string,
-        storedMasterKeyHash: null,
-        mockReturnedHash: "bXlfaGFzaA==",
-        expectedToMatch: false,
-      },
-      {
-        masterKey: makeSymmetricCryptoKey(32),
-        masterPassword: "my_master_password",
-        storedMasterKeyHash: "bXlfaGFzaA==",
-        mockReturnedHash: "zxccbXlfaGFzaA==",
-        expectedToMatch: false,
-      },
-    ];
-
-    it.each(data)(
-      "returns expected match value when calculated hash equals stored hash",
-      async ({
-        masterKey,
-        masterPassword,
-        storedMasterKeyHash,
-        mockReturnedHash,
-        expectedToMatch,
-      }) => {
-        masterPasswordService.masterKeyHashSubject.next(storedMasterKeyHash);
-
-        cryptoFunctionService.pbkdf2
-          .calledWith(masterKey.inner().encryptionKey, masterPassword, "sha256", 2)
-          .mockResolvedValue(Utils.fromB64ToArray(mockReturnedHash));
-
-        const actualDidMatch = await keyService.compareKeyHash(
-          masterPassword,
-          masterKey,
-          mockUserId,
-        );
-
-        expect(actualDidMatch).toBe(expectedToMatch);
-      },
-    );
-
-    test.each([null as unknown as MasterKey, undefined as unknown as MasterKey])(
-      "throws an error if masterKey is %s",
-      async (masterKey) => {
-        await expect(
-          keyService.compareKeyHash("my_master_password", masterKey, mockUserId),
-        ).rejects.toThrow("'masterKey' is required to be non-null.");
-      },
-    );
-
-    test.each([null as unknown as string, undefined as unknown as string])(
-      "returns false when masterPassword is %s",
-      async (masterPassword) => {
-        const result = await keyService.compareKeyHash(
-          masterPassword,
-          makeSymmetricCryptoKey(32),
-          mockUserId,
-        );
-        expect(result).toBe(false);
-      },
-    );
-
-    it("returns false when storedMasterKeyHash is null", async () => {
-      masterPasswordService.masterKeyHashSubject.next(null);
-
-      const result = await keyService.compareKeyHash(
-        "my_master_password",
-        makeSymmetricCryptoKey(32),
-        mockUserId,
-      );
-      expect(result).toBe(false);
-    });
-  });
-
  describe("makeOrgKey", () => {
    const mockUserPublicKey = new Uint8Array(64) as UserPublicKey;
    const shareKey = new SymmetricCryptoKey(new Uint8Array(64));
--- a/libs/key-management/src/key.service.ts
+++ b/libs/key-management/src/key.service.ts
@@ -303,45 +303,6 @@ export class DefaultKeyService implements KeyServiceAbstraction {
    return Utils.fromBufferToB64(hash);
  }

-  async compareKeyHash(
-    masterPassword: string,
-    masterKey: MasterKey,
-    userId: UserId,
-  ): Promise<boolean> {
-    if (masterKey == null) {
-      throw new Error("'masterKey' is required to be non-null.");
-    }
-
-    if (masterPassword == null) {
-      // If they don't give us a master password, we can't hash it, and therefore
-      // it will never match what we have stored.
-      return false;
-    }
-
-    // Retrieve the current password hash
-    const storedPasswordHash = await firstValueFrom(
-      this.masterPasswordService.masterKeyHash$(userId),
-    );
-
-    if (storedPasswordHash == null) {
-      return false;
-    }
-
-    // Hash the key for local use
-    const localKeyHash = await this.hashMasterKey(
-      masterPassword,
-      masterKey,
-      HashPurpose.LocalAuthorization,
-    );
-
-    // Check if the stored hash is already equal to the hash we create locally
-    if (localKeyHash == null || storedPasswordHash !== localKeyHash) {
-      return false;
-    }
-
-    return true;
-  }
-
  async setOrgKeys(
    orgs: ProfileOrganizationResponse[],
    providerOrgs: ProfileProviderOrganizationResponse[],
--- a/libs/vault/src/components/password-reprompt.component.ts
+++ b/libs/vault/src/components/password-reprompt.component.ts
@@ -5,6 +5,7 @@ import { firstValueFrom } from "rxjs";
 import { JslibModule } from "@bitwarden/angular/jslib.module";
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { getUserId } from "@bitwarden/common/auth/services/account.service";
+import { MasterPasswordUnlockService } from "@bitwarden/common/key-management/master-password/abstractions/master-password-unlock.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
 import {
@@ -16,7 +17,6 @@ import {
  IconButtonModule,
  ToastService,
 } from "@bitwarden/components";
-import { KeyService } from "@bitwarden/key-management";

 /**
 * Used to verify the user's Master Password for the "Master Password Re-prompt" feature only.
@@ -43,7 +43,7 @@ export class PasswordRepromptComponent {
  });

  constructor(
-    protected keyService: KeyService,
+    protected masterPasswordUnlockService: MasterPasswordUnlockService,
    protected platformUtilsService: PlatformUtilsService,
    protected i18nService: I18nService,
    protected formBuilder: FormBuilder,
@@ -65,14 +65,9 @@ export class PasswordRepromptComponent {
      throw new Error("An active user is expected while doing password reprompt.");
    }

-    const storedMasterKey = await this.keyService.getOrDeriveMasterKey(
-      this.formGroup.value.masterPassword,
-      userId,
-    );
    if (
-      !(await this.keyService.compareKeyHash(
+      !(await this.masterPasswordUnlockService.proofOfDecryption(
        this.formGroup.value.masterPassword,
-        storedMasterKey,
        userId,
      ))
    ) {