From de97dedba0202559193e7654e183974ab335bcf7 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 5 Dec 2017 17:49:22 +0100 Subject: [PATCH 1/4] move extractOptions from JailReader to helpers (common usage server- / client-side); --- fail2ban/client/fail2banregex.py | 8 +++---- fail2ban/client/jailreader.py | 33 +++----------------------- fail2ban/helpers.py | 28 ++++++++++++++++++++++ fail2ban/server/jail.py | 5 ++-- fail2ban/tests/clientreadertestcase.py | 30 +++++++++++------------ fail2ban/tests/servertestcase.py | 6 ++--- 6 files changed, 55 insertions(+), 55 deletions(-) diff --git a/fail2ban/client/fail2banregex.py b/fail2ban/client/fail2banregex.py index 89225ffb..84218149 100644 --- a/fail2ban/client/fail2banregex.py +++ b/fail2ban/client/fail2banregex.py @@ -46,12 +46,12 @@ except ImportError: FilterSystemd = None from ..version import version -from .jailreader import JailReader from .filterreader import FilterReader from ..server.filter import Filter, FileContainer from ..server.failregex import Regex, RegexException -from ..helpers import str2LogLevel, getVerbosityFormat, FormatterWithTraceBack, getLogger, PREFER_ENC +from ..helpers import str2LogLevel, getVerbosityFormat, FormatterWithTraceBack, getLogger, \ + extractOptions, PREFER_ENC # Gets the instance of the logger. logSys = getLogger("fail2ban") @@ -287,7 +287,7 @@ class Fail2banRegex(object): fltFile = None fltOpt = {} if regextype == 'fail': - fltName, fltOpt = JailReader.extractOptions(value) + fltName, fltOpt = extractOptions(value) if fltName is not None: if "." in fltName[~5:]: tryNames = (fltName,) @@ -606,7 +606,7 @@ class Fail2banRegex(object): return False output( "Use systemd journal" ) output( "Use encoding : %s" % self._encoding ) - backend, beArgs = JailReader.extractOptions(cmd_log) + backend, beArgs = extractOptions(cmd_log) flt = FilterSystemd(None, **beArgs) flt.setLogEncoding(self._encoding) myjournal = flt.getJournalReader() diff --git a/fail2ban/client/jailreader.py b/fail2ban/client/jailreader.py index ffc28752..382d3bc8 100644 --- a/fail2ban/client/jailreader.py +++ b/fail2ban/client/jailreader.py @@ -33,8 +33,7 @@ from .configreader import ConfigReaderUnshared, ConfigReader from .filterreader import FilterReader from .actionreader import ActionReader from ..version import version -from ..helpers import getLogger -from ..helpers import splitwords +from ..helpers import getLogger, extractOptions, splitwords # Gets the instance of the logger. logSys = getLogger(__name__) @@ -42,15 +41,6 @@ logSys = getLogger(__name__) class JailReader(ConfigReader): - # regex, to extract list of options: - optionCRE = re.compile(r"^([^\[]+)(?:\[(.*)\])?\s*$", re.DOTALL) - # regex, to iterate over single option in option list, syntax: - # `action = act[p1="...", p2='...', p3=...]`, where the p3=... not contains `,` or ']' - # since v0.10 separator extended with `]\s*[` for support of multiple option groups, syntax - # `action = act[p1=...][p2=...]` - optionExtractRE = re.compile( - r'([\w\-_\.]+)=(?:"([^"]*)"|\'([^\']*)\'|([^,\]]*))(?:,|\]\s*\[|$)', re.DOTALL) - def __init__(self, name, force_enable=False, **kwargs): ConfigReader.__init__(self, **kwargs) self.__name = name @@ -134,7 +124,7 @@ class JailReader(ConfigReader): # Read filter flt = self.__opts["filter"] if flt: - filterName, filterOpt = JailReader.extractOptions(flt) + filterName, filterOpt = extractOptions(flt) if not filterName: raise JailDefError("Invalid filter definition %r" % flt) self.__filter = FilterReader( @@ -164,7 +154,7 @@ class JailReader(ConfigReader): try: if not act: # skip empty actions continue - actName, actOpt = JailReader.extractOptions(act) + actName, actOpt = extractOptions(act) if not actName: raise JailDefError("Invalid action definition %r" % act) if actName.endswith(".py"): @@ -268,22 +258,5 @@ class JailReader(ConfigReader): stream.insert(0, ["add", self.__name, backend]) return stream - @staticmethod - def extractOptions(option): - match = JailReader.optionCRE.match(option) - if not match: - # TODO proper error handling - return None, None - option_name, optstr = match.groups() - option_opts = dict() - if optstr: - for optmatch in JailReader.optionExtractRE.finditer(optstr): - opt = optmatch.group(1) - value = [ - val for val in optmatch.group(2,3,4) if val is not None][0] - option_opts[opt.strip()] = value.strip() - return option_name, option_opts - - class JailDefError(Exception): pass diff --git a/fail2ban/helpers.py b/fail2ban/helpers.py index 5b027b32..378924df 100644 --- a/fail2ban/helpers.py +++ b/fail2ban/helpers.py @@ -237,6 +237,34 @@ else: return uni_decode(x, enc, 'replace') +# +# Following function used for parse options from parameter (e.g. `name[p1=0, p2="..."][p3='...']`). +# + +# regex, to extract list of options: +OPTION_CRE = re.compile(r"^([^\[]+)(?:\[(.*)\])?\s*$", re.DOTALL) +# regex, to iterate over single option in option list, syntax: +# `action = act[p1="...", p2='...', p3=...]`, where the p3=... not contains `,` or ']' +# 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) + +def extractOptions(option): + match = OPTION_CRE.match(option) + if not match: + # TODO proper error handling + return None, None + option_name, optstr = match.groups() + option_opts = dict() + if optstr: + for optmatch in OPTION_EXTRACT_CRE.finditer(optstr): + opt = optmatch.group(1) + value = [ + val for val in optmatch.group(2,3,4) if val is not None][0] + option_opts[opt.strip()] = value.strip() + return option_name, option_opts + # # Following facilities used for safe recursive interpolation of # tags () in tagged options. diff --git a/fail2ban/server/jail.py b/fail2ban/server/jail.py index 39fdd959..eb990c47 100644 --- a/fail2ban/server/jail.py +++ b/fail2ban/server/jail.py @@ -27,8 +27,7 @@ import logging import Queue from .actions import Actions -from ..client.jailreader import JailReader -from ..helpers import getLogger, MyTime +from ..helpers import getLogger, extractOptions, MyTime # Gets the instance of the logger. logSys = getLogger(__name__) @@ -85,7 +84,7 @@ class Jail(object): return "%s(%r)" % (self.__class__.__name__, self.name) def _setBackend(self, backend): - backend, beArgs = JailReader.extractOptions(backend) + backend, beArgs = extractOptions(backend) backend = backend.lower() # to assure consistent matching backends = self._BACKENDS diff --git a/fail2ban/tests/clientreadertestcase.py b/fail2ban/tests/clientreadertestcase.py index fdef3735..e8cd2912 100644 --- a/fail2ban/tests/clientreadertestcase.py +++ b/fail2ban/tests/clientreadertestcase.py @@ -30,7 +30,7 @@ import tempfile import unittest from ..client.configreader import ConfigReader, ConfigReaderUnshared, NoSectionError from ..client import configparserinc -from ..client.jailreader import JailReader +from ..client.jailreader import JailReader, extractOptions from ..client.filterreader import FilterReader from ..client.jailsreader import JailsReader from ..client.actionreader import ActionReader, CommandAction @@ -260,25 +260,25 @@ class JailReaderTest(LogCaptureTestCase): # Simple example option = "mail-whois[name=SSH]" expected = ('mail-whois', {'name': 'SSH'}) - result = JailReader.extractOptions(option) + result = extractOptions(option) self.assertEqual(expected, result) - self.assertEqual(('mail.who_is', {}), JailReader.extractOptions("mail.who_is")) - self.assertEqual(('mail.who_is', {'a':'cat', 'b':'dog'}), JailReader.extractOptions("mail.who_is[a=cat,b=dog]")) - self.assertEqual(('mail--ho_is', {}), JailReader.extractOptions("mail--ho_is")) + self.assertEqual(('mail.who_is', {}), extractOptions("mail.who_is")) + 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', {}), JailReader.extractOptions("mail--ho_is['s']")) + self.assertEqual(('mail--ho_is', {}), extractOptions("mail--ho_is['s']")) #self.printLog() #self.assertLogged("Invalid argument ['s'] in ''s''") - self.assertEqual(('mail', {'a': ','}), JailReader.extractOptions("mail[a=',']")) + self.assertEqual(('mail', {'a': ','}), extractOptions("mail[a=',']")) - #self.assertRaises(ValueError, JailReader.extractOptions ,'mail-how[') + #self.assertRaises(ValueError, extractOptions ,'mail-how[') # Empty option option = "abc[]" expected = ('abc', {}) - result = JailReader.extractOptions(option) + result = extractOptions(option) self.assertEqual(expected, result) # More complex examples @@ -296,11 +296,11 @@ class JailReaderTest(LogCaptureTestCase): 'opt10': "", 'opt11': "", }) - result = JailReader.extractOptions(option) + result = extractOptions(option) self.assertEqual(expected, result) # And multiple groups (`][` instead of `,`) - result = JailReader.extractOptions(option.replace(',', '][')) + result = extractOptions(option.replace(',', '][')) expected2 = (expected[0], dict((k, v.replace(',', '][')) for k, v in expected[1].iteritems()) ) @@ -439,7 +439,7 @@ class FilterReaderTest(unittest.TestCase): def testFilterReaderSubstitionKnown(self): output = [['set', 'jailname', 'addfailregex', 'to=test,sweet@example.com,test2,sweet@example.com fromip=']] - filterName, filterOpt = JailReader.extractOptions( + filterName, filterOpt = extractOptions( 'substition[honeypot=",", sweet="test,,test2"]') filterReader = FilterReader('substition', "jailname", filterOpt, share_config=TEST_FILES_DIR_SHARE_CFG, basedir=TEST_FILES_DIR) @@ -650,7 +650,7 @@ class JailsReaderTest(LogCaptureTestCase): if jail == 'INCLUDES': continue filterName = jails.get(jail, 'filter') - filterName, filterOpt = JailReader.extractOptions(filterName) + filterName, filterOpt = extractOptions(filterName) allFilters.add(filterName) self.assertTrue(len(filterName)) # moreover we must have a file for it @@ -669,7 +669,7 @@ class JailsReaderTest(LogCaptureTestCase): # somewhat duplicating here what is done in JailsReader if # the jail is enabled for act in actions.split('\n'): - actName, actOpt = JailReader.extractOptions(act) + actName, actOpt = extractOptions(act) self.assertTrue(len(actName)) self.assertTrue(isinstance(actOpt, dict)) if actName == 'iptables-multiport': @@ -696,7 +696,7 @@ class JailsReaderTest(LogCaptureTestCase): if not (a.endswith('common.conf') or a.endswith('-aggressive.conf'))) # get filters of all jails (filter names without options inside filter[...]) filters_jail = set( - JailReader.extractOptions(jail.options['filter'])[0] for jail in jails.jails + extractOptions(jail.options['filter'])[0] for jail in jails.jails ) self.maxDiff = None self.assertTrue(filters.issubset(filters_jail), diff --git a/fail2ban/tests/servertestcase.py b/fail2ban/tests/servertestcase.py index 2c293740..2d034806 100644 --- a/fail2ban/tests/servertestcase.py +++ b/fail2ban/tests/servertestcase.py @@ -42,7 +42,7 @@ from ..server.ticket import BanTicket from ..server.utils import Utils from .dummyjail import DummyJail from .utils import LogCaptureTestCase -from ..helpers import getLogger, PREFER_ENC +from ..helpers import getLogger, extractOptions, PREFER_ENC from .. import version try: @@ -1034,7 +1034,7 @@ class LoggingTests(LogCaptureTestCase): os.remove(f) -from clientreadertestcase import ActionReader, JailReader, JailsReader, CONFIG_DIR, STOCK +from clientreadertestcase import ActionReader, JailsReader, CONFIG_DIR, STOCK class ServerConfigReaderTests(LogCaptureTestCase): @@ -1145,7 +1145,7 @@ class ServerConfigReaderTests(LogCaptureTestCase): def getDefaultJailStream(self, jail, act): act = act.replace('%(__name__)s', jail) - actName, actOpt = JailReader.extractOptions(act) + actName, actOpt = extractOptions(act) stream = [ ['add', jail, 'polling'], # ['set', jail, 'addfailregex', 'DUMMY-REGEX '], From 1bf66364462ff0d4c224cce83315d1b29d41b06a Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 5 Dec 2017 18:54:21 +0100 Subject: [PATCH 2/4] Introduced new parameters for logging within fail2ban-server; Usage `logtarget = target[facility=..., datetime=on|off, format="..."]`: - `facility` - specify syslog facility (default `daemon`, see https://docs.python.org/2/library/logging.handlers.html#sysloghandler for the list of facilities); - `datetime` - add date-time to the message (default on, ignored if `format` specified); - `format` - specify own format how it will be logged, for example for short-log into STDOUT: `fail2ban-server -f --logtarget 'stdout[format="%(relativeCreated)5d | %(message)s"]' start`; Closes gh-1980 --- fail2ban/server/server.py | 21 +++++++++++++++++---- fail2ban/tests/fail2banclienttestcase.py | 5 +++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/fail2ban/server/server.py b/fail2ban/server/server.py index 9cb6f320..4a491903 100644 --- a/fail2ban/server/server.py +++ b/fail2ban/server/server.py @@ -37,7 +37,7 @@ from .filter import FileFilter, JournalFilter from .transmitter import Transmitter from .asyncserver import AsyncServer, AsyncServerException from .. import version -from ..helpers import getLogger, str2LogLevel, getVerbosityFormat, excepthook +from ..helpers import getLogger, extractOptions, str2LogLevel, getVerbosityFormat, excepthook # Gets the instance of the logger. logSys = getLogger(__name__) @@ -547,6 +547,7 @@ class Server: def setLogTarget(self, target): # check reserved targets in uppercase, don't change target, because it can be file: + target, logOptions = extractOptions(target) systarget = target.upper() with self.__loggingLock: # don't set new handlers if already the same @@ -559,7 +560,12 @@ class Server: # set a format which is simpler for console use fmt = "%(name)-24s[%(process)d]: %(levelname)-7s %(message)s" if systarget == "SYSLOG": - facility = logging.handlers.SysLogHandler.LOG_DAEMON + facility = logOptions.get('facility', 'DAEMON').upper() + try: + facility = getattr(logging.handlers.SysLogHandler, 'LOG_' + facility) + except AttributeError: # pragma: no cover + logSys.error("Unable to set facility %r, using 'DAEMON'", logOptions.get('facility')) + facility = logging.handlers.SysLogHandler.LOG_DAEMON if self.__syslogSocket == "auto": import platform self.__syslogSocket = self.__autoSyslogSocketPaths.get( @@ -610,9 +616,16 @@ class Server: if self.__verbose is None: self.__verbose = logging.DEBUG - logger.getEffectiveLevel() + 1 # If handler don't already add date to the message: - addtime = systarget not in ("SYSLOG", "SYSOUT") + addtime = logOptions.get('datetime') + if addtime is not None: + addtime = addtime in ('1', 'on', 'true', 'yes') + else: + addtime = systarget not in ("SYSLOG", "SYSOUT") + # If log-format is redefined in options: + if logOptions.get('format', '') != '': + fmt = logOptions.get('format') # verbose log-format: - if self.__verbose is not None and self.__verbose > 2: # pragma: no cover + elif self.__verbose is not None and self.__verbose > 2: # pragma: no cover fmt = getVerbosityFormat(self.__verbose-1, addtime=addtime) elif addtime: diff --git a/fail2ban/tests/fail2banclienttestcase.py b/fail2ban/tests/fail2banclienttestcase.py index 11b67fb2..76e35856 100644 --- a/fail2ban/tests/fail2banclienttestcase.py +++ b/fail2ban/tests/fail2banclienttestcase.py @@ -171,7 +171,7 @@ def _start_params(tmp, use_stock=False, use_stock_cfg=None, _write_file(pjoin(cfg, "fail2ban.conf"), "w", "[Definition]", "loglevel = INFO", - "logtarget = " + logtarget, + "logtarget = " + logtarget.replace('%', '%%'), "syslogsocket = auto", "socket = " + pjoin(tmp, "f2b.sock"), "pidfile = " + pjoin(tmp, "f2b.pid"), @@ -735,7 +735,8 @@ class Fail2banServerTest(Fail2banClientServerBase): def testKillAfterStart(self, tmp): try: # to prevent fork of test-cases process, start server in background via command: - startparams = _start_params(tmp, logtarget=pjoin(tmp, "f2b.log")) + startparams = _start_params(tmp, logtarget=pjoin(tmp, + 'f2b.log[format="SRV: %(relativeCreated)3d | %(message)s", datetime=off]')) # start (in new process, using the same python version): cmd = (sys.executable, pjoin(BIN, SERVER)) logSys.debug('Start %s ...', cmd) From f9833ddee4340b552026d648251aabea4a7d8b7c Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 5 Dec 2017 18:55:47 +0100 Subject: [PATCH 3/4] Update ChangeLog --- ChangeLog | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ChangeLog b/ChangeLog index 3c13cd0a..077c8ccd 100644 --- a/ChangeLog +++ b/ChangeLog @@ -68,6 +68,13 @@ ver. 0.10.2-dev-1 (2017/??/??) - development edition ### Enhancements * jail.conf: extended with new parameter `mode` for the filters supporting it (gh-1988); * action.d/pf.conf: extended with bulk-unban, command `actionflush` in order to flush all bans at once. +* Introduced new parameters for logging within fail2ban-server (gh-1980). + Usage `logtarget = target[facility=..., datetime=on|off, format="..."]`: + - `facility` - specify syslog facility (default `daemon`, see https://docs.python.org/2/library/logging.handlers.html#sysloghandler + for the list of facilities); + - `datetime` - add date-time to the message (default on, ignored if `format` specified); + - `format` - specify own format how it will be logged, for example for short-log into STDOUT: + `fail2ban-server -f --logtarget 'stdout[format="%(relativeCreated)5d | %(message)s"]' start`; ver. 0.10.1 (2017/10/12) - succeeded-before-friday-the-13th From 55143ce1d93a5fe43e8eeb2bba2281e4a76bf0ef Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 5 Dec 2017 19:32:13 +0100 Subject: [PATCH 4/4] coverage increase --- fail2ban/tests/servertestcase.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fail2ban/tests/servertestcase.py b/fail2ban/tests/servertestcase.py index 2d034806..9d7d9a76 100644 --- a/fail2ban/tests/servertestcase.py +++ b/fail2ban/tests/servertestcase.py @@ -831,8 +831,8 @@ class TransmitterLogging(TransmitterBase): for logTarget in logTargets: os.remove(logTarget) - self.setGetTest("logtarget", "STDOUT") - self.setGetTest("logtarget", "STDERR") + self.setGetTest("logtarget", 'STDOUT[format="%(message)s"]', 'STDOUT') + self.setGetTest("logtarget", 'STDERR[datetime=off]', 'STDERR') def testLogTargetSYSLOG(self): if not os.path.exists("/dev/log"):