From 60c30c276e6d1bbd6be923f98f27fca593be1198 Mon Sep 17 00:00:00 2001 From: "Santiago M. Mola" Date: Fri, 20 Jun 2025 15:57:37 +0200 Subject: [PATCH 1/2] Enable API Security by default and make it lazy loading (#9009) * Lazy API Security initialization * Reapply "Enable API Security by default (#8511)" (#9006) This reverts commit 2f4c864fa47aca970d9c78aa17819ec5453061b0. --- .../java/com/datadog/appsec/AppSecSystem.java | 38 ++++--- .../datadog/appsec/gateway/GatewayBridge.java | 9 +- .../gateway/GatewayBridgeSpecification.groovy | 101 ++++++++++-------- .../datadog/trace/api/ConfigDefaults.java | 2 +- 4 files changed, 89 insertions(+), 61 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/AppSecSystem.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/AppSecSystem.java index 36349842a77..461343252d7 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/AppSecSystem.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/AppSecSystem.java @@ -43,6 +43,8 @@ public class AppSecSystem { private static ReplaceableEventProducerService REPLACEABLE_EVENT_PRODUCER; // testing private static Runnable STOP_SUBSCRIPTION_SERVICE; private static Runnable RESET_SUBSCRIPTION_SERVICE; + private static final AtomicBoolean API_SECURITY_INITIALIZED = new AtomicBoolean(false); + private static volatile ApiSecuritySampler API_SECURITY_SAMPLER = new ApiSecuritySampler.NoOp(); public static void start(SubscriptionService gw, SharedCommunicationObjects sco) { try { @@ -69,18 +71,6 @@ private static void doStart(SubscriptionService gw, SharedCommunicationObjects s EventDispatcher eventDispatcher = new EventDispatcher(); REPLACEABLE_EVENT_PRODUCER.replaceEventProducerService(eventDispatcher); - ApiSecuritySampler requestSampler; - if (Config.get().isApiSecurityEnabled()) { - requestSampler = new ApiSecuritySamplerImpl(); - // When DD_API_SECURITY_ENABLED=true, ths post-processor is set even when AppSec is inactive. - // This should be low overhead since the post-processor exits early if there's no AppSec - // context. - SpanPostProcessor.Holder.INSTANCE = - new AppSecSpanPostProcessor(requestSampler, REPLACEABLE_EVENT_PRODUCER); - } else { - requestSampler = new ApiSecuritySampler.NoOp(); - } - ConfigurationPoller configurationPoller = sco.configurationPoller(config); // may throw and abort startup APP_SEC_CONFIG_SERVICE = @@ -94,7 +84,7 @@ private static void doStart(SubscriptionService gw, SharedCommunicationObjects s new GatewayBridge( gw, REPLACEABLE_EVENT_PRODUCER, - requestSampler, + () -> API_SECURITY_SAMPLER, APP_SEC_CONFIG_SERVICE.getTraceSegmentPostProcessors()); loadModules(eventDispatcher, sco.monitoring); @@ -129,6 +119,9 @@ public static void setActive(boolean status) { log.debug("AppSec is now {}", status ? "active" : "inactive"); ProductChangeCollector.get() .update(new ProductChange().productType(ProductChange.ProductType.APPSEC).enabled(status)); + if (status) { + maybeInitializeApiSecurity(); + } } public static void stop() { @@ -196,6 +189,25 @@ private static void reloadSubscriptions( } } + private static void maybeInitializeApiSecurity() { + if (!Config.get().isApiSecurityEnabled()) { + return; + } + if (!ActiveSubsystems.APPSEC_ACTIVE) { + return; + } + // We initialize API Security the first time AppSec becomes active. + // We never de-initialize it, as that could lead to a leak of open WAF contexts in-flight. + if (API_SECURITY_INITIALIZED.compareAndSet(false, true)) { + if (SpanPostProcessor.Holder.INSTANCE == SpanPostProcessor.Holder.NOOP) { + ApiSecuritySampler requestSampler = new ApiSecuritySamplerImpl(); + SpanPostProcessor.Holder.INSTANCE = + new AppSecSpanPostProcessor(requestSampler, REPLACEABLE_EVENT_PRODUCER); + API_SECURITY_SAMPLER = requestSampler; + } + } + } + public static boolean isStarted() { return STARTED.get(); } 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 b4bdb9b64c9..0eeeb10b998 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 @@ -52,8 +52,10 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Supplier; import java.util.regex.Pattern; import java.util.stream.Collectors; +import javax.annotation.Nonnull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -89,7 +91,7 @@ public class GatewayBridge { private final SubscriptionService subscriptionService; private final EventProducerService producerService; - private final ApiSecuritySampler requestSampler; + private final Supplier requestSamplerSupplier; private final List traceSegmentPostProcessors; // subscriber cache @@ -115,11 +117,11 @@ public class GatewayBridge { public GatewayBridge( SubscriptionService subscriptionService, EventProducerService producerService, - ApiSecuritySampler requestSampler, + @Nonnull Supplier requestSamplerSupplier, List traceSegmentPostProcessors) { this.subscriptionService = subscriptionService; this.producerService = producerService; - this.requestSampler = requestSampler; + this.requestSamplerSupplier = requestSamplerSupplier; this.traceSegmentPostProcessors = traceSegmentPostProcessors; } @@ -778,6 +780,7 @@ private boolean maybeSampleForApiSecurity( if (route != null) { ctx.setRoute(route.toString()); } + ApiSecuritySampler requestSampler = requestSamplerSupplier.get(); return requestSampler.preSampleRequest(ctx); } diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy index 6839b7061b9..d3d62600e73 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy @@ -86,7 +86,7 @@ class GatewayBridgeSpecification extends DDSpecification { TraceSegmentPostProcessor pp = Mock() ApiSecuritySamplerImpl requestSampler = Mock(ApiSecuritySamplerImpl) - GatewayBridge bridge = new GatewayBridge(ig, eventDispatcher, requestSampler, [pp]) + GatewayBridge bridge = new GatewayBridge(ig, eventDispatcher, () -> requestSampler, [pp]) Supplier> requestStartedCB BiFunction> requestEndedCB @@ -258,8 +258,9 @@ class GatewayBridgeSpecification extends DDSpecification { ctx.data.rawURI = '/' ctx.data.peerAddress = '0.0.0.0' eventDispatcher.getDataSubscribers(_) >> nonEmptyDsInfo - eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> - { bundle = it[2]; NoopFlow.INSTANCE } + eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { + bundle = it[2]; NoopFlow.INSTANCE + } and: reqHeadersDoneCB.apply(ctx) @@ -277,8 +278,9 @@ class GatewayBridgeSpecification extends DDSpecification { when: eventDispatcher.getDataSubscribers(_) >> nonEmptyDsInfo - eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> - { bundle = it[2]; gatewayContext = it[3]; NoopFlow.INSTANCE } + eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { + bundle = it[2]; gatewayContext = it[3]; NoopFlow.INSTANCE + } and: reqHeadersDoneCB.apply(ctx) @@ -298,8 +300,9 @@ class GatewayBridgeSpecification extends DDSpecification { when: eventDispatcher.getDataSubscribers(_) >> nonEmptyDsInfo - eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> - { bundle = it[2]; gatewayContext = it[3]; NoopFlow.INSTANCE } + eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { + bundle = it[2]; gatewayContext = it[3]; NoopFlow.INSTANCE + } and: reqHeadersDoneCB.apply(ctx) @@ -319,8 +322,9 @@ class GatewayBridgeSpecification extends DDSpecification { when: eventDispatcher.getDataSubscribers(_) >> nonEmptyDsInfo - eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> - { bundle = it[2]; gatewayContext = it[3]; NoopFlow.INSTANCE } + eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { + bundle = it[2]; gatewayContext = it[3]; NoopFlow.INSTANCE + } and: reqHeadersDoneCB.apply(ctx) @@ -339,9 +343,12 @@ class GatewayBridgeSpecification extends DDSpecification { def adapter = TestURIDataAdapter.create(uri, supportsRaw) when: - eventDispatcher.getDataSubscribers({ KnownAddresses.REQUEST_URI_RAW in it }) >> nonEmptyDsInfo - eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> - { bundle = it[2]; gatewayContext = it[3]; NoopFlow.INSTANCE } + eventDispatcher.getDataSubscribers({ + KnownAddresses.REQUEST_URI_RAW in it + }) >> nonEmptyDsInfo + eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { + bundle = it[2]; gatewayContext = it[3]; NoopFlow.INSTANCE + } and: requestMethodURICB.apply(ctx, 'GET', adapter) @@ -373,9 +380,12 @@ class GatewayBridgeSpecification extends DDSpecification { def adapter = TestURIDataAdapter.create(uri) when: - eventDispatcher.getDataSubscribers({ KnownAddresses.REQUEST_URI_RAW in it }) >> nonEmptyDsInfo - eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> - { bundle = it[2]; gatewayContext = it[3]; NoopFlow.INSTANCE } + eventDispatcher.getDataSubscribers({ + KnownAddresses.REQUEST_URI_RAW in it + }) >> nonEmptyDsInfo + eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { + bundle = it[2]; gatewayContext = it[3]; NoopFlow.INSTANCE + } and: requestMethodURICB.apply(ctx, 'GET', adapter) @@ -406,9 +416,12 @@ class GatewayBridgeSpecification extends DDSpecification { GatewayContext gatewayContext when: - eventDispatcher.getDataSubscribers({ KnownAddresses.REQUEST_PATH_PARAMS in it }) >> nonEmptyDsInfo - eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> - { bundle = it[2]; gatewayContext = it[3]; NoopFlow.INSTANCE } + eventDispatcher.getDataSubscribers({ + KnownAddresses.REQUEST_PATH_PARAMS in it + }) >> nonEmptyDsInfo + eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { + bundle = it[2]; gatewayContext = it[3]; NoopFlow.INSTANCE + } and: pathParamsCB.apply(ctx, [a: 'b']) @@ -663,9 +676,9 @@ class GatewayBridgeSpecification extends DDSpecification { when: Flow flow = requestBodyProcessedCB.apply(ctx, new Object() { - @SuppressWarnings('UnusedPrivateField') - private String foo = 'bar' - }) + @SuppressWarnings('UnusedPrivateField') + private String foo = 'bar' + }) then: 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> @@ -762,9 +775,9 @@ class GatewayBridgeSpecification extends DDSpecification { when: Flow flow = grpcServerRequestMessageCB.apply(ctx, new Object() { - @SuppressWarnings('UnusedPrivateField') - private String foo = 'bar' - }) + @SuppressWarnings('UnusedPrivateField') + private String foo = 'bar' + }) then: 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> @@ -919,28 +932,28 @@ class GatewayBridgeSpecification extends DDSpecification { void 'no appsec events if was not created request context in request_start event'() { RequestContext emptyCtx = new RequestContext() { - final Object data = null - BlockResponseFunction blockResponseFunction - - @Override - Object getData(RequestContextSlot slot) { - data - } - - @Override - final TraceSegment getTraceSegment() { - GatewayBridgeSpecification.this.traceSegment - } - - @Override - def T getOrCreateMetaStructTop(String key, Function defaultValue) { - return null - } - - @Override - void close() throws IOException {} + final Object data = null + BlockResponseFunction blockResponseFunction + + @Override + Object getData(RequestContextSlot slot) { + data } + @Override + final TraceSegment getTraceSegment() { + GatewayBridgeSpecification.this.traceSegment + } + + @Override + def T getOrCreateMetaStructTop(String key, Function defaultValue) { + return null + } + + @Override + void close() throws IOException {} + } + StoredBodySupplier supplier = Stub() IGSpanInfo spanInfo = Stub(AgentSpan) Object obj = 'obj' diff --git a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java index c389eccbd95..7468bf0de96 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java @@ -113,7 +113,7 @@ public final class ConfigDefaults { static final int DEFAULT_APPSEC_TRACE_RATE_LIMIT = 100; static final boolean DEFAULT_APPSEC_WAF_METRICS = true; static final int DEFAULT_APPSEC_WAF_TIMEOUT = 100000; // 0.1 s - static final boolean DEFAULT_API_SECURITY_ENABLED = false; + static final boolean DEFAULT_API_SECURITY_ENABLED = true; static final float DEFAULT_API_SECURITY_SAMPLE_DELAY = 30.0f; // TODO: change to true once the RFC is approved static final boolean DEFAULT_API_SECURITY_ENDPOINT_COLLECTION_ENABLED = false; From aa9dd27535d496bfe616a79b6f7db9493fbc5b64 Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Fri, 20 Jun 2025 22:09:45 +0100 Subject: [PATCH 2/2] Fix NPE in akka-http and pekko-http integrations (#9019) Also fix a related issue where spans were not being finished while polling through leftover scopes. --- .../akkahttp/DatadogServerRequestResponseFlowWrapper.java | 4 ++-- .../pekkohttp/DatadogServerRequestResponseFlowWrapper.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dd-java-agent/instrumentation/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/DatadogServerRequestResponseFlowWrapper.java b/dd-java-agent/instrumentation/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/DatadogServerRequestResponseFlowWrapper.java index f0c211e86bd..46e43ca1467 100644 --- a/dd-java-agent/instrumentation/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/DatadogServerRequestResponseFlowWrapper.java +++ b/dd-java-agent/instrumentation/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/DatadogServerRequestResponseFlowWrapper.java @@ -168,16 +168,16 @@ public void onUpstreamFinish() throws Exception { @Override public void onUpstreamFailure(final Throwable ex) throws Exception { ContextScope scope = scopes.poll(); - AgentSpan span = AgentSpan.fromContext(scope.context()); if (scope != null) { // Mark the span as failed + AgentSpan span = AgentSpan.fromContext(scope.context()); DatadogWrapperHelper.finishSpan(span, ex); } // We will not receive any more responses from the user code, so clean up any // remaining spans scope = scopes.poll(); while (scope != null) { - span.finish(); + AgentSpan.fromContext(scope.context()).finish(); scope = scopes.poll(); } fail(responseOutlet, ex); diff --git a/dd-java-agent/instrumentation/pekko-http-1.0/src/main/java/datadog/trace/instrumentation/pekkohttp/DatadogServerRequestResponseFlowWrapper.java b/dd-java-agent/instrumentation/pekko-http-1.0/src/main/java/datadog/trace/instrumentation/pekkohttp/DatadogServerRequestResponseFlowWrapper.java index 466f6657372..a58933f194f 100644 --- a/dd-java-agent/instrumentation/pekko-http-1.0/src/main/java/datadog/trace/instrumentation/pekkohttp/DatadogServerRequestResponseFlowWrapper.java +++ b/dd-java-agent/instrumentation/pekko-http-1.0/src/main/java/datadog/trace/instrumentation/pekkohttp/DatadogServerRequestResponseFlowWrapper.java @@ -141,16 +141,16 @@ public void onUpstreamFinish() throws Exception { @Override public void onUpstreamFailure(final Throwable ex) throws Exception { ContextScope scope = scopes.poll(); - AgentSpan span = AgentSpan.fromContext(scope.context()); if (scope != null) { // Mark the span as failed + AgentSpan span = AgentSpan.fromContext(scope.context()); DatadogWrapperHelper.finishSpan(span, ex); } // We will not receive any more responses from the user code, so clean up any // remaining spans scope = scopes.poll(); while (scope != null) { - span.finish(); + AgentSpan.fromContext(scope.context()).finish(); scope = scopes.poll(); } fail(responseOutlet, ex);