diff --git a/integration-tests/graphql.spec.js b/integration-tests/graphql.spec.js index 4a66f163a83..0e7285083d8 100644 --- a/integration-tests/graphql.spec.js +++ b/integration-tests/graphql.spec.js @@ -112,7 +112,7 @@ describe('graphql', () => { assert.property(payload[1][0].metrics, '_dd.appsec.waf.duration') assert.propertyVal(payload[1][0].meta, 'appsec.event', 'true') assert.property(payload[1][0].meta, '_dd.appsec.json') - assert.propertyVal(payload[1][0].meta, '_dd.appsec.json', JSON.stringify(result)) + assert.deepStrictEqual(JSON.parse(payload[1][0].meta['_dd.appsec.json']), result) }) await axios({ diff --git a/package.json b/package.json index f7c0ad4d310..f7bfdcca401 100644 --- a/package.json +++ b/package.json @@ -84,7 +84,7 @@ }, "dependencies": { "@datadog/libdatadog": "^0.5.1", - "@datadog/native-appsec": "8.5.2", + "@datadog/native-appsec": "9.0.0", "@datadog/native-iast-taint-tracking": "4.0.0", "@datadog/native-metrics": "^3.1.1", "@datadog/pprof": "5.8.0", diff --git a/packages/dd-trace/src/appsec/reporter.js b/packages/dd-trace/src/appsec/reporter.js index a6d7368bf22..e9bdb0ff524 100644 --- a/packages/dd-trace/src/appsec/reporter.js +++ b/packages/dd-trace/src/appsec/reporter.js @@ -1,5 +1,8 @@ 'use strict' +const dc = require('dc-polyfill') +const zlib = require('zlib') + const Limiter = require('../rate_limiter') const { storage } = require('../../../datadog-core') const web = require('../plugins/util/web') @@ -7,6 +10,7 @@ const { ipHeaderList } = require('../plugins/util/ip_extractor') const { incrementWafInitMetric, incrementWafUpdatesMetric, + incrementWafConfigErrorsMetric, incrementWafRequestsMetric, updateWafRequestsMetricTags, updateRaspRequestsMetricTags, @@ -14,7 +18,6 @@ const { updateRateLimitedMetric, getRequestMetrics } = require('./telemetry') -const zlib = require('zlib') const { keepTrace } = require('../priority_sampler') const { ASM } = require('../standalone/product') @@ -25,6 +28,20 @@ const COLLECTED_REQUEST_BODY_MAX_STRING_LENGTH = 4096 const COLLECTED_REQUEST_BODY_MAX_DEPTH = 20 const COLLECTED_REQUEST_BODY_MAX_ELEMENTS_PER_NODE = 256 +const telemetryLogCh = dc.channel('datadog:telemetry:log') + +const WAF_DIAGNOSTICS_CONFIG_KEYS_TO_REPORT = [ + 'rules', + 'custom_rules', + 'exclusions', + 'actions', + 'processors', + 'scanners', + 'rules_override', + 'rules_data', + 'exclusion_data' +] + // default limiter, configurable with setRateLimit() let limiter = new Limiter(100) @@ -225,6 +242,54 @@ function reportWafInit (wafVersion, rulesVersion, diagnosticsRules = {}, success incrementWafInitMetric(wafVersion, rulesVersion, success) } +function logWafDiagnosticMessage (product, rcConfigId, configKey, message, level) { + telemetryLogCh.publish({ + message, + level, + tags: { + log_type: `rc::${product.toLowerCase()}::diagnostic`, + appsec_config_key: configKey, + rc_config_id: rcConfigId + } + }) +} + +function reportSuccessfulWafUpdate (product, rcConfigId, diagnostics) { + for (const configKey of WAF_DIAGNOSTICS_CONFIG_KEYS_TO_REPORT) { + const configDiagnostics = diagnostics[configKey] + if (!configDiagnostics) continue + + if (configDiagnostics.error) { + logWafDiagnosticMessage(product, rcConfigId, configKey, configDiagnostics.error, 'ERROR') + continue + } + + if (configDiagnostics.errors) { + for (const [errorMessage, errorIds] of Object.entries(configDiagnostics.errors)) { + logWafDiagnosticMessage( + product, + rcConfigId, + configKey, + `"${errorMessage}": ${JSON.stringify(errorIds)}`, + 'ERROR' + ) + } + } + + if (configDiagnostics.warnings) { + for (const [warningMessage, warningIds] of Object.entries(configDiagnostics.warnings)) { + logWafDiagnosticMessage( + product, + rcConfigId, + configKey, + `"${warningMessage}": ${JSON.stringify(warningIds)}`, + 'WARN' + ) + } + } + } +} + function reportMetrics (metrics, raspRule) { const store = storage('legacy').getStore() const rootSpan = store?.req && web.root(store.req) @@ -485,9 +550,11 @@ module.exports = { filterExtendedHeaders, formatHeaderName, reportWafInit, + reportSuccessfulWafUpdate, reportMetrics, reportAttack, reportWafUpdate: incrementWafUpdatesMetric, + reportWafConfigError: incrementWafConfigErrorsMetric, reportRaspRuleSkipped: updateRaspRuleSkippedMetricTags, reportDerivatives, finishRequest, diff --git a/packages/dd-trace/src/appsec/rule_manager.js b/packages/dd-trace/src/appsec/rule_manager.js index 10a66755d49..cd5ef881d5c 100644 --- a/packages/dd-trace/src/appsec/rule_manager.js +++ b/packages/dd-trace/src/appsec/rule_manager.js @@ -3,20 +3,16 @@ const fs = require('fs') const waf = require('./waf') const { ACKNOWLEDGED, ERROR } = require('../remote_config/apply_states') +const Reporter = require('./reporter') const blocking = require('./blocking') -let defaultRules +const ASM_PRODUCTS = new Set(['ASM', 'ASM_DD', 'ASM_DATA']) -let appliedRulesData = new Map() -let appliedRulesetId -let appliedRulesOverride = new Map() -let appliedExclusions = new Map() -let appliedCustomRules = new Map() let appliedActions = new Map() function loadRules (config) { - defaultRules = config.rules + const defaultRules = config.rules ? JSON.parse(fs.readFileSync(config.rules)) : require('./recommended.json') @@ -26,137 +22,66 @@ function loadRules (config) { } function updateWafFromRC ({ toUnapply, toApply, toModify }) { - const batch = new Set() - - const newRulesData = new SpyMap(appliedRulesData) - let newRuleset - let newRulesetId - const newRulesOverride = new SpyMap(appliedRulesOverride) - const newExclusions = new SpyMap(appliedExclusions) - const newCustomRules = new SpyMap(appliedCustomRules) const newActions = new SpyMap(appliedActions) + let wafUpdated = false + let wafUpdatedSuccess = true + for (const item of toUnapply) { - const { product, id } = item + if (!ASM_PRODUCTS.has(item.product)) continue + + try { + waf.wafManager.remove(item.path) + item.apply_state = ACKNOWLEDGED + wafUpdated = true - if (product === 'ASM_DATA') { - newRulesData.delete(id) - } else if (product === 'ASM_DD') { - if (appliedRulesetId === id) { - newRuleset = defaultRules + if (item.product === 'ASM') { + newActions.delete(item.id) } - } else if (product === 'ASM') { - newRulesOverride.delete(id) - newExclusions.delete(id) - newCustomRules.delete(id) - newActions.delete(id) + } catch (e) { + wafUpdatedSuccess = false + item.apply_state = ERROR + item.apply_error = e.toString() + Reporter.reportWafConfigError(waf.wafManager.ddwafVersion, waf.wafManager.rulesVersion) } } for (const item of [...toApply, ...toModify]) { - const { product, id, file } = item + if (!ASM_PRODUCTS.has(item.product)) continue - if (product === 'ASM_DATA') { - if (file && file.rules_data && file.rules_data.length) { - newRulesData.set(id, file.rules_data) - } + try { + const updateResult = waf.wafManager.update(item.product, item.file, item.path) + item.apply_state = updateResult.success ? ACKNOWLEDGED : ERROR - batch.add(item) - } else if (product === 'ASM_DD') { - if (appliedRulesetId && appliedRulesetId !== id && newRuleset !== defaultRules) { - item.apply_state = ERROR - item.apply_error = 'Multiple ruleset received in ASM_DD' + if (updateResult.success) { + wafUpdated = true + Reporter.reportSuccessfulWafUpdate(item.product, item.id, updateResult.diagnostics) } else { - if (file?.rules?.length) { - const { version, metadata, rules, processors, scanners } = file - - newRuleset = { version, metadata, rules, processors, scanners } - newRulesetId = id - } - - batch.add(item) - } - } else if (product === 'ASM') { - if (file?.rules_override?.length) { - newRulesOverride.set(id, file.rules_override) - } - - if (file?.exclusions?.length) { - newExclusions.set(id, file.exclusions) + wafUpdatedSuccess = false + item.apply_error = JSON.stringify(extractErrors(updateResult.diagnostics)) + Reporter.reportWafConfigError(waf.wafManager.ddwafVersion, waf.wafManager.rulesVersion) } - if (file?.custom_rules?.length) { - newCustomRules.set(id, file.custom_rules) + // check asm actions + if (updateResult.success && item.product === 'ASM' && item.file?.actions?.length) { + newActions.set(item.id, item.file.actions) } - - if (file?.actions?.length) { - newActions.set(id, file.actions) - } - - batch.add(item) + } catch (e) { + wafUpdatedSuccess = false + item.apply_state = ERROR + item.apply_error = e.toString() + Reporter.reportWafConfigError(waf.wafManager.ddwafVersion, waf.wafManager.rulesVersion) } } - let newApplyState = ACKNOWLEDGED - let newApplyError - - if (newRulesData.modified || - newRuleset || - newRulesOverride.modified || - newExclusions.modified || - newCustomRules.modified || - newActions.modified - ) { - const payload = newRuleset || {} - - if (newRulesData.modified) { - payload.rules_data = mergeRulesData(newRulesData) - } - if (newRulesOverride.modified) { - payload.rules_override = concatArrays(newRulesOverride) - } - if (newExclusions.modified) { - payload.exclusions = concatArrays(newExclusions) - } - if (newCustomRules.modified) { - payload.custom_rules = concatArrays(newCustomRules) - } - if (newActions.modified) { - payload.actions = concatArrays(newActions) - } - - try { - waf.update(payload) - - if (newRulesData.modified) { - appliedRulesData = newRulesData - } - if (newRuleset) { - appliedRulesetId = newRulesetId - } - if (newRulesOverride.modified) { - appliedRulesOverride = newRulesOverride - } - if (newExclusions.modified) { - appliedExclusions = newExclusions - } - if (newCustomRules.modified) { - appliedCustomRules = newCustomRules - } - if (newActions.modified) { - appliedActions = newActions - - blocking.setDefaultBlockingActionParameters(concatArrays(newActions)) - } - } catch (err) { - newApplyState = ERROR - newApplyError = err.toString() - } + if (wafUpdated) { + Reporter.reportWafUpdate(waf.wafManager.ddwafVersion, waf.wafManager.rulesVersion, wafUpdatedSuccess) } - for (const config of batch) { - config.apply_state = newApplyState - if (newApplyError) config.apply_error = newApplyError + // Manage blocking actions + if (newActions.modified) { + appliedActions = newActions + blocking.setDefaultBlockingActionParameters(concatArrays(newActions)) } } @@ -188,67 +113,31 @@ function concatArrays (files) { return [...files.values()].flat() } -/* - ASM_DATA Merge strategy: - The merge should be based on the id and type. For any duplicate items, the longer expiration should be taken. - As a result, multiple Rule Data may use the same DATA_ID and DATA_TYPE. In this case, all values are considered part - of a set and are merged. For instance, a denylist customized by environment may use a global Rule Data for all - environments and a Rule Data per environment -*/ - -function mergeRulesData (files) { - const mergedRulesData = new Map() - for (const [, file] of files) { - for (const ruleData of file) { - const key = `${ruleData.id}+${ruleData.type}` - if (mergedRulesData.has(key)) { - const existingRulesData = mergedRulesData.get(key) - ruleData.data.reduce(rulesReducer, existingRulesData.data) - } else { - mergedRulesData.set(key, copyRulesData(ruleData)) +function extractErrors (obj) { + if (typeof obj !== 'object' || obj === null) return null + + const result = {} + let isResultPopulated = false + + for (const [key, value] of Object.entries(obj)) { + if (key === 'error' || key === 'errors') { + result[key] = value + isResultPopulated = true + } else if (typeof value === 'object' && value !== null) { + const child = extractErrors(value) + if (child) { + isResultPopulated = true + result[key] = child } } } - return [...mergedRulesData.values()] -} - -function rulesReducer (existingEntries, rulesDataEntry) { - const existingEntry = existingEntries.find((entry) => entry.value === rulesDataEntry.value) - if (existingEntry && !('expiration' in existingEntry)) return existingEntries - if (existingEntry && 'expiration' in rulesDataEntry && rulesDataEntry.expiration > existingEntry.expiration) { - existingEntry.expiration = rulesDataEntry.expiration - } else if (existingEntry && !('expiration' in rulesDataEntry)) { - delete existingEntry.expiration - } else if (!existingEntry) { - existingEntries.push({ ...rulesDataEntry }) - } - return existingEntries -} -function copyRulesData (rulesData) { - const copy = { ...rulesData } - if (copy.data) { - const data = [] - copy.data.forEach(item => { - data.push({ ...item }) - }) - copy.data = data - } - return copy + return isResultPopulated ? result : null } function clearAllRules () { waf.destroy() - - defaultRules = undefined - - appliedRulesData.clear() - appliedRulesetId = undefined - appliedRulesOverride.clear() - appliedExclusions.clear() - appliedCustomRules.clear() appliedActions.clear() - blocking.setDefaultBlockingActionParameters(undefined) } diff --git a/packages/dd-trace/src/appsec/telemetry/index.js b/packages/dd-trace/src/appsec/telemetry/index.js index 32535b79a27..4b09689d854 100644 --- a/packages/dd-trace/src/appsec/telemetry/index.js +++ b/packages/dd-trace/src/appsec/telemetry/index.js @@ -13,6 +13,7 @@ const { trackWafMetrics, incrementWafInit, incrementWafUpdates, + incrementWafConfigErrors, incrementWafRequests } = require('./waf') @@ -121,6 +122,12 @@ function incrementWafUpdatesMetric (wafVersion, rulesVersion, success) { incrementWafUpdates(wafVersion, rulesVersion, success) } +function incrementWafConfigErrorsMetric (wafVersion, rulesVersion) { + if (!enabled) return + + incrementWafConfigErrors(wafVersion, rulesVersion) +} + function incrementWafRequestsMetric (req) { if (!req || !enabled) return @@ -167,6 +174,7 @@ module.exports = { updateRaspRuleSkippedMetricTags, incrementWafInitMetric, incrementWafUpdatesMetric, + incrementWafConfigErrorsMetric, incrementWafRequestsMetric, incrementMissingUserLoginMetric, incrementMissingUserIdMetric, diff --git a/packages/dd-trace/src/appsec/telemetry/waf.js b/packages/dd-trace/src/appsec/telemetry/waf.js index bf8d0e07eae..fa720d2d487 100644 --- a/packages/dd-trace/src/appsec/telemetry/waf.js +++ b/packages/dd-trace/src/appsec/telemetry/waf.js @@ -103,10 +103,11 @@ function incrementWafInit (wafVersion, rulesVersion, success) { function incrementWafUpdates (wafVersion, rulesVersion, success) { const versionsTags = getVersionsTags(wafVersion, rulesVersion) appsecMetrics.count('waf.updates', { ...versionsTags, success }).inc() +} - if (!success) { - appsecMetrics.count('waf.config_errors', versionsTags).inc() - } +function incrementWafConfigErrors (wafVersion, rulesVersion) { + const versionsTags = getVersionsTags(wafVersion, rulesVersion) + appsecMetrics.count('waf.config_errors', versionsTags).inc() } function incrementWafRequests (store) { @@ -137,5 +138,6 @@ module.exports = { trackWafMetrics, incrementWafInit, incrementWafUpdates, + incrementWafConfigErrors, incrementWafRequests } diff --git a/packages/dd-trace/src/appsec/waf/index.js b/packages/dd-trace/src/appsec/waf/index.js index b025a123f46..630d6aaedca 100644 --- a/packages/dd-trace/src/appsec/waf/index.js +++ b/packages/dd-trace/src/appsec/waf/index.js @@ -7,7 +7,6 @@ const waf = { wafManager: null, init, destroy, - update, run: noop, disposeContext: noop } @@ -34,18 +33,6 @@ function destroy () { waf.disposeContext = noop } -function update (newRules) { - // TODO: check race conditions between Appsec enable/disable and WAF updates, the whole RC state management in general - if (!waf.wafManager) throw new Error('Cannot update disabled WAF') - - try { - waf.wafManager.update(newRules) - } catch (err) { - log.error('[ASM] Could not apply rules from remote config') - throw err - } -} - function run (data, req, raspRule) { if (!req) { const store = storage('legacy').getStore() diff --git a/packages/dd-trace/src/appsec/waf/waf_manager.js b/packages/dd-trace/src/appsec/waf/waf_manager.js index e745818aa9e..a70f10374a9 100644 --- a/packages/dd-trace/src/appsec/waf/waf_manager.js +++ b/packages/dd-trace/src/appsec/waf/waf_manager.js @@ -6,12 +6,15 @@ const WAFContextWrapper = require('./waf_context_wrapper') const contexts = new WeakMap() +const DEFAULT_WAF_CONFIG_PATH = 'datadog/00/ASM_DD/default/config' + class WAFManager { constructor (rules, config) { this.config = config this.wafTimeout = config.wafTimeout this.ddwaf = this._loadDDWAF(rules) this.rulesVersion = this.ddwaf.diagnostics.ruleset_version + this.defaultRules = rules Reporter.reportWafInit(this.ddwafVersion, this.rulesVersion, this.ddwaf.diagnostics.rules, true) } @@ -23,7 +26,7 @@ class WAFManager { this.ddwafVersion = DDWAF.version() const { obfuscatorKeyRegex, obfuscatorValueRegex } = this.config - return new DDWAF(rules, { obfuscatorKeyRegex, obfuscatorValueRegex }) + return new DDWAF(rules, DEFAULT_WAF_CONFIG_PATH, { obfuscatorKeyRegex, obfuscatorValueRegex }) } catch (err) { this.ddwafVersion = this.ddwafVersion || 'unknown' Reporter.reportWafInit(this.ddwafVersion, 'unknown') @@ -51,19 +54,34 @@ class WAFManager { return wafContext } - update (newRules) { - try { - this.ddwaf.update(newRules) + update (product, rules, path) { + if (product === 'ASM_DD' && this.ddwaf.configPaths.includes(DEFAULT_WAF_CONFIG_PATH)) { + this.ddwaf.removeConfig(DEFAULT_WAF_CONFIG_PATH) + } - if (this.ddwaf.diagnostics.ruleset_version) { - this.rulesVersion = this.ddwaf.diagnostics.ruleset_version - } + const success = this.ddwaf.createOrUpdateConfig(rules, path) + const diagnostics = this.ddwaf.diagnostics - Reporter.reportWafUpdate(this.ddwafVersion, this.rulesVersion, true) - } catch (error) { - Reporter.reportWafUpdate(this.ddwafVersion, 'unknown', false) + if (diagnostics.ruleset_version) { + this.rulesVersion = diagnostics.ruleset_version + } + + if (product === 'ASM_DD' && !success && !this.ddwaf.configPaths.some(cp => cp.includes('ASM_DD'))) { + this.ddwaf.createOrUpdateConfig(this.defaultRules, DEFAULT_WAF_CONFIG_PATH) + } + + return { success, diagnostics } + } + + remove (path) { + this.ddwaf.removeConfig(path) + + if (!this.ddwaf.configPaths.some(cp => cp.includes('ASM_DD'))) { + this.ddwaf.createOrUpdateConfig(this.defaultRules, DEFAULT_WAF_CONFIG_PATH) + } - throw error + if (this.ddwaf.diagnostics.ruleset_version) { + this.rulesVersion = this.ddwaf.diagnostics.ruleset_version } } diff --git a/packages/dd-trace/test/appsec/index.spec.js b/packages/dd-trace/test/appsec/index.spec.js index a96aeeb6ca8..cfb7fcca733 100644 --- a/packages/dd-trace/test/appsec/index.spec.js +++ b/packages/dd-trace/test/appsec/index.spec.js @@ -1266,6 +1266,7 @@ describe('IP blocking', function () { const toModify = [{ product: 'ASM_DATA', id: '1', + path: 'datadog/00/ASM_DATA/test/IP blocking', file: ruleData }] const htmlDefaultContent = blockedTemplate.html @@ -1388,6 +1389,7 @@ describe('IP blocking', function () { const toModifyCustomActions = [{ product: 'ASM', id: 'custom-actions', + path: 'datadog/00/ASM/test/Custom actions/Default content with custom status', file: { actions: [ { @@ -1448,6 +1450,7 @@ describe('IP blocking', function () { const toModifyCustomActions = [{ product: 'ASM', id: 'custom-actions', + path: 'datadog/00/ASM/test/Custom actions/Redirect on error', file: { actions: [ { diff --git a/packages/dd-trace/test/appsec/reporter.spec.js b/packages/dd-trace/test/appsec/reporter.spec.js index 76cd1949d98..b8ee741b9ea 100644 --- a/packages/dd-trace/test/appsec/reporter.spec.js +++ b/packages/dd-trace/test/appsec/reporter.spec.js @@ -1,8 +1,10 @@ 'use strict' +const dc = require('dc-polyfill') const proxyquire = require('proxyquire') -const { storage } = require('../../../datadog-core') const zlib = require('zlib') + +const { storage } = require('../../../datadog-core') const { ASM } = require('../../src/standalone/product') const { USER_KEEP } = require('../../../../ext/priority') @@ -148,7 +150,7 @@ describe('reporter', () => { }) it('should not add entries to metricsQueue with success false', () => { - Reporter.reportWafInit(wafVersion, rulesVersion, diagnosticsRules, false) + Reporter.reportWafInit(wafVersion, rulesVersion, false) expect(Reporter.metricsQueue.get('_dd.appsec.waf.version')).to.be.undefined expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.loaded')).to.be.undefined @@ -169,10 +171,9 @@ describe('reporter', () => { Reporter.reportWafInit(wafVersion, rulesVersion, diagnosticsRules, true) + expect(telemetry.incrementWafInitMetric).to.have.been.calledOnceWithExactly(wafVersion, rulesVersion, true) expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.loaded')).to.be.eq(0) expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.error_count')).to.be.eq(0) - - expect(telemetry.incrementWafInitMetric).to.have.been.calledOnceWithExactly(wafVersion, rulesVersion, true) }) }) @@ -299,6 +300,76 @@ describe('reporter', () => { }) }) + describe('reportSuccessfulWafUpdate', () => { + it('should send diagnostics using telemetry logs', () => { + const telemetryLogHandlerAssert = sinon.stub() + + const telemetryLogCh = dc.channel('datadog:telemetry:log') + telemetryLogCh.subscribe(telemetryLogHandlerAssert) + + const product = 'ASM_DD' + const rcConfigId = '1' + const diagnostics = { + rules: { + loaded: [], + failed: ['blk-001-001'], + skipped: [], + errors: { + 'missing key operator': [ + 'blk-001-001' + ] + }, + warnings: { + 'invalid tag': [ + 'blk-001-001' + ] + } + }, + processors: { + loaded: ['http-endpoint-fingerprint'], + failed: [], + skipped: [], + warnings: { + 'no mappings defined': [ + 'http-endpoint-fingerprint' + ] + } + } + } + + Reporter.reportSuccessfulWafUpdate(product, rcConfigId, diagnostics) + + expect(telemetryLogHandlerAssert).to.have.been.calledThrice + expect(telemetryLogHandlerAssert.getCall(0)).to.have.been.calledWithExactly({ + message: '"missing key operator": ["blk-001-001"]', + level: 'ERROR', + tags: { + log_type: 'rc::asm_dd::diagnostic', + appsec_config_key: 'rules', + rc_config_id: '1' + } + }, 'datadog:telemetry:log') + expect(telemetryLogHandlerAssert.getCall(1)).to.have.been.calledWithExactly({ + message: '"invalid tag": ["blk-001-001"]', + level: 'WARN', + tags: { + log_type: 'rc::asm_dd::diagnostic', + appsec_config_key: 'rules', + rc_config_id: '1' + } + }, 'datadog:telemetry:log') + expect(telemetryLogHandlerAssert.getCall(2)).to.have.been.calledWithExactly({ + message: '"no mappings defined": ["http-endpoint-fingerprint"]', + level: 'WARN', + tags: { + log_type: 'rc::asm_dd::diagnostic', + appsec_config_key: 'processors', + rc_config_id: '1' + } + }, 'datadog:telemetry:log') + }) + }) + describe('reportAttack', () => { let req diff --git a/packages/dd-trace/test/appsec/rule_manager.spec.js b/packages/dd-trace/test/appsec/rule_manager.spec.js index 0da5e7dbec9..4d6c656a467 100644 --- a/packages/dd-trace/test/appsec/rule_manager.spec.js +++ b/packages/dd-trace/test/appsec/rule_manager.spec.js @@ -1,13 +1,15 @@ 'use strict' -const path = require('path') +const { assert } = require('chai') const fs = require('fs') -const { loadRules, clearAllRules, updateWafFromRC } = require('../../src/appsec/rule_manager') +const path = require('path') +const { loadRules, clearAllRules } = require('../../src/appsec/rule_manager') const Config = require('../../src/config') -const { ACKNOWLEDGED } = require('../../src/remote_config/apply_states') +const { ACKNOWLEDGED, UNACKNOWLEDGED, ERROR } = require('../../src/remote_config/apply_states') const rules = require('../../src/appsec/recommended.json') const waf = require('../../src/appsec/waf') +const WAFManager = require('../../src/appsec/waf/waf_manager') const blocking = require('../../src/appsec/blocking') describe('AppSec Rule Manager', () => { @@ -19,7 +21,6 @@ describe('AppSec Rule Manager', () => { sinon.stub(waf, 'init') sinon.stub(waf, 'destroy') - sinon.stub(waf, 'update') sinon.stub(blocking, 'setDefaultBlockingActionParameters') }) @@ -72,437 +73,284 @@ describe('AppSec Rule Manager', () => { }) describe('updateWafFromRC', () => { - describe('ASM_DATA', () => { - it('should call update with modified rules', () => { - const rulesData = { - rules_data: [{ - data: [ - { value: '1.2.3.4' } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } - - const toModify = [{ + function getRcConfigs () { + return { + toUnapply: [{ + id: 'test.toUnapply', + product: 'ASM_DD', + path: 'test/rule_manager/updateWafFromRC/ASM_DD/01', + file: {}, + apply_state: UNACKNOWLEDGED + }], + toModify: [{ + id: 'test.toModify', product: 'ASM_DATA', - id: '1', - file: rulesData + path: 'test/rule_manager/updateWafFromRC/ASM_DATA/01', + file: { + rules_data: [{ + data: [ + { value: '1.2.3.4' } + ], + id: 'blocked_ips', + type: 'data_with_expiration' + }] + }, + apply_state: UNACKNOWLEDGED + }], + toApply: [{ + id: 'test.toApply', + product: 'ASM', + path: 'test/rule_manager/updateWafFromRC/ASM/01', + file: { + exclusions: [{ + ekey: 'eValue' + }], + rules_override: [{ + roKey: 'roValue' + }], + custom_rules: [{ + piKey: 'piValue' + }] + }, + apply_state: UNACKNOWLEDGED }] - - updateWafFromRC({ toUnapply: [], toApply: [], toModify }) - expect(waf.update).to.have.been.calledOnceWithExactly(rulesData) + } + } + + let RuleManager + let reportWafConfigError, reportSuccessfulWafUpdate, reportWafUpdate + let setDefaultBlockingActionParameters + + beforeEach(() => { + reportWafUpdate = sinon.stub() + reportSuccessfulWafUpdate = sinon.stub() + reportWafConfigError = sinon.stub() + setDefaultBlockingActionParameters = sinon.stub() + + RuleManager = proxyquire.noCallThru()('../src/appsec/rule_manager', { + './reporter': { + reportWafUpdate, + reportSuccessfulWafUpdate, + reportWafConfigError + }, + './blocking': { + setDefaultBlockingActionParameters + } }) - it('should apply/modify the last rule with same id', () => { - const rulesDataFirst = { - rules_data: [{ - data: [ - { value: '1.2.3.4' } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } + waf.init.callThrough() - const rulesDataSecond = { - rules_data: [{ - data: [ - { value: '4.3.2.1' } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } + WAFManager.prototype.update = sinon.stub() + WAFManager.prototype.remove = sinon.stub() - const toModify = [ - { - product: 'ASM_DATA', - id: '1', - file: rulesDataFirst - }, - { - product: 'ASM_DATA', - id: '1', - file: rulesDataSecond - } - ] + RuleManager.clearAllRules() + config = new Config() + RuleManager.loadRules(config.appsec) + sinon.resetHistory() + }) - const expectedPayload = { - rules_data: [ - { data: [{ value: '4.3.2.1' }], id: 'blocked_ips', type: 'data_with_expiration' } - ] - } + it('should not apply configs from non ASM products', () => { + const rcConfigsForNonAsmProducts = { + toUnapply: [{ + id: 'test.toUnapply', + product: 'NON_ASM_PRODUCT', + path: 'test/rule_manager/updateWafFromRC/NON_ASM_PRODUCT/01', + file: {}, + apply_state: UNACKNOWLEDGED + }], + toModify: [{ + id: 'test.toModify', + product: 'NON_ASM_PRODUCT', + path: 'test/rule_manager/updateWafFromRC/NON_ASM_PRODUCT/02', + file: {}, + apply_state: UNACKNOWLEDGED + }], + toApply: [{ + id: 'test.toApply', + product: 'NON_ASM_PRODUCT', + path: 'test/rule_manager/updateWafFromRC/NON_ASM_PRODUCT/03', + file: {}, + apply_state: UNACKNOWLEDGED + }] + } - updateWafFromRC({ toUnapply: [], toApply: [], toModify }) - expect(waf.update).to.have.been.calledOnce - expect(waf.update).calledWithExactly(expectedPayload) + assert.doesNotThrow(() => { + RuleManager.updateWafFromRC(rcConfigsForNonAsmProducts) }) - it('should merge all apply/modify rules', () => { - const toModify = [ - { - product: 'ASM_DATA', - id: '1', - file: { - rules_data: [{ - data: [ - { value: '1.2.3.4' } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } - }, - { - product: 'ASM_DATA', - id: '2', - file: { - rules_data: [{ - data: [ - { value: '4.3.2.1' } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } - } - ] + sinon.assert.notCalled(waf.wafManager.update) + sinon.assert.notCalled(waf.wafManager.remove) + assert.strictEqual(rcConfigsForNonAsmProducts.toUnapply[0].apply_state, UNACKNOWLEDGED) + assert.notProperty(rcConfigsForNonAsmProducts.toUnapply[0], 'apply_error') + assert.strictEqual(rcConfigsForNonAsmProducts.toModify[0].apply_state, UNACKNOWLEDGED) + assert.notProperty(rcConfigsForNonAsmProducts.toModify[0], 'apply_error') + assert.strictEqual(rcConfigsForNonAsmProducts.toApply[0].apply_state, UNACKNOWLEDGED) + assert.notProperty(rcConfigsForNonAsmProducts.toApply[0], 'apply_error') + }) - const expectedPayload = { - rules_data: [ - { data: [{ value: '1.2.3.4' }, { value: '4.3.2.1' }], id: 'blocked_ips', type: 'data_with_expiration' } - ] - } + it('should apply configs from ASM products', () => { + const rcConfigs = getRcConfigs() + + RuleManager.updateWafFromRC(rcConfigs) + + sinon.assert.calledOnceWithExactly(waf.wafManager.remove, rcConfigs.toUnapply[0].path) + sinon.assert.calledTwice(waf.wafManager.update) + sinon.assert.calledWith( + waf.wafManager.update.getCall(0), + rcConfigs.toApply[0].product, + rcConfigs.toApply[0].file, + rcConfigs.toApply[0].path + ) + sinon.assert.calledWith( + waf.wafManager.update.getCall(1), + rcConfigs.toModify[0].product, + rcConfigs.toModify[0].file, + rcConfigs.toModify[0].path + ) + }) - updateWafFromRC({ toUnapply: [], toApply: [], toModify }) - expect(waf.update).to.have.been.calledOnce - expect(waf.update).calledWithExactly(expectedPayload) - }) + it('should update apply_state and apply_error on successful apply', () => { + WAFManager.prototype.update.returns({ success: true }) - it('should merge all apply/modify and unapply rules', () => { - const toModify = [ - { - product: 'ASM_DATA', - id: '1', - file: { - rules_data: [{ - data: [ - { value: '4.3.2.1' } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } - }, - { - product: 'ASM_DATA', - id: '2', - file: { - rules_data: [{ - data: [ - { value: '4.3.2.1' } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } - } - ] + const rcConfigs = getRcConfigs() - const toUnapply = [ - { - product: 'ASM_DATA', - id: '2', - file: { - rules_data: [{ - data: [ - { value: '1.2.3.4' } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } - } - ] + RuleManager.updateWafFromRC(rcConfigs) - const expectedPayload = { - rules_data: [ - { data: [{ value: '4.3.2.1' }], id: 'blocked_ips', type: 'data_with_expiration' } - ] - } + assert.strictEqual(rcConfigs.toUnapply[0].apply_state, ACKNOWLEDGED) + assert.notProperty(rcConfigs.toUnapply[0], 'apply_error') + assert.strictEqual(rcConfigs.toModify[0].apply_state, ACKNOWLEDGED) + assert.notProperty(rcConfigs.toModify[0], 'apply_error') + assert.strictEqual(rcConfigs.toApply[0].apply_state, ACKNOWLEDGED) + assert.notProperty(rcConfigs.toApply[0], 'apply_error') + }) - updateWafFromRC({ toUnapply, toApply: [], toModify }) - expect(waf.update).to.have.been.calledOnce - expect(waf.update).calledWithExactly(expectedPayload) - }) + it('should update apply_state and apply_error on failed config remove', () => { + const removeConfigError = new Error('Error remove config') + WAFManager.prototype.remove.throws(removeConfigError) - it('should merge all apply/modify rules with different expiration', () => { - // TODO: use case from previous tests, not sure if this can happen. - const toApply = [ - { - product: 'ASM_DATA', - id: '1', - file: { - rules_data: [{ - data: [ - { value: '1.2.3.4', expiration: 200 } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } - }, - { - product: 'ASM_DATA', - id: '2', - file: { - rules_data: [{ - data: [ - { value: '1.2.3.4', expiration: 100 } - ], - id: 'blocked_ips', - type: 'data_with_expiration' - }] - } - } - ] + const { toUnapply } = getRcConfigs() - const expectedPayload = { - rules_data: [ - { data: [{ value: '1.2.3.4', expiration: 200 }], id: 'blocked_ips', type: 'data_with_expiration' } - ] - } + RuleManager.updateWafFromRC({ toUnapply, toApply: [], toModify: [] }) - updateWafFromRC({ toUnapply: [], toApply, toModify: [] }) - expect(waf.update).to.have.been.calledOnce - expect(waf.update).calledWithExactly(expectedPayload) - }) + assert.strictEqual(toUnapply[0].apply_state, ERROR) + assert.strictEqual(toUnapply[0].apply_error, removeConfigError.toString()) }) - describe('ASM_DD', () => { - beforeEach(() => { - loadRules(config.appsec) - }) - - it('should apply new rules', () => { - const testRules = { - version: '2.2', - metadata: { rules_version: '1.5.0' }, - rules: [{ - id: 'test-id', - name: 'test-name', - tags: { - type: 'security_scanner', - category: 'attack_attempt', - confidence: '1' - }, - conditions: [] - }], - processors: [{ - id: 'test-processor-id', - generator: 'test-generator', - evaluate: false, - output: true - }], - scanners: [{ - id: 'test-scanner-id', - name: 'Test name', - key: { - operator: 'match_regex', - parameters: { - regex: 'test-regex' - } - }, - value: { - operator: 'match_regex', - parameters: { - regex: 'test-regex-2' - } - }, - tags: { - type: 'card', - card_type: 'test', - category: 'payment' - } - }] + it('should update apply_state and apply_error on failed config update', () => { + const diagnostics = { + rules: { + loaded: [], + failed: ['blk-001-001'], + skipped: [], + errors: { + 'missing key operator': [ + 'blk-001-001' + ] + }, + warnings: { + 'invalid tag': [ + 'blk-001-001' + ] + } + }, + processors: { + loaded: ['http-endpoint-fingerprint'], + failed: [], + skipped: [], + errors: { + 'missing mapping definition': [ + 'http-endpoint-fingerprint' + ] + }, + warnings: { + 'no mappings defined': [ + 'http-endpoint-fingerprint' + ] + } + }, + scanners: { + error: 'Fatal error' } + } + + const expectedApplyError = { + rules: { + errors: diagnostics.rules.errors + }, + processors: { + errors: diagnostics.processors.errors + }, + scanners: { + error: diagnostics.scanners.error + } + } - const toApply = [ - { - product: 'ASM_DD', - id: '1', - file: testRules - } - ] + WAFManager.prototype.update.returns({ success: false, diagnostics }) - updateWafFromRC({ toUnapply: [], toApply, toModify: [] }) - expect(waf.update).to.have.been.calledOnceWithExactly(testRules) - }) + const { toModify, toApply } = getRcConfigs() - it('should maintain previously added exclusions and rules_overrides', () => { - const asm = { - exclusions: [{ - ekey: 'eValue' - }] - } - const testRules = { - version: '2.2', - metadata: { rules_version: '1.5.0' }, - rules: [{ - id: 'test-id', - name: 'test-name', - tags: { - type: 'security_scanner', - category: 'attack_attempt', - confidence: '1' - }, - conditions: [] - }], - processors: [{ - id: 'test-processor-id', - generator: 'test-generator', - evaluate: false, - output: true - }], - scanners: [{ - id: 'test-scanner-id', - name: 'Test name', - key: { - operator: 'match_regex', - parameters: { - regex: 'test-regex' - } - }, - value: { - operator: 'match_regex', - parameters: { - regex: 'test-regex-2' - } - }, - tags: { - type: 'card', - card_type: 'test', - category: 'payment' - } - }] - } + RuleManager.updateWafFromRC({ toUnapply: [], toApply, toModify }) - const toApply = [ - { - product: 'ASM', - id: '1', - file: asm - }, - { - product: 'ASM_DD', - id: '2', - file: testRules - } - ] + assert.strictEqual(toApply[0].apply_state, ERROR) + assert.strictEqual(toApply[0].apply_error, JSON.stringify(expectedApplyError)) + assert.strictEqual(toModify[0].apply_state, ERROR) + assert.strictEqual(toModify[0].apply_error, JSON.stringify(expectedApplyError)) + }) - updateWafFromRC({ toUnapply: [], toApply, toModify: [] }) - expect(waf.update).to.have.been.calledWithExactly({ ...testRules, ...asm }) - }) + it('should report successful waf update', () => { + WAFManager.prototype.update.returns({ success: true, diagnostics: {} }) - it('should support hotswapping ruleset in same batch', () => { - const rules1 = { - product: 'ASM_DD', - id: 'rules1', - file: { - version: '2.2', - metadata: { rules_version: '1.5.0' }, - rules: [{ - id: 'test-id', - name: 'test-name', - tags: { - type: 'security_scanner', - category: 'attack_attempt', - confidence: '1' - }, - conditions: [ - { - parameters: { - inputs: [ - { address: 'http.test' } - ], - data: 'blocked_ips' - }, - operator: 'ip_match' - } - ] - }] - } - } + const rcConfigs = getRcConfigs() - const rules2 = { - product: 'ASM_DD', - id: 'rules2', - file: { - version: '2.2', - metadata: { rules_version: '1.5.0' }, - rules: [{ - id: 'test-id', - name: 'test-name', - tags: { - type: 'security_scanner', - category: 'attack_attempt', - confidence: '1' - }, - conditions: [ - { - parameters: { - inputs: [ - { address: 'http.test' } - ], - data: 'blocked_ips' - }, - operator: 'ip_match' - } - ] - }] - } - } + RuleManager.updateWafFromRC(rcConfigs) - updateWafFromRC({ toUnapply: [], toApply: [rules1], toModify: [] }) + sinon.assert.calledOnceWithExactly( + reportWafUpdate, + waf.wafManager.ddwafVersion, + waf.wafManager.rulesVersion, + true + ) + sinon.assert.calledTwice(reportSuccessfulWafUpdate) + sinon.assert.calledWith(reportSuccessfulWafUpdate, rcConfigs.toModify[0].product, rcConfigs.toModify[0].id, {}) + sinon.assert.calledWith(reportSuccessfulWafUpdate, rcConfigs.toApply[0].product, rcConfigs.toApply[0].id, {}) + }) - updateWafFromRC({ toUnapply: [rules1], toApply: [rules2], toModify: [] }) + it('should report waf config error', () => { + WAFManager.prototype.remove.throws(new Error('Error removing config')) + WAFManager.prototype.update.returns({ success: false, diagnostics: {} }) - expect(rules1.apply_state).to.equal(ACKNOWLEDGED) - expect(rules1.apply_error).to.equal(undefined) - expect(rules2.apply_state).to.equal(ACKNOWLEDGED) - expect(rules2.apply_error).to.equal(undefined) - }) + const rcConfigs = getRcConfigs() + + RuleManager.updateWafFromRC(rcConfigs) + sinon.assert.notCalled(reportWafUpdate) + sinon.assert.calledThrice(reportWafConfigError) + sinon.assert.alwaysCalledWithExactly( + reportWafConfigError, + waf.wafManager.ddwafVersion, + waf.wafManager.rulesVersion + ) }) - describe('ASM', () => { - it('should apply both rules_override and exclusions', () => { - const asm = { - exclusions: [{ - ekey: 'eValue' - }], - rules_override: [{ - roKey: 'roValue' - }], - custom_rules: [{ - piKey: 'piValue' - }] - } + it('should report waf update', () => { + WAFManager.prototype.update.onFirstCall().returns({ success: false, diagnostics: {} }) + WAFManager.prototype.update.onSecondCall().returns({ success: true, diagnostics: {} }) - const toApply = [ - { - product: 'ASM', - id: '1', - file: asm - } - ] + const rcConfigs = getRcConfigs() - updateWafFromRC({ toUnapply: [], toApply, toModify: [] }) + RuleManager.updateWafFromRC(rcConfigs) - expect(waf.update).to.have.been.calledOnceWithExactly(asm) - }) + sinon.assert.calledOnceWithExactly( + reportWafUpdate, + waf.wafManager.ddwafVersion, + waf.wafManager.rulesVersion, + false + ) + }) + describe('ASM', () => { it('should apply blocking actions', () => { + WAFManager.prototype.update.returns({ success: true }) + const toApply = [ { product: 'ASM', @@ -536,32 +384,31 @@ describe('AppSec Rule Manager', () => { } ] - updateWafFromRC({ toUnapply: [], toApply, toModify: [] }) + RuleManager.updateWafFromRC({ toUnapply: [], toApply, toModify: [] }) - const expectedPayload = { - actions: [ - { - id: 'notblock', - parameters: { - location: '/notfound', - status_code: 404 - } - }, - { - id: 'block', - parameters: { - location: '/redirected', - status_code: 302 - } + const expectedActions = [ + { + id: 'notblock', + parameters: { + location: '/notfound', + status_code: 404 } - ] - } + }, + { + id: 'block', + parameters: { + location: '/redirected', + status_code: 302 + } + } + ] - expect(waf.update).to.have.been.calledOnceWithExactly(expectedPayload) - expect(blocking.setDefaultBlockingActionParameters).to.have.been.calledOnceWithExactly(expectedPayload.actions) + sinon.assert.calledOnceWithExactly(setDefaultBlockingActionParameters, expectedActions) }) it('should unapply blocking actions', () => { + WAFManager.prototype.update.returns({ success: true }) + const asm = { actions: [ { @@ -581,11 +428,10 @@ describe('AppSec Rule Manager', () => { file: asm } ] - updateWafFromRC({ toUnapply: [], toApply, toModify: [] }) - expect(waf.update).to.have.been.calledOnceWithExactly(asm) - expect(blocking.setDefaultBlockingActionParameters).to.have.been.calledOnceWithExactly(asm.actions) + RuleManager.updateWafFromRC({ toUnapply: [], toApply, toModify: [] }) + sinon.assert.calledOnceWithExactly(setDefaultBlockingActionParameters, asm.actions) sinon.resetHistory() const toUnapply = [ @@ -595,39 +441,9 @@ describe('AppSec Rule Manager', () => { } ] - updateWafFromRC({ toUnapply, toApply: [], toModify: [] }) - - expect(waf.update).to.have.been.calledOnceWithExactly({ actions: [] }) - expect(blocking.setDefaultBlockingActionParameters).to.have.been.calledOnceWithExactly([]) - }) - - it('should ignore other properties', () => { - const asm = { - exclusions: [{ - ekey: 'eValue' - }], - rules_override: [{ - roKey: 'roValue' - }], - not_supported: [{ - nsKey: 'nsValue' - }] - } - - const toApply = [ - { - product: 'ASM', - id: '1', - file: asm - } - ] - - updateWafFromRC({ toUnapply: [], toApply, toModify: [] }) + RuleManager.updateWafFromRC({ toUnapply, toApply: [], toModify: [] }) - expect(waf.update).to.have.been.calledOnceWithExactly({ - exclusions: asm.exclusions, - rules_override: asm.rules_override - }) + sinon.assert.calledOnceWithExactly(setDefaultBlockingActionParameters, []) }) }) }) diff --git a/packages/dd-trace/test/appsec/telemetry/waf.spec.js b/packages/dd-trace/test/appsec/telemetry/waf.spec.js index a7b7e3c1eb5..bd7fd54c656 100644 --- a/packages/dd-trace/test/appsec/telemetry/waf.spec.js +++ b/packages/dd-trace/test/appsec/telemetry/waf.spec.js @@ -267,22 +267,33 @@ describe('Appsec Waf Telemetry metrics', () => { expect(metrics.series[0].tags).to.include('event_rules_version:0.0.2') expect(metrics.series[0].tags).to.include('success:true') }) + }) + + describe('incrementWafConfigErrors', () => { + it('should increment waf.config_errors metric', () => { + appsecTelemetry.incrementWafConfigErrorsMetric(wafVersion, rulesVersion) - it('should increment waf.updates and waf.config_errors on failed update', () => { + expect(count).to.have.been.calledOnceWithExactly('waf.config_errors', { + waf_version: wafVersion, + event_rules_version: rulesVersion + }) + expect(inc).to.have.been.calledOnce + }) + + it('should increment waf.config_errors metric multiple times', () => { sinon.restore() - appsecTelemetry.incrementWafUpdatesMetric(wafVersion, rulesVersion, false) + appsecTelemetry.incrementWafConfigErrorsMetric(wafVersion, rulesVersion, true) + appsecTelemetry.incrementWafConfigErrorsMetric(wafVersion, rulesVersion, true) + appsecTelemetry.incrementWafConfigErrorsMetric(wafVersion, rulesVersion, true) const { metrics } = appsecNamespace.toJSON() - expect(metrics.series.length).to.be.eq(2) - expect(metrics.series[0].metric).to.be.eq('waf.updates') + expect(metrics.series.length).to.be.eq(1) + expect(metrics.series[0].metric).to.be.eq('waf.config_errors') + expect(metrics.series[0].points.length).to.be.eq(1) + expect(metrics.series[0].points[0][1]).to.be.eq(3) expect(metrics.series[0].tags).to.include('waf_version:0.0.1') expect(metrics.series[0].tags).to.include('event_rules_version:0.0.2') - expect(metrics.series[0].tags).to.include('success:false') - - expect(metrics.series[1].metric).to.be.eq('waf.config_errors') - expect(metrics.series[1].tags).to.include('waf_version:0.0.1') - expect(metrics.series[1].tags).to.include('event_rules_version:0.0.2') }) }) diff --git a/packages/dd-trace/test/appsec/waf/index.spec.js b/packages/dd-trace/test/appsec/waf/index.spec.js index c7ff8682705..00885fcc322 100644 --- a/packages/dd-trace/test/appsec/waf/index.spec.js +++ b/packages/dd-trace/test/appsec/waf/index.spec.js @@ -27,11 +27,6 @@ describe('WAF Manager', () => { DDWAF.version = sinon.stub().returns('1.2.3') DDWAF.prototype.dispose = sinon.stub() DDWAF.prototype.createContext = sinon.stub() - DDWAF.prototype.update = sinon.stub().callsFake(function (newRules) { - if (newRules?.metadata?.rules_version) { - this.diagnostics.ruleset_version = newRules?.metadata?.rules_version - } - }) DDWAF.prototype.diagnostics = { ruleset_version: '1.0.0', rules: { @@ -51,7 +46,6 @@ describe('WAF Manager', () => { sinon.stub(Reporter.metricsQueue, 'set') sinon.stub(Reporter, 'reportMetrics') sinon.stub(Reporter, 'reportAttack') - sinon.stub(Reporter, 'reportWafUpdate') sinon.stub(Reporter, 'reportDerivatives') sinon.spy(Reporter, 'reportWafInit') @@ -170,81 +164,6 @@ describe('WAF Manager', () => { }) }) - describe('wafManager.update', () => { - const wafVersion = '2.3.4' - - beforeEach(() => { - DDWAF.version.returns(wafVersion) - - waf.init(rules, config.appsec) - }) - - it('should call ddwaf.update', () => { - const rules = { - rules_data: [ - { - id: 'blocked_users', - type: 'data_with_expiration', - data: [ - { - expiration: 9999999999, - value: 'user1' - } - ] - } - ] - } - - waf.update(rules) - - expect(DDWAF.prototype.update).to.be.calledOnceWithExactly(rules) - expect(Reporter.reportWafUpdate).to.be.calledOnceWithExactly(wafVersion, '1.0.0', true) - }) - - it('should call Reporter.reportWafUpdate on successful update', () => { - const rules = { - metadata: { - rules_version: '4.2.0' - }, - rules_data: [ - { - id: 'blocked_users', - type: 'data_with_expiration', - data: [ - { - expiration: 9999999999, - value: 'user1' - } - ] - } - ] - } - - waf.update(rules) - - expect(Reporter.reportWafUpdate).to.be.calledOnceWithExactly(wafVersion, '4.2.0', true) - }) - - it('should call Reporter.reportWafUpdate on failed update', () => { - const rules = { - metadata: { - rules_version: '4.2.0' - } - } - const error = new Error('Failed to update rules') - - DDWAF.prototype.update = sinon.stub().throws(error) - - try { - waf.update(rules) - expect.fail('waf.update should have thrown an error') - } catch (err) { - expect(err).to.equal(error) - expect(Reporter.reportWafUpdate).to.be.calledOnceWithExactly(wafVersion, 'unknown', false) - } - }) - }) - describe('WAFContextWrapper', () => { let ddwafContext, wafContextWrapper, req diff --git a/packages/dd-trace/test/appsec/waf/waf_manager.spec.js b/packages/dd-trace/test/appsec/waf/waf_manager.spec.js index 9093f43d921..ee7615e3b59 100644 --- a/packages/dd-trace/test/appsec/waf/waf_manager.spec.js +++ b/packages/dd-trace/test/appsec/waf/waf_manager.spec.js @@ -12,6 +12,9 @@ describe('WAFManager', () => { DDWAF.prototype.knownAddresses = knownAddresses DDWAF.prototype.diagnostics = {} DDWAF.prototype.createContext = sinon.stub() + DDWAF.prototype.createOrUpdateConfig = sinon.stub() + DDWAF.prototype.removeConfig = sinon.stub() + DDWAF.prototype.configPaths = [] WAFContextWrapper = sinon.stub() WAFManager = proxyquire('../../../src/appsec/waf/waf_manager', { @@ -30,4 +33,165 @@ describe('WAFManager', () => { sinon.assert.calledOnceWithMatch(WAFContextWrapper, any, any, any, any, knownAddresses) }) }) + + describe('WAF configurations handling', () => { + let wafManager + + const DEFAULT_RULES = { + version: '2.2', + metadata: { + rules_version: '1.14.2' + }, + rules: [ + { + id: 'blk-001-001', + name: 'Block IP Addresses', + tags: { + type: 'block_ip', + category: 'security_response', + module: 'network-acl' + }, + conditions: [ + { + parameters: { + inputs: [ + { + address: 'http.client_ip' + } + ], + data: 'blocked_ips' + }, + operator: 'ip_match' + } + ], + transformers: [], + on_match: [ + 'block' + ] + } + ] + } + + const ASM_CONFIG = { + rules_override: [], + actions: [], + exclusions: [], + custom_rules: [] + } + + const ASM_DATA_CONFIG = { + rules_data: [ + { + data: [ + { + expiration: 1661848350, + value: '188.243.182.156' + }, + { + expiration: 1661848350, + value: '51.222.158.205' + } + ], + id: 'blocked_ips', + type: 'ip_with_expiration' + } + ] + } + + const ASM_DD_CONFIG = { + version: '2.2', + metadata: { + rules_version: '1.42.11' + }, + rules: [] + } + + beforeEach(() => { + wafManager = new WAFManager(DEFAULT_RULES, {}) + }) + + afterEach(() => { + sinon.restore() + }) + + describe('update config', () => { + it('should update WAF config - ASM / ASM_DATA', () => { + DDWAF.prototype.configPaths = ['datadog/00/ASM_DD/default/config'] + + wafManager.update( + 'ASM', + ASM_CONFIG, + 'datadog/00/ASM/test/update_config' + ) + + wafManager.update( + 'ASM_DATA', + ASM_DATA_CONFIG, + 'datadog/00/ASM_DATA/test/update_config' + ) + + sinon.assert.calledWithExactly( + DDWAF.prototype.createOrUpdateConfig.getCall(0), + ASM_CONFIG, + 'datadog/00/ASM/test/update_config' + ) + sinon.assert.calledWithExactly( + DDWAF.prototype.createOrUpdateConfig.getCall(1), + ASM_DATA_CONFIG, + 'datadog/00/ASM_DATA/test/update_config' + ) + }) + + it('should remove default rules on ASM_DD update', () => { + DDWAF.prototype.configPaths = ['datadog/00/ASM_DD/default/config'] + + wafManager.update('ASM_DD', ASM_DD_CONFIG, 'datadog/00/ASM_DD/test/update_config') + + sinon.assert.calledOnceWithExactly( + DDWAF.prototype.removeConfig, + 'datadog/00/ASM_DD/default/config' + ) + sinon.assert.calledOnceWithExactly( + DDWAF.prototype.createOrUpdateConfig, + ASM_DD_CONFIG, + 'datadog/00/ASM_DD/test/update_config' + ) + }) + + it('should apply default rules when no ASM config is present after config update fail', () => { + DDWAF.prototype.configPaths = [] + DDWAF.prototype.createOrUpdateConfig.returns(false) + + wafManager.update('ASM_DD', ASM_DD_CONFIG, 'datadog/00/ASM_DD/test/update_config') + + sinon.assert.calledWithExactly( + DDWAF.prototype.createOrUpdateConfig.getCall(1), + DEFAULT_RULES, + 'datadog/00/ASM_DD/default/config' + ) + }) + }) + + describe('remove config', () => { + it('should remove WAF config', () => { + DDWAF.prototype.configPaths = ['datadog/00/ASM_DD/default/config'] + + wafManager.remove('path/to/remove') + + sinon.assert.calledOnceWithExactly(DDWAF.prototype.removeConfig, 'path/to/remove') + }) + + it('should apply default rules when no ASM config is present after config removal', () => { + DDWAF.prototype.configPaths = [] + + wafManager.remove('path/to/remove') + + sinon.assert.calledOnceWithExactly( + DDWAF.prototype.createOrUpdateConfig, + DEFAULT_RULES, + 'datadog/00/ASM_DD/default/config' + ) + }) + }) + }) }) diff --git a/yarn.lock b/yarn.lock index f7ee0b29250..17bb9e26fa6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -262,10 +262,10 @@ resolved "https://registry.yarnpkg.com/@datadog/libdatadog/-/libdatadog-0.5.1.tgz#fe5c101c457998b74cb66f555f63197b34cad4ba" integrity sha512-KsdOxTUmtjoygaZInSS5U0+KnqoxPKGpcBjGgOHR9NDKfXzmbpy5AmoaPL7JxmMxQzwknpxSi7qzBOSB3yMoJg== -"@datadog/native-appsec@8.5.2": - version "8.5.2" - resolved "https://registry.yarnpkg.com/@datadog/native-appsec/-/native-appsec-8.5.2.tgz#93a2c15c71c2a90e19e12506fbbdec9ccbc91541" - integrity sha512-lETBaVhBk+9o0pc+LDnXvp2ImDyT8K2deuqLf8A6q4/QjzCCXyR/yZO9R5+Kdoc93jZMRTWV9Pr4pBwHEdJSVA== +"@datadog/native-appsec@9.0.0": + version "9.0.0" + resolved "https://registry.yarnpkg.com/@datadog/native-appsec/-/native-appsec-9.0.0.tgz#3ac854d8597ca75af0aa534aae28fa7f13057d2a" + integrity sha512-C7v16pP4p4Y+Cx0jcxTYmhZTptfVs8TYbn6LH/aQgTkwx2tWsWN5lss7fjBYjWyZoPMwVh2UX/yDm4ES25hJnQ== dependencies: node-gyp-build "^3.9.0"