From f2c58e74c144b91c59b6f3fee9d48098a30f437e Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Thu, 12 Dec 2013 08:24:29 +0000 Subject: [PATCH 01/19] TST: check client.JailReader.setName --- testcases/clientreadertestcase.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/testcases/clientreadertestcase.py b/testcases/clientreadertestcase.py index 773d5072..0e3e6132 100644 --- a/testcases/clientreadertestcase.py +++ b/testcases/clientreadertestcase.py @@ -114,6 +114,8 @@ class JailReaderTest(unittest.TestCase): self.assertTrue(jail.getOptions()) self.assertFalse(jail.isEnabled()) self.assertEqual(jail.getName(), 'ssh-iptables') + jail.setName('ssh-funky-blocker') + self.assertEqual(jail.getName(), 'ssh-funky-blocker') def testSplitAction(self): action = "mail-whois[name=SSH]" From 970fd5d2891f59c7ac3943d8c50715badbd1047c Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Thu, 12 Dec 2013 08:52:01 +0000 Subject: [PATCH 02/19] BF: ensure dangling symlink error message is reachable $ ls -la /tmp/f2b-tempq0ipGY/f2 lrwxrwxrwx. 1 dan dan 11 Dec 12 08:42 /tmp/f2b-tempq0ipGY/f2 -> nonexisting In [3]: os.path.exists('/tmp/f2b-tempq0ipGY/f2') Out[3]: False In [4]: os.path.lexists('/tmp/f2b-tempq0ipGY/f2') Out[4]: True --- client/jailreader.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/client/jailreader.py b/client/jailreader.py index 7fbac423..8980431e 100644 --- a/client/jailreader.py +++ b/client/jailreader.py @@ -65,9 +65,10 @@ class JailReader(ConfigReader): pathList = [] for p in glob.glob(path): if not os.path.exists(p): - logSys.warning("File %s doesn't even exist, thus cannot be monitored" % p) - elif not os.path.lexists(p): - logSys.warning("File %s is a dangling link, thus cannot be monitored" % p) + if os.path.lexists(p): + logSys.warning("File %s is a dangling link, thus cannot be monitored" % p) + else: + logSys.warning("File %s doesn't even exist, thus cannot be monitored" % p) else: pathList.append(p) return pathList From f84a03d6b57b9a82fc42cd8e98dd71c11ddceeb8 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Thu, 12 Dec 2013 09:08:42 +0000 Subject: [PATCH 03/19] BF: remove nonreachable parts of code Glob ensures the file exists so only a check that a missing dangling symlink needs to be done. $ ls -la /tmp/f2b-tempq0ipGY/f2 lrwxrwxrwx. 1 dan dan 11 Dec 12 08:42 /tmp/f2b-tempq0ipGY/f2 -> xisting In [3]: os.path.exists('/tmp/f2b-tempq0ipGY/f2') Out[3]: False In [4]: os.path.lexists('/tmp/f2b-tempq0ipGY/f2') Out[4]: True --- client/jailreader.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/client/jailreader.py b/client/jailreader.py index 8980431e..1c651fe9 100644 --- a/client/jailreader.py +++ b/client/jailreader.py @@ -64,13 +64,10 @@ class JailReader(ConfigReader): """ pathList = [] for p in glob.glob(path): - if not os.path.exists(p): - if os.path.lexists(p): - logSys.warning("File %s is a dangling link, thus cannot be monitored" % p) - else: - logSys.warning("File %s doesn't even exist, thus cannot be monitored" % p) - else: + if os.path.exists(p): pathList.append(p) + else: + logSys.warning("File %s is a dangling link, thus cannot be monitored" % p) return pathList def getOptions(self): From cb4f1e51422bd5976f1598995996ccaa4cf29cc9 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Thu, 12 Dec 2013 09:10:12 +0000 Subject: [PATCH 04/19] TST: remove temp files in glob test --- testcases/clientreadertestcase.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/testcases/clientreadertestcase.py b/testcases/clientreadertestcase.py index 0e3e6132..7f85dbd9 100644 --- a/testcases/clientreadertestcase.py +++ b/testcases/clientreadertestcase.py @@ -135,6 +135,10 @@ class JailReaderTest(unittest.TestCase): self.assertEqual(JailReader._glob(os.path.join(d, '*')), [os.path.join(d, 'f1')]) # since f2 is dangling -- empty list self.assertEqual(JailReader._glob(os.path.join(d, 'f2')), []) + self.assertEqual(JailReader._glob(os.path.join(d, 'nonexisting')), []) + os.remove(os.path.join(d, 'f1')) + os.remove(os.path.join(d, 'f2')) + os.rmdir(d) class JailsReaderTest(unittest.TestCase): From 3036afca9167524e7cd54fb54fb63507f040fb12 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Thu, 12 Dec 2013 10:13:57 +0000 Subject: [PATCH 05/19] TST: check dangling link log message --- testcases/clientreadertestcase.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/testcases/clientreadertestcase.py b/testcases/clientreadertestcase.py index 7f85dbd9..0937a5a5 100644 --- a/testcases/clientreadertestcase.py +++ b/testcases/clientreadertestcase.py @@ -27,6 +27,7 @@ from client.configreader import ConfigReader from client.jailreader import JailReader from client.jailsreader import JailsReader from client.configurator import Configurator +from utils import LogCaptureTestCase class ConfigReaderTest(unittest.TestCase): @@ -106,7 +107,7 @@ option = %s self.assertEqual(self._getoption(), 1) -class JailReaderTest(unittest.TestCase): +class JailReaderTest(LogCaptureTestCase): def testStockSSHJail(self): jail = JailReader('ssh-iptables', basedir='config') # we are running tests from root project dir atm @@ -127,17 +128,21 @@ class JailReaderTest(unittest.TestCase): d = tempfile.mkdtemp(prefix="f2b-temp") # Generate few files # regular file - open(os.path.join(d, 'f1'), 'w').close() + f1 = os.path.join(d, 'f1') + open(f1, 'w').close() # dangling link - os.symlink('nonexisting', os.path.join(d, 'f2')) + + f2 = os.path.join(d, 'f2') + os.symlink('nonexisting',f2) # must be only f1 - self.assertEqual(JailReader._glob(os.path.join(d, '*')), [os.path.join(d, 'f1')]) + self.assertEqual(JailReader._glob(os.path.join(d, '*')), [f1]) # since f2 is dangling -- empty list - self.assertEqual(JailReader._glob(os.path.join(d, 'f2')), []) + self.assertEqual(JailReader._glob(f2), []) + self.assertTrue(self._is_logged('File %s is a dangling link, thus cannot be monitored' % f2)) self.assertEqual(JailReader._glob(os.path.join(d, 'nonexisting')), []) - os.remove(os.path.join(d, 'f1')) - os.remove(os.path.join(d, 'f2')) + os.remove(f1) + os.remove(f2) os.rmdir(d) class JailsReaderTest(unittest.TestCase): From b18ce122dd236776d500799475b675d52d7840b8 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Thu, 12 Dec 2013 20:07:09 +0000 Subject: [PATCH 06/19] BF/ENH: fix error when action doesn't match regex. Document unreachable code. Simplify regex --- client/jailreader.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/client/jailreader.py b/client/jailreader.py index 1c651fe9..2138acee 100644 --- a/client/jailreader.py +++ b/client/jailreader.py @@ -35,7 +35,7 @@ logSys = logging.getLogger("fail2ban.client.config") class JailReader(ConfigReader): - actionCRE = re.compile("^((?:\w|-|_|\.)+)(?:\[(.*)\])?$") + actionCRE = re.compile("^([\w_.-]+)(?:\[(.*)\])?$") def __init__(self, name, force_enable=False, **kwargs): ConfigReader.__init__(self, **kwargs) @@ -173,12 +173,16 @@ class JailReader(ConfigReader): def splitAction(action): m = JailReader.actionCRE.match(action) d = dict() - mgroups = m.groups() + try: + mgroups = m.groups() + except AttributeError: + raise ValueError("While reading action %s we should have got 1 or " + "2 groups. Got: 0" % action) if len(mgroups) == 2: action_name, action_opts = mgroups elif len(mgroups) == 1: action_name, action_opts = mgroups[0], None - else: + else: # pragma: nocover - unreachable - regex only can capture 2 groups raise ValueError("While reading action %s we should have got up to " "2 groups. Got: %r" % (action, mgroups)) if not action_opts is None: From c6d14dcf0eedef9513b3b5fb5b028ff301908734 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Thu, 12 Dec 2013 20:35:30 +0000 Subject: [PATCH 07/19] TST: complete coverage of splitAction --- client/jailreader.py | 2 +- testcases/clientreadertestcase.py | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/client/jailreader.py b/client/jailreader.py index 2138acee..131a4dd5 100644 --- a/client/jailreader.py +++ b/client/jailreader.py @@ -180,7 +180,7 @@ class JailReader(ConfigReader): "2 groups. Got: 0" % action) if len(mgroups) == 2: action_name, action_opts = mgroups - elif len(mgroups) == 1: + elif len(mgroups) == 1: # pragma: nocover - unreachable - .* on second group always matches action_name, action_opts = mgroups[0], None else: # pragma: nocover - unreachable - regex only can capture 2 groups raise ValueError("While reading action %s we should have got up to " diff --git a/testcases/clientreadertestcase.py b/testcases/clientreadertestcase.py index 0937a5a5..c22f028a 100644 --- a/testcases/clientreadertestcase.py +++ b/testcases/clientreadertestcase.py @@ -123,7 +123,19 @@ class JailReaderTest(LogCaptureTestCase): expected = ['mail-whois', {'name': 'SSH'}] result = JailReader.splitAction(action) self.assertEqual(expected, result) + + self.assertEqual(['mail.who_is', {}], JailReader.splitAction("mail.who_is")) + self.assertEqual(['mail.who_is', {'a':'cat', 'b':'dog'}], JailReader.splitAction("mail.who_is[a=cat,b=dog]")) + self.assertEqual(['mail--ho_is', {}], JailReader.splitAction("mail--ho_is")) + + self.assertEqual(['mail--ho_is', {}], JailReader.splitAction("mail--ho_is['s']")) + self.assertTrue(self._is_logged("Invalid argument ['s'] in ''s''")) + + self.assertEqual(['mail', {'a': ','}], JailReader.splitAction("mail[a=',']")) + self.assertRaises(ValueError, JailReader.splitAction ,'mail-how[') + + def testGlob(self): d = tempfile.mkdtemp(prefix="f2b-temp") # Generate few files From 3ddf8da76e6c69e0f91c9f2fdebba96f9932eff3 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Fri, 13 Dec 2013 08:45:10 +0000 Subject: [PATCH 08/19] ENH: ensure filter is defined in jail before its read --- client/jailreader.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/client/jailreader.py b/client/jailreader.py index 131a4dd5..1b5bf1ae 100644 --- a/client/jailreader.py +++ b/client/jailreader.py @@ -87,15 +87,18 @@ class JailReader(ConfigReader): if self.isEnabled(): # Read filter - self.__filter = FilterReader(self.__opts["filter"], self.__name, - basedir=self.getBaseDir()) - ret = self.__filter.read() - if ret: - self.__filter.getOptions(self.__opts) + if self.__opts["filter"]: + self.__filter = FilterReader(self.__opts["filter"], self.__name, + basedir=self.getBaseDir()) + ret = self.__filter.read() + if ret: + self.__filter.getOptions(self.__opts) + else: + logSys.error("Unable to read the filter") + return False else: - logSys.error("Unable to read the filter") - return False - + logSys.warn("No filter set for jail %s" % self.__name) + # Read action for act in self.__opts["action"].split('\n'): try: From d74dd31d2301d7e0c5479192405bdc55a70b3c62 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Fri, 13 Dec 2013 10:00:34 +0000 Subject: [PATCH 09/19] BF: corrected tests for missing jail Previously tests relied on the missing filter to trigger the conditions required for a missing jail. We now handle this explicitly. --- client/configreader.py | 2 +- client/jailreader.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/client/configreader.py b/client/configreader.py index 96aab5f3..470f029f 100644 --- a/client/configreader.py +++ b/client/configreader.py @@ -112,7 +112,7 @@ class ConfigReader(SafeConfigParserWithIncludes): except NoSectionError, e: # No "Definition" section or wrong basedir logSys.error(e) - values[option[1]] = option[2] + return False except NoOptionError: if not option[2] is None: logSys.warn("'%s' not defined in '%s'. Using default one: %r" diff --git a/client/jailreader.py b/client/jailreader.py index 1b5bf1ae..d7c7b84c 100644 --- a/client/jailreader.py +++ b/client/jailreader.py @@ -54,7 +54,7 @@ class JailReader(ConfigReader): return ConfigReader.read(self, "jail") def isEnabled(self): - return self.__force_enable or self.__opts["enabled"] + return self.__force_enable or ( self.__opts and self.__opts["enabled"] ) @staticmethod def _glob(path): @@ -84,6 +84,8 @@ class JailReader(ConfigReader): ["string", "filter", ""], ["string", "action", ""]] self.__opts = ConfigReader.getOptions(self, self.__name, opts) + if not self.__opts: + return False if self.isEnabled(): # Read filter From 1407b955e6673c025c8bd6252f7daca1251ea477 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Fri, 13 Dec 2013 10:03:51 +0000 Subject: [PATCH 10/19] TST: more client/jailreader tests --- MANIFEST | 4 ++++ testcases/clientreadertestcase.py | 19 ++++++++++++++++++- testcases/config/action.d/brokenaction.conf | 4 ++++ testcases/config/fail2ban.conf | 5 +++++ testcases/config/filter.d/simple.conf | 4 ++++ testcases/config/jail.conf | 20 ++++++++++++++++++++ 6 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 testcases/config/action.d/brokenaction.conf create mode 100644 testcases/config/fail2ban.conf create mode 100644 testcases/config/filter.d/simple.conf create mode 100644 testcases/config/jail.conf diff --git a/MANIFEST b/MANIFEST index 0e0eb327..a7fefd5c 100644 --- a/MANIFEST +++ b/MANIFEST @@ -242,3 +242,7 @@ files/fail2ban-tmpfiles.conf files/fail2ban.service files/ipmasq-ZZZzzz_fail2ban.rul files/gen_badbots +testcases/config/jail.conf +testcases/config/fail2ban.conf +testcases/config/filter.d/simple.conf +testcases/config/action.d/brokenaction.conf diff --git a/testcases/clientreadertestcase.py b/testcases/clientreadertestcase.py index c22f028a..2dec428e 100644 --- a/testcases/clientreadertestcase.py +++ b/testcases/clientreadertestcase.py @@ -109,6 +109,22 @@ option = %s class JailReaderTest(LogCaptureTestCase): + def testJailActionEmpty(self): + jail = JailReader('emptyaction', basedir=os.path.join('testcases','config')) + self.assertTrue(jail.read()) + self.assertTrue(jail.getOptions()) + self.assertTrue(jail.isEnabled()) + self.assertTrue(self._is_logged('No filter set for jail emptyaction')) + self.assertTrue(self._is_logged('No actions were defined for emptyaction')) + + def testJailActionBrokenDef(self): + jail = JailReader('brokenactiondef', basedir=os.path.join('testcases','config')) + self.assertTrue(jail.read()) + self.assertFalse(jail.getOptions()) + self.assertTrue(jail.isEnabled()) + self.assertTrue(self._is_logged('Error in action definition joho[foo')) + self.assertTrue(self._is_logged('Caught exception: While reading action joho[foo we should have got 1 or 2 groups. Got: 0')) + def testStockSSHJail(self): jail = JailReader('ssh-iptables', basedir='config') # we are running tests from root project dir atm self.assertTrue(jail.read()) @@ -157,7 +173,7 @@ class JailReaderTest(LogCaptureTestCase): os.remove(f2) os.rmdir(d) -class JailsReaderTest(unittest.TestCase): +class JailsReaderTest(LogCaptureTestCase): def testProvidingBadBasedir(self): if not os.path.exists('/XXX'): @@ -176,6 +192,7 @@ class JailsReaderTest(unittest.TestCase): # We should not "read" some bogus jail old_comm_commands = comm_commands[:] # make a copy self.assertFalse(jails.getOptions("BOGUS")) + self.assertTrue(self._is_logged("No section: 'BOGUS'")) # and there should be no side-effects self.assertEqual(jails.convert(), old_comm_commands) diff --git a/testcases/config/action.d/brokenaction.conf b/testcases/config/action.d/brokenaction.conf new file mode 100644 index 00000000..d2c8d059 --- /dev/null +++ b/testcases/config/action.d/brokenaction.conf @@ -0,0 +1,4 @@ + +[Definition] + +actioban = hit with big stick diff --git a/testcases/config/fail2ban.conf b/testcases/config/fail2ban.conf new file mode 100644 index 00000000..36984c78 --- /dev/null +++ b/testcases/config/fail2ban.conf @@ -0,0 +1,5 @@ +[Definition] + +# 3 = INFO +loglevel = 3 + diff --git a/testcases/config/filter.d/simple.conf b/testcases/config/filter.d/simple.conf new file mode 100644 index 00000000..4a2c0bb7 --- /dev/null +++ b/testcases/config/filter.d/simple.conf @@ -0,0 +1,4 @@ + +[Definition] + +failregex = diff --git a/testcases/config/jail.conf b/testcases/config/jail.conf new file mode 100644 index 00000000..419e2e2c --- /dev/null +++ b/testcases/config/jail.conf @@ -0,0 +1,20 @@ + +[DEFAULT] +filter = simple + +[emptyaction] +enabled = true +filter = +action = + +[brokenactiondef] +enabled = true +action = joho[foo + +[brokenaction] +enabled = true +action = brokenaction + +[missingaction] +enabled = true +action = thefunkychickendance From e916fcdce4cd5a0b927644c9f79e75375e8141d0 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Fri, 13 Dec 2013 10:51:38 +0000 Subject: [PATCH 11/19] TST: test case for actions and filters missing in a jail --- testcases/clientreadertestcase.py | 9 +++++++++ testcases/config/jail.conf | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/testcases/clientreadertestcase.py b/testcases/clientreadertestcase.py index 2dec428e..e85ddff8 100644 --- a/testcases/clientreadertestcase.py +++ b/testcases/clientreadertestcase.py @@ -117,6 +117,15 @@ class JailReaderTest(LogCaptureTestCase): self.assertTrue(self._is_logged('No filter set for jail emptyaction')) self.assertTrue(self._is_logged('No actions were defined for emptyaction')) + def testJailActionFilterMissing(self): + jail = JailReader('missingbitsjail', basedir=os.path.join('testcases','config')) + self.assertTrue(jail.read()) + self.assertFalse(jail.getOptions()) + self.assertTrue(jail.isEnabled()) + #print self._log.getvalue() + self.assertTrue(self._is_logged("Found no accessible config files for 'filter.d/catchallthebadies' under testcases/config")) + self.assertTrue(self._is_logged('Unable to read the filter')) + def testJailActionBrokenDef(self): jail = JailReader('brokenactiondef', basedir=os.path.join('testcases','config')) self.assertTrue(jail.read()) diff --git a/testcases/config/jail.conf b/testcases/config/jail.conf index 419e2e2c..4bdd74cf 100644 --- a/testcases/config/jail.conf +++ b/testcases/config/jail.conf @@ -15,6 +15,6 @@ action = joho[foo enabled = true action = brokenaction -[missingaction] -enabled = true +[missingbitsjail] +filter = catchallthebadies action = thefunkychickendance From f6fb737e6cece7d535e64a848fcc0b137519e51c Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Fri, 13 Dec 2013 10:55:15 +0000 Subject: [PATCH 12/19] TST: remove commented test print --- testcases/clientreadertestcase.py | 1 - 1 file changed, 1 deletion(-) diff --git a/testcases/clientreadertestcase.py b/testcases/clientreadertestcase.py index e85ddff8..96f9134f 100644 --- a/testcases/clientreadertestcase.py +++ b/testcases/clientreadertestcase.py @@ -122,7 +122,6 @@ class JailReaderTest(LogCaptureTestCase): self.assertTrue(jail.read()) self.assertFalse(jail.getOptions()) self.assertTrue(jail.isEnabled()) - #print self._log.getvalue() self.assertTrue(self._is_logged("Found no accessible config files for 'filter.d/catchallthebadies' under testcases/config")) self.assertTrue(self._is_logged('Unable to read the filter')) From 2f3648c458e675de99acac6fef0a506c49a0daac Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Fri, 13 Dec 2013 11:11:58 +0000 Subject: [PATCH 13/19] DOC: add missing jail directives --- man/jail.conf.5 | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/man/jail.conf.5 b/man/jail.conf.5 index 8ea44316..6c114c56 100644 --- a/man/jail.conf.5 +++ b/man/jail.conf.5 @@ -63,6 +63,12 @@ Comments: use '#' for comment lines and ';' (following a space) for inline comme .SH DEFAULT The following options are applicable to all jails. Their meaning is described in the default \fIjail.conf\fR file. .TP +\fBfilter\fR +.TP +\fBlogpath\fR +.TP +\fBaction\fR +.TP \fBignoreip\fR .TP \fBbantime\fR @@ -74,6 +80,10 @@ The following options are applicable to all jails. Their meaning is described in \fBbackend\fR .TP \fBusedns\fR +.TP +\fBfailregex\fR +.TP +\fBignoreregex\fR .SH "ACTION FILES" From b147270be769ff5b456e29d56d12d27dacd95480 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Fri, 13 Dec 2013 11:36:00 +0000 Subject: [PATCH 14/19] BF: allow processing with empty filter --- client/jailreader.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/client/jailreader.py b/client/jailreader.py index d7c7b84c..6f78275b 100644 --- a/client/jailreader.py +++ b/client/jailreader.py @@ -99,6 +99,7 @@ class JailReader(ConfigReader): logSys.error("Unable to read the filter") return False else: + self.__filter = None logSys.warn("No filter set for jail %s" % self.__name) # Read action @@ -168,7 +169,8 @@ class JailReader(ConfigReader): # Do not send a command if the rule is empty. if regex != '': stream.append(["set", self.__name, "addignoreregex", regex]) - stream.extend(self.__filter.convert()) + if self.__filter: + stream.extend(self.__filter.convert()) for action in self.__actions: stream.extend(action.convert()) stream.insert(0, ["add", self.__name, backend]) From 18f0e58caad68212e84f711526e3e1857ae7ae7b Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Fri, 13 Dec 2013 11:41:40 +0000 Subject: [PATCH 15/19] TST: increase coverage in jailreader --- testcases/clientreadertestcase.py | 32 +++++++++++++++++++++++++++++++ testcases/config/jail.conf | 12 ++++++++++++ 2 files changed, 44 insertions(+) diff --git a/testcases/clientreadertestcase.py b/testcases/clientreadertestcase.py index 96f9134f..f55be051 100644 --- a/testcases/clientreadertestcase.py +++ b/testcases/clientreadertestcase.py @@ -188,6 +188,38 @@ class JailsReaderTest(LogCaptureTestCase): reader = JailsReader(basedir='/XXX') self.assertRaises(ValueError, reader.read) + def testReadTestJailConf(self): + jails = JailsReader(basedir=os.path.join('testcases','config')) + self.assertTrue(jails.read()) # opens fine + self.assertFalse(jails.getOptions()) # reads not sof ine + self.assertRaises(ValueError, jails.convert) + comm_commands = jails.convert(allow_no_files=True) + self.maxDiff = None + self.assertEqual(comm_commands, + [['add', 'emptyaction', 'auto'], + ['set', 'emptyaction', 'usedns', 'warn'], + ['set', 'emptyaction', 'addlogpath', '/var/log/messages'], + ['set', 'emptyaction', 'maxretry', 3], + ['set', 'emptyaction', 'findtime', 600], + ['set', 'emptyaction', 'bantime', 600], + ['add', 'special', 'auto'], + ['set', 'special', 'usedns', 'warn'], + ['set', 'special', 'addlogpath', '/var/log/messages'], + ['set', 'special', 'maxretry', 3], + ['set', 'special', 'addfailregex', ''], + ['set', 'special', 'findtime', 600], + ['set', 'special', 'bantime', 600], + ['add', 'missinglogfiles', 'auto'], + ['set', 'missinglogfiles', 'usedns', 'warn'], + ['set', 'missinglogfiles', 'maxretry', 3], + ['set', 'missinglogfiles', 'findtime', 600], + ['set', 'missinglogfiles', 'bantime', 600], + ['set', 'missinglogfiles', 'addfailregex', ''], + ['start', 'emptyaction'], + ['start', 'special'], + ['start', 'missinglogfiles']]) + + def testReadStockJailConf(self): jails = JailsReader(basedir='config') # we are running tests from root project dir atm self.assertTrue(jails.read()) # opens fine diff --git a/testcases/config/jail.conf b/testcases/config/jail.conf index 4bdd74cf..ab791451 100644 --- a/testcases/config/jail.conf +++ b/testcases/config/jail.conf @@ -7,6 +7,14 @@ enabled = true filter = action = +[special] +failregex = +ignoreregex = +ignoreip = + +[missinglogfiles] +logpath = /weapons/of/mass/destruction + [brokenactiondef] enabled = true action = joho[foo @@ -18,3 +26,7 @@ action = brokenaction [missingbitsjail] filter = catchallthebadies action = thefunkychickendance + +[parse_to_end_of_jail.conf] +enabled = true +action = From b39729a2ab1abea9835fb4f5377c60d56308df9b Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Sat, 14 Dec 2013 06:51:36 +0000 Subject: [PATCH 16/19] BF: fix unintential typo --- testcases/config/action.d/brokenaction.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testcases/config/action.d/brokenaction.conf b/testcases/config/action.d/brokenaction.conf index d2c8d059..59e97b7f 100644 --- a/testcases/config/action.d/brokenaction.conf +++ b/testcases/config/action.d/brokenaction.conf @@ -1,4 +1,4 @@ [Definition] -actioban = hit with big stick +actionban = hit with big stick From 603095bc16ff08798cf16b789bc0360aa62ed112 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Sat, 14 Dec 2013 07:00:41 +0000 Subject: [PATCH 17/19] BF: errors in a jail prevents further sections from being parsed. Closes #485 --- client/jailsreader.py | 5 +++-- testcases/clientreadertestcase.py | 30 +++++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/client/jailsreader.py b/client/jailsreader.py index 00c63e3c..d32e6561 100644 --- a/client/jailsreader.py +++ b/client/jailsreader.py @@ -60,6 +60,7 @@ class JailsReader(ConfigReader): sections = [ section ] # Get the options of all jails. + parse_status = True for sec in sections: jail = JailReader(sec, basedir=self.getBaseDir(), force_enable=self.__force_enable) @@ -71,8 +72,8 @@ class JailsReader(ConfigReader): self.__jails.append(jail) else: logSys.error("Errors in jail %r. Skipping..." % sec) - return False - return True + parse_status = False + return parse_status def convert(self, allow_no_files=False): """Convert read before __opts and jails to the commands stream diff --git a/testcases/clientreadertestcase.py b/testcases/clientreadertestcase.py index f55be051..91689836 100644 --- a/testcases/clientreadertestcase.py +++ b/testcases/clientreadertestcase.py @@ -215,9 +215,37 @@ class JailsReaderTest(LogCaptureTestCase): ['set', 'missinglogfiles', 'findtime', 600], ['set', 'missinglogfiles', 'bantime', 600], ['set', 'missinglogfiles', 'addfailregex', ''], + ['add', 'brokenaction', 'auto'], + ['set', 'brokenaction', 'usedns', 'warn'], + ['set', 'brokenaction', 'addlogpath', '/var/log/messages'], + ['set', 'brokenaction', 'maxretry', 3], + ['set', 'brokenaction', 'findtime', 600], + ['set', 'brokenaction', 'bantime', 600], + ['set', 'brokenaction', 'addfailregex', ''], + ['set', 'brokenaction', 'addaction', 'brokenaction'], + ['set', + 'brokenaction', + 'actionban', + 'brokenaction', + 'hit with big stick '], + ['set', 'brokenaction', 'actionstop', 'brokenaction', ''], + ['set', 'brokenaction', 'actionstart', 'brokenaction', ''], + ['set', 'brokenaction', 'actionunban', 'brokenaction', ''], + ['set', 'brokenaction', 'actioncheck', 'brokenaction', ''], + ['add', 'parse_to_end_of_jail.conf', 'auto'], + ['set', 'parse_to_end_of_jail.conf', 'usedns', 'warn'], + ['set', 'parse_to_end_of_jail.conf', 'addlogpath', '/var/log/messages'], + ['set', 'parse_to_end_of_jail.conf', 'maxretry', 3], + ['set', 'parse_to_end_of_jail.conf', 'findtime', 600], + ['set', 'parse_to_end_of_jail.conf', 'bantime', 600], + ['set', 'parse_to_end_of_jail.conf', 'addfailregex', ''], ['start', 'emptyaction'], ['start', 'special'], - ['start', 'missinglogfiles']]) + ['start', 'missinglogfiles'], + ['start', 'brokenaction'], + ['start', 'parse_to_end_of_jail.conf'],]) + self.assertTrue(self._is_logged("Errors in jail 'missingbitsjail'. Skipping...")) + self.assertTrue(self._is_logged("No file(s) found for glob /weapons/of/mass/destruction")) def testReadStockJailConf(self): From 5c26bcbd2b24328a4fc85be0360090ee787bf6d9 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Mon, 16 Dec 2013 10:07:41 +0000 Subject: [PATCH 18/19] TST: hopefully normalise config so that consistent test results occur on travis and locally --- testcases/clientreadertestcase.py | 4 ---- testcases/config/jail.conf | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/testcases/clientreadertestcase.py b/testcases/clientreadertestcase.py index 91689836..42c3fa28 100644 --- a/testcases/clientreadertestcase.py +++ b/testcases/clientreadertestcase.py @@ -198,13 +198,11 @@ class JailsReaderTest(LogCaptureTestCase): self.assertEqual(comm_commands, [['add', 'emptyaction', 'auto'], ['set', 'emptyaction', 'usedns', 'warn'], - ['set', 'emptyaction', 'addlogpath', '/var/log/messages'], ['set', 'emptyaction', 'maxretry', 3], ['set', 'emptyaction', 'findtime', 600], ['set', 'emptyaction', 'bantime', 600], ['add', 'special', 'auto'], ['set', 'special', 'usedns', 'warn'], - ['set', 'special', 'addlogpath', '/var/log/messages'], ['set', 'special', 'maxretry', 3], ['set', 'special', 'addfailregex', ''], ['set', 'special', 'findtime', 600], @@ -217,7 +215,6 @@ class JailsReaderTest(LogCaptureTestCase): ['set', 'missinglogfiles', 'addfailregex', ''], ['add', 'brokenaction', 'auto'], ['set', 'brokenaction', 'usedns', 'warn'], - ['set', 'brokenaction', 'addlogpath', '/var/log/messages'], ['set', 'brokenaction', 'maxretry', 3], ['set', 'brokenaction', 'findtime', 600], ['set', 'brokenaction', 'bantime', 600], @@ -234,7 +231,6 @@ class JailsReaderTest(LogCaptureTestCase): ['set', 'brokenaction', 'actioncheck', 'brokenaction', ''], ['add', 'parse_to_end_of_jail.conf', 'auto'], ['set', 'parse_to_end_of_jail.conf', 'usedns', 'warn'], - ['set', 'parse_to_end_of_jail.conf', 'addlogpath', '/var/log/messages'], ['set', 'parse_to_end_of_jail.conf', 'maxretry', 3], ['set', 'parse_to_end_of_jail.conf', 'findtime', 600], ['set', 'parse_to_end_of_jail.conf', 'bantime', 600], diff --git a/testcases/config/jail.conf b/testcases/config/jail.conf index ab791451..525308e3 100644 --- a/testcases/config/jail.conf +++ b/testcases/config/jail.conf @@ -1,6 +1,7 @@ [DEFAULT] filter = simple +logpath = /non/exist [emptyaction] enabled = true From 729929ada98f63558ae8cd61586868173bc234ef Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Mon, 16 Dec 2013 10:21:46 +0000 Subject: [PATCH 19/19] TST: jails can occur in any order once parsed. Sort results to facilitate comparison --- testcases/clientreadertestcase.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/testcases/clientreadertestcase.py b/testcases/clientreadertestcase.py index 42c3fa28..28387703 100644 --- a/testcases/clientreadertestcase.py +++ b/testcases/clientreadertestcase.py @@ -190,13 +190,13 @@ class JailsReaderTest(LogCaptureTestCase): def testReadTestJailConf(self): jails = JailsReader(basedir=os.path.join('testcases','config')) - self.assertTrue(jails.read()) # opens fine - self.assertFalse(jails.getOptions()) # reads not sof ine + self.assertTrue(jails.read()) + self.assertFalse(jails.getOptions()) self.assertRaises(ValueError, jails.convert) comm_commands = jails.convert(allow_no_files=True) self.maxDiff = None - self.assertEqual(comm_commands, - [['add', 'emptyaction', 'auto'], + self.assertEqual(sorted(comm_commands), + sorted([['add', 'emptyaction', 'auto'], ['set', 'emptyaction', 'usedns', 'warn'], ['set', 'emptyaction', 'maxretry', 3], ['set', 'emptyaction', 'findtime', 600], @@ -239,7 +239,7 @@ class JailsReaderTest(LogCaptureTestCase): ['start', 'special'], ['start', 'missinglogfiles'], ['start', 'brokenaction'], - ['start', 'parse_to_end_of_jail.conf'],]) + ['start', 'parse_to_end_of_jail.conf'],])) self.assertTrue(self._is_logged("Errors in jail 'missingbitsjail'. Skipping...")) self.assertTrue(self._is_logged("No file(s) found for glob /weapons/of/mass/destruction"))