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 <ReactorScram@users.noreply.github.com>

.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

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

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 }

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 <https://github.com/firezone/firezone/issues/2697> and <https://github.com/firezone/firezone/issues/2711>,
+/// 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<String> {
+    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::<DeviceIdJson>(&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<std::path::PathBuf> {
+        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
+    /// <https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch05s08.html>
+    ///
+    /// `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<PathBuf> {
+        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 <https://github.com/firezone/firezone/issues/3712> lands
+    pub(crate) fn path() -> Option<std::path::PathBuf> {
+        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())));
+    }
+}

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")]

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;

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

@@ -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 <https://github.com/firezone/firezone/issues/2697> and <https://github.com/firezone/firezone/issues/2711>,
-/// 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<String, Error> {
-    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::<DeviceIdJson>(&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()
-    }
-}

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<bool> {
-        // 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.");
     }
 }
 

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

@@ -767,9 +767,8 @@ async fn run_controller(
     advanced_settings: AdvancedSettings,
     notify_controller: Arc<Notify>,
 ) -> 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,

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 <https://github.com/firezone/firezone/issues/3713> 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<PathBuf> {
-        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 <https://github.com/firezone/firezone/issues/3712> lands
-    pub(crate) fn device_id() -> Option<PathBuf> {
-        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()

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

@@ -54,6 +54,7 @@ pub(crate) fn setup(log_filter: &str) -> Result<Handles, Error> {
         );
     }
     LogTracer::init()?;
+    tracing::debug!(?log_path, "Log path");
     Ok(Handles {
         logger,
         _reloader: reloader,

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(())
 }
 

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<String>,
 
     /// File logging directory. Should be a path that's writeable by the current user.
     #[arg(short, long, env = "LOG_DIR")]

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

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