Skip to content

Commit

Permalink
Standalone ASM configuration and span tags (#4291)
Browse files Browse the repository at this point in the history
* DD_APM_TRACING_ENABLED and span _dd.apm.enabled tag

* clean up

* Use MANUAL_KEEP const

* Add _dd.p.appsec tag on standalone ASM events

* Include apmTracingEnabled checks

* Appsec Reporter tests

* Appsec sdk track_event test

* Use numeric value for _dd.p.appsec

* Include appsec standalone config in .ts files

* Clean up null and undefined values

* Remove not needed config properties

* standalone module

* Clean up

* standalone proxy test

* Update packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js

Co-authored-by: Ugaitz Urien <[email protected]>

* appsec reporter test

* Use standalone singletone in vulnerability-reporter

* continue applying ratelimiter on appsec standalone events

* Update packages/dd-trace/src/appsec/reporter.js

Co-authored-by: simon-id <[email protected]>

* Add _dd.apm.enabled:0 in root spans with remote parent

* Use a method to add the tag

* Remove apmTracingEnabled config property

* Add _dd.p.appsec tag in trace tags

* Some tests

* Set _dd.apm.enabled in root span

* configure standalone if _tracingInitialized

* Use dd-trace:span:start channel

* Clean up

* use a meta tag

* hasSubscribers check

* test description

* Check span context has tags before using them and check if config has changed

* clean up

* Clean up

* Clean up

* clean up

* Update packages/dd-trace/test/appsec/sdk/track_event.spec.js

Co-authored-by: Ugaitz Urien <[email protected]>

* Update packages/dd-trace/test/appsec/standalone.spec.js

Co-authored-by: Ugaitz Urien <[email protected]>

* protect sample method

* Use assert instead expect

* unsubscribe after test

* suggestions

---------

Co-authored-by: Ugaitz Urien <[email protected]>
Co-authored-by: simon-id <[email protected]>
  • Loading branch information
3 people authored and juan-fernandez committed Jul 11, 2024
1 parent 324d72b commit 1f9b5d7
Show file tree
Hide file tree
Showing 17 changed files with 345 additions and 6 deletions.
5 changes: 5 additions & 0 deletions docs/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ tracer.init({
redactionEnabled: true,
redactionNamePattern: 'password',
redactionValuePattern: 'bearer'
},
appsec: {
standalone: {
enabled: true
}
}
}
})
Expand Down
13 changes: 13 additions & 0 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,19 @@ declare namespace tracer {
*/
redactionValuePattern?: string
}

appsec?: {
/**
* Configuration of Standalone ASM mode
*/
standalone?: {
/**
* Whether to enable Standalone ASM.
* @default false
*/
enabled?: boolean
}
}
};

/**
Expand Down
4 changes: 4 additions & 0 deletions packages/dd-trace/src/appsec/iast/vulnerability-reporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const { MANUAL_KEEP } = require('../../../../../ext/tags')
const LRU = require('lru-cache')
const vulnerabilitiesFormatter = require('./vulnerabilities-formatter')
const { IAST_ENABLED_TAG_KEY, IAST_JSON_TAG_KEY } = require('./tags')
const standalone = require('../standalone')

const VULNERABILITIES_KEY = 'vulnerabilities'
const VULNERABILITY_HASHES_MAX_SIZE = 1000
Expand Down Expand Up @@ -57,6 +58,9 @@ function sendVulnerabilities (vulnerabilities, rootSpan) {
tags[IAST_JSON_TAG_KEY] = JSON.stringify(jsonToSend)
tags[MANUAL_KEEP] = 'true'
span.addTags(tags)

standalone.sample(span)

if (!rootSpan) span.finish()
}
}
Expand Down
10 changes: 8 additions & 2 deletions packages/dd-trace/src/appsec/reporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ const {
getRequestMetrics
} = require('./telemetry')
const zlib = require('zlib')
const { MANUAL_KEEP } = require('../../../../ext/tags')
const standalone = require('./standalone')

// default limiter, configurable with setRateLimit()
let limiter = new Limiter(100)
Expand Down Expand Up @@ -88,7 +90,7 @@ function reportWafInit (wafVersion, rulesVersion, diagnosticsRules = {}) {
metricsQueue.set('_dd.appsec.event_rules.errors', JSON.stringify(diagnosticsRules.errors))
}

metricsQueue.set('manual.keep', 'true')
metricsQueue.set(MANUAL_KEEP, 'true')

incrementWafInitMetric(wafVersion, rulesVersion)
}
Expand Down Expand Up @@ -121,7 +123,9 @@ function reportAttack (attackData) {
newTags['appsec.event'] = 'true'

if (limiter.isAllowed()) {
newTags['manual.keep'] = 'true' // TODO: figure out how to keep appsec traces with sampling revamp
newTags[MANUAL_KEEP] = 'true'

standalone.sample(rootSpan)
}

// TODO: maybe add this to format.js later (to take decision as late as possible)
Expand Down Expand Up @@ -172,6 +176,8 @@ function finishRequest (req, res) {
if (metricsQueue.size) {
rootSpan.addTags(Object.fromEntries(metricsQueue))

standalone.sample(rootSpan)

metricsQueue.clear()
}

Expand Down
3 changes: 3 additions & 0 deletions packages/dd-trace/src/appsec/sdk/track_event.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const log = require('../../log')
const { getRootSpan } = require('./utils')
const { MANUAL_KEEP } = require('../../../../../ext/tags')
const { setUserTags } = require('./set_user')
const standalone = require('../standalone')

function trackUserLoginSuccessEvent (tracer, user, metadata) {
// TODO: better user check here and in _setUser() ?
Expand Down Expand Up @@ -73,6 +74,8 @@ function trackEvent (eventName, fields, sdkMethodName, rootSpan, mode) {
}

rootSpan.addTags(tags)

standalone.sample(rootSpan)
}

module.exports = {
Expand Down
43 changes: 43 additions & 0 deletions packages/dd-trace/src/appsec/standalone.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
'use strict'

const { channel } = require('dc-polyfill')
const { APM_TRACING_ENABLED_KEY, APPSEC_PROPAGATION_KEY } = require('../constants')

const startCh = channel('dd-trace:span:start')

let enabled

function onSpanStart ({ span, fields }) {
const tags = span.context?.()?._tags
if (!tags) return

const { parent } = fields
if (!parent || parent._isRemote) {
tags[APM_TRACING_ENABLED_KEY] = 0
}
}

function configure (config) {
const configChanged = enabled !== config.appsec?.standalone?.enabled
if (!configChanged) return

enabled = config.appsec?.standalone?.enabled

if (enabled) {
startCh.subscribe(onSpanStart)
} else {
startCh.unsubscribe(onSpanStart)
}
}

function sample (span) {
const context = span.context?.()
if (enabled && context._trace?.tags) {
context._trace.tags[APPSEC_PROPAGATION_KEY] = '1'
}
}

module.exports = {
configure,
sample
}
4 changes: 4 additions & 0 deletions packages/dd-trace/src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ class Config {
this._setValue(defaults, 'appsec.rateLimit', 100)
this._setValue(defaults, 'appsec.rules', undefined)
this._setValue(defaults, 'appsec.sca.enabled', null)
this._setValue(defaults, 'appsec.standalone.enabled', undefined)
this._setValue(defaults, 'appsec.stackTrace.enabled', true)
this._setValue(defaults, 'appsec.stackTrace.maxDepth', 32)
this._setValue(defaults, 'appsec.stackTrace.maxStackTraces', 2)
Expand Down Expand Up @@ -542,6 +543,7 @@ class Config {
DD_DOGSTATSD_HOSTNAME,
DD_DOGSTATSD_PORT,
DD_ENV,
DD_EXPERIMENTAL_APPSEC_STANDALONE_ENABLED,
DD_EXPERIMENTAL_PROFILING_ENABLED,
JEST_WORKER_ID,
DD_IAST_DEDUPLICATION_ENABLED,
Expand Down Expand Up @@ -633,6 +635,7 @@ class Config {
this._setString(env, 'appsec.rules', DD_APPSEC_RULES)
// DD_APPSEC_SCA_ENABLED is never used locally, but only sent to the backend
this._setBoolean(env, 'appsec.sca.enabled', DD_APPSEC_SCA_ENABLED)
this._setBoolean(env, 'appsec.standalone.enabled', DD_EXPERIMENTAL_APPSEC_STANDALONE_ENABLED)
this._setBoolean(env, 'appsec.stackTrace.enabled', DD_APPSEC_STACK_TRACE_ENABLED)
this._setValue(env, 'appsec.stackTrace.maxDepth', maybeInt(DD_APPSEC_MAX_STACK_TRACE_DEPTH))
this._envUnprocessed['appsec.stackTrace.maxDepth'] = DD_APPSEC_MAX_STACK_TRACE_DEPTH
Expand Down Expand Up @@ -778,6 +781,7 @@ class Config {
this._setValue(opts, 'appsec.rateLimit', maybeInt(options.appsec.rateLimit))
this._optsUnprocessed['appsec.rateLimit'] = options.appsec.rateLimit
this._setString(opts, 'appsec.rules', options.appsec.rules)
this._setBoolean(opts, 'appsec.standalone.enabled', options.experimental?.appsec?.standalone?.enabled)
this._setBoolean(opts, 'appsec.stackTrace.enabled', options.appsec.stackTrace?.enabled)
this._setValue(opts, 'appsec.stackTrace.maxDepth', maybeInt(options.appsec.stackTrace?.maxDepth))
this._optsUnprocessed['appsec.stackTrace.maxDepth'] = options.appsec.stackTrace?.maxDepth
Expand Down
4 changes: 3 additions & 1 deletion packages/dd-trace/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,7 @@ module.exports = {
PEER_SERVICE_SOURCE_KEY: '_dd.peer.service.source',
PEER_SERVICE_REMAP_KEY: '_dd.peer.service.remapped_from',
SCI_REPOSITORY_URL: '_dd.git.repository_url',
SCI_COMMIT_SHA: '_dd.git.commit.sha'
SCI_COMMIT_SHA: '_dd.git.commit.sha',
APM_TRACING_ENABLED_KEY: '_dd.apm.enabled',
APPSEC_PROPAGATION_KEY: '_dd.p.appsec'
}
5 changes: 4 additions & 1 deletion packages/dd-trace/src/opentracing/span.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,10 @@ class DatadogSpan {
unfinishedRegistry.register(this, operationName, this)
}
spanleak.addSpan(this, operationName)
startCh.publish(this)

if (startCh.hasSubscribers) {
startCh.publish({ span: this, fields })
}
}

toString () {
Expand Down
2 changes: 2 additions & 0 deletions packages/dd-trace/src/proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const NoopDogStatsDClient = require('./noop/dogstatsd')
const spanleak = require('./spanleak')
const { SSIHeuristics } = require('./profiling/ssi-heuristics')
const telemetryLog = require('dc-polyfill').channel('datadog:telemetry:log')
const appsecStandalone = require('./appsec/standalone')

class LazyModule {
constructor (provider) {
Expand Down Expand Up @@ -180,6 +181,7 @@ class Tracer extends NoopProxy {
if (!this._tracingInitialized) {
this._tracer = new DatadogTracer(config)
this.appsec = new AppsecSdk(this._tracer, config)
appsecStandalone.configure(config)
this._tracingInitialized = true
}
if (config.iast.enabled) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
const { addVulnerability, sendVulnerabilities, clearCache, start, stop } =
require('../../../src/appsec/iast/vulnerability-reporter')
const VulnerabilityAnalyzer = require('../../../../dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer')
const appsecStandalone = require('../../../src/appsec/standalone')
const { APPSEC_PROPAGATION_KEY } = require('../../../src/constants')

describe('vulnerability-reporter', () => {
let vulnerabilityAnalyzer

Expand All @@ -9,6 +12,8 @@ describe('vulnerability-reporter', () => {
vulnerabilityAnalyzer = new VulnerabilityAnalyzer('ANALYZER_TYPE')
})

afterEach(sinon.restore)

describe('addVulnerability', () => {
it('should not break with invalid input', () => {
addVulnerability()
Expand Down Expand Up @@ -130,10 +135,13 @@ describe('vulnerability-reporter', () => {

describe('sendVulnerabilities', () => {
let span
let context

beforeEach(() => {
context = { _trace: { tags: {} } }
span = {
addTags: sinon.stub()
addTags: sinon.stub(),
context: sinon.stub().returns(context)
}
start({
iast: {
Expand Down Expand Up @@ -378,6 +386,40 @@ describe('vulnerability-reporter', () => {
'{"spanId":888,"path":"filename.js","line":88}}]}'
})
})

it('should add _dd.p.appsec trace tag with standalone enabled', () => {
appsecStandalone.configure({ appsec: { standalone: { enabled: true } } })
const iastContext = { rootSpan: span }
addVulnerability(iastContext,
vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 999))

sendVulnerabilities(iastContext.vulnerabilities, span)

expect(span.addTags).to.have.been.calledOnceWithExactly({
'manual.keep': 'true',
'_dd.iast.json': '{"sources":[],"vulnerabilities":[{"type":"INSECURE_HASHING","hash":3254801297,' +
'"evidence":{"value":"sha1"},"location":{"spanId":999}}]}'
})

expect(span.context()._trace.tags).to.have.property(APPSEC_PROPAGATION_KEY)
})

it('should not add _dd.p.appsec trace tag with standalone disabled', () => {
appsecStandalone.configure({ appsec: {} })
const iastContext = { rootSpan: span }
addVulnerability(iastContext,
vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 999))

sendVulnerabilities(iastContext.vulnerabilities, span)

expect(span.addTags).to.have.been.calledOnceWithExactly({
'manual.keep': 'true',
'_dd.iast.json': '{"sources":[],"vulnerabilities":[{"type":"INSECURE_HASHING","hash":3254801297,' +
'"evidence":{"value":"sha1"},"location":{"spanId":999}}]}'
})

expect(span.context()._trace.tags).to.not.have.property(APPSEC_PROPAGATION_KEY)
})
})

describe('clearCache', () => {
Expand Down
29 changes: 28 additions & 1 deletion packages/dd-trace/test/appsec/reporter.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ describe('reporter', () => {
let span
let web
let telemetry
let sample

beforeEach(() => {
span = {
Expand All @@ -32,9 +33,14 @@ describe('reporter', () => {
getRequestMetrics: sinon.stub()
}

sample = sinon.stub()

Reporter = proxyquire('../../src/appsec/reporter', {
'../plugins/util/web': web,
'./telemetry': telemetry
'./telemetry': telemetry,
'./standalone': {
sample
}
})
})

Expand Down Expand Up @@ -289,6 +295,27 @@ describe('reporter', () => {
'network.client.ip': '8.8.8.8'
})
})

it('should call standalone sample', () => {
span.context()._tags = { '_dd.appsec.json': '{"triggers":[{"rule":{},"rule_matches":[{}]}]}' }

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

expect(span.addTags).to.have.been.calledOnceWithExactly({
'http.request.headers.host': 'localhost',
'http.request.headers.user-agent': 'arachni',
'appsec.event': 'true',
'manual.keep': 'true',
'_dd.origin': 'appsec',
'_dd.appsec.json': '{"triggers":[{"rule":{},"rule_matches":[{}]},{"rule":{}},{"rule":{},"rule_matches":[{}]}]}',
'http.useragent': 'arachni',
'network.client.ip': '8.8.8.8'
})

expect(sample).to.have.been.calledOnceWithExactly(span)
})
})

describe('reportWafUpdate', () => {
Expand Down
16 changes: 16 additions & 0 deletions packages/dd-trace/test/appsec/sdk/track_event.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ describe('track_event', () => {
let getRootSpan
let setUserTags
let trackUserLoginSuccessEvent, trackUserLoginFailureEvent, trackCustomEvent, trackEvent
let sample

beforeEach(() => {
log = {
Expand All @@ -28,13 +29,18 @@ describe('track_event', () => {

setUserTags = sinon.stub()

sample = sinon.stub()

const trackEvents = proxyquire('../../../src/appsec/sdk/track_event', {
'../../log': log,
'./utils': {
getRootSpan
},
'./set_user': {
setUserTags
},
'../standalone': {
sample
}
})

Expand Down Expand Up @@ -249,6 +255,16 @@ describe('track_event', () => {
'appsec.events.event.metakey2': 'metaValue2'
})
})

it('should call standalone sample', () => {
trackEvent('event', undefined, 'trackEvent', rootSpan, undefined)

expect(rootSpan.addTags).to.have.been.calledOnceWithExactly({
'appsec.events.event.track': 'true',
'manual.keep': 'true'
})
expect(sample).to.have.been.calledOnceWithExactly(rootSpan)
})
})
})

Expand Down
Loading

0 comments on commit 1f9b5d7

Please sign in to comment.