https://launchpad.net/bugs/1521031
aa-easyprof accepts a list of abstractions to include and, by default, execs apparmor_parser to verify the generated profile including any abstractions. However, aa-easyprof didn't provide the same flexibility as apparmor_parser when it came to where in the filesystem the abstraction files could exist. The parser supports --base (defaulting to /etc/apparmor.d) and --Include (defaulting to unset) options to specify the search paths for abstraction files. This patch adds the same options to aa-easyprof to aide in two different situations: 1) Some Ubuntu packages use aa-easyprof to generate AppArmor profiles at build time. Something that has been previously needed is a way for those packages to ship their own abstractions file(s) that are #included in the easyprof-generated profile. That's not been possible since the abstraction file(s) have not yet been installed during the package build. 2) The test-aa-easyprof.py script contains some tests that specify abstractions that should be #included. Without the ability to specify a different --base or --Include directory, the abstractions were required to be present in /etc/apparmor.d/abstractions/ or the tests would fail. This prevents the Python utils from being able to strictly test against in-tree code/profiles/etc. I don't like the names of the command line options --base and --Include. They're not particularly descriptive and the capital 'I' is not user friendly. However, I decided to preserve the name of the options from apparmor_parser. Signed-off-by: Tyler Hicks <tyhi...@canonical.com> Cc: Christian Boltz <appar...@cboltz.de> Cc: Jamie Strandboge <ja...@ubuntu.com> --- A different approach to fixing bug 1521031 was previously sent to the list for discussion: https://lists.ubuntu.com/archives/apparmor/2015-November/008875.html However, that proposed solution didn't receive positive reviews and doesn't address the needs of the utils testsuite. IMO, adding functionality for discovering abstractions equivalent to what the parser supports is the proper solution here. Tyler utils/aa-easyprof.pod | 8 ++++ utils/apparmor/easyprof.py | 43 +++++++++++++++++---- utils/test/test-aa-easyprof.py | 88 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+), 8 deletions(-) diff --git a/utils/aa-easyprof.pod b/utils/aa-easyprof.pod index e82041b..1a08408 100644 --- a/utils/aa-easyprof.pod +++ b/utils/aa-easyprof.pod @@ -64,6 +64,14 @@ usually recommended you use policy groups instead, but this is provided as a convenience. AppArmor abstractions are located in /etc/apparmor.d/abstractions. See apparmor.d(5) for details. +=item -b PATH, --base=PATH + +Set the base PATH for resolving abstractions specified by --abstractions. See the same option in apparmor_parser(8) for details. + +=item -I PATH, --Include=PATH + +Add PATH to the search paths used for resolving abstractions specified by --abstractions. See the same option in apparmor_parser(8) for details. + =item -r PATH, --read-path=PATH Specify a PATH to allow owner reads. May be specified multiple times. If the diff --git a/utils/apparmor/easyprof.py b/utils/apparmor/easyprof.py index d7ca3e1..01c7fd6 100644 --- a/utils/apparmor/easyprof.py +++ b/utils/apparmor/easyprof.py @@ -259,7 +259,7 @@ def open_file_read(path): return orig -def verify_policy(policy): +def verify_policy(policy, base=None, include=None): '''Verify policy compiles''' exe = "/sbin/apparmor_parser" if not os.path.exists(exe): @@ -279,7 +279,14 @@ def verify_policy(policy): os.write(f, policy) os.close(f) - rc, out = cmd([exe, '-QTK', fn]) + command = [exe, '-QTK'] + if base: + command.extend(['-b', base]) + if include: + command.extend(['-I', include]) + command.append(fn) + + rc, out = cmd(command) os.unlink(fn) if rc == 0: return True @@ -302,6 +309,14 @@ class AppArmorEasyProfile: if os.path.isfile(self.conffile): self._get_defaults() + self.parser_base = "/etc/apparmor.d" + if opt.parser_base: + self.parser_base = opt.parser_base + + self.parser_include = None + if opt.parser_include: + self.parser_include = opt.parser_include + if opt.templates_dir and os.path.isdir(opt.templates_dir): self.dirs['templates'] = os.path.abspath(opt.templates_dir) elif not opt.templates_dir and \ @@ -350,8 +365,6 @@ class AppArmorEasyProfile: if not 'policygroups' in self.dirs: raise AppArmorException("Could not find policygroups directory") - self.aa_topdir = "/etc/apparmor.d" - self.binary = binary if binary: if not valid_binary_path(binary): @@ -506,9 +519,15 @@ class AppArmorEasyProfile: def gen_abstraction_rule(self, abstraction): '''Generate an abstraction rule''' - p = os.path.join(self.aa_topdir, "abstractions", abstraction) - if not os.path.exists(p): - raise AppArmorException("%s does not exist" % p) + base = os.path.join(self.parser_base, "abstractions", abstraction) + if not os.path.exists(base): + if not self.parser_include: + raise AppArmorException("%s does not exist" % base) + + include = os.path.join(self.parser_include, "abstractions", abstraction) + if not os.path.exists(include): + raise AppArmorException("Neither %s nor %s exist" % (base, include)) + return "#include <abstractions/%s>" % abstraction def gen_variable_declaration(self, dec): @@ -661,7 +680,7 @@ class AppArmorEasyProfile: if no_verify: debug("Skipping policy verification") - elif not verify_policy(policy): + elif not verify_policy(policy, self.parser_base, self.parser_include): msg("\n" + policy) raise AppArmorException("Invalid policy") @@ -811,6 +830,14 @@ def add_parser_policy_args(parser): dest="abstractions", help="Comma-separated list of abstractions", metavar="ABSTRACTIONS") + parser.add_option("-b", "--base", + dest="parser_base", + help="Set the base directory for resolving abstractions", + metavar="DIR") + parser.add_option("-I", "--Include", + dest="parser_include", + help="Add a directory to the search path when resolving abstractions", + metavar="DIR") parser.add_option("--read-path", action="callback", callback=check_for_manifest_arg_append, diff --git a/utils/test/test-aa-easyprof.py b/utils/test/test-aa-easyprof.py index 5cc1f79..3ebfec6 100755 --- a/utils/test/test-aa-easyprof.py +++ b/utils/test/test-aa-easyprof.py @@ -913,6 +913,94 @@ POLICYGROUPS_DIR="%s/templates" raise raise Exception ("abstraction '%s' should be invalid" % s) + def _create_tmp_base_dir(self, prefix='', abstractions=[], tunables=[]): + '''Create a temporary base dir layout''' + base_name = 'apparmor.d' + if prefix: + base_name = '%s-%s' % (prefix, base_name) + base_dir = os.path.join(self.tmpdir, base_name) + abstractions_dir = os.path.join(base_dir, 'abstractions') + tunables_dir = os.path.join(base_dir, 'tunables') + + os.mkdir(base_dir) + os.mkdir(abstractions_dir) + os.mkdir(tunables_dir) + + for f in abstractions: + contents = ''' + # Abstraction file for testing + /%s r, +''' % (f) + open(os.path.join(abstractions_dir, f), 'w').write(contents) + + for f in tunables: + contents = ''' +# Tunable file for testing +@{AA_TEST_%s}=foo +''' % (f) + open(os.path.join(tunables_dir, f), 'w').write(contents) + + return base_dir + + def test_genpolicy_abstractions_custom_base(self): + '''Test genpolicy (custom base dir)''' + abstraction = "custom-base-dir-test-abstraction" + # The default template #includes the base abstraction and global + # tunable so we need to create placeholders + base = self._create_tmp_base_dir(abstractions=['base', abstraction], tunables=['global']) + args = ['--abstractions=%s' % abstraction, '--base=%s' % base] + + p = self._gen_policy(extra_args=args) + search = "#include <abstractions/%s>" % abstraction + self.assertTrue(search in p, "Could not find '%s' in:\n%s" % (search, p)) + inv_s = '###ABSTRACTIONS###' + self.assertFalse(inv_s in p, "Found '%s' in :\n%s" % (inv_s, p)) + + def test_genpolicy_abstractions_custom_base_bad(self): + '''Test genpolicy (custom base dir - bad base dirs)''' + abstraction = "custom-base-dir-test-abstraction" + bad = [ None, '/etc/apparmor.d', '/' ] + for base in bad: + try: + args = ['--abstractions=%s' % abstraction] + if base: + args.append('--base=%s' % base) + self._gen_policy(extra_args=args) + except easyprof.AppArmorException: + continue + except Exception: + raise + raise Exception ("abstraction '%s' should be invalid" % abstraction) + + def test_genpolicy_abstractions_custom_include(self): + '''Test genpolicy (custom include dir)''' + abstraction = "custom-include-dir-test-abstraction" + # No need to create placeholders for the base abstraction or global + # tunable since we're not adjusting the base directory + include = self._create_tmp_base_dir(abstractions=[abstraction]) + args = ['--abstractions=%s' % abstraction, '--Include=%s' % include] + p = self._gen_policy(extra_args=args) + search = "#include <abstractions/%s>" % abstraction + self.assertTrue(search in p, "Could not find '%s' in:\n%s" % (search, p)) + inv_s = '###ABSTRACTIONS###' + self.assertFalse(inv_s in p, "Found '%s' in :\n%s" % (inv_s, p)) + + def test_genpolicy_abstractions_custom_include_bad(self): + '''Test genpolicy (custom include dir - bad include dirs)''' + abstraction = "custom-include-dir-test-abstraction" + bad = [ None, '/etc/apparmor.d', '/' ] + for include in bad: + try: + args = ['--abstractions=%s' % abstraction] + if include: + args.append('--Include=%s' % include) + self._gen_policy(extra_args=args) + except easyprof.AppArmorException: + continue + except Exception: + raise + raise Exception ("abstraction '%s' should be invalid" % abstraction) + def test_genpolicy_profile_name_bad(self): '''Test genpolicy (profile name - bad values)''' bad = [ -- 2.7.4 -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor