diff --git a/client/configreader.py b/client/configreader.py index e1a2b69f..9028df48 100644 --- a/client/configreader.py +++ b/client/configreader.py @@ -122,8 +122,8 @@ class ConfigReader(SafeConfigParserWithIncludes): values[option[1]] = option[2] except NoOptionError: if not option[2] == None: - logSys.warn("'%s' not defined in '%s'. Using default value" - % (option[1], sec)) + logSys.warn("'%s' not defined in '%s'. Using default one: %r" + % (option[1], sec, option[2])) values[option[1]] = option[2] except ValueError: logSys.warn("Wrong value for '" + option[1] + "' in '" + sec + diff --git a/client/jailreader.py b/client/jailreader.py index ec73ce46..ad69bfa0 100644 --- a/client/jailreader.py +++ b/client/jailreader.py @@ -40,10 +40,11 @@ class JailReader(ConfigReader): actionCRE = re.compile("^((?:\w|-|_|\.)+)(?:\[(.*)\])?$") - def __init__(self, name, **kwargs): + def __init__(self, name, force_enable=False, **kwargs): ConfigReader.__init__(self, **kwargs) self.__name = name self.__filter = None + self.__force_enable = force_enable self.__actions = list() def setName(self, value): @@ -56,7 +57,7 @@ class JailReader(ConfigReader): return ConfigReader.read(self, "jail") def isEnabled(self): - return self.__opts["enabled"] + return self.__force_enable or self.__opts["enabled"] def getOptions(self): opts = [["bool", "enabled", "false"], @@ -87,6 +88,8 @@ class JailReader(ConfigReader): # Read action for act in self.__opts["action"].split('\n'): try: + if not act: # skip empty actions + continue splitAct = JailReader.splitAction(act) action = ActionReader(splitAct, self.__name, basedir=self.getBaseDir()) ret = action.read() @@ -97,8 +100,10 @@ class JailReader(ConfigReader): raise AttributeError("Unable to read action") except Exception, e: logSys.error("Error in action definition " + act) - logSys.debug(e) + logSys.debug("Caught exception: %s" % (e,)) return False + if not len(self.__actions): + logSys.warn("No actions were defined for %s" % self.__name) return True def convert(self): @@ -143,12 +148,20 @@ class JailReader(ConfigReader): def splitAction(action): m = JailReader.actionCRE.match(action) d = dict() - if not m.group(2) == None: + mgroups = m.groups() + if len(mgroups) == 2: + action_name, action_opts = mgroups + elif len(mgroups) == 1: + action_name, action_opts = mgroups[0], None + else: + raise ValueError("While reading action %s we should have got up to " + "2 groups. Got: %r" % (action, mgroups)) + if not action_opts is None: # Huge bad hack :( This method really sucks. TODO Reimplement it. actions = "" escapeChar = None allowComma = False - for c in m.group(2): + for c in action_opts: if c in ('"', "'") and not allowComma: # Start escapeChar = c @@ -173,6 +186,6 @@ class JailReader(ConfigReader): try: d[p[0].strip()] = p[1].strip() except IndexError: - logSys.error("Invalid argument %s in '%s'" % (p, m.group(2))) - return [m.group(1), d] + logSys.error("Invalid argument %s in '%s'" % (p, action_opts)) + return [action_name, d] splitAction = staticmethod(splitAction) diff --git a/client/jailsreader.py b/client/jailsreader.py index e1b8efa3..91e178d6 100644 --- a/client/jailsreader.py +++ b/client/jailsreader.py @@ -36,9 +36,17 @@ logSys = logging.getLogger("fail2ban.client.config") class JailsReader(ConfigReader): - def __init__(self, **kwargs): + def __init__(self, force_enable=False, **kwargs): + """ + Parameters + ---------- + force_enable : bool, optional + Passed to JailReader to force enable the jails. + It is for internal use + """ ConfigReader.__init__(self, **kwargs) self.__jails = list() + self.__force_enable = force_enable def read(self): return ConfigReader.read(self, "jail") @@ -49,7 +57,7 @@ class JailsReader(ConfigReader): if section: # Get the options of a specific jail. - jail = JailReader(section, basedir=self.getBaseDir()) + jail = JailReader(section, basedir=self.getBaseDir(), force_enable=self.__force_enable) jail.read() ret = jail.getOptions() if ret: @@ -62,7 +70,7 @@ class JailsReader(ConfigReader): else: # Get the options of all jails. for sec in self.sections(): - jail = JailReader(sec, basedir=self.getBaseDir()) + jail = JailReader(sec, basedir=self.getBaseDir(), force_enable=self.__force_enable) jail.read() ret = jail.getOptions() if ret: diff --git a/config/filter.d/postfix.conf b/config/filter.d/postfix.conf index 8db7faee..d2dc4a0c 100644 --- a/config/filter.d/postfix.conf +++ b/config/filter.d/postfix.conf @@ -15,6 +15,7 @@ # Values: TEXT # failregex = reject: RCPT from (.*)\[\]: 554 + reject: RCPT from (.*)\[\]: 450 4\.7\.1 : Helo command rejected: Host not found; from=<> to=<> proto=ESMTP helo= *$ # Option: ignoreregex # Notes.: regex to ignore. If this regex matches, the line is ignored. diff --git a/testcases/clientreadertestcase.py b/testcases/clientreadertestcase.py index d581a81f..28eb6b09 100644 --- a/testcases/clientreadertestcase.py +++ b/testcases/clientreadertestcase.py @@ -17,14 +17,8 @@ # along with Fail2Ban; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. -# Author: Cyril Jaquier -# -# $Revision$ - -__author__ = "Cyril Jaquier" -__version__ = "$Revision$" -__date__ = "$Date$" -__copyright__ = "Copyright (c) 2004 Cyril Jaquier" +__author__ = "Cyril Jaquier, Yaroslav Halchenko" +__copyright__ = "Copyright (c) 2004 Cyril Jaquier, 2011-2013 Yaroslav Halchenko" __license__ = "GPL" import os, shutil, tempfile, unittest @@ -130,3 +124,25 @@ class JailsReaderTest(unittest.TestCase): # commands to communicate to the server self.assertEqual(comm_commands, []) + def testReadStockJailConfForceEnabled(self): + # more of a smoke test to make sure that no obvious surprises + # on users' systems when enabling shipped jails + jails = JailsReader(basedir='config', force_enable=True) # we are running tests from root project dir atm + self.assertTrue(jails.read()) # opens fine + self.assertTrue(jails.getOptions()) # reads fine + comm_commands = jails.convert() + + # by default we have lots of jails ;) + self.assertTrue(len(comm_commands)) + + # and we know even some of them by heart + for j in ['ssh-iptables', 'recidive']: + # by default we have 'auto' backend ATM + self.assertTrue(['add', j, 'auto'] in comm_commands) + # and warn on useDNS + self.assertTrue(['set', j, 'usedns', 'warn'] in comm_commands) + self.assertTrue(['start', j] in comm_commands) + # last commands should be the 'start' commands + self.assertEqual(comm_commands[-1][0], 'start') + # TODO: make sure that all of the jails have actions assigned, + # otherwise it makes little to no sense diff --git a/testcases/files/logs/postfix b/testcases/files/logs/postfix new file mode 100644 index 00000000..28ff95a4 --- /dev/null +++ b/testcases/files/logs/postfix @@ -0,0 +1,3 @@ +# per https://github.com/fail2ban/fail2ban/issues/125 +# and https://github.com/fail2ban/fail2ban/issues/126 +Feb 21 09:21:54 xxx postfix/smtpd[14398]: NOQUEUE: reject: RCPT from example.com[192.0.43.10]: 450 4.7.1 : Helo command rejected: Host not found; from=<> to=<> proto=ESMTP helo=