From b7cddd6ca2ca4c1db308e327393aa1eb534a3d1c Mon Sep 17 00:00:00 2001 From: Igor Unanua Date: Fri, 20 Jan 2023 09:59:01 +0100 Subject: [PATCH 1/7] IAST metrics --- .../analyzers/command-injection-analyzer.js | 3 + .../appsec/iast/analyzers/cookie-analyzer.js | 8 +- .../iast/analyzers/ldap-injection-analyzer.js | 3 + .../iast/analyzers/path-traversal-analyzer.js | 34 +- .../iast/analyzers/sql-injection-analyzer.js | 7 +- .../appsec/iast/analyzers/ssrf-analyzer.js | 2 + .../unvalidated-redirect-analyzer.js | 4 +- .../iast/analyzers/vulnerability-analyzer.js | 37 +- .../iast/analyzers/weak-cipher-analyzer.js | 3 + .../iast/analyzers/weak-hash-analyzer.js | 6 +- packages/dd-trace/src/appsec/iast/iast-log.js | 2 +- .../dd-trace/src/appsec/iast/iast-plugin.js | 202 +++++++++ packages/dd-trace/src/appsec/iast/index.js | 11 +- packages/dd-trace/src/appsec/iast/tags.js | 3 +- .../appsec/iast/taint-tracking/csi-methods.js | 27 +- .../src/appsec/iast/taint-tracking/index.js | 6 +- .../appsec/iast/taint-tracking/operations.js | 27 +- .../src/appsec/iast/taint-tracking/plugin.js | 48 ++- .../iast/taint-tracking/rewriter-telemetry.js | 33 ++ .../appsec/iast/taint-tracking/rewriter.js | 39 +- .../{origin-types.js => source-types.js} | 0 .../taint-tracking/taint-tracking-impl.js | 113 ++++-- .../src/appsec/iast/telemetry/iast-metric.js | 101 +++++ .../src/appsec/iast/telemetry/index.js | 53 +++ .../iast/telemetry/{logs.js => log/index.js} | 8 +- .../log-collector.js} | 2 +- .../src/appsec/iast/telemetry/namespaces.js | 76 ++++ .../src/appsec/iast/telemetry/span-tags.js | 53 +++ .../src/appsec/iast/telemetry/verbosity.js | 42 ++ packages/dd-trace/src/config.js | 16 +- .../analyzers/ldap-injection-analyzer.spec.js | 35 ++ .../analyzers/sql-injection-analyzer.spec.js | 60 +++ .../unvalidated-redirect-analyzer.spec.js | 4 +- .../analyzers/vulnerability-analyzer.spec.js | 95 ++++- .../analyzers/weak-cipher-analyzers.spec.js | 2 + .../iast/analyzers/weak-hash-analyzer.spec.js | 2 + .../test/appsec/iast/iast-log.spec.js | 6 +- .../test/appsec/iast/iast-plugin.spec.js | 382 ++++++++++++++++++ .../appsec/iast/taint-tracking/plugin.spec.js | 2 +- .../taint-tracking/rewriter-telemetry.spec.js | 70 ++++ .../iast/taint-tracking/rewriter.spec.js | 25 +- .../taint-tracking.cookie.plugin.spec.js | 2 +- .../taint-tracking.express.plugin.spec.js | 2 +- .../sources/taint-tracking.headers.spec.js | 2 +- .../taint-tracking-operations.spec.js | 76 +++- .../appsec/iast/telemetry/iast-metric.spec.js | 98 +++++ .../test/appsec/iast/telemetry/index.spec.js | 172 ++++++++ .../{logs.spec.js => log/index.spec.js} | 48 +-- .../log-collector.spec.js} | 10 +- .../appsec/iast/telemetry/namespaces.spec.js | 89 ++++ .../appsec/iast/telemetry/span-tags.spec.js | 78 ++++ .../appsec/iast/telemetry/verbosity.spec.js | 49 +++ packages/dd-trace/test/config.spec.js | 19 +- .../test/telemetry/dependencies.spec.js | 1 + 54 files changed, 2112 insertions(+), 186 deletions(-) create mode 100644 packages/dd-trace/src/appsec/iast/iast-plugin.js create mode 100644 packages/dd-trace/src/appsec/iast/taint-tracking/rewriter-telemetry.js rename packages/dd-trace/src/appsec/iast/taint-tracking/{origin-types.js => source-types.js} (100%) create mode 100644 packages/dd-trace/src/appsec/iast/telemetry/iast-metric.js create mode 100644 packages/dd-trace/src/appsec/iast/telemetry/index.js rename packages/dd-trace/src/appsec/iast/telemetry/{logs.js => log/index.js} (89%) rename packages/dd-trace/src/appsec/iast/telemetry/{log_collector.js => log/log-collector.js} (98%) create mode 100644 packages/dd-trace/src/appsec/iast/telemetry/namespaces.js create mode 100644 packages/dd-trace/src/appsec/iast/telemetry/span-tags.js create mode 100644 packages/dd-trace/src/appsec/iast/telemetry/verbosity.js create mode 100644 packages/dd-trace/test/appsec/iast/iast-plugin.spec.js create mode 100644 packages/dd-trace/test/appsec/iast/taint-tracking/rewriter-telemetry.spec.js create mode 100644 packages/dd-trace/test/appsec/iast/telemetry/iast-metric.spec.js create mode 100644 packages/dd-trace/test/appsec/iast/telemetry/index.spec.js rename packages/dd-trace/test/appsec/iast/telemetry/{logs.spec.js => log/index.spec.js} (77%) rename packages/dd-trace/test/appsec/iast/telemetry/{log_collector.spec.js => log/log-collector.spec.js} (86%) create mode 100644 packages/dd-trace/test/appsec/iast/telemetry/namespaces.spec.js create mode 100644 packages/dd-trace/test/appsec/iast/telemetry/span-tags.spec.js create mode 100644 packages/dd-trace/test/appsec/iast/telemetry/verbosity.spec.js diff --git a/packages/dd-trace/src/appsec/iast/analyzers/command-injection-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/command-injection-analyzer.js index 1984d5014dc..eccf8a3814b 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/command-injection-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/command-injection-analyzer.js @@ -5,6 +5,9 @@ const { COMMAND_INJECTION } = require('../vulnerabilities') class CommandInjectionAnalyzer extends InjectionAnalyzer { constructor () { super(COMMAND_INJECTION) + } + + onConfigure () { this.addSub('datadog:child_process:execution:start', ({ command }) => this.analyze(command)) } } diff --git a/packages/dd-trace/src/appsec/iast/analyzers/cookie-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/cookie-analyzer.js index e8997372df1..413653a7af2 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/cookie-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/cookie-analyzer.js @@ -9,7 +9,13 @@ class CookieAnalyzer extends Analyzer { constructor (type, propertyToBeSafe) { super(type) this.propertyToBeSafe = propertyToBeSafe.toLowerCase() - this.addSub('datadog:iast:set-cookie', (cookieInfo) => this.analyze(cookieInfo)) + } + + onConfigure () { + this.addSub( + { channelName: 'datadog:iast:set-cookie', moduleName: 'http' }, + (cookieInfo) => this.analyze(cookieInfo) + ) } _isVulnerable ({ cookieProperties, cookieValue }) { diff --git a/packages/dd-trace/src/appsec/iast/analyzers/ldap-injection-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/ldap-injection-analyzer.js index 07df22aa7df..41d349e6452 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/ldap-injection-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/ldap-injection-analyzer.js @@ -5,6 +5,9 @@ const { LDAP_INJECTION } = require('../vulnerabilities') class LdapInjectionAnalyzer extends InjectionAnalyzer { constructor () { super(LDAP_INJECTION) + } + + onConfigure () { this.addSub('datadog:ldapjs:client:search', ({ base, filter }) => this.analyzeAll(base, filter)) } } diff --git a/packages/dd-trace/src/appsec/iast/analyzers/path-traversal-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/path-traversal-analyzer.js index a10b3461921..83bf2a87085 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/path-traversal-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/path-traversal-analyzer.js @@ -1,9 +1,10 @@ 'use strict' const path = require('path') + +const InjectionAnalyzer = require('./injection-analyzer') const { getIastContext } = require('../iast-context') const { storage } = require('../../../../../datadog-core') -const InjectionAnalyzer = require('./injection-analyzer') const { PATH_TRAVERSAL } = require('../vulnerabilities') const ignoredOperations = ['dir.close', 'close'] @@ -11,7 +12,23 @@ const ignoredOperations = ['dir.close', 'close'] class PathTraversalAnalyzer extends InjectionAnalyzer { constructor () { super(PATH_TRAVERSAL) - this.addSub('apm:fs:operation:start', obj => { + + this.exclusionList = [ + path.join('node_modules', 'send') + path.sep + ] + + this.internalExclusionList = [ + 'node:fs', + 'node:internal/fs', + 'node:internal\\fs', + 'fs.js', + 'internal/fs', + 'internal\\fs' + ] + } + + onConfigure () { + this.addSub('apm:fs:operation:start', (obj) => { if (ignoredOperations.includes(obj.operation)) return const pathArguments = [] @@ -44,19 +61,6 @@ class PathTraversalAnalyzer extends InjectionAnalyzer { } this.analyze(pathArguments) }) - - this.exclusionList = [ - path.join('node_modules', 'send') + path.sep - ] - - this.internalExclusionList = [ - 'node:fs', - 'node:internal/fs', - 'node:internal\\fs', - 'fs.js', - 'internal/fs', - 'internal\\fs' - ] } _isExcluded (location) { diff --git a/packages/dd-trace/src/appsec/iast/analyzers/sql-injection-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/sql-injection-analyzer.js index f2ba289e305..3838513efbf 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/sql-injection-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/sql-injection-analyzer.js @@ -13,6 +13,9 @@ const EXCLUDED_PATHS = getNodeModulesPaths('mysql2', 'sequelize') class SqlInjectionAnalyzer extends InjectionAnalyzer { constructor () { super(SQL_INJECTION) + } + + onConfigure () { this.addSub('apm:mysql:query:start', ({ sql }) => this.analyze(sql, 'MYSQL')) this.addSub('apm:mysql2:query:start', ({ sql }) => this.analyze(sql, 'MYSQL')) this.addSub('apm:pg:query:start', ({ query }) => this.analyze(query.text, 'POSTGRES')) @@ -41,10 +44,9 @@ class SqlInjectionAnalyzer extends InjectionAnalyzer { analyze (value, dialect) { const store = storage.getStore() - if (!(store && store.sqlAnalyzed)) { const iastContext = getIastContext(store) - if (store && !iastContext) return + if (this._isInvalidContext(store, iastContext)) return this._reportIfVulnerable(value, iastContext, dialect) } } @@ -66,6 +68,7 @@ class SqlInjectionAnalyzer extends InjectionAnalyzer { addVulnerability(context, vulnerability) } } + _getExcludedPaths () { return EXCLUDED_PATHS } diff --git a/packages/dd-trace/src/appsec/iast/analyzers/ssrf-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/ssrf-analyzer.js index 87263369941..0164956db54 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/ssrf-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/ssrf-analyzer.js @@ -6,7 +6,9 @@ const { SSRF } = require('../vulnerabilities') class SSRFAnalyzer extends InjectionAnalyzer { constructor () { super(SSRF) + } + onConfigure () { this.addSub('apm:http:client:request:start', ({ args }) => { if (typeof args.originalUrl === 'string') { this.analyze(args.originalUrl) diff --git a/packages/dd-trace/src/appsec/iast/analyzers/unvalidated-redirect-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/unvalidated-redirect-analyzer.js index 25f919844b1..5324546802d 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/unvalidated-redirect-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/unvalidated-redirect-analyzer.js @@ -4,14 +4,16 @@ const InjectionAnalyzer = require('./injection-analyzer') const { UNVALIDATED_REDIRECT } = require('../vulnerabilities') const { getNodeModulesPaths } = require('../path-line') const { getRanges } = require('../taint-tracking/operations') -const { HTTP_REQUEST_HEADER_VALUE } = require('../taint-tracking/origin-types') +const { HTTP_REQUEST_HEADER_VALUE } = require('../taint-tracking/source-types') const EXCLUDED_PATHS = getNodeModulesPaths('express/lib/response.js') class UnvalidatedRedirectAnalyzer extends InjectionAnalyzer { constructor () { super(UNVALIDATED_REDIRECT) + } + onConfigure () { this.addSub('datadog:http:server:response:set-header:finish', ({ name, value }) => this.analyze(name, value)) } diff --git a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js index 890cd19bf8d..b9dc62643a1 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js @@ -1,33 +1,18 @@ 'use strict' -const Plugin = require('../../../../src/plugins/plugin') const { storage } = require('../../../../../datadog-core') -const iastLog = require('../iast-log') const { getFirstNonDDPathAndLine } = require('../path-line') const { addVulnerability } = require('../vulnerability-reporter') const { getIastContext } = require('../iast-context') const overheadController = require('../overhead-controller') +const { SinkIastPlugin } = require('../iast-plugin') -class Analyzer extends Plugin { +class Analyzer extends SinkIastPlugin { constructor (type) { super() this._type = type } - _wrapHandler (handler) { - return (message, name) => { - try { - handler(message, name) - } catch (e) { - iastLog.errorAndPublish(e) - } - } - } - - addSub (channelName, handler) { - super.addSub(channelName, this._wrapHandler(handler)) - } - _isVulnerable (value, context) { return false } @@ -64,17 +49,23 @@ class Analyzer extends Plugin { _getExcludedPaths () {} + _isInvalidContext (store, iastContext) { + return store && !iastContext + } + analyze (value) { const store = storage.getStore() const iastContext = getIastContext(store) - if (store && !iastContext) return + if (this._isInvalidContext(store, iastContext)) return + this._reportIfVulnerable(value, iastContext) } analyzeAll (...values) { const store = storage.getStore() const iastContext = getIastContext(store) - if (store && !iastContext) return + if (this._isInvalidContext(store, iastContext)) return + for (let i = 0; i < values.length; i++) { const value = values[i] if (this._isVulnerable(value, iastContext)) { @@ -119,6 +110,14 @@ class Analyzer extends Plugin { } return hash } + + addSub (iastSubOrChannelName, handler) { + const iastSub = typeof iastSubOrChannelName === 'string' + ? { channelName: iastSubOrChannelName } + : iastSubOrChannelName + + super.addSub({ tag: this._type, ...iastSub }, handler) + } } module.exports = Analyzer diff --git a/packages/dd-trace/src/appsec/iast/analyzers/weak-cipher-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/weak-cipher-analyzer.js index 948c5393ee7..7a5542cc9a5 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/weak-cipher-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/weak-cipher-analyzer.js @@ -14,6 +14,9 @@ const INSECURE_CIPHERS = new Set([ class WeakCipherAnalyzer extends Analyzer { constructor () { super(WEAK_CIPHER) + } + + onConfigure () { this.addSub('datadog:crypto:cipher:start', ({ algorithm }) => this.analyze(algorithm)) } diff --git a/packages/dd-trace/src/appsec/iast/analyzers/weak-hash-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/weak-hash-analyzer.js index 02068979280..4b38913caea 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/weak-hash-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/weak-hash-analyzer.js @@ -25,7 +25,11 @@ const EXCLUDED_PATHS_FROM_STACK = [ class WeakHashAnalyzer extends Analyzer { constructor () { super(WEAK_HASH) - this.addSub('datadog:crypto:hashing:start', ({ algorithm }) => this.analyze(algorithm)) + } + + onConfigure () { + this.addSub('datadog:crypto:hashing:start', ({ algorithm }) => this.analyze(algorithm) + ) } _isVulnerable (algorithm) { diff --git a/packages/dd-trace/src/appsec/iast/iast-log.js b/packages/dd-trace/src/appsec/iast/iast-log.js index 441cc6d7000..6d6e2171de4 100644 --- a/packages/dd-trace/src/appsec/iast/iast-log.js +++ b/packages/dd-trace/src/appsec/iast/iast-log.js @@ -1,7 +1,7 @@ 'use strict' const log = require('../../log') -const telemetryLogs = require('./telemetry/logs') +const telemetryLogs = require('./telemetry/log') const { calculateDDBasePath } = require('../../util') const ddBasePath = calculateDDBasePath(__dirname) diff --git a/packages/dd-trace/src/appsec/iast/iast-plugin.js b/packages/dd-trace/src/appsec/iast/iast-plugin.js new file mode 100644 index 00000000000..6b23440d984 --- /dev/null +++ b/packages/dd-trace/src/appsec/iast/iast-plugin.js @@ -0,0 +1,202 @@ +'use strict' + +const { channel } = require('../../../../diagnostics_channel') + +const iastLog = require('./iast-log') +const Plugin = require('../../plugins/plugin') +const iastTelemetry = require('./telemetry') +const { getInstrumentedMetric, getExecutedMetric, TagKey, EXECUTED_SOURCE } = require('./telemetry/iast-metric') +const { storage } = require('../../../../datadog-core') +const { getIastContext } = require('./iast-context') +const instrumentations = require('../../../../datadog-instrumentations/src/helpers/instrumentations') + +/** + * Used by vulnerability sources and sinks to subscribe diagnostic channel events + * and indicate what kind of metrics the subscription provides + * - moduleName is used identify when a module is loaded and + * to increment the INSTRUMENTED_[SINK|SOURCE] metric when it occurs + * - channelName is the channel used by the hook to publish execution events + * - tag indicates the name of the metric: taint-tracking/source-types for Sources and analyzers type for Sinks + * - tagKey can be only SOURCE_TYPE (Source) or VULNERABILITY_TYPE (Sink) + */ +class IastPluginSubscription { + constructor (moduleName, channelName, tag, tagKey = TagKey.VULNERABILITY_TYPE) { + this.moduleName = moduleName + this.channelName = channelName + this.tag = tag + this.tagKey = tagKey + this.executedMetric = getExecutedMetric(this.tagKey) + this.instrumentedMetric = getInstrumentedMetric(this.tagKey) + this.moduleInstrumented = false + } + + increaseInstrumented () { + if (this.moduleInstrumented) return + + this.moduleInstrumented = true + this.instrumentedMetric.inc(this.tag) + } + + increaseExecuted (iastContext) { + this.executedMetric.inc(this.tag, iastContext) + } + + matchesModuleInstrumented (name) { + // https module is a special case because it's events are published as http + name = name === 'https' ? 'http' : name + return this.moduleName === name + } +} + +class IastPlugin extends Plugin { + constructor () { + super() + this.configured = false + this.pluginSubs = [] + } + + _wrapHandler (handler) { + return (message, name) => { + try { + handler(message, name) + } catch (e) { + iastLog.errorAndPublish(e) + } + } + } + + _getTelemetryHandler (iastSub) { + return () => { + try { + const iastContext = getIastContext(storage.getStore()) + iastSub.increaseExecuted(iastContext) + } catch (e) { + iastLog.errorAndPublish(e) + } + } + } + + _execHandlerAndIncMetric ({ handler, metric, tag, iastContext = getIastContext(storage.getStore()) }) { + try { + const result = handler() + iastTelemetry.isEnabled() && metric.inc(tag, iastContext) + return result + } catch (e) { + iastLog.errorAndPublish(e) + } + } + + addSub (iastSub, handler) { + if (typeof iastSub === 'string') { + super.addSub(iastSub, this._wrapHandler(handler)) + } else { + iastSub = this._getAndRegisterSubscription(iastSub) + if (iastSub) { + super.addSub(iastSub.channelName, this._wrapHandler(handler)) + + if (iastTelemetry.isEnabled()) { + super.addSub(iastSub.channelName, this._getTelemetryHandler(iastSub)) + } + } + } + } + + onConfigure () {} + + configure (config) { + if (config && !this.configured) { + this.onConfigure() + this.configured = true + } + + if (iastTelemetry.isEnabled()) { + if (config) { + this.enableTelemetry() + } else { + this.disableTelemetry() + } + } + + super.configure(config) + } + + _getAndRegisterSubscription ({ moduleName, channelName, tag, tagKey }) { + if (!channelName && !moduleName) return + + if (!moduleName) { + const firstSep = channelName.indexOf(':') + if (firstSep === -1) { + moduleName = channelName + } else { + const lastSep = channelName.indexOf(':', firstSep + 1) + moduleName = channelName.substring(firstSep + 1, lastSep !== -1 ? lastSep : channelName.length) + } + } + + const iastSub = new IastPluginSubscription(moduleName, channelName, tag, tagKey) + this.pluginSubs.push(iastSub) + return iastSub + } + + enableTelemetry () { + if (this.onInstrumentationLoadedListener) return + + this.onInstrumentationLoadedListener = ({ name }) => this._onInstrumentationLoaded(name) + const loadChannel = channel('dd-trace:instrumentation:load') + loadChannel.subscribe(this.onInstrumentationLoadedListener) + + // check for already instrumented modules + for (const name in instrumentations) { + this._onInstrumentationLoaded(name) + } + } + + disableTelemetry () { + if (!this.onInstrumentationLoadedListener) return + + const loadChannel = channel('dd-trace:instrumentation:load') + if (loadChannel.hasSubscribers) { + loadChannel.unsubscribe(this.onInstrumentationLoadedListener) + } + this.onInstrumentationLoadedListener = null + } + + _onInstrumentationLoaded (name) { + this.pluginSubs + .filter(sub => sub.matchesModuleInstrumented(name)) + .forEach(sub => sub.increaseInstrumented()) + } +} + +class SourceIastPlugin extends IastPlugin { + addSub (iastPluginSub, handler) { + return super.addSub({ tagKey: TagKey.SOURCE_TYPE, ...iastPluginSub }, handler) + } + + addInstrumentedSource (moduleName, tag) { + this._getAndRegisterSubscription({ + moduleName, + tag, + tagKey: TagKey.SOURCE_TYPE + }) + } + + execSource (sourceHandlerInfo) { + this._execHandlerAndIncMetric({ + metric: EXECUTED_SOURCE, + ...sourceHandlerInfo + }) + } +} + +class SinkIastPlugin extends IastPlugin { + addSub (iastPluginSub, handler) { + return super.addSub({ tagKey: TagKey.VULNERABILITY_TYPE, ...iastPluginSub }, handler) + } +} + +module.exports = { + SourceIastPlugin, + SinkIastPlugin, + IastPlugin +} diff --git a/packages/dd-trace/src/appsec/iast/index.js b/packages/dd-trace/src/appsec/iast/index.js index 205f16a349d..944c0b4ec07 100644 --- a/packages/dd-trace/src/appsec/iast/index.js +++ b/packages/dd-trace/src/appsec/iast/index.js @@ -13,8 +13,7 @@ const { taintTrackingPlugin } = require('./taint-tracking') const { IAST_ENABLED_TAG_KEY } = require('./tags') - -const telemetryLogs = require('./telemetry/logs') +const iastTelemetry = require('./telemetry') // TODO Change to `apm:http:server:request:[start|close]` when the subscription // order of the callbacks can be enforce @@ -22,24 +21,24 @@ const requestStart = dc.channel('dd-trace:incomingHttpRequestStart') const requestClose = dc.channel('dd-trace:incomingHttpRequestEnd') function enable (config, _tracer) { + iastTelemetry.configure(config, config.iast && config.iast.telemetryVerbosity) enableAllAnalyzers() - enableTaintTracking(config.iast) + enableTaintTracking(config.iast, iastTelemetry.verbosity) requestStart.subscribe(onIncomingHttpRequestStart) requestClose.subscribe(onIncomingHttpRequestEnd) overheadController.configure(config.iast) overheadController.startGlobalContext() vulnerabilityReporter.start(config, _tracer) - telemetryLogs.start() } function disable () { + iastTelemetry.stop() disableAllAnalyzers() disableTaintTracking() overheadController.finishGlobalContext() if (requestStart.hasSubscribers) requestStart.unsubscribe(onIncomingHttpRequestStart) if (requestClose.hasSubscribers) requestClose.unsubscribe(onIncomingHttpRequestEnd) vulnerabilityReporter.stop() - telemetryLogs.stop() } function onIncomingHttpRequestStart (data) { @@ -55,6 +54,7 @@ function onIncomingHttpRequestStart (data) { createTransaction(rootSpan.context().toSpanId(), iastContext) overheadController.initializeRequestContext(iastContext) taintTrackingPlugin.taintHeaders(data.req.headers, iastContext) + iastTelemetry.onRequestStarted(iastContext) } if (rootSpan.addTags) { rootSpan.addTags({ @@ -76,6 +76,7 @@ function onIncomingHttpRequestEnd (data) { const rootSpan = iastContext.rootSpan vulnerabilityReporter.sendVulnerabilities(vulnerabilities, rootSpan) removeTransaction(iastContext) + iastTelemetry.onRequestEnd(iastContext, iastContext.rootSpan) } // TODO web.getContext(data.req) is required when the request is aborted if (iastContextFunctions.cleanIastContext(store, topContext, iastContext)) { diff --git a/packages/dd-trace/src/appsec/iast/tags.js b/packages/dd-trace/src/appsec/iast/tags.js index d61d5727dc1..4f14e8958d7 100644 --- a/packages/dd-trace/src/appsec/iast/tags.js +++ b/packages/dd-trace/src/appsec/iast/tags.js @@ -2,5 +2,6 @@ module.exports = { IAST_ENABLED_TAG_KEY: '_dd.iast.enabled', - IAST_JSON_TAG_KEY: '_dd.iast.json' + IAST_JSON_TAG_KEY: '_dd.iast.json', + IAST_TRACE_METRIC_PREFIX: '_dd.iast.telemetry' } diff --git a/packages/dd-trace/src/appsec/iast/taint-tracking/csi-methods.js b/packages/dd-trace/src/appsec/iast/taint-tracking/csi-methods.js index e064f663dc7..5dd403d71dc 100644 --- a/packages/dd-trace/src/appsec/iast/taint-tracking/csi-methods.js +++ b/packages/dd-trace/src/appsec/iast/taint-tracking/csi-methods.js @@ -2,16 +2,29 @@ const csiMethods = [ { src: 'plusOperator', operator: true }, - { src: 'trim' }, - { src: 'trimStart', dst: 'trim' }, - { src: 'trimEnd' }, { src: 'concat' }, - { src: 'substring' }, - { src: 'substr' }, + { src: 'replace' }, { src: 'slice' }, - { src: 'replace' } + { src: 'substr' }, + { src: 'substring' }, + { src: 'trim' }, + { src: 'trimEnd' }, + { src: 'trimStart', dst: 'trim' } ] +function getExpectedMethods () { + const set = new Set() + for (const definition of csiMethods) { + if (definition.dst) { + set.add(definition.dst) + } else { + set.add(definition.src) + } + } + return [...set] +} + module.exports = { - csiMethods + csiMethods, + getExpectedMethods } diff --git a/packages/dd-trace/src/appsec/iast/taint-tracking/index.js b/packages/dd-trace/src/appsec/iast/taint-tracking/index.js index 137b5c4fe60..37747be4bb3 100644 --- a/packages/dd-trace/src/appsec/iast/taint-tracking/index.js +++ b/packages/dd-trace/src/appsec/iast/taint-tracking/index.js @@ -10,9 +10,9 @@ const { createTransaction, const taintTrackingPlugin = require('./plugin') module.exports = { - enableTaintTracking (config) { - enableRewriter() - enableTaintOperations() + enableTaintTracking (config, telemetryVerbosity) { + enableRewriter(telemetryVerbosity) + enableTaintOperations(telemetryVerbosity) taintTrackingPlugin.enable() setMaxTransactions(config.maxConcurrentRequests) }, diff --git a/packages/dd-trace/src/appsec/iast/taint-tracking/operations.js b/packages/dd-trace/src/appsec/iast/taint-tracking/operations.js index 7724c4edbda..ccc3d7ed7d6 100644 --- a/packages/dd-trace/src/appsec/iast/taint-tracking/operations.js +++ b/packages/dd-trace/src/appsec/iast/taint-tracking/operations.js @@ -3,7 +3,10 @@ const TaintedUtils = require('@datadog/native-iast-taint-tracking') const { IAST_TRANSACTION_ID } = require('../iast-context') const iastLog = require('../iast-log') -const { TaintTracking, TaintTrackingDummy } = require('./taint-tracking-impl') +const iastTelemetry = require('../telemetry') +const { REQUEST_TAINTED } = require('../telemetry/iast-metric') +const { isInfoAllowed } = require('../telemetry/verbosity') +const { getTaintTrackingImpl, getTaintTrackingNoop } = require('./taint-tracking-impl') function createTransaction (id, iastContext) { if (id && iastContext) { @@ -11,9 +14,21 @@ function createTransaction (id, iastContext) { } } +let onRemoveTransaction = (transactionId, iastContext) => {} + +function onRemoveTransactionInformationTelemetry (transactionId, iastContext) { + const metrics = TaintedUtils.getMetrics(transactionId, iastTelemetry.verbosity) + if (metrics && metrics.requestCount) { + REQUEST_TAINTED.add(metrics.requestCount, null, iastContext) + } +} + function removeTransaction (iastContext) { if (iastContext && iastContext[IAST_TRANSACTION_ID]) { const transactionId = iastContext[IAST_TRANSACTION_ID] + + onRemoveTransaction(transactionId, iastContext) + TaintedUtils.removeTransaction(transactionId) delete iastContext[IAST_TRANSACTION_ID] } @@ -96,12 +111,16 @@ function getRanges (iastContext, string) { return result } -function enableTaintOperations () { - global._ddiast = TaintTracking +function enableTaintOperations (telemetryVerbosity) { + if (isInfoAllowed(telemetryVerbosity)) { + onRemoveTransaction = onRemoveTransactionInformationTelemetry + } + + global._ddiast = getTaintTrackingImpl(telemetryVerbosity) } function disableTaintOperations () { - global._ddiast = TaintTrackingDummy + global._ddiast = getTaintTrackingNoop() } function setMaxTransactions (transactions) { diff --git a/packages/dd-trace/src/appsec/iast/taint-tracking/plugin.js b/packages/dd-trace/src/appsec/iast/taint-tracking/plugin.js index a16f2a83d6c..0ff184867f6 100644 --- a/packages/dd-trace/src/appsec/iast/taint-tracking/plugin.js +++ b/packages/dd-trace/src/appsec/iast/taint-tracking/plugin.js @@ -1,6 +1,6 @@ 'use strict' -const Plugin = require('../../../plugins/plugin') +const { SourceIastPlugin } = require('../iast-plugin') const { getIastContext } = require('../iast-context') const { storage } = require('../../../../../datadog-core') const { taintObject } = require('./operations') @@ -12,15 +12,17 @@ const { HTTP_REQUEST_HEADER_NAME, HTTP_REQUEST_PARAMETER, HTTP_REQUEST_PATH_PARAM +} = require('./source-types') -} = require('./origin-types') - -class TaintTrackingPlugin extends Plugin { +class TaintTrackingPlugin extends SourceIastPlugin { constructor () { super() this._type = 'taint-tracking' + } + + onConfigure () { this.addSub( - 'datadog:body-parser:read:finish', + { channelName: 'datadog:body-parser:read:finish', tag: HTTP_REQUEST_BODY }, ({ req }) => { const iastContext = getIastContext(storage.getStore()) if (iastContext && iastContext['body'] !== req.body) { @@ -29,31 +31,41 @@ class TaintTrackingPlugin extends Plugin { } } ) + this.addSub( - 'datadog:qs:parse:finish', + { channelName: 'datadog:qs:parse:finish', tag: HTTP_REQUEST_PARAMETER }, ({ qs }) => this._taintTrackingHandler(HTTP_REQUEST_PARAMETER, qs) ) - this.addSub('apm:express:middleware:next', ({ req }) => { - if (req && req.body && typeof req.body === 'object') { - const iastContext = getIastContext(storage.getStore()) - if (iastContext && iastContext['body'] !== req.body) { - this._taintTrackingHandler(HTTP_REQUEST_BODY, req, 'body', iastContext) - iastContext['body'] = req.body + + this.addSub( + { channelName: 'apm:express:middleware:next', tag: HTTP_REQUEST_BODY }, + ({ req }) => { + if (req && req.body && typeof req.body === 'object') { + const iastContext = getIastContext(storage.getStore()) + if (iastContext && iastContext['body'] !== req.body) { + this._taintTrackingHandler(HTTP_REQUEST_BODY, req, 'body', iastContext) + iastContext['body'] = req.body + } } } - }) + ) + this.addSub( - 'datadog:cookie:parse:finish', + { channelName: 'datadog:cookie:parse:finish', tag: [HTTP_REQUEST_COOKIE_VALUE, HTTP_REQUEST_COOKIE_NAME] }, ({ cookies }) => this._cookiesTaintTrackingHandler(cookies) ) + this.addSub( - 'datadog:express:process_params:start', + { channelName: 'datadog:express:process_params:start', tag: HTTP_REQUEST_PATH_PARAM }, ({ req }) => { if (req && req.params && typeof req.params === 'object') { this._taintTrackingHandler(HTTP_REQUEST_PATH_PARAM, req, 'params') } } ) + + // this is a special case to increment INSTRUMENTED_SOURCE metric for header + this.addInstrumentedSource('http', [HTTP_REQUEST_HEADER_VALUE, HTTP_REQUEST_HEADER_NAME]) } _taintTrackingHandler (type, target, property, iastContext = getIastContext(storage.getStore())) { @@ -70,7 +82,11 @@ class TaintTrackingPlugin extends Plugin { } taintHeaders (headers, iastContext) { - taintObject(iastContext, headers, HTTP_REQUEST_HEADER_VALUE, true, HTTP_REQUEST_HEADER_NAME) + this.execSource({ + handler: () => taintObject(iastContext, headers, HTTP_REQUEST_HEADER_VALUE, true, HTTP_REQUEST_HEADER_NAME), + tag: [HTTP_REQUEST_HEADER_VALUE, HTTP_REQUEST_HEADER_NAME], + iastContext + }) } enable () { diff --git a/packages/dd-trace/src/appsec/iast/taint-tracking/rewriter-telemetry.js b/packages/dd-trace/src/appsec/iast/taint-tracking/rewriter-telemetry.js new file mode 100644 index 00000000000..6643fc85826 --- /dev/null +++ b/packages/dd-trace/src/appsec/iast/taint-tracking/rewriter-telemetry.js @@ -0,0 +1,33 @@ +'use strict' + +const iastTelemetry = require('../telemetry') +const { Verbosity } = require('../telemetry/verbosity') +const { INSTRUMENTED_PROPAGATION } = require('../telemetry/iast-metric') + +const telemetryRewriter = { + off (content, filename, rewriter) { + return rewriter.rewrite(content, filename) + }, + + information (content, filename, rewriter) { + const response = this.off(content, filename, rewriter) + + const metrics = response.metrics + if (metrics && metrics.instrumentedPropagation) { + INSTRUMENTED_PROPAGATION.add(metrics.instrumentedPropagation) + } + + return response + } +} + +function getRewriteFunction (rewriter) { + switch (iastTelemetry.verbosity) { + case Verbosity.OFF: + return (content, filename) => telemetryRewriter.off(content, filename, rewriter) + default: + return (content, filename) => telemetryRewriter.information(content, filename, rewriter) + } +} + +module.exports = { getRewriteFunction } diff --git a/packages/dd-trace/src/appsec/iast/taint-tracking/rewriter.js b/packages/dd-trace/src/appsec/iast/taint-tracking/rewriter.js index 21b2d4f12fb..ae95ec68139 100644 --- a/packages/dd-trace/src/appsec/iast/taint-tracking/rewriter.js +++ b/packages/dd-trace/src/appsec/iast/taint-tracking/rewriter.js @@ -5,20 +5,17 @@ const shimmer = require('../../../../../datadog-shimmer') const iastLog = require('../iast-log') const { isPrivateModule, isNotLibraryFile } = require('./filter') const { csiMethods } = require('./csi-methods') +const { getName } = require('../telemetry/verbosity') +const { getRewriteFunction } = require('./rewriter-telemetry') let rewriter let getPrepareStackTrace -function getRewriter () { +function getRewriter (telemetryVerbosity) { if (!rewriter) { - try { - const iastRewriter = require('@datadog/native-iast-rewriter') - const Rewriter = iastRewriter.Rewriter - getPrepareStackTrace = iastRewriter.getPrepareStackTrace - rewriter = new Rewriter({ csiMethods }) - } catch (e) { - iastLog.error('Unable to initialize TaintTracking Rewriter') - .errorAndPublish(e) - } + const iastRewriter = require('@datadog/native-iast-rewriter') + const Rewriter = iastRewriter.Rewriter + getPrepareStackTrace = iastRewriter.getPrepareStackTrace + rewriter = new Rewriter({ csiMethods, telemetryVerbosity: getName(telemetryVerbosity) }) } return rewriter } @@ -27,6 +24,7 @@ let originalPrepareStackTrace = Error.prepareStackTrace function getPrepareStackTraceAccessor () { let actual = getPrepareStackTrace(originalPrepareStackTrace) return { + configurable: true, get () { return actual }, @@ -38,10 +36,11 @@ function getPrepareStackTraceAccessor () { } function getCompileMethodFn (compileMethod) { + const rewriteFn = getRewriteFunction(rewriter) return function (content, filename) { try { if (isPrivateModule(filename) && isNotLibraryFile(filename)) { - const rewritten = rewriter.rewrite(content, filename) + const rewritten = rewriteFn(content, filename) if (rewritten && rewritten.content) { return compileMethod.apply(this, [rewritten.content, filename]) } @@ -54,11 +53,19 @@ function getCompileMethodFn (compileMethod) { } } -function enableRewriter () { - const rewriter = getRewriter() - if (rewriter) { - Object.defineProperty(global.Error, 'prepareStackTrace', getPrepareStackTraceAccessor()) - shimmer.wrap(Module.prototype, '_compile', compileMethod => getCompileMethodFn(compileMethod)) +function enableRewriter (telemetryVerbosity) { + try { + const rewriter = getRewriter(telemetryVerbosity) + if (rewriter) { + const pstDescriptor = Object.getOwnPropertyDescriptor(global.Error, 'prepareStackTrace') + if (!pstDescriptor || pstDescriptor.configurable) { + Object.defineProperty(global.Error, 'prepareStackTrace', getPrepareStackTraceAccessor()) + } + shimmer.wrap(Module.prototype, '_compile', compileMethod => getCompileMethodFn(compileMethod)) + } + } catch (e) { + iastLog.error('Error enabling TaintTracking Rewriter') + .errorAndPublish(e) } } diff --git a/packages/dd-trace/src/appsec/iast/taint-tracking/origin-types.js b/packages/dd-trace/src/appsec/iast/taint-tracking/source-types.js similarity index 100% rename from packages/dd-trace/src/appsec/iast/taint-tracking/origin-types.js rename to packages/dd-trace/src/appsec/iast/taint-tracking/source-types.js diff --git a/packages/dd-trace/src/appsec/iast/taint-tracking/taint-tracking-impl.js b/packages/dd-trace/src/appsec/iast/taint-tracking/taint-tracking-impl.js index 1136b6b1895..0ea3f41af78 100644 --- a/packages/dd-trace/src/appsec/iast/taint-tracking/taint-tracking-impl.js +++ b/packages/dd-trace/src/appsec/iast/taint-tracking/taint-tracking-impl.js @@ -4,30 +4,45 @@ const TaintedUtils = require('@datadog/native-iast-taint-tracking') const { storage } = require('../../../../../datadog-core') const iastContextFunctions = require('../iast-context') const iastLog = require('../iast-log') +const { EXECUTED_PROPAGATION } = require('../telemetry/iast-metric') +const { isDebugAllowed } = require('../telemetry/verbosity') function noop (res) { return res } -const TaintTrackingDummy = { +// NOTE: methods of this object must be synchronized with csi-methods.js file definitions! +// Otherwise you may end up rewriting a method and not providing its rewritten implementation +const TaintTrackingNoop = { plusOperator: noop, - trim: noop, - trimEnd: noop, concat: noop, - substring: noop, - substr: noop, + replace: noop, slice: noop, - replace: noop + substr: noop, + substring: noop, + trim: noop, + trimEnd: noop } -function getTransactionId () { - const store = storage.getStore() - const iastContext = iastContextFunctions.getIastContext(store) +function getTransactionId (iastContext) { return iastContext && iastContext[iastContextFunctions.IAST_TRANSACTION_ID] } -function getFilteredCsiFn (cb, filter) { +function getContextDefault () { + const store = storage.getStore() + return iastContextFunctions.getIastContext(store) +} + +function getContextDebug () { + const iastContext = getContextDefault() + EXECUTED_PROPAGATION.inc(null, iastContext) + return iastContext +} + +function getFilteredCsiFn (cb, filter, getContext) { return function csiCall (res, fn, target, ...rest) { try { if (filter(res, fn, target)) { return res } - const transactionId = getTransactionId() + + const context = getContext() + const transactionId = getTransactionId(context) if (transactionId) { return cb(transactionId, res, target, ...rest) } @@ -47,7 +62,7 @@ function isValidCsiMethod (fn, protos) { return protos.some(proto => fn === proto) } -function getCsiFn (cb, ...protos) { +function getCsiFn (cb, getContext, ...protos) { let filter if (!protos || protos.length === 0) { filter = (res, fn, target) => notString(res, target) @@ -57,49 +72,73 @@ function getCsiFn (cb, ...protos) { } else { filter = (res, fn, target) => notString(res, target) || !isValidCsiMethod(fn, protos) } - return getFilteredCsiFn(cb, filter) + return getFilteredCsiFn(cb, filter, getContext) } -function csiMethodsDefaults (names, excluded) { +function csiMethodsDefaults (names, excluded, getContext) { const impl = {} names.forEach(name => { if (excluded.indexOf(name) !== -1) return impl[name] = getCsiFn( (transactionId, res, target, ...rest) => TaintedUtils[name](transactionId, res, target, ...rest), + getContext, String.prototype[name] ) }) return impl } -const csiMethodsOverrides = { - plusOperator: function (res, op1, op2) { - try { - if (notString(res) || (notString(op1) && notString(op2))) { return res } - const transactionId = getTransactionId() - if (transactionId) { - return TaintedUtils.concat(transactionId, res, op1, op2) +function csiMethodsOverrides (getContext) { + return { + plusOperator: function (res, op1, op2) { + try { + if (notString(res) || (notString(op1) && notString(op2))) { return res } + const iastContext = getContext() + const transactionId = getTransactionId(iastContext) + if (transactionId) { + return TaintedUtils.concat(transactionId, res, op1, op2) + } + } catch (e) { + iastLog.error(`Error invoking CSI plusOperator`) + .errorAndPublish(e) } - } catch (e) { - iastLog.error(`Error invoking CSI plusOperator`) - .errorAndPublish(e) - } - return res - }, + return res + }, + + trim: getCsiFn( + (transactionId, res, target) => TaintedUtils.trim(transactionId, res, target), + getContext, + String.prototype.trim, + String.prototype.trimStart + ) + } +} + +function createImplWith (getContext) { + const methodNames = Object.keys(TaintTrackingNoop) + const overrides = csiMethodsOverrides(getContext) + + // impls could be cached but at the moment there is only one invocation to getTaintTrackingImpl + return { + ...csiMethodsDefaults(methodNames, Object.keys(overrides), getContext), + ...overrides + } +} + +function getTaintTrackingImpl (telemetryVerbosity, dummy = false) { + if (dummy) return TaintTrackingNoop - trim: getCsiFn( - (transactionId, res, target) => TaintedUtils.trim(transactionId, res, target), - String.prototype.trim, - String.prototype.trimStart - ) + // with Verbosity.DEBUG every invocation of a TaintedUtils method increases the EXECUTED_PROPAGATION metric + return isDebugAllowed(telemetryVerbosity) + ? createImplWith(getContextDebug) + : createImplWith(getContextDefault) } -const TaintTracking = { - ...csiMethodsDefaults(Object.keys(TaintTrackingDummy), Object.keys(csiMethodsOverrides)), - ...csiMethodsOverrides +function getTaintTrackingNoop () { + return getTaintTrackingImpl(null, true) } module.exports = { - TaintTracking, - TaintTrackingDummy + getTaintTrackingImpl, + getTaintTrackingNoop } diff --git a/packages/dd-trace/src/appsec/iast/telemetry/iast-metric.js b/packages/dd-trace/src/appsec/iast/telemetry/iast-metric.js new file mode 100644 index 00000000000..675a7a7237b --- /dev/null +++ b/packages/dd-trace/src/appsec/iast/telemetry/iast-metric.js @@ -0,0 +1,101 @@ +'use strict' + +const { getNamespaceFromContext, globalNamespace } = require('./namespaces') + +const Scope = { + GLOBAL: 'GLOBAL', + REQUEST: 'REQUEST' +} + +const PropagationType = { + STRING: 'STRING', + JSON: 'JSON', + URL: 'URL' +} + +const TagKey = { + VULNERABILITY_TYPE: 'vulnerability_type', + SOURCE_TYPE: 'source_type', + PROPAGATION_TYPE: 'propagation_type' +} + +class IastMetric { + constructor (name, scope, tagKey) { + this.name = name + this.scope = scope + this.tagKey = tagKey + } + + getNamespace (context) { + return getNamespaceFromContext(context) || globalNamespace + } + + getTag (tagValue) { + return tagValue ? { [this.tagKey]: tagValue } : undefined + } + + addValue (value, tagValue, context) { + this.getNamespace(context) + .count(this.name, this.getTag(tagValue)) + .inc(value) + } + + add (value, tagValue, context) { + if (Array.isArray(tagValue)) { + tagValue.forEach(tag => this.addValue(value, tag, context)) + } else { + this.addValue(value, tagValue, context) + } + } + + inc (tagValue, context) { + this.add(1, tagValue, context) + } +} + +function getExecutedMetric (tagKey) { + return tagKey === TagKey.VULNERABILITY_TYPE ? EXECUTED_SINK : EXECUTED_SOURCE +} + +function getInstrumentedMetric (tagKey) { + return tagKey === TagKey.VULNERABILITY_TYPE ? INSTRUMENTED_SINK : INSTRUMENTED_SOURCE +} + +const INSTRUMENTED_PROPAGATION = new IastMetric('instrumented.propagation', Scope.GLOBAL) +const INSTRUMENTED_SOURCE = new IastMetric('instrumented.source', Scope.GLOBAL, TagKey.SOURCE_TYPE) +const INSTRUMENTED_SINK = new IastMetric('instrumented.sink', Scope.GLOBAL, TagKey.VULNERABILITY_TYPE) + +const EXECUTED_SOURCE = new IastMetric('executed.source', Scope.REQUEST, TagKey.SOURCE_TYPE) +const EXECUTED_SINK = new IastMetric('executed.sink', Scope.REQUEST, TagKey.VULNERABILITY_TYPE) + +const REQUEST_TAINTED = new IastMetric('request.tainted', Scope.REQUEST) + +// DEBUG using metrics +const EXECUTED_PROPAGATION = new IastMetric('executed.propagation', Scope.REQUEST) +const EXECUTED_TAINTED = new IastMetric('executed.tainted', Scope.REQUEST) + +// DEBUG using distribution endpoint +const INSTRUMENTATION_TIME = new IastMetric('instrumentation.time', Scope.GLOBAL) + +module.exports = { + INSTRUMENTED_PROPAGATION, + INSTRUMENTED_SOURCE, + INSTRUMENTED_SINK, + + EXECUTED_PROPAGATION, + EXECUTED_SOURCE, + EXECUTED_SINK, + EXECUTED_TAINTED, + + REQUEST_TAINTED, + + INSTRUMENTATION_TIME, + + PropagationType, + TagKey, + + IastMetric, + + getExecutedMetric, + getInstrumentedMetric +} diff --git a/packages/dd-trace/src/appsec/iast/telemetry/index.js b/packages/dd-trace/src/appsec/iast/telemetry/index.js new file mode 100644 index 00000000000..83dd274d043 --- /dev/null +++ b/packages/dd-trace/src/appsec/iast/telemetry/index.js @@ -0,0 +1,53 @@ +'use strict' + +const telemetryMetrics = require('../../../telemetry/metrics') +const telemetryLogs = require('./log') +const { Verbosity, isDebugAllowed, isInfoAllowed, getVerbosity } = require('./verbosity') +const { initRequestNamespace, finalizeRequestNamespace, globalNamespace } = require('./namespaces') + +class Telemetry { + configure (config, verbosity) { + // in order to telemetry be enabled, tracer telemetry and metrics collection have to be enabled + this.enabled = config && config.telemetry && config.telemetry.enabled && config.telemetry.metrics + this.verbosity = this.enabled ? getVerbosity(verbosity) : Verbosity.OFF + + if (this.enabled) { + telemetryMetrics.manager.set('iast', globalNamespace) + } + + telemetryLogs.start() + } + + stop () { + this.enabled = false + telemetryMetrics.manager.delete('iast') + + telemetryLogs.stop() + } + + isEnabled () { + return this.enabled + } + + isDebugEnabled () { + return this.isEnabled() && isDebugAllowed(this.verbosity) + } + + isInformationEnabled () { + return this.isEnabled() && isInfoAllowed(this.verbosity) + } + + onRequestStarted (context) { + if (this.isEnabled() && this.verbosity !== Verbosity.OFF) { + initRequestNamespace(context) + } + } + + onRequestEnd (context, rootSpan) { + if (this.isEnabled() && this.verbosity !== Verbosity.OFF) { + finalizeRequestNamespace(context, rootSpan, this.namespace) + } + } +} + +module.exports = new Telemetry() diff --git a/packages/dd-trace/src/appsec/iast/telemetry/logs.js b/packages/dd-trace/src/appsec/iast/telemetry/log/index.js similarity index 89% rename from packages/dd-trace/src/appsec/iast/telemetry/logs.js rename to packages/dd-trace/src/appsec/iast/telemetry/log/index.js index c14e9fcf3b1..0c052663667 100644 --- a/packages/dd-trace/src/appsec/iast/telemetry/logs.js +++ b/packages/dd-trace/src/appsec/iast/telemetry/log/index.js @@ -1,9 +1,9 @@ 'use strict' -const dc = require('../../../../../diagnostics_channel') -const logCollector = require('./log_collector') -const { sendData } = require('../../../telemetry/send-data') -const log = require('../../../log') +const dc = require('../../../../../../diagnostics_channel') +const logCollector = require('./log-collector') +const { sendData } = require('../../../../telemetry/send-data') +const log = require('../../../../log') const telemetryStartChannel = dc.channel('datadog:telemetry:start') const telemetryStopChannel = dc.channel('datadog:telemetry:stop') diff --git a/packages/dd-trace/src/appsec/iast/telemetry/log_collector.js b/packages/dd-trace/src/appsec/iast/telemetry/log/log-collector.js similarity index 98% rename from packages/dd-trace/src/appsec/iast/telemetry/log_collector.js rename to packages/dd-trace/src/appsec/iast/telemetry/log/log-collector.js index cf055c5ebc1..8e66b98ccae 100644 --- a/packages/dd-trace/src/appsec/iast/telemetry/log_collector.js +++ b/packages/dd-trace/src/appsec/iast/telemetry/log/log-collector.js @@ -1,6 +1,6 @@ 'use strict' -const log = require('../../../log') +const log = require('../../../../log') const logs = new Map() diff --git a/packages/dd-trace/src/appsec/iast/telemetry/namespaces.js b/packages/dd-trace/src/appsec/iast/telemetry/namespaces.js new file mode 100644 index 00000000000..d00e52aef33 --- /dev/null +++ b/packages/dd-trace/src/appsec/iast/telemetry/namespaces.js @@ -0,0 +1,76 @@ +'use strict' + +const log = require('../../../log') +const { Namespace } = require('../../../telemetry/metrics') +const { addMetricsToSpan, filterTags } = require('./span-tags') +const { IAST_TRACE_METRIC_PREFIX } = require('../tags') + +const DD_IAST_METRICS_NAMESPACE = Symbol('_dd.iast.request.metrics.namespace') + +function initRequestNamespace (context) { + if (!context) return + + const namespace = new Namespace('iast') + context[DD_IAST_METRICS_NAMESPACE] = namespace + return namespace +} + +function getNamespaceFromContext (context) { + return context && context[DD_IAST_METRICS_NAMESPACE] +} + +function finalizeRequestNamespace (context, rootSpan) { + try { + const namespace = getNamespaceFromContext(context) + if (!namespace) return + + const metrics = [...namespace.metrics.values()] + namespace.metrics.clear() + + addMetricsToSpan(rootSpan, metrics, IAST_TRACE_METRIC_PREFIX) + + merge(metrics) + } catch (e) { + log.error(e) + } finally { + if (context) { + delete context[DD_IAST_METRICS_NAMESPACE] + } + } +} + +function merge (metrics) { + metrics.forEach(metric => metric.points.forEach(point => { + globalNamespace + .count(metric.metric, getTagsObject(metric.tags)) + .inc(point[1]) + })) +} + +function getTagsObject (tags) { + if (tags && tags.length > 0) { + return filterTags(tags) + } +} + +class IastNamespace extends Namespace { + constructor () { + super('iast') + } + + reset () { + this.metrics.clear() + this.distributions.clear() + } +} + +const globalNamespace = new IastNamespace() + +module.exports = { + initRequestNamespace, + getNamespaceFromContext, + finalizeRequestNamespace, + globalNamespace, + + DD_IAST_METRICS_NAMESPACE +} diff --git a/packages/dd-trace/src/appsec/iast/telemetry/span-tags.js b/packages/dd-trace/src/appsec/iast/telemetry/span-tags.js new file mode 100644 index 00000000000..9d434bdb9ae --- /dev/null +++ b/packages/dd-trace/src/appsec/iast/telemetry/span-tags.js @@ -0,0 +1,53 @@ +'use strict' + +function addMetricsToSpan (rootSpan, metrics, tagPrefix) { + if (!rootSpan || !rootSpan.addTags || !metrics) return + + const flattenMap = new Map() + metrics + .filter(data => data && data.metric) + .forEach(data => { + const name = taggedMetricName(data) + let total = flattenMap.get(name) + const value = flatten(data) + if (!total) { + total = value + } else { + total += value + } + flattenMap.set(name, total) + }) + + for (const [key, value] of flattenMap) { + const tagName = `${tagPrefix}.${key}` + rootSpan.addTags({ + [tagName]: value + }) + } +} + +function flatten (metricData) { + return metricData.points && metricData.points.map(point => point[1]).reduce((total, value) => total + value, 0) +} + +function taggedMetricName (data) { + const metric = data.metric + const tags = data.tags && filterTags(data.tags) + return !tags || !tags.length + ? metric + : `${metric}.${processTagValue(tags)}` +} + +function filterTags (tags) { + return tags.filter(tag => !tag.startsWith('lib_language') && !tag.startsWith('version')) +} + +function processTagValue (tags) { + return tags.map(tag => tag.includes(':') ? tag.split(':')[1] : tag) + .join('_').replace(/\./g, '_') +} + +module.exports = { + addMetricsToSpan, + filterTags +} diff --git a/packages/dd-trace/src/appsec/iast/telemetry/verbosity.js b/packages/dd-trace/src/appsec/iast/telemetry/verbosity.js new file mode 100644 index 00000000000..100cd078828 --- /dev/null +++ b/packages/dd-trace/src/appsec/iast/telemetry/verbosity.js @@ -0,0 +1,42 @@ +'use strict' + +const Verbosity = { + OFF: 0, + MANDATORY: 1, + INFORMATION: 2, + DEBUG: 3 +} + +function isDebugAllowed (value) { + return value >= Verbosity.DEBUG +} + +function isInfoAllowed (value) { + return value >= Verbosity.INFORMATION +} + +function getVerbosity (verbosity) { + if (verbosity) { + verbosity = verbosity.toUpperCase() + return Verbosity[verbosity] !== undefined ? Verbosity[verbosity] : Verbosity.INFORMATION + } else { + return Verbosity.INFORMATION + } +} + +function getName (verbosityValue) { + for (const name in Verbosity) { + if (Verbosity[name] === verbosityValue) { + return name + } + } + return 'OFF' +} + +module.exports = { + Verbosity, + isDebugAllowed, + isInfoAllowed, + getVerbosity, + getName +} diff --git a/packages/dd-trace/src/config.js b/packages/dd-trace/src/config.js index 3a896be6a9c..a91d973d0c5 100644 --- a/packages/dd-trace/src/config.js +++ b/packages/dd-trace/src/config.js @@ -247,6 +247,10 @@ class Config { process.env.DD_TELEMETRY_DEBUG, false ) + const DD_TELEMETRY_METRICS_ENABLED = coalesce( + process.env.DD_TELEMETRY_METRICS_ENABLED, + false + ) const DD_TRACE_AGENT_PROTOCOL_VERSION = coalesce( options.protocolVersion, process.env.DD_TRACE_AGENT_PROTOCOL_VERSION, @@ -461,6 +465,12 @@ ken|consumer_?(?:id|key|secret)|sign(?:ed|ature)?|auth(?:entication|orization)?) true ) + const DD_IAST_TELEMETRY_VERBOSITY = coalesce( + iastOptions && iastOptions.telemetryVerbosity, + process.env.DD_IAST_TELEMETRY_VERBOSITY, + 'INFORMATION' + ) + const DD_CIVISIBILITY_GIT_UPLOAD_ENABLED = coalesce( process.env.DD_CIVISIBILITY_GIT_UPLOAD_ENABLED, true @@ -561,7 +571,8 @@ ken|consumer_?(?:id|key|secret)|sign(?:ed|ature)?|auth(?:entication|orization)?) enabled: DD_TRACE_EXPORTER !== 'datadog' && isTrue(DD_TRACE_TELEMETRY_ENABLED), heartbeatInterval: DD_TELEMETRY_HEARTBEAT_INTERVAL, logCollection: isTrue(DD_TELEMETRY_LOG_COLLECTION_ENABLED), - debug: isTrue(DD_TELEMETRY_DEBUG) + debug: isTrue(DD_TELEMETRY_DEBUG), + metrics: isTrue(DD_TELEMETRY_METRICS_ENABLED) } this.protocolVersion = DD_TRACE_AGENT_PROTOCOL_VERSION this.tagsHeaderMaxLength = parseInt(DD_TRACE_X_DATADOG_TAGS_MAX_LENGTH) @@ -590,7 +601,8 @@ ken|consumer_?(?:id|key|secret)|sign(?:ed|ature)?|auth(?:entication|orization)?) maxConcurrentRequests: DD_IAST_MAX_CONCURRENT_REQUESTS, maxContextOperations: DD_IAST_MAX_CONTEXT_OPERATIONS, deduplicationEnabled: DD_IAST_DEDUPLICATION_ENABLED, - redactionEnabled: DD_IAST_REDACTION_ENABLED + redactionEnabled: DD_IAST_REDACTION_ENABLED, + telemetryVerbosity: DD_IAST_TELEMETRY_VERBOSITY } this.isCiVisibility = isTrue(DD_IS_CIVISIBILITY) diff --git a/packages/dd-trace/test/appsec/iast/analyzers/ldap-injection-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/ldap-injection-analyzer.spec.js index 48223e1f103..59413db0a4f 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/ldap-injection-analyzer.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/ldap-injection-analyzer.spec.js @@ -19,6 +19,8 @@ describe('ldap-injection-analyzer', () => { './injection-analyzer': InjectionAnalyzer }) + ldapInjectionAnalyzer.configure(true) + it('should subscribe to ldapjs client search channel', () => { expect(ldapInjectionAnalyzer._subscriptions).to.have.lengthOf(1) expect(ldapInjectionAnalyzer._subscriptions[0]._channel.name).to.equals('datadog:ldapjs:client:search') @@ -71,4 +73,37 @@ describe('ldap-injection-analyzer', () => { expect(addVulnerability).to.have.been.calledOnce expect(addVulnerability).to.have.been.calledWithMatch({}, { type: 'LDAP_INJECTION' }) }) + + it('should call analyzeAll when datadog:ldapjs:client:search event is published', () => { + const store = {} + const iastContext = {} + const getStore = sinon.stub().returns(store) + const getIastContext = sinon.stub().returns(iastContext) + + const iastPlugin = proxyquire('../../../../src/appsec/iast/iast-plugin', { + '../../../../datadog-core': { storage: { getStore } }, + './iast-context': { getIastContext } + }) + + const ProxyAnalyzer = proxyquire('../../../../src/appsec/iast/analyzers/vulnerability-analyzer', { + '../iast-plugin': iastPlugin, + '../overhead-controller': { hasQuota: () => true } + }) + const InjectionAnalyzer = proxyquire('../../../../src/appsec/iast/analyzers/injection-analyzer', { + '../taint-tracking/operations': TaintTrackingMock, + './vulnerability-analyzer': ProxyAnalyzer + }) + + const ldapInjectionAnalyzer = proxyquire('../../../../src/appsec/iast/analyzers/ldap-injection-analyzer', { + './injection-analyzer': InjectionAnalyzer + }) + const analyzeAll = sinon.stub(ldapInjectionAnalyzer, 'analyzeAll') + ldapInjectionAnalyzer.configure(true) + + const onLdapClientSearch = ldapInjectionAnalyzer._subscriptions[0]._handler + + onLdapClientSearch({ base: 'base', filter: 'filter', name: 'datadog:ldapjs:client:search' }) + + expect(analyzeAll.firstCall).to.be.calledWith('base', 'filter') + }) }) diff --git a/packages/dd-trace/test/appsec/iast/analyzers/sql-injection-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/sql-injection-analyzer.spec.js index bd621a8ac35..f743f0d9288 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/sql-injection-analyzer.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/sql-injection-analyzer.spec.js @@ -26,6 +26,8 @@ describe('sql-injection-analyzer', () => { sinon.restore() }) + sqlInjectionAnalyzer.configure(true) + it('should subscribe to mysql, mysql2 and pg start query channel', () => { expect(sqlInjectionAnalyzer._subscriptions).to.have.lengthOf(5) expect(sqlInjectionAnalyzer._subscriptions[0]._channel.name).to.equals('apm:mysql:query:start') @@ -98,4 +100,62 @@ describe('sql-injection-analyzer', () => { sqlInjectionAnalyzer.configure(false) expect(iastLog.errorAndPublish).not.to.be.called }) + + describe('analyze', () => { + let sqlInjectionAnalyzer, analyze + + const store = {} + const iastContext = {} + + beforeEach(() => { + const getStore = sinon.stub().returns(store) + const getIastContext = sinon.stub().returns(iastContext) + + const iastPlugin = proxyquire('../../../../src/appsec/iast/iast-plugin', { + '../../../../datadog-core': { storage: { getStore } }, + './iast-context': { getIastContext } + }) + + const ProxyAnalyzer = proxyquire('../../../../src/appsec/iast/analyzers/vulnerability-analyzer', { + '../iast-plugin': iastPlugin, + '../overhead-controller': { hasQuota: () => true } + }) + const InjectionAnalyzer = proxyquire('../../../../src/appsec/iast/analyzers/injection-analyzer', { + '../taint-tracking/operations': TaintTrackingMock, + './vulnerability-analyzer': ProxyAnalyzer + }) + + sqlInjectionAnalyzer = proxyquire('../../../../src/appsec/iast/analyzers/sql-injection-analyzer', { + './injection-analyzer': InjectionAnalyzer + }) + analyze = sinon.stub(sqlInjectionAnalyzer, 'analyze') + sqlInjectionAnalyzer.configure(true) + }) + + afterEach(sinon.restore) + + it('should call analyze on apm:mysql:query:start', () => { + const onMysqlQueryStart = sqlInjectionAnalyzer._subscriptions[0]._handler + + onMysqlQueryStart({ sql: 'SELECT 1', name: 'apm:mysql:query:start' }) + + expect(analyze).to.be.calledOnceWith('SELECT 1') + }) + + it('should call analyze on apm:mysql2:query:start', () => { + const onMysql2QueryStart = sqlInjectionAnalyzer._subscriptions[0]._handler + + onMysql2QueryStart({ sql: 'SELECT 1', name: 'apm:mysql2:query:start' }) + + expect(analyze).to.be.calledOnceWith('SELECT 1') + }) + + it('should call analyze on apm:pg:query:start', () => { + const onPgQueryStart = sqlInjectionAnalyzer._subscriptions[0]._handler + + onPgQueryStart({ sql: 'SELECT 1', name: 'apm:pg:query:start' }) + + expect(analyze).to.be.calledOnceWith('SELECT 1') + }) + }) }) diff --git a/packages/dd-trace/test/appsec/iast/analyzers/unvalidated-redirect-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/unvalidated-redirect-analyzer.spec.js index d8b1d9f15db..25d91bf4cc2 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/unvalidated-redirect-analyzer.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/unvalidated-redirect-analyzer.spec.js @@ -4,7 +4,7 @@ const { expect } = require('chai') const proxyquire = require('proxyquire') const overheadController = require('../../../../src/appsec/iast/overhead-controller') const { HTTP_REQUEST_HEADER_VALUE, HTTP_REQUEST_PARAMETER } = - require('../../../../src/appsec/iast/taint-tracking/origin-types') + require('../../../../src/appsec/iast/taint-tracking/source-types') describe('unvalidated-redirect-analyzer', () => { const NOT_TAINTED_LOCATION = 'url.com' @@ -67,6 +67,8 @@ describe('unvalidated-redirect-analyzer', () => { '../taint-tracking/operations': TaintTrackingMock }) + unvalidatedRedirectAnalyzer.configure(true) + it('should subscribe to set-header:finish channel', () => { expect(unvalidatedRedirectAnalyzer._subscriptions).to.have.lengthOf(1) expect(unvalidatedRedirectAnalyzer._subscriptions[0]._channel.name).to diff --git a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js index dc5bc7187b6..0a9392bd5c6 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js @@ -66,7 +66,7 @@ describe('vulnerability-analyzer', () => { reported: false }, { - iastContext: undefined, + iastContext: {}, oceResult: true, isVulnerable: true, reported: true @@ -131,16 +131,23 @@ describe('vulnerability-analyzer', () => { }) const wrapped = vulnerabilityAnalyzer._wrapHandler(handler) + const iastContext = { + name: 'test' + } + iastContextHandler.getIastContext.returns(iastContext) + expect(typeof wrapped).to.be.equal('function') const message = {} const name = 'test' expect(() => wrapped(message, name)).to.not.throw() - expect(handler).to.be.calledOnceWithExactly(message, name) + const args = handler.firstCall.args + expect(args[0]).to.be.eq(message) + expect(args[1]).to.be.eq(name) }) it('should catch thrown Errors inside subscription handlers', () => { const vulnerabilityAnalyzer = new VulnerabilityAnalyzer(ANALYZER_TYPE) - vulnerabilityAnalyzer.addSub('dd-trace:test:error:sub', () => { + vulnerabilityAnalyzer.addSub({ channelName: 'dd-trace:test:error:sub' }, () => { throw new Error('handler Error') }) @@ -151,6 +158,53 @@ describe('vulnerability-analyzer', () => { expect(() => { dc.channel('dd-trace:test:error:sub').publish({}) }).to.not.throw() }) + describe('addSub', () => { + let iastPluginAddSub + beforeEach(() => { + iastPluginAddSub = sinon.spy() + + class SinkIastPlugin { + addSub (iastSub, handler) { + iastPluginAddSub(iastSub, handler) + } + } + + VulnerabilityAnalyzer = proxyquire('../../../../src/appsec/iast/analyzers/vulnerability-analyzer', { + '../vulnerability-reporter': vulnerabilityReporter, + '../path-line': pathLine, + '../overhead-controller': overheadController, + '../iast-context': iastContextHandler, + '../iast-plugin': { + SinkIastPlugin + } + }) + }) + + it('should accept a string argument and convert it to iastSub object', () => { + const vulnerabilityAnalyzer = new VulnerabilityAnalyzer(ANALYZER_TYPE) + const handler = () => {} + vulnerabilityAnalyzer.addSub('dd-trace:test:channelName', handler) + + expect(iastPluginAddSub).to.have.been.calledOnce + expect(iastPluginAddSub.firstCall.args).to.be.deep.equal([{ + channelName: 'dd-trace:test:channelName', + tag: ANALYZER_TYPE + }, handler]) + }) + + it('should accept a iastSub object', () => { + const vulnerabilityAnalyzer = new VulnerabilityAnalyzer(ANALYZER_TYPE) + const handler = () => {} + vulnerabilityAnalyzer.addSub({ channelName: 'dd-trace:test:channelName' }, handler) + + expect(iastPluginAddSub).to.have.been.calledOnce + expect(iastPluginAddSub.firstCall.args).to.be.deep.equal([{ + channelName: 'dd-trace:test:channelName', + tag: ANALYZER_TYPE + }, handler]) + }) + }) + describe('createVulnerability', () => { let vulnerabilityAnalyzer @@ -211,4 +265,39 @@ describe('vulnerability-analyzer', () => { expect(vulnerability.location.spanId).to.be.equal(0) }) }) + + describe('analyzeAll', () => { + const value1 = 'test' + const value2 = 'test2' + + it('should not throw with undefined values', () => { + const vulnerabilityAnalyzer = new VulnerabilityAnalyzer(ANALYZER_TYPE) + + expect(() => vulnerabilityAnalyzer.analyzeAll(undefined, value1, undefined)).to.not.throw + }) + + it('should analyze batch of values', () => { + const vulnerabilityAnalyzer = new VulnerabilityAnalyzer(ANALYZER_TYPE) + const context = { + rootSpan: { + context: sinon.mock().returns({ + toSpanId: sinon.mock().returns(SPAN_ID) + }) + } + } + iastContextHandler.getIastContext.returns(context) + sinon.stub(vulnerabilityAnalyzer, '_isVulnerable').returns((value) => value === 'test') + sinon.stub(vulnerabilityAnalyzer, '_checkOCE').returns(true) + sinon.stub(vulnerabilityAnalyzer, '_createVulnerability') + + vulnerabilityAnalyzer.analyzeAll(value1, value2) + + expect(vulnerabilityAnalyzer._createVulnerability).to.have.been.calledOnceWith( + ANALYZER_TYPE, + { value: 'test' }, + SPAN_ID, + VULNERABILITY_LOCATION + ) + }) + }) }) diff --git a/packages/dd-trace/test/appsec/iast/analyzers/weak-cipher-analyzers.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/weak-cipher-analyzers.spec.js index bb3ab7262dd..b803b7705c9 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/weak-cipher-analyzers.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/weak-cipher-analyzers.spec.js @@ -8,6 +8,8 @@ describe('weak-cipher-analyzer', () => { const VULNERABLE_CIPHER = 'des-ede-cbc' const NON_VULNERABLE_CIPHER = 'sha512' + weakCipherAnalyzer.configure(true) + it('should subscribe to crypto hashing channel', () => { expect(weakCipherAnalyzer._subscriptions).to.have.lengthOf(1) expect(weakCipherAnalyzer._subscriptions[0]._channel.name).to.equals('datadog:crypto:cipher:start') diff --git a/packages/dd-trace/test/appsec/iast/analyzers/weak-hash-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/weak-hash-analyzer.spec.js index 032b51b429c..18cf741ce85 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/weak-hash-analyzer.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/weak-hash-analyzer.spec.js @@ -9,6 +9,8 @@ describe('weak-hash-analyzer', () => { const VULNERABLE_ALGORITHM = 'sha1' const NON_VULNERABLE_ALGORITHM = 'sha512' + weakHashAnalyzer.configure(true) + it('should subscribe to crypto hashing channel', () => { expect(weakHashAnalyzer._subscriptions).to.have.lengthOf(1) expect(weakHashAnalyzer._subscriptions[0]._channel.name).to.equals('datadog:crypto:hashing:start') diff --git a/packages/dd-trace/test/appsec/iast/iast-log.spec.js b/packages/dd-trace/test/appsec/iast/iast-log.spec.js index 0e079625c34..b3925c8f1e7 100644 --- a/packages/dd-trace/test/appsec/iast/iast-log.spec.js +++ b/packages/dd-trace/test/appsec/iast/iast-log.spec.js @@ -47,8 +47,8 @@ describe('IAST log', () => { warn: sinon.stub(), error: sinon.stub() } - telemetryLogs = proxyquire('../../../src/appsec/iast/telemetry/logs', { - '../../../../../diagnostics_channel': { + telemetryLogs = proxyquire('../../../src/appsec/iast/telemetry/log', { + '../../../../../../diagnostics_channel': { channel: (name) => name === 'datadog:telemetry:start' ? telemetryStartChannel : telemetryStopChannel } }) @@ -57,7 +57,7 @@ describe('IAST log', () => { telemetryLogs.start() iastLog = proxyquire('../../../src/appsec/iast/iast-log', { - './telemetry/logs': telemetryLogs, + './telemetry/log': telemetryLogs, '../../log': log }) }) diff --git a/packages/dd-trace/test/appsec/iast/iast-plugin.spec.js b/packages/dd-trace/test/appsec/iast/iast-plugin.spec.js new file mode 100644 index 00000000000..2e36a4bb1b4 --- /dev/null +++ b/packages/dd-trace/test/appsec/iast/iast-plugin.spec.js @@ -0,0 +1,382 @@ +'use strict' + +const { expect } = require('chai') +const { channel } = require('../../../../diagnostics_channel') +const proxyquire = require('proxyquire') +const { getExecutedMetric, getInstrumentedMetric, TagKey } = require('../../../src/appsec/iast/telemetry/iast-metric') + +const VULNERABILITY_TYPE = TagKey.VULNERABILITY_TYPE +const SOURCE_TYPE = TagKey.SOURCE_TYPE + +describe('IAST Plugin', () => { + const loadChannel = channel('dd-trace:instrumentation:load') + + let logError, addSubMock, getIastContext, configureMock, datadogCore + + const handler = () => { + throw new Error('handler error') + } + const channelName = 'datadog:test:start' + + let iastPlugin + + beforeEach(() => { + addSubMock = sinon.stub() + logError = sinon.stub() + getIastContext = sinon.stub() + configureMock = sinon.stub() + }) + + afterEach(() => { + sinon.restore() + }) + + describe('with appsec telemetry disabled', () => { + let IastPlugin + + beforeEach(() => { + class PluginClass { + addSub (channelName, handler) { + addSubMock(channelName, handler) + } + configure (config) { + configureMock(config) + } + } + + datadogCore = { + storage: { + getStore: sinon.stub() + } + } + + const iastPluginMod = proxyquire('../../../src/appsec/iast/iast-plugin', { + '../../plugins/plugin': PluginClass, + './iast-log': { + errorAndPublish: logError + }, + './iast-context': { + getIastContext: getIastContext + }, + './telemetry': { + isEnabled: () => false + }, + './telemetry/metrics': {}, + '../../../../datadog-core': datadogCore + }) + IastPlugin = iastPluginMod.IastPlugin + iastPlugin = new IastPlugin() + }) + + afterEach(() => { + iastPlugin.disableTelemetry() + }) + + describe('addSub', () => { + it('should call Plugin.addSub with channelName and wrapped handler', () => { + iastPlugin.addSub('test', handler) + + expect(addSubMock).to.be.calledOnce + const args = addSubMock.getCall(0).args + expect(args[0]).equal('test') + + const wrapped = args[1] + expect(wrapped).to.be.a('function') + expect(wrapped).to.not.be.equal(handler) + expect(wrapped()).to.not.throw + expect(logError).to.be.calledOnce + }) + + it('should call Plugin.addSub with channelName and wrapped handler after registering iastPluginSub', () => { + const iastPluginSub = { channelName: 'test' } + iastPlugin.addSub(iastPluginSub, handler) + + expect(addSubMock).to.be.calledOnce + const args = addSubMock.getCall(0).args + expect(args[0]).equal('test') + + const wrapped = args[1] + expect(wrapped).to.be.a('function') + expect(wrapped).to.not.be.equal(handler) + expect(wrapped()).to.not.throw + expect(logError).to.be.calledOnce + }) + + it('should infer moduleName from channelName after registering iastPluginSub', () => { + const iastPluginSub = { channelName: 'test' } + iastPlugin.addSub(iastPluginSub, handler) + + expect(iastPlugin.pluginSubs).to.have.lengthOf(1) + expect(iastPlugin.pluginSubs[0].moduleName).eq('test') + }) + + it('should infer moduleName from channelName after registering iastPluginSub with real channelName', () => { + const iastPluginSub = { channelName: 'datadog:test:start' } + iastPlugin.addSub(iastPluginSub, handler) + + expect(iastPlugin.pluginSubs).to.have.lengthOf(1) + expect(iastPlugin.pluginSubs[0].moduleName).eq('test') + }) + + it('should not call _getTelemetryHandler', () => { + const wrapHandler = sinon.stub() + iastPlugin._wrapHandler = wrapHandler + const getTelemetryHandler = sinon.stub() + iastPlugin._getTelemetryHandler = getTelemetryHandler + iastPlugin.addSub({ channelName, tagKey: VULNERABILITY_TYPE }, handler) + + expect(wrapHandler).to.be.calledOnceWith(handler) + expect(getTelemetryHandler).to.be.not.called + + wrapHandler.reset() + getTelemetryHandler.reset() + + iastPlugin.addSub({ channelName, tagKey: SOURCE_TYPE, tag: 'test-tag' }, handler) + expect(wrapHandler).to.be.calledOnceWith(handler) + expect(getTelemetryHandler).to.be.not.called + }) + }) + + describe('configure', () => { + it('should mark Plugin configured and call only once onConfigure', () => { + iastPlugin.onConfigure = sinon.stub() + iastPlugin.configure(true) + iastPlugin.configure(false) + iastPlugin.configure(true) + + expect(iastPlugin.configured).to.be.true + expect(iastPlugin.onConfigure).to.be.calledOnce + }) + }) + + describe('_execHandlerAndIncMetric', () => { + it('should exec handler', () => { + const handler = sinon.spy() + + iastPlugin._execHandlerAndIncMetric({ + handler + }) + + expect(handler).to.be.calledOnce + }) + + it('should exec handler and catch exception if any', () => { + const handler = () => { throw new Error('error') } + + expect(iastPlugin._execHandlerAndIncMetric({ + handler + })).to.not.throw + expect(logError).to.be.calledOnce + }) + + it('should exec handler and not increase metric', () => { + const handler = sinon.spy() + const metric = { + increase: sinon.spy() + } + iastPlugin._execHandlerAndIncMetric({ + handler, + metric + }) + + expect(handler).to.be.calledOnce + expect(metric.increase).to.not.be.called + }) + }) + }) + + describe('with appsec telemetry enabled', () => { + let iastTelemetry + let IastPlugin + + beforeEach(() => { + class PluginClass { + addSub (channelName, handler) { + addSubMock(channelName, handler) + } + configure (config) { + configureMock(config) + } + } + iastTelemetry = { + isEnabled: () => true, + isDebugEnabled: () => true + } + IastPlugin = proxyquire('../../../src/appsec/iast/iast-plugin', { + '../../plugins/plugin': PluginClass, + './iast-log': { + errorAndPublish: logError + }, + './telemetry': iastTelemetry, + '../../../../datadog-instrumentations/src/helpers/instrumentations': {} + }).IastPlugin + + iastPlugin = new IastPlugin() + }) + + afterEach(() => { + iastPlugin.disableTelemetry() + sinon.restore() + }) + + describe('configure', () => { + it('should subscribe dd-trace:instrumentation:load channel', () => { + const onInstrumentationLoadedMock = sinon.stub(iastPlugin, '_onInstrumentationLoaded') + iastPlugin.configure(true) + iastPlugin.configure(false) + iastPlugin.configure(true) + + loadChannel.publish({ name: 'test' }) + + expect(onInstrumentationLoadedMock).to.be.calledWith('test') + }) + }) + + describe('addSub', () => { + it('should call _getTelemetryHandler with correct metrics', () => { + const wrapHandler = sinon.stub() + iastPlugin._wrapHandler = wrapHandler + const getTelemetryHandler = sinon.stub() + iastPlugin._getTelemetryHandler = getTelemetryHandler + iastPlugin.addSub({ channelName, tagKey: VULNERABILITY_TYPE }, handler) + + expect(wrapHandler).to.be.calledOnceWith(handler) + expect(getTelemetryHandler).to.be.calledOnceWith(iastPlugin.pluginSubs[0]) + + wrapHandler.reset() + getTelemetryHandler.reset() + + iastPlugin.addSub({ channelName, tagKey: SOURCE_TYPE, tag: 'test-tag' }, handler) + expect(wrapHandler).to.be.calledOnceWith(handler) + expect(getTelemetryHandler).to.be.calledOnceWith(iastPlugin.pluginSubs[1]) + }) + + it('should register an pluginSubscription and increment a sink metric when a sink module is loaded', () => { + iastPlugin.addSub({ + moduleName: 'sink', + channelName: 'datadog:sink:start', + tag: 'injection', + tagKey: VULNERABILITY_TYPE + }, handler) + iastPlugin.configure(true) + + const metric = getInstrumentedMetric(VULNERABILITY_TYPE) + const metricAdd = sinon.stub(metric, 'add') + + loadChannel.publish({ name: 'sink' }) + + expect(metricAdd).to.be.calledOnceWith(1, 'injection') + }) + + it('should register an pluginSubscription and increment a source metric when a source module is loaded', () => { + iastPlugin.addSub({ + moduleName: 'source', + channelName: 'datadog:source:start', + tag: 'http.source', + tagKey: SOURCE_TYPE + }, handler) + iastPlugin.configure(true) + + const metric = getInstrumentedMetric(SOURCE_TYPE) + const metricAdd = sinon.stub(metric, 'add') + + loadChannel.publish({ name: 'source' }) + + expect(metricAdd).to.be.calledOnceWith(1, 'http.source') + }) + + it('should increment a sink metric when event is received', () => { + iastPlugin.addSub({ + moduleName: 'sink', + channelName: 'datadog:sink:start', + tag: 'injection', + tagKey: VULNERABILITY_TYPE + }, handler) + iastPlugin.configure(true) + + const metric = getExecutedMetric(VULNERABILITY_TYPE) + const metricAdd = sinon.stub(metric, 'add') + + const telemetryHandler = addSubMock.secondCall.args[1] + telemetryHandler() + + expect(metricAdd).to.be.calledOnceWith(1, 'injection') + }) + + it('should increment a source metric when event is received', () => { + iastPlugin.addSub({ + moduleName: 'source', + channelName: 'datadog:source:start', + tag: 'http.source', + tagKey: SOURCE_TYPE + }, handler) + iastPlugin.configure(true) + + const metric = getExecutedMetric(SOURCE_TYPE) + const metricAdd = sinon.stub(metric, 'add') + + const telemetryHandler = addSubMock.secondCall.args[1] + telemetryHandler() + + expect(metricAdd).to.be.calledOnceWith(1, 'http.source') + }) + + it('should increment a source metric when event is received for every tag', () => { + iastPlugin.addSub({ + moduleName: 'source', + channelName: 'datadog:source:start', + tag: [ 'http.source', 'http.source2', 'http.source3' ], + tagKey: SOURCE_TYPE + }, handler) + iastPlugin.configure(true) + + const metric = getExecutedMetric(SOURCE_TYPE) + const metricAdd = sinon.stub(metric, 'add') + + const telemetryHandler = addSubMock.secondCall.args[1] + telemetryHandler() + + expect(metricAdd).to.be.calledOnceWith(1, [ 'http.source', 'http.source2', 'http.source3' ]) + }) + }) + + describe('_execHandlerAndIncMetric', () => { + it('should exec handler', () => { + const handler = sinon.spy() + + iastPlugin._execHandlerAndIncMetric({ + handler + }) + + expect(handler).to.be.calledOnce + }) + + it('should exec handler and catch exception if any', () => { + const handler = () => { throw new Error('error') } + + expect(iastPlugin._execHandlerAndIncMetric({ + handler + })).to.not.throw + expect(logError).to.be.calledOnce + }) + + it('should exec handler and increase metric', () => { + const handler = sinon.spy() + const metric = { + inc: sinon.spy() + } + const tag = 'tag1' + const iastContext = {} + iastPlugin._execHandlerAndIncMetric({ + handler, + metric, + tag, + iastContext + }) + + expect(handler).to.be.calledOnce + expect(metric.inc).to.be.calledOnceWithExactly(tag, iastContext) + }) + }) + }) +}) diff --git a/packages/dd-trace/test/appsec/iast/taint-tracking/plugin.spec.js b/packages/dd-trace/test/appsec/iast/taint-tracking/plugin.spec.js index a2a2a087449..0c4e307eba9 100644 --- a/packages/dd-trace/test/appsec/iast/taint-tracking/plugin.spec.js +++ b/packages/dd-trace/test/appsec/iast/taint-tracking/plugin.spec.js @@ -8,7 +8,7 @@ const { HTTP_REQUEST_COOKIE_VALUE, HTTP_REQUEST_COOKIE_NAME, HTTP_REQUEST_PATH_PARAM -} = require('../../../../src/appsec/iast/taint-tracking/origin-types') +} = require('../../../../src/appsec/iast/taint-tracking/source-types') const middlewareNextChannel = dc.channel('apm:express:middleware:next') const queryParseFinishChannel = dc.channel('datadog:qs:parse:finish') diff --git a/packages/dd-trace/test/appsec/iast/taint-tracking/rewriter-telemetry.spec.js b/packages/dd-trace/test/appsec/iast/taint-tracking/rewriter-telemetry.spec.js new file mode 100644 index 00000000000..0bb6b72316e --- /dev/null +++ b/packages/dd-trace/test/appsec/iast/taint-tracking/rewriter-telemetry.spec.js @@ -0,0 +1,70 @@ +'use strict' + +const proxyquire = require('proxyquire') + +const { INSTRUMENTED_PROPAGATION } = require('../../../../src/appsec/iast/telemetry/iast-metric') +const { Verbosity } = require('../../../../src/appsec/iast/telemetry/verbosity') + +describe('rewriter telemetry', () => { + let iastTelemetry, rewriter, getRewriteFunction + let instrumentedPropagationAdd + + beforeEach(() => { + iastTelemetry = { + add: sinon.spy() + } + const rewriterTelemetry = proxyquire('../../../../src/appsec/iast/taint-tracking/rewriter-telemetry', { + '../telemetry': iastTelemetry + }) + getRewriteFunction = rewriterTelemetry.getRewriteFunction + rewriter = { + rewrite: (content) => { + return { + content: content + 'rewritten', + metrics: { + instrumentedPropagation: 2 + } + } + } + } + instrumentedPropagationAdd = sinon.stub(INSTRUMENTED_PROPAGATION, 'add') + }) + + afterEach(sinon.restore) + + it('should not increase any metrics with OFF verbosity', () => { + iastTelemetry.verbosity = Verbosity.OFF + + const rewriteFn = getRewriteFunction(rewriter) + rewriteFn('const a = b + c', 'test.js') + + expect(instrumentedPropagationAdd).to.not.be.called + }) + + it('should increase information metrics with MANDATORY verbosity', () => { + iastTelemetry.verbosity = Verbosity.MANDATORY + + const rewriteFn = getRewriteFunction(rewriter) + const result = rewriteFn('const a = b + c', 'test.js') + + expect(instrumentedPropagationAdd).to.be.calledOnceWith(result.metrics.instrumentedPropagation) + }) + + it('should increase information metrics with INFORMATION verbosity', () => { + iastTelemetry.verbosity = Verbosity.INFORMATION + + const rewriteFn = getRewriteFunction(rewriter) + const result = rewriteFn('const a = b + c', 'test.js') + + expect(instrumentedPropagationAdd).to.be.calledOnceWith(result.metrics.instrumentedPropagation) + }) + + it('should increase debug metrics with DEBUG verbosity', () => { + iastTelemetry.verbosity = Verbosity.DEBUG + + const rewriteFn = getRewriteFunction(rewriter) + const result = rewriteFn('const a = b + c', 'test.js') + + expect(instrumentedPropagationAdd).to.be.calledOnceWith(result.metrics.instrumentedPropagation) + }) +}) diff --git a/packages/dd-trace/test/appsec/iast/taint-tracking/rewriter.spec.js b/packages/dd-trace/test/appsec/iast/taint-tracking/rewriter.spec.js index b77cbac75af..9eb2e94b23e 100644 --- a/packages/dd-trace/test/appsec/iast/taint-tracking/rewriter.spec.js +++ b/packages/dd-trace/test/appsec/iast/taint-tracking/rewriter.spec.js @@ -13,19 +13,38 @@ describe('IAST Rewriter', () => { }) describe('Enabling rewriter', () => { - let rewriter + let rewriter, iastTelemetry + const shimmer = { wrap: sinon.spy(), unwrap: sinon.spy() } + class Rewriter { + rewrite (content, filename) { + return { + content: content + 'rewritten', + metrics: { + instrumentedPropagation: 2 + } + } + } + } + beforeEach(() => { + iastTelemetry = { + add: sinon.spy() + } rewriter = proxyquire('../../../../src/appsec/iast/taint-tracking/rewriter', { - '../../../../../datadog-shimmer': shimmer + '@datadog/native-iast-rewriter': { Rewriter, getPrepareStackTrace: function () {} }, + '../../../../../datadog-shimmer': shimmer, + '../../telemetry': iastTelemetry }) }) - afterEach(sinon.restore) + afterEach(() => { + sinon.restore() + }) it('Should wrap module compile method on taint tracking enable', () => { rewriter.enableRewriter() diff --git a/packages/dd-trace/test/appsec/iast/taint-tracking/sources/taint-tracking.cookie.plugin.spec.js b/packages/dd-trace/test/appsec/iast/taint-tracking/sources/taint-tracking.cookie.plugin.spec.js index c0c7bea232d..782613e44e4 100644 --- a/packages/dd-trace/test/appsec/iast/taint-tracking/sources/taint-tracking.cookie.plugin.spec.js +++ b/packages/dd-trace/test/appsec/iast/taint-tracking/sources/taint-tracking.cookie.plugin.spec.js @@ -9,7 +9,7 @@ const { isTainted, getRanges } = require('../../../../../src/appsec/iast/taint-t const { HTTP_REQUEST_COOKIE_NAME, HTTP_REQUEST_COOKIE_VALUE -} = require('../../../../../src/appsec/iast/taint-tracking/origin-types') +} = require('../../../../../src/appsec/iast/taint-tracking/source-types') const { testInRequest } = require('../../utils') describe('Cookies sourcing with cookies', () => { diff --git a/packages/dd-trace/test/appsec/iast/taint-tracking/sources/taint-tracking.express.plugin.spec.js b/packages/dd-trace/test/appsec/iast/taint-tracking/sources/taint-tracking.express.plugin.spec.js index d3a5714f631..9823f71efc3 100644 --- a/packages/dd-trace/test/appsec/iast/taint-tracking/sources/taint-tracking.express.plugin.spec.js +++ b/packages/dd-trace/test/appsec/iast/taint-tracking/sources/taint-tracking.express.plugin.spec.js @@ -9,7 +9,7 @@ const { storage } = require('../../../../../../datadog-core') const iast = require('../../../../../src/appsec/iast') const iastContextFunctions = require('../../../../../src/appsec/iast/iast-context') const { isTainted, getRanges } = require('../../../../../src/appsec/iast/taint-tracking/operations') -const { HTTP_REQUEST_PATH_PARAM } = require('../../../../../src/appsec/iast/taint-tracking/origin-types') +const { HTTP_REQUEST_PATH_PARAM } = require('../../../../../src/appsec/iast/taint-tracking/source-types') describe('Path params sourcing with express', () => { let express diff --git a/packages/dd-trace/test/appsec/iast/taint-tracking/sources/taint-tracking.headers.spec.js b/packages/dd-trace/test/appsec/iast/taint-tracking/sources/taint-tracking.headers.spec.js index 12959cfd10a..4a846b875b4 100644 --- a/packages/dd-trace/test/appsec/iast/taint-tracking/sources/taint-tracking.headers.spec.js +++ b/packages/dd-trace/test/appsec/iast/taint-tracking/sources/taint-tracking.headers.spec.js @@ -9,7 +9,7 @@ const { isTainted, getRanges } = require('../../../../../src/appsec/iast/taint-t const { HTTP_REQUEST_HEADER_NAME, HTTP_REQUEST_HEADER_VALUE -} = require('../../../../../src/appsec/iast/taint-tracking/origin-types') +} = require('../../../../../src/appsec/iast/taint-tracking/source-types') const { testInRequest } = require('../../utils') describe('Headers sourcing', () => { diff --git a/packages/dd-trace/test/appsec/iast/taint-tracking/taint-tracking-operations.spec.js b/packages/dd-trace/test/appsec/iast/taint-tracking/taint-tracking-operations.spec.js index 2044114983c..36a0d970ad0 100644 --- a/packages/dd-trace/test/appsec/iast/taint-tracking/taint-tracking-operations.spec.js +++ b/packages/dd-trace/test/appsec/iast/taint-tracking/taint-tracking-operations.spec.js @@ -3,6 +3,10 @@ const { expect } = require('chai') const proxyquire = require('proxyquire') const iastContextFunctions = require('../../../../src/appsec/iast/iast-context') +const { getExpectedMethods } = require('../../../../src/appsec/iast/taint-tracking/csi-methods') +const iastTelemetry = require('../../../../src/appsec/iast/telemetry') +const { EXECUTED_PROPAGATION, REQUEST_TAINTED } = require('../../../../src/appsec/iast/telemetry/iast-metric') +const { Verbosity } = require('../../../../src/appsec/iast/telemetry/verbosity') describe('IAST TaintTracking Operations', () => { let taintTrackingOperations @@ -16,7 +20,12 @@ describe('IAST TaintTracking Operations', () => { isTainted: id => id, getRanges: id => id, concat: id => id, - trim: id => id + trim: id => id, + getMetrics: id => { + return { + requestCount: 5 + } + } } const store = {} @@ -36,7 +45,8 @@ describe('IAST TaintTracking Operations', () => { taintTrackingOperations = proxyquire('../../../../src/appsec/iast/taint-tracking/operations', { '@datadog/native-iast-taint-tracking': taintedUtilsMock, '../../../../../datadog-core': datadogCore, - './taint-tracking-impl': taintTrackingImpl + './taint-tracking-impl': taintTrackingImpl, + '../telemetry': iastTelemetry }) }) @@ -274,6 +284,25 @@ describe('IAST TaintTracking Operations', () => { taintTrackingOperations.removeTransaction(iastContext) expect(taintedUtils.removeTransaction).not.to.be.called }) + + it('Should increment REQUEST_TAINTED metric if INFORMATION or greater verbosity is enabled', () => { + const iastContext = { + [taintTrackingOperations.IAST_TRANSACTION_ID]: 'id' + } + iastTelemetry.configure({ + telemetry: { enabled: true, metrics: true }, + iast: { + telemetryVerbosity: 'INFORMATION' + } + }) + + const requestTaintedAdd = sinon.stub(REQUEST_TAINTED, 'add') + + taintTrackingOperations.enableTaintOperations(iastTelemetry.verbosity) + taintTrackingOperations.removeTransaction(iastContext) + + expect(requestTaintedAdd).to.be.calledOnceWith(5, null, iastContext) + }) }) describe('SetMaxTransactions', () => { @@ -291,11 +320,13 @@ describe('IAST TaintTracking Operations', () => { }) describe('enableTaintTracking', () => { + let context beforeEach(() => { + context = { [taintTrackingOperations.IAST_TRANSACTION_ID]: 'id' } iastContextFunctions.saveIastContext( store, {}, - { [taintTrackingOperations.IAST_TRANSACTION_ID]: 'id' } + context ) }) @@ -322,6 +353,22 @@ describe('IAST TaintTracking Operations', () => { global._ddiast.plusOperator('helloworld', 'hello', 'world') expect(taintedUtils.concat).not.to.be.called }) + + it('Should set debug global._ddiast object', () => { + taintTrackingOperations.enableTaintOperations(Verbosity.DEBUG) + + // dummy taintedUtils is declared in global scope + expect(global._ddiast).not.to.be.undefined + expect(global._ddiast.plusOperator).not.to.be.undefined + + const executedPropagationIncrease = sinon.stub(EXECUTED_PROPAGATION, 'inc') + + // taintedUtils methods are not called + global._ddiast.plusOperator('helloworld', 'hello', 'world') + expect(taintedUtils.concat).to.be.called + + expect(executedPropagationIncrease).to.be.calledOnceWith(null, context) + }) }) describe('newTaintedString', () => { @@ -518,12 +565,27 @@ describe('IAST TaintTracking Operations', () => { }) }) - describe('TaintTrackingDummy', () => { + describe('TaintTrackingNoop', () => { it('should have the same properties as TaintTracking', () => { - const tt = taintTrackingImpl.TaintTracking - const dummy = taintTrackingImpl.TaintTrackingDummy + const tt = taintTrackingImpl.getTaintTrackingImpl() + const noop = taintTrackingImpl.getTaintTrackingNoop() + + expect(noop).to.have.all.keys(Object.keys(tt)) + }) + + it('should have the same properties as TaintTrackingDebug', () => { + const ttDebug = taintTrackingImpl.getTaintTrackingImpl(Verbosity.DEBUG) + const noop = taintTrackingImpl.getTaintTrackingNoop() + + expect(noop).to.have.all.keys(Object.keys(ttDebug)) + }) + + it('should have the same properties as csiMethods', () => { + const tt = taintTrackingImpl.getTaintTrackingImpl() + + const csiExpectedMethods = getExpectedMethods() - expect(dummy).to.have.all.keys(Object.keys(tt)) + expect(tt).to.have.all.keys(csiExpectedMethods) }) }) }) diff --git a/packages/dd-trace/test/appsec/iast/telemetry/iast-metric.spec.js b/packages/dd-trace/test/appsec/iast/telemetry/iast-metric.spec.js new file mode 100644 index 00000000000..f862158ddf3 --- /dev/null +++ b/packages/dd-trace/test/appsec/iast/telemetry/iast-metric.spec.js @@ -0,0 +1,98 @@ +'use strict' + +const proxyquire = require('proxyquire') +const { + getExecutedMetric, + getInstrumentedMetric, + TagKey, + EXECUTED_SINK, + EXECUTED_SOURCE, + INSTRUMENTED_SINK, + INSTRUMENTED_SOURCE +} = require('../../../../src/appsec/iast/telemetry/iast-metric') + +describe('Metrics', () => { + let IastMetric, reqNamespace, inc, context + beforeEach(() => { + context = {} + inc = sinon.stub() + const metricMock = { inc } + + reqNamespace = { + count: sinon.stub().returns(metricMock) + } + + const metric = proxyquire('../../../../src/appsec/iast/telemetry/iast-metric', { + './namespaces': { + getNamespaceFromContext: () => reqNamespace + } + }) + IastMetric = metric.IastMetric + }) + + afterEach(sinon.restore) + + it('should increase by one the metric value', () => { + const metric = new IastMetric('test.metric', 'REQUEST') + + metric.inc(undefined, context) + + expect(reqNamespace.count).to.be.calledOnceWith(metric.name, undefined) + expect(inc).to.be.calledOnceWith(1) + }) + + it('should add by 42 the metric value', () => { + const metric = new IastMetric('test.metric', 'REQUEST', 'tagKey') + + metric.add(42, undefined, context) + + expect(reqNamespace.count).to.be.calledOnceWith(metric.name, undefined) + expect(inc).to.be.calledOnceWith(42) + }) + + it('should increase by one the metric tag value', () => { + const metric = new IastMetric('test.metric', 'REQUEST', 'tagKey') + + metric.inc('tag1', context) + + expect(reqNamespace.count).to.be.calledOnceWith(metric.name, { tagKey: 'tag1' }) + expect(inc).to.be.calledOnceWith(1) + }) + + it('should add by 42 the metric tag value', () => { + const metric = new IastMetric('test.metric', 'REQUEST', 'tagKey') + + metric.add(42, 'tag1', context) + + expect(reqNamespace.count).to.be.calledOnceWith(metric.name, { tagKey: 'tag1' }) + expect(inc).to.be.calledOnceWith(42) + }) + + it('should add by 42 the each metric tag value', () => { + const metric = new IastMetric('test.metric', 'REQUEST', 'tagKey') + + metric.add(42, ['tag1', 'tag2'], context) + + expect(reqNamespace.count).to.be.calledTwice + expect(reqNamespace.count.firstCall.args).to.be.deep.equals([metric.name, { tagKey: 'tag1' }]) + expect(reqNamespace.count.secondCall.args).to.be.deep.equals([metric.name, { tagKey: 'tag2' }]) + }) + + it('getExecutedMetric should return a metric depending on tag', () => { + let metric = getExecutedMetric(TagKey.VULNERABILITY_TYPE) + + expect(metric).to.be.equal(EXECUTED_SINK) + + metric = getExecutedMetric(TagKey.SOURCE_TYPE) + expect(metric).to.be.equal(EXECUTED_SOURCE) + }) + + it('getInstrumentedMetric should return a metric depending on tag', () => { + let metric = getInstrumentedMetric(TagKey.VULNERABILITY_TYPE) + + expect(metric).to.be.equal(INSTRUMENTED_SINK) + + metric = getInstrumentedMetric(TagKey.SOURCE_TYPE) + expect(metric).to.be.equal(INSTRUMENTED_SOURCE) + }) +}) diff --git a/packages/dd-trace/test/appsec/iast/telemetry/index.spec.js b/packages/dd-trace/test/appsec/iast/telemetry/index.spec.js new file mode 100644 index 00000000000..60b80abda62 --- /dev/null +++ b/packages/dd-trace/test/appsec/iast/telemetry/index.spec.js @@ -0,0 +1,172 @@ +'use strict' + +const { expect } = require('chai') +const proxyquire = require('proxyquire') +const { Verbosity } = require('../../../../src/appsec/iast/telemetry/verbosity') + +describe('Telemetry', () => { + let defaultConfig + let telemetryMetrics + let iastTelemetry + let telemetryLogs + let initRequestNamespace + let finalizeRequestNamespace + + beforeEach(() => { + defaultConfig = { + telemetry: { + enabled: true, + metrics: true + } + } + + telemetryLogs = { + registerProvider: () => telemetryLogs, + start: sinon.spy(), + stop: sinon.spy() + } + + telemetryMetrics = { + manager: { + set: sinon.spy(), + delete: sinon.spy() + } + } + + initRequestNamespace = sinon.spy() + finalizeRequestNamespace = sinon.spy() + + iastTelemetry = proxyquire('../../../../src/appsec/iast/telemetry', { + './log': telemetryLogs, + '../../../telemetry/metrics': telemetryMetrics, + './namespaces': { + initRequestNamespace, + finalizeRequestNamespace + } + }) + }) + + afterEach(() => { + sinon.restore() + }) + + describe('configure', () => { + it('should set default verbosity', () => { + iastTelemetry.configure(defaultConfig) + + expect(iastTelemetry.enabled).to.be.true + expect(iastTelemetry.verbosity).to.be.eq(Verbosity.INFORMATION) + expect(telemetryLogs.start).to.be.calledOnce + }) + + it('should set OFF verbosity if not enabled', () => { + defaultConfig.telemetry.enabled = false + iastTelemetry.configure(defaultConfig) + + expect(iastTelemetry.enabled).to.be.false + expect(iastTelemetry.verbosity).to.be.eq(Verbosity.OFF) + expect(telemetryLogs.start).to.be.called + }) + + it('should init metrics even if verbosity is OFF', () => { + const iastTelemetry = proxyquire('../../../../src/appsec/iast/telemetry', { + './log': telemetryLogs, + '../../../telemetry/metrics': telemetryMetrics, + './verbosity': { + getVerbosity: () => Verbosity.OFF + } + }) + + const telemetryConfig = { enabled: true, metrics: true } + iastTelemetry.configure({ + telemetry: telemetryConfig + }) + + expect(iastTelemetry.enabled).to.be.true + expect(iastTelemetry.verbosity).to.be.eq(Verbosity.OFF) + expect(telemetryMetrics.manager.set).to.be.calledOnce + expect(telemetryLogs.start).to.be.calledOnce + }) + + it('should not init metrics if metrics not enabled', () => { + const telemetryConfig = { enabled: true, metrics: false } + iastTelemetry.configure({ + telemetry: telemetryConfig + }) + + expect(iastTelemetry.enabled).to.be.false + expect(iastTelemetry.verbosity).to.be.eq(Verbosity.OFF) + expect(telemetryMetrics.manager.set).to.not.be.called + expect(telemetryLogs.start).to.be.calledOnce + }) + }) + + describe('stop', () => { + it('should set enabled = false and unregister provider', () => { + iastTelemetry.configure(defaultConfig) + + iastTelemetry.stop() + expect(iastTelemetry.enabled).to.be.false + expect(telemetryMetrics.manager.delete).to.be.calledOnce + expect(telemetryLogs.stop).to.be.calledOnce + }) + }) + + describe('onRequestStarted', () => { + it('should call init if enabled and verbosity is not Off', () => { + iastTelemetry.configure(defaultConfig) + + const iastContext = {} + iastTelemetry.onRequestStarted(iastContext) + + expect(initRequestNamespace).to.be.calledOnceWith(iastContext) + }) + + it('should not call init if enabled and verbosity is Off', () => { + const iastTelemetry = proxyquire('../../../../src/appsec/iast/telemetry', { + '../../../telemetry/metrics': telemetryMetrics, + './log': telemetryLogs, + './verbosity': { + getVerbosity: () => Verbosity.OFF + } + }) + iastTelemetry.configure({ + telemetry: { enabled: true } + }) + + const iastContext = {} + iastTelemetry.onRequestStarted(iastContext) + + expect(initRequestNamespace).to.not.be.calledOnce + }) + }) + + describe('onRequestEnd', () => { + it('should call finalizeRequestNamespace if enabled and verbosity is not Off', () => { + iastTelemetry.configure(defaultConfig) + + const iastContext = {} + iastTelemetry.onRequestEnd(iastContext) + + expect(finalizeRequestNamespace).to.be.calledOnceWith(iastContext) + }) + + it('should not call finalizeRequestNamespace if enabled and verbosity is Off', () => { + const iastTelemetry = proxyquire('../../../../src/appsec/iast/telemetry', { + '../../../telemetry/metrics': telemetryMetrics, + './log': telemetryLogs, + './verbosity': { + getVerbosity: () => Verbosity.OFF + } + }) + iastTelemetry.configure({ + telemetry: { enabled: true } + }) + + const iastContext = {} + iastTelemetry.onRequestEnd(iastContext) + + expect(finalizeRequestNamespace).to.not.be.calledOnce + }) + }) +}) diff --git a/packages/dd-trace/test/appsec/iast/telemetry/logs.spec.js b/packages/dd-trace/test/appsec/iast/telemetry/log/index.spec.js similarity index 77% rename from packages/dd-trace/test/appsec/iast/telemetry/logs.spec.js rename to packages/dd-trace/test/appsec/iast/telemetry/log/index.spec.js index 8c5e6d82cc2..6d3dc17b01c 100644 --- a/packages/dd-trace/test/appsec/iast/telemetry/logs.spec.js +++ b/packages/dd-trace/test/appsec/iast/telemetry/log/index.spec.js @@ -48,8 +48,8 @@ describe('telemetry logs', () => { describe('start', () => { it('should be enabled by default and subscribe', () => { - const logs = proxyquire('../../../../src/appsec/iast/telemetry/logs', { - '../../../../../diagnostics_channel': dc + const logs = proxyquire('../../../../../src/appsec/iast/telemetry/log', { + '../../../../../../diagnostics_channel': dc }) logs.start() defaultConfig.telemetry.logCollection = true @@ -60,8 +60,8 @@ describe('telemetry logs', () => { }) it('should be disabled and not subscribe if DD_TELEMETRY_LOG_COLLECTION_ENABLED = false', () => { - const logs = proxyquire('../../../../src/appsec/iast/telemetry/logs', { - '../../../../../diagnostics_channel': dc + const logs = proxyquire('../../../../../src/appsec/iast/telemetry/log', { + '../../../../../../diagnostics_channel': dc }) logs.start() @@ -75,10 +75,10 @@ describe('telemetry logs', () => { const sendData = sinon.stub() let logCollectorCalled = 0 - const logs = proxyquire('../../../../src/appsec/iast/telemetry/logs', { - '../../../../../diagnostics_channel': dc, - '../../../telemetry/send-data': { sendData }, - './log_collector': { + const logs = proxyquire('../../../../../src/appsec/iast/telemetry/log', { + '../../../../../../diagnostics_channel': dc, + '../../../../telemetry/send-data': { sendData }, + './log-collector': { drain: () => { logCollectorCalled++ return { message: 'Error 1', level: 'ERROR' } @@ -104,8 +104,8 @@ describe('telemetry logs', () => { describe('stop', () => { it('should unsubscribe configured listeners', () => { - const logs = proxyquire('../../../../src/appsec/iast/telemetry/logs', { - '../../../../../diagnostics_channel': dc + const logs = proxyquire('../../../../../src/appsec/iast/telemetry/log', { + '../../../../../../diagnostics_channel': dc }) logs.start() onTelemetryStart()(onTelemetryStartMsg) @@ -117,8 +117,8 @@ describe('telemetry logs', () => { }) it('should unsubscribe configured listeners when datadog:telemetry:stop is received', () => { - const logs = proxyquire('../../../../src/appsec/iast/telemetry/logs', { - '../../../../../diagnostics_channel': dc + const logs = proxyquire('../../../../../src/appsec/iast/telemetry/log', { + '../../../../../../diagnostics_channel': dc }) logs.start() onTelemetryStart()(onTelemetryStartMsg) @@ -133,9 +133,9 @@ describe('telemetry logs', () => { describe('sendData', () => { it('should be not called with DEBUG level', () => { const logCollectorAdd = sinon.stub() - const logs = proxyquire('../../../../src/appsec/iast/telemetry/logs', { - '../../../../../diagnostics_channel': dc, - './log_collector': { + const logs = proxyquire('../../../../../src/appsec/iast/telemetry/log', { + '../../../../../../diagnostics_channel': dc, + './log-collector': { add: logCollectorAdd } }) @@ -149,9 +149,9 @@ describe('telemetry logs', () => { it('should be called with WARN level', () => { const logCollectorAdd = sinon.stub() - const logs = proxyquire('../../../../src/appsec/iast/telemetry/logs', { - '../../../../../diagnostics_channel': dc, - './log_collector': { + const logs = proxyquire('../../../../../src/appsec/iast/telemetry/log', { + '../../../../../../diagnostics_channel': dc, + './log-collector': { add: logCollectorAdd } }) @@ -165,9 +165,9 @@ describe('telemetry logs', () => { it('should be called with ERROR level', () => { const logCollectorAdd = sinon.stub() - const logs = proxyquire('../../../../src/appsec/iast/telemetry/logs', { - '../../../../../diagnostics_channel': dc, - './log_collector': { + const logs = proxyquire('../../../../../src/appsec/iast/telemetry/log', { + '../../../../../../diagnostics_channel': dc, + './log-collector': { add: logCollectorAdd } }) @@ -181,9 +181,9 @@ describe('telemetry logs', () => { it('should be called with ERROR level and stack_trace', () => { const logCollectorAdd = sinon.stub() - const logs = proxyquire('../../../../src/appsec/iast/telemetry/logs', { - '../../../../../diagnostics_channel': dc, - './log_collector': { + const logs = proxyquire('../../../../../src/appsec/iast/telemetry/log', { + '../../../../../../diagnostics_channel': dc, + './log-collector': { add: logCollectorAdd } }) diff --git a/packages/dd-trace/test/appsec/iast/telemetry/log_collector.spec.js b/packages/dd-trace/test/appsec/iast/telemetry/log/log-collector.spec.js similarity index 86% rename from packages/dd-trace/test/appsec/iast/telemetry/log_collector.spec.js rename to packages/dd-trace/test/appsec/iast/telemetry/log/log-collector.spec.js index 59c5ec3fe68..dde5f6afb65 100644 --- a/packages/dd-trace/test/appsec/iast/telemetry/log_collector.spec.js +++ b/packages/dd-trace/test/appsec/iast/telemetry/log/log-collector.spec.js @@ -1,10 +1,10 @@ const { expect } = require('chai') -const { calculateDDBasePath } = require('../../../../src/util') +const { calculateDDBasePath } = require('../../../../../src/util') const ddBasePath = calculateDDBasePath(__dirname) describe('telemetry log collector', () => { - const logCollector = require('../../../../src/appsec/iast/telemetry/log_collector') + const logCollector = require('../../../../../src/appsec/iast/telemetry/log/log-collector') afterEach(() => { logCollector.reset(3) @@ -24,14 +24,16 @@ describe('telemetry log collector', () => { }) it('should store logs with same message but different stack', () => { - const ddFrame = `at T (${ddBasePath}packages/dd-trace/test/appsec/iast/telemetry/log_collector.spec.js:29:21)` + const ddFrame = + `at T (${ddBasePath}packages/dd-trace/test/appsec/iast/telemetry/log/log-collector.spec.js:29:21)` expect(logCollector.add({ message: 'Error 1', level: 'ERROR', stack_trace: `stack 1\n${ddFrame}` })).to.be.true expect(logCollector.add({ message: 'Error 1', level: 'ERROR', stack_trace: `stack 2\n${ddFrame}` })).to.be.true expect(logCollector.add({ message: 'Error 1', level: 'ERROR', stack_trace: `stack 3\n${ddFrame}` })).to.be.true }) it('should store logs with same message, same stack but different level', () => { - const ddFrame = `at T (${ddBasePath}packages/dd-trace/test/appsec/iast/telemetry/log_collector.spec.js:29:21)` + const ddFrame = + `at T (${ddBasePath}packages/dd-trace/test/appsec/iast/telemetry/log/log-collector.spec.js:29:21)` expect(logCollector.add({ message: 'Error 1', level: 'ERROR', stack_trace: `stack 1\n${ddFrame}` })).to.be.true expect(logCollector.add({ message: 'Error 1', level: 'WARN', stack_trace: `stack 1\n${ddFrame}` })).to.be.true expect(logCollector.add({ message: 'Error 1', level: 'DEBUG', stack_trace: `stack 1\n${ddFrame}` })).to.be.true diff --git a/packages/dd-trace/test/appsec/iast/telemetry/namespaces.spec.js b/packages/dd-trace/test/appsec/iast/telemetry/namespaces.spec.js new file mode 100644 index 00000000000..57317ada523 --- /dev/null +++ b/packages/dd-trace/test/appsec/iast/telemetry/namespaces.spec.js @@ -0,0 +1,89 @@ +'use strict' + +const { expect } = require('chai') +const { + initRequestNamespace, + finalizeRequestNamespace, + DD_IAST_METRICS_NAMESPACE, + globalNamespace +} = require('../../../../src/appsec/iast/telemetry/namespaces') + +const REQUEST_TAINTED = 'request.tainted' +const EXECUTED_SINK = 'executed.sink' +const TAG_PREFIX = '_dd.iast.telemetry' + +function now () { + return Date.now() / 1e3 +} + +describe('IAST metric namespaces', () => { + let context, namespace, rootSpan + + beforeEach(() => { + rootSpan = { + addTags: sinon.spy() + } + context = {} + namespace = initRequestNamespace(context) + }) + + afterEach(() => { + sinon.restore() + }) + + it('should set a rootSpan tag with the flattened value of the metric', () => { + namespace.metrics.set(REQUEST_TAINTED, { + metric: REQUEST_TAINTED, + points: [[now(), 5], [now(), 5]] + }) + + finalizeRequestNamespace(context, rootSpan) + + expect(rootSpan.addTags).to.be.called + + const tag = rootSpan.addTags.getCalls()[0].args[0] + expect(tag).to.has.property(`${TAG_PREFIX}.${REQUEST_TAINTED}`) + expect(tag[`${TAG_PREFIX}.${REQUEST_TAINTED}`]).to.be.eq(10) + + expect(context[DD_IAST_METRICS_NAMESPACE]).to.be.undefined + }) + + it('should set as many rootSpan tags as different request scoped metrics', () => { + namespace.count(REQUEST_TAINTED).inc(10) + namespace.count(EXECUTED_SINK).inc(1) + namespace.count(REQUEST_TAINTED).inc(5) + + finalizeRequestNamespace(context, rootSpan) + + expect(rootSpan.addTags).to.be.called + + const calls = rootSpan.addTags.getCalls() + const reqTaintedTag = calls[0].args[0] + expect(reqTaintedTag).to.has.property(`${TAG_PREFIX}.${REQUEST_TAINTED}`) + expect(reqTaintedTag[`${TAG_PREFIX}.${REQUEST_TAINTED}`]).to.be.eq(15) + + const execSinkTag = calls[1].args[0] + expect(execSinkTag).to.has.property(`${TAG_PREFIX}.${EXECUTED_SINK}`) + expect(execSinkTag[`${TAG_PREFIX}.${EXECUTED_SINK}`]).to.be.eq(1) + }) + + it('should merge all kind of metrics in global Namespace as gauges', () => { + namespace.count(REQUEST_TAINTED, { tag1: 'test' }).inc(10) + namespace.count(EXECUTED_SINK).inc(1) + + const metric = { + inc: sinon.spy() + } + sinon.stub(globalNamespace, 'count').returns(metric) + + finalizeRequestNamespace(context, rootSpan) + + expect(globalNamespace.count).to.be.calledTwice + expect(globalNamespace.count.firstCall.args).to.be.deep.equal([REQUEST_TAINTED, ['tag1:test']]) + expect(metric.inc).to.be.calledTwice + expect(metric.inc.firstCall.args[0]).to.equal(10) + + expect(globalNamespace.count.secondCall.args).to.be.deep.equal([EXECUTED_SINK, []]) + expect(metric.inc.secondCall.args[0]).to.equal(1) + }) +}) diff --git a/packages/dd-trace/test/appsec/iast/telemetry/span-tags.spec.js b/packages/dd-trace/test/appsec/iast/telemetry/span-tags.spec.js new file mode 100644 index 00000000000..796e06985e2 --- /dev/null +++ b/packages/dd-trace/test/appsec/iast/telemetry/span-tags.spec.js @@ -0,0 +1,78 @@ +'use strict' + +const { expect } = require('chai') +const { EXECUTED_SINK, EXECUTED_SOURCE, REQUEST_TAINTED } = require('../../../../src/appsec/iast/telemetry/iast-metric') +const { addMetricsToSpan } = require('../../../../src/appsec/iast/telemetry/span-tags') +const { + getNamespaceFromContext, + initRequestNamespace, + globalNamespace +} = require('../../../../src/appsec/iast/telemetry/namespaces') + +describe('Telemetry Span tags', () => { + const tagPrefix = '_dd.test' + let rootSpan, context + + beforeEach(() => { + rootSpan = { + addTags: sinon.spy() + } + context = {} + initRequestNamespace(context) + + globalNamespace.metrics.clear() + }) + + afterEach(sinon.restore) + + it('should add span tags with tag name like \'tagPrefix.metricName.tagKey\' for tagged metrics', () => { + EXECUTED_SOURCE.add(42, 'source.type.1', context) + EXECUTED_SINK.add(3, 'sink_type_1', context) + + const { metrics } = getNamespaceFromContext(context).toJSON() + + addMetricsToSpan(rootSpan, metrics.series, tagPrefix) + + expect(rootSpan.addTags).to.be.calledTwice + expect(rootSpan.addTags.firstCall.args[0]).to.deep.eq({ '_dd.test.executed.source.source_type_1': 42 }) + expect(rootSpan.addTags.secondCall.args[0]).to.deep.eq({ '_dd.test.executed.sink.sink_type_1': 3 }) + }) + + it('should add span tags with tag name like \'tagPrefix.metricName.tagKey\' for tagged metrics flattened', () => { + // a request metric with no context it behaves like a global metric + EXECUTED_SOURCE.add(42, 'source.type.1') + EXECUTED_SOURCE.add(32, 'source.type.1') + + const { metrics } = globalNamespace.toJSON() + + addMetricsToSpan(rootSpan, metrics.series, tagPrefix) + + expect(rootSpan.addTags).to.be.calledOnceWithExactly({ '_dd.test.executed.source.source_type_1': 74 }) + }) + + it('should add span tags with tag name like \'tagPrefix.metricName.tagKey\' for different tagged metrics', () => { + // a request metric with no context it behaves like a global metric + EXECUTED_SOURCE.add(42, 'source.type.1') + EXECUTED_SOURCE.add(32, 'source.type.1') + + EXECUTED_SOURCE.add(2, 'source.type.2') + + const { metrics } = globalNamespace.toJSON() + + addMetricsToSpan(rootSpan, metrics.series, tagPrefix) + + expect(rootSpan.addTags).to.be.calledTwice + expect(rootSpan.addTags.firstCall.args[0]).to.deep.eq({ '_dd.test.executed.source.source_type_1': 74 }) + expect(rootSpan.addTags.secondCall.args[0]).to.deep.eq({ '_dd.test.executed.source.source_type_2': 2 }) + }) + + it('should add span tags with tag name like \'tagPrefix.metricName\' for not tagged metrics', () => { + REQUEST_TAINTED.add(42, null, context) + + const { metrics } = getNamespaceFromContext(context).toJSON() + + addMetricsToSpan(rootSpan, metrics.series, tagPrefix) + + expect(rootSpan.addTags).to.be.calledOnceWithExactly({ '_dd.test.request.tainted': 42 }) + }) +}) diff --git a/packages/dd-trace/test/appsec/iast/telemetry/verbosity.spec.js b/packages/dd-trace/test/appsec/iast/telemetry/verbosity.spec.js new file mode 100644 index 00000000000..37d2e0b939d --- /dev/null +++ b/packages/dd-trace/test/appsec/iast/telemetry/verbosity.spec.js @@ -0,0 +1,49 @@ +'use strict' + +const { expect } = require('chai') +const { getVerbosity, getName, Verbosity, isDebugAllowed, isInfoAllowed } = + require('../../../../src/appsec/iast/telemetry/verbosity') + +describe('Telemetry Verbosity', () => { + describe('getVerbosity', () => { + beforeEach(() => { + const path = require.resolve('../../../../src/appsec/iast/telemetry/verbosity') + delete require.cache[path] + }) + + it('should get verbosity regardless of capitalization', () => { + expect(getVerbosity('dEBug')).to.be.eq(Verbosity.DEBUG) + }) + + it('should get verbosity default verbosity if invalid env var', () => { + expect(getVerbosity('Invalid')).to.be.eq(Verbosity.INFORMATION) + }) + + it('should get verbosity default verbosity if empty env var', () => { + expect(getVerbosity()).to.be.eq(Verbosity.INFORMATION) + }) + }) + + describe('getName and others', () => { + it('should obtain name from verbosity', () => { + expect(getName(Verbosity.DEBUG)).to.be.equal('DEBUG') + expect(getName(Verbosity.INFORMATION)).to.be.equal('INFORMATION') + expect(getName(Verbosity.MANDATORY)).to.be.equal('MANDATORY') + expect(getName(Verbosity.OFF)).to.be.equal('OFF') + }) + + it('should handle debug verbosity level', () => { + expect(isDebugAllowed(Verbosity.OFF)).to.be.false + expect(isDebugAllowed(Verbosity.MANDATORY)).to.be.false + expect(isDebugAllowed(Verbosity.INFORMATION)).to.be.false + expect(isDebugAllowed(Verbosity.DEBUG)).to.be.true + }) + + it('should handle info verbosity level', () => { + expect(isInfoAllowed(Verbosity.OFF)).to.be.false + expect(isInfoAllowed(Verbosity.MANDATORY)).to.be.false + expect(isInfoAllowed(Verbosity.INFORMATION)).to.be.true + expect(isInfoAllowed(Verbosity.DEBUG)).to.be.true + }) + }) +}) diff --git a/packages/dd-trace/test/config.spec.js b/packages/dd-trace/test/config.spec.js index cffcfe60cea..71ea7c7c1c8 100644 --- a/packages/dd-trace/test/config.spec.js +++ b/packages/dd-trace/test/config.spec.js @@ -115,6 +115,7 @@ describe('Config', () => { expect(config).to.have.nested.property('remoteConfig.pollInterval', 5) expect(config).to.have.nested.property('iast.enabled', false) expect(config).to.have.nested.property('iast.redactionEnabled', true) + expect(config).to.have.nested.property('iast.telemetryVerbosity', 'INFORMATION') }) it('should support logging', () => { @@ -211,6 +212,7 @@ describe('Config', () => { process.env.DD_IAST_MAX_CONTEXT_OPERATIONS = '4' process.env.DD_IAST_DEDUPLICATION_ENABLED = false process.env.DD_IAST_REDACTION_ENABLED = false + process.env.DD_IAST_TELEMETRY_VERBOSITY = 'DEBUG' process.env.DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED = 'true' process.env.DD_TRACE_128_BIT_TRACEID_LOGGING_ENABLED = 'true' @@ -280,6 +282,7 @@ describe('Config', () => { expect(config).to.have.nested.property('iast.maxContextOperations', 4) expect(config).to.have.nested.property('iast.deduplicationEnabled', false) expect(config).to.have.nested.property('iast.redactionEnabled', false) + expect(config).to.have.nested.property('iast.telemetryVerbosity', 'DEBUG') }) it('should read case-insensitive booleans from environment variables', () => { @@ -392,7 +395,8 @@ describe('Config', () => { maxConcurrentRequests: 4, maxContextOperations: 5, deduplicationEnabled: false, - redactionEnabled: false + redactionEnabled: false, + telemetryVerbosity: 'DEBUG' } }, appsec: false, @@ -445,6 +449,7 @@ describe('Config', () => { expect(config).to.have.nested.property('iast.maxContextOperations', 5) expect(config).to.have.nested.property('iast.deduplicationEnabled', false) expect(config).to.have.nested.property('iast.redactionEnabled', false) + expect(config).to.have.nested.property('iast.telemetryVerbosity', 'DEBUG') expect(config).to.have.deep.nested.property('sampler', { sampleRate: 0.5, rateLimit: 1000, @@ -849,6 +854,7 @@ describe('Config', () => { expect(config.telemetry.heartbeatInterval).to.eq(60000) expect(config.telemetry.logCollection).to.be.false expect(config.telemetry.debug).to.be.false + expect(config.telemetry.metrics).to.be.false }) it('should set DD_TELEMETRY_HEARTBEAT_INTERVAL', () => { @@ -873,6 +879,17 @@ describe('Config', () => { process.env.DD_TRACE_TELEMETRY_ENABLED = origTraceTelemetryValue }) + it('should set DD_TELEMETRY_METRICS_ENABLED', () => { + const origTelemetryMetricsEnabledValue = process.env.DD_TELEMETRY_METRICS_ENABLED + process.env.DD_TELEMETRY_METRICS_ENABLED = 'true' + + const config = new Config() + + expect(config.telemetry.metrics).to.be.true + + process.env.DD_TELEMETRY_METRICS_ENABLED = origTelemetryMetricsEnabledValue + }) + it('should set DD_TELEMETRY_LOG_COLLECTION_ENABLED = false', () => { const origLogCollectionValue = process.env.DD_TELEMETRY_LOG_COLLECTION_ENABLED process.env.DD_TELEMETRY_LOG_COLLECTION_ENABLED = 'false' diff --git a/packages/dd-trace/test/telemetry/dependencies.spec.js b/packages/dd-trace/test/telemetry/dependencies.spec.js index 1b0a304ec60..5ccaa6d18cf 100644 --- a/packages/dd-trace/test/telemetry/dependencies.spec.js +++ b/packages/dd-trace/test/telemetry/dependencies.spec.js @@ -41,6 +41,7 @@ describe('dependencies', () => { afterEach(() => { dependencies.stop() + sendData.reset() global.setImmediate = originalSetImmediate }) From ad67636be864ff8d747be68e93914303dc49d588 Mon Sep 17 00:00:00 2001 From: Ugaitz Urien Date: Mon, 3 Jul 2023 15:55:34 +0200 Subject: [PATCH 2/7] Small changes --- .../src/appsec/iast/analyzers/weak-hash-analyzer.js | 3 +-- packages/dd-trace/src/appsec/iast/iast-plugin.js | 7 +++++-- packages/dd-trace/src/appsec/iast/index.js | 2 +- packages/dd-trace/src/appsec/iast/telemetry/index.js | 2 +- packages/dd-trace/test/appsec/iast/telemetry/index.spec.js | 6 +++--- 5 files changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/dd-trace/src/appsec/iast/analyzers/weak-hash-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/weak-hash-analyzer.js index 4b38913caea..8682529201b 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/weak-hash-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/weak-hash-analyzer.js @@ -28,8 +28,7 @@ class WeakHashAnalyzer extends Analyzer { } onConfigure () { - this.addSub('datadog:crypto:hashing:start', ({ algorithm }) => this.analyze(algorithm) - ) + this.addSub('datadog:crypto:hashing:start', ({ algorithm }) => this.analyze(algorithm)) } _isVulnerable (algorithm) { diff --git a/packages/dd-trace/src/appsec/iast/iast-plugin.js b/packages/dd-trace/src/appsec/iast/iast-plugin.js index 6b23440d984..e70c0d82b5f 100644 --- a/packages/dd-trace/src/appsec/iast/iast-plugin.js +++ b/packages/dd-trace/src/appsec/iast/iast-plugin.js @@ -104,13 +104,16 @@ class IastPlugin extends Plugin { onConfigure () {} configure (config) { - if (config && !this.configured) { + if (typeof config !== 'object') { + config = { enabled: config } + } + if (config.enabled && !this.configured) { this.onConfigure() this.configured = true } if (iastTelemetry.isEnabled()) { - if (config) { + if (config.enabled) { this.enableTelemetry() } else { this.disableTelemetry() diff --git a/packages/dd-trace/src/appsec/iast/index.js b/packages/dd-trace/src/appsec/iast/index.js index 944c0b4ec07..dc4c4fd2930 100644 --- a/packages/dd-trace/src/appsec/iast/index.js +++ b/packages/dd-trace/src/appsec/iast/index.js @@ -54,7 +54,7 @@ function onIncomingHttpRequestStart (data) { createTransaction(rootSpan.context().toSpanId(), iastContext) overheadController.initializeRequestContext(iastContext) taintTrackingPlugin.taintHeaders(data.req.headers, iastContext) - iastTelemetry.onRequestStarted(iastContext) + iastTelemetry.onRequestStart(iastContext) } if (rootSpan.addTags) { rootSpan.addTags({ diff --git a/packages/dd-trace/src/appsec/iast/telemetry/index.js b/packages/dd-trace/src/appsec/iast/telemetry/index.js index 83dd274d043..ec0a6700d0a 100644 --- a/packages/dd-trace/src/appsec/iast/telemetry/index.js +++ b/packages/dd-trace/src/appsec/iast/telemetry/index.js @@ -37,7 +37,7 @@ class Telemetry { return this.isEnabled() && isInfoAllowed(this.verbosity) } - onRequestStarted (context) { + onRequestStart (context) { if (this.isEnabled() && this.verbosity !== Verbosity.OFF) { initRequestNamespace(context) } diff --git a/packages/dd-trace/test/appsec/iast/telemetry/index.spec.js b/packages/dd-trace/test/appsec/iast/telemetry/index.spec.js index 60b80abda62..a70c3457db4 100644 --- a/packages/dd-trace/test/appsec/iast/telemetry/index.spec.js +++ b/packages/dd-trace/test/appsec/iast/telemetry/index.spec.js @@ -112,12 +112,12 @@ describe('Telemetry', () => { }) }) - describe('onRequestStarted', () => { + describe('onRequestStart', () => { it('should call init if enabled and verbosity is not Off', () => { iastTelemetry.configure(defaultConfig) const iastContext = {} - iastTelemetry.onRequestStarted(iastContext) + iastTelemetry.onRequestStart(iastContext) expect(initRequestNamespace).to.be.calledOnceWith(iastContext) }) @@ -135,7 +135,7 @@ describe('Telemetry', () => { }) const iastContext = {} - iastTelemetry.onRequestStarted(iastContext) + iastTelemetry.onRequestStart(iastContext) expect(initRequestNamespace).to.not.be.calledOnce }) From 333a99a9ff1c90bd6c3f59e3ca82b4e42d5f60c7 Mon Sep 17 00:00:00 2001 From: Ugaitz Urien Date: Tue, 4 Jul 2023 16:40:08 +0200 Subject: [PATCH 3/7] Restore iastContext to undefined in a single test --- .../test/appsec/iast/analyzers/vulnerability-analyzer.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js index 0a9392bd5c6..c33f00eec54 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js @@ -66,7 +66,7 @@ describe('vulnerability-analyzer', () => { reported: false }, { - iastContext: {}, + iastContext: undefined, oceResult: true, isVulnerable: true, reported: true From 6e5cfa7b5906169efa27cb8e78da8a0be163262a Mon Sep 17 00:00:00 2001 From: Ugaitz Urien Date: Tue, 4 Jul 2023 17:56:28 +0200 Subject: [PATCH 4/7] Small fixes --- .../iast/analyzers/vulnerability-analyzer.spec.js | 4 ++-- .../dd-trace/test/appsec/iast/iast-plugin.spec.js | 11 ++++------- .../iast/taint-tracking/rewriter-telemetry.spec.js | 4 +++- .../dd-trace/test/appsec/iast/telemetry/index.spec.js | 8 ++++---- .../test/appsec/iast/telemetry/namespaces.spec.js | 1 - 5 files changed, 13 insertions(+), 15 deletions(-) diff --git a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js index c33f00eec54..9bddf81e861 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js @@ -141,8 +141,8 @@ describe('vulnerability-analyzer', () => { const name = 'test' expect(() => wrapped(message, name)).to.not.throw() const args = handler.firstCall.args - expect(args[0]).to.be.eq(message) - expect(args[1]).to.be.eq(name) + expect(args[0]).to.be.equal(message) + expect(args[1]).to.be.equal(name) }) it('should catch thrown Errors inside subscription handlers', () => { diff --git a/packages/dd-trace/test/appsec/iast/iast-plugin.spec.js b/packages/dd-trace/test/appsec/iast/iast-plugin.spec.js index 2e36a4bb1b4..ce0109ac2a7 100644 --- a/packages/dd-trace/test/appsec/iast/iast-plugin.spec.js +++ b/packages/dd-trace/test/appsec/iast/iast-plugin.spec.js @@ -32,8 +32,6 @@ describe('IAST Plugin', () => { }) describe('with appsec telemetry disabled', () => { - let IastPlugin - beforeEach(() => { class PluginClass { addSub (channelName, handler) { @@ -64,8 +62,7 @@ describe('IAST Plugin', () => { './telemetry/metrics': {}, '../../../../datadog-core': datadogCore }) - IastPlugin = iastPluginMod.IastPlugin - iastPlugin = new IastPlugin() + iastPlugin = new iastPluginMod.IastPlugin() }) afterEach(() => { @@ -174,6 +171,7 @@ describe('IAST Plugin', () => { const metric = { increase: sinon.spy() } + iastPlugin._execHandlerAndIncMetric({ handler, metric @@ -187,7 +185,6 @@ describe('IAST Plugin', () => { describe('with appsec telemetry enabled', () => { let iastTelemetry - let IastPlugin beforeEach(() => { class PluginClass { @@ -202,7 +199,7 @@ describe('IAST Plugin', () => { isEnabled: () => true, isDebugEnabled: () => true } - IastPlugin = proxyquire('../../../src/appsec/iast/iast-plugin', { + const IastPlugin = proxyquire('../../../src/appsec/iast/iast-plugin', { '../../plugins/plugin': PluginClass, './iast-log': { errorAndPublish: logError @@ -228,7 +225,7 @@ describe('IAST Plugin', () => { loadChannel.publish({ name: 'test' }) - expect(onInstrumentationLoadedMock).to.be.calledWith('test') + expect(onInstrumentationLoadedMock).to.be.calledOnceWith('test') }) }) diff --git a/packages/dd-trace/test/appsec/iast/taint-tracking/rewriter-telemetry.spec.js b/packages/dd-trace/test/appsec/iast/taint-tracking/rewriter-telemetry.spec.js index 0bb6b72316e..05cdbd9c8b3 100644 --- a/packages/dd-trace/test/appsec/iast/taint-tracking/rewriter-telemetry.spec.js +++ b/packages/dd-trace/test/appsec/iast/taint-tracking/rewriter-telemetry.spec.js @@ -30,7 +30,9 @@ describe('rewriter telemetry', () => { instrumentedPropagationAdd = sinon.stub(INSTRUMENTED_PROPAGATION, 'add') }) - afterEach(sinon.restore) + afterEach(() => { + sinon.restore() + }) it('should not increase any metrics with OFF verbosity', () => { iastTelemetry.verbosity = Verbosity.OFF diff --git a/packages/dd-trace/test/appsec/iast/telemetry/index.spec.js b/packages/dd-trace/test/appsec/iast/telemetry/index.spec.js index a70c3457db4..012176057b0 100644 --- a/packages/dd-trace/test/appsec/iast/telemetry/index.spec.js +++ b/packages/dd-trace/test/appsec/iast/telemetry/index.spec.js @@ -55,7 +55,7 @@ describe('Telemetry', () => { iastTelemetry.configure(defaultConfig) expect(iastTelemetry.enabled).to.be.true - expect(iastTelemetry.verbosity).to.be.eq(Verbosity.INFORMATION) + expect(iastTelemetry.verbosity).to.be.equal(Verbosity.INFORMATION) expect(telemetryLogs.start).to.be.calledOnce }) @@ -64,7 +64,7 @@ describe('Telemetry', () => { iastTelemetry.configure(defaultConfig) expect(iastTelemetry.enabled).to.be.false - expect(iastTelemetry.verbosity).to.be.eq(Verbosity.OFF) + expect(iastTelemetry.verbosity).to.be.equal(Verbosity.OFF) expect(telemetryLogs.start).to.be.called }) @@ -83,7 +83,7 @@ describe('Telemetry', () => { }) expect(iastTelemetry.enabled).to.be.true - expect(iastTelemetry.verbosity).to.be.eq(Verbosity.OFF) + expect(iastTelemetry.verbosity).to.be.equal(Verbosity.OFF) expect(telemetryMetrics.manager.set).to.be.calledOnce expect(telemetryLogs.start).to.be.calledOnce }) @@ -95,7 +95,7 @@ describe('Telemetry', () => { }) expect(iastTelemetry.enabled).to.be.false - expect(iastTelemetry.verbosity).to.be.eq(Verbosity.OFF) + expect(iastTelemetry.verbosity).to.be.equal(Verbosity.OFF) expect(telemetryMetrics.manager.set).to.not.be.called expect(telemetryLogs.start).to.be.calledOnce }) diff --git a/packages/dd-trace/test/appsec/iast/telemetry/namespaces.spec.js b/packages/dd-trace/test/appsec/iast/telemetry/namespaces.spec.js index 57317ada523..4b2e6a5ca78 100644 --- a/packages/dd-trace/test/appsec/iast/telemetry/namespaces.spec.js +++ b/packages/dd-trace/test/appsec/iast/telemetry/namespaces.spec.js @@ -1,6 +1,5 @@ 'use strict' -const { expect } = require('chai') const { initRequestNamespace, finalizeRequestNamespace, From b6012b0ff91cb3f43b710d748f818b46cbd403ff Mon Sep 17 00:00:00 2001 From: Ugaitz Urien Date: Wed, 5 Jul 2023 09:15:50 +0200 Subject: [PATCH 5/7] Small fixes --- packages/dd-trace/src/appsec/iast/telemetry/index.js | 12 ++---------- .../dd-trace/test/appsec/iast/iast-plugin.spec.js | 5 ++--- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/packages/dd-trace/src/appsec/iast/telemetry/index.js b/packages/dd-trace/src/appsec/iast/telemetry/index.js index ec0a6700d0a..6a49e16e9aa 100644 --- a/packages/dd-trace/src/appsec/iast/telemetry/index.js +++ b/packages/dd-trace/src/appsec/iast/telemetry/index.js @@ -2,7 +2,7 @@ const telemetryMetrics = require('../../../telemetry/metrics') const telemetryLogs = require('./log') -const { Verbosity, isDebugAllowed, isInfoAllowed, getVerbosity } = require('./verbosity') +const { Verbosity, getVerbosity } = require('./verbosity') const { initRequestNamespace, finalizeRequestNamespace, globalNamespace } = require('./namespaces') class Telemetry { @@ -29,14 +29,6 @@ class Telemetry { return this.enabled } - isDebugEnabled () { - return this.isEnabled() && isDebugAllowed(this.verbosity) - } - - isInformationEnabled () { - return this.isEnabled() && isInfoAllowed(this.verbosity) - } - onRequestStart (context) { if (this.isEnabled() && this.verbosity !== Verbosity.OFF) { initRequestNamespace(context) @@ -45,7 +37,7 @@ class Telemetry { onRequestEnd (context, rootSpan) { if (this.isEnabled() && this.verbosity !== Verbosity.OFF) { - finalizeRequestNamespace(context, rootSpan, this.namespace) + finalizeRequestNamespace(context, rootSpan) } } } diff --git a/packages/dd-trace/test/appsec/iast/iast-plugin.spec.js b/packages/dd-trace/test/appsec/iast/iast-plugin.spec.js index ce0109ac2a7..26256d537ba 100644 --- a/packages/dd-trace/test/appsec/iast/iast-plugin.spec.js +++ b/packages/dd-trace/test/appsec/iast/iast-plugin.spec.js @@ -196,8 +196,7 @@ describe('IAST Plugin', () => { } } iastTelemetry = { - isEnabled: () => true, - isDebugEnabled: () => true + isEnabled: () => true } const IastPlugin = proxyquire('../../../src/appsec/iast/iast-plugin', { '../../plugins/plugin': PluginClass, @@ -225,7 +224,7 @@ describe('IAST Plugin', () => { loadChannel.publish({ name: 'test' }) - expect(onInstrumentationLoadedMock).to.be.calledOnceWith('test') + expect(onInstrumentationLoadedMock).to.be.calledWith('test') }) }) From d47bb3d44990e3aa6f76db18115425d5d27c65af Mon Sep 17 00:00:00 2001 From: Ugaitz Urien Date: Wed, 5 Jul 2023 13:26:07 +0200 Subject: [PATCH 6/7] Fix and test --- packages/dd-trace/src/appsec/iast/index.js | 2 +- .../src/appsec/iast/telemetry/log/index.js | 2 +- .../test/appsec/iast/telemetry/index.spec.js | 302 ++++++++++-------- 3 files changed, 178 insertions(+), 128 deletions(-) diff --git a/packages/dd-trace/src/appsec/iast/index.js b/packages/dd-trace/src/appsec/iast/index.js index dc4c4fd2930..16697979ed9 100644 --- a/packages/dd-trace/src/appsec/iast/index.js +++ b/packages/dd-trace/src/appsec/iast/index.js @@ -53,8 +53,8 @@ function onIncomingHttpRequestStart (data) { const iastContext = iastContextFunctions.saveIastContext(store, topContext, { rootSpan, req: data.req }) createTransaction(rootSpan.context().toSpanId(), iastContext) overheadController.initializeRequestContext(iastContext) - taintTrackingPlugin.taintHeaders(data.req.headers, iastContext) iastTelemetry.onRequestStart(iastContext) + taintTrackingPlugin.taintHeaders(data.req.headers, iastContext) } if (rootSpan.addTags) { rootSpan.addTags({ diff --git a/packages/dd-trace/src/appsec/iast/telemetry/log/index.js b/packages/dd-trace/src/appsec/iast/telemetry/log/index.js index 0c052663667..55cc0e75c5f 100644 --- a/packages/dd-trace/src/appsec/iast/telemetry/log/index.js +++ b/packages/dd-trace/src/appsec/iast/telemetry/log/index.js @@ -37,7 +37,7 @@ function isLogCollectionEnabled (config) { function onTelemetryStart (msg) { if (!msg || !isLogCollectionEnabled(msg.config)) { - log.info('IAST telemetry logs start received but log collection is not enabled or configuration is incorrect') + log.info('IAST telemetry logs start event received but log collection is not enabled or configuration is incorrect') return false } diff --git a/packages/dd-trace/test/appsec/iast/telemetry/index.spec.js b/packages/dd-trace/test/appsec/iast/telemetry/index.spec.js index 012176057b0..2f54701f1be 100644 --- a/packages/dd-trace/test/appsec/iast/telemetry/index.spec.js +++ b/packages/dd-trace/test/appsec/iast/telemetry/index.spec.js @@ -2,171 +2,221 @@ const { expect } = require('chai') const proxyquire = require('proxyquire') + const { Verbosity } = require('../../../../src/appsec/iast/telemetry/verbosity') +const Config = require('../../../../src/config') +const iast = require('../../../../src/appsec/iast') +const agent = require('../../../plugins/agent') +const axios = require('axios') +const { testInRequest } = require('../utils') describe('Telemetry', () => { - let defaultConfig - let telemetryMetrics - let iastTelemetry - let telemetryLogs - let initRequestNamespace - let finalizeRequestNamespace - - beforeEach(() => { - defaultConfig = { - telemetry: { - enabled: true, - metrics: true + describe('unit test', () => { + let defaultConfig + let telemetryMetrics + let iastTelemetry + let telemetryLogs + let initRequestNamespace + let finalizeRequestNamespace + + beforeEach(() => { + defaultConfig = { + telemetry: { + enabled: true, + metrics: true + } } - } - telemetryLogs = { - registerProvider: () => telemetryLogs, - start: sinon.spy(), - stop: sinon.spy() - } - - telemetryMetrics = { - manager: { - set: sinon.spy(), - delete: sinon.spy() + telemetryLogs = { + registerProvider: () => telemetryLogs, + start: sinon.spy(), + stop: sinon.spy() } - } - - initRequestNamespace = sinon.spy() - finalizeRequestNamespace = sinon.spy() - iastTelemetry = proxyquire('../../../../src/appsec/iast/telemetry', { - './log': telemetryLogs, - '../../../telemetry/metrics': telemetryMetrics, - './namespaces': { - initRequestNamespace, - finalizeRequestNamespace + telemetryMetrics = { + manager: { + set: sinon.spy(), + delete: sinon.spy() + } } - }) - }) - afterEach(() => { - sinon.restore() - }) + initRequestNamespace = sinon.spy() + finalizeRequestNamespace = sinon.spy() - describe('configure', () => { - it('should set default verbosity', () => { - iastTelemetry.configure(defaultConfig) + iastTelemetry = proxyquire('../../../../src/appsec/iast/telemetry', { + './log': telemetryLogs, + '../../../telemetry/metrics': telemetryMetrics, + './namespaces': { + initRequestNamespace, + finalizeRequestNamespace + } + }) + }) - expect(iastTelemetry.enabled).to.be.true - expect(iastTelemetry.verbosity).to.be.equal(Verbosity.INFORMATION) - expect(telemetryLogs.start).to.be.calledOnce + afterEach(() => { + sinon.restore() }) - it('should set OFF verbosity if not enabled', () => { - defaultConfig.telemetry.enabled = false - iastTelemetry.configure(defaultConfig) + describe('configure', () => { + it('should set default verbosity', () => { + iastTelemetry.configure(defaultConfig) - expect(iastTelemetry.enabled).to.be.false - expect(iastTelemetry.verbosity).to.be.equal(Verbosity.OFF) - expect(telemetryLogs.start).to.be.called - }) + expect(iastTelemetry.enabled).to.be.true + expect(iastTelemetry.verbosity).to.be.equal(Verbosity.INFORMATION) + expect(telemetryLogs.start).to.be.calledOnce + }) - it('should init metrics even if verbosity is OFF', () => { - const iastTelemetry = proxyquire('../../../../src/appsec/iast/telemetry', { - './log': telemetryLogs, - '../../../telemetry/metrics': telemetryMetrics, - './verbosity': { - getVerbosity: () => Verbosity.OFF - } + it('should set OFF verbosity if not enabled', () => { + defaultConfig.telemetry.enabled = false + iastTelemetry.configure(defaultConfig) + + expect(iastTelemetry.enabled).to.be.false + expect(iastTelemetry.verbosity).to.be.equal(Verbosity.OFF) + expect(telemetryLogs.start).to.be.called }) - const telemetryConfig = { enabled: true, metrics: true } - iastTelemetry.configure({ - telemetry: telemetryConfig + it('should init metrics even if verbosity is OFF', () => { + const iastTelemetry = proxyquire('../../../../src/appsec/iast/telemetry', { + './log': telemetryLogs, + '../../../telemetry/metrics': telemetryMetrics, + './verbosity': { + getVerbosity: () => Verbosity.OFF + } + }) + + const telemetryConfig = { enabled: true, metrics: true } + iastTelemetry.configure({ + telemetry: telemetryConfig + }) + + expect(iastTelemetry.enabled).to.be.true + expect(iastTelemetry.verbosity).to.be.equal(Verbosity.OFF) + expect(telemetryMetrics.manager.set).to.be.calledOnce + expect(telemetryLogs.start).to.be.calledOnce }) - expect(iastTelemetry.enabled).to.be.true - expect(iastTelemetry.verbosity).to.be.equal(Verbosity.OFF) - expect(telemetryMetrics.manager.set).to.be.calledOnce - expect(telemetryLogs.start).to.be.calledOnce - }) + it('should not init metrics if metrics not enabled', () => { + const telemetryConfig = { enabled: true, metrics: false } + iastTelemetry.configure({ + telemetry: telemetryConfig + }) - it('should not init metrics if metrics not enabled', () => { - const telemetryConfig = { enabled: true, metrics: false } - iastTelemetry.configure({ - telemetry: telemetryConfig + expect(iastTelemetry.enabled).to.be.false + expect(iastTelemetry.verbosity).to.be.equal(Verbosity.OFF) + expect(telemetryMetrics.manager.set).to.not.be.called + expect(telemetryLogs.start).to.be.calledOnce }) - - expect(iastTelemetry.enabled).to.be.false - expect(iastTelemetry.verbosity).to.be.equal(Verbosity.OFF) - expect(telemetryMetrics.manager.set).to.not.be.called - expect(telemetryLogs.start).to.be.calledOnce }) - }) - describe('stop', () => { - it('should set enabled = false and unregister provider', () => { - iastTelemetry.configure(defaultConfig) + describe('stop', () => { + it('should set enabled = false and unregister provider', () => { + iastTelemetry.configure(defaultConfig) - iastTelemetry.stop() - expect(iastTelemetry.enabled).to.be.false - expect(telemetryMetrics.manager.delete).to.be.calledOnce - expect(telemetryLogs.stop).to.be.calledOnce + iastTelemetry.stop() + expect(iastTelemetry.enabled).to.be.false + expect(telemetryMetrics.manager.delete).to.be.calledOnce + expect(telemetryLogs.stop).to.be.calledOnce + }) }) - }) - - describe('onRequestStart', () => { - it('should call init if enabled and verbosity is not Off', () => { - iastTelemetry.configure(defaultConfig) - const iastContext = {} - iastTelemetry.onRequestStart(iastContext) + describe('onRequestStart', () => { + it('should call init if enabled and verbosity is not Off', () => { + iastTelemetry.configure(defaultConfig) - expect(initRequestNamespace).to.be.calledOnceWith(iastContext) - }) + const iastContext = {} + iastTelemetry.onRequestStart(iastContext) - it('should not call init if enabled and verbosity is Off', () => { - const iastTelemetry = proxyquire('../../../../src/appsec/iast/telemetry', { - '../../../telemetry/metrics': telemetryMetrics, - './log': telemetryLogs, - './verbosity': { - getVerbosity: () => Verbosity.OFF - } + expect(initRequestNamespace).to.be.calledOnceWith(iastContext) }) - iastTelemetry.configure({ - telemetry: { enabled: true } + + it('should not call init if enabled and verbosity is Off', () => { + const iastTelemetry = proxyquire('../../../../src/appsec/iast/telemetry', { + '../../../telemetry/metrics': telemetryMetrics, + './log': telemetryLogs, + './verbosity': { + getVerbosity: () => Verbosity.OFF + } + }) + iastTelemetry.configure({ + telemetry: { enabled: true } + }) + + const iastContext = {} + iastTelemetry.onRequestStart(iastContext) + + expect(initRequestNamespace).to.not.be.calledOnce }) + }) - const iastContext = {} - iastTelemetry.onRequestStart(iastContext) + describe('onRequestEnd', () => { + it('should call finalizeRequestNamespace if enabled and verbosity is not Off', () => { + iastTelemetry.configure(defaultConfig) + + const iastContext = {} + iastTelemetry.onRequestEnd(iastContext) + + expect(finalizeRequestNamespace).to.be.calledOnceWith(iastContext) + }) - expect(initRequestNamespace).to.not.be.calledOnce + it('should not call finalizeRequestNamespace if enabled and verbosity is Off', () => { + const iastTelemetry = proxyquire('../../../../src/appsec/iast/telemetry', { + '../../../telemetry/metrics': telemetryMetrics, + './log': telemetryLogs, + './verbosity': { + getVerbosity: () => Verbosity.OFF + } + }) + iastTelemetry.configure({ + telemetry: { enabled: true } + }) + + const iastContext = {} + iastTelemetry.onRequestEnd(iastContext) + + expect(finalizeRequestNamespace).to.not.be.calledOnce + }) }) }) - describe('onRequestEnd', () => { - it('should call finalizeRequestNamespace if enabled and verbosity is not Off', () => { - iastTelemetry.configure(defaultConfig) - - const iastContext = {} - iastTelemetry.onRequestEnd(iastContext) + describe('full feature', () => { + let originalProcessEnv - expect(finalizeRequestNamespace).to.be.calledOnceWith(iastContext) + beforeEach(() => { + originalProcessEnv = process.env + process.env = { + DD_TELEMETRY_METRICS_ENABLED: 'true', + DD_IAST_ENABLED: 'true', + DD_IAST_REQUEST_SAMPLING: '100' + } + const config = new Config() + iast.enable(config) }) - it('should not call finalizeRequestNamespace if enabled and verbosity is Off', () => { - const iastTelemetry = proxyquire('../../../../src/appsec/iast/telemetry', { - '../../../telemetry/metrics': telemetryMetrics, - './log': telemetryLogs, - './verbosity': { - getVerbosity: () => Verbosity.OFF - } - }) - iastTelemetry.configure({ - telemetry: { enabled: true } - }) - - const iastContext = {} - iastTelemetry.onRequestEnd(iastContext) + afterEach(() => { + iast.disable() + process.env = originalProcessEnv + }) - expect(finalizeRequestNamespace).to.not.be.calledOnce + after(() => { + process.env = originalProcessEnv }) + function app () {} + + function tests (config) { + it('should have header source execution metric', (done) => { + agent + .use(traces => { + expect(traces[0][0].metrics['_dd.iast.telemetry.executed.source.http_request_header']).to.be.equal(1) + }) + .then(done) + .catch(done) + axios.get(`http://localhost:${config.port}/`, { + headers: { + 'x-test-header': 'test-value' + } + }).catch(done) + }) + } + testInRequest(app, tests) }) }) From 1d0ce27664c29b3ac32b0579cf63a1ac79e6f203 Mon Sep 17 00:00:00 2001 From: Ugaitz Urien Date: Wed, 5 Jul 2023 13:36:06 +0200 Subject: [PATCH 7/7] Move method to test file --- .../appsec/iast/taint-tracking/csi-methods.js | 17 ++--------------- .../taint-tracking-operations.spec.js | 14 +++++++++++++- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/packages/dd-trace/src/appsec/iast/taint-tracking/csi-methods.js b/packages/dd-trace/src/appsec/iast/taint-tracking/csi-methods.js index 5dd403d71dc..39dd4378590 100644 --- a/packages/dd-trace/src/appsec/iast/taint-tracking/csi-methods.js +++ b/packages/dd-trace/src/appsec/iast/taint-tracking/csi-methods.js @@ -1,8 +1,8 @@ 'use strict' const csiMethods = [ - { src: 'plusOperator', operator: true }, { src: 'concat' }, + { src: 'plusOperator', operator: true }, { src: 'replace' }, { src: 'slice' }, { src: 'substr' }, @@ -12,19 +12,6 @@ const csiMethods = [ { src: 'trimStart', dst: 'trim' } ] -function getExpectedMethods () { - const set = new Set() - for (const definition of csiMethods) { - if (definition.dst) { - set.add(definition.dst) - } else { - set.add(definition.src) - } - } - return [...set] -} - module.exports = { - csiMethods, - getExpectedMethods + csiMethods } diff --git a/packages/dd-trace/test/appsec/iast/taint-tracking/taint-tracking-operations.spec.js b/packages/dd-trace/test/appsec/iast/taint-tracking/taint-tracking-operations.spec.js index 36a0d970ad0..0a8d77fb2a1 100644 --- a/packages/dd-trace/test/appsec/iast/taint-tracking/taint-tracking-operations.spec.js +++ b/packages/dd-trace/test/appsec/iast/taint-tracking/taint-tracking-operations.spec.js @@ -3,11 +3,23 @@ const { expect } = require('chai') const proxyquire = require('proxyquire') const iastContextFunctions = require('../../../../src/appsec/iast/iast-context') -const { getExpectedMethods } = require('../../../../src/appsec/iast/taint-tracking/csi-methods') +const { csiMethods } = require('../../../../src/appsec/iast/taint-tracking/csi-methods') const iastTelemetry = require('../../../../src/appsec/iast/telemetry') const { EXECUTED_PROPAGATION, REQUEST_TAINTED } = require('../../../../src/appsec/iast/telemetry/iast-metric') const { Verbosity } = require('../../../../src/appsec/iast/telemetry/verbosity') +function getExpectedMethods () { + const set = new Set() + for (const definition of csiMethods) { + if (definition.dst) { + set.add(definition.dst) + } else { + set.add(definition.src) + } + } + return [...set] +} + describe('IAST TaintTracking Operations', () => { let taintTrackingOperations let taintTrackingImpl