Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GeoIP ignore support #3467

Open
wants to merge 30 commits into
base: master
Choose a base branch
from
Open

Conversation

Wicwik
Copy link

@Wicwik Wicwik commented Feb 23, 2023

Before submitting your PR, please review the following checklist:

  • CHOOSE CORRECT BRANCH: if filing a bugfix/enhancement
    against certain release version, choose 0.9, 0.10 or 0.11 branch,
    for dev-edition use master branch
  • CONSIDER adding a unit test if your PR resolves an issue
  • LIST ISSUES this PR resolves
  • MAKE SURE this PR doesn't break existing tests
  • KEEP PR small so it could be easily reviewed.
  • AVOID making unnecessary stylistic changes in unrelated code
  • ACCOMPANY each new failregex for filter X with sample log lines
    within fail2ban/tests/files/logs/X file

Added geoip lookup in filter in method _inIgnoreIPList, which checks the countries that should be ignored in __ignoreGeoSet attribute. The __ignoreGeoSet is managed either by ignoregeo statement in jail config or by addignoregeo/delignoregeo commands. If the IP has geolocation country code from the __ignoreGeoSet the IP is ignored.

This PR was inspired by our implementation in websupport.sk that was just hardcoded. It was not sustainable to patch f2b every time a new version was available and it may be a generally good idea to have a geo IP ignore option available.

This will resolve #1854.

@sebres
Copy link
Contributor

sebres commented Feb 23, 2023

Thx for the PR!
I'll try to review it soon and/or merge it with my experimental branch using different approach changing ban factor related to geo-database (I used sqlite rtree with geolocation data instead of geoiplookup), possibly one could combine both approaches,

@Wicwik
Copy link
Author

Wicwik commented Feb 23, 2023

Thanks. I have added also maxminds mmdblookup, as I have recently found out that the geoiplookup may have outdated dbs? ref1 ref2

Either way, currently it should be working with geoiplookup and mmdblookup with file /usr/share/GeoIP/GeoIP2-Country.mmdb. Maybe we can add geoipdb file option to jail configuration?

And sorry for the commit hell, made a typo and accidentally pushed the converted files by 2to3 😅.

Using sqlite with rtree is also an option. It could be better to be more general to allow any db. After this PR, we can probably continue with that.

valentinbreiz added a commit to Honeybrain/Honeypot that referenced this pull request Jan 3, 2024
@Neustradamus
Copy link

@sebres: Any news about this @Wicwik PR?

@sebres
Copy link
Contributor

sebres commented Jan 17, 2024

@Neustradamus please stop to ask on every PR "any news about"...
What're you trying to achieve?
It looks simply like a flood, and especially my GH notifications area stuffers from that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No GeoIP support?
3 participants