[apparmor] [patch] add tests for aamode.py

[Steve Beattie]

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 <steve at nxnw.org>
---
 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
<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/20141127/02da0c89/attachment.pgp>