From 32e462bdce76af58e83c95a6c569c3b4e99c508c Mon Sep 17 00:00:00 2001 From: Alexander Bakker Date: Wed, 1 Mar 2023 18:01:25 +0100 Subject: [PATCH] Load vault file on demand instead of juggling it around in-memory This trades performance for making VaultManager a bit easier to reason about. This also fixes a rare crash that could occur if the user retries to unlock the app after the previous attempt resulted in an error related to parsing the vault. The vault file would no longer be present in memory after the first attempt, causing the second attempt to crash the app. --- .../aegis/PanicTriggerTest.java | 7 +- .../aegis/ui/AuthActivity.java | 15 ++-- .../aegis/ui/IntroActivity.java | 13 ++- .../aegis/ui/MainActivity.java | 30 ++++++- .../aegis/vault/VaultManager.java | 81 +------------------ 5 files changed, 53 insertions(+), 93 deletions(-) diff --git a/app/src/androidTest/java/com/beemdevelopment/aegis/PanicTriggerTest.java b/app/src/androidTest/java/com/beemdevelopment/aegis/PanicTriggerTest.java index 4a16356c..39feca57 100644 --- a/app/src/androidTest/java/com/beemdevelopment/aegis/PanicTriggerTest.java +++ b/app/src/androidTest/java/com/beemdevelopment/aegis/PanicTriggerTest.java @@ -1,7 +1,6 @@ package com.beemdevelopment.aegis; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; @@ -32,11 +31,10 @@ public class PanicTriggerTest extends AegisTest { @Test public void testPanicTriggerDisabled() { assertFalse(_prefs.isPanicTriggerEnabled()); + assertTrue(_vaultManager.isVaultLoaded()); launchPanic(); assertTrue(_vaultManager.isVaultLoaded()); _vaultManager.getVault(); - assertFalse(_vaultManager.isVaultFileLoaded()); - assertNull(_vaultManager.getVaultFileError()); assertTrue(VaultRepository.fileExists(getApp())); } @@ -44,11 +42,10 @@ public class PanicTriggerTest extends AegisTest { public void testPanicTriggerEnabled() { _prefs.setIsPanicTriggerEnabled(true); assertTrue(_prefs.isPanicTriggerEnabled()); + assertTrue(_vaultManager.isVaultLoaded()); launchPanic(); assertFalse(_vaultManager.isVaultLoaded()); assertThrows(IllegalStateException.class, () -> _vaultManager.getVault()); - assertFalse(_vaultManager.isVaultFileLoaded()); - assertNull(_vaultManager.getVaultFileError()); assertFalse(VaultRepository.fileExists(getApp())); } diff --git a/app/src/main/java/com/beemdevelopment/aegis/ui/AuthActivity.java b/app/src/main/java/com/beemdevelopment/aegis/ui/AuthActivity.java index 685aa812..246a0ff6 100644 --- a/app/src/main/java/com/beemdevelopment/aegis/ui/AuthActivity.java +++ b/app/src/main/java/com/beemdevelopment/aegis/ui/AuthActivity.java @@ -36,6 +36,7 @@ import com.beemdevelopment.aegis.ui.dialogs.Dialogs; import com.beemdevelopment.aegis.ui.tasks.PasswordSlotDecryptTask; import com.beemdevelopment.aegis.vault.VaultFile; import com.beemdevelopment.aegis.vault.VaultFileCredentials; +import com.beemdevelopment.aegis.vault.VaultRepository; import com.beemdevelopment.aegis.vault.VaultRepositoryException; import com.beemdevelopment.aegis.vault.slots.BiometricSlot; import com.beemdevelopment.aegis.vault.slots.PasswordSlot; @@ -55,7 +56,9 @@ public class AuthActivity extends AegisActivity { private EditText _textPassword; + private VaultFile _vaultFile; private SlotList _slots; + private SecretKey _bioKey; private BiometricSlot _bioSlot; private BiometricPrompt _bioPrompt; @@ -104,17 +107,17 @@ public class AuthActivity extends AegisActivity { _inhibitBioPrompt = savedInstanceState.getBoolean("inhibitBioPrompt", false); } - if (_vaultManager.getVaultFileError() != null) { - Dialogs.showErrorDialog(this, R.string.vault_load_error, _vaultManager.getVaultFileError(), (dialog, which) -> { + try { + _vaultFile = VaultRepository.readVaultFile(this); + } catch (VaultRepositoryException e) { + Dialogs.showErrorDialog(this, R.string.vault_load_error, e, (dialog, which) -> { getOnBackPressedDispatcher().onBackPressed(); }); return; } - VaultFile vaultFile = _vaultManager.getVaultFile(); - _slots = vaultFile.getHeader().getSlots(); - // only show the biometric prompt if the api version is new enough, permission is granted, a scanner is found and a biometric slot is found + _slots = _vaultFile.getHeader().getSlots(); if (_slots.has(BiometricSlot.class) && BiometricsHelper.isAvailable(this)) { boolean invalidated = false; @@ -287,7 +290,7 @@ public class AuthActivity extends AegisActivity { VaultFileCredentials creds = new VaultFileCredentials(key, _slots); try { - _vaultManager.unlock(creds); + _vaultManager.loadFrom(_vaultFile, creds); if (isSlotRepaired) { saveAndBackupVault(); } diff --git a/app/src/main/java/com/beemdevelopment/aegis/ui/IntroActivity.java b/app/src/main/java/com/beemdevelopment/aegis/ui/IntroActivity.java index 8ab26fe8..74532a3e 100644 --- a/app/src/main/java/com/beemdevelopment/aegis/ui/IntroActivity.java +++ b/app/src/main/java/com/beemdevelopment/aegis/ui/IntroActivity.java @@ -21,7 +21,9 @@ import com.beemdevelopment.aegis.ui.slides.DoneSlide; import com.beemdevelopment.aegis.ui.slides.SecurityPickerSlide; import com.beemdevelopment.aegis.ui.slides.SecuritySetupSlide; import com.beemdevelopment.aegis.ui.slides.WelcomeSlide; +import com.beemdevelopment.aegis.vault.VaultFile; import com.beemdevelopment.aegis.vault.VaultFileCredentials; +import com.beemdevelopment.aegis.vault.VaultRepository; import com.beemdevelopment.aegis.vault.VaultRepositoryException; import com.beemdevelopment.aegis.vault.slots.BiometricSlot; import com.beemdevelopment.aegis.vault.slots.PasswordSlot; @@ -108,8 +110,17 @@ public class IntroActivity extends IntroBaseActivity { return; } } else { + VaultFile vaultFile; try { - _vaultManager.load(creds); + vaultFile = VaultRepository.readVaultFile(this); + } catch (VaultRepositoryException e) { + e.printStackTrace(); + Dialogs.showErrorDialog(this, R.string.vault_load_error, e); + return; + } + + try { + _vaultManager.loadFrom(vaultFile, creds); } catch (VaultRepositoryException e) { e.printStackTrace(); Dialogs.showErrorDialog(this, R.string.vault_load_error, e); diff --git a/app/src/main/java/com/beemdevelopment/aegis/ui/MainActivity.java b/app/src/main/java/com/beemdevelopment/aegis/ui/MainActivity.java index b68ef168..ad3f5fa7 100644 --- a/app/src/main/java/com/beemdevelopment/aegis/ui/MainActivity.java +++ b/app/src/main/java/com/beemdevelopment/aegis/ui/MainActivity.java @@ -55,6 +55,9 @@ import com.beemdevelopment.aegis.ui.tasks.QrDecodeTask; import com.beemdevelopment.aegis.ui.views.EntryListView; import com.beemdevelopment.aegis.util.TimeUtils; import com.beemdevelopment.aegis.vault.VaultEntry; +import com.beemdevelopment.aegis.vault.VaultFile; +import com.beemdevelopment.aegis.vault.VaultRepository; +import com.beemdevelopment.aegis.vault.VaultRepositoryException; import com.google.android.material.bottomsheet.BottomSheetDialog; import com.google.android.material.floatingactionbutton.FloatingActionButton; import com.google.common.base.Strings; @@ -663,9 +666,30 @@ public class MainActivity extends AegisActivity implements EntryListView.Listene return; } - if (!_vaultManager.isVaultLoaded() && !_vaultManager.isVaultFileLoaded()) { - Dialogs.showErrorDialog(this, R.string.vault_load_error, _vaultManager.getVaultFileError(), (dialog1, which) -> finish()); - return; + // If the vault is not loaded yet, try to load it now in case it's plain text + if (!_vaultManager.isVaultLoaded()) { + VaultFile vaultFile; + try { + vaultFile = VaultRepository.readVaultFile(this); + } catch (VaultRepositoryException e) { + e.printStackTrace(); + Dialogs.showErrorDialog(this, R.string.vault_load_error, e, (dialog, which) -> { + finish(); + }); + return; + } + + if (!vaultFile.isEncrypted()) { + try { + _vaultManager.loadFrom(vaultFile); + } catch (VaultRepositoryException e) { + e.printStackTrace(); + Dialogs.showErrorDialog(this, R.string.vault_load_error, e, (dialog, which) -> { + finish(); + }); + return; + } + } } if (!_vaultManager.isVaultLoaded()) { diff --git a/app/src/main/java/com/beemdevelopment/aegis/vault/VaultManager.java b/app/src/main/java/com/beemdevelopment/aegis/vault/VaultManager.java index 2f2218d6..3fbfd294 100644 --- a/app/src/main/java/com/beemdevelopment/aegis/vault/VaultManager.java +++ b/app/src/main/java/com/beemdevelopment/aegis/vault/VaultManager.java @@ -19,7 +19,6 @@ import com.beemdevelopment.aegis.services.NotificationService; import com.beemdevelopment.aegis.ui.dialogs.Dialogs; import java.io.File; -import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; import java.io.OutputStream; @@ -30,8 +29,6 @@ public class VaultManager { private final Context _context; private final Preferences _prefs; - private VaultFile _vaultFile; - private VaultRepositoryException _vaultFileError; private VaultRepository _repo; private final VaultBackupManager _backups; @@ -46,35 +43,11 @@ public class VaultManager { _backups = new VaultBackupManager(_context); _androidBackups = new BackupManager(context); _lockListeners = new ArrayList<>(); - loadVaultFile(); - } - - private void loadVaultFile() { - try { - _vaultFile = VaultRepository.readVaultFile(_context); - } catch (VaultRepositoryException e) { - if (!(e.getCause() instanceof FileNotFoundException)) { - _vaultFileError = e; - e.printStackTrace(); - } - } - - if (_vaultFile != null && !_vaultFile.isEncrypted()) { - try { - loadFrom(_vaultFile, null); - } catch (VaultRepositoryException e) { - e.printStackTrace(); - _vaultFile = null; - _vaultFileError = e; - } - } } /** * Initializes the vault repository with a new empty vault and the given creds. It can * only be called if isVaultLoaded() returns false. - * - * Calling this method removes the manager's internal reference to the raw vault file (if it had one). */ @NonNull public VaultRepository initNew(@Nullable VaultFileCredentials creds) throws VaultRepositoryException { @@ -82,8 +55,6 @@ public class VaultManager { throw new IllegalStateException("Vault manager is already initialized"); } - _vaultFile = null; - _vaultFileError = null; _repo = new VaultRepository(_context, new Vault(), creds); save(); @@ -97,8 +68,6 @@ public class VaultManager { /** * Initializes the vault repository by decrypting the given vaultFile with the given * creds. It can only be called if isVaultLoaded() returns false. - * - * Calling this method removes the manager's internal reference to the raw vault file (if it had one). */ @NonNull public VaultRepository loadFrom(@NonNull VaultFile vaultFile, @Nullable VaultFileCredentials creds) throws VaultRepositoryException { @@ -106,8 +75,6 @@ public class VaultManager { throw new IllegalStateException("Vault manager is already initialized"); } - _vaultFile = null; - _vaultFileError = null; _repo = VaultRepository.fromFile(_context, vaultFile, creds); if (getVault().isEncryptionEnabled()) { @@ -117,32 +84,9 @@ public class VaultManager { return getVault(); } - /** - * Initializes the vault repository by loading and decrypting the vault file stored in - * internal storage, with the given creds. It can only be called if isVaultLoaded() - * returns false. - * - * Calling this method removes the manager's internal reference to the raw vault file (if it had one). - */ @NonNull - public VaultRepository load(@Nullable VaultFileCredentials creds) throws VaultRepositoryException { - if (isVaultLoaded()) { - throw new IllegalStateException("Vault manager is already initialized"); - } - - loadVaultFile(); - if (isVaultLoaded()) { - return _repo; - } - - return loadFrom(getVaultFile(), creds); - } - - @NonNull - public VaultRepository unlock(@NonNull VaultFileCredentials creds) throws VaultRepositoryException { - VaultRepository repo = loadFrom(getVaultFile(), creds); - startNotificationService(); - return repo; + public VaultRepository loadFrom(@NonNull VaultFile vaultFile) throws VaultRepositoryException { + return loadFrom(vaultFile, null); } /** @@ -157,7 +101,6 @@ public class VaultManager { } stopNotificationService(); - loadVaultFile(); } public void enableEncryption(VaultFileCredentials creds) throws VaultRepositoryException { @@ -270,12 +213,8 @@ public class VaultManager { return _repo != null; } - public boolean isVaultFileLoaded() { - return _vaultFile != null; - } - public boolean isVaultInitNeeded() { - return !isVaultLoaded() && !isVaultFileLoaded() && getVaultFileError() == null; + return !isVaultLoaded() && !VaultRepository.fileExists(_context); } @NonNull @@ -287,20 +226,6 @@ public class VaultManager { return _repo; } - @NonNull - public VaultFile getVaultFile() { - if (_vaultFile == null) { - throw new IllegalStateException("Vault file is not in memory"); - } - - return _vaultFile; - } - - @Nullable - public VaultRepositoryException getVaultFileError() { - return _vaultFileError; - } - /** * Starts an external activity, temporarily blocks automatic lock of Aegis and * shows an error dialog if the target activity is not found.