Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve compatibility with SecurityManager #7983

Merged
merged 10 commits into from
Apr 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions docs/advanced-configuration-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines +29 to +30
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
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;

Expand Down Expand Up @@ -64,21 +64,31 @@ private static void startAgent(Instrumentation inst, boolean fromPremain) {

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
laurit marked this conversation as resolved.
Show resolved Hide resolved
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,11 @@
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;
Expand Down Expand Up @@ -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");
Expand All @@ -80,6 +92,7 @@ public AgentClassLoader(File javaagentFile, String internalJarFileName) {
throw new IllegalArgumentException("Internal jar file name should be set");
}

this.isSecurityManagerSupportEnabled = isSecurityManagerSupportEnabled;
bootstrapProxy = new BootstrapClassLoaderProxy(this);

jarEntryPrefix =
Expand Down Expand Up @@ -176,6 +189,17 @@ public Class<?> defineClass(String name, byte[] bytes) {
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];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -22,18 +25,71 @@ 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 {
if (agentClassLoader != null) {
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);
}

/**
Expand Down Expand Up @@ -81,8 +137,17 @@ private static boolean delayAgentStart() {
* 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() {
Expand All @@ -99,7 +164,7 @@ public static ClassLoader getExtensionsClassLoader() {
* @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(
Expand All @@ -108,8 +173,9 @@ private static AgentStarter createAgentStarter(
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() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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 ""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -29,11 +30,16 @@
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
Expand Down Expand Up @@ -61,7 +67,7 @@ public boolean delayStart() {

@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
Expand All @@ -88,6 +94,12 @@ public void start() {
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.
laurit marked this conversation as resolved.
Show resolved Hide resolved
if (System.getSecurityManager() != null) {
Context.current();
}
} catch (Throwable t) {
// this is logged below and not rethrown to avoid logging it twice
startupError = t;
Expand All @@ -112,9 +124,9 @@ public ClassLoader getExtensionClassLoader() {
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 {
Expand Down
Loading