bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2025-12-15 15:53:27 +00:00
Code Issues Releases Wiki Activity

[PM-17984] Remove AES128CBC-HMAC encryption (#13304)

Browse Source 
* Remove AES128CBC-HMAC encryption

* Increase test coverage
This commit is contained in:
Bernd Schoolmann
2025-03-11 14:20:02 +01:00
committed by GitHub
parent 5cd47ac907
commit 9683779dbf
10 changed files with 50 additions and 105 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
libs/common/src/key-management/crypto/abstractions/encrypt.service.ts
View File

@@ -36,7 +36,6 @@ export abstract class EncryptService {
): Promise<Uint8Array | null>;
abstract rsaEncrypt(data: Uint8Array, publicKey: Uint8Array): Promise<EncString>;
abstract rsaDecrypt(data: EncString, privateKey: Uint8Array): Promise<Uint8Array>;
abstract resolveLegacyKey(key: SymmetricCryptoKey, encThing: Encrypted): SymmetricCryptoKey;
/**
* @deprecated Replaced by BulkEncryptService, remove once the feature is tested and the featureflag PM-4154-multi-worker-encryption-service is removed
* @param items The items to decrypt

19
libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts
View File

@@ -78,8 +78,6 @@ export class EncryptServiceImplementation implements EncryptService {
throw new Error("No key provided for decryption.");
}
key = this.resolveLegacyKey(key, encString);
// 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(
@@ -145,8 +143,6 @@ export class EncryptServiceImplementation implements EncryptService {
throw new Error("Nothing provided for decryption.");
}
key = this.resolveLegacyKey(key, encThing);
// 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(
@@ -298,19 +294,4 @@ export class EncryptServiceImplementation implements EncryptService {
this.logService.error(msg);
}
}
/**
* Transform into new key for the old encrypt-then-mac scheme if required, otherwise return the current key unchanged
* @param encThing The encrypted object (e.g. encString or encArrayBuffer) that you want to decrypt
*/
resolveLegacyKey(key: SymmetricCryptoKey, encThing: Encrypted): SymmetricCryptoKey {
if (
encThing.encryptionType === EncryptionType.AesCbc128_HmacSha256_B64 &&
key.encType === EncryptionType.AesCbc256_B64
) {
return new SymmetricCryptoKey(key.key, EncryptionType.AesCbc128_HmacSha256_B64);
}
return key;
}
}

62
libs/common/src/key-management/crypto/services/encrypt.service.spec.ts
View File

@@ -325,6 +325,25 @@ describe("EncryptService", () => {
});
});
describe("decryptToUtf8", () => {
it("throws if no key is provided", () => {
return expect(encryptService.decryptToUtf8(null, null)).rejects.toThrow(
"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,
);
const encString = new EncString(EncryptionType.AesCbc256_B64, "data");
const actual = await encryptService.decryptToUtf8(encString, key);
expect(actual).toBeNull();
expect(logService.error).toHaveBeenCalled();
});
});
describe("rsa", () => {
const data = makeStaticByteArray(10, 100);
const encryptedData = makeStaticByteArray(10, 150);
@@ -370,17 +389,16 @@ describe("EncryptService", () => {
return expect(encryptService.rsaDecrypt(encString, null)).rejects.toThrow("No private key");
});
it.each([
EncryptionType.AesCbc256_B64,
EncryptionType.AesCbc128_HmacSha256_B64,
EncryptionType.AesCbc256_HmacSha256_B64,
])("throws if encryption type is %s", async (encType) => {
encString.encryptionType = encType;
it.each([EncryptionType.AesCbc256_B64, EncryptionType.AesCbc256_HmacSha256_B64])(
"throws if encryption type is %s",
async (encType) => {
encString.encryptionType = encType;
await expect(encryptService.rsaDecrypt(encString, privateKey)).rejects.toThrow(
"Invalid encryption type",
);
});
await expect(encryptService.rsaDecrypt(encString, privateKey)).rejects.toThrow(
"Invalid encryption type",
);
},
);
it("decrypts data with provided key", async () => {
cryptoFunctionService.rsaDecrypt.mockResolvedValue(data);
@@ -398,30 +416,6 @@ describe("EncryptService", () => {
});
});
describe("resolveLegacyKey", () => {
it("creates a legacy key if required", async () => {
const key = new SymmetricCryptoKey(makeStaticByteArray(32), EncryptionType.AesCbc256_B64);
const encString = mock<EncString>();
encString.encryptionType = EncryptionType.AesCbc128_HmacSha256_B64;
const actual = encryptService.resolveLegacyKey(key, encString);
const expected = new SymmetricCryptoKey(key.key, EncryptionType.AesCbc128_HmacSha256_B64);
expect(actual).toEqual(expected);
});
it("does not create a legacy key if not required", async () => {
const encType = EncryptionType.AesCbc256_HmacSha256_B64;
const key = new SymmetricCryptoKey(makeStaticByteArray(64), encType);
const encString = mock<EncString>();
encString.encryptionType = encType;
const actual = encryptService.resolveLegacyKey(key, encString);
expect(actual).toEqual(key);
});
});
describe("hash", () => {
it("hashes a string and returns b64", async () => {
cryptoFunctionService.hash.mockResolvedValue(Uint8Array.from([1, 2, 3]));

7
libs/common/src/platform/enums/encryption-type.enum.ts
View File

@@ -1,6 +1,6 @@
export enum EncryptionType {
AesCbc256_B64 = 0,
AesCbc128_HmacSha256_B64 = 1,
// Type 1 was the unused and removed AesCbc128_HmacSha256_B64
AesCbc256_HmacSha256_B64 = 2,
Rsa2048_OaepSha256_B64 = 3,
Rsa2048_OaepSha1_B64 = 4,
@@ -17,12 +17,10 @@ export function encryptionTypeToString(encryptionType: EncryptionType): string {
}
/** The expected number of parts to a serialized EncString of the given encryption type.
* For example, an EncString of type AesCbc256_B64 will have 2 parts, and an EncString of type
* AesCbc128_HmacSha256_B64 will have 3 parts.
* For example, an EncString of type AesCbc256_B64 will have 2 parts
*
* Example of annotated serialized EncStrings:
* 0.iv|data
* 1.iv|data|mac
* 2.iv|data|mac
* 3.data
* 4.data
@@ -33,7 +31,6 @@ export function encryptionTypeToString(encryptionType: EncryptionType): string {
*/
export const EXPECTED_NUM_PARTS_BY_ENCRYPTION_TYPE = {
[EncryptionType.AesCbc256_B64]: 2,
[EncryptionType.AesCbc128_HmacSha256_B64]: 3,
[EncryptionType.AesCbc256_HmacSha256_B64]: 3,
[EncryptionType.Rsa2048_OaepSha256_B64]: 1,
[EncryptionType.Rsa2048_OaepSha1_B64]: 1,

39
libs/common/src/platform/models/domain/enc-array-buffer.spec.ts
View File

@@ -5,28 +5,28 @@ import { EncArrayBuffer } from "./enc-array-buffer";
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);
test.each([[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 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);
const actual = new EncArrayBuffer(array);
expect(actual.encryptionType).toEqual(encType);
expect(actual.ivBytes).toEqualBuffer(iv);
expect(actual.macBytes).toEqualBuffer(mac);
expect(actual.dataBytes).toEqualBuffer(data);
});
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;
@@ -50,7 +50,6 @@ describe("encArrayBuffer", () => {
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) => {

1
libs/common/src/platform/models/domain/enc-array-buffer.ts
View File

@@ -20,7 +20,6 @@ export class EncArrayBuffer implements Encrypted {
const encType = encBytes[0];
switch (encType) {
case EncryptionType.AesCbc128_HmacSha256_B64:
case EncryptionType.AesCbc256_HmacSha256_B64: {
const minimumLength = ENC_TYPE_LENGTH + IV_LENGTH + MAC_LENGTH + MIN_DATA_LENGTH;
if (encBytes.length < minimumLength) {

2
libs/common/src/platform/models/domain/enc-string.spec.ts
View File

@@ -60,9 +60,7 @@ describe("EncString", () => {
const cases = [
"aXY=|Y3Q=", // AesCbc256_B64 w/out header
"aXY=|Y3Q=|cnNhQ3Q=", // AesCbc128_HmacSha256_B64 w/out header
"0.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==", // AesCbc256_B64 with header
"1.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==|QmFzZTY0UGFydA==", // AesCbc128_HmacSha256_B64
"2.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==|QmFzZTY0UGFydA==", // AesCbc256_HmacSha256_B64
"3.QmFzZTY0UGFydA==", // Rsa2048_OaepSha256_B64
"4.QmFzZTY0UGFydA==", // Rsa2048_OaepSha1_B64

6
libs/common/src/platform/models/domain/enc-string.ts
View File

@@ -89,7 +89,6 @@ export class EncString implements Encrypted {
}
switch (encType) {
case EncryptionType.AesCbc128_HmacSha256_B64:
case EncryptionType.AesCbc256_HmacSha256_B64:
this.iv = encPieces[0];
this.data = encPieces[1];
@@ -132,10 +131,7 @@ export class EncString implements Encrypted {
}
} else {
encPieces = encryptedString.split("|");
encType =
encPieces.length === 3
? EncryptionType.AesCbc128_HmacSha256_B64
: EncryptionType.AesCbc256_B64;
encType = EncryptionType.AesCbc256_B64;
}
return {

15
libs/common/src/platform/models/domain/symmetric-crypto-key.spec.ts
View File

@@ -27,21 +27,6 @@ describe("SymmetricCryptoKey", () => {
});
});
it("AesCbc128_HmacSha256_B64", () => {
const key = makeStaticByteArray(32);
const cryptoKey = new SymmetricCryptoKey(key, EncryptionType.AesCbc128_HmacSha256_B64);
expect(cryptoKey).toEqual({
encKey: key.slice(0, 16),
encKeyB64: "AAECAwQFBgcICQoLDA0ODw==",
encType: 1,
key: key,
keyB64: "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=",
macKey: key.slice(16, 32),
macKeyB64: "EBESExQVFhcYGRobHB0eHw==",
});
});
it("AesCbc256_HmacSha256_B64", () => {
const key = makeStaticByteArray(64);
const cryptoKey = new SymmetricCryptoKey(key);

3
libs/common/src/platform/models/domain/symmetric-crypto-key.ts
View File

@@ -38,9 +38,6 @@ export class SymmetricCryptoKey {
if (encType === EncryptionType.AesCbc256_B64 && key.byteLength === 32) {
this.encKey = key;
this.macKey = null;
} else if (encType === EncryptionType.AesCbc128_HmacSha256_B64 && key.byteLength === 32) {
this.encKey = key.slice(0, 16);
this.macKey = key.slice(16, 32);
} else if (encType === EncryptionType.AesCbc256_HmacSha256_B64 && key.byteLength === 64) {
this.encKey = key.slice(0, 32);
this.macKey = key.slice(32, 64);