Skip to content

Commit bdbeb02

Browse files
IlyasShabiiunanuasimon-id
authored
add support to api security sampling (#4755)
* add support to api security sampling * fix express plugin schema extraction * use priority simpler to get span priority * use lru cache package * use route path instead of url * use route.path or url to generate the key * use ttlcache * Fix standalone integration test * Increase test timeout * simplify force sample * avoid checking is sampled twice * use route.path or url to generate the key * remove unnecessary tests of sample delay * fix non experimental options test * remove unused isSampled * always sample request if delay is 0 --------- Co-authored-by: Igor Unanua <[email protected]> Co-authored-by: simon-id <[email protected]>
1 parent 1670ef9 commit bdbeb02

19 files changed

Lines changed: 352 additions & 350 deletions

‎LICENSE-3rdparty.csv‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ require,@datadog/pprof,Apache license 2.0,Copyright 2019 Google Inc.
88
require,@datadog/sketches-js,Apache license 2.0,Copyright 2020 Datadog Inc.
99
require,@opentelemetry/api,Apache license 2.0,Copyright OpenTelemetry Authors
1010
require,@opentelemetry/core,Apache license 2.0,Copyright OpenTelemetry Authors
11+
require,@isaacs/ttlcache,ISC,Copyright (c) 2022-2023 - Isaac Z. Schlueter and Contributors
1112
require,crypto-randomuuid,MIT,Copyright 2021 Node.js Foundation and contributors
1213
require,dc-polyfill,MIT,Copyright 2023 Datadog Inc.
1314
require,ignore,MIT,Copyright 2013 Kael Zhang and contributors

‎docs/test.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ tracer.init({
115115
},
116116
apiSecurity: {
117117
enabled: true,
118-
requestSampling: 1.0
119118
},
120119
rasp: {
121120
enabled: true

‎index.d.ts‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -662,19 +662,13 @@ declare namespace tracer {
662662
mode?: 'safe' | 'extended' | 'disabled'
663663
},
664664
/**
665-
* Configuration for Api Security sampling
665+
* Configuration for Api Security
666666
*/
667667
apiSecurity?: {
668668
/** Whether to enable Api Security.
669-
* @default false
669+
* @default true
670670
*/
671671
enabled?: boolean,
672-
673-
/** Controls the request sampling rate (between 0 and 1) in which Api Security is triggered.
674-
* The value will be coerced back if it's outside of the 0-1 range.
675-
* @default 0.1
676-
*/
677-
requestSampling?: number
678672
},
679673
/**
680674
* Configuration for RASP

‎integration-tests/standalone-asm.spec.js‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -81,33 +81,42 @@ describe('Standalone ASM', () => {
8181
})
8282
})
8383

84-
it('should keep second req because RateLimiter allows 1 req/min and discard the next', async () => {
85-
// 1st req kept because waf init
86-
// 2nd req kept because it's the first one hitting RateLimiter
87-
// next in the first minute are dropped
88-
await doWarmupRequests(proc)
89-
90-
return curlAndAssertMessage(agent, proc, ({ headers, payload }) => {
84+
it('should keep fifth req because RateLimiter allows 1 req/min', async () => {
85+
const promise = curlAndAssertMessage(agent, proc, ({ headers, payload }) => {
9186
assert.propertyVal(headers, 'datadog-client-computed-stats', 'yes')
9287
assert.isArray(payload)
93-
assert.strictEqual(payload.length, 4)
94-
95-
const secondReq = payload[1]
96-
assert.isArray(secondReq)
97-
assert.strictEqual(secondReq.length, 5)
88+
if (payload.length === 4) {
89+
assertKeep(payload[0][0])
90+
assertDrop(payload[1][0])
91+
assertDrop(payload[2][0])
92+
assertDrop(payload[3][0])
93+
94+
// req after a minute
95+
} else {
96+
const fifthReq = payload[0]
97+
assert.isArray(fifthReq)
98+
assert.strictEqual(fifthReq.length, 5)
99+
100+
const { meta, metrics } = fifthReq[0]
101+
assert.notProperty(meta, 'manual.keep')
102+
assert.notProperty(meta, '_dd.p.appsec')
103+
104+
assert.propertyVal(metrics, '_sampling_priority_v1', AUTO_KEEP)
105+
assert.propertyVal(metrics, '_dd.apm.enabled', 0)
106+
}
107+
}, 70000, 2)
98108

99-
const { meta, metrics } = secondReq[0]
100-
assert.notProperty(meta, 'manual.keep')
101-
assert.notProperty(meta, '_dd.p.appsec')
109+
// 1st req kept because waf init
110+
// next in the first minute are dropped
111+
// 5nd req kept because RateLimiter allows 1 req/min
112+
await doWarmupRequests(proc)
102113

103-
assert.propertyVal(metrics, '_sampling_priority_v1', AUTO_KEEP)
104-
assert.propertyVal(metrics, '_dd.apm.enabled', 0)
114+
await new Promise(resolve => setTimeout(resolve, 60000))
105115

106-
assertDrop(payload[2][0])
116+
await curl(proc)
107117

108-
assertDrop(payload[3][0])
109-
})
110-
})
118+
return promise
119+
}).timeout(70000)
111120

112121
it('should keep attack requests', async () => {
113122
await doWarmupRequests(proc)

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@
8989
"@datadog/native-metrics": "^3.0.1",
9090
"@datadog/pprof": "5.4.1",
9191
"@datadog/sketches-js": "^2.1.0",
92+
"@isaacs/ttlcache": "^1.4.1",
9293
"@opentelemetry/api": ">=1.0.0 <1.9.0",
9394
"@opentelemetry/core": "^1.14.0",
9495
"crypto-randomuuid": "^1.0.0",

‎packages/datadog-instrumentations/src/express.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ function wrapResponseJson (json) {
2929
obj = arguments[1]
3030
}
3131

32-
responseJsonChannel.publish({ req: this.req, body: obj })
32+
responseJsonChannel.publish({ req: this.req, res: this, body: obj })
3333
}
3434

3535
return json.apply(this, arguments)

‎packages/dd-trace/src/appsec/api_security_sampler.js‎

Lines changed: 50 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,61 +1,84 @@
11
'use strict'
22

3+
const TTLCache = require('@isaacs/ttlcache')
4+
const web = require('../plugins/util/web')
35
const log = require('../log')
6+
const { AUTO_REJECT, USER_REJECT } = require('../../../../ext/priority')
7+
8+
const MAX_SIZE = 4096
49

510
let enabled
6-
let requestSampling
11+
let sampledRequests
712

8-
const sampledRequests = new WeakSet()
13+
class NoopTTLCache {
14+
clear () { }
15+
set (key) { return undefined }
16+
has (key) { return false }
17+
}
918

1019
function configure ({ apiSecurity }) {
1120
enabled = apiSecurity.enabled
12-
setRequestSampling(apiSecurity.requestSampling)
21+
sampledRequests = apiSecurity.sampleDelay === 0
22+
? new NoopTTLCache()
23+
: new TTLCache({ max: MAX_SIZE, ttl: apiSecurity.sampleDelay * 1000 })
1324
}
1425

1526
function disable () {
1627
enabled = false
28+
sampledRequests?.clear()
1729
}
1830

19-
function setRequestSampling (sampling) {
20-
requestSampling = parseRequestSampling(sampling)
21-
}
31+
function sampleRequest (req, res, force = false) {
32+
if (!enabled) return false
2233

23-
function parseRequestSampling (requestSampling) {
24-
let parsed = parseFloat(requestSampling)
34+
const key = computeKey(req, res)
35+
if (!key || isSampled(key)) return false
2536

26-
if (isNaN(parsed)) {
27-
log.warn(`Incorrect API Security request sampling value: ${requestSampling}`)
37+
const rootSpan = web.root(req)
38+
if (!rootSpan) return false
2839

29-
parsed = 0
30-
} else {
31-
parsed = Math.min(1, Math.max(0, parsed))
40+
let priority = getSpanPriority(rootSpan)
41+
if (!priority) {
42+
rootSpan._prioritySampler?.sample(rootSpan)
43+
priority = getSpanPriority(rootSpan)
3244
}
3345

34-
return parsed
35-
}
36-
37-
function sampleRequest (req) {
38-
if (!enabled || !requestSampling) {
46+
if (priority === AUTO_REJECT || priority === USER_REJECT) {
3947
return false
4048
}
4149

42-
const shouldSample = Math.random() <= requestSampling
43-
44-
if (shouldSample) {
45-
sampledRequests.add(req)
50+
if (force) {
51+
sampledRequests.set(key)
4652
}
4753

48-
return shouldSample
54+
return true
55+
}
56+
57+
function isSampled (key) {
58+
return sampledRequests.has(key)
59+
}
60+
61+
function computeKey (req, res) {
62+
const route = web.getContext(req)?.paths?.join('') || ''
63+
const method = req.method
64+
const status = res.statusCode
65+
66+
if (!method || !status) {
67+
log.warn('Unsupported groupkey for API security')
68+
return null
69+
}
70+
return method + route + status
4971
}
5072

51-
function isSampled (req) {
52-
return sampledRequests.has(req)
73+
function getSpanPriority (span) {
74+
const spanContext = span.context?.()
75+
return spanContext._sampling?.priority
5376
}
5477

5578
module.exports = {
5679
configure,
5780
disable,
58-
setRequestSampling,
5981
sampleRequest,
60-
isSampled
82+
isSampled,
83+
computeKey
6184
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,6 @@ function incomingHttpStartTranslator ({ req, res, abortController }) {
145145
persistent[addresses.HTTP_CLIENT_IP] = clientIp
146146
}
147147

148-
if (apiSecuritySampler.sampleRequest(req)) {
149-
persistent[addresses.WAF_CONTEXT_PROCESSOR] = { 'extract-schema': true }
150-
}
151-
152148
const actions = waf.run({ persistent }, req)
153149

154150
handleResults(actions, req, res, rootSpan, abortController)
@@ -172,6 +168,10 @@ function incomingHttpEndTranslator ({ req, res }) {
172168
persistent[addresses.HTTP_INCOMING_QUERY] = req.query
173169
}
174170

171+
if (apiSecuritySampler.sampleRequest(req, res, true)) {
172+
persistent[addresses.WAF_CONTEXT_PROCESSOR] = { 'extract-schema': true }
173+
}
174+
175175
if (Object.keys(persistent).length) {
176176
waf.run({ persistent }, req)
177177
}
@@ -228,9 +228,9 @@ function onRequestProcessParams ({ req, res, abortController, params }) {
228228
handleResults(results, req, res, rootSpan, abortController)
229229
}
230230

231-
function onResponseBody ({ req, body }) {
231+
function onResponseBody ({ req, res, body }) {
232232
if (!body || typeof body !== 'object') return
233-
if (!apiSecuritySampler.isSampled(req)) return
233+
if (!apiSecuritySampler.sampleRequest(req, res)) return
234234

235235
// we don't support blocking at this point, so no results needed
236236
waf.run({

‎packages/dd-trace/src/appsec/remote_config/capabilities.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ module.exports = {
1111
ASM_CUSTOM_RULES: 1n << 8n,
1212
ASM_CUSTOM_BLOCKING_RESPONSE: 1n << 9n,
1313
ASM_TRUSTED_IPS: 1n << 10n,
14-
ASM_API_SECURITY_SAMPLE_RATE: 1n << 11n,
14+
ASM_API_SECURITY_SAMPLE_RATE: 1n << 11n, // deprecated
1515
APM_TRACING_SAMPLE_RATE: 1n << 12n,
1616
APM_TRACING_LOGS_INJECTION: 1n << 13n,
1717
APM_TRACING_HTTP_HEADER_TAGS: 1n << 14n,

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ const Activation = require('../activation')
44

55
const RemoteConfigManager = require('./manager')
66
const RemoteConfigCapabilities = require('./capabilities')
7-
const apiSecuritySampler = require('../api_security_sampler')
87

98
let rc
109

@@ -24,18 +23,12 @@ function enable (config, appsec) {
2423
rc.updateCapabilities(RemoteConfigCapabilities.ASM_ACTIVATION, true)
2524
}
2625

27-
if (config.appsec.apiSecurity?.enabled) {
28-
rc.updateCapabilities(RemoteConfigCapabilities.ASM_API_SECURITY_SAMPLE_RATE, true)
29-
}
30-
3126
rc.setProductHandler('ASM_FEATURES', (action, rcConfig) => {
3227
if (!rcConfig) return
3328

3429
if (activation === Activation.ONECLICK) {
3530
enableOrDisableAppsec(action, rcConfig, config, appsec)
3631
}
37-
38-
apiSecuritySampler.setRequestSampling(rcConfig.api_security?.request_sample_rate)
3932
})
4033
}
4134

0 commit comments

Comments
 (0)