On Fri, Aug 04, 2017 at 10:17:42PM +0200, Christian Boltz wrote: > 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. >
Acked-by: Seth Arnold <[email protected]> Acked for both Thanks > [ 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] > -- > AppArmor mailing list > [email protected] > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/apparmor
signature.asc
Description: PGP signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
