This is an automated email from the ASF dual-hosted git repository.

prhomberg pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new 4f0c5c9  GEODE-4775: Apply ArgumentRedactor to JVM arguments.
4f0c5c9 is described below

commit 4f0c5c9cc0280ce4463fe1b575bd694264da20b7
Author: Patrick Rhomberg <prhomb...@pivotal.io>
AuthorDate: Tue Mar 6 13:26:47 2018 -0800

    GEODE-4775: Apply ArgumentRedactor to JVM arguments.
---
 ...cribeConfigAreFullyRedactedAcceptanceTest.java} | 82 ++++++++++++++--------
 .../internal/locator/LocatorStatusResponse.java    |  2 +-
 .../java/org/apache/geode/internal/Banner.java     |  3 +-
 .../geode/internal/util/ArgumentRedactor.java      |  5 ++
 .../cli/commands/DescribeConfigCommand.java        |  5 +-
 .../cli/domain/MemberConfigurationInfo.java        | 17 +++--
 .../GetMemberConfigInformationFunction.java        |  3 +-
 7 files changed, 80 insertions(+), 37 deletions(-)

diff --git 
a/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LogsAreFullyRedactedAcceptanceTest.java
 
b/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LogsAndDescribeConfigAreFullyRedactedAcceptanceTest.java
similarity index 59%
rename from 
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LogsAreFullyRedactedAcceptanceTest.java
rename to 
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LogsAndDescribeConfigAreFullyRedactedAcceptanceTest.java
index e9bfc05..35a21a0 100644
--- 
a/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LogsAreFullyRedactedAcceptanceTest.java
+++ 
b/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LogsAndDescribeConfigAreFullyRedactedAcceptanceTest.java
@@ -17,11 +17,11 @@ package org.apache.geode.management.internal.cli.commands;
 
 import static org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
 import static 
org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
+import static org.assertj.core.api.Assertions.assertThat;
 
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
-import java.io.IOException;
 import java.util.Collection;
 import java.util.Properties;
 import java.util.Scanner;
@@ -36,6 +36,7 @@ import org.junit.experimental.categories.Category;
 import org.apache.geode.examples.security.ExampleSecurityManager;
 import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
 import org.apache.geode.test.junit.categories.AcceptanceTest;
+import org.apache.geode.test.junit.rules.gfsh.GfshExecution;
 import org.apache.geode.test.junit.rules.gfsh.GfshRule;
 import org.apache.geode.util.test.TestUtil;
 
@@ -48,7 +49,7 @@ import org.apache.geode.util.test.TestUtil;
  * Each password shares the string below for easier log scanning.
  */
 @Category(AcceptanceTest.class)
-public class LogsAreFullyRedactedAcceptanceTest {
+public class LogsAndDescribeConfigAreFullyRedactedAcceptanceTest {
   private static String sharedPasswordString = "abcdefg";
 
   private File propertyFile;
@@ -58,7 +59,7 @@ public class LogsAreFullyRedactedAcceptanceTest {
   public GfshRule gfsh = new GfshRule();
 
   @Before
-  public void createDirectoriesAndFiles() throws IOException {
+  public void createDirectoriesAndFiles() throws Exception {
     propertyFile = gfsh.getTemporaryFolder().newFile("geode.properties");
     securityPropertyFile = 
gfsh.getTemporaryFolder().newFile("security.properties");
 
@@ -79,35 +80,43 @@ public class LogsAreFullyRedactedAcceptanceTest {
     try (FileOutputStream fileOutputStream = new 
FileOutputStream(securityPropertyFile)) {
       securityProperties.store(fileOutputStream, null);
     }
+
+    startSecureLocatorAndServer();
+  }
+
+  private void startSecureLocatorAndServer() throws Exception {
+    try {
+      // The json is in the root resource directory.
+      String securityJson = TestUtil.getResourcePath(
+          LogsAndDescribeConfigAreFullyRedactedAcceptanceTest.class, 
"/security.json");
+      // We want to add the folder to the classpath, so we strip off the 
filename.
+      securityJson = securityJson.substring(0, securityJson.length() - 
"security.json".length());
+      String startLocatorCmd =
+          new CommandStringBuilder("start locator").addOption("name", 
"test-locator")
+              .addOption("properties-file", propertyFile.getAbsolutePath())
+              .addOption("security-properties-file", 
securityPropertyFile.getAbsolutePath())
+              .addOption("J", "-Dsecure-username-jd=user-jd")
+              .addOption("J", "-Dsecure-password-jd=password-jd")
+              .addOption("classpath", securityJson).getCommandString();
+
+      String startServerCmd = new CommandStringBuilder("start server")
+          .addOption("name", "test-server").addOption("user", 
"viaStartMemberOptions")
+          .addOption("password", sharedPasswordString + 
"-viaStartMemberOptions")
+          .addOption("properties-file", propertyFile.getAbsolutePath())
+          .addOption("security-properties-file", 
securityPropertyFile.getAbsolutePath())
+          .addOption("J", "-Dsecure-username-jd=user-jd")
+          .addOption("J", "-Dsecure-password-jd=" + sharedPasswordString + 
"-password-jd")
+          .addOption("classpath", securityJson).getCommandString();
+
+      gfsh.execute(startLocatorCmd, startServerCmd);
+    } catch (Exception e) {
+      throw new Exception(
+          "Cluster could not start, failing beyond the intended scope of this 
test.", e);
+    }
   }
 
   @Test
   public void logsDoNotContainStringThatShouldBeRedacted() throws 
FileNotFoundException {
-    // The json is in the root resource directory.
-    String securityJson =
-        TestUtil.getResourcePath(LogsAreFullyRedactedAcceptanceTest.class, 
"/security.json");
-    // We want to add the folder to the classpath, so we strip off the 
filename.
-    securityJson = securityJson.substring(0, securityJson.length() - 
"security.json".length());
-    String startLocatorCmd =
-        new CommandStringBuilder("start locator").addOption("name", 
"test-locator")
-            .addOption("properties-file", propertyFile.getAbsolutePath())
-            .addOption("security-properties-file", 
securityPropertyFile.getAbsolutePath())
-            .addOption("J", "-Dsecure-username-jd=user-jd")
-            .addOption("J", 
"-Dsecure-password-jd=password-jd").addOption("classpath", securityJson)
-            .getCommandString();
-
-    String startServerCmd = new CommandStringBuilder("start server")
-        .addOption("name", "test-server").addOption("user", 
"viaStartMemberOptions")
-        .addOption("password", sharedPasswordString + "-viaStartMemberOptions")
-        .addOption("properties-file", propertyFile.getAbsolutePath())
-        .addOption("security-properties-file", 
securityPropertyFile.getAbsolutePath())
-        .addOption("J", "-Dsecure-username-jd=user-jd")
-        .addOption("J", "-Dsecure-password-jd=" + sharedPasswordString + 
"-password-jd")
-        .addOption("classpath", securityJson).getCommandString();
-
-
-    gfsh.execute(startLocatorCmd, startServerCmd);
-
     Collection<File> logs =
         FileUtils.listFiles(gfsh.getTemporaryFolder().getRoot(), new String[] 
{"log"}, true);
 
@@ -123,4 +132,21 @@ public class LogsAreFullyRedactedAcceptanceTest {
     }
     softly.assertAll();
   }
+
+  @Test
+  public void describeConfigRedactsJvmArguments() {
+    String connectCommand = new CommandStringBuilder("connect")
+        .addOption("user", "viaStartMemberOptions")
+        .addOption("password", sharedPasswordString + 
"-viaStartMemberOptions").getCommandString();
+
+    String describeLocatorConfigCommand = new CommandStringBuilder("describe 
config")
+        .addOption("hide-defaults", "false").addOption("member", 
"test-locator").getCommandString();
+
+    String describeServerConfigCommand = new CommandStringBuilder("describe 
config")
+        .addOption("hide-defaults", "false").addOption("member", 
"test-server").getCommandString();
+
+    GfshExecution execution =
+        gfsh.execute(connectCommand, describeLocatorConfigCommand, 
describeServerConfigCommand);
+    assertThat(execution.getOutputText()).doesNotContain(sharedPasswordString);
+  }
 }
diff --git 
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/locator/LocatorStatusResponse.java
 
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/locator/LocatorStatusResponse.java
index 04a82f0..e972a67 100644
--- 
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/locator/LocatorStatusResponse.java
+++ 
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/locator/LocatorStatusResponse.java
@@ -69,7 +69,7 @@ public class LocatorStatusResponse extends 
ServerLocationResponse {
       final String locatorLogFile, final String locatorName) {
     final RuntimeMXBean runtimeBean = ManagementFactory.getRuntimeMXBean();
     this.pid = identifyPid();
-    this.jvmArgs = runtimeBean.getInputArguments();
+    this.jvmArgs = 
ArgumentRedactor.redactEachInList(runtimeBean.getInputArguments());
     this.uptime = runtimeBean.getUptime();
     this.classpath = runtimeBean.getClassPath();
     this.gemfireVersion = GemFireVersion.getGemFireVersion();
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 2e53c4a..360c057 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
@@ -80,7 +80,8 @@ public class Banner {
     List<String> commandLineArguments = new ArrayList<>();
     RuntimeMXBean runtimeBean = ManagementFactory.getRuntimeMXBean();
     if (runtimeBean != null) {
-      commandLineArguments.addAll(runtimeBean.getInputArguments()); // fixes 
45353
+      // each argument is redacted below.
+      commandLineArguments.addAll(runtimeBean.getInputArguments());
     }
 
     if (args != null && args.length != 0) {
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
index a012504..db67f5f 100644
--- 
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
@@ -18,6 +18,7 @@ package org.apache.geode.internal.util;
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 
 import org.apache.geode.distributed.ConfigurationProperties;
 import org.apache.geode.distributed.internal.DistributionConfig;
@@ -153,4 +154,8 @@ public class ArgumentRedactor {
     }
     return false;
   }
+
+  public static List<String> redactEachInList(List<String> inputArguments) {
+    return 
inputArguments.stream().map(ArgumentRedactor::redact).collect(Collectors.toList());
+  }
 }
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeConfigCommand.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeConfigCommand.java
index b826918..ec86fc1 100644
--- 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeConfigCommand.java
+++ 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeConfigCommand.java
@@ -26,6 +26,7 @@ import org.springframework.shell.core.annotation.CliOption;
 import org.apache.geode.cache.execute.FunctionInvocationTargetException;
 import org.apache.geode.cache.execute.ResultCollector;
 import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.internal.util.ArgumentRedactor;
 import org.apache.geode.management.cli.CliMetaData;
 import org.apache.geode.management.cli.ConverterHint;
 import org.apache.geode.management.cli.Result;
@@ -77,7 +78,9 @@ public class DescribeConfigCommand implements GfshCommand {
         TabularResultData jvmInputArgs = 
crd.addSection().addSection().addTable();
 
         for (String jvmArg : jvmArgsList) {
-          jvmInputArgs.accumulate("JVM command line arguments", jvmArg);
+          // This redaction should be redundant, since jvmArgs should have 
already been redacted in
+          // MemberConfigurationInfo. Still, better redundant than missing.
+          jvmInputArgs.accumulate("JVM command line arguments", 
ArgumentRedactor.redact(jvmArg));
         }
 
         addSection(crd, memberConfigInfo.getGfePropsSetUsingApi(),
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/MemberConfigurationInfo.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/MemberConfigurationInfo.java
index 416c3a9..5be757d 100644
--- 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/MemberConfigurationInfo.java
+++ 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/MemberConfigurationInfo.java
@@ -17,10 +17,20 @@ package org.apache.geode.management.internal.cli.domain;
 import java.io.Serializable;
 import java.lang.management.ManagementFactory;
 import java.lang.management.RuntimeMXBean;
-import java.util.*;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+
+import org.apache.geode.internal.util.ArgumentRedactor;
 
 public class MemberConfigurationInfo implements Serializable {
 
+  /**
+   * JVM arguments can potentially contain sensitive information. If these 
arguments are ever to be
+   * displayed, remember to apply
+   * {@link org.apache.geode.internal.util.ArgumentRedactor#redact(String)}
+   * to each argument.
+   */
   private List<String> jvmInputArguments;
   private Properties systemProperties;
   private Map<String, String> gfePropsSetUsingApi;
@@ -33,7 +43,7 @@ public class MemberConfigurationInfo implements Serializable {
 
   public MemberConfigurationInfo() {
     RuntimeMXBean runtimeBean = ManagementFactory.getRuntimeMXBean();
-    setJvmInputArguments(runtimeBean.getInputArguments());
+    
setJvmInputArguments(ArgumentRedactor.redactEachInList(runtimeBean.getInputArguments()));
 
   }
 
@@ -110,7 +120,4 @@ public class MemberConfigurationInfo implements 
Serializable {
   public void setPdxAttrributes(Map<String, String> pdxAttrributes) {
     this.pdxAttributes = pdxAttrributes;
   }
-
-
-
 }
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetMemberConfigInformationFunction.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetMemberConfigInformationFunction.java
index 7d29b48..d2068b2 100644
--- 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetMemberConfigInformationFunction.java
+++ 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetMemberConfigInformationFunction.java
@@ -36,6 +36,7 @@ import org.apache.geode.internal.cache.CacheConfig;
 import org.apache.geode.internal.cache.GemFireCacheImpl;
 import org.apache.geode.internal.cache.execute.InternalFunction;
 import org.apache.geode.internal.cache.ha.HARegionQueue;
+import org.apache.geode.internal.util.ArgumentRedactor;
 import org.apache.geode.management.internal.cli.domain.MemberConfigurationInfo;
 
 /****
@@ -222,6 +223,6 @@ public class GetMemberConfigInformationFunction implements 
InternalFunction {
 
   private List<String> getJvmInputArguments() {
     RuntimeMXBean runtimeBean = ManagementFactory.getRuntimeMXBean();
-    return runtimeBean.getInputArguments();
+    return ArgumentRedactor.redactEachInList(runtimeBean.getInputArguments());
   }
 }

-- 
To stop receiving notification emails like this one, please contact
prhomb...@apache.org.

Reply via email to