From 965078b4fee3196641c4dd420e1f75c9a0376373 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 13 Aug 2024 18:04:21 +0200 Subject: [PATCH 1/3] quick and dirty fix using mvn properties --- .../sdk/bytebuddy/CustomElementMatchers.java | 76 ++++++++++++++----- .../bytebuddy/CustomElementMatchersTest.java | 45 +++++++++-- .../Log4j2LogCorrelationInstrumentation.java | 6 +- 3 files changed, 98 insertions(+), 29 deletions(-) diff --git a/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/bytebuddy/CustomElementMatchers.java b/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/bytebuddy/CustomElementMatchers.java index f7af74d8a0..a23d3b850d 100644 --- a/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/bytebuddy/CustomElementMatchers.java +++ b/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/bytebuddy/CustomElementMatchers.java @@ -31,6 +31,7 @@ import javax.annotation.Nullable; import java.io.File; import java.io.IOException; +import java.io.InputStream; import java.net.JarURLConnection; import java.net.URISyntaxException; import java.net.URL; @@ -38,9 +39,11 @@ import java.security.CodeSource; import java.security.ProtectionDomain; import java.util.Collection; +import java.util.Properties; import java.util.jar.Attributes; import java.util.jar.JarFile; import java.util.jar.Manifest; +import java.util.zip.ZipEntry; import static net.bytebuddy.matcher.ElementMatchers.nameContains; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; @@ -183,15 +186,23 @@ public static ElementMatcher.Junction isProxy() { * @param version the version to check against * @return an LTE SemVer matcher */ - public static ElementMatcher.Junction implementationVersionLte(final String version) { - return implementationVersion(version, Matcher.LTE); + public static ElementMatcher.Junction implementationVersionLte(String version) { + return implementationVersion(version, Matcher.LTE, null, null); } - public static ElementMatcher.Junction implementationVersionGte(final String version) { - return implementationVersion(version, Matcher.GTE); + public static ElementMatcher.Junction implementationVersionGte(String version) { + return implementationVersion(version, Matcher.GTE, null, null); } - private static ElementMatcher.Junction implementationVersion(final String version, final Matcher matcher) { + public static ElementMatcher.Junction implementationVersionLte(String version, String groupId, String artifactId) { + return implementationVersion(version, Matcher.LTE, groupId, artifactId); + } + + public static ElementMatcher.Junction implementationVersionGte(String version, String groupId, String artifactId) { + return implementationVersion(version, Matcher.GTE, groupId, artifactId); + } + + private static ElementMatcher.Junction implementationVersion(final String version, final Matcher matcher, @Nullable final String mavenGroupId, @Nullable final String mavenArtifactId) { return new ElementMatcher.Junction.AbstractBase() { /** * Returns true if the implementation version read from the manifest file referenced by the given @@ -205,7 +216,7 @@ private static ElementMatcher.Junction implementationVersion(f @Override public boolean matches(@Nullable ProtectionDomain protectionDomain) { try { - Version pdVersion = readImplementationVersionFromManifest(protectionDomain); + Version pdVersion = readImplementationVersion(protectionDomain, mavenGroupId, mavenArtifactId); if (pdVersion != null) { Version limitVersion = Version.of(version); return matcher.match(pdVersion, limitVersion); @@ -221,22 +232,48 @@ public boolean matches(@Nullable ProtectionDomain protectionDomain) { } @Nullable - private static Version readImplementationVersionFromManifest(@Nullable ProtectionDomain protectionDomain) throws IOException, URISyntaxException { + private static Version readImplementationVersion(@Nullable ProtectionDomain protectionDomain, @Nullable String mavenGroupId, @Nullable String mavenArtifactId) throws IOException, URISyntaxException { Version version = null; JarFile jarFile = null; + + if (protectionDomain == null) { + logger.info("Cannot read implementation version - got null ProtectionDomain"); + return null; + } + try { - if (protectionDomain != null) { - CodeSource codeSource = protectionDomain.getCodeSource(); - if (codeSource != null) { - URL jarUrl = codeSource.getLocation(); - if (jarUrl != null) { - // does not yet establish an actual connection - URLConnection urlConnection = jarUrl.openConnection(); - if (urlConnection instanceof JarURLConnection) { - jarFile = ((JarURLConnection) urlConnection).getJarFile(); - } else { - jarFile = new JarFile(new File(jarUrl.toURI())); + CodeSource codeSource = protectionDomain.getCodeSource(); + if (codeSource != null) { + URL jarUrl = codeSource.getLocation(); + if (jarUrl != null) { + // does not yet establish an actual connection + URLConnection urlConnection = jarUrl.openConnection(); + if (urlConnection instanceof JarURLConnection) { + jarFile = ((JarURLConnection) urlConnection).getJarFile(); + } else { + jarFile = new JarFile(new File(jarUrl.toURI())); + } + + // read maven properties first as they have higher priority over manifest + // this is mostly for shaded libraries in "fat-jar" applications + if (mavenGroupId != null && mavenArtifactId != null) { + ZipEntry zipEntry = jarFile.getEntry(String.format("META-INF/maven/%s/%s/pom.properties", mavenGroupId, mavenArtifactId)); + if (zipEntry != null) { + Properties properties = new Properties(); + try (InputStream input = jarFile.getInputStream(zipEntry)) { + properties.load(input); + } + if (mavenGroupId.equals(properties.getProperty("groupId")) && mavenArtifactId.equals(properties.getProperty("artifactId"))) { + String stringVersion = properties.getProperty("version"); + if (stringVersion != null) { + version = Version.of(stringVersion); + } + } } + } + + // reading manifest if library packaged as a jar + if(version == null) { Manifest manifest = jarFile.getManifest(); if (manifest != null) { Attributes attributes = manifest.getMainAttributes(); @@ -248,12 +285,9 @@ private static Version readImplementationVersionFromManifest(@Nullable Protectio if (manifestVersion != null) { version = Version.of(manifestVersion); } - } } } - } else { - logger.info("Cannot read implementation version - got null ProtectionDomain"); } } finally { if (jarFile != null) { diff --git a/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/CustomElementMatchersTest.java b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/CustomElementMatchersTest.java index 8e6bc0ab5b..5e33050b07 100644 --- a/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/CustomElementMatchersTest.java +++ b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/CustomElementMatchersTest.java @@ -18,20 +18,21 @@ */ package co.elastic.apm.agent.sdk.bytebuddy; -import org.apache.http.client.HttpClient; - import net.bytebuddy.description.type.TypeDescription; +import org.apache.http.client.HttpClient; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import java.io.File; -import java.net.MalformedURLException; -import java.net.URISyntaxException; -import java.net.URL; -import java.net.URLClassLoader; +import java.io.IOException; +import java.net.*; +import java.nio.file.*; import java.security.CodeSigner; import java.security.CodeSource; import java.security.ProtectionDomain; +import java.util.HashMap; import java.util.List; +import java.util.Map; import static co.elastic.apm.agent.sdk.bytebuddy.CustomElementMatchers.*; import static net.bytebuddy.matcher.ElementMatchers.none; @@ -63,6 +64,38 @@ void testSemVerLteWithEncodedFileUrl() throws MalformedURLException, URISyntaxEx assertThat(implementationVersionLte("2.1.9").matches(protectionDomain)).isTrue(); } + @Test + void testSemVerFallbackOnMavenProperties(@TempDir Path tempDir) throws URISyntaxException, IOException { + // Relying on Apache httpclient-4.5.6.jar + // creates a copy of the jar without the manifest so we should parse maven properties + URL originalUrl = HttpClient.class.getProtectionDomain().getCodeSource().getLocation(); + Path modifiedJar = tempDir.resolve("test.jar"); + try { + Files.copy(Paths.get(originalUrl.toURI()), modifiedJar); + + Map fsProperties = new HashMap<>(); + fsProperties.put("create", "false"); + + URI fsUri = URI.create("jar:file://" + modifiedJar.toAbsolutePath()); + try (FileSystem zipFs = FileSystems.newFileSystem(fsUri, fsProperties)) { + Files.delete(zipFs.getPath("META-INF", "MANIFEST.MF")); + } + + ProtectionDomain protectionDomain = new ProtectionDomain(new CodeSource(modifiedJar.toUri().toURL(), new CodeSigner[0]), null); + + assertThat(implementationVersionLte("4.5.5", "org.apache.httpcomponents", "httpclient").matches(protectionDomain)).isFalse(); + assertThat(implementationVersionLte("4.5.6", "org.apache.httpcomponents", "httpclient").matches(protectionDomain)).isTrue(); + assertThat(implementationVersionGte("4.5.7", "org.apache.httpcomponents", "httpclient").matches(protectionDomain)).isFalse(); + + + } finally { + if (Files.exists(modifiedJar)) { + Files.delete(modifiedJar); + } + } + + } + private void testSemVerLteMatcher(ProtectionDomain protectionDomain) { assertThat(implementationVersionLte("3").matches(protectionDomain)).isFalse(); assertThat(implementationVersionLte("3.2").matches(protectionDomain)).isFalse(); diff --git a/apm-agent-plugins/apm-logging-plugin/apm-log4j2-plugin/src/main/java/co/elastic/apm/agent/log4j2/correlation/Log4j2LogCorrelationInstrumentation.java b/apm-agent-plugins/apm-logging-plugin/apm-log4j2-plugin/src/main/java/co/elastic/apm/agent/log4j2/correlation/Log4j2LogCorrelationInstrumentation.java index 4375fec954..e765fdd78b 100644 --- a/apm-agent-plugins/apm-logging-plugin/apm-log4j2-plugin/src/main/java/co/elastic/apm/agent/log4j2/correlation/Log4j2LogCorrelationInstrumentation.java +++ b/apm-agent-plugins/apm-logging-plugin/apm-log4j2-plugin/src/main/java/co/elastic/apm/agent/log4j2/correlation/Log4j2LogCorrelationInstrumentation.java @@ -71,9 +71,11 @@ public ElementMatcher getMethodMatcher() { } public static class Log4J2_6LogCorrelationInstrumentation extends Log4j2LogCorrelationInstrumentation { + @Override public ElementMatcher.Junction getProtectionDomainPostFilter() { - return implementationVersionGte("2.6").and(not(implementationVersionGte("2.7"))); + return implementationVersionGte("2.6", "org.apache.logging.log4j", "log4j-core") + .and(not(implementationVersionGte("2.7", "org.apache.logging.log4j", "log4j-core"))); } public static class AdviceClass { @@ -94,7 +96,7 @@ public static void removeFromThreadContext(@Advice.Enter boolean addedToMdc) { public static class Log4J2_7PlusLogCorrelationInstrumentation extends Log4j2LogCorrelationInstrumentation { @Override public ElementMatcher.Junction getProtectionDomainPostFilter() { - return implementationVersionGte("2.7"); + return implementationVersionGte("2.7", "org.apache.logging.log4j", "log4j-core"); } public static class AdviceClass { From ceb044f07f67e0af9f1c4e6f61da50a3ed002d68 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 13 Aug 2024 18:09:57 +0200 Subject: [PATCH 2/3] update changelog --- CHANGELOG.asciidoc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 99ad0570c8..4502be6cea 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -31,6 +31,10 @@ Use subheadings with the "=====" level for adding notes for unreleased changes: === Unreleased +[float] +===== Bug fixes +* Fix log4j2 log correlation with shaded application jar - {pull}3764[#3764] + [[release-notes-1.x]] === Java Agent version 1.x From 338729a173861bec0660cf8a8fc1ad1a4e34d07c Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 13 Aug 2024 18:12:32 +0200 Subject: [PATCH 3/3] code cleanup --- .../agent/sdk/bytebuddy/CustomElementMatchers.java | 11 ++++++----- .../sdk/bytebuddy/CustomElementMatchersTest.java | 12 ++++++++++-- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/bytebuddy/CustomElementMatchers.java b/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/bytebuddy/CustomElementMatchers.java index a23d3b850d..c2a07a593d 100644 --- a/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/bytebuddy/CustomElementMatchers.java +++ b/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/bytebuddy/CustomElementMatchers.java @@ -19,9 +19,9 @@ package co.elastic.apm.agent.sdk.bytebuddy; import co.elastic.apm.agent.sdk.internal.InternalUtil; +import co.elastic.apm.agent.sdk.internal.util.PrivilegedActionUtils; import co.elastic.apm.agent.sdk.logging.Logger; import co.elastic.apm.agent.sdk.logging.LoggerFactory; -import co.elastic.apm.agent.sdk.internal.util.PrivilegedActionUtils; import co.elastic.apm.agent.sdk.weakconcurrent.WeakConcurrent; import co.elastic.apm.agent.sdk.weakconcurrent.WeakMap; import net.bytebuddy.description.NamedElement; @@ -45,9 +45,7 @@ import java.util.jar.Manifest; import java.util.zip.ZipEntry; -import static net.bytebuddy.matcher.ElementMatchers.nameContains; -import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; -import static net.bytebuddy.matcher.ElementMatchers.none; +import static net.bytebuddy.matcher.ElementMatchers.*; public class CustomElementMatchers { @@ -273,7 +271,10 @@ private static Version readImplementationVersion(@Nullable ProtectionDomain prot } // reading manifest if library packaged as a jar - if(version == null) { + // + // doing this after maven properties is important as it might report executable jar version + // when application is packaged as a "fat jar" + if (version == null) { Manifest manifest = jarFile.getManifest(); if (manifest != null) { Attributes attributes = manifest.getMainAttributes(); diff --git a/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/CustomElementMatchersTest.java b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/CustomElementMatchersTest.java index 5e33050b07..6f24e36a8c 100644 --- a/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/CustomElementMatchersTest.java +++ b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/CustomElementMatchersTest.java @@ -25,8 +25,16 @@ import java.io.File; import java.io.IOException; -import java.net.*; -import java.nio.file.*; +import java.net.MalformedURLException; +import java.net.URISyntaxException; +import java.net.URL; +import java.net.URLClassLoader; +import java.net.URI; +import java.nio.file.Files; +import java.nio.file.FileSystem; +import java.nio.file.FileSystems; +import java.nio.file.Path; +import java.nio.file.Paths; import java.security.CodeSigner; import java.security.CodeSource; import java.security.ProtectionDomain;