Add shading to bytebuddy to avoid dependency conflicts on classpath

[ahatzz11]

I'm far from an expert here but I think this should resolve #6, which has some additional information on the bytebuddy repo here: https://github.com/raphw/byte-buddy/issues/1179.

The issue that I seem to be having is classpath conflicts with an older version of bytebuddy bundled in applications. I believe that using the relocate ability of shadowjar will avoid this classpath conflict.

Main Change

• Add a relocate to the bytebuddy dependency to avoid conflicts on the classpath for users of this library

Small Changes

• Update the gradle wrapper to 7.3.3
• Add .editorconfig for some consistency in tab sizes on gradle

[ChaosData]

So I had tried to do a shadow relocate in the devel branch, but it kept leading to weird build and lookup errors b/c I was finding that the code itself was not being modified to use the new names, and that when things were "working" in tests, it was b/c the test setup happened to be pulling in the deps under their normal names into the resulting JARs (undoing that resulted in class lookup failures). And then trying to manually update the code to use the new names also resulted in issues.

How did you test that this worked?

[ChaosData]

Basically, I'm worried that the only reason it works for you might be that the agent code itself starts resolving to the other byte-buddy and not the relocated one.

[ahatzz11]

@ChaosData My understanding of the relocate should be that the references themselves shouldn't need to change and internally the jar should know about the relocation, but I could definitely be wrong on that.

So far my tests were:

• Normal gradle build
• looking at the generated classes
• running the sample test files in the repo

I've built a new version of this with my changes on my own repo and we're going to do some internal testing across our larger environment to see how it goes. I can report back. Here's my release: https://github.com/ahatzz11/log4j-jndi-be-gone/releases/tag/v1.0.1

[ahatzz11]

From shadow docs:

Shadow uses the ASM library to modify class byte code to replace the package name and any import statements for a class. Any non-class files that are stored within a package structure are also relocated to the new location.

I think the important piece is "and any import statements"

source: https://imperceptiblethoughts.com/shadow/configuration/relocation/#filtering-relocation

[ChaosData]

I'm aware that's what should happen. It just didn't happen when I tried exactly that 2 weeks ago.

[ahatzz11]

Oh, interesting! Where did you see it start to break down?

[ChaosData]

$ java -javaagent:build/libs/log4j-jndi-be-gone-1.1.0-standalone.jar -jar tests/jnditest/build/libs/jnditest-tests.jar
Exception in thread "main" java.lang.reflect.InvocationTargetException
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at java.instrument/sun.instrument.InstrumentationImpl.loadClassAndStartAgent(InstrumentationImpl.java:513)
    at java.instrument/sun.instrument.InstrumentationImpl.loadClassAndCallPremain(InstrumentationImpl.java:525)
Caused by: java.lang.NoClassDefFoundError: net/bytebuddy/matcher/ElementMatcher
    at trust.nccgroup.jndibegone.Agent.load(Agent.java:57)
    at trust.nccgroup.jndibegone.PreMain.premain(PreMain.java:27)
    ... 6 more
Caused by: java.lang.ClassNotFoundException: net.bytebuddy.matcher.ElementMatcher
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:581)
    at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
    ... 8 more
*** java.lang.instrument ASSERTION FAILED ***: "result" with message agent load/premain call failed at ./src/java.instrument/share/native/libinstrument/JPLISAgent.c line: 422
FATAL ERROR in native method: processing of -javaagent failed, processJavaStart failed
[1]    99383 abort      java -javaagent:build/libs/log4j-jndi-be-gone-1.1.0-standalone.jar -jar

Agent.java:57 corresponds to JndiLookup__lookup.hook_deep_match(inst).

Disassembling it with javap doesn't show anything other than the relocated classes, and decompiling it w/ procyon similarly shows only the relocated classes being referenced.

package trust.nccgroup.jndibegone.hooks;

import trust.nccgroup.jndibegone.vendor.net.bytebuddy.description.method.ParameterList;
import trust.nccgroup.jndibegone.vendor.net.bytebuddy.description.type.TypeList;
import java.util.Iterator;
import trust.nccgroup.jndibegone.vendor.net.bytebuddy.description.annotation.AnnotationList;
import trust.nccgroup.jndibegone.vendor.net.bytebuddy.description.method.ParameterDescription;
import trust.nccgroup.jndibegone.vendor.net.bytebuddy.description.field.FieldDescription;
import trust.nccgroup.jndibegone.vendor.net.bytebuddy.description.annotation.AnnotationValue;
import trust.nccgroup.jndibegone.vendor.net.bytebuddy.description.method.MethodDescription;
import java.util.HashMap;
import trust.nccgroup.jndibegone.vendor.net.bytebuddy.description.annotation.AnnotationDescription;
import trust.nccgroup.jndibegone.vendor.net.bytebuddy.utility.JavaModule;
import trust.nccgroup.jndibegone.vendor.net.bytebuddy.dynamic.DynamicType;
import trust.nccgroup.jndibegone.vendor.net.bytebuddy.description.type.TypeDescription;
import trust.nccgroup.jndibegone.vendor.net.bytebuddy.agent.builder.AgentBuilder;
import java.lang.instrument.Instrumentation;
import trust.nccgroup.jndibegone.vendor.net.bytebuddy.matcher.ElementMatcher;
import trust.nccgroup.jndibegone.vendor.net.bytebuddy.matcher.ElementMatchers;
import trust.nccgroup.jndibegone.vendor.net.bytebuddy.asm.AsmVisitorWrapper;
import trust.nccgroup.jndibegone.vendor.net.bytebuddy.asm.Advice;

[ChaosData]

I think I figured it out. The test jar, for reasons of the way the tests built, had the uninstrumented version shadowed in. It appears that the lookup for the -javaagent jar was somehow resolving to the -jar jar for class loads, for which those implementations used the clean net.bytebuddy classes, and failed to load classes. Removing that stuff from the test jar made it work, which is sort of scary on its own for a whole host of reasons, but it does mean that the relocate itself was working. I'll have to fix the way the tests work. It's something I've been meaning to do anyway b/c it was making it annoying to detect certain kinds of class resolution issues. I just didn't think something like this would be possible.

[ahatzz11]

Interesting! Seeing the decompiled version referencing the relocated classes is good.

Is this something you would be open to merging now before the test refactoring?

[ChaosData]

The test stuff was simple enough, it just took forever to run the whole suite against all of the combinations.

I have merged the v1.1.0 changes from devel into main with the relocate uncommented out and some other minor fixes I spotted when running the full suite against it. v1.1.0 changes the semantics slightly by doing a bit fancier of a lookup (though they can be disabled with =structureMatch=0 if you want the simple name match behavior).

I still have not seen an example of a repackaged log4j 2.x that doesn't just change the names back before loading in a custom classloader, which is why I was sort of letting the branch languish for a while, but I had most of my other changes in there anyway, so I decided to go with that work rather than diverge main from it.