From e99dda15f467da722069237cc2dac9848a1271d3 Mon Sep 17 00:00:00 2001 From: Christian Boltz Date: Sun, 31 Mar 2024 18:53:12 +0200 Subject: [PATCH] Stop calling ldd in aa-genprof and aa-autodep In some cases, ldd might obtain information by executing the given binary (see ldd(1)) - which is not something we should do on potentially unknown binaries, especially because aa-genprof and aa-autodep (and therefore also ldd) are often started as root. Additionally, the ldd result typically listed libraries already covered by abstractions/base, which makes the ldd call superfluous. While on it, - remove all references to ldd - remove code only used for calling ldd and handling its results - remove tests checking ldd results, and the fake_ldd script - adjust a test where fake_ldd had added some libraries - remove ldd path from logprof.conf [settings] --- utils/aa-autodep.pod | 3 +- utils/apparmor/aa.py | 80 --------------------------------------- utils/logprof.conf | 3 +- utils/test/fake_ldd | 60 ----------------------------- utils/test/logprof.conf | 1 - utils/test/test-aa.py | 35 +---------------- utils/test/test-config.py | 2 +- 7 files changed, 5 insertions(+), 179 deletions(-) delete mode 100755 utils/test/fake_ldd diff --git a/utils/aa-autodep.pod b/utils/aa-autodep.pod index 109eba266..f3b419167 100644 --- a/utils/aa-autodep.pod +++ b/utils/aa-autodep.pod @@ -45,8 +45,7 @@ B is used to generate a minimal AppArmor profile for a set of executables. This program will generate a profile for binary executable as well as interpreted script programs. At a minimum aa-autodep will provide a base profile containing a base include directive which includes basic -profile entries needed by most programs. The profile is generated by -recursively calling ldd(1) on the executables listed on the command line. +profile entries needed by most programs. The I<--force> option will overwrite any existing profile for the executable with the newly generated minimal AppArmor profile. diff --git a/utils/apparmor/aa.py b/utils/apparmor/aa.py index a876ba143..41bed1922 100644 --- a/utils/apparmor/aa.py +++ b/utils/apparmor/aa.py @@ -17,7 +17,6 @@ import atexit import os import re import shutil -import subprocess # nosec import sys import time import traceback @@ -360,81 +359,6 @@ def check_output_dir(output_dir): ) -def get_output(params): - """Runs the program with the given args and returns the return code and stdout (as list of lines)""" - try: - # Get the output of the program - output = subprocess.check_output(params) # nosec - ret = 0 - except OSError as e: - raise AppArmorException( - _("Unable to fork: %(program)s\n\t%(error)s") - % {'program': params[0], 'error': str(e)}) - except subprocess.CalledProcessError as e: # If exit code != 0 - output = e.output - ret = e.returncode - - output = output.decode('utf-8').split('\n') - - # Remove the extra empty string caused due to \n if present - if not output[-1]: - output.pop() - - return (ret, output) - - -def get_reqs(file): - """Returns a list of paths from ldd output""" - pattern1 = re.compile(r'^\s*\S+ => (/\S+)') - pattern2 = re.compile(r'^\s*(/\S+)') - reqs = [] - - ldd = conf.find_first_file(cfg['settings'].get('ldd')) or '/usr/bin/ldd' - if not os.path.isfile(ldd) or not os.access(ldd, os.EX_OK): - raise AppArmorException("Can't find ldd") - - ret, ldd_out = get_output((ldd, file)) - if ret == 0 or ret == 1: - for line in ldd_out: - if 'not a dynamic executable' in line: # comes with ret == 1 - break - if 'cannot read header' in line: - break - if 'statically linked' in line: - break - match = pattern1.search(line) - if match: - reqs.append(match.groups()[0]) - else: - match = pattern2.search(line) - if match: - reqs.append(match.groups()[0]) - return reqs - - -def handle_binfmt(profile, path): - """Modifies the profile to add the requirements""" - reqs_processed = dict() - reqs = get_reqs(path) - while reqs: - library = reqs.pop() - library = get_full_path(library) # resolve symlinks - if not reqs_processed.get(library, False): - if get_reqs(library): - reqs.extend(get_reqs(library)) - reqs_processed[library] = True - - library_rule = FileRule(library, 'mr', None, FileRule.ALL, owner=False, log_event=True) - - if not is_known_rule(profile, 'file', library_rule): - globbed_library = glob_common(library) - if globbed_library: - # glob_common returns a list, just use the first element (typically '/lib/libfoo.so.*') - library_rule = FileRule(globbed_library[0], 'mr', None, FileRule.ALL, owner=False) - - profile['file'].add(library_rule) - - def get_interpreter_and_abstraction(exec_target): """Check if exec_target is a script. If a hashbang is found, check if we have an abstraction for it. @@ -497,11 +421,9 @@ def create_new_profile(localfile, is_stub=False): else: aaui.UI_Important(_("WARNING: Can't find %s, therefore not adding it to the new profile.") % abstraction) - handle_binfmt(local_profile[localfile], interpreter_path) else: local_profile[localfile]['file'].add(FileRule(localfile, 'mr', None, FileRule.ALL, owner=False)) - handle_binfmt(local_profile[localfile], localfile) # Add required hats to the profile if they match the localfile for hatglob in cfg['required_hats'].keys(): if re.search(hatglob, localfile): @@ -1042,8 +964,6 @@ def ask_exec(hashlog): if not aa[profile][hat]['inc_ie'].is_covered(abstraction_rule): aa[profile][hat]['inc_ie'].add(abstraction_rule) - handle_binfmt(aa[profile][hat], interpreter_path) - # Update tracking info based on kind of change if ans == 'CMD_ix': diff --git a/utils/logprof.conf b/utils/logprof.conf index 64d3f6f1e..f458c8a3e 100644 --- a/utils/logprof.conf +++ b/utils/logprof.conf @@ -11,11 +11,10 @@ [settings] profiledir = /etc/apparmor.d /etc/subdomain.d - inactive_profiledir = /usr/share/apparmor/extra-profiles + inactive_profiledir = /usr/share/apparmor/extra-profiles logfiles = /var/log/audit/audit.log /var/log/syslog /var/log/messages parser = /sbin/apparmor_parser /sbin/subdomain_parser - ldd = /usr/bin/ldd logger = /bin/logger /usr/bin/logger # customize how file ownership permissions are presented diff --git a/utils/test/fake_ldd b/utils/test/fake_ldd deleted file mode 100755 index 4d02962d8..000000000 --- a/utils/test/fake_ldd +++ /dev/null @@ -1,60 +0,0 @@ -#!/usr/bin/python3 - -import sys - -if len(sys.argv) != 2: - raise Exception('wrong number of arguments in fake_ldd') - -if sys.argv[1] in ['/AATest/bin/bash', '/bin/bash', '/usr/bin/bash']: - print(' linux-vdso.so.1 (0x00007ffcf97f4000)') - print(' libreadline.so.6 => /AATest/lib64/libreadline.so.6 (0x00007f2c41324000)') - print(' libtinfo.so.6 => /AATest/lib64/libtinfo.so.6 (0x00007f2c410f9000)') - print(' libdl.so.2 => /AATest/lib64/libdl.so.2 (0x00007f2c40ef5000)') - print(' libc.so.6 => /AATest/lib64/libc.so.6 (0x00007f2c40b50000)') - print(' /AATest/lib64/ld-linux-x86-64.so.2 (0x000055782c473000)') - -elif sys.argv[1] == '/AATest/lib64/ld-2.22.so': - print(' linux-vdso.so.1 (0x00007ffcf97f4000)') - -elif sys.argv[1] == '/AATest/lib64/libc-2.22.so': - print(' /AATest/lib64/ld-linux-x86-64.so.2 (0x0000556858473000)') - print(' linux-vdso.so.1 (0x00007ffe98912000)') - -elif sys.argv[1] == '/AATest/lib64/libdl.so.2': - print(' linux-vdso.so.1 (0x00007ffec2538000)') - print(' libc.so.6 => /AATest/lib64/libc.so.6 (0x00007f8865346000)') - print(' /AATest/lib64/ld-linux-x86-64.so.2 (0x0000560c3bcee000)') - -elif sys.argv[1] == '/AATest/lib64/libtinfo.so.6': - print(' linux-vdso.so.1 (0x00007fff30518000)') - print(' libc.so.6 => /AATest/lib64/libc.so.6 (0x00007fb6f2ea3000)') - print(' /AATest/lib64/ld-linux-x86-64.so.2 (0x00005631fe8d3000)') - -elif sys.argv[1] == '/AATest/lib64/libreadline.so.6': - print(' linux-vdso.so.1 (0x00007ffcb5b62000)') - print(' libtinfo.so.6 => /AATest/lib64/libtinfo.so.6 (0x00007f2a4ed07000)') - print(' libc.so.6 => /AATest/lib64/libc.so.6 (0x00007f2a4e961000)') - print(' /AATest/lib64/ld-linux-x86-64.so.2 (0x000055f749c89000)') - -elif sys.argv[1] == '/AATest/lib64/ld-linux-x86-64.so.2': - print(' statically linked') - -elif sys.argv[1] == '/AATest/lib64/libc.so.6': - print(' /AATest/lib64/ld-linux-x86-64.so.2 (0x000055b65f7a9000)') - print(' linux-vdso.so.1 (0x00007ffde132b000)') - - -elif sys.argv[1] == '/AATest/sbin/ldconfig': - print(' not a dynamic executable') - sys.exit(1) # ldd exits with $? == 1 in this case - -elif sys.argv[1].startswith('/tmp/aa-test-'): # test file generated by test-aa.py - print(' not a dynamic executable') - -elif sys.argv[1] == 'TEMPLATE': - print('') - print('') - print('') - -else: - raise Exception('unknown parameter in fake_ldd: ' + sys.argv[1]) diff --git a/utils/test/logprof.conf b/utils/test/logprof.conf index 0276bfea6..4012c7ea7 100644 --- a/utils/test/logprof.conf +++ b/utils/test/logprof.conf @@ -15,7 +15,6 @@ logfiles = /var/log/audit/audit.log /var/log/syslog /var/log/messages parser = ../../parser/apparmor_parser ../parser/apparmor_parser - ldd = /usr/bin/ldd logger = /bin/logger /usr/bin/logger # customize how file ownership permissions are presented diff --git a/utils/test/test-aa.py b/utils/test/test-aa.py index 9096341a6..d19c3c203 100644 --- a/utils/test/test-aa.py +++ b/utils/test/test-aa.py @@ -15,7 +15,7 @@ import unittest import apparmor.aa # needed to set global vars in some tests from apparmor.aa import ( - change_profile_flags, check_for_apparmor, create_new_profile, get_file_perms, get_interpreter_and_abstraction, get_output, get_profile_flags, get_reqs, + change_profile_flags, check_for_apparmor, create_new_profile, get_file_perms, get_interpreter_and_abstraction, get_profile_flags, merged_to_split, parse_profile_data, propose_file_rules, set_options_audit_mode, set_options_owner_mode, split_to_merged) from apparmor.aare import AARE from apparmor.common import AppArmorBug, AppArmorException, is_skippable_file @@ -78,32 +78,6 @@ class AaTest_check_for_apparmor(AaTestWithTempdir): self.assertEqual(self.tmpdir + '/security/apparmor', check_for_apparmor(filesystems, mounts)) -class AATest_get_output(AATest): - tests = ( - (('./fake_ldd', '/AATest/lib64/libc-2.22.so'), (0, [' /AATest/lib64/ld-linux-x86-64.so.2 (0x0000556858473000)', ' linux-vdso.so.1 (0x00007ffe98912000)'])), - (('./fake_ldd', '/tmp/aa-test-foo'), (0, [' not a dynamic executable'])), - (('./fake_ldd', 'invalid'), (1, [])), # stderr is not part of output - ) - - def _run_test(self, params, expected): - self.assertEqual(get_output(params), expected) - - def test_get_output_nonexisting(self): - with self.assertRaises(AppArmorException): - ret, output = get_output(('./_file_/_not_/_found_',)) - - -class AATest_get_reqs(AATest): - tests = ( - ('/AATest/bin/bash', ['/AATest/lib64/libreadline.so.6', '/AATest/lib64/libtinfo.so.6', '/AATest/lib64/libdl.so.2', '/AATest/lib64/libc.so.6', '/AATest/lib64/ld-linux-x86-64.so.2']), - ('/tmp/aa-test-foo', []), - ('/AATest/sbin/ldconfig', []), # comes with $? == 1 - ) - - def _run_test(self, params, expected): - apparmor.aa.cfg['settings']['ldd'] = './fake_ldd' - self.assertEqual(get_reqs(params), expected) - class AaTest_create_new_profile(AATest): tests = ( @@ -114,8 +88,6 @@ class AaTest_create_new_profile(AATest): ) def _run_test(self, params, expected): - apparmor.aa.cfg['settings']['ldd'] = './fake_ldd' - self.createTmpdir() # copy the local profiles to the test directory @@ -146,10 +118,7 @@ class AaTest_create_new_profile(AATest): if exp_interpreter_path: self.assertEqual( set(profile[program]['file'].get_clean()), - {'{} ix,'.format(exp_interpreter_path), '{} r,'.format(program), '', - '/AATest/lib64/libtinfo.so.* mr,', '/AATest/lib64/libc.so.* mr,', - '/AATest/lib64/libdl.so.* mr,', '/AATest/lib64/libreadline.so.* mr,', - '/AATest/lib64/ld-linux-x86-64.so.* mr,'}) + {'{} ix,'.format(exp_interpreter_path), '{} r,'.format(program), '' }) else: self.assertEqual(set(profile[program]['file'].get_clean()), {'{} mr,'.format(program), ''}) diff --git a/utils/test/test-config.py b/utils/test/test-config.py index 66e574978..8542cb59a 100755 --- a/utils/test/test-config.py +++ b/utils/test/test-config.py @@ -24,7 +24,7 @@ class Test(unittest.TestCase): conf = ini_config.read_config('logprof.conf') logprof_sections = ['settings', 'qualifiers', 'required_hats', 'defaulthat', 'globs'] logprof_sections_options = [ - 'profiledir', 'inactive_profiledir', 'logfiles', 'parser', 'ldd', + 'profiledir', 'inactive_profiledir', 'logfiles', 'parser', 'logger', 'default_owner_prompt', 'custom_includes'] logprof_settings_parser = '../../parser/apparmor_parser ../parser/apparmor_parser'