From a3df1ab3f0674fa764be5c67805f5e1a0fd8e153 Mon Sep 17 00:00:00 2001 From: Mart124 <37041094+Mart124@users.noreply.github.com> Date: Mon, 25 Nov 2019 21:50:16 +0100 Subject: [PATCH 01/20] Create bitwarden --- fail2ban/tests/files/logs/bitwarden | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 fail2ban/tests/files/logs/bitwarden diff --git a/fail2ban/tests/files/logs/bitwarden b/fail2ban/tests/files/logs/bitwarden new file mode 100644 index 00000000..2748a944 --- /dev/null +++ b/fail2ban/tests/files/logs/bitwarden @@ -0,0 +1,5 @@ +# failJSON: { "time": "2019-11-26T01:04:49", "match": true , "host": "192.168.0.16" } +2019-11-26 01:04:49.008 +08:00 [WRN] Failed login attempt. 192.168.0.16 + +# failJSON: { "time": "2019-11-25T21:39:58", "match": true , "host": "192.168.0.21" } +2019-11-25 21:39:58.464 +01:00 [WRN] Failed login attempt, 2FA invalid. 192.168.0.21 From ef394b3cf03a065d60f6ad1509f412565d547224 Mon Sep 17 00:00:00 2001 From: Mart124 <37041094+Mart124@users.noreply.github.com> Date: Mon, 25 Nov 2019 21:55:45 +0100 Subject: [PATCH 02/20] Update jail.conf --- config/jail.conf | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/config/jail.conf b/config/jail.conf index b39b3a6c..d1792553 100644 --- a/config/jail.conf +++ b/config/jail.conf @@ -821,6 +821,10 @@ udpport = 1200,27000,27001,27002,27003,27004,27005,27006,27007,27008,27009,27010 action = %(banaction)s[name=%(__name__)s-tcp, port="%(tcpport)s", protocol="tcp", chain="%(chain)s", actname=%(banaction)s-tcp] %(banaction)s[name=%(__name__)s-udp, port="%(udpport)s", protocol="udp", chain="%(chain)s", actname=%(banaction)s-udp] +[centreon] +port = http,https +logpath = /home/*/bwdata/logs/identity/Identity/log.txt + [centreon] port = http,https logpath = /var/log/centreon/login.log From 30e742a849129897f9016cd5df6e3f9be2d495f8 Mon Sep 17 00:00:00 2001 From: Mart124 <37041094+Mart124@users.noreply.github.com> Date: Mon, 25 Nov 2019 21:57:41 +0100 Subject: [PATCH 03/20] Update jail.conf --- config/jail.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/jail.conf b/config/jail.conf index d1792553..41495a09 100644 --- a/config/jail.conf +++ b/config/jail.conf @@ -821,7 +821,7 @@ udpport = 1200,27000,27001,27002,27003,27004,27005,27006,27007,27008,27009,27010 action = %(banaction)s[name=%(__name__)s-tcp, port="%(tcpport)s", protocol="tcp", chain="%(chain)s", actname=%(banaction)s-tcp] %(banaction)s[name=%(__name__)s-udp, port="%(udpport)s", protocol="udp", chain="%(chain)s", actname=%(banaction)s-udp] -[centreon] +[bitwarden] port = http,https logpath = /home/*/bwdata/logs/identity/Identity/log.txt From 79caeaa52024f31635adbae6b8d2b65f2127ba38 Mon Sep 17 00:00:00 2001 From: Mart124 <37041094+Mart124@users.noreply.github.com> Date: Mon, 25 Nov 2019 22:05:29 +0100 Subject: [PATCH 04/20] Create bitwarden.conf --- config/filter.d/bitwarden.conf | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 config/filter.d/bitwarden.conf diff --git a/config/filter.d/bitwarden.conf b/config/filter.d/bitwarden.conf new file mode 100644 index 00000000..4cebcedf --- /dev/null +++ b/config/filter.d/bitwarden.conf @@ -0,0 +1,9 @@ +# Fail2Ban filter for Bitwarden +# Detecting failed login attempts +# Logged in bwdata/logs/identity/Identity/log.txt + +[Init] +datepattern = ^%%Y-%%m-%%d %%H:%%M:%%S + +[Definition] +failregex = ^.*Failed login attempt(, 2FA invalid)\. $ From 869327e9b18e5fd9c84813609e094c4d6646e6b8 Mon Sep 17 00:00:00 2001 From: Mart124 <37041094+Mart124@users.noreply.github.com> Date: Mon, 25 Nov 2019 22:17:58 +0100 Subject: [PATCH 05/20] Update bitwarden.conf --- config/filter.d/bitwarden.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/filter.d/bitwarden.conf b/config/filter.d/bitwarden.conf index 4cebcedf..1c49fdc4 100644 --- a/config/filter.d/bitwarden.conf +++ b/config/filter.d/bitwarden.conf @@ -6,4 +6,4 @@ datepattern = ^%%Y-%%m-%%d %%H:%%M:%%S [Definition] -failregex = ^.*Failed login attempt(, 2FA invalid)\. $ +failregex = ^.*Failed login attempt(, 2FA invalid)?\. $ From d7b707b09d2b7908976153f7e36eff6b14f94457 Mon Sep 17 00:00:00 2001 From: Mart124 <37041094+Mart124@users.noreply.github.com> Date: Wed, 27 Nov 2019 00:09:22 +0100 Subject: [PATCH 06/20] Update bitwarden.conf --- config/filter.d/bitwarden.conf | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/config/filter.d/bitwarden.conf b/config/filter.d/bitwarden.conf index 1c49fdc4..6a9e87e6 100644 --- a/config/filter.d/bitwarden.conf +++ b/config/filter.d/bitwarden.conf @@ -2,8 +2,5 @@ # Detecting failed login attempts # Logged in bwdata/logs/identity/Identity/log.txt -[Init] -datepattern = ^%%Y-%%m-%%d %%H:%%M:%%S - [Definition] -failregex = ^.*Failed login attempt(, 2FA invalid)?\. $ +failregex = ^\s*\[\w+\]\s+Failed login attempt(?:, 2FA invalid)?\. $ From 566cbcdde0a4cd5670f3dcd1415a77845304a396 Mon Sep 17 00:00:00 2001 From: Mart124 <37041094+Mart124@users.noreply.github.com> Date: Wed, 27 Nov 2019 00:14:18 +0100 Subject: [PATCH 07/20] Update bitwarden --- fail2ban/tests/files/logs/bitwarden | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fail2ban/tests/files/logs/bitwarden b/fail2ban/tests/files/logs/bitwarden index 2748a944..3642b3bf 100644 --- a/fail2ban/tests/files/logs/bitwarden +++ b/fail2ban/tests/files/logs/bitwarden @@ -1,4 +1,4 @@ -# failJSON: { "time": "2019-11-26T01:04:49", "match": true , "host": "192.168.0.16" } +# failJSON: { "time": "2019-11-25T18:04:49", "match": true , "host": "192.168.0.16" } 2019-11-26 01:04:49.008 +08:00 [WRN] Failed login attempt. 192.168.0.16 # failJSON: { "time": "2019-11-25T21:39:58", "match": true , "host": "192.168.0.21" } From e763c657c4288df88f9fd2c02b5d2428b688e938 Mon Sep 17 00:00:00 2001 From: Mart124 <37041094+Mart124@users.noreply.github.com> Date: Wed, 27 Nov 2019 00:32:10 +0100 Subject: [PATCH 08/20] Let's get back to WRN --- config/filter.d/bitwarden.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/filter.d/bitwarden.conf b/config/filter.d/bitwarden.conf index 6a9e87e6..29bd4be8 100644 --- a/config/filter.d/bitwarden.conf +++ b/config/filter.d/bitwarden.conf @@ -3,4 +3,4 @@ # Logged in bwdata/logs/identity/Identity/log.txt [Definition] -failregex = ^\s*\[\w+\]\s+Failed login attempt(?:, 2FA invalid)?\. $ +failregex = ^\s*\[WRN\]\s+Failed login attempt(?:, 2FA invalid)?\. $ From 8f6ba1532563dbc8f41743817691be03ddc90881 Mon Sep 17 00:00:00 2001 From: sebres Date: Fri, 20 Sep 2019 16:26:15 +0200 Subject: [PATCH 09/20] avoid unhandled exception during flush, better invariant check (and repair), avoid repair by unban/stop etc... --- config/jail.conf | 3 +- fail2ban/client/actionreader.py | 3 +- fail2ban/client/jailreader.py | 12 +- fail2ban/helpers.py | 12 +- fail2ban/server/action.py | 127 +++++++++++------- fail2ban/server/actions.py | 33 +++-- fail2ban/tests/actionstestcase.py | 52 +++++-- fail2ban/tests/clientreadertestcase.py | 4 +- .../files/action.d/action_modifyainfo.py | 5 + 9 files changed, 168 insertions(+), 83 deletions(-) diff --git a/config/jail.conf b/config/jail.conf index 41495a09..a77a47ba 100644 --- a/config/jail.conf +++ b/config/jail.conf @@ -861,7 +861,8 @@ filter = apache-pass[knocking_url="%(knocking_url)s"] logpath = %(apache_access_log)s blocktype = RETURN returntype = DROP -action = %(action_)s[blocktype=%(blocktype)s, returntype=%(returntype)s] +action = %(action_)s[blocktype=%(blocktype)s, returntype=%(returntype)s, + actionstart_on_demand=True, actionrepair_on_unban=True] bantime = 1h maxretry = 1 findtime = 1 diff --git a/fail2ban/client/actionreader.py b/fail2ban/client/actionreader.py index d7bba61f..0840fe68 100644 --- a/fail2ban/client/actionreader.py +++ b/fail2ban/client/actionreader.py @@ -44,6 +44,7 @@ class ActionReader(DefinitionInitConfigReader): "actionreload": ["string", None], "actioncheck": ["string", None], "actionrepair": ["string", None], + "actionrepair_on_unban": ["string", None], "actionban": ["string", None], "actionunban": ["string", None], "norestored": ["string", None], @@ -78,7 +79,7 @@ class ActionReader(DefinitionInitConfigReader): opts = self.getCombined( ignore=CommandAction._escapedTags | set(('timeout', 'bantime'))) # type-convert only after combined (otherwise boolean converting prevents substitution): - for o in ('norestored', 'actionstart_on_demand'): + for o in ('norestored', 'actionstart_on_demand', 'actionrepair_on_unban'): if opts.get(o): opts[o] = self._convert_to_boolean(opts[o]) diff --git a/fail2ban/client/jailreader.py b/fail2ban/client/jailreader.py index 02368a8b..917a562c 100644 --- a/fail2ban/client/jailreader.py +++ b/fail2ban/client/jailreader.py @@ -33,7 +33,7 @@ from .configreader import ConfigReaderUnshared, ConfigReader from .filterreader import FilterReader from .actionreader import ActionReader from ..version import version -from ..helpers import getLogger, extractOptions, splitwords +from ..helpers import getLogger, extractOptions, splitWithOptions, splitwords # Gets the instance of the logger. logSys = getLogger(__name__) @@ -164,21 +164,15 @@ class JailReader(ConfigReader): self.__filter.getOptions(self.__opts) # Read action - prevln = '' - actlst = self.__opts["action"].split('\n') - for n, act in enumerate(actlst): + for act in splitWithOptions(self.__opts["action"]): try: + act = act.strip() if not act: # skip empty actions continue # join with previous line if needed (consider possible new-line): - if prevln: act = prevln + '\n' + act actName, actOpt = extractOptions(act) prevln = '' if not actName: - # consider possible new-line, so repeat with joined next line's: - if n < len(actlst) - 1: - prevln = act - continue raise JailDefError("Invalid action definition %r" % act) if actName.endswith(".py"): self.__actions.append([ diff --git a/fail2ban/helpers.py b/fail2ban/helpers.py index 44574ff8..213a405f 100644 --- a/fail2ban/helpers.py +++ b/fail2ban/helpers.py @@ -336,6 +336,9 @@ OPTION_CRE = re.compile(r"^([^\[]+)(?:\[(.*)\])?\s*$", re.DOTALL) # `action = act[p1=...][p2=...]` OPTION_EXTRACT_CRE = re.compile( r'([\w\-_\.]+)=(?:"([^"]*)"|\'([^\']*)\'|([^,\]]*))(?:,|\]\s*\[|$)', re.DOTALL) +# split by new-line considering possible new-lines within options [...]: +OPTION_SPLIT_CRE = re.compile( + r'(?:[^\[\n]+(?:\s*\[\s*(?:[\w\-_\.]+=(?:"[^"]*"|\'[^\']*\'|[^,\]]*)\s*(?:,|\]\s*\[)?\s*)*\])?\s*|[^\n]+)(?=\n\s*|$)', re.DOTALL) def extractOptions(option): match = OPTION_CRE.match(option) @@ -352,6 +355,9 @@ def extractOptions(option): option_opts[opt.strip()] = value.strip() return option_name, option_opts +def splitWithOptions(option): + return OPTION_SPLIT_CRE.findall(option) + # # Following facilities used for safe recursive interpolation of # tags () in tagged options. @@ -386,8 +392,7 @@ def substituteRecursiveTags(inptags, conditional='', """ #logSys = getLogger("fail2ban") tre_search = TAG_CRE.search - # copy return tags dict to prevent modifying of inptags: - tags = inptags.copy() + tags = inptags # init: ignore = set(ignore) done = set() @@ -449,6 +454,9 @@ def substituteRecursiveTags(inptags, conditional='', # check still contains any tag - should be repeated (possible embedded-recursive substitution): if tre_search(value): repFlag = True + # copy return tags dict to prevent modifying of inptags: + if id(tags) == id(inptags): + tags = inptags.copy() tags[tag] = value # no more sub tags (and no possible composite), add this tag to done set (just to be faster): if '<' not in value: done.add(tag) diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index e4c83409..0942a3e5 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -50,6 +50,7 @@ allowed_ipv6 = True # capture groups from filter for map to ticket data: FCUSTAG_CRE = re.compile(r''); # currently uppercase only +COND_FAMILIES = ('inet4', 'inet6') CONDITIONAL_FAM_RE = re.compile(r"^(\w+)\?(family)=") # Special tags: @@ -363,8 +364,8 @@ class CommandAction(ActionBase): self.__properties = dict( (key, getattr(self, key)) for key in dir(self) - if not key.startswith("_") and not callable(getattr(self, key))) - # + if not key.startswith("_") and not callable(getattr(self, key)) + ) return self.__properties @property @@ -372,8 +373,13 @@ class CommandAction(ActionBase): return self.__substCache def _getOperation(self, tag, family): + # be sure family is enclosed as conditional value (if not overwritten in action): + if family and self._hasCondSection: + if 'family' not in self._properties and 'family?family='+family not in self._properties: + self._properties['family?family='+family] = family + # replace operation tag (interpolate all values): return self.replaceTag(tag, self._properties, - conditional=('family=' + family), cache=self.__substCache) + conditional=('family='+family if family else ''), cache=self.__substCache) def _executeOperation(self, tag, operation, family=[]): """Executes the operation commands (like "actionstart", "actionstop", etc). @@ -400,7 +406,17 @@ class CommandAction(ActionBase): raise RuntimeError("Error %s action %s/%s: %r" % (operation, self._jail, self._name, e)) return res - COND_FAMILIES = ('inet4', 'inet6') + @property + def _hasCondSection(self): + v = self._properties.get('__hasCondSection') + if v is None: + v = False + for n in self._properties: + if CONDITIONAL_FAM_RE.match(n): + v = True + break + self._properties['__hasCondSection'] = v + return v @property def _startOnDemand(self): @@ -409,11 +425,7 @@ class CommandAction(ActionBase): if v is not None: return v # not set - auto-recognize (depending on conditional): - v = False - for n in self._properties: - if CONDITIONAL_FAM_RE.match(n): - v = True - break + v = self._hasCondSection self._properties['actionstart_on_demand'] = v return v @@ -514,6 +526,17 @@ class CommandAction(ActionBase): """ return self._executeOperation('', 'reloading') + def consistencyCheck(self, beforeRepair=None): + """Executes the invariant check with repair if expected (conditional). + """ + ret = True + # for each started family: + if self.actioncheck: + for (family, started) in self.__started.iteritems(): + if started and not self._invariantCheck(family, beforeRepair): + ret &= False + return ret + @staticmethod def escapeTag(value): """Escape characters which may be used for command injection. @@ -705,7 +728,40 @@ class CommandAction(ActionBase): realCmd = Utils.buildShellCmd(realCmd, varsDict) return realCmd - def _processCmd(self, cmd, aInfo=None, conditional=''): + def _invariantCheck(self, family='', beforeRepair=None): + """Executes a substituted `actioncheck` command. + """ + checkCmd = self._getOperation('', family) + if not checkCmd or self.executeCmd(checkCmd, self.timeout): + return True + # if don't need repair/restore - just return: + if beforeRepair and not beforeRepair(): + return False + self._logSys.error( + "Invariant check failed. Trying to restore a sane environment") + # try to find repair command, if exists - exec it: + repairCmd = self._getOperation('', family) + if repairCmd: + if not self.executeCmd(repairCmd, self.timeout): + self._logSys.critical("Unable to restore environment") + return False + else: + # no repair command, try to restart action... + # [WARNING] TODO: be sure all banactions get a repair command, because + # otherwise stop/start will theoretically remove all the bans, + # but the tickets are still in BanManager, so in case of new failures + # it will not be banned, because "already banned" will happen. + try: + self.stop() + except RuntimeError: # bypass error in stop (if start/check succeeded hereafter). + pass + self.start() + if not self.executeCmd(checkCmd, self.timeout): + self._logSys.critical("Unable to restore environment") + return False + return True + + def _processCmd(self, cmd, aInfo=None): """Executes a command with preliminary checks and substitutions. Before executing any commands, executes the "check" command first @@ -730,47 +786,26 @@ class CommandAction(ActionBase): return True # conditional corresponding family of the given ip: - if conditional == '': - conditional = 'family=inet4' - if allowed_ipv6: - try: - ip = aInfo["ip"] - if ip and asip(ip).isIPv6: - conditional = 'family=inet6' - except KeyError: - pass + try: + family = aInfo["family"] + except KeyError: + family = '' - checkCmd = self.replaceTag('', self._properties, - conditional=conditional, cache=self.__substCache) - if checkCmd: - if not self.executeCmd(checkCmd, self.timeout): - self._logSys.error( - "Invariant check failed. Trying to restore a sane environment") - # try to find repair command, if exists - exec it: - repairCmd = self.replaceTag('', self._properties, - conditional=conditional, cache=self.__substCache) - if repairCmd: - if not self.executeCmd(repairCmd, self.timeout): - self._logSys.critical("Unable to restore environment") - return False - else: - # no repair command, try to restart action... - # [WARNING] TODO: be sure all banactions get a repair command, because - # otherwise stop/start will theoretically remove all the bans, - # but the tickets are still in BanManager, so in case of new failures - # it will not be banned, because "already banned" will happen. - try: - self.stop() - except RuntimeError: # bypass error in stop (if start/check succeeded hereafter). - pass - self.start() - if not self.executeCmd(checkCmd, self.timeout): - self._logSys.critical("Unable to restore environment") + # invariant check: + if self.actioncheck: + # don't repair/restore if unban (no matter): + def _beforeRepair(): + if cmd == '' and not self._properties.get('actionrepair_on_unban'): + self._logSys.error("Invariant check failed. Unban is impossible.") return False + return True + ret = self._invariantCheck(family, _beforeRepair) + if not ret: + return False # Replace static fields realCmd = self.replaceTag(cmd, self._properties, - conditional=conditional, cache=self.__substCache) + conditional=('family='+family if family else ''), cache=self.__substCache) # Replace dynamical tags, important - don't cache, no recursion and auto-escape here if aInfo is not None: diff --git a/fail2ban/server/actions.py b/fail2ban/server/actions.py index 51904ecd..35f2306b 100644 --- a/fail2ban/server/actions.py +++ b/fail2ban/server/actions.py @@ -160,8 +160,8 @@ class Actions(JailThread, Mapping): delacts = OrderedDict((name, action) for name, action in self._actions.iteritems() if name not in self._reload_actions) if len(delacts): - # unban all tickets using remove action only: - self.__flushBan(db=False, actions=delacts) + # unban all tickets using removed actions only: + self.__flushBan(db=False, actions=delacts, stop=True) # stop and remove it: self.stopActions(actions=delacts) delattr(self, '_reload_actions') @@ -326,7 +326,7 @@ class Actions(JailThread, Mapping): self.__checkUnBan(bancnt if bancnt and bancnt < self.unbanMaxCount else self.unbanMaxCount) cnt = 0 - self.__flushBan() + self.__flushBan(stop=True) self.stopActions() return True @@ -494,7 +494,7 @@ class Actions(JailThread, Mapping): cnt, self.__banManager.size(), self._jail.name) return cnt - def __flushBan(self, db=False, actions=None): + def __flushBan(self, db=False, actions=None, stop=False): """Flush the ban list. Unban all IP address which are still in the banning list. @@ -513,11 +513,26 @@ class Actions(JailThread, Mapping): # first we'll execute flush for actions supporting this operation: unbactions = {} for name, action in (actions if actions is not None else self._actions).iteritems(): - if hasattr(action, 'flush') and action.actionflush: - logSys.notice("[%s] Flush ticket(s) with %s", self._jail.name, name) - action.flush() - else: - unbactions[name] = action + try: + if hasattr(action, 'flush') and (not isinstance(action, CommandAction) or action.actionflush): + logSys.notice("[%s] Flush ticket(s) with %s", self._jail.name, name) + action.flush() + else: + unbactions[name] = action + except Exception as e: + logSys.error("Failed to flush bans in jail '%s' action '%s': %s", + self._jail.name, name, e, + exc_info=logSys.getEffectiveLevel()<=logging.DEBUG) + logSys.info("No flush occured, do consistency check") + def _beforeRepair(): + if stop: + self._logSys.error("Invariant check failed. Flush is impossible.") + return False + return True + if not hasattr(action, 'consistencyCheck') or action.consistencyCheck(_beforeRepair): + # fallback to single unbans: + logSys.info("unban tickets each individualy") + unbactions[name] = action actions = unbactions # flush the database also: if db and self._jail.database is not None: diff --git a/fail2ban/tests/actionstestcase.py b/fail2ban/tests/actionstestcase.py index d2dcdadb..86b539f0 100644 --- a/fail2ban/tests/actionstestcase.py +++ b/fail2ban/tests/actionstestcase.py @@ -44,20 +44,20 @@ class ExecuteActions(LogCaptureTestCase): super(ExecuteActions, self).setUp() self.__jail = DummyJail() self.__actions = Actions(self.__jail) - self.__tmpfile, self.__tmpfilename = tempfile.mkstemp() def tearDown(self): super(ExecuteActions, self).tearDown() - os.remove(self.__tmpfilename) - def defaultActions(self): + def defaultAction(self): self.__actions.add('ip') - self.__ip = self.__actions['ip'] - self.__ip.actionstart = 'echo ip start 64 >> "%s"' % self.__tmpfilename - self.__ip.actionban = 'echo ip ban >> "%s"' % self.__tmpfilename - self.__ip.actionunban = 'echo ip unban >> "%s"' % self.__tmpfilename - self.__ip.actioncheck = 'echo ip check >> "%s"' % self.__tmpfilename - self.__ip.actionstop = 'echo ip stop >> "%s"' % self.__tmpfilename + act = self.__actions['ip'] + act.actionstart = 'echo ip start' + act.actionban = 'echo ip ban ' + act.actionunban = 'echo ip unban ' + act.actioncheck = 'echo ip check' + act.actionflush = 'echo ip flush ' + act.actionstop = 'echo ip stop' + return act def testActionsAddDuplicateName(self): self.__actions.add('test') @@ -89,13 +89,12 @@ class ExecuteActions(LogCaptureTestCase): self.assertLogged('Ban 192.0.2.3') def testActionsOutput(self): - self.defaultActions() + self.defaultAction() self.__actions.start() - with open(self.__tmpfilename) as f: - self.assertTrue( Utils.wait_for(lambda: (f.read() == "ip start 64\n"), 3) ) - + self.assertLogged("stdout: %r" % 'ip start', wait=True) self.__actions.stop() self.__actions.join() + self.assertLogged("stdout: %r" % 'ip flush', "stdout: %r" % 'ip stop') self.assertEqual(self.__actions.status(),[("Currently banned", 0 ), ("Total banned", 0 ), ("Banned IP list", [] )]) @@ -211,3 +210,30 @@ class ExecuteActions(LogCaptureTestCase): self.assertLogged('Unbanned 30, 0 ticket(s)') self.assertNotLogged('Unbanned 50, 0 ticket(s)') + + @with_alt_time + def testActionsConsistencyCheck(self): + # flush is broken - test no unhandled except and invariant check: + act = self.defaultAction() + setattr(act, 'actionflush?family=inet6', 'echo ip flush ; exit 1') + act.actionstart_on_demand = True + self.__actions.start() + self.assertNotLogged("stdout: %r" % 'ip start') + + self.assertEqual(self.__actions.addBannedIP('192.0.2.1'), 1) + self.assertEqual(self.__actions.addBannedIP('2001:db8::1'), 1) + self.assertLogged('Ban 192.0.2.1', 'Ban 2001:db8::1', + "stdout: %r" % 'ip start', + "stdout: %r" % 'ip ban 192.0.2.1', + "stdout: %r" % 'ip ban 2001:db8::1', + all=True, wait=True) + + self.__actions._Actions__flushBan() + self.assertLogged('Failed to flush bans', + 'No flush occured, do consistency check', + "stdout: %r" % 'ip ban 192.0.2.1', + all=True, wait=True) + + self.__actions.stop() + self.__actions.join() + diff --git a/fail2ban/tests/clientreadertestcase.py b/fail2ban/tests/clientreadertestcase.py index 5746f3fb..d39860f4 100644 --- a/fail2ban/tests/clientreadertestcase.py +++ b/fail2ban/tests/clientreadertestcase.py @@ -31,7 +31,7 @@ import unittest from ..client.configreader import ConfigReader, ConfigReaderUnshared, \ DefinitionInitConfigReader, NoSectionError from ..client import configparserinc -from ..client.jailreader import JailReader, extractOptions +from ..client.jailreader import JailReader, extractOptions, splitWithOptions from ..client.filterreader import FilterReader from ..client.jailsreader import JailsReader from ..client.actionreader import ActionReader, CommandAction @@ -778,7 +778,7 @@ class JailsReaderTest(LogCaptureTestCase): # somewhat duplicating here what is done in JailsReader if # the jail is enabled - for act in actions.split('\n'): + for act in splitWithOptions(actions): actName, actOpt = extractOptions(act) self.assertTrue(len(actName)) self.assertTrue(isinstance(actOpt, dict)) diff --git a/fail2ban/tests/files/action.d/action_modifyainfo.py b/fail2ban/tests/files/action.d/action_modifyainfo.py index b003edef..d585a7ee 100644 --- a/fail2ban/tests/files/action.d/action_modifyainfo.py +++ b/fail2ban/tests/files/action.d/action_modifyainfo.py @@ -12,4 +12,9 @@ class TestAction(ActionBase): del aInfo['ip'] self._logSys.info("%s unban deleted aInfo IP", self._name) + def flush(self): + # intended error to cover no unhandled exception occurs in flash + # as well as unbans are done individually after errored flush. + raise ValueError("intended error") + Action = TestAction From 31b8d91ba2211595182d8d3fe6d89034b562aef0 Mon Sep 17 00:00:00 2001 From: sebres Date: Fri, 27 Dec 2019 21:58:06 +0100 Subject: [PATCH 10/20] **not ready** amend with more tests (some issue on stop?) --- fail2ban/server/action.py | 18 ++++++++++-------- fail2ban/tests/actionstestcase.py | 20 ++++++++++++++++++-- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index 0942a3e5..94114862 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -341,6 +341,8 @@ class CommandAction(ActionBase): # set: self.__dict__[name] = value + __setitem__ = __setattr__ + def __delattr__(self, name): if not name.startswith('_'): # parameters changed - clear properties and substitution cache: @@ -373,13 +375,12 @@ class CommandAction(ActionBase): return self.__substCache def _getOperation(self, tag, family): - # be sure family is enclosed as conditional value (if not overwritten in action): - if family and self._hasCondSection: - if 'family' not in self._properties and 'family?family='+family not in self._properties: - self._properties['family?family='+family] = family - # replace operation tag (interpolate all values): + # replace operation tag (interpolate all values), be sure family is enclosed as conditional value + # (as lambda in addrepl so only if not overwritten in action): return self.replaceTag(tag, self._properties, - conditional=('family='+family if family else ''), cache=self.__substCache) + conditional=('family='+family if family else ''), + addrepl=(lambda tag:family if tag == 'family' else None), + cache=self.__substCache) def _executeOperation(self, tag, operation, family=[]): """Executes the operation commands (like "actionstart", "actionstop", etc). @@ -564,7 +565,7 @@ class CommandAction(ActionBase): return value @classmethod - def replaceTag(cls, query, aInfo, conditional='', cache=None): + def replaceTag(cls, query, aInfo, conditional='', addrepl=None, cache=None): """Replaces tags in `query` with property values. Parameters @@ -605,7 +606,8 @@ class CommandAction(ActionBase): pass # interpolation of dictionary: if subInfo is None: - subInfo = substituteRecursiveTags(aInfo, conditional, ignore=cls._escapedTags) + subInfo = substituteRecursiveTags(aInfo, conditional, ignore=cls._escapedTags, + addrepl=addrepl) # cache if possible: if csubkey is not None: cache[csubkey] = subInfo diff --git a/fail2ban/tests/actionstestcase.py b/fail2ban/tests/actionstestcase.py index 86b539f0..9d6b16c6 100644 --- a/fail2ban/tests/actionstestcase.py +++ b/fail2ban/tests/actionstestcase.py @@ -213,9 +213,9 @@ class ExecuteActions(LogCaptureTestCase): @with_alt_time def testActionsConsistencyCheck(self): - # flush is broken - test no unhandled except and invariant check: + # flush for inet6 is intentionally "broken" here - test no unhandled except and invariant check: act = self.defaultAction() - setattr(act, 'actionflush?family=inet6', 'echo ip flush ; exit 1') + act['actionflush?family=inet6'] = 'echo ip flush ; exit 1' act.actionstart_on_demand = True self.__actions.start() self.assertNotLogged("stdout: %r" % 'ip start') @@ -228,12 +228,28 @@ class ExecuteActions(LogCaptureTestCase): "stdout: %r" % 'ip ban 2001:db8::1', all=True, wait=True) + # check should fail (so cause stop/start): + self.pruneLog('[test-phase 1] simulate inconsistent env') + act['actioncheck?family=inet6'] = 'echo ip check ; exit 1' + self.__actions._Actions__flushBan() + self.assertLogged('Failed to flush bans', + 'No flush occured, do consistency check', + 'Invariant check failed. Trying to restore a sane environment', + "stdout: %r" % 'ip stop', + "stdout: %r" % 'ip start', + all=True, wait=True) + + # check succeeds: + self.pruneLog('[test-phase 2] consistent env') + act['actioncheck?family=inet6'] = act.actioncheck self.__actions._Actions__flushBan() self.assertLogged('Failed to flush bans', 'No flush occured, do consistency check', "stdout: %r" % 'ip ban 192.0.2.1', all=True, wait=True) + act['actionflush?family=inet6'] = act.actionflush + self.__actions.stop() self.__actions.join() From 3c42c7b9ef07eeafc2c65ea9347cb99c155c766e Mon Sep 17 00:00:00 2001 From: sebres Date: Mon, 30 Dec 2019 22:03:26 +0100 Subject: [PATCH 11/20] **not ready** testActionsConsistencyCheck fixed, but several **broken** tests (todo: fix public interface like action.start()/stop()). --- fail2ban/server/action.py | 68 +++++++++------ fail2ban/server/actions.py | 25 +++--- fail2ban/tests/actionstestcase.py | 138 +++++++++++++++++++++++++++--- 3 files changed, 181 insertions(+), 50 deletions(-) diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index 94114862..b0821026 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -382,7 +382,7 @@ class CommandAction(ActionBase): addrepl=(lambda tag:family if tag == 'family' else None), cache=self.__substCache) - def _executeOperation(self, tag, operation, family=[]): + def _executeOperation(self, tag, operation, family=[], forceExec=False, afterExec=None): """Executes the operation commands (like "actionstart", "actionstop", etc). Replace the tags in the action command with actions properties @@ -395,12 +395,17 @@ class CommandAction(ActionBase): cmd = self._getOperation(tag, 'inet4') if not family or 'inet4' in family: if cmd: - res &= self.executeCmd(cmd, self.timeout) + ret = self.executeCmd(cmd, self.timeout) + res &= ret + if afterExec: afterExec('inet4' if 'inet4' in family else '', ret) # execute ipv6 operation if available (and not the same as ipv4): if allowed_ipv6 and (not family or 'inet6' in family): cmd6 = self._getOperation(tag, 'inet6') - if cmd6 and cmd6 != cmd: # - avoid double execution of same command - res &= self.executeCmd(cmd6, self.timeout) + forceExec |= (family and 'inet6' in family and 'inet4' not in family) + if cmd6 and (forceExec or cmd6 != cmd): # - avoid double execution of same command + ret = self.executeCmd(cmd6, self.timeout) + res &= ret + if afterExec: afterExec('inet6' if 'inet6' in family else '', ret) if not res: raise RuntimeError("Error %s action %s/%s" % (operation, self._jail, self._name,)) except ValueError as e: @@ -440,10 +445,11 @@ class CommandAction(ActionBase): if self._startOnDemand: if not forceStart: return True - elif self.__started.get(family): # pragma: no cover - normally unreachable + elif not forceStart and self.__started.get(family): # pragma: no cover - normally unreachable return True - ret = self._executeOperation('', 'starting', family=family) - self.__started[family] = ret + ret = self._executeOperation('', 'starting', family=[family], forceExec=forceStart) + if ret: + self.__started[family] = 1 return ret def ban(self, aInfo): @@ -459,13 +465,14 @@ class CommandAction(ActionBase): the ban. """ # if we should start the action on demand (conditional by family): + family = aInfo.get('family', '') if self._startOnDemand: - family = aInfo.get('family') if not self.__started.get(family): self.start(family, forceStart=True) # ban: if not self._processCmd('', aInfo): raise RuntimeError("Error banning %(ip)s" % aInfo) + self.__started[family] |= 2; # contains items def unban(self, aInfo): """Executes the "actionunban" command. @@ -479,8 +486,10 @@ class CommandAction(ActionBase): Dictionary which includes information in relation to the ban. """ - if not self._processCmd('', aInfo): - raise RuntimeError("Error unbanning %(ip)s" % aInfo) + family = aInfo.get('family', '') + if self.__started.get(family) & 2: # contains items + if not self._processCmd('', aInfo): + raise RuntimeError("Error unbanning %(ip)s" % aInfo) def flush(self): """Executes the "actionflush" command. @@ -491,27 +500,34 @@ class CommandAction(ActionBase): Replaces the tags in the action command with actions properties and executes the resulting command. """ - family = [] - # collect started families, if started on demand (conditional): - if self._startOnDemand: - family = [f for (f,v) in self.__started.iteritems() if v] - # if no started (on demand) actions: - if not family: return True - return self._executeOperation('', 'flushing', family=family) + # collect started families, may be started on demand (conditional): + family = [f for (f,v) in self.__started.iteritems() if v & 3 == 3]; # started and contains items + # if nothing contains items: + if not family: return True + # flush: + def _afterFlush(family, ret): + if ret and self.__started.get(family): + self.__started[family] &= ~2; # no items anymore + return self._executeOperation('', 'flushing', family=family, afterExec=_afterFlush) - def stop(self): + def stop(self, family=None): """Executes the "actionstop" command. Replaces the tags in the action command with actions properties and executes the resulting command. """ - family = [] # collect started families, if started on demand (conditional): - if self._startOnDemand: + if family is None: family = [f for (f,v) in self.__started.iteritems() if v] # if no started (on demand) actions: if not family: return True - self.__started = {} + self.__started = {} + else: + try: + self.__started[family] &= 0 + family = [family] + except KeyError: # pragma: no cover + return True return self._executeOperation('', 'stopping', family=family) def reload(self, **kwargs): @@ -533,8 +549,9 @@ class CommandAction(ActionBase): ret = True # for each started family: if self.actioncheck: - for (family, started) in self.__started.iteritems(): + for (family, started) in self.__started.items(): if started and not self._invariantCheck(family, beforeRepair): + self.__started[family] = 0 ret &= False return ret @@ -733,6 +750,9 @@ class CommandAction(ActionBase): def _invariantCheck(self, family='', beforeRepair=None): """Executes a substituted `actioncheck` command. """ + # for started action/family only (avoid check not started inet4 if inet6 gets broken): + if family != '' and family not in self.__started and not self.__started.get(''): + return True checkCmd = self._getOperation('', family) if not checkCmd or self.executeCmd(checkCmd, self.timeout): return True @@ -754,10 +774,10 @@ class CommandAction(ActionBase): # but the tickets are still in BanManager, so in case of new failures # it will not be banned, because "already banned" will happen. try: - self.stop() + self.stop(family) except RuntimeError: # bypass error in stop (if start/check succeeded hereafter). pass - self.start() + self.start(family) if not self.executeCmd(checkCmd, self.timeout): self._logSys.critical("Unable to restore environment") return False diff --git a/fail2ban/server/actions.py b/fail2ban/server/actions.py index 35f2306b..b65a77ea 100644 --- a/fail2ban/server/actions.py +++ b/fail2ban/server/actions.py @@ -516,23 +516,24 @@ class Actions(JailThread, Mapping): try: if hasattr(action, 'flush') and (not isinstance(action, CommandAction) or action.actionflush): logSys.notice("[%s] Flush ticket(s) with %s", self._jail.name, name) - action.flush() - else: - unbactions[name] = action + if action.flush(): + continue except Exception as e: logSys.error("Failed to flush bans in jail '%s' action '%s': %s", self._jail.name, name, e, exc_info=logSys.getEffectiveLevel()<=logging.DEBUG) logSys.info("No flush occured, do consistency check") - def _beforeRepair(): - if stop: - self._logSys.error("Invariant check failed. Flush is impossible.") - return False - return True - if not hasattr(action, 'consistencyCheck') or action.consistencyCheck(_beforeRepair): - # fallback to single unbans: - logSys.info("unban tickets each individualy") - unbactions[name] = action + if hasattr(action, 'consistencyCheck'): + def _beforeRepair(): + if stop and not getattr(action, 'actionrepair_on_unban', None): # don't need repair on stop + self._logSys.error("Invariant check failed. Flush is impossible.") + return False + return True + action.consistencyCheck(_beforeRepair) + continue + # fallback to single unbans: + logSys.debug(" Unban tickets each individualy") + unbactions[name] = action actions = unbactions # flush the database also: if db and self._jail.database is not None: diff --git a/fail2ban/tests/actionstestcase.py b/fail2ban/tests/actionstestcase.py index 9d6b16c6..0019b490 100644 --- a/fail2ban/tests/actionstestcase.py +++ b/fail2ban/tests/actionstestcase.py @@ -48,15 +48,15 @@ class ExecuteActions(LogCaptureTestCase): def tearDown(self): super(ExecuteActions, self).tearDown() - def defaultAction(self): + def defaultAction(self, o={}): self.__actions.add('ip') act = self.__actions['ip'] - act.actionstart = 'echo ip start' + act.actionstart = 'echo ip start'+o.get('start', '') act.actionban = 'echo ip ban ' act.actionunban = 'echo ip unban ' - act.actioncheck = 'echo ip check' - act.actionflush = 'echo ip flush ' - act.actionstop = 'echo ip stop' + act.actioncheck = 'echo ip check'+o.get('check', '') + act.actionflush = 'echo ip flush'+o.get('flush', '') + act.actionstop = 'echo ip stop'+o.get('stop', '') return act def testActionsAddDuplicateName(self): @@ -211,11 +211,10 @@ class ExecuteActions(LogCaptureTestCase): self.assertLogged('Unbanned 30, 0 ticket(s)') self.assertNotLogged('Unbanned 50, 0 ticket(s)') - @with_alt_time def testActionsConsistencyCheck(self): + act = self.defaultAction({'check':' ', 'flush':' '}) # flush for inet6 is intentionally "broken" here - test no unhandled except and invariant check: - act = self.defaultAction() - act['actionflush?family=inet6'] = 'echo ip flush ; exit 1' + act['actionflush?family=inet6'] = act.actionflush + '; exit 1' act.actionstart_on_demand = True self.__actions.start() self.assertNotLogged("stdout: %r" % 'ip start') @@ -230,26 +229,137 @@ class ExecuteActions(LogCaptureTestCase): # check should fail (so cause stop/start): self.pruneLog('[test-phase 1] simulate inconsistent env') - act['actioncheck?family=inet6'] = 'echo ip check ; exit 1' + act['actioncheck?family=inet6'] = act.actioncheck + '; exit 1' self.__actions._Actions__flushBan() - self.assertLogged('Failed to flush bans', + self.assertLogged( + "stdout: %r" % 'ip flush inet4', + "stdout: %r" % 'ip flush inet6', + 'Failed to flush bans', 'No flush occured, do consistency check', 'Invariant check failed. Trying to restore a sane environment', - "stdout: %r" % 'ip stop', - "stdout: %r" % 'ip start', + "stdout: %r" % 'ip stop', # same for both families + 'Unable to restore environment', all=True, wait=True) # check succeeds: self.pruneLog('[test-phase 2] consistent env') act['actioncheck?family=inet6'] = act.actioncheck + self.assertEqual(self.__actions.addBannedIP('2001:db8::1'), 1) + self.assertLogged('Ban 2001:db8::1', + "stdout: %r" % 'ip start', # same for both families + "stdout: %r" % 'ip ban 2001:db8::1', + all=True, wait=True) + self.assertNotLogged("stdout: %r" % 'ip check inet4', + all=True) + + self.pruneLog('[test-phase 3] failed flush in consistent env') self.__actions._Actions__flushBan() self.assertLogged('Failed to flush bans', 'No flush occured, do consistency check', - "stdout: %r" % 'ip ban 192.0.2.1', + "stdout: %r" % 'ip flush inet6', + "stdout: %r" % 'ip check inet6', all=True, wait=True) + self.assertNotLogged( + "stdout: %r" % 'ip flush inet4', + "stdout: %r" % 'ip stop', + "stdout: %r" % 'ip start', + 'Unable to restore environment', + all=True) + # stop, flush succeeds: + self.pruneLog('[test-phase end] flush successful') act['actionflush?family=inet6'] = act.actionflush - self.__actions.stop() self.__actions.join() + self.assertLogged( + "stdout: %r" % 'ip flush inet6', + "stdout: %r" % 'ip stop', # same for both families + 'action ip terminated', + all=True, wait=True) + # no flush for inet4 (already successfully flushed): + self.assertNotLogged("ERROR", + "stdout: %r" % 'ip flush inet4', + 'Unban tickets each individualy', + all=True) + def testActionsConsistencyCheckDiffFam(self): + # same as testActionsConsistencyCheck, but different start/stop commands for both families + act = self.defaultAction({'start':' ', 'check':' ', 'flush':' ', 'stop':' '}) + # flush for inet6 is intentionally "broken" here - test no unhandled except and invariant check: + act['actionflush?family=inet6'] = act.actionflush + '; exit 1' + act.actionstart_on_demand = True + self.__actions.start() + self.assertNotLogged("stdout: %r" % 'ip start') + + self.assertEqual(self.__actions.addBannedIP('192.0.2.1'), 1) + self.assertEqual(self.__actions.addBannedIP('2001:db8::1'), 1) + self.assertLogged('Ban 192.0.2.1', 'Ban 2001:db8::1', + "stdout: %r" % 'ip start inet4', + "stdout: %r" % 'ip ban 192.0.2.1', + "stdout: %r" % 'ip start inet6', + "stdout: %r" % 'ip ban 2001:db8::1', + all=True, wait=True) + + # check should fail (so cause stop/start): + self.pruneLog('[test-phase 1] simulate inconsistent env') + act['actioncheck?family=inet6'] = act.actioncheck + '; exit 1' + self.__actions._Actions__flushBan() + self.assertLogged( + "stdout: %r" % 'ip flush inet4', + "stdout: %r" % 'ip flush inet6', + 'Failed to flush bans', + 'No flush occured, do consistency check', + 'Invariant check failed. Trying to restore a sane environment', + "stdout: %r" % 'ip stop inet6', + 'Unable to restore environment', + all=True, wait=True) + # start/stop should be called for inet6 only: + self.assertNotLogged( + "stdout: %r" % 'ip stop inet4', + all=True) + + # check succeeds: + self.pruneLog('[test-phase 2] consistent env') + act['actioncheck?family=inet6'] = act.actioncheck + self.assertEqual(self.__actions.addBannedIP('2001:db8::1'), 1) + self.assertLogged('Ban 2001:db8::1', + "stdout: %r" % 'ip start inet6', + "stdout: %r" % 'ip ban 2001:db8::1', + all=True, wait=True) + self.assertNotLogged( + "stdout: %r" % 'ip check inet4', + "stdout: %r" % 'ip start inet4', + all=True) + + self.pruneLog('[test-phase 3] failed flush in consistent env') + act['actioncheck?family=inet6'] = act.actioncheck + self.__actions._Actions__flushBan() + self.assertLogged('Failed to flush bans', + 'No flush occured, do consistency check', + "stdout: %r" % 'ip flush inet6', + "stdout: %r" % 'ip check inet6', + all=True, wait=True) + self.assertNotLogged( + "stdout: %r" % 'ip flush inet4', + "stdout: %r" % 'ip stop inet4', + "stdout: %r" % 'ip start inet4', + "stdout: %r" % 'ip stop inet6', + "stdout: %r" % 'ip start inet6', + all=True) + + # stop, flush succeeds: + self.pruneLog('[test-phase end] flush successful') + act['actionflush?family=inet6'] = act.actionflush + self.__actions.stop() + self.__actions.join() + self.assertLogged( + "stdout: %r" % 'ip flush inet6', + "stdout: %r" % 'ip stop inet4', + "stdout: %r" % 'ip stop inet6', + 'action ip terminated', + all=True, wait=True) + # no flush for inet4 (already successfully flushed): + self.assertNotLogged("ERROR", + "stdout: %r" % 'ip flush inet4', + 'Unban tickets each individualy', + all=True) From 165b7d66439fa46fb024b3849be8e657f819c564 Mon Sep 17 00:00:00 2001 From: sebres Date: Mon, 6 Jan 2020 21:00:10 +0100 Subject: [PATCH 12/20] tests fixed, prepared for other conditional operations (for subnet usage), operations like repair/flush/stop considering started families (executed for started only) --- fail2ban/server/action.py | 142 +++++++++++++++++++++++-------- fail2ban/tests/actiontestcase.py | 89 ++++++++++++------- 2 files changed, 164 insertions(+), 67 deletions(-) diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index b0821026..0c3bbf2a 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -36,7 +36,8 @@ from .failregex import mapTag2Opt from .ipdns import asip, DNSUtils from .mytime import MyTime from .utils import Utils -from ..helpers import getLogger, _merge_copy_dicts, uni_string, substituteRecursiveTags, TAG_CRE, MAX_TAG_REPLACE_COUNT +from ..helpers import getLogger, _merge_copy_dicts, \ + splitwords, substituteRecursiveTags, uni_string, TAG_CRE, MAX_TAG_REPLACE_COUNT # Gets the instance of the logger. logSys = getLogger(__name__) @@ -51,7 +52,7 @@ allowed_ipv6 = True FCUSTAG_CRE = re.compile(r''); # currently uppercase only COND_FAMILIES = ('inet4', 'inet6') -CONDITIONAL_FAM_RE = re.compile(r"^(\w+)\?(family)=") +CONDITIONAL_FAM_RE = re.compile(r"^(\w+)\?(family)=(.*)$") # Special tags: DYN_REPL_TAGS = { @@ -382,7 +383,33 @@ class CommandAction(ActionBase): addrepl=(lambda tag:family if tag == 'family' else None), cache=self.__substCache) - def _executeOperation(self, tag, operation, family=[], forceExec=False, afterExec=None): + def _operationExecuted(self, tag, family, *args): + """ Get, set or delete command of operation considering family. + """ + key = ('__eOpCmd',tag) + if not len(args): # get + if not callable(family): # pragma: no cover + return self.__substCache.get(key, {}).get(family) + # family as expression - use it to filter values: + return [v for f, v in self.__substCache.get(key, {}).iteritems() if family(f)] + cmd = args[0] + if cmd: # set: + try: + famd = self.__substCache[key] + except KeyError: + famd = self.__substCache[key] = {} + famd[family] = cmd + else: # delete (given family and all other with same command): + try: + famd = self.__substCache[key] + cmd = famd.pop(family) + for family, v in famd.items(): + if v == cmd: + del famd[family] + except KeyError: # pragma: no cover + pass + + def _executeOperation(self, tag, operation, family=[], afterExec=None): """Executes the operation commands (like "actionstart", "actionstop", etc). Replace the tags in the action command with actions properties @@ -390,26 +417,24 @@ class CommandAction(ActionBase): """ # check valid tags in properties (raises ValueError if self recursion, etc.): res = True - try: - # common (resp. ipv4): - cmd = self._getOperation(tag, 'inet4') - if not family or 'inet4' in family: - if cmd: + err = 'Script error' + if not family: # all started: + family = [famoper for (famoper,v) in self.__started.iteritems() if v] + for famoper in family: + try: + cmd = self._getOperation(tag, famoper) + ret = True + # avoid double execution of same command for both families: + if cmd and cmd not in self._operationExecuted(tag, lambda f: f != famoper): ret = self.executeCmd(cmd, self.timeout) res &= ret - if afterExec: afterExec('inet4' if 'inet4' in family else '', ret) - # execute ipv6 operation if available (and not the same as ipv4): - if allowed_ipv6 and (not family or 'inet6' in family): - cmd6 = self._getOperation(tag, 'inet6') - forceExec |= (family and 'inet6' in family and 'inet4' not in family) - if cmd6 and (forceExec or cmd6 != cmd): # - avoid double execution of same command - ret = self.executeCmd(cmd6, self.timeout) - res &= ret - if afterExec: afterExec('inet6' if 'inet6' in family else '', ret) - if not res: - raise RuntimeError("Error %s action %s/%s" % (operation, self._jail, self._name,)) - except ValueError as e: - raise RuntimeError("Error %s action %s/%s: %r" % (operation, self._jail, self._name, e)) + if afterExec: afterExec(famoper, ret) + self._operationExecuted(tag, famoper, cmd if ret else None) + except ValueError as e: + res = False + err = e + if not res: + raise RuntimeError("Error %s action %s/%s: %r" % (operation, self._jail, self._name, err)) return res @property @@ -417,13 +442,33 @@ class CommandAction(ActionBase): v = self._properties.get('__hasCondSection') if v is None: v = False + famset = set() for n in self._properties: - if CONDITIONAL_FAM_RE.match(n): - v = True - break + grp = CONDITIONAL_FAM_RE.match(n) + if grp: + self._properties['__hasCondSection'] = v = True + if self._properties.get('families') or self._startOnDemand: + break + famset.add(grp.group(2)) + self._properties['__families'] = famset self._properties['__hasCondSection'] = v return v + @property + def _families(self): + v = self._properties.get('__families') + if v: return v + v = self._properties.get('families') + if v and not isinstance(v, (list,set)): # pragma: no cover - still unused + v = splitwords(v) + elif self._hasCondSection: # all conditional families: + # todo: check it is needed at all # common (resp. ipv4) + ipv6 if allowed: + v = ['inet4', 'inet6'] if allowed_ipv6 else ['inet4'] + else: # all action tags seems to be the same + v = [''] + self._properties['__families'] = v + return v + @property def _startOnDemand(self): """Checks the action depends on family (conditional)""" @@ -435,7 +480,15 @@ class CommandAction(ActionBase): self._properties['actionstart_on_demand'] = v return v - def start(self, family=None, forceStart=False): + def start(self): + """Executes the "actionstart" command. + + Replace the tags in the action command with actions properties + and executes the resulting command. + """ + return self._start() + + def _start(self, family='', forceStart=False): """Executes the "actionstart" command. Replace the tags in the action command with actions properties @@ -447,9 +500,12 @@ class CommandAction(ActionBase): return True elif not forceStart and self.__started.get(family): # pragma: no cover - normally unreachable return True - ret = self._executeOperation('', 'starting', family=[family], forceExec=forceStart) - if ret: - self.__started[family] = 1 + family = [family] if family != '' else self._families + def _started(family, ret): + if ret: + self._operationExecuted('', family, None) + self.__started[family] = 1 + ret = self._executeOperation('', 'starting', family=family, afterExec=_started) return ret def ban(self, aInfo): @@ -468,11 +524,11 @@ class CommandAction(ActionBase): family = aInfo.get('family', '') if self._startOnDemand: if not self.__started.get(family): - self.start(family, forceStart=True) + self._start(family, forceStart=True) # ban: if not self._processCmd('', aInfo): raise RuntimeError("Error banning %(ip)s" % aInfo) - self.__started[family] |= 2; # contains items + self.__started[family] = self.__started.get(family, 0) | 3; # started and contains items def unban(self, aInfo): """Executes the "actionunban" command. @@ -487,7 +543,7 @@ class CommandAction(ActionBase): the ban. """ family = aInfo.get('family', '') - if self.__started.get(family) & 2: # contains items + if self.__started.get(family, 0) & 2: # contains items if not self._processCmd('', aInfo): raise RuntimeError("Error unbanning %(ip)s" % aInfo) @@ -510,14 +566,22 @@ class CommandAction(ActionBase): self.__started[family] &= ~2; # no items anymore return self._executeOperation('', 'flushing', family=family, afterExec=_afterFlush) - def stop(self, family=None): + def stop(self): + """Executes the "actionstop" command. + + Replaces the tags in the action command with actions properties + and executes the resulting command. + """ + return self._stop() + + def _stop(self, family=''): """Executes the "actionstop" command. Replaces the tags in the action command with actions properties and executes the resulting command. """ # collect started families, if started on demand (conditional): - if family is None: + if not family: family = [f for (f,v) in self.__started.iteritems() if v] # if no started (on demand) actions: if not family: return True @@ -528,7 +592,10 @@ class CommandAction(ActionBase): family = [family] except KeyError: # pragma: no cover return True - return self._executeOperation('', 'stopping', family=family) + def _stopped(family, ret): + if ret: + self._operationExecuted('', family, None) + return self._executeOperation('', 'stopping', family=family, afterExec=_stopped) def reload(self, **kwargs): """Executes the "actionreload" command. @@ -551,7 +618,9 @@ class CommandAction(ActionBase): if self.actioncheck: for (family, started) in self.__started.items(): if started and not self._invariantCheck(family, beforeRepair): + # reset started flag and command of executed operation: self.__started[family] = 0 + self._operationExecuted('', family, None) ret &= False return ret @@ -767,6 +836,7 @@ class CommandAction(ActionBase): if not self.executeCmd(repairCmd, self.timeout): self._logSys.critical("Unable to restore environment") return False + self.__started[family] = 1 else: # no repair command, try to restart action... # [WARNING] TODO: be sure all banactions get a repair command, because @@ -774,10 +844,10 @@ class CommandAction(ActionBase): # but the tickets are still in BanManager, so in case of new failures # it will not be banned, because "already banned" will happen. try: - self.stop(family) + self._stop(family) except RuntimeError: # bypass error in stop (if start/check succeeded hereafter). pass - self.start(family) + self._start(family) if not self.executeCmd(checkCmd, self.timeout): self._logSys.critical("Unable to restore environment") return False diff --git a/fail2ban/tests/actiontestcase.py b/fail2ban/tests/actiontestcase.py index 1c5a0807..a24a2b77 100644 --- a/fail2ban/tests/actiontestcase.py +++ b/fail2ban/tests/actiontestcase.py @@ -34,8 +34,8 @@ from ..server.actions import OrderedDict, Actions from ..server.utils import Utils from .dummyjail import DummyJail -from .utils import LogCaptureTestCase -from .utils import pid_exists +from .utils import pid_exists, with_tmpdir, LogCaptureTestCase + class CommandActionTest(LogCaptureTestCase): @@ -297,18 +297,21 @@ class CommandActionTest(LogCaptureTestCase): "Text 000-567 text 567 '567'") self.assertTrue(len(cache) >= 3) - - def testExecuteActionBan(self): - self.__action.actionstart = "touch /tmp/fail2ban.test" - self.assertEqual(self.__action.actionstart, "touch /tmp/fail2ban.test") - self.__action.actionstop = "rm -f /tmp/fail2ban.test" - self.assertEqual(self.__action.actionstop, 'rm -f /tmp/fail2ban.test') + @with_tmpdir + def testExecuteActionBan(self, tmp): + tmp += "/fail2ban.test" + self.__action.actionstart = "touch '%s'" % tmp + self.__action.actionrepair = self.__action.actionstart + self.assertEqual(self.__action.actionstart, "touch '%s'" % tmp) + self.__action.actionstop = "rm -f '%s'" % tmp + self.assertEqual(self.__action.actionstop, "rm -f '%s'" % tmp) self.__action.actionban = "echo -n" self.assertEqual(self.__action.actionban, 'echo -n') - self.__action.actioncheck = "[ -e /tmp/fail2ban.test ]" - self.assertEqual(self.__action.actioncheck, '[ -e /tmp/fail2ban.test ]') + self.__action.actioncheck = "[ -e '%s' ]" % tmp + self.assertEqual(self.__action.actioncheck, "[ -e '%s' ]" % tmp) self.__action.actionunban = "true" self.assertEqual(self.__action.actionunban, 'true') + self.pruneLog() self.assertNotLogged('returned') # no action was actually executed yet @@ -316,42 +319,66 @@ class CommandActionTest(LogCaptureTestCase): self.__action.ban({'ip': None}) self.assertLogged('Invariant check failed') self.assertLogged('returned successfully') + self.__action.stop() + self.assertLogged(self.__action.actionstop) def testExecuteActionEmptyUnban(self): + # unban will be executed for actions with banned items only: + self.__action.actionban = "" self.__action.actionunban = "" + self.__action.actionflush = "echo -n 'flush'" + self.__action.actionstop = "echo -n 'stop'" + self.__action.start(); + self.__action.ban({}); + self.pruneLog() self.__action.unban({}) - self.assertLogged('Nothing to do') + self.assertLogged('Nothing to do', wait=True) + # same as above but with interim flush, so no unban anymore: + self.__action.ban({}); + self.pruneLog('[phase 2]') + self.__action.flush() + self.__action.unban({}) + self.__action.stop() + self.assertLogged('stop', wait=True) + self.assertNotLogged('Nothing to do') - def testExecuteActionStartCtags(self): + @with_tmpdir + def testExecuteActionStartCtags(self, tmp): + tmp += '/fail2ban.test' self.__action.HOST = "192.0.2.0" - self.__action.actionstart = "touch /tmp/fail2ban.test." - self.__action.actionstop = "rm -f /tmp/fail2ban.test." - self.__action.actioncheck = "[ -e /tmp/fail2ban.test.192.0.2.0 ]" + self.__action.actionstart = "touch '%s.'" % tmp + self.__action.actionstop = "rm -f '%s.'" % tmp + self.__action.actioncheck = "[ -e '%s.192.0.2.0' ]" % tmp self.__action.start() + self.__action.consistencyCheck() - def testExecuteActionCheckRestoreEnvironment(self): + @with_tmpdir + def testExecuteActionCheckRestoreEnvironment(self, tmp): + tmp += '/fail2ban.test' self.__action.actionstart = "" - self.__action.actionstop = "rm -f /tmp/fail2ban.test" - self.__action.actionban = "rm /tmp/fail2ban.test" - self.__action.actioncheck = "[ -e /tmp/fail2ban.test ]" + self.__action.actionstop = "rm -f '%s'" % tmp + self.__action.actionban = "rm '%s'" % tmp + self.__action.actioncheck = "[ -e '%s' ]" % tmp self.assertRaises(RuntimeError, self.__action.ban, {'ip': None}) self.assertLogged('Invariant check failed', 'Unable to restore environment', all=True) # 2nd time, try to restore with producing error in stop, but succeeded start hereafter: self.pruneLog('[phase 2]') - self.__action.actionstart = "touch /tmp/fail2ban.test" - self.__action.actionstop = "rm /tmp/fail2ban.test" - self.__action.actionban = 'printf "%%b\n" >> /tmp/fail2ban.test' - self.__action.actioncheck = "[ -e /tmp/fail2ban.test ]" + self.__action.actionstart = "touch '%s'" % tmp + self.__action.actionstop = "rm '%s'" % tmp + self.__action.actionban = """printf "%%%%b\n" >> '%s'""" % tmp + self.__action.actioncheck = "[ -e '%s' ]" % tmp self.__action.ban({'ip': None}) self.assertLogged('Invariant check failed') self.assertNotLogged('Unable to restore environment') - def testExecuteActionCheckRepairEnvironment(self): + @with_tmpdir + def testExecuteActionCheckRepairEnvironment(self, tmp): + tmp += '/fail2ban.test' self.__action.actionstart = "" self.__action.actionstop = "" - self.__action.actionban = "rm /tmp/fail2ban.test" - self.__action.actioncheck = "[ -e /tmp/fail2ban.test ]" - self.__action.actionrepair = "echo 'repair ...'; touch /tmp/fail2ban.test" + self.__action.actionban = "rm '%s'" % tmp + self.__action.actioncheck = "[ -e '%s' ]" % tmp + self.__action.actionrepair = "echo 'repair ...'; touch '%s'" % tmp # 1st time with success repair: self.__action.ban({'ip': None}) self.assertLogged("Invariant check failed. Trying", "echo 'repair ...'", all=True) @@ -379,13 +406,13 @@ class CommandActionTest(LogCaptureTestCase): 'user': "tester" } }) - self.__action.actionban = "touch /tmp/fail2ban.test.123; echo 'failure of -- from :'" - self.__action.actionunban = "rm /tmp/fail2ban.test.; echo 'user unbanned'" + self.__action.actionban = "echo ', failure of -- from :'" + self.__action.actionunban = "echo ', user unbanned'" self.__action.ban(aInfo) self.__action.unban(aInfo) self.assertLogged( - " -- stdout: 'failure 111 of tester -- from 192.0.2.1:222'", - " -- stdout: 'user tester unbanned'", + " -- stdout: '123, failure 111 of tester -- from 192.0.2.1:222'", + " -- stdout: '123, user tester unbanned'", all=True ) From 67fd75c88e939c0e09ce203f18f50fe460ada84f Mon Sep 17 00:00:00 2001 From: sebres Date: Mon, 6 Jan 2020 21:11:28 +0100 Subject: [PATCH 13/20] pass2allow-ftp: inverted handling - action should prohibit access per default for any IP, so reset start on demand parameter for this action (will be started immediately). --- config/jail.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/jail.conf b/config/jail.conf index a77a47ba..fbc357f7 100644 --- a/config/jail.conf +++ b/config/jail.conf @@ -862,7 +862,7 @@ logpath = %(apache_access_log)s blocktype = RETURN returntype = DROP action = %(action_)s[blocktype=%(blocktype)s, returntype=%(returntype)s, - actionstart_on_demand=True, actionrepair_on_unban=True] + actionstart_on_demand=false, actionrepair_on_unban=true] bantime = 1h maxretry = 1 findtime = 1 From a527fbcae55259fd40851e56f2a88ecf6cfd6247 Mon Sep 17 00:00:00 2001 From: sebres Date: Mon, 6 Jan 2020 21:37:55 +0100 Subject: [PATCH 14/20] small amend: if not on-demand, the families should be specified (or default), also avoids error "dictionary changed size during iteration" --- fail2ban/server/action.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index 0c3bbf2a..b825bb96 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -440,18 +440,14 @@ class CommandAction(ActionBase): @property def _hasCondSection(self): v = self._properties.get('__hasCondSection') - if v is None: - v = False - famset = set() - for n in self._properties: - grp = CONDITIONAL_FAM_RE.match(n) - if grp: - self._properties['__hasCondSection'] = v = True - if self._properties.get('families') or self._startOnDemand: - break - famset.add(grp.group(2)) - self._properties['__families'] = famset - self._properties['__hasCondSection'] = v + if v is not None: + return v + v = False + for n in self._properties: + if CONDITIONAL_FAM_RE.match(n): + v = True + break + self._properties['__hasCondSection'] = v return v @property From b7fe33483ae6943e0d942849605dacae42856b30 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 7 Jan 2020 11:03:41 +0100 Subject: [PATCH 15/20] coverage --- fail2ban/server/action.py | 4 ++-- fail2ban/server/actions.py | 4 ++-- fail2ban/tests/actiontestcase.py | 13 +++++++++++++ fail2ban/tests/filtertestcase.py | 20 ++++++++++++++++---- 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index b825bb96..0c7e1207 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -33,7 +33,7 @@ from abc import ABCMeta from collections import MutableMapping from .failregex import mapTag2Opt -from .ipdns import asip, DNSUtils +from .ipdns import DNSUtils from .mytime import MyTime from .utils import Utils from ..helpers import getLogger, _merge_copy_dicts, \ @@ -175,7 +175,7 @@ class CallingMap(MutableMapping, object): def __len__(self): return len(self.data) - def copy(self): # pragma: no cover + def copy(self): return self.__class__(_merge_copy_dicts(self.data, self.storage)) diff --git a/fail2ban/server/actions.py b/fail2ban/server/actions.py index b65a77ea..ed93c971 100644 --- a/fail2ban/server/actions.py +++ b/fail2ban/server/actions.py @@ -214,10 +214,10 @@ class Actions(JailThread, Mapping): if isinstance(ip, list): # Multiple IPs: - tickets = (BanTicket(ip if isinstance(ip, IPAddr) else IPAddr(ip), unixTime) for ip in ip) + tickets = (BanTicket(ip, unixTime) for ip in ip) else: # Single IP: - tickets = (BanTicket(ip if isinstance(ip, IPAddr) else IPAddr(ip), unixTime),) + tickets = (BanTicket(ip, unixTime),) return self.__checkBan(tickets) diff --git a/fail2ban/tests/actiontestcase.py b/fail2ban/tests/actiontestcase.py index a24a2b77..3425667c 100644 --- a/fail2ban/tests/actiontestcase.py +++ b/fail2ban/tests/actiontestcase.py @@ -587,6 +587,19 @@ class CommandActionTest(LogCaptureTestCase): self.assertEqual(len(m), 3) self.assertIn('c', m) self.assertEqual((m['a'], m['b'], m['c']), (5, 11, 'test')) + # immutability of copy: + m['d'] = 'dddd' + m2 = m.copy() + m2['c'] = lambda self: self['a'] + 7 + m2['a'] = 1 + del m2['b'] + del m2['d'] + self.assertTrue('b' in m) + self.assertTrue('d' in m) + self.assertFalse('b' in m2) + self.assertFalse('d' in m2) + self.assertEqual((m['a'], m['b'], m['c'], m['d']), (5, 11, 'test', 'dddd')) + self.assertEqual((m2['a'], m2['c']), (1, 8)) def testCallingMapRep(self): m = CallingMap({ diff --git a/fail2ban/tests/filtertestcase.py b/fail2ban/tests/filtertestcase.py index 41a6f0bf..d6ad8235 100644 --- a/fail2ban/tests/filtertestcase.py +++ b/fail2ban/tests/filtertestcase.py @@ -40,7 +40,7 @@ from ..server.jail import Jail from ..server.filterpoll import FilterPoll from ..server.filter import FailTicket, Filter, FileFilter, FileContainer from ..server.failmanager import FailManagerEmpty -from ..server.ipdns import getfqdn, DNSUtils, IPAddr +from ..server.ipdns import asip, getfqdn, DNSUtils, IPAddr from ..server.mytime import MyTime from ..server.utils import Utils, uni_decode from .utils import setUpMyTime, tearDownMyTime, mtimesleep, with_tmpdir, LogCaptureTestCase, \ @@ -1843,11 +1843,15 @@ class DNSUtilsNetworkTests(unittest.TestCase): def setUp(self): """Call before every test case.""" super(DNSUtilsNetworkTests, self).setUp() - unittest.F2B.SkipIfNoNetwork() + #unittest.F2B.SkipIfNoNetwork() def test_IPAddr(self): - self.assertTrue(IPAddr('192.0.2.1').isIPv4) - self.assertTrue(IPAddr('2001:DB8::').isIPv6) + ip4 = IPAddr('192.0.2.1') + ip6 = IPAddr('2001:DB8::') + self.assertTrue(ip4.isIPv4) + self.assertTrue(ip6.isIPv6) + self.assertTrue(asip('192.0.2.1').isIPv4) + self.assertTrue(id(asip(ip4)) == id(ip4)) def test_IPAddr_Raw(self): # raw string: @@ -1884,6 +1888,7 @@ class DNSUtilsNetworkTests(unittest.TestCase): def testUseDns(self): res = DNSUtils.textToIp('www.example.com', 'no') self.assertSortedEqual(res, []) + unittest.F2B.SkipIfNoNetwork() res = DNSUtils.textToIp('www.example.com', 'warn') # sort ipaddr, IPv4 is always smaller as IPv6 self.assertSortedEqual(res, ['93.184.216.34', '2606:2800:220:1:248:1893:25c8:1946']) @@ -1892,6 +1897,7 @@ class DNSUtilsNetworkTests(unittest.TestCase): self.assertSortedEqual(res, ['93.184.216.34', '2606:2800:220:1:248:1893:25c8:1946']) def testTextToIp(self): + unittest.F2B.SkipIfNoNetwork() # Test hostnames hostnames = [ 'www.example.com', @@ -1905,6 +1911,8 @@ class DNSUtilsNetworkTests(unittest.TestCase): self.assertSortedEqual(res, ['93.184.216.34', '2606:2800:220:1:248:1893:25c8:1946']) else: self.assertSortedEqual(res, []) + + def testIpToIp(self): # pure ips: for s in ('93.184.216.34', '2606:2800:220:1:248:1893:25c8:1946'): ips = DNSUtils.textToIp(s, 'yes') @@ -2061,6 +2069,7 @@ class DNSUtilsNetworkTests(unittest.TestCase): self.assertTrue(IPAddr("2606:2800:220:1:248:1893:25c8:1946").isInNet(ips)) def testIPAddr_wrongDNS_IP(self): + unittest.F2B.SkipIfNoNetwork() DNSUtils.dnsToIp('`this`.dns-is-wrong.`wrong-nic`-dummy') DNSUtils.ipToName('*') @@ -2083,6 +2092,9 @@ class DNSUtilsNetworkTests(unittest.TestCase): self.assertEqual(getfqdn(lname), lname) # coverage (targeting all branches): FQDN from loopback and DNS blackhole is always the same: self.assertIn(getfqdn('localhost.'), ('localhost', 'localhost.')) + + def testFQDN_DNS(self): + unittest.F2B.SkipIfNoNetwork() self.assertIn(getfqdn('as112.arpa.'), ('as112.arpa.', 'as112.arpa')) From 125da61bda62f8e82f73dd54729cbd5807066c80 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 7 Jan 2020 15:50:54 +0100 Subject: [PATCH 16/20] more cases covered, start in repair distinguish operations, on demand flag etc --- fail2ban/server/action.py | 37 ++++++++++++++------------ fail2ban/tests/actionstestcase.py | 43 +++++++++++++++++++++++++++---- 2 files changed, 58 insertions(+), 22 deletions(-) diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index 0c7e1207..df0e3b8d 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -484,7 +484,7 @@ class CommandAction(ActionBase): """ return self._start() - def _start(self, family='', forceStart=False): + def _start(self, family=None, forceStart=False): """Executes the "actionstart" command. Replace the tags in the action command with actions properties @@ -496,7 +496,7 @@ class CommandAction(ActionBase): return True elif not forceStart and self.__started.get(family): # pragma: no cover - normally unreachable return True - family = [family] if family != '' else self._families + family = [family] if family is not None else self._families def _started(family, ret): if ret: self._operationExecuted('', family, None) @@ -570,14 +570,14 @@ class CommandAction(ActionBase): """ return self._stop() - def _stop(self, family=''): + def _stop(self, family=None): """Executes the "actionstop" command. Replaces the tags in the action command with actions properties and executes the resulting command. """ # collect started families, if started on demand (conditional): - if not family: + if family is None: family = [f for (f,v) in self.__started.iteritems() if v] # if no started (on demand) actions: if not family: return True @@ -812,26 +812,27 @@ class CommandAction(ActionBase): realCmd = Utils.buildShellCmd(realCmd, varsDict) return realCmd - def _invariantCheck(self, family='', beforeRepair=None): + def _invariantCheck(self, family=None, beforeRepair=None, forceStart=True): """Executes a substituted `actioncheck` command. """ # for started action/family only (avoid check not started inet4 if inet6 gets broken): - if family != '' and family not in self.__started and not self.__started.get(''): - return True + if not forceStart and family is not None and family not in self.__started: + return 1 checkCmd = self._getOperation('', family) if not checkCmd or self.executeCmd(checkCmd, self.timeout): - return True + return 1 # if don't need repair/restore - just return: if beforeRepair and not beforeRepair(): - return False + return -1 self._logSys.error( "Invariant check failed. Trying to restore a sane environment") # try to find repair command, if exists - exec it: repairCmd = self._getOperation('', family) if repairCmd: if not self.executeCmd(repairCmd, self.timeout): + self.__started[family] = 0 self._logSys.critical("Unable to restore environment") - return False + return 0 self.__started[family] = 1 else: # no repair command, try to restart action... @@ -843,11 +844,11 @@ class CommandAction(ActionBase): self._stop(family) except RuntimeError: # bypass error in stop (if start/check succeeded hereafter). pass - self._start(family) - if not self.executeCmd(checkCmd, self.timeout): + self._start(family, forceStart=forceStart or not self._startOnDemand) + if self.__started.get(family) and not self.executeCmd(checkCmd, self.timeout): self._logSys.critical("Unable to restore environment") - return False - return True + return 0 + return 1 def _processCmd(self, cmd, aInfo=None): """Executes a command with preliminary checks and substitutions. @@ -876,7 +877,7 @@ class CommandAction(ActionBase): # conditional corresponding family of the given ip: try: family = aInfo["family"] - except KeyError: + except (KeyError, TypeError): family = '' # invariant check: @@ -887,8 +888,10 @@ class CommandAction(ActionBase): self._logSys.error("Invariant check failed. Unban is impossible.") return False return True - ret = self._invariantCheck(family, _beforeRepair) - if not ret: + # check and repair if broken: + ret = self._invariantCheck(family, _beforeRepair, forceStart=(cmd != '')) + # if not sane (and not restored) return: + if ret != 1: return False # Replace static fields diff --git a/fail2ban/tests/actionstestcase.py b/fail2ban/tests/actionstestcase.py index 0019b490..c4dac77f 100644 --- a/fail2ban/tests/actionstestcase.py +++ b/fail2ban/tests/actionstestcase.py @@ -228,8 +228,12 @@ class ExecuteActions(LogCaptureTestCase): all=True, wait=True) # check should fail (so cause stop/start): - self.pruneLog('[test-phase 1] simulate inconsistent env') + self.pruneLog('[test-phase 1a] simulate inconsistent irreparable env by unban') act['actioncheck?family=inet6'] = act.actioncheck + '; exit 1' + self.__actions.removeBannedIP('2001:db8::1') + self.assertLogged('Invariant check failed. Unban is impossible.', + wait=True) + self.pruneLog('[test-phase 1b] simulate inconsistent irreparable env by flush') self.__actions._Actions__flushBan() self.assertLogged( "stdout: %r" % 'ip flush inet4', @@ -238,7 +242,7 @@ class ExecuteActions(LogCaptureTestCase): 'No flush occured, do consistency check', 'Invariant check failed. Trying to restore a sane environment', "stdout: %r" % 'ip stop', # same for both families - 'Unable to restore environment', + 'Failed to flush bans', all=True, wait=True) # check succeeds: @@ -283,11 +287,12 @@ class ExecuteActions(LogCaptureTestCase): all=True) def testActionsConsistencyCheckDiffFam(self): - # same as testActionsConsistencyCheck, but different start/stop commands for both families + # same as testActionsConsistencyCheck, but different start/stop commands for both families and repair on unban act = self.defaultAction({'start':' ', 'check':' ', 'flush':' ', 'stop':' '}) # flush for inet6 is intentionally "broken" here - test no unhandled except and invariant check: act['actionflush?family=inet6'] = act.actionflush + '; exit 1' act.actionstart_on_demand = True + act.actionrepair_on_unban = True self.__actions.start() self.assertNotLogged("stdout: %r" % 'ip start') @@ -301,8 +306,36 @@ class ExecuteActions(LogCaptureTestCase): all=True, wait=True) # check should fail (so cause stop/start): - self.pruneLog('[test-phase 1] simulate inconsistent env') act['actioncheck?family=inet6'] = act.actioncheck + '; exit 1' + self.pruneLog('[test-phase 1a] simulate inconsistent irreparable env by unban') + self.__actions.removeBannedIP('2001:db8::1') + self.assertLogged('Invariant check failed. Trying to restore a sane environment', + "stdout: %r" % 'ip stop inet6', + all=True, wait=True) + self.assertNotLogged( + "stdout: %r" % 'ip start inet6', # start on demand (not on repair) + "stdout: %r" % 'ip stop inet4', # family inet4 is not affected + "stdout: %r" % 'ip start inet4', + all=True) + + self.pruneLog('[test-phase 1b] simulate inconsistent irreparable env by ban') + self.assertEqual(self.__actions.addBannedIP('2001:db8::1'), 1) + self.assertLogged('Invariant check failed. Trying to restore a sane environment', + "stdout: %r" % 'ip stop inet6', + "stdout: %r" % 'ip start inet6', + "stdout: %r" % 'ip check inet6', + 'Unable to restore environment', + 'Failed to execute ban', + all=True, wait=True) + self.assertNotLogged( + "stdout: %r" % 'ip stop inet4', # family inet4 is not affected + "stdout: %r" % 'ip start inet4', + all=True) + + act['actioncheck?family=inet6'] = act.actioncheck + self.assertEqual(self.__actions.addBannedIP('2001:db8::2'), 1) + act['actioncheck?family=inet6'] = act.actioncheck + '; exit 1' + self.pruneLog('[test-phase 1c] simulate inconsistent irreparable env by flush') self.__actions._Actions__flushBan() self.assertLogged( "stdout: %r" % 'ip flush inet4', @@ -311,7 +344,7 @@ class ExecuteActions(LogCaptureTestCase): 'No flush occured, do consistency check', 'Invariant check failed. Trying to restore a sane environment', "stdout: %r" % 'ip stop inet6', - 'Unable to restore environment', + 'Failed to flush bans in jail', all=True, wait=True) # start/stop should be called for inet6 only: self.assertNotLogged( From 1a9bc1905dc273e2977a29e802e3b6df64d613a6 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 7 Jan 2020 17:01:47 +0100 Subject: [PATCH 17/20] auto-detection of IPv6 subsystem availability (important for not on-demand actions or jails, like pass2allow) --- fail2ban/server/action.py | 6 +++--- fail2ban/server/ipdns.py | 5 +++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index df0e3b8d..f26b6bc8 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -45,8 +45,8 @@ logSys = getLogger(__name__) # Create a lock for running system commands _cmd_lock = threading.Lock() -# Todo: make it configurable resp. automatically set, ex.: `[ -f /proc/net/if_inet6 ] && echo 'yes' || echo 'no'`: -allowed_ipv6 = True +# Specifies whether IPv6 subsystem is available: +allowed_ipv6 = DNSUtils.IPv6IsAllowed # capture groups from filter for map to ticket data: FCUSTAG_CRE = re.compile(r''); # currently uppercase only @@ -459,7 +459,7 @@ class CommandAction(ActionBase): v = splitwords(v) elif self._hasCondSection: # all conditional families: # todo: check it is needed at all # common (resp. ipv4) + ipv6 if allowed: - v = ['inet4', 'inet6'] if allowed_ipv6 else ['inet4'] + v = ['inet4', 'inet6'] if allowed_ipv6() else ['inet4'] else: # all action tags seems to be the same v = [''] self._properties['__families'] = v diff --git a/fail2ban/server/ipdns.py b/fail2ban/server/ipdns.py index aeb5102d..6648dac6 100644 --- a/fail2ban/server/ipdns.py +++ b/fail2ban/server/ipdns.py @@ -202,6 +202,11 @@ class DNSUtils: DNSUtils.CACHE_nameToIp.set(key, ips) return ips + @staticmethod + def IPv6IsAllowed(): + # return os.path.exists("/proc/net/if_inet6") || any((':' in ip) for ip in DNSUtils.getSelfIPs()) + return any((':' in ip.ntoa) for ip in DNSUtils.getSelfIPs()) + ## # Class for IP address handling. From f001f8de2a48e517c1711c8dce5e5478f251636e Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 7 Jan 2020 21:25:51 +0100 Subject: [PATCH 18/20] automatic reban (repeat banning action) after repair/restore sane environment, if already logged ticket causes new failures (part of #980, closes #1680); introduces banning epoch for actions and tickets (to distinguish or recognize removed set of tickets) --- fail2ban/server/action.py | 48 +++++++++- fail2ban/server/actions.py | 59 +++++++++++- fail2ban/server/ticket.py | 7 ++ fail2ban/tests/actionstestcase.py | 117 +++++++++++++++++++++-- fail2ban/tests/fail2banclienttestcase.py | 11 +-- fail2ban/tests/servertestcase.py | 2 +- fail2ban/tests/utils.py | 15 ++- 7 files changed, 229 insertions(+), 30 deletions(-) diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index f26b6bc8..e748fa77 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -217,6 +217,7 @@ class ActionBase(object): "start", "stop", "ban", + "reban", "unban", ) for method in required: @@ -250,6 +251,17 @@ class ActionBase(object): """ pass + def reban(self, aInfo): # pragma: no cover - abstract + """Executed when a ban occurs. + + Parameters + ---------- + aInfo : dict + Dictionary which includes information in relation to + the ban. + """ + return self.ban(aInfo) + def unban(self, aInfo): # pragma: no cover - abstract """Executed when a ban expires. @@ -281,6 +293,7 @@ class CommandAction(ActionBase): ---------- actionban actioncheck + actionreban actionreload actionrepair actionstart @@ -301,6 +314,7 @@ class CommandAction(ActionBase): self.actionstart = '' ## Command executed when ticket gets banned. self.actionban = '' + self.actionreban = '' ## Command executed when ticket gets removed. self.actionunban = '' ## Command executed in order to check requirements. @@ -504,8 +518,8 @@ class CommandAction(ActionBase): ret = self._executeOperation('', 'starting', family=family, afterExec=_started) return ret - def ban(self, aInfo): - """Executes the "actionban" command. + def ban(self, aInfo, cmd=''): + """Executes the given command ("actionban" or "actionreban"). Replaces the tags in the action command with actions properties and ban information, and executes the resulting command. @@ -522,7 +536,7 @@ class CommandAction(ActionBase): if not self.__started.get(family): self._start(family, forceStart=True) # ban: - if not self._processCmd('', aInfo): + if not self._processCmd(cmd, aInfo): raise RuntimeError("Error banning %(ip)s" % aInfo) self.__started[family] = self.__started.get(family, 0) | 3; # started and contains items @@ -543,6 +557,21 @@ class CommandAction(ActionBase): if not self._processCmd('', aInfo): raise RuntimeError("Error unbanning %(ip)s" % aInfo) + def reban(self, aInfo): + """Executes the "actionreban" command if available, otherwise simply repeat "actionban". + + Replaces the tags in the action command with actions properties + and ban information, and executes the resulting command. + + Parameters + ---------- + aInfo : dict + Dictionary which includes information in relation to + the ban. + """ + # re-ban: + return self.ban(aInfo, '' if self.actionreban else '') + def flush(self): """Executes the "actionflush" command. @@ -812,6 +841,17 @@ class CommandAction(ActionBase): realCmd = Utils.buildShellCmd(realCmd, varsDict) return realCmd + @property + def banEpoch(self): + return getattr(self, '_banEpoch', 0) + def invalidateBanEpoch(self): + """Increments ban epoch of jail and this action, so already banned tickets would cause + a re-ban for all tickets with previous epoch.""" + if self._jail is not None: + self._banEpoch = self._jail.actions.banEpoch = self._jail.actions.banEpoch + 1 + else: + self._banEpoch = self.banEpoch + 1 + def _invariantCheck(self, family=None, beforeRepair=None, forceStart=True): """Executes a substituted `actioncheck` command. """ @@ -826,6 +866,8 @@ class CommandAction(ActionBase): return -1 self._logSys.error( "Invariant check failed. Trying to restore a sane environment") + # increment ban epoch of jail and this action (allows re-ban on already banned): + self.invalidateBanEpoch() # try to find repair command, if exists - exec it: repairCmd = self._getOperation('', family) if repairCmd: diff --git a/fail2ban/server/actions.py b/fail2ban/server/actions.py index ed93c971..91912eb0 100644 --- a/fail2ban/server/actions.py +++ b/fail2ban/server/actions.py @@ -81,6 +81,8 @@ class Actions(JailThread, Mapping): self._actions = OrderedDict() ## The ban manager. self.__banManager = BanManager() + self.banEpoch = 0 + self.__lastConsistencyCheckTM = 0 ## Precedence of ban (over unban), so max number of tickets banned (to call an unban check): self.banPrecedence = 10 ## Max count of outdated tickets to unban per each __checkUnBan operation: @@ -439,6 +441,7 @@ class Actions(JailThread, Mapping): cnt = 0 if not tickets: tickets = self.__getFailTickets(self.banPrecedence) + rebanacts = None for ticket in tickets: bTicket = BanManager.createBanTicket(ticket) ip = bTicket.getIP() @@ -461,6 +464,8 @@ class Actions(JailThread, Mapping): exc_info=logSys.getEffectiveLevel()<=logging.DEBUG) # after all actions are processed set banned flag: bTicket.banned = True + if self.banEpoch: # be sure tickets always have the same ban epoch (default 0): + bTicket.banEpoch = self.banEpoch else: bTicket = reason['ticket'] # if already banned (otherwise still process some action) @@ -475,11 +480,61 @@ class Actions(JailThread, Mapping): else logging.NOTICE if diftm < 60 \ else logging.WARNING logSys.log(ll, "[%s] %s already banned", self._jail.name, ip) + # if long time after ban - do consistency check (something is wrong here): + if bTicket.banEpoch == self.banEpoch and diftm > 3: + # avoid too often checks: + if not rebanacts and MyTime.time() > self.__lastConsistencyCheckTM + 3: + for action in self._actions.itervalues(): + action.consistencyCheck() + self.__lastConsistencyCheckTM = MyTime.time() + # check epoch in order to reban it: + if bTicket.banEpoch < self.banEpoch: + if not rebanacts: rebanacts = dict( + (name, action) for name, action in self._actions.iteritems() + if action.banEpoch > bTicket.banEpoch) + cnt += self.__reBan(bTicket, actions=rebanacts) + else: + # pragma: no cover - unexpected: ticket is not banned for some reasons - reban using all actions: + cnt += self.__reBan(bTicket) if cnt: logSys.debug("Banned %s / %s, %s ticket(s) in %r", cnt, self.__banManager.getBanTotal(), self.__banManager.size(), self._jail.name) return cnt + def __reBan(self, ticket, actions=None, log=True): + """Repeat bans for the ticket. + + Executes the actions in order to reban the host given in the + ticket. + + Parameters + ---------- + ticket : Ticket + Ticket to reban + """ + actions = actions or self._actions + ip = ticket.getIP() + aInfo = self.__getActionInfo(ticket) + if log: + logSys.notice("[%s] Reban %s%s", self._jail.name, aInfo["ip"], (', action %r' % actions.keys()[0] if len(actions) == 1 else '')) + for name, action in actions.iteritems(): + try: + logSys.debug("[%s] action %r: reban %s", self._jail.name, name, ip) + if not aInfo.immutable: aInfo.reset() + action.reban(aInfo) + except Exception as e: + logSys.error( + "Failed to execute reban jail '%s' action '%s' " + "info '%r': %s", + self._jail.name, name, aInfo, e, + exc_info=logSys.getEffectiveLevel()<=logging.DEBUG) + return 0 + # after all actions are processed set banned flag: + ticket.banned = True + if self.banEpoch: # be sure tickets always have the same ban epoch (default 0): + ticket.banEpoch = self.banEpoch + return 1 + def __checkUnBan(self, maxCount=None): """Check for IP address to unban. @@ -522,7 +577,7 @@ class Actions(JailThread, Mapping): logSys.error("Failed to flush bans in jail '%s' action '%s': %s", self._jail.name, name, e, exc_info=logSys.getEffectiveLevel()<=logging.DEBUG) - logSys.info("No flush occured, do consistency check") + logSys.info("No flush occurred, do consistency check") if hasattr(action, 'consistencyCheck'): def _beforeRepair(): if stop and not getattr(action, 'actionrepair_on_unban', None): # don't need repair on stop @@ -569,8 +624,6 @@ class Actions(JailThread, Mapping): logSys.notice("[%s] Unban %s", self._jail.name, aInfo["ip"]) for name, action in unbactions.iteritems(): try: - if ticket.restored and getattr(action, 'norestored', False): - continue logSys.debug("[%s] action %r: unban %s", self._jail.name, name, ip) if not aInfo.immutable: aInfo.reset() action.unban(aInfo) diff --git a/fail2ban/server/ticket.py b/fail2ban/server/ticket.py index 5a750d02..4f509ea9 100644 --- a/fail2ban/server/ticket.py +++ b/fail2ban/server/ticket.py @@ -206,6 +206,13 @@ class Ticket(object): # return single value of data: return self._data.get(key, default) + @property + def banEpoch(self): + return getattr(self, '_banEpoch', 0) + @banEpoch.setter + def banEpoch(self, value): + self._banEpoch = value + class FailTicket(Ticket): diff --git a/fail2ban/tests/actionstestcase.py b/fail2ban/tests/actionstestcase.py index c4dac77f..e55ef0ce 100644 --- a/fail2ban/tests/actionstestcase.py +++ b/fail2ban/tests/actionstestcase.py @@ -28,11 +28,10 @@ import time import os import tempfile -from ..server.actions import Actions from ..server.ticket import FailTicket from ..server.utils import Utils from .dummyjail import DummyJail -from .utils import LogCaptureTestCase, with_alt_time, MyTime +from .utils import LogCaptureTestCase, with_alt_time, with_tmpdir, MyTime TEST_FILES_DIR = os.path.join(os.path.dirname(__file__), "files") @@ -43,7 +42,7 @@ class ExecuteActions(LogCaptureTestCase): """Call before every test case.""" super(ExecuteActions, self).setUp() self.__jail = DummyJail() - self.__actions = Actions(self.__jail) + self.__actions = self.__jail.actions def tearDown(self): super(ExecuteActions, self).tearDown() @@ -52,8 +51,8 @@ class ExecuteActions(LogCaptureTestCase): self.__actions.add('ip') act = self.__actions['ip'] act.actionstart = 'echo ip start'+o.get('start', '') - act.actionban = 'echo ip ban ' - act.actionunban = 'echo ip unban ' + act.actionban = 'echo ip ban '+o.get('ban', '') + act.actionunban = 'echo ip unban '+o.get('unban', '') act.actioncheck = 'echo ip check'+o.get('check', '') act.actionflush = 'echo ip flush'+o.get('flush', '') act.actionstop = 'echo ip stop'+o.get('stop', '') @@ -239,7 +238,7 @@ class ExecuteActions(LogCaptureTestCase): "stdout: %r" % 'ip flush inet4', "stdout: %r" % 'ip flush inet6', 'Failed to flush bans', - 'No flush occured, do consistency check', + 'No flush occurred, do consistency check', 'Invariant check failed. Trying to restore a sane environment', "stdout: %r" % 'ip stop', # same for both families 'Failed to flush bans', @@ -259,7 +258,7 @@ class ExecuteActions(LogCaptureTestCase): self.pruneLog('[test-phase 3] failed flush in consistent env') self.__actions._Actions__flushBan() self.assertLogged('Failed to flush bans', - 'No flush occured, do consistency check', + 'No flush occurred, do consistency check', "stdout: %r" % 'ip flush inet6', "stdout: %r" % 'ip check inet6', all=True, wait=True) @@ -341,7 +340,7 @@ class ExecuteActions(LogCaptureTestCase): "stdout: %r" % 'ip flush inet4', "stdout: %r" % 'ip flush inet6', 'Failed to flush bans', - 'No flush occured, do consistency check', + 'No flush occurred, do consistency check', 'Invariant check failed. Trying to restore a sane environment', "stdout: %r" % 'ip stop inet6', 'Failed to flush bans in jail', @@ -368,7 +367,7 @@ class ExecuteActions(LogCaptureTestCase): act['actioncheck?family=inet6'] = act.actioncheck self.__actions._Actions__flushBan() self.assertLogged('Failed to flush bans', - 'No flush occured, do consistency check', + 'No flush occurred, do consistency check', "stdout: %r" % 'ip flush inet6', "stdout: %r" % 'ip check inet6', all=True, wait=True) @@ -396,3 +395,103 @@ class ExecuteActions(LogCaptureTestCase): "stdout: %r" % 'ip flush inet4', 'Unban tickets each individualy', all=True) + + @with_alt_time + @with_tmpdir + def testActionsRebanBrokenAfterRepair(self, tmp): + act = self.defaultAction({ + 'start':' ; touch ""', + 'check':' ; test -f ""', + 'flush':' ; echo -n "" > ""', + 'stop': ' ; rm -f ""', + 'ban': ' ; echo " " >> ""', + }) + act['FN'] = tmp+'/' + act.actionstart_on_demand = True + act.actionrepair = 'echo ip repair ; touch ""' + act.actionreban = 'echo ip reban ; echo " -- rebanned" >> ""' + self.pruneLog('[test-phase 0] initial ban') + self.assertEqual(self.__actions.addBannedIP(['192.0.2.1', '2001:db8::1']), 2) + self.assertLogged('Ban 192.0.2.1', 'Ban 2001:db8::1', + "stdout: %r" % 'ip start inet4', + "stdout: %r" % 'ip ban 192.0.2.1 inet4', + "stdout: %r" % 'ip start inet6', + "stdout: %r" % 'ip ban 2001:db8::1 inet6', + all=True) + + self.pruneLog('[test-phase 1] check ban') + self.dumpFile(tmp+'/inet4') + self.assertLogged('192.0.2.1 inet4') + self.assertNotLogged('2001:db8::1 inet6') + self.pruneLog() + self.dumpFile(tmp+'/inet6') + self.assertLogged('2001:db8::1 inet6') + self.assertNotLogged('192.0.2.1 inet4') + + # simulate 3 seconds past: + MyTime.setTime(MyTime.time() + 4) + # already banned produces events: + self.pruneLog('[test-phase 2] check already banned') + self.assertEqual(self.__actions.addBannedIP(['192.0.2.1', '2001:db8::1', '2001:db8::2']), 1) + self.assertLogged( + '192.0.2.1 already banned', '2001:db8::1 already banned', 'Ban 2001:db8::2', + "stdout: %r" % 'ip check inet4', # both checks occurred + "stdout: %r" % 'ip check inet6', + all=True) + self.dumpFile(tmp+'/inet4') + self.dumpFile(tmp+'/inet6') + # no reban should occur: + self.assertNotLogged('Reban 192.0.2.1', 'Reban 2001:db8::1', + "stdout: %r" % 'ip ban 192.0.2.1 inet4', + "stdout: %r" % 'ip reban 192.0.2.1 inet4', + "stdout: %r" % 'ip ban 2001:db8::1 inet6', + "stdout: %r" % 'ip reban 2001:db8::1 inet6', + '192.0.2.1 inet4 -- repaired', + '2001:db8::1 inet6 -- repaired', + all=True) + + # simulate 3 seconds past: + MyTime.setTime(MyTime.time() + 4) + # break env (remove both files, so check would fail): + os.remove(tmp+'/inet4') + os.remove(tmp+'/inet6') + # test again already banned (it shall cause reban now): + self.pruneLog('[test-phase 3a] check reban after sane env repaired') + self.assertEqual(self.__actions.addBannedIP(['192.0.2.1', '2001:db8::1']), 2) + self.assertLogged( + "Invariant check failed. Trying to restore a sane environment", + "stdout: %r" % 'ip repair inet4', # both repairs occurred + "stdout: %r" % 'ip repair inet6', + "Reban 192.0.2.1, action 'ip'", "Reban 2001:db8::1, action 'ip'", # both rebans also + "stdout: %r" % 'ip reban 192.0.2.1 inet4', + "stdout: %r" % 'ip reban 2001:db8::1 inet6', + all=True) + + # now last IP (2001:db8::2) - no repair, but still old epoch of ticket, so it gets rebanned: + self.pruneLog('[test-phase 3a] check reban by epoch mismatch (without repair)') + self.assertEqual(self.__actions.addBannedIP('2001:db8::2'), 1) + self.assertLogged( + "Reban 2001:db8::2, action 'ip'", + "stdout: %r" % 'ip reban 2001:db8::2 inet6', + all=True) + self.assertNotLogged( + "Invariant check failed. Trying to restore a sane environment", + "stdout: %r" % 'ip repair inet4', # both repairs occurred + "stdout: %r" % 'ip repair inet6', + "Reban 192.0.2.1, action 'ip'", "Reban 2001:db8::1, action 'ip'", # both rebans also + "stdout: %r" % 'ip reban 192.0.2.1 inet4', + "stdout: %r" % 'ip reban 2001:db8::1 inet6', + all=True) + + # and bans present in files: + self.pruneLog('[test-phase 4] check reban') + self.dumpFile(tmp+'/inet4') + self.assertLogged('192.0.2.1 inet4 -- rebanned') + self.assertNotLogged('2001:db8::1 inet6 -- rebanned') + self.pruneLog() + self.dumpFile(tmp+'/inet6') + self.assertLogged( + '2001:db8::1 inet6 -- rebanned', + '2001:db8::2 inet6 -- rebanned', all=True) + self.assertNotLogged('192.0.2.1 inet4 -- rebanned') + diff --git a/fail2ban/tests/fail2banclienttestcase.py b/fail2ban/tests/fail2banclienttestcase.py index ad3c7355..770e3ffb 100644 --- a/fail2ban/tests/fail2banclienttestcase.py +++ b/fail2ban/tests/fail2banclienttestcase.py @@ -113,16 +113,7 @@ fail2banclient.input_command = _test_input_command fail2bancmdline.PRODUCTION = \ fail2banserver.PRODUCTION = False - -def _out_file(fn, handle=logSys.debug): - """Helper which outputs content of the file at HEAVYDEBUG loglevels""" - if (handle != logSys.debug or logSys.getEffectiveLevel() <= logging.DEBUG): - handle('---- ' + fn + ' ----') - for line in fileinput.input(fn): - line = line.rstrip('\n') - handle(line) - handle('-'*30) - +_out_file = LogCaptureTestCase.dumpFile def _write_file(fn, mode, *lines): f = open(fn, mode) diff --git a/fail2ban/tests/servertestcase.py b/fail2ban/tests/servertestcase.py index f1121a57..a51c4f85 100644 --- a/fail2ban/tests/servertestcase.py +++ b/fail2ban/tests/servertestcase.py @@ -752,7 +752,7 @@ class Transmitter(TransmitterBase): self.assertSortedEqual( self.transm.proceed(["get", self.jailName, "actionmethods", action])[1], - ['ban', 'start', 'stop', 'testmethod', 'unban']) + ['ban', 'reban', 'start', 'stop', 'testmethod', 'unban']) self.assertEqual( self.transm.proceed(["set", self.jailName, "action", action, "testmethod", '{"text": "world!"}']), diff --git a/fail2ban/tests/utils.py b/fail2ban/tests/utils.py index d4129b2a..f5ffb978 100644 --- a/fail2ban/tests/utils.py +++ b/fail2ban/tests/utils.py @@ -22,6 +22,7 @@ __author__ = "Yaroslav Halchenko" __copyright__ = "Copyright (c) 2013 Yaroslav Halchenko" __license__ = "GPL" +import fileinput import itertools import logging import optparse @@ -741,11 +742,8 @@ class LogCaptureTestCase(unittest.TestCase): self._dirty |= 2 # records changed def setUp(self): - # For extended testing of what gets output into logging # system, we will redirect it to a string - logSys = getLogger("fail2ban") - # Keep old settings self._old_level = logSys.level self._old_handlers = logSys.handlers @@ -762,7 +760,6 @@ class LogCaptureTestCase(unittest.TestCase): """Call after every test case.""" # print "O: >>%s<<" % self._log.getvalue() self.pruneLog() - logSys = getLogger("fail2ban") logSys.handlers = self._old_handlers logSys.level = self._old_level super(LogCaptureTestCase, self).tearDown() @@ -845,5 +842,15 @@ class LogCaptureTestCase(unittest.TestCase): def getLog(self): return self._log.getvalue() + @staticmethod + def dumpFile(fn, handle=logSys.debug): + """Helper which outputs content of the file at HEAVYDEBUG loglevels""" + if (handle != logSys.debug or logSys.getEffectiveLevel() <= logging.DEBUG): + handle('---- ' + fn + ' ----') + for line in fileinput.input(fn): + line = line.rstrip('\n') + handle(line) + handle('-'*30) + pid_exists = Utils.pid_exists From 17a34b152854bd4eba33a20094f30eaf65d72a5b Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 7 Jan 2020 21:31:21 +0100 Subject: [PATCH 19/20] amend with missing parameter of actionreban in actionreader and coverage --- fail2ban/client/actionreader.py | 1 + fail2ban/server/actions.py | 3 +-- fail2ban/tests/actionstestcase.py | 7 +++++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/fail2ban/client/actionreader.py b/fail2ban/client/actionreader.py index 0840fe68..e5bee154 100644 --- a/fail2ban/client/actionreader.py +++ b/fail2ban/client/actionreader.py @@ -46,6 +46,7 @@ class ActionReader(DefinitionInitConfigReader): "actionrepair": ["string", None], "actionrepair_on_unban": ["string", None], "actionban": ["string", None], + "actionreban": ["string", None], "actionunban": ["string", None], "norestored": ["string", None], } diff --git a/fail2ban/server/actions.py b/fail2ban/server/actions.py index 91912eb0..57be293a 100644 --- a/fail2ban/server/actions.py +++ b/fail2ban/server/actions.py @@ -493,8 +493,7 @@ class Actions(JailThread, Mapping): (name, action) for name, action in self._actions.iteritems() if action.banEpoch > bTicket.banEpoch) cnt += self.__reBan(bTicket, actions=rebanacts) - else: - # pragma: no cover - unexpected: ticket is not banned for some reasons - reban using all actions: + else: # pragma: no cover - unexpected: ticket is not banned for some reasons - reban using all actions: cnt += self.__reBan(bTicket) if cnt: logSys.debug("Banned %s / %s, %s ticket(s) in %r", cnt, diff --git a/fail2ban/tests/actionstestcase.py b/fail2ban/tests/actionstestcase.py index e55ef0ce..d97d9921 100644 --- a/fail2ban/tests/actionstestcase.py +++ b/fail2ban/tests/actionstestcase.py @@ -495,3 +495,10 @@ class ExecuteActions(LogCaptureTestCase): '2001:db8::2 inet6 -- rebanned', all=True) self.assertNotLogged('192.0.2.1 inet4 -- rebanned') + # coverage - intended error in reban (no unhandled exception, message logged): + act.actionreban = '' + act.actionban = 'exit 1' + self.assertEqual(self.__actions._Actions__reBan(FailTicket("192.0.2.1", 0)), 0) + self.assertLogged( + 'Failed to execute reban', + 'Error banning 192.0.2.1', all=True) From f30b7ae2446361512ee19a666b011c6586bec398 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 7 Jan 2020 21:31:21 +0100 Subject: [PATCH 20/20] update ChangeLog + spelling --- ChangeLog | 16 ++++++++++++++++ fail2ban/server/actions.py | 2 +- .../tests/files/action.d/action_modifyainfo.py | 2 +- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index a483e88a..93dd445a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -80,6 +80,10 @@ ver. 0.10.5-dev-1 (20??/??/??) - development edition * `filter.d/sendmail-auth.conf`, `filter.d/sendmail-reject.conf` : - ID in prefix can be longer as 14 characters (gh-2563); * all filters would accept square brackets around IPv4 addresses also (e. g. monit-filter, gh-2494) +* avoids unhandled exception during flush (gh-2588) +* fixes pass2allow-ftp jail - due to inverted handling, action should prohibit access per default for any IP, + therefore reset start on demand parameter for this action (it will be started immediately by repair); +* auto-detection of IPv6 subsystem availability (important for not on-demand actions or jails, like pass2allow); ### New Features * new replacement tags for failregex to match subnets in form of IP-addresses with CIDR mask (gh-2559): @@ -136,6 +140,18 @@ filter = flt[logtype=short] * samplestestcase.py (testSampleRegexsFactory) extended: - allow coverage of journal logtype; - new option `fileOptions` to set common filter/test options for whole test-file; +* large enhancement: auto-reban, improved invariant check and conditional operations (gh-2588): + - improves invariant check and repair (avoid unhandled exception, consider family on conditional operations, etc), + prepared for bulk re-ban in repair case (if bulk-ban becomes implemented); + - automatic reban (repeat banning action) after repair/restore sane environment, if already logged ticket causes + new failures (via new action operation `actionreban` or `actionban` if still not defined in action); + * introduces banning epoch for actions and tickets (to distinguish or recognize removed set of the tickets); + * invariant check avoids repair by unban/stop (unless parameter `actionrepair_on_unban` set to `true`); + * better handling for all conditional operations (distinguish families for certain operations like + repair/flush/stop, prepared for other families, e. g. if different handling for subnets expected, etc); + * partially implements gh-980 (more breakdown safe handling); + * closes gh-1680 (better as large-scale banning implementation with on-demand reban by failure, + at least unless a bulk-ban gets implemented); ver. 0.10.4 (2018/10/04) - ten-four-on-due-date-ten-four diff --git a/fail2ban/server/actions.py b/fail2ban/server/actions.py index 57be293a..5488465e 100644 --- a/fail2ban/server/actions.py +++ b/fail2ban/server/actions.py @@ -593,7 +593,7 @@ class Actions(JailThread, Mapping): if db and self._jail.database is not None: logSys.debug(" Flush jail in database") self._jail.database.delBan(self._jail) - # unban each ticket with non-flasheable actions: + # unban each ticket with non-flusheable actions: for ticket in lst: # unban ip: self.__unBan(ticket, actions=actions, log=log) diff --git a/fail2ban/tests/files/action.d/action_modifyainfo.py b/fail2ban/tests/files/action.d/action_modifyainfo.py index d585a7ee..58787dd7 100644 --- a/fail2ban/tests/files/action.d/action_modifyainfo.py +++ b/fail2ban/tests/files/action.d/action_modifyainfo.py @@ -13,7 +13,7 @@ class TestAction(ActionBase): self._logSys.info("%s unban deleted aInfo IP", self._name) def flush(self): - # intended error to cover no unhandled exception occurs in flash + # intended error to cover no unhandled exception occurs in flush # as well as unbans are done individually after errored flush. raise ValueError("intended error")