coreruleset / coreruleset

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

Create GeoIP plugin #2500

Open lifeforms opened 2 years ago

lifeforms commented 2 years ago

Motivation

The reputation rules have been removed from the core.

Proposed solution

Create an optional plugin for GeoIP users.

azurit commented 2 years ago

What is this plugin supposed to do?

airween commented 2 years ago

What is this plugin supposed to do?

I think the goal is to replace the "old" REQUEST-910-IP-REPUTATION.conf, which is no longer exists.

dune73 commented 2 years ago

Exactly.

Recent experience with using ASN information in the assessment of false positives makes me think that full maxmind DB support could be beneficial, thus going beyond the original ModSecurity GeoIP Country feature.

studersi commented 2 years ago

Recent experience with using ASN information in the assessment of false positives makes me think that full maxmind DB support could be beneficial, thus going beyond the original ModSecurity GeoIP Country feature.

+1

studersi commented 2 years ago

Contrary to the previous implementation, I think it would be better to increase the score depending on certain blacklists and stick to the normal CRS blocking evaluation.


Properties to consider could be the following:


Full MaxMind support could be complicated depending on the server technology.

For Apache httpd, it should be fairly straight-forward. You specify the database file, the value you want to look up based on IP, and the environment variable to store the looked-up value in. mod_maxminddb

For nginx it should be simple as well for certain properties, as far as I can tell, but I have not worked with the MaxMind module for nginx. The module appears to be less flexible though. The names of the environment variables appear to be hard-coded and not all properties and database types are listed. ngx_http_geoip_module

For Caddy there could be even more problems. It appears that the Caddy module does not expose the MaxMind environment variables as the other modules do but simply provides directives to restrict access by these values. caddy-maxmind-geolocation


As a side-note with respects to SecGeoLookupDb, the database format for Caddy & Coraza is different than for ModSecurity:

However, I would not expect this to be a problem in practice.

dune73 commented 2 years ago

I wonder whether SecGeoLookupDb is the way to go here. At least on Apache, the MaxMind module is more flexible, helps to work around ModSec 2.9 not supporting the newer format etc. The Apache CRS plugin could then inspect env variables.

RedXanadu commented 2 years ago

Or, if you have some other proxy sitting in front of ModSecurity to provide HA (e.g. HAProxy, etc.), add the geolocation info as request headers at the proxy, evaluate them in ModSec if needed, and the back end servers can record the geo data in the logs, too. Lots of flexibility, more so than using SecGeoLookupDb.

dune73 commented 2 years ago

Agreed. I think there are many options to get the information into ModSec and the best is probably to provide rules to work on env variables. But I seriously doubt this works in nginx.

azurit commented 2 years ago

What about to create really GeoIP only plugin which then can be used by other plugins, like IP reputation?

For flexible GeoIP implementation (supporting both ModSec build-in GeoIP and also external sources [for example headers or enviromental vars]), see my False Positive Plugin, rules 9599020 and 9599110: https://github.com/azurit/modsecurity-false-positive-report-plugin/blob/9c9eec2881f486d970dc94d2ada859bfeb5e6b29/plugins/false-positive-report-config.conf#L60 https://github.com/azurit/modsecurity-false-positive-report-plugin/blob/9c9eec2881f486d970dc94d2ada859bfeb5e6b29/plugins/false-positive-report-after.conf#L27

I can split this into separate plugin.

azurit commented 1 year ago

https://github.com/azurit/modsecurity-geoip-plugin

fzipi commented 6 months ago

@azurit Is this ready to be moved to the crs repository? What is needed here?

azurit commented 6 months ago

@fzipi I was talking with @dune73 about this and he thinks the plugin should be able to at least block requests based on GeoIP data, which it's currently not. My idea is that GeoIP plugin is only adding a unified way how to integrate and use GeoIP in CRS and it's going to be used by other plugins (or custom user rules) - and that 'other plugins' will do blocking and other things. This needs to be discussed.

fzipi commented 6 months ago

Excellent, thanks for documenting this. Do you want to add it in the next meeting agenda?

azurit commented 6 months ago

@fzipi Done.