[apparmor] [patch] make set_profile_flags more strict
Christian Boltz
apparmor at cboltz.de
Sun Mar 15 22:55:35 UTC 2015
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 ]
=== 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)
Regards,
Christian Boltz
--
Ich war da - check. Das Kino auch - check. Ich wollte eigentlich Batman
schauen, aber es lief nur "Schwarz und geräuschlos" von den Machern von
"Unsere Server sind abgestürzt" und "bis die wieder laufen dauert es
noch ein paar Stunden". Die Welt ist am Ende, Kinos stürzen ab.
[Oliver Sch at d]
More information about the AppArmor
mailing list