From 744c1b1b49ac5d2eebb52b7add4c00c99f0845e4 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Tue, 6 May 2025 23:24:53 +0200 Subject: [PATCH] [PM-21001] Move vault code to new encrypt service interface (#14546) * Move vault code to new encrypt service interface * Fix tests --- apps/cli/src/vault/create.command.ts | 2 +- .../vault/components/attachments.component.ts | 4 +-- .../src/vault/components/view.component.ts | 2 +- .../vault/models/domain/attachment.spec.ts | 11 ++++--- .../src/vault/models/domain/attachment.ts | 4 +-- .../src/vault/models/domain/cipher.spec.ts | 17 +++++++--- libs/common/src/vault/models/domain/cipher.ts | 10 ++---- .../src/vault/models/domain/folder.spec.ts | 10 +++++- .../src/vault/services/cipher.service.spec.ts | 14 +++++--- .../src/vault/services/cipher.service.ts | 33 +++++++++++-------- .../services/folder/folder.service.spec.ts | 5 +-- .../vault/services/folder/folder.service.ts | 2 +- ...symmetric-key-regeneration.service.spec.ts | 5 +++ .../download-attachment.component.ts | 2 +- 14 files changed, 76 insertions(+), 45 deletions(-) diff --git a/apps/cli/src/vault/create.command.ts b/apps/cli/src/vault/create.command.ts index 713471356c..5b34d2cb50 100644 --- a/apps/cli/src/vault/create.command.ts +++ b/apps/cli/src/vault/create.command.ts @@ -227,7 +227,7 @@ export class CreateCommand { (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/libs/angular/src/vault/components/attachments.component.ts b/libs/angular/src/vault/components/attachments.component.ts index 8cc4d40856..9e9450c587 100644 --- a/libs/angular/src/vault/components/attachments.component.ts +++ b/libs/angular/src/vault/components/attachments.component.ts @@ -202,7 +202,7 @@ export class AttachmentsComponent implements OnInit { attachment.key != null ? attachment.key : await this.keyService.getOrgKey(this.cipher.organizationId); - const decBuf = await this.encryptService.decryptToBytes(encBuf, key); + const decBuf = await this.encryptService.decryptFileData(encBuf, key); this.fileDownloadService.download({ fileName: attachment.fileName, blobData: decBuf, @@ -281,7 +281,7 @@ export class AttachmentsComponent implements OnInit { attachment.key != null ? attachment.key : await this.keyService.getOrgKey(this.cipher.organizationId); - const decBuf = await this.encryptService.decryptToBytes(encBuf, key); + const decBuf = await this.encryptService.decryptFileData(encBuf, key); const activeUserId = await firstValueFrom( this.accountService.activeAccount$.pipe(getUserId), ); diff --git a/libs/angular/src/vault/components/view.component.ts b/libs/angular/src/vault/components/view.component.ts index 6b6f24f421..9d5a8fe9e6 100644 --- a/libs/angular/src/vault/components/view.component.ts +++ b/libs/angular/src/vault/components/view.component.ts @@ -463,7 +463,7 @@ export class ViewComponent implements OnDestroy, OnInit { attachment.key != null ? attachment.key : await this.keyService.getOrgKey(this.cipher.organizationId); - const decBuf = await this.encryptService.decryptToBytes(encBuf, key); + const decBuf = await this.encryptService.decryptFileData(encBuf, key); this.fileDownloadService.download({ fileName: attachment.fileName, blobData: decBuf, diff --git a/libs/common/src/vault/models/domain/attachment.spec.ts b/libs/common/src/vault/models/domain/attachment.spec.ts index d1ee1dcc1e..40d7ea7f05 100644 --- a/libs/common/src/vault/models/domain/attachment.spec.ts +++ b/libs/common/src/vault/models/domain/attachment.spec.ts @@ -77,7 +77,10 @@ describe("Attachment", () => { attachment.key = mockEnc("key"); attachment.fileName = mockEnc("fileName"); - encryptService.decryptToBytes.mockResolvedValue(makeStaticByteArray(32)); + encryptService.decryptFileData.mockResolvedValue(makeStaticByteArray(32)); + encryptService.unwrapSymmetricKey.mockResolvedValue( + new SymmetricCryptoKey(makeStaticByteArray(64)), + ); const view = await attachment.decrypt(null); @@ -105,7 +108,7 @@ describe("Attachment", () => { await attachment.decrypt(null, "", providedKey); expect(keyService.getUserKeyWithLegacySupport).not.toHaveBeenCalled(); - expect(encryptService.decryptToBytes).toHaveBeenCalledWith(attachment.key, providedKey); + expect(encryptService.unwrapSymmetricKey).toHaveBeenCalledWith(attachment.key, providedKey); }); it("gets an organization key if required", async () => { @@ -115,7 +118,7 @@ describe("Attachment", () => { await attachment.decrypt("orgId", "", null); expect(keyService.getOrgKey).toHaveBeenCalledWith("orgId"); - expect(encryptService.decryptToBytes).toHaveBeenCalledWith(attachment.key, orgKey); + expect(encryptService.unwrapSymmetricKey).toHaveBeenCalledWith(attachment.key, orgKey); }); it("gets the user's decryption key if required", async () => { @@ -125,7 +128,7 @@ describe("Attachment", () => { await attachment.decrypt(null, "", null); expect(keyService.getUserKeyWithLegacySupport).toHaveBeenCalled(); - expect(encryptService.decryptToBytes).toHaveBeenCalledWith(attachment.key, userKey); + expect(encryptService.unwrapSymmetricKey).toHaveBeenCalledWith(attachment.key, userKey); }); }); }); diff --git a/libs/common/src/vault/models/domain/attachment.ts b/libs/common/src/vault/models/domain/attachment.ts index 16f3adbe30..15ce06e088 100644 --- a/libs/common/src/vault/models/domain/attachment.ts +++ b/libs/common/src/vault/models/domain/attachment.ts @@ -66,8 +66,8 @@ export class Attachment extends Domain { } const encryptService = Utils.getContainerService().getEncryptService(); - const decValue = await encryptService.decryptToBytes(this.key, encKey); - return new SymmetricCryptoKey(decValue); + const decValue = await encryptService.unwrapSymmetricKey(this.key, encKey); + return decValue; // FIXME: Remove when updating file. Eslint update // eslint-disable-next-line @typescript-eslint/no-unused-vars } catch (e) { diff --git a/libs/common/src/vault/models/domain/cipher.spec.ts b/libs/common/src/vault/models/domain/cipher.spec.ts index 1b2b093a55..0ef2233120 100644 --- a/libs/common/src/vault/models/domain/cipher.spec.ts +++ b/libs/common/src/vault/models/domain/cipher.spec.ts @@ -1,6 +1,7 @@ import { mock } from "jest-mock-extended"; import { Jsonify } from "type-fest"; +import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { KeyService } from "@bitwarden/key-management"; import { makeStaticByteArray, mockEnc, mockFromJson } from "../../../../spec/utils"; @@ -246,7 +247,9 @@ describe("Cipher DTO", () => { const encryptService = mock(); const cipherService = mock(); - encryptService.decryptToBytes.mockResolvedValue(makeStaticByteArray(64)); + encryptService.unwrapSymmetricKey.mockResolvedValue( + new SymmetricCryptoKey(makeStaticByteArray(64)), + ); (window as any).bitwardenContainerService = new ContainerService(keyService, encryptService); @@ -367,7 +370,9 @@ describe("Cipher DTO", () => { const encryptService = mock(); const cipherService = mock(); - encryptService.decryptToBytes.mockResolvedValue(makeStaticByteArray(64)); + encryptService.unwrapSymmetricKey.mockResolvedValue( + new SymmetricCryptoKey(makeStaticByteArray(64)), + ); (window as any).bitwardenContainerService = new ContainerService(keyService, encryptService); @@ -506,7 +511,9 @@ describe("Cipher DTO", () => { const encryptService = mock(); const cipherService = mock(); - encryptService.decryptToBytes.mockResolvedValue(makeStaticByteArray(64)); + encryptService.unwrapSymmetricKey.mockResolvedValue( + new SymmetricCryptoKey(makeStaticByteArray(64)), + ); (window as any).bitwardenContainerService = new ContainerService(keyService, encryptService); @@ -669,7 +676,9 @@ describe("Cipher DTO", () => { const encryptService = mock(); const cipherService = mock(); - encryptService.decryptToBytes.mockResolvedValue(makeStaticByteArray(64)); + encryptService.unwrapSymmetricKey.mockResolvedValue( + new SymmetricCryptoKey(makeStaticByteArray(64)), + ); (window as any).bitwardenContainerService = new ContainerService(keyService, encryptService); diff --git a/libs/common/src/vault/models/domain/cipher.ts b/libs/common/src/vault/models/domain/cipher.ts index f23e3c0c57..780690217a 100644 --- a/libs/common/src/vault/models/domain/cipher.ts +++ b/libs/common/src/vault/models/domain/cipher.ts @@ -143,17 +143,13 @@ export class Cipher extends Domain implements Decryptable { if (this.key != null) { const encryptService = Utils.getContainerService().getEncryptService(); - const keyBytes = await encryptService.decryptToBytes( - this.key, - encKey, - `Cipher Id: ${this.id}; Content: CipherKey; IsEncryptedByOrgKey: ${this.organizationId != null}`, - ); - if (keyBytes == null) { + const cipherKey = await encryptService.unwrapSymmetricKey(this.key, encKey); + if (cipherKey == null) { model.name = "[error: cannot decrypt]"; model.decryptionFailure = true; return model; } - encKey = new SymmetricCryptoKey(keyBytes); + encKey = cipherKey; bypassValidation = false; } diff --git a/libs/common/src/vault/models/domain/folder.spec.ts b/libs/common/src/vault/models/domain/folder.spec.ts index ff1c38bdd4..cad17a9ee0 100644 --- a/libs/common/src/vault/models/domain/folder.spec.ts +++ b/libs/common/src/vault/models/domain/folder.spec.ts @@ -1,5 +1,7 @@ import { mock, MockProxy } from "jest-mock-extended"; +import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; + import { makeEncString, makeSymmetricCryptoKey, mockEnc, mockFromJson } from "../../../../spec"; import { EncryptService } from "../../../key-management/crypto/abstractions/encrypt.service"; import { EncryptedString, EncString } from "../../../platform/models/domain/enc-string"; @@ -70,7 +72,13 @@ describe("Folder", () => { beforeEach(() => { encryptService = mock(); - encryptService.decryptToUtf8.mockImplementation((value) => { + // Platform code is not migrated yet + encryptService.decryptToUtf8.mockImplementation( + (value: EncString, key: SymmetricCryptoKey, decryptTrace: string) => { + return Promise.resolve(value.data); + }, + ); + encryptService.decryptString.mockImplementation((value) => { return Promise.resolve(value.data); }); }); diff --git a/libs/common/src/vault/services/cipher.service.spec.ts b/libs/common/src/vault/services/cipher.service.spec.ts index 57df5f2a37..a8b37e8adc 100644 --- a/libs/common/src/vault/services/cipher.service.spec.ts +++ b/libs/common/src/vault/services/cipher.service.spec.ts @@ -131,8 +131,8 @@ describe("Cipher Service", () => { let cipherObj: Cipher; beforeEach(() => { - encryptService.encryptToBytes.mockReturnValue(Promise.resolve(ENCRYPTED_BYTES)); - encryptService.encrypt.mockReturnValue(Promise.resolve(new EncString(ENCRYPTED_TEXT))); + encryptService.encryptFileData.mockReturnValue(Promise.resolve(ENCRYPTED_BYTES)); + encryptService.encryptString.mockReturnValue(Promise.resolve(new EncString(ENCRYPTED_TEXT))); (window as any).bitwardenContainerService = new ContainerService(keyService, encryptService); @@ -274,12 +274,14 @@ describe("Cipher Service", () => { cipherView = new CipherView(); cipherView.type = CipherType.Login; - encryptService.decryptToBytes.mockReturnValue(Promise.resolve(makeStaticByteArray(64))); + encryptService.unwrapSymmetricKey.mockResolvedValue( + new SymmetricCryptoKey(makeStaticByteArray(64)), + ); configService.checkServerMeetsVersionRequirement$.mockReturnValue(of(true)); keyService.makeCipherKey.mockReturnValue( Promise.resolve(new SymmetricCryptoKey(makeStaticByteArray(64)) as CipherKey), ); - encryptService.encrypt.mockImplementation(encryptText); + encryptService.encryptString.mockImplementation(encryptText); encryptService.wrapSymmetricKey.mockResolvedValue(new EncString("Re-encrypted Cipher Key")); jest.spyOn(cipherService as any, "getAutofillOnPageLoadDefault").mockResolvedValue(true); @@ -435,7 +437,9 @@ describe("Cipher Service", () => { .spyOn(cipherService, "failedToDecryptCiphers$") .mockImplementation((userId: UserId) => failedCiphers); - encryptService.decryptToBytes.mockResolvedValue(new Uint8Array(32)); + encryptService.unwrapSymmetricKey.mockResolvedValue( + new SymmetricCryptoKey(new Uint8Array(32)), + ); encryptedKey = new EncString("Re-encrypted Cipher Key"); encryptService.wrapSymmetricKey.mockResolvedValue(encryptedKey); diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index 473525711f..24ddeda66d 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -877,9 +877,7 @@ export class CipherService implements CipherServiceAbstraction { const cipherEncKey = cipherKeyEncryptionEnabled && cipher.key != null - ? (new SymmetricCryptoKey( - await this.encryptService.decryptToBytes(cipher.key, encKey), - ) as UserKey) + ? ((await this.encryptService.unwrapSymmetricKey(cipher.key, encKey)) as UserKey) : encKey; //if cipher key encryption is disabled but the item has an individual key, @@ -891,10 +889,10 @@ export class CipherService implements CipherServiceAbstraction { await this.updateWithServer(cipher); } - const encFileName = await this.encryptService.encrypt(filename, cipherEncKey); + const encFileName = await this.encryptService.encryptString(filename, cipherEncKey); const dataEncKey = await this.keyService.makeDataEncKey(cipherEncKey); - const encData = await this.encryptService.encryptToBytes(new Uint8Array(data), dataEncKey[0]); + const encData = await this.encryptService.encryptFileData(new Uint8Array(data), dataEncKey[0]); const response = await this.cipherFileUploadService.upload( cipher, @@ -1490,7 +1488,7 @@ export class CipherService implements CipherServiceAbstraction { const encBuf = await EncArrayBuffer.fromResponse(attachmentResponse); const activeUserId = await firstValueFrom(this.accountService.activeAccount$); const userKey = await this.keyService.getUserKeyWithLegacySupport(activeUserId.id); - const decBuf = await this.encryptService.decryptToBytes(encBuf, userKey); + const decBuf = await this.encryptService.decryptFileData(encBuf, userKey); let encKey: UserKey | OrgKey; encKey = await this.keyService.getOrgKey(organizationId); @@ -1498,8 +1496,11 @@ export class CipherService implements CipherServiceAbstraction { const dataEncKey = await this.keyService.makeDataEncKey(encKey); - const encFileName = await this.encryptService.encrypt(attachmentView.fileName, encKey); - const encData = await this.encryptService.encryptToBytes(new Uint8Array(decBuf), dataEncKey[0]); + const encFileName = await this.encryptService.encryptString(attachmentView.fileName, encKey); + const encData = await this.encryptService.encryptFileData( + new Uint8Array(decBuf), + dataEncKey[0], + ); const fd = new FormData(); try { @@ -1554,7 +1555,7 @@ export class CipherService implements CipherServiceAbstraction { .then(() => { const modelProp = (model as any)[map[theProp] || theProp]; if (modelProp && modelProp !== "") { - return self.encryptService.encrypt(modelProp, key); + return self.encryptService.encryptString(modelProp, key); } return null; }) @@ -1600,7 +1601,7 @@ export class CipherService implements CipherServiceAbstraction { key, ); const uriHash = await this.encryptService.hash(model.login.uris[i].uri, "sha256"); - loginUri.uriChecksum = await this.encryptService.encrypt(uriHash, key); + loginUri.uriChecksum = await this.encryptService.encryptString(uriHash, key); cipher.login.uris.push(loginUri); } } @@ -1627,8 +1628,11 @@ export class CipherService implements CipherServiceAbstraction { }, key, ); - domainKey.counter = await this.encryptService.encrypt(String(viewKey.counter), key); - domainKey.discoverable = await this.encryptService.encrypt( + domainKey.counter = await this.encryptService.encryptString( + String(viewKey.counter), + key, + ); + domainKey.discoverable = await this.encryptService.encryptString( String(viewKey.discoverable), key, ); @@ -1814,8 +1818,9 @@ export class CipherService implements CipherServiceAbstraction { if (cipher.key == null) { decryptedCipherKey = await this.keyService.makeCipherKey(); } else { - decryptedCipherKey = new SymmetricCryptoKey( - await this.encryptService.decryptToBytes(cipher.key, keyForCipherKeyDecryption), + decryptedCipherKey = await this.encryptService.unwrapSymmetricKey( + cipher.key, + keyForCipherKeyDecryption, ); } diff --git a/libs/common/src/vault/services/folder/folder.service.spec.ts b/libs/common/src/vault/services/folder/folder.service.spec.ts index ced4e2dceb..0c61efc288 100644 --- a/libs/common/src/vault/services/folder/folder.service.spec.ts +++ b/libs/common/src/vault/services/folder/folder.service.spec.ts @@ -46,6 +46,7 @@ describe("Folder Service", () => { i18nService.t.mockReturnValue("No Folder"); keyService.userKey$.mockReturnValue(new BehaviorSubject("mockOriginalUserKey" as any)); + encryptService.decryptString.mockResolvedValue("DEC"); encryptService.decryptToUtf8.mockResolvedValue("DEC"); folderService = new FolderService( @@ -110,7 +111,7 @@ describe("Folder Service", () => { model.id = "2"; model.name = "Test Folder"; - encryptService.encrypt.mockResolvedValue(new EncString("ENC")); + encryptService.encryptString.mockResolvedValue(new EncString("ENC")); const result = await folderService.encrypt(model, null); @@ -211,7 +212,7 @@ describe("Folder Service", () => { beforeEach(() => { encryptedKey = new EncString("Re-encrypted Folder"); - encryptService.encrypt.mockResolvedValue(encryptedKey); + encryptService.encryptString.mockResolvedValue(encryptedKey); }); it("returns re-encrypted user folders", async () => { diff --git a/libs/common/src/vault/services/folder/folder.service.ts b/libs/common/src/vault/services/folder/folder.service.ts index 89bc5353f8..ba87f1c314 100644 --- a/libs/common/src/vault/services/folder/folder.service.ts +++ b/libs/common/src/vault/services/folder/folder.service.ts @@ -84,7 +84,7 @@ export class FolderService implements InternalFolderServiceAbstraction { async encrypt(model: FolderView, key: SymmetricCryptoKey): Promise { const folder = new Folder(); folder.id = model.id; - folder.name = await this.encryptService.encrypt(model.name, key); + folder.name = await this.encryptService.encryptString(model.name, key); return folder; } 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 af5828c5ae..8e2ce8ab6b 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 @@ -59,6 +59,7 @@ function setupUserKeyValidation( cipher.key = mockEnc("EncKey"); cipherService.getAll.mockResolvedValue([cipher]); encryptService.decryptToBytes.mockResolvedValue(makeStaticByteArray(64)); + encryptService.decryptString.mockResolvedValue("mockDecryptedString"); (window as any).bitwardenContainerService = new ContainerService(keyService, encryptService); } @@ -280,6 +281,8 @@ describe("regenerateIfNeeded", () => { setupVerificationResponse(mockVerificationResponse, sdkService); setupUserKeyValidation(cipherService, keyService, encryptService); encryptService.decryptToBytes.mockRejectedValue(new Error("error")); + encryptService.decryptString.mockRejectedValue(new Error("error")); + encryptService.unwrapSymmetricKey.mockRejectedValue(new Error("error")); await sut.regenerateIfNeeded(userId); @@ -329,6 +332,8 @@ describe("regenerateIfNeeded", () => { setupVerificationResponse(mockVerificationResponse, sdkService); setupUserKeyValidation(cipherService, keyService, encryptService); encryptService.decryptToBytes.mockRejectedValue(new Error("error")); + encryptService.decryptString.mockRejectedValue(new Error("error")); + encryptService.unwrapSymmetricKey.mockRejectedValue(new Error("error")); await sut.regenerateIfNeeded(userId); diff --git a/libs/vault/src/components/download-attachment/download-attachment.component.ts b/libs/vault/src/components/download-attachment/download-attachment.component.ts index 7db1d47837..d96a40b0f8 100644 --- a/libs/vault/src/components/download-attachment/download-attachment.component.ts +++ b/libs/vault/src/components/download-attachment/download-attachment.component.ts @@ -102,7 +102,7 @@ export class DownloadAttachmentComponent { try { const encBuf = await EncArrayBuffer.fromResponse(response); const key = this.attachment.key != null ? this.attachment.key : this.orgKey; - const decBuf = await this.encryptService.decryptToBytes(encBuf, key); + const decBuf = await this.encryptService.decryptFileData(encBuf, key); this.fileDownloadService.download({ fileName: this.attachment.fileName, blobData: decBuf,