From 1f83c3c1ba14d2cdbc319ac060a08cc7c0911b5c Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 15 Jun 2021 09:55:57 -0500 Subject: [PATCH] Fix separate key storage for non desktop (#409) * Handle non-desktop, non-split key storage * Reset vaultTimeoutService on clear. Fixes issues where unlock was required after login * Specify electron as desktop client * Use ElelectronCryptoService to handle desktop-specific tasks * Linter fixes --- common/src/services/crypto.service.ts | 68 +++++-------------- common/src/services/vaultTimeout.service.ts | 2 + .../src/services/electronCrypto.service.ts | 66 ++++++++++++++++++ 3 files changed, 86 insertions(+), 50 deletions(-) create mode 100644 electron/src/services/electronCrypto.service.ts diff --git a/common/src/services/crypto.service.ts b/common/src/services/crypto.service.ts index 7e106d69c..50ab100a7 100644 --- a/common/src/services/crypto.service.ts +++ b/common/src/services/crypto.service.ts @@ -25,7 +25,7 @@ import { sequentialize } from '../misc/sequentialize'; import { Utils } from '../misc/utils'; import { EEFLongWordList } from '../misc/wordlist'; -const Keys = { +export const Keys = { key: 'key', // Master Key encOrgKeys: 'encOrgKeys', encPrivateKey: 'encPrivateKey', @@ -42,25 +42,15 @@ export class CryptoService implements CryptoServiceAbstraction { private privateKey: ArrayBuffer; private orgKeys: Map; - constructor(private storageService: StorageService, private secureStorageService: StorageService, - private cryptoFunctionService: CryptoFunctionService, private platformUtilService: PlatformUtilsService, - private logService: LogService) { + constructor(private storageService: StorageService, protected secureStorageService: StorageService, + private cryptoFunctionService: CryptoFunctionService, protected platformUtilService: PlatformUtilsService, + protected logService: LogService) { } async setKey(key: SymmetricCryptoKey): Promise { this.key = key; - if (await this.shouldStoreKey('auto')) { - await this.secureStorageService.save(Keys.key, key.keyB64, { keySuffix: 'auto' }); - } else { - this.clearStoredKey('auto'); - } - - if (await this.shouldStoreKey('biometric')) { - await this.secureStorageService.save(Keys.key, key.keyB64, { keySuffix: 'biometric' }); - } else { - this.clearStoredKey('biometric'); - } + await this.storeKey(key); } setKeyHash(keyHash: string): Promise<{}> { @@ -288,9 +278,8 @@ export class CryptoService implements CryptoServiceAbstraction { return this.key != null; } - async hasKeyStored(keySuffix: KeySuffixOptions): Promise { - await this.upgradeSecurelyStoredKey(); - return await this.secureStorageService.has(Keys.key, { keySuffix: keySuffix }); + hasKeyStored(keySuffix: KeySuffixOptions): Promise { + return this.secureStorageService.has(Keys.key, { keySuffix: keySuffix }); } async hasEncKey(): Promise { @@ -651,7 +640,15 @@ export class CryptoService implements CryptoServiceAbstraction { // Helpers - private async shouldStoreKey(keySuffix: KeySuffixOptions) { + protected async storeKey(key: SymmetricCryptoKey) { + if (await this.shouldStoreKey('auto') || await this.shouldStoreKey('biometric')) { + this.secureStorageService.save(Keys.key, key.keyB64); + } else { + this.secureStorageService.remove(Keys.key); + } + } + + protected async shouldStoreKey(keySuffix: KeySuffixOptions) { let shouldStoreKey = false; if (keySuffix === 'auto') { const vaultTimeout = await this.storageService.get(ConstantsService.vaultTimeoutKey); @@ -663,37 +660,8 @@ export class CryptoService implements CryptoServiceAbstraction { return shouldStoreKey; } - private async retrieveKeyFromStorage(keySuffix: KeySuffixOptions) { - await this.upgradeSecurelyStoredKey(); - - return await this.secureStorageService.get(Keys.key, { keySuffix: keySuffix }); - } - - /** - * @deprecated 4 Jun 2021 This is temporary upgrade method to move from a single shared stored key to - * multiple, unique stored keys for each use, e.g. never logout vs. biometric authentication. - */ - private async upgradeSecurelyStoredKey() { - // attempt key upgrade, but if we fail just delete it. Keys will be stored property upon unlock anyway. - const key = await this.secureStorageService.get(Keys.key); - - if (key == null) { - return; - } - - try { - if (await this.shouldStoreKey('auto')) { - await this.secureStorageService.save(Keys.key, key, { keySuffix: 'auto' }); - } - if (await this.shouldStoreKey('biometric')) { - await this.secureStorageService.save(Keys.key, key, { keySuffix: 'biometric' }); - } - } catch (e) { - this.logService.error(`Encountered error while upgrading obsolete Bitwarden secure storage item:`); - this.logService.error(e); - } - - await this.secureStorageService.remove(Keys.key); + protected retrieveKeyFromStorage(keySuffix: KeySuffixOptions) { + return this.secureStorageService.get(Keys.key, { keySuffix: keySuffix }); } private async aesEncrypt(data: ArrayBuffer, key: SymmetricCryptoKey): Promise { diff --git a/common/src/services/vaultTimeout.service.ts b/common/src/services/vaultTimeout.service.ts index 328ba0682..1a6602429 100644 --- a/common/src/services/vaultTimeout.service.ts +++ b/common/src/services/vaultTimeout.service.ts @@ -148,6 +148,8 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { } clear(): Promise { + this.everBeenUnlocked = false; + this.manuallyOrTimerLocked = false; this.pinProtectedKey = null; return this.storageService.remove(ConstantsService.protectedPin); } diff --git a/electron/src/services/electronCrypto.service.ts b/electron/src/services/electronCrypto.service.ts new file mode 100644 index 000000000..dc602e0fb --- /dev/null +++ b/electron/src/services/electronCrypto.service.ts @@ -0,0 +1,66 @@ +import { CryptoFunctionService } from 'jslib-common/abstractions/cryptoFunction.service'; +import { LogService } from 'jslib-common/abstractions/log.service'; +import { PlatformUtilsService } from 'jslib-common/abstractions/platformUtils.service'; +import { KeySuffixOptions, StorageService } from 'jslib-common/abstractions/storage.service'; +import { SymmetricCryptoKey } from 'jslib-common/models/domain/symmetricCryptoKey'; +import { CryptoService, Keys } from 'jslib-common/services/crypto.service'; + +export class ElectronCryptoService extends CryptoService { + + constructor(storageService: StorageService, secureStorageService: StorageService, + cryptoFunctionService: CryptoFunctionService, platformUtilService: PlatformUtilsService, + logService: LogService) { + super(storageService, secureStorageService, cryptoFunctionService, platformUtilService, logService); + } + + async hasKeyStored(keySuffix: KeySuffixOptions): Promise { + await this.upgradeSecurelyStoredKey(); + return super.hasKeyStored(keySuffix); + } + + protected async storeKey(key: SymmetricCryptoKey) { + if (await this.shouldStoreKey('auto')) { + await this.secureStorageService.save(Keys.key, key.keyB64, { keySuffix: 'auto' }); + } else { + this.clearStoredKey('auto'); + } + + if (await this.shouldStoreKey('biometric')) { + await this.secureStorageService.save(Keys.key, key.keyB64, { keySuffix: 'biometric' }); + } else { + this.clearStoredKey('biometric'); + } + } + + protected async retrieveKeyFromStorage(keySuffix: KeySuffixOptions) { + await this.upgradeSecurelyStoredKey(); + return super.retrieveKeyFromStorage(keySuffix); + } + + /** + * @deprecated 4 Jun 2021 This is temporary upgrade method to move from a single shared stored key to + * multiple, unique stored keys for each use, e.g. never logout vs. biometric authentication. + */ + private async upgradeSecurelyStoredKey() { + // attempt key upgrade, but if we fail just delete it. Keys will be stored property upon unlock anyway. + const key = await this.secureStorageService.get(Keys.key); + + if (key == null) { + return; + } + + try { + if (await this.shouldStoreKey('auto')) { + await this.secureStorageService.save(Keys.key, key, { keySuffix: 'auto' }); + } + if (await this.shouldStoreKey('biometric')) { + await this.secureStorageService.save(Keys.key, key, { keySuffix: 'biometric' }); + } + } catch (e) { + this.logService.error(`Encountered error while upgrading obsolete Bitwarden secure storage item:`); + this.logService.error(e); + } + + await this.secureStorageService.remove(Keys.key); + } +}