search

coreruleset / coreruleset

OWASP CRS (Official Repository)
https://coreruleset.org
Apache License 2.0
2.11k stars 360 forks source link

Create RBL plugin #2501

Open lifeforms opened 2 years ago

lifeforms commented 2 years ago

Motivation

The reputation rules have been removed from the core. The RBL check is now gone.

Proposed solution

For the users of @rbl we can bring it back as a plugin.

studersi commented 2 years ago

FYI: I have begun working on this and will update this issue about the progress.

studersi commented 2 years ago

It looks like the responses to the RBL queries can differ quite a bit, some even requiring an API access key whereas others do not. Also, the rules would have to statically list the targeted API.

I think it would be best to have separate plugins for the different RBL API providers. What do you think?

The previous implementation in CRS only made use of dnsbl.httpbl.org so I would start whit a plugin specifically for this RBL.

dune73 commented 2 years ago

Can you make it configurable in the same plugin?

studersi commented 2 years ago

The problem is that for dnsbl.httpbl.org you get a response that is quite different than for other generic RBLs. There is additional threat data like a threat score and freshness information with this RBL that is not there for other RBLs.

The RBL zen.spamhaus.org responds with multiple responses though, I do not see any way to make use of the additional information in ModSecurity but the response differs from the generic response for other RBLs.

Here are a few examples where the actual values have been changed:

$ ./blackcheck.sh 123.123.123.123
IP 123.123.123.123 NAME localhost.
2022-04-28_09:13:32 xxxxxxxxxxxx.123.123.123.123.dnsbl.httpbl.org.     127.2.4.1
2022-04-28_09:13:32 123.123.123.123.cbl.abuseat.org.                   127.0.0.2
2022-04-28_09:13:32 123.123.123.123.dnsbl.sorbs.net.                   127.0.0.6
2022-04-28_09:13:32 123.123.123.123.bl.spamcop.net.                    127.0.0.2
2022-04-28_09:13:32 123.123.123.123.zen.spamhaus.org.                  127.0.0.4 127.0.0.3 127.0.0.11
RBL lookup of xxxxxxxxxxxxx.123.123.123.123.dnsbl.httpbl.org succeeded at TX:test_ip. Suspicious IP: 2 days since last activity, threat score 4
RBL lookup of 123.123.123.123.zen.spamhaus.org succeeded at TX:test_ip (Static UBE sources).
RBL lookup of 123.123.123.123.bl.spamcop.net succeeded at TX:test_ip.

There are also differences in behaviour when the IP is not in the blacklist. Generally, the lookup rule does not match but for dnshttpbl.org the rule does match and the message informs about the error, so we have to do additional checks:

RBL lookup of xxxxxxxxxxxxx.123.123.123.123.dnsbl.httpbl.org failed

Making this configurable would result in a fairly complex construct.

Then there is also another problem. We cannot make the target of the lookup configurable via variables. We would have to list all available RBLs and skip the undesired ones. On the other hand, this would allow you to have multiple RBLs active at the same time.

All in all I am not convinced it would be better to only have one plugin but certainly not set on separate plugins either.

studersi commented 2 years ago

Thinking about this some more, I think we could simply split the plugin into separate files but ship them together as a single plugin. @dune73 What do you think about this approach?

dune73 commented 2 years ago

I do not like the idea of separate plugins and I think we ought to explore the complexer combined option.

How about this construct:

# Lookup

SecRule TX:xxx-plugin-rbl-lookup-httpbl_enabled "@eq 1" \
   ...
   chain
   SecRule TX:REAL_IP "@rbl dnsbl.httpbl.org" \
       setvar:'tx.xxx-plugin-httpbl_msg=%{tx.1}'

SecRule TX:xxx-plugin-rbl-lookup-spamhaus_enabled "@eq 1" \
   ...
   chain
   SecRule TX:REAL_IP "@rbl zen.spamhaus.org"
       setvar:'tx.xxx-plugin-spamhaus_msg=%{tx.1}'
...

# Handling

SecRule TX:xxx-plugin-httpbl_msg "@rx ..."

...

SecRule TX:xxx-plugin-spamhaus_msg "@rx ..."

This way, only enabled resolvers are accessed. The response is written into a message variable and a separate set of rules will then examine said responses, each resolver's response covered with a separate rule or group of rules that are tailored to the specifics of the resolver.

studersi commented 2 years ago

To me this seems overly complicated especially, because we already have to use multiple chained to begin with in order to check if the lookup was successful and what it said about the IP: https://github.com/coreruleset/coreruleset/blob/v3.3.2/rules/REQUEST-910-IP-REPUTATION.conf#L161-L277

SecRule TX:REAL_IP "@rbl dnsbl.httpbl.org" \
    "id:910140,\
    phase:2,\
    pass,\
    capture,\
    t:none,\
    nolog,\
    tag:'application-multi',\
    tag:'language-multi',\
    tag:'platform-multi',\
    tag:'attack-reputation-ip',\
    tag:'paranoia-level/1',\
    tag:'OWASP_CRS',\
    ver:'OWASP_CRS/3.3.2',\
    setvar:'tx.httpbl_msg=%{tx.0}',\
    chain"
    SecRule TX:httpbl_msg "@rx RBL lookup of .*?.dnsbl.httpbl.org succeeded at TX:checkip. (.*?): .*" \
        "capture,\
        t:none,\
        setvar:'tx.httpbl_msg=%{tx.1}'"

# The following regexs are generated based off re_operators.c
SecRule TX:block_search_ip "@eq 1" \
    "id:910150,\
    phase:2,\
    block,\
    t:none,\
    msg:'HTTP Blacklist match for search engine IP',\
    tag:'application-multi',\
    tag:'language-multi',\
    tag:'platform-multi',\
    tag:'attack-reputation-ip',\
    tag:'paranoia-level/1',\
    tag:'OWASP_CRS',\
    ver:'OWASP_CRS/3.3.2',\
    severity:'CRITICAL',\
    chain,\
    skipAfter:END-RBL-CHECK"
    SecRule TX:httpbl_msg "@rx Search Engine" \
        "setvar:'tx.anomaly_score_pl1=+%{tx.critical_anomaly_score}',\
        setvar:'ip.reput_block_flag=1',\
        setvar:'ip.reput_block_reason=%{rule.msg}',\
        setvar:'ip.previous_rbl_check=1',\
        expirevar:'ip.reput_block_flag=%{tx.reput_block_duration}',\
        expirevar:'ip.previous_rbl_check=86400'"

[...]

This could probably be simplified a bit but it would likely remain a complicated construct. Adding more conditions would make this even more complicated.

dune73 commented 2 years ago

This would belong into the handling block in my proposal above. The handling block is apparently complicated, but that's why you hide it all into a separate plugin in CRS is not really affected.

And the handling would have to be specific for every RBL provider, so it's simply a question of putting it all in the same file / plugin or in separate files / plugins.

What I did not get is your above proposal of single-plugin, but with multiple files. Would this be this:

rbl-plugin-provider1-before.conf
rbl-plugin-provider2-before.conf
...

And then you can deploy those files that you like and they would be implicitly activated? I can see how this could work.

But please be aware that the plugin activation / deactivation has to be present in all of these files. So we would separate them nicely, but we would also have to do separate plugin activation in the files ...

github-actions[bot] commented 1 year ago

This issue has been open 120 days with no activity. Remove the stale label or comment, or this will be closed in 14 days

fzipi commented 3 months ago

@coreruleset/core-developers who wants to take this one?

studersi commented 3 months ago

Note that when I was working on this, I was having issues getting reliable results from the DNS lookups. It looked like ModSecurity was not doing the DNS lookups properly, misinterpreting the results, or having issues with error handling of some sort.

I am wondering now, if these issues were related to something like https://github.com/owasp-modsecurity/ModSecurity/issues/3111.

RedXanadu commented 3 months ago

We should consider sunsetting this, unless there is anyone with a strong desire to develop and then maintain this as a new plugin?

fzipi commented 3 months ago

That was the whole idea behind it, right? Moving it out from the core, but keep it as plugin.

RedXanadu commented 3 months ago

No, I mean sunsetting it for good. If there is no one who wants to a.) develop this and then b.) actively maintain such a plugin then we should sunset it.

It looks like it's been 2 years and nobody has stepped up, so interest in this functionality is limited.

If there's a group or integrator/sponsor who wants or needs this functionality then they could always either sponsor its development or, ideally, pick it up themselves.

Simply: if there is no one who will do the work then the work will not be done.

studersi commented 3 months ago

I'd actually be willing to implement this but we would have to get things straightened out in the engine first.

It might take a few weeks for me to get around to this but I can try to reproduce the issues within ModSec and file an issue over at https://github.com/owasp-modsecurity/ModSecurity.

airween commented 3 months ago

Just FYI: https://github.com/owasp-modsecurity/ModSecurity/issues/3131

studersi commented 3 months ago

Oh, I see, thanks for the info.

So, am I following correctly that there were three distinct problems or pitfalls with RBL lookups that we know of,

airween commented 3 months ago

So, am I following correctly that there were three distinct problems or pitfalls with RBL lookups that we know of,

yes, this is a knowing issue - the fix is not hard (I added the "good first issue" label)

this one is already merged (but not released yet)

  • the behaviour of the standard resovler to repeat the query with appended search domain (which can be prevented by appending a . in the ModSec @rbl rule to make the domain fully qualified)?

yes, I asked that question, the mentioned workaround is enough and place it to the documentation or should we fix that in the code? So I don't know is it a problem or not.