[PM-4370] Implement PRF key rotation (#9517)

* Add prf key rotation

* Fix tests

* Re-add comment

* Remove encrypted private key from webauthnlogincredentialresponse

* Refactor to use rotateablekeyset

* Move key rotation logic to webauthn-login-admin service

* Fix type error

* Add parameter validation

* Add documentation

* Add input validation

* Add tests
This commit is contained in:
Bernd Schoolmann 2024-06-17 20:47:06 +02:00 committed by GitHub
parent 06410a0633
commit 1970abf723
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 219 additions and 5 deletions

View File

@ -29,4 +29,42 @@ export class RotateableKeySetService {
const encryptedPublicKey = await this.encryptService.encrypt(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.decryptToBytes(
keySet.encryptedPublicKey,
oldUserKey,
);
const newEncryptedPublicKey = await this.encryptService.encrypt(publicKey, newUserKey);
const newEncryptedUserKey = await this.encryptService.rsaEncrypt(newUserKey.key, publicKey);
const newRotateableKeySet = new RotateableKeySet<ExternalKey>(
newEncryptedUserKey,
newEncryptedPublicKey,
keySet.encryptedPrivateKey,
);
return newRotateableKeySet;
}
}

View File

@ -1,4 +1,6 @@
import { RotateableKeySet } from "@bitwarden/auth/common";
import { BaseResponse } from "@bitwarden/common/models/response/base.response";
import { EncString } from "@bitwarden/common/platform/models/domain/enc-string";
import { WebauthnLoginCredentialPrfStatus } from "../../../enums/webauthn-login-credential-prf-status.enum";
@ -9,11 +11,33 @@ export class WebauthnLoginCredentialResponse extends BaseResponse {
id: string;
name: string;
prfStatus: WebauthnLoginCredentialPrfStatus;
encryptedPublicKey?: string;
encryptedUserKey?: string;
constructor(response: unknown) {
super(response);
this.id = this.getResponseProperty("Id");
this.name = this.getResponseProperty("Name");
this.prfStatus = this.getResponseProperty("PrfStatus");
this.encryptedPublicKey = this.getResponseProperty("EncryptedPublicKey");
this.encryptedUserKey = this.getResponseProperty("EncryptedUserKey");
}
getRotateableKeyset(): RotateableKeySet {
if (!EncString.isSerializedEncString(this.encryptedUserKey)) {
throw new Error("Invalid encrypted user key");
}
if (!EncString.isSerializedEncString(this.encryptedPublicKey)) {
throw new Error("Invalid encrypted public key");
}
return new RotateableKeySet(
new EncString(this.encryptedUserKey),
new EncString(this.encryptedPublicKey),
);
}
hasPrfKeyset(): boolean {
return this.encryptedUserKey != null && this.encryptedPublicKey != null;
}
}

View File

@ -45,8 +45,12 @@ export class WebAuthnLoginAdminApiService {
return true;
}
getCredentials(): Promise<ListResponse<WebauthnLoginCredentialResponse>> {
return this.apiService.send("GET", "/webauthn", null, true, true);
async getCredentials(): Promise<ListResponse<WebauthnLoginCredentialResponse>> {
const response = await this.apiService.send("GET", "/webauthn", null, true, true);
return new ListResponse<WebauthnLoginCredentialResponse>(
response,
WebauthnLoginCredentialResponse,
);
}
async deleteCredential(credentialId: string, request: SecretVerificationRequest): Promise<void> {

View File

@ -9,7 +9,8 @@ import { WebAuthnLoginCredentialAssertionView } from "@bitwarden/common/auth/mod
import { WebAuthnLoginAssertionResponseRequest } from "@bitwarden/common/auth/services/webauthn-login/request/webauthn-login-assertion-response.request";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { EncString } from "@bitwarden/common/platform/models/domain/enc-string";
import { PrfKey } from "@bitwarden/common/types/key";
import { makeSymmetricCryptoKey } from "@bitwarden/common/spec";
import { PrfKey, UserKey } from "@bitwarden/common/types/key";
import { CredentialCreateOptionsView } from "../../views/credential-create-options.view";
import { PendingWebauthnLoginCredentialView } from "../../views/pending-webauthn-login-credential.view";
@ -184,6 +185,67 @@ describe("WebauthnAdminService", () => {
}
});
});
describe("rotateCredentials", () => {
it("should throw when old userkey is null", async () => {
const newUserKey = makeSymmetricCryptoKey(64) as UserKey;
try {
await service.rotateWebAuthnKeys(null, newUserKey);
} catch (error) {
expect(error).toEqual(new Error("oldUserKey is required"));
}
});
it("should throw when new userkey is null", async () => {
const oldUserKey = makeSymmetricCryptoKey(64) as UserKey;
try {
await service.rotateWebAuthnKeys(oldUserKey, null);
} catch (error) {
expect(error).toEqual(new Error("newUserKey is required"));
}
});
it("should call rotateKeySet with the correct parameters", async () => {
const oldUserKey = makeSymmetricCryptoKey(64) as UserKey;
const newUserKey = makeSymmetricCryptoKey(64) as UserKey;
const mockEncryptedPublicKey = new EncString("test_encryptedPublicKey");
const mockEncryptedUserKey = new EncString("test_encryptedUserKey");
jest.spyOn(apiService, "getCredentials").mockResolvedValue({
data: [
{
getRotateableKeyset: () =>
new RotateableKeySet<PrfKey>(mockEncryptedUserKey, mockEncryptedPublicKey),
hasPrfKeyset: () => true,
},
],
} as any);
const rotateKeySetMock = jest
.spyOn(rotateableKeySetService, "rotateKeySet")
.mockResolvedValue(
new RotateableKeySet<PrfKey>(mockEncryptedUserKey, mockEncryptedPublicKey),
);
await service.rotateWebAuthnKeys(oldUserKey, newUserKey);
expect(rotateKeySetMock).toHaveBeenCalledWith(
expect.any(RotateableKeySet),
oldUserKey,
newUserKey,
);
});
it("should skip rotation when no prf keyset is available", async () => {
const oldUserKey = makeSymmetricCryptoKey(64) as UserKey;
const newUserKey = makeSymmetricCryptoKey(64) as UserKey;
jest.spyOn(apiService, "getCredentials").mockResolvedValue({
data: [
{
getRotateableKeyset: () =>
new RotateableKeySet<PrfKey>(new EncString("test_encryptedUserKey"), null),
hasPrfKeyset: () => false,
},
],
} as any);
const rotateKeySetMock = jest.spyOn(rotateableKeySetService, "rotateKeySet");
await service.rotateWebAuthnKeys(oldUserKey, newUserKey);
expect(rotateKeySetMock).not.toHaveBeenCalled();
});
});
});
function createCredentialCreateOptions(): CredentialCreateOptionsView {

View File

@ -4,10 +4,12 @@ import { BehaviorSubject, filter, from, map, Observable, shareReplay, switchMap,
import { PrfKeySet } from "@bitwarden/auth/common";
import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
import { WebAuthnLoginPrfCryptoServiceAbstraction } from "@bitwarden/common/auth/abstractions/webauthn/webauthn-login-prf-crypto.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 { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { UserKey } from "@bitwarden/common/types/key";
import { CredentialCreateOptionsView } from "../../views/credential-create-options.view";
import { PendingWebauthnLoginCredentialView } from "../../views/pending-webauthn-login-credential.view";
@ -273,4 +275,44 @@ export class WebauthnLoginAdminService {
private refresh() {
this._refresh$.next();
}
/**
* Creates rotate credential requests for the purpose of user key rotation.
* This works by fetching the current webauthn credentials, filtering out the ones that have a PRF keyset,
* and rotating these using the rotateable key set service.
*
* @param oldUserKey The old user key
* @param newUserKey The new user key
* @returns A promise that returns an array of rotate credential requests when resolved.
*/
async rotateWebAuthnKeys(
oldUserKey: UserKey,
newUserKey: UserKey,
): Promise<WebauthnRotateCredentialRequest[]> {
if (!oldUserKey) {
throw new Error("oldUserKey is required");
}
if (!newUserKey) {
throw new Error("newUserKey is required");
}
return Promise.all(
(await this.apiService.getCredentials()).data
.filter((credential) => credential.hasPrfKeyset())
.map(async (response) => {
const keyset = response.getRotateableKeyset();
const rotatedKeyset = await this.rotateableKeySetService.rotateKeySet(
keyset,
oldUserKey,
newUserKey,
);
const request = new WebauthnRotateCredentialRequest(
response.id,
rotatedKeyset.encryptedPublicKey,
rotatedKeyset.encryptedUserKey,
);
return request;
}),
);
}
}

View File

@ -1,4 +1,5 @@
import { OrganizationUserResetPasswordWithIdRequest } from "@bitwarden/common/admin-console/abstractions/organization-user/requests";
import { WebauthnRotateCredentialRequest } from "@bitwarden/common/auth/models/request/webauthn-rotate-credential.request";
import { SendWithIdRequest } from "@bitwarden/common/src/tools/send/models/request/send-with-id.request";
import { CipherWithIdRequest } from "@bitwarden/common/src/vault/models/request/cipher-with-id.request";
import { FolderWithIdRequest } from "@bitwarden/common/src/vault/models/request/folder-with-id.request";
@ -14,4 +15,5 @@ export class UpdateKeyRequest {
sends: SendWithIdRequest[] = [];
emergencyAccessKeys: EmergencyAccessWithIdRequest[] = [];
resetPasswordKeys: OrganizationUserResetPasswordWithIdRequest[] = [];
webauthnKeys: WebauthnRotateCredentialRequest[] = [];
}

View File

@ -1,5 +1,5 @@
import { mock, MockProxy } from "jest-mock-extended";
import { BehaviorSubject } from "rxjs";
import { BehaviorSubject, of } from "rxjs";
import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction";
import { KdfConfigService } from "@bitwarden/common/auth/abstractions/kdf-config.service";
@ -30,6 +30,7 @@ import {
} from "../../../../../../libs/common/spec/fake-account-service";
import { OrganizationUserResetPasswordService } from "../../admin-console/organizations/members/services/organization-user-reset-password/organization-user-reset-password.service";
import { StateService } from "../../core";
import { WebauthnLoginAdminService } from "../core";
import { EmergencyAccessService } from "../emergency-access";
import { UserKeyRotationApiService } from "./user-key-rotation-api.service";
@ -51,6 +52,7 @@ describe("KeyRotationService", () => {
let mockConfigService: MockProxy<ConfigService>;
let mockKdfConfigService: MockProxy<KdfConfigService>;
let mockSyncService: MockProxy<SyncService>;
let mockWebauthnLoginAdminService: MockProxy<WebauthnLoginAdminService>;
const mockUserId = Utils.newGuid() as UserId;
const mockAccountService: FakeAccountService = mockAccountServiceWith(mockUserId);
@ -71,6 +73,7 @@ describe("KeyRotationService", () => {
mockConfigService = mock<ConfigService>();
mockKdfConfigService = mock<KdfConfigService>();
mockSyncService = mock<SyncService>();
mockWebauthnLoginAdminService = mock<WebauthnLoginAdminService>();
keyRotationService = new UserKeyRotationService(
mockMasterPasswordService,
@ -87,6 +90,7 @@ describe("KeyRotationService", () => {
mockAccountService,
mockKdfConfigService,
mockSyncService,
mockWebauthnLoginAdminService,
);
});
@ -115,6 +119,9 @@ describe("KeyRotationService", () => {
// Mock private key
mockCryptoService.getPrivateKey.mockResolvedValue("MockPrivateKey" as any);
mockCryptoService.userKey$.mockReturnValue(
of(new SymmetricCryptoKey(new Uint8Array(64)) as UserKey),
);
// Mock ciphers
const mockCiphers = [createMockCipher("1", "Cipher 1"), createMockCipher("2", "Cipher 2")];
@ -130,6 +137,8 @@ describe("KeyRotationService", () => {
sends = new BehaviorSubject<Send[]>(mockSends);
mockSendService.sends$ = sends;
mockWebauthnLoginAdminService.rotateWebAuthnKeys.mockResolvedValue([]);
// Mock encryption methods
mockEncryptService.encrypt.mockResolvedValue({
encryptedString: "mockEncryptedData",

View File

@ -18,6 +18,7 @@ import { CipherWithIdRequest } from "@bitwarden/common/vault/models/request/ciph
import { FolderWithIdRequest } from "@bitwarden/common/vault/models/request/folder-with-id.request";
import { OrganizationUserResetPasswordService } from "../../admin-console/organizations/members/services/organization-user-reset-password/organization-user-reset-password.service";
import { WebauthnLoginAdminService } from "../core";
import { EmergencyAccessService } from "../emergency-access";
import { UpdateKeyRequest } from "./request/update-key.request";
@ -40,6 +41,7 @@ export class UserKeyRotationService {
private accountService: AccountService,
private kdfConfigService: KdfConfigService,
private syncService: SyncService,
private webauthnLoginAdminService: WebauthnLoginAdminService,
) {}
/**
@ -70,6 +72,7 @@ export class UserKeyRotationService {
// Set master key again in case it was lost (could be lost on refresh)
const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
const oldUserKey = await firstValueFrom(this.cryptoService.userKey$(userId));
await this.masterPasswordService.setMasterKey(masterKey, userId);
const [newUserKey, newEncUserKey] = await this.cryptoService.makeUserKey(masterKey);
@ -94,6 +97,10 @@ export class UserKeyRotationService {
request.sends = await this.sendService.getRotatedKeys(newUserKey);
request.emergencyAccessKeys = await this.emergencyAccessService.getRotatedKeys(newUserKey);
request.resetPasswordKeys = await this.resetPasswordService.getRotatedKeys(newUserKey);
request.webauthnKeys = await this.webauthnLoginAdminService.rotateWebAuthnKeys(
oldUserKey,
newUserKey,
);
await this.apiService.postUserKeyUpdate(request);

View File

@ -27,7 +27,7 @@ export class RotateableKeySet<ExternalKey extends SymmetricCryptoKey = Symmetric
readonly encryptedPublicKey: EncString,
/** ExternalKey encrypted PrivateKey */
readonly encryptedPrivateKey: EncString,
readonly encryptedPrivateKey?: EncString,
) {}
}

View File

@ -0,0 +1,26 @@
import { EncString } from "@bitwarden/common/platform/models/domain/enc-string";
import { RotateableKeySet } from "../../../../../auth/src/common/models";
export class WebauthnRotateCredentialRequest {
id: string;
encryptedPublicKey: EncString;
encryptedUserKey: EncString;
constructor(id: string, encryptedPublicKey: EncString, encryptedUserKey: EncString) {
this.id = id;
this.encryptedPublicKey = encryptedPublicKey;
this.encryptedUserKey = encryptedUserKey;
}
static fromRotateableKeyset(
id: string,
keyset: RotateableKeySet,
): WebauthnRotateCredentialRequest {
return new WebauthnRotateCredentialRequest(
id,
keyset.encryptedPublicKey,
keyset.encryptedPrivateKey,
);
}
}