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.
This commit is contained in:
Matt Gibson 2024-05-22 09:56:24 -04:00 committed by GitHub
parent 18f14d8b7d
commit 37eef7731f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 36 additions and 10 deletions

View File

@ -5,6 +5,7 @@ import { EncString } from "@bitwarden/common/platform/models/domain/enc-string";
import { EncryptionType } from "../src/platform/enums"; import { EncryptionType } from "../src/platform/enums";
import { Utils } from "../src/platform/misc/utils"; import { Utils } from "../src/platform/misc/utils";
import { SymmetricCryptoKey } from "../src/platform/models/domain/symmetric-crypto-key";
function newGuid() { function newGuid() {
return "xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx".replace(/[xy]/g, (c) => { return "xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx".replace(/[xy]/g, (c) => {
@ -45,6 +46,10 @@ export function makeStaticByteArray(length: number, start = 0) {
return arr; return arr;
} }
export function makeSymmetricCryptoKey<T extends SymmetricCryptoKey>(length: 32 | 64 = 64) {
return new SymmetricCryptoKey(makeStaticByteArray(length)) as T;
}
/** /**
* Use to mock a return value of a static fromJSON method. * Use to mock a return value of a static fromJSON method.
*/ */

View File

@ -1,5 +1,6 @@
import { mockEnc } from "../../../../spec"; import { makeSymmetricCryptoKey, mockEnc } from "../../../../spec";
import { CollectionId, OrganizationId } from "../../../types/guid"; import { CollectionId, OrganizationId } from "../../../types/guid";
import { OrgKey } from "../../../types/key";
import { CollectionData } from "../data/collection.data"; import { CollectionData } from "../data/collection.data";
import { Collection } from "./collection"; import { Collection } from "./collection";
@ -51,14 +52,16 @@ describe("Collection", () => {
it("Decrypt", async () => { it("Decrypt", async () => {
const collection = new Collection(); const collection = new Collection();
collection.id = "id"; collection.id = "id";
collection.organizationId = "orgId"; collection.organizationId = "orgId" as OrganizationId;
collection.name = mockEnc("encName"); collection.name = mockEnc("encName");
collection.externalId = "extId"; collection.externalId = "extId";
collection.readOnly = false; collection.readOnly = false;
collection.hidePasswords = false; collection.hidePasswords = false;
collection.manage = true; collection.manage = true;
const view = await collection.decrypt(); const key = makeSymmetricCryptoKey<OrgKey>();
const view = await collection.decrypt(key);
expect(view).toEqual({ expect(view).toEqual({
addAccess: false, addAccess: false,

View File

@ -1,5 +1,6 @@
import Domain from "../../../platform/models/domain/domain-base"; import Domain from "../../../platform/models/domain/domain-base";
import { EncString } from "../../../platform/models/domain/enc-string"; import { EncString } from "../../../platform/models/domain/enc-string";
import { OrgKey } from "../../../types/key";
import { CollectionData } from "../data/collection.data"; import { CollectionData } from "../data/collection.data";
import { CollectionView } from "../view/collection.view"; import { CollectionView } from "../view/collection.view";
@ -34,13 +35,14 @@ export class Collection extends Domain {
); );
} }
decrypt(): Promise<CollectionView> { decrypt(orgKey: OrgKey): Promise<CollectionView> {
return this.decryptObj( return this.decryptObj(
new CollectionView(this), new CollectionView(this),
{ {
name: null, name: null,
}, },
this.organizationId, this.organizationId,
orgKey,
); );
} }
} }

View File

@ -12,7 +12,7 @@ import {
DeriveDefinition, DeriveDefinition,
DerivedState, DerivedState,
} from "../../platform/state"; } 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 { CollectionService as CollectionServiceAbstraction } from "../../vault/abstractions/collection.service";
import { CollectionData } from "../models/data/collection.data"; import { CollectionData } from "../models/data/collection.data";
import { Collection } from "../models/domain/collection"; import { Collection } from "../models/domain/collection";
@ -108,9 +108,16 @@ export class CollectionService implements CollectionServiceAbstraction {
return []; return [];
} }
const decCollections: CollectionView[] = []; const decCollections: CollectionView[] = [];
const organizationKeys = await firstValueFrom(this.cryptoService.activeUserOrgKeys$);
const promises: Promise<any>[] = []; const promises: Promise<any>[] = [];
collections.forEach((collection) => { 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); await Promise.all(promises);
return decCollections.sort(Utils.getSortFunction(this.i18nService, "name")); return decCollections.sort(Utils.getSortFunction(this.i18nService, "name"));

View File

@ -1,3 +1,5 @@
import { firstValueFrom } from "rxjs";
import { PinServiceAbstraction } from "@bitwarden/auth/common"; import { PinServiceAbstraction } from "@bitwarden/auth/common";
import { import {
CipherWithIdExport, CipherWithIdExport,
@ -7,6 +9,7 @@ import {
import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; 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 { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view"; import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view";
import { FolderView } from "@bitwarden/common/vault/models/view/folder.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) { if (data.encrypted) {
const collection = CollectionWithIdExport.toDomain(c); const collection = CollectionWithIdExport.toDomain(c);
collection.organizationId = this.organizationId; collection.organizationId = this.organizationId;
collectionView = await collection.decrypt(); collectionView = await firstValueFrom(this.cryptoService.activeUserOrgKeys$).then(
(orgKeys) => collection.decrypt(orgKeys[c.organizationId as OrganizationId]),
);
} else { } else {
collectionView = CollectionWithIdExport.toView(c); collectionView = CollectionWithIdExport.toView(c);
collectionView.organizationId = null; collectionView.organizationId = null;

View File

@ -1,4 +1,5 @@
import * as papa from "papaparse"; import * as papa from "papaparse";
import { firstValueFrom } from "rxjs";
import { PinServiceAbstraction } from "@bitwarden/auth/common"; import { PinServiceAbstraction } from "@bitwarden/auth/common";
import { ApiService } from "@bitwarden/common/abstractions/api.service"; 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 { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service";
import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service";
import { Utils } from "@bitwarden/common/platform/misc/utils"; 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 { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CollectionService } from "@bitwarden/common/vault/abstractions/collection.service"; import { CollectionService } from "@bitwarden/common/vault/abstractions/collection.service";
import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherType } from "@bitwarden/common/vault/enums";
@ -94,7 +96,9 @@ export class OrganizationVaultExportService
exportData.collections.forEach((c) => { exportData.collections.forEach((c) => {
const collection = new Collection(new CollectionData(c as CollectionDetailsResponse)); const collection = new Collection(new CollectionData(c as CollectionDetailsResponse));
exportPromises.push( exportPromises.push(
collection.decrypt().then((decCol) => { firstValueFrom(this.cryptoService.activeUserOrgKeys$)
.then((keys) => collection.decrypt(keys[organizationId as OrganizationId]))
.then((decCol) => {
decCollections.push(decCol); decCollections.push(decCol);
}), }),
); );