[apparmor] [PATCH 5/8] utils: Accept parser base and include options in aa-easyprof
Seth Arnold
seth.arnold at canonical.com
Thu Feb 9 00:22:00 UTC 2017
On Wed, Feb 08, 2017 at 10:01:42PM +0000, Tyler Hicks wrote:
> 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 <tyhicks at canonical.com>
> Cc: Christian Boltz <apparmor at cboltz.de>
> Cc: Jamie Strandboge <jamie at ubuntu.com>
> ---
I'd rather the manpage text wrap before 80 chars but otherwise looks good.
Acked-by: Seth Arnold <seth.arnold at canonical.com>
Thanks
>
> 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 = [
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20170208/e6b853de/attachment.pgp>
More information about the AppArmor
mailing list