Switch alias handling to AliasRule and AliasRuleset

This fixes cases when two aliases with the same left side were
configured - instead of "last one wins" in the dict, AliasRuleset now
keeps both.

ProfileList add_alias() changes its parameters and now expects an
AliasRule object. Adjust all callers to that.

Drop the no longer needed write_alias().

Also adjust the tests to use AliasRule and add a dedup test promised in
an earlier patch series.
This commit is contained in:
Christian Boltz 2020-05-24 15:00:42 +02:00
parent 5bf9b51d4d
commit e3af898ca4
Failed to generate hash of commit
3 changed files with 33 additions and 50 deletions

View file

@ -38,7 +38,6 @@ from apparmor.common import (AppArmorException, AppArmorBug, is_skippable_file,
import apparmor.ui as aaui
from apparmor.regex import (RE_PROFILE_START, RE_PROFILE_END,
RE_PROFILE_ALIAS,
RE_PROFILE_BOOLEAN, RE_PROFILE_CONDITIONAL,
RE_PROFILE_CONDITIONAL_VARIABLE, RE_PROFILE_CONDITIONAL_BOOLEAN,
RE_PROFILE_CHANGE_HAT,
@ -54,6 +53,7 @@ from apparmor.profile_storage import ProfileStorage, add_or_remove_flag, ruletyp
import apparmor.rules as aarules
from apparmor.rule.abi import AbiRule
from apparmor.rule.alias import AliasRule
from apparmor.rule.capability import CapabilityRule
from apparmor.rule.change_profile import ChangeProfileRule
from apparmor.rule.dbus import DbusRule
@ -1855,17 +1855,12 @@ def parse_profile_data(data, file, do_include):
profile_data[profile][hat]['change_profile'].add(ChangeProfileRule.parse(line))
elif RE_PROFILE_ALIAS.search(line):
matches = RE_PROFILE_ALIAS.search(line).groups()
from_name = strip_quotes(matches[0])
to_name = strip_quotes(matches[1])
elif AliasRule.match(line):
if profile and not do_include:
raise AppArmorException(_('Syntax Error: Unexpected alias definition found inside profile in file: %(file)s line: %(line)s') % {
'file': file, 'line': lineno + 1 })
else:
active_profiles.add_alias(file, from_name, to_name)
active_profiles.add_alias(file, AliasRule.parse(line))
elif RlimitRule.match(line):
if not profile:

View file

@ -13,8 +13,8 @@
# ----------------------------------------------------------------------
from apparmor.aare import AARE
from apparmor.common import AppArmorBug, AppArmorException, type_is_str
from apparmor.rule import quote_if_needed
from apparmor.common import AppArmorBug, AppArmorException
from apparmor.rule.alias import AliasRule, AliasRuleset
from apparmor.rule.abi import AbiRule, AbiRuleset
from apparmor.rule.include import IncludeRule, IncludeRuleset
from apparmor.rule.variable import VariableRule, VariableRuleset
@ -47,7 +47,7 @@ class ProfileList:
self.files[filename] = {
'abi': AbiRuleset(),
'alias': {},
'alias': AliasRuleset(),
'inc_ie': IncludeRuleset(),
'variable': VariableRuleset(),
'profiles': [],
@ -92,21 +92,15 @@ class ProfileList:
self.files[filename]['abi'].add(abi_rule)
def add_alias(self, filename, alias, target):
def add_alias(self, filename, alias_rule):
''' Store the given alias rule for the given profile filename preamble '''
if not type_is_str(alias):
raise AppArmorBug('Wrong type given to ProfileList: %s' % alias)
if not type_is_str(target):
raise AppArmorBug('Wrong type given to ProfileList: %s' % target)
if type(alias_rule) is not AliasRule:
raise AppArmorBug('Wrong type given to ProfileList: %s' % alias_rule)
self.init_file(filename)
# allowed in the parser
# if self.files[filename]['alias'].get(alias):
# raise AppArmorException('Trying to re-define alias %s' % alias)
self.files[filename]['alias'][alias] = target
self.files[filename]['alias'].add(alias_rule)
def add_inc_ie(self, filename, inc_rule):
''' Store the given include / include if exists rule for the given profile filename preamble '''
@ -134,7 +128,7 @@ class ProfileList:
deleted = 0
for r_type in ['abi', 'inc_ie', 'variable']: # TODO: don't hardcode
for r_type in ['abi', 'alias', 'inc_ie', 'variable']: # TODO: don't hardcode
deleted += self.files[filename][r_type].delete_duplicates(None) # None means not to check includes -- TODO check if this makes sense for all preamble rule types
return deleted
@ -146,7 +140,7 @@ class ProfileList:
data = []
data += self.files[filename]['abi'].get_raw(depth)
data += write_alias(self.files[filename])
data += self.files[filename]['alias'].get_raw(depth)
data += self.files[filename]['inc_ie'].get_raw(depth)
data += self.files[filename]['variable'].get_raw(depth)
return data
@ -158,7 +152,7 @@ class ProfileList:
data = []
data += self.files[filename]['abi'].get_clean_unsorted(depth)
data += write_alias(self.files[filename])
data += self.files[filename]['alias'].get_clean_unsorted(depth)
data += self.files[filename]['inc_ie'].get_clean_unsorted(depth)
data += self.files[filename]['variable'].get_clean_unsorted(depth)
return data
@ -251,14 +245,3 @@ class ProfileList:
raise AppArmorBug('%s not listed in ProfileList files' % filename)
return self.files[filename]['profiles']
def write_alias(prof_data):
data = []
if prof_data['alias']:
for key in sorted(prof_data['alias'].keys()):
data.append('alias %s -> %s,' % (quote_if_needed(key), quote_if_needed(prof_data['alias'][key])))
data.append('')
return data

View file

@ -19,6 +19,7 @@ import shutil
from apparmor.common import AppArmorBug, AppArmorException
from apparmor.profile_list import ProfileList
from apparmor.rule.abi import AbiRule
from apparmor.rule.alias import AliasRule
from apparmor.rule.include import IncludeRule
from apparmor.rule.variable import VariableRule
@ -205,40 +206,44 @@ class TestAdd_alias(AATest):
self.pl = ProfileList()
def testAdd_alias_1(self):
self.pl.add_alias('/etc/apparmor.d/bin.foo', '/foo', '/bar')
self.pl.add_alias('/etc/apparmor.d/bin.foo', AliasRule('/foo', '/bar'))
self.assertEqual(list(self.pl.files.keys()), ['/etc/apparmor.d/bin.foo'])
self.assertEqual(self.pl.get_clean('/etc/apparmor.d/bin.foo'), ['alias /foo -> /bar,', ''])
self.assertEqual(self.pl.get_raw('/etc/apparmor.d/bin.foo'), ['alias /foo -> /bar,', ''])
def testAdd_alias_2(self):
self.pl.add_alias('/etc/apparmor.d/bin.foo', '/foo', '/bar')
self.pl.add_alias('/etc/apparmor.d/bin.foo', '/xyz', '/zyx')
self.pl.add_alias('/etc/apparmor.d/bin.foo', AliasRule('/foo', '/bar'))
self.pl.add_alias('/etc/apparmor.d/bin.foo', AliasRule('/xyz', '/zyx'))
self.assertEqual(list(self.pl.files.keys()), ['/etc/apparmor.d/bin.foo'])
self.assertEqual(self.pl.get_clean('/etc/apparmor.d/bin.foo'), ['alias /foo -> /bar,', 'alias /xyz -> /zyx,', ''])
self.assertEqual(self.pl.get_raw('/etc/apparmor.d/bin.foo'), ['alias /foo -> /bar,', 'alias /xyz -> /zyx,', ''])
def testAdd_alias_dupe(self):
self.pl.add_alias('/etc/apparmor.d/bin.foo', '/foo', '/bar')
# parser allows re-defining aliases
# with self.assertRaises(AppArmorException):
# self.pl.add_alias('/etc/apparmor.d/bin.foo', '/foo', '/redefine') # attempt to redefine alias
self.pl.add_alias('/etc/apparmor.d/bin.foo', '/foo', '/redefine') # redefine alias
def testAdd_alias_two_targets(self):
self.pl.add_alias('/etc/apparmor.d/bin.foo', AliasRule('/foo', '/bar'))
self.pl.add_alias('/etc/apparmor.d/bin.foo', AliasRule('/foo', '/another_target'))
self.assertEqual(list(self.pl.files.keys()), ['/etc/apparmor.d/bin.foo'])
self.assertEqual(self.pl.get_clean('/etc/apparmor.d/bin.foo'), ['alias /foo -> /redefine,', ''])
self.assertEqual(self.pl.get_raw('/etc/apparmor.d/bin.foo'), ['alias /foo -> /redefine,', ''])
self.assertEqual(self.pl.get_clean('/etc/apparmor.d/bin.foo'), ['alias /foo -> /bar,', 'alias /foo -> /another_target,', ''])
self.assertEqual(self.pl.get_raw('/etc/apparmor.d/bin.foo'), ['alias /foo -> /bar,', 'alias /foo -> /another_target,', ''])
def testAdd_alias_error_1(self):
with self.assertRaises(AppArmorBug):
self.pl.add_alias('/etc/apparmor.d/bin.foo', None, '/foo') # alias None insteadd of str
self.pl.add_alias('/etc/apparmor.d/bin.foo', AliasRule(None, '/foo')) # alias None insteadd of str
self.assertEqual(list(self.pl.files.keys()), [])
def testAdd_alias_error_2(self):
with self.assertRaises(AppArmorBug):
self.pl.add_alias('/etc/apparmor.d/bin.foo', '/foo', None) # target None insteadd of str
self.pl.add_alias('/etc/apparmor.d/bin.foo', AliasRule('/foo', None)) # target None insteadd of str
self.assertEqual(list(self.pl.files.keys()), [])
# def test_dedup_alias_1(self):
# TODO: implement after fixing alias handling (when a profile has two aliases with the same path on the left side)
def test_dedup_alias_1(self):
self.pl.add_alias('/etc/apparmor.d/bin.foo', AliasRule('/foo', '/bar'))
self.pl.add_alias('/etc/apparmor.d/bin.foo', AliasRule('/foo', '/another_target'))
self.pl.add_alias('/etc/apparmor.d/bin.foo', AliasRule('/foo', '/bar')) # duplicate
deleted = self.pl.delete_preamble_duplicates('/etc/apparmor.d/bin.foo')
self.assertEqual(deleted, 1)
self.assertEqual(list(self.pl.files.keys()), ['/etc/apparmor.d/bin.foo'])
self.assertEqual(self.pl.get_clean('/etc/apparmor.d/bin.foo'), ['alias /foo -> /bar,', 'alias /foo -> /another_target,', ''])
self.assertEqual(self.pl.get_raw('/etc/apparmor.d/bin.foo'), ['alias /foo -> /bar,', 'alias /foo -> /another_target,', ''])
class TestAdd_variable(AATest):
def AASetup(self):