Different behavior of the engines implicit `@rx` operator

[airween]

This is not a bug report, but more of a discussion thread.

As you know, ModSecurity allows to create a SecRule without operator. In this case the @rx operator will be used. Here are the documented behaviors:

v2: "This is the default operator; the rules that do not explicitly specify an operator default to @rx."

v3: "This is the default operator; the rules that do not explicitly specify an operator default to @rx."

Let's take a step back and see the next cases.

Create a rule which checks a regular expression:

SecRule ... "@rx attack" "..."

or we can check the exact match:

SecRule ... "@strEq attack" "..."

The argument can be anything - for example it can be a domain part of the e-mail address:

SecRule ... "@rx @modsecurity.org" "..."
SecRule ... "@contains @modsecurity.org" "..."

And now we can leave the operator:

SecRule ... "@modsecurity.org" "..."

Now what happens? It depends on what version of ModSecurity you use.

In case of v2, you will get an error message:

Error creating rule: Failed to resolve operator: modsecurity.org

In case of v3, there won't be any error message, the engine will create a SecRule (what you won't see):

SecRule ... "@rx @modsecurity.org" ...

Now imagine what happens if someone makes a typo in the operator's name...

Expected behavior

The aim of this issue is to discuss about what is the expected behavior in this case.

I think using of implicit configuration at any place is a very bad idea. I would leave it at all, I mean I would completely eliminate it.

[S0obi]

Hey @airween,

Thanks for creating this issue, I would like to share my experience here. I am using a fresh installation of modsecurity v3.0.11, compiled with nginx sources. I faced an unexpected behavior with this particular rule :

SecRule REQUEST_URI "@equals /analytics/matomo.php" "id:2000,phase:2,t:none,pass,nolog,ctl:ruleRemoveById=920420"

The operator @equals doesn't exist (I should have use @strEq), however nginx will start and modsecurity will not complain. The only visible symptom was using the nginx check config nginx -t :

ubuntu@external:~$ sudo nginx -t
nginx: the configuration file /etc/nginx/nginx.conf syntax is ok
nginx: configuration file /etc/nginx/nginx.conf test is successful
ualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsualsuals
ubuntu@external:~$

It is still unclear to me if that's 100% linked to this particular issue however, these "uals" words seem like the part of "[eq]uals". The number of "uals" correspond to the number of inclusion of rule 2000 described above.

This actually resonates with your words :

Now imagine what happens if someone makes a typo in the operator's name...

To me, v2 behavior is healthier and throwing an error message will avoid complex troubleshooting on user side.

[fzipi]

My 2 cents: there should be no implicit operator anymore. If you want @rx, you need to add it to the rule. This will be a breaking change, but it is sanity in the end.

Of course, the result of this new behavior is to fail when there is no operator, (suggesting users to add @rx, if we want backwards compat), and also fail like v2.

[mirkodziadzka-avi]

Personal opinions:

• I agree with @fzipi that there should be always an explicit operator in any sane rule and making @rx the default was in hindsight not the best decicion
• Although I think we should not do a backward breaking changes easily
• In @airween case of SecRule ... "@modsecurity.org" "..." ... I think this should be a syntax error. @ at the beginning of the operator field should be the clear indication that a valid operator is following. So I think v2 is right here.

[mirkodziadzka-avi]

To add a point to to "no backward breaking changes": I glimpsed at the rules in one commercial ruleset I have access to, and there is at least one rule which does not have an operator. So users of modsec are currently relying on this behavior.

[fzipi]

There could be a message a standard deprecation message saying that the syntax is going to be removed and discourage its use. Then in 5 years you just remove it.

[zimmerle]

As the creator of the libModSecurity parser, I'd like to offer my insights into this discussion. Version 2 of the parser was designed to mirror the Apache configuration style, lacking a bespoke parser and instead adapting to the constraints of Apache's configuration files. This approach changed with Version 3, which introduced its own dedicated parser, allowing for expanded language features. This evolution enabled Version 3 to issue deprecation warnings and implement alternative error handling strategies that were not feasible under Apache's configuration system.

It's important to clarify that the differences between versions should not be viewed as problems or issues but rather design differences meant to enhance functionality. While there may be specific cases where our approach could be adjusted, maintaining these differences is crucial for leveraging improved outcomes for users furnther, including warnings and different lifecycle management that Apache configurations do not support.

Illustration -

In Version 2, the @ symbol's use in the Apache look-ahead parser restricted interpretations, forcing anything starting with @ to be matched as an operator or resulting in a failure. This limitation does not exist in Version 3, where @random, for example, is interpreted as a string as long as there is no matching operator. This leads to considerations such as:

• What is the better approach for the user?
  • In my view, the implementation in Version 3 is preferable, more flexible. Warning may be welcomed.
• Is it possible to break compatibility in this area and document the behavior?
  • My recommendation is to enhance our documentation to better explain these changes and their implications. IMHO, v2 and v3 are major versions, behavioral changes are expected.

[fzipi]

Welcome @zimmerle! Thanks for your insights!

[dune73]

I do not have strong feelings here. But I certainly prefer for different engines to behave the same way.

Issuing a warning for a couple of years and then removing the functionality seems to a good policy for me. What I never really liked is ModSec making assumptions silently when it's in fact a misconfiguration that a user should fix. And this is one such example.

[marcstern]

From a philosophical point of view, the best approach is usually the strictest and most orthogonal one. So, obliging the @rx would probably have been the best choice.

Because of the backward compatibility, we have to leave with this behavior, so let's look at the possibilities and potential problems:

1. risk of mistakes - I use @pn instead of @pm
2. risk of using something that will become a keyword at some time, like @bessn that someone could implement for Belgian social security number
3. limitation - I want to check against a string @random and I cannot because there's a syntax problem

With v3:

1. @pn x y z will be interpreted as a regex ⇒ problem
2. @bessn is interpreted as a regex ⇒ correct. Later, it'll incorrectly interpreted as an operator
3. no limitation

With v2:

1. @pn x y z is refused ⇒ I'll fix my syntax
2. @bessn is refused ⇒ I'll use @rx @bessn
3. no limitation, same as point 2

For me, v2 behavior is the best one. I see no disadvantage with it, it's safer.

[airween]

To everyone: thanks for participated in this issue.

First of all: I definitely didn't want to suggest that v2 or v3 behavior's is the good or bad. I just wanted to show the difference and the consequence. I personally think so that v3's behavior is closer to that is in the documentation - but honestly we have to agree it's a bit dangerous.

I like @marcstern's explanation: in case of v3 if user made a mistake that will be hidden, and the engine does not force the user to fix that.

I also agree that we can't remove this feature from the engines yet.

So my suggestion is to change v3's parser so that it doesn't allow @ as an operator argument without a valid operator in front of it.

With this step:

• we can make two engines more compatible
• typos won't be hidden
• we will keep the "default operator" feature

Thoughts?