-
-
Notifications
You must be signed in to change notification settings - Fork 402
Add common tag for all rules in a file #3991
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.
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
Comments
Hi @Kortekaasy, thanks for reporting this. There is already a list of APPROVED_TAGS, you can check in this list whether something solves your issue. I'll add your suggestion to our next monthly agenda meeting, so that we can discuss this further for a probable solution. |
Hi @Xhoenix, thank you for looking at this ticket. It is nice to see the list of approved tags, but that does not remediate the fact that I cannot target entire "rule files" as described in the ticket above with a single tag. I appreciate you will discuss this in your next monthly agenda meeting. |
A quick check shows only MULTIPART-ATTACK file is not consistent, and we just need to add another tag, or perhaps just add the |
Hi @Xhoenix, Thanks for the update. This still does not completely solve my issue. There are still rule files where there is no common tag. I could find:
Furthermore, there are still tags that are used as 'common tag' over multiple files. For example:
Please re-open the issue while this is not resolved |
Looks like a lot of work to do. I'll keep this issue open until a fix has been provided. |
I don't mind opening a PR and updating all of the non-conforming rules, but a bit of guidance on how to tag the rules would be nice then :) |
You can learn more about contributing from here: https://coreruleset.org/docs/6-development/6-1-contribution-guidelines/index.html About guidance on tagging, you can refer my PR: #3992 |
I checked and turns out some tags are in multiple files, which is going to take a lot of refactoring, so it'd be better if you add new tags for each file to the APPROVED_TAGS file, and add those tags in the respective files. |
Do we need to create a new ticket to track the work here, or maybe add sub-issues? |
@fzipi I'll send a PR today. |
@Kortekaasy I'm adding this as a discussion for today's meeting agenda, if you are around. While I see your reasoning, we will be moving away hopefully in the future from specific files and towards a more structured based on tagging, which in turn could help with this approach. |
This is a boring but important observation: We have, for some time, wanted to measure the affect of adding tags on "RuleRemoveByTag" actions. These actions are all (?) regex-based, so if we double the number of tags we currently have, for example, how does that impact the time taken for these actions to execute? And how do the different engines compare? Hopefully these actions are efficient (as efficient as a regex can be), but we really should test this. |
We still have the performance testing framework from a former Google Summer of Code. This was meant to be used to test tagging performance. But it's not in a usable state unfortunately. |
Motivation
Currently, all rules are neatly structured on a per-file basis. This makes it easy to in-/exclude rules for types of attacks that you find relevant, by simply loading or not loading a rule file. However, this convenience breaks down when you want to check different combinations of rule files based on the endpoint. For example, I might want to check for only XSS and SQL injection attacks on endpoint
/a/
, but check for only Java-based attacks on endpoint/b/
.One way this would be possible is by tag-based exclusion, by excluding all rules with a certain tags for endpoint
/a/
and excluding all rules with certain tags for endpoint/b/
. This works in theory, but is hindered in practice by the fact that rule tagging is not consistent per rule file.For example, all SQL-injection based rules (REQUEST-942-APPLICATION-ATTACK-SQLI.conf) all have the tag
tag:'attack-sqli'
, so for endpoints that do not require SQL injection checking I can exclude rules with theattack-sqli
tag. However, for the multipart attack based rules (REQUEST-922-MULTIPART-ATTACK.conf) there are two different tags; rule 922110 has tagattack-protocol
, while rule 922120 has tagattack-deprecated-header
.Furthermore, all rules in REQUEST-920-PROTOCOL-ENFORCEMENT.conf and REQUEST-921-PROTOCOL-ATTACK.conf (and partially REQUEST-922-MULTIPART-ATTACK.conf) share the
attack-protocol
tag.Proposed solution
I propose that all rules grouped together in a file, should also have a common tag to in-/exclude them by. We could tag them based on the name of the rule file. For example, for all rules in REQUEST-920-PROTOCOL-ENFORCEMENT.conf this could look like
tag:'OWASP_CRS/PROTOCOL-ENFORCEMENT'
Alternatives
Alternatively, we can tag rules with their common prefix. For i.e., for all rules in REQUEST-920-PROTOCOL-ENFORCEMENT.conf this could look like
tag:'OWASP_CRS/920'
The text was updated successfully, but these errors were encountered: