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

BUG: fail2ban-client set JAIL addignoreip should be unique #2680

Closed
iosoft opened this issue Apr 5, 2020 · 8 comments
Closed

BUG: fail2ban-client set JAIL addignoreip should be unique #2680

iosoft opened this issue Apr 5, 2020 · 8 comments

Comments

@iosoft
Copy link

iosoft commented Apr 5, 2020

Dear Team,

I was writing a custom shell-script that uses this command to add ignoreip for a jail.
fail2ban-client set my-filter addignoreip 1.1.1.1

While testing, I realized that every time, I run the command, there this multiple/duplicate IP (1.1.1.1) in the list!
The IP list should be unique!

THE REAL PROBLEM:
Since there is duplicate IP in the list, I need to execute the delignoreip command multiple times to actually remove the IP from the ignoreip list!

fail2ban-client version: 0.9.6

@sebres
Copy link
Contributor

sebres commented Apr 6, 2020

The IP list should be unique!

Why? Or rather how it should be a "BUG" of fail2ban?
This parameters was introduced to control "own" IP addresses to avoid false positives on mistakenly recognized failures from definitely legitimate hosts or subnets (small list like localhost/8 etc).
Usage of this list for something else is ugly (simply not suitable for a large set of IPs)...
Either you have to take care of the service generating failures (that should not or not often generate failures for legitimate users/hosts), or you have to optimize failregex to exclude false positives, or if it is really impossible in the other way round, use ignorecommand.

I don't see why we should implement this on fail2ban side. especially don't see that as a bug (let alone I don't like if someone trying to relocate the own needs on our shoulders).
Of course we could rewrite the list as a set (that would be unique per definition), but since the list could contain a subnets too, fail2ban needs to handle that as a iterable list, so it is not a large win by current implementation.
I could take a look later whether it would be possible somehow (but the priority of this task is extremely low).

Since there is duplicate IP in the list, I need to execute the delignoreip command multiple times to actually remove the IP from the ignoreip list!

Well, you can do the same pre-filtering that you wanted we have to do on fail2ban side. I don't see large troubles to do that on client side (validate it against your previous list or the list returned by get <jail> ignoreip, append new to variable and then send it via single command).

Or as already said just you use ignorecommand instead of ignoreip (so you don't need to add ips at all). Also note #2176 (since v. 0.10).

@sebres
Copy link
Contributor

sebres commented Apr 6, 2020

BTW, #1904 solves exactly this (so it is already "fixed" in v.0.10)

@sebres
Copy link
Contributor

sebres commented Apr 6, 2020

not suitable for a large set of IPs

I've extended the ignoreip logic now (fc175fa), so this is no longer true: simplest case (handling of single IPs) is reimplemented as a set, so such huge list is performant enough now (mostly O(1)) and can be considered for regular usage.
The subnet or DNS lookups remain still iterative (checking every single entry individually), so still O(n) in about, but this should not be too large and the lookup is cacheable (#2176).

@iosoft
Copy link
Author

iosoft commented Apr 9, 2020

BTW, #1904 solves exactly this (so it is already "fixed" in v.0.10)

If it is not "BUG", why "fixed"!

@iosoft
Copy link
Author

iosoft commented Apr 9, 2020

Let me tell you why I needed this 'update' -

My webserver do not have Static IP.
Whenever my server is restarted, it may get the same IP or a new IP.
So, I am running a script to Whitelist my own dynamic IP into the Fail2Ban list.

So, this IP list should be unique.
Just my suggestion. No need to be so RUDE....

@sebres
Copy link
Contributor

sebres commented Apr 14, 2020

Whenever my server is restarted, it may get the same IP or a new IP.
So, this IP list should be unique.

As already said it is not a large problem to check whether the IP is already in list (compare it against current list).

So, I am running a script to Whitelist my own dynamic IP into the Fail2Ban list.

If you have some (dynamic) DNS entry mapped to this IP you can add this host name to ignoreip, so you should not update it manually every time it gets changed.

@iosoft
Copy link
Author

iosoft commented Jun 15, 2020

If you have some (dynamic) DNS entry mapped to this IP you can add this host name to ignoreip, so you should not update it manually every time it gets changed.

Is there any command to add the new IP and set it as ignoreip?
What will be the command to use with fail2ban-client?

@sebres
Copy link
Contributor

sebres commented Jun 15, 2020

I don't understand the question.
You have provided an example for fail2ban-client ... addignoreip in the issue description.

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

No branches or pull requests

2 participants