From f997977e6bbcf0a3acebcb6fde7a1b983686939e Mon Sep 17 00:00:00 2001 From: Christian Boltz Date: Mon, 22 Oct 2018 23:56:07 +0200 Subject: [PATCH] Replace existing_profiles & fix minitools for named profiles Technical stuff first: Replace existing_profiles (a dict with the filenames for both active and inactive profiles) with active_profiles and extra_profiles which are ProfileList()s and store the active profiles and those in the extra directory separately. Thanks to ProfileList, now also the relation between attachments and filenames is easily available. Also replace all usage of existing_profiles with active_profiles and extra_profiles, and adjust it to the ProfileList syntax everywhere. With this change, several bugs in aa-complain and the other minitools get fixed: - aa-complain etc. never found profiles that have a profile name (the attachment wasn't checked) - even if the profile name was given as parameter to aa-complain, it first did "which $parameter" so it never matched on named profiles - profile names with alternations (without attachment specification) also never matched because the old code didn't use AARE. References: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=882047#92 (search for "As usual" ;-) Just for completeness - the matching still doesn't honor/expand variables in the profile name. PR: https://gitlab.com/apparmor/apparmor/merge_requests/268 (cherry picked from commit 4d722f18397dd35b208548d4c841b955c41ac7ce) Signed-off-by: John Johansen --- utils/aa-mergeprof | 4 +-- utils/apparmor/aa.py | 44 +++++++++++++---------- utils/apparmor/logparser.py | 13 +++---- utils/test/test-libapparmor-test_multi.py | 13 +++++-- 4 files changed, 45 insertions(+), 29 deletions(-) diff --git a/utils/aa-mergeprof b/utils/aa-mergeprof index 29a819cfa..f07b32149 100755 --- a/utils/aa-mergeprof +++ b/utils/aa-mergeprof @@ -1,7 +1,7 @@ #! /usr/bin/python3 # ---------------------------------------------------------------------- # Copyright (C) 2013 Kshitij Gupta -# Copyright (C) 2014-2017 Christian Boltz +# Copyright (C) 2014-2018 Christian Boltz # # This program is free software; you can redistribute it and/or # modify it under the terms of version 2 of the GNU General Public @@ -57,7 +57,7 @@ def reset_aa(): apparmor.aa.aa = apparmor.aa.hasher() apparmor.aa.filelist = apparmor.aa.hasher() apparmor.aa.include = dict() - apparmor.aa.existing_profiles = apparmor.aa.hasher() + apparmor.aa.active_profiles = apparmor.aa.ProfileList() apparmor.aa.original_aa = apparmor.aa.hasher() def find_profiles_from_files(files): diff --git a/utils/apparmor/aa.py b/utils/apparmor/aa.py index b23e698fb..bb3f2e3fa 100644 --- a/utils/apparmor/aa.py +++ b/utils/apparmor/aa.py @@ -1,6 +1,6 @@ # ---------------------------------------------------------------------- # Copyright (C) 2013 Kshitij Gupta -# Copyright (C) 2014-2017 Christian Boltz +# Copyright (C) 2014-2018 Christian Boltz # # This program is free software; you can redistribute it and/or # modify it under the terms of version 2 of the GNU General Public @@ -49,6 +49,8 @@ from apparmor.regex import (RE_PROFILE_START, RE_PROFILE_END, RE_PROFILE_LINK, RE_PROFILE_UNIX, RE_RULE_HAS_COMMA, RE_HAS_COMMENT_SPLIT, strip_quotes, parse_profile_start_line, re_match_include ) +from apparmor.profile_list import ProfileList + from apparmor.profile_storage import ProfileStorage, add_or_remove_flag, ruletypes, write_abi import apparmor.rules as aarules @@ -88,7 +90,8 @@ extra_profile_dir = None # To keep track of previously included profile fragments include = dict() -existing_profiles = dict() +active_profiles = ProfileList() +extra_profiles = ProfileList() # To store the globs entered by users so they can be provided again # format: user_globs['/foo*'] = AARE('/foo*') @@ -219,7 +222,7 @@ def find_executable(bin_path): def get_profile_filename_from_profile_name(profile, get_new=False): """Returns the full profile name for the given profile name""" - filename = get_profile_filename_orig(profile) + filename = active_profiles.filename_from_profile_name(profile) if filename: return filename @@ -229,18 +232,13 @@ def get_profile_filename_from_profile_name(profile, get_new=False): def get_profile_filename_from_attachment(profile, get_new=False): """Returns the full profile name for the given attachment""" - filename = get_profile_filename_orig(profile) + filename = active_profiles.filename_from_attachment(profile) if filename: return filename if get_new: return get_new_profile_filename(profile) -def get_profile_filename_orig(profile): - """Returns the full profile name""" - if existing_profiles.get(profile, False): - return existing_profiles[profile] - def get_new_profile_filename(profile): '''Compose filename for a new profile''' if profile.startswith('/'): @@ -526,7 +524,8 @@ def get_profile(prof_name): profile_hash[uname]['profile'] = serialize_profile(inactive_profile[prof_name], prof_name, None) profile_hash[uname]['profile_data'] = inactive_profile - existing_profiles.pop(prof_name) # remove profile filename from list to force storing in /etc/apparmor.d/ instead of extra_profile_dir + # no longer necessary after splitting active and extra profiles + # existing_profiles.pop(prof_name) # remove profile filename from list to force storing in /etc/apparmor.d/ instead of extra_profile_dir # If no profiles in repo and no inactive profiles if not profile_hash.keys(): @@ -714,15 +713,16 @@ def profile_exists(program): """Returns True if profile exists, False otherwise""" # Check cache of profiles - if existing_profiles.get(program, False): + if active_profiles.filename_from_attachment(program): return True # Check the disk for profile prof_path = get_profile_filename_from_attachment(program, True) #print(prof_path) if os.path.isfile(prof_path): # Add to cache of profile - existing_profiles[program] = prof_path - return True + raise AppArmorBug('Reached strange condition in profile_exists(), please open a bugreport!') + # active_profiles[program] = prof_path + # return True return False def sync_profile(): @@ -1792,7 +1792,7 @@ def set_logfile(filename): def do_logprof_pass(logmark='', passno=0, log_pid=log_pid): # set up variables for this pass # transitions = hasher() - global existing_profiles + global active_profiles global sev_db # aa = hasher() # profile_changes = hasher() @@ -1809,13 +1809,13 @@ def do_logprof_pass(logmark='', passno=0, log_pid=log_pid): if not sev_db: sev_db = apparmor.severity.Severity(CONFDIR + '/severity.db', _('unknown')) #print(pid) - #print(existing_profiles) + #print(active_profiles) ##if not repo_cf and cfg['repostory']['url']: ## repo_cfg = read_config('repository.conf') ## if not repo_cfg['repository'].get('enabled', False) or repo_cfg['repository]['enabled'] not in ['yes', 'no']: ## UI_ask_to_enable_repo() - log_reader = apparmor.logparser.ReadLog(log_pid, logfile, existing_profiles, profile_dir) + log_reader = apparmor.logparser.ReadLog(log_pid, logfile, active_profiles, profile_dir) log = log_reader.read_log(logmark) #read_log(logmark) @@ -2109,18 +2109,26 @@ def read_profile(file, active_profile): for profile in profile_data: # TODO: also honor hats name = profile_data[profile][profile]['name'] + attachment = profile_data[profile][profile]['attachment'] filename = profile_data[profile][profile]['filename'] - existing_profiles[name] = filename + if not attachment and name.startswith('/'): + active_profiles.add(filename, name, name) # use name as name and attachment + else: + active_profiles.add(filename, name, attachment) elif profile_data: attach_profile_data(extras, profile_data) for profile in profile_data: # TODO: also honor hats name = profile_data[profile][profile]['name'] + attachment = profile_data[profile][profile]['attachment'] filename = profile_data[profile][profile]['filename'] - existing_profiles[name] = filename + if not attachment and name.startswith('/'): + extra_profiles.add(filename, name, name) # use name as name and attachment + else: + extra_profiles.add(filename, name, attachment) def attach_profile_data(profiles, profile_data): # Make deep copy of data to avoid changes to diff --git a/utils/apparmor/logparser.py b/utils/apparmor/logparser.py index 0e74c3f52..d531358b9 100644 --- a/utils/apparmor/logparser.py +++ b/utils/apparmor/logparser.py @@ -1,6 +1,6 @@ # ---------------------------------------------------------------------- # Copyright (C) 2013 Kshitij Gupta -# Copyright (C) 2015-2016 Christian Boltz +# Copyright (C) 2015-2018 Christian Boltz # # This program is free software; you can redistribute it and/or # modify it under the terms of version 2 of the GNU General Public @@ -43,11 +43,11 @@ class ReadLog: # used to pre-filter log lines so that we hand over only relevant lines to LibAppArmor parsing RE_LOG_ALL = re.compile('(' + '|'.join(RE_log_parts) + ')') - def __init__(self, pid, filename, existing_profiles, profile_dir): + def __init__(self, pid, filename, active_profiles, profile_dir): self.filename = filename self.profile_dir = profile_dir self.pid = pid - self.existing_profiles = existing_profiles + self.active_profiles = active_profiles self.log = [] self.debug_logger = DebugLogger('ReadLog') self.LOG = None @@ -446,15 +446,16 @@ class ReadLog: def profile_exists(self, program): """Returns True if profile exists, False otherwise""" # Check cache of profiles - if self.existing_profiles.get(program, False): + if self.active_profiles.filename_from_profile_name(program): return True # Check the disk for profile prof_path = self.get_profile_filename(program) #print(prof_path) if os.path.isfile(prof_path): # Add to cache of profile - self.existing_profiles[program] = prof_path - return True + raise AppArmorBug('This should never happen, please open a bugreport!') + # self.active_profiles[program] = prof_path + # return True return False def get_profile_filename(self, profile): diff --git a/utils/test/test-libapparmor-test_multi.py b/utils/test/test-libapparmor-test_multi.py index feb8fcbda..27b0ebfe3 100644 --- a/utils/test/test-libapparmor-test_multi.py +++ b/utils/test/test-libapparmor-test_multi.py @@ -1,7 +1,7 @@ #! /usr/bin/python3 # ------------------------------------------------------------------ # -# Copyright (C) 2015 Christian Boltz +# Copyright (C) 2015-2018 Christian Boltz # # This program is free software; you can redistribute it and/or # modify it under the terms of version 2 of the GNU General Public @@ -17,6 +17,7 @@ from apparmor.common import open_file_read import apparmor.aa from apparmor.logparser import ReadLog +from apparmor.profile_list import ProfileList class TestLibapparmorTestMulti(AATest): '''Parse all libraries/libapparmor/testsuite/test_multi tests and compare the result with the *.out files''' @@ -222,9 +223,15 @@ class TestLogToProfile(AATest): if '//' in profile: profile, hat = profile.split('//') - apparmor.aa.existing_profiles = {profile: profile_dummy_file} + apparmor.aa.active_profiles = ProfileList() - log_reader = ReadLog(dict(), logfile, apparmor.aa.existing_profiles, '') + # optional for now, might be needed one day + # if profile.startswith('/'): + # apparmor.aa.active_profiles.add(profile_dummy_file, profile, profile) + # else: + apparmor.aa.active_profiles.add(profile_dummy_file, profile, '') + + log_reader = ReadLog(dict(), logfile, apparmor.aa.active_profiles, '') log = log_reader.read_log('') for root in log: