You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by pr...@apache.org on 2018/03/06 21:26:50 UTC
[geode] branch develop updated: GEODE-4775: Apply ArgumentRedactor
to JVM arguments.
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 <pr...@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
prhomberg@apache.org.