From 8e5f7c423df684d41fa006d8f4433a40eaf6dd9b Mon Sep 17 00:00:00 2001 From: neuronull <9162534+neuronull@users.noreply.github.com> Date: Thu, 4 Sep 2025 10:59:48 -0600 Subject: [PATCH] draft1: missing flatpak/snap support --- apps/desktop/desktop_native/Cargo.lock | 5 +- apps/desktop/desktop_native/Cargo.toml | 1 + apps/desktop/desktop_native/core/Cargo.toml | 1 + .../desktop_native/core/src/ssh_agent/mod.rs | 35 ++- .../desktop_native/core/src/ssh_agent/unix.rs | 286 +++++++++++++----- .../core/src/ssh_agent/windows.rs | 24 +- apps/desktop/desktop_native/napi/src/lib.rs | 3 +- 7 files changed, 244 insertions(+), 111 deletions(-) diff --git a/apps/desktop/desktop_native/Cargo.lock b/apps/desktop/desktop_native/Cargo.lock index 70814c74106..795a7f6461c 100644 --- a/apps/desktop/desktop_native/Cargo.lock +++ b/apps/desktop/desktop_native/Cargo.lock @@ -552,9 +552,9 @@ dependencies = [ [[package]] name = "cfg-if" -version = "1.0.0" +version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" +checksum = "2fd1289c04a9ea8cb22300a459a72a385d7c73d3259e2ed7dcb2af674838cfa9" [[package]] name = "cfg_aliases" @@ -879,6 +879,7 @@ dependencies = [ "byteorder", "bytes", "cbc", + "cfg-if", "core-foundation", "desktop_objc", "dirs", diff --git a/apps/desktop/desktop_native/Cargo.toml b/apps/desktop/desktop_native/Cargo.toml index b5819d3688e..18499e1dcef 100644 --- a/apps/desktop/desktop_native/Cargo.toml +++ b/apps/desktop/desktop_native/Cargo.toml @@ -26,6 +26,7 @@ bitwarden-russh = { git = "https://github.com/bitwarden/bitwarden-russh.git", re byteorder = "=1.5.0" bytes = "=1.10.1" cbc = "=0.1.2" +cfg-if = "=1.0.3" core-foundation = "=0.10.0" dirs = "=6.0.0" ed25519 = "=2.2.3" diff --git a/apps/desktop/desktop_native/core/Cargo.toml b/apps/desktop/desktop_native/core/Cargo.toml index 97189e1d7cd..79ffc6471e1 100644 --- a/apps/desktop/desktop_native/core/Cargo.toml +++ b/apps/desktop/desktop_native/core/Cargo.toml @@ -26,6 +26,7 @@ bitwarden-russh = { workspace = true } byteorder = { workspace = true } bytes = { workspace = true } cbc = { workspace = true, features = ["alloc"] } +cfg-if = { workspace = true } dirs = { workspace = true } ed25519 = { workspace = true, features = ["pkcs8"] } futures = { workspace = true } diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs b/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs index d038ba2277f..c60e2940649 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs @@ -1,6 +1,9 @@ -use std::sync::{ - atomic::{AtomicBool, AtomicU32}, - Arc, +use std::{ + collections::HashMap, + sync::{ + atomic::{AtomicBool, AtomicU32}, + Arc, RwLock, + }, }; use base64::{engine::general_purpose::STANDARD, Engine as _}; @@ -24,8 +27,8 @@ pub mod peerinfo; mod request_parser; #[derive(Clone)] -pub struct BitwardenDesktopAgent { - keystore: ssh_agent::KeyStore, +pub struct BitwardenDesktopAgent { + keystore: ssh_agent::KeyStore, cancellation_token: CancellationToken, show_ui_request_tx: tokio::sync::mpsc::Sender, get_ui_response_rx: Arc>>, @@ -76,9 +79,7 @@ impl SshKey for BitwardenSshKey { } } -impl ssh_agent::Agent - for BitwardenDesktopAgent -{ +impl ssh_agent::Agent for BitwardenDesktopAgent { async fn confirm( &self, ssh_key: BitwardenSshKey, @@ -178,7 +179,23 @@ impl ssh_agent::Agent } } -impl BitwardenDesktopAgent { +impl BitwardenDesktopAgent { + /// Create a new `BitwardenDesktopAgent` from the provided auth channel handles. + pub fn new( + auth_request_tx: tokio::sync::mpsc::Sender, + auth_response_rx: Arc>>, + ) -> Self { + Self { + keystore: ssh_agent::KeyStore(Arc::new(RwLock::new(HashMap::new()))), + cancellation_token: CancellationToken::new(), + show_ui_request_tx: auth_request_tx, + get_ui_response_rx: auth_response_rx, + request_id: Arc::new(AtomicU32::new(0)), + needs_unlock: Arc::new(AtomicBool::new(true)), + is_running: Arc::new(AtomicBool::new(false)), + } + } + pub fn stop(&self) { if !self.is_running() { println!("[BitwardenDesktopAgent] Tried to stop agent while it is not running"); diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs b/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs index 53142d4c476..c98d9810058 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs @@ -1,113 +1,241 @@ -use std::{ - collections::HashMap, - fs, - os::unix::fs::PermissionsExt, - sync::{ - atomic::{AtomicBool, AtomicU32}, - Arc, RwLock, - }, -}; +use std::{fs, os::unix::fs::PermissionsExt, path::PathBuf, sync::Arc}; +use anyhow::anyhow; use bitwarden_russh::ssh_agent; use homedir::my_home; use tokio::{net::UnixListener, sync::Mutex}; -use tokio_util::sync::CancellationToken; use crate::ssh_agent::peercred_unix_listener_stream::PeercredUnixListenerStream; -use super::{BitwardenDesktopAgent, BitwardenSshKey, SshAgentUIRequest}; +use super::{BitwardenDesktopAgent, SshAgentUIRequest}; -impl BitwardenDesktopAgent { +/// User can override the default socket path with this env var +const ENV_BITWARDEN_SSH_AUTH_SOCK: &str = "BITWARDEN_SSH_AUTH_SOCK"; + +const FLATPACK_DATA_DIR: &str = ".var/app/com.bitwarden.desktop/data"; + +const LEGACY_SOCKFILE_NAME: &str = ".bitwarden-ssh-agent.sock"; + +const SOCKFILE_NAME: &str = "ssh-agent.sock"; + +impl BitwardenDesktopAgent { + /// Starts the Bitwarden Desktop SSH Agent server. + /// # Errors + /// Will return `Err` if unable to create and set permissions for socket file path or + /// if unable to bind to the socket path. pub fn start_server( auth_request_tx: tokio::sync::mpsc::Sender, auth_response_rx: Arc>>, ) -> Result { - let agent = BitwardenDesktopAgent { - keystore: ssh_agent::KeyStore(Arc::new(RwLock::new(HashMap::new()))), - cancellation_token: CancellationToken::new(), - show_ui_request_tx: auth_request_tx, - get_ui_response_rx: auth_response_rx, - request_id: Arc::new(AtomicU32::new(0)), - needs_unlock: Arc::new(AtomicBool::new(true)), - is_running: Arc::new(AtomicBool::new(false)), - }; - let cloned_agent_state = agent.clone(); - tokio::spawn(async move { - let ssh_path = match std::env::var("BITWARDEN_SSH_AUTH_SOCK") { - Ok(path) => path, - Err(_) => { - println!("[SSH Agent Native Module] BITWARDEN_SSH_AUTH_SOCK not set, using default path"); + // socket_paths are one of the following: + // - only the env var socket path if it is defined + // - or use the legacy $HOME path if flatpak build + // - or use both legacy and forward looking default paths. + // TODO: in PM, remove the legacy path logic and the for loop below. + let socket_paths = if let Ok(path) = std::env::var(ENV_BITWARDEN_SSH_AUTH_SOCK) { + vec![PathBuf::from(path)] + } else { + println!("[SSH Agent Native Module] {ENV_BITWARDEN_SSH_AUTH_SOCK} not set, using default path"); - let ssh_agent_directory = match my_home() { - Ok(Some(home)) => home, - _ => { - println!( - "[SSH Agent Native Module] Could not determine home directory" - ); - return; - } - }; + let mut paths = vec![get_legacy_default_socket_path()?]; - let is_flatpak = std::env::var("container") == Ok("flatpak".to_string()); - if !is_flatpak { - ssh_agent_directory - .join(".bitwarden-ssh-agent.sock") - .to_str() - .expect("Path should be valid") - .to_owned() - } else { - ssh_agent_directory - .join(".var/app/com.bitwarden.desktop/data/.bitwarden-ssh-agent.sock") - .to_str() - .expect("Path should be valid") - .to_owned() - } - } - }; + // TODO: handle flatpak/snap + if !is_flatpak() { + let basedir = get_socket_basedir()?; + let socket_path = get_default_socket_path(basedir); - println!("[SSH Agent Native Module] Starting SSH Agent server on {ssh_path:?}"); - let sockname = std::path::Path::new(&ssh_path); - if let Err(e) = std::fs::remove_file(sockname) { - println!("[SSH Agent Native Module] Could not remove existing socket file: {e}"); - if e.kind() != std::io::ErrorKind::NotFound { - return; - } + // create the bitwarden subdir if needed + fs::create_dir_all( + socket_path + .parent() + .ok_or(anyhow!("Malformed default socket path."))?, + ) + .map_err(|e| anyhow!(format!("Error creating {socket_path:?}: {e}")))?; + + paths.push(socket_path); } + paths + }; - match UnixListener::bind(sockname) { + let agent_state = BitwardenDesktopAgent::new(auth_request_tx, auth_response_rx); + + for sock_path in &socket_paths { + // if the socket is already present and wasn't cleanly removed during a previous + // runtime, remove it before beginning anew. + remove_path(sock_path)?; + + println!("[SSH Agent Native Module] Starting SSH Agent server {sock_path:?}"); + + match UnixListener::bind(sock_path) { Ok(listener) => { // Only the current user should be able to access the socket - if let Err(e) = fs::set_permissions(sockname, fs::Permissions::from_mode(0o600)) - { - println!("[SSH Agent Native Module] Could not set socket permissions: {e}"); - return; - } + set_user_permissions(sock_path)?; let stream = PeercredUnixListenerStream::new(listener); + let cloned_agent_state = agent_state.clone(); let cloned_keystore = cloned_agent_state.keystore.clone(); let cloned_cancellation_token = cloned_agent_state.cancellation_token.clone(); - cloned_agent_state + + tokio::spawn(async move { + let _ = ssh_agent::serve( + stream, + cloned_agent_state.clone(), + cloned_keystore, + cloned_cancellation_token, + ) + .await; + + cloned_agent_state + .is_running + .store(false, std::sync::atomic::Ordering::Relaxed); + + println!("[SSH Agent Native Module] SSH Agent server exited"); + }); + + agent_state .is_running .store(true, std::sync::atomic::Ordering::Relaxed); - let _ = ssh_agent::serve( - stream, - cloned_agent_state.clone(), - cloned_keystore, - cloned_cancellation_token, - ) - .await; - cloned_agent_state - .is_running - .store(false, std::sync::atomic::Ordering::Relaxed); - println!("[SSH Agent Native Module] SSH Agent server exited"); + + println!("[SSH Agent Native Module] SSH Agent is running: {sock_path:?}"); } Err(e) => { eprintln!("[SSH Agent Native Module] Error while starting agent server: {e}"); + return Err(e.into()); } } - }); + } - Ok(agent) + Ok(agent_state) + } +} + +fn is_flatpak() -> bool { + std::env::var("container") == Ok("flatpak".to_string()) +} + +// use the $HOME directory +fn get_legacy_default_socket_path() -> Result { + let Ok(Some(mut ssh_agent_directory)) = my_home() else { + println!("[SSH Agent Native Module] Could not determine home directory"); + return Err(anyhow!("Could not determine home directory.")); + }; + + if std::env::var("container") == Ok("flatpak".to_string()) { + ssh_agent_directory = ssh_agent_directory.join(FLATPACK_DATA_DIR); + } + + ssh_agent_directory = ssh_agent_directory.join(LEGACY_SOCKFILE_NAME); + + Ok(ssh_agent_directory) +} + +// use the platform's base dir +fn get_default_socket_path(mut path: PathBuf) -> PathBuf { + path.push("bitwarden"); + path.push(SOCKFILE_NAME); + path +} + +fn get_socket_basedir() -> Result { + // currently the only delineation between mac and linux is this logic. + // if we need to expand that, we can make unix.rs the platform agnostic shared logic, + // and separate out into linux.rs and macos.rs + cfg_if::cfg_if! { + if #[cfg(target_os = "linux")] { + std::env::var("XDG_RUNTIME_DIR") + .map(|path| PathBuf::from(path)) + .map_err(|_| { anyhow!("$XDG_RUNTIME_DIR is not defined.") }) + + } else if #[cfg(target_os = "macos")]{ + Ok(std::env::temp_dir()) + + } else { + anyhow!("Unsupported platform"); + } + } +} + +fn set_user_permissions(path: &PathBuf) -> Result<(), anyhow::Error> { + fs::set_permissions(path, fs::Permissions::from_mode(0o600)) + .map_err(|e| anyhow!("Could not set socket permissions for {path:?}: {e}")) +} + +// try to remove the given path if it exists +fn remove_path(path: &PathBuf) -> Result<(), anyhow::Error> { + if let Ok(true) = std::fs::exists(path) { + std::fs::remove_file(path).map_err(|e| anyhow!("Error removing socket {path:?}: {e}"))?; + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use rand::{distr::Alphanumeric, Rng}; + + use super::*; + + #[test] + fn test_legacy_default_socket_path_success() { + let path = get_legacy_default_socket_path().unwrap(); + let expected = PathBuf::from_iter([ + std::env::var("HOME").unwrap(), + ".bitwarden-ssh-agent.sock".to_string(), + ]); + assert_eq!(path, expected); + } + + #[test] + fn test_default_socket_path_success() { + let mut expected = PathBuf::from("/"); + expected.push("bitwarden"); + expected.push("ssh-agent.sock"); + + let path = get_default_socket_path(PathBuf::from("/")); + + assert_eq!(path, expected); + } + + fn rand_file_in_temp() -> PathBuf { + let mut path = std::env::temp_dir(); + let s: String = rand::rng() + .sample_iter(&Alphanumeric) + .take(16) + .map(char::from) + .collect(); + path.push(s); + path + } + + #[test] + fn test_remove_path_exists_success() { + let path = rand_file_in_temp(); + fs::write(&path, "").unwrap(); + remove_path(&path).unwrap(); + + assert!(!fs::exists(&path).unwrap()); + } + + #[test] + fn test_remove_path_not_found_success() { + let path = rand_file_in_temp(); + remove_path(&path).unwrap(); + + assert!(!fs::exists(&path).unwrap()); + } + + #[test] + fn test_sock_path_file_permissions() { + let path = rand_file_in_temp(); + fs::write(&path, "").unwrap(); + + set_user_permissions(&path).unwrap(); + + let metadata = fs::metadata(&path).unwrap(); + let permissions = metadata.permissions().mode(); + + assert_eq!(permissions, 0o100_600); + + remove_path(&path).unwrap(); } } diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/windows.rs b/apps/desktop/desktop_native/core/src/ssh_agent/windows.rs index 75c47165960..662a4658ede 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/windows.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/windows.rs @@ -1,32 +1,18 @@ use bitwarden_russh::ssh_agent; pub mod named_pipe_listener_stream; -use std::{ - collections::HashMap, - sync::{ - atomic::{AtomicBool, AtomicU32}, - Arc, RwLock, - }, -}; +use std::sync::Arc; use tokio::sync::Mutex; -use tokio_util::sync::CancellationToken; -use super::{BitwardenDesktopAgent, BitwardenSshKey, SshAgentUIRequest}; +use super::{BitwardenDesktopAgent, SshAgentUIRequest}; -impl BitwardenDesktopAgent { +impl BitwardenDesktopAgent { pub fn start_server( auth_request_tx: tokio::sync::mpsc::Sender, auth_response_rx: Arc>>, ) -> Result { - let agent_state = BitwardenDesktopAgent { - keystore: ssh_agent::KeyStore(Arc::new(RwLock::new(HashMap::new()))), - show_ui_request_tx: auth_request_tx, - get_ui_response_rx: auth_response_rx, - cancellation_token: CancellationToken::new(), - request_id: Arc::new(AtomicU32::new(0)), - needs_unlock: Arc::new(AtomicBool::new(true)), - is_running: Arc::new(AtomicBool::new(true)), - }; + let agent_state = BitwardenDesktopAgent::new(auth_request_tx, auth_response_rx); + let stream = named_pipe_listener_stream::NamedPipeServerStream::new( agent_state.cancellation_token.clone(), agent_state.is_running.clone(), diff --git a/apps/desktop/desktop_native/napi/src/lib.rs b/apps/desktop/desktop_native/napi/src/lib.rs index 24d41bc3831..8a70a99b9cd 100644 --- a/apps/desktop/desktop_native/napi/src/lib.rs +++ b/apps/desktop/desktop_native/napi/src/lib.rs @@ -169,7 +169,6 @@ pub mod clipboards { pub mod sshagent { use std::sync::Arc; - use desktop_core::ssh_agent::BitwardenSshKey; use napi::{ bindgen_prelude::Promise, threadsafe_function::{ErrorStrategy::CalleeHandled, ThreadsafeFunction}, @@ -178,7 +177,7 @@ pub mod sshagent { #[napi] pub struct SshAgentState { - state: desktop_core::ssh_agent::BitwardenDesktopAgent, + state: desktop_core::ssh_agent::BitwardenDesktopAgent, } #[napi(object)]