From 1a4e8e64fe5d5c39ac40953eff7d35c7a146d0cd Mon Sep 17 00:00:00 2001 From: Ugaitz Urien Date: Thu, 27 Jun 2024 09:26:02 +0200 Subject: [PATCH] Collect request headers on user event (#4385) --------- Co-authored-by: Carles Capell <107924659+CarlesDD@users.noreply.github.com> Co-authored-by: simon-id --- packages/dd-trace/src/appsec/reporter.js | 52 ++++-- .../dd-trace/test/appsec/reporter.spec.js | 154 ++++++++++++++++-- 2 files changed, 176 insertions(+), 30 deletions(-) diff --git a/packages/dd-trace/src/appsec/reporter.js b/packages/dd-trace/src/appsec/reporter.js index a75e1c92984..e652eaa2099 100644 --- a/packages/dd-trace/src/appsec/reporter.js +++ b/packages/dd-trace/src/appsec/reporter.js @@ -29,19 +29,17 @@ const contentHeaderList = [ 'content-language' ] -const REQUEST_HEADERS_MAP = mapHeaderAndTags([ +const EVENT_HEADERS_MAP = mapHeaderAndTags([ ...ipHeaderList, 'forwarded', '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', @@ -50,6 +48,14 @@ const IDENTIFICATION_HEADERS_MAP = mapHeaderAndTags([ 'x-sigsci-requestid', 'x-sigsci-tags', 'akamai-user-risk' +] + +// these request headers are always collected - it breaks the expected spec orders +const REQUEST_HEADERS_MAP = mapHeaderAndTags([ + 'content-type', + 'user-agent', + 'accept', + ...identificationHeaders ], 'http.request.headers.') const RESPONSE_HEADERS_MAP = mapHeaderAndTags(contentHeaderList, 'http.response.headers.') @@ -118,9 +124,9 @@ function reportAttack (attackData) { const currentTags = rootSpan.context()._tags - const newTags = filterHeaders(req.headers, REQUEST_HEADERS_MAP) - - newTags['appsec.event'] = 'true' + const newTags = { + 'appsec.event': 'true' + } if (limiter.isAllowed()) { newTags[MANUAL_KEEP] = 'true' @@ -142,11 +148,6 @@ function reportAttack (attackData) { newTags['_dd.appsec.json'] = '{"triggers":' + attackData + '}' } - const ua = newTags['http.request.headers.user-agent'] - if (ua) { - newTags['http.useragent'] = ua - } - newTags['network.client.ip'] = req.socket.remoteAddress rootSpan.addTags(newTags) @@ -205,19 +206,40 @@ 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, REQUEST_HEADERS_MAP) + const ua = mandatoryTags['http.request.headers.user-agent'] + if (ua) { + mandatoryTags['http.useragent'] = ua + } + rootSpan.addTags(mandatoryTags) - if (!rootSpan.context()._tags['appsec.event']) return + const tags = rootSpan.context()._tags + if (!shouldCollectEventHeaders(tags)) return const newTags = filterHeaders(res.getHeaders(), RESPONSE_HEADERS_MAP) + Object.assign(newTags, filterHeaders(req.headers, EVENT_HEADERS_MAP)) - if (req.route && typeof req.route.path === 'string') { + if (tags['appsec.event'] === 'true' && typeof req.route?.path === 'string') { newTags['http.endpoint'] = req.route.path } rootSpan.addTags(newTags) } +function shouldCollectEventHeaders (tags = {}) { + if (tags['appsec.event'] === 'true') { + return true + } + + for (const tagName of Object.keys(tags)) { + if (tagName.startsWith('appsec.events.')) { + return true + } + } + + return false +} + function setRateLimit (rateLimit) { limiter = new Limiter(rateLimit) } diff --git a/packages/dd-trace/test/appsec/reporter.spec.js b/packages/dd-trace/test/appsec/reporter.spec.js index 721534f6d5b..b8ce6d94fb6 100644 --- a/packages/dd-trace/test/appsec/reporter.spec.js +++ b/packages/dd-trace/test/appsec/reporter.spec.js @@ -225,9 +225,6 @@ describe('reporter', () => { 'manual.keep': 'true', '_dd.origin': 'appsec', '_dd.appsec.json': '{"triggers":[{"rule":{},"rule_matches":[{}]}]}', - 'http.request.headers.host': 'localhost', - 'http.request.headers.user-agent': 'arachni', - 'http.useragent': 'arachni', 'network.client.ip': '8.8.8.8' }) }) @@ -267,12 +264,9 @@ describe('reporter', () => { expect(web.root).to.have.been.calledOnceWith(req) expect(span.addTags).to.have.been.calledOnceWithExactly({ - 'http.request.headers.host': 'localhost', - 'http.request.headers.user-agent': 'arachni', 'appsec.event': 'true', 'manual.keep': 'true', '_dd.appsec.json': '{"triggers":[]}', - 'http.useragent': 'arachni', 'network.client.ip': '8.8.8.8' }) }) @@ -285,13 +279,10 @@ describe('reporter', () => { expect(web.root).to.have.been.calledOnceWith(req) expect(span.addTags).to.have.been.calledOnceWithExactly({ - 'http.request.headers.host': 'localhost', - 'http.request.headers.user-agent': 'arachni', 'appsec.event': 'true', 'manual.keep': 'true', '_dd.origin': 'appsec', '_dd.appsec.json': '{"triggers":[{"rule":{},"rule_matches":[{}]},{"rule":{}},{"rule":{},"rule_matches":[{}]}]}', - 'http.useragent': 'arachni', 'network.client.ip': '8.8.8.8' }) }) @@ -304,13 +295,10 @@ describe('reporter', () => { expect(web.root).to.have.been.calledOnceWith(req) expect(span.addTags).to.have.been.calledOnceWithExactly({ - 'http.request.headers.host': 'localhost', - 'http.request.headers.user-agent': 'arachni', 'appsec.event': 'true', 'manual.keep': 'true', '_dd.origin': 'appsec', '_dd.appsec.json': '{"triggers":[{"rule":{},"rule_matches":[{}]},{"rule":{}},{"rule":{},"rule_matches":[{}]}]}', - 'http.useragent': 'arachni', 'network.client.ip': '8.8.8.8' }) @@ -365,6 +353,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() @@ -398,7 +413,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', @@ -409,7 +424,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', + accept: 'j', + 'user-agent': 'k' } } @@ -423,7 +441,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.accept': 'j', + 'http.request.headers.user-agent': 'k', + 'http.useragent': 'k' }) }) @@ -484,6 +506,108 @@ describe('reporter', () => { }) }) + it('should add http request data inside request span when appsec.event is true', () => { + const req = { + headers: { + 'user-agent': 'arachni', + ...requestHeadersAndValuesToTrackOnEvent + } + } + const res = { + getHeaders: () => { + return {} + } + } + span.context()._tags['appsec.event'] = 'true' + + Reporter.finishRequest(req, res) + + expect(span.addTags).to.have.been.calledWithExactly({ + '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: { + 'user-agent': 'arachni', + ...requestHeadersAndValuesToTrackOnEvent + } + } + const res = { + getHeaders: () => { + return {} + } + } + + span.context() + ._tags['appsec.events.users.login.success.track'] = 'true' + + Reporter.finishRequest(req, res) + + expect(span.addTags).to.have.been.calledWithExactly({ + '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: { + 'user-agent': 'arachni', + ...requestHeadersAndValuesToTrackOnEvent + } + } + const res = { + getHeaders: () => { + return {} + } + } + + span.context() + ._tags['appsec.events.users.login.failure.track'] = 'true' + + Reporter.finishRequest(req, res) + + expect(span.addTags).to.have.been.calledWithExactly({ + '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: { + 'user-agent': 'arachni', + ...requestHeadersAndValuesToTrackOnEvent + } + } + const res = { + getHeaders: () => { + return {} + } + } + + span.context() + ._tags['appsec.events.custon.event.track'] = 'true' + + Reporter.finishRequest(req, res) + + expect(span.addTags).to.have.been.calledWithExactly({ + 'http.request.headers.user-agent': 'arachni', + 'http.useragent': 'arachni' + }) + + expect(span.addTags).to.have.been.calledWithExactly(expectedRequestTagsToTrackOnEvent) + }) + it('should call incrementWafRequestsMetric', () => { const req = {} const res = {}