diff --git a/libs/common/src/platform/abstractions/crypto-function.service.ts b/libs/common/src/platform/abstractions/crypto-function.service.ts index 18c14677dd0..56b0ee55afe 100644 --- a/libs/common/src/platform/abstractions/crypto-function.service.ts +++ b/libs/common/src/platform/abstractions/crypto-function.service.ts @@ -1,5 +1,5 @@ import { CsprngArray } from "../../types/csprng"; -import { DecryptParameters } from "../models/domain/decrypt-parameters"; +import { CbcDecryptParameters, EcbDecryptParameters } from "../models/domain/decrypt-parameters"; import { SymmetricCryptoKey } from "../models/domain/symmetric-crypto-key"; export abstract class CryptoFunctionService { @@ -51,11 +51,13 @@ export abstract class CryptoFunctionService { iv: string, mac: string, key: SymmetricCryptoKey, - ): DecryptParameters; - abstract aesDecryptFast( - parameters: DecryptParameters, - mode: "cbc" | "ecb", - ): Promise; + ): CbcDecryptParameters; + abstract aesDecryptFast({ + mode, + parameters, + }: + | { mode: "cbc"; parameters: CbcDecryptParameters } + | { mode: "ecb"; parameters: EcbDecryptParameters }): Promise; abstract aesDecrypt( data: Uint8Array, iv: Uint8Array, diff --git a/libs/common/src/platform/models/domain/decrypt-parameters.ts b/libs/common/src/platform/models/domain/decrypt-parameters.ts index 784826d3bd2..d3b4bf60d42 100644 --- a/libs/common/src/platform/models/domain/decrypt-parameters.ts +++ b/libs/common/src/platform/models/domain/decrypt-parameters.ts @@ -1,10 +1,13 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore -export class DecryptParameters { +export type CbcDecryptParameters = { encKey: T; data: T; iv: T; - macKey: T; - mac: T; + macKey?: T; + mac?: T; macData: T; -} +}; + +export type EcbDecryptParameters = { + encKey: T; + data: T; +}; diff --git a/libs/common/src/platform/models/domain/symmetric-crypto-key.ts b/libs/common/src/platform/models/domain/symmetric-crypto-key.ts index f467cb8d6e4..eab4c7b2114 100644 --- a/libs/common/src/platform/models/domain/symmetric-crypto-key.ts +++ b/libs/common/src/platform/models/domain/symmetric-crypto-key.ts @@ -7,7 +7,7 @@ import { EncryptionType } from "../../enums"; export class SymmetricCryptoKey { key: Uint8Array; - encKey?: Uint8Array; + encKey: Uint8Array; macKey?: Uint8Array; encType: EncryptionType; @@ -48,12 +48,8 @@ export class SymmetricCryptoKey { throw new Error("Unsupported encType/key length."); } - if (this.key != null) { - this.keyB64 = Utils.fromBufferToB64(this.key); - } - if (this.encKey != null) { - this.encKeyB64 = Utils.fromBufferToB64(this.encKey); - } + this.keyB64 = Utils.fromBufferToB64(this.key); + this.encKeyB64 = Utils.fromBufferToB64(this.encKey); if (this.macKey != null) { this.macKeyB64 = Utils.fromBufferToB64(this.macKey); } diff --git a/libs/common/src/platform/services/cryptography/encrypt.service.implementation.ts b/libs/common/src/platform/services/cryptography/encrypt.service.implementation.ts index db353f51c98..68263cadf27 100644 --- a/libs/common/src/platform/services/cryptography/encrypt.service.implementation.ts +++ b/libs/common/src/platform/services/cryptography/encrypt.service.implementation.ts @@ -125,7 +125,7 @@ export class EncryptServiceImplementation implements EncryptService { } } - return await this.cryptoFunctionService.aesDecryptFast(fastParams, "cbc"); + return await this.cryptoFunctionService.aesDecryptFast({ mode: "cbc", parameters: fastParams }); } async decryptToBytes( diff --git a/libs/common/src/platform/services/web-crypto-function.service.spec.ts b/libs/common/src/platform/services/web-crypto-function.service.spec.ts index 71f2828855f..1929e6454ef 100644 --- a/libs/common/src/platform/services/web-crypto-function.service.spec.ts +++ b/libs/common/src/platform/services/web-crypto-function.service.spec.ts @@ -2,7 +2,7 @@ import { mock } from "jest-mock-extended"; import { Utils } from "../../platform/misc/utils"; import { PlatformUtilsService } from "../abstractions/platform-utils.service"; -import { DecryptParameters } from "../models/domain/decrypt-parameters"; +import { EcbDecryptParameters } from "../models/domain/decrypt-parameters"; import { SymmetricCryptoKey } from "../models/domain/symmetric-crypto-key"; import { WebCryptoFunctionService } from "./web-crypto-function.service"; @@ -253,8 +253,13 @@ describe("WebCrypto Function Service", () => { const encData = Utils.fromBufferToB64(encValue); const b64Iv = Utils.fromBufferToB64(iv); const symKey = new SymmetricCryptoKey(key); - const params = cryptoFunctionService.aesDecryptFastParameters(encData, b64Iv, null, symKey); - const decValue = await cryptoFunctionService.aesDecryptFast(params, "cbc"); + const parameters = cryptoFunctionService.aesDecryptFastParameters( + encData, + b64Iv, + null, + symKey, + ); + const decValue = await cryptoFunctionService.aesDecryptFast({ mode: "cbc", parameters }); expect(decValue).toBe(value); }); @@ -276,8 +281,8 @@ describe("WebCrypto Function Service", () => { const iv = Utils.fromBufferToB64(makeStaticByteArray(16)); const symKey = new SymmetricCryptoKey(makeStaticByteArray(32)); const data = "ByUF8vhyX4ddU9gcooznwA=="; - const params = cryptoFunctionService.aesDecryptFastParameters(data, iv, null, symKey); - const decValue = await cryptoFunctionService.aesDecryptFast(params, "cbc"); + const parameters = cryptoFunctionService.aesDecryptFastParameters(data, iv, null, symKey); + const decValue = await cryptoFunctionService.aesDecryptFast({ mode: "cbc", parameters }); expect(decValue).toBe("EncryptMe!"); }); }); @@ -287,10 +292,11 @@ describe("WebCrypto Function Service", () => { const cryptoFunctionService = getWebCryptoFunctionService(); const key = makeStaticByteArray(32); const data = Utils.fromB64ToArray("z5q2XSxYCdQFdI+qK2yLlw=="); - const params = new DecryptParameters(); - params.encKey = Utils.fromBufferToByteString(key); - params.data = Utils.fromBufferToByteString(data); - const decValue = await cryptoFunctionService.aesDecryptFast(params, "ecb"); + const parameters: EcbDecryptParameters = { + encKey: Utils.fromBufferToByteString(key), + data: Utils.fromBufferToByteString(data), + }; + const decValue = await cryptoFunctionService.aesDecryptFast({ mode: "ecb", parameters }); expect(decValue).toBe("EncryptMe!"); }); }); @@ -304,6 +310,15 @@ describe("WebCrypto Function Service", () => { const decValue = await cryptoFunctionService.aesDecrypt(data, iv, key, "cbc"); expect(Utils.fromBufferToUtf8(decValue)).toBe("EncryptMe!"); }); + + it("throws if iv is not provided", async () => { + const cryptoFunctionService = getWebCryptoFunctionService(); + const key = makeStaticByteArray(32); + const data = Utils.fromB64ToArray("ByUF8vhyX4ddU9gcooznwA=="); + await expect(() => cryptoFunctionService.aesDecrypt(data, null, key, "cbc")).rejects.toThrow( + "IV is required for CBC mode", + ); + }); }); describe("aesDecrypt ECB mode", () => { diff --git a/libs/common/src/platform/services/web-crypto-function.service.ts b/libs/common/src/platform/services/web-crypto-function.service.ts index c0592654849..61edf7a13b1 100644 --- a/libs/common/src/platform/services/web-crypto-function.service.ts +++ b/libs/common/src/platform/services/web-crypto-function.service.ts @@ -1,12 +1,10 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import * as argon2 from "argon2-browser"; import * as forge from "node-forge"; import { Utils } from "../../platform/misc/utils"; import { CsprngArray } from "../../types/csprng"; import { CryptoFunctionService } from "../abstractions/crypto-function.service"; -import { DecryptParameters } from "../models/domain/decrypt-parameters"; +import { CbcDecryptParameters, EcbDecryptParameters } from "../models/domain/decrypt-parameters"; import { SymmetricCryptoKey } from "../models/domain/symmetric-crypto-key"; export class WebCryptoFunctionService implements CryptoFunctionService { @@ -14,10 +12,14 @@ export class WebCryptoFunctionService implements CryptoFunctionService { private subtle: SubtleCrypto; private wasmSupported: boolean; - constructor(globalContext: Window | typeof global) { - this.crypto = typeof globalContext.crypto !== "undefined" ? globalContext.crypto : null; - this.subtle = - !!this.crypto && typeof this.crypto.subtle !== "undefined" ? this.crypto.subtle : null; + constructor(globalContext: { crypto: Crypto }) { + if (globalContext?.crypto?.subtle == null) { + throw new Error( + "Could not instantiate WebCryptoFunctionService. Could not locate Subtle crypto.", + ); + } + this.crypto = globalContext.crypto; + this.subtle = this.crypto.subtle; this.wasmSupported = this.checkIfWasmSupported(); } @@ -220,7 +222,7 @@ export class WebCryptoFunctionService implements CryptoFunctionService { hmac.update(a); const mac1 = hmac.digest().getBytes(); - hmac.start(null, null); + hmac.start("sha256", null); hmac.update(b); const mac2 = hmac.digest().getBytes(); @@ -239,10 +241,10 @@ export class WebCryptoFunctionService implements CryptoFunctionService { aesDecryptFastParameters( data: string, iv: string, - mac: string, + mac: string | null, key: SymmetricCryptoKey, - ): DecryptParameters { - const p = new DecryptParameters(); + ): CbcDecryptParameters { + const p = {} as CbcDecryptParameters; if (key.meta != null) { p.encKey = key.meta.encKeyByteString; p.macKey = key.meta.macKeyByteString; @@ -275,7 +277,12 @@ export class WebCryptoFunctionService implements CryptoFunctionService { return p; } - aesDecryptFast(parameters: DecryptParameters, mode: "cbc" | "ecb"): Promise { + aesDecryptFast({ + mode, + parameters, + }: + | { mode: "cbc"; parameters: CbcDecryptParameters } + | { mode: "ecb"; parameters: EcbDecryptParameters }): Promise { const decipher = (forge as any).cipher.createDecipher( this.toWebCryptoAesMode(mode), parameters.encKey, @@ -294,21 +301,27 @@ export class WebCryptoFunctionService implements CryptoFunctionService { async aesDecrypt( data: Uint8Array, - iv: Uint8Array, + iv: Uint8Array | null, key: Uint8Array, mode: "cbc" | "ecb", ): Promise { if (mode === "ecb") { // Web crypto does not support AES-ECB mode, so we need to do this in forge. - const params = new DecryptParameters(); - params.data = this.toByteString(data); - params.encKey = this.toByteString(key); - const result = await this.aesDecryptFast(params, "ecb"); + const parameters: EcbDecryptParameters = { + data: this.toByteString(data), + encKey: this.toByteString(key), + }; + const result = await this.aesDecryptFast({ mode: "ecb", parameters }); return Utils.fromByteStringToArray(result); } const impKey = await this.subtle.importKey("raw", key, { name: "AES-CBC" } as any, false, [ "decrypt", ]); + + // CBC + if (iv == null) { + throw new Error("IV is required for CBC mode."); + } const buffer = await this.subtle.decrypt({ name: "AES-CBC", iv: iv }, impKey, data); return new Uint8Array(buffer); } diff --git a/libs/node/src/services/node-crypto-function.service.spec.ts b/libs/node/src/services/node-crypto-function.service.spec.ts index 61200b92855..3256d85110f 100644 --- a/libs/node/src/services/node-crypto-function.service.spec.ts +++ b/libs/node/src/services/node-crypto-function.service.spec.ts @@ -1,5 +1,5 @@ import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { DecryptParameters } from "@bitwarden/common/platform/models/domain/decrypt-parameters"; +import { EcbDecryptParameters } from "@bitwarden/common/platform/models/domain/decrypt-parameters"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { NodeCryptoFunctionService } from "./node-crypto-function.service"; @@ -193,8 +193,8 @@ describe("NodeCrypto Function Service", () => { const iv = Utils.fromBufferToB64(makeStaticByteArray(16)); const symKey = new SymmetricCryptoKey(makeStaticByteArray(32)); const data = "ByUF8vhyX4ddU9gcooznwA=="; - const params = nodeCryptoFunctionService.aesDecryptFastParameters(data, iv, null, symKey); - const decValue = await nodeCryptoFunctionService.aesDecryptFast(params, "cbc"); + const parameters = nodeCryptoFunctionService.aesDecryptFastParameters(data, iv, null, symKey); + const decValue = await nodeCryptoFunctionService.aesDecryptFast({ mode: "cbc", parameters }); expect(decValue).toBe("EncryptMe!"); }); }); @@ -202,10 +202,11 @@ describe("NodeCrypto Function Service", () => { describe("aesDecryptFast ECB mode", () => { it("should successfully decrypt data", async () => { const nodeCryptoFunctionService = new NodeCryptoFunctionService(); - const params = new DecryptParameters(); - params.encKey = makeStaticByteArray(32); - params.data = Utils.fromB64ToArray("z5q2XSxYCdQFdI+qK2yLlw=="); - const decValue = await nodeCryptoFunctionService.aesDecryptFast(params, "ecb"); + const parameters: EcbDecryptParameters = { + encKey: makeStaticByteArray(32), + data: Utils.fromB64ToArray("z5q2XSxYCdQFdI+qK2yLlw=="), + }; + const decValue = await nodeCryptoFunctionService.aesDecryptFast({ mode: "ecb", parameters }); expect(decValue).toBe("EncryptMe!"); }); }); @@ -219,6 +220,15 @@ describe("NodeCrypto Function Service", () => { const decValue = await nodeCryptoFunctionService.aesDecrypt(data, iv, key, "cbc"); expect(Utils.fromBufferToUtf8(decValue)).toBe("EncryptMe!"); }); + + it("throws if IV is not provided", async () => { + const nodeCryptoFunctionService = new NodeCryptoFunctionService(); + const key = makeStaticByteArray(32); + const data = Utils.fromB64ToArray("ByUF8vhyX4ddU9gcooznwA=="); + await expect( + async () => await nodeCryptoFunctionService.aesDecrypt(data, null, key, "cbc"), + ).rejects.toThrow("Invalid initialization vector"); + }); }); describe("aesDecrypt ECB mode", () => { @@ -454,7 +464,7 @@ function testHmac(algorithm: "sha1" | "sha256" | "sha512", mac: string, fast = f const cryptoFunctionService = new NodeCryptoFunctionService(); const value = Utils.fromUtf8ToArray("SignMe!!"); const key = Utils.fromUtf8ToArray("secretkey"); - let computedMac: ArrayBuffer = null; + let computedMac: ArrayBuffer; if (fast) { computedMac = await cryptoFunctionService.hmacFast(value, key, algorithm); } else { diff --git a/libs/node/src/services/node-crypto-function.service.ts b/libs/node/src/services/node-crypto-function.service.ts index 74d6de2e34c..c06f18023b4 100644 --- a/libs/node/src/services/node-crypto-function.service.ts +++ b/libs/node/src/services/node-crypto-function.service.ts @@ -1,12 +1,13 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import * as crypto from "crypto"; import * as forge from "node-forge"; import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { DecryptParameters } from "@bitwarden/common/platform/models/domain/decrypt-parameters"; +import { + CbcDecryptParameters, + EcbDecryptParameters, +} from "@bitwarden/common/platform/models/domain/decrypt-parameters"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { CsprngArray } from "@bitwarden/common/types/csprng"; @@ -168,10 +169,10 @@ export class NodeCryptoFunctionService implements CryptoFunctionService { aesDecryptFastParameters( data: string, iv: string, - mac: string, + mac: string | null, key: SymmetricCryptoKey, - ): DecryptParameters { - const p = new DecryptParameters(); + ): CbcDecryptParameters { + const p = {} as CbcDecryptParameters; p.encKey = key.encKey; p.data = Utils.fromB64ToArray(data); p.iv = Utils.fromB64ToArray(iv); @@ -191,22 +192,25 @@ export class NodeCryptoFunctionService implements CryptoFunctionService { return p; } - async aesDecryptFast( - parameters: DecryptParameters, - mode: "cbc" | "ecb", - ): Promise { - const decBuf = await this.aesDecrypt(parameters.data, parameters.iv, parameters.encKey, mode); + async aesDecryptFast({ + mode, + parameters, + }: + | { mode: "cbc"; parameters: CbcDecryptParameters } + | { mode: "ecb"; parameters: EcbDecryptParameters }): Promise { + const iv = mode === "cbc" ? parameters.iv : null; + const decBuf = await this.aesDecrypt(parameters.data, iv, parameters.encKey, mode); return Utils.fromBufferToUtf8(decBuf); } aesDecrypt( data: Uint8Array, - iv: Uint8Array, + iv: Uint8Array | null, key: Uint8Array, mode: "cbc" | "ecb", ): Promise { const nodeData = this.toNodeBuffer(data); - const nodeIv = mode === "ecb" ? null : this.toNodeBuffer(iv); + const nodeIv = this.toNodeBufferOrNull(iv); const nodeKey = this.toNodeBuffer(key); const decipher = crypto.createDecipheriv(this.toNodeCryptoAesMode(mode), nodeKey, nodeIv); const decBuf = Buffer.concat([decipher.update(nodeData), decipher.final()]); @@ -311,6 +315,13 @@ export class NodeCryptoFunctionService implements CryptoFunctionService { return Buffer.from(value); } + private toNodeBufferOrNull(value: Uint8Array | null): Buffer | null { + if (value == null) { + return null; + } + return this.toNodeBuffer(value); + } + private toUint8Buffer(value: Buffer | string | Uint8Array): Uint8Array { let buf: Uint8Array; if (typeof value === "string") {