invalid recursion check in substituteRecursiveTags: for example action `bsd-ipfw` produced ValueError('properties contain self referencing definitions and cannot be resolved...')

test cases extended for exactly this case and for all stock actions;
closes gh-1417

fail2ban/server/action.py

@@ -366,17 +366,22 @@ class CommandAction(ActionBase):
 				value = str(tags[tag])
 				# search and replace all tags within value, that can be interpolated using other tags:
 				m = t.search(value)
-				done = []
+				done = {}
+				last_found = tag
 				#logSys.log(5, 'TAG: %s, value: %s' % (tag, value))
 				while m:
 					found_tag = m.group(1)
 					#logSys.log(5, 'found: %s' % found_tag)
-					if found_tag == tag or found_tag in done:
+					curdone = done.get(last_found)
+					if curdone is None:
+						done[last_found] = curdone = []
+					if found_tag == tag or found_tag in curdone:
 						# recursive definitions are bad
 						#logSys.log(5, 'recursion fail tag: %s value: %s' % (tag, value) )
 						raise ValueError(
 							"properties contain self referencing definitions "
-							"and cannot be resolved, fail tag: %s value: %s" % (tag, value))
+							"and cannot be resolved, fail tag: %s, found: %s in %s, value: %s" % 
+							(tag, found_tag, curdone, value))
 					repl = None
 					if found_tag not in cls._escapedTags:
 						repl = tags.get(found_tag + '?' + conditional)
@@ -390,7 +395,8 @@ class CommandAction(ActionBase):
 						continue
 					value = value.replace('<%s>' % found_tag, repl)
 					#logSys.log(5, 'value now: %s' % value)
-					done.append(found_tag)
+					curdone.append(found_tag)
+					last_found = found_tag
 					m = t.search(value, m.start())
 				#logSys.log(5, 'TAG: %s, newvalue: %s' % (tag, value))
 				# was substituted?

fail2ban/tests/actiontestcase.py

@@ -30,6 +30,7 @@ import time
 import unittest
 
 from ..server.action import CommandAction, CallingMap
+from ..server.actions import OrderedDict
 from ..server.utils import Utils
 
 from .utils import LogCaptureTestCase
@@ -65,6 +66,12 @@ class CommandActionTest(LogCaptureTestCase):
 			lambda: CommandAction.substituteRecursiveTags({'A': 'to=<B> fromip=<IP>', 'C': '<B>', 'B': '<C>', 'D': ''}))
 		self.assertRaises(ValueError,
 			lambda: CommandAction.substituteRecursiveTags({'failregex': 'to=<honeypot> fromip=<IP>', 'sweet': '<honeypot>', 'honeypot': '<sweet>', 'ignoreregex': ''}))
+		# No-recursion, just multiple replacement of tag <T>, should be successful
+		if OrderedDict: # we need here an ordered, because the sequence of iteration is very important for this test
+			self.assertEqual(CommandAction.substituteRecursiveTags(
+				OrderedDict((('X', 'x=x<T>'), ('T', '1'), ('Z', '<X> <T> <Y>'), ('Y', 'y=y<T>')))
+				), {'X': 'x=x1', 'T': '1', 'Y': 'y=y1', 'Z': 'x=x1 1 y=y1'}
+			)
 		# missing tags are ok
 		self.assertEqual(CommandAction.substituteRecursiveTags({'A': '<C>'}), {'A': '<C>'})
 		self.assertEqual(CommandAction.substituteRecursiveTags({'A': '<C> <D> <X>','X':'fun'}), {'A': '<C> <D> fun', 'X':'fun'})

fail2ban/tests/servertestcase.py

@@ -998,6 +998,38 @@ class ServerConfigReaderTests(LogCaptureTestCase):
 		self.assertTrue(IPAddr('192.0.2.1').isIPv4)
 		self.assertTrue(IPAddr('2001:DB8::').isIPv6)
 
+	def _testExecActions(self, server):
+		jails = server._Server__jails
+		for jail in jails:
+			# print(jail, jails[jail])
+			for a in jails[jail].actions:
+				action = jails[jail].actions[a]
+				logSys.debug('# ' + ('=' * 50))
+				logSys.debug('# == %-44s ==', jail + ' - ' + action._name)
+				logSys.debug('# ' + ('=' * 50))
+				# we can currently test only command actions:
+				if not isinstance(action, _actions.CommandAction): continue
+				# wrap default command processor, just log if (heavy)debug:
+				action.executeCmd = self._executeCmd
+				# test start :
+				logSys.debug('# === start ==='); self.pruneLog()
+				action.start()
+				# test ban ip4 :
+				logSys.debug('# === ban-ipv4 ==='); self.pruneLog()
+				action.ban({'ip': IPAddr('192.0.2.1')})
+				# test unban ip4 :
+				logSys.debug('# === unban ipv4 ==='); self.pruneLog()
+				action.unban({'ip': IPAddr('192.0.2.1')})
+				# test ban ip6 :
+				logSys.debug('# === ban ipv6 ==='); self.pruneLog()
+				action.ban({'ip': IPAddr('2001:DB8::')})
+				# test unban ip6 :
+				logSys.debug('# === unban ipv6 ==='); self.pruneLog()
+				action.unban({'ip': IPAddr('2001:DB8::')})
+				# test stop :
+				logSys.debug('# === stop ==='); self.pruneLog()
+				action.stop()
+
 	if STOCK:
 
 		def testCheckStockJailActions(self):
@@ -1046,6 +1078,10 @@ class ServerConfigReaderTests(LogCaptureTestCase):
 			# for j in jails:
 			# 	print(j, jails[j])
 
+			# test default stock actions sepecified in all stock jails:
+			if not unittest.F2B.fast:
+				self._testExecActions(server)
+
 		def getDefaultJailStream(self, jail, act):
 			act = act.replace('%(__name__)s', jail)
 			actName, actOpt = JailReader.extractOptions(act)
@@ -1061,6 +1097,25 @@ class ServerConfigReaderTests(LogCaptureTestCase):
 			stream.extend(action.convert())
 			return stream
 
+		def testCheckStockAllActions(self):
+			unittest.F2B.SkipIfFast()
+			import glob
+
+			server = TestServer()
+			transm = server._Server__transm
+
+			for actCfg in glob.glob(os.path.join(CONFIG_DIR, 'action.d', '*.conf')):
+				act = os.path.basename(actCfg).replace('.conf', '')
+				# transmit artifical jail with each action to the server:
+				stream = self.getDefaultJailStream('j-'+act, act)
+				for cmd in stream:
+					# command to server:
+					ret, res = transm.proceed(cmd)
+					self.assertEqual(ret, 0)
+				# test executing action commands:
+				self._testExecActions(server)
+
+
 		def testCheckStockCommandActions(self):
 			# test cases to check valid ipv4/ipv6 action definition, tuple with (('jail', 'action[params]', 'tests', ...)
 			# where tests is a dictionary contains:
@@ -1347,7 +1402,7 @@ class ServerConfigReaderTests(LogCaptureTestCase):
 				# for cmd in stream:
 				# 	print(cmd)
 
-				# filter all start commands (we want not start all jails):
+				# transmit jail to the server:
 				for cmd in stream:
 					# command to server:
 					ret, res = transm.proceed(cmd)