From 14647969034de3330b53b4975f186c96e15a3fa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Thu, 19 Dec 2024 23:25:53 +0100 Subject: [PATCH] Fix more system-tests --- .../datadog/appsec/gateway/GatewayBridge.java | 24 ++++++-- .../gateway/GatewayBridgeSpecification.groovy | 57 ++++++++++--------- .../api/telemetry/WafMetricCollector.java | 14 ++++- .../telemetry/WafMetricCollectorTest.groovy | 6 +- 4 files changed, 64 insertions(+), 37 deletions(-) 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 a5d23687f5f..cdb1f844428 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 @@ -36,6 +36,7 @@ import datadog.trace.api.gateway.SubscriptionService; import datadog.trace.api.http.StoredBodySupplier; import datadog.trace.api.internal.TraceSegment; +import datadog.trace.api.telemetry.LoginEvent; import datadog.trace.api.telemetry.RuleType; import datadog.trace.api.telemetry.WafMetricCollector; import datadog.trace.bootstrap.instrumentation.api.Tags; @@ -83,6 +84,14 @@ public class GatewayBridge { "appsec.events.users.login.success.track", "appsec.events.users.login.failure.track" }; + private static final Map EVENT_MAPPINGS = new HashMap<>(); + + static { + EVENT_MAPPINGS.put("users.login.success", LoginEvent.LOGIN_SUCCESS); + EVENT_MAPPINGS.put("users.login.failure", LoginEvent.LOGIN_FAILURE); + EVENT_MAPPINGS.put("users.signup", LoginEvent.SIGN_UP); + } + private static final String METASTRUCT_EXPLOIT = "exploit"; private final SubscriptionService subscriptionService; @@ -264,18 +273,25 @@ private Flow onLoginEvent( return NoopFlow.INSTANCE; } + // parse the event (might be null for custom events sent via the SDK) + final LoginEvent sourceEvent = EVENT_MAPPINGS.get(eventName); + // skip event if we have an SDK one if (mode != SDK) { segment.setTagTop("_dd.appsec.usr.login", user); - segment.setTagTop("_dd.appsec.usr.id", user); if (ctx.getUserLoginSource() == SDK) { return NoopFlow.INSTANCE; } + } else { + if (sourceEvent == LoginEvent.LOGIN_SUCCESS) { + segment.setTagTop("usr.id", user, false); + } else { + segment.setTagTop("appsec.events." + eventName + ".usr.id", user, true); + } } // update user span tags segment.setTagTop("appsec.events." + eventName + ".usr.login", user, true); - segment.setTagTop("appsec.events." + eventName + ".usr.id", user, true); // update current context with new user login ctx.setUserLoginSource(mode); @@ -289,9 +305,9 @@ private Flow onLoginEvent( final List> addresses = new ArrayList<>(3); addresses.add(KnownAddresses.USER_LOGIN); addresses.add(KnownAddresses.USER_ID); - if (eventName.endsWith("login.success")) { + if (sourceEvent == LoginEvent.LOGIN_SUCCESS) { addresses.add(KnownAddresses.LOGIN_SUCCESS); - } else if (eventName.endsWith("login.failure")) { + } else if (sourceEvent == LoginEvent.LOGIN_FAILURE) { addresses.add(KnownAddresses.LOGIN_FAILURE); } final MapDataBundle.Builder bundleBuilder = 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 97c5589faaf..08c77cb5eea 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 @@ -141,10 +141,10 @@ class GatewayBridgeSpecification extends DDSpecification { void 'request_end closes context reports attacks and publishes event'() { AppSecEvent event = Mock() AppSecRequestContext mockAppSecCtx = Mock(AppSecRequestContext) - mockAppSecCtx.requestHeaders >> ['accept':['header_value']] + mockAppSecCtx.requestHeaders >> ['accept': ['header_value']] mockAppSecCtx.responseHeaders >> [ - 'some-header': ['123'], - 'content-type':['text/html; charset=UTF-8']] + 'some-header' : ['123'], + 'content-type': ['text/html; charset=UTF-8']] RequestContext mockCtx = Stub(RequestContext) { getData(RequestContextSlot.APPSEC) >> mockAppSecCtx getTraceSegment() >> traceSegment @@ -155,7 +155,7 @@ class GatewayBridgeSpecification extends DDSpecification { def flow = requestEndedCB.apply(mockCtx, spanInfo) then: - 1 * spanInfo.getTags() >> ['http.client_ip':'1.1.1.1'] + 1 * spanInfo.getTags() >> ['http.client_ip': '1.1.1.1'] 1 * mockAppSecCtx.transferCollectedEvents() >> [event] 1 * mockAppSecCtx.peerAddress >> '2001::1' 1 * mockAppSecCtx.close(false) @@ -175,7 +175,7 @@ class GatewayBridgeSpecification extends DDSpecification { AppSecRequestContext mockAppSecCtx = Mock(AppSecRequestContext) mockAppSecCtx.requestHeaders >> [ 'x-real-ip': ['10.0.0.1'], - forwarded: ['for=127.0.0.1', 'for="[::1]", for=8.8.8.8'], + forwarded : ['for=127.0.0.1', 'for="[::1]", for=8.8.8.8'], ] RequestContext mockCtx = Stub(RequestContext) { getData(RequestContextSlot.APPSEC) >> mockAppSecCtx @@ -188,7 +188,7 @@ class GatewayBridgeSpecification extends DDSpecification { then: 1 * mockAppSecCtx.transferCollectedEvents() >> [Stub(AppSecEvent)] - 1 * spanInfo.getTags() >> ['http.client_ip':'8.8.8.8'] + 1 * spanInfo.getTags() >> ['http.client_ip': '8.8.8.8'] 1 * traceSegment.setTagTop('actor.ip', '8.8.8.8') } @@ -607,7 +607,7 @@ class GatewayBridgeSpecification extends DDSpecification { Object obj = 'hello' setup: - eventDispatcher.getDataSubscribers({KnownAddresses.REQUEST_BODY_OBJECT in it}) >> nonEmptyDsInfo + eventDispatcher.getDataSubscribers({ KnownAddresses.REQUEST_BODY_OBJECT in it }) >> nonEmptyDsInfo eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { bundle = it[2]; gatewayContext = it[3]; NoopFlow.INSTANCE } @@ -951,18 +951,18 @@ class GatewayBridgeSpecification extends DDSpecification { void 'default request headers are always set when appsec is enabled'() { final mockAppSecCtx = Mock(AppSecRequestContext) mockAppSecCtx.requestHeaders >> [ - 'host': ['localhost'], - 'accept': ['text/plain'], - 'content-type': ['application/json'], - 'user-agent': ['mozilla'], - 'x-amzn-trace-id': ['Root=1-65ae48bc-04fb551979979b6c57973027'], + 'host' : ['localhost'], + 'accept' : ['text/plain'], + 'content-type' : ['application/json'], + 'user-agent' : ['mozilla'], + 'x-amzn-trace-id' : ['Root=1-65ae48bc-04fb551979979b6c57973027'], 'cloudfront-viewer-ja3-fingerprint': ['e7d705a3286e19ea42f587b344ee6865'], - 'cf-ray': ['230b030023ae2822-SJC'], - 'x-cloud-trace-context': ['105445aa7843bc8bf206b12000100000/1'], - 'x-appgw-trace-id': ['ac882cd65a2712a0fe1289ec2bb6aee7'], - 'x-sigsci-requestid': ['55c24b96ca84c02201000001'], - 'x-sigsci-tags': ['SQLI, XSS'], - 'akamai-user-risk': ['uuid=913c4545-757b-4d8d-859d-e1361a828361;status=0'], + 'cf-ray' : ['230b030023ae2822-SJC'], + 'x-cloud-trace-context' : ['105445aa7843bc8bf206b12000100000/1'], + 'x-appgw-trace-id' : ['ac882cd65a2712a0fe1289ec2bb6aee7'], + 'x-sigsci-requestid' : ['55c24b96ca84c02201000001'], + 'x-sigsci-tags' : ['SQLI, XSS'], + 'akamai-user-risk' : ['uuid=913c4545-757b-4d8d-859d-e1361a828361;status=0'], ] final mockCtx = Stub(RequestContext) { getData(RequestContextSlot.APPSEC) >> mockAppSecCtx @@ -1099,12 +1099,11 @@ class GatewayBridgeSpecification extends DDSpecification { 0 * _ } else { 1 * traceSegment.setTagTop('appsec.events.users.signup.usr.login', expectedUser, true) - 1 * traceSegment.setTagTop('appsec.events.users.signup.usr.id', expectedUser, true) if (mode != SDK) { 1 * traceSegment.setTagTop('_dd.appsec.usr.login', expectedUser) - 1 * traceSegment.setTagTop('_dd.appsec.usr.id', expectedUser) 1 * traceSegment.setTagTop('_dd.appsec.events.users.signup.auto.mode', mode.fullName(), true) } else { + 1 * traceSegment.setTagTop('appsec.events.users.signup.usr.id', expectedUser, true) 1 * traceSegment.setTagTop('_dd.appsec.events.users.signup.sdk', true, true) } 1 * traceSegment.setTagTop('appsec.events.users.signup.track', true, true) @@ -1137,12 +1136,11 @@ class GatewayBridgeSpecification extends DDSpecification { 0 * _ } else { 1 * traceSegment.setTagTop('appsec.events.users.login.success.usr.login', expectedUser, true) - 1 * traceSegment.setTagTop('appsec.events.users.login.success.usr.id', expectedUser, true) if (mode != SDK) { 1 * traceSegment.setTagTop('_dd.appsec.usr.login', expectedUser) - 1 * traceSegment.setTagTop('_dd.appsec.usr.id', expectedUser) 1 * traceSegment.setTagTop('_dd.appsec.events.users.login.success.auto.mode', mode.fullName(), true) } else { + 1 * traceSegment.setTagTop('usr.id', expectedUser, false) 1 * traceSegment.setTagTop('_dd.appsec.events.users.login.success.sdk', true, true) } 1 * traceSegment.setTagTop('appsec.events.users.login.success.track', true, true) @@ -1176,12 +1174,11 @@ class GatewayBridgeSpecification extends DDSpecification { 0 * _ } else { 1 * traceSegment.setTagTop('appsec.events.users.login.failure.usr.login', expectedUser, true) - 1 * traceSegment.setTagTop('appsec.events.users.login.failure.usr.id', expectedUser, true) if (mode != SDK) { 1 * traceSegment.setTagTop('_dd.appsec.usr.login', expectedUser) - 1 * traceSegment.setTagTop('_dd.appsec.usr.id', expectedUser) 1 * traceSegment.setTagTop('_dd.appsec.events.users.login.failure.auto.mode', mode.fullName(), true) } else { + 1 * traceSegment.setTagTop('appsec.events.users.login.failure.usr.id', expectedUser, true) 1 * traceSegment.setTagTop('_dd.appsec.events.users.login.failure.sdk', true, true) } 1 * traceSegment.setTagTop('appsec.events.users.login.failure.track', true, true) @@ -1255,10 +1252,12 @@ class GatewayBridgeSpecification extends DDSpecification { then: 1 * traceSegment.setTagTop('appsec.events.users.login.success.usr.login', firstUser, true) - 1 * traceSegment.setTagTop('appsec.events.users.login.success.usr.id', firstUser, true) + 1 * traceSegment.setTagTop('usr.id', firstUser, false) + 1 * traceSegment.setTagTop('_dd.appsec.events.users.login.success.sdk', true, true) + 0 * traceSegment.setTagTop('_dd.appsec.usr.login', _) - 0 * traceSegment.setTagTop('_dd.appsec.usr.id', _) 0 * traceSegment.setTagTop('_dd.appsec.events.users.login.success.auto.mode', _, _) + 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> NoopFlow.INSTANCE when: @@ -1266,10 +1265,12 @@ class GatewayBridgeSpecification extends DDSpecification { then: 0 * traceSegment.setTagTop('appsec.events.users.login.success.usr.login', _, _) - 0 * traceSegment.setTagTop('appsec.events.users.login.success.usr.id', _, _) + 0 * traceSegment.setTagTop('usr.id', _, _) + 0 * traceSegment.setTagTop('_dd.appsec.events.users.login.success.sdk', _, _) + 1 * traceSegment.setTagTop('_dd.appsec.usr.login', secondUser) - 1 * traceSegment.setTagTop('_dd.appsec.usr.id', secondUser) 1 * traceSegment.setTagTop('_dd.appsec.events.users.login.success.auto.mode', IDENTIFICATION.fullName(), true) + 0 * eventDispatcher.publishDataEvent } } diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java b/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java index 76914d4b725..824412fbf24 100644 --- a/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java @@ -109,7 +109,7 @@ public void missingUserLogin(final LoginFramework framework, final LoginEvent ev } public void missingUserId(final LoginFramework framework) { - missingUserLoginQueue.incrementAndGet(framework.ordinal()); + missingUserIdQueue.incrementAndGet(framework.ordinal()); } @Override @@ -215,7 +215,7 @@ public void prepareMetrics() { } } - // Missing user id + // Missing user login for (LoginFramework framework : LoginFramework.values()) { for (LoginEvent event : LoginEvent.values()) { final int ordinal = framework.ordinal() * LoginEvent.getNumValues() + event.ordinal(); @@ -228,6 +228,16 @@ public void prepareMetrics() { } } } + + // Missing user id + for (LoginFramework framework : LoginFramework.values()) { + long counter = missingUserIdQueue.getAndSet(framework.ordinal(), 0); + if (counter > 0) { + if (!rawMetricsQueue.offer(new MissingUserIdMetric(counter, framework.getTag()))) { + return; + } + } + } } public abstract static class WafMetric extends MetricCollector.Metric { diff --git a/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy index 13f76958f93..e039b2598f8 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy @@ -238,13 +238,13 @@ class WafMetricCollectorTest extends DDSpecification { } assert tags["framework"] == LoginFramework.SPRING_SECURITY.getTag() switch (tags["event_type"]) { - case LoginEvent.LOGIN_SUCCESS.getTelemetryTag(): + case LoginEvent.LOGIN_SUCCESS.getTag(): assert metric.value == loginSuccessCount break - case LoginEvent.LOGIN_FAILURE.getTelemetryTag(): + case LoginEvent.LOGIN_FAILURE.getTag(): assert metric.value == loginFailureCount break - case LoginEvent.SIGN_UP.getTelemetryTag(): + case LoginEvent.SIGN_UP.getTag(): assert metric.value == signupCount break default: