Skip to content

Commit 9f23747

Browse files
jandro996jandro996
authored
Extended appsec request/response headers collection (#8724)
What Does This Do Adds the DD_APPSEC_COLLECT_ALL_HEADERS flag, which enables collection of all request and response headers. This feature is disabled by default. Adds the DD_APPSEC_HEADER_COLLECTION_REDACTION_ENABLED flag, which enabled header redaction. This feature is true by deafult. (The redaction is out of the scope, right now we only want to collect the headers without redaction) To enable this feature we need DD_APPSEC_COLLECT_ALL_HEADERS = true and DD_APPSEC_HEADER_COLLECTION_REDACTION_ENABLED = false ( a future RFC should establish how to deal with redaction) Introduces the DD_APPSEC_MAX_COLLECTED_HEADERS setting to limit the maximum number of headers collected. Updates the writeHeaders logic to collect all headers when DD_APPSEC_COLLECT_ALL_HEADERS is enabled. Allowed headers are prioritized and must be collected if present. If the number of headers exceeds DD_APPSEC_MAX_COLLECTED_HEADERS, the following tags are added to the span indicating the number of discarded headers: dd.appsec.request.header_collection.discarded dd.appsec.response.header_collection.discarded
1 parent e8eddc2 commit 9f23747

File tree

8 files changed

+366
-19
lines changed

8 files changed

+366
-19
lines changed

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

Lines changed: 71 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -707,8 +707,15 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) {
707707
traceSeg.setDataTop("appsec", wrapper);
708708

709709
// Report collected request and response headers based on allow list
710-
writeRequestHeaders(traceSeg, REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders());
711-
writeResponseHeaders(traceSeg, RESPONSE_HEADERS_ALLOW_LIST, ctx.getResponseHeaders());
710+
boolean collectAll =
711+
Config.get().isAppSecCollectAllHeaders()
712+
// Until redaction is defined we don't want to collect all headers due to risk of
713+
// leaking sensitive data
714+
&& !Config.get().isAppSecHeaderCollectionRedactionEnabled();
715+
writeRequestHeaders(
716+
traceSeg, REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders(), collectAll);
717+
writeResponseHeaders(
718+
traceSeg, RESPONSE_HEADERS_ALLOW_LIST, ctx.getResponseHeaders(), collectAll);
712719

713720
// Report collected stack traces
714721
List<StackTraceEvent> stackTraces = ctx.getStackTraces();
@@ -718,10 +725,11 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) {
718725

719726
} else if (hasUserInfo(traceSeg)) {
720727
// Report all collected request headers on user tracking event
721-
writeRequestHeaders(traceSeg, REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders());
728+
writeRequestHeaders(traceSeg, REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders(), false);
722729
} else {
723730
// Report minimum set of collected request headers
724-
writeRequestHeaders(traceSeg, DEFAULT_REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders());
731+
writeRequestHeaders(
732+
traceSeg, DEFAULT_REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders(), false);
725733
}
726734
// If extracted any derivatives - commit them
727735
if (!ctx.commitDerivatives(traceSeg)) {
@@ -835,32 +843,76 @@ private static boolean hasUserCollectionEvent(final TraceSegment traceSeg) {
835843
private static void writeRequestHeaders(
836844
final TraceSegment traceSeg,
837845
final Set<String> allowed,
838-
final Map<String, List<String>> headers) {
839-
writeHeaders(traceSeg, "http.request.headers.", allowed, headers);
846+
final Map<String, List<String>> headers,
847+
final boolean collectAll) {
848+
writeHeaders(
849+
traceSeg, "http.request.headers.", "_dd.appsec.request.", allowed, headers, collectAll);
840850
}
841851

842852
private static void writeResponseHeaders(
843853
final TraceSegment traceSeg,
844854
final Set<String> allowed,
845-
final Map<String, List<String>> headers) {
846-
writeHeaders(traceSeg, "http.response.headers.", allowed, headers);
855+
final Map<String, List<String>> headers,
856+
final boolean collectAll) {
857+
writeHeaders(
858+
traceSeg, "http.response.headers.", "_dd.appsec.response.", allowed, headers, collectAll);
847859
}
848860

849861
private static void writeHeaders(
850862
final TraceSegment traceSeg,
851863
final String prefix,
864+
final String discardedPrefix,
852865
final Set<String> allowed,
853-
final Map<String, List<String>> headers) {
854-
if (headers != null) {
855-
headers.forEach(
856-
(name, value) -> {
857-
if (allowed.contains(name)) {
858-
String v = String.join(",", value);
859-
if (!v.isEmpty()) {
860-
traceSeg.setTagTop(prefix + name, v);
861-
}
862-
}
863-
});
866+
final Map<String, List<String>> headers,
867+
final boolean collectAll) {
868+
869+
if (headers == null || headers.isEmpty()) {
870+
return;
871+
}
872+
873+
final int headerLimit = Config.get().getAppsecMaxCollectedHeaders();
874+
final Set<String> added = new HashSet<>();
875+
int excluded = 0;
876+
877+
// Try to add allowed headers (prioritized)
878+
for (String name : allowed) {
879+
if (added.size() >= headerLimit) {
880+
break;
881+
}
882+
List<String> values = headers.get(name);
883+
if (values != null) {
884+
String joined = String.join(",", values);
885+
if (!joined.isEmpty()) {
886+
traceSeg.setTagTop(prefix + name, joined);
887+
added.add(name);
888+
}
889+
}
890+
}
891+
892+
if (collectAll) {
893+
// Add other headers (non-allowed) until total reaches the limit
894+
for (Map.Entry<String, List<String>> entry : headers.entrySet()) {
895+
String name = entry.getKey();
896+
if (added.contains(name)) {
897+
continue;
898+
}
899+
900+
if (added.size() >= headerLimit) {
901+
excluded++;
902+
continue;
903+
}
904+
905+
List<String> values = entry.getValue();
906+
String joined = String.join(",", values);
907+
if (!joined.isEmpty()) {
908+
traceSeg.setTagTop(prefix + name, joined);
909+
added.add(name);
910+
}
911+
}
912+
913+
if (excluded > 0) {
914+
traceSeg.setTagTop(discardedPrefix + "header_collection.discarded", excluded);
915+
}
864916
}
865917
}
866918

‎dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1222,4 +1222,96 @@ class GatewayBridgeSpecification extends DDSpecification {
12221222
1 * traceSegment.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM)
12231223
}
12241224
1225+
1226+
void 'test default writeRequestHeaders'(){
1227+
given:
1228+
def allowedHeaders = ['x-allowed-header', 'x-multiple-allowed-header', 'x-always-included'] as Set
1229+
def headers = [
1230+
'x-allowed-header' : ['value1'],
1231+
'x-multiple-allowed-header' : ['value1A', 'value1B'],
1232+
'x-other-header-1' : ['value2'],
1233+
'x-other-header-2' : ['value3'],
1234+
'x-other-header-3' : ['value4']
1235+
]
1236+
1237+
when:
1238+
GatewayBridge.writeRequestHeaders(traceSegment, allowedHeaders, headers, false)
1239+
1240+
then:
1241+
1 * traceSegment.setTagTop('http.request.headers.x-allowed-header', 'value1')
1242+
1 * traceSegment.setTagTop('http.request.headers.x-multiple-allowed-header', 'value1A,value1B')
1243+
0 * traceSegment.setTagTop(_, _)
1244+
}
1245+
1246+
void 'test default writeResponseHeaders'(){
1247+
given:
1248+
def allowedHeaders = ['x-allowed-header', 'x-multiple-allowed-header', 'x-always-included'] as Set
1249+
def headers = [
1250+
'x-allowed-header' : ['value1'],
1251+
'x-multiple-allowed-header' : ['value1A', 'value1B'],
1252+
'x-other-header-1' : ['value2'],
1253+
'x-other-header-2' : ['value3'],
1254+
'x-other-header-3' : ['value4']
1255+
]
1256+
1257+
when:
1258+
GatewayBridge.writeResponseHeaders(traceSegment, allowedHeaders, headers, false)
1259+
1260+
then:
1261+
1 * traceSegment.setTagTop('http.response.headers.x-allowed-header', 'value1')
1262+
1 * traceSegment.setTagTop('http.response.headers.x-multiple-allowed-header', 'value1A,value1B')
1263+
0 * traceSegment.setTagTop(_, _)
1264+
}
1265+
1266+
void 'test writeRequestHeaders collecting all headers '(){
1267+
setup:
1268+
injectEnvConfig('DD_APPSEC_MAX_COLLECTED_HEADERS', '4')
1269+
1270+
def allowedHeaders = ['x-allowed-header', 'x-multiple-allowed-header', 'x-always-included'] as Set
1271+
def headers = [
1272+
'x-allowed-header' : ['value1'],
1273+
'x-multiple-allowed-header' : ['value1A', 'value1B'],
1274+
'x-other-header-1' : ['value2'],
1275+
'x-other-header-2' : ['value3'],
1276+
'x-other-header-3' : ['value4']
1277+
]
1278+
1279+
when:
1280+
GatewayBridge.writeRequestHeaders(traceSegment, allowedHeaders, headers, true)
1281+
1282+
then:
1283+
1 * traceSegment.setTagTop('http.request.headers.x-allowed-header', 'value1')
1284+
1 * traceSegment.setTagTop('http.request.headers.x-multiple-allowed-header', 'value1A,value1B')
1285+
1 * traceSegment.setTagTop('http.request.headers.x-other-header-1', 'value2')
1286+
1 * traceSegment.setTagTop('http.request.headers.x-other-header-2', 'value3')
1287+
1 * traceSegment.setTagTop('_dd.appsec.request.header_collection.discarded', 1)
1288+
0 * traceSegment.setTagTop(_, _)
1289+
}
1290+
1291+
void 'test writeResponseHeaders collecting all headers '(){
1292+
setup:
1293+
injectEnvConfig('DD_APPSEC_COLLECT_ALL_HEADERS' , 'true')
1294+
injectEnvConfig('DD_APPSEC_MAX_COLLECTED_HEADERS', '4')
1295+
1296+
def allowedHeaders = ['x-allowed-header', 'x-multiple-allowed-header', 'x-always-included'] as Set
1297+
def headers = [
1298+
'x-allowed-header' : ['value1'],
1299+
'x-multiple-allowed-header' : ['value1A', 'value1B'],
1300+
'x-other-header-1' : ['value2'],
1301+
'x-other-header-2' : ['value3'],
1302+
'x-other-header-3' : ['value4']
1303+
]
1304+
1305+
when:
1306+
GatewayBridge.writeResponseHeaders(traceSegment, allowedHeaders, headers, true)
1307+
1308+
then:
1309+
1 * traceSegment.setTagTop('http.response.headers.x-allowed-header', 'value1')
1310+
1 * traceSegment.setTagTop('http.response.headers.x-multiple-allowed-header', 'value1A,value1B')
1311+
1 * traceSegment.setTagTop('http.response.headers.x-other-header-1', 'value2')
1312+
1 * traceSegment.setTagTop('http.response.headers.x-other-header-2', 'value3')
1313+
1 * traceSegment.setTagTop('_dd.appsec.response.header_collection.discarded', 1)
1314+
0 * traceSegment.setTagTop(_, _)
1315+
}
1316+
12251317
}

‎dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.apache.http.client.methods.HttpGet;
1717
import org.apache.http.impl.client.DefaultHttpClient;
1818
import org.springframework.beans.factory.annotation.Autowired;
19+
import org.springframework.http.HttpHeaders;
1920
import org.springframework.http.HttpStatus;
2021
import org.springframework.http.ResponseEntity;
2122
import org.springframework.web.bind.annotation.GetMapping;
@@ -210,6 +211,27 @@ public ResponseEntity<String> apiSecuritySampling(@PathVariable("status_code") i
210211
return ResponseEntity.status(statusCode).body("EXECUTED");
211212
}
212213

214+
@GetMapping("/custom-headers")
215+
public ResponseEntity<String> customHeaders() {
216+
HttpHeaders headers = new HttpHeaders();
217+
headers.add("X-Test-Header-1", "value1");
218+
headers.add("X-Test-Header-2", "value2");
219+
headers.add("X-Test-Header-3", "value3");
220+
headers.add("X-Test-Header-4", "value4");
221+
headers.add("X-Test-Header-5", "value5");
222+
return new ResponseEntity<>("Custom headers added", headers, HttpStatus.OK);
223+
}
224+
225+
@GetMapping("/exceedResponseHeaders")
226+
public ResponseEntity<String> exceedResponseHeaders() {
227+
HttpHeaders headers = new HttpHeaders();
228+
for (int i = 1; i <= 50; i++) {
229+
headers.add("X-Test-Header-" + i, "value" + i);
230+
}
231+
headers.add("content-language", "en-US");
232+
return new ResponseEntity<>("Custom headers added", headers, HttpStatus.OK);
233+
}
234+
213235
private void withProcess(final Operation<Process> op) {
214236
Process process = null;
215237
try {

0 commit comments

Comments
 (0)