diff --git a/dd-java-agent/appsec/build.gradle b/dd-java-agent/appsec/build.gradle index a9bb2b1b001..34a9c2cc088 100644 --- a/dd-java-agent/appsec/build.gradle +++ b/dd-java-agent/appsec/build.gradle @@ -15,6 +15,7 @@ dependencies { implementation project(':internal-api') implementation project(':communication') implementation project(':telemetry') + implementation project(':dd-trace-core') implementation group: 'io.sqreen', name: 'libsqreen', version: '17.3.0' implementation libs.moshi diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java index ce6dab75ae0..2540be7b42d 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java @@ -57,10 +57,26 @@ public ApiSecuritySamplerImpl( @Override public boolean preSampleRequest(final @Nonnull AppSecRequestContext ctx) { - final String route = ctx.getRoute(); + String route = ctx.getRoute(); + + // If route is absent, use http.endpoint as fallback (RFC-1076) if (route == null) { - return false; + // Don't sample blocked requests - they represent attacks, not valid API endpoints + if (ctx.isWafBlocked()) { + return false; + } + final int statusCode = ctx.getResponseStatus(); + // Don't use endpoint for 404 responses as a failsafe + if (statusCode == 404) { + return false; + } + // Try to get or compute the endpoint + route = ctx.getOrComputeEndpoint(); + if (route == null) { + return false; + } } + final String method = ctx.getMethod(); if (method == null) { return false; diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java index 2b197756697..b0c0c0386a1 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java @@ -120,6 +120,9 @@ public class AppSecRequestContext implements DataBundle, Closeable { private String method; private String savedRawURI; private String route; + private String httpUrl; + private String endpoint; + private boolean endpointComputed = false; private final Map> requestHeaders = new LinkedHashMap<>(); private final Map> responseHeaders = new LinkedHashMap<>(); private volatile Map> collectedCookies; @@ -423,6 +426,45 @@ public void setRoute(String route) { this.route = route; } + public String getHttpUrl() { + return httpUrl; + } + + public void setHttpUrl(String httpUrl) { + this.httpUrl = httpUrl; + } + + /** + * Gets or computes the http.endpoint for this request. The endpoint is computed lazily on first + * access and cached to avoid recomputation. + * + * @return the http.endpoint value, or null if it cannot be computed + */ + public String getOrComputeEndpoint() { + if (!endpointComputed) { + if (httpUrl != null && !httpUrl.isEmpty()) { + try { + endpoint = datadog.trace.core.endpoint.EndpointResolver.computeEndpoint(httpUrl); + } catch (Exception e) { + endpoint = null; + } + } + endpointComputed = true; + } + return endpoint; + } + + /** + * Sets the endpoint directly without computing it. This is useful when the endpoint has already + * been computed elsewhere. + * + * @param endpoint the endpoint value to set + */ + public void setEndpoint(String endpoint) { + this.endpoint = endpoint; + this.endpointComputed = true; + } + public void setKeepOpenForApiSecurityPostProcessing(final boolean flag) { this.keepOpenForApiSecurityPostProcessing = flag; } diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index 174f977ff21..f04c53df2f5 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -949,11 +949,16 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { private boolean maybeSampleForApiSecurity( AppSecRequestContext ctx, IGSpanInfo spanInfo, Map tags) { log.debug("Checking API Security for end of request handler on span: {}", spanInfo.getSpanId()); - // API Security sampling requires http.route tag. + // API Security sampling requires http.route tag or http.url for endpoint inference. final Object route = tags.get(Tags.HTTP_ROUTE); if (route != null) { ctx.setRoute(route.toString()); } + // Pass http.url to enable endpoint inference when route is absent + final Object url = tags.get(Tags.HTTP_URL); + if (url != null) { + ctx.setHttpUrl(url.toString()); + } ApiSecuritySampler requestSampler = requestSamplerSupplier.get(); return requestSampler.preSampleRequest(ctx); } diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/api/security/ApiSecuritySamplerTest.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/api/security/ApiSecuritySamplerTest.groovy index 029d23c5a2f..d5d116640f5 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/api/security/ApiSecuritySamplerTest.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/api/security/ApiSecuritySamplerTest.groovy @@ -79,7 +79,7 @@ class ApiSecuritySamplerTest extends DDSpecification { preSampled3 } - void 'preSampleRequest with null route'() { + void 'preSampleRequest with null route and no URL'() { given: def ctx = createContext(null, 'GET', 200) def sampler = new ApiSecuritySamplerImpl() @@ -91,6 +91,113 @@ class ApiSecuritySamplerTest extends DDSpecification { !preSampled } + void 'preSampleRequest with null route but valid URL uses endpoint fallback'() { + given: + def ctx = createContextWithUrl(null, 'GET', 200, 'http://localhost:8080/api/users/123') + def sampler = new ApiSecuritySamplerImpl() + + when: + def preSampled = sampler.preSampleRequest(ctx) + + then: + preSampled + ctx.getOrComputeEndpoint() != null + ctx.getApiSecurityEndpointHash() != null + } + + void 'preSampleRequest with null route and 404 status does not sample'() { + given: + def ctx = createContextWithUrl(null, 'GET', 404, 'http://localhost:8080/unknown/path') + def sampler = new ApiSecuritySamplerImpl() + + when: + def preSampled = sampler.preSampleRequest(ctx) + + then: + !preSampled + } + + void 'preSampleRequest with null route and blocked request does not sample'() { + given: + def ctx = createContextWithUrl(null, 'GET', 403, 'http://localhost:8080/admin/users') + ctx.setWafBlocked() // Request was blocked by AppSec + def sampler = new ApiSecuritySamplerImpl() + + when: + def preSampled = sampler.preSampleRequest(ctx) + + then: + !preSampled // Blocked requests should not be sampled + } + + void 'preSampleRequest with null route and 403 non-blocked API does sample'() { + given: + def ctx = createContextWithUrl(null, 'GET', 403, 'http://localhost:8080/api/forbidden-resource') + // NOT calling setWafBlocked() - this is a legitimate API that returns 403 + def sampler = new ApiSecuritySamplerImpl() + + when: + def preSampled = sampler.preSampleRequest(ctx) + + then: + preSampled // Legitimate APIs that return 403 should be sampled + ctx.getOrComputeEndpoint() != null + ctx.getApiSecurityEndpointHash() != null + } + + void 'preSampleRequest with null route and blocked request with different status codes does not sample'() { + given: + def ctx200 = createContextWithUrl(null, 'GET', 200, 'http://localhost:8080/attack') + ctx200.setWafBlocked() + def ctx500 = createContextWithUrl(null, 'GET', 500, 'http://localhost:8080/attack') + ctx500.setWafBlocked() + def sampler = new ApiSecuritySamplerImpl() + + when: + def preSampled200 = sampler.preSampleRequest(ctx200) + def preSampled500 = sampler.preSampleRequest(ctx500) + + then: + !preSampled200 // Blocked requests should not be sampled regardless of status code + !preSampled500 + } + + void 'second request with same endpoint is not sampled'() { + given: + def ctx1 = createContextWithUrl(null, 'GET', 200, 'http://localhost:8080/api/users/123') + def ctx2 = createContextWithUrl(null, 'GET', 200, 'http://localhost:8080/api/users/456') + def sampler = new ApiSecuritySamplerImpl() + + when: + def preSampled1 = sampler.preSampleRequest(ctx1) + ctx1.setKeepOpenForApiSecurityPostProcessing(true) + def sampled1 = sampler.sampleRequest(ctx1) + sampler.releaseOne() + + then: + preSampled1 + sampled1 + + when: + def preSampled2 = sampler.preSampleRequest(ctx2) + + then: + !preSampled2 // Same endpoint pattern, so not sampled + } + + void 'endpoint is computed only once'() { + given: + def ctx = createContextWithUrl(null, 'GET', 200, 'http://localhost:8080/api/users/123') + + when: + def endpoint1 = ctx.getOrComputeEndpoint() + def endpoint2 = ctx.getOrComputeEndpoint() + + then: + endpoint1 != null + endpoint1 == endpoint2 + } + void 'preSampleRequest with null method'() { given: def ctx = createContext('route1', null, 200) @@ -371,4 +478,13 @@ class ApiSecuritySamplerTest extends DDSpecification { ctx.setResponseStatus(statusCode) ctx } + + private static AppSecRequestContext createContextWithUrl(final String route, final String method, int statusCode, String url) { + final AppSecRequestContext ctx = new AppSecRequestContext() + ctx.setRoute(route) + ctx.setMethod(method) + ctx.setResponseStatus(statusCode) + ctx.setHttpUrl(url) + ctx + } }