From 380dbb84b8203adde180e47dbaa8db63fb599f85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maxime=20B=C3=A9lair?= Date: Thu, 17 Jul 2025 10:39:59 +0200 Subject: [PATCH] utils: Fix priority checking for is_covered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MR !1735 mistakenly assumed that x.is_covered(y) means "x is covered by y" when the opposite is true Fix the logic of is_covered and associated tests. Signed-off-by: Maxime Bélair --- utils/apparmor/rule/__init__.py | 2 +- utils/test/test-aa.py | 19 ++++++++++++++++++- utils/test/test-dbus.py | 4 ++-- utils/test/test-io_uring.py | 4 ++-- utils/test/test-unix.py | 4 ++-- 5 files changed, 25 insertions(+), 8 deletions(-) diff --git a/utils/apparmor/rule/__init__.py b/utils/apparmor/rule/__init__.py index 1f5030fe8..5d58a9eec 100644 --- a/utils/apparmor/rule/__init__.py +++ b/utils/apparmor/rule/__init__.py @@ -194,7 +194,7 @@ class BaseRule(metaclass=ABCMeta): if other_rule.audit and not self.audit: return False - if check_priority and (self.priority or 0) > (other_rule.priority or 0): + if check_priority and (self.priority or 0) < (other_rule.priority or 0): return False # still here? -> then the common part is covered, check rule-specific things now diff --git a/utils/test/test-aa.py b/utils/test/test-aa.py index c5cde894c..c51d18671 100644 --- a/utils/test/test-aa.py +++ b/utils/test/test-aa.py @@ -16,7 +16,7 @@ import unittest import apparmor.aa # needed to set global vars in some tests from apparmor.aa import ( change_profile_flags, check_for_apparmor, create_new_profile, get_file_perms, get_interpreter_and_abstraction, get_profile_flags, - merged_to_split, parse_profile_data, propose_file_rules, set_options_audit_mode, set_options_owner_mode) + merged_to_split, parse_profile_data, propose_file_rules, set_options_audit_mode, set_options_owner_mode, is_known_rule) from apparmor.aare import AARE from apparmor.common import AppArmorBug, AppArmorException, is_skippable_file from apparmor.rule.file import FileRule @@ -761,6 +761,23 @@ class AaTest_merged_to_split(AATest): self.assertTrue(result[profile][hat]) +class AaTest_is_known_rule(AATest): + tests = ( + (FileRule.create_instance("priority=-1 audit deny /foo r,"), True), + (FileRule.create_instance("priority=1 audit deny /foo r,"), False) + ) + + def _run_test(self, params, expected): + d = '/foo xattrs=(user.bar=bar) flags=(complain) {\n}\n' + fr = FileRule.create_instance("audit deny /foo r,") + + prof = parse_profile_data(d.split(), 'somefile', False, False) + prof['/foo']['file'].add(fr) + + self.assertEqual(is_known_rule(prof['/foo'], 'file', params), expected) + self.assertEqual(prof['/foo']['file'].is_covered(params), expected) + + setup_aa(apparmor.aa) setup_all_loops(__name__) if __name__ == '__main__': diff --git a/utils/test/test-dbus.py b/utils/test/test-dbus.py index afe7c73c5..994e70ec8 100644 --- a/utils/test/test-dbus.py +++ b/utils/test/test-dbus.py @@ -733,8 +733,8 @@ class DbusCoveredTest_Priority(DbusCoveredTest): tests = ( # rule equal strict equal covered covered exact - ('priority=-1 dbus send,', (False, False, False, False)), - ('priority=1 dbus send,', (False, False, True, True)), + ('priority=1 dbus send,', (False, False, False, False)), + ('priority=-1 dbus send,', (False, False, True, True)), ('priority=0 dbus send,', (True, False, True, True)), ) diff --git a/utils/test/test-io_uring.py b/utils/test/test-io_uring.py index 56f4a8f65..a272201ba 100644 --- a/utils/test/test-io_uring.py +++ b/utils/test/test-io_uring.py @@ -182,8 +182,8 @@ class IOUringIsCoveredTest(AATest): def test_is_covered_priority(self): obj = IOUringRule(IOUringRule.ALL, 'ba*', priority=0) prio_obj = IOUringRule(IOUringRule.ALL, 'ba*', priority=1) - self.assertTrue(obj.is_covered(prio_obj)) - self.assertFalse(prio_obj.is_covered(obj)) + self.assertFalse(obj.is_covered(prio_obj)) + self.assertTrue(prio_obj.is_covered(obj)) def test_is_covered_priority_2(self): obj = IOUringRule(IOUringRule.ALL, 'ba*') diff --git a/utils/test/test-unix.py b/utils/test/test-unix.py index 1dcd11ae5..65444bdb1 100644 --- a/utils/test/test-unix.py +++ b/utils/test/test-unix.py @@ -147,8 +147,8 @@ class UnixIsCoveredTest(AATest): obj = UnixRule(('accept', 'rw'), {'type': 'F*', 'protocol': 'AA'}, {'addr': 'AA'}, {'addr': 'AA', 'label': 'bb'}, priority=0) prio_obj = UnixRule(('accept', 'rw'), {'type': 'F*', 'protocol': 'AA'}, {'addr': 'AA'}, {'addr': 'AA', 'label': 'bb'}, priority=1) - self.assertTrue(obj.is_covered(prio_obj)) - self.assertFalse(prio_obj.is_covered(obj)) + self.assertFalse(obj.is_covered(prio_obj)) + self.assertTrue(prio_obj.is_covered(obj)) class UnixLogprofHeaderTest(AATest):