From d4caf6f8f5f714456e3225f9fde23e2e088885fa Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Tue, 28 Jan 2025 15:26:10 +0100 Subject: [PATCH 1/8] Implement key rotation v2 --- .../settings/change-password.component.html | 2 +- .../settings/change-password.component.ts | 124 +++++++------- .../request/account-keys.request.ts | 10 ++ .../master-password-unlock-data.request.ts | 31 ++++ .../rotate-user-account-keys.request.ts | 29 ++++ .../request/unlock-data.request.ts | 26 +++ .../key-rotation/request/userdata.request.ts | 19 +++ .../user-key-rotation-api.service.ts | 11 ++ .../user-key-rotation.service.spec.ts | 139 +++++++++++++-- .../key-rotation/user-key-rotation.service.ts | 159 +++++++++++++++++- .../migrate-legacy-encryption.component.ts | 5 +- .../user-verification.service.spec.ts | 4 + .../user-verification.service.ts | 2 +- libs/common/src/auth/types/verification.ts | 4 + libs/common/src/enums/feature-flag.enum.ts | 2 + .../src/abstractions/key.service.ts | 11 ++ libs/key-management/src/key.service.ts | 15 ++ 17 files changed, 519 insertions(+), 74 deletions(-) create mode 100644 apps/web/src/app/key-management/key-rotation/request/account-keys.request.ts create mode 100644 apps/web/src/app/key-management/key-rotation/request/master-password-unlock-data.request.ts create mode 100644 apps/web/src/app/key-management/key-rotation/request/rotate-user-account-keys.request.ts create mode 100644 apps/web/src/app/key-management/key-rotation/request/unlock-data.request.ts create mode 100644 apps/web/src/app/key-management/key-rotation/request/userdata.request.ts diff --git a/apps/web/src/app/auth/settings/change-password.component.html b/apps/web/src/app/auth/settings/change-password.component.html index 91144fdfc1f..34bb74ee473 100644 --- a/apps/web/src/app/auth/settings/change-password.component.html +++ b/apps/web/src/app/auth/settings/change-password.component.html @@ -121,7 +121,7 @@

{{ "changeMasterPassword" | i18n }}

[(ngModel)]="masterPasswordHint" /> - diff --git a/apps/web/src/app/auth/settings/change-password.component.ts b/apps/web/src/app/auth/settings/change-password.component.ts index ebbc762e5b3..389264f4a9b 100644 --- a/apps/web/src/app/auth/settings/change-password.component.ts +++ b/apps/web/src/app/auth/settings/change-password.component.ts @@ -12,15 +12,12 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { PasswordRequest } from "@bitwarden/common/auth/models/request/password.request"; -import { getUserId } from "@bitwarden/common/auth/services/account.service"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; -import { HashPurpose } from "@bitwarden/common/platform/enums"; -import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; -import { UserId } from "@bitwarden/common/types/guid"; -import { MasterKey, UserKey } from "@bitwarden/common/types/key"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { DialogService, ToastService } from "@bitwarden/components"; @@ -37,11 +34,13 @@ export class ChangePasswordComponent extends BaseChangePasswordComponent implements OnInit, OnDestroy { + loading = false; rotateUserKey = false; currentMasterPassword: string; masterPasswordHint: string; checkForBreaches = true; characterMinimumMessage = ""; + userkeyRotationV2 = false; constructor( i18nService: I18nService, @@ -60,9 +59,10 @@ export class ChangePasswordComponent private userVerificationService: UserVerificationService, private keyRotationService: UserKeyRotationService, kdfConfigService: KdfConfigService, - masterPasswordService: InternalMasterPasswordServiceAbstraction, + protected masterPasswordService: InternalMasterPasswordServiceAbstraction, accountService: AccountService, toastService: ToastService, + private configService: ConfigService, ) { super( i18nService, @@ -81,6 +81,8 @@ export class ChangePasswordComponent } async ngOnInit() { + this.userkeyRotationV2 = await this.configService.getFeatureFlag(FeatureFlag.UserKeyRotationV2); + if (!(await this.userVerificationService.hasMasterPassword())) { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises @@ -141,6 +143,7 @@ export class ChangePasswordComponent } async submit() { + this.loading = true; if ( this.masterPasswordHint != null && this.masterPasswordHint.toLowerCase() === this.masterPassword.toLowerCase() @@ -150,6 +153,7 @@ export class ChangePasswordComponent title: this.i18nService.t("errorOccurred"), message: this.i18nService.t("hintEqualsPassword"), }); + this.loading = false; return; } @@ -158,46 +162,59 @@ export class ChangePasswordComponent this.leakedPassword = (await this.auditService.passwordLeaked(this.masterPassword)) > 0; } - await super.submit(); - } + if (!(await this.strongPassword())) { + this.loading = false; + return; + } - async setupSubmitActions() { - if (this.currentMasterPassword == null || this.currentMasterPassword === "") { + try { + if (this.rotateUserKey) { + await this.syncService.fullSync(true); + const user = await firstValueFrom(this.accountService.activeAccount$); + if (this.userkeyRotationV2) { + await this.keyRotationService.rotateUserKeyMasterPasswordAndEncryptedData( + this.currentMasterPassword, + this.masterPassword, + user, + ); + } else { + await this.keyRotationService.rotateUserKeyAndEncryptedDataLegacy( + this.currentMasterPassword, + user, + ); + } + } else { + await this.updatePassword(this.masterPassword); + } + } catch (e) { this.toastService.showToast({ variant: "error", title: this.i18nService.t("errorOccurred"), - message: this.i18nService.t("masterPasswordRequired"), + message: e.message, }); - return false; - } - - if (this.rotateUserKey) { - await this.syncService.fullSync(true); } - - return super.setupSubmitActions(); + this.loading = false; } - async performSubmitActions( - newMasterPasswordHash: string, - newMasterKey: MasterKey, - newUserKey: [UserKey, EncString], - ) { - const masterKey = await this.keyService.makeMasterKey( - this.currentMasterPassword, - await firstValueFrom(this.accountService.activeAccount$.pipe(map((a) => a?.email))), - await this.kdfConfigService.getKdfConfig(), + // todo: move this to a service + // https://bitwarden.atlassian.net/browse/PM-17108 + private async updatePassword(newMasterPassword: string) { + const currentMasterPassword = this.currentMasterPassword; + const { userId, email } = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((a) => ({ userId: a?.id, email: a?.email }))), ); + const kdfConfig = await firstValueFrom(this.kdfConfigService.getKdfConfig$(userId)); - const userId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); - const newLocalKeyHash = await this.keyService.hashMasterKey( - this.masterPassword, - newMasterKey, - HashPurpose.LocalAuthorization, + const currentMasterKey = await this.keyService.makeMasterKey( + currentMasterPassword, + email, + kdfConfig, ); - - const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey, userId); - if (userKey == null) { + const decryptedUserKey = await this.masterPasswordService.decryptUserKeyWithMasterKey( + currentMasterKey, + userId, + ); + if (decryptedUserKey == null) { this.toastService.showToast({ variant: "error", title: null, @@ -206,33 +223,29 @@ export class ChangePasswordComponent return; } + const newMasterKey = await this.keyService.makeMasterKey(newMasterPassword, email, kdfConfig); + const newMasterKeyEncryptedUserKey = await this.keyService.encryptUserKeyWithMasterKey( + newMasterKey, + decryptedUserKey, + ); + const request = new PasswordRequest(); request.masterPasswordHash = await this.keyService.hashMasterKey( this.currentMasterPassword, - masterKey, + currentMasterKey, ); request.masterPasswordHint = this.masterPasswordHint; - request.newMasterPasswordHash = newMasterPasswordHash; - request.key = newUserKey[1].encryptedString; - + request.newMasterPasswordHash = await this.keyService.hashMasterKey( + newMasterPassword, + newMasterKey, + ); + request.key = newMasterKeyEncryptedUserKey[1].encryptedString; try { - if (this.rotateUserKey) { - this.formPromise = this.apiService.postPassword(request).then(async () => { - // we need to save this for local masterkey verification during rotation - await this.masterPasswordService.setMasterKeyHash(newLocalKeyHash, userId as UserId); - await this.masterPasswordService.setMasterKey(newMasterKey, userId as UserId); - return this.updateKey(); - }); - } else { - this.formPromise = this.apiService.postPassword(request); - } - - await this.formPromise; - + await this.apiService.postPassword(request); this.toastService.showToast({ variant: "success", title: this.i18nService.t("masterPasswordChanged"), - message: this.i18nService.t("logBackIn"), + message: this.i18nService.t("masterPasswordChangedDesc"), }); this.messagingService.send("logout"); } catch { @@ -243,9 +256,4 @@ export class ChangePasswordComponent }); } } - - private async updateKey() { - const user = await firstValueFrom(this.accountService.activeAccount$); - await this.keyRotationService.rotateUserKeyAndEncryptedData(this.masterPassword, user); - } } diff --git a/apps/web/src/app/key-management/key-rotation/request/account-keys.request.ts b/apps/web/src/app/key-management/key-rotation/request/account-keys.request.ts new file mode 100644 index 00000000000..1c9b6c9ceca --- /dev/null +++ b/apps/web/src/app/key-management/key-rotation/request/account-keys.request.ts @@ -0,0 +1,10 @@ +export class AccountKeysRequest { + // Other keys encrypted by the userkey + userKeyEncryptedAccountPrivateKey: string; + accountPublicKey: string; + + constructor(userKeyEncryptedAccountPrivateKey: string, accountPublicKey: string) { + this.userKeyEncryptedAccountPrivateKey = userKeyEncryptedAccountPrivateKey; + this.accountPublicKey = accountPublicKey; + } +} diff --git a/apps/web/src/app/key-management/key-rotation/request/master-password-unlock-data.request.ts b/apps/web/src/app/key-management/key-rotation/request/master-password-unlock-data.request.ts new file mode 100644 index 00000000000..5e74d0273b3 --- /dev/null +++ b/apps/web/src/app/key-management/key-rotation/request/master-password-unlock-data.request.ts @@ -0,0 +1,31 @@ +import { Argon2KdfConfig, KdfConfig, KdfType } from "@bitwarden/key-management"; + +export class MasterPasswordUnlockDataRequest { + kdfType: KdfType = KdfType.PBKDF2_SHA256; + kdfIterations: number = 0; + kdfMemory?: number; + kdfParallelism?: number; + + email: string; + masterKeyAuthenticationHash: string; + + masterKeyEncryptedUserKey: string; + + constructor( + kdfConfig: KdfConfig, + email: string, + masterKeyAuthenticationHash: string, + masterKeyEncryptedUserKey: string, + ) { + this.kdfType = kdfConfig.kdfType; + this.kdfIterations = kdfConfig.iterations; + if (kdfConfig.kdfType === KdfType.Argon2id) { + this.kdfMemory = (kdfConfig as Argon2KdfConfig).memory; + this.kdfParallelism = (kdfConfig as Argon2KdfConfig).parallelism; + } + + this.email = email; + this.masterKeyAuthenticationHash = masterKeyAuthenticationHash; + this.masterKeyEncryptedUserKey = masterKeyEncryptedUserKey; + } +} diff --git a/apps/web/src/app/key-management/key-rotation/request/rotate-user-account-keys.request.ts b/apps/web/src/app/key-management/key-rotation/request/rotate-user-account-keys.request.ts new file mode 100644 index 00000000000..f335bb66bbb --- /dev/null +++ b/apps/web/src/app/key-management/key-rotation/request/rotate-user-account-keys.request.ts @@ -0,0 +1,29 @@ +import { AccountKeysRequest } from "./account-keys.request"; +import { UnlockDataRequest } from "./unlock-data.request"; +import { UserDataRequest as AccountDataRequest } from "./userdata.request"; + +export class RotateUserAccountKeysRequest { + constructor( + accountUnlockData: UnlockDataRequest, + accountKeys: AccountKeysRequest, + accountData: AccountDataRequest, + oldMasterKeyAuthenticationHash: string, + ) { + this.accountUnlockData = accountUnlockData; + this.accountKeys = accountKeys; + this.accountData = accountData; + this.oldMasterKeyAuthenticationHash = oldMasterKeyAuthenticationHash; + } + + // Authentication for the request + oldMasterKeyAuthenticationHash: string; + + // All methods to get to the userkey + accountUnlockData: UnlockDataRequest; + + // Other keys encrypted by the userkey + accountKeys: AccountKeysRequest; + + // User vault data encrypted by the userkey + accountData: AccountDataRequest; +} diff --git a/apps/web/src/app/key-management/key-rotation/request/unlock-data.request.ts b/apps/web/src/app/key-management/key-rotation/request/unlock-data.request.ts new file mode 100644 index 00000000000..5cdb56a3e23 --- /dev/null +++ b/apps/web/src/app/key-management/key-rotation/request/unlock-data.request.ts @@ -0,0 +1,26 @@ +import { OrganizationUserResetPasswordWithIdRequest } from "@bitwarden/admin-console/common"; +import { WebauthnRotateCredentialRequest } from "@bitwarden/common/auth/models/request/webauthn-rotate-credential.request"; + +import { EmergencyAccessWithIdRequest } from "../../../auth/emergency-access/request/emergency-access-update.request"; + +import { MasterPasswordUnlockDataRequest } from "./master-password-unlock-data.request"; + +export class UnlockDataRequest { + // All methods to get to the userkey + masterPasswordUnlockData: MasterPasswordUnlockDataRequest; + emergencyAccessUnlockData: EmergencyAccessWithIdRequest[]; + organizationAccountRecoveryUnlockData: OrganizationUserResetPasswordWithIdRequest[]; + passkeyUnlockData: WebauthnRotateCredentialRequest[]; + + constructor( + masterPasswordUnlockData: MasterPasswordUnlockDataRequest, + emergencyAccessUnlockData: EmergencyAccessWithIdRequest[], + organizationAccountRecoveryUnlockData: OrganizationUserResetPasswordWithIdRequest[], + passkeyUnlockData: WebauthnRotateCredentialRequest[], + ) { + this.masterPasswordUnlockData = masterPasswordUnlockData; + this.emergencyAccessUnlockData = emergencyAccessUnlockData; + this.organizationAccountRecoveryUnlockData = organizationAccountRecoveryUnlockData; + this.passkeyUnlockData = passkeyUnlockData; + } +} diff --git a/apps/web/src/app/key-management/key-rotation/request/userdata.request.ts b/apps/web/src/app/key-management/key-rotation/request/userdata.request.ts new file mode 100644 index 00000000000..02204428686 --- /dev/null +++ b/apps/web/src/app/key-management/key-rotation/request/userdata.request.ts @@ -0,0 +1,19 @@ +import { SendWithIdRequest } from "@bitwarden/common/tools/send/models/request/send-with-id.request"; +import { CipherWithIdRequest } from "@bitwarden/common/vault/models/request/cipher-with-id.request"; +import { FolderWithIdRequest } from "@bitwarden/common/vault/models/request/folder-with-id.request"; + +export class UserDataRequest { + ciphers: CipherWithIdRequest[]; + folders: FolderWithIdRequest[]; + sends: SendWithIdRequest[]; + + constructor( + ciphers: CipherWithIdRequest[], + folders: FolderWithIdRequest[], + sends: SendWithIdRequest[], + ) { + this.ciphers = ciphers; + this.folders = folders; + this.sends = sends; + } +} diff --git a/apps/web/src/app/key-management/key-rotation/user-key-rotation-api.service.ts b/apps/web/src/app/key-management/key-rotation/user-key-rotation-api.service.ts index 3c8adc886df..2a947359bcb 100644 --- a/apps/web/src/app/key-management/key-rotation/user-key-rotation-api.service.ts +++ b/apps/web/src/app/key-management/key-rotation/user-key-rotation-api.service.ts @@ -2,6 +2,7 @@ import { inject, Injectable } from "@angular/core"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { RotateUserAccountKeysRequest } from "./request/rotate-user-account-keys.request"; import { UpdateKeyRequest } from "./request/update-key.request"; @Injectable() @@ -11,4 +12,14 @@ export class UserKeyRotationApiService { postUserKeyUpdate(request: UpdateKeyRequest): Promise { return this.apiService.send("POST", "/accounts/key", request, true, false); } + + postUserKeyUpdateV2(request: RotateUserAccountKeysRequest): Promise { + return this.apiService.send( + "POST", + "/accounts/key-management/rotate-user-account-keys", + request, + true, + false, + ); + } } diff --git a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.spec.ts b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.spec.ts index 4f2ae8f77e0..fba75723985 100644 --- a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.spec.ts +++ b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.spec.ts @@ -4,24 +4,31 @@ import { mock, MockProxy } from "jest-mock-extended"; import { BehaviorSubject } from "rxjs"; import { OrganizationUserResetPasswordWithIdRequest } from "@bitwarden/admin-console/common"; +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; +import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; +import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { WebauthnRotateCredentialRequest } from "@bitwarden/common/auth/models/request/webauthn-rotate-credential.request"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { Utils } from "@bitwarden/common/platform/misc/utils"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { SendWithIdRequest } from "@bitwarden/common/tools/send/models/request/send-with-id.request"; import { SendService } from "@bitwarden/common/tools/send/services/send.service.abstraction"; import { UserId } from "@bitwarden/common/types/guid"; -import { UserKey, UserPrivateKey } from "@bitwarden/common/types/key"; +import { UserKey, UserPrivateKey, UserPublicKey } from "@bitwarden/common/types/key"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherWithIdRequest } from "@bitwarden/common/vault/models/request/cipher-with-id.request"; import { FolderWithIdRequest } from "@bitwarden/common/vault/models/request/folder-with-id.request"; -import { KeyService } from "@bitwarden/key-management"; +import { DialogService, ToastService } from "@bitwarden/components"; +import { DEFAULT_KDF_CONFIG, KeyService } from "@bitwarden/key-management"; import { OrganizationUserResetPasswordService } from "../../admin-console/organizations/members/services/organization-user-reset-password/organization-user-reset-password.service"; import { WebauthnLoginAdminService } from "../core"; @@ -48,6 +55,13 @@ describe("KeyRotationService", () => { let mockSyncService: MockProxy; let mockWebauthnLoginAdminService: MockProxy; let mockLogService: MockProxy; + let mockVaultTimeoutService: MockProxy; + let mockeDialogService: MockProxy; + let mockFullApiService: MockProxy; + let mockTokenService: MockProxy; + let mockToastService: MockProxy; + let mockI18nService: MockProxy; + let mockInternalMasterPasswordService: MockProxy; const mockUser = { id: "mockUserId" as UserId, @@ -71,6 +85,13 @@ describe("KeyRotationService", () => { mockSyncService = mock(); mockWebauthnLoginAdminService = mock(); mockLogService = mock(); + mockVaultTimeoutService = mock(); + mockeDialogService = mock(); + mockFullApiService = mock(); + mockTokenService = mock(); + mockToastService = mock(); + mockI18nService = mock(); + mockInternalMasterPasswordService = mock(); keyRotationService = new UserKeyRotationService( mockUserVerificationService, @@ -86,6 +107,13 @@ describe("KeyRotationService", () => { mockSyncService, mockWebauthnLoginAdminService, mockLogService, + mockVaultTimeoutService, + mockeDialogService, + mockFullApiService, + mockTokenService, + mockToastService, + mockI18nService, + mockInternalMasterPasswordService, ); }); @@ -95,6 +123,7 @@ describe("KeyRotationService", () => { describe("rotateUserKeyAndEncryptedData", () => { let privateKey: BehaviorSubject; + let keyPair: BehaviorSubject<{ privateKey: UserPrivateKey; publicKey: UserPublicKey }>; beforeEach(() => { mockKeyService.makeUserKey.mockResolvedValue([ @@ -113,6 +142,8 @@ describe("KeyRotationService", () => { // Mock user verification mockUserVerificationService.verifyUserByMasterPassword.mockResolvedValue({ masterKey: "mockMasterKey" as any, + kdfConfig: DEFAULT_KDF_CONFIG, + email: "mockEmail", policyOptions: null, }); @@ -123,6 +154,12 @@ describe("KeyRotationService", () => { privateKey = new BehaviorSubject("mockPrivateKey" as any); mockKeyService.userPrivateKeyWithLegacySupport$.mockReturnValue(privateKey); + keyPair = new BehaviorSubject({ + privateKey: "mockPrivateKey", + publicKey: "mockPublicKey", + } as any); + mockKeyService.userEncryptionKeyPair$.mockReturnValue(keyPair); + // Mock ciphers const mockCiphers = [createMockCipher("1", "Cipher 1"), createMockCipher("2", "Cipher 2")]; mockCipherService.getRotatedData.mockResolvedValue(mockCiphers); @@ -148,8 +185,8 @@ describe("KeyRotationService", () => { mockWebauthnLoginAdminService.getRotatedData.mockResolvedValue(webauthn); }); - it("rotates the user key and encrypted data", async () => { - await keyRotationService.rotateUserKeyAndEncryptedData("mockMasterPassword", mockUser); + it("rotates the user key and encrypted data legacy", async () => { + await keyRotationService.rotateUserKeyAndEncryptedDataLegacy("mockMasterPassword", mockUser); expect(mockApiService.postUserKeyUpdate).toHaveBeenCalled(); const arg = mockApiService.postUserKeyUpdate.mock.calls[0][0]; @@ -163,9 +200,47 @@ describe("KeyRotationService", () => { expect(arg.webauthnKeys.length).toBe(2); }); + it("rotates the user key and encrypted data", async () => { + await keyRotationService.rotateUserKeyMasterPasswordAndEncryptedData( + "mockMasterPassword", + "mockNewMasterPassword", + mockUser, + ); + + expect(mockApiService.postUserKeyUpdateV2).toHaveBeenCalled(); + const arg = mockApiService.postUserKeyUpdateV2.mock.calls[0][0]; + expect(arg.oldMasterKeyAuthenticationHash).toBe("mockMasterPasswordHash"); + expect(arg.accountUnlockData.masterPasswordUnlockData.email).toBe("mockEmail"); + expect(arg.accountUnlockData.masterPasswordUnlockData.kdfType).toBe( + DEFAULT_KDF_CONFIG.kdfType, + ); + expect(arg.accountUnlockData.masterPasswordUnlockData.kdfIterations).toBe( + DEFAULT_KDF_CONFIG.iterations, + ); + expect(arg.accountKeys.accountPublicKey).toBe(Utils.fromUtf8ToB64("mockPublicKey")); + expect(arg.accountKeys.userKeyEncryptedAccountPrivateKey).toBe("mockEncryptedData"); + expect(arg.accountData.ciphers.length).toBe(2); + expect(arg.accountData.folders.length).toBe(2); + expect(arg.accountData.sends.length).toBe(2); + }); + + it("legacy throws if master password provided is falsey", async () => { + await expect( + keyRotationService.rotateUserKeyAndEncryptedDataLegacy("", mockUser), + ).rejects.toThrow(); + }); + it("throws if master password provided is falsey", async () => { await expect( - keyRotationService.rotateUserKeyAndEncryptedData("", mockUser), + keyRotationService.rotateUserKeyMasterPasswordAndEncryptedData("", "", mockUser), + ).rejects.toThrow(); + }); + + it("legacy throws if user key creation fails", async () => { + mockKeyService.makeUserKey.mockResolvedValueOnce([null, null]); + + await expect( + keyRotationService.rotateUserKeyAndEncryptedDataLegacy("mockMasterPassword", mockUser), ).rejects.toThrow(); }); @@ -173,15 +248,41 @@ describe("KeyRotationService", () => { mockKeyService.makeUserKey.mockResolvedValueOnce([null, null]); await expect( - keyRotationService.rotateUserKeyAndEncryptedData("mockMasterPassword", mockUser), + keyRotationService.rotateUserKeyMasterPasswordAndEncryptedData( + "mockMasterPassword", + "mockMasterPassword1", + mockUser, + ), ).rejects.toThrow(); }); - it("throws if no private key is found", async () => { + it("legacy throws if no private key is found", async () => { privateKey.next(null); await expect( - keyRotationService.rotateUserKeyAndEncryptedData("mockMasterPassword", mockUser), + keyRotationService.rotateUserKeyAndEncryptedDataLegacy("mockMasterPassword", mockUser), + ).rejects.toThrow(); + }); + + it("throws if no private key is found", async () => { + keyPair.next(null); + + await expect( + keyRotationService.rotateUserKeyMasterPasswordAndEncryptedData( + "mockMasterPassword", + "mockMasterPassword1", + mockUser, + ), + ).rejects.toThrow(); + }); + + it("legacy throws if master password is incorrect", async () => { + mockUserVerificationService.verifyUserByMasterPassword.mockRejectedValueOnce( + new Error("Invalid master password"), + ); + + await expect( + keyRotationService.rotateUserKeyAndEncryptedDataLegacy("mockMasterPassword", mockUser), ).rejects.toThrow(); }); @@ -191,15 +292,31 @@ describe("KeyRotationService", () => { ); await expect( - keyRotationService.rotateUserKeyAndEncryptedData("mockMasterPassword", mockUser), + keyRotationService.rotateUserKeyMasterPasswordAndEncryptedData( + "mockMasterPassword", + "mockMasterPassword1", + mockUser, + ), ).rejects.toThrow(); }); - it("throws if server rotation fails", async () => { + it("legacy throws if server rotation fails", async () => { mockApiService.postUserKeyUpdate.mockRejectedValueOnce(new Error("mockError")); await expect( - keyRotationService.rotateUserKeyAndEncryptedData("mockMasterPassword", mockUser), + keyRotationService.rotateUserKeyAndEncryptedDataLegacy("mockMasterPassword", mockUser), + ).rejects.toThrow(); + }); + + it("throws if server rotation fails", async () => { + mockApiService.postUserKeyUpdateV2.mockRejectedValueOnce(new Error("mockError")); + + await expect( + keyRotationService.rotateUserKeyMasterPasswordAndEncryptedData( + "mockMasterPassword", + "mockMasterPassword1", + mockUser, + ), ).rejects.toThrow(); }); }); diff --git a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts index ae47798420e..2e9d7ea29d9 100644 --- a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts +++ b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts @@ -3,13 +3,20 @@ import { Injectable } from "@angular/core"; import { firstValueFrom } from "rxjs"; +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service"; import { Account } from "@bitwarden/common/auth/abstractions/account.service"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; +import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; +import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { VerificationType } from "@bitwarden/common/auth/enums/verification-type"; import { MasterPasswordVerification } from "@bitwarden/common/auth/types/verification"; import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { HashPurpose } from "@bitwarden/common/platform/enums"; +import { Utils } from "@bitwarden/common/platform/misc/utils"; import { EncryptedString } from "@bitwarden/common/platform/models/domain/enc-string"; import { SendService } from "@bitwarden/common/tools/send/services/send.service.abstraction"; import { UserId } from "@bitwarden/common/types/guid"; @@ -17,13 +24,19 @@ import { UserKey } from "@bitwarden/common/types/key"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; +import { DialogService, ToastService } from "@bitwarden/components"; import { KeyService } from "@bitwarden/key-management"; import { OrganizationUserResetPasswordService } from "../../admin-console/organizations/members/services/organization-user-reset-password/organization-user-reset-password.service"; import { WebauthnLoginAdminService } from "../../auth/core"; import { EmergencyAccessService } from "../../auth/emergency-access"; +import { AccountKeysRequest } from "./request/account-keys.request"; +import { MasterPasswordUnlockDataRequest } from "./request/master-password-unlock-data.request"; +import { RotateUserAccountKeysRequest } from "./request/rotate-user-account-keys.request"; +import { UnlockDataRequest } from "./request/unlock-data.request"; import { UpdateKeyRequest } from "./request/update-key.request"; +import { UserDataRequest } from "./request/userdata.request"; import { UserKeyRotationApiService } from "./user-key-rotation-api.service"; @Injectable() @@ -42,14 +55,157 @@ export class UserKeyRotationService { private syncService: SyncService, private webauthnLoginAdminService: WebauthnLoginAdminService, private logService: LogService, + private vaultTimeoutService: VaultTimeoutService, + private dialogService: DialogService, + private fullApiService: ApiService, + private tokenService: TokenService, + private toastService: ToastService, + private i18nService: I18nService, + private masterPasswordService: InternalMasterPasswordServiceAbstraction, ) {} /** * Creates a new user key and re-encrypts all required data with the it. * @param masterPassword current master password (used for validation) */ - async rotateUserKeyAndEncryptedData(masterPassword: string, user: Account): Promise { + async rotateUserKeyMasterPasswordAndEncryptedData( + oldMasterPassword: string, + newMasterPassword: string, + user: Account, + ): Promise { this.logService.info("[Userkey rotation] Starting user key rotation..."); + if (!newMasterPassword) { + this.logService.info("[Userkey rotation] Invalid master password provided. Aborting!"); + throw new Error("Invalid master password"); + } + + if ((await this.syncService.getLastSync()) === null) { + this.logService.info("[Userkey rotation] Client was never synced. Aborting!"); + throw new Error( + "The local vault is de-synced and the keys cannot be rotated. Please log out and log back in to resolve this issue.", + ); + } + + const { + masterKey: oldMasterKey, + email, + kdfConfig, + } = await this.userVerificationService.verifyUserByMasterPassword( + { + type: VerificationType.MasterPassword, + secret: oldMasterPassword, + }, + user.id, + user.email, + ); + + const newMasterKey = await this.keyService.makeMasterKey(newMasterPassword, email, kdfConfig); + + const [newUnencryptedUserKey, newMasterKeyEncryptedUserKey] = + await this.keyService.makeUserKey(newMasterKey); + + if (!newUnencryptedUserKey || !newMasterKeyEncryptedUserKey) { + this.logService.info("[Userkey rotation] User key could not be created. Aborting!"); + throw new Error("User key could not be created"); + } + + const newMasterKeyAuthenticationHash = await this.keyService.hashMasterKey( + newMasterPassword, + newMasterKey, + HashPurpose.ServerAuthorization, + ); + const masterPasswordUnlockData = new MasterPasswordUnlockDataRequest( + kdfConfig, + email, + newMasterKeyAuthenticationHash, + newMasterKeyEncryptedUserKey.encryptedString, + ); + const { privateKey, publicKey } = await firstValueFrom( + this.keyService.userEncryptionKeyPair$(user.id), + ); + const accountKeysRequest = new AccountKeysRequest( + (await this.encryptService.encrypt(privateKey, newUnencryptedUserKey)).encryptedString, + Utils.fromBufferToB64(publicKey), + ); + + const originalUserKey = await firstValueFrom(this.keyService.userKey$(user.id)); + const rotatedCiphers = await this.cipherService.getRotatedData( + originalUserKey, + newUnencryptedUserKey, + user.id, + ); + const rotatedFolders = await this.folderService.getRotatedData( + originalUserKey, + newUnencryptedUserKey, + user.id, + ); + const rotatedSends = await this.sendService.getRotatedData( + originalUserKey, + newUnencryptedUserKey, + user.id, + ); + const accountDataRequest = new UserDataRequest(rotatedCiphers, rotatedFolders, rotatedSends); + + const emergencyAccessUnlockData = await this.emergencyAccessService.getRotatedData( + originalUserKey, + newUnencryptedUserKey, + user.id, + ); + // Note: Reset password keys request model has user verification + // properties, but the rotation endpoint uses its own MP hash. + const organizationAccountRecoveryUnlockData = await this.resetPasswordService.getRotatedData( + originalUserKey, + newUnencryptedUserKey, + user.id, + ); + const passkeyUnlockData = await this.webauthnLoginAdminService.getRotatedData( + originalUserKey, + newUnencryptedUserKey, + user.id, + ); + const unlockDataRequest = new UnlockDataRequest( + masterPasswordUnlockData, + emergencyAccessUnlockData, + organizationAccountRecoveryUnlockData, + passkeyUnlockData, + ); + + const request = new RotateUserAccountKeysRequest( + unlockDataRequest, + accountKeysRequest, + accountDataRequest, + await this.keyService.hashMasterKey(oldMasterPassword, oldMasterKey), + ); + + this.logService.info("[Userkey rotation] Posting user key rotation request to server"); + await this.apiService.postUserKeyUpdateV2(request); + this.logService.info("[Userkey rotation] Userkey rotation request posted to server"); + + // TODO PM-2199: Add device trust rotation support to the user key rotation endpoint + this.logService.info("[Userkey rotation] Rotating device trust..."); + await this.deviceTrustService.rotateDevicesTrust( + user.id, + newUnencryptedUserKey, + newMasterKeyAuthenticationHash, + ); + this.logService.info("[Userkey rotation] Device trust rotation completed"); + this.toastService.showToast({ + variant: "success", + title: this.i18nService.t("rotationCompletedTitle"), + message: this.i18nService.t("rotationCompletedDesc"), + timeout: 15000, + }); + + // temporary until userkey can be better verified + await this.vaultTimeoutService.logOut(); + } + + /** + * Creates a new user key and re-encrypts all required data with the it. + * @param masterPassword current master password (used for validation) + */ + async rotateUserKeyAndEncryptedDataLegacy(masterPassword: string, user: Account): Promise { + this.logService.info("[Userkey rotation] Starting legacy user key rotation..."); if (!masterPassword) { this.logService.info("[Userkey rotation] Invalid master password provided. Aborting!"); throw new Error("Invalid master password"); @@ -167,6 +323,7 @@ export class UserKeyRotationService { this.logService.info("[Userkey rotation] Rotating device trust..."); await this.deviceTrustService.rotateDevicesTrust(user.id, newUserKey, masterPasswordHash); this.logService.info("[Userkey rotation] Device trust rotation completed"); + await this.vaultTimeoutService.logOut(); } private async encryptPrivateKey( diff --git a/apps/web/src/app/key-management/migrate-encryption/migrate-legacy-encryption.component.ts b/apps/web/src/app/key-management/migrate-encryption/migrate-legacy-encryption.component.ts index 2361293fe80..11073dd49d2 100644 --- a/apps/web/src/app/key-management/migrate-encryption/migrate-legacy-encryption.component.ts +++ b/apps/web/src/app/key-management/migrate-encryption/migrate-legacy-encryption.component.ts @@ -4,6 +4,7 @@ import { Component } from "@angular/core"; import { FormControl, FormGroup, Validators } from "@angular/forms"; import { firstValueFrom } from "rxjs"; +import { JslibServicesModule } from "@bitwarden/angular/services/jslib-services.module"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -21,7 +22,7 @@ import { UserKeyRotationService } from "../key-rotation/user-key-rotation.servic // This component is used to migrate from the old encryption scheme to the new one. @Component({ standalone: true, - imports: [SharedModule, UserKeyRotationModule], + imports: [SharedModule, JslibServicesModule, UserKeyRotationModule], templateUrl: "migrate-legacy-encryption.component.html", }) export class MigrateFromLegacyEncryptionComponent { @@ -62,7 +63,7 @@ export class MigrateFromLegacyEncryptionComponent { try { await this.syncService.fullSync(false, true); - await this.keyRotationService.rotateUserKeyAndEncryptedData(masterPassword, activeUser); + await this.keyRotationService.rotateUserKeyAndEncryptedDataLegacy(masterPassword, activeUser); this.toastService.showToast({ variant: "success", diff --git a/libs/common/src/auth/services/user-verification/user-verification.service.spec.ts b/libs/common/src/auth/services/user-verification/user-verification.service.spec.ts index 102e4bac8da..4b36a227090 100644 --- a/libs/common/src/auth/services/user-verification/user-verification.service.spec.ts +++ b/libs/common/src/auth/services/user-verification/user-verification.service.spec.ts @@ -226,6 +226,8 @@ describe("UserVerificationService", () => { expect(result).toEqual({ policyOptions: null, masterKey: "masterKey", + kdfConfig: "kdfConfig", + email: "email", }); }); @@ -284,6 +286,8 @@ describe("UserVerificationService", () => { expect(result).toEqual({ policyOptions: "MasterPasswordPolicyOptions", masterKey: "masterKey", + kdfConfig: "kdfConfig", + email: "email", }); }); diff --git a/libs/common/src/auth/services/user-verification/user-verification.service.ts b/libs/common/src/auth/services/user-verification/user-verification.service.ts index 4735da32b6b..abfbf320543 100644 --- a/libs/common/src/auth/services/user-verification/user-verification.service.ts +++ b/libs/common/src/auth/services/user-verification/user-verification.service.ts @@ -237,7 +237,7 @@ export class UserVerificationService implements UserVerificationServiceAbstracti ); await this.masterPasswordService.setMasterKeyHash(localKeyHash, userId); await this.masterPasswordService.setMasterKey(masterKey, userId); - return { policyOptions, masterKey }; + return { policyOptions, masterKey, kdfConfig, email }; } private async verifyUserByPIN(verification: PinVerification, userId: UserId): Promise { diff --git a/libs/common/src/auth/types/verification.ts b/libs/common/src/auth/types/verification.ts index 2dddd5fb914..31029847828 100644 --- a/libs/common/src/auth/types/verification.ts +++ b/libs/common/src/auth/types/verification.ts @@ -1,3 +1,5 @@ +import { KdfConfig } from "@bitwarden/key-management"; + import { MasterKey } from "../../types/key"; import { VerificationType } from "../enums/verification-type"; import { MasterPasswordPolicyResponse } from "../models/response/master-password-policy.response"; @@ -22,5 +24,7 @@ export type ServerSideVerification = OtpVerification | MasterPasswordVerificatio export type MasterPasswordVerificationResponse = { masterKey: MasterKey; + kdfConfig: KdfConfig; + email: string; policyOptions: MasterPasswordPolicyResponse; }; diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index a988bdbf6a7..2e02dd7f574 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -34,6 +34,7 @@ export enum FeatureFlag { UnauthenticatedExtensionUIRefresh = "unauth-ui-refresh", SSHKeyVaultItem = "ssh-key-vault-item", SSHAgent = "ssh-agent", + UserKeyRotationV2 = "userkey-rotation-v2", CipherKeyEncryption = "cipher-key-encryption", PM11901_RefactorSelfHostingLicenseUploader = "PM-11901-refactor-self-hosting-license-uploader", CriticalApps = "pm-14466-risk-insights-critical-application", @@ -91,6 +92,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.UnauthenticatedExtensionUIRefresh]: FALSE, [FeatureFlag.SSHKeyVaultItem]: FALSE, [FeatureFlag.SSHAgent]: FALSE, + [FeatureFlag.UserKeyRotationV2]: FALSE, [FeatureFlag.CipherKeyEncryption]: FALSE, [FeatureFlag.PM11901_RefactorSelfHostingLicenseUploader]: FALSE, [FeatureFlag.CriticalApps]: FALSE, diff --git a/libs/key-management/src/abstractions/key.service.ts b/libs/key-management/src/abstractions/key.service.ts index da59c2986b2..248701ffa89 100644 --- a/libs/key-management/src/abstractions/key.service.ts +++ b/libs/key-management/src/abstractions/key.service.ts @@ -332,6 +332,17 @@ export abstract class KeyService { */ abstract userPrivateKeyWithLegacySupport$(userId: UserId): Observable; + /** + * Gets an observable stream of the given users decrypted private key and public key, guaranteed to be consistent. + * Will emit null if the user doesn't have a userkey to decrypt the encrypted private key, or null if the user doesn't have a private key + * at all. + * + * @param userId The user id of the user to get the data for. + */ + abstract userEncryptionKeyPair$( + userId: UserId, + ): Observable<{ privateKey: UserPrivateKey; publicKey: UserPublicKey } | null>; + /** * Generates a fingerprint phrase for the user based on their public key * @param fingerprintMaterial Fingerprint material diff --git a/libs/key-management/src/key.service.ts b/libs/key-management/src/key.service.ts index 98474b4babf..4da88d4efa5 100644 --- a/libs/key-management/src/key.service.ts +++ b/libs/key-management/src/key.service.ts @@ -911,6 +911,21 @@ export class DefaultKeyService implements KeyServiceAbstraction { return this.userPrivateKeyHelper$(userId, false).pipe(map((keys) => keys?.userPrivateKey)); } + userEncryptionKeyPair$( + userId: UserId, + ): Observable<{ privateKey: UserPrivateKey; publicKey: UserPublicKey } | null> { + return this.userPrivateKey$(userId).pipe( + switchMap(async (privateKey) => { + if (privateKey == null) { + return null; + } + + const publicKey = await this.derivePublicKey(privateKey); + return { privateKey, publicKey }; + }), + ); + } + userEncryptedPrivateKey$(userId: UserId): Observable { return this.stateProvider.getUser(userId, USER_ENCRYPTED_PRIVATE_KEY).state$; } From 4e3a44e24824e0c0e410d2b424b6ee820076d6fe Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 29 Jan 2025 19:54:17 +0100 Subject: [PATCH 2/8] Pass through masterpassword hint --- apps/web/src/app/auth/settings/change-password.component.ts | 1 + .../request/master-password-unlock-data.request.ts | 4 ++++ .../key-management/key-rotation/user-key-rotation.service.ts | 2 ++ 3 files changed, 7 insertions(+) diff --git a/apps/web/src/app/auth/settings/change-password.component.ts b/apps/web/src/app/auth/settings/change-password.component.ts index 389264f4a9b..65a43d4bfcf 100644 --- a/apps/web/src/app/auth/settings/change-password.component.ts +++ b/apps/web/src/app/auth/settings/change-password.component.ts @@ -176,6 +176,7 @@ export class ChangePasswordComponent this.currentMasterPassword, this.masterPassword, user, + this.masterPasswordHint, ); } else { await this.keyRotationService.rotateUserKeyAndEncryptedDataLegacy( diff --git a/apps/web/src/app/key-management/key-rotation/request/master-password-unlock-data.request.ts b/apps/web/src/app/key-management/key-rotation/request/master-password-unlock-data.request.ts index 5e74d0273b3..95e54e16464 100644 --- a/apps/web/src/app/key-management/key-rotation/request/master-password-unlock-data.request.ts +++ b/apps/web/src/app/key-management/key-rotation/request/master-password-unlock-data.request.ts @@ -11,11 +11,14 @@ export class MasterPasswordUnlockDataRequest { masterKeyEncryptedUserKey: string; + masterPasswordHint?: string; + constructor( kdfConfig: KdfConfig, email: string, masterKeyAuthenticationHash: string, masterKeyEncryptedUserKey: string, + masterPasswordHash?: string, ) { this.kdfType = kdfConfig.kdfType; this.kdfIterations = kdfConfig.iterations; @@ -27,5 +30,6 @@ export class MasterPasswordUnlockDataRequest { this.email = email; this.masterKeyAuthenticationHash = masterKeyAuthenticationHash; this.masterKeyEncryptedUserKey = masterKeyEncryptedUserKey; + this.masterPasswordHint = masterPasswordHash; } } diff --git a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts index 2e9d7ea29d9..2b1f5a7057d 100644 --- a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts +++ b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts @@ -72,6 +72,7 @@ export class UserKeyRotationService { oldMasterPassword: string, newMasterPassword: string, user: Account, + newMasterPasswordHint?: string, ): Promise { this.logService.info("[Userkey rotation] Starting user key rotation..."); if (!newMasterPassword) { @@ -119,6 +120,7 @@ export class UserKeyRotationService { email, newMasterKeyAuthenticationHash, newMasterKeyEncryptedUserKey.encryptedString, + newMasterPasswordHint, ); const { privateKey, publicKey } = await firstValueFrom( this.keyService.userEncryptionKeyPair$(user.id), From e3cd3cdd51ce0315efd3b479b15c2c827b27217c Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 5 Feb 2025 10:52:32 +0100 Subject: [PATCH 3/8] Properly split old and new code --- .../settings/change-password.component.ts | 150 ++++++++++++++++-- 1 file changed, 137 insertions(+), 13 deletions(-) diff --git a/apps/web/src/app/auth/settings/change-password.component.ts b/apps/web/src/app/auth/settings/change-password.component.ts index 241361289ea..ee1232010aa 100644 --- a/apps/web/src/app/auth/settings/change-password.component.ts +++ b/apps/web/src/app/auth/settings/change-password.component.ts @@ -12,11 +12,16 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { PasswordRequest } from "@bitwarden/common/auth/models/request/password.request"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { HashPurpose } from "@bitwarden/common/platform/enums"; +import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; +import { UserId } from "@bitwarden/common/types/guid"; +import { MasterKey, UserKey } from "@bitwarden/common/types/key"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { DialogService, ToastService } from "@bitwarden/components"; @@ -137,6 +142,23 @@ export class ChangePasswordComponent } async submit() { + if (this.userkeyRotationV2) { + await this.submitNew(); + } else { + await this.submitOld(); + } + } + + async submitNew() { + if (this.currentMasterPassword == null || this.currentMasterPassword === "") { + this.toastService.showToast({ + variant: "error", + title: this.i18nService.t("errorOccurred"), + message: this.i18nService.t("masterPasswordRequired"), + }); + return false; + } + this.loading = true; if ( this.masterPasswordHint != null && @@ -165,19 +187,12 @@ export class ChangePasswordComponent if (this.rotateUserKey) { await this.syncService.fullSync(true); const user = await firstValueFrom(this.accountService.activeAccount$); - if (this.userkeyRotationV2) { - await this.keyRotationService.rotateUserKeyMasterPasswordAndEncryptedData( - this.currentMasterPassword, - this.masterPassword, - user, - this.masterPasswordHint, - ); - } else { - await this.keyRotationService.rotateUserKeyAndEncryptedDataLegacy( - this.currentMasterPassword, - user, - ); - } + await this.keyRotationService.rotateUserKeyMasterPasswordAndEncryptedData( + this.currentMasterPassword, + this.masterPassword, + user, + this.masterPasswordHint, + ); } else { await this.updatePassword(this.masterPassword); } @@ -251,4 +266,113 @@ export class ChangePasswordComponent }); } } + + async submitOld() { + if ( + this.masterPasswordHint != null && + this.masterPasswordHint.toLowerCase() === this.masterPassword.toLowerCase() + ) { + this.toastService.showToast({ + variant: "error", + title: this.i18nService.t("errorOccurred"), + message: this.i18nService.t("hintEqualsPassword"), + }); + return; + } + + this.leakedPassword = false; + if (this.checkForBreaches) { + this.leakedPassword = (await this.auditService.passwordLeaked(this.masterPassword)) > 0; + } + + await super.submit(); + } + + async setupSubmitActions() { + if (this.currentMasterPassword == null || this.currentMasterPassword === "") { + this.toastService.showToast({ + variant: "error", + title: this.i18nService.t("errorOccurred"), + message: this.i18nService.t("masterPasswordRequired"), + }); + return false; + } + + if (this.rotateUserKey) { + await this.syncService.fullSync(true); + } + + return super.setupSubmitActions(); + } + + async performSubmitActions( + newMasterPasswordHash: string, + newMasterKey: MasterKey, + newUserKey: [UserKey, EncString], + ) { + const masterKey = await this.keyService.makeMasterKey( + this.currentMasterPassword, + await firstValueFrom(this.accountService.activeAccount$.pipe(map((a) => a?.email))), + await this.kdfConfigService.getKdfConfig(), + ); + + const userId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); + const newLocalKeyHash = await this.keyService.hashMasterKey( + this.masterPassword, + newMasterKey, + HashPurpose.LocalAuthorization, + ); + + const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey, userId); + if (userKey == null) { + this.toastService.showToast({ + variant: "error", + title: null, + message: this.i18nService.t("invalidMasterPassword"), + }); + return; + } + + const request = new PasswordRequest(); + request.masterPasswordHash = await this.keyService.hashMasterKey( + this.currentMasterPassword, + masterKey, + ); + request.masterPasswordHint = this.masterPasswordHint; + request.newMasterPasswordHash = newMasterPasswordHash; + request.key = newUserKey[1].encryptedString; + + try { + if (this.rotateUserKey) { + this.formPromise = this.apiService.postPassword(request).then(async () => { + // we need to save this for local masterkey verification during rotation + await this.masterPasswordService.setMasterKeyHash(newLocalKeyHash, userId as UserId); + await this.masterPasswordService.setMasterKey(newMasterKey, userId as UserId); + return this.updateKey(); + }); + } else { + this.formPromise = this.apiService.postPassword(request); + } + + await this.formPromise; + + this.toastService.showToast({ + variant: "success", + title: this.i18nService.t("masterPasswordChanged"), + message: this.i18nService.t("logBackIn"), + }); + this.messagingService.send("logout"); + } catch { + this.toastService.showToast({ + variant: "error", + title: null, + message: this.i18nService.t("errorOccurred"), + }); + } + } + + private async updateKey() { + const user = await firstValueFrom(this.accountService.activeAccount$); + await this.keyRotationService.rotateUserKeyAndEncryptedDataLegacy(this.masterPassword, user); + } } From b2e37594bcc5bac334536e78133b8b6a5add6908 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 5 Feb 2025 10:53:29 +0100 Subject: [PATCH 4/8] Mark legacy rotation as deprecated --- .../app/key-management/key-rotation/user-key-rotation.service.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts index 2b1f5a7057d..bfc1fc84d18 100644 --- a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts +++ b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts @@ -205,6 +205,7 @@ export class UserKeyRotationService { /** * Creates a new user key and re-encrypts all required data with the it. * @param masterPassword current master password (used for validation) + * @deprecated */ async rotateUserKeyAndEncryptedDataLegacy(masterPassword: string, user: Account): Promise { this.logService.info("[Userkey rotation] Starting legacy user key rotation..."); From b1f50e6921f380cddc0240c33619b6548583716f Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 5 Feb 2025 10:55:05 +0100 Subject: [PATCH 5/8] Throw when data is null --- .../key-management/key-rotation/user-key-rotation.service.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts index bfc1fc84d18..209265c767c 100644 --- a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts +++ b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts @@ -146,6 +146,10 @@ export class UserKeyRotationService { newUnencryptedUserKey, user.id, ); + if (rotatedCiphers == null || rotatedFolders == null || rotatedSends == null) { + this.logService.info("[Userkey rotation] ciphers, folders, or sends are null. Aborting!"); + throw new Error("ciphers, folders, or sends are null"); + } const accountDataRequest = new UserDataRequest(rotatedCiphers, rotatedFolders, rotatedSends); const emergencyAccessUnlockData = await this.emergencyAccessService.getRotatedData( From 098161f2691b97760a1753ff88210df80d15c6b9 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 5 Feb 2025 10:59:42 +0100 Subject: [PATCH 6/8] Cleanup --- .../user-key-rotation.service.spec.ts | 17 +---------------- .../key-rotation/user-key-rotation.service.ts | 9 +-------- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.spec.ts b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.spec.ts index fba75723985..7a28c6acb79 100644 --- a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.spec.ts +++ b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.spec.ts @@ -4,11 +4,8 @@ import { mock, MockProxy } from "jest-mock-extended"; import { BehaviorSubject } from "rxjs"; import { OrganizationUserResetPasswordWithIdRequest } from "@bitwarden/admin-console/common"; -import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; -import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; -import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { WebauthnRotateCredentialRequest } from "@bitwarden/common/auth/models/request/webauthn-rotate-credential.request"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; @@ -27,7 +24,7 @@ import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.serv import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherWithIdRequest } from "@bitwarden/common/vault/models/request/cipher-with-id.request"; import { FolderWithIdRequest } from "@bitwarden/common/vault/models/request/folder-with-id.request"; -import { DialogService, ToastService } from "@bitwarden/components"; +import { ToastService } from "@bitwarden/components"; import { DEFAULT_KDF_CONFIG, KeyService } from "@bitwarden/key-management"; import { OrganizationUserResetPasswordService } from "../../admin-console/organizations/members/services/organization-user-reset-password/organization-user-reset-password.service"; @@ -56,12 +53,8 @@ describe("KeyRotationService", () => { let mockWebauthnLoginAdminService: MockProxy; let mockLogService: MockProxy; let mockVaultTimeoutService: MockProxy; - let mockeDialogService: MockProxy; - let mockFullApiService: MockProxy; - let mockTokenService: MockProxy; let mockToastService: MockProxy; let mockI18nService: MockProxy; - let mockInternalMasterPasswordService: MockProxy; const mockUser = { id: "mockUserId" as UserId, @@ -86,12 +79,8 @@ describe("KeyRotationService", () => { mockWebauthnLoginAdminService = mock(); mockLogService = mock(); mockVaultTimeoutService = mock(); - mockeDialogService = mock(); - mockFullApiService = mock(); - mockTokenService = mock(); mockToastService = mock(); mockI18nService = mock(); - mockInternalMasterPasswordService = mock(); keyRotationService = new UserKeyRotationService( mockUserVerificationService, @@ -108,12 +97,8 @@ describe("KeyRotationService", () => { mockWebauthnLoginAdminService, mockLogService, mockVaultTimeoutService, - mockeDialogService, - mockFullApiService, - mockTokenService, mockToastService, mockI18nService, - mockInternalMasterPasswordService, ); }); diff --git a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts index 209265c767c..c762f896657 100644 --- a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts +++ b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts @@ -3,12 +3,9 @@ import { Injectable } from "@angular/core"; import { firstValueFrom } from "rxjs"; -import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service"; import { Account } from "@bitwarden/common/auth/abstractions/account.service"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; -import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; -import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { VerificationType } from "@bitwarden/common/auth/enums/verification-type"; import { MasterPasswordVerification } from "@bitwarden/common/auth/types/verification"; @@ -24,7 +21,7 @@ import { UserKey } from "@bitwarden/common/types/key"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; -import { DialogService, ToastService } from "@bitwarden/components"; +import { ToastService } from "@bitwarden/components"; import { KeyService } from "@bitwarden/key-management"; import { OrganizationUserResetPasswordService } from "../../admin-console/organizations/members/services/organization-user-reset-password/organization-user-reset-password.service"; @@ -56,12 +53,8 @@ export class UserKeyRotationService { private webauthnLoginAdminService: WebauthnLoginAdminService, private logService: LogService, private vaultTimeoutService: VaultTimeoutService, - private dialogService: DialogService, - private fullApiService: ApiService, - private tokenService: TokenService, private toastService: ToastService, private i18nService: I18nService, - private masterPasswordService: InternalMasterPasswordServiceAbstraction, ) {} /** From de68e2150ae4f05736555bc5cd75eb007acac754 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 5 Feb 2025 11:12:49 +0100 Subject: [PATCH 7/8] Add tests --- libs/key-management/src/key.service.spec.ts | 49 ++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/libs/key-management/src/key.service.spec.ts b/libs/key-management/src/key.service.spec.ts index 62cdce230ea..1c7cd94a422 100644 --- a/libs/key-management/src/key.service.spec.ts +++ b/libs/key-management/src/key.service.spec.ts @@ -1,5 +1,5 @@ import { mock } from "jest-mock-extended"; -import { bufferCount, firstValueFrom, lastValueFrom, of, take, tap } from "rxjs"; +import { BehaviorSubject, bufferCount, firstValueFrom, lastValueFrom, of, take, tap } from "rxjs"; import { EncryptedOrganizationKeyData } from "@bitwarden/common/admin-console/models/data/encrypted-organization-key.data"; @@ -836,4 +836,51 @@ describe("keyService", () => { }, ); }); + + describe("userPrivateKey$", () => { + type SetupKeysParams = { + makeMasterKey: boolean; + makeUserKey: boolean; + }; + + function setupKeys({ makeMasterKey, makeUserKey }: SetupKeysParams): [UserKey, MasterKey] { + const userKeyState = stateProvider.singleUser.getFake(mockUserId, USER_KEY); + const fakeMasterKey = makeMasterKey ? makeSymmetricCryptoKey(64) : null; + masterPasswordService.masterKeySubject.next(fakeMasterKey); + userKeyState.nextState(null); + const fakeUserKey = makeUserKey ? makeSymmetricCryptoKey(64) : null; + userKeyState.nextState(fakeUserKey); + return [fakeUserKey, fakeMasterKey]; + } + + it("returns null private key is null", async () => { + setupKeys({ makeMasterKey: false, makeUserKey: false }); + + keyService.userPrivateKey$ = jest.fn().mockReturnValue(new BehaviorSubject(null)); + const key = await firstValueFrom(keyService.userEncryptionKeyPair$(mockUserId)); + expect(key).toEqual(null); + }); + + it("returns null when private key is undefined", async () => { + setupKeys({ makeUserKey: true, makeMasterKey: false }); + + keyService.userPrivateKey$ = jest.fn().mockReturnValue(new BehaviorSubject(undefined)); + const key = await firstValueFrom(keyService.userEncryptionKeyPair$(mockUserId)); + expect(key).toEqual(null); + }); + + it("returns keys when private key is defined", async () => { + setupKeys({ makeUserKey: false, makeMasterKey: true }); + + keyService.userPrivateKey$ = jest.fn().mockReturnValue(new BehaviorSubject("private key")); + cryptoFunctionService.rsaExtractPublicKey.mockResolvedValue( + Utils.fromUtf8ToArray("public key"), + ); + const key = await firstValueFrom(keyService.userEncryptionKeyPair$(mockUserId)); + expect(key).toEqual({ + privateKey: "private key", + publicKey: Utils.fromUtf8ToArray("public key"), + }); + }); + }); }); From 0ac616bfbd16d03383f707994b38b584f0cedb5f Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 5 Feb 2025 18:05:43 +0100 Subject: [PATCH 8/8] Fix build --- .../key-rotation/user-key-rotation.service.spec.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.spec.ts b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.spec.ts index 730cac4dd92..6783ecb00e0 100644 --- a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.spec.ts +++ b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.spec.ts @@ -47,17 +47,14 @@ describe("KeyRotationService", () => { let mockResetPasswordService: MockProxy; let mockDeviceTrustService: MockProxy; let mockKeyService: MockProxy; - let mock - - : MockProxy; + let mockEncryptService: MockProxy; let mockConfigService: MockProxy; let mockSyncService: MockProxy; let mockWebauthnLoginAdminService: MockProxy; let mockLogService: MockProxy; let mockVaultTimeoutService: MockProxy; let mockToastService: MockProxy; - let mock - Service: MockProxy; + let mockI18nService: MockProxy; const mockUser = { id: "mockUserId" as UserId,