From 98705e443f321411dd3b7c8d54581fe4462cc3c5 Mon Sep 17 00:00:00 2001 From: Federico Maccaroni Date: Tue, 13 Jun 2023 22:38:08 +0200 Subject: [PATCH] PM-2575 Fixed extension freeze when using the return button on the keyboard when unlocking the extension. Also added way to prevent multiple executions of checking the password and logging exceptions. (#2568) --- src/App/Resources/AppResources.Designer.cs | 2 +- src/App/Resources/AppResources.resx | 2 +- .../LockPasswordViewController.cs | 3 +- .../BaseLockPasswordViewController.cs | 176 ++++++++++-------- 4 files changed, 99 insertions(+), 84 deletions(-) diff --git a/src/App/Resources/AppResources.Designer.cs b/src/App/Resources/AppResources.Designer.cs index 51a6cfb8c..f4bf3ff61 100644 --- a/src/App/Resources/AppResources.Designer.cs +++ b/src/App/Resources/AppResources.Designer.cs @@ -6435,7 +6435,7 @@ namespace Bit.App.Resources { } /// - /// Looks up a localized string similar to Unlocking may fail due to insufficient memory. Decrease your KDF memory settings to resolve. + /// Looks up a localized string similar to Unlocking may fail due to insufficient memory. Decrease your KDF memory settings to resolve.. /// public static string UnlockingMayFailDueToInsufficientMemoryDecreaseYourKDFMemorySettingsToResolve { get { diff --git a/src/App/Resources/AppResources.resx b/src/App/Resources/AppResources.resx index f4bae0727..8fa9d4375 100644 --- a/src/App/Resources/AppResources.resx +++ b/src/App/Resources/AppResources.resx @@ -2635,6 +2635,6 @@ Do you want to switch to this account? Master password re-prompt help - Unlocking may fail due to insufficient memory. Decrease your KDF memory settings to resolve + Unlocking may fail due to insufficient memory. Decrease your KDF memory settings to resolve. diff --git a/src/iOS.Autofill/LockPasswordViewController.cs b/src/iOS.Autofill/LockPasswordViewController.cs index a2cad5568..7f5df4c7e 100644 --- a/src/iOS.Autofill/LockPasswordViewController.cs +++ b/src/iOS.Autofill/LockPasswordViewController.cs @@ -1,5 +1,6 @@ using System; using Bit.App.Controls; +using Bit.Core.Utilities; using Bit.iOS.Core.Utilities; using UIKit; @@ -44,7 +45,7 @@ namespace Bit.iOS.Autofill partial void SubmitButton_Activated(UIBarButtonItem sender) { - var task = CheckPasswordAsync(); + CheckPasswordAsync().FireAndForget(); } partial void CancelButton_Activated(UIBarButtonItem sender) diff --git a/src/iOS.Core/Controllers/BaseLockPasswordViewController.cs b/src/iOS.Core/Controllers/BaseLockPasswordViewController.cs index db0a946c2..0edccc732 100644 --- a/src/iOS.Core/Controllers/BaseLockPasswordViewController.cs +++ b/src/iOS.Core/Controllers/BaseLockPasswordViewController.cs @@ -36,6 +36,7 @@ namespace Bit.iOS.Core.Controllers private bool _passwordReprompt = false; private bool _usesKeyConnector; private bool _biometricUnlockOnly = false; + private bool _checkingPassword; protected bool autofillExtension = false; @@ -154,7 +155,7 @@ namespace Bit.iOS.Core.Controllers MasterPasswordCell.TextField.ReturnKeyType = UIReturnKeyType.Go; MasterPasswordCell.TextField.ShouldReturn += (UITextField tf) => { - CheckPasswordAsync().GetAwaiter().GetResult(); + CheckPasswordAsync().FireAndForget(); return true; }; if (_pinLock) @@ -208,108 +209,121 @@ namespace Bit.iOS.Core.Controllers MasterPasswordCell.TextField.BecomeFirstResponder(); } } - + protected async Task CheckPasswordAsync() { - if (string.IsNullOrWhiteSpace(MasterPasswordCell.TextField.Text)) - { - var alert = Dialogs.CreateAlert(AppResources.AnErrorHasOccurred, - string.Format(AppResources.ValidationFieldRequired, - _pinLock ? AppResources.PIN : AppResources.MasterPassword), - AppResources.Ok); - PresentViewController(alert, true, null); - return; - } - - var email = await _stateService.GetEmailAsync(); - var kdfConfig = await _stateService.GetActiveUserCustomDataAsync(a => new KdfConfig(a?.Profile)); - var inputtedValue = MasterPasswordCell.TextField.Text; - - // HACK: iOS extensions have constrained memory, given how it works Argon2Id, it's likely to crash - // the extension depending on the argon2id memory configured. - // So, we warn the user and advise to decrease the configured memory letting them the option to continue, if wanted. - if (kdfConfig.Type == KdfType.Argon2id - && - kdfConfig.Memory > Constants.MaximumArgon2IdMemoryBeforeExtensionCrashing - && - !await _platformUtilsService.ShowDialogAsync(AppResources.UnlockingMayFailDueToInsufficientMemoryDecreaseYourKDFMemorySettingsToResolve, AppResources.Warning, AppResources.Continue, AppResources.Cancel)) + if (_checkingPassword) { return; } + _checkingPassword = true; - if (_pinLock) + try { - var failed = true; - try + if (string.IsNullOrWhiteSpace(MasterPasswordCell.TextField.Text)) { - if (_isPinProtected) + var alert = Dialogs.CreateAlert(AppResources.AnErrorHasOccurred, + string.Format(AppResources.ValidationFieldRequired, + _pinLock ? AppResources.PIN : AppResources.MasterPassword), + AppResources.Ok); + PresentViewController(alert, true, null); + return; + } + + var email = await _stateService.GetEmailAsync(); + var kdfConfig = await _stateService.GetActiveUserCustomDataAsync(a => new KdfConfig(a?.Profile)); + var inputtedValue = MasterPasswordCell.TextField.Text; + + // HACK: iOS extensions have constrained memory, given how it works Argon2Id, it's likely to crash + // the extension depending on the argon2id memory configured. + // So, we warn the user and advise to decrease the configured memory letting them the option to continue, if wanted. + if (kdfConfig.Type == KdfType.Argon2id + && + kdfConfig.Memory > Constants.MaximumArgon2IdMemoryBeforeExtensionCrashing + && + !await _platformUtilsService.ShowDialogAsync(AppResources.UnlockingMayFailDueToInsufficientMemoryDecreaseYourKDFMemorySettingsToResolve, AppResources.Warning, AppResources.Continue, AppResources.Cancel)) + { + return; + } + + if (_pinLock) + { + var failed = true; + try { - var key = await _cryptoService.MakeKeyFromPinAsync(inputtedValue, email, - kdfConfig, - await _stateService.GetPinProtectedKeyAsync()); - var encKey = await _cryptoService.GetEncKeyAsync(key); - var protectedPin = await _stateService.GetProtectedPinAsync(); - var decPin = await _cryptoService.DecryptToUtf8Async(new EncString(protectedPin), encKey); - failed = decPin != inputtedValue; - if (!failed) + if (_isPinProtected) { + var key = await _cryptoService.MakeKeyFromPinAsync(inputtedValue, email, + kdfConfig, + await _stateService.GetPinProtectedKeyAsync()); + var encKey = await _cryptoService.GetEncKeyAsync(key); + var protectedPin = await _stateService.GetProtectedPinAsync(); + var decPin = await _cryptoService.DecryptToUtf8Async(new EncString(protectedPin), encKey); + failed = decPin != inputtedValue; + if (!failed) + { + await AppHelpers.ResetInvalidUnlockAttemptsAsync(); + await SetKeyAndContinueAsync(key); + } + } + else + { + var key2 = await _cryptoService.MakeKeyFromPinAsync(inputtedValue, email, + kdfConfig); + failed = false; await AppHelpers.ResetInvalidUnlockAttemptsAsync(); - await SetKeyAndContinueAsync(key); + await SetKeyAndContinueAsync(key2); } } - else + catch { - var key2 = await _cryptoService.MakeKeyFromPinAsync(inputtedValue, email, - kdfConfig); - failed = false; - await AppHelpers.ResetInvalidUnlockAttemptsAsync(); - await SetKeyAndContinueAsync(key2); + failed = true; } - } - catch - { - failed = true; - } - if (failed) - { - await HandleFailedCredentialsAsync(); - } - } - else - { - var key2 = await _cryptoService.MakeKeyAsync(inputtedValue, email, kdfConfig); - - var storedKeyHash = await _cryptoService.GetKeyHashAsync(); - if (storedKeyHash == null) - { - var oldKey = await _secureStorageService.GetAsync("oldKey"); - if (key2.KeyB64 == oldKey) + if (failed) { - var localKeyHash = await _cryptoService.HashPasswordAsync(inputtedValue, key2, HashPurpose.LocalAuthorization); - await _secureStorageService.RemoveAsync("oldKey"); - await _cryptoService.SetKeyHashAsync(localKeyHash); + await HandleFailedCredentialsAsync(); } } - var passwordValid = await _cryptoService.CompareAndUpdateKeyHashAsync(inputtedValue, key2); - if (passwordValid) - { - if (_isPinProtected) - { - var protectedPin = await _stateService.GetProtectedPinAsync(); - var encKey = await _cryptoService.GetEncKeyAsync(key2); - var decPin = await _cryptoService.DecryptToUtf8Async(new EncString(protectedPin), encKey); - var pinKey = await _cryptoService.MakePinKeyAysnc(decPin, email, - kdfConfig); - await _stateService.SetPinProtectedKeyAsync(await _cryptoService.EncryptAsync(key2.Key, pinKey)); - } - await AppHelpers.ResetInvalidUnlockAttemptsAsync(); - await SetKeyAndContinueAsync(key2, true); - } else { - await HandleFailedCredentialsAsync(); + var key2 = await _cryptoService.MakeKeyAsync(inputtedValue, email, kdfConfig); + + var storedKeyHash = await _cryptoService.GetKeyHashAsync(); + if (storedKeyHash == null) + { + var oldKey = await _secureStorageService.GetAsync("oldKey"); + if (key2.KeyB64 == oldKey) + { + var localKeyHash = await _cryptoService.HashPasswordAsync(inputtedValue, key2, HashPurpose.LocalAuthorization); + await _secureStorageService.RemoveAsync("oldKey"); + await _cryptoService.SetKeyHashAsync(localKeyHash); + } + } + var passwordValid = await _cryptoService.CompareAndUpdateKeyHashAsync(inputtedValue, key2); + if (passwordValid) + { + if (_isPinProtected) + { + var protectedPin = await _stateService.GetProtectedPinAsync(); + var encKey = await _cryptoService.GetEncKeyAsync(key2); + var decPin = await _cryptoService.DecryptToUtf8Async(new EncString(protectedPin), encKey); + var pinKey = await _cryptoService.MakePinKeyAysnc(decPin, email, + kdfConfig); + await _stateService.SetPinProtectedKeyAsync(await _cryptoService.EncryptAsync(key2.Key, pinKey)); + } + await AppHelpers.ResetInvalidUnlockAttemptsAsync(); + await SetKeyAndContinueAsync(key2, true); + } + else + { + await HandleFailedCredentialsAsync(); + } } } + finally + { + _checkingPassword = false; + } } private async Task HandleFailedCredentialsAsync()