From d07df6637071b7ffa8be988a834a6bb71d9bf932 Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Sat, 27 Apr 2013 20:06:09 +0100 Subject: [PATCH] NF: Allow setting of timeout for execution of action commands This uses subprocess.Popen, polling until `timeout` seconds has passed or the command has exit. If the command has not exited, fail2ban then sends SIGTERM, and if this is unsuccessful, SIGKILL. The timeout can be changed for an entire action via action [Init] options, or via jail.conf override, or fail2ban-client. The default timeout period is 60 seconds. --- fail2ban/client/actionreader.py | 4 +- fail2ban/protocol.py | 2 + fail2ban/server/action.py | 82 ++++++++++++++++++++++++-------- fail2ban/server/server.py | 6 +++ fail2ban/server/transmitter.py | 8 ++++ fail2ban/tests/actiontestcase.py | 9 +++- fail2ban/tests/servertestcase.py | 8 ++++ 7 files changed, 97 insertions(+), 22 deletions(-) diff --git a/fail2ban/client/actionreader.py b/fail2ban/client/actionreader.py index ca4080d0..2928a37f 100644 --- a/fail2ban/client/actionreader.py +++ b/fail2ban/client/actionreader.py @@ -61,8 +61,10 @@ class ActionReader(DefinitionInitConfigReader): stream.append(head + ["actionban", self._file, self._opts[opt]]) elif opt == "actionunban": stream.append(head + ["actionunban", self._file, self._opts[opt]]) - # cInfo if self._initOpts: + if "timeout" in self._initOpts: + stream.append(head + ["timeout", self._file, self._opts["timeout"]]) + # cInfo for p in self._initOpts: stream.append(head + ["setcinfo", self._file, p, self._initOpts[p]]) diff --git a/fail2ban/protocol.py b/fail2ban/protocol.py index cacde4c2..607cabed 100644 --- a/fail2ban/protocol.py +++ b/fail2ban/protocol.py @@ -73,6 +73,7 @@ protocol = [ ["set delaction ", "removes the action from "], ["set setcinfo ", "sets for of the action for "], ["set delcinfo ", "removes for the action for "], +["set timeout ", "sets as the command timeout in seconds for the action for "], ["set actionstart ", "sets the start command of the action for "], ["set actionstop ", "sets the stop command of the action for "], ["set actioncheck ", "sets the check command of the action for "], @@ -96,6 +97,7 @@ protocol = [ ["get actionban ", "gets the ban command for the action for "], ["get actionunban ", "gets the unban command for the action for "], ["get cinfo ", "gets the value for for the action for "], +["get timeout ", "gets the command timeout in seconds for the action for "], ] ## diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index b8356799..6c36bbca 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -27,7 +27,7 @@ __date__ = "$Date$" __copyright__ = "Copyright (c) 2004 Cyril Jaquier" __license__ = "GPL" -import logging, os +import logging, os, subprocess, time, signal import threading #from subprocess import call @@ -39,7 +39,7 @@ _cmd_lock = threading.Lock() # Some hints on common abnormal exit codes _RETCODE_HINTS = { - 0x7f00: '"Command not found". Make sure that all commands in %(realCmd)r ' + 127: '"Command not found". Make sure that all commands in %(realCmd)r ' 'are in the PATH of fail2ban-server process ' '(grep -a PATH= /proc/`pidof -x fail2ban-server`/environ). ' 'You may want to start ' @@ -48,6 +48,10 @@ _RETCODE_HINTS = { 'additional informative error messages appear in the terminals.' } +# Dictionary to lookup signal name from number +signame = dict((num, name) + for name, num in signal.__dict__.iteritems() if name.startswith("SIG")) + ## # Execute commands. # @@ -59,6 +63,7 @@ class Action: def __init__(self, name): self.__name = name + self.__timeout = 60 self.__cInfo = dict() ## Command executed in order to initialize the system. self.__actionStart = '' @@ -88,6 +93,23 @@ class Action: def getName(self): return self.__name + ## + # Sets the timeout period for commands. + # + # @param timeout timeout period in seconds + + def setTimeout(self, timeout): + self.__timeout = int(timeout) + logSys.debug("Set action %s timeout = %i" % (self.__name, timeout)) + + ## + # Returns the action timeout period for commands. + # + # @return the timeout period in seconds + + def getTimeout(self): + return self.__timeout + ## # Sets a "CInfo". # @@ -144,7 +166,7 @@ class Action: def execActionStart(self): startCmd = Action.replaceTag(self.__actionStart, self.__cInfo) - return Action.executeCmd(startCmd) + return Action.executeCmd(startCmd, self.__timeout) ## # Set the "ban" command. @@ -240,7 +262,7 @@ class Action: def execActionStop(self): stopCmd = Action.replaceTag(self.__actionStop, self.__cInfo) - return Action.executeCmd(stopCmd) + return Action.executeCmd(stopCmd, self.__timeout) def escapeTag(tag): for c in '\\#&;`|*?~<>^()[]{}$\n\'"': @@ -294,12 +316,12 @@ class Action: return True checkCmd = Action.replaceTag(self.__actionCheck, self.__cInfo) - if not Action.executeCmd(checkCmd): + if not Action.executeCmd(checkCmd, self.__timeout): logSys.error("Invariant check failed. Trying to restore a sane" + " environment") self.execActionStop() self.execActionStart() - if not Action.executeCmd(checkCmd): + if not Action.executeCmd(checkCmd, self.__timeout): logSys.fatal("Unable to restore environment") return False @@ -312,7 +334,7 @@ class Action: # Replace static fields realCmd = Action.replaceTag(realCmd, self.__cInfo) - return Action.executeCmd(realCmd) + return Action.executeCmd(realCmd, self.__timeout) ## # Executes a command. @@ -327,27 +349,47 @@ class Action: # @return True if the command succeeded #@staticmethod - def executeCmd(realCmd): + def executeCmd(realCmd, timeout=60): logSys.debug(realCmd) _cmd_lock.acquire() try: # Try wrapped within another try needed for python version < 2.5 try: - # The following line gives deadlock with multiple jails - #retcode = call(realCmd, shell=True) - retcode = os.system(realCmd) - if retcode == 0: - logSys.debug("%s returned successfully" % realCmd) - return True - else: - msg = _RETCODE_HINTS.get(retcode, None) - logSys.error("%s returned %x" % (realCmd, retcode)) - if msg: - logSys.info("HINT on %x: %s" - % (retcode, msg % locals())) + popen = subprocess.Popen(realCmd, shell=True) + stime = time.time() + retcode = popen.poll() + while time.time() - stime <= timeout and retcode is None: + time.sleep(0.1) + retcode = popen.poll() + if retcode is None: + logSys.error("%s timed out after %i seconds." % + (realCmd, timeout)) + os.kill(popen.pid, signal.SIGTERM) # Terminate the process + time.sleep(0.1) + retcode = popen.poll() + if retcode is None: # Still going... + os.kill(popen.pid, signal.SIGKILL) # Kill the process + time.sleep(0.1) + retcode = popen.poll() except OSError, e: logSys.error("%s failed with %s" % (realCmd, e)) + return False finally: _cmd_lock.release() + + if retcode == 0: + logSys.debug("%s returned successfully" % realCmd) + return True + elif retcode is None: + logSys.error("Unable to kill PID %i: %s" % (popen.pid, realCmd)) + elif retcode < 0: + logSys.error("%s killed with %s" % + (realCmd, signame.get(-retcode, "signal %i" % -retcode))) + else: + msg = _RETCODE_HINTS.get(retcode, None) + logSys.error("%s returned %i" % (realCmd, retcode)) + if msg: + logSys.info("HINT on %i: %s" + % (retcode, msg % locals())) return False executeCmd = staticmethod(executeCmd) diff --git a/fail2ban/server/server.py b/fail2ban/server/server.py index 632d0546..23533fde 100644 --- a/fail2ban/server/server.py +++ b/fail2ban/server/server.py @@ -289,6 +289,12 @@ class Server: def getActionUnban(self, name, action): return self.__jails.getAction(name).getAction(action).getActionUnban() + + def setActionTimeout(self, name, action, value): + self.__jails.getAction(name).getAction(action).setTimeout(value) + + def getActionTimeout(self, name, action): + return self.__jails.getAction(name).getAction(action).getTimeout() # Status def status(self): diff --git a/fail2ban/server/transmitter.py b/fail2ban/server/transmitter.py index 2a4514da..d30809b8 100644 --- a/fail2ban/server/transmitter.py +++ b/fail2ban/server/transmitter.py @@ -234,6 +234,11 @@ class Transmitter: value = " ".join(command[3:]) self.__server.setActionUnban(name, act, value) return self.__server.getActionUnban(name, act) + elif command[1] == "timeout": + act = command[2] + value = int(command[3]) + self.__server.setActionTimeout(name, act, value) + return self.__server.getActionTimeout(name, act) raise Exception("Invalid command (no set action or not yet implemented)") def __commandGet(self, command): @@ -286,6 +291,9 @@ class Transmitter: act = command[2] key = command[3] return self.__server.getCInfo(name, act, key) + elif command[1] == "timeout": + act = command[2] + return self.__server.getActionTimeout(name, act) raise Exception("Invalid command (no get action or not yet implemented)") def status(self, command): diff --git a/fail2ban/tests/actiontestcase.py b/fail2ban/tests/actiontestcase.py index 54c37ef0..6cac0c14 100644 --- a/fail2ban/tests/actiontestcase.py +++ b/fail2ban/tests/actiontestcase.py @@ -95,4 +95,11 @@ class ExecuteAction(unittest.TestCase): def testExecuteIncorrectCmd(self): Action.executeCmd('/bin/ls >/dev/null\nbogusXXX now 2>/dev/null') - self.assertTrue(self._is_logged('HINT on 7f00: "Command not found"')) + self.assertTrue(self._is_logged('HINT on 127: "Command not found"')) + + def testExecuteTimeout(self): + stime = time.time() + Action.executeCmd('sleep 60', timeout=2) # Should take a minute + self.assertAlmostEqual(time.time() - stime, 2.1, places=1) + self.assertTrue(self._is_logged('sleep 60 timed out after 2 seconds')) + self.assertTrue(self._is_logged('sleep 60 killed with SIGTERM')) diff --git a/fail2ban/tests/servertestcase.py b/fail2ban/tests/servertestcase.py index 385b83d4..17c386ab 100644 --- a/fail2ban/tests/servertestcase.py +++ b/fail2ban/tests/servertestcase.py @@ -467,6 +467,14 @@ class Transmitter(TransmitterBase): self.transm.proceed( ["set", self.jailName, "delcinfo", action, "KEY"]), (0, None)) + self.assertEqual( + self.transm.proceed( + ["set", self.jailName, "timeout", action, "10"]), + (0, 10)) + self.assertEqual( + self.transm.proceed( + ["get", self.jailName, "timeout", action]), + (0, 10)) self.assertEqual( self.transm.proceed(["set", self.jailName, "delaction", action]), (0, None))