From 62c8edff04f912a03150a33a17d7d7de7b8344f9 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 3 Mar 2021 17:41:17 +0100 Subject: [PATCH] Choose key loading workflow depending on vaultconfig's key ID and allow KeyLoadingComponent to decide itself, what exceptions it can handle --- .../org/cryptomator/common/vaults/Vault.java | 10 ++++- .../common/vaults/VaultModule.java | 17 ++++++++ .../ui/unlock/KeyLoadingComponent.java | 41 ++++++++++++++++++ .../ui/unlock/KeyLoadingModule.java | 43 +++++++++++++++++++ .../ui/unlock/UnlockComponent.java | 2 +- .../cryptomator/ui/unlock/UnlockModule.java | 3 +- .../cryptomator/ui/unlock/UnlockWorkflow.java | 30 ++++++------- .../MasterkeyFileLoadingComponent.java | 17 +++++--- .../MasterkeyFileLoadingContext.java | 26 ++++++++--- 9 files changed, 159 insertions(+), 30 deletions(-) create mode 100644 main/ui/src/main/java/org/cryptomator/ui/unlock/KeyLoadingComponent.java create mode 100644 main/ui/src/main/java/org/cryptomator/ui/unlock/KeyLoadingModule.java diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java index 30ebea9ec..93df3aa7f 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java @@ -17,6 +17,8 @@ import org.cryptomator.cryptofs.CryptoFileSystem; import org.cryptomator.cryptofs.CryptoFileSystemProperties; import org.cryptomator.cryptofs.CryptoFileSystemProperties.FileSystemFlags; import org.cryptomator.cryptofs.CryptoFileSystemProvider; +import org.cryptomator.cryptofs.VaultConfig; +import org.cryptomator.cryptofs.VaultConfig.UnverifiedVaultConfig; import org.cryptomator.cryptofs.common.Constants; import org.cryptomator.cryptofs.common.FileSystemCapabilityChecker; import org.cryptomator.cryptolib.api.CryptoException; @@ -66,13 +68,14 @@ public class Vault { private final BooleanBinding needsMigration; private final BooleanBinding unknownError; private final StringBinding accessPoint; + private final Optional unverifiedVaultConfig; private final BooleanBinding accessPointPresent; private final BooleanProperty showingStats; private volatile Volume volume; @Inject - Vault(VaultSettings vaultSettings, Provider volumeProvider, @DefaultMountFlags StringBinding defaultMountFlags, AtomicReference cryptoFileSystem, ObjectProperty state, @Named("lastKnownException") ObjectProperty lastKnownException, VaultStats stats) { + Vault(VaultSettings vaultSettings, Provider volumeProvider, @DefaultMountFlags StringBinding defaultMountFlags, AtomicReference cryptoFileSystem, ObjectProperty state, @Named("lastKnownException") ObjectProperty lastKnownException, VaultStats stats, Optional unverifiedVaultConfig) { this.vaultSettings = vaultSettings; this.volumeProvider = volumeProvider; this.defaultMountFlags = defaultMountFlags; @@ -80,6 +83,7 @@ public class Vault { this.state = state; this.lastKnownException = lastKnownException; this.stats = stats; + this.unverifiedVaultConfig = unverifiedVaultConfig; this.displayName = Bindings.createStringBinding(this::getDisplayName, vaultSettings.displayName()); this.displayablePath = Bindings.createStringBinding(this::getDisplayablePath, vaultSettings.path()); this.locked = Bindings.createBooleanBinding(this::isLocked, state); @@ -299,6 +303,10 @@ public class Vault { return stats; } + public Optional getUnverifiedVaultConfig() { + return unverifiedVaultConfig; + } + public Observable[] observables() { return new Observable[]{state}; } diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java index 56a2814cf..4bc592691 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java @@ -8,10 +8,12 @@ package org.cryptomator.common.vaults; import dagger.Module; import dagger.Provides; import org.apache.commons.lang3.SystemUtils; +import org.cryptomator.common.Constants; import org.cryptomator.common.settings.Settings; import org.cryptomator.common.settings.VaultSettings; import org.cryptomator.common.settings.VolumeImpl; import org.cryptomator.cryptofs.CryptoFileSystem; +import org.cryptomator.cryptofs.VaultConfig; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -24,9 +26,11 @@ import javafx.beans.property.ObjectProperty; import javafx.beans.property.ReadOnlyBooleanProperty; import javafx.beans.property.SimpleObjectProperty; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Optional; import java.util.concurrent.atomic.AtomicReference; @Module @@ -53,6 +57,19 @@ public class VaultModule { return new SimpleObjectProperty<>(initialErrorCause); } + @Provides + @PerVault + Optional provideUnverifiedVaultConfig(VaultSettings settings) { + Path vaultRoot = settings.path().get(); + Path configPath = vaultRoot.resolve(Constants.VAULTCONFIG_FILENAME); + try { + String token = Files.readString(configPath, StandardCharsets.US_ASCII); + return Optional.of(VaultConfig.decode(token)); + } catch (IOException e) { + return Optional.empty(); + } + } + @Provides public Volume provideVolume(Settings settings, WebDavVolume webDavVolume, FuseVolume fuseVolume, DokanyVolume dokanyVolume) { diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/KeyLoadingComponent.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/KeyLoadingComponent.java new file mode 100644 index 000000000..ec7e8d89b --- /dev/null +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/KeyLoadingComponent.java @@ -0,0 +1,41 @@ +package org.cryptomator.ui.unlock; + +import org.cryptomator.cryptolib.api.MasterkeyLoader; +import org.cryptomator.cryptolib.api.MasterkeyLoadingFailedException; + +@FunctionalInterface +public interface KeyLoadingComponent { + + /** + * @return A reusable masterkey loader, preconfigured with the vault of the current unlock process + * @throws MasterkeyLoadingFailedException If unable to provide the masterkey loader + */ + MasterkeyLoader masterkeyLoader() throws MasterkeyLoadingFailedException; + + /** + * Allows the component to try and recover from an exception thrown while loading a masterkey. + * + * @param exception An exception thrown either by {@link #masterkeyLoader()} or by the returned {@link MasterkeyLoader}. + * @return true if this component was able to handle the exception and another attempt should be made to load a masterkey + */ + default boolean recoverFromException(MasterkeyLoadingFailedException exception) { + return false; + } + + /** + * Release any ressources or do follow-up tasks after loading a key. + * + * @param unlockedSuccessfully true if successfully unlocked a vault with the loaded key + * @implNote This method might be invoked multiple times, depending on whether multiple attempts to load a key are started. + */ + default void cleanup(boolean unlockedSuccessfully) { + // no-op + } + + static KeyLoadingComponent exceptional(Exception exception) { + return () -> { + throw new MasterkeyLoadingFailedException("Can not load key", exception); + }; + } + +} diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/KeyLoadingModule.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/KeyLoadingModule.java new file mode 100644 index 000000000..98bd478d0 --- /dev/null +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/KeyLoadingModule.java @@ -0,0 +1,43 @@ +package org.cryptomator.ui.unlock; + +import dagger.Module; +import dagger.Provides; +import dagger.multibindings.IntoMap; +import dagger.multibindings.StringKey; +import org.cryptomator.common.vaults.Vault; +import org.cryptomator.cryptofs.VaultConfig.UnverifiedVaultConfig; +import org.cryptomator.ui.unlock.masterkeyfile.MasterkeyFileLoadingComponent; + +import javafx.stage.Stage; +import java.net.URI; +import java.util.Map; +import java.util.Optional; + +@Module(subcomponents = {MasterkeyFileLoadingComponent.class}) +abstract class KeyLoadingModule { + + @Provides + @UnlockScoped + static Optional provideKeyId(@UnlockWindow Vault vault) { + return vault.getUnverifiedVaultConfig().map(UnverifiedVaultConfig::getKeyId); + } + + @Provides + @UnlockScoped + static KeyLoadingComponent provideKeyLoaderProvider(Optional keyId, Map keyLoaderProviders) { + if (keyId.isEmpty()) { + return KeyLoadingComponent.exceptional(new IllegalArgumentException("No key id provided")); + } else { + String scheme = keyId.get().getScheme(); + return keyLoaderProviders.getOrDefault(scheme, KeyLoadingComponent.exceptional(new IllegalArgumentException("Unsupported key id " + scheme))); + } + } + + @Provides + @IntoMap + @StringKey("masterkeyfile") + static KeyLoadingComponent provideMasterkeyFileLoadingComponet(MasterkeyFileLoadingComponent.Builder compBuilder, @UnlockWindow Stage window, @UnlockWindow Vault vault) { + return compBuilder.unlockWindow(window).vault(vault).build(); + } + +} diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockComponent.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockComponent.java index 9c0338c5b..fee0e74a5 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockComponent.java +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockComponent.java @@ -16,7 +16,7 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; @UnlockScoped -@Subcomponent(modules = {UnlockModule.class}) +@Subcomponent(modules = {UnlockModule.class, KeyLoadingModule.class}) public interface UnlockComponent { ExecutorService defaultExecutorService(); diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java index 44ba6670a..f7e25dd4f 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java @@ -12,7 +12,6 @@ import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlLoaderFactory; import org.cryptomator.ui.common.FxmlScene; import org.cryptomator.ui.common.StageFactory; -import org.cryptomator.ui.unlock.masterkeyfile.MasterkeyFileLoadingComponent; import javax.inject.Named; import javax.inject.Provider; @@ -23,7 +22,7 @@ import java.util.Map; import java.util.Optional; import java.util.ResourceBundle; -@Module(subcomponents = {MasterkeyFileLoadingComponent.class}) +@Module abstract class UnlockModule { @Provides diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java index b9945a692..0438e50f0 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java @@ -7,13 +7,11 @@ import org.cryptomator.common.vaults.Vault; import org.cryptomator.common.vaults.VaultState; import org.cryptomator.common.vaults.Volume.VolumeException; import org.cryptomator.cryptolib.api.CryptoException; -import org.cryptomator.cryptolib.api.InvalidPassphraseException; -import org.cryptomator.cryptolib.common.MasterkeyFileAccess; +import org.cryptomator.cryptolib.api.MasterkeyLoadingFailedException; import org.cryptomator.ui.common.ErrorComponent; import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; import org.cryptomator.ui.common.VaultService; -import org.cryptomator.ui.unlock.masterkeyfile.MasterkeyFileLoadingComponent; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -43,17 +41,17 @@ public class UnlockWorkflow extends Task { private final Lazy successScene; private final Lazy invalidMountPointScene; private final ErrorComponent.Builder errorComponent; - private final MasterkeyFileLoadingComponent.Builder masterkeyFileLoadingComponent; + private final KeyLoadingComponent keyLoadingComp; @Inject - UnlockWorkflow(@UnlockWindow Stage window, @UnlockWindow Vault vault, VaultService vaultService, @FxmlScene(FxmlFile.UNLOCK_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.UNLOCK_INVALID_MOUNT_POINT) Lazy invalidMountPointScene, ErrorComponent.Builder errorComponent, MasterkeyFileLoadingComponent.Builder masterkeyFileLoadingComponent) { + UnlockWorkflow(@UnlockWindow Stage window, @UnlockWindow Vault vault, VaultService vaultService, @FxmlScene(FxmlFile.UNLOCK_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.UNLOCK_INVALID_MOUNT_POINT) Lazy invalidMountPointScene, ErrorComponent.Builder errorComponent, KeyLoadingComponent keyLoadingComp) { this.window = window; this.vault = vault; this.vaultService = vaultService; this.successScene = successScene; this.invalidMountPointScene = invalidMountPointScene; this.errorComponent = errorComponent; - this.masterkeyFileLoadingComponent = masterkeyFileLoadingComponent; + this.keyLoadingComp = keyLoadingComp; setOnFailed(event -> { Throwable throwable = event.getSource().getException(); @@ -68,8 +66,7 @@ public class UnlockWorkflow extends Task { @Override protected Boolean call() throws InterruptedException, IOException, VolumeException, InvalidMountPointException, CryptoException { try { - // TODO: allow unlock strategies other than MasterkeyFile-based eventually - attemptUnlockUsingMasterkeyFile(0, null); + attemptUnlock(); handleSuccess(); return true; } catch (UnlockCancelledException e) { @@ -78,17 +75,20 @@ public class UnlockWorkflow extends Task { } } - private void attemptUnlockUsingMasterkeyFile(int attempt, Exception previousError) throws IOException, VolumeException, InvalidMountPointException, CryptoException { - var fileLoadingComp = masterkeyFileLoadingComponent.unlockWindow(window).vault(vault).previousError(previousError).build(); + private void attemptUnlock() throws IOException, VolumeException, InvalidMountPointException, CryptoException { boolean success = false; try { - vault.unlock(fileLoadingComp.masterkeyLoader()); + vault.unlock(keyLoadingComp.masterkeyLoader()); success = true; - } catch (InvalidPassphraseException e) { - LOG.info("Unlock attempt #{} failed due to {}", attempt, e.getMessage()); - attemptUnlockUsingMasterkeyFile(attempt + 1, e); + } catch (MasterkeyLoadingFailedException e) { + if (keyLoadingComp.recoverFromException(e)) { + LOG.info("Unlock attempt threw {}. Reattempting...", e.getClass().getSimpleName()); + attemptUnlock(); + } else { + throw e; + } } finally { - fileLoadingComp.cleanup(success); + keyLoadingComp.cleanup(success); } } diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/masterkeyfile/MasterkeyFileLoadingComponent.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/masterkeyfile/MasterkeyFileLoadingComponent.java index 268e0237b..ab3252e28 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/unlock/masterkeyfile/MasterkeyFileLoadingComponent.java +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/masterkeyfile/MasterkeyFileLoadingComponent.java @@ -3,19 +3,29 @@ package org.cryptomator.ui.unlock.masterkeyfile; import dagger.BindsInstance; import dagger.Subcomponent; import org.cryptomator.common.vaults.Vault; +import org.cryptomator.cryptolib.api.MasterkeyLoadingFailedException; import org.cryptomator.cryptolib.common.MasterkeyFileLoader; +import org.cryptomator.ui.unlock.KeyLoadingComponent; -import javax.annotation.Nullable; import javafx.stage.Stage; @MasterkeyFileLoadingScoped @Subcomponent(modules = {MasterkeyFileLoadingModule.class}) -public interface MasterkeyFileLoadingComponent { +public interface MasterkeyFileLoadingComponent extends KeyLoadingComponent { MasterkeyFileLoadingFinisher finisher(); + MasterkeyFileLoadingContext context(); + + @Override MasterkeyFileLoader masterkeyLoader(); + @Override + default boolean recoverFromException(MasterkeyLoadingFailedException exception) { + return context().recoverFromException(exception); + } + + @Override default void cleanup(boolean unlockedSuccessfully) { finisher().cleanup(unlockedSuccessfully); } @@ -23,9 +33,6 @@ public interface MasterkeyFileLoadingComponent { @Subcomponent.Builder interface Builder { - @BindsInstance - Builder previousError(@Nullable Exception previousError); - @BindsInstance Builder vault(@MasterkeyFileLoading Vault vault); diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/masterkeyfile/MasterkeyFileLoadingContext.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/masterkeyfile/MasterkeyFileLoadingContext.java index 08f5bd155..298ba1d09 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/unlock/masterkeyfile/MasterkeyFileLoadingContext.java +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/masterkeyfile/MasterkeyFileLoadingContext.java @@ -2,6 +2,7 @@ package org.cryptomator.ui.unlock.masterkeyfile; import dagger.Lazy; import org.cryptomator.cryptolib.api.InvalidPassphraseException; +import org.cryptomator.cryptolib.api.MasterkeyLoadingFailedException; import org.cryptomator.cryptolib.common.MasterkeyFileLoaderContext; import org.cryptomator.ui.common.Animations; import org.cryptomator.ui.common.FxmlFile; @@ -11,7 +12,6 @@ import org.cryptomator.ui.unlock.UnlockCancelledException; import org.cryptomator.ui.unlock.masterkeyfile.MasterkeyFileLoadingModule.MasterkeyFileProvision; import org.cryptomator.ui.unlock.masterkeyfile.MasterkeyFileLoadingModule.PasswordEntry; -import javax.annotation.Nullable; import javax.inject.Inject; import javafx.application.Platform; import javafx.scene.Scene; @@ -22,7 +22,7 @@ import java.nio.file.Path; import java.util.concurrent.atomic.AtomicReference; @MasterkeyFileLoadingScoped -class MasterkeyFileLoadingContext implements MasterkeyFileLoaderContext { +public class MasterkeyFileLoadingContext implements MasterkeyFileLoaderContext { private final Stage window; private final Lazy passphraseEntryScene; @@ -31,10 +31,11 @@ class MasterkeyFileLoadingContext implements MasterkeyFileLoaderContext { private final UserInteractionLock masterkeyFileProvisionLock; private final AtomicReference password; private final AtomicReference filePath; - private final Exception previousError; + + private boolean wrongPassword; @Inject - public MasterkeyFileLoadingContext(@MasterkeyFileLoading Stage window, @FxmlScene(FxmlFile.UNLOCK_ENTER_PASSWORD) Lazy passphraseEntryScene, @FxmlScene(FxmlFile.UNLOCK_SELECT_MASTERKEYFILE) Lazy selectMasterkeyFileScene, UserInteractionLock passwordEntryLock, UserInteractionLock masterkeyFileProvisionLock, AtomicReference password, AtomicReference filePath, @Nullable Exception previousError) { + public MasterkeyFileLoadingContext(@MasterkeyFileLoading Stage window, @FxmlScene(FxmlFile.UNLOCK_ENTER_PASSWORD) Lazy passphraseEntryScene, @FxmlScene(FxmlFile.UNLOCK_SELECT_MASTERKEYFILE) Lazy selectMasterkeyFileScene, UserInteractionLock passwordEntryLock, UserInteractionLock masterkeyFileProvisionLock, AtomicReference password, AtomicReference filePath) { this.window = window; this.passphraseEntryScene = passphraseEntryScene; this.selectMasterkeyFileScene = selectMasterkeyFileScene; @@ -42,11 +43,15 @@ class MasterkeyFileLoadingContext implements MasterkeyFileLoaderContext { this.masterkeyFileProvisionLock = masterkeyFileProvisionLock; this.password = password; this.filePath = filePath; - this.previousError = previousError; } @Override public Path getCorrectMasterkeyFilePath(String masterkeyFilePath) { + if (filePath.get() != null) { // e.g. already chosen on previous attempt with wrong password + return filePath.get(); + } + + assert filePath.get() == null; try { if (askForCorrectMasterkeyFile() == MasterkeyFileProvision.MASTERKEYFILE_PROVIDED) { return filePath.get(); @@ -105,11 +110,20 @@ class MasterkeyFileLoadingContext implements MasterkeyFileLoaderContext { } else { window.centerOnScreen(); } - if (previousError instanceof InvalidPassphraseException) { + if (wrongPassword) { Animations.createShakeWindowAnimation(window).play(); } }); return passwordEntryLock.awaitInteraction(); } + public boolean recoverFromException(MasterkeyLoadingFailedException exception) { + if (exception instanceof InvalidPassphraseException) { + this.wrongPassword = true; + password.set(null); + return true; // reattempting key load + } else { + return false; // nothing we can do + } + } }