agent: harden watchdog parent identity checks

Author: rvql

agent/Cargo.toml

@@ -126,7 +126,7 @@ wasmtime = "12.0.1"
 wasmtime-wasi = "12.0.1"
 zstd = "0.13.2"
 
-[target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies]
+[target.'cfg(unix)'.dependencies]
 cgroups-rs = "0.2.9"
 nix = "0.23"
 # As of procfs 0.16.0, Process::fd().iter() still not giving correct results on kernel 2.6.32

agent/src/main.rs

@@ -79,6 +79,10 @@ struct Opts {
     #[clap(long, hide = true)]
     watchdog_parent_pid: Option<u32>,
 
+    #[cfg(unix)]
+    #[clap(long, hide = true)]
+    watchdog_parent_start_time: Option<u64>,
+
     #[cfg(unix)]
     #[clap(long, hide = true)]
     watchdog_liveness_url: Option<String>,
@@ -128,6 +132,8 @@ fn main() -> Result<()> {
     if let Some(parent_pid) = opts.watchdog_parent_pid {
         return watchdog::run(
             parent_pid,
+            opts.watchdog_parent_start_time
+                .ok_or_else(|| anyhow::anyhow!("watchdog parent start time is required"))?,
             opts.watchdog_liveness_url
                 .as_deref()
                 .ok_or_else(|| anyhow::anyhow!("watchdog liveness url is required"))?,

agent/src/watchdog.rs

@@ -30,6 +30,7 @@ use nix::{
     sys::signal::{kill, Signal},
     unistd::Pid,
 };
+use procfs::process::Process;
 
 use crate::utils::environment::get_executable_path;
 
@@ -38,6 +39,12 @@ pub const WATCHDOG_PERIOD: Duration = Duration::from_secs(10);
 pub const WATCHDOG_HTTP_TIMEOUT: Duration = Duration::from_secs(3);
 pub const WATCHDOG_TERMINATION_GRACE: Duration = Duration::from_secs(10);
 
+#[derive(Clone, Copy, Debug)]
+struct ParentIdentity {
+    pid: Pid,
+    start_time_ticks: u64,
+}
+
 pub fn liveness_url(port: u16) -> String {
     format!("http://127.0.0.1:{port}/livez")
 }
@@ -79,20 +86,23 @@ impl ProbeTarget {
     }
 }
 
-pub fn run(parent_pid: u32, liveness_url: &str) -> Result<()> {
-    let parent_pid = Pid::from_raw(parent_pid as i32);
+pub fn run(parent_pid: u32, parent_start_time_ticks: u64, liveness_url: &str) -> Result<()> {
+    let parent = ParentIdentity {
+        pid: Pid::from_raw(parent_pid as i32),
+        start_time_ticks: parent_start_time_ticks,
+    };
     let probe_target = ProbeTarget::from_url(liveness_url)?;
     let mut consecutive_failures = 0;
 
     eprintln!(
-        "[watchdog] monitoring parent pid {} via {}",
-        parent_pid, liveness_url
+        "[watchdog] monitoring parent pid {} start_time={} via {}",
+        parent.pid, parent.start_time_ticks, liveness_url
     );
     loop {
-        if !process_exists(parent_pid)? {
+        if !process_matches(parent)? {
             eprintln!(
-                "[watchdog] parent pid {} exited, watchdog stopping",
-                parent_pid
+                "[watchdog] parent pid {} no longer matches original identity, watchdog stopping",
+                parent.pid
             );
             return Ok(());
         }
@@ -105,50 +115,82 @@ pub fn run(parent_pid: u32, liveness_url: &str) -> Result<()> {
                 consecutive_failures += 1;
                 eprintln!(
                     "[watchdog] liveness returned unhealthy status for parent pid {}, consecutive_failures={}",
-                    parent_pid, consecutive_failures
+                        parent.pid, consecutive_failures
                 );
             }
             Err(e) => {
                 consecutive_failures += 1;
                 eprintln!(
                     "[watchdog] liveness probe for parent pid {} failed: {}, consecutive_failures={}",
-                    parent_pid, e, consecutive_failures
+                        parent.pid, e, consecutive_failures
                 );
             }
         }
 
         if consecutive_failures >= WATCHDOG_FAILURE_THRESHOLD {
             eprintln!(
                 "[watchdog] parent pid {} exceeded liveness failure threshold {}, restarting",
-                parent_pid, WATCHDOG_FAILURE_THRESHOLD
+                parent.pid, WATCHDOG_FAILURE_THRESHOLD
             );
-            return terminate_parent(parent_pid);
+            return terminate_parent(parent);
         }
 
         std::thread::sleep(WATCHDOG_PERIOD);
     }
 }
 
 pub fn spawn(parent_pid: u32, liveness_url: &str) -> Result<Child> {
+    let parent = ParentIdentity {
+        pid: Pid::from_raw(parent_pid as i32),
+        start_time_ticks: read_process_start_time(Pid::from_raw(parent_pid as i32))?.ok_or_else(
+            || {
+                anyhow!(
+                    "parent pid {} disappeared before watchdog spawn",
+                    parent_pid
+                )
+            },
+        )?,
+    };
     let binary = get_executable_path().context("get executable path for watchdog failed")?;
     Command::new(binary)
         .arg("--watchdog-parent-pid")
-        .arg(parent_pid.to_string())
+        .arg(parent.pid.as_raw().to_string())
+        .arg("--watchdog-parent-start-time")
+        .arg(parent.start_time_ticks.to_string())
         .arg("--watchdog-liveness-url")
         .arg(liveness_url)
         .spawn()
         .context("spawn watchdog failed")
 }
 
-fn process_exists(pid: Pid) -> Result<bool> {
-    match kill(pid, None) {
+fn process_matches(parent: ParentIdentity) -> Result<bool> {
+    match kill(parent.pid, None) {
         Ok(_) => Ok(true),
         Err(Errno::EPERM) => Ok(true),
         Err(Errno::ESRCH) => Ok(false),
-        Err(e) => Err(anyhow!("check parent pid {} failed: {}", pid, e)),
+        Err(e) => Err(anyhow!("check parent pid {} failed: {}", parent.pid, e)),
+    }?;
+
+    match read_process_start_time(parent.pid)? {
+        Some(start_time_ticks) => Ok(start_time_ticks == parent.start_time_ticks),
+        None => Ok(false),
     }
 }
 
+fn read_process_start_time(pid: Pid) -> Result<Option<u64>> {
+    let process = match Process::new(pid.as_raw()) {
+        Ok(process) => process,
+        Err(procfs::ProcError::NotFound(_)) => return Ok(None),
+        Err(e) => {
+            return Err(anyhow!("read /proc/{}/stat failed: {}", pid, e));
+        }
+    };
+    let stat = process
+        .stat()
+        .with_context(|| format!("read /proc/{}/stat failed", pid))?;
+    Ok(Some(stat.starttime))
+}
+
 fn parse_addr(uri: &Uri) -> Result<SocketAddr> {
     let host = uri
         .host()
@@ -163,44 +205,52 @@ fn parse_addr(uri: &Uri) -> Result<SocketAddr> {
         .ok_or_else(|| anyhow!("resolve watchdog host {host}:{port} failed"))
 }
 
-fn terminate_parent(parent_pid: Pid) -> Result<()> {
-    match kill(parent_pid, Signal::SIGTERM) {
-        Ok(_) => eprintln!("[watchdog] sent SIGTERM to parent pid {}", parent_pid),
+fn terminate_parent(parent: ParentIdentity) -> Result<()> {
+    if !process_matches(parent)? {
+        eprintln!(
+            "[watchdog] parent pid {} no longer matches original identity, skip termination",
+            parent.pid
+        );
+        return Ok(());
+    }
+
+    match kill(parent.pid, Signal::SIGTERM) {
+        Ok(_) => eprintln!("[watchdog] sent SIGTERM to parent pid {}", parent.pid),
         Err(Errno::ESRCH) => return Ok(()),
         Err(e) => {
             return Err(anyhow!(
                 "send SIGTERM to parent pid {} failed: {}",
-                parent_pid,
+                parent.pid,
                 e
             ))
         }
     }
 
     let deadline = Instant::now() + WATCHDOG_TERMINATION_GRACE;
     while Instant::now() < deadline {
-        if !process_exists(parent_pid)? {
-            eprintln!("[watchdog] parent pid {} exited after SIGTERM", parent_pid);
+        if !process_matches(parent)? {
+            eprintln!("[watchdog] parent pid {} exited after SIGTERM", parent.pid);
             return Ok(());
         }
         std::thread::sleep(Duration::from_millis(200));
     }
 
-    if !process_exists(parent_pid)? {
+    if !process_matches(parent)? {
         return Ok(());
     }
 
-    match kill(parent_pid, Signal::SIGKILL) {
+    match kill(parent.pid, Signal::SIGKILL) {
         Ok(_) => {
             eprintln!(
                 "[watchdog] parent pid {} did not exit in {:?}, sent SIGKILL",
-                parent_pid, WATCHDOG_TERMINATION_GRACE
+                parent.pid, WATCHDOG_TERMINATION_GRACE
             );
             Ok(())
         }
         Err(Errno::ESRCH) => Ok(()),
         Err(e) => Err(anyhow!(
             "send SIGKILL to parent pid {} failed: {}",
-            parent_pid,
+            parent.pid,
             e
         )),
     }