From c5a69c121e8b12b93397bfe846ee313bdf0960f7 Mon Sep 17 00:00:00 2001 From: "shun.yamamoto" Date: Wed, 26 Oct 2022 01:38:35 +0900 Subject: [PATCH 001/367] Add `dd.trace.db.client.split-by-host` Config --- .../decorator/DatabaseClientDecorator.java | 4 ++ .../DatabaseClientDecoratorTest.groovy | 38 ++++++++++++------- .../datadog/trace/api/ConfigDefaults.java | 1 + .../config/TraceInstrumentationConfig.java | 1 + .../main/java/datadog/trace/api/Config.java | 13 +++++++ .../datadog/trace/api/ConfigTest.groovy | 9 +++++ 6 files changed, 53 insertions(+), 13 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/DatabaseClientDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/DatabaseClientDecorator.java index 258bca1229a..237682cb69d 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/DatabaseClientDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/DatabaseClientDecorator.java @@ -50,6 +50,10 @@ public AgentSpan onConnection(final AgentSpan span, final CONNECTION connection) CharSequence hostName = dbHostname(connection); if (hostName != null) { span.setTag(Tags.PEER_HOSTNAME, hostName); + + if (Config.get().isDbClientSplitByHost()) { + span.setServiceName(hostName.toString()); + } } } return span; diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/DatabaseClientDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/DatabaseClientDecoratorTest.groovy index de3d8f0291d..0269f51d5a8 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/DatabaseClientDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/DatabaseClientDecoratorTest.groovy @@ -4,6 +4,7 @@ import datadog.trace.api.DDTags import datadog.trace.bootstrap.instrumentation.api.AgentSpan import datadog.trace.bootstrap.instrumentation.api.Tags +import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_HOST import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_INSTANCE import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_INSTANCE_TYPE_SUFFIX @@ -35,8 +36,9 @@ class DatabaseClientDecoratorTest extends ClientDecoratorTest { def "test onConnection"() { setup: - injectSysConfig(DB_CLIENT_HOST_SPLIT_BY_INSTANCE, "$renameService") - injectSysConfig(DB_CLIENT_HOST_SPLIT_BY_INSTANCE_TYPE_SUFFIX, "$typeSuffix") + injectSysConfig(DB_CLIENT_HOST_SPLIT_BY_INSTANCE, "$renameByInstance") + injectSysConfig(DB_CLIENT_HOST_SPLIT_BY_INSTANCE_TYPE_SUFFIX, "$instanceTypeSuffix") + injectSysConfig(DB_CLIENT_HOST_SPLIT_BY_HOST, "$renameByHost") def decorator = newDecorator() when: @@ -49,24 +51,34 @@ class DatabaseClientDecoratorTest extends ClientDecoratorTest { if (session.hostname != null) { 1 * span.setTag(Tags.PEER_HOSTNAME, session.hostname) } - if (typeSuffix && renameService && session.instance) { + if (instanceTypeSuffix && renameByInstance && session.instance) { 1 * span.setServiceName(session.instance + "-" + decorator.dbType()) - } else if (renameService && session.instance) { + } else if (renameByInstance && session.instance) { 1 * span.setServiceName(session.instance) + } else if (renameByHost) { + 1 * span.setServiceName(session.hostname) } } 0 * _ where: - renameService | typeSuffix | session - false | false | null - true | false | [user: "test-user", hostname: "test-hostname"] - false | false | [instance: "test-instance", hostname: "test-hostname"] - true | false | [user: "test-user", instance: "test-instance"] - false | true | null - true | true | [user: "test-user", hostname: "test-hostname"] - false | true | [instance: "test-instance", hostname: "test-hostname"] - true | true | [user: "test-user", instance: "test-instance"] + renameByInstance | instanceTypeSuffix | renameByHost | session + false | false | false | null + true | false | false | [user: "test-user", hostname: "test-hostname"] + false | false | false | [instance: "test-instance", hostname: "test-hostname"] + true | false | false | [user: "test-user", instance: "test-instance"] + false | true | false | null + true | true | false | [user: "test-user", hostname: "test-hostname"] + false | true | false | [instance: "test-instance", hostname: "test-hostname"] + true | true | false | [user: "test-user", instance: "test-instance"] + false | false | true | null + true | false | true | [user: "test-user", hostname: "test-hostname"] + false | false | true | [instance: "test-instance", hostname: "test-hostname"] + true | false | true | [user: "test-user", instance: "test-instance"] + false | true | true | null + true | true | true | [user: "test-user", hostname: "test-hostname"] + false | true | true | [instance: "test-instance", hostname: "test-hostname"] + true | true | true | [user: "test-user", instance: "test-instance"] } def "test onStatement"() { diff --git a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java index 28217fc0086..4f325ab1276 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java @@ -51,6 +51,7 @@ public final class ConfigDefaults { static final boolean DEFAULT_HTTP_CLIENT_SPLIT_BY_DOMAIN = false; static final boolean DEFAULT_DB_CLIENT_HOST_SPLIT_BY_INSTANCE = false; static final boolean DEFAULT_DB_CLIENT_HOST_SPLIT_BY_INSTANCE_TYPE_SUFFIX = false; + static final boolean DEFAULT_DB_CLIENT_HOST_SPLIT_BY_HOST = false; static final int DEFAULT_SCOPE_DEPTH_LIMIT = 100; static final int DEFAULT_SCOPE_ITERATION_KEEP_ALIVE = 10; // in seconds static final int DEFAULT_PARTIAL_FLUSH_MIN_SPANS = 1000; diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java index c38c96f02b9..65888633d64 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java @@ -36,6 +36,7 @@ public final class TraceInstrumentationConfig { public static final String DB_CLIENT_HOST_SPLIT_BY_INSTANCE = "trace.db.client.split-by-instance"; public static final String DB_CLIENT_HOST_SPLIT_BY_INSTANCE_TYPE_SUFFIX = "trace.db.client.split-by-instance.type.suffix"; + public static final String DB_CLIENT_HOST_SPLIT_BY_HOST = "trace.db.client.split-by-host"; public static final String JDBC_PREPARED_STATEMENT_CLASS_NAME = "trace.jdbc.prepared.statement.class.name"; diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 69d2c7aab52..82061876be2 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -14,6 +14,7 @@ import static datadog.trace.api.ConfigDefaults.DEFAULT_CWS_ENABLED; import static datadog.trace.api.ConfigDefaults.DEFAULT_CWS_TLS_REFRESH; import static datadog.trace.api.ConfigDefaults.DEFAULT_DATA_STREAMS_ENABLED; +import static datadog.trace.api.ConfigDefaults.DEFAULT_DB_CLIENT_HOST_SPLIT_BY_HOST; import static datadog.trace.api.ConfigDefaults.DEFAULT_DB_CLIENT_HOST_SPLIT_BY_INSTANCE; import static datadog.trace.api.ConfigDefaults.DEFAULT_DB_CLIENT_HOST_SPLIT_BY_INSTANCE_TYPE_SUFFIX; import static datadog.trace.api.ConfigDefaults.DEFAULT_DEBUGGER_CLASSFILE_DUMP_ENABLED; @@ -222,6 +223,7 @@ import static datadog.trace.api.config.RemoteConfigConfig.REMOTE_CONFIG_TARGETS_KEY; import static datadog.trace.api.config.RemoteConfigConfig.REMOTE_CONFIG_TARGETS_KEY_ID; import static datadog.trace.api.config.RemoteConfigConfig.REMOTE_CONFIG_URL; +import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_HOST; import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_INSTANCE; import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_INSTANCE_TYPE_SUFFIX; import static datadog.trace.api.config.TraceInstrumentationConfig.GRPC_CLIENT_ERROR_STATUSES; @@ -436,6 +438,7 @@ public class Config { private final boolean httpClientSplitByDomain; private final boolean dbClientSplitByInstance; private final boolean dbClientSplitByInstanceTypeSuffix; + private final boolean dbClientSplitByHost; private final Set splitByTags; private final int scopeDepthLimit; private final boolean scopeStrictMode; @@ -868,6 +871,10 @@ private Config(final ConfigProvider configProvider) { DB_CLIENT_HOST_SPLIT_BY_INSTANCE_TYPE_SUFFIX, DEFAULT_DB_CLIENT_HOST_SPLIT_BY_INSTANCE_TYPE_SUFFIX); + dbClientSplitByHost = + configProvider.getBoolean( + DB_CLIENT_HOST_SPLIT_BY_HOST, DEFAULT_DB_CLIENT_HOST_SPLIT_BY_HOST); + splitByTags = tryMakeImmutableSet(configProvider.getList(SPLIT_BY_TAGS)); scopeDepthLimit = configProvider.getInteger(SCOPE_DEPTH_LIMIT, DEFAULT_SCOPE_DEPTH_LIMIT); @@ -1512,6 +1519,10 @@ public boolean isDbClientSplitByInstanceTypeSuffix() { return dbClientSplitByInstanceTypeSuffix; } + public boolean isDbClientSplitByHost() { + return dbClientSplitByHost; + } + public Set getSplitByTags() { return splitByTags; } @@ -2849,6 +2860,8 @@ public String toString() { + dbClientSplitByInstance + ", dbClientSplitByInstanceTypeSuffix=" + dbClientSplitByInstanceTypeSuffix + + ", dbClientSplitByHost=" + + dbClientSplitByHost + ", splitByTags=" + splitByTags + ", scopeDepthLimit=" diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index 45e05803ec6..7dbe39e59b7 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -76,6 +76,7 @@ import static datadog.trace.api.config.RemoteConfigConfig.REMOTE_CONFIG_ENABLED import static datadog.trace.api.config.RemoteConfigConfig.REMOTE_CONFIG_INITIAL_POLL_INTERVAL import static datadog.trace.api.config.RemoteConfigConfig.REMOTE_CONFIG_MAX_PAYLOAD_SIZE import static datadog.trace.api.config.RemoteConfigConfig.REMOTE_CONFIG_URL +import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_HOST import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_INSTANCE import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_INSTANCE_TYPE_SUFFIX import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN @@ -168,6 +169,7 @@ class ConfigTest extends DDSpecification { prop.setProperty(HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "true") prop.setProperty(DB_CLIENT_HOST_SPLIT_BY_INSTANCE, "true") prop.setProperty(DB_CLIENT_HOST_SPLIT_BY_INSTANCE_TYPE_SUFFIX, "true") + prop.setProperty(DB_CLIENT_HOST_SPLIT_BY_HOST, "true") prop.setProperty(SPLIT_BY_TAGS, "some.tag1,some.tag2,some.tag1") prop.setProperty(PARTIAL_FLUSH_MIN_SPANS, "15") prop.setProperty(TRACE_REPORT_HOSTNAME, "true") @@ -251,6 +253,7 @@ class ConfigTest extends DDSpecification { config.httpClientSplitByDomain == true config.dbClientSplitByInstance == true config.dbClientSplitByInstanceTypeSuffix == true + config.dbClientSplitByHost == true config.splitByTags == ["some.tag1", "some.tag2"].toSet() config.partialFlushMinSpans == 15 config.reportHostName == true @@ -336,6 +339,7 @@ class ConfigTest extends DDSpecification { System.setProperty(PREFIX + HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "true") System.setProperty(PREFIX + DB_CLIENT_HOST_SPLIT_BY_INSTANCE, "true") System.setProperty(PREFIX + DB_CLIENT_HOST_SPLIT_BY_INSTANCE_TYPE_SUFFIX, "true") + System.setProperty(PREFIX + DB_CLIENT_HOST_SPLIT_BY_HOST, "true") System.setProperty(PREFIX + SPLIT_BY_TAGS, "some.tag3, some.tag2, some.tag1") System.setProperty(PREFIX + PARTIAL_FLUSH_MIN_SPANS, "25") System.setProperty(PREFIX + TRACE_REPORT_HOSTNAME, "true") @@ -419,6 +423,7 @@ class ConfigTest extends DDSpecification { config.httpClientSplitByDomain == true config.dbClientSplitByInstance == true config.dbClientSplitByInstanceTypeSuffix == true + config.dbClientSplitByHost == true config.splitByTags == ["some.tag3", "some.tag2", "some.tag1"].toSet() config.partialFlushMinSpans == 25 config.reportHostName == true @@ -549,6 +554,7 @@ class ConfigTest extends DDSpecification { System.setProperty(PREFIX + HTTP_CLIENT_ERROR_STATUSES, "1:1") System.setProperty(PREFIX + HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "invalid") System.setProperty(PREFIX + DB_CLIENT_HOST_SPLIT_BY_INSTANCE, "invalid") + System.setProperty(PREFIX + DB_CLIENT_HOST_SPLIT_BY_HOST, "invalid") System.setProperty(PREFIX + PROPAGATION_STYLE_EXTRACT, "some garbage") System.setProperty(PREFIX + PROPAGATION_STYLE_INJECT, " ") @@ -572,6 +578,7 @@ class ConfigTest extends DDSpecification { config.httpClientSplitByDomain == false config.dbClientSplitByInstance == false config.dbClientSplitByInstanceTypeSuffix == false + config.dbClientSplitByHost == false config.splitByTags == [].toSet() config.propagationStylesToExtract.toList() == [PropagationStyle.DATADOG] config.propagationStylesToInject.toList() == [PropagationStyle.DATADOG] @@ -644,6 +651,7 @@ class ConfigTest extends DDSpecification { properties.setProperty(HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "true") properties.setProperty(DB_CLIENT_HOST_SPLIT_BY_INSTANCE, "true") properties.setProperty(DB_CLIENT_HOST_SPLIT_BY_INSTANCE_TYPE_SUFFIX, "true") + properties.setProperty(DB_CLIENT_HOST_SPLIT_BY_HOST, "true") properties.setProperty(PARTIAL_FLUSH_MIN_SPANS, "15") properties.setProperty(PROPAGATION_STYLE_EXTRACT, "B3 Datadog") properties.setProperty(PROPAGATION_STYLE_INJECT, "Datadog B3") @@ -675,6 +683,7 @@ class ConfigTest extends DDSpecification { config.httpClientSplitByDomain == true config.dbClientSplitByInstance == true config.dbClientSplitByInstanceTypeSuffix == true + config.dbClientSplitByHost == true config.splitByTags == [].toSet() config.partialFlushMinSpans == 15 config.propagationStylesToExtract.toList() == [PropagationStyle.B3, PropagationStyle.DATADOG] From c3ff35be25258c52ebb5b45b1b84c54109417f80 Mon Sep 17 00:00:00 2001 From: jean-philippe bempel Date: Fri, 21 Jul 2023 17:39:50 +0200 Subject: [PATCH 002/367] Fix log template issue for duplicated line probes Having 2 line probes on the same location with same template or accessing the same field, arg or var result in an error evaluating the template. With line probe, the freeze/commit of the data is done just after the evaluation per probe id. The fix is to perform 2 loops on the probe ids, first only evaluation and then commit. --- .../bootstrap/debugger/DebuggerContext.java | 5 ++++ .../debugger/agent/CapturedSnapshotTest.java | 24 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/DebuggerContext.java b/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/DebuggerContext.java index 56a4f6723d0..e2009f95e5f 100644 --- a/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/DebuggerContext.java +++ b/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/DebuggerContext.java @@ -4,6 +4,7 @@ import datadog.trace.bootstrap.debugger.util.TimeoutChecker; import java.time.Duration; import java.time.temporal.ChronoUnit; +import java.util.ArrayList; import java.util.List; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -269,6 +270,7 @@ public static void evalContext( public static void evalContextAndCommit( CapturedContext context, Class callingClass, int line, String... probeIds) { try { + List probeImplementations = new ArrayList<>(); for (String probeId : probeIds) { ProbeImplementation probeImplementation = resolveProbe(probeId, callingClass); if (probeImplementation == null) { @@ -276,6 +278,9 @@ public static void evalContextAndCommit( } context.evaluate( probeId, probeImplementation, callingClass.getTypeName(), -1, MethodLocation.DEFAULT); + probeImplementations.add(probeImplementation); + } + for (ProbeImplementation probeImplementation : probeImplementations) { probeImplementation.commit(context, line); } } catch (Exception ex) { diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java index 3ec71820273..e43422421e0 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java @@ -1,5 +1,6 @@ package com.datadog.debugger.agent; +import static com.datadog.debugger.util.LogProbeTestHelper.parseTemplate; import static com.datadog.debugger.util.MoshiSnapshotHelper.DEPTH_REASON; import static com.datadog.debugger.util.MoshiSnapshotHelper.FIELD_COUNT_REASON; import static com.datadog.debugger.util.MoshiSnapshotHelper.NOT_CAPTURED_REASON; @@ -1527,6 +1528,29 @@ public void beforeForLoopLineProbe() throws IOException, URISyntaxException { assertCaptureLocals(snapshot.getCaptures().getLines().get(46), "count", "int", "31"); } + @Test + public void dupLineProbeSameTemplate() throws IOException, URISyntaxException { + final String CLASS_NAME = "CapturedSnapshot08"; + final String LOG_TEMPLATE = "msg1={typed.fld.fld.msg}"; + LogProbe probe1 = + createProbeBuilder(PROBE_ID1, CLASS_NAME, null, null, "39") + .template(LOG_TEMPLATE, parseTemplate(LOG_TEMPLATE)) + .build(); + LogProbe probe2 = + createProbeBuilder(PROBE_ID2, CLASS_NAME, null, null, "39") + .template(LOG_TEMPLATE, parseTemplate(LOG_TEMPLATE)) + .build(); + DebuggerTransformerTest.TestSnapshotListener listener = + installProbes(CLASS_NAME, probe1, probe2); + Class testClass = compileAndLoadClass(CLASS_NAME); + int result = Reflect.on(testClass).call("main", "1").get(); + Assertions.assertEquals(3, result); + List snapshots = assertSnapshots(listener, 2, PROBE_ID1, PROBE_ID2); + for (Snapshot snapshot : snapshots) { + assertEquals("msg1=hello", snapshot.getMessage()); + } + } + private DebuggerTransformerTest.TestSnapshotListener setupInstrumentTheWorldTransformer( String excludeFileName) { Config config = mock(Config.class); From e273c4270809296714adec55a4a654b01f86c552 Mon Sep 17 00:00:00 2001 From: jean-philippe bempel Date: Thu, 3 Aug 2023 18:06:22 +0200 Subject: [PATCH 003/367] Fix coverage of BatchUploader --- dd-java-agent/agent-debugger/build.gradle | 1 - .../debugger/uploader/BatchUploader.java | 2 +- .../debugger/uploader/BatchUploaderTest.java | 19 +++++++++++++++---- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/dd-java-agent/agent-debugger/build.gradle b/dd-java-agent/agent-debugger/build.gradle index 91c875c8bc4..762229e19cb 100644 --- a/dd-java-agent/agent-debugger/build.gradle +++ b/dd-java-agent/agent-debugger/build.gradle @@ -15,7 +15,6 @@ excludedClassesCoverage += [ 'com.datadog.debugger.agent.DebuggerProbe.When.Threshold', 'com.datadog.debugger.agent.DebuggerAgent.ShutdownHook', 'com.datadog.debugger.agent.DebuggerAgent', - 'com.datadog.debugger.uploader.BatchUploader', // too old for this coverage (JDK 1.2) 'antlr.*', 'com.datadog.debugger.util.MoshiSnapshotHelper' // only static classes diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/uploader/BatchUploader.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/uploader/BatchUploader.java index 422835ab659..3f7a9fe813e 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/uploader/BatchUploader.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/uploader/BatchUploader.java @@ -35,10 +35,10 @@ public class BatchUploader { private static final Logger log = LoggerFactory.getLogger(BatchUploader.class); private static final int MINUTES_BETWEEN_ERROR_LOG = 5; private static final MediaType APPLICATION_JSON = MediaType.parse("application/json"); - private static final String HEADER_DD_API_KEY = "DD-API-KEY"; private static final String HEADER_DD_CONTAINER_ID = "Datadog-Container-ID"; private final String containerId; + static final String HEADER_DD_API_KEY = "DD-API-KEY"; static final int MAX_RUNNING_REQUESTS = 10; static final int MAX_ENQUEUED_REQUESTS = 20; static final int TERMINATION_TIMEOUT = 5; diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/uploader/BatchUploaderTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/uploader/BatchUploaderTest.java index c2fe8cf06b0..e018598f313 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/uploader/BatchUploaderTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/uploader/BatchUploaderTest.java @@ -1,5 +1,6 @@ package com.datadog.debugger.uploader; +import static com.datadog.debugger.uploader.BatchUploader.HEADER_DD_API_KEY; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -13,10 +14,8 @@ import datadog.trace.relocate.api.RatelimitedLogger; import java.io.IOException; import java.net.ConnectException; -import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.time.Duration; -import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; import okhttp3.ConnectionSpec; @@ -41,6 +40,7 @@ public class BatchUploaderTest { private final Duration REQUEST_TIMEOUT = Duration.ofSeconds(10); private final Duration REQUEST_IO_OPERATION_TIMEOUT = Duration.ofSeconds(5); private final Duration FOREVER_REQUEST_TIMEOUT = Duration.ofSeconds(1000); + private static final String API_KEY_VALUE = "testkey"; @Mock private Config config; @Mock private RatelimitedLogger ratelimitedLogger; @@ -178,13 +178,11 @@ public void testTooManyRequests() throws IOException, InterruptedException { uploader.upload(SNAPSHOT_BUFFER); } - final List hangingRequests = new ArrayList<>(); // We schedule one additional request to check case when request would be rejected immediately // rather than added to the queue. for (int i = 0; i < BatchUploader.MAX_ENQUEUED_REQUESTS + 1; i++) { uploader.upload(SNAPSHOT_BUFFER); } - // Make sure all expected requests happened for (int i = 0; i < BatchUploader.MAX_RUNNING_REQUESTS; i++) { assertNotNull(server.takeRequest(5, TimeUnit.SECONDS)); @@ -234,4 +232,17 @@ public void testContainerIdHeader() throws InterruptedException { RecordedRequest request = server.takeRequest(100, TimeUnit.MILLISECONDS); assertEquals("testContainerId", request.getHeader("Datadog-Container-ID")); } + + @Test + public void testApiKey() throws InterruptedException { + server.enqueue(new MockResponse().setResponseCode(200)); + when(config.getApiKey()).thenReturn(API_KEY_VALUE); + + BatchUploader uploaderWithApiKey = new BatchUploader(config, ratelimitedLogger); + uploaderWithApiKey.upload(SNAPSHOT_BUFFER); + uploaderWithApiKey.shutdown(); + + RecordedRequest request = server.takeRequest(100, TimeUnit.MILLISECONDS); + assertEquals(API_KEY_VALUE, request.getHeader(HEADER_DD_API_KEY)); + } } From 651488a6a4906619c37a0bee34164630916caeae Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Mon, 7 Aug 2023 16:55:06 -0400 Subject: [PATCH 004/367] Add configuration option to allow Jax-RS exceptions to not be handled as errors --- .../build.gradle | 4 ++ .../client/WrappingResponseCallback.java | 9 +++- .../WrappingResponseCallbackTest.groovy | 46 +++++++++++++++++++ .../datadog/trace/api/ConfigDefaults.java | 2 + .../config/TraceInstrumentationConfig.java | 5 ++ .../main/java/datadog/trace/api/Config.java | 13 ++++++ .../groovy/datadog/trace/test/util/Flaky.java | 2 +- 7 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/src/test/groovy/WrappingResponseCallbackTest.groovy diff --git a/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/build.gradle b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/build.gradle index 312af6333b7..8ede7159e5f 100644 --- a/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/build.gradle +++ b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/build.gradle @@ -12,4 +12,8 @@ dependencies { compileOnly group: 'org.glassfish.jersey.core', name: 'jersey-client', version: '2.0' compileOnly project(':dd-java-agent:instrumentation:jax-rs-client-2.0') + + testImplementation group: 'org.glassfish.jersey.core', name: 'jersey-client', version: '2.0' + + testImplementation project(':dd-java-agent:instrumentation:jax-rs-client-2.0') } diff --git a/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/src/main/java/org/glassfish/jersey/client/WrappingResponseCallback.java b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/src/main/java/org/glassfish/jersey/client/WrappingResponseCallback.java index d59e180c406..f1df278253a 100644 --- a/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/src/main/java/org/glassfish/jersey/client/WrappingResponseCallback.java +++ b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/src/main/java/org/glassfish/jersey/client/WrappingResponseCallback.java @@ -1,5 +1,6 @@ package org.glassfish.jersey.client; +import datadog.trace.api.Config; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.instrumentation.jaxrs.ClientTracingFilter; import javax.ws.rs.ProcessingException; @@ -30,8 +31,14 @@ public static void handleProcessingException(ClientRequest request, ProcessingEx final Object prop = request.getProperty(ClientTracingFilter.SPAN_PROPERTY_NAME); if (prop instanceof AgentSpan) { final AgentSpan span = (AgentSpan) prop; - span.setError(true); span.addThrowable(error); + + @SuppressWarnings("deprecation") + final boolean isJaxRsExceptionAsErrorEnabled = Config.get().isJaxRsExceptionAsErrorEnabled(); + if (!isJaxRsExceptionAsErrorEnabled) { + span.setError(false); + } + span.finish(); } } diff --git a/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/src/test/groovy/WrappingResponseCallbackTest.groovy b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/src/test/groovy/WrappingResponseCallbackTest.groovy new file mode 100644 index 00000000000..04a0989ceec --- /dev/null +++ b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/src/test/groovy/WrappingResponseCallbackTest.groovy @@ -0,0 +1,46 @@ +import static datadog.trace.api.config.TraceInstrumentationConfig.JAX_RS_EXCEPTION_AS_ERROR_ENABLED + +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.instrumentation.jaxrs.ClientTracingFilter +import javax.ws.rs.ProcessingException +import org.glassfish.jersey.client.ClientConfig +import org.glassfish.jersey.client.ClientRequest +import org.glassfish.jersey.client.JerseyClient +import org.glassfish.jersey.client.WrappingResponseCallback +import org.glassfish.jersey.internal.MapPropertiesDelegate + +class WrappingResponseCallbackTest extends AgentTestRunner { + def "handleProcessingException properly utilizes the config"() { + setup: + if (jaxRsExceptionAsErrorEnabled != null) { + injectSysConfig(JAX_RS_EXCEPTION_AS_ERROR_ENABLED, "$jaxRsExceptionAsErrorEnabled") + } + def testSpan = TEST_TRACER.buildSpan("testInstrumentation", "testSpan").start() + def props = Map.of(ClientTracingFilter.SPAN_PROPERTY_NAME, testSpan) + def propertiesDelegate = new MapPropertiesDelegate(props) + def clientConfig = new ClientConfig(new JerseyClient()) + + def clientRequest = new ClientRequest(new URI("https://www.google.com/"), clientConfig, propertiesDelegate) + def processingException = new ProcessingException("test") + + when: + WrappingResponseCallback.handleProcessingException(clientRequest, processingException) + + then: + assertTraces(1) { + trace(1) { + span { + operationName "testSpan" + resourceName "testSpan" + errored isErrored + } + } + } + + where: + jaxRsExceptionAsErrorEnabled | isErrored + true | true + false | false + null | true + } +} diff --git a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java index c1dc8873d30..62f267ab43f 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java @@ -186,5 +186,7 @@ public final class ConfigDefaults { static final boolean DEFAULT_SPARK_TASK_HISTOGRAM_ENABLED = true; + static final boolean DEFAULT_JAX_RS_EXCEPTION_AS_ERROR_ENABLED = true; + private ConfigDefaults() {} } diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java index 0697353a7f6..0ff5eae7b58 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java @@ -29,6 +29,8 @@ public final class TraceInstrumentationConfig { public static final String TRACE_CLASSES_EXCLUDE_FILE = "trace.classes.exclude.file"; public static final String TRACE_CLASSLOADERS_EXCLUDE = "trace.classloaders.exclude"; public static final String TRACE_CODESOURCES_EXCLUDE = "trace.codesources.exclude"; + + @SuppressWarnings("unused") public static final String TRACE_TESTS_ENABLED = "trace.tests.enabled"; public static final String TRACE_THREAD_POOL_EXECUTORS_EXCLUDE = @@ -116,5 +118,8 @@ public final class TraceInstrumentationConfig { public static final String SPARK_TASK_HISTOGRAM_ENABLED = "spark.task-histogram.enabled"; + public static final String JAX_RS_EXCEPTION_AS_ERROR_ENABLED = + "trace.jax-rs.exception-as-error.enabled"; + private TraceInstrumentationConfig() {} } diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 2df947b1631..618529d6f5a 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -291,6 +291,7 @@ import static datadog.trace.api.config.TraceInstrumentationConfig.HYSTRIX_TAGS_ENABLED; import static datadog.trace.api.config.TraceInstrumentationConfig.IGNITE_CACHE_INCLUDE_KEYS; import static datadog.trace.api.config.TraceInstrumentationConfig.INTEGRATION_SYNAPSE_LEGACY_OPERATION_NAME; +import static datadog.trace.api.config.TraceInstrumentationConfig.JAX_RS_EXCEPTION_AS_ERROR_ENABLED; import static datadog.trace.api.config.TraceInstrumentationConfig.JMS_PROPAGATION_DISABLED_QUEUES; import static datadog.trace.api.config.TraceInstrumentationConfig.JMS_PROPAGATION_DISABLED_TOPICS; import static datadog.trace.api.config.TraceInstrumentationConfig.JMS_UNACKNOWLEDGED_MAX_AGE; @@ -769,6 +770,7 @@ static class HostNameHolder { private final long longRunningTraceFlushInterval; private final boolean elasticsearchBodyAndParamsEnabled; private final boolean sparkTaskHistogramEnabled; + private final boolean jaxRsExceptionAsErrorsEnabled; private final float traceFlushIntervalSeconds; @@ -1702,6 +1704,11 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins configProvider.getBoolean( SPARK_TASK_HISTOGRAM_ENABLED, ConfigDefaults.DEFAULT_SPARK_TASK_HISTOGRAM_ENABLED); + this.jaxRsExceptionAsErrorsEnabled = + configProvider.getBoolean( + JAX_RS_EXCEPTION_AS_ERROR_ENABLED, + ConfigDefaults.DEFAULT_JAX_RS_EXCEPTION_AS_ERROR_ENABLED); + this.traceFlushIntervalSeconds = configProvider.getFloat( TracerConfig.TRACE_FLUSH_INTERVAL, ConfigDefaults.DEFAULT_TRACE_FLUSH_INTERVAL); @@ -2791,6 +2798,10 @@ public boolean isSparkTaskHistogramEnabled() { return sparkTaskHistogramEnabled; } + public boolean isJaxRsExceptionAsErrorEnabled() { + return jaxRsExceptionAsErrorsEnabled; + } + /** @return A map of tags to be applied only to the local application root span. */ public Map getLocalRootSpanTags() { final Map runtimeTags = getRuntimeTags(); @@ -3805,6 +3816,8 @@ public String toString() { + logsInjectionEnabled + ", sparkTaskHistogramEnabled=" + sparkTaskHistogramEnabled + + ", jaxRsExceptionAsErrorsEnabled=" + + jaxRsExceptionAsErrorsEnabled + '}'; } } diff --git a/utils/test-utils/src/main/groovy/datadog/trace/test/util/Flaky.java b/utils/test-utils/src/main/groovy/datadog/trace/test/util/Flaky.java index 1bb8bdc8e70..66ff702b03c 100644 --- a/utils/test-utils/src/main/groovy/datadog/trace/test/util/Flaky.java +++ b/utils/test-utils/src/main/groovy/datadog/trace/test/util/Flaky.java @@ -8,7 +8,7 @@ /** * Use this annotation for suites or test cases that are flaky. When running in CI, these will be - * seggregated to a separate job. + * segregated to a separate job. */ @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.TYPE, ElementType.METHOD}) From 14559b42a69c9dcb57ce253f890a70039505b6e6 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Wed, 9 Aug 2023 12:24:15 +0200 Subject: [PATCH 005/367] Exclude Jetty server 12 from muzzle checks (#5699) --- dd-java-agent/instrumentation/jetty-11/build.gradle | 2 +- dd-java-agent/instrumentation/jetty-9/build.gradle | 4 ++-- dd-java-agent/instrumentation/jetty-appsec-9.3/build.gradle | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dd-java-agent/instrumentation/jetty-11/build.gradle b/dd-java-agent/instrumentation/jetty-11/build.gradle index af27d9e38d4..164a3e4c8ba 100644 --- a/dd-java-agent/instrumentation/jetty-11/build.gradle +++ b/dd-java-agent/instrumentation/jetty-11/build.gradle @@ -6,7 +6,7 @@ muzzle { pass { group = "org.eclipse.jetty" module = 'jetty-server' - versions = "[11,)" + versions = "[11,12)" } } diff --git a/dd-java-agent/instrumentation/jetty-9/build.gradle b/dd-java-agent/instrumentation/jetty-9/build.gradle index 78713213a55..aa50577bf9c 100644 --- a/dd-java-agent/instrumentation/jetty-9/build.gradle +++ b/dd-java-agent/instrumentation/jetty-9/build.gradle @@ -43,10 +43,10 @@ muzzle { javaVersion = 11 } pass { - name = 'after_10' + name = 'between_10_and_12' group = "org.eclipse.jetty" module = 'jetty-server' - versions = "[10,)" + versions = "[10,12)" assertInverse = true javaVersion = 11 } diff --git a/dd-java-agent/instrumentation/jetty-appsec-9.3/build.gradle b/dd-java-agent/instrumentation/jetty-appsec-9.3/build.gradle index fc0df591f77..69bad38c12b 100644 --- a/dd-java-agent/instrumentation/jetty-appsec-9.3/build.gradle +++ b/dd-java-agent/instrumentation/jetty-appsec-9.3/build.gradle @@ -2,7 +2,7 @@ muzzle { pass { group = 'org.eclipse.jetty' module = 'jetty-server' - versions = '[9.3,]' + versions = '[9.3,12)' assertInverse = true } } From 915394fee05c529e82d84a596e76925f2e33fc72 Mon Sep 17 00:00:00 2001 From: "Santiago M. Mola" Date: Wed, 9 Aug 2023 12:55:40 +0200 Subject: [PATCH 006/367] Remove docker-compose install in system-tests (#5695) Not needed anymore. --- .circleci/config.yml | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index f37f89f7d33..96e7adcaf1e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -251,25 +251,12 @@ commands: - restore_build_cache: cacheType: lib - - run: - # TODO: removes this step once host-in-runner is merged on system tests - name: Install good version of docker-compose - command: | - sudo curl -L "https://github.com/docker/compose/releases/download/1.29.2/docker-compose-$(uname -s)-$(uname -m)" -o /usr/local/bin/docker-compose - sudo chmod +x /usr/local/bin/docker-compose - - run: name: Install python 3.9 command: | sudo apt-get install python3.9-full python3.9-dev python3.9-venv echo 'export PATH="$HOME/.local/bin:$PATH"' >>"$BASH_ENV" - - run: - name: versions - command: | - docker --version - docker-compose --version - - run: name: Clone system-tests command: | From 42a3ef6e44adb6cc08e05735a732791e0ede7c45 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Wed, 9 Aug 2023 17:49:43 +0200 Subject: [PATCH 007/367] Fix typo in OpenTracing compatibility module name (#5700) --- dd-trace-ot/build.gradle | 12 ++++++------ .../groovy/OT31ApiTest.groovy | 0 .../java/io/opentracing/tag/Tag.java | 0 .../groovy/OT33ApiTest.groovy | 0 4 files changed, 6 insertions(+), 6 deletions(-) rename dd-trace-ot/src/{ot31CompatabilityTest => ot31CompatibilityTest}/groovy/OT31ApiTest.groovy (100%) rename dd-trace-ot/src/{ot31CompatabilityTest => ot31CompatibilityTest}/java/io/opentracing/tag/Tag.java (100%) rename dd-trace-ot/src/{ot33CompatabilityTest => ot33CompatibilityTest}/groovy/OT33ApiTest.groovy (100%) diff --git a/dd-trace-ot/build.gradle b/dd-trace-ot/build.gradle index 93f4f546844..d5025e9b5c9 100644 --- a/dd-trace-ot/build.gradle +++ b/dd-trace-ot/build.gradle @@ -22,8 +22,8 @@ excludedClassesCoverage += [ "datadog.opentracing.DDTracer.DDTracerBuilder" ] -addTestSuite('ot31CompatabilityTest') -addTestSuite('ot33CompatabilityTest') +addTestSuite('ot31CompatibilityTest') +addTestSuite('ot33CompatibilityTest') dependencies { annotationProcessor deps.autoserviceProcessor @@ -46,17 +46,17 @@ dependencies { testImplementation project(":dd-java-agent:testing") - ot33CompatabilityTestImplementation('io.opentracing:opentracing-api') { + ot33CompatibilityTestImplementation('io.opentracing:opentracing-api') { version { strictly '0.33.0' } } - ot33CompatabilityTestImplementation('io.opentracing:opentracing-util') { + ot33CompatibilityTestImplementation('io.opentracing:opentracing-util') { version { strictly '0.33.0' } } - ot33CompatabilityTestImplementation('io.opentracing:opentracing-noop') { + ot33CompatibilityTestImplementation('io.opentracing:opentracing-noop') { version { strictly '0.33.0' } @@ -73,7 +73,7 @@ configurations.matching({ it.name.startsWith('ot31') }).each({ }) tasks.named("test").configure { - finalizedBy "ot31CompatabilityTest", "ot33CompatabilityTest" + finalizedBy "ot31CompatibilityTest", "ot33CompatibilityTest" } jar { diff --git a/dd-trace-ot/src/ot31CompatabilityTest/groovy/OT31ApiTest.groovy b/dd-trace-ot/src/ot31CompatibilityTest/groovy/OT31ApiTest.groovy similarity index 100% rename from dd-trace-ot/src/ot31CompatabilityTest/groovy/OT31ApiTest.groovy rename to dd-trace-ot/src/ot31CompatibilityTest/groovy/OT31ApiTest.groovy diff --git a/dd-trace-ot/src/ot31CompatabilityTest/java/io/opentracing/tag/Tag.java b/dd-trace-ot/src/ot31CompatibilityTest/java/io/opentracing/tag/Tag.java similarity index 100% rename from dd-trace-ot/src/ot31CompatabilityTest/java/io/opentracing/tag/Tag.java rename to dd-trace-ot/src/ot31CompatibilityTest/java/io/opentracing/tag/Tag.java diff --git a/dd-trace-ot/src/ot33CompatabilityTest/groovy/OT33ApiTest.groovy b/dd-trace-ot/src/ot33CompatibilityTest/groovy/OT33ApiTest.groovy similarity index 100% rename from dd-trace-ot/src/ot33CompatabilityTest/groovy/OT33ApiTest.groovy rename to dd-trace-ot/src/ot33CompatibilityTest/groovy/OT33ApiTest.groovy From 5e9215560050e7edaf6a582c20a18434ba526ea1 Mon Sep 17 00:00:00 2001 From: Javier Santos Date: Wed, 9 Aug 2023 20:06:03 +0200 Subject: [PATCH 008/367] HSTS missing header vulnerability detection (#5520) * IAST - HSTS missing header vulnerability detection --- .../decorator/HttpServerDecorator.java | 33 +++- .../com/datadog/iast/IastRequestContext.java | 30 +++ .../java/com/datadog/iast/IastSystem.java | 12 ++ .../com/datadog/iast/RequestEndedHandler.java | 6 + .../datadog/iast/RequestHeaderHandler.java | 19 ++ .../datadog/iast/model/VulnerabilityType.java | 2 + .../sink/HstsMissingHeaderModuleImpl.java | 88 +++++++++ .../sink/HttpResponseHeaderModuleImpl.java | 25 ++- .../com/datadog/iast/IastSystemTest.groovy | 8 + .../iast/RequestEndedHandlerTest.groovy | 5 + .../iast/RequestHeaderHandlerTest.groovy | 42 ++++ .../sink/HstsMissingHeaderModuleTest.groovy | 185 ++++++++++++++++++ .../sink/HttpResponseHeaderModuleTest.groovy | 30 +++ .../AbstractIastSpringBootTest.groovy | 15 ++ .../controller/IastWebController.java | 6 + .../controller/IastWebController.java | 6 + .../trace/api/iast/InstrumentationBridge.java | 8 + .../trace/api/iast/VulnerabilityTypes.java | 1 + .../iast/sink/HstsMissingHeaderModule.java | 10 + 19 files changed, 520 insertions(+), 11 deletions(-) create mode 100644 dd-java-agent/agent-iast/src/main/java/com/datadog/iast/RequestHeaderHandler.java create mode 100644 dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/HstsMissingHeaderModuleImpl.java create mode 100644 dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/RequestHeaderHandlerTest.groovy create mode 100644 dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/HstsMissingHeaderModuleTest.groovy create mode 100644 internal-api/src/main/java/datadog/trace/api/iast/sink/HstsMissingHeaderModule.java diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java index 3991a262c25..45039bbfaaa 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java @@ -401,20 +401,33 @@ public AgentSpan onError(final AgentSpan span, final Throwable throwable) { } private Flow callIGCallbackRequestHeaders(AgentSpan span, REQUEST_CARRIER carrier) { - CallbackProvider cbp = tracer().getCallbackProvider(RequestContextSlot.APPSEC); + CallbackProvider cbpAppsec = tracer().getCallbackProvider(RequestContextSlot.APPSEC); + CallbackProvider cbpIast = tracer().getCallbackProvider(RequestContextSlot.IAST); RequestContext requestContext = span.getRequestContext(); AgentPropagation.ContextVisitor getter = getter(); - if (requestContext == null || cbp == null || getter == null) { + if (requestContext == null || getter == null) { return Flow.ResultFlow.empty(); } - IGKeyClassifier igKeyClassifier = - IGKeyClassifier.create( - requestContext, - cbp.getCallback(EVENTS.requestHeader()), - cbp.getCallback(EVENTS.requestHeaderDone())); - if (null != igKeyClassifier) { - getter.forEachKey(carrier, igKeyClassifier); - return igKeyClassifier.done(); + if (cbpIast != null) { + IGKeyClassifier igKeyClassifier = + IGKeyClassifier.create( + requestContext, + cbpIast.getCallback(EVENTS.requestHeader()), + cbpIast.getCallback(EVENTS.requestHeaderDone())); + if (null != igKeyClassifier) { + getter.forEachKey(carrier, igKeyClassifier); + } + } + if (cbpAppsec != null) { + IGKeyClassifier igKeyClassifier = + IGKeyClassifier.create( + requestContext, + cbpAppsec.getCallback(EVENTS.requestHeader()), + cbpAppsec.getCallback(EVENTS.requestHeaderDone())); + if (null != igKeyClassifier) { + getter.forEachKey(carrier, igKeyClassifier); + return igKeyClassifier.done(); + } } return Flow.ResultFlow.empty(); } diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastRequestContext.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastRequestContext.java index d6d79978118..337d973516d 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastRequestContext.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastRequestContext.java @@ -19,6 +19,9 @@ public class IastRequestContext implements HasMetricCollector { private final TaintedObjects taintedObjects; private final OverheadContext overheadContext; private final IastMetricCollector collector; + private volatile boolean hstsHeaderIsSet; + private volatile boolean xForwardedProtoIsHtttps; + private volatile String contentType; public IastRequestContext() { this(TaintedObjects.acquire(), null); @@ -32,6 +35,9 @@ public IastRequestContext( final TaintedObjects taintedObjects, final IastMetricCollector collector) { this.vulnerabilityBatch = new VulnerabilityBatch(); this.spanDataIsSet = new AtomicBoolean(false); + this.hstsHeaderIsSet = false; + this.xForwardedProtoIsHtttps = false; + this.contentType = null; this.overheadContext = new OverheadContext(); this.taintedObjects = taintedObjects; this.collector = collector; @@ -41,6 +47,30 @@ public VulnerabilityBatch getVulnerabilityBatch() { return vulnerabilityBatch; } + public void setHstsHeaderIsSet() { + hstsHeaderIsSet = true; + } + + public boolean getHstsHeaderIsSet() { + return hstsHeaderIsSet; + } + + public void setXForwardedProtoIsHtttps() { + xForwardedProtoIsHtttps = true; + } + + public boolean getXForwardedProtoIsHtttps() { + return xForwardedProtoIsHtttps; + } + + public void setContentType(String contentType) { + this.contentType = contentType; + } + + public String getContentType() { + return contentType; + } + public boolean getAndSetSpanDataIsSet() { return spanDataIsSet.getAndSet(true); } diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastSystem.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastSystem.java index df7c8b5cd58..eb95fe38689 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastSystem.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastSystem.java @@ -6,6 +6,7 @@ import com.datadog.iast.propagation.PropagationModuleImpl; import com.datadog.iast.propagation.StringModuleImpl; import com.datadog.iast.sink.CommandInjectionModuleImpl; +import com.datadog.iast.sink.HstsMissingHeaderModuleImpl; import com.datadog.iast.sink.HttpResponseHeaderModuleImpl; import com.datadog.iast.sink.InsecureCookieModuleImpl; import com.datadog.iast.sink.LdapInjectionModuleImpl; @@ -26,6 +27,7 @@ import com.datadog.iast.telemetry.TelemetryRequestStartedHandler; import datadog.trace.api.Config; import datadog.trace.api.ProductActivation; +import datadog.trace.api.function.TriConsumer; import datadog.trace.api.gateway.EventType; import datadog.trace.api.gateway.Events; import datadog.trace.api.gateway.Flow; @@ -71,6 +73,7 @@ public static void start(final SubscriptionService ss, OverheadController overhe iastModules().forEach(registerModule(dependencies)); registerRequestStartedCallback(ss, addTelemetry, dependencies); registerRequestEndedCallback(ss, addTelemetry, dependencies); + registerHeadersCallback(ss, dependencies); LOGGER.debug("IAST started"); } @@ -96,6 +99,7 @@ private static Stream iastModules() { new LdapInjectionModuleImpl(), new PropagationModuleImpl(), new HttpResponseHeaderModuleImpl(), + new HstsMissingHeaderModuleImpl(), new InsecureCookieModuleImpl(), new NoHttpOnlyCookieModuleImpl(), new NoSameSiteCookieModuleImpl(), @@ -124,4 +128,12 @@ private static void registerRequestEndedCallback( final RequestEndedHandler handler = new RequestEndedHandler(dependencies); ss.registerCallback(event, addTelemetry ? new TelemetryRequestEndedHandler(handler) : handler); } + + private static void registerHeadersCallback( + final SubscriptionService ss, final Dependencies dependencies) { + final EventType> event = + Events.get().requestHeader(); + final TriConsumer handler = new RequestHeaderHandler(); + ss.registerCallback(event, handler); + } } diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/RequestEndedHandler.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/RequestEndedHandler.java index 1dcadd078d5..31206008128 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/RequestEndedHandler.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/RequestEndedHandler.java @@ -9,6 +9,8 @@ import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.IGSpanInfo; import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.iast.InstrumentationBridge; +import datadog.trace.api.iast.sink.HstsMissingHeaderModule; import datadog.trace.api.internal.TraceSegment; import java.util.function.BiFunction; import javax.annotation.Nonnull; @@ -26,6 +28,10 @@ public Flow apply(final RequestContext requestContext, final IGSpanInfo ig final TraceSegment traceSegment = requestContext.getTraceSegment(); final IastRequestContext iastRequestContext = IastRequestContext.get(requestContext); if (iastRequestContext != null) { + HstsMissingHeaderModule mod = InstrumentationBridge.HSTS_MISSING_HEADER_MODULE; + if (mod != null) { + mod.onRequestEnd(iastRequestContext, igSpanInfo); + } try { ANALYZED.setTagTop(traceSegment); final TaintedObjects taintedObjects = iastRequestContext.getTaintedObjects(); diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/RequestHeaderHandler.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/RequestHeaderHandler.java new file mode 100644 index 00000000000..a77907aca43 --- /dev/null +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/RequestHeaderHandler.java @@ -0,0 +1,19 @@ +package com.datadog.iast; + +import datadog.trace.api.function.TriConsumer; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; + +public class RequestHeaderHandler implements TriConsumer { + private static final String X_FORWARDED_PROTO_HEADER = "X-Forwarded-Proto"; + + @Override + public void accept(RequestContext requestContext, String key, String value) { + final IastRequestContext ctx = requestContext.getData(RequestContextSlot.IAST); + if (null != ctx + && X_FORWARDED_PROTO_HEADER.equalsIgnoreCase(key) + && "https".equalsIgnoreCase(value)) { + ctx.setXForwardedProtoIsHtttps(); + } + } +} diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/model/VulnerabilityType.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/model/VulnerabilityType.java index 6b204720c96..eb46b79c18f 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/model/VulnerabilityType.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/model/VulnerabilityType.java @@ -16,6 +16,8 @@ public interface VulnerabilityType { new VulnerabilityTypeImpl(VulnerabilityTypes.INSECURE_COOKIE, NOT_MARKED); VulnerabilityType NO_HTTPONLY_COOKIE = new VulnerabilityTypeImpl(VulnerabilityTypes.NO_HTTPONLY_COOKIE, NOT_MARKED); + VulnerabilityType HSTS_HEADER_MISSING = + new VulnerabilityTypeImpl(VulnerabilityTypes.HSTS_HEADER_MISSING, NOT_MARKED); VulnerabilityType NO_SAMESITE_COOKIE = new VulnerabilityTypeImpl(VulnerabilityTypes.NO_SAMESITE_COOKIE, NOT_MARKED); diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/HstsMissingHeaderModuleImpl.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/HstsMissingHeaderModuleImpl.java new file mode 100644 index 00000000000..283332cfde3 --- /dev/null +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/HstsMissingHeaderModuleImpl.java @@ -0,0 +1,88 @@ +package com.datadog.iast.sink; + +import com.datadog.iast.IastRequestContext; +import com.datadog.iast.model.Vulnerability; +import com.datadog.iast.model.VulnerabilityType; +import com.datadog.iast.overhead.Operations; +import datadog.trace.api.gateway.IGSpanInfo; +import datadog.trace.api.iast.sink.HstsMissingHeaderModule; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import java.net.HttpURLConnection; +import java.net.URL; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import javax.annotation.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class HstsMissingHeaderModuleImpl extends SinkModuleBase implements HstsMissingHeaderModule { + private static final Pattern MAX_AGE = + Pattern.compile("max-age=(\\d+)", Pattern.CASE_INSENSITIVE); + + private static final Logger LOGGER = LoggerFactory.getLogger(HstsMissingHeaderModuleImpl.class); + + @Override + public void onRequestEnd(final Object iastRequestContextObject, final IGSpanInfo igSpanInfo) { + + final IastRequestContext iastRequestContext = (IastRequestContext) iastRequestContextObject; + + if (!iastRequestContext.getHstsHeaderIsSet()) { + try { + Map tags = igSpanInfo.getTags(); + String urlString = (String) tags.get("http.url"); + if (null != tags.get("http.status_code")) { + Integer httpStatus = (Integer) tags.get("http.status_code"); + if (httpStatus == HttpURLConnection.HTTP_MOVED_PERM + || httpStatus == HttpURLConnection.HTTP_MOVED_TEMP + || httpStatus == HttpURLConnection.HTTP_NOT_MODIFIED + || httpStatus == HttpURLConnection.HTTP_NOT_FOUND + || httpStatus == HttpURLConnection.HTTP_GONE + || httpStatus == HttpURLConnection.HTTP_INTERNAL_ERROR + || httpStatus == 307) { + return; + } + } + URL url = new URL(urlString); + if (null != iastRequestContext.getContentType() + && (iastRequestContext.getContentType().contains("text/html") + || iastRequestContext.getContentType().contains("application/xhtml+xml")) + && (url.getProtocol().equals("https") + || iastRequestContext.getXForwardedProtoIsHtttps())) { + final AgentSpan span = AgentTracer.activeSpan(); + if (overheadController.consumeQuota(Operations.REPORT_VULNERABILITY, span)) { + reporter.report( + span, new Vulnerability(VulnerabilityType.HSTS_HEADER_MISSING, null, null)); + } + } + } catch (Throwable e) { + LOGGER.debug("Exception while checking for missing HSTS headers vulnerability", e); + } + } + } + + @Override + public void onHstsHeader(String value) { + if (isValidMaxAge(value)) { + final AgentSpan span = AgentTracer.activeSpan(); + final IastRequestContext ctx = IastRequestContext.get(span); + if (ctx == null) { + return; + } else { + ctx.setHstsHeaderIsSet(); + } + } + } + + public static boolean isValidMaxAge(@Nullable final String value) { + if (value == null) { + return false; + } + final Matcher matcher = MAX_AGE.matcher(value); + if (!matcher.find()) { + return false; + } + return Integer.parseInt(matcher.group(1)) > 0; + } +} diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/HttpResponseHeaderModuleImpl.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/HttpResponseHeaderModuleImpl.java index 01f486502ef..e67be4bbbaa 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/HttpResponseHeaderModuleImpl.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/HttpResponseHeaderModuleImpl.java @@ -2,6 +2,7 @@ import static java.util.Collections.singletonList; +import com.datadog.iast.IastRequestContext; import com.datadog.iast.model.Evidence; import com.datadog.iast.model.Location; import com.datadog.iast.model.Vulnerability; @@ -9,6 +10,7 @@ import com.datadog.iast.overhead.Operations; import com.datadog.iast.util.CookieSecurityParser; import datadog.trace.api.iast.InstrumentationBridge; +import datadog.trace.api.iast.sink.HstsMissingHeaderModule; import datadog.trace.api.iast.sink.HttpCookieModule; import datadog.trace.api.iast.sink.HttpResponseHeaderModule; import datadog.trace.api.iast.util.Cookie; @@ -19,14 +21,35 @@ import java.util.List; import java.util.Map; import javax.annotation.Nonnull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class HttpResponseHeaderModuleImpl extends SinkModuleBase implements HttpResponseHeaderModule { + + private static final Logger LOGGER = LoggerFactory.getLogger(HttpResponseHeaderModuleImpl.class); private static final String SET_COOKIE_HEADER = "Set-Cookie"; + private static final String HSTS_HEADER = "Strict-Transport-Security"; + + private static final String CONTENT_TYPE_HEADER = "Content-Type"; @Override public void onHeader(@Nonnull final String name, final String value) { - if (SET_COOKIE_HEADER.equalsIgnoreCase(name)) { + if (HSTS_HEADER.equalsIgnoreCase(name)) { + HstsMissingHeaderModule mod = InstrumentationBridge.HSTS_MISSING_HEADER_MODULE; + if (mod != null) { + mod.onHstsHeader(value); + } + } else if (CONTENT_TYPE_HEADER.equalsIgnoreCase(name)) { + final AgentSpan span = AgentTracer.activeSpan(); + final IastRequestContext ctx = IastRequestContext.get(span); + if (ctx == null) { + return; + } else { + ctx.setContentType(value); + } + } else if (SET_COOKIE_HEADER.equalsIgnoreCase(name)) { + CookieSecurityParser cookieSecurityInfo = new CookieSecurityParser(); onCookies(CookieSecurityParser.parse(value)); } if (null != InstrumentationBridge.UNVALIDATED_REDIRECT) { diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastSystemTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastSystemTest.groovy index f6b610bc45b..eb3e7f31ff2 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastSystemTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastSystemTest.groovy @@ -1,5 +1,6 @@ package com.datadog.iast +import datadog.trace.api.iast.InstrumentationBridge import datadog.trace.api.internal.TraceSegment import datadog.trace.api.gateway.InstrumentationGateway import datadog.trace.api.gateway.RequestContextSlot @@ -11,6 +12,10 @@ import datadog.trace.test.util.DDSpecification class IastSystemTest extends DDSpecification { + def setup() { + InstrumentationBridge.clearIastModules() + } + void 'start'() { given: final ig = new InstrumentationGateway() @@ -33,6 +38,7 @@ class IastSystemTest extends DDSpecification { then: 1 * ss.registerCallback(Events.get().requestStarted(), _) 1 * ss.registerCallback(Events.get().requestEnded(), _) + 1 * ss.registerCallback(Events.get().requestHeader(), _) 0 * _ when: @@ -52,6 +58,8 @@ class IastSystemTest extends DDSpecification { 1 * iastContext.getTaintedObjects() 1 * iastContext.getMetricCollector() 1 * traceSegment.setTagTop('_dd.iast.enabled', 1) + 1 * igSpanInfo.getTags() + 1 * iastContext.getHstsHeaderIsSet() 0 * _ noExceptionThrown() } diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/RequestEndedHandlerTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/RequestEndedHandlerTest.groovy index f1d584b4eae..b205d9d3eba 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/RequestEndedHandlerTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/RequestEndedHandlerTest.groovy @@ -7,6 +7,7 @@ import datadog.trace.api.gateway.Flow import datadog.trace.api.gateway.IGSpanInfo import datadog.trace.api.gateway.RequestContext import datadog.trace.api.gateway.RequestContextSlot +import datadog.trace.api.iast.InstrumentationBridge import datadog.trace.api.internal.TraceSegment import datadog.trace.test.util.DDSpecification import datadog.trace.util.stacktrace.StackWalker @@ -15,6 +16,10 @@ import groovy.transform.CompileDynamic @CompileDynamic class RequestEndedHandlerTest extends DDSpecification { + def setup() { + InstrumentationBridge.clearIastModules() + } + void 'request ends with IAST context'() { given: final OverheadController overheadController = Mock(OverheadController) diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/RequestHeaderHandlerTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/RequestHeaderHandlerTest.groovy new file mode 100644 index 00000000000..82968e3ffcd --- /dev/null +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/RequestHeaderHandlerTest.groovy @@ -0,0 +1,42 @@ +package com.datadog.iast + +import datadog.trace.api.gateway.RequestContext +import datadog.trace.api.gateway.RequestContextSlot +import datadog.trace.test.util.DDSpecification +import groovy.transform.CompileDynamic + +@CompileDynamic +class RequestHeaderHandlerTest extends DDSpecification { + void 'forwarded proto is set'(){ + given: + final handler = new RequestHeaderHandler() + final iastCtx = Mock(IastRequestContext) + final ctx = Mock(RequestContext) + ctx.getData(RequestContextSlot.IAST) >> iastCtx + + when: + handler.accept(ctx, 'X-Forwarded-Proto', 'https') + + then: + 1 * ctx.getData(RequestContextSlot.IAST) >> iastCtx + 1 * iastCtx.setXForwardedProtoIsHtttps() + 0 * _ + } + + + void 'forwarded proto is not set'(){ + given: + final handler = new RequestHeaderHandler() + final iastCtx = Mock(IastRequestContext) + final ctx = Mock(RequestContext) + ctx.getData(RequestContextSlot.IAST) >> iastCtx + + when: + handler.accept(ctx, 'Custom-Header', 'https') + + then: + 1 * ctx.getData(RequestContextSlot.IAST) >> iastCtx + 0 * iastCtx.setXForwardedProtoIsHtttps() + 0 * _ + } +} diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/HstsMissingHeaderModuleTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/HstsMissingHeaderModuleTest.groovy new file mode 100644 index 00000000000..2e4768d7bca --- /dev/null +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/HstsMissingHeaderModuleTest.groovy @@ -0,0 +1,185 @@ +package com.datadog.iast.sink + +import com.datadog.iast.HasDependencies +import com.datadog.iast.IastModuleImplTestBase +import com.datadog.iast.IastRequestContext +import com.datadog.iast.RequestEndedHandler +import com.datadog.iast.model.Vulnerability +import com.datadog.iast.model.VulnerabilityType +import com.datadog.iast.overhead.OverheadController +import datadog.trace.api.Config +import datadog.trace.api.gateway.Flow +import datadog.trace.api.gateway.IGSpanInfo +import datadog.trace.api.gateway.RequestContext +import datadog.trace.api.gateway.RequestContextSlot +import datadog.trace.api.iast.InstrumentationBridge +import datadog.trace.api.internal.TraceSegment +import datadog.trace.bootstrap.instrumentation.api.AgentSpan +import datadog.trace.util.stacktrace.StackWalker + +public class HstsMissingHeaderModuleTest extends IastModuleImplTestBase { + + private List objectHolder + + private IastRequestContext ctx + + private HstsMissingHeaderModuleImpl module + + private AgentSpan span + + def setup() { + InstrumentationBridge.clearIastModules() + module = registerDependencies(new HstsMissingHeaderModuleImpl()) + InstrumentationBridge.registerIastModule(module) + objectHolder = [] + ctx = new IastRequestContext() + final reqCtx = Mock(RequestContext) { + getData(RequestContextSlot.IAST) >> ctx + } + span = Mock(AgentSpan) { + getSpanId() >> 123456 + getRequestContext() >> reqCtx + } + } + + + void 'hsts vulnerability'() { + given: + Vulnerability savedVul1 + final OverheadController overheadController = Mock(OverheadController) + final iastCtx = Mock(IastRequestContext) + iastCtx.getXForwardedProtoIsHtttps() >> true + iastCtx.getContentType() >> "text/html" + final StackWalker stackWalker = Mock(StackWalker) + final dependencies = new HasDependencies.Dependencies( + Config.get(), reporter, overheadController, stackWalker + ) + final handler = new RequestEndedHandler(dependencies) + final TraceSegment traceSegment = Mock(TraceSegment) + final reqCtx = Mock(RequestContext) + reqCtx.getTraceSegment() >> traceSegment + reqCtx.getData(RequestContextSlot.IAST) >> iastCtx + final tags = Mock(Map) + tags.get("http.url") >> "https://localhost/a" + tags.get("http.status_code") >> 200i + final spanInfo = Mock(IGSpanInfo) + spanInfo.getTags() >> tags + IastRequestContext.get(span) >> iastCtx + + + when: + def flow = handler.apply(reqCtx, spanInfo) + + then: + flow.getAction() == Flow.Action.Noop.INSTANCE + flow.getResult() == null + 1 * reqCtx.getData(RequestContextSlot.IAST) >> iastCtx + 1 * reqCtx.getTraceSegment() >> traceSegment + 1 * traceSegment.setTagTop("_dd.iast.enabled", 1) + 1 * iastCtx.getTaintedObjects() >> null + 1 * overheadController.releaseRequest() + 1 * spanInfo.getTags() >> tags + 1 * tags.get('http.url') >> "https://localhost/a" + 2 * tags.get('http.status_code') >> 200i + 1 * iastCtx.getHstsHeaderIsSet() + 1 * tracer.activeSpan() >> span + 2 * iastCtx.getContentType() >> "text/html" + 1 * reporter.report(_, _ as Vulnerability) >> { + savedVul1 = it[1] + } + + with(savedVul1) { + type == VulnerabilityType.HSTS_HEADER_MISSING + } + } + + + void 'no hsts vulnerability reported'() { + given: + Vulnerability savedVul1 + final OverheadController overheadController = Mock(OverheadController) + final iastCtx = Mock(IastRequestContext) + iastCtx.getXForwardedProtoIsHtttps() >> true + iastCtx.getContentType() >> "text/html" + final StackWalker stackWalker = Mock(StackWalker) + final dependencies = new HasDependencies.Dependencies( + Config.get(), reporter, overheadController, stackWalker + ) + final handler = new RequestEndedHandler(dependencies) + final TraceSegment traceSegment = Mock(TraceSegment) + final reqCtx = Mock(RequestContext) + reqCtx.getTraceSegment() >> traceSegment + reqCtx.getData(RequestContextSlot.IAST) >> iastCtx + final tags = Mock(Map) + tags.get("http.url") >> url + tags.get("http.status_code") >> status + final spanInfo = Mock(IGSpanInfo) + spanInfo.getTags() >> tags + IastRequestContext.get(span) >> iastCtx + + + when: + def flow = handler.apply(reqCtx, spanInfo) + + then: + flow.getAction() == Flow.Action.Noop.INSTANCE + flow.getResult() == null + 1 * reqCtx.getData(RequestContextSlot.IAST) >> iastCtx + 1 * reqCtx.getTraceSegment() >> traceSegment + 1 * traceSegment.setTagTop("_dd.iast.enabled", 1) + 1 * iastCtx.getTaintedObjects() >> null + 1 * overheadController.releaseRequest() + 1 * spanInfo.getTags() >> tags + 1 * tags.get('http.url') >> url + 2 * tags.get('http.status_code') >> status + 1 * iastCtx.getHstsHeaderIsSet() + 0 * _ + + where: + url | status + "https://localhost/a" | 307i + "https://localhost/a" | HttpURLConnection.HTTP_MOVED_PERM + "https://localhost/a" | HttpURLConnection.HTTP_MOVED_TEMP + "https://localhost/a" | HttpURLConnection.HTTP_NOT_MODIFIED + "https://localhost/a" | HttpURLConnection.HTTP_NOT_FOUND + "https://localhost/a" | HttpURLConnection.HTTP_GONE + "https://localhost/a" | HttpURLConnection.HTTP_INTERNAL_ERROR + } + + + + void 'throw exception if context is null'(){ + when: + module.onRequestEnd(null, null) + + then: + thrown(NullPointerException) + } + + void 'exception not thrown if igSpanInfo is null'(){ + when: + module.onRequestEnd(ctx, null) + + then: + noExceptionThrown() + } + + void 'test max age'(){ + when: + final result = HstsMissingHeaderModuleImpl.isValidMaxAge(value) + + then: + result == expected + + where: + value | expected + "max-age=0" | false + "max-age=-1" | false + null | false + "" | false + "max-age-3" | false + "ramdom" | false + "max-age=10" | true + "max-age=0122344" | true + } +} diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/HttpResponseHeaderModuleTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/HttpResponseHeaderModuleTest.groovy index 34e26d60e99..3f8cb4ed5fb 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/HttpResponseHeaderModuleTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/HttpResponseHeaderModuleTest.groovy @@ -1,12 +1,15 @@ package com.datadog.iast.sink + import com.datadog.iast.IastModuleImplTestBase import com.datadog.iast.IastRequestContext import com.datadog.iast.model.Vulnerability import com.datadog.iast.model.VulnerabilityType +import com.datadog.iast.taint.TaintedObjects import datadog.trace.api.gateway.RequestContext import datadog.trace.api.gateway.RequestContextSlot import datadog.trace.api.iast.InstrumentationBridge +import datadog.trace.api.iast.telemetry.IastMetricCollector import datadog.trace.api.iast.util.Cookie import datadog.trace.bootstrap.instrumentation.api.AgentSpan @@ -23,6 +26,7 @@ class HttpResponseHeaderModuleTest extends IastModuleImplTestBase { def setup() { InstrumentationBridge.clearIastModules() module = registerDependencies(new HttpResponseHeaderModuleImpl()) + InstrumentationBridge.registerIastModule(module) InstrumentationBridge.registerIastModule(new InsecureCookieModuleImpl()) InstrumentationBridge.registerIastModule(new NoHttpOnlyCookieModuleImpl()) InstrumentationBridge.registerIastModule(new NoSameSiteCookieModuleImpl()) @@ -87,4 +91,30 @@ class HttpResponseHeaderModuleTest extends IastModuleImplTestBase { 1 * overheadController.consumeQuota(_,_) 0 * _ } + + void 'exercise IastRequestController'(){ + given: + final taintedObjects = Mock(TaintedObjects) + IastRequestContext ctx = new IastRequestContext(taintedObjects) + + when: + ctx.setXForwardedProtoIsHtttps() + + then: + ctx.getXForwardedProtoIsHtttps() + } + + void 'exercise IastRequestContext'(){ + given: + final taintedObjects = Mock(TaintedObjects) + final iastMetricsCollector = Mock(IastMetricCollector) + + when: + IastRequestContext ctx = new IastRequestContext(taintedObjects, iastMetricsCollector) + ctx.setXForwardedProtoIsHtttps() + ctx.setContentType("text/html") + + then: + 0 * _ + } } diff --git a/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy b/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy index 83fc9de3bae..386e3f2ba1d 100644 --- a/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy +++ b/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy @@ -127,6 +127,21 @@ abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest { } } + void 'hsts header missing vulnerability is present'() { + setup: + String url = "http://localhost:${httpPort}/hstsmissing" + def request = new Request.Builder().url(url).header("X-Forwarded-Proto", "https").get().build() + + when: + def response = client.newCall(request).execute() + + then: + response.isSuccessful() + hasVulnerability { vul -> + vul.type == 'HSTS_HEADER_MISSING' + } + } + void 'no HttpOnly cookie vulnerability is present'() { setup: String url = "http://localhost:${httpPort}/insecure_cookie" diff --git a/dd-smoke-tests/spring-boot-2.6-webmvc/src/main/java/datadog/smoketest/springboot/controller/IastWebController.java b/dd-smoke-tests/spring-boot-2.6-webmvc/src/main/java/datadog/smoketest/springboot/controller/IastWebController.java index eff7eeb5f56..bf085a7a286 100644 --- a/dd-smoke-tests/spring-boot-2.6-webmvc/src/main/java/datadog/smoketest/springboot/controller/IastWebController.java +++ b/dd-smoke-tests/spring-boot-2.6-webmvc/src/main/java/datadog/smoketest/springboot/controller/IastWebController.java @@ -295,6 +295,12 @@ public String trustBoundaryViolationForCookie(final HttpServletRequest request) return "Trust Boundary violation with cookie page"; } + @GetMapping(value = "/hstsmissing", produces = "text/html") + public String hstsHeaderMissing(HttpServletResponse response) { + response.setStatus(HttpStatus.OK.value()); + return "ok"; + } + private void withProcess(final Operation op) { Process process = null; try { diff --git a/dd-smoke-tests/springboot/src/main/java/datadog/smoketest/springboot/controller/IastWebController.java b/dd-smoke-tests/springboot/src/main/java/datadog/smoketest/springboot/controller/IastWebController.java index 35db70370d0..70fdb08471f 100644 --- a/dd-smoke-tests/springboot/src/main/java/datadog/smoketest/springboot/controller/IastWebController.java +++ b/dd-smoke-tests/springboot/src/main/java/datadog/smoketest/springboot/controller/IastWebController.java @@ -301,6 +301,12 @@ public void xssWrite(final HttpServletRequest request, final HttpServletResponse } } + @GetMapping(value = "/hstsmissing", produces = "text/html") + public String hstsHeaderMissing(HttpServletResponse response) { + response.setStatus(HttpStatus.OK.value()); + return "ok"; + } + private void withProcess(final Operation op) { Process process = null; try { diff --git a/internal-api/src/main/java/datadog/trace/api/iast/InstrumentationBridge.java b/internal-api/src/main/java/datadog/trace/api/iast/InstrumentationBridge.java index 73b40a06576..b37f7c3780d 100644 --- a/internal-api/src/main/java/datadog/trace/api/iast/InstrumentationBridge.java +++ b/internal-api/src/main/java/datadog/trace/api/iast/InstrumentationBridge.java @@ -4,6 +4,7 @@ import datadog.trace.api.iast.propagation.PropagationModule; import datadog.trace.api.iast.propagation.StringModule; import datadog.trace.api.iast.sink.CommandInjectionModule; +import datadog.trace.api.iast.sink.HstsMissingHeaderModule; import datadog.trace.api.iast.sink.HttpResponseHeaderModule; import datadog.trace.api.iast.sink.InsecureCookieModule; import datadog.trace.api.iast.sink.LdapInjectionModule; @@ -41,6 +42,7 @@ public abstract class InstrumentationBridge { public static volatile UnvalidatedRedirectModule UNVALIDATED_REDIRECT; public static volatile WeakRandomnessModule WEAK_RANDOMNESS; public static volatile HttpResponseHeaderModule RESPONSE_HEADER_MODULE; + public static volatile HstsMissingHeaderModule HSTS_MISSING_HEADER_MODULE; public static volatile TrustBoundaryViolationModule TRUST_BOUNDARY_VIOLATION; public static volatile XPathInjectionModule XPATH_INJECTION; @@ -84,6 +86,8 @@ public static void registerIastModule(final IastModule module) { WEAK_RANDOMNESS = (WeakRandomnessModule) module; } else if (module instanceof HttpResponseHeaderModule) { RESPONSE_HEADER_MODULE = (HttpResponseHeaderModule) module; + } else if (module instanceof HstsMissingHeaderModule) { + HSTS_MISSING_HEADER_MODULE = (HstsMissingHeaderModule) module; } else if (module instanceof XPathInjectionModule) { XPATH_INJECTION = (XPathInjectionModule) module; } else if (module instanceof TrustBoundaryViolationModule) { @@ -151,6 +155,9 @@ public static E getIastModule(final Class type) { if (type == HttpResponseHeaderModule.class) { return (E) RESPONSE_HEADER_MODULE; } + if (type == HstsMissingHeaderModule.class) { + return (E) HSTS_MISSING_HEADER_MODULE; + } if (type == TrustBoundaryViolationModule.class) { return (E) TRUST_BOUNDARY_VIOLATION; } @@ -179,6 +186,7 @@ public static void clearIastModules() { UNVALIDATED_REDIRECT = null; WEAK_RANDOMNESS = null; RESPONSE_HEADER_MODULE = null; + HSTS_MISSING_HEADER_MODULE = null; XPATH_INJECTION = null; TRUST_BOUNDARY_VIOLATION = null; XSS = null; diff --git a/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityTypes.java b/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityTypes.java index 4cb381e6068..703c67be939 100644 --- a/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityTypes.java +++ b/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityTypes.java @@ -13,6 +13,7 @@ private VulnerabilityTypes() {} public static final String SSRF = "SSRF"; public static final String INSECURE_COOKIE = "INSECURE_COOKIE"; public static final String NO_HTTPONLY_COOKIE = "NO_HTTPONLY_COOKIE"; + public static final String HSTS_HEADER_MISSING = "HSTS_HEADER_MISSING"; public static final String NO_SAMESITE_COOKIE = "NO_SAMESITE_COOKIE"; public static final String UNVALIDATED_REDIRECT = "UNVALIDATED_REDIRECT"; public static final String WEAK_RANDOMNESS = "WEAK_RANDOMNESS"; diff --git a/internal-api/src/main/java/datadog/trace/api/iast/sink/HstsMissingHeaderModule.java b/internal-api/src/main/java/datadog/trace/api/iast/sink/HstsMissingHeaderModule.java new file mode 100644 index 00000000000..bf448d61b65 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/iast/sink/HstsMissingHeaderModule.java @@ -0,0 +1,10 @@ +package datadog.trace.api.iast.sink; + +import datadog.trace.api.gateway.IGSpanInfo; +import datadog.trace.api.iast.IastModule; + +public interface HstsMissingHeaderModule extends IastModule { + void onRequestEnd(Object iastRequestContext, IGSpanInfo igSpanInfo); + + void onHstsHeader(String value); +} From 8c080b404616b03796869fc60aa76f2b045a052b Mon Sep 17 00:00:00 2001 From: "Santiago M. Mola" Date: Thu, 10 Aug 2023 11:03:10 +0200 Subject: [PATCH 009/367] Update system-tests to 68da674b83d7a1a1730387a3b27a1d4328392f3c (#5703) --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 96e7adcaf1e..8f214299972 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -23,7 +23,7 @@ instrumentation_modules: &instrumentation_modules "dd-java-agent/instrumentation debugger_modules: &debugger_modules "dd-java-agent/agent-debugger|dd-java-agent/agent-bootstrap|dd-java-agent/agent-builder|internal-api|communication|dd-trace-core" profiling_modules: &profiling_modules "dd-java-agent/agent-profiling" -default_system_tests_commit: &default_system_tests_commit 18afda441d6e92fe3befeddd4dd20ff558ca4f0c +default_system_tests_commit: &default_system_tests_commit 68da674b83d7a1a1730387a3b27a1d4328392f3c parameters: gradle_flags: From a748d30c986526e2acf9708be6830f1f525d93bc Mon Sep 17 00:00:00 2001 From: Javier Santos Date: Thu, 10 Aug 2023 17:22:52 +0200 Subject: [PATCH 010/367] Improved logging using placeholders (#4619) Removing statements that log sensitive information and exceptions since they would be sent over the wire by the telemetry logging API --- .../src/main/java/datadog/trace/bootstrap/Agent.java | 4 ++-- .../src/main/java/datadog/trace/bootstrap/AgentJarIndex.java | 2 +- .../instrumentation/jdbc/JDBCConnectionUrlParser.java | 4 ++-- .../java/com/datadog/debugger/agent/DebuggerTransformer.java | 2 +- .../src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java | 3 +-- .../main/java/com/datadog/profiling/agent/ProfilingAgent.java | 2 +- .../src/main/java/datadog/trace/agent/tooling/AgentCLI.java | 2 +- .../java/datadog/trace/agent/tooling/KnownTypesIndex.java | 2 +- .../datadog/trace/agent/tooling/csi/StringConcatExample.java | 2 +- .../trace/agent/tooling/csi/StringPlusConstantsExample.java | 2 +- .../datadog/trace/agent/tooling/csi/StringPlusExample.java | 2 +- .../agent/tooling/csi/StringPlusLoadWithTagsExample.java | 2 +- .../com/datadog/appsec/config/AppSecConfigServiceImpl.java | 2 +- .../java/datadog/trace/instrumentation/jsp/JSPDecorator.java | 2 +- .../datadog/trace/common/writer/TraceStructureWriter.java | 2 +- .../datadog/trace/common/writer/ddintake/DDEvpProxyApi.java | 2 +- .../src/main/java/datadog/trace/core/CoreTracer.java | 2 +- .../datadog/trace/correlation/CorrelationIdInjectors.java | 2 +- internal-api/src/main/java/datadog/trace/api/Config.java | 3 ++- .../trace/bootstrap/config/provider/ConfigProvider.java | 2 +- .../main/java/datadog/remoteconfig/state/ProductState.java | 2 +- 21 files changed, 24 insertions(+), 24 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index b45f2f655d5..7bdbbf36dba 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -640,7 +640,7 @@ private static synchronized void registerDeadlockDetectionEvent() { private static synchronized void initializeJmxSystemAccessProvider( final ClassLoader classLoader) { if (log.isDebugEnabled()) { - log.debug("Initializing JMX system access provider for " + classLoader.toString()); + log.debug("Initializing JMX system access provider for {}", classLoader); } try { final Class tracerInstallerClass = @@ -1036,7 +1036,7 @@ private static boolean isFeatureEnabled(AgentFeature feature) { } } - /** @see datadog.trace.api.ProductActivationConfig#fromString(String) */ + /** @see datadog.trace.api.ProductActivation#fromString(String) */ private static boolean isAppSecFullyDisabled() { // must be kept in sync with logic from Config! final String featureEnabledSysprop = AgentFeature.APPSEC.systemProp; diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/AgentJarIndex.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/AgentJarIndex.java index 0186fd56bdd..382f8708ac6 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/AgentJarIndex.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/AgentJarIndex.java @@ -81,7 +81,7 @@ public static AgentJarIndex readIndex(JarFile agentJar) { return new AgentJarIndex(prefixes, ClassNameTrie.readFrom(in)); } } catch (Throwable e) { - log.error("Unable to read " + AGENT_INDEX_FILE_NAME, e); + log.error("Unable to read {}", AGENT_INDEX_FILE_NAME, e); return null; } } diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java index 9013329c6d6..6ef0d634f0b 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java @@ -868,7 +868,7 @@ private static void populateStandardProperties( try { builder.port(Integer.parseInt(portNumber)); } catch (final NumberFormatException e) { - ExceptionLogger.LOGGER.debug("Error parsing portnumber property: " + portNumber, e); + ExceptionLogger.LOGGER.debug("Error parsing portnumber property: {}", portNumber, e); } } @@ -877,7 +877,7 @@ private static void populateStandardProperties( try { builder.port(Integer.parseInt(portNumber)); } catch (final NumberFormatException e) { - ExceptionLogger.LOGGER.debug("Error parsing portNumber property: " + portNumber, e); + ExceptionLogger.LOGGER.debug("Error parsing portNumber property: {}", portNumber, e); } } } diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerTransformer.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerTransformer.java index 4419a498f5b..d15f17d7f50 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerTransformer.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerTransformer.java @@ -328,7 +328,7 @@ private byte[] writeClassFile(ClassLoader loader, String classFilePath, ClassNod try { classNode.accept(writer); } catch (Throwable t) { - log.error("Cannot write classfile for class: {}", classFilePath, t); + log.error("Cannot write classfile for class: {} Exception: ", classFilePath, t.getMessage()); } byte[] data = writer.toByteArray(); dumpInstrumentedClassFile(classFilePath, data); diff --git a/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java b/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java index e4b6b171afc..5f905fe5585 100644 --- a/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java +++ b/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java @@ -95,7 +95,6 @@ private static void run(final StatsDClientManager statsDClientManager, final Con // App should be run as daemon otherwise CLI apps would not exit once main method exits. .daemon(true) .embedded(true) - .exitWatcher(new TraceConfigExitWatcher()) .confdDirectory(jmxFetchConfigDir) .yamlFileList(jmxFetchConfigs) .targetDirectInstances(true) @@ -132,7 +131,7 @@ public void run() { if (!appConfig.getExitWatcher().shouldExit()) { try { final int result = app.run(); - log.error("jmx collector exited with result: " + result); + log.error("jmx collector exited with result: {}", result); } catch (final Exception e) { log.error("Exception in jmx collector thread", e); } diff --git a/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ProfilingAgent.java b/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ProfilingAgent.java index 6d4bc6891db..3a0832f5c8e 100644 --- a/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ProfilingAgent.java +++ b/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ProfilingAgent.java @@ -159,7 +159,7 @@ public static synchronized void run(final boolean isStartingFirst, ClassLoader a log.warn(e.getMessage()); log.debug("", e); } catch (final ConfigurationException e) { - log.warn("Failed to initialize profiling agent! " + e.getMessage()); + log.warn("Failed to initialize profiling agent! {}", e.getMessage()); log.debug("Failed to initialize profiling agent!", e); } } diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentCLI.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentCLI.java index 59db1a00ded..85114370c4c 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentCLI.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentCLI.java @@ -128,7 +128,7 @@ private static void recursiveDependencySearch(Consumer invoker, File origi private static void unzipJar(Consumer invoker, File file) throws IOException { try (JarFile jar = new JarFile(file)) { - log.debug("Finding entries in file:" + file.getName()); + log.debug("Finding entries in file: {}", file.getName()); jar.stream() .forEach( diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/KnownTypesIndex.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/KnownTypesIndex.java index 1c6afeb6841..60388260b30 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/KnownTypesIndex.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/KnownTypesIndex.java @@ -70,7 +70,7 @@ public static KnownTypesIndex readIndex() { } return new KnownTypesIndex(multipleIdTable, ClassNameTrie.readFrom(in)); } catch (Throwable e) { - log.error("Problem reading " + KNOWN_TYPES_INDEX_NAME, e); + log.error("Problem reading {}", KNOWN_TYPES_INDEX_NAME, e); } } return buildIndex(); // fallback to runtime generation when testing diff --git a/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/csi/StringConcatExample.java b/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/csi/StringConcatExample.java index 136517b8c08..facd9ce4ab9 100644 --- a/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/csi/StringConcatExample.java +++ b/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/csi/StringConcatExample.java @@ -11,7 +11,7 @@ public class StringConcatExample implements BiFunction { public String apply(final String first, final String second) { LOGGER.debug("Before apply"); final String result = first.concat(second); - LOGGER.debug("After apply " + result); + LOGGER.debug("After apply {}", result); return result; } } diff --git a/dd-java-agent/agent-tooling/src/test/java11/datadog/trace/agent/tooling/csi/StringPlusConstantsExample.java b/dd-java-agent/agent-tooling/src/test/java11/datadog/trace/agent/tooling/csi/StringPlusConstantsExample.java index 23df53b87a7..a71e58609a0 100644 --- a/dd-java-agent/agent-tooling/src/test/java11/datadog/trace/agent/tooling/csi/StringPlusConstantsExample.java +++ b/dd-java-agent/agent-tooling/src/test/java11/datadog/trace/agent/tooling/csi/StringPlusConstantsExample.java @@ -11,7 +11,7 @@ public class StringPlusConstantsExample implements TriFunction injectorClass = Class.forName(className); injectorClass.getConstructor(InternalTracer.class).newInstance(tracer); } catch (ReflectiveOperationException e) { - log.warn("Failed to enable injector " + className + ".", e); + log.warn("Failed to enable injector {}.", className, e); } } diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 2df947b1631..37b7e48faf7 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -799,7 +799,8 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins tmpApiKey = new String(Files.readAllBytes(Paths.get(apiKeyFile)), StandardCharsets.UTF_8).trim(); } catch (final IOException e) { - log.error("Cannot read API key from file {}, skipping", apiKeyFile, e); + log.error( + "Cannot read API key from file {}, skipping. Exception {}", apiKeyFile, e.getMessage()); } } site = configProvider.getString(SITE, DEFAULT_SITE); diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java index 5289483200f..a2072017991 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java @@ -249,7 +249,7 @@ public BitSet getIntegerRange(final String key, final BitSet defaultValue) { try { return value == null ? defaultValue : ConfigConverter.parseIntegerRangeSet(value, key); } catch (final NumberFormatException e) { - log.warn("Invalid configuration for " + key, e); + log.warn("Invalid configuration for {}", key, e); return defaultValue; } } diff --git a/remote-config/src/main/java/datadog/remoteconfig/state/ProductState.java b/remote-config/src/main/java/datadog/remoteconfig/state/ProductState.java index ae26cc31b62..a82db5fa6aa 100644 --- a/remote-config/src/main/java/datadog/remoteconfig/state/ProductState.java +++ b/remote-config/src/main/java/datadog/remoteconfig/state/ProductState.java @@ -80,7 +80,7 @@ public boolean apply( try { callListenerCommit(hinter); } catch (Exception ex) { - log.error("Error committing changes for product" + product, ex); + log.error("Error committing changes for product {}", product, ex); } } From d4ab230109cd45cd2fc8d5d4b548fb70b256c056 Mon Sep 17 00:00:00 2001 From: Brian Devins-Suresh Date: Thu, 10 Aug 2023 15:07:39 -0400 Subject: [PATCH 011/367] Add a workaround for bug exposed by reactor-rabbitmq (#5707) --- .../bytebuddy/matcher/ignored_class_name.trie | 3 + .../rabbitmq-amqp-2.7/build.gradle | 10 +++ .../amqp/RabbitChannelInstrumentation.java | 5 +- .../groovy/ReactorRabbitMQTest.groovy | 68 +++++++++++++++++++ 4 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/reactorTest/groovy/ReactorRabbitMQTest.groovy diff --git a/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie b/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie index ad2b96727af..8e925a155df 100644 --- a/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie +++ b/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie @@ -330,5 +330,8 @@ # -------- SPECIAL CASES -------- +# incomplete wrapper that throws exceptions for all but one method +1 reactor.rabbitmq.ChannelProxy + 3 com.mchange.v2.c3p0.* 4 org.springframework.http.converter.* diff --git a/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/build.gradle b/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/build.gradle index 9c3fc2ee9e0..de8ae2554d9 100644 --- a/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/build.gradle +++ b/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/build.gradle @@ -11,6 +11,9 @@ apply from: "$rootDir/gradle/java.gradle" addTestSuiteForDir('latestDepTest', 'test') +addTestSuiteForDir('latestReactorTest', 'reactorTest') +addTestSuite('reactorTest') + dependencies { compileOnly group: 'com.rabbitmq', name: 'amqp-client', version: '2.7.0' @@ -22,6 +25,9 @@ dependencies { latestDepTestImplementation group: 'com.rabbitmq', name: 'amqp-client', version: '+' latestDepTestImplementation group: 'org.springframework.amqp', name: 'spring-rabbit', version: '2.+' + + reactorTestImplementation group: 'io.projectreactor.rabbitmq', name: 'reactor-rabbitmq', version: '1.0.0.RELEASE' + latestReactorTestImplementation group: 'io.projectreactor.rabbitmq', name: 'reactor-rabbitmq', version: '+' } configurations.testRuntimeOnly { @@ -33,3 +39,7 @@ configurations.testRuntimeOnly { tasks.withType(Test).configureEach { usesService(testcontainersLimit) } + +tasks.named("latestDepTest").configure { + finalizedBy latestReactorTest +} diff --git a/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/RabbitChannelInstrumentation.java b/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/RabbitChannelInstrumentation.java index 33ba116e614..4194816c7e8 100644 --- a/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/RabbitChannelInstrumentation.java +++ b/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/RabbitChannelInstrumentation.java @@ -1,5 +1,6 @@ package datadog.trace.instrumentation.rabbitmq.amqp; +import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.extendsClass; import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.implementsInterface; import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.nameEndsWith; import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; @@ -64,7 +65,9 @@ public String hierarchyMarkerType() { @Override public ElementMatcher hierarchyMatcher() { - return implementsInterface(named(hierarchyMarkerType())); + return implementsInterface(named(hierarchyMarkerType())) + // Class is added to ignores trie, but it's not final so just being safe + .and(not(extendsClass(named("reactor.rabbitmq.ChannelProxy")))); } @Override diff --git a/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/reactorTest/groovy/ReactorRabbitMQTest.groovy b/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/reactorTest/groovy/ReactorRabbitMQTest.groovy new file mode 100644 index 00000000000..99df4e68360 --- /dev/null +++ b/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/reactorTest/groovy/ReactorRabbitMQTest.groovy @@ -0,0 +1,68 @@ +import com.rabbitmq.client.AMQP +import com.rabbitmq.client.Channel +import com.rabbitmq.client.ConnectionFactory +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.utils.PortUtils +import org.testcontainers.containers.RabbitMQContainer +import reactor.core.publisher.Mono +import reactor.core.scheduler.Schedulers +import reactor.rabbitmq.RabbitFlux +import reactor.rabbitmq.Sender +import reactor.rabbitmq.SenderOptions +import spock.lang.Shared + +import java.time.Duration +import java.util.concurrent.TimeUnit + +class ReactorRabbitMQTest extends AgentTestRunner { + @Shared + def rabbitMQContainer + @Shared + def defaultRabbitMQPort = 5672 + @Shared + InetSocketAddress rabbitmqAddress = new InetSocketAddress("127.0.0.1", defaultRabbitMQPort) + + ConnectionFactory factory + + def setup() { + factory = new ConnectionFactory(host: rabbitmqAddress.hostName, port: rabbitmqAddress.port) + } + + def setupSpec() { + rabbitMQContainer = new RabbitMQContainer('rabbitmq:3.9.20-alpine') + .withExposedPorts(defaultRabbitMQPort) + .withStartupTimeout(Duration.ofSeconds(120)) + rabbitMQContainer.start() + rabbitmqAddress = new InetSocketAddress( + rabbitMQContainer.getHost(), + rabbitMQContainer.getMappedPort(defaultRabbitMQPort) + ) + PortUtils.waitForPortToOpen(rabbitmqAddress.hostString, rabbitmqAddress.port, 5, TimeUnit.SECONDS) + } + + def cleanupSpec() { + if (rabbitMQContainer) { + rabbitMQContainer.stop() + } + } + + def "test reactor-rabbit does not cause exceptions from calling channel.asyncCompletableRpc"() { + setup: + SenderOptions senderOptions = new SenderOptions() + .connectionFactory(factory) + .resourceManagementScheduler(Schedulers.elastic()) + Sender sender = RabbitFlux.createSender(senderOptions) + + Mono channelMono = sender.getChannelMonoForResourceManagement(null) + Channel channel1 = channelMono.block() + + expect: + channel1.getClass().getCanonicalName() == "reactor.rabbitmq.ChannelProxy" + + when: + channel1.asyncCompletableRpc(new AMQP.Connection.Close.Builder().build()) + + then: + noExceptionThrown() + } +} From 58fc2e2c1c85c7467b7104cd1d28030c47adb9f5 Mon Sep 17 00:00:00 2001 From: "Santiago M. Mola" Date: Fri, 11 Aug 2023 09:12:43 +0200 Subject: [PATCH 012/367] Test deserialization with unknown keys in AppSec rules (#5690) --- ...pSecConfigDeserializerSpecification.groovy | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigDeserializerSpecification.groovy diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigDeserializerSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigDeserializerSpecification.groovy new file mode 100644 index 00000000000..52775d4acb2 --- /dev/null +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigDeserializerSpecification.groovy @@ -0,0 +1,61 @@ +package com.datadog.appsec.config + +import spock.lang.Specification + +import java.nio.charset.StandardCharsets + +class AppSecConfigDeserializerSpecification extends Specification { + + void "deserialize rule with unknown key"() { + given: + final deser = AppSecConfigDeserializer.INSTANCE + final input = """ + { + "version": "2.9999", + "metadata": { + "rules_version": "1.7.1" + }, + "exclusions": [ + { + "UNKNOWN_FIELD": "UNKNOWN_VALUE" + } + ], + "rules": [ + { + "UNKNOWN_FIELD": "UNKNOWN_VALUE", + "id": "blk-001-001", + "name": "Block IP Addresses", + "tags": { + "type": "block_ip", + "category": "security_response" + }, + "conditions": [ + { + "parameters": { + "inputs": [ + { + "address": "http.client_ip" + } + ], + "data": "blocked_ips" + }, + "operator": "ip_match" + } + ], + "transformers": [], + "on_match": [ + "block" + ] + } + ] + } + """ + + when: + def result = deser.deserialize(input.getBytes(StandardCharsets.UTF_8)) + + then: + result != null + result.getNumberOfRules() == 1 + } +} From 9e16e31acb134f05a153b79e74d426379bdac2e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Fri, 11 Aug 2023 09:34:42 +0200 Subject: [PATCH 013/367] Redact empty sensitive ranges (#5706) --- .../iast/model/json/EvidenceAdapter.java | 4 +-- .../java/com/datadog/iast/util/Ranged.java | 6 +++- .../model/json/EvidenceRedactionTest.groovy | 3 -- .../redaction/evidence-redaction-suite.yml | 34 +++++++++++++++++++ 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/model/json/EvidenceAdapter.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/model/json/EvidenceAdapter.java index a14e47db13d..4dadaa2e75f 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/model/json/EvidenceAdapter.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/model/json/EvidenceAdapter.java @@ -339,7 +339,7 @@ private RedactedValuePart(final String value) { @Override public void write(final Context ctx, final JsonWriter writer) throws IOException { - if (value == null || value.isEmpty()) { + if (value == null) { return; } writer.beginObject(); @@ -470,7 +470,7 @@ private TaintedValuePart( @Override public void write(final Context ctx, final JsonWriter writer) throws IOException { - if (value == null || value.isEmpty()) { + if (value == null) { return; } writer.beginObject(); diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/util/Ranged.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/util/Ranged.java index ed4ef8135f0..20b8057edef 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/util/Ranged.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/util/Ranged.java @@ -88,7 +88,11 @@ default boolean isBefore(final Ranged range) { if (range == null) { return true; } - return getStart() <= range.getStart(); + final int offset = getStart() - range.getStart(); + if (offset == 0) { + return getLength() <= range.getLength(); // put smaller ranges first + } + return offset < 0; } static Ranged build(int start, int end) { diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/model/json/EvidenceRedactionTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/model/json/EvidenceRedactionTest.groovy index ab9ce895d37..255d531f819 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/model/json/EvidenceRedactionTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/model/json/EvidenceRedactionTest.groovy @@ -70,11 +70,8 @@ class EvidenceRedactionTest extends DDSpecification { new StringValuePart(null) | _ new StringValuePart('') | _ new RedactedValuePart(null) | _ - new RedactedValuePart('') | _ new TaintedValuePart(Mock(JsonAdapter), null, null, true) | _ - new TaintedValuePart(Mock(JsonAdapter), null, '', true) | _ new TaintedValuePart(Mock(JsonAdapter), null, null, false) | _ - new TaintedValuePart(Mock(JsonAdapter), null, '', false) | _ } void 'test #suite'() { diff --git a/dd-java-agent/agent-iast/src/test/resources/redaction/evidence-redaction-suite.yml b/dd-java-agent/agent-iast/src/test/resources/redaction/evidence-redaction-suite.yml index 734f02bf153..ebd2e71e054 100644 --- a/dd-java-agent/agent-iast/src/test/resources/redaction/evidence-redaction-suite.yml +++ b/dd-java-agent/agent-iast/src/test/resources/redaction/evidence-redaction-suite.yml @@ -1605,3 +1605,37 @@ suite: } ] } + + - type: 'VULNERABILITIES' + description: 'SQLi exploited' + input: > + [ + { + "type": "SQL_INJECTION", + "evidence": { + "value": "SELECT * FROM Users WHERE email = '' OR TRUE --' AND password = '81dc9bdb52d04dc20036dbd8313ed055' AND deletedAt IS NULL", + "ranges": [ + { "start" : 35, "length" : 12, "source": { "origin": "http.request.parameter", "name": "email", "value": "' OR TRUE --" } } + ] + } + } + ] + expected: > + { + "sources": [ + { "origin": "http.request.parameter", "name": "email", "value": "' OR TRUE --" } + ], + "vulnerabilities": [ + { + "type": "SQL_INJECTION", + "evidence": { + "valueParts": [ + { "value": "SELECT * FROM Users WHERE email = '" }, + { "redacted": true }, + { "source": 0, "value": "' OR TRUE --" }, + { "redacted": true } + ] + } + } + ] + } From cc683eabb98687009f633fbd64cea4d9fdba4819 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Mon, 14 Aug 2023 10:45:04 +0200 Subject: [PATCH 014/367] Set concrete types for the response instrumentation (#5714) --- ...pServletResponseInstrumentationTest.groovy | 27 +++++++++++++++- .../src/test/java/foo/bar/DummyResponse.java | 10 ++++++ ...rtaHttpServletResponseInstrumentation.java | 10 ++++-- ...pServletResponseInstrumentationTest.groovy | 31 +++++++++++++++++-- .../java/foo/bar/smoketest/DummyResponse.java | 10 ++++++ 5 files changed, 82 insertions(+), 6 deletions(-) diff --git a/dd-java-agent/instrumentation/servlet-common/src/test/groovy/HttpServletResponseInstrumentationTest.groovy b/dd-java-agent/instrumentation/servlet-common/src/test/groovy/HttpServletResponseInstrumentationTest.groovy index 87a42a2213d..b53b79b3d91 100644 --- a/dd-java-agent/instrumentation/servlet-common/src/test/groovy/HttpServletResponseInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/servlet-common/src/test/groovy/HttpServletResponseInstrumentationTest.groovy @@ -75,7 +75,7 @@ class HttpServletResponseInstrumentationTest extends AgentTestRunner { final response = new DummyResponse() when: - response.addCookie(null) + response.addCookie((Cookie) null) then: 0 * _ @@ -198,4 +198,29 @@ class HttpServletResponseInstrumentationTest extends AgentTestRunner { 1 * module.taintIfInputIsTainted(_, "http://dummy.url.com") 0 * _ } + + void 'test instrumentation with unknown types'() { + setup: + final module = Mock(HttpResponseHeaderModule) + InstrumentationBridge.registerIastModule(module) + final response = new DummyResponse() + + when: + response.addCookie(new DummyResponse.CustomCookie()) + + then: + 0 * _ + + when: + response.addHeader(new DummyResponse.CustomHeaderName(), "value") + + then: + 0 * _ + + when: + response.setHeader(new DummyResponse.CustomHeaderName(), "value") + + then: + 0 * _ + } } diff --git a/dd-java-agent/instrumentation/servlet-common/src/test/java/foo/bar/DummyResponse.java b/dd-java-agent/instrumentation/servlet-common/src/test/java/foo/bar/DummyResponse.java index 71c04e343f2..adcc8440641 100644 --- a/dd-java-agent/instrumentation/servlet-common/src/test/java/foo/bar/DummyResponse.java +++ b/dd-java-agent/instrumentation/servlet-common/src/test/java/foo/bar/DummyResponse.java @@ -11,6 +11,8 @@ public class DummyResponse implements HttpServletResponse { @Override public void addCookie(Cookie cookie) {} + public void addCookie(CustomCookie cookie) {} + @Override public boolean containsHeader(String name) { return false; @@ -54,9 +56,13 @@ public void addDateHeader(String name, long date) {} @Override public void setHeader(String name, String value) {} + public void setHeader(CustomHeaderName name, String value) {} + @Override public void addHeader(String name, String value) {} + public void addHeader(CustomHeaderName name, String value) {} + @Override public void setIntHeader(String name, int value) {} @@ -119,4 +125,8 @@ public void setLocale(Locale loc) {} public Locale getLocale() { return null; } + + public static class CustomCookie {} + + public static class CustomHeaderName {} } diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaHttpServletResponseInstrumentation.java b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaHttpServletResponseInstrumentation.java index a47b5c9129e..f7c7c3b03ff 100644 --- a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaHttpServletResponseInstrumentation.java +++ b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaHttpServletResponseInstrumentation.java @@ -7,6 +7,7 @@ import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.returns; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; @@ -41,9 +42,14 @@ public ElementMatcher hierarchyMatcher() { @Override public void adviceTransformations(AdviceTransformation transformation) { - transformation.applyAdvice(named("addCookie"), getClass().getName() + "$AddCookieAdvice"); transformation.applyAdvice( - namedOneOf("setHeader", "addHeader"), getClass().getName() + "$AddHeaderAdvice"); + named("addCookie") + .and(takesArguments(1)) + .and(takesArgument(0, named("jakarta.servlet.http.Cookie"))), + getClass().getName() + "$AddCookieAdvice"); + transformation.applyAdvice( + namedOneOf("setHeader", "addHeader").and(takesArguments(String.class, String.class)), + getClass().getName() + "$AddHeaderAdvice"); transformation.applyAdvice( namedOneOf("encodeRedirectURL", "encodeURL") .and(takesArgument(0, String.class)) diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/test/groovy/JakartaHttpServletResponseInstrumentationTest.groovy b/dd-java-agent/instrumentation/servlet/request-5/src/test/groovy/JakartaHttpServletResponseInstrumentationTest.groovy index 8c164826e12..f077b6c0307 100644 --- a/dd-java-agent/instrumentation/servlet/request-5/src/test/groovy/JakartaHttpServletResponseInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/servlet/request-5/src/test/groovy/JakartaHttpServletResponseInstrumentationTest.groovy @@ -82,7 +82,7 @@ class JakartaHttpServletResponseInstrumentationTest extends AgentTestRunner { final response = new DummyResponse() when: - response.addCookie(null) + response.addCookie((Cookie) null) then: 0 * _ @@ -108,7 +108,7 @@ class JakartaHttpServletResponseInstrumentationTest extends AgentTestRunner { final response = new DummyResponse() when: - response.addHeader(null, null) + response.addHeader((String) null, null) then: noExceptionThrown() @@ -135,7 +135,7 @@ class JakartaHttpServletResponseInstrumentationTest extends AgentTestRunner { final response = new DummyResponse() when: - response.setHeader(null, null) + response.setHeader((String) null, null) then: noExceptionThrown() @@ -230,4 +230,29 @@ class JakartaHttpServletResponseInstrumentationTest extends AgentTestRunner { 1 * module.taintIfInputIsTainted(_, "http://dummy.url.com") 0 * _ } + + void 'test instrumentation with unknown types'() { + setup: + final module = Mock(HttpResponseHeaderModule) + InstrumentationBridge.registerIastModule(module) + final response = new DummyResponse() + + when: + response.addCookie(new DummyResponse.CustomCookie()) + + then: + 0 * _ + + when: + response.addHeader(new DummyResponse.CustomHeaderName(), "value") + + then: + 0 * _ + + when: + response.setHeader(new DummyResponse.CustomHeaderName(), "value") + + then: + 0 * _ + } } diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/DummyResponse.java b/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/DummyResponse.java index 8170e771e75..cdd806f3580 100644 --- a/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/DummyResponse.java +++ b/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/DummyResponse.java @@ -12,6 +12,8 @@ public class DummyResponse implements HttpServletResponse { @Override public void addCookie(Cookie cookie) {} + public void addCookie(CustomCookie cookie) {} + @Override public boolean containsHeader(String name) { return false; @@ -55,9 +57,13 @@ public void addDateHeader(String name, long date) {} @Override public void setHeader(String name, String value) {} + public void setHeader(CustomHeaderName name, String value) {} + @Override public void addHeader(String name, String value) {} + public void addHeader(CustomHeaderName name, String value) {} + @Override public void setIntHeader(String name, int value) {} @@ -151,4 +157,8 @@ public void setLocale(Locale loc) {} public Locale getLocale() { return null; } + + public static class CustomCookie {} + + public static class CustomHeaderName {} } From 7dfca99786850b7f206088aa1f7aeee5ddbe4202 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko <121111529+nikita-tkachenko-datadog@users.noreply.github.com> Date: Mon, 14 Aug 2023 14:07:10 +0200 Subject: [PATCH 015/367] Exclude org.mockito package from CI Visibility code coverage by default (#5712) --- .../src/main/java/datadog/trace/api/ConfigDefaults.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java index c1dc8873d30..19dc5170c4d 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java @@ -115,7 +115,7 @@ public final class ConfigDefaults { static final boolean DEFAULT_CIVISIBILITY_COMPILER_PLUGIN_AUTO_CONFIGURATION_ENABLED = true; static final String DEFAULT_CIVISIBILITY_COMPILER_PLUGIN_VERSION = "0.1.6"; static final String DEFAULT_CIVISIBILITY_JACOCO_PLUGIN_EXCLUDES = - "datadog.trace.*:org.apache.commons.*"; + "datadog.trace.*:org.apache.commons.*:org.mockito.*"; static final boolean DEFAULT_CIVISIBILITY_GIT_UPLOAD_ENABLED = true; static final boolean DEFAULT_CIVISIBILITY_GIT_UNSHALLOW_ENABLED = true; static final long DEFAULT_CIVISIBILITY_GIT_COMMAND_TIMEOUT_MILLIS = 30_000; From 1e57538964a8e1ce33956ea3c6eebed00da5b3bd Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Mon, 14 Aug 2023 14:29:56 -0400 Subject: [PATCH 016/367] Apply PR feedback --- .../org/glassfish/jersey/client/WrappingResponseCallback.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/src/main/java/org/glassfish/jersey/client/WrappingResponseCallback.java b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/src/main/java/org/glassfish/jersey/client/WrappingResponseCallback.java index f1df278253a..6cf90a985e3 100644 --- a/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/src/main/java/org/glassfish/jersey/client/WrappingResponseCallback.java +++ b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/src/main/java/org/glassfish/jersey/client/WrappingResponseCallback.java @@ -35,9 +35,7 @@ public static void handleProcessingException(ClientRequest request, ProcessingEx @SuppressWarnings("deprecation") final boolean isJaxRsExceptionAsErrorEnabled = Config.get().isJaxRsExceptionAsErrorEnabled(); - if (!isJaxRsExceptionAsErrorEnabled) { - span.setError(false); - } + span.setError(isJaxRsExceptionAsErrorEnabled); span.finish(); } From db9b2adad006edeaff3566ef4fa16c06b425f318 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Tue, 15 Aug 2023 09:19:06 -0400 Subject: [PATCH 017/367] Update WrappingResponseCallbackTest.groovy --- .../src/test/groovy/WrappingResponseCallbackTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/src/test/groovy/WrappingResponseCallbackTest.groovy b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/src/test/groovy/WrappingResponseCallbackTest.groovy index 04a0989ceec..3decbeb0044 100644 --- a/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/src/test/groovy/WrappingResponseCallbackTest.groovy +++ b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-jersey/src/test/groovy/WrappingResponseCallbackTest.groovy @@ -16,7 +16,7 @@ class WrappingResponseCallbackTest extends AgentTestRunner { injectSysConfig(JAX_RS_EXCEPTION_AS_ERROR_ENABLED, "$jaxRsExceptionAsErrorEnabled") } def testSpan = TEST_TRACER.buildSpan("testInstrumentation", "testSpan").start() - def props = Map.of(ClientTracingFilter.SPAN_PROPERTY_NAME, testSpan) + def props = [(ClientTracingFilter.SPAN_PROPERTY_NAME): testSpan] def propertiesDelegate = new MapPropertiesDelegate(props) def clientConfig = new ClientConfig(new JerseyClient()) From b1b3f37f7c5dc534eb64323121f2ca51c60151d6 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Wed, 9 Aug 2023 11:41:12 +0200 Subject: [PATCH 018/367] add _dd.base_service to disambiguate service map --- .../agent/test/asserts/TagsAssert.groovy | 2 +- .../main/java/datadog/trace/api/DDTags.java | 1 + .../datadog/trace/core/DDSpanContext.java | 13 +----- .../core/tagprocessor/BaseServiceAdder.java | 30 +++++++++++++ .../core/tagprocessor/PostProcessorChain.java | 9 +++- .../core/tagprocessor/TagsPostProcessor.java | 6 +++ .../TagsPostProcessorFactory.java | 44 +++++++++++++++++++ .../tagprocessor/BaseServiceAdderTest.groovy | 28 ++++++++++++ .../core/test/DDCoreSpecification.groovy | 11 +++++ 9 files changed, 131 insertions(+), 13 deletions(-) create mode 100644 dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/BaseServiceAdder.java create mode 100644 dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/TagsPostProcessorFactory.java create mode 100644 dd-trace-core/src/test/groovy/datadog/trace/core/tagprocessor/BaseServiceAdderTest.groovy diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/TagsAssert.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/TagsAssert.groovy index 5048b18096d..c1e9defdaa2 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/TagsAssert.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/TagsAssert.groovy @@ -60,7 +60,7 @@ class TagsAssert { assertedTags.add(DDTags.PID_TAG) assertedTags.add(DDTags.SCHEMA_VERSION_TAG_KEY) assertedTags.add(DDTags.PROFILING_ENABLED) - + assertedTags.add(DDTags.BASE_SERVICE) assert tags["thread.name"] != null assert tags["thread.id"] != null diff --git a/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java b/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java index f9a25b047a8..c2a7397b4fb 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java @@ -52,4 +52,5 @@ public class DDTags { public static final String INTERNAL_GIT_COMMIT_SHA = "_dd.git.commit.sha"; public static final String PROFILING_ENABLED = "_dd.profiling.enabled"; + public static final String BASE_SERVICE = "_dd.base_service"; } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index 0bb3b916f89..613e222b945 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -2,7 +2,6 @@ import static datadog.trace.api.cache.RadixTreeCache.HTTP_STATUSES; -import datadog.trace.api.Config; import datadog.trace.api.DDTags; import datadog.trace.api.DDTraceId; import datadog.trace.api.Functions; @@ -25,10 +24,7 @@ import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import datadog.trace.core.propagation.PropagationTags; import datadog.trace.core.taginterceptor.TagInterceptor; -import datadog.trace.core.tagprocessor.PeerServiceCalculator; -import datadog.trace.core.tagprocessor.PostProcessorChain; -import datadog.trace.core.tagprocessor.QueryObfuscator; -import datadog.trace.core.tagprocessor.TagsPostProcessor; +import datadog.trace.core.tagprocessor.TagsPostProcessorFactory; import datadog.trace.util.TagsHelper; import java.io.Closeable; import java.io.IOException; @@ -84,11 +80,6 @@ public class DDSpanContext private volatile short httpStatusCode; - private static final TagsPostProcessor postProcessor = - new PostProcessorChain( - new PeerServiceCalculator(), - new QueryObfuscator(Config.get().getObfuscationQueryRegexp())); - /** * Tags are associated to the current span, they will not propagate to the children span. * @@ -791,7 +782,7 @@ public void processTagsAndBaggage(final MetadataConsumer consumer, int longRunni new Metadata( threadId, threadName, - postProcessor.processTags(unsafeTags), + TagsPostProcessorFactory.instance().processTagsWithContext(unsafeTags, this), baggageItemsWithPropagationTags, samplingPriority != PrioritySampling.UNSET ? samplingPriority : getSamplingPriority(), measured, diff --git a/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/BaseServiceAdder.java b/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/BaseServiceAdder.java new file mode 100644 index 00000000000..ad07ad81607 --- /dev/null +++ b/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/BaseServiceAdder.java @@ -0,0 +1,30 @@ +package datadog.trace.core.tagprocessor; + +import datadog.trace.api.DDTags; +import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; +import datadog.trace.core.DDSpanContext; +import java.util.Map; +import javax.annotation.Nullable; + +public class BaseServiceAdder implements TagsPostProcessor { + private final UTF8BytesString ddService; + + public BaseServiceAdder(@Nullable final String ddService) { + this.ddService = ddService != null ? UTF8BytesString.create(ddService) : null; + } + + @Override + public Map processTags(Map unsafeTags) { + // we are only able to do something if the span service name is known + return unsafeTags; + } + + @Override + public Map processTagsWithContext( + Map unsafeTags, DDSpanContext spanContext) { + if (ddService != null && !ddService.toString().equals(spanContext.getServiceName())) { + unsafeTags.put(DDTags.BASE_SERVICE, ddService); + } + return unsafeTags; + } +} diff --git a/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/PostProcessorChain.java b/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/PostProcessorChain.java index 0bcee92062a..ffadb336710 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/PostProcessorChain.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/PostProcessorChain.java @@ -1,5 +1,6 @@ package datadog.trace.core.tagprocessor; +import datadog.trace.core.DDSpanContext; import java.util.Map; import java.util.Objects; import javax.annotation.Nonnull; @@ -13,9 +14,15 @@ public PostProcessorChain(@Nonnull final TagsPostProcessor... processors) { @Override public Map processTags(Map unsafeTags) { + return processTagsWithContext(unsafeTags, null); + } + + @Override + public Map processTagsWithContext( + Map unsafeTags, DDSpanContext spanContext) { Map currentTags = unsafeTags; for (final TagsPostProcessor tagsPostProcessor : chain) { - currentTags = tagsPostProcessor.processTags(currentTags); + currentTags = tagsPostProcessor.processTagsWithContext(currentTags, spanContext); } return currentTags; } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/TagsPostProcessor.java b/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/TagsPostProcessor.java index 9dae970348a..55baaa2d4ad 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/TagsPostProcessor.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/TagsPostProcessor.java @@ -1,7 +1,13 @@ package datadog.trace.core.tagprocessor; +import datadog.trace.core.DDSpanContext; import java.util.Map; public interface TagsPostProcessor { Map processTags(Map unsafeTags); + + default Map processTagsWithContext( + Map unsafeTags, DDSpanContext spanContext) { + return processTags(unsafeTags); + } } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/TagsPostProcessorFactory.java b/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/TagsPostProcessorFactory.java new file mode 100644 index 00000000000..74aa7215629 --- /dev/null +++ b/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/TagsPostProcessorFactory.java @@ -0,0 +1,44 @@ +package datadog.trace.core.tagprocessor; + +import datadog.trace.api.Config; +import java.util.ArrayList; +import java.util.List; + +public final class TagsPostProcessorFactory { + private static boolean addBaseService = true; + + private static class Lazy { + private static TagsPostProcessor create() { + final List processors = new ArrayList<>(addBaseService ? 3 : 2); + processors.add(new PeerServiceCalculator()); + if (addBaseService) { + processors.add(new BaseServiceAdder(Config.get().getServiceName())); + } + processors.add(new QueryObfuscator(Config.get().getObfuscationQueryRegexp())); + return new PostProcessorChain( + processors.toArray(processors.toArray(new TagsPostProcessor[0]))); + } + + private static TagsPostProcessor instance = create(); + } + + public static TagsPostProcessor instance() { + return Lazy.instance; + } + + /** + * Mostly used for test purposes. + * + * @param enabled if false, {@link BaseServiceAdder} is not put in the chain. + */ + public static void withAddBaseService(boolean enabled) { + addBaseService = enabled; + Lazy.instance = Lazy.create(); + } + + /** Used for testing purposes. It reset the singleton and restore default options */ + public static void reset() { + withAddBaseService(true); + Lazy.instance = Lazy.create(); + } +} diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/tagprocessor/BaseServiceAdderTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/tagprocessor/BaseServiceAdderTest.groovy new file mode 100644 index 00000000000..177f5c4a612 --- /dev/null +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/tagprocessor/BaseServiceAdderTest.groovy @@ -0,0 +1,28 @@ +package datadog.trace.core.tagprocessor + + +import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString +import datadog.trace.core.DDSpanContext +import datadog.trace.test.util.DDSpecification + +class BaseServiceAdderTest extends DDSpecification { + def "should add _dd.base_service when service differs to ddService"() { + setup: + def calculator = new BaseServiceAdder("test") + def spanContext = Mock(DDSpanContext) + + when: + def enrichedTags = calculator.processTagsWithContext([:], spanContext) + + then: + 1 * spanContext.getServiceName() >> serviceName + + and: + assert enrichedTags == expectedTags + + where: + serviceName | expectedTags + "anotherOne" | ["_dd.base_service": UTF8BytesString.create("test")] + "test" | [:] + } +} diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/test/DDCoreSpecification.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/test/DDCoreSpecification.groovy index ddcc1207a73..a9b48d5454f 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/test/DDCoreSpecification.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/test/DDCoreSpecification.groovy @@ -12,6 +12,7 @@ import datadog.trace.core.CoreTracer.CoreTracerBuilder import datadog.trace.core.DDSpan import datadog.trace.core.DDSpanContext import datadog.trace.core.propagation.PropagationTags +import datadog.trace.core.tagprocessor.TagsPostProcessorFactory import datadog.trace.test.util.DDSpecification abstract class DDCoreSpecification extends DDSpecification { @@ -24,6 +25,16 @@ abstract class DDCoreSpecification extends DDSpecification { return true } + @Override + void setupSpec() { + TagsPostProcessorFactory.withAddBaseService(false) + } + + @Override + void cleanupSpec() { + TagsPostProcessorFactory.reset() + } + protected CoreTracerBuilder tracerBuilder() { def builder = CoreTracer.builder() if (useNoopStatsDClient()) { From 51cca993556c987b7143ad8b035603905ef6a1e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Gonz=C3=A1lez=20Garc=C3=ADa?= Date: Wed, 16 Aug 2023 13:18:57 +0200 Subject: [PATCH 019/367] Add more org.owasp.esapi.Encoder escape functions tainting support (#5624) Add taint tracking support to: org.owasp.esapi.Encoder#canonicalize(String) -> mark for XSS org.owasp.esapi.Encoder#canonicalize(String, boolean) -> mark for XSS org.owasp.esapi.Encoder#canonicalize(String, boolean, boolean) -> mark for XSS org.owasp.esapi.Encoder#encodeForLDAP -> mark for LDAP_INJECTION org.owasp.esapi.Encoder#encodeForOS -> mark for COMMAND_INJECTION org.owasp.esapi.Encoder#encodeForSQL -> mark for SQL_INJECTION --- .../owasp/esapi/EncoderCallSite.java | 108 ++++++++++++++++++ .../owasp/esapi/EncoderCallSiteTest.groovy | 15 ++- .../test/java/foo/bar/TestEncoderSuite.java | 25 ++++ 3 files changed, 144 insertions(+), 4 deletions(-) diff --git a/dd-java-agent/instrumentation/owasp-esapi-2/src/main/java/datadog/trace/instrumentation/owasp/esapi/EncoderCallSite.java b/dd-java-agent/instrumentation/owasp-esapi-2/src/main/java/datadog/trace/instrumentation/owasp/esapi/EncoderCallSite.java index 9b184735eca..3c9fef4c79c 100644 --- a/dd-java-agent/instrumentation/owasp-esapi-2/src/main/java/datadog/trace/instrumentation/owasp/esapi/EncoderCallSite.java +++ b/dd-java-agent/instrumentation/owasp-esapi-2/src/main/java/datadog/trace/instrumentation/owasp/esapi/EncoderCallSite.java @@ -8,6 +8,7 @@ import datadog.trace.api.iast.propagation.PropagationModule; import javax.annotation.Nonnull; import org.owasp.esapi.Encoder; +import org.owasp.esapi.codecs.Codec; @Propagation @CallSite(spi = IastCallSites.class) @@ -28,4 +29,111 @@ public static String afterEncodeForHTML( } return result; } + + @CallSite.After("java.lang.String org.owasp.esapi.Encoder.canonicalize(java.lang.String)") + public static String afterCanonicalize1( + @CallSite.This final Encoder encoder, + @CallSite.Argument(0) @Nonnull final String input, + @CallSite.Return final String result) { + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module != null) { + try { + module.taintIfInputIsTaintedWithMarks(result, input, VulnerabilityMarks.XSS_MARK); + } catch (final Throwable e) { + module.onUnexpectedException("afterCanonicalize1 threw", e); + } + } + return result; + } + + @CallSite.After( + "java.lang.String org.owasp.esapi.Encoder.canonicalize(java.lang.String, boolean)") + public static String afterCanonicalize2( + @CallSite.This final Encoder encoder, + @CallSite.Argument(0) @Nonnull final String input, + @CallSite.Argument(1) final boolean strict, + @CallSite.Return final String result) { + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module != null) { + try { + module.taintIfInputIsTaintedWithMarks(result, input, VulnerabilityMarks.XSS_MARK); + } catch (final Throwable e) { + module.onUnexpectedException("afterCanonicalize2 threw", e); + } + } + return result; + } + + @CallSite.After( + "java.lang.String org.owasp.esapi.Encoder.canonicalize(java.lang.String, boolean, boolean)") + public static String afterCanonicalize3( + @CallSite.This final Encoder encoder, + @CallSite.Argument(0) @Nonnull final String input, + @CallSite.Argument(1) final boolean strict, + @CallSite.Argument(2) final boolean restrictMixed, + @CallSite.Return final String result) { + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module != null) { + try { + module.taintIfInputIsTaintedWithMarks(result, input, VulnerabilityMarks.XSS_MARK); + } catch (final Throwable e) { + module.onUnexpectedException("afterCanonicalize3 threw", e); + } + } + return result; + } + + @CallSite.After("java.lang.String org.owasp.esapi.Encoder.encodeForLDAP(java.lang.String)") + public static String afterEncodeForLDAP( + @CallSite.This final Encoder encoder, + @CallSite.Argument(0) @Nonnull final String input, + @CallSite.Return final String result) { + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module != null) { + try { + module.taintIfInputIsTaintedWithMarks( + result, input, VulnerabilityMarks.LDAP_INJECTION_MARK); + } catch (final Throwable e) { + module.onUnexpectedException("afterEncodeForLDAP threw", e); + } + } + return result; + } + + @CallSite.After( + "java.lang.String org.owasp.esapi.Encoder.encodeForOS(org.owasp.esapi.codecs.Codec, java.lang.String)") + public static String afterEncodeForOS( + @CallSite.This final Encoder encoder, + @CallSite.Argument(0) @Nonnull final Codec codec, + @CallSite.Argument(1) @Nonnull final String input, + @CallSite.Return final String result) { + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module != null) { + try { + module.taintIfInputIsTaintedWithMarks( + result, input, VulnerabilityMarks.COMMAND_INJECTION_MARK); + } catch (final Throwable e) { + module.onUnexpectedException("afterEncodeForOS threw", e); + } + } + return result; + } + + @CallSite.After( + "java.lang.String org.owasp.esapi.Encoder.encodeForSQL(org.owasp.esapi.codecs.Codec, java.lang.String)") + public static String afterEncodeForSQL( + @CallSite.This final Encoder encoder, + @CallSite.Argument(0) @Nonnull final Codec codec, + @CallSite.Argument(1) @Nonnull final String input, + @CallSite.Return final String result) { + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module != null) { + try { + module.taintIfInputIsTaintedWithMarks(result, input, VulnerabilityMarks.SQL_INJECTION_MARK); + } catch (final Throwable e) { + module.onUnexpectedException("afterEncodeForSQL threw", e); + } + } + return result; + } } diff --git a/dd-java-agent/instrumentation/owasp-esapi-2/src/test/groovy/datadog/trace/instrumentation/owasp/esapi/EncoderCallSiteTest.groovy b/dd-java-agent/instrumentation/owasp-esapi-2/src/test/groovy/datadog/trace/instrumentation/owasp/esapi/EncoderCallSiteTest.groovy index 5abc9f5bad4..6b9e267e0ed 100644 --- a/dd-java-agent/instrumentation/owasp-esapi-2/src/test/groovy/datadog/trace/instrumentation/owasp/esapi/EncoderCallSiteTest.groovy +++ b/dd-java-agent/instrumentation/owasp-esapi-2/src/test/groovy/datadog/trace/instrumentation/owasp/esapi/EncoderCallSiteTest.groovy @@ -6,6 +6,7 @@ import datadog.trace.api.iast.VulnerabilityMarks import datadog.trace.api.iast.propagation.PropagationModule import foo.bar.TestEncoderSuite import org.owasp.esapi.Encoder +import org.owasp.esapi.codecs.Codec class EncoderCallSiteTest extends AgentTestRunner { @@ -14,7 +15,7 @@ class EncoderCallSiteTest extends AgentTestRunner { injectSysConfig("dd.iast.enabled", "true") } - void 'test #method'() { + void 'test #method propagation with mark #mark'() { given: final module = Mock(PropagationModule) final testSuite = new TestEncoderSuite(Mock(Encoder)) @@ -24,11 +25,17 @@ class EncoderCallSiteTest extends AgentTestRunner { testSuite.&"$method".call(args) then: - 1 * module.taintIfInputIsTaintedWithMarks(_, args[0], VulnerabilityMarks.XSS_MARK) + 1 * module.taintIfInputIsTaintedWithMarks(_, _, mark) 0 * module._ where: - method | args - 'encodeForHTML' | ['Ø-This is a quote'] + method | args | mark + 'encodeForHTML' | ['Ø-This is a quote'] | VulnerabilityMarks.XSS_MARK + 'canonicalize' | ['Ø-This is a quote'] | VulnerabilityMarks.XSS_MARK + 'canonicalize' | ['Ø-This is a quote', true] | VulnerabilityMarks.XSS_MARK + 'canonicalize' | ['Ø-This is a quote', true, true] | VulnerabilityMarks.XSS_MARK + 'encodeForLDAP' | ['Ø-This is a quote'] | VulnerabilityMarks.LDAP_INJECTION_MARK + 'encodeForOS' | [Mock(Codec), 'Ø-This is a quote'] | VulnerabilityMarks.COMMAND_INJECTION_MARK + 'encodeForSQL' | [Mock(Codec), 'Ø-This is a quote'] | VulnerabilityMarks.SQL_INJECTION_MARK } } diff --git a/dd-java-agent/instrumentation/owasp-esapi-2/src/test/java/foo/bar/TestEncoderSuite.java b/dd-java-agent/instrumentation/owasp-esapi-2/src/test/java/foo/bar/TestEncoderSuite.java index 213f45db540..21f91858fc0 100644 --- a/dd-java-agent/instrumentation/owasp-esapi-2/src/test/java/foo/bar/TestEncoderSuite.java +++ b/dd-java-agent/instrumentation/owasp-esapi-2/src/test/java/foo/bar/TestEncoderSuite.java @@ -1,6 +1,7 @@ package foo.bar; import org.owasp.esapi.Encoder; +import org.owasp.esapi.codecs.Codec; public class TestEncoderSuite { @@ -13,4 +14,28 @@ public class TestEncoderSuite { public String encodeForHTML(String input) { return encoder.encodeForHTML(input); } + + public String canonicalize(String input) { + return encoder.canonicalize(input); + } + + public String canonicalize(String input, boolean strict) { + return encoder.canonicalize(input, strict); + } + + public String canonicalize(String input, boolean strict, boolean restrictMixed) { + return encoder.canonicalize(input, strict, restrictMixed); + } + + public String encodeForLDAP(String input) { + return encoder.encodeForLDAP(input); + } + + public String encodeForOS(Codec codec, String input) { + return encoder.encodeForOS(codec, input); + } + + public String encodeForSQL(Codec codec, String input) { + return encoder.encodeForSQL(codec, input); + } } From 09bdc634ba6a091799e54558a9325dd8182c096d Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Wed, 16 Aug 2023 16:57:39 +0200 Subject: [PATCH 020/367] Support java.util.Timer once scheduling (#5708) * Support java.util.Timer once scheduling * review --- .../bytebuddy/matcher/ignored_class_name.trie | 1 + .../concurrent/JavaTimerInstrumentation.java | 74 ++++++++++++ .../concurrent/TimerTaskInstrumentation.java | 62 ++++++++++ .../groovy/TimerTaskContinuationTest.groovy | 112 ++++++++++++++++++ 4 files changed, 249 insertions(+) create mode 100644 dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/JavaTimerInstrumentation.java create mode 100644 dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/TimerTaskInstrumentation.java create mode 100644 dd-java-agent/instrumentation/java-concurrent/src/test/groovy/TimerTaskContinuationTest.groovy diff --git a/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie b/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie index 8e925a155df..e06d9e64c11 100644 --- a/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie +++ b/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie @@ -52,6 +52,7 @@ 0 java.nio.DirectByteBuffer 0 java.nio.ByteBuffer 0 java.rmi.* +0 java.util.Timer 0 java.util.concurrent.* 1 java.util.concurrent.ConcurrentHashMap* 1 java.util.concurrent.atomic.* diff --git a/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/JavaTimerInstrumentation.java b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/JavaTimerInstrumentation.java new file mode 100644 index 00000000000..bb1a943c964 --- /dev/null +++ b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/JavaTimerInstrumentation.java @@ -0,0 +1,74 @@ +package datadog.trace.instrumentation.java.concurrent; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static datadog.trace.bootstrap.instrumentation.java.concurrent.AdviceUtils.cancelTask; +import static datadog.trace.bootstrap.instrumentation.java.concurrent.AdviceUtils.capture; +import static datadog.trace.bootstrap.instrumentation.java.concurrent.QueueTimerHelper.startQueuingTimer; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isPrivate; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.bootstrap.ContextStore; +import datadog.trace.bootstrap.InstrumentationContext; +import datadog.trace.bootstrap.instrumentation.java.concurrent.State; +import java.util.Map; +import java.util.Timer; +import java.util.TimerTask; +import net.bytebuddy.asm.Advice; + +@AutoService(Instrumenter.class) +public class JavaTimerInstrumentation extends Instrumenter.Tracing + implements Instrumenter.ForBootstrap, Instrumenter.ForSingleType { + public JavaTimerInstrumentation() { + super("java_timer", "java_concurrent", "runnable"); + } + + @Override + public String instrumentedType() { + return "java.util.Timer"; + } + + @Override + public Map contextStore() { + return singletonMap("java.lang.Runnable", State.class.getName()); + } + + @Override + public void adviceTransformations(AdviceTransformation transformation) { + transformation.applyAdvice( + isMethod() + .and(isPrivate()) + .and( + named("sched") + .and(takesArguments(3)) + .and(takesArgument(0, named("java.util.TimerTask"))) + .and(takesArgument(1, long.class)) + .and(takesArgument(2, long.class))), + getClass().getName() + "$TimerScheduleAdvice"); + } + + public static final class TimerScheduleAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void before(@Advice.Argument(0) TimerTask task, @Advice.Argument(2) long period) { + // don't propagate fixed time / rate executions + if (period != 0) { + return; + } + ContextStore contextStore = + InstrumentationContext.get(Runnable.class, State.class); + capture(contextStore, task, true); + startQueuingTimer(contextStore, Timer.class, task); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class) + public static void after(@Advice.Argument(0) TimerTask task, @Advice.Thrown Throwable thrown) { + if (null != thrown) { + cancelTask(InstrumentationContext.get(Runnable.class, State.class), task); + } + } + } +} diff --git a/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/TimerTaskInstrumentation.java b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/TimerTaskInstrumentation.java new file mode 100644 index 00000000000..8f315f48bfa --- /dev/null +++ b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/TimerTaskInstrumentation.java @@ -0,0 +1,62 @@ +package datadog.trace.instrumentation.java.concurrent; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.extendsClass; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.bootstrap.InstrumentationContext; +import datadog.trace.bootstrap.instrumentation.java.concurrent.AdviceUtils; +import datadog.trace.bootstrap.instrumentation.java.concurrent.State; +import java.util.Map; +import java.util.TimerTask; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +/** + * Instrument {@link java.util.TimerTask} + * + *

Only the cancel part is handled here because the execution is handled by the {@link + * RunnableInstrumentation} + */ +@AutoService(Instrumenter.class) +public final class TimerTaskInstrumentation extends Instrumenter.Tracing + implements Instrumenter.ForBootstrap, Instrumenter.ForTypeHierarchy { + + public TimerTaskInstrumentation() { + super("java_timer", AbstractExecutorInstrumentation.EXEC_NAME, "runnable"); + } + + @Override + public String hierarchyMarkerType() { + return null; // bootstrap type + } + + @Override + public ElementMatcher hierarchyMatcher() { + return extendsClass(named(TimerTask.class.getName())); + } + + @Override + public Map contextStore() { + return singletonMap(Runnable.class.getName(), State.class.getName()); + } + + @Override + public void adviceTransformations(AdviceTransformation transformation) { + transformation.applyAdvice( + named("cancel").and(takesArguments(0)).and(isPublic()), + TimerTaskInstrumentation.class.getName() + "$CancelAdvice"); + } + + public static class CancelAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onCancel(@Advice.This TimerTask self) { + AdviceUtils.cancelTask(InstrumentationContext.get(Runnable.class, State.class), self); + } + } +} diff --git a/dd-java-agent/instrumentation/java-concurrent/src/test/groovy/TimerTaskContinuationTest.groovy b/dd-java-agent/instrumentation/java-concurrent/src/test/groovy/TimerTaskContinuationTest.groovy new file mode 100644 index 00000000000..77f0e052947 --- /dev/null +++ b/dd-java-agent/instrumentation/java-concurrent/src/test/groovy/TimerTaskContinuationTest.groovy @@ -0,0 +1,112 @@ +import static datadog.trace.agent.test.utils.TraceUtils.basicSpan +import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan + +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.bootstrap.instrumentation.api.AgentScope +import datadog.trace.bootstrap.instrumentation.api.AgentSpan +import spock.lang.Shared + +import java.time.Instant +import java.util.concurrent.CountDownLatch + +class TimerTaskContinuationTest extends AgentTestRunner { + @Shared + Timer timer = new Timer() + + TimerTask timerTask + + CountDownLatch runLatch = new CountDownLatch(1) + + @Override + boolean useStrictTraceWrites() { + false + } + + @Override + def setup() { + timerTask = new TimerTask() { + @Override + void run() { + AgentSpan span = startSpan("test", "child") + AgentScope scope = activateSpan(span) + try { + span.finish() + } finally { + scope.close() + runLatch.countDown() + } + } + } + } + + def "test continuation activated when TimerTask runs (long)"() { + when: + runUnderTrace("parent", { + timer.schedule(timerTask, 100) + }) + runLatch.await() + + + then: + assertTraces(1) { + sortSpansByStart() + trace(2) { + basicSpan(it, "parent") + basicSpan(it, "child", trace(0).get(0)) + } + } + } + + def "test continuation activated when TimerTask runs (date)"() { + when: + runUnderTrace("parent", { + timer.schedule(timerTask, Instant.now().plusMillis(100).toDate()) + }) + runLatch.await() + + then: + assertTraces(1) { + sortSpansByStart() + trace(2) { + basicSpan(it, "parent") + basicSpan(it, "child", trace(0).get(0)) + } + } + } + + def "test continuation canceled when TimerTask is canceled"() { + when: + runUnderTrace("parent", { + timer.schedule(timerTask, 1000) + }) + + then: + assert timerTask.cancel() + assertTraces(1) { + trace(1) { + basicSpan(it, "parent") + } + } + } + + def "test not propagated when scheduled more than once"() { + when: + runUnderTrace("parent", { + timer.schedule(timerTask, 100, 1000) + }) + runLatch.await() + timerTask.cancel() + + then: + assertTraces(2) { + trace(1) { + basicSpan(it, "parent") + } + trace(1) { + basicSpan(it, "child") + } + } + } +} From 88fdf40003e014f3e98ae75a42f410428d6562f2 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Wed, 16 Aug 2023 19:20:45 +0200 Subject: [PATCH 021/367] Ensure OpenTelemetry spans are not modifiable when finished (#5722) --- .../opentelemetry14/OtelSpan.java | 24 +++++++++------ .../test/groovy/OpenTelemetry14Test.groovy | 29 +++++++++++++++++++ 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/main/java/datadog/trace/instrumentation/opentelemetry14/OtelSpan.java b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/main/java/datadog/trace/instrumentation/opentelemetry14/OtelSpan.java index 03e72208a90..c10119a051f 100644 --- a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/main/java/datadog/trace/instrumentation/opentelemetry14/OtelSpan.java +++ b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/main/java/datadog/trace/instrumentation/opentelemetry14/OtelSpan.java @@ -39,7 +39,9 @@ public static Span invalid() { @Override public Span setAttribute(AttributeKey key, T value) { - this.delegate.setTag(key.getKey(), value); + if (this.recording) { + this.delegate.setTag(key.getKey(), value); + } return this; } @@ -57,13 +59,15 @@ public Span addEvent(String name, Attributes attributes, long timestamp, TimeUni @Override public Span setStatus(StatusCode statusCode, String description) { - if (this.statusCode == UNSET) { - this.statusCode = statusCode; - this.delegate.setError(statusCode == ERROR); - this.delegate.setErrorMessage(statusCode == ERROR ? description : null); - } else if (this.statusCode == ERROR && statusCode == OK) { - this.delegate.setError(false); - this.delegate.setErrorMessage(null); + if (this.recording) { + if (this.statusCode == UNSET) { + this.statusCode = statusCode; + this.delegate.setError(statusCode == ERROR); + this.delegate.setErrorMessage(statusCode == ERROR ? description : null); + } else if (this.statusCode == ERROR && statusCode == OK) { + this.delegate.setError(false); + this.delegate.setErrorMessage(null); + } } return this; } @@ -76,7 +80,9 @@ public Span recordException(Throwable exception, Attributes additionalAttributes @Override public Span updateName(String name) { - this.delegate.setOperationName(name); + if (this.recording) { + this.delegate.setOperationName(name); + } return this; } diff --git a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/OpenTelemetry14Test.groovy b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/OpenTelemetry14Test.groovy index c3664099d53..a7063857638 100644 --- a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/OpenTelemetry14Test.groovy +++ b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/OpenTelemetry14Test.groovy @@ -341,4 +341,33 @@ class OpenTelemetry14Test extends AgentTestRunner { } } } + + def "test span update after end"() { + setup: + def builder = tracer.spanBuilder("some-name") + def result = builder.startSpan() + + when: + result.setAttribute("string", "value") + result.setStatus(ERROR) + result.end() + result.updateName("other-name") + result.setAttribute("string", "other-value") + result.setStatus(OK) + + then: + assertTraces(1) { + trace(1) { + span { + parent() + operationName "some-name" + errored true + tags { + defaultTags() + "string" "value" + } + } + } + } + } } From b2d110c311d25133c868fa3e58c0b6feed244cb0 Mon Sep 17 00:00:00 2001 From: Gustavo Lopes Date: Wed, 26 Jul 2023 11:16:45 +0100 Subject: [PATCH 022/367] Netty: block on response --- .../netty38/ChannelPipelineAdviceUtil.java | 5 + .../netty38/ChannelTraceContext.java | 38 +++++ .../NettyChannelPipelineInstrumentation.java | 3 +- .../HttpServerRequestTracingHandler.java | 6 +- .../HttpServerResponseTracingHandler.java | 3 +- .../server/MaybeBlockResponseHandler.java | 123 ++++++++++++++++ .../server/NettyHttpServerDecorator.java | 5 + .../netty38/Netty38ServerTest.groovy | 7 +- .../netty40/AttributeKeys.java | 10 ++ .../NettyChannelPipelineInstrumentation.java | 14 +- .../HttpServerRequestTracingHandler.java | 15 +- .../server/MaybeBlockResponseHandler.java | 139 ++++++++++++++++++ .../server/NettyHttpServerDecorator.java | 5 + .../src/test/groovy/Netty40ServerTest.groovy | 39 ++--- .../netty41/AttributeKeys.java | 10 ++ .../NettyChannelPipelineInstrumentation.java | 14 +- .../HttpServerRequestTracingHandler.java | 15 +- .../server/MaybeBlockResponseHandler.java | 136 +++++++++++++++++ .../server/NettyHttpServerDecorator.java | 5 + .../src/test/groovy/Netty41ServerTest.groovy | 5 + .../server/RatpackHttpServerTest.groovy | 5 + .../server/VertxHttpServerForkedTest.groovy | 5 + .../server/VertxHttpServerForkedTest.groovy | 5 + 23 files changed, 581 insertions(+), 31 deletions(-) create mode 100644 dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/server/MaybeBlockResponseHandler.java create mode 100644 dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/MaybeBlockResponseHandler.java create mode 100644 dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/MaybeBlockResponseHandler.java diff --git a/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/ChannelPipelineAdviceUtil.java b/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/ChannelPipelineAdviceUtil.java index 28c16c3c90f..bcfb4349fb0 100644 --- a/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/ChannelPipelineAdviceUtil.java +++ b/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/ChannelPipelineAdviceUtil.java @@ -8,6 +8,7 @@ import datadog.trace.instrumentation.netty38.server.HttpServerRequestTracingHandler; import datadog.trace.instrumentation.netty38.server.HttpServerResponseTracingHandler; import datadog.trace.instrumentation.netty38.server.HttpServerTracingHandler; +import datadog.trace.instrumentation.netty38.server.MaybeBlockResponseHandler; import org.jboss.netty.channel.Channel; import org.jboss.netty.channel.ChannelHandler; import org.jboss.netty.channel.ChannelPipeline; @@ -33,6 +34,8 @@ public static void wrapHandler( if (handler instanceof HttpServerCodec) { pipeline.addLast( HttpServerTracingHandler.class.getName(), new HttpServerTracingHandler(contextStore)); + pipeline.addLast( + MaybeBlockResponseHandler.class.getName(), new MaybeBlockResponseHandler(contextStore)); } else if (handler instanceof HttpRequestDecoder) { pipeline.addLast( HttpServerRequestTracingHandler.class.getName(), @@ -41,6 +44,8 @@ public static void wrapHandler( pipeline.addLast( HttpServerResponseTracingHandler.class.getName(), new HttpServerResponseTracingHandler(contextStore)); + pipeline.addLast( + MaybeBlockResponseHandler.class.getName(), new MaybeBlockResponseHandler(contextStore)); } else // Client pipeline handlers if (handler instanceof HttpClientCodec) { diff --git a/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/ChannelTraceContext.java b/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/ChannelTraceContext.java index b0fbe257517..51b277f958d 100644 --- a/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/ChannelTraceContext.java +++ b/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/ChannelTraceContext.java @@ -3,6 +3,7 @@ import datadog.trace.bootstrap.ContextStore; import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import org.jboss.netty.handler.codec.http.HttpHeaders; public class ChannelTraceContext { public static class Factory implements ContextStore.Factory { @@ -18,6 +19,43 @@ public ChannelTraceContext create() { AgentSpan serverSpan; AgentSpan clientSpan; AgentSpan clientParentSpan; + HttpHeaders requestHeaders; + boolean analyzedResponse; + boolean blockedResponse; + + public void reset() { + this.connectionContinuation = null; + this.serverSpan = null; + this.clientSpan = null; + this.clientParentSpan = null; + this.requestHeaders = null; + this.analyzedResponse = false; + this.blockedResponse = false; + } + + public void setRequestHeaders(HttpHeaders headers) { + this.requestHeaders = headers; + } + + public HttpHeaders getRequestHeaders() { + return requestHeaders; + } + + public boolean isAnalyzedResponse() { + return analyzedResponse; + } + + public void setAnalyzedResponse(boolean analyzedResponse) { + this.analyzedResponse = analyzedResponse; + } + + public boolean isBlockedResponse() { + return blockedResponse; + } + + public void setBlockedResponse(boolean blockedResponse) { + this.blockedResponse = blockedResponse; + } public AgentScope.Continuation getConnectionContinuation() { return connectionContinuation; diff --git a/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/NettyChannelPipelineInstrumentation.java b/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/NettyChannelPipelineInstrumentation.java index fe8875acb03..b862eed82ff 100644 --- a/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/NettyChannelPipelineInstrumentation.java +++ b/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/NettyChannelPipelineInstrumentation.java @@ -64,7 +64,8 @@ public String[] helperClassNames() { packageName + ".server.BlockingResponseHandler", packageName + ".server.HttpServerRequestTracingHandler", packageName + ".server.HttpServerResponseTracingHandler", - packageName + ".server.HttpServerTracingHandler" + packageName + ".server.HttpServerTracingHandler", + packageName + ".server.MaybeBlockResponseHandler", }; } diff --git a/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/server/HttpServerRequestTracingHandler.java b/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/server/HttpServerRequestTracingHandler.java index fdce46eb8db..ab6f168339d 100644 --- a/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/server/HttpServerRequestTracingHandler.java +++ b/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/server/HttpServerRequestTracingHandler.java @@ -26,8 +26,7 @@ public HttpServerRequestTracingHandler( } @Override - public void messageReceived(final ChannelHandlerContext ctx, final MessageEvent msg) - throws Exception { + public void messageReceived(final ChannelHandlerContext ctx, final MessageEvent msg) { final ChannelTraceContext channelTraceContext = contextStore.putIfAbsent(ctx.getChannel(), ChannelTraceContext.Factory.INSTANCE); @@ -49,6 +48,9 @@ public void messageReceived(final ChannelHandlerContext ctx, final MessageEvent final Context.Extracted context = DECORATE.extract(headers); final AgentSpan span = DECORATE.startSpan(headers, context); + channelTraceContext.reset(); + channelTraceContext.setRequestHeaders(headers); + try (final AgentScope scope = activateSpan(span)) { DECORATE.afterStart(span); DECORATE.onRequest(span, ctx.getChannel(), request, context); diff --git a/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/server/HttpServerResponseTracingHandler.java b/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/server/HttpServerResponseTracingHandler.java index 811d73b5f30..78762a88104 100644 --- a/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/server/HttpServerResponseTracingHandler.java +++ b/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/server/HttpServerResponseTracingHandler.java @@ -24,8 +24,7 @@ public HttpServerResponseTracingHandler( } @Override - public void writeRequested(final ChannelHandlerContext ctx, final MessageEvent msg) - throws Exception { + public void writeRequested(final ChannelHandlerContext ctx, final MessageEvent msg) { final ChannelTraceContext channelTraceContext = contextStore.putIfAbsent(ctx.getChannel(), ChannelTraceContext.Factory.INSTANCE); diff --git a/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/server/MaybeBlockResponseHandler.java b/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/server/MaybeBlockResponseHandler.java new file mode 100644 index 00000000000..7511cff8c56 --- /dev/null +++ b/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/server/MaybeBlockResponseHandler.java @@ -0,0 +1,123 @@ +package datadog.trace.instrumentation.netty38.server; + +import static datadog.trace.instrumentation.netty38.server.NettyHttpServerDecorator.DECORATE; +import static datadog.trace.instrumentation.netty38.server.NettyHttpServerDecorator.NettyBlockResponseFunction.log; +import static org.jboss.netty.handler.codec.http.HttpHeaders.setContentLength; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.bootstrap.ContextStore; +import datadog.trace.bootstrap.blocking.BlockingActionHelper; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.instrumentation.netty38.ChannelTraceContext; +import java.util.Map; +import org.jboss.netty.buffer.ChannelBuffer; +import org.jboss.netty.buffer.ChannelBuffers; +import org.jboss.netty.channel.Channel; +import org.jboss.netty.channel.ChannelFuture; +import org.jboss.netty.channel.ChannelHandlerContext; +import org.jboss.netty.channel.Channels; +import org.jboss.netty.channel.MessageEvent; +import org.jboss.netty.channel.SimpleChannelDownstreamHandler; +import org.jboss.netty.handler.codec.http.DefaultHttpResponse; +import org.jboss.netty.handler.codec.http.HttpHeaders; +import org.jboss.netty.handler.codec.http.HttpResponse; +import org.jboss.netty.handler.codec.http.HttpResponseStatus; + +public class MaybeBlockResponseHandler extends SimpleChannelDownstreamHandler { + private final ContextStore contextStore; + + public MaybeBlockResponseHandler(final ContextStore contextStore) { + this.contextStore = contextStore; + } + + @Override + public void writeRequested(ChannelHandlerContext ctx, MessageEvent msg) throws Exception { + final ChannelTraceContext channelTraceContext = + contextStore.putIfAbsent(ctx.getChannel(), ChannelTraceContext.Factory.INSTANCE); + + AgentSpan span = channelTraceContext.getServerSpan(); + RequestContext requestContext; + if (span == null + || (requestContext = span.getRequestContext()) == null + || requestContext.getData(RequestContextSlot.APPSEC) == null) { + ctx.sendDownstream(msg); + return; + } + + if (channelTraceContext.isAnalyzedResponse()) { + if (channelTraceContext.isBlockedResponse()) { + // block further writes + } else { + ctx.sendDownstream(msg); + } + return; + } + + if (!(msg.getMessage() instanceof HttpResponse)) { + ctx.sendDownstream(msg); + return; + } + + HttpResponse origResponse = (HttpResponse) msg.getMessage(); + if (origResponse.getStatus() == HttpResponseStatus.CONTINUE) { + ctx.sendDownstream(msg); + return; + } + + Flow flow = + DECORATE.callIGCallbackResponseAndHeaders( + span, origResponse, origResponse.getStatus().getCode(), ResponseExtractAdapter.GETTER); + channelTraceContext.setAnalyzedResponse(true); + Flow.Action action = flow.getAction(); + if (!(action instanceof Flow.Action.RequestBlockingAction)) { + ctx.sendDownstream(msg); + return; + } + + channelTraceContext.setBlockedResponse(true); + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; + int httpCode = BlockingActionHelper.getHttpCode(rba.getStatusCode()); + HttpResponseStatus httpResponseStatus = HttpResponseStatus.valueOf(httpCode); + DefaultHttpResponse response = + new DefaultHttpResponse(origResponse.getProtocolVersion(), httpResponseStatus); + + HttpHeaders headers = response.headers(); + headers.set("Connection", "close"); + + for (Map.Entry h : rba.getExtraHeaders().entrySet()) { + headers.set(h.getKey(), h.getValue()); + } + + response.setChunked(false); + BlockingContentType bct = rba.getBlockingContentType(); + if (bct != BlockingContentType.NONE) { + String acceptHeader = channelTraceContext.getRequestHeaders().get("accept"); + BlockingActionHelper.TemplateType type = + BlockingActionHelper.determineTemplateType(bct, acceptHeader); + headers.set("Content-type", BlockingActionHelper.getContentType(type)); + byte[] template = BlockingActionHelper.getTemplate(type); + setContentLength(response, template.length); + ChannelBuffer buf = ChannelBuffers.wrappedBuffer(template); + response.setContent(buf); + } + + ChannelFuture future = Channels.future(ctx.getChannel()); + future.addListener( + fut -> { + if (!fut.isSuccess()) { + log.warn("Write of blocking response failed", fut.getCause()); + } + // close the connection because it can be in an invalid state at this point + // For instance, in a POST request we will still be receiving data from the + // client + fut.getChannel().close(); + }); + + requestContext.getTraceSegment().effectivelyBlocked(); + + Channels.write(ctx, future, response); + } +} diff --git a/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/server/NettyHttpServerDecorator.java b/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/server/NettyHttpServerDecorator.java index 1bb3b19ee79..4903e84d82d 100644 --- a/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/server/NettyHttpServerDecorator.java +++ b/dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/server/NettyHttpServerDecorator.java @@ -106,6 +106,11 @@ protected int status(final HttpResponse httpResponse) { return httpResponse.getStatus().getCode(); } + @Override + protected boolean isAppSecOnResponseSeparate() { + return true; + } + @Override protected BlockResponseFunction createBlockResponseFunction( HttpRequest httpRequest, Channel channel) { diff --git a/dd-java-agent/instrumentation/netty-3.8/src/test/groovy/datadog/trace/instrumentation/netty38/Netty38ServerTest.groovy b/dd-java-agent/instrumentation/netty-3.8/src/test/groovy/datadog/trace/instrumentation/netty38/Netty38ServerTest.groovy index 3bc6f11c12a..41395fee069 100644 --- a/dd-java-agent/instrumentation/netty-3.8/src/test/groovy/datadog/trace/instrumentation/netty38/Netty38ServerTest.groovy +++ b/dd-java-agent/instrumentation/netty-3.8/src/test/groovy/datadog/trace/instrumentation/netty38/Netty38ServerTest.groovy @@ -201,7 +201,12 @@ abstract class Netty38ServerTest extends HttpServerTest { @Override boolean testBlocking() { - true + false + } + + @Override + boolean testBlockingOnResponse() { + false } @Ignore("https://github.com/DataDog/dd-trace-java/pull/5213") diff --git a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/AttributeKeys.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/AttributeKeys.java index 1b17ac9da4b..eff307e153e 100644 --- a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/AttributeKeys.java +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/AttributeKeys.java @@ -5,6 +5,7 @@ import datadog.trace.api.GenericClassValue; import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import io.netty.handler.codec.http.HttpHeaders; import io.netty.util.AttributeKey; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -23,6 +24,15 @@ public final class AttributeKeys { CONNECT_PARENT_CONTINUATION_ATTRIBUTE_KEY = attributeKey("datadog.connect.parent.continuation"); + public static final AttributeKey REQUEST_HEADERS_ATTRIBUTE_KEY = + attributeKey("datadog.server.request.headers"); + + public static final AttributeKey ANALYZED_RESPONSE_KEY = + new AttributeKey<>("datadog.server.analyzed_response"); + + public static final AttributeKey BLOCKED_RESPONSE_KEY = + new AttributeKey<>("datadog.server.blocked_response"); + /** * Generate an attribute key or reuse the one existing in the global app map. This implementation * creates attributes only once even if the current class is loaded by several class loaders and diff --git a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/NettyChannelPipelineInstrumentation.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/NettyChannelPipelineInstrumentation.java index 58022fc7f4c..684eb82bb05 100644 --- a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/NettyChannelPipelineInstrumentation.java +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/NettyChannelPipelineInstrumentation.java @@ -19,6 +19,7 @@ import datadog.trace.instrumentation.netty40.server.HttpServerRequestTracingHandler; import datadog.trace.instrumentation.netty40.server.HttpServerResponseTracingHandler; import datadog.trace.instrumentation.netty40.server.HttpServerTracingHandler; +import datadog.trace.instrumentation.netty40.server.MaybeBlockResponseHandler; import io.netty.channel.ChannelHandler; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelPipeline; @@ -72,7 +73,8 @@ public String[] helperClassNames() { packageName + ".server.BlockingResponseHandler$IgnoreAllWritesHandler", packageName + ".server.HttpServerRequestTracingHandler", packageName + ".server.HttpServerResponseTracingHandler", - packageName + ".server.HttpServerTracingHandler" + packageName + ".server.HttpServerTracingHandler", + packageName + ".server.MaybeBlockResponseHandler", }; } @@ -133,13 +135,16 @@ public static void addHandler( try { ChannelHandler toAdd = null; + ChannelHandler toAdd2 = null; // Server pipeline handlers if (handler instanceof HttpServerCodec) { toAdd = new HttpServerTracingHandler(); + toAdd2 = MaybeBlockResponseHandler.INSTANCE; } else if (handler instanceof HttpRequestDecoder) { toAdd = HttpServerRequestTracingHandler.INSTANCE; } else if (handler instanceof HttpResponseEncoder) { toAdd = HttpServerResponseTracingHandler.INSTANCE; + toAdd2 = MaybeBlockResponseHandler.INSTANCE; } else // Client pipeline handlers if (handler instanceof HttpClientCodec) { @@ -159,6 +164,13 @@ public static void addHandler( pipeline.remove(existing); } pipeline.addAfter(handlerName, toAdd.getClass().getName(), toAdd); + if (toAdd2 != null) { + ChannelHandler existing2 = pipeline.get(toAdd2.getClass()); + if (existing2 != null) { + pipeline.remove(existing2); + } + pipeline.addAfter(toAdd.getClass().getName(), toAdd2.getClass().getName(), toAdd2); + } } } } catch (final IllegalArgumentException e) { diff --git a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerRequestTracingHandler.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerRequestTracingHandler.java index f66b0ebbbfe..e2b560aff86 100644 --- a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerRequestTracingHandler.java +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerRequestTracingHandler.java @@ -1,6 +1,9 @@ package datadog.trace.instrumentation.netty40.server; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.instrumentation.netty40.AttributeKeys.ANALYZED_RESPONSE_KEY; +import static datadog.trace.instrumentation.netty40.AttributeKeys.BLOCKED_RESPONSE_KEY; +import static datadog.trace.instrumentation.netty40.AttributeKeys.REQUEST_HEADERS_ATTRIBUTE_KEY; import static datadog.trace.instrumentation.netty40.AttributeKeys.SPAN_ATTRIBUTE_KEY; import static datadog.trace.instrumentation.netty40.server.NettyHttpServerDecorator.DECORATE; @@ -8,6 +11,7 @@ import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentSpan.Context; +import io.netty.channel.Channel; import io.netty.channel.ChannelHandler; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelInboundHandlerAdapter; @@ -21,8 +25,9 @@ public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapte @Override public void channelRead(final ChannelHandlerContext ctx, final Object msg) { + Channel channel = ctx.channel(); if (!(msg instanceof HttpRequest)) { - final AgentSpan span = ctx.channel().attr(SPAN_ATTRIBUTE_KEY).get(); + final AgentSpan span = channel.attr(SPAN_ATTRIBUTE_KEY).get(); if (span == null) { ctx.fireChannelRead(msg); // superclass does not throw } else { @@ -41,11 +46,15 @@ public void channelRead(final ChannelHandlerContext ctx, final Object msg) { try (final AgentScope scope = activateSpan(span)) { DECORATE.afterStart(span); - DECORATE.onRequest(span, ctx.channel(), request, extractedContext); + DECORATE.onRequest(span, channel, request, extractedContext); scope.setAsyncPropagation(true); - ctx.channel().attr(SPAN_ATTRIBUTE_KEY).set(span); + channel.attr(ANALYZED_RESPONSE_KEY).set(null); + channel.attr(BLOCKED_RESPONSE_KEY).set(null); + + channel.attr(SPAN_ATTRIBUTE_KEY).set(span); + channel.attr(REQUEST_HEADERS_ATTRIBUTE_KEY).set(request.headers()); Flow.Action.RequestBlockingAction rba = span.getRequestBlockingAction(); if (rba != null) { diff --git a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/MaybeBlockResponseHandler.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/MaybeBlockResponseHandler.java new file mode 100644 index 00000000000..6e3ec443840 --- /dev/null +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/MaybeBlockResponseHandler.java @@ -0,0 +1,139 @@ +package datadog.trace.instrumentation.netty40.server; + +import static datadog.trace.instrumentation.netty40.AttributeKeys.ANALYZED_RESPONSE_KEY; +import static datadog.trace.instrumentation.netty40.AttributeKeys.BLOCKED_RESPONSE_KEY; +import static datadog.trace.instrumentation.netty40.AttributeKeys.REQUEST_HEADERS_ATTRIBUTE_KEY; +import static datadog.trace.instrumentation.netty40.AttributeKeys.SPAN_ATTRIBUTE_KEY; +import static datadog.trace.instrumentation.netty40.server.NettyHttpServerDecorator.DECORATE; +import static io.netty.handler.codec.http.HttpHeaders.setContentLength; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.bootstrap.blocking.BlockingActionHelper; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import io.netty.channel.Channel; +import io.netty.channel.ChannelHandler; +import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelOutboundHandler; +import io.netty.channel.ChannelOutboundHandlerAdapter; +import io.netty.channel.ChannelPromise; +import io.netty.handler.codec.http.DefaultFullHttpResponse; +import io.netty.handler.codec.http.FullHttpResponse; +import io.netty.handler.codec.http.HttpHeaders; +import io.netty.handler.codec.http.HttpResponse; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.netty.util.ReferenceCountUtil; +import java.util.Map; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@ChannelHandler.Sharable +public class MaybeBlockResponseHandler extends ChannelOutboundHandlerAdapter { + public static final ChannelOutboundHandler INSTANCE = new MaybeBlockResponseHandler(); + public static final Logger log = LoggerFactory.getLogger(MaybeBlockResponseHandler.class); + + private MaybeBlockResponseHandler() {} + + private static boolean isAnalyzedResponse(Channel ch) { + return ch.attr(ANALYZED_RESPONSE_KEY).get() != null; + } + + private static void markAnalyzedResponse(Channel ch) { + ch.attr(ANALYZED_RESPONSE_KEY).set(Boolean.TRUE); + } + + private static boolean isBlockedResponse(Channel ch) { + return ch.attr(BLOCKED_RESPONSE_KEY).get() != null; + } + + private static void markBlockedResponse(Channel ch) { + ch.attr(BLOCKED_RESPONSE_KEY).set(Boolean.TRUE); + } + + @Override + public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise prm) throws Exception { + Channel channel = ctx.channel(); + + AgentSpan span = channel.attr(SPAN_ATTRIBUTE_KEY).get(); + RequestContext requestContext; + if (span == null + || (requestContext = span.getRequestContext()) == null + || requestContext.getData(RequestContextSlot.APPSEC) == null) { + super.write(ctx, msg, prm); + return; + } + + if (isAnalyzedResponse(channel)) { + if (isBlockedResponse(channel)) { + // block further writes + log.debug("Write suppressed, msg {} dropped", msg); + ReferenceCountUtil.release(msg); + } else { + super.write(ctx, msg, prm); + } + return; + } + + if (!(msg instanceof HttpResponse)) { + super.write(ctx, msg, prm); + return; + } + HttpResponse origResponse = (HttpResponse) msg; + if (origResponse.getStatus().code() == HttpResponseStatus.CONTINUE.code()) { + super.write(ctx, msg, prm); + return; + } + + Flow flow = + DECORATE.callIGCallbackResponseAndHeaders( + span, origResponse, origResponse.getStatus().code(), ResponseExtractAdapter.GETTER); + markAnalyzedResponse(channel); + Flow.Action action = flow.getAction(); + if (!(action instanceof Flow.Action.RequestBlockingAction)) { + super.write(ctx, msg, prm); + return; + } + + markBlockedResponse(channel); + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; + int httpCode = BlockingActionHelper.getHttpCode(rba.getStatusCode()); + HttpResponseStatus httpResponseStatus = HttpResponseStatus.valueOf(httpCode); + FullHttpResponse response = + new DefaultFullHttpResponse(origResponse.getProtocolVersion(), httpResponseStatus); + ReferenceCountUtil.release(msg); + + HttpHeaders headers = response.headers(); + headers.set("Connection", "close"); + + for (Map.Entry h : rba.getExtraHeaders().entrySet()) { + headers.set(h.getKey(), h.getValue()); + } + + BlockingContentType bct = rba.getBlockingContentType(); + if (bct != BlockingContentType.NONE) { + HttpHeaders reqHeaders = ctx.attr(REQUEST_HEADERS_ATTRIBUTE_KEY).get(); + String acceptHeader = reqHeaders != null ? reqHeaders.get("accept") : null; + BlockingActionHelper.TemplateType type = + BlockingActionHelper.determineTemplateType(bct, acceptHeader); + headers.set("Content-type", BlockingActionHelper.getContentType(type)); + byte[] template = BlockingActionHelper.getTemplate(type); + setContentLength(response, template.length); + response.content().writeBytes(template); + } + + requestContext.getTraceSegment().effectivelyBlocked(); + log.debug("About to write and flush blocking response {}", response); + ctx.writeAndFlush(response, prm) + .addListener( + fut -> { + if (!fut.isSuccess()) { + log.warn("Write of blocking response failed", fut.cause()); + } else { + log.debug("Write of blocking response succeeded"); + } + channel.close(); + }); + } +} diff --git a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/NettyHttpServerDecorator.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/NettyHttpServerDecorator.java index 02314124c0f..b6bfb5c3503 100644 --- a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/NettyHttpServerDecorator.java +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/NettyHttpServerDecorator.java @@ -102,6 +102,11 @@ protected int status(final HttpResponse httpResponse) { return httpResponse.getStatus().code(); } + @Override + protected boolean isAppSecOnResponseSeparate() { + return true; + } + @Override protected BlockResponseFunction createBlockResponseFunction( HttpRequest httpRequest, Channel channel) { diff --git a/dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ServerTest.groovy b/dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ServerTest.groovy index 069dd167ccd..1c03e4d9bf8 100644 --- a/dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ServerTest.groovy +++ b/dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ServerTest.groovy @@ -1,20 +1,3 @@ -import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR -import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION -import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.FORWARDED -import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND -import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_ENCODED_BOTH -import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_ENCODED_QUERY -import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM -import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT -import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS -import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.USER_BLOCK -import static io.netty.handler.codec.http.HttpHeaders.Names.CONTENT_LENGTH -import static io.netty.handler.codec.http.HttpHeaders.Names.CONTENT_TYPE -import static io.netty.handler.codec.http.HttpHeaders.is100ContinueExpected -import static io.netty.handler.codec.http.HttpResponseStatus.CONTINUE -import static io.netty.handler.codec.http.HttpResponseStatus.INTERNAL_SERVER_ERROR -import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1 - import datadog.appsec.api.blocking.Blocking import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.base.HttpServerTest @@ -43,6 +26,23 @@ import io.netty.handler.logging.LoggingHandler import io.netty.util.CharsetUtil import spock.lang.Ignore +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.FORWARDED +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_ENCODED_BOTH +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_ENCODED_QUERY +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.USER_BLOCK +import static io.netty.handler.codec.http.HttpHeaders.Names.CONTENT_LENGTH +import static io.netty.handler.codec.http.HttpHeaders.Names.CONTENT_TYPE +import static io.netty.handler.codec.http.HttpHeaders.is100ContinueExpected +import static io.netty.handler.codec.http.HttpResponseStatus.CONTINUE +import static io.netty.handler.codec.http.HttpResponseStatus.INTERNAL_SERVER_ERROR +import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1 + abstract class Netty40ServerTest extends HttpServerTest { static final LoggingHandler LOGGING_HANDLER = new LoggingHandler(SERVER_LOGGER.name, LogLevel.DEBUG) @@ -169,6 +169,11 @@ abstract class Netty40ServerTest extends HttpServerTest { true } + @Override + boolean testBlockingOnResponse() { + true + } + @Ignore("https://github.com/DataDog/dd-trace-java/pull/5213") @Override boolean testBadUrl() { diff --git a/dd-java-agent/instrumentation/netty-4.1-shared/src/main/java/datadog/trace/instrumentation/netty41/AttributeKeys.java b/dd-java-agent/instrumentation/netty-4.1-shared/src/main/java/datadog/trace/instrumentation/netty41/AttributeKeys.java index 397377df963..74d124fb866 100644 --- a/dd-java-agent/instrumentation/netty-4.1-shared/src/main/java/datadog/trace/instrumentation/netty41/AttributeKeys.java +++ b/dd-java-agent/instrumentation/netty-4.1-shared/src/main/java/datadog/trace/instrumentation/netty41/AttributeKeys.java @@ -5,6 +5,7 @@ import datadog.trace.api.GenericClassValue; import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import io.netty.handler.codec.http.HttpHeaders; import io.netty.util.AttributeKey; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -23,6 +24,15 @@ public final class AttributeKeys { CONNECT_PARENT_CONTINUATION_ATTRIBUTE_KEY = attributeKey("datadog.connect.parent.continuation"); + public static final AttributeKey REQUEST_HEADERS_ATTRIBUTE_KEY = + attributeKey("datadog.server.request.headers"); + + public static final AttributeKey ANALYZED_RESPONSE_KEY = + attributeKey("datadog.server.analyzed_response"); + + public static final AttributeKey BLOCKED_RESPONSE_KEY = + attributeKey("datadog.server.blocked_response"); + /** * Generate an attribute key or reuse the one existing in the global app map. This implementation * creates attributes only once even if the current class is loaded by several class loaders and diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyChannelPipelineInstrumentation.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyChannelPipelineInstrumentation.java index 505d7d384fc..ed87823cc8f 100644 --- a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyChannelPipelineInstrumentation.java +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyChannelPipelineInstrumentation.java @@ -19,6 +19,7 @@ import datadog.trace.instrumentation.netty41.server.HttpServerRequestTracingHandler; import datadog.trace.instrumentation.netty41.server.HttpServerResponseTracingHandler; import datadog.trace.instrumentation.netty41.server.HttpServerTracingHandler; +import datadog.trace.instrumentation.netty41.server.MaybeBlockResponseHandler; import io.netty.channel.ChannelHandler; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelPipeline; @@ -72,7 +73,8 @@ public String[] helperClassNames() { packageName + ".server.BlockingResponseHandler$IgnoreAllWritesHandler", packageName + ".server.HttpServerRequestTracingHandler", packageName + ".server.HttpServerResponseTracingHandler", - packageName + ".server.HttpServerTracingHandler" + packageName + ".server.HttpServerTracingHandler", + packageName + ".server.MaybeBlockResponseHandler", }; } @@ -133,13 +135,16 @@ public static void addHandler( try { ChannelHandler toAdd = null; + ChannelHandler toAdd2 = null; // Server pipeline handlers if (handler instanceof HttpServerCodec) { toAdd = new HttpServerTracingHandler(); + toAdd2 = MaybeBlockResponseHandler.INSTANCE; } else if (handler instanceof HttpRequestDecoder) { toAdd = HttpServerRequestTracingHandler.INSTANCE; } else if (handler instanceof HttpResponseEncoder) { toAdd = HttpServerResponseTracingHandler.INSTANCE; + toAdd2 = MaybeBlockResponseHandler.INSTANCE; } else // Client pipeline handlers if (handler instanceof HttpClientCodec) { @@ -159,6 +164,13 @@ public static void addHandler( pipeline.remove(existing); } pipeline.addAfter(handlerName, null, toAdd); + if (toAdd2 != null) { + ChannelHandler existing2 = pipeline.get(toAdd2.getClass()); + if (existing2 != null) { + pipeline.remove(existing2); + } + pipeline.addAfter(pipeline.context(toAdd).name(), null, toAdd2); + } } } } catch (final IllegalArgumentException e) { diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/HttpServerRequestTracingHandler.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/HttpServerRequestTracingHandler.java index d2928abf105..68aefac1341 100644 --- a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/HttpServerRequestTracingHandler.java +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/HttpServerRequestTracingHandler.java @@ -1,6 +1,9 @@ package datadog.trace.instrumentation.netty41.server; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.instrumentation.netty41.AttributeKeys.ANALYZED_RESPONSE_KEY; +import static datadog.trace.instrumentation.netty41.AttributeKeys.BLOCKED_RESPONSE_KEY; +import static datadog.trace.instrumentation.netty41.AttributeKeys.REQUEST_HEADERS_ATTRIBUTE_KEY; import static datadog.trace.instrumentation.netty41.AttributeKeys.SPAN_ATTRIBUTE_KEY; import static datadog.trace.instrumentation.netty41.server.NettyHttpServerDecorator.DECORATE; @@ -8,6 +11,7 @@ import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentSpan.Context; +import io.netty.channel.Channel; import io.netty.channel.ChannelHandler; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelInboundHandlerAdapter; @@ -20,8 +24,9 @@ public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapte @Override public void channelRead(final ChannelHandlerContext ctx, final Object msg) { + Channel channel = ctx.channel(); if (!(msg instanceof HttpRequest)) { - final AgentSpan span = ctx.channel().attr(SPAN_ATTRIBUTE_KEY).get(); + final AgentSpan span = channel.attr(SPAN_ATTRIBUTE_KEY).get(); if (span == null) { ctx.fireChannelRead(msg); // superclass does not throw } else { @@ -40,11 +45,15 @@ public void channelRead(final ChannelHandlerContext ctx, final Object msg) { try (final AgentScope scope = activateSpan(span)) { DECORATE.afterStart(span); - DECORATE.onRequest(span, ctx.channel(), request, extractedContext); + DECORATE.onRequest(span, channel, request, extractedContext); scope.setAsyncPropagation(true); - ctx.channel().attr(SPAN_ATTRIBUTE_KEY).set(span); + channel.attr(ANALYZED_RESPONSE_KEY).set(null); + channel.attr(BLOCKED_RESPONSE_KEY).set(null); + + channel.attr(SPAN_ATTRIBUTE_KEY).set(span); + channel.attr(REQUEST_HEADERS_ATTRIBUTE_KEY).set(request.headers()); Flow.Action.RequestBlockingAction rba = span.getRequestBlockingAction(); if (rba != null) { diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/MaybeBlockResponseHandler.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/MaybeBlockResponseHandler.java new file mode 100644 index 00000000000..9139e318703 --- /dev/null +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/MaybeBlockResponseHandler.java @@ -0,0 +1,136 @@ +package datadog.trace.instrumentation.netty41.server; + +import static datadog.trace.instrumentation.netty41.AttributeKeys.ANALYZED_RESPONSE_KEY; +import static datadog.trace.instrumentation.netty41.AttributeKeys.BLOCKED_RESPONSE_KEY; +import static datadog.trace.instrumentation.netty41.AttributeKeys.REQUEST_HEADERS_ATTRIBUTE_KEY; +import static datadog.trace.instrumentation.netty41.AttributeKeys.SPAN_ATTRIBUTE_KEY; +import static datadog.trace.instrumentation.netty41.server.NettyHttpServerDecorator.DECORATE; +import static io.netty.handler.codec.http.HttpHeaders.setContentLength; + +import datadog.appsec.api.blocking.BlockingContentType; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.bootstrap.blocking.BlockingActionHelper; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import io.netty.channel.Channel; +import io.netty.channel.ChannelHandler; +import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelOutboundHandler; +import io.netty.channel.ChannelOutboundHandlerAdapter; +import io.netty.channel.ChannelPromise; +import io.netty.handler.codec.http.DefaultFullHttpResponse; +import io.netty.handler.codec.http.FullHttpResponse; +import io.netty.handler.codec.http.HttpHeaders; +import io.netty.handler.codec.http.HttpResponse; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.netty.util.ReferenceCountUtil; +import java.util.Map; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@ChannelHandler.Sharable +public class MaybeBlockResponseHandler extends ChannelOutboundHandlerAdapter { + public static final ChannelOutboundHandler INSTANCE = new MaybeBlockResponseHandler(); + public static final Logger log = LoggerFactory.getLogger(MaybeBlockResponseHandler.class); + + private MaybeBlockResponseHandler() {} + + private static boolean isAnalyzedResponse(Channel ch) { + return ch.attr(ANALYZED_RESPONSE_KEY).get() != null; + } + + private static void markAnalyzedResponse(Channel ch) { + ch.attr(ANALYZED_RESPONSE_KEY).set(Boolean.TRUE); + } + + private static boolean isBlockedResponse(Channel ch) { + return ch.attr(BLOCKED_RESPONSE_KEY).get() != null; + } + + private static void markBlockedResponse(Channel ch) { + ch.attr(BLOCKED_RESPONSE_KEY).set(Boolean.TRUE); + } + + @Override + public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise prm) throws Exception { + Channel channel = ctx.channel(); + + AgentSpan span = channel.attr(SPAN_ATTRIBUTE_KEY).get(); + RequestContext requestContext; + if (span == null || (requestContext = span.getRequestContext()) == null) { + super.write(ctx, msg, prm); + return; + } + + if (isAnalyzedResponse(channel)) { + if (isBlockedResponse(channel)) { + // block further writes + log.debug("Write suppressed, msg {} dropped", msg); + ReferenceCountUtil.release(msg); + } else { + super.write(ctx, msg, prm); + } + return; + } + + if (!(msg instanceof HttpResponse)) { + super.write(ctx, msg, prm); + return; + } + HttpResponse origResponse = (HttpResponse) msg; + if (origResponse.status().code() == HttpResponseStatus.CONTINUE.code()) { + super.write(ctx, msg, prm); + return; + } + + Flow flow = + DECORATE.callIGCallbackResponseAndHeaders( + span, origResponse, origResponse.getStatus().code(), ResponseExtractAdapter.GETTER); + markAnalyzedResponse(channel); + Flow.Action action = flow.getAction(); + if (!(action instanceof Flow.Action.RequestBlockingAction)) { + super.write(ctx, msg, prm); + return; + } + + markBlockedResponse(channel); + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; + int httpCode = BlockingActionHelper.getHttpCode(rba.getStatusCode()); + HttpResponseStatus httpResponseStatus = HttpResponseStatus.valueOf(httpCode); + FullHttpResponse response = + new DefaultFullHttpResponse(origResponse.getProtocolVersion(), httpResponseStatus); + ReferenceCountUtil.release(msg); + + HttpHeaders headers = response.headers(); + headers.set("Connection", "close"); + + for (Map.Entry h : rba.getExtraHeaders().entrySet()) { + headers.set(h.getKey(), h.getValue()); + } + + BlockingContentType bct = rba.getBlockingContentType(); + if (bct != BlockingContentType.NONE) { + HttpHeaders reqHeaders = ctx.attr(REQUEST_HEADERS_ATTRIBUTE_KEY).get(); + String acceptHeader = reqHeaders != null ? reqHeaders.get("accept") : null; + BlockingActionHelper.TemplateType type = + BlockingActionHelper.determineTemplateType(bct, acceptHeader); + headers.set("Content-type", BlockingActionHelper.getContentType(type)); + byte[] template = BlockingActionHelper.getTemplate(type); + setContentLength(response, template.length); + response.content().writeBytes(template); + } + + requestContext.getTraceSegment().effectivelyBlocked(); + log.debug("About to write and flush blocking response {}", response); + ctx.writeAndFlush(response, prm) + .addListener( + fut -> { + if (!fut.isSuccess()) { + log.warn("Write of blocking response failed", fut.cause()); + } else { + log.debug("Write of blocking response succeeded"); + } + channel.close(); + }); + } +} diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/NettyHttpServerDecorator.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/NettyHttpServerDecorator.java index 008da200c14..a7bd8aba2cf 100644 --- a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/NettyHttpServerDecorator.java +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/NettyHttpServerDecorator.java @@ -100,6 +100,11 @@ protected int status(final HttpResponse httpResponse) { return httpResponse.status().code(); } + @Override + protected boolean isAppSecOnResponseSeparate() { + return true; + } + @Override protected BlockResponseFunction createBlockResponseFunction( HttpRequest httpRequest, Channel channel) { diff --git a/dd-java-agent/instrumentation/netty-4.1/src/test/groovy/Netty41ServerTest.groovy b/dd-java-agent/instrumentation/netty-4.1/src/test/groovy/Netty41ServerTest.groovy index d2f542943c2..f91910d3269 100644 --- a/dd-java-agent/instrumentation/netty-4.1/src/test/groovy/Netty41ServerTest.groovy +++ b/dd-java-agent/instrumentation/netty-4.1/src/test/groovy/Netty41ServerTest.groovy @@ -203,6 +203,11 @@ abstract class Netty41ServerTest extends HttpServerTest { true } + @Override + boolean testBlockingOnResponse() { + true + } + //@Ignore("https://github.com/DataDog/dd-trace-java/pull/5213") @Override boolean testBadUrl() { diff --git a/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy b/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy index 0c4188afcc4..be276be3567 100644 --- a/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy +++ b/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy @@ -88,6 +88,11 @@ class RatpackHttpServerTest extends HttpServerTest { true } + @Override + boolean testBlockingOnResponse() { + true + } + @Override Serializable expectedServerSpanRoute(ServerEndpoint endpoint) { return String diff --git a/dd-java-agent/instrumentation/vertx-web-3.4/src/test/groovy/server/VertxHttpServerForkedTest.groovy b/dd-java-agent/instrumentation/vertx-web-3.4/src/test/groovy/server/VertxHttpServerForkedTest.groovy index 01075ae5377..d8d374acce1 100644 --- a/dd-java-agent/instrumentation/vertx-web-3.4/src/test/groovy/server/VertxHttpServerForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx-web-3.4/src/test/groovy/server/VertxHttpServerForkedTest.groovy @@ -87,6 +87,11 @@ class VertxHttpServerForkedTest extends HttpServerTest { true } + @Override + boolean testBlockingOnResponse() { + true + } + @Override Class expectedExceptionType() { return RuntimeException diff --git a/dd-java-agent/instrumentation/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy b/dd-java-agent/instrumentation/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy index 89b55bac3b0..da84aae0ab3 100644 --- a/dd-java-agent/instrumentation/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy @@ -82,6 +82,11 @@ class VertxHttpServerForkedTest extends HttpServerTest { true } + @Override + boolean testBlockingOnResponse() { + true + } + @Override boolean isRequestBodyNoStreaming() { true From 9dd556a58b99d8aca1d5030521ce061dbf944413 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Gonz=C3=A1lez=20Garc=C3=ADa?= Date: Thu, 17 Aug 2023 09:42:06 +0200 Subject: [PATCH 023/367] Upgrade IAST XSS smoke tests (#5635) # What Does This Do - Add more xss tests for springboot - Place all the XSS springboot xss endpoint in a new XssController # Motivation Test all instrumented PrintWriter instrumented methods --- .../AbstractIastSpringBootTest.groovy | 25 ++- .../controller/IastWebController.java | 10 - .../springboot/controller/XssController.java | 193 ++++++++++++++++++ .../controller/IastWebController.java | 10 - .../springboot/controller/XssController.java | 193 ++++++++++++++++++ 5 files changed, 408 insertions(+), 23 deletions(-) create mode 100644 dd-smoke-tests/spring-boot-2.6-webmvc/src/main/java/datadog/smoketest/springboot/controller/XssController.java create mode 100644 dd-smoke-tests/springboot/src/main/java/datadog/smoketest/springboot/controller/XssController.java diff --git a/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy b/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy index 386e3f2ba1d..66f647ab435 100644 --- a/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy +++ b/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy @@ -303,16 +303,35 @@ abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest { hasVulnerability { vul -> vul.type == 'TRUST_BOUNDARY_VIOLATION' } } - void 'xss is present when write String'() { + void 'xss is present'() { setup: - final url = "http://localhost:${httpPort}/xss/write?string=test" + final url = "http://localhost:${httpPort}/xss/${method}?string=${param}" final request = new Request.Builder().url(url).get().build() when: client.newCall(request).execute() then: - hasVulnerability { vul -> vul.type == 'XSS' && vul.location.method == 'xssWrite' } + hasVulnerability { vul -> vul.type == 'XSS' && vul.location.method == method } + + where: + method | param + 'write' | 'test' + 'write2' | 'test' + 'write3' | 'test' + 'write4' | 'test' + 'print' | 'test' + 'print2' | 'test' + 'println' | 'test' + 'println2' | 'test' + 'printf' | 'Formatted%20like%3A%20%251%24s%20and%20%252%24s.' + 'printf2' | 'test' + 'printf3' | 'Formatted%20like%3A%20%251%24s%20and%20%252%24s.' + 'printf4' | 'test' + 'format' | 'Formatted%20like%3A%20%251%24s%20and%20%252%24s.' + 'format2' | 'test' + 'format3' | 'Formatted%20like%3A%20%251%24s%20and%20%252%24s.' + 'format4' | 'test' } void 'trust boundary violation with cookie propagation'() { diff --git a/dd-smoke-tests/spring-boot-2.6-webmvc/src/main/java/datadog/smoketest/springboot/controller/IastWebController.java b/dd-smoke-tests/spring-boot-2.6-webmvc/src/main/java/datadog/smoketest/springboot/controller/IastWebController.java index bf085a7a286..f9864cf3e57 100644 --- a/dd-smoke-tests/spring-boot-2.6-webmvc/src/main/java/datadog/smoketest/springboot/controller/IastWebController.java +++ b/dd-smoke-tests/spring-boot-2.6-webmvc/src/main/java/datadog/smoketest/springboot/controller/IastWebController.java @@ -265,16 +265,6 @@ public String xpathInjectionEvaluate(final HttpServletRequest request) return "XPath Injection page"; } - @GetMapping("/xss/write") - @SuppressFBWarnings - public void xssWrite(final HttpServletRequest request, final HttpServletResponse response) { - try { - response.getWriter().write(request.getParameter("string")); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - @GetMapping("/trust_boundary_violation") public String trustBoundaryViolation(final HttpServletRequest request) { String paramValue = request.getParameter("paramValue"); diff --git a/dd-smoke-tests/spring-boot-2.6-webmvc/src/main/java/datadog/smoketest/springboot/controller/XssController.java b/dd-smoke-tests/spring-boot-2.6-webmvc/src/main/java/datadog/smoketest/springboot/controller/XssController.java new file mode 100644 index 00000000000..8feb683a886 --- /dev/null +++ b/dd-smoke-tests/spring-boot-2.6-webmvc/src/main/java/datadog/smoketest/springboot/controller/XssController.java @@ -0,0 +1,193 @@ +package datadog.smoketest.springboot.controller; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import java.io.IOException; +import java.util.Locale; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +@RestController +@RequestMapping("/xss") +public class XssController { + + @GetMapping("/write") + @SuppressFBWarnings + public void write(final HttpServletRequest request, final HttpServletResponse response) { + try { + response.getWriter().write(request.getParameter("string")); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/write2") + @SuppressFBWarnings + public void write2(final HttpServletRequest request, final HttpServletResponse response) { + try { + response.getWriter().write(request.getParameter("string").toCharArray()); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/write3") + @SuppressFBWarnings + public void write3(final HttpServletRequest request, final HttpServletResponse response) { + try { + String insecure = request.getParameter("string"); + response.getWriter().write(insecure, 0, insecure.length()); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/write4") + @SuppressFBWarnings + public void write4(final HttpServletRequest request, final HttpServletResponse response) { + try { + char[] buf = request.getParameter("string").toCharArray(); + response.getWriter().write(buf, 0, buf.length); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/print") + @SuppressFBWarnings + public void print(final HttpServletRequest request, final HttpServletResponse response) { + try { + response.getWriter().print(request.getParameter("string")); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/print2") + @SuppressFBWarnings + public void print2(final HttpServletRequest request, final HttpServletResponse response) { + try { + response.getWriter().print(request.getParameter("string").toCharArray()); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/println") + @SuppressFBWarnings + public void println(final HttpServletRequest request, final HttpServletResponse response) { + try { + response.getWriter().println(request.getParameter("string")); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/println2") + @SuppressFBWarnings + public void println2(final HttpServletRequest request, final HttpServletResponse response) { + try { + response.getWriter().println(request.getParameter("string").toCharArray()); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/printf") + @SuppressFBWarnings + public void printf(final HttpServletRequest request, final HttpServletResponse response) { + try { + String format = request.getParameter("string"); + String[] array = {"A", "B"}; + response.getWriter().printf(format, array); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/printf2") + @SuppressFBWarnings + public void printf2(final HttpServletRequest request, final HttpServletResponse response) { + try { + String format = "Formatted like: %1$s and %2$s."; + String[] array = {"A", request.getParameter("string")}; + response.getWriter().printf(format, array); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/printf3") + @SuppressFBWarnings + public void printf3(final HttpServletRequest request, final HttpServletResponse response) { + try { + String format = request.getParameter("string"); + String[] array = {"A", "B"}; + response.getWriter().printf(Locale.getDefault(), format, array); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/printf4") + @SuppressFBWarnings + public void printf4(final HttpServletRequest request, final HttpServletResponse response) { + try { + String format = "Formatted like: %1$s and %2$s."; + String[] array = {"A", request.getParameter("string")}; + response.getWriter().printf(Locale.getDefault(), format, array); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/format") + @SuppressFBWarnings + public void format(final HttpServletRequest request, final HttpServletResponse response) { + try { + String format = request.getParameter("string"); + String[] array = {"A", "B"}; + response.getWriter().format(format, array); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/format2") + @SuppressFBWarnings + public void format2(final HttpServletRequest request, final HttpServletResponse response) { + try { + String format = "Formatted like: %1$s and %2$s."; + String[] array = {"A", request.getParameter("string")}; + response.getWriter().format(format, array); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/format3") + @SuppressFBWarnings + public void format3(final HttpServletRequest request, final HttpServletResponse response) { + try { + String format = request.getParameter("string"); + String[] array = {"A", "B"}; + response.getWriter().format(Locale.getDefault(), format, array); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/format4") + @SuppressFBWarnings + public void format4(final HttpServletRequest request, final HttpServletResponse response) { + try { + String format = "Formatted like: %1$s and %2$s."; + String[] array = {"A", request.getParameter("string")}; + response.getWriter().format(Locale.getDefault(), format, array); + } catch (IOException e) { + throw new RuntimeException(e); + } + } +} diff --git a/dd-smoke-tests/springboot/src/main/java/datadog/smoketest/springboot/controller/IastWebController.java b/dd-smoke-tests/springboot/src/main/java/datadog/smoketest/springboot/controller/IastWebController.java index 70fdb08471f..166ffac74e3 100644 --- a/dd-smoke-tests/springboot/src/main/java/datadog/smoketest/springboot/controller/IastWebController.java +++ b/dd-smoke-tests/springboot/src/main/java/datadog/smoketest/springboot/controller/IastWebController.java @@ -291,16 +291,6 @@ public String trustBoundaryViolationForCookie(final HttpServletRequest request) return "Trust Boundary violation with cookie page"; } - @GetMapping("/xss/write") - @SuppressFBWarnings - public void xssWrite(final HttpServletRequest request, final HttpServletResponse response) { - try { - response.getWriter().write(request.getParameter("string")); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - @GetMapping(value = "/hstsmissing", produces = "text/html") public String hstsHeaderMissing(HttpServletResponse response) { response.setStatus(HttpStatus.OK.value()); diff --git a/dd-smoke-tests/springboot/src/main/java/datadog/smoketest/springboot/controller/XssController.java b/dd-smoke-tests/springboot/src/main/java/datadog/smoketest/springboot/controller/XssController.java new file mode 100644 index 00000000000..8feb683a886 --- /dev/null +++ b/dd-smoke-tests/springboot/src/main/java/datadog/smoketest/springboot/controller/XssController.java @@ -0,0 +1,193 @@ +package datadog.smoketest.springboot.controller; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import java.io.IOException; +import java.util.Locale; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +@RestController +@RequestMapping("/xss") +public class XssController { + + @GetMapping("/write") + @SuppressFBWarnings + public void write(final HttpServletRequest request, final HttpServletResponse response) { + try { + response.getWriter().write(request.getParameter("string")); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/write2") + @SuppressFBWarnings + public void write2(final HttpServletRequest request, final HttpServletResponse response) { + try { + response.getWriter().write(request.getParameter("string").toCharArray()); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/write3") + @SuppressFBWarnings + public void write3(final HttpServletRequest request, final HttpServletResponse response) { + try { + String insecure = request.getParameter("string"); + response.getWriter().write(insecure, 0, insecure.length()); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/write4") + @SuppressFBWarnings + public void write4(final HttpServletRequest request, final HttpServletResponse response) { + try { + char[] buf = request.getParameter("string").toCharArray(); + response.getWriter().write(buf, 0, buf.length); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/print") + @SuppressFBWarnings + public void print(final HttpServletRequest request, final HttpServletResponse response) { + try { + response.getWriter().print(request.getParameter("string")); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/print2") + @SuppressFBWarnings + public void print2(final HttpServletRequest request, final HttpServletResponse response) { + try { + response.getWriter().print(request.getParameter("string").toCharArray()); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/println") + @SuppressFBWarnings + public void println(final HttpServletRequest request, final HttpServletResponse response) { + try { + response.getWriter().println(request.getParameter("string")); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/println2") + @SuppressFBWarnings + public void println2(final HttpServletRequest request, final HttpServletResponse response) { + try { + response.getWriter().println(request.getParameter("string").toCharArray()); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/printf") + @SuppressFBWarnings + public void printf(final HttpServletRequest request, final HttpServletResponse response) { + try { + String format = request.getParameter("string"); + String[] array = {"A", "B"}; + response.getWriter().printf(format, array); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/printf2") + @SuppressFBWarnings + public void printf2(final HttpServletRequest request, final HttpServletResponse response) { + try { + String format = "Formatted like: %1$s and %2$s."; + String[] array = {"A", request.getParameter("string")}; + response.getWriter().printf(format, array); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/printf3") + @SuppressFBWarnings + public void printf3(final HttpServletRequest request, final HttpServletResponse response) { + try { + String format = request.getParameter("string"); + String[] array = {"A", "B"}; + response.getWriter().printf(Locale.getDefault(), format, array); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/printf4") + @SuppressFBWarnings + public void printf4(final HttpServletRequest request, final HttpServletResponse response) { + try { + String format = "Formatted like: %1$s and %2$s."; + String[] array = {"A", request.getParameter("string")}; + response.getWriter().printf(Locale.getDefault(), format, array); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/format") + @SuppressFBWarnings + public void format(final HttpServletRequest request, final HttpServletResponse response) { + try { + String format = request.getParameter("string"); + String[] array = {"A", "B"}; + response.getWriter().format(format, array); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/format2") + @SuppressFBWarnings + public void format2(final HttpServletRequest request, final HttpServletResponse response) { + try { + String format = "Formatted like: %1$s and %2$s."; + String[] array = {"A", request.getParameter("string")}; + response.getWriter().format(format, array); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/format3") + @SuppressFBWarnings + public void format3(final HttpServletRequest request, final HttpServletResponse response) { + try { + String format = request.getParameter("string"); + String[] array = {"A", "B"}; + response.getWriter().format(Locale.getDefault(), format, array); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/format4") + @SuppressFBWarnings + public void format4(final HttpServletRequest request, final HttpServletResponse response) { + try { + String format = "Formatted like: %1$s and %2$s."; + String[] array = {"A", request.getParameter("string")}; + response.getWriter().format(Locale.getDefault(), format, array); + } catch (IOException e) { + throw new RuntimeException(e); + } + } +} From 750b9a3f517fc5cb8462eb4395599541bcc52020 Mon Sep 17 00:00:00 2001 From: Santiago Mola Date: Wed, 9 Aug 2023 09:51:07 +0200 Subject: [PATCH 024/367] Report dd-trace-java and its dependencies to telemetry While this might be sometimes confusing, we want to report the full picture accurately. --- .../dependency/LocationsCollectingTransformer.java | 5 +---- .../LocationsCollectingTransformerSpecification.groovy | 10 ---------- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/telemetry/src/main/java/datadog/telemetry/dependency/LocationsCollectingTransformer.java b/telemetry/src/main/java/datadog/telemetry/dependency/LocationsCollectingTransformer.java index b3633350b33..3d355ac0299 100644 --- a/telemetry/src/main/java/datadog/telemetry/dependency/LocationsCollectingTransformer.java +++ b/telemetry/src/main/java/datadog/telemetry/dependency/LocationsCollectingTransformer.java @@ -17,9 +17,6 @@ class LocationsCollectingTransformer implements ClassFileTransformer { private final DDCache seenDomains = DDCaches.newFixedSizeWeakKeyCache(MAX_CACHED_JARS); - private final ProtectionDomain tracerDomain = - LocationsCollectingTransformer.class.getProtectionDomain(); - public LocationsCollectingTransformer(DependencyService dependencyService) { this.dependencyService = dependencyService; } @@ -31,7 +28,7 @@ public byte[] transform( Class classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) { - if (protectionDomain != null && !protectionDomain.equals(tracerDomain)) { + if (protectionDomain != null) { seenDomains.computeIfAbsent(protectionDomain, this::addDependency); } // returning 'null' is the best way to indicate that no transformation has been done. diff --git a/telemetry/src/test/groovy/datadog/telemetry/dependency/LocationsCollectingTransformerSpecification.groovy b/telemetry/src/test/groovy/datadog/telemetry/dependency/LocationsCollectingTransformerSpecification.groovy index 1f526798e02..90134052a90 100644 --- a/telemetry/src/test/groovy/datadog/telemetry/dependency/LocationsCollectingTransformerSpecification.groovy +++ b/telemetry/src/test/groovy/datadog/telemetry/dependency/LocationsCollectingTransformerSpecification.groovy @@ -14,16 +14,6 @@ class LocationsCollectingTransformerSpecification extends DepSpecification { LocationsCollectingTransformer transformer = new LocationsCollectingTransformer(depService) - void 'no dependency for agent jar'() { - when: - transformer.transform(null, null, null, getClass().getProtectionDomain(), null) - - then: - depService.resolveOneDependency() - def dependencies = depService.drainDeterminedDependencies() - dependencies.isEmpty() - } - void 'no dependency if null protection domain'() { when: transformer.transform(null, null, null, null, null) From 18caf239437b7dc777cf236a786a87b6a1085314 Mon Sep 17 00:00:00 2001 From: Santiago Mola Date: Wed, 9 Aug 2023 17:36:50 +0200 Subject: [PATCH 025/367] Preserve pom.properties of our dependencies --- dd-java-agent/build.gradle | 5 ++++- dd-trace-ot/build.gradle | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/build.gradle b/dd-java-agent/build.gradle index 9958a0d0671..8f2d23f6ea1 100644 --- a/dd-java-agent/build.gradle +++ b/dd-java-agent/build.gradle @@ -30,7 +30,10 @@ targetCompatibility = JavaVersion.VERSION_1_7 ext.generalShadowJarConfig = { mergeServiceFiles() - exclude '**/META-INF/maven/' + // Remove some cruft from the final jar. + // These patterns should NOT include **/META-INF/maven/**/pom.properties, which is + // used to report our own dependencies. + exclude '**/META-INF/maven/**/pom.xml' exclude '**/META-INF/proguard/' exclude '**/META-INF/*.kotlin_module' exclude '**/module-info.class' diff --git a/dd-trace-ot/build.gradle b/dd-trace-ot/build.gradle index d5025e9b5c9..223b36da20a 100644 --- a/dd-trace-ot/build.gradle +++ b/dd-trace-ot/build.gradle @@ -117,7 +117,11 @@ shadowJar { exclude('datadog.trace.api.sampling.*') exclude('datadog.trace.context.*') } - exclude('META-INF/maven/') + + // Remove some cruft from the final jar. + // These patterns should NOT include **/META-INF/maven/**/pom.properties, which is + // used to report our own dependencies. + exclude '**/META-INF/maven/**/pom.xml' exclude('META-INF/proguard/') exclude('/META-INF/*.kotlin_module') } From 71a6ad382258767ea42c0c4512398e8986ad31a7 Mon Sep 17 00:00:00 2001 From: Santiago Mola Date: Thu, 17 Aug 2023 10:11:21 +0200 Subject: [PATCH 026/367] Set up shadowJar to fail on duplicate entries This will ensure we do not reintroduce JAR corruption that was observed in the past. --- dd-java-agent/build.gradle | 2 ++ dd-trace-ot/build.gradle | 2 ++ 2 files changed, 4 insertions(+) diff --git a/dd-java-agent/build.gradle b/dd-java-agent/build.gradle index 8f2d23f6ea1..d667f7908ca 100644 --- a/dd-java-agent/build.gradle +++ b/dd-java-agent/build.gradle @@ -30,6 +30,8 @@ targetCompatibility = JavaVersion.VERSION_1_7 ext.generalShadowJarConfig = { mergeServiceFiles() + duplicatesStrategy = DuplicatesStrategy.FAIL + // Remove some cruft from the final jar. // These patterns should NOT include **/META-INF/maven/**/pom.properties, which is // used to report our own dependencies. diff --git a/dd-trace-ot/build.gradle b/dd-trace-ot/build.gradle index 223b36da20a..a90bdbe4c89 100644 --- a/dd-trace-ot/build.gradle +++ b/dd-trace-ot/build.gradle @@ -118,6 +118,8 @@ shadowJar { exclude('datadog.trace.context.*') } + duplicatesStrategy = DuplicatesStrategy.FAIL + // Remove some cruft from the final jar. // These patterns should NOT include **/META-INF/maven/**/pom.properties, which is // used to report our own dependencies. From 5bb31ca076cff9c2b0ec3fc07e2073d8178e51fc Mon Sep 17 00:00:00 2001 From: ValentinZakharov Date: Thu, 17 Aug 2023 11:02:36 +0200 Subject: [PATCH 027/367] Fixed NPE in User Events Tracking (#5732) * Fixed SpringSecurity NullPointerException when user creation happens outside request. * Spotless --- .../springbootsecurity/WebSecurityConfig.java | 27 ++++++++++++++++++- .../src/main/resources/application.properties | 2 ++ .../java/datadog/trace/core/CoreTracer.java | 6 ++++- 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/dd-smoke-tests/appsec/springboot-security/src/main/java/datadog/smoketest/appsec/springbootsecurity/WebSecurityConfig.java b/dd-smoke-tests/appsec/springboot-security/src/main/java/datadog/smoketest/appsec/springbootsecurity/WebSecurityConfig.java index 20b341ee843..819244a520a 100644 --- a/dd-smoke-tests/appsec/springboot-security/src/main/java/datadog/smoketest/appsec/springbootsecurity/WebSecurityConfig.java +++ b/dd-smoke-tests/appsec/springboot-security/src/main/java/datadog/smoketest/appsec/springbootsecurity/WebSecurityConfig.java @@ -3,9 +3,14 @@ import javax.sql.DataSource; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.DependsOn; +import org.springframework.core.io.ClassPathResource; +import org.springframework.jdbc.datasource.init.DataSourceInitializer; +import org.springframework.jdbc.datasource.init.ResourceDatabasePopulator; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.config.annotation.web.configurers.LogoutConfigurer; +import org.springframework.security.core.userdetails.User; import org.springframework.security.provisioning.JdbcUserDetailsManager; import org.springframework.security.provisioning.UserDetailsManager; import org.springframework.security.web.SecurityFilterChain; @@ -36,7 +41,27 @@ public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Excepti } @Bean + @DependsOn("dataSource") + public DataSourceInitializer dataSourceInitializer() { + ResourceDatabasePopulator populator = new ResourceDatabasePopulator(); + populator.addScript(new ClassPathResource("schema.sql")); + + DataSourceInitializer initializer = new DataSourceInitializer(); + initializer.setDataSource(dataSource); + initializer.setDatabasePopulator(populator); + + return initializer; + } + + @Bean + @DependsOn("dataSourceInitializer") public UserDetailsManager userDetailsService() { - return new JdbcUserDetailsManager(dataSource); + UserDetailsManager userDetailsManager = new JdbcUserDetailsManager(dataSource); + + // Create some default for case when user creation happens outside request + userDetailsManager.createUser( + User.withUsername("default_user").password("{noop}").roles("USER").build()); + + return userDetailsManager; } } diff --git a/dd-smoke-tests/appsec/springboot-security/src/main/resources/application.properties b/dd-smoke-tests/appsec/springboot-security/src/main/resources/application.properties index 88fe6c2784b..7f052cabbea 100644 --- a/dd-smoke-tests/appsec/springboot-security/src/main/resources/application.properties +++ b/dd-smoke-tests/appsec/springboot-security/src/main/resources/application.properties @@ -1,6 +1,8 @@ spring.datasource.url=jdbc:h2:mem:authDB;DB_CLOSE_DELAY=-1;DB_CLOSE_ON_EXIT=FALSE spring.datasource.username=sa spring.datasource.password= +# DB Initialization happens in WebSecurityConfig.dataSourceInitializer() +spring.sql.init.enabled=false spring.jpa.properties.hibernate.dialect=org.hibernate.dialect.H2Dialect spring.jpa.hibernate.ddl-auto=update diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 9480a10219c..ab2d47216ed 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -1229,7 +1229,11 @@ public ProfilingContextIntegration getProfilingContext() { @Override public TraceSegment getTraceSegment() { - AgentSpan.Context ctx = activeSpan().context(); + AgentSpan activeSpan = activeSpan(); + if (activeSpan == null) { + return null; + } + AgentSpan.Context ctx = activeSpan.context(); if (ctx instanceof DDSpanContext) { return ((DDSpanContext) ctx).getTraceSegment(); } From 6cecaf87694d9ea7e6119d83b5f850670636eb84 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Thu, 17 Aug 2023 09:00:38 -0400 Subject: [PATCH 028/367] Jax-RS exception configuration option should apply to Resteasy (#5731) * Jax-RS exception configuration option should apply to Resteasy * Map.of -> Groovy Map * failedFuture -> completeExceptionally --- .../build.gradle | 4 + ...yClientConnectionErrorInstrumentation.java | 8 +- .../resteasy/WrappedFuture.java | 31 ++++---- ...ConnectionErrorsInstrumentationTest.groovy | 65 +++++++++++++++ .../src/test/groovy/WrappedFutureTest.groovy | 79 +++++++++++++++++++ 5 files changed, 172 insertions(+), 15 deletions(-) create mode 100644 dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/src/test/groovy/ResteasyClientConnectionErrorsInstrumentationTest.groovy create mode 100644 dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/src/test/groovy/WrappedFutureTest.groovy diff --git a/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/build.gradle b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/build.gradle index 3f7d770790d..063ce129aa0 100644 --- a/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/build.gradle +++ b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/build.gradle @@ -12,4 +12,8 @@ dependencies { compileOnly group: 'org.jboss.resteasy', name: 'resteasy-client', version: '3.0.0.Final' compileOnly project(':dd-java-agent:instrumentation:jax-rs-client-2.0') + + testImplementation group: 'org.jboss.resteasy', name: 'resteasy-client', version: '3.0.0.Final' + + testImplementation project(':dd-java-agent:instrumentation:jax-rs-client-2.0') } diff --git a/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/src/main/java/datadog/trace/instrumentation/connection_error/resteasy/ResteasyClientConnectionErrorInstrumentation.java b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/src/main/java/datadog/trace/instrumentation/connection_error/resteasy/ResteasyClientConnectionErrorInstrumentation.java index cdc885c610b..b8b316fd073 100644 --- a/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/src/main/java/datadog/trace/instrumentation/connection_error/resteasy/ResteasyClientConnectionErrorInstrumentation.java +++ b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/src/main/java/datadog/trace/instrumentation/connection_error/resteasy/ResteasyClientConnectionErrorInstrumentation.java @@ -7,6 +7,7 @@ import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.api.Config; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.instrumentation.jaxrs.ClientTracingFilter; import java.util.concurrent.Future; @@ -58,8 +59,13 @@ public static void handleError( final Object prop = context.getProperty(ClientTracingFilter.SPAN_PROPERTY_NAME); if (prop instanceof AgentSpan) { final AgentSpan span = (AgentSpan) prop; - span.setError(true); span.addThrowable(throwable); + + @SuppressWarnings("deprecation") + final boolean isJaxRsExceptionAsErrorEnabled = + Config.get().isJaxRsExceptionAsErrorEnabled(); + span.setError(isJaxRsExceptionAsErrorEnabled); + span.finish(); } } diff --git a/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/src/main/java/datadog/trace/instrumentation/connection_error/resteasy/WrappedFuture.java b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/src/main/java/datadog/trace/instrumentation/connection_error/resteasy/WrappedFuture.java index 066b7546cd1..9f2ea0de202 100644 --- a/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/src/main/java/datadog/trace/instrumentation/connection_error/resteasy/WrappedFuture.java +++ b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/src/main/java/datadog/trace/instrumentation/connection_error/resteasy/WrappedFuture.java @@ -1,5 +1,6 @@ package datadog.trace.instrumentation.connection_error.resteasy; +import datadog.trace.api.Config; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.instrumentation.jaxrs.ClientTracingFilter; import java.util.concurrent.ExecutionException; @@ -38,13 +39,7 @@ public T get() throws InterruptedException, ExecutionException { try { return wrapped.get(); } catch (final ExecutionException e) { - final Object prop = context.getProperty(ClientTracingFilter.SPAN_PROPERTY_NAME); - if (prop instanceof AgentSpan) { - final AgentSpan span = (AgentSpan) prop; - span.setError(true); - span.addThrowable(e.getCause()); - span.finish(); - } + handleExecutionException(e); throw e; } } @@ -55,14 +50,22 @@ public T get(final long timeout, final TimeUnit unit) try { return wrapped.get(timeout, unit); } catch (final ExecutionException e) { - final Object prop = context.getProperty(ClientTracingFilter.SPAN_PROPERTY_NAME); - if (prop instanceof AgentSpan) { - final AgentSpan span = (AgentSpan) prop; - span.setError(true); - span.addThrowable(e.getCause()); - span.finish(); - } + handleExecutionException(e); throw e; } } + + private void handleExecutionException(ExecutionException e) { + final Object prop = context.getProperty(ClientTracingFilter.SPAN_PROPERTY_NAME); + if (prop instanceof AgentSpan) { + final AgentSpan span = (AgentSpan) prop; + span.addThrowable(e.getCause()); + + @SuppressWarnings("deprecation") + final boolean isJaxRsExceptionAsErrorEnabled = Config.get().isJaxRsExceptionAsErrorEnabled(); + span.setError(isJaxRsExceptionAsErrorEnabled); + + span.finish(); + } + } } diff --git a/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/src/test/groovy/ResteasyClientConnectionErrorsInstrumentationTest.groovy b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/src/test/groovy/ResteasyClientConnectionErrorsInstrumentationTest.groovy new file mode 100644 index 00000000000..53a87922730 --- /dev/null +++ b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/src/test/groovy/ResteasyClientConnectionErrorsInstrumentationTest.groovy @@ -0,0 +1,65 @@ +import static datadog.trace.api.config.TraceInstrumentationConfig.JAX_RS_EXCEPTION_AS_ERROR_ENABLED + +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.instrumentation.connection_error.resteasy.ResteasyClientConnectionErrorInstrumentation +import datadog.trace.instrumentation.jaxrs.ClientTracingFilter +import org.jboss.resteasy.client.jaxrs.internal.ClientConfiguration +import org.jboss.resteasy.spi.ResteasyProviderFactory + +class ResteasyClientConnectionErrorsInstrumentationTest extends AgentTestRunner { + def "handleError does not generate traces for non-exceptions"() { + setup: + if (jaxRsExceptionAsErrorEnabled != null) { + injectSysConfig(JAX_RS_EXCEPTION_AS_ERROR_ENABLED, "$jaxRsExceptionAsErrorEnabled") + } + def testSpan = TEST_TRACER.buildSpan("testInstrumentation", "testSpan").start() + def props = [(ClientTracingFilter.SPAN_PROPERTY_NAME): testSpan] + + def clientConfig = new ClientConfiguration(new ResteasyProviderFactory()) + clientConfig.setProperties(props) + + when: + ResteasyClientConnectionErrorInstrumentation.InvokeAdvice.handleError(clientConfig, null) + + then: + assertTraces(0) {} + + where: + jaxRsExceptionAsErrorEnabled | isErrored + true | true + false | false + null | true + } + + def "handleError properly utilizes the config"() { + setup: + if (jaxRsExceptionAsErrorEnabled != null) { + injectSysConfig(JAX_RS_EXCEPTION_AS_ERROR_ENABLED, "$jaxRsExceptionAsErrorEnabled") + } + def testSpan = TEST_TRACER.buildSpan("testInstrumentation", "testSpan").start() + def props = [(ClientTracingFilter.SPAN_PROPERTY_NAME): testSpan] + + def clientConfig = new ClientConfiguration(new ResteasyProviderFactory()) + clientConfig.setProperties(props) + + when: + ResteasyClientConnectionErrorInstrumentation.InvokeAdvice.handleError(clientConfig, new RuntimeException("failed")) + + then: + assertTraces(1) { + trace(1) { + span { + operationName "testSpan" + resourceName "testSpan" + errored isErrored + } + } + } + + where: + jaxRsExceptionAsErrorEnabled | isErrored + true | true + false | false + null | true + } +} diff --git a/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/src/test/groovy/WrappedFutureTest.groovy b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/src/test/groovy/WrappedFutureTest.groovy new file mode 100644 index 00000000000..59f1fe1de68 --- /dev/null +++ b/dd-java-agent/instrumentation/jax-rs-client-2.0/connection-error-handling-resteasy/src/test/groovy/WrappedFutureTest.groovy @@ -0,0 +1,79 @@ +import static datadog.trace.api.config.TraceInstrumentationConfig.JAX_RS_EXCEPTION_AS_ERROR_ENABLED + +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.instrumentation.connection_error.resteasy.WrappedFuture +import datadog.trace.instrumentation.jaxrs.ClientTracingFilter +import java.util.concurrent.CompletableFuture +import org.jboss.resteasy.client.jaxrs.internal.ClientConfiguration +import org.jboss.resteasy.spi.ResteasyProviderFactory + +class WrappedFutureTest extends AgentTestRunner { + def "wrappedFuture does not generate traces for non-exceptions"() { + setup: + if (jaxRsExceptionAsErrorEnabled != null) { + injectSysConfig(JAX_RS_EXCEPTION_AS_ERROR_ENABLED, "$jaxRsExceptionAsErrorEnabled") + } + def testSpan = TEST_TRACER.buildSpan("testInstrumentation", "testSpan").start() + def props = [(ClientTracingFilter.SPAN_PROPERTY_NAME): testSpan] + + def completedFuture = CompletableFuture.completedFuture("passed") + + def clientConfig = new ClientConfiguration(new ResteasyProviderFactory()) + clientConfig.setProperties(props) + def wrappedFuture = new WrappedFuture(completedFuture, clientConfig) + + when: + try { + wrappedFuture.get() + } catch (ignored) { + } + + then: + assertTraces(0) {} + + where: + jaxRsExceptionAsErrorEnabled | isErrored + true | true + false | false + null | true + } + + def "wrappedFuture handleProcessingException properly utilizes the config"() { + setup: + if (jaxRsExceptionAsErrorEnabled != null) { + injectSysConfig(JAX_RS_EXCEPTION_AS_ERROR_ENABLED, "$jaxRsExceptionAsErrorEnabled") + } + def testSpan = TEST_TRACER.buildSpan("testInstrumentation", "testSpan").start() + def props = [(ClientTracingFilter.SPAN_PROPERTY_NAME): testSpan] + + def future = new CompletableFuture() + future.completeExceptionally(new RuntimeException("failed")) + + def clientConfig = new ClientConfiguration(new ResteasyProviderFactory()) + clientConfig.setProperties(props) + def wrappedFuture = new WrappedFuture(future, clientConfig) + + when: + try { + wrappedFuture.get() + } catch (ignored) { + } + + then: + assertTraces(1) { + trace(1) { + span { + operationName "testSpan" + resourceName "testSpan" + errored isErrored + } + } + } + + where: + jaxRsExceptionAsErrorEnabled | isErrored + true | true + false | false + null | true + } +} From 33a6218c17073dafc05c823a63001cbef6a7cb92 Mon Sep 17 00:00:00 2001 From: Santiago Mola Date: Tue, 8 Aug 2023 14:37:51 +0200 Subject: [PATCH 029/367] Use Circle CI dynamic configuration Now `.circleci/config.continue.yml.j2` is a Jinja2 template that will get various variables depending on the branch. This allows finer-grained control on which jobs we run for each type of pipeline. --- .../{config.yml => config.continue.yml.j2} | 66 +++++--------- .circleci/render_config.py | 88 +++++++++++++++++++ .gitignore | 1 + 3 files changed, 110 insertions(+), 45 deletions(-) rename .circleci/{config.yml => config.continue.yml.j2} (97%) create mode 100755 .circleci/render_config.py diff --git a/.circleci/config.yml b/.circleci/config.continue.yml.j2 similarity index 97% rename from .circleci/config.yml rename to .circleci/config.continue.yml.j2 index 8f214299972..b913dde32bc 100644 --- a/.circleci/config.yml +++ b/.circleci/config.continue.yml.j2 @@ -7,11 +7,17 @@ defaults: &defaults test_matrix: &test_matrix parameters: - testJvm: [ "ibm8", "semeru8", "zulu8", "oracle8", "11", "zulu11", "17", "ubuntu17" ] + testJvm: +{% for jdk in nocov_jdks %} + - "{{ jdk }}" +{% endfor %} profiling_test_matrix: &profiling_test_matrix parameters: - testJvm: [ "8", "zulu8", "oracle8", "11", "zulu11", "17", "ubuntu17" ] + testJvm: +{% for jdk in all_jdks %} + - "{{ jdk }}" +{% endfor %} system_test_matrix: &system_test_matrix parameters: @@ -179,6 +185,7 @@ commands: # 2) Cache keys are prefix matched, and the most recently updated cache that matches will be picked # # There is a weekly job that runs on Monday mornings that builds a new cache from scratch. + {% raw %} restore_dependency_cache: parameters: cacheType: @@ -239,6 +246,7 @@ commands: # Workspace - ~/dd-trace-java/.gradle - ~/dd-trace-java/workspace +{% endraw %} setup_system_tests: parameters: @@ -1048,6 +1056,7 @@ build_test_jobs: &build_test_jobs maxWorkers: 4 testJvm: "8" +{% if flaky %} - tests: requires: - ok_to_test @@ -1088,6 +1097,7 @@ build_test_jobs: &build_test_jobs parallelism: 4 maxWorkers: 4 testJvm: "8" +{% endif %} - tests: requires: @@ -1128,30 +1138,6 @@ build_test_jobs: &build_test_jobs matrix: <<: *test_matrix - - tests: - requires: - - ok_to_test - name: test_semeru11_smoke - gradleTarget: "stageMainDist :smokeTest" - gradleParameters: "-PskipFlakyTests" - stage: smoke - cacheType: smoke - parallelism: 4 - maxWorkers: 3 - testJvm: "semeru11" - - - tests: - requires: - - ok_to_test - name: test_semeru17_smoke - gradleTarget: "stageMainDist :smokeTest" - gradleParameters: "-PskipFlakyTests" - stage: smoke - cacheType: smoke - parallelism: 4 - maxWorkers: 3 - testJvm: "semeru17" - - tests: requires: - ok_to_test @@ -1231,24 +1217,18 @@ build_test_jobs: &build_test_jobs - fan_in: requires: - test_published_artifacts - - test_8_profiling - - test_oracle8_profiling - - test_zulu8_profiling - - test_zulu11_profiling - - test_11_profiling - - test_17_profiling +{% for jdk in all_jdks %} + - "test_{{ jdk }}_profiling" +{% endfor %} name: profiling stage: profiling - fan_in: requires: - test_published_artifacts - - test_8_debugger - - test_oracle8_debugger - - test_zulu8_debugger - - test_zulu11_debugger - - test_11_debugger - - test_17_debugger +{% for jdk in all_jdks %} + - "test_{{ jdk }}_debugger" +{% endfor %} name: debugger stage: debugger @@ -1259,13 +1239,9 @@ build_test_jobs: &build_test_jobs - check - test_published_artifacts - agent_integration_tests - - test_8 - - test_ibm8 - - test_11 - - test_semeru11_smoke - - test_17 - - test_semeru17_smoke - - test_zulu8 +{% for jdk in all_jdks %} + - "test_{{ jdk }}" +{% endfor %} - profiling - debugger name: required diff --git a/.circleci/render_config.py b/.circleci/render_config.py new file mode 100755 index 00000000000..d485680b818 --- /dev/null +++ b/.circleci/render_config.py @@ -0,0 +1,88 @@ +#!/usr/bin/env python3 + +import os +import os.path +import time + +import jinja2 +import requests + +SCRIPT_DIR = os.path.dirname(__file__) + +TPL_FILENAME = "config.continue.yml.j2" +OUT_FILENAME = "config.continue.yml" +GENERATED_CONFIG_PATH = os.path.join(SCRIPT_DIR, OUT_FILENAME) + +# JDKs that will run on every pipeline. +ALWAYS_ON_JDKS = {"8", "11", "17"} +# And these will run only in master and release/ branches. +MASTER_ONLY_JDKS = { + "ibm8", + "oracle8", + "semeru8", + "zulu8", + "semeru11", + "zulu11", + "semeru17", + "ubuntu17", +} + +# Get labels from pull requests to override some defaults for jobs to run. +# `run-tests: all` will run all tests. +# `run-tests: ibm8` will run the IBM 8 tests. +# `run-tests: flaky` for flaky tests jobs. +pr_url = os.environ.get("CIRCLE_PULL_REQUEST") +if pr_url: + pr_num = int(pr_url.split("/")[-1]) + headers = {} + gh_token = os.environ.get("GH_TOKEN") + if gh_token: + headers["Authorization"] = gh_token + else: + print("Missing GH_TOKEN, trying anonymously") + for _ in range(20): + try: + resp = requests.get( + f"https://api.github.com/repos/DataDog/dd-trace-java/pulls/{pr_num}", + timeout=1, + headers=headers, + ) + resp.raise_for_status() + except Exception as e: + print(f"Request filed: {e}") + time.sleep(1) + continue + data = resp.json() + break + + labels = data.get("labels", []) + labels = [l["name"] for l in labels] + labels = { + l.replace("run-tests: ", "") for l in labels if l.startswith("run-tests: ") + } +else: + labels = set() + + +branch = os.environ.get("CIRCLE_BRANCH", "") +if branch == "master" or branch.startswith("release/v") or "all" in labels: + all_jdks = ALWAYS_ON_JDKS | MASTER_ONLY_JDKS +else: + all_jdks = ALWAYS_ON_JDKS | (MASTER_ONLY_JDKS & labels) +nocov_jdks = [j for j in all_jdks if j != "8"] + +vars = { + "all_jdks": all_jdks, + "nocov_jdks": nocov_jdks, + "flaky": branch == "master" or "flaky" in labels or "all" in labels, +} + +print(f"Variables for this build: {vars}") + +loader = jinja2.FileSystemLoader(searchpath=SCRIPT_DIR) +env = jinja2.Environment(loader=loader) +tpl = env.get_template(TPL_FILENAME) +out = tpl.render(**vars) + +with open(GENERATED_CONFIG_PATH, "w", encoding="utf-8") as f: + f.write(out) diff --git a/.gitignore b/.gitignore index 2e975650fca..fcedfdbdb4d 100644 --- a/.gitignore +++ b/.gitignore @@ -68,6 +68,7 @@ replay_pid* ############ _circle_ci_cache_* upstream.env +/.circleci/config.continue.yml # Benchmarks # benchmark/reports From 0d6a7babddf29a5e5b51dd5a0d68c79f3a88e7b1 Mon Sep 17 00:00:00 2001 From: Santiago Mola Date: Wed, 16 Aug 2023 16:53:50 +0200 Subject: [PATCH 030/367] Add the new .circleci/config.yml Added in a separate commit to facilitate some git diff'ing of the now renamed configuration. --- .circleci/config.yml | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 .circleci/config.yml diff --git a/.circleci/config.yml b/.circleci/config.yml new file mode 100644 index 00000000000..7a8d0472a79 --- /dev/null +++ b/.circleci/config.yml @@ -0,0 +1,27 @@ +version: 2.1 +setup: true +python310_image: &python310_image cimg/python:3.10 +orbs: + continuation: circleci/continuation@0.1.2 +executors: + python310: + docker: + - image: *python310_image + resource_class: small +jobs: + setup: + executor: python310 + steps: + - checkout + - run: + name: Install dependencies + command: pip3 install jinja2 requests + - run: + name: Generate config + command: .circleci/render_config.py + - continuation/continue: + configuration_path: .circleci/config.continue.yml +workflows: + setup: + jobs: + - setup From 1af4d4ef4a8fe19d2f8cbebcbd39cc1792934a45 Mon Sep 17 00:00:00 2001 From: Santiago Mola Date: Fri, 11 Aug 2023 19:33:10 +0200 Subject: [PATCH 031/367] tweak test conditions to exclude Semeru --- .../datadog/trace/civisibility/ipc/ChannelContextTest.groovy | 5 +++-- .../debugger/agent/LogMessageTemplateBuilderTest.java | 2 ++ .../datadog/debugger/agent/SnapshotSerializationTest.java | 2 ++ .../agent-profiling/profiling-controller-ddprof/build.gradle | 1 + .../agent-profiling/profiling-controller-jfr/build.gradle | 2 ++ .../profiling-controller-openjdk/build.gradle | 1 + .../groovy/datadog/trace/agent/tooling/WeakMapTest.groovy | 4 +++- .../trace/agent/tooling/muzzle/ReferenceMatcherTest.groovy | 2 ++ .../instrumentation/java-directbytebuffer/build.gradle | 1 + .../java/lang/invoke/StringConcatFactoryCallSiteTest.groovy | 5 +++++ dd-java-agent/instrumentation/spark/build.gradle | 3 --- .../src/test/groovy/SparkStructuredStreamingTest.groovy | 5 +++++ .../instrumentation/spark/src/test/groovy/SparkTest.groovy | 5 +++++ 13 files changed, 32 insertions(+), 6 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ipc/ChannelContextTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ipc/ChannelContextTest.groovy index 0f5d508380a..5281d6abae7 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ipc/ChannelContextTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ipc/ChannelContextTest.groovy @@ -1,5 +1,6 @@ package datadog.trace.civisibility.ipc +import datadog.trace.api.Platform import spock.lang.IgnoreIf import spock.lang.Specification @@ -7,8 +8,8 @@ import java.nio.ByteBuffer import java.nio.channels.ByteChannel import java.util.concurrent.ThreadLocalRandom -@IgnoreIf(reason = "JVM crash with IBM JDK", value = { - System.getProperty("java.vendor").contains("IBM") && System.getProperty("java.version").contains("1.8.") +@IgnoreIf(reason = "JVM crash with OpenJ9", value = { + Platform.isJ9() }) class ChannelContextTest extends Specification { diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/LogMessageTemplateBuilderTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/LogMessageTemplateBuilderTest.java index ed26ece891f..cf5a1f75458 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/LogMessageTemplateBuilderTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/LogMessageTemplateBuilderTest.java @@ -16,6 +16,7 @@ import java.util.List; import java.util.Map; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledIf; import org.junit.jupiter.api.condition.EnabledOnJre; import org.junit.jupiter.api.condition.JRE; @@ -203,6 +204,7 @@ public void argComplexObjectArrayTemplate() { @Test @EnabledOnJre(JRE.JAVA_17) + @DisabledIf("datadog.trace.api.Platform#isJ9") public void argInaccessibleFieldTemplate() { LogProbe probe = createLogProbe("{obj}"); LogMessageTemplateBuilder summaryBuilder = new LogMessageTemplateBuilder(probe.getSegments()); diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/SnapshotSerializationTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/SnapshotSerializationTest.java index 7492d3c75a3..344b6de3390 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/SnapshotSerializationTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/SnapshotSerializationTest.java @@ -61,6 +61,7 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledIf; import org.junit.jupiter.api.condition.EnabledOnJre; import org.junit.jupiter.api.condition.JRE; @@ -96,6 +97,7 @@ public void roundTripProbeDetails() throws IOException { @Test @EnabledOnJre(JRE.JAVA_17) + @DisabledIf("datadog.trace.api.Platform#isJ9") public void roundTripCapturedValue() throws IOException, URISyntaxException { JsonAdapter adapter = createSnapshotAdapter(); Snapshot snapshot = createSnapshot(); diff --git a/dd-java-agent/agent-profiling/profiling-controller-ddprof/build.gradle b/dd-java-agent/agent-profiling/profiling-controller-ddprof/build.gradle index 49302e0ad0c..855c5b68bca 100644 --- a/dd-java-agent/agent-profiling/profiling-controller-ddprof/build.gradle +++ b/dd-java-agent/agent-profiling/profiling-controller-ddprof/build.gradle @@ -6,6 +6,7 @@ ext { skipSettingTestJavaVersion = true // need access to jdk.jfr package skipSettingCompilerRelease = true + excludeJdk = ['SEMERU11', 'SEMERU17'] } apply from: "$rootDir/gradle/java.gradle" diff --git a/dd-java-agent/agent-profiling/profiling-controller-jfr/build.gradle b/dd-java-agent/agent-profiling/profiling-controller-jfr/build.gradle index 36047c0e6f0..5dc47d1c0f6 100644 --- a/dd-java-agent/agent-profiling/profiling-controller-jfr/build.gradle +++ b/dd-java-agent/agent-profiling/profiling-controller-jfr/build.gradle @@ -6,6 +6,8 @@ ext { // need access to jdk.jfr package skipSettingCompilerRelease = true + + excludeJdk = ['SEMERU11', 'SEMERU17'] } apply from: "$rootDir/gradle/java.gradle" diff --git a/dd-java-agent/agent-profiling/profiling-controller-openjdk/build.gradle b/dd-java-agent/agent-profiling/profiling-controller-openjdk/build.gradle index facdd7fb8c7..5de37b3baaa 100644 --- a/dd-java-agent/agent-profiling/profiling-controller-openjdk/build.gradle +++ b/dd-java-agent/agent-profiling/profiling-controller-openjdk/build.gradle @@ -3,6 +3,7 @@ ext { minJavaVersionForTests = JavaVersion.VERSION_11 // Zulu has backported profiling support forceJdk = ['ZULU8'] + excludeJdk = ['SEMERU11', 'SEMERU17'] // By default tests with be compiled for `minJavaVersionForTests` version, // but in this case we would like to avoid this since we would like to run with ZULU8 skipSettingTestJavaVersion = true diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/WeakMapTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/WeakMapTest.groovy index 7c1f8eea0d4..e501161c6fe 100644 --- a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/WeakMapTest.groovy +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/WeakMapTest.groovy @@ -1,8 +1,8 @@ package datadog.trace.agent.tooling -import datadog.trace.test.util.Flaky import datadog.trace.test.util.GCUtils import datadog.trace.test.util.DDSpecification +import spock.lang.IgnoreIf import java.lang.ref.WeakReference import java.util.concurrent.TimeUnit @@ -36,6 +36,7 @@ class WeakMapTest extends DDSpecification { } //@Flaky("awaitGC usage is flaky") + @IgnoreIf(reason="Often fails in Semeru runtime", value = { System.getProperty("java.runtime.name").contains("Semeru") }) def "Unreferenced map gets cleaned up"() { setup: def map = WeakMaps.newWeakMap() @@ -51,6 +52,7 @@ class WeakMapTest extends DDSpecification { } //@Flaky("awaitGC usage is flaky") + @IgnoreIf(reason="Often fails in Semeru runtime", value = { System.getProperty("java.runtime.name").contains("Semeru") }) def "Unreferenced keys get cleaned up"() { setup: def key = new Object() diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/muzzle/ReferenceMatcherTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/muzzle/ReferenceMatcherTest.groovy index d04ec546e83..cdfcd656e51 100644 --- a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/muzzle/ReferenceMatcherTest.groovy +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/muzzle/ReferenceMatcherTest.groovy @@ -7,6 +7,7 @@ import datadog.trace.agent.tooling.muzzle.TestAdviceClasses.MethodBodyAdvice import datadog.trace.test.util.DDSpecification import net.bytebuddy.jar.asm.Type import net.bytebuddy.pool.TypePool +import spock.lang.IgnoreIf import spock.lang.Shared import static datadog.trace.agent.tooling.muzzle.Reference.EXPECTS_INTERFACE @@ -60,6 +61,7 @@ class ReferenceMatcherTest extends DDSpecification { getMismatchClassSet(refMatcher.getMismatchedReferenceSources(unsafeClasspath)) == new HashSet<>([MissingClass]) } + @IgnoreIf(reason="Often fails in Semeru runtime", value = { System.getProperty("java.runtime.name").contains("Semeru") }) def "matching does not hold a strong reference to classloaders"() { expect: MuzzleWeakReferenceTest.classLoaderRefIsGarbageCollected() diff --git a/dd-java-agent/instrumentation/java-directbytebuffer/build.gradle b/dd-java-agent/instrumentation/java-directbytebuffer/build.gradle index 5a6fe5e869e..453c95e476c 100644 --- a/dd-java-agent/instrumentation/java-directbytebuffer/build.gradle +++ b/dd-java-agent/instrumentation/java-directbytebuffer/build.gradle @@ -1,5 +1,6 @@ ext { minJavaVersionForTests = JavaVersion.VERSION_11 + excludeJdk = ['SEMERU11', 'SEMERU17'] } muzzle { pass { diff --git a/dd-java-agent/instrumentation/java-lang/java-lang-9/src/test/groovy/datadog/trace/instrumentation/java/lang/invoke/StringConcatFactoryCallSiteTest.groovy b/dd-java-agent/instrumentation/java-lang/java-lang-9/src/test/groovy/datadog/trace/instrumentation/java/lang/invoke/StringConcatFactoryCallSiteTest.groovy index 5d444723d3f..a5b6d6ed901 100644 --- a/dd-java-agent/instrumentation/java-lang/java-lang-9/src/test/groovy/datadog/trace/instrumentation/java/lang/invoke/StringConcatFactoryCallSiteTest.groovy +++ b/dd-java-agent/instrumentation/java-lang/java-lang-9/src/test/groovy/datadog/trace/instrumentation/java/lang/invoke/StringConcatFactoryCallSiteTest.groovy @@ -1,10 +1,12 @@ package datadog.trace.instrumentation.java.lang.invoke import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.Platform import datadog.trace.api.iast.InstrumentationBridge import datadog.trace.api.iast.propagation.StringModule import foo.bar.TestStringConcatFactorySuite import groovy.transform.CompileDynamic +import spock.lang.IgnoreIf import spock.lang.Requires import static foo.bar.TestStringConcatFactorySuite.stringPlusWithPrimitive @@ -12,6 +14,9 @@ import static foo.bar.TestStringConcatFactorySuite.stringPlusWithPrimitive @Requires({ jvm.java9Compatible }) +@IgnoreIf(reason = "https://github.com/DataDog/dd-trace-java/issues/5715", value = { + Platform.isJ9() +}) @CompileDynamic class StringConcatFactoryCallSiteTest extends AgentTestRunner { diff --git a/dd-java-agent/instrumentation/spark/build.gradle b/dd-java-agent/instrumentation/spark/build.gradle index 4eeee5f2f36..5ae9f1c339b 100644 --- a/dd-java-agent/instrumentation/spark/build.gradle +++ b/dd-java-agent/instrumentation/spark/build.gradle @@ -11,9 +11,6 @@ apply from: "$rootDir/gradle/java.gradle" addTestSuiteForDir('latestDepTest', 'test') ext { - // Hadoop does not behave correctly with OpenJ9 https://issues.apache.org/jira/browse/HADOOP-18174 - excludeJdk = ['SEMERU8'] - // Spark does not support Java > 11 until 3.3.0 https://issues.apache.org/jira/browse/SPARK-33772 maxJavaVersionForTests = JavaVersion.VERSION_11 } diff --git a/dd-java-agent/instrumentation/spark/src/test/groovy/SparkStructuredStreamingTest.groovy b/dd-java-agent/instrumentation/spark/src/test/groovy/SparkStructuredStreamingTest.groovy index 81178a69a2a..6161816b375 100644 --- a/dd-java-agent/instrumentation/spark/src/test/groovy/SparkStructuredStreamingTest.groovy +++ b/dd-java-agent/instrumentation/spark/src/test/groovy/SparkStructuredStreamingTest.groovy @@ -1,11 +1,16 @@ import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.Platform import org.apache.spark.sql.Encoders import org.apache.spark.sql.execution.streaming.MemoryStream import org.apache.spark.sql.SparkSession import scala.Option import scala.collection.JavaConverters import scala.collection.immutable.Seq +import spock.lang.IgnoreIf +@IgnoreIf(reason="https://issues.apache.org/jira/browse/HADOOP-18174", value = { + Platform.isJ9() +}) class SparkStructuredStreamingTest extends AgentTestRunner { @Override diff --git a/dd-java-agent/instrumentation/spark/src/test/groovy/SparkTest.groovy b/dd-java-agent/instrumentation/spark/src/test/groovy/SparkTest.groovy index 12ec1b4134e..f7cfa395a45 100644 --- a/dd-java-agent/instrumentation/spark/src/test/groovy/SparkTest.groovy +++ b/dd-java-agent/instrumentation/spark/src/test/groovy/SparkTest.groovy @@ -1,6 +1,7 @@ import datadog.trace.agent.test.AgentTestRunner import datadog.trace.api.DDSpanId import datadog.trace.api.DDTraceId +import datadog.trace.api.Platform import datadog.trace.instrumentation.spark.DatabricksParentContext import datadog.trace.instrumentation.spark.DatadogSparkListener import datadog.trace.test.util.Flaky @@ -11,7 +12,11 @@ import org.apache.spark.deploy.yarn.ApplicationMaster import org.apache.spark.deploy.yarn.ApplicationMasterArguments import org.apache.spark.sql.SparkSession import scala.reflect.ClassTag$ +import spock.lang.IgnoreIf +@IgnoreIf(reason="https://issues.apache.org/jira/browse/HADOOP-18174", value = { + Platform.isJ9() +}) class SparkTest extends AgentTestRunner { @Override From 686943ecc43eaa6ae267601c75443be0a73387df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Gonz=C3=A1lez=20Garc=C3=ADa?= Date: Fri, 18 Aug 2023 12:49:03 +0200 Subject: [PATCH 032/367] Add freemarker.template.utility.StringUtil tainting support (#5645) Add tainting support to: freemarker.template.utility.StringUtil#HTMLEnc freemarker.template.utility.StringUtil#XMLEnc freemarker.template.utility.StringUtil#XHTMLEnc freemarker.template.utility.StringUtil#javaStringEnc freemarker.template.utility.StringUtil#javaScriptStringEnc freemarker.template.utility.StringUtil#jsonStringEnc --- .../instrumentation/freemarker/build.gradle | 23 +++++++ .../freemarker/StringUtilCallSite.java | 39 ++++++++++++ .../freemarker/StringUtilCallSiteTest.groovy | 62 +++++++++++++++++++ .../java/foo/bar/TestStringUtilSuite.java | 30 +++++++++ settings.gradle | 1 + 5 files changed, 155 insertions(+) create mode 100644 dd-java-agent/instrumentation/freemarker/build.gradle create mode 100644 dd-java-agent/instrumentation/freemarker/src/main/java/datadog/trace/instrumentation/freemarker/StringUtilCallSite.java create mode 100644 dd-java-agent/instrumentation/freemarker/src/test/groovy/datadog/trace/instrumentation/freemarker/StringUtilCallSiteTest.groovy create mode 100644 dd-java-agent/instrumentation/freemarker/src/test/java/foo/bar/TestStringUtilSuite.java diff --git a/dd-java-agent/instrumentation/freemarker/build.gradle b/dd-java-agent/instrumentation/freemarker/build.gradle new file mode 100644 index 00000000000..f2d5ef612aa --- /dev/null +++ b/dd-java-agent/instrumentation/freemarker/build.gradle @@ -0,0 +1,23 @@ +muzzle { + pass { + group = 'org.freemarker' + module = 'freemarker' + versions = '[2.3.32,]' + assertInverse = true + } +} + +apply from: "$rootDir/gradle/java.gradle" +apply plugin: 'call-site-instrumentation' + +addTestSuiteForDir('latestDepTest', 'test') + +dependencies { + compileOnly group: 'org.freemarker', name: 'freemarker', version: '2.3.32' + + testImplementation group: 'org.freemarker', name: 'freemarker', version: '2.3.32' + + testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter') + + latestDepTestImplementation group: 'org.freemarker', name: 'freemarker', version: '+' +} diff --git a/dd-java-agent/instrumentation/freemarker/src/main/java/datadog/trace/instrumentation/freemarker/StringUtilCallSite.java b/dd-java-agent/instrumentation/freemarker/src/main/java/datadog/trace/instrumentation/freemarker/StringUtilCallSite.java new file mode 100644 index 00000000000..8ee9bc4ea2a --- /dev/null +++ b/dd-java-agent/instrumentation/freemarker/src/main/java/datadog/trace/instrumentation/freemarker/StringUtilCallSite.java @@ -0,0 +1,39 @@ +package datadog.trace.instrumentation.freemarker; + +import datadog.trace.agent.tooling.csi.CallSite; +import datadog.trace.api.iast.IastCallSites; +import datadog.trace.api.iast.InstrumentationBridge; +import datadog.trace.api.iast.Propagation; +import datadog.trace.api.iast.VulnerabilityMarks; +import datadog.trace.api.iast.propagation.PropagationModule; +import javax.annotation.Nullable; + +@Propagation +@CallSite(spi = IastCallSites.class) +public class StringUtilCallSite { + + @CallSite.After( + "java.lang.String freemarker.template.utility.StringUtil.HTMLEnc(java.lang.String)") + @CallSite.After( + "java.lang.String freemarker.template.utility.StringUtil.XMLEnc(java.lang.String)") + @CallSite.After( + "java.lang.String freemarker.template.utility.StringUtil.XHTMLEnc(java.lang.String)") + @CallSite.After( + "java.lang.String freemarker.template.utility.StringUtil.javaStringEnc(java.lang.String)") + @CallSite.After( + "java.lang.String freemarker.template.utility.StringUtil.javaScriptStringEnc(java.lang.String)") + @CallSite.After( + "java.lang.String freemarker.template.utility.StringUtil.jsonStringEnc(java.lang.String)") + public static String afterEscape( + @CallSite.Argument(0) @Nullable final String input, @CallSite.Return final String result) { + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module != null) { + try { + module.taintIfInputIsTaintedWithMarks(result, input, VulnerabilityMarks.XSS_MARK); + } catch (final Throwable e) { + module.onUnexpectedException("afterEscape threw", e); + } + } + return result; + } +} diff --git a/dd-java-agent/instrumentation/freemarker/src/test/groovy/datadog/trace/instrumentation/freemarker/StringUtilCallSiteTest.groovy b/dd-java-agent/instrumentation/freemarker/src/test/groovy/datadog/trace/instrumentation/freemarker/StringUtilCallSiteTest.groovy new file mode 100644 index 00000000000..51b0512a5e3 --- /dev/null +++ b/dd-java-agent/instrumentation/freemarker/src/test/groovy/datadog/trace/instrumentation/freemarker/StringUtilCallSiteTest.groovy @@ -0,0 +1,62 @@ +package datadog.trace.instrumentation.freemarker + +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.iast.InstrumentationBridge +import datadog.trace.api.iast.VulnerabilityMarks +import datadog.trace.api.iast.propagation.PropagationModule +import foo.bar.TestStringUtilSuite + +class StringUtilCallSiteTest extends AgentTestRunner { + + @Override + protected void configurePreAgent() { + injectSysConfig("dd.iast.enabled", "true") + } + + void 'test #method'() { + given: + final module = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(module) + + when: + final result = TestStringUtilSuite.&"$method".call(args) + + then: + result == expected + 1 * module.taintIfInputIsTaintedWithMarks(_ as String, args[0], VulnerabilityMarks.XSS_MARK) + 0 * _ + + where: + method | args | expected + 'HTMLEnc' | ['"escape this < '] | '<htmlTag>"escape this < </htmlTag>' + 'XMLEnc' | ['"escape this < '] | '<xmlTag>"escape this < </xmlTag>' + 'XHTMLEnc' | ['"escape this < '] | '<htmlTag>"escape this < </htmlTag>' + 'javaStringEnc' | ['