From b3266ba44d3311a35bd18a3055a77dc27f577085 Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Sat, 3 May 2014 14:28:13 +0100 Subject: [PATCH 1/3] BF: Tags not fully recursively substituted Note: recursive check ignored for "matches", as tags would be escaped, and hence shouldn't match "<%s>" as "" would become "\". This therefore maintains advantage of delayed call for {ip,jail,}matches. Fixes gh-713 --- fail2ban/server/action.py | 11 +++++++++-- fail2ban/tests/actiontestcase.py | 13 ++++++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index 0098c546..d1883338 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -371,8 +371,11 @@ class CommandAction(ActionBase): within the values recursively replaced. """ t = re.compile(r'<([^ >]+)>') - for tag, value in tags.iteritems(): - value = str(value) + for tag in tags.iterkeys(): + if tag.endswith('matches'): + # Escapped so wont match + continue + value = str(tags[tag]) m = t.search(value) done = [] #logSys.log(5, 'TAG: %s, value: %s' % (tag, value)) @@ -383,6 +386,9 @@ class CommandAction(ActionBase): # recursive definitions are bad #logSys.log(5, 'recursion fail tag: %s value: %s' % (tag, value) ) return False + elif found_tag.endswith('matches'): + # Escapped so wont match + continue else: if tags.has_key(found_tag): value = value.replace('<%s>' % found_tag , tags[found_tag]) @@ -441,6 +447,7 @@ class CommandAction(ActionBase): `query` string with tags replaced. """ string = query + aInfo = cls.substituteRecursiveTags(aInfo) for tag in aInfo: if "<%s>" % tag in query: value = str(aInfo[tag]) # assure string diff --git a/fail2ban/tests/actiontestcase.py b/fail2ban/tests/actiontestcase.py index cb004b4d..f1ea77ce 100644 --- a/fail2ban/tests/actiontestcase.py +++ b/fail2ban/tests/actiontestcase.py @@ -100,17 +100,24 @@ class CommandActionTest(LogCaptureTestCase): {'ipjailmatches': "some >char< should \< be[ escap}ed&\n"}), "some \\>char\\< should \\\\\\< be\\[ escap\\}ed\\&\n") + + # Recursive + aInfo["ABC"] = "" + self.assertEqual( + self.__action.replaceTag("Text text ABC", aInfo), + "Text 890 text 890 ABC") + # Callable self.assertEqual( - self.__action.replaceTag("09 11", - CallingMap(callme=lambda: str(10))), + self.__action.replaceTag("09 11", + CallingMap(matches=lambda: str(10))), "09 10 11") # As tag not present, therefore callable should not be called # Will raise ValueError if it is self.assertEqual( self.__action.replaceTag("abc", - CallingMap(callme=lambda: int("a"))), "abc") + CallingMap(matches=lambda: int("a"))), "abc") def testExecuteActionBan(self): self.__action.actionstart = "touch /tmp/fail2ban.test" From 904b362215e587c1038e556681a787cea61eeccc Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Fri, 9 May 2014 20:25:44 +0100 Subject: [PATCH 2/3] DOC: ChangeLog update for recursive tag bug fix Also minor typo fixes in comments --- ChangeLog | 2 ++ fail2ban/server/action.py | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index bbe32054..ec6a3b28 100644 --- a/ChangeLog +++ b/ChangeLog @@ -22,6 +22,8 @@ ver. 0.9.1 (2014/xx/xx) - better, faster, stronger * Nginx filter to support missing server_name. Closes gh-676 * fail2ban-regex assertion error caused by miscount missed lines with multiline regex + * Recursive action tags now fully processed. Fixes issue with bsd-ipfw + action - New features: diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index d1883338..736386b1 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -373,7 +373,7 @@ class CommandAction(ActionBase): t = re.compile(r'<([^ >]+)>') for tag in tags.iterkeys(): if tag.endswith('matches'): - # Escapped so wont match + # Escapped so won't match continue value = str(tags[tag]) m = t.search(value) @@ -387,7 +387,7 @@ class CommandAction(ActionBase): #logSys.log(5, 'recursion fail tag: %s value: %s' % (tag, value) ) return False elif found_tag.endswith('matches'): - # Escapped so wont match + # Escapped so won't match continue else: if tags.has_key(found_tag): From 1e586fb0e94248cd0185e2df9a33dbbef9734299 Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Sun, 11 May 2014 14:49:49 +0100 Subject: [PATCH 3/3] ENH: explicitly define tags which should be escaped --- fail2ban/server/action.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index 736386b1..fefe2c2c 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -194,6 +194,8 @@ class CommandAction(ActionBase): timeout """ + _escapedTags = set(('matches', 'ipmatches', 'ipjailmatches')) + def __init__(self, jail, name): super(CommandAction, self).__init__(jail, name) self.timeout = 60 @@ -351,8 +353,8 @@ class CommandAction(ActionBase): if not self.executeCmd(stopCmd, self.timeout): raise RuntimeError("Error stopping action") - @staticmethod - def substituteRecursiveTags(tags): + @classmethod + def substituteRecursiveTags(cls, tags): """Sort out tag definitions within other tags. so: becomes: @@ -372,8 +374,8 @@ class CommandAction(ActionBase): """ t = re.compile(r'<([^ >]+)>') for tag in tags.iterkeys(): - if tag.endswith('matches'): - # Escapped so won't match + if tag in cls._escapedTags: + # Escaped so won't match continue value = str(tags[tag]) m = t.search(value) @@ -386,8 +388,8 @@ class CommandAction(ActionBase): # recursive definitions are bad #logSys.log(5, 'recursion fail tag: %s value: %s' % (tag, value) ) return False - elif found_tag.endswith('matches'): - # Escapped so won't match + elif found_tag in cls._escapedTags: + # Escaped so won't match continue else: if tags.has_key(found_tag): @@ -451,7 +453,7 @@ class CommandAction(ActionBase): for tag in aInfo: if "<%s>" % tag in query: value = str(aInfo[tag]) # assure string - if tag.endswith('matches'): + if tag in cls._escapedTags: # That one needs to be escaped since its content is # out of our control value = cls.escapeTag(value)