mirror of
https://gitlab.com/apparmor/apparmor
synced 2025-08-22 10:07:12 +00:00
Merge Update utils/aa-notify to add running in the foreground and continue on "read_profiles" permission error
Hello! I run AppArmor daily on my personal machine and use `aa-notify` to receive alerts for any audit events. I wanted to submit two features and one bugfix for problems that I've seen while running `aa-notify`. ### Here are the two features in this merge request: 1. Allow `aa-notify` to run in the foreground. I understand that `aa-notify` is ment to be run as a background notification daemon, however there are situations when running in the foreground would be better suited. One example is any startup "launcher" that creates and monitors it's child processes (my setup basically does this) and when `aa-notify` forks, the launcher percieves it as crashing on startup. This merge request adds an option "-F"/"--foreground" to prevent background forking and perserves the default behavior, while allowing `aa-notify` to run like a standard foreground application. The test cases in `utils/test/test-aa-notify.py` are also updated to reflect the argument changes. 2. Prevent `aa-notify` from exiting with a fatal error when the AppArmor profiles directory cannot be read. During startup, `aa-notify` will attempt to read the AppArmor profiles from the profile directory using the `aa.read_profiles` function. If this function fails due to a permissions check, `aa-notify` will exit with an error. In my setups, the standard user does not have any read access to the AppArmor profiles directory (reasoning: as an attacker, I could read the profiles to find something that would have the weakest permissions for explitation, but with that route blocked, this becomes significantly harder). In this merge request, an optional paramater `skip_perm_error` that is by-default False, is added to the `read_profiles` function call in `aa-notify`. In `aa.py`, this function has two added lines, which are under `except (OSError, TypeError):`. The extra code checks if `skip_perm_error` is True, and if so will print a warning out using the `aaui.UI_Info` function and returns cleanly. During my test cases, I have not run into any issues running `aa-notify` without reading any profiles. ### BugFixes 1. Crash during `aa-notify` polling during audit events that cause `rl.parse_record(event)` to return None I've noticed certain events will cause `aa-notify` to crash, specifically the ones in the attached log snipped will cause `ev` to be `None`. In this merge request, I've added a simple `if ev is None:` check before attempting to read from `ev`. If `ev` is None, it will fall into `continue` and prevent a crash from occuring. The crash log is also attached for additional information. Please let me know if there's any additional questions or information you may need! And thank you for all your hard work on this project! MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1706 Approved-by: Maxime Bélair <maxime.belair@canonical.com> Approved-by: Christian Boltz <apparmor@cboltz.de> Merged-by: Christian Boltz <apparmor@cboltz.de>
This commit is contained in:
commit
a606397417
@ -123,8 +123,8 @@ def is_event_in_filter(event, filters):
|
|||||||
return True
|
return True
|
||||||
|
|
||||||
|
|
||||||
def daemonize():
|
def daemonize(foreground):
|
||||||
"""Run the notification daemon in the background."""
|
"""Run the notification daemon in the background or foreground."""
|
||||||
# Kill other instances of aa-notify if already running
|
# Kill other instances of aa-notify if already running
|
||||||
for process in psutil.process_iter():
|
for process in psutil.process_iter():
|
||||||
# Find the process that has the same name as this script, e.g. aa-notify.py
|
# Find the process that has the same name as this script, e.g. aa-notify.py
|
||||||
@ -132,8 +132,13 @@ def daemonize():
|
|||||||
print(_('Killing old daemon (PID {})...').format(process.pid))
|
print(_('Killing old daemon (PID {})...').format(process.pid))
|
||||||
os.kill(process.pid, 15)
|
os.kill(process.pid, 15)
|
||||||
|
|
||||||
# Spawn/fork into the background and stay running
|
if foreground:
|
||||||
newpid = os.fork()
|
# Stay in the foreground
|
||||||
|
newpid = 0
|
||||||
|
else:
|
||||||
|
# Spawn/fork into the background and stay running
|
||||||
|
newpid = os.fork()
|
||||||
|
|
||||||
if newpid == 0:
|
if newpid == 0:
|
||||||
|
|
||||||
# Follow the logfile and stream notifications
|
# Follow the logfile and stream notifications
|
||||||
@ -835,6 +840,7 @@ def main():
|
|||||||
parser.add_argument('-u', '--user', type=str, help=_('user to drop privileges to when not using sudo'))
|
parser.add_argument('-u', '--user', type=str, help=_('user to drop privileges to when not using sudo'))
|
||||||
parser.add_argument('-w', '--wait', type=int, metavar=('NUM'), help=_('wait NUM seconds before displaying notifications (with -p)'))
|
parser.add_argument('-w', '--wait', type=int, metavar=('NUM'), help=_('wait NUM seconds before displaying notifications (with -p)'))
|
||||||
parser.add_argument('-m', '--merge-notifications', action='store_true', help=_('Merge notification for improved readability (with -p)'))
|
parser.add_argument('-m', '--merge-notifications', action='store_true', help=_('Merge notification for improved readability (with -p)'))
|
||||||
|
parser.add_argument('-F', '--foreground', action='store_true', help=_('Do not fork to the background'))
|
||||||
parser.add_argument('--prompt-filter', type=str, metavar=('PF'), help=_('kind of operations which display a popup prompt'))
|
parser.add_argument('--prompt-filter', type=str, metavar=('PF'), help=_('kind of operations which display a popup prompt'))
|
||||||
parser.add_argument('--debug', action='store_true', help=_('debug mode'))
|
parser.add_argument('--debug', action='store_true', help=_('debug mode'))
|
||||||
parser.add_argument('--configdir', type=str, help=argparse.SUPPRESS)
|
parser.add_argument('--configdir', type=str, help=argparse.SUPPRESS)
|
||||||
@ -1087,10 +1093,10 @@ def main():
|
|||||||
rl = ReadLog('', '', '')
|
rl = ReadLog('', '', '')
|
||||||
|
|
||||||
# Initialize the list of profiles for can_allow_rule
|
# Initialize the list of profiles for can_allow_rule
|
||||||
aa.read_profiles()
|
aa.read_profiles(skip_perm_error=True)
|
||||||
|
|
||||||
drop_privileges()
|
drop_privileges()
|
||||||
daemonize()
|
daemonize(args.foreground)
|
||||||
raise_privileges()
|
raise_privileges()
|
||||||
|
|
||||||
if args.merge_notifications:
|
if args.merge_notifications:
|
||||||
@ -1140,6 +1146,11 @@ def main():
|
|||||||
for (event, message) in notify_about_new_entries(logfile, filters, args.wait):
|
for (event, message) in notify_about_new_entries(logfile, filters, args.wait):
|
||||||
ev = rl.parse_record(event)
|
ev = rl.parse_record(event)
|
||||||
|
|
||||||
|
# "parse_record" may return None if the event was invalid or misformed.
|
||||||
|
# Catch it here to prevent any errors.
|
||||||
|
if ev is None:
|
||||||
|
continue
|
||||||
|
|
||||||
# @TODO redo special behaviours with a more regular function
|
# @TODO redo special behaviours with a more regular function
|
||||||
# We ignore capability denials for binaries in ignore_denied_capability
|
# We ignore capability denials for binaries in ignore_denied_capability
|
||||||
if ev['operation'] == 'capable' and ev['comm'] in ignore_denied_capability:
|
if ev['operation'] == 'capable' and ev['comm'] in ignore_denied_capability:
|
||||||
|
@ -1604,7 +1604,7 @@ def update_profiles(ui_msg=False, skip_profiles=()):
|
|||||||
print(_("Error while loading profiles: {}").format(e))
|
print(_("Error while loading profiles: {}").format(e))
|
||||||
|
|
||||||
|
|
||||||
def read_profiles(ui_msg=False, skip_profiles=(), skip_disabled=True):
|
def read_profiles(ui_msg=False, skip_profiles=(), skip_disabled=True, skip_perm_error=False):
|
||||||
# we'll read all profiles from disk, so reset the storage first (autodep() might have created/stored
|
# we'll read all profiles from disk, so reset the storage first (autodep() might have created/stored
|
||||||
# a profile already, which would cause a 'Conflicting profile' error in attach_profile_data())
|
# a profile already, which would cause a 'Conflicting profile' error in attach_profile_data())
|
||||||
#
|
#
|
||||||
@ -1619,6 +1619,9 @@ def read_profiles(ui_msg=False, skip_profiles=(), skip_disabled=True):
|
|||||||
try:
|
try:
|
||||||
os.listdir(profile_dir)
|
os.listdir(profile_dir)
|
||||||
except (OSError, TypeError):
|
except (OSError, TypeError):
|
||||||
|
if skip_perm_error:
|
||||||
|
aaui.UI_Info(_("WARNING: Can't read AppArmor profiles in %s") % profile_dir)
|
||||||
|
return
|
||||||
fatal_error(_("Can't read AppArmor profiles in %s") % profile_dir)
|
fatal_error(_("Can't read AppArmor profiles in %s") % profile_dir)
|
||||||
|
|
||||||
for file in os.listdir(profile_dir):
|
for file in os.listdir(profile_dir):
|
||||||
|
@ -171,7 +171,11 @@ class ReadLog:
|
|||||||
return None
|
return None
|
||||||
|
|
||||||
def parse_record(self, event):
|
def parse_record(self, event):
|
||||||
"""Parse the record from LibAppArmor into key value pairs"""
|
"""
|
||||||
|
Parse the record from LibAppArmor into key value pairs
|
||||||
|
|
||||||
|
If the event is invalid or misformed, this function may return 'None'.
|
||||||
|
"""
|
||||||
ev = dict()
|
ev = dict()
|
||||||
ev['resource'] = event.info
|
ev['resource'] = event.info
|
||||||
ev['active_hat'] = event.active_hat
|
ev['active_hat'] = event.active_hat
|
||||||
|
@ -168,7 +168,7 @@ class AANotifyTest(AANotifyBase):
|
|||||||
expected_return_code = 0
|
expected_return_code = 0
|
||||||
expected_output_1 = \
|
expected_output_1 = \
|
||||||
'''usage: aa-notify [-h] [-p] [--display DISPLAY] [-f FILE] [-l] [-s NUM] [-v]
|
'''usage: aa-notify [-h] [-p] [--display DISPLAY] [-f FILE] [-l] [-s NUM] [-v]
|
||||||
[-u USER] [-w NUM] [-m] [--prompt-filter PF] [--debug]
|
[-u USER] [-w NUM] [-m] [-F] [--prompt-filter PF] [--debug]
|
||||||
[--filter.profile PROFILE] [--filter.operation OPERATION]
|
[--filter.profile PROFILE] [--filter.operation OPERATION]
|
||||||
[--filter.name NAME] [--filter.denied DENIED]
|
[--filter.name NAME] [--filter.denied DENIED]
|
||||||
[--filter.family FAMILY] [--filter.socket SOCKET]
|
[--filter.family FAMILY] [--filter.socket SOCKET]
|
||||||
@ -192,6 +192,7 @@ Display AppArmor notifications or messages for DENIED entries.
|
|||||||
-p)
|
-p)
|
||||||
-m, --merge-notifications
|
-m, --merge-notifications
|
||||||
Merge notification for improved readability (with -p)
|
Merge notification for improved readability (with -p)
|
||||||
|
-F, --foreground Do not fork to the background
|
||||||
--prompt-filter PF kind of operations which display a popup prompt
|
--prompt-filter PF kind of operations which display a popup prompt
|
||||||
--debug debug mode
|
--debug debug mode
|
||||||
|
|
||||||
|
Loading…
x
Reference in New Issue
Block a user