From 31bba0fc64342808d75b038faca1b4db604fb563 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Tue, 28 Jul 2026 20:37:24 +0200 Subject: [PATCH 1/9] update flake inputs --- flake.lock | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/flake.lock b/flake.lock index 480dea16..5105eea3 100644 --- a/flake.lock +++ b/flake.lock @@ -20,11 +20,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1777268161, - "narHash": "sha256-bxrdOn8SCOv8tN4JbTF/TXq7kjo9ag4M+C8yzzIRYbE=", + "lastModified": 1785090369, + "narHash": "sha256-m0pDuRJG7EDo9ri+4Ksu83VsI+PlxNC9lNBfydejce4=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "1c3fe55ad329cbcb28471bb30f05c9827f724c76", + "rev": "624af665418d3c65d544145b4d34ad696439570e", "type": "github" }, "original": { @@ -48,11 +48,11 @@ ] }, "locked": { - "lastModified": 1777432579, - "narHash": "sha256-Ce11TStDsqCge2vAAfLKe2+4lDI5cSX5ZYZOuKJBKKQ=", + "lastModified": 1785216007, + "narHash": "sha256-KrGilCiFVxEQOaBD3TODhklZzE7uICQvqsBWgycIABA=", "owner": "oxalica", "repo": "rust-overlay", - "rev": "3ecb5e6ab380ced3272ef7fcfe398bffbcc0f152", + "rev": "8ec8a5a41f8d8244e672829c9cd705416139d3f0", "type": "github" }, "original": { From 6b664c6654bc92264781fb6da6dadfa8aa9f4643 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Tue, 28 Jul 2026 20:47:34 +0200 Subject: [PATCH 2/9] add regression test --- src/tests/mock_wgapi.rs | 85 ++++++++++++++++++++++++++++++++++++++++- src/tests/mod.rs | 1 + src/tests/recovery.rs | 51 +++++++++++++++++++++++++ 3 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 src/tests/recovery.rs diff --git a/src/tests/mock_wgapi.rs b/src/tests/mock_wgapi.rs index 261daac5..39acdf73 100644 --- a/src/tests/mock_wgapi.rs +++ b/src/tests/mock_wgapi.rs @@ -1,4 +1,7 @@ -use std::net::IpAddr; +use std::{ + net::IpAddr, + sync::atomic::{AtomicBool, Ordering}, +}; use defguard_wireguard_rs::{ InterfaceConfiguration, WireguardInterfaceApi, error::WireguardInterfaceError, host::Host, @@ -64,3 +67,83 @@ impl WireguardInterfaceApi for NullWgApi { Err(WireguardInterfaceError::Interface("test".into())) } } + +/// A mock WireGuard API that tracks whether the interface exists. +/// +/// Used to reproduce the bug where `purge()` removes the interface, then +/// `configure()` tries to configure a non-existent device and gets `NoDevice`. +pub(crate) struct StatefulMockWgApi { + interface_exists: AtomicBool, +} + +impl StatefulMockWgApi { + pub(crate) fn new() -> Self { + Self { + interface_exists: AtomicBool::new(true), + } + } +} + +impl WireguardInterfaceApi for StatefulMockWgApi { + fn create_interface(&mut self) -> Result<(), WireguardInterfaceError> { + self.interface_exists.store(true, Ordering::Relaxed); + Ok(()) + } + + fn assign_address(&self, _address: &IpAddrMask) -> Result<(), WireguardInterfaceError> { + Ok(()) + } + + fn configure_peer_routing(&self, _peers: &[WgPeer]) -> Result<(), WireguardInterfaceError> { + Ok(()) + } + + fn configure_interface( + &self, + _config: &InterfaceConfiguration, + ) -> Result<(), WireguardInterfaceError> { + if self.interface_exists.load(Ordering::Relaxed) { + Ok(()) + } else { + Err(WireguardInterfaceError::Interface("no such device".into())) + } + } + + #[cfg(not(target_os = "windows"))] + fn remove_interface(&self) -> Result<(), WireguardInterfaceError> { + self.interface_exists.store(false, Ordering::Relaxed); + Ok(()) + } + + #[cfg(target_os = "windows")] + fn remove_interface(&mut self) -> Result<(), WireguardInterfaceError> { + self.interface_exists.store(false, Ordering::Relaxed); + Ok(()) + } + + fn configure_peer(&self, _peer: &WgPeer) -> Result<(), WireguardInterfaceError> { + Ok(()) + } + + fn remove_peer(&self, _peer_pubkey: &Key) -> Result<(), WireguardInterfaceError> { + Ok(()) + } + + fn read_interface_data(&self) -> Result { + if self.interface_exists.load(Ordering::Relaxed) { + Ok(Host::default()) + } else { + Err(WireguardInterfaceError::ReadInterfaceError( + "no such device".into(), + )) + } + } + + fn configure_dns( + &self, + _dns: &[IpAddr], + _search_domains: &[&str], + ) -> Result<(), WireguardInterfaceError> { + Ok(()) + } +} diff --git a/src/tests/mod.rs b/src/tests/mod.rs index d5fb0405..113ec4f5 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -1,2 +1,3 @@ mod mock_wgapi; mod mtls; +mod recovery; diff --git a/src/tests/recovery.rs b/src/tests/recovery.rs new file mode 100644 index 00000000..130f0865 --- /dev/null +++ b/src/tests/recovery.rs @@ -0,0 +1,51 @@ +use super::mock_wgapi::StatefulMockWgApi; +use crate::{config::Config, gateway::Gateway, proto::gateway::Configuration}; + +/// Reproduction test for the interface recovery bug. +/// +/// After a disconnect, `Gateway::purge()` calls `remove_interface()` which +/// deletes the kernel WireGuard link. On the next `configure()`, the gateway +/// tries to configure a non-existent device and fails with `NoDevice`. +/// +/// This test proves the bug by: +/// 1. Building a Gateway with the stateful mock (interface starts as existing) +/// 2. Calling `purge()` which drives `remove_interface()` -> interface gone +/// 3. Calling `configure()` with a minimal config -> fails on current code +#[test] +fn purge_then_configure_recovers_interface() { + let config = Config::default(); + let mut gateway = Gateway::new(config, StatefulMockWgApi::new()).unwrap(); + + // Verify the interface exists before purge. + assert!( + gateway.wgapi.lock().unwrap().read_interface_data().is_ok(), + "interface should exist initially" + ); + + // Simulate what happens on disconnect: purge removes the interface. + gateway.purge(); + + // After purge, the interface should be gone. + assert!( + gateway.wgapi.lock().unwrap().read_interface_data().is_err(), + "interface should not exist after purge" + ); + + // Simulate reconnect: Core pushes a new configuration. + let new_config = Configuration { + name: "wg0".into(), + private_key: "aBcDeFgHiJkLmNoPqRsTuVwXyZ0123456789abC=".into(), + port: 51820, + peers: vec![], + addresses: vec!["10.0.0.1/24".into()], + firewall_config: None, + mtu: 1420, + fwmark: 0, + }; + + let result = gateway.configure(new_config); + assert!( + result.is_ok(), + "configure should succeed (but fails on current code): {result:?}", + ); +} From f03bc2abafc86716863d3832721585e2e889e5ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Tue, 28 Jul 2026 20:58:58 +0200 Subject: [PATCH 3/9] recreate interface on configure if necessary --- src/gateway.rs | 15 +++++++++++++++ src/gateway_server.rs | 26 -------------------------- src/tests/recovery.rs | 3 ++- 3 files changed, 17 insertions(+), 27 deletions(-) diff --git a/src/gateway.rs b/src/gateway.rs index cd33e919..a66a33e2 100644 --- a/src/gateway.rs +++ b/src/gateway.rs @@ -402,6 +402,21 @@ impl Gateway { mask!(new_configuration, private_key) ); + // configure() is the sole owner of interface existence; recreate it if + // it was removed (e.g. by purge during a disconnect). + if self + .wgapi + .lock() + .expect("Failed to lock Gateway::wgapi") + .read_interface_data() + .is_err() + { + self.wgapi + .lock() + .expect("Failed to lock Gateway::wgapi") + .create_interface()?; + } + // check if new configuration is different than current one let new_interface_configuration = new_configuration.clone().into(); diff --git a/src/gateway_server.rs b/src/gateway_server.rs index c6c4e94a..e331c81c 100644 --- a/src/gateway_server.rs +++ b/src/gateway_server.rs @@ -20,8 +20,6 @@ use tonic::{Request, Response, Status, Streaming, service::InterceptorLayer, tra use tower::ServiceBuilder; use tracing::instrument; -#[cfg(target_os = "linux")] -use crate::enterprise::firewall::api::{FirewallApi, FirewallManagementApi}; use crate::{ CORE_CLIENT_CERT_NAME, GRPC_CA_CERT_NAME, GRPC_CERT_NAME, GRPC_KEY_NAME, VERSION, config::Config, @@ -63,30 +61,6 @@ impl GatewayServer { pub(crate) async fn start(self, config: Config) -> Result<(), GatewayError> { info!("Starting Defguard Gateway version {VERSION} with configuration: {config:?}"); - // Try to create network interface for WireGuard. - // FIXME: check if the interface already exists, or somehow be more clever. - { - #[allow(unused)] - let mut gateway = &mut self.gateway.lock().expect("gateway mutex poison"); - if let Err(err) = gateway - .wgapi - .lock() - .expect("wgapi mutex poison") - .create_interface() - { - warn!( - "Couldn't create network interface {}: {err}. Proceeding anyway.", - config.ifname - ); - } else { - #[cfg(target_os = "linux")] - if !config.disable_firewall_management && config.masquerade { - let mut firewall_api = FirewallApi::new(&config.ifname)?; - firewall_api.setup_nat(config.masquerade, &[])?; - } - } - } - if let Some(post_up) = &config.post_up { debug!("Executing specified POST_UP command: {post_up}"); execute_command(post_up)?; diff --git a/src/tests/recovery.rs b/src/tests/recovery.rs index 130f0865..3a2e3f88 100644 --- a/src/tests/recovery.rs +++ b/src/tests/recovery.rs @@ -13,7 +13,8 @@ use crate::{config::Config, gateway::Gateway, proto::gateway::Configuration}; /// 3. Calling `configure()` with a minimal config -> fails on current code #[test] fn purge_then_configure_recovers_interface() { - let config = Config::default(); + let mut config = Config::default(); + config.disable_firewall_management = true; let mut gateway = Gateway::new(config, StatefulMockWgApi::new()).unwrap(); // Verify the interface exists before purge. From baa9b1fed616a6544d533ca171fad313926e18c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Tue, 28 Jul 2026 21:08:36 +0200 Subject: [PATCH 4/9] remove peers instead of full purge on disconnect --- src/gateway.rs | 29 +++++++++++++++++++++++++++++ src/gateway_server.rs | 4 +--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/gateway.rs b/src/gateway.rs index a66a33e2..663e4d5d 100644 --- a/src/gateway.rs +++ b/src/gateway.rs @@ -207,6 +207,35 @@ impl Gateway { self.connected.store(false, Ordering::Relaxed); } + /// Tear down peer state on Core disconnect without removing the WireGuard + /// interface. + /// + /// Removes all peers from the kernel interface (fail-closed security) and + /// resets gateway state, but leaves the interface alive so it can be + /// reconfigured when Core reconnects. Unlike [`purge`](Self::purge), this + /// does not call `remove_interface()`. + pub(crate) fn disconnect_cleanup(&mut self) { + // Remove all peers from the kernel interface. + { + let wgapi = self.wgapi.lock().expect("Failed to lock Gateway::wgapi"); + for pubkey in self.peers.keys() { + if let Err(err) = wgapi.remove_peer(&pubkey.as_str().try_into().unwrap_or_default()) + { + error!("Failed to remove peer {pubkey} during disconnect cleanup: {err}"); + } + } + } + // Cleanup the firewall. + if let Err(err) = self.cleanup_firewall() { + error!("Gateway disconnect cleanup failed to cleanup firewall rules: {err}"); + } + // Reset connection state but keep the interface alive. + self.interface_configuration = None; + self.peers.clear(); + self.client_tx = None; + self.connected.store(false, Ordering::Relaxed); + } + // Replace current peer map with a new list of peers. fn replace_peers(&mut self, new_peers: Vec) { debug!("Replacing stored peers with {} new peers", new_peers.len()); diff --git a/src/gateway_server.rs b/src/gateway_server.rs index e331c81c..79d1fb48 100644 --- a/src/gateway_server.rs +++ b/src/gateway_server.rs @@ -226,9 +226,7 @@ impl gateway_server::Gateway for GatewayServer { } info!("Defguard Core gRPC stream has been disconnected: {address}"); if let Ok(mut gateway) = gateway.lock() { - gateway.connected.store(false, Ordering::Relaxed); - gateway.client_tx = None; - gateway.purge(); + gateway.disconnect_cleanup(); } }); From 126fe59a05e66c4b87ed7957fa69ed2761c2dd10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Tue, 28 Jul 2026 21:21:05 +0200 Subject: [PATCH 5/9] fix test name to follow convention --- src/tests/mtls.rs | 10 +++++----- src/tests/recovery.rs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tests/mtls.rs b/src/tests/mtls.rs index a85104fa..cde71673 100644 --- a/src/tests/mtls.rs +++ b/src/tests/mtls.rs @@ -189,7 +189,7 @@ async fn call_bidi(client: &mut GatewayClient) -> tonic::Status { /// `start()` must return `Err` immediately when no `TlsConfig` has been set. #[tokio::test] -async fn start_errors_without_tls_config() { +async fn test_start_errors_without_tls_config() { let config = Config::default(); let gateway = build_gateway(&config); let gateway = Arc::new(Mutex::new(gateway)); @@ -213,7 +213,7 @@ async fn start_errors_without_tls_config() { /// but it must NOT be rejected with `Unauthenticated` - that would indicate the mTLS layer /// or serial-pin interceptor wrongly rejected the cert. #[tokio::test] -async fn valid_mtls_client_accepted() { +async fn test_valid_mtls_client_accepted() { init_crypto(); let certs = TestCerts::generate(); let (port, shutdown_tx) = spawn_test_gateway(&certs).await; @@ -242,7 +242,7 @@ async fn valid_mtls_client_accepted() { /// tonic may reject the connection eagerly (at `connect()` time) or lazily /// (on the first RPC). Both outcomes are treated as the expected rejection. #[tokio::test] -async fn no_client_cert_rejected() { +async fn test_no_client_cert_rejected() { init_crypto(); let certs = TestCerts::generate(); let (port, shutdown_tx) = spawn_test_gateway(&certs).await; @@ -271,7 +271,7 @@ async fn no_client_cert_rejected() { /// A client presenting a cert from the correct CA but with the wrong serial must be rejected /// by the serial-pin interceptor with `Unauthenticated`. #[tokio::test] -async fn wrong_serial_rejected() { +async fn test_wrong_serial_rejected() { init_crypto(); let certs = TestCerts::generate(); let (port, shutdown_tx) = spawn_test_gateway(&certs).await; @@ -304,7 +304,7 @@ async fn wrong_serial_rejected() { /// but the rejection must not be `FailedPrecondition`, which would indicate /// the cert bypassed CA verification and reached the gRPC handler. #[tokio::test] -async fn rogue_ca_client_rejected() { +async fn test_rogue_ca_client_rejected() { init_crypto(); let certs = TestCerts::generate(); let (port, shutdown_tx) = spawn_test_gateway(&certs).await; diff --git a/src/tests/recovery.rs b/src/tests/recovery.rs index 3a2e3f88..49707167 100644 --- a/src/tests/recovery.rs +++ b/src/tests/recovery.rs @@ -12,7 +12,7 @@ use crate::{config::Config, gateway::Gateway, proto::gateway::Configuration}; /// 2. Calling `purge()` which drives `remove_interface()` -> interface gone /// 3. Calling `configure()` with a minimal config -> fails on current code #[test] -fn purge_then_configure_recovers_interface() { +fn test_purge_then_configure_recovers_interface() { let mut config = Config::default(); config.disable_firewall_management = true; let mut gateway = Gateway::new(config, StatefulMockWgApi::new()).unwrap(); From f06ffbb2ec1bec04a81979d9474d285fb51c23f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Wed, 29 Jul 2026 07:34:19 +0200 Subject: [PATCH 6/9] cleanup --- src/gateway.rs | 49 ++++++++++++++++++++++++++--------------- src/tests/mock_wgapi.rs | 1 + src/tests/recovery.rs | 17 +++++++------- 3 files changed, 41 insertions(+), 26 deletions(-) diff --git a/src/gateway.rs b/src/gateway.rs index 663e4d5d..3690166c 100644 --- a/src/gateway.rs +++ b/src/gateway.rs @@ -219,8 +219,17 @@ impl Gateway { { let wgapi = self.wgapi.lock().expect("Failed to lock Gateway::wgapi"); for pubkey in self.peers.keys() { - if let Err(err) = wgapi.remove_peer(&pubkey.as_str().try_into().unwrap_or_default()) - { + let key = match pubkey.as_str().try_into() { + Ok(key) => key, + Err(err) => { + error!( + "Failed to parse peer public key {pubkey} during disconnect \ + cleanup; peer NOT removed: {err}" + ); + continue; + } + }; + if let Err(err) = wgapi.remove_peer(&key) { error!("Failed to remove peer {pubkey} during disconnect cleanup: {err}"); } } @@ -433,17 +442,15 @@ impl Gateway { // configure() is the sole owner of interface existence; recreate it if // it was removed (e.g. by purge during a disconnect). - if self - .wgapi - .lock() - .expect("Failed to lock Gateway::wgapi") - .read_interface_data() - .is_err() { - self.wgapi - .lock() - .expect("Failed to lock Gateway::wgapi") - .create_interface()?; + let mut wgapi = self.wgapi.lock().expect("Failed to lock Gateway::wgapi"); + if wgapi.read_interface_data().is_err() { + info!( + "WireGuard interface {} is not present, creating it", + new_configuration.name + ); + wgapi.create_interface()?; + } } // check if new configuration is different than current one @@ -517,12 +524,18 @@ impl Gateway { if update.update_type == 2 { debug!("Deleting peer {peer_config:?}"); self.peers.remove(&peer_config.pubkey); - if let Err(err) = - self.wgapi.lock().unwrap().remove_peer( - &peer_config.pubkey.as_str().try_into().unwrap_or_default(), - ) - { - error!("Failed to delete peer: {err}"); + match peer_config.pubkey.as_str().try_into() { + Ok(key) => { + if let Err(err) = self.wgapi.lock().unwrap().remove_peer(&key) { + error!("Failed to delete peer: {err}"); + } + } + Err(err) => { + error!( + "Failed to parse public key of peer {} to delete: {err}", + peer_config.pubkey + ); + } } } // UpdateType::Create, UpdateType::Modify diff --git a/src/tests/mock_wgapi.rs b/src/tests/mock_wgapi.rs index 39acdf73..e00d79d5 100644 --- a/src/tests/mock_wgapi.rs +++ b/src/tests/mock_wgapi.rs @@ -77,6 +77,7 @@ pub(crate) struct StatefulMockWgApi { } impl StatefulMockWgApi { + #[must_use] pub(crate) fn new() -> Self { Self { interface_exists: AtomicBool::new(true), diff --git a/src/tests/recovery.rs b/src/tests/recovery.rs index 49707167..9432940c 100644 --- a/src/tests/recovery.rs +++ b/src/tests/recovery.rs @@ -1,16 +1,17 @@ use super::mock_wgapi::StatefulMockWgApi; use crate::{config::Config, gateway::Gateway, proto::gateway::Configuration}; -/// Reproduction test for the interface recovery bug. +/// Regression test for the interface recovery bug. /// /// After a disconnect, `Gateway::purge()` calls `remove_interface()` which -/// deletes the kernel WireGuard link. On the next `configure()`, the gateway -/// tries to configure a non-existent device and fails with `NoDevice`. +/// deletes the kernel WireGuard link. Before the fix, the next `configure()` +/// tried to configure a non-existent device and failed; `configure()` now +/// recreates the interface when it is missing, so recovery succeeds. /// -/// This test proves the bug by: -/// 1. Building a Gateway with the stateful mock (interface starts as existing) -/// 2. Calling `purge()` which drives `remove_interface()` -> interface gone -/// 3. Calling `configure()` with a minimal config -> fails on current code +/// The test: +/// 1. Builds a Gateway with the stateful mock (interface starts as existing) +/// 2. Calls `purge()`, which drives `remove_interface()` -> interface gone +/// 3. Calls `configure()` and asserts it recreates the interface and succeeds #[test] fn test_purge_then_configure_recovers_interface() { let mut config = Config::default(); @@ -47,6 +48,6 @@ fn test_purge_then_configure_recovers_interface() { let result = gateway.configure(new_config); assert!( result.is_ok(), - "configure should succeed (but fails on current code): {result:?}", + "configure should recover the interface and succeed: {result:?}", ); } From b2794c329c80deeee17c046d5681f3024808b9f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Wed, 29 Jul 2026 07:40:50 +0200 Subject: [PATCH 7/9] fix wording --- src/gateway.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gateway.rs b/src/gateway.rs index 3690166c..d34b55cc 100644 --- a/src/gateway.rs +++ b/src/gateway.rs @@ -210,12 +210,12 @@ impl Gateway { /// Tear down peer state on Core disconnect without removing the WireGuard /// interface. /// - /// Removes all peers from the kernel interface (fail-closed security) and + /// Removes all peers from the interface (fail-closed security) and /// resets gateway state, but leaves the interface alive so it can be /// reconfigured when Core reconnects. Unlike [`purge`](Self::purge), this /// does not call `remove_interface()`. pub(crate) fn disconnect_cleanup(&mut self) { - // Remove all peers from the kernel interface. + // Remove all peers from the interface. { let wgapi = self.wgapi.lock().expect("Failed to lock Gateway::wgapi"); for pubkey in self.peers.keys() { From 821ff902acfa8be0205bb29b9df435bb4cac58a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Wed, 29 Jul 2026 07:43:02 +0200 Subject: [PATCH 8/9] add issue ID --- src/tests/recovery.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/recovery.rs b/src/tests/recovery.rs index 9432940c..dfaa1068 100644 --- a/src/tests/recovery.rs +++ b/src/tests/recovery.rs @@ -1,7 +1,7 @@ use super::mock_wgapi::StatefulMockWgApi; use crate::{config::Config, gateway::Gateway, proto::gateway::Configuration}; -/// Regression test for the interface recovery bug. +/// Regression test for the interface recovery bug (#354). /// /// After a disconnect, `Gateway::purge()` calls `remove_interface()` which /// deletes the kernel WireGuard link. Before the fix, the next `configure()` From 0586fd790abda218d3a6f222e31bea7e86c2c2dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Wed, 29 Jul 2026 09:39:04 +0200 Subject: [PATCH 9/9] change cleanup approach to delay + purge --- src/config.rs | 19 +++++++++++++++++++ src/gateway.rs | 40 ++++------------------------------------ src/gateway_server.rs | 30 ++++++++++++++++++++++++++++-- 3 files changed, 51 insertions(+), 38 deletions(-) diff --git a/src/config.rs b/src/config.rs index bbc7941d..529baa7c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -23,6 +23,10 @@ fn default_stats_period() -> u64 { 30 } +fn default_core_disconnect_grace_period() -> u64 { + 30 +} + fn default_ifname() -> String { String::from("wg0") } @@ -66,6 +70,20 @@ pub struct Config { #[serde(default = "default_stats_period")] pub stats_period: u64, + /// Seconds to wait after losing the Defguard Core connection before tearing down + /// the WireGuard interface. Bridges transient Core outages (e.g. a container + /// restart) without dropping VPN traffic; if Core reconnects within the window, + /// the teardown is skipped. Set to 0 to tear down immediately. NOTE: during the + /// window peers keep forwarding while Core is unreachable, so revoked or expired + /// sessions retain access until it elapses. + #[arg( + long, + env = "DEFGUARD_CORE_DISCONNECT_GRACE_PERIOD", + default_value = "30" + )] + #[serde(default = "default_core_disconnect_grace_period")] + pub core_disconnect_grace_period: u64, + /// Network interface name (e.g. wg0) #[arg(long, short = 'i', env = "DEFGUARD_IFNAME", default_value = "wg0")] #[serde(default = "default_ifname")] @@ -184,6 +202,7 @@ impl Default for Config { grpc_port: 50066, userspace: false, stats_period: 15, + core_disconnect_grace_period: 30, ifname: "wg0".into(), pidfile: None, use_syslog: false, diff --git a/src/gateway.rs b/src/gateway.rs index d34b55cc..d45e0ecf 100644 --- a/src/gateway.rs +++ b/src/gateway.rs @@ -207,42 +207,10 @@ impl Gateway { self.connected.store(false, Ordering::Relaxed); } - /// Tear down peer state on Core disconnect without removing the WireGuard - /// interface. - /// - /// Removes all peers from the interface (fail-closed security) and - /// resets gateway state, but leaves the interface alive so it can be - /// reconfigured when Core reconnects. Unlike [`purge`](Self::purge), this - /// does not call `remove_interface()`. - pub(crate) fn disconnect_cleanup(&mut self) { - // Remove all peers from the interface. - { - let wgapi = self.wgapi.lock().expect("Failed to lock Gateway::wgapi"); - for pubkey in self.peers.keys() { - let key = match pubkey.as_str().try_into() { - Ok(key) => key, - Err(err) => { - error!( - "Failed to parse peer public key {pubkey} during disconnect \ - cleanup; peer NOT removed: {err}" - ); - continue; - } - }; - if let Err(err) = wgapi.remove_peer(&key) { - error!("Failed to remove peer {pubkey} during disconnect cleanup: {err}"); - } - } - } - // Cleanup the firewall. - if let Err(err) = self.cleanup_firewall() { - error!("Gateway disconnect cleanup failed to cleanup firewall rules: {err}"); - } - // Reset connection state but keep the interface alive. - self.interface_configuration = None; - self.peers.clear(); - self.client_tx = None; - self.connected.store(false, Ordering::Relaxed); + /// Grace period to wait after a Core disconnect before purging the interface. + /// Zero means purge immediately (no grace). + pub(crate) fn disconnect_grace_period(&self) -> Duration { + Duration::from_secs(self.config.core_disconnect_grace_period) } // Replace current peer map with a new list of peers. diff --git a/src/gateway_server.rs b/src/gateway_server.rs index 79d1fb48..e5795afa 100644 --- a/src/gateway_server.rs +++ b/src/gateway_server.rs @@ -13,7 +13,9 @@ use defguard_version::{ }; use tokio::{ fs::remove_file, + spawn, sync::{mpsc, oneshot}, + time::sleep, }; use tokio_stream::wrappers::UnboundedReceiverStream; use tonic::{Request, Response, Status, Streaming, service::InterceptorLayer, transport::Server}; @@ -225,8 +227,32 @@ impl gateway_server::Gateway for GatewayServer { } } info!("Defguard Core gRPC stream has been disconnected: {address}"); - if let Ok(mut gateway) = gateway.lock() { - gateway.disconnect_cleanup(); + // Mark disconnected immediately, but delay tearing down the interface by + // the grace period so a transient Core outage (e.g. a container restart) + // doesn't drop VPN traffic. If Core reconnects within the window the purge + // is skipped; otherwise we fail closed. + let grace_period = { + let mut gateway = gateway.lock().expect("Gateway lock poison"); + gateway.connected.store(false, Ordering::Relaxed); + gateway.client_tx = None; + gateway.disconnect_grace_period() + }; + if grace_period.is_zero() { + gateway.lock().expect("Gateway lock poison").purge(); + } else { + let gateway = Arc::clone(&gateway); + spawn(async move { + sleep(grace_period).await; + let mut gateway = gateway.lock().expect("Gateway lock poison"); + if gateway.client_tx.is_none() { + info!( + "Core still disconnected after {grace_period:?}; purging (fail-closed)" + ); + gateway.purge(); + } else { + debug!("Core reconnected within grace period; skipping purge"); + } + }); } });