From b04a51246f8fa4d1c87d34a3bc6b31369c968e1f Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 20 Jan 2015 11:32:15 +0100 Subject: [PATCH 1/4] infinite busy loop on _escapedTags match in substituteRecursiveTags gh-907 --- ChangeLog | 2 ++ fail2ban/server/action.py | 22 +++++++++------------- fail2ban/tests/actiontestcase.py | 2 ++ 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0a901547..c24c9dc8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -11,6 +11,8 @@ ver. 0.9.2 (2014/XX/XXX) - wanna-be-released ----------- - Fixes: + * infinite busy loop on _escapedTags match in substituteRecursiveTags gh-907. + Thanks TonyThompson * port[s] typo in jail.conf/nginx-http-auth gh-913. Thanks Frederik Wagner (fnerdwq) * $ typo in jail.conf. Thanks Skibbi. Debian bug #767255 * grep'ing for IP in *mail-whois-lines.conf should now match also diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index da7517f6..c69ba88f 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -394,20 +394,16 @@ class CommandAction(ActionBase): # recursive definitions are bad #logSys.log(5, 'recursion fail tag: %s value: %s' % (tag, value) ) return False - elif found_tag in cls._escapedTags: - # Escaped so won't match + if found_tag in cls._escapedTags or not tags.has_key(found_tag): + # Escaped or missing tags - just continue on searching after end of match + # Missing tags are ok - cInfo can contain aInfo elements like and valid shell + # constructs like . + m = t.search(value, m.end()) continue - else: - if tags.has_key(found_tag): - value = value.replace('<%s>' % found_tag , tags[found_tag]) - #logSys.log(5, 'value now: %s' % value) - done.append(found_tag) - m = t.search(value, m.start()) - else: - # Missing tags are ok so we just continue on searching. - # cInfo can contain aInfo elements like and valid shell - # constructs like . - m = t.search(value, m.start() + 1) + value = value.replace('<%s>' % found_tag , tags[found_tag]) + #logSys.log(5, 'value now: %s' % value) + done.append(found_tag) + m = t.search(value, m.start()) #logSys.log(5, 'TAG: %s, newvalue: %s' % (tag, value)) tags[tag] = value return tags diff --git a/fail2ban/tests/actiontestcase.py b/fail2ban/tests/actiontestcase.py index 5a58149f..36e0ddb8 100644 --- a/fail2ban/tests/actiontestcase.py +++ b/fail2ban/tests/actiontestcase.py @@ -59,6 +59,8 @@ class CommandActionTest(LogCaptureTestCase): self.assertEqual(CommandAction.substituteRecursiveTags({'A': ''}), {'A': ''}) self.assertEqual(CommandAction.substituteRecursiveTags({'A': ' ','X':'fun'}), {'A': ' fun', 'X':'fun'}) self.assertEqual(CommandAction.substituteRecursiveTags({'A': ' ', 'B': 'cool'}), {'A': ' cool', 'B': 'cool'}) + # Escaped tags should be ignored + self.assertEqual(CommandAction.substituteRecursiveTags({'A': ' ', 'B': 'cool'}), {'A': ' cool', 'B': 'cool'}) # Multiple stuff on same line is ok self.assertEqual(CommandAction.substituteRecursiveTags({'failregex': 'to= fromip= evilperson=', 'honeypot': 'pokie', 'ignoreregex': ''}), { 'failregex': "to=pokie fromip= evilperson=pokie", From 33e9e2174ad98466c6f33dd78ba7baf30f3a8340 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 20 Jan 2015 17:09:13 +0100 Subject: [PATCH 2/4] recursive/embedded version of issue/907; test cases merged from remote-tracking branch 'yarikoptic:enh/embedded_tags' into issue/907 infinite busy loop on _escapedTags match in substituteRecursiveTags gh-907 --- ChangeLog | 3 ++ fail2ban/server/action.py | 61 ++++++++++++++++++-------------- fail2ban/tests/actiontestcase.py | 5 +++ 3 files changed, 42 insertions(+), 27 deletions(-) diff --git a/ChangeLog b/ChangeLog index c24c9dc8..f01ed930 100644 --- a/ChangeLog +++ b/ChangeLog @@ -30,6 +30,9 @@ ver. 0.9.2 (2014/XX/XXX) - wanna-be-released - New Features: - New filter: - postfix-rbl Thanks Lee Clemens + - New recursive embedded substitution feature added: + - `<HOST>` becomes `` for PREF=`IPV4`; + - `<HOST>` becomes `1.2.3.4` for PREF=`IPV4` and IPV4HOST=`1.2.3.4`; - New interpolation feature for config readers - `%(known/parameter)s`. (means last known option with name `parameter`). This interpolation makes possible to extend a stock filter or jail regexp in .local file diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index c69ba88f..bfd2adc2 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -378,34 +378,41 @@ class CommandAction(ActionBase): Dictionary of tags(keys) and their values, with tags within the values recursively replaced. """ - t = re.compile(r'<([^ >]+)>') - for tag in tags.iterkeys(): - if tag in cls._escapedTags: - # Escaped so won't match - continue - value = str(tags[tag]) - m = t.search(value) - done = [] - #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: - # recursive definitions are bad - #logSys.log(5, 'recursion fail tag: %s value: %s' % (tag, value) ) - return False - if found_tag in cls._escapedTags or not tags.has_key(found_tag): - # Escaped or missing tags - just continue on searching after end of match - # Missing tags are ok - cInfo can contain aInfo elements like and valid shell - # constructs like . - m = t.search(value, m.end()) + t = re.compile(r'<([^ <>]+)>') + while True: + repFlag = False + for tag in tags.iterkeys(): + if tag in cls._escapedTags: + # Escaped so won't match continue - value = value.replace('<%s>' % found_tag , tags[found_tag]) - #logSys.log(5, 'value now: %s' % value) - done.append(found_tag) - m = t.search(value, m.start()) - #logSys.log(5, 'TAG: %s, newvalue: %s' % (tag, value)) - tags[tag] = value + value = str(tags[tag]) + m = t.search(value) + done = [] + #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: + # recursive definitions are bad + #logSys.log(5, 'recursion fail tag: %s value: %s' % (tag, value) ) + return False + if found_tag in cls._escapedTags or not tags.has_key(found_tag): + # Escaped or missing tags - just continue on searching after end of match + # Missing tags are ok - cInfo can contain aInfo elements like and valid shell + # constructs like . + m = t.search(value, m.end()) + continue + value = value.replace('<%s>' % found_tag , tags[found_tag]) + #logSys.log(5, 'value now: %s' % value) + done.append(found_tag) + m = t.search(value, m.start()) + #logSys.log(5, 'TAG: %s, newvalue: %s' % (tag, value)) + # if was substituted, check again later: + if tags[tag] != value: + repFlag = True + tags[tag] = value + if not repFlag: + break return tags @staticmethod diff --git a/fail2ban/tests/actiontestcase.py b/fail2ban/tests/actiontestcase.py index 36e0ddb8..7cd0cd4d 100644 --- a/fail2ban/tests/actiontestcase.py +++ b/fail2ban/tests/actiontestcase.py @@ -73,6 +73,11 @@ class CommandActionTest(LogCaptureTestCase): 'ABC': '123 192.0.2.0', 'xyz': '890 123 192.0.2.0', }) + # obscure embedded case + self.assertEqual(CommandAction.substituteRecursiveTags({'A': '<HOST>', 'PREF': 'IPV4'}), + {'A': '', 'PREF': 'IPV4'}) + self.assertEqual(CommandAction.substituteRecursiveTags({'A': '<HOST>', 'PREF': 'IPV4', 'IPV4HOST': '1.2.3.4'}), + {'A': '1.2.3.4', 'PREF': 'IPV4', 'IPV4HOST': '1.2.3.4'}) def testReplaceTag(self): aInfo = { From 6b42878b8cea7a3d3ea8ad261a0e51f480d051e4 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 20 Jan 2015 17:31:17 +0100 Subject: [PATCH 3/4] better recognition of embedded-recursive substitution to repeat interpolation --- fail2ban/server/action.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index bfd2adc2..70b3eb7e 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -407,9 +407,11 @@ class CommandAction(ActionBase): done.append(found_tag) m = t.search(value, m.start()) #logSys.log(5, 'TAG: %s, newvalue: %s' % (tag, value)) - # if was substituted, check again later: + # was substituted? if tags[tag] != value: - repFlag = True + # check again later if embedded-recursive substitution: + if value.startswith('<') and value.endswith('>'): + repFlag = True tags[tag] = value if not repFlag: break From d0b932aaca91cb044861a3e61496c6c06b03c263 Mon Sep 17 00:00:00 2001 From: sebres Date: Wed, 21 Jan 2015 09:44:55 +0100 Subject: [PATCH 4/4] code review + more test cases (embedded replace in a string) --- fail2ban/server/action.py | 8 ++++++-- fail2ban/tests/actiontestcase.py | 3 +++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index 70b3eb7e..8e0cd97f 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -362,6 +362,7 @@ class CommandAction(ActionBase): @classmethod def substituteRecursiveTags(cls, tags): """Sort out tag definitions within other tags. + Since v.0.9.2 supports embedded interpolation (see test cases for examples). so: becomes: a = 3 a = 3 @@ -379,13 +380,16 @@ class CommandAction(ActionBase): within the values recursively replaced. """ t = re.compile(r'<([^ <>]+)>') + # repeat substitution while embedded-recursive (repFlag is True) while True: repFlag = False + # substitute each value: for tag in tags.iterkeys(): if tag in cls._escapedTags: # Escaped so won't match continue value = str(tags[tag]) + # search and replace all tags within value, that can be interpolated using other tags: m = t.search(value) done = [] #logSys.log(5, 'TAG: %s, value: %s' % (tag, value)) @@ -409,8 +413,8 @@ class CommandAction(ActionBase): #logSys.log(5, 'TAG: %s, newvalue: %s' % (tag, value)) # was substituted? if tags[tag] != value: - # check again later if embedded-recursive substitution: - if value.startswith('<') and value.endswith('>'): + # check still contains any tag - should be repeated (possible embedded-recursive substitution): + if t.search(value): repFlag = True tags[tag] = value if not repFlag: diff --git a/fail2ban/tests/actiontestcase.py b/fail2ban/tests/actiontestcase.py index 7cd0cd4d..2e826a8e 100644 --- a/fail2ban/tests/actiontestcase.py +++ b/fail2ban/tests/actiontestcase.py @@ -78,6 +78,9 @@ class CommandActionTest(LogCaptureTestCase): {'A': '', 'PREF': 'IPV4'}) self.assertEqual(CommandAction.substituteRecursiveTags({'A': '<HOST>', 'PREF': 'IPV4', 'IPV4HOST': '1.2.3.4'}), {'A': '1.2.3.4', 'PREF': 'IPV4', 'IPV4HOST': '1.2.3.4'}) + # more embedded within a string and two interpolations + self.assertEqual(CommandAction.substituteRecursiveTags({'A': 'A HOST> B IP C', 'PREF': 'V4', 'IPV4HOST': '1.2.3.4'}), + {'A': 'A 1.2.3.4 B IPV4 C', 'PREF': 'V4', 'IPV4HOST': '1.2.3.4'}) def testReplaceTag(self): aInfo = {