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

Rafael Winterhalter winterhalter at openjdk.java.net
Tue Apr 20 20:26:04 UTC 2021

On Fri, 16 Apr 2021 20:30:15 GMT, Rafael Winterhalter <winterhalter at openjdk.org> wrote:

>> To allow agents the definition of auxiliary classes, an API is needed to allow this. Currently, this is often achieved by using `sun.misc.Unsafe` or `jdk.internal.misc.Unsafe` ever since the `defineClass` method was removed from `sun.misc.Unsafe`.
> Rafael Winterhalter has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
>   8200559: Java agents doing instrumentation need a means to define auxiliary classes

I fully understand your concerns about ByteBuddyAgent.install(). It is
simply a convenience for something that can be meaningful in some contexts
where I prefer offering a simple API. I use it mainly for two purposes:

a) For testing Java agents and integrations against Instrumentation within
the current VM when tests are triggered by tools that do not support
javaagents, also because builds do not bundle jars until after tests are

b) For purposefully "hacky" test libraries like Mockito that need agent
capabilities without this being meant to be used in production
environments. I have earlier proposed to offer a "jdk.test" module that
offers the  Instrumentation instance via a simple API similar to Byte
Buddy's. The JVM would not load this module unless requested on the command
line. Build tools like Maven's surefire or Gradle's testrunner could then
standardize on loading this module as a convention to give access to this
test module by default such that libraries like Mockito could continue to
function out of the box without the libraries functioning on a standard VM
without extra configuration. As far as I know, mainly test libraries need
this API. This would also emphasise that Mockito and others are meant for
testing and fewer people would abuse it for production applications. People
would also have an explicit means of running a JVM for a production
application or for executing a test.

As for adding the API, my thought is that if the Instrumentation API were
to throw exceptions on some methods/arguments for dynamic agents in the
future, for example for retransformClasses(Object.class), this breaking
change would then simply extend to the proposed "defineClass" method. In
this sense, the Instrumentation API already assumes full power, I find it
not problematic to add the missing bit to this API even if it was
restricted in the future in the same spirit as other methods of the API
would be.

I mentioned JNI as it is a well-known approach to defining a class today,
using a minimal native binding to an interface that directly calls down to

jclass DefineClass(JNIEnv *env, const char *name, jobject loader, const
jbyte *buf, jsize bufLen);

This interface can then simply be used to define any class just as I
propse, even when not writing an agent or attaching. This method makes
class definitions also already trivial for JVMTI agents compared to Java
agents. Unless restricting JNI, the defineClass method is already a low
hanging fruit, but at the cost of having to maintain a tiny bit of native
code. I'd rather see this avoided and a standard API being offered to
agents up to the time that Panama is in place and a JNI restriction is
possibly also included. As a bonus: Once JNI is restricted, Byte Buddy's
"install" would no longer work unless self-attachment (or JNI) is
explicitly allowed. The emulation already requires to run native code while
the Virtual Machine API explicitly checks for the process id of the current
VM against the one that is targeted. With both disabled, self-attachment
would no longer be practically be possible without needing to prune the
capabilities of dynamic agents which is what I understand would be the
desired effect.

>From this viewpoint, I think that adding Instrumentation::defineClass
method does no harm compared to the status quo. And on the upside, it gives
agents an API to migrate to, avoiding the last need of using unsafe. To
make the JVM a safe platform, binding native code would anyways need
restriction and this would then also solve the problem of dynamic agents
attaching from the same VM being used in libraries. This would in my eyes
be the cleanest solution to the self-attachment problem without disturbing
the existing landscape of dynamic agents. To run Mockito, one would then
instead configure Maven surefire or Gradle to run the JVM with
-Djdk.attach.allowAttachSelf=true. Ideally, some "jdk.test" module would be
added at some point, to avoid the overhead of self-attachment, but I think
this better fits into separate debate.

Am Di., 20. Apr. 2021 um 15:38 Uhr schrieb mlbridge[bot] <

> *Mailing list message from Alan Bateman ***@***.***> on
> core-libs-dev ***@***.***>:*
> 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.
>> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/openjdk/jdk/pull/3546#issuecomment-823281169>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ABCIA4FE2B4DGBZS4QO6SM3TJV7T5ANCNFSM43BSDEGQ>
> .


PR: https://git.openjdk.java.net/jdk/pull/3546

More information about the core-libs-dev mailing list