mir/apparmor
2
0
Fork 0
mirror of https://gitlab.com/apparmor/apparmor synced 2025-08-30 22:05:27 +00:00
Code Issues Releases Wiki Activity

Prevent 'wa' conflicts for file rules

Browse Source 
get_file_perms() and propose_file_rules() happily collect all file
permissions. This could lead to proposing 'wa' permissions in
aa-logprof, which then errored out because of conflicting permissions.

This patch adds a check to both functions that removes 'a' if 'w' is
present, and extends the tests to check this.


Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk and 2.11.

Note: Both functions (including this bug) were introduced together with
FileRule, so older releases are not affected.
This commit is contained in:
Christian Boltz
2017-08-04 22:26:41 +02:00
parent 1d0790c0c6
commit 12cfc5ecf4
2 changed files with 10 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
utils/apparmor/aa.py
View File

@@ -3415,6 +3415,9 @@ def get_file_perms(profile, path, audit, deny):
for perm in incperms[allow_or_deny][owner_or_all]:
perms[allow_or_deny][owner_or_all].add(perm)
if 'a' in perms[allow_or_deny][owner_or_all] and 'w' in perms[allow_or_deny][owner_or_all]:
perms[allow_or_deny][owner_or_all].remove('a') # a is a subset of w, so remove it
for incpath in incperms['paths']:
perms['paths'].add(incpath)
@@ -3439,6 +3442,9 @@ def propose_file_rules(profile_obj, rule_obj):
merged_rule_obj.perms.add(perm)
merged_rule_obj.raw_rule = None
if 'a' in merged_rule_obj.perms and 'w' in merged_rule_obj.perms:
merged_rule_obj.perms.remove('a') # a is a subset of w, so remove it
pathlist = {original_path} | existing_perms['paths'] | set(glob_common(original_path))
for user_glob in user_globs:

4
utils/test/test-aa.py
View File

@@ -781,6 +781,7 @@ class AaTest_get_file_perms_1(AATest):
class AaTest_get_file_perms_2(AATest):
tests = [
('/usr/share/common-licenses/foo/bar', {'allow': {'all': {'r'}, 'owner': {'w'} }, 'deny': {'all':set(), 'owner': set()}, 'paths': {'/usr/share/common-licenses/**'} }),
('/usr/share/common-licenses/what/ever', {'allow': {'all': {'r'}, 'owner': {'w'} }, 'deny': {'all':set(), 'owner': set()}, 'paths': {'/usr/share/common-licenses/**', '/usr/share/common-licenses/what/ever'} }),
('/dev/null', {'allow': {'all': {'r', 'w', 'k'}, 'owner': set() }, 'deny': {'all':set(), 'owner': set()}, 'paths': {'/dev/null'} }),
('/foo/bar', {'allow': {'all': {'r', 'w'}, 'owner': set() }, 'deny': {'all':set(), 'owner': set()}, 'paths': {'/foo/bar'} }), # exec perms not included
('/no/thing', {'allow': {'all': set(), 'owner': set() }, 'deny': {'all':set(), 'owner': set()}, 'paths': set() }),
@@ -808,6 +809,7 @@ class AaTest_get_file_perms_2(AATest):
profile['include']['abstractions/enchant'] = True # includes abstractions/aspell
profile['file'].add(FileRule.parse('owner /usr/share/common-licenses/** w,'))
profile['file'].add(FileRule.parse('owner /usr/share/common-licenses/what/ever a,')) # covered by the above 'w' rule, so 'a' should be ignored
profile['file'].add(FileRule.parse('/dev/null rwk,'))
profile['file'].add(FileRule.parse('/foo/bar rwix,'))
@@ -822,6 +824,7 @@ class AaTest_propose_file_rules(AATest):
(['/foo/bar', 'rw'], ['/foo/bar rw,'] ),
(['/usr/lib/ispell/', 'w'], ['/{usr/,}lib{,32,64}/** rw,', '/usr/lib/ispell/ rw,'] ),
(['/usr/lib/aspell/some.so', 'k'], ['/usr/lib/aspell/* mrk,', '/usr/lib/aspell/*.so mrk,', '/{usr/,}lib{,32,64}/** mrk,', '/usr/lib/aspell/some.so mrk,'] ),
(['/foo/log', 'w'], ['/foo/log w,'] ),
]
def _run_test(self, params, expected):
@@ -850,6 +853,7 @@ class AaTest_propose_file_rules(AATest):
profile['file'].add(FileRule.parse('owner /usr/share/common-licenses/** w,'))
profile['file'].add(FileRule.parse('/dev/null rwk,'))
profile['file'].add(FileRule.parse('/foo/bar rwix,'))
profile['file'].add(FileRule.parse('/foo/log a,')) # will be replaced with '/foo/log w,' (not 'wa')
rule_obj = FileRule(params[0], params[1], None, FileRule.ALL, owner=False, log_event=True)
proposals = propose_file_rules(profile, rule_obj)