Service location posture checks

src-tauri/client-cli/src/commands/connect.rs

@@ -159,7 +159,7 @@ pub async fn handle(
                 let psk = authorize_posture_session(location)
                     .await
                     .map_err(|e| CliError::Other(e.to_string()))?;
-                (location.name.clone(), Some(psk), state.app_config.mtu())
+                (location.name.clone(), psk, state.app_config.mtu())
             } else {
                 (location.name.clone(), None, state.app_config.mtu())
             }

src-tauri/client-cli/src/resolve.rs

@@ -19,11 +19,27 @@ pub enum ResolvedTarget {
     Tunnel(Tunnel<Id>),
 }
 
-/// Resolve a target for the `connect` command.
+/// Resolve a target for the `connect` command. Rejects service locations.
 pub async fn resolve_connect_target(
     spec: &TargetSpec,
     pool: &DbPool,
 ) -> Result<ResolvedTarget, CliError> {
+    let target = resolve_user_target(spec, pool).await?;
+
+    if let ResolvedTarget::Location(location) = &target {
+        if location.is_service_location() {
+            return Err(CliError::InvalidInput(format!(
+                "'{}' is a service location and is managed by the defguard service",
+                location.name
+            )));
+        }
+    }
+
+    Ok(target)
+}
+
+/// Resolution proper, without the service-location check its callers rely on.
+async fn resolve_user_target(spec: &TargetSpec, pool: &DbPool) -> Result<ResolvedTarget, CliError> {
     // --id fast path
     if let Some(id) = spec.id {
         if spec.tunnel {
@@ -250,6 +266,71 @@ mod tests {
         }
     }
 
+    /// Service locations belong to the daemon: connecting to one would make the app and the daemon
+    /// supersede each other's session indefinitely. `--name` is safe because
+    /// `Location::find_by_name` filters them out in SQL; `--id` is not safe.
+    #[sqlx::test(migrations = "../migrations")]
+    async fn test_service_location_is_not_resolvable(pool: DbPool) {
+        let i = sample_instance("acme").save(&pool).await.unwrap();
+        let mut location = sample_location("headless", i.id);
+        location.service_location_mode = ServiceLocationMode::AlwaysOn;
+        let location = location.save(&pool).await.unwrap();
+
+        let by_name = TargetSpec {
+            name: Some("headless".into()),
+            tunnel: false,
+            id: None,
+            instance: None,
+        };
+        // Already filtered in SQL, so this reports "not found" rather than reaching the guard.
+        let err = expect_err(resolve_connect_target(&by_name, &pool).await);
+        assert!(
+            matches!(err, CliError::NotFound(_)),
+            "expected NotFound for a service location by name, got {err:?}"
+        );
+
+        let by_id = TargetSpec {
+            name: None,
+            tunnel: false,
+            id: Some(location.id),
+            instance: None,
+        };
+        let err = expect_err(resolve_connect_target(&by_id, &pool).await);
+        assert!(
+            matches!(err, CliError::InvalidInput(_)),
+            "expected InvalidInput for a service location by id, got {err:?}"
+        );
+
+        // Disconnect resolves through the same function, so it is covered too.
+        let err = expect_err(resolve_disconnect_target(&by_id, &pool).await);
+        assert!(
+            matches!(err, CliError::InvalidInput(_)),
+            "expected InvalidInput when disconnecting a service location, got {err:?}"
+        );
+    }
+
+    /// A regular location on the same instance must still resolve, so the check above is not simply
+    /// rejecting everything.
+    #[sqlx::test(migrations = "../migrations")]
+    async fn test_regular_location_resolves_alongside_a_service_location(pool: DbPool) {
+        let i = sample_instance("acme").save(&pool).await.unwrap();
+        let mut service = sample_location("headless", i.id);
+        service.service_location_mode = ServiceLocationMode::AlwaysOn;
+        service.save(&pool).await.unwrap();
+        sample_location("office", i.id).save(&pool).await.unwrap();
+
+        let spec = TargetSpec {
+            name: Some("office".into()),
+            tunnel: false,
+            id: None,
+            instance: None,
+        };
+        match expect_ok(resolve_connect_target(&spec, &pool).await) {
+            ResolvedTarget::Location(l) => assert_eq!(l.name, "office"),
+            _ => panic!("expected Location"),
+        }
+    }
+
     #[sqlx::test(migrations = "../migrations")]
     async fn test_unique_tunnel_by_name(pool: DbPool) {
         sample_tunnel("gateway").save(&pool).await.unwrap();

src-tauri/client-proto/build.rs

@@ -2,6 +2,8 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
     println!("cargo:rerun-if-changed=../proto");
 
     tonic_prost_build::configure()
+        // These types contain sensitive data.
+        .skip_debug(["SaveServiceLocationsRequest"])
         // Enable optional fields.
         .protoc_arg("--experimental_allow_proto3_optional")
         // Make sure empty DNS is deserialized correctly as `None`.
@@ -12,6 +14,18 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         )
         // Make all messages serde-serializable.
         .type_attribute(".", "#[derive(serde::Serialize,serde::Deserialize)]")
+        // `ServiceLocation` is persisted as JSON by the daemon. Tolerate these fields being absent
+        // in files written by older clients. Deliberately per-field rather than a container-level
+        // `#[serde(default)]`, so a truncated or corrupt file still fails to deserialize instead of
+        // quietly becoming "no locations".
+        .field_attribute(
+            ".defguard.client.v1.ServiceLocation.network_id",
+            "#[serde(default)]",
+        )
+        .field_attribute(
+            ".defguard.client.v1.ServiceLocation.posture_check_required",
+            "#[serde(default)]",
+        )
         // Use proto defaults for missing fields in enrollment types that
         // may differ across proxy versions.
         .type_attribute(".defguard.client_types.AdminInfo", "#[serde(default)]")

src-tauri/daemon/src/daemon.rs

@@ -11,7 +11,7 @@ use std::{fs, path::Path};
 
 use defguard_client_common::dns_borrow;
 #[cfg(windows)]
-use defguard_client_posture::inspector::device_posture_data;
+use defguard_client_posture::inspector::{device_posture_data, DiskEncryptionTarget};
 use defguard_client_proto::defguard::{
     client::v1::{
         desktop_daemon_service_server::{DesktopDaemonService, DesktopDaemonServiceServer},
@@ -23,7 +23,7 @@ use defguard_client_proto::defguard::{
 };
 use defguard_client_service_locations::ServiceLocationError;
 #[cfg(any(windows, target_os = "linux"))]
-use defguard_client_service_locations::ServiceLocationManager;
+use defguard_client_service_locations::{ReconcileSignal, ServiceLocationManager};
 #[cfg(not(target_os = "macos"))]
 use defguard_wireguard_rs::Kernel;
 #[cfg(target_os = "macos")]
@@ -57,10 +57,12 @@ pub(super) const DAEMON_SOCKET_PATH: &str = "/var/run/defguard.socket";
 #[cfg(target_os = "linux")]
 pub(super) const DAEMON_SOCKET_GROUP: &str = "defguard";
 
+/// How often the reconciler brings running tunnels back in line with what is on disk.
+///
+/// On Windows this is a backstop, since the watchers wake it on network, logon and resume events. On
+/// Linux nothing wakes it, so this is the only trigger and sets the worst-case recovery time.
 #[cfg(any(windows, target_os = "linux"))]
-pub(crate) const SERVICE_LOCATION_CONNECT_RETRY_COUNT: u32 = 5;
-#[cfg(any(windows, target_os = "linux"))]
-pub(crate) const SERVICE_LOCATION_CONNECT_RETRY_DELAY: Duration = Duration::from_secs(30);
+pub(crate) const SERVICE_LOCATION_RECONCILE_INTERVAL: Duration = Duration::from_secs(30);
 
 #[derive(Debug, thiserror::Error)]
 pub enum DaemonError {
@@ -217,11 +219,7 @@ impl DesktopDaemonService for DaemonService {
         self.service_location_manager
             .write()
             .unwrap()
-            .save_service_locations(
-                service_location.service_locations.as_slice(),
-                &service_location.instance_id,
-                &service_location.private_key,
-            )
+            .save_service_locations(&service_location)
             .map_err(|err| {
                 let msg = format!("Failed to save service locations: {err}");
                 error!(msg);
@@ -238,10 +236,11 @@ impl DesktopDaemonService for DaemonService {
         _request: tonic::Request<()>,
     ) -> Result<Response<DevicePostureData>, Status> {
         warn!(
-            "Daemon service received a get_posture_data request. Daemon posture requests are only supported on windows systems. Unix systems perform client-side posture checks."
+            "Received a get_posture_data request. Only Windows needs the service to collect posture \
+            data; elsewhere the app evaluates it itself."
         );
         Err(Status::unimplemented(
-            "Service-side posture checks are only supported on Unix systems",
+            "Service-side posture checks are not supported on this platform",
         ))
     }
 
@@ -544,13 +543,20 @@ impl DesktopDaemonService for DaemonService {
         Ok(Response::new(ListInterfacesResponse { interfaces }))
     }
 
+    /// Collects this device's posture data on the app's behalf.
+    ///
+    /// Windows-only, because only SYSTEM can query the WMI encryption namespace. This answers a
+    /// *user-initiated* check, hence `ClientDatabase` - inert on Windows, where the probe reports on
+    /// the system volume regardless, but it states which question is being answered.
     #[cfg(windows)]
     async fn get_posture_data(
         &self,
         _request: tonic::Request<()>,
     ) -> Result<Response<DevicePostureData>, Status> {
         debug!("Get posture data request received");
-        Ok(Response::new(device_posture_data()))
+        Ok(Response::new(device_posture_data(
+            DiskEncryptionTarget::ClientDatabase,
+        )))
     }
 }
 
@@ -560,14 +566,14 @@ pub async fn run_server(config: Config) -> anyhow::Result<()> {
 
     #[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.
     #[cfg(target_os = "linux")]
-    tokio::spawn(
-        defguard_client_service_locations::connect_service_locations(
-            service_location_manager.clone(),
-            SERVICE_LOCATION_CONNECT_RETRY_COUNT,
-            SERVICE_LOCATION_CONNECT_RETRY_DELAY,
-        ),
-    );
+    tokio::spawn(defguard_client_service_locations::run_reconciler(
+        service_location_manager.clone(),
+        ReconcileSignal::default(),
+        SERVICE_LOCATION_RECONCILE_INTERVAL,
+    ));
 
     let daemon_service = DaemonService::new(
         &config,