From 7526f5070694b9a0e05a16a172112edf1e5e3b1a Mon Sep 17 00:00:00 2001 From: sebres Date: Mon, 22 Jan 2018 19:08:26 +0100 Subject: [PATCH] amend to a54b401ee28117442a9fb02233d0cc3b54221322 (0.11-fix-get-current-bans): - differentiate between unknown ban-time after upgrade (-2) and persistent ban-time (-1); - better handling to correct current ban-time, uses `bantime.maxtime` (if increment allowed) or `bantime` of jail; - extended test-cases in order to cover the change of `bantime.maxtime`. --- fail2ban/server/database.py | 21 ++++++++++++++------- fail2ban/server/jail.py | 15 ++++++++++++--- fail2ban/tests/databasetestcase.py | 4 ++-- fail2ban/tests/observertestcase.py | 4 +++- 4 files changed, 31 insertions(+), 13 deletions(-) diff --git a/fail2ban/server/database.py b/fail2ban/server/database.py index ba1df573..bd66075f 100644 --- a/fail2ban/server/database.py +++ b/fail2ban/server/database.py @@ -378,9 +378,9 @@ class Fail2BanDb(object): "COMMIT;" % Fail2BanDb._CREATE_TABS['logs']) if version < 3 and self._tableExists(cur, "bans"): - # set ban-time to -1 (note it means rather unknown, as persistent, will be fixed by restore): + # set ban-time to -2 (note it means rather unknown, as persistent, will be fixed by restore): cur.executescript("BEGIN TRANSACTION;" - "CREATE TEMPORARY TABLE bans_temp AS SELECT jail, ip, timeofban, -1 as bantime, 1 as bancount, data FROM bans;" + "CREATE TEMPORARY TABLE bans_temp AS SELECT jail, ip, timeofban, -2 as bantime, 1 as bancount, data FROM bans;" "DROP TABLE bans;" "%s;\n" "INSERT INTO bans SELECT * from bans_temp;" @@ -747,7 +747,7 @@ class Fail2BanDb(object): if ip is not None: query += " AND ip=?" queryArgs.append(ip) - query += " AND (timeofban + bantime > ? OR bantime = -1)" + query += " AND (timeofban + bantime > ? OR bantime <= -1)" queryArgs.append(fromtime) if forbantime not in (None, -1): # not specified or persistent (all) query += " AND timeofban > ?" @@ -773,7 +773,9 @@ class Fail2BanDb(object): tickets = [] ticket = None if correctBanTime is True: - correctBanTime = jail.actions.getBanTime() if jail is not None else None + correctBanTime = jail.getMaxBanTime() if jail is not None else None + # don't change if persistent allowed: + if correctBanTime == -1: correctBanTime = None for ticket in self._getCurrentBans(cur, jail=jail, ip=ip, forbantime=forbantime, fromtime=fromtime @@ -781,12 +783,17 @@ class Fail2BanDb(object): # can produce unpack error (database may return sporadical wrong-empty row): try: banip, timeofban, bantime, bancount, data = ticket - # if persistent ban (or still unknown after upgrade), use current bantime of the jail: - if correctBanTime and (bantime == -1 or bantime > correctBanTime): - bantime = correctBanTime # additionally check for empty values: if banip is None or banip == "": # pragma: no cover raise ValueError('unexpected value %r' % (banip,)) + # if bantime unknown (after upgrade-db from earlier version), just use min known ban-time: + if bantime == -2: # todo: remove it in future version + bantime = jail.actions.getBanTime() if jail is not None else ( + correctBanTime if correctBanTime else 600) + elif correctBanTime: + # if persistent ban (or greater as max), use current max-bantime of the jail: + if bantime == -1 or bantime > correctBanTime: + bantime = correctBanTime except ValueError as e: # pragma: no cover logSys.debug("get current bans: ignore row %r - %s", ticket, e) continue diff --git a/fail2ban/server/jail.py b/fail2ban/server/jail.py index 7361fcce..3e65fdbb 100644 --- a/fail2ban/server/jail.py +++ b/fail2ban/server/jail.py @@ -263,14 +263,23 @@ class Jail(object): return self._banExtra.get(opt, None) return self._banExtra + def getMaxBanTime(self): + """Returns max possible ban-time of jail. + """ + return self._banExtra.get("maxtime", -1) \ + if self._banExtra.get('increment') else self.actions.getBanTime() + def restoreCurrentBans(self, correctBanTime=True): """Restore any previous valid bans from the database. """ try: if self.database is not None: - forbantime = None; - # use ban time as search time if we have not enabled a increasing: - if not self.getBanTimeExtra('increment'): + if self._banExtra.get('increment'): + forbantime = None; + if correctBanTime: + correctBanTime = self.getMaxBanTime() + else: + # use ban time as search time if we have not enabled a increasing: forbantime = self.actions.getBanTime() for ticket in self.database.getCurrentBans(jail=self, forbantime=forbantime, correctBanTime=correctBanTime diff --git a/fail2ban/tests/databasetestcase.py b/fail2ban/tests/databasetestcase.py index 8827df52..46c1d8e0 100644 --- a/fail2ban/tests/databasetestcase.py +++ b/fail2ban/tests/databasetestcase.py @@ -170,9 +170,9 @@ class DatabaseTest(LogCaptureTestCase): self.assertEqual(self.db.updateDb(Fail2BanDb.__version__), Fail2BanDb.__version__) self.assertRaises(NotImplementedError, self.db.updateDb, Fail2BanDb.__version__ + 1) # check current bans (should find exactly 1 ticket after upgrade): - tickets = self.db.getCurrentBans(fromtime=1388009242, correctBanTime=False) + tickets = self.db.getCurrentBans(fromtime=1388009242, correctBanTime=123456) self.assertEqual(len(tickets), 1) - self.assertEqual(tickets[0].getBanTime(), -1); # ban-time still unknown (normally updated from jail) + self.assertEqual(tickets[0].getBanTime(), 123456); # ban-time was unknown (normally updated from jail) finally: if self.db and self.db._dbFilename != ":memory:": os.remove(self.db._dbBackupFilename) diff --git a/fail2ban/tests/observertestcase.py b/fail2ban/tests/observertestcase.py index 28881c5d..f0016f1e 100644 --- a/fail2ban/tests/observertestcase.py +++ b/fail2ban/tests/observertestcase.py @@ -386,6 +386,7 @@ class BanTimeIncrDB(unittest.TestCase): # two separate jails : jail1 = DummyJail(backend='polling') + jail1.setBanTimeExtra('increment', 'true') jail1.database = self.db self.db.addJail(jail1) jail2 = DummyJail(name='DummyJail-2', backend='polling') @@ -433,7 +434,8 @@ class BanTimeIncrDB(unittest.TestCase): # jail2 does not restore any bans (because all ban tickets should be already expired: stime-6000): jail2.restoreCurrentBans(correctBanTime=False) self.assertEqual(jail2.getFailTicket(), False) - # test again, but now normally (with maximum ban-time of restored ticket allowed): + # test again, but now normally (with maximum ban-time of restored ticket = allowed 10m = 600): + jail1.setBanTimeExtra('maxtime', '10m') jail1.restoreCurrentBans() ticket = jail1.getFailTicket() self.assertTrue(ticket.restored)