bitwarden/jslib
1
0
Fork 0
mirror of https://github.com/bitwarden/jslib synced 2025-12-26 05:03:41 +00:00
Code Issues Releases Wiki Activity

[bug] Remove keySuffix storage option and split uses into unique methods

Browse Source 
The keySuffix options don't work with saving serialized json as a storage object - use cases simply overwrite each other in state.
This commit breaks Auto and Biometric keys into distinct storage items and adjusts logic accordingly.
This commit is contained in:
addison
2021-11-16 09:56:01 -05:00
parent 2d74334f61
commit 4e65a5ac4f
7 changed files with 50 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
common/src/abstractions/state.service.ts
View File

@@ -65,8 +65,12 @@ export abstract class StateService {
setConvertAccountToKeyConnector: (value: boolean, options?: StorageOptions) => Promise<void>;
getCryptoMasterKey: (options?: StorageOptions) => Promise<SymmetricCryptoKey>;
setCryptoMasterKey: (value: SymmetricCryptoKey, options?: StorageOptions) => Promise<void>;
getCryptoMasterKeyAuto: (options?: StorageOptions) => Promise<SymmetricCryptoKey>;
setCryptoMasterKeyAuto: (value: SymmetricCryptoKey, options?: StorageOptions) => Promise<void>;
getCryptoMasterKeyB64: (options: StorageOptions) => Promise<string>;
setCryptoMasterKeyB64: (value: string, options: StorageOptions) => Promise<void>;
getCryptoMasterKeyBiometric: (options?: StorageOptions) => Promise<SymmetricCryptoKey>;
setCryptoMasterKeyBiometric: (value: SymmetricCryptoKey, options?: StorageOptions) => Promise<void>;
getDecodedToken: (options?: StorageOptions) => Promise<any>;
setDecodedToken: (value: any, options?: StorageOptions) => Promise<void>;
getDecryptedCiphers: (options?: StorageOptions) => Promise<CipherView[]>;

2
common/src/models/domain/account.ts
View File

@@ -34,6 +34,8 @@ export class Account {
encryptedCiphers: { [id: string]: CipherData };
decryptedCiphers: CipherView[];
cryptoMasterKey: SymmetricCryptoKey;
cryptoMasterKeyBiometric: SymmetricCryptoKey;
cryptoMasterKeyAuto: SymmetricCryptoKey;
cryptoMasterKeyB64: string;
encryptedCryptoSymmetricKey: string;
decryptedCryptoSymmetricKey: SymmetricCryptoKey;

2
common/src/models/domain/storageOptions.ts
View File

@@ -1,9 +1,7 @@
import { HtmlStorageLocation } from '../../enums/htmlStorageLocation';
import { KeySuffixOptions } from '../../enums/keySuffixOptions';
import { StorageLocation } from '../../enums/storageLocation';
export type StorageOptions = {
keySuffix?: KeySuffixOptions;
storageLocation?: StorageLocation;
useSecureStorage?: boolean;
userId?: string;

29
common/src/services/crypto.service.ts
View File

@@ -84,10 +84,7 @@ export class CryptoService implements CryptoServiceAbstraction {
}
async getKey(keySuffix?: KeySuffixOptions, userId?: string): Promise<SymmetricCryptoKey> {
const inMemoryKey = await this.stateService.getCryptoMasterKey(
userId ? { userId: userId } :
null
);
const inMemoryKey = await this.stateService.getCryptoMasterKey({ userId: userId });
if (inMemoryKey != null) {
return inMemoryKey;
@@ -106,7 +103,7 @@ export class CryptoService implements CryptoServiceAbstraction {
async getKeyFromStorage(keySuffix: KeySuffixOptions, userId?: string): Promise<SymmetricCryptoKey> {
const key = await this.retrieveKeyFromStorage(keySuffix, userId);
if (key != null) {
const symmetricKey = new SymmetricCryptoKey(Utils.fromB64ToArray(key).buffer);
const symmetricKey = new SymmetricCryptoKey(Utils.fromB64ToArray(key.keyB64).buffer);
if (!await this.validateKey(symmetricKey)) {
this.logService.warning('Wrong key, throwing away stored key');
@@ -323,7 +320,11 @@ export class CryptoService implements CryptoServiceAbstraction {
}
async hasKeyStored(keySuffix: KeySuffixOptions, userId?: string): Promise<boolean> {
return await this.stateService.getCryptoMasterKeyB64({ keySuffix: keySuffix, userId: userId }) != null;
const key = keySuffix === KeySuffixOptions.Auto ?
await this.stateService.getCryptoMasterKeyAuto({ userId: userId }) :
await this.stateService.getCryptoMasterKeyBiometric({ userId: userId });
return key != null;
}
async hasEncKey(): Promise<boolean> {
@@ -339,7 +340,9 @@ export class CryptoService implements CryptoServiceAbstraction {
}
async clearStoredKey(keySuffix: KeySuffixOptions) {
await this.stateService.setCryptoMasterKeyB64(null, { keySuffix: keySuffix });
keySuffix === KeySuffixOptions.Auto ?
await this.stateService.setCryptoMasterKeyAuto(null) :
await this.stateService.setCryptoMasterKeyBiometric(null);
}
async clearKeyHash(): Promise<any> {
@@ -706,7 +709,9 @@ export class CryptoService implements CryptoServiceAbstraction {
}
protected async retrieveKeyFromStorage(keySuffix: KeySuffixOptions, userId?: string) {
return await this.stateService.getCryptoMasterKeyB64({ keySuffix: keySuffix, userId: userId });
return keySuffix === KeySuffixOptions.Auto ?
await this.stateService.getCryptoMasterKeyAuto({ userId: userId }) :
await this.stateService.getCryptoMasterKeyBiometric({ userId: userId });
}
private async aesEncrypt(data: ArrayBuffer, key: SymmetricCryptoKey): Promise<EncryptedObject> {
@@ -866,16 +871,16 @@ export class CryptoService implements CryptoServiceAbstraction {
}
private async clearSecretKeyStore(userId?: string): Promise<void> {
await this.stateService.setCryptoMasterKeyB64(null, { keySuffix: KeySuffixOptions.Auto, userId: userId });
await this.stateService.setCryptoMasterKeyB64(null, { keySuffix: KeySuffixOptions.Biometric, userId: userId });
await this.stateService.setCryptoMasterKeyAuto(null, { userId: userId });
await this.stateService.setCryptoMasterKeyBiometric(null, { userId: userId });
}
private async storeKey(key: SymmetricCryptoKey, userId?: string) {
const shouldStoreAuto = await this.shouldStoreKey(KeySuffixOptions.Auto, userId);
await this.stateService.setCryptoMasterKeyB64(shouldStoreAuto ? key.keyB64 : null, { userId: userId, keySuffix: KeySuffixOptions.Auto});
await this.stateService.setCryptoMasterKeyAuto(shouldStoreAuto ? key : null, { userId: userId });
const shouldStoreBiometric = await this.shouldStoreKey(KeySuffixOptions.Biometric, userId);
await this.stateService.setCryptoMasterKeyB64(shouldStoreBiometric ? key.keyB64 : null, { userId: userId, keySuffix: KeySuffixOptions.Biometric });
await this.stateService.setCryptoMasterKeyBiometric(shouldStoreBiometric ? key : null, { userId: userId });
}
}

43
common/src/services/state.service.ts
View File

@@ -254,34 +254,32 @@ export class StateService implements StateServiceAbstraction {
}
async getCryptoMasterKey(options?: StorageOptions): Promise<SymmetricCryptoKey> {
options = this.reconcileOptions(options, { userId: await this.getActiveUserIdFromStorage()});
return (await this.getAccount(this.reconcileOptions(options, this.defaultInMemoryOptions)))?.cryptoMasterKey;
}
async setCryptoMasterKey(value: SymmetricCryptoKey, options?: StorageOptions): Promise<void> {
options = this.reconcileOptions(options, { userId: await this.getActiveUserIdFromStorage()});
const account = await this.getAccount(this.reconcileOptions(options, this.defaultInMemoryOptions));
account.cryptoMasterKey = value;
await this.saveAccount(account, this.reconcileOptions(options, this.defaultInMemoryOptions));
}
async getCryptoMasterKeyAuto(options?: StorageOptions): Promise<SymmetricCryptoKey> {
return (await this.getAccount(this.reconcileOptions(options, await this.defaultSecureStorageOptions())))?.cryptoMasterKeyAuto;
}
async setCryptoMasterKeyAuto(value: SymmetricCryptoKey, options?: StorageOptions): Promise<void> {
const account = await this.getAccount(this.reconcileOptions(options, await this.defaultSecureStorageOptions()));
account.cryptoMasterKeyAuto = value;
await this.saveAccount(account, this.reconcileOptions(options, await this.defaultSecureStorageOptions()));
}
async getCryptoMasterKeyB64(options: StorageOptions): Promise<string> {
try {
if (options?.keySuffix == null) {
throw new RequiredSuffixError();
}
const value = (await this.getAccount(this.reconcileOptions(options, await this.defaultSecureStorageOptions())))?.cryptoMasterKeyB64;
return value;
} catch (e) {
this.logService.error(e);
}
const value = (await this.getAccount(this.reconcileOptions(options, await this.defaultSecureStorageOptions())))?.cryptoMasterKeyB64;
return value;
}
async setCryptoMasterKeyB64(value: string, options: StorageOptions): Promise<void> {
try {
if (value != null && options?.keySuffix == null) {
throw new RequiredSuffixError();
}
const account = await this.getAccount(this.reconcileOptions(options, await this.defaultSecureStorageOptions()));
if (account != null) {
account.cryptoMasterKeyB64 = value;
@@ -292,6 +290,16 @@ export class StateService implements StateServiceAbstraction {
}
}
async getCryptoMasterKeyBiometric(options?: StorageOptions): Promise<SymmetricCryptoKey> {
return (await this.getAccount(this.reconcileOptions(options, await this.defaultSecureStorageOptions())))?.cryptoMasterKeyBiometric;
}
async setCryptoMasterKeyBiometric(value: SymmetricCryptoKey, options?: StorageOptions): Promise<void> {
const account = await this.getAccount(this.reconcileOptions(options, await this.defaultSecureStorageOptions()));
account.cryptoMasterKeyBiometric = value;
await this.saveAccount(account, this.reconcileOptions(options, await this.defaultSecureStorageOptions()));
}
async getDecodedToken(options?: StorageOptions): Promise<any> {
return (await this.getAccount(this.reconcileOptions(options, this.defaultInMemoryOptions)))?.decodedToken;
}
@@ -1332,7 +1340,6 @@ export class StateService implements StateServiceAbstraction {
}
requestedOptions.userId = requestedOptions?.userId ?? defaultOptions.userId;
requestedOptions.storageLocation = requestedOptions?.storageLocation ?? defaultOptions.storageLocation;
requestedOptions.keySuffix = requestedOptions?.keySuffix ?? defaultOptions.keySuffix;
requestedOptions.useSecureStorage = requestedOptions?.useSecureStorage ?? defaultOptions.useSecureStorage;
requestedOptions.htmlStorageLocation = requestedOptions?.htmlStorageLocation ?? defaultOptions.htmlStorageLocation;
return requestedOptions;
@@ -1382,9 +1389,3 @@ export class StateService implements StateServiceAbstraction {
return state?.activeUserId;
}
}
class RequiredSuffixError extends Error {
constructor(public message: string = 'The suffix option is required to get/set this key.') {
super(message);
}
}

9
electron/src/services/electronCrypto.service.ts
View File

@@ -6,6 +6,7 @@ import { StateService } from 'jslib-common/abstractions/state.service';
import { CryptoService } from 'jslib-common/services/crypto.service';
import { KeySuffixOptions } from 'jslib-common/enums/keySuffixOptions';
import { StorageLocation } from 'jslib-common/enums/storageLocation';
export class ElectronCryptoService extends CryptoService {
@@ -30,7 +31,7 @@ export class ElectronCryptoService extends CryptoService {
*/
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.stateService.getCryptoMasterKey();
const key = await this.stateService.getCryptoMasterKey({ storageLocation: StorageLocation.Disk, useSecureStorage: true });
if (key == null) {
return;
@@ -38,16 +39,16 @@ export class ElectronCryptoService extends CryptoService {
try {
if (await this.shouldStoreKey(KeySuffixOptions.Auto)) {
await this.stateService.getCryptoMasterKeyB64({ keySuffix: KeySuffixOptions.Auto });
await this.stateService.setCryptoMasterKeyAuto(key);
}
if (await this.shouldStoreKey(KeySuffixOptions.Biometric)) {
await this.stateService.getCryptoMasterKey({ keySuffix: KeySuffixOptions.Biometric });
await this.stateService.setCryptoMasterKeyBiometric(key);
}
} catch (e) {
this.logService.error(`Encountered error while upgrading obsolete Bitwarden secure storage item:`);
this.logService.error(e);
}
await this.stateService.setCryptoMasterKey(null);
await this.stateService.setCryptoMasterKey(null, { storageLocation: StorageLocation.Disk, useSecureStorage: true });
}
}

4
electron/src/services/electronRendererSecureStorage.service.ts
View File

@@ -9,7 +9,6 @@ export class ElectronRendererSecureStorageService implements StorageService {
const val = ipcRenderer.sendSync('keytar', {
action: 'getPassword',
key: key,
keySuffix: options?.keySuffix ?? '',
});
return Promise.resolve(val != null ? JSON.parse(val) as T : null);
}
@@ -18,7 +17,6 @@ export class ElectronRendererSecureStorageService implements StorageService {
const val = ipcRenderer.sendSync('keytar', {
action: 'hasPassword',
key: key,
keySuffix: options?.keySuffix ?? '',
});
return Promise.resolve(!!val);
}
@@ -27,7 +25,6 @@ export class ElectronRendererSecureStorageService implements StorageService {
ipcRenderer.sendSync('keytar', {
action: 'setPassword',
key: key,
keySuffix: options?.keySuffix ?? '',
value: JSON.stringify(obj),
});
return Promise.resolve();
@@ -37,7 +34,6 @@ export class ElectronRendererSecureStorageService implements StorageService {
ipcRenderer.sendSync('keytar', {
action: 'deletePassword',
key: key,
keySuffix: options?.keySuffix ?? '',
});
return Promise.resolve();
}