Remove old biometrics masterkey logic (#9943)

This commit is contained in:
Bernd Schoolmann 2024-07-22 15:40:19 +02:00 committed by GitHub
parent 7d5df0209c
commit 457c0795be
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 3 additions and 168 deletions

View File

@ -1026,8 +1026,6 @@ export default class MainBackground {
this.accountService,
);
this.nativeMessagingBackground = new NativeMessagingBackground(
this.accountService,
this.masterPasswordService,
this.cryptoService,
this.cryptoFunctionService,
this.runtimeBackground,

View File

@ -1,8 +1,6 @@
import { firstValueFrom } from "rxjs";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service";
@ -15,7 +13,7 @@ import { BiometricStateService } from "@bitwarden/common/platform/biometrics/bio
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { EncString } from "@bitwarden/common/platform/models/domain/enc-string";
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
import { UserKey, MasterKey } from "@bitwarden/common/types/key";
import { UserKey } from "@bitwarden/common/types/key";
import { BrowserApi } from "../platform/browser/browser-api";
@ -73,8 +71,6 @@ export class NativeMessagingBackground {
private validatingFingerprint: boolean;
constructor(
private accountService: AccountService,
private masterPasswordService: InternalMasterPasswordServiceAbstraction,
private cryptoService: CryptoService,
private cryptoFunctionService: CryptoFunctionService,
private runtimeBackground: RuntimeBackground,
@ -355,27 +351,6 @@ export class NativeMessagingBackground {
Utils.fromB64ToArray(message.userKeyB64),
) as UserKey;
await this.cryptoService.setUserKey(userKey);
} else if (message.keyB64) {
const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
// Backwards compatibility to support cases in which the user hasn't updated their desktop app
// TODO: Remove after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3472)
const encUserKeyPrim = await this.stateService.getEncryptedCryptoSymmetricKey();
const encUserKey =
encUserKeyPrim != null
? new EncString(encUserKeyPrim)
: await this.masterPasswordService.getMasterKeyEncryptedUserKey(userId);
if (!encUserKey) {
throw new Error("No encrypted user key found");
}
const masterKey = new SymmetricCryptoKey(
Utils.fromB64ToArray(message.keyB64),
) as MasterKey;
const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(
masterKey,
encUserKey,
);
await this.masterPasswordService.setMasterKey(masterKey, userId);
await this.cryptoService.setUserKey(userKey);
} else {
throw new Error("No key received");
}

View File

@ -133,12 +133,6 @@ class SafariWebExtensionHandler: NSObject, NSExtensionRequestHandling {
status = SecKeychainFindGenericPassword(nil, UInt32(ServiceNameBiometric.utf8.count), ServiceNameBiometric, UInt32(fallbackName.utf8.count), fallbackName, &passwordLength, &passwordPtr, nil)
}
// TODO: Remove after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3473)
if status != errSecSuccess {
let secondaryFallbackName = "_masterkey_biometric"
status = SecKeychainFindGenericPassword(nil, UInt32(ServiceNameBiometric.utf8.count), ServiceNameBiometric, UInt32(secondaryFallbackName.utf8.count), secondaryFallbackName, &passwordLength, &passwordPtr, nil)
}
if status == errSecSuccess {
let result = NSString(bytes: passwordPtr!, length: Int(passwordLength), encoding: String.Encoding.utf8.rawValue) as String?
SecKeychainItemFreeContent(nil, passwordPtr)

View File

@ -109,14 +109,6 @@ describe("electronCryptoService", () => {
userId: mockUserId,
});
});
it("clears the old deprecated Biometric key whenever a User Key is set", async () => {
await sut.setUserKey(mockUserKey, mockUserId);
expect(stateService.setCryptoMasterKeyBiometric).toHaveBeenCalledWith(null, {
userId: mockUserId,
});
});
});
});
});

View File

@ -13,13 +13,12 @@ import { StateService } from "@bitwarden/common/platform/abstractions/state.serv
import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service";
import { KeySuffixOptions } from "@bitwarden/common/platform/enums";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { EncString } from "@bitwarden/common/platform/models/domain/enc-string";
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
import { CryptoService } from "@bitwarden/common/platform/services/crypto.service";
import { StateProvider } from "@bitwarden/common/platform/state";
import { CsprngString } from "@bitwarden/common/types/csprng";
import { UserId } from "@bitwarden/common/types/guid";
import { UserKey, MasterKey } from "@bitwarden/common/types/key";
import { UserKey } from "@bitwarden/common/types/key";
export class ElectronCryptoService extends CryptoService {
constructor(
@ -53,9 +52,7 @@ export class ElectronCryptoService extends CryptoService {
override async hasUserKeyStored(keySuffix: KeySuffixOptions, userId?: UserId): Promise<boolean> {
if (keySuffix === KeySuffixOptions.Biometric) {
// TODO: Remove after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3474)
const oldKey = await this.stateService.hasCryptoMasterKeyBiometric({ userId: userId });
return oldKey || (await this.stateService.hasUserKeyBiometric({ userId: userId }));
return await this.stateService.hasUserKeyBiometric({ userId: userId });
}
return super.hasUserKeyStored(keySuffix, userId);
}
@ -90,7 +87,6 @@ export class ElectronCryptoService extends CryptoService {
userId?: UserId,
): Promise<UserKey> {
if (keySuffix === KeySuffixOptions.Biometric) {
await this.migrateBiometricKeyIfNeeded(userId);
const userKey = await this.stateService.getUserKeyBiometric({ userId: userId });
return userKey == null
? null
@ -149,46 +145,4 @@ export class ElectronCryptoService extends CryptoService {
return biometricKey;
}
// --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)
override async clearDeprecatedKeys(keySuffix: KeySuffixOptions, userId?: UserId) {
if (keySuffix === KeySuffixOptions.Biometric) {
await this.stateService.setCryptoMasterKeyBiometric(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
super.clearDeprecatedKeys(keySuffix, userId);
}
private async migrateBiometricKeyIfNeeded(userId?: UserId) {
if (await this.stateService.hasCryptoMasterKeyBiometric({ userId })) {
const oldBiometricKey = await this.stateService.getCryptoMasterKeyBiometric({ userId });
// decrypt
const masterKey = new SymmetricCryptoKey(Utils.fromB64ToArray(oldBiometricKey)) as MasterKey;
userId ??= (await firstValueFrom(this.accountService.activeAccount$))?.id;
const encUserKeyPrim = await this.stateService.getEncryptedCryptoSymmetricKey({
userId: userId,
});
const encUserKey =
encUserKeyPrim != null
? new EncString(encUserKeyPrim)
: await this.masterPasswordService.getMasterKeyEncryptedUserKey(userId);
if (!encUserKey) {
throw new Error("No user key found during biometric migration");
}
const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(
masterKey,
encUserKey,
userId,
);
// migrate
await this.storeBiometricKey(userKey, userId);
await this.stateService.setCryptoMasterKeyBiometric(null, { userId });
}
}
}

View File

@ -3,7 +3,6 @@ import { firstValueFrom, map } from "rxjs";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { MasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service";
import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service";
@ -34,7 +33,6 @@ export class NativeMessagingService {
private sharedSecrets = new Map<string, SymmetricCryptoKey>();
constructor(
private masterPasswordService: MasterPasswordServiceAbstraction,
private cryptoFunctionService: CryptoFunctionService,
private cryptoService: CryptoService,
private platformUtilService: PlatformUtilsService,
@ -177,19 +175,12 @@ export class NativeMessagingService {
KeySuffixOptions.Biometric,
message.userId,
);
const masterKey = await firstValueFrom(
this.masterPasswordService.masterKey$(message.userId as UserId),
);
if (userKey != null) {
// we send the master key still for backwards compatibility
// with older browser extensions
// TODO: Remove after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3472)
await this.send(
{
command: "biometricUnlock",
response: "unlocked",
keyB64: masterKey?.keyB64,
userKeyB64: userKey.keyB64,
},
appId,

View File

@ -405,9 +405,6 @@ export class PinService implements PinServiceAbstraction {
await this.clearOldPinKeyEncryptedMasterKey(userId);
// This also clears the old Biometrics key since the new Biometrics key will be created when the user key is set.
await this.stateService.setCryptoMasterKeyBiometric(null, { userId: userId });
return userKey;
}

View File

@ -455,8 +455,6 @@ describe("PinService", () => {
await sut.setUserKeyEncryptedPin(mockUserKeyEncryptedPin, mockUserId);
await sut.clearOldPinKeyEncryptedMasterKey(mockUserId);
await stateService.setCryptoMasterKeyBiometric(null, { userId: mockUserId });
}
function mockDecryptUserKeyFn() {

View File

@ -56,18 +56,6 @@ export abstract class StateService<T extends Account = Account> {
* @deprecated For migration purposes only, use setUserKeyAuto instead
*/
setCryptoMasterKeyAuto: (value: string, options?: StorageOptions) => Promise<void>;
/**
* @deprecated For migration purposes only, use getUserKeyBiometric instead
*/
getCryptoMasterKeyBiometric: (options?: StorageOptions) => Promise<string>;
/**
* @deprecated For migration purposes only, use hasUserKeyBiometric instead
*/
hasCryptoMasterKeyBiometric: (options?: StorageOptions) => Promise<boolean>;
/**
* @deprecated For migration purposes only, use setUserKeyBiometric instead
*/
setCryptoMasterKeyBiometric: (value: BiometricKey, options?: StorageOptions) => Promise<void>;
getDuckDuckGoSharedKey: (options?: StorageOptions) => Promise<string>;
setDuckDuckGoSharedKey: (value: string, options?: StorageOptions) => Promise<void>;
getIsAuthenticated: (options?: StorageOptions) => Promise<boolean>;

View File

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

View File

@ -33,7 +33,6 @@ const partialKeys = {
userBiometricKey: "_user_biometric",
autoKey: "_masterkey_auto",
biometricKey: "_masterkey_biometric",
masterKey: "_masterkey",
};
@ -254,54 +253,6 @@ export class StateService<
await this.saveSecureStorageKey(partialKeys.masterKey, value, options);
}
/**
* @deprecated Use UserKeyBiometric instead
*/
async getCryptoMasterKeyBiometric(options?: StorageOptions): Promise<string> {
options = this.reconcileOptions(
this.reconcileOptions(options, { keySuffix: "biometric" }),
await this.defaultSecureStorageOptions(),
);
if (options?.userId == null) {
return null;
}
return await this.secureStorageService.get<string>(
`${options.userId}${partialKeys.biometricKey}`,
options,
);
}
/**
* @deprecated Use UserKeyBiometric instead
*/
async hasCryptoMasterKeyBiometric(options?: StorageOptions): Promise<boolean> {
options = this.reconcileOptions(
this.reconcileOptions(options, { keySuffix: "biometric" }),
await this.defaultSecureStorageOptions(),
);
if (options?.userId == null) {
return false;
}
return await this.secureStorageService.has(
`${options.userId}${partialKeys.biometricKey}`,
options,
);
}
/**
* @deprecated Use UserKeyBiometric instead
*/
async setCryptoMasterKeyBiometric(value: BiometricKey, options?: StorageOptions): Promise<void> {
options = this.reconcileOptions(
this.reconcileOptions(options, { keySuffix: "biometric" }),
await this.defaultSecureStorageOptions(),
);
if (options?.userId == null) {
return;
}
await this.saveSecureStorageKey(partialKeys.biometricKey, value, options);
}
async getDuckDuckGoSharedKey(options?: StorageOptions): Promise<string> {
options = this.reconcileOptions(options, await this.defaultSecureStorageOptions());
if (options?.userId == null) {
@ -678,7 +629,6 @@ export class StateService<
await this.setUserKeyAutoUnlock(null, { userId: userId });
await this.setUserKeyBiometric(null, { userId: userId });
await this.setCryptoMasterKeyAuto(null, { userId: userId });
await this.setCryptoMasterKeyBiometric(null, { userId: userId });
await this.setCryptoMasterKeyB64(null, { userId: userId });
}