[apparmor] [patch] [33/38] Add cleanup flag to *Ruleset.add()

[Christian Boltz]

Hello,

adding a rule to *Ruleset means it simply gets added. This also means
that then-superfluous rules will be kept.

This patch adds an optional cleanup flag to add(). If set, rules covered
by the new rule will be deleted. The difference to delete_duplicates()
is that cleanup only deletes rules that are covered by the new rule, but
keeps other, unrelated superfluous rules.

Also return the number of deleted rules to give the UI a chance to
report this number.

Finally, adjust the existing tests for FileRuleset to ensure default
mode (without cleanup) doesn't delete any rules, and add a test using
the cleanup flag.


[ 33-ruleset-cleanup-duplicates-on-add.diff ]

=== modified file ./utils/apparmor/rule/__init__.py
--- utils/apparmor/rule/__init__.py	2016-08-08 23:55:34.092316443 +0200
+++ utils/apparmor/rule/__init__.py	2016-08-11 22:43:12.334234040 +0200
@@ -345,10 +345,28 @@
         else:
             return '<%s (empty) />' % classname
 
-    def add(self, rule):
-        '''add a rule object'''
+    def add(self, rule, cleanup=False):
+        '''add a rule object
+           if cleanup is specified, delete rules that are covered by the new rules
+           (the difference to delete_duplicates() is: cleanup only deletes rules that
+           are covered by the new rule, but keeps other, unrelated superfluous rules)
+        '''
+        deleted = 0
+
+        if cleanup:
+            oldrules = self.rules
+            self.rules = []
+
+            for oldrule in oldrules:
+                if not rule.is_covered(oldrule):
+                    self.rules.append(oldrule)
+                else:
+                    deleted += 1
+
         self.rules.append(rule)
 
+        return deleted
+
     def get_raw(self, depth=0):
         '''return all raw rules (if possible/not modified in their original formatting).
            Returns an array of lines, with depth * leading whitespace'''
=== modified file ./utils/test/test-file.py
--- utils/test/test-file.py	2016-08-08 23:55:34.096316427 +0200
+++ utils/test/test-file.py	2016-08-11 22:51:48.316040452 +0200
@@ -837,9 +837,11 @@
             '',
         ]
 
+        deleted = 0
         for rule in rules:
-            ruleset.add(FileRule.parse(rule))
+            deleted += ruleset.add(FileRule.parse(rule))
 
+        self.assertEqual(deleted, 0)
         self.assertEqual(expected_raw, ruleset.get_raw())
         self.assertEqual(expected_clean, ruleset.get_clean())
 
@@ -866,12 +868,53 @@
              '',
         ]
 
+        deleted = 0
         for rule in rules:
-            ruleset.add(FileRule.parse(rule))
+            deleted += ruleset.add(FileRule.parse(rule))
 
+        self.assertEqual(deleted, 0)
         self.assertEqual(expected_raw, ruleset.get_raw(1))
         self.assertEqual(expected_clean, ruleset.get_clean(1))
 
+    def test_ruleset_cleanup_add_1(self):
+        ruleset = FileRuleset()
+        rules = [
+            '/foo/bar r,',
+            '/foo/baz rw,',
+            '/foo/baz rwk,',
+        ]
+
+        rules_with_cleanup = [
+            '/foo/* r,',
+        ]
+
+        expected_raw = [
+            '  /foo/baz rw,',
+            '  /foo/baz rwk,',
+            '  /foo/* r,',
+             '',
+        ]
+
+        expected_clean = [
+            '  /foo/* r,',
+            '  /foo/baz rw,',
+            '  /foo/baz rwk,',
+             '',
+        ]
+
+        deleted = 0
+        for rule in rules:
+            deleted += ruleset.add(FileRule.parse(rule))
+
+        self.assertEqual(deleted, 0)  # rules[] are added without cleanup mode, so the superfluous '/foo/baz rw,' should be kept
+
+        for rule in rules_with_cleanup:
+            deleted += ruleset.add(FileRule.parse(rule), cleanup=True)
+
+        self.assertEqual(deleted, 1)  # rules_with_cleanup made '/foo/bar r,' superfluous
+        self.assertEqual(expected_raw, ruleset.get_raw(1))
+        self.assertEqual(expected_clean, ruleset.get_clean(1))
+
 
 #class FileDeleteTest(AATest):
 #    pass



Regards,

Christian Boltz
-- 
<intrigeri> there's enough of "curl | sudo bash ..." around..
* mhayden hides under his desk
[from #apparmor]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20160812/ea79d54c/attachment.pgp>