1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-10 21:33:27 +00:00

[PM-3475] Remove deprecated keys (#13266)

* Remove deprecated keys

* Fix cli build

* Fix build
This commit is contained in:
Bernd Schoolmann
2025-03-31 16:58:02 +02:00
committed by GitHub
parent 0311681803
commit 22039d038d
14 changed files with 32 additions and 427 deletions

View File

@@ -655,9 +655,7 @@ export default class MainBackground {
this.kdfConfigService, this.kdfConfigService,
this.keyGenerationService, this.keyGenerationService,
this.logService, this.logService,
this.masterPasswordService,
this.stateProvider, this.stateProvider,
this.stateService,
); );
this.keyService = new DefaultKeyService( this.keyService = new DefaultKeyService(

View File

@@ -436,9 +436,7 @@ export class ServiceContainer {
this.kdfConfigService, this.kdfConfigService,
this.keyGenerationService, this.keyGenerationService,
this.logService, this.logService,
this.masterPasswordService,
this.stateProvider, this.stateProvider,
this.stateService,
); );
this.keyService = new KeyService( this.keyService = new KeyService(

View File

@@ -1173,9 +1173,7 @@ const safeProviders: SafeProvider[] = [
KdfConfigService, KdfConfigService,
KeyGenerationServiceAbstraction, KeyGenerationServiceAbstraction,
LogService, LogService,
MasterPasswordServiceAbstraction,
StateProvider, StateProvider,
StateServiceAbstraction,
], ],
}), }),
safeProvider({ safeProvider({

View File

@@ -1,4 +1,4 @@
import { EncString, EncryptedString } from "@bitwarden/common/platform/models/domain/enc-string"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string";
import { UserId } from "@bitwarden/common/types/guid"; import { UserId } from "@bitwarden/common/types/guid";
import { PinKey, UserKey } from "@bitwarden/common/types/key"; import { PinKey, UserKey } from "@bitwarden/common/types/key";
import { KdfConfig } from "@bitwarden/key-management"; import { KdfConfig } from "@bitwarden/key-management";
@@ -90,17 +90,6 @@ export abstract class PinServiceAbstraction {
*/ */
abstract clearUserKeyEncryptedPin(userId: UserId): Promise<void>; abstract clearUserKeyEncryptedPin(userId: UserId): Promise<void>;
/**
* Gets the old MasterKey, encrypted by the PinKey (formerly called `pinProtected`).
* Deprecated and used for migration purposes only.
*/
abstract getOldPinKeyEncryptedMasterKey: (userId: UserId) => Promise<EncryptedString | null>;
/**
* Clears the old MasterKey, encrypted by the PinKey.
*/
abstract clearOldPinKeyEncryptedMasterKey: (userId: UserId) => Promise<void>;
/** /**
* Makes a PinKey from the provided PIN. * Makes a PinKey from the provided PIN.
*/ */

View File

@@ -4,11 +4,9 @@ import { firstValueFrom, map } from "rxjs";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
import { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction";
import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service"; import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service";
import { KeyGenerationService } from "@bitwarden/common/platform/abstractions/key-generation.service"; import { KeyGenerationService } from "@bitwarden/common/platform/abstractions/key-generation.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
import { EncString, EncryptedString } from "@bitwarden/common/platform/models/domain/enc-string"; import { EncString, EncryptedString } from "@bitwarden/common/platform/models/domain/enc-string";
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
import { import {
@@ -18,7 +16,7 @@ import {
UserKeyDefinition, UserKeyDefinition,
} from "@bitwarden/common/platform/state"; } from "@bitwarden/common/platform/state";
import { UserId } from "@bitwarden/common/types/guid"; import { UserId } from "@bitwarden/common/types/guid";
import { MasterKey, PinKey, UserKey } from "@bitwarden/common/types/key"; import { PinKey, UserKey } from "@bitwarden/common/types/key";
import { KdfConfig, KdfConfigService } from "@bitwarden/key-management"; import { KdfConfig, KdfConfigService } from "@bitwarden/key-management";
import { PinServiceAbstraction } from "../../abstractions/pin.service.abstraction"; import { PinServiceAbstraction } from "../../abstractions/pin.service.abstraction";
@@ -73,19 +71,6 @@ export const USER_KEY_ENCRYPTED_PIN = new UserKeyDefinition<EncryptedString>(
}, },
); );
/**
* The old MasterKey, encrypted by the PinKey (formerly called `pinProtected`).
* Deprecated and used for migration purposes only.
*/
export const OLD_PIN_KEY_ENCRYPTED_MASTER_KEY = new UserKeyDefinition<EncryptedString>(
PIN_DISK,
"oldPinKeyEncryptedMasterKey",
{
deserializer: (jsonValue) => jsonValue,
clearOn: ["logout"],
},
);
export class PinService implements PinServiceAbstraction { export class PinService implements PinServiceAbstraction {
constructor( constructor(
private accountService: AccountService, private accountService: AccountService,
@@ -94,9 +79,7 @@ export class PinService implements PinServiceAbstraction {
private kdfConfigService: KdfConfigService, private kdfConfigService: KdfConfigService,
private keyGenerationService: KeyGenerationService, private keyGenerationService: KeyGenerationService,
private logService: LogService, private logService: LogService,
private masterPasswordService: MasterPasswordServiceAbstraction,
private stateProvider: StateProvider, private stateProvider: StateProvider,
private stateService: StateService,
) {} ) {}
async getPinKeyEncryptedUserKeyPersistent(userId: UserId): Promise<EncString | null> { async getPinKeyEncryptedUserKeyPersistent(userId: UserId): Promise<EncString | null> {
@@ -190,9 +173,7 @@ export class PinService implements PinServiceAbstraction {
this.accountService.accounts$.pipe(map((accounts) => accounts[userId].email)), this.accountService.accounts$.pipe(map((accounts) => accounts[userId].email)),
); );
const kdfConfig = await this.kdfConfigService.getKdfConfig(); const kdfConfig = await this.kdfConfigService.getKdfConfig();
const pinKey = await this.makePinKey(pin, email, kdfConfig); const pinKey = await this.makePinKey(pin, email, kdfConfig);
return await this.encryptService.encrypt(userKey.key, pinKey); return await this.encryptService.encrypt(userKey.key, pinKey);
} }
@@ -242,20 +223,6 @@ export class PinService implements PinServiceAbstraction {
return await this.encryptService.encrypt(pin, userKey); return await this.encryptService.encrypt(pin, userKey);
} }
async getOldPinKeyEncryptedMasterKey(userId: UserId): Promise<EncryptedString | null> {
this.validateUserId(userId, "Cannot get oldPinKeyEncryptedMasterKey.");
return await firstValueFrom(
this.stateProvider.getUserState$(OLD_PIN_KEY_ENCRYPTED_MASTER_KEY, userId),
);
}
async clearOldPinKeyEncryptedMasterKey(userId: UserId): Promise<void> {
this.validateUserId(userId, "Cannot clear oldPinKeyEncryptedMasterKey.");
await this.stateProvider.setUserState(OLD_PIN_KEY_ENCRYPTED_MASTER_KEY, null, userId);
}
async makePinKey(pin: string, salt: string, kdfConfig: KdfConfig): Promise<PinKey> { async makePinKey(pin: string, salt: string, kdfConfig: KdfConfig): Promise<PinKey> {
const pinKey = await this.keyGenerationService.deriveKeyFromPassword(pin, salt, kdfConfig); const pinKey = await this.keyGenerationService.deriveKeyFromPassword(pin, salt, kdfConfig);
return (await this.keyGenerationService.stretchKey(pinKey)) as PinKey; return (await this.keyGenerationService.stretchKey(pinKey)) as PinKey;
@@ -264,23 +231,13 @@ export class PinService implements PinServiceAbstraction {
async getPinLockType(userId: UserId): Promise<PinLockType> { async getPinLockType(userId: UserId): Promise<PinLockType> {
this.validateUserId(userId, "Cannot get PinLockType."); this.validateUserId(userId, "Cannot get PinLockType.");
/**
* We can't check the `userKeyEncryptedPin` (formerly called `protectedPin`) for both because old
* accounts only used it for MP on Restart
*/
const aUserKeyEncryptedPinIsSet = !!(await this.getUserKeyEncryptedPin(userId)); const aUserKeyEncryptedPinIsSet = !!(await this.getUserKeyEncryptedPin(userId));
const aPinKeyEncryptedUserKeyPersistentIsSet = const aPinKeyEncryptedUserKeyPersistentIsSet =
!!(await this.getPinKeyEncryptedUserKeyPersistent(userId)); !!(await this.getPinKeyEncryptedUserKeyPersistent(userId));
const anOldPinKeyEncryptedMasterKeyIsSet =
!!(await this.getOldPinKeyEncryptedMasterKey(userId));
if (aPinKeyEncryptedUserKeyPersistentIsSet || anOldPinKeyEncryptedMasterKeyIsSet) { if (aPinKeyEncryptedUserKeyPersistentIsSet) {
return "PERSISTENT"; return "PERSISTENT";
} else if ( } else if (aUserKeyEncryptedPinIsSet && !aPinKeyEncryptedUserKeyPersistentIsSet) {
aUserKeyEncryptedPinIsSet &&
!aPinKeyEncryptedUserKeyPersistentIsSet &&
!anOldPinKeyEncryptedMasterKeyIsSet
) {
return "EPHEMERAL"; return "EPHEMERAL";
} else { } else {
return "DISABLED"; return "DISABLED";
@@ -302,7 +259,7 @@ export class PinService implements PinServiceAbstraction {
case "DISABLED": case "DISABLED":
return false; return false;
case "PERSISTENT": case "PERSISTENT":
// The above getPinLockType call ensures that we have either a PinKeyEncryptedUserKey or OldPinKeyEncryptedMasterKey set. // The above getPinLockType call ensures that we have either a PinKeyEncryptedUserKey set.
return true; return true;
case "EPHEMERAL": { case "EPHEMERAL": {
// The above getPinLockType call ensures that we have a UserKeyEncryptedPin set. // The above getPinLockType call ensures that we have a UserKeyEncryptedPin set.
@@ -326,31 +283,21 @@ export class PinService implements PinServiceAbstraction {
try { try {
const pinLockType = await this.getPinLockType(userId); const pinLockType = await this.getPinLockType(userId);
const requireMasterPasswordOnClientRestart = pinLockType === "EPHEMERAL";
const { pinKeyEncryptedUserKey, oldPinKeyEncryptedMasterKey } = const pinKeyEncryptedUserKey = await this.getPinKeyEncryptedKeys(pinLockType, userId);
await this.getPinKeyEncryptedKeys(pinLockType, userId);
const email = await firstValueFrom( const email = await firstValueFrom(
this.accountService.accounts$.pipe(map((accounts) => accounts[userId].email)), this.accountService.accounts$.pipe(map((accounts) => accounts[userId].email)),
); );
const kdfConfig = await this.kdfConfigService.getKdfConfig(); const kdfConfig = await this.kdfConfigService.getKdfConfig();
let userKey: UserKey; const userKey: UserKey = await this.decryptUserKey(
userId,
if (oldPinKeyEncryptedMasterKey) { pin,
userKey = await this.decryptAndMigrateOldPinKeyEncryptedMasterKey( email,
userId, kdfConfig,
pin, pinKeyEncryptedUserKey,
email, );
kdfConfig,
requireMasterPasswordOnClientRestart,
oldPinKeyEncryptedMasterKey,
);
} else {
userKey = await this.decryptUserKey(userId, pin, email, kdfConfig, pinKeyEncryptedUserKey);
}
if (!userKey) { if (!userKey) {
this.logService.warning(`User key null after pin key decryption.`); this.logService.warning(`User key null after pin key decryption.`);
return null; return null;
@@ -394,109 +341,23 @@ export class PinService implements PinServiceAbstraction {
} }
/** /**
* Creates a new `pinKeyEncryptedUserKey` and clears the `oldPinKeyEncryptedMasterKey`. * Gets the user's `pinKeyEncryptedUserKey` (persistent or ephemeral)
* @returns UserKey
*/
private async decryptAndMigrateOldPinKeyEncryptedMasterKey(
userId: UserId,
pin: string,
email: string,
kdfConfig: KdfConfig,
requireMasterPasswordOnClientRestart: boolean,
oldPinKeyEncryptedMasterKey: EncString,
): Promise<UserKey> {
this.validateUserId(userId, "Cannot decrypt and migrate oldPinKeyEncryptedMasterKey.");
const masterKey = await this.decryptMasterKeyWithPin(
userId,
pin,
email,
kdfConfig,
oldPinKeyEncryptedMasterKey,
);
const encUserKey = await this.stateService.getEncryptedCryptoSymmetricKey({ userId: userId });
const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(
masterKey,
userId,
encUserKey ? new EncString(encUserKey) : undefined,
);
const pinKeyEncryptedUserKey = await this.createPinKeyEncryptedUserKey(pin, userKey, userId);
await this.storePinKeyEncryptedUserKey(
pinKeyEncryptedUserKey,
requireMasterPasswordOnClientRestart,
userId,
);
const userKeyEncryptedPin = await this.createUserKeyEncryptedPin(pin, userKey);
await this.setUserKeyEncryptedPin(userKeyEncryptedPin, userId);
await this.clearOldPinKeyEncryptedMasterKey(userId);
return userKey;
}
// Only for migration purposes
private async decryptMasterKeyWithPin(
userId: UserId,
pin: string,
salt: string,
kdfConfig: KdfConfig,
oldPinKeyEncryptedMasterKey?: EncString,
): Promise<MasterKey> {
this.validateUserId(userId, "Cannot decrypt master key with PIN.");
if (!oldPinKeyEncryptedMasterKey) {
const oldPinKeyEncryptedMasterKeyString = await this.getOldPinKeyEncryptedMasterKey(userId);
if (oldPinKeyEncryptedMasterKeyString == null) {
throw new Error("No oldPinKeyEncrytedMasterKey found.");
}
oldPinKeyEncryptedMasterKey = new EncString(oldPinKeyEncryptedMasterKeyString);
}
const pinKey = await this.makePinKey(pin, salt, kdfConfig);
const masterKey = await this.encryptService.decryptToBytes(oldPinKeyEncryptedMasterKey, pinKey);
return new SymmetricCryptoKey(masterKey) as MasterKey;
}
/**
* Gets the user's `pinKeyEncryptedUserKey` (persistent or ephemeral) and `oldPinKeyEncryptedMasterKey`
* (if one exists) based on the user's PinLockType. * (if one exists) based on the user's PinLockType.
* *
* @remarks The `oldPinKeyEncryptedMasterKey` (formerly `pinProtected`) is only used for migration and
* will be null for all migrated accounts.
* @throws If PinLockType is 'DISABLED' or if userId is not provided * @throws If PinLockType is 'DISABLED' or if userId is not provided
*/ */
private async getPinKeyEncryptedKeys( private async getPinKeyEncryptedKeys(
pinLockType: PinLockType, pinLockType: PinLockType,
userId: UserId, userId: UserId,
): Promise<{ pinKeyEncryptedUserKey: EncString; oldPinKeyEncryptedMasterKey?: EncString }> { ): Promise<EncString> {
this.validateUserId(userId, "Cannot get PinKey encrypted keys."); this.validateUserId(userId, "Cannot get PinKey encrypted keys.");
switch (pinLockType) { switch (pinLockType) {
case "PERSISTENT": { case "PERSISTENT": {
const pinKeyEncryptedUserKey = await this.getPinKeyEncryptedUserKeyPersistent(userId); return await this.getPinKeyEncryptedUserKeyPersistent(userId);
const oldPinKeyEncryptedMasterKey = await this.getOldPinKeyEncryptedMasterKey(userId);
return {
pinKeyEncryptedUserKey,
oldPinKeyEncryptedMasterKey: oldPinKeyEncryptedMasterKey
? new EncString(oldPinKeyEncryptedMasterKey)
: undefined,
};
} }
case "EPHEMERAL": { case "EPHEMERAL": {
const pinKeyEncryptedUserKey = await this.getPinKeyEncryptedUserKeyEphemeral(userId); return await this.getPinKeyEncryptedUserKeyEphemeral(userId);
return {
pinKeyEncryptedUserKey,
oldPinKeyEncryptedMasterKey: undefined, // Going forward, we only migrate non-ephemeral version
};
} }
case "DISABLED": case "DISABLED":
throw new Error("Pin is disabled"); throw new Error("Pin is disabled");

View File

@@ -1,11 +1,9 @@
import { mock } from "jest-mock-extended"; import { mock } from "jest-mock-extended";
import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
import { FakeMasterPasswordService } from "@bitwarden/common/key-management/master-password/services/fake-master-password.service";
import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service"; import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service";
import { KeyGenerationService } from "@bitwarden/common/platform/abstractions/key-generation.service"; import { KeyGenerationService } from "@bitwarden/common/platform/abstractions/key-generation.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
import { Utils } from "@bitwarden/common/platform/misc/utils"; import { Utils } from "@bitwarden/common/platform/misc/utils";
import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string";
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
@@ -15,14 +13,13 @@ import {
mockAccountServiceWith, mockAccountServiceWith,
} from "@bitwarden/common/spec"; } from "@bitwarden/common/spec";
import { UserId } from "@bitwarden/common/types/guid"; import { UserId } from "@bitwarden/common/types/guid";
import { MasterKey, PinKey, UserKey } from "@bitwarden/common/types/key"; import { PinKey, UserKey } from "@bitwarden/common/types/key";
import { DEFAULT_KDF_CONFIG, KdfConfigService } from "@bitwarden/key-management"; import { DEFAULT_KDF_CONFIG, KdfConfigService } from "@bitwarden/key-management";
import { import {
PinService, PinService,
PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT,
PIN_KEY_ENCRYPTED_USER_KEY_EPHEMERAL, PIN_KEY_ENCRYPTED_USER_KEY_EPHEMERAL,
OLD_PIN_KEY_ENCRYPTED_MASTER_KEY,
USER_KEY_ENCRYPTED_PIN, USER_KEY_ENCRYPTED_PIN,
PinLockType, PinLockType,
} from "./pin.service.implementation"; } from "./pin.service.implementation";
@@ -31,7 +28,6 @@ describe("PinService", () => {
let sut: PinService; let sut: PinService;
let accountService: FakeAccountService; let accountService: FakeAccountService;
let masterPasswordService: FakeMasterPasswordService;
let stateProvider: FakeStateProvider; let stateProvider: FakeStateProvider;
const cryptoFunctionService = mock<CryptoFunctionService>(); const cryptoFunctionService = mock<CryptoFunctionService>();
@@ -39,11 +35,9 @@ describe("PinService", () => {
const kdfConfigService = mock<KdfConfigService>(); const kdfConfigService = mock<KdfConfigService>();
const keyGenerationService = mock<KeyGenerationService>(); const keyGenerationService = mock<KeyGenerationService>();
const logService = mock<LogService>(); const logService = mock<LogService>();
const stateService = mock<StateService>();
const mockUserId = Utils.newGuid() as UserId; const mockUserId = Utils.newGuid() as UserId;
const mockUserKey = new SymmetricCryptoKey(randomBytes(64)) as UserKey; const mockUserKey = new SymmetricCryptoKey(randomBytes(64)) as UserKey;
const mockMasterKey = new SymmetricCryptoKey(randomBytes(32)) as MasterKey;
const mockPinKey = new SymmetricCryptoKey(randomBytes(32)) as PinKey; const mockPinKey = new SymmetricCryptoKey(randomBytes(32)) as PinKey;
const mockUserEmail = "user@example.com"; const mockUserEmail = "user@example.com";
const mockPin = "1234"; const mockPin = "1234";
@@ -57,15 +51,10 @@ describe("PinService", () => {
"2.fb5kOEZvh9zPABbP8WRmSQ==|Yi6ZAJY+UtqCKMUSqp1ahY9Kf8QuneKXs6BMkpNsakLVOzTYkHHlilyGABMF7GzUO8QHyZi7V/Ovjjg+Naf3Sm8qNhxtDhibITv4k8rDnM0=|TFkq3h2VNTT1z5BFbebm37WYuxyEHXuRo0DZJI7TQnw=", "2.fb5kOEZvh9zPABbP8WRmSQ==|Yi6ZAJY+UtqCKMUSqp1ahY9Kf8QuneKXs6BMkpNsakLVOzTYkHHlilyGABMF7GzUO8QHyZi7V/Ovjjg+Naf3Sm8qNhxtDhibITv4k8rDnM0=|TFkq3h2VNTT1z5BFbebm37WYuxyEHXuRo0DZJI7TQnw=",
); );
const oldPinKeyEncryptedMasterKeyPostMigration: any = null;
const oldPinKeyEncryptedMasterKeyPreMigrationPersistent =
"2.fb5kOEZvh9zPABbP8WRmSQ==|Yi6ZAJY+UtqCKMUSqp1ahY9Kf8QuneKXs6BMkpNsakLVOzTYkHHlilyGABMF7GzUO8QHyZi7V/Ovjjg+Naf3Sm8qNhxtDhibITv4k8rDnM0=|TFkq3h2VNTT1z5BFbebm37WYuxyEHXuRo0DZJI7TQnw=";
beforeEach(() => { beforeEach(() => {
jest.clearAllMocks(); jest.clearAllMocks();
accountService = mockAccountServiceWith(mockUserId, { email: mockUserEmail }); accountService = mockAccountServiceWith(mockUserId, { email: mockUserEmail });
masterPasswordService = new FakeMasterPasswordService();
stateProvider = new FakeStateProvider(accountService); stateProvider = new FakeStateProvider(accountService);
sut = new PinService( sut = new PinService(
@@ -75,9 +64,7 @@ describe("PinService", () => {
kdfConfigService, kdfConfigService,
keyGenerationService, keyGenerationService,
logService, logService,
masterPasswordService,
stateProvider, stateProvider,
stateService,
); );
}); });
@@ -111,12 +98,6 @@ describe("PinService", () => {
await expect(sut.clearUserKeyEncryptedPin(undefined)).rejects.toThrow( await expect(sut.clearUserKeyEncryptedPin(undefined)).rejects.toThrow(
"User ID is required. Cannot clear userKeyEncryptedPin.", "User ID is required. Cannot clear userKeyEncryptedPin.",
); );
await expect(sut.getOldPinKeyEncryptedMasterKey(undefined)).rejects.toThrow(
"User ID is required. Cannot get oldPinKeyEncryptedMasterKey.",
);
await expect(sut.clearOldPinKeyEncryptedMasterKey(undefined)).rejects.toThrow(
"User ID is required. Cannot clear oldPinKeyEncryptedMasterKey.",
);
await expect( await expect(
sut.createPinKeyEncryptedUserKey(mockPin, mockUserKey, undefined), sut.createPinKeyEncryptedUserKey(mockPin, mockUserKey, undefined),
).rejects.toThrow("User ID is required. Cannot create pinKeyEncryptedUserKey."); ).rejects.toThrow("User ID is required. Cannot create pinKeyEncryptedUserKey.");
@@ -288,31 +269,6 @@ describe("PinService", () => {
}); });
}); });
describe("oldPinKeyEncryptedMasterKey methods", () => {
describe("getOldPinKeyEncryptedMasterKey()", () => {
it("should get the oldPinKeyEncryptedMasterKey of the specified userId", async () => {
await sut.getOldPinKeyEncryptedMasterKey(mockUserId);
expect(stateProvider.mock.getUserState$).toHaveBeenCalledWith(
OLD_PIN_KEY_ENCRYPTED_MASTER_KEY,
mockUserId,
);
});
});
describe("clearOldPinKeyEncryptedMasterKey()", () => {
it("should clear the oldPinKeyEncryptedMasterKey of the specified userId", async () => {
await sut.clearOldPinKeyEncryptedMasterKey(mockUserId);
expect(stateProvider.mock.setUserState).toHaveBeenCalledWith(
OLD_PIN_KEY_ENCRYPTED_MASTER_KEY,
null,
mockUserId,
);
});
});
});
describe("makePinKey()", () => { describe("makePinKey()", () => {
it("should make a PinKey", async () => { it("should make a PinKey", async () => {
// Arrange // Arrange
@@ -346,26 +302,10 @@ describe("PinService", () => {
expect(result).toBe("PERSISTENT"); expect(result).toBe("PERSISTENT");
}); });
it("should return 'PERSISTENT' if an old oldPinKeyEncryptedMasterKey is found", async () => { it("should return 'EPHEMERAL' if a pinKeyEncryptedUserKey (persistent version) is not found but a userKeyEncryptedPin is found", async () => {
// Arrange
sut.getUserKeyEncryptedPin = jest.fn().mockResolvedValue(null);
sut.getPinKeyEncryptedUserKeyPersistent = jest.fn().mockResolvedValue(null);
sut.getOldPinKeyEncryptedMasterKey = jest
.fn()
.mockResolvedValue(oldPinKeyEncryptedMasterKeyPreMigrationPersistent);
// Act
const result = await sut.getPinLockType(mockUserId);
// Assert
expect(result).toBe("PERSISTENT");
});
it("should return 'EPHEMERAL' if neither a pinKeyEncryptedUserKey (persistent version) nor an old oldPinKeyEncryptedMasterKey are found, but a userKeyEncryptedPin is found", async () => {
// Arrange // Arrange
sut.getUserKeyEncryptedPin = jest.fn().mockResolvedValue(mockUserKeyEncryptedPin); sut.getUserKeyEncryptedPin = jest.fn().mockResolvedValue(mockUserKeyEncryptedPin);
sut.getPinKeyEncryptedUserKeyPersistent = jest.fn().mockResolvedValue(null); sut.getPinKeyEncryptedUserKeyPersistent = jest.fn().mockResolvedValue(null);
sut.getOldPinKeyEncryptedMasterKey = jest.fn().mockResolvedValue(null);
// Act // Act
const result = await sut.getPinLockType(mockUserId); const result = await sut.getPinLockType(mockUserId);
@@ -374,11 +314,10 @@ describe("PinService", () => {
expect(result).toBe("EPHEMERAL"); expect(result).toBe("EPHEMERAL");
}); });
it("should return 'DISABLED' if ALL three of these are NOT found: userKeyEncryptedPin, pinKeyEncryptedUserKey (persistent version), oldPinKeyEncryptedMasterKey", async () => { it("should return 'DISABLED' if both of these are NOT found: userKeyEncryptedPin, pinKeyEncryptedUserKey (persistent version)", async () => {
// Arrange // Arrange
sut.getUserKeyEncryptedPin = jest.fn().mockResolvedValue(null); sut.getUserKeyEncryptedPin = jest.fn().mockResolvedValue(null);
sut.getPinKeyEncryptedUserKeyPersistent = jest.fn().mockResolvedValue(null); sut.getPinKeyEncryptedUserKeyPersistent = jest.fn().mockResolvedValue(null);
sut.getOldPinKeyEncryptedMasterKey = jest.fn().mockResolvedValue(null);
// Act // Act
const result = await sut.getPinLockType(mockUserId); const result = await sut.getPinLockType(mockUserId);
@@ -476,46 +415,20 @@ describe("PinService", () => {
}); });
describe("decryptUserKeyWithPin()", () => { describe("decryptUserKeyWithPin()", () => {
async function setupDecryptUserKeyWithPinMocks( async function setupDecryptUserKeyWithPinMocks(pinLockType: PinLockType) {
pinLockType: PinLockType,
migrationStatus: "PRE" | "POST" = "POST",
) {
sut.getPinLockType = jest.fn().mockResolvedValue(pinLockType); sut.getPinLockType = jest.fn().mockResolvedValue(pinLockType);
mockPinEncryptedKeyDataByPinLockType(pinLockType, migrationStatus); mockPinEncryptedKeyDataByPinLockType(pinLockType);
kdfConfigService.getKdfConfig.mockResolvedValue(DEFAULT_KDF_CONFIG); kdfConfigService.getKdfConfig.mockResolvedValue(DEFAULT_KDF_CONFIG);
if (pinLockType === "PERSISTENT" && migrationStatus === "PRE") { mockDecryptUserKeyFn();
await mockDecryptAndMigrateOldPinKeyEncryptedMasterKeyFn();
} else {
mockDecryptUserKeyFn();
}
sut.getUserKeyEncryptedPin = jest.fn().mockResolvedValue(mockUserKeyEncryptedPin); sut.getUserKeyEncryptedPin = jest.fn().mockResolvedValue(mockUserKeyEncryptedPin);
encryptService.decryptToUtf8.mockResolvedValue(mockPin); encryptService.decryptToUtf8.mockResolvedValue(mockPin);
cryptoFunctionService.compareFast.calledWith(mockPin, "1234").mockResolvedValue(true); cryptoFunctionService.compareFast.calledWith(mockPin, "1234").mockResolvedValue(true);
} }
async function mockDecryptAndMigrateOldPinKeyEncryptedMasterKeyFn() {
sut.makePinKey = jest.fn().mockResolvedValue(mockPinKey);
encryptService.decryptToBytes.mockResolvedValue(mockMasterKey.key);
stateService.getEncryptedCryptoSymmetricKey.mockResolvedValue(mockUserKey.keyB64);
masterPasswordService.mock.decryptUserKeyWithMasterKey.mockResolvedValue(mockUserKey);
sut.createPinKeyEncryptedUserKey = jest
.fn()
.mockResolvedValue(pinKeyEncryptedUserKeyPersistant);
await sut.storePinKeyEncryptedUserKey(pinKeyEncryptedUserKeyPersistant, false, mockUserId);
sut.createUserKeyEncryptedPin = jest.fn().mockResolvedValue(mockUserKeyEncryptedPin);
await sut.setUserKeyEncryptedPin(mockUserKeyEncryptedPin, mockUserId);
await sut.clearOldPinKeyEncryptedMasterKey(mockUserId);
}
function mockDecryptUserKeyFn() { function mockDecryptUserKeyFn() {
sut.getPinKeyEncryptedUserKeyPersistent = jest sut.getPinKeyEncryptedUserKeyPersistent = jest
.fn() .fn()
@@ -524,26 +437,12 @@ describe("PinService", () => {
encryptService.decryptToBytes.mockResolvedValue(mockUserKey.key); encryptService.decryptToBytes.mockResolvedValue(mockUserKey.key);
} }
function mockPinEncryptedKeyDataByPinLockType( function mockPinEncryptedKeyDataByPinLockType(pinLockType: PinLockType) {
pinLockType: PinLockType,
migrationStatus: "PRE" | "POST" = "POST",
) {
switch (pinLockType) { switch (pinLockType) {
case "PERSISTENT": case "PERSISTENT":
sut.getPinKeyEncryptedUserKeyPersistent = jest sut.getPinKeyEncryptedUserKeyPersistent = jest
.fn() .fn()
.mockResolvedValue(pinKeyEncryptedUserKeyPersistant); .mockResolvedValue(pinKeyEncryptedUserKeyPersistant);
if (migrationStatus === "PRE") {
sut.getOldPinKeyEncryptedMasterKey = jest
.fn()
.mockResolvedValue(oldPinKeyEncryptedMasterKeyPreMigrationPersistent);
} else {
sut.getOldPinKeyEncryptedMasterKey = jest
.fn()
.mockResolvedValue(oldPinKeyEncryptedMasterKeyPostMigration); // null
}
break; break;
case "EPHEMERAL": case "EPHEMERAL":
sut.getPinKeyEncryptedUserKeyEphemeral = jest sut.getPinKeyEncryptedUserKeyEphemeral = jest
@@ -557,49 +456,16 @@ describe("PinService", () => {
} }
} }
const testCases: { pinLockType: PinLockType; migrationStatus: "PRE" | "POST" }[] = [ const testCases: { pinLockType: PinLockType }[] = [
{ pinLockType: "PERSISTENT", migrationStatus: "PRE" }, { pinLockType: "PERSISTENT" },
{ pinLockType: "PERSISTENT", migrationStatus: "POST" }, { pinLockType: "EPHEMERAL" },
{ pinLockType: "EPHEMERAL", migrationStatus: "POST" },
]; ];
testCases.forEach(({ pinLockType, migrationStatus }) => { testCases.forEach(({ pinLockType }) => {
describe(`given a ${pinLockType} PIN (${migrationStatus} migration)`, () => { describe(`given a ${pinLockType} PIN)`, () => {
if (pinLockType === "PERSISTENT" && migrationStatus === "PRE") {
it("should clear the oldPinKeyEncryptedMasterKey from state", async () => {
// Arrange
await setupDecryptUserKeyWithPinMocks(pinLockType, migrationStatus);
// Act
await sut.decryptUserKeyWithPin(mockPin, mockUserId);
// Assert
expect(stateProvider.mock.setUserState).toHaveBeenCalledWith(
OLD_PIN_KEY_ENCRYPTED_MASTER_KEY,
null,
mockUserId,
);
});
it("should set the new pinKeyEncrypterUserKeyPersistent to state", async () => {
// Arrange
await setupDecryptUserKeyWithPinMocks(pinLockType, migrationStatus);
// Act
await sut.decryptUserKeyWithPin(mockPin, mockUserId);
// Assert
expect(stateProvider.mock.setUserState).toHaveBeenCalledWith(
PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT,
pinKeyEncryptedUserKeyPersistant.encryptedString,
mockUserId,
);
});
}
it(`should successfully decrypt and return user key when using a valid PIN`, async () => { it(`should successfully decrypt and return user key when using a valid PIN`, async () => {
// Arrange // Arrange
await setupDecryptUserKeyWithPinMocks(pinLockType, migrationStatus); await setupDecryptUserKeyWithPinMocks(pinLockType);
// Act // Act
const result = await sut.decryptUserKeyWithPin(mockPin, mockUserId); const result = await sut.decryptUserKeyWithPin(mockPin, mockUserId);
@@ -610,7 +476,7 @@ describe("PinService", () => {
it(`should return null when PIN is incorrect and user key cannot be decrypted`, async () => { it(`should return null when PIN is incorrect and user key cannot be decrypted`, async () => {
// Arrange // Arrange
await setupDecryptUserKeyWithPinMocks(pinLockType, migrationStatus); await setupDecryptUserKeyWithPinMocks(pinLockType);
sut.decryptUserKeyWithPin = jest.fn().mockResolvedValue(null); sut.decryptUserKeyWithPin = jest.fn().mockResolvedValue(null);
// Act // Act
@@ -623,7 +489,7 @@ describe("PinService", () => {
// not sure if this is a realistic scenario but going to test it anyway // not sure if this is a realistic scenario but going to test it anyway
it(`should return null when PIN doesn't match after successful user key decryption`, async () => { it(`should return null when PIN doesn't match after successful user key decryption`, async () => {
// Arrange // Arrange
await setupDecryptUserKeyWithPinMocks(pinLockType, migrationStatus); await setupDecryptUserKeyWithPinMocks(pinLockType);
encryptService.decryptToUtf8.mockResolvedValue("9999"); // non matching PIN encryptService.decryptToUtf8.mockResolvedValue("9999"); // non matching PIN
// Act // Act

View File

@@ -163,19 +163,6 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr
throw new Error("No master key found."); throw new Error("No master key found.");
} }
// Try one more way to get the user key if it still wasn't found.
if (userKey == null) {
const deprecatedKey = await this.stateService.getEncryptedCryptoSymmetricKey({
userId: userId,
});
if (deprecatedKey == null) {
throw new Error("No encrypted user key found.");
}
userKey = new EncString(deprecatedKey);
}
let decUserKey: Uint8Array; let decUserKey: Uint8Array;
if (userKey.encryptionType === EncryptionType.AesCbc256_B64) { if (userKey.encryptionType === EncryptionType.AesCbc256_B64) {

View File

@@ -147,7 +147,6 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction {
await this.masterPasswordService.clearMasterKey(lockingUserId); await this.masterPasswordService.clearMasterKey(lockingUserId);
await this.stateService.setUserKeyAutoUnlock(null, { userId: lockingUserId }); await this.stateService.setUserKeyAutoUnlock(null, { userId: lockingUserId });
await this.stateService.setCryptoMasterKeyAuto(null, { userId: lockingUserId });
await this.cipherService.clearCache(lockingUserId); await this.cipherService.clearCache(lockingUserId);

View File

@@ -50,14 +50,6 @@ export abstract class StateService<T extends Account = Account> {
value: boolean, value: boolean,
options?: StorageOptions, options?: StorageOptions,
) => Promise<void>; ) => Promise<void>;
/**
* @deprecated For migration purposes only, use getUserKeyMasterKey instead
*/
getEncryptedCryptoSymmetricKey: (options?: StorageOptions) => Promise<string>;
/**
* @deprecated For migration purposes only, use setUserKeyAuto instead
*/
setCryptoMasterKeyAuto: (value: string | null, options?: StorageOptions) => Promise<void>;
getDuckDuckGoSharedKey: (options?: StorageOptions) => Promise<string>; getDuckDuckGoSharedKey: (options?: StorageOptions) => Promise<string>;
setDuckDuckGoSharedKey: (value: string, options?: StorageOptions) => Promise<void>; setDuckDuckGoSharedKey: (value: string, options?: StorageOptions) => Promise<void>;

View File

@@ -48,8 +48,6 @@ export class EncryptionPair<TEncrypted, TDecrypted> {
export class AccountKeys { export class AccountKeys {
publicKey?: Uint8Array; publicKey?: Uint8Array;
/** @deprecated July 2023, left for migration purposes*/
cryptoMasterKeyAuto?: string;
/** @deprecated July 2023, left for migration purposes*/ /** @deprecated July 2023, left for migration purposes*/
cryptoSymmetricKey?: EncryptionPair<string, SymmetricCryptoKey> = new EncryptionPair< cryptoSymmetricKey?: EncryptionPair<string, SymmetricCryptoKey> = new EncryptionPair<
string, string,

View File

@@ -222,45 +222,6 @@ export class StateService<
await this.saveSecureStorageKey(partialKeys.userBiometricKey, value, options); await this.saveSecureStorageKey(partialKeys.userBiometricKey, value, options);
} }
/**
* @deprecated Use UserKeyAuto instead
*/
async setCryptoMasterKeyAuto(value: string | null, options?: StorageOptions): Promise<void> {
options = this.reconcileOptions(
this.reconcileOptions(options, { keySuffix: "auto" }),
await this.defaultSecureStorageOptions(),
);
if (options?.userId == null) {
return;
}
await this.saveSecureStorageKey(partialKeys.autoKey, value, options);
}
/**
* @deprecated I don't see where this is even used
*/
async getCryptoMasterKeyB64(options?: StorageOptions): Promise<string> {
options = this.reconcileOptions(options, await this.defaultSecureStorageOptions());
if (options?.userId == null) {
return null;
}
return await this.secureStorageService.get<string>(
`${options?.userId}${partialKeys.masterKey}`,
options,
);
}
/**
* @deprecated I don't see where this is even used
*/
async setCryptoMasterKeyB64(value: string, options?: StorageOptions): Promise<void> {
options = this.reconcileOptions(options, await this.defaultSecureStorageOptions());
if (options?.userId == null) {
return;
}
await this.saveSecureStorageKey(partialKeys.masterKey, value, options);
}
async getDuckDuckGoSharedKey(options?: StorageOptions): Promise<string> { async getDuckDuckGoSharedKey(options?: StorageOptions): Promise<string> {
options = this.reconcileOptions(options, await this.defaultSecureStorageOptions()); options = this.reconcileOptions(options, await this.defaultSecureStorageOptions());
if (options?.userId == null) { if (options?.userId == null) {
@@ -619,8 +580,6 @@ export class StateService<
await this.setUserKeyAutoUnlock(null, { userId: userId }); await this.setUserKeyAutoUnlock(null, { userId: userId });
await this.setUserKeyBiometric(null, { userId: userId }); await this.setUserKeyBiometric(null, { userId: userId });
await this.setCryptoMasterKeyAuto(null, { userId: userId });
await this.setCryptoMasterKeyB64(null, { userId: userId });
} }
protected async removeAccountFromMemory(userId: string = null): Promise<void> { protected async removeAccountFromMemory(userId: string = null): Promise<void> {

View File

@@ -390,14 +390,6 @@ export abstract class KeyService {
publicKey: string; publicKey: string;
privateKey: EncString; privateKey: EncString;
}>; }>;
/**
* Previously, the master key was used for any additional key like the biometrics or pin key.
* We have switched to using the user key for these purposes. This method is for clearing the state
* of the older keys on logout or post migration.
* @param keySuffix The desired type of key to clear
* @param userId The desired user
*/
abstract clearDeprecatedKeys(keySuffix: KeySuffixOptions, userId?: string): Promise<void>;
/** /**
* Retrieves all the keys needed for decrypting Ciphers * Retrieves all the keys needed for decrypting Ciphers

View File

@@ -252,14 +252,6 @@ describe("keyService", () => {
userId: mockUserId, userId: mockUserId,
}); });
}); });
it("clears the old deprecated Auto key whenever a User Key is set", async () => {
await keyService.setUserKey(mockUserKey, mockUserId);
expect(stateService.setCryptoMasterKeyAuto).toHaveBeenCalledWith(null, {
userId: mockUserId,
});
});
}); });
it("throws if key is null", async () => { it("throws if key is null", async () => {

View File

@@ -254,16 +254,10 @@ export class DefaultKeyService implements KeyServiceAbstraction {
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises // eslint-disable-next-line @typescript-eslint/no-floating-promises
this.stateService.setUserKeyAutoUnlock(null, { userId: userId }); this.stateService.setUserKeyAutoUnlock(null, { userId: userId });
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.clearDeprecatedKeys(KeySuffixOptions.Auto, userId);
} }
if (keySuffix === KeySuffixOptions.Pin && userId != null) { if (keySuffix === KeySuffixOptions.Pin && userId != null) {
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
this.pinService.clearPinKeyEncryptedUserKeyEphemeral(userId); this.pinService.clearPinKeyEncryptedUserKeyEphemeral(userId);
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.clearDeprecatedKeys(KeySuffixOptions.Pin, userId);
} }
} }
@@ -565,7 +559,6 @@ export class DefaultKeyService implements KeyServiceAbstraction {
await this.pinService.clearPinKeyEncryptedUserKeyPersistent(userId); await this.pinService.clearPinKeyEncryptedUserKeyPersistent(userId);
await this.pinService.clearPinKeyEncryptedUserKeyEphemeral(userId); await this.pinService.clearPinKeyEncryptedUserKeyEphemeral(userId);
await this.pinService.clearUserKeyEncryptedPin(userId); await this.pinService.clearUserKeyEncryptedPin(userId);
await this.clearDeprecatedKeys(KeySuffixOptions.Pin, userId);
} }
async makeSendKey(keyMaterial: CsprngArray): Promise<SymmetricCryptoKey> { async makeSendKey(keyMaterial: CsprngArray): Promise<SymmetricCryptoKey> {
@@ -726,7 +719,6 @@ export class DefaultKeyService implements KeyServiceAbstraction {
} else { } else {
await this.stateService.setUserKeyAutoUnlock(null, { userId: userId }); await this.stateService.setUserKeyAutoUnlock(null, { userId: userId });
} }
await this.clearDeprecatedKeys(KeySuffixOptions.Auto, userId);
const storePin = await this.shouldStoreKey(KeySuffixOptions.Pin, userId); const storePin = await this.shouldStoreKey(KeySuffixOptions.Pin, userId);
if (storePin) { if (storePin) {
@@ -749,9 +741,6 @@ export class DefaultKeyService implements KeyServiceAbstraction {
noPreExistingPersistentKey, noPreExistingPersistentKey,
userId, userId,
); );
// We can't always clear deprecated keys because the pin is only
// migrated once used to unlock
await this.clearDeprecatedKeys(KeySuffixOptions.Pin, userId);
} else { } else {
await this.pinService.clearPinKeyEncryptedUserKeyPersistent(userId); await this.pinService.clearPinKeyEncryptedUserKeyPersistent(userId);
await this.pinService.clearPinKeyEncryptedUserKeyEphemeral(userId); await this.pinService.clearPinKeyEncryptedUserKeyEphemeral(userId);
@@ -835,19 +824,6 @@ export class DefaultKeyService implements KeyServiceAbstraction {
return [new SymmetricCryptoKey(newSymKey) as T, protectedSymKey]; return [new SymmetricCryptoKey(newSymKey) as T, protectedSymKey];
} }
// --LEGACY METHODS--
// We previously used the master key for additional keys, but now we use the user key.
// These methods support migrating the old keys to the new ones.
// TODO: Remove after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3475)
async clearDeprecatedKeys(keySuffix: KeySuffixOptions, userId?: UserId) {
if (keySuffix === KeySuffixOptions.Auto) {
await this.stateService.setCryptoMasterKeyAuto(null, { userId: userId });
} else if (keySuffix === KeySuffixOptions.Pin && userId != null) {
await this.pinService.clearOldPinKeyEncryptedMasterKey(userId);
}
}
userKey$(userId: UserId): Observable<UserKey | null> { userKey$(userId: UserId): Observable<UserKey | null> {
return this.stateProvider.getUser(userId, USER_KEY).state$; return this.stateProvider.getUser(userId, USER_KEY).state$;
} }