Skip to content

Commit

Permalink
Update native-appsec module to 8.0.1 (#4347)
Browse files Browse the repository at this point in the history
* Bump @datadog/native-appsec to v8.0.1

* Move action state keeping to waf. Handle new result actions

* Removed unused type argument in block function

* Lint

* Handle waf result in graphql

* Check blocking action

* Refactor handle waf result

* Refactor isBlockingAction

* Remove batchConfiguration thing.

* Optional chaining in conditionals

* Remove unrelated change in yarn.lock

* Fix return from waf run stub when no actions in result

* Rephrase test

* Fix waf actions test

* Get blocking action test with empty object instead undefined
  • Loading branch information
CarlesDD authored and juan-fernandez committed Jun 4, 2024
1 parent a3cf6a9 commit 17f8065
Show file tree
Hide file tree
Showing 13 changed files with 165 additions and 172 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
"node": ">=16"
},
"dependencies": {
"@datadog/native-appsec": "7.1.1",
"@datadog/native-appsec": "8.0.1",
"@datadog/native-iast-rewriter": "2.3.1",
"@datadog/native-iast-taint-tracking": "2.1.0",
"@datadog/native-metrics": "^2.0.0",
Expand Down
44 changes: 19 additions & 25 deletions packages/dd-trace/src/appsec/blocking.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ const detectedSpecificEndpoints = {}
let templateHtml = blockedTemplates.html
let templateJson = blockedTemplates.json
let templateGraphqlJson = blockedTemplates.graphqlJson
let blockingConfiguration

const specificBlockingTypes = {
GRAPHQL: 'graphql'
Expand All @@ -22,13 +21,13 @@ function addSpecificEndpoint (method, url, type) {
detectedSpecificEndpoints[getSpecificKey(method, url)] = type
}

function getBlockWithRedirectData (rootSpan) {
let statusCode = blockingConfiguration.parameters.status_code
function getBlockWithRedirectData (rootSpan, actionParameters) {
let statusCode = actionParameters.status_code
if (!statusCode || statusCode < 300 || statusCode >= 400) {
statusCode = 303
}
const headers = {
Location: blockingConfiguration.parameters.location
Location: actionParameters.location
}

rootSpan.addTags({
Expand All @@ -48,10 +47,9 @@ function getSpecificBlockingData (type) {
}
}

function getBlockWithContentData (req, specificType, rootSpan) {
function getBlockWithContentData (req, specificType, rootSpan, actionParameters) {
let type
let body
let statusCode

const specificBlockingType = specificType || detectedSpecificEndpoints[getSpecificKey(req.method, req.url)]
if (specificBlockingType) {
Expand All @@ -64,7 +62,7 @@ function getBlockWithContentData (req, specificType, rootSpan) {
// parse the Accept header, ex: Accept: text/html, application/xhtml+xml, application/xml;q=0.9, */*;q=0.8
const accept = req.headers.accept?.split(',').map((str) => str.split(';', 1)[0].trim())

if (!blockingConfiguration || blockingConfiguration.parameters.type === 'auto') {
if (!actionParameters || actionParameters.type === 'auto') {
if (accept?.includes('text/html') && !accept.includes('application/json')) {
type = 'text/html; charset=utf-8'
body = templateHtml
Expand All @@ -73,7 +71,7 @@ function getBlockWithContentData (req, specificType, rootSpan) {
body = templateJson
}
} else {
if (blockingConfiguration.parameters.type === 'html') {
if (actionParameters.type === 'html') {
type = 'text/html; charset=utf-8'
body = templateHtml
} else {
Expand All @@ -83,11 +81,7 @@ function getBlockWithContentData (req, specificType, rootSpan) {
}
}

if (blockingConfiguration?.type === 'block_request' && blockingConfiguration.parameters.status_code) {
statusCode = blockingConfiguration.parameters.status_code
} else {
statusCode = 403
}
const statusCode = actionParameters?.status_code || 403

const headers = {
'Content-Type': type,
Expand All @@ -101,27 +95,31 @@ function getBlockWithContentData (req, specificType, rootSpan) {
return { body, statusCode, headers }
}

function getBlockingData (req, specificType, rootSpan) {
if (blockingConfiguration?.type === 'redirect_request' && blockingConfiguration.parameters.location) {
return getBlockWithRedirectData(rootSpan)
function getBlockingData (req, specificType, rootSpan, actionParameters) {
if (actionParameters?.location) {
return getBlockWithRedirectData(rootSpan, actionParameters)
} else {
return getBlockWithContentData(req, specificType, rootSpan)
return getBlockWithContentData(req, specificType, rootSpan, actionParameters)
}
}

function block (req, res, rootSpan, abortController, type) {
function block (req, res, rootSpan, abortController, actionParameters) {
if (res.headersSent) {
log.warn('Cannot send blocking response when headers have already been sent')
return
}

const { body, headers, statusCode } = getBlockingData(req, type, rootSpan)
const { body, headers, statusCode } = getBlockingData(req, null, rootSpan, actionParameters)

res.writeHead(statusCode, headers).end(body)

abortController?.abort()
}

function getBlockingAction (actions) {
return actions?.block_request || actions?.redirect_request
}

function setTemplates (config) {
if (config.appsec.blockedTemplateHtml) {
templateHtml = config.appsec.blockedTemplateHtml
Expand All @@ -142,15 +140,11 @@ function setTemplates (config) {
}
}

function updateBlockingConfiguration (newBlockingConfiguration) {
blockingConfiguration = newBlockingConfiguration
}

module.exports = {
addSpecificEndpoint,
block,
specificBlockingTypes,
getBlockingData,
setTemplates,
updateBlockingConfiguration
getBlockingAction,
setTemplates
}
13 changes: 10 additions & 3 deletions packages/dd-trace/src/appsec/graphql.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
'use strict'

const { storage } = require('../../../datadog-core')
const { addSpecificEndpoint, specificBlockingTypes, getBlockingData } = require('./blocking')
const {
addSpecificEndpoint,
specificBlockingTypes,
getBlockingData,
getBlockingAction
} = require('./blocking')
const waf = require('./waf')
const addresses = require('./addresses')
const web = require('../plugins/util/web')
Expand Down Expand Up @@ -32,10 +37,12 @@ function onGraphqlStartResolve ({ context, resolverInfo }) {
if (!resolverInfo || typeof resolverInfo !== 'object') return

const actions = waf.run({ ephemeral: { [addresses.HTTP_INCOMING_GRAPHQL_RESOLVER]: resolverInfo } }, req)
if (actions?.includes('block')) {
const blockingAction = getBlockingAction(actions)
if (blockingAction) {
const requestData = graphqlRequestData.get(req)
if (requestData?.isInGraphqlRequest) {
requestData.blocked = true
requestData.wafAction = blockingAction
context?.abortController?.abort()
}
}
Expand Down Expand Up @@ -87,7 +94,7 @@ function beforeWriteApolloGraphqlResponse ({ abortController, abortData }) {
const rootSpan = web.root(req)
if (!rootSpan) return

const blockingData = getBlockingData(req, specificBlockingTypes.GRAPHQL, rootSpan)
const blockingData = getBlockingData(req, specificBlockingTypes.GRAPHQL, rootSpan, requestData.wafAction)
abortData.statusCode = blockingData.statusCode
abortData.headers = blockingData.headers
abortData.message = blockingData.body
Expand Down
7 changes: 4 additions & 3 deletions packages/dd-trace/src/appsec/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const apiSecuritySampler = require('./api_security_sampler')
const web = require('../plugins/util/web')
const { extractIp } = require('../plugins/util/ip_extractor')
const { HTTP_CLIENT_IP } = require('../../../../ext/tags')
const { block, setTemplates } = require('./blocking')
const { block, setTemplates, getBlockingAction } = require('./blocking')
const { passportTrackEvent } = require('./passport')
const { storage } = require('../../../datadog-core')
const graphql = require('./graphql')
Expand Down Expand Up @@ -223,8 +223,9 @@ function onPassportVerify ({ credentials, user }) {
function handleResults (actions, req, res, rootSpan, abortController) {
if (!actions || !req || !res || !rootSpan || !abortController) return

if (actions.includes('block')) {
block(req, res, rootSpan, abortController)
const blockingAction = getBlockingAction(actions)
if (blockingAction) {
block(req, res, rootSpan, abortController, blockingAction)
}
}

Expand Down
40 changes: 15 additions & 25 deletions packages/dd-trace/src/appsec/rule_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
const fs = require('fs')
const waf = require('./waf')
const { ACKNOWLEDGED, ERROR } = require('./remote_config/apply_states')
const blocking = require('./blocking')

let defaultRules

Expand All @@ -20,10 +19,6 @@ function loadRules (config) {
: require('./recommended.json')

waf.init(defaultRules, config)

if (defaultRules.actions) {
blocking.updateBlockingConfiguration(defaultRules.actions.find(action => action.id === 'block'))
}
}

function updateWafFromRC ({ toUnapply, toApply, toModify }) {
Expand Down Expand Up @@ -68,7 +63,7 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) {
item.apply_state = ERROR
item.apply_error = 'Multiple ruleset received in ASM_DD'
} else {
if (file && file.rules && file.rules.length) {
if (file?.rules?.length) {
const { version, metadata, rules, processors, scanners } = file

newRuleset = { version, metadata, rules, processors, scanners }
Expand All @@ -78,30 +73,23 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) {
batch.add(item)
}
} else if (product === 'ASM') {
let batchConfiguration = false
if (file && file.rules_override && file.rules_override.length) {
batchConfiguration = true
if (file?.rules_override?.length) {
newRulesOverride.set(id, file.rules_override)
}

if (file && file.exclusions && file.exclusions.length) {
batchConfiguration = true
if (file?.exclusions?.length) {
newExclusions.set(id, file.exclusions)
}

if (file && file.custom_rules && file.custom_rules.length) {
batchConfiguration = true
if (file?.custom_rules?.length) {
newCustomRules.set(id, file.custom_rules)
}

if (file && file.actions && file.actions.length) {
if (file?.actions?.length) {
newActions.set(id, file.actions)
}

// "actions" data is managed by tracer and not by waf
if (batchConfiguration) {
batch.add(item)
}
batch.add(item)
}
}

Expand All @@ -112,7 +100,9 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) {
newRuleset ||
newRulesOverride.modified ||
newExclusions.modified ||
newCustomRules.modified) {
newCustomRules.modified ||
newActions.modified
) {
const payload = newRuleset || {}

if (newRulesData.modified) {
Expand All @@ -127,6 +117,9 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) {
if (newCustomRules.modified) {
payload.custom_rules = concatArrays(newCustomRules)
}
if (newActions.modified) {
payload.actions = concatArrays(newActions)
}

try {
waf.update(payload)
Expand All @@ -146,6 +139,9 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) {
if (newCustomRules.modified) {
appliedCustomRules = newCustomRules
}
if (newActions.modified) {
appliedActions = newActions
}
} catch (err) {
newApplyState = ERROR
newApplyError = err.toString()
Expand All @@ -156,11 +152,6 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) {
config.apply_state = newApplyState
if (newApplyError) config.apply_error = newApplyError
}

if (newActions.modified) {
blocking.updateBlockingConfiguration(concatArrays(newActions).find(action => action.id === 'block'))
appliedActions = newActions
}
}

// A Map with a new prop `modified`, a bool that indicates if the Map was modified
Expand Down Expand Up @@ -242,7 +233,6 @@ function copyRulesData (rulesData) {

function clearAllRules () {
waf.destroy()
blocking.updateBlockingConfiguration(undefined)

defaultRules = undefined

Expand Down
7 changes: 2 additions & 5 deletions packages/dd-trace/src/appsec/sdk/user_blocking.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,14 @@
const { USER_ID } = require('../addresses')
const waf = require('../waf')
const { getRootSpan } = require('./utils')
const { block } = require('../blocking')
const { block, getBlockingAction } = require('../blocking')
const { storage } = require('../../../../datadog-core')
const { setUserTags } = require('./set_user')
const log = require('../../log')

function isUserBlocked (user) {
const actions = waf.run({ persistent: { [USER_ID]: user.id } })

if (!actions) return false

return actions.includes('block')
return !!getBlockingAction(actions)
}

function checkUserAndSetUser (tracer, user) {
Expand Down
4 changes: 3 additions & 1 deletion packages/dd-trace/src/appsec/waf/waf_context_wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const log = require('../../log')
const Reporter = require('../reporter')
const addresses = require('../addresses')
const { getBlockingAction } = require('../blocking')

// TODO: remove once ephemeral addresses are implemented
const preventDuplicateAddresses = new Set([
Expand Down Expand Up @@ -60,7 +61,8 @@ class WAFContextWrapper {
this.addressesToSkip = newAddressesToSkip

const ruleTriggered = !!result.events?.length
const blockTriggered = result.actions?.includes('block')

const blockTriggered = !!getBlockingAction(result.actions)

Reporter.reportMetrics({
duration: result.totalRuntime / 1e3,
Expand Down
Loading

0 comments on commit 17f8065

Please sign in to comment.