From 6ce057dede0b8938213369f0fcb19f8531b8421f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Vidal=20Dom=C3=ADnguez?= Date: Mon, 30 Sep 2024 12:14:34 +0200 Subject: [PATCH] Clean instrumentation --- .../instrumentation/iast/NamedContext.java | 10 ---- .../agent-jmxfetch/integrations-core | 2 +- .../iastinstrumenter/iast_exclusion.trie | 2 - .../jackson/core/json/Json2ParserHelper.java | 11 ---- .../sym/ByteQuadsCanonicalizerHelper.java | 53 ------------------- .../core/Json2ParserInstrumentation.java | 15 ------ .../Json2ParserInstrumentationTest.groovy | 27 +--------- .../iast/KafkaIastDeserializerTest.groovy | 4 ++ .../KafkaIastDeserializerForkedTest.groovy | 4 ++ .../server/IastWebFluxTest.groovy | 8 +++ .../server/IastWebFluxTest.groovy | 8 +++ .../AbstractIastVertxSmokeTest.groovy | 3 ++ 12 files changed, 29 insertions(+), 118 deletions(-) delete mode 100644 dd-java-agent/instrumentation/jackson-core/src/main/java/com/fasterxml/jackson/core/json/Json2ParserHelper.java delete mode 100644 dd-java-agent/instrumentation/jackson-core/src/main/java/com/fasterxml/jackson/core/sym/ByteQuadsCanonicalizerHelper.java diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/iast/NamedContext.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/iast/NamedContext.java index 9132b703bf3..a78c0591f50 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/iast/NamedContext.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/iast/NamedContext.java @@ -19,8 +19,6 @@ public abstract class NamedContext { public abstract void taintName(@Nullable String name); - public abstract void setCurrentName(@Nullable final String name); - @Nonnull public static NamedContext getOrCreate( @Nonnull final ContextStore store, @Nonnull final E target) { @@ -49,9 +47,6 @@ public void taintValue(@Nullable final String value) {} @Override public void taintName(@Nullable final String name) {} - - @Override - public void setCurrentName(@Nullable final String name) {} } private static class NamedContextImpl extends NamedContext { @@ -83,11 +78,6 @@ public void taintName(@Nullable final String name) { } } - @Override - public void setCurrentName(@Nullable final String name) { - currentName = name; - } - private IastContext iastCtx() { if (!fetched) { fetched = true; diff --git a/dd-java-agent/agent-jmxfetch/integrations-core b/dd-java-agent/agent-jmxfetch/integrations-core index 5240f2a7cdc..45e2bed15b1 160000 --- a/dd-java-agent/agent-jmxfetch/integrations-core +++ b/dd-java-agent/agent-jmxfetch/integrations-core @@ -1 +1 @@ -Subproject commit 5240f2a7cdcabc6ae7787b9191b9189438671f3e +Subproject commit 45e2bed15b147a28b2c3994497945c6046c6025f diff --git a/dd-java-agent/instrumentation/iast-instrumenter/src/main/resources/datadog/trace/instrumentation/iastinstrumenter/iast_exclusion.trie b/dd-java-agent/instrumentation/iast-instrumenter/src/main/resources/datadog/trace/instrumentation/iastinstrumenter/iast_exclusion.trie index f3a05cf60ab..7d7d95269b1 100644 --- a/dd-java-agent/instrumentation/iast-instrumenter/src/main/resources/datadog/trace/instrumentation/iastinstrumenter/iast_exclusion.trie +++ b/dd-java-agent/instrumentation/iast-instrumenter/src/main/resources/datadog/trace/instrumentation/iastinstrumenter/iast_exclusion.trie @@ -125,8 +125,6 @@ 1 graphql.* 1 ibm.security.* 1 io.dropwizard.* -2 io.ebean.* -2 io.ebeaninternal.* 1 io.github.lukehutch.fastclasspathscanner.* 1 io.grpc.* 1 io.leangen.geantyref.* diff --git a/dd-java-agent/instrumentation/jackson-core/src/main/java/com/fasterxml/jackson/core/json/Json2ParserHelper.java b/dd-java-agent/instrumentation/jackson-core/src/main/java/com/fasterxml/jackson/core/json/Json2ParserHelper.java deleted file mode 100644 index b9c9d05abb4..00000000000 --- a/dd-java-agent/instrumentation/jackson-core/src/main/java/com/fasterxml/jackson/core/json/Json2ParserHelper.java +++ /dev/null @@ -1,11 +0,0 @@ -package com.fasterxml.jackson.core.json; - -import com.fasterxml.jackson.core.sym.ByteQuadsCanonicalizerHelper; - -public final class Json2ParserHelper { - private Json2ParserHelper() {} - - public static boolean fetchInterned(UTF8StreamJsonParser jsonParser) { - return ByteQuadsCanonicalizerHelper.fetchInterned(jsonParser._symbols); - } -} diff --git a/dd-java-agent/instrumentation/jackson-core/src/main/java/com/fasterxml/jackson/core/sym/ByteQuadsCanonicalizerHelper.java b/dd-java-agent/instrumentation/jackson-core/src/main/java/com/fasterxml/jackson/core/sym/ByteQuadsCanonicalizerHelper.java deleted file mode 100644 index e972809f1bc..00000000000 --- a/dd-java-agent/instrumentation/jackson-core/src/main/java/com/fasterxml/jackson/core/sym/ByteQuadsCanonicalizerHelper.java +++ /dev/null @@ -1,53 +0,0 @@ -package com.fasterxml.jackson.core.sym; - -import java.lang.reflect.Field; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -public final class ByteQuadsCanonicalizerHelper { - private ByteQuadsCanonicalizerHelper() {} - - private static final Logger log = LoggerFactory.getLogger(ByteQuadsCanonicalizerHelper.class); - - private static final Field INTERN = prepareIntern(); - private static final Field INTERNER = prepareInterner(); - - private static Field prepareIntern() { - try { - return ByteQuadsCanonicalizer.class.getDeclaredField("_intern"); - } catch (Throwable e) { - log.debug("Failed to get ByteQuadsCanonicalizer _intern field", e); - return null; - } - } - - private static Field prepareInterner() { - try { - return ByteQuadsCanonicalizer.class.getDeclaredField("_interner"); - } catch (Throwable e) { - log.debug("Failed to get ByteQuadsCanonicalizer _interner field", e); - return null; - } - } - - public static boolean fetchInterned(ByteQuadsCanonicalizer symbols) { - if (INTERN != null) { - try { - return (boolean) INTERN.get(symbols); - } catch (IllegalAccessException e) { - log.debug("Failed fetching ByteQuadsCanonicalizer _intern field", e); - return false; - } - } else { - if (INTERNER == null) { - return false; - } - try { - return INTERNER.get(symbols) != null; - } catch (IllegalAccessException e) { - log.debug("Failed fetching ByteQuadsCanonicalizer _interner field", e); - return false; - } - } - } -} diff --git a/dd-java-agent/instrumentation/jackson-core/src/main/java/datadog/trace/instrumentation/jackson/core/Json2ParserInstrumentation.java b/dd-java-agent/instrumentation/jackson-core/src/main/java/datadog/trace/instrumentation/jackson/core/Json2ParserInstrumentation.java index 106a8503393..713128d7302 100644 --- a/dd-java-agent/instrumentation/jackson-core/src/main/java/datadog/trace/instrumentation/jackson/core/Json2ParserInstrumentation.java +++ b/dd-java-agent/instrumentation/jackson-core/src/main/java/datadog/trace/instrumentation/jackson/core/Json2ParserInstrumentation.java @@ -10,8 +10,6 @@ import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.core.JsonToken; -import com.fasterxml.jackson.core.json.Json2ParserHelper; -import com.fasterxml.jackson.core.json.UTF8StreamJsonParser; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; @@ -70,14 +68,6 @@ public Map contextStore() { return singletonMap(TARGET_TYPE, "datadog.trace.bootstrap.instrumentation.iast.NamedContext"); } - @Override - public String[] helperClassNames() { - return new String[] { - "com.fasterxml.jackson.core.json" + ".Json2ParserHelper", - "com.fasterxml.jackson.core.sym" + ".ByteQuadsCanonicalizerHelper", - }; - } - public static class TextAdvice { @Advice.OnMethodExit(suppress = Throwable.class) @@ -115,11 +105,6 @@ public static void onExit(@Advice.This JsonParser jsonParser, @Advice.Return Str final ContextStore store = InstrumentationContext.get(JsonParser.class, NamedContext.class); final NamedContext context = NamedContext.getOrCreate(store, jsonParser); - if (jsonParser instanceof UTF8StreamJsonParser - && Json2ParserHelper.fetchInterned((UTF8StreamJsonParser) jsonParser)) { - context.setCurrentName(result); - return; - } context.taintName(result); } } diff --git a/dd-java-agent/instrumentation/jackson-core/src/test/groovy/Json2ParserInstrumentationTest.groovy b/dd-java-agent/instrumentation/jackson-core/src/test/groovy/Json2ParserInstrumentationTest.groovy index 17f8480ab23..52027c39a75 100644 --- a/dd-java-agent/instrumentation/jackson-core/src/test/groovy/Json2ParserInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/jackson-core/src/test/groovy/Json2ParserInstrumentationTest.groovy @@ -42,32 +42,7 @@ class Json2ParserInstrumentationTest extends AgentTestRunner { 0 * _ where: - target << [JSON_STRING] - } - - void 'test json parsing (tainted but field names)'() { - given: - final source = new SourceImpl(origin: SourceTypes.REQUEST_BODY, name: 'body', value: JSON_STRING) - final module = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(module) - - and: - final reader = new ObjectMapper().readerFor(Map) - - when: - final taintedResult = reader.readValue(target) as Map - - then: - JsonOutput.toJson(taintedResult) == JSON_STRING - _ * module.taintObjectIfTainted(_, _) - _ * module.findSource(_) >> source - 1 * module.taintString(_, 'root_value', source.origin, 'root', JSON_STRING) - 1 * module.taintString(_, 'array_0', source.origin, 'nested_array', JSON_STRING) - 1 * module.taintString(_, 'array_1', source.origin, 'nested_array', JSON_STRING) - 0 * _ - - where: - target << [new ByteArrayInputStream(JSON_STRING.getBytes(Charset.defaultCharset()))] + target << testSuite() } void 'test json parsing (not tainted)'() { diff --git a/dd-java-agent/instrumentation/kafka-clients-0.11/src/iastLatestDepTest3/groovy/iast/KafkaIastDeserializerTest.groovy b/dd-java-agent/instrumentation/kafka-clients-0.11/src/iastLatestDepTest3/groovy/iast/KafkaIastDeserializerTest.groovy index d92d70ecd55..418f50d7043 100644 --- a/dd-java-agent/instrumentation/kafka-clients-0.11/src/iastLatestDepTest3/groovy/iast/KafkaIastDeserializerTest.groovy +++ b/dd-java-agent/instrumentation/kafka-clients-0.11/src/iastLatestDepTest3/groovy/iast/KafkaIastDeserializerTest.groovy @@ -124,6 +124,10 @@ class KafkaIastDeserializerTest extends IastAgentTestRunner { value(instanceOf(TestBean)) range(0, Integer.MAX_VALUE, source(origin as byte)) } + to.hasTaintedObject { + value('name') + range(0, 4, source(origin as byte, 'name', 'name')) + } to.hasTaintedObject { value('Mr Bean') range(0, 7, source(origin as byte, 'name', 'Mr Bean')) diff --git a/dd-java-agent/instrumentation/kafka-clients-0.11/src/test/groovy/iast/KafkaIastDeserializerForkedTest.groovy b/dd-java-agent/instrumentation/kafka-clients-0.11/src/test/groovy/iast/KafkaIastDeserializerForkedTest.groovy index fefca9feee0..6e20dd8142c 100644 --- a/dd-java-agent/instrumentation/kafka-clients-0.11/src/test/groovy/iast/KafkaIastDeserializerForkedTest.groovy +++ b/dd-java-agent/instrumentation/kafka-clients-0.11/src/test/groovy/iast/KafkaIastDeserializerForkedTest.groovy @@ -122,6 +122,10 @@ class KafkaIastDeserializerForkedTest extends IastAgentTestRunner { value(instanceOf(TestBean)) range(0, Integer.MAX_VALUE, source(origin as byte)) } + to.hasTaintedObject { + value('name') + range(0, 4, source(origin as byte, 'name', 'name')) + } to.hasTaintedObject { value('Mr Bean') range(0, 7, source(origin as byte, 'name', 'Mr Bean')) diff --git a/dd-java-agent/instrumentation/spring-webflux-5/src/iastTest/groovy/datadog/trace/instrumentation/springwebflux/server/IastWebFluxTest.groovy b/dd-java-agent/instrumentation/spring-webflux-5/src/iastTest/groovy/datadog/trace/instrumentation/springwebflux/server/IastWebFluxTest.groovy index 97819067c09..01851dcc5d1 100644 --- a/dd-java-agent/instrumentation/spring-webflux-5/src/iastTest/groovy/datadog/trace/instrumentation/springwebflux/server/IastWebFluxTest.groovy +++ b/dd-java-agent/instrumentation/spring-webflux-5/src/iastTest/groovy/datadog/trace/instrumentation/springwebflux/server/IastWebFluxTest.groovy @@ -275,6 +275,14 @@ class IastWebFluxTest extends IastRequestTestRunner { then: // source values take the value of the current object as the body is never converted to a CharSequence + toc.hasTaintedObject { + value 'var1' + range 0, 4, source(SourceTypes.REQUEST_BODY, 'var1', 'var1') + } + toc.hasTaintedObject { + value 'var2' + range 0, 4, source(SourceTypes.REQUEST_BODY, 'var2', 'var2') + } toc.hasTaintedObject { value 'foo' range 0, 3, source(SourceTypes.REQUEST_BODY, 'var1', 'foo') diff --git a/dd-java-agent/instrumentation/spring-webflux-6/src/iastTest/groovy/datadog/trace/instrumentation/springwebflux6/server/IastWebFluxTest.groovy b/dd-java-agent/instrumentation/spring-webflux-6/src/iastTest/groovy/datadog/trace/instrumentation/springwebflux6/server/IastWebFluxTest.groovy index 4359ec06008..30a079dc146 100644 --- a/dd-java-agent/instrumentation/spring-webflux-6/src/iastTest/groovy/datadog/trace/instrumentation/springwebflux6/server/IastWebFluxTest.groovy +++ b/dd-java-agent/instrumentation/spring-webflux-6/src/iastTest/groovy/datadog/trace/instrumentation/springwebflux6/server/IastWebFluxTest.groovy @@ -278,6 +278,14 @@ class IastWebFluxTest extends IastRequestTestRunner { then: // source values take the value of the current object as the body is never converted to a CharSequence + toc.hasTaintedObject { + value 'var1' + range 0, 4, source(SourceTypes.REQUEST_BODY, 'var1', 'var1') + } + toc.hasTaintedObject { + value 'var2' + range 0, 4, source(SourceTypes.REQUEST_BODY, 'var2', 'var2') + } toc.hasTaintedObject { value 'foo' range 0, 3, source(SourceTypes.REQUEST_BODY, 'var1', 'foo') diff --git a/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastVertxSmokeTest.groovy b/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastVertxSmokeTest.groovy index 6166922544d..2fb2e879112 100644 --- a/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastVertxSmokeTest.groovy +++ b/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastVertxSmokeTest.groovy @@ -166,6 +166,9 @@ abstract class AbstractIastVertxSmokeTest extends AbstractIastServerSmokeTest { client.newCall(request).execute() then: + hasTainted { tainted -> + tainted.value == 'my_key' && tainted.ranges[0].source.origin == 'http.request.body' + } hasTainted { tainted -> tainted.value == 'my_value' && tainted.ranges[0].source.origin == 'http.request.body' }