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 12 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
73 changes: 42 additions & 31 deletions fapolicy_analyzer/ui/fapd_manager.py
View file
Open in desktop
Expand Up @@ -15,6 +15,7 @@

import logging
import os
import sys
import subprocess


Expand Down Expand Up @@ -63,20 +64,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 +134,11 @@ 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):
with self._fapd_lock:
self._fapd_ref.start()
self.mode = FapdMode.ONLINE

else:
# PROFILING
logging.debug("fapd is initiating a PROFILING session")
Expand Down Expand Up @@ -189,9 +191,9 @@ 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):
with self._fapd_lock:
self._fapd_ref.stop()

else:
logging.debug("fapd is terminating a PROFILING session")
Expand All @@ -209,15 +211,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 +230,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 +248,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