From 6281dc3633de31fb13037ae35a8a41cd3e05c8e6 Mon Sep 17 00:00:00 2001 From: sebres Date: Mon, 2 Mar 2020 17:05:00 +0100 Subject: [PATCH] failmanager, ticket: avoid reset of retry count by pause between attempts near to findTime - adjust time of ticket will now change current attempts considering findTime as an estimation from rate by previous known interval (if it exceeds the findTime); this should avoid some false positives as well as provide more safe handling around `maxretry/findtime` relation especially on busy circumstances. --- fail2ban/server/failmanager.py | 9 +++---- fail2ban/server/ticket.py | 43 ++++++++++++++++---------------- fail2ban/tests/tickettestcase.py | 28 +++++++++++++-------- 3 files changed, 42 insertions(+), 38 deletions(-) diff --git a/fail2ban/server/failmanager.py b/fail2ban/server/failmanager.py index 80a6414a..eee979fd 100644 --- a/fail2ban/server/failmanager.py +++ b/fail2ban/server/failmanager.py @@ -92,10 +92,7 @@ class FailManager: if attempt <= 0: attempt += 1 unixTime = ticket.getTime() - fData.setLastTime(unixTime) - if fData.getLastReset() < unixTime - self.__maxTime: - fData.setLastReset(unixTime) - fData.setRetry(0) + fData.adjustTime(unixTime, self.__maxTime) fData.inc(matches, attempt, count) # truncate to maxMatches: if self.maxMatches: @@ -136,7 +133,7 @@ class FailManager: def cleanup(self, time): with self.__lock: todelete = [fid for fid,item in self.__failList.iteritems() \ - if item.getLastTime() + self.__maxTime <= time] + if item.getTime() + self.__maxTime <= time] if len(todelete) == len(self.__failList): # remove all: self.__failList = dict() @@ -150,7 +147,7 @@ class FailManager: else: # create new dictionary without items to be deleted: self.__failList = dict((fid,item) for fid,item in self.__failList.iteritems() \ - if item.getLastTime() + self.__maxTime > time) + if item.getTime() + self.__maxTime > time) self.__bgSvc.service() def delFailure(self, fid): diff --git a/fail2ban/server/ticket.py b/fail2ban/server/ticket.py index 4f509ea9..8feeac9a 100644 --- a/fail2ban/server/ticket.py +++ b/fail2ban/server/ticket.py @@ -218,21 +218,20 @@ class FailTicket(Ticket): def __init__(self, ip=None, time=None, matches=None, data={}, ticket=None): # this class variables: - self.__retry = 0 - self.__lastReset = None + self._firstTime = None + self._retry = 1 # create/copy using default ticket constructor: Ticket.__init__(self, ip, time, matches, data, ticket) # init: - if ticket is None: - self.__lastReset = time if time is not None else self.getTime() - if not self.__retry: - self.__retry = self._data['failures']; + if not isinstance(ticket, FailTicket): + self._firstTime = time if time is not None else self.getTime() + self._retry = self._data.get('failures', 1) def setRetry(self, value): """ Set artificial retry count, normally equal failures / attempt, used in incremental features (BanTimeIncr) to increase retry count for bad IPs """ - self.__retry = value + self._retry = value if not self._data['failures']: self._data['failures'] = 1 if not value: @@ -243,10 +242,23 @@ class FailTicket(Ticket): """ Returns failures / attempt count or artificial retry count increased for bad IPs """ - return max(self.__retry, self._data['failures']) + return self._retry + + def adjustTime(self, time, maxTime): + """ Adjust time of ticket and current attempts count considering given maxTime + as estimation from rate by previous known interval (if it exceeds the findTime) + """ + if time > self._time: + # expand current interval and attemps count (considering maxTime): + if self._firstTime < time - maxTime: + # adjust retry calculated as estimation from rate by previous known interval: + self._retry = int(round(self._retry / float(time - self._firstTime) * maxTime)) + self._firstTime = time - maxTime + # last time of failure: + self._time = time def inc(self, matches=None, attempt=1, count=1): - self.__retry += count + self._retry += count self._data['failures'] += attempt if matches: # we should duplicate "matches", because possibly referenced to multiple tickets: @@ -255,19 +267,6 @@ class FailTicket(Ticket): else: self._data['matches'] = matches - def setLastTime(self, value): - if value > self._time: - self._time = value - - def getLastTime(self): - return self._time - - def getLastReset(self): - return self.__lastReset - - def setLastReset(self, value): - self.__lastReset = value - ## # Ban Ticket. # diff --git a/fail2ban/tests/tickettestcase.py b/fail2ban/tests/tickettestcase.py index 277c2f28..d7d5f19a 100644 --- a/fail2ban/tests/tickettestcase.py +++ b/fail2ban/tests/tickettestcase.py @@ -69,10 +69,10 @@ class TicketTests(unittest.TestCase): self.assertEqual(ft.getTime(), tm) self.assertEqual(ft.getMatches(), matches2) ft.setAttempt(2) - self.assertEqual(ft.getAttempt(), 2) - # retry is max of set retry and failures: - self.assertEqual(ft.getRetry(), 2) ft.setRetry(1) + self.assertEqual(ft.getAttempt(), 2) + self.assertEqual(ft.getRetry(), 1) + ft.setRetry(2) self.assertEqual(ft.getRetry(), 2) ft.setRetry(3) self.assertEqual(ft.getRetry(), 3) @@ -86,13 +86,21 @@ class TicketTests(unittest.TestCase): self.assertEqual(ft.getRetry(), 14) self.assertEqual(ft.getMatches(), matches3) # last time (ignore if smaller as time): - self.assertEqual(ft.getLastTime(), tm) - ft.setLastTime(tm-60) self.assertEqual(ft.getTime(), tm) - self.assertEqual(ft.getLastTime(), tm) - ft.setLastTime(tm+60) + ft.adjustTime(tm-60, 3600) + self.assertEqual(ft.getTime(), tm) + self.assertEqual(ft.getRetry(), 14) + ft.adjustTime(tm+60, 3600) self.assertEqual(ft.getTime(), tm+60) - self.assertEqual(ft.getLastTime(), tm+60) + self.assertEqual(ft.getRetry(), 14) + ft.adjustTime(tm+3600, 3600) + self.assertEqual(ft.getTime(), tm+3600) + self.assertEqual(ft.getRetry(), 14) + # adjust time so interval is larger than find time (3600), so reset retry count: + ft.adjustTime(tm+7200, 3600) + self.assertEqual(ft.getTime(), tm+7200) + self.assertEqual(ft.getRetry(), 7); # estimated attempts count + self.assertEqual(ft.getAttempt(), 4); # real known failure count ft.setData('country', 'DE') self.assertEqual(ft.getData(), {'matches': ['first', 'second', 'third'], 'failures': 4, 'country': 'DE'}) @@ -102,10 +110,10 @@ class TicketTests(unittest.TestCase): self.assertEqual(ft, ft2) self.assertEqual(ft.getData(), ft2.getData()) self.assertEqual(ft2.getAttempt(), 4) - self.assertEqual(ft2.getRetry(), 14) + self.assertEqual(ft2.getRetry(), 7) self.assertEqual(ft2.getMatches(), matches3) self.assertEqual(ft2.getTime(), ft.getTime()) - self.assertEqual(ft2.getLastTime(), ft.getLastTime()) + self.assertEqual(ft2.getTime(), ft.getTime()) self.assertEqual(ft2.getBanTime(), ft.getBanTime()) def testTicketFlags(self):