Send mandatory request headers always

DataDog · Jun 10, 2024 · ccac65a · ccac65a
1 parent a5f0438
commit ccac65a
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 25 deletions.
diff --git a/packages/dd-trace/src/appsec/reporter.js b/packages/dd-trace/src/appsec/reporter.js
@@ -32,13 +32,11 @@ const REQUEST_HEADERS_MAP = mapHeaderAndTags([
   'via',
   ...contentHeaderList,
   'host',
-  'user-agent',
-  'accept',
   'accept-encoding',
   'accept-language'
 ], 'http.request.headers.')
 
-const IDENTIFICATION_HEADERS_MAP = mapHeaderAndTags([
+const identificationHeaders = [
   'x-amzn-trace-id',
   'cloudfront-viewer-ja3-fingerprint',
   'cf-ray',
@@ -47,6 +45,14 @@ const IDENTIFICATION_HEADERS_MAP = mapHeaderAndTags([
   'x-sigsci-requestid',
   'x-sigsci-tags',
   'akamai-user-risk'
+]
+
+// this request headers are always collected, and it breaks the expected spec orders
+const MANDATORY_REQUEST_HEADERS_MAP = mapHeaderAndTags([
+  'content-type',
+  'user-agent',
+  'accept',
+  ...identificationHeaders
 ], 'http.request.headers.')
 
 const RESPONSE_HEADERS_MAP = mapHeaderAndTags(contentHeaderList, 'http.response.headers.')
@@ -178,7 +184,12 @@ function finishRequest (req, res) {
   incrementWafRequestsMetric(req)
 
   // collect some headers even when no attack is detected
-  rootSpan.addTags(filterHeaders(req.headers, IDENTIFICATION_HEADERS_MAP))
+  const mandatoryTags = filterHeaders(req.headers, MANDATORY_REQUEST_HEADERS_MAP)
+  const ua = mandatoryTags['http.request.headers.user-agent']
+  if (ua) {
+    mandatoryTags['http.useragent'] = ua
+  }
+  rootSpan.addTags(mandatoryTags)
 
   const tags = rootSpan.context()._tags
   if (!shouldTrackHeaders(tags)) return
@@ -190,12 +201,6 @@ function finishRequest (req, res) {
   if (req.route && typeof req.route.path === 'string' && appsecEvent) {
     newTags['http.endpoint'] = req.route.path
   }
-
-  const ua = newTags['http.request.headers.user-agent']
-  if (ua) {
-    newTags['http.useragent'] = ua
-  }
-
   rootSpan.addTags(newTags)
 }
 

diff --git a/packages/dd-trace/test/appsec/reporter.spec.js b/packages/dd-trace/test/appsec/reporter.spec.js
@@ -314,6 +314,33 @@ describe('reporter', () => {
   describe('finishRequest', () => {
     let wafContext
 
+    const requestHeadersToTrackOnEvent = [
+      'x-forwarded-for',
+      'x-real-ip',
+      'true-client-ip',
+      'x-client-ip',
+      'x-forwarded',
+      'forwarded-for',
+      'x-cluster-client-ip',
+      'fastly-client-ip',
+      'cf-connecting-ip',
+      'cf-connecting-ipv6',
+      'forwarded',
+      'via',
+      'content-length',
+      'content-encoding',
+      'content-language',
+      'host',
+      'accept-encoding',
+      'accept-language'
+    ]
+    const requestHeadersAndValuesToTrackOnEvent = {}
+    const expectedRequestTagsToTrackOnEvent = {}
+    requestHeadersToTrackOnEvent.forEach((header, index) => {
+      requestHeadersAndValuesToTrackOnEvent[header] = `val-${index}`
+      expectedRequestTagsToTrackOnEvent[`http.request.headers.${header}`] = `val-${index}`
+    })
+
     beforeEach(() => {
       wafContext = {
         dispose: sinon.stub()
@@ -347,7 +374,7 @@ describe('reporter', () => {
       expect(Reporter.metricsQueue).to.be.empty
     })
 
-    it('should only add identification headers when no attack was previously found', () => {
+    it('should only add mandatory headers when no attack or event was previously found', () => {
       const req = {
         headers: {
           'not-included': 'hello',
@@ -358,7 +385,10 @@ describe('reporter', () => {
           'x-appgw-trace-id': 'e',
           'x-sigsci-requestid': 'f',
           'x-sigsci-tags': 'g',
-          'akamai-user-risk': 'h'
+          'akamai-user-risk': 'h',
+          'content-type': 'i',
+          'user-agent': 'j',
+          accept: 'k'
         }
       }
 
@@ -372,7 +402,11 @@ describe('reporter', () => {
         'http.request.headers.x-appgw-trace-id': 'e',
         'http.request.headers.x-sigsci-requestid': 'f',
         'http.request.headers.x-sigsci-tags': 'g',
-        'http.request.headers.akamai-user-risk': 'h'
+        'http.request.headers.akamai-user-risk': 'h',
+        'http.request.headers.content-type': 'i',
+        'http.request.headers.user-agent': 'j',
+        'http.request.headers.accept': 'k',
+        'http.useragent': 'j'
       })
     })
 
@@ -436,8 +470,8 @@ describe('reporter', () => {
     it('should add http request data inside request span when appsec.event is true', () => {
       const req = {
         headers: {
-          host: 'localhost',
-          'user-agent': 'arachni'
+          'user-agent': 'arachni',
+          ...requestHeadersAndValuesToTrackOnEvent
         }
       }
       const res = {
@@ -450,17 +484,18 @@ describe('reporter', () => {
       Reporter.finishRequest(req, res)
 
       expect(span.addTags).to.have.been.calledWithExactly({
-        'http.request.headers.host': 'localhost',
         'http.request.headers.user-agent': 'arachni',
         'http.useragent': 'arachni'
       })
+
+      expect(span.addTags).to.have.been.calledWithExactly(expectedRequestTagsToTrackOnEvent)
     })
 
     it('should add http request data inside request span when user login success is tracked', () => {
       const req = {
         headers: {
-          host: 'localhost',
-          'user-agent': 'arachni'
+          'user-agent': 'arachni',
+          ...requestHeadersAndValuesToTrackOnEvent
         }
       }
       const res = {
@@ -475,17 +510,18 @@ describe('reporter', () => {
       Reporter.finishRequest(req, res)
 
       expect(span.addTags).to.have.been.calledWithExactly({
-        'http.request.headers.host': 'localhost',
         'http.request.headers.user-agent': 'arachni',
         'http.useragent': 'arachni'
       })
+
+      expect(span.addTags).to.have.been.calledWithExactly(expectedRequestTagsToTrackOnEvent)
     })
 
     it('should add http request data inside request span when user login failure is tracked', () => {
       const req = {
         headers: {
-          host: 'localhost',
-          'user-agent': 'arachni'
+          'user-agent': 'arachni',
+          ...requestHeadersAndValuesToTrackOnEvent
         }
       }
       const res = {
@@ -500,17 +536,18 @@ describe('reporter', () => {
       Reporter.finishRequest(req, res)
 
       expect(span.addTags).to.have.been.calledWithExactly({
-        'http.request.headers.host': 'localhost',
         'http.request.headers.user-agent': 'arachni',
         'http.useragent': 'arachni'
       })
+
+      expect(span.addTags).to.have.been.calledWithExactly(expectedRequestTagsToTrackOnEvent)
     })
 
     it('should add http request data inside request span when user custom event is tracked', () => {
       const req = {
         headers: {
-          host: 'localhost',
-          'user-agent': 'arachni'
+          'user-agent': 'arachni',
+          ...requestHeadersAndValuesToTrackOnEvent
         }
       }
       const res = {
@@ -525,10 +562,11 @@ describe('reporter', () => {
       Reporter.finishRequest(req, res)
 
       expect(span.addTags).to.have.been.calledWithExactly({
-        'http.request.headers.host': 'localhost',
         'http.request.headers.user-agent': 'arachni',
         'http.useragent': 'arachni'
       })
+
+      expect(span.addTags).to.have.been.calledWithExactly(expectedRequestTagsToTrackOnEvent)
     })
 
     it('should call incrementWafRequestsMetric', () => {