bantime.rndtime also affects initial bantime (of still unknown tickets without prolongation);

closes gh-2834

ChangeLog

@@ -28,6 +28,9 @@ ver. 1.1.1-dev-1 (20??/??/??) - development nightly edition
   several log messages will be tagged with as originating from a process named "sshd-session" rather than "sshd" (gh-3782)
 
 ### New Features and Enhancements
+* `bantime.rndtime` also affects the initial `bantime` (still unknown tickets without prolongation),
+  this caused that rndtime will be prolonged too and therefore multiplied by the exponent and factor 
+  during the bantime.increment process (see gh-2834);
 * new jail option `skip_if_nologs` to ignore jail if no `logpath` matches found, fail2ban continue to start with warnings/errors,
   thus other jails become running (gh-2756)
 * `action.d/*-ipset.conf`:

fail2ban/server/actions.py

@@ -494,17 +494,24 @@ class Actions(JailThread, Mapping):
 		for ticket in tickets:
 
 			bTicket = BanTicket.wrap(ticket)
-			btime = ticket.getBanTime(self.banManager.getBanTime())
 			ip = bTicket.getID()
-			aInfo = self._getActionInfo(bTicket)
 			reason = {}
 			if self.banManager.addBanTicket(bTicket, reason=reason):
 				cnt += 1
+				btime = bTicket.getBanTime(self.banManager.calcBanTime)
 				# report ticket to observer, to check time should be increased and hereafter observer writes ban to database (asynchronous)
 				if Observers.Main is not None and not bTicket.restored:
 					Observers.Main.add('banFound', bTicket, self._jail, btime)
-				logSys.notice("[%s] %sBan %s", self._jail.name, ('' if not bTicket.restored else 'Restore '), ip)
+				# because of rndtime the initial bantime may be variable so now we'll add it to ban message too:
+				if btime != -1:
+					bendtime = bTicket.getTime() + btime
+					logtime = (MyTime.seconds2str(btime), MyTime.time2str(bendtime))
+				else:
+					logtime = ('permanent', 'infinite')
+				logtime = " (%s -> %s)" % logtime
+				logSys.notice("[%s] %sBan %s%s", self._jail.name, ('' if not bTicket.restored else 'Restore '), ip, logtime)
 				# do actions :
+				aInfo = self._getActionInfo(bTicket)
 				for name, action in self._actions.items():
 					try:
 						if bTicket.restored and getattr(action, 'norestored', False):

fail2ban/server/banmanager.py

@@ -24,6 +24,8 @@ __author__ = "Cyril Jaquier"
 __copyright__ = "Copyright (c) 2004 Cyril Jaquier"
 __license__ = "GPL"
 
+import random
+
 from threading import Lock
 
 from .ticket import BanTicket
@@ -54,6 +56,8 @@ class BanManager:
 		self.__banList = dict()
 		## The amount of time an IP address gets banned.
 		self.__banTime = 600
+		## Additional random part of bantime.
+		self.__rndTime = 0
 		## Total number of banned IP address
 		self.__banTotal = 0
 		## The time for next unban process (for performance and load reasons):
@@ -66,7 +70,7 @@ class BanManager:
 	# @param value the time
 	
 	def setBanTime(self, value):
-			self.__banTime = int(value)
+		self.__banTime = int(value)
 	
 	##
 	# Get the ban time.
@@ -75,15 +79,22 @@ class BanManager:
 	# @return the time
 	
 	def getBanTime(self):
-			return self.__banTime
-	
+		return self.__banTime
+
+
+	def setRndTime(self, value):
+		self.__rndTime = int(value) if value else 0
+
+	def calcBanTime(self):
+		return self.__banTime + (random.random() * self.__rndTime if self.__rndTime and self.__banTime != -1 else 0)
+
 	##
 	# Set the total number of banned address.
 	#
 	# @param value total number
 	
 	def setBanTotal(self, value):
-			self.__banTotal = value
+		self.__banTotal = value
 	
 	##
 	# Get the total number of banned address.
@@ -91,7 +102,7 @@ class BanManager:
 	# @return the total number
 	
 	def getBanTotal(self):
-			return self.__banTotal
+		return self.__banTotal
 
 	##
 	# Returns a copy of the IP list.
@@ -266,7 +277,7 @@ class BanManager:
 	# @return True if the IP address is not in the ban list
 	
 	def addBanTicket(self, ticket, reason={}):
-		eob = ticket.getEndOfBanTime(self.__banTime)
+		eob = ticket.getEndOfBanTime(self.calcBanTime)
 		if eob < MyTime.time():
 			reason['expired'] = 1
 			return False
@@ -280,7 +291,7 @@ class BanManager:
 				if eob > oldticket.getEndOfBanTime(self.__banTime):
 					# we have longest ban - set new (increment) ban time
 					reason['prolong'] = 1
-					btm = ticket.getBanTime(self.__banTime)
+					btm = ticket.getBanTime(self.calcBanTime)
 					# if not permanent:
 					if btm != -1:
 						diftm = ticket.getTime() - oldticket.getTime()
@@ -292,6 +303,9 @@ class BanManager:
 			self.__banList[fid] = ticket
 			self.__banTotal += 1
 			ticket.incrBanCount()
+			# if bantime not yet set and random part exists - set fixed bantime here:
+			if ticket._banTime is None and self.__rndTime and eob != BanTicket.MAX_TIME:
+				ticket.setBanTime(eob - ticket.getTime())
 			# correct next unban time:
 			if self._nextUnbanTime > eob:
 				self._nextUnbanTime = eob

fail2ban/server/jail.py

@@ -25,7 +25,6 @@ __license__ = "GPL"
 
 import logging
 import math
-import random
 import queue
 
 from .actions import Actions
@@ -238,9 +237,11 @@ class Jail(object):
 				logSys.warning("ban time increment is not available as long jail database is not set")
 		if opt in ['maxtime', 'rndtime']:
 			if not value is None:
-				be[opt] = MyTime.str2seconds(value)
+				be[opt] = value = MyTime.str2seconds(value)
+			if opt == 'rndtime':
+				self.actions.banManager.setRndTime(value)
 		# prepare formula lambda:
-		if opt in ['formula', 'factor', 'maxtime', 'rndtime', 'multipliers'] or be.get('evformula', None) is None:
+		if opt in ['formula', 'factor', 'maxtime', 'multipliers'] or be.get('evformula', None) is None:
 			# split multifiers to an array begins with 0 (or empty if not set):
 			if opt == 'multipliers':
 				be['evmultipliers'] = [int(i) for i in (value.split(' ') if value is not None and value != '' else [])]
@@ -259,10 +260,6 @@ class Jail(object):
 			if not be.get('maxtime', None) is None:
 				maxtime = be['maxtime']
 				evformula = lambda ban, evformula=evformula: min(evformula(ban), maxtime)
-			# mix lambda with random time (to prevent bot-nets to calculate exact time IP can be unbanned):
-			if not be.get('rndtime', None) is None:
-				rndtime = be['rndtime']
-				evformula = lambda ban, evformula=evformula: (evformula(ban) + random.random() * rndtime)
 			# set to extra dict:
 			be['evformula'] = evformula
 		#logSys.info('banTimeExtra : %s' % json.dumps(be))

fail2ban/server/mytime.py

@@ -227,6 +227,7 @@ class MyTime:
 			if s >= 60: # a minute
 				r += str(s//60) + 'm '; s %= 60
 			if s: # remaining seconds
+				if isinstance(s, float): s = round(s, 3) if not r else int(round(s))
 				r += str(s) + 's '
 			elif not self.sec: # 0s
 				r = '0 '

fail2ban/server/observer.py

@@ -476,11 +476,12 @@ class ObserverThread(JailThread):
 			oldbtime = btime
 			ip = ticket.getID()
 			logSys.debug("[%s] Observer: ban found %s, %s", jail.name, ip, btime)
-			# if not permanent and ban time was not set - check time should be increased:
-			if btime != -1 and ticket.getBanTime() is None:
-				btime = self.incrBanTime(jail, btime, ticket)
+			# if not permanent and ban time was not yet prolonged - check time should be increased:
+			if btime != -1 and not ticket.prolonged:
+				btime = self.incrBanTime(jail, ticket.getBanTime(btime), ticket)
 				# if we should prolong ban time:
 				if btime == -1 or btime > oldbtime:
+					ticket.prolonged = True
 					ticket.setBanTime(btime)
 			# if not permanent
 			if btime != -1:

fail2ban/server/ticket.py

@@ -37,8 +37,9 @@ class Ticket(object):
 
 	MAX_TIME = 0X7FFFFFFFFFFF ;# 4461763-th year
 	
-	RESTORED = 0x01
-	BANNED   = 0x08
+	RESTORED  = 0x01
+	PROLONGED = 0x04
+	BANNED    = 0x08
 
 	def __init__(self, ip=None, time=None, matches=None, data={}, ticket=None):
 		"""Ticket constructor
@@ -108,7 +109,7 @@ class Ticket(object):
 		self._banTime = value
 
 	def getBanTime(self, defaultBT=None):
-		return (self._banTime if self._banTime is not None else defaultBT)
+		return (self._banTime if self._banTime is not None else (defaultBT() if callable(defaultBT) else defaultBT))
 
 	def setBanCount(self, value, always=False):
 		if always or value > self._banCount:
@@ -121,7 +122,7 @@ class Ticket(object):
 		return self._banCount;
 
 	def getEndOfBanTime(self, defaultBT=None):
-		bantime = (self._banTime if self._banTime is not None else defaultBT)
+		bantime = self.getBanTime(defaultBT)
 		# permanent
 		if bantime == -1:
 			return Ticket.MAX_TIME
@@ -129,7 +130,7 @@ class Ticket(object):
 		return self._time + bantime
 
 	def isTimedOut(self, time, defaultBT=None):
-		bantime = (self._banTime if self._banTime is not None else defaultBT)
+		bantime = self.getBanTime(defaultBT)
 		# permanent
 		if bantime == -1:
 			return False
@@ -175,6 +176,16 @@ class Ticket(object):
 		else:
 			self._flags &= ~(Ticket.BANNED)
 
+	@property
+	def prolonged(self):
+		return self._flags & Ticket.PROLONGED
+	@prolonged.setter
+	def prolonged(self, value):
+		if value:
+			self._flags |= Ticket.PROLONGED
+		else:
+			self._flags &= ~(Ticket.PROLONGED)
+
 	def setData(self, *args, **argv):
 		# if overwrite - set data and filter None values:
 		if len(args) == 1:

fail2ban/tests/banmanagertestcase.py

@@ -99,6 +99,28 @@ class AddFailure(unittest.TestCase):
 		ticket = BanTicket('111.111.1.111', 1167605999.0)
 		self.assertFalse(self.__banManager._inBanList(ticket))
 		
+	def testRndBanTime(self):
+		bm = self.__banManager
+		bm.setBanTime(600)
+		bm.setRndTime(300)
+		for i in range(20): # try multiple times (almost impossible to get random 0 always)
+			t = BanTicket(self.__ticket.getID(), self.__ticket.getTime())
+			bm.addBanTicket(t);
+			bm.flushBanList()
+			if t.getBanTime(bm.getBanTime) > bm.getBanTime():
+				break
+		self.assertTrue(isinstance(t.getBanTime(bm.getBanTime), float))
+		self.assertTrue(t.getBanTime(bm.getBanTime) > bm.getBanTime())
+		bm.setRndTime(None)
+		for i in range(20):
+			t = BanTicket(self.__ticket.getID(), self.__ticket.getTime())
+			bm.addBanTicket(t);
+			bm.flushBanList()
+			if t.getBanTime(bm.getBanTime) > bm.getBanTime():
+				break
+		self.assertTrue(isinstance(t.getBanTime(bm.getBanTime), int))
+		self.assertTrue(t.getBanTime(bm.getBanTime) == bm.getBanTime())
+
 	def testBanTimeIncr(self):
 		ticket = BanTicket(self.__ticket.getID(), self.__ticket.getTime())
 		## increase twice and at end permanent, check time/count increase:

fail2ban/tests/fail2banclienttestcase.py

@@ -1663,6 +1663,39 @@ class Fail2banServerTest(Fail2banClientServerBase):
 		self.assertLogged(
 			"192.0.2.11", "+ 600 =", all=True, wait=MID_WAITTIME)
 
+		self.pruneLog("[test-phase 3) time+31m]")
+		# jump to the future (+20 minutes):
+		_time_shift(20)
+		_observer_wait_idle()
+		self.assertLogged(
+			"stdout: '[test-jail1] test-action1: -- unban 192.0.2.11",
+			"stdout: '[test-jail1] test-action2: -- unban 192.0.2.11",
+			"0 ticket(s) in 'test-jail1'",
+			all=True, wait=MID_WAITTIME)
+		_observer_wait_idle()
+
+		self.execCmd(SUCCESS, startparams, "set", "test-jail1", "bantime.rndtime", "300s")
+
+		# generate bad ip:
+		_write_file(test1log, "w+", *(
+		  (str(int(MyTime.time())) + " failure 401 from 192.0.2.11: I'm \"evildoer\"",) * 1
+		))
+		# wait for ban:
+		self.assertLogged(
+			"stdout: '[test-jail1] test-action1: ++ ban 192.0.2.11 ",
+			"stdout: '[test-jail1] test-action2: ++ ban 192.0.2.11 ",
+			all=True, wait=MID_WAITTIME)
+
+		self.pruneLog("[test-phase 4) time+32m]")
+		# jump to the future (+1 minute):
+		_time_shift(1)
+		# wait for observer idle (write all tickets to db):
+		_observer_wait_idle()
+		# wait for prolong:
+		self.assertLogged(
+			"stdout: '[test-jail1] test-action2: ++ prolong 192.0.2.11 ",
+			all=True, wait=MID_WAITTIME)
+
 		# test stop with busy observer:
 		self.pruneLog("[test-phase end) stop on busy observer]")
 		tearDownMyTime()

fail2ban/tests/observertestcase.py

@@ -104,14 +104,15 @@ class BanTimeIncr(LogCaptureTestCase):
 			[1200, 2400, 4800, 9600, 19200, 38400, 43200, 43200, 43200, 43200]
 		)
 		a.setBanTimeExtra('maxtime', '24h')
-		## test randomization - not possible all 10 times we have random = 0:
+		## test randomization - impossible accurately - all 10 times we may get random 0:
+		bm = a.actions.banManager
 		a.setBanTimeExtra('rndtime', '5m')
 		self.assertTrue(
-			False in [1200 in [a.calcBanTime(600, 1) for i in range(10)] for c in range(10)]
+			True in [bm.calcBanTime() > bm.getBanTime() for i in range(10)]
 		)
 		a.setBanTimeExtra('rndtime', None)
-		self.assertFalse(
-			False in [1200 in [a.calcBanTime(600, 1) for i in range(10)] for c in range(10)]
+		self.assertTrue(
+			True in [bm.calcBanTime() == bm.getBanTime() for i in range(10)]
 		)
 		# restore default:
 		a.setBanTimeExtra('multipliers', None)
@@ -159,14 +160,15 @@ class BanTimeIncr(LogCaptureTestCase):
 			[1200, 2400, 4800, 9600, 19200, 38400, 43200, 43200, 43200, 43200]
 		)
 		a.setBanTimeExtra('maxtime', '24h')
-		## test randomization - not possible all 10 times we have random = 0:
+		## test randomization - impossible accurately - all 10 times we may get random 0:
+		bm = a.actions.banManager
 		a.setBanTimeExtra('rndtime', '5m')
 		self.assertTrue(
-			False in [1200 in [int(a.calcBanTime(600, 1)) for i in range(10)] for c in range(10)]
+			True in [bm.calcBanTime() > bm.getBanTime() for i in range(10)]
 		)
 		a.setBanTimeExtra('rndtime', None)
-		self.assertFalse(
-			False in [1200 in [int(a.calcBanTime(600, 1)) for i in range(10)] for c in range(10)]
+		self.assertTrue(
+			True in [bm.calcBanTime() == bm.getBanTime() for i in range(10)]
 		)
 		# restore default:
 		a.setBanTimeExtra('factor', None);