From 7110e3cda65e471543e51d5c6d6161b765f7a7b4 Mon Sep 17 00:00:00 2001 From: Jacob Fink Date: Tue, 13 Jun 2023 15:55:59 -0400 Subject: [PATCH] fix EncString serialization issues & various fixes Co-authored-by: Matt Gibson --- .../background/nativeMessaging.background.ts | 11 ++++++---- .../services/browser-crypto.service.ts | 6 +++--- .../services/electron-crypto.service.ts | 21 +++++++++---------- .../native-message-handler.service.ts | 6 ++++-- .../src/auth/components/lock.component.ts | 3 ++- .../src/platform/models/domain/account.ts | 5 ++--- .../src/platform/models/domain/enc-string.ts | 14 +++++++------ .../src/platform/services/crypto.service.ts | 2 +- .../src/platform/services/state.service.ts | 18 +++++++++------- 9 files changed, 47 insertions(+), 39 deletions(-) diff --git a/apps/browser/src/background/nativeMessaging.background.ts b/apps/browser/src/background/nativeMessaging.background.ts index 90c114a8650..eb3936c192c 100644 --- a/apps/browser/src/background/nativeMessaging.background.ts +++ b/apps/browser/src/background/nativeMessaging.background.ts @@ -10,7 +10,10 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; -import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; +import { + MasterKey, + SymmetricCryptoKey, +} from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { BrowserApi } from "../platform/browser/browser-api"; @@ -320,8 +323,8 @@ export class NativeMessagingBackground { } if (message.response === "unlocked") { - await this.cryptoService.setKey( - new SymmetricCryptoKey(Utils.fromB64ToArray(message.keyB64).buffer) + await this.cryptoService.setMasterKey( + new SymmetricCryptoKey(Utils.fromB64ToArray(message.keyB64).buffer) as MasterKey ); // Verify key is correct by attempting to decrypt a secret @@ -329,7 +332,7 @@ export class NativeMessagingBackground { await this.cryptoService.getFingerprint(await this.stateService.getUserId()); } catch (e) { this.logService.error("Unable to verify key: " + e); - await this.cryptoService.clearKey(); + await this.cryptoService.clearKeys(); this.showWrongUserDialog(); // Exit early diff --git a/apps/browser/src/platform/services/browser-crypto.service.ts b/apps/browser/src/platform/services/browser-crypto.service.ts index 1018c270cb2..75558436b26 100644 --- a/apps/browser/src/platform/services/browser-crypto.service.ts +++ b/apps/browser/src/platform/services/browser-crypto.service.ts @@ -3,11 +3,11 @@ import { CryptoService } from "@bitwarden/common/platform/services/crypto.servic export class BrowserCryptoService extends CryptoService { protected async retrieveKeyFromStorage(keySuffix: KeySuffixOptions) { - if (keySuffix === "biometric") { + if (keySuffix === KeySuffixOptions.Biometric) { await this.platformUtilService.authenticateBiometric(); - return (await this.getKey())?.keyB64; + return (await this.getUserKeyFromMemory())?.keyB64; } - return await super.retrieveKeyFromStorage(keySuffix); + return await super.retrieveUserKeyFromStorage(keySuffix); } } diff --git a/apps/desktop/src/platform/services/electron-crypto.service.ts b/apps/desktop/src/platform/services/electron-crypto.service.ts index e97087fd449..c414b404119 100644 --- a/apps/desktop/src/platform/services/electron-crypto.service.ts +++ b/apps/desktop/src/platform/services/electron-crypto.service.ts @@ -42,16 +42,12 @@ export class ElectronCryptoService extends CryptoService { keySuffix: KeySuffixOptions, userId?: string ): Promise { - const userKey = super.retrieveUserKeyFromStorage(keySuffix, userId); - if (userKey) { - return userKey; - } if (keySuffix === KeySuffixOptions.Biometric) { await this.migrateBiometricKeyIfNeeded(userId); const userKey = await this.stateService.getUserSymKeyBiometric({ userId: userId }); return new SymmetricCryptoKey(Utils.fromB64ToArray(userKey).buffer) as UserSymKey; } - return null; + return await super.retrieveUserKeyFromStorage(keySuffix, userId); } protected async storeBiometricKey(key: UserSymKey, userId?: string): Promise { @@ -86,15 +82,18 @@ export class ElectronCryptoService extends CryptoService { } private async migrateBiometricKeyIfNeeded(userId?: string) { - const oldBiometricKey = await this.stateService.getCryptoMasterKeyBiometric({ userId }); - if (oldBiometricKey) { + if (await this.stateService.hasCryptoMasterKeyBiometric({ userId })) { + const oldBiometricKey = await this.stateService.getCryptoMasterKeyBiometric({ userId }); // decrypt - const masterKey = new SymmetricCryptoKey( - Utils.fromB64ToArray(oldBiometricKey).buffer - ) as MasterKey; + const masterKey = new SymmetricCryptoKey(Utils.fromB64ToArray(oldBiometricKey)) as MasterKey; + let encUserKey = await this.stateService.getEncryptedCryptoSymmetricKey(); + encUserKey = encUserKey ?? (await this.stateService.getUserSymKeyMasterKey()); + if (!encUserKey) { + throw new Error("No user key found during biometric migration"); + } const userSymKey = await this.decryptUserSymKeyWithMasterKey( masterKey, - new EncString(await this.stateService.getEncryptedCryptoSymmetricKey()) + new EncString(encUserKey) ); // migrate await this.storeBiometricKey(userSymKey, userId); diff --git a/apps/desktop/src/services/native-message-handler.service.ts b/apps/desktop/src/services/native-message-handler.service.ts index 1921ea5fa05..c842e698d21 100644 --- a/apps/desktop/src/services/native-message-handler.service.ts +++ b/apps/desktop/src/services/native-message-handler.service.ts @@ -8,7 +8,7 @@ import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.se import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; +import { EncryptedString, EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { StateService } from "@bitwarden/common/platform/services/state.service"; @@ -144,7 +144,9 @@ export class NativeMessageHandlerService { } private async handleEncryptedMessage(message: EncryptedMessage) { - message.encryptedCommand = EncString.fromJSON(message.encryptedCommand.toString()); + message.encryptedCommand = EncString.fromJSON( + message.encryptedCommand.toString() as EncryptedString + ); const decryptedCommandData = await this.decryptPayload(message); const { command } = decryptedCommandData; diff --git a/libs/angular/src/auth/components/lock.component.ts b/libs/angular/src/auth/components/lock.component.ts index a0610a7ff59..2902992c738 100644 --- a/libs/angular/src/auth/components/lock.component.ts +++ b/libs/angular/src/auth/components/lock.component.ts @@ -162,7 +162,8 @@ export class LockComponent implements OnInit, OnDestroy { } else { // MP on restart disabled userSymKeyPin = await this.stateService.getUserSymKeyPin(); - oldPinProtected = new EncString(await this.stateService.getEncryptedPinProtected()); + const oldEncryptedKey = await this.stateService.getEncryptedPinProtected(); + oldPinProtected = oldEncryptedKey ? new EncString(oldEncryptedKey) : undefined; } let userSymKey: UserSymKey; diff --git a/libs/common/src/platform/models/domain/account.ts b/libs/common/src/platform/models/domain/account.ts index 2499d30569a..d68e29d9eab 100644 --- a/libs/common/src/platform/models/domain/account.ts +++ b/libs/common/src/platform/models/domain/account.ts @@ -235,8 +235,8 @@ export class AccountSettings { passwordGenerationOptions?: any; usernameGenerationOptions?: any; generatorOptions?: any; - userSymKeyPin?: EncString; - userSymKeyPinEphemeral?: EncString; + userSymKeyPin?: EncryptedString; + userSymKeyPinEphemeral?: EncryptedString; protectedPin?: string; pinProtected?: EncryptionPair = new EncryptionPair(); // Deprecated settings?: AccountSettingsSettings; // TODO: Merge whatever is going on here into the AccountSettings model properly @@ -256,7 +256,6 @@ export class AccountSettings { return Object.assign(new AccountSettings(), obj, { environmentUrls: EnvironmentUrls.fromJSON(obj?.environmentUrls), - userSymKeyPin: EncString.fromJSON(obj.userSymKeyPin), pinProtected: EncryptionPair.fromJSON( obj?.pinProtected, EncString.fromJSON diff --git a/libs/common/src/platform/models/domain/enc-string.ts b/libs/common/src/platform/models/domain/enc-string.ts index 8fe9fbe93f9..26e5cb28e65 100644 --- a/libs/common/src/platform/models/domain/enc-string.ts +++ b/libs/common/src/platform/models/domain/enc-string.ts @@ -1,4 +1,4 @@ -import { Jsonify } from "type-fest"; +import { Jsonify, Opaque } from "type-fest"; import { EncryptionType, EXPECTED_NUM_PARTS_BY_ENCRYPTION_TYPE } from "../../../enums"; import { Utils } from "../../../platform/misc/utils"; @@ -7,7 +7,7 @@ import { Encrypted } from "../../interfaces/encrypted"; import { SymmetricCryptoKey } from "./symmetric-crypto-key"; export class EncString implements Encrypted { - encryptedString?: string; + encryptedString?: EncryptedString; encryptionType?: EncryptionType; decryptedValue?: string; data?: string; @@ -53,14 +53,14 @@ export class EncString implements Encrypted { private initFromData(encType: EncryptionType, data: string, iv: string, mac: string) { if (iv != null) { - this.encryptedString = encType + "." + iv + "|" + data; + this.encryptedString = (encType + "." + iv + "|" + data) as EncryptedString; } else { - this.encryptedString = encType + "." + data; + this.encryptedString = (encType + "." + data) as EncryptedString; } // mac if (mac != null) { - this.encryptedString += "|" + mac; + this.encryptedString = (this.encryptedString + "|" + mac) as EncryptedString; } this.encryptionType = encType; @@ -70,7 +70,7 @@ export class EncString implements Encrypted { } private initFromEncryptedString(encryptedString: string) { - this.encryptedString = encryptedString as string; + this.encryptedString = encryptedString as EncryptedString; if (!this.encryptedString) { return; } @@ -165,3 +165,5 @@ export class EncString implements Encrypted { : await cryptoService.getKeyForUserEncryption(); } } + +export type EncryptedString = Opaque; diff --git a/libs/common/src/platform/services/crypto.service.ts b/libs/common/src/platform/services/crypto.service.ts index e7d0a24dd82..d7bd999ee75 100644 --- a/libs/common/src/platform/services/crypto.service.ts +++ b/libs/common/src/platform/services/crypto.service.ts @@ -506,7 +506,7 @@ export class CryptoService implements CryptoServiceAbstraction { pinProtectedUserSymKey?: EncString ): Promise { pinProtectedUserSymKey ||= await this.stateService.getUserSymKeyPin(); - if (pinProtectedUserSymKey) { + if (!pinProtectedUserSymKey) { throw new Error("No PIN protected key found."); } const pinKey = await this.makePinKey(pin, salt, kdf, kdfConfig); diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index f729c1ea7de..7f242bdb7d4 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -715,16 +715,17 @@ export class StateService< } async getUserSymKeyPin(options?: StorageOptions): Promise { - return ( - await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions())) - )?.settings?.userSymKeyPin; + return EncString.fromJSON( + (await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) + ?.settings?.userSymKeyPin + ); } async setUserSymKeyPin(value: EncString, options?: StorageOptions): Promise { const account = await this.getAccount( this.reconcileOptions(options, await this.defaultOnDiskOptions()) ); - account.settings.userSymKeyPin = value; + account.settings.userSymKeyPin = value?.encryptedString; await this.saveAccount( account, this.reconcileOptions(options, await this.defaultOnDiskOptions()) @@ -732,16 +733,17 @@ export class StateService< } async getUserSymKeyPinEphemeral(options?: StorageOptions): Promise { - return ( - await this.getAccount(this.reconcileOptions(options, await this.defaultInMemoryOptions())) - )?.settings?.userSymKeyPinEphemeral; + return EncString.fromJSON( + (await this.getAccount(this.reconcileOptions(options, await this.defaultInMemoryOptions()))) + ?.settings?.userSymKeyPinEphemeral + ); } async setUserSymKeyPinEphemeral(value: EncString, options?: StorageOptions): Promise { const account = await this.getAccount( this.reconcileOptions(options, await this.defaultInMemoryOptions()) ); - account.settings.userSymKeyPinEphemeral = value; + account.settings.userSymKeyPinEphemeral = value?.encryptedString; await this.saveAccount( account, this.reconcileOptions(options, await this.defaultInMemoryOptions())