From 63acc862b139bb1d8b45edb0b3716044b36c7113 Mon Sep 17 00:00:00 2001 From: sebres Date: Wed, 24 Feb 2021 18:21:42 +0100 Subject: [PATCH 1/3] `action.d/nginx-block-map.conf`: reload nginx only if it is running (also avoid error in nginx-errorlog, gh-2949) and better test coverage for the action --- config/action.d/nginx-block-map.conf | 11 +++++++++-- fail2ban/tests/fail2banclienttestcase.py | 4 +++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/config/action.d/nginx-block-map.conf b/config/action.d/nginx-block-map.conf index ee702907..0de382bd 100644 --- a/config/action.d/nginx-block-map.conf +++ b/config/action.d/nginx-block-map.conf @@ -84,8 +84,15 @@ srv_cfg_path = /etc/nginx/ #srv_cmd = nginx -c %(srv_cfg_path)s/nginx.conf srv_cmd = nginx -# first test configuration is correct, hereafter send reload signal: -blck_lst_reload = %(srv_cmd)s -qt; if [ $? -eq 0 ]; then +# pid file (used to check nginx is running): +srv_pid = /run/nginx.pid + +# command used to check whether nginx is running and configuration is valid: +srv_is_running = [ -f "%(srv_pid)s" ] +srv_check_cmd = %(srv_is_running)s && %(srv_cmd)s -qt + +# first test nginx is running and configuration is correct, hereafter send reload signal: +blck_lst_reload = %(srv_check_cmd)s; if [ $? -eq 0 ]; then %(srv_cmd)s -s reload; if [ $? -ne 0 ]; then echo 'reload failed.'; fi; fi; diff --git a/fail2ban/tests/fail2banclienttestcase.py b/fail2ban/tests/fail2banclienttestcase.py index 03b1d7ce..d1aec5ab 100644 --- a/fail2ban/tests/fail2banclienttestcase.py +++ b/fail2ban/tests/fail2banclienttestcase.py @@ -1281,7 +1281,7 @@ class Fail2banServerTest(Fail2banClientServerBase): 'backend = polling', 'usedns = no', 'logpath = %(tmp)s/blck-failures.log', - 'action = nginx-block-map[blck_lst_reload="", blck_lst_file="%(tmp)s/blck-lst.map"]', + 'action = nginx-block-map[srv_cmd="echo nginx", srv_pid="%(tmp)s/f2b.pid", blck_lst_file="%(tmp)s/blck-lst.map"]', ' blocklist_de[actionban=\'curl() { echo "*** curl" "$*";}; \', email="Fail2Ban ", ' 'apikey="TEST-API-KEY", agent="fail2ban-test-agent", service=]', 'filter =', @@ -1321,6 +1321,8 @@ class Fail2banServerTest(Fail2banClientServerBase): self.assertIn('\\125-000-004 1;\n', mp) self.assertIn('\\125-000-005 1;\n', mp) + # check nginx reload is logged (pid of fail2ban is used to simulate success check nginx is running): + self.assertLogged("stdout: 'nginx -qt'", "stdout: 'nginx -s reload'", all=True) # check blocklist_de substitution (e. g. new-line after ): self.assertLogged( "stdout: '*** curl --fail --data-urlencode server=Fail2Ban " From a45b1c974c969e9486326872447c33feca9ce0b2 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 2 Mar 2021 19:24:03 +0100 Subject: [PATCH 2/3] filter.d/ignorecommands/apache-fakegooglebot: added timeout parameter (default 55 seconds) - avoid fail with timeout (default 1 minute) by reverse lookup on some slow DNS services (googlebots must be resolved fast); closes gh-2951 --- .../ignorecommands/apache-fakegooglebot | 25 +++++++++++++------ fail2ban/tests/filtertestcase.py | 13 +++++----- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/config/filter.d/ignorecommands/apache-fakegooglebot b/config/filter.d/ignorecommands/apache-fakegooglebot index b11f0d98..8351efa2 100755 --- a/config/filter.d/ignorecommands/apache-fakegooglebot +++ b/config/filter.d/ignorecommands/apache-fakegooglebot @@ -6,24 +6,35 @@ # import sys from fail2ban.server.ipdns import DNSUtils, IPAddr +from threading import Thread def process_args(argv): - if len(argv) != 2: - raise ValueError("Please provide a single IP as an argument. Got: %s\n" - % (argv[1:])) + if len(argv) - 1 not in (1, 2): + raise ValueError("Usage %s ip ?timeout?. Got: %s\n" + % (argv[0], argv[1:])) ip = argv[1] if not IPAddr(ip).isValid: raise ValueError("Argument must be a single valid IP. Got: %s\n" % ip) - return ip + return argv[1:] google_ips = None -def is_googlebot(ip): +def is_googlebot(ip, timeout=55): import re - host = DNSUtils.ipToName(ip) + timeout = float(timeout or 0) + if timeout: + def ipToNameTO(host, ip, timeout): + host[0] = DNSUtils.ipToName(ip) + host = [None] + th = Thread(target=ipToNameTO, args=(host, ip, timeout)); th.daemon=True; th.start() + th.join(timeout) + host = host[0] + else: + host = DNSUtils.ipToName(ip) + if not host or not re.match(r'.*\.google(bot)?\.com$', host): return False host_ips = DNSUtils.dnsToIp(host) @@ -31,7 +42,7 @@ def is_googlebot(ip): if __name__ == '__main__': # pragma: no cover try: - ret = is_googlebot(process_args(sys.argv)) + ret = is_googlebot(*process_args(sys.argv)) except ValueError as e: sys.stderr.write(str(e)) sys.exit(2) diff --git a/fail2ban/tests/filtertestcase.py b/fail2ban/tests/filtertestcase.py index b9b7e8aa..4f716663 100644 --- a/fail2ban/tests/filtertestcase.py +++ b/fail2ban/tests/filtertestcase.py @@ -606,13 +606,14 @@ class IgnoreIPDNS(LogCaptureTestCase): cmd = os.path.join(STOCK_CONF_DIR, "filter.d/ignorecommands/apache-fakegooglebot") ## below test direct as python module: mod = Utils.load_python_module(cmd) - self.assertFalse(mod.is_googlebot(mod.process_args([cmd, "128.178.222.69"]))) - self.assertFalse(mod.is_googlebot(mod.process_args([cmd, "192.0.2.1"]))) + self.assertFalse(mod.is_googlebot(*mod.process_args([cmd, "128.178.222.69"]))) + self.assertFalse(mod.is_googlebot(*mod.process_args([cmd, "192.0.2.1"]))) + self.assertFalse(mod.is_googlebot(*mod.process_args([cmd, "192.0.2.1", 0.1]))) bot_ips = ['66.249.66.1'] for ip in bot_ips: - self.assertTrue(mod.is_googlebot(mod.process_args([cmd, str(ip)])), "test of googlebot ip %s failed" % ip) - self.assertRaises(ValueError, lambda: mod.is_googlebot(mod.process_args([cmd]))) - self.assertRaises(ValueError, lambda: mod.is_googlebot(mod.process_args([cmd, "192.0"]))) + self.assertTrue(mod.is_googlebot(*mod.process_args([cmd, str(ip)])), "test of googlebot ip %s failed" % ip) + self.assertRaises(ValueError, lambda: mod.is_googlebot(*mod.process_args([cmd]))) + self.assertRaises(ValueError, lambda: mod.is_googlebot(*mod.process_args([cmd, "192.0"]))) ## via command: self.filter.ignoreCommand = cmd + " " for ip in bot_ips: @@ -624,7 +625,7 @@ class IgnoreIPDNS(LogCaptureTestCase): self.pruneLog() self.filter.ignoreCommand = cmd + " bad arguments " self.assertFalse(self.filter.inIgnoreIPList("192.0")) - self.assertLogged('Please provide a single IP as an argument.') + self.assertLogged('Usage') From 04aba6168c5f9b4b7b2bc4e2f4d128b62c3b7633 Mon Sep 17 00:00:00 2001 From: sebres Date: Wed, 3 Mar 2021 13:02:00 +0100 Subject: [PATCH 3/3] fixed typo, `--` is not expected in options declaration, so `--dump-pretty` did never work (only `--dp` is working) --- fail2ban/client/fail2bancmdline.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fail2ban/client/fail2bancmdline.py b/fail2ban/client/fail2bancmdline.py index 03683cad..c2f6d0be 100644 --- a/fail2ban/client/fail2bancmdline.py +++ b/fail2ban/client/fail2bancmdline.py @@ -192,7 +192,7 @@ class Fail2banCmdLine(): cmdOpts = 'hc:s:p:xfbdtviqV' cmdLongOpts = ['loglevel=', 'logtarget=', 'syslogsocket=', 'test', 'async', 'conf=', 'pidfile=', 'pname=', 'socket=', - 'timeout=', 'str2sec=', 'help', 'version', 'dp', '--dump-pretty'] + 'timeout=', 'str2sec=', 'help', 'version', 'dp', 'dump-pretty'] optList, self._args = getopt.getopt(self._argv[1:], cmdOpts, cmdLongOpts) except getopt.GetoptError: self.dispUsage()