severity: replace load_variables() with set_variables()

Add set_variables() to severity.py to set the variables for severity
rating. It typically gets the data from the get_all_merged_variables()
result.

This replaces the slightly broken load_variables() that parsed profile
files for variables. (For example, parsing "@{foo} = /bar" resulted
in a variable name "@{foo} " with trailing space.)

Also adjust aa.py and the severity tests to use set_variables() (with
get_all_merged_variables()) instead of load_variables().

This also re-adds the checks that were removed in the "Store variables
in active_profiles (ProfileList)" commit earlier, while still fixing
lp:1331856.

With this change, unload_variables() becomes useless (the variables get
overwritten in set_variables() anyway), drop it and its calls.

Note that load_variables() silently ignored non-existing files while the
get_all_merged_variables() call only works for existing files that are
known to active_profiles. Since the input of ask_the_questions() and
ask_exec() comes from log_dict (= audit.log or a profile to merge), add
a check if that profile actually exists in the set of active profiles.

Also adjust the severity tests to use set_variables().

Finally, drop the tests that check for handling non-existing include
files, redefining and adding to non-existing variables - all these
things get now handled in include_list_recursive() and
get_all_merged_variables() and their tests.

Fixes: https://bugs.launchpad.net/apparmor/+bug/1331856

utils/apparmor/aa.py

@@ -852,8 +852,12 @@ def ask_exec(hashlog):
                         #
                         parent_uses_ld_xxx = check_for_LD_XXX(profile)
 
-                        sev_db.unload_variables()
+                        prof_filename = get_profile_filename_from_profile_name(profile)
-                        sev_db.load_variables(get_profile_filename_from_profile_name(profile, True))
+                        if prof_filename and active_profiles.files.get(prof_filename):
+                            sev_db.set_variables(active_profiles.get_all_merged_variables(prof_filename, include_list_recursive(active_profiles.files[prof_filename]), profile_dir))
+                        else:
+                            sev_db.set_variables( {} )
+
                         severity = sev_db.rank_path(exec_target, 'x')
 
                         # Prompt portion starts
@@ -1044,8 +1048,11 @@ def ask_the_questions(log_dict):
             raise AppArmorBug(_('Invalid mode found: %s') % aamode)
 
         for profile in sorted(log_dict[aamode].keys()):
-            sev_db.unload_variables()
+            prof_filename = get_profile_filename_from_profile_name(profile)
-            sev_db.load_variables(get_profile_filename_from_profile_name(profile, True))
+            if prof_filename and active_profiles.files.get(prof_filename):
+                sev_db.set_variables(active_profiles.get_all_merged_variables(prof_filename, include_list_recursive(active_profiles.files[prof_filename]), profile_dir))
+            else:
+                sev_db.set_variables( {} )
 
             # Sorted list of hats with the profile name coming first
             hats = list(filter(lambda key: key != profile, sorted(log_dict[aamode][profile].keys())))

utils/apparmor/severity.py

@@ -12,10 +12,8 @@
 #
 # ----------------------------------------------------------------------
 from __future__ import with_statement
-import os
 import re
 from apparmor.common import AppArmorException, open_file_read, warn, convert_regexp  # , msg, error, debug
-from apparmor.regex import re_match_include
 
 class Severity(object):
     def __init__(self, dbname=None, default_rank=10):
@@ -176,37 +174,6 @@ class Severity(object):
             replacement = replacement[:-1]
         return resource.replace(variable, replacement)
 
-    def load_variables(self, prof_path):
+    def set_variables(self, vars):
-        """Loads the variables for the given profile"""
+        ''' Set the profile variables to use for rating the severity '''
-        if os.path.isfile(prof_path):
+        self.severity['VARIABLES'] = vars
-            with open_file_read(prof_path) as f_in:
-                for line in f_in:
-                    line = line.strip()
-                    # If any includes, load variables from them first
-                    match = re_match_include(line)
-                    if match:
-                        new_path = match
-                        if not new_path.startswith('/'):
-                            new_path = self.PROF_DIR + '/' + match
-                        self.load_variables(new_path)
-                    else:
-                        # Remove any comments
-                        if '#' in line:
-                            line = line.split('#')[0].rstrip()
-                        # Expected format is @{Variable} = value1 value2 ..
-                        if line.startswith('@') and '=' in line:
-                            if '+=' in line:
-                                line = line.split('+=')
-                                try:
-                                    self.severity['VARIABLES'][line[0]] += [i.strip('"') for i in line[1].split()]
-                                except KeyError:
-                                    raise AppArmorException("Variable %s was not previously declared, but is being assigned additional value in file: %s" % (line[0], prof_path))
-                            else:
-                                line = line.split('=')
-                                if line[0] in self.severity['VARIABLES'].keys():
-                                    raise AppArmorException("Variable %s was previously declared in file: %s" % (line[0], prof_path))
-                                self.severity['VARIABLES'][line[0]] = [i.strip('"') for i in line[1].split()]
-
-    def unload_variables(self):
-        """Clears all loaded variables"""
-        self.severity['VARIABLES'] = dict()

utils/test/test-profile-list.py

@@ -355,6 +355,10 @@ class AaTest_get_all_merged_variables(AATest):
         with self.assertRaises(AppArmorException):
             apparmor.aa.active_profiles.get_all_merged_variables(os.path.join(self.profile_dir, 'usr.sbin.dnsmasq'), apparmor.aa.include_list_recursive(apparmor.aa.active_profiles.files[prof_filename]), self.profile_dir)
 
+    def test_vars_from_nonexisting_profile(self):
+        with self.assertRaises(AppArmorBug):
+            apparmor.aa.active_profiles.get_all_merged_variables(os.path.join(self.profile_dir, 'file.not.found'), list(), self.profile_dir)
+
 
 setup_aa(apparmor.aa)
 setup_all_loops(__name__)

utils/test/test-severity.py

@@ -75,32 +75,6 @@ class SeverityTestCap(SeverityBaseTest):
 
 
 class SeverityVarsTest(SeverityBaseTest):
-
-    VARIABLE_DEFINITIONS = '''
-@{HOME}=@{HOMEDIRS}/*/ /root/
-@{HOMEDIRS}=/home/
-# add another path to @{HOMEDIRS}
-@{HOMEDIRS}+=/storage/
-@{multiarch}=*-linux-gnu*
-@{TFTP_DIR}=/var/tftp /srv/tftpboot
-@{PROC}=/proc/
-@{pid}={[1-9],[1-9][0-9],[1-9][0-9][0-9],[1-9][0-9][0-9][0-9],[1-9][0-9][0-9][0-9][0-9],[1-9][0-9][0-9][0-9][0-9][0-9]}
-@{tid}=@{pid}
-@{pids}=@{pid}
-@{somepaths}=/home/foo/downloads @{HOMEDIRS}/foo/.ssh/
-'''
-
-    def _init_tunables(self, content=''):
-        if not content:
-            content = self.VARIABLE_DEFINITIONS
-
-        self.rules_file = self.writeTmpfile('tunables', content)
-
-        self.sev_db.load_variables(self.rules_file)
-
-    def AATeardown(self):
-        self.sev_db.unload_variables()
-
     tests = [
         (['@{PROC}/sys/vm/overcommit_memory',           'r'],    6),
         (['@{HOME}/sys/@{PROC}/overcommit_memory',      'r'],    4),
@@ -110,22 +84,17 @@ class SeverityVarsTest(SeverityBaseTest):
     ]
 
     def _run_test(self, params, expected):
-        self._init_tunables()
+        vars = {
+            '@{HOME}':      {'@{HOMEDIRS}/*/', '/root/'},
+            '@{HOMEDIRS}':  {'/home/', '/storage/'},
+            '@{multiarch}': {'*-linux-gnu*'},
+            '@{TFTP_DIR}':  {'/var/tftp /srv/tftpboot'},
+            '@{PROC}':      {'/proc/'},
+            '@{somepaths}': {'/home/foo/downloads', '@{HOMEDIRS}/foo/.ssh/'},
+        }
+        self.sev_db.set_variables(vars)
         self._simple_severity_w_perm(params[0], params[1], expected)
 
-    def test_include(self):
-        self._init_tunables('#include <file/not/found>')  # including non-existing files doesn't raise an exception
-
-        self.assertTrue(True)  # this test only makes sure that loading the tunables file works
-
-    def test_invalid_variable_add(self):
-        with self.assertRaises(AppArmorException):
-            self._init_tunables('@{invalid} += /home/')
-
-    def test_invalid_variable_double_definition(self):
-        with self.assertRaises(AppArmorException):
-            self._init_tunables('@{foo} = /home/\n@{foo} = /root/')
-
 class SeverityDBTest(AATest):
     def _test_db(self, contents):
         self.db_file = self.writeTmpfile('severity.db', contents)