8000 Extended appsec request/response headers collection by jandro996 · Pull Request #8724 · DataDog/dd-trace-java · GitHub
[go: up one dir, main page]
Skip to content

Extended appsec request/response headers collection < 8000 span class="f1-light color-fg-muted">#8724

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 21, 2025
Merged

Extended appsec request/response headers collection #8724

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
09b77c3
WIP
jandro996 Apr 22, 2025
97433cd
Update dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConf…
jandro996 May 20, 2025
e6cb2dd
Update dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConf…
jandro996 May 20, 2025
37a5094
Update dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConf…
jandro996 May 20, 2025
bcac4de
Merge branch 'master' into alejandro.gonzalez/WaPo-headers
jandro996 May 21, 2025
4e995e9
fix smoke tests
jandro996 May 21, 2025
1989174
rollback properties changes
jandro996 May 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
F 8000 ailed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 71 additions & 19 deletions dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -707,8 +707,15 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) {
traceSeg.setDataTop("appsec", wrapper);

// Report collected request and response headers based on allow list
writeRequestHeaders(traceSeg, REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders());
writeResponseHeaders(traceSeg, RESPONSE_HEADERS_ALLOW_LIST, ctx.getResponseHeaders());
boolean collectAll =
Config.get().isAppSecCollectAllHeaders()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

So can we also clarify in the PR description that, right now, if redaction is enabled, we do not collect headers at all?

Copy link
Member Author
@jandro996 jandro996 May 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! I'm going to clarify this. Until a new RFC establish how to deal with redaction on those cases we prefer to avoid collection of potential sensitive data. So long story sort to enable this feature we need DD_APPSEC_COLLECT_ALL_HEADERS = true and DD_APPSEC_HEADER_COLLECTION_REDACTION_ENABLED = false

// Until redaction is defined we don't want to collect all headers due to risk of
// leaking sensitive data
&& !Config.get().isAppSecHeaderCollectionRedactionEnabled();
writeRequestHeaders(
traceSeg, REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders(), collectAll);
writeResponseHeaders(
traceSeg, RESPONSE_HEADERS_ALLOW_LIST, ctx.getResponseHeaders(), collectAll);

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

} else if (hasUserInfo(traceSeg)) {
// Report all collected request headers on user tracking event
writeRequestHeaders(traceSeg, REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders());
writeRequestHeaders(traceSeg, REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders(), false);
} else {
// Report minimum set of collected request headers
writeRequestHeaders(traceSeg, DEFAULT_REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders());
writeRequestHeaders(
traceSeg, DEFAULT_REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders(), false);
}
// If extracted any derivatives - commit them
if (!ctx.commitDerivatives(traceSeg)) {
Expand Down Expand Up @@ -835,32 +843,76 @@ private static boolean hasUserCollectionEvent(final TraceSegment traceSeg) {
private static void writeRequestHeaders(
final TraceSegment traceSeg,
final Set<String> allowed,
final Map<String, List<String>> headers) {
writeHeaders(traceSeg, "http.request.headers.", allowed, headers);
final Map<String, List<String>> headers,
final boolean collectAll) {
writeHeaders(
traceSeg, "http.request.headers.", "_dd.appsec.request.", allowed, headers, collectAll);
}

private static void writeResponseHeaders(
final TraceSegment traceSeg,
final Set<String> allowed,
final Map<String, List<String>> headers) {
writeHeaders(traceSeg, "http.response.headers.", allowed, headers);
final Map<String, List<String>> headers,
final boolean collectAll) {
writeHeaders(
traceSeg, "http.response.headers.", "_dd.appsec.response.", allowed, headers, collectAll);
}

private static void writeHeaders(
final TraceSegment traceSeg,
final String prefix,
final String discardedPrefix,
final Set<String> allowed,
final Map<String, List<String>> headers) {
if (headers != null) {
headers.forEach(
(name, value) -> {
if (allowed.contains(name)) {
String v = String.join(",", value);
if (!v.isEmpty()) {
traceSeg.setTagTop(prefix + name, v);
}
}
});
final Map<String, List<String>> headers,
final boolean collectAll) {

if (headers == null || headers.isEmpty()) {
return;
}

final int headerLimit = Config.get().getAppsecMaxCollectedHeaders();
final Set<String> added = new HashSet<>();
int excluded = 0;

// Try to add allowed headers (prioritized)
for (String name : allowed) {
if (added.size() >= headerLimit) {
break;
}
List<String> values = headers.get(name);
if (values != null) {
String joined = String.join(",", values);
if (!joined.isEmpty()) {
traceSeg.setTagTop(prefix + name, joined);
added.add(name);
}
}
}

if (collectAll) {
// Add other headers (non-allowed) until total reaches the limit
for (Map.Entry<String, List<String>> entry : headers.entrySet()) {
String name = entry.getKey();
if (added.contains(name)) {
continue;
}

if (added.size() >= headerLimit) {
excluded++;
continue;
}

List<String> values = entry.getValue();
String joined = String.join(",", values);
if (!joined.isEmpty()) {
traceSeg.setTagTop(prefix + name, joined);
added.add(name);
}
}

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

Expand Down
92 changes: 92 additions & 0 deletions ...agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1222,4 +1222,96 @@ class GatewayBridgeSpecification extends DDSpecification {
1 * traceSegment.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM)
}


void 'test default writeRequestHeaders'(){
given:
def allowedHeaders = ['x-allowed-header', 'x-multiple-allowed-header', 'x-always-included'] as Set
def headers = [
'x-allowed-header' : ['value1'],
'x-multiple-allowed-header' : ['value1A', 'value1B'],
'x-other-header-1' : ['value2'],
'x-other-header-2' : ['value3'],
'x-other-header-3' : ['value4']
]

when:
GatewayBridge.writeRequestHeaders(traceSegment, allowedHeaders, headers, false)

then:
1 * traceSegment.setTagTop('http.request.headers.x-allowed-header', 'value1')
1 * traceSegment.setTagTop('http.request.headers.x-multiple-allowed-header', 'value1A,value1B')
0 * traceSegment.setTagTop(_, _)
}

void 'test default writeResponseHeaders'(){
given:
def allowedHeaders = ['x-allowed-header', 'x-multiple-allowed-header', 'x-always-included'] as Set
def headers = [
'x-allowed-header' : ['value1'],
'x-multiple-allowed-header' : ['value1A', 'value1B'],
'x-other-header-1' : ['value2'],
'x-other-header-2' : ['value3'],
'x-other-header-3' : ['value4']
]

when:
GatewayBridge.writeResponseHeaders(traceSegment, allowedHeaders, headers, false)

then:
1 * traceSegment.setTagTop('http.response.headers.x-allowed-header', 'value1')
1 * traceSegment.setTagTop('http.response.headers.x-multiple-allowed-header', 'value1A,value1B')
0 * traceSegment.setTagTop(_, _)
}

void 'test writeRequestHeaders collecting all headers '(){
setup:
injectEnvConfig('DD_APPSEC_MAX_COLLECTED_HEADERS', '4')

def allowedHeaders = ['x-allowed-header', 'x-multiple-allowed-header', 'x-always-included'] as Set
def headers = [
'x-allowed-header' : ['value1'],
'x-multiple-allowed-header' : ['value1A', 'value1B'],
'x-other-header-1' : ['value2'],
'x-other-header-2' : ['value3'],
'x-other-header-3' : ['value4']
]

when:
GatewayBridge.writeRequestHeaders(traceSegment, allowedHeaders, headers, true)

then:
1 * traceSegment.setTagTop('http.request.headers.x-allowed-header', 'value1')
1 * traceSegment.setTagTop('http.request.headers.x-multiple-allowed-header', 'value1A,value1B')
1 * traceSegment.setTagTop('http.request.headers.x-other-header-1', 'value2')
1 * traceSegment.setTagTop('http.request.headers.x-other-header-2', 'value3')
1 * traceSegment.setTagTop('_dd.appsec.request.header_collection.discarded', 1)
0 * traceSegment.setTagTop(_, _)
}

void 'test writeResponseHeaders collecting all headers '(){
setup:
injectEnvConfig('DD_APPSEC_COLLECT_ALL_HEADERS' , 'true')
injectEnvConfig('DD_APPSEC_MAX_COLLECTED_HEADERS', '4')

def allowedHeaders = ['x-allowed-header', 'x-multiple-allowed-header', 'x-always-included'] as Set
def headers = [
'x-allowed-header' : ['value1'],
'x-multiple-allowed-header' : ['value1A', 'value1B'],
'x-other-header-1' : ['value2'],
'x-other-header-2' : ['value3'],
'x-other-header-3' : ['value4']
]

when:
GatewayBridge.writeResponseHeaders(traceSegment, allowedHeaders, headers, true)

then:
1 * traceSegment.setTagTop('http.response.headers.x-allowed-header', 'value1')
1 * traceSegment.setTagTop('http.response.headers.x-multiple-allowed-header', 'value1A,value1B')
1 * traceSegment.setTagTop('http.response.headers.x-other-header-1', 'value2')
1 * traceSegment.setTagTop('http.response.headers.x-other-header-2', 'value3')
1 * traceSegment.setTagTop('_dd.appsec.response.header_collection.discarded', 1)
0 * traceSegment.setTagTop(_, _)
}

}
22 changes: 22 additions & 0 deletions ...pringboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.apache.http.client.methods.HttpGet;
import org.apache.http.impl.client.DefaultHttpClient;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
Expand Down Expand Up @@ -210,6 +211,27 @@ public ResponseEntity<String> apiSecuritySampling(@PathVariable("status_code") i
return ResponseEntity.status(statusCode).body("EXECUTED");
}

@GetMapping("/custom-headers")
public ResponseEntity<String> customHeaders() {
HttpHeaders headers = new HttpHeaders();
headers.add("X-Test-Header-1", "value1");
headers.add("X-Test-Header-2", "value2");
headers.add("X-Test-Header-3", "value3");
headers.add("X-Test-Header-4", "value4");
headers.add("X-Test-Header-5", "value5");
return new ResponseEntity<>("Custom headers added", headers, HttpStatus.OK);
}

@GetMapping("/exceedResponseHeaders")
public ResponseEntity<String> exceedResponseHeaders() {
HttpHeaders headers = new HttpHeaders();
for (int i = 1; i <= 50; i++) {
headers.add("X-Test-Header-" + i, "value" + i);
}
headers.add("content-language", "en-US");
return new ResponseEntity<>("Custom headers added", headers, HttpStatus.OK);
}

private void withProcess(final Operation<Process> op) {
Process process = null;
try {
Expand Down
Loading
0