Skip to content

Implementation of the basic classes that will be used in IAST and the insecure hashing vulnerability detection#3225

Merged
NachoEchevarria merged 66 commits into
masterfrom
nacho/insecureHashing
Oct 13, 2022
Merged

Implementation of the basic classes that will be used in IAST and the insecure hashing vulnerability detection#3225
NachoEchevarria merged 66 commits into
masterfrom
nacho/insecureHashing

Conversation

@NachoEchevarria

Copy link
Copy Markdown
Collaborator

Summary of changes

Implementation of insecure hashing and some of the basic classes that will be used in IAST, taking as a model the Java implementation.

Reason for change

The IAST implementation starts with the insecure hashing implementation.

Implementation details

Implementation of insecure hashing and some of the basic classes that will be used in IAST, taking as a model the Java implementation.

Work included:
Hashing instrumentation for netcore.
Weak hashing detection and span creation.
Basic IAST infrastructure classes and configuration.
Integration tests for weak hashing calls.

Work NOT included:
Implement the request related variables:
https://datadoghq.atlassian.net/wiki/spaces/APS/pages/2604597520/ASM+IAST+Configuration+Properties
Implement DD_IAST_MAX_CONCURRENT_REQUESTS
Implement DD_IAST_MAX_CONTEXT_OPERATIONS
Implement DD_IAST_REQUEST_SAMPLING
Avoid sending repeated vulnerabilities within a request
Net framework support is not implemented in this PR: it would require to instrumentate mscorlib

Notes:
Right now, with this PR, we send a span everytime a weak hashing method is called. In the future, if we are in a request, we will send all the vulnerabilities after the request finishes and we will eliminate the repeated ones.
In the IastJson label of the span, a Json is sent containing the vulnerability data. The filename and line number of the vulnerability should be sent if possible. Otherwise, the full methodName is sent.
We were using in the tracer the class MD5 for uuid generation. This algorithm is considered insecure and would raise a span. The callsite instrumentation will avoid us sending spans generated from our own code. Anyway, the MD5 class has been replaced by a secure one.
Right now, we always set the IastEnabled tag to "1" in the span, but in the future, it might be zero to indicate that a request has not been analyzed

This is the actual tag field of the span:
Tags: {
_dd.iast.enabled: 1,
_dd.iast.json:
{
"Vulnerabilities": [
{
"Type": "WEAK_HASH",
"Location": {
"Path": "c:\myfile.cs",
"Line": 50
},
"Evidence": {
"Value": "System.Security.Cryptography.HMACMD5"
}
}
]
}
}

Test coverage

Other details

@NachoEchevarria
NachoEchevarria requested review from a team as code owners September 16, 2022 16:20
@NachoEchevarria NachoEchevarria changed the title Nacho/insecure hashing Implementation the basic classes that will be used in IAST and the insecure hashing vulnerability detection Sep 16, 2022
@andrewlock

This comment has been minimized.

@andrewlock

This comment has been minimized.

@robertpi
robertpi marked this pull request as draft September 19, 2022 09:31
@robertpi

Copy link
Copy Markdown
Member

Converted this to a draft, as it has failing tests that need to be addressed.

@andrewlock

This comment has been minimized.

@andrewlock

This comment has been minimized.

@andrewlock

This comment has been minimized.

@andrewlock

This comment has been minimized.

@andrewlock

This comment has been minimized.

@andrewlock

This comment has been minimized.

@andrewlock

This comment has been minimized.

@andrewlock

This comment has been minimized.

@andrewlock

This comment has been minimized.

@andrewlock

This comment has been minimized.

@NachoEchevarria
NachoEchevarria marked this pull request as ready for review October 5, 2022 09:24
@NachoEchevarria
NachoEchevarria requested a review from a team as a code owner October 5, 2022 09:24
@andrewlock

This comment has been minimized.

@andrewlock

This comment has been minimized.

@andrewlock

This comment has been minimized.

@andrewlock

This comment has been minimized.

@andrewlock

Copy link
Copy Markdown
Member

Benchmarks Report 🐌

Benchmarks for #3225 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 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net472 787μs 594ns 2.3μs 0.393 0 0 3.22 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 506μs 563ns 2.18μs 0 0 0 2.63 KB
#3225 WriteAndFlushEnrichedTraces net472 782μs 798ns 3.09μs 0.391 0 0 3.22 KB
#3225 WriteAndFlushEnrichedTraces netcoreapp3.1 533μs 183ns 684ns 0 0 0 2.63 KB
Benchmarks.Trace.AppSecBodyBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master AllCycleSimpleBody net472 199ns 0.0442ns 0.165ns 0.0727 0 0 457 B
master AllCycleSimpleBody netcoreapp3.1 264ns 0.785ns 2.72ns 0.00628 0 0 456 B
master AllCycleMoreComplexBody net472 201ns 0.0299ns 0.104ns 0.0688 0 0 433 B
master AllCycleMoreComplexBody netcoreapp3.1 249ns 0.125ns 0.486ns 0.00584 0 0 432 B
master BodyExtractorSimpleBody net472 283ns 1.15ns 4.44ns 0.0574 0 0 361 B
master BodyExtractorSimpleBody netcoreapp3.1 236ns 0.0874ns 0.327ns 0.00367 0 0 272 B
master BodyExtractorMoreComplexBody net472 15.8μs 13ns 50.5ns 1.21 0.0157 0 7.62 KB
master BodyExtractorMoreComplexBody netcoreapp3.1 12.7μs 3.42ns 12.8ns 0.0885 0 0 6.75 KB
#3225 AllCycleSimpleBody net472 197ns 0.0418ns 0.151ns 0.0727 0 0 457 B
#3225 AllCycleSimpleBody netcoreapp3.1 252ns 0.177ns 0.686ns 0.00631 0 0 456 B
#3225 AllCycleMoreComplexBody net472 194ns 0.0456ns 0.164ns 0.0688 0 0 433 B
#3225 AllCycleMoreComplexBody netcoreapp3.1 256ns 0.179ns 0.695ns 0.006 0 0 432 B
#3225 BodyExtractorSimpleBody net472 275ns 0.2ns 0.775ns 0.0573 0 0 361 B
#3225 BodyExtractorSimpleBody netcoreapp3.1 234ns 0.145ns 0.543ns 0.00377 0 0 272 B
#3225 BodyExtractorMoreComplexBody net472 16.6μs 16.5ns 63.8ns 1.21 0.0166 0 7.62 KB
#3225 BodyExtractorMoreComplexBody netcoreapp3.1 13.2μs 7.32ns 27.4ns 0.0857 0 0 6.75 KB
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendRequest net472 0ns 0ns 0ns 0 0 0 0 b
master SendRequest netcoreapp3.1 181μs 225ns 872ns 0.271 0 0 20.68 KB
#3225 SendRequest net472 0ns 0ns 0ns 0 0 0 0 b
#3225 SendRequest netcoreapp3.1 180μs 169ns 631ns 0.182 0 0 20.68 KB
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteNonQuery net472 1.75μs 0.822ns 3.19ns 0.155 0.000874 0 979 B
master ExecuteNonQuery netcoreapp3.1 1.42μs 1.31ns 5.08ns 0.0128 0 0 968 B
#3225 ExecuteNonQuery net472 1.84μs 0.637ns 2.47ns 0.155 0.000924 0 979 B
#3225 ExecuteNonQuery netcoreapp3.1 1.45μs 0.372ns 1.34ns 0.0131 0 0 968 B
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master CallElasticsearch net472 2.52μs 0.66ns 2.56ns 0.188 0 0 1.19 KB
master CallElasticsearch netcoreapp3.1 1.51μs 0.516ns 1.86ns 0.0151 0 0 1.13 KB
master CallElasticsearchAsync net472 2.71μs 0.583ns 2.1ns 0.21 0 0 1.32 KB
master CallElasticsearchAsync netcoreapp3.1 1.66μs 0.422ns 1.58ns 0.0167 0 0 1.25 KB
#3225 CallElasticsearch net472 2.58μs 2.39ns 9.24ns 0.187 0 0 1.19 KB
#3225 CallElasticsearch netcoreapp3.1 1.56μs 0.703ns 2.63ns 0.0151 0 0 1.13 KB
#3225 CallElasticsearchAsync net472 2.78μs 0.918ns 3.43ns 0.21 0 0 1.32 KB
#3225 CallElasticsearchAsync netcoreapp3.1 1.65μs 0.372ns 1.34ns 0.0165 0 0 1.25 KB
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteAsync net472 2.94μs 1.31ns 5.08ns 0.23 0 0 1.45 KB
master ExecuteAsync netcoreapp3.1 1.73μs 0.653ns 2.35ns 0.0182 0 0 1.38 KB
#3225 ExecuteAsync net472 2.79μs 1.16ns 4.34ns 0.23 0 0 1.45 KB
#3225 ExecuteAsync netcoreapp3.1 1.82μs 0.575ns 1.99ns 0.0182 0 0 1.38 KB
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendAsync net472 5.93μs 1.8ns 6.72ns 0.443 0 0 2.8 KB
master SendAsync netcoreapp3.1 3.78μs 1.29ns 4.66ns 0.0361 0 0 2.63 KB
#3225 SendAsync net472 5.96μs 2.7ns 9.74ns 0.444 0 0 2.8 KB
#3225 SendAsync netcoreapp3.1 3.63μs 1.71ns 6.41ns 0.0359 0 0 2.63 KB
Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net472 3.19μs 3.1ns 12ns 0.298 0 0 1.88 KB
master EnrichedLog netcoreapp3.1 2.57μs 0.867ns 3.24ns 0.026 0 0 1.91 KB
#3225 EnrichedLog net472 3.08μs 2.68ns 10ns 0.297 0 0 1.88 KB
#3225 EnrichedLog netcoreapp3.1 2.44μs 1.3ns 5.03ns 0.0256 0 0 1.91 KB
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net472 151μs 84.6ns 317ns 0.679 0.226 0 4.72 KB
master EnrichedLog netcoreapp3.1 115μs 137ns 514ns 0 0 0 4.55 KB
#3225 EnrichedLog net472 150μs 93.1ns 360ns 0.749 0.225 0 4.72 KB
#3225 EnrichedLog netcoreapp3.1 122μs 224ns 869ns 0.0608 0 0 4.55 KB
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net472 5.66μs 1.27ns 4.42ns 0.578 0.00284 0 3.65 KB
master EnrichedLog netcoreapp3.1 4.28μs 2.51ns 9.38ns 0.0537 0 0 3.98 KB
#3225 EnrichedLog net472 5.77μs 2.33ns 8.73ns 0.578 0.0029 0 3.65 KB
#3225 EnrichedLog netcoreapp3.1 4.41μs 2.95ns 11.4ns 0.0526 0 0 3.98 KB
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendReceive net472 2.38μs 2.29ns 8.87ns 0.223 0 0 1.4 KB
master SendReceive netcoreapp3.1 1.84μs 0.605ns 2.26ns 0.0185 0 0 1.35 KB
#3225 SendReceive net472 2.34μs 1.59ns 5.93ns 0.223 0 0 1.4 KB
#3225 SendReceive netcoreapp3.1 1.85μs 0.468ns 1.75ns 0.0185 0 0 1.35 KB
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net472 5.18μs 2.13ns 7.67ns 0.363 0 0 2.3 KB
master EnrichedLog netcoreapp3.1 4.37μs 2.34ns 8.77ns 0.024 0 0 1.86 KB
#3225 EnrichedLog net472 5.06μs 1.88ns 7.04ns 0.365 0 0 2.3 KB
#3225 EnrichedLog netcoreapp3.1 4.15μs 1.47ns 5.3ns 0.0249 0 0 1.86 KB
Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net472 1.16μs 0.358ns 1.39ns 0.133 0 0 842 B
master StartFinishSpan netcoreapp3.1 968ns 0.36ns 1.35ns 0.0106 0 0 792 B
master StartFinishScope net472 1.49μs 0.257ns 0.928ns 0.146 0 0 923 B
master StartFinishScope netcoreapp3.1 1.08μs 0.372ns 1.39ns 0.0124 0 0 912 B
#3225 StartFinishSpan net472 1.22μs 0.256ns 0.959ns 0.134 0 0 842 B
#3225 StartFinishSpan netcoreapp3.1 953ns 0.528ns 1.98ns 0.0106 0 0 792 B
#3225 StartFinishScope net472 1.37μs 0.752ns 2.91ns 0.146 0 0 923 B
#3225 StartFinishScope netcoreapp3.1 1.13μs 0.382ns 1.43ns 0.0123 0 0 912 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunOnMethodBegin net472 1.47μs 0.201ns 0.698ns 0.146 0 0 923 B
master RunOnMethodBegin netcoreapp3.1 1.17μs 0.403ns 1.51ns 0.0123 0 0 912 B
#3225 RunOnMethodBegin net472 1.58μs 0.71ns 2.75ns 0.146 0 0 923 B
#3225 RunOnMethodBegin netcoreapp3.1 1.22μs 2.7ns 10.5ns 0.0121 0 0 912 B

@NachoEchevarria NachoEchevarria changed the title Implementation the basic classes that will be used in IAST and the insecure hashing vulnerability detection Implementation of the basic classes that will be used in IAST and the insecure hashing vulnerability detection Oct 13, 2022
@andrewlock

Copy link
Copy Markdown
Member

Code Coverage Report 📊

⚠️ Merging #3225 into master will will decrease line coverage by 1%
⚠️ Merging #3225 into master will will decrease branch coverage by 1%
⛔ Merging #3225 into master will will increase complexity by 169

master #3225 Change
Lines 19367 / 26705 19358 / 26924
Lines % 73% 72% -1% ⚠️
Branches 11527 / 16615 11479 / 16747
Branches % 69% 69% -1% ⚠️
Complexity 17944 18113 169

View the full report for further details:

Datadog.Trace Breakdown ⚠️

master #3225 Change
Lines % 73% 72% -1% ⚠️
Branches % 69% 69% -1% ⚠️
Complexity 17944 18113 169

The following classes have significant coverage changes.

File Line coverage change Branch coverage change Complexity change
Datadog.Trace.Agent.TraceSamplers.AnalyticsEventsSampler -100% -100% 0 ✔️
Datadog.Trace.Agent.TraceSamplers.ErrorSampler -100% -100% 0 ✔️
Datadog.Trace.Agent.TraceSamplers.PrioritySampler -100% 0% ✔️ 0 ✔️
Datadog.Trace.Sampling.GlobalSamplingRule -100% 0% ✔️ 0 ✔️
Datadog.Trace.Agent.TraceSamplers.RareSampler -89% -96% 0 ✔️
Datadog.Trace.Util.SamplingHelpers -50% -50% 0 ✔️
Datadog.Trace.Telemetry.Transports.JsonTelemetryTransport -33% -42% 0 ✔️
Datadog.Trace.Logging.DirectSubmission.DirectLogSubmissionManager -33% -33% 0 ✔️
Datadog.Trace.Logging.DirectSubmission.Sink.NullDatadogSink -25% 0% ✔️ 0 ✔️
Datadog.Trace.Processors.NormalizerTraceProcessor -22% -19% 0 ✔️
...And 8 more

The following classes were added in #3225:

File Line coverage Branch coverage Complexity
Datadog.Trace.ClrProfiler.AutoInstrumentation.HashAlgorithm.HashAlgorithmIntegration 100% 100% 2
Datadog.Trace.ClrProfiler.AutoInstrumentation.HashAlgorithm.HashAlgorithmIntegrationBis 100% 100% 2
Datadog.Trace.ClrProfiler.AutoInstrumentation.HashAlgorithm.HashAlgorithmIntegrationCommon 50% 58% 13
Datadog.Trace.ClrProfiler.AutoInstrumentation.HashAlgorithm.HashAlgorithmIntegrationTer 100% 100% 2
Datadog.Trace.Iast.Evidence 100% 100% 1
...And 11 more

View the full reports for further details:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants