From a4b8f6e49e81340f3fdc31d2dd92935ea22160f5 Mon Sep 17 00:00:00 2001 From: sebres Date: Thu, 12 May 2016 20:21:42 +0200 Subject: [PATCH 1/2] [part. cherry-picked from 0.10] 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; closes gh-1417 --- fail2ban/server/action.py | 11 ++++++++--- fail2ban/tests/actiontestcase.py | 7 +++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index de0c8efc..b1af659e 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -400,12 +400,16 @@ 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) ) return False @@ -417,7 +421,8 @@ class CommandAction(ActionBase): continue value = value.replace('<%s>' % found_tag , tags[found_tag]) #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 289d8896..e20a6341 100644 --- a/fail2ban/tests/actiontestcase.py +++ b/fail2ban/tests/actiontestcase.py @@ -29,6 +29,7 @@ import time import tempfile from ..server.action import CommandAction, CallingMap +from ..server.actions import OrderedDict from .utils import LogCaptureTestCase from .utils import pid_exists @@ -58,6 +59,12 @@ class CommandActionTest(LogCaptureTestCase): # Unresolveable substition self.assertFalse(CommandAction.substituteRecursiveTags({'A': 'to= fromip=', 'C': '', 'B': '', 'D': ''})) self.assertFalse(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'}) From cce63926ce9bb7b96fa1820e7893af67e0ac021a Mon Sep 17 00:00:00 2001 From: sebres Date: Fri, 13 May 2016 14:37:48 +0200 Subject: [PATCH 2/2] ChangeLog entry added --- ChangeLog | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ChangeLog b/ChangeLog index 033cd9ec..9ac9a752 100644 --- a/ChangeLog +++ b/ChangeLog @@ -15,6 +15,9 @@ ver. 0.9.5 (2016/XX/XXX) - wanna-be-released - failregex of previous monit version merged as single expression. * filter.d/postfix.conf, filter.d/postfix-sasl.conf - extended failregex daemon part, matching also `postfix/smtps/smtpd` now (gh-1391) + * fixed a grave bug within tags substitutions because of incorrect detection of recursion + in case of multiple inline substitutions of the same tag (affected actions: `bsd-ipfw`, etc). + Now tracks the actual list of the already substituted tags (per tag instead of single list) - New Features: * New Actions: