From 3d3735706b71da567de00b0158beba8915da4c94 Mon Sep 17 00:00:00 2001 From: sebres Date: Thu, 12 May 2016 20:21:42 +0200 Subject: [PATCH] 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 | 14 +++++--- fail2ban/tests/actiontestcase.py | 7 ++++ fail2ban/tests/servertestcase.py | 57 +++++++++++++++++++++++++++++++- 3 files changed, 73 insertions(+), 5 deletions(-) diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index cdd4e28c..ba3d4633 100644 --- a/fail2ban/server/action.py +++ b/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? diff --git a/fail2ban/tests/actiontestcase.py b/fail2ban/tests/actiontestcase.py index f3778022..00a4d6be 100644 --- a/fail2ban/tests/actiontestcase.py +++ b/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= fromip=', 'C': '', 'B': '', 'D': ''})) self.assertRaises(ValueError, lambda: CommandAction.substituteRecursiveTags({'failregex': 'to= fromip=', 'sweet': '', 'honeypot': '', 'ignoreregex': ''})) + # No-recursion, just multiple replacement of tag , 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', '1'), ('Z', ' '), ('Y', 'y=y'))) + ), {'X': 'x=x1', 'T': '1', 'Y': 'y=y1', 'Z': 'x=x1 1 y=y1'} + ) # missing tags are ok self.assertEqual(CommandAction.substituteRecursiveTags({'A': ''}), {'A': ''}) self.assertEqual(CommandAction.substituteRecursiveTags({'A': ' ','X':'fun'}), {'A': ' fun', 'X':'fun'}) diff --git a/fail2ban/tests/servertestcase.py b/fail2ban/tests/servertestcase.py index 214520a6..cc14e97f 100644 --- a/fail2ban/tests/servertestcase.py +++ b/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)