Feature: Pm custom separator

[M4tteoP]

Summary

Hi folks, I'm opening this issue in order to extend the conversation about providing a custom separator for the @pm operator.
It is an open feature request from such a long time inside the Modsecurity repo (see owasp-modsecurity/ModSecurity#682) and Wasm is yet another player that would take advantage of such a feature.

Basic example

I made a first PoC under ModSec repo: owasp-modsecurity/ModSecurity#2786.
This first proposal takes advantage of a fixed string at the beginning of the rule (PmCustomSeparator:) in order to provide and figure out that we are going to use a custom separator.
An overall example of rule syntax is the following:

SecRule REQUEST_BODY "@Pm PmCustomSeparator:| single_word|<this> <is> <a> <string>|trailing_space " "id:999,phase:2,t:lowercase,deny"

Motivation

• Make the @pm operator more flexible and able to handle some cases that currently can only be managed by PmFromFile.
• Wasm environment that is lacking file system support would benefit from it.
• Depending on the design agreed, it may lead also to looking for piped payloads (currently, to the best of my knowledge, it is not possible to match the pipe | character and do not interpret it as Suricata syntax)

Extra details

• Even a different operator may be a way to go in order to avoid tweaking a known one and not messing with its default design.
• If we agree on the usefulness of that feature and on a proper design, I would be happy to implement it on Coraza.

Thanks for any feedback!

cc: @jcchavezs

[jptosso]

Hey @M4tteoP, thank you for your interest in Coraza. CRS has solved this problem by using text wrappers like |, but what you are talking about WASM is right, and it should concern us.
I have been thinking about a different solution for a while, creating seclang datasets like:

datasets.conf

SecDataset malicious_browsers `
hacker_browser
# this is a comment
hacker_browser_2
`
# or maybe
SecDataset malicious_browsers \
  "hacker_browser", \
  "hacker_browser_2" 

rules.conf

SecRule REQUEST_HEADERS:user-agent "@pmfromdict malicious_browsers" "..."
# or something like :
SecRule REQUEST_HEADERS:user-agent "@pm %{dict_to_pm malicious_browsers}" "..."

This would be consistent with the idea of helpers and code snippets for future versions, but your solution would be easier to implement and understand.
Looking forward to your feedback.

[M4tteoP]

Hi @jptosso, thanks a lot for the prompt reply!
I feel that SecDataset solution mimics in a great way the PmFromFile logic. Thinking about supporting as many characters as possible I would prefer your first example:

SecDataset malicious_browsers `
hacker_browser
# this is a comment
hacker_browser_2
`

As far as I can see, it could permit to look also for % and " characters.

Summarizing:
SecDataset solution would be:

• harder to implement, maybe a bit less intuitive (but something like PmFromDataset should easily remind a similar behaviour to PmFromFile)
• a bit more complex. A simple match like "a b c" would still require writing both the SecDataset and the rule.
• With more characters supported.
• Working fine in a Wasm env just like PmCustomSeparator.

A couple of questions:

1. What about The Snort-Suricata syntax, would you just mimic PmFromFile leaving that syntax only for the @pm directive?
2. What about the implementation? Do you feel that it could have to deal with big problems in Coraza?

As a side note, I'm dreaming about an implementation that may also land on ModSecurity, but there it may be way more tricky...

[jptosso]

1. Snort-Suricata is not supported, but we haven't received any request or issue. I have monitored this repo in case we want to implement Suricata syntax, and we should be completely open to implementing it as long as it is compatible with WASM. Feel free to create an issue if you want.
2. The only major change for the seclang processor would be the ``` wrappers, as we haven't touched the lex processor in more than a year, but we can handle it.

I will open this discussion for the August Monthly Meeting

[M4tteoP]

Just a note that may add another element to the discussion: the same concept may be applied to ipMatch/ipMatchFromFile operators, having something like SecDataset with the list of IPs and an ipMatchFromDict operator. Here I can see that it would just be about readability instead of relying on a long list of inlined IPs.

[jptosso]

Could we mark this as solved with the new SecDataset operator? #361

[fzipi]

@M4tteoP ☝️ ?

[M4tteoP]

Sorry to have kept you waiting, but #393 gave me pause wondering about overlaps in functionalities with SecDataset. In the end, I still see value in SecDataset (e.g. easy way to inline PmFromFile-like rules avoiding the need to recompile the Wasm plugin with the new rules embedded.)
I close it, thank you!