From 3700a9e5230822d33b2377afed9b6de38676e564 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 26 Jan 2021 20:25:58 +0100 Subject: [PATCH 1/3] invalidate IP/DNS caches by reload, so inter alia would allow to recognize IPv6IsAllowed immediately, previously retarded up to cache max-time (5m); closes gh-2804 --- fail2ban/server/server.py | 9 ++++++++- fail2ban/tests/utils.py | 2 ++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/fail2ban/server/server.py b/fail2ban/server/server.py index b12c8f9f..475fd706 100644 --- a/fail2ban/server/server.py +++ b/fail2ban/server/server.py @@ -33,7 +33,7 @@ import stat import sys from .jails import Jails -from .filter import FileFilter, JournalFilter +from .filter import DNSUtils, FileFilter, JournalFilter from .transmitter import Transmitter from .asyncserver import AsyncServer, AsyncServerException from .. import version @@ -274,6 +274,11 @@ class Server: for name in self.__jails.keys(): self.delJail(name, stop=False, join=True) + def clearCaches(self): + # we need to clear caches, to be able to recognize new IPs/families etc: + DNSUtils.CACHE_nameToIp.clear() + DNSUtils.CACHE_ipToName.clear() + def reloadJails(self, name, opts, begin): if begin: # begin reload: @@ -295,6 +300,8 @@ class Server: if "--restart" in opts: self.stopJail(name) else: + # invalidate caches by reload + self.clearCaches() # first unban all ips (will be not restored after (re)start): if "--unban" in opts: self.setUnbanIP() diff --git a/fail2ban/tests/utils.py b/fail2ban/tests/utils.py index b54581f5..921427db 100644 --- a/fail2ban/tests/utils.py +++ b/fail2ban/tests/utils.py @@ -320,6 +320,7 @@ def initTests(opts): # precache all invalid ip's (TEST-NET-1, ..., TEST-NET-3 according to RFC 5737): c = DNSUtils.CACHE_ipToName + c.clear = lambda: logSys.warn('clear CACHE_ipToName is disabled in test suite') # increase max count and max time (too many entries, long time testing): c.setOptions(maxCount=10000, maxTime=5*60) for i in xrange(256): @@ -337,6 +338,7 @@ def initTests(opts): c.set('8.8.4.4', 'dns.google') # precache all dns to ip's used in test cases: c = DNSUtils.CACHE_nameToIp + c.clear = lambda: logSys.warn('clear CACHE_nameToIp is disabled in test suite') for i in ( ('999.999.999.999', set()), ('abcdef.abcdef', set()), From c75748c5d3b4de1e51318e1db4a5b72519c47c1f Mon Sep 17 00:00:00 2001 From: sebres Date: Wed, 27 Jan 2021 17:05:37 +0100 Subject: [PATCH 2/3] fail2ban.conf: added new fail2ban configuration option "allowipv6" (default auto), can be used to allow or disallow IPv6 interface in fail2ban immediately by start (e. g. if fail2ban starts before network interfaces). closes gh-2804 --- config/fail2ban.conf | 6 ++++ fail2ban/client/fail2banreader.py | 2 ++ fail2ban/server/ipdns.py | 41 +++++++++++++++++++------- fail2ban/server/server.py | 5 ++++ fail2ban/server/transmitter.py | 5 ++++ fail2ban/tests/clientreadertestcase.py | 1 + fail2ban/tests/servertestcase.py | 15 +++++++++- man/jail.conf.5 | 5 ++++ 8 files changed, 69 insertions(+), 11 deletions(-) diff --git a/config/fail2ban.conf b/config/fail2ban.conf index f3867839..601402d8 100644 --- a/config/fail2ban.conf +++ b/config/fail2ban.conf @@ -55,6 +55,12 @@ socket = /var/run/fail2ban/fail2ban.sock # pidfile = /var/run/fail2ban/fail2ban.pid +# Option: allowipv6 +# Notes.: Allows IPv6 interface: +# Default: auto +# Values: [ auto yes (on, true, 1) no (off, false, 0) ] Default: auto +#allowipv6 = auto + # Options: dbfile # Notes.: Set the file for the fail2ban persistent data to be stored. # A value of ":memory:" means database is only stored in memory diff --git a/fail2ban/client/fail2banreader.py b/fail2ban/client/fail2banreader.py index 3270b767..1f135cf8 100644 --- a/fail2ban/client/fail2banreader.py +++ b/fail2ban/client/fail2banreader.py @@ -53,6 +53,7 @@ class Fail2banReader(ConfigReader): opts = [["string", "loglevel", "INFO" ], ["string", "logtarget", "STDERR"], ["string", "syslogsocket", "auto"], + ["string", "allowipv6", "auto"], ["string", "dbfile", "/var/lib/fail2ban/fail2ban.sqlite3"], ["int", "dbmaxmatches", None], ["string", "dbpurgeage", "1d"]] @@ -74,6 +75,7 @@ class Fail2banReader(ConfigReader): # Also dbfile should be set before all other database options. # So adding order indices into items, to be stripped after sorting, upon return order = {"thread":0, "syslogsocket":11, "loglevel":12, "logtarget":13, + "allowipv6": 14, "dbfile":50, "dbmaxmatches":51, "dbpurgeage":51} stream = list() for opt in self.__opts: diff --git a/fail2ban/server/ipdns.py b/fail2ban/server/ipdns.py index 571ccc4f..5f3e4571 100644 --- a/fail2ban/server/ipdns.py +++ b/fail2ban/server/ipdns.py @@ -169,27 +169,31 @@ class DNSUtils: DNSUtils.CACHE_ipToName.set(key, name) return name + # key find cached own hostnames (this tuple-key cannot be used elsewhere): + _getSelfNames_key = ('self','dns') + @staticmethod def getSelfNames(): """Get own host names of self""" - # try find cached own hostnames (this tuple-key cannot be used elsewhere): - key = ('self','dns') - names = DNSUtils.CACHE_ipToName.get(key) + # try find cached own hostnames: + names = DNSUtils.CACHE_ipToName.get(DNSUtils._getSelfNames_key) # get it using different ways (a set with names of localhost, hostname, fully qualified): if names is None: names = set([ 'localhost', DNSUtils.getHostname(False), DNSUtils.getHostname(True) ]) - set(['']) # getHostname can return '' # cache and return : - DNSUtils.CACHE_ipToName.set(key, names) + DNSUtils.CACHE_ipToName.set(DNSUtils._getSelfNames_key, names) return names + # key to find cached own IPs (this tuple-key cannot be used elsewhere): + _getSelfIPs_key = ('self','ips') + @staticmethod def getSelfIPs(): """Get own IP addresses of self""" - # try find cached own IPs (this tuple-key cannot be used elsewhere): - key = ('self','ips') - ips = DNSUtils.CACHE_nameToIp.get(key) + # to find cached own IPs: + ips = DNSUtils.CACHE_nameToIp.get(DNSUtils._getSelfIPs_key) # get it using different ways (a set with IPs of localhost, hostname, fully qualified): if ips is None: ips = set() @@ -199,13 +203,30 @@ class DNSUtils: except Exception as e: # pragma: no cover logSys.warning("Retrieving own IPs of %s failed: %s", hostname, e) # cache and return : - DNSUtils.CACHE_nameToIp.set(key, ips) + DNSUtils.CACHE_nameToIp.set(DNSUtils._getSelfIPs_key, ips) return ips + _IPv6IsAllowed = None + + @staticmethod + def setIPv6IsAllowed(value): + DNSUtils._IPv6IsAllowed = value + logSys.debug("IPv6 is %s", ('on' if value else 'off') if value is not None else 'auto') + return value + + # key to find cached value of IPv6 allowance (this tuple-key cannot be used elsewhere): + _IPv6IsAllowed_key = ('self','ipv6-allowed') + @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()) + if DNSUtils._IPv6IsAllowed is not None: + return DNSUtils._IPv6IsAllowed + v = DNSUtils.CACHE_nameToIp.get(DNSUtils._IPv6IsAllowed_key) + if v is not None: + return v + v = any((':' in ip.ntoa) for ip in DNSUtils.getSelfIPs()) + DNSUtils.CACHE_nameToIp.set(DNSUtils._IPv6IsAllowed_key, v) + return v ## diff --git a/fail2ban/server/server.py b/fail2ban/server/server.py index 475fd706..bd2c7ad3 100644 --- a/fail2ban/server/server.py +++ b/fail2ban/server/server.py @@ -770,6 +770,11 @@ class Server: logSys.info("flush performed on %s" % self.__logTarget) return "flushed" + @staticmethod + def setIPv6IsAllowed(value): + value = _as_bool(value) if value != 'auto' else None + return DNSUtils.setIPv6IsAllowed(value) + def setThreadOptions(self, value): for o, v in value.iteritems(): if o == 'stacksize': diff --git a/fail2ban/server/transmitter.py b/fail2ban/server/transmitter.py index 10cfd163..e40f2f51 100644 --- a/fail2ban/server/transmitter.py +++ b/fail2ban/server/transmitter.py @@ -173,6 +173,11 @@ class Transmitter: return self.__server.getSyslogSocket() else: raise Exception("Failed to change syslog socket") + elif name == "allowipv6": + value = command[1] + self.__server.setIPv6IsAllowed(value) + if self.__quiet: return + return value #Thread elif name == "thread": value = command[1] diff --git a/fail2ban/tests/clientreadertestcase.py b/fail2ban/tests/clientreadertestcase.py index 2cfaff77..54850bca 100644 --- a/fail2ban/tests/clientreadertestcase.py +++ b/fail2ban/tests/clientreadertestcase.py @@ -975,6 +975,7 @@ class JailsReaderTest(LogCaptureTestCase): ['set', 'syslogsocket', 'auto'], ['set', 'loglevel', "INFO"], ['set', 'logtarget', '/var/log/fail2ban.log'], + ['set', 'allowipv6', 'auto'], ['set', 'dbfile', '/var/lib/fail2ban/fail2ban.sqlite3'], ['set', 'dbmaxmatches', 10], ['set', 'dbpurgeage', '1d'], diff --git a/fail2ban/tests/servertestcase.py b/fail2ban/tests/servertestcase.py index eaf1b346..b7b9d802 100644 --- a/fail2ban/tests/servertestcase.py +++ b/fail2ban/tests/servertestcase.py @@ -35,7 +35,7 @@ import platform from ..server.failregex import Regex, FailRegex, RegexException from ..server import actions as _actions from ..server.server import Server -from ..server.ipdns import IPAddr +from ..server.ipdns import DNSUtils, IPAddr from ..server.jail import Jail from ..server.jailthread import JailThread from ..server.ticket import BanTicket @@ -175,6 +175,19 @@ class Transmitter(TransmitterBase): def testVersion(self): self.assertEqual(self.transm.proceed(["version"]), (0, version.version)) + def testSetIPv6(self): + try: + self.assertEqual(self.transm.proceed(["set", "allowipv6", 'yes']), (0, 'yes')) + self.assertTrue(DNSUtils.IPv6IsAllowed()) + self.assertLogged("IPv6 is on"); self.pruneLog() + self.assertEqual(self.transm.proceed(["set", "allowipv6", 'no']), (0, 'no')) + self.assertFalse(DNSUtils.IPv6IsAllowed()) + self.assertLogged("IPv6 is off"); self.pruneLog() + finally: + # restore back to auto: + self.assertEqual(self.transm.proceed(["set", "allowipv6", "auto"]), (0, "auto")) + self.assertLogged("IPv6 is auto"); self.pruneLog() + def testSleep(self): if not unittest.F2B.fast: t0 = time.time() diff --git a/man/jail.conf.5 b/man/jail.conf.5 index dc226ac2..788fad2b 100644 --- a/man/jail.conf.5 +++ b/man/jail.conf.5 @@ -151,6 +151,11 @@ PID filename. Default: /var/run/fail2ban/fail2ban.pid .br This is used to store the process ID of the fail2ban server. .TP +.B allowipv6 +option to allow IPv6 interface - auto, yes (on, true, 1) or no (off, false, 0). Default: auto +.br +This value can be used to declare fail2ban whether IPv6 is allowed or not. +.TP .B dbfile Database filename. Default: /var/lib/fail2ban/fail2ban.sqlite3 .br From 366c64cb9da3c5d833eefc83f5bb200713db29b5 Mon Sep 17 00:00:00 2001 From: sebres Date: Wed, 3 Feb 2021 14:45:30 +0100 Subject: [PATCH 3/3] extractOptions: ensure options are parsed completely - avoids unexpected skip or truncate of parameters, produces more verbose error message in case of incorrect syntax; added more tests covering several cases WARN: potential incompatibility (since it doesn't silently ignore wrong syntax anymore) --- fail2ban/client/fail2banregex.py | 40 +++++++++++++++---------- fail2ban/client/jailreader.py | 15 +++++----- fail2ban/helpers.py | 12 ++++++-- fail2ban/tests/clientreadertestcase.py | 17 ++++++----- fail2ban/tests/fail2banregextestcase.py | 6 ++++ 5 files changed, 57 insertions(+), 33 deletions(-) diff --git a/fail2ban/client/fail2banregex.py b/fail2ban/client/fail2banregex.py index 5d5f4a1c..90e178f9 100644 --- a/fail2ban/client/fail2banregex.py +++ b/fail2ban/client/fail2banregex.py @@ -35,6 +35,7 @@ __license__ = "GPL" import getopt import logging +import re import os import shlex import sys @@ -329,26 +330,33 @@ class Fail2banRegex(object): regex = regextype + 'regex' # try to check - we've case filter?[options...]?: basedir = self._opts.config + fltName = value fltFile = None fltOpt = {} if regextype == 'fail': - fltName, fltOpt = extractOptions(value) - if fltName is not None: - if "." in fltName[~5:]: - tryNames = (fltName,) - else: - tryNames = (fltName, fltName + '.conf', fltName + '.local') - for fltFile in tryNames: - if not "/" in fltFile: - if os.path.basename(basedir) == 'filter.d': - fltFile = os.path.join(basedir, fltFile) - else: - fltFile = os.path.join(basedir, 'filter.d', fltFile) + if re.search(r'^/{0,3}[\w/_\-.]+(?:\[.*\])?$', value): + try: + fltName, fltOpt = extractOptions(value) + if "." in fltName[~5:]: + tryNames = (fltName,) else: - basedir = os.path.dirname(fltFile) - if os.path.isfile(fltFile): - break - fltFile = None + tryNames = (fltName, fltName + '.conf', fltName + '.local') + for fltFile in tryNames: + if not "/" in fltFile: + if os.path.basename(basedir) == 'filter.d': + fltFile = os.path.join(basedir, fltFile) + else: + fltFile = os.path.join(basedir, 'filter.d', fltFile) + else: + basedir = os.path.dirname(fltFile) + if os.path.isfile(fltFile): + break + fltFile = None + except Exception as e: + output("ERROR: Wrong filter name or options: %s" % (str(e),)) + output(" while parsing: %s" % (value,)) + if self._verbose: raise(e) + return False # if it is filter file: if fltFile is not None: if (basedir == self._opts.config diff --git a/fail2ban/client/jailreader.py b/fail2ban/client/jailreader.py index 1d7db0dc..0a27c644 100644 --- a/fail2ban/client/jailreader.py +++ b/fail2ban/client/jailreader.py @@ -133,9 +133,10 @@ class JailReader(ConfigReader): # Read filter flt = self.__opts["filter"] if flt: - filterName, filterOpt = extractOptions(flt) - if not filterName: - raise JailDefError("Invalid filter definition %r" % flt) + try: + filterName, filterOpt = extractOptions(flt) + except ValueError as e: + raise JailDefError("Invalid filter definition %r: %s" % (flt, e)) self.__filter = FilterReader( filterName, self.__name, filterOpt, share_config=self.share_config, basedir=self.getBaseDir()) @@ -167,10 +168,10 @@ class JailReader(ConfigReader): if not act: # skip empty actions continue # join with previous line if needed (consider possible new-line): - actName, actOpt = extractOptions(act) - prevln = '' - if not actName: - raise JailDefError("Invalid action definition %r" % act) + try: + actName, actOpt = extractOptions(act) + except ValueError as e: + raise JailDefError("Invalid action definition %r: %s" % (act, e)) if actName.endswith(".py"): self.__actions.append([ "set", diff --git a/fail2ban/helpers.py b/fail2ban/helpers.py index c45be849..5c1750a6 100644 --- a/fail2ban/helpers.py +++ b/fail2ban/helpers.py @@ -371,7 +371,7 @@ OPTION_CRE = re.compile(r"^([^\[]+)(?:\[(.*)\])?\s*$", re.DOTALL) # since v0.10 separator extended with `]\s*[` for support of multiple option groups, syntax # `action = act[p1=...][p2=...]` OPTION_EXTRACT_CRE = re.compile( - r'([\w\-_\.]+)=(?:"([^"]*)"|\'([^\']*)\'|([^,\]]*))(?:,|\]\s*\[|$)', re.DOTALL) + r'\s*([\w\-_\.]+)=(?:"([^"]*)"|\'([^\']*)\'|([^,\]]*))(?:,|\]\s*\[|$|(?P.+))|,?\s*$|(?P.+)', re.DOTALL) # split by new-line considering possible new-lines within options [...]: OPTION_SPLIT_CRE = re.compile( r'(?:[^\[\s]+(?:\s*\[\s*(?:[\w\-_\.]+=(?:"[^"]*"|\'[^\']*\'|[^,\]]*)\s*(?:,|\]\s*\[)?\s*)*\])?\s*|\S+)(?=\n\s*|\s+|$)', re.DOTALL) @@ -379,13 +379,19 @@ OPTION_SPLIT_CRE = re.compile( def extractOptions(option): match = OPTION_CRE.match(option) if not match: - # TODO proper error handling - return None, None + raise ValueError("unexpected option syntax") option_name, optstr = match.groups() option_opts = dict() if optstr: for optmatch in OPTION_EXTRACT_CRE.finditer(optstr): + if optmatch.group("wrngA"): + raise ValueError("unexpected syntax at %d after option %r: %s" % ( + optmatch.start("wrngA"), optmatch.group(1), optmatch.group("wrngA")[0:25])) + if optmatch.group("wrngB"): + raise ValueError("expected option, wrong syntax at %d: %s" % ( + optmatch.start("wrngB"), optmatch.group("wrngB")[0:25])) opt = optmatch.group(1) + if not opt: continue value = [ val for val in optmatch.group(2,3,4) if val is not None][0] option_opts[opt.strip()] = value.strip() diff --git a/fail2ban/tests/clientreadertestcase.py b/fail2ban/tests/clientreadertestcase.py index 54850bca..e92edd48 100644 --- a/fail2ban/tests/clientreadertestcase.py +++ b/fail2ban/tests/clientreadertestcase.py @@ -381,13 +381,16 @@ class JailReaderTest(LogCaptureTestCase): self.assertEqual(('mail.who_is', {'a':'cat', 'b':'dog'}), extractOptions("mail.who_is[a=cat,b=dog]")) self.assertEqual(('mail--ho_is', {}), extractOptions("mail--ho_is")) - self.assertEqual(('mail--ho_is', {}), extractOptions("mail--ho_is['s']")) - #print(self.getLog()) - #self.assertLogged("Invalid argument ['s'] in ''s''") - self.assertEqual(('mail', {'a': ','}), extractOptions("mail[a=',']")) + self.assertEqual(('mail', {'a': 'b'}), extractOptions("mail[a=b, ]")) - #self.assertRaises(ValueError, extractOptions ,'mail-how[') + self.assertRaises(ValueError, extractOptions ,'mail-how[') + + self.assertRaises(ValueError, extractOptions, """mail[a="test with interim (wrong) "" quotes"]""") + self.assertRaises(ValueError, extractOptions, """mail[a='test with interim (wrong) '' quotes']""") + self.assertRaises(ValueError, extractOptions, """mail[a='x, y, z', b=x, y, z]""") + + self.assertRaises(ValueError, extractOptions, """mail['s']""") # Empty option option = "abc[]" @@ -752,9 +755,9 @@ class JailsReaderTest(LogCaptureTestCase): ['add', 'tz_correct', 'auto'], ['start', 'tz_correct'], ['config-error', - "Jail 'brokenactiondef' skipped, because of wrong configuration: Invalid action definition 'joho[foo'"], + "Jail 'brokenactiondef' skipped, because of wrong configuration: Invalid action definition 'joho[foo': unexpected option syntax"], ['config-error', - "Jail 'brokenfilterdef' skipped, because of wrong configuration: Invalid filter definition 'flt[test'"], + "Jail 'brokenfilterdef' skipped, because of wrong configuration: Invalid filter definition 'flt[test': unexpected option syntax"], ['config-error', "Jail 'missingaction' skipped, because of wrong configuration: Unable to read action 'noactionfileforthisaction'"], ['config-error', diff --git a/fail2ban/tests/fail2banregextestcase.py b/fail2ban/tests/fail2banregextestcase.py index 4d878a24..85fe4f15 100644 --- a/fail2ban/tests/fail2banregextestcase.py +++ b/fail2ban/tests/fail2banregextestcase.py @@ -141,6 +141,12 @@ class Fail2banRegexTest(LogCaptureTestCase): )) self.assertLogged("Unable to compile regular expression") + def testWrongFilterOptions(self): + self.assertFalse(_test_exec( + "test", "flt[a='x,y,z',b=z,y,x]" + )) + self.assertLogged("Wrong filter name or options", "wrong syntax at 14: y,x", all=True) + def testDirectFound(self): self.assertTrue(_test_exec( "--datepattern", r"^(?:%a )?%b %d %H:%M:%S(?:\.%f)?(?: %ExY)?",