diff --git a/libs/common/spec/services/encrypt.service.spec.ts b/libs/common/spec/services/encrypt.service.spec.ts index 74a95f9acb..9567d8fbb2 100644 --- a/libs/common/spec/services/encrypt.service.spec.ts +++ b/libs/common/spec/services/encrypt.service.spec.ts @@ -4,6 +4,7 @@ import { CryptoFunctionService } from "@bitwarden/common/abstractions/cryptoFunc import { LogService } from "@bitwarden/common/abstractions/log.service"; import { EncryptionType } from "@bitwarden/common/enums/encryptionType"; import { EncArrayBuffer } from "@bitwarden/common/models/domain/encArrayBuffer"; +import { EncString } from "@bitwarden/common/models/domain/encString"; import { SymmetricCryptoKey } from "@bitwarden/common/models/domain/symmetricCryptoKey"; import { EncryptService } from "@bitwarden/common/services/encrypt.service"; @@ -160,4 +161,28 @@ describe("EncryptService", () => { expect(cryptoFunctionService.aesDecrypt).not.toHaveBeenCalled(); }); }); + + describe("resolveLegacyKey", () => { + it("creates a legacy key if required", async () => { + const key = new SymmetricCryptoKey(makeStaticByteArray(32), EncryptionType.AesCbc256_B64); + const encString = mock(); + encString.encryptionType = EncryptionType.AesCbc128_HmacSha256_B64; + + const actual = encryptService.resolveLegacyKey(key, encString); + + const expected = new SymmetricCryptoKey(key.key, EncryptionType.AesCbc128_HmacSha256_B64); + expect(actual).toEqual(expected); + }); + + it("does not create a legacy key if not required", async () => { + const encType = EncryptionType.AesCbc256_HmacSha256_B64; + const key = new SymmetricCryptoKey(makeStaticByteArray(64), encType); + const encString = mock(); + encString.encryptionType = encType; + + const actual = encryptService.resolveLegacyKey(key, encString); + + expect(actual).toEqual(key); + }); + }); }); diff --git a/libs/common/spec/services/stateMigration.service.spec.ts b/libs/common/spec/services/stateMigration.service.spec.ts index 7ac12f51e3..e306e64e29 100644 --- a/libs/common/spec/services/stateMigration.service.spec.ts +++ b/libs/common/spec/services/stateMigration.service.spec.ts @@ -127,4 +127,20 @@ describe("State Migration Service", () => { expect(migratedAccount).toEqual(expectedAccount); }); }); + + describe("StateVersion 5 to 6 migration", () => { + it("deletes account.keys.legacyEtmKey value", async () => { + const accountVersion5 = new Account({ + keys: { + legacyEtmKey: "legacy key", + }, + } as any); + + const migratedAccount = await (stateMigrationService as any).migrateAccountFrom5To6( + accountVersion5 + ); + + expect(migratedAccount.keys.legacyEtmKey).toBeUndefined(); + }); + }); }); diff --git a/libs/common/src/abstractions/abstractEncrypt.service.ts b/libs/common/src/abstractions/abstractEncrypt.service.ts index cad893765e..c683f22d33 100644 --- a/libs/common/src/abstractions/abstractEncrypt.service.ts +++ b/libs/common/src/abstractions/abstractEncrypt.service.ts @@ -12,4 +12,5 @@ export abstract class AbstractEncryptService { ) => Promise; abstract decryptToUtf8: (encString: EncString, key: SymmetricCryptoKey) => Promise; abstract decryptToBytes: (encThing: IEncrypted, key: SymmetricCryptoKey) => Promise; + abstract resolveLegacyKey: (key: SymmetricCryptoKey, encThing: IEncrypted) => SymmetricCryptoKey; } diff --git a/libs/common/src/abstractions/crypto.service.ts b/libs/common/src/abstractions/crypto.service.ts index de11e1f5a6..842d9e1a7c 100644 --- a/libs/common/src/abstractions/crypto.service.ts +++ b/libs/common/src/abstractions/crypto.service.ts @@ -29,6 +29,7 @@ export abstract class CryptoService { getOrgKeys: () => Promise>; getOrgKey: (orgId: string) => Promise; getProviderKey: (providerId: string) => Promise; + getKeyForUserEncryption: (key?: SymmetricCryptoKey) => Promise; hasKey: () => Promise; hasKeyInMemory: (userId?: string) => Promise; hasKeyStored: (keySuffix?: KeySuffixOptions, userId?: string) => Promise; diff --git a/libs/common/src/abstractions/state.service.ts b/libs/common/src/abstractions/state.service.ts index b88926635c..cf4ddca7ee 100644 --- a/libs/common/src/abstractions/state.service.ts +++ b/libs/common/src/abstractions/state.service.ts @@ -248,8 +248,6 @@ export abstract class StateService { setLastActive: (value: number, options?: StorageOptions) => Promise; getLastSync: (options?: StorageOptions) => Promise; setLastSync: (value: string, options?: StorageOptions) => Promise; - getLegacyEtmKey: (options?: StorageOptions) => Promise; - setLegacyEtmKey: (value: SymmetricCryptoKey, options?: StorageOptions) => Promise; getLocalData: (options?: StorageOptions) => Promise; setLocalData: (value: string, options?: StorageOptions) => Promise; getLocale: (options?: StorageOptions) => Promise; diff --git a/libs/common/src/enums/stateVersion.ts b/libs/common/src/enums/stateVersion.ts index cc36f53f57..2b6cbbf474 100644 --- a/libs/common/src/enums/stateVersion.ts +++ b/libs/common/src/enums/stateVersion.ts @@ -4,5 +4,6 @@ export enum StateVersion { Three = 3, // Fix migration of users' premium status Four = 4, // Fix 'Never Lock' option by removing stale data Five = 5, // Migrate to new storage of encrypted organization keys - Latest = Five, + Six = 6, // Delete account.keys.legacyEtmKey property + Latest = Six, } diff --git a/libs/common/src/models/domain/account.ts b/libs/common/src/models/domain/account.ts index ff4b60345d..0c9c32eddc 100644 --- a/libs/common/src/models/domain/account.ts +++ b/libs/common/src/models/domain/account.ts @@ -82,7 +82,6 @@ export class AccountKeys { Map >(); privateKey?: EncryptionPair = new EncryptionPair(); - legacyEtmKey?: SymmetricCryptoKey; publicKey?: ArrayBuffer; publicKeySerialized?: string; apiKeyClientSecret?: string; diff --git a/libs/common/src/services/crypto.service.ts b/libs/common/src/services/crypto.service.ts index 9f78296f24..9b024cf7f2 100644 --- a/libs/common/src/services/crypto.service.ts +++ b/libs/common/src/services/crypto.service.ts @@ -337,7 +337,6 @@ export class CryptoService implements CryptoServiceAbstraction { async clearKey(clearSecretStorage = true, userId?: string): Promise { await this.stateService.setCryptoMasterKey(null, { userId: userId }); - await this.stateService.setLegacyEtmKey(null, { userId: userId }); if (clearSecretStorage) { await this.clearSecretKeyStore(userId); } @@ -497,7 +496,7 @@ export class CryptoService implements CryptoServiceAbstraction { } async makeEncKey(key: SymmetricCryptoKey): Promise<[SymmetricCryptoKey, EncString]> { - const theKey = await this.getKeyForEncryption(key); + const theKey = await this.getKeyForUserEncryption(key); const encKey = await this.cryptoFunctionService.randomBytes(64); return this.buildEncKey(theKey, encKey); } @@ -512,13 +511,21 @@ export class CryptoService implements CryptoServiceAbstraction { return this.buildEncKey(key, encKey.key); } + /** + * @deprecated July 25 2022: Get the key you need from CryptoService (getKeyForUserEncryption or getOrgKey) + * and then call encryptService.encrypt + */ async encrypt(plainValue: string | ArrayBuffer, key?: SymmetricCryptoKey): Promise { - key = await this.getKeyForEncryption(key); + key = await this.getKeyForUserEncryption(key); return await this.encryptService.encrypt(plainValue, key); } + /** + * @deprecated July 25 2022: Get the key you need from CryptoService (getKeyForUserEncryption or getOrgKey) + * and then call encryptService.encryptToBytes + */ async encryptToBytes(plainValue: ArrayBuffer, key?: SymmetricCryptoKey): Promise { - key = await this.getKeyForEncryption(key); + key = await this.getKeyForUserEncryption(key); return this.encryptService.encryptToBytes(plainValue, key); } @@ -587,25 +594,34 @@ export class CryptoService implements CryptoServiceAbstraction { return this.cryptoFunctionService.rsaDecrypt(data, privateKey, alg); } + /** + * @deprecated July 25 2022: Get the key you need from CryptoService (getKeyForUserEncryption or getOrgKey) + * and then call encryptService.decryptToBytes + */ async decryptToBytes(encString: EncString, key?: SymmetricCryptoKey): Promise { - const keyForEnc = await this.getKeyForEncryption(key); - const theKey = await this.resolveLegacyKey(encString.encryptionType, keyForEnc); - return this.encryptService.decryptToBytes(encString, theKey); + const keyForEnc = await this.getKeyForUserEncryption(key); + return this.encryptService.decryptToBytes(encString, keyForEnc); } + /** + * @deprecated July 25 2022: Get the key you need from CryptoService (getKeyForUserEncryption or getOrgKey) + * and then call encryptService.decryptToUtf8 + */ async decryptToUtf8(encString: EncString, key?: SymmetricCryptoKey): Promise { - key = await this.getKeyForEncryption(key); - key = await this.resolveLegacyKey(encString.encryptionType, key); + key = await this.getKeyForUserEncryption(key); return await this.encryptService.decryptToUtf8(encString, key); } + /** + * @deprecated July 25 2022: Get the key you need from CryptoService (getKeyForUserEncryption or getOrgKey) + * and then call encryptService.decryptToBytes + */ async decryptFromBytes(encBuffer: EncArrayBuffer, key: SymmetricCryptoKey): Promise { if (encBuffer == null) { throw new Error("No buffer provided for decryption."); } - key = await this.getKeyForEncryption(key); - key = await this.resolveLegacyKey(encBuffer.encryptionType, key); + key = await this.getKeyForUserEncryption(key); return this.encryptService.decryptToBytes(encBuffer, key); } @@ -693,7 +709,7 @@ export class CryptoService implements CryptoServiceAbstraction { : await this.stateService.getCryptoMasterKeyBiometric({ userId: userId }); } - private async getKeyForEncryption(key?: SymmetricCryptoKey): Promise { + async getKeyForUserEncryption(key?: SymmetricCryptoKey): Promise { if (key != null) { return key; } @@ -703,29 +719,11 @@ export class CryptoService implements CryptoServiceAbstraction { return encKey; } + // Legacy support: encryption used to be done with the user key (derived from master password). + // Users who have not migrated will have a null encKey and must use the user key instead. return await this.getKey(); } - private async resolveLegacyKey( - encType: EncryptionType, - key: SymmetricCryptoKey - ): Promise { - if ( - encType === EncryptionType.AesCbc128_HmacSha256_B64 && - key.encType === EncryptionType.AesCbc256_B64 - ) { - // Old encrypt-then-mac scheme, make a new key - let legacyKey = await this.stateService.getLegacyEtmKey(); - if (legacyKey == null) { - legacyKey = new SymmetricCryptoKey(key.key, EncryptionType.AesCbc128_HmacSha256_B64); - await this.stateService.setLegacyEtmKey(legacyKey); - } - return legacyKey; - } - - return key; - } - private async stretchKey(key: SymmetricCryptoKey): Promise { const newKey = new Uint8Array(64); const encKey = await this.cryptoFunctionService.hkdfExpand(key.key, "enc", 32, "sha256"); diff --git a/libs/common/src/services/encrypt.service.ts b/libs/common/src/services/encrypt.service.ts index ac1b06197c..0b3f9731ca 100644 --- a/libs/common/src/services/encrypt.service.ts +++ b/libs/common/src/services/encrypt.service.ts @@ -6,6 +6,7 @@ import { EncryptedObject } from "@bitwarden/common/models/domain/encryptedObject import { SymmetricCryptoKey } from "@bitwarden/common/models/domain/symmetricCryptoKey"; import { AbstractEncryptService } from "../abstractions/abstractEncrypt.service"; +import { EncryptionType } from "../enums/encryptionType"; import { IEncrypted } from "../interfaces/IEncrypted"; import { EncArrayBuffer } from "../models/domain/encArrayBuffer"; @@ -63,9 +64,11 @@ export class EncryptService implements AbstractEncryptService { async decryptToUtf8(encString: EncString, key: SymmetricCryptoKey): Promise { if (key == null) { - throw new Error("No encryption key provided."); + throw new Error("No key provided for decryption."); } + key = this.resolveLegacyKey(key, encString); + if (key.macKey != null && encString?.mac == null) { this.logService.error("mac required."); return null; @@ -107,6 +110,8 @@ export class EncryptService implements AbstractEncryptService { throw new Error("Nothing provided for decryption."); } + key = this.resolveLegacyKey(key, encThing); + if (key.macKey != null && encThing.macBytes == null) { return null; } @@ -165,4 +170,19 @@ export class EncryptService implements AbstractEncryptService { this.logService.error(msg); } } + + /** + * Transform into new key for the old encrypt-then-mac scheme if required, otherwise return the current key unchanged + * @param encThing The encrypted object (e.g. encString or encArrayBuffer) that you want to decrypt + */ + resolveLegacyKey(key: SymmetricCryptoKey, encThing: IEncrypted): SymmetricCryptoKey { + if ( + encThing.encryptionType === EncryptionType.AesCbc128_HmacSha256_B64 && + key.encType === EncryptionType.AesCbc256_B64 + ) { + return new SymmetricCryptoKey(key.key, EncryptionType.AesCbc128_HmacSha256_B64); + } + + return key; + } } diff --git a/libs/common/src/services/state.service.ts b/libs/common/src/services/state.service.ts index 22836e0de7..8570d3ef64 100644 --- a/libs/common/src/services/state.service.ts +++ b/libs/common/src/services/state.service.ts @@ -1753,24 +1753,6 @@ export class StateService< ); } - @withPrototype(SymmetricCryptoKey, SymmetricCryptoKey.initFromJson) - async getLegacyEtmKey(options?: StorageOptions): Promise { - return ( - await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions())) - )?.keys?.legacyEtmKey; - } - - async setLegacyEtmKey(value: SymmetricCryptoKey, options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()) - ); - account.keys.legacyEtmKey = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultOnDiskOptions()) - ); - } - async getLocalData(options?: StorageOptions): Promise { return ( await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskLocalOptions())) diff --git a/libs/common/src/services/stateMigration.service.ts b/libs/common/src/services/stateMigration.service.ts index 0d723e9847..034d69f31a 100644 --- a/libs/common/src/services/stateMigration.service.ts +++ b/libs/common/src/services/stateMigration.service.ts @@ -164,6 +164,15 @@ export class StateMigrationService< await this.setCurrentStateVersion(StateVersion.Five); break; } + case StateVersion.Five: { + const authenticatedAccounts = await this.getAuthenticatedAccounts(); + for (const account of authenticatedAccounts) { + const migratedAccount = await this.migrateAccountFrom5To6(account); + await this.set(account.profile.userId, migratedAccount); + } + await this.setCurrentStateVersion(StateVersion.Six); + break; + } } currentStateVersion += 1; @@ -511,6 +520,11 @@ export class StateMigrationService< return account; } + protected async migrateAccountFrom5To6(account: TAccount): Promise { + delete (account as any).keys?.legacyEtmKey; + return account; + } + protected get options(): StorageOptions { return { htmlStorageLocation: HtmlStorageLocation.Local }; }