From d93f547cfb8dae60d72692a964490ddf199ff7f1 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Tue, 20 May 2025 19:45:40 +0200 Subject: [PATCH] [PM-21001] Move platform code to new encrypt service interface (#14544) * Move platform code to new encrypt service interface * Fix tests * Fix tests * Fix cli build --- ...cal-backed-session-storage.service.spec.ts | 31 ++++++-------- .../local-backed-session-storage.service.ts | 11 +++-- apps/cli/src/commands/download.command.ts | 2 +- apps/cli/src/commands/edit.command.ts | 2 +- apps/cli/src/commands/get.command.ts | 3 +- .../node-env-secure-storage.service.ts | 4 +- .../platform/services/electron-key.service.ts | 2 +- .../default-collection.service.spec.ts | 3 -- .../default-vnext-collection.service.spec.ts | 11 +---- .../models/domain/domain-base.spec.ts | 26 ++++-------- .../platform/models/domain/enc-string.spec.ts | 40 ++++++------------- .../src/platform/models/domain/enc-string.ts | 19 +-------- 12 files changed, 47 insertions(+), 107 deletions(-) diff --git a/apps/browser/src/platform/services/local-backed-session-storage.service.spec.ts b/apps/browser/src/platform/services/local-backed-session-storage.service.spec.ts index 1b93e33a94..1b4665b322 100644 --- a/apps/browser/src/platform/services/local-backed-session-storage.service.spec.ts +++ b/apps/browser/src/platform/services/local-backed-session-storage.service.spec.ts @@ -46,22 +46,18 @@ describe("LocalBackedSessionStorage", () => { it("returns a decrypted value when one is stored in local storage", async () => { const encrypted = makeEncString("encrypted"); localStorage.internalStore["session_test"] = encrypted.encryptedString; - encryptService.decryptToUtf8.mockResolvedValue(JSON.stringify("decrypted")); + encryptService.decryptString.mockResolvedValue(JSON.stringify("decrypted")); const result = await sut.get("test"); // FIXME: Remove when updating file. Eslint update // eslint-disable-next-line @typescript-eslint/no-unused-expressions - expect(encryptService.decryptToUtf8).toHaveBeenCalledWith( - encrypted, - sessionKey, - "browser-session-key", - ), + expect(encryptService.decryptString).toHaveBeenCalledWith(encrypted, sessionKey), expect(result).toEqual("decrypted"); }); it("caches the decrypted value when one is stored in local storage", async () => { const encrypted = makeEncString("encrypted"); localStorage.internalStore["session_test"] = encrypted.encryptedString; - encryptService.decryptToUtf8.mockResolvedValue(JSON.stringify("decrypted")); + encryptService.decryptString.mockResolvedValue(JSON.stringify("decrypted")); await sut.get("test"); expect(sut["cache"]["test"]).toEqual("decrypted"); }); @@ -69,22 +65,18 @@ describe("LocalBackedSessionStorage", () => { it("returns a decrypted value when one is stored in local storage", async () => { const encrypted = makeEncString("encrypted"); localStorage.internalStore["session_test"] = encrypted.encryptedString; - encryptService.decryptToUtf8.mockResolvedValue(JSON.stringify("decrypted")); + encryptService.decryptString.mockResolvedValue(JSON.stringify("decrypted")); const result = await sut.get("test"); // FIXME: Remove when updating file. Eslint update // eslint-disable-next-line @typescript-eslint/no-unused-expressions - expect(encryptService.decryptToUtf8).toHaveBeenCalledWith( - encrypted, - sessionKey, - "browser-session-key", - ), + expect(encryptService.decryptString).toHaveBeenCalledWith(encrypted, sessionKey), expect(result).toEqual("decrypted"); }); it("caches the decrypted value when one is stored in local storage", async () => { const encrypted = makeEncString("encrypted"); localStorage.internalStore["session_test"] = encrypted.encryptedString; - encryptService.decryptToUtf8.mockResolvedValue(JSON.stringify("decrypted")); + encryptService.decryptString.mockResolvedValue(JSON.stringify("decrypted")); await sut.get("test"); expect(sut["cache"]["test"]).toEqual("decrypted"); }); @@ -104,7 +96,7 @@ describe("LocalBackedSessionStorage", () => { it("returns true when the key is in local storage", async () => { localStorage.internalStore["session_test"] = makeEncString("encrypted").encryptedString; - encryptService.decryptToUtf8.mockResolvedValue(JSON.stringify("decrypted")); + encryptService.decryptString.mockResolvedValue(JSON.stringify("decrypted")); const result = await sut.has("test"); expect(result).toBe(true); }); @@ -119,7 +111,7 @@ describe("LocalBackedSessionStorage", () => { async (nullish) => { localStorage.internalStore["session_test"] = nullish; await expect(sut.has("test")).resolves.toBe(false); - expect(encryptService.decryptToUtf8).not.toHaveBeenCalled(); + expect(encryptService.decryptString).not.toHaveBeenCalled(); }, ); }); @@ -127,7 +119,7 @@ describe("LocalBackedSessionStorage", () => { describe("save", () => { const encString = makeEncString("encrypted"); beforeEach(() => { - encryptService.encrypt.mockResolvedValue(encString); + encryptService.encryptString.mockResolvedValue(encString); }); it("logs a warning when saving the same value twice and in a dev environment", async () => { @@ -157,7 +149,10 @@ describe("LocalBackedSessionStorage", () => { it("encrypts and saves the value to local storage", async () => { await sut.save("test", "value"); - expect(encryptService.encrypt).toHaveBeenCalledWith(JSON.stringify("value"), sessionKey); + expect(encryptService.encryptString).toHaveBeenCalledWith( + JSON.stringify("value"), + sessionKey, + ); expect(localStorage.internalStore["session_test"]).toEqual(encString.encryptedString); }); diff --git a/apps/browser/src/platform/services/local-backed-session-storage.service.ts b/apps/browser/src/platform/services/local-backed-session-storage.service.ts index 0e6922e308..1507bf20c4 100644 --- a/apps/browser/src/platform/services/local-backed-session-storage.service.ts +++ b/apps/browser/src/platform/services/local-backed-session-storage.service.ts @@ -118,11 +118,7 @@ export class LocalBackedSessionStorageService return null; } - const valueJson = await this.encryptService.decryptToUtf8( - new EncString(local), - encKey, - "browser-session-key", - ); + const valueJson = await this.encryptService.decryptString(new EncString(local), encKey); if (valueJson == null) { // error with decryption, value is lost, delete state and start over await this.localStorage.remove(this.sessionStorageKey(key)); @@ -139,7 +135,10 @@ export class LocalBackedSessionStorageService } const valueJson = JSON.stringify(value); - const encValue = await this.encryptService.encrypt(valueJson, await this.sessionKey.get()); + const encValue = await this.encryptService.encryptString( + valueJson, + await this.sessionKey.get(), + ); await this.localStorage.save(this.sessionStorageKey(key), encValue.encryptedString); } diff --git a/apps/cli/src/commands/download.command.ts b/apps/cli/src/commands/download.command.ts index 01ef675d2a..472b084f5d 100644 --- a/apps/cli/src/commands/download.command.ts +++ b/apps/cli/src/commands/download.command.ts @@ -47,7 +47,7 @@ export abstract class DownloadCommand { try { const encBuf = await EncArrayBuffer.fromResponse(response); - const decBuf = await this.encryptService.decryptToBytes(encBuf, key); + const decBuf = await this.encryptService.decryptFileData(encBuf, key); if (process.env.BW_SERVE === "true") { const res = new FileResponse(Buffer.from(decBuf), fileName); return Response.success(res); diff --git a/apps/cli/src/commands/edit.command.ts b/apps/cli/src/commands/edit.command.ts index 4dcf805661..677139d545 100644 --- a/apps/cli/src/commands/edit.command.ts +++ b/apps/cli/src/commands/edit.command.ts @@ -195,7 +195,7 @@ export class EditCommand { (u) => new SelectionReadOnlyRequest(u.id, u.readOnly, u.hidePasswords, u.manage), ); const request = new CollectionRequest(); - request.name = (await this.encryptService.encrypt(req.name, orgKey)).encryptedString; + request.name = (await this.encryptService.encryptString(req.name, orgKey)).encryptedString; request.externalId = req.externalId; request.groups = groups; request.users = users; diff --git a/apps/cli/src/commands/get.command.ts b/apps/cli/src/commands/get.command.ts index c3ba6044f8..60be0a8d2c 100644 --- a/apps/cli/src/commands/get.command.ts +++ b/apps/cli/src/commands/get.command.ts @@ -453,10 +453,9 @@ export class GetCommand extends DownloadCommand { const response = await this.apiService.getCollectionAccessDetails(options.organizationId, id); const decCollection = new CollectionView(response); - decCollection.name = await this.encryptService.decryptToUtf8( + decCollection.name = await this.encryptService.decryptString( new EncString(response.name), orgKey, - `orgkey-${options.organizationId}`, ); const groups = response.groups == null diff --git a/apps/cli/src/platform/services/node-env-secure-storage.service.ts b/apps/cli/src/platform/services/node-env-secure-storage.service.ts index 5e31995606..6486534000 100644 --- a/apps/cli/src/platform/services/node-env-secure-storage.service.ts +++ b/apps/cli/src/platform/services/node-env-secure-storage.service.ts @@ -61,7 +61,7 @@ export class NodeEnvSecureStorageService implements AbstractStorageService { if (sessionKey == null) { throw new Error("No session key available."); } - const encValue = await this.encryptService.encryptToBytes( + const encValue = await this.encryptService.encryptFileData( Utils.fromB64ToArray(plainValue), sessionKey, ); @@ -80,7 +80,7 @@ export class NodeEnvSecureStorageService implements AbstractStorageService { } const encBuf = EncArrayBuffer.fromB64(encValue); - const decValue = await this.encryptService.decryptToBytes(encBuf, sessionKey); + const decValue = await this.encryptService.decryptFileData(encBuf, sessionKey); if (decValue == null) { this.logService.info("Failed to decrypt."); return null; diff --git a/apps/desktop/src/platform/services/electron-key.service.ts b/apps/desktop/src/platform/services/electron-key.service.ts index d272a9a9bd..5ecde57ec5 100644 --- a/apps/desktop/src/platform/services/electron-key.service.ts +++ b/apps/desktop/src/platform/services/electron-key.service.ts @@ -110,7 +110,7 @@ export class ElectronKeyService extends DefaultKeyService { // Set a key half if it doesn't exist const keyBytes = await this.cryptoFunctionService.randomBytes(32); clientKeyHalf = Utils.fromBufferToUtf8(keyBytes) as CsprngString; - const encKey = await this.encryptService.encrypt(clientKeyHalf, userKey); + const encKey = await this.encryptService.encryptString(clientKeyHalf, userKey); await this.biometricStateService.setEncryptedClientKeyHalf(encKey, userId); } diff --git a/libs/admin-console/src/common/collections/services/default-collection.service.spec.ts b/libs/admin-console/src/common/collections/services/default-collection.service.spec.ts index c5f57f38dd..11a114dc04 100644 --- a/libs/admin-console/src/common/collections/services/default-collection.service.spec.ts +++ b/libs/admin-console/src/common/collections/services/default-collection.service.spec.ts @@ -123,9 +123,6 @@ const mockCryptoService = () => { encryptService.decryptString .calledWith(expect.any(EncString), expect.anything()) .mockResolvedValue("DECRYPTED_STRING"); - encryptService.decryptToUtf8 - .calledWith(expect.any(EncString), expect.anything(), expect.anything()) - .mockResolvedValue("DECRYPTED_STRING"); (window as any).bitwardenContainerService = new ContainerService(keyService, encryptService); diff --git a/libs/admin-console/src/common/collections/services/default-vnext-collection.service.spec.ts b/libs/admin-console/src/common/collections/services/default-vnext-collection.service.spec.ts index d4bc026b5b..03921f71ee 100644 --- a/libs/admin-console/src/common/collections/services/default-vnext-collection.service.spec.ts +++ b/libs/admin-console/src/common/collections/services/default-vnext-collection.service.spec.ts @@ -51,11 +51,6 @@ describe("DefaultvNextCollectionService", () => { .mockImplementation((encString, key) => Promise.resolve(encString.data.replace("ENC_", "DEC_")), ); - encryptService.decryptToUtf8 - .calledWith(expect.any(EncString), expect.any(SymmetricCryptoKey), expect.any(String)) - .mockImplementation((encString, key) => - Promise.resolve(encString.data.replace("ENC_", "DEC_")), - ); (window as any).bitwardenContainerService = new ContainerService(keyService, encryptService); @@ -109,15 +104,13 @@ describe("DefaultvNextCollectionService", () => { // Assert that the correct org keys were used for each encrypted string // This should be replaced with decryptString when the platform PR (https://github.com/bitwarden/clients/pull/14544) is merged - expect(encryptService.decryptToUtf8).toHaveBeenCalledWith( + expect(encryptService.decryptString).toHaveBeenCalledWith( expect.objectContaining(new EncString(collection1.name)), orgKey1, - expect.any(String), ); - expect(encryptService.decryptToUtf8).toHaveBeenCalledWith( + expect(encryptService.decryptString).toHaveBeenCalledWith( expect.objectContaining(new EncString(collection2.name)), orgKey2, - expect.any(String), ); }); diff --git a/libs/common/src/platform/models/domain/domain-base.spec.ts b/libs/common/src/platform/models/domain/domain-base.spec.ts index 0c13f9a211..5f68bd8702 100644 --- a/libs/common/src/platform/models/domain/domain-base.spec.ts +++ b/libs/common/src/platform/models/domain/domain-base.spec.ts @@ -2,7 +2,6 @@ import { mock, MockProxy } from "jest-mock-extended"; import { makeEncString, makeSymmetricCryptoKey } from "../../../../spec"; import { EncryptService } from "../../../key-management/crypto/abstractions/encrypt.service"; -import { Utils } from "../../misc/utils"; import Domain from "./domain-base"; import { EncString } from "./enc-string"; @@ -22,24 +21,13 @@ describe("DomainBase", () => { }); function setUpCryptography() { - encryptService.encrypt.mockImplementation((value) => { - let data: string; - if (typeof value === "string") { - data = value; - } else { - data = Utils.fromBufferToUtf8(value); - } + encryptService.encryptString.mockImplementation((value) => + Promise.resolve(makeEncString(value)), + ); - return Promise.resolve(makeEncString(data)); - }); - - encryptService.decryptToUtf8.mockImplementation((value) => { + encryptService.decryptString.mockImplementation((value) => { return Promise.resolve(value.data); }); - - encryptService.decryptToBytes.mockImplementation((value) => { - return Promise.resolve(value.dataBytes); - }); } describe("decryptWithKey", () => { @@ -82,7 +70,7 @@ describe("DomainBase", () => { const domain = new TestDomain(); - domain.encToString = await encryptService.encrypt("string", key); + domain.encToString = await encryptService.encryptString("string", key); const decrypted = await domain["decryptObjWithKey"](["encToString"], key, encryptService); @@ -96,8 +84,8 @@ describe("DomainBase", () => { const domain = new TestDomain(); - domain.encToString = await encryptService.encrypt("string", key); - domain.encString2 = await encryptService.encrypt("string2", key); + domain.encToString = await encryptService.encryptString("string", key); + domain.encString2 = await encryptService.encryptString("string2", key); const decrypted = await domain["decryptObjWithKey"]( ["encToString", "encString2"], diff --git a/libs/common/src/platform/models/domain/enc-string.spec.ts b/libs/common/src/platform/models/domain/enc-string.spec.ts index c3f257d442..1ab61745eb 100644 --- a/libs/common/src/platform/models/domain/enc-string.spec.ts +++ b/libs/common/src/platform/models/domain/enc-string.spec.ts @@ -7,7 +7,6 @@ import { EncryptService } from "../../../key-management/crypto/abstractions/encr import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; import { UserKey, OrgKey } from "../../../types/key"; import { EncryptionType } from "../../enums"; -import { Utils } from "../../misc/utils"; import { ContainerService } from "../../services/container.service"; import { EncString } from "./enc-string"; @@ -87,7 +86,7 @@ describe("EncString", () => { ); const encryptService = mock(); - encryptService.decryptToUtf8 + encryptService.decryptString .calledWith(encString, expect.anything()) .mockResolvedValue("decrypted"); @@ -106,7 +105,7 @@ describe("EncString", () => { it("result should be cached", async () => { const decrypted = await encString.decrypt(null); - expect(encryptService.decryptToUtf8).toBeCalledTimes(1); + expect(encryptService.decryptString).toBeCalledTimes(1); expect(decrypted).toBe("decrypted"); }); @@ -118,24 +117,17 @@ describe("EncString", () => { const keyService = mock(); const encryptService = mock(); - encryptService.decryptToUtf8 + encryptService.decryptString .calledWith(encString, expect.anything()) .mockResolvedValue("decrypted"); function setupEncryption() { - encryptService.encrypt.mockImplementation(async (data, key) => { - if (typeof data === "string") { - return makeEncString(data); - } else { - return makeEncString(Utils.fromBufferToUtf8(data)); - } + encryptService.encryptString.mockImplementation(async (data, key) => { + return makeEncString(data); }); - encryptService.decryptToUtf8.mockImplementation(async (encString, key) => { + encryptService.decryptString.mockImplementation(async (encString, key) => { return encString.data; }); - encryptService.decryptToBytes.mockImplementation(async (encString, key) => { - return encString.dataBytes; - }); } beforeEach(() => { @@ -148,7 +140,7 @@ describe("EncString", () => { const key = new SymmetricCryptoKey(makeStaticByteArray(32)); await encString.decryptWithKey(key, encryptService); - expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(encString, key, "domain-withkey"); + expect(encryptService.decryptString).toHaveBeenCalledWith(encString, key); }); it("fails to decrypt when key is null", async () => { @@ -169,7 +161,7 @@ describe("EncString", () => { }); it("fails to decrypt when encryptService throws", async () => { - encryptService.decryptToUtf8.mockRejectedValue("error"); + encryptService.decryptString.mockRejectedValue("error"); const decrypted = await encString.decryptWithKey( new SymmetricCryptoKey(makeStaticByteArray(32)), @@ -330,7 +322,7 @@ describe("EncString", () => { }); it("handles value it can't decrypt", async () => { - encryptService.decryptToUtf8.mockRejectedValue("error"); + encryptService.decryptString.mockRejectedValue("error"); (window as any).bitwardenContainerService = new ContainerService(keyService, encryptService); @@ -350,7 +342,7 @@ describe("EncString", () => { await encString.decrypt(null, key); expect(keyService.getUserKeyWithLegacySupport).not.toHaveBeenCalled(); - expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(encString, key, "provided-key"); + expect(encryptService.decryptString).toHaveBeenCalledWith(encString, key); }); it("gets an organization key if required", async () => { @@ -361,11 +353,7 @@ describe("EncString", () => { await encString.decrypt("orgId", null); expect(keyService.getOrgKey).toHaveBeenCalledWith("orgId"); - expect(encryptService.decryptToUtf8).toHaveBeenCalledWith( - encString, - orgKey, - "domain-orgkey-orgId", - ); + expect(encryptService.decryptString).toHaveBeenCalledWith(encString, orgKey); }); it("gets the user's decryption key if required", async () => { @@ -376,11 +364,7 @@ describe("EncString", () => { await encString.decrypt(null, null); expect(keyService.getUserKeyWithLegacySupport).toHaveBeenCalledWith(); - expect(encryptService.decryptToUtf8).toHaveBeenCalledWith( - encString, - userKey, - "domain-withlegacysupport-masterkey", - ); + expect(encryptService.decryptString).toHaveBeenCalledWith(encString, userKey); }); }); diff --git a/libs/common/src/platform/models/domain/enc-string.ts b/libs/common/src/platform/models/domain/enc-string.ts index b0b03e0fb3..8ac6fe6b60 100644 --- a/libs/common/src/platform/models/domain/enc-string.ts +++ b/libs/common/src/platform/models/domain/enc-string.ts @@ -163,31 +163,16 @@ export class EncString implements Encrypted { return this.decryptedValue; } - let decryptTrace = "provided-key"; try { if (key == null) { key = await this.getKeyForDecryption(orgId); - decryptTrace = orgId == null ? `domain-orgkey-${orgId}` : "domain-userkey|masterkey"; - if (orgId != null) { - decryptTrace = `domain-orgkey-${orgId}`; - } else { - const cryptoService = Utils.getContainerService().getKeyService(); - decryptTrace = - (await cryptoService.getUserKey()) == null - ? "domain-withlegacysupport-masterkey" - : "domain-withlegacysupport-userkey"; - } } if (key == null) { throw new Error("No key to decrypt EncString with orgId " + orgId); } const encryptService = Utils.getContainerService().getEncryptService(); - this.decryptedValue = await encryptService.decryptToUtf8( - this, - key, - decryptTrace == null ? context : `${decryptTrace}${context || ""}`, - ); + this.decryptedValue = await encryptService.decryptString(this, key); // FIXME: Remove when updating file. Eslint update // eslint-disable-next-line @typescript-eslint/no-unused-vars } catch (e) { @@ -206,7 +191,7 @@ export class EncString implements Encrypted { throw new Error("No key to decrypt EncString"); } - this.decryptedValue = await encryptService.decryptToUtf8(this, key, decryptTrace); + this.decryptedValue = await encryptService.decryptString(this, key); // FIXME: Remove when updating file. Eslint update // eslint-disable-next-line @typescript-eslint/no-unused-vars } catch (e) {