Make BaseRule a proper abstract base class

utils/apparmor/rule/__init__.py

@@ -13,7 +13,7 @@
 #
 # ----------------------------------------------------------------------
 
-from abc import abstractmethod
+from abc import ABCMeta, abstractmethod
 
 from apparmor.aare import AARE
 from apparmor.common import AppArmorBug
@@ -22,7 +22,7 @@ from apparmor.translations import init_translation
 _ = init_translation()
 
 
-class BaseRule:
+class BaseRule(metaclass=ABCMeta):
     """Base class to handle and store a single rule"""
 
     # type specific rules should inherit from this class.
@@ -89,12 +89,7 @@ class BaseRule:
                 % {'partname': partname, 'classname': self.__class__.__name__, 'rulepart': str(rulepart)})
 
     def __repr__(self):
-        classname = self.__class__.__name__
+        return '<%s> %s' % (self.__class__.__name__, self.get_raw())
-        try:
-            raw_content = self.get_raw()  # will fail for BaseRule
-            return '<%s> %s' % (classname, raw_content)
-        except NotImplementedError:
-            return '<%s (NotImplementedError - get_clean() not implemented?)>' % classname
 
     @classmethod
     def match(cls, raw_rule):
@@ -282,18 +277,16 @@ class BaseRule:
            returns {'label1': 'value1', 'label2': 'value2'}"""
         raise NotImplementedError("'%s' needs to implement logprof_header(), but didn't" % (str(self)))
 
-    @abstractmethod
+    # NOTE: edit_header, validate_edit, and store_edit are not implemented by every subclass.
     def edit_header(self):
         """return the prompt for, and the path to edit when using '(N)ew'"""
         raise NotImplementedError("'%s' needs to implement edit_header(), but didn't" % (str(self)))
 
-    @abstractmethod
     def validate_edit(self, newpath):
         """validate the new path.
            Returns True if it covers the previous path, False if it doesn't."""
         raise NotImplementedError("'%s' needs to implement validate_edit(), but didn't" % (str(self)))
 
-    @abstractmethod
     def store_edit(self, newpath):
         """store the changed path.
            This is done even if the new path doesn't match the original one."""

utils/test/test-abi.py

@@ -18,8 +18,6 @@ from collections import namedtuple
 from common_test import AATest, setup_all_loops
 
 from apparmor.common import AppArmorBug, AppArmorException
-# from apparmor.logparser import ReadLog
-# from apparmor.rule import BaseRule
 from apparmor.rule.abi import AbiRule, AbiRuleset
 from apparmor.translations import init_translation
 
@@ -261,18 +259,22 @@ class AbiCoveredTest_02(AbiCoveredTest):
 #             obj.is_covered(testobj)
 #
 #     def test_invalid_is_covered(self):
-#         obj = AbiRule.create_instance('abi send,')
+#         raw_rule = 'abi send,'
-#
+#         class SomeOtherClass(AbiRule):
-#         testobj = BaseRule()  # different type
+#             pass
 #
+#         obj = AbiRule.create_instance(raw_rule)
+#         testobj = SomeOtherClass.create_instance(raw_rule)  # different type
 #         with self.assertRaises(AppArmorBug):
 #             obj.is_covered(testobj)
 #
 #     def test_invalid_is_equal(self):
-#         obj = AbiRule.create_instance('abi send,')
+#         raw_rule = 'abi send,'
-#
+#         class SomeOtherClass(AbiRule):
-#         testobj = BaseRule()  # different type
+#             pass
 #
+#         obj = AbiRule.create_instance(raw_rule)
+#         testobj = SomeOtherClass.create_instance(raw_rule)  # different type
 #         with self.assertRaises(AppArmorBug):
 #             obj.is_equal(testobj)
 

utils/test/test-alias.py

@@ -17,7 +17,6 @@ import unittest
 from collections import namedtuple
 
 from apparmor.common import AppArmorBug, AppArmorException
-from apparmor.rule import BaseRule
 from apparmor.rule.alias import AliasRule, AliasRuleset
 from apparmor.translations import init_translation
 from common_test import AATest, setup_all_loops
@@ -218,18 +217,22 @@ class AliasCoveredTest_Invalid(AATest):
     #         obj.is_covered(testobj)
 
     def test_invalid_is_covered_3(self):
-        obj = AliasRule.create_instance('alias /foo -> /bar,')
+        raw_rule = 'alias /foo -> /bar,'
-
+        class SomeOtherClass(AliasRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = AliasRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_covered(testobj)
 
     def test_invalid_is_equal(self):
-        obj = AliasRule.create_instance('alias /foo -> /bar,')
+        raw_rule = 'alias /foo -> /bar,'
-
+        class SomeOtherClass(AliasRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = AliasRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_equal(testobj)
 

utils/test/test-baserule.py

@@ -19,6 +19,35 @@ from common_test import AATest, setup_all_loops
 
 
 class TestBaserule(AATest):
+
+    class ValidSubclass(BaseRule):
+        @classmethod
+        def _match(cls, raw_rule): pass
+
+        @classmethod
+        def _create_instance(cls, raw_rule): pass
+
+        def get_clean(self, depth=0): pass
+
+        def is_covered_localvars(self, other_rule): pass
+
+        def is_equal_localvars(self, other_rule, strict): pass
+
+        def logprof_header_localvars(self): pass
+
+    def test_implemented_abstract_methods(self):
+        self.ValidSubclass()
+
+    def test_unimplemented_abstract_methods(self):
+        with self.assertRaises(TypeError):
+            BaseRule()
+
+        class InvalidSubclass(BaseRule):
+            pass
+
+        with self.assertRaises(TypeError):
+            InvalidSubclass()
+
     def test_abstract__create_instance(self):
         with self.assertRaises(NotImplementedError):
             BaseRule._create_instance('foo')
@@ -35,21 +64,6 @@ class TestBaserule(AATest):
         with self.assertRaises(NotImplementedError):
             BaseRule.match('foo')
 
-    def test_abstract_get_clean(self):
-        obj = BaseRule()
-        with self.assertRaises(NotImplementedError):
-            obj.get_clean()
-
-    def test_is_equal_localvars(self):
-        obj = BaseRule()
-        with self.assertRaises(NotImplementedError):
-            obj.is_equal_localvars(BaseRule(), False)
-
-    def test_is_covered_localvars(self):
-        obj = BaseRule()
-        with self.assertRaises(NotImplementedError):
-            obj.is_covered_localvars(None)
-
     def test_parse_modifiers_invalid(self):
         regex = re.compile('^\s*(?P<audit>audit\s+)?(?P<allow>allow\s+|deny\s+|invalid\s+)?')
         matches = regex.search('audit invalid ')
@@ -59,27 +73,22 @@ class TestBaserule(AATest):
 
     def test_default_severity(self):
         sev_db = severity.Severity('../severity.db', 'unknown')
-        obj = BaseRule()
+        obj = self.ValidSubclass()
         rank = obj.severity(sev_db)
         self.assertEqual(rank, sev_db.NOT_IMPLEMENTED)
 
-    def test_logprof_header_localvars(self):
-        obj = BaseRule()
-        with self.assertRaises(NotImplementedError):
-            obj.logprof_header_localvars()
-
     def test_edit_header_localvars(self):
-        obj = BaseRule()
+        obj = self.ValidSubclass()
         with self.assertRaises(NotImplementedError):
             obj.edit_header()
 
     def test_validate_edit_localvars(self):
-        obj = BaseRule()
+        obj = self.ValidSubclass()
         with self.assertRaises(NotImplementedError):
             obj.validate_edit('/foo')
 
     def test_store_edit_localvars(self):
-        obj = BaseRule()
+        obj = self.ValidSubclass()
         with self.assertRaises(NotImplementedError):
             obj.store_edit('/foo')
 

utils/test/test-boolean.py

@@ -17,7 +17,6 @@ import unittest
 from collections import namedtuple
 
 from apparmor.common import AppArmorBug, AppArmorException
-from apparmor.rule import BaseRule
 from apparmor.rule.boolean import BooleanRule, BooleanRuleset
 from apparmor.translations import init_translation
 from common_test import AATest, setup_all_loops
@@ -227,18 +226,22 @@ class BooleanCoveredTest_Invalid(AATest):
             obj.is_covered(testobj)
 
     def test_invalid_is_covered_3(self):
-        obj = BooleanRule.create_instance('$foo = true')
+        raw_rule = '$foo = true'
-
+        class SomeOtherClass(BooleanRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = BooleanRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_covered(testobj)
 
     def test_invalid_is_equal(self):
-        obj = BooleanRule.create_instance('$foo = true')
+        raw_rule = '$foo = true'
-
+        class SomeOtherClass(BooleanRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = BooleanRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_equal(testobj)
 

utils/test/test-capability.py

@@ -18,7 +18,6 @@ import unittest
 import apparmor.severity as severity
 from apparmor.common import AppArmorBug, AppArmorException, hasher
 from apparmor.logparser import ReadLog
-from apparmor.rule import BaseRule
 from apparmor.rule.capability import CapabilityRule, CapabilityRuleset
 from apparmor.translations import init_translation
 from common_test import AATest, setup_all_loops
@@ -393,10 +392,12 @@ class CapabilityCoveredTest(AATest):
         self.assertFalse(self._is_covered(obj, 'deny capability,'))
 
     def test_invalid_is_covered(self):
-        obj = CapabilityRule.create_instance('capability sys_admin,')
+        raw_rule = 'capability sys_admin,'
-
+        class SomeOtherClass(CapabilityRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = CapabilityRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_covered(testobj)
 
@@ -410,10 +411,12 @@ class CapabilityCoveredTest(AATest):
             obj.is_covered(testobj)
 
     def test_invalid_is_equal(self):
-        obj = CapabilityRule.create_instance('capability sys_admin,')
+        raw_rule = 'capability sys_admin,'
-
+        class SomeOtherClass(CapabilityRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = CapabilityRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_equal(testobj)
 

utils/test/test-change_profile.py

@@ -18,7 +18,6 @@ from collections import namedtuple
 
 from apparmor.common import AppArmorBug, AppArmorException
 from apparmor.logparser import ReadLog
-from apparmor.rule import BaseRule
 from apparmor.rule.change_profile import ChangeProfileRule, ChangeProfileRuleset
 from apparmor.translations import init_translation
 from common_test import AATest, setup_all_loops
@@ -378,18 +377,22 @@ class ChangeProfileCoveredTest_Invalid(AATest):
             obj.is_covered(testobj)
 
     def test_invalid_is_covered(self):
-        obj = ChangeProfileRule.create_instance('change_profile /foo,')
+        raw_rule = 'change_profile /foo,'
-
+        class SomeOtherClass(ChangeProfileRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = ChangeProfileRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_covered(testobj)
 
     def test_invalid_is_equal(self):
-        obj = ChangeProfileRule.create_instance('change_profile -> /bar,')
+        raw_rule = 'change_profile -> /bar,'
-
+        class SomeOtherClass(ChangeProfileRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = ChangeProfileRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_equal(testobj)
 

utils/test/test-dbus.py

@@ -18,7 +18,6 @@ from collections import namedtuple
 
 from apparmor.common import AppArmorBug, AppArmorException
 from apparmor.logparser import ReadLog
-from apparmor.rule import BaseRule
 from apparmor.rule.dbus import DbusRule, DbusRuleset
 from apparmor.translations import init_translation
 from common_test import AATest, setup_all_loops
@@ -767,18 +766,22 @@ class DbusCoveredTest_Invalid(AATest):
             self.obj.is_covered(self.testobj)
 
     def test_invalid_is_covered(self):
-        obj = DbusRule.create_instance('dbus send,')
+        raw_rule = 'dbus send,'
-
+        class SomeOtherClass(DbusRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = DbusRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_covered(testobj)
 
     def test_invalid_is_equal(self):
-        obj = DbusRule.create_instance('dbus send,')
+        raw_rule = 'dbus send,'
-
+        class SomeOtherClass(DbusRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = DbusRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_equal(testobj)
 

utils/test/test-file.py

@@ -19,7 +19,6 @@ from collections import namedtuple
 import apparmor.severity as severity
 from apparmor.common import AppArmorBug, AppArmorException
 from apparmor.logparser import ReadLog
-from apparmor.rule import BaseRule
 from apparmor.rule.file import FileRule, FileRuleset
 from apparmor.translations import init_translation
 from common_test import AATest, setup_all_loops
@@ -791,18 +790,22 @@ class FileCoveredTest_ManualOrInvalid(AATest):
             self.obj.is_covered(self.testobj)
 
     def test_invalid_is_covered(self):
-        obj = FileRule.create_instance('file,')
+        raw_rule = 'file,'
-
+        class SomeOtherClass(FileRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = FileRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_covered(testobj)
 
     def test_invalid_is_equal(self):
-        obj = FileRule.create_instance('file,')
+        raw_rule = 'file,'
-
+        class SomeOtherClass(FileRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = FileRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_equal(testobj)
 

utils/test/test-include.py

@@ -19,8 +19,6 @@ import unittest
 from collections import namedtuple
 
 from apparmor.common import AppArmorBug, AppArmorException
-# from apparmor.logparser import ReadLog
-from apparmor.rule import BaseRule
 from apparmor.rule.include import IncludeRule, IncludeRuleset
 from apparmor.translations import init_translation
 from common_test import AATest, setup_all_loops, write_file
@@ -298,18 +296,22 @@ class IncludeCoveredTest_Invalid(AATest):
     #         obj.is_covered(testobj)
 
     def test_invalid_is_covered(self):
-        obj = IncludeRule.create_instance('include <abstractions/base>')
+        raw_rule = 'include <abstractions/base>'
-
+        class SomeOtherClass(IncludeRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = IncludeRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_covered(testobj)
 
     def test_invalid_is_equal(self):
-        obj = IncludeRule.create_instance('include <abstractions/base>')
+        raw_rule = 'include <abstractions/base>'
-
+        class SomeOtherClass(IncludeRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = IncludeRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_equal(testobj)
 

utils/test/test-network.py

@@ -18,7 +18,6 @@ from collections import namedtuple
 
 from apparmor.common import AppArmorBug, AppArmorException, cmd
 from apparmor.logparser import ReadLog
-from apparmor.rule import BaseRule
 from apparmor.rule.network import NetworkRule, NetworkRuleset, network_domain_keywords
 from apparmor.translations import init_translation
 from common_test import AATest, setup_all_loops
@@ -370,18 +369,22 @@ class NetworkCoveredTest_Invalid(AATest):
             obj.is_covered(testobj)
 
     def test_invalid_is_covered(self):
-        obj = NetworkRule.create_instance('network inet,')
+        raw_rule = 'network inet,'
-
+        class SomeOtherClass(NetworkRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = NetworkRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_covered(testobj)
 
     def test_invalid_is_equal(self):
-        obj = NetworkRule.create_instance('network inet,')
+        raw_rule = 'network inet,'
-
+        class SomeOtherClass(NetworkRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = NetworkRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_equal(testobj)
 

utils/test/test-ptrace.py

@@ -18,7 +18,6 @@ from collections import namedtuple
 
 from apparmor.common import AppArmorBug, AppArmorException
 from apparmor.logparser import ReadLog
-from apparmor.rule import BaseRule
 from apparmor.rule.ptrace import PtraceRule, PtraceRuleset
 from apparmor.translations import init_translation
 from common_test import AATest, setup_all_loops
@@ -455,18 +454,22 @@ class PtraceCoveredTest_Invalid(AATest):
             obj.is_covered(testobj)
 
     def test_invalid_is_covered(self):
-        obj = PtraceRule.create_instance('ptrace read,')
+        raw_rule = 'ptrace read,'
-
+        class SomeOtherClass(PtraceRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = PtraceRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_covered(testobj)
 
     def test_invalid_is_equal_1(self):
-        obj = PtraceRule.create_instance('ptrace read,')
+        raw_rule = 'ptrace read,'
-
+        class SomeOtherClass(PtraceRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = PtraceRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_equal(testobj)
 

utils/test/test-rlimit.py

@@ -18,7 +18,6 @@ from collections import namedtuple
 
 from apparmor.common import AppArmorBug, AppArmorException
 # from apparmor.logparser import ReadLog
-from apparmor.rule import BaseRule
 from apparmor.rule.rlimit import RlimitRule, RlimitRuleset, split_unit
 from apparmor.translations import init_translation
 from common_test import AATest, setup_all_loops
@@ -354,18 +353,22 @@ class RlimitCoveredTest_Invalid(AATest):
             obj.is_covered(testobj)
 
     def test_invalid_is_covered(self):
-        obj = RlimitRule.create_instance('set rlimit cpu <= 1024,')
+        raw_rule = 'set rlimit cpu <= 1024,'
-
+        class SomeOtherClass(RlimitRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = RlimitRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_covered(testobj)
 
     def test_invalid_is_equal(self):
-        obj = RlimitRule.create_instance('set rlimit cpu <= 1024,')
+        raw_rule = 'set rlimit cpu <= 1024,'
-
+        class SomeOtherClass(RlimitRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = RlimitRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_equal(testobj)
 

utils/test/test-signal.py

@@ -18,7 +18,6 @@ from collections import namedtuple
 
 from apparmor.common import AppArmorBug, AppArmorException
 from apparmor.logparser import ReadLog
-from apparmor.rule import BaseRule
 from apparmor.rule.signal import SignalRule, SignalRuleset
 from apparmor.translations import init_translation
 from common_test import AATest, setup_all_loops
@@ -526,18 +525,22 @@ class SignalCoveredTest_Invalid(AATest):
             obj.is_covered(testobj)
 
     def test_invalid_is_covered(self):
-        obj = SignalRule.create_instance('signal send,')
+        raw_rule = 'signal send,'
-
+        class SomeOtherClass(SignalRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = SignalRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_covered(testobj)
 
     def test_invalid_is_equal(self):
-        obj = SignalRule.create_instance('signal send,')
+        raw_rule = 'signal send,'
-
+        class SomeOtherClass(SignalRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = SignalRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_equal(testobj)
 

utils/test/test-variable.py

@@ -18,7 +18,6 @@ from collections import namedtuple
 from common_test import AATest, setup_all_loops
 
 from apparmor.common import AppArmorBug, AppArmorException
-from apparmor.rule import BaseRule
 from apparmor.rule.variable import VariableRule, VariableRuleset, separate_vars
 from apparmor.translations import init_translation
 
@@ -298,18 +297,22 @@ class VariableCoveredTest_Invalid(AATest):
             obj.is_covered(testobj)
 
     def test_invalid_is_covered_3(self):
-        obj = VariableRule.create_instance('@{foo} = /bar')
+        raw_rule = '@{foo} = /bar'
-
+        class SomeOtherClass(VariableRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = VariableRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_covered(testobj)
 
     def test_invalid_is_equal(self):
-        obj = VariableRule.create_instance('@{foo} = /bar')
+        raw_rule = '@{foo} = /bar'
-
+        class SomeOtherClass(VariableRule):
-        testobj = BaseRule()  # different type
+            pass
 
+        obj = VariableRule.create_instance(raw_rule)
+        testobj = SomeOtherClass.create_instance(raw_rule)  # different type
         with self.assertRaises(AppArmorBug):
             obj.is_equal(testobj)