From 7a29a952c133ab54ae69159636b291b70f2ae846 Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Thu, 25 Apr 2013 22:29:50 +0100 Subject: [PATCH 1/8] TST: Add test case for jails with multiple of the same action --- fail2ban/tests/clientreadertestcase.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/fail2ban/tests/clientreadertestcase.py b/fail2ban/tests/clientreadertestcase.py index 62d994c8..1f766fc6 100644 --- a/fail2ban/tests/clientreadertestcase.py +++ b/fail2ban/tests/clientreadertestcase.py @@ -302,3 +302,26 @@ class JailsReaderTest(unittest.TestCase): configurator._Configurator__jails.setBaseDir('/tmp') self.assertEqual(configurator._Configurator__jails.getBaseDir(), '/tmp') self.assertEqual(configurator.getBaseDir(), CONFIG_DIR) + + def testMultipleSameAction(self): + basedir = tempfile.mkdtemp("fail2ban_conf") + os.mkdir(os.path.join(basedir, "filter.d")) + os.mkdir(os.path.join(basedir, "action.d")) + open(os.path.join(basedir, "action.d", "testaction1.conf"), 'w').close() + open(os.path.join(basedir, "filter.d", "testfilter1.conf"), 'w').close() + jailfd = open(os.path.join(basedir, "jail.conf"), 'w') + jailfd.write(""" +[testjail1] +action = testaction1[name=test1] + testaction1[name=test2] +filter = testfilter1 +""") + jailfd.close() + jails = JailsReader(basedir=basedir) + self.assertTrue(jails.read()) + self.assertTrue(jails.getOptions()) + comm_commands = jails.convert() + + action_names = [comm[-1] for comm in comm_commands if comm[:3] == ['set', 'testjail1', 'addaction']] + + self.assertNotEqual(len(set(action_names)), 1) From 45c9c45b41e6b1c6de3a2ae9764666ed515e7694 Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Thu, 25 Apr 2013 22:36:08 +0100 Subject: [PATCH 2/8] BF+RF: Allow multiple of same action in a single jail --- fail2ban/client/actionreader.py | 27 +++++++++++++++++++-------- fail2ban/client/configreader.py | 10 +++++----- fail2ban/client/filterreader.py | 6 +++--- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/fail2ban/client/actionreader.py b/fail2ban/client/actionreader.py index ca4080d0..d2d71a74 100644 --- a/fail2ban/client/actionreader.py +++ b/fail2ban/client/actionreader.py @@ -43,27 +43,38 @@ class ActionReader(DefinitionInitConfigReader): ["string", "actionunban", ""], ] + def __init__(self, file_, jailName, initOpts, **kwargs): + self._name = initOpts.pop("name", file_) + DefinitionInitConfigReader.__init__( + self, file_, jailName, initOpts, **kwargs) + + def setName(self, name): + self._name = name + + def getName(self): + return self._name + def read(self): return ConfigReader.read(self, os.path.join("action.d", self._file)) def convert(self): - head = ["set", self._name] + head = ["set", self._jailName] stream = list() - stream.append(head + ["addaction", self._file]) + stream.append(head + ["addaction", self._name]) for opt in self._opts: if opt == "actionstart": - stream.append(head + ["actionstart", self._file, self._opts[opt]]) + stream.append(head + ["actionstart", self._name, self._opts[opt]]) elif opt == "actionstop": - stream.append(head + ["actionstop", self._file, self._opts[opt]]) + stream.append(head + ["actionstop", self._name, self._opts[opt]]) elif opt == "actioncheck": - stream.append(head + ["actioncheck", self._file, self._opts[opt]]) + stream.append(head + ["actioncheck", self._name, self._opts[opt]]) elif opt == "actionban": - stream.append(head + ["actionban", self._file, self._opts[opt]]) + stream.append(head + ["actionban", self._name, self._opts[opt]]) elif opt == "actionunban": - stream.append(head + ["actionunban", self._file, self._opts[opt]]) + stream.append(head + ["actionunban", self._name, self._opts[opt]]) # cInfo if self._initOpts: for p in self._initOpts: - stream.append(head + ["setcinfo", self._file, p, self._initOpts[p]]) + stream.append(head + ["setcinfo", self._name, p, self._initOpts[p]]) return stream diff --git a/fail2ban/client/configreader.py b/fail2ban/client/configreader.py index 9a1fedaf..ac6af8f7 100644 --- a/fail2ban/client/configreader.py +++ b/fail2ban/client/configreader.py @@ -145,7 +145,7 @@ class DefinitionInitConfigReader(ConfigReader): def __init__(self, file_, jailName, initOpts, **kwargs): ConfigReader.__init__(self, **kwargs) self._file = file_ - self._name = jailName + self._jailName = jailName self._initOpts = initOpts def setFile(self, fileName): @@ -154,11 +154,11 @@ class DefinitionInitConfigReader(ConfigReader): def getFile(self): return self.__file - def setName(self, name): - self._name = name + def setJailName(self, jailName): + self._jailName = jailName - def getName(self): - return self._name + def getJailName(self): + return self._jailName def read(self): return ConfigReader.read(self, self._file) diff --git a/fail2ban/client/filterreader.py b/fail2ban/client/filterreader.py index 62424ddb..5369bc5f 100644 --- a/fail2ban/client/filterreader.py +++ b/fail2ban/client/filterreader.py @@ -50,14 +50,14 @@ class FilterReader(DefinitionInitConfigReader): for regex in self._opts[opt].split('\n'): # Do not send a command if the rule is empty. if regex != '': - stream.append(["set", self._name, "addfailregex", regex]) + stream.append(["set", self._jailName, "addfailregex", regex]) elif opt == "ignoreregex": for regex in self._opts[opt].split('\n'): # Do not send a command if the rule is empty. if regex != '': - stream.append(["set", self._name, "addignoreregex", regex]) + stream.append(["set", self._jailName, "addignoreregex", regex]) if self._initOpts: if 'maxlines' in self._initOpts: - stream.append(["set", self._name, "maxlines", self._initOpts["maxlines"]]) + stream.append(["set", self._jailName, "maxlines", self._initOpts["maxlines"]]) return stream From bec70cbe4be49dafa5cf91b009186f961db4db6c Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Sat, 27 Apr 2013 19:11:58 +0100 Subject: [PATCH 3/8] TST: Clean up after jails test for MultipleSameAction --- fail2ban/tests/clientreadertestcase.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fail2ban/tests/clientreadertestcase.py b/fail2ban/tests/clientreadertestcase.py index 1f766fc6..012fc2bf 100644 --- a/fail2ban/tests/clientreadertestcase.py +++ b/fail2ban/tests/clientreadertestcase.py @@ -325,3 +325,5 @@ filter = testfilter1 action_names = [comm[-1] for comm in comm_commands if comm[:3] == ['set', 'testjail1', 'addaction']] self.assertNotEqual(len(set(action_names)), 1) + + shutil.rmtree(basedir) From a3e216b0b24789f4be6368ccff883f08484b958d Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Sat, 27 Apr 2013 20:56:31 +0100 Subject: [PATCH 4/8] BF: Change name->actname for multi action jails to avoid clash Primary examples is `name` is used in iptables actions for the chain. Also changed pop->get so actname can be used as keyword --- fail2ban/client/actionreader.py | 2 +- fail2ban/tests/clientreadertestcase.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/fail2ban/client/actionreader.py b/fail2ban/client/actionreader.py index d2d71a74..7ae3c45b 100644 --- a/fail2ban/client/actionreader.py +++ b/fail2ban/client/actionreader.py @@ -44,7 +44,7 @@ class ActionReader(DefinitionInitConfigReader): ] def __init__(self, file_, jailName, initOpts, **kwargs): - self._name = initOpts.pop("name", file_) + self._name = initOpts.get("actname", file_) DefinitionInitConfigReader.__init__( self, file_, jailName, initOpts, **kwargs) diff --git a/fail2ban/tests/clientreadertestcase.py b/fail2ban/tests/clientreadertestcase.py index 012fc2bf..61101aa5 100644 --- a/fail2ban/tests/clientreadertestcase.py +++ b/fail2ban/tests/clientreadertestcase.py @@ -312,8 +312,8 @@ class JailsReaderTest(unittest.TestCase): jailfd = open(os.path.join(basedir, "jail.conf"), 'w') jailfd.write(""" [testjail1] -action = testaction1[name=test1] - testaction1[name=test2] +action = testaction1[actname=test1] + testaction1[actname=test2] filter = testfilter1 """) jailfd.close() From 6d2ff47e712e2f9c621a65de0f88eb3c5590e5da Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Sun, 28 Apr 2013 13:23:06 +0100 Subject: [PATCH 5/8] NF: Allow return of list of actions from jail via fail2ban-client --- fail2ban/client/beautifier.py | 6 ++++++ fail2ban/protocol.py | 1 + fail2ban/server/actions.py | 8 ++++++++ fail2ban/server/server.py | 3 +++ fail2ban/server/transmitter.py | 2 ++ fail2ban/tests/servertestcase.py | 6 +++++- 6 files changed, 25 insertions(+), 1 deletion(-) diff --git a/fail2ban/client/beautifier.py b/fail2ban/client/beautifier.py index a0ff8ff3..a92e9520 100644 --- a/fail2ban/client/beautifier.py +++ b/fail2ban/client/beautifier.py @@ -132,6 +132,12 @@ class Beautifier: msg = msg + "|- [" + str(c) + "]: " + ip + "\n" c += 1 msg = msg + "`- [" + str(c) + "]: " + response[len(response)-1] + elif inC[2] == "actions": + if len(response) == 0: + msg = "No actions for jail %s" % inC[1] + else: + msg = "The jail %s has the following actions:\n" % inC[1] + msg += ", ".join(action.getName() for action in response) except Exception: logSys.warning("Beautifier error. Please report the error") logSys.error("Beautify " + `response` + " with " + `self.__inputCmd` + diff --git a/fail2ban/protocol.py b/fail2ban/protocol.py index cacde4c2..334e7908 100644 --- a/fail2ban/protocol.py +++ b/fail2ban/protocol.py @@ -90,6 +90,7 @@ protocol = [ ["get maxretry", "gets the number of failures allowed for "], ["get maxlines", "gets the number of lines to buffer for "], ["get addaction", "gets the last action which has been added for "], +["get actions", "gets a list of actions for "], ["get actionstart ", "gets the start command for the action for "], ["get actionstop ", "gets the stop command for the action for "], ["get actioncheck ", "gets the check command for the action for "], diff --git a/fail2ban/server/actions.py b/fail2ban/server/actions.py index b7e58975..c0e6899d 100644 --- a/fail2ban/server/actions.py +++ b/fail2ban/server/actions.py @@ -104,6 +104,14 @@ class Actions(JailThread): self.__actions.append(action) return action + ## + # Returns the list of actions + # + # @return list of actions + + def getActions(self): + return self.__actions + ## # Set the ban time. # diff --git a/fail2ban/server/server.py b/fail2ban/server/server.py index 632d0546..092867bf 100644 --- a/fail2ban/server/server.py +++ b/fail2ban/server/server.py @@ -236,6 +236,9 @@ class Server: def getLastAction(self, name): return self.__jails.getAction(name).getLastAction() + def getActions(self, name): + return self.__jails.getAction(name).getActions() + def delAction(self, name, value): self.__jails.getAction(name).delAction(value) diff --git a/fail2ban/server/transmitter.py b/fail2ban/server/transmitter.py index 2a4514da..b293217e 100644 --- a/fail2ban/server/transmitter.py +++ b/fail2ban/server/transmitter.py @@ -265,6 +265,8 @@ class Transmitter: # Action elif command[1] == "bantime": return self.__server.getBanTime(name) + elif command[1] == "actions": + return self.__server.getActions(name) elif command[1] == "addaction": return self.__server.getLastAction(name).getName() elif command[1] == "actionstart": diff --git a/fail2ban/tests/servertestcase.py b/fail2ban/tests/servertestcase.py index 385b83d4..67c7e6ae 100644 --- a/fail2ban/tests/servertestcase.py +++ b/fail2ban/tests/servertestcase.py @@ -440,8 +440,12 @@ class Transmitter(TransmitterBase): self.transm.proceed(["set", self.jailName, "addaction", action]), (0, action)) self.assertEqual( - self.transm.proceed(["get", self.jailName, "addaction", action]), + self.transm.proceed(["get", self.jailName, "addaction"]), (0, action)) + self.assertEqual( + self.transm.proceed( + ["get", self.jailName, "actions"])[1][0].getName(), + action) for cmd, value in zip(cmdList, cmdValueList): self.assertEqual( self.transm.proceed( From 219860ed8e57c022d0d911e589ef40c2c0f3b0f8 Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Sun, 28 Apr 2013 13:23:57 +0100 Subject: [PATCH 6/8] BF: Raise ValueError for adding of duplicate named action --- fail2ban/server/actions.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fail2ban/server/actions.py b/fail2ban/server/actions.py index c0e6899d..0f7e6b72 100644 --- a/fail2ban/server/actions.py +++ b/fail2ban/server/actions.py @@ -65,6 +65,9 @@ class Actions(JailThread): # @param name The action name def addAction(self, name): + # Check is action name already exists + if name in [action.getName() for action in self.__actions]: + raise ValueError("Action %s already exists" % name) action = Action(name) self.__actions.append(action) From f196709be1d2390461fe85c6af1dee6ac6704ebf Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Mon, 29 Apr 2013 23:40:18 +0100 Subject: [PATCH 7/8] ENH: Update asterisk example jail.conf entry for multiaction --- config/jail.conf | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/config/jail.conf b/config/jail.conf index c6eea0ef..1e663a22 100644 --- a/config/jail.conf +++ b/config/jail.conf @@ -457,23 +457,15 @@ ignoreip = 168.192.0.1 # Miscelaneous # -# Multiple jails, 1 per protocol, are necessary ATM: -# see https://github.com/fail2ban/fail2ban/issues/37 -[asterisk-tcp] +[asterisk] -filter = asterisk port = 5060,5061 -protocol = tcp -logpath = /var/log/asterisk/messages -maxretry = 10 - -[asterisk-udp] - -filter = asterisk -port = 5060,5061 -protocol = udp logpath = /var/log/asterisk/messages maxretry = 10 +# Astrix requires both tcp and udp +action = %(banaction)s[name=%(__name__)s-tcp, port="%(port)s", protocol="tcp", chain="%(chain)s", actname=%(banaction)s-tcp] + %(banaction)s[name=%(__name__)s-udp, port="%(port)s", protocol="udp", chain="%(chain)s", actname=%(banaction)s-udp] + %(mta)s-whois[name=%(__name__)s, dest="%(destemail)s"] # To log wrong MySQL access attempts add to /etc/my.cnf: # log-error=/var/log/mysqld.log From aab9df9f909ef37d0623d39b8d5d038dc5bbde4d Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Mon, 29 Apr 2013 23:41:10 +0100 Subject: [PATCH 8/8] DOC: Document use of multiple actions with `actname` in jail.conf man --- man/jail.conf.5 | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/man/jail.conf.5 b/man/jail.conf.5 index d571dc7b..3f9c0884 100644 --- a/man/jail.conf.5 +++ b/man/jail.conf.5 @@ -59,7 +59,15 @@ The following options are applicable to all jails. Their meaning is described in \fBbackend\fR .TP \fBusedns\fR - +.PP +Each jail can be configured with only a single filter, but may have multiple actions. By default, the name of a action is the action filename. In the case where multiple of the same action are to be used, the \fBactname\fR option can be assigned to the action to avoid duplicatione.g.: +.PP +.nf +[ssh-iptables-ipset] +enabled = true +action = sendmail[name=ssh, dest=john@example.com, actname=mail-john] + sendmail[name=ssh, dest=paul@example.com, actname=mail-paul] +.fi .SH "ACTION FILES" Action files specify which commands are executed to ban and unban an IP address. They are located under \fI/etc/fail2ban/action.d\fR.