coreruleset / nextcloud-rule-exclusions-plugin

Rule exclusion plugin for Nextcloud
Apache License 2.0
11 stars 7 forks source link

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

Closed EsadCetiner closed 7 months ago

EsadCetiner commented 9 months ago

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 Nextcloud plugin for your Nextcloud 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.

azurit commented 9 months ago

@EsadCetiner From where you get the information that SecAction cannot be removed using ctl? I just tried it with ModSec2 and it's fully working.

EsadCetiner commented 9 months ago

@azurit I've just tested on ModSec2 and I'm able to disable SecAction via ctl action. However, I can't for libmodsecurity3, looks like this is a bug in the libmodsecurity3 engine.

azurit commented 9 months ago

@airween Can you confirm that Modsec3 is not able to remove/disable SecAction rules using ctl?

theseion commented 9 months ago

@azurit I've just tested on ModSec2 and I'm able to disable SecAction via ctl action. However, I can't for libmodsecurity3, looks like this is a bug in the libmodsecurity3 engine.

What a surprise...

EsadCetiner commented 7 months ago

now that modsecurity has been transferred to OWASP, should I try and get this fixed there? what about users that are using the outdated libmodsecurity versions provided by their distro?

theseion commented 7 months ago

now that modsecurity has been transferred to OWASP, should I try and get this fixed there?

Yes, absolutely.

what about users that are using the outdated libmodsecurity versions provided by their distro?

I would write the rules for the latest version and document the issue + possible workaround in the documentation.

EsadCetiner commented 7 months ago

Opened an issue: https://github.com/owasp-modsecurity/ModSecurity/issues/3053

airween commented 7 months ago

Really sorry guys, I didn't see this issue (next time poke me on Slack if I don't answer :))

I think this work on libmodsecurity3 as we expected.

@EsadCetiner thanks for very detailed report, I was able to build the test environment - I added this extra config to my REQUEST-900-EXCLUSION-RULES-BEFORE-CRS.conf:

SecRule REQUEST_HEADERS:Host "!@streq example.com" "id:10,phase:1,pass,t:none,nolog,ctl:ruleRemoveById=20"

SecAction \
    "id:20,phase:1,pass,t:none,nolog,ctl:ruleRemoveByTag=OWASP_CRS"

as you can see I changed only the rule ID's just for sure.

I sent this request:

curl -v http://localhost?exec/bin/bash

and here is what I see in the log:

$ grep "Skipped due to a ruleRemoveByTag action" /var/log/nginx/modsec_debug.log 
[170725108235.456451] [/?exec/bin/bash] [9] Skipped rule id '911100'. Skipped due to a ruleRemoveByTag action.
[170725108235.456451] [/?exec/bin/bash] [9] Skipped rule id '913100'. Skipped due to a ruleRemoveByTag action.
[170725108235.456451] [/?exec/bin/bash] [9] Skipped rule id '920100'. Skipped due to a ruleRemoveByTag action.
[170725108235.456451] [/?exec/bin/bash] [9] Skipped rule id '920160'. Skipped due to a ruleRemoveByTag action.
[170725108235.456451] [/?exec/bin/bash] [9] Skipped rule id '920170'. Skipped due to a ruleRemoveByTag action.
[170725108235.456451] [/?exec/bin/bash] [9] Skipped rule id '920171'. Skipped due to a ruleRemoveByTag action.
[170725108235.456451] [/?exec/bin/bash] [9] Skipped rule id '920180'. Skipped due to a ruleRemoveByTag action.
[170725108235.456451] [/?exec/bin/bash] [9] Skipped rule id '920181'. Skipped due to a ruleRemoveByTag action.
...
...

$ grep "Skipped due to a ruleRemoveByTag action" /var/log/nginx/modsec_debug.log | wc -l
321

$ grep "tag:'OWASP_CRS'" *.conf  | wc -l
321

I think that really works as well.

(I will refer to this answer there.)

Let me know if I can help you anything.

airween commented 7 months ago

Okay, apologize everyone, I misunderstood the issue for fist check. It's definitely a bug. See my comment there.

EsadCetiner commented 7 months ago

@theseion

I would write the rules for the latest version and document the issue + possible workaround in the documentation.

Wouldn't it be easier and better for the end user if we just applied the workaround in the plugin like I've done here?

azurit commented 7 months ago

I agree with @EsadCetiner - we should make it work out-of-the-box for both modsec versions. It's not such a big deal to use @unconditionalMatch instead of SecAction.

azurit commented 7 months ago

BTW, is there a modsec3 version without this bug?

airween commented 7 months ago

BTW, is there a modsec3 version without this bug?

Yes, there is.

azurit commented 7 months ago

BTW, is there a modsec3 version without this bug?

Yes, there is.

Sorry, can't see it there. Can you be more specific?

airween commented 7 months ago

Can you be more specific?

Sure: as @EsadCetiner explained, if you create an exclusion with a tag, eg:

SecRule REQUEST_FILENAME "@beginsWith /admin" \
    "id:100,\
    phase:1,\
    pass,\
    nolog,\
    ctl:ruleRemoveByTag=OWASP_CRS"

then SecAction's won't be excluded - despite they have actin tag:OWASP_CRS.

EsadCetiner commented 7 months ago

@airween I think @azurit is asking if there's a libModSecurity3 version that's not affected by the bug, not how to reproduce the bug.

theseion commented 7 months ago

@theseion

I would write the rules for the latest version and document the issue + possible workaround in the documentation.

Wouldn't it be easier and better for the end user if we just applied the workaround in the plugin like I've done here?

Sure, also an option :)

airween commented 7 months ago

@airween I think @azurit is asking if there's a libModSecurity3 version that's not affected by the bug, not how to reproduce the bug.

sorry, that wasn't clear. Btw no, there is no such version that isn't affected. The issue presents in each versions.