[EC-271] Refactor CryptoService - move symmetric encryption to EncryptService (#3042)

* move decryptFromBytes, decryptToBytes, and encryptToBytes from CryptoService to EncryptService
* leave redirects in CryptoService
* combine encryptService decryptFromBytes and decryptToBytes methods
* move parsing logic into EncArrayBuffer
* add tests

libs/common/spec/domain/encArrayBuffer.spec.ts

@@ -0,0 +1,76 @@
+import { EncryptionType } from "@bitwarden/common/enums/encryptionType";
+import { EncArrayBuffer } from "@bitwarden/common/models/domain/encArrayBuffer";
+
+import { makeStaticByteArray } from "../utils";
+
+describe("encArrayBuffer", () => {
+  describe("parses the buffer", () => {
+    test.each([
+      [EncryptionType.AesCbc128_HmacSha256_B64, "AesCbc128_HmacSha256_B64"],
+      [EncryptionType.AesCbc256_HmacSha256_B64, "AesCbc256_HmacSha256_B64"],
+    ])("with %c%s", (encType: EncryptionType) => {
+      const iv = makeStaticByteArray(16, 10);
+      const mac = makeStaticByteArray(32, 20);
+      // We use the minimum data length of 1 to test the boundary of valid lengths
+      const data = makeStaticByteArray(1, 100);
+
+      const array = new Uint8Array(1 + iv.byteLength + mac.byteLength + data.byteLength);
+      array.set([encType]);
+      array.set(iv, 1);
+      array.set(mac, 1 + iv.byteLength);
+      array.set(data, 1 + iv.byteLength + mac.byteLength);
+
+      const actual = new EncArrayBuffer(array.buffer);
+
+      expect(actual.encryptionType).toEqual(encType);
+      expect(actual.ivBytes).toEqualBuffer(iv);
+      expect(actual.macBytes).toEqualBuffer(mac);
+      expect(actual.dataBytes).toEqualBuffer(data);
+    });
+
+    it("with AesCbc256_B64", () => {
+      const encType = EncryptionType.AesCbc256_B64;
+      const iv = makeStaticByteArray(16, 10);
+      // We use the minimum data length of 1 to test the boundary of valid lengths
+      const data = makeStaticByteArray(1, 100);
+
+      const array = new Uint8Array(1 + iv.byteLength + data.byteLength);
+      array.set([encType]);
+      array.set(iv, 1);
+      array.set(data, 1 + iv.byteLength);
+
+      const actual = new EncArrayBuffer(array.buffer);
+
+      expect(actual.encryptionType).toEqual(encType);
+      expect(actual.ivBytes).toEqualBuffer(iv);
+      expect(actual.dataBytes).toEqualBuffer(data);
+      expect(actual.macBytes).toBeNull();
+    });
+  });
+
+  describe("throws if the buffer has an invalid length", () => {
+    test.each([
+      [EncryptionType.AesCbc128_HmacSha256_B64, 50, "AesCbc128_HmacSha256_B64"],
+      [EncryptionType.AesCbc256_HmacSha256_B64, 50, "AesCbc256_HmacSha256_B64"],
+      [EncryptionType.AesCbc256_B64, 18, "AesCbc256_B64"],
+    ])("with %c%c%s", (encType: EncryptionType, minLength: number) => {
+      // Generate invalid byte array
+      // Minus 1 to leave room for the encType, minus 1 to make it invalid
+      const invalidBytes = makeStaticByteArray(minLength - 2);
+
+      const invalidArray = new Uint8Array(1 + invalidBytes.buffer.byteLength);
+      invalidArray.set([encType]);
+      invalidArray.set(invalidBytes, 1);
+
+      expect(() => new EncArrayBuffer(invalidArray.buffer)).toThrow(
+        "Error parsing encrypted ArrayBuffer"
+      );
+    });
+  });
+
+  it("doesn't parse the buffer if the encryptionType is not supported", () => {
+    // Starting at 9 implicitly gives us an invalid encType
+    const bytes = makeStaticByteArray(50, 9);
+    expect(() => new EncArrayBuffer(bytes)).toThrow("Error parsing encrypted ArrayBuffer");
+  });
+});

libs/common/spec/matchers/toEqualBuffer.spec.ts

@@ -0,0 +1,25 @@
+import { makeStaticByteArray } from "../utils";
+
+describe("toEqualBuffer custom matcher", () => {
+  it("matches identical ArrayBuffers", () => {
+    const array = makeStaticByteArray(10);
+    expect(array.buffer).toEqualBuffer(array.buffer);
+  });
+
+  it("matches an identical ArrayBuffer and Uint8Array", () => {
+    const array = makeStaticByteArray(10);
+    expect(array.buffer).toEqualBuffer(array);
+  });
+
+  it("doesn't match different ArrayBuffers", () => {
+    const array1 = makeStaticByteArray(10);
+    const array2 = makeStaticByteArray(10, 11);
+    expect(array1.buffer).not.toEqualBuffer(array2.buffer);
+  });
+
+  it("doesn't match a different ArrayBuffer and Uint8Array", () => {
+    const array1 = makeStaticByteArray(10);
+    const array2 = makeStaticByteArray(10, 11);
+    expect(array1.buffer).not.toEqualBuffer(array2);
+  });
+});

libs/common/spec/matchers/toEqualBuffer.ts

@@ -0,0 +1,34 @@
+/**
+ * The inbuilt toEqual() matcher will always return TRUE when provided with 2 ArrayBuffers.
+ * This is because an ArrayBuffer must be wrapped in a new Uint8Array to be accessible.
+ * This custom matcher will automatically instantiate a new Uint8Array on the recieved value
+ * (and optionally, the expected value) and then call toEqual() on the resulting Uint8Arrays.
+ */
+export const toEqualBuffer: jest.CustomMatcher = function (
+  received: ArrayBuffer,
+  expected: Uint8Array | ArrayBuffer
+) {
+  received = new Uint8Array(received);
+
+  if (expected instanceof ArrayBuffer) {
+    expected = new Uint8Array(expected);
+  }
+
+  if (this.equals(received, expected)) {
+    return {
+      message: () => `expected
+${received}
+not to match
+${expected}`,
+      pass: true,
+    };
+  }
+
+  return {
+    message: () => `expected
+${received}
+to match
+${expected}`,
+    pass: false,
+  };
+};

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

@@ -8,7 +8,6 @@ import { LogService } from "@bitwarden/common/abstractions/log.service";
 import { SearchService } from "@bitwarden/common/abstractions/search.service";
 import { SettingsService } from "@bitwarden/common/abstractions/settings.service";
 import { StateService } from "@bitwarden/common/abstractions/state.service";
-import { Utils } from "@bitwarden/common/misc/utils";
 import { Cipher } from "@bitwarden/common/models/domain/cipher";
 import { EncArrayBuffer } from "@bitwarden/common/models/domain/encArrayBuffer";
 import { EncString } from "@bitwarden/common/models/domain/encString";
@@ -16,7 +15,7 @@ import { SymmetricCryptoKey } from "@bitwarden/common/models/domain/symmetricCry
 import { CipherService } from "@bitwarden/common/services/cipher.service";
 
 const ENCRYPTED_TEXT = "This data has been encrypted";
-const ENCRYPTED_BYTES = new EncArrayBuffer(Utils.fromUtf8ToArray(ENCRYPTED_TEXT).buffer);
+const ENCRYPTED_BYTES = Substitute.for<EncArrayBuffer>();
 
 describe("Cipher Service", () => {
   let cryptoService: SubstituteOf<CryptoService>;

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

@@ -0,0 +1,38 @@
+import { mock, mockReset } from "jest-mock-extended";
+
+import { AbstractEncryptService } from "@bitwarden/common/abstractions/abstractEncrypt.service";
+import { CryptoFunctionService } from "@bitwarden/common/abstractions/cryptoFunction.service";
+import { LogService } from "@bitwarden/common/abstractions/log.service";
+import { PlatformUtilsService } from "@bitwarden/common/abstractions/platformUtils.service";
+import { StateService } from "@bitwarden/common/abstractions/state.service";
+import { CryptoService } from "@bitwarden/common/services/crypto.service";
+
+describe("cryptoService", () => {
+  let cryptoService: CryptoService;
+
+  const cryptoFunctionService = mock<CryptoFunctionService>();
+  const encryptService = mock<AbstractEncryptService>();
+  const platformUtilService = mock<PlatformUtilsService>();
+  const logService = mock<LogService>();
+  const stateService = mock<StateService>();
+
+  beforeEach(() => {
+    mockReset(cryptoFunctionService);
+    mockReset(encryptService);
+    mockReset(platformUtilService);
+    mockReset(logService);
+    mockReset(stateService);
+
+    cryptoService = new CryptoService(
+      cryptoFunctionService,
+      encryptService,
+      platformUtilService,
+      logService,
+      stateService
+    );
+  });
+
+  it("instantiates", () => {
+    expect(cryptoService).not.toBeFalsy();
+  });
+});

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

@@ -0,0 +1,163 @@
+import { mockReset, mock, MockProxy } from "jest-mock-extended";
+
+import { CryptoFunctionService } from "@bitwarden/common/abstractions/cryptoFunction.service";
+import { LogService } from "@bitwarden/common/abstractions/log.service";
+import { EncryptionType } from "@bitwarden/common/enums/encryptionType";
+import { EncArrayBuffer } from "@bitwarden/common/models/domain/encArrayBuffer";
+import { SymmetricCryptoKey } from "@bitwarden/common/models/domain/symmetricCryptoKey";
+import { EncryptService } from "@bitwarden/common/services/encrypt.service";
+
+import { makeStaticByteArray } from "../utils";
+
+describe("EncryptService", () => {
+  const cryptoFunctionService = mock<CryptoFunctionService>();
+  const logService = mock<LogService>();
+
+  let encryptService: EncryptService;
+
+  beforeEach(() => {
+    mockReset(cryptoFunctionService);
+    mockReset(logService);
+
+    encryptService = new EncryptService(cryptoFunctionService, logService, true);
+  });
+
+  describe("encryptToBytes", () => {
+    const plainValue = makeStaticByteArray(16, 1);
+    const iv = makeStaticByteArray(16, 30);
+    const mac = makeStaticByteArray(32, 40);
+    const encryptedData = makeStaticByteArray(20, 50);
+
+    it("throws if no key is provided", () => {
+      return expect(encryptService.encryptToBytes(plainValue, null)).rejects.toThrow(
+        "No encryption key"
+      );
+    });
+
+    describe("encrypts data", () => {
+      beforeEach(() => {
+        cryptoFunctionService.randomBytes.calledWith(16).mockResolvedValueOnce(iv.buffer);
+        cryptoFunctionService.aesEncrypt.mockResolvedValue(encryptedData.buffer);
+      });
+
+      it("using a key which supports mac", async () => {
+        const key = mock<SymmetricCryptoKey>();
+        const encType = EncryptionType.AesCbc128_HmacSha256_B64;
+        key.encType = encType;
+
+        key.macKey = makeStaticByteArray(16, 20);
+
+        cryptoFunctionService.hmac.mockResolvedValue(mac.buffer);
+
+        const actual = await encryptService.encryptToBytes(plainValue, key);
+
+        expect(actual.encryptionType).toEqual(encType);
+        expect(actual.ivBytes).toEqualBuffer(iv);
+        expect(actual.macBytes).toEqualBuffer(mac);
+        expect(actual.dataBytes).toEqualBuffer(encryptedData);
+        expect(actual.buffer.byteLength).toEqual(
+          1 + iv.byteLength + mac.byteLength + encryptedData.byteLength
+        );
+      });
+
+      it("using a key which doesn't support mac", async () => {
+        const key = mock<SymmetricCryptoKey>();
+        const encType = EncryptionType.AesCbc256_B64;
+        key.encType = encType;
+
+        key.macKey = null;
+
+        const actual = await encryptService.encryptToBytes(plainValue, key);
+
+        expect(cryptoFunctionService.hmac).not.toBeCalled();
+
+        expect(actual.encryptionType).toEqual(encType);
+        expect(actual.ivBytes).toEqualBuffer(iv);
+        expect(actual.macBytes).toBeNull();
+        expect(actual.dataBytes).toEqualBuffer(encryptedData);
+        expect(actual.buffer.byteLength).toEqual(1 + iv.byteLength + encryptedData.byteLength);
+      });
+    });
+  });
+
+  describe("decryptToBytes", () => {
+    const encType = EncryptionType.AesCbc256_HmacSha256_B64;
+    const key = new SymmetricCryptoKey(makeStaticByteArray(64, 100), encType);
+    const computedMac = new Uint8Array(1).buffer;
+    const encBuffer = new EncArrayBuffer(makeStaticByteArray(60, encType));
+
+    beforeEach(() => {
+      cryptoFunctionService.hmac.mockResolvedValue(computedMac);
+    });
+
+    it("throws if no key is provided", () => {
+      return expect(encryptService.decryptToBytes(encBuffer, null)).rejects.toThrow(
+        "No encryption key"
+      );
+    });
+
+    it("throws if no encrypted value is provided", () => {
+      return expect(encryptService.decryptToBytes(null, key)).rejects.toThrow(
+        "Nothing provided for decryption"
+      );
+    });
+
+    it("decrypts data with provided key", async () => {
+      const decryptedBytes = makeStaticByteArray(10, 200).buffer;
+
+      cryptoFunctionService.hmac.mockResolvedValue(makeStaticByteArray(1).buffer);
+      cryptoFunctionService.compare.mockResolvedValue(true);
+      cryptoFunctionService.aesDecrypt.mockResolvedValueOnce(decryptedBytes);
+
+      const actual = await encryptService.decryptToBytes(encBuffer, key);
+
+      expect(cryptoFunctionService.aesDecrypt).toBeCalledWith(
+        expect.toEqualBuffer(encBuffer.dataBytes),
+        expect.toEqualBuffer(encBuffer.ivBytes),
+        expect.toEqualBuffer(key.encKey)
+      );
+
+      expect(actual).toEqualBuffer(decryptedBytes);
+    });
+
+    it("compares macs using CryptoFunctionService", async () => {
+      const expectedMacData = new Uint8Array(
+        encBuffer.ivBytes.byteLength + encBuffer.dataBytes.byteLength
+      );
+      expectedMacData.set(new Uint8Array(encBuffer.ivBytes));
+      expectedMacData.set(new Uint8Array(encBuffer.dataBytes), encBuffer.ivBytes.byteLength);
+
+      await encryptService.decryptToBytes(encBuffer, key);
+
+      expect(cryptoFunctionService.hmac).toBeCalledWith(
+        expect.toEqualBuffer(expectedMacData),
+        key.macKey,
+        "sha256"
+      );
+
+      expect(cryptoFunctionService.compare).toBeCalledWith(
+        expect.toEqualBuffer(encBuffer.macBytes),
+        expect.toEqualBuffer(computedMac)
+      );
+    });
+
+    it("returns null if macs don't match", async () => {
+      cryptoFunctionService.compare.mockResolvedValue(false);
+
+      const actual = await encryptService.decryptToBytes(encBuffer, key);
+      expect(cryptoFunctionService.compare).toHaveBeenCalled();
+      expect(cryptoFunctionService.aesDecrypt).not.toHaveBeenCalled();
+      expect(actual).toBeNull();
+    });
+
+    it("returns null if encTypes don't match", async () => {
+      key.encType = EncryptionType.AesCbc256_B64;
+      cryptoFunctionService.compare.mockResolvedValue(true);
+
+      const actual = await encryptService.decryptToBytes(encBuffer, key);
+
+      expect(actual).toBeNull();
+      expect(cryptoFunctionService.aesDecrypt).not.toHaveBeenCalled();
+    });
+  });
+});

libs/common/spec/test.setup.ts

@@ -1,5 +1,27 @@
 import { webcrypto } from "crypto";
 
+import { toEqualBuffer } from "./matchers/toEqualBuffer";
+
 Object.defineProperty(window, "crypto", {
   value: webcrypto,
 });
+
+// Add custom matchers
+
+expect.extend({
+  toEqualBuffer: toEqualBuffer,
+});
+
+interface CustomMatchers<R = unknown> {
+  toEqualBuffer(expected: Uint8Array | ArrayBuffer): R;
+}
+
+/* eslint-disable */
+declare global {
+  namespace jest {
+    interface Expect extends CustomMatchers {}
+    interface Matchers<R> extends CustomMatchers<R> {}
+    interface InverseAsymmetricMatchers extends CustomMatchers {}
+  }
+}
+/* eslint-enable */

libs/common/spec/utils.ts

@@ -28,10 +28,10 @@ export function mockEnc(s: string): EncString {
   return mock;
 }
 
-export function makeStaticByteArray(length: number) {
+export function makeStaticByteArray(length: number, start = 0) {
   const arr = new Uint8Array(length);
   for (let i = 0; i < length; i++) {
-    arr[i] = i;
+    arr[i] = start + i;
   }
   return arr;
 }