From cfe2458935bc7c997e06b423cc385c4ab0623b20 Mon Sep 17 00:00:00 2001 From: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> Date: Thu, 13 Nov 2025 10:07:13 -0600 Subject: [PATCH] [PM-24107] Migrate KM's usage of getUserKey from the key service (#17117) * Remove internal use of getUserKey in the key service * Move ownership of RotateableKeySet and remove usage of getUserKey * Add input validation to createKeySet --- .../rotateable-key-set.service.spec.ts | 57 ------ .../services/rotateable-key-set.service.ts | 85 -------- .../webauthn-login-credential.response.ts | 2 +- .../webauthn-login-admin.service.spec.ts | 37 +++- .../webauthn-login-admin.service.ts | 38 +++- .../create-credential-dialog.component.ts | 10 +- .../enable-encryption-dialog.component.ts | 7 +- ...initial-password.service.implementation.ts | 5 +- .../src/services/jslib-services.module.ts | 7 + libs/auth/src/common/models/domain/index.ts | 1 - .../models/domain/rotateable-key-set.ts | 36 ---- ...authn-login-prf-key.service.abstraction.ts | 2 +- .../webauthn-rotate-credential.request.ts | 5 +- .../response/protected-device.response.ts | 5 +- .../device-trust.service.implementation.ts | 7 +- .../services/device-trust.service.spec.ts | 23 +-- .../keys/models/rotateable-key-set.ts | 34 ++++ .../rotateable-key-set.service.ts | 30 +++ ...default-rotateable-key-set.service.spec.ts | 185 ++++++++++++++++++ .../default-rotateable-key-set.service.ts | 83 ++++++++ .../src/abstractions/key.service.ts | 6 +- libs/key-management/src/key.service.spec.ts | 45 +++++ libs/key-management/src/key.service.ts | 15 +- 23 files changed, 488 insertions(+), 237 deletions(-) delete mode 100644 apps/web/src/app/auth/core/services/rotateable-key-set.service.spec.ts delete mode 100644 apps/web/src/app/auth/core/services/rotateable-key-set.service.ts delete mode 100644 libs/auth/src/common/models/domain/rotateable-key-set.ts create mode 100644 libs/common/src/key-management/keys/models/rotateable-key-set.ts create mode 100644 libs/common/src/key-management/keys/services/abstractions/rotateable-key-set.service.ts create mode 100644 libs/common/src/key-management/keys/services/default-rotateable-key-set.service.spec.ts create mode 100644 libs/common/src/key-management/keys/services/default-rotateable-key-set.service.ts diff --git a/apps/web/src/app/auth/core/services/rotateable-key-set.service.spec.ts b/apps/web/src/app/auth/core/services/rotateable-key-set.service.spec.ts deleted file mode 100644 index 8579c4c1dc8..00000000000 --- a/apps/web/src/app/auth/core/services/rotateable-key-set.service.spec.ts +++ /dev/null @@ -1,57 +0,0 @@ -import { TestBed } from "@angular/core/testing"; -import { mock, MockProxy } from "jest-mock-extended"; - -import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; -import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; -import { KeyService } from "@bitwarden/key-management"; - -import { RotateableKeySetService } from "./rotateable-key-set.service"; - -describe("RotateableKeySetService", () => { - let testBed!: TestBed; - let keyService!: MockProxy; - let encryptService!: MockProxy; - let service!: RotateableKeySetService; - - beforeEach(() => { - keyService = mock(); - encryptService = mock(); - testBed = TestBed.configureTestingModule({ - providers: [ - { provide: KeyService, useValue: keyService }, - { provide: EncryptService, useValue: encryptService }, - ], - }); - service = testBed.inject(RotateableKeySetService); - }); - - describe("createKeySet", () => { - it("should create a new key set", async () => { - const externalKey = createSymmetricKey(); - const userKey = createSymmetricKey(); - const encryptedUserKey = Symbol(); - const encryptedPublicKey = Symbol(); - const encryptedPrivateKey = Symbol(); - keyService.makeKeyPair.mockResolvedValue(["publicKey", encryptedPrivateKey as any]); - keyService.getUserKey.mockResolvedValue({ key: userKey.key } as any); - encryptService.encapsulateKeyUnsigned.mockResolvedValue(encryptedUserKey as any); - encryptService.wrapEncapsulationKey.mockResolvedValue(encryptedPublicKey as any); - - const result = await service.createKeySet(externalKey as any); - - expect(result).toEqual({ - encryptedUserKey, - encryptedPublicKey, - encryptedPrivateKey, - }); - }); - }); -}); - -function createSymmetricKey() { - const key = Utils.fromB64ToArray( - "1h-TuPwSbX5qoX0aVgjmda_Lfq85qAcKssBlXZnPIsQC3HNDGIecunYqXhJnp55QpdXRh-egJiLH3a0wqlVQsQ", - ); - return new SymmetricCryptoKey(key); -} diff --git a/apps/web/src/app/auth/core/services/rotateable-key-set.service.ts b/apps/web/src/app/auth/core/services/rotateable-key-set.service.ts deleted file mode 100644 index 0a150b26ae2..00000000000 --- a/apps/web/src/app/auth/core/services/rotateable-key-set.service.ts +++ /dev/null @@ -1,85 +0,0 @@ -import { inject, Injectable } from "@angular/core"; - -import { RotateableKeySet } from "@bitwarden/auth/common"; -import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; -import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; -import { KeyService } from "@bitwarden/key-management"; - -@Injectable({ providedIn: "root" }) -export class RotateableKeySetService { - private readonly keyService = inject(KeyService); - private readonly encryptService = inject(EncryptService); - - /** - * Create a new rotateable key set for the current user, using the provided external key. - * For more information on rotateable key sets, see {@link RotateableKeySet} - * - * @param externalKey The `ExternalKey` used to encrypt {@link RotateableKeySet.encryptedPrivateKey} - * @returns RotateableKeySet containing the current users `UserKey` - */ - async createKeySet( - externalKey: ExternalKey, - ): Promise> { - const [publicKey, encryptedPrivateKey] = await this.keyService.makeKeyPair(externalKey); - - const userKey = await this.keyService.getUserKey(); - const rawPublicKey = Utils.fromB64ToArray(publicKey); - const encryptedUserKey = await this.encryptService.encapsulateKeyUnsigned( - userKey, - rawPublicKey, - ); - const encryptedPublicKey = await this.encryptService.wrapEncapsulationKey( - rawPublicKey, - userKey, - ); - return new RotateableKeySet(encryptedUserKey, encryptedPublicKey, encryptedPrivateKey); - } - - /** - * Rotates the current user's `UserKey` and updates the provided `RotateableKeySet` with the new keys. - * - * @param keySet The current `RotateableKeySet` for the user - * @returns The updated `RotateableKeySet` with the new `UserKey` - */ - async rotateKeySet( - keySet: RotateableKeySet, - oldUserKey: SymmetricCryptoKey, - newUserKey: SymmetricCryptoKey, - ): Promise> { - // validate parameters - if (!keySet) { - throw new Error("failed to rotate key set: keySet is required"); - } - if (!oldUserKey) { - throw new Error("failed to rotate key set: oldUserKey is required"); - } - if (!newUserKey) { - throw new Error("failed to rotate key set: newUserKey is required"); - } - - const publicKey = await this.encryptService.unwrapEncapsulationKey( - keySet.encryptedPublicKey, - oldUserKey, - ); - if (publicKey == null) { - throw new Error("failed to rotate key set: could not decrypt public key"); - } - const newEncryptedPublicKey = await this.encryptService.wrapEncapsulationKey( - publicKey, - newUserKey, - ); - const newEncryptedUserKey = await this.encryptService.encapsulateKeyUnsigned( - newUserKey, - publicKey, - ); - - const newRotateableKeySet = new RotateableKeySet( - newEncryptedUserKey, - newEncryptedPublicKey, - keySet.encryptedPrivateKey, - ); - - return newRotateableKeySet; - } -} diff --git a/apps/web/src/app/auth/core/services/webauthn-login/response/webauthn-login-credential.response.ts b/apps/web/src/app/auth/core/services/webauthn-login/response/webauthn-login-credential.response.ts index aba5940d752..603e0f2a77d 100644 --- a/apps/web/src/app/auth/core/services/webauthn-login/response/webauthn-login-credential.response.ts +++ b/apps/web/src/app/auth/core/services/webauthn-login/response/webauthn-login-credential.response.ts @@ -1,7 +1,7 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { RotateableKeySet } from "@bitwarden/auth/common"; import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; +import { RotateableKeySet } from "@bitwarden/common/key-management/keys/models/rotateable-key-set"; import { BaseResponse } from "@bitwarden/common/models/response/base.response"; import { WebauthnLoginCredentialPrfStatus } from "../../../enums/webauthn-login-credential-prf-status.enum"; diff --git a/apps/web/src/app/auth/core/services/webauthn-login/webauthn-login-admin.service.spec.ts b/apps/web/src/app/auth/core/services/webauthn-login/webauthn-login-admin.service.spec.ts index 74323773e66..7e263b638e0 100644 --- a/apps/web/src/app/auth/core/services/webauthn-login/webauthn-login-admin.service.spec.ts +++ b/apps/web/src/app/auth/core/services/webauthn-login/webauthn-login-admin.service.spec.ts @@ -3,23 +3,26 @@ import { randomBytes } from "crypto"; import { mock, MockProxy } from "jest-mock-extended"; +import { of } from "rxjs"; -import { RotateableKeySet } from "@bitwarden/auth/common"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { WebAuthnLoginPrfKeyServiceAbstraction } from "@bitwarden/common/auth/abstractions/webauthn/webauthn-login-prf-key.service.abstraction"; import { WebAuthnLoginCredentialAssertionView } from "@bitwarden/common/auth/models/view/webauthn-login/webauthn-login-credential-assertion.view"; import { WebAuthnLoginAssertionResponseRequest } from "@bitwarden/common/auth/services/webauthn-login/request/webauthn-login-assertion-response.request"; import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; +import { RotateableKeySet } from "@bitwarden/common/key-management/keys/models/rotateable-key-set"; +import { RotateableKeySetService } from "@bitwarden/common/key-management/keys/services/abstractions/rotateable-key-set.service"; import { ListResponse } from "@bitwarden/common/models/response/list.response"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { makeEncString, makeSymmetricCryptoKey } from "@bitwarden/common/spec"; import { PrfKey, UserKey } from "@bitwarden/common/types/key"; +import { newGuid } from "@bitwarden/guid"; +import { KeyService } from "@bitwarden/key-management"; import { UserId } from "@bitwarden/user-core"; import { WebauthnLoginCredentialPrfStatus } from "../../enums/webauthn-login-credential-prf-status.enum"; import { CredentialCreateOptionsView } from "../../views/credential-create-options.view"; import { PendingWebauthnLoginCredentialView } from "../../views/pending-webauthn-login-credential.view"; -import { RotateableKeySetService } from "../rotateable-key-set.service"; import { EnableCredentialEncryptionRequest } from "./request/enable-credential-encryption.request"; import { WebauthnLoginCredentialResponse } from "./response/webauthn-login-credential.response"; @@ -32,9 +35,12 @@ describe("WebauthnAdminService", () => { let rotateableKeySetService!: MockProxy; let webAuthnLoginPrfKeyService!: MockProxy; let credentials: MockProxy; + let keyService: MockProxy; let service!: WebauthnLoginAdminService; let originalAuthenticatorAssertionResponse!: AuthenticatorAssertionResponse | any; + const mockUserId = newGuid() as UserId; + const mockUserKey = makeSymmetricCryptoKey(64) as UserKey; beforeAll(() => { // Polyfill missing class @@ -45,12 +51,14 @@ describe("WebauthnAdminService", () => { userVerificationService = mock(); rotateableKeySetService = mock(); webAuthnLoginPrfKeyService = mock(); + keyService = mock(); credentials = mock(); service = new WebauthnLoginAdminService( apiService, userVerificationService, rotateableKeySetService, webAuthnLoginPrfKeyService, + keyService, credentials, ); @@ -58,6 +66,8 @@ describe("WebauthnAdminService", () => { originalAuthenticatorAssertionResponse = global.AuthenticatorAssertionResponse; // Mock the global AuthenticatorAssertionResponse class b/c the class is only available in secure contexts global.AuthenticatorAssertionResponse = MockAuthenticatorAssertionResponse; + + keyService.userKey$.mockReturnValue(of(mockUserKey)); }); beforeEach(() => { @@ -124,7 +134,7 @@ describe("WebauthnAdminService", () => { const request = new EnableCredentialEncryptionRequest(); request.token = assertionOptions.token; request.deviceResponse = assertionOptions.deviceResponse; - request.encryptedUserKey = prfKeySet.encryptedUserKey.encryptedString; + request.encryptedUserKey = prfKeySet.encapsulatedDownstreamKey.encryptedString; request.encryptedPublicKey = prfKeySet.encryptedPublicKey.encryptedString; request.encryptedPrivateKey = prfKeySet.encryptedPrivateKey.encryptedString; @@ -135,10 +145,10 @@ describe("WebauthnAdminService", () => { const updateCredentialMock = jest.spyOn(apiService, "updateCredential").mockResolvedValue(); // Act - await service.enableCredentialEncryption(assertionOptions); + await service.enableCredentialEncryption(assertionOptions, mockUserId); // Assert - expect(createKeySetMock).toHaveBeenCalledWith(assertionOptions.prfKey); + expect(createKeySetMock).toHaveBeenCalledWith(assertionOptions.prfKey, mockUserKey); expect(updateCredentialMock).toHaveBeenCalledWith(request); }); @@ -161,7 +171,7 @@ describe("WebauthnAdminService", () => { // Act try { - await service.enableCredentialEncryption(assertionOptions); + await service.enableCredentialEncryption(assertionOptions, mockUserId); } catch (error) { // Assert expect(error).toEqual(new Error("invalid credential")); @@ -170,6 +180,19 @@ describe("WebauthnAdminService", () => { } }); + test.each([null, undefined, ""])("should throw an error when userId is %p", async (userId) => { + const response = new MockPublicKeyCredential(); + const assertionOptions: WebAuthnLoginCredentialAssertionView = + new WebAuthnLoginCredentialAssertionView( + "enable_credential_encryption_test_token", + new WebAuthnLoginAssertionResponseRequest(response), + {} as PrfKey, + ); + await expect( + service.enableCredentialEncryption(assertionOptions, userId as any), + ).rejects.toThrow("userId is required"); + }); + it("should throw error when WehAuthnLoginCredentialAssertionView is undefined", async () => { // Arrange const assertionOptions: WebAuthnLoginCredentialAssertionView = undefined; @@ -182,7 +205,7 @@ describe("WebauthnAdminService", () => { // Act try { - await service.enableCredentialEncryption(assertionOptions); + await service.enableCredentialEncryption(assertionOptions, mockUserId); } catch (error) { // Assert expect(error).toEqual(new Error("invalid credential")); diff --git a/apps/web/src/app/auth/core/services/webauthn-login/webauthn-login-admin.service.ts b/apps/web/src/app/auth/core/services/webauthn-login/webauthn-login-admin.service.ts index edcf521efb8..7765d01f75c 100644 --- a/apps/web/src/app/auth/core/services/webauthn-login/webauthn-login-admin.service.ts +++ b/apps/web/src/app/auth/core/services/webauthn-login/webauthn-login-admin.service.ts @@ -1,24 +1,34 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import { Injectable, Optional } from "@angular/core"; -import { BehaviorSubject, filter, from, map, Observable, shareReplay, switchMap, tap } from "rxjs"; +import { + BehaviorSubject, + filter, + firstValueFrom, + from, + map, + Observable, + shareReplay, + switchMap, + tap, +} from "rxjs"; -import { PrfKeySet } from "@bitwarden/auth/common"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { WebAuthnLoginPrfKeyServiceAbstraction } from "@bitwarden/common/auth/abstractions/webauthn/webauthn-login-prf-key.service.abstraction"; import { WebauthnRotateCredentialRequest } from "@bitwarden/common/auth/models/request/webauthn-rotate-credential.request"; import { WebAuthnLoginCredentialAssertionOptionsView } from "@bitwarden/common/auth/models/view/webauthn-login/webauthn-login-credential-assertion-options.view"; import { WebAuthnLoginCredentialAssertionView } from "@bitwarden/common/auth/models/view/webauthn-login/webauthn-login-credential-assertion.view"; import { Verification } from "@bitwarden/common/auth/types/verification"; +import { PrfKeySet } from "@bitwarden/common/key-management/keys/models/rotateable-key-set"; +import { RotateableKeySetService } from "@bitwarden/common/key-management/keys/services/abstractions/rotateable-key-set.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { UserId } from "@bitwarden/common/types/guid"; import { UserKey } from "@bitwarden/common/types/key"; -import { UserKeyRotationDataProvider } from "@bitwarden/key-management"; +import { KeyService, UserKeyRotationDataProvider } from "@bitwarden/key-management"; import { CredentialCreateOptionsView } from "../../views/credential-create-options.view"; import { PendingWebauthnLoginCredentialView } from "../../views/pending-webauthn-login-credential.view"; import { WebauthnLoginCredentialView } from "../../views/webauthn-login-credential.view"; -import { RotateableKeySetService } from "../rotateable-key-set.service"; import { EnableCredentialEncryptionRequest } from "./request/enable-credential-encryption.request"; import { SaveCredentialRequest } from "./request/save-credential.request"; @@ -55,6 +65,7 @@ export class WebauthnLoginAdminService private userVerificationService: UserVerificationService, private rotateableKeySetService: RotateableKeySetService, private webAuthnLoginPrfKeyService: WebAuthnLoginPrfKeyServiceAbstraction, + private keyService: KeyService, @Optional() navigatorCredentials?: CredentialsContainer, @Optional() private logService?: LogService, ) { @@ -131,10 +142,12 @@ export class WebauthnLoginAdminService * This will trigger the browsers WebAuthn API to generate a PRF-output. * * @param pendingCredential A credential created using `createCredential`. + * @param userId The target users id. * @returns A key set that can be saved to the server. Undefined is returned if the credential doesn't support PRF. */ async createKeySet( pendingCredential: PendingWebauthnLoginCredentialView, + userId: UserId, ): Promise { const nativeOptions: CredentialRequestOptions = { publicKey: { @@ -166,7 +179,8 @@ export class WebauthnLoginAdminService const symmetricPrfKey = await this.webAuthnLoginPrfKeyService.createSymmetricKeyFromPrf(prfResult); - return await this.rotateableKeySetService.createKeySet(symmetricPrfKey); + const userKey = await firstValueFrom(this.keyService.userKey$(userId)); + return await this.rotateableKeySetService.createKeySet(symmetricPrfKey, userKey); } catch (error) { this.logService?.error(error); return undefined; @@ -190,7 +204,7 @@ export class WebauthnLoginAdminService request.token = credential.createOptions.token; request.name = name; request.supportsPrf = credential.supportsPrf; - request.encryptedUserKey = prfKeySet?.encryptedUserKey.encryptedString; + request.encryptedUserKey = prfKeySet?.encapsulatedDownstreamKey.encryptedString; request.encryptedPublicKey = prfKeySet?.encryptedPublicKey.encryptedString; request.encryptedPrivateKey = prfKeySet?.encryptedPrivateKey.encryptedString; await this.apiService.saveCredential(request); @@ -204,23 +218,31 @@ export class WebauthnLoginAdminService * if there was a problem with the Credential Assertion. * * @param assertionOptions Options received from the server using `getCredentialAssertOptions`. + * @param userId The target users id. * @returns void */ async enableCredentialEncryption( assertionOptions: WebAuthnLoginCredentialAssertionView, + userId: UserId, ): Promise { if (assertionOptions === undefined || assertionOptions?.prfKey === undefined) { throw new Error("invalid credential"); } + if (!userId) { + throw new Error("userId is required"); + } + + const userKey = await firstValueFrom(this.keyService.userKey$(userId)); const prfKeySet: PrfKeySet = await this.rotateableKeySetService.createKeySet( assertionOptions.prfKey, + userKey, ); const request = new EnableCredentialEncryptionRequest(); request.token = assertionOptions.token; request.deviceResponse = assertionOptions.deviceResponse; - request.encryptedUserKey = prfKeySet.encryptedUserKey.encryptedString; + request.encryptedUserKey = prfKeySet.encapsulatedDownstreamKey.encryptedString; request.encryptedPublicKey = prfKeySet.encryptedPublicKey.encryptedString; request.encryptedPrivateKey = prfKeySet.encryptedPrivateKey.encryptedString; await this.apiService.updateCredential(request); @@ -317,7 +339,7 @@ export class WebauthnLoginAdminService const request = new WebauthnRotateCredentialRequest( response.id, rotatedKeyset.encryptedPublicKey, - rotatedKeyset.encryptedUserKey, + rotatedKeyset.encapsulatedDownstreamKey, ); return request; }), diff --git a/apps/web/src/app/auth/settings/webauthn-login-settings/create-credential-dialog/create-credential-dialog.component.ts b/apps/web/src/app/auth/settings/webauthn-login-settings/create-credential-dialog/create-credential-dialog.component.ts index 89b7410baba..8ccf99f1aef 100644 --- a/apps/web/src/app/auth/settings/webauthn-login-settings/create-credential-dialog/create-credential-dialog.component.ts +++ b/apps/web/src/app/auth/settings/webauthn-login-settings/create-credential-dialog/create-credential-dialog.component.ts @@ -8,12 +8,13 @@ import { TwoFactorAuthSecurityKeyFailedIcon, TwoFactorAuthSecurityKeyIcon, } from "@bitwarden/assets/svg"; -import { PrfKeySet } from "@bitwarden/auth/common"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { Verification } from "@bitwarden/common/auth/types/verification"; +import { PrfKeySet } from "@bitwarden/common/key-management/keys/models/rotateable-key-set"; import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; -import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { DialogConfig, DialogRef, DialogService, ToastService } from "@bitwarden/components"; import { WebauthnLoginAdminService } from "../../../core"; @@ -67,10 +68,10 @@ export class CreateCredentialDialogComponent implements OnInit { private formBuilder: FormBuilder, private dialogRef: DialogRef, private webauthnService: WebauthnLoginAdminService, - private platformUtilsService: PlatformUtilsService, private i18nService: I18nService, private logService: LogService, private toastService: ToastService, + private accountService: AccountService, ) {} ngOnInit(): void { @@ -146,13 +147,14 @@ export class CreateCredentialDialogComponent implements OnInit { if (this.formGroup.controls.credentialNaming.controls.name.invalid) { return; } + const userId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); let keySet: PrfKeySet | undefined; if ( this.pendingCredential.supportsPrf && this.formGroup.value.credentialNaming.useForEncryption ) { - keySet = await this.webauthnService.createKeySet(this.pendingCredential); + keySet = await this.webauthnService.createKeySet(this.pendingCredential, userId); if (keySet === undefined) { this.formGroup.controls.credentialNaming.controls.useForEncryption?.setErrors({ diff --git a/apps/web/src/app/auth/settings/webauthn-login-settings/enable-encryption-dialog/enable-encryption-dialog.component.ts b/apps/web/src/app/auth/settings/webauthn-login-settings/enable-encryption-dialog/enable-encryption-dialog.component.ts index 24a711cb5b4..053da609345 100644 --- a/apps/web/src/app/auth/settings/webauthn-login-settings/enable-encryption-dialog/enable-encryption-dialog.component.ts +++ b/apps/web/src/app/auth/settings/webauthn-login-settings/enable-encryption-dialog/enable-encryption-dialog.component.ts @@ -2,11 +2,13 @@ // @ts-strict-ignore import { Component, Inject, OnDestroy, OnInit } from "@angular/core"; import { FormBuilder, Validators } from "@angular/forms"; -import { Subject } from "rxjs"; +import { firstValueFrom, Subject } from "rxjs"; import { takeUntil } from "rxjs/operators"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { WebAuthnLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/webauthn/webauthn-login.service.abstraction"; import { WebAuthnLoginCredentialAssertionOptionsView } from "@bitwarden/common/auth/models/view/webauthn-login/webauthn-login-credential-assertion-options.view"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { Verification } from "@bitwarden/common/auth/types/verification"; import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; import { DIALOG_DATA, DialogConfig, DialogRef } from "@bitwarden/components"; @@ -47,6 +49,7 @@ export class EnableEncryptionDialogComponent implements OnInit, OnDestroy { private dialogRef: DialogRef, private webauthnService: WebauthnLoginAdminService, private webauthnLoginService: WebAuthnLoginServiceAbstraction, + private accountService: AccountService, ) {} ngOnInit(): void { @@ -60,6 +63,7 @@ export class EnableEncryptionDialogComponent implements OnInit, OnDestroy { if (this.credential === undefined) { return; } + const userId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); this.dialogRef.disableClose = true; try { @@ -68,6 +72,7 @@ export class EnableEncryptionDialogComponent implements OnInit, OnDestroy { ); await this.webauthnService.enableCredentialEncryption( await this.webauthnLoginService.assertCredential(this.credentialOptions), + userId, ); } catch (error) { if (error instanceof ErrorResponse && error.statusCode === 400) { diff --git a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts index 01e87cae0ed..dcd4fd93cba 100644 --- a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts +++ b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts @@ -182,7 +182,10 @@ export class DefaultSetInitialPasswordService implements SetInitialPasswordServi if (userKey == null) { masterKeyEncryptedUserKey = await this.keyService.makeUserKey(masterKey); } else { - masterKeyEncryptedUserKey = await this.keyService.encryptUserKeyWithMasterKey(masterKey); + masterKeyEncryptedUserKey = await this.keyService.encryptUserKeyWithMasterKey( + masterKey, + userKey, + ); } return masterKeyEncryptedUserKey; diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index a608b290f09..97aa1869575 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -181,7 +181,9 @@ import { ChangeKdfService } from "@bitwarden/common/key-management/kdf/change-kd 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 { KeyApiService } from "@bitwarden/common/key-management/keys/services/abstractions/key-api-service.abstraction"; +import { RotateableKeySetService } from "@bitwarden/common/key-management/keys/services/abstractions/rotateable-key-set.service"; import { DefaultKeyApiService } from "@bitwarden/common/key-management/keys/services/default-key-api-service.service"; +import { DefaultRotateableKeySetService } from "@bitwarden/common/key-management/keys/services/default-rotateable-key-set.service"; import { MasterPasswordUnlockService } from "@bitwarden/common/key-management/master-password/abstractions/master-password-unlock.service"; import { InternalMasterPasswordServiceAbstraction, @@ -1738,6 +1740,11 @@ const safeProviders: SafeProvider[] = [ ConfigService, ], }), + safeProvider({ + provide: RotateableKeySetService, + useClass: DefaultRotateableKeySetService, + deps: [KeyService, EncryptService], + }), safeProvider({ provide: NewDeviceVerificationComponentService, useClass: DefaultNewDeviceVerificationComponentService, diff --git a/libs/auth/src/common/models/domain/index.ts b/libs/auth/src/common/models/domain/index.ts index b8b83711a4a..cebfa847569 100644 --- a/libs/auth/src/common/models/domain/index.ts +++ b/libs/auth/src/common/models/domain/index.ts @@ -1,3 +1,2 @@ -export * from "./rotateable-key-set"; export * from "./login-credentials"; export * from "./user-decryption-options"; diff --git a/libs/auth/src/common/models/domain/rotateable-key-set.ts b/libs/auth/src/common/models/domain/rotateable-key-set.ts deleted file mode 100644 index 7578a4e9754..00000000000 --- a/libs/auth/src/common/models/domain/rotateable-key-set.ts +++ /dev/null @@ -1,36 +0,0 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore -import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; -import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; -import { PrfKey } from "@bitwarden/common/types/key"; - -declare const tag: unique symbol; - -/** - * A set of keys where a `UserKey` is protected by an encrypted public/private key-pair. - * The `UserKey` is used to encrypt/decrypt data, while the public/private key-pair is - * used to rotate the `UserKey`. - * - * The `PrivateKey` is protected by an `ExternalKey`, such as a `DeviceKey`, or `PrfKey`, - * and the `PublicKey` is protected by the `UserKey`. This setup allows: - * - * - Access to `UserKey` by knowing the `ExternalKey` - * - Rotation to a `NewUserKey` by knowing the current `UserKey`, - * without needing access to the `ExternalKey` - */ -export class RotateableKeySet { - private readonly [tag]: ExternalKey; - - constructor( - /** PublicKey encrypted UserKey */ - readonly encryptedUserKey: EncString, - - /** UserKey encrypted PublicKey */ - readonly encryptedPublicKey: EncString, - - /** ExternalKey encrypted PrivateKey */ - readonly encryptedPrivateKey?: EncString, - ) {} -} - -export type PrfKeySet = RotateableKeySet; diff --git a/libs/common/src/auth/abstractions/webauthn/webauthn-login-prf-key.service.abstraction.ts b/libs/common/src/auth/abstractions/webauthn/webauthn-login-prf-key.service.abstraction.ts index d47b7ccbcef..191bcaf44a8 100644 --- a/libs/common/src/auth/abstractions/webauthn/webauthn-login-prf-key.service.abstraction.ts +++ b/libs/common/src/auth/abstractions/webauthn/webauthn-login-prf-key.service.abstraction.ts @@ -11,7 +11,7 @@ export abstract class WebAuthnLoginPrfKeyServiceAbstraction { /** * Create a symmetric key from the PRF-output by stretching it. - * This should be used as `ExternalKey` with `RotateableKeySet`. + * This should be used as `UpstreamKey` with `RotateableKeySet`. */ abstract createSymmetricKeyFromPrf(prf: ArrayBuffer): Promise; } diff --git a/libs/common/src/auth/models/request/webauthn-rotate-credential.request.ts b/libs/common/src/auth/models/request/webauthn-rotate-credential.request.ts index 2d32d3c21ee..7b831917815 100644 --- a/libs/common/src/auth/models/request/webauthn-rotate-credential.request.ts +++ b/libs/common/src/auth/models/request/webauthn-rotate-credential.request.ts @@ -1,10 +1,7 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore - -// FIXME: remove `src` and fix import -// eslint-disable-next-line no-restricted-imports -import { RotateableKeySet } from "../../../../../auth/src/common/models"; import { EncString } from "../../../key-management/crypto/models/enc-string"; +import { RotateableKeySet } from "../../../key-management/keys/models/rotateable-key-set"; export class WebauthnRotateCredentialRequest { id: string; diff --git a/libs/common/src/auth/models/response/protected-device.response.ts b/libs/common/src/auth/models/response/protected-device.response.ts index 1c69db8f358..77a077ac1af 100644 --- a/libs/common/src/auth/models/response/protected-device.response.ts +++ b/libs/common/src/auth/models/response/protected-device.response.ts @@ -2,12 +2,9 @@ // @ts-strict-ignore import { Jsonify } from "type-fest"; -// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. -// eslint-disable-next-line no-restricted-imports -import { RotateableKeySet } from "@bitwarden/auth/common"; - import { DeviceType } from "../../../enums"; import { EncString } from "../../../key-management/crypto/models/enc-string"; +import { RotateableKeySet } from "../../../key-management/keys/models/rotateable-key-set"; import { BaseResponse } from "../../../models/response/base.response"; export class ProtectedDeviceResponse extends BaseResponse { diff --git a/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts b/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts index 58a2c680afa..aa14c7f0c4f 100644 --- a/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts +++ b/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts @@ -4,7 +4,7 @@ import { firstValueFrom, map, Observable, Subject } from "rxjs"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports -import { RotateableKeySet, UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; +import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports import { KeyService } from "@bitwarden/key-management"; @@ -33,6 +33,7 @@ import { KeyGenerationService } from "../../crypto"; import { CryptoFunctionService } from "../../crypto/abstractions/crypto-function.service"; import { EncryptService } from "../../crypto/abstractions/encrypt.service"; import { EncString } from "../../crypto/models/enc-string"; +import { RotateableKeySet } from "../../keys/models/rotateable-key-set"; import { DeviceTrustServiceAbstraction } from "../abstractions/device-trust.service.abstraction"; /** Uses disk storage so that the device key can persist after log out and tab removal. */ @@ -145,7 +146,7 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { } // Attempt to get user key - const userKey: UserKey = await this.keyService.getUserKey(userId); + const userKey = await firstValueFrom(this.keyService.userKey$(userId)); // If user key is not found, throw error if (!userKey) { @@ -240,7 +241,7 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { const request = new OtherDeviceKeysUpdateRequest(); request.encryptedPublicKey = newRotateableKeySet.encryptedPublicKey.encryptedString; - request.encryptedUserKey = newRotateableKeySet.encryptedUserKey.encryptedString; + request.encryptedUserKey = newRotateableKeySet.encapsulatedDownstreamKey.encryptedString; request.deviceId = device.id; return request; }) diff --git a/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts b/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts index 7ed28518012..e735295f42b 100644 --- a/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts +++ b/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts @@ -366,7 +366,6 @@ describe("deviceTrustService", () => { let makeDeviceKeySpy: jest.SpyInstance; let rsaGenerateKeyPairSpy: jest.SpyInstance; - let cryptoSvcGetUserKeySpy: jest.SpyInstance; let cryptoSvcRsaEncryptSpy: jest.SpyInstance; let encryptServiceWrapDecapsulationKeySpy: jest.SpyInstance; let encryptServiceWrapEncapsulationKeySpy: jest.SpyInstance; @@ -402,6 +401,8 @@ describe("deviceTrustService", () => { "mockDeviceKeyEncryptedDevicePrivateKey", ); + keyService.userKey$.mockReturnValue(of(mockUserKey)); + // TypeScript will allow calling private methods if the object is of type 'any' makeDeviceKeySpy = jest .spyOn(deviceTrustService as any, "makeDeviceKey") @@ -411,10 +412,6 @@ describe("deviceTrustService", () => { .spyOn(cryptoFunctionService, "rsaGenerateKeyPair") .mockResolvedValue(mockDeviceRsaKeyPair); - cryptoSvcGetUserKeySpy = jest - .spyOn(keyService, "getUserKey") - .mockResolvedValue(mockUserKey); - cryptoSvcRsaEncryptSpy = jest .spyOn(encryptService, "encapsulateKeyUnsigned") .mockResolvedValue(mockDevicePublicKeyEncryptedUserKey); @@ -448,7 +445,7 @@ describe("deviceTrustService", () => { expect(makeDeviceKeySpy).toHaveBeenCalledTimes(1); expect(rsaGenerateKeyPairSpy).toHaveBeenCalledTimes(1); - expect(cryptoSvcGetUserKeySpy).toHaveBeenCalledTimes(1); + expect(keyService.userKey$).toHaveBeenCalledTimes(1); expect(cryptoSvcRsaEncryptSpy).toHaveBeenCalledTimes(1); @@ -473,18 +470,13 @@ describe("deviceTrustService", () => { }); it("throws specific error if user key is not found", async () => { - // setup the spy to return null - cryptoSvcGetUserKeySpy.mockResolvedValue(null); + keyService.userKey$.mockReturnValueOnce(of(null)); // check if the expected error is thrown await expect(deviceTrustService.trustDevice(mockUserId)).rejects.toThrow( "User symmetric key not found", ); - // reset the spy - cryptoSvcGetUserKeySpy.mockReset(); - - // setup the spy to return undefined - cryptoSvcGetUserKeySpy.mockResolvedValue(undefined); + keyService.userKey$.mockReturnValueOnce(of(undefined)); // check if the expected error is thrown await expect(deviceTrustService.trustDevice(mockUserId)).rejects.toThrow( "User symmetric key not found", @@ -502,11 +494,6 @@ describe("deviceTrustService", () => { spy: () => rsaGenerateKeyPairSpy, errorText: "rsaGenerateKeyPair error", }, - { - method: "getUserKey", - spy: () => cryptoSvcGetUserKeySpy, - errorText: "getUserKey error", - }, { method: "rsaEncrypt", spy: () => cryptoSvcRsaEncryptSpy, diff --git a/libs/common/src/key-management/keys/models/rotateable-key-set.ts b/libs/common/src/key-management/keys/models/rotateable-key-set.ts new file mode 100644 index 00000000000..630fa2eebba --- /dev/null +++ b/libs/common/src/key-management/keys/models/rotateable-key-set.ts @@ -0,0 +1,34 @@ +import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; +import { PrfKey } from "../../../types/key"; +import { EncString } from "../../crypto/models/enc-string"; + +declare const tag: unique symbol; + +/** + * A set of keys where a symmetric `DownstreamKey` is protected by an encrypted public/private key-pair. + * The `DownstreamKey` is used to encrypt/decrypt data, while the public/private key-pair is + * used to rotate the `DownstreamKey`. + * + * The `PrivateKey` is protected by an `UpstreamKey`, such as a `DeviceKey`, or `PrfKey`, + * and the `PublicKey` is protected by the `DownstreamKey`. This setup allows: + * + * - Access to `DownstreamKey` by knowing the `UpstreamKey` + * - Rotation to a new `DownstreamKey` by knowing the current `DownstreamKey`, + * without needing access to the `UpstreamKey` + */ +export class RotateableKeySet { + private readonly [tag]!: UpstreamKey; + + constructor( + /** `DownstreamKey` protected by publicKey */ + readonly encapsulatedDownstreamKey: EncString, + + /** DownstreamKey encrypted PublicKey */ + readonly encryptedPublicKey: EncString, + + /** UpstreamKey encrypted PrivateKey */ + readonly encryptedPrivateKey?: EncString, + ) {} +} + +export type PrfKeySet = RotateableKeySet; diff --git a/libs/common/src/key-management/keys/services/abstractions/rotateable-key-set.service.ts b/libs/common/src/key-management/keys/services/abstractions/rotateable-key-set.service.ts new file mode 100644 index 00000000000..bdb3d56259d --- /dev/null +++ b/libs/common/src/key-management/keys/services/abstractions/rotateable-key-set.service.ts @@ -0,0 +1,30 @@ +import { SymmetricCryptoKey } from "../../../../platform/models/domain/symmetric-crypto-key"; +import { RotateableKeySet } from "../../models/rotateable-key-set"; + +export abstract class RotateableKeySetService { + /** + * Create a new rotatable key set for the provided downstreamKey, using the provided upstream key. + * For more information on rotatable key sets, see {@link RotateableKeySet} + * @param upstreamKey The `UpstreamKey` used to encrypt {@link RotateableKeySet.encryptedPrivateKey} + * @param downstreamKey The symmetric key to be contained within the `RotateableKeySet`. + * @returns RotateableKeySet containing the provided symmetric downstreamKey. + */ + abstract createKeySet( + upstreamKey: UpstreamKey, + downstreamKey: SymmetricCryptoKey, + ): Promise>; + + /** + * Rotates the provided `RotateableKeySet` with the new key. + * + * @param keySet The current `RotateableKeySet` to be rotated. + * @param oldDownstreamKey The current downstreamKey used to decrypt the `PublicKey`. + * @param newDownstreamKey The new downstreamKey to encrypt the `PublicKey`. + * @returns The updated `RotateableKeySet` that contains the new downstreamKey. + */ + abstract rotateKeySet( + keySet: RotateableKeySet, + oldDownstreamKey: SymmetricCryptoKey, + newDownstreamKey: SymmetricCryptoKey, + ): Promise>; +} diff --git a/libs/common/src/key-management/keys/services/default-rotateable-key-set.service.spec.ts b/libs/common/src/key-management/keys/services/default-rotateable-key-set.service.spec.ts new file mode 100644 index 00000000000..1880bd18b2a --- /dev/null +++ b/libs/common/src/key-management/keys/services/default-rotateable-key-set.service.spec.ts @@ -0,0 +1,185 @@ +import { mock, MockProxy } from "jest-mock-extended"; + +// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. +// eslint-disable-next-line no-restricted-imports +import { KeyService } from "@bitwarden/key-management"; + +import { Utils } from "../../../platform/misc/utils"; +import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; +import { EncryptService } from "../../crypto/abstractions/encrypt.service"; +import { EncString } from "../../crypto/models/enc-string"; +import { RotateableKeySet } from "../models/rotateable-key-set"; + +import { DefaultRotateableKeySetService } from "./default-rotateable-key-set.service"; + +describe("DefaultRotateableKeySetService", () => { + let keyService!: MockProxy; + let encryptService!: MockProxy; + let service!: DefaultRotateableKeySetService; + + beforeEach(() => { + keyService = mock(); + encryptService = mock(); + service = new DefaultRotateableKeySetService(keyService, encryptService); + }); + + describe("createKeySet", () => { + test.each([null, undefined])( + "throws error when downstreamKey parameter is %s", + async (downstreamKey) => { + const externalKey = createSymmetricKey(); + await expect(service.createKeySet(externalKey, downstreamKey as any)).rejects.toThrow( + "failed to create key set: downstreamKey is required", + ); + }, + ); + + test.each([null, undefined])( + "throws error when upstreamKey parameter is %s", + async (upstreamKey) => { + const userKey = createSymmetricKey(); + await expect(service.createKeySet(upstreamKey as any, userKey)).rejects.toThrow( + "failed to create key set: upstreamKey is required", + ); + }, + ); + + it("should create a new key set", async () => { + const externalKey = createSymmetricKey(); + const userKey = createSymmetricKey(); + const encryptedUserKey = new EncString("encryptedUserKey"); + const encryptedPublicKey = new EncString("encryptedPublicKey"); + const encryptedPrivateKey = new EncString("encryptedPrivateKey"); + keyService.makeKeyPair.mockResolvedValue(["publicKey", encryptedPrivateKey]); + encryptService.encapsulateKeyUnsigned.mockResolvedValue(encryptedUserKey); + encryptService.wrapEncapsulationKey.mockResolvedValue(encryptedPublicKey); + + const result = await service.createKeySet(externalKey, userKey); + + expect(result).toEqual( + new RotateableKeySet(encryptedUserKey, encryptedPublicKey, encryptedPrivateKey), + ); + expect(keyService.makeKeyPair).toHaveBeenCalledWith(externalKey); + expect(encryptService.encapsulateKeyUnsigned).toHaveBeenCalledWith( + userKey, + Utils.fromB64ToArray("publicKey"), + ); + expect(encryptService.wrapEncapsulationKey).toHaveBeenCalledWith( + Utils.fromB64ToArray("publicKey"), + userKey, + ); + }); + }); + + describe("rotateKeySet", () => { + const keySet = new RotateableKeySet( + new EncString("encUserKey"), + new EncString("encPublicKey"), + new EncString("encPrivateKey"), + ); + const dataValidationTests = [ + { + keySet: null as any as RotateableKeySet, + oldDownstreamKey: createSymmetricKey(), + newDownstreamKey: createSymmetricKey(), + expectedError: "failed to rotate key set: keySet is required", + }, + { + keySet: undefined as any as RotateableKeySet, + oldDownstreamKey: createSymmetricKey(), + newDownstreamKey: createSymmetricKey(), + expectedError: "failed to rotate key set: keySet is required", + }, + { + keySet: keySet, + oldDownstreamKey: null, + newDownstreamKey: createSymmetricKey(), + expectedError: "failed to rotate key set: oldDownstreamKey is required", + }, + { + keySet: keySet, + oldDownstreamKey: undefined, + newDownstreamKey: createSymmetricKey(), + expectedError: "failed to rotate key set: oldDownstreamKey is required", + }, + { + keySet: keySet, + oldDownstreamKey: createSymmetricKey(), + newDownstreamKey: null, + expectedError: "failed to rotate key set: newDownstreamKey is required", + }, + { + keySet: keySet, + oldDownstreamKey: createSymmetricKey(), + newDownstreamKey: undefined, + expectedError: "failed to rotate key set: newDownstreamKey is required", + }, + ]; + + test.each(dataValidationTests)( + "should throw error when required parameter is missing", + async ({ keySet, oldDownstreamKey, newDownstreamKey, expectedError }) => { + await expect( + service.rotateKeySet(keySet, oldDownstreamKey as any, newDownstreamKey as any), + ).rejects.toThrow(expectedError); + }, + ); + + it("throws an error if the public key cannot be decrypted", async () => { + const oldDownstreamKey = createSymmetricKey(); + const newDownstreamKey = createSymmetricKey(); + + encryptService.unwrapEncapsulationKey.mockResolvedValue(null as any); + + await expect( + service.rotateKeySet(keySet, oldDownstreamKey, newDownstreamKey), + ).rejects.toThrow("failed to rotate key set: could not decrypt public key"); + + expect(encryptService.unwrapEncapsulationKey).toHaveBeenCalledWith( + keySet.encryptedPublicKey, + oldDownstreamKey, + ); + + expect(encryptService.wrapEncapsulationKey).not.toHaveBeenCalled(); + expect(encryptService.encapsulateKeyUnsigned).not.toHaveBeenCalled(); + }); + + it("rotates the key set", async () => { + const oldDownstreamKey = createSymmetricKey(); + const newDownstreamKey = new SymmetricCryptoKey(new Uint8Array(64)); + const publicKey = Utils.fromB64ToArray("decryptedPublicKey"); + const newEncryptedPublicKey = new EncString("newEncPublicKey"); + const newEncryptedRotateableKey = new EncString("newEncUserKey"); + + encryptService.unwrapEncapsulationKey.mockResolvedValue(publicKey); + encryptService.wrapEncapsulationKey.mockResolvedValue(newEncryptedPublicKey); + encryptService.encapsulateKeyUnsigned.mockResolvedValue(newEncryptedRotateableKey); + + const result = await service.rotateKeySet(keySet, oldDownstreamKey, newDownstreamKey); + + expect(result).toEqual( + new RotateableKeySet( + newEncryptedRotateableKey, + newEncryptedPublicKey, + keySet.encryptedPrivateKey, + ), + ); + expect(encryptService.unwrapEncapsulationKey).toHaveBeenCalledWith( + keySet.encryptedPublicKey, + oldDownstreamKey, + ); + expect(encryptService.wrapEncapsulationKey).toHaveBeenCalledWith(publicKey, newDownstreamKey); + expect(encryptService.encapsulateKeyUnsigned).toHaveBeenCalledWith( + newDownstreamKey, + publicKey, + ); + }); + }); +}); + +function createSymmetricKey() { + const key = Utils.fromB64ToArray( + "1h-TuPwSbX5qoX0aVgjmda_Lfq85qAcKssBlXZnPIsQC3HNDGIecunYqXhJnp55QpdXRh-egJiLH3a0wqlVQsQ", + ); + return new SymmetricCryptoKey(key); +} diff --git a/libs/common/src/key-management/keys/services/default-rotateable-key-set.service.ts b/libs/common/src/key-management/keys/services/default-rotateable-key-set.service.ts new file mode 100644 index 00000000000..3e568cb2aee --- /dev/null +++ b/libs/common/src/key-management/keys/services/default-rotateable-key-set.service.ts @@ -0,0 +1,83 @@ +// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. +// eslint-disable-next-line no-restricted-imports +import { KeyService } from "@bitwarden/key-management"; + +import { Utils } from "../../../platform/misc/utils"; +import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; +import { EncryptService } from "../../crypto/abstractions/encrypt.service"; +import { RotateableKeySet } from "../models/rotateable-key-set"; + +import { RotateableKeySetService } from "./abstractions/rotateable-key-set.service"; + +export class DefaultRotateableKeySetService implements RotateableKeySetService { + constructor( + private keyService: KeyService, + private encryptService: EncryptService, + ) {} + + async createKeySet( + upstreamKey: UpstreamKey, + downstreamKey: SymmetricCryptoKey, + ): Promise> { + if (!upstreamKey) { + throw new Error("failed to create key set: upstreamKey is required"); + } + if (!downstreamKey) { + throw new Error("failed to create key set: downstreamKey is required"); + } + + const [publicKey, encryptedPrivateKey] = await this.keyService.makeKeyPair(upstreamKey); + + const rawPublicKey = Utils.fromB64ToArray(publicKey); + const encryptedRotateableKey = await this.encryptService.encapsulateKeyUnsigned( + downstreamKey, + rawPublicKey, + ); + const encryptedPublicKey = await this.encryptService.wrapEncapsulationKey( + rawPublicKey, + downstreamKey, + ); + return new RotateableKeySet(encryptedRotateableKey, encryptedPublicKey, encryptedPrivateKey); + } + + async rotateKeySet( + keySet: RotateableKeySet, + oldDownstreamKey: SymmetricCryptoKey, + newDownstreamKey: SymmetricCryptoKey, + ): Promise> { + // validate parameters + if (!keySet) { + throw new Error("failed to rotate key set: keySet is required"); + } + if (!oldDownstreamKey) { + throw new Error("failed to rotate key set: oldDownstreamKey is required"); + } + if (!newDownstreamKey) { + throw new Error("failed to rotate key set: newDownstreamKey is required"); + } + + const publicKey = await this.encryptService.unwrapEncapsulationKey( + keySet.encryptedPublicKey, + oldDownstreamKey, + ); + if (publicKey == null) { + throw new Error("failed to rotate key set: could not decrypt public key"); + } + const newEncryptedPublicKey = await this.encryptService.wrapEncapsulationKey( + publicKey, + newDownstreamKey, + ); + const newEncryptedRotateableKey = await this.encryptService.encapsulateKeyUnsigned( + newDownstreamKey, + publicKey, + ); + + const newRotateableKeySet = new RotateableKeySet( + newEncryptedRotateableKey, + newEncryptedPublicKey, + keySet.encryptedPrivateKey, + ); + + return newRotateableKeySet; + } +} diff --git a/libs/key-management/src/abstractions/key.service.ts b/libs/key-management/src/abstractions/key.service.ts index a4015f4e615..f86ca922c9e 100644 --- a/libs/key-management/src/abstractions/key.service.ts +++ b/libs/key-management/src/abstractions/key.service.ts @@ -166,16 +166,16 @@ export abstract class KeyService { */ abstract makeMasterKey(password: string, email: string, kdfConfig: KdfConfig): Promise; /** - * Encrypts the existing (or provided) user key with the - * provided master key + * Encrypts the provided user key with the provided master key. * @deprecated Interacting with the master key directly is prohibited. Use a high level function from MasterPasswordService instead. * @param masterKey The user's master key * @param userKey The user key + * @throws Error when userKey or masterKey is null/undefined. * @returns The user key and the master key protected version of it */ abstract encryptUserKeyWithMasterKey( masterKey: MasterKey, - userKey?: UserKey, + userKey: UserKey, ): Promise<[UserKey, EncString]>; /** * Creates a master password hash from the user's master password. Can diff --git a/libs/key-management/src/key.service.spec.ts b/libs/key-management/src/key.service.spec.ts index 5d5340d4900..a5b4eb01c7c 100644 --- a/libs/key-management/src/key.service.spec.ts +++ b/libs/key-management/src/key.service.spec.ts @@ -1357,6 +1357,51 @@ describe("keyService", () => { }); }); + describe("encryptUserKeyWithMasterKey", () => { + const mockMasterKey = makeSymmetricCryptoKey(32); + const mockUserKey = makeSymmetricCryptoKey(64); + + test.each([null as unknown as MasterKey, undefined as unknown as MasterKey])( + "throws when the provided master key is %s", + async (key) => { + await expect(keyService.encryptUserKeyWithMasterKey(key, mockUserKey)).rejects.toThrow( + "masterKey is required.", + ); + }, + ); + + test.each([null as unknown as UserKey, undefined as unknown as UserKey])( + "throws when the provided userKey key is %s", + async (key) => { + await expect(keyService.encryptUserKeyWithMasterKey(mockMasterKey, key)).rejects.toThrow( + "userKey is required.", + ); + }, + ); + + it("throws with invalid master key size", async () => { + const invalidMasterKey = new SymmetricCryptoKey(new Uint8Array(78)) as MasterKey; + + await expect( + keyService.encryptUserKeyWithMasterKey(invalidMasterKey, mockUserKey), + ).rejects.toThrow("Invalid key size."); + }); + + it("encrypts the user key with the master key", async () => { + const mockEncryptedUserKey = makeEncString("encryptedUserKey"); + + encryptService.wrapSymmetricKey.mockResolvedValue(mockEncryptedUserKey); + const stretchedMasterKey = new SymmetricCryptoKey(new Uint8Array(64)); + keyGenerationService.stretchKey.mockResolvedValue(stretchedMasterKey); + + const result = await keyService.encryptUserKeyWithMasterKey(mockMasterKey, mockUserKey); + + expect(encryptService.wrapSymmetricKey).toHaveBeenCalledWith(mockUserKey, stretchedMasterKey); + expect(result[0]).toBe(mockUserKey); + expect(result[1]).toBe(mockEncryptedUserKey); + }); + }); + describe("makeKeyPair", () => { test.each([null as unknown as SymmetricCryptoKey, undefined as unknown as SymmetricCryptoKey])( "throws when the provided key is %s", diff --git a/libs/key-management/src/key.service.ts b/libs/key-management/src/key.service.ts index c734a84b239..2701109fde2 100644 --- a/libs/key-management/src/key.service.ts +++ b/libs/key-management/src/key.service.ts @@ -166,6 +166,9 @@ export class DefaultKeyService implements KeyServiceAbstraction { return this.stateProvider.getUserState$(USER_KEY, userId); } + /** + * @deprecated Use {@link userKey$} with a required {@link UserId} instead. + */ async getUserKey(userId?: UserId): Promise { const userKey = await firstValueFrom(this.stateProvider.getUserState$(USER_KEY, userId)); return userKey; @@ -298,9 +301,15 @@ export class DefaultKeyService implements KeyServiceAbstraction { */ async encryptUserKeyWithMasterKey( masterKey: MasterKey, - userKey?: UserKey, + userKey: UserKey, ): Promise<[UserKey, EncString]> { - userKey ||= await this.getUserKey(); + if (masterKey == null) { + throw new Error("masterKey is required."); + } + if (userKey == null) { + throw new Error("userKey is required."); + } + return await this.buildProtectedSymmetricKey(masterKey, userKey); } @@ -630,7 +639,7 @@ export class DefaultKeyService implements KeyServiceAbstraction { } // Verify user key doesn't exist - const existingUserKey = await this.getUserKey(userId); + const existingUserKey = await firstValueFrom(this.userKey$(userId)); if (existingUserKey != null) { this.logService.error("Tried to initialize account with existing user key.");