JDK-8177154 : Default configuration should disallow loading agents
  • Type: Enhancement
  • Component: core-svc
  • Sub-Component: tools
  • Affected Version: 9
  • Priority: P2
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2017-03-20
  • Updated: 2017-05-30
  • Resolved: 2017-04-11
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 9
9-repo-jigsawFixed
Related Reports
Relates :  
JDK-8178380 - Module system implementation refresh (5/2017)
Description
As part of making the platform more secure by default, and improving the integrity of the platform, then we need to re-examine the ability to load arbitrary code (both native and java) into a running VM with the attach mechanism.

This issue tracks changing the VM side of the attach mechanism to disallow the "load" command by default. The "load" command is what the VirtualMachine.loadAgentXXX methods use to load java and JVM TI agents into the target VM. A non-manageable command line (-XX:+EnableDynamicAgentLoading or better name) will be needed to allow opt-in and allow agents to be loaded. Note that the VM already has -XX:+DisableAttachMechanism to completely disable the attach mechanism but that disables it completely and prevents the use of the cooperative troubleshooting tools.

A few notes on the proposal:

1. It should have no impact on command-line/troubleshooting tools. 

2. It should have no impact on tools that start the JMX agent with the attach mechanism.

3. No impact on the JVM TI or java.lang.instrument specifications as Java SE does not specify the mechanism, it just allows for the possibility of agents being loaded in a running VM.

4 The changes to implement this are likely to be small and low-risk. The main thing is to make sure that the error on the attach API side is useful.  A small number of existing tests will need to be updated to run with the new XX option.

5. The change should only impact a small number of tools but it will need to be documented in the JDK 9 release notes.


Also a few notes on some of the alternatives that have explored: 

1. Do not resolve the jdk.attach module by default. Unfortunately it's not possible to do this in a reliable way. One reason is that several tools with APIs require jdk.attach. Service binding also complicates this. In short, it's not feasible without completely changing the policy for root modules in JEP 261. Even then, it runs into the same issues as #3 below.

2. Degrading the redefineXXXX and retransformClasses functions/methods so that it can't be used to redefine modules or classes defined to the boot or platform loaders. This helps preserve the integrity of the platform modules but is not completely robust without disallowing core modules to export internal packages to tool modules. Even if these issues are solved then it would still require a command line option to allow the platform classes and modules to be transformed by late binding agents.

3. Disable "self attach". The attach API was never intended to be used this way. It was suggested during the original development to disallow this and has also been looked at several times since then. To date then efforts here have been futile (sneaky libraries will just start a new VM that attaches back to the parent).
Comments
The changes to add the EnableDynamicAgentLoading option have been pushed to the jigsaw/jake forest so that there are EA builds to try out. The EnableDynamicAgentLoading option is on default, will be looked at again in JDK 10. Getting the option into JDK 9 means that launch scripts that use this option on JDK 10 will work on JDK 9 without change. It also allow for early testing of future behavior on JDK 9.
11-04-2017
Further discussion, on jigsaw-dev: http://mail.openjdk.java.net/pipermail/jigsaw-dev/2017-March/011919.html TL;DR: This change will improve integrity, not merely security, and improving integrity is one of the main goals of the entire modularity effort.
30-03-2017
I have already made a request for a better justification of the need to default to disabling dynamic loading in the thread started by an RFR for a proposed fix posted to runtime-dev http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2017-March/022944.html I'm linking that thread here to ensure that this request is not missed and left unanswered. In brief, I would like: i) to see better evidence that there is a significant security hole at stake here that outweighs the inconvenience that would arise from picking this default ii) to be convinced that any such hole is best filled by resetting this default rather than by resorting to existing security measures provided by the underlying OS Example cases would be very welcome.
29-03-2017
I am also worried this will have a negative impact on third party tools. The whole point of attach is to be able to load an agent after the JVM has started. If you have to supply a command line flag you might as weill specify -javaagent directly.
27-03-2017
I've committed a change to improve the message of dynamic JVMTI agent loading in JDK-8164913 . However, it is cause of JPRT faulure, so it has been backouted. This change is working on JDK-8165869 . So I hope this enhancement work together with JDK-8165869 .
27-03-2017
The proposal here is limited to the "load" command and so only impacts tools using the VirtualMachine.loadAgentXXX methods.
23-03-2017
Maybe I am misunderstanding something here but this seems to be taking us back to where we were in Java 5 and earlier before dynamic attach was added in 6 under JDK-6173612. Seems very clear that being able to start the VM without a special flag was a very important part of the utility of this feature, yet now we no longer deem it important? Edit: okay this is only in relation to one aspect of dynamic attach - the "load" command. I'm unclear what API exposes this "load" command and which tools the use it? But the requirement to specify a flag at startup still seems very limiting for the same reasons outlined in 6173612.
23-03-2017
> 1. It should have no impact on command-line/troubleshooting tools. > 5. The change should only impact a small number of tools but it will need to be documented in the JDK 9 release notes. These seem contradictory. What tools does #5 refer to that are not covered by #1?
23-03-2017
Yes, I've just figured out the return with the JNI_ERR is missed.
22-03-2017
For preliminary patch then I assume it would not dispatch and additionally an error would be returned to the tool (maybe JNI_ERR or another error). The main thing to check is the behavior on the tool side so that it can be translated into an appropriate exception.
22-03-2017
A preliminary suggested fix is: diff -r 40ad6af5e434 src/share/vm/runtime/globals.hpp --- a/src/share/vm/runtime/globals.hpp Mon Mar 20 23:49:33 2017 +0100 +++ b/src/share/vm/runtime/globals.hpp Tue Mar 21 23:51:34 2017 -0700 @@ -4043,6 +4043,9 @@ experimental(bool, AlwaysAtomicAccesses, false, \ "Accesses to all variables should always be atomic") \ \ + product(bool, EnableDynamicAgentLoading, false, \ + "Enable dynamic agent loading with the Attach API") \ + \ product(bool, EnableTracing, false, \ "Enable event-based tracing") \ \ diff -r 40ad6af5e434 src/share/vm/services/attachListener.cpp --- a/src/share/vm/services/attachListener.cpp Mon Mar 20 23:49:33 2017 +0100 +++ b/src/share/vm/services/attachListener.cpp Tue Mar 21 23:51:34 2017 -0700 @@ -322,6 +322,12 @@ if (strcmp(op->name(), AttachOperation::detachall_operation_name()) == 0) { AttachListener::detachall(); } else { + if (!EnableDynamicAgentLoading && strcmp(op->name(), "load") == 0) { + // Dynamic loading agents is disallowed in default configuration. + // Provide some usefull error message. + st.print("Dynamic agent loading is disallowed in default configuration! " + "Use flag -XX:+EnableDynamicAgentLoading to launch target VM."); + } // find the function to dispatch too AttachOperationFunctionInfo* info = NULL; for (int i=0; funcs[i].name != NULL; i++) { @@ -340,6 +346,7 @@ if (info != NULL) { // dispatch to the function that implements this operation + res = (info->func)(op, &st); } else { st.print("Operation %s not recognized!", op->name()); Need to make sure if any other spots need tweaks. The tests discussed in previous comments need an update too. Should this bug report be a confidential?
22-03-2017
Got it. Thanks, Alan!
21-03-2017
[~sspitsyn] Q1 - the attach mechanism is local only (no remote). Q2 - as Mandy said, whether there is a security manager on the tool side is not interesting here. Q3 - the proposal covers java agents too.
21-03-2017
It might make sense to disable certain JVMTI capability only when agent is loading into running VM, not entire agent loading.
21-03-2017
This issue proposes to disallow loading agents regardless of the presence of security manager. So the permission check does not really help. Furthermore, to obtain a VirtualMachine to load an agent, VirtualMachine::attach already performs the security check when the security manager is present.
20-03-2017
My understanding is that the "load" command is about loading Native/JVMTI agents only. It seems, it should not impact loading Java agents directly. But the Java agents can be impacted indirectly when the instrument library is loaded. More questions: Q1: How should this work if attaching happens from the same (local) machine? Is this for remote attach only? Q2: What about the attaching processes that have a SecurityManager set? Does it make sense to introduce new AttachPermission named "loadNativeAgent" or having the permissions "attachVirtualMahine" and "createAttachProvider" is enough? Q3: "No impact on the JVM TI or java.lang.instrument specifications" does not mean there is no impact on the JVMTI agents, right?
20-03-2017