From 857767f04b66f0b61f1b23c4ed7816439ae6920a Mon Sep 17 00:00:00 2001 From: Ben RUBSON Date: Tue, 27 Feb 2018 10:12:22 +0100 Subject: [PATCH 1/5] Add 'any' badips.py bancategory (#2056) action.d/badips.py: allow `any` as bancategory to retrieve IPs from all categories --- ChangeLog | 1 + config/action.d/badips.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index f32f806e..f37ac992 100644 --- a/ChangeLog +++ b/ChangeLog @@ -46,6 +46,7 @@ ver. 0.10.3-dev-1 (20??/??/??) - development edition the epoch-pattern similar to `{DATE}` patterns does the capture and cuts out the match of whole pattern from the log-line, e. g. date-pattern `^\[{LEPOCH}\]\s+:` will match and cut out `[1516469849551000] :` from begin of the log-line. * badips.py now uses https instead of plain http when requesting badips.com (gh-2057); +* add support for "any" badips.py bancategory, to be able to retrieve IPs from all categories with a desired score (gh-2056); ver. 0.10.2 (2018/01/18) - nothing-burns-like-the-cold diff --git a/config/action.d/badips.py b/config/action.d/badips.py index afb58950..0df34c12 100644 --- a/config/action.d/badips.py +++ b/config/action.d/badips.py @@ -220,7 +220,7 @@ class BadIPsAction(ActionBase): # pragma: no cover - may be unavailable @bancategory.setter def bancategory(self, bancategory): - if bancategory not in self.getCategories(incParents=True): + if bancategory != "any" and bancategory not in self.getCategories(incParents=True): self._logSys.error("Category name '%s' not valid. " "see badips.com for list of valid categories", bancategory) From b112250ef0840ddeadec3e59e4cb54a8543d5b7b Mon Sep 17 00:00:00 2001 From: Ben RUBSON Date: Tue, 27 Feb 2018 10:18:59 +0100 Subject: [PATCH 2/5] (Free)BSD IPFW does not allow 2 identical rules (#2054) ipfw actionban fixed to allow same rule added several times (and actionunban to ignore error by deletion of missing rule) --- ChangeLog | 1 + config/action.d/bsd-ipfw.conf | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index f37ac992..e410966a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -37,6 +37,7 @@ ver. 0.10.3-dev-1 (20??/??/??) - development edition ### Fixes * `filter.d/exim.conf`: failregex extended - SMTP call dropped: too many syntax or protocol errors (gh-2048); * `action.d/badips.py`: implicit convert IPAddr to str, solves an issue "expected string, IPAddr found" (gh-2059); +* (Free)BSD ipfw actionban fixed to allow same rule added several times (gh-2054); ### New Features diff --git a/config/action.d/bsd-ipfw.conf b/config/action.d/bsd-ipfw.conf index cbd6a15d..4fbe9195 100644 --- a/config/action.d/bsd-ipfw.conf +++ b/config/action.d/bsd-ipfw.conf @@ -38,7 +38,7 @@ actioncheck = # Values: CMD # # requires an ipfw rule like "deny ip from table(1) to me" -actionban = e=`ipfw table add 2>&1`; x=$?; [ $x -eq 0 -o "$e" = 'ipfw: setsockopt(IP_FW_TABLE_XADD): File exists' ] || { echo "$e" 1>&2; exit $x; } +actionban = e=`ipfw table
add 2>&1`; x=$?; [ $x -eq 0 -o "$e" = 'ipfw: setsockopt(IP_FW_TABLE_XADD): File exists' ] || echo "$e" | grep -q "record already exists" || { echo "$e" 1>&2; exit $x; } # Option: actionunban @@ -47,7 +47,7 @@ actionban = e=`ipfw table
add 2>&1`; x=$?; [ $x -eq 0 -o "$e" = 'ip # Tags: See jail.conf(5) man page # Values: CMD # -actionunban = e=`ipfw table
delete 2>&1`; x=$?; [ $x -eq 0 -o "$e" = 'ipfw: setsockopt(IP_FW_TABLE_XDEL): No such process' ] || { echo "$e" 1>&2; exit $x; } +actionunban = e=`ipfw table
delete 2>&1`; x=$?; [ $x -eq 0 -o "$e" = 'ipfw: setsockopt(IP_FW_TABLE_XDEL): No such process' ] || echo "$e" | grep -q "record not found" || { echo "$e" 1>&2; exit $x; } [Init] # Option: table From 8c291cad384cfe62234246f4a4eebf6bfcec2594 Mon Sep 17 00:00:00 2001 From: sebres Date: Fri, 2 Mar 2018 09:15:34 +0100 Subject: [PATCH 3/5] filter.d/asterisk.conf: fixed failregex prefix by log over remote syslog server (gh-2060) --- ChangeLog | 1 + config/filter.d/asterisk.conf | 2 +- fail2ban/tests/files/logs/asterisk | 3 +++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index e410966a..382ffc4c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -35,6 +35,7 @@ ver. 0.10.3-dev-1 (20??/??/??) - development edition ----------- ### Fixes +* `filter.d/asterisk.conf`: fixed failregex prefix by log over remote syslog server (gh-2060); * `filter.d/exim.conf`: failregex extended - SMTP call dropped: too many syntax or protocol errors (gh-2048); * `action.d/badips.py`: implicit convert IPAddr to str, solves an issue "expected string, IPAddr found" (gh-2059); * (Free)BSD ipfw actionban fixed to allow same rule added several times (gh-2054); diff --git a/config/filter.d/asterisk.conf b/config/filter.d/asterisk.conf index 337e9573..6f7ae5d5 100644 --- a/config/filter.d/asterisk.conf +++ b/config/filter.d/asterisk.conf @@ -16,7 +16,7 @@ __pid_re = (?:\s*\[\d+\]) iso8601 = \d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d+[+-]\d{4} # All Asterisk log messages begin like this: -log_prefix= (?:NOTICE|SECURITY|WARNING)%(__pid_re)s:?(?:\[C-[\da-f]*\])? [^:]+:\d*(?:(?: in)? \w+:)? +log_prefix= (?:NOTICE|SECURITY|WARNING)%(__pid_re)s:?(?:\[C-[\da-f]*\])?:? [^:]+:\d*(?:(?: in)? [^:]+:)? prefregex = ^%(__prefix_line)s%(log_prefix)s .+$ diff --git a/fail2ban/tests/files/logs/asterisk b/fail2ban/tests/files/logs/asterisk index 0955cfe7..7bd011fc 100644 --- a/fail2ban/tests/files/logs/asterisk +++ b/fail2ban/tests/files/logs/asterisk @@ -106,3 +106,6 @@ Nov 4 18:30:40 localhost asterisk[32229]: NOTICE[32257]: chan_sip.c:23417 in han # #_dis_failJSON: { "time": "2016-05-06T07:08:09", "match": true, "host": "192.0.2.6" } # [2016-05-06 07:08:09] WARNING[6410][C-00000bac] Ext. +012345: Friendly Scanner from 192.0.2.6 # # Yes, this does have quotes around it. + +# failJSON: { "time": "2005-03-01T15:35:53", "match": true , "host": "192.0.2.2", "desc": "log over remote syslog server" } +Mar 1 15:35:53 pbx asterisk[2350]: WARNING[1195][C-00000b43]: Ext. s:6 in @ from-sip-external: "Rejecting unknown SIP connection from 192.0.2.2" From fa520f36c32100ef48c8aba48601997b77eeac47 Mon Sep 17 00:00:00 2001 From: sebres Date: Fri, 2 Mar 2018 17:00:01 +0100 Subject: [PATCH 4/5] stability test-cases fix: avoid rare sporadic error on start of server (threaded in foreground); additionally show the log output of the thread-server in case of any error there. --- fail2ban/tests/clientreadertestcase.py | 6 ++-- fail2ban/tests/fail2banclienttestcase.py | 39 +++++++++++++++++++++--- fail2ban/tests/utils.py | 3 -- 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/fail2ban/tests/clientreadertestcase.py b/fail2ban/tests/clientreadertestcase.py index 9e5f0dfe..184595ab 100644 --- a/fail2ban/tests/clientreadertestcase.py +++ b/fail2ban/tests/clientreadertestcase.py @@ -266,7 +266,7 @@ class JailReaderTest(LogCaptureTestCase): self.assertEqual(('mail--ho_is', {}), extractOptions("mail--ho_is")) self.assertEqual(('mail--ho_is', {}), extractOptions("mail--ho_is['s']")) - #self.printLog() + #print(self.getLog()) #self.assertLogged("Invalid argument ['s'] in ''s''") self.assertEqual(('mail', {'a': ','}), extractOptions("mail[a=',']")) @@ -528,7 +528,7 @@ class JailsReaderTestCache(LogCaptureTestCase): # how many times jail.local was read: cnt = self._getLoggedReadCount('jail.local') # if cnt > 1: - # self.printLog() + # print(self.getLog()) self.assertTrue(cnt == 1, "Unexpected count by reading of jail files, cnt = %s" % cnt) # read whole configuration like a file2ban-client, again ... @@ -648,7 +648,7 @@ class JailsReaderTest(LogCaptureTestCase): ## We should not "read" some bogus jail #old_comm_commands = comm_commands[:] # make a copy #self.assertRaises(ValueError, jails.getOptions, "BOGUS") - #self.printLog() + #print(self.getLog()) #self.assertLogged("No section: 'BOGUS'") ## and there should be no side-effects #self.assertEqual(jails.convert(), old_comm_commands) diff --git a/fail2ban/tests/fail2banclienttestcase.py b/fail2ban/tests/fail2banclienttestcase.py index 318a1c50..ed3d9868 100644 --- a/fail2ban/tests/fail2banclienttestcase.py +++ b/fail2ban/tests/fail2banclienttestcase.py @@ -210,6 +210,12 @@ def _start_params(tmp, use_stock=False, use_stock_cfg=None, "--timeout", str(fail2bancmdline.MAX_WAITTIME), ) +def _inherited_log(startparams): + try: + return startparams[startparams.index('--logtarget')+1] == 'INHERITED' + except ValueError: + return False + def _get_pid_from_file(pidfile): pid = None try: @@ -325,6 +331,13 @@ def with_foreground_server_thread(startextra={}): self.pruneLog() # several commands to server in body of decorated function: return f(self, tmp, startparams, *args, **kwargs) + except Exception as e: # pragma: no cover + print('=== Catch an exception: %s' % e) + log = self.getLog() + if log: + print('=== Error of server, log: ===\n%s===' % log) + self.pruneLog() + raise finally: if th: # wait for server end (if not yet already exited): @@ -369,7 +382,8 @@ class Fail2banClientServerBase(LogCaptureTestCase): else: raise FailExitException() - def _wait_for_srv(self, tmp, ready=True, startparams=None, phase={}): + def _wait_for_srv(self, tmp, ready=True, startparams=None, phase=None): + if not phase: phase = {} try: sock = pjoin(tmp, "f2b.sock") # wait for server (socket): @@ -384,14 +398,17 @@ class Fail2banClientServerBase(LogCaptureTestCase): ret = Utils.wait_for(lambda: "Server ready" in self.getLog(), MAX_WAITTIME) if not ret: # pragma: no cover - test-failure case only raise Exception( - 'Unexpected: Server ready was not found.\nStart failed: %r' - % (startparams,) + 'Unexpected: Server ready was not found, phase %r.\nStart failed: %r' + % (phase, startparams,) ) except: # pragma: no cover + if _inherited_log(startparams): + print('=== Error by wait fot server, log: ===\n%s===' % self.getLog()) + self.pruneLog() log = pjoin(tmp, "f2b.log") if isfile(log): _out_file(log) - else: + elif not _inherited_log(startparams): logSys.debug("No log file %s to examine details of error", log) raise @@ -410,6 +427,7 @@ class Fail2banClientServerBase(LogCaptureTestCase): self.execCmd(SUCCESS, ("-f",) + startparams, "start") finally: # end : + phase['start'] = False phase['end'] = True logSys.debug("end of test worker") @@ -1192,7 +1210,7 @@ class Fail2banServerTest(Fail2banClientServerBase): 'failregex = ^ failure "[^"]+" - ', 'maxretry = 1', # ban by first failure 'enabled = true', - ) + ) }) def testServerActions_NginxBlockMap(self, tmp, startparams): cfg = pjoin(tmp, "config") @@ -1251,3 +1269,14 @@ class Fail2banServerTest(Fail2banClientServerBase): _out_file(mpfn) mp = _read_file(mpfn) self.assertEqual(mp, '') + + # test multiple start/stop of the server (threaded in foreground) -- + if False: # pragma: no cover + @with_foreground_server_thread() + def _testServerStartStop(self, tmp, startparams): + # stop server and wait for end: + self.stopAndWaitForServerEnd(SUCCESS) + + def testServerStartStop(self): + for i in xrange(2000): + self._testServerStartStop() diff --git a/fail2ban/tests/utils.py b/fail2ban/tests/utils.py index f681db76..3eeb8eb4 100644 --- a/fail2ban/tests/utils.py +++ b/fail2ban/tests/utils.py @@ -807,8 +807,5 @@ class LogCaptureTestCase(unittest.TestCase): def getLog(self): return self._log.getvalue() - def printLog(self): - print(self._log.getvalue()) - pid_exists = Utils.pid_exists From 5f021aa648c42ee188d8c31a81937d764982a58a Mon Sep 17 00:00:00 2001 From: sebres Date: Fri, 2 Mar 2018 17:08:23 +0100 Subject: [PATCH 5/5] shutdown sockets before close, avoid socket leakage by use of the explicit socket close in async_chat; better error handling with error counting, differentiate special case ([Errno 24] Too many open files), with resulting stop of the server (avoid flood the log file, closes gh-991 and similar issues); restored auto-garbage, because of non-reference-counting python's (like pypy), otherwise it may leak there on objects like unix-socket, etc. --- fail2ban/client/csocket.py | 1 + fail2ban/helpers.py | 4 +- fail2ban/server/asyncserver.py | 75 +++++++++++++++++++++++++++------- 3 files changed, 64 insertions(+), 16 deletions(-) diff --git a/fail2ban/client/csocket.py b/fail2ban/client/csocket.py index 6b478460..86dd17c9 100644 --- a/fail2ban/client/csocket.py +++ b/fail2ban/client/csocket.py @@ -61,6 +61,7 @@ class CSocket: return if sendEnd: self.__csock.sendall(CSPROTO.CLOSE + CSPROTO.END) + self.__csock.shutdown(socket.SHUT_RDWR) self.__csock.close() self.__csock = None diff --git a/fail2ban/helpers.py b/fail2ban/helpers.py index 98d59fa1..6a3ed2fd 100644 --- a/fail2ban/helpers.py +++ b/fail2ban/helpers.py @@ -393,7 +393,9 @@ class BgService(object): self.__count = self.__threshold; if hasattr(gc, 'set_threshold'): gc.set_threshold(0) - gc.disable() + # don't disable auto garbage, because of non-reference-counting python's (like pypy), + # otherwise it may leak there on objects like unix-socket, etc. + #gc.disable() def service(self, force=False, wait=False): self.__count -= 1 diff --git a/fail2ban/server/asyncserver.py b/fail2ban/server/asyncserver.py index e254979d..eb99c69a 100644 --- a/fail2ban/server/asyncserver.py +++ b/fail2ban/server/asyncserver.py @@ -42,21 +42,36 @@ from ..helpers import logging, getLogger, formatExceptionInfo # Gets the instance of the logger. logSys = getLogger(__name__) + ## # Request handler class. # # This class extends asynchat in order to provide a request handler for # incoming query. - class RequestHandler(asynchat.async_chat): def __init__(self, conn, transmitter): asynchat.async_chat.__init__(self, conn) + self.__conn = conn self.__transmitter = transmitter self.__buffer = [] # Sets the terminator. self.set_terminator(CSPROTO.END) + def __close(self): + if self.__conn: + conn = self.__conn + self.__conn = None + try: + conn.shutdown(socket.SHUT_RDWR) + conn.close() + except socket.error: # pragma: no cover - normally unreachable + pass + + def handle_close(self): + self.__close() + asynchat.async_chat.handle_close(self) + def collect_incoming_data(self, data): #logSys.debug("Received raw data: " + str(data)) self.__buffer.append(data) @@ -111,14 +126,15 @@ class RequestHandler(asynchat.async_chat): self.close_when_done() -def loop(active, timeout=None, use_poll=False): +def loop(active, timeout=None, use_poll=False, err_count=None): """Custom event loop implementation Uses poll instead of loop to respect `active` flag, to avoid loop timeout mistake: different in poll and poll2 (sec vs ms), and to prevent sporadic errors like EBADF 'Bad file descriptor' etc. (see gh-161) """ - errCount = 0 + if not err_count: err_count={} + err_count['listen'] = 0 if timeout is None: timeout = Utils.DEFAULT_SLEEP_TIME poll = asyncore.poll @@ -133,22 +149,29 @@ def loop(active, timeout=None, use_poll=False): while active(): try: poll(timeout) - if errCount: - errCount -= 1 + if err_count['listen']: + err_count['listen'] -= 1 except Exception as e: if not active(): break - errCount += 1 - if errCount < 20: + err_count['listen'] += 1 + if err_count['listen'] < 20: # errno.ENOTCONN - 'Socket is not connected' # errno.EBADF - 'Bad file descriptor' if e.args[0] in (errno.ENOTCONN, errno.EBADF): # pragma: no cover (too sporadic) logSys.info('Server connection was closed: %s', str(e)) else: logSys.error('Server connection was closed: %s', str(e)) - elif errCount == 20: + elif err_count['listen'] == 20: logSys.exception(e) logSys.error('Too many errors - stop logging connection errors') + elif err_count['listen'] > 100: # pragma: no cover - normally unreachable + if ( + e.args[0] == errno.EMFILE # [Errno 24] Too many open files + or sum(err_count.itervalues()) > 1000 + ): + logSys.critical("Too many errors - critical count reached %r", err_count) + break ## @@ -165,6 +188,7 @@ class AsyncServer(asyncore.dispatcher): self.__sock = "/var/run/fail2ban/fail2ban.sock" self.__init = False self.__active = False + self.__errCount = {'accept': 0, 'listen': 0} self.onstart = None ## @@ -176,12 +200,25 @@ class AsyncServer(asyncore.dispatcher): def handle_accept(self): try: conn, addr = self.accept() - except socket.error: # pragma: no cover - logSys.warning("Socket error") + except socket.error as e: # pragma: no cover + self.__errCount['accept'] += 1 + if self.__errCount['accept'] < 20: + logSys.warning("Socket error: %s", e) + elif self.__errCount['accept'] == 20: + logSys.error("Too many acceptor errors - stop logging errors") + elif self.__errCount['accept'] > 100: + if ( + e.args[0] == errno.EMFILE # [Errno 24] Too many open files + or sum(self.__errCount.itervalues()) > 1000 + ): + logSys.critical("Too many errors - critical count reached %r", err_count) + self.stop() return - except TypeError: # pragma: no cover - logSys.warning("Type error") + except TypeError as e: # pragma: no cover + logSys.warning("Type error: %s", e) return + if self.__errCount['accept']: + self.__errCount['accept'] -= 1; AsyncServer.__markCloseOnExec(conn) # Creates an instance of the handler class to handle the # request/response on the incoming connection. @@ -219,7 +256,7 @@ class AsyncServer(asyncore.dispatcher): if self.onstart: self.onstart() # Event loop as long as active: - loop(lambda: self.__loop, timeout=timeout, use_poll=use_poll) + loop(lambda: self.__loop, timeout=timeout, use_poll=use_poll, err_count=self.__errCount) self.__active = False # Cleanup all self.stop() @@ -246,13 +283,21 @@ class AsyncServer(asyncore.dispatcher): # Stops the communication server. def stop_communication(self): - logSys.debug("Stop communication") - self.__transmitter = None + if self.__transmitter: + logSys.debug("Stop communication") + self.__transmitter = None + # shutdown socket here: + if self.socket: + try: + self.socket.shutdown(socket.SHUT_RDWR) + except socket.error: # pragma: no cover - normally unreachable + pass ## # Stops the server. def stop(self): + self.stop_communication() self.close() # better remains a method (not a property) since used as a callable for wait_for