From 37772ddd1c41fc033e84f585900b67c1981784fa Mon Sep 17 00:00:00 2001 From: Todd Martin Date: Tue, 15 Oct 2024 12:28:27 -0400 Subject: [PATCH 1/4] Updated decryptUserKeyWithMasterKey to requireUserId --- .../web/src/app/auth/settings/change-password.component.ts | 2 +- libs/angular/src/auth/components/lock.component.ts | 1 + libs/angular/src/auth/guards/auth.guard.ts | 1 + libs/auth/src/angular/lock/lock.component.ts | 1 + .../common/login-strategies/auth-request-login.strategy.ts | 5 ++++- .../src/common/login-strategies/password-login.strategy.ts | 5 ++++- .../src/common/login-strategies/sso-login.strategy.spec.ts | 4 ++-- .../auth/src/common/login-strategies/sso-login.strategy.ts | 2 +- .../login-strategies/user-api-login.strategy.spec.ts | 2 +- .../src/common/login-strategies/user-api-login.strategy.ts | 5 ++++- .../services/auth-request/auth-request.service.spec.ts | 2 +- .../common/services/auth-request/auth-request.service.ts | 2 +- .../src/common/services/pin/pin.service.implementation.ts | 1 + .../abstractions/master-password.service.abstraction.ts | 4 ++-- .../master-password/fake-master-password.service.ts | 4 ++-- .../services/master-password/master-password.service.ts | 7 +++++-- 16 files changed, 32 insertions(+), 16 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 2cc7c101d0bc..536a323451a4 100644 --- a/apps/web/src/app/auth/settings/change-password.component.ts +++ b/apps/web/src/app/auth/settings/change-password.component.ts @@ -194,7 +194,7 @@ export class ChangePasswordComponent HashPurpose.LocalAuthorization, ); - const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey); + const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey, userId); if (userKey == null) { this.toastService.showToast({ variant: "error", diff --git a/libs/angular/src/auth/components/lock.component.ts b/libs/angular/src/auth/components/lock.component.ts index 5fc8f51d5756..2f34e926e574 100644 --- a/libs/angular/src/auth/components/lock.component.ts +++ b/libs/angular/src/auth/components/lock.component.ts @@ -268,6 +268,7 @@ export class LockComponent implements OnInit, OnDestroy { const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey( response.masterKey, + userId, ); await this.setUserKeyAndContinue(userKey, userId, true); } diff --git a/libs/angular/src/auth/guards/auth.guard.ts b/libs/angular/src/auth/guards/auth.guard.ts index 1486b9b57d8a..facfbff6cf75 100644 --- a/libs/angular/src/auth/guards/auth.guard.ts +++ b/libs/angular/src/auth/guards/auth.guard.ts @@ -28,6 +28,7 @@ export const authGuard: CanActivateFn = async ( const masterPasswordService = inject(MasterPasswordServiceAbstraction); const authStatus = await authService.getAuthStatus(); + x; if (authStatus === AuthenticationStatus.LoggedOut) { messagingService.send("authBlocked", { url: routerState.url }); diff --git a/libs/auth/src/angular/lock/lock.component.ts b/libs/auth/src/angular/lock/lock.component.ts index 33d318ac0588..49474c545954 100644 --- a/libs/auth/src/angular/lock/lock.component.ts +++ b/libs/auth/src/angular/lock/lock.component.ts @@ -481,6 +481,7 @@ export class LockV2Component implements OnInit, OnDestroy { const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey( masterPasswordVerificationResponse.masterKey, + this.activeAccount.id, ); await this.setUserKeyAndContinue(userKey, true); } diff --git a/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts b/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts index ae0024d21813..32d0a4ec1199 100644 --- a/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts @@ -114,7 +114,10 @@ export class AuthRequestLoginStrategy extends LoginStrategy { private async trySetUserKeyWithMasterKey(userId: UserId): Promise { const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); if (masterKey) { - const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey); + const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey( + masterKey, + userId, + ); await this.cryptoService.setUserKey(userKey, userId); } } diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.ts b/libs/auth/src/common/login-strategies/password-login.strategy.ts index 7f73898ff6e7..3858d00b1c39 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.ts @@ -186,7 +186,10 @@ export class PasswordLoginStrategy extends LoginStrategy { const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); if (masterKey) { - const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey); + const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey( + masterKey, + userId, + ); await this.cryptoService.setUserKey(userKey, userId); } } diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts index f5de10766c08..94bbebf52caa 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts @@ -499,7 +499,7 @@ describe("SsoLoginStrategy", () => { expect(masterPasswordService.mock.decryptUserKeyWithMasterKey).toHaveBeenCalledWith( masterKey, - undefined, + userId, undefined, ); expect(cryptoService.setUserKey).toHaveBeenCalledWith(userKey, userId); @@ -555,7 +555,7 @@ describe("SsoLoginStrategy", () => { expect(masterPasswordService.mock.decryptUserKeyWithMasterKey).toHaveBeenCalledWith( masterKey, - undefined, + userId, undefined, ); expect(cryptoService.setUserKey).toHaveBeenCalledWith(userKey, userId); diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.ts index 5ddf7428d24f..22d2a7b93a93 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.ts @@ -338,7 +338,7 @@ export class SsoLoginStrategy extends LoginStrategy { return; } - const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey); + const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey, userId); await this.cryptoService.setUserKey(userKey, userId); } diff --git a/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts index d299a8e0ced8..d71e842cbb39 100644 --- a/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts @@ -216,7 +216,7 @@ describe("UserApiLoginStrategy", () => { expect(masterPasswordService.mock.decryptUserKeyWithMasterKey).toHaveBeenCalledWith( masterKey, - undefined, + userId, undefined, ); expect(cryptoService.setUserKey).toHaveBeenCalledWith(userKey, userId); diff --git a/libs/auth/src/common/login-strategies/user-api-login.strategy.ts b/libs/auth/src/common/login-strategies/user-api-login.strategy.ts index 3b112c79a0f5..90127e66ecda 100644 --- a/libs/auth/src/common/login-strategies/user-api-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/user-api-login.strategy.ts @@ -69,7 +69,10 @@ export class UserApiLoginStrategy extends LoginStrategy { if (response.apiUseKeyConnector) { const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); if (masterKey) { - const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey); + const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey( + masterKey, + userId, + ); await this.cryptoService.setUserKey(userKey, userId); } } diff --git a/libs/auth/src/common/services/auth-request/auth-request.service.spec.ts b/libs/auth/src/common/services/auth-request/auth-request.service.spec.ts index 58dbae6d789f..1c22fec0f2aa 100644 --- a/libs/auth/src/common/services/auth-request/auth-request.service.spec.ts +++ b/libs/auth/src/common/services/auth-request/auth-request.service.spec.ts @@ -200,7 +200,7 @@ describe("AuthRequestService", () => { ); expect(masterPasswordService.mock.decryptUserKeyWithMasterKey).toHaveBeenCalledWith( mockDecryptedMasterKey, - undefined, + mockUserId, undefined, ); expect(cryptoService.setUserKey).toHaveBeenCalledWith(mockDecryptedUserKey, mockUserId); diff --git a/libs/auth/src/common/services/auth-request/auth-request.service.ts b/libs/auth/src/common/services/auth-request/auth-request.service.ts index 51926d659832..0aae7a775379 100644 --- a/libs/auth/src/common/services/auth-request/auth-request.service.ts +++ b/libs/auth/src/common/services/auth-request/auth-request.service.ts @@ -150,7 +150,7 @@ export class AuthRequestService implements AuthRequestServiceAbstraction { ); // Decrypt and set user key in state - const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey); + const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey, userId); // Set masterKey + masterKeyHash in state after decryption (in case decryption fails) await this.masterPasswordService.setMasterKey(masterKey, userId); diff --git a/libs/auth/src/common/services/pin/pin.service.implementation.ts b/libs/auth/src/common/services/pin/pin.service.implementation.ts index 39bb80e0b73f..2a01802fa57a 100644 --- a/libs/auth/src/common/services/pin/pin.service.implementation.ts +++ b/libs/auth/src/common/services/pin/pin.service.implementation.ts @@ -418,6 +418,7 @@ export class PinService implements PinServiceAbstraction { const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey( masterKey, + userId, encUserKey ? new EncString(encUserKey) : undefined, ); diff --git a/libs/common/src/auth/abstractions/master-password.service.abstraction.ts b/libs/common/src/auth/abstractions/master-password.service.abstraction.ts index bd4d73a0f220..c3a0f135a06f 100644 --- a/libs/common/src/auth/abstractions/master-password.service.abstraction.ts +++ b/libs/common/src/auth/abstractions/master-password.service.abstraction.ts @@ -33,16 +33,16 @@ export abstract class MasterPasswordServiceAbstraction { /** * Decrypts the user key with the provided master key * @param masterKey The user's master key + * * @param userId The desired user * @param userKey The user's encrypted symmetric key - * @param userId The desired user * @throws If either the MasterKey or UserKey are not resolved, or if the UserKey encryption type * is neither AesCbc256_B64 nor AesCbc256_HmacSha256_B64 * @returns The user key */ abstract decryptUserKeyWithMasterKey: ( masterKey: MasterKey, + userId: string, userKey?: EncString, - userId?: string, ) => Promise; } diff --git a/libs/common/src/auth/services/master-password/fake-master-password.service.ts b/libs/common/src/auth/services/master-password/fake-master-password.service.ts index f57614f5d511..0357018e6152 100644 --- a/libs/common/src/auth/services/master-password/fake-master-password.service.ts +++ b/libs/common/src/auth/services/master-password/fake-master-password.service.ts @@ -64,9 +64,9 @@ export class FakeMasterPasswordService implements InternalMasterPasswordServiceA decryptUserKeyWithMasterKey( masterKey: MasterKey, + userId: string, userKey?: EncString, - userId?: string, ): Promise { - return this.mock.decryptUserKeyWithMasterKey(masterKey, userKey, userId); + return this.mock.decryptUserKeyWithMasterKey(masterKey, userId, userKey); } } diff --git a/libs/common/src/auth/services/master-password/master-password.service.ts b/libs/common/src/auth/services/master-password/master-password.service.ts index e20c8c00e6e0..3a565e1c7864 100644 --- a/libs/common/src/auth/services/master-password/master-password.service.ts +++ b/libs/common/src/auth/services/master-password/master-password.service.ts @@ -1,5 +1,7 @@ import { firstValueFrom, map, Observable } from "rxjs"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; + import { EncryptService } from "../../../platform/abstractions/encrypt.service"; import { KeyGenerationService } from "../../../platform/abstractions/key-generation.service"; import { StateService } from "../../../platform/abstractions/state.service"; @@ -55,6 +57,7 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr private stateService: StateService, private keyGenerationService: KeyGenerationService, private encryptService: EncryptService, + private logService: LogService, ) {} masterKey$(userId: UserId): Observable { @@ -149,10 +152,9 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr async decryptUserKeyWithMasterKey( masterKey: MasterKey, + userId: UserId, userKey?: EncString, - userId?: UserId, ): Promise { - userId ??= await firstValueFrom(this.stateProvider.activeUserId$); userKey ??= await this.getMasterKeyEncryptedUserKey(userId); masterKey ??= await firstValueFrom(this.masterKey$(userId)); @@ -185,6 +187,7 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr } if (decUserKey == null) { + this.logService.warning("Failed to decrypt user key with master key."); return null; } From 85f0ed3366be70a254394d9ab32614f9c9c1023b Mon Sep 17 00:00:00 2001 From: Todd Martin Date: Tue, 15 Oct 2024 13:06:59 -0400 Subject: [PATCH 2/4] Removed unintended extra character. --- libs/angular/src/auth/guards/auth.guard.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/libs/angular/src/auth/guards/auth.guard.ts b/libs/angular/src/auth/guards/auth.guard.ts index facfbff6cf75..1486b9b57d8a 100644 --- a/libs/angular/src/auth/guards/auth.guard.ts +++ b/libs/angular/src/auth/guards/auth.guard.ts @@ -28,7 +28,6 @@ export const authGuard: CanActivateFn = async ( const masterPasswordService = inject(MasterPasswordServiceAbstraction); const authStatus = await authService.getAuthStatus(); - x; if (authStatus === AuthenticationStatus.LoggedOut) { messagingService.send("authBlocked", { url: routerState.url }); From 0e823e1259e4bbf676dec8da64b11787a8865996 Mon Sep 17 00:00:00 2001 From: Todd Martin Date: Tue, 15 Oct 2024 13:29:45 -0400 Subject: [PATCH 3/4] Added dependency to LogService. --- apps/browser/src/background/main.background.ts | 1 + apps/cli/src/service-container/service-container.ts | 1 + libs/angular/src/services/jslib-services.module.ts | 8 +++++++- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 318b856b324c..d6caf07b22f8 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -622,6 +622,7 @@ export default class MainBackground { this.stateService, this.keyGenerationService, this.encryptService, + this.logService, ); this.i18nService = new I18nService(BrowserApi.getUILanguage(), this.globalStateProvider); diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index 7643683221ad..a4c7ecbe2e8b 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -395,6 +395,7 @@ export class ServiceContainer { this.stateService, this.keyGenerationService, this.encryptService, + this.logService, ); this.kdfConfigService = new KdfConfigService(this.stateProvider); diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index cc7af0c0b052..3f9d9c260eaa 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -912,7 +912,13 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: InternalMasterPasswordServiceAbstraction, useClass: MasterPasswordService, - deps: [StateProvider, StateServiceAbstraction, KeyGenerationServiceAbstraction, EncryptService], + deps: [ + StateProvider, + StateServiceAbstraction, + KeyGenerationServiceAbstraction, + EncryptService, + LogService, + ], }), safeProvider({ provide: MasterPasswordServiceAbstraction, From c8b7ae49597a849e727147bb548b79c22ce75cfd Mon Sep 17 00:00:00 2001 From: Todd Martin Date: Tue, 15 Oct 2024 15:47:50 -0400 Subject: [PATCH 4/4] Fixed unlock command. --- apps/cli/src/auth/commands/unlock.command.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/cli/src/auth/commands/unlock.command.ts b/apps/cli/src/auth/commands/unlock.command.ts index bebaa9460401..ac5b89fc1f75 100644 --- a/apps/cli/src/auth/commands/unlock.command.ts +++ b/apps/cli/src/auth/commands/unlock.command.ts @@ -68,7 +68,7 @@ export class UnlockCommand { return Response.error(e.message); } - const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey); + const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey, userId); await this.cryptoService.setUserKey(userKey, userId); if (await this.keyConnectorService.getConvertAccountRequired()) {