2
0
mirror of https://gitlab.com/apparmor/apparmor synced 2025-09-02 07:15:18 +00:00

store include rules (also) in 'inc_ie' (IncludeRueset)

... and write them (only) from 'inc_ie' (IncludeRuleset), which can
handle both "include" and "include if exists" rules.

This duplicates storage of include rules because 'include' is still used
and needed at various places that work on/with the include rules.

With this, we get removal of duplicate include lines insinde a profile
in aa-cleanprof "for free" - extend cleanprof_test.in to confirm this.
This commit is contained in:
Christian Boltz
2020-05-13 21:45:00 +02:00
parent a7a727b1b0
commit 23af115fa5
5 changed files with 40 additions and 21 deletions

View File

@@ -50,7 +50,7 @@ from apparmor.regex import (RE_PROFILE_START, RE_PROFILE_END,
from apparmor.profile_list import ProfileList from apparmor.profile_list import ProfileList
from apparmor.profile_storage import (ProfileStorage, add_or_remove_flag, ruletypes, from apparmor.profile_storage import (ProfileStorage, add_or_remove_flag, ruletypes,
write_includes, write_list_vars ) write_list_vars )
import apparmor.rules as aarules import apparmor.rules as aarules
@@ -443,6 +443,8 @@ def create_new_profile(localfile, is_stub=False):
local_profile[localfile] = ProfileStorage('NEW', localfile, 'create_new_profile()') local_profile[localfile] = ProfileStorage('NEW', localfile, 'create_new_profile()')
local_profile[localfile]['flags'] = 'complain' local_profile[localfile]['flags'] = 'complain'
local_profile[localfile]['include']['abstractions/base'] = 1 local_profile[localfile]['include']['abstractions/base'] = 1
# currently store in both 'include' (above) and 'inc_ie' (below). TODO: Drop 'include' once all code using it is migrated.
local_profile[localfile]['inc_ie'].add(IncludeRule('abstractions/base', False, True))
if os.path.exists(localfile) and os.path.isfile(localfile): if os.path.exists(localfile) and os.path.isfile(localfile):
interpreter_path, abstraction = get_interpreter_and_abstraction(localfile) interpreter_path, abstraction = get_interpreter_and_abstraction(localfile)
@@ -453,6 +455,8 @@ def create_new_profile(localfile, is_stub=False):
if abstraction: if abstraction:
local_profile[localfile]['include'][abstraction] = True local_profile[localfile]['include'][abstraction] = True
# currently store in both 'include' (above) and 'inc_ie' (below). TODO: Drop 'include' once all code using it is migrated.
local_profile[localfile]['inc_ie'].add(IncludeRule(abstraction, False, True))
handle_binfmt(local_profile[localfile], interpreter_path) handle_binfmt(local_profile[localfile], interpreter_path)
else: else:
@@ -571,6 +575,8 @@ def autodep(bin_name, pname=''):
if not filelist.get(file, False): if not filelist.get(file, False):
filelist[file] = hasher() filelist[file] = hasher()
filelist[file]['include']['tunables/global'] = True filelist[file]['include']['tunables/global'] = True
# currently store in both 'include' (above) and 'inc_ie' (below). TODO: Drop 'include' once all code using it is migrated.
active_profiles.add_inc_ie(file, IncludeRule('tunables/global', False, True))
write_profile_ui_feedback(pname) write_profile_ui_feedback(pname)
def get_profile_flags(filename, program): def get_profile_flags(filename, program):
@@ -946,6 +952,8 @@ def ask_exec(hashlog):
if abstraction: if abstraction:
aa[profile][hat]['include'][abstraction] = True aa[profile][hat]['include'][abstraction] = True
# currently store in both 'include' (above) and 'inc_ie' (below). TODO: Drop 'include' once all code using it is migrated.
aa[profile][hat]['inc_ie'].add(IncludeRule(abstraction, False, True))
handle_binfmt(aa[profile][hat], interpreter_path) handle_binfmt(aa[profile][hat], interpreter_path)
@@ -1223,6 +1231,8 @@ def ask_rule_questions(prof_events, profile_name, the_profile, r_types):
deleted = delete_all_duplicates(the_profile, inc, r_types) deleted = delete_all_duplicates(the_profile, inc, r_types)
the_profile['include'][inc] = True the_profile['include'][inc] = True
# currently store in both 'include' (above) and 'inc_ie' (below). TODO: Drop 'include' once all code using it is migrated.
the_profile['inc_ie'].add(IncludeRule.parse(selection))
aaui.UI_Info(_('Adding %s to profile.') % selection) aaui.UI_Info(_('Adding %s to profile.') % selection)
if deleted: if deleted:
@@ -1955,10 +1965,15 @@ def parse_profile_data(data, file, do_include):
if profile: if profile:
profile_data[profile][hat]['include'][include_name] = True profile_data[profile][hat]['include'][include_name] = True
# currently store in both 'include' (above) and 'inc_ie' (below). TODO: Drop 'include' once all code using it is migrated.
profile_data[profile][hat]['inc_ie'].add(IncludeRule.parse(line))
else: else:
if not filelist.get(file): if not filelist.get(file):
filelist[file] = hasher() filelist[file] = hasher()
filelist[file]['include'][include_name] = True filelist[file]['include'][include_name] = True
# currently store in both 'include' (above) and 'inc_ie' (below). TODO: Drop 'include' once all code using it is migrated.
active_profiles.add_inc_ie(file, IncludeRule.parse(line))
# If include is a directory # If include is a directory
if os.path.isdir(get_include_path(include_name)): if os.path.isdir(get_include_path(include_name)):
for file_name in include_dir_filelist(profile_dir, include_name): for file_name in include_dir_filelist(profile_dir, include_name):
@@ -2298,7 +2313,6 @@ def serialize_profile(profile_data, name, options):
if filelist.get(prof_filename, False): if filelist.get(prof_filename, False):
data += active_profiles.get_clean_first(prof_filename, 0) data += active_profiles.get_clean_first(prof_filename, 0)
data += write_list_vars(filelist[prof_filename], 0) data += write_list_vars(filelist[prof_filename], 0)
data += write_includes(filelist[prof_filename], 0)
data += active_profiles.get_clean(prof_filename, 0) data += active_profiles.get_clean(prof_filename, 0)

View File

@@ -219,21 +219,7 @@ def write_list_vars(ref, depth):
return data return data
def write_includes(prof_data, depth): def write_includes(prof_data, depth):
pre = ' ' * depth return [] # now handled via 'inc_ie' / IncludeRulset
data = []
for key in sorted(prof_data['include'].keys()):
if key.startswith('/'):
qkey = '"%s"' % key
else:
qkey = '<%s>' % quote_if_needed(key)
data.append('%s#include %s' % (pre, qkey))
if data:
data.append('')
return data
def var_transform(ref): def var_transform(ref):
data = [] data = []

View File

@@ -17,6 +17,8 @@
#include <abstractions/base> #include <abstractions/base>
#include if exists <foo> #include if exists <foo>
#include if exists <abstractions/base>
include <abstractions/base>
capability sys_admin, capability sys_admin,
audit capability, audit capability,

View File

@@ -5,8 +5,7 @@ alias /foo -> /bar,
@{asdf} = "" foo @{asdf} = "" foo
@{xy} = x y @{xy} = x y
#include <tunables/global> include <tunables/global>
include if exists <tunables/nothing> include if exists <tunables/nothing>
# A simple test comment which will persist # A simple test comment which will persist
@@ -15,8 +14,7 @@ include if exists <tunables/nothing>
/usr/bin/a/simple/cleanprof/test/profile { /usr/bin/a/simple/cleanprof/test/profile {
abi "abi/4.20", abi "abi/4.20",
#include <abstractions/base> include <abstractions/base>
include if exists <foo> include if exists <foo>
set rlimit nofile <= 256, set rlimit nofile <= 256,

View File

@@ -25,6 +25,7 @@ from apparmor.aa import (check_for_apparmor, get_output, get_reqs, get_interpret
from apparmor.aare import AARE from apparmor.aare import AARE
from apparmor.common import AppArmorException, AppArmorBug from apparmor.common import AppArmorException, AppArmorBug
from apparmor.rule.file import FileRule from apparmor.rule.file import FileRule
from apparmor.rule.include import IncludeRule
class AaTestWithTempdir(AATest): class AaTestWithTempdir(AATest):
def AASetup(self): def AASetup(self):
@@ -150,8 +151,12 @@ class AaTest_create_new_profile(AATest):
if exp_abstraction: if exp_abstraction:
self.assertEqual(set(profile[program][program]['include'].keys()), {exp_abstraction, 'abstractions/base'}) self.assertEqual(set(profile[program][program]['include'].keys()), {exp_abstraction, 'abstractions/base'})
# currently stored in both 'include' (above) and 'inc_ie' (below). TODO: Drop 'include' once all code using it is migrated.
self.assertEqual(profile[program][program]['inc_ie'].get_clean(), ['include <abstractions/base>', 'include <%s>' % exp_abstraction, ''])
else: else:
self.assertEqual(set(profile[program][program]['include'].keys()), {'abstractions/base'}) self.assertEqual(set(profile[program][program]['include'].keys()), {'abstractions/base'})
# currently stored in both 'include' (above) and 'inc_ie' (below). TODO: Drop 'include' once all code using it is migrated.
self.assertEqual(profile[program][program]['inc_ie'].get_clean(), ['include <abstractions/base>', ''])
class AaTest_get_interpreter_and_abstraction(AATest): class AaTest_get_interpreter_and_abstraction(AATest):
tests = [ tests = [
@@ -832,6 +837,10 @@ class AaTest_get_file_perms_2(AATest):
profile['include']['abstractions/base'] = True profile['include']['abstractions/base'] = True
profile['include']['abstractions/bash'] = True profile['include']['abstractions/bash'] = True
profile['include']['abstractions/enchant'] = True # includes abstractions/aspell profile['include']['abstractions/enchant'] = True # includes abstractions/aspell
# currently stored in both 'include' (above) and 'inc_ie' (below). TODO: Drop 'include' once all code using it is migrated.
profile['inc_ie'].add(IncludeRule.parse('include <abstractions/base>'))
profile['inc_ie'].add(IncludeRule.parse('include <abstractions/bash>'))
profile['inc_ie'].add(IncludeRule.parse('include <abstractions/enchant>'))
profile['file'].add(FileRule.parse('owner /usr/share/common-licenses/** w,')) 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('owner /usr/share/common-licenses/what/ever a,')) # covered by the above 'w' rule, so 'a' should be ignored
@@ -874,6 +883,11 @@ class AaTest_propose_file_rules(AATest):
profile['include']['abstractions/base'] = True profile['include']['abstractions/base'] = True
profile['include']['abstractions/bash'] = True profile['include']['abstractions/bash'] = True
profile['include']['abstractions/enchant'] = True # includes abstractions/aspell profile['include']['abstractions/enchant'] = True # includes abstractions/aspell
# currently stored in both 'include' (above) and 'inc_ie' (below). TODO: Drop 'include' once all code using it is migrated.
profile['inc_ie'].add(IncludeRule.parse('include <abstractions/base>'))
profile['inc_ie'].add(IncludeRule.parse('include <abstractions/bash>'))
profile['inc_ie'].add(IncludeRule.parse('include <abstractions/enchant>'))
profile['file'].add(FileRule.parse('owner /usr/share/common-licenses/** w,')) profile['file'].add(FileRule.parse('owner /usr/share/common-licenses/** w,'))
profile['file'].add(FileRule.parse('/dev/null rwk,')) profile['file'].add(FileRule.parse('/dev/null rwk,'))
@@ -919,6 +933,11 @@ class AaTest_propose_file_rules_with_absolute_includes(AATest):
profile['include'][abs_include1] = False profile['include'][abs_include1] = False
profile['include'][abs_include2] = False profile['include'][abs_include2] = False
profile['include'][abs_include3] = False profile['include'][abs_include3] = False
# currently stored in both 'include' (above) and 'inc_ie' (below). TODO: Drop 'include' once all code using it is migrated.
profile['inc_ie'].add(IncludeRule.parse('include <abstractions/base>'))
profile['inc_ie'].add(IncludeRule.parse('include "%s"' % abs_include1))
profile['inc_ie'].add(IncludeRule.parse('include "%s"' % abs_include2))
profile['inc_ie'].add(IncludeRule.parse('include "%s"' % abs_include3))
rule_obj = FileRule(params[0], params[1], None, FileRule.ALL, owner=False, log_event=True) rule_obj = FileRule(params[0], params[1], None, FileRule.ALL, owner=False, log_event=True)
proposals = propose_file_rules(profile, rule_obj) proposals = propose_file_rules(profile, rule_obj)