[apparmor] [patch] make set_profile_flags more strict
Steve Beattie
steve at nxnw.org
Fri Apr 3 07:50:57 UTC 2015
On Sun, Mar 15, 2015 at 11:55:35PM +0100, Christian Boltz wrote:
> Hello,
>
> this patch makes set_profile_flags more strict:
> - raise AppArmorBug if newflags contains only whitespace
> - raise AppArmorBug if the file doesn't contain the specified profile or
> no profile at all
>
> The tests are adjusted to expect AppArmorBug instead of a silent
> failure. Also, some tests are added for profile=None, which means to
> change the flags for all profiles in a file.
> - test_set_flags_08 is now test_set_flags_invalid_04
> - test_set_flags_invalid_03 is changed to only contain one reason for a
> failure, not two ;-)
>
>
> [ 23-more-strict-set_profile_flags.diff ]
Acked-by: Steve Beattie <steve at nxnw.org>.
> === modified file utils/apparmor/aa.py
> --- utils/apparmor/aa.py 2015-03-15 23:46:02.005030993 +0100
> +++ utils/apparmor/aa.py 2015-03-15 23:17:33.917992932 +0100
> @@ -1,6 +1,6 @@
> # ----------------------------------------------------------------------
> # Copyright (C) 2013 Kshitij Gupta <kgupta8592 at gmail.com>
> -# Copyright (C) 2014 Christian Boltz <apparmor at cboltz.de>
> +# Copyright (C) 2014-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
> @@ -31,7 +31,7 @@
>
> from copy import deepcopy
>
> -from apparmor.common import (AppArmorException, open_file_read, valid_path, hasher,
> +from apparmor.common import (AppArmorException, AppArmorBug, open_file_read, valid_path, hasher,
> open_file_write, convert_regexp, DebugLogger)
>
> import apparmor.ui as aaui
> @@ -653,6 +653,12 @@
> # so that code calling this function can make sure to only report success if there was a match
> # TODO: use RE_PROFILE_HAT_DEF for matching the hat (regex_hat_flag is totally broken!)
> #regex_hat_flag = re.compile('^([a-z]*)\s+([A-Z]*)\s*(#.*)?$')
> +
> + found = False
> +
> + if newflags and newflags.strip() == '':
> + raise AppArmorBug('New flags for %s contain only whitespace' % prof_filename)
> +
> with open_file_read(prof_filename) as f_in:
> temp_file = tempfile.NamedTemporaryFile('w', prefix=prof_filename, suffix='~', delete=False, dir=profile_dir)
> shutil.copymode(prof_filename, temp_file.name)
> @@ -664,6 +670,7 @@
> profile = matches['profile']
>
> if profile == program or program is None:
> + found = True
> header_data = {
> 'attachment': matches['attachment'] or '',
> 'flags': newflags,
> @@ -683,6 +690,12 @@
> f_out.write(line)
> os.rename(temp_file.name, prof_filename)
>
> + if not found:
> + if program is None:
> + raise AppArmorBug("%(file)s doesn't contain a valid profile (syntax error?)" % {'file': prof_filename})
> + else:
> + raise AppArmorBug("%(file)s doesn't contain a valid profile for %(profile)s (syntax error?)" % {'file': prof_filename, 'profile': program})
> +
> def profile_exists(program):
> """Returns True if profile exists, False otherwise"""
> # Check cache of profiles
> === modified file utils/test/test-aa.py
> --- utils/test/test-aa.py 2015-03-15 23:46:02.008030815 +0100
> +++ utils/test/test-aa.py 2015-03-15 23:49:00.282475772 +0100
> @@ -1,7 +1,7 @@
> #! /usr/bin/env python
> # ------------------------------------------------------------------
> #
> -# Copyright (C) 2014 Christian Boltz
> +# Copyright (C) 2014-2015 Christian Boltz
> #
> # This program is free software; you can redistribute it and/or
> # modify it under the terms of version 2 of the GNU General Public
> @@ -154,7 +154,9 @@
> self._test_set_flags('profile xy /foo', 'flags=(complain)', 'complain', profile_name='xy')
> def test_set_flags_nochange_10(self):
> self._test_set_flags('profile "/foo bar"', 'flags=(complain)', 'complain', profile_name='/foo bar')
> + def test_set_flags_nochange_11(self):
> + self._test_set_flags('/foo', '(complain)', 'complain', profile_name=None)
> - #def test_set_flags_nochange_11(self):
> + #def test_set_flags_nochange_12(self):
> # XXX changes the flags for the child profile (which happens to have the same profile name) to 'complain'
> # self._test_set_flags('/foo', 'flags=(complain)', 'complain', more_rules=' profile /foo {\n}')
>
> @@ -172,9 +174,7 @@
> def test_set_flags_07(self):
> self._test_set_flags('/foo', 'flags=(complain, audit)', '', expected_flags=None)
> def test_set_flags_08(self):
> - # XXX this creates an invalid profile with "flags=( )"
> - # should raise an exception instead
> - self._test_set_flags('/foo', 'flags=(complain, audit)', ' ')
> + self._test_set_flags('/foo', '( complain )', 'audit ', whitespace=' ', profile_name=None)
> def test_set_flags_09(self):
> self._test_set_flags('profile /foo', 'flags=(complain)', 'audit')
> def test_set_flags_10(self):
> @@ -199,15 +199,30 @@
> self._test_set_flags('/foo', 'flags=()', None, check_new_flags=False)
> def test_set_flags_invalid_03(self):
> with self.assertRaises(AppArmorException):
> - self._test_set_flags('/foo', '( )', ' ', check_new_flags=False)
> + self._test_set_flags('/foo', '( )', '', check_new_flags=False)
> + def test_set_flags_invalid_04(self):
> + with self.assertRaises(AppArmorBug):
> + self._test_set_flags('/foo', 'flags=(complain, audit)', ' ', check_new_flags=False) # whitespace-only newflags
>
> def test_set_flags_other_profile(self):
> # test behaviour if the file doesn't contain the specified /foo profile
> orig_prof = '/no-such-profile flags=(complain) {\n}'
> self.file = write_file(self.tmpdir, 'profile', orig_prof)
>
> - # XXX this silently fails - should it raise an exception instead if it doesn't find the requested profile in the file?
> - set_profile_flags(self.file, '/foo', 'audit')
> + with self.assertRaises(AppArmorBug):
> + set_profile_flags(self.file, '/foo', 'audit')
> +
> + # the file should not be changed
> + real_new_prof = read_file(self.file)
> + self.assertEqual(orig_prof, real_new_prof)
> +
> + def test_set_flags_no_profile_found(self):
> + # test behaviour if the file doesn't contain any profile
> + orig_prof = '# /comment flags=(complain) {\n# }'
> + self.file = write_file(self.tmpdir, 'profile', orig_prof)
> +
> + with self.assertRaises(AppArmorBug):
> + set_profile_flags(self.file, None, 'audit')
>
> # the file should not be changed
> real_new_prof = read_file(self.file)
--
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/20150403/5283d295/attachment-0001.pgp>
More information about the AppArmor
mailing list