Skip to content
This repository was archived by the owner on Feb 1, 2024. It is now read-only.

Commit 2f0819a

Browse files
authored
Improve compatibility with SecurityManager (open-telemetry#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.
1 parent e0ecb56 commit 2f0819a

File tree

10 files changed

+238
-35
lines changed

10 files changed

+238
-35
lines changed

docs/advanced-configuration-options.md

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

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

+19-9
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
import java.io.IOException;
1313
import java.lang.instrument.Instrumentation;
1414
import java.net.URISyntaxException;
15-
import java.security.CodeSource;
15+
import java.net.URL;
1616
import java.util.jar.JarFile;
1717
import java.util.jar.Manifest;
1818

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

6565
private static synchronized File installBootstrapJar(Instrumentation inst)
6666
throws IOException, URISyntaxException {
67-
68-
CodeSource codeSource = OpenTelemetryAgent.class.getProtectionDomain().getCodeSource();
69-
70-
if (codeSource == null) {
71-
throw new IllegalStateException("could not get agent jar location");
67+
// we are not using OpenTelemetryAgent.class.getProtectionDomain().getCodeSource() to get agent
68+
// location because getProtectionDomain does a permission check with security manager
69+
ClassLoader classLoader = OpenTelemetryAgent.class.getClassLoader();
70+
if (classLoader == null) {
71+
classLoader = ClassLoader.getSystemClassLoader();
7272
}
73-
74-
File javaagentFile = new File(codeSource.getLocation().toURI());
73+
URL url =
74+
classLoader.getResource(OpenTelemetryAgent.class.getName().replace('.', '/') + ".class");
75+
if (url == null || !"jar".equals(url.getProtocol())) {
76+
throw new IllegalStateException("could not get agent jar location from url " + url);
77+
}
78+
String resourcePath = url.toURI().getSchemeSpecificPart();
79+
int protocolSeparatorIndex = resourcePath.indexOf(":");
80+
int resourceSeparatorIndex = resourcePath.indexOf("!/");
81+
if (protocolSeparatorIndex == -1 || resourceSeparatorIndex == -1) {
82+
throw new IllegalStateException("could not get agent location from url " + url);
83+
}
84+
String agentPath = resourcePath.substring(protocolSeparatorIndex + 1, resourceSeparatorIndex);
85+
File javaagentFile = new File(agentPath);
7586

7687
if (!javaagentFile.isFile()) {
7788
throw new IllegalStateException(
7889
"agent jar location doesn't appear to be a file: " + javaagentFile.getAbsolutePath());
7990
}
8091

81-
// passing verify false for vendors who sign the agent jar, because jar file signature
8292
// verification is very slow before the JIT compiler starts up, which on Java 8 is not until
8393
// after premain execution completes
8494
JarFile agentJar = new JarFile(javaagentFile, false);

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

+25-1
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,11 @@
1616
import java.net.URLClassLoader;
1717
import java.net.URLConnection;
1818
import java.net.URLStreamHandler;
19+
import java.security.AllPermission;
1920
import java.security.CodeSource;
2021
import java.security.Permission;
22+
import java.security.PermissionCollection;
23+
import java.security.Permissions;
2124
import java.security.cert.Certificate;
2225
import java.util.Enumeration;
2326
import java.util.jar.JarEntry;
@@ -63,15 +66,24 @@ public class AgentClassLoader extends URLClassLoader {
6366
private final URL jarBase;
6467
private final String jarEntryPrefix;
6568
private final CodeSource codeSource;
69+
private final boolean isSecurityManagerSupportEnabled;
6670
private final Manifest manifest;
6771

72+
// Used by tests
73+
public AgentClassLoader(File javaagentFile) {
74+
this(javaagentFile, "", false);
75+
}
76+
6877
/**
6978
* Construct a new AgentClassLoader.
7079
*
7180
* @param javaagentFile Used for resource lookups.
7281
* @param internalJarFileName File name of the internal jar
82+
* @param isSecurityManagerSupportEnabled Whether this class loader should define classes with all
83+
* permissions
7384
*/
74-
public AgentClassLoader(File javaagentFile, String internalJarFileName) {
85+
public AgentClassLoader(
86+
File javaagentFile, String internalJarFileName, boolean isSecurityManagerSupportEnabled) {
7587
super(new URL[] {}, getParentClassLoader());
7688
if (javaagentFile == null) {
7789
throw new IllegalArgumentException("Agent jar location should be set");
@@ -80,6 +92,7 @@ public AgentClassLoader(File javaagentFile, String internalJarFileName) {
8092
throw new IllegalArgumentException("Internal jar file name should be set");
8193
}
8294

95+
this.isSecurityManagerSupportEnabled = isSecurityManagerSupportEnabled;
8396
bootstrapProxy = new BootstrapClassLoaderProxy(this);
8497

8598
jarEntryPrefix =
@@ -176,6 +189,17 @@ public Class<?> defineClass(String name, byte[] bytes) {
176189
return defineClass(name, bytes, 0, bytes.length, codeSource);
177190
}
178191

192+
@Override
193+
protected PermissionCollection getPermissions(CodeSource codeSource) {
194+
if (isSecurityManagerSupportEnabled) {
195+
Permissions permissions = new Permissions();
196+
permissions.add(new AllPermission());
197+
return permissions;
198+
}
199+
200+
return super.getPermissions(codeSource);
201+
}
202+
179203
private byte[] getJarEntryBytes(JarEntry jarEntry) throws IOException {
180204
int size = (int) jarEntry.getSize();
181205
byte[] buffer = new byte[size];

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

+75-9
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,12 @@
55

66
package io.opentelemetry.javaagent.bootstrap;
77

8+
import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil;
89
import java.io.File;
910
import java.lang.instrument.Instrumentation;
1011
import java.lang.reflect.Constructor;
12+
import java.security.PrivilegedAction;
13+
import java.security.PrivilegedExceptionAction;
1114
import javax.annotation.Nullable;
1215

1316
/**
@@ -22,18 +25,71 @@ public final class AgentInitializer {
2225

2326
@Nullable private static ClassLoader agentClassLoader = null;
2427
@Nullable private static AgentStarter agentStarter = null;
28+
private static boolean isSecurityManagerSupportEnabled = false;
2529

2630
public static void initialize(Instrumentation inst, File javaagentFile, boolean fromPremain)
2731
throws Exception {
2832
if (agentClassLoader != null) {
2933
return;
3034
}
3135

32-
agentClassLoader = createAgentClassLoader("inst", javaagentFile);
33-
agentStarter = createAgentStarter(agentClassLoader, inst, javaagentFile);
34-
if (!fromPremain || !delayAgentStart()) {
35-
agentStarter.start();
36+
// we expect that at this point agent jar has been appended to boot class path and all agent
37+
// classes are loaded in boot loader
38+
if (AgentInitializer.class.getClassLoader() != null) {
39+
throw new IllegalStateException("agent initializer should be loaded in boot loader");
3640
}
41+
42+
isSecurityManagerSupportEnabled = isSecurityManagerSupportEnabled();
43+
44+
// this call deliberately uses anonymous class instead of lambda because using lambdas too
45+
// early on early jdk8 (see isEarlyOracle18 method) causes jvm to crash. See CrashEarlyJdk8Test.
46+
execute(
47+
new PrivilegedExceptionAction<Void>() {
48+
@Override
49+
public Void run() throws Exception {
50+
agentClassLoader = createAgentClassLoader("inst", javaagentFile);
51+
agentStarter = createAgentStarter(agentClassLoader, inst, javaagentFile);
52+
if (!fromPremain || !delayAgentStart()) {
53+
agentStarter.start();
54+
}
55+
return null;
56+
}
57+
});
58+
}
59+
60+
private static void execute(PrivilegedExceptionAction<Void> action) throws Exception {
61+
if (isSecurityManagerSupportEnabled && System.getSecurityManager() != null) {
62+
doPrivilegedExceptionAction(action);
63+
} else {
64+
action.run();
65+
}
66+
}
67+
68+
private static boolean isSecurityManagerSupportEnabled() {
69+
return getBoolean("otel.javaagent.experimental.security-manager-support.enabled", false);
70+
}
71+
72+
private static boolean getBoolean(String property, boolean defaultValue) {
73+
// this call deliberately uses anonymous class instead of lambda because using lambdas too
74+
// early on early jdk8 (see isEarlyOracle18 method) causes jvm to crash. See CrashEarlyJdk8Test.
75+
return doPrivileged(
76+
new PrivilegedAction<Boolean>() {
77+
@Override
78+
public Boolean run() {
79+
return ConfigPropertiesUtil.getBoolean(property, defaultValue);
80+
}
81+
});
82+
}
83+
84+
@SuppressWarnings({"deprecation", "removal"}) // AccessController is deprecated
85+
private static <T> T doPrivilegedExceptionAction(PrivilegedExceptionAction<T> action)
86+
throws Exception {
87+
return java.security.AccessController.doPrivileged(action);
88+
}
89+
90+
@SuppressWarnings({"deprecation", "removal"}) // AccessController is deprecated
91+
private static <T> T doPrivileged(PrivilegedAction<T> action) {
92+
return java.security.AccessController.doPrivileged(action);
3793
}
3894

3995
/**
@@ -81,8 +137,17 @@ private static boolean delayAgentStart() {
81137
* Call to this method is inserted into {@code sun.launcher.LauncherHelper.checkAndLoadMain()}.
82138
*/
83139
@SuppressWarnings("unused")
84-
public static void delayedStartHook() {
85-
agentStarter.start();
140+
public static void delayedStartHook() throws Exception {
141+
// this call deliberately uses anonymous class instead of lambda because using lambdas too
142+
// early on early jdk8 (see isEarlyOracle18 method) causes jvm to crash. See CrashEarlyJdk8Test.
143+
execute(
144+
new PrivilegedExceptionAction<Void>() {
145+
@Override
146+
public Void run() {
147+
agentStarter.start();
148+
return null;
149+
}
150+
});
86151
}
87152

88153
public static ClassLoader getExtensionsClassLoader() {
@@ -99,7 +164,7 @@ public static ClassLoader getExtensionsClassLoader() {
99164
* @return Agent Classloader
100165
*/
101166
private static ClassLoader createAgentClassLoader(String innerJarFilename, File javaagentFile) {
102-
return new AgentClassLoader(javaagentFile, innerJarFilename);
167+
return new AgentClassLoader(javaagentFile, innerJarFilename, isSecurityManagerSupportEnabled);
103168
}
104169

105170
private static AgentStarter createAgentStarter(
@@ -108,8 +173,9 @@ private static AgentStarter createAgentStarter(
108173
Class<?> starterClass =
109174
agentClassLoader.loadClass("io.opentelemetry.javaagent.tooling.AgentStarterImpl");
110175
Constructor<?> constructor =
111-
starterClass.getDeclaredConstructor(Instrumentation.class, File.class);
112-
return (AgentStarter) constructor.newInstance(instrumentation, javaagentFile);
176+
starterClass.getDeclaredConstructor(Instrumentation.class, File.class, boolean.class);
177+
return (AgentStarter)
178+
constructor.newInstance(instrumentation, javaagentFile, isSecurityManagerSupportEnabled);
113179
}
114180

115181
private AgentInitializer() {}

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ class AgentClassLoaderTest extends Specification {
1919
def className2 = 'some/class/Name2'
2020
// any jar would do, use opentelemety sdk
2121
URL testJarLocation = JavaVersionSpecific.getProtectionDomain().getCodeSource().getLocation()
22-
AgentClassLoader loader = new AgentClassLoader(new File(testJarLocation.toURI()), "")
22+
AgentClassLoader loader = new AgentClassLoader(new File(testJarLocation.toURI()))
2323
Phaser threadHoldLockPhase = new Phaser(2)
2424
Phaser acquireLockFromMainThreadPhase = new Phaser(2)
2525

@@ -58,7 +58,7 @@ class AgentClassLoaderTest extends Specification {
5858
boolean jdk8 = "1.8" == System.getProperty("java.specification.version")
5959
// sdk is a multi release jar
6060
URL multiReleaseJar = JavaVersionSpecific.getProtectionDomain().getCodeSource().getLocation()
61-
AgentClassLoader loader = new AgentClassLoader(new File(multiReleaseJar.toURI()), "") {
61+
AgentClassLoader loader = new AgentClassLoader(new File(multiReleaseJar.toURI())) {
6262
@Override
6363
protected String getClassSuffix() {
6464
return ""

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ class UnsafeTest extends Specification {
1414
setup:
1515
ByteBuddyAgent.install()
1616
URL testJarLocation = AgentClassLoader.getProtectionDomain().getCodeSource().getLocation()
17-
AgentClassLoader loader = new AgentClassLoader(new File(testJarLocation.toURI()), "")
17+
AgentClassLoader loader = new AgentClassLoader(new File(testJarLocation.toURI()))
1818
UnsafeInitializer.initialize(ByteBuddyAgent.getInstrumentation(), loader, false)
1919

2020
expect:

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

+17-5
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package io.opentelemetry.javaagent.tooling;
77

8+
import io.opentelemetry.context.Context;
89
import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil;
910
import io.opentelemetry.instrumentation.api.internal.cache.weaklockfree.WeakConcurrentMapCleaner;
1011
import io.opentelemetry.javaagent.bootstrap.AgentInitializer;
@@ -29,11 +30,16 @@
2930
public class AgentStarterImpl implements AgentStarter {
3031
private final Instrumentation instrumentation;
3132
private final File javaagentFile;
33+
private final boolean isSecurityManagerSupportEnabled;
3234
private ClassLoader extensionClassLoader;
3335

34-
public AgentStarterImpl(Instrumentation instrumentation, File javaagentFile) {
36+
public AgentStarterImpl(
37+
Instrumentation instrumentation,
38+
File javaagentFile,
39+
boolean isSecurityManagerSupportEnabled) {
3540
this.instrumentation = instrumentation;
3641
this.javaagentFile = javaagentFile;
42+
this.isSecurityManagerSupportEnabled = isSecurityManagerSupportEnabled;
3743
}
3844

3945
@Override
@@ -61,7 +67,7 @@ public boolean delayStart() {
6167

6268
@Override
6369
public void start() {
64-
extensionClassLoader = createExtensionClassLoader(getClass().getClassLoader(), javaagentFile);
70+
extensionClassLoader = createExtensionClassLoader(getClass().getClassLoader());
6571

6672
String loggerImplementationName = ConfigPropertiesUtil.getString("otel.javaagent.logging");
6773
// default to the built-in stderr slf4j-simple logger
@@ -88,6 +94,12 @@ public void start() {
8894
loggingCustomizer.init();
8995
AgentInstaller.installBytebuddyAgent(instrumentation, extensionClassLoader);
9096
WeakConcurrentMapCleaner.start();
97+
98+
// LazyStorage reads system properties. Initialize it here where we have permissions to avoid
99+
// failing permission checks when it is initialized from user code.
100+
if (System.getSecurityManager() != null) {
101+
Context.current();
102+
}
91103
} catch (Throwable t) {
92104
// this is logged below and not rethrown to avoid logging it twice
93105
startupError = t;
@@ -112,9 +124,9 @@ public ClassLoader getExtensionClassLoader() {
112124
return extensionClassLoader;
113125
}
114126

115-
private static ClassLoader createExtensionClassLoader(
116-
ClassLoader agentClassLoader, File javaagentFile) {
117-
return ExtensionClassLoader.getInstance(agentClassLoader, javaagentFile);
127+
private ClassLoader createExtensionClassLoader(ClassLoader agentClassLoader) {
128+
return ExtensionClassLoader.getInstance(
129+
agentClassLoader, javaagentFile, isSecurityManagerSupportEnabled);
118130
}
119131

120132
private static class LaunchHelperClassFileTransformer implements ClassFileTransformer {

0 commit comments

Comments
 (0)