Merge branch 'ainfo-copy' of https://github.com/kwirk/fail2ban

* 'ainfo-copy' of https://github.com/kwirk/fail2ban:
  TST: actions modifying aInfo test more robust
  TST: Test for actions modifying (un)ban aInfo
  BF: aInfo could be modified by actions, causing unexpected behaviour

config/action.d/smtp.py

@@ -45,7 +45,7 @@ messages['ban'] = {}
 messages['ban']['head'] = \
 """Hi,
 
-The IP %(ip)s has just been banned for %(bantime)s seconds
+The IP %(ip)s has just been banned for %(bantime)i seconds
 by Fail2Ban after %(failures)i attempts against %(jailname)s.
 """
 messages['ban']['tail'] = \

fail2ban/server/action.py

@@ -69,6 +69,9 @@ class CallingMap(MutableMapping):
 	def __init__(self, *args, **kwargs):
 		self.data = dict(*args, **kwargs)
 
+	def __repr__(self):
+		return "%s(%r)" % (self.__class__.__name__, self.data)
+
 	def __getitem__(self, key):
 		value = self.data[key]
 		if callable(value):
@@ -88,6 +91,9 @@ class CallingMap(MutableMapping):
 	def __len__(self):
 		return len(self.data)
 
+	def copy(self):
+		return self.__class__(self.data.copy())
+
 class ActionBase(object):
 	"""An abstract base class for actions in Fail2Ban.
 

fail2ban/server/actions.py

@@ -274,11 +274,12 @@ class Actions(JailThread, Mapping):
 				logSys.notice("[%s] Ban %s" % (self._jail.name, aInfo["ip"]))
 				for name, action in self._actions.iteritems():
 					try:
-						action.ban(aInfo)
+						action.ban(aInfo.copy())
 					except Exception as e:
 						logSys.error(
-							"Failed to execute ban jail '%s' action '%s': %s",
-							self._jail.name, name, e,
+							"Failed to execute ban jail '%s' action '%s' "
+							"info '%r': %s",
+							self._jail.name, name, aInfo, e,
 							exc_info=logSys.getEffectiveLevel()<=logging.DEBUG)
 				return True
 			else:
@@ -322,11 +323,12 @@ class Actions(JailThread, Mapping):
 		logSys.notice("[%s] Unban %s" % (self._jail.name, aInfo["ip"]))
 		for name, action in self._actions.iteritems():
 			try:
-				action.unban(aInfo)
+				action.unban(aInfo.copy())
 			except Exception as e:
 				logSys.error(
-					"Failed to execute unban jail '%s' action '%s': %s",
-					self._jail.name, name, e,
+					"Failed to execute unban jail '%s' action '%s' "
+					"info '%r': %s",
+					self._jail.name, name, aInfo, e,
 					exc_info=logSys.getEffectiveLevel()<=logging.DEBUG)
 
 	@property

fail2ban/tests/actionstestcase.py

@@ -29,6 +29,7 @@ import os
 import tempfile
 
 from ..server.actions import Actions
+from ..server.ticket import FailTicket
 from .dummyjail import DummyJail
 from .utils import LogCaptureTestCase
 
@@ -140,3 +141,28 @@ class ExecuteActions(LogCaptureTestCase):
 		self.__actions.stop()
 		self.__actions.join()
 		self.assertTrue(self._is_logged("Failed to stop"))
+
+	def testBanActionsAInfo(self):
+		# Action which deletes IP address from aInfo
+		self.__actions.add(
+			"action1",
+			os.path.join(TEST_FILES_DIR, "action.d/action_modifyainfo.py"),
+			{})
+		self.__actions.add(
+			"action2",
+			os.path.join(TEST_FILES_DIR, "action.d/action_modifyainfo.py"),
+			{})
+		self.__jail.putFailTicket(FailTicket("1.2.3.4", 0))
+		self.__actions._Actions__checkBan()
+		# Will fail if modification of aInfo from first action propagates
+		# to second action, as both delete same key
+		self.assertFalse(self._is_logged("Failed to execute ban"))
+		self.assertTrue(self._is_logged("action1 ban deleted aInfo IP"))
+		self.assertTrue(self._is_logged("action2 ban deleted aInfo IP"))
+
+		self.__actions._Actions__flushBan()
+		# Will fail if modification of aInfo from first action propagates
+		# to second action, as both delete same key
+		self.assertFalse(self._is_logged("Failed to execute unban"))
+		self.assertTrue(self._is_logged("action1 unban deleted aInfo IP"))
+		self.assertTrue(self._is_logged("action2 unban deleted aInfo IP"))

fail2ban/tests/files/action.d/action_modifyainfo.py

@@ -0,0 +1,14 @@
+
+from fail2ban.server.action import ActionBase
+
+class TestAction(ActionBase):
+
+    def ban(self, aInfo):
+        del aInfo['ip']
+        self._logSys.info("%s ban deleted aInfo IP", self._name)
+
+    def unban(self, aInfo):
+        del aInfo['ip']
+        self._logSys.info("%s unban deleted aInfo IP", self._name)
+
+Action = TestAction