1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-11 13:53:34 +00:00

[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
This commit is contained in:
Thomas Avery
2025-11-13 10:07:13 -06:00
committed by GitHub
parent 42a79e65cf
commit cfe2458935
23 changed files with 488 additions and 237 deletions

View File

@@ -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<KeyService>;
let encryptService!: MockProxy<EncryptService>;
let service!: RotateableKeySetService;
beforeEach(() => {
keyService = mock<KeyService>();
encryptService = mock<EncryptService>();
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);
}

View File

@@ -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 extends SymmetricCryptoKey>(
externalKey: ExternalKey,
): Promise<RotateableKeySet<ExternalKey>> {
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<ExternalKey extends SymmetricCryptoKey>(
keySet: RotateableKeySet<ExternalKey>,
oldUserKey: SymmetricCryptoKey,
newUserKey: SymmetricCryptoKey,
): Promise<RotateableKeySet<ExternalKey>> {
// 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<ExternalKey>(
newEncryptedUserKey,
newEncryptedPublicKey,
keySet.encryptedPrivateKey,
);
return newRotateableKeySet;
}
}

View File

@@ -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";

View File

@@ -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<RotateableKeySetService>;
let webAuthnLoginPrfKeyService!: MockProxy<WebAuthnLoginPrfKeyServiceAbstraction>;
let credentials: MockProxy<CredentialsContainer>;
let keyService: MockProxy<KeyService>;
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<UserVerificationService>();
rotateableKeySetService = mock<RotateableKeySetService>();
webAuthnLoginPrfKeyService = mock<WebAuthnLoginPrfKeyServiceAbstraction>();
keyService = mock<KeyService>();
credentials = mock<CredentialsContainer>();
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"));

View File

@@ -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<PrfKeySet | undefined> {
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<void> {
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;
}),

View File

@@ -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({

View File

@@ -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) {