From 37eef7731fb2beab0d9e7b45d1dac3699e835401 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Wed, 22 May 2024 09:56:24 -0400 Subject: [PATCH] Prefer prefetching keys (#9289) magical black-box key retrieval is slow, due to needing to repeatedly resolve (and potentially decrypt) keys. Doing the work up front is both more explicit and much faster. It would be preferable to move OrganizationId to an opaque type in the models, but that has rippling effects all over the place and ultimately is stopped by vault filtering on strings rather than ids, but still calling the property `organizationId`, we need to fix that first. --- libs/common/spec/utils.ts | 5 +++++ .../common/src/vault/models/domain/collection.spec.ts | 9 ++++++--- libs/common/src/vault/models/domain/collection.ts | 4 +++- libs/common/src/vault/services/collection.service.ts | 11 +++++++++-- .../importers/bitwarden/bitwarden-json-importer.ts | 7 ++++++- .../src/services/org-vault-export.service.ts | 10 +++++++--- 6 files changed, 36 insertions(+), 10 deletions(-) diff --git a/libs/common/spec/utils.ts b/libs/common/spec/utils.ts index 93142d2571d..d3722329370 100644 --- a/libs/common/spec/utils.ts +++ b/libs/common/spec/utils.ts @@ -5,6 +5,7 @@ import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { EncryptionType } from "../src/platform/enums"; import { Utils } from "../src/platform/misc/utils"; +import { SymmetricCryptoKey } from "../src/platform/models/domain/symmetric-crypto-key"; function newGuid() { return "xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx".replace(/[xy]/g, (c) => { @@ -45,6 +46,10 @@ export function makeStaticByteArray(length: number, start = 0) { return arr; } +export function makeSymmetricCryptoKey(length: 32 | 64 = 64) { + return new SymmetricCryptoKey(makeStaticByteArray(length)) as T; +} + /** * Use to mock a return value of a static fromJSON method. */ diff --git a/libs/common/src/vault/models/domain/collection.spec.ts b/libs/common/src/vault/models/domain/collection.spec.ts index 4ee725be57f..9ec1d34ff7c 100644 --- a/libs/common/src/vault/models/domain/collection.spec.ts +++ b/libs/common/src/vault/models/domain/collection.spec.ts @@ -1,5 +1,6 @@ -import { mockEnc } from "../../../../spec"; +import { makeSymmetricCryptoKey, mockEnc } from "../../../../spec"; import { CollectionId, OrganizationId } from "../../../types/guid"; +import { OrgKey } from "../../../types/key"; import { CollectionData } from "../data/collection.data"; import { Collection } from "./collection"; @@ -51,14 +52,16 @@ describe("Collection", () => { it("Decrypt", async () => { const collection = new Collection(); collection.id = "id"; - collection.organizationId = "orgId"; + collection.organizationId = "orgId" as OrganizationId; collection.name = mockEnc("encName"); collection.externalId = "extId"; collection.readOnly = false; collection.hidePasswords = false; collection.manage = true; - const view = await collection.decrypt(); + const key = makeSymmetricCryptoKey(); + + const view = await collection.decrypt(key); expect(view).toEqual({ addAccess: false, diff --git a/libs/common/src/vault/models/domain/collection.ts b/libs/common/src/vault/models/domain/collection.ts index 29030dcb52f..89da8bef880 100644 --- a/libs/common/src/vault/models/domain/collection.ts +++ b/libs/common/src/vault/models/domain/collection.ts @@ -1,5 +1,6 @@ import Domain from "../../../platform/models/domain/domain-base"; import { EncString } from "../../../platform/models/domain/enc-string"; +import { OrgKey } from "../../../types/key"; import { CollectionData } from "../data/collection.data"; import { CollectionView } from "../view/collection.view"; @@ -34,13 +35,14 @@ export class Collection extends Domain { ); } - decrypt(): Promise { + decrypt(orgKey: OrgKey): Promise { return this.decryptObj( new CollectionView(this), { name: null, }, this.organizationId, + orgKey, ); } } diff --git a/libs/common/src/vault/services/collection.service.ts b/libs/common/src/vault/services/collection.service.ts index 897fb4a772d..2c91651a0e0 100644 --- a/libs/common/src/vault/services/collection.service.ts +++ b/libs/common/src/vault/services/collection.service.ts @@ -12,7 +12,7 @@ import { DeriveDefinition, DerivedState, } from "../../platform/state"; -import { CollectionId, UserId } from "../../types/guid"; +import { CollectionId, OrganizationId, UserId } from "../../types/guid"; import { CollectionService as CollectionServiceAbstraction } from "../../vault/abstractions/collection.service"; import { CollectionData } from "../models/data/collection.data"; import { Collection } from "../models/domain/collection"; @@ -108,9 +108,16 @@ export class CollectionService implements CollectionServiceAbstraction { return []; } const decCollections: CollectionView[] = []; + + const organizationKeys = await firstValueFrom(this.cryptoService.activeUserOrgKeys$); + const promises: Promise[] = []; collections.forEach((collection) => { - promises.push(collection.decrypt().then((c) => decCollections.push(c))); + promises.push( + collection + .decrypt(organizationKeys[collection.organizationId as OrganizationId]) + .then((c) => decCollections.push(c)), + ); }); await Promise.all(promises); return decCollections.sort(Utils.getSortFunction(this.i18nService, "name")); diff --git a/libs/importer/src/importers/bitwarden/bitwarden-json-importer.ts b/libs/importer/src/importers/bitwarden/bitwarden-json-importer.ts index 5858fdf35be..bef707c3e53 100644 --- a/libs/importer/src/importers/bitwarden/bitwarden-json-importer.ts +++ b/libs/importer/src/importers/bitwarden/bitwarden-json-importer.ts @@ -1,3 +1,5 @@ +import { firstValueFrom } from "rxjs"; + import { PinServiceAbstraction } from "@bitwarden/auth/common"; import { CipherWithIdExport, @@ -7,6 +9,7 @@ import { import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; +import { OrganizationId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view"; import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; @@ -194,7 +197,9 @@ export class BitwardenJsonImporter extends BaseImporter implements Importer { if (data.encrypted) { const collection = CollectionWithIdExport.toDomain(c); collection.organizationId = this.organizationId; - collectionView = await collection.decrypt(); + collectionView = await firstValueFrom(this.cryptoService.activeUserOrgKeys$).then( + (orgKeys) => collection.decrypt(orgKeys[c.organizationId as OrganizationId]), + ); } else { collectionView = CollectionWithIdExport.toView(c); collectionView.organizationId = null; diff --git a/libs/tools/export/vault-export/vault-export-core/src/services/org-vault-export.service.ts b/libs/tools/export/vault-export/vault-export-core/src/services/org-vault-export.service.ts index 652f53acf52..390b9e0b5a1 100644 --- a/libs/tools/export/vault-export/vault-export-core/src/services/org-vault-export.service.ts +++ b/libs/tools/export/vault-export/vault-export-core/src/services/org-vault-export.service.ts @@ -1,4 +1,5 @@ import * as papa from "papaparse"; +import { firstValueFrom } from "rxjs"; import { PinServiceAbstraction } from "@bitwarden/auth/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; @@ -7,6 +8,7 @@ import { CipherWithIdExport, CollectionWithIdExport } from "@bitwarden/common/mo import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service"; import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { OrganizationId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CollectionService } from "@bitwarden/common/vault/abstractions/collection.service"; import { CipherType } from "@bitwarden/common/vault/enums"; @@ -94,9 +96,11 @@ export class OrganizationVaultExportService exportData.collections.forEach((c) => { const collection = new Collection(new CollectionData(c as CollectionDetailsResponse)); exportPromises.push( - collection.decrypt().then((decCol) => { - decCollections.push(decCol); - }), + firstValueFrom(this.cryptoService.activeUserOrgKeys$) + .then((keys) => collection.decrypt(keys[organizationId as OrganizationId])) + .then((decCol) => { + decCollections.push(decCol); + }), ); }); }