Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/dd-trace/src/appsec/blocking.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const log = require('../log')
const blockedTemplates = require('./blocked_templates')
const { updateBlockFailureMetric } = require('./telemetry')

const detectedSpecificEndpoints = {}

Expand Down Expand Up @@ -128,6 +129,7 @@ function block (req, res, rootSpan, abortController, actionParameters = defaultB
rootSpan?.setTag('_dd.appsec.block.failed', 1)
log.error('[ASM] Blocking error', err)

updateBlockFailureMetric(req)
return false
}
}
Expand Down
4 changes: 3 additions & 1 deletion packages/dd-trace/src/appsec/graphql.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const {
apolloChannel,
apolloServerCoreChannel
} = require('./channels')
const { updateBlockFailureMetric } = require('./telemetry')

const graphqlRequestData = new WeakMap()

Expand Down Expand Up @@ -106,8 +107,9 @@ function beforeWriteApolloGraphqlResponse ({ abortController, abortData }) {
abortController?.abort()
} catch (err) {
rootSpan.setTag('_dd.appsec.block.failed', 1)

log.error('[ASM] Blocking error', err)

updateBlockFailureMetric(req)
}
}

Expand Down
5 changes: 4 additions & 1 deletion packages/dd-trace/src/appsec/reporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ const {
updateRaspRequestsMetricTags,
incrementWafUpdatesMetric,
incrementWafRequestsMetric,
getRequestMetrics
getRequestMetrics,
updateRateLimitedMetric
} = require('./telemetry')
const zlib = require('zlib')
const { keepTrace } = require('../priority_sampler')
Expand Down Expand Up @@ -149,6 +150,8 @@ function reportAttack (attackData) {

if (limiter.isAllowed()) {
keepTrace(rootSpan, ASM)
} else {
updateRateLimitedMetric(req)
}

// TODO: maybe add this to format.js later (to take decision as late as possible)
Expand Down
9 changes: 6 additions & 3 deletions packages/dd-trace/src/appsec/telemetry/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,15 @@
const DD_TELEMETRY_REQUEST_METRICS = Symbol('_dd.appsec.telemetry.request.metrics')

const tags = {
BLOCK_FAILURE: 'block_failure',
EVENT_RULES_VERSION: 'event_rules_version',
INPUT_TRUNCATED: 'input_truncated',
RATE_LIMITED: 'rate_limited',
REQUEST_BLOCKED: 'request_blocked',
RULE_TRIGGERED: 'rule_triggered',
WAF_ERROR: 'waf_error',
WAF_TIMEOUT: 'waf_timeout',
WAF_VERSION: 'waf_version',
EVENT_RULES_VERSION: 'event_rules_version',
INPUT_TRUNCATED: 'input_truncated'
WAF_VERSION: 'waf_version'
}

function getVersionsTags (wafVersion, rulesVersion) {
Expand Down
16 changes: 16 additions & 0 deletions packages/dd-trace/src/appsec/telemetry/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,20 @@ function updateWafRequestsMetricTags (metrics, req) {
return trackWafMetrics(store, metrics)
}

function updateRateLimitedMetric (req) {
if (!enabled) return

const store = getStore(req)
trackWafMetrics(store, { rateLimited: true })
}

function updateBlockFailureMetric (req) {
if (!enabled) return

const store = getStore(req)
trackWafMetrics(store, { blockFailed: true })
}

function incrementWafInitMetric (wafVersion, rulesVersion, success) {
if (!enabled) return

Expand Down Expand Up @@ -119,6 +133,8 @@ module.exports = {
disable,

updateWafRequestsMetricTags,
updateRateLimitedMetric,
updateBlockFailureMetric,
updateRaspRequestsMetricTags,
incrementWafInitMetric,
incrementWafUpdatesMetric,
Expand Down
24 changes: 19 additions & 5 deletions packages/dd-trace/src/appsec/telemetry/waf.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,28 @@ function trackWafMetrics (store, metrics) {

const metricTags = getOrCreateMetricTags(store, versionsTags)

const { blockTriggered, ruleTriggered, wafTimeout } = metrics
if (metrics.blockFailed) {
metricTags[tags.BLOCK_FAILURE] = true
}

if (blockTriggered) {
if (metrics.blockTriggered) {
metricTags[tags.REQUEST_BLOCKED] = true
}

if (ruleTriggered) {
if (metrics.rateLimited) {
metricTags[tags.RATE_LIMITED] = true
}

if (metrics.ruleTriggered) {
metricTags[tags.RULE_TRIGGERED] = true
}

if (wafTimeout) {
if (metrics.errorCode) {
metricTags[tags.WAF_ERROR] = true
appsecMetrics.count('waf.error', { ...versionsTags, waf_error: metrics.errorCode }).inc()
}

if (metrics.wafTimeout) {
metricTags[tags.WAF_TIMEOUT] = true
}

Expand All @@ -78,10 +89,13 @@ function getOrCreateMetricTags (store, versionsTags) {

if (!metricTags) {
metricTags = {
[tags.BLOCK_FAILURE]: false,
[tags.INPUT_TRUNCATED]: false,
[tags.RATE_LIMITED]: false,
[tags.REQUEST_BLOCKED]: false,
[tags.RULE_TRIGGERED]: false,
[tags.WAF_ERROR]: false,
[tags.WAF_TIMEOUT]: false,
[tags.INPUT_TRUNCATED]: false,

...versionsTags
}
Expand Down
15 changes: 13 additions & 2 deletions packages/dd-trace/test/appsec/blocking.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe('blocking', () => {
}
}

let log
let log, telemetry
let block, setTemplates
let req, res, rootSpan

Expand All @@ -22,9 +22,14 @@ describe('blocking', () => {
warn: sinon.stub()
}

telemetry = {
updateBlockFailureMetric: sinon.stub()
}

const blocking = proxyquire('../src/appsec/blocking', {
'../log': log,
'./blocked_templates': defaultBlockedTemplate
'./blocked_templates': defaultBlockedTemplate,
'./telemetry': telemetry
})

block = blocking.block
Expand Down Expand Up @@ -66,6 +71,7 @@ describe('blocking', () => {
expect(rootSpan.setTag).to.have.been.calledOnceWithExactly('_dd.appsec.block.failed', 1)
expect(res.setHeader).to.not.have.been.called
expect(res.constructor.prototype.end).to.not.have.been.called
expect(telemetry.updateBlockFailureMetric).to.be.calledOnceWithExactly(req)
})

it('should send blocking response with html type if present in the headers', () => {
Expand All @@ -79,6 +85,7 @@ describe('blocking', () => {
'Content-Length': 12
})
expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('htmlBodyéé')
expect(telemetry.updateBlockFailureMetric).to.not.have.been.called
})

it('should send blocking response with json type if present in the headers in priority', () => {
Expand All @@ -92,6 +99,7 @@ describe('blocking', () => {
'Content-Length': 8
})
expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('jsonBody')
expect(telemetry.updateBlockFailureMetric).to.not.have.been.called
})

it('should send blocking response with json type if neither html or json is present in the headers', () => {
Expand All @@ -104,6 +112,7 @@ describe('blocking', () => {
'Content-Length': 8
})
expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('jsonBody')
expect(telemetry.updateBlockFailureMetric).to.not.have.been.called
})

it('should send blocking response and call abortController if passed in arguments', () => {
Expand All @@ -118,6 +127,7 @@ describe('blocking', () => {
})
expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('jsonBody')
expect(abortController.signal.aborted).to.be.true
expect(telemetry.updateBlockFailureMetric).to.not.have.been.called
})

it('should remove all headers before sending blocking response', () => {
Expand All @@ -135,6 +145,7 @@ describe('blocking', () => {
'Content-Length': 8
})
expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('jsonBody')
expect(telemetry.updateBlockFailureMetric).to.not.have.been.called
})
})

Expand Down
12 changes: 9 additions & 3 deletions packages/dd-trace/test/appsec/graphql.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ const {
} = require('../../src/appsec/channels')

describe('GraphQL', () => {
let graphql
let blocking
let graphql, blocking, telemetry

beforeEach(() => {
const getBlockingData = sinon.stub()
Expand All @@ -29,8 +28,13 @@ describe('GraphQL', () => {
statusCode: 403
})

telemetry = {
updateBlockFailureMetric: sinon.stub()
}

graphql = proxyquire('../../src/appsec/graphql', {
'./blocking': blocking
'./blocking': blocking,
'./telemetry': telemetry
})
})

Expand Down Expand Up @@ -234,6 +238,7 @@ describe('GraphQL', () => {
expect(blocking.getBlockingData).to.have.been.calledOnceWithExactly(req, 'graphql', blockParameters)

expect(rootSpan.setTag).to.have.been.calledOnceWithExactly('appsec.blocked', 'true')
expect(telemetry.updateBlockFailureMetric).to.not.have.been.called
})

it('Should catch error when block fails', () => {
Expand Down Expand Up @@ -263,6 +268,7 @@ describe('GraphQL', () => {
expect(blocking.getBlockingData).to.have.been.calledOnceWithExactly(req, 'graphql', blockParameters)

expect(rootSpan.setTag).to.have.been.calledOnceWithExactly('_dd.appsec.block.failed', 1)
expect(telemetry.updateBlockFailureMetric).to.be.calledOnceWithExactly(req)
})
})
})
26 changes: 18 additions & 8 deletions packages/dd-trace/test/appsec/reporter.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ describe('reporter', () => {
updateRaspRequestsMetricTags: sinon.stub(),
incrementWafUpdatesMetric: sinon.stub(),
incrementWafRequestsMetric: sinon.stub(),
updateRateLimitedMetric: sinon.stub(),
getRequestMetrics: sinon.stub()
}

Expand Down Expand Up @@ -296,6 +297,7 @@ describe('reporter', () => {
'_dd.appsec.json': '{"triggers":[{"rule":{},"rule_matches":[{}]}]}'
})
expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM)
expect(telemetry.updateRateLimitedMetric).to.not.have.been.called
})

it('should add tags to request span', () => {
Expand All @@ -310,38 +312,43 @@ describe('reporter', () => {
'network.client.ip': '8.8.8.8'
})
expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM)
expect(telemetry.updateRateLimitedMetric).to.not.have.been.called
})

it('should not add manual.keep when rate limit is reached', (done) => {
const addTags = span.addTags
const params = {}

expect(Reporter.reportAttack('', params)).to.not.be.false
expect(Reporter.reportAttack('', params)).to.not.be.false
expect(Reporter.reportAttack('', params)).to.not.be.false
expect(Reporter.reportAttack('')).to.not.be.false
expect(Reporter.reportAttack('')).to.not.be.false
expect(Reporter.reportAttack('')).to.not.be.false

expect(prioritySampler.setPriority).to.have.callCount(3)
expect(telemetry.updateRateLimitedMetric).to.not.have.been.called

Reporter.setRateLimit(1)

expect(Reporter.reportAttack('', params)).to.not.be.false
expect(Reporter.reportAttack('')).to.not.be.false
expect(addTags.getCall(3).firstArg).to.have.property('appsec.event').that.equals('true')
expect(prioritySampler.setPriority).to.have.callCount(4)
expect(Reporter.reportAttack('', params)).to.not.be.false
expect(telemetry.updateRateLimitedMetric).to.not.have.been.called

expect(Reporter.reportAttack('')).to.not.be.false
expect(addTags.getCall(4).firstArg).to.have.property('appsec.event').that.equals('true')
expect(prioritySampler.setPriority).to.have.callCount(4)
expect(telemetry.updateRateLimitedMetric).to.be.calledOnceWithExactly(req)

setTimeout(() => {
expect(Reporter.reportAttack('', params)).to.not.be.false
expect(Reporter.reportAttack('')).to.not.be.false
expect(prioritySampler.setPriority).to.have.callCount(5)
expect(telemetry.updateRateLimitedMetric).to.be.calledOnceWithExactly(req)
done()
}, 1020)
})

it('should not overwrite origin tag', () => {
span.context()._tags = { '_dd.origin': 'tracer' }

const result = Reporter.reportAttack('[]', {})
const result = Reporter.reportAttack('[]')
expect(result).to.not.be.false
expect(web.root).to.have.been.calledOnceWith(req)

Expand All @@ -351,6 +358,7 @@ describe('reporter', () => {
'network.client.ip': '8.8.8.8'
})
expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM)
expect(telemetry.updateRateLimitedMetric).to.not.have.been.called
})

it('should merge attacks json', () => {
Expand All @@ -367,6 +375,7 @@ describe('reporter', () => {
'network.client.ip': '8.8.8.8'
})
expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM)
expect(telemetry.updateRateLimitedMetric).to.not.have.been.called
})

it('should call standalone sample', () => {
Expand All @@ -384,6 +393,7 @@ describe('reporter', () => {
})

expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, ASM)
expect(telemetry.updateRateLimitedMetric).to.not.have.been.called
})
})

Expand Down
7 changes: 5 additions & 2 deletions packages/dd-trace/test/appsec/telemetry/rasp.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,15 @@ describe('Appsec Rasp Telemetry metrics', () => {
appsecTelemetry.incrementWafRequestsMetric(req)

expect(count).to.have.been.calledWithExactly('waf.requests', {
block_failure: false,
input_truncated: false,
request_blocked: false,
rate_limited: false,
rule_triggered: false,
waf_error: false,
waf_timeout: false,
waf_version: wafVersion,
event_rules_version: rulesVersion,
input_truncated: false
event_rules_version: rulesVersion
})
})
})
Expand Down
Loading
Loading