[ASM][IAST] Add source tainting for Grpc dotnet #5473
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
e-n-0
force-pushed
the
flavien/asm/iast/grpc_source
branch
from
9442d17
to
99b53b3
Compare
e-n-0
force-pushed
the
flavien/asm/iast/grpc_source
branch
from
99b53b3
to
3897009
Compare
Datadog ReportBranch report: ✅ 0 Failed, 327244 Passed, 1550 Skipped, 49m 47.38s Wall Time New Flaky Tests (1)
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing the following branches/commits: Execution-time benchmarks measure the whole time it takes to execute a program. And are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are shown in red. The following thresholds were used for comparing the execution times:
Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard. Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph). gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.6.2)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5473) - mean (73ms) : 66, 79
. : milestone, 73,
master - mean (74ms) : 64, 84
. : milestone, 74,
section CallTarget+Inlining+NGEN
This PR (5473) - mean (1,017ms) : 998, 1036
. : milestone, 1017,
master - mean (1,015ms) : 994, 1036
. : milestone, 1015,
gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5473) - mean (110ms) : 106, 115
. : milestone, 110,
master - mean (111ms) : 108, 114
. : milestone, 111,
section CallTarget+Inlining+NGEN
This PR (5473) - mean (737ms) : 713, 760
. : milestone, 737,
master - mean (737ms) : 714, 761
. : milestone, 737,
gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5473) - mean (94ms) : 91, 96
. : milestone, 94,
master - mean (95ms) : 91, 98
. : milestone, 95,
section CallTarget+Inlining+NGEN
This PR (5473) - mean (684ms) : 656, 712
. : milestone, 684,
master - mean (691ms) : 667, 716
. : milestone, 691,
gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5473) - mean (193ms) : 188, 198
. : milestone, 193,
master - mean (195ms) : 189, 201
. : milestone, 195,
section CallTarget+Inlining+NGEN
This PR (5473) - mean (1,091ms) : 1069, 1113
. : milestone, 1091,
master - mean (1,099ms) : 1071, 1127
. : milestone, 1099,
gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5473) - mean (279ms) : 273, 284
. : milestone, 279,
master - mean (284ms) : 277, 292
. : milestone, 284,
section CallTarget+Inlining+NGEN
This PR (5473) - mean (889ms) : 865, 913
. : milestone, 889,
master - mean (897ms) : 877, 917
. : milestone, 897,
gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5473) - mean (264ms) : 260, 268
. : milestone, 264,
master - mean (267ms) : 261, 274
. : milestone, 267,
section CallTarget+Inlining+NGEN
This PR (5473) - mean (869ms) : 843, 895
. : milestone, 869,
master - mean (873ms) : 850, 897
. : milestone, 873,
|
Benchmarks Report for tracer 🐌Benchmarks for #5473 compared to master:
The following thresholds were used for comparing the benchmark speeds:
Allocation changes below 0.5% are ignored. Benchmark detailsBenchmarks.Trace.ActivityBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️Raw results
|
Benchmarks Report for appsec 🐌Benchmarks for #5473 compared to master:
The following thresholds were used for comparing the benchmark speeds:
Allocation changes below 0.5% are ignored. Benchmark detailsBenchmarks.Trace.Asm.AppSecBodyBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.Asm.AppSecEncoderBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.Asm.AppSecWafBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ More allocations
|
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1 | 203.51 KB | 211.95 KB | 8.44 KB | 4.15% |
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0 | 215.61 KB | 213.94 KB | -1.66 KB | -0.77% |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | StringConcatBenchmark |
net6.0 | 60.4μs | 757ns | 7.49μs | 0 | 0 | 0 | 43.44 KB |
| master | StringConcatBenchmark |
netcoreapp3.1 | 62.7μs | 847ns | 8.47μs | 0 | 0 | 0 | 42.64 KB |
| master | StringConcatBenchmark |
net472 | 37.6μs | 74.3ns | 257ns | 0 | 0 | 0 | 61.87 KB |
| master | StringConcatAspectBenchmark |
net6.0 | 303μs | 1.31μs | 8.09μs | 0 | 0 | 0 | 215.61 KB |
| master | StringConcatAspectBenchmark |
netcoreapp3.1 | 307μs | 1.42μs | 5.13μs | 0 | 0 | 0 | 203.51 KB |
| master | StringConcatAspectBenchmark |
net472 | 264μs | 4.87μs | 46.4μs | 0 | 0 | 0 | 221.18 KB |
| #5473 | StringConcatBenchmark |
net6.0 | 60.8μs | 931ns | 9.31μs | 0 | 0 | 0 | 43.44 KB |
| #5473 | StringConcatBenchmark |
netcoreapp3.1 | 61.4μs | 907ns | 8.8μs | 0 | 0 | 0 | 42.64 KB |
| #5473 | StringConcatBenchmark |
net472 | 37.2μs | 80.8ns | 302ns | 0 | 0 | 0 | 61.82 KB |
| #5473 | StringConcatAspectBenchmark |
net6.0 | 290μs | 1.29μs | 4.83μs | 0 | 0 | 0 | 213.94 KB |
| #5473 | StringConcatAspectBenchmark |
netcoreapp3.1 | 310μs | 1.42μs | 7.37μs | 0 | 0 | 0 | 211.95 KB |
| #5473 | StringConcatAspectBenchmark |
net472 | 238μs | 3.07μs | 30μs | 0 | 0 | 0 | 221.18 KB |
tracer/src/Datadog.Trace.Trimming/build/Datadog.Trace.Trimming.xml
Outdated
Show resolved
Hide resolved
daniel-romano-DD
approved these changes
Apr 22, 2024
Throughput/Crank Report:zap:Throughput results for AspNetCoreSimpleController comparing the following branches/commits: Cases where throughput results for the PR are worse than latest master (5% drop or greater), results are shown in red. Note that these results are based on a single point-in-time result for each branch. For full results, see one of the many, many dashboards! gantt
title Throughput Linux x64 (Total requests)
dateFormat X
axisFormat %s
section Baseline
This PR (5473) (11.477M) : 0, 11477354
master (11.692M) : 0, 11692244
benchmarks/2.9.0 (11.965M) : 0, 11965184
section Automatic
This PR (5473) (7.673M) : 0, 7673264
master (7.853M) : 0, 7853275
benchmarks/2.9.0 (8.522M) : 0, 8521981
section Trace stats
master (8.249M) : 0, 8248908
section Manual
This PR (5473) (10.167M) : 0, 10166539
master (10.073M) : 0, 10072568
section Manual + Automatic
This PR (5473) (7.460M) : 0, 7460287
master (7.364M) : 0, 7363516
section Version Conflict
master (6.579M) : 0, 6579336
gantt
title Throughput Linux arm64 (Total requests)
dateFormat X
axisFormat %s
section Baseline
This PR (5473) (9.510M) : 0, 9510397
master (9.426M) : 0, 9426051
benchmarks/2.9.0 (9.684M) : 0, 9683662
section Automatic
This PR (5473) (6.363M) : 0, 6363153
master (6.685M) : 0, 6684971
section Trace stats
master (6.782M) : 0, 6781528
section Manual
This PR (5473) (8.235M) : 0, 8235370
master (8.176M) : 0, 8176096
section Manual + Automatic
This PR (5473) (6.171M) : 0, 6170650
master (6.104M) : 0, 6104327
section Version Conflict
master (5.580M) : 0, 5580419
gantt
title Throughput Windows x64 (Total requests)
dateFormat X
axisFormat %s
section Baseline
This PR (5473) (9.942M) : 0, 9941920
master (9.405M) : 0, 9405155
benchmarks/2.9.0 (9.684M) : 0, 9683680
section Automatic
This PR (5473) (6.881M) : 0, 6880532
master (6.833M) : 0, 6832698
benchmarks/2.9.0 (7.178M) : 0, 7177843
section Trace stats
master (7.114M) : 0, 7113893
section Manual
This PR (5473) (8.461M) : 0, 8461092
master (8.420M) : 0, 8420465
section Manual + Automatic
This PR (5473) (6.616M) : 0, 6615788
master (6.522M) : 0, 6521941
section Version Conflict
master (5.925M) : 0, 5924905
|
e-n-0
force-pushed
the
flavien/asm/iast/grpc_source
branch
2 times, most recently
from
2ec8010
to
696bf39
Compare
e-n-0
force-pushed
the
flavien/asm/iast/grpc_source
branch
7 times, most recently
from
9f32463
to
8b5e57d
Compare
andrewlock
approved these changes
May 3, 2024
tracer/test/Datadog.Trace.Security.IntegrationTests/IAST/Grpc/GrpcDotNetTests.cs
Show resolved
Hide resolved
tracer/test/Datadog.Trace.Security.IntegrationTests/IAST/Grpc/GrpcDotNetTests.cs
Outdated
Show resolved
Hide resolved
e-n-0
force-pushed
the
flavien/asm/iast/grpc_source
branch
from
daf703e
to
aecb079
Compare
tonyredondo
added a commit
that referenced
this pull request
May 10, 2024
* Initial commit with the CallTargetRefStruct.cs readonly struct * Managed test * Native code implementation * fixes * Fixes and test suite * revert * Add instrumentation on grpc request object + Visitor to track strings and taint them * Add new sample + integration test * Add grpc Source Type * Update Sample to add more tests * Add problematic instrumentation * Implement new instrumentation for reading grpc messages strings * Move Sample + Working integration tests * Add the missing telemetry * Revert debug on triming file * Remove Security Sample * Update integration tests on existing APM sample * Lower the minimum version of Google.Protobuf to support older versions * Missing integration in the group * Exclude net fx * Skip build on unsupported * Update snapshot with deduplication enabled * Scrub location * Applied comments --------- Co-authored-by: Tony Redondo <tony.redondo@datadoghq.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes
Add Grpc source tainting to Grpc for IAST.
Implementation details
This PR use the new call target ref struct changes.
Add a new call target instrumentation on the
Google.Protobufassembly:System.String Google.Protobuf.ParsingPrimitives::ReadRawStringAll strings parsed of all messages will be tainted.
Test coverage
Add Iast tests to the existing
Samples.GrpcDotnet.These tests are enabled through the environment variable
IAST_GRPC_SOURCE_TEST.All messages transmissions are tested: