From 7211e883388b8f8b16d5eff60812bb5fd083192f Mon Sep 17 00:00:00 2001 From: Reactor Scram Date: Fri, 8 Mar 2024 10:13:59 -0600 Subject: [PATCH] feat(linux-client): generate firezone-id (device ID) automatically if it's not provided at launch (#3920) Closes #3815 Changes that are breaking (but these aren't in production so it should be okay) - Windows, renaming `device_id.json` to `firezone-id.json` to match the rest of the code - Linux GUI, storing the firezone-id under `/var/lib` instead of under `$HOME` - Linux GUI, bails out if not run with `sudo --preserve-env` by detecting `$HOME == root` or `$USER != root` --------- Signed-off-by: Reactor Scram --- .github/workflows/_rust.yml | 1 + docker-compose.yml | 1 - rust/connlib/shared/Cargo.toml | 3 +- rust/connlib/shared/src/device_id.rs | 111 ++++++++++++++++++ rust/connlib/shared/src/lib.rs | 6 + rust/gui-client/src-tauri/src/client.rs | 1 - .../src-tauri/src/client/device_id.rs | 65 ---------- .../src-tauri/src/client/elevation.rs | 18 ++- rust/gui-client/src-tauri/src/client/gui.rs | 5 +- .../src-tauri/src/client/known_dirs.rs | 27 +---- .../src-tauri/src/client/logging.rs | 1 + .../src-tauri/src/client/settings.rs | 3 +- rust/linux-client/src/main.rs | 12 +- scripts/tests/smoke-test-gui-linux.sh | 54 ++++++--- scripts/tests/smoke-test-gui-windows.sh | 24 +++- 15 files changed, 208 insertions(+), 124 deletions(-) create mode 100644 rust/connlib/shared/src/device_id.rs delete mode 100644 rust/gui-client/src-tauri/src/client/device_id.rs diff --git a/.github/workflows/_rust.yml b/.github/workflows/_rust.yml index 362fb679e..edaf1d834 100644 --- a/.github/workflows/_rust.yml +++ b/.github/workflows/_rust.yml @@ -81,6 +81,7 @@ jobs: runs-on: ${{ matrix.runs-on }} defaults: run: + # Must be in this dir for `pnpm` to work working-directory: ./rust/gui-client env: CONNLIB_LOG_UPLOAD_INTERVAL_SECS: 300 diff --git a/docker-compose.yml b/docker-compose.yml index dbaeb2878..e05b80ad0 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -113,7 +113,6 @@ services: FIREZONE_TOKEN: "n.SFMyNTY.g2gDaANtAAAAJGM4OWJjYzhjLTkzOTItNGRhZS1hNDBkLTg4OGFlZjZkMjhlMG0AAAAkN2RhN2QxY2QtMTExYy00NGE3LWI1YWMtNDAyN2I5ZDIzMGU1bQAAACtBaUl5XzZwQmstV0xlUkFQenprQ0ZYTnFJWktXQnMyRGR3XzJ2Z0lRdkZnbgYAGUmu74wBYgABUYA.UN3vSLLcAMkHeEh5VHumPOutkuue8JA6wlxM9JxJEPE" RUST_LOG: firezone_linux_client=trace,wire=trace,connlib_client_shared=trace,firezone_tunnel=trace,connlib_shared=trace,boringtun=debug,snownet=debug,str0m=debug,info FIREZONE_API_URL: ws://api:8081 - FIREZONE_ID: D0455FDE-8F65-4960-A778-B934E4E85A5F init: true build: target: debug diff --git a/rust/connlib/shared/Cargo.toml b/rust/connlib/shared/Cargo.toml index 94fb4ab09..31e8bd135 100644 --- a/rust/connlib/shared/Cargo.toml +++ b/rust/connlib/shared/Cargo.toml @@ -9,6 +9,7 @@ edition = "2021" mock = [] [dependencies] +anyhow = "1.0.80" # Will be needed to safely modify `/etc/resolv.conf` atomicwrites = "0.4.3" secrecy = { workspace = true, features = ["serde", "bytes"] } @@ -28,7 +29,7 @@ resolv-conf = "0.7.0" serde = { version = "1.0", default-features = false, features = ["derive", "std"] } serde_json = { version = "1.0", default-features = false, features = ["std"] } thiserror = { version = "1.0", default-features = false } -tokio = { version = "1.36", default-features = false, features = ["rt", "rt-multi-thread"]} +tokio = { version = "1.36", default-features = false, features = ["fs", "rt", "rt-multi-thread"]} tokio-stream = { version = "0.1", features = ["time"] } tokio-tungstenite = { version = "0.21", default-features = false, features = ["connect", "handshake", "rustls-tls-webpki-roots"] } tracing = { workspace = true } diff --git a/rust/connlib/shared/src/device_id.rs b/rust/connlib/shared/src/device_id.rs new file mode 100644 index 000000000..3b7eab95b --- /dev/null +++ b/rust/connlib/shared/src/device_id.rs @@ -0,0 +1,111 @@ +use anyhow::{Context, Result}; +use std::fs; +use std::io::Write; + +/// Returns the device ID, generating it and saving it to disk if needed. +/// +/// Per and , +/// clients must generate their own random IDs and persist them to disk, to handle situations like VMs where a hardware ID is not unique or not available. +/// +/// Returns: The UUID as a String, suitable for sending verbatim to `connlib_client_shared::Session::connect`. +/// +/// Errors: If the disk is unwritable when initially generating the ID, or unwritable when re-generating an invalid ID. +pub fn get() -> Result { + let dir = imp::path().context("Failed to compute path for firezone-id file")?; + let path = dir.join("firezone-id.json"); + + // Try to read it from the disk + if let Some(j) = fs::read_to_string(&path) + .ok() + .and_then(|s| serde_json::from_str::(&s).ok()) + { + let device_id = j.device_id(); + tracing::debug!(?device_id, "Loaded device ID from disk"); + return Ok(device_id); + } + + // Couldn't read, it's missing or invalid, generate a new one and save it. + let id = uuid::Uuid::new_v4(); + let j = DeviceIdJson { id }; + // TODO: This file write has the same possible problems with power loss as described here https://github.com/firezone/firezone/pull/2757#discussion_r1416374516 + // Since the device ID is random, typically only written once in the device's lifetime, and the read will error out if it's corrupted, it's low-risk. + fs::create_dir_all(&dir).context("Failed to create dir for firezone-id")?; + + let content = + serde_json::to_string(&j).context("Impossible: Failed to serialize firezone-id")?; + + let file = + atomicwrites::AtomicFile::new(&path, atomicwrites::OverwriteBehavior::DisallowOverwrite); + file.write(|f| f.write_all(content.as_bytes())) + .context("Failed to write firezone-id file")?; + + let device_id = j.device_id(); + tracing::debug!(?device_id, "Saved device ID to disk"); + Ok(j.device_id()) +} + +#[derive(serde::Deserialize, serde::Serialize)] +struct DeviceIdJson { + id: uuid::Uuid, +} + +impl DeviceIdJson { + fn device_id(&self) -> String { + self.id.to_string() + } +} + +#[cfg(not(any(target_os = "linux", target_os = "windows")))] +mod imp { + pub(crate) fn path() -> Option { + panic!("This function is only implemented on Linux and Windows since those have pure-Rust clients") + } +} + +#[cfg(target_os = "linux")] +mod imp { + use std::path::PathBuf; + /// `/var/lib/$BUNDLE_ID/config/firezone-id` + /// + /// `/var/lib` because this is the correct place to put state data not meant for users + /// to touch, which is specific to one host and persists across reboots + /// + /// + /// `BUNDLE_ID` because we need our own subdir + /// + /// `config` to make how Windows has `config` and `data` both under `AppData/Local/$BUNDLE_ID` + pub(crate) fn path() -> Option { + Some( + PathBuf::from("/var/lib") + .join(crate::BUNDLE_ID) + .join("config"), + ) + } +} + +#[cfg(target_os = "windows")] +mod imp { + use known_folders::{get_known_folder_path, KnownFolder}; + + /// e.g. `C:\ProgramData\dev.firezone.client\config` + /// + /// Device ID is stored here until lands + pub(crate) fn path() -> Option { + Some( + get_known_folder_path(KnownFolder::ProgramData)? + .join(crate::BUNDLE_ID) + .join("config"), + ) + } +} + +#[cfg(test)] +mod tests { + #[test] + fn smoke() { + let dir = super::imp::path().expect("should have gotten Some(path)"); + assert!(dir + .components() + .any(|x| x == std::path::Component::Normal("dev.firezone.client".as_ref()))); + } +} diff --git a/rust/connlib/shared/src/lib.rs b/rust/connlib/shared/src/lib.rs index e40a006a6..5e3d7856e 100644 --- a/rust/connlib/shared/src/lib.rs +++ b/rust/connlib/shared/src/lib.rs @@ -9,6 +9,12 @@ pub mod control; pub mod error; pub mod messages; +/// Module to generate and store a persistent device ID on disk +/// +/// Only properly implemented on Linux and Windows (platforms with Tauri and headless client) +pub mod device_id; + +// Must be compiled on Mac so the Mac runner can do `version-check` on `linux-client` pub mod linux; #[cfg(target_os = "windows")] diff --git a/rust/gui-client/src-tauri/src/client.rs b/rust/gui-client/src-tauri/src/client.rs index ac1dac2d0..724da9f73 100644 --- a/rust/gui-client/src-tauri/src/client.rs +++ b/rust/gui-client/src-tauri/src/client.rs @@ -7,7 +7,6 @@ mod auth; mod crash_handling; mod debug_commands; mod deep_link; -mod device_id; mod elevation; mod gui; mod known_dirs; diff --git a/rust/gui-client/src-tauri/src/client/device_id.rs b/rust/gui-client/src-tauri/src/client/device_id.rs deleted file mode 100644 index dd7d10607..000000000 --- a/rust/gui-client/src-tauri/src/client/device_id.rs +++ /dev/null @@ -1,65 +0,0 @@ -use tokio::fs; - -#[derive(thiserror::Error, Debug)] -pub(crate) enum Error { - #[error("Couldn't create app-specific dir in `ProgramData`: {0}")] - CreateProgramDataDir(std::io::Error), - #[error("Can't find well-known folder")] - KnownFolder, - #[error("Couldn't write device ID file: {0}")] - WriteDeviceIdFile(std::io::Error), -} - -/// Returns the device ID, generating it and saving it to disk if needed. -/// -/// Per and , -/// clients must generate their own random IDs and persist them to disk, to handle situations like VMs where a hardware ID is not unique or not available. -/// -/// Returns: The UUID as a String, suitable for sending verbatim to `connlib_client_shared::Session::connect`. -/// -/// Errors: If the disk is unwritable when initially generating the ID, or unwritable when re-generating an invalid ID. -pub(crate) async fn device_id() -> Result { - let dir = crate::client::known_dirs::device_id().ok_or(Error::KnownFolder)?; - let path = dir.join("device_id.json"); - - // Try to read it back from disk - if let Some(j) = fs::read_to_string(&path) - .await - .ok() - .and_then(|s| serde_json::from_str::(&s).ok()) - { - let device_id = j.device_id(); - tracing::debug!(?device_id, "Loaded device ID from disk"); - return Ok(device_id); - } - - // Couldn't read, it's missing or invalid, generate a new one and save it. - let id = uuid::Uuid::new_v4(); - let j = DeviceIdJson { id }; - // TODO: This file write has the same possible problems with power loss as described here https://github.com/firezone/firezone/pull/2757#discussion_r1416374516 - // Since the device ID is random, typically only written once in the device's lifetime, and the read will error out if it's corrupted, it's low-risk. - fs::create_dir_all(&dir) - .await - .map_err(Error::CreateProgramDataDir)?; - fs::write( - &path, - serde_json::to_string(&j).expect("Device ID should always be serializable"), - ) - .await - .map_err(Error::WriteDeviceIdFile)?; - - let device_id = j.device_id(); - tracing::debug!(?device_id, "Saved device ID to disk"); - Ok(j.device_id()) -} - -#[derive(serde::Deserialize, serde::Serialize)] -struct DeviceIdJson { - id: uuid::Uuid, -} - -impl DeviceIdJson { - fn device_id(&self) -> String { - self.id.to_string() - } -} diff --git a/rust/gui-client/src-tauri/src/client/elevation.rs b/rust/gui-client/src-tauri/src/client/elevation.rs index 8422a5a08..4c6d6e3b7 100644 --- a/rust/gui-client/src-tauri/src/client/elevation.rs +++ b/rust/gui-client/src-tauri/src/client/elevation.rs @@ -2,15 +2,27 @@ pub(crate) use imp::{check, elevate}; #[cfg(target_os = "linux")] mod imp { - use anyhow::Result; + use anyhow::{Context, Result}; pub(crate) fn check() -> Result { - // TODO + // 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 with root permissions to set up DNS. Re-run it with `sudo --preserve-env`"); + return Ok(false); + } + let home = std::env::var("HOME").context("HOME env var should be set")?; + if home == "/root" { + eprintln!("If Firezone is run with `$HOME == /root`, deep links will not work. Re-run it with `sudo --preserve-env`"); + // If we don't bail out here, this message will probably never be read. + return Ok(false); + } Ok(true) } pub(crate) fn elevate() -> Result<()> { - todo!() + anyhow::bail!("Firezone does not self-elevate on Linux."); } } diff --git a/rust/gui-client/src-tauri/src/client/gui.rs b/rust/gui-client/src-tauri/src/client/gui.rs index 349e1abe0..5af8a22f5 100644 --- a/rust/gui-client/src-tauri/src/client/gui.rs +++ b/rust/gui-client/src-tauri/src/client/gui.rs @@ -767,9 +767,8 @@ async fn run_controller( advanced_settings: AdvancedSettings, notify_controller: Arc, ) -> Result<()> { - let device_id = client::device_id::device_id() - .await - .context("Failed to read / create device ID")?; + let device_id = + connlib_shared::device_id::get().context("Failed to read / create device ID")?; let mut controller = Controller { advanced_settings, diff --git a/rust/gui-client/src-tauri/src/client/known_dirs.rs b/rust/gui-client/src-tauri/src/client/known_dirs.rs index df08567f8..a7210c36e 100644 --- a/rust/gui-client/src-tauri/src/client/known_dirs.rs +++ b/rust/gui-client/src-tauri/src/client/known_dirs.rs @@ -7,23 +7,13 @@ //! //! I wanted the ProgramData folder on Windows, which `dirs` alone doesn't provide. -pub(crate) use imp::{device_id, logs, runtime, session, settings}; +pub(crate) use imp::{logs, runtime, session, settings}; #[cfg(any(target_os = "linux", target_os = "macos"))] mod imp { use connlib_shared::BUNDLE_ID; use std::path::PathBuf; - /// e.g. `/home/alice/.config/dev.firezone.client/config` - /// - /// Device ID is stored here until lands - /// - /// Linux has no direct equivalent to Window's `ProgramData` dir, `/var` doesn't seem - /// to be writable by normal users. - pub(crate) fn device_id() -> Option { - Some(dirs::config_local_dir()?.join(BUNDLE_ID).join("config")) - } - /// e.g. `/home/alice/.cache/dev.firezone.client/data/logs` /// /// Logs are considered cache because they're not configs and it's technically okay @@ -58,21 +48,8 @@ mod imp { #[cfg(target_os = "windows")] mod imp { - use connlib_shared::BUNDLE_ID; - use known_folders::{get_known_folder_path, KnownFolder}; use std::path::PathBuf; - /// e.g. `C:\ProgramData\dev.firezone.client\config` - /// - /// Device ID is stored here until lands - pub(crate) fn device_id() -> Option { - Some( - get_known_folder_path(KnownFolder::ProgramData)? - .join(BUNDLE_ID) - .join("config"), - ) - } - /// e.g. `C:\Users\Alice\AppData\Local\dev.firezone.client\data\logs` /// /// See connlib docs for details @@ -125,7 +102,7 @@ mod tests { #[test] fn smoke() { - for dir in [device_id(), logs(), runtime(), session(), settings()] { + for dir in [logs(), runtime(), session(), settings()] { let dir = dir.expect("should have gotten Some(path)"); assert!(dir .components() diff --git a/rust/gui-client/src-tauri/src/client/logging.rs b/rust/gui-client/src-tauri/src/client/logging.rs index 256aeb1a2..99dbc3249 100644 --- a/rust/gui-client/src-tauri/src/client/logging.rs +++ b/rust/gui-client/src-tauri/src/client/logging.rs @@ -54,6 +54,7 @@ pub(crate) fn setup(log_filter: &str) -> Result { ); } LogTracer::init()?; + tracing::debug!(?log_path, "Log path"); Ok(Handles { logger, _reloader: reloader, diff --git a/rust/gui-client/src-tauri/src/client/settings.rs b/rust/gui-client/src-tauri/src/client/settings.rs index 7ac84d4eb..6299d321a 100644 --- a/rust/gui-client/src-tauri/src/client/settings.rs +++ b/rust/gui-client/src-tauri/src/client/settings.rs @@ -110,7 +110,8 @@ pub(crate) async fn save(settings: &AdvancedSettings) -> Result<()> { .parent() .context("settings path should have a parent")?; tokio::fs::create_dir_all(dir).await?; - tokio::fs::write(path, serde_json::to_string(settings)?).await?; + tokio::fs::write(&path, serde_json::to_string(settings)?).await?; + tracing::debug!(?path, "Saved settings"); Ok(()) } diff --git a/rust/linux-client/src/main.rs b/rust/linux-client/src/main.rs index 7959397d9..126138725 100644 --- a/rust/linux-client/src/main.rs +++ b/rust/linux-client/src/main.rs @@ -19,10 +19,16 @@ fn main() -> Result<()> { handle, }; + // AKA "Device ID", not the Firezone slug + let firezone_id = match cli.firezone_id { + Some(id) => id, + None => connlib_shared::device_id::get().context("Could not get `firezone_id` from CLI, could not read it from disk, could not generate it and save it to disk")?, + }; + let mut session = Session::connect( cli.common.api_url, SecretString::from(cli.common.token), - cli.firezone_id, + firezone_id, None, None, callbacks, @@ -143,8 +149,10 @@ struct Cli { common: CommonArgs, /// Identifier used by the portal to identify and display the device. + /// + /// AKA `device_id` in the Windows and Linux GUI clients #[arg(short = 'i', long, env = "FIREZONE_ID")] - pub firezone_id: String, + pub firezone_id: Option, /// File logging directory. Should be a path that's writeable by the current user. #[arg(short, long, env = "LOG_DIR")] diff --git a/scripts/tests/smoke-test-gui-linux.sh b/scripts/tests/smoke-test-gui-linux.sh index 5c7120f41..17111264a 100755 --- a/scripts/tests/smoke-test-gui-linux.sh +++ b/scripts/tests/smoke-test-gui-linux.sh @@ -3,49 +3,69 @@ set -euo pipefail BUNDLE_ID="dev.firezone.client" -DUMP_PATH="$HOME/.cache/$BUNDLE_ID/data/logs/last_crash.dmp" + +DEVICE_ID_PATH="/var/lib/$BUNDLE_ID/config/firezone-id.json" +LOGS_PATH="$HOME/.cache/$BUNDLE_ID/data/logs" +DUMP_PATH="$LOGS_PATH/last_crash.dmp" +SETTINGS_PATH="$HOME/.config/$BUNDLE_ID/config/advanced_settings.json" + export FIREZONE_DISABLE_SYSTRAY=true PACKAGE=firezone-gui-client export RUST_LOG=firezone_gui_client=debug,warn export WEBKIT_DISABLE_COMPOSITING_MODE=1 +cargo build -p "$PACKAGE" + function smoke_test() { # Make sure the files we want to check don't exist on the system yet - stat "$HOME/.cache/$BUNDLE_ID/data/logs" && exit 1 - stat "$HOME/.config/$BUNDLE_ID/config/advanced_settings.json" && exit 1 - stat "$HOME/.config/$BUNDLE_ID/config/device_id.json" && exit 1 + sudo stat "$LOGS_PATH" && exit 1 + sudo stat "$SETTINGS_PATH" && exit 1 + sudo stat "$DEVICE_ID_PATH" && exit 1 # Run the smoke test normally - xvfb-run --auto-servernum cargo run -p "$PACKAGE" -- smoke-test + sudo --preserve-env xvfb-run --auto-servernum ../target/debug/"$PACKAGE" smoke-test + + # Note the device ID + DEVICE_ID_1=$(cat "$DEVICE_ID_PATH") # Make sure the files were written in the right paths # TODO: Inject some bogus sign-in sequence to test the actor_name file - stat "$HOME/.cache/$BUNDLE_ID/data/logs/"connlib*log - stat "$HOME/.config/$BUNDLE_ID/config/advanced_settings.json" - stat "$HOME/.config/$BUNDLE_ID/config/device_id.json" + # https://stackoverflow.com/questions/41321092 + sudo bash -c "stat \"${LOGS_PATH}/\"connlib*log" + sudo stat "$SETTINGS_PATH" + sudo stat "$DEVICE_ID_PATH" + + # Run the test again and make sure the device ID is not changed + sudo --preserve-env xvfb-run --auto-servernum ../target/debug/"$PACKAGE" smoke-test + DEVICE_ID_2=$(cat "$DEVICE_ID_PATH") + + if [ "$DEVICE_ID_1" != "$DEVICE_ID_2" ] + then + echo "The device ID should not change if the file is intact between runs" + exit 1 + fi # Clean up the files but not the folders - rm -rf "$HOME/.cache/$BUNDLE_ID/data/logs" - rm "$HOME/.config/$BUNDLE_ID/config/advanced_settings.json" - rm "$HOME/.config/$BUNDLE_ID/config/device_id.json" + sudo rm -rf "$LOGS_PATH" + sudo rm "$SETTINGS_PATH" + sudo rm "$DEVICE_ID_PATH" } function crash_test() { # Delete the crash file if present - rm -f "$DUMP_PATH" + sudo rm -f "$DUMP_PATH" # Fail if it returns success, this is supposed to crash - xvfb-run --auto-servernum cargo run -p "$PACKAGE" -- --crash && exit 1 + sudo --preserve-env xvfb-run --auto-servernum ../target/debug/"$PACKAGE" --crash && exit 1 # Fail if the crash file wasn't written - stat "$DUMP_PATH" + sudo stat "$DUMP_PATH" # Clean up - rm "$DUMP_PATH" + sudo rm "$DUMP_PATH" } -# Run the tests twice to make sure it's okay for the directories to stay intact, -# and to make sure the tests can cycle. +# Run the tests twice to make sure it's okay for the directories to stay intact smoke_test smoke_test crash_test diff --git a/scripts/tests/smoke-test-gui-windows.sh b/scripts/tests/smoke-test-gui-windows.sh index d417da436..9c687fd65 100755 --- a/scripts/tests/smoke-test-gui-windows.sh +++ b/scripts/tests/smoke-test-gui-windows.sh @@ -5,21 +5,22 @@ set -euo pipefail -BUNDLE_ID="dev.firezone.client" -DUMP_PATH="$LOCALAPPDATA/$BUNDLE_ID/data/logs/last_crash.dmp" -PACKAGE=firezone-gui-client - # This prevents a `shellcheck` lint warning about using an unset CamelCase var if [[ -z "$ProgramData" ]]; then echo "The env var \$ProgramData should be set to \`C:\ProgramData\` or similar" exit 1 fi +BUNDLE_ID="dev.firezone.client" +DEVICE_ID_PATH="$ProgramData/$BUNDLE_ID/config/firezone-id.json" +DUMP_PATH="$LOCALAPPDATA/$BUNDLE_ID/data/logs/last_crash.dmp" +PACKAGE=firezone-gui-client + function smoke_test() { files=( "$LOCALAPPDATA/$BUNDLE_ID/config/advanced_settings.json" "$LOCALAPPDATA/$BUNDLE_ID/data/wintun.dll" - "$ProgramData/$BUNDLE_ID/config/device_id.json" + "$DEVICE_ID_PATH" ) # Make sure the files we want to check don't exist on the system yet @@ -33,6 +34,19 @@ function smoke_test() { # Run the smoke test normally cargo run -p "$PACKAGE" -- smoke-test + # Note the device ID + DEVICE_ID_1=$(cat "$DEVICE_ID_PATH") + + # Run the test again and make sure the device ID is not changed + cargo run -p "$PACKAGE" -- smoke-test + DEVICE_ID_2=$(cat "$DEVICE_ID_PATH") + + if [ "$DEVICE_ID_1" != "$DEVICE_ID_2" ] + then + echo "The device ID should not change if the file is intact between runs" + exit 1 + fi + # Make sure the files were written in the right paths for file in "${files[@]}" do