[apparmor] [PATCH 5/8] utils: Accept parser base and include options in aa-easyprof
Tyler Hicks
tyhicks at canonical.com
Thu Feb 9 06:10:13 UTC 2017
On 02/08/2017 06:22 PM, Seth Arnold wrote:
> 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.
Me too. Fixed locally.
> Acked-by: Seth Arnold <seth.arnold at canonical.com>
> Thanks
Thanks for your reviews!
Tyler
>
>
>>
>> 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 = [
>>
>>
More information about the AppArmor
mailing list