Improve compatibility with SecurityManager (#7983)

This pr gives classes defined in agent and extension class loaders all
permissions. Injected helper classes are also defined with all
permissions. Agent startup is altered so that we won't call methods that
require permission before we are able to get those permissions.
This pr does not attempt to address issues where agent code could allow
user code to circumvent security manager e.g.
https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InstrumentationHolder.java
gives access to `Instrumentation` that could be used to redefine classes
and remove security checks. Also this pr does not address failed
permission checks that could arise from user code calling agent code.
When user code, that does not have privileges, calls agent code, that
has the privileges, and agent code performs a sensitive operation then
permission check would fail because it is performed for all calling
classes, including the user classes. To fix this agent code should uses
`AccessController.doPrivileged` which basically means that, hey I have
done all the checks, run this call with my privileges and ignore the
privileges of my callers.

docs/advanced-configuration-options.md

@@ -17,3 +17,14 @@ which could have unknown side-effects.
 | System property                | Environment variable           | Purpose                                                                                           |
 |--------------------------------|--------------------------------|---------------------------------------------------------------------------------------------------|
 | otel.javaagent.exclude-classes | OTEL_JAVAAGENT_EXCLUDE_CLASSES | Suppresses all instrumentation for specific classes, format is "my.package.MyClass,my.package2.*" |
+
+## Running application with security manager
+
+This option can be used to let agent run with all privileges without being affected by security policy restricting some operations.
+
+| System property                                              | Environment variable                                         | Purpose                               |
+|--------------------------------------------------------------|--------------------------------------------------------------|---------------------------------------|
+| otel.javaagent.experimental.security-manager-support.enabled | OTEL_JAVAAGENT_EXPERIMENTAL_SECURITY_MANAGER_SUPPORT_ENABLED | Grant all privileges to agent code[1] |
+
+[1] Disclaimer: agent can provide application means for escaping security manager sandbox. Do not use
+this option if your application relies on security manager to run untrusted code.

javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/OpenTelemetryAgent.java

@@ -12,7 +12,7 @@ import java.io.File;
 import java.io.IOException;
 import java.lang.instrument.Instrumentation;
 import java.net.URISyntaxException;
-import java.security.CodeSource;
+import java.net.URL;
 import java.util.jar.JarFile;
 import java.util.jar.Manifest;
 
@@ -64,21 +64,31 @@ public final class OpenTelemetryAgent {
 
   private static synchronized File installBootstrapJar(Instrumentation inst)
       throws IOException, URISyntaxException {
-
-    CodeSource codeSource = OpenTelemetryAgent.class.getProtectionDomain().getCodeSource();
-
-    if (codeSource == null) {
-      throw new IllegalStateException("could not get agent jar location");
+    // we are not using OpenTelemetryAgent.class.getProtectionDomain().getCodeSource() to get agent
+    // location because getProtectionDomain does a permission check with security manager
+    ClassLoader classLoader = OpenTelemetryAgent.class.getClassLoader();
+    if (classLoader == null) {
+      classLoader = ClassLoader.getSystemClassLoader();
     }
-
-    File javaagentFile = new File(codeSource.getLocation().toURI());
+    URL url =
+        classLoader.getResource(OpenTelemetryAgent.class.getName().replace('.', '/') + ".class");
+    if (url == null || !"jar".equals(url.getProtocol())) {
+      throw new IllegalStateException("could not get agent jar location from url " + url);
+    }
+    String resourcePath = url.toURI().getSchemeSpecificPart();
+    int protocolSeparatorIndex = resourcePath.indexOf(":");
+    int resourceSeparatorIndex = resourcePath.indexOf("!/");
+    if (protocolSeparatorIndex == -1 || resourceSeparatorIndex == -1) {
+      throw new IllegalStateException("could not get agent location from url " + url);
+    }
+    String agentPath = resourcePath.substring(protocolSeparatorIndex + 1, resourceSeparatorIndex);
+    File javaagentFile = new File(agentPath);
 
     if (!javaagentFile.isFile()) {
       throw new IllegalStateException(
           "agent jar location doesn't appear to be a file: " + javaagentFile.getAbsolutePath());
     }
 
-    // passing verify false for vendors who sign the agent jar, because jar file signature
     // verification is very slow before the JIT compiler starts up, which on Java 8 is not until
     // after premain execution completes
     JarFile agentJar = new JarFile(javaagentFile, false);

javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/AgentClassLoader.java

@@ -16,8 +16,11 @@ import java.net.URL;
 import java.net.URLClassLoader;
 import java.net.URLConnection;
 import java.net.URLStreamHandler;
+import java.security.AllPermission;
 import java.security.CodeSource;
 import java.security.Permission;
+import java.security.PermissionCollection;
+import java.security.Permissions;
 import java.security.cert.Certificate;
 import java.util.Enumeration;
 import java.util.jar.JarEntry;
@@ -63,15 +66,24 @@ public class AgentClassLoader extends URLClassLoader {
   private final URL jarBase;
   private final String jarEntryPrefix;
   private final CodeSource codeSource;
+  private final boolean isSecurityManagerSupportEnabled;
   private final Manifest manifest;
 
+  // Used by tests
+  public AgentClassLoader(File javaagentFile) {
+    this(javaagentFile, "", false);
+  }
+
   /**
    * Construct a new AgentClassLoader.
    *
    * @param javaagentFile Used for resource lookups.
    * @param internalJarFileName File name of the internal jar
+   * @param isSecurityManagerSupportEnabled Whether this class loader should define classes with all
+   *     permissions
    */
-  public AgentClassLoader(File javaagentFile, String internalJarFileName) {
+  public AgentClassLoader(
+      File javaagentFile, String internalJarFileName, boolean isSecurityManagerSupportEnabled) {
     super(new URL[] {}, getParentClassLoader());
     if (javaagentFile == null) {
       throw new IllegalArgumentException("Agent jar location should be set");
@@ -80,6 +92,7 @@ public class AgentClassLoader extends URLClassLoader {
       throw new IllegalArgumentException("Internal jar file name should be set");
     }
 
+    this.isSecurityManagerSupportEnabled = isSecurityManagerSupportEnabled;
     bootstrapProxy = new BootstrapClassLoaderProxy(this);
 
     jarEntryPrefix =
@@ -176,6 +189,17 @@ public class AgentClassLoader extends URLClassLoader {
     return defineClass(name, bytes, 0, bytes.length, codeSource);
   }
 
+  @Override
+  protected PermissionCollection getPermissions(CodeSource codeSource) {
+    if (isSecurityManagerSupportEnabled) {
+      Permissions permissions = new Permissions();
+      permissions.add(new AllPermission());
+      return permissions;
+    }
+
+    return super.getPermissions(codeSource);
+  }
+
   private byte[] getJarEntryBytes(JarEntry jarEntry) throws IOException {
     int size = (int) jarEntry.getSize();
     byte[] buffer = new byte[size];

javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/AgentInitializer.java

@@ -5,9 +5,12 @@
 
 package io.opentelemetry.javaagent.bootstrap;
 
+import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil;
 import java.io.File;
 import java.lang.instrument.Instrumentation;
 import java.lang.reflect.Constructor;
+import java.security.PrivilegedAction;
+import java.security.PrivilegedExceptionAction;
 import javax.annotation.Nullable;
 
 /**
@@ -22,6 +25,7 @@ public final class AgentInitializer {
 
   @Nullable private static ClassLoader agentClassLoader = null;
   @Nullable private static AgentStarter agentStarter = null;
+  private static boolean isSecurityManagerSupportEnabled = false;
 
   public static void initialize(Instrumentation inst, File javaagentFile, boolean fromPremain)
       throws Exception {
@@ -29,11 +33,63 @@ public final class AgentInitializer {
       return;
     }
 
-    agentClassLoader = createAgentClassLoader("inst", javaagentFile);
-    agentStarter = createAgentStarter(agentClassLoader, inst, javaagentFile);
-    if (!fromPremain || !delayAgentStart()) {
-      agentStarter.start();
+    // we expect that at this point agent jar has been appended to boot class path and all agent
+    // classes are loaded in boot loader
+    if (AgentInitializer.class.getClassLoader() != null) {
+      throw new IllegalStateException("agent initializer should be loaded in boot loader");
     }
+
+    isSecurityManagerSupportEnabled = isSecurityManagerSupportEnabled();
+
+    // this call deliberately uses anonymous class instead of lambda because using lambdas too
+    // early on early jdk8 (see isEarlyOracle18 method) causes jvm to crash. See CrashEarlyJdk8Test.
+    execute(
+        new PrivilegedExceptionAction<Void>() {
+          @Override
+          public Void run() throws Exception {
+            agentClassLoader = createAgentClassLoader("inst", javaagentFile);
+            agentStarter = createAgentStarter(agentClassLoader, inst, javaagentFile);
+            if (!fromPremain || !delayAgentStart()) {
+              agentStarter.start();
+            }
+            return null;
+          }
+        });
+  }
+
+  private static void execute(PrivilegedExceptionAction<Void> action) throws Exception {
+    if (isSecurityManagerSupportEnabled && System.getSecurityManager() != null) {
+      doPrivilegedExceptionAction(action);
+    } else {
+      action.run();
+    }
+  }
+
+  private static boolean isSecurityManagerSupportEnabled() {
+    return getBoolean("otel.javaagent.experimental.security-manager-support.enabled", false);
+  }
+
+  private static boolean getBoolean(String property, boolean defaultValue) {
+    // this call deliberately uses anonymous class instead of lambda because using lambdas too
+    // early on early jdk8 (see isEarlyOracle18 method) causes jvm to crash. See CrashEarlyJdk8Test.
+    return doPrivileged(
+        new PrivilegedAction<Boolean>() {
+          @Override
+          public Boolean run() {
+            return ConfigPropertiesUtil.getBoolean(property, defaultValue);
+          }
+        });
+  }
+
+  @SuppressWarnings({"deprecation", "removal"}) // AccessController is deprecated
+  private static <T> T doPrivilegedExceptionAction(PrivilegedExceptionAction<T> action)
+      throws Exception {
+    return java.security.AccessController.doPrivileged(action);
+  }
+
+  @SuppressWarnings({"deprecation", "removal"}) // AccessController is deprecated
+  private static <T> T doPrivileged(PrivilegedAction<T> action) {
+    return java.security.AccessController.doPrivileged(action);
   }
 
   /**
@@ -81,8 +137,17 @@ public final class AgentInitializer {
    * Call to this method is inserted into {@code sun.launcher.LauncherHelper.checkAndLoadMain()}.
    */
   @SuppressWarnings("unused")
-  public static void delayedStartHook() {
-    agentStarter.start();
+  public static void delayedStartHook() throws Exception {
+    // this call deliberately uses anonymous class instead of lambda because using lambdas too
+    // early on early jdk8 (see isEarlyOracle18 method) causes jvm to crash. See CrashEarlyJdk8Test.
+    execute(
+        new PrivilegedExceptionAction<Void>() {
+          @Override
+          public Void run() {
+            agentStarter.start();
+            return null;
+          }
+        });
   }
 
   public static ClassLoader getExtensionsClassLoader() {
@@ -99,7 +164,7 @@ public final class AgentInitializer {
    * @return Agent Classloader
    */
   private static ClassLoader createAgentClassLoader(String innerJarFilename, File javaagentFile) {
-    return new AgentClassLoader(javaagentFile, innerJarFilename);
+    return new AgentClassLoader(javaagentFile, innerJarFilename, isSecurityManagerSupportEnabled);
   }
 
   private static AgentStarter createAgentStarter(
@@ -108,8 +173,9 @@ public final class AgentInitializer {
     Class<?> starterClass =
         agentClassLoader.loadClass("io.opentelemetry.javaagent.tooling.AgentStarterImpl");
     Constructor<?> constructor =
-        starterClass.getDeclaredConstructor(Instrumentation.class, File.class);
-    return (AgentStarter) constructor.newInstance(instrumentation, javaagentFile);
+        starterClass.getDeclaredConstructor(Instrumentation.class, File.class, boolean.class);
+    return (AgentStarter)
+        constructor.newInstance(instrumentation, javaagentFile, isSecurityManagerSupportEnabled);
   }
 
   private AgentInitializer() {}

javaagent-bootstrap/src/test/groovy/io/opentelemetry/javaagent/bootstrap/AgentClassLoaderTest.groovy

@@ -19,7 +19,7 @@ class AgentClassLoaderTest extends Specification {
     def className2 = 'some/class/Name2'
     // any jar would do, use opentelemety sdk
     URL testJarLocation = JavaVersionSpecific.getProtectionDomain().getCodeSource().getLocation()
-    AgentClassLoader loader = new AgentClassLoader(new File(testJarLocation.toURI()), "")
+    AgentClassLoader loader = new AgentClassLoader(new File(testJarLocation.toURI()))
     Phaser threadHoldLockPhase = new Phaser(2)
     Phaser acquireLockFromMainThreadPhase = new Phaser(2)
 
@@ -58,7 +58,7 @@ class AgentClassLoaderTest extends Specification {
     boolean jdk8 = "1.8" == System.getProperty("java.specification.version")
     // sdk is a multi release jar
     URL multiReleaseJar = JavaVersionSpecific.getProtectionDomain().getCodeSource().getLocation()
-    AgentClassLoader loader = new AgentClassLoader(new File(multiReleaseJar.toURI()), "") {
+    AgentClassLoader loader = new AgentClassLoader(new File(multiReleaseJar.toURI())) {
       @Override
       protected String getClassSuffix() {
         return ""

javaagent-tooling/javaagent-tooling-java9/src/test/groovy/UnsafeTest.groovy

@@ -14,7 +14,7 @@ class UnsafeTest extends Specification {
     setup:
     ByteBuddyAgent.install()
     URL testJarLocation = AgentClassLoader.getProtectionDomain().getCodeSource().getLocation()
-    AgentClassLoader loader = new AgentClassLoader(new File(testJarLocation.toURI()), "")
+    AgentClassLoader loader = new AgentClassLoader(new File(testJarLocation.toURI()))
     UnsafeInitializer.initialize(ByteBuddyAgent.getInstrumentation(), loader, false)
 
     expect:

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentStarterImpl.java

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.javaagent.tooling;
 
+import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil;
 import io.opentelemetry.instrumentation.api.internal.cache.weaklockfree.WeakConcurrentMapCleaner;
 import io.opentelemetry.javaagent.bootstrap.AgentInitializer;
@@ -29,11 +30,16 @@ import org.objectweb.asm.Type;
 public class AgentStarterImpl implements AgentStarter {
   private final Instrumentation instrumentation;
   private final File javaagentFile;
+  private final boolean isSecurityManagerSupportEnabled;
   private ClassLoader extensionClassLoader;
 
-  public AgentStarterImpl(Instrumentation instrumentation, File javaagentFile) {
+  public AgentStarterImpl(
+      Instrumentation instrumentation,
+      File javaagentFile,
+      boolean isSecurityManagerSupportEnabled) {
     this.instrumentation = instrumentation;
     this.javaagentFile = javaagentFile;
+    this.isSecurityManagerSupportEnabled = isSecurityManagerSupportEnabled;
   }
 
   @Override
@@ -61,7 +67,7 @@ public class AgentStarterImpl implements AgentStarter {
 
   @Override
   public void start() {
-    extensionClassLoader = createExtensionClassLoader(getClass().getClassLoader(), javaagentFile);
+    extensionClassLoader = createExtensionClassLoader(getClass().getClassLoader());
 
     String loggerImplementationName = ConfigPropertiesUtil.getString("otel.javaagent.logging");
     // default to the built-in stderr slf4j-simple logger
@@ -88,6 +94,12 @@ public class AgentStarterImpl implements AgentStarter {
       loggingCustomizer.init();
       AgentInstaller.installBytebuddyAgent(instrumentation, extensionClassLoader);
       WeakConcurrentMapCleaner.start();
+
+      // LazyStorage reads system properties. Initialize it here where we have permissions to avoid
+      // failing permission checks when it is initialized from user code.
+      if (System.getSecurityManager() != null) {
+        Context.current();
+      }
     } catch (Throwable t) {
       // this is logged below and not rethrown to avoid logging it twice
       startupError = t;
@@ -112,9 +124,9 @@ public class AgentStarterImpl implements AgentStarter {
     return extensionClassLoader;
   }
 
-  private static ClassLoader createExtensionClassLoader(
-      ClassLoader agentClassLoader, File javaagentFile) {
-    return ExtensionClassLoader.getInstance(agentClassLoader, javaagentFile);
+  private ClassLoader createExtensionClassLoader(ClassLoader agentClassLoader) {
+    return ExtensionClassLoader.getInstance(
+        agentClassLoader, javaagentFile, isSecurityManagerSupportEnabled);
   }
 
   private static class LaunchHelperClassFileTransformer implements ClassFileTransformer {

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ExtensionClassLoader.java

@@ -15,6 +15,10 @@ import java.net.URLClassLoader;
 import java.nio.channels.Channels;
 import java.nio.channels.ReadableByteChannel;
 import java.nio.file.Files;
+import java.security.AllPermission;
+import java.security.CodeSource;
+import java.security.PermissionCollection;
+import java.security.Permissions;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Enumeration;
@@ -37,6 +41,8 @@ import net.bytebuddy.dynamic.loading.MultipleParentClassLoader;
 public class ExtensionClassLoader extends URLClassLoader {
   public static final String EXTENSIONS_CONFIG = "otel.javaagent.extensions";
 
+  private final boolean isSecurityManagerSupportEnabled;
+
   // NOTE it's important not to use logging in this class, because this class is used before logging
   // is initialized
 
@@ -44,7 +50,8 @@ public class ExtensionClassLoader extends URLClassLoader {
     ClassLoader.registerAsParallelCapable();
   }
 
-  public static ClassLoader getInstance(ClassLoader parent, File javaagentFile) {
+  public static ClassLoader getInstance(
+      ClassLoader parent, File javaagentFile, boolean isSecurityManagerSupportEnabled) {
     List<URL> extensions = new ArrayList<>();
 
     includeEmbeddedExtensionsIfFound(parent, extensions, javaagentFile);
@@ -69,7 +76,7 @@ public class ExtensionClassLoader extends URLClassLoader {
 
     List<ClassLoader> delegates = new ArrayList<>(extensions.size());
     for (URL url : extensions) {
-      delegates.add(getDelegate(parent, url));
+      delegates.add(getDelegate(parent, url, isSecurityManagerSupportEnabled));
     }
     return new MultipleParentClassLoader(parent, delegates);
   }
@@ -119,8 +126,9 @@ public class ExtensionClassLoader extends URLClassLoader {
     return tempDirectory;
   }
 
-  private static URLClassLoader getDelegate(ClassLoader parent, URL extensionUrl) {
-    return new ExtensionClassLoader(new URL[] {extensionUrl}, parent);
+  private static URLClassLoader getDelegate(
+      ClassLoader parent, URL extensionUrl, boolean isSecurityManagerSupportEnabled) {
+    return new ExtensionClassLoader(extensionUrl, parent, isSecurityManagerSupportEnabled);
   }
 
   // visible for testing
@@ -179,7 +187,19 @@ public class ExtensionClassLoader extends URLClassLoader {
     }
   }
 
-  private ExtensionClassLoader(URL[] urls, ClassLoader parent) {
-    super(urls, parent);
+  @Override
+  protected PermissionCollection getPermissions(CodeSource codesource) {
+    if (isSecurityManagerSupportEnabled) {
+      Permissions permissions = new Permissions();
+      permissions.add(new AllPermission());
+      return permissions;
+    }
+    return super.getPermissions(codesource);
+  }
+
+  private ExtensionClassLoader(
+      URL url, ClassLoader parent, boolean isSecurityManagerSupportEnabled) {
+    super(new URL[] {url}, parent);
+    this.isSecurityManagerSupportEnabled = isSecurityManagerSupportEnabled;
   }
 }

muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java

@@ -17,6 +17,7 @@ import java.io.IOException;
 import java.lang.instrument.Instrumentation;
 import java.net.URL;
 import java.nio.file.Files;
+import java.security.PrivilegedAction;
 import java.security.ProtectionDomain;
 import java.security.SecureClassLoader;
 import java.util.Collection;
@@ -54,6 +55,9 @@ public class HelperInjector implements Transformer {
   private static final TransformSafeLogger logger =
       TransformSafeLogger.getLogger(HelperInjector.class);
 
+  private static final ProtectionDomain PROTECTION_DOMAIN =
+      HelperInjector.class.getProtectionDomain();
+
   // a hook for static instrumentation used to save additional classes created by the agent
   // see https://github.com/open-telemetry/opentelemetry-java-contrib/tree/main/static-instrumenter
   private static volatile HelperInjectorListener helperInjectorListener;
@@ -385,10 +389,24 @@ public class HelperInjector implements Transformer {
     }
 
     Class<?> inject(ClassLoader classLoader, String className) {
+      // if security manager is present byte buddy calls
+      // checkPermission(new ReflectPermission("suppressAccessChecks")) so we must call class
+      // injection with AccessController.doPrivileged when security manager is enabled
       Map<String, Class<?>> result =
-          new ClassInjector.UsingReflection(classLoader)
-              .injectRaw(Collections.singletonMap(className, bytes.get()));
+          execute(
+              () ->
+                  new ClassInjector.UsingReflection(classLoader, PROTECTION_DOMAIN)
+                      .injectRaw(Collections.singletonMap(className, bytes.get())));
       return result.get(className);
     }
   }
+
+  @SuppressWarnings({"deprecation", "removal"}) // AccessController is deprecated
+  private static <T> T execute(PrivilegedAction<T> action) {
+    if (System.getSecurityManager() != null) {
+      return java.security.AccessController.doPrivileged(action);
+    } else {
+      return action.run();
+    }
+  }
 }

smoke-tests/src/test/groovy/io/opentelemetry/smoketest/SecurityManagerSmokeTest.groovy

@@ -0,0 +1,42 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.smoketest
+
+import io.opentelemetry.proto.collector.trace.v1.ExportTraceServiceRequest
+import spock.lang.IgnoreIf
+import spock.lang.Unroll
+
+import static io.opentelemetry.smoketest.TestContainerManager.useWindowsContainers
+
+@IgnoreIf({ useWindowsContainers() })
+class SecurityManagerSmokeTest extends SmokeTest {
+
+  @Override
+  protected String getTargetImage(String jdk) {
+    "ghcr.io/open-telemetry/opentelemetry-java-instrumentation/smoke-test-security-manager:jdk$jdk-20230323.4502979551"
+  }
+
+  @Override
+  protected Map<String, String> getExtraEnv() {
+    return Collections.singletonMap("OTEL_JAVAAGENT_EXPERIMENTAL_SECURITY_MANAGER_SUPPORT_ENABLED", "true")
+  }
+
+  @Unroll
+  def "security manager smoke test on JDK #jdk"(int jdk) {
+    setup:
+    startTarget(jdk)
+
+    expect:
+    Collection<ExportTraceServiceRequest> traces = waitForTraces()
+    countSpansByName(traces, 'test') == 1
+
+    cleanup:
+    stopTarget()
+
+    where:
+    jdk << [8, 11, 17, 19]
+  }
+}