[apparmor] How I found several bugs in less than an hour - without even searching for them
Christian Boltz
apparmor at cboltz.de
Sun Jul 5 19:25:35 UTC 2015
Hello,
yes, that's possible, and you won't believe how easy it was! I even got
additional 5% test coverage with this simple trick! Read on to see the
most useful patch since sliced bread!
Let's see if the clickbait approach in the subject helps to get this
patch reviewed quickly ;-) - and now back to serious mode ;-)
[patch] Parse all parser simple_tests with the utils code
This patch adds a testcase that parses all tests in the parser/tst/simple_tests/
directory with parse_profile_data() to ensure that everything with valid
syntax is accepted, and that all tests marked as FAIL raise an
exception.
This already resulted in
- several patches to fix low-hanging fruits (including some bugs in the
parser simple_tests itsself)
- a list of tests that don't behave as expected. Those files get their
expected result reverted to make sure we notice any change in the
tools behaviour, especially changing to the really expected resulted.
This method also makes sure that the testcase doesn't report any of
the known failures.
- a 5% improvement in test coverage - mostly caused by nearly completely
covering parse_profile_data. BTW: there are no parser tests for
pivot_root, and some FAIL tests with various rules outside of a
profile would also be a good idea. (See aa.py parse_profile_data()
coverage to get an idea which tests are missing.)
As indicated above, the tools don't work as expected on all test
profiles - most of the failures happen on expected-to-fail tests that
pass parse_profile_data() without raising an exception. There are also
some tests failing despite valid syntax, often with rarely used syntax
like if conditions and qualifier blocks.
Nevertheless, having 1311 tests with the expected result, 181 not raised
exceptions, 21 completely unknown lines and 29 tests failing as syntax error
doesn't look that bad ;-)
[ 69-test-parser-simple-tests.diff ]
=== modified file utils/test/test-parser-simple-tests.py
--- utils/test/test-parser-simple-tests.py 2015-07-05 20:46:56.467393181 +0200
+++ utils/test/test-parser-simple-tests.py 2015-07-05 18:46:41.638636268 +0200
@@ -0,0 +1,395 @@
+#! /usr/bin/env python
+# ------------------------------------------------------------------
+#
+# Copyright (C) 2015 Christian Boltz <apparmor at cboltz.de>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of version 2 of the GNU General Public
+# License published by the Free Software Foundation.
+#
+# ------------------------------------------------------------------
+
+import unittest
+from common_test import AATest, setup_all_loops
+import apparmor.aa as apparmor
+
+import os
+from apparmor.common import open_file_read, AppArmorException
+
+# testcases that should raise an exception, but don't
+exception_not_raised = [
+ 'capability/bad_1.sd',
+ 'capability/bad_2.sd',
+ 'capability/bad_3.sd',
+ 'capability/bad_4.sd',
+ 'change_hat/bad_parsing.sd',
+ 'dbus/bad_bind_1.sd',
+ 'dbus/bad_bind_2.sd',
+ 'dbus/bad_eavesdrop_1.sd',
+ 'dbus/bad_modifier_1.sd',
+ 'dbus/bad_modifier_2.sd',
+ 'dbus/bad_modifier_3.sd',
+ 'dbus/bad_modifier_4.sd',
+ 'dbus/bad_peer_1.sd',
+ 'dbus/bad_regex_01.sd',
+ 'dbus/bad_regex_02.sd',
+ 'dbus/bad_regex_03.sd',
+ 'dbus/bad_regex_04.sd',
+ 'dbus/bad_regex_05.sd',
+ 'dbus/bad_regex_06.sd',
+ 'file/bad_append_1.sd',
+ 'file/bad_append_2.sd',
+ 'file/bad_embedded_spaces_1.sd',
+ 'file/bad_re_brace_2.sd',
+ 'file/bad_re_brace_3.sd',
+ 'file/file/bad_append_1.sd',
+ 'file/file/bad_embedded_spaces_1.sd',
+ 'file/file/owner/bad_3.sd',
+ 'file/file/owner/bad_5.sd',
+ 'file/owner/bad_3.sd',
+ 'file/owner/bad_5.sd',
+ 'mount/bad_opt_10.sd',
+ 'mount/bad_opt_11.sd',
+ 'mount/bad_opt_12.sd',
+ 'mount/bad_opt_13.sd',
+ 'mount/bad_opt_14.sd',
+ 'mount/bad_opt_15.sd',
+ 'mount/bad_opt_16.sd',
+ 'mount/bad_opt_17.sd',
+ 'mount/bad_opt_18.sd',
+ 'mount/bad_opt_19.sd',
+ 'mount/bad_opt_1.sd',
+ 'mount/bad_opt_20.sd',
+ 'mount/bad_opt_21.sd',
+ 'mount/bad_opt_22.sd',
+ 'mount/bad_opt_23.sd',
+ 'mount/bad_opt_24.sd',
+ 'mount/bad_opt_2.sd',
+ 'mount/bad_opt_3.sd',
+ 'mount/bad_opt_4.sd',
+ 'mount/bad_opt_5.sd',
+ 'mount/bad_opt_6.sd',
+ 'mount/bad_opt_7.sd',
+ 'mount/bad_opt_8.sd',
+ 'mount/bad_opt_9.sd',
+ 'profile/flags/flags_bad10.sd',
+ 'profile/flags/flags_bad11.sd',
+ 'profile/flags/flags_bad12.sd',
+ 'profile/flags/flags_bad13.sd',
+ 'profile/flags/flags_bad15.sd',
+ 'profile/flags/flags_bad18.sd',
+ 'profile/flags/flags_bad19.sd',
+ 'profile/flags/flags_bad20.sd',
+ 'profile/flags/flags_bad2.sd',
+ 'profile/flags/flags_bad3.sd',
+ 'profile/flags/flags_bad4.sd',
+ 'profile/flags/flags_bad5.sd',
+ 'profile/flags/flags_bad6.sd',
+ 'profile/flags/flags_bad7.sd',
+ 'profile/flags/flags_bad8.sd',
+ 'profile/flags/flags_bad_debug_1.sd',
+ 'profile/flags/flags_bad_debug_2.sd',
+ 'profile/flags/flags_bad_debug_3.sd',
+ 'profile/flags/flags_bad_debug_4.sd',
+ 'profile/simple_bad_no_close_brace4.sd',
+ 'ptrace/bad_01.sd',
+ 'ptrace/bad_02.sd',
+ 'ptrace/bad_03.sd',
+ 'ptrace/bad_04.sd',
+ 'ptrace/bad_05.sd',
+ 'ptrace/bad_06.sd',
+ 'ptrace/bad_07.sd',
+ 'ptrace/bad_08.sd',
+ 'ptrace/bad_10.sd',
+ 'signal/bad_01.sd',
+ 'signal/bad_02.sd',
+ 'signal/bad_03.sd',
+ 'signal/bad_04.sd',
+ 'signal/bad_05.sd',
+ 'signal/bad_06.sd',
+ 'signal/bad_07.sd',
+ 'signal/bad_08.sd',
+ 'signal/bad_09.sd',
+ 'signal/bad_10.sd',
+ 'signal/bad_11.sd',
+ 'signal/bad_12.sd',
+ 'signal/bad_13.sd',
+ 'signal/bad_14.sd',
+ 'signal/bad_15.sd',
+ 'signal/bad_16.sd',
+ 'signal/bad_17.sd',
+ 'signal/bad_18.sd',
+ 'signal/bad_19.sd',
+ 'signal/bad_20.sd',
+ 'signal/bad_21.sd',
+ 'unix/bad_attr_1.sd',
+ 'unix/bad_attr_2.sd',
+ 'unix/bad_attr_3.sd',
+ 'unix/bad_attr_4.sd',
+ 'unix/bad_bind_1.sd',
+ 'unix/bad_bind_2.sd',
+ 'unix/bad_create_1.sd',
+ 'unix/bad_create_2.sd',
+ 'unix/bad_listen_1.sd',
+ 'unix/bad_listen_2.sd',
+ 'unix/bad_modifier_1.sd',
+ 'unix/bad_modifier_2.sd',
+ 'unix/bad_modifier_3.sd',
+ 'unix/bad_modifier_4.sd',
+ 'unix/bad_opt_1.sd',
+ 'unix/bad_opt_2.sd',
+ 'unix/bad_opt_3.sd',
+ 'unix/bad_opt_4.sd',
+ 'unix/bad_peer_1.sd',
+ 'unix/bad_regex_01.sd',
+ 'unix/bad_regex_02.sd',
+ 'unix/bad_regex_03.sd',
+ 'unix/bad_regex_04.sd',
+ 'unix/bad_shutdown_1.sd',
+ 'unix/bad_shutdown_2.sd',
+ 'vars/boolean/boolean_bad_1.sd',
+ 'vars/boolean/boolean_bad_2.sd',
+ 'vars/boolean/boolean_bad_3.sd',
+ 'vars/boolean/boolean_bad_4.sd',
+ 'vars/boolean/boolean_bad_6.sd',
+ 'vars/boolean/boolean_bad_7.sd',
+ 'vars/boolean/boolean_bad_8.sd',
+ 'vars/vars_bad_1.sd',
+ 'vars/vars_bad_2.sd',
+ 'vars/vars_bad_3.sd',
+ 'vars/vars_bad_4.sd',
+ 'vars/vars_bad_5.sd',
+ 'vars/vars_bad_7.sd',
+ 'vars/vars_bad_8.sd',
+ 'vars/vars_bad_trailing_comma_1.sd',
+ 'vars/vars_bad_trailing_comma_2.sd',
+ 'vars/vars_bad_trailing_comma_3.sd',
+ 'vars/vars_bad_trailing_comma_4.sd',
+ 'vars/vars_bad_trailing_garbage_1.sd',
+ 'vars/vars_dbus_bad_01.sd',
+ 'vars/vars_dbus_bad_02.sd',
+ 'vars/vars_dbus_bad_03.sd',
+ 'vars/vars_dbus_bad_04.sd',
+ 'vars/vars_dbus_bad_05.sd',
+ 'vars/vars_dbus_bad_06.sd',
+ 'vars/vars_dbus_bad_07.sd',
+ 'vars/vars_file_evaluation_7.sd',
+ 'vars/vars_file_evaluation_8.sd',
+ 'vars/vars_recursion_1.sd',
+ 'vars/vars_recursion_2.sd',
+ 'vars/vars_recursion_3.sd',
+ 'vars/vars_recursion_4.sd',
+ 'vars/vars_simple_assignment_10.sd',
+ 'vars/vars_simple_assignment_3.sd',
+ 'vars/vars_simple_assignment_8.sd',
+ 'vars/vars_simple_assignment_9.sd',
+ 'xtrans/simple_bad_conflicting_x_10.sd',
+ 'xtrans/simple_bad_conflicting_x_11.sd',
+ 'xtrans/simple_bad_conflicting_x_12.sd',
+ 'xtrans/simple_bad_conflicting_x_13.sd',
+ 'xtrans/simple_bad_conflicting_x_14.sd',
+ 'xtrans/simple_bad_conflicting_x_15.sd',
+ 'xtrans/simple_bad_conflicting_x_1.sd',
+ 'xtrans/simple_bad_conflicting_x_2.sd',
+ 'xtrans/simple_bad_conflicting_x_3.sd',
+ 'xtrans/simple_bad_conflicting_x_4.sd',
+ 'xtrans/simple_bad_conflicting_x_5.sd',
+ 'xtrans/simple_bad_conflicting_x_6.sd',
+ 'xtrans/simple_bad_conflicting_x_7.sd',
+ 'xtrans/simple_bad_conflicting_x_8.sd',
+ 'xtrans/simple_bad_conflicting_x_9.sd',
+ 'xtrans/x-conflict.sd',
+]
+
+# testcases with lines that don't match any regex and end up as "unknown line"
+unknown_line = [
+ # 'other' keyword
+ 'file/allow/ok_other_1.sd',
+ 'file/allow/ok_other_2.sd',
+ 'file/ok_other_1.sd',
+ 'file/ok_other_2.sd',
+ 'file/ok_other_3.sd',
+
+ # permissions before path
+ 'file/file/front_perms_ok_1.sd',
+ 'file/front_perms_ok_1.sd',
+ 'profile/local/local_named_profile_ok1.sd',
+ 'profile/local/local_profile_ok1.sd',
+ 'xtrans/simple_ok_cx_1.sd',
+
+ # permissions before path and owner / audit {...} blocks
+ 'file/file/owner/ok_1.sd',
+ 'file/owner/ok_1.sd',
+ 'profile/entry_mods_audit_ok1.sd',
+
+ # namespace
+ 'profile/profile_ns_named_ok1.sd', # profile keyword?
+ 'profile/profile_ns_named_ok2.sd', # profile keyword?
+ 'profile/profile_ns_named_ok3.sd', # profile keyword?
+ 'profile/profile_ns_ok1.sd',
+ 'profile/profile_ns_ok2.sd',
+ 'profile/profile_ns_ok3.sd', # profile keyword?
+ 'profile/re_named_ok4.sd', # profile keyword
+ 'profile/re_ok4.sd',
+]
+
+# testcases with various unexpected failures
+syntax_failure = [
+ # profile keyword?
+ 'profile/re_named_ok2.sd',
+
+ # Syntax Error: Unexpected hat definition found (external hat)
+ 'change_hat/new_style4.sd',
+
+ # Syntax Errors caused by boolean conditions (parse_profile_data() gets confused by the closing '}')
+ 'conditional/defined_1.sd',
+ 'conditional/defined_2.sd',
+ 'conditional/else_1.sd',
+ 'conditional/else_2.sd',
+ 'conditional/else_3.sd',
+ 'conditional/else_if_1.sd',
+ 'conditional/else_if_2.sd',
+ 'conditional/else_if_3.sd',
+ 'conditional/else_if_5.sd',
+ 'conditional/ok_1.sd',
+ 'conditional/ok_2.sd',
+ 'conditional/ok_3.sd',
+ 'conditional/ok_4.sd',
+ 'conditional/ok_5.sd',
+ 'conditional/ok_6.sd',
+ 'conditional/ok_7.sd',
+ 'conditional/ok_8.sd',
+ 'conditional/ok_9.sd',
+ 'conditional/stress_1.sd',
+
+ # unexpected uppercase vs. lowercase in *x rules
+ 'file/ok_5.sd', # Invalid mode UX
+ 'file/ok_2.sd', # Invalid mode RWM
+ 'file/ok_4.sd', # Invalid mode iX
+ 'xtrans/simple_ok_pix_1.sd', # Invalid mode pIx
+ 'xtrans/simple_ok_pux_1.sd', # Invalid mode rPux
+
+ # misc
+ 'rlimits/test1.sd', # Ambiguous value 60m in rlimit rttime rule XXX needs testcase fix
+ 'vars/vars_simple_assignment_12.sd', # Redefining existing variable @{BAR} ('\' not handled)
+ 'rewrite/alias_good_5.sd', # Values added to a non-existing variable @{FOO} (defined in include, lp:1331856)
+]
+
+class TestParseParserTests(AATest):
+ tests = [] # filled by parse_test_profiles()
+
+ def _run_test(self, params, expected):
+ with open_file_read(params['file']) as f_in:
+ data = f_in.readlines()
+
+ if params['disabled']:
+ # skip disabled testcases
+ return
+
+ if params['tools_wrong']:
+ # if the tools are marked as being wrong about a profile, expect the opposite result
+ # this makes sure we notice any behaviour change, especially not being wrong anymore
+ expected = not expected
+
+ if expected:
+ apparmor.parse_profile_data(data, params['file'], 0)
+ else:
+ with self.assertRaises(AppArmorException):
+ apparmor.parse_profile_data(data, params['file'], 0)
+
+def parse_test_profiles(file_with_path):
+ '''parse the test-related headers of a profile (for example EXRESULT) and add the profile to the set of tests'''
+ exresult = None
+ exresult_found = False
+ description = None
+ todo = False
+ disabled = False
+
+ with open_file_read(file_with_path) as f_in:
+ data = f_in.readlines()
+
+ relfile = os.path.relpath(file_with_path, apparmor.profile_dir)
+
+ for line in data:
+ if line.startswith('#=EXRESULT '):
+ exresult = line.split()[1]
+ if exresult == 'PASS':
+ exresult == True
+ exresult_found = True
+ elif exresult == 'FAIL':
+ exresult = False
+ exresult_found = True
+ else:
+ raise Exception('%s contains unknown EXRESULT %s' % (file_with_path, exresult))
+
+ elif line.upper().startswith('#=DESCRIPTION '):
+ description = line.split()[1]
+
+ elif line.rstrip() == '#=TODO':
+ todo = True
+
+ elif line.rstrip() == '#=DISABLED':
+ disabled = True
+
+ if not exresult_found:
+ raise Exception('%s does not contain EXRESULT' % file_with_path)
+
+ if not description:
+ raise Exception('%s does not contain description' % file_with_path)
+
+ tools_wrong = False
+ if relfile in exception_not_raised and not exresult:
+ if exresult:
+ raise Exception("%s listed in exception_not_raised, but has EXRESULT PASS" % file_with_path)
+ tools_wrong = 'EXCEPTION_NOT_RAISED'
+ elif relfile in unknown_line:
+ if not exresult:
+ raise Exception("%s listed in unknown_line, but has EXRESULT FAIL" % file_with_path)
+ tools_wrong = 'UNKNOWN_LINE'
+ elif relfile in syntax_failure:
+ if not exresult:
+ raise Exception("%s listed in syntax_failure, but has EXRESULT FAIL" % file_with_path)
+ tools_wrong = 'SYNTAX_FAILURE'
+
+ params = {
+ 'file': file_with_path,
+ 'relfile': relfile,
+ 'todo': todo,
+ 'disabled': disabled,
+ 'tools_wrong': tools_wrong,
+ 'exresult': exresult,
+ }
+
+ TestParseParserTests.tests.append((params, exresult))
+
+
+def find_and_setup_test_profiles(profile_dir):
+ '''find all profiles in the given profile_dir, excluding
+ - skippable files
+ - include directories
+ - files in the main directory (readme, todo etc.)
+ '''
+ profile_dir = os.path.abspath(profile_dir)
+
+ apparmor.profile_dir = profile_dir
+
+ for root, dirs, files in os.walk(profile_dir):
+ relpath = os.path.relpath(root, profile_dir)
+
+ if relpath == '.':
+ # include files are checked as part of the profiles that include them (also, they don't contain EXRESULT)
+ dirs.remove('includes')
+ dirs.remove('include_tests')
+ dirs.remove('includes-preamble')
+
+ for file in files:
+ file_with_path = os.path.join(root, file)
+ if not apparmor.is_skippable_file(file) and relpath != '.':
+ parse_test_profiles(file_with_path)
+
+
+find_and_setup_test_profiles('../../parser/tst/simple_tests/')
+
+setup_all_loops(__name__)
+if __name__ == '__main__':
+ unittest.main(verbosity=2)
Regards,
Christian Boltz
PS: Non-random sig - I already moved my laptop to a not-so-hot [1] room
[1] I have to avoid the word "cooler" because it is definitively _not_
cool here.
--
Ich komm' mir fast vor wie in nem Kochrezept... "lassen sie Ihr Hirn
nun bei mindestens 40°C fuer > 12h im eigenen Saft koecheln..."
[David Haller in suse-linux-faq]
More information about the AppArmor
mailing list