diff --git a/dd-java-agent/appsec/build.gradle b/dd-java-agent/appsec/build.gradle index 879560d0c03..8a375b2541e 100644 --- a/dd-java-agent/appsec/build.gradle +++ b/dd-java-agent/appsec/build.gradle @@ -16,6 +16,7 @@ dependencies { implementation project(':communication') implementation project(':products:metrics:metrics-api') 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 11162674339..419c6263a20 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 @@ -13,6 +13,7 @@ import datadog.trace.api.Config; import datadog.trace.api.http.StoredBodySupplier; import datadog.trace.api.internal.TraceSegment; +import datadog.trace.core.endpoint.EndpointResolver; import datadog.trace.util.Numbers; import datadog.trace.util.stacktrace.StackTraceEvent; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -121,6 +122,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; @@ -424,6 +428,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 = 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..c7a06886a31 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) @@ -364,6 +471,155 @@ class ApiSecuritySamplerTest extends DDSpecification { sampler.accessMap.get(hash) == 0L // Still has the value from preSampleRequest } + // RFC-1076: Verify endpoint is computed and used for sampling but NOT set as a context field for tagging + void 'endpoint computed for sampling is stored internally but not exposed as tag'() { + 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 + // Endpoint was computed and used for the hash + ctx.getApiSecurityEndpointHash() != null + + // Endpoint is available via getOrComputeEndpoint (cached) + def endpoint = ctx.getOrComputeEndpoint() + endpoint != null + endpoint == '/api/users/{param:int}' + + // Verify endpoint is NOT transferred to any tag-like structure in AppSecRequestContext + // AppSecRequestContext doesn't have a method to expose endpoint as a tag + // The endpoint field is internal and only used for sampling decisions + } + + void 'sampler uses endpoint (not route) to compute hash when route is absent'() { + 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: 'first request uses endpoint to compute hash' + sampler.preSampleRequest(ctx1) + def hash1 = ctx1.getApiSecurityEndpointHash() + def endpoint1 = ctx1.getOrComputeEndpoint() + + and: 'second request with same endpoint pattern' + sampler.preSampleRequest(ctx2) + def hash2 = ctx2.getApiSecurityEndpointHash() + def endpoint2 = ctx2.getOrComputeEndpoint() + + then: 'both endpoints are simplified to the same pattern' + endpoint1 == '/api/users/{param:int}' + endpoint2 == '/api/users/{param:int}' + + and: 'both hashes are identical (computed from endpoint, method, status)' + hash1 == hash2 + } + + void 'sampler computes different hashes for different endpoints'() { + given: + def ctx1 = createContextWithUrl(null, 'GET', 200, 'http://localhost:8080/api/users/123') + def ctx2 = createContextWithUrl(null, 'GET', 200, 'http://localhost:8080/api/orders/456') + def sampler = new ApiSecuritySamplerImpl() + + when: + sampler.preSampleRequest(ctx1) + sampler.preSampleRequest(ctx2) + def hash1 = ctx1.getApiSecurityEndpointHash() + def hash2 = ctx2.getApiSecurityEndpointHash() + def endpoint1 = ctx1.getOrComputeEndpoint() + def endpoint2 = ctx2.getOrComputeEndpoint() + + then: 'endpoints are different' + endpoint1 == '/api/users/{param:int}' + endpoint2 == '/api/orders/{param:int}' + + and: 'hashes are different' + hash1 != hash2 + } + + void 'RFC-1076: when route is present, sampler uses route and does not compute endpoint'() { + given: + def ctx = createContextWithUrl('/api/users/{userId}', 'GET', 200, 'http://localhost:8080/api/users/123') + def sampler = new ApiSecuritySamplerImpl() + + when: + def preSampled = sampler.preSampleRequest(ctx) + + then: + preSampled + ctx.getApiSecurityEndpointHash() != null + + // Endpoint was NOT computed (route was used instead) + // We can verify this by checking that getOrComputeEndpoint returns the computed value + // but the sampler used the route directly + def endpoint = ctx.getOrComputeEndpoint() + endpoint == '/api/users/{param:int}' // Now it's computed because we called it + + // The hash was computed using the route, not the endpoint + def hashFromRoute = computeApiHash('/api/users/{userId}', 'GET', 200) + ctx.getApiSecurityEndpointHash() == hashFromRoute + } + + void 'RFC-1076: endpoint is computed at most once even with multiple getOrComputeEndpoint calls'() { + given: + def ctx = createContextWithUrl(null, 'GET', 200, 'http://localhost:8080/api/users/123/profile/settings') + + when: 'endpoint is computed multiple times' + def endpoint1 = ctx.getOrComputeEndpoint() + def endpoint2 = ctx.getOrComputeEndpoint() + def endpoint3 = ctx.getOrComputeEndpoint() + + then: 'all return the same instance (cached)' + endpoint1 != null + endpoint1 == endpoint2 + endpoint2 == endpoint3 + endpoint1 == '/api/users/{param:int}/profile/settings' + } + + void 'RFC-1076: 404 with valid endpoint does not sample'() { + given: + def ctx = createContextWithUrl(null, 'GET', 404, 'http://localhost:8080/api/nonexistent/resource') + def sampler = new ApiSecuritySamplerImpl() + + when: + def preSampled = sampler.preSampleRequest(ctx) + + then: + !preSampled + // Even though endpoint can be computed, 404s are not sampled + def endpoint = ctx.getOrComputeEndpoint() + endpoint != null // Endpoint is computable + ctx.getApiSecurityEndpointHash() == null // But hash was never set because sampling failed + } + + void 'RFC-1076: blocked request with valid endpoint does not sample'() { + given: + def ctx = createContextWithUrl(null, 'POST', 403, 'http://localhost:8080/api/admin/users') + ctx.setWafBlocked() // Request blocked by AppSec WAF + def sampler = new ApiSecuritySamplerImpl() + + when: + def preSampled = sampler.preSampleRequest(ctx) + + then: + !preSampled + // Blocked requests represent attacks, not legitimate API endpoints + ctx.getApiSecurityEndpointHash() == null + } + + // Helper method to compute hash same way as ApiSecuritySamplerImpl + private static long computeApiHash(final String route, final String method, final int statusCode) { + long result = 17 + result = 31 * result + route.hashCode() + result = 31 * result + method.hashCode() + result = 31 * result + statusCode + return result + } + private static AppSecRequestContext createContext(final String route, final String method, int statusCode) { final AppSecRequestContext ctx = new AppSecRequestContext() ctx.setRoute(route) @@ -371,4 +627,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 + } }