Iast kafka consumer support by iunanua · Pull Request #3977 · DataDog/dd-trace-js

iunanua · 2024-01-18T10:46:40Z

What does this PR do?

modify KafkajsConsumerPlugin to publish events when a consumer starts and finishes handling a message
when these events are received by KafkaContextPlugin, it creates and destroys an iastContext
and when KafkaConsumerIastPlugin receives the events, it taints the kafka message key and value.
add two new vulnerability sources: kafka.message.key and kafka.message.value
rewrite JSON.parse calls to help with the taint tracking (for the case the kafka qeue that is encoding the messages as json)

Motivation

Be able to detect vulnerabilities in kafka consumers treating kafka messages as a source of "dangerous" input.

Plugin Checklist

Additional Notes

For non appsec team reviewers: IAST needs to be notified after the kafka consumer span is created and before span is finished so this PR introduces two new channels (dd-trace:kafkajs:consumer:afterStart and dd-trace:kafkajs:consumer:beforeFinish) in KafkajsConsumerPlugin to notify when such events occur.

Security

Datadog employees:

If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

pr-commenter · 2024-01-18T10:53:49Z

Benchmarks

Benchmark execution time: 2024-02-08 10:23:18

Comparing candidate commit a6cee13 in PR branch igor/kafka-iast with baseline commit e368f4e in branch master.

Found 1 performance improvements and 0 performance regressions! Performance is the same for 261 metrics, 4 unstable metrics.

scenario:plugin-graphql-with-depth-and-collapse-on-18

🟩 max_rss_usage [-147.386MB; -101.842MB] or [-15.356%; -10.611%]

github-actions · 2024-01-19T08:03:11Z

Overall package size

Self size: 6.02 MB
Deduped: 61.9 MB
No deduping: 62.66 MB

Dependency sizes

name	version	self size	total size
@datadog/native-iast-taint-tracking	1.7.0	16.71 MB	16.72 MB
@datadog/native-appsec	7.0.0	14.51 MB	14.52 MB
@datadog/pprof	5.0.0	9.59 MB	10.44 MB
protobufjs	7.2.5	2.77 MB	6.56 MB
@datadog/native-iast-rewriter	2.2.3	2.19 MB	2.28 MB
@opentelemetry/core	1.14.0	872.87 kB	1.47 MB
@datadog/native-metrics	2.0.0	898.77 kB	1.3 MB
@opentelemetry/api	1.4.1	780.32 kB	780.32 kB
import-in-the-middle	1.7.3	67.62 kB	731.01 kB
pprof-format	2.0.7	588.12 kB	588.12 kB
msgpack-lite	0.1.26	201.16 kB	281.59 kB
opentracing	0.14.7	194.81 kB	194.81 kB
semver	7.5.4	93.4 kB	123.8 kB
@datadog/sketches-js	2.1.0	109.9 kB	109.9 kB
lodash.sortby	4.7.0	75.76 kB	75.76 kB
lru-cache	7.14.0	74.95 kB	74.95 kB
ipaddr.js	2.1.0	60.23 kB	60.23 kB
ignore	5.2.4	51.22 kB	51.22 kB
int64-buffer	0.1.10	49.18 kB	49.18 kB
shell-quote	1.8.1	44.96 kB	44.96 kB
istanbul-lib-coverage	3.2.0	29.34 kB	29.34 kB
tlhunter-sorted-set	0.1.0	24.94 kB	24.94 kB
limiter	1.1.5	23.17 kB	23.17 kB
dc-polyfill	0.1.4	23.1 kB	23.1 kB
retry	0.13.1	18.85 kB	18.85 kB
node-abort-controller	3.1.1	16.89 kB	16.89 kB
jest-docblock	29.7.0	8.99 kB	12.76 kB
crypto-randomuuid	1.0.0	11.18 kB	11.18 kB
path-to-regexp	0.1.7	6.78 kB	6.78 kB
koalas	1.0.2	6.47 kB	6.47 kB
methods	1.1.2	5.29 kB	5.29 kB
module-details-from-path	1.0.3	4.47 kB	4.47 kB

_{🤖 This report was automatically generated by heaviest-objects-in-the-universe}

codecov · 2024-01-19T08:03:31Z

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (e368f4e) 85.15% compared to head (a6cee13) 85.31%.
Report is 25 commits behind head on master.

Files	Patch %	Lines
...dd-trace/src/appsec/iast/context/context-plugin.js	97.43%	1 Missing ⚠️
...sec/iast/taint-tracking/operations-taint-object.js	96.77%	1 Missing ⚠️
.../appsec/iast/taint-tracking/taint-tracking-impl.js	92.85%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3977      +/-   ##
==========================================
+ Coverage   85.15%   85.31%   +0.15%     
==========================================
  Files         243      247       +4     
  Lines       10504    10634     +130     
  Branches       33       33              
==========================================
+ Hits         8945     9072     +127     
- Misses       1559     1562       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…n JSON.parse

…s in taint-tracking-impl

jbertran · 2024-01-31T11:31:58Z

        .setCheckpoint(['direction:in', `group:${groupId}`, `topic:${topic}`, 'type:kafka'], span, payloadSize)
    }
+
+    if (aferStartCh.hasSubscribers) {


Suggested change

if (aferStartCh.hasSubscribers) {

if (afterStartCh.hasSubscribers) {

We should have a test that spys whether the afterStartCh handler was called (or some way to verify this behaviour E2E).

You're right, Igor I miss some kafka tests, like we have for express in packages/dd-trace/test/appsec/iast/analyzers/unvalidated-redirect-analyzer.express.plugin.spec.js

Something testing that we are detecting vulnerabilities in kafka.

PS: the typo is in var definition also, so it is not a bug, just a typo :-)

oops, totally forgotten

added tests https://github.com/DataDog/dd-trace-js/pull/3977/files#diff-ccef7a05e2500e71ad27e4a0493e300d69ac9dc1e7846cc982a94bb3399470de

* kafka poc * IastContextPlugin tests * More tests * Upgrade taint-tracking version to 1.7.0 * More tests * Increase tests coverage * Rename kafka message sources * Remove parameterName from kafka tainteds and propagate tainted type in JSON.parse * Move taintObject method to its own module to avoid circular references in taint-tracking-impl * Remove json.value from source-types * Rename kafkajs channels * add afterStart and beforeFinish tests and fix typo * Remove not used handler from startCtxOn and finishCtxOn * Fix kafkajs tests * Sort csiMethod * Kafka message tainting integration test

iunanua added semver-minor asm-iast labels Jan 18, 2024

iunanua commented Jan 18, 2024

View reviewed changes

Comment thread packages/datadog-plugin-kafkajs/src/consumer.js Outdated

uurien reviewed Jan 18, 2024

View reviewed changes

Comment thread packages/dd-trace/src/appsec/iast/taint-tracking/source-types.js Outdated

iunanua commented Jan 18, 2024

View reviewed changes

Comment thread packages/dd-trace/src/appsec/iast/taint-tracking/index.js Outdated

iunanua commented Jan 18, 2024

View reviewed changes

Comment thread packages/dd-trace/src/appsec/iast/taint-tracking/operations.js Outdated

iunanua added 2 commits January 19, 2024 09:02

kafka poc

1db1b54

IastContextPlugin tests

4fb8eea

iunanua force-pushed the igor/kafka-iast branch from b2bec20 to 4fb8eea Compare January 19, 2024 08:02

iunanua added 3 commits January 19, 2024 13:08

More tests

1f3ff59

Upgrade taint-tracking version to 1.7.0

e6d24a6

More tests

8472547

iunanua force-pushed the igor/kafka-iast branch from 6a02a47 to 8472547 Compare January 22, 2024 11:27

iunanua added 4 commits January 22, 2024 13:10

Increase tests coverage

b08c855

Rename kafka message sources

c3b62a0

Remove parameterName from kafka tainteds and propagate tainted type i…

5f7ddc3

…n JSON.parse

Move taintObject method to its own module to avoid circular reference…

9e10e63

…s in taint-tracking-impl

iunanua commented Jan 31, 2024

View reviewed changes

Comment thread packages/datadog-plugin-kafkajs/src/consumer.js Outdated

iunanua added 2 commits January 31, 2024 09:28

Remove json.value from source-types

07bac87

Rename kafkajs channels

168afc3

iunanua force-pushed the igor/kafka-iast branch from f5cb30f to 168afc3 Compare January 31, 2024 11:00

iunanua marked this pull request as ready for review January 31, 2024 11:23

iunanua requested review from a team as code owners January 31, 2024 11:23

iunanua requested a review from jbertran January 31, 2024 11:23

jbertran reviewed Jan 31, 2024

View reviewed changes

Fix test :S

2e4699c

Qard previously approved these changes Feb 2, 2024

View reviewed changes

iunanua added 2 commits February 5, 2024 11:52

Fix plugin tests

3f889cc

Kafka message tainting integration test

3eaec2d

iunanua dismissed Qard’s stale review via 3eaec2d February 5, 2024 16:37

iunanua added 4 commits February 6, 2024 09:04

Merge branch 'master' into igor/kafka-iast

51faefa

Restore dependencies order

7c2f833

sort property

f94c758

sort property again

7a6bdb4

iunanua requested review from CarlesDD, Qard, jbertran and uurien February 6, 2024 08:45

Clean up

33d7b30

uurien reviewed Feb 7, 2024

View reviewed changes

Clean up tests

a6cee13

iunanua requested a review from uurien February 15, 2024 09:28

uurien approved these changes Feb 15, 2024

View reviewed changes

iunanua merged commit 192ab40 into master Feb 16, 2024

iunanua deleted the igor/kafka-iast branch February 16, 2024 09:42

CarlesDD mentioned this pull request Feb 19, 2024

v5.4.0 #4075

Merged

CarlesDD mentioned this pull request Feb 20, 2024

v3.49.0 #4077

Merged

CarlesDD mentioned this pull request Feb 20, 2024

v4.28.0 #4078

Merged

erikvanbrakel mentioned this pull request Jul 5, 2024

[DEVX-3565] add neo4j plugin for v4.41 mazedesignhq/dd-trace-js#15

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Iast kafka consumer support#3977

Iast kafka consumer support#3977
iunanua merged 26 commits into
masterfrom
igor/kafka-iast

iunanua commented Jan 18, 2024 •

edited

Loading

Uh oh!

pr-commenter Bot commented Jan 18, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jan 19, 2024 •

edited

Loading

Uh oh!

codecov Bot commented Jan 19, 2024 •

edited

Loading

Uh oh!

Uh oh!

jbertran Jan 31, 2024

Uh oh!

uurien Jan 31, 2024 •

edited

Loading

Uh oh!

iunanua Jan 31, 2024

Uh oh!

iunanua Jan 31, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	if (aferStartCh.hasSubscribers) {
	if (afterStartCh.hasSubscribers) {

Uh oh!

Conversation

iunanua commented Jan 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Plugin Checklist

Additional Notes

Security

Uh oh!

pr-commenter Bot commented Jan 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

scenario:plugin-graphql-with-depth-and-collapse-on-18

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jan 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overall package size

Dependency sizes

Uh oh!

codecov Bot commented Jan 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

jbertran Jan 31, 2024

Choose a reason for hiding this comment

Uh oh!

uurien Jan 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iunanua Jan 31, 2024

Choose a reason for hiding this comment

Uh oh!

iunanua Jan 31, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

iunanua commented Jan 18, 2024 •

edited

Loading

pr-commenter Bot commented Jan 18, 2024 •

edited

Loading

github-actions Bot commented Jan 19, 2024 •

edited

Loading

codecov Bot commented Jan 19, 2024 •

edited

Loading

uurien Jan 31, 2024 •

edited

Loading