[EC-499] Add encryptService to domain model decryption (#3385)

Co-authored-by: Matt Gibson <mgibson@bitwarden.com>

apps/browser/src/background/main.background.ts

@@ -413,7 +413,7 @@ export default class MainBackground {
       this.eventService,
       this.logService
     );
-    this.containerService = new ContainerService(this.cryptoService);
+    this.containerService = new ContainerService(this.cryptoService, this.encryptService);
     this.auditService = new AuditService(this.cryptoFunctionService, this.apiService);
     this.exportService = new ExportService(
       this.folderService,

apps/cli/src/bw.ts

@@ -193,7 +193,7 @@ export class Main {
 
     this.organizationApiService = new OrganizationApiService(this.apiService);
 
-    this.containerService = new ContainerService(this.cryptoService);
+    this.containerService = new ContainerService(this.cryptoService, this.encryptService);
 
     this.settingsService = new SettingsService(this.stateService);
 

apps/desktop/src/app/services/init.service.ts

@@ -2,6 +2,7 @@ import { Inject, Injectable } from "@angular/core";
 
 import { WINDOW } from "@bitwarden/angular/services/jslib-services.module";
 import { AbstractThemingService } from "@bitwarden/angular/services/theming/theming.service.abstraction";
+import { AbstractEncryptService } from "@bitwarden/common/abstractions/abstractEncrypt.service";
 import { CryptoService as CryptoServiceAbstraction } from "@bitwarden/common/abstractions/crypto.service";
 import { EnvironmentService as EnvironmentServiceAbstraction } from "@bitwarden/common/abstractions/environment.service";
 import { EventService as EventServiceAbstraction } from "@bitwarden/common/abstractions/event.service";
@@ -34,7 +35,8 @@ export class InitService {
     private stateService: StateServiceAbstraction,
     private cryptoService: CryptoServiceAbstraction,
     private nativeMessagingService: NativeMessagingService,
-    private themingService: AbstractThemingService
+    private themingService: AbstractThemingService,
+    private encryptService: AbstractEncryptService
   ) {}
 
   init() {
@@ -65,7 +67,7 @@ export class InitService {
         await this.stateService.setInstalledVersion(currentVersion);
       }
 
-      const containerService = new ContainerService(this.cryptoService);
+      const containerService = new ContainerService(this.cryptoService, this.encryptService);
       containerService.attachToGlobal(this.win);
     };
   }

apps/web/src/app/core/init.service.ts

@@ -2,6 +2,7 @@ import { Inject, Injectable } from "@angular/core";
 
 import { WINDOW } from "@bitwarden/angular/services/jslib-services.module";
 import { AbstractThemingService } from "@bitwarden/angular/services/theming/theming.service.abstraction";
+import { AbstractEncryptService } from "@bitwarden/common/abstractions/abstractEncrypt.service";
 import { CryptoService as CryptoServiceAbstraction } from "@bitwarden/common/abstractions/crypto.service";
 import {
   EnvironmentService as EnvironmentServiceAbstraction,
@@ -31,7 +32,8 @@ export class InitService {
     private twoFactorService: TwoFactorServiceAbstraction,
     private stateService: StateServiceAbstraction,
     private cryptoService: CryptoServiceAbstraction,
-    private themingService: AbstractThemingService
+    private themingService: AbstractThemingService,
+    private encryptService: AbstractEncryptService
   ) {}
 
   init() {
@@ -51,7 +53,7 @@ export class InitService {
       const htmlEl = this.win.document.documentElement;
       htmlEl.classList.add("locale_" + this.i18nService.translationLocale);
       await this.themingService.monitorThemeChanges();
-      const containerService = new ContainerService(this.cryptoService);
+      const containerService = new ContainerService(this.cryptoService, this.encryptService);
       containerService.attachToGlobal(this.win);
     };
   }

libs/common/spec/models/domain/attachment.spec.ts

@@ -1,8 +1,10 @@
-import Substitute, { Arg } from "@fluffy-spoon/substitute";
+import { mock, MockProxy } from "jest-mock-extended";
 
+import { AbstractEncryptService } from "@bitwarden/common/abstractions/abstractEncrypt.service";
 import { CryptoService } from "@bitwarden/common/abstractions/crypto.service";
 import { AttachmentData } from "@bitwarden/common/models/data/attachmentData";
 import { Attachment } from "@bitwarden/common/models/domain/attachment";
+import { EncString } from "@bitwarden/common/models/domain/encString";
 import { SymmetricCryptoKey } from "@bitwarden/common/models/domain/symmetricCryptoKey";
 import { ContainerService } from "@bitwarden/common/services/container.service";
 
@@ -54,7 +56,21 @@ describe("Attachment", () => {
     expect(attachment.toAttachmentData()).toEqual(data);
   });
 
-  it("Decrypt", async () => {
+  describe("decrypt", () => {
+    let cryptoService: MockProxy<CryptoService>;
+    let encryptService: MockProxy<AbstractEncryptService>;
+
+    beforeEach(() => {
+      cryptoService = mock<CryptoService>();
+      encryptService = mock<AbstractEncryptService>();
+
+      (window as any).bitwardenContainerService = new ContainerService(
+        cryptoService,
+        encryptService
+      );
+    });
+
+    it("expected output", async () => {
       const attachment = new Attachment();
       attachment.id = "id";
       attachment.url = "url";
@@ -63,11 +79,7 @@ describe("Attachment", () => {
       attachment.key = mockEnc("key");
       attachment.fileName = mockEnc("fileName");
 
-    const cryptoService = Substitute.for<CryptoService>();
+      encryptService.decryptToBytes.mockResolvedValue(makeStaticByteArray(32));
-    cryptoService.getOrgKey(null).resolves(null);
-    cryptoService.decryptToBytes(Arg.any(), Arg.any()).resolves(makeStaticByteArray(32));
-
-    (window as any).bitwardenContainerService = new ContainerService(cryptoService);
 
       const view = await attachment.decrypt(null);
 
@@ -80,4 +92,43 @@ describe("Attachment", () => {
         key: expect.any(SymmetricCryptoKey),
       });
     });
+
+    describe("decrypts attachment.key", () => {
+      let attachment: Attachment;
+
+      beforeEach(() => {
+        attachment = new Attachment();
+        attachment.key = mock<EncString>();
+      });
+
+      it("uses the provided key without depending on CryptoService", async () => {
+        const providedKey = mock<SymmetricCryptoKey>();
+
+        await attachment.decrypt(null, providedKey);
+
+        expect(cryptoService.getKeyForUserEncryption).not.toHaveBeenCalled();
+        expect(encryptService.decryptToBytes).toHaveBeenCalledWith(attachment.key, providedKey);
+      });
+
+      it("gets an organization key if required", async () => {
+        const orgKey = mock<SymmetricCryptoKey>();
+        cryptoService.getOrgKey.calledWith("orgId").mockResolvedValue(orgKey);
+
+        await attachment.decrypt("orgId", null);
+
+        expect(cryptoService.getOrgKey).toHaveBeenCalledWith("orgId");
+        expect(encryptService.decryptToBytes).toHaveBeenCalledWith(attachment.key, orgKey);
+      });
+
+      it("gets the user's decryption key if required", async () => {
+        const userKey = mock<SymmetricCryptoKey>();
+        cryptoService.getKeyForUserEncryption.mockResolvedValue(userKey);
+
+        await attachment.decrypt(null, null);
+
+        expect(cryptoService.getKeyForUserEncryption).toHaveBeenCalled();
+        expect(encryptService.decryptToBytes).toHaveBeenCalledWith(attachment.key, userKey);
+      });
+    });
+  });
 });

libs/common/spec/models/domain/encString.spec.ts

@@ -1,5 +1,7 @@
 import Substitute, { Arg } from "@fluffy-spoon/substitute";
+import { mock, MockProxy } from "jest-mock-extended";
 
+import { AbstractEncryptService } from "@bitwarden/common/abstractions/abstractEncrypt.service";
 import { CryptoService } from "@bitwarden/common/abstractions/crypto.service";
 import { EncryptionType } from "@bitwarden/common/enums/encryptionType";
 import { EncString } from "@bitwarden/common/models/domain/encString";
@@ -48,10 +50,15 @@ describe("EncString", () => {
 
       const cryptoService = Substitute.for<CryptoService>();
       cryptoService.getOrgKey(null).resolves(null);
-      cryptoService.decryptToUtf8(encString, Arg.any()).resolves("decrypted");
+
+      const encryptService = Substitute.for<AbstractEncryptService>();
+      encryptService.decryptToUtf8(encString, Arg.any()).resolves("decrypted");
 
       beforeEach(() => {
-        (window as any).bitwardenContainerService = new ContainerService(cryptoService);
+        (window as any).bitwardenContainerService = new ContainerService(
+          cryptoService,
+          encryptService
+        );
       });
 
       it("decrypts correctly", async () => {
@@ -62,7 +69,7 @@ describe("EncString", () => {
 
       it("result should be cached", async () => {
         const decrypted = await encString.decrypt(null);
-        cryptoService.received(1).decryptToUtf8(Arg.any(), Arg.any());
+        encryptService.received(1).decryptToUtf8(Arg.any(), Arg.any());
 
         expect(decrypted).toBe("decrypted");
       });
@@ -148,25 +155,28 @@ describe("EncString", () => {
   });
 
   describe("decrypt", () => {
-    it("throws exception when bitwarden container not initialized", async () => {
+    let cryptoService: MockProxy<CryptoService>;
-      const encString = new EncString(null);
+    let encryptService: MockProxy<AbstractEncryptService>;
+    let encString: EncString;
 
-      expect.assertions(1);
+    beforeEach(() => {
-      try {
+      cryptoService = mock<CryptoService>();
-        await encString.decrypt(null);
+      encryptService = mock<AbstractEncryptService>();
-      } catch (e) {
+      encString = new EncString(null);
-        expect(e.message).toEqual("global bitwardenContainerService not initialized.");
+
-      }
+      (window as any).bitwardenContainerService = new ContainerService(
+        cryptoService,
+        encryptService
+      );
     });
 
     it("handles value it can't decrypt", async () => {
-      const encString = new EncString(null);
+      encryptService.decryptToUtf8.mockRejectedValue("error");
 
-      const cryptoService = Substitute.for<CryptoService>();
+      (window as any).bitwardenContainerService = new ContainerService(
-      cryptoService.getOrgKey(null).resolves(null);
+        cryptoService,
-      cryptoService.decryptToUtf8(encString, Arg.any()).throws("error");
+        encryptService
-
+      );
-      (window as any).bitwardenContainerService = new ContainerService(cryptoService);
 
       const decrypted = await encString.decrypt(null);
 
@@ -178,18 +188,35 @@ describe("EncString", () => {
       });
     });
 
-    it("passes along key", async () => {
+    it("uses provided key without depending on CryptoService", async () => {
-      const encString = new EncString(null);
+      const key = mock<SymmetricCryptoKey>();
-      const key = Substitute.for<SymmetricCryptoKey>();
-
-      const cryptoService = Substitute.for<CryptoService>();
-      cryptoService.getOrgKey(null).resolves(null);
-
-      (window as any).bitwardenContainerService = new ContainerService(cryptoService);
 
       await encString.decrypt(null, key);
 
-      cryptoService.received().decryptToUtf8(encString, key);
+      expect(cryptoService.getKeyForUserEncryption).not.toHaveBeenCalled();
+      expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(encString, key);
+    });
+
+    it("gets an organization key if required", async () => {
+      const orgKey = mock<SymmetricCryptoKey>();
+
+      cryptoService.getOrgKey.calledWith("orgId").mockResolvedValue(orgKey);
+
+      await encString.decrypt("orgId", null);
+
+      expect(cryptoService.getOrgKey).toHaveBeenCalledWith("orgId");
+      expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(encString, orgKey);
+    });
+
+    it("gets the user's decryption key if required", async () => {
+      const userKey = mock<SymmetricCryptoKey>();
+
+      cryptoService.getKeyForUserEncryption.mockResolvedValue(userKey);
+
+      await encString.decrypt(null, null);
+
+      expect(cryptoService.getKeyForUserEncryption).toHaveBeenCalledWith();
+      expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(encString, userKey);
     });
   });
 

libs/common/spec/models/domain/send.spec.ts

@@ -1,5 +1,6 @@
 import Substitute, { Arg, SubstituteOf } from "@fluffy-spoon/substitute";
 
+import { AbstractEncryptService } from "@bitwarden/common/abstractions/abstractEncrypt.service";
 import { CryptoService } from "@bitwarden/common/abstractions/crypto.service";
 import { SendType } from "@bitwarden/common/enums/sendType";
 import { SendData } from "@bitwarden/common/models/data/sendData";
@@ -110,7 +111,9 @@ describe("Send", () => {
     cryptoService.decryptToBytes(send.key, null).resolves(makeStaticByteArray(32));
     cryptoService.makeSendKey(Arg.any()).resolves("cryptoKey" as any);
 
-    (window as any).bitwardenContainerService = new ContainerService(cryptoService);
+    const encryptService = Substitute.for<AbstractEncryptService>();
+
+    (window as any).bitwardenContainerService = new ContainerService(cryptoService, encryptService);
 
     const view = await send.decrypt();
 

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

@@ -1,6 +1,7 @@
 import { Arg, Substitute, SubstituteOf } from "@fluffy-spoon/substitute";
 import { BehaviorSubject, firstValueFrom } from "rxjs";
 
+import { AbstractEncryptService } from "@bitwarden/common/abstractions/abstractEncrypt.service";
 import { CipherService } from "@bitwarden/common/abstractions/cipher.service";
 import { CryptoService } from "@bitwarden/common/abstractions/crypto.service";
 import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
@@ -15,6 +16,7 @@ describe("Folder Service", () => {
   let folderService: FolderService;
 
   let cryptoService: SubstituteOf<CryptoService>;
+  let encryptService: SubstituteOf<AbstractEncryptService>;
   let i18nService: SubstituteOf<I18nService>;
   let cipherService: SubstituteOf<CipherService>;
   let stateService: SubstituteOf<StateService>;
@@ -23,6 +25,7 @@ describe("Folder Service", () => {
 
   beforeEach(() => {
     cryptoService = Substitute.for();
+    encryptService = Substitute.for();
     i18nService = Substitute.for();
     cipherService = Substitute.for();
     stateService = Substitute.for();
@@ -34,7 +37,7 @@ describe("Folder Service", () => {
     });
     stateService.activeAccount$.returns(activeAccount);
     stateService.activeAccountUnlocked$.returns(activeAccountUnlocked);
-    (window as any).bitwardenContainerService = new ContainerService(cryptoService);
+    (window as any).bitwardenContainerService = new ContainerService(cryptoService, encryptService);
 
     folderService = new FolderService(cryptoService, i18nService, cipherService, stateService);
   });

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

@@ -1,6 +1,7 @@
 import { Arg, Substitute, SubstituteOf } from "@fluffy-spoon/substitute";
 import { BehaviorSubject, firstValueFrom } from "rxjs";
 
+import { AbstractEncryptService } from "@bitwarden/common/abstractions/abstractEncrypt.service";
 import { CryptoService } from "@bitwarden/common/abstractions/crypto.service";
 import { ContainerService } from "@bitwarden/common/services/container.service";
 import { SettingsService } from "@bitwarden/common/services/settings.service";
@@ -10,12 +11,14 @@ describe("SettingsService", () => {
   let settingsService: SettingsService;
 
   let cryptoService: SubstituteOf<CryptoService>;
+  let encryptService: SubstituteOf<AbstractEncryptService>;
   let stateService: SubstituteOf<StateService>;
   let activeAccount: BehaviorSubject<string>;
   let activeAccountUnlocked: BehaviorSubject<boolean>;
 
   beforeEach(() => {
     cryptoService = Substitute.for();
+    encryptService = Substitute.for();
     stateService = Substitute.for();
     activeAccount = new BehaviorSubject("123");
     activeAccountUnlocked = new BehaviorSubject(true);
@@ -23,7 +26,7 @@ describe("SettingsService", () => {
     stateService.getSettings().resolves({ equivalentDomains: [["test"], ["domains"]] });
     stateService.activeAccount$.returns(activeAccount);
     stateService.activeAccountUnlocked$.returns(activeAccountUnlocked);
-    (window as any).bitwardenContainerService = new ContainerService(cryptoService);
+    (window as any).bitwardenContainerService = new ContainerService(cryptoService, encryptService);
 
     settingsService = new SettingsService(stateService);
   });

libs/common/src/misc/utils.ts

@@ -3,6 +3,7 @@ import * as tldjs from "tldjs";
 
 import { CryptoService } from "@bitwarden/common/abstractions/crypto.service";
 
+import { AbstractEncryptService } from "../abstractions/abstractEncrypt.service";
 import { I18nService } from "../abstractions/i18n.service";
 
 const nodeURL = typeof window === "undefined" ? require("url") : null;
@@ -14,6 +15,7 @@ declare global {
 
 interface BitwardenContainerService {
   getCryptoService: () => CryptoService;
+  getEncryptService: () => AbstractEncryptService;
 }
 
 export class Utils {
@@ -368,6 +370,16 @@ export class Utils {
     return s.charAt(0).toUpperCase() + s.slice(1);
   }
 
+  /**
+   * @throws Will throw an error if the ContainerService has not been attached to the window object
+   */
+  static getContainerService(): BitwardenContainerService {
+    if (this.global.bitwardenContainerService == null) {
+      throw new Error("global bitwardenContainerService not initialized.");
+    }
+    return this.global.bitwardenContainerService;
+  }
+
   private static validIpAddress(ipString: string): boolean {
     const ipRegex =
       /^(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$/;

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

@@ -1,4 +1,3 @@
-import { CryptoService } from "../../abstractions/crypto.service";
 import { Utils } from "../../misc/utils";
 import { AttachmentData } from "../data/attachmentData";
 import { AttachmentView } from "../view/attachmentView";
@@ -47,24 +46,31 @@ export class Attachment extends Domain {
     );
 
     if (this.key != null) {
-      let cryptoService: CryptoService;
+      view.key = await this.decryptAttachmentKey(orgId, encKey);
-      const containerService = Utils.global.bitwardenContainerService;
-      if (containerService) {
-        cryptoService = containerService.getCryptoService();
-      } else {
-        throw new Error("global bitwardenContainerService not initialized.");
     }
 
+    return view;
+  }
+
+  private async decryptAttachmentKey(orgId: string, encKey?: SymmetricCryptoKey) {
     try {
-        const orgKey = await cryptoService.getOrgKey(orgId);
+      if (encKey == null) {
-        const decValue = await cryptoService.decryptToBytes(this.key, orgKey ?? encKey);
+        encKey = await this.getKeyForDecryption(orgId);
-        view.key = new SymmetricCryptoKey(decValue);
+      }
+
+      const encryptService = Utils.getContainerService().getEncryptService();
+      const decValue = await encryptService.decryptToBytes(this.key, encKey);
+      return new SymmetricCryptoKey(decValue);
     } catch (e) {
       // TODO: error?
     }
   }
 
-    return view;
+  private async getKeyForDecryption(orgId: string) {
+    const cryptoService = Utils.getContainerService().getCryptoService();
+    return orgId != null
+      ? await cryptoService.getOrgKey(orgId)
+      : await cryptoService.getKeyForUserEncryption();
   }
 
   toAttachmentData(): AttachmentData {

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

@@ -2,7 +2,6 @@ import { Jsonify } from "type-fest";
 
 import { IEncrypted } from "@bitwarden/common/interfaces/IEncrypted";
 
-import { CryptoService } from "../../abstractions/crypto.service";
 import { EncryptionType } from "../../enums/encryptionType";
 import { Utils } from "../../misc/utils";
 
@@ -29,30 +28,6 @@ export class EncString implements IEncrypted {
     }
   }
 
-  async decrypt(orgId: string, key: SymmetricCryptoKey = null): Promise<string> {
-    if (this.decryptedValue != null) {
-      return this.decryptedValue;
-    }
-
-    let cryptoService: CryptoService;
-    const containerService = Utils.global.bitwardenContainerService;
-    if (containerService) {
-      cryptoService = containerService.getCryptoService();
-    } else {
-      throw new Error("global bitwardenContainerService not initialized.");
-    }
-
-    try {
-      if (key == null) {
-        key = await cryptoService.getOrgKey(orgId);
-      }
-      this.decryptedValue = await cryptoService.decryptToUtf8(this, key);
-    } catch (e) {
-      this.decryptedValue = "[error: cannot decrypt]";
-    }
-    return this.decryptedValue;
-  }
-
   get ivBytes(): ArrayBuffer {
     return this.iv == null ? null : Utils.fromB64ToArray(this.iv).buffer;
   }
@@ -160,4 +135,32 @@ export class EncString implements IEncrypted {
       encPieces,
     };
   }
+
+  async decrypt(orgId: string, key: SymmetricCryptoKey = null): Promise<string> {
+    if (this.decryptedValue != null) {
+      return this.decryptedValue;
+    }
+
+    try {
+      if (key == null) {
+        key = await this.getKeyForDecryption(orgId);
+      }
+      if (key == null) {
+        throw new Error("No key to decrypt EncString with orgId " + orgId);
+      }
+
+      const encryptService = Utils.getContainerService().getEncryptService();
+      this.decryptedValue = await encryptService.decryptToUtf8(this, key);
+    } catch (e) {
+      this.decryptedValue = "[error: cannot decrypt]";
+    }
+    return this.decryptedValue;
+  }
+
+  private async getKeyForDecryption(orgId: string) {
+    const cryptoService = Utils.getContainerService().getCryptoService();
+    return orgId != null
+      ? await cryptoService.getOrgKey(orgId)
+      : await cryptoService.getKeyForUserEncryption();
+  }
 }

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

@@ -1,4 +1,3 @@
-import { CryptoService } from "../../abstractions/crypto.service";
 import { SendType } from "../../enums/sendType";
 import { Utils } from "../../misc/utils";
 import { SendData } from "../data/sendData";
@@ -71,13 +70,7 @@ export class Send extends Domain {
   async decrypt(): Promise<SendView> {
     const model = new SendView(this);
 
-    let cryptoService: CryptoService;
+    const cryptoService = Utils.getContainerService().getCryptoService();
-    const containerService = Utils.global.bitwardenContainerService;
-    if (containerService) {
-      cryptoService = containerService.getCryptoService();
-    } else {
-      throw new Error("global bitwardenContainerService not initialized.");
-    }
 
     try {
       model.key = await cryptoService.decryptToBytes(this.key, null);

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

@@ -1,7 +1,11 @@
+import { AbstractEncryptService } from "../abstractions/abstractEncrypt.service";
 import { CryptoService } from "../abstractions/crypto.service";
 
 export class ContainerService {
-  constructor(private cryptoService: CryptoService) {}
+  constructor(
+    private cryptoService: CryptoService,
+    private encryptService: AbstractEncryptService
+  ) {}
 
   attachToGlobal(global: any) {
     if (!global.bitwardenContainerService) {
@@ -9,7 +13,23 @@ export class ContainerService {
     }
   }
 
+  /**
+   * @throws Will throw if CryptoService was not instantiated and provided to the ContainerService constructor
+   */
   getCryptoService(): CryptoService {
+    if (this.cryptoService == null) {
+      throw new Error("ContainerService.cryptoService not initialized.");
+    }
     return this.cryptoService;
   }
+
+  /**
+   * @throws Will throw if EncryptService was not instantiated and provided to the ContainerService constructor
+   */
+  getEncryptService(): AbstractEncryptService {
+    if (this.encryptService == null) {
+      throw new Error("ContainerService.encryptService not initialized.");
+    }
+    return this.encryptService;
+  }
 }