Merge pull request #2114 from newrelic/sanitize-environment

Sanitize environment

newrelic-agent/src/main/java/com/newrelic/agent/attributes/ExcludeIncludeFilterImpl.java

@@ -27,9 +27,19 @@ public class ExcludeIncludeFilterImpl implements ExcludeIncludeFilter {
      * @param includes   the collection of keys to be included, if empty, all keys are considered included
      */
     public ExcludeIncludeFilterImpl(String identifier, Collection<String> excludes, Collection<String> includes) {
+        this(identifier, excludes, includes, includes == null || includes.isEmpty());
+    }
+
+    /**
+     * @param identifier used in logs so filters can be distinguished
+     * @param excludes   the collection of keys to be excluded
+     * @param includes   the collection of keys to be included
+     * @param includeByDefault if true, keys not in the include list will be included, otherwise they will be excluded
+     */
+    public ExcludeIncludeFilterImpl(String identifier, Collection<String> excludes, Collection<String> includes, boolean includeByDefault) {
         rootNode = new RootConfigAttributesNode(identifier);
 
-        this.includeByDefault = includes == null || includes.isEmpty();
+        this.includeByDefault = includeByDefault;
 
         boolean notDefault = false;
 

newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfig.java

@@ -189,6 +189,8 @@ public interface AgentConfig extends com.newrelic.api.agent.Config, DataSenderCo
      */
     AttributesConfig getAttributesConfig();
 
+    ObfuscateJvmPropsConfig getObfuscateJvmPropsConfig();
+
     /**
      * The agent language (java).
      */

newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigImpl.java

@@ -74,6 +74,7 @@ public class AgentConfigImpl extends BaseConfig implements AgentConfig {
     public static final String METRIC_INGEST_URI = "metric_ingest_uri";
     public static final String EVENT_INGEST_URI = "event_ingest_uri";
     public static final String METRIC_DEBUG = "metric_debug";
+    public static final String OBFUSCATE_JVM_PROPS = "obfuscate_jvm_props";
     public static final String PLATFORM_INFORMATION_ENABLED = "platform_information_enabled";
     public static final String PORT = "port";
     public static final String PROXY_HOST = "proxy_host";
@@ -276,6 +277,8 @@ public class AgentConfigImpl extends BaseConfig implements AgentConfig {
     private final TransactionTracerConfigImpl transactionTracerConfig;
     private final UtilizationDataConfig utilizationConfig;
 
+    private final ObfuscateJvmPropsConfig obfuscateJvmPropsConfig;
+
     private final Map<String, Object> flattenedProperties;
     private final CommandParserConfig commandParserConfig;
 
@@ -371,6 +374,7 @@ private AgentConfigImpl(Map<String, Object> props) {
         commandParserConfig = initCommandParserConfig();
         normalizationRuleConfig = new NormalizationRuleConfig(props);
         slowTransactionsConfig = initSlowTransactionsConfig();
+        obfuscateJvmPropsConfig = initObfuscateJvmPropsConfig();
         superAgentIntegrationConfig = initSuperAgentHealthCheckConfig();
 
         Map<String, Object> flattenedProps = new HashMap<>();
@@ -786,6 +790,11 @@ private ClassTransformerConfig initClassTransformerConfig(boolean liteMode) {
         return ClassTransformerConfigImpl.createClassTransformerConfig(props, customTracingEnabled, liteMode, addSecurityExcludes);
     }
 
+    private ObfuscateJvmPropsConfig initObfuscateJvmPropsConfig() {
+        Map<String, Object> props = nestedProps(OBFUSCATE_JVM_PROPS);
+        return new ObfuscateJvmPropsConfigImpl(props);
+    }
+
     private CircuitBreakerConfig initCircuitBreakerConfig() {
         Map<String, Object> props = nestedProps(CircuitBreakerConfig.PROPERTY_NAME);
         return new CircuitBreakerConfig(props);
@@ -1241,6 +1250,11 @@ public AttributesConfig getAttributesConfig() {
         return attributesConfig;
     }
 
+    @Override
+    public ObfuscateJvmPropsConfig getObfuscateJvmPropsConfig() {
+        return obfuscateJvmPropsConfig;
+    }
+
     @Override
     public ReinstrumentConfig getReinstrumentConfig() {
         return reinstrumentConfig;

newrelic-agent/src/main/java/com/newrelic/agent/config/ObfuscateJvmPropsConfig.java

@@ -0,0 +1,12 @@
+package com.newrelic.agent.config;
+
+import java.util.Set;
+
+public interface ObfuscateJvmPropsConfig {
+
+    boolean isEnabled();
+
+    Set<String> getAllow();
+
+    Set<String> getBlock();
+}

newrelic-agent/src/main/java/com/newrelic/agent/config/ObfuscateJvmPropsConfigImpl.java

@@ -0,0 +1,44 @@
+package com.newrelic.agent.config;
+
+import java.util.*;
+
+public class ObfuscateJvmPropsConfigImpl extends BaseConfig implements ObfuscateJvmPropsConfig {
+
+    private static final String SYSTEM_PROPERTY_ROOT="newrelic.config.obfuscate_jvm_props.";
+    private static final String ALLOW = "allow";
+    private static final String BLOCK = "block";
+    private static final String ENABLED = "enabled";
+    private static final boolean DEFAULT_ENABLED = true;
+    private static final Set<String> DEFAULT_ALLOW = Collections.singleton("-X*");
+    private final boolean isEnabled;
+    private final Set<String> blockList;
+    private final Set<String> allowList;
+
+    public ObfuscateJvmPropsConfigImpl(Map<String, Object> props) {
+        super(props, SYSTEM_PROPERTY_ROOT);
+        isEnabled = getProperty(ENABLED, DEFAULT_ENABLED);
+        blockList = initializeBlock();
+        allowList = initializeAllow();
+    }
+
+    @Override
+    public boolean isEnabled() {
+        return isEnabled;
+    }
+
+    @Override
+    public Set<String> getAllow() { return allowList; }
+
+    @Override
+    public Set<String> getBlock() { return blockList; }
+
+    private Set<String> initializeBlock() {
+        return new HashSet<>(getUniqueStrings(BLOCK));
+    }
+
+    private Set<String> initializeAllow() {
+        Set<String> tempAllow = new HashSet<>(getUniqueStrings(ALLOW));
+        tempAllow.addAll(DEFAULT_ALLOW);
+        return tempAllow;
+    }
+}

newrelic-agent/src/main/java/com/newrelic/agent/environment/Environment.java

@@ -9,8 +9,11 @@
 
 import com.google.common.annotations.VisibleForTesting;
 import com.newrelic.agent.Agent;
+import com.newrelic.agent.attributes.ExcludeIncludeFilter;
+import com.newrelic.agent.attributes.ExcludeIncludeFilterImpl;
 import com.newrelic.agent.MetricNames;
 import com.newrelic.agent.config.AgentConfig;
+import com.newrelic.agent.config.ObfuscateJvmPropsConfig;
 import com.newrelic.agent.samplers.MemorySampler;
 import com.newrelic.api.agent.NewRelic;
 import org.apache.commons.lang3.StringUtils;
@@ -27,6 +30,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Set;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.logging.Level;
 import java.util.regex.Matcher;
@@ -43,6 +47,7 @@ public class Environment implements JSONStreamAware, Cloneable {
     private static final String SOLR_VERSION_KEY = "Solr Version";
     private static final String AZURE_SITE_EXT_INSTALL_TYPE = "AzureSiteExtension";
     private static final Pattern JSON_WORKAROUND = Pattern.compile("\\\\+$");
+    private static final String OBFUSCATED = "=obfuscated";
 
     private final List<EnvironmentChangeListener> listeners = new CopyOnWriteArrayList<>();
     private final List<List<?>> environmentMap = new ArrayList<>();
@@ -90,6 +95,7 @@ public Environment(AgentConfig config, String logFilePath) {
             if (config.isSendJvmProps()) {
                 RuntimeMXBean runtimeMXBean = ManagementFactory.getRuntimeMXBean();
                 List<String> inputArguments = fixInputArguments(runtimeMXBean.getInputArguments());
+                inputArguments = obfuscateProps(inputArguments, config.getObfuscateJvmPropsConfig());
                 environmentMap.add(Arrays.asList("JVM arguments", inputArguments));
             }
         }
@@ -144,6 +150,36 @@ public Environment(AgentConfig config, String logFilePath) {
         agentIdentity = new AgentIdentity(dispatcher, null, serverPort, instanceName);
     }
 
+    @VisibleForTesting
+    protected List<String> obfuscateProps(List<String> inputArgs, ObfuscateJvmPropsConfig jvmPropsConfig) {
+        if (!jvmPropsConfig.isEnabled()) {
+            return inputArgs;
+        }
+        ExcludeIncludeFilter filter = createJvmPropsFilter(jvmPropsConfig);
+        List<String> sanitized = new ArrayList<>();
+        for (String prop: inputArgs) {
+            String[] propNameAndVal = prop.split("=");
+            String propName = propNameAndVal[0];
+            //small optimization: apply the filter rules only if the prop has a value after an equals sign
+            if (propNameAndVal.length > 1 && shouldObfuscate(propName, filter)) {
+                sanitized.add(propName + OBFUSCATED);
+            } else {
+                sanitized.add(prop);
+            }
+        }
+        return sanitized;
+    }
+
+    private boolean shouldObfuscate(String arg, ExcludeIncludeFilter filter) {
+        return !filter.shouldInclude(arg);
+    }
+
+    private ExcludeIncludeFilter createJvmPropsFilter(ObfuscateJvmPropsConfig jvmPropsConfig) {
+        Set<String> block = jvmPropsConfig.getBlock();
+        Set<String> allow = jvmPropsConfig.getAllow();
+        return new ExcludeIncludeFilterImpl("obfuscate_jvm_props", block, allow, false);
+    }
+
     public void addEnvironmentChangeListener(EnvironmentChangeListener listener) {
         this.listeners.add(listener);
     }

newrelic-agent/src/test/java/com/newrelic/agent/environment/EnvironmentTest.java

@@ -7,8 +7,11 @@
 
 package com.newrelic.agent.environment;
 
+import java.util.List;
+import java.util.ArrayList;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import org.junit.After;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -19,6 +22,13 @@
 
 public class EnvironmentTest {
 
+    @After
+    public void clearObfuscateJvmPropsSettings() {
+        System.clearProperty("newrelic.config.obfuscate_jvm_props.enabled");
+        System.clearProperty("newrelic.config.obfuscate_jvm_props.allow");
+        System.clearProperty("newrelic.config.obfuscate_jvm_props.block");
+    }
+
     @Test
     public void overrideServerPort() throws Exception {
         System.setProperty("newrelic.config.appserver_port", "666");
@@ -125,4 +135,125 @@ public void testJvmProp() {
             System.clearProperty(property);
         }
     }
+
+    //bunch of jvm props tests
+
+    @Test
+    public void obfuscateJvmProps_obfuscatesByDefault() {
+        List<String> props = exampleJvmProps();
+        List<String> obfuscatedProps = getObfuscatedProps(props);
+        //nothing has been configured, all values should be obfuscated
+        Assert.assertEquals(props.size(), obfuscatedProps.size());
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.A=obfuscated"));
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.B=obfuscated"));
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.B.extended=obfuscated"));
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.C=obfuscated"));
+        Assert.assertTrue(obfuscatedProps.contains("-javaagent:1234562"));
+    }
+
+    @Test
+    public void obfuscateJvmProps_doesNotObfuscateWhenDisabled() {
+        System.setProperty("newrelic.config.obfuscate_jvm_props.enabled", "false");
+        List<String> props = exampleJvmProps();
+        List<String> obfuscatedProps = getObfuscatedProps(props);
+
+        Assert.assertEquals(props.size(), obfuscatedProps.size());
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.A=one"));
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.B=two"));
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.B.extended=three"));
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.C=four"));
+        Assert.assertTrue(obfuscatedProps.contains("-javaagent:1234562"));
+    }
+
+    @Test
+    public void obfuscateJvmProps_doesNotTouchAgentFlag() {
+        //the current implementation does not modify the -javaagent prop.
+        //if the implementation changes we want to make sure the agent flag is still sent unmodified!
+        List<String> props = new ArrayList<>();
+        props.add("-javaagent:1234562abcdefg");
+        List<String> obfuscatedProps = getObfuscatedProps(props);
+        Assert.assertEquals(props.size(), obfuscatedProps.size());
+        Assert.assertTrue(obfuscatedProps.contains("-javaagent:1234562abcdefg"));
+    }
+
+    @Test
+    public void obfuscateJvmProps_doesNotTouchXPrefixes_unlessOverridden() {
+        //we should leave -X and -XX alone even if they have equals signs unless overridden via configuration
+        System.setProperty("newrelic.config.obfuscate_jvm_props.block", "-XX:MaxNewSize");
+        List<String> props = new ArrayList<>();
+        props.add("-XX:MaxNewSize=256m");
+        props.add("-XX:MaxPermSize=128m");
+        props.add("-Xsomething=234");
+        List<String> obfuscatedProps = getObfuscatedProps(props);
+
+        Assert.assertEquals(3, obfuscatedProps.size());
+        Assert.assertTrue(obfuscatedProps.contains("-XX:MaxNewSize=obfuscated"));
+        Assert.assertTrue(obfuscatedProps.contains("-XX:MaxPermSize=128m"));
+        Assert.assertTrue(obfuscatedProps.contains("-Xsomething=234"));
+
+    }
+
+    @Test
+    public void obfuscateJvmProps_allowAndBlock() {
+        System.setProperty("newrelic.config.obfuscate_jvm_props.allow", "-Dprop.A, -Dprop.B*");
+        List<String> props = exampleJvmProps();
+        List<String> obfuscatedProps = getObfuscatedProps(props);
+
+        Assert.assertEquals(props.size(), obfuscatedProps.size());
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.A=one"));
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.B=two"));
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.B.extended=three"));
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.C=obfuscated"));
+        Assert.assertTrue(obfuscatedProps.contains("-javaagent:1234562"));
+    }
+
+    @Test
+    public void obfuscateJvmProps_specificityAlwaysWins() {
+        //when there is overlap, the more specific rule should always apply
+        //so propA.extended and propB.extended rules apply regardless of which list they belong to
+        System.setProperty("newrelic.config.obfuscate_jvm_props.allow", "-Dprop.A.extended, -Dprop.B*");
+        System.setProperty("newrelic.config.obfuscate_jvm_props.block", "-Dprop.A*, -Dprop.B.extended");
+        List<String> props = exampleJvmProps();
+        props.add("-Dprop.A.extended=six");
+        List<String> obfuscatedProps = getObfuscatedProps(props);
+
+        Assert.assertEquals(props.size(), obfuscatedProps.size());
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.A=obfuscated"));
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.A.extended=six"));
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.B=two"));
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.B.extended=obfuscated"));
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.C=obfuscated"));
+        Assert.assertTrue(obfuscatedProps.contains("-javaagent:1234562"));
+    }
+
+    @Test
+    public void obfuscateJvmProps_blockWins() {
+        System.setProperty("newrelic.config.obfuscate_jvm_props.allow", "-Dprop.A, -Dprop.B*");
+        System.setProperty("newrelic.config.obfuscate_jvm_props.block", "-Dprop.A");
+        List<String> props = exampleJvmProps();
+        List<String> obfuscatedProps = getObfuscatedProps(props);
+
+        Assert.assertEquals(props.size(), obfuscatedProps.size());
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.A=obfuscated"));
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.B=two"));
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.B.extended=three"));
+        Assert.assertTrue(obfuscatedProps.contains("-Dprop.C=obfuscated"));
+        Assert.assertTrue(obfuscatedProps.contains("-javaagent:1234562"));
+    }
+
+    private List<String> exampleJvmProps() {
+        List<String> props = new ArrayList<>();
+        props.add("-Dprop.A=one");
+        props.add("-Dprop.B=two");
+        props.add("-Dprop.B.extended=three");
+        props.add("-Dprop.C=four");
+        props.add("-javaagent:1234562");
+        return props;
+    }
+
+    private List<String> getObfuscatedProps(List<String> props) {
+        AgentConfig config = AgentConfigFactory.createAgentConfig(null, null, null);
+        Environment env = new Environment(config, "c:\\test\\log");
+        return env.obfuscateProps(props, config.getObfuscateJvmPropsConfig());
+    }
 }