From bbcbefd494a7120c4c8ac5341e859ae318d7a22f Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Tue, 22 Apr 2014 19:17:25 +0100 Subject: [PATCH] BF: bantime < 0 database should return all bans, as they are persistent --- ChangeLog | 1 + fail2ban/server/database.py | 13 ++++++++----- fail2ban/tests/databasetestcase.py | 13 +++++++++++-- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index bb1fd707..e8626930 100644 --- a/ChangeLog +++ b/ChangeLog @@ -20,6 +20,7 @@ ver. 0.9.1 (2014/xx/xx) - better, faster, stronger * Handle case when no sqlite library is available for persistent database * Only reban once per IP from database on fail2ban restart * Nginx filter to support missing server_name. Closes gh-676 + * Database now returns persistent bans on restart (bantime < 0) - New features: diff --git a/fail2ban/server/database.py b/fail2ban/server/database.py index 93186222..54cca4d3 100644 --- a/fail2ban/server/database.py +++ b/fail2ban/server/database.py @@ -380,7 +380,7 @@ class Fail2BanDb(object): if jail is not None: query += " AND jail=?" queryArgs.append(jail.name) - if bantime is not None: + if bantime is not None and bantime >= 0: query += " AND timeofban > ?" queryArgs.append(MyTime.time() - bantime) if ip is not None: @@ -399,7 +399,8 @@ class Fail2BanDb(object): Jail that the ban belongs to. Default `None`; all jails. bantime : int Ban time in seconds, such that bans returned would still be - valid now. Default `None`; no limit. + valid now. Negative values are equivalent to `None`. + Default `None`; no limit. ip : str IP Address to filter bans by. Default `None`; all IPs. @@ -427,7 +428,8 @@ class Fail2BanDb(object): Jail that the ban belongs to. Default `None`; all jails. bantime : int Ban time in seconds, such that bans returned would still be - valid now. Default `None`; no limit. + valid now. Negative values are equivalent to `None`. + Default `None`; no limit. ip : str IP Address to filter bans by. Default `None`; all IPs. @@ -438,7 +440,8 @@ class Fail2BanDb(object): in a list. When `ip` argument passed, a single `Ticket` is returned. """ - if bantime is None: + cacheKey = None + if bantime is None or bantime < 0: cacheKey = (ip, jail) if cacheKey in self._bansMergedCache: return self._bansMergedCache[cacheKey] @@ -468,7 +471,7 @@ class Fail2BanDb(object): ticket.setAttempt(failures) tickets.append(ticket) - if bantime is None: + if cacheKey: self._bansMergedCache[cacheKey] = tickets if ip is None else ticket return tickets if ip is None else ticket diff --git a/fail2ban/tests/databasetestcase.py b/fail2ban/tests/databasetestcase.py index 84101c50..2cf8577e 100644 --- a/fail2ban/tests/databasetestcase.py +++ b/fail2ban/tests/databasetestcase.py @@ -177,10 +177,15 @@ class DatabaseTest(unittest.TestCase): if Fail2BanDb is None: # pragma: no cover return self.testAddJail() - ticket = FailTicket("127.0.0.1", MyTime.time() - 40, ["abc\n"]) - self.db.addBan(self.jail, ticket) + self.db.addBan( + self.jail, FailTicket("127.0.0.1", MyTime.time() - 60, ["abc\n"])) + self.db.addBan( + self.jail, FailTicket("127.0.0.1", MyTime.time() - 40, ["abc\n"])) self.assertEqual(len(self.db.getBans(jail=self.jail,bantime=50)), 1) self.assertEqual(len(self.db.getBans(jail=self.jail,bantime=20)), 0) + # Negative values are for persistent bans, and such all bans should + # be returned + self.assertEqual(len(self.db.getBans(jail=self.jail,bantime=-1)), 2) def testGetBansMerged(self): if Fail2BanDb is None: # pragma: no cover @@ -251,6 +256,10 @@ class DatabaseTest(unittest.TestCase): self.assertEqual(len(tickets), 1) tickets = self.db.getBansMerged(bantime=5) self.assertEqual(len(tickets), 0) + # Negative values are for persistent bans, and such all bans should + # be returned + tickets = self.db.getBansMerged(bantime=-1) + self.assertEqual(len(tickets), 2) def testPurge(self): if Fail2BanDb is None: # pragma: no cover