Merge aa-logprof/aa-genprof: Adding support for --allow-all, --output-dir and --no-abstraction

- Adding support for --output-dir in aa-logprof and aa-genprof, allowing to work on profiles without applying the modified version
 - Adding support for --allow-all in aa-logprof that creates non-interactively 'allow' rules for all logs
 - Adding support for --no-abstraction in aa-logprof and aa-genprof

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1177
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>

utils/aa-genprof

@@ -69,6 +69,8 @@ parser.add_argument('-d', '--dir', type=str, help=_('path to profiles'))
 parser.add_argument('-f', '--file', type=str, help=_('path to logfile'))
 parser.add_argument('program', type=str, help=_('name of program to profile'))
 parser.add_argument('-j', '--json', action="store_true", help=_('Input and Output in JSON'))
+parser.add_argument('--no-abstraction', action='store_true', help=_('Do not use any abstractions in profiles'))
+parser.add_argument('-o', '--output-dir', type=str, help=_('Output Directory for profiles'))
 parser.add_argument('--configdir', type=str, help=argparse.SUPPRESS)
 args = parser.parse_args()
 
@@ -78,6 +80,10 @@ apparmor.init_aa(confdir=args.configdir, profiledir=args.dir)
 
 if args.json:
     aaui.set_json_mode(apparmor.cfg)
+if args.output_dir:
+    apparmor.check_output_dir(args.output_dir)
+if args.no_abstraction:
+    apparmor.disable_abstractions()
 
 apparmor.set_logfile(args.file)
 
@@ -174,7 +180,7 @@ while not done_profiling:
     ans, arg = q.promptUser('noexit')
 
     if ans == 'CMD_SCAN':
-        apparmor.do_logprof_pass(logmark)
+        apparmor.do_logprof_pass(logmark, out_dir=args.output_dir)
     else:
         done_profiling = True
 

utils/aa-logprof

@@ -28,16 +28,26 @@ parser.add_argument('-d', '--dir', type=str, help=_('path to profiles'))
 parser.add_argument('-f', '--file', type=str, help=_('path to logfile'))
 parser.add_argument('-m', '--mark', type=str, help=_('mark in the log to start processing after'))
 parser.add_argument('-j', '--json', action='store_true', help=_('Input and Output in JSON'))
+parser.add_argument('-a', '--allow-all', action='store_true', help=_('Accept silently all rules'))
+parser.add_argument('--no-abstraction', action='store_true', help=_('Do not use any abstractions in profiles'))
+parser.add_argument('-o', '--output-dir', type=str, help=_('Output Directory for profiles'))
 parser.add_argument('--configdir', type=str, help=argparse.SUPPRESS)
 parser.add_argument('--no-check-mountpoint', action='store_true', help=argparse.SUPPRESS)
 args = parser.parse_args()
 
+
 logmark = args.mark or ''
 
 apparmor.init_aa(confdir=args.configdir, profiledir=args.dir)
 
 if args.json:
     aaui.set_json_mode(apparmor.cfg)
+if args.allow_all:
+    aaui.set_allow_all_mode()
+if args.no_abstraction:
+    apparmor.disable_abstractions()
+if args.output_dir:
+    apparmor.check_output_dir(args.output_dir)
 
 apparmor.set_logfile(args.file)
 
@@ -48,4 +58,4 @@ if not aa_mountpoint and not args.no_check_mountpoint:
 apparmor.loadincludes()
 
 apparmor.read_profiles(True)
-apparmor.do_logprof_pass(logmark)
+apparmor.do_logprof_pass(logmark, out_dir=args.output_dir)

utils/apparmor/aa.py

@@ -74,6 +74,8 @@ cfg = None
 parser = None
 profile_dir = None
 extra_profile_dir = None
+
+use_abstractions = True
 ### end our
 # To keep track of previously included profile fragments
 include = dict()
@@ -294,6 +296,11 @@ def set_enforce(filename, program):
     change_profile_flags(filename, program, ['complain', 'kill', 'unconfined', 'prompt','default_allow'], False)  # remove conflicting and complain mode flags
 
 
+def disable_abstractions():
+    global use_abstractions
+    use_abstractions = False
+
+
 def delete_symlink(subdir, filename):
     path = filename
     link = re.sub('^%s' % profile_dir, '%s/%s' % (profile_dir, subdir), path)
@@ -338,6 +345,20 @@ def head(file):
         raise AppArmorException(_('Unable to read first line from %s: File Not Found') % file)
 
 
+def check_output_dir(output_dir):
+    if os.path.isdir(output_dir):
+        return True
+    elif os.path.exists(output_dir):
+        raise AppArmorException(_("%(dir) exists and is not a directory") % {'dir': output_dir})
+    try:
+        os.mkdir(output_dir, mode=0o700)
+    except OSError as e:
+        raise AppArmorException(
+            _("Unable to create output directory %(dir)s\n\t%(error)s")
+            % {'dir': output_dir, 'error': str(e)}
+        )
+
+
 def get_output(params):
     """Runs the program with the given args and returns the return code and stdout (as list of lines)"""
     try:
@@ -898,6 +919,7 @@ def ask_exec(hashlog):
 
                     exec_toggle = False
                     q.functions.extend(build_x_functions(default, options, exec_toggle))
+                    q.already_have_profile = get_profile_filename_from_attachment(exec_target)
 
                     # ask user about the exec mode to use
                     ans = ''
@@ -1202,7 +1224,8 @@ def ask_rule_questions(prof_events, profile_name, the_profile, r_types):
             newincludes = match_includes(the_profile, ruletype, rule_obj)
             q = aaui.PromptQuestion()
             if newincludes:
-                options.extend(map(lambda inc: 'include <%s>' % inc, sorted(set(newincludes))))
+                if use_abstractions:
+                    options.extend(map(lambda inc: 'include <%s>' % inc, sorted(set(newincludes))))
 
             if ruletype == 'file' and rule_obj.path:
                 options += propose_file_rules(the_profile, rule_obj)
@@ -1270,15 +1293,20 @@ def ask_rule_questions(prof_events, profile_name, the_profile, r_types):
 
                         the_profile['inc_ie'].add(IncludeRule.create_instance(selection))
 
-                        aaui.UI_Info(_('Adding %s to profile.') % selection)
+                        if aaui.UI_mode == 'allow_all':
+                            aaui.UI_Info(_('Adding %s to profile %s.') % (selection, profile_name))
+                        else:
+                            aaui.UI_Info(_('Adding %s to profile.') % selection)
                         if deleted:
                             aaui.UI_Info(_('Deleted %s previous matching profile entries.') % deleted)
 
                     else:
                         rule_obj = rule_obj.create_instance(selection)
                         deleted = the_profile[ruletype].add(rule_obj, cleanup=True)
-
+                        if aaui.UI_mode == 'allow_all':
-                        aaui.UI_Info(_('Adding %s to profile.') % rule_obj.get_clean())
+                            aaui.UI_Info(_('Adding %s to profile %s.') % (rule_obj.get_clean(), profile_name))
+                        else:
+                            aaui.UI_Info(_('Adding %s to profile.') % rule_obj.get_clean())
                         if deleted:
                             aaui.UI_Info(_('Deleted %s previous matching profile entries.') % deleted)
 
@@ -1539,7 +1567,7 @@ def set_logfile(filename):
         raise AppArmorException(_('%s is a directory. Please specify a file as logfile') % logfile)
 
 
-def do_logprof_pass(logmark=''):
+def do_logprof_pass(logmark='', out_dir=None):
     # set up variables for this pass
     global active_profiles
     global sev_db
@@ -1563,10 +1591,10 @@ def do_logprof_pass(logmark=''):
 
     ask_the_questions(log_dict)
 
-    save_profiles()
+    save_profiles(out_dir=out_dir)
 
 
-def save_profiles(is_mergeprof=False):
+def save_profiles(is_mergeprof=False, out_dir=None):
     # Ensure the changed profiles are actual active profiles
     for prof_name in changed.keys():
         if not aa.get(prof_name, False):
@@ -1600,7 +1628,7 @@ def save_profiles(is_mergeprof=False):
             profile_name = options[arg]
 
             if ans == 'CMD_SAVE_SELECTED':
-                write_profile_ui_feedback(profile_name)
+                write_profile_ui_feedback(profile_name, out_dir=out_dir)
                 reload_base(profile_name)
                 q.selected = 0  # saving the selected profile removes it from the list, therefore reset selection
 
@@ -1626,7 +1654,7 @@ def save_profiles(is_mergeprof=False):
                 changed.pop(options[arg])
 
         for profile_name in sorted(changed.keys()):
-            write_profile_ui_feedback(profile_name)
+            write_profile_ui_feedback(profile_name, out_dir=out_dir)
             reload_base(profile_name)
 
 
@@ -2281,12 +2309,12 @@ def serialize_profile(profile_data, name, options):
     return string + '\n'
 
 
-def write_profile_ui_feedback(profile, is_attachment=False):
+def write_profile_ui_feedback(profile, is_attachment=False, out_dir=None):
     aaui.UI_Info(_('Writing updated profile for %s.') % profile)
-    write_profile(profile, is_attachment)
+    write_profile(profile, is_attachment, out_dir=out_dir)
 
 
-def write_profile(profile, is_attachment=False):
+def write_profile(profile, is_attachment=False, out_dir=None):
     if aa[profile][profile].get('filename', False):
         prof_filename = aa[profile][profile]['filename']
     elif is_attachment:
@@ -2296,8 +2324,9 @@ def write_profile(profile, is_attachment=False):
 
     serialize_options = {'METADATA': True, 'is_attachment': is_attachment}
     profile_string = serialize_profile(split_to_merged(aa), profile, serialize_options)
+
     try:
-        with NamedTemporaryFile('w', suffix='~', delete=False, dir=profile_dir) as newprof:
+        with NamedTemporaryFile('w', suffix='~', delete=False, dir=out_dir or profile_dir) as newprof:
             if os.path.exists(prof_filename):
                 shutil.copymode(prof_filename, newprof.name)
             else:
@@ -2308,7 +2337,11 @@ def write_profile(profile, is_attachment=False):
     except PermissionError as e:
         raise AppArmorException(e)
 
-    os.rename(newprof.name, prof_filename)
+    if out_dir is None:
+        os.rename(newprof.name, prof_filename)
+    else:
+        out_filename = out_dir + "/" + prof_filename.split('/')[-1]
+        os.rename(newprof.name, out_filename)
 
     if profile in changed:
         changed.pop(profile)

utils/apparmor/ui.py

@@ -75,6 +75,11 @@ def set_text_mode():
     UI_mode = 'text'
 
 
+def set_allow_all_mode():
+    global UI_mode
+    UI_mode = 'allow_all'
+
+
 # reads the response on command line for json and verifies the response
 # for the dialog type
 def json_response(dialog_type):
@@ -151,7 +156,10 @@ def UI_YesNo(text, default):
                 sys.stdout.write('\n[%s] / %s\n' % (yes, no))
             else:
                 sys.stdout.write('\n%s / [%s]\n' % (yes, no))
-            ans = getkey()
+            if UI_mode == 'allow_all':
+                ans = nokey
+            else:
+                ans = getkey()
             if ans:
                 # Get back to english from localised answer
                 ans = ans.lower()
@@ -197,7 +205,10 @@ def UI_YesNoCancel(text, default):
                 sys.stdout.write('\n%s / [%s] / %s\n' % (yes, no, cancel))
             else:
                 sys.stdout.write('\n%s / %s / [%s]\n' % (yes, no, cancel))
-            ans = getkey()
+            if UI_mode == 'allow_all':
+                ans = nokey
+            else:
+                ans = getkey()
             if ans:
                 # Get back to english from localised answer
                 ans = ans.lower()
@@ -377,6 +388,7 @@ class PromptQuestion:
     default = None
     selected = None
     helptext = None
+    already_have_profile = False
 
     def __init__(self):
         self.headers = []
@@ -502,6 +514,17 @@ class PromptQuestion:
                 hm = json_response('promptuser')
                 ans = hm["response_key"]
                 selected = hm["selected"]
+
+            elif UI_mode == 'allow_all':
+                if self.already_have_profile:
+                    expected_keys = ['CMD_px', 'CMD_ix', 'CMD_ALLOW', 'CMD_SAVE_CHANGES']
+                else:
+                    expected_keys = ['CMD_ix', 'CMD_ALLOW', 'CMD_SAVE_CHANGES']
+                for exp in expected_keys:
+                    if exp in functions:
+                        ans = get_translated_hotkey(CMDS[exp])
+                        break
+
             else:  # text mode
                 sys.stdout.write(prompt + '\n')
                 ans = getkey().lower()
@@ -517,6 +540,7 @@ class PromptQuestion:
                         selected += 1
                     ans = 'XXXINVALIDXXX'
 
+
     #             elif keys.get(ans, False) == 'CMD_HELP':
     #                 sys.stdout.write('\n%s\n' %helptext)
     #                 ans = 'XXXINVALIDXXX'

utils/test/logprof/ping.allowlog

@@ -0,0 +1,6 @@
+Updating AppArmor profiles in /etc/apparmor.d.
+Reading log entries from /var/log/audit/audit.log.
+Complain-mode changes:
+Enforce-mode changes:
+Adding owner /proc/*/cmdline r, to profile ping.
+Writing updated profile for ping.

utils/test/test-logprof.py

@@ -40,13 +40,13 @@ class TestLogprof(AATest):
     def AATeardown(self):
         self._terminate()
 
-    def _startLogprof(self, auditlog):
+    def _startLogprof(self, auditlog, mode):
         exe = [sys.executable]
 
         if 'coverage' in sys.modules:
             exe = exe + ['-m', 'coverage', 'run', '--branch', '-p']
 
-        exe = exe + ['../aa-logprof', '--json', '--configdir', './', '-f', auditlog, '-d', self.profile_dir, '--no-check-mountpoint']
+        exe = exe + ['../aa-logprof', '--' + mode, '--configdir', './', '-f', auditlog, '-d', self.profile_dir, '--no-check-mountpoint', '--output-dir', self.tmpdir]
 
         process = subprocess.Popen(
             exe,
@@ -76,7 +76,7 @@ class TestLogprof(AATest):
         jlog = jlog.replace('/var/log/audit/audit.log', auditlog)
         jlog = jlog.strip().split('\n')
 
-        self.process = self._startLogprof(auditlog)
+        self.process = self._startLogprof(auditlog, 'json')
 
         for line in jlog:
             if line.startswith('o '):  # read from stdout
@@ -101,7 +101,7 @@ class TestLogprof(AATest):
 
         for file in expected:
             exp = read_file('./logprof/%s.%s' % (params, file))
-            actual = read_file(os.path.join(self.profile_dir, file))
+            actual = read_file(os.path.join(self.tmpdir, file))
 
             # remove '# Last Modified:' line from updated profile
             actual = actual.split('\n')
@@ -111,6 +111,23 @@ class TestLogprof(AATest):
 
             self.assertEqual(actual, exp)
 
+    def test_allow_all(self):
+        auditlog = './logprof/%s.auditlog' % 'ping'
+        allowlog = './logprof/%s.allowlog' % 'ping'
+
+        slog = read_file(allowlog)
+        slog = slog.replace('/etc/apparmor.d', self.profile_dir)
+        slog = slog.replace('/var/log/audit/audit.log', auditlog)
+        slog = slog.strip().split('\n')
+
+        self.process = self._startLogprof(auditlog, 'allow-all')
+
+        for line in slog:
+            output = self.process.stdout.readline().decode("utf-8").strip()
+            self.assertEqual(output, line)
+        # give logprof some time to write the updated profile and terminate
+        self.process.wait(timeout=0.3)
+        self.assertEqual(self.process.returncode, 0)
 
 # if you import apparmor.aa and call init_aa() in your tests, uncomment this
 # setup_aa(aa)