From 2e0908ab151363ed8dc644e930a16af296f08b09 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Fri, 21 Apr 2023 09:40:34 +0200 Subject: [PATCH] replace config manually instead of using CryptoFileSystem.init() --- .../HubToPasswordConvertController.java | 59 +++++++----- .../HubToPasswordConvertControllerTest.java | 95 +++++++++++++++---- 2 files changed, 114 insertions(+), 40 deletions(-) diff --git a/src/main/java/org/cryptomator/ui/convertvault/HubToPasswordConvertController.java b/src/main/java/org/cryptomator/ui/convertvault/HubToPasswordConvertController.java index b796dd6b6..6300f0682 100644 --- a/src/main/java/org/cryptomator/ui/convertvault/HubToPasswordConvertController.java +++ b/src/main/java/org/cryptomator/ui/convertvault/HubToPasswordConvertController.java @@ -2,12 +2,12 @@ package org.cryptomator.ui.convertvault; import com.google.common.base.Preconditions; import dagger.Lazy; +import org.cryptomator.common.Constants; import org.cryptomator.common.Passphrase; import org.cryptomator.common.vaults.Vault; -import org.cryptomator.cryptofs.CryptoFileSystemProperties; -import org.cryptomator.cryptofs.CryptoFileSystemProvider; +import org.cryptomator.cryptofs.VaultConfig; +import org.cryptomator.cryptofs.VaultVersionMismatchException; import org.cryptomator.cryptofs.common.BackupHelper; -import org.cryptomator.cryptolib.api.MasterkeyLoader; import org.cryptomator.cryptolib.api.MasterkeyLoadingFailedException; import org.cryptomator.cryptolib.common.MasterkeyFileAccess; import org.cryptomator.ui.changepassword.NewPasswordController; @@ -31,6 +31,7 @@ import javafx.scene.control.Button; import javafx.scene.control.ContentDisplay; import javafx.stage.Stage; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; @@ -39,7 +40,8 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.ExecutorService; -import static org.cryptomator.common.Constants.DEFAULT_KEY_ID; +import static java.nio.file.StandardOpenOption.CREATE_NEW; +import static java.nio.file.StandardOpenOption.WRITE; import static org.cryptomator.common.Constants.MASTERKEY_BACKUP_SUFFIX; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; import static org.cryptomator.common.Constants.VAULTCONFIG_FILENAME; @@ -100,9 +102,9 @@ public class HubToPasswordConvertController implements FxController { @FXML public void convert() { Preconditions.checkState(newPasswordController.isGoodPassword()); - LOG.info("Converting hub vault {} to password", vault.getPath()); + LOG.info("Converting access method of vault {} from hub to password", vault.getPath()); CompletableFuture.runAsync(() -> conversionStarted.setValue(true), Platform::runLater) // - .thenRunAsync(this::convertInternal, backgroundExecutorService) + .thenRunAsync(this::convertInternal, backgroundExecutorService) // .whenCompleteAsync((result, exception) -> { if (exception == null) { LOG.info("Conversion of vault {} succeeded.", vault.getPath()); @@ -117,11 +119,19 @@ public class HubToPasswordConvertController implements FxController { //visible for testing void convertInternal() throws CompletionException, IllegalArgumentException { var passphrase = newPasswordController.getNewPassword(); + var vaultPath = vault.getPath(); try { - recoveryKeyFactory.newMasterkeyFileWithPassphrase(vault.getPath(), recoveryKey.get(), passphrase); - LOG.debug("Successfully created masterkey file for vault {}", vault.getPath()); - backupHubConfig(vault.getPath().resolve(VAULTCONFIG_FILENAME)); - replaceWithPasswordConfig(passphrase); + //create masterkey + recoveryKeyFactory.newMasterkeyFileWithPassphrase(vaultPath, recoveryKey.get(), passphrase); + LOG.debug("Successfully created masterkey file for vault {}", vaultPath); + //create password config + Path passwordConfigPath = vaultPath.resolve("passwordBased." + VAULTCONFIG_FILENAME + ".tmp"); + passwordConfigPath = createPasswordConfig(passwordConfigPath, vaultPath.resolve(MASTERKEY_FILENAME), passphrase); + //backup hub config + var hubConfigPath = vaultPath.resolve(VAULTCONFIG_FILENAME); + backupHubConfig(hubConfigPath); + //replace hub by password + Files.move(passwordConfigPath, hubConfigPath, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE); } catch (MasterkeyLoadingFailedException e) { throw new CompletionException(new IOException("Vault conversion failed", e)); } catch (IOException e) { @@ -134,25 +144,28 @@ public class HubToPasswordConvertController implements FxController { //visible for testing void backupHubConfig(Path hubConfigPath) throws IOException { byte[] hubConfigBytes = Files.readAllBytes(hubConfigPath); - Path backupPath = vault.getPath().resolve(VAULTCONFIG_FILENAME + BackupHelper.generateFileIdSuffix(hubConfigBytes) + MASTERKEY_BACKUP_SUFFIX); - Files.move(hubConfigPath, backupPath, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE); //TODO: should this be an atomic move? - LOG.debug("Successfully created vault config backup {} for vault {}", backupPath.getFileName(), vault.getPath()); + Path backupPath = hubConfigPath.resolveSibling(VAULTCONFIG_FILENAME + BackupHelper.generateFileIdSuffix(hubConfigBytes) + MASTERKEY_BACKUP_SUFFIX); + Files.copy(hubConfigPath, backupPath, StandardCopyOption.REPLACE_EXISTING); + LOG.debug("Successfully created hub config backup {}", backupPath.getFileName()); } //visible for testing - void replaceWithPasswordConfig(Passphrase passphrase) throws IOException, MasterkeyLoadingFailedException { + Path createPasswordConfig(Path passwordConfigPath, Path masterkeyFile, Passphrase passphrase) throws IOException, MasterkeyLoadingFailedException { var unverifiedVaultConfig = vault.getVaultConfigCache().get(); - try (var masterkey = masterkeyFileAccess.load(vault.getPath().resolve(MASTERKEY_FILENAME), passphrase)) { - var vaultConfig = unverifiedVaultConfig.verify(masterkey.getEncoded(), unverifiedVaultConfig.allegedVaultVersion()); - MasterkeyLoader loader = ignored -> masterkey.copy(); - CryptoFileSystemProperties fsProps = CryptoFileSystemProperties.cryptoFileSystemProperties() // - .withCipherCombo(vaultConfig.getCipherCombo()) // - .withKeyLoader(loader) // + try (var masterkey = masterkeyFileAccess.load(masterkeyFile, passphrase)) { + var hubConfig = unverifiedVaultConfig.verify(masterkey.getEncoded(), unverifiedVaultConfig.allegedVaultVersion()); + var passwordConfig = VaultConfig.createNew() // + .cipherCombo(hubConfig.getCipherCombo()) // + .shorteningThreshold(hubConfig.getShorteningThreshold()) // .build(); - CryptoFileSystemProvider.initialize(vault.getPath(), fsProps, DEFAULT_KEY_ID); + if (passwordConfig.getVaultVersion() != hubConfig.getVaultVersion()) { + throw new VaultVersionMismatchException("Only vaults of version " + passwordConfig.getVaultVersion() + "can be converted."); + } + var token = passwordConfig.toToken(Constants.DEFAULT_KEY_ID.toString(), masterkey.getEncoded()); + Files.writeString(passwordConfigPath, token, StandardCharsets.US_ASCII, WRITE, CREATE_NEW); + LOG.debug("Successfully created password config {}", passwordConfigPath); + return passwordConfigPath; } } - /* Getter/Setter */ - } diff --git a/src/test/java/org/cryptomator/ui/convertvault/HubToPasswordConvertControllerTest.java b/src/test/java/org/cryptomator/ui/convertvault/HubToPasswordConvertControllerTest.java index fb5eac044..63e3c5faa 100644 --- a/src/test/java/org/cryptomator/ui/convertvault/HubToPasswordConvertControllerTest.java +++ b/src/test/java/org/cryptomator/ui/convertvault/HubToPasswordConvertControllerTest.java @@ -4,6 +4,10 @@ import dagger.Lazy; import org.cryptomator.common.Constants; import org.cryptomator.common.Passphrase; import org.cryptomator.common.vaults.Vault; +import org.cryptomator.common.vaults.VaultConfigCache; +import org.cryptomator.cryptofs.VaultConfig; +import org.cryptomator.cryptolib.api.CryptorProvider; +import org.cryptomator.cryptolib.api.Masterkey; import org.cryptomator.cryptolib.api.MasterkeyLoadingFailedException; import org.cryptomator.cryptolib.common.MasterkeyFileAccess; import org.cryptomator.ui.changepassword.NewPasswordController; @@ -11,6 +15,7 @@ import org.cryptomator.ui.fxapp.FxApplicationWindows; import org.cryptomator.ui.recoverykey.RecoveryKeyFactory; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -25,17 +30,24 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; +import java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; import java.util.Optional; import java.util.ResourceBundle; import java.util.concurrent.CompletionException; import java.util.concurrent.ExecutorService; +import java.util.concurrent.atomic.AtomicReference; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.times; public class HubToPasswordConvertControllerTest { + @TempDir + Path tmpDir; + Stage window; Vault vault; StringProperty recoveryKey; @@ -66,14 +78,14 @@ public class HubToPasswordConvertControllerTest { newPasswordController = Mockito.mock(NewPasswordController.class); inTest = new HubToPasswordConvertController(window, successScene, appWindows, vault, recoveryKey, recoveryKeyFactory, masterkeyFileAccess, backgroundExecutorService, resourceBundle); inTest.newPasswordController = newPasswordController; + Mockito.when(vault.getPath()).thenReturn(tmpDir); } @Test - public void testBackupHubConfig(@TempDir Path tmpDir) throws IOException { + public void testBackupHubConfig() throws IOException { + var configContent = "Hello Config!".getBytes(); Path configPath = tmpDir.resolve(Constants.VAULTCONFIG_FILENAME); - Files.writeString(configPath, "hello config", StandardCharsets.UTF_8, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING); - - Mockito.when(vault.getPath()).thenReturn(tmpDir); + Files.write(configPath, configContent, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE); inTest.backupHubConfig(configPath); Optional result = Files.list(tmpDir).filter(p -> { @@ -81,17 +93,60 @@ public class HubToPasswordConvertControllerTest { return fileName.startsWith(Constants.VAULTCONFIG_FILENAME) && fileName.endsWith(Constants.MASTERKEY_BACKUP_SUFFIX); }).findAny(); - Assertions.assertTrue(Files.notExists(configPath)); + Assertions.assertTrue(Files.exists(configPath)); Assertions.assertTrue(result.isPresent()); - Assertions.assertEquals("hello config", Files.readString(result.get(), StandardCharsets.UTF_8)); + Assertions.assertArrayEquals(configContent, Files.readAllBytes(result.get())); + } + + + @Test + @DisplayName("createPasswordConfig creates valid config with password key id") + public void integrationTestCreatePasswordConfig(@TempDir Path tmpDir) throws NoSuchAlgorithmException, IOException { + //prepare + var csprng = SecureRandom.getInstanceStrong(); + var key = Masterkey.generate(csprng); + var masterkeyPath = tmpDir.resolve("masterkey"); + MasterkeyFileAccess mkAccess = new MasterkeyFileAccess(Constants.PEPPER, csprng); + mkAccess.persist(key, masterkeyPath, ""); + + var config = VaultConfig.createNew().cipherCombo(CryptorProvider.Scheme.SIV_GCM).shorteningThreshold(42).build(); + var token = config.toToken("test", key.getEncoded()); + var hubConfig = VaultConfig.decode(token); + var configCache = Mockito.mock(VaultConfigCache.class); + Mockito.when(vault.getVaultConfigCache()).thenReturn(configCache); + Mockito.when(configCache.get()).thenReturn(hubConfig); + + var passwordConfigPath = tmpDir.resolve("passwordConfig"); + + inTest = new HubToPasswordConvertController(window, successScene, appWindows, vault, recoveryKey, recoveryKeyFactory, mkAccess, backgroundExecutorService, resourceBundle); + + //execute + Path result = inTest.createPasswordConfig(passwordConfigPath, masterkeyPath, Passphrase.copyOf("")); + + //check + AtomicReference unverifiedCfg = new AtomicReference<>(); + AtomicReference cfg = new AtomicReference<>(); + Assertions.assertTrue(Files.exists(result)); + Assertions.assertDoesNotThrow(() -> { + var tmp = VaultConfig.decode(Files.readString(result, StandardCharsets.US_ASCII)); + unverifiedCfg.set(tmp); + }); + Assertions.assertDoesNotThrow(() -> { + var tmp = unverifiedCfg.get().verify(key.getEncoded(), config.getVaultVersion()); + cfg.set(tmp); + }); + Assertions.assertAll( // + () -> Assertions.assertEquals(config.getCipherCombo(), cfg.get().getCipherCombo()), // + () -> Assertions.assertEquals(config.getVaultVersion(), cfg.get().getVaultVersion()), // + () -> Assertions.assertEquals(config.getShorteningThreshold(), cfg.get().getShorteningThreshold()), // + () -> Assertions.assertEquals(Constants.DEFAULT_KEY_ID, unverifiedCfg.get().getKeyId())); } @Nested class ConvertInternalTests { + Passphrase passphrase = Mockito.mock(Passphrase.class); - Path vaultPath = Mockito.mock(Path.class, "/vault/path"); - Path configPath = Mockito.mock(Path.class, "/vault/path/config"); String actualRecoveryKey = "recoveryKey"; HubToPasswordConvertController inSpy; @@ -100,28 +155,33 @@ public class HubToPasswordConvertControllerTest { inSpy = Mockito.spy(inTest); Mockito.when(newPasswordController.getNewPassword()).thenReturn(passphrase); Mockito.when(recoveryKey.get()).thenReturn(actualRecoveryKey); - Mockito.when(vault.getPath()).thenReturn(vaultPath); - Mockito.when(vaultPath.resolve(anyString())).thenReturn(configPath); Mockito.doNothing().when(recoveryKeyFactory).newMasterkeyFileWithPassphrase(any(), anyString(), any()); Mockito.doNothing().when(inSpy).backupHubConfig(any()); - Mockito.doNothing().when(inSpy).replaceWithPasswordConfig(any()); Mockito.doNothing().when(passphrase).destroy(); } @Test public void testConvertInternal() throws IOException { + var passwordConfigContent = "Hello Config!".getBytes(); + Path passwordConfigPath = tmpDir.resolve("passwordConfig"); + Path configPath = tmpDir.resolve(Constants.VAULTCONFIG_FILENAME); + Files.write(passwordConfigPath, passwordConfigContent, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE); + Mockito.doReturn(passwordConfigPath).when(inSpy).createPasswordConfig(any(), any(), eq(passphrase)); + inSpy.convertInternal(); - Mockito.verify(recoveryKeyFactory, times(1)).newMasterkeyFileWithPassphrase(vaultPath, actualRecoveryKey, passphrase); - Mockito.verify(inSpy, times(1)).backupHubConfig(configPath); - Mockito.verify(inSpy, times(1)).replaceWithPasswordConfig(passphrase); - Mockito.verify(passphrase, times(1)).destroy(); + var inOrder = Mockito.inOrder(inSpy, recoveryKeyFactory, passphrase); + inOrder.verify(recoveryKeyFactory).newMasterkeyFileWithPassphrase(tmpDir, actualRecoveryKey, passphrase); + inOrder.verify(inSpy).createPasswordConfig(any(), Mockito.any(), eq(passphrase)); + inOrder.verify(inSpy).backupHubConfig(configPath); + inOrder.verify(passphrase).destroy(); + Assertions.assertArrayEquals(passwordConfigContent, Files.readAllBytes(configPath)); } @Test public void testConvertInternalWrapsCryptoException() throws IOException { - Mockito.doThrow(new MasterkeyLoadingFailedException("yadda")).when(inSpy).replaceWithPasswordConfig(any()); + Mockito.doThrow(new MasterkeyLoadingFailedException("yadda")).when(inSpy).createPasswordConfig(any(), any(), any()); Assertions.assertThrows(CompletionException.class, inSpy::convertInternal); @@ -130,6 +190,7 @@ public class HubToPasswordConvertControllerTest { @Test public void testConvertInternalWrapsIOException() throws IOException { + Mockito.doReturn(Mockito.mock(Path.class)).when(inSpy).createPasswordConfig(any(), any(), eq(passphrase)); Mockito.doThrow(new IOException("yudu")).when(inSpy).backupHubConfig(any()); Assertions.assertThrows(CompletionException.class, inSpy::convertInternal); @@ -139,7 +200,7 @@ public class HubToPasswordConvertControllerTest { @Test public void testConvertInternalNotWrapsIAE() throws IOException { - Mockito.doThrow(new IllegalArgumentException("yudu")).when(recoveryKeyFactory).newMasterkeyFileWithPassphrase(any(), anyString(), any()); + Mockito.doThrow(new IllegalArgumentException("yolo")).when(recoveryKeyFactory).newMasterkeyFileWithPassphrase(any(), anyString(), any()); Assertions.assertThrows(IllegalArgumentException.class, inSpy::convertInternal);