Skip to content

Upgrade libddwaf native bindings #5792

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

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion integration-tests/graphql.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ describe('graphql', () => {
assert.property(payload[1][0].metrics, '_dd.appsec.waf.duration')
assert.propertyVal(payload[1][0].meta, 'appsec.event', 'true')
assert.property(payload[1][0].meta, '_dd.appsec.json')
assert.propertyVal(payload[1][0].meta, '_dd.appsec.json', JSON.stringify(result))
assert.deepStrictEqual(JSON.parse(payload[1][0].meta['_dd.appsec.json']), result)
})

await axios({
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@
},
"dependencies": {
"@datadog/libdatadog": "^0.5.1",
"@datadog/native-appsec": "8.5.2",
"@datadog/native-appsec": "9.0.0",
"@datadog/native-iast-taint-tracking": "4.0.0",
"@datadog/native-metrics": "^3.1.1",
"@datadog/pprof": "5.8.0",
Expand Down
69 changes: 68 additions & 1 deletion packages/dd-trace/src/appsec/reporter.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,23 @@
'use strict'

const dc = require('dc-polyfill')
const zlib = require('zlib')

const Limiter = require('../rate_limiter')
const { storage } = require('../../../datadog-core')
const web = require('../plugins/util/web')
const { ipHeaderList } = require('../plugins/util/ip_extractor')
const {
incrementWafInitMetric,
incrementWafUpdatesMetric,
incrementWafConfigErrorsMetric,
incrementWafRequestsMetric,
updateWafRequestsMetricTags,
updateRaspRequestsMetricTags,
updateRaspRuleSkippedMetricTags,
updateRateLimitedMetric,
getRequestMetrics
} = require('./telemetry')
const zlib = require('zlib')
const { keepTrace } = require('../priority_sampler')
const { ASM } = require('../standalone/product')

Expand All @@ -25,6 +28,20 @@ const COLLECTED_REQUEST_BODY_MAX_STRING_LENGTH = 4096
const COLLECTED_REQUEST_BODY_MAX_DEPTH = 20
const COLLECTED_REQUEST_BODY_MAX_ELEMENTS_PER_NODE = 256

const telemetryLogCh = dc.channel('datadog:telemetry:log')

const WAF_DIAGNOSTICS_CONFIG_KEYS_TO_REPORT = [
'rules',
'custom_rules',
'exclusions',
'actions',
'processors',
'scanners',
'rules_override',
'rules_data',
'exclusion_data'
]

// default limiter, configurable with setRateLimit()
let limiter = new Limiter(100)

Expand Down Expand Up @@ -225,6 +242,54 @@ function reportWafInit (wafVersion, rulesVersion, diagnosticsRules = {}, success
incrementWafInitMetric(wafVersion, rulesVersion, success)
}

function logWafDiagnosticMessage (product, rcConfigId, configKey, message, level) {
telemetryLogCh.publish({
Copy link
Collaborator

Choose a reason for hiding this comment

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

These logs are sent via telemetry, but aren't shown in the output if the logs are enabled, is the expected behaviour?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The RFC only establishes the propagation of diagnostics by telemetry and RC. Specifically it states that:

Note that aside from through telemetry logs, none of the diagnostics presented here should be logged to a regular logger using a level above debug, as it will inevitably result in customer complaints.

message,
level,
tags: {
log_type: `rc::${product.toLowerCase()}::diagnostic`,
appsec_config_key: configKey,
rc_config_id: rcConfigId
}
})
}

function reportSuccessfulWafUpdate (product, rcConfigId, diagnostics) {
for (const configKey of WAF_DIAGNOSTICS_CONFIG_KEYS_TO_REPORT) {
const configDiagnostics = diagnostics[configKey]
if (!configDiagnostics) continue

if (configDiagnostics.error) {
logWafDiagnosticMessage(product, rcConfigId, configKey, configDiagnostics.error, 'ERROR')
continue
}

if (configDiagnostics.errors) {
for (const [errorMessage, errorIds] of Object.entries(configDiagnostics.errors)) {
logWafDiagnosticMessage(
product,
rcConfigId,
configKey,
`"${errorMessage}": ${JSON.stringify(errorIds)}`,
'ERROR'
)
}
}

if (configDiagnostics.warnings) {
for (const [warningMessage, warningIds] of Object.entries(configDiagnostics.warnings)) {
logWafDiagnosticMessage(
product,
rcConfigId,
configKey,
`"${warningMessage}": ${JSON.stringify(warningIds)}`,
'WARN'
)
}
}
Comment on lines +267 to +289
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion (I think this is already good as is, so it's up to you): this could be unified with a loop over ['errors', 'warnings']

}
}

function reportMetrics (metrics, raspRule) {
const store = storage('legacy').getStore()
const rootSpan = store?.req && web.root(store.req)
Expand Down Expand Up @@ -485,9 +550,11 @@ module.exports = {
filterExtendedHeaders,
formatHeaderName,
reportWafInit,
reportSuccessfulWafUpdate,
reportMetrics,
reportAttack,
reportWafUpdate: incrementWafUpdatesMetric,
reportWafConfigError: incrementWafConfigErrorsMetric,
reportRaspRuleSkipped: updateRaspRuleSkippedMetricTags,
reportDerivatives,
finishRequest,
Expand Down
Loading