waf requests telemetry metrics (#5384)

* waf requests telemetry metrics

* fix undefined versions

* add more tests

* fix tags order

* update waf metrics integration test

* remove duplicate input_truncated on test

* fix waf requests telemtry test

* fix rate limiter

* report waf error metric

* fix updateWafRateLimitedMetric reporter test

* fix functions order

* change function names

* add versions to report attack

* fix reporting rate limiting metric

Author: IlyasShabi

packages/dd-trace/src/appsec/blocking.js

@@ -2,6 +2,7 @@
 
 const log = require('../log')
 const blockedTemplates = require('./blocked_templates')
+const { updateBlockFailureMetric } = require('./telemetry')
 
 const detectedSpecificEndpoints = {}
 
@@ -128,6 +129,7 @@ function block (req, res, rootSpan, abortController, actionParameters = defaultB
     rootSpan?.setTag('_dd.appsec.block.failed', 1)
     log.error('[ASM] Blocking error', err)
 
+    updateBlockFailureMetric(req)
     return false
   }
 }

packages/dd-trace/src/appsec/graphql.js

@@ -17,6 +17,7 @@ const {
   apolloChannel,
   apolloServerCoreChannel
 } = require('./channels')
+const { updateBlockFailureMetric } = require('./telemetry')
 
 const graphqlRequestData = new WeakMap()
 
@@ -106,8 +107,9 @@ function beforeWriteApolloGraphqlResponse ({ abortController, abortData }) {
       abortController?.abort()
     } catch (err) {
       rootSpan.setTag('_dd.appsec.block.failed', 1)
-
       log.error('[ASM] Blocking error', err)
+
+      updateBlockFailureMetric(req)
     }
   }
 

packages/dd-trace/src/appsec/reporter.js

@@ -10,7 +10,8 @@ const {
   updateRaspRequestsMetricTags,
   incrementWafUpdatesMetric,
   incrementWafRequestsMetric,
-  getRequestMetrics
+  getRequestMetrics,
+  updateRateLimitedMetric
 } = require('./telemetry')
 const zlib = require('zlib')
 const { keepTrace } = require('../priority_sampler')
@@ -149,6 +150,8 @@ function reportAttack (attackData) {
 
   if (limiter.isAllowed()) {
     keepTrace(rootSpan, ASM)
+  } else {
+    updateRateLimitedMetric(req)
   }
 
   // TODO: maybe add this to format.js later (to take decision as late as possible)

packages/dd-trace/src/appsec/telemetry/common.js

@@ -3,12 +3,15 @@
 const DD_TELEMETRY_REQUEST_METRICS = Symbol('_dd.appsec.telemetry.request.metrics')
 
 const tags = {
+  BLOCK_FAILURE: 'block_failure',
+  EVENT_RULES_VERSION: 'event_rules_version',
+  INPUT_TRUNCATED: 'input_truncated',
+  RATE_LIMITED: 'rate_limited',
   REQUEST_BLOCKED: 'request_blocked',
   RULE_TRIGGERED: 'rule_triggered',
+  WAF_ERROR: 'waf_error',
   WAF_TIMEOUT: 'waf_timeout',
-  WAF_VERSION: 'waf_version',
-  EVENT_RULES_VERSION: 'event_rules_version',
-  INPUT_TRUNCATED: 'input_truncated'
+  WAF_VERSION: 'waf_version'
 }
 
 function getVersionsTags (wafVersion, rulesVersion) {

packages/dd-trace/src/appsec/telemetry/index.js

@@ -74,6 +74,20 @@ function updateWafRequestsMetricTags (metrics, req) {
   return trackWafMetrics(store, metrics)
 }
 
+function updateRateLimitedMetric (req) {
+  if (!enabled) return
+
+  const store = getStore(req)
+  trackWafMetrics(store, { rateLimited: true })
+}
+
+function updateBlockFailureMetric (req) {
+  if (!enabled) return
+
+  const store = getStore(req)
+  trackWafMetrics(store, { blockFailed: true })
+}
+
 function incrementWafInitMetric (wafVersion, rulesVersion, success) {
   if (!enabled) return
 
@@ -119,6 +133,8 @@ module.exports = {
   disable,
 
   updateWafRequestsMetricTags,
+  updateRateLimitedMetric,
+  updateBlockFailureMetric,
   updateRaspRequestsMetricTags,
   incrementWafInitMetric,
   incrementWafUpdatesMetric,

packages/dd-trace/src/appsec/telemetry/waf.js

@@ -50,17 +50,28 @@ function trackWafMetrics (store, metrics) {
 
   const metricTags = getOrCreateMetricTags(store, versionsTags)
 
-  const { blockTriggered, ruleTriggered, wafTimeout } = metrics
+  if (metrics.blockFailed) {
+    metricTags[tags.BLOCK_FAILURE] = true
+  }
 
-  if (blockTriggered) {
+  if (metrics.blockTriggered) {
     metricTags[tags.REQUEST_BLOCKED] = true
   }
 
-  if (ruleTriggered) {
+  if (metrics.rateLimited) {
+    metricTags[tags.RATE_LIMITED] = true
+  }
+
+  if (metrics.ruleTriggered) {
     metricTags[tags.RULE_TRIGGERED] = true
   }
 
-  if (wafTimeout) {
+  if (metrics.errorCode) {
+    metricTags[tags.WAF_ERROR] = true
+    appsecMetrics.count('waf.error', { ...versionsTags, waf_error: metrics.errorCode }).inc()
+  }
+
+  if (metrics.wafTimeout) {
     metricTags[tags.WAF_TIMEOUT] = true
   }
 
@@ -78,10 +89,13 @@ function getOrCreateMetricTags (store, versionsTags) {
 
   if (!metricTags) {
     metricTags = {
+      [tags.BLOCK_FAILURE]: false,
+      [tags.INPUT_TRUNCATED]: false,
+      [tags.RATE_LIMITED]: false,
       [tags.REQUEST_BLOCKED]: false,
       [tags.RULE_TRIGGERED]: false,
+      [tags.WAF_ERROR]: false,
       [tags.WAF_TIMEOUT]: false,
-      [tags.INPUT_TRUNCATED]: false,
 
       ...versionsTags
     }

packages/dd-trace/test/appsec/blocking.spec.js

@@ -13,7 +13,7 @@ describe('blocking', () => {
     }
   }
 
-  let log
+  let log, telemetry
   let block, setTemplates
   let req, res, rootSpan
 
@@ -22,9 +22,14 @@ describe('blocking', () => {
       warn: sinon.stub()
     }
 
+    telemetry = {
+      updateBlockFailureMetric: sinon.stub()
+    }
+
     const blocking = proxyquire('../src/appsec/blocking', {
       '../log': log,
-      './blocked_templates': defaultBlockedTemplate
+      './blocked_templates': defaultBlockedTemplate,
+      './telemetry': telemetry
     })
 
     block = blocking.block
@@ -66,6 +71,7 @@ describe('blocking', () => {
       expect(rootSpan.setTag).to.have.been.calledOnceWithExactly('_dd.appsec.block.failed', 1)
       expect(res.setHeader).to.not.have.been.called
       expect(res.constructor.prototype.end).to.not.have.been.called
+      expect(telemetry.updateBlockFailureMetric).to.be.calledOnceWithExactly(req)
     })
 
     it('should send blocking response with html type if present in the headers', () => {
@@ -79,6 +85,7 @@ describe('blocking', () => {
         'Content-Length': 12
       })
       expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('htmlBodyéé')
+      expect(telemetry.updateBlockFailureMetric).to.not.have.been.called
     })
 
     it('should send blocking response with json type if present in the headers in priority', () => {
@@ -92,6 +99,7 @@ describe('blocking', () => {
         'Content-Length': 8
       })
       expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('jsonBody')
+      expect(telemetry.updateBlockFailureMetric).to.not.have.been.called
     })
 
     it('should send blocking response with json type if neither html or json is present in the headers', () => {
@@ -104,6 +112,7 @@ describe('blocking', () => {
         'Content-Length': 8
       })
       expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('jsonBody')
+      expect(telemetry.updateBlockFailureMetric).to.not.have.been.called
     })
 
     it('should send blocking response and call abortController if passed in arguments', () => {
@@ -118,6 +127,7 @@ describe('blocking', () => {
       })
       expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('jsonBody')
       expect(abortController.signal.aborted).to.be.true
+      expect(telemetry.updateBlockFailureMetric).to.not.have.been.called
     })
 
     it('should remove all headers before sending blocking response', () => {
@@ -135,6 +145,7 @@ describe('blocking', () => {
         'Content-Length': 8
       })
       expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('jsonBody')
+      expect(telemetry.updateBlockFailureMetric).to.not.have.been.called
     })
   })
 

packages/dd-trace/test/appsec/graphql.spec.js

@@ -12,8 +12,7 @@ const {
 } = require('../../src/appsec/channels')
 
 describe('GraphQL', () => {
-  let graphql
-  let blocking
+  let graphql, blocking, telemetry
 
   beforeEach(() => {
     const getBlockingData = sinon.stub()
@@ -29,8 +28,13 @@ describe('GraphQL', () => {
       statusCode: 403
     })
 
+    telemetry = {
+      updateBlockFailureMetric: sinon.stub()
+    }
+
     graphql = proxyquire('../../src/appsec/graphql', {
-      './blocking': blocking
+      './blocking': blocking,
+      './telemetry': telemetry
     })
   })
 
@@ -234,6 +238,7 @@ describe('GraphQL', () => {
       expect(blocking.getBlockingData).to.have.been.calledOnceWithExactly(req, 'graphql', blockParameters)
 
       expect(rootSpan.setTag).to.have.been.calledOnceWithExactly('appsec.blocked', 'true')
+      expect(telemetry.updateBlockFailureMetric).to.not.have.been.called
     })
 
     it('Should catch error when block fails', () => {
@@ -263,6 +268,7 @@ describe('GraphQL', () => {
       expect(blocking.getBlockingData).to.have.been.calledOnceWithExactly(req, 'graphql', blockParameters)
 
       expect(rootSpan.setTag).to.have.been.calledOnceWithExactly('_dd.appsec.block.failed', 1)
+      expect(telemetry.updateBlockFailureMetric).to.be.calledOnceWithExactly(req)
     })
   })
 })

packages/dd-trace/test/appsec/reporter.spec.js

@@ -38,6 +38,7 @@ describe('reporter', () => {
       updateRaspRequestsMetricTags: sinon.stub(),
       incrementWafUpdatesMetric: sinon.stub(),
       incrementWafRequestsMetric: sinon.stub(),
+      updateRateLimitedMetric: sinon.stub(),
       getRequestMetrics: sinon.stub()
     }
 
@@ -296,6 +297,7 @@ describe('reporter', () => {
         '_dd.appsec.json': '{"triggers":[{"rule":{},"rule_matches":[{}]}]}'
       })
       expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM)
+      expect(telemetry.updateRateLimitedMetric).to.not.have.been.called
     })
 
     it('should add tags to request span', () => {
@@ -310,38 +312,43 @@ describe('reporter', () => {
         'network.client.ip': '8.8.8.8'
       })
       expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM)
+      expect(telemetry.updateRateLimitedMetric).to.not.have.been.called
     })
 
     it('should not add manual.keep when rate limit is reached', (done) => {
       const addTags = span.addTags
-      const params = {}
 
-      expect(Reporter.reportAttack('', params)).to.not.be.false
-      expect(Reporter.reportAttack('', params)).to.not.be.false
-      expect(Reporter.reportAttack('', params)).to.not.be.false
+      expect(Reporter.reportAttack('')).to.not.be.false
+      expect(Reporter.reportAttack('')).to.not.be.false
+      expect(Reporter.reportAttack('')).to.not.be.false
 
       expect(prioritySampler.setPriority).to.have.callCount(3)
+      expect(telemetry.updateRateLimitedMetric).to.not.have.been.called
 
       Reporter.setRateLimit(1)
 
-      expect(Reporter.reportAttack('', params)).to.not.be.false
+      expect(Reporter.reportAttack('')).to.not.be.false
       expect(addTags.getCall(3).firstArg).to.have.property('appsec.event').that.equals('true')
       expect(prioritySampler.setPriority).to.have.callCount(4)
-      expect(Reporter.reportAttack('', params)).to.not.be.false
+      expect(telemetry.updateRateLimitedMetric).to.not.have.been.called
+
+      expect(Reporter.reportAttack('')).to.not.be.false
       expect(addTags.getCall(4).firstArg).to.have.property('appsec.event').that.equals('true')
       expect(prioritySampler.setPriority).to.have.callCount(4)
+      expect(telemetry.updateRateLimitedMetric).to.be.calledOnceWithExactly(req)
 
       setTimeout(() => {
-        expect(Reporter.reportAttack('', params)).to.not.be.false
+        expect(Reporter.reportAttack('')).to.not.be.false
         expect(prioritySampler.setPriority).to.have.callCount(5)
+        expect(telemetry.updateRateLimitedMetric).to.be.calledOnceWithExactly(req)
         done()
       }, 1020)
     })
 
     it('should not overwrite origin tag', () => {
       span.context()._tags = { '_dd.origin': 'tracer' }
 
-      const result = Reporter.reportAttack('[]', {})
+      const result = Reporter.reportAttack('[]')
       expect(result).to.not.be.false
       expect(web.root).to.have.been.calledOnceWith(req)
 
@@ -351,6 +358,7 @@ describe('reporter', () => {
         'network.client.ip': '8.8.8.8'
       })
       expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM)
+      expect(telemetry.updateRateLimitedMetric).to.not.have.been.called
     })
 
     it('should merge attacks json', () => {
@@ -367,6 +375,7 @@ describe('reporter', () => {
         'network.client.ip': '8.8.8.8'
       })
       expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM)
+      expect(telemetry.updateRateLimitedMetric).to.not.have.been.called
     })
 
     it('should call standalone sample', () => {
@@ -384,6 +393,7 @@ describe('reporter', () => {
       })
 
       expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM)
+      expect(telemetry.updateRateLimitedMetric).to.not.have.been.called
     })
   })
 

packages/dd-trace/test/appsec/telemetry/rasp.spec.js

@@ -138,12 +138,15 @@ describe('Appsec Rasp Telemetry metrics', () => {
         appsecTelemetry.incrementWafRequestsMetric(req)
 
         expect(count).to.have.been.calledWithExactly('waf.requests', {
+          block_failure: false,
+          input_truncated: false,
           request_blocked: false,
+          rate_limited: false,
           rule_triggered: false,
+          waf_error: false,
           waf_timeout: false,
           waf_version: wafVersion,
-          event_rules_version: rulesVersion,
-          input_truncated: false
+          event_rules_version: rulesVersion
         })
       })
     })