fix: stop sequence for sshd

Author: desbma

Cargo.toml

@@ -34,7 +34,7 @@ function_name = { version = "0.3.0", default-features = false }
 goblin = { version = "0.10.1", default-features = false, features = ["std", "elf32", "elf64", "endian_fd"] }
 itertools = { version = "0.14.0", default-features = false, features = ["use_std"] }
 log = { version = "0.4.28", default-features = false, features = ["max_level_trace", "release_max_level_info"] }
-nix = { version = "0.30.1", default-features = false, features = ["fs", "user", "process"] }
+nix = { version = "0.30.1", default-features = false, features = ["fs", "user", "process", "signal"] }
 nom = { version = "8.0.0", default-features = false, features = ["std"] }
 path-clean = { version = "1.0.1", default-features = false }
 phf = { version = "0.12.1", default-features = false, features = ["std", "macros"] }

README.md

@@ -21,7 +21,7 @@ Automatic [systemd](https://systemd.io/) service hardening guided by [strace](ht
 
 ### Dependencies
 
-Strace needs to be installed and its executable available in the path. A recent Strace version is strongly recommended.
+Strace needs to be installed and its executable reachable in `PATH`. **Strace version >= 6.6 is currently required.**
 
 ### From source
 

src/main.rs

@@ -8,7 +8,9 @@ use std::{
     io,
     path::Path,
     process::Command,
-    thread,
+    sync::Arc,
+    thread::{self, sleep},
+    time::Duration,
 };
 
 use anyhow::Context as _;
@@ -90,11 +92,10 @@ fn main() -> anyhow::Result<()> {
     log::info!(
         "Detected versions: Systemd {sd_version}, Linux kernel {kernel_version}, strace {strace_version}"
     );
-    if strace_version < strace::StraceVersion::new(6, 4) {
-        log::warn!(
-            "Strace version >=6.4 is strongly recommended, if you experience strace output parsing errors, please consider upgrading"
-        );
-    }
+    anyhow::ensure!(
+        strace_version >= strace::StraceVersion::new(6, 6),
+        "Strace version >=6.6 is required"
+    );
 
     // Parse cl args
     let args = cl::Args::parse();
@@ -122,17 +123,30 @@ fn main() -> anyhow::Result<()> {
                 &hardening_opts,
             );
 
+            // Run strace
+            let cmd = command.iter().map(|a| &**a).collect::<Vec<&str>>();
+            let st = Arc::new(
+                strace::Strace::run(&cmd, strace_log_path)
+                    .context("Failed to setup strace profiling")?,
+            );
+
             // Start signal handling thread
             let mut signals = signal_hook::iterator::Signals::new([
                 signal_hook::consts::signal::SIGINT,
                 signal_hook::consts::signal::SIGQUIT,
                 signal_hook::consts::signal::SIGTERM,
             ])
             .context("Failed to setup signal handlers")?;
+            let st_sig = Arc::clone(&st);
             thread::spawn(move || {
                 for sig in signals.forever() {
-                    // The strace, and its watched child processes already get the signal, so the iterator will stop naturally
-                    log::info!("Got signal {sig:?}, ignoring");
+                    // Propagate signal to strace after this delay, in most cases everything should have already stopped
+                    const SIGNAL_STRACE_STOP_DELAY: Duration = Duration::from_secs(5);
+
+                    log::info!("Got signal {sig:?}");
+                    sleep(SIGNAL_STRACE_STOP_DELAY);
+                    log::info!("Stopping strace");
+                    st_sig.stop();
                 }
             });
 
@@ -146,11 +160,6 @@ fn main() -> anyhow::Result<()> {
                 env::current_dir().context("Failed to read current directory")?,
             );
 
-            // Run strace
-            let cmd = command.iter().map(|a| &**a).collect::<Vec<&str>>();
-            let st = strace::Strace::run(&cmd, strace_log_path)
-                .context("Failed to setup strace profiling")?;
-
             // Summarize actions
             let logs = st
                 .log_lines()

src/strace/run.rs

@@ -4,6 +4,7 @@ use std::{
     env,
     fs::File,
     io::BufReader,
+    os::unix::process::CommandExt as _,
     path::{Path, PathBuf},
     process::{Child, Command, Stdio},
 };
@@ -46,8 +47,10 @@ impl Strace {
         // Start process
         // TODO setuid/setgid execution will be broken unless strace runs as root
         let child = Command::new(STRACE_BIN)
+            .process_group(0) // set dedicated process group for easy signal handling, without affecting the shh process
             .args([
                 "--daemonize=grandchild",
+                "--kill-on-exit",
                 "--relative-timestamps",
                 "--follow-forks",
                 "--status=successful,failed",
@@ -81,6 +84,18 @@ impl Strace {
         })
     }
 
+    pub(crate) fn stop(&self) {
+        // Strace runs with `--deamonize=grandchild`, so will become child of the init process.
+        // Consequently, the pid we have is already gone, but because we have started it with a unique process group,
+        // we can still reliably kill strace.
+        // Strace also runs with `--kill-on-exit` so the subtree will be reliably killed even if the tracee created
+        // another process group (sshd does this).
+        #[expect(clippy::cast_possible_wrap)]
+        let pgid = nix::unistd::Pid::from_raw(-(self.process.id() as i32));
+        // Ignore errors because it may have already naturally stopped
+        let _ = nix::sys::signal::kill(pgid, nix::sys::signal::Signal::SIGKILL);
+    }
+
     fn pipe_path(dir: &Path) -> PathBuf {
         dir.join("strace.pipe")
     }