From 6987a66be76ea30e7dc53d4fa608c83e1c2d9d66 Mon Sep 17 00:00:00 2001 From: ishabi Date: Thu, 3 Oct 2024 09:33:45 +0200 Subject: [PATCH] add support to api security sampling --- docs/test.ts | 1 - index.d.ts | 7 +- .../src/appsec/api_security_sampler.js | 54 +++----- .../src/appsec/api_security_sampler_cache.js | 53 +++++++ packages/dd-trace/src/appsec/index.js | 9 +- .../src/appsec/remote_config/capabilities.js | 1 - .../src/appsec/remote_config/index.js | 7 - packages/dd-trace/src/config.js | 7 +- .../test/appsec/api_security_sampler.spec.js | 92 +++++++------ .../appsec/api_security_sampler_cache.spec.js | 65 +++++++++ .../appsec/index.sequelize.plugin.spec.js | 2 +- packages/dd-trace/test/appsec/index.spec.js | 116 +++++++--------- .../test/appsec/remote_config/index.spec.js | 129 ------------------ .../test/appsec/response_blocking.spec.js | 5 +- packages/dd-trace/test/config.spec.js | 50 ++----- 15 files changed, 257 insertions(+), 341 deletions(-) create mode 100644 packages/dd-trace/src/appsec/api_security_sampler_cache.js create mode 100644 packages/dd-trace/test/appsec/api_security_sampler_cache.spec.js diff --git a/docs/test.ts b/docs/test.ts index e37177e0898..7b4367d504c 100644 --- a/docs/test.ts +++ b/docs/test.ts @@ -115,7 +115,6 @@ tracer.init({ }, apiSecurity: { enabled: true, - requestSampling: 1.0 }, rasp: { enabled: true diff --git a/index.d.ts b/index.d.ts index 02c84fb47d3..826ae831fd5 100644 --- a/index.d.ts +++ b/index.d.ts @@ -654,7 +654,7 @@ declare namespace tracer { mode?: 'safe' | 'extended' | 'disabled' }, /** - * Configuration for Api Security sampling + * Configuration for Api Security */ apiSecurity?: { /** Whether to enable Api Security. @@ -662,11 +662,6 @@ declare namespace tracer { */ enabled?: boolean, - /** Controls the request sampling rate (between 0 and 1) in which Api Security is triggered. - * The value will be coerced back if it's outside of the 0-1 range. - * @default 0.1 - */ - requestSampling?: number }, /** * Configuration for RASP diff --git a/packages/dd-trace/src/appsec/api_security_sampler.js b/packages/dd-trace/src/appsec/api_security_sampler.js index 68bd896af7e..6baaed4f87e 100644 --- a/packages/dd-trace/src/appsec/api_security_sampler.js +++ b/packages/dd-trace/src/appsec/api_security_sampler.js @@ -1,61 +1,51 @@ 'use strict' -const log = require('../log') +const ApiSecuritySamplerCache = require('./api_security_sampler_cache') +const web = require('../plugins/util/web') +const { USER_KEEP, AUTO_KEEP } = require('../../../../ext/priority') let enabled -let requestSampling - -const sampledRequests = new WeakSet() +let sampledRequests function configure ({ apiSecurity }) { enabled = apiSecurity.enabled - setRequestSampling(apiSecurity.requestSampling) + sampledRequests = new ApiSecuritySamplerCache(apiSecurity.sampleDelay) } function disable () { enabled = false + sampledRequests?.clear() } -function setRequestSampling (sampling) { - requestSampling = parseRequestSampling(sampling) -} - -function parseRequestSampling (requestSampling) { - let parsed = parseFloat(requestSampling) - - if (isNaN(parsed)) { - log.warn(`Incorrect API Security request sampling value: ${requestSampling}`) +function sampleRequest (req, res) { + if (!enabled) return false - parsed = 0 - } else { - parsed = Math.min(1, Math.max(0, parsed)) - } + const rootSpan = web.root(req) + if (!rootSpan) return false - return parsed -} + const priority = getSpanPriority(rootSpan) -function sampleRequest (req) { - if (!enabled || !requestSampling) { + if (priority !== AUTO_KEEP && priority !== USER_KEEP) { return false } - const shouldSample = Math.random() <= requestSampling + const key = sampledRequests.computeKey(req, res) + const isSampled = sampledRequests.isSampled(key) - if (shouldSample) { - sampledRequests.add(req) - } + if (isSampled) return false + + sampledRequests.set(key) - return shouldSample + return true } -function isSampled (req) { - return sampledRequests.has(req) +function getSpanPriority (span) { + const spanContext = span.context?.() + return spanContext._sampling?.priority // default ?? } module.exports = { configure, disable, - setRequestSampling, - sampleRequest, - isSampled + sampleRequest } diff --git a/packages/dd-trace/src/appsec/api_security_sampler_cache.js b/packages/dd-trace/src/appsec/api_security_sampler_cache.js new file mode 100644 index 00000000000..4c328c43428 --- /dev/null +++ b/packages/dd-trace/src/appsec/api_security_sampler_cache.js @@ -0,0 +1,53 @@ +'use strict' + +const crypto = require('node:crypto') +const log = require('../log') + +const MAX_SIZE = 4096 +const DEFAULT_DELAY = 30 // 30s + +class ApiSecuritySamplerCache extends Map { + constructor (delay) { + super() + this.delay = this._parseSampleDelay(delay) + } + + _parseSampleDelay (delay) { + if (typeof delay === 'number' && Number.isFinite(delay) && delay > 0) { + return delay + } else { + log.warn('Invalid delay value. Delay must be a positive number.') + return DEFAULT_DELAY + } + } + + computeKey (req, res) { + const route = req.url + const method = req.method.toLowerCase() + const statusCode = res.statusCode + const str = route + statusCode + method + return crypto.createHash('md5').update(str).digest('hex') + } + + isSampled (key) { + if (!super.has(key)) { + return false + } + const previous = super.get(key) + return Date.now() - previous < (this.delay * 1000) + } + + set (key) { + if (super.has(key)) { + super.delete(key) + } + + super.set(key, Date.now()) + if (super.size > MAX_SIZE) { + const oldestKey = super.keys().next().value + super.delete(oldestKey) + } + } +} + +module.exports = ApiSecuritySamplerCache diff --git a/packages/dd-trace/src/appsec/index.js b/packages/dd-trace/src/appsec/index.js index 10e63ebd2de..56b645dbcbc 100644 --- a/packages/dd-trace/src/appsec/index.js +++ b/packages/dd-trace/src/appsec/index.js @@ -104,10 +104,6 @@ function incomingHttpStartTranslator ({ req, res, abortController }) { persistent[addresses.HTTP_CLIENT_IP] = clientIp } - if (apiSecuritySampler.sampleRequest(req)) { - persistent[addresses.WAF_CONTEXT_PROCESSOR] = { 'extract-schema': true } - } - const actions = waf.run({ persistent }, req) handleResults(actions, req, res, rootSpan, abortController) @@ -136,6 +132,10 @@ function incomingHttpEndTranslator ({ req, res }) { persistent[addresses.HTTP_INCOMING_QUERY] = req.query } + if (apiSecuritySampler.sampleRequest(req, res)) { + persistent[addresses.WAF_CONTEXT_PROCESSOR] = { 'extract-schema': true } + } + if (Object.keys(persistent).length) { waf.run({ persistent }, req) } @@ -202,7 +202,6 @@ function onRequestCookieParser ({ req, res, abortController, cookies }) { function onResponseBody ({ req, body }) { if (!body || typeof body !== 'object') return - if (!apiSecuritySampler.isSampled(req)) return // we don't support blocking at this point, so no results needed waf.run({ diff --git a/packages/dd-trace/src/appsec/remote_config/capabilities.js b/packages/dd-trace/src/appsec/remote_config/capabilities.js index f42d7358203..4c2fb8f8ff8 100644 --- a/packages/dd-trace/src/appsec/remote_config/capabilities.js +++ b/packages/dd-trace/src/appsec/remote_config/capabilities.js @@ -11,7 +11,6 @@ module.exports = { ASM_CUSTOM_RULES: 1n << 8n, ASM_CUSTOM_BLOCKING_RESPONSE: 1n << 9n, ASM_TRUSTED_IPS: 1n << 10n, - ASM_API_SECURITY_SAMPLE_RATE: 1n << 11n, APM_TRACING_SAMPLE_RATE: 1n << 12n, APM_TRACING_LOGS_INJECTION: 1n << 13n, APM_TRACING_HTTP_HEADER_TAGS: 1n << 14n, diff --git a/packages/dd-trace/src/appsec/remote_config/index.js b/packages/dd-trace/src/appsec/remote_config/index.js index b63b3690102..64007da69df 100644 --- a/packages/dd-trace/src/appsec/remote_config/index.js +++ b/packages/dd-trace/src/appsec/remote_config/index.js @@ -4,7 +4,6 @@ const Activation = require('../activation') const RemoteConfigManager = require('./manager') const RemoteConfigCapabilities = require('./capabilities') -const apiSecuritySampler = require('../api_security_sampler') let rc @@ -24,18 +23,12 @@ function enable (config, appsec) { rc.updateCapabilities(RemoteConfigCapabilities.ASM_ACTIVATION, true) } - if (config.appsec.apiSecurity?.enabled) { - rc.updateCapabilities(RemoteConfigCapabilities.ASM_API_SECURITY_SAMPLE_RATE, true) - } - rc.setProductHandler('ASM_FEATURES', (action, rcConfig) => { if (!rcConfig) return if (activation === Activation.ONECLICK) { enableOrDisableAppsec(action, rcConfig, config, appsec) } - - apiSecuritySampler.setRequestSampling(rcConfig.api_security?.request_sample_rate) }) } diff --git a/packages/dd-trace/src/config.js b/packages/dd-trace/src/config.js index dc5bb524d1a..05e69568cb6 100644 --- a/packages/dd-trace/src/config.js +++ b/packages/dd-trace/src/config.js @@ -444,7 +444,7 @@ class Config { const defaults = setHiddenProperty(this, '_defaults', {}) this._setValue(defaults, 'appsec.apiSecurity.enabled', true) - this._setValue(defaults, 'appsec.apiSecurity.requestSampling', 0.1) + this._setValue(defaults, 'appsec.apiSecurity.sampleDelay', 30) this._setValue(defaults, 'appsec.blockedTemplateGraphql', undefined) this._setValue(defaults, 'appsec.blockedTemplateHtml', undefined) this._setValue(defaults, 'appsec.blockedTemplateJson', undefined) @@ -555,7 +555,7 @@ class Config { AWS_LAMBDA_FUNCTION_NAME, DD_AGENT_HOST, DD_API_SECURITY_ENABLED, - DD_API_SECURITY_REQUEST_SAMPLE_RATE, + DD_API_SECURITY_SAMPLE_DELAY, DD_APPSEC_AUTOMATED_USER_EVENTS_TRACKING, DD_APPSEC_ENABLED, DD_APPSEC_GRAPHQL_BLOCKED_TEMPLATE_JSON, @@ -671,7 +671,7 @@ class Config { DD_API_SECURITY_ENABLED && isTrue(DD_API_SECURITY_ENABLED), DD_EXPERIMENTAL_API_SECURITY_ENABLED && isTrue(DD_EXPERIMENTAL_API_SECURITY_ENABLED) )) - this._setUnit(env, 'appsec.apiSecurity.requestSampling', DD_API_SECURITY_REQUEST_SAMPLE_RATE) + this._setValue(env, 'appsec.apiSecurity.sampleDelay', maybeFloat(DD_API_SECURITY_SAMPLE_DELAY)) this._setValue(env, 'appsec.blockedTemplateGraphql', maybeFile(DD_APPSEC_GRAPHQL_BLOCKED_TEMPLATE_JSON)) this._setValue(env, 'appsec.blockedTemplateHtml', maybeFile(DD_APPSEC_HTTP_BLOCKED_TEMPLATE_HTML)) this._envUnprocessed['appsec.blockedTemplateHtml'] = DD_APPSEC_HTTP_BLOCKED_TEMPLATE_HTML @@ -838,7 +838,6 @@ class Config { tagger.add(tags, options.tags) this._setBoolean(opts, 'appsec.apiSecurity.enabled', options.appsec.apiSecurity?.enabled) - this._setUnit(opts, 'appsec.apiSecurity.requestSampling', options.appsec.apiSecurity?.requestSampling) this._setValue(opts, 'appsec.blockedTemplateGraphql', maybeFile(options.appsec.blockedTemplateGraphql)) this._setValue(opts, 'appsec.blockedTemplateHtml', maybeFile(options.appsec.blockedTemplateHtml)) this._optsUnprocessed['appsec.blockedTemplateHtml'] = options.appsec.blockedTemplateHtml diff --git a/packages/dd-trace/test/appsec/api_security_sampler.spec.js b/packages/dd-trace/test/appsec/api_security_sampler.spec.js index 5a69af05a5c..08b4355084a 100644 --- a/packages/dd-trace/test/appsec/api_security_sampler.spec.js +++ b/packages/dd-trace/test/appsec/api_security_sampler.spec.js @@ -1,71 +1,75 @@ 'use strict' -const apiSecuritySampler = require('../../src/appsec/api_security_sampler') +const proxyquire = require('proxyquire') -describe('Api Security Sampler', () => { - let config +describe('API Security Sampler', () => { + let apiSecuritySampler, webStub, clock beforeEach(() => { - config = { - apiSecurity: { - enabled: true, - requestSampling: 1 - } - } - - sinon.stub(Math, 'random').returns(0.3) + webStub = { root: sinon.stub() } + clock = sinon.useFakeTimers(Date.now()) + + apiSecuritySampler = proxyquire('../../src/appsec/api_security_sampler', { + '../plugins/util/web': webStub + }) }) - afterEach(sinon.restore) + afterEach(() => { + clock.restore() + }) describe('sampleRequest', () => { - it('should sample request if enabled and sampling 1', () => { - apiSecuritySampler.configure(config) - - expect(apiSecuritySampler.sampleRequest({})).to.true + beforeEach(() => { + apiSecuritySampler.configure({ apiSecurity: { enabled: true, sampleDelay: 30 } }) }) - it('should not sample request if enabled and sampling 0', () => { - config.apiSecurity.requestSampling = 0 - apiSecuritySampler.configure(config) - - expect(apiSecuritySampler.sampleRequest({})).to.false + it('should return false if not enabled', () => { + apiSecuritySampler.disable() + expect(apiSecuritySampler.sampleRequest({}, {})).to.be.false }) - it('should sample request if enabled and sampling greater than random', () => { - config.apiSecurity.requestSampling = 0.5 - - apiSecuritySampler.configure(config) - - expect(apiSecuritySampler.sampleRequest({})).to.true + it('should return false if no root span', () => { + webStub.root.returns(null) + expect(apiSecuritySampler.sampleRequest({}, {})).to.be.false }) - it('should not sample request if enabled and sampling less than random', () => { - config.apiSecurity.requestSampling = 0.1 - - apiSecuritySampler.configure(config) - - expect(apiSecuritySampler.sampleRequest()).to.false + it('should return true and put request in cache if priority is AUTO_KEEP', () => { + const rootSpan = { context: () => ({ _sampling: { priority: 2 } }) } + webStub.root.returns(rootSpan) + const req = { url: '/test', method: 'GET' } + const res = { statusCode: 200 } + expect(apiSecuritySampler.sampleRequest(req, res)).to.be.true }) - it('should not sample request if incorrect config value', () => { - config.apiSecurity.requestSampling = NaN - - apiSecuritySampler.configure(config) + it('should return true and put request in cache if priority is USER_KEEP', () => { + const rootSpan = { context: () => ({ _sampling: { priority: 1 } }) } + webStub.root.returns(rootSpan) + const req = { url: '/test', method: 'GET' } + const res = { statusCode: 200 } + expect(apiSecuritySampler.sampleRequest(req, res)).to.be.true + }) - expect(apiSecuritySampler.sampleRequest()).to.false + it('should return false if priority is neither AUTO_KEEP nor USER_KEEP', () => { + const rootSpan = { context: () => ({ _sampling: { priority: 0 } }) } + webStub.root.returns(rootSpan) + expect(apiSecuritySampler.sampleRequest({}, {})).to.be.false }) + }) - it('should sample request according to the config', () => { - config.apiSecurity.requestSampling = 1 + describe('disable', () => { + it('should set enabled to false and clear the cache', () => { + const req = { url: '/test', method: 'GET' } + const res = { statusCode: 200 } - apiSecuritySampler.configure(config) + const rootSpan = { context: () => ({ _sampling: { priority: 2 } }) } + webStub.root.returns(rootSpan) - expect(apiSecuritySampler.sampleRequest({})).to.true + apiSecuritySampler.configure({ apiSecurity: { enabled: true, sampleDelay: 30 } }) + expect(apiSecuritySampler.sampleRequest(req, res)).to.be.true - apiSecuritySampler.setRequestSampling(0) + apiSecuritySampler.disable() - expect(apiSecuritySampler.sampleRequest()).to.false + expect(apiSecuritySampler.sampleRequest({}, {})).to.be.false }) }) }) diff --git a/packages/dd-trace/test/appsec/api_security_sampler_cache.spec.js b/packages/dd-trace/test/appsec/api_security_sampler_cache.spec.js new file mode 100644 index 00000000000..868705cd458 --- /dev/null +++ b/packages/dd-trace/test/appsec/api_security_sampler_cache.spec.js @@ -0,0 +1,65 @@ +'use strict' + +const ApiSecuritySamplerCache = require('../../src/appsec/api_security_sampler_cache') + +describe('ApiSecuritySamplerCache', () => { + let cache + let clock + + beforeEach(() => { + cache = new ApiSecuritySamplerCache() + clock = sinon.useFakeTimers(Date.now()) + }) + + afterEach(() => { + clock.restore() + }) + + describe('Standard behavior with default delay', () => { + it('should not sample when first seen', () => { + const req = { url: '/test', method: 'GET' } + const res = { status_code: 200 } + const key = cache.computeKey(req, res) + expect(cache.isSampled(key)).to.be.false + }) + + it('should sample within 30 seconds of first seen', () => { + const req = { url: '/test', method: 'GET' } + const res = { status_code: 200 } + const key = cache.computeKey(req, res) + cache.set(key) + clock.tick(29000) + expect(cache.isSampled(key)).to.be.true + }) + + it('should not sample after 30 seconds', () => { + const req = { url: '/test', method: 'GET' } + const res = { status_code: 200 } + const key = cache.computeKey(req, res) + cache.set(key) + clock.tick(31000) + expect(cache.isSampled(key)).to.be.false + }) + }) + + describe('Max size behavior', () => { + it('should remove oldest entry when max size is exceeded', () => { + const baseReq = { method: 'GET' } + const baseRes = { status_code: 200 } + + for (let i = 0; i < 4097; i++) { + const req = { ...baseReq, url: `test${i}` } + const key = cache.computeKey(req, baseRes) + cache.set(key) + } + + expect(cache.size).to.equal(4096) + + const firstKey = cache.computeKey({ ...baseReq, url: 'test0' }, baseRes) + expect(cache.isSampled(firstKey)).to.be.false + + const lastKey = cache.computeKey({ ...baseReq, url: 'test4096' }, baseRes) + expect(cache.isSampled(lastKey)).to.be.true + }) + }) +}) diff --git a/packages/dd-trace/test/appsec/index.sequelize.plugin.spec.js b/packages/dd-trace/test/appsec/index.sequelize.plugin.spec.js index 07013a570d2..f844666716a 100644 --- a/packages/dd-trace/test/appsec/index.sequelize.plugin.spec.js +++ b/packages/dd-trace/test/appsec/index.sequelize.plugin.spec.js @@ -21,7 +21,7 @@ describe('sequelize', () => { rules: path.join(__dirname, 'express-rules.json'), apiSecurity: { enabled: true, - requestSampling: 1 + sampleDelay: 10 } } })) diff --git a/packages/dd-trace/test/appsec/index.spec.js b/packages/dd-trace/test/appsec/index.spec.js index b8a41d840b5..c83852750fc 100644 --- a/packages/dd-trace/test/appsec/index.spec.js +++ b/packages/dd-trace/test/appsec/index.spec.js @@ -66,7 +66,7 @@ describe('AppSec Index', function () { }, apiSecurity: { enabled: false, - requestSampling: 0 + sampleDelay: 10 }, rasp: { enabled: true @@ -102,9 +102,10 @@ describe('AppSec Index', function () { disable: sinon.stub() } - apiSecuritySampler = require('../../src/appsec/api_security_sampler') + apiSecuritySampler = proxyquire('../../src/appsec/api_security_sampler', { + '../plugins/util/web': web + }) sinon.spy(apiSecuritySampler, 'sampleRequest') - sinon.spy(apiSecuritySampler, 'isSampled') rasp = { enable: sinon.stub(), @@ -466,45 +467,10 @@ describe('AppSec Index', function () { web.root.returns(rootSpan) }) - it('should not trigger schema extraction with sampling disabled', () => { - config.appsec.apiSecurity = { - enabled: true, - requestSampling: 0 - } - - AppSec.enable(config) - - const req = { - url: '/path', - headers: { - 'user-agent': 'Arachni', - host: 'localhost', - cookie: 'a=1;b=2' - }, - method: 'POST', - socket: { - remoteAddress: '127.0.0.1', - remotePort: 8080 - } - } - const res = {} - - AppSec.incomingHttpStartTranslator({ req, res }) - - expect(waf.run).to.have.been.calledOnceWithExactly({ - persistent: { - 'server.request.uri.raw': '/path', - 'server.request.headers.no_cookies': { 'user-agent': 'Arachni', host: 'localhost' }, - 'server.request.method': 'POST', - 'http.client_ip': '127.0.0.1' - } - }, req) - }) - it('should not trigger schema extraction with feature disabled', () => { config.appsec.apiSecurity = { enabled: false, - requestSampling: 1 + sampleDelay: 1 } AppSec.enable(config) @@ -520,18 +486,34 @@ describe('AppSec Index', function () { socket: { remoteAddress: '127.0.0.1', remotePort: 8080 + }, + body: { + a: '1' + }, + query: { + b: '2' + }, + route: { + path: '/path/:c' } } - const res = {} + const res = { + getHeaders: () => ({ + 'content-type': 'application/json', + 'content-lenght': 42 + }), + statusCode: 201 + } - AppSec.incomingHttpStartTranslator({ req, res }) + web.patch(req) + + sinon.stub(Reporter, 'finishRequest') + AppSec.incomingHttpEndTranslator({ req, res }) expect(waf.run).to.have.been.calledOnceWithExactly({ persistent: { - 'server.request.uri.raw': '/path', - 'server.request.headers.no_cookies': { 'user-agent': 'Arachni', host: 'localhost' }, - 'server.request.method': 'POST', - 'http.client_ip': '127.0.0.1' + 'server.request.body': { a: '1' }, + 'server.request.query': { b: '2' } } }, req) }) @@ -539,7 +521,7 @@ describe('AppSec Index', function () { it('should trigger schema extraction with sampling enabled', () => { config.appsec.apiSecurity = { enabled: true, - requestSampling: 1 + sampleDelay: 1 } AppSec.enable(config) @@ -548,25 +530,34 @@ describe('AppSec Index', function () { url: '/path', headers: { 'user-agent': 'Arachni', - host: 'localhost', - cookie: 'a=1;b=2' + host: 'localhost' }, method: 'POST', socket: { remoteAddress: '127.0.0.1', remotePort: 8080 + }, + body: { + a: '1' } } - const res = {} + const res = { + getHeaders: () => ({ + 'content-type': 'application/json', + 'content-lenght': 42 + }), + statusCode: 201 + } - AppSec.incomingHttpStartTranslator({ req, res }) + const rootSpan = { context: () => ({ _sampling: { priority: 2 } }) } + + web.root.returns(rootSpan) + + AppSec.incomingHttpEndTranslator({ req, res }) expect(waf.run).to.have.been.calledOnceWithExactly({ persistent: { - 'server.request.uri.raw': '/path', - 'server.request.headers.no_cookies': { 'user-agent': 'Arachni', host: 'localhost' }, - 'server.request.method': 'POST', - 'http.client_ip': '127.0.0.1', + 'server.request.body': { a: '1' }, 'waf.context.processor': { 'extract-schema': true } } }, req) @@ -576,7 +567,7 @@ describe('AppSec Index', function () { beforeEach(() => { config.appsec.apiSecurity = { enabled: true, - requestSampling: 1 + sampleDelay: 1 } AppSec.enable(config) }) @@ -589,28 +580,15 @@ describe('AppSec Index', function () { responseBody.publish({ req: {}, body: 'string' }) responseBody.publish({ req: {}, body: null }) - expect(apiSecuritySampler.isSampled).to.not.been.called - expect(waf.run).to.not.been.called - }) - - it('should not call to the waf if it is not a sampled request', () => { - apiSecuritySampler.isSampled = apiSecuritySampler.isSampled.instantiateFake(() => false) - const req = {} - - responseBody.publish({ req, body: {} }) - - expect(apiSecuritySampler.isSampled).to.have.been.calledOnceWith(req) expect(waf.run).to.not.been.called }) - it('should call to the waf if it is a sampled request', () => { - apiSecuritySampler.isSampled = apiSecuritySampler.isSampled.instantiateFake(() => true) + it('should call to the waf if body is an object', () => { const req = {} const body = {} responseBody.publish({ req, body }) - expect(apiSecuritySampler.isSampled).to.have.been.calledOnceWith(req) expect(waf.run).to.been.calledOnceWith({ persistent: { [addresses.HTTP_INCOMING_RESPONSE_BODY]: body diff --git a/packages/dd-trace/test/appsec/remote_config/index.spec.js b/packages/dd-trace/test/appsec/remote_config/index.spec.js index fd923c9a92b..e3fe644687f 100644 --- a/packages/dd-trace/test/appsec/remote_config/index.spec.js +++ b/packages/dd-trace/test/appsec/remote_config/index.spec.js @@ -9,7 +9,6 @@ let RemoteConfigManager let RuleManager let appsec let remoteConfig -let apiSecuritySampler describe('Remote Config index', () => { beforeEach(() => { @@ -33,11 +32,6 @@ describe('Remote Config index', () => { updateWafFromRC: sinon.stub() } - apiSecuritySampler = { - configure: sinon.stub(), - setRequestSampling: sinon.stub() - } - appsec = { enable: sinon.spy(), disable: sinon.spy() @@ -46,7 +40,6 @@ describe('Remote Config index', () => { remoteConfig = proxyquire('../src/appsec/remote_config', { './manager': RemoteConfigManager, '../rule_manager': RuleManager, - '../api_security_sampler': apiSecuritySampler, '..': appsec }) }) @@ -84,28 +77,6 @@ describe('Remote Config index', () => { expect(rc.setProductHandler).to.not.have.been.called }) - it('should listen ASM_API_SECURITY_SAMPLE_RATE when appsec.enabled=undefined and appSecurity.enabled=true', () => { - config.appsec = { enabled: undefined, apiSecurity: { enabled: true } } - - remoteConfig.enable(config) - - expect(RemoteConfigManager).to.have.been.calledOnceWithExactly(config) - expect(rc.updateCapabilities) - .to.have.been.calledWithExactly(RemoteConfigCapabilities.ASM_ACTIVATION, true) - expect(rc.updateCapabilities) - .to.have.been.calledWithExactly(RemoteConfigCapabilities.ASM_API_SECURITY_SAMPLE_RATE, true) - }) - - it('should listen ASM_API_SECURITY_SAMPLE_RATE when appsec.enabled=true and appSecurity.enabled=true', () => { - config.appsec = { enabled: true, apiSecurity: { enabled: true } } - - remoteConfig.enable(config) - - expect(RemoteConfigManager).to.have.been.calledOnceWithExactly(config) - expect(rc.updateCapabilities) - .to.have.been.calledWithExactly(RemoteConfigCapabilities.ASM_API_SECURITY_SAMPLE_RATE, true) - }) - describe('ASM_FEATURES remote config listener', () => { let listener @@ -142,106 +113,6 @@ describe('Remote Config index', () => { expect(appsec.disable).to.not.have.been.called }) }) - - describe('API Security Request Sampling', () => { - describe('OneClick', () => { - let listener - - beforeEach(() => { - config = { - appsec: { - enabled: undefined, - apiSecurity: { - requestSampling: 0.1 - } - } - } - - remoteConfig.enable(config) - - listener = rc.setProductHandler.firstCall.args[1] - }) - - it('should update apiSecuritySampler config', () => { - listener('apply', { - api_security: { - request_sample_rate: 0.5 - } - }) - - expect(apiSecuritySampler.setRequestSampling).to.be.calledOnceWithExactly(0.5) - }) - - it('should update apiSecuritySampler config and disable it', () => { - listener('apply', { - api_security: { - request_sample_rate: 0 - } - }) - - expect(apiSecuritySampler.setRequestSampling).to.be.calledOnceWithExactly(0) - }) - - it('should not update apiSecuritySampler config with values greater than 1', () => { - listener('apply', { - api_security: { - request_sample_rate: 5 - } - }) - - expect(apiSecuritySampler.configure).to.not.be.called - }) - - it('should not update apiSecuritySampler config with values less than 0', () => { - listener('apply', { - api_security: { - request_sample_rate: -0.4 - } - }) - - expect(apiSecuritySampler.configure).to.not.be.called - }) - - it('should not update apiSecuritySampler config with incorrect values', () => { - listener('apply', { - api_security: { - request_sample_rate: 'not_a_number' - } - }) - - expect(apiSecuritySampler.configure).to.not.be.called - }) - }) - - describe('Enabled', () => { - let listener - - beforeEach(() => { - config = { - appsec: { - enabled: true, - apiSecurity: { - requestSampling: 0.1 - } - } - } - - remoteConfig.enable(config) - - listener = rc.setProductHandler.firstCall.args[1] - }) - - it('should update config apiSecurity.requestSampling property value', () => { - listener('apply', { - api_security: { - request_sample_rate: 0.5 - } - }) - - expect(apiSecuritySampler.setRequestSampling).to.be.calledOnceWithExactly(0.5) - }) - }) - }) }) describe('enableWafUpdate', () => { diff --git a/packages/dd-trace/test/appsec/response_blocking.spec.js b/packages/dd-trace/test/appsec/response_blocking.spec.js index 2868a42b05b..fc003e4cf27 100644 --- a/packages/dd-trace/test/appsec/response_blocking.spec.js +++ b/packages/dd-trace/test/appsec/response_blocking.spec.js @@ -52,7 +52,10 @@ describe('HTTP Response Blocking', () => { appsec.enable(new Config({ appsec: { enabled: true, - rules: path.join(__dirname, 'response_blocking_rules.json') + rules: path.join(__dirname, 'response_blocking_rules.json'), + apiSecurity: { + enabled: false + } } })) }) diff --git a/packages/dd-trace/test/config.spec.js b/packages/dd-trace/test/config.spec.js index ca4d8b142d3..8648d1bfedd 100644 --- a/packages/dd-trace/test/config.spec.js +++ b/packages/dd-trace/test/config.spec.js @@ -248,7 +248,7 @@ describe('Config', () => { expect(config).to.have.nested.property('appsec.eventTracking.enabled', true) expect(config).to.have.nested.property('appsec.eventTracking.mode', 'safe') expect(config).to.have.nested.property('appsec.apiSecurity.enabled', true) - expect(config).to.have.nested.property('appsec.apiSecurity.requestSampling', 0.1) + expect(config).to.have.nested.property('appsec.apiSecurity.sampleDelay', 30) expect(config).to.have.nested.property('appsec.sca.enabled', null) expect(config).to.have.nested.property('appsec.standalone.enabled', undefined) expect(config).to.have.nested.property('remoteConfig.enabled', true) @@ -485,7 +485,7 @@ describe('Config', () => { process.env.DD_PROFILING_ENABLED = 'true' process.env.DD_INJECTION_ENABLED = 'profiler' process.env.DD_API_SECURITY_ENABLED = 'true' - process.env.DD_API_SECURITY_REQUEST_SAMPLE_RATE = 1 + process.env.DD_API_SECURITY_SAMPLE_DELAY = '25' process.env.DD_INSTRUMENTATION_INSTALL_ID = '68e75c48-57ca-4a12-adfc-575c4b05fcbe' process.env.DD_INSTRUMENTATION_INSTALL_TYPE = 'k8s_single_step' process.env.DD_INSTRUMENTATION_INSTALL_TIME = '1703188212' @@ -565,7 +565,7 @@ describe('Config', () => { expect(config).to.have.nested.property('appsec.eventTracking.enabled', true) expect(config).to.have.nested.property('appsec.eventTracking.mode', 'extended') expect(config).to.have.nested.property('appsec.apiSecurity.enabled', true) - expect(config).to.have.nested.property('appsec.apiSecurity.requestSampling', 1) + expect(config).to.have.nested.property('appsec.apiSecurity.sampleDelay', 25) expect(config).to.have.nested.property('appsec.sca.enabled', true) expect(config).to.have.nested.property('appsec.standalone.enabled', true) expect(config).to.have.nested.property('remoteConfig.enabled', false) @@ -1078,7 +1078,7 @@ describe('Config', () => { process.env.DD_APPSEC_GRAPHQL_BLOCKED_TEMPLATE_JSON = BLOCKED_TEMPLATE_JSON_PATH // json and html here process.env.DD_APPSEC_AUTOMATED_USER_EVENTS_TRACKING = 'disabled' process.env.DD_API_SECURITY_ENABLED = 'false' - process.env.DD_API_SECURITY_REQUEST_SAMPLE_RATE = 0.5 + process.env.DD_API_SECURITY_SAMPLE_DELAY = '10' process.env.DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS = 11 process.env.DD_IAST_ENABLED = 'false' process.env.DD_IAST_REDACTION_NAME_PATTERN = 'name_pattern_to_be_overriden_by_options' @@ -1141,8 +1141,7 @@ describe('Config', () => { mode: 'safe' }, apiSecurity: { - enabled: true, - requestSampling: 1.0 + enabled: true }, rasp: { enabled: false @@ -1211,7 +1210,7 @@ describe('Config', () => { expect(config).to.have.nested.property('appsec.eventTracking.enabled', true) expect(config).to.have.nested.property('appsec.eventTracking.mode', 'safe') expect(config).to.have.nested.property('appsec.apiSecurity.enabled', true) - expect(config).to.have.nested.property('appsec.apiSecurity.requestSampling', 1.0) + expect(config).to.have.nested.property('appsec.apiSecurity.sampleDelay', 10) expect(config).to.have.nested.property('remoteConfig.pollInterval', 42) expect(config).to.have.nested.property('iast.enabled', true) expect(config).to.have.nested.property('iast.requestSampling', 30) @@ -1239,8 +1238,7 @@ describe('Config', () => { mode: 'disabled' }, apiSecurity: { - enabled: true, - requestSampling: 1.0 + enabled: true }, rasp: { enabled: false @@ -1272,8 +1270,7 @@ describe('Config', () => { mode: 'safe' }, apiSecurity: { - enabled: false, - requestSampling: 0.5 + enabled: false }, rasp: { enabled: true @@ -1309,7 +1306,7 @@ describe('Config', () => { }, apiSecurity: { enabled: true, - requestSampling: 1.0 + sampleDelay: 30 }, sca: { enabled: null @@ -1990,35 +1987,6 @@ describe('Config', () => { }) }) - it('should sanitize values for API Security sampling between 0 and 1', () => { - expect(new Config({ - appsec: { - apiSecurity: { - enabled: true, - requestSampling: 5 - } - } - })).to.have.nested.property('appsec.apiSecurity.requestSampling', 1) - - expect(new Config({ - appsec: { - apiSecurity: { - enabled: true, - requestSampling: -5 - } - } - })).to.have.nested.property('appsec.apiSecurity.requestSampling', 0) - - expect(new Config({ - appsec: { - apiSecurity: { - enabled: true, - requestSampling: 0.1 - } - } - })).to.have.nested.property('appsec.apiSecurity.requestSampling', 0.1) - }) - context('payload tagging', () => { let env