Merge pull request #1616 from sebres/fix-1194

[fix-gh-1194] Fixed misleading errors logged from ignorecommand in success case on retcode 1

ChangeLog

@@ -28,6 +28,7 @@ TODO: implementing of options resp. other tasks from PR #1346
 * Pyinotify-backend: stability fix for sporadically errors in multi-threaded
   environment (without lock)
 * Fixed sporadically error in testCymruInfoNxdomain, because of unsorted values
+* Misleading errors logged from ignorecommand in success case on retcode 1 (gh-1194)
 
 ### New Features
 * IPv6 support:

fail2ban/server/action.py

@@ -584,7 +584,7 @@ class CommandAction(ActionBase):
 		return self.executeCmd(realCmd, self.timeout)
 
 	@staticmethod
-	def executeCmd(realCmd, timeout=60):
+	def executeCmd(realCmd, timeout=60, **kwargs):
 		"""Executes a command.
 
 		Parameters
@@ -613,6 +613,6 @@ class CommandAction(ActionBase):
 
 		_cmd_lock.acquire()
 		try:
-			return Utils.executeCmd(realCmd, timeout, shell=True, output=False)
+			return Utils.executeCmd(realCmd, timeout, shell=True, output=False, **kwargs)
 		finally:
 			_cmd_lock.release()

fail2ban/server/filter.py

@@ -455,7 +455,8 @@ class Filter(JailThread):
 		if self.__ignoreCommand:
 			command = CommandAction.replaceTag(self.__ignoreCommand, { 'ip': ip } )
 			logSys.debug('ignore command: ' + command)
-			ret_ignore = CommandAction.executeCmd(command)
+			ret, ret_ignore = CommandAction.executeCmd(command, success_codes=(0, 1))
+			ret_ignore = ret and ret_ignore == 0
 			self.logIgnoreIp(ip, log_ignore and ret_ignore, ignore_source="command")
 			return ret_ignore
 

fail2ban/server/utils.py

@@ -110,7 +110,7 @@ class Utils():
 		return flags
 
 	@staticmethod
-	def executeCmd(realCmd, timeout=60, shell=True, output=False, tout_kill_tree=True):
+	def executeCmd(realCmd, timeout=60, shell=True, output=False, tout_kill_tree=True, success_codes=(0,)):
 		"""Executes a command.
 
 		Parameters
@@ -178,7 +178,7 @@ class Utils():
 			if not popen:
 				return False if not output else (False, stdout, stderr, retcode)
 
-		std_level = retcode == 0 and logging.DEBUG or logging.ERROR
+		std_level = logging.DEBUG if retcode in success_codes else logging.ERROR
 		# if we need output (to return or to log it): 
 		if output or std_level >= logSys.getEffectiveLevel():
 			# if was timeouted (killed/terminated) - to prevent waiting, set std handles to non-blocking mode.
@@ -208,8 +208,8 @@ class Utils():
 				popen.stderr.close()
 
 		success = False
-		if retcode == 0:
+		if retcode in success_codes:
-			logSys.debug("%-.40s -- returned successfully", realCmd)
+			logSys.debug("%-.40s -- returned successfully %i", realCmd, retcode)
 			success = True
 		elif retcode is None:
 			logSys.error("%-.40s -- unable to kill PID %i", realCmd, popen.pid)
@@ -223,7 +223,9 @@ class Utils():
 			logSys.error("%-.40s -- returned %i", realCmd, retcode)
 			if msg:
 				logSys.info("HINT on %i: %s", retcode, msg % locals())
-		return success if not output else (success, stdout, stderr, retcode)
+		if output:
+			return success, stdout, stderr, retcode
+		return success if len(success_codes) == 1 else (success, retcode)
 	
 	@staticmethod
 	def wait_for(cond, timeout, interval=None):

fail2ban/tests/files/ignorecommand.py

@@ -1,5 +1,8 @@
 #!/usr/bin/env fail2ban-python
 import sys
+if len(sys.argv) != 2 or sys.argv[1] == "":
+	sys.stderr.write('usage: ignorecommand IP')
+	exit(10)
 if sys.argv[1] == "10.0.0.1":
 	exit(0)
 exit(1)

fail2ban/tests/filtertestcase.py

@@ -377,6 +377,10 @@ class IgnoreIP(LogCaptureTestCase):
 		self.filter.setIgnoreCommand(sys.executable + ' ' + os.path.join(TEST_FILES_DIR, "ignorecommand.py <ip>"))
 		self.assertTrue(self.filter.inIgnoreIPList("10.0.0.1"))
 		self.assertFalse(self.filter.inIgnoreIPList("10.0.0.0"))
+		self.assertLogged("returned successfully 0", "returned successfully 1", all=True)
+		self.pruneLog()
+		self.assertFalse(self.filter.inIgnoreIPList(""))
+		self.assertLogged("usage: ignorecommand IP", "returned 10", all=True)
 
 	def testIgnoreCauseOK(self):
 		ip = "93.184.216.34"