Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standalone ASM configuration and span tags #4291

Merged
merged 48 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from 46 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
f14601e
DD_APM_TRACING_ENABLED and span _dd.apm.enabled tag
iunanua May 9, 2024
2817602
clean up
iunanua May 13, 2024
957760a
Use MANUAL_KEEP const
iunanua May 16, 2024
5d6210d
Add _dd.p.appsec tag on standalone ASM events
iunanua May 17, 2024
089c26d
Include apmTracingEnabled checks
iunanua May 20, 2024
18d9fef
Appsec Reporter tests
iunanua May 20, 2024
d903394
Appsec sdk track_event test
iunanua May 20, 2024
736014a
Use numeric value for _dd.p.appsec
iunanua May 20, 2024
396a8fc
Include appsec standalone config in .ts files
iunanua May 21, 2024
cb73c58
Clean up null and undefined values
iunanua May 21, 2024
67b171a
Merge branch 'master' into igor/standalone-asm-config-and-tags
iunanua May 22, 2024
460d773
Remove not needed config properties
iunanua May 23, 2024
31f8951
standalone module
iunanua May 23, 2024
0b8a067
Clean up
iunanua May 23, 2024
dc1b7b9
standalone proxy test
iunanua May 23, 2024
626bbfa
Update packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js
iunanua May 23, 2024
d989a73
appsec reporter test
iunanua May 23, 2024
7734ae7
Use standalone singletone in vulnerability-reporter
iunanua May 23, 2024
0ba2f3d
continue applying ratelimiter on appsec standalone events
iunanua May 23, 2024
4518d52
Update packages/dd-trace/src/appsec/reporter.js
iunanua May 23, 2024
167d5d5
Add _dd.apm.enabled:0 in root spans with remote parent
iunanua May 31, 2024
6f328fe
Use a method to add the tag
iunanua May 31, 2024
3627b99
Remove apmTracingEnabled config property
iunanua Jun 4, 2024
9a2a105
Merge branch 'master' into igor/standalone-asm-config-and-tags
iunanua Jun 5, 2024
e57525d
Add _dd.p.appsec tag in trace tags
iunanua Jun 6, 2024
b15c51c
Some tests
iunanua Jun 6, 2024
5554ff7
Set _dd.apm.enabled in root span
iunanua Jun 6, 2024
f658168
configure standalone if _tracingInitialized
iunanua Jun 6, 2024
53b1dc0
Use dd-trace:span:start channel
iunanua Jun 7, 2024
a2e3a6c
Clean up
iunanua Jun 7, 2024
05728b5
use a meta tag
iunanua Jun 10, 2024
096a05f
Merge branch 'master' into igor/standalone-asm-config-and-tags
iunanua Jun 10, 2024
9c4a3da
hasSubscribers check
iunanua Jun 10, 2024
b0ddeab
test description
iunanua Jun 10, 2024
eea2b95
Check span context has tags before using them and check if config has…
iunanua Jun 10, 2024
80369e2
clean up
iunanua Jun 11, 2024
db4621d
Clean up
iunanua Jun 11, 2024
c37423a
Clean up
iunanua Jun 11, 2024
d0f154e
clean up
iunanua Jun 12, 2024
d4fd15e
Update packages/dd-trace/test/appsec/sdk/track_event.spec.js
iunanua Jun 12, 2024
888c81f
Update packages/dd-trace/test/appsec/standalone.spec.js
iunanua Jun 12, 2024
58a9c6a
protect sample method
iunanua Jun 12, 2024
99675fb
Use assert instead expect
iunanua Jun 12, 2024
2a03d12
unsubscribe after test
iunanua Jun 13, 2024
0547b74
Merge branch 'master' into igor/standalone-asm-config-and-tags
iunanua Jun 13, 2024
0b82fe8
Merge branch 'master' into igor/standalone-asm-config-and-tags
iunanua Jun 14, 2024
999f136
Merge branch 'master' into igor/standalone-asm-config-and-tags
iunanua Jun 18, 2024
7dc9981
suggestions
iunanua Jun 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,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)
simon-id marked this conversation as resolved.
Show resolved Hide resolved

standalone.sample(span)

if (!rootSpan) span.finish()
}
}
Expand Down
10 changes: 8 additions & 2 deletions packages/dd-trace/src/appsec/reporter.js
simon-id marked this conversation as resolved.
Show resolved Hide resolved
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) {
simon-id marked this conversation as resolved.
Show resolved Hide resolved
startCh.subscribe(onSpanStart)
} else {
startCh.unsubscribe(onSpanStart)
}
}

function sample (span) {
simon-id marked this conversation as resolved.
Show resolved Hide resolved
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.wafTimeout', 5e3) // µs
this._setValue(defaults, 'clientIpEnabled', false)
this._setValue(defaults, 'clientIpHeader', null)
Expand Down Expand Up @@ -536,6 +537,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 @@ -627,6 +629,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._setValue(env, 'appsec.wafTimeout', maybeInt(DD_APPSEC_WAF_TIMEOUT))
this._envUnprocessed['appsec.wafTimeout'] = DD_APPSEC_WAF_TIMEOUT
this._setBoolean(env, 'clientIpEnabled', DD_TRACE_CLIENT_IP_ENABLED)
Expand Down Expand Up @@ -767,6 +770,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._setValue(opts, 'appsec.wafTimeout', maybeInt(options.appsec.wafTimeout))
this._optsUnprocessed['appsec.wafTimeout'] = options.appsec.wafTimeout
this._setBoolean(opts, 'clientIpEnabled', options.clientIpEnabled)
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'
uurien marked this conversation as resolved.
Show resolved Hide resolved
}
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 })
simon-id marked this conversation as resolved.
Show resolved Hide resolved
}
}

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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we could move this configure() somewhere in appsec territory ? it's not super important but if we can do it in a clean way it could be nice

Copy link
Contributor Author

@iunanua iunanua Jun 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need it in proxy.js to replace the default PrioritySampler via DatadogTracer constructor, something you could see in the next standalone PR.
Also standalone is not related with AppsecSdk so I'd prefer not to mix the two features.

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
Loading