From cafea3235f46a92af42c135ecf720084095e891d Mon Sep 17 00:00:00 2001 From: Marko Hauptvogel Date: Mon, 10 Nov 2025 14:29:58 +0100 Subject: [PATCH 1/6] test Ticket.getIP() return value type This covers the original problem where getIP() would return a str when it should be an IPAddr. One testcase fails now. --- fail2ban/tests/tickettestcase.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fail2ban/tests/tickettestcase.py b/fail2ban/tests/tickettestcase.py index 6b250548..c13e9e69 100644 --- a/fail2ban/tests/tickettestcase.py +++ b/fail2ban/tests/tickettestcase.py @@ -26,6 +26,7 @@ from ..server.mytime import MyTime import unittest from ..server.ticket import Ticket, FailTicket, BanTicket +from ..server.ipdns import IPAddr class TicketTests(unittest.TestCase): @@ -40,6 +41,7 @@ class TicketTests(unittest.TestCase): # Ticket t = Ticket('193.168.0.128', tm, matches) self.assertEqual(t.getID(), '193.168.0.128') + self.assertIsInstance(t.getIP(), IPAddr) self.assertEqual(t.getIP(), '193.168.0.128') self.assertEqual(t.getTime(), tm) self.assertEqual(t.getMatches(), matches2) @@ -67,6 +69,7 @@ class TicketTests(unittest.TestCase): ft = FailTicket('193.168.0.128', tm, matches) ft.setBanTime(60*60) self.assertEqual(ft.getID(), '193.168.0.128') + self.assertIsInstance(t.getIP(), IPAddr) self.assertEqual(ft.getIP(), '193.168.0.128') self.assertEqual(ft.getTime(), tm) self.assertEqual(ft.getMatches(), matches2) @@ -110,6 +113,7 @@ class TicketTests(unittest.TestCase): # copy all from another ticket: ft2 = FailTicket(ticket=ft) self.assertEqual(ft, ft2) + self.assertEqual(ft.getIP(), ft2.getIP()) self.assertEqual(ft.getData(), ft2.getData()) self.assertEqual(ft2.getAttempt(), 4) self.assertEqual(ft2.getRetry(), 7) @@ -123,10 +127,12 @@ class TicketTests(unittest.TestCase): # different ID (string) and IP: t = Ticket('123-456-678', tm, data={'ip':'192.0.2.1'}) self.assertEqual(t.getID(), '123-456-678') + self.assertIsInstance(t.getIP(), IPAddr) self.assertEqual(t.getIP(), '192.0.2.1') # different ID (tuple) and IP: t = Ticket(('192.0.2.1', '5000'), tm, data={'ip':'192.0.2.1'}) self.assertEqual(t.getID(), ('192.0.2.1', '5000')) + self.assertIsInstance(t.getIP(), IPAddr) self.assertEqual(t.getIP(), '192.0.2.1') def testTicketFlags(self): From 5663e9e02927ad7318dcd0bb64e63ffaef5629ed Mon Sep 17 00:00:00 2001 From: Marko Hauptvogel Date: Tue, 11 Nov 2025 10:54:08 +0100 Subject: [PATCH 2/6] make Ticket.getIP() return IPAddr Store the ip internally as IPAddr, converting the data if needed. The original data can bei either a str or IPAddr, anything else (like an int) will cause an error. Fall back to the id only if it is an IPAddr. All testcases pass again. --- ChangeLog | 1 + fail2ban/server/ticket.py | 17 ++++++++++++----- fail2ban/tests/tickettestcase.py | 9 +++++++++ 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index d6588117..bac9312b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -28,6 +28,7 @@ ver. 1.1.1-dev-1 (20??/??/??) - development nightly edition e. g. setting `blocktype="DROP"` via jail for action would now apply for IPv4 and IPv6 chains, to submit different `blocktype` for IPv4 and IPv6 from jail, one can pass them like in this example: `banaction = iptables-ipset[blocktype="...", blocktype?family=inet6="..."]` +* fixes restoring bans with custom failure-id * `jail.conf`: - default banactions need to be specified in `paths-*.conf` (maintainer level) now - since stock fail2ban includes `paths-debian.conf` by default, banactions are `nftables` diff --git a/fail2ban/server/ticket.py b/fail2ban/server/ticket.py index 72573ec4..f25ea509 100644 --- a/fail2ban/server/ticket.py +++ b/fail2ban/server/ticket.py @@ -25,7 +25,7 @@ __copyright__ = "Copyright (c) 2004 Cyril Jaquier" __license__ = "GPL" from ..helpers import getLogger -from .ipdns import IPAddr +from .ipdns import IPAddr, asip from .mytime import MyTime # Gets the instance of the logger. @@ -56,8 +56,11 @@ class Ticket(object): self._data = {'matches': matches or [], 'failures': 0} if data is not None: for k,v in data.items(): - if v is not None: - self._data[k] = v + if v is None: + continue + if k == 'ip': + v = asip(v) + self._data[k] = v if ticket: # ticket available - copy whole information from ticket: self.update(ticket) @@ -95,8 +98,12 @@ class Ticket(object): def getID(self): return self._id - def getIP(self): - return self._data.get('ip', self._id) + def getIP(self) -> IPAddr: + if 'ip' in self._data: + return self._data['ip'] + if isinstance(self._id, IPAddr): + return self._id + raise ValueError("No IP available") def setTime(self, value): self._time = value diff --git a/fail2ban/tests/tickettestcase.py b/fail2ban/tests/tickettestcase.py index c13e9e69..d2b6ef68 100644 --- a/fail2ban/tests/tickettestcase.py +++ b/fail2ban/tests/tickettestcase.py @@ -135,6 +135,15 @@ class TicketTests(unittest.TestCase): self.assertIsInstance(t.getIP(), IPAddr) self.assertEqual(t.getIP(), '192.0.2.1') + # invalid ip type causes an error + with self.assertRaises(TypeError): + Ticket('123-456-789', tm, data={'ip':192021}) + + # no IPAddr causes an error + t = Ticket(('192.0.2.1', '5000'), tm, data={}) + with self.assertRaises(ValueError): + t.getIP() + def testTicketFlags(self): flags = ('restored', 'banned') ticket = Ticket('test', 0) From 92176d729cedbd4b8bcfcf21787bad577d4e0f53 Mon Sep 17 00:00:00 2001 From: Marko Hauptvogel Date: Tue, 11 Nov 2025 10:54:08 +0100 Subject: [PATCH 3/6] test Ticket.getID() return value type Two testcases fail now. --- fail2ban/tests/tickettestcase.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/fail2ban/tests/tickettestcase.py b/fail2ban/tests/tickettestcase.py index d2b6ef68..3a51a547 100644 --- a/fail2ban/tests/tickettestcase.py +++ b/fail2ban/tests/tickettestcase.py @@ -40,6 +40,7 @@ class TicketTests(unittest.TestCase): # Ticket t = Ticket('193.168.0.128', tm, matches) + self.assertIsInstance(t.getID(), str) self.assertEqual(t.getID(), '193.168.0.128') self.assertIsInstance(t.getIP(), IPAddr) self.assertEqual(t.getIP(), '193.168.0.128') @@ -63,11 +64,16 @@ class TicketTests(unittest.TestCase): self.assertFalse(t.isTimedOut(tm + 60 + 1)) t.setBanTime(60) + # wrong id type causes error + with self.assertRaises(TypeError): + Ticket(id=123) + # BanTicket tm = MyTime.time() matches = ['first', 'second'] ft = FailTicket('193.168.0.128', tm, matches) ft.setBanTime(60*60) + self.assertIsInstance(t.getID(), str) self.assertEqual(ft.getID(), '193.168.0.128') self.assertIsInstance(t.getIP(), IPAddr) self.assertEqual(ft.getIP(), '193.168.0.128') @@ -113,6 +119,7 @@ class TicketTests(unittest.TestCase): # copy all from another ticket: ft2 = FailTicket(ticket=ft) self.assertEqual(ft, ft2) + self.assertEqual(ft.getID(), ft2.getID()) self.assertEqual(ft.getIP(), ft2.getIP()) self.assertEqual(ft.getData(), ft2.getData()) self.assertEqual(ft2.getAttempt(), 4) @@ -126,11 +133,15 @@ class TicketTests(unittest.TestCase): tm = MyTime.time() # different ID (string) and IP: t = Ticket('123-456-678', tm, data={'ip':'192.0.2.1'}) + self.assertIsInstance(t.getID(), str) self.assertEqual(t.getID(), '123-456-678') self.assertIsInstance(t.getIP(), IPAddr) self.assertEqual(t.getIP(), '192.0.2.1') # different ID (tuple) and IP: t = Ticket(('192.0.2.1', '5000'), tm, data={'ip':'192.0.2.1'}) + self.assertIsInstance(t.getID(), tuple) + self.assertIsInstance(t.getID()[0], str) + self.assertIsInstance(t.getID()[1], str) self.assertEqual(t.getID(), ('192.0.2.1', '5000')) self.assertIsInstance(t.getIP(), IPAddr) self.assertEqual(t.getIP(), '192.0.2.1') From 6eb17a5602005900975c925dce7329a4ac322d3b Mon Sep 17 00:00:00 2001 From: Marko Hauptvogel Date: Tue, 11 Nov 2025 10:54:08 +0100 Subject: [PATCH 4/6] make Ticket.getID() return str If the original id is a str, it's internally stored as IPAddr. Instead of returning the IPAddr, the getter now only returns a plain str. If it originally was a tuple or None, that is passed as-is. Also now throws an error if the id has an unexpected type. One testcase now fails. --- fail2ban/server/ticket.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/fail2ban/server/ticket.py b/fail2ban/server/ticket.py index f25ea509..787f4e84 100644 --- a/fail2ban/server/ticket.py +++ b/fail2ban/server/ticket.py @@ -24,6 +24,8 @@ __author__ = "Cyril Jaquier" __copyright__ = "Copyright (c) 2004 Cyril Jaquier" __license__ = "GPL" +from typing import Union + from ..helpers import getLogger from .ipdns import IPAddr, asip from .mytime import MyTime @@ -89,13 +91,17 @@ class Ticket(object): if v is not None: setattr(self, n, v) - def setID(self, value): + def setID(self, value: Union[str, IPAddr, tuple, None]) -> None: # guarantee using IPAddr instead of unicode, str for the IP if isinstance(value, str): value = IPAddr(value) + if not isinstance(value, (IPAddr, tuple, type(None))): + raise TypeError(f"ID has unsupported type: {type(value).__name__}") self._id = value - def getID(self): + def getID(self) -> Union[IPAddr, tuple, None]: + if isinstance(self._id, IPAddr): + return str(self._id) return self._id def getIP(self) -> IPAddr: From fffa65c2493e75d763aa38b944cbe8e1953cf767 Mon Sep 17 00:00:00 2001 From: Marko Hauptvogel Date: Tue, 11 Nov 2025 10:54:08 +0100 Subject: [PATCH 5/6] fix jail problem When checking against the ignoreip list, you obviously have to use the ip and not id. One testcase still fails though. --- fail2ban/server/jail.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fail2ban/server/jail.py b/fail2ban/server/jail.py index 7e52a01a..57ed83ee 100644 --- a/fail2ban/server/jail.py +++ b/fail2ban/server/jail.py @@ -297,7 +297,7 @@ class Jail(object): # mark ticked was restored from database - does not put it again into db: ticket.restored = True #logSys.debug('restored ticket: %s', ticket) - if self.filter._inIgnoreIPList(ticket.getID(), ticket): continue + if self.filter._inIgnoreIPList(ticket.getIP(), ticket): continue # correct start time / ban time (by the same end of ban): btm = ticket.getBanTime(forbantime) diftm = MyTime.time() - ticket.getTime() From be6885d82d703aa147b43b371d55392c948378c7 Mon Sep 17 00:00:00 2001 From: Marko Hauptvogel Date: Tue, 11 Nov 2025 10:54:08 +0100 Subject: [PATCH 6/6] fix actions problem Give the BanManager a new method returning a list of banned ips, which can be used to check if a specific ip is currently banned. All testcases pass again. --- fail2ban/server/actions.py | 2 +- fail2ban/server/banmanager.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/fail2ban/server/actions.py b/fail2ban/server/actions.py index 581c5724..8e3bb50e 100644 --- a/fail2ban/server/actions.py +++ b/fail2ban/server/actions.py @@ -288,7 +288,7 @@ class Actions(JailThread, Mapping): if not isinstance(ip, IPAddr): ipa = IPAddr(ip) if not ipa.isSingle: # subnet (mask/cidr) or raw (may be dns/hostname): - ips = list(filter(ipa.contains, self.banManager.getBanList())) + ips = list(filter(ipa.contains, self.banManager.getBannedIPs())) if ips: return self.removeBannedIP(ips, db, ifexists) # not found: diff --git a/fail2ban/server/banmanager.py b/fail2ban/server/banmanager.py index d3e89820..bb9f729f 100644 --- a/fail2ban/server/banmanager.py +++ b/fail2ban/server/banmanager.py @@ -115,6 +115,9 @@ class BanManager: ) for t in lst] return [t[0].getID() for t in lst] + def getBannedIPs(self): + return list(sorted(ticket.getIP() for ticket in self.__banList.values())) + ## # Returns a iterator to ban list (used in reload, so idle). #