Refactor Keyloading to use only the required info

* new record KeyLoadingRef(config, name)
* masterkey vaults still need Vault.java
* hub relies on KeyLoadingRef

Signed-off-by: Armin Schrenk <armin.schrenk@skymatic.de>
This commit is contained in:
Armin Schrenk
2026-08-31 15:53:54 +02:00
parent b01478f0fa
commit d2b1f86d85
15 changed files with 167 additions and 81 deletions
+1 -1
View File
@@ -33,7 +33,7 @@
<nonModularGroupIds>org.ow2.asm,org.apache.jackrabbit,org.apache.httpcomponents</nonModularGroupIds> <nonModularGroupIds>org.ow2.asm,org.apache.jackrabbit,org.apache.httpcomponents</nonModularGroupIds>
<!-- cryptomator dependencies --> <!-- cryptomator dependencies -->
<cryptomator.cryptofs.version>2.10.0</cryptomator.cryptofs.version> <cryptomator.cryptofs.version>2.11.0-SNAPSHOT</cryptomator.cryptofs.version>
<cryptomator.cryptolib.version>2.2.2</cryptomator.cryptolib.version> <cryptomator.cryptolib.version>2.2.2</cryptomator.cryptolib.version>
<cryptomator.integrations.version>1.9.0</cryptomator.integrations.version> <cryptomator.integrations.version>1.9.0</cryptomator.integrations.version>
<cryptomator.integrations.win.version>1.6.1</cryptomator.integrations.win.version> <cryptomator.integrations.win.version>1.6.1</cryptomator.integrations.win.version>
@@ -18,15 +18,16 @@ import java.util.UUID;
/** /**
* Requests opening a Hub vault from an {@code org.cryptomator://vault/open#vaultConfig=…} deeplink. * Requests opening a Hub vault from an {@code org.cryptomator://vault/open#vaultConfig=…} deeplink.
* <p> * <p>
* The single parameter is the vault's {@code vault.cryptomator}, a compact JWS embedded verbatim - a compact JWS * The single parameter is the vault's {@code vault.cryptomator}, a compact JWS embedded verbatim, carried in the fragment part.
* consists only of characters that are unreserved in a URI fragment, so it needs no further encoding. It is carried in
* the fragment part so that it stays out of the logs of any web page that mints such a link.
* <p> * <p>
* The config is read <em>unverified</em>: its signature is keyed on the masterkey, which is only obtainable from Hub * Notes:
* later on. Everything consumed before that point is therefore validated here rather than trusted. * <ul>
* <li> The config is read <em>unverified</em>, since its signature is keyed on the masterkey, which is only obtainable from Hu later on. </li>
* <li> The deeplink parsing makes a strict validation due to untrusted input</li>
* </ul>
* *
* @param vaultConfig the decoded, unverified vault config * @param vaultConfig the decoded, unverified vault config
* @param vaultId the vault's id within its Hub instance, taken from the config's key id * @param vaultId the vault's id within its Hub instance, taken from the config's {@code jti} claim
*/ */
public record OpenHubVaultEvent(VaultConfig.UnverifiedVaultConfig vaultConfig, UUID vaultId) implements AppLaunchEvent { public record OpenHubVaultEvent(VaultConfig.UnverifiedVaultConfig vaultConfig, UUID vaultId) implements AppLaunchEvent {
@@ -59,7 +60,7 @@ public record OpenHubVaultEvent(VaultConfig.UnverifiedVaultConfig vaultConfig, U
} }
var vaultConfig = decode(token); var vaultConfig = decode(token);
requireHubVault(vaultConfig); requireHubVault(vaultConfig);
var vaultId = extractVaultId(vaultConfig.getKeyId()); var vaultId = extractVaultId(vaultConfig);
var leftoverParams = params.keySet().stream().filter(k -> !k.equals(PARAM_VAULT_CONFIG)).toList(); var leftoverParams = params.keySet().stream().filter(k -> !k.equals(PARAM_VAULT_CONFIG)).toList();
if (!leftoverParams.isEmpty()) { if (!leftoverParams.isEmpty()) {
@@ -123,8 +124,8 @@ public record OpenHubVaultEvent(VaultConfig.UnverifiedVaultConfig vaultConfig, U
if (!uri.isAbsolute() || uri.getHost() == null) { if (!uri.isAbsolute() || uri.getHost() == null) {
throw new IllegalArgumentException("Vault config's hub " + field + " is not an absolute url with a host, but was '" + uri + "'."); throw new IllegalArgumentException("Vault config's hub " + field + " is not an absolute url with a host, but was '" + uri + "'.");
} }
// Whether an http host is acceptable (it is, for local development) is decided by CheckHostTrustController, the // Whether an http host is acceptable (it is, for local development) is decided by CheckHostTrustController
// authority on host trust. Here we only ensure the endpoint is shaped like something that decision can be made on. // Here we only ensure the endpoint is shaped like something that decision can be made on.
var scheme = uri.getScheme(); var scheme = uri.getScheme();
if (!"https".equalsIgnoreCase(scheme) && !"http".equalsIgnoreCase(scheme)) { if (!"https".equalsIgnoreCase(scheme) && !"http".equalsIgnoreCase(scheme)) {
throw new IllegalArgumentException("Vault config's hub " + field + " is neither http nor https, but was '" + uri + "'."); throw new IllegalArgumentException("Vault config's hub " + field + " is neither http nor https, but was '" + uri + "'.");
@@ -132,24 +133,33 @@ public record OpenHubVaultEvent(VaultConfig.UnverifiedVaultConfig vaultConfig, U
} }
/** /**
* Reads the vault id from the trailing path segment of a key id such as * Reads the vault id from the config's {@code jti} claim.
* {@code hub+https://hub.example.com/api/vaults/<vaultId>}.
* <p> * <p>
* Requiring a UUID matters beyond well-formedness: the id is interpolated into the {@code api/vaults/{vaultId}/…} * Requiring a UUID matters beyond well-formedness: the id is interpolated into the {@code api/vaults/{vaultId}/…}
* request path, so it must not be able to introduce a path segment. Re-serializing the parsed {@link UUID} rather * request path, so it must not be able to introduce a path segment. A {@code jti} is an arbitrary string, so parsing
* than passing the raw segment on keeps that guarantee. * it as a {@link UUID} and passing that on - rather than the raw claim - is what keeps that guarantee.
* <p>
* Hub writes the same id into the key id's trailing path segment, and the two have always agreed, so a config where
* they differ is forged or broken and is rejected.
*/ */
private static UUID extractVaultId(URI keyId) { private static UUID extractVaultId(VaultConfig.UnverifiedVaultConfig vaultConfig) {
var path = keyId.getPath(); var allegedVaultId = vaultConfig.allegedVaultId();
if (path == null || path.isEmpty()) { if (allegedVaultId == null || allegedVaultId.isBlank()) {
throw new IllegalArgumentException("Vault config's key id contains no vault id, but was '" + keyId + "'."); throw new IllegalArgumentException("Vault config declares no vault id.");
} }
var lastSegment = path.substring(path.lastIndexOf('/') + 1); UUID vaultId;
try { try {
return UUID.fromString(lastSegment); vaultId = UUID.fromString(allegedVaultId);
} catch (IllegalArgumentException e) { } catch (IllegalArgumentException e) {
throw new IllegalArgumentException("Vault config's key id does not end in a vault id, but was '" + keyId + "'.", e); throw new IllegalArgumentException("Vault config's vault id is not a uuid, but was '" + allegedVaultId + "'.", e);
} }
var keyId = vaultConfig.getKeyId();
var path = keyId.getPath();
var lastSegment = path == null ? "" : path.substring(path.lastIndexOf('/') + 1);
if (!vaultId.toString().equalsIgnoreCase(lastSegment)) {
throw new IllegalArgumentException("Vault config's vault id '" + vaultId + "' does not match its key id '" + keyId + "'.");
}
return vaultId;
} }
private static Map<String, String> parseParams(String rawParams) { private static Map<String, String> parseParams(String rawParams) {
@@ -62,9 +62,9 @@ public class Dialogs {
.setOkButtonKey(BUTTON_KEY_CLOSE); .setOkButtonKey(BUTTON_KEY_CLOSE);
} }
public SimpleDialog.Builder prepareHubVaultArchived(Stage window, Vault vault) { public SimpleDialog.Builder prepareHubVaultArchived(Stage window, String vaultDisplayName) {
return createDialogBuilder().setOwner(window) // return createDialogBuilder().setOwner(window) //
.setTitleKey("unlock.title", vault.getDisplayName()) // .setTitleKey("unlock.title", vaultDisplayName) //
.setMessageKey("hub.archived.message") // .setMessageKey("hub.archived.message") //
.setDescriptionKey("hub.archived.description") // .setDescriptionKey("hub.archived.description") //
.setIcon(FontAwesome5Icon.BAN)// .setIcon(FontAwesome5Icon.BAN)//
@@ -16,6 +16,7 @@ import org.cryptomator.ui.common.FxmlLoaderFactory;
import org.cryptomator.ui.common.FxmlScene; import org.cryptomator.ui.common.FxmlScene;
import org.cryptomator.ui.common.StageFactory; import org.cryptomator.ui.common.StageFactory;
import org.cryptomator.ui.keyloading.KeyLoadingComponent; import org.cryptomator.ui.keyloading.KeyLoadingComponent;
import org.cryptomator.ui.keyloading.KeyLoadingRef;
import org.cryptomator.ui.keyloading.KeyLoadingStrategy; import org.cryptomator.ui.keyloading.KeyLoadingStrategy;
import javax.inject.Named; import javax.inject.Named;
@@ -64,7 +65,11 @@ abstract class HealthCheckModule {
@HealthCheckWindow @HealthCheckWindow
@HealthCheckScoped @HealthCheckScoped
static KeyLoadingStrategy provideKeyLoadingStrategy(KeyLoadingComponent.Factory compFactory, @HealthCheckWindow Vault vault, @Named("unlockWindow") Stage window ) { static KeyLoadingStrategy provideKeyLoadingStrategy(KeyLoadingComponent.Factory compFactory, @HealthCheckWindow Vault vault, @Named("unlockWindow") Stage window ) {
return compFactory.create(vault, window).keyloadingStrategy(); try {
return compFactory.create(KeyLoadingRef.forVault(vault), vault, window).keyloadingStrategy();
} catch (IOException e) {
return KeyLoadingStrategy.failed(e);
}
} }
@Provides @Provides
@@ -2,6 +2,7 @@ package org.cryptomator.ui.keyloading;
import dagger.BindsInstance; import dagger.BindsInstance;
import dagger.Subcomponent; import dagger.Subcomponent;
import org.cryptomator.common.Nullable;
import org.cryptomator.common.vaults.Vault; import org.cryptomator.common.vaults.Vault;
import javafx.stage.Stage; import javafx.stage.Stage;
@@ -16,7 +17,14 @@ public interface KeyLoadingComponent {
@Subcomponent.Factory @Subcomponent.Factory
interface Factory { interface Factory {
KeyLoadingComponent create(@BindsInstance @KeyLoading Vault vault, @KeyLoading @BindsInstance Stage window); /**
* @param vaultRef the {@link KeyLoadingRef} containing the info to load the key
* @param vault the local vault, or {@code null} if it is not set up on this machine.
* @param window the window to show the key loading scenes in
*/
KeyLoadingComponent create(@BindsInstance @KeyLoading KeyLoadingRef vaultRef, //
@BindsInstance @KeyLoading @Nullable Vault vault, //
@BindsInstance @KeyLoading Stage window);
} }
} }
@@ -2,7 +2,6 @@ package org.cryptomator.ui.keyloading;
import dagger.Module; import dagger.Module;
import dagger.Provides; import dagger.Provides;
import org.cryptomator.common.vaults.Vault;
import org.cryptomator.ui.common.DefaultSceneFactory; import org.cryptomator.ui.common.DefaultSceneFactory;
import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.common.FxController;
import org.cryptomator.ui.common.FxmlLoaderFactory; import org.cryptomator.ui.common.FxmlLoaderFactory;
@@ -10,7 +9,6 @@ import org.cryptomator.ui.keyloading.hub.HubKeyLoadingModule;
import org.cryptomator.ui.keyloading.masterkeyfile.MasterkeyFileLoadingModule; import org.cryptomator.ui.keyloading.masterkeyfile.MasterkeyFileLoadingModule;
import javax.inject.Provider; import javax.inject.Provider;
import java.io.IOException;
import java.util.Map; import java.util.Map;
import java.util.ResourceBundle; import java.util.ResourceBundle;
@@ -27,14 +25,10 @@ abstract class KeyLoadingModule {
@Provides @Provides
@KeyLoading @KeyLoading
@KeyLoadingScoped @KeyLoadingScoped
static KeyLoadingStrategy provideKeyLoadingStrategy(@KeyLoading Vault vault, Map<String, Provider<KeyLoadingStrategy>> strategies) { static KeyLoadingStrategy provideKeyLoadingStrategy(@KeyLoading KeyLoadingRef vaultRef, Map<String, Provider<KeyLoadingStrategy>> strategies) {
try { String scheme = vaultRef.keyId().getScheme();
String scheme = vault.getVaultConfigCache().get().getKeyId().getScheme(); var fallback = KeyLoadingStrategy.failed(new IllegalArgumentException("Unsupported key id " + scheme));
var fallback = KeyLoadingStrategy.failed(new IllegalArgumentException("Unsupported key id " + scheme)); return strategies.getOrDefault(scheme, () -> fallback).get();
return strategies.getOrDefault(scheme, () -> fallback).get();
} catch (IOException e) {
return KeyLoadingStrategy.failed(e);
}
} }
} }
@@ -0,0 +1,50 @@
package org.cryptomator.ui.keyloading;
import org.cryptomator.common.vaults.Vault;
import org.cryptomator.cryptofs.VaultConfig;
import java.io.IOException;
import java.net.URI;
/**
* Identifies the vault a key is being loaded for, independently of whether that vault exists on this machine.
* <p>
* Key loading needs the (unverified) vault config selecting the strategy and addresses
* the vault within the strategy, and the display name titles the windows.
* <p>
* The config is <em>unverified</em>: its signature is keyed on the masterkey, which is exactly what key loading is
* about to obtain.
*
* @param vaultConfig the vault's unverified config
* @param displayName the vault's name, as shown to the user
*/
public record KeyLoadingRef(VaultConfig.UnverifiedVaultConfig vaultConfig, String displayName) {
/**
* Describes a vault that is already set up on this machine.
*
* @param vault the vault to load a key for
* @throws IOException if the vault's config cannot be read
*/
public static KeyLoadingRef forVault(Vault vault) throws IOException {
return new KeyLoadingRef(vault.getVaultConfigCache().get(), vault.getDisplayName());
}
/**
* The key id, whose scheme selects the key loading strategy.
*/
public URI keyId() {
return vaultConfig.getKeyId();
}
/**
* The vault's id, i.e. how the vault is addressed within its Hub instance.
* <p>
* Read from the config's {@code jti} claim, which is the authoritative source: the key id carries the same id in its
* trailing path segment, but only its <em>scheme</em> is a source of truth here.
*/
public String vaultId() {
return vaultConfig.allegedVaultId();
}
}
@@ -7,7 +7,6 @@ import dagger.Provides;
import dagger.multibindings.IntoMap; import dagger.multibindings.IntoMap;
import dagger.multibindings.StringKey; import dagger.multibindings.StringKey;
import org.cryptomator.common.settings.DeviceKey; import org.cryptomator.common.settings.DeviceKey;
import org.cryptomator.common.vaults.Vault;
import org.cryptomator.cryptolib.common.MessageDigestSupplier; import org.cryptomator.cryptolib.common.MessageDigestSupplier;
import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.common.FxController;
import org.cryptomator.ui.common.FxControllerKey; import org.cryptomator.ui.common.FxControllerKey;
@@ -15,13 +14,12 @@ import org.cryptomator.ui.common.FxmlFile;
import org.cryptomator.ui.common.FxmlLoaderFactory; import org.cryptomator.ui.common.FxmlLoaderFactory;
import org.cryptomator.ui.common.FxmlScene; import org.cryptomator.ui.common.FxmlScene;
import org.cryptomator.ui.keyloading.KeyLoading; import org.cryptomator.ui.keyloading.KeyLoading;
import org.cryptomator.ui.keyloading.KeyLoadingRef;
import org.cryptomator.ui.keyloading.KeyLoadingScoped; import org.cryptomator.ui.keyloading.KeyLoadingScoped;
import org.cryptomator.ui.keyloading.KeyLoadingStrategy; import org.cryptomator.ui.keyloading.KeyLoadingStrategy;
import javax.inject.Named; import javax.inject.Named;
import javafx.scene.Scene; import javafx.scene.Scene;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.util.Objects; import java.util.Objects;
import java.util.ResourceBundle; import java.util.ResourceBundle;
import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletableFuture;
@@ -32,19 +30,15 @@ public abstract class HubKeyLoadingModule {
@Provides @Provides
@KeyLoadingScoped @KeyLoadingScoped
static HubConfig provideHubConfig(@KeyLoading Vault vault) { static HubConfig provideHubConfig(@KeyLoading KeyLoadingRef vaultRef) {
try { return vaultRef.vaultConfig().getHeader("hub", HubConfig.class);
return vault.getVaultConfigCache().get().getHeader("hub", HubConfig.class);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
} }
@Provides @Provides
@KeyLoadingScoped @KeyLoadingScoped
@Named("windowTitle") @Named("windowTitle")
static String provideWindowTitle(@KeyLoading Vault vault, ResourceBundle resourceBundle) { static String provideWindowTitle(@KeyLoading KeyLoadingRef vaultRef, ResourceBundle resourceBundle) {
return String.format(resourceBundle.getString("unlock.title"), vault.getDisplayName()); return String.format(resourceBundle.getString("unlock.title"), vaultRef.displayName());
} }
@@ -7,12 +7,12 @@ import com.google.common.base.Preconditions;
import com.nimbusds.jose.JWEObject; import com.nimbusds.jose.JWEObject;
import dagger.Lazy; import dagger.Lazy;
import org.cryptomator.common.Constants; import org.cryptomator.common.Constants;
import org.cryptomator.common.vaults.Vault;
import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.common.FxController;
import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlFile;
import org.cryptomator.ui.common.FxmlScene; import org.cryptomator.ui.common.FxmlScene;
import org.cryptomator.ui.dialogs.Dialogs; import org.cryptomator.ui.dialogs.Dialogs;
import org.cryptomator.ui.keyloading.KeyLoading; import org.cryptomator.ui.keyloading.KeyLoading;
import org.cryptomator.ui.keyloading.KeyLoadingRef;
import org.cryptomator.ui.keyloading.KeyLoadingScoped; import org.cryptomator.ui.keyloading.KeyLoadingScoped;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
@@ -60,10 +60,10 @@ public class ReceiveKeyController implements FxController {
private final Lazy<Scene> invalidLicenseScene; private final Lazy<Scene> invalidLicenseScene;
private final HttpClient httpClient; private final HttpClient httpClient;
private final Dialogs dialogs; private final Dialogs dialogs;
private final Vault vault; private final KeyLoadingRef vaultRef;
@Inject @Inject
public ReceiveKeyController(@KeyLoading Vault vault, // public ReceiveKeyController(@KeyLoading KeyLoadingRef vaultRef, //
ExecutorService executor, // ExecutorService executor, //
@KeyLoading Stage window, // @KeyLoading Stage window, //
HubConfig hubConfig, // HubConfig hubConfig, //
@@ -79,7 +79,7 @@ public class ReceiveKeyController implements FxController {
Dialogs dialogs) { Dialogs dialogs) {
this.window = window; this.window = window;
this.hubConfig = hubConfig; this.hubConfig = hubConfig;
this.vaultId = extractVaultId(vault.getVaultConfigCache().getUnchecked().getKeyId()); // TODO: access vault config's JTI directly (requires changes in cryptofs) this.vaultId = vaultRef.vaultId();
this.deviceId = deviceId; this.deviceId = deviceId;
this.bearerToken = Objects.requireNonNull(tokenRef.get()); this.bearerToken = Objects.requireNonNull(tokenRef.get());
this.fsOwnerId = fsOwnerId; this.fsOwnerId = fsOwnerId;
@@ -92,7 +92,7 @@ public class ReceiveKeyController implements FxController {
this.window.addEventHandler(WindowEvent.WINDOW_HIDING, this::windowClosed); this.window.addEventHandler(WindowEvent.WINDOW_HIDING, this::windowClosed);
this.httpClient = HttpClient.newBuilder().version(HttpClient.Version.HTTP_1_1).executor(executor).build(); this.httpClient = HttpClient.newBuilder().version(HttpClient.Version.HTTP_1_1).executor(executor).build();
this.dialogs = dialogs; this.dialogs = dialogs;
this.vault = vault; this.vaultRef = vaultRef;
} }
@FXML @FXML
@@ -313,7 +313,7 @@ public class ReceiveKeyController implements FxController {
private void accessGoneVaultArchived() { private void accessGoneVaultArchived() {
window.close(); window.close();
dialogs.prepareHubVaultArchived((Stage)window.getOwner(), vault).build().showAndWait(); dialogs.prepareHubVaultArchived((Stage)window.getOwner(), vaultRef.displayName()).build().showAndWait();
} }
private void accountInitializationRequired() { private void accountInitializationRequired() {
@@ -343,12 +343,6 @@ public class ReceiveKeyController implements FxController {
} }
} }
private static String extractVaultId(URI vaultKeyUri) {
assert vaultKeyUri.getScheme().startsWith(HubKeyLoadingStrategy.SCHEME_PREFIX);
var path = vaultKeyUri.getPath();
return path.substring(path.lastIndexOf('/') + 1);
}
@JsonIgnoreProperties(ignoreUnknown = true) @JsonIgnoreProperties(ignoreUnknown = true)
private record UserDto(@JsonProperty(value = "name", required = true) String name) {} private record UserDto(@JsonProperty(value = "name", required = true) String name) {}
@@ -1,5 +1,6 @@
package org.cryptomator.ui.keyloading.masterkeyfile; package org.cryptomator.ui.keyloading.masterkeyfile;
import org.cryptomator.common.Nullable;
import org.cryptomator.common.recovery.RecoveryActionType; import org.cryptomator.common.recovery.RecoveryActionType;
import org.cryptomator.common.vaults.Vault; import org.cryptomator.common.vaults.Vault;
import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.common.FxController;
@@ -18,6 +19,7 @@ import javafx.stage.Stage;
import javafx.stage.WindowEvent; import javafx.stage.WindowEvent;
import java.io.File; import java.io.File;
import java.nio.file.Path; import java.nio.file.Path;
import java.util.Objects;
import java.util.ResourceBundle; import java.util.ResourceBundle;
import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletableFuture;
@@ -41,12 +43,12 @@ public class ChooseMasterkeyFileController implements FxController {
@Inject @Inject
public ChooseMasterkeyFileController(@KeyLoading Stage window, // public ChooseMasterkeyFileController(@KeyLoading Stage window, //
@KeyLoading Vault vault, // @KeyLoading @Nullable Vault vault, //
CompletableFuture<Path> result, // CompletableFuture<Path> result, //
RecoveryKeyComponent.Factory recoveryKeyWindow, // RecoveryKeyComponent.Factory recoveryKeyWindow, //
ResourceBundle resourceBundle) { ResourceBundle resourceBundle) {
this.window = window; this.window = window;
this.vault = vault; this.vault = Objects.requireNonNull(vault, MasterkeyFileLoadingModule.NO_LOCAL_VAULT);
this.result = result; this.result = result;
this.recoveryKeyWindow = recoveryKeyWindow; this.recoveryKeyWindow = recoveryKeyWindow;
this.resourceBundle = resourceBundle; this.resourceBundle = resourceBundle;
@@ -5,6 +5,7 @@ import dagger.Module;
import dagger.Provides; import dagger.Provides;
import dagger.multibindings.IntoMap; import dagger.multibindings.IntoMap;
import dagger.multibindings.StringKey; import dagger.multibindings.StringKey;
import org.cryptomator.common.Nullable;
import org.cryptomator.common.keychain.KeychainManager; import org.cryptomator.common.keychain.KeychainManager;
import org.cryptomator.common.vaults.Vault; import org.cryptomator.common.vaults.Vault;
import org.cryptomator.integrations.keychain.KeychainAccessException; import org.cryptomator.integrations.keychain.KeychainAccessException;
@@ -15,20 +16,27 @@ import org.cryptomator.ui.keyloading.KeyLoadingStrategy;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import javax.inject.Named; import javax.inject.Named;
import java.util.Objects;
import java.util.Optional; import java.util.Optional;
@Module(subcomponents = {ForgetPasswordComponent.class, PassphraseEntryComponent.class, ChooseMasterkeyFileComponent.class}) @Module(subcomponents = {ForgetPasswordComponent.class, PassphraseEntryComponent.class, ChooseMasterkeyFileComponent.class})
public interface MasterkeyFileLoadingModule { public interface MasterkeyFileLoadingModule {
/**
* Key loading may run for a yet-to-setup vault (i.e. deeplink with only a config) - Masterkey loading requires in the current implementation
* an already setup vault.
*/
String NO_LOCAL_VAULT = "masterkey file loading requires a local vault";
@Provides @Provides
@Named("savedPassword") @Named("savedPassword")
@KeyLoadingScoped @KeyLoadingScoped
static Optional<char[]> provideStoredPassword(KeychainManager keychain, @KeyLoading Vault vault) { static Optional<char[]> provideStoredPassword(KeychainManager keychain, @KeyLoading @Nullable Vault vault) {
if (!keychain.isSupported() || keychain.isLocked()) { if (!keychain.isSupported() || keychain.isLocked()) {
return Optional.empty(); return Optional.empty();
} else { } else {
try { try {
return Optional.ofNullable(keychain.loadPassphrase(vault.getId())); return Optional.ofNullable(keychain.loadPassphrase(Objects.requireNonNull(vault, NO_LOCAL_VAULT).getId()));
} catch (KeychainAccessException e) { } catch (KeychainAccessException e) {
LoggerFactory.getLogger(MasterkeyFileLoadingModule.class).error("Failed to load entry from system keychain.", e); LoggerFactory.getLogger(MasterkeyFileLoadingModule.class).error("Failed to load entry from system keychain.", e);
return Optional.empty(); return Optional.empty();
@@ -2,6 +2,7 @@ package org.cryptomator.ui.keyloading.masterkeyfile;
import com.google.common.base.Preconditions; import com.google.common.base.Preconditions;
import org.cryptomator.common.Constants; import org.cryptomator.common.Constants;
import org.cryptomator.common.Nullable;
import org.cryptomator.common.Passphrase; import org.cryptomator.common.Passphrase;
import org.cryptomator.common.keychain.KeychainManager; import org.cryptomator.common.keychain.KeychainManager;
import org.cryptomator.common.vaults.Vault; import org.cryptomator.common.vaults.Vault;
@@ -25,6 +26,7 @@ import java.io.IOException;
import java.net.URI; import java.net.URI;
import java.nio.file.Files; import java.nio.file.Files;
import java.nio.file.Path; import java.nio.file.Path;
import java.util.Objects;
import java.util.Optional; import java.util.Optional;
import java.util.ResourceBundle; import java.util.ResourceBundle;
import java.util.concurrent.CancellationException; import java.util.concurrent.CancellationException;
@@ -48,8 +50,8 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy {
private boolean wrongPassphrase; private boolean wrongPassphrase;
@Inject @Inject
public MasterkeyFileLoadingStrategy(@KeyLoading Vault vault, MasterkeyFileAccess masterkeyFileAccess, @KeyLoading Stage window, @Named("savedPassword") Optional<char[]> savedPassphrase, PassphraseEntryComponent.Builder passphraseEntry, ChooseMasterkeyFileComponent.Builder masterkeyFileChoice, KeychainManager keychain, ResourceBundle resourceBundle) { public MasterkeyFileLoadingStrategy(@KeyLoading @Nullable Vault vault, MasterkeyFileAccess masterkeyFileAccess, @KeyLoading Stage window, @Named("savedPassword") Optional<char[]> savedPassphrase, PassphraseEntryComponent.Builder passphraseEntry, ChooseMasterkeyFileComponent.Builder masterkeyFileChoice, KeychainManager keychain, ResourceBundle resourceBundle) {
this.vault = vault; this.vault = Objects.requireNonNull(vault, MasterkeyFileLoadingModule.NO_LOCAL_VAULT);
this.masterkeyFileAccess = masterkeyFileAccess; this.masterkeyFileAccess = masterkeyFileAccess;
this.window = window; this.window = window;
this.passphraseEntry = passphraseEntry; this.passphraseEntry = passphraseEntry;
@@ -35,6 +35,7 @@ import javafx.scene.transform.Translate;
import javafx.stage.Stage; import javafx.stage.Stage;
import javafx.stage.WindowEvent; import javafx.stage.WindowEvent;
import javafx.util.Duration; import javafx.util.Duration;
import java.util.Objects;
import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutorService; import java.util.concurrent.ExecutorService;
@@ -66,9 +67,9 @@ public class PassphraseEntryController implements FxController {
public Animation unlockAnimation; public Animation unlockAnimation;
@Inject @Inject
public PassphraseEntryController(@KeyLoading Stage window, @KeyLoading Vault vault, CompletableFuture<PassphraseEntryResult> result, @Nullable @Named("savedPassword") Passphrase savedPassword, ForgetPasswordComponent.Builder forgetPassword, KeychainManager keychain, ExecutorService backgroundExecutorService) { public PassphraseEntryController(@KeyLoading Stage window, @KeyLoading @Nullable Vault vault, CompletableFuture<PassphraseEntryResult> result, @Nullable @Named("savedPassword") Passphrase savedPassword, ForgetPasswordComponent.Builder forgetPassword, KeychainManager keychain, ExecutorService backgroundExecutorService) {
this.window = window; this.window = window;
this.vault = vault; this.vault = Objects.requireNonNull(vault, MasterkeyFileLoadingModule.NO_LOCAL_VAULT);
this.result = result; this.result = result;
this.savedPassword = savedPassword; this.savedPassword = savedPassword;
this.forgetPassword = forgetPassword; this.forgetPassword = forgetPassword;
@@ -14,6 +14,7 @@ import org.cryptomator.ui.common.FxmlLoaderFactory;
import org.cryptomator.ui.common.FxmlScene; import org.cryptomator.ui.common.FxmlScene;
import org.cryptomator.ui.common.StageFactory; import org.cryptomator.ui.common.StageFactory;
import org.cryptomator.ui.keyloading.KeyLoadingComponent; import org.cryptomator.ui.keyloading.KeyLoadingComponent;
import org.cryptomator.ui.keyloading.KeyLoadingRef;
import org.cryptomator.ui.keyloading.KeyLoadingStrategy; import org.cryptomator.ui.keyloading.KeyLoadingStrategy;
import org.cryptomator.ui.recoverykey.RecoveryKeyComponent; import org.cryptomator.ui.recoverykey.RecoveryKeyComponent;
import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.Nullable;
@@ -25,6 +26,7 @@ import javafx.beans.property.SimpleObjectProperty;
import javafx.scene.Scene; import javafx.scene.Scene;
import javafx.stage.Modality; import javafx.stage.Modality;
import javafx.stage.Stage; import javafx.stage.Stage;
import java.io.IOException;
import java.util.Map; import java.util.Map;
import java.util.ResourceBundle; import java.util.ResourceBundle;
@@ -58,7 +60,11 @@ abstract class UnlockModule {
@UnlockWindow @UnlockWindow
@UnlockScoped @UnlockScoped
static KeyLoadingStrategy provideKeyLoadingStrategy(KeyLoadingComponent.Factory compFactory, @UnlockWindow Vault vault, @UnlockWindow Stage window) { static KeyLoadingStrategy provideKeyLoadingStrategy(KeyLoadingComponent.Factory compFactory, @UnlockWindow Vault vault, @UnlockWindow Stage window) {
return compFactory.create(vault, window).keyloadingStrategy(); try {
return compFactory.create(KeyLoadingRef.forVault(vault), vault, window).keyloadingStrategy();
} catch (IOException e) {
return KeyLoadingStrategy.failed(e);
}
} }
@Provides @Provides
@@ -73,21 +73,32 @@ public class OpenHubVaultEventTest {
@Test @Test
@DisplayName("a config without a hub header is rejected") @DisplayName("a config without a hub header is rejected")
public void testNoHubHeader() { public void testNoHubHeader() {
var token = vaultConfig(KEY_ID, null); var token = vaultConfig(KEY_ID, VAULT_ID, null);
Assertions.assertThrows(IllegalArgumentException.class, // Assertions.assertThrows(IllegalArgumentException.class, //
() -> OpenHubVaultEvent.tryParse(URI.create("org.cryptomator://vault/open#vaultConfig=" + token))); () -> OpenHubVaultEvent.tryParse(URI.create("org.cryptomator://vault/open#vaultConfig=" + token)));
} }
@ParameterizedTest @ParameterizedTest
@DisplayName("a key id whose last segment is not a vault id is rejected") @DisplayName("a vault id that is not a uuid is rejected")
@ValueSource(strings = { // @ValueSource(strings = { //
"hub+https://hub.example.com/api/vaults/not-a-uuid", // "not-a-uuid", //
"hub+https://hub.example.com/api/vaults/", // empty segment "", // absent
"hub+https://hub.example.com" // no path at all "../../evil" // must never reach the api/vaults/{vaultId}/... request path
}) })
public void testInvalidVaultId(String keyId) { public void testInvalidVaultId(String vaultId) {
var token = hubVaultConfig(keyId); var token = vaultConfig(KEY_ID, vaultId, hubHeader("https://hub.example.com/api", "https://login.example.com/auth"));
Assertions.assertThrows(IllegalArgumentException.class, //
() -> OpenHubVaultEvent.tryParse(URI.create("org.cryptomator://vault/open#vaultConfig=" + token)));
}
@Test
@DisplayName("a vault id disagreeing with the key id is rejected")
public void testVaultIdMismatch() {
// Hub writes the same id into both, so a config where they differ is forged or broken
var token = vaultConfig(KEY_ID, "11111111-2222-3333-4444-555555555555", //
hubHeader("https://hub.example.com/api", "https://login.example.com/auth"));
Assertions.assertThrows(IllegalArgumentException.class, // Assertions.assertThrows(IllegalArgumentException.class, //
() -> OpenHubVaultEvent.tryParse(URI.create("org.cryptomator://vault/open#vaultConfig=" + token))); () -> OpenHubVaultEvent.tryParse(URI.create("org.cryptomator://vault/open#vaultConfig=" + token)));
@@ -101,7 +112,7 @@ public class OpenHubVaultEventTest {
"https:///api" // no host "https:///api" // no host
}) })
public void testUnusableApiBaseUrl(String apiBaseUrl) { public void testUnusableApiBaseUrl(String apiBaseUrl) {
var token = vaultConfig(KEY_ID, hubHeader(apiBaseUrl, "https://login.example.com/auth")); var token = vaultConfig(KEY_ID, VAULT_ID, hubHeader(apiBaseUrl, "https://login.example.com/auth"));
Assertions.assertThrows(IllegalArgumentException.class, // Assertions.assertThrows(IllegalArgumentException.class, //
() -> OpenHubVaultEvent.tryParse(URI.create("org.cryptomator://vault/open#vaultConfig=" + token))); () -> OpenHubVaultEvent.tryParse(URI.create("org.cryptomator://vault/open#vaultConfig=" + token)));
@@ -110,7 +121,7 @@ public class OpenHubVaultEventTest {
@Test @Test
@DisplayName("an unusable auth endpoint is rejected") @DisplayName("an unusable auth endpoint is rejected")
public void testUnusableAuthEndpoint() { public void testUnusableAuthEndpoint() {
var token = vaultConfig(KEY_ID, hubHeader("https://hub.example.com/api", "not a url")); var token = vaultConfig(KEY_ID, VAULT_ID, hubHeader("https://hub.example.com/api", "not a url"));
Assertions.assertThrows(IllegalArgumentException.class, // Assertions.assertThrows(IllegalArgumentException.class, //
() -> OpenHubVaultEvent.tryParse(URI.create("org.cryptomator://vault/open#vaultConfig=" + token))); () -> OpenHubVaultEvent.tryParse(URI.create("org.cryptomator://vault/open#vaultConfig=" + token)));
@@ -119,7 +130,7 @@ public class OpenHubVaultEventTest {
@Test @Test
@DisplayName("an http endpoint is accepted here, host trust decides later") @DisplayName("an http endpoint is accepted here, host trust decides later")
public void testHttpEndpointAccepted() { public void testHttpEndpointAccepted() {
var token = vaultConfig(KEY_ID, hubHeader("http://localhost:8080/api", "http://localhost:8080/auth")); var token = vaultConfig(KEY_ID, VAULT_ID, hubHeader("http://localhost:8080/api", "http://localhost:8080/auth"));
Assertions.assertTrue(OpenHubVaultEvent.tryParse(URI.create("org.cryptomator://vault/open#vaultConfig=" + token)).isPresent()); Assertions.assertTrue(OpenHubVaultEvent.tryParse(URI.create("org.cryptomator://vault/open#vaultConfig=" + token)).isPresent());
} }
@@ -180,14 +191,14 @@ public class OpenHubVaultEventTest {
} }
private static String hubVaultConfig(String keyId) { private static String hubVaultConfig(String keyId) {
return vaultConfig(keyId, hubHeader("https://hub.example.com/api", "https://login.example.com/auth")); return vaultConfig(keyId, VAULT_ID, hubHeader("https://hub.example.com/api", "https://login.example.com/auth"));
} }
/** /**
* Builds a vault config token. Its signature is keyed on the masterkey, which the deeplink never carries, so a dummy * Builds a vault config token. Its signature is keyed on the masterkey, which the deeplink never carries, so a dummy
* signature is exactly what the parser operates on. * signature is exactly what the parser operates on.
*/ */
private static String vaultConfig(String keyId, String extraHeaderFields) { private static String vaultConfig(String keyId, String vaultId, String extraHeaderFields) {
var header = """ var header = """
{ "kid":"%s",\ { "kid":"%s",\
"typ":"JWT",\ "typ":"JWT",\
@@ -195,10 +206,11 @@ public class OpenHubVaultEventTest {
%s %s
}""".formatted(keyId, extraHeaderFields == null ? "" : extraHeaderFields); }""".formatted(keyId, extraHeaderFields == null ? "" : extraHeaderFields);
var payload = """ var payload = """
{ "format":8,\ { "jti":"%s",\
"format":8,\
"cipherCombo":"SIV_GCM",\ "cipherCombo":"SIV_GCM",\
"shorteningThreshold":220\ "shorteningThreshold":220\
}"""; }""".formatted(vaultId);
var encoder = Base64.getUrlEncoder().withoutPadding(); var encoder = Base64.getUrlEncoder().withoutPadding();
return encoder.encodeToString(header.getBytes(StandardCharsets.UTF_8)) // return encoder.encodeToString(header.getBytes(StandardCharsets.UTF_8)) //
+ "." + encoder.encodeToString(payload.getBytes(StandardCharsets.UTF_8)) // + "." + encoder.encodeToString(payload.getBytes(StandardCharsets.UTF_8)) //