1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-16 08:13:42 +00:00

update pin key when the user symmetric key is set

- always set the protected pin so we can recreate pin key from user symmetric key
- stop using EncryptionPair in account
- use EncString for both pin key storage
- update migration from old strategy on lock component
This commit is contained in:
Jacob Fink
2023-06-07 09:44:34 -04:00
parent 7837202180
commit 6fc7c406a4
14 changed files with 134 additions and 97 deletions

View File

@@ -4,7 +4,10 @@ import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
import {
SymmetricCryptoKey,
UserSymKey,
} from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
import { CryptoService } from "@bitwarden/common/platform/services/crypto.service";
import { CsprngString } from "@bitwarden/common/types/csprng";
@@ -21,7 +24,7 @@ export class ElectronCryptoService extends CryptoService {
super(cryptoFunctionService, encryptService, platformUtilsService, logService, stateService);
}
protected override async storeAdditionalKeys(key: SymmetricCryptoKey, userId?: string) {
protected override async storeAdditionalKeys(key: UserSymKey, userId?: string) {
await super.storeAdditionalKeys(key, userId);
const storeBiometricKey = await this.shouldStoreKey(KeySuffixOptions.Biometric, userId);

View File

@@ -153,15 +153,15 @@ export class LockComponent implements OnInit, OnDestroy {
try {
const kdf = await this.stateService.getKdfType();
const kdfConfig = await this.stateService.getKdfConfig();
let oldPinProtected: EncString;
let userSymKeyPin: EncString;
let oldPinProtected: EncString;
if (this.pinSet[0]) {
// MP on restart enabled
userSymKeyPin = await this.stateService.getDecryptedUserSymKeyPin();
userSymKeyPin = await this.stateService.getUserSymKeyPinEphemeral();
oldPinProtected = await this.stateService.getDecryptedPinProtected();
} else {
// MP on restart disabled
userSymKeyPin = new EncString(await this.stateService.getEncryptedUserSymKeyPin());
userSymKeyPin = await this.stateService.getUserSymKeyPin();
oldPinProtected = new EncString(await this.stateService.getEncryptedPinProtected());
}
@@ -178,14 +178,12 @@ export class LockComponent implements OnInit, OnDestroy {
);
}
if (this.pinSet[0]) {
const protectedPin = await this.stateService.getProtectedPin();
const decryptedPin = await this.cryptoService.decryptToUtf8(
new EncString(protectedPin),
userSymKey
);
failed = decryptedPin !== this.pin;
}
const protectedPin = await this.stateService.getProtectedPin();
const decryptedPin = await this.cryptoService.decryptToUtf8(
new EncString(protectedPin),
userSymKey
);
failed = decryptedPin !== this.pin;
if (!failed) {
await this.setKeyAndContinue(userSymKey);
@@ -317,10 +315,9 @@ export class LockComponent implements OnInit, OnDestroy {
private async load() {
this.pinSet = await this.vaultTimeoutSettingsService.isPinLockSet();
let decryptedPinSet = await this.stateService.getDecryptedUserSymKeyPin();
decryptedPinSet ||= await this.stateService.getDecryptedPinProtected();
// (is MP on Restart enabled && MP already entered) || (Pin is set and MP on Restart disabled)
this.pinLock = (this.pinSet[0] && decryptedPinSet != null) || this.pinSet[1];
let ephemeralPinSet = await this.stateService.getUserSymKeyPinEphemeral();
ephemeralPinSet ||= await this.stateService.getDecryptedPinProtected();
this.pinLock = (this.pinSet[0] && !!ephemeralPinSet) || this.pinSet[1];
this.supportsBiometric = await this.platformUtilsService.supportsBiometric();
this.biometricLock =
@@ -401,10 +398,13 @@ export class LockComponent implements OnInit, OnDestroy {
const pinProtectedKey = await this.cryptoService.encrypt(userSymKey.key, pinKey);
if (masterPasswordOnRestart) {
await this.stateService.setDecryptedPinProtected(null);
await this.stateService.setDecryptedUserSymKeyPin(pinProtectedKey);
await this.stateService.setUserSymKeyPinEphemeral(pinProtectedKey);
} else {
await this.stateService.setEncryptedPinProtected(null);
await this.stateService.setEncryptedUserSymKeyPin(pinProtectedKey.encryptedString);
await this.stateService.setUserSymKeyPin(pinProtectedKey);
// always set the protected pin, even if MP on Restart is disabled
const encPin = await this.cryptoService.encrypt(this.pin, userSymKey);
await this.stateService.setProtectedPin(encPin.encryptedString);
}
return userSymKey;
}

View File

@@ -35,18 +35,20 @@ export class SetPinComponent implements OnInit {
this.modalRef.close(false);
}
const kdf = await this.stateService.getKdfType();
const kdfConfig = await this.stateService.getKdfConfig();
const email = await this.stateService.getEmail();
const pinKey = await this.cryptoService.makePinKey(this.pin, email, kdf, kdfConfig);
const pinKey = await this.cryptoService.makePinKey(
this.pin,
await this.stateService.getEmail(),
await this.stateService.getKdfType(),
await this.stateService.getKdfConfig()
);
const userKey = await this.cryptoService.getUserKeyFromMemory();
const pinProtectedKey = await this.cryptoService.encrypt(userKey.key, pinKey);
const encPin = await this.cryptoService.encrypt(this.pin, userKey);
await this.stateService.setProtectedPin(encPin.encryptedString);
if (this.masterPassOnRestart) {
const encPin = await this.cryptoService.encrypt(this.pin, userKey);
await this.stateService.setProtectedPin(encPin.encryptedString);
await this.stateService.setDecryptedUserSymKeyPin(pinProtectedKey);
await this.stateService.setUserSymKeyPinEphemeral(pinProtectedKey);
} else {
await this.stateService.setEncryptedUserSymKeyPin(pinProtectedKey.encryptedString);
await this.stateService.setUserSymKeyPin(pinProtectedKey);
}
this.modalRef.close(true);

View File

@@ -7,6 +7,10 @@ export abstract class VaultTimeoutSettingsService {
) => Promise<void>;
getVaultTimeout: (userId?: string) => Promise<number>;
getVaultTimeoutAction: (userId?: string) => Promise<VaultTimeoutAction>;
/**
* Has the user enabled unlock with Pin.
* @returns [Pin with MP on Restart enabled, Pin without MP on Restart enabled]
*/
isPinLockSet: () => Promise<[boolean, boolean]>;
isBiometricLockSet: () => Promise<boolean>;
clear: (userId?: string) => Promise<void>;

View File

@@ -132,6 +132,7 @@ export abstract class LogInStrategy {
result.forcePasswordReset = ForceResetPasswordReason.AdminForcePasswordReset;
}
// Must come before setting keys, user key needs email to update additional keys
await this.saveAccountInformation(response);
if (response.twoFactorToken != null) {

View File

@@ -1,4 +1,5 @@
export enum KeySuffixOptions {
Auto = "auto",
Biometric = "biometric",
Pin = "pin",
}

View File

@@ -24,7 +24,6 @@ export abstract class CryptoService {
hasUserKeyStored: (keySuffix?: KeySuffixOptions, userId?: string) => Promise<boolean>;
makeUserSymKey: (key: SymmetricCryptoKey) => Promise<[UserSymKey, EncString]>;
clearUserKey: (clearSecretStorage?: boolean, userId?: string) => Promise<void>;
clearUserKeyFromStorage: (keySuffix: KeySuffixOptions) => Promise<void>;
setUserSymKeyMasterKey: (UserSymKeyMasterKey: string, userId?: string) => Promise<void>;
setMasterKey: (key: MasterKey, userId?: string) => Promise<void>;
getMasterKey: (userId?: string) => Promise<MasterKey>;

View File

@@ -88,25 +88,25 @@ export abstract class StateService<T extends Account = Account> {
hasUserSymKeyBiometric: (options?: StorageOptions) => Promise<boolean>;
setUserSymKeyBiometric: (value: BiometricKey, options?: StorageOptions) => Promise<void>;
/**
* Gets the encrypted version of the user's symmetric key encrypted by the Pin key.
* Gets the user's symmetric key encrypted by the Pin key.
* Used when Master Password on Reset is disabled
*/
getEncryptedUserSymKeyPin: (options?: StorageOptions) => Promise<string>;
getUserSymKeyPin: (options?: StorageOptions) => Promise<EncString>;
/**
* Sets the encrypted version of the user's symmetric key encrypted by the Pin key.
* Sets the user's symmetric key encrypted by the Pin key.
* Used when Master Password on Reset is disabled
*/
setEncryptedUserSymKeyPin: (value: string, options?: StorageOptions) => Promise<void>;
setUserSymKeyPin: (value: EncString, options?: StorageOptions) => Promise<void>;
/**
* Gets the decrypted version of the user's symmetric key encrypted by the Pin key.
* Gets the ephemeral version of the user's symmetric key encrypted by the Pin key.
* Used when Master Password on Reset is enabled
*/
getDecryptedUserSymKeyPin: (options?: StorageOptions) => Promise<EncString>;
getUserSymKeyPinEphemeral: (options?: StorageOptions) => Promise<EncString>;
/**
* Sets the decrypted version of the user's symmetric key encrypted by the Pin key.
* Sets the ephemeral version of the user's symmetric key encrypted by the Pin key.
* Used when Master Password on Reset is enabled
*/
setDecryptedUserSymKeyPin: (value: EncString, options?: StorageOptions) => Promise<void>;
setUserSymKeyPinEphemeral: (value: EncString, options?: StorageOptions) => Promise<void>;
// deprecated keys
/**

View File

@@ -9,16 +9,12 @@ describe("AccountSettings", () => {
it("should deserialize userSymKeyPin", () => {
const accountSettings = new AccountSettings();
accountSettings.userSymKeyPin = EncryptionPair.fromJSON<string, EncString>({
encrypted: "encrypted",
decrypted: "3.data",
});
accountSettings.userSymKeyPin = EncString.fromJSON("encrypted");
const jsonObj = JSON.parse(JSON.stringify(accountSettings));
const actual = AccountSettings.fromJSON(jsonObj);
expect(actual.userSymKeyPin).toBeInstanceOf(EncryptionPair);
expect(actual.userSymKeyPin.encrypted).toEqual("encrypted");
expect(actual.userSymKeyPin.decrypted.encryptedString).toEqual("3.data");
expect(actual.userSymKeyPin).toBeInstanceOf(EncString);
expect(actual.userSymKeyPin.encryptedString).toEqual("encrypted");
});
it("should deserialize pinProtected", () => {

View File

@@ -235,9 +235,10 @@ export class AccountSettings {
passwordGenerationOptions?: any;
usernameGenerationOptions?: any;
generatorOptions?: any;
userSymKeyPin?: EncryptionPair<string, EncString> = new EncryptionPair<string, EncString>();
pinProtected?: EncryptionPair<string, EncString> = new EncryptionPair<string, EncString>(); // Deprecated
userSymKeyPin?: EncString;
userSymKeyPinEphemeral?: EncString;
protectedPin?: string;
pinProtected?: EncryptionPair<string, EncString> = new EncryptionPair<string, EncString>(); // Deprecated
settings?: AccountSettingsSettings; // TODO: Merge whatever is going on here into the AccountSettings model properly
vaultTimeout?: number;
vaultTimeoutAction?: string = "lock";
@@ -255,10 +256,7 @@ export class AccountSettings {
return Object.assign(new AccountSettings(), obj, {
environmentUrls: EnvironmentUrls.fromJSON(obj?.environmentUrls),
userSymKeyPin: EncryptionPair.fromJSON<string, EncString>(
obj?.userSymKeyPin,
EncString.fromJSON
),
userSymKeyPin: EncString.fromJSON(obj.userSymKeyPin),
pinProtected: EncryptionPair.fromJSON<string, EncString>(
obj?.pinProtected,
EncString.fromJSON

View File

@@ -66,8 +66,6 @@ export class CryptoService implements CryptoServiceAbstraction {
*/
async setUserKey(key: UserSymKey, userId?: string): Promise<void> {
await this.stateService.setUserSymKey(key, { userId: userId });
// TODO(Jake): Should we include additional keys here? When we set the memory key from storage,
// it will reset the keys in storage as well
await this.storeAdditionalKeys(key, userId);
}
@@ -88,7 +86,10 @@ export class CryptoService implements CryptoServiceAbstraction {
* @param userId The desired user
* @returns The user's symmetric key
*/
async getUserKeyFromStorage(keySuffix: KeySuffixOptions, userId?: string): Promise<UserSymKey> {
async getUserKeyFromStorage(
keySuffix: KeySuffixOptions.Auto | KeySuffixOptions.Biometric,
userId?: string
): Promise<UserSymKey> {
const userKey = await this.retrieveUserKeyFromStorage(keySuffix, userId);
if (userKey != null) {
if (!(await this.validateUserKey(userKey))) {
@@ -154,27 +155,17 @@ export class CryptoService implements CryptoServiceAbstraction {
/**
* Clears the user's symmetric key
* @param clearSecretStorage Clears all stored versions of the user keys as well,
* @param clearStoredKeys Clears all stored versions of the user keys as well,
* such as the biometrics key
* @param userId The desired user
*/
async clearUserKey(clearSecretStorage = true, userId?: string): Promise<void> {
async clearUserKey(clearStoredKeys = true, userId?: string): Promise<void> {
await this.stateService.setUserSymKey(null, { userId: userId });
if (clearSecretStorage) {
if (clearStoredKeys) {
await this.clearStoredUserKeys(userId);
}
}
/**
* Clears the specified version of the user's symmetric key from storage
* @param keySuffix The desired version of the user's key to clear
*/
async clearUserKeyFromStorage(keySuffix: KeySuffixOptions): Promise<void> {
keySuffix === KeySuffixOptions.Auto
? await this.stateService.setUserSymKeyAuto(null)
: await this.stateService.setUserSymKeyBiometric(null);
}
/**
* Stores the master key encrypted user symmetric key
* @param userSymKeyMasterKey The master key encrypted user symmetric key to set
@@ -681,8 +672,8 @@ export class CryptoService implements CryptoServiceAbstraction {
* @param userId The desired user
*/
async clearPinProtectedKey(userId?: string): Promise<void> {
await this.stateService.setUserSymKeyPin(null, { userId: userId });
await this.stateService.setEncryptedPinProtected(null, { userId: userId });
await this.stateService.setEncryptedUserSymKeyPin(null, { userId: userId });
}
async decryptUserSymKeyWithPin(
@@ -692,12 +683,9 @@ export class CryptoService implements CryptoServiceAbstraction {
kdfConfig: KdfConfig,
pinProtectedUserSymKey?: EncString
): Promise<UserSymKey> {
if (!pinProtectedUserSymKey) {
const pinProtectedUserSymKeyString = await this.stateService.getEncryptedUserSymKeyPin();
if (pinProtectedUserSymKeyString == null) {
throw new Error("No PIN protected key found.");
}
pinProtectedUserSymKey = new EncString(pinProtectedUserSymKeyString);
pinProtectedUserSymKey ||= await this.stateService.getUserSymKeyPin();
if (pinProtectedUserSymKey) {
throw new Error("No PIN protected key found.");
}
const pinKey = await this.makePinKey(pin, salt, kdf, kdfConfig);
const userSymKey = await this.decryptToBytes(pinProtectedUserSymKey, pinKey);
@@ -886,30 +874,67 @@ export class CryptoService implements CryptoServiceAbstraction {
return true;
}
protected async storeAdditionalKeys(key: SymmetricCryptoKey, userId?: string) {
/**
* Regenerates any additional keys if needed. Useful to make sure
* other keys stay in sync when the user's symmetric key has been rotated.
* @param key The user's symmetric key
* @param userId The desired user
*/
protected async storeAdditionalKeys(key: UserSymKey, userId?: string) {
const storeAuto = await this.shouldStoreKey(KeySuffixOptions.Auto, userId);
if (storeAuto) {
await this.stateService.setUserSymKeyAuto(key.keyB64, { userId: userId });
} else {
await this.stateService.setUserSymKeyAuto(null, { userId: userId });
}
const storePin = await this.shouldStoreKey(KeySuffixOptions.Pin, userId);
if (storePin) {
await this.storePinKey(key);
} else {
await this.stateService.setUserSymKeyPin(null, { userId: userId });
}
}
protected async storePinKey(key: UserSymKey) {
const email = await this.stateService.getEmail();
const kdf = await this.stateService.getKdfType();
const kdfConfig = await this.stateService.getKdfConfig();
const pin = await this.decryptToUtf8(
new EncString(await this.stateService.getProtectedPin()),
key
);
const pinKey = await this.makePinKey(pin, email, kdf, kdfConfig);
await this.stateService.setUserSymKeyPin(await this.encrypt(key.key, pinKey));
}
protected async shouldStoreKey(keySuffix: KeySuffixOptions, userId?: string) {
let shouldStoreKey = false;
if (keySuffix === KeySuffixOptions.Auto) {
const vaultTimeout = await this.stateService.getVaultTimeout({ userId: userId });
shouldStoreKey = vaultTimeout == null;
} else if (keySuffix === KeySuffixOptions.Biometric) {
const biometricUnlock = await this.stateService.getBiometricUnlock({ userId: userId });
shouldStoreKey = biometricUnlock && this.platformUtilService.supportsSecureStorage();
switch (keySuffix) {
case KeySuffixOptions.Auto: {
const vaultTimeout = await this.stateService.getVaultTimeout({ userId: userId });
shouldStoreKey = vaultTimeout == null;
break;
}
case KeySuffixOptions.Biometric: {
const biometricUnlock = await this.stateService.getBiometricUnlock({ userId: userId });
shouldStoreKey = biometricUnlock && this.platformUtilService.supportsSecureStorage();
break;
}
case KeySuffixOptions.Pin: {
const protectedPin = await this.stateService.getProtectedPin();
// This could cause a possible timing issue. Need to make sure the ephemeral key is set before
// we set our user key
const userSymKeyPinEphemeral = await this.stateService.getUserSymKeyPinEphemeral();
shouldStoreKey = !!protectedPin && !userSymKeyPinEphemeral;
break;
}
}
return shouldStoreKey;
}
protected async retrieveUserKeyFromStorage(
keySuffix: KeySuffixOptions,
keySuffix: KeySuffixOptions.Auto | KeySuffixOptions.Biometric,
userId?: string
): Promise<UserSymKey> {
let userKey: string;
@@ -969,6 +994,7 @@ export class CryptoService implements CryptoServiceAbstraction {
private async clearStoredUserKeys(userId?: string): Promise<void> {
await this.stateService.setUserSymKeyAuto(null, { userId: userId });
await this.stateService.setUserSymKeyBiometric(null, { userId: userId });
await this.stateService.setUserSymKeyPinEphemeral(null, { userId: userId });
}
async makeKey(

View File

@@ -720,34 +720,34 @@ export class StateService<
await this.saveSecureStorageKey(partialKeys.userBiometricKey, value, options);
}
async getEncryptedUserSymKeyPin(options?: StorageOptions): Promise<string> {
async getUserSymKeyPin(options?: StorageOptions): Promise<EncString> {
return (
await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions()))
)?.settings?.userSymKeyPin?.encrypted;
)?.settings?.userSymKeyPin;
}
async setEncryptedUserSymKeyPin(value: string, options?: StorageOptions): Promise<void> {
async setUserSymKeyPin(value: EncString, options?: StorageOptions): Promise<void> {
const account = await this.getAccount(
this.reconcileOptions(options, await this.defaultOnDiskOptions())
);
account.settings.userSymKeyPin.encrypted = value;
account.settings.userSymKeyPin = value;
await this.saveAccount(
account,
this.reconcileOptions(options, await this.defaultOnDiskOptions())
);
}
async getDecryptedUserSymKeyPin(options?: StorageOptions): Promise<EncString> {
async getUserSymKeyPinEphemeral(options?: StorageOptions): Promise<EncString> {
return (
await this.getAccount(this.reconcileOptions(options, await this.defaultInMemoryOptions()))
)?.settings?.userSymKeyPin?.decrypted;
)?.settings?.userSymKeyPinEphemeral;
}
async setDecryptedUserSymKeyPin(value: EncString, options?: StorageOptions): Promise<void> {
async setUserSymKeyPinEphemeral(value: EncString, options?: StorageOptions): Promise<void> {
const account = await this.getAccount(
this.reconcileOptions(options, await this.defaultInMemoryOptions())
);
account.settings.userSymKeyPin.decrypted = value;
account.settings.userSymKeyPinEphemeral = value;
await this.saveAccount(
account,
this.reconcileOptions(options, await this.defaultInMemoryOptions())

View File

@@ -71,8 +71,10 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction {
if (await this.keyConnectorService.getUsesKeyConnector()) {
const pinSet = await this.vaultTimeoutSettingsService.isPinLockSet();
const pinLock =
(pinSet[0] && (await this.stateService.getDecryptedPinProtected()) != null) || pinSet[1];
let ephemeralPinSet = await this.stateService.getUserSymKeyPinEphemeral();
ephemeralPinSet ||= await this.stateService.getDecryptedPinProtected();
const pinLock = (pinSet[0] && ephemeralPinSet != null) || pinSet[1];
if (!pinLock && !(await this.vaultTimeoutSettingsService.isBiometricLockSet())) {
await this.logOut(userId);

View File

@@ -45,11 +45,16 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA
}
async isPinLockSet(): Promise<[boolean, boolean]> {
const protectedPin = await this.stateService.getProtectedPin();
let pinProtectedKey = await this.stateService.getEncryptedUserSymKeyPin();
pinProtectedKey ||= await this.stateService.getEncryptedPinProtected();
// we can't check the protected pin for both because old accounts only
// used it for MP on Restart
const pinIsEnabled = !!(await this.stateService.getProtectedPin());
const aUserSymKeyPinIsSet = !!(await this.stateService.getUserSymKeyPin());
const anOldUserSymKeyPinIsSet = !!(await this.stateService.getEncryptedPinProtected());
return [protectedPin != null, pinProtectedKey != null];
return [
pinIsEnabled && !aUserSymKeyPinIsSet && !anOldUserSymKeyPinIsSet,
aUserSymKeyPinIsSet || anOldUserSymKeyPinIsSet,
];
}
async isBiometricLockSet(): Promise<boolean> {
@@ -106,8 +111,8 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA
async clear(userId?: string): Promise<void> {
await this.stateService.setEverBeenUnlocked(false, { userId: userId });
await this.stateService.setDecryptedPinProtected(null, { userId: userId });
await this.stateService.setDecryptedUserSymKeyPin(null, { userId: userId });
await this.stateService.setUserSymKeyPinEphemeral(null, { userId: userId });
await this.stateService.setProtectedPin(null, { userId: userId });
await this.stateService.setDecryptedPinProtected(null, { userId: userId });
}
}