Skip to content

Commit 3ed46b7

Browse files
CarlesDDsimon-id
andauthored
Extended Request Data Collection (DataDog#5709)
* Headers collection config * Revamp headers collection + extended collection * Fix condition for extended collection * Body collection config * Check for RASP events * Testing extend header collection * RASP body collection tests * Minor reformats * Lint * Clearer condition * Truncate body on body collection * Complete config test * Fix lint * Reporter init w/ appsec config * Remove TODO * Clarify collection comments * Update test.ts with new configuration * Fix linting * Integration test for RASP request body collection * Set tag when reported request body is truncated * Integration test for data collection * Fix linting * Check for header names in integration test * Set the correct value for request body exceeded size tag * Add support for toJSON in request body truncation * Invert condition for early return * Avoid slicing on request body truncation and implement it in a for loop * Rename reporter config * Simplify condition logic * Create block scope in switch case to limit the scope of declared vars * Initialize Array with a fixed size Co-authored-by: simon-id <[email protected]> * Switch to for loop Co-authored-by: simon-id <[email protected]> * Set env var as strings Co-authored-by: simon-id <[email protected]> * Set env var as string Co-authored-by: simon-id <[email protected]> * Refactor headers group declaration + using set instead of arrays * Cache res.getHeaders() * Fix config and its test * Fix config test for defaults values * Check reported request body truncation on integration test * Add a comment to clarify the test case * Add a test case to check no request body is collected when feat is disabled * Manage custom toJSON function in arrays --------- Co-authored-by: simon-id <[email protected]>
1 parent 6329088 commit 3ed46b7

File tree

15 files changed

+1037
-86
lines changed

15 files changed

+1037
-86
lines changed

docs/test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,18 @@ tracer.init({
118118
enabled: true,
119119
},
120120
rasp: {
121-
enabled: true
121+
enabled: true,
122+
bodyCollection: true
122123
},
123124
stackTrace: {
124125
enabled: true,
125126
maxStackTraces: 5,
126127
maxDepth: 42
128+
},
129+
extendedHeadersCollection: {
130+
enabled: true,
131+
redaction: false,
132+
maxHeaders: 42
127133
}
128134
},
129135
iast: {

index.d.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,12 @@ declare namespace tracer {
713713
/** Whether to enable RASP.
714714
* @default false
715715
*/
716-
enabled?: boolean
716+
enabled?: boolean,
717+
718+
/** Whether to enable request body collection on RASP event
719+
* @default false
720+
*/
721+
bodyCollection?: boolean
717722
},
718723
/**
719724
* Configuration for stack trace reporting
@@ -733,6 +738,25 @@ declare namespace tracer {
733738
* @default 32
734739
*/
735740
maxDepth?: number,
741+
},
742+
/**
743+
* Configuration for extended headers collection tied to security events
744+
*/
745+
extendedHeadersCollection?: {
746+
/** Whether to enable extended headers collection
747+
* @default false
748+
*/
749+
enabled: boolean,
750+
751+
/** Whether to redact collected headers
752+
* @default true
753+
*/
754+
redaction: boolean,
755+
756+
/** Specifies the maximum number of headers collected.
757+
* @default 50
758+
*/
759+
maxHeaders: number,
736760
}
737761
}
738762

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
'use strict'
2+
3+
const { assert } = require('chai')
4+
const getPort = require('get-port')
5+
const path = require('path')
6+
const Axios = require('axios')
7+
8+
const {
9+
createSandbox,
10+
FakeAgent,
11+
spawnProc
12+
} = require('../helpers')
13+
14+
describe('ASM Data collection', () => {
15+
let axios, sandbox, cwd, appPort, appFile, agent, proc
16+
17+
before(async () => {
18+
sandbox = await createSandbox(['express'])
19+
appPort = await getPort()
20+
cwd = sandbox.folder
21+
appFile = path.join(cwd, 'appsec/data-collection/index.js')
22+
axios = Axios.create({
23+
baseURL: `http://localhost:${appPort}`
24+
})
25+
})
26+
27+
after(async () => {
28+
await sandbox.remove()
29+
})
30+
31+
function startServer (extendedDataCollection) {
32+
beforeEach(async () => {
33+
agent = await new FakeAgent().start()
34+
35+
const env = {
36+
APP_PORT: appPort,
37+
DD_TRACE_AGENT_PORT: agent.port,
38+
DD_APPSEC_ENABLED: true,
39+
DD_APPSEC_RULES: path.join(cwd, 'appsec', 'data-collection', 'data-collection-rules.json')
40+
}
41+
42+
if (extendedDataCollection) {
43+
env.DD_APPSEC_COLLECT_ALL_HEADERS = true
44+
env.DD_APPSEC_HEADER_COLLECTION_REDACTION_ENABLED = false
45+
env.DD_APPSEC_MAX_COLLECTED_HEADERS = 25
46+
}
47+
48+
proc = await spawnProc(appFile, { cwd, env, execArgv: [] })
49+
})
50+
51+
afterEach(async () => {
52+
proc.kill()
53+
await agent.stop()
54+
})
55+
}
56+
57+
async function assertHeadersReported (requestHeaders, responseHeaders) {
58+
await agent.assertMessageReceived(({ headers, payload }) => {
59+
// Request headers
60+
assert.equal(
61+
Object.keys(payload[0][0].meta).filter(tagName => tagName.startsWith('http.request.headers.')).length,
62+
requestHeaders.length
63+
)
64+
requestHeaders.forEach((headerName) => {
65+
assert.property(payload[0][0].meta, `http.request.headers.${headerName}`)
66+
})
67+
68+
// Response headers
69+
assert.equal(
70+
Object.keys(payload[0][0].meta).filter(tagName => tagName.startsWith('http.response.headers.')).length,
71+
responseHeaders.length
72+
)
73+
responseHeaders.forEach((headerName) => {
74+
assert.property(payload[0][0].meta, `http.response.headers.${headerName}`)
75+
})
76+
})
77+
}
78+
79+
describe('Basic data collection', () => {
80+
startServer(false)
81+
82+
it('should collect event headers', async () => {
83+
const expectedRequestHeaders = [
84+
'user-agent',
85+
'accept',
86+
'host',
87+
'accept-encoding'
88+
]
89+
90+
const expectedResponseHeaders = [
91+
'content-type',
92+
'content-language'
93+
]
94+
95+
await axios.get('/', { headers: { 'User-Agent': 'Arachni/v1' } })
96+
await assertHeadersReported(expectedRequestHeaders, expectedResponseHeaders)
97+
})
98+
})
99+
100+
describe('Extended data collection', () => {
101+
startServer(true)
102+
103+
it('should collect extended headers', async () => {
104+
const expectedRequestHeaders = [
105+
'user-agent',
106+
'accept',
107+
'host',
108+
'accept-encoding',
109+
'connection'
110+
]
111+
112+
// DD_APPSEC_MAX_COLLECTED_HEADERS is set to 25, so it is expected to collect
113+
// 22 x-datadog-res-XX headers + x-powered-by, content-type and content-language, for a total of 25.
114+
const expectedResponseHeaders = [
115+
...Array.from({ length: 22 }, (_, i) =>
116+
`x-datadog-res-${i}`
117+
),
118+
'x-powered-by',
119+
'content-type',
120+
'content-language'
121+
]
122+
123+
await axios.get('/', { headers: { 'User-Agent': 'Arachni/v1' } })
124+
await assertHeadersReported(expectedRequestHeaders, expectedResponseHeaders)
125+
})
126+
})
127+
})
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
{
2+
"version": "2.1",
3+
"metadata": {
4+
"rules_version": "1.2.6"
5+
},
6+
"rules": [
7+
{
8+
"id": "arachni_rule",
9+
"name": "Arachni",
10+
"tags": {
11+
"type": "security_scanner",
12+
"category": "attack_attempt"
13+
},
14+
"conditions": [
15+
{
16+
"parameters": {
17+
"inputs": [
18+
{
19+
"address": "server.request.headers.no_cookies",
20+
"key_path": [
21+
"user-agent"
22+
]
23+
}
24+
],
25+
"regex": "^Arachni\\/v"
26+
},
27+
"operator": "match_regex"
28+
}
29+
]
30+
}
31+
]
32+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
'use strict'
2+
const tracer = require('dd-trace')
3+
tracer.init({
4+
flushInterval: 0
5+
})
6+
7+
const express = require('express')
8+
9+
const app = express()
10+
const port = process.env.APP_PORT || 3000
11+
12+
app.get('/', (req, res) => {
13+
// Content headers
14+
res.set('content-type', 'text/plain')
15+
res.set('content-language', 'en')
16+
17+
// Custom headers
18+
for (let i = 0; i < 25; i++) {
19+
res.set(`x-datadog-res-${i}`, `ext-res-${i}`)
20+
}
21+
22+
res.end('end')
23+
})
24+
25+
app.listen(port, () => {
26+
process.send({ port })
27+
})

integration-tests/appsec/index.spec.js

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const getPort = require('get-port')
44
const path = require('path')
55
const Axios = require('axios')
66
const { assert } = require('chai')
7+
const msgpack = require('@msgpack/msgpack')
78
const { createSandbox, FakeAgent, spawnProc } = require('../helpers')
89

910
describe('RASP', () => {
@@ -27,7 +28,7 @@ describe('RASP', () => {
2728
await sandbox.remove()
2829
})
2930

30-
function startServer (abortOnUncaughtException) {
31+
function startServer (abortOnUncaughtException, collectRequestBody = false) {
3132
beforeEach(async () => {
3233
let execArgv = process.execArgv
3334
if (abortOnUncaughtException) {
@@ -42,7 +43,8 @@ describe('RASP', () => {
4243
APP_PORT: appPort,
4344
DD_APPSEC_ENABLED: true,
4445
DD_APPSEC_RASP_ENABLED: true,
45-
DD_APPSEC_RULES: path.join(cwd, 'appsec/rasp/rasp_rules.json')
46+
DD_APPSEC_RULES: path.join(cwd, 'appsec/rasp/rasp_rules.json'),
47+
DD_APPSEC_RASP_COLLECT_REQUEST_BODY: collectRequestBody
4648
}
4749
}, stdOutputHandler, stdOutputHandler)
4850
})
@@ -60,6 +62,17 @@ describe('RASP', () => {
6062
})
6163
}
6264

65+
async function assertBodyReported (expectedBody, truncated) {
66+
await agent.assertMessageReceived(({ headers, payload }) => {
67+
assert.property(payload[0][0].meta_struct, 'http.request.body')
68+
assert.deepStrictEqual(msgpack.decode(payload[0][0].meta_struct['http.request.body']), expectedBody)
69+
70+
if (truncated) {
71+
assert.property(payload[0][0].meta, '_dd.appsec.rasp.request_body_size.exceeded')
72+
}
73+
})
74+
}
75+
6376
describe('--abort-on-uncaught-exception is not configured', () => {
6477
startServer(false)
6578

@@ -339,4 +352,65 @@ describe('RASP', () => {
339352
})
340353
})
341354
})
355+
356+
describe('extended data collection', () => {
357+
describe('with feature enabled', () => {
358+
startServer(false, true)
359+
360+
it('should report body request', async () => {
361+
const requestBody = { host: 'localhost/ifconfig.pro' }
362+
try {
363+
await axios.post('/ssrf', requestBody)
364+
} catch (e) {
365+
if (!e.response) {
366+
throw e
367+
}
368+
369+
await assertBodyReported(requestBody)
370+
}
371+
})
372+
373+
it('should report truncated body request', async () => {
374+
const requestBody = {
375+
host: 'localhost/ifconfig.pro',
376+
objectWithLotsOfNodes: Object.fromEntries([...Array(300).keys()].map(i => [i, i])),
377+
arr: Array(300).fill('foo')
378+
}
379+
try {
380+
await axios.post('/ssrf', requestBody)
381+
} catch (e) {
382+
if (!e.response) {
383+
throw e
384+
}
385+
386+
const expectedReportedBody = {
387+
host: 'localhost/ifconfig.pro',
388+
objectWithLotsOfNodes: Object.fromEntries([...Array(256).keys()].map(i => [i, i])),
389+
arr: Array(256).fill('foo')
390+
}
391+
392+
await assertBodyReported(expectedReportedBody, true)
393+
}
394+
})
395+
})
396+
397+
describe('with feature disabled', () => {
398+
startServer(false, false)
399+
400+
it('should not report body request', async () => {
401+
const requestBody = { host: 'localhost/ifconfig.pro' }
402+
try {
403+
await axios.post('/ssrf', requestBody)
404+
} catch (e) {
405+
if (!e.response) {
406+
throw e
407+
}
408+
409+
await agent.assertMessageReceived(({ headers, payload }) => {
410+
assert.notProperty(payload[0][0].meta_struct, 'http.request.body')
411+
})
412+
}
413+
})
414+
})
415+
})
342416
})

integration-tests/appsec/rasp/index.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ function httpGetPromise (host) {
4242
})
4343
}
4444

45+
app.use(express.json())
46+
4547
app.get('/crash', () => {
4648
process.nextTick(() => {
4749
throw new Error('Crash')
@@ -205,6 +207,11 @@ app.get('/ssrf/http/unhandled-promise', (req, res) => {
205207
.then(() => res.end('end'))
206208
})
207209

210+
app.post('/ssrf', (req, res) => {
211+
axios.get(`https://${req.body.host}`)
212+
.then(() => res.end('end'))
213+
})
214+
208215
app.listen(port, () => {
209216
process.send({ port })
210217
})

packages/dd-trace/src/appsec/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ function enable (_config) {
5858

5959
remoteConfig.enableWafUpdate(_config.appsec)
6060

61-
Reporter.setRateLimit(_config.appsec.rateLimit)
61+
Reporter.init(_config.appsec)
6262

6363
apiSecuritySampler.configure(_config)
6464

0 commit comments

Comments
 (0)