Skip to content

Commit

Permalink
Move action state keeping to waf. Handle new result actions
Browse files Browse the repository at this point in the history
  • Loading branch information
CarlesDD committed May 24, 2024
1 parent f982d4d commit eecc669
Show file tree
Hide file tree
Showing 9 changed files with 93 additions and 137 deletions.
39 changes: 15 additions & 24 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,7 +47,7 @@ function getSpecificBlockingData (type) {
}
}

function getBlockWithContentData (req, specificType, rootSpan) {
function getBlockWithContentData (req, specificType, rootSpan, actionParameters) {
let type
let body
let statusCode
Expand All @@ -64,7 +63,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 +72,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 +82,7 @@ function getBlockWithContentData (req, specificType, rootSpan) {
}
}

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

Check failure on line 85 in packages/dd-trace/src/appsec/blocking.js

View workflow job for this annotation

GitHub Actions / lint

'statusCode' is never reassigned. Use 'const' instead

const headers = {
'Content-Type': type,
Expand All @@ -101,21 +96,22 @@ 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) {
// Is anybody using type argument?
function block (req, res, rootSpan, abortController, type, 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, type, rootSpan, actionParameters)

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

Expand All @@ -142,15 +138,10 @@ function setTemplates (config) {
}
}

function updateBlockingConfiguration (newBlockingConfiguration) {
blockingConfiguration = newBlockingConfiguration
}

module.exports = {
addSpecificEndpoint,
block,
specificBlockingTypes,
getBlockingData,
setTemplates,
updateBlockingConfiguration
setTemplates
}
8 changes: 6 additions & 2 deletions packages/dd-trace/src/appsec/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,12 @@ 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)
if (actions.block_request) {
block(req, res, rootSpan, abortController, null, actions.block_request)
}

if (actions.redirect_request) {
block(req, res, rootSpan, abortController, null, actions.redirect_request)
}
}

Expand Down
21 changes: 10 additions & 11 deletions packages/dd-trace/src/appsec/rule_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,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 @@ -95,6 +91,7 @@ function updateWafFromRC ({ toUnapply, toApply, toModify }) {
}

if (file && file.actions && file.actions.length) {
batchConfiguration = true
newActions.set(id, file.actions)
}

Expand All @@ -112,7 +109,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 +126,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 +148,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 +161,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 +242,6 @@ function copyRulesData (rulesData) {

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

defaultRules = undefined

Expand Down
2 changes: 1 addition & 1 deletion packages/dd-trace/src/appsec/sdk/user_blocking.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ function isUserBlocked (user) {

if (!actions) return false

return actions.includes('block')
return Object.keys(actions).includes('block_request') || Object.keys(actions).includes('redirect_request')
}

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 @@ -60,7 +60,9 @@ class WAFContextWrapper {
this.addressesToSkip = newAddressesToSkip

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

const blockTriggered = result.actions && (Object.keys(result.actions).includes('block_request') ||
Object.keys(result.actions).includes('redirect_request'))

Reporter.reportMetrics({
duration: result.totalRuntime / 1e3,
Expand Down
98 changes: 35 additions & 63 deletions packages/dd-trace/test/appsec/blocking.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,126 +149,98 @@ describe('blocking', () => {
}

it('should block with default html template and custom status', () => {
updateBlockingConfiguration({
id: 'block',
type: 'block_request',
parameters: {
status_code: 401,
type: 'auto'
}
})
const actionParameters = {
status_code: 401,
type: 'auto'
}
req.headers.accept = 'text/html'
setTemplates(config)

block(req, res, rootSpan)
block(req, res, rootSpan, null, null, actionParameters)

expect(res.writeHead).to.have.been.calledOnceWith(401)
expect(res.end).to.have.been.calledOnceWithExactly(defaultBlockedTemplate.html)
})

it('should block with default json template and custom status ' +
'when type is forced to json and accept is html', () => {
updateBlockingConfiguration({
id: 'block',
type: 'block_request',
parameters: {
status_code: 401,
type: 'json'
}
})
const actionParameters = {
status_code: 401,
type: 'json'
}
req.headers.accept = 'text/html'
setTemplates(config)

block(req, res, rootSpan)
block(req, res, rootSpan, null, null, actionParameters)

expect(res.writeHead).to.have.been.calledOnceWith(401)
expect(res.end).to.have.been.calledOnceWithExactly(defaultBlockedTemplate.json)
})

it('should block with default html template and custom status ' +
'when type is forced to html and accept is html', () => {
updateBlockingConfiguration({
id: 'block',
type: 'block_request',
parameters: {
status_code: 401,
type: 'html'
}
})
const actionParameters = {
status_code: 401,
type: 'html'
}
req.headers.accept = 'text/html'
setTemplates(config)

block(req, res, rootSpan)
block(req, res, rootSpan, null, null, actionParameters)

expect(res.writeHead).to.have.been.calledOnceWith(401)
expect(res.end).to.have.been.calledOnceWithExactly(defaultBlockedTemplate.html)
})

it('should block with default json template and custom status', () => {
updateBlockingConfiguration({
id: 'block',
type: 'block_request',
parameters: {
status_code: 401,
type: 'auto'
}
})
const actionParameters = {
status_code: 401,
type: 'auto'
}
setTemplates(config)

block(req, res, rootSpan)
block(req, res, rootSpan, null, null, actionParameters)

expect(res.writeHead).to.have.been.calledOnceWith(401)
expect(res.end).to.have.been.calledOnceWithExactly(defaultBlockedTemplate.json)
})

it('should block with default json template and custom status ' +
'when type is forced to json and accept is not defined', () => {
updateBlockingConfiguration({
id: 'block',
type: 'block_request',
parameters: {
status_code: 401,
type: 'json'
}
})
const actionParameters = {
status_code: 401,
type: 'json'
}
setTemplates(config)

block(req, res, rootSpan)
block(req, res, rootSpan, null, null, actionParameters)

expect(res.writeHead).to.have.been.calledOnceWith(401)
expect(res.end).to.have.been.calledOnceWithExactly(defaultBlockedTemplate.json)
})

it('should block with default html template and custom status ' +
'when type is forced to html and accept is not defined', () => {
updateBlockingConfiguration({
id: 'block',
type: 'block_request',
parameters: {
status_code: 401,
type: 'html'
}
})
const actionParameters = {
status_code: 401,
type: 'html'
}
setTemplates(config)

block(req, res, rootSpan)
block(req, res, rootSpan, null, null, actionParameters)

expect(res.writeHead).to.have.been.calledOnceWith(401)
expect(res.end).to.have.been.calledOnceWithExactly(defaultBlockedTemplate.html)
})

it('should block with custom redirect', () => {
updateBlockingConfiguration({
id: 'block',
type: 'redirect_request',
parameters: {
status_code: 301,
location: '/you-have-been-blocked'
}
})
const actionParameters = {
status_code: 301,
location: '/you-have-been-blocked'
}
setTemplates(config)

block(req, res, rootSpan)
block(req, res, rootSpan, null, null, actionParameters)

expect(res.writeHead).to.have.been.calledOnceWithExactly(301, {
Location: '/you-have-been-blocked'
Expand Down
Loading

0 comments on commit eecc669

Please sign in to comment.