rasp lfi subs delayed (#4715)

* Delay Appsec fs plugin subscription to fs:operations until the first req is received * disable rasp in tests * fix tests recursive call * Avoid multiple subscriptions to incomingHttpRequestStart * another try * replace spy with stub * execute unsubscribe asynchronously * sinon.assert async * clarify comment
DataDog · Sep 30, 2024 · 467d916 · 467d916
1 parent 72510cc
commit 467d916
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 11 deletions.
diff --git a/packages/dd-trace/src/appsec/rasp/lfi.js b/packages/dd-trace/src/appsec/rasp/lfi.js
@@ -1,6 +1,6 @@
 'use strict'
 
-const { fsOperationStart } = require('../channels')
+const { fsOperationStart, incomingHttpRequestStart } = require('../channels')
 const { storage } = require('../../../../datadog-core')
 const { enable: enableFsPlugin, disable: disableFsPlugin } = require('./fs-plugin')
 const { FS_OPERATION_PATH } = require('../addresses')
@@ -9,19 +9,36 @@ const { RULE_TYPES, handleResult } = require('./utils')
 const { isAbsolute } = require('path')
 
 let config
+let enabled
 
 function enable (_config) {
   config = _config
 
-  enableFsPlugin('rasp')
+  if (enabled) return
 
-  fsOperationStart.subscribe(analyzeLfi)
+  enabled = true
+
+  incomingHttpRequestStart.subscribe(onFirstReceivedRequest)
 }
 
 function disable () {
   if (fsOperationStart.hasSubscribers) fsOperationStart.unsubscribe(analyzeLfi)
+  if (incomingHttpRequestStart.hasSubscribers) incomingHttpRequestStart.unsubscribe(onFirstReceivedRequest)
 
   disableFsPlugin('rasp')
+
+  enabled = false
+}
+
+function onFirstReceivedRequest () {
+  // nodejs unsubscribe during publish bug: https://github.com/nodejs/node/pull/55116
+  process.nextTick(() => {
+    incomingHttpRequestStart.unsubscribe(onFirstReceivedRequest)
+  })
+
+  enableFsPlugin('rasp')
+
+  fsOperationStart.subscribe(analyzeLfi)
 }
 
 function analyzeLfi (ctx) {

diff --git a/packages/dd-trace/test/appsec/index.express.plugin.spec.js b/packages/dd-trace/test/appsec/index.express.plugin.spec.js
@@ -95,6 +95,9 @@ withVersions('express', 'express', version => {
             rules: path.join(__dirname, 'api_security_rules.json'),
             apiSecurity: {
               enabled: true
+            },
+            rasp: {
+              enabled: false
             }
           }
         })

diff --git a/packages/dd-trace/test/appsec/index.spec.js b/packages/dd-trace/test/appsec/index.spec.js
@@ -24,6 +24,7 @@ const blockedTemplate = require('../../src/appsec/blocked_templates')
 const { storage } = require('../../../datadog-core')
 const telemetryMetrics = require('../../src/telemetry/metrics')
 const addresses = require('../../src/appsec/addresses')
+const { disable: disableLfi } = require('../../src/appsec/rasp/lfi')
 
 const resultActions = {
   block_request: {
@@ -1062,6 +1063,8 @@ describe('IP blocking', function () {
       }
     }))
 
+    disableLfi()
+
     RuleManager.updateWafFromRC({ toUnapply: [], toApply: [], toModify })
   })
 

diff --git a/packages/dd-trace/test/appsec/rasp/lfi.spec.js b/packages/dd-trace/test/appsec/rasp/lfi.spec.js
@@ -1,11 +1,11 @@
 'use strict'
 
 const proxyquire = require('proxyquire')
-const { fsOperationStart } = require('../../../src/appsec/channels')
+const { fsOperationStart, incomingHttpRequestStart } = require('../../../src/appsec/channels')
 const { FS_OPERATION_PATH } = require('../../../src/appsec/addresses')
 
 describe('RASP - lfi.js', () => {
-  let waf, datadogCore, lfi, web, blocking, appsecFsPlugin
+  let waf, datadogCore, lfi, web, blocking, appsecFsPlugin, config
 
   beforeEach(() => {
     datadogCore = {
@@ -39,7 +39,7 @@ describe('RASP - lfi.js', () => {
       './fs-plugin': appsecFsPlugin
     })
 
-    const config = {
+    config = {
       appsec: {
         stackTrace: {
           enabled: true,
@@ -48,8 +48,6 @@ describe('RASP - lfi.js', () => {
         }
       }
     }
-
-    lfi.enable(config)
   })
 
   afterEach(() => {
@@ -58,13 +56,33 @@ describe('RASP - lfi.js', () => {
   })
 
   describe('enable', () => {
-    it('should enable AppsecFsPlugin', () => {
+    it('should subscribe to first http req', () => {
+      const subscribe = sinon.stub(incomingHttpRequestStart, 'subscribe')
+
+      lfi.enable(config)
+
+      sinon.assert.calledOnce(subscribe)
+    })
+
+    it('should enable AppsecFsPlugin after the first request', () => {
+      const unsubscribe = sinon.stub(incomingHttpRequestStart, 'unsubscribe')
+
+      lfi.enable(config)
+
+      incomingHttpRequestStart.publish({})
+
       sinon.assert.calledOnceWithExactly(appsecFsPlugin.enable, 'rasp')
+
+      process.nextTick(() => {
+        sinon.assert.calledOnce(unsubscribe)
+      })
     })
   })
 
   describe('disable', () => {
     it('should disable AppsecFsPlugin', () => {
+      lfi.enable(config)
+
       lfi.disable()
       sinon.assert.calledOnceWithExactly(appsecFsPlugin.disable, 'rasp')
     })
@@ -75,6 +93,12 @@ describe('RASP - lfi.js', () => {
     const ctx = { path }
     const req = {}
 
+    beforeEach(() => {
+      lfi.enable(config)
+
+      incomingHttpRequestStart.publish({})
+    })
+
     it('should analyze lfi for root fs operations', () => {
       const fs = { root: true }
       datadogCore.storage.getStore.returns({ req, fs })

diff --git a/packages/dd-trace/test/appsec/response_blocking.spec.js b/packages/dd-trace/test/appsec/response_blocking.spec.js
@@ -9,7 +9,7 @@ const path = require('path')
 const WafContext = require('../../src/appsec/waf/waf_context_wrapper')
 const blockingResponse = JSON.parse(require('../../src/appsec/blocked_templates').json)
 const fs = require('fs')
-const { disable: disableFsPlugin } = require('../../src/appsec/rasp/fs-plugin')
+const { disable: disableLfi } = require('../../src/appsec/rasp/lfi')
 
 describe('HTTP Response Blocking', () => {
   let server
@@ -57,7 +57,7 @@ describe('HTTP Response Blocking', () => {
       }
     }))
 
-    disableFsPlugin('rasp')
+    disableLfi()
   })
 
   beforeEach(() => {