From 4c2c199c1334387a8ca131aa00586fc1102a2b35 Mon Sep 17 00:00:00 2001 From: Hureru <3507039083@qq.com> Date: Sat, 6 Jun 2026 21:17:41 +0800 Subject: [PATCH 1/3] Add multi-device shared storage mode --- config.toml.example | 22 + crates/sshwarden-agent/src/control.rs | 11 +- crates/sshwarden-config/src/lib.rs | 182 ++++++- crates/sshwarden-config/src/session.rs | 49 +- crates/sshwarden-config/src/ssh_config.rs | 33 +- crates/sshwarden-config/src/unlock_slots.rs | 98 ++++ ...vice-shared-storage-with-device-runtime.md | 92 ++++ src/main.rs | 497 +++++++++++++++--- 8 files changed, 863 insertions(+), 121 deletions(-) create mode 100644 crates/sshwarden-config/src/unlock_slots.rs create mode 100644 docs/adr/0024-multi-device-shared-storage-with-device-runtime.md diff --git a/config.toml.example b/config.toml.example index ef3bbe3..203c144 100644 --- a/config.toml.example +++ b/config.toml.example @@ -129,6 +129,28 @@ auto_unlock_on_request = true portable = false # portable_dir = "./sshwarden-data" +# 多设备共享模式（适合把 SSHWarden 放在 OneDrive/Dropbox 等同步目录中）： +# - 共享：config.toml、local-key-cache.json、bindings.json、keys/、sshwarden_config +# - 每设备独立：session、pid、log、runtime socket、Windows Hello/native unlock slot +# 这样两台机器可以共享同一份 Bitwarden SSH key 本地投影和 Host 绑定， +# 但不会互相覆盖运行态文件。 +# 若启用，建议同时设置 [ssh_config].path_style = "home_relative"，避免 +# C:\Users\zheng 与 C:\Users\Administrator 这样的用户名差异写入绝对路径。 +multi_device = false +# device_id = "auto" # 默认 auto；也可用 SSHWARDEN_DEVICE_ID 覆盖 + +# ============================================ +# SSH config / Host binding 生成配置 +# ============================================ +[ssh_config] +# 托管 snippet 路径。留空时：普通模式默认 exe 同目录；multi_device=true 时默认共享目录下的 sshwarden_config。 +# managed_path = "sshwarden_config" + +# 写入 ~/.ssh/config Include 和托管 snippet 中 IdentityFile 的路径风格： +# - "absolute": 写绝对路径（默认） +# - "home_relative": 对用户 home 下路径写成 ~/...，适合 OneDrive 跨 Windows 用户名共享 +path_style = "absolute" + # ============================================ # Socket 配置（高级） # ============================================ diff --git a/crates/sshwarden-agent/src/control.rs b/crates/sshwarden-agent/src/control.rs index 5dac8d3..3d11858 100644 --- a/crates/sshwarden-agent/src/control.rs +++ b/crates/sshwarden-agent/src/control.rs @@ -234,7 +234,13 @@ pub enum ControlAction { SetPin { pin: String, }, - Forget, + Forget { + /// In multi-device storage mode, also remove shared remembered-secret + /// cache files (`local-key-cache.json` and legacy `vault.enc`). Plain + /// `forget` is device-only there; on legacy storage this flag is + /// effectively always true for backwards compatibility. + shared: bool, + }, /// Cleanly shut down the daemon: cancel the main loop, stop the agent and /// control server, and remove the PID file. Used by `stop` / `restart`. Stop, @@ -360,7 +366,8 @@ async fn dispatch_control_command( "status" => ControlAction::Status { json: false }, "status-json" => ControlAction::Status { json: true }, "sync" => ControlAction::Sync, - "forget" => ControlAction::Forget, + "forget" => ControlAction::Forget { shared: false }, + "forget-shared" => ControlAction::Forget { shared: true }, "stop" => ControlAction::Stop, "bind-hosts-dialog" => ControlAction::BindHostsDialog, s if s.starts_with("unlock-pin:") => { diff --git a/crates/sshwarden-config/src/lib.rs b/crates/sshwarden-config/src/lib.rs index f8ab2d0..238a637 100644 --- a/crates/sshwarden-config/src/lib.rs +++ b/crates/sshwarden-config/src/lib.rs @@ -2,6 +2,7 @@ pub mod bindings; pub mod cache; pub mod session; pub mod ssh_config; +pub mod unlock_slots; pub mod vault; use std::path::PathBuf; @@ -10,12 +11,18 @@ use std::sync::OnceLock; use anyhow::Context; use serde::{Deserialize, Serialize}; -/// Cached resolution of the data directory, populated on first call to [`config_dir`]. +/// Cached resolution of the shared storage root, populated on first call to +/// [`config_dir`] / [`shared_data_dir`]. /// /// All callers share a single resolution outcome for the lifetime of the process — /// this prevents flapping when, for example, a portable probe file is written /// after some path has already been computed. -static RESOLVED_DIR: OnceLock = OnceLock::new(); +static RESOLVED_SHARED_DIR: OnceLock = OnceLock::new(); + +/// Cached resolution of the current device's data directory. In multi-device +/// mode this is `{shared_data_dir}/devices/{device-id}`; otherwise it is the +/// shared directory for backwards compatibility. +static RESOLVED_DEVICE_DIR: OnceLock = OnceLock::new(); #[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct Config { @@ -243,15 +250,31 @@ pub struct SocketConfig { pub path: Option, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] +#[serde(rename_all = "snake_case")] +pub enum SshConfigPathStyle { + /// Always write absolute paths into generated OpenSSH config. + #[default] + Absolute, + /// Prefer `~/...` for paths under the current user's home directory. This + /// lets one shared OneDrive-managed snippet work across Windows accounts + /// whose paths differ only by `C:\\Users\\`. + HomeRelative, +} + #[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct SshConfigConfig { /// Optional path for SSHWarden's generated OpenSSH Include snippet. /// /// Supports `~`, `~/...`, and `~\\...`. Relative paths are resolved under - /// SSHWarden's config directory. The default is `sshwarden_config` beside - /// the running executable, keeping this generated file out of the user's - /// device-wide `.ssh` directory unless explicitly configured otherwise. + /// SSHWarden's shared data directory. The default is `sshwarden_config` + /// in the shared data directory when multi-device mode is enabled, otherwise + /// beside the running executable for backwards compatibility. pub managed_path: Option, + /// Formatting style for paths written into `~/.ssh/config` and the managed + /// snippet. `home_relative` is recommended for OneDrive multi-device use. + #[serde(default)] + pub path_style: SshConfigPathStyle, } #[derive(Debug, Clone, Default, Serialize, Deserialize)] @@ -261,6 +284,23 @@ pub struct StorageConfig { pub portable: bool, /// Optional explicit portable directory. Used only when portable is true. pub portable_dir: Option, + /// Enable shared portable data with per-device runtime/session state. + /// + /// In this mode `config.toml`, `local-key-cache.json`, `bindings.json`, + /// `keys/`, and (by default) `sshwarden_config` remain in the shared data + /// directory, while `session.enc`, `sshwarden.pid`, `sshwarden.log`, and + /// runtime sockets are stored under `devices//`. + #[serde(default)] + pub multi_device: bool, + /// Explicit per-device directory name. Leave empty or set to `auto` to use + /// a stable ID derived from host/user information; can also be overridden by + /// `SSHWARDEN_DEVICE_ID`. + #[serde(default = "default_device_id")] + pub device_id: String, +} + +fn default_device_id() -> String { + "auto".to_string() } impl Config { @@ -290,7 +330,7 @@ impl Config { } } -/// Get the base directory for persistent SSHWarden configuration/data files. +/// Get the shared base directory for persistent SSHWarden configuration/data. /// /// Resolution priority (highest first): /// 1. `SSHWARDEN_HOME=` environment variable (explicit override). @@ -300,21 +340,60 @@ impl Config { /// executable's directory. /// 4. Platform-standard config directory (e.g. `%APPDATA%\SSHWarden` on Windows). /// -/// The resolution is cached in [`RESOLVED_DIR`] on first call, so subsequent calls -/// from any module return the same path even if the probe file is added or removed -/// mid-run. -pub fn config_dir() -> anyhow::Result { - if let Some(dir) = RESOLVED_DIR.get() { +/// In multi-device mode this is the OneDrive-synced shared directory containing +/// `config.toml`, `local-key-cache.json`, `bindings.json`, and `keys/`. +pub fn shared_data_dir() -> anyhow::Result { + if let Some(dir) = RESOLVED_SHARED_DIR.get() { return Ok(dir.clone()); } - let resolved = resolve_data_dir()?; + let resolved = resolve_shared_data_dir()?; // Ignore the race-loser case where another caller populated the cache first; // both callers would have computed the same value. - let _ = RESOLVED_DIR.set(resolved.clone()); + let _ = RESOLVED_SHARED_DIR.set(resolved.clone()); + Ok(resolved) +} + +/// Backwards-compatible name for the shared data directory. +/// +/// New code that stores runtime/session state should use [`device_data_dir`] +/// instead. Shared Bitwarden-projection files continue to use this directory. +pub fn config_dir() -> anyhow::Result { + shared_data_dir() +} + +/// Current device's private data directory. +/// +/// When `[storage] multi_device = true`, runtime/session/native-unlock files are +/// stored under `{shared_data_dir}/devices/{device-id}` while vault projection +/// files remain shared. Without multi-device mode this returns the shared data +/// directory for full backwards compatibility. +pub fn device_data_dir() -> anyhow::Result { + if let Some(dir) = RESOLVED_DEVICE_DIR.get() { + return Ok(dir.clone()); + } + let shared = shared_data_dir()?; + let config = Config::load()?; + let resolved = if config.storage.multi_device { + shared + .join("devices") + .join(current_device_id_from_config(&config)?) + } else { + shared + }; + let _ = RESOLVED_DEVICE_DIR.set(resolved.clone()); Ok(resolved) } -fn resolve_data_dir() -> anyhow::Result { +pub fn current_device_id() -> anyhow::Result { + let config = Config::load()?; + current_device_id_from_config(&config) +} + +pub fn multi_device_enabled() -> anyhow::Result { + Ok(Config::load()?.storage.multi_device) +} + +fn resolve_shared_data_dir() -> anyhow::Result { if let Some(dir) = env_path("SSHWARDEN_HOME") { return Ok(dir); } @@ -377,7 +456,7 @@ fn probe_portable_from_exe() -> Option { } pub fn config_path() -> anyhow::Result { - Ok(config_dir()?.join("config.toml")) + Ok(shared_data_dir()?.join("config.toml")) } pub fn runtime_dir() -> anyhow::Result { @@ -387,7 +466,7 @@ pub fn runtime_dir() -> anyhow::Result { #[cfg(windows)] { - Ok(config_dir()?.join("run")) + Ok(device_data_dir()?.join("run")) } #[cfg(target_os = "linux")] @@ -395,17 +474,17 @@ pub fn runtime_dir() -> anyhow::Result { if let Some(dir) = env_path("XDG_RUNTIME_DIR") { return Ok(dir.join("sshwarden")); } - Ok(config_dir()?.join("run")) + Ok(device_data_dir()?.join("run")) } #[cfg(target_os = "macos")] { - Ok(config_dir()?.join("run")) + Ok(device_data_dir()?.join("run")) } #[cfg(all(not(windows), not(target_os = "linux"), not(target_os = "macos")))] { - Ok(config_dir()?.join("run")) + Ok(device_data_dir()?.join("run")) } } @@ -437,6 +516,7 @@ pub fn managed_ssh_config_path(config: &Config) -> anyhow::Result { match config.ssh_config.managed_path.as_deref().map(str::trim) { Some("") => anyhow::bail!("ssh_config.managed_path is present but empty"), Some(path) => expand_config_path(path), + None if config.storage.multi_device => Ok(shared_data_dir()?.join("sshwarden_config")), None => default_managed_ssh_config_path(), } } @@ -460,7 +540,7 @@ pub fn expand_config_path(path: &str) -> anyhow::Result { if expanded.is_absolute() { Ok(expanded) } else { - Ok(config_dir()?.join(expanded)) + Ok(shared_data_dir()?.join(expanded)) } } @@ -495,6 +575,68 @@ fn home_dir_any() -> anyhow::Result { .context("HOME or USERPROFILE environment variable not set") } +fn current_device_id_from_config(config: &Config) -> anyhow::Result { + if let Some(id) = std::env::var_os("SSHWARDEN_DEVICE_ID") + .and_then(|value| value.into_string().ok()) + .map(|value| value.trim().to_string()) + .filter(|value| !value.is_empty()) + { + return Ok(sanitize_device_id(&id)); + } + + let configured = config.storage.device_id.trim(); + if !configured.is_empty() && !configured.eq_ignore_ascii_case("auto") { + return Ok(sanitize_device_id(configured)); + } + + Ok(auto_device_id()) +} + +fn auto_device_id() -> String { + let host = hostname_for_device_id(); + let user = std::env::var("USERNAME") + .or_else(|_| std::env::var("USER")) + .unwrap_or_else(|_| "user".to_string()); + sanitize_device_id(&format!("{host}-{user}")) +} + +fn hostname_for_device_id() -> String { + std::env::var("COMPUTERNAME") + .or_else(|_| std::env::var("HOSTNAME")) + .ok() + .filter(|s| !s.trim().is_empty()) + .unwrap_or_else(|| { + std::process::Command::new("hostname") + .output() + .ok() + .and_then(|output| String::from_utf8(output.stdout).ok()) + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + .unwrap_or_else(|| "device".to_string()) + }) +} + +fn sanitize_device_id(value: &str) -> String { + let mut out = String::new(); + let mut last_dash = false; + for ch in value.trim().chars().flat_map(char::to_lowercase) { + let allowed = ch.is_ascii_alphanumeric() || ch == '_' || ch == '.'; + if allowed { + out.push(ch); + last_dash = false; + } else if !last_dash { + out.push('-'); + last_dash = true; + } + } + let sanitized = out.trim_matches('-'); + if sanitized.is_empty() { + "device".to_string() + } else { + sanitized.to_string() + } +} + fn env_path(name: &str) -> Option { std::env::var_os(name) .filter(|value| !value.is_empty()) diff --git a/crates/sshwarden-config/src/session.rs b/crates/sshwarden-config/src/session.rs index 1b066ac..6d1cd71 100644 --- a/crates/sshwarden-config/src/session.rs +++ b/crates/sshwarden-config/src/session.rs @@ -6,15 +6,17 @@ use std::os::unix::fs::PermissionsExt; use anyhow::Context; use serde::{Deserialize, Serialize}; -/// Device-specific session file stored alongside the executable. +/// Device-specific session file. /// -/// Each device gets its own session file (`session-{hostname}.enc`) so that -/// multiple machines sharing the same exe directory via OneDrive do not -/// interfere with each other. +/// In multi-device storage mode this lives under +/// `{shared_data_dir}/devices//session-{hostname}.enc` so two +/// OneDrive-synced machines do not overwrite each other's Bitwarden refresh +/// token/session state. Without multi-device mode it remains in the historical +/// shared data directory. /// -/// The session file stores an encrypted refresh token that allows the daemon -/// to restore a Bitwarden API session after a PIN/Hello unlock without -/// requiring the master password. +/// The session file stores an encrypted refresh token that allows the daemon to +/// restore a Bitwarden API session after a PIN/Hello unlock without requiring +/// the master password. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct SessionFile { /// File format version (currently 1). @@ -35,18 +37,35 @@ impl SessionFile { /// Supported on-disk format versions for the device session file. const SUPPORTED_VERSIONS: &'static [u32] = &[1]; - /// Path to the session file: `{config_dir}/session-{hostname}.enc` + /// Path to the current device's session file. pub fn path() -> anyhow::Result { + Ok(crate::device_data_dir()?.join(Self::file_name())) + } + + /// Historical path used before multi-device runtime/session state was + /// separated. Used as a best-effort read/delete fallback for migration. + fn legacy_path() -> anyhow::Result { + Ok(crate::shared_data_dir()?.join(Self::file_name())) + } + + fn file_name() -> String { let hostname = hostname(); - Ok(crate::config_dir()?.join(format!("session-{hostname}.enc"))) + format!("session-{hostname}.enc") } /// Load the session file from disk. Returns `None` if the file does not exist. pub fn load() -> anyhow::Result> { let path = Self::path()?; - if !path.exists() { - return Ok(None); - } + let path = if path.exists() { + path + } else { + let legacy = Self::legacy_path()?; + if legacy.exists() { + legacy + } else { + return Ok(None); + } + }; let content = std::fs::read_to_string(&path) .with_context(|| format!("Failed to read session file: {}", path.display()))?; let session: SessionFile = serde_json::from_str(&content) @@ -84,6 +103,12 @@ impl SessionFile { std::fs::remove_file(&path) .with_context(|| format!("Failed to delete session file: {}", path.display()))?; } + let legacy = Self::legacy_path()?; + if legacy.exists() && legacy != path { + std::fs::remove_file(&legacy).with_context(|| { + format!("Failed to delete legacy session file: {}", legacy.display()) + })?; + } Ok(()) } } diff --git a/crates/sshwarden-config/src/ssh_config.rs b/crates/sshwarden-config/src/ssh_config.rs index 2503323..060cf3b 100644 --- a/crates/sshwarden-config/src/ssh_config.rs +++ b/crates/sshwarden-config/src/ssh_config.rs @@ -1,4 +1,6 @@ -use std::path::Path; +use std::path::{Path, PathBuf}; + +use crate::SshConfigPathStyle; pub const SSHWARDEN_INCLUDE_MARKER: &str = "# SSHWarden managed key selector snippets"; @@ -28,10 +30,24 @@ pub fn path_arg(path: &Path) -> String { quote_ssh_config_arg(&path.to_string_lossy()) } +pub fn path_arg_with_style(path: &Path, style: SshConfigPathStyle) -> String { + let display_path = match style { + SshConfigPathStyle::Absolute => path.to_path_buf(), + SshConfigPathStyle::HomeRelative => { + home_relative_path(path).unwrap_or_else(|| path.to_path_buf()) + } + }; + quote_ssh_config_arg(&display_path.to_string_lossy()) +} + pub fn include_line(include_path: &Path) -> String { format!("Include {}", path_arg(include_path)) } +pub fn include_line_with_style(include_path: &Path, style: SshConfigPathStyle) -> String { + format!("Include {}", path_arg_with_style(include_path, style)) +} + pub fn legacy_unquoted_include_line(include_path: &Path) -> String { format!("Include {}", include_path.display()) } @@ -40,7 +56,9 @@ pub fn legacy_unquoted_include_line(include_path: &Path) -> String { /// written by earlier SSHWarden builds. pub fn line_matches_sshwarden_include(line: &str, include_path: &Path) -> bool { let trimmed = line.trim(); - trimmed == include_line(include_path) || trimmed == legacy_unquoted_include_line(include_path) + trimmed == include_line(include_path) + || trimmed == include_line_with_style(include_path, SshConfigPathStyle::HomeRelative) + || trimmed == legacy_unquoted_include_line(include_path) } pub fn ensure_ssh_dir_permissions(path: &Path) -> anyhow::Result<()> { @@ -84,6 +102,17 @@ fn normalize_ssh_config_path(value: &str) -> String { } } +fn home_relative_path(path: &Path) -> Option { + let home = std::env::var_os("HOME") + .or_else(|| std::env::var_os("USERPROFILE")) + .map(PathBuf::from)?; + let relative = path.strip_prefix(&home).ok()?; + if relative.as_os_str().is_empty() { + return Some(PathBuf::from("~")); + } + Some(PathBuf::from("~").join(relative)) +} + #[cfg(test)] #[allow(clippy::unwrap_used)] mod tests { diff --git a/crates/sshwarden-config/src/unlock_slots.rs b/crates/sshwarden-config/src/unlock_slots.rs new file mode 100644 index 0000000..89f071d --- /dev/null +++ b/crates/sshwarden-config/src/unlock_slots.rs @@ -0,0 +1,98 @@ +use std::path::{Path, PathBuf}; + +#[cfg(unix)] +use std::os::unix::fs::PermissionsExt; + +use anyhow::Context; +use serde::{Deserialize, Serialize}; + +/// Per-device platform unlock slots for a shared Local Key Cache. +/// +/// The shared `local-key-cache.json` keeps the encrypted key payload and the +/// shared PIN slot. Platform-native unlock material (Windows Hello / macOS +/// Keychain / Linux Secret Service) is device-specific and belongs here under +/// `{device_data_dir}/unlock-slots.json` so multiple OneDrive-synced machines do +/// not overwrite one another's native unlock slot. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct UnlockSlotsFile { + pub version: u32, + #[serde(skip_serializing_if = "Option::is_none")] + pub hello_challenge: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub hello_encrypted: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub native_encrypted: Option, +} + +impl Default for UnlockSlotsFile { + fn default() -> Self { + Self { + version: 1, + hello_challenge: None, + hello_encrypted: None, + native_encrypted: None, + } + } +} + +impl UnlockSlotsFile { + const SUPPORTED_VERSIONS: &'static [u32] = &[1]; + + pub fn path() -> anyhow::Result { + Ok(crate::device_data_dir()?.join("unlock-slots.json")) + } + + pub fn load() -> anyhow::Result> { + let path = Self::path()?; + if !path.exists() { + return Ok(None); + } + let content = std::fs::read_to_string(&path) + .with_context(|| format!("Failed to read unlock slots file: {}", path.display()))?; + let slots: UnlockSlotsFile = serde_json::from_str(&content) + .with_context(|| format!("Failed to parse unlock slots file: {}", path.display()))?; + if !Self::SUPPORTED_VERSIONS.contains(&slots.version) { + anyhow::bail!( + "Unsupported unlock slots version {} (supported: {:?}): {}", + slots.version, + Self::SUPPORTED_VERSIONS, + path.display() + ); + } + Ok(Some(slots)) + } + + pub fn save(&self) -> anyhow::Result<()> { + let path = Self::path()?; + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).with_context(|| { + format!( + "Failed to create unlock slots directory: {}", + parent.display() + ) + })?; + } + let content = + serde_json::to_string_pretty(self).context("Failed to serialize unlock slots")?; + write_owner_only_file(&path, content) + .with_context(|| format!("Failed to write unlock slots file: {}", path.display()))?; + Ok(()) + } + + pub fn delete() -> anyhow::Result<()> { + let path = Self::path()?; + if path.exists() { + std::fs::remove_file(&path).with_context(|| { + format!("Failed to delete unlock slots file: {}", path.display()) + })?; + } + Ok(()) + } +} + +fn write_owner_only_file(path: &Path, content: impl AsRef<[u8]>) -> anyhow::Result<()> { + std::fs::write(path, content)?; + #[cfg(unix)] + std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))?; + Ok(()) +} diff --git a/docs/adr/0024-multi-device-shared-storage-with-device-runtime.md b/docs/adr/0024-multi-device-shared-storage-with-device-runtime.md new file mode 100644 index 0000000..87c748c --- /dev/null +++ b/docs/adr/0024-multi-device-shared-storage-with-device-runtime.md @@ -0,0 +1,92 @@ +# Multi-device shared storage with device-local runtime state + +## Status + +Accepted + +## Context + +Some Windows users keep a portable SSHWarden installation under a cloud-synced directory such as OneDrive and use it from multiple machines. The Bitwarden vault remains the source of truth for SSH keys, but users may want the local Bitwarden projection (`local-key-cache.json`), host bindings, and public selector files to be shared across those machines so every device sees the same cached key identities and OpenSSH host routing rules. + +The previous portable layout stored all SSHWarden files in one directory. That causes conflicts when the same directory is synced across devices: `sshwarden.pid`, logs, runtime sockets, Bitwarden session files, and platform-native unlock slots are device-local state and must not be overwritten by another device. At the same time, `bindings.json` and `keys/` are safe and useful to share, and a shared `local-key-cache.json` is acceptable when the user intentionally uses the same PIN across devices. + +Windows OneDrive paths may differ only by username, for example: + +```text +C:\Users\zheng\OneDrive\Program\SSHWarden +C:\Users\Administrator\OneDrive\Program\SSHWarden +``` + +A shared OpenSSH snippet must therefore avoid writing absolute paths containing a specific username when the user chooses shared SSH config. + +## Decision + +SSHWarden supports an opt-in multi-device storage mode: + +```toml +[storage] +portable = true +multi_device = true +device_id = "auto" + +[ssh_config] +path_style = "home_relative" +``` + +In this mode SSHWarden splits storage into a shared data directory and a current-device data directory: + +```text +shared_data_dir/ + config.toml + local-key-cache.json + bindings.json + keys/ + sshwarden_config + +shared_data_dir/devices// + session-.enc + unlock-slots.json + sshwarden.pid + sshwarden.log + run/ +``` + +`config_dir()` remains the shared data directory for backward-compatible callers that manage Bitwarden projection files. New runtime/session callers use `device_data_dir()`. + +Shared files: + +- `config.toml` +- `local-key-cache.json` (encrypted shared Bitwarden SSH key projection and shared PIN slot) +- `bindings.json` +- `keys/` public selector files +- `sshwarden_config` by default in multi-device mode + +Device-local files: + +- Bitwarden device session file +- `sshwarden.pid` +- `sshwarden.log` +- runtime socket directory +- `unlock-slots.json` containing Windows Hello / native unlock material for the current device + +Platform-native unlock slots are not stored in the shared local key cache in multi-device mode. A startup migration copies older shared Hello/native slots into the current device's `unlock-slots.json`; future cache refreshes strip platform slots from the shared cache. + +When `[ssh_config].path_style = "home_relative"`, generated `Include` and `IdentityFile` arguments prefer `~/...` for paths under the current user's home directory. This allows one shared `sshwarden_config` to work across Windows accounts whose OneDrive roots differ only by `C:\Users\`. + +`sshwarden forget` changes semantics in multi-device mode: it forgets only the current device's session and native unlock material while preserving the shared local key cache. To remove the shared remembered-secret cache, the user must pass `sshwarden forget --shared-cache`. + +## Rationale + +- The Bitwarden vault remains the source of truth; the shared local key cache is an encrypted projection intentionally shared by the user. +- Host bindings and public selector files are local preferences but are not device secrets, and sharing them gives consistent OpenSSH key selection across machines. +- PID files, logs, runtime sockets, and refresh-token sessions are inherently device/process-local and should not be cloud-synced as one shared file. +- Windows Hello, macOS Keychain, and Linux Secret Service unlock material is device-local by construction; sharing it would cause devices to overwrite each other's platform unlock state. +- Home-relative OpenSSH paths avoid embedding a particular Windows username into shared snippets. +- Device-only `forget` avoids deleting a shared cache that other devices still use. + +## Consequences + +- Users who enable multi-device mode should use a strong PIN because the encrypted SSH key projection is intentionally stored in a cloud-synced directory. +- A shared `sshwarden_config` works best when the synced directory has the same path relative to `~` on every device. If not, users can still share `bindings.json` and `keys/` but should configure a device-local managed snippet path. +- If two devices update shared files concurrently, the cloud provider may create conflict copies. Running `sshwarden sync` again from a healthy unlocked device should regenerate the shared cache and selector files from Bitwarden. +- Legacy `vault.enc` remains supported for migration but is not the preferred shared cache format. diff --git a/src/main.rs b/src/main.rs index cc7b4a2..9fe5808 100644 --- a/src/main.rs +++ b/src/main.rs @@ -175,8 +175,12 @@ enum Commands { }, /// Sync keys from Bitwarden into the running agent. [needs running agent + Bitwarden] Sync, - /// Forget remembered device material (local cache, session, PIN). [needs running agent] - Forget, + /// Forget remembered device material. In multi-device mode this is device-only by default. [needs running agent] + Forget { + /// Also remove the shared Local Key Cache / legacy vault cache. + #[arg(long)] + shared_cache: bool, + }, /// Lock the vault (clear private keys from memory). [needs running agent] Lock, /// Unlock the vault. [needs running agent] @@ -436,7 +440,13 @@ fn main() -> anyhow::Result<()> { } Some(Commands::SetPin) => cmd_set_pin().await, Some(Commands::Sync) => cmd_control("sync").await, - Some(Commands::Forget) => cmd_control("forget").await, + Some(Commands::Forget { shared_cache }) => { + if shared_cache { + cmd_control("forget-shared").await + } else { + cmd_control("forget").await + } + } Some(Commands::Env { shell }) => cmd_env(&config, &shell), } }) @@ -1875,6 +1885,7 @@ fn ssh_config_snippet_with_bindings( keys: &[ManagedKey], bindings: &sshwarden_config::bindings::HostBindingsFile, ) -> anyhow::Result { + let path_style = sshwarden_config::Config::load()?.ssh_config.path_style; let mut lines = vec![ "# SSHWarden managed SSH config".to_string(), "# You may edit Host lines in SSHWarden key blocks; they are imported before regeneration." @@ -1904,7 +1915,7 @@ fn ssh_config_snippet_with_bindings( lines.push(format!("Host {}", binding.hosts.join(" "))); lines.push(format!( " IdentityFile {}", - sshwarden_config::ssh_config::path_arg(&path) + sshwarden_config::ssh_config::path_arg_with_style(&path, path_style) )); lines.push(" IdentitiesOnly yes".to_string()); lines.push(String::new()); @@ -1920,7 +1931,7 @@ fn ssh_config_snippet_with_bindings( lines.push("# Host ".to_string()); lines.push(format!( "# IdentityFile {}", - sshwarden_config::ssh_config::path_arg(&path) + sshwarden_config::ssh_config::path_arg_with_style(&path, path_style) )); lines.push("# IdentitiesOnly yes".to_string()); lines.push(String::new()); @@ -2098,11 +2109,16 @@ fn write_sshwarden_include_line( })?; } - let include_line = sshwarden_config::ssh_config::include_line(include_path); + let include_line = sshwarden_config::ssh_config::include_line_with_style( + include_path, + sshwarden_config::Config::load()?.ssh_config.path_style, + ); let existing = std::fs::read_to_string(config_path).unwrap_or_default(); - if existing.lines().any(|line| { - sshwarden_config::ssh_config::line_matches_sshwarden_include(line, include_path) - }) { + // Return only when the desired line is already present. A legacy absolute + // line may still "match" SSHWarden's include semantically, but in + // home-relative multi-device mode we want the marked block below to rewrite + // it so a synced ~/.ssh/config works across different Windows usernames. + if existing.lines().any(|line| line.trim() == include_line) { return Ok(()); } @@ -2110,6 +2126,7 @@ fn write_sshwarden_include_line( let mut rewritten: Vec = Vec::with_capacity(existing.lines().count() + 2); let mut marker_waiting_for_include = false; let mut replaced_marked_include = false; + let mut dropped_unmarked_include = false; for line in existing.lines() { let trimmed = line.trim(); @@ -2133,6 +2150,13 @@ fn write_sshwarden_include_line( } marker_waiting_for_include = false; } + if line_matches_current_or_legacy_sshwarden_include(trimmed, include_path) { + // Drop unmarked older SSHWarden Include lines. A fresh marked line + // with the desired path style is appended below if no marked block + // was rewritten. + dropped_unmarked_include = true; + continue; + } rewritten.push(line.to_string()); } @@ -2146,7 +2170,11 @@ fn write_sshwarden_include_line( return Ok(()); } - let mut new_config = existing; + let mut new_config = if dropped_unmarked_include { + rewritten.join("\n") + } else { + existing + }; if !new_config.is_empty() && !new_config.ends_with('\n') { new_config.push('\n'); } @@ -2380,31 +2408,44 @@ fn write_envelope_local_key_cache( sshwarden_api::crypto::SymmetricKey, )> { let local_cache_key = sshwarden_api::crypto::random_symmetric_key(); - let (pin_encrypted, pin_salt) = encrypt_local_cache_key_with_pin(&local_cache_key, pin)?; + write_envelope_local_key_cache_with_key(keys, email, server_url, pin, &local_cache_key) +} + +fn write_envelope_local_key_cache_with_key( + keys: &[(String, String, String)], + email: &str, + server_url: &str, + pin: &str, + local_cache_key: &sshwarden_api::crypto::SymmetricKey, +) -> anyhow::Result<( + sshwarden_config::cache::LocalKeyCacheFile, + sshwarden_api::crypto::SymmetricKey, +)> { + let (pin_encrypted, pin_salt) = encrypt_local_cache_key_with_pin(local_cache_key, pin)?; let mut cache = build_envelope_local_key_cache( keys, email, server_url, - &local_cache_key, + local_cache_key, Some(pin_encrypted), Some(pin_salt), None, None, None, )?; - if let Err(e) = enroll_native_for_local_key_cache(&mut cache, &local_cache_key) { + if let Err(e) = enroll_native_for_local_key_cache(&mut cache, local_cache_key) { tracing::debug!("Native unlock enrollment skipped: {}", e); } #[cfg(windows)] { if sshwarden_ui::unlock::hello_crypto::hello_available() { - if let Err(e) = enroll_hello_for_local_key_cache(&mut cache, &local_cache_key) { + if let Err(e) = enroll_hello_for_local_key_cache(&mut cache, local_cache_key) { tracing::warn!("Failed to enroll Windows Hello for local key cache: {}", e); } } } cache.save()?; - Ok((cache, local_cache_key)) + Ok((cache, local_cache_key.clone())) } fn refresh_envelope_local_key_cache( @@ -2412,6 +2453,7 @@ fn refresh_envelope_local_key_cache( existing: &sshwarden_config::cache::LocalKeyCacheFile, local_cache_key: &sshwarden_api::crypto::SymmetricKey, ) -> anyhow::Result { + let keep_platform_slots_in_shared_cache = !multi_device_mode(); let cache = build_envelope_local_key_cache( keys, &existing.header.email, @@ -2419,14 +2461,120 @@ fn refresh_envelope_local_key_cache( local_cache_key, existing.local_cache_key.pin_encrypted.clone(), existing.local_cache_key.pin_salt.clone(), - existing.local_cache_key.hello_challenge.clone(), - existing.local_cache_key.hello_encrypted.clone(), - existing.local_cache_key.native_encrypted.clone(), + keep_platform_slots_in_shared_cache + .then(|| existing.local_cache_key.hello_challenge.clone()) + .flatten(), + keep_platform_slots_in_shared_cache + .then(|| existing.local_cache_key.hello_encrypted.clone()) + .flatten(), + keep_platform_slots_in_shared_cache + .then(|| existing.local_cache_key.native_encrypted.clone()) + .flatten(), )?; cache.save()?; Ok(cache) } +fn multi_device_mode() -> bool { + sshwarden_config::multi_device_enabled().unwrap_or(false) +} + +fn load_device_unlock_slots() -> Option { + sshwarden_config::unlock_slots::UnlockSlotsFile::load() + .ok() + .flatten() +} + +fn current_native_unlock_slot( + cache: &sshwarden_config::cache::LocalKeyCacheFile, +) -> Option { + let device_slot = load_device_unlock_slots().and_then(|slots| slots.native_encrypted); + if multi_device_mode() { + device_slot + } else { + device_slot.or_else(|| cache.local_cache_key.native_encrypted.clone()) + } +} + +fn persist_device_unlock_slots_from_cache( + cache: &sshwarden_config::cache::LocalKeyCacheFile, +) -> anyhow::Result<()> { + if !multi_device_mode() { + return Ok(()); + } + + let mut slots = load_device_unlock_slots().unwrap_or_default(); + let mut changed = false; + if slots.native_encrypted.is_none() && cache.local_cache_key.native_encrypted.is_some() { + slots.native_encrypted = cache.local_cache_key.native_encrypted.clone(); + changed = true; + } + if slots.hello_challenge.is_none() && cache.local_cache_key.hello_challenge.is_some() { + slots.hello_challenge = cache.local_cache_key.hello_challenge.clone(); + changed = true; + } + if slots.hello_encrypted.is_none() && cache.local_cache_key.hello_encrypted.is_some() { + slots.hello_encrypted = cache.local_cache_key.hello_encrypted.clone(); + changed = true; + } + if changed { + slots.save()?; + } + Ok(()) +} + +async fn reload_shared_local_cache_from_disk_if_multi_device( + local_key_cache_data: &Arc>>, +) { + if !multi_device_mode() { + return; + } + match sshwarden_config::cache::LocalKeyCacheFile::load() { + Ok(cache) => { + if let Some(ref cache) = cache { + if let Err(e) = persist_device_unlock_slots_from_cache(cache) { + tracing::warn!("Failed to persist device unlock slots: {}", e); + } + } + *local_key_cache_data.write().await = cache; + } + Err(e) => tracing::warn!("Failed to reload shared local key cache: {}", e), + } +} + +#[cfg(windows)] +fn current_hello_unlock_info( + cache: &sshwarden_config::cache::LocalKeyCacheFile, +) -> Option<(String, String)> { + let device_slot = load_device_unlock_slots() + .and_then(|slots| Some((slots.hello_challenge?, slots.hello_encrypted?))); + if multi_device_mode() { + device_slot + } else { + device_slot.or_else(|| { + Some(( + cache.local_cache_key.hello_challenge.clone()?, + cache.local_cache_key.hello_encrypted.clone()?, + )) + }) + } +} + +#[cfg(windows)] +fn current_hello_challenge_b64() -> Option { + let device_challenge = load_device_unlock_slots().and_then(|slots| slots.hello_challenge); + if multi_device_mode() { + device_challenge + } else { + device_challenge.or_else(|| { + sshwarden_config::vault::VaultFile::load() + .ok() + .flatten() + .and_then(|vault| vault.hello_challenge) + }) + } +} + fn enroll_native_for_local_key_cache( cache: &mut sshwarden_config::cache::LocalKeyCacheFile, local_cache_key: &sshwarden_api::crypto::SymmetricKey, @@ -2437,7 +2585,14 @@ fn enroll_native_for_local_key_cache( let encoded_local_cache_key = sshwarden_api::crypto::encode_symmetric_key(local_cache_key); let native_slot = sshwarden_ui::unlock::native::native_encrypt_local_cache_key(&encoded_local_cache_key)?; - cache.local_cache_key.native_encrypted = Some(native_slot); + if multi_device_mode() { + let mut slots = load_device_unlock_slots().unwrap_or_default(); + slots.native_encrypted = Some(native_slot); + slots.save()?; + cache.local_cache_key.native_encrypted = None; + } else { + cache.local_cache_key.native_encrypted = Some(native_slot); + } Ok(()) } @@ -2448,9 +2603,18 @@ fn enroll_hello_for_local_key_cache( ) -> anyhow::Result<()> { let challenge: [u8; 16] = rand::random(); let hello_encrypted = encrypt_local_cache_key_with_hello(local_cache_key, &challenge)?; - cache.local_cache_key.hello_challenge = - Some(base64::engine::general_purpose::STANDARD.encode(challenge)); - cache.local_cache_key.hello_encrypted = Some(hello_encrypted); + let hello_challenge = base64::engine::general_purpose::STANDARD.encode(challenge); + if multi_device_mode() { + let mut slots = load_device_unlock_slots().unwrap_or_default(); + slots.hello_challenge = Some(hello_challenge); + slots.hello_encrypted = Some(hello_encrypted); + slots.save()?; + cache.local_cache_key.hello_challenge = None; + cache.local_cache_key.hello_encrypted = None; + } else { + cache.local_cache_key.hello_challenge = Some(hello_challenge); + cache.local_cache_key.hello_encrypted = Some(hello_encrypted); + } Ok(()) } @@ -2493,12 +2657,9 @@ fn decrypt_envelope_payload( fn decrypt_envelope_local_key_cache_with_native( cache: &sshwarden_config::cache::LocalKeyCacheFile, ) -> anyhow::Result<(String, sshwarden_api::crypto::SymmetricKey)> { - let native_slot = cache - .local_cache_key - .native_encrypted - .as_deref() - .context("Local key cache has no native unlock slot")?; - let encoded_lck = sshwarden_ui::unlock::native::native_decrypt_local_cache_key(native_slot) + let native_slot = + current_native_unlock_slot(cache).context("Local key cache has no native unlock slot")?; + let encoded_lck = sshwarden_ui::unlock::native::native_decrypt_local_cache_key(&native_slot) .context("Failed to unlock Local Cache Key with native unlock")?; let local_cache_key = sshwarden_api::crypto::decode_symmetric_key(&encoded_lck) .context("Failed to decode Local Cache Key")?; @@ -2588,25 +2749,17 @@ fn migrate_pin_salt_to_v3( fn decrypt_envelope_local_key_cache_with_hello( cache: &sshwarden_config::cache::LocalKeyCacheFile, ) -> anyhow::Result<(String, sshwarden_api::crypto::SymmetricKey)> { - let challenge_b64 = cache - .local_cache_key - .hello_challenge - .as_deref() - .context("Local key cache has no Windows Hello challenge")?; - let hello_encrypted = cache - .local_cache_key - .hello_encrypted - .as_deref() - .context("Local key cache has no Windows Hello unlock slot")?; + let (challenge_b64, hello_encrypted) = current_hello_unlock_info(cache) + .context("Local key cache has no Windows Hello unlock slot for this device")?; let challenge_bytes = base64::engine::general_purpose::STANDARD - .decode(challenge_b64) + .decode(&challenge_b64) .context("Failed to decode Windows Hello challenge")?; if challenge_bytes.len() != 16 { anyhow::bail!("Invalid Windows Hello challenge length"); } let mut challenge = [0u8; 16]; challenge.copy_from_slice(&challenge_bytes); - let encoded_lck = try_hello_unlock(&challenge, hello_encrypted) + let encoded_lck = try_hello_unlock(&challenge, &hello_encrypted) .context("Failed to unlock Local Cache Key with Windows Hello")?; let local_cache_key = sshwarden_api::crypto::decode_symmetric_key(&encoded_lck) .context("Failed to decode Local Cache Key")?; @@ -3025,10 +3178,21 @@ async fn run_foreground( info!("Server: {}", config.server.base_url); // Check for persisted cache/vault files BEFORE prompting for master password. - let local_key_cache = sshwarden_config::cache::LocalKeyCacheFile::load().unwrap_or_else(|e| { - tracing::warn!("Failed to load local key cache: {}", e); - None - }); + let local_key_cache = sshwarden_config::cache::LocalKeyCacheFile::load() + .unwrap_or_else(|e| { + tracing::warn!("Failed to load local key cache: {}", e); + None + }) + .map(|cache| { + // Multi-device migration aid: if an older shared cache still carries + // platform unlock slots, copy them into this device's private + // unlock-slots file. Future cache writes strip platform slots from + // the shared file so devices stop overwriting each other. + if let Err(e) = persist_device_unlock_slots_from_cache(&cache) { + tracing::warn!("Failed to persist device unlock slots: {}", e); + } + cache + }); let vault_file = sshwarden_config::vault::VaultFile::load().unwrap_or_else(|e| { tracing::warn!("Failed to load vault file: {}", e); None @@ -3678,11 +3842,24 @@ async fn build_status_response( let authenticated = api_client.read().await.is_some(); let pending = pending_sync.load(std::sync::atomic::Ordering::Relaxed); let notification = notification_state.read().await.clone(); - // P1-3: surface the resolved data directory so users can tell where their - // secrets actually live (docs historically disagreed on this). - let data_dir = sshwarden_config::config_dir() + // P1-3 / multi-device: surface both the shared Bitwarden-projection + // directory and this device's runtime/session directory so users can tell + // exactly which OneDrive files are shared and which are device-local. + let shared_data_dir = sshwarden_config::shared_data_dir() .map(|p| p.display().to_string()) .unwrap_or_else(|_| "".to_string()); + let device_data_dir = sshwarden_config::device_data_dir() + .map(|p| p.display().to_string()) + .unwrap_or_else(|_| "".to_string()); + let device_id = + sshwarden_config::current_device_id().unwrap_or_else(|_| "".to_string()); + let multi_device = sshwarden_config::multi_device_enabled().unwrap_or(false); + let ssh_config_path_style = sshwarden_config::Config::load() + .map(|config| match config.ssh_config.path_style { + sshwarden_config::SshConfigPathStyle::Absolute => "absolute", + sshwarden_config::SshConfigPathStyle::HomeRelative => "home_relative", + }) + .unwrap_or("absolute"); let details = serde_json::json!({ "locked": locked, @@ -3695,7 +3872,12 @@ async fn build_status_response( "legacy_migration_available": has_vault && !has_local_key_cache, "authenticated": authenticated, "pending_sync": pending, - "data_dir": data_dir, + "data_dir": device_data_dir, + "shared_data_dir": shared_data_dir, + "device_data_dir": device_data_dir, + "device_id": device_id, + "multi_device": multi_device, + "ssh_config_path_style": ssh_config_path_style, "notification": notification.to_json(), }); @@ -3726,6 +3908,9 @@ async fn build_status_response( if authenticated { extras.push("API session restored"); } + if multi_device { + extras.push("multi-device storage"); + } if pending { extras.push("pending sync"); } @@ -3774,6 +3959,17 @@ async fn handle_control_command( ) -> sshwarden_agent::ControlResponse { use sshwarden_agent::ControlAction; + if matches!( + &action, + ControlAction::Unlock + | ControlAction::UnlockNative + | ControlAction::UnlockHello + | ControlAction::UnlockPin { .. } + | ControlAction::Sync + ) { + reload_shared_local_cache_from_disk_if_multi_device(local_key_cache_data).await; + } + match action { // `Stop` is intercepted by the run_foreground select! loop (it breaks out // to run the cancel-token cleanup); this arm is only defensive. @@ -4593,6 +4789,22 @@ async fn handle_control_command( // Report that honestly and mark a pending sync so the next unlock // applies the keys, instead of claiming the running agent was updated. let was_locked = vault_locked.load(std::sync::atomic::Ordering::Relaxed); + if !was_locked { + // If this unlocked session was restored without keeping the Local + // Cache Key in memory, a plain sync would update the live agent but + // leave `sshwarden keys` showing the old cache header. Prompt for + // PIN first so the synced key set can be persisted atomically. + if let Err(e) = ensure_local_cache_key_for_manual_sync( + local_key_cache_data, + local_cache_key_state, + ui_request_tx, + pin_failures, + ) + .await + { + return sshwarden_agent::ControlResponse::err(&e); + } + } match do_sync( api_client, cached_key_tuples, @@ -4621,42 +4833,47 @@ async fn handle_control_command( Err(e) => sshwarden_agent::ControlResponse::err(&e), } } - ControlAction::Forget => { + ControlAction::Forget { shared } => { // EH-06: accumulate failures to delete on-disk material so a // revocation that left secrets on disk is not reported as success. let mut failures: Vec = Vec::new(); - if let Err(e) = sshwarden_config::cache::LocalKeyCacheFile::delete() { - tracing::warn!("Failed to delete local key cache: {}", e); - failures.push(format!("local key cache ({e})")); - } - if let Err(e) = sshwarden_config::vault::VaultFile::delete() { - tracing::warn!("Failed to delete legacy vault file: {}", e); - failures.push(format!("legacy vault file ({e})")); + let multi_device = multi_device_mode(); + let remove_shared_cache = shared || !multi_device; + + if remove_shared_cache { + if let Err(e) = sshwarden_config::cache::LocalKeyCacheFile::delete() { + tracing::warn!("Failed to delete local key cache: {}", e); + failures.push(format!("local key cache ({e})")); + } + if let Err(e) = sshwarden_config::vault::VaultFile::delete() { + tracing::warn!("Failed to delete legacy vault file: {}", e); + failures.push(format!("legacy vault file ({e})")); + } } + let native_slot = local_key_cache_data .read() .await .as_ref() - .and_then(|cache| cache.local_cache_key.native_encrypted.clone()); + .and_then(current_native_unlock_slot); if let Err(e) = sshwarden_ui::unlock::native::native_delete_local_cache_key(native_slot.as_deref()) { tracing::warn!("Failed to delete native unlock material: {}", e); failures.push(format!("native unlock material ({e})")); } + if let Err(e) = sshwarden_config::unlock_slots::UnlockSlotsFile::delete() { + tracing::warn!("Failed to delete device unlock slots file: {}", e); + failures.push(format!("device unlock slots file ({e})")); + } if let Err(e) = sshwarden_config::session::SessionFile::delete() { tracing::warn!("Failed to delete device session file: {}", e); failures.push(format!("device session file ({e})")); } - *local_key_cache_data.write().await = None; - *vault_file_data.write().await = None; - *pin_encrypted_keys.write().await = None; local_cache_key_state.write().await.clear(); authorization_memory.write().await.clear(); cached_key_tuples.write().await.clear(); - public_key_identity_tuples.write().await.clear(); - key_names.write().await.clear(); *api_client.write().await = None; pending_sync.store(false, std::sync::atomic::Ordering::Relaxed); { @@ -4668,13 +4885,59 @@ async fn handle_control_command( client.stop(); } *notification_rx = None; - let _ = agent.clear_keys(); vault_locked.store(true, std::sync::atomic::Ordering::Relaxed); + if remove_shared_cache { + *local_key_cache_data.write().await = None; + *vault_file_data.write().await = None; + *pin_encrypted_keys.write().await = None; + public_key_identity_tuples.write().await.clear(); + key_names.write().await.clear(); + let _ = agent.clear_keys(); + } else { + // Multi-device plain `forget` is device-only: remove this + // device's API session/native unlock material, but keep the + // shared key cache listable and PIN-unlockable for all devices. + let cache = sshwarden_config::cache::LocalKeyCacheFile::load() + .ok() + .flatten(); + *local_key_cache_data.write().await = cache.clone(); + *vault_file_data.write().await = + sshwarden_config::vault::VaultFile::load().ok().flatten(); + *pin_encrypted_keys.write().await = vault_file_data + .read() + .await + .as_ref() + .map(|v| v.pin_encrypted.clone()); + public_key_identity_tuples.write().await.clear(); + let _ = agent.clear_keys(); + if let Some(cache) = cache { + let identities = key_tuples_from_cache_header(&cache); + if let Err(e) = agent.set_public_identities(identities.clone()) { + tracing::warn!( + "Failed to reload public identities after device forget: {}", + e + ); + } + public_key_identity_tuples + .write() + .await + .set(identities.clone()); + let mut names = key_names.write().await; + names.clear(); + for (_, name, vault_item_id) in identities { + names.insert(vault_item_id, name); + } + } + } + if failures.is_empty() { - sshwarden_agent::ControlResponse::ok( - "Forgot local key cache, legacy vault file, and device session material", - ) + let message = if remove_shared_cache { + "Forgot shared local key cache, legacy vault file, and device session material" + } else { + "Forgot this device's session and native unlock material; shared key cache was kept" + }; + sshwarden_agent::ControlResponse::ok(message) } else { sshwarden_agent::ControlResponse::err(&format!( "Cleared in-memory state, but FAILED to delete on-disk material: {}. \ @@ -4697,7 +4960,18 @@ async fn handle_control_command( let email = config.auth.email.clone(); let server_url = config.server.base_url.clone(); - match write_envelope_local_key_cache(&keys, &email, &server_url, &pin) { + let cache_write_result = match local_cache_key_state.read().await.clone_key() { + Some(local_cache_key) => write_envelope_local_key_cache_with_key( + &keys, + &email, + &server_url, + &pin, + &local_cache_key, + ), + None => write_envelope_local_key_cache(&keys, &email, &server_url, &pin), + }; + + match cache_write_result { Ok((cache, local_cache_key)) => { *local_key_cache_data.write().await = Some(cache); local_cache_key_state.write().await.set(local_cache_key); @@ -4900,6 +5174,70 @@ fn gate_pin_validator(inner: PinValidator, pin_failures: &PinFailureHandle) -> P }) } +/// Manual `sync` is user-initiated and should not leave the live agent newer +/// than the encrypted Local Key Cache when the vault is already unlocked. If the +/// daemon has private keys loaded but no in-memory Local Cache Key, recover that +/// key via the user's PIN before syncing so `do_sync()` can refresh +/// `local-key-cache.json` with the newly pulled key set. +async fn ensure_local_cache_key_for_manual_sync( + local_key_cache_data: &Arc>>, + local_cache_key_state: &LocalCacheKeyHandle, + ui_request_tx: &UIRequestTx, + pin_failures: &PinFailureHandle, +) -> Result<(), String> { + if multi_device_mode() { + reload_shared_local_cache_from_disk_if_multi_device(local_key_cache_data).await; + } + + let cache = match local_key_cache_data.read().await.as_ref().cloned() { + Some(cache) => cache, + None => return Ok(()), + }; + + if let Some(local_cache_key) = local_cache_key_state.read().await.clone_key() { + if decrypt_envelope_payload(&cache, local_cache_key.clone()).is_ok() { + return Ok(()); + } + tracing::warn!( + "In-memory Local Cache Key no longer decrypts the shared cache; PIN is required" + ); + local_cache_key_state.write().await.clear(); + } + + if cache.local_cache_key.pin_encrypted.is_none() { + return Ok(()); + } + + info!("Manual sync needs PIN to refresh the Local Key Cache"); + let key_holder: Arc>> = + Arc::new(std::sync::Mutex::new(None)); + let key_holder_inner = key_holder.clone(); + let validator: PinValidator = Arc::new(move |pin: &str| -> bool { + match decrypt_envelope_local_key_cache_with_pin(&cache, pin) { + Ok((_keys_json, local_cache_key)) => { + *key_holder_inner.lock().unwrap_or_else(|e| e.into_inner()) = Some(local_cache_key); + true + } + Err(_) => false, + } + }); + let validator = gate_pin_validator(validator, pin_failures); + + match sshwarden_ui::unlock::request_pin_dialog(ui_request_tx, validator).await { + Some(_) => match key_holder.lock().unwrap_or_else(|e| e.into_inner()).take() { + Some(local_cache_key) => { + local_cache_key_state.write().await.set(local_cache_key); + Ok(()) + } + None => Err("PIN accepted but the Local Cache Key was not recovered".to_string()), + }, + None => Err( + "Sync cancelled: PIN is required to refresh the Local Key Cache for this unlocked session" + .to_string(), + ), + } +} + /// Try to restore an API session from the device session file after Hello unlock. /// /// Uses the Hello-encrypted refresh token stored in the session file. @@ -4920,7 +5258,7 @@ async fn try_restore_api_session_hello( _ => return, }; - // Need hello_encrypted_token and the vault's hello_challenge + // Need hello_encrypted_token and this device's Hello challenge. let hello_enc_token = match session.hello_encrypted_token { Some(ref t) => t.clone(), None => { @@ -4929,14 +5267,8 @@ async fn try_restore_api_session_hello( } }; - // Get challenge from vault file - let vault_file = match sshwarden_config::vault::VaultFile::load() { - Ok(Some(v)) => v, - _ => return, - }; - - let challenge_b64 = match vault_file.hello_challenge { - Some(ref c) => c.clone(), + let challenge_b64 = match current_hello_challenge_b64() { + Some(c) => c, None => return, }; @@ -5342,11 +5674,7 @@ fn create_or_preserve_hello_encrypted_token( ) -> Option { #[cfg(windows)] { - let vault = match sshwarden_config::vault::VaultFile::load() { - Ok(Some(v)) => v, - _ => return existing, - }; - let challenge_b64 = match vault.hello_challenge { + let challenge_b64 = match current_hello_challenge_b64() { Some(challenge) => challenge, None => return existing, }; @@ -5530,8 +5858,7 @@ async fn handle_ui_request( // No UI prompt; if the slot is present and unlock succeeds, use it. { let cache_opt = local_key_cache_data.read().await.clone(); - if let Some(cache) = cache_opt.filter(|c| c.local_cache_key.native_encrypted.is_some()) - { + if let Some(cache) = cache_opt.filter(|c| current_native_unlock_slot(c).is_some()) { let cache_for_unlock = cache.clone(); let native_result = tokio::task::spawn_blocking(move || { decrypt_envelope_local_key_cache_with_native(&cache_for_unlock) @@ -5582,7 +5909,7 @@ async fn handle_ui_request( { // 2a. Envelope-based Hello unlock from local-key-cache.json. let cache_opt = local_key_cache_data.read().await.clone(); - if let Some(cache) = cache_opt.filter(|c| c.local_cache_key.hello_encrypted.is_some()) { + if let Some(cache) = cache_opt.filter(|c| current_hello_unlock_info(c).is_some()) { let cache_for_unlock = cache.clone(); let hello_result = tokio::task::spawn_blocking(move || { decrypt_envelope_local_key_cache_with_hello(&cache_for_unlock) @@ -6202,7 +6529,7 @@ async fn prompt_setup_pin( /// Get the runtime data directory for SSHWarden (same as exe directory for portability). fn data_dir() -> anyhow::Result { - sshwarden_config::config_dir() + sshwarden_config::device_data_dir() } /// Get the PID file path. From f60fc195655e1dc61d660c80e9daf88dcedf3c55 Mon Sep 17 00:00:00 2001 From: Hureru <3507039083@qq.com> Date: Mon, 8 Jun 2026 11:16:22 +0800 Subject: [PATCH 2/3] fix: address review findings in multi-device shared storage A multi-dimensional review of the feature surfaced correctness, security, and CI issues. Fix all confirmed findings: - clippy -D warnings (CI gate): use inspect instead of map-for-side-effect; take the recovered key out of the MutexGuard before awaiting - device id path traversal: reject "."/".." in sanitize_device_id and assert a single normal path segment in device_data_dir() - cache the multi_device decision (OnceLock) so a transient config.toml parse failure can no longer flip storage mode mid-run (also fixes a destructive forget that could delete the shared cache) - enroll this device's Hello/native slot after a PIN unlock so a late-joining device is not stuck PIN-only forever - atomic owner-only writes (tmp + rename) for local-key-cache.json and unlock-slots.json via a shared write_owner_only_file helper - case-insensitive home-relative path match on Windows; warn on fallback - gate the manual-sync PIN prompt to multi-device mode (no single-device regression) - device-only forget clears key_names unconditionally - keep the in-memory cache when a shared reload finds the file absent - add tests for sanitize_device_id and home_relative_path; doc fixes Gates green: cargo fmt --check, clippy --all-targets -D warnings, test --workspace (56 passed). --- config.toml.example | 4 + crates/sshwarden-config/src/cache.rs | 14 +- crates/sshwarden-config/src/lib.rs | 128 +++++++++++++-- crates/sshwarden-config/src/ssh_config.rs | 109 ++++++++++++- crates/sshwarden-config/src/unlock_slots.rs | 14 +- src/main.rs | 163 ++++++++++++++++---- 6 files changed, 357 insertions(+), 75 deletions(-) diff --git a/config.toml.example b/config.toml.example index 203c144..0f2c3b1 100644 --- a/config.toml.example +++ b/config.toml.example @@ -134,6 +134,10 @@ portable = false # - 每设备独立：session、pid、log、runtime socket、Windows Hello/native unlock slot # 这样两台机器可以共享同一份 Bitwarden SSH key 本地投影和 Host 绑定， # 但不会互相覆盖运行态文件。 +# 重要：multi_device 本身不改变数据根目录的位置，只在已有数据目录下拆分 +# 共享/设备文件。要让数据真正落在同步目录，仍需用 portable=true（配合 +# portable_dir）或 SSHWARDEN_HOME / SSHWARDEN_PORTABLE 把数据根指向该 +# OneDrive/Dropbox 目录；否则数据仍在平台默认目录（如 %APPDATA%）而不会被共享。 # 若启用，建议同时设置 [ssh_config].path_style = "home_relative"，避免 # C:\Users\zheng 与 C:\Users\Administrator 这样的用户名差异写入绝对路径。 multi_device = false diff --git a/crates/sshwarden-config/src/cache.rs b/crates/sshwarden-config/src/cache.rs index 5a82950..bfe1ea7 100644 --- a/crates/sshwarden-config/src/cache.rs +++ b/crates/sshwarden-config/src/cache.rs @@ -1,7 +1,4 @@ -use std::path::{Path, PathBuf}; - -#[cfg(unix)] -use std::os::unix::fs::PermissionsExt; +use std::path::PathBuf; use anyhow::Context; use serde::{Deserialize, Serialize}; @@ -86,7 +83,7 @@ impl LocalKeyCacheFile { } let content = serde_json::to_string_pretty(self).context("Failed to serialize local key cache")?; - write_owner_only_file(&path, content) + crate::write_owner_only_file(&path, content) .with_context(|| format!("Failed to write local key cache: {}", path.display()))?; Ok(()) } @@ -100,10 +97,3 @@ impl LocalKeyCacheFile { Ok(()) } } - -fn write_owner_only_file(path: &Path, content: impl AsRef<[u8]>) -> anyhow::Result<()> { - std::fs::write(path, content)?; - #[cfg(unix)] - std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))?; - Ok(()) -} diff --git a/crates/sshwarden-config/src/lib.rs b/crates/sshwarden-config/src/lib.rs index 238a637..82528c8 100644 --- a/crates/sshwarden-config/src/lib.rs +++ b/crates/sshwarden-config/src/lib.rs @@ -5,7 +5,7 @@ pub mod ssh_config; pub mod unlock_slots; pub mod vault; -use std::path::PathBuf; +use std::path::{Component, Path, PathBuf}; use std::sync::OnceLock; use anyhow::Context; @@ -24,6 +24,17 @@ static RESOLVED_SHARED_DIR: OnceLock = OnceLock::new(); /// shared directory for backwards compatibility. static RESOLVED_DEVICE_DIR: OnceLock = OnceLock::new(); +/// Cached `[storage] multi_device` decision and resolved device id. +/// +/// Resolved once from the first successful [`Config::load`] and reused for the +/// process lifetime. This keeps every caller in agreement about the active +/// storage layout and, crucially, prevents a transient `config.toml` parse +/// failure (e.g. an OneDrive conflict copy appearing mid-run) from flipping +/// `multi_device` and mis-routing device-local unlock material into the shared +/// cache or deleting a shared cache other devices still depend on. +static RESOLVED_MULTI_DEVICE: OnceLock = OnceLock::new(); +static RESOLVED_DEVICE_ID: OnceLock = OnceLock::new(); + #[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct Config { #[serde(default)] @@ -288,8 +299,8 @@ pub struct StorageConfig { /// /// In this mode `config.toml`, `local-key-cache.json`, `bindings.json`, /// `keys/`, and (by default) `sshwarden_config` remain in the shared data - /// directory, while `session.enc`, `sshwarden.pid`, `sshwarden.log`, and - /// runtime sockets are stored under `devices//`. + /// directory, while `session-.enc`, `sshwarden.pid`, + /// `sshwarden.log`, and runtime sockets are stored under `devices//`. #[serde(default)] pub multi_device: bool, /// Explicit per-device directory name. Leave empty or set to `auto` to use @@ -372,11 +383,24 @@ pub fn device_data_dir() -> anyhow::Result { return Ok(dir.clone()); } let shared = shared_data_dir()?; - let config = Config::load()?; - let resolved = if config.storage.multi_device { - shared - .join("devices") - .join(current_device_id_from_config(&config)?) + let resolved = if multi_device_enabled()? { + let device_id = current_device_id()?; + // Defense in depth: the device id must be exactly one normal path + // segment so `{shared}/devices/{id}` can never escape the devices/ + // subtree (a ".."/"." id would otherwise collapse device-local secrets + // back onto the shared, cloud-synced root). `sanitize_device_id` already + // rejects "."/".."; this guard ensures a future change cannot silently + // reintroduce traversal. + let mut components = Path::new(&device_id).components(); + let single_segment = + matches!(components.next(), Some(Component::Normal(_))) && components.next().is_none(); + if !single_segment { + anyhow::bail!( + "Invalid device id {device_id:?}: must be a single path segment \ + without separators or '.'/'..'" + ); + } + shared.join("devices").join(&device_id) } else { shared }; @@ -385,12 +409,22 @@ pub fn device_data_dir() -> anyhow::Result { } pub fn current_device_id() -> anyhow::Result { + if let Some(id) = RESOLVED_DEVICE_ID.get() { + return Ok(id.clone()); + } let config = Config::load()?; - current_device_id_from_config(&config) + let id = current_device_id_from_config(&config)?; + let _ = RESOLVED_DEVICE_ID.set(id.clone()); + Ok(id) } pub fn multi_device_enabled() -> anyhow::Result { - Ok(Config::load()?.storage.multi_device) + if let Some(value) = RESOLVED_MULTI_DEVICE.get() { + return Ok(*value); + } + let value = Config::load()?.storage.multi_device; + let _ = RESOLVED_MULTI_DEVICE.set(value); + Ok(value) } fn resolve_shared_data_dir() -> anyhow::Result { @@ -630,7 +664,12 @@ fn sanitize_device_id(value: &str) -> String { } } let sanitized = out.trim_matches('-'); - if sanitized.is_empty() { + // Reject path-relative segments ("." / ".." / any all-dots value): joined + // under `{shared}/devices/`, these would resolve back onto the shared root + // or its parent and defeat device isolation. `.` stays allowed inside a + // normal id (e.g. a `host.domain` name) but a segment that is *only* dots is + // never a valid device directory name. + if sanitized.is_empty() || sanitized.chars().all(|c| c == '.') { "device".to_string() } else { sanitized.to_string() @@ -692,6 +731,49 @@ fn home_dir() -> anyhow::Result { .context("HOME environment variable not set") } +/// Atomically write `content` to `path` with owner-only permissions. +/// +/// Serializes to a temp file (set to mode `0600` before promotion on unix, so +/// the bytes are never briefly group/world-readable), then renames it over the +/// destination. On Windows, where renaming over an existing file can fail, this +/// falls back to the same rollback-safe backup+promote flow used by +/// [`bindings::HostBindingsFile::save`]. This bounds a crash or cloud-sync +/// collision to a recoverable file-level event instead of truncating the only +/// copy in place. +pub(crate) fn write_owner_only_file(path: &Path, content: impl AsRef<[u8]>) -> anyhow::Result<()> { + let tmp = path.with_extension("json.tmp"); + std::fs::write(&tmp, content) + .with_context(|| format!("Failed to write tmp file: {}", tmp.display()))?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&tmp, std::fs::Permissions::from_mode(0o600)) + .with_context(|| format!("Failed to set permissions on tmp file: {}", tmp.display()))?; + } + if let Err(e) = std::fs::rename(&tmp, path) { + if path.exists() { + let backup = path.with_extension("json.bak"); + let _ = std::fs::remove_file(&backup); + std::fs::rename(path, &backup).with_context(|| { + format!( + "Failed to back up {} after rename error: {e}", + path.display() + ) + })?; + if let Err(promote_err) = std::fs::rename(&tmp, path) { + let _ = std::fs::rename(&backup, path); + return Err(anyhow::Error::from(promote_err) + .context(format!("Failed to replace file: {}", path.display()))); + } + let _ = std::fs::remove_file(&backup); + } else { + return Err(anyhow::Error::from(e) + .context(format!("Failed to rename tmp file: {}", path.display()))); + } + } + Ok(()) +} + #[cfg(test)] #[allow(clippy::unwrap_used)] mod tests { @@ -737,4 +819,28 @@ mod tests { std::path::PathBuf::from("relative/sshwarden_config") ); } + + #[test] + fn sanitize_device_id_normalizes_and_collapses() { + assert_eq!(sanitize_device_id("My-Host_01"), "my-host_01"); + assert_eq!(sanitize_device_id("Host PC@home"), "host-pc-home"); + assert_eq!(sanitize_device_id(" spaced "), "spaced"); + assert_eq!(sanitize_device_id("a///b"), "a-b"); + assert_eq!(sanitize_device_id("--lead-trail--"), "lead-trail"); + assert_eq!(sanitize_device_id("desktop.example"), "desktop.example"); + } + + #[test] + fn sanitize_device_id_rejects_path_traversal() { + // A segment that is only dots must never become a device directory name, + // or `{shared}/devices/{id}` would escape the devices/ subtree. + assert_eq!(sanitize_device_id("."), "device"); + assert_eq!(sanitize_device_id(".."), "device"); + assert_eq!(sanitize_device_id("..."), "device"); + assert_eq!(sanitize_device_id(""), "device"); + assert_eq!(sanitize_device_id("///"), "device"); + // Separators are neutralized to '-', so traversal characters cannot form + // a real parent reference even when mixed with dots. + assert_eq!(sanitize_device_id("../.."), "..-.."); + } } diff --git a/crates/sshwarden-config/src/ssh_config.rs b/crates/sshwarden-config/src/ssh_config.rs index 060cf3b..27b5a09 100644 --- a/crates/sshwarden-config/src/ssh_config.rs +++ b/crates/sshwarden-config/src/ssh_config.rs @@ -33,9 +33,21 @@ pub fn path_arg(path: &Path) -> String { pub fn path_arg_with_style(path: &Path, style: SshConfigPathStyle) -> String { let display_path = match style { SshConfigPathStyle::Absolute => path.to_path_buf(), - SshConfigPathStyle::HomeRelative => { - home_relative_path(path).unwrap_or_else(|| path.to_path_buf()) - } + SshConfigPathStyle::HomeRelative => match home_relative_path(path) { + Some(relative) => relative, + None => { + // Not under the user's home, so it cannot be made portable. Fall + // back to the absolute path, but warn: on Windows this embeds the + // concrete `C:\Users\` and will not be shareable across + // accounts via a synced snippet. + tracing::warn!( + "ssh_config path_style=home_relative but {} is not under the user home; \ + writing an absolute path that may embed a username", + path.display() + ); + path.to_path_buf() + } + }, }; quote_ssh_config_arg(&display_path.to_string_lossy()) } @@ -106,11 +118,53 @@ fn home_relative_path(path: &Path) -> Option { let home = std::env::var_os("HOME") .or_else(|| std::env::var_os("USERPROFILE")) .map(PathBuf::from)?; - let relative = path.strip_prefix(&home).ok()?; + home_relative_path_with_home(path, &home) +} + +/// Rewrite `path` as `~/...` when it lives under `home`. +/// +/// On Windows the comparison is case-insensitive: the OneDrive key paths and the +/// `USERPROFILE` casing Windows reports can differ (drive-letter or profile-folder +/// case), and a byte-exact `strip_prefix` would otherwise silently fall back to an +/// absolute, username-bearing path and defeat cross-account sharing. +fn home_relative_path_with_home(path: &Path, home: &Path) -> Option { + if let Ok(relative) = path.strip_prefix(home) { + return Some(tilde_join(relative)); + } + #[cfg(windows)] + { + if let Some(relative) = strip_prefix_case_insensitive(path, home) { + return Some(tilde_join(&relative)); + } + } + None +} + +fn tilde_join(relative: &Path) -> PathBuf { if relative.as_os_str().is_empty() { - return Some(PathBuf::from("~")); + PathBuf::from("~") + } else { + PathBuf::from("~").join(relative) } - Some(PathBuf::from("~").join(relative)) +} + +/// Strip `prefix` from `path`, comparing each component case-insensitively. +/// Used on Windows, whose filesystem is case-insensitive. +#[cfg(windows)] +fn strip_prefix_case_insensitive(path: &Path, prefix: &Path) -> Option { + let mut path_components = path.components(); + for prefix_component in prefix.components() { + let next = path_components.next()?; + let actual = next.as_os_str().to_string_lossy().to_lowercase(); + let expected = prefix_component + .as_os_str() + .to_string_lossy() + .to_lowercase(); + if actual != expected { + return None; + } + } + Some(path_components.as_path().to_path_buf()) } #[cfg(test)] @@ -147,4 +201,47 @@ mod tests { path )); } + + #[test] + fn home_relative_rewrites_paths_under_home() { + let home = Path::new("/home/alice"); + assert_eq!( + home_relative_path_with_home(Path::new("/home/alice/OneDrive/keys/id.pub"), home), + Some(PathBuf::from("~/OneDrive/keys/id.pub")) + ); + } + + #[test] + fn home_relative_path_equal_to_home_is_tilde() { + let home = Path::new("/home/alice"); + assert_eq!( + home_relative_path_with_home(Path::new("/home/alice"), home), + Some(PathBuf::from("~")) + ); + } + + #[test] + fn home_relative_path_outside_home_is_none() { + let home = Path::new("/home/alice"); + assert_eq!( + home_relative_path_with_home(Path::new("/etc/ssh/keys/id.pub"), home), + None + ); + } + + #[cfg(windows)] + #[test] + fn home_relative_path_is_case_insensitive_on_windows() { + // USERPROFILE casing as Windows reports it differs from the on-disk path + // casing; the rewrite must still succeed instead of leaking an absolute, + // username-bearing path into a shared snippet. + let home = Path::new(r"C:\users\administrator"); + assert_eq!( + home_relative_path_with_home( + Path::new(r"C:\Users\Administrator\OneDrive\keys\id.pub"), + home + ), + Some(PathBuf::from(r"~\OneDrive\keys\id.pub")) + ); + } } diff --git a/crates/sshwarden-config/src/unlock_slots.rs b/crates/sshwarden-config/src/unlock_slots.rs index 89f071d..5dfe286 100644 --- a/crates/sshwarden-config/src/unlock_slots.rs +++ b/crates/sshwarden-config/src/unlock_slots.rs @@ -1,7 +1,4 @@ -use std::path::{Path, PathBuf}; - -#[cfg(unix)] -use std::os::unix::fs::PermissionsExt; +use std::path::PathBuf; use anyhow::Context; use serde::{Deserialize, Serialize}; @@ -74,7 +71,7 @@ impl UnlockSlotsFile { } let content = serde_json::to_string_pretty(self).context("Failed to serialize unlock slots")?; - write_owner_only_file(&path, content) + crate::write_owner_only_file(&path, content) .with_context(|| format!("Failed to write unlock slots file: {}", path.display()))?; Ok(()) } @@ -89,10 +86,3 @@ impl UnlockSlotsFile { Ok(()) } } - -fn write_owner_only_file(path: &Path, content: impl AsRef<[u8]>) -> anyhow::Result<()> { - std::fs::write(path, content)?; - #[cfg(unix)] - std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))?; - Ok(()) -} diff --git a/src/main.rs b/src/main.rs index 9fe5808..2400ecf 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2476,13 +2476,36 @@ fn refresh_envelope_local_key_cache( } fn multi_device_mode() -> bool { - sshwarden_config::multi_device_enabled().unwrap_or(false) + match sshwarden_config::multi_device_enabled() { + Ok(value) => value, + Err(e) => { + // The decision is cached after the first successful Config::load, so + // this only fires if config is unreadable very early. Warn instead of + // silently assuming single-device, which would otherwise mis-route + // device-local unlock material into the shared cache. + tracing::warn!( + "Could not determine multi-device mode (assuming disabled): {}", + e + ); + false + } + } } fn load_device_unlock_slots() -> Option { - sshwarden_config::unlock_slots::UnlockSlotsFile::load() - .ok() - .flatten() + match sshwarden_config::unlock_slots::UnlockSlotsFile::load() { + Ok(slots) => slots, + Err(e) => { + // Distinguish a corrupt/torn slots file from a missing one: log loudly + // so a truncated unlock-slots.json is not silently treated as "no + // platform unlock" without any signal. + tracing::warn!( + "Failed to load device unlock slots (treating as none): {}", + e + ); + None + } + } } fn current_native_unlock_slot( @@ -2530,13 +2553,17 @@ async fn reload_shared_local_cache_from_disk_if_multi_device( return; } match sshwarden_config::cache::LocalKeyCacheFile::load() { - Ok(cache) => { - if let Some(ref cache) = cache { - if let Err(e) = persist_device_unlock_slots_from_cache(cache) { - tracing::warn!("Failed to persist device unlock slots: {}", e); - } + Ok(Some(cache)) => { + if let Err(e) = persist_device_unlock_slots_from_cache(&cache) { + tracing::warn!("Failed to persist device unlock slots: {}", e); } - *local_key_cache_data.write().await = cache; + *local_key_cache_data.write().await = Some(cache); + } + Ok(None) => { + // The shared cache is momentarily absent (e.g. an in-flight OneDrive + // rename or conflict-copy swap). Keep the valid in-memory copy rather + // than dropping it, which would make the next unlock fail spuriously. + tracing::debug!("Shared local key cache not found on reload; keeping in-memory copy"); } Err(e) => tracing::warn!("Failed to reload shared local key cache: {}", e), } @@ -2582,15 +2609,14 @@ fn enroll_native_for_local_key_cache( if !sshwarden_ui::unlock::native::native_available() { anyhow::bail!("native unlock is not available"); } - let encoded_local_cache_key = sshwarden_api::crypto::encode_symmetric_key(local_cache_key); - let native_slot = - sshwarden_ui::unlock::native::native_encrypt_local_cache_key(&encoded_local_cache_key)?; if multi_device_mode() { - let mut slots = load_device_unlock_slots().unwrap_or_default(); - slots.native_encrypted = Some(native_slot); - slots.save()?; + // Device-local slot only; keep platform material out of the shared cache. + native_encrypt_device_slot(local_cache_key)?; cache.local_cache_key.native_encrypted = None; } else { + let encoded_local_cache_key = sshwarden_api::crypto::encode_symmetric_key(local_cache_key); + let native_slot = + sshwarden_ui::unlock::native::native_encrypt_local_cache_key(&encoded_local_cache_key)?; cache.local_cache_key.native_encrypted = Some(native_slot); } Ok(()) @@ -2601,23 +2627,75 @@ fn enroll_hello_for_local_key_cache( cache: &mut sshwarden_config::cache::LocalKeyCacheFile, local_cache_key: &sshwarden_api::crypto::SymmetricKey, ) -> anyhow::Result<()> { - let challenge: [u8; 16] = rand::random(); - let hello_encrypted = encrypt_local_cache_key_with_hello(local_cache_key, &challenge)?; - let hello_challenge = base64::engine::general_purpose::STANDARD.encode(challenge); if multi_device_mode() { - let mut slots = load_device_unlock_slots().unwrap_or_default(); - slots.hello_challenge = Some(hello_challenge); - slots.hello_encrypted = Some(hello_encrypted); - slots.save()?; + // Device-local slot only; keep platform material out of the shared cache. + hello_encrypt_device_slot(local_cache_key)?; cache.local_cache_key.hello_challenge = None; cache.local_cache_key.hello_encrypted = None; } else { + let challenge: [u8; 16] = rand::random(); + let hello_encrypted = encrypt_local_cache_key_with_hello(local_cache_key, &challenge)?; + let hello_challenge = base64::engine::general_purpose::STANDARD.encode(challenge); cache.local_cache_key.hello_challenge = Some(hello_challenge); cache.local_cache_key.hello_encrypted = Some(hello_encrypted); } Ok(()) } +/// Native-encrypt the Local Cache Key into THIS device's `unlock-slots.json`. +fn native_encrypt_device_slot( + local_cache_key: &sshwarden_api::crypto::SymmetricKey, +) -> anyhow::Result<()> { + let encoded_local_cache_key = sshwarden_api::crypto::encode_symmetric_key(local_cache_key); + let native_slot = + sshwarden_ui::unlock::native::native_encrypt_local_cache_key(&encoded_local_cache_key)?; + let mut slots = load_device_unlock_slots().unwrap_or_default(); + slots.native_encrypted = Some(native_slot); + slots.save() +} + +/// Windows Hello-encrypt the Local Cache Key into THIS device's `unlock-slots.json`. +#[cfg(windows)] +fn hello_encrypt_device_slot( + local_cache_key: &sshwarden_api::crypto::SymmetricKey, +) -> anyhow::Result<()> { + let challenge: [u8; 16] = rand::random(); + let hello_encrypted = encrypt_local_cache_key_with_hello(local_cache_key, &challenge)?; + let hello_challenge = base64::engine::general_purpose::STANDARD.encode(challenge); + let mut slots = load_device_unlock_slots().unwrap_or_default(); + slots.hello_challenge = Some(hello_challenge); + slots.hello_encrypted = Some(hello_encrypted); + slots.save() +} + +/// Multi-device: after a successful PIN unlock, make sure THIS device has its own +/// platform unlock slot. The shared cache no longer carries Hello/native slots +/// (refresh strips them in multi-device mode), so a device that joins later would +/// otherwise be stuck PIN-only forever. Best-effort: failures are logged but never +/// surfaced, and a slot that already exists is left untouched. +fn enroll_device_platform_slots_if_missing(local_cache_key: &sshwarden_api::crypto::SymmetricKey) { + if !multi_device_mode() { + return; + } + let slots = load_device_unlock_slots().unwrap_or_default(); + if slots.native_encrypted.is_none() && sshwarden_ui::unlock::native::native_available() { + match native_encrypt_device_slot(local_cache_key) { + Ok(()) => info!("Enrolled this device's native unlock slot"), + Err(e) => tracing::warn!("Failed to enroll device native unlock slot: {}", e), + } + } + #[cfg(windows)] + { + if slots.hello_encrypted.is_none() && sshwarden_ui::unlock::hello_crypto::hello_available() + { + match hello_encrypt_device_slot(local_cache_key) { + Ok(()) => info!("Enrolled this device's Windows Hello unlock slot"), + Err(e) => tracing::warn!("Failed to enroll device Hello unlock slot: {}", e), + } + } + } +} + /// Encrypt the local cache key with a PIN, generating a fresh random salt. /// Returns `(pin_encrypted, pin_salt_b64)` for the v3 cache format (SEC-04). fn encrypt_local_cache_key_with_pin( @@ -3183,15 +3261,14 @@ async fn run_foreground( tracing::warn!("Failed to load local key cache: {}", e); None }) - .map(|cache| { + .inspect(|cache| { // Multi-device migration aid: if an older shared cache still carries // platform unlock slots, copy them into this device's private // unlock-slots file. Future cache writes strip platform slots from // the shared file so devices stop overwriting each other. - if let Err(e) = persist_device_unlock_slots_from_cache(&cache) { + if let Err(e) = persist_device_unlock_slots_from_cache(cache) { tracing::warn!("Failed to persist device unlock slots: {}", e); } - cache }); let vault_file = sshwarden_config::vault::VaultFile::load().unwrap_or_else(|e| { tracing::warn!("Failed to load vault file: {}", e); @@ -4517,6 +4594,12 @@ async fn handle_control_command( } } } + // Multi-device: seed THIS device's own platform unlock + // slot from the freshly recovered key, so a later-joining + // device isn't stuck PIN-only after the shared cache is + // stripped of platform slots. No-op in single-device mode + // or when a slot already exists. + enroll_device_platform_slots_if_missing(&local_cache_key); local_cache_key_state.write().await.set(local_cache_key); let resp = finish_unlock_with_json( &keys_json, @@ -4789,7 +4872,11 @@ async fn handle_control_command( // Report that honestly and mark a pending sync so the next unlock // applies the keys, instead of claiming the running agent was updated. let was_locked = vault_locked.load(std::sync::atomic::Ordering::Relaxed); - if !was_locked { + // Only multi-device mode needs the shared cache reconciled before a + // manual sync. Gating this avoids a NEW PIN prompt (and a hard failure + // on cancel) for legacy single-device users whose unlock path did not + // keep the Local Cache Key in memory. + if !was_locked && multi_device_mode() { // If this unlocked session was restored without keeping the Local // Cache Key in memory, a plain sync would update the live agent but // leave `sshwarden keys` showing the old cache header. Prompt for @@ -4910,6 +4997,10 @@ async fn handle_control_command( .as_ref() .map(|v| v.pin_encrypted.clone()); public_key_identity_tuples.write().await.clear(); + // Clear unconditionally (mirroring the shared-cache branch): if the + // shared cache fails to reload below, stale labels from the + // just-forgotten session must not linger in memory. + key_names.write().await.clear(); let _ = agent.clear_keys(); if let Some(cache) = cache { let identities = key_tuples_from_cache_header(&cache); @@ -4924,7 +5015,6 @@ async fn handle_control_command( .await .set(identities.clone()); let mut names = key_names.write().await; - names.clear(); for (_, name, vault_item_id) in identities { names.insert(vault_item_id, name); } @@ -5224,13 +5314,18 @@ async fn ensure_local_cache_key_for_manual_sync( let validator = gate_pin_validator(validator, pin_failures); match sshwarden_ui::unlock::request_pin_dialog(ui_request_tx, validator).await { - Some(_) => match key_holder.lock().unwrap_or_else(|e| e.into_inner()).take() { - Some(local_cache_key) => { - local_cache_key_state.write().await.set(local_cache_key); - Ok(()) + Some(_) => { + // Take the recovered key OUT of the guard before awaiting, so no + // std::sync::MutexGuard is held across the .await (clippy::await_holding_lock). + let recovered = key_holder.lock().unwrap_or_else(|e| e.into_inner()).take(); + match recovered { + Some(local_cache_key) => { + local_cache_key_state.write().await.set(local_cache_key); + Ok(()) + } + None => Err("PIN accepted but the Local Cache Key was not recovered".to_string()), } - None => Err("PIN accepted but the Local Cache Key was not recovered".to_string()), - }, + } None => Err( "Sync cancelled: PIN is required to refresh the Local Key Cache for this unlocked session" .to_string(), From 105c8721a8469b21deecf91bf80e2d3ff0d774e9 Mon Sep 17 00:00:00 2001 From: Hureru <3507039083@qq.com> Date: Mon, 8 Jun 2026 13:55:50 +0800 Subject: [PATCH 3/3] fix: harden review follow-ups in multi-device shared storage Address a second review pass over the previous fix commit: - ssh_config: stop line_matches_sshwarden_include from emitting a home_relative warning on every ~/.ssh/config comparison; warn only on the write path via a new no-log path_arg_with_style_quiet - unlock_slots: validate version in save() (shared helper with load()) so an unreadable file is never persisted; drop the exists()-then-IO TOCTOU in load()/delete() in favor of matching ErrorKind::NotFound - session: use the crate-level atomic write_owner_only_file instead of a local non-atomic write; generalize its tmp/backup naming to append a suffix so .enc files keep their extension (session-x.enc.tmp) - main: current_hello_challenge_b64 reads the envelope cache (local-key-cache.json) before vault.enc in single-device mode, fixing Hello token restore after envelope migration - main: seed this device's platform unlock slot after the SSH-request PIN auto-unlock in handle_ui_request, matching the control-path behavior Gates green: cargo fmt --check, clippy --all-targets -D warnings, test --workspace (56 passed). --- crates/sshwarden-config/src/lib.rs | 13 +++++- crates/sshwarden-config/src/session.rs | 14 +----- crates/sshwarden-config/src/ssh_config.rs | 38 +++++++++------- crates/sshwarden-config/src/unlock_slots.rs | 48 +++++++++++++++------ src/main.rs | 26 ++++++++--- 5 files changed, 89 insertions(+), 50 deletions(-) diff --git a/crates/sshwarden-config/src/lib.rs b/crates/sshwarden-config/src/lib.rs index 82528c8..872dc20 100644 --- a/crates/sshwarden-config/src/lib.rs +++ b/crates/sshwarden-config/src/lib.rs @@ -741,7 +741,7 @@ fn home_dir() -> anyhow::Result { /// collision to a recoverable file-level event instead of truncating the only /// copy in place. pub(crate) fn write_owner_only_file(path: &Path, content: impl AsRef<[u8]>) -> anyhow::Result<()> { - let tmp = path.with_extension("json.tmp"); + let tmp = with_suffix(path, ".tmp"); std::fs::write(&tmp, content) .with_context(|| format!("Failed to write tmp file: {}", tmp.display()))?; #[cfg(unix)] @@ -752,7 +752,7 @@ pub(crate) fn write_owner_only_file(path: &Path, content: impl AsRef<[u8]>) -> a } if let Err(e) = std::fs::rename(&tmp, path) { if path.exists() { - let backup = path.with_extension("json.bak"); + let backup = with_suffix(path, ".bak"); let _ = std::fs::remove_file(&backup); std::fs::rename(path, &backup).with_context(|| { format!( @@ -774,6 +774,15 @@ pub(crate) fn write_owner_only_file(path: &Path, content: impl AsRef<[u8]>) -> a Ok(()) } +/// Append `suffix` to the full file name. Unlike [`Path::with_extension`] this +/// preserves the original extension, so `session-x.enc` becomes +/// `session-x.enc.tmp` rather than `session-x.tmp`. +fn with_suffix(path: &Path, suffix: &str) -> PathBuf { + let mut name = path.as_os_str().to_owned(); + name.push(suffix); + PathBuf::from(name) +} + #[cfg(test)] #[allow(clippy::unwrap_used)] mod tests { diff --git a/crates/sshwarden-config/src/session.rs b/crates/sshwarden-config/src/session.rs index 6d1cd71..a84531e 100644 --- a/crates/sshwarden-config/src/session.rs +++ b/crates/sshwarden-config/src/session.rs @@ -1,7 +1,4 @@ -use std::path::{Path, PathBuf}; - -#[cfg(unix)] -use std::os::unix::fs::PermissionsExt; +use std::path::PathBuf; use anyhow::Context; use serde::{Deserialize, Serialize}; @@ -91,7 +88,7 @@ impl SessionFile { } let content = serde_json::to_string_pretty(self).context("Failed to serialize session file")?; - write_owner_only_file(&path, content) + crate::write_owner_only_file(&path, content) .with_context(|| format!("Failed to write session file: {}", path.display()))?; Ok(()) } @@ -113,13 +110,6 @@ impl SessionFile { } } -fn write_owner_only_file(path: &Path, content: impl AsRef<[u8]>) -> anyhow::Result<()> { - std::fs::write(path, content)?; - #[cfg(unix)] - std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))?; - Ok(()) -} - /// Get the machine hostname, sanitised for use in file names. fn hostname() -> String { std::env::var("COMPUTERNAME") diff --git a/crates/sshwarden-config/src/ssh_config.rs b/crates/sshwarden-config/src/ssh_config.rs index 27b5a09..e024197 100644 --- a/crates/sshwarden-config/src/ssh_config.rs +++ b/crates/sshwarden-config/src/ssh_config.rs @@ -31,23 +31,29 @@ pub fn path_arg(path: &Path) -> String { } pub fn path_arg_with_style(path: &Path, style: SshConfigPathStyle) -> String { + if matches!(style, SshConfigPathStyle::HomeRelative) && home_relative_path(path).is_none() { + // Fell back to the absolute path: on Windows this embeds the concrete + // `C:\Users\` and will not be shareable across accounts via a + // synced snippet. Warn only here, on the write path — never from the + // match helpers below, which would otherwise log on every comparison + // while parsing ~/.ssh/config. + tracing::warn!( + "ssh_config path_style=home_relative but {} is not under the user home; \ + writing an absolute path that may embed a username", + path.display() + ); + } + path_arg_with_style_quiet(path, style) +} + +/// Like [`path_arg_with_style`] but never logs. Used by include-line matching so +/// that parsing `~/.ssh/config` does not emit a warning on every comparison. +fn path_arg_with_style_quiet(path: &Path, style: SshConfigPathStyle) -> String { let display_path = match style { SshConfigPathStyle::Absolute => path.to_path_buf(), - SshConfigPathStyle::HomeRelative => match home_relative_path(path) { - Some(relative) => relative, - None => { - // Not under the user's home, so it cannot be made portable. Fall - // back to the absolute path, but warn: on Windows this embeds the - // concrete `C:\Users\` and will not be shareable across - // accounts via a synced snippet. - tracing::warn!( - "ssh_config path_style=home_relative but {} is not under the user home; \ - writing an absolute path that may embed a username", - path.display() - ); - path.to_path_buf() - } - }, + SshConfigPathStyle::HomeRelative => { + home_relative_path(path).unwrap_or_else(|| path.to_path_buf()) + } }; quote_ssh_config_arg(&display_path.to_string_lossy()) } @@ -57,7 +63,7 @@ pub fn include_line(include_path: &Path) -> String { } pub fn include_line_with_style(include_path: &Path, style: SshConfigPathStyle) -> String { - format!("Include {}", path_arg_with_style(include_path, style)) + format!("Include {}", path_arg_with_style_quiet(include_path, style)) } pub fn legacy_unquoted_include_line(include_path: &Path) -> String { diff --git a/crates/sshwarden-config/src/unlock_slots.rs b/crates/sshwarden-config/src/unlock_slots.rs index 5dfe286..e67f74c 100644 --- a/crates/sshwarden-config/src/unlock_slots.rs +++ b/crates/sshwarden-config/src/unlock_slots.rs @@ -1,4 +1,4 @@ -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use anyhow::Context; use serde::{Deserialize, Serialize}; @@ -41,26 +41,42 @@ impl UnlockSlotsFile { pub fn load() -> anyhow::Result> { let path = Self::path()?; - if !path.exists() { - return Ok(None); - } - let content = std::fs::read_to_string(&path) - .with_context(|| format!("Failed to read unlock slots file: {}", path.display()))?; + // Read directly and match the error kind instead of an exists() pre-check, + // which would race if the file is removed/renamed between the two calls. + let content = match std::fs::read_to_string(&path) { + Ok(content) => content, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(e) => { + return Err(anyhow::Error::from(e).context(format!( + "Failed to read unlock slots file: {}", + path.display() + ))) + } + }; let slots: UnlockSlotsFile = serde_json::from_str(&content) .with_context(|| format!("Failed to parse unlock slots file: {}", path.display()))?; - if !Self::SUPPORTED_VERSIONS.contains(&slots.version) { + Self::ensure_supported_version(slots.version, &path)?; + Ok(Some(slots)) + } + + /// Reject versions this build cannot round-trip. Used by both `load` and + /// `save` so we never persist a file we would later refuse to read back. + fn ensure_supported_version(version: u32, path: &Path) -> anyhow::Result<()> { + if !Self::SUPPORTED_VERSIONS.contains(&version) { anyhow::bail!( "Unsupported unlock slots version {} (supported: {:?}): {}", - slots.version, + version, Self::SUPPORTED_VERSIONS, path.display() ); } - Ok(Some(slots)) + Ok(()) } pub fn save(&self) -> anyhow::Result<()> { let path = Self::path()?; + // Refuse to persist a version we could not read back (mirrors `load`). + Self::ensure_supported_version(self.version, &path)?; if let Some(parent) = path.parent() { std::fs::create_dir_all(parent).with_context(|| { format!( @@ -78,11 +94,15 @@ impl UnlockSlotsFile { pub fn delete() -> anyhow::Result<()> { let path = Self::path()?; - if path.exists() { - std::fs::remove_file(&path).with_context(|| { - format!("Failed to delete unlock slots file: {}", path.display()) - })?; + // Remove directly and tolerate NotFound rather than exists()-then-remove, + // which can race with concurrent removal/sync. + match std::fs::remove_file(&path) { + Ok(()) => Ok(()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(anyhow::Error::from(e).context(format!( + "Failed to delete unlock slots file: {}", + path.display() + ))), } - Ok(()) } } diff --git a/src/main.rs b/src/main.rs index 2400ecf..43b4a7c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2593,12 +2593,21 @@ fn current_hello_challenge_b64() -> Option { if multi_device_mode() { device_challenge } else { - device_challenge.or_else(|| { - sshwarden_config::vault::VaultFile::load() - .ok() - .flatten() - .and_then(|vault| vault.hello_challenge) - }) + device_challenge + .or_else(|| { + // After set-pin / envelope migration the Hello challenge lives in + // the envelope cache (local-key-cache.json), not vault.enc. + sshwarden_config::cache::LocalKeyCacheFile::load() + .ok() + .flatten() + .and_then(|cache| cache.local_cache_key.hello_challenge) + }) + .or_else(|| { + sshwarden_config::vault::VaultFile::load() + .ok() + .flatten() + .and_then(|vault| vault.hello_challenge) + }) } } @@ -6184,6 +6193,11 @@ async fn handle_ui_request( if let Some(kh) = lck_holder { let maybe_lck = kh.lock().unwrap().take(); if let Some(lck) = maybe_lck { + // Multi-device: seed this device's own platform unlock + // slot from the freshly recovered key, mirroring the + // explicit unlock-pin control path (no-op in + // single-device mode or if a slot already exists). + enroll_device_platform_slots_if_missing(&lck); local_cache_key_state.write().await.set(lck); } }