Skip to content

Commit f034310

Browse files
IlyasShabirochdev
IlyasShabi
authored and
rochdev
committed
report waf results (#5655)
1 parent 77f61e6 commit f034310

File tree

15 files changed

+86
-76
lines changed

15 files changed

+86
-76
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ function onGraphqlStartResolve ({ context, resolverInfo }) {
3838

3939
if (!resolverInfo || typeof resolverInfo !== 'object') return
4040

41-
const actions = waf.run({ ephemeral: { [addresses.HTTP_INCOMING_GRAPHQL_RESOLVER]: resolverInfo } }, req)
42-
const blockingAction = getBlockingAction(actions)
41+
const result = waf.run({ ephemeral: { [addresses.HTTP_INCOMING_GRAPHQL_RESOLVER]: resolverInfo } }, req)
42+
const blockingAction = getBlockingAction(result?.actions)
4343
if (blockingAction) {
4444
const requestData = graphqlRequestData.get(req)
4545
if (requestData?.isInGraphqlRequest) {

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ function onRequestBodyParsed ({ req, res, body, abortController }) {
106106
}
107107
}, req)
108108

109-
handleResults(results, req, res, rootSpan, abortController)
109+
handleResults(results?.actions, req, res, rootSpan, abortController)
110110
}
111111

112112
function onRequestCookieParser ({ req, res, abortController, cookies }) {
@@ -121,7 +121,7 @@ function onRequestCookieParser ({ req, res, abortController, cookies }) {
121121
}
122122
}, req)
123123

124-
handleResults(results, req, res, rootSpan, abortController)
124+
handleResults(results?.actions, req, res, rootSpan, abortController)
125125
}
126126

127127
function incomingHttpStartTranslator ({ req, res, abortController }) {
@@ -149,9 +149,9 @@ function incomingHttpStartTranslator ({ req, res, abortController }) {
149149
persistent[addresses.HTTP_CLIENT_IP] = clientIp
150150
}
151151

152-
const actions = waf.run({ persistent }, req)
152+
const results = waf.run({ persistent }, req)
153153

154-
handleResults(actions, req, res, rootSpan, abortController)
154+
handleResults(results?.actions, req, res, rootSpan, abortController)
155155
}
156156

157157
function incomingHttpEndTranslator ({ req, res }) {
@@ -198,7 +198,7 @@ function onPassportVerify ({ framework, login, user, success, abortController })
198198

199199
const results = UserTracking.trackLogin(framework, login, user, success, rootSpan)
200200

201-
handleResults(results, store.req, store.req.res, rootSpan, abortController)
201+
handleResults(results?.actions, store.req, store.req.res, rootSpan, abortController)
202202
}
203203

204204
function onPassportDeserializeUser ({ user, abortController }) {
@@ -212,7 +212,7 @@ function onPassportDeserializeUser ({ user, abortController }) {
212212

213213
const results = UserTracking.trackUser(user, rootSpan)
214214

215-
handleResults(results, store.req, store.req.res, rootSpan, abortController)
215+
handleResults(results?.actions, store.req, store.req.res, rootSpan, abortController)
216216
}
217217

218218
function onExpressSession ({ req, res, sessionId, abortController }) {
@@ -231,7 +231,7 @@ function onExpressSession ({ req, res, sessionId, abortController }) {
231231
}
232232
}, req)
233233

234-
handleResults(results, req, res, rootSpan, abortController)
234+
handleResults(results?.actions, req, res, rootSpan, abortController)
235235
}
236236

237237
function onRequestQueryParsed ({ req, res, query, abortController }) {
@@ -251,7 +251,7 @@ function onRequestQueryParsed ({ req, res, query, abortController }) {
251251
}
252252
}, req)
253253

254-
handleResults(results, req, res, rootSpan, abortController)
254+
handleResults(results?.actions, req, res, rootSpan, abortController)
255255
}
256256

257257
function onRequestProcessParams ({ req, res, abortController, params }) {
@@ -266,7 +266,7 @@ function onRequestProcessParams ({ req, res, abortController, params }) {
266266
}
267267
}, req)
268268

269-
handleResults(results, req, res, rootSpan, abortController)
269+
handleResults(results?.actions, req, res, rootSpan, abortController)
270270
}
271271

272272
function onResponseBody ({ req, res, body }) {
@@ -308,7 +308,7 @@ function onResponseWriteHead ({ req, res, abortController, statusCode, responseH
308308

309309
responseAnalyzedSet.add(res)
310310

311-
handleResults(results, req, res, rootSpan, abortController)
311+
handleResults(results?.actions, req, res, rootSpan, abortController)
312312
}
313313

314314
function onResponseSetHeader ({ res, abortController }) {

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,12 @@ function blockOnDatadogRaspAbortError ({ error }) {
8585
const abortError = findDatadogRaspAbortError(error)
8686
if (!abortError) return false
8787

88-
const { req, res, blockingAction, raspRule } = abortError
88+
const { req, res, blockingAction, raspRule, ruleTriggered } = abortError
8989
if (!isBlocked(res)) {
9090
const blocked = block(req, res, web.root(req), null, blockingAction)
91-
updateRaspRuleMatchMetricTags(req, raspRule, true, blocked)
91+
if (ruleTriggered) {
92+
updateRaspRuleMatchMetricTags(req, raspRule, true, blocked)
93+
}
9294
}
9395

9496
return true

‎packages/dd-trace/src/appsec/rasp/utils.js‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,23 +20,26 @@ const RULE_TYPES = {
2020
}
2121

2222
class DatadogRaspAbortError extends Error {
23-
constructor (req, res, blockingAction, raspRule) {
23+
constructor (req, res, blockingAction, raspRule, ruleTriggered) {
2424
super('DatadogRaspAbortError')
2525
this.name = 'DatadogRaspAbortError'
2626
this.req = req
2727
this.res = res
2828
this.blockingAction = blockingAction
2929
this.raspRule = raspRule
30+
this.ruleTriggered = ruleTriggered
3031
}
3132
}
3233

33-
function handleResult (actions, req, res, abortController, config, raspRule) {
34-
const generateStackTraceAction = actions?.generate_stack
34+
function handleResult (result, req, res, abortController, config, raspRule) {
35+
const generateStackTraceAction = result?.actions?.generate_stack
3536

3637
const { enabled, maxDepth, maxStackTraces } = config.appsec.stackTrace
3738

3839
const rootSpan = web.root(req)
3940

41+
const ruleTriggered = !!result?.events?.length
42+
4043
if (generateStackTraceAction && enabled && canReportStackTrace(rootSpan, maxStackTraces)) {
4144
const frames = getCallsiteFrames(maxDepth)
4245

@@ -48,11 +51,11 @@ function handleResult (actions, req, res, abortController, config, raspRule) {
4851
}
4952

5053
if (abortController && !abortOnUncaughtException) {
51-
const blockingAction = getBlockingAction(actions)
54+
const blockingAction = getBlockingAction(result?.actions)
5255

5356
// Should block only in express
5457
if (blockingAction && rootSpan?.context()._name === 'express.request') {
55-
const abortError = new DatadogRaspAbortError(req, res, blockingAction, raspRule)
58+
const abortError = new DatadogRaspAbortError(req, res, blockingAction, raspRule, ruleTriggered)
5659
abortController.abort(abortError)
5760

5861
// TODO Delete this when support for node 16 is removed
@@ -64,7 +67,9 @@ function handleResult (actions, req, res, abortController, config, raspRule) {
6467
}
6568
}
6669

67-
updateRaspRuleMatchMetricTags(req, raspRule, false, false)
70+
if (ruleTriggered) {
71+
updateRaspRuleMatchMetricTags(req, raspRule, false, false)
72+
}
6873
}
6974

7075
module.exports = {

‎packages/dd-trace/src/appsec/sdk/user_blocking.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ const { setUserTags } = require('./set_user')
99
const log = require('../../log')
1010

1111
function isUserBlocked (user) {
12-
const actions = waf.run({ persistent: { [USER_ID]: user.id } })
13-
return !!getBlockingAction(actions)
12+
const results = waf.run({ persistent: { [USER_ID]: user.id } })
13+
return !!getBlockingAction(results?.actions)
1414
}
1515

1616
function checkUserAndSetUser (tracer, user) {

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,7 @@ function newStore () {
4141
wafErrorCode: null,
4242
raspErrorCode: null,
4343
wafVersion: null,
44-
rulesVersion: null,
45-
ruleTriggered: null
44+
rulesVersion: null
4645
}
4746
}
4847
}

‎packages/dd-trace/src/appsec/telemetry/rasp.js‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,6 @@ function trackRaspMetrics (store, metrics, raspRule) {
4949
telemetryMetrics.rulesVersion = metrics.rulesVersion
5050
}
5151

52-
if (metrics.ruleTriggered) {
53-
telemetryMetrics.ruleTriggered = true
54-
}
55-
5652
appsecMetrics.count('rasp.rule.eval', tags).inc(1)
5753

5854
if (metrics.errorCode) {
@@ -68,7 +64,6 @@ function trackRaspMetrics (store, metrics, raspRule) {
6864

6965
function trackRaspRuleMatch (store, raspRule, blockTriggered, blocked) {
7066
const telemetryMetrics = store[DD_TELEMETRY_REQUEST_METRICS]
71-
if (!telemetryMetrics.ruleTriggered) return
7267

7368
const tags = {
7469
waf_version: telemetryMetrics.wafVersion,
@@ -82,10 +77,6 @@ function trackRaspRuleMatch (store, raspRule, blockTriggered, blocked) {
8277
}
8378

8479
appsecMetrics.count('rasp.rule.match', tags).inc(1)
85-
86-
// this is needed to not count it twice for the same match
87-
// but it also means it can only be called once per waf call even if there are multiple rasp match
88-
telemetryMetrics.ruleTriggered = null
8980
}
9081

9182
function trackRaspRuleSkipped (raspRule, reason) {

‎packages/dd-trace/src/appsec/waf/waf_context_wrapper.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ class WAFContextWrapper {
1919
this.rulesVersion = rulesVersion
2020
this.knownAddresses = knownAddresses
2121
this.addressesToSkip = new Set()
22-
this.cachedUserIdActions = new Map()
22+
this.cachedUserIdResults = new Map()
2323
}
2424

2525
run ({ persistent, ephemeral }, raspRule) {
@@ -36,9 +36,9 @@ class WAFContextWrapper {
3636
// TODO: make this universal
3737
const userId = persistent?.[addresses.USER_ID] || ephemeral?.[addresses.USER_ID]
3838
if (userId) {
39-
const cachedAction = this.cachedUserIdActions.get(userId)
40-
if (cachedAction) {
41-
return cachedAction
39+
const cachedResults = this.cachedUserIdResults.get(userId)
40+
if (cachedResults) {
41+
return cachedResults
4242
}
4343
}
4444

@@ -142,7 +142,7 @@ class WAFContextWrapper {
142142

143143
Reporter.reportDerivatives(result.derivatives)
144144

145-
return result.actions
145+
return result
146146
} catch (err) {
147147
log.error('[ASM] Error while running the AppSec WAF', err)
148148

@@ -168,7 +168,7 @@ class WAFContextWrapper {
168168
const parameter = match.parameters[k]
169169

170170
if (parameter?.address === addresses.USER_ID) {
171-
this.cachedUserIdActions.set(userId, result.actions)
171+
this.cachedUserIdResults.set(userId, result)
172172
return
173173
}
174174
}

‎packages/dd-trace/test/appsec/graphql.spec.js‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,9 @@ describe('GraphQL', () => {
217217
const abortController = context.abortController
218218

219219
sinon.stub(waf, 'run').returns({
220-
block_request: blockParameters
220+
actions: {
221+
block_request: blockParameters
222+
}
221223
})
222224

223225
sinon.stub(web, 'root').returns(rootSpan)
@@ -247,7 +249,9 @@ describe('GraphQL', () => {
247249
const abortController = context.abortController
248250

249251
sinon.stub(waf, 'run').returns({
250-
block_request: blockParameters
252+
actions: {
253+
block_request: blockParameters
254+
}
251255
})
252256

253257
sinon.stub(web, 'root').returns(rootSpan)

‎packages/dd-trace/test/appsec/index.spec.js‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,12 @@ const telemetryMetrics = require('../../src/telemetry/metrics')
3232
const addresses = require('../../src/appsec/addresses')
3333

3434
const resultActions = {
35-
block_request: {
36-
status_code: '401',
37-
type: 'auto',
38-
grpc_status_code: '10'
35+
actions: {
36+
block_request: {
37+
status_code: '401',
38+
type: 'auto',
39+
grpc_status_code: '10'
40+
}
3941
}
4042
}
4143

0 commit comments

Comments
 (0)