Rework sanitizer workflow:

* select set of performed HealthChecks prior to loading masterkey
* two predefined healthcheck sets and custom one
* start sequential execution of checks after successful masterkey loading
* removed vault config info
* destroy masterkey on window close, on cancellation, on check finish
This commit is contained in:
Armin Schrenk
2021-04-27 17:41:28 +02:00
parent f3a03c71ec
commit 70baa4d09f
6 changed files with 144 additions and 68 deletions

View File

@@ -1,38 +1,33 @@
package org.cryptomator.ui.health;
import com.tobiasdiez.easybind.EasyBind;
import com.tobiasdiez.easybind.EasyBinding;
import com.tobiasdiez.easybind.optional.OptionalBinding;
import dagger.Lazy;
import org.cryptomator.cryptofs.VaultConfig;
import org.cryptomator.cryptofs.health.api.DiagnosticResult;
import org.cryptomator.cryptofs.health.api.HealthCheck;
import org.cryptomator.ui.common.FxController;
import javax.inject.Inject;
import javafx.beans.binding.Binding;
import javafx.beans.binding.Bindings;
import javafx.beans.binding.BooleanBinding;
import javafx.beans.binding.BooleanExpression;
import javafx.beans.binding.ObjectBinding;
import javafx.beans.property.ObjectProperty;
import javafx.beans.property.ReadOnlyObjectProperty;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import javafx.concurrent.Worker;
import javafx.fxml.FXML;
import javafx.scene.control.ListView;
import javafx.stage.Stage;
import java.util.Collection;
import java.util.Objects;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.atomic.AtomicReference;
@HealthCheckScoped
public class CheckController implements FxController {
private final Stage window;
private final VaultConfig vaultConfig;
private final Runnable masterkeyDestructor;
private final ExecutorService executor;
private final ObjectProperty<HealthCheckTask> selectedTask;
private final ObservableList<HealthCheckTask> tasks;
@@ -46,10 +41,10 @@ public class CheckController implements FxController {
@Inject
public CheckController(@HealthCheckWindow Stage window, AtomicReference<VaultConfig> vaultConfigRef, Lazy<Collection<HealthCheckTask>> tasks, ExecutorService executor, ObjectProperty<HealthCheckTask> selectedTask) {
this.window = window;
public CheckController(AtomicReference<VaultConfig> vaultConfigRef, Runnable masterkeyDestructor, Lazy<Collection<HealthCheckTask>> tasks, ObjectProperty<HealthCheckTask> selectedTask) {
this.vaultConfig = Objects.requireNonNull(vaultConfigRef.get());
this.executor = executor;
this.masterkeyDestructor = masterkeyDestructor;
this.executor = Executors.newSingleThreadExecutor();
this.selectedTask = selectedTask;
this.selectedResults = Bindings.createObjectBinding(this::getSelectedResults, selectedTask);
this.selectedTaskState = EasyBind.wrapNullable(selectedTask).mapObservable(HealthCheckTask::stateProperty);
@@ -65,19 +60,21 @@ public class CheckController implements FxController {
resultsListView.itemsProperty().bind(selectedResults);
resultsListView.setCellFactory(ignored -> new ResultListCell());
selectedTask.bind(checksListView.getSelectionModel().selectedItemProperty());
checksListView.getSelectionModel().select(0);
tasks.forEach(task -> executor.execute(task));
//ensure that at the end the masterkey is destroyed
executor.submit(masterkeyDestructor);
}
@FXML
public void runCheck() {
assert selectedTask.get() != null;
executor.execute(selectedTask.get());
}
@FXML
public void cancelCheck() {
assert selectedTask.get() != null;
assert selectedTask.get().isRunning();
selectedTask.get().cancel();
//assert selectedTask.get().isRunning(); Replace with something like executor.isRunning()
executor.shutdownNow();
masterkeyDestructor.run();
}
/* Getter/Setter */
public VaultConfig getVaultConfig() {

View File

@@ -3,7 +3,6 @@ package org.cryptomator.ui.health;
import dagger.Binds;
import dagger.Module;
import dagger.Provides;
import dagger.multibindings.ElementsIntoSet;
import dagger.multibindings.IntoMap;
import org.cryptomator.common.vaults.Vault;
import org.cryptomator.cryptofs.VaultConfig;
@@ -23,16 +22,15 @@ import org.cryptomator.ui.mainwindow.MainWindow;
import javax.inject.Provider;
import javafx.beans.property.ObjectProperty;
import javafx.beans.property.SimpleObjectProperty;
import javafx.beans.value.ChangeListener;
import javafx.scene.Scene;
import javafx.stage.Modality;
import javafx.stage.Stage;
import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Map;
import java.util.Optional;
import java.util.ResourceBundle;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
@Module(subcomponents = {KeyLoadingComponent.class})
@@ -50,12 +48,31 @@ abstract class HealthCheckModule {
return new AtomicReference<>();
}
@Provides
@HealthCheckScoped
static Runnable provideMasterkeyDestructor(AtomicReference<Masterkey> masterkeyRef) {
return () -> Optional.ofNullable(masterkeyRef.getAndSet(null)).ifPresent(Masterkey::destroy);
}
@Provides
@HealthCheckScoped
static AtomicReference<VaultConfig> provideVaultConfigRef() {
return new AtomicReference<>();
}
@Provides
@HealthCheckScoped
static Collection<HealthCheck> provideSelectedHealthChecks() {
return new ArrayList<>();
}
/* Only inject with Lazy-Wrapper!*/
@Provides
@HealthCheckScoped
static Collection<HealthCheckTask> provideSelectedHealthCheckTasks(Collection<HealthCheck> selectedHealthChecks, @HealthCheckWindow Vault vault, AtomicReference<Masterkey> masterkeyRef, AtomicReference<VaultConfig> vaultConfigRef, SecureRandom csprng) {
return selectedHealthChecks.stream().map(check -> new HealthCheckTask(vault.getPath(), vaultConfigRef.get(), masterkeyRef.get(), csprng, check)).toList(); //TODO: for every task clone the masterkey and destroy it afterwards
}
@Provides
@HealthCheckWindow
@HealthCheckScoped
@@ -63,12 +80,6 @@ abstract class HealthCheckModule {
return compBuilder.vault(vault).window(window).build().keyloadingStrategy();
}
@Provides
@HealthCheckScoped
static Collection<HealthCheckTask> provideHealthCheckTasks(@HealthCheckWindow Vault vault, AtomicReference<Masterkey> masterkeyRef, AtomicReference<VaultConfig> vaultConfigRef, SecureRandom csprng) {
return HealthCheck.allChecks().stream().map(check -> new HealthCheckTask(vault.getPath(), vaultConfigRef.get(), masterkeyRef.get(), csprng, check)).toList();
}
@Provides
@HealthCheckWindow
@HealthCheckScoped
@@ -79,24 +90,18 @@ abstract class HealthCheckModule {
@Provides
@HealthCheckWindow
@HealthCheckScoped
static Stage provideStage(StageFactory factory, @MainWindow Stage owner, ResourceBundle resourceBundle, ChangeListener<Boolean> showingListener) {
static Stage provideStage(StageFactory factory, @MainWindow Stage owner, ResourceBundle resourceBundle, Runnable masterkeyDestructor) {
Stage stage = factory.create();
stage.setTitle(resourceBundle.getString("health.title"));
stage.setResizable(true);
stage.initModality(Modality.WINDOW_MODAL);
stage.initOwner(owner);
stage.showingProperty().addListener(showingListener);
return stage;
}
@Provides
@HealthCheckScoped
static ChangeListener<Boolean> provideWindowShowingChangeListener(AtomicReference<Masterkey> masterkey) {
return (observable, wasShowing, isShowing) -> {
stage.showingProperty().addListener((observable, wasShowing, isShowing) -> { //TODO: should we use showingProperty or onCloseRequest
if (!isShowing) {
Optional.ofNullable(masterkey.getAndSet(null)).ifPresent(Masterkey::destroy);
masterkeyDestructor.run();
}
};
});
return stage;
}
@Provides

View File

@@ -8,17 +8,13 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import javafx.application.Platform;
import javafx.beans.binding.BooleanBinding;
import javafx.beans.property.ReadOnlyBooleanProperty;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import javafx.concurrent.Task;
import javafx.concurrent.Worker;
import java.nio.file.Path;
import java.security.SecureRandom;
import java.util.Objects;
import java.util.concurrent.CancellationException;
import java.util.function.Consumer;
class HealthCheckTask extends Task<Void> {
@@ -42,8 +38,9 @@ class HealthCheckTask extends Task<Void> {
@Override
protected Void call() {
try (var cryptor = vaultConfig.getCipherCombo().getCryptorProvider(csprng).withKey(masterkey)) {
check.check(vaultPath, vaultConfig, masterkey, cryptor, result -> {
try (var masterkeyClone = masterkey.clone(); //
var cryptor = vaultConfig.getCipherCombo().getCryptorProvider(csprng).withKey(masterkeyClone)) {
check.check(vaultPath, vaultConfig, masterkeyClone, cryptor, result -> { //TODO: why using both masterkey and Cryptor ??
if (isCancelled()) {
throw new CancellationException();
}

View File

@@ -5,12 +5,13 @@ import org.cryptomator.common.vaults.Vault;
import org.cryptomator.cryptofs.VaultConfig;
import org.cryptomator.cryptofs.VaultConfigLoadException;
import org.cryptomator.cryptofs.VaultKeyInvalidException;
import org.cryptomator.cryptofs.health.api.HealthCheck;
import org.cryptomator.cryptofs.health.dirid.DirIdCheck;
import org.cryptomator.cryptolib.api.Masterkey;
import org.cryptomator.cryptolib.api.MasterkeyLoadingFailedException;
import org.cryptomator.ui.common.FxController;
import org.cryptomator.ui.common.FxmlFile;
import org.cryptomator.ui.common.FxmlScene;
import org.cryptomator.ui.fxapp.FxApplication;
import org.cryptomator.ui.keyloading.KeyLoadingStrategy;
import org.cryptomator.ui.unlock.UnlockCancelledException;
import org.slf4j.Logger;
@@ -18,11 +19,22 @@ import org.slf4j.LoggerFactory;
import javax.inject.Inject;
import javafx.application.Platform;
import javafx.beans.binding.BooleanBinding;
import javafx.beans.property.BooleanProperty;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.collections.FXCollections;
import javafx.fxml.FXML;
import javafx.scene.Scene;
import javafx.scene.control.ListView;
import javafx.scene.control.RadioButton;
import javafx.scene.control.ToggleGroup;
import javafx.scene.control.cell.CheckBoxListCell;
import javafx.stage.Stage;
import java.net.URI;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.atomic.AtomicReference;
@@ -32,22 +44,58 @@ public class StartController implements FxController {
private static final Logger LOG = LoggerFactory.getLogger(StartController.class);
private final Stage window;
private final Collection<HealthCheck> availableChecks;
private final Map<HealthCheck, BooleanProperty> availableCheckListSelectProperties;
private final Optional<VaultConfig.UnverifiedVaultConfig> unverifiedVaultConfig;
private final KeyLoadingStrategy keyLoadingStrategy;
private final ExecutorService executor;
private final AtomicReference<Masterkey> masterkeyRef;
private final AtomicReference<VaultConfig> vaultConfigRef;
private final Collection<HealthCheck> selectedChecks;
private final Lazy<Scene> checkScene;
/* FXML */
public ListView availableChecksList;
public RadioButton customCheckSetButton;
public RadioButton quickCheckSetButton;
public RadioButton fullCheckSetButton;
public ToggleGroup checksSetToggleGroup;
@Inject
public StartController(@HealthCheckWindow Vault vault, @HealthCheckWindow Stage window, @HealthCheckWindow KeyLoadingStrategy keyLoadingStrategy, ExecutorService executor, AtomicReference<Masterkey> masterkeyRef, AtomicReference<VaultConfig> vaultConfigRef, @FxmlScene(FxmlFile.HEALTH_CHECK) Lazy<Scene> checkScene) {
public StartController(@HealthCheckWindow Vault vault, @HealthCheckWindow Stage window, @HealthCheckWindow KeyLoadingStrategy keyLoadingStrategy, ExecutorService executor, AtomicReference<Masterkey> masterkeyRef, AtomicReference<VaultConfig> vaultConfigRef, Collection<HealthCheck> selectedChecks, @FxmlScene(FxmlFile.HEALTH_CHECK) Lazy<Scene> checkScene) {
this.window = window;
this.unverifiedVaultConfig = vault.getUnverifiedVaultConfig();
this.unverifiedVaultConfig = vault.getUnverifiedVaultConfig(); //TODO: prevent workflow at all, if the vault config is emtpy
this.keyLoadingStrategy = keyLoadingStrategy;
this.executor = executor;
this.masterkeyRef = masterkeyRef;
this.vaultConfigRef = vaultConfigRef;
this.selectedChecks = selectedChecks;
this.checkScene = checkScene;
this.availableChecks = HealthCheck.allChecks();
this.availableCheckListSelectProperties = new HashMap<>();
availableChecks.forEach(check -> availableCheckListSelectProperties.put(check, new SimpleBooleanProperty(false)));
}
public void initialize() {
availableChecksList.setItems(FXCollections.observableList(availableChecks.stream().toList()));
availableChecksList.setCellFactory(CheckBoxListCell.forListView(availableCheckListSelectProperties::get));
availableChecksList.setEditable(false);
var availableCheckIds = availableChecks.stream().map(HealthCheck::identifier).toList();
if (PredefinedCheckSet.QUICK.getCheckIds().stream().anyMatch(checkId -> !availableCheckIds.contains(checkId))) {
quickCheckSetButton.setVisible(false);
quickCheckSetButton.setManaged(false);
}
if (PredefinedCheckSet.FULL.getCheckIds().stream().anyMatch(checkId -> !availableCheckIds.contains(checkId))) {
fullCheckSetButton.setVisible(false);
fullCheckSetButton.setManaged(false);
}
quickCheckSetButton.setUserData(PredefinedCheckSet.QUICK);
fullCheckSetButton.setUserData(PredefinedCheckSet.FULL);
customCheckSetButton.setUserData(PredefinedCheckSet.CUSTOM);
}
@FXML
@@ -58,6 +106,15 @@ public class StartController implements FxController {
@FXML
public void next() {
switch ((PredefinedCheckSet) checksSetToggleGroup.getSelectedToggle().getUserData()) {
case FULL -> selectedChecks.addAll(availableChecks);
case QUICK -> selectedChecks.addAll(availableChecks.stream().filter(check -> PredefinedCheckSet.QUICK.getCheckIds().contains(check.identifier())).toList());
case CUSTOM -> availableCheckListSelectProperties.forEach((check, selected) -> {
if (selected.get()) {
selectedChecks.add(check);
}
});
}
LOG.trace("StartController.next()");
executor.submit(this::loadKey);
}
@@ -109,16 +166,27 @@ public class StartController implements FxController {
return unverifiedVaultConfig.isEmpty();
}
public int getVaultVersion() {
return unverifiedVaultConfig
.map(VaultConfig.UnverifiedVaultConfig::allegedVaultVersion)
.orElse(-1);
public BooleanBinding anyCheckSetSelectedProperty() {
return checksSetToggleGroup.selectedToggleProperty().isNotNull();
}
public String getKeyId() {
return unverifiedVaultConfig
.map(VaultConfig.UnverifiedVaultConfig::getKeyId)
.map(URI::toString)
.orElse(null);
public boolean isAnyCheckSetSelected() {
return anyCheckSetSelectedProperty().get();
}
enum PredefinedCheckSet {
QUICK(DirIdCheck.class.getCanonicalName()), //TODO: get identifier via static method?
FULL(DirIdCheck.class.getCanonicalName()),
CUSTOM();
private Collection<String> checkIds;
PredefinedCheckSet(String... checkIds) {
this.checkIds = Set.of(checkIds);
}
Collection<String> getCheckIds() {
return checkIds;
}
}
}

View File

@@ -3,10 +3,9 @@
<?import javafx.geometry.Insets?>
<?import javafx.scene.control.Button?>
<?import javafx.scene.control.ButtonBar?>
<?import javafx.scene.control.Label?>
<?import javafx.scene.layout.VBox?>
<?import javafx.scene.layout.HBox?>
<?import javafx.scene.control.ListView?>
<?import javafx.scene.layout.HBox?>
<?import javafx.scene.layout.VBox?>
<HBox xmlns:fx="http://javafx.com/fxml"
xmlns="http://javafx.com/javafx"
fx:controller="org.cryptomator.ui.health.CheckController"
@@ -20,14 +19,11 @@
<children>
<ListView fx:id="checksListView"/>
<VBox>
<Label text="${controller.vaultConfig.cipherCombo}"/>
<ListView fx:id="resultsListView"/>
<ButtonBar buttonMinWidth="120" buttonOrder="+CA">
<buttons>
<Button text="%generic.button.cancel" ButtonBar.buttonData="CANCEL_CLOSE" onAction="#cancelCheck" disable="${!controller.running}"/>
<Button text="TODO run check" ButtonBar.buttonData="APPLY" defaultButton="true" onAction="#runCheck" disable="${!controller.ready}"/>
</buttons>
</ButtonBar>
</VBox>

View File

@@ -4,6 +4,10 @@
<?import javafx.scene.control.Button?>
<?import javafx.scene.control.ButtonBar?>
<?import javafx.scene.control.Label?>
<?import javafx.scene.control.ListView?>
<?import javafx.scene.control.RadioButton?>
<?import javafx.scene.control.ToggleGroup?>
<?import javafx.scene.layout.AnchorPane?>
<?import javafx.scene.layout.VBox?>
<VBox xmlns:fx="http://javafx.com/fxml"
xmlns="http://javafx.com/javafx"
@@ -12,20 +16,29 @@
maxWidth="400"
minHeight="145"
spacing="12">
<fx:define>
<ToggleGroup fx:id="checksSetToggleGroup"/>
</fx:define>
<padding>
<Insets topRightBottomLeft="12"/>
</padding>
<children>
<Label text="${controller.keyId}"/>
<Label text="${controller.vaultVersion}"/>
<Label text="TODO: Invalid vault config" visible="${controller.invalidConfig}" managed="${controller.invalidConfig}"/>
<Label text="Please select, which check(s) should be performed:"/>
<VBox spacing="6" disable="${controller.invalidConfig}">
<RadioButton fx:id="quickCheckSetButton" toggleGroup="${checksSetToggleGroup}" text="Quick Check"/>
<RadioButton fx:id="fullCheckSetButton" toggleGroup="${checksSetToggleGroup}" text="Full Check"/>
<AnchorPane >
<RadioButton fx:id="customCheckSetButton" toggleGroup="${checksSetToggleGroup}" text="Custom Check"/>
<ListView fx:id="availableChecksList" AnchorPane.leftAnchor="20.0" AnchorPane.topAnchor="20.0" maxHeight="100" visible="${customCheckSetButton.selected}"/>
</AnchorPane>
</VBox>
<ButtonBar buttonMinWidth="120" buttonOrder="+CX">
<buttons>
<Button text="%generic.button.cancel" ButtonBar.buttonData="CANCEL_CLOSE" cancelButton="true" onAction="#close"/>
<Button text="%generic.button.next" ButtonBar.buttonData="NEXT_FORWARD" disable="${controller.invalidConfig}" defaultButton="true" onAction="#next"/>
<Button text="%generic.button.next" ButtonBar.buttonData="NEXT_FORWARD" disable="${controller.invalidConfig || !controller.anyCheckSetSelected}" defaultButton="true" onAction="#next"/>
</buttons>
</ButtonBar>
</children>