Hello, 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.
I propose this patch for trunk and 2.11.
Both functions (including this bug) were introduced together with
FileRule, so older releases are not affected.
[ 04-prevent-wa-conflicts.diff ]
=== modified file ./utils/apparmor/aa.py
--- utils/apparmor/aa.py 2017-07-16 21:43:30.714865518 +0200
+++ utils/apparmor/aa.py 2017-08-04 22:06:40.089466392 +0200
@@ -3420,6 +3420,9 @@
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)
@@ -3444,6 +3447,9 @@
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:
=== modified file ./utils/test/test-aa.py
--- utils/test/test-aa.py 2017-07-11 13:33:54.882914000 +0200
+++ utils/test/test-aa.py 2017-08-04 22:04:29.821944241 +0200
@@ -781,6 +781,7 @@
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 @@
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 @@
(['/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 @@
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)
Regards,
Christian Boltz
--
> I'm hesitant to accept this SR, it's fixing just a typo in description
> but will lead to rebuild of many packages in KDE3. I think this is not
> necessary and only waste of resources.
Don't worry. It's cold in Nuremberg atm, additional head produced
doesn't matter :)
[> dsterba and lnussel, https://build.opensuse.org/request/show/490049]
signature.asc
Description: This is a digitally signed message part.
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
