Merge pull request #1453 from newrelic/1447-security-agent-excludes

Add logic to remove default class_transformer.excludes when security …

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

@@ -7,9 +7,13 @@
 
 package com.newrelic.agent.config;
 
+import com.google.common.collect.Sets;
 import com.newrelic.api.agent.Config;
 import com.newrelic.api.agent.NewRelic;
 
+import java.util.Collections;
+import java.util.Set;
+
 /* Default config should look like:
  *
  * security:
@@ -48,6 +52,8 @@ public class SecurityAgentConfig {
     private static final Config config = NewRelic.getAgent().getConfig();
     private static final String ENABLED = "enabled";
     private static final String DISABLED = "disabled";
+    public static final Set<String> SECURITY_AGENT_CLASS_TRANSFORMER_EXCLUDES_TO_IGNORE = Collections.unmodifiableSet(
+            Sets.newHashSet("^java/security/.*", "^javax/crypto/.*", "^net/sf/saxon.*"));
 
     /**
      * Create supportability metrics showing the enabled status of the security agent.

newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/ClassNameFilter.java

@@ -23,6 +23,9 @@
 import java.util.Set;
 import java.util.regex.Pattern;
 
+import static com.newrelic.agent.config.SecurityAgentConfig.SECURITY_AGENT_CLASS_TRANSFORMER_EXCLUDES_TO_IGNORE;
+import static com.newrelic.agent.config.SecurityAgentConfig.shouldInitializeSecurityAgent;
+
 /**
  * This filter is used to skip certain classes during the classloading transform callback based on the class name.
  */
@@ -38,8 +41,13 @@ public ClassNameFilter(IAgentLogger logger) {
     }
 
     /**
-     * Is the class excluded.
-     * 
+     * Check if a class should be excluded from class transformation based on an excludes rule.
+     * <p>
+     * excludePatterns is a list of exclude rules merged together from user config and the default META-INF/excludes file.
+     * Exclude rules are evaluated when determining whether to transform weaved classes in the InstrumentationContextManager
+     * or pointcuts in the PointCutClassTransformer.
+     *
+     * @param className name of the class to check exclusion rule for
      * @return <code>true</code> if this is an excluded class, <code>false</code> if not
      */
     public boolean isExcluded(String className) {
@@ -53,7 +61,7 @@ public boolean isExcluded(String className) {
 
     /**
      * Is the class included.
-     * 
+     *
      * @return <code>true</code> if this is an included class, <code>false</code> if not
      */
     public boolean isIncluded(String className) {
@@ -79,6 +87,10 @@ public void addConfigClassFilters(AgentConfig config) {
             for (String exclude : excludes) {
                 sb.append("\n").append(exclude);
                 addExclude(exclude);
+                if (SECURITY_AGENT_CLASS_TRANSFORMER_EXCLUDES_TO_IGNORE.contains(exclude)) {
+                    logger.finer(exclude + " class_transformer exclude explicitly added by user config. The user configured exclude rule will" +
+                            " take precedence and will not be ignored due to the security agent being enabled.");
+                }
             }
             logger.finer(sb.toString());
         }
@@ -89,7 +101,7 @@ public void addConfigClassFilters(AgentConfig config) {
     }
 
     /**
-     * Add excluded classes in the excludes file.
+     * Add excluded classes in the META-INF/excludes file.
      */
     public void addExcludeFileClassFilters() {
         InputStream iStream = this.getClass().getResourceAsStream(EXCLUDES_FILE);
@@ -112,15 +124,34 @@ public void addExcludeFileClassFilters() {
                 // ignore
             }
         }
+        boolean ignoreExcludeForSecurityAgent = false;
+        String formattedIgnoredExcludes = String.join(",", SECURITY_AGENT_CLASS_TRANSFORMER_EXCLUDES_TO_IGNORE);
+        boolean shouldInitializeSecurityAgent = shouldInitializeSecurityAgent();
+
         for (String exclude : excludeList) {
-            addExclude(exclude);
+            ignoreExcludeForSecurityAgent = shouldInitializeSecurityAgent && SECURITY_AGENT_CLASS_TRANSFORMER_EXCLUDES_TO_IGNORE.contains(exclude);
+            if (ignoreExcludeForSecurityAgent) {
+                logger.finer("Ignored " + exclude + " class_transformer exclude defined in META-INF/excludes because the security agent is enabled. " +
+                        "This can be overridden by explicitly setting newrelic.config.class_transformer.excludes=" + formattedIgnoredExcludes +
+                        " or NEW_RELIC_CLASS_TRANSFORMER_EXCLUDES=" + formattedIgnoredExcludes);
+            } else {
+                addExclude(exclude);
+            }
+        }
+
+        if (ignoreExcludeForSecurityAgent) {
+            for (String exclude : SECURITY_AGENT_CLASS_TRANSFORMER_EXCLUDES_TO_IGNORE) {
+                // Remove the default exclude rule if the security agent is enabled. If a user explicitly adds one of these exclude rules via agent config
+                // then it will be added back to the list via ClassNameFilter.addConfigClassFilters, effectively taking precedence over this removal logic.
+                excludeList.remove(exclude);
+            }
         }
         logger.finer("Excludes initialized: " + excludeList);
     }
 
     /**
      * Include matching classes.
-     * 
+     *
      * @param include either a class name or a regular expression
      */
     public void addInclude(String include) {
@@ -133,7 +164,7 @@ public void addInclude(String include) {
 
     /**
      * Include the given class.
-     * 
+     *
      * @param className the name of the class to include
      */
     public void addIncludeClass(String className) {
@@ -143,7 +174,7 @@ public void addIncludeClass(String className) {
 
     /**
      * Include classes matching the regular expression.
-     * 
+     *
      * @param regex a regular expression matching classes to include
      */
     public void addIncludeRegex(String regex) {
@@ -155,7 +186,7 @@ public void addIncludeRegex(String regex) {
 
     /**
      * Exclude matching classes.
-     * 
+     *
      * @param exclude either a class name or a regular expression
      */
     public void addExclude(String exclude) {
@@ -168,7 +199,7 @@ public void addExclude(String exclude) {
 
     /**
      * Exclude the given class.
-     * 
+     *
      * @param className the name of the class to exclude
      */
     public void addExcludeClass(String className) {
@@ -178,7 +209,7 @@ public void addExcludeClass(String className) {
 
     /**
      * Exclude classes matching the regular expression.
-     * 
+     *
      * @param regex a regular expression matching classes to include
      */
     public void addExcludeRegex(String regex) {
@@ -190,7 +221,7 @@ public void addExcludeRegex(String regex) {
 
     /**
      * Create a regular expression for a class.
-     * 
+     *
      * @return a regular expression that matches only the given class
      */
     private String classNameToRegex(String className) {
@@ -199,7 +230,7 @@ private String classNameToRegex(String className) {
 
     /**
      * Compile a regular expression.
-     * 
+     *
      * @return a pattern or null if the regular expression could not be compiled
      */
     private Pattern regexToPattern(String regex) {
@@ -214,7 +245,7 @@ private Pattern regexToPattern(String regex) {
 
     /**
      * Is the argument a regular expression (rather than a class name).
-     * 
+     *
      * @param value
      * @return <code>true</code> if this is a regular expression, <code>false</code> if not
      */

newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationContextManager.java

@@ -31,7 +31,6 @@
 
 import java.lang.instrument.ClassFileTransformer;
 import java.lang.instrument.Instrumentation;
-import java.text.MessageFormat;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
@@ -220,11 +219,6 @@ public boolean shouldTransform(final String internalClassName, final ClassLoader
             Agent.LOG.log(Level.FINEST, "Skipping transform of {0}. Classloader {1} is excluded.", internalClassName, classloader);
             return false;
         }
-        if (internalClassName.startsWith("javax/crypto/")) {
-            // crypto classes can cause class circularity errors if they get too far along in the class transformer
-            Agent.LOG.finest(MessageFormat.format("Instrumentation skipped by ''javax crypto'' rule: {0}", internalClassName));
-            return false;
-        }
         if (classNameFilter.isIncluded(internalClassName)) {
             Agent.LOG.log(Level.FINEST, "Class {0} is explicitly included", internalClassName);
             return true;

newrelic-agent/src/main/resources/META-INF/excludes

@@ -57,10 +57,6 @@
 # adding exclusions for obfuscated jar - see ticket 197106
 ^asposewobfuscated/.*
 ^com/aspose/words/.*
-# saxon xlst classes - see ticket 195949
-^net/sf/saxon.*
-# exclude java.security and sun.reflect classes from transformation
-^java/security/.*
 ^sun/reflect/.*
 # exclude lambdas by excluding java.lang.invoke.* classes
 ^java/lang/invoke/.*
@@ -94,3 +90,10 @@
 ^java/util/stream/MatchOps\$MatchOp
 ^java/util/stream/MatchOps\$BooleanTerminalSink
 ^javax/security/auth/Subject\$SecureSet\$1
+### All default excludes listed below get ignored when the security agent is enabled
+# exclude java.security and sun.reflect classes from transformation
+^java/security/.*
+# crypto classes can cause class circularity errors if they get too far along in the class transformer
+^javax/crypto/.*
+# saxon xlst classes - see ticket 195949
+^net/sf/saxon.*