From 08d9beb6b8c4316c8feb4ecb0f7e64ddf1b5d7ce Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Tue, 8 Oct 2019 14:58:55 +0200 Subject: [PATCH] Externalized logic of recovery key creation to reusable utility class --- .../cryptomator/ui/fxapp/FxApplication.java | 2 - .../RecoveryKeyCreationController.java | 19 +---- .../ui/recoverykey/RecoveryKeyFactory.java | 78 +++++++++++++++++++ .../ui/recoverykey/WordEncoder.java | 7 +- .../recoverykey/RecoveryKeyFactoryTest.java | 59 ++++++++++++++ 5 files changed, 147 insertions(+), 18 deletions(-) create mode 100644 main/ui/src/main/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactory.java create mode 100644 main/ui/src/test/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactoryTest.java diff --git a/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java b/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java index 0935cbf1d..f52a9b2ed 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java +++ b/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java @@ -26,8 +26,6 @@ import org.slf4j.LoggerFactory; import javax.inject.Inject; import java.awt.desktop.QuitResponse; import java.util.Optional; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionStage; @FxApplicationScoped public class FxApplication extends Application { 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 5b466311c..b69b6612e 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 @@ -24,23 +24,22 @@ import java.util.concurrent.ExecutorService; public class RecoveryKeyCreationController implements FxController { private static final Logger LOG = LoggerFactory.getLogger(RecoveryKeyCreationController.class); - private static final String MASTERKEY_FILENAME = "masterkey.cryptomator"; // TODO: deduplicate constant declared in multiple classes private final Stage window; private final Vault vault; private final ExecutorService executor; private final CharSequence prefilledPassword; - private final WordEncoder wordEncoder; + private final RecoveryKeyFactory recoveryKeyFactory; private final StringProperty recoveryKey; public NiceSecurePasswordField passwordField; @Inject - public RecoveryKeyCreationController(@RecoveryKeyWindow Stage window, @RecoveryKeyWindow Vault vault, ExecutorService executor, @Nullable CharSequence prefilledPassword) { + public RecoveryKeyCreationController(@RecoveryKeyWindow Stage window, @RecoveryKeyWindow Vault vault, RecoveryKeyFactory recoveryKeyFactory, ExecutorService executor, @Nullable CharSequence prefilledPassword) { this.window = window; this.vault = vault; this.executor = executor; this.prefilledPassword = prefilledPassword; - this.wordEncoder = new WordEncoder(); + this.recoveryKeyFactory = recoveryKeyFactory; this.recoveryKey = new SimpleStringProperty(); } @@ -54,17 +53,7 @@ public class RecoveryKeyCreationController implements FxController { @FXML public void createRecoveryKey() { Tasks.create(() -> { - byte[] rawKey = CryptoFileSystemProvider.exportRawKey(vault.getPath(), MASTERKEY_FILENAME, new byte[0], passwordField.getCharacters()); - assert rawKey.length == 64; - byte[] paddedKey = Arrays.copyOf(rawKey, 66); - // TODO add two-byte CRC - - try { - return wordEncoder.encodePadded(paddedKey); - } finally { - Arrays.fill(rawKey, (byte) 0x00); - Arrays.fill(paddedKey, (byte) 0x00); - } + return recoveryKeyFactory.createRecoveryKey(vault.getPath(), passwordField.getCharacters()); }).onSuccess(result -> { recoveryKey.set(result); }).onError(IOException.class, e -> { 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 new file mode 100644 index 000000000..00e4002e2 --- /dev/null +++ b/main/ui/src/main/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactory.java @@ -0,0 +1,78 @@ +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.cryptolib.api.InvalidPassphraseException; + +import javax.inject.Inject; +import javax.inject.Singleton; +import java.io.IOException; +import java.nio.file.Path; +import java.util.Arrays; + +@Singleton +public class RecoveryKeyFactory { + + private static final String MASTERKEY_FILENAME = "masterkey.cryptomator"; // TODO: deduplicate constant declared in multiple classes + + private final WordEncoder wordEncoder; + + @Inject + public RecoveryKeyFactory(WordEncoder wordEncoder) { + this.wordEncoder = wordEncoder; + } + + /** + * @param vaultPath Path to the storage location of a vault + * @param password The vault's password + * @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 + * @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, new byte[0], password); + try { + return createRecoveryKey(rawKey); + } finally { + Arrays.fill(rawKey, (byte) 0x00); + } + } + + // visible for testing + String createRecoveryKey(byte[] rawKey) { + Preconditions.checkArgument(rawKey.length == 64, "key should be 64 bytes"); + byte[] paddedKey = Arrays.copyOf(rawKey, 66); + try { + // copy 16 most significant bits of CRC32(rawKey) to the end of paddedKey: + Hashing.crc32().hashBytes(rawKey).writeBytesTo(paddedKey, 64, 2); + return wordEncoder.encodePadded(paddedKey); + } finally { + Arrays.fill(paddedKey, (byte) 0x00); + } + } + + /** + * Checks whether a String is a syntactically correct recovery key with a valid checksum + * @param recoveryKey A word sequence which might be a recovery key + * @return true if this seems to be a legitimate recovery key + */ + public boolean validateRecoveryKey(String recoveryKey) { + final byte[] paddedKey; + try { + paddedKey = wordEncoder.decode(recoveryKey); + } catch (IllegalArgumentException e) { + return false; + } + if (paddedKey.length != 66) { + return false; + } + byte[] rawKey = Arrays.copyOf(paddedKey, 64); + byte[] expectedCrc16 = Arrays.copyOfRange(paddedKey, 64, 66); + byte[] actualCrc32 = Hashing.crc32().hashBytes(rawKey).asBytes(); + byte[] actualCrc16 = Arrays.copyOf(actualCrc32, 2); + return Arrays.equals(expectedCrc16, actualCrc16); + } + +} diff --git a/main/ui/src/main/java/org/cryptomator/ui/recoverykey/WordEncoder.java b/main/ui/src/main/java/org/cryptomator/ui/recoverykey/WordEncoder.java index 2fc02c183..b06d06902 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/recoverykey/WordEncoder.java +++ b/main/ui/src/main/java/org/cryptomator/ui/recoverykey/WordEncoder.java @@ -3,6 +3,8 @@ package org.cryptomator.ui.recoverykey; import com.google.common.base.Preconditions; import com.google.common.base.Splitter; +import javax.inject.Inject; +import javax.inject.Singleton; import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; @@ -14,16 +16,19 @@ import java.util.Map; import java.util.stream.Collectors; import java.util.stream.IntStream; +@Singleton class WordEncoder { + private static final String DEFAULT_WORD_FILE = "/i18n/4096words_en.txt"; private static final int WORD_COUNT = 4096; private static final char DELIMITER = ' '; private final List words; private final Map indices; + @Inject public WordEncoder() { - this("/i18n/4096words_en.txt"); + this(DEFAULT_WORD_FILE); } public WordEncoder(String wordFile) { 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 new file mode 100644 index 000000000..99ee549fd --- /dev/null +++ b/main/ui/src/test/java/org/cryptomator/ui/recoverykey/RecoveryKeyFactoryTest.java @@ -0,0 +1,59 @@ +package org.cryptomator.ui.recoverykey; + +import com.google.common.base.Splitter; +import org.cryptomator.cryptofs.CryptoFileSystemProvider; +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 java.io.IOException; +import java.nio.file.Path; + +class RecoveryKeyFactoryTest { + + private WordEncoder wordEncoder = new WordEncoder(); + private RecoveryKeyFactory inTest = new RecoveryKeyFactory(wordEncoder); + + @Test + @DisplayName("createRecoveryKey() creates 44 words") + public void testCreateRecoveryKey(@TempDir Path pathToVault) throws IOException { + CryptoFileSystemProvider.initialize(pathToVault, "masterkey.cryptomator", "asd"); + String recoveryKey = inTest.createRecoveryKey(pathToVault, "asd"); + Assertions.assertNotNull(recoveryKey); + Assertions.assertEquals(44, Splitter.on(' ').splitToList(recoveryKey).size()); // 66 bytes encoded as 44 words + } + + @Test + @DisplayName("validateRecoveryKey() with garbage input") + public void testValidateValidateRecoveryKeyWithGarbageInput() { + boolean result = inTest.validateRecoveryKey("löl"); + Assertions.assertFalse(result); + } + + @Test + @DisplayName("validateRecoveryKey() with too short input") + public void testValidateValidateRecoveryKeyWithTooShortInput() { + boolean result = inTest.validateRecoveryKey("them circumstances"); + Assertions.assertFalse(result); + } + + @Test + @DisplayName("validateRecoveryKey() with invalid crc32/16") + public void testValidateValidateRecoveryKeyWithInvalidCrc() { + boolean result = inTest.validateRecoveryKey("them circumstances conduct providing have gesture aged extraordinary generally silently" + + " beasts rights sit country highest career wrought silently liberal altogether capacity David conscious word issue" + + " ancient directed solitary how spain look smile see won't although dying obtain vol with c. asleep along listen circumstances"); + Assertions.assertFalse(result); + } + + @Test + @DisplayName("validateRecoveryKey() with valid key") + public void testValidateValidateRecoveryKeyWithValidKey() { + boolean result = inTest.validateRecoveryKey("them circumstances conduct providing have gesture aged extraordinary generally silently" + + " beasts rights sit country highest career wrought silently liberal altogether capacity David conscious word issue" + + " ancient directed solitary how spain look smile see won't although dying obtain vol with c. asleep along listen riding"); + Assertions.assertTrue(result); + } + +} \ No newline at end of file