[ASM] Waf and ruleset update#3087
Merged
Merged
Conversation
This comment has been minimized.
This comment has been minimized.
anna-git
force-pushed
the
dani/waf_v1_5_0
branch
from
August 31, 2022 11:04
cbda4e6 to
f69cc40
Compare
This comment has been minimized.
This comment has been minimized.
anna-git
force-pushed
the
dani/waf_v1_5_0
branch
from
August 31, 2022 13:55
f69cc40 to
3f77b91
Compare
This comment has been minimized.
This comment has been minimized.
anna-git
force-pushed
the
dani/waf_v1_5_0
branch
from
August 31, 2022 16:43
3f77b91 to
7a5a41a
Compare
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.
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.
This comment has been minimized.
This comment has been minimized.
anna-git
force-pushed
the
dani/waf_v1_5_0
branch
from
September 1, 2022 13:59
fe31743 to
fbe519a
Compare
This comment has been minimized.
This comment has been minimized.
anna-git
added
area:asm
status:do-not-merge
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.
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.
anna-git
force-pushed
the
dani/waf_v1_5_0
branch
from
September 2, 2022 14:21
6fd3e0b to
6334a3e
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
anna-git
force-pushed
the
dani/waf_v1_5_0
branch
from
September 5, 2022 14:09
6334a3e to
82f5ddf
Compare
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.
This comment has been minimized.
anna-git
force-pushed
the
dani/waf_v1_5_0
branch
from
September 7, 2022 11:40
82f5ddf to
1e1238d
Compare
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.
This comment has been minimized.
Fix tests update to alpha 1
Fix small modification sin snapshosts, keypath is now a string instead of an int fix smoke tests
Fix tests update to alpha 1
anna-git
force-pushed
the
dani/waf_v1_5_0
branch
from
September 16, 2022 10:09
32b9045 to
22b304b
Compare
Contributor
Snapshots difference summaryThe following differences have been observed in snapshots. So diff is simplistic, so please check some files anyway while we improve it. 10 occurrences of : - _dd.appsec.json: {"triggers":[{"rule":{"id":"ublock","name":"Hello","tags":{"category":"attack_attempt","type":"security_scanner"}},"rule_matches":[{"operator":"match_regex","operator_value":"Hello\\/v","parameters":[{"address":"server.request.headers.no_cookies","highlight":["Hello/V"],"key_path":["user-agent",0],"value":"Mistake Not... Hello/V"}]}]}]},
+ _dd.appsec.json: {"triggers":[{"rule":{"id":"ublock","name":"Hello","tags":{"category":"attack_attempt","type":"security_scanner"}},"rule_matches":[{"operator":"match_regex","operator_value":"Hello\\/v","parameters":[{"address":"server.request.headers.no_cookies","highlight":["Hello/V"],"key_path":["user-agent","0"],"value":"Mistake Not... Hello/V"}]}]}]},
25 occurrences of : - _dd.appsec.event_rules.version: 1.3.1,
+ _dd.appsec.event_rules.version: 1.4.0,
20 occurrences of : - _dd.appsec.event_rules.version: 1.3.1,
- _dd.appsec.json: {"triggers":[{"rule":{"id":"crs-942-290","name":"Finds basic MongoDB SQL injection attempts","tags":{"category":"attack_attempt","type":"nosql_injection"}},"rule_matches":[{"operator":"match_regex","operator_value":"(?i:(?:\\[\\$(?:ne|eq|lte?|gte?|n?in|mod|all|size|exists|type|slice|x?or|div|like|between|and)\\]))","parameters":[{"address":"server.request.query","highlight":["[$slice]"],"key_path":["[$slice]"],"value":"[$slice]"}]}]}]},
+ _dd.appsec.event_rules.version: 1.4.0,
+ _dd.appsec.json: {"triggers":[{"rule":{"id":"crs-942-290","name":"Finds basic MongoDB SQL injection attempts","tags":{"category":"attack_attempt","type":"nosql_injection"}},"rule_matches":[{"operator":"match_regex","operator_value":"(?i:(?:\\[?\\$(?:(?:s(?:lic|iz)|wher)e|e(?:lemMatch|xists|q)|n(?:o[rt]|in?|e)|l(?:ike|te?)|t(?:ext|ype)|a(?:ll|nd)|jsonSchema|between|regex|x?or|div|mod)\\]?))","parameters":[{"address":"server.request.query","highlight":["[$slice]"],"key_path":["[$slice]"],"value":"[$slice]"}]}]}]},
5 occurrences of : - _dd.appsec.event_rules.version: 1.3.1,
- _dd.appsec.json: {"triggers":[{"rule":{"id":"crs-942-290","name":"Finds basic MongoDB SQL injection attempts","tags":{"category":"attack_attempt","type":"nosql_injection"}},"rule_matches":[{"operator":"match_regex","operator_value":"(?i:(?:\\[\\$(?:ne|eq|lte?|gte?|n?in|mod|all|size|exists|type|slice|x?or|div|like|between|and)\\]))","parameters":[{"address":"server.request.query","highlight":["[$slice]"],"key_path":["[$slice]"],"value":"[$slice]"}]}]}]},
- _dd.appsec.waf.version: 1.3.0,
+ _dd.appsec.event_rules.version: 1.4.0,
+ _dd.appsec.json: {"triggers":[{"rule":{"id":"crs-942-290","name":"Finds basic MongoDB SQL injection attempts","tags":{"category":"attack_attempt","type":"nosql_injection"}},"rule_matches":[{"operator":"match_regex","operator_value":"(?i:(?:\\[?\\$(?:(?:s(?:lic|iz)|wher)e|e(?:lemMatch|xists|q)|n(?:o[rt]|in?|e)|l(?:ike|te?)|t(?:ext|ype)|a(?:ll|nd)|jsonSchema|between|regex|x?or|div|mod)\\]?))","parameters":[{"address":"server.request.query","highlight":["[$slice]"],"key_path":["[$slice]"],"value":"[$slice]"}]}]}]},
+ _dd.appsec.waf.version: 1.5.0,
5 occurrences of : - _dd.appsec.event_rules.loaded: 126.0,
+ _dd.appsec.event_rules.loaded: 131.0,
35 occurrences of : - _dd.appsec.json: {"triggers":[{"rule":{"id":"crs-942-290","name":"Finds basic MongoDB SQL injection attempts","tags":{"category":"attack_attempt","type":"nosql_injection"}},"rule_matches":[{"operator":"match_regex","operator_value":"(?i:(?:\\[\\$(?:ne|eq|lte?|gte?|n?in|mod|all|size|exists|type|slice|x?or|div|like|between|and)\\]))","parameters":[{"address":"server.request.query","highlight":["[$slice]"],"key_path":["arg",0],"value":"[$slice]"}]}]}]},
+ _dd.appsec.json: {"triggers":[{"rule":{"id":"crs-942-290","name":"Finds basic MongoDB SQL injection attempts","tags":{"category":"attack_attempt","type":"nosql_injection"}},"rule_matches":[{"operator":"match_regex","operator_value":"(?i:(?:\\[\\$(?:ne|eq|lte?|gte?|n?in|mod|all|size|exists|type|slice|x?or|div|like|between|and)\\]))","parameters":[{"address":"server.request.query","highlight":["[$slice]"],"key_path":["arg","0"],"value":"[$slice]"}]}]}]},
1 occurrences of : - _dd.appsec.waf.version: 1.3.0,
+ _dd.appsec.waf.version: 1.5.0,
15 occurrences of : - _dd.appsec.json: {"triggers":[{"rule":{"id":"crs-942-290","name":"Finds basic MongoDB SQL injection attempts","tags":{"category":"attack_attempt","type":"nosql_injection"}},"rule_matches":[{"operator":"match_regex","operator_value":"(?i:(?:\\[\\$(?:ne|eq|lte?|gte?|n?in|mod|all|size|exists|type|slice|x?or|div|like|between|and)\\]))","parameters":[{"address":"server.request.query","highlight":["[$slice]"],"key_path":["param",0],"value":"[$slice]"}]}]}]},
+ _dd.appsec.json: {"triggers":[{"rule":{"id":"crs-942-290","name":"Finds basic MongoDB SQL injection attempts","tags":{"category":"attack_attempt","type":"nosql_injection"}},"rule_matches":[{"operator":"match_regex","operator_value":"(?i:(?:\\[\\$(?:ne|eq|lte?|gte?|n?in|mod|all|size|exists|type|slice|x?or|div|like|between|and)\\]))","parameters":[{"address":"server.request.query","highlight":["[$slice]"],"key_path":["param","0"],"value":"[$slice]"}]}]}]},
|
Member
Benchmarks Report 🐌Benchmarks for #3087 compared to master:
The following thresholds were used for comparing the benchmark speeds:
Allocation changes below 0.5% are ignored. Benchmark detailsBenchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AppSecBodyBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AspNetCoreBenchmark - 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
|
anna-git
reviewed
Sep 16, 2022
| return ret == DDWAF_RET_CODE.DDWAF_OK; | ||
| } | ||
|
|
||
| internal static List<object> MergeRuleDatas(IEnumerable<RuleData[]> res) |
Contributor
There was a problem hiding this comment.
here we receive several files of content
"rules_data": [
…,
{
"id": “blocklist”,
"type": “data_with_expiration”,
"data": [
…,
{ “value”: “127.0.0.1”, “expiration”: … },
…,
]
},
…
]
As a result, multiple Rule Data may use the same DATA_ID and DATA_TYPE. In this case, all values must be merged
When a value is associated with an expiration date, the latest date takes precedence
For the sake of readability and convenience, I used linq here, I believe without linq it would be very long and cumbersome 😅
Member
Code Coverage Report 📊✔️ Merging #3087 into master will not change line coverage
View the full report for further details: Datadog.Trace Breakdown ✔️
The following classes have significant coverage changes.
1 classes were removed from Datadog.Trace in #3087 View the full reports for further details: |
robertpi
approved these changes
Sep 16, 2022
andrewlock
approved these changes
Sep 16, 2022
| span.kind: server, | ||
| _dd.appsec.event_rules.version: 1.3.0, | ||
| _dd.appsec.json: {"triggers":[{"rule":{"id":"ublock","name":"Hello","tags":{"category":"attack_attempt","type":"security_scanner"}},"rule_matches":[{"operator":"match_regex","operator_value":"Hello\\/v","parameters":[{"address":"server.request.headers.no_cookies","highlight":["Hello/V"],"key_path":["user-agent",0],"value":"Mistake Not... Hello/V"}]}]}]}, | ||
| _dd.appsec.json: {"triggers":[{"rule":{"id":"ublock","name":"Hello","tags":{"category":"attack_attempt","type":"security_scanner"}},"rule_matches":[{"operator":"match_regex","operator_value":"Hello\\/v","parameters":[{"address":"server.request.headers.no_cookies","highlight":["Hello/V"],"key_path":["user-agent","0"],"value":"Mistake Not... Hello/V"}]}]}]}, |
Member
There was a problem hiding this comment.
Just checking, is the change in user-agent from a number to a string correct?
Contributor
There was a problem hiding this comment.
yes indeed, it's been something that changed with waf 1.5 and that has been changed accordingly in the system tests
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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
Update waf version to 1.5.0 > https://github.com/DataDog/libddwaf/blob/master/UPGRADING.md
Update local ruleset to 1.4.0
Ability to merge rule datas because the rule datas need to be merged before being sent to the waf.
Reason for change
libddwaf has a new version which is needed for blocking requests later on
It is now able to execute rules based on rule datas that can be updated.
Implementation details
libddwaf 1.5.0 has some API breaking changes:
ddwaf_context_initno longer needs the FreeFunction argumentddwaf_get_versionnow returns a string instead of filling up aVersionStructDDWAF_GOODtoDDWAF_OKandDDWAF_MONITORtoDDWAF_MATCH)Test coverage
Other details