On Thu, Nov 27, 2014 at 02:06:11PM +0100, Christian Boltz wrote:
> With the old code:
>
> # python3
> Python 3.4.1 (default, May 23 2014, 17:48:28) [GCC] on linux
> Type "help", "copyright", "credits" or "license" for more information.
> >>> from apparmor.aamode import split_log_mode
> >>> split_log_mode('r::w::r')
> ('r', 'w::r')
>
> With Peter's patch:
>
> # python3
> Python 3.4.1 (default, May 23 2014, 17:48:28) [GCC] on linux
> Type "help", "copyright", "credits" or "license" for more information.
> >>> from apparmor.aamode import split_log_mode
> >>> split_log_mode('r::w::r')
> Traceback (most recent call last):
> File "<stdin>", line 1, in <module>
> File "/home/cb/apparmor/HEAD-clean/utils/apparmor/aamode.py", line
> 106, in split_log_mode
> user, other = mode.split("::")
> ValueError: too many values to unpack (expected 2)
>
>
> The only function that calls split_log_mode() is str_to_mode() in
> aamode.py. str_to_mode() doesn't do any validation of the result, so
> raising an exception sounds like the better way.If we're going to raise an exception that nobody's going to check for, wuld it be better to raise AppArmorBug, as that seems more appropriate, that either there's a bug in our code or we got a dodgy log message with a weird permission mode. Plus we can attach information about why things went wrong. Patch follows: Signed-off-by: Steve Beattie <[email protected]> --- utils/apparmor/aamode.py | 9 ++++++++- utils/test/test-aamode.py | 7 +++++++ 2 files changed, 15 insertions(+), 1 deletion(-) Index: b/utils/apparmor/aamode.py =================================================================== --- a/utils/apparmor/aamode.py +++ b/utils/apparmor/aamode.py @@ -12,6 +12,7 @@ # # ---------------------------------------------------------------------- import re +from apparmor.common import AppArmorBug def AA_OTHER(mode): other = set() @@ -103,11 +104,17 @@ def split_log_mode(mode): other = '' if "::" in mode: - user, other = mode.split("::") + try: + user, other = mode.split("::") + except ValueError as e: + raise AppArmorBug("Got ValueError '%s' when splitting %s" % (str(e), mode)) else: user = mode other = mode #print ('split_logmode:', user, mode) + if ":" in user or ":" in other: + raise AppArmorBug("After splitting %s, user (%s) or other (%s) contained ':' " % (mode, user, other)) + return user, other def mode_contains(mode, subset): Index: b/utils/test/test-aamode.py =================================================================== --- a/utils/test/test-aamode.py +++ b/utils/test/test-aamode.py @@ -12,6 +12,7 @@ import unittest from apparmor.aamode import split_log_mode, sub_str_to_mode +from apparmor.common import AppArmorBug class AamodeTest_split_log_mode(unittest.TestCase): def test_split_log_mode_1(self): @@ -26,6 +27,12 @@ class AamodeTest_split_log_mode(unittest self.assertEqual(split_log_mode('r::w'), ('r', 'w')) def test_split_log_mode_6(self): self.assertEqual(split_log_mode('rw::rw'), ('rw', 'rw')) + def test_split_log_mode_invalid_1(self): + with self.assertRaises(AppArmorBug): + split_log_mode('r::w::r') + def test_split_log_mode_invalid_2(self): + with self.assertRaises(AppArmorBug): + split_log_mode('r:::r') class AamodeTest_sub_str_to_mode(unittest.TestCase): def test_sub_str_to_mode_1(self): -- Steve Beattie <[email protected]> http://NxNW.org/~steve/
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
