diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 2cb647142..0d8bd91f6 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -1909,6 +1909,7 @@ dependencies = [ "keyring", "minidumper", "native-dialog", + "nix 0.28.0", "output_vt100", "rand 0.8.5", "reqwest", diff --git a/rust/gui-client/src-tauri/Cargo.toml b/rust/gui-client/src-tauri/Cargo.toml index 92235760d..be66ffe37 100644 --- a/rust/gui-client/src-tauri/Cargo.toml +++ b/rust/gui-client/src-tauri/Cargo.toml @@ -55,6 +55,7 @@ zip = { version = "0.6.6", features = ["deflate", "time"], default-features = fa dirs = "5.0.1" # Used for infinite `pending` on not-yet-implemented functions futures = "0.3.30" +nix = { version = "0.28.0", features = ["user"] } tokio-util = { version = "0.7.10", features = ["codec"] } [target.'cfg(target_os = "macos")'.dependencies] diff --git a/rust/gui-client/src-tauri/src/client.rs b/rust/gui-client/src-tauri/src/client.rs index 12467e942..b3250dc3f 100644 --- a/rust/gui-client/src-tauri/src/client.rs +++ b/rust/gui-client/src-tauri/src/client.rs @@ -66,13 +66,18 @@ pub(crate) fn run() -> Result<()> { match cli.command { None => { - if elevation::check()? { + match elevation::check() { // We're already elevated, just run the GUI - run_gui(cli) - } else { - // We're not elevated, ask Powershell / sudo to re-launch us, then exit - elevation::elevate()?; - Ok(()) + Ok(true) => run_gui(cli), + Ok(false) => { + // We're not elevated, ask Powershell to re-launch us, then exit. On Linux this is completely different. + elevation::elevate()?; + Ok(()) + } + Err(error) => { + show_error_dialog(&error)?; + Err(error.into()) + } } } Some(Cmd::CrashHandlerServer { socket_path }) => crash_handling::server(socket_path), @@ -122,12 +127,15 @@ fn run_gui(cli: Cli) -> Result<()> { } fn show_error_dialog(error: &gui::Error) -> Result<()> { - #[allow(clippy::wildcard_enum_match_arm)] + // Decision to put the error strings here: let error_msg = match error { // TODO: Update this URL gui::Error::WebViewNotInstalled => "Firezone cannot start because WebView2 is not installed. Follow the instructions at .".to_string(), gui::Error::DeepLink(deep_link::Error::CantListen) => "Firezone is already running. If it's not responding, force-stop it.".to_string(), - error => error.to_string(), + gui::Error::DeepLink(deep_link::Error::Other(error)) => error.to_string(), + gui::Error::Logging(_) => "Logging error".to_string(), + gui::Error::UserNotInFirezoneGroup => "You are not a member of the group `firezone`. Try `sudo adduser $USER firezone` and then reboot".to_string(), + gui::Error::Other(error) => error.to_string(), }; native_dialog::MessageDialog::new() diff --git a/rust/gui-client/src-tauri/src/client/elevation.rs b/rust/gui-client/src-tauri/src/client/elevation.rs index 010aeaa50..3faeecf66 100644 --- a/rust/gui-client/src-tauri/src/client/elevation.rs +++ b/rust/gui-client/src-tauri/src/client/elevation.rs @@ -2,17 +2,24 @@ pub(crate) use imp::{check, elevate}; #[cfg(target_os = "linux")] mod imp { + use crate::client::gui::Error; use anyhow::{Context, Result}; #[allow(clippy::print_stderr)] - pub(crate) fn check() -> Result { + pub(crate) fn check() -> Result { // 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); } + + let fz_gid = firezone_headless_client::imp::firezone_group()?.gid; + let groups = nix::unistd::getgroups().context("`nix::unistd::getgroups`")?; + if !groups.contains(&fz_gid) { + return Err(Error::UserNotInFirezoneGroup); + } + Ok(true) } @@ -26,7 +33,7 @@ mod imp { mod imp { use anyhow::Result; - pub(crate) fn check() -> Result { + pub(crate) fn check() -> Result { Ok(true) } @@ -37,14 +44,14 @@ mod imp { #[cfg(target_os = "windows")] mod imp { - use crate::client::wintun_install; + use crate::client::{gui::Error, wintun_install}; use anyhow::{Context, Result}; use std::{os::windows::process::CommandExt, str::FromStr}; /// Check if we have elevated privileges, extract wintun.dll if needed. /// /// Returns true if already elevated, false if not elevated, error if we can't be sure - pub(crate) fn check() -> Result { + pub(crate) fn check() -> Result { // Almost the same as the code in tun_windows.rs in connlib const TUNNEL_UUID: &str = "72228ef4-cb84-4ca5-a4e6-3f8636e75757"; const TUNNEL_NAME: &str = "Firezone Elevation Check"; @@ -52,7 +59,11 @@ mod imp { let path = match wintun_install::ensure_dll() { Ok(x) => x, Err(wintun_install::Error::PermissionDenied) => return Ok(false), - Err(e) => return Err(e).context("Failed to ensure wintun.dll is installed"), + Err(e) => { + return Err(e) + .context("Failed to ensure wintun.dll is installed") + .map_err(Error::Other) + } }; // SAFETY: Unsafe needed because we're loading a DLL from disk and it has arbitrary C code in it. diff --git a/rust/gui-client/src-tauri/src/client/gui.rs b/rust/gui-client/src-tauri/src/client/gui.rs index 017e8487f..f4e3d4b78 100644 --- a/rust/gui-client/src-tauri/src/client/gui.rs +++ b/rust/gui-client/src-tauri/src/client/gui.rs @@ -80,7 +80,7 @@ impl Managed { } // TODO: Replace with `anyhow` gradually per -#[cfg_attr(target_os = "macos", allow(dead_code))] +#[allow(dead_code)] #[derive(Debug, thiserror::Error)] pub(crate) enum Error { #[error("Deep-link module error: {0}")] @@ -88,7 +88,9 @@ pub(crate) enum Error { #[error("Logging module error: {0}")] Logging(#[from] logging::Error), - // `client.rs` provides a more user-friendly message when showing the error dialog box + // `client.rs` provides a more user-friendly message when showing the error dialog box for certain variants + #[error("UserNotInFirezoneGroup")] + UserNotInFirezoneGroup, #[error("WebViewNotInstalled")] WebViewNotInstalled, #[error(transparent)] diff --git a/rust/gui-client/src-tauri/src/main.rs b/rust/gui-client/src-tauri/src/main.rs index 55636c3ec..c7914b24b 100644 --- a/rust/gui-client/src-tauri/src/main.rs +++ b/rust/gui-client/src-tauri/src/main.rs @@ -1,4 +1,4 @@ -//! The Firezone GUI client for Windows +//! The Firezone GUI client for Linux and Windows // Prevents additional console window on Windows in release, DO NOT REMOVE!! #![cfg_attr(not(debug_assertions), windows_subsystem = "windows")] diff --git a/rust/headless-client/src/imp_linux.rs b/rust/headless-client/src/imp_linux.rs index d3a4261fd..c719397da 100644 --- a/rust/headless-client/src/imp_linux.rs +++ b/rust/headless-client/src/imp_linux.rs @@ -184,6 +184,13 @@ pub(crate) fn run_ipc_service(cli: Cli) -> Result<()> { rt.block_on(async { ipc_listen(cli).await }) } +pub fn firezone_group() -> Result { + let group = nix::unistd::Group::from_name("firezone") + .context("can't get group by name")? + .context("firezone group must exist on the system")?; + Ok(group) +} + async fn ipc_listen(cli: Cli) -> Result<()> { // Remove the socket if a previous run left it there let sock_path = sock_path(); diff --git a/scripts/tests/smoke-test-gui-linux.sh b/scripts/tests/smoke-test-gui-linux.sh index 63aa916a4..1470c2b91 100755 --- a/scripts/tests/smoke-test-gui-linux.sh +++ b/scripts/tests/smoke-test-gui-linux.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash -source "./scripts/tests/lib.sh" +set -euox pipefail BUNDLE_ID="dev.firezone.client" @@ -22,6 +22,17 @@ cargo install --quiet --locked dump_syms minidump-stackwalk dump_syms ../target/debug/firezone-gui-client --output "$SYMS_PATH" ls -lash ../target/debug +sudo groupadd --force firezone +sudo adduser "$USER" firezone + +function run_fz_gui() { + pwd + # Does what it says + sudo --preserve-env \ + su --login "$USER" --command \ + "xvfb-run --auto-servernum $PWD/../target/debug/$PACKAGE $*" +} + function smoke_test() { # Make sure the files we want to check don't exist on the system yet stat "$LOGS_PATH" && exit 1 @@ -31,7 +42,7 @@ function smoke_test() { stat "$RAN_BEFORE_PATH" && exit 1 # Run the smoke test normally - if ! xvfb-run --auto-servernum ../target/debug/"$PACKAGE" --no-deep-links smoke-test + if ! run_fz_gui --no-deep-links smoke-test then minidump-stackwalk --symbols-path "$SYMS_PATH" "$DUMP_PATH" exit 1 @@ -49,7 +60,7 @@ function smoke_test() { stat "$RAN_BEFORE_PATH" # Run the test again and make sure the device ID is not changed - xvfb-run --auto-servernum ../target/debug/"$PACKAGE" --no-deep-links smoke-test + run_fz_gui --no-deep-links smoke-test # DEVICE_ID_2=$(cat "$DEVICE_ID_PATH") #if [ "$DEVICE_ID_1" != "$DEVICE_ID_2" ] @@ -70,7 +81,7 @@ function crash_test() { rm -f "$DUMP_PATH" # Fail if it returns success, this is supposed to crash - xvfb-run --auto-servernum ../target/debug/"$PACKAGE" --crash --no-deep-links && exit 1 + run_fz_gui --crash --no-deep-links && exit 1 # Fail if the crash file wasn't written stat "$DUMP_PATH"