Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[IAST] Cookie filter implementation #5947

Merged
merged 3 commits into from
Aug 29, 2024
Merged

[IAST] Cookie filter implementation #5947

merged 3 commits into from
Aug 29, 2024

Conversation

daniel-romano-DD
Copy link
Contributor

@daniel-romano-DD daniel-romano-DD commented Aug 26, 2024 •
edited
Loading

Summary of changes

Filter Cookie vulnerabilities by configurable regex as described in this RFC

dd-go PR here

Reason for change

Sometimes, randomly generated cookies could cause an infinite number of cookie related vulnerabilities

Implementation details

Read DD_IAST_COOKIE_FILTER_PATTERN env var and apply it to cookie name. If matches, the vulnerability hash will be fixed (vuln:Filtered), effectively grouping all these vulnerabilities.

Test coverage

Added unit tests

Other details

@daniel-romano-DD daniel-romano-DD requested review from a team as code owners August 26, 2024 22:44
@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Aug 26, 2024 •
edited
Loading

Datadog Report

Branch report: dani/iast/cookie_filter
Commit report: 37ec8db
Test service: dd-trace-dotnet

✅ 0 Failed, 300069 Passed, 1581 Skipped, 12h 50m 53.07s Total Time

@andrewlock
Copy link
Member

andrewlock commented Aug 26, 2024 •
edited
Loading

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:

  • Welch test with statistical test for significance of 5%
  • Only results indicating a difference greater than 5% and 5 ms are considered.

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).

@andrewlock
Copy link
Member

andrewlock commented Aug 26, 2024 •
edited
Loading

Benchmarks Report for appsec 🐌

Benchmarks for #5947 compared to master:

  • 2 benchmarks are faster, with geometric mean 1.179
  • 2 benchmarks have fewer 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.Asm.AppSecBodyBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #5947

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody‑net6.0 1.238 184.48 149.02

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master AllCycleSimpleBody net6.0 72.5μs 131ns 509ns 0.0726 0 0 6.01 KB
master AllCycleSimpleBody netcoreapp3.1 62.7μs 106ns 410ns 0.0937 0 0 6.95 KB
master AllCycleSimpleBody net472 48.8μs 21.7ns 81.4ns 1.32 0 0 8.34 KB
master AllCycleMoreComplexBody net6.0 78.1μs 79.7ns 309ns 0.117 0 0 9.51 KB
master AllCycleMoreComplexBody netcoreapp3.1 68.8μs 119ns 463ns 0.137 0 0 10.37 KB
master AllCycleMoreComplexBody net472 56.3μs 32.6ns 126ns 1.87 0.0283 0 11.85 KB
master ObjectExtractorSimpleBody net6.0 184ns 0.103ns 0.399ns 0.00393 0 0 280 B
master ObjectExtractorSimpleBody netcoreapp3.1 204ns 0.906ns 3.51ns 0.00366 0 0 272 B
master ObjectExtractorSimpleBody net472 166ns 0.178ns 0.64ns 0.0446 0 0 281 B
master ObjectExtractorMoreComplexBody net6.0 3.12μs 3.01ns 11.3ns 0.053 0 0 3.78 KB
master ObjectExtractorMoreComplexBody netcoreapp3.1 3.93μs 1.94ns 7.52ns 0.051 0 0 3.69 KB
master ObjectExtractorMoreComplexBody net472 3.82μs 2.45ns 9.47ns 0.603 0.00574 0 3.8 KB
#5947 AllCycleSimpleBody net6.0 73.9μs 85.8ns 332ns 0.0739 0 0 6 KB
#5947 AllCycleSimpleBody netcoreapp3.1 61.8μs 118ns 457ns 0.092 0 0 6.95 KB
#5947 AllCycleSimpleBody net472 49.3μs 45.1ns 163ns 1.31 0 0 8.34 KB
#5947 AllCycleMoreComplexBody net6.0 78.9μs 92ns 356ns 0.117 0 0 9.51 KB
#5947 AllCycleMoreComplexBody netcoreapp3.1 68.3μs 88.2ns 330ns 0.137 0 0 10.37 KB
#5947 AllCycleMoreComplexBody net472 56.1μs 51.1ns 198ns 1.88 0.028 0 11.85 KB
#5947 ObjectExtractorSimpleBody net6.0 150ns 0.643ns 2.49ns 0.00396 0 0 280 B
#5947 ObjectExtractorSimpleBody netcoreapp3.1 200ns 0.199ns 0.769ns 0.00373 0 0 272 B
#5947 ObjectExtractorSimpleBody net472 166ns 0.101ns 0.391ns 0.0446 0 0 281 B
#5947 ObjectExtractorMoreComplexBody net6.0 3.11μs 10.5ns 40.7ns 0.0535 0 0 3.78 KB
#5947 ObjectExtractorMoreComplexBody netcoreapp3.1 3.92μs 2.31ns 8.65ns 0.0492 0 0 3.69 KB
#5947 ObjectExtractorMoreComplexBody net472 3.86μs 2.77ns 10.7ns 0.602 0.00586 0 3.8 KB
Benchmarks.Trace.Asm.AppSecEncoderBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #5947

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.Asm.AppSecEncoderBenchmark.EncodeLegacyArgs‑net6.0 1.123 79,318.73 70,644.42

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EncodeArgs net6.0 37.1μs 18.6ns 69.6ns 0.461 0 0 32.4 KB
master EncodeArgs netcoreapp3.1 54.5μs 24ns 93.1ns 0.436 0 0 32.4 KB
master EncodeArgs net472 66.7μs 25.2ns 97.8ns 5.15 0.0665 0 32.5 KB
master EncodeLegacyArgs net6.0 79.3μs 47.7ns 185ns 0 0 0 2.14 KB
master EncodeLegacyArgs netcoreapp3.1 106μs 267ns 1.03μs 0 0 0 2.14 KB
master EncodeLegacyArgs net472 157μs 138ns 536ns 0.314 0 0 2.15 KB
#5947 EncodeArgs net6.0 37.4μs 26.9ns 101ns 0.449 0 0 32.4 KB
#5947 EncodeArgs netcoreapp3.1 55.8μs 40.7ns 158ns 0.445 0 0 32.4 KB
#5947 EncodeArgs net472 66.1μs 37ns 143ns 5.13 0.0658 0 32.5 KB
#5947 EncodeLegacyArgs net6.0 70.6μs 22.5ns 87.1ns 0 0 0 2.14 KB
#5947 EncodeLegacyArgs netcoreapp3.1 108μs 405ns 1.57μs 0 0 0 2.14 KB
#5947 EncodeLegacyArgs net472 152μs 136ns 527ns 0.306 0 0 2.15 KB
Benchmarks.Trace.Asm.AppSecWafBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunWafRealisticBenchmark net6.0 185μs 88.9ns 333ns 0 0 0 2.42 KB
master RunWafRealisticBenchmark netcoreapp3.1 198μs 103ns 398ns 0 0 0 2.37 KB
master RunWafRealisticBenchmark net472 210μs 33.1ns 124ns 0.315 0 0 2.43 KB
master RunWafRealisticBenchmarkWithAttack net6.0 123μs 62.4ns 242ns 0 0 0 1.46 KB
master RunWafRealisticBenchmarkWithAttack netcoreapp3.1 131μs 130ns 502ns 0 0 0 1.45 KB
master RunWafRealisticBenchmarkWithAttack net472 139μs 27.4ns 103ns 0.209 0 0 1.48 KB
#5947 RunWafRealisticBenchmark net6.0 184μs 164ns 633ns 0 0 0 2.42 KB
#5947 RunWafRealisticBenchmark netcoreapp3.1 199μs 360ns 1.39μs 0 0 0 2.37 KB
#5947 RunWafRealisticBenchmark net472 208μs 139ns 522ns 0.311 0 0 2.43 KB
#5947 RunWafRealisticBenchmarkWithAttack net6.0 123μs 54.8ns 205ns 0 0 0 1.46 KB
#5947 RunWafRealisticBenchmarkWithAttack netcoreapp3.1 130μs 243ns 941ns 0 0 0 1.45 KB
#5947 RunWafRealisticBenchmarkWithAttack net472 139μs 27.5ns 106ns 0.208 0 0 1.48 KB
Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ Fewer allocations 🎉

Fewer allocations 🎉 in #5947

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472 60.58 KB 59.1 KB -1.48 KB -2.44%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1 262.79 KB 253.13 KB -9.66 KB -3.68%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StringConcatBenchmark net6.0 58.7μs 803ns 7.95μs 0 0 0 43.44 KB
master StringConcatBenchmark netcoreapp3.1 62.4μs 823ns 8.02μs 0 0 0 42.64 KB
master StringConcatBenchmark net472 37μs 127ns 522ns 0 0 0 60.58 KB
master StringConcatAspectBenchmark net6.0 312μs 1.78μs 12.7μs 0 0 0 255.56 KB
master StringConcatAspectBenchmark netcoreapp3.1 336μs 2.62μs 25.4μs 0 0 0 262.79 KB
master StringConcatAspectBenchmark net472 276μs 4.96μs 48.4μs 0 0 0 278.53 KB
#5947 StringConcatBenchmark net6.0 61.5μs 611ns 5.95μs 0 0 0 43.44 KB
#5947 StringConcatBenchmark netcoreapp3.1 54.5μs 293ns 1.52μs 0 0 0 42.64 KB
#5947 StringConcatBenchmark net472 36.9μs 76.3ns 275ns 0 0 0 59.1 KB
#5947 StringConcatAspectBenchmark net6.0 315μs 1.81μs 13.7μs 0 0 0 254.84 KB
#5947 StringConcatAspectBenchmark netcoreapp3.1 329μs 1.28μs 5.87μs 0 0 0 253.13 KB
#5947 StringConcatAspectBenchmark net472 308μs 8.5μs 83.7μs 0 0 0 278.53 KB

@andrewlock
Copy link
Member

andrewlock commented Aug 26, 2024 •
edited
Loading

Benchmarks Report for tracer 🐌

Benchmarks for #5947 compared to master:

  • 1 benchmarks are slower, with geometric mean 1.207
  • 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.ActivityBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartStopWithChild net6.0 7.7μs 40.9ns 228ns 0.0151 0.00757 0 5.43 KB
master StartStopWithChild netcoreapp3.1 10μs 53ns 281ns 0.0189 0.00946 0 5.62 KB
master StartStopWithChild net472 16μs 39.2ns 147ns 1.01 0.303 0.0958 6.06 KB
#5947 StartStopWithChild net6.0 8.06μs 46ns 328ns 0.0156 0.00778 0 5.43 KB
#5947 StartStopWithChild netcoreapp3.1 9.9μs 55.9ns 379ns 0.0243 0.0097 0 5.62 KB
#5947 StartStopWithChild net472 16.5μs 74ns 277ns 1.02 0.31 0.0897 6.05 KB
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 460μs 394ns 1.53μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 639μs 469ns 1.82μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 853μs 253ns 877ns 0.425 0 0 3.3 KB
#5947 WriteAndFlushEnrichedTraces net6.0 452μs 165ns 595ns 0 0 0 2.7 KB
#5947 WriteAndFlushEnrichedTraces netcoreapp3.1 640μs 261ns 1.01μs 0 0 0 2.7 KB
#5947 WriteAndFlushEnrichedTraces net472 840μs 420ns 1.63μs 0.417 0 0 3.3 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 net6.0 192μs 1.08μs 7.55μs 0.199 0 0 18.45 KB
master SendRequest netcoreapp3.1 208μs 1.09μs 5.78μs 0.217 0 0 20.61 KB
master SendRequest net472 0.00168ns 0.000753ns 0.00292ns 0 0 0 0 b
#5947 SendRequest net6.0 185μs 503ns 1.81μs 0.186 0 0 18.45 KB
#5947 SendRequest netcoreapp3.1 210μs 1.14μs 7.95μs 0.205 0 0 20.61 KB
#5947 SendRequest net472 1.19E‑06ns 1.19E‑06ns 4.6E‑06ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 558μs 2.04μs 7.89μs 0.285 0 0 41.6 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 671μs 3.23μs 12.5μs 0.324 0 0 41.63 KB
master WriteAndFlushEnrichedTraces net472 874μs 2.61μs 9.75μs 8.08 2.55 0.425 53.31 KB
#5947 WriteAndFlushEnrichedTraces net6.0 553μs 2.49μs 9.63μs 0.558 0 0 41.54 KB
#5947 WriteAndFlushEnrichedTraces netcoreapp3.1 684μs 3.41μs 15.2μs 0.327 0 0 41.83 KB
#5947 WriteAndFlushEnrichedTraces net472 891μs 3.75μs 14.5μs 8.3 2.62 0.437 53.39 KB
Benchmarks.Trace.DbCommandBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #5947

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.DbCommandBenchmark.ExecuteNonQuery‑net6.0 1.207 1,132.18 1,366.39

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteNonQuery net6.0 1.13μs 1.21ns 4.7ns 0.0142 0 0 1.02 KB
master ExecuteNonQuery netcoreapp3.1 1.76μs 1.22ns 4.71ns 0.0133 0 0 1.02 KB
master ExecuteNonQuery net472 1.96μs 2.46ns 9.54ns 0.156 0 0 987 B
#5947 ExecuteNonQuery net6.0 1.37μs 0.81ns 3.03ns 0.0144 0 0 1.02 KB
#5947 ExecuteNonQuery netcoreapp3.1 1.72μs 1.01ns 3.77ns 0.0132 0 0 1.02 KB
#5947 ExecuteNonQuery net472 2.03μs 1.74ns 6.52ns 0.156 0 0 987 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 net6.0 1.24μs 0.58ns 2.17ns 0.0136 0 0 976 B
master CallElasticsearch netcoreapp3.1 1.49μs 0.794ns 2.97ns 0.0134 0 0 976 B
master CallElasticsearch net472 2.54μs 1.93ns 7.47ns 0.157 0.00128 0 995 B
master CallElasticsearchAsync net6.0 1.22μs 0.439ns 1.64ns 0.0129 0 0 952 B
master CallElasticsearchAsync netcoreapp3.1 1.67μs 0.398ns 1.43ns 0.0134 0 0 1.02 KB
master CallElasticsearchAsync net472 2.62μs 2.87ns 11.1ns 0.166 0.00131 0 1.05 KB
#5947 CallElasticsearch net6.0 1.2μs 1.58ns 6.11ns 0.0133 0 0 976 B
#5947 CallElasticsearch netcoreapp3.1 1.56μs 0.394ns 1.53ns 0.0133 0 0 976 B
#5947 CallElasticsearch net472 2.38μs 4.06ns 15.7ns 0.158 0.00119 0 995 B
#5947 CallElasticsearchAsync net6.0 1.36μs 0.758ns 2.94ns 0.013 0 0 952 B
#5947 CallElasticsearchAsync netcoreapp3.1 1.6μs 1.18ns 4.56ns 0.0138 0 0 1.02 KB
#5947 CallElasticsearchAsync net472 2.61μs 1.4ns 5.41ns 0.167 0.00131 0 1.05 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 net6.0 1.25μs 0.521ns 1.95ns 0.0132 0 0 952 B
master ExecuteAsync netcoreapp3.1 1.68μs 0.847ns 3.28ns 0.0125 0 0 952 B
master ExecuteAsync net472 1.78μs 1.12ns 4.21ns 0.145 0 0 915 B
#5947 ExecuteAsync net6.0 1.26μs 1.51ns 5.85ns 0.0136 0 0 952 B
#5947 ExecuteAsync netcoreapp3.1 1.68μs 0.664ns 2.3ns 0.0127 0 0 952 B
#5947 ExecuteAsync net472 1.72μs 0.448ns 1.68ns 0.145 0 0 915 B
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendAsync net6.0 4.17μs 2.57ns 9.61ns 0.0314 0 0 2.22 KB
master SendAsync netcoreapp3.1 5.03μs 1.61ns 6.02ns 0.0377 0 0 2.76 KB
master SendAsync net472 7.73μs 1.53ns 5.73ns 0.499 0 0 3.15 KB
#5947 SendAsync net6.0 4.15μs 1.22ns 4.71ns 0.0312 0 0 2.22 KB
#5947 SendAsync netcoreapp3.1 5.05μs 1.59ns 6.18ns 0.0378 0 0 2.76 KB
#5947 SendAsync net472 7.79μs 1.87ns 7.26ns 0.498 0 0 3.15 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 net6.0 1.51μs 0.708ns 2.65ns 0.0228 0 0 1.64 KB
master EnrichedLog netcoreapp3.1 2.28μs 1.05ns 3.92ns 0.0217 0 0 1.64 KB
master EnrichedLog net472 2.68μs 2.01ns 7.79ns 0.249 0 0 1.57 KB
#5947 EnrichedLog net6.0 1.51μs 0.655ns 2.45ns 0.0226 0 0 1.64 KB
#5947 EnrichedLog netcoreapp3.1 2.32μs 1ns 3.75ns 0.0215 0 0 1.64 KB
#5947 EnrichedLog net472 2.68μs 0.946ns 3.54ns 0.249 0 0 1.57 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 net6.0 115μs 172ns 667ns 0.0581 0 0 4.28 KB
master EnrichedLog netcoreapp3.1 119μs 210ns 813ns 0.0589 0 0 4.28 KB
master EnrichedLog net472 150μs 199ns 770ns 0.669 0.223 0 4.46 KB
#5947 EnrichedLog net6.0 117μs 183ns 709ns 0.0588 0 0 4.28 KB
#5947 EnrichedLog netcoreapp3.1 119μs 197ns 763ns 0 0 0 4.28 KB
#5947 EnrichedLog net472 148μs 215ns 833ns 0.665 0.222 0 4.46 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 net6.0 3.03μs 8.36ns 32.4ns 0.03 0 0 2.2 KB
master EnrichedLog netcoreapp3.1 4.09μs 1.05ns 3.93ns 0.0286 0 0 2.2 KB
master EnrichedLog net472 4.8μs 0.966ns 3.62ns 0.321 0 0 2.02 KB
#5947 EnrichedLog net6.0 2.89μs 0.839ns 3.25ns 0.0303 0 0 2.2 KB
#5947 EnrichedLog netcoreapp3.1 4.26μs 1.95ns 7.3ns 0.0298 0 0 2.2 KB
#5947 EnrichedLog net472 4.8μs 1.22ns 4.74ns 0.319 0 0 2.02 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 net6.0 1.32μs 2.86ns 11.1ns 0.0158 0 0 1.14 KB
master SendReceive netcoreapp3.1 1.71μs 0.812ns 3.15ns 0.0155 0 0 1.14 KB
master SendReceive net472 2.12μs 1.44ns 5.4ns 0.183 0 0 1.16 KB
#5947 SendReceive net6.0 1.34μs 0.634ns 2.37ns 0.0162 0 0 1.14 KB
#5947 SendReceive netcoreapp3.1 1.85μs 5.12ns 19.8ns 0.0155 0 0 1.14 KB
#5947 SendReceive net472 2.23μs 1.52ns 5.9ns 0.183 0.00112 0 1.16 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 net6.0 2.91μs 1.21ns 4.69ns 0.0218 0 0 1.6 KB
master EnrichedLog netcoreapp3.1 3.97μs 1.53ns 5.94ns 0.0216 0 0 1.65 KB
master EnrichedLog net472 4.41μs 2.31ns 8.95ns 0.324 0 0 2.04 KB
#5947 EnrichedLog net6.0 2.87μs 1.04ns 4.03ns 0.0216 0 0 1.6 KB
#5947 EnrichedLog netcoreapp3.1 3.91μs 0.783ns 2.93ns 0.0211 0 0 1.65 KB
#5947 EnrichedLog net472 4.42μs 2.38ns 8.91ns 0.322 0 0 2.04 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 net6.0 411ns 0.149ns 0.577ns 0.00797 0 0 576 B
master StartFinishSpan netcoreapp3.1 576ns 0.217ns 0.813ns 0.00782 0 0 576 B
master StartFinishSpan net472 623ns 0.536ns 2.08ns 0.0917 0 0 578 B
master StartFinishScope net6.0 529ns 0.203ns 0.786ns 0.00985 0 0 696 B
master StartFinishScope netcoreapp3.1 713ns 0.309ns 1.16ns 0.00911 0 0 696 B
master StartFinishScope net472 848ns 0.346ns 1.34ns 0.104 0 0 658 B
#5947 StartFinishSpan net6.0 397ns 0.463ns 1.8ns 0.00806 0 0 576 B
#5947 StartFinishSpan netcoreapp3.1 579ns 0.359ns 1.39ns 0.00785 0 0 576 B
#5947 StartFinishSpan net472 571ns 0.362ns 1.4ns 0.0916 0 0 578 B
#5947 StartFinishScope net6.0 497ns 0.302ns 1.17ns 0.00988 0 0 696 B
#5947 StartFinishScope netcoreapp3.1 780ns 0.414ns 1.61ns 0.00975 0 0 696 B
#5947 StartFinishScope net472 906ns 2.2ns 8.53ns 0.104 0 0 658 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 net6.0 660ns 0.332ns 1.29ns 0.00977 0 0 696 B
master RunOnMethodBegin netcoreapp3.1 961ns 1.22ns 4.74ns 0.00937 0 0 696 B
master RunOnMethodBegin net472 1.07μs 0.354ns 1.37ns 0.104 0 0 658 B
#5947 RunOnMethodBegin net6.0 653ns 0.487ns 1.89ns 0.00957 0 0 696 B
#5947 RunOnMethodBegin netcoreapp3.1 968ns 0.334ns 1.2ns 0.00966 0 0 696 B
#5947 RunOnMethodBegin net472 1.07μs 0.231ns 0.893ns 0.104 0 0 658 B

@github-actions GitHub Actions
Copy link
Contributor

Snapshots difference summary

The following differences have been observed in committed snapshots. It is meant to help the reviewer.
The diff is simplistic, so please check some files anyway while we improve it.

1 occurrences of :

+    },
+    {
+      "type": "NO_SAMESITE_COOKIE",
+      "hash": -1837181716,
+      "evidence": {
+        "value": "LongCookie.abcdefghijklmnopqrstuvwxyz0123456789.0"
+      }
+    },
+    {
+      "type": "NO_HTTPONLY_COOKIE",
+      "hash": 1990393425,
+      "evidence": {
+        "value": "LongCookie.abcdefghijklmnopqrstuvwxyz0123456789.0"
+      }
+    },
+    {
+      "type": "INSECURE_COOKIE",
+      "hash": 1170867602,
+      "evidence": {
+        "value": "LongCookie.abcdefghijklmnopqrstuvwxyz0123456789.0"
+      }
+    },
+    {
+      "type": "NO_SAMESITE_COOKIE",
+      "hash": -1837181716,
+      "evidence": {
+        "value": "LongCookie.abcdefghijklmnopqrstuvwxyz0123456789.1"
+      }
+    },
+    {
+      "type": "NO_HTTPONLY_COOKIE",
+      "hash": 1990393425,
+      "evidence": {
+        "value": "LongCookie.abcdefghijklmnopqrstuvwxyz0123456789.1"
+      }
+    },
+    {
+      "type": "INSECURE_COOKIE",
+      "hash": 1170867602,
+      "evidence": {
+        "value": "LongCookie.abcdefghijklmnopqrstuvwxyz0123456789.1"
+      }
+    },
+    {
+      "type": "NO_SAMESITE_COOKIE",
+      "hash": -1837181716,
+      "evidence": {
+        "value": "LongCookie.abcdefghijklmnopqrstuvwxyz0123456789.2"
+      }
+    },
+    {
+      "type": "NO_HTTPONLY_COOKIE",
+      "hash": 1990393425,
+      "evidence": {
+        "value": "LongCookie.abcdefghijklmnopqrstuvwxyz0123456789.2"
+      }
+    },
+    {
+      "type": "INSECURE_COOKIE",
+      "hash": 1170867602,
+      "evidence": {
+        "value": "LongCookie.abcdefghijklmnopqrstuvwxyz0123456789.2"
+      }

andrewlock
andrewlock approved these changes Aug 27, 2024
View reviewed changes
Copy link
Member

@andrewlock andrewlock left a comment

Choose a reason for hiding this comment

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

LGTM, just a few minor comments

tracer/src/Datadog.Trace/Iast/Settings/IastSettings.cs Show resolved Hide resolved
tracer/src/Datadog.Trace/Iast/CookieAnalyzer.cs Outdated Show resolved Hide resolved
tracer/test/Datadog.Trace.Tests/Configuration/Telemetry/ConfigurationBuilderTests.cs
@@ -176,7 +176,7 @@ public void RecordsTelemetryCorrectly(object value)
},
{ } i => new List<Entry>
{
Entry.String(Key, i.ToString(), ConfigurationOrigins.Code, error: null),
Entry.String(Key, Convert.ToString(i, CultureInfo.InvariantCulture), ConfigurationOrigins.Code, error: null),
Copy link
Member

Choose a reason for hiding this comment

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

nit: I don't think you need to use Convert do you? 🤔

Suggested change
Entry.String(Key, Convert.ToString(i, CultureInfo.InvariantCulture), ConfigurationOrigins.Code, error: null),
Entry.String(Key, i.ToString(CultureInfo.InvariantCulture), ConfigurationOrigins.Code, error: null),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Your option was my first choice, but that overload is missing in some frameworks.

Copy link
Member

Choose a reason for hiding this comment

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

It's there since .NET Framework 1.0 AFAICT 😕

tracer/test/Datadog.Trace.Tests/Telemetry/config_norm_rules.json
@@ -516,6 +516,7 @@
"DD_IAST_TELEMETRY_VERBOSITY": "iast_telemetry_verbosity",
"DD_IAST_TRUNCATION_MAX_VALUE_LENGTH": "iast_truncation_max_value_length",
"DD_IAST_DB_ROWS_TO_TAINT": "iast_db_rows_to_taint",
"DD_IAST_COOKIE_FILTER_PATTERN": "iast_cookie_filter_pattern",
Copy link
Member

Choose a reason for hiding this comment

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

Don't forget to update dd-go too 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm waiting for the RFC approval, so I don't have to change the literal. But you are right

...shots/Security.AspNetCore5.enableIast=True.path =_Iast_AllVulnerabilitiesCookie.verified.txt Show resolved Hide resolved
@andrewlock
Copy link
Member

Throughput/Crank Report ⚡

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 (5947) (11.625M)   : 0, 11625170
    master (11.555M)   : 0, 11555237
    benchmarks/2.9.0 (11.711M)   : 0, 11710619

    section Automatic
    This PR (5947) (7.711M)   : 0, 7711225
    master (7.792M)   : 0, 7792363
    benchmarks/2.9.0 (8.265M)   : 0, 8264630

    section Trace stats
    master (8.104M)   : 0, 8103920

    section Manual
    master (11.572M)   : 0, 11572105

    section Manual + Automatic
    This PR (5947) (7.127M)   : 0, 7127442
    master (7.316M)   : 0, 7315600

    section DD_TRACE_ENABLED=0
    master (10.685M)   : 0, 10684738
Loading 
gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5947) (9.776M)   : 0, 9775588
    master (9.687M)   : 0, 9687318
    benchmarks/2.9.0 (9.717M)   : 0, 9716586

    section Automatic
    This PR (5947) (6.594M)   : 0, 6593615
    master (6.541M)   : 0, 6541465

    section Trace stats
    master (6.901M)   : 0, 6900663

    section Manual
    master (9.548M)   : 0, 9548348

    section Manual + Automatic
    This PR (5947) (6.101M)   : 0, 6101405
    master (6.135M)   : 0, 6134999

    section DD_TRACE_ENABLED=0
    master (8.963M)   : 0, 8963423
Loading 
gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5947) (10.279M)   : 0, 10278916
    master (10.302M)   : 0, 10301875

    section Automatic
    This PR (5947) (6.731M)   : 0, 6730592
    master (6.752M)   : 0, 6752127

    section Trace stats
    master (7.494M)   : 0, 7493970

    section Manual
    master (10.315M)   : 0, 10315116

    section Manual + Automatic
    This PR (5947) (6.250M)   : 0, 6250257
    master (6.251M)   : 0, 6251394

    section DD_TRACE_ENABLED=0
    master (9.586M)   : 0, 9585809
Loading

@daniel-romano-DD daniel-romano-DD merged commit 63d3f07 into master Aug 29, 2024
70 of 71 checks passed
@daniel-romano-DD daniel-romano-DD deleted the dani/iast/cookie_filter branch August 29, 2024 09:16
@github-actions github-actions bot added this to the vNext-v3 milestone Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewlock andrewlock andrewlock approved these changes

@NachoEchevarria NachoEchevarria NachoEchevarria approved these changes

Assignees
No one assigned
Labels
area:asm-iast
Projects
None yet
Milestone
3.2.0
Development

Successfully merging this pull request may close these issues.
3 participants
@daniel-romano-DD @andrewlock @NachoEchevarria