Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id B28B9200BE7 for ; Tue, 20 Dec 2016 23:23:36 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id B1149160B29; Tue, 20 Dec 2016 22:23:36 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 14E39160B12 for ; Tue, 20 Dec 2016 23:23:34 +0100 (CET) Received: (qmail 20555 invoked by uid 500); 20 Dec 2016 22:23:34 -0000 Mailing-List: contact commits-help@geode.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@geode.apache.org Delivered-To: mailing list commits@geode.apache.org Received: (qmail 20546 invoked by uid 99); 20 Dec 2016 22:23:34 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 20 Dec 2016 22:23:34 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 2F495DFB86; Tue, 20 Dec 2016 22:23:34 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: klund@apache.org To: commits@geode.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: geode git commit: GEODE-2119: gfsh user and password visible in clear text Date: Tue, 20 Dec 2016 22:23:34 +0000 (UTC) archived-at: Tue, 20 Dec 2016 22:23:36 -0000 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 Authored: Wed Dec 7 13:23:38 2016 -0800 Committer: Kirk Lund 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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 allArgs = new ArrayList(); + List 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 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 map) { + StringBuilder redacted = new StringBuilder(); + for (Entry 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 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.
+ * Example:
+ * 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; *

* 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 launchShell is set to true. - * + * * @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 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: - * + * * 

    *         123
    *         45678
    *         9
    *         01234
    *
- * + * * @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 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 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))); + } +}