[AppSec] Update waf and ruleset 1.3.0 by anna-git · Pull Request #2638 · DataDog/dd-trace-dotnet

anna-git · 2022-04-04T17:58:16Z

Summary of changes

Update of the security component libddwaf to 1.3.0
Update of ruleset for waf to 1.3.0
Initialization code refactor to collect results from the waf new metrics
Added metrics and tags to appsec spans

Reason for change

Updating waf with new rules and new metrics
https://docs.google.com/document/d/1qBDsS_ZKeov226CPx2DneolxaARd66hUJJ5Lh9wjhlE/edit#heading=h.o5gstqo08gu5
New version also provides an embedded obfuscator

Implementation details

Cookies waf unit tests are skipped because they have been removed from the new rule sets
As we are mostly interested in testing integration with waf and rules and the whole cycle, we keep rule set 3.0 for integration tests as an injection rule has completely changed and is breaking the tests.

Test coverage

Added integration tests for waf initialization

Other details

robertpi · 2022-04-08T06:51:26Z

-        }
+        internal void ObjectFreePtr(IntPtr input) => _freeObjectield(input);
+
+        internal void ResultFree(ref DdwafResultStruct output) => _freeResultField(ref output);


Looks like this isn't being used, I think it should be.

It is used here
and disposed in security with
**using** var wafResult = additiveContext.Run(args, _settings.WafTimeoutMicroSeconds);

robertpi

I think you need to double check the WAF memory structures are being freed correctly. The throughput tests, will probably show increased memory usage if they are not, so this could be a good way to check.

Also, it looks like you've not implemented the env var DD_APPSEC_OBFUSCATION_PARAMETER_KEY_REGEXP and DD_APPSEC_OBFUSCATION_PARAMETER_VALUE_REGEXP. I'm okay with that going in a separate PR if you want to get this one merged as is.

Fixing tests to match 1.3.0 fix snapshots

Add default rule file

…hould be a map coming from the waf

andrewlock · 2022-04-11T09:31:33Z

Code Coverage Report 📊

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

	master	#2638	Change
Lines	`13449` / `18473`	`13505` / `18607`
Lines %	`73%`	`73%`	`0%` ✔️
Branches	`7764` / `11017`	`7808` / `11101`
Branches %	`70%`	`70%`	`0%` ✔️
Complexity	`12313`	`12407`	`94` ⛔

View the full report for further details:

Datadog.Trace Breakdown ✔️

	master	#2638	Change
Lines %	`73%`	`73%`	`0%` ✔️
Branches %	`70%`	`70%`	`0%` ✔️
Complexity	`12313`	`12407`	`94` ⛔

The following classes have significant coverage changes.

File	Line coverage change	Branch coverage change	Complexity change
Datadog.Trace.Telemetry.JsonHttpClientTelemetryTransport	`-26%` ⛔	`-38%` ⛔	`0` ✔️
Datadog.Trace.AppSec.Waf.Initialization.WafConfigurator	`-3%` ⚠️	`-9%` ⛔	`16` ⛔
Datadog.Trace.AppSec.Waf.Waf	`3%` ✔️	`-12%` ⛔	`2` ⛔
Datadog.Trace.AppSec.Waf.Context	`7%` ✔️	`0%` ✔️	`0` ✔️
Datadog.Trace.Telemetry.JsonWebRequestTelemetryTransport	`27%` ✔️	`57%` ✔️	`0` ✔️

The following classes were added in #2638:

File	Line coverage	Branch coverage	Complexity
Datadog.Trace.AppSec.Waf.ReturnTypesManaged.InitializationResult	`39%`	`30%`	`13`
Datadog.Trace.TraceProcessors.ObfuscatorTraceProcessor	`81%`	`84%`	`59`

View the full reports for further details:

andrewlock · 2022-04-11T12:40:23Z

Benchmarks Report 🐌

Benchmarks for #2638 compared to master:

2 benchmarks are slower, with geometric mean 1.172
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	Allocated
master	`WriteAndFlushEnrichedTraces`	net472	925μs	4.92μs	26.5μs	3.17 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	700μs	3.35μs	13μs	2.57 KB
#2638	`WriteAndFlushEnrichedTraces`	net472	931μs	4.87μs	27.5μs	3.16 KB
#2638	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	703μs	3.51μs	14.9μs	2.57 KB

Benchmarks.Trace.AppSecBodyBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #2638

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.AppSecBodyBenchmark.BodyExtractorMoreComplexBody‑netcoreapp3.1	1.172	20,126.09	23,591.39

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net472	294ns	1.38ns	5.36ns	0.0519	0	329 B
master	`AllCycleSimpleBody`	netcoreapp3.1	367ns	2ns	11.1ns	0.00449	0	328 B
master	`AllCycleMoreComplexBody`	net472	286ns	0.823ns	2.97ns	0.0482	0	305 B
master	`AllCycleMoreComplexBody`	netcoreapp3.1	365ns	1.43ns	5.56ns	0.00411	0	304 B
master	`BodyExtractorSimpleBody`	net472	417ns	1.91ns	7.14ns	0.057	0	361 B
master	`BodyExtractorSimpleBody`	netcoreapp3.1	428ns	1.97ns	7.63ns	0.00376	0	272 B
master	`BodyExtractorMoreComplexBody`	net472	23.7μs	118ns	526ns	1.19	0.0117	7.62 KB
master	`BodyExtractorMoreComplexBody`	netcoreapp3.1	20.2μs	95ns	380ns	0.0896	0	6.75 KB
#2638	`AllCycleSimpleBody`	net472	300ns	1.68ns	10.2ns	0.052	0	329 B
#2638	`AllCycleSimpleBody`	netcoreapp3.1	385ns	1.27ns	4.93ns	0.00459	0	328 B
#2638	`AllCycleMoreComplexBody`	net472	292ns	1.23ns	4.24ns	0.0482	0	305 B
#2638	`AllCycleMoreComplexBody`	netcoreapp3.1	372ns	1.88ns	8.81ns	0.00416	0	304 B
#2638	`BodyExtractorSimpleBody`	net472	434ns	2.24ns	11ns	0.057	0	361 B
#2638	`BodyExtractorSimpleBody`	netcoreapp3.1	437ns	2.05ns	7.95ns	0.00366	0	272 B
#2638	`BodyExtractorMoreComplexBody`	net472	24.8μs	123ns	565ns	1.19	0.0118	7.62 KB
#2638	`BodyExtractorMoreComplexBody`	netcoreapp3.1	23.5μs	120ns	551ns	0.096	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	294μs	1.45μs	7.54μs	0.148	19.74 KB
#2638	`SendRequest`	net472	0ns	0ns	0ns	0	0 b
#2638	`SendRequest`	netcoreapp3.1	288μs	1.3μs	5.03μs	0.142	19.74 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.79μs	9.77ns	54.4ns	0.0938	0	594 B
master	`ExecuteNonQuery`	netcoreapp3.1	1.46μs	6.34ns	23.7ns	0.00891	0	632 B
#2638	`ExecuteNonQuery`	net472	1.79μs	9.9ns	61.8ns	0.0941	0.00088	594 B
#2638	`ExecuteNonQuery`	netcoreapp3.1	1.46μs	6.69ns	26.8ns	0.00864	0	632 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`CallElasticsearch`	net472	2.66μs	12.3ns	46.1ns	0.127	0	803 B
master	`CallElasticsearch`	netcoreapp3.1	1.62μs	8.16ns	38.3ns	0.0106	0	792 B
master	`CallElasticsearchAsync`	net472	2.9μs	14.2ns	63.3ns	0.149	0.00144	939 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.78μs	5.51ns	20.6ns	0.0125	0	912 B
#2638	`CallElasticsearch`	net472	2.8μs	16.1ns	123ns	0.125	0	802 B
#2638	`CallElasticsearch`	netcoreapp3.1	1.81μs	5.73ns	21.5ns	0.0108	0	792 B
#2638	`CallElasticsearchAsync`	net472	2.89μs	12.4ns	46.6ns	0.149	0.00147	939 B
#2638	`CallElasticsearchAsync`	netcoreapp3.1	1.83μs	8.89ns	36.6ns	0.0119	0	912 B

Benchmarks.Trace.GraphQLBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #2638

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync‑net472	1.171	3,152.99	3,693.22

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net472	3.16μs	18ns	127ns	0.165	1.06 KB
master	`ExecuteAsync`	netcoreapp3.1	1.98μs	7.63ns	28.5ns	0.0139	1.03 KB
#2638	`ExecuteAsync`	net472	3.7μs	18.5ns	80.7ns	0.168	1.06 KB
#2638	`ExecuteAsync`	netcoreapp3.1	2.1μs	12.2ns	101ns	0.0142	1.03 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net472	7.19μs	38ns	231ns	0.354	2.28 KB
master	`SendAsync`	netcoreapp3.1	4.88μs	28ns	222ns	0.0306	2.21 KB
#2638	`SendAsync`	net472	6.93μs	34.1ns	180ns	0.36	2.28 KB
#2638	`SendAsync`	netcoreapp3.1	5.13μs	27.8ns	155ns	0.031	2.21 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	3.38μs	16.4ns	71.6ns	0.228	1.45 KB
master	`EnrichedLog`	netcoreapp3.1	3.04μs	11.1ns	44.3ns	0.0205	1.53 KB
#2638	`EnrichedLog`	net472	3.4μs	17.1ns	97ns	0.227	1.45 KB
#2638	`EnrichedLog`	netcoreapp3.1	3.07μs	14.7ns	58.8ns	0.0208	1.53 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net472	295μs	1.39μs	5.74μs	0.442	0.147	4.33 KB
master	`EnrichedLog`	netcoreapp3.1	231μs	1.11μs	4.29μs	0	0	4.21 KB
#2638	`EnrichedLog`	net472	295μs	1.66μs	10.7μs	0.457	0.152	4.33 KB
#2638	`EnrichedLog`	netcoreapp3.1	232μs	982ns	3.54μs	0	0	4.21 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	8.04μs	38.6ns	189ns	0.503	3.23 KB
master	`EnrichedLog`	netcoreapp3.1	6.38μs	25.1ns	118ns	0.0489	3.6 KB
#2638	`EnrichedLog`	net472	7.95μs	43.8ns	324ns	0.506	3.23 KB
#2638	`EnrichedLog`	netcoreapp3.1	6.98μs	39.5ns	364ns	0.0471	3.6 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendReceive`	net472	2.3μs	8.37ns	32.4ns	0.16	1.01 KB
master	`SendReceive`	netcoreapp3.1	2.01μs	5.67ns	21.2ns	0.0141	1.01 KB
#2638	`SendReceive`	net472	2.45μs	12.5ns	83.1ns	0.158	1.01 KB
#2638	`SendReceive`	netcoreapp3.1	2.18μs	11.9ns	81.5ns	0.0142	1.01 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	5.88μs	22.5ns	84.1ns	0.292	1.87 KB
master	`EnrichedLog`	netcoreapp3.1	5.01μs	27.2ns	178ns	0.0195	1.49 KB
#2638	`EnrichedLog`	net472	6.13μs	32.5ns	169ns	0.291	1.87 KB
#2638	`EnrichedLog`	netcoreapp3.1	5.4μs	28.3ns	142ns	0.0189	1.49 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net472	941ns	4.71ns	22.1ns	0.0719	457 B
master	`StartFinishSpan`	netcoreapp3.1	852ns	2.78ns	10.4ns	0.00612	456 B
master	`StartFinishScope`	net472	1.15μs	6.56ns	46.4ns	0.0842	538 B
master	`StartFinishScope`	netcoreapp3.1	1.02μs	4.92ns	20.3ns	0.00806	576 B
#2638	`StartFinishSpan`	net472	908ns	5.02ns	34.1ns	0.0717	457 B
#2638	`StartFinishSpan`	netcoreapp3.1	868ns	3.95ns	15.3ns	0.00599	456 B
#2638	`StartFinishScope`	net472	1.13μs	6.43ns	45.5ns	0.084	538 B
#2638	`StartFinishScope`	netcoreapp3.1	1.08μs	6.15ns	46.8ns	0.00804	576 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net472	1.27μs	6.47ns	30.4ns	0.0842	538 B
master	`RunOnMethodBegin`	netcoreapp3.1	1.26μs	6.4ns	29.3ns	0.00794	576 B
#2638	`RunOnMethodBegin`	net472	1.3μs	6.77ns	41.7ns	0.0843	538 B
#2638	`RunOnMethodBegin`	netcoreapp3.1	1.17μs	3.92ns	15.2ns	0.00789	576 B

anna-git added the area:app-sec label Apr 4, 2022

anna-git force-pushed the anna/appsec/update-waf-and-ruleset-1.3.0 branch from d4b9cdf to c82b21b Compare April 4, 2022 18:00

anna-git changed the title ~~Anna/appsec/update waf and ruleset 1.3.0~~ [AppSec] Update waf and ruleset 1.3.0 Apr 4, 2022

anna-git force-pushed the anna/appsec/update-waf-and-ruleset-1.3.0 branch 3 times, most recently from c709be4 to 6b0e9d2 Compare April 4, 2022 23:15

This comment has been minimized.

Sign in to view

anna-git force-pushed the anna/appsec/update-waf-and-ruleset-1.3.0 branch from 715100c to 34683c1 Compare April 5, 2022 16:40

This comment has been minimized.

Sign in to view

anna-git force-pushed the anna/appsec/update-waf-and-ruleset-1.3.0 branch 2 times, most recently from 211b43b to 0dadf62 Compare April 6, 2022 09:59

anna-git marked this pull request as ready for review April 6, 2022 13:33

anna-git requested a review from a team as a code owner April 6, 2022 13:33

This comment has been minimized.

Sign in to view

lucaspimentel reviewed Apr 6, 2022

View reviewed changes

Comment thread tracer/src/Datadog.Trace/AppSec/Waf/ReturnTypes.Managed/InitializationResult.cs Outdated

lucaspimentel reviewed Apr 6, 2022

View reviewed changes

Comment thread tracer/src/Datadog.Trace/Tags.cs

lucaspimentel approved these changes Apr 6, 2022

View reviewed changes

lucaspimentel requested a review from robertpi April 6, 2022 21:36

This comment has been minimized.

Sign in to view

robertpi reviewed Apr 7, 2022

View reviewed changes

Comment thread tracer/src/Datadog.Trace/AppSec/Waf/NativeBindings/WafNative.cs Outdated

robertpi reviewed Apr 7, 2022

View reviewed changes

Comment thread tracer/src/Datadog.Trace/AppSec/Waf/rule-set.json Outdated

robertpi reviewed Apr 7, 2022

View reviewed changes

Comment thread ...est-applications/security/aspnet/Samples.Security.AspNetMvc5/Controllers/RenderController.cs

robertpi reviewed Apr 7, 2022

View reviewed changes

Comment thread tracer/src/Datadog.Trace/AppSec/Waf/NativeBindings/DdwafObjectStruct.cs Outdated

zacharycmontoya reviewed Apr 7, 2022

View reviewed changes

Comment thread tracer/src/Datadog.Trace/AppSec/Waf/NativeBindings/DdwafRuleSetInfoStruct.cs Outdated

zacharycmontoya reviewed Apr 8, 2022

View reviewed changes

Comment thread tracer/src/Datadog.Trace/AppSec/Waf/ReturnTypes.Managed/InitializationResult.cs Outdated

zacharycmontoya reviewed Apr 8, 2022

View reviewed changes

Comment thread tracer/src/Datadog.Trace/AppSec/Waf/Initialization/WafConfigurator.cs Outdated

zacharycmontoya reviewed Apr 8, 2022

View reviewed changes

Comment thread tracer/src/Datadog.Trace/AppSec/Waf/Initialization/WafConfigurator.cs

robertpi reviewed Apr 8, 2022

View reviewed changes

Comment thread tracer/src/Datadog.Trace/AppSec/Waf/NativeBindings/WafNative.cs

robertpi reviewed Apr 8, 2022

View reviewed changes

Comment thread tracer/src/Datadog.Trace/AppSec/Waf/ReturnTypes.Managed/InitializationResult.cs Outdated

robertpi reviewed Apr 8, 2022

View reviewed changes

anna-git force-pushed the anna/appsec/update-waf-and-ruleset-1.3.0 branch from bfdc6ac to 7318349 Compare April 8, 2022 09:52

This comment has been minimized.

Sign in to view

anna-git force-pushed the anna/appsec/update-waf-and-ruleset-1.3.0 branch from fd0d20b to 50c1359 Compare April 8, 2022 13:52

robertpi approved these changes Apr 8, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

anna-git added 13 commits April 11, 2022 10:19

First update classes

189772b

Update of the waf

dd0c6d3

Waf tests and init fixed

417a61a

change snapshots and obfuscate values

3195408

Adding some integration tests for initialization

497189c

update waf to 1.3.0

ff0f144

Skip cookies tests because of new ruleset

34c2e97

Fix tests to not run parallel and snapshots

2166a34

Fixing tests to match 1.3.0 fix snapshots

fix snapshots

bfeb64b

Response to comments

fa98926

Response to 2nd round of comments

0dcfc40

Add default rule file

fix initialization

5795922

Return if ddwafObjectStruct from resultsetinfo is not a map, but it s…

114abe9

…hould be a map coming from the waf

anna-git force-pushed the anna/appsec/update-waf-and-ruleset-1.3.0 branch from 2e852af to 114abe9 Compare April 11, 2022 08:23

anna-git merged commit 660ae6e into master Apr 11, 2022

anna-git deleted the anna/appsec/update-waf-and-ruleset-1.3.0 branch April 11, 2022 15:47

github-actions Bot added this to the vNext milestone Apr 11, 2022

lucaspimentel added area:asm and removed area:app-sec labels Aug 25, 2022

Uh oh!

Conversation

anna-git commented Apr 4, 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.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

robertpi Apr 8, 2022

Choose a reason for hiding this comment

Uh oh!

anna-git Apr 8, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

robertpi left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

andrewlock commented Apr 11, 2022

Code Coverage Report 📊

Datadog.Trace Breakdown ✔️

Uh oh!

andrewlock commented Apr 11, 2022

Benchmarks Report 🐌

Benchmark details

Raw results

Slower ⚠️ in #2638

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #2638

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

anna-git commented Apr 4, 2022 •

edited

Loading