diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a48b2ac6f..14309bcab0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ * Error in log when setting [server_urls](https://www.elastic.co/guide/en/apm/agent/java/current/config-reporter.html#config-server-urls) to an empty string - `co.elastic.apm.agent.configuration.ApmServerConfigurationSource - Expected previousException not to be null` * Avoid terminating the TCP connection to APM Server when polling for configuration updates (#823) +* Fixes potential segfault if attaching the agent with long arguments (#865) # 1.9.0 diff --git a/apm-agent-attach/src/main/java/co/elastic/apm/attach/ElasticApmAttacher.java b/apm-agent-attach/src/main/java/co/elastic/apm/attach/ElasticApmAttacher.java index b3f0da69a3..8ea774f692 100644 --- a/apm-agent-attach/src/main/java/co/elastic/apm/attach/ElasticApmAttacher.java +++ b/apm-agent-attach/src/main/java/co/elastic/apm/attach/ElasticApmAttacher.java @@ -11,9 +11,9 @@ * the Apache License, Version 2.0 (the "License"); you may * not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY @@ -46,6 +46,13 @@ */ public class ElasticApmAttacher { + /** + * This key is very short on purpose. + * The longer the agent argument ({@code -javaagent:=}), the greater the chance that the max length of the agent argument is reached. + * Because of a bug in the {@linkplain ByteBuddyAgent.AttachmentProvider.ForEmulatedAttachment emulated attachment}, + * this can even lead to segfaults. + */ + private static final String TEMP_PROPERTIES_FILE_KEY = "c"; private static final ByteBuddyAgent.AttachmentProvider ATTACHMENT_PROVIDER = new ByteBuddyAgent.AttachmentProvider.Compound( ByteBuddyAgent.AttachmentProvider.ForEmulatedAttachment.INSTANCE, ByteBuddyAgent.AttachmentProvider.ForModularizedVm.INSTANCE, @@ -97,7 +104,32 @@ public static void attach(Map configuration) { if (Boolean.getBoolean("ElasticApm.attached")) { return; } - ByteBuddyAgent.attach(AgentJarFileHolder.INSTANCE.agentJarFile, ByteBuddyAgent.ProcessProvider.ForCurrentVm.INSTANCE, toAgentArgs(configuration), ATTACHMENT_PROVIDER); + File tempFile = createTempProperties(configuration); + String agentArgs = tempFile == null ? null : TEMP_PROPERTIES_FILE_KEY + "=" + tempFile.getAbsolutePath(); + + ByteBuddyAgent.attach(AgentJarFileHolder.INSTANCE.agentJarFile, ByteBuddyAgent.ProcessProvider.ForCurrentVm.INSTANCE, agentArgs, ATTACHMENT_PROVIDER); + if (tempFile != null) { + if (!tempFile.delete()) { + tempFile.deleteOnExit(); + } + } + } + + static File createTempProperties(Map configuration) { + File tempFile = null; + if (!configuration.isEmpty()) { + Properties properties = new Properties(); + properties.putAll(configuration); + try { + tempFile = File.createTempFile("elstcapm", ".tmp"); + try (FileOutputStream outputStream = new FileOutputStream(tempFile)) { + properties.store(outputStream, null); + } + } catch (IOException e) { + e.printStackTrace(); + } + } + return tempFile; } static String toAgentArgs(Map configuration) { diff --git a/apm-agent-attach/src/test/java/co/elastic/apm/attach/ElasticApmAttacherTest.java b/apm-agent-attach/src/test/java/co/elastic/apm/attach/ElasticApmAttacherTest.java index 326705f1e3..6bb9f68efb 100644 --- a/apm-agent-attach/src/test/java/co/elastic/apm/attach/ElasticApmAttacherTest.java +++ b/apm-agent-attach/src/test/java/co/elastic/apm/attach/ElasticApmAttacherTest.java @@ -11,9 +11,9 @@ * the Apache License, Version 2.0 (the "License"); you may * not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY @@ -27,6 +27,11 @@ import org.junit.jupiter.api.Test; import wiremock.org.apache.commons.codec.digest.DigestUtils; +import java.io.File; +import java.io.FileReader; +import java.util.Map; +import java.util.Properties; + import static org.assertj.core.api.Assertions.assertThat; class ElasticApmAttacherTest { @@ -36,4 +41,15 @@ void testHash() throws Exception { assertThat(ElasticApmAttacher.md5Hash(getClass().getResourceAsStream(ElasticApmAttacher.class.getSimpleName() + ".class"))) .isEqualTo(DigestUtils.md5Hex(getClass().getResourceAsStream(ElasticApmAttacher.class.getSimpleName() + ".class"))); } + + @Test + void testCreateTempProperties() throws Exception { + File tempProperties = ElasticApmAttacher.createTempProperties(Map.of("foo", "bär")); + assertThat(tempProperties).isNotNull(); + tempProperties.deleteOnExit(); + Properties properties = new Properties(); + properties.load(new FileReader(tempProperties)); + assertThat(properties.get("foo")).isEqualTo("bär"); + + } } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/AgentArgumentsConfigurationSource.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/AgentArgumentsConfigurationSource.java index 538bfe24f8..14740f9a48 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/AgentArgumentsConfigurationSource.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/AgentArgumentsConfigurationSource.java @@ -11,9 +11,9 @@ * the Apache License, Version 2.0 (the "License"); you may * not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY @@ -27,6 +27,7 @@ import org.stagemonitor.configuration.source.AbstractConfigurationSource; import org.stagemonitor.util.StringUtils; +import javax.annotation.Nullable; import java.util.HashMap; import java.util.Map; @@ -51,6 +52,7 @@ public static AgentArgumentsConfigurationSource parse(String agentAgruments) { } @Override + @Nullable public String getValue(String key) { return config.get(key); } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/source/PropertyFileConfigurationSource.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/source/PropertyFileConfigurationSource.java index 35bef7d206..04f4a11e83 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/source/PropertyFileConfigurationSource.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/source/PropertyFileConfigurationSource.java @@ -11,9 +11,9 @@ * the Apache License, Version 2.0 (the "License"); you may * not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY @@ -78,7 +78,7 @@ public static Properties getFromClasspath(String classpathLocation, ClassLoader return null; } - private static Properties getFromFileSystem(String location) { + public static Properties getFromFileSystem(String location) { Properties props = new Properties(); try (InputStream input = new FileInputStream(location)) { props.load(input); diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracerBuilder.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracerBuilder.java index 2081274460..4c04bb2637 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracerBuilder.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracerBuilder.java @@ -53,10 +53,15 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Properties; import java.util.concurrent.TimeUnit; public class ElasticApmTracerBuilder { + /** + * See {@link co.elastic.apm.attach.ElasticApmAttacher#TEMP_PROPERTIES_FILE_KEY} + */ + private static final String TEMP_PROPERTIES_FILE_KEY = "c"; private final Logger logger; @Nullable private ConfigurationRegistry configurationRegistry; @@ -153,7 +158,12 @@ private ConfigurationRegistry getDefaultConfigurationRegistry(List getConfigSources(@Nullable String agentArguments) { List result = new ArrayList<>(); if (agentArguments != null && !agentArguments.isEmpty()) { - result.add(AgentArgumentsConfigurationSource.parse(agentArguments)); + AgentArgumentsConfigurationSource agentArgs = AgentArgumentsConfigurationSource.parse(agentArguments); + result.add(agentArgs); + ConfigurationSource attachmentConfig = getAttachmentArguments(agentArgs.getValue(TEMP_PROPERTIES_FILE_KEY)); + if (attachmentConfig != null) { + result.add(attachmentConfig); + } } result.add(new PrefixingConfigurationSourceWrapper(new SystemPropertyConfigurationSource(), "elastic.apm.")); result.add(new PrefixingConfigurationSourceWrapper(new EnvironmentVariableConfigurationSource(), "ELASTIC_APM_")); @@ -180,4 +190,22 @@ public String getName() { return result; } + /** + * Loads the configuration from the temporary properties file created by ElasticApmAttacher + */ + @Nullable + private ConfigurationSource getAttachmentArguments(@Nullable String configFileLocation) { + if (configFileLocation != null) { + Properties fromFileSystem = PropertyFileConfigurationSource.getFromFileSystem(configFileLocation); + if (fromFileSystem != null) { + SimpleSource attachmentConfig = new SimpleSource("Attachment configuration"); + for (String key : fromFileSystem.stringPropertyNames()) { + attachmentConfig.add(key, fromFileSystem.getProperty(key)); + } + return attachmentConfig; + } + } + return null; + } + } diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/ElasticApmTracerBuilderTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/ElasticApmTracerBuilderTest.java index 9cd8211cf2..4e873ef206 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/ElasticApmTracerBuilderTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/ElasticApmTracerBuilderTest.java @@ -11,9 +11,9 @@ * the Apache License, Version 2.0 (the "License"); you may * not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY @@ -39,7 +39,6 @@ class ElasticApmTracerBuilderTest { - @AfterEach void tearDown() { System.clearProperty("elastic.apm." + CoreConfiguration.CONFIG_FILE); @@ -59,4 +58,14 @@ void testConfigFileLocation(@TempDir Path tempDir) throws IOException { configurationRegistry.getString(CoreConfiguration.CONFIG_FILE); assertThat(configurationRegistry.getString(CoreConfiguration.CONFIG_FILE)).isEqualTo(file.toString()); } + + @Test + void testTempAttacherPropertiesFile(@TempDir Path tempDir) throws Exception { + Path file = Files.createFile(tempDir.resolve("elstcapm.tmp")); + Files.write(file, List.of("instrument=false")); + + ConfigurationRegistry configurationRegistry = new ElasticApmTracerBuilder("c=" + file.toAbsolutePath()).build().getConfigurationRegistry(); + CoreConfiguration config = configurationRegistry.getConfig(CoreConfiguration.class); + assertThat(config.isInstrument()).isFalse(); + } } diff --git a/docs/configuration.asciidoc b/docs/configuration.asciidoc index a093b498ad..f438e051c2 100644 --- a/docs/configuration.asciidoc +++ b/docs/configuration.asciidoc @@ -358,7 +358,7 @@ you should add an additional entry to this list (make sure to also include the d ==== `disable_instrumentations` A list of instrumentations which should be disabled. -Valid options are `annotations`, `apache-httpclient`, `asynchttpclient`, `concurrent`, `elasticsearch-restclient`, `exception-handler`, `executor`, `hibernate-search`, `http-client`, `incubating`, `jax-rs`, `jax-ws`, `jdbc`, `jedis`, `jms`, `jsf`, `logging`, `okhttp`, `opentracing`, `public-api`, `quartz`, `redis`, `render`, `scheduled`, `servlet-api`, `servlet-api-async`, `servlet-input-stream`, `slf4j`, `spring-mvc`, `spring-resttemplate`, `spring-service-name`, `spring-view-render`, `urlconnection`. +Valid options are `annotations`, `apache-httpclient`, `asynchttpclient`, `concurrent`, `elasticsearch-restclient`, `exception-handler`, `executor`, `hibernate-search`, `http-client`, `incubating`, `jax-rs`, `jax-ws`, `jdbc`, `jedis`, `jms`, `jsf`, `logging`, `mule`, `okhttp`, `opentracing`, `public-api`, `quartz`, `redis`, `render`, `scheduled`, `servlet-api`, `servlet-api-async`, `servlet-input-stream`, `slf4j`, `spring-mvc`, `spring-resttemplate`, `spring-service-name`, `spring-view-render`, `urlconnection`. If you want to try out incubating features, set the value to an empty string. @@ -1551,7 +1551,7 @@ The default unit for this option is `ms` # sanitize_field_names=password,passwd,pwd,secret,*key,*token*,*session*,*credit*,*card*,authorization,set-cookie # A list of instrumentations which should be disabled. -# Valid options are `annotations`, `apache-httpclient`, `asynchttpclient`, `concurrent`, `elasticsearch-restclient`, `exception-handler`, `executor`, `hibernate-search`, `http-client`, `incubating`, `jax-rs`, `jax-ws`, `jdbc`, `jedis`, `jms`, `jsf`, `logging`, `okhttp`, `opentracing`, `public-api`, `quartz`, `redis`, `render`, `scheduled`, `servlet-api`, `servlet-api-async`, `servlet-input-stream`, `slf4j`, `spring-mvc`, `spring-resttemplate`, `spring-service-name`, `spring-view-render`, `urlconnection`. +# Valid options are `annotations`, `apache-httpclient`, `asynchttpclient`, `concurrent`, `elasticsearch-restclient`, `exception-handler`, `executor`, `hibernate-search`, `http-client`, `incubating`, `jax-rs`, `jax-ws`, `jdbc`, `jedis`, `jms`, `jsf`, `logging`, `mule`, `okhttp`, `opentracing`, `public-api`, `quartz`, `redis`, `render`, `scheduled`, `servlet-api`, `servlet-api-async`, `servlet-input-stream`, `slf4j`, `spring-mvc`, `spring-resttemplate`, `spring-service-name`, `spring-view-render`, `urlconnection`. # If you want to try out incubating features, # set the value to an empty string. #