From bb3080bbfe7a9638e980cd3d5cfac8f8d7f61417 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 26 Nov 2025 11:41:11 +0100 Subject: [PATCH] Migrate to sdk rsa extract public key --- .../src/abstractions/key.service.ts | 14 ---- libs/key-management/src/key.service.spec.ts | 65 --------------- libs/key-management/src/key.service.ts | 83 +++++++------------ package-lock.json | 16 ++-- package.json | 4 +- 5 files changed, 40 insertions(+), 142 deletions(-) diff --git a/libs/key-management/src/abstractions/key.service.ts b/libs/key-management/src/abstractions/key.service.ts index f86ca922c9e..1c1a2ab63f1 100644 --- a/libs/key-management/src/abstractions/key.service.ts +++ b/libs/key-management/src/abstractions/key.service.ts @@ -68,20 +68,6 @@ export abstract class KeyService { * @param userId The desired user */ abstract setUserKey(key: UserKey, userId: UserId): Promise; - /** - * Sets the provided user keys and stores any other necessary versions - * (such as auto, biometrics, or pin). - * Also sets the user's encrypted private key in storage and - * clears the decrypted private key from memory - * Note: does not clear the private key if null is provided - * - * @throws Error when userKey, encPrivateKey or userId is null - * @throws UserPrivateKeyDecryptionFailedError when the userKey cannot decrypt encPrivateKey - * @param userKey The user key to set - * @param encPrivateKey An encrypted private key - * @param userId The desired user - */ - abstract setUserKeys(userKey: UserKey, encPrivateKey: string, userId: UserId): Promise; /** * Gets the user key from memory and sets it again, * kicking off a refresh of any additional keys diff --git a/libs/key-management/src/key.service.spec.ts b/libs/key-management/src/key.service.spec.ts index a5b4eb01c7c..7b2377c588b 100644 --- a/libs/key-management/src/key.service.spec.ts +++ b/libs/key-management/src/key.service.spec.ts @@ -49,7 +49,6 @@ import { } from "@bitwarden/common/types/key"; import { KdfConfigService } from "./abstractions/kdf-config.service"; -import { UserPrivateKeyDecryptionFailedError } from "./abstractions/key.service"; import { DefaultKeyService } from "./key.service"; import { KdfConfig } from "./models/kdf-config"; @@ -255,70 +254,6 @@ describe("keyService", () => { }); }); - describe("setUserKeys", () => { - let mockUserKey: UserKey; - let mockEncPrivateKey: EncryptedString; - let everHadUserKeyState: FakeSingleUserState; - - beforeEach(() => { - const mockRandomBytes = new Uint8Array(64) as CsprngArray; - mockUserKey = new SymmetricCryptoKey(mockRandomBytes) as UserKey; - mockEncPrivateKey = new SymmetricCryptoKey(mockRandomBytes).toString() as EncryptedString; - everHadUserKeyState = stateProvider.singleUser.getFake(mockUserId, USER_EVER_HAD_USER_KEY); - - // Initialize storage - everHadUserKeyState.nextState(null); - - // Mock private key decryption - encryptService.unwrapDecapsulationKey.mockResolvedValue(mockRandomBytes); - }); - - it("throws if userKey is null", async () => { - await expect( - keyService.setUserKeys(null as unknown as UserKey, mockEncPrivateKey, mockUserId), - ).rejects.toThrow("No userKey provided."); - }); - - it("throws if encPrivateKey is null", async () => { - await expect( - keyService.setUserKeys(mockUserKey, null as unknown as EncryptedString, mockUserId), - ).rejects.toThrow("No encPrivateKey provided."); - }); - - it("throws if userId is null", async () => { - await expect( - keyService.setUserKeys(mockUserKey, mockEncPrivateKey, null as unknown as UserId), - ).rejects.toThrow("No userId provided."); - }); - - it("throws if encPrivateKey cannot be decrypted with the userKey", async () => { - encryptService.unwrapDecapsulationKey.mockResolvedValue(null); - - await expect( - keyService.setUserKeys(mockUserKey, mockEncPrivateKey, mockUserId), - ).rejects.toThrow(UserPrivateKeyDecryptionFailedError); - }); - - // We already have tests for setUserKey, so we just need to test that the correct methods are called - it("calls setUserKey with the userKey and userId", async () => { - const setUserKeySpy = jest.spyOn(keyService, "setUserKey"); - - await keyService.setUserKeys(mockUserKey, mockEncPrivateKey, mockUserId); - - expect(setUserKeySpy).toHaveBeenCalledWith(mockUserKey, mockUserId); - }); - - // We already have tests for setPrivateKey, so we just need to test that the correct methods are called - // TODO: Move those tests into here since `setPrivateKey` will be converted to a private method - it("calls setPrivateKey with the encPrivateKey and userId", async () => { - const setEncryptedPrivateKeySpy = jest.spyOn(keyService, "setPrivateKey"); - - await keyService.setUserKeys(mockUserKey, mockEncPrivateKey, mockUserId); - - expect(setEncryptedPrivateKeySpy).toHaveBeenCalledWith(mockEncPrivateKey, mockUserId); - }); - }); - describe("makeSendKey", () => { const mockRandomBytes = new Uint8Array(16) as CsprngArray; it("calls keyGenerationService with expected hard coded parameters", async () => { diff --git a/libs/key-management/src/key.service.ts b/libs/key-management/src/key.service.ts index 2701109fde2..6aee9e18240 100644 --- a/libs/key-management/src/key.service.ts +++ b/libs/key-management/src/key.service.ts @@ -3,6 +3,7 @@ import { NEVER, Observable, combineLatest, + concatMap, distinctUntilChanged, filter, firstValueFrom, @@ -32,6 +33,7 @@ import { VaultTimeoutStringType } from "@bitwarden/common/key-management/vault-t import { VAULT_TIMEOUT } from "@bitwarden/common/key-management/vault-timeout/services/vault-timeout-settings.state"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { SdkLoadService } from "@bitwarden/common/platform/abstractions/sdk/sdk-load.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { KeySuffixOptions, HashPurpose, EncryptionType } from "@bitwarden/common/platform/enums"; import { convertValues } from "@bitwarden/common/platform/misc/convert-values"; @@ -58,12 +60,12 @@ import { UserPrivateKey, UserPublicKey, } from "@bitwarden/common/types/key"; +import { PureCrypto } from "@bitwarden/sdk-internal"; import { KdfConfigService } from "./abstractions/kdf-config.service"; import { CipherDecryptionKeys, KeyService as KeyServiceAbstraction, - UserPrivateKeyDecryptionFailedError, } from "./abstractions/key.service"; import { KdfConfig } from "./models/kdf-config"; @@ -119,30 +121,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { } } - async setUserKeys( - userKey: UserKey, - encPrivateKey: EncryptedString, - userId: UserId, - ): Promise { - if (userKey == null) { - throw new Error("No userKey provided. Lock the user to clear the key"); - } - if (encPrivateKey == null) { - throw new Error("No encPrivateKey provided."); - } - if (userId == null) { - throw new Error("No userId provided."); - } - - const decryptedPrivateKey = await this.decryptPrivateKey(encPrivateKey, userKey); - if (decryptedPrivateKey == null) { - throw new UserPrivateKeyDecryptionFailedError(); - } - - await this.setUserKey(userKey, userId); - await this.setPrivateKey(encPrivateKey, userId); - } - async refreshAdditionalKeys(userId: UserId): Promise { if (userId == null) { throw new Error("UserId is required."); @@ -610,7 +588,8 @@ export class DefaultKeyService implements KeyServiceAbstraction { } // Can successfully derive public key - const publicKey = await this.derivePublicKey(privateKey); + await SdkLoadService.Ready; + const publicKey = PureCrypto.rsa_extract_public_key(encPrivateKey, key.toEncoded()); if (publicKey == null) { // failed to decrypt @@ -764,34 +743,30 @@ export class DefaultKeyService implements KeyServiceAbstraction { } userPublicKey$(userId: UserId) { - return this.userPrivateKey$(userId).pipe( - switchMap(async (pk) => await this.derivePublicKey(pk)), - ); - } - - private async derivePublicKey(privateKey: UserPrivateKey | null) { - if (privateKey == null) { - return null; - } - - return await this.cryptoFunctionService.rsaExtractPublicKey(privateKey); + return this.userEncryptionKeyPair$(userId).pipe(map((keyPair) => keyPair?.publicKey ?? null)); } userPrivateKey$(userId: UserId): Observable { - return this.userPrivateKeyHelper$(userId).pipe(map((keys) => keys?.userPrivateKey ?? null)); + return this.userEncryptionKeyPair$(userId).pipe(map((keyPair) => keyPair?.privateKey ?? null)); } userEncryptionKeyPair$( userId: UserId, ): Observable<{ privateKey: UserPrivateKey; publicKey: UserPublicKey } | null> { - return this.userPrivateKey$(userId).pipe( - switchMap(async (privateKey) => { - if (privateKey == null) { + return combineLatest([this.userPrivateKeyHelper$(userId)]).pipe( + concatMap(async ([privateKeyInfo]) => { + if (privateKeyInfo == null || privateKeyInfo.userPrivateKey == null) { return null; } - const publicKey = (await this.derivePublicKey(privateKey))! as UserPublicKey; - return { privateKey, publicKey }; + await SdkLoadService.Ready; + return { + privateKey: privateKeyInfo.userPrivateKey, + publicKey: PureCrypto.rsa_extract_public_key( + privateKeyInfo.encryptedPrivateKey, + privateKeyInfo.userKey.toEncoded(), + ) as UserPublicKey, + }; }), ); } @@ -811,16 +786,20 @@ export class DefaultKeyService implements KeyServiceAbstraction { return this.stateProvider.getUser(userId, USER_ENCRYPTED_PRIVATE_KEY).state$.pipe( switchMap(async (encryptedPrivateKey) => { try { - return await this.decryptPrivateKey(encryptedPrivateKey, userKey); + return { + privateKey: await this.decryptPrivateKey(encryptedPrivateKey, userKey), + encryptedPrivateKey, + }; } catch (e) { this.logService.error("Failed to decrypt private key for user ", userId, e); throw e; } }), // Combine outerscope info with user private key - map((userPrivateKey) => ({ + map(({ privateKey, encryptedPrivateKey }) => ({ userKey, - userPrivateKey, + userPrivateKey: privateKey, + encryptedPrivateKey, })), ); }), @@ -901,20 +880,18 @@ export class DefaultKeyService implements KeyServiceAbstraction { } encryptedOrgKeys$(userId: UserId): Observable> { - return this.userPrivateKey$(userId)?.pipe( - switchMap((userPrivateKey) => { - if (userPrivateKey == null) { + return this.userEncryptionKeyPair$(userId)?.pipe( + switchMap((userPublicKeyEncryptionKeyPair) => { + if (userPublicKeyEncryptionKeyPair == null) { // We can't do any org based decryption return of({}); } return combineLatest([ this.stateProvider.getUser(userId, USER_ENCRYPTED_ORGANIZATION_KEYS).state$, - this.providerKeysHelper$(userId, userPrivateKey), + this.providerKeysHelper$(userId, userPublicKeyEncryptionKeyPair.privateKey), ]).pipe( switchMap(async ([encryptedOrgKeys, providerKeys]) => { - const userPubKey = await this.derivePublicKey(userPrivateKey); - const result: Record = {}; encryptedOrgKeys = encryptedOrgKeys ?? {}; for (const orgId of Object.keys(encryptedOrgKeys) as OrganizationId[]) { @@ -937,7 +914,7 @@ export class DefaultKeyService implements KeyServiceAbstraction { } orgKey = await this.encryptService.encapsulateKeyUnsigned( await encrypted.decrypt(this.encryptService, providerKeys!), - userPubKey!, + userPublicKeyEncryptionKeyPair.publicKey, ); } else { orgKey = encrypted.encryptedOrganizationKey; diff --git a/package-lock.json b/package-lock.json index 005eb38d80b..1cb67d601ed 100644 --- a/package-lock.json +++ b/package-lock.json @@ -23,8 +23,8 @@ "@angular/platform-browser": "19.2.14", "@angular/platform-browser-dynamic": "19.2.14", "@angular/router": "19.2.14", - "@bitwarden/commercial-sdk-internal": "0.2.0-main.375", - "@bitwarden/sdk-internal": "0.2.0-main.375", + "@bitwarden/commercial-sdk-internal": "0.2.0-main.398", + "@bitwarden/sdk-internal": "0.2.0-main.398", "@electron/fuses": "1.8.0", "@emotion/css": "11.13.5", "@koa/multer": "4.0.0", @@ -4620,9 +4620,9 @@ "link": true }, "node_modules/@bitwarden/commercial-sdk-internal": { - "version": "0.2.0-main.375", - "resolved": "https://registry.npmjs.org/@bitwarden/commercial-sdk-internal/-/commercial-sdk-internal-0.2.0-main.375.tgz", - "integrity": "sha512-UMVfLjMh79+5et1if7qqOi+pSGP5Ay3AcGp4E5oLZ0p0yFsN2Q54UFv+SLju0/oI0qTvVZP1RkEtTJXHdNrpTg==", + "version": "0.2.0-main.398", + "resolved": "https://registry.npmjs.org/@bitwarden/commercial-sdk-internal/-/commercial-sdk-internal-0.2.0-main.398.tgz", + "integrity": "sha512-G6LqiIwwTSlCoHuvqPPTHW7Ii1oesh+ReRPDvos0I8H3pavNx54EiuwNVVcrqlolPaMfn4HPp+UCRJs+Dexm5A==", "license": "BITWARDEN SOFTWARE DEVELOPMENT KIT LICENSE AGREEMENT", "dependencies": { "type-fest": "^4.41.0" @@ -4725,9 +4725,9 @@ "link": true }, "node_modules/@bitwarden/sdk-internal": { - "version": "0.2.0-main.375", - "resolved": "https://registry.npmjs.org/@bitwarden/sdk-internal/-/sdk-internal-0.2.0-main.375.tgz", - "integrity": "sha512-kf2SKFkAdSmV2/ORo6u1eegwYW2ha62NHUsx2ij2uPWmm7mzXUoNa7z8mqhJV1ozg5o7yBqBuXd6Wqo9Ww+/RA==", + "version": "0.2.0-main.398", + "resolved": "https://registry.npmjs.org/@bitwarden/sdk-internal/-/sdk-internal-0.2.0-main.398.tgz", + "integrity": "sha512-yKFpiDT+4hLEA1NPqZIrZiBextAYMPVh3Ce3lv80owA9+PT4OiLouz36DX/5HYmEGiPUruYVZFUTOEvEkz0xFw==", "license": "GPL-3.0", "dependencies": { "type-fest": "^4.41.0" diff --git a/package.json b/package.json index 7ca866b3c4d..957a2fe5c1c 100644 --- a/package.json +++ b/package.json @@ -160,8 +160,8 @@ "@angular/platform-browser": "19.2.14", "@angular/platform-browser-dynamic": "19.2.14", "@angular/router": "19.2.14", - "@bitwarden/sdk-internal": "0.2.0-main.375", - "@bitwarden/commercial-sdk-internal": "0.2.0-main.375", + "@bitwarden/sdk-internal": "0.2.0-main.398", + "@bitwarden/commercial-sdk-internal": "0.2.0-main.398", "@electron/fuses": "1.8.0", "@emotion/css": "11.13.5", "@koa/multer": "4.0.0",