[apparmor] [patch] fix handling of adding to variables
Christian Boltz
apparmor at cboltz.de
Tue Apr 7 22:25:05 UTC 2015
Hello,
the LibreOffice profile uncovered that handling of @{var} += is broken:
File ".../utils/apparmor/aa.py", line 3272, in store_list_var
var[list_var] = set(var[list_var] + vlist)
TypeError: unsupported operand type(s) for +: 'set' and 'list'
This patch fixes it:
- change separate_vars() to use and return a set instead of a list
(FYI: separate_vars() is only called by store_list_var())
- adoptstore_list_var() to expect a set
- remove some old comments in these functions
- explain the less-intuitive parameters of store_list_var()
Also add some tests for separate_vars() and store_list_var().
The tests were developed based on the old code, but not all of them
succeed with the old code.
As usual, the tests uncovered some interesting[tm] behaviour in
separate_vars() (see the XXX comments and tell me what the really
expected behaviour is ;-)
Since this is broken in trunk and the 2.9 branch, I propose this patch
for trunk and 2.9.
Oh, and I noticed that serialize_profile_from_old_profile() also crashes
when it hits "@{var} +=", but I didn't debug that yet.
[ 33-fix-add-to-variable-and-add-tests.diff ]
=== modified file utils/apparmor/aa.py
--- utils/apparmor/aa.py 2015-04-08 00:09:02.931423160 +0200
+++ utils/apparmor/aa.py 2015-04-08 00:13:59.273822091 +0200
@@ -3241,13 +3241,12 @@
def separate_vars(vs):
"""Returns a list of all the values for a variable"""
- data = []
+ data = set()
- #data = [i.strip('"') for i in vs.split()]
RE_VARS = re.compile('\s*((\".+?\")|([^\"]\S+))\s*(.*)$')
while RE_VARS.search(vs):
matches = RE_VARS.search(vs).groups()
- data.append(strip_quotes(matches[0]))
+ data.add(strip_quotes(matches[0]))
vs = matches[3]
return data
@@ -3259,17 +3258,19 @@
return False
def store_list_var(var, list_var, value, var_operation, filename):
- """Store(add new variable or add values to variable) the variables encountered in the given list_var"""
+ """Store(add new variable or add values to variable) the variables encountered in the given list_var
+ - the 'var' parameter will be modified
+ - 'list_var' is the variable name, for example '@{foo}'
+ """
vlist = separate_vars(value)
if var_operation == '=':
if not var.get(list_var, False):
var[list_var] = set(vlist)
else:
- #print('Ignored: New definition for variable for:',list_var,'=', value, 'operation was:',var_operation,'old value=', var[list_var])
raise AppArmorException(_('Redefining existing variable %(variable)s: %(value)s in %(file)s') % { 'variable': list_var, 'value': value, 'file': filename })
elif var_operation == '+=':
if var.get(list_var, False):
- var[list_var] = set(var[list_var] + vlist)
+ var[list_var] |= vlist
else:
raise AppArmorException(_('Values added to a non-existing variable %(variable)s: %(value)s in %(file)s') % { 'variable': list_var, 'value': value, 'file': filename })
else:
=== modified file utils/test/test-aa.py
--- utils/test/test-aa.py 2015-04-08 00:09:02.934422982 +0200
+++ utils/test/test-aa.py 2015-04-07 23:14:27.316826914 +0200
@@ -16,7 +16,7 @@
import tempfile
from common_test import read_file, write_file
-from apparmor.aa import check_for_apparmor, get_profile_flags, set_profile_flags, is_skippable_file, is_skippable_dir, parse_profile_start, write_header, serialize_parse_profile_start
+from apparmor.aa import check_for_apparmor, get_profile_flags, set_profile_flags, is_skippable_file, is_skippable_dir, parse_profile_start, separate_vars, store_list_var, write_header, serialize_parse_profile_start
from apparmor.common import AppArmorException, AppArmorBug
class AaTestWithTempdir(AATest):
@@ -349,6 +349,66 @@
with self.assertRaises(AppArmorBug):
self._parse('xy', '/bar', '/bar') # not a profile start
+class AaTest_separate_vars(AATest):
+ tests = [
+ ('' , set() ),
+ (' ' , set() ),
+ (' foo bar' , {'foo', 'bar' }),
+ ('foo " ' , {'foo' }), # XXX " is ignored
+ (' " foo ' , {' "', 'foo' }), # XXX really?
+ (' foo bar ' , {'foo', 'bar' }),
+ (' foo bar # comment' , {'foo', 'bar', 'comment' }), # XXX should comments be stripped?
+ ('foo' , {'foo' }),
+ ('"foo" "bar baz"' , {'foo', 'bar baz' }),
+ ('foo "bar baz" xy' , {'foo', 'bar baz', 'xy' }),
+ ('foo "bar baz ' , {'foo', 'bar', 'baz' }), # half-quoted
+ (' " foo" bar' , {' foo', 'bar' }),
+ ]
+
+ def _run_test(self, params, expected):
+ result = separate_vars(params)
+ self.assertEqual(result, expected)
+
+
+class AaTest_store_list_var(AATest):
+ tests = [
+ # old var value operation expected (False for exception)
+ ([ {} , 'foo' , '=' ], {'foo'} ), # set
+ ([ {} , 'foo bar' , '=' ], {'foo', 'bar'} ), # set multi
+ ([ {'@{var}': {'foo'}} , 'bar' , '=' ], False ), # redefine var
+ ([ {} , 'bar' , '+=' ], False ), # add to undefined var
+ ([ {'@{var}': {'foo'}} , 'bar' , '+=' ], {'foo', 'bar'} ), # add
+ ([ {'@{var}': {'foo'}} , 'bar baz' , '+=' ], {'foo', 'bar', 'baz'} ), # add multi
+ ([ {'@{var}': {'foo', 'xy'}} , 'bar baz' , '+=' ], {'foo', 'xy', 'bar', 'baz'} ), # add multi to multi
+ ([ {} , 'foo' , '-=' ], False ), # unknown operation
+ ]
+
+ def _run_test(self, params, expected):
+ var = params[0]
+ value = params[1]
+ operation = params[2]
+
+ if not expected:
+ with self.assertRaises(AppArmorException):
+ store_list_var(var, '@{var}', value, operation, 'somefile')
+ return
+
+ # dumy value that must not be changed
+ var['@{foo}'] = {'one', 'two'}
+
+ exp_var = {
+ '@{foo}': {'one', 'two'},
+ '@{var}': expected,
+ }
+
+ store_list_var(var, '@{var}', value, operation, 'somefile')
+
+ self.assertEqual(var.keys(), exp_var.keys())
+
+ for key in exp_var:
+ self.assertEqual(var[key], exp_var[key])
+
+
class AaTest_write_header(AATest):
tests = [
# name embedded_hat write_flags depth flags attachment prof.keyw. comment expected
Regards,
Christian Boltz
--
[lange Antwort schreib] [begreif] [lange falsche Antwort lösch]
Genial.
[Ratti in fontlinge-devel]
More information about the AppArmor
mailing list