Mask secrets in CommandInvokedEvent and Command logger

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<String, Object> attrs) {
 
         for (Map.Entry<String, Object> 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<String> 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 */);
 }

appserver/itest-tools/src/main/java/org/glassfish/main/itest/tools/GlassFishTestEnvironment.java

@@ -71,6 +71,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);
@@ -86,9 +87,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());
     }
 
@@ -253,7 +256,6 @@ public static void createFileUser(String realmName, String user, String password
         }
     }
 
-
     /**
      * This will delete the jobs.xml file
      */

appserver/itest-tools/src/main/java/org/glassfish/main/itest/tools/asadmin/Asadmin.java

@@ -13,14 +13,18 @@
  *
  * 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;
 import com.sun.enterprise.universal.process.ProcessManagerException;
 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<String, String> environment = new HashMap<>();
-
+    private final Map<String, String> 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 <code>AS_ADMIN_PASSWORD=...</code>
+     * @param adminPasswordFile - a file containing admin's password set as
+     * <code>AS_ADMIN_PASSWORD=...</code>
      */
     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 <code>AS_ADMIN_PASSWORD=...</code>
+     * @param adminPasswordFile - a file containing admin's password set as
+     * <code>AS_ADMIN_PASSWORD=...</code>
      * @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 <T> KeyAndValue<T> getValue(final String key, final Function<String, T> transformer) {
         List<KeyAndValue<T>> result = get(key, transformer);
         if (result.isEmpty()) {
@@ -121,20 +145,19 @@ public <T> KeyAndValue<T> getValue(final String key, final Function<String, T> t
         return result.get(0);
     }
 
-
     public <T> List<KeyAndValue<T>> get(final String key, final Function<String, T> 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 <code>Job ID: [0-9]+</code>
+     * 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 <code>Job ID: [0-9]+</code>
      *
      * @param args
      * @return {@link AsadminResult} never null.
@@ -144,16 +167,19 @@ 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.
      */
     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<String> 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<String> 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;
     }
+
 }

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<String> lines, String command) {
         assertThat("log", lines, everyItem(not(containsString(command))));
     }
 
-    private void assertCommandLogged(final List<String> lines, String user, String fullCommand) {
-        assertThat("server.log", lines, hasItem(allOf(containsString(user),
-                containsString(fullCommand)
-        )
-        ));
+    private void assertCommandLogged(final List<String> 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)));
     }
 
 }

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<String> 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 */
+    );
 }