From dc75f19fcb52ba60603d25a4853ce6a94dc8324b Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Thu, 1 Jan 2026 15:12:25 +0100 Subject: [PATCH] Remove inividual user key states and migrate to account cryptographic state --- .../browser/src/background/main.background.ts | 9 +- .../src/popup/services/services.module.ts | 4 + .../service-container/service-container.ts | 13 +- .../src/app/services/services.module.ts | 1 + .../electron-key.service.spec.ts | 3 + .../key-management/electron-key.service.ts | 3 + ...initial-password.service.implementation.ts | 1 - ...fault-set-initial-password.service.spec.ts | 5 +- .../src/services/jslib-services.module.ts | 4 +- ...login-decryption-options.component.spec.ts | 7 - .../login-decryption-options.component.ts | 22 --- .../auth-request-login.strategy.spec.ts | 10 +- .../auth-request-login.strategy.ts | 12 +- .../login-strategies/login.strategy.spec.ts | 45 +---- .../common/login-strategies/login.strategy.ts | 7 +- .../password-login.strategy.spec.ts | 5 +- .../password-login.strategy.ts | 12 +- .../sso-login.strategy.spec.ts | 3 +- .../login-strategies/sso-login.strategy.ts | 16 +- .../user-api-login.strategy.spec.ts | 5 +- .../user-api-login.strategy.ts | 12 +- .../webauthn-login.strategy.spec.ts | 5 +- .../webauthn-login.strategy.ts | 12 +- .../services/key-connector.service.spec.ts | 20 -- .../services/key-connector.service.ts | 17 -- .../abstractions/security-state.service.ts | 7 - .../services/security-state.service.ts | 28 +-- .../state/security-state.state.ts | 12 -- .../services/key-state/user-key.state.spec.ts | 25 +-- .../services/key-state/user-key.state.ts | 29 --- .../sync/default-sync.service.spec.ts | 17 +- .../src/platform/sync/default-sync.service.ts | 29 +-- .../src/abstractions/key.service.ts | 31 --- libs/key-management/src/key.service.spec.ts | 187 +++++------------- libs/key-management/src/key.service.ts | 126 +++++------- ...symmetric-key-regeneration.service.spec.ts | 37 ++-- ...ser-asymmetric-key-regeneration.service.ts | 11 +- libs/state/src/state-migrations/migrate.ts | 6 +- ...-remove-user-encrypted-private-key.spec.ts | 54 +++++ .../75-remove-user-encrypted-private-key.ts | 64 ++++++ 40 files changed, 355 insertions(+), 561 deletions(-) delete mode 100644 libs/common/src/key-management/security-state/state/security-state.state.ts create mode 100644 libs/state/src/state-migrations/migrations/75-remove-user-encrypted-private-key.spec.ts create mode 100644 libs/state/src/state-migrations/migrations/75-remove-user-encrypted-private-key.ts diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 3cd8b59aabc..0da56cceaf2 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -661,6 +661,10 @@ export default class MainBackground { this.stateProvider, ); + this.accountCryptographicStateService = new DefaultAccountCryptographicStateService( + this.stateProvider, + ); + this.backgroundSyncService = new BackgroundSyncService(this.taskSchedulerService); this.backgroundSyncService.register(() => this.fullSync()); @@ -732,6 +736,7 @@ export default class MainBackground { this.accountService, this.stateProvider, this.kdfConfigService, + this.accountCryptographicStateService, ); const pinStateService = new PinStateService(this.stateProvider); @@ -847,10 +852,6 @@ export default class MainBackground { this.configService, ); - this.accountCryptographicStateService = new DefaultAccountCryptographicStateService( - this.stateProvider, - ); - this.keyConnectorService = new KeyConnectorService( this.accountService, this.masterPasswordService, diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 7a2ded5bb83..63d761fac2e 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -73,6 +73,7 @@ import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abs import { PhishingDetectionSettingsServiceAbstraction } from "@bitwarden/common/dirt/services/abstractions/phishing-detection-settings.service.abstraction"; import { PhishingDetectionSettingsService } from "@bitwarden/common/dirt/services/phishing-detection/phishing-detection-settings.service"; import { ClientType } from "@bitwarden/common/enums"; +import { AccountCryptographicStateService } from "@bitwarden/common/key-management/account-cryptography/account-cryptographic-state.service"; import { KeyGenerationService } from "@bitwarden/common/key-management/crypto"; import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; @@ -294,6 +295,7 @@ const safeProviders: SafeProvider[] = [ accountService: AccountServiceAbstraction, stateProvider: StateProvider, kdfConfigService: KdfConfigService, + accountCryptographicStateService: AccountCryptographicStateService, ) => { const keyService = new DefaultKeyService( masterPasswordService, @@ -306,6 +308,7 @@ const safeProviders: SafeProvider[] = [ accountService, stateProvider, kdfConfigService, + accountCryptographicStateService, ); new ContainerService(keyService, encryptService).attachToGlobal(self); return keyService; @@ -321,6 +324,7 @@ const safeProviders: SafeProvider[] = [ AccountServiceAbstraction, StateProvider, KdfConfigService, + AccountCryptographicStateService, ], }), safeProvider({ diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index d98b5f0a861..645c429e393 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -439,7 +439,13 @@ export class ServiceContainer { this.derivedStateProvider, ); - this.securityStateService = new DefaultSecurityStateService(this.stateProvider); + this.accountCryptographicStateService = new DefaultAccountCryptographicStateService( + this.stateProvider, + ); + + this.securityStateService = new DefaultSecurityStateService( + this.accountCryptographicStateService, + ); this.environmentService = new DefaultEnvironmentService( this.stateProvider, @@ -493,6 +499,7 @@ export class ServiceContainer { this.accountService, this.stateProvider, this.kdfConfigService, + this.accountCryptographicStateService, ); const pinStateService = new PinStateService(this.stateProvider); @@ -635,10 +642,6 @@ export class ServiceContainer { this.accountService, ); - this.accountCryptographicStateService = new DefaultAccountCryptographicStateService( - this.stateProvider, - ); - const sdkClientFactory = flagEnabled("sdk") ? new DefaultSdkClientFactory() : new NoopSdkClientFactory(); diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index 382b680efbc..9be8e0cbb85 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -338,6 +338,7 @@ const safeProviders: SafeProvider[] = [ BiometricStateService, KdfConfigService, DesktopBiometricsService, + AccountCryptographicStateService, ], }), safeProvider({ diff --git a/apps/desktop/src/key-management/electron-key.service.spec.ts b/apps/desktop/src/key-management/electron-key.service.spec.ts index c5e010b1766..f70dfccb19a 100644 --- a/apps/desktop/src/key-management/electron-key.service.spec.ts +++ b/apps/desktop/src/key-management/electron-key.service.spec.ts @@ -1,5 +1,6 @@ import { mock } from "jest-mock-extended"; +import { AccountCryptographicStateService } from "@bitwarden/common/key-management/account-cryptography/account-cryptographic-state.service"; import { KeyGenerationService } from "@bitwarden/common/key-management/crypto"; import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; @@ -36,6 +37,7 @@ describe("ElectronKeyService", () => { const kdfConfigService = mock(); const biometricStateService = mock(); const biometricService = mock(); + const accountCryptographicStateService = mock(); let stateProvider: FakeStateProvider; const mockUserId = Utils.newGuid() as UserId; @@ -62,6 +64,7 @@ describe("ElectronKeyService", () => { biometricStateService, kdfConfigService, biometricService, + accountCryptographicStateService, ); }); diff --git a/apps/desktop/src/key-management/electron-key.service.ts b/apps/desktop/src/key-management/electron-key.service.ts index 8de079a826b..ebea7e3ea9a 100644 --- a/apps/desktop/src/key-management/electron-key.service.ts +++ b/apps/desktop/src/key-management/electron-key.service.ts @@ -1,4 +1,5 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AccountCryptographicStateService } from "@bitwarden/common/key-management/account-cryptography/account-cryptographic-state.service"; import { KeyGenerationService } from "@bitwarden/common/key-management/crypto"; import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; @@ -33,6 +34,7 @@ export class ElectronKeyService extends DefaultKeyService { private biometricStateService: BiometricStateService, kdfConfigService: KdfConfigService, private biometricService: DesktopBiometricsService, + accountCryptographicStateService: AccountCryptographicStateService, ) { super( masterPasswordService, @@ -45,6 +47,7 @@ export class ElectronKeyService extends DefaultKeyService { accountService, stateProvider, kdfConfigService, + accountCryptographicStateService, ); } diff --git a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts index 2f5c43e2db9..10880f4dc08 100644 --- a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts +++ b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts @@ -180,7 +180,6 @@ export class DefaultSetInitialPasswordService implements SetInitialPasswordServi if (!keyPair[1].encryptedString) { throw new Error("encrypted private key not found. Could not set private key in state."); } - await this.keyService.setPrivateKey(keyPair[1].encryptedString, userId); await this.accountCryptographicStateService.setAccountCryptographicState( { V1: { diff --git a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts index af4505371d3..6c12f2c47cd 100644 --- a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts +++ b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts @@ -397,7 +397,6 @@ describe("DefaultSetInitialPasswordService", () => { // Assert expect(masterPasswordApiService.setPassword).toHaveBeenCalledWith(setPasswordRequest); - expect(keyService.setPrivateKey).toHaveBeenCalledWith(keyPair[1].encryptedString, userId); expect( accountCryptographicStateService.setAccountCryptographicState, ).toHaveBeenCalledWith( @@ -640,7 +639,9 @@ describe("DefaultSetInitialPasswordService", () => { // Assert expect(masterPasswordApiService.setPassword).toHaveBeenCalledWith(setPasswordRequest); - expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + expect( + accountCryptographicStateService.setAccountCryptographicState, + ).not.toHaveBeenCalled(); }); it("should set the local master key hash to state", async () => { diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 7899ff5281a..ca46dee0216 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -759,12 +759,13 @@ const safeProviders: SafeProvider[] = [ AccountServiceAbstraction, StateProvider, KdfConfigService, + AccountCryptographicStateService, ], }), safeProvider({ provide: SecurityStateService, useClass: DefaultSecurityStateService, - deps: [StateProvider], + deps: [AccountCryptographicStateService], }), safeProvider({ provide: RestrictedItemTypesService, @@ -1692,6 +1693,7 @@ const safeProviders: SafeProvider[] = [ SdkService, ApiServiceAbstraction, ConfigService, + AccountCryptographicStateService, ], }), safeProvider({ diff --git a/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.spec.ts b/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.spec.ts index 248eaa608af..1c7cdfac675 100644 --- a/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.spec.ts +++ b/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.spec.ts @@ -278,13 +278,6 @@ describe("LoginDecryptionOptionsComponent", () => { const expectedUserKey = new SymmetricCryptoKey(new Uint8Array(mockUserKeyBytes)); // Verify keys were set - expect(keyService.setPrivateKey).toHaveBeenCalledWith(mockPrivateKey, mockUserId); - expect(keyService.setSignedPublicKey).toHaveBeenCalledWith(mockSignedPublicKey, mockUserId); - expect(keyService.setUserSigningKey).toHaveBeenCalledWith(mockSigningKey, mockUserId); - expect(securityStateService.setAccountSecurityState).toHaveBeenCalledWith( - mockSecurityState, - mockUserId, - ); expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalledWith( expect.objectContaining({ V2: { diff --git a/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.ts b/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.ts index 06263ef7371..87117dba0fd 100644 --- a/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.ts +++ b/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.ts @@ -34,11 +34,6 @@ import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { AccountCryptographicStateService } from "@bitwarden/common/key-management/account-cryptography/account-cryptographic-state.service"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction"; import { SecurityStateService } from "@bitwarden/common/key-management/security-state/abstractions/security-state.service"; -import { - SignedPublicKey, - SignedSecurityState, - WrappedSigningKey, -} from "@bitwarden/common/key-management/types"; import { KeysRequest } from "@bitwarden/common/models/request/keys.request"; import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; @@ -322,23 +317,6 @@ export class LoginDecryptionOptionsComponent implements OnInit { register_result.account_cryptographic_state, userId, ); - // Legacy individual states - await this.keyService.setPrivateKey( - register_result.account_cryptographic_state.V2.private_key, - userId, - ); - await this.keyService.setSignedPublicKey( - register_result.account_cryptographic_state.V2.signed_public_key as SignedPublicKey, - userId, - ); - await this.keyService.setUserSigningKey( - register_result.account_cryptographic_state.V2.signing_key as WrappedSigningKey, - userId, - ); - await this.securityStateService.setAccountSecurityState( - register_result.account_cryptographic_state.V2.security_state as SignedSecurityState, - userId, - ); // TDE unlock await this.deviceTrustService.setDeviceKey( diff --git a/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts index 4703472d480..275f2d97aa4 100644 --- a/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts @@ -180,7 +180,10 @@ describe("AuthRequestLoginStrategy", () => { ); expect(keyService.setUserKey).toHaveBeenCalledWith(userKey, mockUserId); expect(deviceTrustService.trustDeviceIfRequired).toHaveBeenCalled(); - expect(keyService.setPrivateKey).toHaveBeenCalledWith(tokenResponse.privateKey, mockUserId); + expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalledWith( + { V1: { private_key: tokenResponse.privateKey } }, + mockUserId, + ); }); it("sets keys after a successful authentication when only userKey provided in login credentials", async () => { @@ -207,7 +210,10 @@ describe("AuthRequestLoginStrategy", () => { mockUserId, ); expect(keyService.setUserKey).toHaveBeenCalledWith(decUserKey, mockUserId); - expect(keyService.setPrivateKey).toHaveBeenCalledWith(tokenResponse.privateKey, mockUserId); + expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalledWith( + { V1: { private_key: tokenResponse.privateKey } }, + mockUserId, + ); // trustDeviceIfRequired should be called expect(deviceTrustService.trustDeviceIfRequired).not.toHaveBeenCalled(); 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 16af5fa77dc..66b9ee83919 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 @@ -120,20 +120,14 @@ export class AuthRequestLoginStrategy extends LoginStrategy { } } - protected override async setPrivateKey( + protected override async setAccountCryptographicState( response: IdentityTokenResponse, userId: UserId, ): Promise { - await this.keyService.setPrivateKey( - response.privateKey ?? (await this.createKeyPairForOldAccount(userId)), + await this.accountCryptographicStateService.setAccountCryptographicState( + response.accountKeysResponseModel.toWrappedAccountCryptographicState(), userId, ); - if (response.accountKeysResponseModel) { - await this.accountCryptographicStateService.setAccountCryptographicState( - response.accountKeysResponseModel.toWrappedAccountCryptographicState(), - userId, - ); - } } exportCache(): CacheData { diff --git a/libs/auth/src/common/login-strategies/login.strategy.spec.ts b/libs/auth/src/common/login-strategies/login.strategy.spec.ts index 113f9f3f0d9..ad945848116 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.spec.ts @@ -19,7 +19,6 @@ import { TwoFactorService } from "@bitwarden/common/auth/two-factor"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { AccountCryptographicStateService } from "@bitwarden/common/key-management/account-cryptography/account-cryptographic-state.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; -import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; import { FakeMasterPasswordService } from "@bitwarden/common/key-management/master-password/services/fake-master-password.service"; import { MasterKeyWrappedUserKey, @@ -38,15 +37,13 @@ import { MessagingService } from "@bitwarden/common/platform/abstractions/messag import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; import { PasswordStrengthServiceAbstraction, PasswordStrengthService, } from "@bitwarden/common/tools/password-strength"; -import { CsprngArray } from "@bitwarden/common/types/csprng"; import { UserId } from "@bitwarden/common/types/guid"; -import { UserKey, MasterKey } from "@bitwarden/common/types/key"; +import { UserKey } from "@bitwarden/common/types/key"; import { KdfConfigService, KeyService, PBKDF2KdfConfig } from "@bitwarden/key-management"; import { LoginStrategyServiceAbstraction } from "../abstractions"; @@ -109,6 +106,12 @@ export function identityTokenResponseFactory( token_type: "Bearer", MasterPasswordPolicy: masterPasswordPolicyResponse, UserDecryptionOptions: userDecryptionOptions || defaultUserDecryptionOptionsServerResponse, + AccountKeys: { + publicKeyEncryptionKeyPair: { + wrappedPrivateKey: privateKey, + publicKey: "PUBLIC_KEY", + }, + }, }); } @@ -201,19 +204,7 @@ describe("LoginStrategy", () => { }); describe("base class", () => { - const userKeyBytesLength = 64; - const masterKeyBytesLength = 64; - let userKey: UserKey; - let masterKey: MasterKey; - beforeEach(() => { - userKey = new SymmetricCryptoKey( - new Uint8Array(userKeyBytesLength).buffer as CsprngArray, - ) as UserKey; - masterKey = new SymmetricCryptoKey( - new Uint8Array(masterKeyBytesLength).buffer as CsprngArray, - ) as MasterKey; - const mockVaultTimeoutAction = VaultTimeoutAction.Lock; const mockVaultTimeoutActionBSub = new BehaviorSubject( mockVaultTimeoutAction, @@ -336,28 +327,6 @@ describe("LoginStrategy", () => { ); }); - it("makes a new public and private key for an old account", async () => { - const tokenResponse = identityTokenResponseFactory(); - tokenResponse.privateKey = null; - keyService.makeKeyPair.mockResolvedValue(["PUBLIC_KEY", new EncString("PRIVATE_KEY")]); - keyService.userKey$.mockReturnValue(new BehaviorSubject(userKey).asObservable()); - - apiService.postIdentityToken.mockResolvedValue(tokenResponse); - masterPasswordService.masterKeySubject.next(masterKey); - masterPasswordService.mock.decryptUserKeyWithMasterKey.mockResolvedValue(userKey); - - await passwordLoginStrategy.logIn(credentials); - - // User symmetric key must be set before the new RSA keypair is generated - expect(keyService.setUserKey).toHaveBeenCalled(); - expect(keyService.makeKeyPair).toHaveBeenCalled(); - expect(keyService.setUserKey.mock.invocationCallOrder[0]).toBeLessThan( - keyService.makeKeyPair.mock.invocationCallOrder[0], - ); - - expect(apiService.postAccountKeys).toHaveBeenCalled(); - }); - it("throws if userKey is CoseEncrypt0 (V2 encryption) in createKeyPairForOldAccount", async () => { keyService.userKey$.mockReturnValue( new BehaviorSubject({ diff --git a/libs/auth/src/common/login-strategies/login.strategy.ts b/libs/auth/src/common/login-strategies/login.strategy.ts index 16f5e1e4320..51bc164912a 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.ts @@ -265,7 +265,7 @@ export abstract class LoginStrategy { await this.setMasterKey(response, userId); await this.setUserKey(response, userId); - await this.setPrivateKey(response, userId); + await this.setAccountCryptographicState(response, userId); // This needs to run after the keys are set because it checks for the existence of the encrypted private key await this.processForceSetPasswordReason(response.forcePasswordReset, userId); @@ -283,7 +283,10 @@ export abstract class LoginStrategy { protected abstract setUserKey(response: IdentityTokenResponse, userId: UserId): Promise; - protected abstract setPrivateKey(response: IdentityTokenResponse, userId: UserId): Promise; + protected abstract setAccountCryptographicState( + response: IdentityTokenResponse, + userId: UserId, + ): Promise; // Old accounts used master key for encryption. We are forcing migrations but only need to // check on password logins diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts index 4188b779d81..00f43fd0423 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts @@ -216,7 +216,10 @@ describe("PasswordLoginStrategy", () => { userId, ); expect(keyService.setUserKey).toHaveBeenCalledWith(userKey, userId); - expect(keyService.setPrivateKey).toHaveBeenCalledWith(tokenResponse.privateKey, userId); + expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalledWith( + { V1: { private_key: tokenResponse.privateKey } }, + userId, + ); }); it("does not force the user to update their master password when there are no requirements", async () => { 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 63fff52194b..ac74f976e77 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.ts @@ -148,20 +148,14 @@ export class PasswordLoginStrategy extends LoginStrategy { } } - protected override async setPrivateKey( + protected override async setAccountCryptographicState( response: IdentityTokenResponse, userId: UserId, ): Promise { - await this.keyService.setPrivateKey( - response.privateKey ?? (await this.createKeyPairForOldAccount(userId)), + await this.accountCryptographicStateService.setAccountCryptographicState( + response.accountKeysResponseModel.toWrappedAccountCryptographicState(), userId, ); - if (response.accountKeysResponseModel) { - await this.accountCryptographicStateService.setAccountCryptographicState( - response.accountKeysResponseModel.toWrappedAccountCryptographicState(), - userId, - ); - } } protected override encryptionKeyMigrationRequired(response: IdentityTokenResponse): boolean { 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 484beb785d3..4f0a6bbf73f 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 @@ -196,13 +196,14 @@ describe("SsoLoginStrategy", () => { const tokenResponse = identityTokenResponseFactory(); tokenResponse.key = null; tokenResponse.privateKey = null; + tokenResponse.accountKeysResponseModel = null; apiService.postIdentityToken.mockResolvedValue(tokenResponse); await ssoLoginStrategy.logIn(credentials); expect(masterPasswordService.mock.setMasterKey).not.toHaveBeenCalled(); expect(keyService.setUserKey).not.toHaveBeenCalled(); - expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + expect(accountCryptographicStateService.setAccountCryptographicState).not.toHaveBeenCalled(); }); it("sets master key encrypted user key for existing SSO users", async () => { 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 a9d60fca21a..6a57d11e29d 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.ts @@ -335,7 +335,7 @@ export class SsoLoginStrategy extends LoginStrategy { await this.keyService.setUserKey(userKey, userId); } - protected override async setPrivateKey( + protected override async setAccountCryptographicState( tokenResponse: IdentityTokenResponse, userId: UserId, ): Promise { @@ -345,20 +345,6 @@ export class SsoLoginStrategy extends LoginStrategy { userId, ); } - - if (tokenResponse.hasMasterKeyEncryptedUserKey()) { - // User has masterKeyEncryptedUserKey, so set the userKeyEncryptedPrivateKey - // Note: new JIT provisioned SSO users will not yet have a user asymmetric key pair - // and so we don't want them falling into the createKeyPairForOldAccount flow - await this.keyService.setPrivateKey( - tokenResponse.privateKey ?? (await this.createKeyPairForOldAccount(userId)), - userId, - ); - } else if (tokenResponse.privateKey) { - // User doesn't have masterKeyEncryptedUserKey but they do have a userKeyEncryptedPrivateKey - // This is just existing TDE users or a TDE offboarder on an untrusted device - await this.keyService.setPrivateKey(tokenResponse.privateKey, userId); - } } exportCache(): CacheData { 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 02613e527ec..af3f295df57 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 @@ -188,7 +188,10 @@ describe("UserApiLoginStrategy", () => { tokenResponse.key, userId, ); - expect(keyService.setPrivateKey).toHaveBeenCalledWith(tokenResponse.privateKey, userId); + expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalledWith( + { V1: { private_key: tokenResponse.privateKey } }, + userId, + ); }); it("gets and sets the master key if Key Connector is enabled", async () => { 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 c5a9110d63e..0aaefdbcfe0 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 @@ -79,20 +79,14 @@ export class UserApiLoginStrategy extends LoginStrategy { } } - protected override async setPrivateKey( + protected override async setAccountCryptographicState( response: IdentityTokenResponse, userId: UserId, ): Promise { - await this.keyService.setPrivateKey( - response.privateKey ?? (await this.createKeyPairForOldAccount(userId)), + await this.accountCryptographicStateService.setAccountCryptographicState( + response.accountKeysResponseModel.toWrappedAccountCryptographicState(), userId, ); - if (response.accountKeysResponseModel) { - await this.accountCryptographicStateService.setAccountCryptographicState( - response.accountKeysResponseModel.toWrappedAccountCryptographicState(), - userId, - ); - } } // Overridden to save client ID and secret to token service diff --git a/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts index 2ae79f46d7c..365e018328f 100644 --- a/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts @@ -261,7 +261,10 @@ describe("WebAuthnLoginStrategy", () => { mockPrfPrivateKey, ); expect(keyService.setUserKey).toHaveBeenCalledWith(mockUserKey, userId); - expect(keyService.setPrivateKey).toHaveBeenCalledWith(idTokenResponse.privateKey, userId); + expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalledWith( + { V1: { private_key: idTokenResponse.privateKey } }, + userId, + ); // Master key and private key should not be set expect(masterPasswordService.mock.setMasterKey).not.toHaveBeenCalled(); diff --git a/libs/auth/src/common/login-strategies/webauthn-login.strategy.ts b/libs/auth/src/common/login-strategies/webauthn-login.strategy.ts index 77a881b5964..e0ba4e1e265 100644 --- a/libs/auth/src/common/login-strategies/webauthn-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/webauthn-login.strategy.ts @@ -99,20 +99,14 @@ export class WebAuthnLoginStrategy extends LoginStrategy { } } - protected override async setPrivateKey( + protected override async setAccountCryptographicState( response: IdentityTokenResponse, userId: UserId, ): Promise { - await this.keyService.setPrivateKey( - response.privateKey ?? (await this.createKeyPairForOldAccount(userId)), + await this.accountCryptographicStateService.setAccountCryptographicState( + response.accountKeysResponseModel.toWrappedAccountCryptographicState(), userId, ); - if (response.accountKeysResponseModel) { - await this.accountCryptographicStateService.setAccountCryptographicState( - response.accountKeysResponseModel.toWrappedAccountCryptographicState(), - userId, - ); - } } exportCache(): CacheData { diff --git a/libs/common/src/key-management/key-connector/services/key-connector.service.spec.ts b/libs/common/src/key-management/key-connector/services/key-connector.service.spec.ts index b8ee5d7df64..329ae547160 100644 --- a/libs/common/src/key-management/key-connector/services/key-connector.service.spec.ts +++ b/libs/common/src/key-management/key-connector/services/key-connector.service.spec.ts @@ -524,14 +524,6 @@ describe("KeyConnectorService", () => { }, mockUserId, ); - expect(keyService.setPrivateKey).toHaveBeenCalledWith(mockPrivateKey, mockUserId); - expect(keyService.setUserSigningKey).toHaveBeenCalledWith(mockSigningKey, mockUserId); - expect(securityStateService.setAccountSecurityState).toHaveBeenCalledWith( - mockSecurityState, - mockUserId, - ); - expect(keyService.setSignedPublicKey).toHaveBeenCalledWith(mockSignedPublicKey, mockUserId); - expect(await firstValueFrom(conversionState.state$)).toBeNull(); }); @@ -557,10 +549,6 @@ describe("KeyConnectorService", () => { expect( accountCryptographicStateService.setAccountCryptographicState, ).not.toHaveBeenCalled(); - expect(keyService.setPrivateKey).not.toHaveBeenCalled(); - expect(keyService.setUserSigningKey).not.toHaveBeenCalled(); - expect(securityStateService.setAccountSecurityState).not.toHaveBeenCalled(); - expect(keyService.setSignedPublicKey).not.toHaveBeenCalled(); }); it("should throw error when account cryptographic state is not V2", async () => { @@ -595,10 +583,6 @@ describe("KeyConnectorService", () => { expect( accountCryptographicStateService.setAccountCryptographicState, ).not.toHaveBeenCalled(); - expect(keyService.setPrivateKey).not.toHaveBeenCalled(); - expect(keyService.setUserSigningKey).not.toHaveBeenCalled(); - expect(securityStateService.setAccountSecurityState).not.toHaveBeenCalled(); - expect(keyService.setSignedPublicKey).not.toHaveBeenCalled(); }); it("should throw error when post_keys_for_key_connector_registration fails", async () => { @@ -625,10 +609,6 @@ describe("KeyConnectorService", () => { expect( accountCryptographicStateService.setAccountCryptographicState, ).not.toHaveBeenCalled(); - expect(keyService.setPrivateKey).not.toHaveBeenCalled(); - expect(keyService.setUserSigningKey).not.toHaveBeenCalled(); - expect(securityStateService.setAccountSecurityState).not.toHaveBeenCalled(); - expect(keyService.setSignedPublicKey).not.toHaveBeenCalled(); }); }); diff --git a/libs/common/src/key-management/key-connector/services/key-connector.service.ts b/libs/common/src/key-management/key-connector/services/key-connector.service.ts index 751f1ec8594..606e9c3bfcd 100644 --- a/libs/common/src/key-management/key-connector/services/key-connector.service.ts +++ b/libs/common/src/key-management/key-connector/services/key-connector.service.ts @@ -38,7 +38,6 @@ import { KeyGenerationService } from "../../crypto"; import { EncString } from "../../crypto/models/enc-string"; import { InternalMasterPasswordServiceAbstraction } from "../../master-password/abstractions/master-password.service.abstraction"; import { SecurityStateService } from "../../security-state/abstractions/security-state.service"; -import { SignedPublicKey, SignedSecurityState, WrappedSigningKey } from "../../types"; import { KeyConnectorService as KeyConnectorServiceAbstraction } from "../abstractions/key-connector.service"; import { KeyConnectorDomainConfirmation } from "../models/key-connector-domain-confirmation"; import { KeyConnectorUserKeyRequest } from "../models/key-connector-user-key.request"; @@ -246,22 +245,6 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { result.account_cryptographic_state, userId, ); - // Legacy states - await this.keyService.setPrivateKey(result.account_cryptographic_state.V2.private_key, userId); - await this.keyService.setUserSigningKey( - result.account_cryptographic_state.V2.signing_key as WrappedSigningKey, - userId, - ); - await this.securityStateService.setAccountSecurityState( - result.account_cryptographic_state.V2.security_state as SignedSecurityState, - userId, - ); - if (result.account_cryptographic_state.V2.signed_public_key != null) { - await this.keyService.setSignedPublicKey( - result.account_cryptographic_state.V2.signed_public_key as SignedPublicKey, - userId, - ); - } } async convertNewSsoUserToKeyConnectorV1( diff --git a/libs/common/src/key-management/security-state/abstractions/security-state.service.ts b/libs/common/src/key-management/security-state/abstractions/security-state.service.ts index 466095c2f45..108f012f076 100644 --- a/libs/common/src/key-management/security-state/abstractions/security-state.service.ts +++ b/libs/common/src/key-management/security-state/abstractions/security-state.service.ts @@ -11,11 +11,4 @@ export abstract class SecurityStateService { * must be used. This security state is validated on initialization of the SDK. */ abstract accountSecurityState$(userId: UserId): Observable; - /** - * Sets the security state for the provided user. - */ - abstract setAccountSecurityState( - accountSecurityState: SignedSecurityState, - userId: UserId, - ): Promise; } diff --git a/libs/common/src/key-management/security-state/services/security-state.service.ts b/libs/common/src/key-management/security-state/services/security-state.service.ts index 789d5171072..5e6bafb9e4e 100644 --- a/libs/common/src/key-management/security-state/services/security-state.service.ts +++ b/libs/common/src/key-management/security-state/services/security-state.service.ts @@ -1,26 +1,28 @@ -import { Observable } from "rxjs"; +import { map, Observable } from "rxjs"; -import { StateProvider } from "@bitwarden/common/platform/state"; import { UserId } from "@bitwarden/common/types/guid"; +import { AccountCryptographicStateService } from "../../account-cryptography/account-cryptographic-state.service"; import { SignedSecurityState } from "../../types"; import { SecurityStateService } from "../abstractions/security-state.service"; -import { ACCOUNT_SECURITY_STATE } from "../state/security-state.state"; export class DefaultSecurityStateService implements SecurityStateService { - constructor(protected stateProvider: StateProvider) {} + constructor(private accountCryptographicStateService: AccountCryptographicStateService) {} // Emits the provided user's security state, or null if there is no security state present for the user. accountSecurityState$(userId: UserId): Observable { - return this.stateProvider.getUserState$(ACCOUNT_SECURITY_STATE, userId); - } + return this.accountCryptographicStateService.accountCryptographicState$(userId).pipe( + map((cryptographicState) => { + if (cryptographicState == null) { + return null; + } - // Sets the security state for the provided user. - // This is not yet validated, and is only validated upon SDK initialization. - async setAccountSecurityState( - accountSecurityState: SignedSecurityState, - userId: UserId, - ): Promise { - await this.stateProvider.setUserState(ACCOUNT_SECURITY_STATE, accountSecurityState, userId); + if ("V2" in cryptographicState) { + return cryptographicState.V2.security_state as SignedSecurityState; + } else { + return null; + } + }), + ); } } diff --git a/libs/common/src/key-management/security-state/state/security-state.state.ts b/libs/common/src/key-management/security-state/state/security-state.state.ts deleted file mode 100644 index e471ef17d76..00000000000 --- a/libs/common/src/key-management/security-state/state/security-state.state.ts +++ /dev/null @@ -1,12 +0,0 @@ -import { CRYPTO_DISK, UserKeyDefinition } from "@bitwarden/common/platform/state"; - -import { SignedSecurityState } from "../../types"; - -export const ACCOUNT_SECURITY_STATE = new UserKeyDefinition( - CRYPTO_DISK, - "accountSecurityState", - { - deserializer: (obj) => obj, - clearOn: ["logout"], - }, -); diff --git a/libs/common/src/platform/services/key-state/user-key.state.spec.ts b/libs/common/src/platform/services/key-state/user-key.state.spec.ts index 2ea3c31bc1b..7fe6618ff72 100644 --- a/libs/common/src/platform/services/key-state/user-key.state.spec.ts +++ b/libs/common/src/platform/services/key-state/user-key.state.spec.ts @@ -1,13 +1,4 @@ -import { EncryptedString, EncString } from "../../../key-management/crypto/models/enc-string"; -import { EncryptionType } from "../../enums"; -import { Utils } from "../../misc/utils"; - -import { USER_ENCRYPTED_PRIVATE_KEY, USER_EVER_HAD_USER_KEY } from "./user-key.state"; - -function makeEncString(data?: string) { - data ??= Utils.newGuid(); - return new EncString(EncryptionType.AesCbc256_HmacSha256_B64, data, "test", "test"); -} +import { USER_EVER_HAD_USER_KEY } from "./user-key.state"; describe("Ever had user key", () => { const sut = USER_EVER_HAD_USER_KEY; @@ -20,17 +11,3 @@ describe("Ever had user key", () => { expect(result).toEqual(everHadUserKey); }); }); - -describe("Encrypted private key", () => { - const sut = USER_ENCRYPTED_PRIVATE_KEY; - - it("should deserialize encrypted private key", () => { - const encryptedPrivateKey = makeEncString().encryptedString; - - const result = sut.deserializer( - JSON.parse(JSON.stringify(encryptedPrivateKey as unknown)) as unknown as EncryptedString, - ); - - expect(result).toEqual(encryptedPrivateKey); - }); -}); diff --git a/libs/common/src/platform/services/key-state/user-key.state.ts b/libs/common/src/platform/services/key-state/user-key.state.ts index 64577768c8d..58560a8bf0b 100644 --- a/libs/common/src/platform/services/key-state/user-key.state.ts +++ b/libs/common/src/platform/services/key-state/user-key.state.ts @@ -1,5 +1,3 @@ -import { EncryptedString } from "../../../key-management/crypto/models/enc-string"; -import { SignedPublicKey, WrappedSigningKey } from "../../../key-management/types"; import { UserKey } from "../../../types/key"; import { SymmetricCryptoKey } from "../../models/domain/symmetric-crypto-key"; import { CRYPTO_DISK, CRYPTO_MEMORY, UserKeyDefinition } from "../../state"; @@ -13,34 +11,7 @@ export const USER_EVER_HAD_USER_KEY = new UserKeyDefinition( }, ); -export const USER_ENCRYPTED_PRIVATE_KEY = new UserKeyDefinition( - CRYPTO_DISK, - "privateKey", - { - deserializer: (obj) => obj, - clearOn: ["logout"], - }, -); - export const USER_KEY = new UserKeyDefinition(CRYPTO_MEMORY, "userKey", { deserializer: (obj) => SymmetricCryptoKey.fromJSON(obj) as UserKey, clearOn: ["logout", "lock"], }); - -export const USER_KEY_ENCRYPTED_SIGNING_KEY = new UserKeyDefinition( - CRYPTO_DISK, - "userSigningKey", - { - deserializer: (obj) => obj, - clearOn: ["logout"], - }, -); - -export const USER_SIGNED_PUBLIC_KEY = new UserKeyDefinition( - CRYPTO_DISK, - "userSignedPublicKey", - { - deserializer: (obj) => obj, - clearOn: ["logout"], - }, -); diff --git a/libs/common/src/platform/sync/default-sync.service.spec.ts b/libs/common/src/platform/sync/default-sync.service.spec.ts index fc83954ee7d..ec67a71703b 100644 --- a/libs/common/src/platform/sync/default-sync.service.spec.ts +++ b/libs/common/src/platform/sync/default-sync.service.spec.ts @@ -199,7 +199,10 @@ describe("DefaultSyncService", () => { new EncString("encryptedUserKey"), user1, ); - expect(keyService.setPrivateKey).toHaveBeenCalledWith("privateKey", user1); + expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalledWith( + { V1: { private_key: "privateKey" } }, + user1, + ); expect(keyService.setProviderKeys).toHaveBeenCalledWith([], user1); expect(keyService.setOrgKeys).toHaveBeenCalledWith([], [], user1); }); @@ -242,7 +245,10 @@ describe("DefaultSyncService", () => { new EncString("encryptedUserKey"), user1, ); - expect(keyService.setPrivateKey).toHaveBeenCalledWith("wrappedPrivateKey", user1); + expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalledWith( + { V1: { private_key: "wrappedPrivateKey" } }, + user1, + ); expect(keyService.setProviderKeys).toHaveBeenCalledWith([], user1); expect(keyService.setOrgKeys).toHaveBeenCalledWith([], [], user1); }); @@ -293,12 +299,7 @@ describe("DefaultSyncService", () => { new EncString("encryptedUserKey"), user1, ); - expect(keyService.setPrivateKey).toHaveBeenCalledWith("wrappedPrivateKey", user1); - expect(keyService.setUserSigningKey).toHaveBeenCalledWith("wrappedSigningKey", user1); - expect(securityStateService.setAccountSecurityState).toHaveBeenCalledWith( - "securityState", - user1, - ); + expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalled(); expect(keyService.setProviderKeys).toHaveBeenCalledWith([], user1); expect(keyService.setOrgKeys).toHaveBeenCalledWith([], [], user1); }); diff --git a/libs/common/src/platform/sync/default-sync.service.ts b/libs/common/src/platform/sync/default-sync.service.ts index fdd05927b50..1cd887210a1 100644 --- a/libs/common/src/platform/sync/default-sync.service.ts +++ b/libs/common/src/platform/sync/default-sync.service.ts @@ -14,6 +14,7 @@ import { SecurityStateService } from "@bitwarden/common/key-management/security- // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports import { KdfConfigService, KeyService } from "@bitwarden/key-management"; +import { EncString as SdkEncString } from "@bitwarden/sdk-internal"; // FIXME: remove `src` and fix import // eslint-disable-next-line no-restricted-imports @@ -245,29 +246,15 @@ export class DefaultSyncService extends CoreSyncService { response.accountKeys.toWrappedAccountCryptographicState(), response.id, ); - - // V1 and V2 users - await this.keyService.setPrivateKey( - response.accountKeys.publicKeyEncryptionKeyPair.wrappedPrivateKey, + } else { + await this.accountCryptographicStateService.setAccountCryptographicState( + { + V1: { + private_key: response.privateKey as SdkEncString, + }, + }, response.id, ); - // V2 users only - if (response.accountKeys.isV2Encryption()) { - await this.keyService.setUserSigningKey( - response.accountKeys.signatureKeyPair.wrappedSigningKey, - response.id, - ); - await this.securityStateService.setAccountSecurityState( - response.accountKeys.securityState.securityState, - response.id, - ); - await this.keyService.setSignedPublicKey( - response.accountKeys.publicKeyEncryptionKeyPair.signedPublicKey, - response.id, - ); - } - } else { - await this.keyService.setPrivateKey(response.privateKey, response.id); } await this.keyService.setProviderKeys(response.providers, response.id); await this.keyService.setOrgKeys( diff --git a/libs/key-management/src/abstractions/key.service.ts b/libs/key-management/src/abstractions/key.service.ts index feb4a38ac27..1c3b17e6229 100644 --- a/libs/key-management/src/abstractions/key.service.ts +++ b/libs/key-management/src/abstractions/key.service.ts @@ -68,20 +68,6 @@ export abstract class KeyService { * @param userId The desired user */ abstract setUserKey(key: UserKey, userId: UserId): Promise; - /** - * Sets the provided user keys and stores any other necessary versions - * (such as auto, biometrics, or pin). - * Also sets the user's encrypted private key in storage and - * clears the decrypted private key from memory - * Note: does not clear the private key if null is provided - * - * @throws Error when userKey, encPrivateKey or userId is null - * @throws UserPrivateKeyDecryptionFailedError when the userKey cannot decrypt encPrivateKey - * @param userKey The user key to set - * @param encPrivateKey An encrypted private key - * @param userId The desired user - */ - abstract setUserKeys(userKey: UserKey, encPrivateKey: string, userId: UserId): Promise; /** * Gets the user key from memory and sets it again, * kicking off a refresh of any additional keys @@ -263,21 +249,6 @@ export abstract class KeyService { * @returns The new encrypted OrgKey | ProviderKey and the decrypted key itself */ abstract makeOrgKey(userId: UserId): Promise<[EncString, T]>; - /** - * Sets the user's encrypted private key in storage and - * clears the decrypted private key from memory - * Note: does not clear the private key if null is provided - * @param encPrivateKey An encrypted private key - */ - abstract setPrivateKey(encPrivateKey: string, userId: UserId): Promise; - /** - * Sets the user's encrypted signing key in storage - * In contrast to the private key, the decrypted signing key - * is not stored in memory outside of the SDK. - * @param encryptedSigningKey An encrypted signing key - * @param userId The user id of the user to set the signing key for - */ - abstract setUserSigningKey(encryptedSigningKey: WrappedSigningKey, userId: UserId): Promise; /** * Gets an observable stream of the given users decrypted private key, will emit null if the user @@ -429,7 +400,5 @@ export abstract class KeyService { */ abstract validateUserKey(key: UserKey, userId: UserId): Promise; - abstract setSignedPublicKey(signedPublicKey: SignedPublicKey, userId: UserId): Promise; - abstract userSignedPublicKey$(userId: UserId): Observable; } diff --git a/libs/key-management/src/key.service.spec.ts b/libs/key-management/src/key.service.spec.ts index c0af62fe6e9..56de3aadb93 100644 --- a/libs/key-management/src/key.service.spec.ts +++ b/libs/key-management/src/key.service.spec.ts @@ -2,6 +2,7 @@ import { mock } from "jest-mock-extended"; import { BehaviorSubject, bufferCount, firstValueFrom, lastValueFrom, of, take } from "rxjs"; import { EncryptedOrganizationKeyData } from "@bitwarden/common/admin-console/models/data/encrypted-organization-key.data"; +import { AccountCryptographicStateService } from "@bitwarden/common/key-management/account-cryptography/account-cryptographic-state.service"; import { KeyGenerationService } from "@bitwarden/common/key-management/crypto"; import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; @@ -10,7 +11,7 @@ import { EncryptedString, } from "@bitwarden/common/key-management/crypto/models/enc-string"; import { FakeMasterPasswordService } from "@bitwarden/common/key-management/master-password/services/fake-master-password.service"; -import { UnsignedPublicKey, WrappedSigningKey } from "@bitwarden/common/key-management/types"; +import { UnsignedPublicKey } from "@bitwarden/common/key-management/types"; import { VaultTimeoutStringType } from "@bitwarden/common/key-management/vault-timeout"; import { VAULT_TIMEOUT } from "@bitwarden/common/key-management/vault-timeout/services/vault-timeout-settings.state"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -22,10 +23,8 @@ import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/sym import { USER_ENCRYPTED_ORGANIZATION_KEYS } from "@bitwarden/common/platform/services/key-state/org-keys.state"; import { USER_ENCRYPTED_PROVIDER_KEYS } from "@bitwarden/common/platform/services/key-state/provider-keys.state"; import { - USER_ENCRYPTED_PRIVATE_KEY, USER_EVER_HAD_USER_KEY, USER_KEY, - USER_KEY_ENCRYPTED_SIGNING_KEY, } from "@bitwarden/common/platform/services/key-state/user-key.state"; import { UserKeyDefinition } from "@bitwarden/common/platform/state"; import { @@ -49,7 +48,6 @@ import { } from "@bitwarden/common/types/key"; import { KdfConfigService } from "./abstractions/kdf-config.service"; -import { UserPrivateKeyDecryptionFailedError } from "./abstractions/key.service"; import { DefaultKeyService } from "./key.service"; import { KdfConfig } from "./models/kdf-config"; @@ -63,6 +61,7 @@ describe("keyService", () => { const logService = mock(); const stateService = mock(); const kdfConfigService = mock(); + const accountCryptographicStateService = mock(); let stateProvider: FakeStateProvider; const mockUserId = Utils.newGuid() as UserId; @@ -87,6 +86,7 @@ describe("keyService", () => { accountService, stateProvider, kdfConfigService, + accountCryptographicStateService, ); }); @@ -257,70 +257,6 @@ describe("keyService", () => { }); }); - describe("setUserKeys", () => { - let mockUserKey: UserKey; - let mockEncPrivateKey: EncryptedString; - let everHadUserKeyState: FakeSingleUserState; - - beforeEach(() => { - const mockRandomBytes = new Uint8Array(64) as CsprngArray; - mockUserKey = new SymmetricCryptoKey(mockRandomBytes) as UserKey; - mockEncPrivateKey = new SymmetricCryptoKey(mockRandomBytes).toString() as EncryptedString; - everHadUserKeyState = stateProvider.singleUser.getFake(mockUserId, USER_EVER_HAD_USER_KEY); - - // Initialize storage - everHadUserKeyState.nextState(null); - - // Mock private key decryption - encryptService.unwrapDecapsulationKey.mockResolvedValue(mockRandomBytes); - }); - - it("throws if userKey is null", async () => { - await expect( - keyService.setUserKeys(null as unknown as UserKey, mockEncPrivateKey, mockUserId), - ).rejects.toThrow("No userKey provided."); - }); - - it("throws if encPrivateKey is null", async () => { - await expect( - keyService.setUserKeys(mockUserKey, null as unknown as EncryptedString, mockUserId), - ).rejects.toThrow("No encPrivateKey provided."); - }); - - it("throws if userId is null", async () => { - await expect( - keyService.setUserKeys(mockUserKey, mockEncPrivateKey, null as unknown as UserId), - ).rejects.toThrow("No userId provided."); - }); - - it("throws if encPrivateKey cannot be decrypted with the userKey", async () => { - encryptService.unwrapDecapsulationKey.mockResolvedValue(null); - - await expect( - keyService.setUserKeys(mockUserKey, mockEncPrivateKey, mockUserId), - ).rejects.toThrow(UserPrivateKeyDecryptionFailedError); - }); - - // We already have tests for setUserKey, so we just need to test that the correct methods are called - it("calls setUserKey with the userKey and userId", async () => { - const setUserKeySpy = jest.spyOn(keyService, "setUserKey"); - - await keyService.setUserKeys(mockUserKey, mockEncPrivateKey, mockUserId); - - expect(setUserKeySpy).toHaveBeenCalledWith(mockUserKey, mockUserId); - }); - - // We already have tests for setPrivateKey, so we just need to test that the correct methods are called - // TODO: Move those tests into here since `setPrivateKey` will be converted to a private method - it("calls setPrivateKey with the encPrivateKey and userId", async () => { - const setEncryptedPrivateKeySpy = jest.spyOn(keyService, "setPrivateKey"); - - await keyService.setUserKeys(mockUserKey, mockEncPrivateKey, mockUserId); - - expect(setEncryptedPrivateKeySpy).toHaveBeenCalledWith(mockEncPrivateKey, mockUserId); - }); - }); - describe("makeSendKey", () => { const mockRandomBytes = new Uint8Array(16) as CsprngArray; it("calls keyGenerationService with expected hard coded parameters", async () => { @@ -367,22 +303,19 @@ describe("keyService", () => { }, ); - describe.each([ - USER_ENCRYPTED_ORGANIZATION_KEYS, - USER_ENCRYPTED_PROVIDER_KEYS, - USER_ENCRYPTED_PRIVATE_KEY, - USER_KEY_ENCRYPTED_SIGNING_KEY, - USER_KEY, - ])("key removal", (key: UserKeyDefinition) => { - it(`clears ${key.key} for the specified user when specified`, async () => { - const userId = "someOtherUser" as UserId; - await keyService.clearKeys(userId); + describe.each([USER_ENCRYPTED_ORGANIZATION_KEYS, USER_ENCRYPTED_PROVIDER_KEYS, USER_KEY])( + "key removal", + (key: UserKeyDefinition) => { + it(`clears ${key.key} for the specified user when specified`, async () => { + const userId = "someOtherUser" as UserId; + await keyService.clearKeys(userId); - const encryptedOrgKeyState = stateProvider.singleUser.getFake(userId, key); - expect(encryptedOrgKeyState.nextMock).toHaveBeenCalledTimes(1); - expect(encryptedOrgKeyState.nextMock).toHaveBeenCalledWith(null); - }); - }); + const encryptedOrgKeyState = stateProvider.singleUser.getFake(userId, key); + expect(encryptedOrgKeyState.nextMock).toHaveBeenCalledTimes(1); + expect(encryptedOrgKeyState.nextMock).toHaveBeenCalledWith(null); + }); + }, + ); }); describe("userPrivateKey$", () => { @@ -395,9 +328,9 @@ describe("keyService", () => { mockEncryptedPrivateKey = makeEncString("encryptedPrivateKey").encryptedString!; mockUserPrivateKey = makeStaticByteArray(10, 1); stateProvider.singleUser.getFake(mockUserId, USER_KEY).nextState(mockUserKey); - stateProvider.singleUser - .getFake(mockUserId, USER_ENCRYPTED_PRIVATE_KEY) - .nextState(mockEncryptedPrivateKey); + accountCryptographicStateService.accountCryptographicState$.mockReturnValue( + of({ V1: { private_key: mockEncryptedPrivateKey } }), + ); encryptService.unwrapDecapsulationKey.mockResolvedValue(mockUserPrivateKey); }); @@ -431,7 +364,7 @@ describe("keyService", () => { }); it("returns null if encrypted private key is not set", async () => { - stateProvider.singleUser.getFake(mockUserId, USER_ENCRYPTED_PRIVATE_KEY).nextState(null); + accountCryptographicStateService.accountCryptographicState$.mockReturnValue(of(null)); const result = await firstValueFrom(keyService.userPrivateKey$(mockUserId)); @@ -441,6 +374,13 @@ describe("keyService", () => { it("reacts to changes in user key or encrypted private key", async () => { // Initial state: both set + const accountStateSubject = new BehaviorSubject({ + V1: { private_key: mockEncryptedPrivateKey }, + }); + accountCryptographicStateService.accountCryptographicState$.mockReturnValue( + accountStateSubject.asObservable(), + ); + let result = await firstValueFrom(keyService.userPrivateKey$(mockUserId)); expect(result).toEqual(mockUserPrivateKey); @@ -454,7 +394,7 @@ describe("keyService", () => { // Restore user key, remove encrypted private key stateProvider.singleUser.getFake(mockUserId, USER_KEY).nextState(mockUserKey); - stateProvider.singleUser.getFake(mockUserId, USER_ENCRYPTED_PRIVATE_KEY).nextState(null); + accountStateSubject.next(null); result = await firstValueFrom(keyService.userPrivateKey$(mockUserId)); @@ -462,52 +402,16 @@ describe("keyService", () => { }); }); - describe("userSigningKey$", () => { - it("returns the signing key when the user has a signing key set", async () => { - const fakeSigningKey = "" as WrappedSigningKey; - const fakeSigningKeyState = stateProvider.singleUser.getFake( - mockUserId, - USER_KEY_ENCRYPTED_SIGNING_KEY, - ); - fakeSigningKeyState.nextState(fakeSigningKey); - - const signingKey = await firstValueFrom(keyService.userSigningKey$(mockUserId)); - - expect(signingKey).toEqual(fakeSigningKey); - }); - - it("returns null when the user does not have a signing key set", async () => { - const signingKey = await firstValueFrom(keyService.userSigningKey$(mockUserId)); - - expect(signingKey).toBeFalsy(); - }); - }); - - describe("setUserSigningKey", () => { - it("throws if the signing key is null", async () => { - await expect(keyService.setUserSigningKey(null as any, mockUserId)).rejects.toThrow( - "No user signing key provided.", - ); - }); - it("throws if the userId is null", async () => { - await expect( - keyService.setUserSigningKey("" as WrappedSigningKey, null as unknown as UserId), - ).rejects.toThrow("No userId provided."); - }); - it("sets the signing key for the user", async () => { - const fakeSigningKey = "" as WrappedSigningKey; - const fakeSigningKeyState = stateProvider.singleUser.getFake( - mockUserId, - USER_KEY_ENCRYPTED_SIGNING_KEY, - ); - fakeSigningKeyState.nextState(null); - await keyService.setUserSigningKey(fakeSigningKey, mockUserId); - expect(fakeSigningKeyState.nextMock).toHaveBeenCalledTimes(1); - expect(fakeSigningKeyState.nextMock).toHaveBeenCalledWith(fakeSigningKey); - }); - }); - describe("cipherDecryptionKeys$", () => { + let accountStateSubject: BehaviorSubject; + + beforeEach(() => { + accountStateSubject = new BehaviorSubject(null); + accountCryptographicStateService.accountCryptographicState$.mockReturnValue( + accountStateSubject.asObservable(), + ); + }); + function fakePrivateKeyDecryption(encryptedPrivateKey: EncString, key: SymmetricCryptoKey) { const output = new Uint8Array(64); output.set(encryptedPrivateKey.dataBytes); @@ -544,11 +448,9 @@ describe("keyService", () => { } if ("encryptedPrivateKey" in keys) { - const userEncryptedPrivateKey = stateProvider.singleUser.getFake( - mockUserId, - USER_ENCRYPTED_PRIVATE_KEY, - ); - userEncryptedPrivateKey.nextState(keys.encryptedPrivateKey!.encryptedString!); + accountStateSubject.next({ + V1: { private_key: keys.encryptedPrivateKey!.encryptedString! }, + }); } if ("orgKeys" in keys) { @@ -1240,17 +1142,16 @@ describe("keyService", () => { it("successfully initializes account with new keys", async () => { const keyCreationSize = 512; - const privateKeyState = stateProvider.singleUser.getFake( - mockUserId, - USER_ENCRYPTED_PRIVATE_KEY, - ); const result = await keyService.initAccount(mockUserId); expect(keyGenerationService.createKey).toHaveBeenCalledWith(keyCreationSize); expect(keyService.makeKeyPair).toHaveBeenCalledWith(userKey); expect(keyService.setUserKey).toHaveBeenCalledWith(userKey, mockUserId); - expect(privateKeyState.nextMock).toHaveBeenCalledWith(mockPrivateKey.encryptedString); + expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalledWith( + { V1: { private_key: mockPrivateKey.encryptedString } }, + mockUserId, + ); expect(result).toEqual({ userKey: userKey, publicKey: mockPublicKey, diff --git a/libs/key-management/src/key.service.ts b/libs/key-management/src/key.service.ts index 621a8135d1e..5c97a76f0fd 100644 --- a/libs/key-management/src/key.service.ts +++ b/libs/key-management/src/key.service.ts @@ -20,6 +20,7 @@ import { ProfileOrganizationResponse } from "@bitwarden/common/admin-console/mod import { ProfileProviderOrganizationResponse } from "@bitwarden/common/admin-console/models/response/profile-provider-organization.response"; import { ProfileProviderResponse } from "@bitwarden/common/admin-console/models/response/profile-provider.response"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AccountCryptographicStateService } from "@bitwarden/common/key-management/account-cryptography/account-cryptographic-state.service"; import { KeyGenerationService } from "@bitwarden/common/key-management/crypto"; import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; @@ -42,11 +43,8 @@ import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/sym import { USER_ENCRYPTED_ORGANIZATION_KEYS } from "@bitwarden/common/platform/services/key-state/org-keys.state"; import { USER_ENCRYPTED_PROVIDER_KEYS } from "@bitwarden/common/platform/services/key-state/provider-keys.state"; import { - USER_ENCRYPTED_PRIVATE_KEY, USER_EVER_HAD_USER_KEY, USER_KEY, - USER_KEY_ENCRYPTED_SIGNING_KEY, - USER_SIGNED_PUBLIC_KEY, } from "@bitwarden/common/platform/services/key-state/user-key.state"; import { StateProvider } from "@bitwarden/common/platform/state"; import { CsprngArray } from "@bitwarden/common/types/csprng"; @@ -60,12 +58,12 @@ import { UserPrivateKey, UserPublicKey, } from "@bitwarden/common/types/key"; +import { WrappedAccountCryptographicState } from "@bitwarden/sdk-internal"; import { KdfConfigService } from "./abstractions/kdf-config.service"; import { CipherDecryptionKeys, KeyService as KeyServiceAbstraction, - UserPrivateKeyDecryptionFailedError, } from "./abstractions/key.service"; import { KdfConfig } from "./models/kdf-config"; @@ -90,6 +88,7 @@ export class DefaultKeyService implements KeyServiceAbstraction { protected accountService: AccountService, protected stateProvider: StateProvider, protected kdfConfigService: KdfConfigService, + protected accountCryptographyStateService: AccountCryptographicStateService, ) { this.activeUserOrgKeys$ = this.stateProvider.activeUserId$.pipe( switchMap((userId) => (userId != null ? this.orgKeys$(userId) : NEVER)), @@ -121,30 +120,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { } } - async setUserKeys( - userKey: UserKey, - encPrivateKey: EncryptedString, - userId: UserId, - ): Promise { - if (userKey == null) { - throw new Error("No userKey provided. Lock the user to clear the key"); - } - if (encPrivateKey == null) { - throw new Error("No encPrivateKey provided."); - } - if (userId == null) { - throw new Error("No userId provided."); - } - - const decryptedPrivateKey = await this.decryptPrivateKey(encPrivateKey, userKey); - if (decryptedPrivateKey == null) { - throw new UserPrivateKeyDecryptionFailedError(); - } - - await this.setUserKey(userKey, userId); - await this.setPrivateKey(encPrivateKey, userId); - } - async refreshAdditionalKeys(userId: UserId): Promise { if (userId == null) { throw new Error("UserId is required."); @@ -479,16 +454,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { return [encShareKey, shareKey as T]; } - async setPrivateKey(encPrivateKey: EncryptedString, userId: UserId): Promise { - if (encPrivateKey == null) { - return; - } - - await this.stateProvider - .getUser(userId, USER_ENCRYPTED_PRIVATE_KEY) - .update(() => encPrivateKey); - } - async getFingerprint(fingerprintMaterial: string, publicKey: Uint8Array): Promise { if (publicKey == null) { throw new Error("Public key is required to generate a fingerprint."); @@ -515,18 +480,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { return [publicB64, privateEnc]; } - /** - * Clears the user's key pair - * @param userId The desired user - */ - private async clearKeyPair(userId: UserId): Promise { - await this.stateProvider.setUserState(USER_ENCRYPTED_PRIVATE_KEY, null, userId); - } - - private async clearSigningKey(userId: UserId): Promise { - await this.stateProvider.setUserState(USER_KEY_ENCRYPTED_SIGNING_KEY, null, userId); - } - async makeSendKey(keyMaterial: CsprngArray): Promise { return await this.keyGenerationService.deriveKeyFromMaterial( keyMaterial, @@ -548,8 +501,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { await this.clearUserKey(userId); await this.clearOrgKeys(userId); await this.clearProviderKeys(userId); - await this.clearKeyPair(userId); - await this.clearSigningKey(userId); await this.stateProvider.setUserState(USER_EVER_HAD_USER_KEY, null, userId); } @@ -595,9 +546,7 @@ export class DefaultKeyService implements KeyServiceAbstraction { } try { - const encPrivateKey = await firstValueFrom( - this.stateProvider.getUser(userId, USER_ENCRYPTED_PRIVATE_KEY).state$, - ); + const encPrivateKey = await firstValueFrom(this.userEncryptedPrivateKey$(userId)); if (encPrivateKey == null) { return false; @@ -655,9 +604,14 @@ export class DefaultKeyService implements KeyServiceAbstraction { } await this.setUserKey(userKey, userId); - await this.stateProvider - .getUser(userId, USER_ENCRYPTED_PRIVATE_KEY) - .update(() => privateKey.encryptedString!); + await this.accountCryptographyStateService.setAccountCryptographicState( + { + V1: { + private_key: privateKey.encryptedString, + }, + }, + userId, + ); return { userKey, @@ -801,7 +755,20 @@ export class DefaultKeyService implements KeyServiceAbstraction { } userEncryptedPrivateKey$(userId: UserId): Observable { - return this.stateProvider.getUser(userId, USER_ENCRYPTED_PRIVATE_KEY).state$; + return this.accountCryptographyStateService.accountCryptographicState$(userId).pipe( + map((state: WrappedAccountCryptographicState | null) => { + if (state == null) { + return null; + } + if ("V2" in state) { + return state.V2.private_key; + } else if ("V1" in state) { + return state.V1.private_key; + } else { + return null; + } + }), + ); } private userPrivateKeyHelper$(userId: UserId) { @@ -812,7 +779,7 @@ export class DefaultKeyService implements KeyServiceAbstraction { return of(null); } - return this.stateProvider.getUser(userId, USER_ENCRYPTED_PRIVATE_KEY).state$.pipe( + return this.userEncryptedPrivateKey$(userId).pipe( switchMap(async (encryptedPrivateKey) => { try { return await this.decryptPrivateKey(encryptedPrivateKey, userKey); @@ -879,23 +846,17 @@ export class DefaultKeyService implements KeyServiceAbstraction { ); } - async setUserSigningKey(userSigningKey: WrappedSigningKey, userId: UserId): Promise { - if (userSigningKey == null) { - throw new Error("No user signing key provided."); - } - if (userId == null) { - throw new Error("No userId provided."); - } - await this.stateProvider.setUserState(USER_KEY_ENCRYPTED_SIGNING_KEY, userSigningKey, userId); - } - userSigningKey$(userId: UserId): Observable { - return this.stateProvider.getUser(userId, USER_KEY_ENCRYPTED_SIGNING_KEY).state$.pipe( - map((encryptedSigningKey) => { - if (encryptedSigningKey == null) { + return this.accountCryptographyStateService.accountCryptographicState$(userId).pipe( + map((state: WrappedAccountCryptographicState | null) => { + if (state == null) { + return null; + } + if ("V2" in state) { + return state.V2.signing_key as WrappedSigningKey; + } else { return null; } - return encryptedSigningKey as WrappedSigningKey; }), ); } @@ -1017,11 +978,18 @@ export class DefaultKeyService implements KeyServiceAbstraction { ); } - async setSignedPublicKey(signedPublicKey: SignedPublicKey, userId: UserId): Promise { - await this.stateProvider.setUserState(USER_SIGNED_PUBLIC_KEY, signedPublicKey, userId); - } - userSignedPublicKey$(userId: UserId): Observable { - return this.stateProvider.getUserState$(USER_SIGNED_PUBLIC_KEY, userId); + return this.accountCryptographyStateService.accountCryptographicState$(userId).pipe( + map((state: WrappedAccountCryptographicState | null) => { + if (state == null) { + return null; + } + if ("V2" in state) { + return state.V2.signed_public_key as SignedPublicKey; + } else { + return null; + } + }), + ); } } diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts index 92e5240a187..3e771b242bb 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts @@ -2,6 +2,7 @@ import { MockProxy, mock } from "jest-mock-extended"; import { of, throwError } from "rxjs"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { AccountCryptographicStateService } from "@bitwarden/common/key-management/account-cryptography/account-cryptographic-state.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { EncryptedString } from "@bitwarden/common/key-management/crypto/models/enc-string"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; @@ -70,6 +71,7 @@ describe("regenerateIfNeeded", () => { let apiService: MockProxy; let configService: MockProxy; let encryptService: MockProxy; + let accountCryptographicStateService: MockProxy; beforeEach(() => { keyService = mock(); @@ -80,6 +82,7 @@ describe("regenerateIfNeeded", () => { apiService = mock(); configService = mock(); encryptService = mock(); + accountCryptographicStateService = mock(); sut = new DefaultUserAsymmetricKeysRegenerationService( keyService, @@ -89,6 +92,7 @@ describe("regenerateIfNeeded", () => { sdkService, apiService, configService, + accountCryptographicStateService, ); configService.getFeatureFlag.mockResolvedValue(true); @@ -131,7 +135,7 @@ describe("regenerateIfNeeded", () => { expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, ).not.toHaveBeenCalled(); - expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + expect(accountCryptographicStateService.setAccountCryptographicState).not.toHaveBeenCalled(); }); it("should not regenerate when private key is decryptable and valid", async () => { @@ -146,7 +150,7 @@ describe("regenerateIfNeeded", () => { expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, ).not.toHaveBeenCalled(); - expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + expect(accountCryptographicStateService.setAccountCryptographicState).not.toHaveBeenCalled(); }); it("should not regenerate when user symmetric key is unavailable", async () => { @@ -162,7 +166,7 @@ describe("regenerateIfNeeded", () => { expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, ).not.toHaveBeenCalled(); - expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + expect(accountCryptographicStateService.setAccountCryptographicState).not.toHaveBeenCalled(); }); it("should not regenerate when user's encrypted private key is unavailable", async () => { @@ -180,7 +184,7 @@ describe("regenerateIfNeeded", () => { expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, ).not.toHaveBeenCalled(); - expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + expect(accountCryptographicStateService.setAccountCryptographicState).not.toHaveBeenCalled(); }); it("should not regenerate when user's public key is unavailable", async () => { @@ -196,7 +200,7 @@ describe("regenerateIfNeeded", () => { expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, ).not.toHaveBeenCalled(); - expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + expect(accountCryptographicStateService.setAccountCryptographicState).not.toHaveBeenCalled(); }); it("should regenerate when private key is decryptable and invalid", async () => { @@ -211,7 +215,7 @@ describe("regenerateIfNeeded", () => { expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, ).toHaveBeenCalled(); - expect(keyService.setPrivateKey).toHaveBeenCalled(); + expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalled(); }); it("should not set private key on known API error", async () => { @@ -230,7 +234,7 @@ describe("regenerateIfNeeded", () => { expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, ).toHaveBeenCalled(); - expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + expect(accountCryptographicStateService.setAccountCryptographicState).not.toHaveBeenCalled(); }); it("should not set private key on unknown API error", async () => { @@ -249,7 +253,7 @@ describe("regenerateIfNeeded", () => { expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, ).toHaveBeenCalled(); - expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + expect(accountCryptographicStateService.setAccountCryptographicState).not.toHaveBeenCalled(); }); it("should regenerate when private key is not decryptable and user key is valid", async () => { @@ -265,7 +269,7 @@ describe("regenerateIfNeeded", () => { expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, ).toHaveBeenCalled(); - expect(keyService.setPrivateKey).toHaveBeenCalled(); + expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalled(); }); it("should not regenerate when private key is not decryptable and user key is invalid", async () => { @@ -283,7 +287,7 @@ describe("regenerateIfNeeded", () => { expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, ).not.toHaveBeenCalled(); - expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + expect(accountCryptographicStateService.setAccountCryptographicState).not.toHaveBeenCalled(); }); it("should not regenerate when private key is not decryptable and no ciphers to check", async () => { @@ -299,7 +303,7 @@ describe("regenerateIfNeeded", () => { expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, ).not.toHaveBeenCalled(); - expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + expect(accountCryptographicStateService.setAccountCryptographicState).not.toHaveBeenCalled(); }); it("should regenerate when private key is not decryptable and invalid and user key is valid", async () => { @@ -315,7 +319,7 @@ describe("regenerateIfNeeded", () => { expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, ).toHaveBeenCalled(); - expect(keyService.setPrivateKey).toHaveBeenCalled(); + expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalled(); }); it("should not regenerate when private key is not decryptable and invalid and user key is invalid", async () => { @@ -333,7 +337,7 @@ describe("regenerateIfNeeded", () => { expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, ).not.toHaveBeenCalled(); - expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + expect(accountCryptographicStateService.setAccountCryptographicState).not.toHaveBeenCalled(); }); it("should not regenerate when private key is not decryptable and invalid and no ciphers to check", async () => { @@ -349,7 +353,7 @@ describe("regenerateIfNeeded", () => { expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, ).not.toHaveBeenCalled(); - expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + expect(accountCryptographicStateService.setAccountCryptographicState).not.toHaveBeenCalled(); }); it("should not regenerate when userKey type is CoseEncrypt0 (V2 encryption)", async () => { @@ -364,7 +368,7 @@ describe("regenerateIfNeeded", () => { expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, ).not.toHaveBeenCalled(); - expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + expect(accountCryptographicStateService.setAccountCryptographicState).not.toHaveBeenCalled(); expect(logService.error).toHaveBeenCalledWith( "[UserAsymmetricKeyRegeneration] Cannot regenerate asymmetric keys for accounts on V2 encryption.", ); @@ -382,6 +386,7 @@ describe("regenerateUserPublicKeyEncryptionKeyPair", () => { let sdkService: MockSdkService; let apiService: MockProxy; let configService: MockProxy; + let accountCryptographicStateService: MockProxy; beforeEach(() => { keyService = mock(); @@ -391,6 +396,7 @@ describe("regenerateUserPublicKeyEncryptionKeyPair", () => { sdkService = new MockSdkService(); apiService = mock(); configService = mock(); + accountCryptographicStateService = mock(); sut = new DefaultUserAsymmetricKeysRegenerationService( keyService, @@ -400,6 +406,7 @@ describe("regenerateUserPublicKeyEncryptionKeyPair", () => { sdkService, apiService, configService, + accountCryptographicStateService, ); }); diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts index 36bf9c8a421..0faf848f723 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts @@ -2,6 +2,7 @@ import { combineLatest, firstValueFrom, map } from "rxjs"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { AccountCryptographicStateService } from "@bitwarden/common/key-management/account-cryptography/account-cryptographic-state.service"; import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -24,6 +25,7 @@ export class DefaultUserAsymmetricKeysRegenerationService implements UserAsymmet private sdkService: SdkService, private apiService: ApiService, private configService: ConfigService, + private accountCryptographyStateService: AccountCryptographicStateService, ) {} async regenerateIfNeeded(userId: UserId): Promise { @@ -161,7 +163,14 @@ export class DefaultUserAsymmetricKeysRegenerationService implements UserAsymmet return; } - await this.keyService.setPrivateKey(makeKeyPairResponse.userKeyEncryptedPrivateKey, userId); + await this.accountCryptographyStateService.setAccountCryptographicState( + { + V1: { + private_key: makeKeyPairResponse.userKeyEncryptedPrivateKey, + }, + }, + userId, + ); this.logService.info( "[UserAsymmetricKeyRegeneration] User's asymmetric keys successfully regenerated.", ); diff --git a/libs/state/src/state-migrations/migrate.ts b/libs/state/src/state-migrations/migrate.ts index a051c25695a..42cb4aa3ae1 100644 --- a/libs/state/src/state-migrations/migrate.ts +++ b/libs/state/src/state-migrations/migrate.ts @@ -71,12 +71,13 @@ import { RemoveNewCustomizationOptionsCalloutDismissed } from "./migrations/71-r import { RemoveAccountDeprovisioningBannerDismissed } from "./migrations/72-remove-account-deprovisioning-banner-dismissed"; import { AddMasterPasswordUnlockData } from "./migrations/73-add-master-password-unlock-data"; import { RemoveLegacyPin } from "./migrations/74-remove-legacy-pin"; +import { RemoveUserEncryptedPrivateKey } from "./migrations/75-remove-user-encrypted-private-key"; import { MoveStateVersionMigrator } from "./migrations/8-move-state-version"; import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-settings-to-global"; import { MinVersionMigrator } from "./migrations/min-version"; export const MIN_VERSION = 3; -export const CURRENT_VERSION = 74; +export const CURRENT_VERSION = 75; export type MinVersion = typeof MIN_VERSION; export function createMigrationBuilder() { @@ -152,7 +153,8 @@ export function createMigrationBuilder() { .with(RemoveNewCustomizationOptionsCalloutDismissed, 70, 71) .with(RemoveAccountDeprovisioningBannerDismissed, 71, 72) .with(AddMasterPasswordUnlockData, 72, 73) - .with(RemoveLegacyPin, 73, CURRENT_VERSION); + .with(RemoveLegacyPin, 73, 74) + .with(RemoveUserEncryptedPrivateKey, 74, CURRENT_VERSION); } export async function currentVersion( diff --git a/libs/state/src/state-migrations/migrations/75-remove-user-encrypted-private-key.spec.ts b/libs/state/src/state-migrations/migrations/75-remove-user-encrypted-private-key.spec.ts new file mode 100644 index 00000000000..7c1169276ce --- /dev/null +++ b/libs/state/src/state-migrations/migrations/75-remove-user-encrypted-private-key.spec.ts @@ -0,0 +1,54 @@ +import { runMigrator } from "../migration-helper.spec"; +import { IRREVERSIBLE } from "../migrator"; + +import { RemoveUserEncryptedPrivateKey } from "./75-remove-user-encrypted-private-key"; + +describe("RemoveUserEncryptedPrivateKey", () => { + const sut = new RemoveUserEncryptedPrivateKey(74, 75); + + describe("migrate", () => { + it("deletes user encrypted private key, signing key, and signed public key from all users", async () => { + const output = await runMigrator(sut, { + global_account_accounts: { + user1: { + email: "user1@email.com", + name: "User 1", + emailVerified: true, + }, + user2: { + email: "user2@email.com", + name: "User 2", + emailVerified: true, + }, + }, + user_user1_CRYPTO_DISK_privateKey: "abc", + user_user2_CRYPTO_DISK_privateKey: "def", + user_user1_CRYPTO_DISK_userSigningKey: "sign1", + user_user2_CRYPTO_DISK_userSigningKey: "sign2", + user_user1_CRYPTO_DISK_userSignedPublicKey: "pub1", + user_user2_CRYPTO_DISK_userSignedPublicKey: "pub2", + }); + + expect(output).toEqual({ + global_account_accounts: { + user1: { + email: "user1@email.com", + name: "User 1", + emailVerified: true, + }, + user2: { + email: "user2@email.com", + name: "User 2", + emailVerified: true, + }, + }, + }); + }); + }); + + describe("rollback", () => { + it("is irreversible", async () => { + await expect(runMigrator(sut, {}, "rollback")).rejects.toThrow(IRREVERSIBLE); + }); + }); +}); diff --git a/libs/state/src/state-migrations/migrations/75-remove-user-encrypted-private-key.ts b/libs/state/src/state-migrations/migrations/75-remove-user-encrypted-private-key.ts new file mode 100644 index 00000000000..5c5857b22ff --- /dev/null +++ b/libs/state/src/state-migrations/migrations/75-remove-user-encrypted-private-key.ts @@ -0,0 +1,64 @@ +export const accountSecurityState: KeyDefinitionLike = { + key: "accountSecurityState", + stateDefinition: { + name: "CRYPTO_DISK", + }, +}; +import { KeyDefinitionLike, MigrationHelper } from "../migration-helper"; +import { IRREVERSIBLE, Migrator } from "../migrator"; + +type ExpectedAccountType = NonNullable; + +export const userEncryptedPrivateKey: KeyDefinitionLike = { + key: "privateKey", + stateDefinition: { + name: "CRYPTO_DISK", + }, +}; + +export const userKeyEncryptedSigningKey: KeyDefinitionLike = { + key: "userSigningKey", + stateDefinition: { + name: "CRYPTO_DISK", + }, +}; + +export const userSignedPublicKey: KeyDefinitionLike = { + key: "userSignedPublicKey", + stateDefinition: { + name: "CRYPTO_DISK", + }, +}; + +export class RemoveUserEncryptedPrivateKey extends Migrator<74, 75> { + async migrate(helper: MigrationHelper): Promise { + const accounts = await helper.getAccounts(); + async function migrateAccount(userId: string, account: ExpectedAccountType): Promise { + // Remove privateKey + const key = await helper.getFromUser(userId, userEncryptedPrivateKey); + if (key != null) { + await helper.removeFromUser(userId, userEncryptedPrivateKey); + } + // Remove userSigningKey + const signingKey = await helper.getFromUser(userId, userKeyEncryptedSigningKey); + if (signingKey != null) { + await helper.removeFromUser(userId, userKeyEncryptedSigningKey); + } + // Remove userSignedPublicKey + const signedPubKey = await helper.getFromUser(userId, userSignedPublicKey); + if (signedPubKey != null) { + await helper.removeFromUser(userId, userSignedPublicKey); + } + // Remove accountSecurityState + const accountSecurity = await helper.getFromUser(userId, accountSecurityState); + if (accountSecurity != null) { + await helper.removeFromUser(userId, accountSecurityState); + } + } + await Promise.all([...accounts.map(({ userId, account }) => migrateAccount(userId, account))]); + } + + async rollback(helper: MigrationHelper): Promise { + throw IRREVERSIBLE; + } +}