From 190dd5ab138d92315b26c5a14dcffd5f64e0430d Mon Sep 17 00:00:00 2001 From: Jacek Chmielewski Date: Mon, 26 Oct 2026 09:49:48 +0100 Subject: [PATCH 1/4] implement daemon interface cleanup on exit --- src-tauri/daemon/src/daemon.rs | 191 +++++++++++++++--- .../enterprise/service-locations/src/linux.rs | 40 ++++ 2 files changed, 206 insertions(+), 25 deletions(-) diff --git a/src-tauri/daemon/src/daemon.rs b/src-tauri/daemon/src/daemon.rs index 3b9622758..8bf483d22 100644 --- a/src-tauri/daemon/src/daemon.rs +++ b/src-tauri/daemon/src/daemon.rs @@ -40,6 +40,8 @@ use defguard_wireguard_rs::{ use nix::unistd::{chown, Group}; #[cfg(unix)] use tokio::net::UnixListener; +#[cfg(target_os = "linux")] +use tokio::signal::unix::{signal, SignalKind}; use tokio::{sync::mpsc, task::JoinHandle, time::interval}; #[cfg(unix)] use tokio_stream::wrappers::UnixListenerStream; @@ -121,6 +123,41 @@ impl DaemonService { } } +#[cfg(target_os = "linux")] +impl DaemonService { + fn stop_stat_tasks(&self) { + match self.stat_tasks.lock() { + Ok(mut tasks) => { + for (_, handle) in tasks.drain() { + handle.abort(); + } + } + Err(err) => error!("Failed to lock statistics tasks during shutdown: {err}"), + } + } + + fn remove_interfaces(&self) { + match self.wgapis.write() { + Ok(mut wgapis) => { + for (ifname, wgapi) in wgapis.drain() { + match wgapi.remove_interface() { + Ok(()) => info!("Removed WireGuard interface {ifname} during shutdown"), + Err(err) => error!( + "Failed to remove WireGuard interface {ifname} during shutdown: {err}" + ), + } + } + } + Err(err) => error!("Failed to lock WireGuard interfaces during shutdown: {err}"), + } + + match self.service_location_manager.write() { + Ok(mut manager) => manager.remove_interfaces(), + Err(err) => error!("Failed to lock service location manager during shutdown: {err}"), + } + } +} + /// Helper function used to perform required configuration steps for a new interface. /// /// This allows us to roll back interface creation if some configuration step fails. @@ -586,21 +623,11 @@ pub async fn run_server(config: Config) -> anyhow::Result<()> { debug!("Starting Defguard interface management daemon"); #[cfg(target_os = "linux")] - let service_location_manager = Arc::new(RwLock::new(ServiceLocationManager::init()?)); - // Nothing wakes the reconciler on Linux - there are no network, logon or resume watchers - so - // the tick is its only trigger. + let mut interrupt = signal(SignalKind::interrupt())?; #[cfg(target_os = "linux")] - let reconciler_handle = tokio::spawn(run_reconciler( - service_location_manager.clone(), - ReconcileSignal::default(), - SERVICE_LOCATION_RECONCILE_INTERVAL, - )); - - let daemon_service = DaemonService::new( - &config, - #[cfg(target_os = "linux")] - service_location_manager, - ); + let mut terminate = signal(SignalKind::terminate())?; + #[cfg(target_os = "linux")] + let service_location_manager = Arc::new(RwLock::new(ServiceLocationManager::init()?)); // Remove existing socket if it exists if Path::new(DAEMON_SOCKET_PATH).exists() { @@ -633,6 +660,21 @@ pub async fn run_server(config: Config) -> anyhow::Result<()> { fs::set_permissions(DAEMON_SOCKET_PATH, fs::Permissions::from_mode(0o660))?; } + let daemon_service = Arc::new(DaemonService::new( + &config, + #[cfg(target_os = "linux")] + Arc::clone(&service_location_manager), + )); + + // Nothing wakes the reconciler on Linux - there are no network, logon or resume watchers - so + // the tick is its only trigger. + #[cfg(target_os = "linux")] + let mut reconciler_handle = tokio::spawn(run_reconciler( + service_location_manager, + ReconcileSignal::default(), + SERVICE_LOCATION_RECONCILE_INTERVAL, + )); + let uds_stream = UnixListenerStream::new(uds); info!("Defguard daemon version {VERSION} started, listening on socket {DAEMON_SOCKET_PATH}",); @@ -640,24 +682,60 @@ pub async fn run_server(config: Config) -> anyhow::Result<()> { let server = Server::builder() .trace_fn(|_| tracing::info_span!("defguard_client_service")) - .add_service(DesktopDaemonServiceServer::new(daemon_service)) - .serve_with_incoming(uds_stream); + .add_service(DesktopDaemonServiceServer::from_arc(Arc::clone( + &daemon_service, + ))); #[cfg(target_os = "linux")] - tokio::select! { - result = server => result?, - result = reconciler_handle => { - let message = match result { - Ok(()) => "Service location reconciler ended unexpectedly".to_string(), - Err(err) => format!("Service location reconciler task failed: {err}"), - }; - error!("{message}"); + { + let mut reconciler_error = None; + let result = server + .serve_with_incoming_shutdown(uds_stream, async { + let reconciler_result = tokio::select! { + _ = interrupt.recv() => { + info!("Received SIGINT, stopping Defguard daemon"); + None + } + _ = terminate.recv() => { + info!("Received SIGTERM, stopping Defguard daemon"); + None + } + result = &mut reconciler_handle => Some(result), + }; + + if let Some(result) = reconciler_result { + let message = match result { + Ok(()) => "Service location reconciler ended unexpectedly".to_string(), + Err(err) => format!("Service location reconciler task failed: {err}"), + }; + error!("{message}"); + reconciler_error = Some(message); + } else { + reconciler_handle.abort(); + let _ = (&mut reconciler_handle).await; + } + + // close statistics streams so they do not hold up graceful server shutdown + daemon_service.stop_stat_tasks(); + }) + .await; + + // server errors can bypass the shutdown future + if !reconciler_handle.is_finished() { + reconciler_handle.abort(); + let _ = reconciler_handle.await; + } + daemon_service.stop_stat_tasks(); + daemon_service.remove_interfaces(); + + result?; + if let Some(message) = reconciler_error { return Err(anyhow::anyhow!(message)); } } #[cfg(not(target_os = "linux"))] - server.await?; + server.serve_with_incoming(uds_stream).await?; Ok(()) } @@ -683,3 +761,66 @@ pub(crate) async fn run_server( Ok(()) } + +#[cfg(all(test, target_os = "linux"))] +mod tests { + use super::*; + + #[tokio::test] + async fn test_stopping_stat_tasks_closes_streams() { + let service = DaemonService::default(); + let mut receivers = Vec::new(); + for ifname in ["wg0", "wg1"] { + let (tx, rx) = mpsc::channel::<()>(1); + let handle = tokio::spawn(async move { + tx.closed().await; + }); + service + .stat_tasks + .lock() + .unwrap() + .insert(ifname.to_string(), handle); + receivers.push(rx); + } + + service.stop_stat_tasks(); + + assert!(service.stat_tasks.lock().unwrap().is_empty()); + for mut rx in receivers { + assert_eq!( + tokio::time::timeout(Duration::from_secs(1), rx.recv()) + .await + .expect("statistics stream remained open"), + None + ); + } + } + + #[test] + #[ignore = "requires an isolated network namespace, CAP_NET_ADMIN and a DNS backend"] + fn test_remove_interfaces_continues_after_failure_and_preserves_unmanaged_interface() { + let service = DaemonService::default(); + let mut unmanaged = WG::new("dgd-other").unwrap(); + unmanaged.create_interface().unwrap(); + let ifnames = ["dgd-test0", "dgd-test1", "dgd-test2"]; + { + let mut wgapis = service.wgapis.write().unwrap(); + for ifname in ifnames { + wgapis.insert(ifname.into(), WG::new(ifname).unwrap()); + } + // leave the first interface absent to exercise failure before successful removals + for (_, wgapi) in wgapis.iter_mut().skip(1) { + wgapi.create_interface().unwrap(); + } + } + + service.remove_interfaces(); + + assert!(service.wgapis.read().unwrap().is_empty()); + for ifname in ifnames { + assert!(WG::new(ifname).unwrap().read_interface_data().is_err()); + } + assert!(unmanaged.read_interface_data().is_ok()); + unmanaged.remove_interface().unwrap(); + } +} diff --git a/src-tauri/enterprise/service-locations/src/linux.rs b/src-tauri/enterprise/service-locations/src/linux.rs index cfdadbed2..84fde7b03 100644 --- a/src-tauri/enterprise/service-locations/src/linux.rs +++ b/src-tauri/enterprise/service-locations/src/linux.rs @@ -263,6 +263,18 @@ impl ServiceLocationManager { Ok(()) } + pub fn remove_interfaces(&mut self) { + let ifnames = self.wgapis.keys().cloned().collect::>(); + for ifname in ifnames { + if let Err(err) = self.remove_tracked_interface(&ifname) { + error!( + "Failed to remove Linux service location interface {ifname} during shutdown: \ + {err}" + ); + } + } + } + pub fn disconnect_service_locations_by_instance( &mut self, instance_id: &str, @@ -688,6 +700,34 @@ impl ServiceLocationManager { mod tests { use super::*; + #[test] + #[ignore = "requires an isolated network namespace, CAP_NET_ADMIN and a DNS backend"] + fn test_remove_interfaces_continues_after_failure_and_preserves_unmanaged_interface() { + let mut manager = ServiceLocationManager::default(); + let mut unmanaged: WGApi = WGApi::new("dgl-other").unwrap(); + unmanaged.create_interface().unwrap(); + let ifnames = ["dgl-test0", "dgl-test1", "dgl-test2"]; + for ifname in ifnames { + manager.wgapis.insert(ifname.into(), WGApi::new(ifname).unwrap()); + } + let missing = manager.wgapis.keys().next().unwrap().clone(); + // leave the first interface absent to exercise failure before successful removals + for (_, wgapi) in manager.wgapis.iter_mut().skip(1) { + wgapi.create_interface().unwrap(); + } + + manager.remove_interfaces(); + + assert_eq!(manager.wgapis.len(), 1); + assert!(manager.wgapis.contains_key(&missing)); + for ifname in ifnames { + let wgapi: WGApi = WGApi::new(ifname).unwrap(); + assert!(wgapi.read_interface_data().is_err()); + } + assert!(unmanaged.read_interface_data().is_ok()); + unmanaged.remove_interface().unwrap(); + } + #[test] fn test_removing_a_preshared_key_emits_an_explicit_zero_key() { assert_eq!(preshared_key_update(None).unwrap(), Key::default()); From 27fc9e6ce476f189ce11502363c3d961e8ce32d1 Mon Sep 17 00:00:00 2001 From: Jacek Chmielewski Date: Mon, 26 Oct 2026 12:27:40 +0100 Subject: [PATCH 2/4] installation script fixes --- resources-linux/postinst | 3 +-- resources-linux/postrm | 11 ++++++++++- resources-linux/prerm | 2 +- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/resources-linux/postinst b/resources-linux/postinst index b42221e18..de504d733 100644 --- a/resources-linux/postinst +++ b/resources-linux/postinst @@ -80,11 +80,10 @@ case "$1" in # ($2 holds the previous version) restart it so the new daemon binary # is loaded - otherwise an already-running old daemon keeps serving and # lacks newly added RPCs (e.g. list_interfaces used by the CLI). - # try-restart leaves a deliberately-stopped service stopped. if [ -z "$2" ]; then systemctl start "$SERVICE_NAME" else - systemctl try-restart "$SERVICE_NAME" + systemctl restart "$SERVICE_NAME" fi fi ;; diff --git a/resources-linux/postrm b/resources-linux/postrm index 924e6eb03..5581928c3 100644 --- a/resources-linux/postrm +++ b/resources-linux/postrm @@ -5,7 +5,16 @@ GROUP_NAME="defguard" SERVICE_NAME="defguard-service" case "$1" in - 0|1|remove) + 1) + # rpm runs this for the old package after installing the new version + if [ -d /run/systemd/system ]; then + systemctl daemon-reload + systemctl enable "$SERVICE_NAME" + systemctl restart "$SERVICE_NAME" + fi + ;; + + 0|remove) # Service file still exists, just disable it if [ -d /run/systemd/system ]; then systemctl disable "$SERVICE_NAME" || true diff --git a/resources-linux/prerm b/resources-linux/prerm index 40dbc1e33..9e12e5567 100644 --- a/resources-linux/prerm +++ b/resources-linux/prerm @@ -4,7 +4,7 @@ set -e SERVICE_NAME="defguard-service" case "$1" in - 0|1|remove|upgrade|deconfigure) + 0|remove|upgrade|deconfigure) if [ -d /run/systemd/system ]; then # Stop the service before removal/upgrade systemctl stop "$SERVICE_NAME" || true From 071304a614063b7e525d0465f3e26975f5f15105 Mon Sep 17 00:00:00 2001 From: Jacek Chmielewski Date: Tue, 27 Oct 2026 11:36:11 +0100 Subject: [PATCH 3/4] remove ignored tests --- src-tauri/daemon/src/daemon.rs | 28 ------------------- .../enterprise/service-locations/src/linux.rs | 28 ------------------- 2 files changed, 56 deletions(-) diff --git a/src-tauri/daemon/src/daemon.rs b/src-tauri/daemon/src/daemon.rs index 8bf483d22..b66652f63 100644 --- a/src-tauri/daemon/src/daemon.rs +++ b/src-tauri/daemon/src/daemon.rs @@ -795,32 +795,4 @@ mod tests { ); } } - - #[test] - #[ignore = "requires an isolated network namespace, CAP_NET_ADMIN and a DNS backend"] - fn test_remove_interfaces_continues_after_failure_and_preserves_unmanaged_interface() { - let service = DaemonService::default(); - let mut unmanaged = WG::new("dgd-other").unwrap(); - unmanaged.create_interface().unwrap(); - let ifnames = ["dgd-test0", "dgd-test1", "dgd-test2"]; - { - let mut wgapis = service.wgapis.write().unwrap(); - for ifname in ifnames { - wgapis.insert(ifname.into(), WG::new(ifname).unwrap()); - } - // leave the first interface absent to exercise failure before successful removals - for (_, wgapi) in wgapis.iter_mut().skip(1) { - wgapi.create_interface().unwrap(); - } - } - - service.remove_interfaces(); - - assert!(service.wgapis.read().unwrap().is_empty()); - for ifname in ifnames { - assert!(WG::new(ifname).unwrap().read_interface_data().is_err()); - } - assert!(unmanaged.read_interface_data().is_ok()); - unmanaged.remove_interface().unwrap(); - } } diff --git a/src-tauri/enterprise/service-locations/src/linux.rs b/src-tauri/enterprise/service-locations/src/linux.rs index 84fde7b03..aba0cf04f 100644 --- a/src-tauri/enterprise/service-locations/src/linux.rs +++ b/src-tauri/enterprise/service-locations/src/linux.rs @@ -700,34 +700,6 @@ impl ServiceLocationManager { mod tests { use super::*; - #[test] - #[ignore = "requires an isolated network namespace, CAP_NET_ADMIN and a DNS backend"] - fn test_remove_interfaces_continues_after_failure_and_preserves_unmanaged_interface() { - let mut manager = ServiceLocationManager::default(); - let mut unmanaged: WGApi = WGApi::new("dgl-other").unwrap(); - unmanaged.create_interface().unwrap(); - let ifnames = ["dgl-test0", "dgl-test1", "dgl-test2"]; - for ifname in ifnames { - manager.wgapis.insert(ifname.into(), WGApi::new(ifname).unwrap()); - } - let missing = manager.wgapis.keys().next().unwrap().clone(); - // leave the first interface absent to exercise failure before successful removals - for (_, wgapi) in manager.wgapis.iter_mut().skip(1) { - wgapi.create_interface().unwrap(); - } - - manager.remove_interfaces(); - - assert_eq!(manager.wgapis.len(), 1); - assert!(manager.wgapis.contains_key(&missing)); - for ifname in ifnames { - let wgapi: WGApi = WGApi::new(ifname).unwrap(); - assert!(wgapi.read_interface_data().is_err()); - } - assert!(unmanaged.read_interface_data().is_ok()); - unmanaged.remove_interface().unwrap(); - } - #[test] fn test_removing_a_preshared_key_emits_an_explicit_zero_key() { assert_eq!(preshared_key_update(None).unwrap(), Key::default()); From 9f75b202ee0d94e96d0b7ffd925ba8058c9145f0 Mon Sep 17 00:00:00 2001 From: Jacek Chmielewski Date: Tue, 27 Oct 2026 13:25:59 +0100 Subject: [PATCH 4/4] centralize linux daemon shutdown sequence --- src-tauri/daemon/src/daemon.rs | 85 ++++++++++++++++++++-------------- 1 file changed, 50 insertions(+), 35 deletions(-) diff --git a/src-tauri/daemon/src/daemon.rs b/src-tauri/daemon/src/daemon.rs index b66652f63..28de140bc 100644 --- a/src-tauri/daemon/src/daemon.rs +++ b/src-tauri/daemon/src/daemon.rs @@ -41,7 +41,10 @@ use nix::unistd::{chown, Group}; #[cfg(unix)] use tokio::net::UnixListener; #[cfg(target_os = "linux")] -use tokio::signal::unix::{signal, SignalKind}; +use tokio::{ + signal::unix::{signal, SignalKind}, + sync::oneshot, +}; use tokio::{sync::mpsc, task::JoinHandle, time::interval}; #[cfg(unix)] use tokio_stream::wrappers::UnixListenerStream; @@ -688,44 +691,56 @@ pub async fn run_server(config: Config) -> anyhow::Result<()> { #[cfg(target_os = "linux")] { - let mut reconciler_error = None; - let result = server - .serve_with_incoming_shutdown(uds_stream, async { - let reconciler_result = tokio::select! { - _ = interrupt.recv() => { - info!("Received SIGINT, stopping Defguard daemon"); - None - } - _ = terminate.recv() => { - info!("Received SIGTERM, stopping Defguard daemon"); - None - } - result = &mut reconciler_handle => Some(result), - }; + enum ShutdownReason { + Signal, + Server(Result<(), tonic::transport::Error>), + Reconciler(Result<(), tokio::task::JoinError>), + } - if let Some(result) = reconciler_result { - let message = match result { - Ok(()) => "Service location reconciler ended unexpectedly".to_string(), - Err(err) => format!("Service location reconciler task failed: {err}"), - }; - error!("{message}"); - reconciler_error = Some(message); - } else { - reconciler_handle.abort(); - let _ = (&mut reconciler_handle).await; - } + let (shutdown_tx, shutdown_rx) = oneshot::channel(); + let server = server.serve_with_incoming_shutdown(uds_stream, async { + let _ = shutdown_rx.await; + }); + tokio::pin!(server); - // close statistics streams so they do not hold up graceful server shutdown - daemon_service.stop_stat_tasks(); - }) - .await; + let reason = tokio::select! { + _ = interrupt.recv() => { + info!("Received SIGINT, stopping Defguard daemon"); + ShutdownReason::Signal + } + _ = terminate.recv() => { + info!("Received SIGTERM, stopping Defguard daemon"); + ShutdownReason::Signal + } + result = &mut reconciler_handle => ShutdownReason::Reconciler(result), + result = &mut server => ShutdownReason::Server(result), + }; - // server errors can bypass the shutdown future - if !reconciler_handle.is_finished() { - reconciler_handle.abort(); - let _ = reconciler_handle.await; - } + let reconciler_error = match &reason { + ShutdownReason::Reconciler(result) => { + let message = match result { + Ok(()) => "Service location reconciler ended unexpectedly".to_string(), + Err(err) => format!("Service location reconciler task failed: {err}"), + }; + error!("{message}"); + Some(message) + } + _ => { + reconciler_handle.abort(); + let _ = reconciler_handle.await; + None + } + }; + + // close statistics streams so they do not hold up graceful server shutdown daemon_service.stop_stat_tasks(); + let result = match reason { + ShutdownReason::Server(result) => result, + _ => { + let _ = shutdown_tx.send(()); + server.await + } + }; daemon_service.remove_interfaces(); result?;