[apparmor] [patch] Several fixes for variable handling
Christian Boltz
apparmor at cboltz.de
Sun Nov 15 19:37:09 UTC 2015
Hello,
parsing variables was broken in several ways:
- empty quotes (representing an intentionally empty value) were lost,
causing parser failures
- items consisting of only one letter were lost due to a bug in RE_VARS
- RE_VARS didn't start with ^, which means leading garbage (= syntax
errors) was ignored
- trailing garbage was also ignored
This patch fixes those issues in separate_vars() and changes
var_transform() to write out empty quotes (instead of nothing) for empty
values.
Also add some tests for separate_vars() with empty quotes and adjust
several tests with invalid syntax to expect an AppArmorException.
var_transform() gets some tests added.
Finally, remove 3 testcases from the "fails to raise an exception" list
in test-parser-simple-tests.py.
I propose this patch for trunk and 2.9 (except the test-parser-simple-tests.py
changes because this file isn't in 2.9)
[ 20-variable-fixes.diff ]
=== modified file ./utils/apparmor/aa.py
--- utils/apparmor/aa.py 2015-11-15 20:30:42.251877998 +0100
+++ utils/apparmor/aa.py 2015-11-15 20:01:17.937507807 +0100
@@ -2800,7 +2800,7 @@
list_var = strip_quotes(matches[0])
var_operation = matches[1]
- value = strip_quotes(matches[2])
+ value = matches[2]
if profile:
if not profile_data[profile][hat].get('lvar', False):
@@ -3165,12 +3165,16 @@
def separate_vars(vs):
"""Returns a list of all the values for a variable"""
data = set()
+ vs = vs.strip()
- RE_VARS = re.compile('\s*((\".+?\")|([^\"]\S+))\s*(.*)$')
+ RE_VARS = re.compile('^(("[^"]*")|([^"\s]+))\s*(.*)$')
while RE_VARS.search(vs):
matches = RE_VARS.search(vs).groups()
data.add(strip_quotes(matches[0]))
- vs = matches[3]
+ vs = matches[3].strip()
+
+ if vs:
+ raise AppArmorException('Variable assignments contains invalid parts (unbalanced quotes?): %s' % vs)
return data
@@ -3298,6 +3302,8 @@
def var_transform(ref):
data = []
for value in ref:
+ if not value:
+ value = '""'
data.append(quote_if_needed(value))
return ' '.join(data)
=== modified file ./utils/test/test-aa.py
--- utils/test/test-aa.py 2015-11-15 20:30:42.251877998 +0100
+++ utils/test/test-aa.py 2015-11-15 20:32:26.799188737 +0100
@@ -17,7 +17,8 @@
from apparmor.aa import (check_for_apparmor, get_interpreter_and_abstraction, create_new_profile,
get_profile_flags, set_profile_flags, is_skippable_file, is_skippable_dir,
- parse_profile_start, parse_profile_data, separate_vars, store_list_var, write_header, serialize_parse_profile_start)
+ parse_profile_start, parse_profile_data, separate_vars, store_list_var, write_header,
+ var_transform, serialize_parse_profile_start)
from apparmor.common import AppArmorException, AppArmorBug
class AaTestWithTempdir(AATest):
@@ -484,20 +485,29 @@
('' , set() ),
(' ' , set() ),
(' foo bar' , {'foo', 'bar' }),
- ('foo " ' , {'foo' }), # XXX " is ignored
- (' " foo ' , {' "', 'foo' }), # XXX really?
+ ('foo " ' , AppArmorException ),
+ (' " foo ' , AppArmorException ), # half-quoted
(' foo bar ' , {'foo', 'bar' }),
- (' foo bar # comment' , {'foo', 'bar', 'comment' }), # XXX should comments be stripped?
+ (' 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 baz ' , AppArmorException ), # half-quoted
(' " foo" bar' , {' foo', 'bar' }),
+ (' " foo" bar x' , {' foo', 'bar', 'x' }),
+ ('""' , {'' }), # empty value
+ ('"" foo' , {'', 'foo' }), # empty value + 'foo'
+ ('"" foo "bar"' , {'', 'foo', 'bar' }), # empty value + 'foo' + 'bar' (bar has superfluous quotes)
+ ('"bar"' , {'bar' }), # 'bar' with superfluous quotes
]
def _run_test(self, params, expected):
- result = separate_vars(params)
- self.assertEqual(result, expected)
+ if expected == AppArmorException:
+ with self.assertRaises(expected):
+ separate_vars(params)
+ else:
+ result = separate_vars(params)
+ self.assertEqual(result, expected)
class AaTest_store_list_var(AATest):
@@ -579,6 +589,17 @@
result = write_header(prof_data, depth, name, embedded_hat, write_flags)
self.assertEqual(result, [expected])
+class AaTest_var_transform(AATest):
+ tests = [
+ (['foo', ''], 'foo ""' ),
+ (['foo', 'bar'], 'foo bar' ),
+ ([''], '""' ),
+ (['bar baz', 'foo'], '"bar baz" foo' ),
+ ]
+
+ def _run_test(self, params, expected):
+ self.assertEqual(var_transform(params), expected)
+
class AaTest_serialize_parse_profile_start(AATest):
def _parse(self, line, profile, hat, prof_data_profile, prof_data_external):
# 'correct' is always True in the code that uses serialize_parse_profile_start() (set some lines above the function call)
=== modified file ./utils/test/test-parser-simple-tests.py
--- utils/test/test-parser-simple-tests.py 2015-11-15 20:30:42.251877998 +0100
+++ utils/test/test-parser-simple-tests.py 2015-11-11 00:58:06.378155721 +0100
@@ -167,8 +167,6 @@
'vars/boolean/boolean_bad_6.sd',
'vars/boolean/boolean_bad_7.sd',
'vars/boolean/boolean_bad_8.sd',
- 'vars/vars_bad_1.sd',
- 'vars/vars_bad_2.sd',
'vars/vars_bad_3.sd',
'vars/vars_bad_4.sd',
'vars/vars_bad_5.sd',
@@ -178,7 +176,6 @@
'vars/vars_bad_trailing_comma_2.sd',
'vars/vars_bad_trailing_comma_3.sd',
'vars/vars_bad_trailing_comma_4.sd',
- 'vars/vars_bad_trailing_garbage_1.sd',
'vars/vars_dbus_bad_01.sd',
'vars/vars_dbus_bad_02.sd',
'vars/vars_dbus_bad_03.sd',
Regards,
Christian Boltz
--
We work *with* SUSE, but not *for* SUSE. Using @suse.de
would imply that to the world that we are somehow employed
by SUSE, and I haven't seen a paycheck from them yet. :-)
[Bryen M Yunashko in opensuse-project]
More information about the AppArmor
mailing list