From 7e4c6cf65ac7c9b2576c446c64315e3b1fcaf809 Mon Sep 17 00:00:00 2001 From: simon-id Date: Mon, 10 Jun 2024 16:06:27 +0200 Subject: [PATCH] Suspicious Response Blocking (#3837) * add RC capability for response blocking * merge headers in writeHead() * clear implicit headers before sending blocking response * support implicit write() * support array syntax for WriteHead() * don't run request end waf if not useful * avoid double call and write after end * little polyfill for node 16 * check for subscribers early exit --------- Co-authored-by: Ugaitz Urien Co-authored-by: Igor Unanua --- .../src/http/server.js | 98 +++++++ packages/dd-trace/src/appsec/blocking.js | 4 + packages/dd-trace/src/appsec/channels.js | 1 + packages/dd-trace/src/appsec/index.js | 54 +++- .../src/appsec/remote_config/capabilities.js | 1 + .../src/appsec/remote_config/index.js | 2 + .../dd-trace/test/appsec/blocking.spec.js | 20 +- packages/dd-trace/test/appsec/index.spec.js | 138 +++++++-- .../test/appsec/remote_config/index.spec.js | 8 + .../test/appsec/response_blocking.spec.js | 271 ++++++++++++++++++ .../test/appsec/response_blocking_rules.json | 110 +++++++ packages/dd-trace/test/appsec/streamtest.txt | 1 + 12 files changed, 675 insertions(+), 33 deletions(-) create mode 100644 packages/dd-trace/test/appsec/response_blocking.spec.js create mode 100644 packages/dd-trace/test/appsec/response_blocking_rules.json create mode 100644 packages/dd-trace/test/appsec/streamtest.txt diff --git a/packages/datadog-instrumentations/src/http/server.js b/packages/datadog-instrumentations/src/http/server.js index 14bccd88994..18a00ac2789 100644 --- a/packages/datadog-instrumentations/src/http/server.js +++ b/packages/datadog-instrumentations/src/http/server.js @@ -10,6 +10,7 @@ const startServerCh = channel('apm:http:server:request:start') const exitServerCh = channel('apm:http:server:request:exit') const errorServerCh = channel('apm:http:server:request:error') const finishServerCh = channel('apm:http:server:request:finish') +const startWriteHeadCh = channel('apm:http:server:response:writeHead:start') const finishSetHeaderCh = channel('datadog:http:server:response:set-header:finish') const requestFinishedSet = new WeakSet() @@ -20,6 +21,9 @@ const httpsNames = ['https', 'node:https'] addHook({ name: httpNames }, http => { shimmer.wrap(http.ServerResponse.prototype, 'emit', wrapResponseEmit) shimmer.wrap(http.Server.prototype, 'emit', wrapEmit) + shimmer.wrap(http.ServerResponse.prototype, 'writeHead', wrapWriteHead) + shimmer.wrap(http.ServerResponse.prototype, 'write', wrapWrite) + shimmer.wrap(http.ServerResponse.prototype, 'end', wrapEnd) return http }) @@ -86,3 +90,97 @@ function wrapSetHeader (res) { } }) } + +function wrapWriteHead (writeHead) { + return function wrappedWriteHead (statusCode, reason, obj) { + if (!startWriteHeadCh.hasSubscribers) { + return writeHead.apply(this, arguments) + } + + const abortController = new AbortController() + + if (typeof reason !== 'string') { + obj ??= reason + } + + // support writeHead(200, ['key1', 'val1', 'key2', 'val2']) + if (Array.isArray(obj)) { + const headers = {} + + for (let i = 0; i < obj.length; i += 2) { + headers[obj[i]] = obj[i + 1] + } + + obj = headers + } + + // this doesn't support explicit duplicate headers, but it's an edge case + const responseHeaders = Object.assign(this.getHeaders(), obj) + + startWriteHeadCh.publish({ + req: this.req, + res: this, + abortController, + statusCode, + responseHeaders + }) + + if (abortController.signal.aborted) { + return this + } + + return writeHead.apply(this, arguments) + } +} + +function wrapWrite (write) { + return function wrappedWrite () { + if (!startWriteHeadCh.hasSubscribers) { + return write.apply(this, arguments) + } + + const abortController = new AbortController() + + const responseHeaders = this.getHeaders() + + startWriteHeadCh.publish({ + req: this.req, + res: this, + abortController, + statusCode: this.statusCode, + responseHeaders + }) + + if (abortController.signal.aborted) { + return true + } + + return write.apply(this, arguments) + } +} + +function wrapEnd (end) { + return function wrappedEnd () { + if (!startWriteHeadCh.hasSubscribers) { + return end.apply(this, arguments) + } + + const abortController = new AbortController() + + const responseHeaders = this.getHeaders() + + startWriteHeadCh.publish({ + req: this.req, + res: this, + abortController, + statusCode: this.statusCode, + responseHeaders + }) + + if (abortController.signal.aborted) { + return this + } + + return end.apply(this, arguments) + } +} diff --git a/packages/dd-trace/src/appsec/blocking.js b/packages/dd-trace/src/appsec/blocking.js index 141667b9d57..f3d2103b59e 100644 --- a/packages/dd-trace/src/appsec/blocking.js +++ b/packages/dd-trace/src/appsec/blocking.js @@ -111,6 +111,10 @@ function block (req, res, rootSpan, abortController, actionParameters) { const { body, headers, statusCode } = getBlockingData(req, null, rootSpan, actionParameters) + for (const headerName of res.getHeaderNames()) { + res.removeHeader(headerName) + } + res.writeHead(statusCode, headers).end(body) abortController?.abort() diff --git a/packages/dd-trace/src/appsec/channels.js b/packages/dd-trace/src/appsec/channels.js index 57a3c29676c..82e7eb5f6f7 100644 --- a/packages/dd-trace/src/appsec/channels.js +++ b/packages/dd-trace/src/appsec/channels.js @@ -18,5 +18,6 @@ module.exports = { nextBodyParsed: dc.channel('apm:next:body-parsed'), nextQueryParsed: dc.channel('apm:next:query-parsed'), responseBody: dc.channel('datadog:express:response:json:start'), + responseWriteHead: dc.channel('apm:http:server:response:writeHead:start'), httpClientRequestStart: dc.channel('apm:http:client:request:start') } diff --git a/packages/dd-trace/src/appsec/index.js b/packages/dd-trace/src/appsec/index.js index 76e67a0ef72..af0ffc934f3 100644 --- a/packages/dd-trace/src/appsec/index.js +++ b/packages/dd-trace/src/appsec/index.js @@ -12,7 +12,8 @@ const { queryParser, nextBodyParsed, nextQueryParsed, - responseBody + responseBody, + responseWriteHead } = require('./channels') const waf = require('./waf') const addresses = require('./addresses') @@ -60,6 +61,7 @@ function enable (_config) { queryParser.subscribe(onRequestQueryParsed) cookieParser.subscribe(onRequestCookieParser) responseBody.subscribe(onResponseBody) + responseWriteHead.subscribe(onResponseWriteHead) if (_config.appsec.eventTracking.enabled) { passportVerify.subscribe(onPassportVerify) @@ -110,14 +112,7 @@ function incomingHttpStartTranslator ({ req, res, abortController }) { } function incomingHttpEndTranslator ({ req, res }) { - // TODO: this doesn't support headers sent with res.writeHead() - const responseHeaders = Object.assign({}, res.getHeaders()) - delete responseHeaders['set-cookie'] - - const persistent = { - [addresses.HTTP_INCOMING_RESPONSE_CODE]: '' + res.statusCode, - [addresses.HTTP_INCOMING_RESPONSE_HEADERS]: responseHeaders - } + const persistent = {} // we need to keep this to support other body parsers // TODO: no need to analyze it if it was already done by the body-parser hook @@ -139,7 +134,9 @@ function incomingHttpEndTranslator ({ req, res }) { persistent[addresses.HTTP_INCOMING_QUERY] = req.query } - waf.run({ persistent }, req) + if (Object.keys(persistent).length) { + waf.run({ persistent }, req) + } waf.disposeContext(req) @@ -225,12 +222,48 @@ function onPassportVerify ({ credentials, user }) { passportTrackEvent(credentials, user, rootSpan, config.appsec.eventTracking.mode) } +const responseAnalyzedSet = new WeakSet() +const responseBlockedSet = new WeakSet() + +function onResponseWriteHead ({ req, res, abortController, statusCode, responseHeaders }) { + // avoid "write after end" error + if (responseBlockedSet.has(res)) { + abortController?.abort() + return + } + + // avoid double waf call + if (responseAnalyzedSet.has(res)) { + return + } + + const rootSpan = web.root(req) + if (!rootSpan) return + + responseHeaders = Object.assign({}, responseHeaders) + delete responseHeaders['set-cookie'] + + const results = waf.run({ + persistent: { + [addresses.HTTP_INCOMING_RESPONSE_CODE]: '' + statusCode, + [addresses.HTTP_INCOMING_RESPONSE_HEADERS]: responseHeaders + } + }, req) + + responseAnalyzedSet.add(res) + + handleResults(results, req, res, rootSpan, abortController) +} + function handleResults (actions, req, res, rootSpan, abortController) { if (!actions || !req || !res || !rootSpan || !abortController) return const blockingAction = getBlockingAction(actions) if (blockingAction) { block(req, res, rootSpan, abortController, blockingAction) + if (!abortController.signal || abortController.signal.aborted) { + responseBlockedSet.add(res) + } } } @@ -256,6 +289,7 @@ function disable () { if (cookieParser.hasSubscribers) cookieParser.unsubscribe(onRequestCookieParser) if (responseBody.hasSubscribers) responseBody.unsubscribe(onResponseBody) if (passportVerify.hasSubscribers) passportVerify.unsubscribe(onPassportVerify) + if (responseWriteHead.hasSubscribers) responseWriteHead.unsubscribe(onResponseWriteHead) } module.exports = { diff --git a/packages/dd-trace/src/appsec/remote_config/capabilities.js b/packages/dd-trace/src/appsec/remote_config/capabilities.js index 61684e171f0..6e320493336 100644 --- a/packages/dd-trace/src/appsec/remote_config/capabilities.js +++ b/packages/dd-trace/src/appsec/remote_config/capabilities.js @@ -6,6 +6,7 @@ module.exports = { ASM_DD_RULES: 1n << 3n, ASM_EXCLUSIONS: 1n << 4n, ASM_REQUEST_BLOCKING: 1n << 5n, + ASM_RESPONSE_BLOCKING: 1n << 6n, ASM_USER_BLOCKING: 1n << 7n, ASM_CUSTOM_RULES: 1n << 8n, ASM_CUSTOM_BLOCKING_RESPONSE: 1n << 9n, diff --git a/packages/dd-trace/src/appsec/remote_config/index.js b/packages/dd-trace/src/appsec/remote_config/index.js index f39d02347eb..169e5c2dff7 100644 --- a/packages/dd-trace/src/appsec/remote_config/index.js +++ b/packages/dd-trace/src/appsec/remote_config/index.js @@ -71,6 +71,7 @@ function enableWafUpdate (appsecConfig) { rc.updateCapabilities(RemoteConfigCapabilities.ASM_DD_RULES, true) rc.updateCapabilities(RemoteConfigCapabilities.ASM_EXCLUSIONS, true) rc.updateCapabilities(RemoteConfigCapabilities.ASM_REQUEST_BLOCKING, true) + rc.updateCapabilities(RemoteConfigCapabilities.ASM_RESPONSE_BLOCKING, true) rc.updateCapabilities(RemoteConfigCapabilities.ASM_CUSTOM_RULES, true) rc.updateCapabilities(RemoteConfigCapabilities.ASM_CUSTOM_BLOCKING_RESPONSE, true) rc.updateCapabilities(RemoteConfigCapabilities.ASM_TRUSTED_IPS, true) @@ -92,6 +93,7 @@ function disableWafUpdate () { rc.updateCapabilities(RemoteConfigCapabilities.ASM_DD_RULES, false) rc.updateCapabilities(RemoteConfigCapabilities.ASM_EXCLUSIONS, false) rc.updateCapabilities(RemoteConfigCapabilities.ASM_REQUEST_BLOCKING, false) + rc.updateCapabilities(RemoteConfigCapabilities.ASM_RESPONSE_BLOCKING, false) rc.updateCapabilities(RemoteConfigCapabilities.ASM_CUSTOM_RULES, false) rc.updateCapabilities(RemoteConfigCapabilities.ASM_CUSTOM_BLOCKING_RESPONSE, false) rc.updateCapabilities(RemoteConfigCapabilities.ASM_TRUSTED_IPS, false) diff --git a/packages/dd-trace/test/appsec/blocking.spec.js b/packages/dd-trace/test/appsec/blocking.spec.js index a0d454a77c7..04a3c496b46 100644 --- a/packages/dd-trace/test/appsec/blocking.spec.js +++ b/packages/dd-trace/test/appsec/blocking.spec.js @@ -37,7 +37,9 @@ describe('blocking', () => { res = { setHeader: sinon.stub(), writeHead: sinon.stub(), - end: sinon.stub() + end: sinon.stub(), + getHeaderNames: sinon.stub().returns([]), + removeHeader: sinon.stub() } res.writeHead.returns(res) @@ -109,6 +111,22 @@ describe('blocking', () => { expect(res.end).to.have.been.calledOnceWithExactly('jsonBody') expect(abortController.signal.aborted).to.be.true }) + + it('should remove all headers before sending blocking response', () => { + res.getHeaderNames.returns(['header1', 'header2']) + + block(req, res, rootSpan) + + expect(rootSpan.addTags).to.have.been.calledOnceWithExactly({ 'appsec.blocked': 'true' }) + expect(res.removeHeader).to.have.been.calledTwice + expect(res.removeHeader.firstCall).to.have.been.calledWithExactly('header1') + expect(res.removeHeader.secondCall).to.have.been.calledWithExactly('header2') + expect(res.writeHead).to.have.been.calledOnceWithExactly(403, { + 'Content-Type': 'application/json', + 'Content-Length': 8 + }) + expect(res.end).to.have.been.calledOnceWithExactly('jsonBody') + }) }) describe('block with default templates', () => { diff --git a/packages/dd-trace/test/appsec/index.spec.js b/packages/dd-trace/test/appsec/index.spec.js index febac128f83..652aae028ec 100644 --- a/packages/dd-trace/test/appsec/index.spec.js +++ b/packages/dd-trace/test/appsec/index.spec.js @@ -12,7 +12,8 @@ const { incomingHttpRequestEnd, queryParser, passportVerify, - responseBody + responseBody, + responseWriteHead } = require('../../src/appsec/channels') const Reporter = require('../../src/appsec/reporter') const agent = require('../plugins/agent') @@ -166,6 +167,7 @@ describe('AppSec Index', () => { expect(cookieParser.hasSubscribers).to.be.false expect(queryParser.hasSubscribers).to.be.false expect(passportVerify.hasSubscribers).to.be.false + expect(responseWriteHead.hasSubscribers).to.be.false AppSec.enable(config) @@ -173,6 +175,7 @@ describe('AppSec Index', () => { expect(cookieParser.hasSubscribers).to.be.true expect(queryParser.hasSubscribers).to.be.true expect(passportVerify.hasSubscribers).to.be.true + expect(responseWriteHead.hasSubscribers).to.be.true }) it('should not subscribe to passportVerify if eventTracking is disabled', () => { @@ -249,6 +252,7 @@ describe('AppSec Index', () => { expect(cookieParser.hasSubscribers).to.be.false expect(queryParser.hasSubscribers).to.be.false expect(passportVerify.hasSubscribers).to.be.false + expect(responseWriteHead.hasSubscribers).to.be.false }) it('should call appsec telemetry disable', () => { @@ -320,7 +324,7 @@ describe('AppSec Index', () => { web.root.returns(rootSpan) }) - it('should propagate incoming http end data', () => { + it('should not propagate incoming http end data without express', () => { const req = { url: '/path', headers: { @@ -348,17 +352,12 @@ describe('AppSec Index', () => { AppSec.incomingHttpEndTranslator({ req, res }) - expect(waf.run).to.have.been.calledOnceWithExactly({ - persistent: { - 'server.response.status': '201', - 'server.response.headers.no_cookies': { 'content-type': 'application/json', 'content-lenght': 42 } - } - }, req) + expect(waf.run).to.have.not.been.called expect(Reporter.finishRequest).to.have.been.calledOnceWithExactly(req, res) }) - it('should propagate incoming http end data with invalid framework properties', () => { + it('should not propagate incoming http end data with invalid framework properties', () => { const req = { url: '/path', headers: { @@ -391,12 +390,7 @@ describe('AppSec Index', () => { AppSec.incomingHttpEndTranslator({ req, res }) - expect(waf.run).to.have.been.calledOnceWithExactly({ - persistent: { - 'server.response.status': '201', - 'server.response.headers.no_cookies': { 'content-type': 'application/json', 'content-lenght': 42 } - } - }, req) + expect(waf.run).to.have.not.been.called expect(Reporter.finishRequest).to.have.been.calledOnceWithExactly(req, res) }) @@ -446,8 +440,6 @@ describe('AppSec Index', () => { expect(waf.run).to.have.been.calledOnceWithExactly({ persistent: { - 'server.response.status': '201', - 'server.response.headers.no_cookies': { 'content-type': 'application/json', 'content-lenght': 42 }, 'server.request.body': { a: '1' }, 'server.request.path_params': { c: '3' }, 'server.request.cookies': { d: '4', e: '5' }, @@ -652,7 +644,8 @@ describe('AppSec Index', () => { 'content-lenght': 42 }), writeHead: sinon.stub(), - end: sinon.stub() + end: sinon.stub(), + getHeaderNames: sinon.stub().returns([]) } res.writeHead.returns(res) @@ -660,10 +653,6 @@ describe('AppSec Index', () => { AppSec.incomingHttpStartTranslator({ req, res }) }) - afterEach(() => { - AppSec.disable() - }) - describe('onRequestBodyParsed', () => { it('Should not block without body', () => { sinon.stub(waf, 'run') @@ -822,6 +811,111 @@ describe('AppSec Index', () => { expect(passport.passportTrackEvent).not.to.have.been.called }) }) + + describe('onResponseWriteHead', () => { + it('should call abortController if response was already blocked', () => { + sinon.stub(waf, 'run').returns(resultActions) + + const responseHeaders = { + 'content-type': 'application/json', + 'content-lenght': 42, + 'set-cookie': 'a=1;b=2' + } + + responseWriteHead.publish({ req, res, abortController, statusCode: 404, responseHeaders }) + + expect(waf.run).to.have.been.calledOnceWithExactly({ + persistent: { + 'server.response.status': '404', + 'server.response.headers.no_cookies': { + 'content-type': 'application/json', + 'content-lenght': 42 + } + } + }, req) + expect(abortController.abort).to.have.been.calledOnce + expect(res.end).to.have.been.calledOnce + + abortController.abort.resetHistory() + + responseWriteHead.publish({ req, res, abortController, statusCode: 404, responseHeaders }) + + expect(waf.run).to.have.been.calledOnce + expect(abortController.abort).to.have.been.calledOnce + expect(res.end).to.have.been.calledOnce + }) + + it('should not call the WAF if response was already analyzed', () => { + sinon.stub(waf, 'run').returns(null) + + const responseHeaders = { + 'content-type': 'application/json', + 'content-lenght': 42, + 'set-cookie': 'a=1;b=2' + } + + responseWriteHead.publish({ req, res, abortController, statusCode: 404, responseHeaders }) + + expect(waf.run).to.have.been.calledOnceWithExactly({ + persistent: { + 'server.response.status': '404', + 'server.response.headers.no_cookies': { + 'content-type': 'application/json', + 'content-lenght': 42 + } + } + }, req) + expect(abortController.abort).to.have.not.been.called + expect(res.end).to.have.not.been.called + + responseWriteHead.publish({ req, res, abortController, statusCode: 404, responseHeaders }) + + expect(waf.run).to.have.been.calledOnce + expect(abortController.abort).to.have.not.been.called + expect(res.end).to.have.not.been.called + }) + + it('should not do anything without a root span', () => { + web.root.returns(null) + sinon.stub(waf, 'run').returns(null) + + const responseHeaders = { + 'content-type': 'application/json', + 'content-lenght': 42, + 'set-cookie': 'a=1;b=2' + } + + responseWriteHead.publish({ req, res, abortController, statusCode: 404, responseHeaders }) + + expect(waf.run).to.have.not.been.called + expect(abortController.abort).to.have.not.been.called + expect(res.end).to.have.not.been.called + }) + + it('should call the WAF with responde code and headers', () => { + sinon.stub(waf, 'run').returns(resultActions) + + const responseHeaders = { + 'content-type': 'application/json', + 'content-lenght': 42, + 'set-cookie': 'a=1;b=2' + } + + responseWriteHead.publish({ req, res, abortController, statusCode: 404, responseHeaders }) + + expect(waf.run).to.have.been.calledOnceWithExactly({ + persistent: { + 'server.response.status': '404', + 'server.response.headers.no_cookies': { + 'content-type': 'application/json', + 'content-lenght': 42 + } + } + }, req) + expect(abortController.abort).to.have.been.calledOnce + expect(res.end).to.have.been.calledOnce + }) + }) }) describe('Metrics', () => { diff --git a/packages/dd-trace/test/appsec/remote_config/index.spec.js b/packages/dd-trace/test/appsec/remote_config/index.spec.js index 1307fbed444..d954e41e15b 100644 --- a/packages/dd-trace/test/appsec/remote_config/index.spec.js +++ b/packages/dd-trace/test/appsec/remote_config/index.spec.js @@ -276,6 +276,8 @@ describe('Remote Config index', () => { .to.have.been.calledWithExactly(RemoteConfigCapabilities.ASM_EXCLUSIONS, true) expect(rc.updateCapabilities) .to.have.been.calledWithExactly(RemoteConfigCapabilities.ASM_REQUEST_BLOCKING, true) + expect(rc.updateCapabilities) + .to.have.been.calledWithExactly(RemoteConfigCapabilities.ASM_RESPONSE_BLOCKING, true) expect(rc.updateCapabilities) .to.have.been.calledWithExactly(RemoteConfigCapabilities.ASM_CUSTOM_RULES, true) expect(rc.updateCapabilities) @@ -304,6 +306,8 @@ describe('Remote Config index', () => { .to.have.been.calledWithExactly(RemoteConfigCapabilities.ASM_EXCLUSIONS, true) expect(rc.updateCapabilities) .to.have.been.calledWithExactly(RemoteConfigCapabilities.ASM_REQUEST_BLOCKING, true) + expect(rc.updateCapabilities) + .to.have.been.calledWithExactly(RemoteConfigCapabilities.ASM_RESPONSE_BLOCKING, true) expect(rc.updateCapabilities) .to.have.been.calledWithExactly(RemoteConfigCapabilities.ASM_CUSTOM_RULES, true) expect(rc.updateCapabilities) @@ -334,6 +338,8 @@ describe('Remote Config index', () => { .to.have.been.calledWithExactly(RemoteConfigCapabilities.ASM_EXCLUSIONS, true) expect(rc.updateCapabilities) .to.have.been.calledWithExactly(RemoteConfigCapabilities.ASM_REQUEST_BLOCKING, true) + expect(rc.updateCapabilities) + .to.have.been.calledWithExactly(RemoteConfigCapabilities.ASM_RESPONSE_BLOCKING, true) expect(rc.updateCapabilities) .to.have.been.calledWithExactly(RemoteConfigCapabilities.ASM_CUSTOM_RULES, true) expect(rc.updateCapabilities) @@ -359,6 +365,8 @@ describe('Remote Config index', () => { .to.have.been.calledWithExactly(RemoteConfigCapabilities.ASM_EXCLUSIONS, false) expect(rc.updateCapabilities) .to.have.been.calledWithExactly(RemoteConfigCapabilities.ASM_REQUEST_BLOCKING, false) + expect(rc.updateCapabilities) + .to.have.been.calledWithExactly(RemoteConfigCapabilities.ASM_RESPONSE_BLOCKING, false) expect(rc.updateCapabilities) .to.have.been.calledWithExactly(RemoteConfigCapabilities.ASM_CUSTOM_RULES, false) expect(rc.updateCapabilities) diff --git a/packages/dd-trace/test/appsec/response_blocking.spec.js b/packages/dd-trace/test/appsec/response_blocking.spec.js new file mode 100644 index 00000000000..672933784e7 --- /dev/null +++ b/packages/dd-trace/test/appsec/response_blocking.spec.js @@ -0,0 +1,271 @@ +'use strict' + +const { assert } = require('chai') +const getPort = require('get-port') +const agent = require('../plugins/agent') +const Axios = require('axios') +const appsec = require('../../src/appsec') +const Config = require('../../src/config') +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') + +describe('HTTP Response Blocking', () => { + let server + let responseHandler + let axios + + before(async () => { + const port = await getPort() + + await agent.load('http') + + const http = require('http') + + server = new http.Server((req, res) => { + // little polyfill, older versions of node don't have setHeaders() + if (typeof res.setHeaders !== 'function') { + res.setHeaders = headers => headers.forEach((v, k) => res.setHeader(k, v)) + } + + if (responseHandler) { + responseHandler(req, res) + } else { + res.writeHead(200) + res.end('OK') + } + }) + + await new Promise((resolve, reject) => { + server.listen(port, 'localhost') + .once('listening', resolve) + .once('error', reject) + }) + + axios = Axios.create(({ + baseURL: `http://localhost:${port}`, + validateStatus: null + })) + + appsec.enable(new Config({ + appsec: { + enabled: true, + rules: path.join(__dirname, 'response_blocking_rules.json') + } + })) + }) + + beforeEach(() => { + sinon.spy(WafContext.prototype, 'run') + }) + + afterEach(() => { + sinon.restore() + responseHandler = null + }) + + after(() => { + appsec.disable() + server?.close() + return agent.close({ ritmReset: false }) + }) + + it('should block with implicit statusCode + setHeader() + end()', async () => { + responseHandler = (req, res) => { + res.statusCode = 404 + res.setHeader('k', '404') + res.end('end') + } + + const res = await axios.get('/') + + assertBlocked(res) + }) + + it('should block with setHeader() + setHeaders() + writeHead() headers', async () => { + responseHandler = (req, res) => { + res.setHeaders(new Map(Object.entries({ a: 'bad1', b: 'good' }))) + res.setHeader('c', 'bad2') + res.writeHead(200, { d: 'bad3' }) + res.end('end') + } + + const res = await axios.get('/') + + assertBlocked(res) + }) + + it('should block with setHeader() + array writeHead() ', async () => { + responseHandler = (req, res) => { + res.setHeader('a', 'bad1') + res.writeHead(200, 'OK', ['b', 'bad2', 'c', 'bad3']) + res.end('end') + } + + const res = await axios.get('/') + + assertBlocked(res) + }) + + it('should not block with array writeHead() when attack is in the header name and not in header value', async () => { + responseHandler = (req, res) => { + res.writeHead(200, 'OK', ['a', 'bad1', 'b', 'bad2', 'bad3', 'c']) + res.end('end') + } + + const res = await axios.get('/') + + assert.equal(res.status, 200) + assert.hasAllKeys(cloneHeaders(res.headers), [ + 'a', + 'b', + 'bad3', + 'date', + 'connection', + 'transfer-encoding' + ]) + assert.deepEqual(res.data, 'end') + }) + + it('should block with implicit statusCode + setHeader() + flushHeaders()', async () => { + responseHandler = (req, res) => { + res.statusCode = 404 + res.setHeader('k', '404') + res.flushHeaders() + res.end('end') + } + + const res = await axios.get('/') + + assertBlocked(res) + }) + + it('should block with implicit statusCode + setHeader() + write()', async () => { + responseHandler = (req, res) => { + res.statusCode = 404 + res.setHeader('k', '404') + res.write('write') + res.end('end') + } + + const res = await axios.get('/') + + assertBlocked(res) + }) + + it('should block with implicit statusCode + setHeader() + stream pipe', async () => { + responseHandler = (req, res) => { + res.statusCode = 404 + res.setHeader('k', '404') + streamFile(res) + } + + const res = await axios.get('/') + + assertBlocked(res) + }) + + it('should block with writeHead() + write()', async () => { + responseHandler = (req, res) => { + res.writeHead(404, { k: '404' }) + res.write('write') + res.end('end') + } + + const res = await axios.get('/') + + assertBlocked(res) + }) + + it('should block with every methods combined', async () => { + responseHandler = (req, res) => { + res.setHeaders(new Map(Object.entries({ a: 'bad1', b: 'good' }))) + res.setHeader('c', 'bad2') + res.setHeader('d', 'good') + res.writeHead(200, 'OK', { d: 'good', e: 'bad3' }) + res.flushHeaders() + res.write('write') + res.addTrailers({ k: 'v' }) + streamFile(res) + } + + const res = await axios.get('/') + + assertBlocked(res) + }) + + it('should not block with every methods combined but no attack', async () => { + responseHandler = (req, res) => { + res.setHeaders(new Map(Object.entries({ a: 'good', b: 'good' }))) + res.setHeader('c', 'good') + res.setHeader('d', 'good') + res.writeHead(201, 'OK', { d: 'good', e: 'good' }) + res.flushHeaders() + res.write('write') + res.addTrailers({ k: 'v' }) + streamFile(res) + } + + const res = await axios.get('/') + + assert.equal(res.status, 201) + assert.hasAllKeys(cloneHeaders(res.headers), [ + 'a', + 'b', + 'c', + 'd', + 'e', + 'date', + 'connection', + 'transfer-encoding' + ]) + assert.deepEqual(res.data, 'writefileend') + }) + + it('should ignore subsequent response writes after blocking', async () => { + responseHandler = (req, res) => { + res.statusCode = 404 + res.setHeader('k', '404') + res.flushHeaders() + res.writeHead(200, { k: '200' }) + res.write('write1') + setTimeout(() => { + res.write('write2') + res.end('end') + }, 1000) + } + + const res = await axios.get('/') + + assertBlocked(res) + }) +}) + +function cloneHeaders (headers) { + // clone the headers accessor to a flat object + // and delete the keep-alive header as it's not always present + headers = Object.fromEntries(Object.entries(headers)) + delete headers['keep-alive'] + + return headers +} + +function assertBlocked (res) { + assert.equal(res.status, 403) + assert.hasAllKeys(cloneHeaders(res.headers), [ + 'content-type', + 'content-length', + 'date', + 'connection' + ]) + assert.deepEqual(res.data, blockingResponse) + + sinon.assert.callCount(WafContext.prototype.run, 2) +} + +function streamFile (res) { + const stream = fs.createReadStream(path.join(__dirname, 'streamtest.txt'), { encoding: 'utf8' }) + stream.pipe(res, { end: false }) + stream.on('end', () => res.end('end')) +} diff --git a/packages/dd-trace/test/appsec/response_blocking_rules.json b/packages/dd-trace/test/appsec/response_blocking_rules.json new file mode 100644 index 00000000000..bd7d1279892 --- /dev/null +++ b/packages/dd-trace/test/appsec/response_blocking_rules.json @@ -0,0 +1,110 @@ +{ + "version": "2.2", + "metadata": { + "rules_version": "1.5.0" + }, + "rules": [ + { + "id": "test-rule-id-1", + "name": "test-rule-name-1", + "tags": { + "type": "security_scanner1", + "category": "attack_attempt1" + }, + "conditions": [ + { + "operator": "match_regex", + "parameters": { + "inputs": [ + { + "address": "server.response.status" + } + ], + "regex": "^404$", + "options": { + "case_sensitive": true + } + } + }, + { + "operator": "match_regex", + "parameters": { + "inputs": [ + { + "address": "server.response.headers.no_cookies" + } + ], + "regex": "^404$", + "options": { + "case_sensitive": false + } + } + } + ], + "transformers": [ + "lowercase" + ], + "on_match": [ + "block" + ] + }, + { + "id": "test-rule-id-2", + "name": "test-rule-name-2", + "tags": { + "type": "security_scanner2", + "category": "attack_attempt2" + }, + "conditions": [ + { + "operator": "match_regex", + "parameters": { + "inputs": [ + { + "address": "server.response.headers.no_cookies" + } + ], + "regex": "^bad1$", + "options": { + "case_sensitive": false + } + } + }, + { + "operator": "match_regex", + "parameters": { + "inputs": [ + { + "address": "server.response.headers.no_cookies" + } + ], + "regex": "^bad2$", + "options": { + "case_sensitive": false + } + } + }, + { + "operator": "match_regex", + "parameters": { + "inputs": [ + { + "address": "server.response.headers.no_cookies" + } + ], + "regex": "^bad3$", + "options": { + "case_sensitive": false + } + } + } + ], + "transformers": [ + "lowercase" + ], + "on_match": [ + "block" + ] + } + ] +} diff --git a/packages/dd-trace/test/appsec/streamtest.txt b/packages/dd-trace/test/appsec/streamtest.txt new file mode 100644 index 00000000000..1a010b1c0f0 --- /dev/null +++ b/packages/dd-trace/test/appsec/streamtest.txt @@ -0,0 +1 @@ +file \ No newline at end of file