diff --git a/ChangeLog b/ChangeLog index 22db55d9..43aefcb4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -17,6 +17,7 @@ ver. 0.9.1 (2014/xx/xx) - better, faster, stronger * journalmatch for recidive incorrect PRIORITY * loglevel couldn't be changed in fail2ban.conf * Handle case when no sqlite library is available for persistent database + * Only reban once per IP from database on fail2ban restart - New features: diff --git a/fail2ban/server/database.py b/fail2ban/server/database.py index 0fad396b..93186222 100644 --- a/fail2ban/server/database.py +++ b/fail2ban/server/database.py @@ -361,7 +361,10 @@ class Fail2BanDb(object): ticket : BanTicket Ticket of the ban to be added. """ - self._bansMergedCache = {} + try: + del self._bansMergedCache[(ticket.getIP(), jail)] + except KeyError: + pass #TODO: Implement data parts once arbitrary match keys completed cur.execute( "INSERT INTO bans(jail, ip, timeofban, data) VALUES(?, ?, ?, ?)", @@ -383,7 +386,7 @@ class Fail2BanDb(object): if ip is not None: query += " AND ip=?" queryArgs.append(ip) - query += " ORDER BY timeofban" + query += " ORDER BY ip, timeofban" return cur.execute(query, queryArgs) @@ -412,7 +415,7 @@ class Fail2BanDb(object): tickets[-1].setAttempt(data['failures']) return tickets - def getBansMerged(self, ip, jail=None, **kwargs): + def getBansMerged(self, ip=None, jail=None, bantime=None): """Get bans from the database, merged into single ticket. This is the same as `getBans`, but bans merged into single @@ -430,22 +433,44 @@ class Fail2BanDb(object): Returns ------- - Ticket - Single ticket representing bans stored in database. + list or Ticket + Single ticket representing bans stored in database per IP + in a list. When `ip` argument passed, a single `Ticket` is + returned. """ - cacheKey = ip if jail is None else "%s|%s" % (ip, jail.name) - if cacheKey in self._bansMergedCache: - return self._bansMergedCache[cacheKey] - matches = [] - failures = 0 - for ip, timeofban, data in self._getBans(ip=ip, jail=jail, **kwargs): - #TODO: Implement data parts once arbitrary match keys completed - matches.extend(data['matches']) - failures += data['failures'] - ticket = FailTicket(ip, timeofban, matches) - ticket.setAttempt(failures) - self._bansMergedCache[cacheKey] = ticket - return ticket + if bantime is None: + cacheKey = (ip, jail) + if cacheKey in self._bansMergedCache: + return self._bansMergedCache[cacheKey] + + tickets = [] + ticket = None + + results = list(self._getBans(ip=ip, jail=jail, bantime=bantime)) + if results: + prev_banip = results[0][0] + matches = [] + failures = 0 + for banip, timeofban, data in results: + #TODO: Implement data parts once arbitrary match keys completed + if banip != prev_banip: + ticket = FailTicket(prev_banip, prev_timeofban, matches) + ticket.setAttempt(failures) + tickets.append(ticket) + # Reset variables + prev_banip = banip + matches = [] + failures = 0 + matches.extend(data['matches']) + failures += data['failures'] + prev_timeofban = timeofban + ticket = FailTicket(banip, prev_timeofban, matches) + ticket.setAttempt(failures) + tickets.append(ticket) + + if bantime is None: + self._bansMergedCache[cacheKey] = tickets if ip is None else ticket + return tickets if ip is None else ticket @commitandrollback def purge(self, cur): diff --git a/fail2ban/server/jail.py b/fail2ban/server/jail.py index dec04c73..a80fd60d 100644 --- a/fail2ban/server/jail.py +++ b/fail2ban/server/jail.py @@ -209,7 +209,7 @@ class Jail: self.actions.start() # Restore any previous valid bans from the database if self.database is not None: - for ticket in self.database.getBans( + for ticket in self.database.getBansMerged( jail=self, bantime=self.actions.getBanTime()): if not self.filter.inIgnoreIPList(ticket.getIP()): self.__queue.put(ticket) diff --git a/fail2ban/tests/databasetestcase.py b/fail2ban/tests/databasetestcase.py index 1cfd78b9..84101c50 100644 --- a/fail2ban/tests/databasetestcase.py +++ b/fail2ban/tests/databasetestcase.py @@ -190,16 +190,16 @@ class DatabaseTest(unittest.TestCase): jail2 = DummyJail() self.db.addJail(jail2) - ticket = FailTicket("127.0.0.1", 10, ["abc\n"]) + ticket = FailTicket("127.0.0.1", MyTime.time() - 40, ["abc\n"]) ticket.setAttempt(10) self.db.addBan(self.jail, ticket) - ticket = FailTicket("127.0.0.1", 20, ["123\n"]) + ticket = FailTicket("127.0.0.1", MyTime.time() - 30, ["123\n"]) ticket.setAttempt(20) self.db.addBan(self.jail, ticket) - ticket = FailTicket("127.0.0.2", 30, ["ABC\n"]) + ticket = FailTicket("127.0.0.2", MyTime.time() - 20, ["ABC\n"]) ticket.setAttempt(30) self.db.addBan(self.jail, ticket) - ticket = FailTicket("127.0.0.1", 40, ["ABC\n"]) + ticket = FailTicket("127.0.0.1", MyTime.time() - 10, ["ABC\n"]) ticket.setAttempt(40) self.db.addBan(jail2, ticket) @@ -220,7 +220,15 @@ class DatabaseTest(unittest.TestCase): id(ticket), id(self.db.getBansMerged("127.0.0.1", jail=self.jail))) - newTicket = FailTicket("127.0.0.1", 40, ["ABC\n"]) + newTicket = FailTicket("127.0.0.2", MyTime.time() - 20, ["ABC\n"]) + ticket.setAttempt(40) + # Add ticket, but not for same IP, so cache still valid + self.db.addBan(self.jail, newTicket) + self.assertEqual( + id(ticket), + id(self.db.getBansMerged("127.0.0.1", jail=self.jail))) + + newTicket = FailTicket("127.0.0.1", MyTime.time() - 10, ["ABC\n"]) ticket.setAttempt(40) self.db.addBan(self.jail, newTicket) # Added ticket, so cache should have been cleared @@ -228,6 +236,22 @@ class DatabaseTest(unittest.TestCase): id(ticket), id(self.db.getBansMerged("127.0.0.1", jail=self.jail))) + tickets = self.db.getBansMerged() + self.assertEqual(len(tickets), 2) + self.assertEqual( + sorted(list(set(ticket.getIP() for ticket in tickets))), + sorted([ticket.getIP() for ticket in tickets])) + + tickets = self.db.getBansMerged(jail=jail2) + self.assertEqual(len(tickets), 1) + + tickets = self.db.getBansMerged(bantime=25) + self.assertEqual(len(tickets), 2) + tickets = self.db.getBansMerged(bantime=15) + self.assertEqual(len(tickets), 1) + tickets = self.db.getBansMerged(bantime=5) + self.assertEqual(len(tickets), 0) + def testPurge(self): if Fail2BanDb is None: # pragma: no cover return