Skip to content

Commit f2779e4

Browse files
committed
WIP
1 parent 8b96d89 commit f2779e4

File tree

5 files changed

+184
-4
lines changed

5 files changed

+184
-4
lines changed

dd-java-agent/appsec/build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ dependencies {
1616
implementation project(':communication')
1717
implementation project(':products:metrics:metrics-api')
1818
implementation project(':telemetry')
19+
implementation project(':dd-trace-core')
1920
implementation group: 'io.sqreen', name: 'libsqreen', version: '17.3.0'
2021
implementation libs.moshi
2122

dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,26 @@ public ApiSecuritySamplerImpl(
5757

5858
@Override
5959
public boolean preSampleRequest(final @Nonnull AppSecRequestContext ctx) {
60-
final String route = ctx.getRoute();
60+
String route = ctx.getRoute();
61+
62+
// If route is absent, use http.endpoint as fallback (RFC-1076)
6163
if (route == null) {
62-
return false;
64+
// Don't sample blocked requests - they represent attacks, not valid API endpoints
65+
if (ctx.isWafBlocked()) {
66+
return false;
67+
}
68+
final int statusCode = ctx.getResponseStatus();
69+
// Don't use endpoint for 404 responses as a failsafe
70+
if (statusCode == 404) {
71+
return false;
72+
}
73+
// Try to get or compute the endpoint
74+
route = ctx.getOrComputeEndpoint();
75+
if (route == null) {
76+
return false;
77+
}
6378
}
79+
6480
final String method = ctx.getMethod();
6581
if (method == null) {
6682
return false;

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,9 @@ public class AppSecRequestContext implements DataBundle, Closeable {
120120
private String method;
121121
private String savedRawURI;
122122
private String route;
123+
private String httpUrl;
124+
private String endpoint;
125+
private boolean endpointComputed = false;
123126
private final Map<String, List<String>> requestHeaders = new LinkedHashMap<>();
124127
private final Map<String, List<String>> responseHeaders = new LinkedHashMap<>();
125128
private volatile Map<String, List<String>> collectedCookies;
@@ -423,6 +426,45 @@ public void setRoute(String route) {
423426
this.route = route;
424427
}
425428

429+
public String getHttpUrl() {
430+
return httpUrl;
431+
}
432+
433+
public void setHttpUrl(String httpUrl) {
434+
this.httpUrl = httpUrl;
435+
}
436+
437+
/**
438+
* Gets or computes the http.endpoint for this request. The endpoint is computed lazily on first
439+
* access and cached to avoid recomputation.
440+
*
441+
* @return the http.endpoint value, or null if it cannot be computed
442+
*/
443+
public String getOrComputeEndpoint() {
444+
if (!endpointComputed) {
445+
if (httpUrl != null && !httpUrl.isEmpty()) {
446+
try {
447+
endpoint = datadog.trace.core.endpoint.EndpointResolver.computeEndpoint(httpUrl);
448+
} catch (Exception e) {
449+
endpoint = null;
450+
}
451+
}
452+
endpointComputed = true;
453+
}
454+
return endpoint;
455+
}
456+
457+
/**
458+
* Sets the endpoint directly without computing it. This is useful when the endpoint has already
459+
* been computed elsewhere.
460+
*
461+
* @param endpoint the endpoint value to set
462+
*/
463+
public void setEndpoint(String endpoint) {
464+
this.endpoint = endpoint;
465+
this.endpointComputed = true;
466+
}
467+
426468
public void setKeepOpenForApiSecurityPostProcessing(final boolean flag) {
427469
this.keepOpenForApiSecurityPostProcessing = flag;
428470
}

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -949,11 +949,16 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) {
949949
private boolean maybeSampleForApiSecurity(
950950
AppSecRequestContext ctx, IGSpanInfo spanInfo, Map<String, Object> tags) {
951951
log.debug("Checking API Security for end of request handler on span: {}", spanInfo.getSpanId());
952-
// API Security sampling requires http.route tag.
952+
// API Security sampling requires http.route tag or http.url for endpoint inference.
953953
final Object route = tags.get(Tags.HTTP_ROUTE);
954954
if (route != null) {
955955
ctx.setRoute(route.toString());
956956
}
957+
// Pass http.url to enable endpoint inference when route is absent
958+
final Object url = tags.get(Tags.HTTP_URL);
959+
if (url != null) {
960+
ctx.setHttpUrl(url.toString());
961+
}
957962
ApiSecuritySampler requestSampler = requestSamplerSupplier.get();
958963
return requestSampler.preSampleRequest(ctx);
959964
}

dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/api/security/ApiSecuritySamplerTest.groovy

Lines changed: 117 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ class ApiSecuritySamplerTest extends DDSpecification {
7979
preSampled3
8080
}
8181

82-
void 'preSampleRequest with null route'() {
82+
void 'preSampleRequest with null route and no URL'() {
8383
given:
8484
def ctx = createContext(null, 'GET', 200)
8585
def sampler = new ApiSecuritySamplerImpl()
@@ -91,6 +91,113 @@ class ApiSecuritySamplerTest extends DDSpecification {
9191
!preSampled
9292
}
9393

94+
void 'preSampleRequest with null route but valid URL uses endpoint fallback'() {
95+
given:
96+
def ctx = createContextWithUrl(null, 'GET', 200, 'http://localhost:8080/api/users/123')
97+
def sampler = new ApiSecuritySamplerImpl()
98+
99+
when:
100+
def preSampled = sampler.preSampleRequest(ctx)
101+
102+
then:
103+
preSampled
104+
ctx.getOrComputeEndpoint() != null
105+
ctx.getApiSecurityEndpointHash() != null
106+
}
107+
108+
void 'preSampleRequest with null route and 404 status does not sample'() {
109+
given:
110+
def ctx = createContextWithUrl(null, 'GET', 404, 'http://localhost:8080/unknown/path')
111+
def sampler = new ApiSecuritySamplerImpl()
112+
113+
when:
114+
def preSampled = sampler.preSampleRequest(ctx)
115+
116+
then:
117+
!preSampled
118+
}
119+
120+
void 'preSampleRequest with null route and blocked request does not sample'() {
121+
given:
122+
def ctx = createContextWithUrl(null, 'GET', 403, 'http://localhost:8080/admin/users')
123+
ctx.setWafBlocked() // Request was blocked by AppSec
124+
def sampler = new ApiSecuritySamplerImpl()
125+
126+
when:
127+
def preSampled = sampler.preSampleRequest(ctx)
128+
129+
then:
130+
!preSampled // Blocked requests should not be sampled
131+
}
132+
133+
void 'preSampleRequest with null route and 403 non-blocked API does sample'() {
134+
given:
135+
def ctx = createContextWithUrl(null, 'GET', 403, 'http://localhost:8080/api/forbidden-resource')
136+
// NOT calling setWafBlocked() - this is a legitimate API that returns 403
137+
def sampler = new ApiSecuritySamplerImpl()
138+
139+
when:
140+
def preSampled = sampler.preSampleRequest(ctx)
141+
142+
then:
143+
preSampled // Legitimate APIs that return 403 should be sampled
144+
ctx.getOrComputeEndpoint() != null
145+
ctx.getApiSecurityEndpointHash() != null
146+
}
147+
148+
void 'preSampleRequest with null route and blocked request with different status codes does not sample'() {
149+
given:
150+
def ctx200 = createContextWithUrl(null, 'GET', 200, 'http://localhost:8080/attack')
151+
ctx200.setWafBlocked()
152+
def ctx500 = createContextWithUrl(null, 'GET', 500, 'http://localhost:8080/attack')
153+
ctx500.setWafBlocked()
154+
def sampler = new ApiSecuritySamplerImpl()
155+
156+
when:
157+
def preSampled200 = sampler.preSampleRequest(ctx200)
158+
def preSampled500 = sampler.preSampleRequest(ctx500)
159+
160+
then:
161+
!preSampled200 // Blocked requests should not be sampled regardless of status code
162+
!preSampled500
163+
}
164+
165+
void 'second request with same endpoint is not sampled'() {
166+
given:
167+
def ctx1 = createContextWithUrl(null, 'GET', 200, 'http://localhost:8080/api/users/123')
168+
def ctx2 = createContextWithUrl(null, 'GET', 200, 'http://localhost:8080/api/users/456')
169+
def sampler = new ApiSecuritySamplerImpl()
170+
171+
when:
172+
def preSampled1 = sampler.preSampleRequest(ctx1)
173+
ctx1.setKeepOpenForApiSecurityPostProcessing(true)
174+
def sampled1 = sampler.sampleRequest(ctx1)
175+
sampler.releaseOne()
176+
177+
then:
178+
preSampled1
179+
sampled1
180+
181+
when:
182+
def preSampled2 = sampler.preSampleRequest(ctx2)
183+
184+
then:
185+
!preSampled2 // Same endpoint pattern, so not sampled
186+
}
187+
188+
void 'endpoint is computed only once'() {
189+
given:
190+
def ctx = createContextWithUrl(null, 'GET', 200, 'http://localhost:8080/api/users/123')
191+
192+
when:
193+
def endpoint1 = ctx.getOrComputeEndpoint()
194+
def endpoint2 = ctx.getOrComputeEndpoint()
195+
196+
then:
197+
endpoint1 != null
198+
endpoint1 == endpoint2
199+
}
200+
94201
void 'preSampleRequest with null method'() {
95202
given:
96203
def ctx = createContext('route1', null, 200)
@@ -371,4 +478,13 @@ class ApiSecuritySamplerTest extends DDSpecification {
371478
ctx.setResponseStatus(statusCode)
372479
ctx
373480
}
481+
482+
private static AppSecRequestContext createContextWithUrl(final String route, final String method, int statusCode, String url) {
483+
final AppSecRequestContext ctx = new AppSecRequestContext()
484+
ctx.setRoute(route)
485+
ctx.setMethod(method)
486+
ctx.setResponseStatus(statusCode)
487+
ctx.setHttpUrl(url)
488+
ctx
489+
}
374490
}

0 commit comments

Comments
 (0)