Skip to content

[Appsec] Scanning request body by the waf#2495

Merged
anna-git merged 24 commits into
masterfrom
appsec/parsed-body-common
Mar 31, 2022
Merged

[Appsec] Scanning request body by the waf#2495
anna-git merged 24 commits into
masterfrom
appsec/parsed-body-common

Conversation

@robertpi

@robertpi robertpi commented Feb 23, 2022 •
edited by anna-git
Loading

Copy link
Copy Markdown
Member

Summary of changes

Monitoring incoming body's request
https://datadoghq.atlassian.net/wiki/spaces/APS/pages/2356905034/server.request.body

Reason for change

Getting an http request's body scanned by the waf, in both core and framework

Implementation details

Adding instrumentation points for Core and Framework (mvc, web api, webforms, razor pages)
Adding benchmarks on the body extractor part

Test coverage

Adding unit tests
Adding integration tests

Other details

@andrewlock

This comment has been minimized.

Sign in to view
@robertpi
robertpi force-pushed the appsec/parsed-body-common branch from f0e5fcb to 4919b76 Compare February 23, 2022 17:00
@andrewlock

This comment has been minimized.

Sign in to view
@anna-git
anna-git force-pushed the appsec/parsed-body-common branch 2 times, most recently from 60fc69f to f61ac89 Compare February 24, 2022 16:57
@andrewlock

This comment has been minimized.

Sign in to view
@anna-git
anna-git force-pushed the appsec/parsed-body-common branch 2 times, most recently from 8b00c37 to bc0ea81 Compare February 28, 2022 18:55
@andrewlock

This comment has been minimized.

Sign in to view
@anna-git
anna-git force-pushed the appsec/parsed-body-common branch from bc0ea81 to dce5b97 Compare March 1, 2022 09:19
@andrewlock

This comment has been minimized.

Sign in to view
@andrewlock

This comment has been minimized.

Sign in to view
@anna-git
anna-git force-pushed the appsec/parsed-body-common branch from 3173a4d to 0c676f6 Compare March 2, 2022 14:05
@anna-git anna-git changed the title Appsec/parsed body common [Appsec] Scanning request body by the waf Mar 2, 2022
@anna-git
anna-git force-pushed the appsec/parsed-body-common branch from 0c676f6 to 9ebebfe Compare March 2, 2022 14:36
@andrewlock

This comment has been minimized.

Sign in to view
@anna-git
anna-git force-pushed the appsec/parsed-body-common branch 2 times, most recently from f984bab to 7f9f485 Compare March 3, 2022 13:10
@andrewlock

This comment has been minimized.

Sign in to view
@andrewlock

This comment has been minimized.

Sign in to view
@anna-git
anna-git force-pushed the appsec/parsed-body-common branch from 1c97e04 to cc28fef Compare March 4, 2022 15:01
@andrewlock

This comment has been minimized.

Sign in to view
@anna-git
anna-git force-pushed the appsec/parsed-body-common branch from 8eb45a5 to ee291cc Compare March 7, 2022 19:36
@andrewlock

This comment has been minimized.

Sign in to view
@anna-git
anna-git force-pushed the appsec/parsed-body-common branch from ee291cc to 15b96e2 Compare March 8, 2022 10:36
@robertpi
robertpi marked this pull request as ready for review March 8, 2022 13:30
@robertpi
robertpi requested a review from a team as a code owner March 8, 2022 13:30
@andrewlock

This comment has been minimized.

Sign in to view
@anna-git
anna-git force-pushed the appsec/parsed-body-common branch from 3425158 to 68036f9 Compare March 9, 2022 10:48
robertpi and others added 19 commits March 31, 2022 09:52
@robertpi @anna-git
Read the parsed body in .NET Framework
fb9280e 
Includes:
* Win Forms
* WebApi
* MVC
@anna-git
Read the parsed body in Net Core (#2501)
d69c5c0 
Adding a more complex model

Removing tests with block enabled

Removing tests with block enabled Renaming samples wth security prefix to avoid nuke confusion Changing sln accordingly
@anna-git
Adapt build steps for linux and rename integration right
1e1699b
@anna-git
Changing snapshot files to match new ressponse content length
ac1e51a
@anna-git
Publish views dll for linux cause we use the publish folder for inte …
cc7c854 
…test

fix build
@robertpi @anna-git
Start of unit tests for parsed body
fc88693
@robertpi @anna-git
Unit tests and handle some serialization corner cases
bcc6729
@robertpi @anna-git
Move code from IG into BodyExtractor.cs
b1f9d75
@robertpi @anna-git
Better detection of primatives
e6f9f36 
Detect object graphs with cycles
Only look at instance properties
@anna-git
Use fields instead of property to avoid side effects
54647cb 
Use fields instead of property to avoid side effects

Fix possible null ref
@anna-git @robertpi
[Appsec]Benchmarks (#2537)
72aaa6a 
* first appsec benchmark

* Remove required filter and add more benchmarks

* Fix benchmarks for .NET framework

* add a more complex object

* fix build steps

* Env variables

* fix benchmarks

* Init httpcontext outside of benchmark

* remove global setup

Co-authored-by: Robert Pickering <[email protected]>
@anna-git
Adding enum to body extractor
305ceac
@robertpi @anna-git
Remove unneeded code containing a TODO
b961f15 
Adding verified files
@anna-git
First response to comments
7a42897
@anna-git
Fixing duck typing
8f949c8 
Factorize peek and pop with Func

remove duck type on real type
@anna-git
Repsonse to comments
6d951fd 
Response to comments
@robertpi @anna-git
Fix integration tests
f8e8bee
@anna-git
Fix called several times if 2 binding sources with different id , fix…
217541b 
… derived attribute
@anna-git
Adding instrumentation scenario filter to instrumentations
36fe7b9
@anna-git
anna-git force-pushed the appsec/parsed-body-common branch from 718e5ca to 36fe7b9 Compare March 31, 2022 08:16
@andrewlock

andrewlock commented Mar 31, 2022

Copy link
Copy Markdown
Member

Code Coverage Report 📊

✔️ Merging #2495 into master will not change line coverage
✔️ Merging #2495 into master will not change branch coverage
⛔ Merging #2495 into master will will increase complexity by 121

master #2495 Change
Lines 12899 / 17777 13008 / 17924
Lines % 73% 73% 0% ✔️
Branches 7293 / 10411 7358 / 10519
Branches % 70% 70% 0% ✔️
Complexity 11666 11787 121 ⛔

View the full report for further details:

Datadog.Trace Breakdown ✔️

master #2495 Change
Lines % 73% 73% 0% ✔️
Branches % 70% 70% 0% ✔️
Complexity 11666 11787 121 ⛔

The following classes have significant coverage changes.

File Line coverage change Branch coverage change Complexity change
Datadog.Trace.AppSec.Waf.Initialization.WafConfigurator -4% ⚠️ -11% ⛔ 0 ✔️
Datadog.Trace.ClrProfiler.NativeMethods -3% ⚠️ -8% ⛔ 0 ✔️
Datadog.Trace.AspNet.TracingHttpModule -3% ⚠️ -6% ⛔ 6 ⛔
Datadog.Trace.Telemetry.JsonWebRequestTelemetryTransport 27% ✔️ 57% ✔️ 0 ✔️

The following classes were added in #2495:

File Line coverage Branch coverage Complexity
Datadog.Trace.AppSec.BodyExtractor 57% 40% 54
Datadog.Trace.ClrProfiler.AspNetCore.DefaultModelBindingContext_SetResult_Integration 100% 85% 20
Datadog.Trace.ClrProfiler.AutoInstrumentation.AspNet.ControllerActionInvoker_InvokeAction_Integration 0% 0% 5
Datadog.Trace.ClrProfiler.AutoInstrumentation.AspNet.ReflectedHttpActionDescriptor_ExecuteAsync_Integration 0% 0% 5

View the full reports for further details:

@andrewlock

andrewlock commented Mar 31, 2022

Copy link
Copy Markdown
Member

Benchmarks Report 🐌

Benchmarks for #2495 compared to master:

  • 2 benchmarks are faster, with geometric mean 1.150
  • 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 990μs 5.74μs 49.7μs 0 0 0 3.16 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 742μs 1.82μs 7.05μs 0 0 0 2.57 KB
#2495 WriteAndFlushEnrichedTraces net472 960μs 4.43μs 17.2μs 0 0 0 3.16 KB
#2495 WriteAndFlushEnrichedTraces netcoreapp3.1 765μs 3.62μs 14.5μs 0 0 0 2.57 KB
Benchmarks.Trace.AppSecBodyBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
#2495 AllCycleSimpleBody net472 309ns 1.48ns 5.92ns 0.0519 0 0 329 B
#2495 AllCycleSimpleBody netcoreapp3.1 384ns 1.8ns 6.99ns 0.00459 0 0 328 B
#2495 AllCycleMoreComplexBody net472 318ns 1.86ns 17.7ns 0.0482 0 0 305 B
#2495 AllCycleMoreComplexBody netcoreapp3.1 378ns 0.642ns 2.4ns 0.00415 0 0 304 B
#2495 BodyExtractorSimpleBody net472 479ns 2.58ns 13.7ns 0.0569 0 0 361 B
#2495 BodyExtractorSimpleBody netcoreapp3.1 480ns 2.21ns 7.98ns 0.0038 0 0 272 B
#2495 BodyExtractorMoreComplexBody net472 25.1μs 109ns 406ns 1.19 0.0128 0 7.62 KB
#2495 BodyExtractorMoreComplexBody netcoreapp3.1 21.3μs 61.5ns 230ns 0.0856 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 334μs 795ns 2.75μs 0.167 0 0 19.74 KB
#2495 SendRequest net472 0ns 0ns 0ns 0 0 0 0 b
#2495 SendRequest netcoreapp3.1 343μs 1.43μs 5.54μs 0.17 0 0 19.74 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.89μs 7.67ns 27.6ns 0.0944 0.000925 0 594 B
master ExecuteNonQuery netcoreapp3.1 1.56μs 6.76ns 25.3ns 0.00859 0 0 632 B
#2495 ExecuteNonQuery net472 1.88μs 8.58ns 32.1ns 0.0941 0 0 594 B
#2495 ExecuteNonQuery netcoreapp3.1 1.59μs 9.3ns 90.6ns 0.00839 0 0 632 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.84μs 16.1ns 119ns 0.125 0 0 802 B
master CallElasticsearch netcoreapp3.1 1.81μs 6.2ns 22.4ns 0.011 0 0 792 B
master CallElasticsearchAsync net472 3.2μs 15.8ns 72.2ns 0.148 0 0 939 B
master CallElasticsearchAsync netcoreapp3.1 1.91μs 6.36ns 23.8ns 0.0126 0 0 912 B
#2495 CallElasticsearch net472 3.01μs 17.2ns 121ns 0.125 0 0 802 B
#2495 CallElasticsearch netcoreapp3.1 1.71μs 7.66ns 29.7ns 0.0105 0 0 792 B
#2495 CallElasticsearchAsync net472 2.97μs 13.6ns 52.7ns 0.149 0 0 939 B
#2495 CallElasticsearchAsync netcoreapp3.1 1.95μs 7.81ns 29.2ns 0.0127 0 0 912 B
Benchmarks.Trace.GraphQLBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #2495

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync‑net472 1.136 3,793.60 3,338.83

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteAsync net472 3.81μs 20.4ns 126ns 0.165 0 0 1.06 KB
master ExecuteAsync netcoreapp3.1 2.05μs 9.96ns 41.1ns 0.0145 0 0 1.03 KB
#2495 ExecuteAsync net472 3.37μs 16.3ns 71ns 0.168 0.00166 0 1.06 KB
#2495 ExecuteAsync netcoreapp3.1 2.05μs 8.28ns 32.1ns 0.0141 0 0 1.03 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 7.38μs 30.2ns 109ns 0.36 0 0 2.28 KB
master SendAsync netcoreapp3.1 4.8μs 19.4ns 72.7ns 0.0312 0 0 2.21 KB
#2495 SendAsync net472 7.3μs 38.4ns 277ns 0.354 0 0 2.28 KB
#2495 SendAsync netcoreapp3.1 5.26μs 27ns 124ns 0.0311 0 0 2.21 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.54μs 14.2ns 58.7ns 0.227 0 0 1.45 KB
master EnrichedLog netcoreapp3.1 3.28μs 15.5ns 61.9ns 0.0216 0 0 1.53 KB
#2495 EnrichedLog net472 3.55μs 13.2ns 50.9ns 0.228 0 0 1.45 KB
#2495 EnrichedLog netcoreapp3.1 3.34μs 16.2ns 85.5ns 0.0218 0 0 1.53 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 304μs 1.22μs 4.73μs 0.446 0.149 0 4.33 KB
master EnrichedLog netcoreapp3.1 238μs 669ns 2.5μs 0 0 0 4.21 KB
#2495 EnrichedLog net472 318μs 1.79μs 12.4μs 0.316 0.158 0 4.33 KB
#2495 EnrichedLog netcoreapp3.1 244μs 908ns 3.52μs 0 0 0 4.21 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 8.49μs 46.2ns 333ns 0.503 0 0 3.23 KB
master EnrichedLog netcoreapp3.1 6.62μs 23.9ns 92.6ns 0.0483 0 0 3.6 KB
#2495 EnrichedLog net472 9.1μs 27.6ns 103ns 0.503 0 0 3.23 KB
#2495 EnrichedLog netcoreapp3.1 6.81μs 25.3ns 98.2ns 0.0508 0 0 3.6 KB
Benchmarks.Trace.RedisBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #2495

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.RedisBenchmark.SendReceive‑netcoreapp3.1 1.164 2,398.91 2,060.16

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendReceive net472 2.57μs 14.3ns 93.7ns 0.158 0 0 1.01 KB
master SendReceive netcoreapp3.1 2.4μs 13.2ns 79.1ns 0.0144 0 0 1.01 KB
#2495 SendReceive net472 2.54μs 12.7ns 53.9ns 0.161 0.00124 0 1.01 KB
#2495 SendReceive netcoreapp3.1 2.06μs 9.18ns 35.6ns 0.0136 0 0 1.01 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 6.36μs 29ns 112ns 0.292 0 0 1.87 KB
master EnrichedLog netcoreapp3.1 5.51μs 25.4ns 101ns 0.0187 0 0 1.49 KB
#2495 EnrichedLog net472 6.22μs 20.5ns 79.3ns 0.29 0 0 1.87 KB
#2495 EnrichedLog netcoreapp3.1 5.53μs 27.8ns 130ns 0.0188 0 0 1.49 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 928ns 4.45ns 18.4ns 0.0716 0 0 457 B
master StartFinishSpan netcoreapp3.1 909ns 4.53ns 18.7ns 0.00634 0 0 456 B
master StartFinishScope net472 1.15μs 5.29ns 20.5ns 0.0843 0 0 538 B
master StartFinishScope netcoreapp3.1 1.3μs 3.43ns 12.8ns 0.00821 0 0 576 B
#2495 StartFinishSpan net472 954ns 4.28ns 16.6ns 0.0717 0 0 457 B
#2495 StartFinishSpan netcoreapp3.1 915ns 3.73ns 14.4ns 0.00634 0 0 456 B
#2495 StartFinishScope net472 1.14μs 3.89ns 14.5ns 0.0842 0 0 538 B
#2495 StartFinishScope netcoreapp3.1 1.28μs 7.31ns 51.7ns 0.00802 0 0 576 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.42μs 4.14ns 16ns 0.0842 0 0 538 B
master RunOnMethodBegin netcoreapp3.1 1.25μs 7.05ns 49.3ns 0.00779 0 0 576 B
#2495 RunOnMethodBegin net472 1.4μs 8ns 62ns 0.0844 0 0 538 B
#2495 RunOnMethodBegin netcoreapp3.1 1.2μs 4.08ns 16.3ns 0.00766 0 0 576 B

@anna-git
anna-git merged commit 9df9066 into master Mar 31, 2022
@anna-git
anna-git deleted the appsec/parsed-body-common branch March 31, 2022 14:43
@github-actions github-actions Bot added this to the vNext milestone Mar 31, 2022
This was referenced Apr 1, 2022
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tonyredondo tonyredondo tonyredondo left review comments
@anna-git anna-git anna-git left review comments
@andrewlock andrewlock andrewlock left review comments
@pierotibou pierotibou pierotibou approved these changes
@lucaspimentel lucaspimentel lucaspimentel approved these changes

Assignees

No one assigned

Labels

area:asm

Projects

None yet

Milestone

2.6.0

Development

Successfully merging this pull request may close these issues.

6 participants

@robertpi @andrewlock @anna-git @tonyredondo @lucaspimentel @pierotibou