Merge pull request #2588 from sebres/0.10-invariant-improve

0.10 auto-reban, improved invariant check and conditional operations

ChangeLog

@@ -80,6 +80,10 @@ ver. 0.10.5-dev-1 (20??/??/??) - development edition
 * `filter.d/sendmail-auth.conf`, `filter.d/sendmail-reject.conf` :
   - ID in prefix can be longer as 14 characters (gh-2563);
 * all filters would accept square brackets around IPv4 addresses also (e. g. monit-filter, gh-2494)
+* avoids unhandled exception during flush (gh-2588)
+* fixes pass2allow-ftp jail - due to inverted handling, action should prohibit access per default for any IP,
+  therefore reset start on demand parameter for this action (it will be started immediately by repair);
+* auto-detection of IPv6 subsystem availability (important for not on-demand actions or jails, like pass2allow);
 
 ### New Features
 * new replacement tags for failregex to match subnets in form of IP-addresses with CIDR mask (gh-2559):
@@ -136,6 +140,18 @@ filter = flt[logtype=short]
 * samplestestcase.py (testSampleRegexsFactory) extended:
   - allow coverage of journal logtype;
   - new option `fileOptions` to set common filter/test options for whole test-file;
+* large enhancement: auto-reban, improved invariant check and conditional operations (gh-2588):
+  - improves invariant check and repair (avoid unhandled exception, consider family on conditional operations, etc),
+    prepared for bulk re-ban in repair case (if bulk-ban becomes implemented);
+  - automatic reban (repeat banning action) after repair/restore sane environment, if already logged ticket causes
+    new failures (via new action operation `actionreban` or `actionban` if still not defined in action);
+  * introduces banning epoch for actions and tickets (to distinguish or recognize removed set of the tickets);
+  * invariant check avoids repair by unban/stop (unless parameter `actionrepair_on_unban` set to `true`);
+  * better handling for all conditional operations (distinguish families for certain operations like 
+    repair/flush/stop, prepared for other families, e. g. if different handling for subnets expected, etc);
+  * partially implements gh-980 (more breakdown safe handling);
+  * closes gh-1680 (better as large-scale banning implementation with on-demand reban by failure, 
+    at least unless a bulk-ban gets implemented);
 
 
 ver. 0.10.4 (2018/10/04) - ten-four-on-due-date-ten-four

config/jail.conf

@@ -861,7 +861,8 @@ filter       = apache-pass[knocking_url="%(knocking_url)s"]
 logpath      = %(apache_access_log)s
 blocktype    = RETURN
 returntype   = DROP
-action       = %(action_)s[blocktype=%(blocktype)s, returntype=%(returntype)s]
+action       = %(action_)s[blocktype=%(blocktype)s, returntype=%(returntype)s,
+                        actionstart_on_demand=false, actionrepair_on_unban=true]
 bantime      = 1h
 maxretry     = 1
 findtime     = 1

fail2ban/client/actionreader.py

@@ -44,7 +44,9 @@ class ActionReader(DefinitionInitConfigReader):
 		"actionreload": ["string", None],
 		"actioncheck": ["string", None],
 		"actionrepair": ["string", None],
+		"actionrepair_on_unban": ["string", None],
 		"actionban": ["string", None],
+		"actionreban": ["string", None],
 		"actionunban": ["string", None],
 		"norestored": ["string", None],
 	}
@@ -78,7 +80,7 @@ class ActionReader(DefinitionInitConfigReader):
 		opts = self.getCombined(
 			ignore=CommandAction._escapedTags | set(('timeout', 'bantime')))
 		# type-convert only after combined (otherwise boolean converting prevents substitution):
-		for o in ('norestored', 'actionstart_on_demand'):
+		for o in ('norestored', 'actionstart_on_demand', 'actionrepair_on_unban'):
 			if opts.get(o):
 				opts[o] = self._convert_to_boolean(opts[o])
 		

fail2ban/client/jailreader.py

@@ -33,7 +33,7 @@ from .configreader import ConfigReaderUnshared, ConfigReader
 from .filterreader import FilterReader
 from .actionreader import ActionReader
 from ..version import version
-from ..helpers import getLogger, extractOptions, splitwords
+from ..helpers import getLogger, extractOptions, splitWithOptions, splitwords
 
 # Gets the instance of the logger.
 logSys = getLogger(__name__)
@@ -164,21 +164,15 @@ class JailReader(ConfigReader):
 				self.__filter.getOptions(self.__opts)
 		
 			# Read action
-			prevln = ''
+			for act in splitWithOptions(self.__opts["action"]):
-			actlst = self.__opts["action"].split('\n')
-			for n, act in enumerate(actlst):
 				try:
+					act = act.strip()
 					if not act:			  # skip empty actions
 						continue
 					# join with previous line if needed (consider possible new-line):
-					if prevln: act = prevln + '\n' + act
 					actName, actOpt = extractOptions(act)
 					prevln = ''
 					if not actName:
-						# consider possible new-line, so repeat with joined next line's:
-						if n < len(actlst) - 1:
-							prevln = act
-							continue
 						raise JailDefError("Invalid action definition %r" % act)
 					if actName.endswith(".py"):
 						self.__actions.append([

fail2ban/helpers.py

@@ -336,6 +336,9 @@ OPTION_CRE = re.compile(r"^([^\[]+)(?:\[(.*)\])?\s*$", re.DOTALL)
 # `action = act[p1=...][p2=...]`
 OPTION_EXTRACT_CRE = re.compile(
 	r'([\w\-_\.]+)=(?:"([^"]*)"|\'([^\']*)\'|([^,\]]*))(?:,|\]\s*\[|$)', re.DOTALL)
+# split by new-line considering possible new-lines within options [...]:
+OPTION_SPLIT_CRE = re.compile(
+	r'(?:[^\[\n]+(?:\s*\[\s*(?:[\w\-_\.]+=(?:"[^"]*"|\'[^\']*\'|[^,\]]*)\s*(?:,|\]\s*\[)?\s*)*\])?\s*|[^\n]+)(?=\n\s*|$)', re.DOTALL)
 
 def extractOptions(option):
 	match = OPTION_CRE.match(option)
@@ -352,6 +355,9 @@ def extractOptions(option):
 			option_opts[opt.strip()] = value.strip()
 	return option_name, option_opts
 
+def splitWithOptions(option):
+	return OPTION_SPLIT_CRE.findall(option)
+
 #
 # Following facilities used for safe recursive interpolation of
 # tags (<tag>) in tagged options.
@@ -386,8 +392,7 @@ def substituteRecursiveTags(inptags, conditional='',
 	"""
 	#logSys = getLogger("fail2ban")
 	tre_search = TAG_CRE.search
-	# copy return tags dict to prevent modifying of inptags:
+	tags = inptags
-	tags = inptags.copy()
 	# init:
 	ignore = set(ignore)
 	done = set()
@@ -449,6 +454,9 @@ def substituteRecursiveTags(inptags, conditional='',
 				# check still contains any tag - should be repeated (possible embedded-recursive substitution):
 				if tre_search(value):
 					repFlag = True
+				# copy return tags dict to prevent modifying of inptags:
+				if id(tags) == id(inptags):
+					tags = inptags.copy()
 				tags[tag] = value
 			# no more sub tags (and no possible composite), add this tag to done set (just to be faster):
 			if '<' not in value: done.add(tag)

fail2ban/server/action.py

@@ -33,10 +33,11 @@ from abc import ABCMeta
 from collections import MutableMapping
 
 from .failregex import mapTag2Opt
-from .ipdns import asip, DNSUtils
+from .ipdns import DNSUtils
 from .mytime import MyTime
 from .utils import Utils
-from ..helpers import getLogger, _merge_copy_dicts, uni_string, substituteRecursiveTags, TAG_CRE, MAX_TAG_REPLACE_COUNT
+from ..helpers import getLogger, _merge_copy_dicts, \
+	splitwords, substituteRecursiveTags, uni_string, TAG_CRE, MAX_TAG_REPLACE_COUNT
 
 # Gets the instance of the logger.
 logSys = getLogger(__name__)
@@ -44,13 +45,14 @@ logSys = getLogger(__name__)
 # Create a lock for running system commands
 _cmd_lock = threading.Lock()
 
-# Todo: make it configurable resp. automatically set, ex.: `[ -f /proc/net/if_inet6 ] && echo 'yes' || echo 'no'`:
+# Specifies whether IPv6 subsystem is available:
-allowed_ipv6 = True
+allowed_ipv6 = DNSUtils.IPv6IsAllowed
 
 # capture groups from filter for map to ticket data:
 FCUSTAG_CRE = re.compile(r'<F-([A-Z0-9_\-]+)>'); # currently uppercase only
 
-CONDITIONAL_FAM_RE = re.compile(r"^(\w+)\?(family)=")
+COND_FAMILIES = ('inet4', 'inet6')
+CONDITIONAL_FAM_RE = re.compile(r"^(\w+)\?(family)=(.*)$")
 
 # Special tags:
 DYN_REPL_TAGS = {
@@ -173,7 +175,7 @@ class CallingMap(MutableMapping, object):
 	def __len__(self):
 		return len(self.data)
 
-	def copy(self): # pragma: no cover
+	def copy(self):
 		return self.__class__(_merge_copy_dicts(self.data, self.storage))
 
 
@@ -215,6 +217,7 @@ class ActionBase(object):
 			"start",
 			"stop",
 			"ban",
+			"reban",
 			"unban",
 			)
 		for method in required:
@@ -248,6 +251,17 @@ class ActionBase(object):
 		"""
 		pass
 
+	def reban(self, aInfo): # pragma: no cover - abstract
+		"""Executed when a ban occurs.
+
+		Parameters
+		----------
+		aInfo : dict
+			Dictionary which includes information in relation to
+			the ban.
+		"""
+		return self.ban(aInfo)
+
 	def unban(self, aInfo): # pragma: no cover - abstract
 		"""Executed when a ban expires.
 
@@ -279,6 +293,7 @@ class CommandAction(ActionBase):
 	----------
 	actionban
 	actioncheck
+	actionreban
 	actionreload
 	actionrepair
 	actionstart
@@ -299,6 +314,7 @@ class CommandAction(ActionBase):
 			self.actionstart = ''
 			## Command executed when ticket gets banned.
 			self.actionban = ''
+			self.actionreban = ''
 			## Command executed when ticket gets removed.
 			self.actionunban = ''
 			## Command executed in order to check requirements.
@@ -340,6 +356,8 @@ class CommandAction(ActionBase):
 		# set:
 		self.__dict__[name] = value
 
+	__setitem__ = __setattr__
+
 	def __delattr__(self, name):
 		if not name.startswith('_'):
 			# parameters changed - clear properties and substitution cache:
@@ -363,8 +381,8 @@ class CommandAction(ActionBase):
 		self.__properties = dict(
 			(key, getattr(self, key))
 			for key in dir(self)
-			if not key.startswith("_") and not callable(getattr(self, key)))
+			if not key.startswith("_") and not callable(getattr(self, key))
-		#
+		)
 		return self.__properties
 
 	@property
@@ -372,10 +390,40 @@ class CommandAction(ActionBase):
 		return self.__substCache
 
 	def _getOperation(self, tag, family):
+		# replace operation tag (interpolate all values), be sure family is enclosed as conditional value
+		# (as lambda in addrepl so only if not overwritten in action):
 		return self.replaceTag(tag, self._properties,
-			conditional=('family=' + family), cache=self.__substCache)
+			conditional=('family='+family if family else ''),
+			addrepl=(lambda tag:family if tag == 'family' else None),
+			cache=self.__substCache)
 
-	def _executeOperation(self, tag, operation, family=[]):
+	def _operationExecuted(self, tag, family, *args):
+		""" Get, set or delete command of operation considering family.
+		"""
+		key = ('__eOpCmd',tag)
+		if not len(args): # get
+			if not callable(family): # pragma: no cover
+				return self.__substCache.get(key, {}).get(family)
+			# family as expression - use it to filter values:
+			return [v for f, v in self.__substCache.get(key, {}).iteritems() if family(f)]
+		cmd = args[0]
+		if cmd: # set:
+			try:
+				famd = self.__substCache[key]
+			except KeyError:
+				famd = self.__substCache[key] = {}
+			famd[family] = cmd
+		else: # delete (given family and all other with same command):
+			try:
+				famd = self.__substCache[key]
+				cmd = famd.pop(family)
+				for family, v in famd.items():
+					if v == cmd:
+						del famd[family]
+			except KeyError: # pragma: no cover
+				pass
+
+	def _executeOperation(self, tag, operation, family=[], afterExec=None):
 		"""Executes the operation commands (like "actionstart", "actionstop", etc).
 
 		Replace the tags in the action command with actions properties
@@ -383,24 +431,53 @@ class CommandAction(ActionBase):
 		"""
 		# check valid tags in properties (raises ValueError if self recursion, etc.):
 		res = True
-		try:
+		err = 'Script error'
-			# common (resp. ipv4):
+		if not family: # all started:
-			cmd = self._getOperation(tag, 'inet4')
+			family = [famoper for (famoper,v) in self.__started.iteritems() if v]
-			if not family or 'inet4' in family:
+		for famoper in family:
-				if cmd:
+			try:
-					res &= self.executeCmd(cmd, self.timeout)
+				cmd = self._getOperation(tag, famoper)
-			# execute ipv6 operation if available (and not the same as ipv4):
+				ret = True
-			if allowed_ipv6 and (not family or 'inet6' in family):
+				# avoid double execution of same command for both families:
-				cmd6 = self._getOperation(tag, 'inet6')
+				if cmd and cmd not in self._operationExecuted(tag, lambda f: f != famoper):
-				if cmd6 and cmd6 != cmd: # - avoid double execution of same command
+					ret = self.executeCmd(cmd, self.timeout)
-					res &= self.executeCmd(cmd6, self.timeout)
+					res &= ret
-			if not res:
+				if afterExec: afterExec(famoper, ret)
-				raise RuntimeError("Error %s action %s/%s" % (operation, self._jail, self._name,))
+				self._operationExecuted(tag, famoper, cmd if ret else None)
-		except ValueError as e:
+			except ValueError as e:
-			raise RuntimeError("Error %s action %s/%s: %r" % (operation, self._jail, self._name, e))
+				res = False
+				err = e
+		if not res:
+			raise RuntimeError("Error %s action %s/%s: %r" % (operation, self._jail, self._name, err))
 		return res
 
-	COND_FAMILIES = ('inet4', 'inet6')
+	@property
+	def _hasCondSection(self):
+		v = self._properties.get('__hasCondSection')
+		if v is not None:
+			return v
+		v = False
+		for n in self._properties:
+			if CONDITIONAL_FAM_RE.match(n):
+				v = True
+				break
+		self._properties['__hasCondSection'] = v
+		return v
+
+	@property
+	def _families(self):
+		v = self._properties.get('__families')
+		if v: return v
+		v = self._properties.get('families')
+		if v and not isinstance(v, (list,set)): # pragma: no cover - still unused
+			v = splitwords(v)
+		elif self._hasCondSection: # all conditional families:
+			# todo: check it is needed at all # common (resp. ipv4) + ipv6 if allowed:
+			v = ['inet4', 'inet6'] if allowed_ipv6() else ['inet4']
+		else: # all action tags seems to be the same
+			v = ['']
+		self._properties['__families'] = v
+		return v
 
 	@property
 	def _startOnDemand(self):
@@ -409,15 +486,19 @@ class CommandAction(ActionBase):
 		if v is not None:
 			return v
 		# not set - auto-recognize (depending on conditional):
-		v = False
+		v = self._hasCondSection
-		for n in self._properties:
-			if CONDITIONAL_FAM_RE.match(n):
-				v = True
-				break
 		self._properties['actionstart_on_demand'] = v
 		return v
 
-	def start(self, family=None, forceStart=False):
+	def start(self):
+		"""Executes the "actionstart" command.
+
+		Replace the tags in the action command with actions properties
+		and executes the resulting command.
+		"""
+		return self._start()
+
+	def _start(self, family=None, forceStart=False):
 		"""Executes the "actionstart" command.
 
 		Replace the tags in the action command with actions properties
@@ -427,14 +508,18 @@ class CommandAction(ActionBase):
 		if self._startOnDemand:
 			if not forceStart:
 				return True
-		elif self.__started.get(family): # pragma: no cover - normally unreachable
+		elif not forceStart and self.__started.get(family): # pragma: no cover - normally unreachable
 			return True
-		ret = self._executeOperation('<actionstart>', 'starting', family=family)
+		family = [family] if family is not None else self._families
-		self.__started[family] = ret
+		def _started(family, ret):
+			if ret:
+				self._operationExecuted('<actionstop>', family, None)
+				self.__started[family] = 1
+		ret = self._executeOperation('<actionstart>', 'starting', family=family, afterExec=_started)
 		return ret
 
-	def ban(self, aInfo):
+	def ban(self, aInfo, cmd='<actionban>'):
-		"""Executes the "actionban" command.
+		"""Executes the given command ("actionban" or "actionreban").
 
 		Replaces the tags in the action command with actions properties
 		and ban information, and executes the resulting command.
@@ -446,13 +531,14 @@ class CommandAction(ActionBase):
 			the ban.
 		"""
 		# if we should start the action on demand (conditional by family):
+		family = aInfo.get('family', '')
 		if self._startOnDemand:
-			family = aInfo.get('family')
 			if not self.__started.get(family):
-				self.start(family, forceStart=True)
+				self._start(family, forceStart=True)
 		# ban:
-		if not self._processCmd('<actionban>', aInfo):
+		if not self._processCmd(cmd, aInfo):
 			raise RuntimeError("Error banning %(ip)s" % aInfo)
+		self.__started[family] = self.__started.get(family, 0) | 3; # started and contains items
 
 	def unban(self, aInfo):
 		"""Executes the "actionunban" command.
@@ -466,8 +552,25 @@ class CommandAction(ActionBase):
 			Dictionary which includes information in relation to
 			the ban.
 		"""
-		if not self._processCmd('<actionunban>', aInfo):
+		family = aInfo.get('family', '')
-			raise RuntimeError("Error unbanning %(ip)s" % aInfo)
+		if self.__started.get(family, 0) & 2: # contains items
+			if not self._processCmd('<actionunban>', aInfo):
+				raise RuntimeError("Error unbanning %(ip)s" % aInfo)
+
+	def reban(self, aInfo):
+		"""Executes the "actionreban" command if available, otherwise simply repeat "actionban".
+
+		Replaces the tags in the action command with actions properties
+		and ban information, and executes the resulting command.
+
+		Parameters
+		----------
+		aInfo : dict
+			Dictionary which includes information in relation to
+			the ban.
+		"""
+		# re-ban:
+		return self.ban(aInfo, '<actionreban>' if self.actionreban else '<actionban>')
 
 	def flush(self):
 		"""Executes the "actionflush" command.
@@ -478,13 +581,15 @@ class CommandAction(ActionBase):
 		Replaces the tags in the action command with actions properties
 		and executes the resulting command.
 		"""
-		family = []
+		# collect started families, may be started on demand (conditional):
-		# collect started families, if started on demand (conditional):
+		family = [f for (f,v) in self.__started.iteritems() if v & 3 == 3]; # started and contains items
-		if self._startOnDemand:
+		# if nothing contains items:
-			family = [f for (f,v) in self.__started.iteritems() if v]
+		if not family: return True
-			# if no started (on demand) actions:
+		# flush:
-			if not family: return True
+		def _afterFlush(family, ret):
-		return self._executeOperation('<actionflush>', 'flushing', family=family)
+			if ret and self.__started.get(family):
+				self.__started[family] &= ~2; # no items anymore
+		return self._executeOperation('<actionflush>', 'flushing', family=family, afterExec=_afterFlush)
 
 	def stop(self):
 		"""Executes the "actionstop" command.
@@ -492,14 +597,30 @@ class CommandAction(ActionBase):
 		Replaces the tags in the action command with actions properties
 		and executes the resulting command.
 		"""
-		family = []
+		return self._stop()
+
+	def _stop(self, family=None):
+		"""Executes the "actionstop" command.
+
+		Replaces the tags in the action command with actions properties
+		and executes the resulting command.
+		"""
 		# collect started families, if started on demand (conditional):
-		if self._startOnDemand:
+		if family is None:
 			family = [f for (f,v) in self.__started.iteritems() if v]
 			# if no started (on demand) actions:
 			if not family: return True
-		self.__started = {}
+			self.__started = {}
-		return self._executeOperation('<actionstop>', 'stopping', family=family)
+		else:
+			try:
+				self.__started[family] &= 0
+				family = [family]
+			except KeyError: # pragma: no cover
+				return True
+		def _stopped(family, ret):
+			if ret:
+				self._operationExecuted('<actionstart>', family, None)
+		return self._executeOperation('<actionstop>', 'stopping', family=family, afterExec=_stopped)
 
 	def reload(self, **kwargs):
 		"""Executes the "actionreload" command.
@@ -514,6 +635,20 @@ class CommandAction(ActionBase):
 		"""
 		return self._executeOperation('<actionreload>', 'reloading')
 
+	def consistencyCheck(self, beforeRepair=None):
+		"""Executes the invariant check with repair if expected (conditional).
+		"""
+		ret = True
+		# for each started family:
+		if self.actioncheck:
+			for (family, started) in self.__started.items():
+				if started and not self._invariantCheck(family, beforeRepair):
+					# reset started flag and command of executed operation:
+					self.__started[family] = 0
+					self._operationExecuted('<actionstart>', family, None)
+					ret &= False
+		return ret
+
 	@staticmethod
 	def escapeTag(value):
 		"""Escape characters which may be used for command injection.
@@ -541,7 +676,7 @@ class CommandAction(ActionBase):
 		return value
 
 	@classmethod
-	def replaceTag(cls, query, aInfo, conditional='', cache=None):
+	def replaceTag(cls, query, aInfo, conditional='', addrepl=None, cache=None):
 		"""Replaces tags in `query` with property values.
 
 		Parameters
@@ -582,7 +717,8 @@ class CommandAction(ActionBase):
 					pass
 			# interpolation of dictionary:
 			if subInfo is None:
-				subInfo = substituteRecursiveTags(aInfo, conditional, ignore=cls._escapedTags)
+				subInfo = substituteRecursiveTags(aInfo, conditional, ignore=cls._escapedTags,
+					addrepl=addrepl)
 			# cache if possible:
 			if csubkey is not None:
 				cache[csubkey] = subInfo
@@ -705,7 +841,58 @@ class CommandAction(ActionBase):
 			realCmd = Utils.buildShellCmd(realCmd, varsDict)
 		return realCmd
 
-	def _processCmd(self, cmd, aInfo=None, conditional=''):
+	@property
+	def banEpoch(self):
+		return getattr(self, '_banEpoch', 0)
+	def invalidateBanEpoch(self):
+		"""Increments ban epoch of jail and this action, so already banned tickets would cause
+		a re-ban for all tickets with previous epoch."""
+		if self._jail is not None:
+			self._banEpoch = self._jail.actions.banEpoch = self._jail.actions.banEpoch + 1
+		else:
+			self._banEpoch = self.banEpoch + 1
+
+	def _invariantCheck(self, family=None, beforeRepair=None, forceStart=True):
+		"""Executes a substituted `actioncheck` command.
+		"""
+		# for started action/family only (avoid check not started inet4 if inet6 gets broken):
+		if not forceStart and family is not None and family not in self.__started:
+			return 1
+		checkCmd = self._getOperation('<actioncheck>', family)
+		if not checkCmd or self.executeCmd(checkCmd, self.timeout):
+			return 1
+		# if don't need repair/restore - just return:
+		if beforeRepair and not beforeRepair():
+			return -1
+		self._logSys.error(
+			"Invariant check failed. Trying to restore a sane environment")
+		# increment ban epoch of jail and this action (allows re-ban on already banned):
+		self.invalidateBanEpoch()
+		# try to find repair command, if exists - exec it:
+		repairCmd = self._getOperation('<actionrepair>', family)
+		if repairCmd:
+			if not self.executeCmd(repairCmd, self.timeout):
+				self.__started[family] = 0
+				self._logSys.critical("Unable to restore environment")
+				return 0
+			self.__started[family] = 1
+		else:
+			# no repair command, try to restart action...
+			# [WARNING] TODO: be sure all banactions get a repair command, because
+			#    otherwise stop/start will theoretically remove all the bans,
+			#    but the tickets are still in BanManager, so in case of new failures
+			#    it will not be banned, because "already banned" will happen.
+			try:
+				self._stop(family)
+			except RuntimeError: # bypass error in stop (if start/check succeeded hereafter).
+				pass
+			self._start(family, forceStart=forceStart or not self._startOnDemand)
+		if self.__started.get(family) and not self.executeCmd(checkCmd, self.timeout):
+			self._logSys.critical("Unable to restore environment")
+			return 0
+		return 1
+
+	def _processCmd(self, cmd, aInfo=None):
 		"""Executes a command with preliminary checks and substitutions.
 
 		Before executing any commands, executes the "check" command first
@@ -730,47 +917,28 @@ class CommandAction(ActionBase):
 			return True
 
 		# conditional corresponding family of the given ip:
-		if conditional == '':
+		try:
-			conditional = 'family=inet4'
+			family = aInfo["family"]
-			if allowed_ipv6:
+		except (KeyError, TypeError):
-				try:
+			family = ''
-					ip = aInfo["ip"]
-					if ip and asip(ip).isIPv6:
-						conditional = 'family=inet6'
-				except KeyError:
-					pass
 
-		checkCmd = self.replaceTag('<actioncheck>', self._properties, 
+		# invariant check:
-			conditional=conditional, cache=self.__substCache)
+		if self.actioncheck:
-		if checkCmd:
+			# don't repair/restore if unban (no matter):
-			if not self.executeCmd(checkCmd, self.timeout):
+			def _beforeRepair():
-				self._logSys.error(
+				if cmd == '<actionunban>' and not self._properties.get('actionrepair_on_unban'):
-					"Invariant check failed. Trying to restore a sane environment")
+					self._logSys.error("Invariant check failed. Unban is impossible.")
-				# try to find repair command, if exists - exec it:
-				repairCmd = self.replaceTag('<actionrepair>', self._properties, 
-					conditional=conditional, cache=self.__substCache)
-				if repairCmd:
-					if not self.executeCmd(repairCmd, self.timeout):
-						self._logSys.critical("Unable to restore environment")
-						return False
-				else:
-					# no repair command, try to restart action...
-					# [WARNING] TODO: be sure all banactions get a repair command, because
-					#    otherwise stop/start will theoretically remove all the bans,
-					#    but the tickets are still in BanManager, so in case of new failures
-					#    it will not be banned, because "already banned" will happen.
-					try:
-						self.stop()
-					except RuntimeError: # bypass error in stop (if start/check succeeded hereafter).
-						pass
-					self.start()
-				if not self.executeCmd(checkCmd, self.timeout):
-					self._logSys.critical("Unable to restore environment")
 					return False
+				return True
+			# check and repair if broken:
+			ret = self._invariantCheck(family, _beforeRepair, forceStart=(cmd != '<actionunban>'))
+			# if not sane (and not restored) return:
+			if ret != 1:
+				return False
 
 		# Replace static fields
 		realCmd = self.replaceTag(cmd, self._properties, 
-			conditional=conditional, cache=self.__substCache)
+			conditional=('family='+family if family else ''), cache=self.__substCache)
 
 		# Replace dynamical tags, important - don't cache, no recursion and auto-escape here
 		if aInfo is not None:

fail2ban/server/actions.py

@@ -81,6 +81,8 @@ class Actions(JailThread, Mapping):
 		self._actions = OrderedDict()
 		## The ban manager.
 		self.__banManager = BanManager()
+		self.banEpoch = 0
+		self.__lastConsistencyCheckTM = 0
 		## Precedence of ban (over unban), so max number of tickets banned (to call an unban check):
 		self.banPrecedence = 10
 		## Max count of outdated tickets to unban per each __checkUnBan operation:
@@ -160,8 +162,8 @@ class Actions(JailThread, Mapping):
 				delacts = OrderedDict((name, action) for name, action in self._actions.iteritems()
 					if name not in self._reload_actions)
 				if len(delacts):
-					# unban all tickets using remove action only:
+					# unban all tickets using removed actions only:
-					self.__flushBan(db=False, actions=delacts)
+					self.__flushBan(db=False, actions=delacts, stop=True)
 					# stop and remove it:
 					self.stopActions(actions=delacts)
 				delattr(self, '_reload_actions')
@@ -214,10 +216,10 @@ class Actions(JailThread, Mapping):
 
 		if isinstance(ip, list):
 			# Multiple IPs:
-			tickets = (BanTicket(ip if isinstance(ip, IPAddr) else IPAddr(ip), unixTime) for ip in ip)
+			tickets = (BanTicket(ip, unixTime) for ip in ip)
 		else:
 			# Single IP:
-			tickets = (BanTicket(ip if isinstance(ip, IPAddr) else IPAddr(ip), unixTime),)
+			tickets = (BanTicket(ip, unixTime),)
 
 		return self.__checkBan(tickets)
 
@@ -326,7 +328,7 @@ class Actions(JailThread, Mapping):
 					self.__checkUnBan(bancnt if bancnt and bancnt < self.unbanMaxCount else self.unbanMaxCount)
 				cnt = 0
 		
-		self.__flushBan()
+		self.__flushBan(stop=True)
 		self.stopActions()
 		return True
 
@@ -439,6 +441,7 @@ class Actions(JailThread, Mapping):
 		cnt = 0
 		if not tickets:
 			tickets = self.__getFailTickets(self.banPrecedence)
+		rebanacts = None
 		for ticket in tickets:
 			bTicket = BanManager.createBanTicket(ticket)
 			ip = bTicket.getIP()
@@ -461,6 +464,8 @@ class Actions(JailThread, Mapping):
 							exc_info=logSys.getEffectiveLevel()<=logging.DEBUG)
 				# after all actions are processed set banned flag:
 				bTicket.banned = True
+				if self.banEpoch: # be sure tickets always have the same ban epoch (default 0):
+					bTicket.banEpoch = self.banEpoch
 			else:
 				bTicket = reason['ticket']
 				# if already banned (otherwise still process some action)
@@ -475,11 +480,60 @@ class Actions(JailThread, Mapping):
 					else logging.NOTICE  if diftm < 60 \
 					else logging.WARNING
 					logSys.log(ll, "[%s] %s already banned", self._jail.name, ip)
+					# if long time after ban - do consistency check (something is wrong here):
+					if bTicket.banEpoch == self.banEpoch and diftm > 3:
+						# avoid too often checks:
+						if not rebanacts and MyTime.time() > self.__lastConsistencyCheckTM + 3:
+							for action in self._actions.itervalues():
+								action.consistencyCheck()
+							self.__lastConsistencyCheckTM = MyTime.time()
+					# check epoch in order to reban it:
+					if bTicket.banEpoch < self.banEpoch:
+						if not rebanacts: rebanacts = dict(
+							(name, action) for name, action in self._actions.iteritems()
+								if action.banEpoch > bTicket.banEpoch)
+						cnt += self.__reBan(bTicket, actions=rebanacts)
+				else: # pragma: no cover - unexpected: ticket is not banned for some reasons - reban using all actions:
+					cnt += self.__reBan(bTicket)
 		if cnt:
 			logSys.debug("Banned %s / %s, %s ticket(s) in %r", cnt, 
 				self.__banManager.getBanTotal(), self.__banManager.size(), self._jail.name)
 		return cnt
 
+	def __reBan(self, ticket, actions=None, log=True):
+		"""Repeat bans for the ticket.
+
+		Executes the actions in order to reban the host given in the
+		ticket.
+
+		Parameters
+		----------
+		ticket : Ticket
+			Ticket to reban
+		"""
+		actions = actions or self._actions
+		ip = ticket.getIP()
+		aInfo = self.__getActionInfo(ticket)
+		if log:
+			logSys.notice("[%s] Reban %s%s", self._jail.name, aInfo["ip"], (', action %r' % actions.keys()[0] if len(actions) == 1 else ''))
+		for name, action in actions.iteritems():
+			try:
+				logSys.debug("[%s] action %r: reban %s", self._jail.name, name, ip)
+				if not aInfo.immutable: aInfo.reset()
+				action.reban(aInfo)
+			except Exception as e:
+				logSys.error(
+					"Failed to execute reban jail '%s' action '%s' "
+					"info '%r': %s",
+					self._jail.name, name, aInfo, e,
+					exc_info=logSys.getEffectiveLevel()<=logging.DEBUG)
+				return 0
+		# after all actions are processed set banned flag:
+		ticket.banned = True
+		if self.banEpoch: # be sure tickets always have the same ban epoch (default 0):
+			ticket.banEpoch = self.banEpoch
+		return 1
+
 	def __checkUnBan(self, maxCount=None):
 		"""Check for IP address to unban.
 
@@ -494,7 +548,7 @@ class Actions(JailThread, Mapping):
 				cnt, self.__banManager.size(), self._jail.name)
 		return cnt
 
-	def __flushBan(self, db=False, actions=None):
+	def __flushBan(self, db=False, actions=None, stop=False):
 		"""Flush the ban list.
 
 		Unban all IP address which are still in the banning list.
@@ -513,17 +567,33 @@ class Actions(JailThread, Mapping):
 		# first we'll execute flush for actions supporting this operation:
 		unbactions = {}
 		for name, action in (actions if actions is not None else self._actions).iteritems():
-			if hasattr(action, 'flush') and action.actionflush:
+			try:
-				logSys.notice("[%s] Flush ticket(s) with %s", self._jail.name, name)
+				if hasattr(action, 'flush') and (not isinstance(action, CommandAction) or action.actionflush):
-				action.flush()
+					logSys.notice("[%s] Flush ticket(s) with %s", self._jail.name, name)
-			else:
+					if action.flush():
-				unbactions[name] = action
+						continue
+			except Exception as e:
+				logSys.error("Failed to flush bans in jail '%s' action '%s': %s",
+					self._jail.name, name, e,
+					exc_info=logSys.getEffectiveLevel()<=logging.DEBUG)
+				logSys.info("No flush occurred, do consistency check")
+				if hasattr(action, 'consistencyCheck'):
+					def _beforeRepair():
+						if stop and not getattr(action, 'actionrepair_on_unban', None): # don't need repair on stop
+							self._logSys.error("Invariant check failed. Flush is impossible.")
+							return False
+						return True
+					action.consistencyCheck(_beforeRepair)
+					continue
+			# fallback to single unbans:
+			logSys.debug("  Unban tickets each individualy")
+			unbactions[name] = action
 		actions = unbactions
 		# flush the database also:
 		if db and self._jail.database is not None:
 			logSys.debug("  Flush jail in database")
 			self._jail.database.delBan(self._jail)
-		# unban each ticket with non-flasheable actions:
+		# unban each ticket with non-flusheable actions:
 		for ticket in lst:
 			# unban ip:
 			self.__unBan(ticket, actions=actions, log=log)
@@ -553,8 +623,6 @@ class Actions(JailThread, Mapping):
 			logSys.notice("[%s] Unban %s", self._jail.name, aInfo["ip"])
 		for name, action in unbactions.iteritems():
 			try:
-				if ticket.restored and getattr(action, 'norestored', False):
-					continue
 				logSys.debug("[%s] action %r: unban %s", self._jail.name, name, ip)
 				if not aInfo.immutable: aInfo.reset()
 				action.unban(aInfo)

fail2ban/server/ipdns.py

@@ -202,6 +202,11 @@ class DNSUtils:
 		DNSUtils.CACHE_nameToIp.set(key, ips)
 		return ips
 
+	@staticmethod
+	def IPv6IsAllowed():
+		# return os.path.exists("/proc/net/if_inet6") || any((':' in ip) for ip in DNSUtils.getSelfIPs())
+		return any((':' in ip.ntoa) for ip in DNSUtils.getSelfIPs())
+
 
 ##
 # Class for IP address handling.

fail2ban/server/ticket.py

@@ -206,6 +206,13 @@ class Ticket(object):
 		# return single value of data:
 		return self._data.get(key, default)
 
+	@property
+	def banEpoch(self):
+		return getattr(self, '_banEpoch', 0)
+	@banEpoch.setter
+	def banEpoch(self, value):
+		self._banEpoch = value
+
 
 class FailTicket(Ticket):
 

fail2ban/tests/actionstestcase.py

@@ -28,11 +28,10 @@ import time
 import os
 import tempfile
 
-from ..server.actions import Actions
 from ..server.ticket import FailTicket
 from ..server.utils import Utils
 from .dummyjail import DummyJail
-from .utils import LogCaptureTestCase, with_alt_time, MyTime
+from .utils import LogCaptureTestCase, with_alt_time, with_tmpdir, MyTime
 
 TEST_FILES_DIR = os.path.join(os.path.dirname(__file__), "files")
 
@@ -43,21 +42,21 @@ class ExecuteActions(LogCaptureTestCase):
 		"""Call before every test case."""
 		super(ExecuteActions, self).setUp()
 		self.__jail = DummyJail()
-		self.__actions = Actions(self.__jail)
+		self.__actions = self.__jail.actions
-		self.__tmpfile, self.__tmpfilename  = tempfile.mkstemp()
 
 	def tearDown(self):
 		super(ExecuteActions, self).tearDown()
-		os.remove(self.__tmpfilename)
 
-	def defaultActions(self):
+	def defaultAction(self, o={}):
 		self.__actions.add('ip')
-		self.__ip = self.__actions['ip']
+		act = self.__actions['ip']
-		self.__ip.actionstart = 'echo ip start 64 >> "%s"' % self.__tmpfilename
+		act.actionstart = 'echo ip start'+o.get('start', '')
-		self.__ip.actionban = 'echo ip ban <ip> >> "%s"' % self.__tmpfilename
+		act.actionban = 'echo ip ban <ip>'+o.get('ban', '')
-		self.__ip.actionunban = 'echo ip unban <ip> >> "%s"' % self.__tmpfilename
+		act.actionunban = 'echo ip unban <ip>'+o.get('unban', '')
-		self.__ip.actioncheck = 'echo ip check <ip> >> "%s"' % self.__tmpfilename
+		act.actioncheck = 'echo ip check'+o.get('check', '')
-		self.__ip.actionstop = 'echo ip stop >> "%s"' % self.__tmpfilename
+		act.actionflush = 'echo ip flush'+o.get('flush', '')
+		act.actionstop = 'echo ip stop'+o.get('stop', '')
+		return act
 
 	def testActionsAddDuplicateName(self):
 		self.__actions.add('test')
@@ -89,13 +88,12 @@ class ExecuteActions(LogCaptureTestCase):
 		self.assertLogged('Ban 192.0.2.3')
 
 	def testActionsOutput(self):
-		self.defaultActions()
+		self.defaultAction()
 		self.__actions.start()
-		with open(self.__tmpfilename) as f:
+		self.assertLogged("stdout: %r" % 'ip start', wait=True)
-			self.assertTrue( Utils.wait_for(lambda: (f.read() == "ip start 64\n"), 3) )
-
 		self.__actions.stop()
 		self.__actions.join()
+		self.assertLogged("stdout: %r" % 'ip flush', "stdout: %r" % 'ip stop')
 		self.assertEqual(self.__actions.status(),[("Currently banned", 0 ),
                ("Total banned", 0 ), ("Banned IP list", [] )])
 
@@ -211,3 +209,296 @@ class ExecuteActions(LogCaptureTestCase):
 
 		self.assertLogged('Unbanned 30, 0 ticket(s)')
 		self.assertNotLogged('Unbanned 50, 0 ticket(s)')
+
+	def testActionsConsistencyCheck(self):
+		act = self.defaultAction({'check':' <family>', 'flush':' <family>'})
+		# flush for inet6 is intentionally "broken" here - test no unhandled except and invariant check:
+		act['actionflush?family=inet6'] = act.actionflush + '; exit 1'
+		act.actionstart_on_demand = True
+		self.__actions.start()
+		self.assertNotLogged("stdout: %r" % 'ip start')
+
+		self.assertEqual(self.__actions.addBannedIP('192.0.2.1'), 1)
+		self.assertEqual(self.__actions.addBannedIP('2001:db8::1'), 1)
+		self.assertLogged('Ban 192.0.2.1', 'Ban 2001:db8::1',
+			"stdout: %r" % 'ip start',
+			"stdout: %r" % 'ip ban 192.0.2.1',
+			"stdout: %r" % 'ip ban 2001:db8::1',
+			all=True, wait=True)
+
+		# check should fail (so cause stop/start):
+		self.pruneLog('[test-phase 1a] simulate inconsistent irreparable env by unban')
+		act['actioncheck?family=inet6'] = act.actioncheck + '; exit 1'
+		self.__actions.removeBannedIP('2001:db8::1')
+		self.assertLogged('Invariant check failed. Unban is impossible.',
+			wait=True)
+		self.pruneLog('[test-phase 1b] simulate inconsistent irreparable env by flush')
+		self.__actions._Actions__flushBan()
+		self.assertLogged(
+			"stdout: %r" % 'ip flush inet4',
+			"stdout: %r" % 'ip flush inet6',
+			'Failed to flush bans',
+			'No flush occurred, do consistency check',
+			'Invariant check failed. Trying to restore a sane environment',
+			"stdout: %r" % 'ip stop',  # same for both families
+			'Failed to flush bans',
+			all=True, wait=True)
+
+		# check succeeds:
+		self.pruneLog('[test-phase 2] consistent env')
+		act['actioncheck?family=inet6'] = act.actioncheck
+		self.assertEqual(self.__actions.addBannedIP('2001:db8::1'), 1)
+		self.assertLogged('Ban 2001:db8::1',
+			"stdout: %r" % 'ip start',   # same for both families
+			"stdout: %r" % 'ip ban 2001:db8::1',
+			all=True, wait=True)
+		self.assertNotLogged("stdout: %r" % 'ip check inet4',
+			all=True)
+
+		self.pruneLog('[test-phase 3] failed flush in consistent env')
+		self.__actions._Actions__flushBan()
+		self.assertLogged('Failed to flush bans',
+			'No flush occurred, do consistency check',
+			"stdout: %r" % 'ip flush inet6',
+			"stdout: %r" % 'ip check inet6',
+			all=True, wait=True)
+		self.assertNotLogged(
+			"stdout: %r" % 'ip flush inet4',
+			"stdout: %r" % 'ip stop',
+			"stdout: %r" % 'ip start',
+			'Unable to restore environment',
+			all=True)
+
+		# stop, flush succeeds:
+		self.pruneLog('[test-phase end] flush successful')
+		act['actionflush?family=inet6'] = act.actionflush
+		self.__actions.stop()
+		self.__actions.join()
+		self.assertLogged(
+			"stdout: %r" % 'ip flush inet6',
+			"stdout: %r" % 'ip stop',    # same for both families
+			'action ip terminated',
+			all=True, wait=True)
+		# no flush for inet4 (already successfully flushed):
+		self.assertNotLogged("ERROR",
+			"stdout: %r" % 'ip flush inet4',
+			'Unban tickets each individualy',
+			all=True)
+
+	def testActionsConsistencyCheckDiffFam(self):
+		# same as testActionsConsistencyCheck, but different start/stop commands for both families and repair on unban
+		act = self.defaultAction({'start':' <family>', 'check':' <family>', 'flush':' <family>', 'stop':' <family>'})
+		# flush for inet6 is intentionally "broken" here - test no unhandled except and invariant check:
+		act['actionflush?family=inet6'] = act.actionflush + '; exit 1'
+		act.actionstart_on_demand = True
+		act.actionrepair_on_unban = True
+		self.__actions.start()
+		self.assertNotLogged("stdout: %r" % 'ip start')
+
+		self.assertEqual(self.__actions.addBannedIP('192.0.2.1'), 1)
+		self.assertEqual(self.__actions.addBannedIP('2001:db8::1'), 1)
+		self.assertLogged('Ban 192.0.2.1', 'Ban 2001:db8::1',
+			"stdout: %r" % 'ip start inet4',
+			"stdout: %r" % 'ip ban 192.0.2.1',
+			"stdout: %r" % 'ip start inet6',
+			"stdout: %r" % 'ip ban 2001:db8::1',
+			all=True, wait=True)
+
+		# check should fail (so cause stop/start):
+		act['actioncheck?family=inet6'] = act.actioncheck + '; exit 1'
+		self.pruneLog('[test-phase 1a] simulate inconsistent irreparable env by unban')
+		self.__actions.removeBannedIP('2001:db8::1')
+		self.assertLogged('Invariant check failed. Trying to restore a sane environment',
+			"stdout: %r" % 'ip stop inet6',
+			all=True, wait=True)
+		self.assertNotLogged(
+			"stdout: %r" % 'ip start inet6', # start on demand (not on repair)
+			"stdout: %r" % 'ip stop inet4',  # family inet4 is not affected
+			"stdout: %r" % 'ip start inet4',
+			all=True)
+
+		self.pruneLog('[test-phase 1b] simulate inconsistent irreparable env by ban')
+		self.assertEqual(self.__actions.addBannedIP('2001:db8::1'), 1)
+		self.assertLogged('Invariant check failed. Trying to restore a sane environment',
+			"stdout: %r" % 'ip stop inet6',
+			"stdout: %r" % 'ip start inet6',
+			"stdout: %r" % 'ip check inet6',
+			'Unable to restore environment',
+			'Failed to execute ban',
+			all=True, wait=True)
+		self.assertNotLogged(
+			"stdout: %r" % 'ip stop inet4',  # family inet4 is not affected
+			"stdout: %r" % 'ip start inet4',
+			all=True)
+
+		act['actioncheck?family=inet6'] = act.actioncheck
+		self.assertEqual(self.__actions.addBannedIP('2001:db8::2'), 1)
+		act['actioncheck?family=inet6'] = act.actioncheck + '; exit 1'
+		self.pruneLog('[test-phase 1c] simulate inconsistent irreparable env by flush')
+		self.__actions._Actions__flushBan()
+		self.assertLogged(
+			"stdout: %r" % 'ip flush inet4',
+			"stdout: %r" % 'ip flush inet6',
+			'Failed to flush bans',
+			'No flush occurred, do consistency check',
+			'Invariant check failed. Trying to restore a sane environment',
+			"stdout: %r" % 'ip stop inet6',
+			'Failed to flush bans in jail',
+			all=True, wait=True)
+		# start/stop should be called for inet6 only:
+		self.assertNotLogged(
+			"stdout: %r" % 'ip stop inet4',
+			all=True)
+
+		# check succeeds:
+		self.pruneLog('[test-phase 2] consistent env')
+		act['actioncheck?family=inet6'] = act.actioncheck
+		self.assertEqual(self.__actions.addBannedIP('2001:db8::1'), 1)
+		self.assertLogged('Ban 2001:db8::1',
+			"stdout: %r" % 'ip start inet6',
+			"stdout: %r" % 'ip ban 2001:db8::1',
+			all=True, wait=True)
+		self.assertNotLogged(
+			"stdout: %r" % 'ip check inet4',
+			"stdout: %r" % 'ip start inet4',
+			all=True)
+
+		self.pruneLog('[test-phase 3] failed flush in consistent env')
+		act['actioncheck?family=inet6'] = act.actioncheck
+		self.__actions._Actions__flushBan()
+		self.assertLogged('Failed to flush bans',
+			'No flush occurred, do consistency check',
+			"stdout: %r" % 'ip flush inet6',
+			"stdout: %r" % 'ip check inet6',
+			all=True, wait=True)
+		self.assertNotLogged(
+			"stdout: %r" % 'ip flush inet4',
+			"stdout: %r" % 'ip stop inet4',
+			"stdout: %r" % 'ip start inet4',
+			"stdout: %r" % 'ip stop inet6',
+			"stdout: %r" % 'ip start inet6',
+			all=True)
+
+		# stop, flush succeeds:
+		self.pruneLog('[test-phase end] flush successful')
+		act['actionflush?family=inet6'] = act.actionflush
+		self.__actions.stop()
+		self.__actions.join()
+		self.assertLogged(
+			"stdout: %r" % 'ip flush inet6',
+			"stdout: %r" % 'ip stop inet4',
+			"stdout: %r" % 'ip stop inet6',
+			'action ip terminated',
+			all=True, wait=True)
+		# no flush for inet4 (already successfully flushed):
+		self.assertNotLogged("ERROR",
+			"stdout: %r" % 'ip flush inet4',
+			'Unban tickets each individualy',
+			all=True)
+
+	@with_alt_time
+	@with_tmpdir
+	def testActionsRebanBrokenAfterRepair(self, tmp):
+		act = self.defaultAction({
+			'start':' <family>; touch "<FN>"',
+			'check':' <family>; test -f "<FN>"',
+			'flush':' <family>; echo -n "" > "<FN>"',
+			'stop': ' <family>; rm -f "<FN>"',
+			'ban':  ' <family>; echo "<ip> <family>" >> "<FN>"',
+		})
+		act['FN'] = tmp+'/<family>'
+		act.actionstart_on_demand = True
+		act.actionrepair = 'echo ip repair <family>; touch "<FN>"'
+		act.actionreban = 'echo ip reban <ip> <family>; echo "<ip> <family> -- rebanned" >> "<FN>"'
+		self.pruneLog('[test-phase 0] initial ban')
+		self.assertEqual(self.__actions.addBannedIP(['192.0.2.1', '2001:db8::1']), 2)
+		self.assertLogged('Ban 192.0.2.1', 'Ban 2001:db8::1',
+			"stdout: %r" % 'ip start inet4',
+			"stdout: %r" % 'ip ban 192.0.2.1 inet4',
+			"stdout: %r" % 'ip start inet6',
+			"stdout: %r" % 'ip ban 2001:db8::1 inet6',
+			all=True)
+
+		self.pruneLog('[test-phase 1] check ban')
+		self.dumpFile(tmp+'/inet4')
+		self.assertLogged('192.0.2.1 inet4')
+		self.assertNotLogged('2001:db8::1 inet6')
+		self.pruneLog()
+		self.dumpFile(tmp+'/inet6')
+		self.assertLogged('2001:db8::1 inet6')
+		self.assertNotLogged('192.0.2.1 inet4')
+
+		# simulate 3 seconds past:
+		MyTime.setTime(MyTime.time() + 4)
+		# already banned produces events:
+		self.pruneLog('[test-phase 2] check already banned')
+		self.assertEqual(self.__actions.addBannedIP(['192.0.2.1', '2001:db8::1', '2001:db8::2']), 1)
+		self.assertLogged(
+			'192.0.2.1 already banned', '2001:db8::1 already banned', 'Ban 2001:db8::2',
+			"stdout: %r" % 'ip check inet4', # both checks occurred
+			"stdout: %r" % 'ip check inet6',
+			all=True)
+		self.dumpFile(tmp+'/inet4')
+		self.dumpFile(tmp+'/inet6')
+		# no reban should occur:
+		self.assertNotLogged('Reban 192.0.2.1', 'Reban 2001:db8::1',
+			"stdout: %r" % 'ip ban 192.0.2.1 inet4',
+			"stdout: %r" % 'ip reban 192.0.2.1 inet4',
+			"stdout: %r" % 'ip ban 2001:db8::1 inet6',
+			"stdout: %r" % 'ip reban 2001:db8::1 inet6',
+			'192.0.2.1 inet4 -- repaired',
+			'2001:db8::1 inet6 -- repaired',
+			all=True)
+
+		# simulate 3 seconds past:
+		MyTime.setTime(MyTime.time() + 4)
+		# break env (remove both files, so check would fail):
+		os.remove(tmp+'/inet4')
+		os.remove(tmp+'/inet6')
+		# test again already banned (it shall cause reban now):
+		self.pruneLog('[test-phase 3a] check reban after sane env repaired')
+		self.assertEqual(self.__actions.addBannedIP(['192.0.2.1', '2001:db8::1']), 2)
+		self.assertLogged(
+			"Invariant check failed. Trying to restore a sane environment",
+			"stdout: %r" % 'ip repair inet4', # both repairs occurred
+			"stdout: %r" % 'ip repair inet6',
+			"Reban 192.0.2.1, action 'ip'", "Reban 2001:db8::1, action 'ip'", # both rebans also
+			"stdout: %r" % 'ip reban 192.0.2.1 inet4',
+			"stdout: %r" % 'ip reban 2001:db8::1 inet6',
+			all=True)
+
+		# now last IP (2001:db8::2) - no repair, but still old epoch of ticket, so it gets rebanned:
+		self.pruneLog('[test-phase 3a] check reban by epoch mismatch (without repair)')
+		self.assertEqual(self.__actions.addBannedIP('2001:db8::2'), 1)
+		self.assertLogged(
+			"Reban 2001:db8::2, action 'ip'",
+			"stdout: %r" % 'ip reban 2001:db8::2 inet6',
+			all=True)
+		self.assertNotLogged(
+			"Invariant check failed. Trying to restore a sane environment",
+			"stdout: %r" % 'ip repair inet4', # both repairs occurred
+			"stdout: %r" % 'ip repair inet6',
+			"Reban 192.0.2.1, action 'ip'", "Reban 2001:db8::1, action 'ip'", # both rebans also
+			"stdout: %r" % 'ip reban 192.0.2.1 inet4',
+			"stdout: %r" % 'ip reban 2001:db8::1 inet6',
+			all=True)
+
+		# and bans present in files:
+		self.pruneLog('[test-phase 4] check reban')
+		self.dumpFile(tmp+'/inet4')
+		self.assertLogged('192.0.2.1 inet4 -- rebanned')
+		self.assertNotLogged('2001:db8::1 inet6 -- rebanned')
+		self.pruneLog()
+		self.dumpFile(tmp+'/inet6')
+		self.assertLogged(
+			'2001:db8::1 inet6 -- rebanned', 
+			'2001:db8::2 inet6 -- rebanned', all=True)
+		self.assertNotLogged('192.0.2.1 inet4 -- rebanned')
+
+		# coverage - intended error in reban (no unhandled exception, message logged):
+		act.actionreban = ''
+		act.actionban = 'exit 1'
+		self.assertEqual(self.__actions._Actions__reBan(FailTicket("192.0.2.1", 0)), 0)
+		self.assertLogged(
+			'Failed to execute reban',
+			'Error banning 192.0.2.1', all=True)

fail2ban/tests/actiontestcase.py

@@ -34,8 +34,8 @@ from ..server.actions import OrderedDict, Actions
 from ..server.utils import Utils
 
 from .dummyjail import DummyJail
-from .utils import LogCaptureTestCase
+from .utils import pid_exists, with_tmpdir, LogCaptureTestCase
-from .utils import pid_exists
+
 
 class CommandActionTest(LogCaptureTestCase):
 
@@ -297,18 +297,21 @@ class CommandActionTest(LogCaptureTestCase):
 				"Text 000-567 text 567 '567'")
 		self.assertTrue(len(cache) >= 3)
 
-
+	@with_tmpdir
-	def testExecuteActionBan(self):
+	def testExecuteActionBan(self, tmp):
-		self.__action.actionstart = "touch /tmp/fail2ban.test"
+		tmp += "/fail2ban.test"
-		self.assertEqual(self.__action.actionstart, "touch /tmp/fail2ban.test")
+		self.__action.actionstart = "touch '%s'" % tmp
-		self.__action.actionstop = "rm -f /tmp/fail2ban.test"
+		self.__action.actionrepair = self.__action.actionstart
-		self.assertEqual(self.__action.actionstop, 'rm -f /tmp/fail2ban.test')
+		self.assertEqual(self.__action.actionstart, "touch '%s'" % tmp)
+		self.__action.actionstop = "rm -f '%s'" % tmp
+		self.assertEqual(self.__action.actionstop, "rm -f '%s'" % tmp)
 		self.__action.actionban = "echo -n"
 		self.assertEqual(self.__action.actionban, 'echo -n')
-		self.__action.actioncheck = "[ -e /tmp/fail2ban.test ]"
+		self.__action.actioncheck = "[ -e '%s' ]" % tmp
-		self.assertEqual(self.__action.actioncheck, '[ -e /tmp/fail2ban.test ]')
+		self.assertEqual(self.__action.actioncheck, "[ -e '%s' ]" % tmp)
 		self.__action.actionunban = "true"
 		self.assertEqual(self.__action.actionunban, 'true')
+		self.pruneLog()
 
 		self.assertNotLogged('returned')
 		# no action was actually executed yet
@@ -316,42 +319,66 @@ class CommandActionTest(LogCaptureTestCase):
 		self.__action.ban({'ip': None})
 		self.assertLogged('Invariant check failed')
 		self.assertLogged('returned successfully')
+		self.__action.stop()
+		self.assertLogged(self.__action.actionstop)
 
 	def testExecuteActionEmptyUnban(self):
+		# unban will be executed for actions with banned items only:
+		self.__action.actionban = ""
 		self.__action.actionunban = ""
+		self.__action.actionflush = "echo -n 'flush'"
+		self.__action.actionstop = "echo -n 'stop'"
+		self.__action.start();
+		self.__action.ban({});
+		self.pruneLog()
 		self.__action.unban({})
-		self.assertLogged('Nothing to do')
+		self.assertLogged('Nothing to do', wait=True)
+		# same as above but with interim flush, so no unban anymore:
+		self.__action.ban({});
+		self.pruneLog('[phase 2]')
+		self.__action.flush()
+		self.__action.unban({})
+		self.__action.stop()
+		self.assertLogged('stop', wait=True)
+		self.assertNotLogged('Nothing to do')
 
-	def testExecuteActionStartCtags(self):
+	@with_tmpdir
+	def testExecuteActionStartCtags(self, tmp):
+		tmp += '/fail2ban.test'
 		self.__action.HOST = "192.0.2.0"
-		self.__action.actionstart = "touch /tmp/fail2ban.test.<HOST>"
+		self.__action.actionstart = "touch '%s.<HOST>'" % tmp
-		self.__action.actionstop = "rm -f /tmp/fail2ban.test.<HOST>"
+		self.__action.actionstop = "rm -f '%s.<HOST>'" % tmp
-		self.__action.actioncheck = "[ -e /tmp/fail2ban.test.192.0.2.0 ]"
+		self.__action.actioncheck = "[ -e '%s.192.0.2.0' ]" % tmp
 		self.__action.start()
+		self.__action.consistencyCheck()
 
-	def testExecuteActionCheckRestoreEnvironment(self):
+	@with_tmpdir
+	def testExecuteActionCheckRestoreEnvironment(self, tmp):
+		tmp += '/fail2ban.test'
 		self.__action.actionstart = ""
-		self.__action.actionstop = "rm -f /tmp/fail2ban.test"
+		self.__action.actionstop = "rm -f '%s'" % tmp
-		self.__action.actionban = "rm /tmp/fail2ban.test"
+		self.__action.actionban = "rm '%s'" % tmp
-		self.__action.actioncheck = "[ -e /tmp/fail2ban.test ]"
+		self.__action.actioncheck = "[ -e '%s' ]" % tmp
 		self.assertRaises(RuntimeError, self.__action.ban, {'ip': None})
 		self.assertLogged('Invariant check failed', 'Unable to restore environment', all=True)
 		# 2nd time, try to restore with producing error in stop, but succeeded start hereafter:
 		self.pruneLog('[phase 2]')
-		self.__action.actionstart = "touch /tmp/fail2ban.test"
+		self.__action.actionstart = "touch '%s'" % tmp
-		self.__action.actionstop = "rm /tmp/fail2ban.test"
+		self.__action.actionstop = "rm '%s'" % tmp
-		self.__action.actionban = 'printf "%%b\n" <ip> >> /tmp/fail2ban.test'
+		self.__action.actionban = """printf "%%%%b\n" <ip> >> '%s'""" % tmp
-		self.__action.actioncheck = "[ -e /tmp/fail2ban.test ]"
+		self.__action.actioncheck = "[ -e '%s' ]" % tmp
 		self.__action.ban({'ip': None})
 		self.assertLogged('Invariant check failed')
 		self.assertNotLogged('Unable to restore environment')
 
-	def testExecuteActionCheckRepairEnvironment(self):
+	@with_tmpdir
+	def testExecuteActionCheckRepairEnvironment(self, tmp):
+		tmp += '/fail2ban.test'
 		self.__action.actionstart = ""
 		self.__action.actionstop = ""
-		self.__action.actionban = "rm /tmp/fail2ban.test"
+		self.__action.actionban = "rm '%s'" % tmp
-		self.__action.actioncheck = "[ -e /tmp/fail2ban.test ]"
+		self.__action.actioncheck = "[ -e '%s' ]" % tmp
-		self.__action.actionrepair = "echo 'repair ...'; touch /tmp/fail2ban.test"
+		self.__action.actionrepair = "echo 'repair ...'; touch '%s'" % tmp
 		# 1st time with success repair:
 		self.__action.ban({'ip': None})
 		self.assertLogged("Invariant check failed. Trying", "echo 'repair ...'", all=True)
@@ -379,13 +406,13 @@ class CommandActionTest(LogCaptureTestCase):
 				'user': "tester"
 			}
 		})
-		self.__action.actionban = "touch /tmp/fail2ban.test.123; echo 'failure <F-ID> of <F-USER> -<F-TEST>- from <ip>:<F-PORT>'"
+		self.__action.actionban = "echo '<ABC>, failure <F-ID> of <F-USER> -<F-TEST>- from <ip>:<F-PORT>'"
-		self.__action.actionunban = "rm /tmp/fail2ban.test.<ABC>; echo 'user <F-USER> unbanned'"
+		self.__action.actionunban = "echo '<ABC>, user <F-USER> unbanned'"
 		self.__action.ban(aInfo)
 		self.__action.unban(aInfo)
 		self.assertLogged(
-			" -- stdout: 'failure 111 of tester -- from 192.0.2.1:222'",
+			" -- stdout: '123, failure 111 of tester -- from 192.0.2.1:222'",
-			" -- stdout: 'user tester unbanned'",
+			" -- stdout: '123, user tester unbanned'",
 			all=True
 		)
 
@@ -560,6 +587,19 @@ class CommandActionTest(LogCaptureTestCase):
 		self.assertEqual(len(m), 3)
 		self.assertIn('c', m)
 		self.assertEqual((m['a'], m['b'], m['c']), (5, 11, 'test'))
+		# immutability of copy:
+		m['d'] = 'dddd'
+		m2 = m.copy()
+		m2['c'] = lambda self: self['a'] + 7
+		m2['a'] = 1
+		del m2['b']
+		del m2['d']
+		self.assertTrue('b' in m)
+		self.assertTrue('d' in m)
+		self.assertFalse('b' in m2)
+		self.assertFalse('d' in m2)
+		self.assertEqual((m['a'], m['b'], m['c'], m['d']), (5, 11, 'test', 'dddd'))
+		self.assertEqual((m2['a'], m2['c']), (1, 8))
 
 	def testCallingMapRep(self):
 		m = CallingMap({

fail2ban/tests/clientreadertestcase.py

@@ -31,7 +31,7 @@ import unittest
 from ..client.configreader import ConfigReader, ConfigReaderUnshared, \
 	DefinitionInitConfigReader, NoSectionError
 from ..client import configparserinc
-from ..client.jailreader import JailReader, extractOptions
+from ..client.jailreader import JailReader, extractOptions, splitWithOptions
 from ..client.filterreader import FilterReader
 from ..client.jailsreader import JailsReader
 from ..client.actionreader import ActionReader, CommandAction
@@ -778,7 +778,7 @@ class JailsReaderTest(LogCaptureTestCase):
 
 			# somewhat duplicating here what is done in JailsReader if
 			# the jail is enabled
-			for act in actions.split('\n'):
+			for act in splitWithOptions(actions):
 				actName, actOpt = extractOptions(act)
 				self.assertTrue(len(actName))
 				self.assertTrue(isinstance(actOpt, dict))

fail2ban/tests/fail2banclienttestcase.py

@@ -113,16 +113,7 @@ fail2banclient.input_command = _test_input_command
 fail2bancmdline.PRODUCTION = \
 fail2banserver.PRODUCTION = False
 
-
+_out_file = LogCaptureTestCase.dumpFile
-def _out_file(fn, handle=logSys.debug):
-	"""Helper which outputs content of the file at HEAVYDEBUG loglevels"""
-	if (handle != logSys.debug or logSys.getEffectiveLevel() <= logging.DEBUG):
-		handle('---- ' + fn + ' ----')
-		for line in fileinput.input(fn):
-			line = line.rstrip('\n')
-			handle(line)
-		handle('-'*30)
-
 
 def _write_file(fn, mode, *lines):
 	f = open(fn, mode)

fail2ban/tests/files/action.d/action_modifyainfo.py

@@ -12,4 +12,9 @@ class TestAction(ActionBase):
         del aInfo['ip']
         self._logSys.info("%s unban deleted aInfo IP", self._name)
 
+    def flush(self):
+        # intended error to cover no unhandled exception occurs in flush
+        # as well as unbans are done individually after errored flush.
+        raise ValueError("intended error")
+
 Action = TestAction

fail2ban/tests/filtertestcase.py

@@ -40,7 +40,7 @@ from ..server.jail import Jail
 from ..server.filterpoll import FilterPoll
 from ..server.filter import FailTicket, Filter, FileFilter, FileContainer
 from ..server.failmanager import FailManagerEmpty
-from ..server.ipdns import getfqdn, DNSUtils, IPAddr
+from ..server.ipdns import asip, getfqdn, DNSUtils, IPAddr
 from ..server.mytime import MyTime
 from ..server.utils import Utils, uni_decode
 from .utils import setUpMyTime, tearDownMyTime, mtimesleep, with_tmpdir, LogCaptureTestCase, \
@@ -1843,11 +1843,15 @@ class DNSUtilsNetworkTests(unittest.TestCase):
 	def setUp(self):
 		"""Call before every test case."""
 		super(DNSUtilsNetworkTests, self).setUp()
-		unittest.F2B.SkipIfNoNetwork()
+		#unittest.F2B.SkipIfNoNetwork()
 
 	def test_IPAddr(self):
-		self.assertTrue(IPAddr('192.0.2.1').isIPv4)
+		ip4 = IPAddr('192.0.2.1')
-		self.assertTrue(IPAddr('2001:DB8::').isIPv6)
+		ip6 = IPAddr('2001:DB8::')
+		self.assertTrue(ip4.isIPv4)
+		self.assertTrue(ip6.isIPv6)
+		self.assertTrue(asip('192.0.2.1').isIPv4)
+		self.assertTrue(id(asip(ip4)) == id(ip4))
 
 	def test_IPAddr_Raw(self):
 		# raw string:
@@ -1884,6 +1888,7 @@ class DNSUtilsNetworkTests(unittest.TestCase):
 	def testUseDns(self):
 		res = DNSUtils.textToIp('www.example.com', 'no')
 		self.assertSortedEqual(res, [])
+		unittest.F2B.SkipIfNoNetwork()
 		res = DNSUtils.textToIp('www.example.com', 'warn')
 		# sort ipaddr, IPv4 is always smaller as IPv6
 		self.assertSortedEqual(res, ['93.184.216.34', '2606:2800:220:1:248:1893:25c8:1946'])
@@ -1892,6 +1897,7 @@ class DNSUtilsNetworkTests(unittest.TestCase):
 		self.assertSortedEqual(res, ['93.184.216.34', '2606:2800:220:1:248:1893:25c8:1946'])
 
 	def testTextToIp(self):
+		unittest.F2B.SkipIfNoNetwork()
 		# Test hostnames
 		hostnames = [
 			'www.example.com',
@@ -1905,6 +1911,8 @@ class DNSUtilsNetworkTests(unittest.TestCase):
 				self.assertSortedEqual(res, ['93.184.216.34', '2606:2800:220:1:248:1893:25c8:1946'])
 			else:
 				self.assertSortedEqual(res, [])
+
+	def testIpToIp(self):
 		# pure ips:
 		for s in ('93.184.216.34', '2606:2800:220:1:248:1893:25c8:1946'):
 			ips = DNSUtils.textToIp(s, 'yes')
@@ -2061,6 +2069,7 @@ class DNSUtilsNetworkTests(unittest.TestCase):
 		self.assertTrue(IPAddr("2606:2800:220:1:248:1893:25c8:1946").isInNet(ips))
 
 	def testIPAddr_wrongDNS_IP(self):
+		unittest.F2B.SkipIfNoNetwork()
 		DNSUtils.dnsToIp('`this`.dns-is-wrong.`wrong-nic`-dummy')
 		DNSUtils.ipToName('*')
 
@@ -2083,6 +2092,9 @@ class DNSUtilsNetworkTests(unittest.TestCase):
 		self.assertEqual(getfqdn(lname), lname)
 		# coverage (targeting all branches): FQDN from loopback and DNS blackhole is always the same:
 		self.assertIn(getfqdn('localhost.'), ('localhost', 'localhost.'))
+	
+	def testFQDN_DNS(self):
+		unittest.F2B.SkipIfNoNetwork()
 		self.assertIn(getfqdn('as112.arpa.'), ('as112.arpa.', 'as112.arpa'))
 
 

fail2ban/tests/servertestcase.py

@@ -752,7 +752,7 @@ class Transmitter(TransmitterBase):
 		self.assertSortedEqual(
 			self.transm.proceed(["get", self.jailName, "actionmethods",
 				action])[1],
-			['ban', 'start', 'stop', 'testmethod', 'unban'])
+			['ban', 'reban', 'start', 'stop', 'testmethod', 'unban'])
 		self.assertEqual(
 			self.transm.proceed(["set", self.jailName, "action", action,
 				"testmethod", '{"text": "world!"}']),

fail2ban/tests/utils.py

@@ -22,6 +22,7 @@ __author__ = "Yaroslav Halchenko"
 __copyright__ = "Copyright (c) 2013 Yaroslav Halchenko"
 __license__ = "GPL"
 
+import fileinput
 import itertools
 import logging
 import optparse
@@ -741,11 +742,8 @@ class LogCaptureTestCase(unittest.TestCase):
 				self._dirty |= 2 # records changed
 
 	def setUp(self):
-
 		# For extended testing of what gets output into logging
 		# system, we will redirect it to a string
-		logSys = getLogger("fail2ban")
-
 		# Keep old settings
 		self._old_level = logSys.level
 		self._old_handlers = logSys.handlers
@@ -762,7 +760,6 @@ class LogCaptureTestCase(unittest.TestCase):
 		"""Call after every test case."""
 		# print "O: >>%s<<" % self._log.getvalue()
 		self.pruneLog()
-		logSys = getLogger("fail2ban")
 		logSys.handlers = self._old_handlers
 		logSys.level = self._old_level
 		super(LogCaptureTestCase, self).tearDown()
@@ -845,5 +842,15 @@ class LogCaptureTestCase(unittest.TestCase):
 	def getLog(self):
 		return self._log.getvalue()
 
+	@staticmethod
+	def dumpFile(fn, handle=logSys.debug):
+		"""Helper which outputs content of the file at HEAVYDEBUG loglevels"""
+		if (handle != logSys.debug or logSys.getEffectiveLevel() <= logging.DEBUG):
+			handle('---- ' + fn + ' ----')
+			for line in fileinput.input(fn):
+				line = line.rstrip('\n')
+				handle(line)
+			handle('-'*30)
+
 
 pid_exists = Utils.pid_exists