From 699d7b5c83fa4a832a8e55131e966e2c46f93791 Mon Sep 17 00:00:00 2001 From: Georgia Garcia Date: Fri, 28 Mar 2025 17:52:23 -0300 Subject: [PATCH] utils: fix cleanprof regression on header generation Commit c9d41a3ebb introduced a regression on profile header generation. This commit removes the name parameter from the get_header function since the ProfileStorage should already contain all the information required to generate the header for profiles and hats. The tests needed to be updated as well to make sure the ProfileStorage object contained the information needed by the get_header method. Fixes: c9d41a3ebb ("utils: fix profile and hat header generation") Signed-off-by: Georgia Garcia --- utils/apparmor/aa.py | 18 +++++++----------- utils/apparmor/profile_storage.py | 9 ++++----- utils/test/test-libapparmor-test_multi.py | 5 +++-- utils/test/test-profile-storage.py | 8 ++++---- 4 files changed, 18 insertions(+), 22 deletions(-) diff --git a/utils/apparmor/aa.py b/utils/apparmor/aa.py index fc1ef2b98..0f60f9c05 100644 --- a/utils/apparmor/aa.py +++ b/utils/apparmor/aa.py @@ -383,7 +383,7 @@ def get_interpreter_and_abstraction(exec_target): def create_new_profile(localfile, is_stub=False): local_profile = {} - local_profile[localfile] = ProfileStorage('NEW', localfile, 'create_new_profile()') + local_profile[localfile] = ProfileStorage(localfile, localfile, 'create_new_profile()') local_profile[localfile]['flags'] = 'complain' if os.path.join(profile_dir, 'abstractions/base') in include: @@ -413,7 +413,7 @@ def create_new_profile(localfile, is_stub=False): for hat in sorted(cfg['required_hats'][hatglob].split()): full_hat = combine_profname((localfile, hat)) if not local_profile.get(full_hat, False): - local_profile[full_hat] = ProfileStorage('NEW', hat, 'create_new_profile() required_hats') + local_profile[full_hat] = ProfileStorage(localfile, hat, 'create_new_profile() required_hats') local_profile[full_hat]['parent'] = localfile local_profile[full_hat]['is_hat'] = True local_profile[full_hat]['flags'] = 'complain' @@ -577,7 +577,7 @@ def change_profile_flags(prof_filename, program, flag, set_flag): prof_storage['flags'] = newflags - line = prof_storage.get_header(depth, profile, False) + line = prof_storage.get_header(depth, False) line = '%s\n' % line[0] elif RE_PROFILE_HAT_DEF.search(line): depth += 1 @@ -587,7 +587,7 @@ def change_profile_flags(prof_filename, program, flag, set_flag): newflags = ', '.join(add_or_remove_flag(old_flags, flag, set_flag)) prof_storage['flags'] = newflags - line = prof_storage.get_header(depth, profile, False) + line = prof_storage.get_header(depth, False) line = '%s\n' % line[0] elif RE_PROFILE_END.search(line): depth -= 1 @@ -1938,15 +1938,11 @@ def merged_to_split(profile_data): def write_piece(profile_data, depth, name, nhat): pre = ' ' * depth data = [] - wname = None inhat = False - if name == nhat: - wname = name - else: - wname = name + '//' + nhat + if name != nhat: name = nhat inhat = True - data += profile_data[name].get_header(depth, wname, False) + data += profile_data[name].get_header(depth, False) data += profile_data[name].get_rules_clean(depth + 1) pre2 = ' ' * (depth + 1) @@ -1964,7 +1960,7 @@ def write_piece(profile_data, depth, name, nhat): if not profile_data[hat]['external']: data.append('') - data += profile_data[hat].get_header(depth + 1, only_hat, True) + data += profile_data[hat].get_header(depth + 1, True) data += profile_data[hat].get_rules_clean(depth + 2) diff --git a/utils/apparmor/profile_storage.py b/utils/apparmor/profile_storage.py index e3d35bd4e..d4345203f 100644 --- a/utils/apparmor/profile_storage.py +++ b/utils/apparmor/profile_storage.py @@ -69,7 +69,6 @@ class ProfileStorage: def __init__(self, profilename, hat, calledby): data = dict() - # self.data['info'] isn't used anywhere, but can be helpful in debugging. data['info'] = {'profile': profilename, 'hat': hat, 'calledby': calledby} for rule in ruletypes: @@ -128,7 +127,7 @@ class ProfileStorage: def __repr__(self): classname = type(self).__name__ - header = '\n'.join(self.get_header(0, self['name'], False)) + header = '\n'.join(self.get_header(0, False)) rules = '\n'.join(self.get_rules_clean(1)) endprofile = '}' return f'\n<{classname}>\n{header}\n{rules}\n{endprofile}\n\n' @@ -139,11 +138,11 @@ class ProfileStorage: else: raise AppArmorBug('attempt to read unknown key %s' % key) - def get_header(self, depth, name, embedded_hat): + def get_header(self, depth, embedded_hat): pre = ' ' * int(depth * 2) data = [] - unquoted_name = name - name = quote_if_needed(name) + unquoted_name = self.data['info']['profile'] + name = quote_if_needed(self.data['info']['profile']) attachment = '' if self.data['attachment']: diff --git a/utils/test/test-libapparmor-test_multi.py b/utils/test/test-libapparmor-test_multi.py index 1e85f4bc9..ff41ebcfd 100644 --- a/utils/test/test-libapparmor-test_multi.py +++ b/utils/test/test-libapparmor-test_multi.py @@ -222,7 +222,7 @@ def logfile_to_profile(logfile): full_profile = parsed_event['profile'] profile, hat = split_name(full_profile) - dummy_prof = apparmor.aa.ProfileStorage('TEST DUMMY for active_profiles', profile_dummy_file, 'logprof_to_profile()') + dummy_prof = apparmor.aa.ProfileStorage(profile, hat, 'logprof_to_profile()') # optional for now, might be needed one day # if profile.startswith('/'): @@ -256,8 +256,9 @@ def logfile_to_profile(logfile): # initialize parent profile in log_dict as ProfileStorage to ensure writing the profile doesn't fail # (in "normal" usage outside of this test, log_dict will not be handed over to serialize_profile()) - log_dict[aamode][profile] = apparmor.aa.ProfileStorage('TEST DUMMY for empty parent profile', profile_dummy_file, 'logfile_to_profile()') + log_dict[aamode][profile] = apparmor.aa.ProfileStorage(profile, '', 'logfile_to_profile()') log_dict[aamode][parsed_event['profile']]['is_hat'] = True # for historical reasons, generate hats, not child profiles + log_dict[aamode][parsed_event['profile']]['parent'] = profile log_is_empty = True diff --git a/utils/test/test-profile-storage.py b/utils/test/test-profile-storage.py index e37185f78..93c981cb9 100644 --- a/utils/test/test-profile-storage.py +++ b/utils/test/test-profile-storage.py @@ -79,7 +79,7 @@ class AaTest_get_header(AATest): prof_storage['profile_keyword'] = params[6] prof_storage['header_comment'] = params[7] - result = prof_storage.get_header(depth, name, embedded_hat) + result = prof_storage.get_header(depth, embedded_hat) self.assertEqual(result, [expected]) @@ -103,7 +103,7 @@ class AaTest_get_header_01(AATest): if params.get(param) is not None: prof_storage[param] = params[param] - result = prof_storage.get_header(depth, name, embedded_hat) + result = prof_storage.get_header(depth, embedded_hat) self.assertEqual(result, [expected]) @@ -166,7 +166,7 @@ class AaTest_get_header_after_parse(AATest): def _run_test(self, params, expected): (profile, hat, prof_storage) = ProfileStorage.parse(params[0], 'somefile', 1, params[1], params[2]) - header = prof_storage.get_header(params[4], profile, params[3]) + header = prof_storage.get_header(params[4], params[3]) self.assertEqual(header, [expected], prof_storage.data) @@ -194,7 +194,7 @@ class TestSetInvalid(AATest): class AaTest_repr(AATest): def testRepr(self): - prof_storage = ProfileStorage('/test/foo', 'hat', 'TEST') + prof_storage = ProfileStorage('foo', 'hat', 'TEST') prof_storage['name'] = 'foo' prof_storage['xattrs'] = 'user.bar=bar' prof_storage['capability'].add(CapabilityRule('dac_override'))