From 2f694fa2fa304dcbf666e935b7ece5ba5ac4584b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ondro=20Mih=C3=A1lyi?= Date: Wed, 15 Jan 2025 08:06:10 +0100 Subject: [PATCH] Mask secrets in CommandInvokedEvent and Command logger (#25307) * Mask secrets in CommandInvokedEvent and Command logger Extracted ExecutionContext inner class to package-private class CommandRunnerExecutionContext. Was too big and deserved a file on its own. Moved mask method to ParameterMap and added unit tests. A test to verfiy that the command logger doesn't log passwords --- .../admingui/common/util/RestUtil.java | 17 +- .../itest/tools/GlassFishTestEnvironment.java | 6 +- .../main/itest/tools/asadmin/Asadmin.java | 88 ++++-- .../commandlogger/CommandLoggerITest.java | 33 ++- .../org/glassfish/common/util/Constants.java | 15 + nucleus/common/glassfish-api/pom.xml | 4 + .../org/glassfish/api/admin/ParameterMap.java | 21 ++ .../glassfish/api/admin/ParameterMapTest.java | 87 ++++++ .../api/events/CommandInvokedEvent.java | 12 +- .../sun/enterprise/v3/admin/AdminAdapter.java | 2 +- .../admin/CommandRunnerExecutionContext.java | 259 ++++++++++++++++++ .../v3/admin/CommandRunnerImpl.java | 242 +--------------- 12 files changed, 506 insertions(+), 280 deletions(-) create mode 100644 nucleus/common/glassfish-api/src/test/java/org/glassfish/api/admin/ParameterMapTest.java create mode 100644 nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/admin/CommandRunnerExecutionContext.java diff --git a/appserver/admingui/common/src/main/java/org/glassfish/admingui/common/util/RestUtil.java b/appserver/admingui/common/src/main/java/org/glassfish/admingui/common/util/RestUtil.java index 7bdd9134651..67e7650988f 100644 --- a/appserver/admingui/common/src/main/java/org/glassfish/admingui/common/util/RestUtil.java +++ b/appserver/admingui/common/src/main/java/org/glassfish/admingui/common/util/RestUtil.java @@ -41,7 +41,6 @@ import java.io.IOException; import java.net.URLEncoder; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -71,6 +70,7 @@ import org.xml.sax.SAXException; import static com.sun.logging.LogCleanerUtil.neutralizeForLog; +import static org.glassfish.common.util.Constants.PASSWORD_ATTRIBUTE_NAMES; /** * @@ -286,7 +286,7 @@ public static Map maskOffPassword(Map attrs) { for (Map.Entry e : attrs.entrySet()) { String key = e.getKey().toLowerCase(GuiUtil.guiLocale); - if (pswdAttrList.contains(key)) { + if (PASSWORD_ATTRIBUTE_NAMES.contains(key)) { masked.put(e.getKey(), "*******"); } else { masked.put(e.getKey(), e.getValue()); @@ -967,17 +967,4 @@ public boolean verify(String host, SSLSession sslSession) { } } - /* - * This is a list of attribute name of password for different command. We need to mask its value during logging. - */ - private static final List pswdAttrList = Arrays.asList( - "sshpassword", /* create-node-ssh , setup-ssh , update-node, update-node-ssh */ - "dbpassword", /* jms-availability-service */ - "jmsdbpassword", /* configure-jms-cluster */ - "password", /* change-admin-password */ - "newpassword", /* change-admin-password */ - "jmsdbpassword", /* configure-jms-cluster */ - "mappedpassword", /* create-connector-security-map, update-connector-security-map */ - "userpassword", /* create-file-user , update-file-user */ - "aliaspassword" /* create-password-alias , update-password-alias */); } diff --git a/appserver/itest-tools/src/main/java/org/glassfish/main/itest/tools/GlassFishTestEnvironment.java b/appserver/itest-tools/src/main/java/org/glassfish/main/itest/tools/GlassFishTestEnvironment.java index 2b43bd07dcb..001892153e4 100644 --- a/appserver/itest-tools/src/main/java/org/glassfish/main/itest/tools/GlassFishTestEnvironment.java +++ b/appserver/itest-tools/src/main/java/org/glassfish/main/itest/tools/GlassFishTestEnvironment.java @@ -72,6 +72,7 @@ public class GlassFishTestEnvironment { private static final File PASSWORD_FILE = findPasswordFile("password.txt"); private static final int ASADMIN_START_DOMAIN_TIMEOUT = 30_000; + private static final int ASADMIN_START_DOMAIN_TIMEOUT_FOR_DEBUG = 300_000; static { LOG.log(Level.INFO, "Using basedir: {0}", BASEDIR); @@ -87,9 +88,11 @@ public class GlassFishTestEnvironment { // START_DOMAIN_TIMEOUT for us waiting for the end of the asadmin start-domain process. asadmin.withEnv("AS_START_TIMEOUT", Integer.toString(ASADMIN_START_DOMAIN_TIMEOUT - 5000)); } + final int timeout = isStartDomainSuspendEnabled() + ? ASADMIN_START_DOMAIN_TIMEOUT_FOR_DEBUG : ASADMIN_START_DOMAIN_TIMEOUT; // This is the absolutely first start - if it fails, all other starts will fail too. // Note: --suspend implicitly enables --debug - assertThat(asadmin.exec(ASADMIN_START_DOMAIN_TIMEOUT, "start-domain", + assertThat(asadmin.exec(timeout,"start-domain", isStartDomainSuspendEnabled() ? "--suspend" : "--debug"), asadminOK()); } @@ -259,7 +262,6 @@ public static void createFileUser(String realmName, String user, String password } } - /** * This will delete the jobs.xml file */ diff --git a/appserver/itest-tools/src/main/java/org/glassfish/main/itest/tools/asadmin/Asadmin.java b/appserver/itest-tools/src/main/java/org/glassfish/main/itest/tools/asadmin/Asadmin.java index d28ea0c8f33..1ea7b81f68e 100644 --- a/appserver/itest-tools/src/main/java/org/glassfish/main/itest/tools/asadmin/Asadmin.java +++ b/appserver/itest-tools/src/main/java/org/glassfish/main/itest/tools/asadmin/Asadmin.java @@ -13,7 +13,6 @@ * * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 */ - package org.glassfish.main.itest.tools.asadmin; import com.sun.enterprise.universal.process.ProcessManager; @@ -21,6 +20,11 @@ import com.sun.enterprise.universal.process.ProcessManagerTimeoutException; import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -37,12 +41,12 @@ import static org.hamcrest.MatcherAssert.assertThat; /** - * Tool for executing asadmin/asadmin.bat commands. - * The tool is stateless. + * Tool for executing asadmin/asadmin.bat commands. The tool is stateless. * * @author David Matejcek */ public class Asadmin { + private static final Logger LOG = Logger.getLogger(Asadmin.class.getName()); private static final int DEFAULT_TIMEOUT_MSEC = 30 * 1000; @@ -59,26 +63,27 @@ public class Asadmin { private final File adminPasswordFile; private final boolean terse; private final Map environment = new HashMap<>(); - + private final Map additionalPasswords = new HashMap<>(); /** * Creates a stateless instance of the tool. * * @param asadmin - executable file * @param adminUser - username authorized to use the domain - * @param adminPasswordFile - a file containing admin's password set as AS_ADMIN_PASSWORD=... + * @param adminPasswordFile - a file containing admin's password set as + * AS_ADMIN_PASSWORD=... */ public Asadmin(final File asadmin, final String adminUser, final File adminPasswordFile) { this(asadmin, adminUser, adminPasswordFile, false); } - /** * Creates a stateless instance of the tool. * * @param asadmin - executable file * @param adminUser - username authorized to use the domain - * @param adminPasswordFile - a file containing admin's password set as AS_ADMIN_PASSWORD=... + * @param adminPasswordFile - a file containing admin's password set as + * AS_ADMIN_PASSWORD=... * @param terse - to produce output, minimized and suitable for parsing. */ public Asadmin(final File asadmin, final String adminUser, final File adminPasswordFile, final boolean terse) { @@ -88,7 +93,6 @@ public Asadmin(final File asadmin, final String adminUser, final File adminPassw this.terse = terse; } - /** * Adds environment property set for the asadmin execution. * @@ -101,6 +105,27 @@ public Asadmin withEnv(final String name, final String value) { return this; } + /** + * Adds a password to the password file. + * + * @param name Name in the password file + * @param secretValue Value in the password file + * @return this + */ + public Asadmin withPassword(final String name, final String secretValue) { + this.additionalPasswords.put(name, secretValue); + return this; + } + + /** + * Removes all custom passwords. + * + * @return this + */ + public Asadmin resetPasswords() { + this.additionalPasswords.clear(); + return this; + } /** * @return asadmin command file name @@ -109,7 +134,6 @@ public String getCommandName() { return asadmin.getName(); } - public KeyAndValue getValue(final String key, final Function transformer) { List> result = get(key, transformer); if (result.isEmpty()) { @@ -121,20 +145,19 @@ public KeyAndValue getValue(final String key, final Function t return result.get(0); } - public List> get(final String key, final Function transformer) { AsadminResult result = exec("get", key); assertThat(result, asadminOK()); return Arrays.stream(result.getStdOut().split(System.lineSeparator())).map(KEYVAL_SPLITTER) - .filter(Objects::nonNull).map(kv -> new KeyAndValue<>(kv.getKey(), transformer.apply(kv.getValue()))) - .collect(Collectors.toList()); + .filter(Objects::nonNull).map(kv -> new KeyAndValue<>(kv.getKey(), transformer.apply(kv.getValue()))) + .collect(Collectors.toList()); } - /** - * Executes the command with arguments asynchronously with {@value #DEFAULT_TIMEOUT_MSEC} ms timeout. - * The command can be attached by the attach command. - * You should find the job id in the {@link AsadminResult#getStdOut()} as Job ID: [0-9]+ + * Executes the command with arguments asynchronously with + * {@value #DEFAULT_TIMEOUT_MSEC} ms timeout. The command can be attached by + * the attach command. You should find the job id in the + * {@link AsadminResult#getStdOut()} as Job ID: [0-9]+ * * @param args * @return {@link AsadminResult} never null. @@ -144,7 +167,8 @@ public DetachedTerseAsadminResult execDetached(final String... args) { } /** - * Executes the command with arguments synchronously with {@value #DEFAULT_TIMEOUT_MSEC} ms timeout. + * Executes the command with arguments synchronously with + * {@value #DEFAULT_TIMEOUT_MSEC} ms timeout. * * @param args * @return {@link AsadminResult} never null. @@ -152,8 +176,10 @@ public DetachedTerseAsadminResult execDetached(final String... args) { public AsadminResult exec(final String... args) { return exec(DEFAULT_TIMEOUT_MSEC, false, args); } + /** - * Executes the command with arguments synchronously with given timeout in millis. + * Executes the command with arguments synchronously with given timeout in + * millis. * * @param timeout timeout in millis * @param args command and arguments. @@ -163,24 +189,43 @@ public AsadminResult exec(final int timeout, final String... args) { return exec(timeout, false, args); } + private File getPasswordFile() { + try { + if (!additionalPasswords.isEmpty()) { + final Path tempPasswordFile = Files.createTempFile("pwd", "txt"); + Files.copy(adminPasswordFile.toPath(), tempPasswordFile, StandardCopyOption.REPLACE_EXISTING); + String additionalContent = additionalPasswords.entrySet().stream() + .map(entry -> entry.getKey() + "=" + entry.getValue()) + .collect(Collectors.joining("\n")); + Files.writeString(tempPasswordFile, "\n" + additionalContent, StandardOpenOption.APPEND); + return tempPasswordFile.toFile(); + } + return adminPasswordFile; + } catch (IOException e) { + throw new IllegalStateException("Could not create the temporary password file.", e); + } + + } + /** * Executes the command with arguments. * * @param timeout timeout in millis - * @param detachedAndTerse detached command is executed asynchronously, can be attached later by the attach command. + * @param detachedAndTerse detached command is executed asynchronously, can + * be attached later by the attach command. * @param args command and arguments. * @return {@link AsadminResult} never null. */ private AsadminResult exec(final int timeout, final boolean detachedAndTerse, final String... args) { final List parameters = Arrays.asList(args); LOG.log(Level.INFO, "exec(timeout={0}, detached={1}, args={2})", - new Object[] {timeout, detachedAndTerse, parameters}); + new Object[]{timeout, detachedAndTerse, parameters}); final List command = new ArrayList<>(); command.add(asadmin.getAbsolutePath()); command.add("--user"); command.add(adminUser); command.add("--passwordfile"); - command.add(adminPasswordFile.getAbsolutePath()); + command.add(getPasswordFile().getAbsolutePath()); if (detachedAndTerse) { command.add("--terse=true"); command.add("--detach"); @@ -230,4 +275,5 @@ private AsadminResult exec(final int timeout, final boolean detachedAndTerse, fi } return result; } + } diff --git a/appserver/tests/extras/command-logger/src/test/java/org/glassfish/main/test/extras/commandlogger/CommandLoggerITest.java b/appserver/tests/extras/command-logger/src/test/java/org/glassfish/main/test/extras/commandlogger/CommandLoggerITest.java index 191dbf63561..92984889e54 100644 --- a/appserver/tests/extras/command-logger/src/test/java/org/glassfish/main/test/extras/commandlogger/CommandLoggerITest.java +++ b/appserver/tests/extras/command-logger/src/test/java/org/glassfish/main/test/extras/commandlogger/CommandLoggerITest.java @@ -14,6 +14,8 @@ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 */ package org.glassfish.main.test.extras.commandlogger; +import com.google.common.collect.Streams; + import static org.glassfish.main.itest.tools.GlassFishTestEnvironment.getAsadmin; import static org.glassfish.main.itest.tools.asadmin.AsadminResultMatcher.asadminOK; import static org.hamcrest.CoreMatchers.allOf; @@ -24,12 +26,18 @@ import static org.hamcrest.Matchers.not; import java.io.IOException; +import java.util.Arrays; import java.util.List; + import org.glassfish.main.itest.tools.asadmin.Asadmin; import org.glassfish.main.itest.tools.asadmin.CollectLogFiles; +import org.hamcrest.CoreMatchers; +import org.hamcrest.Matcher; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; +import static java.util.stream.Stream.of; + /** * * @author Ondro Mihalyi @@ -58,7 +66,11 @@ public void testLogWriteCommands(String logMode, boolean logWriteOp, boolean log clearLogFile(); // execute some write command, it doesn't have to complete successfully - ASADMIN.exec("delete-system-property", "X"); + ASADMIN.exec("delete-system-property", "propertyX"); + ASADMIN.withPassword("AS_ADMIN_ALIASPASSWORD", "secretPassword") + .exec("create-password-alias", "mytestalias"); + ASADMIN.resetPasswords(); + ASADMIN.exec("delete-password-alias", "mytestalias"); // execute some read command ASADMIN.exec("list-applications", "--long"); // exxecute some internal command @@ -68,14 +80,17 @@ public void testLogWriteCommands(String logMode, boolean logWriteOp, boolean log .collect() .getServerLogLines(); + assertCommandNotLogged(lines, "secretPassword"); if (logWriteOp) { - assertCommandLogged(lines, "admin", "delete-system-property X"); + assertCommandLogged(lines, "admin", "delete-system-property", "propertyX"); + assertCommandLogged(lines, "admin", "create-password-alias", "mytestalias"); } else { assertCommandNotLogged(lines, "delete-system-property"); + assertCommandNotLogged(lines, "create-password-alias"); } if (logReadOp) { - assertCommandLogged(lines, "admin", "list-applications --long"); + assertCommandLogged(lines, "admin", "list-applications", "--long"); } else { assertCommandNotLogged(lines, "list-applications"); } @@ -96,11 +111,13 @@ private void assertCommandNotLogged(final List lines, String command) { assertThat("log", lines, everyItem(not(containsString(command)))); } - private void assertCommandLogged(final List lines, String user, String fullCommand) { - assertThat("server.log", lines, hasItem(allOf(containsString(user), - containsString(fullCommand) - ) - )); + private void assertCommandLogged(final List lines, String user, String... commandParts) { + final Matcher[] containsAllStrings = Streams.concat(of(containsString(user)), + Arrays.stream(commandParts) + .map(CoreMatchers::containsString)) + .toArray(Matcher[]::new); + + assertThat("server.log", lines, hasItem(allOf(containsAllStrings))); } } diff --git a/nucleus/common/common-util/src/main/java/org/glassfish/common/util/Constants.java b/nucleus/common/common-util/src/main/java/org/glassfish/common/util/Constants.java index 93ec5251c2f..d6145a0f5ce 100644 --- a/nucleus/common/common-util/src/main/java/org/glassfish/common/util/Constants.java +++ b/nucleus/common/common-util/src/main/java/org/glassfish/common/util/Constants.java @@ -16,6 +16,8 @@ package org.glassfish.common.util; +import java.util.Set; + /** * These are constants that can be used throughout the server * @@ -37,4 +39,17 @@ public class Constants { */ public final static int IMPORTANT_RUN_LEVEL_SERVICE = 50; + /* + * This is a list of attribute names that hold passwords for various admin commands. We need to mask their value during logging. + */ + public final static Set PASSWORD_ATTRIBUTE_NAMES = Set.of( + "sshpassword", /* create-node-ssh , setup-ssh , update-node, update-node-ssh */ + "dbpassword", /* jms-availability-service */ + "password", /* change-admin-password */ + "newpassword", /* change-admin-password */ + "jmsdbpassword", /* configure-jms-cluster */ + "mappedpassword", /* create-connector-security-map, update-connector-security-map */ + "userpassword", /* create-file-user , update-file-user */ + "aliaspassword" /* create-password-alias , update-password-alias */ + ); } diff --git a/nucleus/common/glassfish-api/pom.xml b/nucleus/common/glassfish-api/pom.xml index 1ea885206f4..debeab6093b 100644 --- a/nucleus/common/glassfish-api/pom.xml +++ b/nucleus/common/glassfish-api/pom.xml @@ -87,6 +87,10 @@ org.junit.jupiter junit-jupiter-engine + + org.junit.jupiter + junit-jupiter-params + org.hamcrest hamcrest diff --git a/nucleus/common/glassfish-api/src/main/java/org/glassfish/api/admin/ParameterMap.java b/nucleus/common/glassfish-api/src/main/java/org/glassfish/api/admin/ParameterMap.java index 551ba06fc8b..241c70a3c4d 100644 --- a/nucleus/common/glassfish-api/src/main/java/org/glassfish/api/admin/ParameterMap.java +++ b/nucleus/common/glassfish-api/src/main/java/org/glassfish/api/admin/ParameterMap.java @@ -16,6 +16,9 @@ package org.glassfish.api.admin; +import java.util.List; +import java.util.Set; + import org.jvnet.hk2.component.MultiMap; /** @@ -51,4 +54,22 @@ public ParameterMap insert(String k, String v) { add(k, v); return this; } + + /** + * Get a copy of this map with values of secret parameters changed into *******" to hide their values. + * Parameters which are not secret, remain without any change. + * + * @param secretParameters A set of parameter names which are considered secret. Not null. + * @return A copy of this map with masked values + */ + public ParameterMap getMaskedMap(Set secretParameters) { + final ParameterMap maskedParameters = new ParameterMap(this); + maskedParameters.entrySet().forEach(entry -> { + if (secretParameters.contains(entry.getKey())) { + entry.setValue(List.of("*******")); + } + }); + return maskedParameters; + } + } diff --git a/nucleus/common/glassfish-api/src/test/java/org/glassfish/api/admin/ParameterMapTest.java b/nucleus/common/glassfish-api/src/test/java/org/glassfish/api/admin/ParameterMapTest.java new file mode 100644 index 00000000000..9e1177d77be --- /dev/null +++ b/nucleus/common/glassfish-api/src/test/java/org/glassfish/api/admin/ParameterMapTest.java @@ -0,0 +1,87 @@ +/* + * Copyright (c) 2024 Contributors to the Eclipse Foundation + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0, which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the + * Eclipse Public License v. 2.0 are satisfied: GNU General Public License, + * version 2 with the GNU Classpath Exception, which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + */ +package org.glassfish.api.admin; + +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Stream; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.junit.jupiter.api.Assertions.assertIterableEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * + * @author Ondro Mihalyi + */ +public class ParameterMapTest { + + @ParameterizedTest + @MethodSource("valuesForMaskValues") + void maskValues(final Map mapWithSecrets, final Set secretKeys) { + ParameterMap parameterMap = new ParameterMap(); + mapWithSecrets.forEach(parameterMap::add); + final ParameterMap maskedMap = parameterMap.getMaskedMap(secretKeys); + + assertIterableEquals(mapWithSecrets.keySet(), maskedMap.keySet(), "Masked map should contian the same keys"); + maskedMap.entrySet().forEach(entry -> { + assertTrue(!keyIsSecret(entry, secretKeys) || valueIsMasked(entry), + () -> "Entry is either not secret or should contain masked value. Value is: " + entry.getValue()); + }); + } + + private static Stream valuesForMaskValues() { + return Stream.of( + Arguments.of( + Map.of("secretKey", "secret"), + Set.of("secretKey") + ), + Arguments.of( + Map.of(), + Set.of("secretKey") + ), + Arguments.of( + Map.of("secretKey", "secret", + "notSecretKey", "hello"), + Set.of("secretKey") + ), + Arguments.of( + Map.of("anotherNotSecretKey", "hello", + "secretKey", "secret", + "anotherSecretKey", "secret", + "notSecretKey", "hello"), + Set.of("secretKey", "anotherSecretKey") + ), + Arguments.of( + Map.of("anotherNotSecretKey", "hello", + "notSecretKey", "hello"), + Set.of("") + ) + ); + } + + static boolean keyIsSecret(Map.Entry> entry, Set secretKeys) { + return secretKeys.contains(entry.getKey()); + } + + static boolean valueIsMasked(Map.Entry> entry) { + return entry.getValue().size() == 1 && entry.getValue().get(0).startsWith("*"); + } +} diff --git a/nucleus/common/internal-api/src/main/java/org/glassfish/internal/api/events/CommandInvokedEvent.java b/nucleus/common/internal-api/src/main/java/org/glassfish/internal/api/events/CommandInvokedEvent.java index 6836c55a179..d7423794bb8 100644 --- a/nucleus/common/internal-api/src/main/java/org/glassfish/internal/api/events/CommandInvokedEvent.java +++ b/nucleus/common/internal-api/src/main/java/org/glassfish/internal/api/events/CommandInvokedEvent.java @@ -69,7 +69,11 @@ public class CommandInvokedEvent { public CommandInvokedEvent(String commandName, ParameterMap parameters, Subject subject) { this.commandName = commandName; - this.parameters = parameters; + if (parameters != null) { + this.parameters = parameters; + } else { + this.parameters = new ParameterMap(); + } this.subject = subject; } @@ -77,10 +81,16 @@ public CommandInvokedEvent(String commandName, ParameterMap parameters, Subject private final ParameterMap parameters; private final Subject subject; + /** + * @return Name of the executed command + */ public String getCommandName() { return commandName; } + /** + * @return Parameters used with the command. If not parameters, the result is an empty map. Never null. + */ public ParameterMap getParameters() { return parameters; } diff --git a/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/admin/AdminAdapter.java b/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/admin/AdminAdapter.java index 63121312916..26388ab8d14 100644 --- a/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/admin/AdminAdapter.java +++ b/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/admin/AdminAdapter.java @@ -536,7 +536,7 @@ private ActionReport doCommand(String requestURI, Request req, ActionReport repo inv.parameters(parameters).inbound(inboundPayload).outbound(outboundPayload).execute(); try { // note it has become extraordinarily difficult to change the reporter! - CommandRunnerImpl.ExecutionContext inv2 = (CommandRunnerImpl.ExecutionContext) inv; + CommandRunnerExecutionContext inv2 = (CommandRunnerExecutionContext) inv; report = inv2.report(); } catch(Exception e) { diff --git a/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/admin/CommandRunnerExecutionContext.java b/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/admin/CommandRunnerExecutionContext.java new file mode 100644 index 00000000000..4cdda2c82ce --- /dev/null +++ b/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/admin/CommandRunnerExecutionContext.java @@ -0,0 +1,259 @@ +/* + * Copyright (c) 2024 Contributors to the Eclipse Foundation + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0, which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the + * Eclipse Public License v. 2.0 are satisfied: GNU General Public License, + * version 2 with the GNU Classpath Exception, which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + */ +package com.sun.enterprise.v3.admin; + +import com.sun.enterprise.admin.event.AdminCommandEventBrokerImpl; +import com.sun.enterprise.util.AnnotationUtil; + +import java.security.AccessController; +import java.security.PrivilegedAction; +import java.util.ArrayList; +import java.util.List; +import java.util.logging.Logger; + +import javax.security.auth.Subject; + +import org.glassfish.api.ActionReport; +import org.glassfish.api.admin.AdminCommand; +import org.glassfish.api.admin.AdminCommandContext; +import org.glassfish.api.admin.AdminCommandEventBroker; +import org.glassfish.api.admin.AdminCommandState; +import org.glassfish.api.admin.CommandParameters; +import org.glassfish.api.admin.CommandRunner; +import org.glassfish.api.admin.CommandSupport; +import org.glassfish.api.admin.Job; +import org.glassfish.api.admin.JobCreator; +import org.glassfish.api.admin.JobManager; +import org.glassfish.api.admin.ManagedJob; +import org.glassfish.api.admin.ParameterMap; +import org.glassfish.api.admin.Payload; +import org.glassfish.api.admin.ProgressStatus; + +/* + * Some private classes used in the implementation of CommandRunner. + */ +/** + * ExecutionContext is a CommandInvocation, which + * defines a command excecution context like the requested + * name of the command to execute, the parameters of the command, etc. + */ +class CommandRunnerExecutionContext implements CommandRunner.CommandInvocation { + + private final CommandRunnerImpl commandRunner; + private static final Logger LOGGER = Logger.getLogger(CommandRunnerExecutionContext.class.getName()); + + CommandRunnerExecutionContext(String scope, String name, ActionReport report, Subject subject, boolean isNotify, final CommandRunnerImpl commandRunner) { + this.commandRunner = commandRunner; + this.scope = scope; + this.name = name; + this.report = report; + this.subject = subject; + this.isNotify = isNotify; + } + + private class NameListerPair { + + private final String nameRegexp; + private final AdminCommandEventBroker.AdminCommandListener listener; + + public NameListerPair(String nameRegexp, AdminCommandEventBroker.AdminCommandListener listener) { + this.nameRegexp = nameRegexp; + this.listener = listener; + } + } + protected String scope; + protected String name; + protected ActionReport report; + protected ParameterMap params; + protected CommandParameters paramObject; + protected Payload.Inbound inbound; + protected Payload.Outbound outbound; + protected Subject subject; + protected ProgressStatus progressStatusChild; + protected boolean isManagedJob; + protected boolean isNotify; + private final List nameListerPairs = new ArrayList<>(); + + @Override + public CommandRunner.CommandInvocation parameters(CommandParameters paramObject) { + this.paramObject = paramObject; + return this; + } + + @Override + public CommandRunner.CommandInvocation parameters(ParameterMap params) { + this.params = params; + return this; + } + + @Override + public CommandRunner.CommandInvocation inbound(Payload.Inbound inbound) { + this.inbound = inbound; + return this; + } + + @Override + public CommandRunner.CommandInvocation outbound(Payload.Outbound outbound) { + this.outbound = outbound; + return this; + } + + @Override + public CommandRunner.CommandInvocation listener(String nameRegexp, AdminCommandEventBroker.AdminCommandListener listener) { + nameListerPairs.add(new NameListerPair(nameRegexp, listener)); + return this; + } + + @Override + public CommandRunner.CommandInvocation progressStatusChild(ProgressStatus ps) { + this.progressStatusChild = ps; + return this; + } + + @Override + public CommandRunner.CommandInvocation managedJob() { + this.isManagedJob = true; + return this; + } + + @Override + public void execute() { + execute(null); + } + + ParameterMap parameters() { + return params; + } + + CommandParameters typedParams() { + return paramObject; + } + + String name() { + return name; + } + + private String scope() { + return scope; + } + + @Override + public ActionReport report() { + return report; + } + + void setReport(ActionReport ar) { + report = ar; + } + + Payload.Inbound inboundPayload() { + return inbound; + } + + Payload.Outbound outboundPayload() { + return outbound; + } + + void executeFromCheckpoint(JobManager.Checkpoint checkpoint, boolean revert, AdminCommandEventBroker eventBroker) { + Job job = checkpoint.getJob(); + if (subject == null) { + subject = checkpoint.getContext().getSubject(); + } + parameters(job.getParameters()); + AdminCommandContext context = checkpoint.getContext(); + this.report = context.getActionReport(); + this.inbound = context.getInboundPayload(); + this.outbound = context.getOutboundPayload(); + this.scope = job.getScope(); + this.name = job.getName(); + if (eventBroker == null) { + eventBroker = job.getEventBroker() == null ? new AdminCommandEventBrokerImpl() : job.getEventBroker(); + } + ((AdminCommandInstanceImpl) job).setEventBroker(eventBroker); + ((AdminCommandInstanceImpl) job).setState(revert ? AdminCommandState.State.REVERTING : AdminCommandState.State.RUNNING_RETRYABLE); + JobManager jobManager = commandRunner.serviceLocator.getService(JobManagerService.class); + jobManager.registerJob(job); + //command + AdminCommand command = checkpoint.getCommand(); + if (command == null) { + command = commandRunner.getCommand(job.getScope(), job.getName(), report(), LOGGER); + if (command == null) { + return; + } + } + //execute + commandRunner.doCommand(this, command, subject, job); + job.complete(report(), outboundPayload()); + if (progressStatusChild != null) { + progressStatusChild.complete(); + } + CommandSupport.done(commandRunner.serviceLocator, command, job); + } + + @Override + public void execute(AdminCommand command) { + if (command == null) { + command = commandRunner.getCommand(scope(), name(), report(), LOGGER); + if (command == null) { + return; + } + } + /* + * The caller should have set the subject explicitly. In case + * it didn't, try setting it from the current access controller context + * since the command framework will have set that before invoking + * the original command's execute method. + */ + if (subject == null) { + subject = AccessController.doPrivileged(new PrivilegedAction() { + @Override + public Subject run() { + return Subject.getSubject(AccessController.getContext()); + } + }); + } + if (!isManagedJob) { + isManagedJob = AnnotationUtil.presentTransitive(ManagedJob.class, command.getClass()); + } + JobCreator jobCreator = null; + JobManager jobManager = null; + jobCreator = commandRunner.serviceLocator.getService(JobCreator.class, scope + "job-creator"); + jobManager = commandRunner.serviceLocator.getService(JobManagerService.class); + if (jobCreator == null) { + jobCreator = commandRunner.serviceLocator.getService(JobCreatorService.class); + } + Job job = null; + if (isManagedJob) { + job = jobCreator.createJob(jobManager.getNewId(), scope(), name(), subject, isManagedJob, parameters()); + } else { + job = jobCreator.createJob(null, scope(), name(), subject, isManagedJob, parameters()); + } + //Register the brokers else the detach functionality will not work + for (NameListerPair nameListerPair : nameListerPairs) { + job.getEventBroker().registerListener(nameListerPair.nameRegexp, nameListerPair.listener); + } + if (isManagedJob) { + jobManager.registerJob(job); + } + commandRunner.doCommand(this, command, subject, job); + job.complete(report(), outboundPayload()); + if (progressStatusChild != null) { + progressStatusChild.complete(); + } + CommandSupport.done(commandRunner.serviceLocator, command, job, isNotify); + } + +} diff --git a/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/admin/CommandRunnerImpl.java b/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/admin/CommandRunnerImpl.java index 3cd27bb82ff..479532265ae 100644 --- a/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/admin/CommandRunnerImpl.java +++ b/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/admin/CommandRunnerImpl.java @@ -17,7 +17,6 @@ package com.sun.enterprise.v3.admin; -import com.sun.enterprise.admin.event.AdminCommandEventBrokerImpl; import com.sun.enterprise.admin.util.CachedCommandModel; import com.sun.enterprise.admin.util.ClusterOperationUtil; import com.sun.enterprise.admin.util.CommandSecurityChecker; @@ -26,7 +25,6 @@ import com.sun.enterprise.config.serverbeans.Domain; import com.sun.enterprise.universal.collections.ManifestUtils; import com.sun.enterprise.universal.glassfish.AdminCommandResponse; -import com.sun.enterprise.util.AnnotationUtil; import com.sun.enterprise.util.LocalStringManagerImpl; import com.sun.enterprise.util.StringUtils; import com.sun.enterprise.v3.common.XMLContentActionReporter; @@ -63,6 +61,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.Properties; import java.util.Set; import java.util.concurrent.locks.Lock; @@ -78,7 +77,6 @@ import org.glassfish.api.admin.AdminCommandContext; import org.glassfish.api.admin.AdminCommandContextImpl; import org.glassfish.api.admin.AdminCommandEventBroker; -import org.glassfish.api.admin.AdminCommandEventBroker.AdminCommandListener; import org.glassfish.api.admin.AdminCommandLock; import org.glassfish.api.admin.AdminCommandLockException; import org.glassfish.api.admin.AdminCommandLockTimeoutException; @@ -91,13 +89,10 @@ import org.glassfish.api.admin.CommandSupport; import org.glassfish.api.admin.FailurePolicy; import org.glassfish.api.admin.Job; -import org.glassfish.api.admin.JobCreator; import org.glassfish.api.admin.JobManager; -import org.glassfish.api.admin.ManagedJob; import org.glassfish.api.admin.ParameterMap; import org.glassfish.api.admin.Payload; import org.glassfish.api.admin.ProcessEnvironment; -import org.glassfish.api.admin.ProgressStatus; import org.glassfish.api.admin.RuntimeType; import org.glassfish.api.admin.ServerEnvironment; import org.glassfish.api.admin.Supplemental; @@ -129,6 +124,7 @@ import static com.sun.enterprise.util.Utility.isEmpty; import static java.util.logging.Level.SEVERE; +import static org.glassfish.common.util.Constants.PASSWORD_ATTRIBUTE_NAMES; /** * Encapsulates the logic needed to execute a server-side command (for example, @@ -153,7 +149,7 @@ public class CommandRunnerImpl implements CommandRunner { private static final InjectionManager injectionMgr = new InjectionManager(); @Inject - private ServiceLocator serviceLocator; + ServiceLocator serviceLocator; @Inject private ServerContext serverContext; @@ -392,7 +388,7 @@ public CommandInvocation getCommandInvocation(String name, ActionReport report, @Override public CommandInvocation getCommandInvocation(String scope, String name, ActionReport report, Subject subject, boolean isNotify) { - return new ExecutionContext(scope, name, report, subject, isNotify); + return new CommandRunnerExecutionContext(scope, name, report, subject, isNotify, this); } @@ -1131,7 +1127,7 @@ private static CommandModel getModel(AdminCommand command) { /** * Called from ExecutionContext.execute. */ - private void doCommand(ExecutionContext inv, AdminCommand command, + void doCommand(CommandRunnerExecutionContext inv, AdminCommand command, final Subject subject, final Job job) { publishCommandInvokedEvent(inv, subject); @@ -1635,238 +1631,20 @@ private Map buildEnvMap(final ParameterMap params) { } public void executeFromCheckpoint(JobManager.Checkpoint checkpoint, boolean revert, AdminCommandEventBroker eventBroker) { - ExecutionContext ec = new ExecutionContext(null, null, null, null,false); + CommandRunnerExecutionContext ec = new CommandRunnerExecutionContext(null, null, null, null,false, this); ec.executeFromCheckpoint(checkpoint, revert, eventBroker); } - private void publishCommandInvokedEvent(ExecutionContext invocation, Subject subject) { + private void publishCommandInvokedEvent(CommandRunnerExecutionContext invocation, Subject subject) { + final ParameterMap parameters = Objects.requireNonNullElseGet(invocation.parameters(), ParameterMap::new); + final ParameterMap maskedMap = parameters.getMaskedMap(PASSWORD_ATTRIBUTE_NAMES); final CommandInvokedEvent event = new CommandInvokedEvent( - invocation.name(), - invocation.parameters(), + invocation.name(), maskedMap, subject); eventService.getCommandInvokedTopic() .publish(event); } - /* - * Some private classes used in the implementation of CommandRunner. - */ - /** - * ExecutionContext is a CommandInvocation, which - * defines a command excecution context like the requested - * name of the command to execute, the parameters of the command, etc. - */ - class ExecutionContext implements CommandInvocation { - - private class NameListerPair { - - private final String nameRegexp; - private final AdminCommandEventBroker.AdminCommandListener listener; - - public NameListerPair(String nameRegexp, AdminCommandListener listener) { - this.nameRegexp = nameRegexp; - this.listener = listener; - } - - } - - protected String scope; - protected String name; - protected ActionReport report; - protected ParameterMap params; - protected CommandParameters paramObject; - protected Payload.Inbound inbound; - protected Payload.Outbound outbound; - protected Subject subject; - protected ProgressStatus progressStatusChild; - protected boolean isManagedJob; - protected boolean isNotify; - private final List nameListerPairs = new ArrayList<>(); - - private ExecutionContext(String scope, String name, ActionReport report, Subject subject, boolean isNotify) { - this.scope = scope; - this.name = name; - this.report = report; - this.subject = subject; - this.isNotify = isNotify; - } - - @Override - public CommandInvocation parameters(CommandParameters paramObject) { - this.paramObject = paramObject; - return this; - } - - @Override - public CommandInvocation parameters(ParameterMap params) { - this.params = params; - return this; - } - - @Override - public CommandInvocation inbound(Payload.Inbound inbound) { - this.inbound = inbound; - return this; - } - - @Override - public CommandInvocation outbound(Payload.Outbound outbound) { - this.outbound = outbound; - return this; - } - - @Override - public CommandInvocation listener(String nameRegexp, AdminCommandEventBroker.AdminCommandListener listener) { - nameListerPairs.add(new NameListerPair(nameRegexp, listener)); - return this; - } - - @Override - public CommandInvocation progressStatusChild(ProgressStatus ps) { - this.progressStatusChild = ps; - return this; - } - - @Override - public CommandInvocation managedJob() { - this.isManagedJob = true; - return this; - } - - @Override - public void execute() { - execute(null); - } - - private ParameterMap parameters() { - return params; - } - - private CommandParameters typedParams() { - return paramObject; - } - - private String name() { - return name; - } - - private String scope() { - return scope; - } - - @Override - public ActionReport report() { - return report; - } - - private void setReport(ActionReport ar) { - report = ar; - } - - private Payload.Inbound inboundPayload() { - return inbound; - } - - private Payload.Outbound outboundPayload() { - return outbound; - } - - private void executeFromCheckpoint(JobManager.Checkpoint checkpoint, boolean revert, AdminCommandEventBroker eventBroker) { - Job job = checkpoint.getJob(); - if (subject == null) { - subject = checkpoint.getContext().getSubject(); - } - parameters(job.getParameters()); - AdminCommandContext context = checkpoint.getContext(); - this.report = context.getActionReport(); - this.inbound = context.getInboundPayload(); - this.outbound = context.getOutboundPayload(); - this.scope = job.getScope(); - this.name = job.getName(); - if (eventBroker == null) { - eventBroker = job.getEventBroker() == null ? new AdminCommandEventBrokerImpl() : job.getEventBroker(); - } - ((AdminCommandInstanceImpl) job).setEventBroker(eventBroker); - ((AdminCommandInstanceImpl) job).setState(revert ? AdminCommandState.State.REVERTING : AdminCommandState.State.RUNNING_RETRYABLE); - JobManager jobManager = serviceLocator.getService(JobManagerService.class); - jobManager.registerJob(job); - //command - AdminCommand command = checkpoint.getCommand(); - if (command == null) { - command = getCommand(job.getScope(), job.getName(), report(), logger); - if (command == null) { - return; - } - } - //execute - CommandRunnerImpl.this.doCommand(this, command, subject, job); - job.complete(report(), outboundPayload()); - if (progressStatusChild != null) { - progressStatusChild.complete(); - } - CommandSupport.done(serviceLocator, command, job); - } - - @Override - public void execute(AdminCommand command) { - if (command == null) { - command = getCommand(scope(), name(), report(), logger); - if (command == null) { - return; - } - } - /* - * The caller should have set the subject explicitly. In case - * it didn't, try setting it from the current access controller context - * since the command framework will have set that before invoking - * the original command's execute method. - */ - if (subject == null) { - subject = AccessController.doPrivileged(new PrivilegedAction() { - @Override - public Subject run() { - return Subject.getSubject(AccessController.getContext()); - } - }); - } - - if(!isManagedJob) { - isManagedJob = AnnotationUtil.presentTransitive(ManagedJob.class, command.getClass()); - } - JobCreator jobCreator = null; - JobManager jobManager = null; - - jobCreator = serviceLocator.getService(JobCreator.class,scope+"job-creator"); - jobManager = serviceLocator.getService(JobManagerService.class); - - if (jobCreator == null ) { - jobCreator = serviceLocator.getService(JobCreatorService.class); - - } - - Job job = null; - if (isManagedJob) { - job = jobCreator.createJob(jobManager.getNewId(), scope(), name(), subject, isManagedJob, parameters()); - } else { - job = jobCreator.createJob(null, scope(), name(), subject, isManagedJob, parameters()); - } - - //Register the brokers else the detach functionality will not work - for (NameListerPair nameListerPair : nameListerPairs) { - job.getEventBroker().registerListener(nameListerPair.nameRegexp, nameListerPair.listener); - } - - if (isManagedJob) { - jobManager.registerJob(job); - } - CommandRunnerImpl.this.doCommand(this, command, subject, job); - job.complete(report(), outboundPayload()); - if (progressStatusChild != null) { - progressStatusChild.complete(); - } - CommandSupport.done(serviceLocator, command, job, isNotify); - } - } /** * An InjectionResolver that uses an Object as the source of