[EC-364] Expose key getters on CryptoService (#3170)

* Move resolveLegacyKey to encryptService for utf8 decryption

* Deprecate account.keys.legacyEtmKey

Includes migration to tidy up leftover data

* Use new IEncrypted interface

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>();
+      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>();
+      encString.encryptionType = encType;
+
+      const actual = encryptService.resolveLegacyKey(key, encString);
+
+      expect(actual).toEqual(key);
+    });
+  });
 });

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();
+    });
+  });
 });

libs/common/src/abstractions/abstractEncrypt.service.ts

@@ -12,4 +12,5 @@ export abstract class AbstractEncryptService {
   ) => Promise<EncArrayBuffer>;
   abstract decryptToUtf8: (encString: EncString, key: SymmetricCryptoKey) => Promise<string>;
   abstract decryptToBytes: (encThing: IEncrypted, key: SymmetricCryptoKey) => Promise<ArrayBuffer>;
+  abstract resolveLegacyKey: (key: SymmetricCryptoKey, encThing: IEncrypted) => SymmetricCryptoKey;
 }

libs/common/src/abstractions/crypto.service.ts

@@ -29,6 +29,7 @@ export abstract class CryptoService {
   getOrgKeys: () => Promise<Map<string, SymmetricCryptoKey>>;
   getOrgKey: (orgId: string) => Promise<SymmetricCryptoKey>;
   getProviderKey: (providerId: string) => Promise<SymmetricCryptoKey>;
+  getKeyForUserEncryption: (key?: SymmetricCryptoKey) => Promise<SymmetricCryptoKey>;
   hasKey: () => Promise<boolean>;
   hasKeyInMemory: (userId?: string) => Promise<boolean>;
   hasKeyStored: (keySuffix?: KeySuffixOptions, userId?: string) => Promise<boolean>;

libs/common/src/abstractions/state.service.ts

@@ -248,8 +248,6 @@ export abstract class StateService<T extends Account = Account> {
   setLastActive: (value: number, options?: StorageOptions) => Promise<void>;
   getLastSync: (options?: StorageOptions) => Promise<string>;
   setLastSync: (value: string, options?: StorageOptions) => Promise<void>;
-  getLegacyEtmKey: (options?: StorageOptions) => Promise<SymmetricCryptoKey>;
-  setLegacyEtmKey: (value: SymmetricCryptoKey, options?: StorageOptions) => Promise<void>;
   getLocalData: (options?: StorageOptions) => Promise<any>;
   setLocalData: (value: string, options?: StorageOptions) => Promise<void>;
   getLocale: (options?: StorageOptions) => Promise<string>;

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,
 }

libs/common/src/models/domain/account.ts

@@ -82,7 +82,6 @@ export class AccountKeys {
     Map<string, SymmetricCryptoKey>
   >();
   privateKey?: EncryptionPair<string, ArrayBuffer> = new EncryptionPair<string, ArrayBuffer>();
-  legacyEtmKey?: SymmetricCryptoKey;
   publicKey?: ArrayBuffer;
   publicKeySerialized?: string;
   apiKeyClientSecret?: string;

libs/common/src/services/crypto.service.ts

@@ -337,7 +337,6 @@ export class CryptoService implements CryptoServiceAbstraction {
 
   async clearKey(clearSecretStorage = true, userId?: string): Promise<any> {
     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<EncString> {
-    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<EncArrayBuffer> {
-    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<ArrayBuffer> {
-    const keyForEnc = await this.getKeyForEncryption(key);
+    const keyForEnc = await this.getKeyForUserEncryption(key);
-    const theKey = await this.resolveLegacyKey(encString.encryptionType, keyForEnc);
+    return this.encryptService.decryptToBytes(encString, keyForEnc);
-    return this.encryptService.decryptToBytes(encString, theKey);
   }
 
+  /**
+   * @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<string> {
-    key = await this.getKeyForEncryption(key);
+    key = await this.getKeyForUserEncryption(key);
-    key = await this.resolveLegacyKey(encString.encryptionType, 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<ArrayBuffer> {
     if (encBuffer == null) {
       throw new Error("No buffer provided for decryption.");
     }
 
-    key = await this.getKeyForEncryption(key);
+    key = await this.getKeyForUserEncryption(key);
-    key = await this.resolveLegacyKey(encBuffer.encryptionType, 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<SymmetricCryptoKey> {
+  async getKeyForUserEncryption(key?: SymmetricCryptoKey): Promise<SymmetricCryptoKey> {
     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<SymmetricCryptoKey> {
-    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<SymmetricCryptoKey> {
     const newKey = new Uint8Array(64);
     const encKey = await this.cryptoFunctionService.hkdfExpand(key.key, "enc", 32, "sha256");

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<string> {
     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;
+  }
 }

libs/common/src/services/state.service.ts

@@ -1753,24 +1753,6 @@ export class StateService<
     );
   }
 
-  @withPrototype(SymmetricCryptoKey, SymmetricCryptoKey.initFromJson)
-  async getLegacyEtmKey(options?: StorageOptions): Promise<SymmetricCryptoKey> {
-    return (
-      await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions()))
-    )?.keys?.legacyEtmKey;
-  }
-
-  async setLegacyEtmKey(value: SymmetricCryptoKey, options?: StorageOptions): Promise<void> {
-    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<any> {
     return (
       await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskLocalOptions()))

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<TAccount> {
+    delete (account as any).keys?.legacyEtmKey;
+    return account;
+  }
+
   protected get options(): StorageOptions {
     return { htmlStorageLocation: HtmlStorageLocation.Local };
   }