Skip to content

Commit 09b77c3

Browse files
jandro996jandro996
committed
WIP
1 parent a33e422 commit 09b77c3

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)