From 941a2b6c82a392c94dac7419a1a92e1600b5dbde Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 10 May 2016 20:00:24 +0200 Subject: [PATCH] clean up unnecessarily resp. directly unused action properties, because they are ambiguous now; implemented caching functionality for same substitutions inside replaceTag: very actual and extreme performance growth (up to 1000 times) for ban/unban because too slow substituteRecursiveTags by several tags and many includes, but totally unnecessary as long as parameters are not changing; --- fail2ban/server/action.py | 182 ++++++++++++++----------------- fail2ban/tests/actiontestcase.py | 42 +++++++ 2 files changed, 123 insertions(+), 101 deletions(-) diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index cfa03403..cdd4e28c 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -33,6 +33,7 @@ from abc import ABCMeta from collections import MutableMapping from .ipdns import asip +from .mytime import MyTime from .utils import Utils from ..helpers import getLogger @@ -203,36 +204,40 @@ class CommandAction(ActionBase): _escapedTags = set(('matches', 'ipmatches', 'ipjailmatches')) + timeout = 60 + ## Command executed in order to initialize the system. + actionstart = '' + ## Command executed when an IP address gets banned. + actionban = '' + ## Command executed when an IP address gets removed. + actionunban = '' + ## Command executed in order to check requirements. + actioncheck = '' + ## Command executed in order to stop the system. + actionstop = '' + def __init__(self, jail, name): super(CommandAction, self).__init__(jail, name) - self.timeout = 60 - ## Command executed in order to initialize the system. - self.actionstart = '' - ## Command executed when an IP address gets banned. - self.actionban = '' - ## Command executed when an IP address gets removed. - self.actionunban = '' - ## Command executed in order to check requirements. - self.actioncheck = '' - ## Command executed in order to stop the system. - self.actionstop = '' + self.__properties = None + self.__substCache = {} self._logSys.debug("Created %s" % self.__class__) @classmethod def __subclasshook__(cls, C): return NotImplemented # Standard checks - @property - def timeout(self): - """Time out period in seconds for execution of commands. - """ - return self._timeout - - @timeout.setter - def timeout(self, timeout): - self._timeout = int(timeout) - self._logSys.debug("Set action %s timeout = %i" % - (self._name, self.timeout)) + def __setattr__(self, name, value): + if not name.startswith('_') and not callable(value): + # special case for some pasrameters: + if name == 'timeout': + value = MyTime.str2seconds(value) + # parameters changed - clear properties and substitution cache: + self.__properties = None + self.__substCache.clear() + #self._logSys.debug("Set action %r %s = %r", self._name, name, value) + self._logSys.debug(" Set %s = %r", name, value) + # set: + self.__dict__[name] = value @property def _properties(self): @@ -240,21 +245,20 @@ class CommandAction(ActionBase): This is used to subsitute "tags" in the commands. """ - return dict( + # if we have a properties - return it: + if self.__properties is not None: + return self.__properties + # otherwise retrieve: + self.__properties = dict( (key, getattr(self, key)) for key in dir(self) if not key.startswith("_") and not callable(getattr(self, key))) + # + return self.__properties @property - def actionstart(self): - """The command executed on start of the jail/action. - """ - return self._actionstart - - @actionstart.setter - def actionstart(self, value): - self._actionstart = value - self._logSys.debug("Set actionstart = %s" % value) + def _substCache(self): + return self.__substCache def start(self): """Executes the "actionstart" command. @@ -263,29 +267,21 @@ class CommandAction(ActionBase): and executes the resulting command. """ # check valid tags in properties (raises ValueError if self recursion, etc.): - if self._properties: - self.substituteRecursiveTags(self._properties) - # common (resp. ipv4): - startCmd = self.replaceTag('', self._properties, conditional='family=inet4') - res = self.executeCmd(startCmd, self.timeout) - # start ipv6 actions if available: - if allowed_ipv6: - startCmd6 = self.replaceTag('', self._properties, conditional='family=inet6') - if startCmd6 != startCmd: - res &= self.executeCmd(startCmd6, self.timeout) - if not res: - raise RuntimeError("Error starting action") - - @property - def actionban(self): - """The command used when a ban occurs. - """ - return self._actionban - - @actionban.setter - def actionban(self, value): - self._actionban = value - self._logSys.debug("Set actionban = %s" % value) + try: + # common (resp. ipv4): + startCmd = self.replaceTag('', self._properties, + conditional='family=inet4', cache=self.__substCache) + res = self.executeCmd(startCmd, self.timeout) + # start ipv6 actions if available: + if allowed_ipv6: + startCmd6 = self.replaceTag('', self._properties, + conditional='family=inet6', cache=self.__substCache) + if startCmd6 != startCmd: + res &= self.executeCmd(startCmd6, self.timeout) + if not res: + raise RuntimeError("Error starting action %s/%s" % (self._jail, self._name,)) + except ValueError, e: + raise RuntimeError("Error starting action %s/%s: %r" % (self._jail, self._name, e)) def ban(self, aInfo): """Executes the "actionban" command. @@ -302,17 +298,6 @@ class CommandAction(ActionBase): if not self._processCmd('', aInfo): raise RuntimeError("Error banning %(ip)s" % aInfo) - @property - def actionunban(self): - """The command used when an unban occurs. - """ - return self._actionunban - - @actionunban.setter - def actionunban(self, value): - self._actionunban = value - self._logSys.debug("Set actionunban = %s" % value) - def unban(self, aInfo): """Executes the "actionunban" command. @@ -328,32 +313,6 @@ class CommandAction(ActionBase): if not self._processCmd('', aInfo): raise RuntimeError("Error unbanning %(ip)s" % aInfo) - @property - def actioncheck(self): - """The command used to check the environment. - - This is used prior to a ban taking place to ensure the - environment is appropriate. If this check fails, `stop` and - `start` is executed prior to the check being called again. - """ - return self._actioncheck - - @actioncheck.setter - def actioncheck(self, value): - self._actioncheck = value - self._logSys.debug("Set actioncheck = %s" % value) - - @property - def actionstop(self): - """The command executed when the jail/actions stops. - """ - return self._actionstop - - @actionstop.setter - def actionstop(self, value): - self._actionstop = value - self._logSys.debug("Set actionstop = %s" % value) - def stop(self): """Executes the "actionstop" command. @@ -361,18 +320,20 @@ class CommandAction(ActionBase): and executes the resulting command. """ # common (resp. ipv4): - stopCmd = self.replaceTag('', self._properties, conditional='family=inet4') + stopCmd = self.replaceTag('', self._properties, + conditional='family=inet4', cache=self.__substCache) res = self.executeCmd(stopCmd, self.timeout) # ipv6 actions if available: if allowed_ipv6: - stopCmd6 = self.replaceTag('', self._properties, conditional='family=inet6') + stopCmd6 = self.replaceTag('', self._properties, + conditional='family=inet6', cache=self.__substCache) if stopCmd6 != stopCmd: res &= self.executeCmd(stopCmd6, self.timeout) if not res: raise RuntimeError("Error stopping action") @classmethod - def substituteRecursiveTags(cls, tags, conditional=''): + def substituteRecursiveTags(cls, inptags, conditional=''): """Sort out tag definitions within other tags. Since v.0.9.2 supports embedded interpolation (see test cases for examples). @@ -382,7 +343,7 @@ class CommandAction(ActionBase): Parameters ---------- - tags : dict + inptags : dict Dictionary of tags(keys) and their values. Returns @@ -391,6 +352,8 @@ class CommandAction(ActionBase): Dictionary of tags(keys) and their values, with tags within the values recursively replaced. """ + # copy return tags dict to prevent modifying of inptags: + tags = inptags.copy() t = TAG_CRE # repeat substitution while embedded-recursive (repFlag is True) while True: @@ -467,7 +430,7 @@ class CommandAction(ActionBase): return value @classmethod - def replaceTag(cls, query, aInfo, conditional=''): + def replaceTag(cls, query, aInfo, conditional='', cache=None): """Replaces tags in `query` with property values. Parameters @@ -482,18 +445,32 @@ class CommandAction(ActionBase): str `query` string with tags replaced. """ + # use cache if allowed: + if cache is not None: + ckey = (query, conditional) + string = cache.get(ckey) + if string is not None: + return string + # replace: string = query aInfo = cls.substituteRecursiveTags(aInfo, conditional) for tag in aInfo: if "<%s>" % tag in query: - value = str(aInfo[tag]) # assure string + value = aInfo.get(tag + '?' + conditional) + if value is None: + value = aInfo.get(tag) + value = str(value) # assure string if tag in cls._escapedTags: # That one needs to be escaped since its content is # out of our control value = cls.escapeTag(value) string = string.replace('<' + tag + '>', value) - # New line + # New line, space string = reduce(lambda s, kv: s.replace(*kv), (("
", '\n'), ("", " ")), string) + # cache if properties: + if cache is not None: + cache[ckey] = string + # return string def _processCmd(self, cmd, aInfo=None, conditional=''): @@ -520,6 +497,7 @@ class CommandAction(ActionBase): self._logSys.debug("Nothing to do") return True + # conditional corresponding family of the given ip: if conditional == '': conditional = 'family=inet4' if allowed_ipv6: @@ -530,7 +508,8 @@ class CommandAction(ActionBase): except KeyError: pass - checkCmd = self.replaceTag('', self._properties, conditional=conditional) + checkCmd = self.replaceTag('', self._properties, + conditional=conditional, cache=self.__substCache) if not self.executeCmd(checkCmd, self.timeout): self._logSys.error( "Invariant check failed. Trying to restore a sane environment") @@ -541,7 +520,8 @@ class CommandAction(ActionBase): return False # Replace static fields - realCmd = self.replaceTag(cmd, self._properties, conditional=conditional) + realCmd = self.replaceTag(cmd, self._properties, + conditional=conditional, cache=self.__substCache) # Replace tags if aInfo is not None: diff --git a/fail2ban/tests/actiontestcase.py b/fail2ban/tests/actiontestcase.py index d0c1495a..4a34a305 100644 --- a/fail2ban/tests/actiontestcase.py +++ b/fail2ban/tests/actiontestcase.py @@ -139,6 +139,48 @@ class CommandActionTest(LogCaptureTestCase): self.__action.replaceTag("abc", CallingMap(matches=lambda: int("a"))), "abc") + def testReplaceTagConditionalCached(self): + setattr(self.__action, 'abc', "123") + setattr(self.__action, 'abc?family=inet4', "345") + setattr(self.__action, 'abc?family=inet6', "567") + setattr(self.__action, 'xyz', "890-") + setattr(self.__action, 'banaction', "Text text ") + # test replacement in sub tags and direct, conditional, cached: + cache = self.__action._substCache + for i in range(2): + self.assertEqual( + self.__action.replaceTag("", self.__action._properties, + conditional="", cache=cache), + "Text 890-123 text 123") + self.assertEqual( + self.__action.replaceTag("", self.__action._properties, + conditional="family=inet4", cache=cache), + "Text 890-345 text 345") + self.assertEqual( + self.__action.replaceTag("", self.__action._properties, + conditional="family=inet6", cache=cache), + "Text 890-567 text 567") + self.assertEqual(len(cache) if cache is not None else -1, 3) + # set one parameter - internal properties and cache should be reseted: + setattr(self.__action, 'xyz', "000-") + self.assertEqual(len(cache) if cache is not None else -1, 0) + # test againg, should have 000 instead of 890: + for i in range(2): + self.assertEqual( + self.__action.replaceTag("", self.__action._properties, + conditional="", cache=cache), + "Text 000-123 text 123") + self.assertEqual( + self.__action.replaceTag("", self.__action._properties, + conditional="family=inet4", cache=cache), + "Text 000-345 text 345") + self.assertEqual( + self.__action.replaceTag("", self.__action._properties, + conditional="family=inet6", cache=cache), + "Text 000-567 text 567") + self.assertEqual(len(cache), 3) + + def testExecuteActionBan(self): self.__action.actionstart = "touch /tmp/fail2ban.test" self.assertEqual(self.__action.actionstart, "touch /tmp/fail2ban.test")