8000 Add common tag for all rules in a file · Issue #3991 · coreruleset/coreruleset · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Closed
Kortekaasy opened this issue Feb 6, 2025 · 13 comments · Fixed by #3992 or #4031
Closed

Add common tag for all rules in a file #3991

Kortekaasy opened this issue Feb 6, 2025 · 13 comments · Fixed by #3992 or #4031

Comments

@Kortekaasy
Copy link

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 the attack-sqli tag. However, for the multipart attack based rules (REQUEST-922-MULTIPART-ATTACK.conf) there are two different tags; rule 922110 has tag attack-protocol, while rule 922120 has tag attack-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'

@Xhoenix
Copy link
Member
Xhoenix commented Feb 7, 2025

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.

@Kortekaasy
Copy link
Author

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.

@Xhoenix
Copy link
Member
Xhoenix commented Feb 7, 2025

A quick check shows only MULTIPART-ATTACK file is not consistent, and we just need to add another tag, or perhaps just add the attack-multipart-header tag to all rules in the file. If you find any other file to be not consistent, let me know.

@Kortekaasy
Copy link
Author
Kortekaasy commented Feb 7, 2025

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

@Xhoenix
Copy link
Member
Xhoenix commented Feb 7, 2025

Looks like a lot of work to do. I'll keep this issue open until a fix has been provided.

8000
@Xhoenix Xhoenix reopened this Feb 7, 2025
@Kortekaasy
Copy link
Author

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

@Xhoenix
Copy link
Member
Xhoenix commented Feb 8, 2025

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

@Xhoenix
Copy link
Member
Xhoenix commented Feb 8, 2025

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.

@fzipi
Copy link
Member
fzipi commented Mar 1, 2025

Do we need to create a new ticket to track the work here, or maybe add sub-issues?

@fzipi fzipi closed this as completed Mar 1, 2025
@fzipi fzipi reopened this Mar 1, 2025
@Xhoenix
Copy link
Member
Xhoenix commented Mar 3, 2025

@fzipi I'll send a PR today.

@fzipi
Copy link
Member
fzipi commented Mar 3, 2025

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

@RedXanadu
Copy link
Member

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.

@dune73
Copy link
Member
dune73 commented Mar 3, 2025

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
0