From 8cfe331a005dbaddd47d5bda6b1c5ad392a99ccf Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 26 Feb 2025 15:43:31 +0100 Subject: [PATCH 1/6] Refactor symmetric keys and increase test coverage --- .../encrypt.service.implementation.ts | 257 ++++++++++-------- .../crypto/services/encrypt.service.spec.ts | 197 ++++++++++++-- .../models/domain/encrypted-object.ts | 5 - .../domain/symmetric-crypto-key.spec.ts | 53 +++- .../models/domain/symmetric-crypto-key.ts | 112 ++++++-- 5 files changed, 444 insertions(+), 180 deletions(-) diff --git a/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts b/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts index d426340c277..3e2d4e73b1c 100644 --- a/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts +++ b/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts @@ -13,7 +13,10 @@ import { Utils } from "@bitwarden/common/platform/misc/utils"; import { EncArrayBuffer } from "@bitwarden/common/platform/models/domain/enc-array-buffer"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { EncryptedObject } from "@bitwarden/common/platform/models/domain/encrypted-object"; -import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; +import { + Aes256CbcHmacKey, + SymmetricCryptoKey, +} from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { EncryptService } from "../abstractions/encrypt.service"; @@ -40,11 +43,16 @@ export class EncryptServiceImplementation implements EncryptService { plainBuf = plainValue; } - const encObj = await this.aesEncrypt(plainBuf, key); - const iv = Utils.fromBufferToB64(encObj.iv); - const data = Utils.fromBufferToB64(encObj.data); - const mac = encObj.mac != null ? Utils.fromBufferToB64(encObj.mac) : null; - return new EncString(encObj.key.encType, data, iv, mac); + const innerKey = key.inner(); + if (innerKey.type === EncryptionType.AesCbc256_HmacSha256_B64) { + const encObj = await this.aesEncrypt(plainBuf, innerKey); + const iv = Utils.fromBufferToB64(encObj.iv); + const data = Utils.fromBufferToB64(encObj.data); + const mac = Utils.fromBufferToB64(encObj.mac); + return new EncString(innerKey.type, data, iv, mac); + } else { + throw new Error(`Encrypt is not supported for keys of type ${innerKey.type}`); + } } async encryptToBytes(plainValue: Uint8Array, key: SymmetricCryptoKey): Promise { @@ -52,21 +60,21 @@ export class EncryptServiceImplementation implements EncryptService { throw new Error("No encryption key provided."); } - const encValue = await this.aesEncrypt(plainValue, key); - let macLen = 0; - if (encValue.mac != null) { - macLen = encValue.mac.byteLength; - } - - const encBytes = new Uint8Array(1 + encValue.iv.byteLength + macLen + encValue.data.byteLength); - encBytes.set([encValue.key.encType]); - encBytes.set(new Uint8Array(encValue.iv), 1); - if (encValue.mac != null) { + const innerKey = key.inner(); + if (innerKey.type === EncryptionType.AesCbc256_HmacSha256_B64) { + const encValue = await this.aesEncrypt(plainValue, innerKey); + const macLen = encValue.mac.length; + const encBytes = new Uint8Array( + 1 + encValue.iv.byteLength + macLen + encValue.data.byteLength, + ); + encBytes.set([innerKey.type]); + encBytes.set(new Uint8Array(encValue.iv), 1); encBytes.set(new Uint8Array(encValue.mac), 1 + encValue.iv.byteLength); + encBytes.set(new Uint8Array(encValue.data), 1 + encValue.iv.byteLength + macLen); + return new EncArrayBuffer(encBytes); + } else { + throw new Error(`Encrypt is not supported for keys of type ${innerKey.type}`); } - - encBytes.set(new Uint8Array(encValue.data), 1 + encValue.iv.byteLength + macLen); - return new EncArrayBuffer(encBytes); } async decryptToUtf8( @@ -78,36 +86,25 @@ export class EncryptServiceImplementation implements EncryptService { throw new Error("No key provided for decryption."); } - // DO NOT REMOVE OR MOVE. This prevents downgrade to mac-less CBC, which would compromise integrity and confidentiality. - if (key.macKey != null && encString?.mac == null) { - this.logService.error( - "[Encrypt service] Key has mac key but payload is missing mac bytes. Key type " + - encryptionTypeName(key.encType) + - "Payload type " + - encryptionTypeName(encString.encryptionType), - "Decrypt context: " + decryptContext, - ); - return null; - } + const innerKey = key.inner(); + if (innerKey.type === EncryptionType.AesCbc256_HmacSha256_B64) { + if (encString.encryptionType !== EncryptionType.AesCbc256_HmacSha256_B64) { + this.logDecryptError( + "Key encryption type does not match payload encryption type", + key.encType, + encString.encryptionType, + decryptContext, + ); + return null; + } - if (key.encType !== encString.encryptionType) { - this.logService.error( - "[Encrypt service] Key encryption type does not match payload encryption type. Key type " + - encryptionTypeName(key.encType) + - "Payload type " + - encryptionTypeName(encString.encryptionType), - "Decrypt context: " + decryptContext, + const fastParams = this.cryptoFunctionService.aesDecryptFastParameters( + encString.data, + encString.iv, + encString.mac, + key, ); - return null; - } - const fastParams = this.cryptoFunctionService.aesDecryptFastParameters( - encString.data, - encString.iv, - encString.mac, - key, - ); - if (fastParams.macKey != null && fastParams.mac != null) { const computedMac = await this.cryptoFunctionService.hmacFast( fastParams.macData, fastParams.macKey, @@ -116,18 +113,41 @@ export class EncryptServiceImplementation implements EncryptService { const macsEqual = await this.cryptoFunctionService.compareFast(fastParams.mac, computedMac); if (!macsEqual) { this.logMacFailed( - "[Encrypt service] decryptToUtf8 MAC comparison failed. Key or payload has changed. Key type " + - encryptionTypeName(key.encType) + - "Payload type " + - encryptionTypeName(encString.encryptionType) + - " Decrypt context: " + - decryptContext, + "decryptToUtf8 MAC comparison failed. Key or payload has changed.", + key.encType, + encString.encryptionType, + decryptContext, + ); + return null; + } + return await this.cryptoFunctionService.aesDecryptFast({ + mode: "cbc", + parameters: fastParams, + }); + } else if (innerKey.type === EncryptionType.AesCbc256_B64) { + if (encString.encryptionType !== EncryptionType.AesCbc256_B64) { + this.logDecryptError( + "Key encryption type does not match payload encryption type", + key.encType, + encString.encryptionType, + decryptContext, ); return null; } - } - return await this.cryptoFunctionService.aesDecryptFast({ mode: "cbc", parameters: fastParams }); + const fastParams = this.cryptoFunctionService.aesDecryptFastParameters( + encString.data, + encString.iv, + null, + key, + ); + return await this.cryptoFunctionService.aesDecryptFast({ + mode: "cbc", + parameters: fastParams, + }); + } else { + throw new Error(`Unsupported encryption type`); + } } async decryptToBytes( @@ -143,72 +163,60 @@ export class EncryptServiceImplementation implements EncryptService { throw new Error("Nothing provided for decryption."); } - // DO NOT REMOVE OR MOVE. This prevents downgrade to mac-less CBC, which would compromise integrity and confidentiality. - if (key.macKey != null && encThing.macBytes == null) { - this.logService.error( - "[Encrypt service] Key has mac key but payload is missing mac bytes. Key type " + - encryptionTypeName(key.encType) + - " Payload type " + - encryptionTypeName(encThing.encryptionType) + - " Decrypt context: " + + const inner = key.inner(); + if (inner.type === EncryptionType.AesCbc256_HmacSha256_B64) { + if ( + encThing.encryptionType !== EncryptionType.AesCbc256_HmacSha256_B64 || + encThing.macBytes === null + ) { + this.logDecryptError( + "Encryption key type mismatch", + key.encType, + encThing.encryptionType, decryptContext, - ); - return null; - } + ); + return null; + } - if (key.encType !== encThing.encryptionType) { - this.logService.error( - "[Encrypt service] Key encryption type does not match payload encryption type. Key type " + - encryptionTypeName(key.encType) + - " Payload type " + - encryptionTypeName(encThing.encryptionType) + - " Decrypt context: " + - decryptContext, - ); - return null; - } - - if (key.macKey != null && encThing.macBytes != null) { const macData = new Uint8Array(encThing.ivBytes.byteLength + encThing.dataBytes.byteLength); macData.set(new Uint8Array(encThing.ivBytes), 0); macData.set(new Uint8Array(encThing.dataBytes), encThing.ivBytes.byteLength); const computedMac = await this.cryptoFunctionService.hmac(macData, key.macKey, "sha256"); - if (computedMac === null) { - this.logMacFailed( - "[Encrypt service#decryptToBytes] Failed to compute MAC." + - " Key type " + - encryptionTypeName(key.encType) + - " Payload type " + - encryptionTypeName(encThing.encryptionType) + - " Decrypt context: " + - decryptContext, - ); - return null; - } - const macsMatch = await this.cryptoFunctionService.compare(encThing.macBytes, computedMac); if (!macsMatch) { this.logMacFailed( - "[Encrypt service#decryptToBytes]: MAC comparison failed. Key or payload has changed." + - " Key type " + - encryptionTypeName(key.encType) + - " Payload type " + - encryptionTypeName(encThing.encryptionType) + - " Decrypt context: " + - decryptContext, + "MAC comparison failed. Key or payload has changed.", + key.encType, + encThing.encryptionType, + decryptContext, ); return null; } + + return await this.cryptoFunctionService.aesDecrypt( + encThing.dataBytes, + encThing.ivBytes, + key.encKey, + "cbc", + ); + } else if (inner.type === EncryptionType.AesCbc256_B64) { + if (encThing.encryptionType !== EncryptionType.AesCbc256_B64) { + this.logDecryptError( + "Encryption key type mismatch", + key.encType, + encThing.encryptionType, + decryptContext, + ); + return null; + } + + return await this.cryptoFunctionService.aesDecrypt( + encThing.dataBytes, + encThing.ivBytes, + key.encKey, + "cbc", + ); } - - const result = await this.cryptoFunctionService.aesDecrypt( - encThing.dataBytes, - encThing.ivBytes, - key.encKey, - "cbc", - ); - - return result ?? null; } async rsaEncrypt(data: Uint8Array, publicKey: Uint8Array): Promise { @@ -273,25 +281,38 @@ export class EncryptServiceImplementation implements EncryptService { return Utils.fromBufferToB64(hashArray); } - private async aesEncrypt(data: Uint8Array, key: SymmetricCryptoKey): Promise { + private async aesEncrypt(data: Uint8Array, key: Aes256CbcHmacKey): Promise { const obj = new EncryptedObject(); - obj.key = key; obj.iv = await this.cryptoFunctionService.randomBytes(16); - obj.data = await this.cryptoFunctionService.aesEncrypt(data, obj.iv, obj.key.encKey); + obj.data = await this.cryptoFunctionService.aesEncrypt(data, obj.iv, key.encryptionKey); - if (obj.key.macKey != null) { - const macData = new Uint8Array(obj.iv.byteLength + obj.data.byteLength); - macData.set(new Uint8Array(obj.iv), 0); - macData.set(new Uint8Array(obj.data), obj.iv.byteLength); - obj.mac = await this.cryptoFunctionService.hmac(macData, obj.key.macKey, "sha256"); - } + const macData = new Uint8Array(obj.iv.byteLength + obj.data.byteLength); + macData.set(new Uint8Array(obj.iv), 0); + macData.set(new Uint8Array(obj.data), obj.iv.byteLength); + obj.mac = await this.cryptoFunctionService.hmac(macData, key.authenticationKey, "sha256"); return obj; } - private logMacFailed(msg: string) { + private logDecryptError( + msg: string, + keyEncType: EncryptionType, + dataEncType: EncryptionType, + decryptContext: string, + ) { + this.logService.error( + `[Encrypt service] ${msg} Key type ${encryptionTypeName(keyEncType)} Payload type ${encryptionTypeName(dataEncType)} Decrypt context: ${decryptContext}`, + ); + } + + private logMacFailed( + msg: string, + keyEncType: EncryptionType, + dataEncType: EncryptionType, + decryptContext: string, + ) { if (this.logMacFailures) { - this.logService.error(msg); + this.logDecryptError(msg, keyEncType, dataEncType, decryptContext); } } } diff --git a/libs/common/src/key-management/crypto/services/encrypt.service.spec.ts b/libs/common/src/key-management/crypto/services/encrypt.service.spec.ts index faa703cbecc..efc1a9dd307 100644 --- a/libs/common/src/key-management/crypto/services/encrypt.service.spec.ts +++ b/libs/common/src/key-management/crypto/services/encrypt.service.spec.ts @@ -37,12 +37,29 @@ describe("EncryptService", () => { const actual = await encryptService.encrypt(null, key); expect(actual).toBeNull(); }); + it("throws for AesCbc256_B64", async () => { + const key = new SymmetricCryptoKey(makeStaticByteArray(32)); + await expect(encryptService.encrypt("data", key)).rejects.toThrow( + "Encrypt is not supported for keys of type 0", + ); + }); + it("creates an EncString for AesCbc256_HmacSha256_B64", async () => { + const key = new SymmetricCryptoKey(makeStaticByteArray(64)); + const plainValue = "data"; + cryptoFunctionService.hmac.mockResolvedValue(makeStaticByteArray(32)); + cryptoFunctionService.aesEncrypt.mockResolvedValue(makeStaticByteArray(32)); + cryptoFunctionService.randomBytes.mockResolvedValue(makeStaticByteArray(16) as CsprngArray); + const result = await encryptService.encrypt(plainValue, key); + expect(cryptoFunctionService.aesEncrypt).toHaveBeenCalled(); + expect(cryptoFunctionService.hmac).toHaveBeenCalled(); + expect(Utils.fromB64ToArray(result.data).length).toEqual(32); + expect(Utils.fromB64ToArray(result.iv).length).toEqual(16); + expect(Utils.fromB64ToArray(result.mac).length).toEqual(32); + }); }); describe("encryptToBytes", () => { const plainValue = makeStaticByteArray(16, 1); - const iv = makeStaticByteArray(16, 30); - const encryptedData = makeStaticByteArray(20, 50); it("throws if no key is provided", () => { return expect(encryptService.encryptToBytes(plainValue, null)).rejects.toThrow( @@ -50,35 +67,40 @@ describe("EncryptService", () => { ); }); - describe("encrypts data", () => { - beforeEach(() => { - cryptoFunctionService.randomBytes.calledWith(16).mockResolvedValueOnce(iv as CsprngArray); - cryptoFunctionService.aesEncrypt.mockResolvedValue(encryptedData); - }); + it("throws for an AesCbc256_B64 key", async () => { + const key = new SymmetricCryptoKey(makeStaticByteArray(32, 100)); + key.encType = EncryptionType.AesCbc256_B64; - it("using a key which doesn't support mac", async () => { - const key = mock(); - const encType = EncryptionType.AesCbc256_B64; - key.encType = encType; + await expect(encryptService.encryptToBytes(plainValue, key)).rejects.toThrow( + "Encrypt is not supported for keys of type 0", + ); + }); - key.macKey = null; + it("encrypts data with provided key and returns correct encbuffer", async () => { + const key = new SymmetricCryptoKey(makeStaticByteArray(64, 0)); + const iv = makeStaticByteArray(16, 80); + const mac = makeStaticByteArray(32, 100); + const cipherText = makeStaticByteArray(20, 150); + cryptoFunctionService.randomBytes.mockResolvedValue(iv as CsprngArray); + cryptoFunctionService.aesEncrypt.mockResolvedValue(cipherText); + cryptoFunctionService.hmac.mockResolvedValue(mac); - const actual = await encryptService.encryptToBytes(plainValue, key); + const actual = await encryptService.encryptToBytes(plainValue, key); + const expectedBytes = new Uint8Array( + 1 + iv.byteLength + mac.byteLength + cipherText.byteLength, + ); + expectedBytes.set([EncryptionType.AesCbc256_HmacSha256_B64]); + expectedBytes.set(iv, 1); + expectedBytes.set(mac, 1 + iv.byteLength); + expectedBytes.set(cipherText, 1 + iv.byteLength + mac.byteLength); - 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); - }); + expect(actual.buffer).toEqualBuffer(expectedBytes); }); }); describe("decryptToBytes", () => { const encType = EncryptionType.AesCbc256_HmacSha256_B64; - const key = new SymmetricCryptoKey(makeStaticByteArray(64, 100), encType); + const key = new SymmetricCryptoKey(makeStaticByteArray(64, 100)); const computedMac = new Uint8Array(1); const encBuffer = new EncArrayBuffer(makeStaticByteArray(60, encType)); @@ -98,7 +120,28 @@ describe("EncryptService", () => { ); }); - it("decrypts data with provided key", async () => { + it("decrypts data with provided key for AesCbc256Hmac", async () => { + const decryptedBytes = makeStaticByteArray(10, 200); + + cryptoFunctionService.hmac.mockResolvedValue(makeStaticByteArray(1)); + 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), + "cbc", + ); + + expect(actual).toEqualBuffer(decryptedBytes); + }); + + it("decrypts data with provided key for AesCbc256", async () => { + const key = new SymmetricCryptoKey(makeStaticByteArray(32, 0)); + const encBuffer = new EncArrayBuffer(makeStaticByteArray(60, EncryptionType.AesCbc256_B64)); const decryptedBytes = makeStaticByteArray(10, 200); cryptoFunctionService.hmac.mockResolvedValue(makeStaticByteArray(1)); @@ -147,8 +190,8 @@ describe("EncryptService", () => { expect(actual).toBeNull(); }); - it("returns null if encTypes don't match", async () => { - key.encType = EncryptionType.AesCbc256_B64; + it("returns null if key is AesCbc256 but encbuffer is AesCbc256_HMAC", async () => { + const key = new SymmetricCryptoKey(makeStaticByteArray(32, 0)); cryptoFunctionService.compare.mockResolvedValue(true); const actual = await encryptService.decryptToBytes(encBuffer, key); @@ -156,6 +199,16 @@ describe("EncryptService", () => { expect(actual).toBeNull(); expect(cryptoFunctionService.aesDecrypt).not.toHaveBeenCalled(); }); + + it("returns null if key is AesCbc256_HMAC but encbuffer is AesCbc256", async () => { + const key = new SymmetricCryptoKey(makeStaticByteArray(64, 0)); + cryptoFunctionService.compare.mockResolvedValue(true); + const buffer = new EncArrayBuffer(makeStaticByteArray(200, EncryptionType.AesCbc256_B64)); + const actual = await encryptService.decryptToBytes(buffer, key); + + expect(actual).toBeNull(); + expect(cryptoFunctionService.aesDecrypt).not.toHaveBeenCalled(); + }); }); describe("decryptToUtf8", () => { @@ -164,17 +217,74 @@ describe("EncryptService", () => { "No key provided for decryption.", ); }); - it("returns null if key is mac key but encstring has no mac", async () => { - const key = new SymmetricCryptoKey( - makeStaticByteArray(64, 0), - EncryptionType.AesCbc256_HmacSha256_B64, - ); + + it("decrypts data with provided key for AesCbc256_HmacSha256", async () => { + const key = new SymmetricCryptoKey(makeStaticByteArray(64, 0)); + const encString = new EncString(EncryptionType.AesCbc256_HmacSha256_B64, "data"); + cryptoFunctionService.aesDecryptFastParameters.mockReturnValue({ + macData: makeStaticByteArray(32, 0), + macKey: makeStaticByteArray(32, 0), + mac: makeStaticByteArray(32, 0), + } as any); + cryptoFunctionService.hmacFast.mockResolvedValue(makeStaticByteArray(32, 0)); + cryptoFunctionService.compareFast.mockResolvedValue(true); + cryptoFunctionService.aesDecryptFast.mockResolvedValue("data"); + + const actual = await encryptService.decryptToUtf8(encString, key); + expect(actual).toEqual("data"); + expect(cryptoFunctionService.compareFast).toHaveBeenCalled(); + }); + + it("decrypts data with provided key for AesCbc256", async () => { + const key = new SymmetricCryptoKey(makeStaticByteArray(32, 0)); + const encString = new EncString(EncryptionType.AesCbc256_B64, "data"); + cryptoFunctionService.aesDecryptFastParameters.mockReturnValue({ + macData: makeStaticByteArray(32, 0), + macKey: makeStaticByteArray(32, 0), + mac: makeStaticByteArray(32, 0), + } as any); + cryptoFunctionService.hmacFast.mockResolvedValue(makeStaticByteArray(32, 0)); + cryptoFunctionService.compareFast.mockResolvedValue(true); + cryptoFunctionService.aesDecryptFast.mockResolvedValue("data"); + + const actual = await encryptService.decryptToUtf8(encString, key); + expect(actual).toEqual("data"); + expect(cryptoFunctionService.compareFast).not.toHaveBeenCalled(); + }); + + it("returns null if key is AesCbc256_HMAC but encstring is AesCbc256", async () => { + const key = new SymmetricCryptoKey(makeStaticByteArray(64, 0)); const encString = new EncString(EncryptionType.AesCbc256_B64, "data"); const actual = await encryptService.decryptToUtf8(encString, key); expect(actual).toBeNull(); expect(logService.error).toHaveBeenCalled(); }); + + it("returns null if key is AesCbc256 but encstring is AesCbc256_HMAC", async () => { + const key = new SymmetricCryptoKey(makeStaticByteArray(32, 0)); + const encString = new EncString(EncryptionType.AesCbc256_HmacSha256_B64, "data"); + + const actual = await encryptService.decryptToUtf8(encString, key); + expect(actual).toBeNull(); + expect(logService.error).toHaveBeenCalled(); + }); + + it("returns null if macs don't match", async () => { + const key = new SymmetricCryptoKey(makeStaticByteArray(64, 0)); + const encString = new EncString(EncryptionType.AesCbc256_HmacSha256_B64, "data"); + cryptoFunctionService.aesDecryptFastParameters.mockReturnValue({ + macData: makeStaticByteArray(32, 0), + macKey: makeStaticByteArray(32, 0), + mac: makeStaticByteArray(32, 0), + } as any); + cryptoFunctionService.hmacFast.mockResolvedValue(makeStaticByteArray(32, 0)); + cryptoFunctionService.compareFast.mockResolvedValue(false); + cryptoFunctionService.aesDecryptFast.mockResolvedValue("data"); + + const actual = await encryptService.decryptToUtf8(encString, key); + expect(actual).toBeNull(); + }); }); describe("rsa", () => { @@ -248,4 +358,31 @@ describe("EncryptService", () => { }); }); }); + + describe("hash", () => { + it("hashes a string and returns b64", async () => { + cryptoFunctionService.hash.mockResolvedValue(Uint8Array.from([1, 2, 3])); + expect(await encryptService.hash("test", "sha256")).toEqual("AQID"); + expect(cryptoFunctionService.hash).toHaveBeenCalledWith("test", "sha256"); + }); + }); + + describe("decryptItems", () => { + it("returns empty array if no items are provided", async () => { + const key = mock(); + const actual = await encryptService.decryptItems(null, key); + expect(actual).toEqual([]); + }); + + it("returns items decrypted with provided key", async () => { + const key = mock(); + const decryptable = { + decrypt: jest.fn().mockResolvedValue("decrypted"), + }; + const items = [decryptable]; + const actual = await encryptService.decryptItems(items as any, key); + expect(actual).toEqual(["decrypted"]); + expect(decryptable.decrypt).toHaveBeenCalledWith(key); + }); + }); }); diff --git a/libs/common/src/platform/models/domain/encrypted-object.ts b/libs/common/src/platform/models/domain/encrypted-object.ts index 92153b27636..1d54062e18f 100644 --- a/libs/common/src/platform/models/domain/encrypted-object.ts +++ b/libs/common/src/platform/models/domain/encrypted-object.ts @@ -1,10 +1,5 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore -import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; - export class EncryptedObject { iv: Uint8Array; data: Uint8Array; mac: Uint8Array; - key: SymmetricCryptoKey; } diff --git a/libs/common/src/platform/models/domain/symmetric-crypto-key.spec.ts b/libs/common/src/platform/models/domain/symmetric-crypto-key.spec.ts index 0186e2ebe24..d88ce76eb47 100644 --- a/libs/common/src/platform/models/domain/symmetric-crypto-key.spec.ts +++ b/libs/common/src/platform/models/domain/symmetric-crypto-key.spec.ts @@ -1,4 +1,6 @@ import { makeStaticByteArray } from "../../../../spec"; +import { EncryptionType } from "../../enums"; +import { Utils } from "../../misc/utils"; import { SymmetricCryptoKey } from "./symmetric-crypto-key"; @@ -19,10 +21,15 @@ describe("SymmetricCryptoKey", () => { expect(cryptoKey).toEqual({ encKey: key, encKeyB64: "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=", - encType: 0, + encType: EncryptionType.AesCbc256_B64, key: key, keyB64: "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=", macKey: null, + macKeyB64: null, + innerKey: { + type: EncryptionType.AesCbc256_B64, + encryptionKey: key, + }, }); }); @@ -33,12 +40,17 @@ describe("SymmetricCryptoKey", () => { expect(cryptoKey).toEqual({ encKey: key.slice(0, 32), encKeyB64: "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=", - encType: 2, + encType: EncryptionType.AesCbc256_HmacSha256_B64, key: key, keyB64: "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4vMDEyMzQ1Njc4OTo7PD0+Pw==", macKey: key.slice(32, 64), macKeyB64: "ICEiIyQlJicoKSorLC0uLzAxMjM0NTY3ODk6Ozw9Pj8=", + innerKey: { + type: EncryptionType.AesCbc256_HmacSha256_B64, + encryptionKey: key.slice(0, 32), + authenticationKey: key.slice(32), + }, }); }); @@ -47,7 +59,7 @@ describe("SymmetricCryptoKey", () => { new SymmetricCryptoKey(makeStaticByteArray(30)); }; - expect(t).toThrowError("Unable to determine encType."); + expect(t).toThrowError(`Unsupported encType/key length 30`); }); }); @@ -67,4 +79,39 @@ describe("SymmetricCryptoKey", () => { expect(actual).toEqual(expected); expect(actual).toBeInstanceOf(SymmetricCryptoKey); }); + + it("inner returns inner key", () => { + const key = new SymmetricCryptoKey(makeStaticByteArray(64)); + const actual = key.inner(); + + expect(actual).toEqual({ + type: EncryptionType.AesCbc256_HmacSha256_B64, + encryptionKey: key.encKey, + authenticationKey: key.macKey, + }); + }); + + it("toEncoded returns encoded key for AesCbc256_B64", () => { + const key = new SymmetricCryptoKey(makeStaticByteArray(32)); + const actual = key.toEncoded(); + + expect(actual).toEqual(key.encKey); + }); + + it("toEncoded returns encoded key for AesCbc256_HmacSha256_B64", () => { + const keyBytes = makeStaticByteArray(64); + const key = new SymmetricCryptoKey(keyBytes); + const actual = key.toEncoded(); + + expect(actual).toEqual(keyBytes); + }); + + it("toBase64 returns base64 encoded key", () => { + const keyBytes = makeStaticByteArray(64); + const keyB64 = Utils.fromBufferToB64(keyBytes); + const key = new SymmetricCryptoKey(keyBytes); + const actual = key.toBase64(); + + expect(actual).toEqual(keyB64); + }); }); 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 372b869fd9c..d5221fbee85 100644 --- a/libs/common/src/platform/models/domain/symmetric-crypto-key.ts +++ b/libs/common/src/platform/models/domain/symmetric-crypto-key.ts @@ -5,7 +5,25 @@ import { Jsonify } from "type-fest"; import { Utils } from "../../../platform/misc/utils"; import { EncryptionType } from "../../enums"; +export type Aes256CbcHmacKey = { + type: EncryptionType.AesCbc256_HmacSha256_B64; + encryptionKey: Uint8Array; + authenticationKey: Uint8Array; +}; + +export type Aes256CbcKey = { + type: EncryptionType.AesCbc256_B64; + encryptionKey: Uint8Array; +}; + +/** + * A symmetric crypto key represents a symmetric key usable for symmetric encryption and decryption operations. + * The specific algorithm used is private to the key, and should only be exposed to encrypt service implementations. + * This can be done via `inner()`. + */ export class SymmetricCryptoKey { + private innerKey: Aes256CbcHmacKey | Aes256CbcKey; + key: Uint8Array; encKey: Uint8Array; macKey?: Uint8Array; @@ -17,38 +35,45 @@ export class SymmetricCryptoKey { meta: any; - constructor(key: Uint8Array, encType?: EncryptionType) { + /** + * @param key The key in one of the permitted serialization formats + */ + constructor(key: Uint8Array) { if (key == null) { throw new Error("Must provide key"); } - if (encType == null) { - if (key.byteLength === 32) { - encType = EncryptionType.AesCbc256_B64; - } else if (key.byteLength === 64) { - encType = EncryptionType.AesCbc256_HmacSha256_B64; - } else { - throw new Error("Unable to determine encType."); - } - } + if (key.byteLength === 32) { + this.innerKey = { + type: EncryptionType.AesCbc256_B64, + encryptionKey: key, + }; + this.encType = EncryptionType.AesCbc256_B64; + this.key = key; + this.keyB64 = Utils.fromBufferToB64(this.key); - this.key = key; - this.encType = encType; - - if (encType === EncryptionType.AesCbc256_B64 && key.byteLength === 32) { this.encKey = key; - this.macKey = null; - } else if (encType === EncryptionType.AesCbc256_HmacSha256_B64 && key.byteLength === 64) { - this.encKey = key.slice(0, 32); - this.macKey = key.slice(32, 64); - } else { - throw new Error("Unsupported encType/key length."); - } + this.encKeyB64 = Utils.fromBufferToB64(this.encKey); - this.keyB64 = Utils.fromBufferToB64(this.key); - this.encKeyB64 = Utils.fromBufferToB64(this.encKey); - if (this.macKey != null) { + this.macKey = null; + this.macKeyB64 = null; + } else if (key.byteLength === 64) { + this.innerKey = { + type: EncryptionType.AesCbc256_HmacSha256_B64, + encryptionKey: key.slice(0, 32), + authenticationKey: key.slice(32), + }; + this.encType = EncryptionType.AesCbc256_HmacSha256_B64; + this.key = key; + this.keyB64 = Utils.fromBufferToB64(this.key); + + this.encKey = key.slice(0, 32); + this.encKeyB64 = Utils.fromBufferToB64(this.encKey); + + this.macKey = key.slice(32); this.macKeyB64 = Utils.fromBufferToB64(this.macKey); + } else { + throw new Error(`Unsupported encType/key length ${key.byteLength}`); } } @@ -57,6 +82,45 @@ export class SymmetricCryptoKey { return { keyB64: this.keyB64 }; } + /** + * @returns The inner key instance that can be directly used for encryption primitives + */ + inner(): Aes256CbcHmacKey | Aes256CbcKey { + return this.innerKey; + } + + /** + * @returns The serialized key in base64 format + */ + toBase64(): string { + return Utils.fromBufferToB64(this.toEncoded()); + } + + /** + * Serializes the key to a format that can be written to state or shared + * The currently permitted format is: + * - AesCbc256_B64: 32 bytes (the raw key) + * - AesCbc256_HmacSha256_B64: 64 bytes (32 bytes encryption key, 32 bytes authentication key, concatenated) + * + * @returns The serialized key that can be written to state or encrypted and then written to state / shared + */ + toEncoded(): Uint8Array { + if (this.innerKey.type === EncryptionType.AesCbc256_B64) { + return this.innerKey.encryptionKey; + } else if (this.innerKey.type === EncryptionType.AesCbc256_HmacSha256_B64) { + const encodedKey = new Uint8Array(64); + encodedKey.set(this.innerKey.encryptionKey, 0); + encodedKey.set(this.innerKey.authenticationKey, 32); + return encodedKey; + } else { + throw new Error("Unsupported encryption type."); + } + } + + /** + * @param s The serialized key in base64 format + * @returns A SymmetricCryptoKey instance + */ static fromString(s: string): SymmetricCryptoKey { if (s == null) { return null; From c8ab140000750f40aed4bba3f8e39fc2a55cbf5a Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Fri, 28 Feb 2025 16:13:32 +0100 Subject: [PATCH 2/6] Re-add type 0 encryption --- .../encrypt.service.implementation.ts | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts b/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts index 3e2d4e73b1c..4e91ebc47fc 100644 --- a/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts +++ b/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts @@ -15,6 +15,7 @@ import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { EncryptedObject } from "@bitwarden/common/platform/models/domain/encrypted-object"; import { Aes256CbcHmacKey, + Aes256CbcKey, SymmetricCryptoKey, } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; @@ -72,8 +73,13 @@ export class EncryptServiceImplementation implements EncryptService { encBytes.set(new Uint8Array(encValue.mac), 1 + encValue.iv.byteLength); encBytes.set(new Uint8Array(encValue.data), 1 + encValue.iv.byteLength + macLen); return new EncArrayBuffer(encBytes); - } else { - throw new Error(`Encrypt is not supported for keys of type ${innerKey.type}`); + } else if (innerKey.type === EncryptionType.AesCbc256_B64) { + const encValue = await this.aesEncryptLegacy(plainValue, innerKey); + const encBytes = new Uint8Array(1 + encValue.iv.byteLength + encValue.data.byteLength); + encBytes.set([innerKey.type]); + encBytes.set(new Uint8Array(encValue.iv), 1); + encBytes.set(new Uint8Array(encValue.data), 1 + encValue.iv.byteLength); + return new EncArrayBuffer(encBytes); } } @@ -294,6 +300,16 @@ export class EncryptServiceImplementation implements EncryptService { return obj; } + /** + * @deprecated Removed once AesCbc256_B64 support is removed + */ + private async aesEncryptLegacy(data: Uint8Array, key: Aes256CbcKey): Promise { + const obj = new EncryptedObject(); + obj.iv = await this.cryptoFunctionService.randomBytes(16); + obj.data = await this.cryptoFunctionService.aesEncrypt(data, obj.iv, key.encryptionKey); + return obj; + } + private logDecryptError( msg: string, keyEncType: EncryptionType, From b7e2d28ead3b9ff26b4762fd35522c0dd17b380e Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Fri, 28 Feb 2025 16:35:31 +0100 Subject: [PATCH 3/6] Fix ts strict warning --- libs/common/src/platform/models/domain/encrypted-object.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libs/common/src/platform/models/domain/encrypted-object.ts b/libs/common/src/platform/models/domain/encrypted-object.ts index 1d54062e18f..3caa7ae68d1 100644 --- a/libs/common/src/platform/models/domain/encrypted-object.ts +++ b/libs/common/src/platform/models/domain/encrypted-object.ts @@ -1,3 +1,6 @@ +// FIXME: Update this file to be type safe and remove this and next line +// @ts-strict-ignore + export class EncryptedObject { iv: Uint8Array; data: Uint8Array; From b4226cdd949c3ab338be05d573f39d026635abe5 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Fri, 28 Feb 2025 18:00:34 +0100 Subject: [PATCH 4/6] Remove old symmetric key representations in symmetriccryptokey --- .../os-biometrics-windows.service.ts | 10 +++- .../auth-request/auth-request.service.spec.ts | 6 ++- .../auth-request/auth-request.service.ts | 7 ++- .../services/key-connector.service.spec.ts | 8 ++- .../auth/services/key-connector.service.ts | 8 ++- .../encrypt.service.implementation.ts | 22 ++++---- .../crypto/services/encrypt.service.spec.ts | 11 ++-- .../domain/symmetric-crypto-key.spec.ts | 18 ++----- .../models/domain/symmetric-crypto-key.ts | 26 +--------- .../services/web-crypto-function.service.ts | 50 ++++++++----------- .../services/node-crypto-function.service.ts | 40 +++++++++------ 11 files changed, 101 insertions(+), 105 deletions(-) diff --git a/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.ts b/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.ts index cd8c94329bc..53647549295 100644 --- a/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.ts +++ b/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.ts @@ -1,5 +1,7 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { EncryptionType } from "@bitwarden/common/platform/enums"; +import { Utils } from "@bitwarden/common/platform/misc/utils"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { biometrics, passwords } from "@bitwarden/desktop-napi"; @@ -218,7 +220,13 @@ export default class OsBiometricsServiceWindows implements OsBiometricService { symmetricKey: SymmetricCryptoKey, clientKeyPartB64: string | undefined, ): biometrics.KeyMaterial { - const key = symmetricKey?.macKeyB64 ?? symmetricKey?.keyB64; + let key = null; + const innerKey = symmetricKey.inner(); + if (innerKey.type === EncryptionType.AesCbc256_HmacSha256_B64) { + key = Utils.fromBufferToB64(innerKey.authenticationKey); + } else { + key = Utils.fromBufferToB64(innerKey.encryptionKey); + } const result = { osKeyPartB64: key, diff --git a/libs/auth/src/common/services/auth-request/auth-request.service.spec.ts b/libs/auth/src/common/services/auth-request/auth-request.service.spec.ts index 2ea6d427641..76b965511a8 100644 --- a/libs/auth/src/common/services/auth-request/auth-request.service.spec.ts +++ b/libs/auth/src/common/services/auth-request/auth-request.service.spec.ts @@ -103,7 +103,9 @@ describe("AuthRequestService", () => { }); it("should use the master key and hash if they exist", async () => { - masterPasswordService.masterKeySubject.next({ encKey: new Uint8Array(64) } as MasterKey); + masterPasswordService.masterKeySubject.next( + new SymmetricCryptoKey(new Uint8Array(32)) as MasterKey, + ); masterPasswordService.masterKeyHashSubject.next("MASTER_KEY_HASH"); await sut.approveOrDenyAuthRequest( @@ -111,7 +113,7 @@ describe("AuthRequestService", () => { new AuthRequestResponse({ id: "123", publicKey: "KEY" }), ); - expect(encryptService.rsaEncrypt).toHaveBeenCalledWith(new Uint8Array(64), expect.anything()); + expect(encryptService.rsaEncrypt).toHaveBeenCalledWith(new Uint8Array(32), expect.anything()); }); it("should use the user key if the master key and hash do not exist", async () => { diff --git a/libs/auth/src/common/services/auth-request/auth-request.service.ts b/libs/auth/src/common/services/auth-request/auth-request.service.ts index 5bc200ae1e8..41b3d4805f4 100644 --- a/libs/auth/src/common/services/auth-request/auth-request.service.ts +++ b/libs/auth/src/common/services/auth-request/auth-request.service.ts @@ -14,7 +14,10 @@ import { AuthRequestPushNotification } from "@bitwarden/common/models/response/n import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; -import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; +import { + Aes256CbcKey, + SymmetricCryptoKey, +} from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { AUTH_REQUEST_DISK_LOCAL, StateProvider, @@ -111,7 +114,7 @@ export class AuthRequestService implements AuthRequestServiceAbstraction { Utils.fromUtf8ToArray(masterKeyHash), pubKey, ); - keyToEncrypt = masterKey.encKey; + keyToEncrypt = (masterKey.inner() as Aes256CbcKey).encryptionKey; } else { const userKey = await this.keyService.getUserKey(); keyToEncrypt = userKey.key; diff --git a/libs/common/src/auth/services/key-connector.service.spec.ts b/libs/common/src/auth/services/key-connector.service.spec.ts index ec03c7ece55..d67ddf65bd3 100644 --- a/libs/common/src/auth/services/key-connector.service.spec.ts +++ b/libs/common/src/auth/services/key-connector.service.spec.ts @@ -252,7 +252,9 @@ describe("KeyConnectorService", () => { const organization = organizationData(true, true, "https://key-connector-url.com", 2, false); const masterKey = getMockMasterKey(); masterPasswordService.masterKeySubject.next(masterKey); - const keyConnectorRequest = new KeyConnectorUserKeyRequest(masterKey.encKeyB64); + const keyConnectorRequest = new KeyConnectorUserKeyRequest( + Utils.fromBufferToB64(masterKey.inner().encryptionKey), + ); jest.spyOn(keyConnectorService, "getManagingOrganization").mockResolvedValue(organization); jest.spyOn(apiService, "postUserKeyToKeyConnector").mockResolvedValue(); @@ -273,7 +275,9 @@ describe("KeyConnectorService", () => { // Arrange const organization = organizationData(true, true, "https://key-connector-url.com", 2, false); const masterKey = getMockMasterKey(); - const keyConnectorRequest = new KeyConnectorUserKeyRequest(masterKey.encKeyB64); + const keyConnectorRequest = new KeyConnectorUserKeyRequest( + Utils.fromBufferToB64(masterKey.inner().encryptionKey), + ); const error = new Error("Failed to post user key to key connector"); organizationService.organizations$.mockReturnValue(of([organization])); diff --git a/libs/common/src/auth/services/key-connector.service.ts b/libs/common/src/auth/services/key-connector.service.ts index f6f76579ee5..081459556db 100644 --- a/libs/common/src/auth/services/key-connector.service.ts +++ b/libs/common/src/auth/services/key-connector.service.ts @@ -95,7 +95,9 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { userId ??= (await firstValueFrom(this.accountService.activeAccount$))?.id; const organization = await this.getManagingOrganization(userId); const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); - const keyConnectorRequest = new KeyConnectorUserKeyRequest(masterKey.encKeyB64); + const keyConnectorRequest = new KeyConnectorUserKeyRequest( + Utils.fromBufferToB64(masterKey.inner().encryptionKey), + ); try { await this.apiService.postUserKeyToKeyConnector( @@ -157,7 +159,9 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { await this.tokenService.getEmail(), kdfConfig, ); - const keyConnectorRequest = new KeyConnectorUserKeyRequest(masterKey.encKeyB64); + const keyConnectorRequest = new KeyConnectorUserKeyRequest( + Utils.fromBufferToB64(masterKey.inner().encryptionKey), + ); await this.masterPasswordService.setMasterKey(masterKey, userId); const userKey = await this.keyService.makeUserKey(masterKey); diff --git a/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts b/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts index 4e91ebc47fc..f939cacca1f 100644 --- a/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts +++ b/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts @@ -97,7 +97,7 @@ export class EncryptServiceImplementation implements EncryptService { if (encString.encryptionType !== EncryptionType.AesCbc256_HmacSha256_B64) { this.logDecryptError( "Key encryption type does not match payload encryption type", - key.encType, + key.inner().type, encString.encryptionType, decryptContext, ); @@ -120,7 +120,7 @@ export class EncryptServiceImplementation implements EncryptService { if (!macsEqual) { this.logMacFailed( "decryptToUtf8 MAC comparison failed. Key or payload has changed.", - key.encType, + key.inner().type, encString.encryptionType, decryptContext, ); @@ -134,7 +134,7 @@ export class EncryptServiceImplementation implements EncryptService { if (encString.encryptionType !== EncryptionType.AesCbc256_B64) { this.logDecryptError( "Key encryption type does not match payload encryption type", - key.encType, + key.inner().type, encString.encryptionType, decryptContext, ); @@ -177,7 +177,7 @@ export class EncryptServiceImplementation implements EncryptService { ) { this.logDecryptError( "Encryption key type mismatch", - key.encType, + inner.type, encThing.encryptionType, decryptContext, ); @@ -187,12 +187,16 @@ export class EncryptServiceImplementation implements EncryptService { const macData = new Uint8Array(encThing.ivBytes.byteLength + encThing.dataBytes.byteLength); macData.set(new Uint8Array(encThing.ivBytes), 0); macData.set(new Uint8Array(encThing.dataBytes), encThing.ivBytes.byteLength); - const computedMac = await this.cryptoFunctionService.hmac(macData, key.macKey, "sha256"); + const computedMac = await this.cryptoFunctionService.hmac( + macData, + inner.authenticationKey, + "sha256", + ); const macsMatch = await this.cryptoFunctionService.compare(encThing.macBytes, computedMac); if (!macsMatch) { this.logMacFailed( "MAC comparison failed. Key or payload has changed.", - key.encType, + inner.type, encThing.encryptionType, decryptContext, ); @@ -202,14 +206,14 @@ export class EncryptServiceImplementation implements EncryptService { return await this.cryptoFunctionService.aesDecrypt( encThing.dataBytes, encThing.ivBytes, - key.encKey, + inner.encryptionKey, "cbc", ); } else if (inner.type === EncryptionType.AesCbc256_B64) { if (encThing.encryptionType !== EncryptionType.AesCbc256_B64) { this.logDecryptError( "Encryption key type mismatch", - key.encType, + inner.type, encThing.encryptionType, decryptContext, ); @@ -219,7 +223,7 @@ export class EncryptServiceImplementation implements EncryptService { return await this.cryptoFunctionService.aesDecrypt( encThing.dataBytes, encThing.ivBytes, - key.encKey, + inner.encryptionKey, "cbc", ); } diff --git a/libs/common/src/key-management/crypto/services/encrypt.service.spec.ts b/libs/common/src/key-management/crypto/services/encrypt.service.spec.ts index b5a934e5191..d5f214d7931 100644 --- a/libs/common/src/key-management/crypto/services/encrypt.service.spec.ts +++ b/libs/common/src/key-management/crypto/services/encrypt.service.spec.ts @@ -6,7 +6,10 @@ import { EncryptionType } from "@bitwarden/common/platform/enums"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { EncArrayBuffer } from "@bitwarden/common/platform/models/domain/enc-array-buffer"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; -import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; +import { + Aes256CbcHmacKey, + SymmetricCryptoKey, +} from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { CsprngArray } from "@bitwarden/common/types/csprng"; import { makeStaticByteArray } from "../../../../spec"; @@ -133,7 +136,7 @@ describe("EncryptService", () => { expect(cryptoFunctionService.aesDecrypt).toBeCalledWith( expect.toEqualBuffer(encBuffer.dataBytes), expect.toEqualBuffer(encBuffer.ivBytes), - expect.toEqualBuffer(key.encKey), + expect.toEqualBuffer(key.inner().encryptionKey), "cbc", ); @@ -154,7 +157,7 @@ describe("EncryptService", () => { expect(cryptoFunctionService.aesDecrypt).toBeCalledWith( expect.toEqualBuffer(encBuffer.dataBytes), expect.toEqualBuffer(encBuffer.ivBytes), - expect.toEqualBuffer(key.encKey), + expect.toEqualBuffer(key.inner().encryptionKey), "cbc", ); @@ -172,7 +175,7 @@ describe("EncryptService", () => { expect(cryptoFunctionService.hmac).toBeCalledWith( expect.toEqualBuffer(expectedMacData), - key.macKey, + (key.inner() as Aes256CbcHmacKey).authenticationKey, "sha256", ); diff --git a/libs/common/src/platform/models/domain/symmetric-crypto-key.spec.ts b/libs/common/src/platform/models/domain/symmetric-crypto-key.spec.ts index 30c3f5c071a..6b641ad443a 100644 --- a/libs/common/src/platform/models/domain/symmetric-crypto-key.spec.ts +++ b/libs/common/src/platform/models/domain/symmetric-crypto-key.spec.ts @@ -2,7 +2,7 @@ import { makeStaticByteArray } from "../../../../spec"; import { EncryptionType } from "../../enums"; import { Utils } from "../../misc/utils"; -import { SymmetricCryptoKey } from "./symmetric-crypto-key"; +import { Aes256CbcHmacKey, SymmetricCryptoKey } from "./symmetric-crypto-key"; describe("SymmetricCryptoKey", () => { it("errors if no key", () => { @@ -19,13 +19,8 @@ describe("SymmetricCryptoKey", () => { const cryptoKey = new SymmetricCryptoKey(key); expect(cryptoKey).toEqual({ - encKey: key, - encKeyB64: "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=", - encType: EncryptionType.AesCbc256_B64, key: key, keyB64: "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=", - macKey: null, - macKeyB64: null, innerKey: { type: EncryptionType.AesCbc256_B64, encryptionKey: key, @@ -38,14 +33,9 @@ describe("SymmetricCryptoKey", () => { const cryptoKey = new SymmetricCryptoKey(key); expect(cryptoKey).toEqual({ - encKey: key.slice(0, 32), - encKeyB64: "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=", - encType: EncryptionType.AesCbc256_HmacSha256_B64, key: key, keyB64: "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4vMDEyMzQ1Njc4OTo7PD0+Pw==", - macKey: key.slice(32, 64), - macKeyB64: "ICEiIyQlJicoKSorLC0uLzAxMjM0NTY3ODk6Ozw9Pj8=", innerKey: { type: EncryptionType.AesCbc256_HmacSha256_B64, encryptionKey: key.slice(0, 32), @@ -86,8 +76,8 @@ describe("SymmetricCryptoKey", () => { expect(actual).toEqual({ type: EncryptionType.AesCbc256_HmacSha256_B64, - encryptionKey: key.encKey, - authenticationKey: key.macKey, + encryptionKey: key.inner().encryptionKey, + authenticationKey: (key.inner() as Aes256CbcHmacKey).authenticationKey, }); }); @@ -95,7 +85,7 @@ describe("SymmetricCryptoKey", () => { const key = new SymmetricCryptoKey(makeStaticByteArray(32)); const actual = key.toEncoded(); - expect(actual).toEqual(key.encKey); + expect(actual).toEqual(key.inner().encryptionKey); }); it("toEncoded returns encoded key for AesCbc256_HmacSha256_B64", () => { 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 d5221fbee85..010483eefd2 100644 --- a/libs/common/src/platform/models/domain/symmetric-crypto-key.ts +++ b/libs/common/src/platform/models/domain/symmetric-crypto-key.ts @@ -25,15 +25,7 @@ export class SymmetricCryptoKey { private innerKey: Aes256CbcHmacKey | Aes256CbcKey; key: Uint8Array; - encKey: Uint8Array; - macKey?: Uint8Array; - encType: EncryptionType; - keyB64: string; - encKeyB64: string; - macKeyB64: string; - - meta: any; /** * @param key The key in one of the permitted serialization formats @@ -48,30 +40,16 @@ export class SymmetricCryptoKey { type: EncryptionType.AesCbc256_B64, encryptionKey: key, }; - this.encType = EncryptionType.AesCbc256_B64; this.key = key; - this.keyB64 = Utils.fromBufferToB64(this.key); - - this.encKey = key; - this.encKeyB64 = Utils.fromBufferToB64(this.encKey); - - this.macKey = null; - this.macKeyB64 = null; + this.keyB64 = this.toBase64(); } else if (key.byteLength === 64) { this.innerKey = { type: EncryptionType.AesCbc256_HmacSha256_B64, encryptionKey: key.slice(0, 32), authenticationKey: key.slice(32), }; - this.encType = EncryptionType.AesCbc256_HmacSha256_B64; this.key = key; - this.keyB64 = Utils.fromBufferToB64(this.key); - - this.encKey = key.slice(0, 32); - this.encKeyB64 = Utils.fromBufferToB64(this.encKey); - - this.macKey = key.slice(32); - this.macKeyB64 = Utils.fromBufferToB64(this.macKey); + this.keyB64 = this.toBase64(); } else { throw new Error(`Unsupported encType/key length ${key.byteLength}`); } 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 61edf7a13b1..a6edccf6e47 100644 --- a/libs/common/src/platform/services/web-crypto-function.service.ts +++ b/libs/common/src/platform/services/web-crypto-function.service.ts @@ -4,6 +4,7 @@ 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 { EncryptionType } from "../enums"; import { CbcDecryptParameters, EcbDecryptParameters } from "../models/domain/decrypt-parameters"; import { SymmetricCryptoKey } from "../models/domain/symmetric-crypto-key"; @@ -244,37 +245,26 @@ export class WebCryptoFunctionService implements CryptoFunctionService { mac: string | null, key: SymmetricCryptoKey, ): CbcDecryptParameters { - const p = {} as CbcDecryptParameters; - if (key.meta != null) { - p.encKey = key.meta.encKeyByteString; - p.macKey = key.meta.macKeyByteString; + const innerKey = key.inner(); + if (innerKey.type === EncryptionType.AesCbc256_B64) { + return { + iv: forge.util.decode64(iv), + data: forge.util.decode64(data), + encKey: forge.util.createBuffer(innerKey.encryptionKey).getBytes(), + } as CbcDecryptParameters; + } else if (innerKey.type === EncryptionType.AesCbc256_HmacSha256_B64) { + const macData = forge.util.decode64(iv) + forge.util.decode64(data); + return { + iv: forge.util.decode64(iv), + data: forge.util.decode64(data), + encKey: forge.util.createBuffer(innerKey.encryptionKey).getBytes(), + macKey: forge.util.createBuffer(innerKey.authenticationKey).getBytes(), + mac: forge.util.decode64(mac!), + macData, + } as CbcDecryptParameters; + } else { + throw new Error("Unsupported encryption type."); } - - if (p.encKey == null) { - p.encKey = forge.util.decode64(key.encKeyB64); - } - p.data = forge.util.decode64(data); - p.iv = forge.util.decode64(iv); - p.macData = p.iv + p.data; - if (p.macKey == null && key.macKeyB64 != null) { - p.macKey = forge.util.decode64(key.macKeyB64); - } - if (mac != null) { - p.mac = forge.util.decode64(mac); - } - - // cache byte string keys for later - if (key.meta == null) { - key.meta = {}; - } - if (key.meta.encKeyByteString == null) { - key.meta.encKeyByteString = p.encKey; - } - if (p.macKey != null && key.meta.macKeyByteString == null) { - key.meta.macKeyByteString = p.macKey; - } - - return p; } aesDecryptFast({ diff --git a/libs/node/src/services/node-crypto-function.service.ts b/libs/node/src/services/node-crypto-function.service.ts index c06f18023b4..d080e750c8f 100644 --- a/libs/node/src/services/node-crypto-function.service.ts +++ b/libs/node/src/services/node-crypto-function.service.ts @@ -3,6 +3,7 @@ import * as crypto from "crypto"; import * as forge from "node-forge"; import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service"; +import { EncryptionType } from "@bitwarden/common/platform/enums"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { CbcDecryptParameters, @@ -172,24 +173,33 @@ export class NodeCryptoFunctionService implements CryptoFunctionService { mac: string | null, key: SymmetricCryptoKey, ): CbcDecryptParameters { - const p = {} as CbcDecryptParameters; - p.encKey = key.encKey; - p.data = Utils.fromB64ToArray(data); - p.iv = Utils.fromB64ToArray(iv); + const dataBytes = Utils.fromB64ToArray(data); + const ivBytes = Utils.fromB64ToArray(iv); + const macBytes = mac != null ? Utils.fromB64ToArray(mac) : null; - const macData = new Uint8Array(p.iv.byteLength + p.data.byteLength); - macData.set(new Uint8Array(p.iv), 0); - macData.set(new Uint8Array(p.data), p.iv.byteLength); - p.macData = macData; + const innerKey = key.inner(); - if (key.macKey != null) { - p.macKey = key.macKey; + if (innerKey.type === EncryptionType.AesCbc256_B64) { + return { + iv: ivBytes, + data: dataBytes, + encKey: innerKey.encryptionKey, + } as CbcDecryptParameters; + } else if (innerKey.type === EncryptionType.AesCbc256_HmacSha256_B64) { + const macData = new Uint8Array(ivBytes.length + dataBytes.length); + macData.set(ivBytes, 0); + macData.set(dataBytes, ivBytes.length); + return { + iv: ivBytes, + data: dataBytes, + mac: macBytes, + macData: macData, + encKey: innerKey.encryptionKey, + macKey: innerKey.authenticationKey, + } as CbcDecryptParameters; + } else { + throw new Error("Unsupported encryption type"); } - if (mac != null) { - p.mac = Utils.fromB64ToArray(mac); - } - - return p; } async aesDecryptFast({ From 111555b8d4792a3d2c0f02bd0144a6dfcc3190d8 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Fri, 28 Feb 2025 18:14:27 +0100 Subject: [PATCH 5/6] Fix desktop build --- .../key-management/biometrics/main-biometrics.service.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/desktop/src/key-management/biometrics/main-biometrics.service.spec.ts b/apps/desktop/src/key-management/biometrics/main-biometrics.service.spec.ts index 88dd8c60ed5..c22f24e6eb3 100644 --- a/apps/desktop/src/key-management/biometrics/main-biometrics.service.spec.ts +++ b/apps/desktop/src/key-management/biometrics/main-biometrics.service.spec.ts @@ -300,7 +300,7 @@ describe("MainBiometricsService", function () { expect(userKey).not.toBeNull(); expect(userKey!.keyB64).toBe(biometricKey); - expect(userKey!.encType).toBe(EncryptionType.AesCbc256_HmacSha256_B64); + expect(userKey!.inner().encryptionKey).toBe(EncryptionType.AesCbc256_HmacSha256_B64); expect(osBiometricsService.getBiometricKey).toHaveBeenCalledWith( "Bitwarden_biometric", `${userId}_user_biometric`, From 6b3b399f2e3af0d5904bde15fdf476d9124f58b9 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Fri, 28 Feb 2025 18:18:43 +0100 Subject: [PATCH 6/6] Fix test --- .../key-management/biometrics/main-biometrics.service.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/desktop/src/key-management/biometrics/main-biometrics.service.spec.ts b/apps/desktop/src/key-management/biometrics/main-biometrics.service.spec.ts index c22f24e6eb3..09a4dcef4b3 100644 --- a/apps/desktop/src/key-management/biometrics/main-biometrics.service.spec.ts +++ b/apps/desktop/src/key-management/biometrics/main-biometrics.service.spec.ts @@ -300,7 +300,7 @@ describe("MainBiometricsService", function () { expect(userKey).not.toBeNull(); expect(userKey!.keyB64).toBe(biometricKey); - expect(userKey!.inner().encryptionKey).toBe(EncryptionType.AesCbc256_HmacSha256_B64); + expect(userKey!.inner().type).toBe(EncryptionType.AesCbc256_HmacSha256_B64); expect(osBiometricsService.getBiometricKey).toHaveBeenCalledWith( "Bitwarden_biometric", `${userId}_user_biometric`,