Fix appsec.rasp.error and appsec.waf.error telemetry metrics

dd-java-agent/appsec/src/main/java/com/datadog/appsec/ddwaf/WAFModule.java

@@ -448,16 +448,12 @@ public void onDataAvailable(
         if (!reqCtx.isWafContextClosed()) {
           log.error("Error calling WAF", e);
         }
+        // TODO this is wrong and will be fixed in another PR
         WafMetricCollector.get().wafRequestError();
+        incrementErrorCodeMetric(gwCtx, e.code);
         return;
       } catch (AbstractWafException e) {
-        if (gwCtx.isRasp) {
-          reqCtx.increaseRaspErrorCode(e.code);
-          WafMetricCollector.get().raspErrorCode(gwCtx.raspRuleType, e.code);
-        } else {
-          reqCtx.increaseWafErrorCode(e.code);
-          WafMetricCollector.get().wafErrorCode(gwCtx.raspRuleType, e.code);
-        }
+        incrementErrorCodeMetric(gwCtx, e.code);
         return;
       } finally {
         if (log.isDebugEnabled()) {
@@ -653,6 +649,14 @@ private Waf.ResultWithData runWafContext(
     }
   }
 
+  private static void incrementErrorCodeMetric(GatewayContext gwCtx, int code) {
+    if (gwCtx.isRasp) {
+      WafMetricCollector.get().raspErrorCode(gwCtx.raspRuleType, code);
+    } else {
+      WafMetricCollector.get().wafErrorCode(code);
+    }
+  }
+
   private Waf.ResultWithData runWafTransient(
       WafContext wafContext, WafMetrics metrics, DataBundle bundle, CtxAndAddresses ctxAndAddr)
       throws AbstractWafException {

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

@@ -74,9 +74,6 @@ public class AppSecRequestContext implements DataBundle, Closeable {
   public static final Set<String> RESPONSE_HEADERS_ALLOW_LIST =
       new TreeSet<>(
           Arrays.asList("content-length", "content-type", "content-encoding", "content-language"));
-  public static final int DD_WAF_RUN_INTERNAL_ERROR = -3;
-  public static final int DD_WAF_RUN_INVALID_OBJECT_ERROR = -2;
-  public static final int DD_WAF_RUN_INVALID_ARGUMENT_ERROR = -1;
 
   static {
     REQUEST_HEADERS_ALLOW_LIST.addAll(DEFAULT_REQUEST_HEADERS_ALLOW_LIST);
@@ -125,12 +122,6 @@ public class AppSecRequestContext implements DataBundle, Closeable {
   private volatile boolean blocked;
   private volatile int wafTimeouts;
   private volatile int raspTimeouts;
-  private volatile int raspInternalErrors;
-  private volatile int raspInvalidObjectErrors;
-  private volatile int raspInvalidArgumentErrors;
-  private volatile int wafInternalErrors;
-  private volatile int wafInvalidObjectErrors;
-  private volatile int wafInvalidArgumentErrors;
 
   // keep a reference to the last published usr.id
   private volatile String userId;
@@ -150,29 +141,6 @@ public class AppSecRequestContext implements DataBundle, Closeable {
   private static final AtomicIntegerFieldUpdater<AppSecRequestContext> RASP_TIMEOUTS_UPDATER =
       AtomicIntegerFieldUpdater.newUpdater(AppSecRequestContext.class, "raspTimeouts");
 
-  private static final AtomicIntegerFieldUpdater<AppSecRequestContext>
-      RASP_INTERNAL_ERRORS_UPDATER =
-          AtomicIntegerFieldUpdater.newUpdater(AppSecRequestContext.class, "raspInternalErrors");
-  private static final AtomicIntegerFieldUpdater<AppSecRequestContext>
-      RASP_INVALID_OBJECT_ERRORS_UPDATER =
-          AtomicIntegerFieldUpdater.newUpdater(
-              AppSecRequestContext.class, "raspInvalidObjectErrors");
-  private static final AtomicIntegerFieldUpdater<AppSecRequestContext>
-      RASP_INVALID_ARGUMENT_ERRORS_UPDATER =
-          AtomicIntegerFieldUpdater.newUpdater(
-              AppSecRequestContext.class, "raspInvalidArgumentErrors");
-
-  private static final AtomicIntegerFieldUpdater<AppSecRequestContext> WAF_INTERNAL_ERRORS_UPDATER =
-      AtomicIntegerFieldUpdater.newUpdater(AppSecRequestContext.class, "wafInternalErrors");
-  private static final AtomicIntegerFieldUpdater<AppSecRequestContext>
-      WAF_INVALID_OBJECT_ERRORS_UPDATER =
-          AtomicIntegerFieldUpdater.newUpdater(
-              AppSecRequestContext.class, "wafInvalidObjectErrors");
-  private static final AtomicIntegerFieldUpdater<AppSecRequestContext>
-      WAF_INVALID_ARGUMENT_ERRORS_UPDATER =
-          AtomicIntegerFieldUpdater.newUpdater(
-              AppSecRequestContext.class, "wafInvalidArgumentErrors");
-
   // to be called by the Event Dispatcher
   public void addAll(DataBundle newData) {
     for (Map.Entry<Address<?>, Object> entry : newData) {
@@ -222,38 +190,6 @@ public void increaseRaspTimeouts() {
     RASP_TIMEOUTS_UPDATER.incrementAndGet(this);
   }
 
-  public void increaseRaspErrorCode(int code) {
-    switch (code) {
-      case DD_WAF_RUN_INTERNAL_ERROR:
-        RASP_INTERNAL_ERRORS_UPDATER.incrementAndGet(this);
-        break;
-      case DD_WAF_RUN_INVALID_OBJECT_ERROR:
-        RASP_INVALID_OBJECT_ERRORS_UPDATER.incrementAndGet(this);
-        break;
-      case DD_WAF_RUN_INVALID_ARGUMENT_ERROR:
-        RASP_INVALID_ARGUMENT_ERRORS_UPDATER.incrementAndGet(this);
-        break;
-      default:
-        break;
-    }
-  }
-
-  public void increaseWafErrorCode(int code) {
-    switch (code) {
-      case DD_WAF_RUN_INTERNAL_ERROR:
-        WAF_INTERNAL_ERRORS_UPDATER.incrementAndGet(this);
-        break;
-      case DD_WAF_RUN_INVALID_OBJECT_ERROR:
-        WAF_INVALID_OBJECT_ERRORS_UPDATER.incrementAndGet(this);
-        break;
-      case DD_WAF_RUN_INVALID_ARGUMENT_ERROR:
-        WAF_INVALID_ARGUMENT_ERRORS_UPDATER.incrementAndGet(this);
-        break;
-      default:
-        break;
-    }
-  }
-
   public int getWafTimeouts() {
     return wafTimeouts;
   }
@@ -262,32 +198,6 @@ public int getRaspTimeouts() {
     return raspTimeouts;
   }
 
-  public int getRaspError(int code) {
-    switch (code) {
-      case DD_WAF_RUN_INTERNAL_ERROR:
-        return raspInternalErrors;
-      case DD_WAF_RUN_INVALID_OBJECT_ERROR:
-        return raspInvalidObjectErrors;
-      case DD_WAF_RUN_INVALID_ARGUMENT_ERROR:
-        return raspInvalidArgumentErrors;
-      default:
-        return 0;
-    }
-  }
-
-  public int getWafError(int code) {
-    switch (code) {
-      case DD_WAF_RUN_INTERNAL_ERROR:
-        return wafInternalErrors;
-      case DD_WAF_RUN_INVALID_OBJECT_ERROR:
-        return wafInvalidObjectErrors;
-      case DD_WAF_RUN_INVALID_ARGUMENT_ERROR:
-        return wafInvalidArgumentErrors;
-      default:
-        return 0;
-    }
-  }
-
   public WafContext getOrCreateWafContext(WafHandle ctx, boolean createMetrics, boolean isRasp) {
 
     if (createMetrics) {

dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/ddwaf/WAFModuleSpecification.groovy

@@ -1,5 +1,8 @@
 package com.datadog.appsec.ddwaf
 
+import com.datadog.ddwaf.WafErrorCode as LibWafErrorCode
+import datadog.trace.api.telemetry.WafMetricCollector.WafErrorCode as InternalWafErrorCode
+
 import com.datadog.appsec.AppSecModule
 import com.datadog.appsec.config.AppSecConfig
 import com.datadog.appsec.config.AppSecData
@@ -17,6 +20,11 @@ import com.datadog.appsec.event.data.MapDataBundle
 import com.datadog.appsec.gateway.AppSecRequestContext
 import com.datadog.appsec.gateway.GatewayContext
 import com.datadog.appsec.report.AppSecEvent
+import com.datadog.ddwaf.exception.AbstractWafException
+import com.datadog.ddwaf.exception.InternalWafException
+import com.datadog.ddwaf.exception.InvalidArgumentWafException
+import com.datadog.ddwaf.exception.InvalidObjectWafException
+import com.datadog.ddwaf.exception.UnclassifiedWafException
 import datadog.trace.api.telemetry.RuleType
 import datadog.trace.util.stacktrace.StackTraceEvent
 import com.datadog.appsec.test.StubAppSecConfigService
@@ -1755,6 +1763,94 @@ class WAFModuleSpecification extends DDSpecification {
     0 * _
   }
 
+  void 'test raspErrorCode metric is increased when waf  call throws #wafErrorCode '() {
+    setup:
+    ChangeableFlow flow = Mock()
+    GatewayContext gwCtxMock = new GatewayContext(false, RuleType.SQL_INJECTION)
+    WafContext wafContext = Mock()
+
+    when:
+    setupWithStubConfigService('rules_with_data_config.json')
+    dataListener = wafModule.dataSubscriptions.first()
+
+    def bundle = MapDataBundle.of(
+      KnownAddresses.USER_ID,
+      'legit-user'
+      )
+    dataListener.onDataAvailable(flow, ctx, bundle, gwCtxMock)
+
+    then:
+    (1..2) * ctx.isWafContextClosed() >> false // if UnclassifiedWafException it's called twice
+    1 * ctx.getOrCreateWafContext(_, true, true) >> wafContext
+    1 * wafMetricCollector.raspRuleEval(RuleType.SQL_INJECTION)
+    1 * wafContext.run(_, _, _) >> { throw createWafException(wafErrorCode) }
+    1 * wafMetricCollector.wafInit(Waf.LIB_VERSION, _, true)
+    1 * ctx.getRaspMetrics()
+    1 * ctx.getRaspMetricsCounter()
+    (0..1) * WafMetricCollector.get().wafRequestError() // TODO: remove this line when WAFModule is removed
+    1 * wafMetricCollector.raspErrorCode(RuleType.SQL_INJECTION, wafErrorCode.code)
+    0 * _
+
+    where:
+    wafErrorCode << LibWafErrorCode.values()
+  }
+
+  void 'test wafErrorCode metric is increased when waf  call throws #wafErrorCode '() {
+    setup:
+    ChangeableFlow flow = Mock()
+    WafContext wafContext = Mock()
+
+    when:
+    setupWithStubConfigService('rules_with_data_config.json')
+    dataListener = wafModule.dataSubscriptions.first()
+
+    def bundle = MapDataBundle.of(
+      KnownAddresses.USER_ID,
+      'legit-user'
+      )
+    dataListener.onDataAvailable(flow, ctx, bundle, gwCtx)
+
+    then:
+    (1..2) * ctx.isWafContextClosed() >> false // if UnclassifiedWafException it's called twice
+    1 * ctx.getOrCreateWafContext(_, true, false) >> wafContext
+    1 * wafContext.run(_, _, _) >> { throw createWafException(wafErrorCode) }
+    1 * wafMetricCollector.wafInit(Waf.LIB_VERSION, _, true)
+    2 * ctx.getWafMetrics()
+    (0..1) * WafMetricCollector.get().wafRequestError() // TODO: remove this line when WAFModule is removed
+    1 * wafMetricCollector.wafErrorCode(wafErrorCode.code)
+    0 * _
+
+    where:
+    wafErrorCode << LibWafErrorCode.values()
+  }
+
+  def 'internal-api WafErrorCode enum matches libddwaf-java by name and code'() {
+    given:
+    def libddwaf = LibWafErrorCode.values().collectEntries { [it.name(), it.code] }
+    def internal = InternalWafErrorCode.values().collectEntries { [it.name(), it.code] }
+
+    expect:
+    internal == libddwaf
+  }
+
+  /**
+   * Helper to return a concrete Waf exception for each WafErrorCode
+   */
+  static AbstractWafException createWafException(LibWafErrorCode code) {
+    switch (code) {
+      case LibWafErrorCode.INVALID_ARGUMENT:
+        return new InvalidArgumentWafException(code.code)
+      case LibWafErrorCode.INVALID_OBJECT:
+        return new InvalidObjectWafException(code.code)
+      case LibWafErrorCode.INTERNAL_ERROR:
+        return new InternalWafException(code.code)
+      case LibWafErrorCode.BINDING_ERROR:
+        return new UnclassifiedWafException(code.code)
+      default:
+        throw new IllegalStateException("Unhandled WafErrorCode: $code")
+    }
+  }
+
   private Map<String, Object> getDefaultConfig() {
     def service = new StubAppSecConfigService()
     service.init()

dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/AppSecRequestContextSpecification.groovy

@@ -289,32 +289,6 @@ class AppSecRequestContextSpecification extends DDSpecification {
     ctx.getRaspTimeouts() == 2
   }
 
-  def "test increase and get RaspErrors"() {
-    when:
-    ctx.increaseRaspErrorCode(AppSecRequestContext.DD_WAF_RUN_INTERNAL_ERROR)
-    ctx.increaseRaspErrorCode(AppSecRequestContext.DD_WAF_RUN_INTERNAL_ERROR)
-    ctx.increaseRaspErrorCode(AppSecRequestContext.DD_WAF_RUN_INVALID_OBJECT_ERROR)
-
-    then:
-    ctx.getRaspError(AppSecRequestContext.DD_WAF_RUN_INTERNAL_ERROR) == 2
-    ctx.getRaspError(AppSecRequestContext.DD_WAF_RUN_INVALID_OBJECT_ERROR) == 1
-    ctx.getRaspError(AppSecRequestContext.DD_WAF_RUN_INVALID_ARGUMENT_ERROR) == 0
-    ctx.getRaspError(0) == 0
-  }
-
-  def "test increase and get WafErrors"() {
-    when:
-    ctx.increaseWafErrorCode(AppSecRequestContext.DD_WAF_RUN_INTERNAL_ERROR)
-    ctx.increaseWafErrorCode(AppSecRequestContext.DD_WAF_RUN_INTERNAL_ERROR)
-    ctx.increaseWafErrorCode(AppSecRequestContext.DD_WAF_RUN_INVALID_OBJECT_ERROR)
-
-    then:
-    ctx.getWafError(AppSecRequestContext.DD_WAF_RUN_INTERNAL_ERROR) == 2
-    ctx.getWafError(AppSecRequestContext.DD_WAF_RUN_INVALID_OBJECT_ERROR) == 1
-    ctx.getWafError(AppSecRequestContext.DD_WAF_RUN_INVALID_ARGUMENT_ERROR) == 0
-    ctx.getWafError(0) == 0
-  }
-
   void 'close logs if request end was not called'() {
     given:
     TestLogCollector.enable()