mirror of
https://github.com/bitwarden/browser
synced 2025-12-06 00:13:28 +00:00
[PM-24377] Use PureCrypto for decryptUserKeyWithMasterKey on the master password service (#16522)
* use PureCrypto in master password service decryptUserKeyWithMasterKey * test for legacy AES256-CBC * update SDK version to include the `PureCrypto.decrypt_user_key_with_master_key` * change from integration to unit tests, use fake state provider
This commit is contained in:
@@ -698,9 +698,7 @@ export default class MainBackground {
|
|||||||
|
|
||||||
this.masterPasswordService = new MasterPasswordService(
|
this.masterPasswordService = new MasterPasswordService(
|
||||||
this.stateProvider,
|
this.stateProvider,
|
||||||
this.stateService,
|
|
||||||
this.keyGenerationService,
|
this.keyGenerationService,
|
||||||
this.encryptService,
|
|
||||||
this.logService,
|
this.logService,
|
||||||
this.cryptoFunctionService,
|
this.cryptoFunctionService,
|
||||||
this.accountService,
|
this.accountService,
|
||||||
|
|||||||
@@ -455,9 +455,7 @@ export class ServiceContainer {
|
|||||||
this.kdfConfigService = new DefaultKdfConfigService(this.stateProvider);
|
this.kdfConfigService = new DefaultKdfConfigService(this.stateProvider);
|
||||||
this.masterPasswordService = new MasterPasswordService(
|
this.masterPasswordService = new MasterPasswordService(
|
||||||
this.stateProvider,
|
this.stateProvider,
|
||||||
this.stateService,
|
|
||||||
this.keyGenerationService,
|
this.keyGenerationService,
|
||||||
this.encryptService,
|
|
||||||
this.logService,
|
this.logService,
|
||||||
this.cryptoFunctionService,
|
this.cryptoFunctionService,
|
||||||
this.accountService,
|
this.accountService,
|
||||||
|
|||||||
@@ -1069,9 +1069,7 @@ const safeProviders: SafeProvider[] = [
|
|||||||
useClass: MasterPasswordService,
|
useClass: MasterPasswordService,
|
||||||
deps: [
|
deps: [
|
||||||
StateProvider,
|
StateProvider,
|
||||||
StateServiceAbstraction,
|
|
||||||
KeyGenerationService,
|
KeyGenerationService,
|
||||||
EncryptService,
|
|
||||||
LogService,
|
LogService,
|
||||||
CryptoFunctionServiceAbstraction,
|
CryptoFunctionServiceAbstraction,
|
||||||
AccountServiceAbstraction,
|
AccountServiceAbstraction,
|
||||||
|
|||||||
@@ -6,22 +6,22 @@ import { SdkLoadService } from "@bitwarden/common/platform/abstractions/sdk/sdk-
|
|||||||
import { Utils } from "@bitwarden/common/platform/misc/utils";
|
import { Utils } from "@bitwarden/common/platform/misc/utils";
|
||||||
// eslint-disable-next-line no-restricted-imports
|
// eslint-disable-next-line no-restricted-imports
|
||||||
import { Argon2KdfConfig, KdfConfig, KdfType, PBKDF2KdfConfig } from "@bitwarden/key-management";
|
import { Argon2KdfConfig, KdfConfig, KdfType, PBKDF2KdfConfig } from "@bitwarden/key-management";
|
||||||
|
import { PureCrypto } from "@bitwarden/sdk-internal";
|
||||||
|
|
||||||
import {
|
import {
|
||||||
FakeAccountService,
|
FakeAccountService,
|
||||||
FakeStateProvider,
|
FakeStateProvider,
|
||||||
|
makeEncString,
|
||||||
makeSymmetricCryptoKey,
|
makeSymmetricCryptoKey,
|
||||||
mockAccountServiceWith,
|
mockAccountServiceWith,
|
||||||
} from "../../../../spec";
|
} from "../../../../spec";
|
||||||
import { ForceSetPasswordReason } from "../../../auth/models/domain/force-set-password-reason";
|
import { ForceSetPasswordReason } from "../../../auth/models/domain/force-set-password-reason";
|
||||||
import { LogService } from "../../../platform/abstractions/log.service";
|
import { LogService } from "../../../platform/abstractions/log.service";
|
||||||
import { StateService } from "../../../platform/abstractions/state.service";
|
|
||||||
import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key";
|
import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key";
|
||||||
import { UserId } from "../../../types/guid";
|
import { UserId } from "../../../types/guid";
|
||||||
import { MasterKey, UserKey } from "../../../types/key";
|
import { MasterKey, UserKey } from "../../../types/key";
|
||||||
import { KeyGenerationService } from "../../crypto";
|
import { KeyGenerationService } from "../../crypto";
|
||||||
import { CryptoFunctionService } from "../../crypto/abstractions/crypto-function.service";
|
import { CryptoFunctionService } from "../../crypto/abstractions/crypto-function.service";
|
||||||
import { EncryptService } from "../../crypto/abstractions/encrypt.service";
|
|
||||||
import { EncString } from "../../crypto/models/enc-string";
|
import { EncString } from "../../crypto/models/enc-string";
|
||||||
import {
|
import {
|
||||||
MasterKeyWrappedUserKey,
|
MasterKeyWrappedUserKey,
|
||||||
@@ -31,6 +31,7 @@ import {
|
|||||||
|
|
||||||
import {
|
import {
|
||||||
FORCE_SET_PASSWORD_REASON,
|
FORCE_SET_PASSWORD_REASON,
|
||||||
|
MASTER_KEY,
|
||||||
MASTER_KEY_ENCRYPTED_USER_KEY,
|
MASTER_KEY_ENCRYPTED_USER_KEY,
|
||||||
MASTER_PASSWORD_UNLOCK_KEY,
|
MASTER_PASSWORD_UNLOCK_KEY,
|
||||||
MasterPasswordService,
|
MasterPasswordService,
|
||||||
@@ -39,9 +40,7 @@ import {
|
|||||||
describe("MasterPasswordService", () => {
|
describe("MasterPasswordService", () => {
|
||||||
let sut: MasterPasswordService;
|
let sut: MasterPasswordService;
|
||||||
|
|
||||||
let stateService: MockProxy<StateService>;
|
|
||||||
let keyGenerationService: MockProxy<KeyGenerationService>;
|
let keyGenerationService: MockProxy<KeyGenerationService>;
|
||||||
let encryptService: MockProxy<EncryptService>;
|
|
||||||
let logService: MockProxy<LogService>;
|
let logService: MockProxy<LogService>;
|
||||||
let cryptoFunctionService: MockProxy<CryptoFunctionService>;
|
let cryptoFunctionService: MockProxy<CryptoFunctionService>;
|
||||||
let accountService: FakeAccountService;
|
let accountService: FakeAccountService;
|
||||||
@@ -53,18 +52,12 @@ describe("MasterPasswordService", () => {
|
|||||||
const kdfArgon2: KdfConfig = new Argon2KdfConfig(4, 64, 3);
|
const kdfArgon2: KdfConfig = new Argon2KdfConfig(4, 64, 3);
|
||||||
const salt = "test@bitwarden.com" as MasterPasswordSalt;
|
const salt = "test@bitwarden.com" as MasterPasswordSalt;
|
||||||
const userKey = makeSymmetricCryptoKey(64, 2) as UserKey;
|
const userKey = makeSymmetricCryptoKey(64, 2) as UserKey;
|
||||||
const testUserKey: SymmetricCryptoKey = makeSymmetricCryptoKey(64, 1);
|
|
||||||
const testMasterKey: MasterKey = makeSymmetricCryptoKey(32, 2);
|
|
||||||
const testStretchedMasterKey: SymmetricCryptoKey = makeSymmetricCryptoKey(64, 3);
|
|
||||||
const testMasterKeyEncryptedKey =
|
const testMasterKeyEncryptedKey =
|
||||||
"0.gbauOANURUHqvhLTDnva1A==|nSW+fPumiuTaDB/s12+JO88uemV6rhwRSR+YR1ZzGr5j6Ei3/h+XEli2Unpz652NlZ9NTuRpHxeOqkYYJtp7J+lPMoclgteXuAzUu9kqlRc=";
|
"0.gbauOANURUHqvhLTDnva1A==|nSW+fPumiuTaDB/s12+JO88uemV6rhwRSR+YR1ZzGr5j6Ei3/h+XEli2Unpz652NlZ9NTuRpHxeOqkYYJtp7J+lPMoclgteXuAzUu9kqlRc=";
|
||||||
const testStretchedMasterKeyEncryptedKey =
|
const sdkLoadServiceReady = jest.fn();
|
||||||
"2.gbauOANURUHqvhLTDnva1A==|nSW+fPumiuTaDB/s12+JO88uemV6rhwRSR+YR1ZzGr5j6Ei3/h+XEli2Unpz652NlZ9NTuRpHxeOqkYYJtp7J+lPMoclgteXuAzUu9kqlRc=|DeUFkhIwgkGdZA08bDnDqMMNmZk21D+H5g8IostPKAY=";
|
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
stateService = mock<StateService>();
|
|
||||||
keyGenerationService = mock<KeyGenerationService>();
|
keyGenerationService = mock<KeyGenerationService>();
|
||||||
encryptService = mock<EncryptService>();
|
|
||||||
logService = mock<LogService>();
|
logService = mock<LogService>();
|
||||||
cryptoFunctionService = mock<CryptoFunctionService>();
|
cryptoFunctionService = mock<CryptoFunctionService>();
|
||||||
accountService = mockAccountServiceWith(userId);
|
accountService = mockAccountServiceWith(userId);
|
||||||
@@ -72,18 +65,18 @@ describe("MasterPasswordService", () => {
|
|||||||
|
|
||||||
sut = new MasterPasswordService(
|
sut = new MasterPasswordService(
|
||||||
stateProvider,
|
stateProvider,
|
||||||
stateService,
|
|
||||||
keyGenerationService,
|
keyGenerationService,
|
||||||
encryptService,
|
|
||||||
logService,
|
logService,
|
||||||
cryptoFunctionService,
|
cryptoFunctionService,
|
||||||
accountService,
|
accountService,
|
||||||
);
|
);
|
||||||
|
|
||||||
encryptService.unwrapSymmetricKey.mockResolvedValue(makeSymmetricCryptoKey(64, 1));
|
|
||||||
keyGenerationService.stretchKey.mockResolvedValue(makeSymmetricCryptoKey(64, 3));
|
keyGenerationService.stretchKey.mockResolvedValue(makeSymmetricCryptoKey(64, 3));
|
||||||
Object.defineProperty(SdkLoadService, "Ready", {
|
Object.defineProperty(SdkLoadService, "Ready", {
|
||||||
value: Promise.resolve(),
|
value: new Promise((resolve) => {
|
||||||
|
sdkLoadServiceReady();
|
||||||
|
resolve(undefined);
|
||||||
|
}),
|
||||||
configurable: true,
|
configurable: true,
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
@@ -159,41 +152,62 @@ describe("MasterPasswordService", () => {
|
|||||||
expect(state).toEqual(ForceSetPasswordReason.None);
|
expect(state).toEqual(ForceSetPasswordReason.None);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("decryptUserKeyWithMasterKey", () => {
|
describe("decryptUserKeyWithMasterKey", () => {
|
||||||
it("decrypts a userkey wrapped in AES256-CBC", async () => {
|
const masterKey = makeSymmetricCryptoKey(64, 0) as MasterKey;
|
||||||
encryptService.unwrapSymmetricKey.mockResolvedValue(testUserKey);
|
const userKey = makeSymmetricCryptoKey(64, 1) as UserKey;
|
||||||
await sut.decryptUserKeyWithMasterKey(
|
const masterKeyEncryptedUserKey = makeEncString("test-encrypted-user-key");
|
||||||
testMasterKey,
|
|
||||||
|
const decryptUserKeyWithMasterKeyMock = jest.spyOn(
|
||||||
|
PureCrypto,
|
||||||
|
"decrypt_user_key_with_master_key",
|
||||||
|
);
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
decryptUserKeyWithMasterKeyMock.mockReturnValue(userKey.toEncoded());
|
||||||
|
});
|
||||||
|
|
||||||
|
it("successfully decrypts", async () => {
|
||||||
|
const decryptedUserKey = await sut.decryptUserKeyWithMasterKey(
|
||||||
|
masterKey,
|
||||||
userId,
|
userId,
|
||||||
new EncString(testMasterKeyEncryptedKey),
|
masterKeyEncryptedUserKey,
|
||||||
);
|
);
|
||||||
expect(encryptService.unwrapSymmetricKey).toHaveBeenCalledWith(
|
|
||||||
new EncString(testMasterKeyEncryptedKey),
|
expect(decryptedUserKey).toEqual(new SymmetricCryptoKey(userKey.toEncoded()));
|
||||||
testMasterKey,
|
expect(sdkLoadServiceReady).toHaveBeenCalled();
|
||||||
|
expect(PureCrypto.decrypt_user_key_with_master_key).toHaveBeenCalledWith(
|
||||||
|
masterKeyEncryptedUserKey.toSdk(),
|
||||||
|
masterKey.toEncoded(),
|
||||||
|
);
|
||||||
|
expect(sdkLoadServiceReady.mock.invocationCallOrder[0]).toBeLessThan(
|
||||||
|
decryptUserKeyWithMasterKeyMock.mock.invocationCallOrder[0],
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
it("decrypts a userkey wrapped in AES256-CBC-HMAC", async () => {
|
|
||||||
encryptService.unwrapSymmetricKey.mockResolvedValue(testUserKey);
|
it("returns null when failed to decrypt", async () => {
|
||||||
keyGenerationService.stretchKey.mockResolvedValue(testStretchedMasterKey);
|
decryptUserKeyWithMasterKeyMock.mockImplementation(() => {
|
||||||
await sut.decryptUserKeyWithMasterKey(
|
throw new Error("Decryption failed");
|
||||||
testMasterKey,
|
});
|
||||||
|
|
||||||
|
const decryptedUserKey = await sut.decryptUserKeyWithMasterKey(
|
||||||
|
masterKey,
|
||||||
userId,
|
userId,
|
||||||
new EncString(testStretchedMasterKeyEncryptedKey),
|
masterKeyEncryptedUserKey,
|
||||||
);
|
);
|
||||||
expect(encryptService.unwrapSymmetricKey).toHaveBeenCalledWith(
|
expect(decryptedUserKey).toBeNull();
|
||||||
new EncString(testStretchedMasterKeyEncryptedKey),
|
|
||||||
testStretchedMasterKey,
|
|
||||||
);
|
|
||||||
expect(keyGenerationService.stretchKey).toHaveBeenCalledWith(testMasterKey);
|
|
||||||
});
|
});
|
||||||
it("returns null if failed to decrypt", async () => {
|
|
||||||
encryptService.unwrapSymmetricKey.mockRejectedValue(new Error("Decryption failed"));
|
it("returns error when master key is null", async () => {
|
||||||
const result = await sut.decryptUserKeyWithMasterKey(
|
stateProvider.singleUser.getFake(userId, MASTER_KEY).nextState(null);
|
||||||
testMasterKey,
|
|
||||||
userId,
|
await expect(
|
||||||
new EncString(testStretchedMasterKeyEncryptedKey),
|
sut.decryptUserKeyWithMasterKey(
|
||||||
);
|
null as unknown as MasterKey,
|
||||||
expect(result).toBeNull();
|
userId,
|
||||||
|
masterKeyEncryptedUserKey,
|
||||||
|
),
|
||||||
|
).rejects.toThrow("No master key found.");
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -331,11 +345,11 @@ describe("MasterPasswordService", () => {
|
|||||||
it.each([kdfPBKDF2, kdfArgon2])(
|
it.each([kdfPBKDF2, kdfArgon2])(
|
||||||
"sets the master password unlock data kdf %o in the state",
|
"sets the master password unlock data kdf %o in the state",
|
||||||
async (kdfConfig) => {
|
async (kdfConfig) => {
|
||||||
const masterPasswordUnlockData = await sut.makeMasterPasswordUnlockData(
|
const masterKeyWrappedUserKey = makeEncString().toSdk() as MasterKeyWrappedUserKey;
|
||||||
"test-password",
|
const masterPasswordUnlockData = new MasterPasswordUnlockData(
|
||||||
kdfConfig,
|
|
||||||
salt,
|
salt,
|
||||||
userKey,
|
kdfConfig,
|
||||||
|
masterKeyWrappedUserKey,
|
||||||
);
|
);
|
||||||
|
|
||||||
await sut.setMasterPasswordUnlockData(masterPasswordUnlockData, userId);
|
await sut.setMasterPasswordUnlockData(masterPasswordUnlockData, userId);
|
||||||
|
|||||||
@@ -12,8 +12,6 @@ import { PureCrypto } from "@bitwarden/sdk-internal";
|
|||||||
|
|
||||||
import { ForceSetPasswordReason } from "../../../auth/models/domain/force-set-password-reason";
|
import { ForceSetPasswordReason } from "../../../auth/models/domain/force-set-password-reason";
|
||||||
import { LogService } from "../../../platform/abstractions/log.service";
|
import { LogService } from "../../../platform/abstractions/log.service";
|
||||||
import { StateService } from "../../../platform/abstractions/state.service";
|
|
||||||
import { EncryptionType } from "../../../platform/enums";
|
|
||||||
import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key";
|
import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key";
|
||||||
import {
|
import {
|
||||||
MASTER_PASSWORD_DISK,
|
MASTER_PASSWORD_DISK,
|
||||||
@@ -26,7 +24,6 @@ import { UserId } from "../../../types/guid";
|
|||||||
import { MasterKey, UserKey } from "../../../types/key";
|
import { MasterKey, UserKey } from "../../../types/key";
|
||||||
import { KeyGenerationService } from "../../crypto";
|
import { KeyGenerationService } from "../../crypto";
|
||||||
import { CryptoFunctionService } from "../../crypto/abstractions/crypto-function.service";
|
import { CryptoFunctionService } from "../../crypto/abstractions/crypto-function.service";
|
||||||
import { EncryptService } from "../../crypto/abstractions/encrypt.service";
|
|
||||||
import { EncryptedString, EncString } from "../../crypto/models/enc-string";
|
import { EncryptedString, EncString } from "../../crypto/models/enc-string";
|
||||||
import { InternalMasterPasswordServiceAbstraction } from "../abstractions/master-password.service.abstraction";
|
import { InternalMasterPasswordServiceAbstraction } from "../abstractions/master-password.service.abstraction";
|
||||||
import {
|
import {
|
||||||
@@ -38,7 +35,7 @@ import {
|
|||||||
} from "../types/master-password.types";
|
} from "../types/master-password.types";
|
||||||
|
|
||||||
/** Memory since master key shouldn't be available on lock */
|
/** Memory since master key shouldn't be available on lock */
|
||||||
const MASTER_KEY = new UserKeyDefinition<MasterKey>(MASTER_PASSWORD_MEMORY, "masterKey", {
|
export const MASTER_KEY = new UserKeyDefinition<MasterKey>(MASTER_PASSWORD_MEMORY, "masterKey", {
|
||||||
deserializer: (masterKey) => SymmetricCryptoKey.fromJSON(masterKey) as MasterKey,
|
deserializer: (masterKey) => SymmetricCryptoKey.fromJSON(masterKey) as MasterKey,
|
||||||
clearOn: ["lock", "logout"],
|
clearOn: ["lock", "logout"],
|
||||||
});
|
});
|
||||||
@@ -82,9 +79,7 @@ export const MASTER_PASSWORD_UNLOCK_KEY = new UserKeyDefinition<MasterPasswordUn
|
|||||||
export class MasterPasswordService implements InternalMasterPasswordServiceAbstraction {
|
export class MasterPasswordService implements InternalMasterPasswordServiceAbstraction {
|
||||||
constructor(
|
constructor(
|
||||||
private stateProvider: StateProvider,
|
private stateProvider: StateProvider,
|
||||||
private stateService: StateService,
|
|
||||||
private keyGenerationService: KeyGenerationService,
|
private keyGenerationService: KeyGenerationService,
|
||||||
private encryptService: EncryptService,
|
|
||||||
private logService: LogService,
|
private logService: LogService,
|
||||||
private cryptoFunctionService: CryptoFunctionService,
|
private cryptoFunctionService: CryptoFunctionService,
|
||||||
private accountService: AccountService,
|
private accountService: AccountService,
|
||||||
@@ -219,33 +214,15 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr
|
|||||||
throw new Error("No master key found.");
|
throw new Error("No master key found.");
|
||||||
}
|
}
|
||||||
|
|
||||||
let decUserKey: SymmetricCryptoKey;
|
await SdkLoadService.Ready;
|
||||||
|
try {
|
||||||
if (userKey.encryptionType === EncryptionType.AesCbc256_B64) {
|
return new SymmetricCryptoKey(
|
||||||
try {
|
PureCrypto.decrypt_user_key_with_master_key(userKey.toSdk(), masterKey.toEncoded()),
|
||||||
decUserKey = await this.encryptService.unwrapSymmetricKey(userKey, masterKey);
|
) as UserKey;
|
||||||
} catch {
|
} catch {
|
||||||
this.logService.warning("Failed to decrypt user key with master key.");
|
this.logService.warning("Failed to decrypt user key with master key.");
|
||||||
return null;
|
|
||||||
}
|
|
||||||
} else if (userKey.encryptionType === EncryptionType.AesCbc256_HmacSha256_B64) {
|
|
||||||
try {
|
|
||||||
const newKey = await this.keyGenerationService.stretchKey(masterKey);
|
|
||||||
decUserKey = await this.encryptService.unwrapSymmetricKey(userKey, newKey);
|
|
||||||
} catch {
|
|
||||||
this.logService.warning("Failed to decrypt user key with stretched master key.");
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
throw new Error("Unsupported encryption type.");
|
|
||||||
}
|
|
||||||
|
|
||||||
if (decUserKey == null) {
|
|
||||||
this.logService.warning("Failed to decrypt user key with master key, user key is null.");
|
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
return decUserKey as UserKey;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
async makeMasterPasswordAuthenticationData(
|
async makeMasterPasswordAuthenticationData(
|
||||||
|
|||||||
Reference in New Issue
Block a user