diff --git a/pom.xml b/pom.xml index 4e66590f4..cd49412fc 100644 --- a/pom.xml +++ b/pom.xml @@ -33,7 +33,7 @@ org.ow2.asm,org.apache.jackrabbit,org.apache.httpcomponents - 2.10.0 + 2.11.0-SNAPSHOT 2.2.2 1.9.0 1.6.1 diff --git a/src/main/java/org/cryptomator/launcher/OpenHubVaultEvent.java b/src/main/java/org/cryptomator/launcher/OpenHubVaultEvent.java index f33580d9c..29c60e5ea 100644 --- a/src/main/java/org/cryptomator/launcher/OpenHubVaultEvent.java +++ b/src/main/java/org/cryptomator/launcher/OpenHubVaultEvent.java @@ -18,15 +18,16 @@ import java.util.UUID; /** * Requests opening a Hub vault from an {@code org.cryptomator://vault/open#vaultConfig=…} deeplink. *

- * The single parameter is the vault's {@code vault.cryptomator}, a compact JWS embedded verbatim - a compact JWS - * 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. + * The single parameter is the vault's {@code vault.cryptomator}, a compact JWS embedded verbatim, carried in the fragment part. *

- * The config is read unverified: its signature is keyed on the masterkey, which is only obtainable from Hub - * later on. Everything consumed before that point is therefore validated here rather than trusted. + * Notes: + *

* * @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 { @@ -59,7 +60,7 @@ public record OpenHubVaultEvent(VaultConfig.UnverifiedVaultConfig vaultConfig, U } var vaultConfig = decode(token); requireHubVault(vaultConfig); - var vaultId = extractVaultId(vaultConfig.getKeyId()); + var vaultId = extractVaultId(vaultConfig); var leftoverParams = params.keySet().stream().filter(k -> !k.equals(PARAM_VAULT_CONFIG)).toList(); if (!leftoverParams.isEmpty()) { @@ -123,8 +124,8 @@ public record OpenHubVaultEvent(VaultConfig.UnverifiedVaultConfig vaultConfig, U 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 + "'."); } - // Whether an http host is acceptable (it is, for local development) is decided by CheckHostTrustController, the - // authority on host trust. Here we only ensure the endpoint is shaped like something that decision can be made on. + // Whether an http host is acceptable (it is, for local development) is decided by CheckHostTrustController + // Here we only ensure the endpoint is shaped like something that decision can be made on. var scheme = uri.getScheme(); if (!"https".equalsIgnoreCase(scheme) && !"http".equalsIgnoreCase(scheme)) { 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 - * {@code hub+https://hub.example.com/api/vaults/}. + * Reads the vault id from the config's {@code jti} claim. *

* 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 - * than passing the raw segment on keeps that guarantee. + * request path, so it must not be able to introduce a path segment. A {@code jti} is an arbitrary string, so parsing + * it as a {@link UUID} and passing that on - rather than the raw claim - is what keeps that guarantee. + *

+ * 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) { - var path = keyId.getPath(); - if (path == null || path.isEmpty()) { - throw new IllegalArgumentException("Vault config's key id contains no vault id, but was '" + keyId + "'."); + private static UUID extractVaultId(VaultConfig.UnverifiedVaultConfig vaultConfig) { + var allegedVaultId = vaultConfig.allegedVaultId(); + if (allegedVaultId == null || allegedVaultId.isBlank()) { + throw new IllegalArgumentException("Vault config declares no vault id."); } - var lastSegment = path.substring(path.lastIndexOf('/') + 1); + UUID vaultId; try { - return UUID.fromString(lastSegment); + vaultId = UUID.fromString(allegedVaultId); } 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 parseParams(String rawParams) { diff --git a/src/main/java/org/cryptomator/ui/dialogs/Dialogs.java b/src/main/java/org/cryptomator/ui/dialogs/Dialogs.java index 4933cefc7..33116fba2 100644 --- a/src/main/java/org/cryptomator/ui/dialogs/Dialogs.java +++ b/src/main/java/org/cryptomator/ui/dialogs/Dialogs.java @@ -62,9 +62,9 @@ public class Dialogs { .setOkButtonKey(BUTTON_KEY_CLOSE); } - public SimpleDialog.Builder prepareHubVaultArchived(Stage window, Vault vault) { + public SimpleDialog.Builder prepareHubVaultArchived(Stage window, String vaultDisplayName) { return createDialogBuilder().setOwner(window) // - .setTitleKey("unlock.title", vault.getDisplayName()) // + .setTitleKey("unlock.title", vaultDisplayName) // .setMessageKey("hub.archived.message") // .setDescriptionKey("hub.archived.description") // .setIcon(FontAwesome5Icon.BAN)// diff --git a/src/main/java/org/cryptomator/ui/health/HealthCheckModule.java b/src/main/java/org/cryptomator/ui/health/HealthCheckModule.java index 19595e0e9..c7d5740ed 100644 --- a/src/main/java/org/cryptomator/ui/health/HealthCheckModule.java +++ b/src/main/java/org/cryptomator/ui/health/HealthCheckModule.java @@ -16,6 +16,7 @@ import org.cryptomator.ui.common.FxmlLoaderFactory; import org.cryptomator.ui.common.FxmlScene; import org.cryptomator.ui.common.StageFactory; import org.cryptomator.ui.keyloading.KeyLoadingComponent; +import org.cryptomator.ui.keyloading.KeyLoadingRef; import org.cryptomator.ui.keyloading.KeyLoadingStrategy; import javax.inject.Named; @@ -64,7 +65,11 @@ abstract class HealthCheckModule { @HealthCheckWindow @HealthCheckScoped 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 diff --git a/src/main/java/org/cryptomator/ui/keyloading/KeyLoadingComponent.java b/src/main/java/org/cryptomator/ui/keyloading/KeyLoadingComponent.java index f035388f6..5c5657964 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/KeyLoadingComponent.java +++ b/src/main/java/org/cryptomator/ui/keyloading/KeyLoadingComponent.java @@ -2,6 +2,7 @@ package org.cryptomator.ui.keyloading; import dagger.BindsInstance; import dagger.Subcomponent; +import org.cryptomator.common.Nullable; import org.cryptomator.common.vaults.Vault; import javafx.stage.Stage; @@ -16,7 +17,14 @@ public interface KeyLoadingComponent { @Subcomponent.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); } } diff --git a/src/main/java/org/cryptomator/ui/keyloading/KeyLoadingModule.java b/src/main/java/org/cryptomator/ui/keyloading/KeyLoadingModule.java index 5dbb4adbd..e778bf7aa 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/KeyLoadingModule.java +++ b/src/main/java/org/cryptomator/ui/keyloading/KeyLoadingModule.java @@ -2,7 +2,6 @@ package org.cryptomator.ui.keyloading; import dagger.Module; import dagger.Provides; -import org.cryptomator.common.vaults.Vault; import org.cryptomator.ui.common.DefaultSceneFactory; import org.cryptomator.ui.common.FxController; 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 javax.inject.Provider; -import java.io.IOException; import java.util.Map; import java.util.ResourceBundle; @@ -27,14 +25,10 @@ abstract class KeyLoadingModule { @Provides @KeyLoading @KeyLoadingScoped - static KeyLoadingStrategy provideKeyLoadingStrategy(@KeyLoading Vault vault, Map> strategies) { - try { - String scheme = vault.getVaultConfigCache().get().getKeyId().getScheme(); - var fallback = KeyLoadingStrategy.failed(new IllegalArgumentException("Unsupported key id " + scheme)); - return strategies.getOrDefault(scheme, () -> fallback).get(); - } catch (IOException e) { - return KeyLoadingStrategy.failed(e); - } + static KeyLoadingStrategy provideKeyLoadingStrategy(@KeyLoading KeyLoadingRef vaultRef, Map> strategies) { + String scheme = vaultRef.keyId().getScheme(); + var fallback = KeyLoadingStrategy.failed(new IllegalArgumentException("Unsupported key id " + scheme)); + return strategies.getOrDefault(scheme, () -> fallback).get(); } } diff --git a/src/main/java/org/cryptomator/ui/keyloading/KeyLoadingRef.java b/src/main/java/org/cryptomator/ui/keyloading/KeyLoadingRef.java new file mode 100644 index 000000000..781c63ffb --- /dev/null +++ b/src/main/java/org/cryptomator/ui/keyloading/KeyLoadingRef.java @@ -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. + *

+ * Key loading needs the (unverified) vault config selecting the strategy and addresses + * the vault within the strategy, and the display name titles the windows. + *

+ * The config is unverified: 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. + *

+ * 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 scheme is a source of truth here. + */ + public String vaultId() { + return vaultConfig.allegedVaultId(); + } + +} diff --git a/src/main/java/org/cryptomator/ui/keyloading/hub/HubKeyLoadingModule.java b/src/main/java/org/cryptomator/ui/keyloading/hub/HubKeyLoadingModule.java index 86a9cb800..056fe0065 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/hub/HubKeyLoadingModule.java +++ b/src/main/java/org/cryptomator/ui/keyloading/hub/HubKeyLoadingModule.java @@ -7,7 +7,6 @@ import dagger.Provides; import dagger.multibindings.IntoMap; import dagger.multibindings.StringKey; import org.cryptomator.common.settings.DeviceKey; -import org.cryptomator.common.vaults.Vault; import org.cryptomator.cryptolib.common.MessageDigestSupplier; import org.cryptomator.ui.common.FxController; 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.FxmlScene; import org.cryptomator.ui.keyloading.KeyLoading; +import org.cryptomator.ui.keyloading.KeyLoadingRef; import org.cryptomator.ui.keyloading.KeyLoadingScoped; import org.cryptomator.ui.keyloading.KeyLoadingStrategy; import javax.inject.Named; import javafx.scene.Scene; -import java.io.IOException; -import java.io.UncheckedIOException; import java.util.Objects; import java.util.ResourceBundle; import java.util.concurrent.CompletableFuture; @@ -32,19 +30,15 @@ public abstract class HubKeyLoadingModule { @Provides @KeyLoadingScoped - static HubConfig provideHubConfig(@KeyLoading Vault vault) { - try { - return vault.getVaultConfigCache().get().getHeader("hub", HubConfig.class); - } catch (IOException e) { - throw new UncheckedIOException(e); - } + static HubConfig provideHubConfig(@KeyLoading KeyLoadingRef vaultRef) { + return vaultRef.vaultConfig().getHeader("hub", HubConfig.class); } @Provides @KeyLoadingScoped @Named("windowTitle") - static String provideWindowTitle(@KeyLoading Vault vault, ResourceBundle resourceBundle) { - return String.format(resourceBundle.getString("unlock.title"), vault.getDisplayName()); + static String provideWindowTitle(@KeyLoading KeyLoadingRef vaultRef, ResourceBundle resourceBundle) { + return String.format(resourceBundle.getString("unlock.title"), vaultRef.displayName()); } diff --git a/src/main/java/org/cryptomator/ui/keyloading/hub/ReceiveKeyController.java b/src/main/java/org/cryptomator/ui/keyloading/hub/ReceiveKeyController.java index db041037d..0bca71e90 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/hub/ReceiveKeyController.java +++ b/src/main/java/org/cryptomator/ui/keyloading/hub/ReceiveKeyController.java @@ -7,12 +7,12 @@ import com.google.common.base.Preconditions; import com.nimbusds.jose.JWEObject; import dagger.Lazy; import org.cryptomator.common.Constants; -import org.cryptomator.common.vaults.Vault; import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; import org.cryptomator.ui.dialogs.Dialogs; import org.cryptomator.ui.keyloading.KeyLoading; +import org.cryptomator.ui.keyloading.KeyLoadingRef; import org.cryptomator.ui.keyloading.KeyLoadingScoped; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -60,10 +60,10 @@ public class ReceiveKeyController implements FxController { private final Lazy invalidLicenseScene; private final HttpClient httpClient; private final Dialogs dialogs; - private final Vault vault; + private final KeyLoadingRef vaultRef; @Inject - public ReceiveKeyController(@KeyLoading Vault vault, // + public ReceiveKeyController(@KeyLoading KeyLoadingRef vaultRef, // ExecutorService executor, // @KeyLoading Stage window, // HubConfig hubConfig, // @@ -79,7 +79,7 @@ public class ReceiveKeyController implements FxController { Dialogs dialogs) { this.window = window; 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.bearerToken = Objects.requireNonNull(tokenRef.get()); this.fsOwnerId = fsOwnerId; @@ -92,7 +92,7 @@ public class ReceiveKeyController implements FxController { this.window.addEventHandler(WindowEvent.WINDOW_HIDING, this::windowClosed); this.httpClient = HttpClient.newBuilder().version(HttpClient.Version.HTTP_1_1).executor(executor).build(); this.dialogs = dialogs; - this.vault = vault; + this.vaultRef = vaultRef; } @FXML @@ -313,7 +313,7 @@ public class ReceiveKeyController implements FxController { private void accessGoneVaultArchived() { window.close(); - dialogs.prepareHubVaultArchived((Stage)window.getOwner(), vault).build().showAndWait(); + dialogs.prepareHubVaultArchived((Stage)window.getOwner(), vaultRef.displayName()).build().showAndWait(); } 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) private record UserDto(@JsonProperty(value = "name", required = true) String name) {} diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileController.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileController.java index 4e1c663e9..b7ff167df 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileController.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileController.java @@ -1,5 +1,6 @@ package org.cryptomator.ui.keyloading.masterkeyfile; +import org.cryptomator.common.Nullable; import org.cryptomator.common.recovery.RecoveryActionType; import org.cryptomator.common.vaults.Vault; import org.cryptomator.ui.common.FxController; @@ -18,6 +19,7 @@ import javafx.stage.Stage; import javafx.stage.WindowEvent; import java.io.File; import java.nio.file.Path; +import java.util.Objects; import java.util.ResourceBundle; import java.util.concurrent.CompletableFuture; @@ -41,12 +43,12 @@ public class ChooseMasterkeyFileController implements FxController { @Inject public ChooseMasterkeyFileController(@KeyLoading Stage window, // - @KeyLoading Vault vault, // + @KeyLoading @Nullable Vault vault, // CompletableFuture result, // RecoveryKeyComponent.Factory recoveryKeyWindow, // ResourceBundle resourceBundle) { this.window = window; - this.vault = vault; + this.vault = Objects.requireNonNull(vault, MasterkeyFileLoadingModule.NO_LOCAL_VAULT); this.result = result; this.recoveryKeyWindow = recoveryKeyWindow; this.resourceBundle = resourceBundle; diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingModule.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingModule.java index 5e4d148cb..0d13d4209 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingModule.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingModule.java @@ -5,6 +5,7 @@ import dagger.Module; import dagger.Provides; import dagger.multibindings.IntoMap; import dagger.multibindings.StringKey; +import org.cryptomator.common.Nullable; import org.cryptomator.common.keychain.KeychainManager; import org.cryptomator.common.vaults.Vault; import org.cryptomator.integrations.keychain.KeychainAccessException; @@ -15,20 +16,27 @@ import org.cryptomator.ui.keyloading.KeyLoadingStrategy; import org.slf4j.LoggerFactory; import javax.inject.Named; +import java.util.Objects; import java.util.Optional; @Module(subcomponents = {ForgetPasswordComponent.class, PassphraseEntryComponent.class, ChooseMasterkeyFileComponent.class}) 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 @Named("savedPassword") @KeyLoadingScoped - static Optional provideStoredPassword(KeychainManager keychain, @KeyLoading Vault vault) { + static Optional provideStoredPassword(KeychainManager keychain, @KeyLoading @Nullable Vault vault) { if (!keychain.isSupported() || keychain.isLocked()) { return Optional.empty(); } else { try { - return Optional.ofNullable(keychain.loadPassphrase(vault.getId())); + return Optional.ofNullable(keychain.loadPassphrase(Objects.requireNonNull(vault, NO_LOCAL_VAULT).getId())); } catch (KeychainAccessException e) { LoggerFactory.getLogger(MasterkeyFileLoadingModule.class).error("Failed to load entry from system keychain.", e); return Optional.empty(); diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java index 16c7d6256..de58e8c08 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java @@ -2,6 +2,7 @@ package org.cryptomator.ui.keyloading.masterkeyfile; import com.google.common.base.Preconditions; import org.cryptomator.common.Constants; +import org.cryptomator.common.Nullable; import org.cryptomator.common.Passphrase; import org.cryptomator.common.keychain.KeychainManager; import org.cryptomator.common.vaults.Vault; @@ -25,6 +26,7 @@ import java.io.IOException; import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Objects; import java.util.Optional; import java.util.ResourceBundle; import java.util.concurrent.CancellationException; @@ -48,8 +50,8 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { private boolean wrongPassphrase; @Inject - public MasterkeyFileLoadingStrategy(@KeyLoading Vault vault, MasterkeyFileAccess masterkeyFileAccess, @KeyLoading Stage window, @Named("savedPassword") Optional savedPassphrase, PassphraseEntryComponent.Builder passphraseEntry, ChooseMasterkeyFileComponent.Builder masterkeyFileChoice, KeychainManager keychain, ResourceBundle resourceBundle) { - this.vault = vault; + public MasterkeyFileLoadingStrategy(@KeyLoading @Nullable Vault vault, MasterkeyFileAccess masterkeyFileAccess, @KeyLoading Stage window, @Named("savedPassword") Optional savedPassphrase, PassphraseEntryComponent.Builder passphraseEntry, ChooseMasterkeyFileComponent.Builder masterkeyFileChoice, KeychainManager keychain, ResourceBundle resourceBundle) { + this.vault = Objects.requireNonNull(vault, MasterkeyFileLoadingModule.NO_LOCAL_VAULT); this.masterkeyFileAccess = masterkeyFileAccess; this.window = window; this.passphraseEntry = passphraseEntry; diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryController.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryController.java index 37b2f2e1f..705e61420 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryController.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryController.java @@ -35,6 +35,7 @@ import javafx.scene.transform.Translate; import javafx.stage.Stage; import javafx.stage.WindowEvent; import javafx.util.Duration; +import java.util.Objects; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; @@ -66,9 +67,9 @@ public class PassphraseEntryController implements FxController { public Animation unlockAnimation; @Inject - public PassphraseEntryController(@KeyLoading Stage window, @KeyLoading Vault vault, CompletableFuture result, @Nullable @Named("savedPassword") Passphrase savedPassword, ForgetPasswordComponent.Builder forgetPassword, KeychainManager keychain, ExecutorService backgroundExecutorService) { + public PassphraseEntryController(@KeyLoading Stage window, @KeyLoading @Nullable Vault vault, CompletableFuture result, @Nullable @Named("savedPassword") Passphrase savedPassword, ForgetPasswordComponent.Builder forgetPassword, KeychainManager keychain, ExecutorService backgroundExecutorService) { this.window = window; - this.vault = vault; + this.vault = Objects.requireNonNull(vault, MasterkeyFileLoadingModule.NO_LOCAL_VAULT); this.result = result; this.savedPassword = savedPassword; this.forgetPassword = forgetPassword; diff --git a/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java b/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java index 50240c685..6cb568fb3 100644 --- a/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java +++ b/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java @@ -14,6 +14,7 @@ import org.cryptomator.ui.common.FxmlLoaderFactory; import org.cryptomator.ui.common.FxmlScene; import org.cryptomator.ui.common.StageFactory; import org.cryptomator.ui.keyloading.KeyLoadingComponent; +import org.cryptomator.ui.keyloading.KeyLoadingRef; import org.cryptomator.ui.keyloading.KeyLoadingStrategy; import org.cryptomator.ui.recoverykey.RecoveryKeyComponent; import org.jetbrains.annotations.Nullable; @@ -25,6 +26,7 @@ import javafx.beans.property.SimpleObjectProperty; import javafx.scene.Scene; import javafx.stage.Modality; import javafx.stage.Stage; +import java.io.IOException; import java.util.Map; import java.util.ResourceBundle; @@ -58,7 +60,11 @@ abstract class UnlockModule { @UnlockWindow @UnlockScoped 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 diff --git a/src/test/java/org/cryptomator/launcher/OpenHubVaultEventTest.java b/src/test/java/org/cryptomator/launcher/OpenHubVaultEventTest.java index 0bdc497c2..409361b62 100644 --- a/src/test/java/org/cryptomator/launcher/OpenHubVaultEventTest.java +++ b/src/test/java/org/cryptomator/launcher/OpenHubVaultEventTest.java @@ -73,21 +73,32 @@ public class OpenHubVaultEventTest { @Test @DisplayName("a config without a hub header is rejected") public void testNoHubHeader() { - var token = vaultConfig(KEY_ID, null); + var token = vaultConfig(KEY_ID, VAULT_ID, null); Assertions.assertThrows(IllegalArgumentException.class, // () -> OpenHubVaultEvent.tryParse(URI.create("org.cryptomator://vault/open#vaultConfig=" + token))); } @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 = { // - "hub+https://hub.example.com/api/vaults/not-a-uuid", // - "hub+https://hub.example.com/api/vaults/", // empty segment - "hub+https://hub.example.com" // no path at all + "not-a-uuid", // + "", // absent + "../../evil" // must never reach the api/vaults/{vaultId}/... request path }) - public void testInvalidVaultId(String keyId) { - var token = hubVaultConfig(keyId); + public void testInvalidVaultId(String vaultId) { + 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, // () -> OpenHubVaultEvent.tryParse(URI.create("org.cryptomator://vault/open#vaultConfig=" + token))); @@ -101,7 +112,7 @@ public class OpenHubVaultEventTest { "https:///api" // no host }) 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, // () -> OpenHubVaultEvent.tryParse(URI.create("org.cryptomator://vault/open#vaultConfig=" + token))); @@ -110,7 +121,7 @@ public class OpenHubVaultEventTest { @Test @DisplayName("an unusable auth endpoint is rejected") 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, // () -> OpenHubVaultEvent.tryParse(URI.create("org.cryptomator://vault/open#vaultConfig=" + token))); @@ -119,7 +130,7 @@ public class OpenHubVaultEventTest { @Test @DisplayName("an http endpoint is accepted here, host trust decides later") 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()); } @@ -180,14 +191,14 @@ public class OpenHubVaultEventTest { } 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 * 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 = """ { "kid":"%s",\ "typ":"JWT",\ @@ -195,10 +206,11 @@ public class OpenHubVaultEventTest { %s }""".formatted(keyId, extraHeaderFields == null ? "" : extraHeaderFields); var payload = """ - { "format":8,\ + { "jti":"%s",\ + "format":8,\ "cipherCombo":"SIV_GCM",\ "shorteningThreshold":220\ - }"""; + }""".formatted(vaultId); var encoder = Base64.getUrlEncoder().withoutPadding(); return encoder.encodeToString(header.getBytes(StandardCharsets.UTF_8)) // + "." + encoder.encodeToString(payload.getBytes(StandardCharsets.UTF_8)) //