From d29614afcc847a99c601189b14acd3144c18be02 Mon Sep 17 00:00:00 2001 From: crschnick Date: Mon, 19 Feb 2024 00:27:04 +0000 Subject: [PATCH] More secret and elevation rework --- .../java/io/xpipe/app/prefs/AppPrefs.java | 7 ++- .../io/xpipe/app/prefs/SecurityCategory.java | 5 +- .../java/io/xpipe/app/util/AskpassAlert.java | 38 ++++++++++++-- .../io/xpipe/app/util/ElevationAccess.java | 47 ----------------- .../app/util/ElevationAccessChoiceComp.java | 27 ---------- .../java/io/xpipe/app/util/SecretManager.java | 4 +- .../java/io/xpipe/app/util/SecretQuery.java | 50 ++++++++++++++++++- .../xpipe/app/util/SecretQueryProgress.java | 9 +++- .../app/util/SecretRetrievalStrategy.java | 2 +- .../app/util/TerminalLauncherManager.java | 4 +- .../resources/lang/translations_en.properties | 4 +- .../io/xpipe/core/process/ShellDialect.java | 2 + .../core/process/ShellSecurityPolicy.java | 8 --- 13 files changed, 105 insertions(+), 102 deletions(-) delete mode 100644 app/src/main/java/io/xpipe/app/util/ElevationAccess.java delete mode 100644 app/src/main/java/io/xpipe/app/util/ElevationAccessChoiceComp.java diff --git a/app/src/main/java/io/xpipe/app/prefs/AppPrefs.java b/app/src/main/java/io/xpipe/app/prefs/AppPrefs.java index c0e21632d..a665b4a78 100644 --- a/app/src/main/java/io/xpipe/app/prefs/AppPrefs.java +++ b/app/src/main/java/io/xpipe/app/prefs/AppPrefs.java @@ -10,7 +10,6 @@ import io.xpipe.app.fxcomps.Comp; import io.xpipe.app.fxcomps.util.PlatformThread; import io.xpipe.app.storage.DataStorage; import io.xpipe.app.util.ApplicationHelper; -import io.xpipe.app.util.ElevationAccess; import io.xpipe.app.util.PasswordLockSecretValue; import io.xpipe.core.util.InPlaceSecretValue; import io.xpipe.core.util.ModuleHelper; @@ -170,9 +169,9 @@ public class AppPrefs { return disableTerminalRemotePasswordPreparation; } - public final Property elevationPolicy = map(new SimpleObjectProperty<>(ElevationAccess.ALLOW), "elevationPolicy", ElevationAccess.class); - public ObservableValue elevationPolicy() { - return elevationPolicy; + public final Property alwaysConfirmElevation = map(new SimpleObjectProperty<>(false), "alwaysConfirmElevation", Boolean.class); + public ObservableValue alwaysConfirmElevation() { + return alwaysConfirmElevation; } public final BooleanProperty dontCachePasswords = map(new SimpleBooleanProperty(false), "dontCachePasswords", Boolean.class); diff --git a/app/src/main/java/io/xpipe/app/prefs/SecurityCategory.java b/app/src/main/java/io/xpipe/app/prefs/SecurityCategory.java index d73f697d1..396955ce1 100644 --- a/app/src/main/java/io/xpipe/app/prefs/SecurityCategory.java +++ b/app/src/main/java/io/xpipe/app/prefs/SecurityCategory.java @@ -1,7 +1,6 @@ package io.xpipe.app.prefs; import io.xpipe.app.fxcomps.Comp; -import io.xpipe.app.util.ElevationAccessChoiceComp; import io.xpipe.app.util.OptionsBuilder; public class SecurityCategory extends AppPrefsCategory { @@ -16,8 +15,8 @@ public class SecurityCategory extends AppPrefsCategory { var builder = new OptionsBuilder(); builder.addTitle("securityPolicy") .sub(new OptionsBuilder() - .nameAndDescription("elevationPolicy") - .addComp(new ElevationAccessChoiceComp(prefs.elevationPolicy).minWidth(250), prefs.elevationPolicy) + .nameAndDescription("alwaysConfirmElevation") + .addToggle(prefs.alwaysConfirmElevation) .nameAndDescription("dontCachePasswords") .addToggle(prefs.dontCachePasswords) .nameAndDescription("denyTempScriptCreation") diff --git a/app/src/main/java/io/xpipe/app/util/AskpassAlert.java b/app/src/main/java/io/xpipe/app/util/AskpassAlert.java index 704c40e57..3e8cff183 100644 --- a/app/src/main/java/io/xpipe/app/util/AskpassAlert.java +++ b/app/src/main/java/io/xpipe/app/util/AskpassAlert.java @@ -4,6 +4,7 @@ import io.xpipe.app.core.AppI18n; import io.xpipe.app.core.AppWindowHelper; import io.xpipe.app.fxcomps.impl.SecretFieldComp; import io.xpipe.core.util.InPlaceSecretValue; +import javafx.animation.AnimationTimer; import javafx.application.Platform; import javafx.beans.property.SimpleObjectProperty; import javafx.scene.control.Alert; @@ -12,25 +13,54 @@ import javafx.stage.Stage; public class AskpassAlert { - public static SecretQueryResult queryRaw(String prompt) { - var prop = new SimpleObjectProperty(); + public static SecretQueryResult queryRaw(String prompt, InPlaceSecretValue secretValue) { + var prop = new SimpleObjectProperty<>(secretValue); var r = AppWindowHelper.showBlockingAlert(alert -> { alert.setTitle(AppI18n.get("askpassAlertTitle")); alert.setHeaderText(prompt); alert.setAlertType(Alert.AlertType.CONFIRMATION); - var text = new SecretFieldComp(prop).createRegion(); + var text = new SecretFieldComp(prop).createStructure().get(); alert.getDialogPane().setContent(new StackPane(text)); var stage = (Stage) alert.getDialogPane().getScene().getWindow(); stage.setAlwaysOnTop(true); + var anim = new AnimationTimer() { + + private long lastRun = 0; + + @Override + public void handle(long now) { + if (lastRun == 0) { + lastRun = now; + return; + } + + long elapsed = (now - lastRun) / 1_000_000; + if (elapsed < 1000) { + return; + } + + stage.requestFocus(); + lastRun = now; + } + }; + alert.setOnShown(event -> { stage.requestFocus(); + anim.start(); // Wait 1 pulse before focus so that the scene can be assigned to text - Platform.runLater(text::requestFocus); + Platform.runLater(() -> { + text.requestFocus(); + text.end(); + }); event.consume(); }); + alert.setOnHiding(event -> { + anim.stop(); + }); + }) .filter(b -> b.getButtonData().isDefaultButton()) .map(t -> { diff --git a/app/src/main/java/io/xpipe/app/util/ElevationAccess.java b/app/src/main/java/io/xpipe/app/util/ElevationAccess.java deleted file mode 100644 index 39da269bc..000000000 --- a/app/src/main/java/io/xpipe/app/util/ElevationAccess.java +++ /dev/null @@ -1,47 +0,0 @@ -package io.xpipe.app.util; - -import io.xpipe.app.core.AppI18n; -import io.xpipe.app.core.AppWindowHelper; -import io.xpipe.app.prefs.AppPrefs; -import io.xpipe.app.storage.DataStorage; -import io.xpipe.core.process.ShellControl; - -public enum ElevationAccess { - - ALLOW { - @Override - public boolean requestElevationUsage(ShellControl shellControl) { - return true; - } - }, - ASK { - @Override - public boolean requestElevationUsage(ShellControl shellControl) { - var name = shellControl.getSourceStore().flatMap(shellStore -> DataStorage.get().getStoreEntryIfPresent(shellStore)) - .map(entry -> entry.getName()).orElse("a system"); - return AppWindowHelper.showConfirmationAlert( - AppI18n.observable("elevationRequestTitle"), - AppI18n.observable("elevationRequestHeader", name), - AppI18n.observable("elevationRequestDescription") - ); - } - }, - DENY { - @Override - public boolean requestElevationUsage(ShellControl shellControl) { - return false; - } - }; - - public boolean requestElevationUsage(ShellControl shellControl) { - return false; - } - - public static boolean request(ShellControl shellControl) { - if (AppPrefs.get() == null) { - return true; - } - - return AppPrefs.get().elevationPolicy().getValue().requestElevationUsage(shellControl); - } -} diff --git a/app/src/main/java/io/xpipe/app/util/ElevationAccessChoiceComp.java b/app/src/main/java/io/xpipe/app/util/ElevationAccessChoiceComp.java deleted file mode 100644 index 3da475844..000000000 --- a/app/src/main/java/io/xpipe/app/util/ElevationAccessChoiceComp.java +++ /dev/null @@ -1,27 +0,0 @@ -package io.xpipe.app.util; - -import io.xpipe.app.core.AppI18n; -import io.xpipe.app.fxcomps.SimpleComp; -import io.xpipe.app.fxcomps.impl.ChoiceComp; -import javafx.beans.property.Property; -import javafx.beans.value.ObservableValue; -import javafx.scene.layout.Region; - -import java.util.LinkedHashMap; - -public class ElevationAccessChoiceComp extends SimpleComp { - - private final Property value; - - public ElevationAccessChoiceComp(Property value) {this.value = value;} - - @Override - protected Region createSimple() { - var map = new LinkedHashMap>(); - map.put(ElevationAccess.ALLOW, AppI18n.observable("allow")); - map.put(ElevationAccess.ASK, AppI18n.observable("ask")); - map.put(ElevationAccess.DENY, AppI18n.observable("deny")); - var c = new ChoiceComp<>(value, map, false); - return c.createRegion(); - } -} diff --git a/app/src/main/java/io/xpipe/app/util/SecretManager.java b/app/src/main/java/io/xpipe/app/util/SecretManager.java index 9cdad9b55..4770193fc 100644 --- a/app/src/main/java/io/xpipe/app/util/SecretManager.java +++ b/app/src/main/java/io/xpipe/app/util/SecretManager.java @@ -26,8 +26,8 @@ public class SecretManager { .findFirst(); } - public static SecretQueryProgress expectCacheablePrompt(UUID request, UUID storeId, CountDown countDown) { - var p = new SecretQueryProgress(request, storeId, List.of(SecretQuery.prompt(true)), SecretQuery.prompt(false), countDown); + public static SecretQueryProgress expectElevationPrompt(UUID request, UUID secretId, CountDown countDown, boolean askIfNeeded) { + var p = new SecretQueryProgress(request, secretId, List.of(askIfNeeded ? SecretQuery.elevation(secretId) : SecretQuery.prompt(true)), SecretQuery.prompt(false), countDown); progress.add(p); return p; } diff --git a/app/src/main/java/io/xpipe/app/util/SecretQuery.java b/app/src/main/java/io/xpipe/app/util/SecretQuery.java index e193d91ac..9acef8cea 100644 --- a/app/src/main/java/io/xpipe/app/util/SecretQuery.java +++ b/app/src/main/java/io/xpipe/app/util/SecretQuery.java @@ -1,12 +1,55 @@ package io.xpipe.app.util; +import io.xpipe.app.prefs.AppPrefs; +import io.xpipe.core.util.SecretReference; + +import java.util.Optional; +import java.util.UUID; + public interface SecretQuery { + static SecretQuery elevation(UUID secretId) { + return new SecretQuery() { + + @Override + public SecretQueryResult query(String prompt) { + return AskpassAlert.queryRaw(prompt, null); + } + + @Override + public Optional retrieveCache(String prompt, SecretReference reference) { + var found = SecretQuery.super.retrieveCache(prompt, reference); + if (found.isEmpty()) { + return Optional.empty(); + } + + var ask = AppPrefs.get().alwaysConfirmElevation().getValue(); + if (!ask) { + return found; + } + + var inPlace = found.get().getSecret().inPlace(); + var r = AskpassAlert.queryRaw(prompt, inPlace); + return r.isCancelled() ? Optional.empty() : found; + } + + @Override + public boolean cache() { + return true; + } + + @Override + public boolean retryOnFail() { + return true; + } + }; + } + static SecretQuery prompt(boolean cache) { return new SecretQuery() { @Override public SecretQueryResult query(String prompt) { - return AskpassAlert.queryRaw(prompt); + return AskpassAlert.queryRaw(prompt, null); } @Override @@ -21,6 +64,11 @@ public interface SecretQuery { }; } + default Optional retrieveCache(String prompt, SecretReference reference) { + var r = SecretManager.get(reference); + return r.map(secretValue -> new SecretQueryResult(secretValue, false)); + } + SecretQueryResult query(String prompt); boolean cache(); diff --git a/app/src/main/java/io/xpipe/app/util/SecretQueryProgress.java b/app/src/main/java/io/xpipe/app/util/SecretQueryProgress.java index 043405af9..fe8c89e17 100644 --- a/app/src/main/java/io/xpipe/app/util/SecretQueryProgress.java +++ b/app/src/main/java/io/xpipe/app/util/SecretQueryProgress.java @@ -78,9 +78,14 @@ public class SecretQueryProgress { } if (shouldCache) { - var cached = SecretManager.get(ref); + var cached = sup.retrieveCache(prompt, ref); if (cached.isPresent()) { - return cached.get(); + if (cached.get().isCancelled()) { + requestCancelled = true; + return null; + } + + return cached.get().getSecret(); } } diff --git a/app/src/main/java/io/xpipe/app/util/SecretRetrievalStrategy.java b/app/src/main/java/io/xpipe/app/util/SecretRetrievalStrategy.java index 21be467b9..77cd76135 100644 --- a/app/src/main/java/io/xpipe/app/util/SecretRetrievalStrategy.java +++ b/app/src/main/java/io/xpipe/app/util/SecretRetrievalStrategy.java @@ -83,7 +83,7 @@ public interface SecretRetrievalStrategy { return new SecretQuery() { @Override public SecretQueryResult query(String prompt) { - return AskpassAlert.queryRaw(prompt); + return AskpassAlert.queryRaw(prompt, null); } @Override diff --git a/app/src/main/java/io/xpipe/app/util/TerminalLauncherManager.java b/app/src/main/java/io/xpipe/app/util/TerminalLauncherManager.java index b9cbc26d8..775c32ba6 100644 --- a/app/src/main/java/io/xpipe/app/util/TerminalLauncherManager.java +++ b/app/src/main/java/io/xpipe/app/util/TerminalLauncherManager.java @@ -3,6 +3,7 @@ package io.xpipe.app.util; import io.xpipe.beacon.ClientException; import io.xpipe.beacon.ServerException; import io.xpipe.core.process.ProcessControl; +import io.xpipe.core.process.ProcessOutputException; import io.xpipe.core.process.TerminalInitScriptConfig; import lombok.Setter; import lombok.Value; @@ -64,7 +65,8 @@ public class TerminalLauncherManager { var r = e.getResult(); if (r instanceof ResultFailure failure) { entries.remove(request); - throw new ServerException(failure.getThrowable()); + var t = failure.getThrowable(); + throw new ServerException(t instanceof ProcessOutputException pex ? pex.getOutput() : t.getMessage()); } return ((ResultSuccess) r).getTargetScript(); diff --git a/app/src/main/resources/io/xpipe/app/resources/lang/translations_en.properties b/app/src/main/resources/io/xpipe/app/resources/lang/translations_en.properties index fe491aa77..3f3017a10 100644 --- a/app/src/main/resources/io/xpipe/app/resources/lang/translations_en.properties +++ b/app/src/main/resources/io/xpipe/app/resources/lang/translations_en.properties @@ -7,8 +7,8 @@ common=Common key=Key color=Color roadmap=Roadmap and feature requests -elevationPolicy=Elevation policy -elevationPolicyDescription=Controls how to handle cases when elevated access might be required to run a command on a system, e.g. with sudo.\n\nThis can be overridden by connection-specific settings. +alwaysConfirmElevation=Always confirm elevation +alwaysConfirmElevationDescription=Controls how to handle cases when elevated access is required to run a command on a system, e.g. with sudo.\n\nBy default, any sudo credentials are cached during a session and automatically provided when needed. If this option is enabled, it will ask you to confirm the elevation access every time. allow=Allow ask=Ask deny=Deny diff --git a/core/src/main/java/io/xpipe/core/process/ShellDialect.java b/core/src/main/java/io/xpipe/core/process/ShellDialect.java index a4c4b6a9b..77132ef20 100644 --- a/core/src/main/java/io/xpipe/core/process/ShellDialect.java +++ b/core/src/main/java/io/xpipe/core/process/ShellDialect.java @@ -95,6 +95,8 @@ public interface ShellDialect { String getDiscardOperator(); + String nullStdin(String command); + String getScriptPermissionsCommand(String file); ShellDialectAskpass getAskpass(); diff --git a/core/src/main/java/io/xpipe/core/process/ShellSecurityPolicy.java b/core/src/main/java/io/xpipe/core/process/ShellSecurityPolicy.java index 9fed33653..6d378a992 100644 --- a/core/src/main/java/io/xpipe/core/process/ShellSecurityPolicy.java +++ b/core/src/main/java/io/xpipe/core/process/ShellSecurityPolicy.java @@ -2,13 +2,5 @@ package io.xpipe.core.process; public interface ShellSecurityPolicy { - boolean checkElevate(ShellControl shellControl); - - default void elevateOrThrow(ShellControl shellControl) { - if (!checkElevate(shellControl)) { - throw new UnsupportedOperationException("Elevation is not allowed for this system"); - } - } - boolean permitTempScriptCreation(); }