From 5408a62b7df4631564601fb5fc056e718b6c74f9 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Mon, 12 May 2025 11:41:45 +0200 Subject: [PATCH] [PM-21001] Move KM usage of encrypt service (#14541) * Add new encrypt service functions * Undo changes * Cleanup * Fix build * Fix comments * Move KM usage of encrypt service * Fix build --- .../background/nativeMessaging.background.ts | 5 +-- .../biometric-message-handler.service.spec.ts | 8 ++-- .../biometric-message-handler.service.ts | 4 +- .../crypto/abstractions/encrypt.service.ts | 1 + .../device-trust.service.implementation.ts | 11 +++-- .../services/device-trust.service.spec.ts | 42 ++++++++++--------- .../services/master-password.service.ts | 16 ++----- libs/key-management/src/key.service.spec.ts | 20 +++++---- libs/key-management/src/key.service.ts | 5 +-- ...symmetric-key-regeneration.service.spec.ts | 3 ++ 10 files changed, 59 insertions(+), 56 deletions(-) diff --git a/apps/browser/src/background/nativeMessaging.background.ts b/apps/browser/src/background/nativeMessaging.background.ts index d92826765d..7172b98d72 100644 --- a/apps/browser/src/background/nativeMessaging.background.ts +++ b/apps/browser/src/background/nativeMessaging.background.ts @@ -357,7 +357,7 @@ export class NativeMessagingBackground { await this.secureCommunication(); } - return await this.encryptService.encrypt( + return await this.encryptService.encryptString( JSON.stringify(message), this.secureChannel!.sharedSecret!, ); @@ -401,10 +401,9 @@ export class NativeMessagingBackground { return; } message = JSON.parse( - await this.encryptService.decryptToUtf8( + await this.encryptService.decryptString( rawMessage as EncString, this.secureChannel.sharedSecret, - "ipc-desktop-ipc-channel-key", ), ); } else { diff --git a/apps/desktop/src/services/biometric-message-handler.service.spec.ts b/apps/desktop/src/services/biometric-message-handler.service.spec.ts index 9ddc3da8ed..af18828b59 100644 --- a/apps/desktop/src/services/biometric-message-handler.service.spec.ts +++ b/apps/desktop/src/services/biometric-message-handler.service.spec.ts @@ -347,7 +347,7 @@ describe("BiometricMessageHandlerService", () => { trusted: false, }), ); - encryptService.decryptToUtf8.mockResolvedValue( + encryptService.decryptString.mockResolvedValue( JSON.stringify({ command: "biometricUnlock", messageId: 0, @@ -382,7 +382,7 @@ describe("BiometricMessageHandlerService", () => { ngZone.run.mockReturnValue({ closed: of(true), }); - encryptService.decryptToUtf8.mockResolvedValue( + encryptService.decryptString.mockResolvedValue( JSON.stringify({ command: BiometricsCommands.UnlockWithBiometricsForUser, messageId: 0, @@ -433,7 +433,7 @@ describe("BiometricMessageHandlerService", () => { ngZone.run.mockReturnValue({ closed: of(false), }); - encryptService.decryptToUtf8.mockResolvedValue( + encryptService.decryptString.mockResolvedValue( JSON.stringify({ command: BiometricsCommands.UnlockWithBiometricsForUser, messageId: 0, @@ -480,7 +480,7 @@ describe("BiometricMessageHandlerService", () => { trusted: true, }), ); - encryptService.decryptToUtf8.mockResolvedValue( + encryptService.decryptString.mockResolvedValue( JSON.stringify({ command: BiometricsCommands.UnlockWithBiometricsForUser, messageId: 0, diff --git a/apps/desktop/src/services/biometric-message-handler.service.ts b/apps/desktop/src/services/biometric-message-handler.service.ts index 3851f40505..42d7b8aae5 100644 --- a/apps/desktop/src/services/biometric-message-handler.service.ts +++ b/apps/desktop/src/services/biometric-message-handler.service.ts @@ -175,7 +175,7 @@ export class BiometricMessageHandlerService { } const message: LegacyMessage = JSON.parse( - await this.encryptService.decryptToUtf8( + await this.encryptService.decryptString( rawMessage as EncString, SymmetricCryptoKey.fromString(sessionSecret), ), @@ -365,7 +365,7 @@ export class BiometricMessageHandlerService { throw new Error("Session secret is missing"); } - const encrypted = await this.encryptService.encrypt( + const encrypted = await this.encryptService.encryptString( JSON.stringify(message), SymmetricCryptoKey.fromString(sessionSecret), ); diff --git a/libs/common/src/key-management/crypto/abstractions/encrypt.service.ts b/libs/common/src/key-management/crypto/abstractions/encrypt.service.ts index 7a6c9bcd80..8bd58a21b6 100644 --- a/libs/common/src/key-management/crypto/abstractions/encrypt.service.ts +++ b/libs/common/src/key-management/crypto/abstractions/encrypt.service.ts @@ -49,6 +49,7 @@ export abstract class EncryptService { key: SymmetricCryptoKey, decryptTrace?: string, ): Promise; + /** * @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 diff --git a/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts b/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts index 06999cab9c..5e89e0a5cb 100644 --- a/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts +++ b/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts @@ -209,7 +209,7 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { devices.data .filter((device) => device.isTrusted) .map(async (device) => { - const publicKey = await this.encryptService.decryptToBytes( + const publicKey = await this.encryptService.unwrapEncapsulationKey( new EncString(device.encryptedPublicKey), oldUserKey, ); @@ -220,7 +220,10 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { return null; } - const newEncryptedPublicKey = await this.encryptService.encrypt(publicKey, newUserKey); + const newEncryptedPublicKey = await this.encryptService.wrapEncapsulationKey( + publicKey, + newUserKey, + ); const newEncryptedUserKey = await this.encryptService.encapsulateKeyUnsigned( newUserKey, publicKey, @@ -278,7 +281,7 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { const currentDeviceKeys = await this.devicesApiService.getDeviceKeys(deviceIdentifier); // Decrypt the existing device public key with the old user key - const decryptedDevicePublicKey = await this.encryptService.decryptToBytes( + const decryptedDevicePublicKey = await this.encryptService.unwrapEncapsulationKey( currentDeviceKeys.encryptedPublicKey, oldUserKey, ); @@ -394,7 +397,7 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { try { // attempt to decrypt encryptedDevicePrivateKey with device key - const devicePrivateKey = await this.encryptService.decryptToBytes( + const devicePrivateKey = await this.encryptService.unwrapDecapsulationKey( encryptedDevicePrivateKey, deviceKey, ); diff --git a/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts b/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts index 7d4a25c1f0..de9de5d781 100644 --- a/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts +++ b/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts @@ -623,9 +623,9 @@ describe("deviceTrustService", () => { }); it("successfully returns the user key when provided keys (including device key) can decrypt it", async () => { - const decryptToBytesSpy = jest - .spyOn(encryptService, "decryptToBytes") - .mockResolvedValue(new Uint8Array(userKeyBytesLength)); + const unwrapDecapsulationKeySpy = jest + .spyOn(encryptService, "unwrapDecapsulationKey") + .mockResolvedValue(new Uint8Array(2048)); const rsaDecryptSpy = jest .spyOn(encryptService, "decapsulateKeyUnsigned") .mockResolvedValue(new SymmetricCryptoKey(new Uint8Array(userKeyBytesLength))); @@ -638,13 +638,13 @@ describe("deviceTrustService", () => { ); expect(result).toEqual(mockUserKey); - expect(decryptToBytesSpy).toHaveBeenCalledTimes(1); + expect(unwrapDecapsulationKeySpy).toHaveBeenCalledTimes(1); expect(rsaDecryptSpy).toHaveBeenCalledTimes(1); }); it("returns null and removes device key when the decryption fails", async () => { - const decryptToBytesSpy = jest - .spyOn(encryptService, "decryptToBytes") + const unwrapDecapsulationKeySpy = jest + .spyOn(encryptService, "unwrapDecapsulationKey") .mockRejectedValue(new Error("Decryption error")); const setDeviceKeySpy = jest.spyOn(deviceTrustService as any, "setDeviceKey"); @@ -656,7 +656,7 @@ describe("deviceTrustService", () => { ); expect(result).toBeNull(); - expect(decryptToBytesSpy).toHaveBeenCalledTimes(1); + expect(unwrapDecapsulationKeySpy).toHaveBeenCalledTimes(1); expect(setDeviceKeySpy).toHaveBeenCalledTimes(1); expect(setDeviceKeySpy).toHaveBeenCalledWith(mockUserId, null); }); @@ -704,8 +704,8 @@ describe("deviceTrustService", () => { DeviceResponse, ), ); - encryptService.decryptToBytes.mockResolvedValue(null); - encryptService.encrypt.mockResolvedValue(new EncString("test_encrypted_data")); + encryptService.decryptBytes.mockResolvedValue(null); + encryptService.encryptString.mockResolvedValue(new EncString("test_encrypted_data")); encryptService.encapsulateKeyUnsigned.mockResolvedValue( new EncString("test_encrypted_data"), ); @@ -752,9 +752,11 @@ describe("deviceTrustService", () => { DeviceResponse, ), ); - encryptService.decryptToBytes.mockResolvedValue(new Uint8Array(64)); - encryptService.encrypt.mockResolvedValue(new EncString("test_encrypted_data")); - encryptService.rsaEncrypt.mockResolvedValue(new EncString("test_encrypted_data")); + encryptService.unwrapEncapsulationKey.mockResolvedValue(new Uint8Array(64)); + encryptService.wrapEncapsulationKey.mockResolvedValue(new EncString("test_encrypted_data")); + encryptService.encapsulateKeyUnsigned.mockResolvedValue( + new EncString("test_encrypted_data"), + ); const protectedDeviceResponse = new ProtectedDeviceResponse({ id: "", @@ -862,13 +864,15 @@ describe("deviceTrustService", () => { }); // Mock the decryption of the public key with the old user key - encryptService.decryptToBytes.mockImplementationOnce((_encValue, privateKeyValue) => { - expect(privateKeyValue.inner().type).toBe(EncryptionType.AesCbc256_HmacSha256_B64); - expect(new Uint8Array(privateKeyValue.toEncoded())[0]).toBe(FakeOldUserKeyMarker); - const data = new Uint8Array(250); - data.fill(FakeDecryptedPublicKeyMarker, 0, 1); - return Promise.resolve(data); - }); + encryptService.unwrapEncapsulationKey.mockImplementationOnce( + (_encValue, privateKeyValue) => { + expect(privateKeyValue.inner().type).toBe(EncryptionType.AesCbc256_HmacSha256_B64); + expect(new Uint8Array(privateKeyValue.toEncoded())[0]).toBe(FakeOldUserKeyMarker); + const data = new Uint8Array(250); + data.fill(FakeDecryptedPublicKeyMarker, 0, 1); + return Promise.resolve(data); + }, + ); // Mock the encryption of the new user key with the decrypted public key encryptService.encapsulateKeyUnsigned.mockImplementationOnce((data, publicKey) => { diff --git a/libs/common/src/key-management/master-password/services/master-password.service.ts b/libs/common/src/key-management/master-password/services/master-password.service.ts index 72b18d8bfb..b9b11d6cbe 100644 --- a/libs/common/src/key-management/master-password/services/master-password.service.ts +++ b/libs/common/src/key-management/master-password/services/master-password.service.ts @@ -174,21 +174,13 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr throw new Error("No master key found."); } - let decUserKey: Uint8Array; + let decUserKey: SymmetricCryptoKey; if (userKey.encryptionType === EncryptionType.AesCbc256_B64) { - decUserKey = await this.encryptService.decryptToBytes( - userKey, - masterKey, - "Content: User Key; Encrypting Key: Master Key", - ); + decUserKey = await this.encryptService.unwrapSymmetricKey(userKey, masterKey); } else if (userKey.encryptionType === EncryptionType.AesCbc256_HmacSha256_B64) { const newKey = await this.keyGenerationService.stretchKey(masterKey); - decUserKey = await this.encryptService.decryptToBytes( - userKey, - newKey, - "Content: User Key; Encrypting Key: Stretched Master Key", - ); + decUserKey = await this.encryptService.unwrapSymmetricKey(userKey, newKey); } else { throw new Error("Unsupported encryption type."); } @@ -198,6 +190,6 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr return null; } - return new SymmetricCryptoKey(decUserKey) as UserKey; + return decUserKey as UserKey; } } diff --git a/libs/key-management/src/key.service.spec.ts b/libs/key-management/src/key.service.spec.ts index 5729146292..25d8aff99f 100644 --- a/libs/key-management/src/key.service.spec.ts +++ b/libs/key-management/src/key.service.spec.ts @@ -330,7 +330,7 @@ describe("keyService", () => { everHadUserKeyState.nextState(null); // Mock private key decryption - encryptService.decryptToBytes.mockResolvedValue(mockRandomBytes); + encryptService.unwrapDecapsulationKey.mockResolvedValue(mockRandomBytes); }); it("throws if userKey is null", async () => { @@ -352,7 +352,7 @@ describe("keyService", () => { }); it("throws if encPrivateKey cannot be decrypted with the userKey", async () => { - encryptService.decryptToBytes.mockResolvedValue(null); + encryptService.unwrapDecapsulationKey.mockResolvedValue(null); await expect( keyService.setUserKeys(mockUserKey, mockEncPrivateKey, mockUserId), @@ -452,17 +452,16 @@ describe("keyService", () => { // Decryption of the user private key const fakeDecryptedUserPrivateKey = makeStaticByteArray(10, 1); - encryptService.decryptToBytes.mockResolvedValue(fakeDecryptedUserPrivateKey); + encryptService.unwrapDecapsulationKey.mockResolvedValue(fakeDecryptedUserPrivateKey); const fakeUserPublicKey = makeStaticByteArray(10, 2); cryptoFunctionService.rsaExtractPublicKey.mockResolvedValue(fakeUserPublicKey); const userPrivateKey = await firstValueFrom(keyService.userPrivateKey$(mockUserId)); - expect(encryptService.decryptToBytes).toHaveBeenCalledWith( + expect(encryptService.unwrapDecapsulationKey).toHaveBeenCalledWith( fakeEncryptedUserPrivateKey, userKey, - "Content: Encrypted Private Key", ); expect(userPrivateKey).toBe(fakeDecryptedUserPrivateKey); @@ -473,7 +472,7 @@ describe("keyService", () => { const userPrivateKey = await firstValueFrom(keyService.userPrivateKey$(mockUserId)); - expect(encryptService.decryptToBytes).not.toHaveBeenCalled(); + expect(encryptService.unwrapDecapsulationKey).not.toHaveBeenCalled(); expect(userPrivateKey).toBeFalsy(); }); @@ -552,10 +551,12 @@ describe("keyService", () => { providerKeysState.nextState(keys.providerKeys!); } - encryptService.decryptToBytes.mockImplementation((encryptedPrivateKey, userKey) => { - // TOOD: Branch between provider and private key? + encryptService.unwrapDecapsulationKey.mockImplementation((encryptedPrivateKey, userKey) => { return Promise.resolve(fakePrivateKeyDecryption(encryptedPrivateKey, userKey)); }); + encryptService.unwrapSymmetricKey.mockImplementation((encryptedPrivateKey, userKey) => { + return Promise.resolve(new SymmetricCryptoKey(new Uint8Array(64))); + }); encryptService.decapsulateKeyUnsigned.mockImplementation((data, privateKey) => { return Promise.resolve(new SymmetricCryptoKey(fakeOrgKeyDecryption(data, privateKey))); @@ -617,6 +618,7 @@ describe("keyService", () => { }); it("returns decryption keys when some of the org keys are providers", async () => { + encryptService.decryptToBytes.mockResolvedValue(new Uint8Array(64)); const org2Id = "org2Id" as OrganizationId; updateKeys({ userKey: makeSymmetricCryptoKey(64), @@ -647,7 +649,7 @@ describe("keyService", () => { const org2Key = decryptionKeys!.orgKeys![org2Id]; expect(org2Key).not.toBeNull(); - expect(org2Key.keyB64).toContain("provider1Key"); + expect(org2Key.toEncoded()).toHaveLength(64); }); it("returns a stream that pays attention to updates of all data", async () => { diff --git a/libs/key-management/src/key.service.ts b/libs/key-management/src/key.service.ts index 01e7b713e1..849b9db6a5 100644 --- a/libs/key-management/src/key.service.ts +++ b/libs/key-management/src/key.service.ts @@ -738,7 +738,7 @@ export class DefaultKeyService implements KeyServiceAbstraction { const storePin = await this.shouldStoreKey(KeySuffixOptions.Pin, userId); if (storePin) { // Decrypt userKeyEncryptedPin with user key - const pin = await this.encryptService.decryptToUtf8( + const pin = await this.encryptService.decryptString( (await this.pinService.getUserKeyEncryptedPin(userId))!, key, ); @@ -945,10 +945,9 @@ export class DefaultKeyService implements KeyServiceAbstraction { return null; } - return (await this.encryptService.decryptToBytes( + return (await this.encryptService.unwrapDecapsulationKey( new EncString(encryptedPrivateKey), key, - "Content: Encrypted Private Key", )) as UserPrivateKey; } diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts index 8e2ce8ab6b..35cef91458 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts @@ -58,6 +58,9 @@ function setupUserKeyValidation( cipher.notes = mockEnc("EncryptedString"); cipher.key = mockEnc("EncKey"); cipherService.getAll.mockResolvedValue([cipher]); + encryptService.unwrapSymmetricKey.mockResolvedValue( + new SymmetricCryptoKey(makeStaticByteArray(64)), + ); encryptService.decryptToBytes.mockResolvedValue(makeStaticByteArray(64)); encryptService.decryptString.mockResolvedValue("mockDecryptedString"); (window as any).bitwardenContainerService = new ContainerService(keyService, encryptService);