Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add exception handling around fapd handle is_active() call #560

Merged
merged 15 commits into from Aug 17, 2022
Merged

Add exception handling around fapd handle is_active() call #560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
51b240c
Add exception handling around fapd handle is_active() call
tparchambault Jul 26, 2022
a97e3a8
Merge branch 'master' into fa544_ui_unresponsive_rollback
tparchambault Jul 26, 2022
941215a
Merge branch 'master' into fa544_ui_unresponsive_rollback
tparchambault Jul 27, 2022
df8fc46
Adding additional delay to test if an fapolicyd.active() error may a…
tparchambault Jul 27, 2022
2a837e8
Sync with master
tparchambault Jul 29, 2022
93f59b3
Overwrite my local fapolicyd.rs with latest from master
tparchambault Jul 29, 2022
ff82265
Merge branch 'master' into fa544_ui_unresponsive_rollback
tparchambault Aug 1, 2022
b5bede9
More specific exception msg when acquiring first fapd status
tparchambault Aug 1, 2022
167f9ff
Merge branch 'master' into fa544_ui_unresponsive_rollback
tparchambault Aug 15, 2022
9cdbbf3
Fix test target command
tparchambault Aug 15, 2022
167d0d9
Merge branch 'master' into fa544_ui_unresponsive_rollback
tparchambault Aug 16, 2022
2a10fe0
Moved blocking lock acquisition into context manager blocks
tparchambault Aug 16, 2022
5eee4a2
Merge branch 'master' into fa544_ui_unresponsive_rollback
jw3 Aug 17, 2022
8270b6b
Add toaster error msgs when fapd start/stop exceptions occur
tparchambault Aug 17, 2022
6b01a66
Merge branch 'master' into fa544_ui_unresponsive_rollback
tparchambault Aug 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Expand Up @@ -85,7 +85,7 @@ test: pytest cargo-test
# Execute the python unit tests
pytest: build
@echo -e "${GRN} |--- Python unit-testing: Invoking pytest...${NC}"
pipenv run xvfb-run -a pytest -s --cov fapolicy_analyzer/
pipenv run xvfb-run -a pytest -s --cov=fapolicy_analyzer fapolicy_analyzer/tests/

# Execute the Rust unit-tests
cargo-test: build
Expand Down
85 changes: 46 additions & 39 deletions fapolicy_analyzer/locale/fapolicy_analyzer.pot
View file
Open in desktop
Expand Up @@ -7,9 +7,9 @@
#, fuzzy
msgid ""
msgstr ""
"Project-Id-Version: fapolicy-analyzer 0.3.1+113.g6112ca0\n"
"Project-Id-Version: fapolicy-analyzer 0.5.0+72.g5eee4a2.dirty\n"
"Report-Msgid-Bugs-To: EMAIL@ADDRESS\n"
"POT-Creation-Date: 2022-07-29 09:19-0400\n"
"POT-Creation-Date: 2022-08-17 17:32-0400\n"
"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
"Language-Team: LANGUAGE <LL@li.org>\n"
Expand Down Expand Up @@ -56,7 +56,7 @@ msgstr ""
msgid "Reverting to previous settings in {i+1} seconds"
msgstr ""

#: fapolicy_analyzer/ui/main_window.py:262
#: fapolicy_analyzer/ui/main_window.py:261
msgid ""
"An error occurred trying to open the session file, "
"{self.strSessionFilename}"
Expand Down Expand Up @@ -247,30 +247,38 @@ msgid "Any files"
msgstr ""

#: fapolicy_analyzer/ui/strings.py:88
msgid "Profiling target file chown failure"
msgid "On-line fapolicyd start failed"
msgstr ""

#: fapolicy_analyzer/ui/strings.py:89
msgid "On-line fapolicyd stop failed"
msgstr ""

#: fapolicy_analyzer/ui/strings.py:91
msgid "Profiling target file chown failure"
msgstr ""

#: fapolicy_analyzer/ui/strings.py:92
msgid "Profiling target Popen failure"
msgstr ""

#: fapolicy_analyzer/ui/strings.py:90
#: fapolicy_analyzer/ui/strings.py:93
msgid "Profiling target redirection failure"
msgstr ""

#: fapolicy_analyzer/ui/strings.py:92
#: fapolicy_analyzer/ui/strings.py:95
msgid "File path(s) contains embedded whitespace."
msgstr ""

#: fapolicy_analyzer/ui/strings.py:93
#: fapolicy_analyzer/ui/strings.py:96
msgid ""
"fapolicyd currently does not support paths containing spaces. The "
"following paths will not be added to the Trusted Files List.\n"
"(fapolicyd: V TBD)\n"
"\n"
msgstr ""

#: fapolicy_analyzer/ui/strings.py:98
#: fapolicy_analyzer/ui/strings.py:101
msgid ""
"\n"
" Restore your prior session now?\n"
Expand All @@ -287,57 +295,57 @@ msgid ""
" "
msgstr ""

#: fapolicy_analyzer/ui/strings.py:114
#: fapolicy_analyzer/ui/strings.py:117
msgid "An error occurred trying to restore a prior autosaved edit session"
msgstr ""

#: fapolicy_analyzer/glade/loader.glade:44 fapolicy_analyzer/ui/strings.py:118
#: fapolicy_analyzer/glade/loader.glade:44 fapolicy_analyzer/ui/strings.py:121
msgid "Loading..."
msgstr ""

#: fapolicy_analyzer/ui/strings.py:120
#: fapolicy_analyzer/ui/strings.py:123
msgid "file"
msgstr ""

#: fapolicy_analyzer/ui/strings.py:121
#: fapolicy_analyzer/ui/strings.py:124
msgid "files"
msgstr ""

#: fapolicy_analyzer/ui/strings.py:122
#: fapolicy_analyzer/ui/strings.py:125
msgid "user"
msgstr ""

#: fapolicy_analyzer/ui/strings.py:123
#: fapolicy_analyzer/ui/strings.py:126
msgid "users"
msgstr ""

#: fapolicy_analyzer/ui/strings.py:124
#: fapolicy_analyzer/ui/strings.py:127
msgid "group"
msgstr ""

#: fapolicy_analyzer/ui/strings.py:125
#: fapolicy_analyzer/ui/strings.py:128
msgid "groups"
msgstr ""

#: fapolicy_analyzer/ui/strings.py:126
#: fapolicy_analyzer/ui/strings.py:129
msgid ""
"An error occurred trying to parse the event log file. Please try again or"
" select a different file."
msgstr ""

#: fapolicy_analyzer/ui/strings.py:129
#: fapolicy_analyzer/ui/strings.py:132
msgid "An error occurred trying to retrieve the user list. Please try again."
msgstr ""

#: fapolicy_analyzer/ui/strings.py:132
#: fapolicy_analyzer/ui/strings.py:135
msgid "An error occurred trying to retrieve the group list. Please try again."
msgstr ""

#: fapolicy_analyzer/ui/strings.py:136
#: fapolicy_analyzer/ui/strings.py:139
msgid "Could not load application resources"
msgstr ""

#: fapolicy_analyzer/ui/strings.py:137
#: fapolicy_analyzer/ui/strings.py:140
msgid ""
"The required application resource files could not be loaded from disk.\n"
"The fapolicy analyzer application cannot open.\n"
Expand All @@ -349,11 +357,11 @@ msgid ""
"2. An incorrectly configured fapolicyd rule set."
msgstr ""

#: fapolicy_analyzer/ui/strings.py:148
#: fapolicy_analyzer/ui/strings.py:151
msgid "Trust Database"
msgstr ""

#: fapolicy_analyzer/ui/strings.py:149
#: fapolicy_analyzer/ui/strings.py:152
msgid ""
"\n"
"The fapolicyd trusted resources database\n"
Expand All @@ -378,7 +386,7 @@ msgid ""
" "
msgstr ""

#: fapolicy_analyzer/ui/strings.py:173
#: fapolicy_analyzer/ui/strings.py:176
msgid "Error applying changes"
msgstr ""

Expand Down Expand Up @@ -433,7 +441,6 @@ msgid "No"
msgstr ""

#: fapolicy_analyzer/glade/confirm_change_dialog.glade:63
#: fapolicy_analyzer/glade/profile_dialog.glade:52
#: fapolicy_analyzer/glade/trust_reconciliation_dialog.glade:67
msgid "Cancel"
msgstr ""
Expand Down Expand Up @@ -520,32 +527,32 @@ msgstr ""
msgid "x"
msgstr ""

#: fapolicy_analyzer/glade/profile_dialog.glade:24
msgid "File Acccess Profiler"
#: fapolicy_analyzer/glade/profiler_page.glade:94
msgid "Execute File"
msgstr ""

#: fapolicy_analyzer/glade/profile_dialog.glade:39
msgid "Run Test"
#: fapolicy_analyzer/glade/profiler_page.glade:106
msgid "Run as User"
msgstr ""

#: fapolicy_analyzer/glade/profile_dialog.glade:129
msgid "Execute File"
#: fapolicy_analyzer/glade/profiler_page.glade:120
msgid "Environment Variables"
msgstr ""

#: fapolicy_analyzer/glade/profile_dialog.glade:140
msgid "Arguments"
#: fapolicy_analyzer/glade/profiler_page.glade:132
msgid "Working Directory"
msgstr ""

#: fapolicy_analyzer/glade/profile_dialog.glade:151
msgid "Run as User"
#: fapolicy_analyzer/glade/profiler_page.glade:144
msgid "Arguments"
msgstr ""

#: fapolicy_analyzer/glade/profile_dialog.glade:162
msgid "Environment Variables"
#: fapolicy_analyzer/glade/profiler_page.glade:166
msgid "Persistent"
msgstr ""

#: fapolicy_analyzer/glade/profile_dialog.glade:173
msgid "Working Directory"
#: fapolicy_analyzer/glade/profiler_page.glade:215
msgid "Target Output"
msgstr ""

#: fapolicy_analyzer/glade/remove_deleted_dialog.glade:24
Expand Down
89 changes: 58 additions & 31 deletions fapolicy_analyzer/ui/fapd_manager.py
View file
Open in desktop
Expand Up @@ -15,12 +15,17 @@

import logging
import os
import sys
import subprocess


from datetime import datetime as DT
from enum import Enum
from fapolicy_analyzer import Handle
from fapolicy_analyzer.ui.actions import (NotificationType, add_notification)
from fapolicy_analyzer.ui.store import dispatch
from fapolicy_analyzer.ui.strings import (FAPD_DBUS_STOP_ERROR_MSG,
FAPD_DBUS_START_ERROR_MSG)
from threading import Lock
from time import time, sleep

Expand Down Expand Up @@ -63,20 +68,20 @@ def __init__(self, fapd_control_enabled=True):
# Run thread to peroidically poll online fapd status
self._fapd_monitoring = False
self._fapd_ref = Handle("fapolicyd")
self._fapd_profiling_ref = Handle("fapd_profiler")
self._fapd_profiler_pid = None

# SU_OVERRIDE allows mode changes in development environment
self._fapd_control_enabled = fapd_control_enabled
self._fapd_control_override = os.environ.get("SU_OVERRIDE", False)

# Verify that fapolicyd is installed, if so initialize fapd status
self._fapd_lock = Lock()
self.mode = FapdMode.DISABLED
self.initial_daemon_status()

# Initialize fapd profiling instance vars
self._fapd_profiler_pid = None
self.procProfile = None
self._fapd_profiling_timestamp = None

# Verify that fapolicyd is intalled, if so initialize fapd status
self.initial_daemon_status()
# SU_OVERRIDE allows mode changes in development environment
self._fapd_control_enabled = fapd_control_enabled
self._fapd_control_override = os.environ.get("SU_OVERRIDE", False)

# ToDo: Verify that FAPD_LOGPATH works in the pkexec environment
fapd_logpath_tmp = os.environ.get("FAPD_LOGPATH")
Expand Down Expand Up @@ -133,10 +138,17 @@ def _start(self, instance=FapdMode.ONLINE):

if instance == FapdMode.ONLINE:
logging.debug("fapd is initiating an ONLINE session")
if (self._fapd_status != ServiceStatus.UNKNOWN) and (self._fapd_lock.acquire()):
self._fapd_ref.start()
self.mode = FapdMode.ONLINE
self._fapd_lock.release()
if (self._fapd_status != ServiceStatus.UNKNOWN):
try:
with self._fapd_lock:
self._fapd_ref.start()
self.mode = FapdMode.ONLINE

except Exception:
logging.error(FAPD_DBUS_START_ERROR_MSG)
dispatch(add_notification(FAPD_DBUS_START_ERROR_MSG,
NotificationType.ERROR))

else:
# PROFILING
logging.debug("fapd is initiating a PROFILING session")
Expand Down Expand Up @@ -189,9 +201,15 @@ def _stop(self, instance=FapdMode.ONLINE):

if instance == FapdMode.ONLINE:
logging.debug("fapd is terminating an ONLINE session")
if (self._fapd_status != ServiceStatus.UNKNOWN) and (self._fapd_lock.acquire()):
self._fapd_ref.stop()
self._fapd_lock.release()
if (self._fapd_status != ServiceStatus.UNKNOWN):
try:
with self._fapd_lock:
self._fapd_ref.stop()

except Exception:
logging.error(FAPD_DBUS_STOP_ERROR_MSG)
dispatch(add_notification(FAPD_DBUS_STOP_ERROR_MSG,
NotificationType.ERROR))

else:
logging.debug("fapd is terminating a PROFILING session")
Expand All @@ -209,15 +227,16 @@ def _status(self):
logging.debug("fapd is currently DISABLED")
return ServiceStatus.UNKNOWN
elif self.mode == FapdMode.ONLINE:
try:
if self._fapd_lock.acquire(blocking=False):
if self._fapd_lock.acquire(blocking=False):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use a with block here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked into that approach originally. The fly/ointment thing is the blocking=False argument, i.e. if the lock is not available then don't wait and return False. Fapd control operations (start/stop) are given priority over the never-ending periodic status requests.

Still digging around. I would still prefer to have locking somewhere given the status requests and stop/start requests originate on differing threads. I don't care if it's under the hood in rust-land or not.

try:
bStatus = ServiceStatus(self._fapd_ref.is_active())
if bStatus != self._fapd_status:
logging.debug(f"_status({bStatus} updated")
self._fapd_status = bStatus
self._fapd_lock.release()
except Exception:
logging.warning("Daemon monitor query/update dispatch failed.")
except Exception:
logging.warning("Daemon status query/update failed.")

self._fapd_lock.release()
return self._fapd_status
else:
logging.debug("fapd is in a PROFILING session")
Expand All @@ -227,10 +246,15 @@ def _status(self):
return ServiceStatus.FALSE

def capture_online_state(self):
logging.debug("capture_online_state()")
updated_online_state = ServiceStatus(self._fapd_ref.is_active())
self._fapd_prior_online_state = updated_online_state
self._fapd_cur_online_state = updated_online_state
logging.debug("capture_online_state(): Capturing current online status")
with self._fapd_lock:
try:
updated_online_state = ServiceStatus(self._fapd_ref.is_active())
self._fapd_prior_online_state = updated_online_state
self._fapd_cur_online_state = updated_online_state
except Exception:
logging.warning("Daemon online status query/update failed.")

logging.debug(f"prior_online_state = {self._fapd_prior_online_state}")

def revert_online_state(self):
Expand All @@ -240,10 +264,13 @@ def revert_online_state(self):
self._start(FapdMode.ONLINE)

def initial_daemon_status(self):
if self._fapd_lock.acquire():
self._fapd_ref = Handle("fapolicyd")
if self._fapd_ref.is_valid():
self._fapd_status = ServiceStatus(self._fapd_ref.is_active())
else:
self._fapd_status = ServiceStatus.UNKNOWN
self._fapd_lock.release()
with self._fapd_lock:
try:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and again.

if self._fapd_ref.is_valid():
self._fapd_status = ServiceStatus(self._fapd_ref.is_active())
else:
self._fapd_status = ServiceStatus.UNKNOWN
except Exception as e:
print(f"Fapd Handle is_valid() or is_active() exception: {e}",
file=sys.stderr)
raise
3 changes: 3 additions & 0 deletions fapolicy_analyzer/ui/strings.py
View file
Open in desktop
Expand Up @@ -85,6 +85,9 @@
FA_ARCHIVE_FILES_FILTER_LABEL = _("fapolicyd archive files")
ANY_FILES_FILTER_LABEL = _("Any files")

FAPD_DBUS_START_ERROR_MSG = _("On-line fapolicyd start failed")
FAPD_DBUS_STOP_ERROR_MSG = _("On-line fapolicyd stop failed")

FAPROFILER_TGT_EUID_CHOWN_ERROR_MSG = _("Profiling target file chown failure")
FAPROFILER_TGT_POPEN_ERROR_MSG = _("Profiling target Popen failure")
FAPROFILER_TGT_REDIRECTION_ERROR_MSG = _("Profiling target redirection failure")
Expand Down