Active checks with dbus

crates/daemon/src/error.rs

@@ -6,7 +6,6 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
-use std::string::FromUtf8Error;
 use thiserror::Error;
 
 // errors that can occur in this crate
@@ -23,7 +22,4 @@ pub enum Error {
 
     #[error("{0}")]
     ServiceCheckFailure(String),
-
-    #[error("Failed to parse systemctl output")]
-    ServiceCheckParseFailure(#[from] FromUtf8Error),
 }

crates/daemon/src/fapolicyd.rs

@@ -8,13 +8,6 @@
 
 // todo;; tracking the fapolicyd specific bits in here to determine if bindings are worthwhile
 
-use std::thread::sleep;
-use std::time::Duration;
-
-use crate::error::Error;
-use crate::error::Error::FapolicydReloadFail;
-use crate::svc::Handle;
-
 pub const TRUST_DB_PATH: &str = "/var/lib/fapolicyd";
 pub const TRUST_DB_NAME: &str = "trust.db";
 pub const TRUST_FILE_PATH: &str = "/etc/fapolicyd/fapolicyd.trust";
@@ -33,28 +26,6 @@ pub enum Version {
     Release { major: u8, minor: u8, patch: u8 },
 }
 
-const RETRIES: u8 = 15;
-pub fn reload() -> Result<(), Error> {
-    let fapolicyd = Handle::default();
-    fapolicyd.stop()?;
-    for _ in 0..RETRIES {
-        sleep(Duration::from_secs(1));
-        if !fapolicyd.active()? {
-            fapolicyd.start()?;
-            break;
-        }
-    }
-    for _ in 0..RETRIES {
-        sleep(Duration::from_secs(1));
-        if fapolicyd.active()? {
-            return Ok(());
-        }
-    }
-    Err(FapolicydReloadFail(
-        "Could not reload after 10 tries".to_string(),
-    ))
-}
-
 /// filtering logic as implemented by fapolicyd rpm backend
 pub(crate) fn keep_entry(p: &str) -> bool {
     match p {

crates/daemon/src/lib.rs

@@ -8,7 +8,6 @@
 
 pub mod error;
 pub mod fapolicyd;
-pub use fapolicyd::reload;
 
 pub mod rpm;
 pub use rpm::fapolicyd_version as version;

crates/daemon/src/svc.rs

@@ -6,8 +6,8 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
+use dbus::arg::messageitem::MessageItem;
 use std::fmt;
-use std::process::Command;
 use std::time::Duration;
 
 use dbus::blocking::{BlockingSender, Connection};
@@ -52,6 +52,7 @@ fn msg(m: Method, unit: &str) -> Result<Message, Error> {
 #[derive(Clone)]
 pub struct Handle {
     name: String,
+    unit: String,
 }
 
 impl Default for Handle {
@@ -60,58 +61,94 @@ impl Default for Handle {
     }
 }
 
+#[derive(Clone, Debug, PartialEq)]
+pub enum State {
+    Active,
+    Inactive,
+    Failed,
+    Other(String),
+}
+
+impl State {
+    pub fn can_be(&self, other: State) -> bool {
+        use State::*;
+
+        match self {
+            Inactive if other == Failed => true,
+            _ => *self == other,
+        }
+    }
+}
+
 impl Handle {
     pub fn new(name: &str) -> Handle {
         Handle {
-            name: format!("{}.service", name),
+            name: name.to_string(),
+            unit: format!("{}.service", name),
         }
     }
 
     pub fn start(&self) -> Result<(), Error> {
-        msg(StartUnit, &self.name).and_then(call).map(|_| ())
+        msg(StartUnit, &self.unit).and_then(call).map(|_| ())
     }
 
     pub fn stop(&self) -> Result<(), Error> {
-        msg(StopUnit, &self.name).and_then(call).map(|_| ())
+        msg(StopUnit, &self.unit).and_then(call).map(|_| ())
     }
 
     pub fn enable(&self) -> Result<(), Error> {
-        msg(EnableUnitFiles, &self.name).and_then(call).map(|_| ())
+        msg(EnableUnitFiles, &self.unit).and_then(call).map(|_| ())
     }
 
     pub fn disable(&self) -> Result<(), Error> {
-        msg(DisableUnitFiles, &self.name).and_then(call).map(|_| ())
+        msg(DisableUnitFiles, &self.unit).and_then(call).map(|_| ())
     }
 
-    // todo;; replace with direct dbus calls
-    // systemctl return codes
-    // 0 - unit is active
-    // 1 - unit not failed
-    // 2 - unused
-    // 3 - unit is not active
-    // 4 - no such unit
     pub fn active(&self) -> Result<bool, Error> {
-        Command::new("systemctl")
-            .arg("--no-pager")
-            .arg("-n0")
-            .arg("status")
-            .arg(&self.name)
-            .output()
-            .map(|o| {
-                if o.status.success() {
-                    Ok(String::from_utf8(o.stdout)?)
-                } else {
-                    match o.status.code() {
-                        Some(1 | 3) => Ok(String::from_utf8(o.stdout)?),
-                        Some(4) => Err(ServiceCheckFailure(
-                            String::from_utf8(o.stderr)?.trim().into(),
-                        )),
-                        // unlikely; either got an unused 2-code or a sigint
-                        _ => Err(ServiceCheckFailure("Unexpected".into())),
-                    }
-                }
+        self.state().map(|state| matches!(state, State::Active))
+    }
+
+    pub fn state(&self) -> Result<State, Error> {
+        use dbus::arg::messageitem::Props;
+        use dbus::ffidisp::Connection;
+        use State::*;
+
+        let c = Connection::new_system()?;
+        let p = Props::new(
+            &c,
+            "org.freedesktop.systemd1",
+            // todo;; the path name may need to be fetched dynamically via a Message
+            format!("/org/freedesktop/systemd1/unit/{}_2eservice", self.name),
+            "org.freedesktop.systemd1.Unit",
+            5000,
+        );
+
+        if let MessageItem::Str(state) = p.get("ActiveState")? {
+            Ok(match state.as_str() {
+                "active" => Active,
+                "inactive" => Inactive,
+                "failed" => Failed,
+                _ => Other(state),
             })
-            .map_err(|_| ServiceCheckFailure("Failed to execute systemctl".into()))?
-            .map(|txt| txt.contains("Active: active"))
+        } else {
+            Err(ServiceCheckFailure(
+                "DBUS unit active check failed".to_string(),
+            ))
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::svc::State::*;
+
+    #[test]
+    fn state_can_be() {
+        // Failed is Inactive
+        assert!(Inactive.can_be(Failed));
+        // Inactive is not Failed
+        assert!(!Failed.can_be(Inactive));
+        // Identity
+        assert!(Active.can_be(Active))
     }
 }

crates/pyo3/src/daemon.rs

@@ -8,7 +8,7 @@
 
 use crate::system::PySystem;
 use fapolicy_daemon::fapolicyd::Version;
-use fapolicy_daemon::svc::Handle;
+use fapolicy_daemon::svc::{Handle, State};
 use pyo3::exceptions::PyRuntimeError;
 use pyo3::prelude::*;
 use std::thread::sleep;
@@ -81,7 +81,7 @@ impl PyHandle {
 fn start_fapolicyd() -> PyResult<()> {
     match Handle::default().start() {
         Ok(_) => {
-            println!("starting fapolicyd daemon");
+            eprintln!("starting fapolicyd daemon");
             Ok(())
         }
         Err(e) => Err(PyRuntimeError::new_err(format!("{:?}", e))),
@@ -92,7 +92,7 @@ fn start_fapolicyd() -> PyResult<()> {
 fn stop_fapolicyd() -> PyResult<()> {
     match Handle::default().stop() {
         Ok(_) => {
-            println!("stopped fapolicyd daemon");
+            eprintln!("stopped fapolicyd daemon");
             Ok(())
         }
         Err(e) => Err(PyRuntimeError::new_err(format!("{:?}", e))),
@@ -111,13 +111,17 @@ fn fapolicyd_version() -> Option<String> {
     }
 }
 
-#[pyfunction]
-fn rollback_fapolicyd(to: PySystem) -> PyResult<()> {
+pub(crate) fn deploy(system: &PySystem) -> PyResult<()> {
     stop_fapolicyd()
-        .and_then(|_| wait_for_daemon(State::Down))
-        .and_then(|_| to.deploy_only())
+        .and_then(|_| wait_for_daemon(State::Inactive))
+        .and_then(|_| system.deploy_only())
         .and_then(|_| start_fapolicyd())
-        .and_then(|_| wait_for_daemon(State::Up))
+        .and_then(|_| wait_for_daemon(State::Active))
+}
+
+#[pyfunction]
+fn rollback_fapolicyd(to: PySystem) -> PyResult<()> {
+    deploy(&to)
 }
 
 #[pyfunction]
@@ -127,24 +131,27 @@ fn is_fapolicyd_active() -> PyResult<bool> {
         .map_err(|e| PyRuntimeError::new_err(format!("{:?}", e)))
 }
 
-enum State {
-    Up,
-    Down,
-}
-
-fn wait_for_daemon(state: State) -> PyResult<()> {
-    let dir: bool = matches!(state, State::Up);
-    for _ in 0..10 {
+fn wait_for_daemon(target_state: State) -> PyResult<()> {
+    for _ in 0..15 {
+        eprintln!("waiting on daemon to be {target_state:?}...");
         sleep(Duration::from_secs(1));
-        if dir
-            == Handle::default()
-                .active()
-                .map_err(|e| PyRuntimeError::new_err(format!("{:?}", e)))?
+        if Handle::default()
+            .state()
+            .map(|state| target_state.can_be(state))
+            .unwrap_or(false)
         {
+            eprintln!("done waiting, daemon is {target_state:?}");
             return Ok(());
         }
     }
-    Err(PyRuntimeError::new_err("Daemon unresponsive"))
+
+    let actual_state = Handle::default()
+        .state()
+        .map_err(|e| PyRuntimeError::new_err(format!("{:?}", e)))?;
+
+    Err(PyRuntimeError::new_err(format!(
+        "Daemon is unresponsive in {actual_state:?} state"
+    )))
 }
 
 pub fn init_module(_py: Python, m: &PyModule) -> PyResult<()> {

crates/pyo3/src/system.rs

@@ -19,9 +19,9 @@ use fapolicy_app::sys::deploy_app_state;
 use super::trust::PyTrust;
 use crate::acl::{PyGroup, PyUser};
 use crate::analysis::PyEventLog;
-use crate::rules;
 use crate::rules::PyRule;
 use crate::trust;
+use crate::{daemon, rules};
 
 #[pyclass(module = "app", name = "System")]
 #[derive(Clone)]
@@ -99,10 +99,7 @@ impl PySystem {
 
     /// Update the host system with this state of this System and signal fapolicyd to reload trust
     pub fn deploy(&self) -> PyResult<()> {
-        self.deploy_only().and_then(|_| {
-            fapolicy_daemon::reload()
-                .map_err(|e| exceptions::PyRuntimeError::new_err(format!("{:?}", e)))
-        })
+        daemon::deploy(self).map_err(|e| exceptions::PyRuntimeError::new_err(format!("{:?}", e)))
     }
 
     /// Update the host system with this state of this System