Skip to content
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

fix: SecAction can't be disabled via ctl action #26

Merged
merged 5 commits into from
Feb 16, 2024

Conversation

EsadCetiner
Copy link
Member

SecAction rules can't be disabled via a ctl action or run time rule exclusion
This can be problematic in reverse proxy deployments where you only want to enable the WordPress plugin for your WordPress domain.
This PR fixes that by replacing the SecAction with a SecRule that's functionally the exact same, except now it can be disabled via a ctl action.

@theseion
Copy link
Contributor

I've thought about this change a little. CRS isn't designed to be enabled on a per location / server basis (mainly due to ModSecurity). I can see the use case though. However, I wonder whether this change is the best way to achieve what you want. If I understand correctly, you would write a rule for each location / server, where you wanted to disable the exclusion rule set, something like ... crl:ruleRemoveById=9508000-9508999. This is bad for two reasons:

  • the rules are disabled dynamically, i.e., they are are still loaded into memory (not sure whether this makes a difference if they are loaded at least once anyway).
  • it's cumbersome to read and write

Personally, I would try to solve this with the SkipAfter directive and a configuration variable, something like this (you'd need some end marker, of course):

...
setvar:'tx.enable_location_mylocation=1'
...

SecRule TX:ENABLE_LOCATION_MYLOCATION "@eq 1" "id:9508999,phase:1,pass,nolog,skipAfter:END-MARKER"

@dune73, what do you think?

@EsadCetiner
Copy link
Member Author

You'd just have to write one ModSec rule where if the host header doesn't equal say example.com, then you'd disable the plugin via a ctl action. This is how I personally do it. I'm not too sure about the performance but this shouldn't be combersome to write at all since it's just one rule.

SecRule &TX:wordpress-rule-exclusions-plugin_enabled "@eq 0" \
    "id:9507010,\
    phase:1,\
    pass,\
    nolog,\
    chain"
    SecRule REQUEST_HEADERS:Host "!@streq example.com" \
        "t:none,\
        setvar:'tx.wordpress-rule-exclusions-plugin_enabled=0'"

or if you wanted to disable per location

SecRule &TX:wordpress-rule-exclusions-plugin_enabled "@eq 0" \
    "id:9507010,\
    phase:1,\
    pass,\
    nolog,\
    chain"
    SecRule REQUEST_FILENAME "!@beginsWith /wordpresss/" \
        "t:none,\
        setvar:'tx.wordpress-rule-exclusions-plugin_enabled=0'"

@azurit
Copy link
Member

azurit commented Dec 18, 2023

you would write a rule for each location / server, where you wanted to disable the exclusion rule set

You can disable plugin globally and enable it only where it's needed.

@theseion theseion merged commit 47ad545 into coreruleset:master Feb 16, 2024
3 checks passed
@azurit
Copy link
Member

azurit commented Feb 16, 2024

We should write an explanation of using unconditionalMatch instead of SecAction in the comment above the rule

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

Successfully merging this pull request may close these issues.

3 participants