Improve compatibility with SecurityManager #7983
Conversation
|
I added to SIG agenda, just want to talk through and understand this part better:
|
laurit
force-pushed
the
security-manager
branch
from
70b1cd2
to
43133e0
Compare
javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/AgentInitializer.java
Outdated
Show resolved
Hide resolved
javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/OpenTelemetryAgent.java
Show resolved
Hide resolved
| // 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); | ||
| JarFile agentJar = ((JarURLConnection) url.openConnection()).getJarFile(); |
There was a problem hiding this comment.
does this now reuse the underlying JarFile that was already opened for reading OpenTelemetryAgent class file? and by doing so bypass (re-)verification?
There was a problem hiding this comment.
I thought this would have access to an already open JarFile, but debugger shows that it is opening a new one. Reverted this back to new JarFile(javaagentFile, false);
There are more places where jar files are open with verification. When the main agent class (OpenTelemetryAgent) is loaded then the agent jar is open with verification. I think this could be avoided by adding the agent jar to boot classpath, could use Boot-Class-Path manifest attribute. When byte buddy starts retransforming then class file locators call getResourceAsStream on BootstrapClassLoaderProxy which results in jar verification (adding agent to boot loader should also fix these).
If you are serious about speeding up jdk8 startup with jar verification then your best bet is to enable the delay agent start thing in AgentInitializer for all jdk8.
There was a problem hiding this comment.
If you are serious about speeding up jdk8 startup with jar verification then your best bet is to enable the delay agent start thing in
AgentInitializerfor all jdk8.
oh, good idea, I'll try this out, thx
javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/AgentClassLoader.java
Outdated
Show resolved
Hide resolved
javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentStarterImpl.java
Show resolved
Hide resolved
javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ExtensionClassLoader.java
Outdated
Show resolved
Hide resolved
muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java
Outdated
Show resolved
Hide resolved
smoke-tests/src/test/groovy/io/opentelemetry/smoketest/SecurityManagerSmokeTest.groovy
Show resolved
Hide resolved
|
@trask The final remaining issue is where should we document this flag and add the disclaimer. Any ideas? |
I forgot this existed, but could at least go here: https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/docs/advanced-configuration-options.md |
javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/AgentClassLoader.java
Outdated
Show resolved
Hide resolved
| [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. |
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
Instrumentationthat 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 usesAccessController.doPrivilegedwhich basically means that, hey I have done all the checks, run this call with my privileges and ignore the privileges of my callers.