mirror of
https://github.com/bitwarden/browser
synced 2026-01-05 18:13:26 +00:00
[PM-10741] Refactor biometrics interface & add dynamic status (#10973)
This commit is contained in:
@@ -2,18 +2,12 @@
|
||||
// @ts-strict-ignore
|
||||
import { ipcMain } from "electron";
|
||||
|
||||
import { BiometricKey } from "@bitwarden/common/auth/types/biometric-key";
|
||||
import { ConsoleLogService } from "@bitwarden/common/platform/services/console-log.service";
|
||||
import { passwords } from "@bitwarden/desktop-napi";
|
||||
|
||||
import { DesktopBiometricsService } from "../../key-management/biometrics/index";
|
||||
|
||||
const AuthRequiredSuffix = "_biometric";
|
||||
|
||||
export class DesktopCredentialStorageListener {
|
||||
constructor(
|
||||
private serviceName: string,
|
||||
private biometricService: DesktopBiometricsService,
|
||||
private logService: ConsoleLogService,
|
||||
) {}
|
||||
|
||||
@@ -54,13 +48,7 @@ export class DesktopCredentialStorageListener {
|
||||
|
||||
// Gracefully handle old keytar values, and if detected updated the entry to the proper format
|
||||
private async getPassword(serviceName: string, key: string, keySuffix: string) {
|
||||
let val: string;
|
||||
// todo: remove this when biometrics has been migrated to desktop_native
|
||||
if (keySuffix === AuthRequiredSuffix) {
|
||||
val = (await this.biometricService.getBiometricKey(serviceName, key)) ?? null;
|
||||
} else {
|
||||
val = await passwords.getPassword(serviceName, key);
|
||||
}
|
||||
const val = await passwords.getPassword(serviceName, key);
|
||||
|
||||
try {
|
||||
JSON.parse(val);
|
||||
@@ -72,25 +60,10 @@ export class DesktopCredentialStorageListener {
|
||||
}
|
||||
|
||||
private async setPassword(serviceName: string, key: string, value: string, keySuffix: string) {
|
||||
if (keySuffix === AuthRequiredSuffix) {
|
||||
const valueObj = JSON.parse(value) as BiometricKey;
|
||||
await this.biometricService.setEncryptionKeyHalf({
|
||||
service: serviceName,
|
||||
key,
|
||||
value: valueObj?.clientEncKeyHalf,
|
||||
});
|
||||
// Value is usually a JSON string, but we need to pass the key half as well, so we re-stringify key here.
|
||||
await this.biometricService.setBiometricKey(serviceName, key, JSON.stringify(valueObj?.key));
|
||||
} else {
|
||||
await passwords.setPassword(serviceName, key, value);
|
||||
}
|
||||
await passwords.setPassword(serviceName, key, value);
|
||||
}
|
||||
|
||||
private async deletePassword(serviceName: string, key: string, keySuffix: string) {
|
||||
if (keySuffix === AuthRequiredSuffix) {
|
||||
await this.biometricService.deleteBiometricKey(serviceName, key);
|
||||
} else {
|
||||
await passwords.deletePassword(serviceName, key);
|
||||
}
|
||||
await passwords.deletePassword(serviceName, key);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -87,6 +87,7 @@ const nativeMessaging = {
|
||||
},
|
||||
sendMessage: (message: {
|
||||
appId: string;
|
||||
messageId?: number;
|
||||
command?: string;
|
||||
sharedSecret?: string;
|
||||
message?: EncString;
|
||||
|
||||
@@ -1,115 +0,0 @@
|
||||
// FIXME: Update this file to be type safe and remove this and next line
|
||||
// @ts-strict-ignore
|
||||
import { FakeStateProvider } from "@bitwarden/common/../spec/fake-state-provider";
|
||||
import { mock } from "jest-mock-extended";
|
||||
|
||||
import { PinServiceAbstraction } from "@bitwarden/auth/common";
|
||||
import { FakeMasterPasswordService } from "@bitwarden/common/auth/services/master-password/fake-master-password.service";
|
||||
import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service";
|
||||
import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service";
|
||||
import { KeyGenerationService } from "@bitwarden/common/platform/abstractions/key-generation.service";
|
||||
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
|
||||
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
|
||||
import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
|
||||
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
|
||||
import { makeEncString } from "@bitwarden/common/spec";
|
||||
import { CsprngArray } from "@bitwarden/common/types/csprng";
|
||||
import { UserId } from "@bitwarden/common/types/guid";
|
||||
import { UserKey } from "@bitwarden/common/types/key";
|
||||
import { KdfConfigService, BiometricStateService } from "@bitwarden/key-management";
|
||||
|
||||
import {
|
||||
FakeAccountService,
|
||||
mockAccountServiceWith,
|
||||
} from "../../../../../libs/common/spec/fake-account-service";
|
||||
|
||||
import { ElectronKeyService } from "./electron-key.service";
|
||||
|
||||
describe("electronKeyService", () => {
|
||||
let sut: ElectronKeyService;
|
||||
|
||||
const pinService = mock<PinServiceAbstraction>();
|
||||
const keyGenerationService = mock<KeyGenerationService>();
|
||||
const cryptoFunctionService = mock<CryptoFunctionService>();
|
||||
const encryptService = mock<EncryptService>();
|
||||
const platformUtilService = mock<PlatformUtilsService>();
|
||||
const logService = mock<LogService>();
|
||||
const stateService = mock<StateService>();
|
||||
let masterPasswordService: FakeMasterPasswordService;
|
||||
let accountService: FakeAccountService;
|
||||
let stateProvider: FakeStateProvider;
|
||||
const biometricStateService = mock<BiometricStateService>();
|
||||
const kdfConfigService = mock<KdfConfigService>();
|
||||
|
||||
const mockUserId = "mock user id" as UserId;
|
||||
|
||||
beforeEach(() => {
|
||||
accountService = mockAccountServiceWith("userId" as UserId);
|
||||
masterPasswordService = new FakeMasterPasswordService();
|
||||
stateProvider = new FakeStateProvider(accountService);
|
||||
|
||||
sut = new ElectronKeyService(
|
||||
pinService,
|
||||
masterPasswordService,
|
||||
keyGenerationService,
|
||||
cryptoFunctionService,
|
||||
encryptService,
|
||||
platformUtilService,
|
||||
logService,
|
||||
stateService,
|
||||
accountService,
|
||||
stateProvider,
|
||||
biometricStateService,
|
||||
kdfConfigService,
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
jest.resetAllMocks();
|
||||
});
|
||||
|
||||
describe("setUserKey", () => {
|
||||
let mockUserKey: UserKey;
|
||||
|
||||
beforeEach(() => {
|
||||
const mockRandomBytes = new Uint8Array(64) as CsprngArray;
|
||||
mockUserKey = new SymmetricCryptoKey(mockRandomBytes) as UserKey;
|
||||
});
|
||||
|
||||
describe("Biometric Key refresh", () => {
|
||||
const encClientKeyHalf = makeEncString();
|
||||
const decClientKeyHalf = "decrypted client key half";
|
||||
|
||||
beforeEach(() => {
|
||||
encClientKeyHalf.decrypt = jest.fn().mockResolvedValue(decClientKeyHalf);
|
||||
});
|
||||
|
||||
it("sets a Biometric key if getBiometricUnlock is true and the platform supports secure storage", async () => {
|
||||
biometricStateService.getBiometricUnlockEnabled.mockResolvedValue(true);
|
||||
platformUtilService.supportsSecureStorage.mockReturnValue(true);
|
||||
biometricStateService.getRequirePasswordOnStart.mockResolvedValue(true);
|
||||
biometricStateService.getEncryptedClientKeyHalf.mockResolvedValue(encClientKeyHalf);
|
||||
|
||||
await sut.setUserKey(mockUserKey, mockUserId);
|
||||
|
||||
expect(stateService.setUserKeyBiometric).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ key: expect.any(String), clientEncKeyHalf: decClientKeyHalf }),
|
||||
{
|
||||
userId: mockUserId,
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("clears the Biometric key if getBiometricUnlock is false or the platform does not support secure storage", async () => {
|
||||
biometricStateService.getBiometricUnlockEnabled.mockResolvedValue(true);
|
||||
platformUtilService.supportsSecureStorage.mockReturnValue(false);
|
||||
|
||||
await sut.setUserKey(mockUserKey, mockUserId);
|
||||
|
||||
expect(stateService.setUserKeyBiometric).toHaveBeenCalledWith(null, {
|
||||
userId: mockUserId,
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1,7 +1,5 @@
|
||||
// FIXME: Update this file to be type safe and remove this and next line
|
||||
// @ts-strict-ignore
|
||||
import { firstValueFrom } from "rxjs";
|
||||
|
||||
import { PinServiceAbstraction } from "@bitwarden/auth/common";
|
||||
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
|
||||
import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction";
|
||||
@@ -13,7 +11,6 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl
|
||||
import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
|
||||
import { KeySuffixOptions } from "@bitwarden/common/platform/enums";
|
||||
import { Utils } from "@bitwarden/common/platform/misc/utils";
|
||||
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
|
||||
import { StateProvider } from "@bitwarden/common/platform/state";
|
||||
import { CsprngString } from "@bitwarden/common/types/csprng";
|
||||
import { UserId } from "@bitwarden/common/types/guid";
|
||||
@@ -24,6 +21,8 @@ import {
|
||||
BiometricStateService,
|
||||
} from "@bitwarden/key-management";
|
||||
|
||||
import { DesktopBiometricsService } from "src/key-management/biometrics/desktop.biometrics.service";
|
||||
|
||||
export class ElectronKeyService extends DefaultKeyService {
|
||||
constructor(
|
||||
pinService: PinServiceAbstraction,
|
||||
@@ -38,6 +37,7 @@ export class ElectronKeyService extends DefaultKeyService {
|
||||
stateProvider: StateProvider,
|
||||
private biometricStateService: BiometricStateService,
|
||||
kdfConfigService: KdfConfigService,
|
||||
private biometricService: DesktopBiometricsService,
|
||||
) {
|
||||
super(
|
||||
pinService,
|
||||
@@ -55,19 +55,10 @@ export class ElectronKeyService extends DefaultKeyService {
|
||||
}
|
||||
|
||||
override async hasUserKeyStored(keySuffix: KeySuffixOptions, userId?: UserId): Promise<boolean> {
|
||||
if (keySuffix === KeySuffixOptions.Biometric) {
|
||||
return await this.stateService.hasUserKeyBiometric({ userId: userId });
|
||||
}
|
||||
return super.hasUserKeyStored(keySuffix, userId);
|
||||
}
|
||||
|
||||
override async clearStoredUserKey(keySuffix: KeySuffixOptions, userId?: UserId): Promise<void> {
|
||||
if (keySuffix === KeySuffixOptions.Biometric) {
|
||||
await this.stateService.setUserKeyBiometric(null, { userId: userId });
|
||||
await this.biometricStateService.removeEncryptedClientKeyHalf(userId);
|
||||
await this.clearDeprecatedKeys(KeySuffixOptions.Biometric, userId);
|
||||
return;
|
||||
}
|
||||
// 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
|
||||
await super.clearStoredUserKey(keySuffix, userId);
|
||||
@@ -76,52 +67,35 @@ export class ElectronKeyService extends DefaultKeyService {
|
||||
protected override async storeAdditionalKeys(key: UserKey, userId: UserId) {
|
||||
await super.storeAdditionalKeys(key, userId);
|
||||
|
||||
const storeBiometricKey = await this.shouldStoreKey(KeySuffixOptions.Biometric, userId);
|
||||
|
||||
if (storeBiometricKey) {
|
||||
await this.storeBiometricKey(key, userId);
|
||||
} else {
|
||||
await this.stateService.setUserKeyBiometric(null, { userId: userId });
|
||||
if (await this.biometricStateService.getBiometricUnlockEnabled(userId)) {
|
||||
await this.storeBiometricsProtectedUserKey(key, userId);
|
||||
}
|
||||
await this.clearDeprecatedKeys(KeySuffixOptions.Biometric, userId);
|
||||
}
|
||||
|
||||
protected override async getKeyFromStorage(
|
||||
keySuffix: KeySuffixOptions,
|
||||
userId?: UserId,
|
||||
): Promise<UserKey> {
|
||||
if (keySuffix === KeySuffixOptions.Biometric) {
|
||||
const userKey = await this.stateService.getUserKeyBiometric({ userId: userId });
|
||||
return userKey == null
|
||||
? null
|
||||
: (new SymmetricCryptoKey(Utils.fromB64ToArray(userKey)) as UserKey);
|
||||
}
|
||||
return await super.getKeyFromStorage(keySuffix, userId);
|
||||
}
|
||||
|
||||
protected async storeBiometricKey(key: UserKey, userId?: UserId): Promise<void> {
|
||||
protected async storeBiometricsProtectedUserKey(
|
||||
userKey: UserKey,
|
||||
userId?: UserId,
|
||||
): Promise<void> {
|
||||
// May resolve to null, in which case no client key have is required
|
||||
const clientEncKeyHalf = await this.getBiometricEncryptionClientKeyHalf(key, userId);
|
||||
await this.stateService.setUserKeyBiometric(
|
||||
{ key: key.keyB64, clientEncKeyHalf },
|
||||
{ userId: userId },
|
||||
);
|
||||
// TODO: Move to windows implementation
|
||||
const clientEncKeyHalf = await this.getBiometricEncryptionClientKeyHalf(userKey, userId);
|
||||
await this.biometricService.setClientKeyHalfForUser(userId, clientEncKeyHalf);
|
||||
await this.biometricService.setBiometricProtectedUnlockKeyForUser(userId, userKey.keyB64);
|
||||
}
|
||||
|
||||
protected async shouldStoreKey(keySuffix: KeySuffixOptions, userId?: UserId): Promise<boolean> {
|
||||
if (keySuffix === KeySuffixOptions.Biometric) {
|
||||
const biometricUnlockPromise =
|
||||
userId == null
|
||||
? firstValueFrom(this.biometricStateService.biometricUnlockEnabled$)
|
||||
: this.biometricStateService.getBiometricUnlockEnabled(userId);
|
||||
const biometricUnlock = await biometricUnlockPromise;
|
||||
return biometricUnlock && this.platformUtilService.supportsSecureStorage();
|
||||
}
|
||||
return await super.shouldStoreKey(keySuffix, userId);
|
||||
}
|
||||
|
||||
protected override async clearAllStoredUserKeys(userId?: UserId): Promise<void> {
|
||||
await this.clearStoredUserKey(KeySuffixOptions.Biometric, userId);
|
||||
await this.biometricService.deleteBiometricUnlockKeyForUser(userId);
|
||||
await super.clearAllStoredUserKeys(userId);
|
||||
}
|
||||
|
||||
@@ -135,18 +109,18 @@ export class ElectronKeyService extends DefaultKeyService {
|
||||
}
|
||||
|
||||
// Retrieve existing key half if it exists
|
||||
let biometricKey = await this.biometricStateService
|
||||
let clientKeyHalf = await this.biometricStateService
|
||||
.getEncryptedClientKeyHalf(userId)
|
||||
.then((result) => result?.decrypt(null /* user encrypted */, userKey))
|
||||
.then((result) => result as CsprngString);
|
||||
if (biometricKey == null && userKey != null) {
|
||||
if (clientKeyHalf == null && userKey != null) {
|
||||
// Set a key half if it doesn't exist
|
||||
const keyBytes = await this.cryptoFunctionService.randomBytes(32);
|
||||
biometricKey = Utils.fromBufferToUtf8(keyBytes) as CsprngString;
|
||||
const encKey = await this.encryptService.encrypt(biometricKey, userKey);
|
||||
clientKeyHalf = Utils.fromBufferToUtf8(keyBytes) as CsprngString;
|
||||
const encKey = await this.encryptService.encrypt(clientKeyHalf, userKey);
|
||||
await this.biometricStateService.setEncryptedClientKeyHalf(encKey, userId);
|
||||
}
|
||||
|
||||
return biometricKey;
|
||||
return clientKeyHalf;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user