From 237b17329fbde0d6fbbf5fba028e21649b91d74d Mon Sep 17 00:00:00 2001 From: Christian Boltz Date: Sun, 15 Jun 2025 16:38:37 +0200 Subject: [PATCH 1/5] test-mount: test invalid fstype via tests array ... instead of duplicating the logic --- utils/test/test-mount.py | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/utils/test/test-mount.py b/utils/test/test-mount.py index f6fd324da..c2e8f4609 100644 --- a/utils/test/test-mount.py +++ b/utils/test/test-mount.py @@ -1,6 +1,7 @@ #!/usr/bin/python3 # ---------------------------------------------------------------------- # Copyright (C) 2024 Canonical, Ltd. +# Copyright (C) 2025 Christian Boltz # # This program is free software; you can redistribute it and/or # modify it under the terms of version 2 of the GNU General Public @@ -123,6 +124,8 @@ class MountTestParseInvalid(AATest): ('mount options=(),', AppArmorException), ('mount option=(invalid),', AppArmorException), ('mount option=(ext3ext4),', AppArmorException), + ('mount fstype=({unclosed_regex),', AppArmorException), # invalid AARE + ('mount fstype=({closed}twice}),', AppArmorException), # invalid AARE ) def _run_test(self, rawrule, expected): @@ -144,16 +147,6 @@ class MountTestParseInvalid(AATest): with self.assertRaises(AppArmorBug): MountRule('mount', ('ext3', 'ext4'), MountRule.ALL, MountRule.ALL, MountRule.ALL) # fstype[0] should be '=' or 'in' - def test_diff_invalid_fstype_aare(self): - tests = [ - 'mount fstype=({unclosed_regex),', - 'mount fstype=({closed}twice}),', - ] - - for t in tests: - with self.assertRaises(AppArmorException): - MountRule.create_instance(t) - def test_diff_invalid_fstype_aare_2(self): fslists = [ ['invalid_{_regex'], From fd89e3185c18a7b99a4c78089df512f6dcbbd812 Mon Sep 17 00:00:00 2001 From: Christian Boltz Date: Sun, 15 Jun 2025 17:30:39 +0200 Subject: [PATCH 2/5] test-capability: switch CapabilityTestParseInvalid to tests array --- utils/test/test-capability.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/utils/test/test-capability.py b/utils/test/test-capability.py index 666f48cc8..648bcd897 100644 --- a/utils/test/test-capability.py +++ b/utils/test/test-capability.py @@ -1,6 +1,6 @@ #!/usr/bin/python3 # ---------------------------------------------------------------------- -# Copyright (C) 2014 Christian Boltz +# Copyright (C) 2014-2025 Christian Boltz # # This program is free software; you can redistribute it and/or # modify it under the terms of version 2 of the GNU General Public @@ -240,21 +240,21 @@ class CapabilityTest(AATest): }) +class CapabilityTestParseInvalid(AATest): + tests = ( + # rule exception, matches regex? + ('capability', (AppArmorException, False)), # missing comma + ('network,', (AppArmorException, False)), # not a capability rule + ) + + def _run_test(self, rawrule, expected): + exp_exception, matches_regex = expected + self.assertEqual(matches_regex, CapabilityRule.match(rawrule)) # does the invalid rules still match the main regex? + with self.assertRaises(exp_exception): + CapabilityRule.create_instance(rawrule) + + class InvalidCapabilityTest(AATest): - def _check_invalid_rawrule(self, rawrule): - obj = None - with self.assertRaises(AppArmorException): - obj = CapabilityRule.create_instance(rawrule) - - self.assertFalse(CapabilityRule.match(rawrule)) - self.assertIsNone(obj, 'CapbilityRule handed back an object unexpectedly') - - def test_invalid_cap_missing_comma(self): - self._check_invalid_rawrule('capability') # missing comma - - def test_invalid_cap_non_CapabilityRule(self): - self._check_invalid_rawrule('network,') # not a capability rule - def test_empty_cap_set(self): obj = CapabilityRule('chown') obj.capability.clear() From 6d2a0f6ba71e1c7b8300ed1a28cc58cc14a07a7a Mon Sep 17 00:00:00 2001 From: Christian Boltz Date: Sun, 15 Jun 2025 18:03:56 +0200 Subject: [PATCH 3/5] InvalidChangeProfileInit: fix testing for missing params - use valid values for given params - add testcase with two given / one missing params --- utils/test/test-change_profile.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/utils/test/test-change_profile.py b/utils/test/test-change_profile.py index 0263a6c00..717609545 100644 --- a/utils/test/test-change_profile.py +++ b/utils/test/test-change_profile.py @@ -182,7 +182,11 @@ class InvalidChangeProfileInit(AATest): def test_missing_params_2(self): with self.assertRaises(TypeError): - ChangeProfileRule('inet') + ChangeProfileRule(None) + + def test_missing_params_3(self): + with self.assertRaises(TypeError): + ChangeProfileRule(None, ChangeProfileRule.ALL) class InvalidChangeProfileTest(AATest): From 58f5c2b7e865a324e8923bb01c874ac5c1ba4dbc Mon Sep 17 00:00:00 2001 From: Christian Boltz Date: Sun, 15 Jun 2025 17:54:50 +0200 Subject: [PATCH 4/5] ChangeProfileTestParseInvalid: allow tests that match the regex (even if the existing tests all don't match it) --- utils/test/test-change_profile.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/utils/test/test-change_profile.py b/utils/test/test-change_profile.py index 717609545..7e4386512 100644 --- a/utils/test/test-change_profile.py +++ b/utils/test/test-change_profile.py @@ -1,6 +1,6 @@ #!/usr/bin/python3 # ---------------------------------------------------------------------- -# Copyright (C) 2015 Christian Boltz +# Copyright (C) 2015-2025 Christian Boltz # # This program is free software; you can redistribute it and/or # modify it under the terms of version 2 of the GNU General Public @@ -84,15 +84,17 @@ class ChangeProfileTestParse(ChangeProfileTest): class ChangeProfileTestParseInvalid(ChangeProfileTest): tests = ( - ('change_profile -> ,', AppArmorException), - ('change_profile foo -> ,', AppArmorException), - ('change_profile notsafe,', AppArmorException), - ('change_profile safety -> /bar,', AppArmorException), + # rule exception, matches regex? + ('change_profile -> ,', (AppArmorException, False)), + ('change_profile foo -> ,', (AppArmorException, False)), + ('change_profile notsafe,', (AppArmorException, False)), + ('change_profile safety -> /bar,', (AppArmorException, False)), ) def _run_test(self, rawrule, expected): - self.assertFalse(ChangeProfileRule.match(rawrule)) - with self.assertRaises(expected): + exp_exception, matches_regex = expected + self.assertEqual(matches_regex, ChangeProfileRule.match(rawrule)) # does the invalid rules still match the main regex? + with self.assertRaises(exp_exception): ChangeProfileRule.create_instance(rawrule) From f977530f3982a0562d35a0cf6866112d8c70e687 Mon Sep 17 00:00:00 2001 From: Christian Boltz Date: Sun, 15 Jun 2025 18:18:48 +0200 Subject: [PATCH 5/5] Move some tests to class ChangeProfileTestParseInvalid ... to reduce code duplication --- utils/test/test-change_profile.py | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/utils/test/test-change_profile.py b/utils/test/test-change_profile.py index 7e4386512..ffd5b7c41 100644 --- a/utils/test/test-change_profile.py +++ b/utils/test/test-change_profile.py @@ -85,6 +85,8 @@ class ChangeProfileTestParse(ChangeProfileTest): class ChangeProfileTestParseInvalid(ChangeProfileTest): tests = ( # rule exception, matches regex? + ('change_profile', (AppArmorException, False)), # missing comma + ('dbus,', (AppArmorException, False)), # not a change_profile rule ('change_profile -> ,', (AppArmorException, False)), ('change_profile foo -> ,', (AppArmorException, False)), ('change_profile notsafe,', (AppArmorException, False)), @@ -192,20 +194,6 @@ class InvalidChangeProfileInit(AATest): class InvalidChangeProfileTest(AATest): - def _check_invalid_rawrule(self, rawrule): - obj = None - self.assertFalse(ChangeProfileRule.match(rawrule)) - with self.assertRaises(AppArmorException): - obj = ChangeProfileRule.create_instance(rawrule) - - self.assertIsNone(obj, 'ChangeProfileRule handed back an object unexpectedly') - - def test_invalid_net_missing_comma(self): - self._check_invalid_rawrule('change_profile') # missing comma - - def test_invalid_net_non_ChangeProfileRule(self): - self._check_invalid_rawrule('dbus,') # not a change_profile rule - def test_empty_net_data_1(self): obj = ChangeProfileRule(None, '/foo', '/bar') obj.execcond = ''