From 502cf6a37ba09c930273b247e572307f79f7c880 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 25 Apr 2025 08:38:39 +0200 Subject: [PATCH 1/4] Collect parsed request body if RASP event improve truncation wip wip - not working wip - fix --- .../com/datadog/appsec/ddwaf/WAFModule.java | 1 + .../appsec/gateway/AppSecRequestContext.java | 19 ++ .../datadog/appsec/gateway/GatewayBridge.java | 20 ++- .../ObjectIntrospectionSpecification.groovy | 120 ++++++++----- .../ExtendedDataCollectionSmokeTest.groovy | 163 ++++++++++++++++++ .../appsec/SpringBootSmokeTest.groovy | 4 + .../trace/api/config/AppSecConfig.java | 1 + .../main/java/datadog/trace/api/Config.java | 7 + 8 files changed, 291 insertions(+), 44 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/ddwaf/WAFModule.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/ddwaf/WAFModule.java index a360b06d676..2954ed2eacc 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/ddwaf/WAFModule.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/ddwaf/WAFModule.java @@ -480,6 +480,7 @@ public void onDataAvailable( } if (gwCtx.isRasp) { + reqCtx.setRaspMatched(true); WafMetricCollector.get().raspRuleMatch(gwCtx.raspRuleType); } diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java index 6314b76ba44..a73de84d7d0 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java @@ -129,6 +129,9 @@ public class AppSecRequestContext implements DataBundle, Closeable { private volatile int wafTimeouts; private volatile int raspTimeouts; + private volatile Object processedRequestBody; + private volatile boolean raspMatched; + // keep a reference to the last published usr.id private volatile String userId; // keep a reference to the last published usr.login @@ -675,4 +678,20 @@ public boolean isWafContextClosed() { void setRequestEndCalled() { requestEndCalled = true; } + + public void setProcessedRequestBody(Object processedRequestBody) { + this.processedRequestBody = processedRequestBody; + } + + public Object getProcessedRequestBody() { + return processedRequestBody; + } + + public boolean isRaspMatched() { + return raspMatched; + } + + public void setRaspMatched(boolean raspMatched) { + this.raspMatched = raspMatched; + } } diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index 310dc5e4853..6848aadcc4a 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -77,6 +77,7 @@ public class GatewayBridge { private static final String USER_COLLECTION_MODE_TAG = "_dd.appsec.user.collection_mode"; private static final Map> EVENT_MAPPINGS = new EnumMap<>(LoginEvent.class); + private static final String METASTRUCT_REQUEST_BODY = "http.request.body"; static { EVENT_MAPPINGS.put(LoginEvent.LOGIN_SUCCESS, KnownAddresses.LOGIN_SUCCESS); @@ -472,7 +473,7 @@ private Flow onGrpcServerRequestMessage(RequestContext ctx_, Object obj) { if (subInfo == null || subInfo.isEmpty()) { return NoopFlow.INSTANCE; } - Object convObj = ObjectIntrospection.convert(obj, ctx); + Object convObj = ObjectIntrospection.convert(obj, ctx).getValue(); DataBundle bundle = new SingletonDataBundle<>(KnownAddresses.GRPC_SERVER_REQUEST_MESSAGE, convObj); try { @@ -572,9 +573,16 @@ private Flow onRequestBodyProcessed(RequestContext ctx_, Object obj) { if (subInfo == null || subInfo.isEmpty()) { return NoopFlow.INSTANCE; } + ObjectIntrospection.ConversionResult converted = + ObjectIntrospection.convert(obj, ctx); + if (Config.get().isAppSecRaspCollectRequestBody()) { + ctx.setProcessedRequestBody(converted.getValue()); + if (converted.isAnyTruncated()) { + ctx_.getTraceSegment().setTagTop("_dd.appsec.rasp.request_body_size.exceeded", true); + } + } DataBundle bundle = - new SingletonDataBundle<>( - KnownAddresses.REQUEST_BODY_OBJECT, ObjectIntrospection.convert(obj, ctx)); + new SingletonDataBundle<>(KnownAddresses.REQUEST_BODY_OBJECT, converted.getValue()); try { GatewayContext gwCtx = new GatewayContext(false); return producerService.publishDataEvent(subInfo, ctx, bundle, gwCtx); @@ -723,6 +731,12 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { StackUtils.addStacktraceEventsToMetaStruct(ctx_, METASTRUCT_EXPLOIT, stackTraces); } + // Report collected parsed request body if there is a RASP event + if (ctx.isRaspMatched() && ctx.getProcessedRequestBody() != null) { + ctx_.getOrCreateMetaStructTop( + METASTRUCT_REQUEST_BODY, k -> ctx.getProcessedRequestBody()); + } + } else if (hasUserInfo(traceSeg)) { // Report all collected request headers on user tracking event writeRequestHeaders(traceSeg, REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders(), false); diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy index 0910e636d7d..919b6a70759 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy @@ -28,12 +28,12 @@ class ObjectIntrospectionSpecification extends DDSpecification { void 'null is preserved'() { expect: - convert(null, ctx) == null + convert(null, ctx).getValue() == null } void 'type #type is preserved'() { when: - def result = convert(input, ctx) + def result = convert(input, ctx).getValue() then: input.getClass() == type @@ -56,7 +56,7 @@ class ObjectIntrospectionSpecification extends DDSpecification { void 'type #type is converted to string'() { when: - def result = convert(input, ctx) + def result = convert(input, ctx).getValue() then: type.isAssignableFrom(input.getClass()) @@ -83,9 +83,9 @@ class ObjectIntrospectionSpecification extends DDSpecification { } expect: - convert(iter, ctx) instanceof List - convert(iter, ctx) == ['a', 'b'] - convert(['a', 'b'], ctx) == ['a', 'b'] + convert(iter, ctx).getValue() instanceof List + convert(iter, ctx) .getValue()== ['a', 'b'] + convert(['a', 'b'], ctx).getValue() == ['a', 'b'] } void 'maps are converted to hash maps'() { @@ -95,21 +95,21 @@ class ObjectIntrospectionSpecification extends DDSpecification { } expect: - convert(map, ctx) instanceof HashMap - convert(map, ctx) == [a: 'b'] - convert([(6): 'b'], ctx) == ['6': 'b'] - convert([(null): 'b'], ctx) == ['null': 'b'] - convert([(true): 'b'], ctx) == ['true': 'b'] - convert([('a' as Character): 'b'], ctx) == ['a': 'b'] - convert([(createCharBuffer('a')): 'b'], ctx) == ['a': 'b'] + convert(map, ctx).getValue() instanceof HashMap + convert(map, ctx).getValue() == [a: 'b'] + convert([(6): 'b'], ctx).getValue() == ['6': 'b'] + convert([(null): 'b'], ctx).getValue() == ['null': 'b'] + convert([(true): 'b'], ctx).getValue() == ['true': 'b'] + convert([('a' as Character): 'b'], ctx).getValue() == ['a': 'b'] + convert([(createCharBuffer('a')): 'b'], ctx).getValue() == ['a': 'b'] } void 'arrays are converted into lists'() { expect: - convert([6, 'b'] as Object[], ctx) == [6, 'b'] - convert([null, null] as Object[], ctx) == [null, null] - convert([1, 2] as int[], ctx) == [1 as int, 2 as int] - convert([1, 2] as byte[], ctx) == [1 as byte, 2 as byte] + convert([6, 'b'] as Object[], ctx).getValue() == [6, 'b'] + convert([null, null] as Object[], ctx).getValue() == [null, null] + convert([1, 2] as int[], ctx).getValue() == [1 as int, 2 as int] + convert([1, 2] as byte[], ctx).getValue() == [1 as byte, 2 as byte] } @SuppressWarnings('UnusedPrivateField') @@ -127,8 +127,8 @@ class ObjectIntrospectionSpecification extends DDSpecification { void 'other objects are converted into hash maps'() { expect: - convert(new ClassToBeConverted(), ctx) instanceof HashMap - convert(new ClassToBeConvertedExt(), ctx) == [c: 'd', a: 'b', l: [1, 2]] + convert(new ClassToBeConverted(), ctx).getValue() instanceof HashMap + convert(new ClassToBeConvertedExt(), ctx) .getValue()== [c: 'd', a: 'b', l: [1, 2]] } class ProtobufLikeClass { @@ -139,15 +139,15 @@ class ObjectIntrospectionSpecification extends DDSpecification { void 'some field names are ignored'() { expect: - convert(new ProtobufLikeClass(), ctx) instanceof HashMap - convert(new ProtobufLikeClass(), ctx) == [c: 'd'] + convert(new ProtobufLikeClass(), ctx).getValue() instanceof HashMap + convert(new ProtobufLikeClass(), ctx).getValue() == [c: 'd'] } void 'invalid keys are converted to special strings'() { expect: - convert(Collections.singletonMap(new ClassToBeConverted(), 'a'), ctx) == ['invalid_key:1': 'a'] - convert([new ClassToBeConverted(): 'a', new ClassToBeConverted(): 'b'], ctx) == ['invalid_key:1': 'a', 'invalid_key:2': 'b'] - convert(Collections.singletonMap([1, 2], 'a'), ctx) == ['invalid_key:1': 'a'] + convert(Collections.singletonMap(new ClassToBeConverted(), 'a'), ctx).getValue() == ['invalid_key:1': 'a'] + convert([new ClassToBeConverted(): 'a', new ClassToBeConverted(): 'b'], ctx).getValue() == ['invalid_key:1': 'a', 'invalid_key:2': 'b'] + convert(Collections.singletonMap([1, 2], 'a'), ctx).getValue() == ['invalid_key:1': 'a'] } void 'max number of elements is honored'() { @@ -156,16 +156,28 @@ class ObjectIntrospectionSpecification extends DDSpecification { 128.times { m[it] = 'b' } when: - def result1 = convert([['a'] * 255], ctx)[0] - def result2 = convert([['a'] * 255 as String[]], ctx)[0] + def result1 = convert([['a'] * 255], ctx) + def result2 = convert([['a'] * 255 as String[]], ctx) def result3 = convert(m, ctx) then: - result1.size() == 254 // +2 for the lists - result2.size() == 254 // +2 for the lists - result3.size() == 127 // +1 for the map, 2 for each entry (key and value) + result1.getValue()[0].size() == 254 // +2 for the lists + result1.isAnyTruncated() + result1.isCollectionTruncated() + !result1.isDepthTruncated() + !result1.isStringTruncated() + result2.getValue()[0].size() == 254 // +2 for the lists + result2.isAnyTruncated() + result2.isCollectionTruncated() + !result2.isDepthTruncated() + !result2.isStringTruncated() + result3.getValue().size() == 127// +1 for the map, 2 for each entry (key and value) + !result3.isAnyTruncated() + !result3.isCollectionTruncated() + !result3.isDepthTruncated() + !result3.isStringTruncated() 2 * ctx.setWafTruncated() - 2 * wafMetricCollector.wafInputTruncated(false, true, false) + //2 * wafMetricCollector.wafInputTruncated(false, true, false) } @@ -179,18 +191,23 @@ class ObjectIntrospectionSpecification extends DDSpecification { when: // Invoke conversion with context def result = convert(objArray, ctx) + def converted = result.getValue() then: // Traverse converted arrays to count actual depth int depth = 0 - for (p = result; p != null; p = p[0]) { + for (p = converted; p != null; p = p[0]) { depth++ } depth == 21 // after max depth we have nulls // Should record a truncation due to depth 1 * ctx.setWafTruncated() - 1 * wafMetricCollector.wafInputTruncated(false, false, true) + result.isDepthTruncated() + !result.isCollectionTruncated() + result.isAnyTruncated() + !result.isStringTruncated() + //1 * wafMetricCollector.wafInputTruncated(false, false, true) } void 'max depth is honored — list version'() { @@ -203,18 +220,23 @@ class ObjectIntrospectionSpecification extends DDSpecification { when: // Invoke conversion with context def result = convert(list, ctx) + def converted = result.getValue() then: // Traverse converted lists to count actual depth int depth = 0 - for (p = result; p != null; p = p[0]) { + for (p = converted; p != null; p = p[0]) { depth++ } depth == 21 // after max depth we have nulls // Should record a truncation due to depth 1 * ctx.setWafTruncated() - 1 * wafMetricCollector.wafInputTruncated(false, false, true) + result.isDepthTruncated() + !result.isCollectionTruncated() + result.isAnyTruncated() + !result.isStringTruncated() + //1 * wafMetricCollector.wafInputTruncated(false, false, true) } def 'max depth is honored — map version'() { @@ -227,18 +249,23 @@ class ObjectIntrospectionSpecification extends DDSpecification { when: // Invoke conversion with context def result = convert(map, ctx) + def converted = result.getValue() then: // Traverse converted maps to count actual depth int depth = 0 - for (p = result; p != null; p = p['a']) { + for (p = converted; p != null; p = p['a']) { depth++ } depth == 21 // after max depth we have nulls // Should record a truncation due to depth 1 * ctx.setWafTruncated() - 1 * wafMetricCollector.wafInputTruncated(false, false, true) + result.isDepthTruncated() + !result.isCollectionTruncated() + result.isAnyTruncated() + !result.isStringTruncated() + //1 * wafMetricCollector.wafInputTruncated(false, false, true) } void 'truncate long #typeName to 4096 chars and set truncation flag'() { @@ -246,7 +273,8 @@ class ObjectIntrospectionSpecification extends DDSpecification { def longInput = rawInput when: - def converted = convert(longInput, ctx) + def result = convert(longInput, ctx) + def converted = result.getValue() then: // Should always produce a String of exactly 4096 chars @@ -255,7 +283,11 @@ class ObjectIntrospectionSpecification extends DDSpecification { // Should record a truncation due to string length 1 * ctx.setWafTruncated() - 1 * wafMetricCollector.wafInputTruncated(true, false, false) + //1 * wafMetricCollector.wafInputTruncated(true, false, false) + result.isStringTruncated() + !result.isDepthTruncated() + !result.isCollectionTruncated() + result.isAnyTruncated() where: typeName | rawInput @@ -271,7 +303,8 @@ class ObjectIntrospectionSpecification extends DDSpecification { when: // convert returns Pair - def converted = convert(inputMap, ctx) + def result = convert(inputMap, ctx) + def converted = result.getValue() then: // Extract the single truncated key @@ -281,7 +314,11 @@ class ObjectIntrospectionSpecification extends DDSpecification { truncatedKey.length() == 4096 1 * ctx.setWafTruncated() - 1 * wafMetricCollector.wafInputTruncated(true, false, false) + //1 * wafMetricCollector.wafInputTruncated(true, false, false) + result.isStringTruncated() + !result.isDepthTruncated() + !result.isCollectionTruncated() + result.isAnyTruncated() where: @@ -302,6 +339,7 @@ class ObjectIntrospectionSpecification extends DDSpecification { } expect: - convert([cs], ctx) == ['error:my exception'] + convert([cs], ctx).getValue() == ['error:my exception'] } } + diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/ExtendedDataCollectionSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/ExtendedDataCollectionSmokeTest.groovy index da2862dce12..5b86000e318 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/ExtendedDataCollectionSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/ExtendedDataCollectionSmokeTest.groovy @@ -1,19 +1,63 @@ package datadog.smoketest.appsec import datadog.trace.agent.test.utils.OkHttpUtils +import okhttp3.FormBody import okhttp3.Request +import spock.lang.Shared class ExtendedDataCollectionSmokeTest extends AbstractAppSecServerSmokeTest { + @Shared + String buildDir = new File(System.getProperty("datadog.smoketest.builddir")).absolutePath + @Shared + String customRulesPath = "${buildDir}/appsec_custom_rules.json" + + def prepareCustomRules() { + // Prepare ruleset with additional test rules + mergeRules( + customRulesPath, + [ + [ + id : 'rasp-932-100', // to replace default rule + name : 'Shell command injection exploit', + enable : 'true', + tags : [ + type: 'command_injection', + category: 'vulnerability_trigger', + cwe: '77', + capec: '1000/152/248/88', + confidence: '0', + module: 'rasp' + ], + conditions : [ + [ + parameters: [ + resource: [[address: 'server.sys.shell.cmd']], + params : [[address: 'server.request.body']], + ], + operator : "shi_detector", + ], + ], + transformers: [], + on_match : ['block'] + ] + ]) + } + @Override ProcessBuilder createProcessBuilder() { + // We run this here to ensure it runs before starting the process. Child setupSpec runs after parent setupSpec, + // so it is not a valid location. + prepareCustomRules() + String springBootShadowJar = System.getProperty("datadog.smoketest.appsec.springboot.shadowJar.path") List command = new ArrayList<>() command.add(javaPath()) command.addAll(defaultJavaProperties) command.addAll(defaultAppSecProperties) + command.add('-Ddd.appsec.rasp.collect.request.body=true') command.add('-Ddd.appsec.collect.all.headers=true') command.add('-Ddd.appsec.header.collection.redaction.enabled=false') command.addAll((String[]) ["-jar", springBootShadowJar, "--server.port=${httpPort}"]) @@ -146,6 +190,125 @@ class ExtendedDataCollectionSmokeTest extends AbstractAppSecServerSmokeTest { rootSpan.meta.get('http.response.headers.content-language') == 'en-US' } + void 'test request body collection if RASP event'(){ + when: + String url = "http://localhost:${httpPort}/shi/cmd" + def formBuilder = new FormBody.Builder() + formBuilder.add('cmd', '$(cat /etc/passwd 1>&2 ; echo .)') + final body = formBuilder.build() + def request = new Request.Builder() + .url(url) + .post(body) + .build() + def response = client.newCall(request).execute() + def responseBodyStr = response.body().string() + + then: + response.code() == 403 + responseBodyStr.contains('You\'ve been blocked') + + when: + waitForTraceCount(1) + + then: + def rootSpans = this.rootSpans.toList() + rootSpans.size() == 1 + def rootSpan = rootSpans[0] + + def trigger = null + for (t in rootSpan.triggers) { + if (t['rule']['id'] == 'rasp-932-100') { + trigger = t + break + } + } + assert trigger != null, 'test trigger not found' + + rootSpan.span.metaStruct != null + def requestBody = rootSpan.span.metaStruct.get('http.request.body') + assert requestBody != null, 'request body is not set' + !rootSpan.meta.containsKey('_dd.appsec.rasp.request_body_size.exceeded') + + } + + void 'test request body collection if RASP event exceeded'(){ + when: + String url = "http://localhost:${httpPort}/shi/cmd" + def formBuilder = new FormBody.Builder() + formBuilder.add('cmd', '$(cat /etc/passwd 1>&2 ; echo .)'+'A' * 5000) + final body = formBuilder.build() + def request = new Request.Builder() + .url(url) + .post(body) + .build() + def response = client.newCall(request).execute() + def responseBodyStr = response.body().string() + + then: + response.code() == 403 + responseBodyStr.contains('You\'ve been blocked') + + when: + waitForTraceCount(1) + + then: + def rootSpans = this.rootSpans.toList() + rootSpans.size() == 1 + def rootSpan = rootSpans[0] + + def trigger = null + for (t in rootSpan.triggers) { + if (t['rule']['id'] == 'rasp-932-100') { + trigger = t + break + } + } + assert trigger != null, 'test trigger not found' + + rootSpan.span.metaStruct != null + def requestBody = rootSpan.span.metaStruct.get('http.request.body') + assert requestBody != null, 'request body is not set' + rootSpan.meta.containsKey('_dd.appsec.rasp.request_body_size.exceeded') + + } + + void 'test request body not collected if no RASP event'(){ + when: + String url = "http://localhost:${httpPort}/greeting" + def formBuilder = new FormBody.Builder() + formBuilder.add('cmd', 'test') + final body = formBuilder.build() + def request = new Request.Builder() + .url(url) + .post(body) + .build() + def response = client.newCall(request).execute() + def responseBodyStr = response.body().string() + + then: + response.code() == 200 + + when: + waitForTraceCount(1) + + then: + def rootSpans = this.rootSpans.toList() + rootSpans.size() == 1 + def rootSpan = rootSpans[0] + + def trigger = null + for (t in rootSpan.triggers) { + if (t['rule']['id'] == 'rasp-932-100') { + trigger = t + break + } + } + assert trigger == null, 'test trigger found' + + rootSpan.span.metaStruct == null + + } + } diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy index 42768f998b3..8476c19173c 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy @@ -469,6 +469,7 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { } } assert trigger != null, 'test trigger not found' + rootSpan.span.metaStruct == null where: variant | _ @@ -508,6 +509,7 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { } } assert trigger != null, 'test trigger not found' + rootSpan.span.metaStruct == null where: variant | _ @@ -600,6 +602,7 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { } } assert trigger != null, 'test trigger not found' + rootSpan.span.metaStruct == null where: endpoint | cmd | params @@ -650,6 +653,7 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { } } assert trigger != null, 'test trigger not found' + rootSpan.span.metaStruct == null where: endpoint | cmd | params diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java index c194828038a..e65fbbfbf07 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java @@ -47,6 +47,7 @@ public final class AppSecConfig { public static final String APPSEC_MAX_COLLECTED_HEADERS = "appsec.max.collected.headers"; public static final String APPSEC_HEADER_COLLECTION_REDACTION_ENABLED = "appsec.header.collection.redaction.enabled"; + public static final String APPSEC_RASP_COLLECT_REQUEST_BODY = "appsec.rasp.collect.request.body"; private AppSecConfig() {} } 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 18d1ac74981..05426072f35 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -295,6 +295,7 @@ public static String getHostName() { private final boolean appSecCollectAllHeaders; private final boolean appSecHeaderCollectionRedactionEnabled; private final int appSecMaxCollectedHeaders; + private final boolean appSecRaspCollectRequestBody; private final boolean apiSecurityEnabled; private final float apiSecuritySampleDelay; private final boolean apiSecurityEndpointCollectionEnabled; @@ -1395,6 +1396,8 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) appSecMaxCollectedHeaders = configProvider.getInteger( APPSEC_MAX_COLLECTED_HEADERS, DEFAULT_APPSEC_MAX_COLLECTED_HEADERS); + appSecRaspCollectRequestBody = + configProvider.getBoolean(APPSEC_RASP_COLLECT_REQUEST_BODY, false); apiSecurityEnabled = configProvider.getBoolean( API_SECURITY_ENABLED, DEFAULT_API_SECURITY_ENABLED, API_SECURITY_ENABLED_EXPERIMENTAL); @@ -4214,6 +4217,10 @@ public int getAppsecMaxCollectedHeaders() { return appSecMaxCollectedHeaders; } + public boolean isAppSecRaspCollectRequestBody() { + return appSecRaspCollectRequestBody; + } + public boolean isCloudPayloadTaggingEnabledFor(String serviceName) { return cloudPayloadTaggingServices.contains(serviceName); } From cff33403e9bb9ae6b399d16683b9b38251289b60 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 21 May 2025 13:22:00 +0200 Subject: [PATCH 2/4] wip - fix conversion --- .../event/data/ObjectIntrospection.java | 28 ++++ .../datadog/appsec/gateway/GatewayBridge.java | 22 ++-- .../ObjectIntrospectionSpecification.groovy | 120 ++++++------------ 3 files changed, 82 insertions(+), 88 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java index dbb235304f4..31b0a4c88f5 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java @@ -38,6 +38,20 @@ public final class ObjectIntrospection { private ObjectIntrospection() {} + /** + * Listener interface for optional per-call truncation logic. Single-method invoked when any + * truncation occurs, receiving only the request context. + */ + @FunctionalInterface + public interface TruncationListener { + /** + * Called after default truncation handling if any truncation occurred. + * + * @param requestContext the request context for this conversion + */ + void onTruncation(AppSecRequestContext requestContext); + } + /** * Converts arbitrary objects compatible with ddwaf_object. Possible types in the result are: * @@ -68,12 +82,26 @@ private ObjectIntrospection() {} * @return the converted object */ public static Object convert(Object obj, AppSecRequestContext requestContext) { + return convert(obj, requestContext, null); + } + + /** + * Core conversion method with an optional per-call truncation listener. Always applies default + * truncation logic, then invokes listener if provided. + */ + public static Object convert( + Object obj, AppSecRequestContext requestContext, TruncationListener listener) { State state = new State(requestContext); Object converted = guardedConversion(obj, 0, state); if (state.stringTooLong || state.listMapTooLarge || state.objectTooDeep) { + // Default truncation handling: always run requestContext.setWafTruncated(); WafMetricCollector.get() .wafInputTruncated(state.stringTooLong, state.listMapTooLarge, state.objectTooDeep); + // Optional extra per-call logic: only requestContext is passed + if (listener != null) { + listener.onTruncation(requestContext); + } } return converted; } diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index 6848aadcc4a..63bb5ec2795 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -473,7 +473,7 @@ private Flow onGrpcServerRequestMessage(RequestContext ctx_, Object obj) { if (subInfo == null || subInfo.isEmpty()) { return NoopFlow.INSTANCE; } - Object convObj = ObjectIntrospection.convert(obj, ctx).getValue(); + Object convObj = ObjectIntrospection.convert(obj, ctx); DataBundle bundle = new SingletonDataBundle<>(KnownAddresses.GRPC_SERVER_REQUEST_MESSAGE, convObj); try { @@ -573,16 +573,20 @@ private Flow onRequestBodyProcessed(RequestContext ctx_, Object obj) { if (subInfo == null || subInfo.isEmpty()) { return NoopFlow.INSTANCE; } - ObjectIntrospection.ConversionResult converted = - ObjectIntrospection.convert(obj, ctx); + Object converted = + ObjectIntrospection.convert( + obj, + ctx, + (rc) -> { + if (Config.get().isAppSecRaspCollectRequestBody()) { + ctx_.getTraceSegment() + .setTagTop("_dd.appsec.rasp.request_body_size.exceeded", true); + } + }); if (Config.get().isAppSecRaspCollectRequestBody()) { - ctx.setProcessedRequestBody(converted.getValue()); - if (converted.isAnyTruncated()) { - ctx_.getTraceSegment().setTagTop("_dd.appsec.rasp.request_body_size.exceeded", true); - } + ctx.setProcessedRequestBody(converted); } - DataBundle bundle = - new SingletonDataBundle<>(KnownAddresses.REQUEST_BODY_OBJECT, converted.getValue()); + DataBundle bundle = new SingletonDataBundle<>(KnownAddresses.REQUEST_BODY_OBJECT, converted); try { GatewayContext gwCtx = new GatewayContext(false); return producerService.publishDataEvent(subInfo, ctx, bundle, gwCtx); diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy index 919b6a70759..0910e636d7d 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy @@ -28,12 +28,12 @@ class ObjectIntrospectionSpecification extends DDSpecification { void 'null is preserved'() { expect: - convert(null, ctx).getValue() == null + convert(null, ctx) == null } void 'type #type is preserved'() { when: - def result = convert(input, ctx).getValue() + def result = convert(input, ctx) then: input.getClass() == type @@ -56,7 +56,7 @@ class ObjectIntrospectionSpecification extends DDSpecification { void 'type #type is converted to string'() { when: - def result = convert(input, ctx).getValue() + def result = convert(input, ctx) then: type.isAssignableFrom(input.getClass()) @@ -83,9 +83,9 @@ class ObjectIntrospectionSpecification extends DDSpecification { } expect: - convert(iter, ctx).getValue() instanceof List - convert(iter, ctx) .getValue()== ['a', 'b'] - convert(['a', 'b'], ctx).getValue() == ['a', 'b'] + convert(iter, ctx) instanceof List + convert(iter, ctx) == ['a', 'b'] + convert(['a', 'b'], ctx) == ['a', 'b'] } void 'maps are converted to hash maps'() { @@ -95,21 +95,21 @@ class ObjectIntrospectionSpecification extends DDSpecification { } expect: - convert(map, ctx).getValue() instanceof HashMap - convert(map, ctx).getValue() == [a: 'b'] - convert([(6): 'b'], ctx).getValue() == ['6': 'b'] - convert([(null): 'b'], ctx).getValue() == ['null': 'b'] - convert([(true): 'b'], ctx).getValue() == ['true': 'b'] - convert([('a' as Character): 'b'], ctx).getValue() == ['a': 'b'] - convert([(createCharBuffer('a')): 'b'], ctx).getValue() == ['a': 'b'] + convert(map, ctx) instanceof HashMap + convert(map, ctx) == [a: 'b'] + convert([(6): 'b'], ctx) == ['6': 'b'] + convert([(null): 'b'], ctx) == ['null': 'b'] + convert([(true): 'b'], ctx) == ['true': 'b'] + convert([('a' as Character): 'b'], ctx) == ['a': 'b'] + convert([(createCharBuffer('a')): 'b'], ctx) == ['a': 'b'] } void 'arrays are converted into lists'() { expect: - convert([6, 'b'] as Object[], ctx).getValue() == [6, 'b'] - convert([null, null] as Object[], ctx).getValue() == [null, null] - convert([1, 2] as int[], ctx).getValue() == [1 as int, 2 as int] - convert([1, 2] as byte[], ctx).getValue() == [1 as byte, 2 as byte] + convert([6, 'b'] as Object[], ctx) == [6, 'b'] + convert([null, null] as Object[], ctx) == [null, null] + convert([1, 2] as int[], ctx) == [1 as int, 2 as int] + convert([1, 2] as byte[], ctx) == [1 as byte, 2 as byte] } @SuppressWarnings('UnusedPrivateField') @@ -127,8 +127,8 @@ class ObjectIntrospectionSpecification extends DDSpecification { void 'other objects are converted into hash maps'() { expect: - convert(new ClassToBeConverted(), ctx).getValue() instanceof HashMap - convert(new ClassToBeConvertedExt(), ctx) .getValue()== [c: 'd', a: 'b', l: [1, 2]] + convert(new ClassToBeConverted(), ctx) instanceof HashMap + convert(new ClassToBeConvertedExt(), ctx) == [c: 'd', a: 'b', l: [1, 2]] } class ProtobufLikeClass { @@ -139,15 +139,15 @@ class ObjectIntrospectionSpecification extends DDSpecification { void 'some field names are ignored'() { expect: - convert(new ProtobufLikeClass(), ctx).getValue() instanceof HashMap - convert(new ProtobufLikeClass(), ctx).getValue() == [c: 'd'] + convert(new ProtobufLikeClass(), ctx) instanceof HashMap + convert(new ProtobufLikeClass(), ctx) == [c: 'd'] } void 'invalid keys are converted to special strings'() { expect: - convert(Collections.singletonMap(new ClassToBeConverted(), 'a'), ctx).getValue() == ['invalid_key:1': 'a'] - convert([new ClassToBeConverted(): 'a', new ClassToBeConverted(): 'b'], ctx).getValue() == ['invalid_key:1': 'a', 'invalid_key:2': 'b'] - convert(Collections.singletonMap([1, 2], 'a'), ctx).getValue() == ['invalid_key:1': 'a'] + convert(Collections.singletonMap(new ClassToBeConverted(), 'a'), ctx) == ['invalid_key:1': 'a'] + convert([new ClassToBeConverted(): 'a', new ClassToBeConverted(): 'b'], ctx) == ['invalid_key:1': 'a', 'invalid_key:2': 'b'] + convert(Collections.singletonMap([1, 2], 'a'), ctx) == ['invalid_key:1': 'a'] } void 'max number of elements is honored'() { @@ -156,28 +156,16 @@ class ObjectIntrospectionSpecification extends DDSpecification { 128.times { m[it] = 'b' } when: - def result1 = convert([['a'] * 255], ctx) - def result2 = convert([['a'] * 255 as String[]], ctx) + def result1 = convert([['a'] * 255], ctx)[0] + def result2 = convert([['a'] * 255 as String[]], ctx)[0] def result3 = convert(m, ctx) then: - result1.getValue()[0].size() == 254 // +2 for the lists - result1.isAnyTruncated() - result1.isCollectionTruncated() - !result1.isDepthTruncated() - !result1.isStringTruncated() - result2.getValue()[0].size() == 254 // +2 for the lists - result2.isAnyTruncated() - result2.isCollectionTruncated() - !result2.isDepthTruncated() - !result2.isStringTruncated() - result3.getValue().size() == 127// +1 for the map, 2 for each entry (key and value) - !result3.isAnyTruncated() - !result3.isCollectionTruncated() - !result3.isDepthTruncated() - !result3.isStringTruncated() + result1.size() == 254 // +2 for the lists + result2.size() == 254 // +2 for the lists + result3.size() == 127 // +1 for the map, 2 for each entry (key and value) 2 * ctx.setWafTruncated() - //2 * wafMetricCollector.wafInputTruncated(false, true, false) + 2 * wafMetricCollector.wafInputTruncated(false, true, false) } @@ -191,23 +179,18 @@ class ObjectIntrospectionSpecification extends DDSpecification { when: // Invoke conversion with context def result = convert(objArray, ctx) - def converted = result.getValue() then: // Traverse converted arrays to count actual depth int depth = 0 - for (p = converted; p != null; p = p[0]) { + for (p = result; p != null; p = p[0]) { depth++ } depth == 21 // after max depth we have nulls // Should record a truncation due to depth 1 * ctx.setWafTruncated() - result.isDepthTruncated() - !result.isCollectionTruncated() - result.isAnyTruncated() - !result.isStringTruncated() - //1 * wafMetricCollector.wafInputTruncated(false, false, true) + 1 * wafMetricCollector.wafInputTruncated(false, false, true) } void 'max depth is honored — list version'() { @@ -220,23 +203,18 @@ class ObjectIntrospectionSpecification extends DDSpecification { when: // Invoke conversion with context def result = convert(list, ctx) - def converted = result.getValue() then: // Traverse converted lists to count actual depth int depth = 0 - for (p = converted; p != null; p = p[0]) { + for (p = result; p != null; p = p[0]) { depth++ } depth == 21 // after max depth we have nulls // Should record a truncation due to depth 1 * ctx.setWafTruncated() - result.isDepthTruncated() - !result.isCollectionTruncated() - result.isAnyTruncated() - !result.isStringTruncated() - //1 * wafMetricCollector.wafInputTruncated(false, false, true) + 1 * wafMetricCollector.wafInputTruncated(false, false, true) } def 'max depth is honored — map version'() { @@ -249,23 +227,18 @@ class ObjectIntrospectionSpecification extends DDSpecification { when: // Invoke conversion with context def result = convert(map, ctx) - def converted = result.getValue() then: // Traverse converted maps to count actual depth int depth = 0 - for (p = converted; p != null; p = p['a']) { + for (p = result; p != null; p = p['a']) { depth++ } depth == 21 // after max depth we have nulls // Should record a truncation due to depth 1 * ctx.setWafTruncated() - result.isDepthTruncated() - !result.isCollectionTruncated() - result.isAnyTruncated() - !result.isStringTruncated() - //1 * wafMetricCollector.wafInputTruncated(false, false, true) + 1 * wafMetricCollector.wafInputTruncated(false, false, true) } void 'truncate long #typeName to 4096 chars and set truncation flag'() { @@ -273,8 +246,7 @@ class ObjectIntrospectionSpecification extends DDSpecification { def longInput = rawInput when: - def result = convert(longInput, ctx) - def converted = result.getValue() + def converted = convert(longInput, ctx) then: // Should always produce a String of exactly 4096 chars @@ -283,11 +255,7 @@ class ObjectIntrospectionSpecification extends DDSpecification { // Should record a truncation due to string length 1 * ctx.setWafTruncated() - //1 * wafMetricCollector.wafInputTruncated(true, false, false) - result.isStringTruncated() - !result.isDepthTruncated() - !result.isCollectionTruncated() - result.isAnyTruncated() + 1 * wafMetricCollector.wafInputTruncated(true, false, false) where: typeName | rawInput @@ -303,8 +271,7 @@ class ObjectIntrospectionSpecification extends DDSpecification { when: // convert returns Pair - def result = convert(inputMap, ctx) - def converted = result.getValue() + def converted = convert(inputMap, ctx) then: // Extract the single truncated key @@ -314,11 +281,7 @@ class ObjectIntrospectionSpecification extends DDSpecification { truncatedKey.length() == 4096 1 * ctx.setWafTruncated() - //1 * wafMetricCollector.wafInputTruncated(true, false, false) - result.isStringTruncated() - !result.isDepthTruncated() - !result.isCollectionTruncated() - result.isAnyTruncated() + 1 * wafMetricCollector.wafInputTruncated(true, false, false) where: @@ -339,7 +302,6 @@ class ObjectIntrospectionSpecification extends DDSpecification { } expect: - convert([cs], ctx).getValue() == ['error:my exception'] + convert([cs], ctx) == ['error:my exception'] } } - From 44e8dda25210649ae61a3220987226427a3ef3af Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 21 May 2025 13:47:21 +0200 Subject: [PATCH 3/4] wip - add tests and modify listener --- .../appsec/event/data/ObjectIntrospection.java | 10 +++------- .../com/datadog/appsec/gateway/GatewayBridge.java | 2 +- .../data/ObjectIntrospectionSpecification.groovy | 14 ++++++++++++++ 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java index 31b0a4c88f5..9d191877c1c 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java @@ -44,12 +44,8 @@ private ObjectIntrospection() {} */ @FunctionalInterface public interface TruncationListener { - /** - * Called after default truncation handling if any truncation occurred. - * - * @param requestContext the request context for this conversion - */ - void onTruncation(AppSecRequestContext requestContext); + /** Called after default truncation handling if any truncation occurred. */ + void onTruncation(); } /** @@ -100,7 +96,7 @@ public static Object convert( .wafInputTruncated(state.stringTooLong, state.listMapTooLarge, state.objectTooDeep); // Optional extra per-call logic: only requestContext is passed if (listener != null) { - listener.onTruncation(requestContext); + listener.onTruncation(); } } return converted; diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index 63bb5ec2795..b4bdb9b64c9 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -577,7 +577,7 @@ private Flow onRequestBodyProcessed(RequestContext ctx_, Object obj) { ObjectIntrospection.convert( obj, ctx, - (rc) -> { + () -> { if (Config.get().isAppSecRaspCollectRequestBody()) { ctx_.getTraceSegment() .setTagTop("_dd.appsec.rasp.request_body_size.exceeded", true); diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy index 0910e636d7d..d85a04fd47b 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy @@ -304,4 +304,18 @@ class ObjectIntrospectionSpecification extends DDSpecification { expect: convert([cs], ctx) == ['error:my exception'] } + + void 'truncated conversion triggers truncation listener if available '() { + setup: + def listener = Mock(ObjectIntrospection.TruncationListener) + def object = 'A' * 5000 + + when: + convert(object, ctx, listener) + + then: + 1 * ctx.setWafTruncated() + 1 * wafMetricCollector.wafInputTruncated(true, false, false) + 1 * listener.onTruncation() + } } From c483a1be3d06797fba9810e4b06f4fbe9a01b56c Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 21 May 2025 16:05:44 +0200 Subject: [PATCH 4/4] wip - fix codenarc --- .../smoketest/appsec/ExtendedDataCollectionSmokeTest.groovy | 1 - 1 file changed, 1 deletion(-) diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/ExtendedDataCollectionSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/ExtendedDataCollectionSmokeTest.groovy index 5b86000e318..6789a7ba0cc 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/ExtendedDataCollectionSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/ExtendedDataCollectionSmokeTest.groovy @@ -283,7 +283,6 @@ class ExtendedDataCollectionSmokeTest extends AbstractAppSecServerSmokeTest { .post(body) .build() def response = client.newCall(request).execute() - def responseBodyStr = response.body().string() then: response.code() == 200