iis: fail closed (HTTP 500) on ModSecurity config load failure - #3617
iis: fail closed (HTTP 500) on ModSecurity config load failure#3617A13501350 wants to merge 1 commit into
Conversation
Previously, when the ModSecurity configuration could not be retrieved or parsed, OnBeginRequest set hr=S_OK and fell through to RQ_NOTIFICATION_CONTINUE, so requests silently bypassed the WAF (fail-open). On a malformed or missing config this leaves the site unprotected. Now explicitly return HTTP 500 with a clear error when the config cannot be loaded. A short retry window (10s) is cached so a transient failure does not hammer the event log, and a persistent failure recycles the w3wp worker. Adapted from microsoft/ModSecurity waf_iis.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
1 participant
Summary
In
iis/mymodule.cpp,CMyHttpModule::OnBeginRequestcurrently handles a failure to retrieve or parse the ModSecurity configuration by settinghr = S_OKand falling through toRQ_NOTIFICATION_CONTINUE. That means a request for which the WAF configuration could not be loaded is passed through without any inspection -- i.e. the WAF silently fails open.On a malformed or missing
modsecurity.conf(or a config that fails to parse), this leaves the site completely unprotected, with no signal to the client.Change
WAF internal error. ...) andSetRequestHandled()instead of continuing.configFailTime/configLoadingFailedon the stored context so a transient failure does not spam the event log every request: within a 10s window the cached failure is reported immediately; after that the w3wp worker is recycled so it re-reads the config.#include <time.h>(the file now usestime()/difftime()).Why fail closed
For a security control, failing open on a bad config is the more dangerous default. Returning 500 makes the failure visible and avoids silently unprotected traffic. (If a deployment truly needs fail-open behaviour this can be discussed, but the safe default is fail closed.)
Provenance
Adapted from
microsoft/ModSecuritybranchwaf_iis(config-load-failure handling). Verified still missing on currentv2/mastertip (0875b19, v2.9.14).