refactor(gui-client): finish refactors from #4978 (#5158)

```[tasklist]
### Before opening for review
- [ ] ~~Wait for some other refactors to merge~~
- [x] Test Windows
- [x] Test Linux
```

rust/gui-client/src-tauri/src/client.rs

@@ -87,7 +87,7 @@ pub(crate) fn run() -> Result<()> {
         }
         Some(Cmd::SmokeTest) => {
             if !elevation::is_normal_user()? {
-                anyhow::bail!("`smoke-test` failed its elevation check");
+                anyhow::bail!("`smoke-test` must run as a normal user");
             }
 
             let result = gui::run(cli);

rust/gui-client/src-tauri/src/client/elevation.rs

@@ -7,13 +7,13 @@ mod imp {
 
     /// Returns true if we're running without root privileges
     ///
-    /// On Linux we've already switched to IPC, so the process must NOT be elevated
-    #[allow(clippy::print_stderr)]
+    /// Everything that needs root / admin powers happens in the IPC services,
+    /// so for security and practicality reasons the GUIs must be non-root.
+    /// (In Linux by default a root GUI app barely works at all)
     pub(crate) fn is_normal_user() -> anyhow::Result<bool, Error> {
         // Must use `eprintln` here because `tracing` won't be initialized yet.
         let user = std::env::var("USER").context("USER env var should be set")?;
         if user == "root" {
-            eprintln!("Firezone must run as a normal user, not with `sudo` or as root");
             return Ok(false);
         }
 
@@ -42,7 +42,7 @@ mod imp {
 
     // Returns true on Windows
     ///
-    /// On Windows we are switching to IPC, and checking for elevation is complicated,
+    /// On Windows, checking for elevation is complicated,
     /// so it just always returns true. The Windows GUI does work correctly even if
     /// elevated, so we should warn users that it doesn't need elevation, but it's
     /// not a show-stopper if they accidentally "Run as admin".

rust/headless-client/src/lib.rs

@@ -252,6 +252,8 @@ pub fn run_only_headless_client() -> Result<()> {
         return Ok(());
     }
 
+    // TODO: Can theoretically be removed the same way the Windows IPC service works,
+    // using an abort handle
     let (on_disconnect_tx, mut on_disconnect_rx) = mpsc::channel(1);
     let callback_handler = CallbackHandler { on_disconnect_tx };
 
@@ -333,9 +335,7 @@ pub(crate) fn run_debug_ipc_service() -> Result<()> {
                 tracing::info!("Caught Interrupt signal");
                 Ok(())
             }
-            future::Either::Right((Ok(()), _)) => {
-                bail!("Impossible, ipc_listen can't return Ok");
-            }
+            future::Either::Right((Ok(impossible), _)) => match impossible {},
             future::Either::Right((Err(error), _)) => Err(error).context("ipc_listen failed"),
         }
     })
@@ -383,7 +383,7 @@ impl Callbacks for CallbackHandlerIpc {
     }
 }
 
-async fn ipc_listen() -> Result<()> {
+async fn ipc_listen() -> Result<std::convert::Infallible> {
     let mut server = platform::IpcServer::new().await?;
     loop {
         connlib_shared::deactivate_dns_control()?;

rust/headless-client/src/linux.rs

@@ -167,7 +167,8 @@ pub(crate) fn run_ipc_service(cli: CliCommon) -> Result<()> {
         anyhow::bail!("This is the IPC service binary, it's not meant to run interactively.");
     }
     let rt = tokio::runtime::Runtime::new()?;
-    rt.block_on(async { crate::ipc_listen().await })
+    rt.block_on(crate::ipc_listen())?;
+    Ok(())
 }
 
 pub fn firezone_group() -> Result<nix::unistd::Group> {

rust/headless-client/src/windows.rs

@@ -5,20 +5,17 @@
 //! We must tell Windows explicitly when our service is stopping.
 
 use crate::{CliCommon, SignalKind};
-use anyhow::{anyhow, Context as _, Result};
+use anyhow::{Context as _, Result};
 use connlib_client_shared::file_logger;
 use connlib_shared::BUNDLE_ID;
 use std::{
     ffi::{c_void, OsString},
-    future::Future,
     net::IpAddr,
     path::{Path, PathBuf},
-    pin::pin,
     str::FromStr,
-    task::Poll,
     time::Duration,
 };
-use tokio::{net::windows::named_pipe, sync::mpsc};
+use tokio::net::windows::named_pipe;
 use tracing::subscriber::set_global_default;
 use tracing_subscriber::{layer::SubscriberExt as _, EnvFilter, Layer, Registry};
 use windows::Win32::Security as WinSec;
@@ -102,7 +99,13 @@ fn fallible_windows_service_run(arguments: Vec<OsString>) -> Result<()> {
     tracing::info!(?arguments, "fallible_windows_service_run");
 
     let rt = tokio::runtime::Runtime::new()?;
-    let (shutdown_tx, mut shutdown_rx) = mpsc::channel(1);
+
+    // Fixes <https://github.com/firezone/firezone/issues/4899>,
+    // DNS rules persisting after reboot
+    connlib_shared::deactivate_dns_control().ok();
+
+    let ipc_task = rt.spawn(super::ipc_listen());
+    let ipc_task_ah = ipc_task.abort_handle();
 
     let event_handler = move |control_event| -> ServiceControlHandlerResult {
         tracing::debug!(?control_event);
@@ -111,7 +114,7 @@ fn fallible_windows_service_run(arguments: Vec<OsString>) -> Result<()> {
             ServiceControl::Interrogate => ServiceControlHandlerResult::NoError,
             ServiceControl::Stop => {
                 tracing::info!("Got stop signal from service controller");
-                shutdown_tx.blocking_send(()).unwrap();
+                ipc_task_ah.abort();
                 ServiceControlHandlerResult::NoError
             }
             ServiceControl::UserEvent(_) => ServiceControlHandlerResult::NoError,
@@ -133,10 +136,6 @@ fn fallible_windows_service_run(arguments: Vec<OsString>) -> Result<()> {
         }
     };
 
-    // Fixes <https://github.com/firezone/firezone/issues/4899>,
-    // DNS rules persisting after reboot
-    connlib_shared::deactivate_dns_control().ok();
-
     // Tell Windows that we're running (equivalent to sd_notify in systemd)
     let status_handle = service_control_handler::register(SERVICE_NAME, event_handler)?;
     status_handle.set_service_status(ServiceStatus {
@@ -149,36 +148,18 @@ fn fallible_windows_service_run(arguments: Vec<OsString>) -> Result<()> {
         process_id: None,
     })?;
 
-    let mut ipc_service = pin!(super::ipc_listen());
-    let result = rt.block_on(async {
-        std::future::poll_fn(|cx| {
-            match shutdown_rx.poll_recv(cx) {
-                Poll::Ready(Some(())) => {
-                    tracing::info!("Got shutdown signal");
-                    return Poll::Ready(Ok(()));
-                }
-                Poll::Ready(None) => {
-                    return Poll::Ready(Err(anyhow!(
-                        "shutdown channel unexpectedly dropped, shutting down"
-                    )))
-                }
-                Poll::Pending => {}
-            }
-
-            match ipc_service.as_mut().poll(cx) {
-                Poll::Ready(Ok(())) => {
-                    return Poll::Ready(Err(anyhow!("Impossible, ipc_listen can't return Ok")))
-                }
-                Poll::Ready(Err(error)) => {
-                    return Poll::Ready(Err(error.context("ipc_listen failed")))
-                }
-                Poll::Pending => {}
-            }
-
-            Poll::Pending
-        })
-        .await
-    });
+    let result = match rt.block_on(ipc_task) {
+        Err(join_error) if join_error.is_cancelled() => {
+            // We cancelled because Windows asked us to shut down.
+            Ok(())
+        }
+        Err(join_error) => {
+            // The IPC task may have panicked
+            Err(join_error.into())
+        }
+        Ok(Err(error)) => Err(error.context("ipc_listen failed")),
+        Ok(Ok(impossible)) => match impossible {},
+    };
 
     // Tell Windows that we're stopping
     status_handle.set_service_status(ServiceStatus {