From 81b128e4a3461039abfde053ed7a32e5a391b811 Mon Sep 17 00:00:00 2001 From: "Michael N. Lipp" Date: Sat, 22 Feb 2025 21:24:58 +0100 Subject: [PATCH] Clarify responsibilities of display secret monitor and reconciler. --- .../manager/DisplaySecretMonitor.java | 210 +--------------- .../manager/DisplaySecretReconciler.java | 224 +++++++++++++++++- .../vmoperator/manager/Reconciler.java | 5 +- webpages/vm-operator/upgrading.md | 24 +- webpages/vm-operator/user-gui.md | 12 +- 5 files changed, 252 insertions(+), 223 deletions(-) diff --git a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/DisplaySecretMonitor.java b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/DisplaySecretMonitor.java index 152f91e..99c8a11 100644 --- a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/DisplaySecretMonitor.java +++ b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/DisplaySecretMonitor.java @@ -1,6 +1,6 @@ /* * VM-Operator - * Copyright (C) 2024 Michael N. Lipp + * Copyright (C) 2025 Michael N. Lipp * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as @@ -18,8 +18,6 @@ package org.jdrupes.vmoperator.manager; -import com.google.gson.JsonObject; -import io.kubernetes.client.apimachinery.GroupVersionKind; import io.kubernetes.client.custom.V1Patch; import io.kubernetes.client.openapi.ApiException; import io.kubernetes.client.openapi.models.V1Secret; @@ -28,52 +26,26 @@ import io.kubernetes.client.util.Watch.Response; import io.kubernetes.client.util.generic.options.ListOptions; import io.kubernetes.client.util.generic.options.PatchOptions; import java.io.IOException; -import java.security.NoSuchAlgorithmException; -import java.security.SecureRandom; -import java.time.Instant; -import java.util.Collections; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.Scanner; import java.util.logging.Level; import static org.jdrupes.vmoperator.common.Constants.APP_NAME; -import static org.jdrupes.vmoperator.common.Constants.VM_OP_GROUP; -import static org.jdrupes.vmoperator.common.Constants.VM_OP_KIND_VM; import static org.jdrupes.vmoperator.common.Constants.VM_OP_NAME; import org.jdrupes.vmoperator.common.K8sClient; import org.jdrupes.vmoperator.common.K8sV1PodStub; import org.jdrupes.vmoperator.common.K8sV1SecretStub; -import org.jdrupes.vmoperator.common.VmDefinitionStub; import static org.jdrupes.vmoperator.manager.Constants.COMP_DISPLAY_SECRET; -import static org.jdrupes.vmoperator.manager.Constants.DATA_DISPLAY_PASSWORD; -import static org.jdrupes.vmoperator.manager.Constants.DATA_PASSWORD_EXPIRY; import org.jdrupes.vmoperator.manager.events.ChannelDictionary; -import org.jdrupes.vmoperator.manager.events.PrepareConsole; import org.jdrupes.vmoperator.manager.events.VmChannel; -import org.jdrupes.vmoperator.manager.events.VmDefChanged; import org.jgrapes.core.Channel; -import org.jgrapes.core.CompletionLock; -import org.jgrapes.core.Event; -import org.jgrapes.core.annotation.Handler; -import org.jgrapes.util.events.ConfigurationUpdate; -import org.jose4j.base64url.Base64; /** - * Watches for changes of display secrets. The component supports the - * following configuration properties: - * - * * `passwordValidity`: the validity of the random password in seconds. - * Used to calculate the password expiry time in the generated secret. + * Watches for changes of display secrets. Updates an artifical attribute + * of the pod running the VM in response to force an update of the files + * in the pod that reflect the information from the secret. */ @SuppressWarnings({ "PMD.DataflowAnomalyAnalysis", "PMD.TooManyStaticImports" }) public class DisplaySecretMonitor extends AbstractMonitor { - private int passwordValidity = 10; - private final List pendingPrepares - = Collections.synchronizedList(new LinkedList<>()); private final ChannelDictionary channelDictionary; /** @@ -93,27 +65,6 @@ public class DisplaySecretMonitor options(options); } - /** - * On configuration update. - * - * @param event the event - */ - @Handler - @Override - public void onConfigurationUpdate(ConfigurationUpdate event) { - super.onConfigurationUpdate(event); - event.structured(componentPath()).ifPresent(c -> { - try { - if (c.containsKey("passwordValidity")) { - passwordValidity = Integer - .parseInt((String) c.get("passwordValidity")); - } - } catch (ClassCastException e) { - logger.config("Malformed configuration: " + e.getMessage()); - } - }); - } - @Override protected void prepareMonitoring() throws IOException, ApiException { client(new K8sClient()); @@ -168,157 +119,4 @@ public class DisplaySecretMonitor + "\"}]"), patchOpts); } - - /** - * On get display secrets. - * - * @param event the event - * @param channel the channel - * @throws ApiException the api exception - */ - @Handler - @SuppressWarnings("PMD.StringInstantiation") - public void onPrepareConsole(PrepareConsole event, VmChannel channel) - throws ApiException { - // Update console user in status - var vmStub = VmDefinitionStub.get(client(), - new GroupVersionKind(VM_OP_GROUP, "", VM_OP_KIND_VM), - event.vmDefinition().namespace(), event.vmDefinition().name()); - var optVmDef = vmStub.updateStatus(from -> { - JsonObject status = from.statusJson(); - status.addProperty("consoleUser", event.user()); - return status; - }); - if (optVmDef.isEmpty()) { - return; - } - var vmDef = optVmDef.get(); - - // Check if access is possible - if (event.loginUser() - ? !vmDef.conditionStatus("Booted").orElse(false) - : !vmDef.conditionStatus("Running").orElse(false)) { - return; - } - - // Look for secret - ListOptions options = new ListOptions(); - options.setLabelSelector("app.kubernetes.io/name=" + APP_NAME + "," - + "app.kubernetes.io/component=" + COMP_DISPLAY_SECRET + "," - + "app.kubernetes.io/instance=" + vmDef.name()); - var stubs = K8sV1SecretStub.list(client(), vmDef.namespace(), options); - if (stubs.isEmpty()) { - // No secret means no password for this VM wanted - event.setResult(null); - return; - } - var stub = stubs.iterator().next(); - - // Check validity - var secret = stub.model().get(); - @SuppressWarnings("PMD.StringInstantiation") - var expiry = Optional.ofNullable(secret.getData() - .get(DATA_PASSWORD_EXPIRY)).map(b -> new String(b)).orElse(null); - if (secret.getData().get(DATA_DISPLAY_PASSWORD) != null - && stillValid(expiry)) { - // Fixed secret, don't touch - event.setResult( - new String(secret.getData().get(DATA_DISPLAY_PASSWORD))); - return; - } - updatePassword(stub, event); - } - - @SuppressWarnings("PMD.StringInstantiation") - private void updatePassword(K8sV1SecretStub stub, PrepareConsole event) - throws ApiException { - SecureRandom random = null; - try { - random = SecureRandom.getInstanceStrong(); - } catch (NoSuchAlgorithmException e) { // NOPMD - // "Every implementation of the Java platform is required - // to support at least one strong SecureRandom implementation." - } - byte[] bytes = new byte[16]; - random.nextBytes(bytes); - var password = Base64.encode(bytes); - var model = stub.model().get(); - model.setStringData(Map.of(DATA_DISPLAY_PASSWORD, password, - DATA_PASSWORD_EXPIRY, - Long.toString(Instant.now().getEpochSecond() + passwordValidity))); - event.setResult(password); - - // Prepare wait for confirmation (by VM status change) - var pending = new PendingGet(event, - event.vmDefinition().displayPasswordSerial().orElse(0L) + 1, - new CompletionLock(event, 1500)); - pendingPrepares.add(pending); - Event.onCompletion(event, e -> { - pendingPrepares.remove(pending); - }); - - // Update, will (eventually) trigger confirmation - stub.update(model).getObject(); - } - - private boolean stillValid(String expiry) { - if (expiry == null || "never".equals(expiry)) { - return true; - } - @SuppressWarnings({ "PMD.CloseResource", "resource" }) - var scanner = new Scanner(expiry); - if (!scanner.hasNextLong()) { - return false; - } - long expTime = scanner.nextLong(); - return expTime > Instant.now().getEpochSecond() + passwordValidity; - } - - /** - * On vm def changed. - * - * @param event the event - * @param channel the channel - */ - @Handler - @SuppressWarnings("PMD.AvoidSynchronizedStatement") - public void onVmDefChanged(VmDefChanged event, Channel channel) { - synchronized (pendingPrepares) { - String vmName = event.vmDefinition().name(); - for (var pending : pendingPrepares) { - if (pending.event.vmDefinition().name().equals(vmName) - && event.vmDefinition().displayPasswordSerial() - .map(s -> s >= pending.expectedSerial).orElse(false)) { - pending.lock.remove(); - // pending will be removed from pendingGest by - // waiting thread, see updatePassword - continue; - } - } - } - } - - /** - * The Class PendingGet. - */ - @SuppressWarnings("PMD.DataClass") - private static class PendingGet { - public final PrepareConsole event; - public final long expectedSerial; - public final CompletionLock lock; - - /** - * Instantiates a new pending get. - * - * @param event the event - * @param expectedSerial the expected serial - */ - public PendingGet(PrepareConsole event, long expectedSerial, - CompletionLock lock) { - super(); - this.event = event; - this.expectedSerial = expectedSerial; - this.lock = lock; - } - } } diff --git a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/DisplaySecretReconciler.java b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/DisplaySecretReconciler.java index dcae3a3..a281b8e 100644 --- a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/DisplaySecretReconciler.java +++ b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/DisplaySecretReconciler.java @@ -1,6 +1,6 @@ /* * VM-Operator - * Copyright (C) 2023 Michael N. Lipp + * Copyright (C) 2025 Michael N. Lipp * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as @@ -18,7 +18,9 @@ package org.jdrupes.vmoperator.manager; +import com.google.gson.JsonObject; import freemarker.template.TemplateException; +import io.kubernetes.client.apimachinery.GroupVersionKind; import io.kubernetes.client.openapi.ApiException; import io.kubernetes.client.openapi.models.V1ObjectMeta; import io.kubernetes.client.openapi.models.V1Secret; @@ -26,25 +28,83 @@ import io.kubernetes.client.util.generic.options.ListOptions; import java.io.IOException; import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; +import java.time.Instant; +import java.util.Collections; +import java.util.LinkedList; +import java.util.List; import java.util.Map; +import java.util.Optional; +import java.util.Scanner; import java.util.logging.Logger; +import static org.jdrupes.vmoperator.common.Constants.APP_NAME; +import static org.jdrupes.vmoperator.common.Constants.VM_OP_GROUP; +import static org.jdrupes.vmoperator.common.Constants.VM_OP_KIND_VM; import org.jdrupes.vmoperator.common.K8sV1SecretStub; -import static org.jdrupes.vmoperator.manager.Constants.APP_NAME; +import org.jdrupes.vmoperator.common.VmDefinitionStub; import static org.jdrupes.vmoperator.manager.Constants.COMP_DISPLAY_SECRET; import static org.jdrupes.vmoperator.manager.Constants.DATA_DISPLAY_PASSWORD; import static org.jdrupes.vmoperator.manager.Constants.DATA_PASSWORD_EXPIRY; +import org.jdrupes.vmoperator.manager.events.PrepareConsole; import org.jdrupes.vmoperator.manager.events.VmChannel; import org.jdrupes.vmoperator.manager.events.VmDefChanged; import org.jdrupes.vmoperator.util.DataPath; +import org.jgrapes.core.Channel; +import org.jgrapes.core.CompletionLock; +import org.jgrapes.core.Component; +import org.jgrapes.core.Event; +import org.jgrapes.core.annotation.Handler; +import org.jgrapes.util.events.ConfigurationUpdate; import org.jose4j.base64url.Base64; /** - * Delegee for reconciling the display secret + * The properties of the display secret do not only depend on the + * VM definition, but also on events that occur during runtime. + * The reconciler for the display secret is therefore a separate + * component. + * + * The reconciler supports the following configuration properties: + * + * * `passwordValidity`: the validity of the random password in seconds. + * Used to calculate the password expiry time in the generated secret. */ @SuppressWarnings({ "PMD.DataflowAnomalyAnalysis", "PMD.TooManyStaticImports" }) -/* default */ class DisplaySecretReconciler { +public class DisplaySecretReconciler extends Component { protected final Logger logger = Logger.getLogger(getClass().getName()); + private int passwordValidity = 10; + private final List pendingPrepares + = Collections.synchronizedList(new LinkedList<>()); + + /** + * On configuration update. + * + * @param event the event + */ + @Handler + public void onConfigurationUpdate(ConfigurationUpdate event) { + event.structured(componentPath()) + // for backward compatibility + .or(() -> { + var oldConfig = event + .structured("/Manager/Controller/DisplaySecretMonitor"); + if (oldConfig.isPresent()) { + logger.warning(() -> "Using configuration with old " + + "path '/Manager/Controller/DisplaySecretMonitor' " + + "for `passwordValidity`, please update " + + "the configuration."); + } + return oldConfig; + }).ifPresent(c -> { + try { + if (c.containsKey("passwordValidity")) { + passwordValidity = Integer + .parseInt((String) c.get("passwordValidity")); + } + } catch (ClassCastException e) { + logger.config("Malformed configuration: " + e.getMessage()); + } + }); + } /** * Reconcile. If the configuration prevents generating a secret @@ -104,4 +164,160 @@ import org.jose4j.base64url.Base64; K8sV1SecretStub.create(channel.client(), secret); } + /** + * Prepares access to the console for the user from the event. + * Generates a new password and sends it to the runner. + * Requests the VM (via the runner) to login the user if specified + * in the event. + * + * @param event the event + * @param channel the channel + * @throws ApiException the api exception + */ + @Handler + @SuppressWarnings("PMD.StringInstantiation") + public void onPrepareConsole(PrepareConsole event, VmChannel channel) + throws ApiException { + // Update console user in status + var vmStub = VmDefinitionStub.get(channel.client(), + new GroupVersionKind(VM_OP_GROUP, "", VM_OP_KIND_VM), + event.vmDefinition().namespace(), event.vmDefinition().name()); + var optVmDef = vmStub.updateStatus(from -> { + JsonObject status = from.statusJson(); + status.addProperty("consoleUser", event.user()); + return status; + }); + if (optVmDef.isEmpty()) { + return; + } + var vmDef = optVmDef.get(); + + // Check if access is possible + if (event.loginUser() + ? !vmDef.conditionStatus("Booted").orElse(false) + : !vmDef.conditionStatus("Running").orElse(false)) { + return; + } + + // Look for secret + ListOptions options = new ListOptions(); + options.setLabelSelector("app.kubernetes.io/name=" + APP_NAME + "," + + "app.kubernetes.io/component=" + COMP_DISPLAY_SECRET + "," + + "app.kubernetes.io/instance=" + vmDef.name()); + var stubs = K8sV1SecretStub.list(channel.client(), vmDef.namespace(), + options); + if (stubs.isEmpty()) { + // No secret means no password for this VM wanted + event.setResult(null); + return; + } + var stub = stubs.iterator().next(); + + // Check validity + var secret = stub.model().get(); + @SuppressWarnings("PMD.StringInstantiation") + var expiry = Optional.ofNullable(secret.getData() + .get(DATA_PASSWORD_EXPIRY)).map(b -> new String(b)).orElse(null); + if (secret.getData().get(DATA_DISPLAY_PASSWORD) != null + && stillValid(expiry)) { + // Fixed secret, don't touch + event.setResult( + new String(secret.getData().get(DATA_DISPLAY_PASSWORD))); + return; + } + updatePassword(stub, event); + } + + @SuppressWarnings("PMD.StringInstantiation") + private void updatePassword(K8sV1SecretStub stub, PrepareConsole event) + throws ApiException { + SecureRandom random = null; + try { + random = SecureRandom.getInstanceStrong(); + } catch (NoSuchAlgorithmException e) { // NOPMD + // "Every implementation of the Java platform is required + // to support at least one strong SecureRandom implementation." + } + byte[] bytes = new byte[16]; + random.nextBytes(bytes); + var password = Base64.encode(bytes); + var model = stub.model().get(); + model.setStringData(Map.of(DATA_DISPLAY_PASSWORD, password, + DATA_PASSWORD_EXPIRY, + Long.toString(Instant.now().getEpochSecond() + passwordValidity))); + event.setResult(password); + + // Prepare wait for confirmation (by VM status change) + var pending = new PendingGet(event, + event.vmDefinition().displayPasswordSerial().orElse(0L) + 1, + new CompletionLock(event, 1500)); + pendingPrepares.add(pending); + Event.onCompletion(event, e -> { + pendingPrepares.remove(pending); + }); + + // Update, will (eventually) trigger confirmation + stub.update(model).getObject(); + } + + private boolean stillValid(String expiry) { + if (expiry == null || "never".equals(expiry)) { + return true; + } + @SuppressWarnings({ "PMD.CloseResource", "resource" }) + var scanner = new Scanner(expiry); + if (!scanner.hasNextLong()) { + return false; + } + long expTime = scanner.nextLong(); + return expTime > Instant.now().getEpochSecond() + passwordValidity; + } + + /** + * On vm def changed. + * + * @param event the event + * @param channel the channel + */ + @Handler + @SuppressWarnings("PMD.AvoidSynchronizedStatement") + public void onVmDefChanged(VmDefChanged event, Channel channel) { + synchronized (pendingPrepares) { + String vmName = event.vmDefinition().name(); + for (var pending : pendingPrepares) { + if (pending.event.vmDefinition().name().equals(vmName) + && event.vmDefinition().displayPasswordSerial() + .map(s -> s >= pending.expectedSerial).orElse(false)) { + pending.lock.remove(); + // pending will be removed from pendingGest by + // waiting thread, see updatePassword + continue; + } + } + } + } + + /** + * The Class PendingGet. + */ + @SuppressWarnings("PMD.DataClass") + private static class PendingGet { + public final PrepareConsole event; + public final long expectedSerial; + public final CompletionLock lock; + + /** + * Instantiates a new pending get. + * + * @param event the event + * @param expectedSerial the expected serial + */ + public PendingGet(PrepareConsole event, long expectedSerial, + CompletionLock lock) { + super(); + this.event = event; + this.expectedSerial = expectedSerial; + this.lock = lock; + } + } } diff --git a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/Reconciler.java b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/Reconciler.java index 7dbb410..7969d46 100644 --- a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/Reconciler.java +++ b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/Reconciler.java @@ -138,6 +138,8 @@ import org.jgrapes.util.events.ConfigurationUpdate; * properties to be used by the runners managed by the controller. * This property is a string that holds the content of * a logging.properties file. + * + * @see org.jdrupes.vmoperator.manager.DisplaySecretReconciler */ @SuppressWarnings({ "PMD.DataflowAnomalyAnalysis", "PMD.AvoidDuplicateLiterals" }) @@ -163,6 +165,7 @@ public class Reconciler extends Component { * * @param componentChannel the component channel */ + @SuppressWarnings("PMD.ConstructorCallsOverridableMethod") public Reconciler(Channel componentChannel) { super(componentChannel); @@ -177,7 +180,7 @@ public class Reconciler extends Component { fmConfig.setClassForTemplateLoading(Reconciler.class, ""); cmReconciler = new ConfigMapReconciler(fmConfig); - dsReconciler = new DisplaySecretReconciler(); + dsReconciler = attach(new DisplaySecretReconciler()); stsReconciler = new StatefulSetReconciler(fmConfig); pvcReconciler = new PvcReconciler(fmConfig); podReconciler = new PodReconciler(fmConfig); diff --git a/webpages/vm-operator/upgrading.md b/webpages/vm-operator/upgrading.md index 77cacad..2c4253e 100644 --- a/webpages/vm-operator/upgrading.md +++ b/webpages/vm-operator/upgrading.md @@ -7,16 +7,24 @@ layout: vm-operator ## To version 4.0.0 -The VmViewer conlet has been renamed to VmAccess. This affects the -[configuration](https://jdrupes.org/vm-operator/user-gui.html). Configuration information using the old path -"/Manager/GuiHttpServer/ConsoleWeblet/WebConsole/ComponentCollector/VmViewer" -is still accepted for backward compatibility, but should be updated. + * The VmViewer conlet has been renamed to VmAccess. This affects the + [configuration](https://jdrupes.org/vm-operator/user-gui.html). Configuration + information using the old path + `/Manager/GuiHttpServer/ConsoleWeblet/WebConsole/ComponentCollector/VmViewer` + is still accepted for backward compatibility until the next major version, + but should be updated. -The change of name also causes conlets added to the overview page by -users to "disappear" from the GUI. They have to be re-added. + The change of name also causes conlets added to the overview page by + users to "disappear" from the GUI. They have to be re-added. -The latter behavior also applies to the VmConlet conlet which has been -renamed to VmMgmt. + The latter behavior also applies to the VmConlet conlet which has been + renamed to VmMgmt. + + * The configuration property `passwordValidity` has been moved from component + `/Manager/Controller/DisplaySecretMonitor` to + `/Manager/Controller/Reconciler/DisplaySecretReconciler`. The old path is + still accepted for backward compatibility until the next major version, + but should be updated. ## To version 3.4.0 diff --git a/webpages/vm-operator/user-gui.md b/webpages/vm-operator/user-gui.md index 0439db2..bc0b93e 100644 --- a/webpages/vm-operator/user-gui.md +++ b/webpages/vm-operator/user-gui.md @@ -127,16 +127,20 @@ of 16 (strong) random bytes (128 random bits). It is valid for 10 seconds only. This may be challenging on a slower computer or if users may not enable automatic open for connection files in the browser. The validity can therefore be adjusted in the -configuration. +configuration.[^oldPath] ```yaml "/Manager": "/Controller": - "/DisplaySecretMonitor": - # Validity of generated password in seconds - passwordValidity: 10 + "/Reconciler": + "/DisplaySecretReconciler": + # Validity of generated password in seconds + passwordValidity: 10 ``` +[^oldPath]: Before version 4.0, the path for `passwordValidity` was + `/Manager/Controller/DisplaySecretMonitor`. + Taking into account that the controller generates a display secret automatically by default, this approach to securing console access should be sufficient in all cases. (Any feedback