geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kl...@apache.org
Subject geode git commit: GEODE-2119: gfsh user and password visible in clear text
Date Tue, 20 Dec 2016 22:23:34 GMT
Repository: geode
Updated Branches:
  refs/heads/feature/GEODE-2119 [created] 3fad7a38b


GEODE-2119: gfsh user and password visible in clear text

Made changes to suppress the password from being displayed in the log files and on the console.

* this closes #311 [klund]


Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/3fad7a38
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/3fad7a38
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/3fad7a38

Branch: refs/heads/feature/GEODE-2119
Commit: 3fad7a38bfa90fbf9f931d1ada01da2f00e83c31
Parents: 4067ddc
Author: Kevin J. Duling <kduling@pivotal.io>
Authored: Wed Dec 7 13:23:38 2016 -0800
Committer: Kirk Lund <klund@apache.org>
Committed: Tue Dec 20 10:00:53 2016 -0800

----------------------------------------------------------------------
 .../geode/distributed/AbstractLauncher.java     | 101 ++++++------
 .../java/org/apache/geode/internal/Banner.java  |  44 ++----
 .../geode/internal/util/ArgumentRedactor.java   | 154 +++++++++++++++++++
 .../management/internal/cli/shell/Gfsh.java     |  95 ++++++------
 .../util/ArgumentRedactorJUnitTest.java         | 125 +++++++++++++++
 5 files changed, 398 insertions(+), 121 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/3fad7a38/geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java b/geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java
index 0e40785..d5626dd 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java
@@ -15,6 +15,8 @@
 
 package org.apache.geode.distributed;
 
+import static org.apache.geode.distributed.ConfigurationProperties.NAME;
+
 import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.distributed.internal.InternalDistributedSystem;
 import org.apache.geode.distributed.internal.unsafe.RegisterSignalHandlerSupport;
@@ -28,28 +30,37 @@ import org.apache.geode.internal.lang.StringUtils;
 import org.apache.geode.internal.lang.SystemUtils;
 import org.apache.geode.internal.process.PidUnavailableException;
 import org.apache.geode.internal.process.ProcessUtils;
+import org.apache.geode.internal.util.ArgumentRedactor;
 import org.apache.geode.internal.util.SunAPINotFoundException;
 import org.apache.geode.management.internal.cli.json.GfJsonObject;
 
-import java.io.*;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
 import java.net.BindException;
 import java.net.InetAddress;
 import java.net.URL;
 import java.sql.Timestamp;
 import java.text.SimpleDateFormat;
-import java.util.*;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.logging.FileHandler;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-import static org.apache.geode.distributed.ConfigurationProperties.*;
-
 /**
  * The AbstractLauncher class is a base class for implementing various launchers to construct
and
  * run different GemFire processes, like Cache Servers, Locators, Managers, HTTP servers
and so on.
- * 
+ *
  * @see java.lang.Comparable
  * @see java.lang.Runnable
  * @see org.apache.geode.lang.Identifiable
@@ -85,7 +96,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
   protected final transient AtomicBoolean running = new AtomicBoolean(false);
 
   protected Logger logger = Logger.getLogger(getClass().getName()); // TODO:KIRK: does this
need
-                                                                    // log4j2?
+  // log4j2?
 
   public AbstractLauncher() {
     try {
@@ -135,7 +146,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
    * Properties. The property is considered "set" if the String value of the property is
not
    * non-null, non-empty and non-blank. Therefore, the Properties may "have" a property with
name,
    * but having no value as determined by this method.
-   * 
+   *
    * @param properties the Properties used in determining whether the given property is set.
    * @param propertyName a String indicating the name of the property to check if set.
    * @return a boolean indicating whether the specified property with name has been given
a value in
@@ -148,7 +159,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
   /**
    * Loads the GemFire properties at the specified URL.
-   * 
+   *
    * @param url the URL to the gemfire.properties to load.
    * @return a Properties instance populated with the gemfire.properties.
    * @see java.net.URL
@@ -188,7 +199,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
   /**
    * This method attempts to make a best effort determination for whether the Attach API
classes are
    * on the classpath.
-   * 
+   *
    * @param t the Throwable being evaluated for missing Attach API classes.
    * @return a boolean indicating whether the Exception or Error condition is a result of
the Attach
    *         API missing from the classpath.
@@ -223,7 +234,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
   /**
    * Determines if the Attach API is on the classpath.
-   * 
+   *
    * @return a boolean value indicating if the Attach API is on the classpath.
    */
   boolean isAttachAPIOnClasspath() {
@@ -233,7 +244,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
   /**
    * Determines whether the Locator launcher is in debug mode.
-   * 
+   *
    * @return a boolean to indicate whether the Locator launcher is in debug mode.
    * @see #setDebug(boolean)
    */
@@ -244,7 +255,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
   /**
    * Sets the debug mode of the GemFire launcher class. This mutable property of the launcher
    * enables the user to turn the debug mode on and off programmatically.
-   * 
+   *
    * @param debug a boolean used to enable or disable debug mode.
    * @see #isDebugging()
    */
@@ -254,7 +265,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
   /**
    * Determines whether the Locator referenced by this launcher is running.
-   * 
+   *
    * @return a boolean valued indicating if the referenced Locator is running.
    */
   public boolean isRunning() {
@@ -319,7 +330,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
   /**
    * Gets the name of the log file used to log information about this GemFire service.
-   * 
+   *
    * @return a String value indicating the name of this GemFire service's log file.
    */
   public abstract String getLogFileName();
@@ -328,7 +339,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
    * Gets the name or ID of the member in the GemFire distributed system. This method prefers
name
    * if specified, otherwise the ID is returned. If name was not specified to the Builder
that
    * created this Launcher and this call is not in-process, then null is returned.
-   * 
+   *
    * @return a String value indicating the member's name if specified, otherwise the member's
ID is
    *         returned if this call is made in-process, or finally, null is returned if neither
name
    *         name was specified or the call is out-of-process.
@@ -367,21 +378,21 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
   /**
    * Gets the user-specified process ID (PID) of the running GemFire service that AbstractLauncher
    * implementations can use to determine status, or stop the service.
-   * 
+   *
    * @return an Integer value indicating the process ID (PID) of the running GemFire service.
    */
   public abstract Integer getPid();
 
   /**
    * Gets the name of the GemFire service.
-   * 
+   *
    * @return a String indicating the name of the GemFire service.
    */
   public abstract String getServiceName();
 
   /**
    * Gets the working directory pathname in which the process will be run.
-   * 
+   *
    * @return a String value indicating the pathname of the Server's working directory.
    */
   public String getWorkingDirectory() {
@@ -391,7 +402,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
   /**
    * Prints the specified debug message to standard err, replacing any placeholder values
with the
    * specified arguments on output, if debugging has been enabled.
-   * 
+   *
    * @param message the String value written to standard err.
    * @param args an Object array containing arguments to replace the placeholder values in
the
    *        message.
@@ -412,7 +423,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
   /**
    * Prints the stack trace of the given Throwable to standard err if debugging has been
enabled.
-   * 
+   *
    * @param t the Throwable who's stack trace is printed to standard err.
    * @see java.lang.System#err
    * @see #isDebugging()
@@ -427,7 +438,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
   /**
    * Prints the specified informational message to standard err, replacing any placeholder
values
    * with the specified arguments on output.
-   * 
+   *
    * @param message the String value written to standard err.
    * @param args an Object array containing arguments to replace the placeholder values in
the
    *        message.
@@ -445,7 +456,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
   /**
    * Redirects the standard out and standard err to the configured log file as specified
in the
    * GemFire distributed system properties.
-   * 
+   *
    * @param distributedSystem the GemFire model for a distributed system.
    * @throws IOException if the standard out and err redirection was unsuccessful.
    */
@@ -458,7 +469,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
   /**
    * Gets the version of GemFire currently running.
-   * 
+   *
    * @return a String representation of GemFire's version.
    */
   public String version() {
@@ -635,7 +646,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
     /**
      * Gets the Java classpath used when launching the GemFire service.
-     * 
+     *
      * @return a String value indicating the Java classpath used when launching the GemFire
service.
      * @see java.lang.System#getProperty(String) with 'java.class.path'
      */
@@ -645,7 +656,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
     /**
      * Gets the version of GemFire used to launch and run the GemFire service.
-     * 
+     *
      * @return a String indicating the version of GemFire used in the running GemFire service.
      */
     public String getGemFireVersion() {
@@ -654,7 +665,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
     /**
      * Gets the version of Java used to launch and run the GemFire service.
-     * 
+     *
      * @return a String indicating the version of the Java runtime used in the running GemFire
      *         service.
      * @see java.lang.System#getProperty(String) with 'java.verson'
@@ -665,7 +676,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
     /**
      * Gets the arguments passed to the JVM process that is running the GemFire service.
-     * 
+     *
      * @return a List of String value each representing an argument passed to the JVM of
the GemFire
      *         service.
      * @see java.lang.management.RuntimeMXBean#getInputArguments()
@@ -676,7 +687,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
     /**
      * Gets GemFire member's name for the process.
-     * 
+     *
      * @return a String indicating the GemFire member's name for the process.
      */
     public String getMemberName() {
@@ -685,7 +696,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
     /**
      * Gets the process ID of the running GemFire service if known, otherwise returns null.
-     * 
+     *
      * @return a integer value indicating the process ID (PID) of the running GemFire service,
or
      *         null if the PID cannot be determined.
      */
@@ -695,7 +706,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
     /**
      * Gets the location of the GemFire service (usually the host in combination with the
port).
-     * 
+     *
      * @return a String indication the location (such as host/port) of the GemFire service.
      */
     public String getServiceLocation() {
@@ -704,14 +715,14 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
     /**
      * Gets the name of the GemFire service.
-     * 
+     *
      * @return a String indicating the name of the GemFire service.
      */
     protected abstract String getServiceName();
 
     /**
      * Gets the state of the GemFire service.
-     * 
+     *
      * @return a Status enumerated type representing the state of the GemFire service.
      * @see org.apache.geode.distributed.AbstractLauncher.Status
      */
@@ -721,7 +732,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
     /**
      * Gets description of the the service's current state.
-     * 
+     *
      * @return a String describing the service's current state.
      */
     public String getStatusMessage() {
@@ -730,7 +741,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
     /**
      * The date and time the GemFire service was last in this state.
-     * 
+     *
      * @return a Timestamp signifying the last date and time the GemFire service was in this
state.
      * @see java.sql.Timestamp
      */
@@ -741,7 +752,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
     /**
      * Gets the amount of time in milliseconds that the JVM process with the GemFire service
has
      * been running.
-     * 
+     *
      * @return a long value indicating the number of milliseconds that the GemFire service
JVM has
      *         been running.
      * @see java.lang.management.RuntimeMXBean#getUptime()
@@ -753,7 +764,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
     /**
      * Gets the directory in which the GemFire service is running. This is also the location
where
      * all GemFire service files (log files, the PID file, and so on) are written.
-     * 
+     *
      * @return a String value indicating the GemFire service's working (running) directory.
      */
     public String getWorkingDirectory() {
@@ -762,7 +773,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
     /**
      * Gets the path of the log file for the process.
-     * 
+     *
      * @return a String value indicating the path of the log file for the process.
      */
     public String getLogFile() {
@@ -771,7 +782,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
     /**
      * Gets the host or IP address for the process and its service.
-     * 
+     *
      * @return a String value representing the host or IP address for the process and its
service.
      */
     public String getHost() {
@@ -780,7 +791,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
     /**
      * Gets the port for the process and its service.
-     * 
+     *
      * @return an Integer value indicating the port for the process and its service.
      */
     public String getPort() {
@@ -789,7 +800,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
     /**
      * Gets a String describing the state of the GemFire service.
-     * 
+     *
      * @return a String describing the state of the GemFire service.
      */
     @Override
@@ -799,14 +810,14 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
           return LocalizedStrings.Launcher_ServiceStatus_STARTING_MESSAGE.toLocalizedString(
               getServiceName(), getWorkingDirectory(), getServiceLocation(), getMemberName(),
               toString(getTimestamp()), toString(getPid()), toString(getGemFireVersion()),
-              toString(getJavaVersion()), getLogFile(), toString(getJvmArguments().toArray()),
+              toString(getJavaVersion()), getLogFile(), ArgumentRedactor.redact(getJvmArguments()),
               toString(getClasspath()));
         case ONLINE:
           return LocalizedStrings.Launcher_ServiceStatus_RUNNING_MESSAGE.toLocalizedString(
               getServiceName(), getWorkingDirectory(), getServiceLocation(), getMemberName(),
               getStatus(), toString(getPid()), toDaysHoursMinutesSeconds(getUptime()),
               toString(getGemFireVersion()), toString(getJavaVersion()), getLogFile(),
-              toString(getJvmArguments().toArray()), toString(getClasspath()));
+              ArgumentRedactor.redact(getJvmArguments()), toString(getClasspath()));
         case STOPPED:
           return LocalizedStrings.Launcher_ServiceStatus_STOPPED_MESSAGE
               .toLocalizedString(getServiceName(), getWorkingDirectory(), getServiceLocation());
@@ -856,7 +867,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
     /**
      * Looks up the Status enum type by description. The lookup operation is case-insensitive.
-     * 
+     *
      * @param description a String value describing the Locator's status.
      * @return a Status enumerated type matching the description.
      */
@@ -872,7 +883,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
     /**
      * Gets the description of the Status enum type.
-     * 
+     *
      * @return a String describing the Status enum type.
      */
     public String getDescription() {
@@ -881,7 +892,7 @@ public abstract class AbstractLauncher<T extends Comparable<T>>
implements Runna
 
     /**
      * Gets a String representation of the Status enum type.
-     * 
+     *
      * @return a String representing the Status enum type.
      * @see #getDescription()
      */

http://git-wip-us.apache.org/repos/asf/geode/blob/3fad7a38/geode-core/src/main/java/org/apache/geode/internal/Banner.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/Banner.java b/geode-core/src/main/java/org/apache/geode/internal/Banner.java
index 5ad70b8..4cb7dd0 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/Banner.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/Banner.java
@@ -16,14 +16,20 @@ package org.apache.geode.internal;
 
 import org.apache.geode.SystemFailure;
 import org.apache.geode.distributed.internal.DistributionConfig;
-import org.apache.geode.distributed.internal.DistributionConfigImpl;
 import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.internal.util.ArgumentRedactor;
 
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.lang.management.ManagementFactory;
 import java.lang.management.RuntimeMXBean;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.StringTokenizer;
+import java.util.TreeMap;
 
 /**
  * Utility class to print banner information at manager startup.
@@ -45,10 +51,10 @@ public class Banner {
 
   /**
    * Print information about this process to the specified stream.
-   * 
+   *
    * @param args possibly null list of command line arguments
    */
-  private static void print(PrintWriter out, String args[]) {
+  static void print(PrintWriter out, String args[]) {
     Map sp = new TreeMap((Properties) System.getProperties().clone()); // fix for 46822
     int processId = -1;
     final String SEPERATOR =
@@ -105,7 +111,7 @@ public class Banner {
     sp.remove("user.dir");
     out.println("Home dir: " + sp.get("user.home"));
     sp.remove("user.home");
-    List<String> allArgs = new ArrayList<String>();
+    List<String> allArgs = new ArrayList<>();
     {
       RuntimeMXBean runtimeBean = ManagementFactory.getRuntimeMXBean();
       if (runtimeBean != null) {
@@ -114,16 +120,13 @@ public class Banner {
     }
 
     if (args != null && args.length != 0) {
-      for (int i = 0; i < args.length; i++) {
-        allArgs.add(args[i]);
-      }
+      Collections.addAll(allArgs, args);
     }
     if (!allArgs.isEmpty()) {
       out.println("Command Line Parameters:");
-      for (String arg : allArgs) {
-        out.println("  " + arg);
-      }
+      out.println(ArgumentRedactor.redact(allArgs));
     }
+
     out.println("Class Path:");
     prettyPrintPath((String) sp.get("java.class.path"), out);
     sp.remove("java.class.path");
@@ -135,22 +138,7 @@ public class Banner {
       out.println("System property logging disabled.");
     } else {
       out.println("System Properties:");
-      Iterator it = sp.entrySet().iterator();
-      while (it.hasNext()) {
-        Map.Entry me = (Map.Entry) it.next();
-        String key = me.getKey().toString();
-        // SW: Filter out the security properties since they may contain
-        // sensitive information.
-        if (!key
-            .startsWith(DistributionConfig.GEMFIRE_PREFIX + DistributionConfig.SECURITY_PREFIX_NAME)
-            && !key.startsWith(DistributionConfigImpl.SECURITY_SYSTEM_PREFIX
-                + DistributionConfig.SECURITY_PREFIX_NAME)
-            && !key.toLowerCase().contains("password") /* bug 45381 */) {
-          out.println("    " + key + " = " + me.getValue());
-        } else {
-          out.println("    " + key + " = " + "********");
-        }
-      }
+      out.println(ArgumentRedactor.redact(sp));
       out.println("Log4J 2 Configuration:");
       out.println("    " + LogService.getConfigInformation());
     }
@@ -159,7 +147,7 @@ public class Banner {
 
   /**
    * Return a string containing the banner information.
-   * 
+   *
    * @param args possibly null list of command line arguments
    */
   public static String getString(String args[]) {

http://git-wip-us.apache.org/repos/asf/geode/blob/3fad7a38/geode-core/src/main/java/org/apache/geode/internal/util/ArgumentRedactor.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/util/ArgumentRedactor.java
b/geode-core/src/main/java/org/apache/geode/internal/util/ArgumentRedactor.java
new file mode 100644
index 0000000..419f3f9
--- /dev/null
+++ b/geode-core/src/main/java/org/apache/geode/internal/util/ArgumentRedactor.java
@@ -0,0 +1,154 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information
regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version
2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain
a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under
the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
express
+ * or implied. See the License for the specific language governing permissions and limitations
under
+ * the License.
+ */
+
+package org.apache.geode.internal.util;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+
+public class ArgumentRedactor {
+
+  private ArgumentRedactor() {}
+
+  public static String redact(final List<String> args) {
+    StringBuilder redacted = new StringBuilder();
+    for (String arg : args) {
+      redacted.append(redact(arg)).append(" ");
+    }
+    return redacted.toString().trim();
+  }
+
+  /**
+   * Accept a map of key/value pairs and produce a printable string, redacting any necessary
values.
+   * 
+   * @param map A {@link Map} of key/value pairs such as a collection of properties
+   *
+   * @return A printable string with redacted fields. E.g., "username=jdoe password=********"
+   */
+  public static String redact(final Map<String, String> map) {
+    StringBuilder redacted = new StringBuilder();
+    for (Entry<String, String> entry : map.entrySet()) {
+      redacted.append(entry.getKey());
+      redacted.append("=");
+      redacted.append(redact(entry));
+      redacted.append(" ");
+    }
+    return redacted.toString().trim();
+  }
+
+  /**
+   * Returns the redacted value of the {@link Entry} if the key indicates redaction is necessary.
+   * Otherwise, value is returned, unchanged.
+   * 
+   * @param entry A key/value pair
+   *
+   * @return The redacted string for value.
+   */
+  public static String redact(Entry<String, String> entry) {
+    return redact(entry.getKey(), entry.getValue());
+  }
+
+  /**
+   * Parse a string to find key=value pairs and redact the values if necessary. If more than
one
+   * key=value pair exists in the input, each pair must be preceeded by a hyphen '-' to delineate
+   * the pairs. <br>
+   * Example:<br>
+   * Single value: "password=secret" or "--password=secret" Mulitple values: "-Dflag -Dkey=value
+   * --classpath=."
+   * 
+   * @param line The input to be parsed
+   * @return A redacted string that has sensitive information obscured.
+   */
+  public static String redact(String line) {
+    StringBuilder redacted = new StringBuilder();
+    if (line.startsWith("-")) {
+      line = " " + line;
+      String[] args = line.split(" -");
+      StringBuilder param = new StringBuilder();
+      for (String arg : args) {
+        if (arg.isEmpty()) {
+          param.append("-");
+        } else {
+          String[] pair = arg.split("=", 2);
+          param.append(pair[0].trim());
+          if (pair.length == 1) {
+            redacted.append(param);
+          } else {
+            redacted.append(param).append("=").append(redact(param.toString(), pair[1].trim()));
+          }
+          redacted.append(" ");
+        }
+        param.setLength(0);
+        param.append("-");
+      }
+    } else {
+      String[] args = line.split("=", 2);
+      if (args.length == 1) {
+        redacted.append(line);
+      } else {
+        redacted.append(args[0].trim()).append("=").append(redact(args[0], args[1]));
+      }
+      redacted.append(" ");
+    }
+    return redacted.toString().trim();
+  }
+
+  /**
+   * Return a redacted value if the key indicates redaction is necessary. Otherwise, return
the
+   * value unchanged.
+   *
+   * @param key A string such as a system property, jvm parameter or similar in a key=value
+   *        situation.
+   * @param value A string that is the value assigned to the key.
+   *
+   * @return A redacted string if the key indicates it should be redacted, otherwise the
string is
+   *         unchanged.
+   */
+  public static String redact(String key, String value) {
+    if (shouldBeRedacted(key)) {
+      return "********";
+    }
+    return value.trim();
+  }
+
+  /**
+   * Determine whether a key's value should be redacted.
+   * 
+   * @param key The key in question.
+   *
+   * @return true if the value should be redacted, otherwise false.
+   */
+  private static boolean shouldBeRedacted(String key) {
+    if (key == null) {
+      return false;
+    }
+
+    // Clean off any flags such as -J and -D to get to the actual start of the parameter
+    String compareKey = key;
+    if (key.startsWith("-J")) {
+      compareKey = key.substring(2);
+    }
+    if (compareKey.startsWith("-D")) {
+      compareKey = compareKey.substring(2);
+    }
+    return compareKey.toLowerCase().contains("password");
+    // return compareKey
+    // .startsWith(DistributionConfig.GEMFIRE_PREFIX + DistributionConfig.SECURITY_PREFIX_NAME)
+    // || compareKey.startsWith(
+    // DistributionConfigImpl.SECURITY_SYSTEM_PREFIX + DistributionConfig.SECURITY_PREFIX_NAME)
+    // || compareKey.toLowerCase().contains("password");
+  }
+}

http://git-wip-us.apache.org/repos/asf/geode/blob/3fad7a38/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
index cc5882a..bcf1b41 100755
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
@@ -14,42 +14,13 @@
  */
 package org.apache.geode.management.internal.cli.shell;
 
-import java.io.BufferedReader;
-import java.io.File;
-import java.io.FileReader;
-import java.io.IOException;
-import java.io.PrintStream;
-import java.net.URL;
-import java.text.MessageFormat;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Enumeration;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Scanner;
-import java.util.Set;
-import java.util.TreeMap;
-import java.util.logging.Level;
-import java.util.logging.LogManager;
-import java.util.logging.Logger;
-
 import jline.Terminal;
 import jline.console.ConsoleReader;
-import org.springframework.shell.core.AbstractShell;
-import org.springframework.shell.core.CommandMarker;
-import org.springframework.shell.core.Converter;
-import org.springframework.shell.core.ExecutionStrategy;
-import org.springframework.shell.core.ExitShellRequest;
-import org.springframework.shell.core.JLineLogHandler;
-import org.springframework.shell.core.JLineShell;
-import org.springframework.shell.core.Parser;
-import org.springframework.shell.event.ShellStatus.Status;
-
 import org.apache.geode.internal.Banner;
 import org.apache.geode.internal.GemFireVersion;
 import org.apache.geode.internal.lang.ClassUtils;
 import org.apache.geode.internal.process.signal.AbstractSignalNotificationHandler;
+import org.apache.geode.internal.util.ArgumentRedactor;
 import org.apache.geode.internal.util.HostName;
 import org.apache.geode.internal.util.SunAPINotFoundException;
 import org.apache.geode.management.cli.CommandProcessingException;
@@ -70,6 +41,35 @@ import org.apache.geode.management.internal.cli.shell.jline.GfshHistory;
 import org.apache.geode.management.internal.cli.shell.jline.GfshUnsupportedTerminal;
 import org.apache.geode.management.internal.cli.shell.unsafe.GfshSignalHandler;
 import org.apache.geode.management.internal.cli.util.CommentSkipHelper;
+import org.springframework.shell.core.AbstractShell;
+import org.springframework.shell.core.CommandMarker;
+import org.springframework.shell.core.Converter;
+import org.springframework.shell.core.ExecutionStrategy;
+import org.springframework.shell.core.ExitShellRequest;
+import org.springframework.shell.core.JLineLogHandler;
+import org.springframework.shell.core.JLineShell;
+import org.springframework.shell.core.Parser;
+import org.springframework.shell.event.ShellStatus.Status;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.PrintStream;
+import java.net.URL;
+import java.text.MessageFormat;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Scanner;
+import java.util.Set;
+import java.util.TreeMap;
+import java.util.logging.Level;
+import java.util.logging.LogManager;
+import java.util.logging.Logger;
 
 /**
  * Extends an interactive shell provided by
@@ -84,7 +84,7 @@ import org.apache.geode.management.internal.cli.util.CommentSkipHelper;
  * <p />
  * Additionally, this class is used to maintain GemFire SHell (gfsh) specific information
like:
  * environment TODO
- *
+ * 
  *
  * @since GemFire 7.0
  */
@@ -180,7 +180,7 @@ public class Gfsh extends JLineShell {
 
   /**
    * Create a GemFire shell with console using the specified arguments.
-   *
+   * 
    * @param args arguments to be used to create a GemFire shell instance
    * @throws IOException
    * @throws ClassNotFoundException
@@ -192,7 +192,7 @@ public class Gfsh extends JLineShell {
   /**
    * Create a GemFire shell using the specified arguments. Console for user inputs is made
available
    * if <code>launchShell</code> is set to <code>true</code>.
-   *
+   * 
    * @param launchShell whether to make Console available
    * @param args arguments to be used to create a GemFire shell instance or execute command
    * @throws IOException
@@ -208,10 +208,10 @@ public class Gfsh extends JLineShell {
     this.gfshFileLogger = LogWrapper.getInstance();
     this.gfshFileLogger.configure(this.gfshConfig);
     this.ansiHandler = ANSIHandler.getInstance(this.gfshConfig.isANSISupported()); // TODO
-
-                                                                                   // Abhishek
:
-                                                                                   // should
take it
-                                                                                   // from
-                                                                                   // ConsoleReader.terminal??
+    // Abhishek :
+    // should take it
+    // from
+    // ConsoleReader.terminal??
 
     /* 3. log system properties & gfsh environment */
     this.gfshFileLogger.info(Banner.getString(args));
@@ -397,7 +397,7 @@ public class Gfsh extends JLineShell {
   /**
    * Returns the {@link ExecutionStrategy} implementation used by this implementation of
    * {@link AbstractShell}. {@link Gfsh} uses {@link GfshExecutionStrategy}.
-   *
+   * 
    * @return ExecutionStrategy used by Gfsh
    */
   @Override
@@ -408,7 +408,7 @@ public class Gfsh extends JLineShell {
   /**
    * Returns the {@link Parser} implementation used by this implementation of
    * {@link AbstractShell}.{@link Gfsh} uses {@link GfshParser}.
-   *
+   * 
    * @return Parser used by Gfsh
    */
   @Override
@@ -420,7 +420,7 @@ public class Gfsh extends JLineShell {
   /**
    * Executes the given command string. We have over-ridden the behavior to extend the original
    * implementation to store the 'last command execution status'.
-   *
+   * 
    * @param line command string to be executed
    * @return true if execution is successful; false otherwise
    */
@@ -684,7 +684,7 @@ public class Gfsh extends JLineShell {
 
   /**
    * Set the last command execution status
-   *
+   * 
    * @param lastExecutionStatus last command execution status
    */
   public void setLastExecutionStatus(int lastExecutionStatus) {
@@ -782,7 +782,7 @@ public class Gfsh extends JLineShell {
         String lineRead = "";
         StringBuilder linesBuffer = new StringBuilder();
         String linesBufferString = ""; // used to check whether the string in a buffer contains
a
-                                       // ";".
+        // ";".
         int commandSrNum = 0;
         CommentSkipHelper commentSkipper = new CommentSkipHelper();
 
@@ -804,13 +804,12 @@ public class Gfsh extends JLineShell {
           if (!linesBufferString.endsWith(SyntaxConstants.CONTINUATION_CHARACTER)) { // see
45893
             // String command = null;
 
-
             List<String> commandList = MultiCommandHelper.getMultipleCommands(linesBufferString);
             for (String cmdLet : commandList) {
-              String trimmedCommand = cmdLet.trim();
-              if (!trimmedCommand.isEmpty()) {
+              if (!cmdLet.isEmpty()) {
+                String redactedCmdLet = ArgumentRedactor.redact(cmdLet);
                 ++commandSrNum;
-                Gfsh.println(commandSrNum + ". Executing - " + cmdLet);
+                Gfsh.println(commandSrNum + ". Executing - " + redactedCmdLet);
                 Gfsh.println();
                 boolean executeSuccess = executeScriptLine(cmdLet);
                 if (!executeSuccess) {
@@ -1138,14 +1137,14 @@ public class Gfsh extends JLineShell {
    *
    * For example: if the terminal width were 5 and the string "123 456789 01234" were passed
in with
    * an indentation level of 2, then the returned string would be:
-   * 
+   *
    * <pre>
    *         123
    *         45678
    *         9
    *         01234
    * </pre>
-   * 
+   *
    * @param string String to wrap (add breakpoints and indent)
    * @param indentationLevel The number of indentation levels to use.
    * @return The wrapped string.

http://git-wip-us.apache.org/repos/asf/geode/blob/3fad7a38/geode-core/src/test/java/org/apache/geode/internal/util/ArgumentRedactorJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/util/ArgumentRedactorJUnitTest.java
b/geode-core/src/test/java/org/apache/geode/internal/util/ArgumentRedactorJUnitTest.java
new file mode 100644
index 0000000..b40d485
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/internal/util/ArgumentRedactorJUnitTest.java
@@ -0,0 +1,125 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information
regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version
2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain
a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under
the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
express
+ * or implied. See the License for the specific language governing permissions and limitations
under
+ * the License.
+ */
+
+package org.apache.geode.internal.util;
+
+import static org.apache.geode.internal.util.ArgumentRedactor.redact;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.geode.test.junit.categories.UnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * ArgumentRedactor Tester.
+ */
+@Category(UnitTest.class)
+public class ArgumentRedactorJUnitTest {
+  @Test
+  public void testRedactArgList() throws Exception {
+    List<String> argList = new ArrayList<>();
+    argList.add("gemfire.security-password=secret");
+    argList.add("gemfire.security-properties=./security.properties");
+    argList.add("gemfire.sys.security-value=someValue");
+    argList.add("gemfire.use-cluster-configuration=true");
+    argList.add("someotherstringvalue");
+    argList.add("login-password=secret");
+    argList.add("login-name=admin");
+    argList.add("gemfire-password = super-secret");
+    argList.add("geode-password= confidential");
+    argList.add("some-other-password =shhhh");
+    String redacted = redact(argList);
+    assertTrue(redacted.contains("gemfire.security-password=********"));
+    assertTrue(redacted.contains("gemfire.security-properties=./security.properties"));
+    assertTrue(redacted.contains("gemfire.sys.security-value=someValue"));
+    assertTrue(redacted.contains("gemfire.use-cluster-configuration=true"));
+    assertTrue(redacted.contains("someotherstringvalue"));
+    assertTrue(redacted.contains("login-password=********"));
+    assertTrue(redacted.contains("login-name=admin"));
+    assertTrue(redacted.contains("gemfire-password=********"));
+    assertTrue(redacted.contains("geode-password=********"));
+    assertTrue(redacted.contains("some-other-password=********"));
+  }
+
+  @Test
+  public void testRedactMap() throws Exception {
+    Map<String, String> argMap = new HashMap<>();
+    argMap.put("gemfire.security-password", "secret");
+    argMap.put("gemfire.security-properties", "./security.properties");
+    argMap.put("gemfire.sys.security-value", "someValue");
+    argMap.put("gemfire.use-cluster-configuration", "true");
+    argMap.put("login-password", "secret");
+    argMap.put("login-name", "admin");
+    String redacted = redact(argMap);
+    assertTrue(redacted.contains("gemfire.security-password=********"));
+    assertTrue(redacted.contains("gemfire.security-properties=./security.properties"));
+    assertTrue(redacted.contains("gemfire.sys.security-value=someValue"));
+    assertTrue(redacted.contains("gemfire.use-cluster-configuration=true"));
+    assertTrue(redacted.contains("login-password=********"));
+    assertTrue(redacted.contains("login-name=admin"));
+  }
+
+  @Test
+  public void testRedactArg() throws Exception {
+    String arg = "-Dgemfire.security-password=secret";
+    assertTrue(redact(arg).endsWith("password=********"));
+
+    arg = "-Dgemfire.security-properties=./security-properties";
+    assertEquals(arg, (redact(arg)));
+
+    arg = "-J-Dgemfire.sys.security-value=someValue";
+    assertEquals(arg, (redact(arg)));
+
+    arg = "-Dgemfire.sys.value=printable";
+    assertEquals(arg, redact(arg));
+
+    arg = "-Dgemfire.use-cluster-configuration=true";
+    assertEquals(arg, redact(arg));
+
+    arg = "someotherstringvalue";
+    assertEquals(arg, redact(arg));
+
+    arg = "--password=foo";
+    assertEquals("--password=********", redact(arg));
+
+    arg = "--classpath=.";
+    assertEquals(arg, redact(arg));
+
+    arg = "-DmyArg -Duser-password=foo --classpath=.";
+    assertEquals("-DmyArg -Duser-password=******** --classpath=.", redact(arg));
+
+    arg = "-DmyArg -Duser-password=foo -DOtherArg -Dsystem-password=bar";
+    assertEquals("-DmyArg -Duser-password=******** -DOtherArg -Dsystem-password=********",
+        redact(arg));
+
+    arg =
+        "-Dlogin-password=secret -Dlogin-name=admin -Dgemfire-password = super-secret --geode-password=
confidential -J-Dsome-other-password =shhhh";
+    String redacted = redact(arg);
+    assertTrue(redacted.contains("login-password=********"));
+    assertTrue(redacted.contains("login-name=admin"));
+    assertTrue(redacted.contains("gemfire-password=********"));
+    assertTrue(redacted.contains("geode-password=********"));
+    assertTrue(redacted.contains("some-other-password=********"));
+
+    arg = "-Dgemfire.security-properties=\"c:\\Program Files (x86)\\My Folder\"";
+    assertEquals(arg, (redact(arg)));
+  }
+}


Mime
View raw message