From ae28de41590e9d8c732f2fa1f2d7a92403dae996 Mon Sep 17 00:00:00 2001 From: Oscar Hinton Date: Sun, 9 Aug 2020 03:33:49 +0200 Subject: [PATCH] Invalidate biometric on change (#1026) * Initial working version for Android * Add a fallback for when upgrading from older app version. * Ensure biometric validity is re-checked on focus * Only setup biometric integrity key if biometric is turned on. * Fix styling according to comments * Fallback for Android 5. * Improve comment * Add boilerplate for iOS * Change BiometricService to public * Untested iOS implementation. * Convert IBiometricService to async. Fix code style for iOS. * Base64 NSData. * Review comments for Android BiometricService. * Rename methods in BiometricService to append Async * Ensure we wait for async SetupBiometricAsync. * Update BiometricService.cs Co-authored-by: Kyle Spearrin --- src/Android/Android.csproj | 1 + src/Android/MainApplication.cs | 2 + src/Android/Services/BiometricService.cs | 90 +++++++++++++++++++ src/App/Pages/Accounts/LockPage.xaml | 6 +- src/App/Pages/Accounts/LockPageViewModel.cs | 21 ++++- .../SettingsPage/SettingsPageViewModel.cs | 3 + src/App/Resources/AppResources.en-GB.resx | 37 ++++---- src/App/Resources/AppResources.resx | 3 + src/Core/Abstractions/IBiometricService.cs | 10 +++ src/iOS.Core/Services/BiometricService.cs | 62 +++++++++++++ src/iOS.Core/Utilities/iOSCoreHelpers.cs | 2 + src/iOS.Core/iOS.Core.csproj | 1 + 12 files changed, 218 insertions(+), 20 deletions(-) create mode 100644 src/Android/Services/BiometricService.cs create mode 100644 src/Core/Abstractions/IBiometricService.cs create mode 100644 src/iOS.Core/Services/BiometricService.cs diff --git a/src/Android/Android.csproj b/src/Android/Android.csproj index fbaab9427db..bc28a165434 100644 --- a/src/Android/Android.csproj +++ b/src/Android/Android.csproj @@ -133,6 +133,7 @@ + diff --git a/src/Android/MainApplication.cs b/src/Android/MainApplication.cs index 358d8ed1e9c..262e006c555 100644 --- a/src/Android/MainApplication.cs +++ b/src/Android/MainApplication.cs @@ -98,6 +98,7 @@ private void RegisterLocalServices() broadcasterService, () => ServiceContainer.Resolve("eventService")); var platformUtilsService = new MobilePlatformUtilsService(deviceActionService, messagingService, broadcasterService); + var biometricService = new BiometricService(); ServiceContainer.Register("broadcasterService", broadcasterService); ServiceContainer.Register("messagingService", messagingService); @@ -108,6 +109,7 @@ private void RegisterLocalServices() ServiceContainer.Register("secureStorageService", secureStorageService); ServiceContainer.Register("deviceActionService", deviceActionService); ServiceContainer.Register("platformUtilsService", platformUtilsService); + ServiceContainer.Register("biometricService", biometricService); // Push #if FDROID diff --git a/src/Android/Services/BiometricService.cs b/src/Android/Services/BiometricService.cs new file mode 100644 index 00000000000..327bd5b5ee9 --- /dev/null +++ b/src/Android/Services/BiometricService.cs @@ -0,0 +1,90 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Android.OS; +using Android.Security.Keystore; +using Bit.Core.Abstractions; +using Java.Security; +using Javax.Crypto; + +namespace Bit.Droid.Services +{ + public class BiometricService : IBiometricService + { + private const string KeyName = "com.8bit.bitwarden.biometric_integrity"; + + private const string KeyStoreName = "AndroidKeyStore"; + + private const string KeyAlgorithm = KeyProperties.KeyAlgorithmAes; + private const string BlockMode = KeyProperties.BlockModeCbc; + private const string EncryptionPadding = KeyProperties.EncryptionPaddingPkcs7; + private const string Transformation = KeyAlgorithm + "/" + BlockMode + "/" + EncryptionPadding; + + private readonly KeyStore _keystore; + + public BiometricService() + { + _keystore = KeyStore.GetInstance(KeyStoreName); + _keystore.Load(null); + } + + public Task SetupBiometricAsync() + { + if (Build.VERSION.SdkInt >= BuildVersionCodes.M) + { + CreateKey(); + } + + return Task.FromResult(true); + } + + public Task ValidateIntegrityAsync() + { + if (Build.VERSION.SdkInt < BuildVersionCodes.M) + { + return Task.FromResult(true); + } + + _keystore.Load(null); + IKey key = _keystore.GetKey(KeyName, null); + Cipher cipher = Cipher.GetInstance(Transformation); + + try + { + cipher.Init(CipherMode.EncryptMode, key); + } + catch (KeyPermanentlyInvalidatedException e) + { + // Biometric has changed + return Task.FromResult(false); + } + catch (UnrecoverableKeyException e) + { + // Biometric was disabled and re-enabled + return Task.FromResult(false); + } + catch (InvalidKeyException e) + { + // Fallback for old bitwarden users without a key + CreateKey(); + } + + return Task.FromResult(false); + } + + private void CreateKey() + { + KeyGenerator keyGen = KeyGenerator.GetInstance(KeyAlgorithm, KeyStoreName); + KeyGenParameterSpec keyGenSpec = + new KeyGenParameterSpec.Builder(KeyName, KeyStorePurpose.Encrypt | KeyStorePurpose.Decrypt) + .SetBlockModes(BlockMode) + .SetEncryptionPaddings(EncryptionPadding) + .SetUserAuthenticationRequired(true) + .Build(); + keyGen.Init(keyGenSpec); + keyGen.GenerateKey(); + } + } +} diff --git a/src/App/Pages/Accounts/LockPage.xaml b/src/App/Pages/Accounts/LockPage.xaml index 66691b0f2de..9e3bbc53abb 100644 --- a/src/App/Pages/Accounts/LockPage.xaml +++ b/src/App/Pages/Accounts/LockPage.xaml @@ -106,8 +106,12 @@ Margin="0, 10, 0, 0" /> + diff --git a/src/App/Pages/Accounts/LockPageViewModel.cs b/src/App/Pages/Accounts/LockPageViewModel.cs index 198f5b7327c..fec02b33d0d 100644 --- a/src/App/Pages/Accounts/LockPageViewModel.cs +++ b/src/App/Pages/Accounts/LockPageViewModel.cs @@ -24,11 +24,13 @@ public class LockPageViewModel : BaseViewModel private readonly IStorageService _secureStorageService; private readonly IEnvironmentService _environmentService; private readonly IStateService _stateService; + private readonly IBiometricService _biometricService; private string _email; private bool _showPassword; private bool _pinLock; private bool _biometricLock; + private bool _biometricIntegrityValid = true; private string _biometricButtonText; private string _loggedInAsText; private string _lockedVerifyText; @@ -47,6 +49,7 @@ public LockPageViewModel() _secureStorageService = ServiceContainer.Resolve("secureStorageService"); _environmentService = ServiceContainer.Resolve("environmentService"); _stateService = ServiceContainer.Resolve("stateService"); + _biometricService = ServiceContainer.Resolve("biometricService"); PageTitle = AppResources.VerifyMasterPassword; TogglePasswordCommand = new Command(TogglePassword); @@ -75,6 +78,12 @@ public bool BiometricLock set => SetProperty(ref _biometricLock, value); } + public bool BiometricIntegrityValid + { + get => _biometricIntegrityValid; + set => SetProperty(ref _biometricIntegrityValid, value); + } + public string BiometricButtonText { get => _biometricButtonText; @@ -133,7 +142,8 @@ public async Task InitAsync(bool autoPromptBiometric) BiometricButtonText = supportsFace ? AppResources.UseFaceIDToUnlock : AppResources.UseFingerprintToUnlock; } - if (autoPromptBiometric) + BiometricIntegrityValid = await _biometricService.ValidateIntegrityAsync(); + if (autoPromptBiometric & _biometricIntegrityValid) { var tasks = Task.Run(async () => { @@ -238,6 +248,12 @@ public async Task SubmitAsync() } MasterPassword = string.Empty; await SetKeyAndContinueAsync(key); + + // Re-enable biometrics + if (BiometricLock & !BiometricIntegrityValid) + { + await _biometricService.SetupBiometricAsync(); + } } else { @@ -267,7 +283,8 @@ public void TogglePassword() public async Task PromptBiometricAsync() { - if (!BiometricLock) + BiometricIntegrityValid = await _biometricService.ValidateIntegrityAsync(); + if (!BiometricLock || !BiometricIntegrityValid) { return; } diff --git a/src/App/Pages/Settings/SettingsPage/SettingsPageViewModel.cs b/src/App/Pages/Settings/SettingsPage/SettingsPageViewModel.cs index 10a1e7bbf99..3656b54650f 100644 --- a/src/App/Pages/Settings/SettingsPage/SettingsPageViewModel.cs +++ b/src/App/Pages/Settings/SettingsPage/SettingsPageViewModel.cs @@ -22,6 +22,7 @@ public class SettingsPageViewModel : BaseViewModel private readonly IVaultTimeoutService _vaultTimeoutService; private readonly IStorageService _storageService; private readonly ISyncService _syncService; + private readonly IBiometricService _biometricService; private bool _supportsBiometric; private bool _pin; @@ -60,6 +61,7 @@ public SettingsPageViewModel() _vaultTimeoutService = ServiceContainer.Resolve("vaultTimeoutService"); _storageService = ServiceContainer.Resolve("storageService"); _syncService = ServiceContainer.Resolve("syncService"); + _biometricService = ServiceContainer.Resolve("biometricService"); GroupedItems = new ExtendedObservableCollection(); PageTitle = AppResources.Settings; @@ -306,6 +308,7 @@ public async Task UpdateBiometricAsync() } if (_biometric) { + await _biometricService.SetupBiometricAsync(); await _storageService.SaveAsync(Constants.BiometricUnlockKey, true); } else diff --git a/src/App/Resources/AppResources.en-GB.resx b/src/App/Resources/AppResources.en-GB.resx index 5823bc24251..4b062406961 100644 --- a/src/App/Resources/AppResources.en-GB.resx +++ b/src/App/Resources/AppResources.en-GB.resx @@ -59,46 +59,46 @@ : using a System.ComponentModel.TypeConverter : and then encoded with base64 encoding. --> - - + + - + - - - - + + + + - - + + - - + + - - - - + + + + - + - + @@ -1674,4 +1674,7 @@ Do you really want to send to the bin? Confirmation alert message when soft-deleting a cipher. + + Biometric change detected, login using Master Password to enable again. + \ No newline at end of file diff --git a/src/App/Resources/AppResources.resx b/src/App/Resources/AppResources.resx index e0faf47d4d8..6fd7f97febb 100644 --- a/src/App/Resources/AppResources.resx +++ b/src/App/Resources/AppResources.resx @@ -1674,6 +1674,9 @@ Do you really want to send to the trash? Confirmation alert message when soft-deleting a cipher. + + Biometric change detected, login using Master Password to enable again. + Enable sync on refresh diff --git a/src/Core/Abstractions/IBiometricService.cs b/src/Core/Abstractions/IBiometricService.cs new file mode 100644 index 00000000000..5e77a632945 --- /dev/null +++ b/src/Core/Abstractions/IBiometricService.cs @@ -0,0 +1,10 @@ +using System.Threading.Tasks; + +namespace Bit.Core.Abstractions +{ + public interface IBiometricService + { + Task SetupBiometricAsync(); + Task ValidateIntegrityAsync(); + } +} diff --git a/src/iOS.Core/Services/BiometricService.cs b/src/iOS.Core/Services/BiometricService.cs new file mode 100644 index 00000000000..6e61ed77db7 --- /dev/null +++ b/src/iOS.Core/Services/BiometricService.cs @@ -0,0 +1,62 @@ +using System.Threading.Tasks; +using Bit.Core.Abstractions; +using Foundation; +using LocalAuthentication; + +namespace Bit.iOS.Core.Services +{ + public class BiometricService : IBiometricService + { + private IStorageService _storageService; + + public BiometricService(IStorageService storageService) + { + _storageService = storageService; + } + + public async Task SetupBiometricAsync() + { + var state = GetState(); + await _storageService.SaveAsync("biometricState", ToBase64(state)); + + return true; + } + + public async Task ValidateIntegrityAsync() + { + var oldState = await _storageService.GetAsync("biometricState"); + if (oldState == null) + { + // Fallback for upgraded devices + await SetupBiometricAsync(); + + return true; + } + else + { + var state = GetState(); + + return FromBase64(oldState) == state; + } + } + + private NSData GetState() + { + var context = new LAContext(); + context.CanEvaluatePolicy(LAPolicy.DeviceOwnerAuthenticationWithBiometrics, out _); + + return context.EvaluatedPolicyDomainState; + } + + private string ToBase64(NSData data) + { + return System.Convert.ToBase64String(data.ToArray()); + } + + private NSData FromBase64(string data) + { + var bytes = System.Convert.FromBase64String(data); + return NSData.FromArray(bytes); + } + } +} diff --git a/src/iOS.Core/Utilities/iOSCoreHelpers.cs b/src/iOS.Core/Utilities/iOSCoreHelpers.cs index 86501255bcd..47acc2d8318 100644 --- a/src/iOS.Core/Utilities/iOSCoreHelpers.cs +++ b/src/iOS.Core/Utilities/iOSCoreHelpers.cs @@ -55,6 +55,7 @@ public static void RegisterLocalServices() var deviceActionService = new DeviceActionService(mobileStorageService, messagingService); var platformUtilsService = new MobilePlatformUtilsService(deviceActionService, messagingService, broadcasterService); + var biometricService = new BiometricService(mobileStorageService); ServiceContainer.Register("broadcasterService", broadcasterService); ServiceContainer.Register("messagingService", messagingService); @@ -65,6 +66,7 @@ public static void RegisterLocalServices() ServiceContainer.Register("secureStorageService", secureStorageService); ServiceContainer.Register("deviceActionService", deviceActionService); ServiceContainer.Register("platformUtilsService", platformUtilsService); + ServiceContainer.Register("biometricService", biometricService); } public static void Bootstrap(Func postBootstrapFunc = null) diff --git a/src/iOS.Core/iOS.Core.csproj b/src/iOS.Core/iOS.Core.csproj index 9f00e5daa8f..6f067c1c6d4 100644 --- a/src/iOS.Core/iOS.Core.csproj +++ b/src/iOS.Core/iOS.Core.csproj @@ -163,6 +163,7 @@ +