[apparmor] How I found several bugs in less than an hour - without even searching for them
Steve Beattie
steve at nxnw.org
Wed Jul 8 17:51:13 UTC 2015
On Sun, Jul 05, 2015 at 09:25:35PM +0200, Christian Boltz wrote:
> 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 ;-)
Want more bugs to fix? Try running 'make check -C ../parser' before
running 'make check' in the utils directory to get other 70,000 parser
sanity tests to run against. It goes a bit poorly:
Ran 71405 tests in 62.301s
FAILED (failures=11733, errors=4117)
That said, I do like this, once I realized that the listed profiles
were just the exceptions and that the set of test files is computed
from the contents of the parser's directory. I'm just not ready to
have it be part of 'make check' yet, until we can address the above
in some meaningful way.
> [ 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]
>
>
> --
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
--
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150708/2c95f12a/attachment-0001.pgp>
More information about the AppArmor
mailing list