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

@@ -92,10 +92,7 @@ class FailManager:
 					if attempt <= 0:
 						attempt += 1
 				unixTime = ticket.getTime()
-				fData.setLastTime(unixTime)
+				fData.adjustTime(unixTime, self.__maxTime)
-				if fData.getLastReset() < unixTime - self.__maxTime:
-					fData.setLastReset(unixTime)
-					fData.setRetry(0)
 				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):

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._firstTime = None
-		self.__lastReset = None
+		self._retry = 1
 		# create/copy using default ticket constructor:
 		Ticket.__init__(self, ip, time, matches, data, ticket)
 		# init:
-		if ticket is None:
+		if not isinstance(ticket, FailTicket):
-			self.__lastReset = time if time is not None else self.getTime()
+			self._firstTime = time if time is not None else self.getTime()
-		if not self.__retry:
+			self._retry = self._data.get('failures', 1)
-			self.__retry = self._data['failures'];
 
 	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.
 #

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.adjustTime(tm-60, 3600)
-    ft.setLastTime(tm+60)
+    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):