From c0a9a95e4fcf6870c1492ffba630040ee93a07b2 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Tue, 8 Dec 2020 14:39:46 +0100 Subject: [PATCH 01/29] Adjusted to CryptoFS 2.0.0 --- .../org/cryptomator/common/CommonsModule.java | 12 +++++ .../org/cryptomator/common/Constants.java | 2 + .../org/cryptomator/common/vaults/Vault.java | 21 ++++---- .../common/vaults/VaultListManager.java | 7 +-- main/pom.xml | 2 +- .../CreateNewVaultPasswordController.java | 48 ++++++++++++++----- .../ChangePasswordController.java | 31 ++++++++++-- .../ui/mainwindow/MainWindowController.java | 7 +-- .../ui/migration/MigrationRunController.java | 5 +- .../RecoveryKeyCreationController.java | 3 +- .../ui/recoverykey/RecoveryKeyFactory.java | 37 ++++++++++---- .../cryptomator/ui/unlock/UnlockWorkflow.java | 5 +- .../recoverykey/RecoveryKeyFactoryTest.java | 23 +++++++-- .../org.mockito.plugins.MockMaker | 1 + 14 files changed, 156 insertions(+), 48 deletions(-) create mode 100644 main/ui/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker diff --git a/main/commons/src/main/java/org/cryptomator/common/CommonsModule.java b/main/commons/src/main/java/org/cryptomator/common/CommonsModule.java index ed278e1ab..0ea3428ed 100644 --- a/main/commons/src/main/java/org/cryptomator/common/CommonsModule.java +++ b/main/commons/src/main/java/org/cryptomator/common/CommonsModule.java @@ -25,6 +25,8 @@ import javafx.beans.binding.Binding; import javafx.beans.binding.Bindings; import javafx.collections.ObservableList; import java.net.InetSocketAddress; +import java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; import java.util.Comparator; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -53,6 +55,16 @@ public abstract class CommonsModule { + "r0DzRyj4ixPIt38CQB8="; } + @Provides + @Singleton + static SecureRandom provideCSPRNG() { + try { + return SecureRandom.getInstanceStrong(); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException("A strong algorithm must exist in every Java platform.", e); + } + } + @Provides @Singleton @Named("SemVer") diff --git a/main/commons/src/main/java/org/cryptomator/common/Constants.java b/main/commons/src/main/java/org/cryptomator/common/Constants.java index 432cd08e5..cbde9642a 100644 --- a/main/commons/src/main/java/org/cryptomator/common/Constants.java +++ b/main/commons/src/main/java/org/cryptomator/common/Constants.java @@ -3,5 +3,7 @@ package org.cryptomator.common; public interface Constants { String MASTERKEY_FILENAME = "masterkey.cryptomator"; + String VAULTCONFIG_FILENAME = "vault.cryptomator"; + byte[] PEPPER = new byte[0]; } 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 730f0fb6d..c27920b20 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 @@ -21,6 +21,7 @@ import org.cryptomator.cryptofs.common.Constants; import org.cryptomator.cryptofs.common.FileSystemCapabilityChecker; import org.cryptomator.cryptolib.api.CryptoException; import org.cryptomator.cryptolib.api.InvalidPassphraseException; +import org.cryptomator.cryptolib.common.MasterkeyFile; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,6 +46,7 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; +import static org.cryptomator.common.Constants.PEPPER; @PerVault public class Vault { @@ -111,14 +113,17 @@ public class Vault { LOG.info("Storing file name length limit of {}", limit); } assert vaultSettings.filenameLengthLimit().get() > 0; - CryptoFileSystemProperties fsProps = CryptoFileSystemProperties.cryptoFileSystemProperties() // - .withPassphrase(passphrase) // - .withFlags(flags) // - .withMasterkeyFilename(MASTERKEY_FILENAME) // - .withMaxPathLength(vaultSettings.filenameLengthLimit().get() + Constants.MAX_ADDITIONAL_PATH_LENGTH) // - .withMaxNameLength(vaultSettings.filenameLengthLimit().get()) // - .build(); - return CryptoFileSystemProvider.newFileSystem(getPath(), fsProps); + + Path masterkeyPath = getPath().resolve(MASTERKEY_FILENAME); + try (var keyLoader = MasterkeyFile.withContentFromFile(masterkeyPath).unlock(passphrase, PEPPER, Optional.empty())) { + CryptoFileSystemProperties fsProps = CryptoFileSystemProperties.cryptoFileSystemProperties() // + .withKeyLoader(keyLoader) // + .withFlags(flags) // + .withMaxPathLength(vaultSettings.filenameLengthLimit().get() + Constants.MAX_ADDITIONAL_PATH_LENGTH) // + .withMaxNameLength(vaultSettings.filenameLengthLimit().get()) // + .build(); + return CryptoFileSystemProvider.newFileSystem(getPath(), fsProps); + } } public synchronized void unlock(CharSequence passphrase) throws CryptoException, IOException, VolumeException, InvalidMountPointException { diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java index 984008ba9..cbf196d8a 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java @@ -28,6 +28,7 @@ import java.util.ResourceBundle; import java.util.stream.Collectors; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; +import static org.cryptomator.common.Constants.VAULTCONFIG_FILENAME; @Singleton public class VaultListManager { @@ -54,7 +55,7 @@ public class VaultListManager { public Vault add(Path pathToVault) throws NoSuchFileException { Path normalizedPathToVault = pathToVault.normalize().toAbsolutePath(); - if (!CryptoFileSystemProvider.containsVault(normalizedPathToVault, MASTERKEY_FILENAME)) { + if (!CryptoFileSystemProvider.containsVault(normalizedPathToVault, VAULTCONFIG_FILENAME, MASTERKEY_FILENAME)) { throw new NoSuchFileException(normalizedPathToVault.toString(), null, "Not a vault directory"); } Optional alreadyExistingVault = get(normalizedPathToVault); @@ -124,9 +125,9 @@ public class VaultListManager { } private static VaultState determineVaultState(Path pathToVault) throws IOException { - if (!CryptoFileSystemProvider.containsVault(pathToVault, MASTERKEY_FILENAME)) { + if (!CryptoFileSystemProvider.containsVault(pathToVault, VAULTCONFIG_FILENAME, MASTERKEY_FILENAME)) { return VaultState.MISSING; - } else if (Migrators.get().needsMigration(pathToVault, MASTERKEY_FILENAME)) { + } else if (Migrators.get().needsMigration(pathToVault, VAULTCONFIG_FILENAME, MASTERKEY_FILENAME)) { return VaultState.NEEDS_MIGRATION; } else { return VaultState.LOCKED; diff --git a/main/pom.xml b/main/pom.xml index 77690a641..20587a864 100644 --- a/main/pom.xml +++ b/main/pom.xml @@ -24,7 +24,7 @@ UTF-8 - 1.9.13 + 2.0.0-beta1 0.1.6 0.1.0-beta1 0.1.0-beta3 diff --git a/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/CreateNewVaultPasswordController.java b/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/CreateNewVaultPasswordController.java index c4e226359..25cb45395 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/CreateNewVaultPasswordController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/CreateNewVaultPasswordController.java @@ -5,6 +5,13 @@ import org.cryptomator.common.vaults.Vault; import org.cryptomator.common.vaults.VaultListManager; import org.cryptomator.cryptofs.CryptoFileSystemProperties; import org.cryptomator.cryptofs.CryptoFileSystemProvider; +import org.cryptomator.cryptofs.VaultCipherCombo; +import org.cryptomator.cryptolib.api.CryptoException; +import org.cryptomator.cryptolib.api.InvalidPassphraseException; +import org.cryptomator.cryptolib.api.Masterkey; +import org.cryptomator.cryptolib.api.UnsupportedVaultFormatException; +import org.cryptomator.cryptolib.common.MasterkeyFile; +import org.cryptomator.cryptolib.common.MasterkeyFileLoader; import org.cryptomator.ui.common.ErrorComponent; import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.common.FxmlFile; @@ -29,6 +36,7 @@ import javafx.scene.control.ContentDisplay; import javafx.scene.control.Toggle; import javafx.scene.control.ToggleGroup; import javafx.stage.Stage; +import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.UncheckedIOException; import java.nio.channels.WritableByteChannel; @@ -37,12 +45,15 @@ import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.StandardOpenOption; +import java.security.SecureRandom; import java.util.Collections; +import java.util.Optional; import java.util.ResourceBundle; import java.util.concurrent.ExecutorService; import static java.nio.charset.StandardCharsets.US_ASCII; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; +import static org.cryptomator.common.Constants.PEPPER; @AddVaultWizardScoped public class CreateNewVaultPasswordController implements FxController { @@ -64,6 +75,7 @@ public class CreateNewVaultPasswordController implements FxController { private final ResourceBundle resourceBundle; private final ObjectProperty password; private final ReadmeGenerator readmeGenerator; + private final SecureRandom csprng; private final BooleanProperty processing; private final BooleanProperty readyToCreateVault; private final ObjectBinding createVaultButtonState; @@ -73,7 +85,7 @@ public class CreateNewVaultPasswordController implements FxController { public Toggle skipRecoveryKey; @Inject - CreateNewVaultPasswordController(@AddVaultWizardWindow Stage window, @FxmlScene(FxmlFile.ADDVAULT_NEW_LOCATION) Lazy chooseLocationScene, @FxmlScene(FxmlFile.ADDVAULT_NEW_RECOVERYKEY) Lazy recoveryKeyScene, @FxmlScene(FxmlFile.ADDVAULT_SUCCESS) Lazy successScene, ErrorComponent.Builder errorComponent, ExecutorService executor, RecoveryKeyFactory recoveryKeyFactory, @Named("vaultName") StringProperty vaultName, ObjectProperty vaultPath, @AddVaultWizardWindow ObjectProperty vault, @Named("recoveryKey") StringProperty recoveryKey, VaultListManager vaultListManager, ResourceBundle resourceBundle, @Named("newPassword") ObjectProperty password, ReadmeGenerator readmeGenerator) { + CreateNewVaultPasswordController(@AddVaultWizardWindow Stage window, @FxmlScene(FxmlFile.ADDVAULT_NEW_LOCATION) Lazy chooseLocationScene, @FxmlScene(FxmlFile.ADDVAULT_NEW_RECOVERYKEY) Lazy recoveryKeyScene, @FxmlScene(FxmlFile.ADDVAULT_SUCCESS) Lazy successScene, ErrorComponent.Builder errorComponent, ExecutorService executor, RecoveryKeyFactory recoveryKeyFactory, @Named("vaultName") StringProperty vaultName, ObjectProperty vaultPath, @AddVaultWizardWindow ObjectProperty vault, @Named("recoveryKey") StringProperty recoveryKey, VaultListManager vaultListManager, ResourceBundle resourceBundle, @Named("newPassword") ObjectProperty password, ReadmeGenerator readmeGenerator, SecureRandom csprng) { this.window = window; this.chooseLocationScene = chooseLocationScene; this.recoveryKeyScene = recoveryKeyScene; @@ -89,6 +101,7 @@ public class CreateNewVaultPasswordController implements FxController { this.resourceBundle = resourceBundle; this.password = password; this.readmeGenerator = readmeGenerator; + this.csprng = csprng; this.processing = new SimpleBooleanProperty(); this.readyToCreateVault = new SimpleBooleanProperty(); this.createVaultButtonState = Bindings.createObjectBinding(this::getCreateVaultButtonState, processing); @@ -161,23 +174,34 @@ public class CreateNewVaultPasswordController implements FxController { } private void initializeVault(Path path, CharSequence passphrase) throws IOException { - CryptoFileSystemProvider.initialize(path, MASTERKEY_FILENAME, passphrase); - CryptoFileSystemProperties fsProps = CryptoFileSystemProperties.cryptoFileSystemProperties() // - .withPassphrase(passphrase) // - .withFlags(Collections.emptySet()) // - .withMasterkeyFilename(MASTERKEY_FILENAME) // - .build(); - - String vaultReadmeFileName = resourceBundle.getString("addvault.new.readme.accessLocation.fileName"); - try (FileSystem fs = CryptoFileSystemProvider.newFileSystem(path, fsProps); // - WritableByteChannel ch = Files.newByteChannel(fs.getPath("/", vaultReadmeFileName), StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE)) { - ch.write(US_ASCII.encode(readmeGenerator.createVaultAccessLocationReadmeRtf())); + // 1. write masterkey: + Path masterkeyFilePath = path.resolve(MASTERKEY_FILENAME); + try (Masterkey masterkey = Masterkey.createNew(csprng)) { + byte[] serialized = MasterkeyFile.lock(masterkey, passphrase, PEPPER, 999, csprng); + Files.write(masterkeyFilePath, serialized, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE); } + // 2. verify masterkey and initialize vault: + try (var loader = MasterkeyFile.withContentFromFile(masterkeyFilePath).unlock(passphrase, PEPPER, Optional.of(999))) { + CryptoFileSystemProperties fsProps = CryptoFileSystemProperties.cryptoFileSystemProperties().withKeyLoader(loader).build(); + CryptoFileSystemProvider.initialize(path, fsProps, MasterkeyFileLoader.KEY_ID); + + // 3. write vault-internal readme file: + String vaultReadmeFileName = resourceBundle.getString("addvault.new.readme.accessLocation.fileName"); + try (FileSystem fs = CryptoFileSystemProvider.newFileSystem(path, fsProps); // + WritableByteChannel ch = Files.newByteChannel(fs.getPath("/", vaultReadmeFileName), StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE)) { + ch.write(US_ASCII.encode(readmeGenerator.createVaultAccessLocationReadmeRtf())); + } + } catch (CryptoException e) { + throw new IOException("Failed initialize vault.", e); + } + + // 4. write vault-external readme file: String storagePathReadmeFileName = resourceBundle.getString("addvault.new.readme.storageLocation.fileName"); try (WritableByteChannel ch = Files.newByteChannel(path.resolve(storagePathReadmeFileName), StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE)) { ch.write(US_ASCII.encode(readmeGenerator.createVaultStorageLocationReadmeRtf())); } + LOG.info("Created vault at {}", path); } diff --git a/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordController.java b/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordController.java index 52cfe2b81..436103436 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordController.java @@ -3,7 +3,10 @@ package org.cryptomator.ui.changepassword; import org.cryptomator.common.keychain.KeychainManager; import org.cryptomator.common.vaults.Vault; import org.cryptomator.cryptofs.CryptoFileSystemProvider; +import org.cryptomator.cryptofs.common.MasterkeyBackupHelper; +import org.cryptomator.cryptolib.api.CryptoException; import org.cryptomator.cryptolib.api.InvalidPassphraseException; +import org.cryptomator.cryptolib.common.MasterkeyFile; import org.cryptomator.integrations.keychain.KeychainAccessException; import org.cryptomator.ui.common.Animations; import org.cryptomator.ui.common.ErrorComponent; @@ -23,6 +26,12 @@ import javafx.scene.control.CheckBox; import javafx.stage.Stage; import java.io.IOException; import java.nio.CharBuffer; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; +import java.security.SecureRandom; +import java.text.Normalizer; import java.util.Optional; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; @@ -31,24 +40,27 @@ import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; public class ChangePasswordController implements FxController { private static final Logger LOG = LoggerFactory.getLogger(ChangePasswordController.class); + private static final String MASTERKEY_BACKUP_SUFFIX = ".bkup"; private final Stage window; private final Vault vault; private final ObjectProperty newPassword; private final ErrorComponent.Builder errorComponent; private final KeychainManager keychain; + private final SecureRandom csprng; public NiceSecurePasswordField oldPasswordField; public CheckBox finalConfirmationCheckbox; public Button finishButton; @Inject - public ChangePasswordController(@ChangePasswordWindow Stage window, @ChangePasswordWindow Vault vault, @Named("newPassword") ObjectProperty newPassword, ErrorComponent.Builder errorComponent, KeychainManager keychain) { + public ChangePasswordController(@ChangePasswordWindow Stage window, @ChangePasswordWindow Vault vault, @Named("newPassword") ObjectProperty newPassword, ErrorComponent.Builder errorComponent, KeychainManager keychain, SecureRandom csprng) { this.window = window; this.vault = vault; this.newPassword = newPassword; this.errorComponent = errorComponent; this.keychain = keychain; + this.csprng = csprng; } @FXML @@ -67,17 +79,26 @@ public class ChangePasswordController implements FxController { @FXML public void finish() { try { - CryptoFileSystemProvider.changePassphrase(vault.getPath(), MASTERKEY_FILENAME, oldPasswordField.getCharacters(), newPassword.get()); + //String normalizedOldPassphrase = Normalizer.normalize(oldPasswordField.getCharacters(), Normalizer.Form.NFC); + //String normalizedNewPassphrase = Normalizer.normalize(newPassword.get(), Normalizer.Form.NFC); + CharSequence oldPassphrase = oldPasswordField.getCharacters(); // TODO verify: is this already NFC-normalized? + CharSequence newPassphrase = newPassword.get(); // TODO verify: is this already NFC-normalized? + Path masterkeyPath = vault.getPath().resolve(MASTERKEY_FILENAME); + byte[] oldMasterkeyBytes = Files.readAllBytes(masterkeyPath); + byte[] newMasterkeyBytes = MasterkeyFile.changePassphrase(oldMasterkeyBytes, oldPassphrase, newPassphrase, new byte[0], csprng); + Path backupKeyPath = vault.getPath().resolve(MASTERKEY_FILENAME + MasterkeyBackupHelper.generateFileIdSuffix(oldMasterkeyBytes) + MASTERKEY_BACKUP_SUFFIX); + Files.move(masterkeyPath, backupKeyPath, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE); + Files.write(masterkeyPath, newMasterkeyBytes, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE); LOG.info("Successfully changed password for {}", vault.getDisplayName()); window.close(); updatePasswordInSystemkeychain(); - } catch (IOException e) { - LOG.error("IO error occured during password change. Unable to perform operation.", e); - errorComponent.cause(e).window(window).returnToScene(window.getScene()).build().showErrorScene(); } catch (InvalidPassphraseException e) { Animations.createShakeWindowAnimation(window).play(); oldPasswordField.selectAll(); oldPasswordField.requestFocus(); + } catch (IOException | CryptoException e) { + LOG.error("Password change failed. Unable to perform operation.", e); + errorComponent.cause(e).window(window).returnToScene(window.getScene()).build().showErrorScene(); } } diff --git a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/MainWindowController.java b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/MainWindowController.java index 65c3a8fcf..5fd4cc62d 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/MainWindowController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/MainWindowController.java @@ -27,6 +27,7 @@ import java.util.Set; import java.util.stream.Collectors; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; +import static org.cryptomator.common.Constants.VAULTCONFIG_FILENAME; @MainWindowScoped public class MainWindowController implements FxController { @@ -91,9 +92,9 @@ public class MainWindowController implements FxController { } private boolean containsVault(Path path) { - if (path.getFileName().toString().equals(MASTERKEY_FILENAME)) { + if (path.getFileName().toString().equals(VAULTCONFIG_FILENAME)) { return true; - } else if (Files.isDirectory(path) && Files.exists(path.resolve(MASTERKEY_FILENAME))) { + } else if (Files.isDirectory(path) && Files.exists(path.resolve(VAULTCONFIG_FILENAME))) { return true; } else { return false; @@ -102,7 +103,7 @@ public class MainWindowController implements FxController { private void addVault(Path pathToVault) { try { - if (pathToVault.getFileName().toString().equals(MASTERKEY_FILENAME)) { + if (pathToVault.getFileName().toString().equals(VAULTCONFIG_FILENAME)) { vaultListManager.add(pathToVault.getParent()); } else { vaultListManager.add(pathToVault); diff --git a/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java b/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java index 830e15819..ff18510d9 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java @@ -43,6 +43,7 @@ import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; +import static org.cryptomator.common.Constants.VAULTCONFIG_FILENAME; @MigrationScoped public class MigrationRunController implements FxController { @@ -110,8 +111,8 @@ public class MigrationRunController implements FxController { }, 0, MIGRATION_PROGRESS_UPDATE_MILLIS, TimeUnit.MILLISECONDS); Tasks.create(() -> { Migrators migrators = Migrators.get(); - migrators.migrate(vault.getPath(), MASTERKEY_FILENAME, password, this::migrationProgressChanged, this::migrationRequiresInput); - return migrators.needsMigration(vault.getPath(), MASTERKEY_FILENAME); + migrators.migrate(vault.getPath(), VAULTCONFIG_FILENAME, MASTERKEY_FILENAME, password, this::migrationProgressChanged, this::migrationRequiresInput); + return migrators.needsMigration(vault.getPath(), VAULTCONFIG_FILENAME, MASTERKEY_FILENAME); }).onSuccess(needsAnotherMigration -> { if (needsAnotherMigration) { LOG.info("Migration of '{}' succeeded, but another migration is required.", vault.getDisplayName()); diff --git a/main/ui/src/main/java/org/cryptomator/ui/recoverykey/RecoveryKeyCreationController.java b/main/ui/src/main/java/org/cryptomator/ui/recoverykey/RecoveryKeyCreationController.java index 902f2cd50..104794604 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/recoverykey/RecoveryKeyCreationController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/recoverykey/RecoveryKeyCreationController.java @@ -2,6 +2,7 @@ package org.cryptomator.ui.recoverykey; import dagger.Lazy; import org.cryptomator.common.vaults.Vault; +import org.cryptomator.cryptolib.api.CryptoException; import org.cryptomator.cryptolib.api.InvalidPassphraseException; import org.cryptomator.ui.common.Animations; import org.cryptomator.ui.common.ErrorComponent; @@ -80,7 +81,7 @@ public class RecoveryKeyCreationController implements FxController { } @Override - protected String call() throws IOException { + protected String call() throws IOException, CryptoException { return recoveryKeyFactory.createRecoveryKey(vault.getPath(), passwordField.getCharacters()); } diff --git a/main/ui/src/main/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactory.java b/main/ui/src/main/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactory.java index 15d4e651e..a53113020 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactory.java +++ b/main/ui/src/main/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactory.java @@ -2,28 +2,39 @@ package org.cryptomator.ui.recoverykey; import com.google.common.base.Preconditions; import com.google.common.hash.Hashing; -import org.cryptomator.cryptofs.CryptoFileSystemProvider; +import org.cryptomator.cryptofs.common.MasterkeyBackupHelper; +import org.cryptomator.cryptolib.api.CryptoException; import org.cryptomator.cryptolib.api.InvalidPassphraseException; +import org.cryptomator.cryptolib.api.Masterkey; +import org.cryptomator.cryptolib.common.MasterkeyFile; import javax.inject.Inject; import javax.inject.Singleton; import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; +import java.security.SecureRandom; import java.util.Arrays; import java.util.Collection; +import java.util.Optional; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; +import static org.cryptomator.common.Constants.PEPPER; @Singleton public class RecoveryKeyFactory { - private static final byte[] PEPPER = new byte[0]; + private static final String MASTERKEY_BACKUP_SUFFIX = ".bkup"; private final WordEncoder wordEncoder; + private final SecureRandom csprng; @Inject - public RecoveryKeyFactory(WordEncoder wordEncoder) { + public RecoveryKeyFactory(WordEncoder wordEncoder, SecureRandom csprng) { this.wordEncoder = wordEncoder; + this.csprng = csprng; } public Collection getDictionary() { @@ -36,11 +47,14 @@ public class RecoveryKeyFactory { * @return The recovery key of the vault at the given path * @throws IOException If the masterkey file could not be read * @throws InvalidPassphraseException If the provided password is wrong + * @throws CryptoException In case of other cryptographic errors * @apiNote This is a long-running operation and should be invoked in a background thread */ - public String createRecoveryKey(Path vaultPath, CharSequence password) throws IOException, InvalidPassphraseException { - byte[] rawKey = CryptoFileSystemProvider.exportRawKey(vaultPath, MASTERKEY_FILENAME, PEPPER, password); - try { + public String createRecoveryKey(Path vaultPath, CharSequence password) throws IOException, InvalidPassphraseException, CryptoException { + Path masterkeyPath = vaultPath.resolve(MASTERKEY_FILENAME); + byte[] rawKey = new byte[0]; + try (var masterkey = MasterkeyFile.withContentFromFile(masterkeyPath).unlock(password, PEPPER, Optional.empty()).loadKeyAndClose()) { + rawKey = masterkey.getEncoded(); return createRecoveryKey(rawKey); } finally { Arrays.fill(rawKey, (byte) 0x00); @@ -72,8 +86,15 @@ public class RecoveryKeyFactory { */ public void resetPasswordWithRecoveryKey(Path vaultPath, String recoveryKey, CharSequence newPassword) throws IOException, IllegalArgumentException { final byte[] rawKey = decodeRecoveryKey(recoveryKey); - try { - CryptoFileSystemProvider.restoreRawKey(vaultPath, MASTERKEY_FILENAME, rawKey, PEPPER, newPassword); + try (var masterkey = Masterkey.createFromRaw(rawKey)) { + byte[] restoredKey = MasterkeyFile.lock(masterkey, newPassword, PEPPER, 999, csprng); + Path masterkeyPath = vaultPath.resolve(MASTERKEY_FILENAME); + if (Files.exists(masterkeyPath)) { + byte[] oldMasterkeyBytes = Files.readAllBytes(masterkeyPath); + Path backupKeyPath = vaultPath.resolve(MASTERKEY_FILENAME + MasterkeyBackupHelper.generateFileIdSuffix(oldMasterkeyBytes) + MASTERKEY_BACKUP_SUFFIX); + Files.move(masterkeyPath, backupKeyPath, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE); + } + Files.write(masterkeyPath, restoredKey, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE); } finally { Arrays.fill(rawKey, (byte) 0x00); } 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 8cb7e0752..475ba92a5 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,6 +7,7 @@ import org.cryptomator.common.vaults.MountPointRequirement; 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.integrations.keychain.KeychainAccessException; import org.cryptomator.ui.common.Animations; @@ -85,7 +86,7 @@ public class UnlockWorkflow extends Task { } @Override - protected Boolean call() throws InterruptedException, IOException, VolumeException, InvalidMountPointException { + protected Boolean call() throws InterruptedException, IOException, VolumeException, InvalidMountPointException, CryptoException { try { if (attemptUnlock()) { handleSuccess(); @@ -100,7 +101,7 @@ public class UnlockWorkflow extends Task { } } - private boolean attemptUnlock() throws InterruptedException, IOException, VolumeException, InvalidMountPointException { + private boolean attemptUnlock() throws InterruptedException, IOException, VolumeException, InvalidMountPointException, CryptoException { boolean proceed = password.get() != null || askForPassword(false) == PasswordEntry.PASSWORD_ENTERED; while (proceed) { try { diff --git a/main/ui/src/test/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactoryTest.java b/main/ui/src/test/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactoryTest.java index 6ca1fd891..a8c7d4844 100644 --- a/main/ui/src/test/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactoryTest.java +++ b/main/ui/src/test/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactoryTest.java @@ -2,23 +2,40 @@ package org.cryptomator.ui.recoverykey; import com.google.common.base.Splitter; import org.cryptomator.cryptofs.CryptoFileSystemProvider; +import org.cryptomator.cryptolib.api.CryptoException; +import org.cryptomator.cryptolib.api.Masterkey; +import org.cryptomator.cryptolib.common.MasterkeyFile; +import org.cryptomator.cryptolib.common.MasterkeyFileLoader; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import java.io.IOException; import java.nio.file.Path; +import java.security.SecureRandom; class RecoveryKeyFactoryTest { private WordEncoder wordEncoder = new WordEncoder(); - private RecoveryKeyFactory inTest = new RecoveryKeyFactory(wordEncoder); + private SecureRandom csprng = Mockito.mock(SecureRandom.class); + private RecoveryKeyFactory inTest = new RecoveryKeyFactory(wordEncoder, csprng); @Test @DisplayName("createRecoveryKey() creates 44 words") - public void testCreateRecoveryKey(@TempDir Path pathToVault) throws IOException { - CryptoFileSystemProvider.initialize(pathToVault, "masterkey.cryptomator", "asd"); + public void testCreateRecoveryKey() throws IOException, CryptoException { + Path pathToVault = Path.of("path/to/vault"); + MockedStatic masterkeyFileClass = Mockito.mockStatic(MasterkeyFile.class); + MasterkeyFile masterkeyFile = Mockito.mock(MasterkeyFile.class); + MasterkeyFileLoader keyLoader = Mockito.mock(MasterkeyFileLoader.class); + Masterkey masterkey = Mockito.mock(Masterkey.class); + masterkeyFileClass.when(() -> MasterkeyFile.withContentFromFile(Path.of("path/to/vault/masterkey.cryptomator"))).thenReturn(masterkeyFile); + Mockito.when(masterkeyFile.unlock(Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(keyLoader); + Mockito.when(keyLoader.loadKeyAndClose()).thenReturn(masterkey); + Mockito.when(masterkey.getEncoded()).thenReturn(new byte[64]); + String recoveryKey = inTest.createRecoveryKey(pathToVault, "asd"); Assertions.assertNotNull(recoveryKey); Assertions.assertEquals(44, Splitter.on(' ').splitToList(recoveryKey).size()); // 66 bytes encoded as 44 words diff --git a/main/ui/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker b/main/ui/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker new file mode 100644 index 000000000..ca6ee9cea --- /dev/null +++ b/main/ui/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker @@ -0,0 +1 @@ +mock-maker-inline \ No newline at end of file From efebbc059a5343ec27d809a51a0d2480020a08e5 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Tue, 19 Jan 2021 15:09:47 +0100 Subject: [PATCH 02/29] keep CTR+HMAC for now (until GCM is supported on all platforms) --- .../ui/addvaultwizard/CreateNewVaultPasswordController.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/CreateNewVaultPasswordController.java b/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/CreateNewVaultPasswordController.java index 25cb45395..84dfbd02e 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/CreateNewVaultPasswordController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/CreateNewVaultPasswordController.java @@ -183,7 +183,7 @@ public class CreateNewVaultPasswordController implements FxController { // 2. verify masterkey and initialize vault: try (var loader = MasterkeyFile.withContentFromFile(masterkeyFilePath).unlock(passphrase, PEPPER, Optional.of(999))) { - CryptoFileSystemProperties fsProps = CryptoFileSystemProperties.cryptoFileSystemProperties().withKeyLoader(loader).build(); + CryptoFileSystemProperties fsProps = CryptoFileSystemProperties.cryptoFileSystemProperties().withCipherCombo(VaultCipherCombo.SIV_CTRMAC).withKeyLoader(loader).build(); CryptoFileSystemProvider.initialize(path, fsProps, MasterkeyFileLoader.KEY_ID); // 3. write vault-internal readme file: From 4b670a59a37b39174684adab9552f962fc9878e8 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Mon, 25 Jan 2021 21:31:16 +0100 Subject: [PATCH 03/29] adjusted to new cryptolib/cryptofs API --- .../org/cryptomator/common/CommonsModule.java | 7 +++ .../org/cryptomator/common/vaults/Vault.java | 59 +++++++++---------- main/pom.xml | 6 +- .../CreateNewVaultPasswordController.java | 25 ++++---- .../StaticMasterkeyFileLoaderContext.java | 28 +++++++++ .../ChangePasswordController.java | 11 ++-- .../ui/recoverykey/RecoveryKeyFactory.java | 11 ++-- .../cryptomator/ui/unlock/UnlockWorkflow.java | 24 ++++++-- .../main/resources/license/THIRD-PARTY.txt | 4 +- .../recoverykey/RecoveryKeyFactoryTest.java | 17 ++---- 10 files changed, 114 insertions(+), 78 deletions(-) create mode 100644 main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/StaticMasterkeyFileLoaderContext.java diff --git a/main/commons/src/main/java/org/cryptomator/common/CommonsModule.java b/main/commons/src/main/java/org/cryptomator/common/CommonsModule.java index 0ea3428ed..9126b758b 100644 --- a/main/commons/src/main/java/org/cryptomator/common/CommonsModule.java +++ b/main/commons/src/main/java/org/cryptomator/common/CommonsModule.java @@ -15,6 +15,7 @@ import org.cryptomator.common.settings.SettingsProvider; import org.cryptomator.common.vaults.Vault; import org.cryptomator.common.vaults.VaultComponent; import org.cryptomator.common.vaults.VaultListManager; +import org.cryptomator.cryptolib.common.MasterkeyFileAccess; import org.cryptomator.frontend.webdav.WebDavServer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -65,6 +66,12 @@ public abstract class CommonsModule { } } + @Provides + @Singleton + static MasterkeyFileAccess provideMasterkeyFileAccess(SecureRandom csprng) { + return new MasterkeyFileAccess(Constants.PEPPER, csprng); + } + @Provides @Singleton @Named("SemVer") 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 44c6897e4..0dff26096 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 @@ -20,8 +20,8 @@ import org.cryptomator.cryptofs.CryptoFileSystemProvider; import org.cryptomator.cryptofs.common.Constants; import org.cryptomator.cryptofs.common.FileSystemCapabilityChecker; import org.cryptomator.cryptolib.api.CryptoException; -import org.cryptomator.cryptolib.api.InvalidPassphraseException; -import org.cryptomator.cryptolib.common.MasterkeyFile; +import org.cryptomator.cryptolib.api.MasterkeyLoader; +import org.cryptomator.cryptolib.api.MasterkeyLoadingFailedException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -36,7 +36,6 @@ import javafx.beans.property.BooleanProperty; import javafx.beans.property.ObjectProperty; import javafx.beans.property.SimpleBooleanProperty; import java.io.IOException; -import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.Paths; import java.util.EnumSet; @@ -45,9 +44,6 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; -import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; -import static org.cryptomator.common.Constants.PEPPER; - @PerVault public class Vault { @@ -101,7 +97,7 @@ public class Vault { // Commands // ********************************************************************************/ - private CryptoFileSystem createCryptoFileSystem(CharSequence passphrase) throws NoSuchFileException, IOException, InvalidPassphraseException, CryptoException { + private CryptoFileSystem createCryptoFileSystem(MasterkeyLoader keyLoader) throws IOException, MasterkeyLoadingFailedException { Set flags = EnumSet.noneOf(FileSystemFlags.class); if (vaultSettings.usesReadOnlyMode().get()) { flags.add(FileSystemFlags.READONLY); @@ -114,20 +110,16 @@ public class Vault { } assert vaultSettings.filenameLengthLimit().get() > 0; - Path masterkeyPath = getPath().resolve(MASTERKEY_FILENAME); - try (var keyLoader = MasterkeyFile.withContentFromFile(masterkeyPath).unlock(passphrase, PEPPER, Optional.empty())) { - CryptoFileSystemProperties fsProps = CryptoFileSystemProperties.cryptoFileSystemProperties() // - .withKeyLoader(keyLoader) // - .withFlags(flags) // - .withMaxPathLength(vaultSettings.filenameLengthLimit().get() + Constants.MAX_ADDITIONAL_PATH_LENGTH) // - .withMaxNameLength(vaultSettings.filenameLengthLimit().get()) // - .build(); - return CryptoFileSystemProvider.newFileSystem(getPath(), fsProps); - } + CryptoFileSystemProperties fsProps = CryptoFileSystemProperties.cryptoFileSystemProperties() // + .withKeyLoaders(keyLoader) // + .withFlags(flags) // + .withMaxPathLength(vaultSettings.filenameLengthLimit().get() + Constants.MAX_ADDITIONAL_PATH_LENGTH) // + .withMaxNameLength(vaultSettings.filenameLengthLimit().get()) // + .build(); + return CryptoFileSystemProvider.newFileSystem(getPath(), fsProps); } - private void destroyCryptoFileSystem() { - CryptoFileSystem fs = cryptoFileSystem.getAndSet(null); + private void destroyCryptoFileSystem(CryptoFileSystem fs) { if (fs != null) { try { fs.close(); @@ -137,20 +129,22 @@ public class Vault { } } - public synchronized void unlock(CharSequence passphrase) throws CryptoException, IOException, VolumeException, InvalidMountPointException { - if (cryptoFileSystem.get() == null) { - CryptoFileSystem fs = createCryptoFileSystem(passphrase); - cryptoFileSystem.set(fs); - try { - volume = volumeProvider.get(); - volume.mount(fs, getEffectiveMountFlags()); - } catch (IOException | InvalidMountPointException | VolumeException e) { - destroyCryptoFileSystem(); - throw e; - } - } else { + public synchronized void unlock(MasterkeyLoader keyLoader) throws CryptoException, IOException, VolumeException, InvalidMountPointException { + if (cryptoFileSystem.get() != null) { throw new IllegalStateException("Already unlocked."); } + CryptoFileSystem fs = createCryptoFileSystem(keyLoader); + boolean success = false; + try { + cryptoFileSystem.set(fs); + volume = volumeProvider.get(); + volume.mount(fs, getEffectiveMountFlags()); + success = true; + } finally { + if (!success) { + destroyCryptoFileSystem(fs); + } + } } public synchronized void lock(boolean forced) throws VolumeException { @@ -159,7 +153,8 @@ public class Vault { } else { volume.unmount(); } - destroyCryptoFileSystem(); + CryptoFileSystem fs = cryptoFileSystem.getAndSet(null); + destroyCryptoFileSystem(fs); } public void reveal(Volume.Revealer vaultRevealer) throws VolumeException { diff --git a/main/pom.xml b/main/pom.xml index 26a093185..b23747cd8 100644 --- a/main/pom.xml +++ b/main/pom.xml @@ -24,7 +24,7 @@ UTF-8 - 2.0.0-beta1 + 2.0.0-beta2 0.1.6 0.2.1 0.1.0-beta3 @@ -38,8 +38,8 @@ 3.11 3.11.0 2.1.0 - 30.0-jre - 2.29.1 + 30.1-jre + 2.31 2.8.6 1.7.30 1.2.3 diff --git a/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/CreateNewVaultPasswordController.java b/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/CreateNewVaultPasswordController.java index 84dfbd02e..e4f02ac8e 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/CreateNewVaultPasswordController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/CreateNewVaultPasswordController.java @@ -7,10 +7,8 @@ import org.cryptomator.cryptofs.CryptoFileSystemProperties; import org.cryptomator.cryptofs.CryptoFileSystemProvider; import org.cryptomator.cryptofs.VaultCipherCombo; import org.cryptomator.cryptolib.api.CryptoException; -import org.cryptomator.cryptolib.api.InvalidPassphraseException; import org.cryptomator.cryptolib.api.Masterkey; -import org.cryptomator.cryptolib.api.UnsupportedVaultFormatException; -import org.cryptomator.cryptolib.common.MasterkeyFile; +import org.cryptomator.cryptolib.common.MasterkeyFileAccess; import org.cryptomator.cryptolib.common.MasterkeyFileLoader; import org.cryptomator.ui.common.ErrorComponent; import org.cryptomator.ui.common.FxController; @@ -36,7 +34,6 @@ import javafx.scene.control.ContentDisplay; import javafx.scene.control.Toggle; import javafx.scene.control.ToggleGroup; import javafx.stage.Stage; -import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.UncheckedIOException; import java.nio.channels.WritableByteChannel; @@ -46,14 +43,11 @@ import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.StandardOpenOption; import java.security.SecureRandom; -import java.util.Collections; -import java.util.Optional; import java.util.ResourceBundle; import java.util.concurrent.ExecutorService; import static java.nio.charset.StandardCharsets.US_ASCII; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; -import static org.cryptomator.common.Constants.PEPPER; @AddVaultWizardScoped public class CreateNewVaultPasswordController implements FxController { @@ -76,6 +70,7 @@ public class CreateNewVaultPasswordController implements FxController { private final ObjectProperty password; private final ReadmeGenerator readmeGenerator; private final SecureRandom csprng; + private final MasterkeyFileAccess masterkeyFileAccess; private final BooleanProperty processing; private final BooleanProperty readyToCreateVault; private final ObjectBinding createVaultButtonState; @@ -85,7 +80,7 @@ public class CreateNewVaultPasswordController implements FxController { public Toggle skipRecoveryKey; @Inject - CreateNewVaultPasswordController(@AddVaultWizardWindow Stage window, @FxmlScene(FxmlFile.ADDVAULT_NEW_LOCATION) Lazy chooseLocationScene, @FxmlScene(FxmlFile.ADDVAULT_NEW_RECOVERYKEY) Lazy recoveryKeyScene, @FxmlScene(FxmlFile.ADDVAULT_SUCCESS) Lazy successScene, ErrorComponent.Builder errorComponent, ExecutorService executor, RecoveryKeyFactory recoveryKeyFactory, @Named("vaultName") StringProperty vaultName, ObjectProperty vaultPath, @AddVaultWizardWindow ObjectProperty vault, @Named("recoveryKey") StringProperty recoveryKey, VaultListManager vaultListManager, ResourceBundle resourceBundle, @Named("newPassword") ObjectProperty password, ReadmeGenerator readmeGenerator, SecureRandom csprng) { + CreateNewVaultPasswordController(@AddVaultWizardWindow Stage window, @FxmlScene(FxmlFile.ADDVAULT_NEW_LOCATION) Lazy chooseLocationScene, @FxmlScene(FxmlFile.ADDVAULT_NEW_RECOVERYKEY) Lazy recoveryKeyScene, @FxmlScene(FxmlFile.ADDVAULT_SUCCESS) Lazy successScene, ErrorComponent.Builder errorComponent, ExecutorService executor, RecoveryKeyFactory recoveryKeyFactory, @Named("vaultName") StringProperty vaultName, ObjectProperty vaultPath, @AddVaultWizardWindow ObjectProperty vault, @Named("recoveryKey") StringProperty recoveryKey, VaultListManager vaultListManager, ResourceBundle resourceBundle, @Named("newPassword") ObjectProperty password, ReadmeGenerator readmeGenerator, SecureRandom csprng, MasterkeyFileAccess masterkeyFileAccess) { this.window = window; this.chooseLocationScene = chooseLocationScene; this.recoveryKeyScene = recoveryKeyScene; @@ -102,6 +97,7 @@ public class CreateNewVaultPasswordController implements FxController { this.password = password; this.readmeGenerator = readmeGenerator; this.csprng = csprng; + this.masterkeyFileAccess = masterkeyFileAccess; this.processing = new SimpleBooleanProperty(); this.readyToCreateVault = new SimpleBooleanProperty(); this.createVaultButtonState = Bindings.createObjectBinding(this::getCreateVaultButtonState, processing); @@ -177,14 +173,15 @@ public class CreateNewVaultPasswordController implements FxController { // 1. write masterkey: Path masterkeyFilePath = path.resolve(MASTERKEY_FILENAME); try (Masterkey masterkey = Masterkey.createNew(csprng)) { - byte[] serialized = MasterkeyFile.lock(masterkey, passphrase, PEPPER, 999, csprng); - Files.write(masterkeyFilePath, serialized, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE); + masterkeyFileAccess.persist(masterkey, masterkeyFilePath, passphrase); } - // 2. verify masterkey and initialize vault: - try (var loader = MasterkeyFile.withContentFromFile(masterkeyFilePath).unlock(passphrase, PEPPER, Optional.of(999))) { - CryptoFileSystemProperties fsProps = CryptoFileSystemProperties.cryptoFileSystemProperties().withCipherCombo(VaultCipherCombo.SIV_CTRMAC).withKeyLoader(loader).build(); - CryptoFileSystemProvider.initialize(path, fsProps, MasterkeyFileLoader.KEY_ID); + // 2. initialize vault: + var context = new StaticMasterkeyFileLoaderContext(masterkeyFilePath, passphrase); + var loader = masterkeyFileAccess.keyLoader(path, context); + try { + CryptoFileSystemProperties fsProps = CryptoFileSystemProperties.cryptoFileSystemProperties().withCipherCombo(VaultCipherCombo.SIV_CTRMAC).withKeyLoaders(loader).build(); + CryptoFileSystemProvider.initialize(path, fsProps, MasterkeyFileLoader.keyId(MASTERKEY_FILENAME)); // 3. write vault-internal readme file: String vaultReadmeFileName = resourceBundle.getString("addvault.new.readme.accessLocation.fileName"); diff --git a/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/StaticMasterkeyFileLoaderContext.java b/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/StaticMasterkeyFileLoaderContext.java new file mode 100644 index 000000000..4ccc06e3e --- /dev/null +++ b/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/StaticMasterkeyFileLoaderContext.java @@ -0,0 +1,28 @@ +package org.cryptomator.ui.addvaultwizard; + +import com.google.common.base.Preconditions; +import org.cryptomator.cryptolib.common.MasterkeyFileLoaderContext; + +import java.nio.file.Path; + +class StaticMasterkeyFileLoaderContext implements MasterkeyFileLoaderContext { + + private final Path masterkeyFilePath; + private final CharSequence passphrase; + + StaticMasterkeyFileLoaderContext(Path masterkeyFilePath, CharSequence passphrase) { + this.masterkeyFilePath = masterkeyFilePath; + this.passphrase = passphrase; + } + + @Override + public Path getMasterkeyFilePath(String s) { + return masterkeyFilePath; + } + + @Override + public CharSequence getPassphrase(Path path) { + Preconditions.checkArgument(masterkeyFilePath.equals(path)); + return passphrase; + } +} diff --git a/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordController.java b/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordController.java index 436103436..93c7e0576 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordController.java @@ -2,11 +2,10 @@ package org.cryptomator.ui.changepassword; import org.cryptomator.common.keychain.KeychainManager; import org.cryptomator.common.vaults.Vault; -import org.cryptomator.cryptofs.CryptoFileSystemProvider; import org.cryptomator.cryptofs.common.MasterkeyBackupHelper; import org.cryptomator.cryptolib.api.CryptoException; import org.cryptomator.cryptolib.api.InvalidPassphraseException; -import org.cryptomator.cryptolib.common.MasterkeyFile; +import org.cryptomator.cryptolib.common.MasterkeyFileAccess; import org.cryptomator.integrations.keychain.KeychainAccessException; import org.cryptomator.ui.common.Animations; import org.cryptomator.ui.common.ErrorComponent; @@ -31,8 +30,6 @@ import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; import java.security.SecureRandom; -import java.text.Normalizer; -import java.util.Optional; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; @@ -48,19 +45,21 @@ public class ChangePasswordController implements FxController { private final ErrorComponent.Builder errorComponent; private final KeychainManager keychain; private final SecureRandom csprng; + private final MasterkeyFileAccess masterkeyFileAccess; public NiceSecurePasswordField oldPasswordField; public CheckBox finalConfirmationCheckbox; public Button finishButton; @Inject - public ChangePasswordController(@ChangePasswordWindow Stage window, @ChangePasswordWindow Vault vault, @Named("newPassword") ObjectProperty newPassword, ErrorComponent.Builder errorComponent, KeychainManager keychain, SecureRandom csprng) { + public ChangePasswordController(@ChangePasswordWindow Stage window, @ChangePasswordWindow Vault vault, @Named("newPassword") ObjectProperty newPassword, ErrorComponent.Builder errorComponent, KeychainManager keychain, SecureRandom csprng, MasterkeyFileAccess masterkeyFileAccess) { this.window = window; this.vault = vault; this.newPassword = newPassword; this.errorComponent = errorComponent; this.keychain = keychain; this.csprng = csprng; + this.masterkeyFileAccess = masterkeyFileAccess; } @FXML @@ -85,7 +84,7 @@ public class ChangePasswordController implements FxController { CharSequence newPassphrase = newPassword.get(); // TODO verify: is this already NFC-normalized? Path masterkeyPath = vault.getPath().resolve(MASTERKEY_FILENAME); byte[] oldMasterkeyBytes = Files.readAllBytes(masterkeyPath); - byte[] newMasterkeyBytes = MasterkeyFile.changePassphrase(oldMasterkeyBytes, oldPassphrase, newPassphrase, new byte[0], csprng); + byte[] newMasterkeyBytes = masterkeyFileAccess.changePassphrase(oldMasterkeyBytes, oldPassphrase, newPassphrase); Path backupKeyPath = vault.getPath().resolve(MASTERKEY_FILENAME + MasterkeyBackupHelper.generateFileIdSuffix(oldMasterkeyBytes) + MASTERKEY_BACKUP_SUFFIX); Files.move(masterkeyPath, backupKeyPath, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE); Files.write(masterkeyPath, newMasterkeyBytes, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE); diff --git a/main/ui/src/main/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactory.java b/main/ui/src/main/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactory.java index a53113020..28aaa1d71 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactory.java +++ b/main/ui/src/main/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactory.java @@ -6,7 +6,7 @@ import org.cryptomator.cryptofs.common.MasterkeyBackupHelper; import org.cryptomator.cryptolib.api.CryptoException; import org.cryptomator.cryptolib.api.InvalidPassphraseException; import org.cryptomator.cryptolib.api.Masterkey; -import org.cryptomator.cryptolib.common.MasterkeyFile; +import org.cryptomator.cryptolib.common.MasterkeyFileAccess; import javax.inject.Inject; import javax.inject.Singleton; @@ -30,11 +30,13 @@ public class RecoveryKeyFactory { private final WordEncoder wordEncoder; private final SecureRandom csprng; + private final MasterkeyFileAccess masterkeyFileAccess; @Inject - public RecoveryKeyFactory(WordEncoder wordEncoder, SecureRandom csprng) { + public RecoveryKeyFactory(WordEncoder wordEncoder, SecureRandom csprng, MasterkeyFileAccess masterkeyFileAccess) { this.wordEncoder = wordEncoder; this.csprng = csprng; + this.masterkeyFileAccess = masterkeyFileAccess; } public Collection getDictionary() { @@ -53,7 +55,7 @@ public class RecoveryKeyFactory { public String createRecoveryKey(Path vaultPath, CharSequence password) throws IOException, InvalidPassphraseException, CryptoException { Path masterkeyPath = vaultPath.resolve(MASTERKEY_FILENAME); byte[] rawKey = new byte[0]; - try (var masterkey = MasterkeyFile.withContentFromFile(masterkeyPath).unlock(password, PEPPER, Optional.empty()).loadKeyAndClose()) { + try (var masterkey = masterkeyFileAccess.load(masterkeyPath, password)) { rawKey = masterkey.getEncoded(); return createRecoveryKey(rawKey); } finally { @@ -87,14 +89,13 @@ public class RecoveryKeyFactory { public void resetPasswordWithRecoveryKey(Path vaultPath, String recoveryKey, CharSequence newPassword) throws IOException, IllegalArgumentException { final byte[] rawKey = decodeRecoveryKey(recoveryKey); try (var masterkey = Masterkey.createFromRaw(rawKey)) { - byte[] restoredKey = MasterkeyFile.lock(masterkey, newPassword, PEPPER, 999, csprng); Path masterkeyPath = vaultPath.resolve(MASTERKEY_FILENAME); if (Files.exists(masterkeyPath)) { byte[] oldMasterkeyBytes = Files.readAllBytes(masterkeyPath); Path backupKeyPath = vaultPath.resolve(MASTERKEY_FILENAME + MasterkeyBackupHelper.generateFileIdSuffix(oldMasterkeyBytes) + MASTERKEY_BACKUP_SUFFIX); Files.move(masterkeyPath, backupKeyPath, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE); } - Files.write(masterkeyPath, restoredKey, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE); + masterkeyFileAccess.persist(masterkey, masterkeyPath, newPassword); } finally { Arrays.fill(rawKey, (byte) 0x00); } 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 475ba92a5..1624ba45f 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 @@ -9,6 +9,10 @@ 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.api.MasterkeyLoader; +import org.cryptomator.cryptolib.common.MasterkeyFileAccess; +import org.cryptomator.cryptolib.common.MasterkeyFileLoader; +import org.cryptomator.cryptolib.common.MasterkeyFileLoaderContext; import org.cryptomator.integrations.keychain.KeychainAccessException; import org.cryptomator.ui.common.Animations; import org.cryptomator.ui.common.ErrorComponent; @@ -32,6 +36,7 @@ import java.nio.CharBuffer; import java.nio.file.DirectoryNotEmptyException; import java.nio.file.FileAlreadyExistsException; import java.nio.file.NotDirectoryException; +import java.nio.file.Path; import java.util.Arrays; import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; @@ -43,7 +48,7 @@ import java.util.concurrent.atomic.AtomicReference; * This class runs the unlock process and controls when to display which UI. */ @UnlockScoped -public class UnlockWorkflow extends Task { +public class UnlockWorkflow extends Task implements MasterkeyFileLoaderContext { private static final Logger LOG = LoggerFactory.getLogger(UnlockWorkflow.class); @@ -59,9 +64,10 @@ public class UnlockWorkflow extends Task { private final Lazy successScene; private final Lazy invalidMountPointScene; private final ErrorComponent.Builder errorComponent; + private final MasterkeyFileAccess masterkeyFileAccess; @Inject - UnlockWorkflow(@UnlockWindow Stage window, @UnlockWindow Vault vault, VaultService vaultService, AtomicReference password, @Named("savePassword") AtomicBoolean savePassword, @Named("savedPassword") Optional savedPassword, UserInteractionLock passwordEntryLock, KeychainManager keychain, @FxmlScene(FxmlFile.UNLOCK) Lazy unlockScene, @FxmlScene(FxmlFile.UNLOCK_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.UNLOCK_INVALID_MOUNT_POINT) Lazy invalidMountPointScene, ErrorComponent.Builder errorComponent) { + UnlockWorkflow(@UnlockWindow Stage window, @UnlockWindow Vault vault, VaultService vaultService, AtomicReference password, @Named("savePassword") AtomicBoolean savePassword, @Named("savedPassword") Optional savedPassword, UserInteractionLock passwordEntryLock, KeychainManager keychain, @FxmlScene(FxmlFile.UNLOCK) Lazy unlockScene, @FxmlScene(FxmlFile.UNLOCK_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.UNLOCK_INVALID_MOUNT_POINT) Lazy invalidMountPointScene, ErrorComponent.Builder errorComponent, MasterkeyFileAccess masterkeyFileAccess) { this.window = window; this.vault = vault; this.vaultService = vaultService; @@ -74,6 +80,7 @@ public class UnlockWorkflow extends Task { this.successScene = successScene; this.invalidMountPointScene = invalidMountPointScene; this.errorComponent = errorComponent; + this.masterkeyFileAccess = masterkeyFileAccess; setOnFailed(event -> { Throwable throwable = event.getSource().getException(); @@ -105,7 +112,7 @@ public class UnlockWorkflow extends Task { boolean proceed = password.get() != null || askForPassword(false) == PasswordEntry.PASSWORD_ENTERED; while (proceed) { try { - vault.unlock(CharBuffer.wrap(password.get())); + vault.unlock(masterkeyFileAccess.keyLoader(vault.getPath(), this)); return true; } catch (InvalidPassphraseException e) { proceed = askForPassword(true) == PasswordEntry.PASSWORD_ENTERED; @@ -114,6 +121,16 @@ public class UnlockWorkflow extends Task { return false; } + @Override + public Path getMasterkeyFilePath(String masterkeyFilePath) { + return null; // TODO non-standard paths not yet supported (cancel unlock attempt) + } + + @Override + public CharSequence getPassphrase(Path path) { + return CharBuffer.wrap(password.get()); + } + private PasswordEntry askForPassword(boolean animateShake) throws InterruptedException { Platform.runLater(() -> { window.setScene(unlockScene.get()); @@ -238,5 +255,4 @@ public class UnlockWorkflow extends Task { protected void cancelled() { vault.setState(VaultState.LOCKED); } - } diff --git a/main/ui/src/main/resources/license/THIRD-PARTY.txt b/main/ui/src/main/resources/license/THIRD-PARTY.txt index 30a364776..6d60273ac 100644 --- a/main/ui/src/main/resources/license/THIRD-PARTY.txt +++ b/main/ui/src/main/resources/license/THIRD-PARTY.txt @@ -19,10 +19,10 @@ Cryptomator uses 46 third-party dependencies under the following licenses: - jnr-ffi (com.github.jnr:jnr-ffi:2.1.12 - http://github.com/jnr/jnr-ffi) - FindBugs-jsr305 (com.google.code.findbugs:jsr305:3.0.2 - http://findbugs.sourceforge.net/) - Gson (com.google.code.gson:gson:2.8.6 - https://github.com/google/gson/gson) - - Dagger (com.google.dagger:dagger:2.29.1 - https://github.com/google/dagger) + - Dagger (com.google.dagger:dagger:2.31 - https://github.com/google/dagger) - error-prone annotations (com.google.errorprone:error_prone_annotations:2.3.4 - http://nexus.sonatype.org/oss-repository-hosting.html/error_prone_parent/error_prone_annotations) - Guava InternalFutureFailureAccess and InternalFutures (com.google.guava:failureaccess:1.0.1 - https://github.com/google/guava/failureaccess) - - Guava: Google Core Libraries for Java (com.google.guava:guava:30.0-jre - https://github.com/google/guava/guava) + - Guava: Google Core Libraries for Java (com.google.guava:guava:30.1-jre - https://github.com/google/guava/guava) - Guava ListenableFuture only (com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava - https://github.com/google/guava/listenablefuture) - J2ObjC Annotations (com.google.j2objc:j2objc-annotations:1.3 - https://github.com/google/j2objc/) - Apache Commons CLI (commons-cli:commons-cli:1.4 - http://commons.apache.org/proper/commons-cli/) diff --git a/main/ui/src/test/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactoryTest.java b/main/ui/src/test/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactoryTest.java index a8c7d4844..88cd41b0b 100644 --- a/main/ui/src/test/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactoryTest.java +++ b/main/ui/src/test/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactoryTest.java @@ -1,16 +1,12 @@ package org.cryptomator.ui.recoverykey; import com.google.common.base.Splitter; -import org.cryptomator.cryptofs.CryptoFileSystemProvider; import org.cryptomator.cryptolib.api.CryptoException; import org.cryptomator.cryptolib.api.Masterkey; -import org.cryptomator.cryptolib.common.MasterkeyFile; -import org.cryptomator.cryptolib.common.MasterkeyFileLoader; +import org.cryptomator.cryptolib.common.MasterkeyFileAccess; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.io.TempDir; -import org.mockito.MockedStatic; import org.mockito.Mockito; import java.io.IOException; @@ -21,19 +17,16 @@ class RecoveryKeyFactoryTest { private WordEncoder wordEncoder = new WordEncoder(); private SecureRandom csprng = Mockito.mock(SecureRandom.class); - private RecoveryKeyFactory inTest = new RecoveryKeyFactory(wordEncoder, csprng); + private MasterkeyFileAccess masterkeyFileAccess = Mockito.mock(MasterkeyFileAccess.class); + private RecoveryKeyFactory inTest = new RecoveryKeyFactory(wordEncoder, csprng, masterkeyFileAccess); @Test @DisplayName("createRecoveryKey() creates 44 words") public void testCreateRecoveryKey() throws IOException, CryptoException { Path pathToVault = Path.of("path/to/vault"); - MockedStatic masterkeyFileClass = Mockito.mockStatic(MasterkeyFile.class); - MasterkeyFile masterkeyFile = Mockito.mock(MasterkeyFile.class); - MasterkeyFileLoader keyLoader = Mockito.mock(MasterkeyFileLoader.class); Masterkey masterkey = Mockito.mock(Masterkey.class); - masterkeyFileClass.when(() -> MasterkeyFile.withContentFromFile(Path.of("path/to/vault/masterkey.cryptomator"))).thenReturn(masterkeyFile); - Mockito.when(masterkeyFile.unlock(Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(keyLoader); - Mockito.when(keyLoader.loadKeyAndClose()).thenReturn(masterkey); + Mockito.when(masterkeyFileAccess.load(pathToVault.resolve("masterkey.cryptomator"), "asd")).thenReturn(masterkey); + Mockito.when(masterkey.getEncoded()).thenReturn(new byte[64]); String recoveryKey = inTest.createRecoveryKey(pathToVault, "asd"); From ff17b60f56316fb91b51766234f4d7b4bc6d8bb1 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 27 Jan 2021 12:09:09 +0100 Subject: [PATCH 04/29] Refactored UnlockWorkflow using new vault format 8 APIs --- main/pom.xml | 2 +- .../StaticMasterkeyFileLoaderContext.java | 2 +- .../cryptomator/ui/unlock/UnlockWorkflow.java | 77 ++++++++++++------- 3 files changed, 52 insertions(+), 29 deletions(-) diff --git a/main/pom.xml b/main/pom.xml index 0ba69d20f..edd0b1c91 100644 --- a/main/pom.xml +++ b/main/pom.xml @@ -24,7 +24,7 @@ UTF-8 - 2.0.0-beta2 + 2.0.0-beta3 0.1.6 0.2.1 0.1.0-beta3 diff --git a/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/StaticMasterkeyFileLoaderContext.java b/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/StaticMasterkeyFileLoaderContext.java index 4ccc06e3e..13c3b05b1 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/StaticMasterkeyFileLoaderContext.java +++ b/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/StaticMasterkeyFileLoaderContext.java @@ -16,7 +16,7 @@ class StaticMasterkeyFileLoaderContext implements MasterkeyFileLoaderContext { } @Override - public Path getMasterkeyFilePath(String s) { + public Path getCorrectMasterkeyFilePath(String s) { return masterkeyFilePath; } 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 0e6baf032..8c250665a 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 @@ -10,8 +10,8 @@ import org.cryptomator.common.vaults.Volume.VolumeException; import org.cryptomator.cryptolib.api.CryptoException; import org.cryptomator.cryptolib.api.InvalidPassphraseException; import org.cryptomator.cryptolib.api.MasterkeyLoader; +import org.cryptomator.cryptolib.api.MasterkeyLoadingFailedException; import org.cryptomator.cryptolib.common.MasterkeyFileAccess; -import org.cryptomator.cryptolib.common.MasterkeyFileLoader; import org.cryptomator.cryptolib.common.MasterkeyFileLoaderContext; import org.cryptomator.integrations.keychain.KeychainAccessException; import org.cryptomator.ui.common.Animations; @@ -66,6 +66,8 @@ public class UnlockWorkflow extends Task implements MasterkeyFileLoader private final ErrorComponent.Builder errorComponent; private final MasterkeyFileAccess masterkeyFileAccess; + private boolean didEnterWrongPassphrase = false; + @Inject UnlockWorkflow(@UnlockWindow Stage window, @UnlockWindow Vault vault, VaultService vaultService, AtomicReference password, @Named("savePassword") AtomicBoolean savePassword, @Named("savedPassword") Optional savedPassword, UserInteractionLock passwordEntryLock, KeychainManager keychain, @FxmlScene(FxmlFile.UNLOCK) Lazy unlockScene, @FxmlScene(FxmlFile.UNLOCK_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.UNLOCK_INVALID_MOUNT_POINT) Lazy invalidMountPointScene, ErrorComponent.Builder errorComponent, MasterkeyFileAccess masterkeyFileAccess) { this.window = window; @@ -95,43 +97,57 @@ public class UnlockWorkflow extends Task implements MasterkeyFileLoader @Override protected Boolean call() throws InterruptedException, IOException, VolumeException, InvalidMountPointException, CryptoException { try { - if (attemptUnlock()) { - handleSuccess(); - return true; - } else { - cancel(false); // set Tasks state to cancelled - return false; - } + MasterkeyLoader keyLoader = masterkeyFileAccess.keyLoader(vault.getPath(), this); + attemptUnlock(keyLoader, 0); + handleSuccess(); + return true; + } catch (PasswordEntryCancelledException e) { + cancel(false); // set Tasks state to cancelled + return false; } finally { wipePassword(password.get()); wipePassword(savedPassword.orElse(null)); } } - private boolean attemptUnlock() throws InterruptedException, IOException, VolumeException, InvalidMountPointException, CryptoException { - boolean proceed = password.get() != null || askForPassword(false) == PasswordEntry.PASSWORD_ENTERED; - while (proceed) { - try { - vault.unlock(masterkeyFileAccess.keyLoader(vault.getPath(), this)); - return true; - } catch (InvalidPassphraseException e) { - proceed = askForPassword(true) == PasswordEntry.PASSWORD_ENTERED; - } + private void attemptUnlock(MasterkeyLoader keyLoader, int attempt) throws IOException, VolumeException, InvalidMountPointException, CryptoException { + try { + vault.unlock(keyLoader); + } catch (InvalidPassphraseException e) { + LOG.info("Unlock attempt #{} failed due to incorrect password", attempt); + wipePassword(password.getAndSet(null)); + didEnterWrongPassphrase = true; + attemptUnlock(keyLoader, attempt + 1); } - return false; } @Override - public Path getMasterkeyFilePath(String masterkeyFilePath) { - return null; // TODO non-standard paths not yet supported (cancel unlock attempt) + public Path getCorrectMasterkeyFilePath(String masterkeyFilePath) { + LOG.warn("Did not find masterkey file at expected path: {}", masterkeyFilePath); + throw new MasterkeyLoadingFailedException(masterkeyFilePath + " not found.", new UnsupportedOperationException("getCorrectMasterkeyFilePath() not implemented")); } @Override - public CharSequence getPassphrase(Path path) { - return CharBuffer.wrap(password.get()); + public CharSequence getPassphrase(Path path) throws PasswordEntryCancelledException { + if (password.get() != null) { // e.g. pre-filled from keychain + return CharBuffer.wrap(password.get()); + } + + assert password.get() == null; + try { + if (askForPassphrase() == PasswordEntry.PASSWORD_ENTERED) { + assert password.get() != null; + return CharBuffer.wrap(password.get()); + } else { + throw new PasswordEntryCancelledException("Password entry cancelled."); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new PasswordEntryCancelledException("Password entry interrupted", e); + } } - private PasswordEntry askForPassword(boolean animateShake) throws InterruptedException { + private PasswordEntry askForPassphrase() throws InterruptedException { Platform.runLater(() -> { window.setScene(unlockScene.get()); window.show(); @@ -142,7 +158,7 @@ public class UnlockWorkflow extends Task implements MasterkeyFileLoader } else { window.centerOnScreen(); } - if (animateShake) { + if (didEnterWrongPassphrase) { Animations.createShakeWindowAnimation(window).play(); } }); @@ -191,15 +207,12 @@ public class UnlockWorkflow extends Task implements MasterkeyFileLoader LOG.error("Unlock failed. Mountpoint doesn't exist (needs to be a folder): {}", cause.getMessage()); } showInvalidMountPointScene(); - return; } else if (cause instanceof FileAlreadyExistsException) { LOG.error("Unlock failed. Mountpoint already exists: {}", cause.getMessage()); showInvalidMountPointScene(); - return; } else if (cause instanceof DirectoryNotEmptyException) { LOG.error("Unlock failed. Mountpoint not an empty directory: {}", cause.getMessage()); showInvalidMountPointScene(); - return; } else { handleGenericError(impExc); } @@ -241,4 +254,14 @@ public class UnlockWorkflow extends Task implements MasterkeyFileLoader protected void cancelled() { vault.setState(VaultState.LOCKED); } + + private static class PasswordEntryCancelledException extends MasterkeyLoadingFailedException { + public PasswordEntryCancelledException(String message) { + super(message); + } + + public PasswordEntryCancelledException(String message, Throwable cause) { + super(message, cause); + } + } } From b15471b4ff49962a85df4f604ea2f5ce3739c839 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 29 Jan 2021 17:44:45 +0100 Subject: [PATCH 05/29] add new (optional) "choose masterkey file" step to unlock dialog --- .../org/cryptomator/ui/common/FxmlFile.java | 1 + .../cryptomator/ui/unlock/UnlockModule.java | 31 ++++++++ .../UnlockSelectMasterkeyFileController.java | 76 +++++++++++++++++++ .../cryptomator/ui/unlock/UnlockWorkflow.java | 50 +++++++++--- .../fxml/unlock_select_masterkeyfile.fxml | 43 +++++++++++ .../main/resources/i18n/strings.properties | 2 + 6 files changed, 193 insertions(+), 10 deletions(-) create mode 100644 main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockSelectMasterkeyFileController.java create mode 100644 main/ui/src/main/resources/fxml/unlock_select_masterkeyfile.fxml diff --git a/main/ui/src/main/java/org/cryptomator/ui/common/FxmlFile.java b/main/ui/src/main/java/org/cryptomator/ui/common/FxmlFile.java index 310e11747..d79237baa 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/common/FxmlFile.java +++ b/main/ui/src/main/java/org/cryptomator/ui/common/FxmlFile.java @@ -28,6 +28,7 @@ public enum FxmlFile { REMOVE_VAULT("/fxml/remove_vault.fxml"), // UNLOCK("/fxml/unlock.fxml"), UNLOCK_INVALID_MOUNT_POINT("/fxml/unlock_invalid_mount_point.fxml"), // + UNLOCK_SELECT_MASTERKEYFILE("/fxml/unlock_select_masterkeyfile.fxml"), // UNLOCK_SUCCESS("/fxml/unlock_success.fxml"), // VAULT_OPTIONS("/fxml/vault_options.fxml"), // VAULT_STATISTICS("/fxml/stats.fxml"), // 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 da5e2884d..97821ab25 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 @@ -24,6 +24,7 @@ import javax.inject.Provider; import javafx.scene.Scene; import javafx.stage.Modality; import javafx.stage.Stage; +import java.nio.file.Path; import java.util.Map; import java.util.Optional; import java.util.ResourceBundle; @@ -40,12 +41,23 @@ abstract class UnlockModule { CANCELED } + public enum MasterkeyFileProvision { + MASTERKEYFILE_PROVIDED, + CANCELED + } + @Provides @UnlockScoped static UserInteractionLock providePasswordEntryLock() { return new UserInteractionLock<>(null); } + @Provides + @UnlockScoped + static UserInteractionLock provideMasterkeyFileProvisionLock() { + return new UserInteractionLock<>(null); + } + @Provides @Named("savedPassword") @UnlockScoped @@ -62,6 +74,13 @@ abstract class UnlockModule { } } + @Provides + @Named("userProvidedMasterkeyPath") + @UnlockScoped + static AtomicReference provideUserProvidedMasterkeyPath() { + return new AtomicReference(); + } + @Provides @UnlockScoped static AtomicReference providePassword(@Named("savedPassword") Optional storedPassword) { @@ -105,6 +124,13 @@ abstract class UnlockModule { return fxmlLoaders.createScene("/fxml/unlock.fxml"); } + @Provides + @FxmlScene(FxmlFile.UNLOCK_SELECT_MASTERKEYFILE) + @UnlockScoped + static Scene provideUnlockSelectMasterkeyFileScene(@UnlockWindow FXMLLoaderFactory fxmlLoaders) { + return fxmlLoaders.createScene("/fxml/unlock_select_masterkeyfile.fxml"); + } + @Provides @FxmlScene(FxmlFile.UNLOCK_SUCCESS) @UnlockScoped @@ -126,6 +152,11 @@ abstract class UnlockModule { @FxControllerKey(UnlockController.class) abstract FxController bindUnlockController(UnlockController controller); + @Binds + @IntoMap + @FxControllerKey(UnlockSelectMasterkeyFileController.class) + abstract FxController bindUnlockSelectMasterkeyFileController(UnlockSelectMasterkeyFileController controller); + @Binds @IntoMap @FxControllerKey(UnlockSuccessController.class) diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockSelectMasterkeyFileController.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockSelectMasterkeyFileController.java new file mode 100644 index 000000000..656f39b6d --- /dev/null +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockSelectMasterkeyFileController.java @@ -0,0 +1,76 @@ +package org.cryptomator.ui.unlock; + +import org.cryptomator.ui.common.FxController; +import org.cryptomator.ui.common.UserInteractionLock; +import org.cryptomator.ui.unlock.UnlockModule.MasterkeyFileProvision; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.inject.Inject; +import javax.inject.Named; +import javafx.beans.property.BooleanProperty; +import javafx.beans.property.SimpleBooleanProperty; +import javafx.fxml.FXML; +import javafx.stage.FileChooser; +import javafx.stage.Stage; +import javafx.stage.WindowEvent; +import java.io.File; +import java.nio.file.Path; +import java.util.ResourceBundle; +import java.util.concurrent.atomic.AtomicReference; + +@UnlockScoped +public class UnlockSelectMasterkeyFileController implements FxController { + + private static final Logger LOG = LoggerFactory.getLogger(UnlockSelectMasterkeyFileController.class); + + private final BooleanProperty proceedButtonDisabled = new SimpleBooleanProperty(); + private final Stage window; + private final AtomicReference masterkeyPath; + private final UserInteractionLock masterkeyFileProvisionLock; + private final ResourceBundle resourceBundle; + + @Inject + public UnlockSelectMasterkeyFileController(@UnlockWindow Stage window, @Named("userProvidedMasterkeyPath") AtomicReference masterkeyPath, UserInteractionLock masterkeyFileProvisionLock, ResourceBundle resourceBundle) { + this.window = window; + this.masterkeyPath = masterkeyPath; + this.masterkeyFileProvisionLock = masterkeyFileProvisionLock; + this.resourceBundle = resourceBundle; + this.window.setOnHiding(this::windowClosed); + } + + @FXML + public void cancel() { + window.close(); + } + + private void windowClosed(WindowEvent windowEvent) { + // if not already interacted, mark this workflow as cancelled: + if (masterkeyFileProvisionLock.awaitingInteraction().get()) { + LOG.debug("Unlock canceled by user."); + masterkeyFileProvisionLock.interacted(MasterkeyFileProvision.CANCELED); + } + } + + @FXML + public void proceed() { + LOG.trace("proceed()"); + FileChooser fileChooser = new FileChooser(); + fileChooser.setTitle(resourceBundle.getString("unlock.chooseMasterkey.filePickerTitle")); + fileChooser.getExtensionFilters().add(new FileChooser.ExtensionFilter("Cryptomator Masterkey", "*.cryptomator")); + File masterkeyFile = fileChooser.showOpenDialog(window); + if (masterkeyFile != null) { + LOG.debug("Chose masterkey file: {}", masterkeyFile); + masterkeyPath.set(masterkeyFile.toPath()); + masterkeyFileProvisionLock.interacted(MasterkeyFileProvision.MASTERKEYFILE_PROVIDED); + } + } + + public BooleanProperty proceedButtonDisabledProperty() { + return proceedButtonDisabled; + } + + public boolean isProceedButtonDisabled() { + return proceedButtonDisabled.get(); + } +} 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 8c250665a..2c6e3be65 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 @@ -20,6 +20,7 @@ import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; import org.cryptomator.ui.common.UserInteractionLock; import org.cryptomator.ui.common.VaultService; +import org.cryptomator.ui.unlock.UnlockModule.MasterkeyFileProvision; import org.cryptomator.ui.unlock.UnlockModule.PasswordEntry; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -58,9 +59,12 @@ public class UnlockWorkflow extends Task implements MasterkeyFileLoader private final AtomicReference password; private final AtomicBoolean savePassword; private final Optional savedPassword; + private final AtomicReference correctMasterkeyPath; private final UserInteractionLock passwordEntryLock; + private final UserInteractionLock masterkeyFileProvisionLock; private final KeychainManager keychain; private final Lazy unlockScene; + private final Lazy selectMasterkeyFileScene; private final Lazy successScene; private final Lazy invalidMountPointScene; private final ErrorComponent.Builder errorComponent; @@ -69,16 +73,19 @@ public class UnlockWorkflow extends Task implements MasterkeyFileLoader private boolean didEnterWrongPassphrase = false; @Inject - UnlockWorkflow(@UnlockWindow Stage window, @UnlockWindow Vault vault, VaultService vaultService, AtomicReference password, @Named("savePassword") AtomicBoolean savePassword, @Named("savedPassword") Optional savedPassword, UserInteractionLock passwordEntryLock, KeychainManager keychain, @FxmlScene(FxmlFile.UNLOCK) Lazy unlockScene, @FxmlScene(FxmlFile.UNLOCK_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.UNLOCK_INVALID_MOUNT_POINT) Lazy invalidMountPointScene, ErrorComponent.Builder errorComponent, MasterkeyFileAccess masterkeyFileAccess) { + UnlockWorkflow(@UnlockWindow Stage window, @UnlockWindow Vault vault, VaultService vaultService, AtomicReference password, @Named("savePassword") AtomicBoolean savePassword, @Named("savedPassword") Optional savedPassword, @Named("userProvidedMasterkeyPath") AtomicReference correctMasterkeyPath, UserInteractionLock passwordEntryLock, UserInteractionLock masterkeyFileProvisionLock, KeychainManager keychain, @FxmlScene(FxmlFile.UNLOCK) Lazy unlockScene, @FxmlScene(FxmlFile.UNLOCK_SELECT_MASTERKEYFILE) Lazy selectMasterkeyFileScene, @FxmlScene(FxmlFile.UNLOCK_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.UNLOCK_INVALID_MOUNT_POINT) Lazy invalidMountPointScene, ErrorComponent.Builder errorComponent, MasterkeyFileAccess masterkeyFileAccess) { this.window = window; this.vault = vault; this.vaultService = vaultService; this.password = password; this.savePassword = savePassword; this.savedPassword = savedPassword; + this.correctMasterkeyPath = correctMasterkeyPath; this.passwordEntryLock = passwordEntryLock; + this.masterkeyFileProvisionLock = masterkeyFileProvisionLock; this.keychain = keychain; this.unlockScene = unlockScene; + this.selectMasterkeyFileScene = selectMasterkeyFileScene; this.successScene = successScene; this.invalidMountPointScene = invalidMountPointScene; this.errorComponent = errorComponent; @@ -101,7 +108,7 @@ public class UnlockWorkflow extends Task implements MasterkeyFileLoader attemptUnlock(keyLoader, 0); handleSuccess(); return true; - } catch (PasswordEntryCancelledException e) { + } catch (UnlockCancelledException e) { cancel(false); // set Tasks state to cancelled return false; } finally { @@ -123,12 +130,35 @@ public class UnlockWorkflow extends Task implements MasterkeyFileLoader @Override public Path getCorrectMasterkeyFilePath(String masterkeyFilePath) { - LOG.warn("Did not find masterkey file at expected path: {}", masterkeyFilePath); - throw new MasterkeyLoadingFailedException(masterkeyFilePath + " not found.", new UnsupportedOperationException("getCorrectMasterkeyFilePath() not implemented")); + try { + if (askForCorrectMasterkeyFile() == MasterkeyFileProvision.MASTERKEYFILE_PROVIDED) { + return correctMasterkeyPath.get(); + } else { + throw new UnlockCancelledException("Password entry cancelled."); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new UnlockCancelledException("Password entry interrupted", e); + } + } + + private MasterkeyFileProvision askForCorrectMasterkeyFile() throws InterruptedException { + Platform.runLater(() -> { + window.setScene(selectMasterkeyFileScene.get()); + window.show(); + Window owner = window.getOwner(); + if (owner != null) { + window.setX(owner.getX() + (owner.getWidth() - window.getWidth()) / 2); + window.setY(owner.getY() + (owner.getHeight() - window.getHeight()) / 2); + } else { + window.centerOnScreen(); + } + }); + return masterkeyFileProvisionLock.awaitInteraction(); } @Override - public CharSequence getPassphrase(Path path) throws PasswordEntryCancelledException { + public CharSequence getPassphrase(Path path) throws UnlockCancelledException { if (password.get() != null) { // e.g. pre-filled from keychain return CharBuffer.wrap(password.get()); } @@ -139,11 +169,11 @@ public class UnlockWorkflow extends Task implements MasterkeyFileLoader assert password.get() != null; return CharBuffer.wrap(password.get()); } else { - throw new PasswordEntryCancelledException("Password entry cancelled."); + throw new UnlockCancelledException("Password entry cancelled."); } } catch (InterruptedException e) { Thread.currentThread().interrupt(); - throw new PasswordEntryCancelledException("Password entry interrupted", e); + throw new UnlockCancelledException("Password entry interrupted", e); } } @@ -255,12 +285,12 @@ public class UnlockWorkflow extends Task implements MasterkeyFileLoader vault.setState(VaultState.LOCKED); } - private static class PasswordEntryCancelledException extends MasterkeyLoadingFailedException { - public PasswordEntryCancelledException(String message) { + private static class UnlockCancelledException extends MasterkeyLoadingFailedException { + public UnlockCancelledException(String message) { super(message); } - public PasswordEntryCancelledException(String message, Throwable cause) { + public UnlockCancelledException(String message, Throwable cause) { super(message, cause); } } diff --git a/main/ui/src/main/resources/fxml/unlock_select_masterkeyfile.fxml b/main/ui/src/main/resources/fxml/unlock_select_masterkeyfile.fxml new file mode 100644 index 000000000..91a643ef6 --- /dev/null +++ b/main/ui/src/main/resources/fxml/unlock_select_masterkeyfile.fxml @@ -0,0 +1,43 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +