From 1707560df8033341e4fb8a2e79db3ea684b42386 Mon Sep 17 00:00:00 2001 From: benrubson <6764151+benrubson@users.noreply.github.com> Date: Wed, 26 Feb 2020 10:41:55 +0100 Subject: [PATCH 1/4] Enhance Guacamole jail --- config/filter.d/guacamole.conf | 50 ++++++++++++++++++++++------- config/jail.conf | 1 + fail2ban/tests/files/logs/guacamole | 5 +++ 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/config/filter.d/guacamole.conf b/config/filter.d/guacamole.conf index 09b4e7b0..bc6dbea9 100644 --- a/config/filter.d/guacamole.conf +++ b/config/filter.d/guacamole.conf @@ -5,21 +5,47 @@ [Definition] -# Option: failregex -# Notes.: regex to match the password failures messages in the logfile. -# Values: TEXT -# +logging = catalina +failregex = /failregex> +maxlines = /maxlines> +datepattern = /datepattern> + +[L_catalina] + failregex = ^.*\nWARNING: Authentication attempt from for user "[^"]*" failed\.$ -# Option: ignoreregex -# Notes.: regex to ignore. If this regex matches, the line is ignored. -# Values: TEXT -# -ignoreregex = - -# "maxlines" is number of log lines to buffer for multi-line regex searches maxlines = 2 datepattern = ^%%b %%d, %%ExY %%I:%%M:%%S %%p ^WARNING:()** - {^LN-BEG} \ No newline at end of file + {^LN-BEG} + +[L_webapp] + +failregex = ^ \[\S+\] WARN \S+ - Authentication attempt from for user "[^"]+" failed. + +maxlines = 1 + +datepattern = ^%%H:%%M:%%S.%%f + +# DEV Notes: +# +# failregex is based on the default pattern given in Guacamole documentation : +# https://guacamole.apache.org/doc/gug/configuring-guacamole.html#webapp-logging +# +# The following logback.xml Guacamole configuration file can then be used accordingly : +# +# +# /var/log/guacamole.log +# +# /var/log/guacamole.%d.log.gz +# 32 +# +# +# %d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n +# +# +# +# +# +# diff --git a/config/jail.conf b/config/jail.conf index 39ebbe6d..40827707 100644 --- a/config/jail.conf +++ b/config/jail.conf @@ -440,6 +440,7 @@ backend = %(syslog_backend)s port = http,https logpath = /var/log/tomcat*/catalina.out +#logpath = /var/log/guacamole.log [monit] #Ban clients brute-forcing the monit gui login diff --git a/fail2ban/tests/files/logs/guacamole b/fail2ban/tests/files/logs/guacamole index 3de67454..ebb7afb0 100644 --- a/fail2ban/tests/files/logs/guacamole +++ b/fail2ban/tests/files/logs/guacamole @@ -10,3 +10,8 @@ WARNING: Authentication attempt from 192.0.2.0 for user "null" failed. apr 16, 2013 8:32:28 AM org.slf4j.impl.JCLLoggerAdapter warn # failJSON: { "time": "2013-04-16T08:32:28", "match": true , "host": "192.0.2.0" } WARNING: Authentication attempt from 192.0.2.0 for user "pippo" failed. + +# filterOptions: {"logging": "webapp"} + +# failJSON: { "time": "2005-08-13T12:57:32", "match": true , "host": "182.23.72.36" } +12:57:32.907 [http-nio-8080-exec-10] WARN o.a.g.r.auth.AuthenticationService - Authentication attempt from 182.23.72.36 for user "guacadmin" failed. From 7b05c1ce7a487f2c269815f8a17b7444213ae555 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 25 Aug 2020 14:52:22 +0200 Subject: [PATCH 2/4] do type-convert only in getCombined (otherwise int/bool conversion prevents substitution or section-related interpolation of tags) --- fail2ban/client/actionreader.py | 11 ++---- fail2ban/client/configreader.py | 54 ++++++++++++++++++++------ fail2ban/tests/clientreadertestcase.py | 7 ++-- 3 files changed, 49 insertions(+), 23 deletions(-) diff --git a/fail2ban/client/actionreader.py b/fail2ban/client/actionreader.py index 131e37cb..011a213d 100644 --- a/fail2ban/client/actionreader.py +++ b/fail2ban/client/actionreader.py @@ -38,17 +38,17 @@ class ActionReader(DefinitionInitConfigReader): _configOpts = { "actionstart": ["string", None], - "actionstart_on_demand": ["string", None], + "actionstart_on_demand": ["bool", None], "actionstop": ["string", None], "actionflush": ["string", None], "actionreload": ["string", None], "actioncheck": ["string", None], "actionrepair": ["string", None], - "actionrepair_on_unban": ["string", None], + "actionrepair_on_unban": ["bool", None], "actionban": ["string", None], "actionreban": ["string", None], "actionunban": ["string", None], - "norestored": ["string", None], + "norestored": ["bool", None], } def __init__(self, file_, jailName, initOpts, **kwargs): @@ -83,11 +83,6 @@ class ActionReader(DefinitionInitConfigReader): def convert(self): 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', 'actionrepair_on_unban'): - if opts.get(o): - opts[o] = self._convert_to_boolean(opts[o]) - # stream-convert: head = ["set", self._jailName] stream = list() diff --git a/fail2ban/client/configreader.py b/fail2ban/client/configreader.py index 20709b72..f9a6d288 100644 --- a/fail2ban/client/configreader.py +++ b/fail2ban/client/configreader.py @@ -228,7 +228,7 @@ class ConfigReaderUnshared(SafeConfigParserWithIncludes): # Or it is a dict: # {name: [type, default], ...} - def getOptions(self, sec, options, pOptions=None, shouldExist=False): + def getOptions(self, sec, options, pOptions=None, shouldExist=False, convert=True): values = dict() if pOptions is None: pOptions = {} @@ -244,12 +244,15 @@ class ConfigReaderUnshared(SafeConfigParserWithIncludes): if optname in pOptions: continue try: - if opttype == "bool": - v = self.getboolean(sec, optname) - if v is None: continue - elif opttype == "int": - v = self.getint(sec, optname) - if v is None: continue + if convert: + if opttype == "bool": + v = self.getboolean(sec, optname) + if v is None: continue + elif opttype == "int": + v = self.getint(sec, optname) + if v is None: continue + else: + v = self.get(sec, optname, vars=pOptions) else: v = self.get(sec, optname, vars=pOptions) values[optname] = v @@ -267,7 +270,7 @@ class ConfigReaderUnshared(SafeConfigParserWithIncludes): values[optname] = optvalue # elif logSys.getEffectiveLevel() <= logLevel: # logSys.log(logLevel, "Non essential option '%s' not defined in '%s'.", optname, sec) - except ValueError: + except ValueError: logSys.warning("Wrong value for '" + optname + "' in '" + sec + "'. Using default one: '" + repr(optvalue) + "'") values[optname] = optvalue @@ -324,8 +327,9 @@ class DefinitionInitConfigReader(ConfigReader): pOpts = dict() if self._initOpts: pOpts = _merge_dicts(pOpts, self._initOpts) + # type-convert only in combined (otherwise int/bool converting prevents substitution): self._opts = ConfigReader.getOptions( - self, "Definition", self._configOpts, pOpts) + self, "Definition", self._configOpts, pOpts, convert=False) self._pOpts = pOpts if self.has_section("Init"): # get only own options (without options from default): @@ -346,10 +350,34 @@ class DefinitionInitConfigReader(ConfigReader): if opt == '__name__' or opt in self._opts: continue self._opts[opt] = self.get("Definition", opt) + def convertOptions(self, opts, pOptions={}): + options = self._configOpts + for optname in options: + if isinstance(options, (list,tuple)): + if len(optname) > 2: + opttype, optname, optvalue = optname + else: + (opttype, optname), optvalue = optname, None + else: + opttype, optvalue = options[optname] + if optname in pOptions: + continue + try: + if opttype == "bool": + v = opts.get(optname) + if v is None or isinstance(v, bool): continue + v = _as_bool(v) + opts[optname] = v + elif opttype == "int": + v = opts.get(optname) + if v is None or isinstance(v, (int, long)): continue + v = int(v) + opts[optname] = v + except ValueError: + logSys.warning("Wrong %s value %r for %r. Using default one: %r", + opttype, v, optname, optvalue) + opts[optname] = optvalue - def _convert_to_boolean(self, value): - return _as_bool(value) - def getCombOption(self, optname): """Get combined definition option (as string) using pre-set and init options as preselection (values with higher precedence as specified in section). @@ -384,6 +412,8 @@ class DefinitionInitConfigReader(ConfigReader): ignore=ignore, addrepl=self.getCombOption) if not opts: raise ValueError('recursive tag definitions unable to be resolved') + # convert options after all interpolations: + self.convertOptions(opts) return opts def convert(self): diff --git a/fail2ban/tests/clientreadertestcase.py b/fail2ban/tests/clientreadertestcase.py index 2c1d0a0e..ae39c157 100644 --- a/fail2ban/tests/clientreadertestcase.py +++ b/fail2ban/tests/clientreadertestcase.py @@ -490,7 +490,9 @@ class FilterReaderTest(unittest.TestCase): self.__share_cfg = {} def testConvert(self): - output = [['multi-set', 'testcase01', 'addfailregex', [ + output = [ + ['set', 'testcase01', 'maxlines', 1], + ['multi-set', 'testcase01', 'addfailregex', [ "^\\s*(?:\\S+ )?(?:kernel: \\[\\d+\\.\\d+\\] )?(?:@vserver_\\S+ )" "?(?:(?:\\[\\d+\\])?:\\s+[\\[\\(]?sshd(?:\\(\\S+\\))?[\\]\\)]?:?|" "[\\[\\(]?sshd(?:\\(\\S+\\))?[\\]\\)]?:?(?:\\[\\d+\\])?:)?\\s*(?:" @@ -512,7 +514,6 @@ class FilterReaderTest(unittest.TestCase): ['set', 'testcase01', 'addjournalmatch', "FIELD= with spaces ", "+", "AFIELD= with + char and spaces"], ['set', 'testcase01', 'datepattern', "%Y %m %d %H:%M:%S"], - ['set', 'testcase01', 'maxlines', 1], # Last for overide test ] filterReader = FilterReader("testcase01", "testcase01", {}) filterReader.setBaseDir(TEST_FILES_DIR) @@ -529,7 +530,7 @@ class FilterReaderTest(unittest.TestCase): filterReader.read() #filterReader.getOptions(["failregex", "ignoreregex"]) filterReader.getOptions(None) - output[-1][-1] = "5" + output[0][-1] = 5; # maxlines = 5 self.assertSortedEqual(filterReader.convert(), output) def testFilterReaderSubstitionDefault(self): From d9b8796792ff2d876fe02e5a469767d33469c276 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 25 Aug 2020 18:01:34 +0200 Subject: [PATCH 3/4] amend with better (common) handling, documentation and tests --- fail2ban/client/configreader.py | 93 +++++++++++++------------- fail2ban/tests/clientreadertestcase.py | 30 +++++++-- 2 files changed, 71 insertions(+), 52 deletions(-) diff --git a/fail2ban/client/configreader.py b/fail2ban/client/configreader.py index f9a6d288..d8817bed 100644 --- a/fail2ban/client/configreader.py +++ b/fail2ban/client/configreader.py @@ -34,6 +34,30 @@ from ..helpers import getLogger, _as_bool, _merge_dicts, substituteRecursiveTags # Gets the instance of the logger. logSys = getLogger(__name__) +CONVERTER = { + "bool": _as_bool, + "int": int, +} +def _OptionsTemplateGen(options): + """Iterator over the options template with default options. + + Each options entry is composed of an array or tuple with: + [[type, name, ?default?], ...] + Or it is a dict: + {name: [type, default], ...} + """ + if isinstance(options, (list,tuple)): + for optname in options: + if len(optname) > 2: + opttype, optname, optvalue = optname + else: + (opttype, optname), optvalue = optname, None + yield opttype, optname, optvalue + else: + for optname in options: + opttype, optvalue = options[optname] + yield opttype, optname, optvalue + class ConfigReader(): """Generic config reader class. @@ -233,29 +257,17 @@ class ConfigReaderUnshared(SafeConfigParserWithIncludes): if pOptions is None: pOptions = {} # Get only specified options: - for optname in options: - if isinstance(options, (list,tuple)): - if len(optname) > 2: - opttype, optname, optvalue = optname - else: - (opttype, optname), optvalue = optname, None - else: - opttype, optvalue = options[optname] + for opttype, optname, optvalue in _OptionsTemplateGen(options): if optname in pOptions: continue try: - if convert: - if opttype == "bool": - v = self.getboolean(sec, optname) - if v is None: continue - elif opttype == "int": - v = self.getint(sec, optname) - if v is None: continue - else: - v = self.get(sec, optname, vars=pOptions) - else: - v = self.get(sec, optname, vars=pOptions) + v = self.get(sec, optname, vars=pOptions) values[optname] = v + if convert: + conv = CONVERTER.get(opttype) + if conv: + if v is None: continue + values[optname] = conv(v) except NoSectionError as e: if shouldExist: raise @@ -350,33 +362,20 @@ class DefinitionInitConfigReader(ConfigReader): if opt == '__name__' or opt in self._opts: continue self._opts[opt] = self.get("Definition", opt) - def convertOptions(self, opts, pOptions={}): - options = self._configOpts - for optname in options: - if isinstance(options, (list,tuple)): - if len(optname) > 2: - opttype, optname, optvalue = optname - else: - (opttype, optname), optvalue = optname, None - else: - opttype, optvalue = options[optname] - if optname in pOptions: - continue - try: - if opttype == "bool": - v = opts.get(optname) - if v is None or isinstance(v, bool): continue - v = _as_bool(v) - opts[optname] = v - elif opttype == "int": - v = opts.get(optname) - if v is None or isinstance(v, (int, long)): continue - v = int(v) - opts[optname] = v - except ValueError: - logSys.warning("Wrong %s value %r for %r. Using default one: %r", - opttype, v, optname, optvalue) - opts[optname] = optvalue + def convertOptions(self, opts, configOpts): + """Convert interpolated combined options to expected type. + """ + for opttype, optname, optvalue in _OptionsTemplateGen(configOpts): + conv = CONVERTER.get(opttype) + if conv: + v = opts.get(optname) + if v is None: continue + try: + opts[optname] = conv(v) + except ValueError: + logSys.warning("Wrong %s value %r for %r. Using default one: %r", + opttype, v, optname, optvalue) + opts[optname] = optvalue def getCombOption(self, optname): """Get combined definition option (as string) using pre-set and init @@ -413,7 +412,7 @@ class DefinitionInitConfigReader(ConfigReader): if not opts: raise ValueError('recursive tag definitions unable to be resolved') # convert options after all interpolations: - self.convertOptions(opts) + self.convertOptions(opts, self._configOpts) return opts def convert(self): diff --git a/fail2ban/tests/clientreadertestcase.py b/fail2ban/tests/clientreadertestcase.py index ae39c157..8abfd4a5 100644 --- a/fail2ban/tests/clientreadertestcase.py +++ b/fail2ban/tests/clientreadertestcase.py @@ -87,6 +87,21 @@ option = %s self.assertTrue(self.c.read(f)) # we got some now return self.c.getOptions('section', [("int", 'option')])['option'] + def testConvert(self): + self.c.add_section("Definition") + self.c.set("Definition", "a", "1") + self.c.set("Definition", "b", "1") + self.c.set("Definition", "c", "test") + opts = self.c.getOptions("Definition", + (('int', 'a', 0), ('bool', 'b', 0), ('int', 'c', 0))) + self.assertSortedEqual(opts, {'a': 1, 'b': True, 'c': 0}) + opts = self.c.getOptions("Definition", + (('int', 'a'), ('bool', 'b'), ('int', 'c'))) + self.assertSortedEqual(opts, {'a': 1, 'b': True, 'c': None}) + opts = self.c.getOptions("Definition", + {'a': ('int', 0), 'b': ('bool', 0), 'c': ('int', 0)}) + self.assertSortedEqual(opts, {'a': 1, 'b': True, 'c': 0}) + def testInaccessibleFile(self): f = os.path.join(self.d, "d.conf") # inaccessible file self._write('d.conf', 0) @@ -483,11 +498,7 @@ class JailReaderTest(LogCaptureTestCase): self.assertRaises(NoSectionError, c.getOptions, 'test', {}) -class FilterReaderTest(unittest.TestCase): - - def __init__(self, *args, **kwargs): - super(FilterReaderTest, self).__init__(*args, **kwargs) - self.__share_cfg = {} +class FilterReaderTest(LogCaptureTestCase): def testConvert(self): output = [ @@ -533,6 +544,15 @@ class FilterReaderTest(unittest.TestCase): output[0][-1] = 5; # maxlines = 5 self.assertSortedEqual(filterReader.convert(), output) + def testConvertOptions(self): + filterReader = FilterReader("testcase01", "testcase01", {'maxlines': '', 'test': 'X'}, + share_config=TEST_FILES_DIR_SHARE_CFG, basedir=TEST_FILES_DIR) + filterReader.read() + filterReader.getOptions(None) + opts = filterReader.getCombined(); + self.assertNotEqual(opts['maxlines'], 'X'); # wrong int value 'X' for 'maxlines' + self.assertLogged("Wrong int value 'X' for 'maxlines'. Using default one:") + def testFilterReaderSubstitionDefault(self): output = [['set', 'jailname', 'addfailregex', 'to=sweet@example.com fromip=']] filterReader = FilterReader('substition', "jailname", {}, From 2945fe8cbd39b6e02ae8065878ec2dffc68f0021 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 25 Aug 2020 18:25:32 +0200 Subject: [PATCH 4/4] changelog --- ChangeLog | 3 +++ fail2ban/client/configreader.py | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 59624d42..cacfdee7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -44,6 +44,8 @@ ver. 0.10.6-dev (20??/??/??) - development edition * ensure we've unique action name per jail (also if parameter `actname` is not set but name deviates from standard name, gh-2686) * don't use `%(banaction)s` interpolation because it can be complex value (containing `[...]` and/or quotes), so would bother the action interpolation +* fixed type conversion in config readers (take place after all interpolations get ready), that allows to + specify typed parameters variable (as substitutions) as well as to supply it in other sections or as init parameters. * `action.d/*-ipset*.conf`: several ipset actions fixed (no timeout per default anymore), so no discrepancy between ipset and fail2ban (removal from ipset will be managed by fail2ban only, gh-2703) * `action.d/cloudflare.conf`: fixed `actionunban` (considering new-line chars and optionally real json-parsing @@ -61,6 +63,7 @@ ver. 0.10.6-dev (20??/??/??) - development edition ### New Features and Enhancements * new filter and jail for GitLab recognizing failed application logins (gh-2689) +* `filter.d/guacamole.conf` extended with `logging` parameter to follow webapp-logging if it's configured (gh-2631) * introduced new prefix `{UNB}` for `datepattern` to disable word boundaries in regex; * datetemplate: improved anchor detection for capturing groups `(^...)`; * datepattern: improved handling with wrong recognized timestamps (timezones, no datepattern, etc) diff --git a/fail2ban/client/configreader.py b/fail2ban/client/configreader.py index d8817bed..1b5a56a2 100644 --- a/fail2ban/client/configreader.py +++ b/fail2ban/client/configreader.py @@ -282,7 +282,7 @@ class ConfigReaderUnshared(SafeConfigParserWithIncludes): values[optname] = optvalue # elif logSys.getEffectiveLevel() <= logLevel: # logSys.log(logLevel, "Non essential option '%s' not defined in '%s'.", optname, sec) - except ValueError: + except ValueError: logSys.warning("Wrong value for '" + optname + "' in '" + sec + "'. Using default one: '" + repr(optvalue) + "'") values[optname] = optvalue