[EC-135] Delay decryption of provider-encrypted org keys (#2902)

libs/common/spec/services/stateMigration.service.spec.ts

@@ -9,6 +9,9 @@ import { StateMigrationService } from "@bitwarden/common/services/stateMigration
 
 const userId = "USER_ID";
 
+// Note: each test calls the private migration method for that migration,
+// so that we don't accidentally run all following migrations as well
+
 describe("State Migration Service", () => {
   let storageService: SubstituteOf<AbstractStorageService>;
   let secureStorageService: SubstituteOf<AbstractStorageService>;
@@ -66,13 +69,13 @@ describe("State Migration Service", () => {
 
       storageService.get(userId, Arg.any()).resolves(accountVersion3);
 
-      await stateMigrationService.migrate();
+      await (stateMigrationService as any).migrateStateFrom3To4();
 
       storageService.received(1).save(userId, expectedAccountVersion4, Arg.any());
     });
 
     it("updates StateVersion number", async () => {
-      await stateMigrationService.migrate();
+      await (stateMigrationService as any).migrateStateFrom3To4();
 
       storageService.received(1).save(
         "global",
@@ -81,4 +84,47 @@ describe("State Migration Service", () => {
       );
     });
   });
+
+  describe("StateVersion 4 to 5 migration", () => {
+    it("migrates organization keys to new format", async () => {
+      const accountVersion4 = new Account({
+        keys: {
+          organizationKeys: {
+            encrypted: {
+              orgOneId: "orgOneEncKey",
+              orgTwoId: "orgTwoEncKey",
+              orgThreeId: "orgThreeEncKey",
+            },
+          },
+        },
+      } as any);
+
+      const expectedAccount = new Account({
+        keys: {
+          organizationKeys: {
+            encrypted: {
+              orgOneId: {
+                type: "organization",
+                key: "orgOneEncKey",
+              },
+              orgTwoId: {
+                type: "organization",
+                key: "orgTwoEncKey",
+              },
+              orgThreeId: {
+                type: "organization",
+                key: "orgThreeEncKey",
+              },
+            },
+          },
+        },
+      });
+
+      const migratedAccount = await (stateMigrationService as any).migrateAccountFrom4To5(
+        accountVersion4
+      );
+
+      expect(migratedAccount).toEqual(expectedAccount);
+    });
+  });
 });

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

@@ -5,6 +5,7 @@ import { ThemeType } from "../enums/themeType";
 import { UriMatchType } from "../enums/uriMatchType";
 import { CipherData } from "../models/data/cipherData";
 import { CollectionData } from "../models/data/collectionData";
+import { EncryptedOrganizationKeyData } from "../models/data/encryptedOrganizationKeyData";
 import { EventData } from "../models/data/eventData";
 import { FolderData } from "../models/data/folderData";
 import { OrganizationData } from "../models/data/organizationData";
@@ -191,9 +192,11 @@ export abstract class StateService<T extends Account = Account> {
     value: { [id: string]: FolderData },
     options?: StorageOptions
   ) => Promise<void>;
-  getEncryptedOrganizationKeys: (options?: StorageOptions) => Promise<any>;
+  getEncryptedOrganizationKeys: (
+    options?: StorageOptions
+  ) => Promise<{ [orgId: string]: EncryptedOrganizationKeyData }>;
   setEncryptedOrganizationKeys: (
-    value: Map<string, SymmetricCryptoKey>,
+    value: { [orgId: string]: EncryptedOrganizationKeyData },
     options?: StorageOptions
   ) => Promise<void>;
   getEncryptedPasswordGenerationHistory: (

libs/common/src/enums/stateVersion.ts

@@ -3,5 +3,6 @@ export enum StateVersion {
   Two = 2, // Move to a typed State object
   Three = 3, // Fix migration of users' premium status
   Four = 4, // Fix 'Never Lock' option by removing stale data
-  Latest = Four,
+  Five = 5, // Migrate to new storage of encrypted organization keys
+  Latest = Five,
 }

libs/common/src/models/data/encryptedOrganizationKeyData.ts

@@ -0,0 +1,14 @@
+export type EncryptedOrganizationKeyData =
+  | OrganizationEncryptedOrganizationKeyData
+  | ProviderEncryptedOrganizationKeyData;
+
+type OrganizationEncryptedOrganizationKeyData = {
+  type: "organization";
+  key: string;
+};
+
+type ProviderEncryptedOrganizationKeyData = {
+  type: "provider";
+  key: string;
+  providerId: string;
+};

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

@@ -3,6 +3,7 @@ import { KdfType } from "../../enums/kdfType";
 import { UriMatchType } from "../../enums/uriMatchType";
 import { CipherData } from "../data/cipherData";
 import { CollectionData } from "../data/collectionData";
+import { EncryptedOrganizationKeyData } from "../data/encryptedOrganizationKeyData";
 import { EventData } from "../data/eventData";
 import { FolderData } from "../data/folderData";
 import { OrganizationData } from "../data/organizationData";
@@ -69,8 +70,11 @@ export class AccountKeys {
     string,
     SymmetricCryptoKey
   >();
-  organizationKeys?: EncryptionPair<any, Map<string, SymmetricCryptoKey>> = new EncryptionPair<
+  organizationKeys?: EncryptionPair<
-    any,
+    { [orgId: string]: EncryptedOrganizationKeyData },
+    Map<string, SymmetricCryptoKey>
+  > = new EncryptionPair<
+    { [orgId: string]: EncryptedOrganizationKeyData },
     Map<string, SymmetricCryptoKey>
   >();
   providerKeys?: EncryptionPair<any, Map<string, SymmetricCryptoKey>> = new EncryptionPair<

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

@@ -0,0 +1,56 @@
+import { CryptoService } from "../../abstractions/crypto.service";
+import { EncryptedOrganizationKeyData } from "../../models/data/encryptedOrganizationKeyData";
+
+import { EncString } from "./encString";
+import { SymmetricCryptoKey } from "./symmetricCryptoKey";
+
+export abstract class BaseEncryptedOrganizationKey {
+  decrypt: (cryptoService: CryptoService) => Promise<SymmetricCryptoKey>;
+
+  static fromData(data: EncryptedOrganizationKeyData) {
+    switch (data.type) {
+      case "organization":
+        return new EncryptedOrganizationKey(data.key);
+
+      case "provider":
+        return new ProviderEncryptedOrganizationKey(data.key, data.providerId);
+
+      default:
+        return null;
+    }
+  }
+}
+
+export class EncryptedOrganizationKey implements BaseEncryptedOrganizationKey {
+  constructor(private key: string) {}
+
+  async decrypt(cryptoService: CryptoService) {
+    const decValue = await cryptoService.rsaDecrypt(this.key);
+    return new SymmetricCryptoKey(decValue);
+  }
+
+  toData(): EncryptedOrganizationKeyData {
+    return {
+      type: "organization",
+      key: this.key,
+    };
+  }
+}
+
+export class ProviderEncryptedOrganizationKey implements BaseEncryptedOrganizationKey {
+  constructor(private key: string, private providerId: string) {}
+
+  async decrypt(cryptoService: CryptoService) {
+    const providerKey = await cryptoService.getProviderKey(this.providerId);
+    const decValue = await cryptoService.decryptToBytes(new EncString(this.key), providerKey);
+    return new SymmetricCryptoKey(decValue);
+  }
+
+  toData(): EncryptedOrganizationKeyData {
+    return {
+      type: "provider",
+      key: this.key,
+      providerId: this.providerId,
+    };
+  }
+}

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

@@ -13,9 +13,11 @@ import { KeySuffixOptions } from "../enums/keySuffixOptions";
 import { sequentialize } from "../misc/sequentialize";
 import { Utils } from "../misc/utils";
 import { EEFLongWordList } from "../misc/wordlist";
+import { EncryptedOrganizationKeyData } from "../models/data/encryptedOrganizationKeyData";
 import { EncArrayBuffer } from "../models/domain/encArrayBuffer";
 import { EncString } from "../models/domain/encString";
 import { EncryptedObject } from "../models/domain/encryptedObject";
+import { BaseEncryptedOrganizationKey } from "../models/domain/encryptedOrganizationKey";
 import { SymmetricCryptoKey } from "../models/domain/symmetricCryptoKey";
 import { ProfileOrganizationResponse } from "../models/response/profileOrganizationResponse";
 import { ProfileProviderOrganizationResponse } from "../models/response/profileProviderOrganizationResponse";
@@ -58,23 +60,28 @@ export class CryptoService implements CryptoServiceAbstraction {
   }
 
   async setOrgKeys(
-    orgs: ProfileOrganizationResponse[],
+    orgs: ProfileOrganizationResponse[] = [],
-    providerOrgs: ProfileProviderOrganizationResponse[]
+    providerOrgs: ProfileProviderOrganizationResponse[] = []
   ): Promise<void> {
-    const orgKeys: any = {};
+    const encOrgKeyData: { [orgId: string]: EncryptedOrganizationKeyData } = {};
+
     orgs.forEach((org) => {
-      orgKeys[org.id] = org.key;
+      encOrgKeyData[org.id] = {
+        type: "organization",
+        key: org.key,
+      };
     });
 
-    for (const providerOrg of providerOrgs) {
+    providerOrgs.forEach((org) => {
-      // Convert provider encrypted keys to user encrypted.
+      encOrgKeyData[org.id] = {
-      const providerKey = await this.getProviderKey(providerOrg.providerId);
+        type: "provider",
-      const decValue = await this.decryptToBytes(new EncString(providerOrg.key), providerKey);
+        providerId: org.providerId,
-      orgKeys[providerOrg.id] = (await this.rsaEncrypt(decValue)).encryptedString;
+        key: org.key,
-    }
+      };
+    });
 
     await this.stateService.setDecryptedOrganizationKeys(null);
-    return await this.stateService.setEncryptedOrganizationKeys(orgKeys);
+    return await this.stateService.setEncryptedOrganizationKeys(encOrgKeyData);
   }
 
   async setProviderKeys(providers: ProfileProviderResponse[]): Promise<void> {
@@ -211,35 +218,36 @@ export class CryptoService implements CryptoServiceAbstraction {
 
   @sequentialize(() => "getOrgKeys")
   async getOrgKeys(): Promise<Map<string, SymmetricCryptoKey>> {
-    const orgKeys: Map<string, SymmetricCryptoKey> = new Map<string, SymmetricCryptoKey>();
+    const result: Map<string, SymmetricCryptoKey> = new Map<string, SymmetricCryptoKey>();
     const decryptedOrganizationKeys = await this.stateService.getDecryptedOrganizationKeys();
     if (decryptedOrganizationKeys != null && decryptedOrganizationKeys.size > 0) {
       return decryptedOrganizationKeys;
     }
 
-    const encOrgKeys = await this.stateService.getEncryptedOrganizationKeys();
+    const encOrgKeyData = await this.stateService.getEncryptedOrganizationKeys();
-    if (encOrgKeys == null) {
+    if (encOrgKeyData == null) {
       return null;
     }
 
     let setKey = false;
 
-    for (const orgId in encOrgKeys) {
+    for (const orgId of Object.keys(encOrgKeyData)) {
-      // eslint-disable-next-line
+      if (result.has(orgId)) {
-      if (!encOrgKeys.hasOwnProperty(orgId)) {
         continue;
       }
 
-      const decValue = await this.rsaDecrypt(encOrgKeys[orgId]);
+      const encOrgKey = BaseEncryptedOrganizationKey.fromData(encOrgKeyData[orgId]);
-      orgKeys.set(orgId, new SymmetricCryptoKey(decValue));
+      const decOrgKey = await encOrgKey.decrypt(this);
+      result.set(orgId, decOrgKey);
+
       setKey = true;
     }
 
     if (setKey) {
-      await this.stateService.setDecryptedOrganizationKeys(orgKeys);
+      await this.stateService.setDecryptedOrganizationKeys(result);
     }
 
-    return orgKeys;
+    return result;
   }
 
   async getOrgKey(orgId: string): Promise<SymmetricCryptoKey> {

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

@@ -13,6 +13,7 @@ import { StateFactory } from "../factories/stateFactory";
 import { Utils } from "../misc/utils";
 import { CipherData } from "../models/data/cipherData";
 import { CollectionData } from "../models/data/collectionData";
+import { EncryptedOrganizationKeyData } from "../models/data/encryptedOrganizationKeyData";
 import { EventData } from "../models/data/eventData";
 import { FolderData } from "../models/data/folderData";
 import { OrganizationData } from "../models/data/organizationData";
@@ -1344,14 +1345,16 @@ export class StateService<
     );
   }
 
-  async getEncryptedOrganizationKeys(options?: StorageOptions): Promise<any> {
+  async getEncryptedOrganizationKeys(
+    options?: StorageOptions
+  ): Promise<{ [orgId: string]: EncryptedOrganizationKeyData }> {
     return (
       await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions()))
     )?.keys?.organizationKeys.encrypted;
   }
 
   async setEncryptedOrganizationKeys(
-    value: Map<string, SymmetricCryptoKey>,
+    value: { [orgId: string]: EncryptedOrganizationKeyData },
     options?: StorageOptions
   ): Promise<void> {
     const account = await this.getAccount(

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

@@ -155,6 +155,15 @@ export class StateMigrationService<
         case StateVersion.Three:
           await this.migrateStateFrom3To4();
           break;
+        case StateVersion.Four: {
+          const authenticatedAccounts = await this.getAuthenticatedAccounts();
+          for (const account of authenticatedAccounts) {
+            const migratedAccount = await this.migrateAccountFrom4To5(account);
+            await this.set(account.profile.userId, migratedAccount);
+          }
+          await this.setCurrentStateVersion(StateVersion.Five);
+          break;
+        }
       }
 
       currentStateVersion += 1;
@@ -488,6 +497,20 @@ export class StateMigrationService<
     await this.set(keys.global, globals);
   }
 
+  protected async migrateAccountFrom4To5(account: TAccount): Promise<TAccount> {
+    const encryptedOrgKeys = account.keys?.organizationKeys?.encrypted;
+    if (encryptedOrgKeys != null) {
+      for (const [orgId, encKey] of Object.entries(encryptedOrgKeys)) {
+        encryptedOrgKeys[orgId] = {
+          type: "organization",
+          key: encKey as unknown as string, // Account v4 does not reflect the current account model so we have to cast
+        };
+      }
+    }
+
+    return account;
+  }
+
   protected get options(): StorageOptions {
     return { htmlStorageLocation: HtmlStorageLocation.Local };
   }
@@ -510,4 +533,15 @@ export class StateMigrationService<
   protected async getCurrentStateVersion(): Promise<StateVersion> {
     return (await this.getGlobals())?.stateVersion ?? StateVersion.One;
   }
+
+  protected async setCurrentStateVersion(newVersion: StateVersion): Promise<void> {
+    const globals = await this.getGlobals();
+    globals.stateVersion = newVersion;
+    await this.set(keys.global, globals);
+  }
+
+  protected async getAuthenticatedAccounts(): Promise<TAccount[]> {
+    const authenticatedUserIds = await this.get<string[]>(keys.authenticatedAccounts);
+    return Promise.all(authenticatedUserIds.map((id) => this.get<TAccount>(id)));
+  }
 }