From 4afe5de87b00aa9824682034dc710ed5137ca642 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Thu, 16 May 2024 15:48:03 -0400 Subject: [PATCH] Revert "[PM-7810] Handle Multithread Decryption Through Offscreen API (#8978)" (#9221) This reverts commit f51042f8131c0d6ba375520b5800683d5787dccb. --- .../browser/src/background/main.background.ts | 18 ++-- .../abstractions/offscreen-document.ts | 2 - .../offscreen-document.spec.ts | 50 ---------- .../offscreen-document/offscreen-document.ts | 35 +------ ...ead-encrypt.service.implementation.spec.ts | 97 ------------------- ...tithread-encrypt.service.implementation.ts | 91 ----------------- ...tithread-encrypt.service.implementation.ts | 40 ++------ 7 files changed, 21 insertions(+), 312 deletions(-) delete mode 100644 apps/browser/src/platform/services/browser-multithread-encrypt.service.implementation.spec.ts delete mode 100644 apps/browser/src/platform/services/browser-multithread-encrypt.service.implementation.ts diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 41f300270e..e77203f83e 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -109,6 +109,7 @@ import { DefaultConfigService } from "@bitwarden/common/platform/services/config import { ConsoleLogService } from "@bitwarden/common/platform/services/console-log.service"; import { ContainerService } from "@bitwarden/common/platform/services/container.service"; import { EncryptServiceImplementation } from "@bitwarden/common/platform/services/cryptography/encrypt.service.implementation"; +import { MultithreadEncryptServiceImplementation } from "@bitwarden/common/platform/services/cryptography/multithread-encrypt.service.implementation"; import { Fido2AuthenticatorService } from "@bitwarden/common/platform/services/fido2/fido2-authenticator.service"; import { Fido2ClientService } from "@bitwarden/common/platform/services/fido2/fido2-client.service"; import { FileUploadService } from "@bitwarden/common/platform/services/file-upload/file-upload.service"; @@ -220,7 +221,6 @@ import { BrowserCryptoService } from "../platform/services/browser-crypto.servic import { BrowserEnvironmentService } from "../platform/services/browser-environment.service"; import BrowserLocalStorageService from "../platform/services/browser-local-storage.service"; import BrowserMemoryStorageService from "../platform/services/browser-memory-storage.service"; -import { BrowserMultithreadEncryptServiceImplementation } from "../platform/services/browser-multithread-encrypt.service.implementation"; import { BrowserScriptInjectorService } from "../platform/services/browser-script-injector.service"; import { DefaultBrowserStateService } from "../platform/services/default-browser-state.service"; import I18nService from "../platform/services/i18n.service"; @@ -479,14 +479,14 @@ export default class MainBackground { storageServiceProvider, ); - this.encryptService = flagEnabled("multithreadDecryption") - ? new BrowserMultithreadEncryptServiceImplementation( - this.cryptoFunctionService, - this.logService, - true, - this.offscreenDocumentService, - ) - : new EncryptServiceImplementation(this.cryptoFunctionService, this.logService, true); + this.encryptService = + flagEnabled("multithreadDecryption") && BrowserApi.isManifestVersion(2) + ? new MultithreadEncryptServiceImplementation( + this.cryptoFunctionService, + this.logService, + true, + ) + : new EncryptServiceImplementation(this.cryptoFunctionService, this.logService, true); this.singleUserStateProvider = new DefaultSingleUserStateProvider( storageServiceProvider, diff --git a/apps/browser/src/platform/offscreen-document/abstractions/offscreen-document.ts b/apps/browser/src/platform/offscreen-document/abstractions/offscreen-document.ts index 2a67d55c96..2d3c6a3e71 100644 --- a/apps/browser/src/platform/offscreen-document/abstractions/offscreen-document.ts +++ b/apps/browser/src/platform/offscreen-document/abstractions/offscreen-document.ts @@ -2,7 +2,6 @@ export type OffscreenDocumentExtensionMessage = { [key: string]: any; command: string; text?: string; - decryptRequest?: string; }; type OffscreenExtensionMessageEventParams = { @@ -14,7 +13,6 @@ export type OffscreenDocumentExtensionMessageHandlers = { [key: string]: ({ message, sender }: OffscreenExtensionMessageEventParams) => any; offscreenCopyToClipboard: ({ message }: OffscreenExtensionMessageEventParams) => any; offscreenReadFromClipboard: () => any; - offscreenDecryptItems: ({ message }: OffscreenExtensionMessageEventParams) => Promise; }; export interface OffscreenDocument { diff --git a/apps/browser/src/platform/offscreen-document/offscreen-document.spec.ts b/apps/browser/src/platform/offscreen-document/offscreen-document.spec.ts index 9d3cadbba8..933cd08c2e 100644 --- a/apps/browser/src/platform/offscreen-document/offscreen-document.spec.ts +++ b/apps/browser/src/platform/offscreen-document/offscreen-document.spec.ts @@ -1,25 +1,7 @@ -import { mock } from "jest-mock-extended"; - -import { Decryptable } from "@bitwarden/common/platform/interfaces/decryptable.interface"; -import { InitializerMetadata } from "@bitwarden/common/platform/interfaces/initializer-metadata.interface"; -import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; - import { flushPromises, sendExtensionRuntimeMessage } from "../../autofill/spec/testing-utils"; import { BrowserApi } from "../browser/browser-api"; import BrowserClipboardService from "../services/browser-clipboard.service"; -jest.mock( - "@bitwarden/common/platform/services/cryptography/multithread-encrypt.service.implementation", - () => ({ - MultithreadEncryptServiceImplementation: class MultithreadEncryptServiceImplementation { - getDecryptedItemsFromWorker = async ( - items: Decryptable[], - _key: SymmetricCryptoKey, - ): Promise => JSON.stringify(items); - }, - }), -); - describe("OffscreenDocument", () => { const browserApiMessageListenerSpy = jest.spyOn(BrowserApi, "messageListener"); const browserClipboardServiceCopySpy = jest.spyOn(BrowserClipboardService, "copy"); @@ -78,37 +60,5 @@ describe("OffscreenDocument", () => { expect(browserClipboardServiceReadSpy).toHaveBeenCalledWith(window); }); }); - - describe("handleOffscreenDecryptItems", () => { - it("returns an empty array as a string if the decrypt request is not present in the message", async () => { - let response: string | undefined; - sendExtensionRuntimeMessage( - { command: "offscreenDecryptItems" }, - mock(), - (res: string) => (response = res), - ); - await flushPromises(); - - expect(response).toBe("[]"); - }); - - it("decrypts the items and sends back the response as a string", async () => { - const items = [{ id: "test" }]; - const key = { id: "test" }; - const decryptRequest = JSON.stringify({ items, key }); - let response: string | undefined; - - sendExtensionRuntimeMessage( - { command: "offscreenDecryptItems", decryptRequest }, - mock(), - (res: string) => { - response = res; - }, - ); - await flushPromises(); - - expect(response).toBe(JSON.stringify(items)); - }); - }); }); }); diff --git a/apps/browser/src/platform/offscreen-document/offscreen-document.ts b/apps/browser/src/platform/offscreen-document/offscreen-document.ts index 509193d5ee..4994a6e9ba 100644 --- a/apps/browser/src/platform/offscreen-document/offscreen-document.ts +++ b/apps/browser/src/platform/offscreen-document/offscreen-document.ts @@ -1,35 +1,21 @@ import { ConsoleLogService } from "@bitwarden/common/platform/services/console-log.service"; -import { MultithreadEncryptServiceImplementation } from "@bitwarden/common/platform/services/cryptography/multithread-encrypt.service.implementation"; -import { WebCryptoFunctionService } from "@bitwarden/common/platform/services/web-crypto-function.service"; import { BrowserApi } from "../browser/browser-api"; import BrowserClipboardService from "../services/browser-clipboard.service"; import { - OffscreenDocument as OffscreenDocumentInterface, OffscreenDocumentExtensionMessage, OffscreenDocumentExtensionMessageHandlers, + OffscreenDocument as OffscreenDocumentInterface, } from "./abstractions/offscreen-document"; class OffscreenDocument implements OffscreenDocumentInterface { - private readonly consoleLogService: ConsoleLogService; - private encryptService: MultithreadEncryptServiceImplementation; + private consoleLogService: ConsoleLogService = new ConsoleLogService(false); private readonly extensionMessageHandlers: OffscreenDocumentExtensionMessageHandlers = { offscreenCopyToClipboard: ({ message }) => this.handleOffscreenCopyToClipboard(message), offscreenReadFromClipboard: () => this.handleOffscreenReadFromClipboard(), - offscreenDecryptItems: ({ message }) => this.handleOffscreenDecryptItems(message), }; - constructor() { - const cryptoFunctionService = new WebCryptoFunctionService(self); - this.consoleLogService = new ConsoleLogService(false); - this.encryptService = new MultithreadEncryptServiceImplementation( - cryptoFunctionService, - this.consoleLogService, - true, - ); - } - /** * Initializes the offscreen document extension. */ @@ -53,23 +39,6 @@ class OffscreenDocument implements OffscreenDocumentInterface { return await BrowserClipboardService.read(self); } - /** - * Decrypts the items in the message using the encrypt service. - * - * @param message - The extension message containing the items to decrypt - */ - private async handleOffscreenDecryptItems( - message: OffscreenDocumentExtensionMessage, - ): Promise { - const { decryptRequest } = message; - if (!decryptRequest) { - return "[]"; - } - - const request = JSON.parse(decryptRequest); - return await this.encryptService.getDecryptedItemsFromWorker(request.items, request.key); - } - /** * Sets up the listener for extension messages. */ diff --git a/apps/browser/src/platform/services/browser-multithread-encrypt.service.implementation.spec.ts b/apps/browser/src/platform/services/browser-multithread-encrypt.service.implementation.spec.ts deleted file mode 100644 index db5b3df7a3..0000000000 --- a/apps/browser/src/platform/services/browser-multithread-encrypt.service.implementation.spec.ts +++ /dev/null @@ -1,97 +0,0 @@ -import { mock, MockProxy } from "jest-mock-extended"; - -import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service"; -import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; -import { EncryptionType } from "@bitwarden/common/platform/enums"; -import { Decryptable } from "@bitwarden/common/platform/interfaces/decryptable.interface"; -import { InitializerMetadata } from "@bitwarden/common/platform/interfaces/initializer-metadata.interface"; -import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; -import { InitializerKey } from "@bitwarden/common/platform/services/cryptography/initializer-key"; -import { makeStaticByteArray } from "@bitwarden/common/spec"; - -import { BrowserApi } from "../browser/browser-api"; -import { OffscreenDocumentService } from "../offscreen-document/abstractions/offscreen-document"; - -import { BrowserMultithreadEncryptServiceImplementation } from "./browser-multithread-encrypt.service.implementation"; - -describe("BrowserMultithreadEncryptServiceImplementation", () => { - let cryptoFunctionServiceMock: MockProxy; - let logServiceMock: MockProxy; - let offscreenDocumentServiceMock: MockProxy; - let encryptService: BrowserMultithreadEncryptServiceImplementation; - const manifestVersionSpy = jest.spyOn(BrowserApi, "manifestVersion", "get"); - const sendMessageWithResponseSpy = jest.spyOn(BrowserApi, "sendMessageWithResponse"); - const encType = EncryptionType.AesCbc256_HmacSha256_B64; - const key = new SymmetricCryptoKey(makeStaticByteArray(64, 100), encType); - const items: Decryptable[] = [ - { - decrypt: jest.fn(), - initializerKey: InitializerKey.Cipher, - }, - ]; - - beforeEach(() => { - cryptoFunctionServiceMock = mock(); - logServiceMock = mock(); - offscreenDocumentServiceMock = mock({ - withDocument: jest.fn((_, __, callback) => callback() as any), - }); - encryptService = new BrowserMultithreadEncryptServiceImplementation( - cryptoFunctionServiceMock, - logServiceMock, - false, - offscreenDocumentServiceMock, - ); - manifestVersionSpy.mockReturnValue(3); - sendMessageWithResponseSpy.mockResolvedValue(JSON.stringify([])); - }); - - afterEach(() => { - jest.clearAllMocks(); - }); - - it("decrypts items using web workers if the chrome.offscreen API is not supported", async () => { - manifestVersionSpy.mockReturnValue(2); - - await encryptService.decryptItems([], key); - - expect(offscreenDocumentServiceMock.withDocument).not.toHaveBeenCalled(); - }); - - it("decrypts items using the chrome.offscreen API if it is supported", async () => { - sendMessageWithResponseSpy.mockResolvedValue(JSON.stringify(items)); - - await encryptService.decryptItems(items, key); - - expect(offscreenDocumentServiceMock.withDocument).toHaveBeenCalledWith( - [chrome.offscreen.Reason.WORKERS], - "Use web worker to decrypt items.", - expect.any(Function), - ); - expect(BrowserApi.sendMessageWithResponse).toHaveBeenCalledWith("offscreenDecryptItems", { - decryptRequest: expect.any(String), - }); - }); - - it("returns an empty array if the passed items are not defined", async () => { - const result = await encryptService.decryptItems(null, key); - - expect(result).toEqual([]); - }); - - it("returns an empty array if the offscreen document message returns an empty value", async () => { - sendMessageWithResponseSpy.mockResolvedValue(""); - - const result = await encryptService.decryptItems(items, key); - - expect(result).toEqual([]); - }); - - it("returns an empty array if the offscreen document message returns an empty array", async () => { - sendMessageWithResponseSpy.mockResolvedValue("[]"); - - const result = await encryptService.decryptItems(items, key); - - expect(result).toEqual([]); - }); -}); diff --git a/apps/browser/src/platform/services/browser-multithread-encrypt.service.implementation.ts b/apps/browser/src/platform/services/browser-multithread-encrypt.service.implementation.ts deleted file mode 100644 index ace5015c8e..0000000000 --- a/apps/browser/src/platform/services/browser-multithread-encrypt.service.implementation.ts +++ /dev/null @@ -1,91 +0,0 @@ -import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service"; -import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; -import { Decryptable } from "@bitwarden/common/platform/interfaces/decryptable.interface"; -import { InitializerMetadata } from "@bitwarden/common/platform/interfaces/initializer-metadata.interface"; -import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; -import { MultithreadEncryptServiceImplementation } from "@bitwarden/common/platform/services/cryptography/multithread-encrypt.service.implementation"; - -import { BrowserApi } from "../browser/browser-api"; -import { OffscreenDocumentService } from "../offscreen-document/abstractions/offscreen-document"; - -export class BrowserMultithreadEncryptServiceImplementation extends MultithreadEncryptServiceImplementation { - constructor( - cryptoFunctionService: CryptoFunctionService, - logService: LogService, - logMacFailures: boolean, - private offscreenDocumentService: OffscreenDocumentService, - ) { - super(cryptoFunctionService, logService, logMacFailures); - } - - /** - * Handles decryption of items, will use the offscreen document if supported. - * - * @param items - The items to decrypt. - * @param key - The key to use for decryption. - */ - async decryptItems( - items: Decryptable[], - key: SymmetricCryptoKey, - ): Promise { - if (!this.isOffscreenDocumentSupported()) { - return await super.decryptItems(items, key); - } - - return await this.decryptItemsInOffscreenDocument(items, key); - } - - /** - * Decrypts items using the offscreen document api. - * - * @param items - The items to decrypt. - * @param key - The key to use for decryption. - */ - private async decryptItemsInOffscreenDocument( - items: Decryptable[], - key: SymmetricCryptoKey, - ): Promise { - if (items == null || items.length < 1) { - return []; - } - - const request = { - id: Utils.newGuid(), - items: items, - key: key, - }; - - const response = await this.offscreenDocumentService.withDocument( - [chrome.offscreen.Reason.WORKERS], - "Use web worker to decrypt items.", - async () => { - return (await BrowserApi.sendMessageWithResponse("offscreenDecryptItems", { - decryptRequest: JSON.stringify(request), - })) as string; - }, - ); - - if (!response) { - return []; - } - - const responseItems = JSON.parse(response); - if (responseItems?.length < 1) { - return []; - } - - return this.initializeItems(responseItems); - } - - /** - * Checks if the offscreen document api is supported. - */ - private isOffscreenDocumentSupported() { - return ( - BrowserApi.isManifestVersion(3) && - typeof chrome !== "undefined" && - typeof chrome.offscreen !== "undefined" - ); - } -} diff --git a/libs/common/src/platform/services/cryptography/multithread-encrypt.service.implementation.ts b/libs/common/src/platform/services/cryptography/multithread-encrypt.service.implementation.ts index 75a571fef2..6ac343bcb6 100644 --- a/libs/common/src/platform/services/cryptography/multithread-encrypt.service.implementation.ts +++ b/libs/common/src/platform/services/cryptography/multithread-encrypt.service.implementation.ts @@ -19,36 +19,17 @@ export class MultithreadEncryptServiceImplementation extends EncryptServiceImple private clear$ = new Subject(); /** - * Decrypts items using a web worker if the environment supports it. - * Will fall back to the main thread if the window object is not available. + * Sends items to a web worker to decrypt them. + * This utilises multithreading to decrypt items faster without interrupting other operations (e.g. updating UI). */ async decryptItems( items: Decryptable[], key: SymmetricCryptoKey, ): Promise { - if (typeof window === "undefined") { - return super.decryptItems(items, key); - } - if (items == null || items.length < 1) { return []; } - const decryptedItems = await this.getDecryptedItemsFromWorker(items, key); - const parsedItems = JSON.parse(decryptedItems); - - return this.initializeItems(parsedItems); - } - - /** - * Sends items to a web worker to decrypt them. This utilizes multithreading to decrypt items - * faster without interrupting other operations (e.g. updating UI). This method returns values - * prior to deserialization to support forwarding results to another party - */ - async getDecryptedItemsFromWorker( - items: Decryptable[], - key: SymmetricCryptoKey, - ): Promise { this.logService.info("Starting decryption using multithreading"); this.worker ??= new Worker( @@ -72,20 +53,19 @@ export class MultithreadEncryptServiceImplementation extends EncryptServiceImple return await firstValueFrom( fromEvent(this.worker, "message").pipe( filter((response: MessageEvent) => response.data?.id === request.id), - map((response) => response.data.items), + map((response) => JSON.parse(response.data.items)), + map((items) => + items.map((jsonItem: Jsonify) => { + const initializer = getClassInitializer(jsonItem.initializerKey); + return initializer(jsonItem); + }), + ), takeUntil(this.clear$), - defaultIfEmpty("[]"), + defaultIfEmpty([]), ), ); } - protected initializeItems(items: Jsonify[]): T[] { - return items.map((jsonItem: Jsonify) => { - const initializer = getClassInitializer(jsonItem.initializerKey); - return initializer(jsonItem); - }); - } - private clear() { this.clear$.next(); this.worker?.terminate();