Skip to content

Commit 225e336

Browse files
jandro996jandro996
authored
Update Extended appsec request/response headers and request body collection (#9428)
What Does This Do Support new extended_data_collection waf rule action Deprecate DD_APPSEC_HEADER_COLLECTION_REDACTION_ENABLED, DD_APPSEC_COLLECT_ALL_HEADERS, DD_APPSEC_MAX_COLLECTED_HEADERS, DD_APPSEC_RASP_COLLECT_REQUEST_BODY and remove usage as are not used by any client. _dd.appsec.rasp.request_body_size.exceeded renamed to _dd.appsec.request_body_size.exceeded Remove report collected parsed request body only if there is a RASP event, the new specification claims that we need to collect it also for WAF events DD_APPSEC_HEADER_COLLECTION_REDACTION_ENABLED double opt-in safeguard removed Add CAPABILITY_ASM_EXTENDED_DATA_COLLECTION Motivation Update implementation to meet the new RFC requirements
1 parent df07d4f commit 225e336

File tree

11 files changed

+483
-84
lines changed

11 files changed

+483
-84
lines changed

‎dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_DD_RULES;
1010
import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_EXCLUSIONS;
1111
import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_EXCLUSION_DATA;
12+
import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_EXTENDED_DATA_COLLECTION;
1213
import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_HEADER_FINGERPRINT;
1314
import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_IP_BLOCKING;
1415
import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_NETWORK_FINGERPRINT;
@@ -168,7 +169,8 @@ private long getRulesAndDataCapabilities() {
168169
| CAPABILITY_ASM_SESSION_FINGERPRINT
169170
| CAPABILITY_ASM_NETWORK_FINGERPRINT
170171
| CAPABILITY_ASM_HEADER_FINGERPRINT
171-
| CAPABILITY_ASM_TRACE_TAGGING_RULES;
172+
| CAPABILITY_ASM_TRACE_TAGGING_RULES
173+
| CAPABILITY_ASM_EXTENDED_DATA_COLLECTION;
172174
if (tracerConfig.isAppSecRaspEnabled()) {
173175
capabilities |= CAPABILITY_ASM_RASP_SQLI;
174176
capabilities |= CAPABILITY_ASM_RASP_SSRF;
@@ -554,7 +556,8 @@ public void close() {
554556
| CAPABILITY_ASM_SESSION_FINGERPRINT
555557
| CAPABILITY_ASM_NETWORK_FINGERPRINT
556558
| CAPABILITY_ASM_HEADER_FINGERPRINT
557-
| CAPABILITY_ASM_TRACE_TAGGING_RULES);
559+
| CAPABILITY_ASM_TRACE_TAGGING_RULES
560+
| CAPABILITY_ASM_EXTENDED_DATA_COLLECTION);
558561
this.configurationPoller.removeListeners(Product.ASM_DD);
559562
this.configurationPoller.removeListeners(Product.ASM_DATA);
560563
this.configurationPoller.removeListeners(Product.ASM);

‎dd-java-agent/appsec/src/main/java/com/datadog/appsec/ddwaf/WAFModule.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,26 @@ public void onDataAvailable(
395395
} else {
396396
log.debug("Ignoring action with type generate_stack (disabled by config)");
397397
}
398+
} else if ("extended_data_collection".equals(actionInfo.type)) {
399+
// Extended data collection is handled by the GatewayBridge
400+
reqCtx.setExtendedDataCollection(true);
401+
// Handle max_collected_headers parameter which can come as Number or String
402+
// representation of a number
403+
int maxHeaders = AppSecRequestContext.DEFAULT_EXTENDED_DATA_COLLECTION_MAX_HEADERS;
404+
Object maxHeadersParam =
405+
actionInfo.parameters.getOrDefault(
406+
"max_collected_headers",
407+
AppSecRequestContext.DEFAULT_EXTENDED_DATA_COLLECTION_MAX_HEADERS);
408+
if (maxHeadersParam instanceof Number) {
409+
maxHeaders = ((Number) maxHeadersParam).intValue();
410+
} else if (maxHeadersParam instanceof String) {
411+
try {
412+
maxHeaders = Integer.parseInt((String) maxHeadersParam);
413+
} catch (NumberFormatException e) {
414+
log.debug("Failed to parse max_collected_headers value: {}", maxHeadersParam);
415+
}
416+
}
417+
reqCtx.setExtendedDataCollectionMaxHeaders(maxHeaders);
398418
} else {
399419
log.info("Ignoring action with type {}", actionInfo.type);
400420
if (!gwCtx.isRasp) {

‎dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131
public class AppSecRequestContext implements DataBundle, Closeable {
3232
private static final Logger log = LoggerFactory.getLogger(AppSecRequestContext.class);
3333

34+
public static final int DEFAULT_EXTENDED_DATA_COLLECTION_MAX_HEADERS = 50;
35+
3436
// Values MUST be lowercase! Lookup with Ignore Case
3537
// was removed due performance reason
3638
// request headers that will always be set when appsec is enabled
@@ -77,6 +79,19 @@ public class AppSecRequestContext implements DataBundle, Closeable {
7779
new TreeSet<>(
7880
Arrays.asList("content-length", "content-type", "content-encoding", "content-language"));
7981

82+
// headers related with authorization
83+
public static final Set<String> AUTHORIZATION_HEADERS =
84+
new TreeSet<>(
85+
Arrays.asList(
86+
"authorization",
87+
"proxy-authorization",
88+
"www-authenticate",
89+
"proxy-authenticate",
90+
"authentication-info",
91+
"proxy-authentication-info",
92+
"cookie",
93+
"set-cookie"));
94+
8095
static {
8196
REQUEST_HEADERS_ALLOW_LIST.addAll(DEFAULT_REQUEST_HEADERS_ALLOW_LIST);
8297
}
@@ -99,6 +114,9 @@ public class AppSecRequestContext implements DataBundle, Closeable {
99114
private int peerPort;
100115
private String inferredClientIp;
101116

117+
private boolean extendedDataCollection = false;
118+
private int extendedDataCollectionMaxHeaders = DEFAULT_EXTENDED_DATA_COLLECTION_MAX_HEADERS;
119+
102120
private volatile StoredBodySupplier storedRequestBodySupplier;
103121
private String dbType;
104122

@@ -133,6 +151,7 @@ public class AppSecRequestContext implements DataBundle, Closeable {
133151
private volatile int raspTimeouts;
134152

135153
private volatile Object processedRequestBody;
154+
private volatile boolean processedResponseBodySizeExceeded;
136155
private volatile boolean raspMatched;
137156

138157
// keep a reference to the last published usr.id
@@ -266,6 +285,22 @@ public int getRaspTimeouts() {
266285
return raspTimeouts;
267286
}
268287

288+
public boolean isExtendedDataCollection() {
289+
return extendedDataCollection;
290+
}
291+
292+
public void setExtendedDataCollection(boolean extendedDataCollection) {
293+
this.extendedDataCollection = extendedDataCollection;
294+
}
295+
296+
public int getExtendedDataCollectionMaxHeaders() {
297+
return extendedDataCollectionMaxHeaders;
298+
}
299+
300+
public void setExtendedDataCollectionMaxHeaders(int extendedDataCollectionMaxHeaders) {
301+
this.extendedDataCollectionMaxHeaders = extendedDataCollectionMaxHeaders;
302+
}
303+
269304
public WafContext getOrCreateWafContext(
270305
WafHandle wafHandle, boolean createMetrics, boolean isRasp) {
271306
if (createMetrics) {
@@ -964,6 +999,14 @@ public Object getProcessedRequestBody() {
964999
return processedRequestBody;
9651000
}
9661001

1002+
public boolean isProcessedResponseBodySizeExceeded() {
1003+
return processedResponseBodySizeExceeded;
1004+
}
1005+
1006+
public void setProcessedResponseBodySizeExceeded(boolean processedResponseBodySizeExceeded) {
1007+
this.processedResponseBodySizeExceeded = processedResponseBodySizeExceeded;
1008+
}
1009+
9671010
public boolean isRaspMatched() {
9681011
return raspMatched;
9691012
}

‎dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java‎

Lines changed: 45 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import static com.datadog.appsec.event.data.MapDataBundle.Builder.CAPACITY_0_2;
44
import static com.datadog.appsec.event.data.MapDataBundle.Builder.CAPACITY_3_4;
55
import static com.datadog.appsec.event.data.MapDataBundle.Builder.CAPACITY_6_10;
6+
import static com.datadog.appsec.gateway.AppSecRequestContext.AUTHORIZATION_HEADERS;
67
import static com.datadog.appsec.gateway.AppSecRequestContext.DEFAULT_REQUEST_HEADERS_ALLOW_LIST;
78
import static com.datadog.appsec.gateway.AppSecRequestContext.REQUEST_HEADERS_ALLOW_LIST;
89
import static com.datadog.appsec.gateway.AppSecRequestContext.RESPONSE_HEADERS_ALLOW_LIST;
@@ -705,14 +706,9 @@ private Flow<Void> onRequestBodyProcessed(RequestContext ctx_, Object obj) {
705706
obj,
706707
ctx,
707708
() -> {
708-
if (Config.get().isAppSecRaspCollectRequestBody()) {
709-
ctx_.getTraceSegment()
710-
.setTagTop("_dd.appsec.rasp.request_body_size.exceeded", true);
711-
}
709+
ctx.setProcessedResponseBodySizeExceeded(true);
712710
});
713-
if (Config.get().isAppSecRaspCollectRequestBody()) {
714-
ctx.setProcessedRequestBody(converted);
715-
}
711+
ctx.setProcessedRequestBody(converted);
716712
DataBundle bundle = new SingletonDataBundle<>(KnownAddresses.REQUEST_BODY_OBJECT, converted);
717713
try {
718714
GatewayContext gwCtx = new GatewayContext(false);
@@ -885,35 +881,34 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) {
885881
traceSeg.setDataTop("appsec", wrapper);
886882

887883
// Report collected request and response headers based on allow list
888-
boolean collectAll =
889-
Config.get().isAppSecCollectAllHeaders()
890-
// Until redaction is defined we don't want to collect all headers due to risk of
891-
// leaking sensitive data
892-
&& !Config.get().isAppSecHeaderCollectionRedactionEnabled();
884+
boolean collectAll = ctx.isExtendedDataCollection();
893885
writeRequestHeaders(
894-
traceSeg, REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders(), collectAll);
886+
ctx, traceSeg, REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders(), collectAll);
895887
writeResponseHeaders(
896-
traceSeg, RESPONSE_HEADERS_ALLOW_LIST, ctx.getResponseHeaders(), collectAll);
888+
ctx, traceSeg, RESPONSE_HEADERS_ALLOW_LIST, ctx.getResponseHeaders(), collectAll);
897889

898890
// Report collected stack traces
899891
List<StackTraceEvent> stackTraces = ctx.getStackTraces();
900892
if (stackTraces != null && !stackTraces.isEmpty()) {
901893
StackUtils.addStacktraceEventsToMetaStruct(ctx_, METASTRUCT_EXPLOIT, stackTraces);
902894
}
903895

904-
// Report collected parsed request body if there is a RASP event
905-
if (ctx.isRaspMatched() && ctx.getProcessedRequestBody() != null) {
896+
if (ctx.isExtendedDataCollection() && ctx.getProcessedRequestBody() != null) {
906897
ctx_.getOrCreateMetaStructTop(
907898
METASTRUCT_REQUEST_BODY, k -> ctx.getProcessedRequestBody());
899+
if (ctx.isProcessedResponseBodySizeExceeded()) {
900+
traceSeg.setTagTop("_dd.appsec.request_body_size.exceeded", true);
901+
}
908902
}
909903

910904
} else if (hasUserInfo(traceSeg)) {
911905
// Report all collected request headers on user tracking event
912-
writeRequestHeaders(traceSeg, REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders(), false);
906+
writeRequestHeaders(
907+
ctx, traceSeg, REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders(), false);
913908
} else {
914909
// Report minimum set of collected request headers
915910
writeRequestHeaders(
916-
traceSeg, DEFAULT_REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders(), false);
911+
ctx, traceSeg, DEFAULT_REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders(), false);
917912
}
918913
// If extracted any derivatives - commit them
919914
if (!ctx.commitDerivatives(traceSeg)) {
@@ -1026,42 +1021,60 @@ private static boolean hasUserCollectionEvent(final TraceSegment traceSeg) {
10261021
}
10271022

10281023
private static void writeRequestHeaders(
1024+
AppSecRequestContext ctx,
10291025
final TraceSegment traceSeg,
10301026
final Set<String> allowed,
10311027
final Map<String, List<String>> headers,
10321028
final boolean collectAll) {
10331029
writeHeaders(
1034-
traceSeg, "http.request.headers.", "_dd.appsec.request.", allowed, headers, collectAll);
1030+
ctx,
1031+
traceSeg,
1032+
"http.request.headers.",
1033+
"_dd.appsec.request.",
1034+
allowed,
1035+
headers,
1036+
collectAll,
1037+
true);
10351038
}
10361039

10371040
private static void writeResponseHeaders(
1041+
AppSecRequestContext ctx,
10381042
final TraceSegment traceSeg,
10391043
final Set<String> allowed,
10401044
final Map<String, List<String>> headers,
10411045
final boolean collectAll) {
10421046
writeHeaders(
1043-
traceSeg, "http.response.headers.", "_dd.appsec.response.", allowed, headers, collectAll);
1047+
ctx,
1048+
traceSeg,
1049+
"http.response.headers.",
1050+
"_dd.appsec.response.",
1051+
allowed,
1052+
headers,
1053+
collectAll,
1054+
false);
10441055
}
10451056

10461057
private static void writeHeaders(
1058+
AppSecRequestContext ctx,
10471059
final TraceSegment traceSeg,
10481060
final String prefix,
10491061
final String discardedPrefix,
10501062
final Set<String> allowed,
10511063
final Map<String, List<String>> headers,
1052-
final boolean collectAll) {
1064+
final boolean collectAll,
1065+
final boolean checkCookie) {
10531066

10541067
if (headers == null || headers.isEmpty()) {
10551068
return;
10561069
}
10571070

1058-
final int headerLimit = Config.get().getAppsecMaxCollectedHeaders();
1071+
final int headerLimit = ctx.getExtendedDataCollectionMaxHeaders();
10591072
final Set<String> added = new HashSet<>();
10601073
int excluded = 0;
10611074

10621075
// Try to add allowed headers (prioritized)
10631076
for (String name : allowed) {
1064-
if (added.size() >= headerLimit) {
1077+
if (collectAll && added.size() >= headerLimit) {
10651078
break;
10661079
}
10671080
List<String> values = headers.get(name);
@@ -1086,14 +1099,20 @@ private static void writeHeaders(
10861099
excluded++;
10871100
continue;
10881101
}
1089-
1090-
List<String> values = entry.getValue();
1091-
String joined = String.join(",", values);
1102+
String joined;
1103+
if (AUTHORIZATION_HEADERS.contains(name)) {
1104+
joined = String.join(",", "<redacted>");
1105+
} else {
1106+
joined = String.join(",", entry.getValue());
1107+
}
10921108
if (!joined.isEmpty()) {
10931109
traceSeg.setTagTop(prefix + name, joined);
10941110
added.add(name);
10951111
}
10961112
}
1113+
if (checkCookie && !ctx.getCookies().isEmpty()) {
1114+
traceSeg.setTagTop(prefix + "cookie", "<redacted>");
1115+
}
10971116

10981117
if (excluded > 0) {
10991118
traceSeg.setTagTop(discardedPrefix + "header_collection.discarded", excluded);

‎dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigServiceImplSpecification.groovy‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package com.datadog.appsec.config
22

33
import com.datadog.appsec.AppSecSystem
44
import com.datadog.appsec.util.AbortStartupException
5+
import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_EXTENDED_DATA_COLLECTION
56
import datadog.remoteconfig.ConfigurationChangesTypedListener
67
import datadog.remoteconfig.ConfigurationDeserializer
78
import datadog.remoteconfig.ConfigurationEndListener
@@ -289,7 +290,8 @@ class AppSecConfigServiceImplSpecification extends DDSpecification {
289290
| CAPABILITY_ASM_SESSION_FINGERPRINT
290291
| CAPABILITY_ASM_NETWORK_FINGERPRINT
291292
| CAPABILITY_ASM_HEADER_FINGERPRINT
292-
| CAPABILITY_ASM_TRACE_TAGGING_RULES)
293+
| CAPABILITY_ASM_TRACE_TAGGING_RULES
294+
| CAPABILITY_ASM_EXTENDED_DATA_COLLECTION)
293295
0 * poller._
294296

295297
when:
@@ -444,7 +446,8 @@ class AppSecConfigServiceImplSpecification extends DDSpecification {
444446
| CAPABILITY_ASM_SESSION_FINGERPRINT
445447
| CAPABILITY_ASM_NETWORK_FINGERPRINT
446448
| CAPABILITY_ASM_HEADER_FINGERPRINT
447-
| CAPABILITY_ASM_TRACE_TAGGING_RULES)
449+
| CAPABILITY_ASM_TRACE_TAGGING_RULES
450+
| CAPABILITY_ASM_EXTENDED_DATA_COLLECTION)
448451
0 * poller._
449452

450453
when:
@@ -540,7 +543,8 @@ class AppSecConfigServiceImplSpecification extends DDSpecification {
540543
| CAPABILITY_ASM_SESSION_FINGERPRINT
541544
| CAPABILITY_ASM_NETWORK_FINGERPRINT
542545
| CAPABILITY_ASM_HEADER_FINGERPRINT
543-
| CAPABILITY_ASM_TRACE_TAGGING_RULES)
546+
| CAPABILITY_ASM_TRACE_TAGGING_RULES
547+
| CAPABILITY_ASM_EXTENDED_DATA_COLLECTION)
544548
4 * poller.removeListeners(_)
545549
1 * poller.removeConfigurationEndListener(_)
546550
1 * poller.stop()

0 commit comments

Comments
 (0)