From 34995088ba5d199c69603831bd3e53d5cb9756f7 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 23 Apr 2021 10:24:31 +0200 Subject: [PATCH] addressed some issues identified during code review --- .../main/java/org/cryptomator/common/Constants.java | 1 + .../java/org/cryptomator/common/vaults/Vault.java | 5 +++-- .../org/cryptomator/common/vaults/VaultModule.java | 4 ---- .../ui/changepassword/ChangePasswordController.java | 2 +- .../ui/recoverykey/RecoveryKeyFactory.java | 12 +++--------- .../resources/fxml/unlock_select_masterkeyfile.fxml | 2 +- main/ui/src/main/resources/i18n/strings.properties | 1 + 7 files changed, 10 insertions(+), 17 deletions(-) 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 cbde9642a..06dedfc2d 100644 --- a/main/commons/src/main/java/org/cryptomator/common/Constants.java +++ b/main/commons/src/main/java/org/cryptomator/common/Constants.java @@ -3,6 +3,7 @@ package org.cryptomator.common; public interface Constants { String MASTERKEY_FILENAME = "masterkey.cryptomator"; + String MASTERKEY_BACKUP_SUFFIX = ".bkup"; 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 dfbeabffb..e36369c35 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 @@ -53,6 +53,7 @@ public class Vault { private static final Logger LOG = LoggerFactory.getLogger(Vault.class); private static final Path HOME_DIR = Paths.get(SystemUtils.USER_HOME); + private static final int UNLIMITED_FILENAME_LENGTH = Integer.MAX_VALUE; private final VaultSettings vaultSettings; private final Provider volumeProvider; @@ -114,11 +115,11 @@ public class Vault { int cleartextLimit = checker.determineSupportedCleartextFileNameLength(getPath()); vaultSettings.maxCleartextFilenameLength().set(cleartextLimit); } else { - vaultSettings.maxCleartextFilenameLength().setValue(Integer.MAX_VALUE); + vaultSettings.maxCleartextFilenameLength().setValue(UNLIMITED_FILENAME_LENGTH); } } - if (vaultSettings.maxCleartextFilenameLength().get() < Integer.MAX_VALUE) { + if (vaultSettings.maxCleartextFilenameLength().get() < UNLIMITED_FILENAME_LENGTH) { LOG.warn("Limiting cleartext filename length on this device to {}.", vaultSettings.maxCleartextFilenameLength().get()); } diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java index 97bc8d625..19d577975 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java @@ -8,12 +8,10 @@ package org.cryptomator.common.vaults; import dagger.Module; import dagger.Provides; import org.apache.commons.lang3.SystemUtils; -import org.cryptomator.common.Constants; import org.cryptomator.common.settings.Settings; import org.cryptomator.common.settings.VaultSettings; import org.cryptomator.common.settings.VolumeImpl; import org.cryptomator.cryptofs.CryptoFileSystem; -import org.cryptomator.cryptofs.VaultConfig; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -26,11 +24,9 @@ import javafx.beans.property.ObjectProperty; import javafx.beans.property.ReadOnlyBooleanProperty; import javafx.beans.property.SimpleObjectProperty; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Optional; import java.util.concurrent.atomic.AtomicReference; @Module 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 93c7e0576..b0ad164e2 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 @@ -31,13 +31,13 @@ import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; import java.security.SecureRandom; +import static org.cryptomator.common.Constants.MASTERKEY_BACKUP_SUFFIX; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; @ChangePasswordScoped 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; 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 ee159e072..c078c718b 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 @@ -14,28 +14,21 @@ 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_BACKUP_SUFFIX; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; -import static org.cryptomator.common.Constants.PEPPER; @Singleton public class RecoveryKeyFactory { - private static final String MASTERKEY_BACKUP_SUFFIX = ".bkup"; - private final WordEncoder wordEncoder; - private final SecureRandom csprng; private final MasterkeyFileAccess masterkeyFileAccess; @Inject - public RecoveryKeyFactory(WordEncoder wordEncoder, SecureRandom csprng, MasterkeyFileAccess masterkeyFileAccess) { + public RecoveryKeyFactory(WordEncoder wordEncoder, MasterkeyFileAccess masterkeyFileAccess) { this.wordEncoder = wordEncoder; - this.csprng = csprng; this.masterkeyFileAccess = masterkeyFileAccess; } @@ -92,6 +85,7 @@ public class RecoveryKeyFactory { Path masterkeyPath = vaultPath.resolve(MASTERKEY_FILENAME); if (Files.exists(masterkeyPath)) { byte[] oldMasterkeyBytes = Files.readAllBytes(masterkeyPath); + // TODO: deduplicate with ChangePasswordController: Path backupKeyPath = vaultPath.resolve(MASTERKEY_FILENAME + MasterkeyBackupHelper.generateFileIdSuffix(oldMasterkeyBytes) + MASTERKEY_BACKUP_SUFFIX); Files.move(masterkeyPath, backupKeyPath, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE); } diff --git a/main/ui/src/main/resources/fxml/unlock_select_masterkeyfile.fxml b/main/ui/src/main/resources/fxml/unlock_select_masterkeyfile.fxml index 493f76e70..b6539f88f 100644 --- a/main/ui/src/main/resources/fxml/unlock_select_masterkeyfile.fxml +++ b/main/ui/src/main/resources/fxml/unlock_select_masterkeyfile.fxml @@ -26,7 +26,7 @@ - diff --git a/main/ui/src/main/resources/i18n/strings.properties b/main/ui/src/main/resources/i18n/strings.properties index c2c173784..2f5332358 100644 --- a/main/ui/src/main/resources/i18n/strings.properties +++ b/main/ui/src/main/resources/i18n/strings.properties @@ -101,6 +101,7 @@ unlock.passwordPrompt=Enter password for "%s": unlock.savePassword=Remember Password unlock.unlockBtn=Unlock ## +unlock.chooseMasterkey.prompt=Could not find the masterkey file for this vault at its expected location. Please choose the key file manually. unlock.chooseMasterkey.filePickerTitle=Select Masterkey File ## Success unlock.success.message=Unlocked "%s" successfully! Your vault is now accessible via its virtual drive.