From 6b72f86e7b38e70eb17cd6abebc7ba24456b5ee6 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Sun, 9 Apr 2023 11:28:18 +0200 Subject: [PATCH] Mobile: Security: Prevent bypassing fingerprint lock on certain devices --- .eslintignore | 1 + .gitignore | 1 + .../components/biometrics/BiometricPopup.tsx | 18 ++++----- .../biometrics/biometricAuthenticate.ts | 28 +++++++++++++ .../components/biometrics/sensorInfo.ts | 12 ++++++ .../components/screens/ConfigScreen.tsx | 39 ++++++++++++++++--- packages/app-mobile/root.tsx | 2 +- 7 files changed, 83 insertions(+), 18 deletions(-) create mode 100644 packages/app-mobile/components/biometrics/biometricAuthenticate.ts diff --git a/.eslintignore b/.eslintignore index 62b4cdd85..d9e75125f 100644 --- a/.eslintignore +++ b/.eslintignore @@ -405,6 +405,7 @@ packages/app-mobile/components/SideMenu.js packages/app-mobile/components/TextInput.js packages/app-mobile/components/app-nav.js packages/app-mobile/components/biometrics/BiometricPopup.js +packages/app-mobile/components/biometrics/biometricAuthenticate.js packages/app-mobile/components/biometrics/sensorInfo.js packages/app-mobile/components/getResponsiveValue.js packages/app-mobile/components/getResponsiveValue.test.js diff --git a/.gitignore b/.gitignore index 07ac8254d..231023227 100644 --- a/.gitignore +++ b/.gitignore @@ -392,6 +392,7 @@ packages/app-mobile/components/SideMenu.js packages/app-mobile/components/TextInput.js packages/app-mobile/components/app-nav.js packages/app-mobile/components/biometrics/BiometricPopup.js +packages/app-mobile/components/biometrics/biometricAuthenticate.js packages/app-mobile/components/biometrics/sensorInfo.js packages/app-mobile/components/getResponsiveValue.js packages/app-mobile/components/getResponsiveValue.test.js diff --git a/packages/app-mobile/components/biometrics/BiometricPopup.tsx b/packages/app-mobile/components/biometrics/BiometricPopup.tsx index 2c700e5f9..1d5b19ab9 100644 --- a/packages/app-mobile/components/biometrics/BiometricPopup.tsx +++ b/packages/app-mobile/components/biometrics/BiometricPopup.tsx @@ -2,10 +2,10 @@ const React = require('react'); import Setting from '@joplin/lib/models/Setting'; import { useEffect, useMemo, useState } from 'react'; import { View, Dimensions, Alert, Button } from 'react-native'; -import FingerprintScanner from 'react-native-fingerprint-scanner'; import { SensorInfo } from './sensorInfo'; import { _ } from '@joplin/lib/locale'; import Logger from '@joplin/lib/Logger'; +import biometricAuthenticate from './biometricAuthenticate'; const logger = Logger.create('BiometricPopup'); @@ -21,7 +21,7 @@ export default (props: Props) => { // doesn't work properly, we disable it. We only want the user to enable the // feature after they've read the description in the config screen. const [initialPromptDone, setInitialPromptDone] = useState(true); // useState(Setting.value('security.biometricsInitialPromptDone')); - const [display, setDisplay] = useState(!!props.sensorInfo.supportedSensors && (props.sensorInfo.enabled || !initialPromptDone)); + const [display, setDisplay] = useState(props.sensorInfo.enabled || !initialPromptDone); const [tryBiometricsCheck, setTryBiometricsCheck] = useState(initialPromptDone); logger.info('Render start'); @@ -37,18 +37,14 @@ export default (props: Props) => { logger.info('biometricsCheck: start'); try { - logger.info('biometricsCheck: authenticate...'); - await FingerprintScanner.authenticate({ description: _('Verify your identity') }); - logger.info('biometricsCheck: authenticate done'); - setTryBiometricsCheck(false); + await biometricAuthenticate(); setDisplay(false); } catch (error) { - Alert.alert(_('Could not verify your identify'), error.message); - setTryBiometricsCheck(false); - } finally { - FingerprintScanner.release(); + Alert.alert(error.message); } + setTryBiometricsCheck(false); + logger.info('biometricsCheck: end'); }; @@ -97,7 +93,7 @@ export default (props: Props) => { }, ] ); - }, [initialPromptDone, props.sensorInfo.supportedSensors, display, props.dispatch]); + }, [initialPromptDone, display, props.dispatch]); const windowSize = useMemo(() => { return { diff --git a/packages/app-mobile/components/biometrics/biometricAuthenticate.ts b/packages/app-mobile/components/biometrics/biometricAuthenticate.ts new file mode 100644 index 000000000..fffb4c318 --- /dev/null +++ b/packages/app-mobile/components/biometrics/biometricAuthenticate.ts @@ -0,0 +1,28 @@ +import Logger from '@joplin/lib/Logger'; +import FingerprintScanner, { Errors } from 'react-native-fingerprint-scanner'; +import { _ } from '@joplin/lib/locale'; + +const logger = Logger.create('biometricAuthenticate'); + +export default async () => { + try { + logger.info('Authenticate...'); + await FingerprintScanner.authenticate({ description: _('Verify your identity') }); + logger.info('Authenticate done'); + } catch (error) { + const errorName = (error as Errors).name; + + let errorMessage = error.message; + if (errorName === 'FingerprintScannerNotEnrolled' || errorName === 'FingerprintScannerNotAvailable') { + errorMessage = _('Biometric unlock is not setup on the device. Please set it up in order to unlock Joplin. If the device is on lockout, consider switching it off and on to reset biometrics scanning.'); + } + + error.message = _('Could not verify your identify: %s', errorMessage); + + logger.warn(error); + + throw error; + } finally { + FingerprintScanner.release(); + } +}; diff --git a/packages/app-mobile/components/biometrics/sensorInfo.ts b/packages/app-mobile/components/biometrics/sensorInfo.ts index 2b4e2f12d..fc780ebe7 100644 --- a/packages/app-mobile/components/biometrics/sensorInfo.ts +++ b/packages/app-mobile/components/biometrics/sensorInfo.ts @@ -30,6 +30,18 @@ export default async (): Promise => { try { logger.info('Getting isSensorAvailable...'); + + // Note: If `isSensorAvailable()` doesn't return anything, it seems we + // could assume that biometrics are not setup on the device, and thus we + // can unlock the app. However that's not always correct - on some + // devices (eg Galaxy S22), `isSensorAvailable()` will return nothing if + // the device is on lockout - i.e. if the user gave the wrong + // fingerprint multiple times. + // + // So we definitely can't unlock the app in that case, and it means + // `isSensorAvailable()` is pretty much useless. Instead we ask for + // fingerprint when the user turns on the feature and at that point we + // know if the device supports biometrics or not. const result = await FingerprintScanner.isSensorAvailable(); logger.info('isSensorAvailable result', result); supportedSensors = result; diff --git a/packages/app-mobile/components/screens/ConfigScreen.tsx b/packages/app-mobile/components/screens/ConfigScreen.tsx index b51cc5f42..7f74dbdd0 100644 --- a/packages/app-mobile/components/screens/ConfigScreen.tsx +++ b/packages/app-mobile/components/screens/ConfigScreen.tsx @@ -23,6 +23,7 @@ const { themeStyle } = require('../global-style.js'); const shared = require('@joplin/lib/components/shared/config-shared.js'); import SyncTargetRegistry from '@joplin/lib/SyncTargetRegistry'; import { openDocumentTree } from '@joplin/react-native-saf-x'; +import biometricAuthenticate from '../biometrics/biometricAuthenticate'; class ConfigScreenComponent extends BaseScreenComponent { public static navigationOptions(): any { @@ -463,7 +464,7 @@ class ConfigScreenComponent extends BaseScreenComponent { {label} - updateSettingValue(key, value)} /> + void updateSettingValue(key, value)} /> {descriptionComp} @@ -474,13 +475,39 @@ class ConfigScreenComponent extends BaseScreenComponent { return !hasDescription ? this.styles().settingContainer : this.styles().settingContainerNoBottomBorder; } + private async handleSetting(key: string, value: any): Promise { + // When the user tries to enable biometrics unlock, we ask for the + // fingerprint or Face ID, and if it's correct we save immediately. If + // it's not, we don't turn on the setting. + if (key === 'security.biometricsEnabled' && !!value) { + try { + await biometricAuthenticate(); + shared.updateSettingValue(this, key, value); + await this.saveButton_press(); + } catch (error) { + shared.updateSettingValue(this, key, false); + Alert.alert(error.message); + } + return true; + } + + if (key === 'security.biometricsEnabled' && !value) { + shared.updateSettingValue(this, key, value); + await this.saveButton_press(); + return true; + } + + return false; + } + public settingToComponent(key: string, value: any) { const themeId = this.props.themeId; const theme = themeStyle(themeId); const output: any = null; - const updateSettingValue = (key: string, value: any) => { - return shared.updateSettingValue(this, key, value); + const updateSettingValue = async (key: string, value: any) => { + const handled = await this.handleSetting(key, value); + if (!handled) shared.updateSettingValue(this, key, value); }; const md = Setting.settingMetadata(key); @@ -517,7 +544,7 @@ class ConfigScreenComponent extends BaseScreenComponent { fontSize: theme.fontSize, }} onValueChange={(itemValue: string) => { - updateSettingValue(key, itemValue); + void updateSettingValue(key, itemValue); }} /> @@ -553,7 +580,7 @@ class ConfigScreenComponent extends BaseScreenComponent { {unitLabel} - updateSettingValue(key, value)} /> + void updateSettingValue(key, value)} /> ); @@ -577,7 +604,7 @@ class ConfigScreenComponent extends BaseScreenComponent { {md.label()} - updateSettingValue(key, value)} secureTextEntry={!!md.secure} /> + void updateSettingValue(key, value)} secureTextEntry={!!md.secure} /> ); } else { diff --git a/packages/app-mobile/root.tsx b/packages/app-mobile/root.tsx index 541662163..0d0238cf5 100644 --- a/packages/app-mobile/root.tsx +++ b/packages/app-mobile/root.tsx @@ -501,7 +501,7 @@ async function initialize(dispatch: Function) { if (Setting.value('env') === 'prod') { await db.open({ name: getDatabaseName(currentProfile, isSubProfile) }); } else { - await db.open({ name: getDatabaseName(currentProfile, isSubProfile, '-1') }); + await db.open({ name: getDatabaseName(currentProfile, isSubProfile, '-3') }); // await db.clearForTesting(); }