RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]

[Alan Bateman]

On 19/04/2021 22:20, Rafael Winterhalter wrote:
> :
> At the moment, it is required for root to switch to the user that owns the
> JVM process as the domain socket is only accessible to that user to avoid
> that users without access to the JVM can inject themselves into a JVM. I am
> not sure if operations teams would be thrilled to have a monitoring agent
> required to run as root, even in these times of Kubernetes.
>
> I mainly have two comments:
>
> 1. The problem is the possibility of self-attach. I think this is the
> problem to solve, a library getting agent privileges without being an
> agent. I think this should be prevented while dynamic attach should
> continue to be possible in today's format. It has proven to be so useful,
> it would be a shame if the current tooling convenience would disappear from
> the JVM. As it's my understanding, JNI is supposed to be restricted in the
> future, in line with Panama. Without this restriction, JNI already allows
> for random class definition, for example, which similarly to an agent
> offers surpassing the majority of JVM restrictions. The second restriction
> would be a control to restrict how a JVM process starts new processes. I
> think both are reasonable restrictions for a library to face which require
> explicit enabling. Especially with the security manager on it's way out,
> certain capabilities should be rethought to begin with. If both are no
> longer freely available, self-attachment is no longer possible anyways and
> dynamic agents could retain their capabilities.
>
> 2. The question of introducing an Instrumentation::defineClass method is
> fully independent of that first question. If a dynamic agent was to be
> restricted, the method could reject classloader/package combinations for
> dynamically loaded agents the same way that
> Instrumentation::retransformClasses would need to. At the same time,
> introducing the method would allow agents to move to an official API with a
> Java 17 baseline which will be the next long-standing base line. I fully
> understand it needs a thorough discussion but it is a less complicated
> problem then (1) and could therefore be decided prior to having found a
> satisfactory solution for it.
I should have been clearer, it's the combination of the two that creates 
the attractive nuisance. I don't think there are any objections to a 
defineClass for agents specified on the command line with -javaagent. 
However we have to be cautious about extending that capability to agents 
that are loaded into a running VM with the attach mechanism.

ByteBuddy looks great for code generation and transforming classes but 
ByteBuddyAgent makes me nervous. It looks like I can deploy 
byte-buddy-agent-<version>.jar on my class path and invoke the public 
static ByteBuddyAgent.install() method to get the Instrumentation object 
for the current VM. That may be convenient for some but this is the 
all-powerful Instrumentation object that shouldn't be leaked to library 
or application code. Now combine this with the proposed defineClass and 
it means that any code on the class path could inject a class into 
java.lang or any run-time package without any agent voodoo or opt-in via 
the command line. That would be difficult genie to re-bottle if it were 
to get traction.

You mentioned restricting JNI in the future. I'm not aware of a definite 
plan or time-frame. Project Panama is pioneering restricting access to 
native operations as a bug or mis-use with the linker API can easily 
crash the VM or breakage in other ways. Extending this to JNI would be a 
logical next step but I could imagine it taking a long time and many 
releases to get there.

As regards this PR then I would be happy to work with you on a revised 
proposed that would limit it to agents specified with -javaagent. That 
would not preclude extending the capability, maybe in a more restricted 
form, to agents loaded into a running VM in the future.

-Alan.