IAST: Weak cipher algorithms detection by NachoEchevarria · Pull Request #3353 · DataDog/dd-trace-dotnet

NachoEchevarria · 2022-10-13T14:42:58Z

Summary of changes

The weak cipher algorithms detection is implemented in this PR. This detection works, for now, only in netcore apps. The support to the .net framework will be extended after the callsite instrumentation implementation.

Reason for change

This work is included in the IAST chapter0 delivery.

Implementation details

Test coverage

Other details

andrewlock

LGTM 👍 just a few minor comments 🙂

Co-authored-by: Andrew Lock <[email protected]>

…net framework. Minor cosmetic fixes.

andrewlock · 2022-10-18T15:19:09Z

Benchmarks Report 🐌

Benchmarks for #3353 compared to master:

All benchmarks have the same speed
All benchmarks have the same allocations

The following thresholds were used for comparing the benchmark speeds:

Mann–Whitney U test with statistical test for significance of 5%
Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`WriteAndFlushEnrichedTraces`	net472	808μs	590ns	2.21μs	0.403	3.22 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	556μs	272ns	1.02μs	0	2.62 KB
#3353	`WriteAndFlushEnrichedTraces`	net472	803μs	815ns	3.16μs	0.401	3.22 KB
#3353	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	562μs	251ns	938ns	0	2.62 KB

Benchmarks.Trace.AppSecBodyBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net472	197ns	0.0869ns	0.313ns	0.0701	9.85E-05	441 B
master	`AllCycleSimpleBody`	netcoreapp3.1	257ns	0.24ns	0.928ns	0.00605	0	440 B
master	`AllCycleMoreComplexBody`	net472	196ns	0.0453ns	0.17ns	0.0663	0	417 B
master	`AllCycleMoreComplexBody`	netcoreapp3.1	252ns	0.244ns	0.945ns	0.00567	0	416 B
master	`BodyExtractorSimpleBody`	net472	270ns	0.365ns	1.37ns	0.0573	0	361 B
master	`BodyExtractorSimpleBody`	netcoreapp3.1	244ns	0.123ns	0.461ns	0.00368	0	272 B
master	`BodyExtractorMoreComplexBody`	net472	16.3μs	17.7ns	68.4ns	1.2	0.0163	7.62 KB
master	`BodyExtractorMoreComplexBody`	netcoreapp3.1	12.9μs	6.84ns	26.5ns	0.0895	0	6.75 KB
#3353	`AllCycleSimpleBody`	net472	195ns	0.106ns	0.395ns	0.0701	0	441 B
#3353	`AllCycleSimpleBody`	netcoreapp3.1	253ns	0.283ns	1.1ns	0.00607	0	440 B
#3353	`AllCycleMoreComplexBody`	net472	196ns	0.101ns	0.39ns	0.0662	0	417 B
#3353	`AllCycleMoreComplexBody`	netcoreapp3.1	251ns	0.154ns	0.596ns	0.00571	0	416 B
#3353	`BodyExtractorSimpleBody`	net472	270ns	0.298ns	1.12ns	0.0574	0	361 B
#3353	`BodyExtractorSimpleBody`	netcoreapp3.1	234ns	0.154ns	0.556ns	0.00379	0	272 B
#3353	`BodyExtractorMoreComplexBody`	net472	15.7μs	13.8ns	51.7ns	1.21	0.0155	7.62 KB
#3353	`BodyExtractorMoreComplexBody`	netcoreapp3.1	12.8μs	4.14ns	16.1ns	0.0896	0	6.75 KB

Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendRequest`	net472	0ns	0ns	0ns	0	0 b
master	`SendRequest`	netcoreapp3.1	184μs	186ns	720ns	0.274	20.66 KB
#3353	`SendRequest`	net472	0ns	0ns	0ns	0	0 b
#3353	`SendRequest`	netcoreapp3.1	183μs	262ns	1.01μs	0.183	20.66 KB

Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`ExecuteNonQuery`	net472	1.81μs	0.509ns	1.91ns	0.158	0.000903	995 B
master	`ExecuteNonQuery`	netcoreapp3.1	1.4μs	0.516ns	1.86ns	0.0133	0	984 B
#3353	`ExecuteNonQuery`	net472	1.74μs	0.61ns	2.28ns	0.157	0.000874	995 B
#3353	`ExecuteNonQuery`	netcoreapp3.1	1.47μs	0.452ns	1.63ns	0.0132	0	984 B

Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`CallElasticsearch`	net472	2.5μs	0.855ns	3.08ns	0.191	1.2 KB
master	`CallElasticsearch`	netcoreapp3.1	1.51μs	0.938ns	3.51ns	0.0158	1.14 KB
master	`CallElasticsearchAsync`	net472	2.63μs	0.441ns	1.59ns	0.212	1.34 KB
master	`CallElasticsearchAsync`	netcoreapp3.1	1.58μs	0.483ns	1.74ns	0.0173	1.26 KB
#3353	`CallElasticsearch`	net472	2.53μs	0.727ns	2.81ns	0.19	1.2 KB
#3353	`CallElasticsearch`	netcoreapp3.1	1.52μs	1.05ns	3.78ns	0.0153	1.14 KB
#3353	`CallElasticsearchAsync`	net472	2.69μs	0.911ns	3.53ns	0.212	1.34 KB
#3353	`CallElasticsearchAsync`	netcoreapp3.1	1.58μs	0.43ns	1.49ns	0.017	1.26 KB

Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net472	2.77μs	3.81ns	14.2ns	0.233	1.47 KB
master	`ExecuteAsync`	netcoreapp3.1	1.74μs	1.45ns	5.41ns	0.0184	1.39 KB
#3353	`ExecuteAsync`	net472	2.83μs	1.08ns	4.05ns	0.232	1.47 KB
#3353	`ExecuteAsync`	netcoreapp3.1	1.68μs	0.671ns	2.51ns	0.0193	1.39 KB

Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net472	5.96μs	1.27ns	4.77ns	0.446	2.82 KB
master	`SendAsync`	netcoreapp3.1	3.69μs	1.98ns	7.68ns	0.035	2.65 KB
#3353	`SendAsync`	net472	5.85μs	1.99ns	7.45ns	0.447	2.82 KB
#3353	`SendAsync`	netcoreapp3.1	3.68μs	3.78ns	14.1ns	0.0363	2.65 KB

Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	3.23μs	4.46ns	16.1ns	0.297	1.88 KB
master	`EnrichedLog`	netcoreapp3.1	2.51μs	1.84ns	6.9ns	0.0264	1.91 KB
#3353	`EnrichedLog`	net472	3.31μs	3.52ns	13.2ns	0.297	1.88 KB
#3353	`EnrichedLog`	netcoreapp3.1	2.54μs	3.06ns	11.9ns	0.0265	1.91 KB

Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net472	151μs	170ns	657ns	0.679	0.226	4.72 KB
master	`EnrichedLog`	netcoreapp3.1	121μs	136ns	510ns	0.0597	0	4.55 KB
#3353	`EnrichedLog`	net472	152μs	194ns	751ns	0.689	0.23	4.72 KB
#3353	`EnrichedLog`	netcoreapp3.1	120μs	213ns	797ns	0.0597	0	4.55 KB

Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net472	5.66μs	1.64ns	6.35ns	0.579	0.00284	3.65 KB
master	`EnrichedLog`	netcoreapp3.1	4.42μs	1.63ns	6.09ns	0.053	0	3.98 KB
#3353	`EnrichedLog`	net472	5.83μs	1.59ns	5.73ns	0.578	0.0029	3.65 KB
#3353	`EnrichedLog`	netcoreapp3.1	4.57μs	3.07ns	11.5ns	0.0546	0	3.98 KB

Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendReceive`	net472	2.24μs	4.02ns	15.6ns	0.226	1.42 KB
master	`SendReceive`	netcoreapp3.1	1.84μs	0.668ns	2.59ns	0.0184	1.37 KB
#3353	`SendReceive`	net472	2.33μs	2.29ns	8.86ns	0.225	1.42 KB
#3353	`SendReceive`	netcoreapp3.1	1.82μs	0.784ns	2.93ns	0.0192	1.37 KB

Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	4.92μs	3.09ns	12ns	0.365	2.3 KB
master	`EnrichedLog`	netcoreapp3.1	4.34μs	2.07ns	7.45ns	0.0242	1.86 KB
#3353	`EnrichedLog`	net472	5.14μs	3.14ns	11.7ns	0.364	2.3 KB
#3353	`EnrichedLog`	netcoreapp3.1	4.17μs	0.83ns	3.1ns	0.0251	1.86 KB

Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net472	1.18μs	0.48ns	1.8ns	0.136	859 B
master	`StartFinishSpan`	netcoreapp3.1	936ns	0.362ns	1.31ns	0.011	808 B
master	`StartFinishScope`	net472	1.42μs	0.462ns	1.73ns	0.149	939 B
master	`StartFinishScope`	netcoreapp3.1	1.04μs	0.754ns	2.92ns	0.0125	928 B
#3353	`StartFinishSpan`	net472	1.16μs	0.771ns	2.89ns	0.136	859 B
#3353	`StartFinishSpan`	netcoreapp3.1	979ns	1.43ns	5.54ns	0.0106	808 B
#3353	`StartFinishScope`	net472	1.39μs	0.317ns	1.23ns	0.149	939 B
#3353	`StartFinishScope`	netcoreapp3.1	1.02μs	0.57ns	2.21ns	0.0127	928 B

Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net472	1.44μs	0.546ns	2.12ns	0.149	939 B
master	`RunOnMethodBegin`	netcoreapp3.1	1.17μs	0.438ns	1.7ns	0.0122	928 B
#3353	`RunOnMethodBegin`	net472	1.46μs	0.859ns	3.33ns	0.149	939 B
#3353	`RunOnMethodBegin`	netcoreapp3.1	1.17μs	0.385ns	1.44ns	0.0124	928 B

andrewlock · 2022-10-18T15:49:19Z

Code Coverage Report 📊

✔️ Merging #3353 into master will not change line coverage
✔️ Merging #3353 into master will not change branch coverage
✔️ Merging #3353 into master will will decrease complexity by 53

	master	#3353	Change
Lines	`19602` / `27011`	`19598` / `27028`
Lines %	`73%`	`73%`	`0%` ✔️
Branches	`11633` / `16784`	`11593` / `16728`
Branches %	`69%`	`69%`	`0%` ✔️
Complexity	`18160`	`18107`	`-53` ✔️

View the full report for further details:

Datadog.Trace Breakdown ✔️

	master	#3353	Change
Lines %	`73%`	`73%`	`0%` ✔️
Branches %	`69%`	`69%`	`0%` ✔️
Complexity	`18160`	`18107`	`-53` ✔️

The following classes have significant coverage changes.

File	Line coverage change	Branch coverage change	Complexity change
Datadog.Trace.Iast.StackWalker	`0%` ✔️	`6%` ✔️	`0` ✔️
Datadog.Trace.Iast.IastModule	`11%` ✔️	`17%` ✔️	`18` ⛔

The following classes were added in #3353:

File	Line coverage	Branch coverage	Complexity
Datadog.Trace.ClrProfiler.AutoInstrumentation.CryptographyAlgorithm.SymmetricAlgorithmIntegration	`100%`	`100%`	`2`
Datadog.Trace.ClrProfiler.AutoInstrumentation.CryptographyAlgorithm.SymmetricAlgorithmIntegrationCommon	`44%`	`50%`	`5`

View the full reports for further details:

andrewlock

Looks good, thanks 👍

This comment has been minimized.

Sign in to view

NachoEchevarria closed this Oct 15, 2022

NachoEchevarria reopened this Oct 15, 2022

NachoEchevarria force-pushed the nacho/weakCipher branch from 96b40d7 to 31b9110 Compare October 15, 2022 09:49

This comment has been minimized.

Sign in to view

NachoEchevarria changed the title ~~Nacho/weak cipher~~ IAST: Weak cipher algorithms detection Oct 17, 2022

NachoEchevarria marked this pull request as ready for review October 17, 2022 07:29

NachoEchevarria requested review from a team as code owners October 17, 2022 07:29

This comment has been minimized.

Sign in to view

robertpi reviewed Oct 18, 2022

View reviewed changes

Comment thread ...Trace/ClrProfiler/AutoInstrumentation/CryptographyAlgorithm/SymmetricAlgorithmIntegration.cs Outdated

robertpi approved these changes Oct 18, 2022

View reviewed changes

andrewlock reviewed Oct 18, 2022

View reviewed changes

NachoEchevarria and others added 20 commits October 18, 2022 16:21

Fix

3afa8b1

fix comments

9ed6d0a

Fix linux weak cipher tests

edd79fa

Move comment

4902a6d

fix linux tests

bdaa112

debug: find CI stack

e549f63

Send more info in the evidence

2cd791b

Add debug info in evidence

655a29c

test again

f84c7b9

fix tests

6d99f8f

Avoid providers launching double spans

7368171

Integration test fix.

14b0864

Check for integration enabled tests

3b27de0

minor spell fix

c72dee6

Update tracer/src/Datadog.Trace/Configuration/ConfigurationKeys.Iast.cs

1bafa01

Co-authored-by: Andrew Lock <[email protected]>

Update tracer/src/Datadog.Trace/Configuration/ConfigurationKeys.Iast.cs

e2f5ea5

Co-authored-by: Andrew Lock <[email protected]>

Update tracer/src/Datadog.Trace/IAST/IastModule.cs

489e95d

Co-authored-by: Andrew Lock <[email protected]>

Update tracer/src/Datadog.Trace/IAST/IastModule.cs

b35f780

Co-authored-by: Andrew Lock <[email protected]>

Use file scoped namespaces. Exclude weak cipher instrumentation for .…

ab9937a

…net framework. Minor cosmetic fixes.

remove not needed code

bef37ef

NachoEchevarria force-pushed the nacho/weakCipher branch from 79d007a to bef37ef Compare October 18, 2022 14:24

Add comment

f13612c

andrewlock approved these changes Oct 18, 2022

View reviewed changes

NachoEchevarria merged commit 164a816 into master Oct 18, 2022

NachoEchevarria deleted the nacho/weakCipher branch October 18, 2022 16:15

github-actions Bot added this to the vNext milestone Oct 18, 2022

anna-git added area:asm area:asm-iast labels Oct 25, 2022

Uh oh!

Conversation

NachoEchevarria commented Oct 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Reason for change

Implementation details

Test coverage

Other details

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Uh oh!

andrewlock left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andrewlock commented Oct 18, 2022

Benchmarks Report 🐌

Benchmark details

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Uh oh!

andrewlock commented Oct 18, 2022

Code Coverage Report 📊

Datadog.Trace Breakdown ✔️

Uh oh!

andrewlock left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

NachoEchevarria commented Oct 13, 2022 •

edited

Loading