ModSecurity-nginx icon indicating copy to clipboard operation
ModSecurity-nginx copied to clipboard

Question about conflicting modsecurity_rules/modsecurity_rules_file precedence

Open mac-chaffee opened this issue 3 years ago • 10 comments

This question is in relation to some troubles the downstream ingress-nginx project has with modsecurity: https://github.com/kubernetes/ingress-nginx/issues/8388

When conflicting modsecurity rules/settings are loaded, how is that conflict resolved? Is it always "last write wins"? Seems that's not always the case.

For example, say I have the following config:

modsecurity on;
modsecurity_rules 'SecRuleEngine On';
modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf;
  # modsecurity.conf includes the line: "SecRuleEngine DetectionOnly"
modsecurity_rules_file /etc/nginx/owasp-modsecurity-crs/nginx-modsecurity.conf;
  # Just Includes all the coreruleset rules files

If "last write wins", I would expect that modsecurity_rules 'SecRuleEngine On'; would do nothing; the subsequent line would override that and set the mode to DetectionOnly. But for me, the mode stays "On" with the above config and requests get blocked instead of just logged. However other users have reported the opposite behavior as well: https://github.com/kubernetes/ingress-nginx/pull/8021

Looking at the source code, I would expect that "last write wins" is the intended behavior: https://github.com/SpiderLabs/ModSecurity/blob/5519f6cfae45a47d8dc3ac1b084319611a9b386b/headers/modsecurity/rules_set_properties.h#L45-L48

But are there situations where "last write wins" doesn't hold?

mac-chaffee avatar Apr 11 '22 18:04 mac-chaffee

Hi @mac-chaffee ,

I'd have to take a more in-depth look to provide a definitive answer, but a similar-sounding question arose at least once before ( https://github.com/SpiderLabs/ModSecurity-nginx/issues/183 ).

The hypothesis in that case was that using two different nginx directives (i.e. both 'modsecurity_rules' and 'modsecurity_rules_file') may result in the precedence of those directives being dominant.

Based on that earlier issue, if you were to use either 'modsecurity_rules' or 'modsecurity_rules_file' in both of those cases, that would likely give you the 'last wins' behaviour that you are expecting.

martinhsv avatar Apr 11 '22 20:04 martinhsv

'modsecurity_rules' and 'modsecurity_rules_file' have precedence of their own outside of just the order in which they're listed in nginx.conf? Just skimming the code, it looks like those directives both just edit the same RulesSet struct:

https://github.com/SpiderLabs/ModSecurity-nginx/blob/2497e6ac654d0b117b9534aa735b757c6b11c84f/src/ngx_http_modsecurity_module.c#L310-L375

Which calls these functions:

https://github.com/SpiderLabs/ModSecurity/blob/b89c737ad3f20cccfe966631b28157f3b5d25832/src/rules_set.cc#L294-L311

mac-chaffee avatar Apr 11 '22 20:04 mac-chaffee

Oh I do see they are changing mmcf->rules_inline vs. mmcf->rules_file, so maybe there is a difference, hmm

EDIT: oh that's just a counter, nvm I should stop guessing haha

mac-chaffee avatar Apr 11 '22 20:04 mac-chaffee

it will just merge.

liudongmiao avatar Apr 21 '22 05:04 liudongmiao

Anything more on this?

martinhsv avatar Apr 25 '22 13:04 martinhsv

Yes, sounds like we haven't identified exactly what should take precedence over what.

It would be good to document this somewhere because the prevailing wisdom is incorrect: that last writer always wins and they will "just merge".

In my experiments, it seems that modsecurity_rules does take precedence over modsecurity_rules_file, but as I mentioned other users have reported the opposite. So there's also the possibility there could be a bug in the code that makes the order inconsistent.

We'd need a firm answer on how rule precedence works in modsecurity-nginx before documenting anything I think.

mac-chaffee avatar Apr 25 '22 15:04 mac-chaffee

So your issue is resolved? -- perhaps by using either of those directives both times and not mixing?

martinhsv avatar Apr 25 '22 16:04 martinhsv

If mixing modsecurity_rules/modsecurity_rules_file is supported, then my question is still unanswered. But if you're saying mixing modsecurity_rules/modsecurity_rules_file isn't supported, I'd think we should definitely document that.

I'd be happy to contribute documentation and inform the ingress-nginx folks (they mix directives too), just not sure what to tell them.

mac-chaffee avatar Apr 25 '22 21:04 mac-chaffee

Hi @mac-chaffee ,

I didn't say it isn't supported. I was only suggesting:

  • a probable reason that you were experiencing a result that surprised you and
  • a way to avoid that surprising result (not mixing the two)

At the moment, I cannot really devote time to investigating a more in-depth answer. I did hope, however, to help you deal with your immediate issue.

martinhsv avatar Apr 26 '22 13:04 martinhsv

Any new info here? What's the expected behaviour with both "modsecurity_rules" and "modsecurity_rules_file" options in use, as that's what you easily end up with when using k8s ingress-nginx modsecurity features and config options..

pasikarkkainen avatar Jan 31 '23 16:01 pasikarkkainen