Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Collect request headers on user event #4385

Merged
merged 12 commits into from
Jun 27, 2024
Merged

Collect request headers on user event #4385

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
a5f0438
Collect request headers on user event
uurien Jun 6, 2024
ccac65a
Send mandatory request headers always
uurien Jun 7, 2024
2ce6fcf
Merge branch 'master' into ugaitz/collect-headers-on-user-event
uurien Jun 11, 2024
54297ad
Small changes
uurien Jun 11, 2024
6b1b221
Update packages/dd-trace/src/appsec/reporter.js
uurien Jun 11, 2024
3cd1943
Merge branch 'master' into ugaitz/collect-headers-on-user-event
uurien Jun 12, 2024
3e25a3a
Update packages/dd-trace/src/appsec/reporter.js
uurien Jun 24, 2024
00997c3
Update packages/dd-trace/src/appsec/reporter.js
uurien Jun 24, 2024
465e1e0
Comments in the PR
uurien Jun 24, 2024
1916bd6
Apply git comment
uurien Jun 24, 2024
0cff8cf
Merge branch 'master' into ugaitz/collect-headers-on-user-event
uurien Jun 24, 2024
6d9d9cf
Fix new test
uurien Jun 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 37 additions & 15 deletions packages/dd-trace/src/appsec/reporter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,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',
Expand All @@ -48,6 +46,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
uurien marked this conversation as resolved.
Show resolved Hide resolved
const REQUEST_HEADERS_MAP = mapHeaderAndTags([
'content-type',
'user-agent',
'accept',
...identificationHeaders
simon-id marked this conversation as resolved.
Show resolved Hide resolved
], 'http.request.headers.')

const RESPONSE_HEADERS_MAP = mapHeaderAndTags(contentHeaderList, 'http.response.headers.')
Expand Down Expand Up @@ -116,9 +122,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' // TODO: figure out how to keep appsec traces with sampling revamp
Expand All @@ -138,11 +144,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)
Expand Down Expand Up @@ -199,19 +200,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) {
simon-id marked this conversation as resolved.
Show resolved Hide resolved
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
}
uurien marked this conversation as resolved.
Show resolved Hide resolved

rootSpan.addTags(newTags)
}

function shouldCollectEventHeaders (tags) {
uurien marked this conversation as resolved.
Show resolved Hide resolved
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)
}
Expand Down
151 changes: 139 additions & 12 deletions packages/dd-trace/test/appsec/reporter.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,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'
})
})
Expand Down Expand Up @@ -261,12 +258,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'
})
})
Expand All @@ -279,13 +273,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'
})
})
Expand Down Expand Up @@ -338,6 +329,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()
Expand Down Expand Up @@ -371,7 +389,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',
Expand All @@ -382,7 +400,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'
}
}

Expand All @@ -396,7 +417,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'
})
})

Expand Down Expand Up @@ -457,6 +482,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 = {}
Expand Down
Loading