mirror of
https://github.com/cryptomator/cryptomator.git
synced 2026-05-17 10:11:27 +00:00
Merge pull request #2161 from cryptomator/feature/check-vaultconfig-sig-during-key-recovery
Prevent key recovery for foreign vault
This commit is contained in:
@@ -7,6 +7,7 @@ import org.cryptomator.cryptolib.api.CryptoException;
|
||||
import org.cryptomator.cryptolib.api.InvalidPassphraseException;
|
||||
import org.cryptomator.cryptolib.api.Masterkey;
|
||||
import org.cryptomator.cryptolib.common.MasterkeyFileAccess;
|
||||
import org.jetbrains.annotations.Nullable;
|
||||
|
||||
import javax.inject.Inject;
|
||||
import javax.inject.Singleton;
|
||||
@@ -16,6 +17,7 @@ import java.nio.file.Path;
|
||||
import java.nio.file.StandardCopyOption;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collection;
|
||||
import java.util.function.Predicate;
|
||||
|
||||
import static org.cryptomator.common.Constants.MASTERKEY_BACKUP_SUFFIX;
|
||||
import static org.cryptomator.common.Constants.MASTERKEY_FILENAME;
|
||||
@@ -102,12 +104,29 @@ public class RecoveryKeyFactory {
|
||||
* @return <code>true</code> if this seems to be a legitimate recovery key
|
||||
*/
|
||||
public boolean validateRecoveryKey(String recoveryKey) {
|
||||
return validateRecoveryKey(recoveryKey, null);
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether a String is a syntactically correct recovery key with a valid checksum and passes the extended validation.
|
||||
*
|
||||
* @param recoveryKey A word sequence which might be a recovery key
|
||||
* @param extendedValidation Additional verification of the decoded key (optional)
|
||||
* @return <code>true</code> if this seems to be a legitimate recovery key and passes the extended validation
|
||||
*/
|
||||
public boolean validateRecoveryKey(String recoveryKey, @Nullable Predicate<byte[]> extendedValidation) {
|
||||
byte[] key = new byte[0];
|
||||
try {
|
||||
byte[] key = decodeRecoveryKey(recoveryKey);
|
||||
Arrays.fill(key, (byte) 0x00);
|
||||
return true;
|
||||
key = decodeRecoveryKey(recoveryKey);
|
||||
if (extendedValidation != null) {
|
||||
return extendedValidation.test(key);
|
||||
} else {
|
||||
return true;
|
||||
}
|
||||
} catch (IllegalArgumentException e) {
|
||||
return false;
|
||||
} finally {
|
||||
Arrays.fill(key, (byte) 0x00);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -4,7 +4,9 @@ import dagger.Binds;
|
||||
import dagger.Module;
|
||||
import dagger.Provides;
|
||||
import dagger.multibindings.IntoMap;
|
||||
import org.cryptomator.common.Nullable;
|
||||
import org.cryptomator.common.vaults.Vault;
|
||||
import org.cryptomator.cryptofs.VaultConfig;
|
||||
import org.cryptomator.ui.common.DefaultSceneFactory;
|
||||
import org.cryptomator.ui.common.FxController;
|
||||
import org.cryptomator.ui.common.FxControllerKey;
|
||||
@@ -22,12 +24,25 @@ import javafx.beans.property.StringProperty;
|
||||
import javafx.scene.Scene;
|
||||
import javafx.stage.Modality;
|
||||
import javafx.stage.Stage;
|
||||
import java.io.IOException;
|
||||
import java.util.Map;
|
||||
import java.util.ResourceBundle;
|
||||
|
||||
@Module
|
||||
abstract class RecoveryKeyModule {
|
||||
|
||||
@Provides
|
||||
@Nullable
|
||||
@RecoveryKeyWindow
|
||||
@RecoveryKeyScoped
|
||||
static VaultConfig.UnverifiedVaultConfig vaultConfig(@RecoveryKeyWindow Vault vault) {
|
||||
try {
|
||||
return vault.getVaultConfigCache().get();
|
||||
} catch (IOException e) {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
@Provides
|
||||
@RecoveryKeyWindow
|
||||
@RecoveryKeyScoped
|
||||
|
||||
@@ -3,10 +3,16 @@ package org.cryptomator.ui.recoverykey;
|
||||
import com.google.common.base.CharMatcher;
|
||||
import com.google.common.base.Strings;
|
||||
import dagger.Lazy;
|
||||
import org.cryptomator.common.Nullable;
|
||||
import org.cryptomator.common.vaults.Vault;
|
||||
import org.cryptomator.cryptofs.VaultConfig;
|
||||
import org.cryptomator.cryptofs.VaultConfigLoadException;
|
||||
import org.cryptomator.cryptofs.VaultKeyInvalidException;
|
||||
import org.cryptomator.ui.common.FxController;
|
||||
import org.cryptomator.ui.common.FxmlFile;
|
||||
import org.cryptomator.ui.common.FxmlScene;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
|
||||
import javax.inject.Inject;
|
||||
import javafx.beans.binding.Bindings;
|
||||
@@ -24,10 +30,12 @@ import java.util.Optional;
|
||||
@RecoveryKeyScoped
|
||||
public class RecoveryKeyRecoverController implements FxController {
|
||||
|
||||
private final static CharMatcher ALLOWED_CHARS = CharMatcher.inRange('a', 'z').or(CharMatcher.is(' '));
|
||||
private static final Logger LOG = LoggerFactory.getLogger(RecoveryKeyCreationController.class);
|
||||
private static final CharMatcher ALLOWED_CHARS = CharMatcher.inRange('a', 'z').or(CharMatcher.is(' '));
|
||||
|
||||
private final Stage window;
|
||||
private final Vault vault;
|
||||
private final VaultConfig.UnverifiedVaultConfig unverifiedVaultConfig;
|
||||
private final StringProperty recoveryKey;
|
||||
private final RecoveryKeyFactory recoveryKeyFactory;
|
||||
private final BooleanBinding validRecoveryKey;
|
||||
@@ -37,9 +45,10 @@ public class RecoveryKeyRecoverController implements FxController {
|
||||
public TextArea textarea;
|
||||
|
||||
@Inject
|
||||
public RecoveryKeyRecoverController(@RecoveryKeyWindow Stage window, @RecoveryKeyWindow Vault vault, @RecoveryKeyWindow StringProperty recoveryKey, RecoveryKeyFactory recoveryKeyFactory, @FxmlScene(FxmlFile.RECOVERYKEY_RESET_PASSWORD) Lazy<Scene> resetPasswordScene) {
|
||||
public RecoveryKeyRecoverController(@RecoveryKeyWindow Stage window, @RecoveryKeyWindow Vault vault, @RecoveryKeyWindow @Nullable VaultConfig.UnverifiedVaultConfig unverifiedVaultConfig, @RecoveryKeyWindow StringProperty recoveryKey, RecoveryKeyFactory recoveryKeyFactory, @FxmlScene(FxmlFile.RECOVERYKEY_RESET_PASSWORD) Lazy<Scene> resetPasswordScene) {
|
||||
this.window = window;
|
||||
this.vault = vault;
|
||||
this.unverifiedVaultConfig = unverifiedVaultConfig;
|
||||
this.recoveryKey = recoveryKey;
|
||||
this.recoveryKeyFactory = recoveryKeyFactory;
|
||||
this.validRecoveryKey = Bindings.createBooleanBinding(this::isValidRecoveryKey, recoveryKey);
|
||||
@@ -96,6 +105,20 @@ public class RecoveryKeyRecoverController implements FxController {
|
||||
window.setScene(resetPasswordScene.get());
|
||||
}
|
||||
|
||||
private boolean checkKeyAgainstVaultConfig(byte[] key) {
|
||||
try {
|
||||
var config = unverifiedVaultConfig.verify(key, unverifiedVaultConfig.allegedVaultVersion());
|
||||
LOG.info("Provided recovery key matches vault config signature for vault {}", config.getId());
|
||||
return true;
|
||||
} catch (VaultKeyInvalidException e) {
|
||||
LOG.debug("Provided recovery key does not match vault config signature.");
|
||||
return false;
|
||||
} catch (VaultConfigLoadException e) {
|
||||
LOG.error("Failed to parse vault config", e);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/* Getter/Setter */
|
||||
|
||||
public Vault getVault() {
|
||||
@@ -107,7 +130,11 @@ public class RecoveryKeyRecoverController implements FxController {
|
||||
}
|
||||
|
||||
public boolean isValidRecoveryKey() {
|
||||
return recoveryKeyFactory.validateRecoveryKey(recoveryKey.get());
|
||||
if (unverifiedVaultConfig != null) {
|
||||
return recoveryKeyFactory.validateRecoveryKey(recoveryKey.get(), this::checkKeyAgainstVaultConfig);
|
||||
} else {
|
||||
return recoveryKeyFactory.validateRecoveryKey(recoveryKey.get());
|
||||
}
|
||||
}
|
||||
|
||||
public TextFormatter getRecoveryKeyTextFormatter() {
|
||||
|
||||
@@ -7,10 +7,13 @@ 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.params.ParameterizedTest;
|
||||
import org.junit.jupiter.params.provider.ValueSource;
|
||||
import org.mockito.Mockito;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.nio.file.Path;
|
||||
import java.util.function.Predicate;
|
||||
|
||||
public class RecoveryKeyFactoryTest {
|
||||
|
||||
@@ -75,4 +78,19 @@ public class RecoveryKeyFactoryTest {
|
||||
Assertions.assertTrue(result);
|
||||
}
|
||||
|
||||
@ParameterizedTest(name = "passing validation = {0}")
|
||||
@DisplayName("validateRecoveryKey() with extended validation")
|
||||
@ValueSource(booleans = {true, false})
|
||||
public void testValidateValidateRecoveryKeyWithValidKey(boolean extendedValidationResult) {
|
||||
Predicate<byte[]> validator = Mockito.mock(Predicate.class);
|
||||
Mockito.doReturn(extendedValidationResult).when(validator).test(Mockito.any());
|
||||
boolean result = inTest.validateRecoveryKey("""
|
||||
pathway lift abuse plenty export texture gentleman landscape beyond ceiling around leaf cafe charity \
|
||||
border breakdown victory surely computer cat linger restrict infer crowd live computer true written amazed \
|
||||
investor boot depth left theory snow whereby terminal weekly reject happiness circuit partial cup ad \
|
||||
""", validator);
|
||||
Mockito.verify(validator).test(Mockito.any());
|
||||
Assertions.assertEquals(extendedValidationResult, result);
|
||||
}
|
||||
|
||||
}
|
||||
Reference in New Issue
Block a user