diff --git a/Cargo.lock b/Cargo.lock index 427727e..914ae11 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5263,6 +5263,7 @@ dependencies = [ "bitwarden-russh", "bytes", "futures", + "libc", "pin-project", "serde", "serde_json", diff --git a/README.md b/README.md index 4f1385d..9e6f6fe 100644 --- a/README.md +++ b/README.md @@ -21,13 +21,13 @@ SSHWarden 的目标不是复刻完整 Bitwarden Desktop，而是提供一个专 | Bitwarden 登录与 SSH Key 同步 | 已实现 | | SSH Agent 协议服务 | Windows 已实现；Unix socket 有基础实现 | | 签名授权对话框 | Slint 跨平台 UI 已实现 | -| PIN 解锁 | 已实现，后续需迁移到 envelope encryption 模型 | -| Windows Hello | 已有实现，后续需迁移到 envelope encryption 模型 | -| IPC 控制命令 | Windows 已实现；Linux/macOS 待补独立 control socket | -| Local Key Cache | 已实现旧模型；目标模型见 ADR | -| 标准平台存储目录 | 待实现；当前代码仍使用 exe 同目录 | +| PIN 解锁 | 已实现（信封加密 + 每库随机盐 + 失败延迟/锁定） | +| Windows Hello | 已实现（信封加密模型） | +| IPC 控制命令 | 已实现；Windows Named Pipe 与 Unix socket 均含调用方鉴权 | +| Local Key Cache | 已实现信封加密模型（格式 v3，PIN 随机盐） | +| 标准平台存储目录 | 已实现，默认平台标准目录（`%APPDATA%`/`$XDG_CONFIG_HOME`/`~/Library/Application Support`）；便携模式可选 | | 自启动 | Windows 已实现；macOS/Linux 待实现 | -| Shell integration | 待实现 `sshwarden env` | +| Shell integration | `sshwarden env` 已实现（sh/fish/powershell/cmd） | | macOS native unlock | 已实现（Phase 6，Keychain；仍需平台实机验证） | | Linux native unlock | 已实现（Phase 6，Secret Service；仍需平台实机验证） | diff --git a/config.toml.example b/config.toml.example index 821ecfa..ef3bbe3 100644 --- a/config.toml.example +++ b/config.toml.example @@ -46,6 +46,9 @@ email = "" # 可选值: # - "always": 每次请求都提示（最安全） # - "never": 从不提示（自动批准所有请求） +# ⚠ 不安全:SSHWarden 不对调用方做进程级鉴权,因此本机上任何以你的用户身份 +# 运行的进程都能在你不知情的情况下让 agent 静默签名。仅在完全受信任的环境 +# (如隔离的 CI runner)使用。 # - "remember_until_lock": 记住决定直到锁定（平衡安全与便利） # 默认: "always" prompt_behavior = "always" diff --git a/crates/sshwarden-agent/Cargo.toml b/crates/sshwarden-agent/Cargo.toml index 8eec218..a7d2941 100644 --- a/crates/sshwarden-agent/Cargo.toml +++ b/crates/sshwarden-agent/Cargo.toml @@ -16,7 +16,7 @@ serde_json = { workspace = true } ssh-key = { workspace = true, features = ["encryption", "ed25519", "rsa", "getrandom"] } sysinfo = { workspace = true, features = ["windows"] } thiserror = { workspace = true } -tokio = { workspace = true, features = ["io-util", "sync", "macros", "net"] } +tokio = { workspace = true, features = ["io-util", "sync", "macros", "net", "time"] } tokio-util = { workspace = true, features = ["codec"] } tracing = { workspace = true } sshwarden-config = { path = "../sshwarden-config" } @@ -26,8 +26,14 @@ pin-project = "1" windows = { version = "0.61", features = [ "Win32_Foundation", "Win32_System_Pipes", + "Win32_System_Threading", + "Win32_Security", + "Win32_Security_Authorization", ] } +[target.'cfg(unix)'.dependencies] +libc = "0.2" + [dev-dependencies] tokio = { workspace = true, features = ["rt-multi-thread", "macros"] } diff --git a/crates/sshwarden-agent/src/agent.rs b/crates/sshwarden-agent/src/agent.rs index cc3f0f4..19c6765 100644 --- a/crates/sshwarden-agent/src/agent.rs +++ b/crates/sshwarden-agent/src/agent.rs @@ -68,6 +68,11 @@ pub struct SshWardenAgent { request_id: Arc, needs_unlock: Arc, is_running: Arc, + /// Reports a fatal agent-transport failure (e.g. the SSH agent endpoint + /// could not be claimed). The main loop watches this so the daemon shuts + /// down instead of running as a zombie that still answers status/unlock + /// while serving no SSH client (RT-01). + fatal_tx: Arc>>, } impl ssh_agent::Agent for SshWardenAgent { @@ -102,7 +107,8 @@ impl ssh_agent::Agent for SshWardenAgent { ); let mut rx_channel = self.ui_response_tx.subscribe(); - self.show_ui_request_tx + if self + .show_ui_request_tx .send(SshAgentUIRequest { request_id, cipher_id: Some(ssh_key.cipher_uuid.clone()), @@ -114,7 +120,13 @@ impl ssh_agent::Agent for SshWardenAgent { is_forwarding: info.is_forwarding(), }) .await - .expect("Should send request to ui"); + .is_err() + { + // The host application's UI channel is gone (daemon shutting down or + // main loop exited). Fail closed instead of panicking the serve task. + error!("UI request channel closed; denying sign request"); + return false; + } while let Ok((id, response)) = rx_channel.recv().await { if id == request_id { return response; @@ -141,10 +153,11 @@ impl ssh_agent::Agent for SshWardenAgent { operation_kind: crate::request_parser::OperationKind::SshAuthentication, is_forwarding: info.is_forwarding(), }; - self.show_ui_request_tx - .send(message) - .await - .expect("Should send request to ui"); + if self.show_ui_request_tx.send(message).await.is_err() { + // UI channel gone (daemon shutting down); fail closed, don't panic. + error!("UI request channel closed; denying list request"); + return false; + } while let Ok((id, response)) = rx_channel.recv().await { if id == request_id { return response; @@ -175,6 +188,7 @@ impl SshWardenAgent { auth_request_tx: tokio::sync::mpsc::Sender, auth_response_tx: Arc>, ) -> Self { + let (fatal_tx, _fatal_rx) = tokio::sync::watch::channel(None); Self { keystore: ssh_agent::KeyStore(Arc::new(RwLock::new(HashMap::new()))), cancellation_token: CancellationToken::new(), @@ -183,6 +197,7 @@ impl SshWardenAgent { request_id: Arc::new(AtomicU32::new(0)), needs_unlock: Arc::new(AtomicBool::new(true)), is_running: Arc::new(AtomicBool::new(false)), + fatal_tx: Arc::new(fatal_tx), } } @@ -229,7 +244,7 @@ impl SshWardenAgent { .public_key() .to_bytes() .expect("Cipher private key is always correctly parsed"); - keystore.0.write().expect("RwLock is not poisoned").insert( + let displaced = keystore.0.write().expect("RwLock is not poisoned").insert( public_key_bytes.clone(), SshWardenKey { private_key: Some(private_key), @@ -238,6 +253,17 @@ impl SshWardenAgent { cipher_uuid: cipher_id.clone(), }, ); + if displaced.is_some() { + // LOGIC-7: the keystore is keyed by public key bytes, so + // two vault items sharing a public key collapse into one. + // Warn so the operator understands why status/key_count + // can be lower than the number of vault keys. + tracing::warn!( + key = %name, + "Duplicate public key: overwrote an earlier agent keystore entry; \ + status key_count will be lower than the vault key count" + ); + } } Err(e) => { error!(error=%e, "Error while parsing key"); @@ -324,6 +350,20 @@ impl SshWardenAgent { .len() } + /// Number of loaded keys that actually hold private material and can sign. + /// While the vault is locked this is 0 even though `key_count()` (listed + /// identities) may be non-zero. status reports this so what it shows matches + /// what the agent can actually sign. + pub fn signable_key_count(&self) -> usize { + self.keystore + .0 + .read() + .expect("RwLock is not poisoned") + .values() + .filter(|k| k.private_key.is_some()) + .count() + } + pub fn start_server( auth_request_tx: tokio::sync::mpsc::Sender, auth_response_tx: Arc>, @@ -349,6 +389,18 @@ impl SshWardenAgent { self.is_running.clone() } + /// A receiver the main loop watches for a fatal agent-transport failure. + /// When a value is sent, the daemon should shut down rather than keep + /// running as a zombie that still answers status/unlock (RT-01). + pub fn fatal_rx(&self) -> tokio::sync::watch::Receiver> { + self.fatal_tx.subscribe() + } + + /// A clonable sender the transport listener uses to report a fatal failure. + pub fn fatal_tx(&self) -> Arc>> { + self.fatal_tx.clone() + } + pub fn keystore_clone(&self) -> ssh_agent::KeyStore { self.keystore.clone() } @@ -430,4 +482,32 @@ nGZV/aEAZ3ZMrsrA3g32AAAAEHRlc3RAZXhhbXBsZS5jb20BAgMEBQ== assert!(!signature.as_bytes().is_empty()); assert_eq!(signature.algorithm(), ssh_key::Algorithm::Ed25519); } + + // lock-keystore: after lock(), identities remain listable (ssh-add -l keeps + // working so auto-unlock-on-request can fire) but hold no private material, + // so signable_key_count drops to 0 and status can report the truth. + #[test] + fn lock_keeps_keys_listable_but_not_signable() { + let (mut agent, _request_rx, _response_tx) = create_test_agent(); + agent.is_running.store(true, Ordering::Relaxed); + agent + .set_keys(vec![( + TEST_ED25519_KEY.to_string(), + "ed25519-key".to_string(), + "ed25519-uuid".to_string(), + )]) + .expect("set_keys should succeed"); + + assert_eq!(agent.key_count(), 1); + assert_eq!(agent.signable_key_count(), 1); + + agent.lock().expect("lock should succeed"); + + assert_eq!(agent.key_count(), 1, "identity stays listable while locked"); + assert_eq!( + agent.signable_key_count(), + 0, + "no private material is signable while locked" + ); + } } diff --git a/crates/sshwarden-agent/src/control.rs b/crates/sshwarden-agent/src/control.rs index 96aadfe..f957065 100644 --- a/crates/sshwarden-agent/src/control.rs +++ b/crates/sshwarden-agent/src/control.rs @@ -67,6 +67,121 @@ impl ControlResponse { pub const CONTROL_PIPE_NAME: &str = r"\\.\pipe\sshwarden-control"; +/// Maximum number of bytes accepted for a single control command line on the +/// daemon side. The control protocol is one short JSON object per connection, +/// so anything larger is malformed or hostile; capping the read avoids +/// unbounded buffering from a local process flooding the channel (EH-08). +const MAX_CONTROL_LINE_BYTES: u64 = 64 * 1024; + +/// SEC-01 (Windows): build a security descriptor restricting the control pipe +/// to the current user + LocalSystem, so the named pipe is not created with the +/// default DACL (which grants Everyone read access). Best-effort: callers fall +/// back to the default DACL if the descriptor cannot be built. +#[cfg(windows)] +mod win_security { + use windows::core::{HSTRING, PWSTR}; + use windows::Win32::Foundation::LocalFree; + use windows::Win32::Foundation::{CloseHandle, HANDLE, HLOCAL}; + use windows::Win32::Security::Authorization::{ + ConvertSidToStringSidW, ConvertStringSecurityDescriptorToSecurityDescriptorW, + SDDL_REVISION_1, + }; + use windows::Win32::Security::{ + GetTokenInformation, TokenUser, PSECURITY_DESCRIPTOR, SECURITY_ATTRIBUTES, TOKEN_QUERY, + TOKEN_USER, + }; + use windows::Win32::System::Threading::{GetCurrentProcess, OpenProcessToken}; + + /// Owns the security descriptor and the SECURITY_ATTRIBUTES referencing it; + /// frees the descriptor on drop. + pub struct PipeSecurity { + sd: PSECURITY_DESCRIPTOR, + sa: SECURITY_ATTRIBUTES, + } + + impl PipeSecurity { + /// Build a DACL granting only the current user and LocalSystem full + /// control. Returns None on any failure (caller uses the default DACL). + pub fn current_user_only() -> Option { + unsafe { + let sid = current_user_sid_string()?; + let sddl = format!("D:P(A;;GA;;;{sid})(A;;GA;;;SY)"); + let mut sd = PSECURITY_DESCRIPTOR(core::ptr::null_mut()); + ConvertStringSecurityDescriptorToSecurityDescriptorW( + &HSTRING::from(&sddl), + SDDL_REVISION_1, + &mut sd, + None, + ) + .ok()?; + if sd.0.is_null() { + return None; + } + let sa = SECURITY_ATTRIBUTES { + nLength: core::mem::size_of::() as u32, + lpSecurityDescriptor: sd.0, + bInheritHandle: false.into(), + }; + Some(Self { sd, sa }) + } + } + + /// Raw pointer to the SECURITY_ATTRIBUTES for + /// `create_with_security_attributes_raw`. Valid while `self` is alive. + pub fn as_attrs_ptr(&mut self) -> *mut core::ffi::c_void { + &mut self.sa as *mut _ as *mut core::ffi::c_void + } + } + + impl Drop for PipeSecurity { + fn drop(&mut self) { + if !self.sd.0.is_null() { + unsafe { + let _ = LocalFree(Some(HLOCAL(self.sd.0))); + } + } + } + } + + // SAFETY: the owned security descriptor is process-global heap memory + // (LocalAlloc'd) accessed only for pipe creation and freed on drop, so the + // owning value is safe to move between tokio worker threads. + unsafe impl Send for PipeSecurity {} + + unsafe fn current_user_sid_string() -> Option { + let mut token = HANDLE::default(); + OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut token).ok()?; + + // First call sizes the buffer, the second fills it. + let mut len = 0u32; + let _ = GetTokenInformation(token, TokenUser, None, 0, &mut len); + if len == 0 { + let _ = CloseHandle(token); + return None; + } + let mut buf = vec![0u8; len as usize]; + let res = GetTokenInformation( + token, + TokenUser, + Some(buf.as_mut_ptr() as *mut core::ffi::c_void), + len, + &mut len, + ); + let _ = CloseHandle(token); + res.ok()?; + + let token_user = &*(buf.as_ptr() as *const TOKEN_USER); + let mut pwstr = PWSTR::null(); + ConvertSidToStringSidW(token_user.User.Sid, &mut pwstr).ok()?; + if pwstr.is_null() { + return None; + } + let sid = pwstr.to_string().ok(); + let _ = LocalFree(Some(HLOCAL(pwstr.0 as *mut core::ffi::c_void))); + sid + } +} + /// A request sent from the control server to the main loop. pub struct ControlRequest { pub action: ControlAction, @@ -108,33 +223,64 @@ pub async fn start_control_server( tx: tokio::sync::mpsc::Sender, cancel: tokio_util::sync::CancellationToken, ) { - use tokio::io::{AsyncBufReadExt, BufReader}; + use tokio::io::{AsyncBufReadExt, AsyncReadExt, BufReader}; use tokio::net::windows::named_pipe::ServerOptions; info!("Control server starting on {}", CONTROL_PIPE_NAME); + // SEC-01: restrict the control pipe to the current user + SYSTEM. If the + // descriptor cannot be built, fall back to the default DACL. + let mut pipe_security = win_security::PipeSecurity::current_user_only(); + if pipe_security.is_none() { + error!("Could not build control pipe security descriptor; using default DACL"); + } + + // SEC-01: we must create the FIRST instance of the pipe ourselves so OUR + // security descriptor governs the DACL. Windows derives every additional + // instance's DACL from whoever created the first one, so attaching to a + // pre-existing pipe would silently inherit a (possibly hostile) DACL. Claim + // the first instance with FILE_FLAG_FIRST_PIPE_INSTANCE; once we own it, + // subsequent instances must drop the flag (the first instance still exists). + let mut first_instance = true; + loop { - // Create a new pipe instance for each connection - let server = match ServerOptions::new() - .first_pipe_instance(false) - .create(CONTROL_PIPE_NAME) - { + // Create a new pipe instance for each connection. The SECURITY_ATTRIBUTES + // pointer is derived and consumed entirely within this (await-free) block + // so it is never held across an await point (which would make the task + // non-Send). sa_ptr is null (default DACL) or points to pipe_security's + // SECURITY_ATTRIBUTES, which outlives the call. + let created = { + let sa_ptr = pipe_security + .as_mut() + .map(|s| s.as_attrs_ptr()) + .unwrap_or(std::ptr::null_mut()); + unsafe { + ServerOptions::new() + .first_pipe_instance(first_instance) + .create_with_security_attributes_raw(CONTROL_PIPE_NAME, sa_ptr) + } + }; + let server = match created { Ok(s) => s, Err(e) => { - // If this is the very first instance, try with first_pipe_instance(true) - match ServerOptions::new() - .first_pipe_instance(true) - .create(CONTROL_PIPE_NAME) - { - Ok(s) => s, - Err(e2) => { - error!("Failed to create control pipe: {} / {}", e, e2); - tokio::time::sleep(std::time::Duration::from_secs(1)).await; - continue; - } + if first_instance { + // SEC-01: fail closed. We could not claim the first instance, + // so another process already owns the name and would dictate + // the DACL for any instance we attach to. Refuse rather than + // inherit a foreign descriptor. + error!( + "Refusing to attach to existing control pipe (could not claim first instance): {}", + e + ); + return; } + error!("Failed to create control pipe: {}", e); + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + continue; } }; + // We own the first instance; further instances must not set the flag. + first_instance = false; // Wait for a client to connect, or cancellation tokio::select! { @@ -150,9 +296,9 @@ pub async fn start_control_server( } } - // Read one line from the client + // Read one line from the client (bounded; EH-08) let (reader, mut writer) = tokio::io::split(server); - let mut buf_reader = BufReader::new(reader); + let mut buf_reader = BufReader::new(reader.take(MAX_CONTROL_LINE_BYTES)); let mut line = String::new(); match buf_reader.read_line(&mut line).await { @@ -247,7 +393,7 @@ pub async fn start_control_server( cancel: tokio_util::sync::CancellationToken, ) { use std::os::unix::fs::PermissionsExt; - use tokio::io::{AsyncBufReadExt, BufReader}; + use tokio::io::{AsyncBufReadExt, AsyncReadExt, BufReader}; use tokio::net::UnixListener; let path = match sshwarden_config::default_control_socket_path() { @@ -312,10 +458,31 @@ pub async fn start_control_server( } }; + // SEC-01: only the owning user may drive the control channel. The 0600 + // socket already blocks other users at the filesystem layer; this is a + // defence-in-depth check rejecting any peer whose uid differs from ours + // (e.g. if the socket permissions were somehow widened). + match stream.peer_cred() { + Ok(cred) => { + let our_uid = unsafe { libc::geteuid() }; + if cred.uid() != our_uid { + error!( + peer_uid = cred.uid(), + our_uid, "Rejecting control connection from a different uid" + ); + continue; + } + } + Err(e) => { + error!(error = %e, "Could not read control peer credentials; rejecting"); + continue; + } + } + let tx = tx.clone(); tokio::spawn(async move { let (reader, mut writer) = tokio::io::split(stream); - let mut buf_reader = BufReader::new(reader); + let mut buf_reader = BufReader::new(reader.take(MAX_CONTROL_LINE_BYTES)); let mut line = String::new(); match buf_reader.read_line(&mut line).await { diff --git a/crates/sshwarden-agent/src/named_pipe_listener_stream.rs b/crates/sshwarden-agent/src/named_pipe_listener_stream.rs index 39f197e..90834dc 100644 --- a/crates/sshwarden-agent/src/named_pipe_listener_stream.rs +++ b/crates/sshwarden-agent/src/named_pipe_listener_stream.rs @@ -28,11 +28,11 @@ pub struct NamedPipeServerStream { } impl NamedPipeServerStream { - #[allow(clippy::unwrap_used)] pub fn new( endpoint: Option, cancellation_token: CancellationToken, is_running: Arc, + fatal_tx: Arc>>, ) -> Self { let (tx, rx) = tokio::sync::mpsc::channel(16); tokio::spawn(async move { @@ -42,12 +42,28 @@ impl NamedPipeServerStream { .unwrap_or(PIPE_NAME) .to_string(); info!("Creating named pipe server on {}", pipe_name); - let mut listener = match ServerOptions::new().create(&pipe_name) { + // RT-01: claim exclusive ownership of the endpoint with + // first_pipe_instance(true). If another SSH agent (most commonly the + // Windows OpenSSH `ssh-agent` service) already owns the pipe, create() + // fails here instead of silently coexisting as a second instance and + // stealing half of the client connections. + let mut listener = match ServerOptions::new() + .first_pipe_instance(true) + .create(&pipe_name) + { Ok(pipe) => pipe, Err(e) => { - error!(error = %e, "Encountered an error creating the first pipe. The system's openssh service must likely be disabled"); - cancellation_token.cancel(); + let reason = format!( + "Failed to claim SSH agent endpoint {pipe_name}: {e}. Another SSH agent \ + already owns it (most likely the Windows OpenSSH 'ssh-agent' service). \ + Stop and disable it (admin PowerShell: Stop-Service ssh-agent; \ + Set-Service ssh-agent -StartupType Disabled), or set [socket] path to a \ + custom endpoint, then restart SSHWarden." + ); + error!(error = %e, "{reason}"); is_running.store(false, Ordering::Relaxed); + cancellation_token.cancel(); + let _ = fatal_tx.send(Some(reason)); return; } }; @@ -78,14 +94,22 @@ impl NamedPipeServerStream { Ok(info) => info, }; - tx.send((listener, peer_info)).await.unwrap(); + if tx.send((listener, peer_info)).await.is_err() { + // Consumer (the served stream) was dropped, e.g. the + // agent is stopping. Exit the listener cleanly. + return; + } listener = match ServerOptions::new().create(&pipe_name) { Ok(pipe) => pipe, Err(e) => { - error!(error = %e, "Encountered an error creating a new pipe"); - cancellation_token.cancel(); + let reason = format!( + "SSH agent pipe {pipe_name} could not be recreated: {e}" + ); + error!(error = %e, "{reason}"); is_running.store(false, Ordering::Relaxed); + cancellation_token.cancel(); + let _ = fatal_tx.send(Some(reason)); return; } }; diff --git a/crates/sshwarden-agent/src/request_parser.rs b/crates/sshwarden-agent/src/request_parser.rs index fd5b39a..1062d58 100644 --- a/crates/sshwarden-agent/src/request_parser.rs +++ b/crates/sshwarden-agent/src/request_parser.rs @@ -52,12 +52,25 @@ fn operation_kind_from_namespace(namespace: &str) -> OperationKind { } pub(crate) fn parse_request(data: &[u8]) -> Result { - let mut data = Bytes::copy_from_slice(data); let magic_header = "SSHSIG"; + + // A plain SSH signature request is just the raw data to be signed; only + // SSHSIG blobs (git/file signing) carry a magic header + namespace. Any + // payload shorter than the magic header cannot be an SSHSIG blob, so treat + // it as a plain signature request rather than panicking in `split_to`. + if data.len() < magic_header.len() { + return Ok(SshAgentSignRequest::SignRequest(SignRequest {})); + } + + let mut data = Bytes::copy_from_slice(data); let header = data.split_to(magic_header.len()); if header == magic_header.as_bytes() { - let _version = data.get_u32(); + // SSHSIG = magic || u32 version || ... — guard the version read so a + // truncated blob returns an error instead of panicking in `get_u32`. + let _version = data + .try_get_u32() + .map_err(|_| anyhow::anyhow!("Truncated SSHSIG request: missing version"))?; let namespace = data .into_iter() @@ -73,3 +86,61 @@ pub(crate) fn parse_request(data: &[u8]) -> Result assert_eq!(req.namespace, "git"), + other => panic!("expected SshSigRequest, got {other:?}"), + } + } + + #[test] + fn non_sshsig_payload_is_plain_sign_request() { + assert!(matches!( + parse_request(b"not-a-sshsig-blob-just-raw-bytes") + .expect("non-SSHSIG payload should not error"), + SshAgentSignRequest::SignRequest(_) + )); + } +} diff --git a/crates/sshwarden-agent/src/windows.rs b/crates/sshwarden-agent/src/windows.rs index 7f4dd8c..1c943e2 100644 --- a/crates/sshwarden-agent/src/windows.rs +++ b/crates/sshwarden-agent/src/windows.rs @@ -17,6 +17,7 @@ impl SshWardenAgent { endpoint, agent_state.cancellation_token(), agent_state.is_running_flag(), + agent_state.fatal_tx(), ); let cloned_agent_state = agent_state.clone(); diff --git a/crates/sshwarden-api/src/client.rs b/crates/sshwarden-api/src/client.rs index f961cf1..960c3a5 100644 --- a/crates/sshwarden-api/src/client.rs +++ b/crates/sshwarden-api/src/client.rs @@ -19,6 +19,21 @@ pub struct BitwardenClient { user_key: Option, } +impl Drop for BitwardenClient { + fn drop(&mut self) { + // SEC-05: scrub bearer tokens from memory when the client is dropped + // (e.g. on logout/forget/lock). Defence-in-depth alongside the + // ZeroizeOnDrop SymmetricKey user_key. + use zeroize::Zeroize; + if let Some(token) = self.access_token.as_mut() { + token.zeroize(); + } + if let Some(token) = self.refresh_token.as_mut() { + token.zeroize(); + } + } +} + /// A decrypted SSH key ready for use by the agent. /// The private key PEM is wrapped in `Zeroizing` for automatic memory cleanup. #[derive(Debug, Clone)] @@ -159,8 +174,17 @@ impl BitwardenClient { let token_resp: TokenResponse = serde_json::from_str(&body).context("Failed to parse token response")?; - self.access_token = Some(token_resp.access_token); - self.refresh_token = token_resp.refresh_token.clone(); + self.set_access_token(token_resp.access_token); + { + // SEC-05: scrub any previous refresh token before replacing it. + use zeroize::Zeroize; + if let Some(old) = self.refresh_token.as_mut() { + old.zeroize(); + } + } + // Move (not clone) so no extra plaintext copy of the freshly issued token + // lingers in token_resp on the heap until it drops. + self.refresh_token = token_resp.refresh_token; self.token_expiry = Some(std::time::Instant::now() + std::time::Duration::from_secs(token_resp.expires_in)); @@ -376,8 +400,23 @@ impl BitwardenClient { .map(ToOwned::to_owned)) } + /// Replace the access token, scrubbing the previous value first (SEC-05) so + /// rotated tokens don't linger on the heap (Drop only covers the final value). + fn set_access_token(&mut self, token: String) { + use zeroize::Zeroize; + if let Some(old) = self.access_token.as_mut() { + old.zeroize(); + } + self.access_token = Some(token); + } + /// Set the refresh token (e.g., restored from session file). pub fn set_refresh_token(&mut self, token: String) { + // SEC-05: scrub any previous refresh token before overwriting it. + use zeroize::Zeroize; + if let Some(old) = self.refresh_token.as_mut() { + old.zeroize(); + } self.refresh_token = Some(token); } @@ -437,9 +476,9 @@ impl BitwardenClient { let token_resp: TokenResponse = serde_json::from_str(&body).context("Failed to parse token refresh response")?; - self.access_token = Some(token_resp.access_token); + self.set_access_token(token_resp.access_token); if let Some(new_refresh) = token_resp.refresh_token { - self.refresh_token = Some(new_refresh); + self.set_refresh_token(new_refresh); } self.token_expiry = Some(std::time::Instant::now() + std::time::Duration::from_secs(token_resp.expires_in)); diff --git a/crates/sshwarden-api/src/crypto.rs b/crates/sshwarden-api/src/crypto.rs index 62292d7..daa4466 100644 --- a/crates/sshwarden-api/src/crypto.rs +++ b/crates/sshwarden-api/src/crypto.rs @@ -255,21 +255,37 @@ pub fn encrypt_enc_string(data: &[u8], key: &SymmetricKey) -> anyhow::Result anyhow::Result { +/// Retained so pre-v3 local caches and the legacy `vault.enc` (both encrypted +/// with this salt) stay decryptable; new material uses `random_pin_salt()`. +pub fn legacy_pin_salt() -> [u8; 32] { use sha2::Digest; - let salt = Sha256::digest(b"sshwarden-pin-key-derivation"); + let digest = Sha256::digest(b"sshwarden-pin-key-derivation"); + let mut salt = [0u8; 32]; + salt.copy_from_slice(&digest); + salt +} + +/// A fresh random 16-byte salt for PIN key derivation. Stored alongside the +/// PIN-encrypted material so an attacker cannot precompute a single Argon2id +/// table against every user's cache (the 4-character PIN keyspace is small). +pub fn random_pin_salt() -> [u8; 16] { + use rand::RngCore; + let mut salt = [0u8; 16]; + rand::thread_rng().fill_bytes(&mut salt); + salt +} +/// Derive a SymmetricKey from a PIN and an explicit salt using Argon2id. +pub fn derive_pin_key_with_salt(pin: &str, salt: &[u8]) -> anyhow::Result { let params = argon2::Params::new(64 * 1024, 3, 1, Some(64)) .map_err(|e| anyhow!("Invalid Argon2 params: {e}"))?; let argon2 = argon2::Argon2::new(argon2::Algorithm::Argon2id, argon2::Version::V0x13, params); let mut key_material = Zeroizing::new(vec![0u8; 64]); argon2 - .hash_password_into(pin.as_bytes(), &salt, &mut key_material) + .hash_password_into(pin.as_bytes(), salt, &mut key_material) .map_err(|e| anyhow!("Argon2 PIN derivation failed: {e}"))?; Ok(SymmetricKey { @@ -278,6 +294,14 @@ pub fn derive_pin_key(pin: &str) -> anyhow::Result { }) } +/// Derive a SymmetricKey from a PIN using the fixed legacy salt. +/// +/// Backward-compatibility shim for the legacy `vault.enc` and pre-v3 caches. +/// New code should use `derive_pin_key_with_salt` with a random salt. +pub fn derive_pin_key(pin: &str) -> anyhow::Result { + derive_pin_key_with_salt(pin, &legacy_pin_salt()) +} + pub fn random_symmetric_key() -> SymmetricKey { use rand::RngCore; let mut enc_key = vec![0u8; 32]; @@ -316,19 +340,29 @@ pub fn decode_symmetric_key(encoded: &str) -> anyhow::Result { Ok(key) } -/// Encrypt a string with a PIN-derived key. -pub fn pin_encrypt(data: &str, pin: &str) -> anyhow::Result { - let key = derive_pin_key(pin)?; +/// Encrypt a string with a PIN-derived key using an explicit salt (SEC-04). +pub fn pin_encrypt_with_salt(data: &str, pin: &str, salt: &[u8]) -> anyhow::Result { + let key = derive_pin_key_with_salt(pin, salt)?; encrypt_enc_string(data.as_bytes(), &key) } -/// Decrypt a string with a PIN-derived key. -pub fn pin_decrypt(enc_string: &str, pin: &str) -> anyhow::Result { - let key = derive_pin_key(pin)?; +/// Decrypt a string with a PIN-derived key using an explicit salt (SEC-04). +pub fn pin_decrypt_with_salt(enc_string: &str, pin: &str, salt: &[u8]) -> anyhow::Result { + let key = derive_pin_key_with_salt(pin, salt)?; let bytes = decrypt_enc_string(enc_string, &key)?; String::from_utf8(bytes).context("PIN-decrypted data is not valid UTF-8") } +/// Encrypt a string with a PIN-derived key using the fixed legacy salt. +pub fn pin_encrypt(data: &str, pin: &str) -> anyhow::Result { + pin_encrypt_with_salt(data, pin, &legacy_pin_salt()) +} + +/// Decrypt a string with a PIN-derived key using the fixed legacy salt. +pub fn pin_decrypt(enc_string: &str, pin: &str) -> anyhow::Result { + pin_decrypt_with_salt(enc_string, pin, &legacy_pin_salt()) +} + #[cfg(test)] #[allow(clippy::unwrap_used)] mod tests { @@ -395,4 +429,31 @@ mod tests { let decrypted = decrypt_enc_string(&encrypted, &key).unwrap(); assert_eq!(decrypted, original); } + + #[test] + fn test_pin_salt_roundtrip_and_legacy_compat() { + let data = r#"[["k","n","i"]]"#; + let pin = "1234"; + let salt = random_pin_salt(); + + // Round-trip with an explicit random salt. + let enc = pin_encrypt_with_salt(data, pin, &salt).unwrap(); + assert_eq!(pin_decrypt_with_salt(&enc, pin, &salt).unwrap(), data); + + // A different salt yields a different key, so decryption fails. + let other = random_pin_salt(); + assert_ne!(salt, other); + assert!(pin_decrypt_with_salt(&enc, pin, &other).is_err()); + + // The legacy wrappers are equivalent to using the legacy salt explicitly, + // so existing vault.enc / pre-v3 caches stay decryptable. + let legacy = pin_encrypt(data, pin).unwrap(); + assert_eq!( + pin_decrypt_with_salt(&legacy, pin, &legacy_pin_salt()).unwrap(), + data + ); + // ...and a legacy ciphertext is NOT decryptable with a random salt, + // confirming the v2->v3 migration boundary is real. + assert!(pin_decrypt_with_salt(&legacy, pin, &random_pin_salt()).is_err()); + } } diff --git a/crates/sshwarden-config/src/cache.rs b/crates/sshwarden-config/src/cache.rs index 57db72b..5a82950 100644 --- a/crates/sshwarden-config/src/cache.rs +++ b/crates/sshwarden-config/src/cache.rs @@ -32,6 +32,11 @@ pub struct KeyIdentity { pub struct LocalCacheKeySlots { #[serde(skip_serializing_if = "Option::is_none")] pub pin_encrypted: Option, + /// Base64-encoded random salt for PIN key derivation (format v3+). When + /// absent (pre-v3 caches), the fixed legacy salt is used; a successful + /// unlock transparently re-saves with a fresh random salt (SEC-04). + #[serde(skip_serializing_if = "Option::is_none")] + pub pin_salt: Option, #[serde(skip_serializing_if = "Option::is_none")] pub hello_challenge: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -41,6 +46,10 @@ pub struct LocalCacheKeySlots { } impl LocalKeyCacheFile { + /// Supported on-disk format versions. v2 is the original envelope format; + /// v3 adds a per-cache random PIN salt (`LocalCacheKeySlots::pin_salt`). + const SUPPORTED_VERSIONS: &'static [u32] = &[2, 3]; + pub fn path() -> anyhow::Result { Ok(crate::config_dir()?.join("local-key-cache.json")) } @@ -54,6 +63,14 @@ impl LocalKeyCacheFile { .with_context(|| format!("Failed to read local key cache: {}", path.display()))?; let cache: LocalKeyCacheFile = serde_json::from_str(&content) .with_context(|| format!("Failed to parse local key cache: {}", path.display()))?; + if !Self::SUPPORTED_VERSIONS.contains(&cache.version) { + anyhow::bail!( + "Unsupported local key cache version {} (supported: {:?}): {}", + cache.version, + Self::SUPPORTED_VERSIONS, + path.display() + ); + } Ok(Some(cache)) } diff --git a/crates/sshwarden-config/src/session.rs b/crates/sshwarden-config/src/session.rs index 699dc46..1b066ac 100644 --- a/crates/sshwarden-config/src/session.rs +++ b/crates/sshwarden-config/src/session.rs @@ -32,6 +32,9 @@ pub struct SessionFile { } 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` pub fn path() -> anyhow::Result { let hostname = hostname(); @@ -48,6 +51,14 @@ impl SessionFile { .with_context(|| format!("Failed to read session file: {}", path.display()))?; let session: SessionFile = serde_json::from_str(&content) .with_context(|| format!("Failed to parse session file: {}", path.display()))?; + if !Self::SUPPORTED_VERSIONS.contains(&session.version) { + anyhow::bail!( + "Unsupported session file version {} (supported: {:?}): {}", + session.version, + Self::SUPPORTED_VERSIONS, + path.display() + ); + } Ok(Some(session)) } diff --git a/crates/sshwarden-config/src/vault.rs b/crates/sshwarden-config/src/vault.rs index b1a6065..3fae7fa 100644 --- a/crates/sshwarden-config/src/vault.rs +++ b/crates/sshwarden-config/src/vault.rs @@ -27,6 +27,9 @@ pub struct VaultFile { } impl VaultFile { + /// Supported on-disk format versions for the legacy vault file. + const SUPPORTED_VERSIONS: &'static [u32] = &[1]; + /// Path to the vault file (alongside the executable). pub fn path() -> anyhow::Result { Ok(crate::config_dir()?.join("vault.enc")) @@ -42,22 +45,15 @@ impl VaultFile { .with_context(|| format!("Failed to read vault file: {}", path.display()))?; let vault: VaultFile = serde_json::from_str(&content) .with_context(|| format!("Failed to parse vault file: {}", path.display()))?; - Ok(Some(vault)) - } - - /// Save the vault file to disk, creating the parent directory if needed. - 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 vault directory: {}", parent.display()) - })?; + if !Self::SUPPORTED_VERSIONS.contains(&vault.version) { + anyhow::bail!( + "Unsupported vault file version {} (supported: {:?}): {}", + vault.version, + Self::SUPPORTED_VERSIONS, + path.display() + ); } - let content = - serde_json::to_string_pretty(self).context("Failed to serialize vault file")?; - std::fs::write(&path, content) - .with_context(|| format!("Failed to write vault file: {}", path.display()))?; - Ok(()) + Ok(Some(vault)) } /// Delete the vault file from disk (if it exists). diff --git a/crates/sshwarden-ui/src/unlock/native.rs b/crates/sshwarden-ui/src/unlock/native.rs index d3e2594..dc37157 100644 --- a/crates/sshwarden-ui/src/unlock/native.rs +++ b/crates/sshwarden-ui/src/unlock/native.rs @@ -10,7 +10,14 @@ mod platform { const ACCOUNT: &str = "SSHWarden"; pub fn native_available() -> bool { - true + // XP-2: the macOS path reads/writes the login Keychain with no + // user-presence ceremony (Touch ID / password), which does not satisfy + // ADR-0015. Until a SecAccessControl (kSecAccessControlUserPresence) + + // LAContext ceremony is implemented and verified on real hardware, + // disable native unlock so SSHWarden falls back to PIN/password instead + // of silently using the Keychain. + // TODO(XP-2): implement the user-presence ceremony, then return true. + false } pub fn native_encrypt_local_cache_key(encoded_local_cache_key: &str) -> Result { @@ -91,9 +98,12 @@ mod platform { pub fn native_delete_local_cache_key(_slot: Option<&str>) -> Result<()> { let Some(secret_tool) = which_secret_tool() else { + // No keyring tool present — nothing was stored, nothing to delete. return Ok(()); }; - let _ = std::process::Command::new(&secret_tool) + // XP-5: check the exit status so a failed revocation is reported (and + // surfaced by `forget`) instead of silently claiming success. + let status = std::process::Command::new(&secret_tool) .args([ "clear", "application", @@ -101,7 +111,13 @@ mod platform { "kind", "local-cache-key", ]) - .status(); + .status() + .map_err(|e| anyhow!("secret-tool clear failed to run: {e}"))?; + if !status.success() { + return Err(anyhow!( + "secret-tool clear exited unsuccessfully; native unlock material may remain in the keyring" + )); + } Ok(()) } diff --git a/llmdoc/guides/how-to-use-cli-commands.md b/llmdoc/guides/how-to-use-cli-commands.md index 2deeeb2..73e4d27 100644 --- a/llmdoc/guides/how-to-use-cli-commands.md +++ b/llmdoc/guides/how-to-use-cli-commands.md @@ -1,8 +1,10 @@ # How to Use SSHWarden CLI Commands -SSHWarden 提供守护进程模式和多个 CLI 子命令。守护进程处理 SSH Agent 请求，CLI 子命令通过 IPC 控制通道与守护进程通信。支持三种解锁路径：Windows Hello 签名、PIN、主密码。所有数据文件（config.toml、vault.enc、sshwarden.log、sshwarden.pid）均存放在 exe 所在目录（完全便携模式）。 +SSHWarden 提供守护进程模式和多个 CLI 子命令。守护进程处理 SSH Agent 请求，CLI 子命令通过 IPC 控制通道与守护进程通信。支持三种解锁路径：Windows Hello 签名、PIN、主密码。 -1. **启动守护进程:** 运行 `sshwarden`（无子命令）。若 exe 同目录下存在 vault.enc 文件，守护进程直接启动并进入锁定状态，等待 Hello/PIN/Password 解锁；若无 vault.enc，程序提示输入 Bitwarden 主密码，登录后加载 SSH 密钥。确认输出 `SSH Agent is running` 即表示就绪。参见 `src/main.rs:323-504`. +**数据目录解析（4 层优先级，见 `crates/sshwarden-config/src/lib.rs` `resolve_data_dir`）：** ① 环境变量 `SSHWARDEN_HOME`；② `SSHWARDEN_PORTABLE=1` → exe 所在目录；③ exe 同目录 `config.toml` 中 `[storage] portable = true`（可选 `portable_dir`）→ 便携目录；④ **默认：平台标准目录**（Windows `%APPDATA%\SSHWarden`、Linux `$XDG_CONFIG_HOME/sshwarden` 或 `~/.config/sshwarden`、macOS `~/Library/Application Support/SSHWarden`）。所有数据文件（`config.toml`、`local-key-cache.json`、旧版 `vault.enc`、`sshwarden.log`、`sshwarden.pid`、`session-*.enc`）都存放在解析出的该目录下。运行 `sshwarden status` 可查看实际解析出的 `data_dir`。 + +1. **启动守护进程:** 运行 `sshwarden`（无子命令）。若数据目录下存在 `local-key-cache.json`（或旧版 `vault.enc`），守护进程直接启动并进入锁定状态，等待 Hello/PIN/Password 解锁；若无缓存，程序提示输入 Bitwarden 主密码，登录后加载 SSH 密钥。确认输出 `SSH Agent is running` 即表示就绪。参见 `src/main.rs` `run_foreground`. 2. **查看状态:** 运行 `sshwarden status`。显示锁定状态（locked/unlocked）、密钥数量、是否配置 PIN、是否有 vault.enc 文件。若守护进程未运行则提示连接失败。参见 `src/main.rs:151`. diff --git a/llmdoc/reference/ipc-control-protocol.md b/llmdoc/reference/ipc-control-protocol.md index 76afeef..7929261 100644 --- a/llmdoc/reference/ipc-control-protocol.md +++ b/llmdoc/reference/ipc-control-protocol.md @@ -2,15 +2,15 @@ ## 1. Core Summary -SSHWarden 守护进程在 `\\.\pipe\sshwarden-control` Named Pipe 上监听 JSON 控制命令。客户端连接后发送一行 JSON，守护进程处理后回写一行 JSON 响应并关闭连接。协议为单命令-单响应模型。支持 8 种命令，涵盖锁定/解锁（3 种路径）/状态查询/同步/PIN 设置。 +SSHWarden 守护进程在 `\\.\pipe\sshwarden-control` Named Pipe（非 Windows：运行时目录下的 Unix socket，权限 0600）上监听 JSON 控制命令。客户端连接后发送一行 JSON，守护进程处理后回写一行 JSON 响应并关闭连接。协议为单命令-单响应模型。支持 12 种命令字符串，涵盖锁定/解锁（自动/Hello/原生/PIN/主密码）/状态（人类可读与 JSON）/同步/遗忘/PIN 设置/主机绑定对话框。守护进程仅接受同一用户的连接（Unix 校验 peer uid；Windows 管道 DACL 限定当前用户 + SYSTEM）。 ## 2. Source of Truth -- **Primary Code:** `crates/sshwarden-agent/src/control.rs` -- 完整的 IPC 服务端和客户端实现，包含数据结构定义。 -- **Business Logic:** `src/main.rs:508-943` (`handle_control_command`) -- 各命令的具体处理逻辑。 -- **Vault Persistence:** `crates/sshwarden-config/src/vault.rs` (`VaultFile`) -- vault.enc 文件读写。 +- **Primary Code:** `crates/sshwarden-agent/src/control.rs` -- 完整的 IPC 服务端和客户端实现，包含数据结构定义、调用方鉴权与命令分发。 +- **Business Logic:** `src/main.rs` (`handle_control_command`) -- 各命令的具体处理逻辑。 +- **Cache Persistence:** `crates/sshwarden-config/src/cache.rs` (`LocalKeyCacheFile`，信封格式 v3，含 `pin_salt`) 与旧版 `crates/sshwarden-config/src/vault.rs` (`VaultFile`)。 - **Hello Crypto:** `crates/sshwarden-ui/src/unlock/hello_crypto.rs` -- Hello 签名路径加解密。 -- **Configuration:** `crates/sshwarden-config/src/lib.rs` -- 相关配置项（`lock_timeout`, `auto_unlock_on_request`）及便携路径解析（`config_dir()` 基于 exe 所在目录）。 +- **Configuration:** `crates/sshwarden-config/src/lib.rs` -- 相关配置项（`lock_timeout`, `auto_unlock_on_request`）及数据目录解析（`config_dir()` 默认平台标准目录，便携模式可选，见 `resolve_data_dir`）。 - **Related Architecture:** `/llmdoc/architecture/ipc-control-channel.md` -- IPC 通道架构文档。 ## 3. Protocol Details @@ -27,16 +27,23 @@ SSHWarden 守护进程在 `\\.\pipe\sshwarden-control` Named Pipe 上监听 JSON ### Command List +`dispatch_control_command` (`crates/sshwarden-agent/src/control.rs`) maps 12 +command strings to `ControlAction` variants: + | Command | ControlAction | Description | |---|---|---| | `lock` | `Lock` | 清除私钥，锁定密码库 | -| `unlock` | `Unlock` | 自动解锁：优先 Hello 签名路径 -> 降级 Hello UV -> 从内存缓存重载 | -| `unlock-hello` | `UnlockHello` | 仅 Hello 签名路径解锁（需 vault.enc 含 hello_challenge） | -| `unlock-pin:{pin}` | `UnlockPin { pin }` | PIN 解密密钥缓存后重载（优先内存，降级 vault.enc） | +| `unlock` | `Unlock` | 自动解锁：原生缓存 -> Hello 信封 -> 旧 Hello 签名路径 -> PIN 对话框 | +| `unlock-hello` | `UnlockHello` | 仅 Hello 路径解锁（需缓存含 hello_challenge/hello_encrypted） | +| `unlock-native` | `UnlockNative` | 仅平台原生（Keychain/Secret Service/DPAPI）信封解锁 | +| `unlock-pin:{pin}` | `UnlockPin { pin }` | PIN 解密密钥缓存后重载（信封缓存优先，降级旧 vault.enc）；带失败延迟/锁定 | | `unlock-password:{password}` | `UnlockPassword { password }` | 主密码重新登录 Bitwarden 并同步密钥 | -| `status` | `Status` | 返回锁定状态、密钥数量、PIN/vault.enc 状态 | -| `sync` | `Sync` | 重新同步 Bitwarden 密码库（需已认证） | -| `set-pin:{pin}` | `SetPin { pin }` | 用 PIN 加密当前密钥缓存，持久化到 vault.enc，可选注册 Hello 签名路径 | +| `status` | `Status { json: false }` | 返回锁定状态、密钥数量等（人类可读 message） | +| `status-json` | `Status { json: true }` | 同上，仅返回 `details` JSON | +| `sync` | `Sync` | 重新同步 Bitwarden 密码库（需已认证）；锁定时仅刷新缓存并置 pending_sync | +| `forget` | `Forget` | 删除本地密钥缓存/会话材料并清空 agent | +| `set-pin:{pin}` | `SetPin { pin }` | 用 PIN（随机盐，格式 v3）加密当前密钥缓存并持久化，可选注册 Hello/原生 | +| `bind-hosts-dialog` | `BindHostsDialog` | 打开主机绑定管理对话框，对话框关闭后返回 | ### Response Format @@ -46,14 +53,15 @@ SSHWarden 守护进程在 `\\.\pipe\sshwarden-control` Named Pipe 上监听 JSON "message": "optional message", "error": "optional error (when ok=false)", "locked": true, - "key_count": 3 + "key_count": 3, + "details": { "...": "Status/status-json 的结构化字段" } } ``` - `ok`: 操作是否成功。 -- `message`: 成功时的描述信息。Status 命令附加 PIN/vault.enc 状态。 +- `message`: 成功时的描述信息。 - `error`: 失败时的错误描述。 -- `locked`: 仅 `status` 命令返回，当前锁定状态。 -- `key_count`: 仅 `status` 命令返回，当前加载的密钥数量。 +- `locked` / `key_count`: 仅 `status` 命令返回。 +- `details`: 仅 `status`/`status-json` 返回，含 `locked`、`key_count`、`signable_key_count`（可签名密钥数，锁定时为 0）、`agent_running`（SSH 端点是否在服务）、`has_pin`、`has_vault_file`、`has_local_key_cache`、`legacy_migration_available`、`authenticated`、`pending_sync`、`data_dir`（解析出的数据目录）、`notification`。 所有字段除 `ok` 外均为 optional（`#[serde(skip_serializing_if = "Option::is_none")]`）。 diff --git a/src/main.rs b/src/main.rs index e544b8d..d5a768e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,7 +1,6 @@ use std::sync::Arc; use anyhow::Context; -#[cfg(windows)] use base64::Engine; use clap::{Parser, Subcommand}; use tokio::sync::RwLock; @@ -366,22 +365,31 @@ fn main() -> anyhow::Result<()> { let ui_tx = ui_request_tx.clone(); let tokio_handle = std::thread::spawn(move || -> anyhow::Result<()> { rt.block_on(async move { - if is_daemon_mode { - if is_daemon_running() { - info!("SSHWarden daemon is already running"); + // RT-02: single-instance guard for ANY run_foreground entry — + // both `sshwarden daemon` and a bare foreground `sshwarden` are + // the one singleton daemon (one agent + one control server + one + // OpenSSH endpoint); two cannot coexist. Claim the PID file + // atomically so two near-simultaneous starters cannot both pass a + // check-then-write gate and spin up competing agents/pipes. + match claim_pid_file() { + Ok(true) => {} + Ok(false) => { + info!("SSHWarden is already running"); return Ok(()); } - #[cfg(windows)] + Err(e) => return Err(e), + } + #[cfg(windows)] + if is_daemon_mode { detach_console(); - - write_pid_file()?; - info!("SSHWarden daemon started (PID: {})", std::process::id()); - let result = run_foreground(config, ui_tx).await; - remove_pid_file(); - result - } else { - run_foreground(config, ui_tx).await } + #[cfg(not(windows))] + let _ = is_daemon_mode; + + info!("SSHWarden started (PID: {})", std::process::id()); + let result = run_foreground(config, ui_tx).await; + remove_pid_file(); + result }) }); @@ -469,9 +477,9 @@ fn main() -> anyhow::Result<()> { let path = sshwarden_config::config_path()?; if !path.exists() { config.save()?; - info!("Created default config at: {}", path.display()); + out_line(format!("Created default config at: {}", path.display())); } else { - info!("Config file: {}", path.display()); + out_line(format!("Config file: {}", path.display())); } Ok(()) } @@ -546,6 +554,20 @@ fn run_slint_event_loop(mut ui_request_rx: tokio::sync::mpsc::Receiver anyhow::Result<()> { match sshwarden_agent::control::send_control_command(cmd).await { Ok(response) => { @@ -556,41 +578,38 @@ async fn cmd_control(cmd: &str) -> anyhow::Result<()> { .as_ref() .cloned() .unwrap_or_else(|| serde_json::to_value(&response).unwrap_or_default()); - #[allow(clippy::print_stdout)] - { - println!("{}", serde_json::to_string_pretty(&value)?); - } + out_line(serde_json::to_string_pretty(&value)?); return Ok(()); } if let Some(msg) = &response.message { - info!("{}", msg); + out_line(msg); } if let Some(locked) = response.locked { - info!(" Locked: {}", locked); + out_line(format!(" Locked: {locked}")); } if let Some(count) = response.key_count { - info!(" Keys: {}", count); + out_line(format!(" Keys: {count}")); } if let Some(details) = &response.details { if let Some(notification) = details.get("notification") { - info!(" Notification: {}", notification); + out_line(format!(" Notification: {notification}")); } if let Some(pending) = details.get("pending_sync") { - info!(" Pending sync: {}", pending); + out_line(format!(" Pending sync: {pending}")); } if let Some(authenticated) = details.get("authenticated") { - info!(" Authenticated: {}", authenticated); + out_line(format!(" Authenticated: {authenticated}")); } } } else { let err = response.error.as_deref().unwrap_or("Unknown error"); - info!("Error: {}", err); + err_line(format!("Error: {err}")); } Ok(()) } Err(e) => { - info!("Could not connect to SSHWarden daemon: {}", e); - info!("Is the daemon running? Start it with: sshwarden"); + err_line(format!("Could not connect to SSHWarden daemon: {e}")); + err_line("Is the daemon running? Start it with: sshwarden"); Ok(()) } } @@ -621,7 +640,9 @@ impl DoctorCheck { } } -#[cfg(windows)] +/// Fetch daemon status for the doctor report over the control channel. Works +/// on every platform — the Unix control client is fully implemented, so this is +/// no longer Windows-only (XP-1/UX-6/LOGIC-5). async fn fetch_status_details_for_doctor() -> anyhow::Result { let response = sshwarden_agent::control::send_control_command("status-json").await?; Ok(response @@ -630,11 +651,6 @@ async fn fetch_status_details_for_doctor() -> anyhow::Result .unwrap_or_else(|| serde_json::to_value(&response).unwrap_or_default())) } -#[cfg(not(windows))] -async fn fetch_status_details_for_doctor() -> anyhow::Result { - anyhow::bail!("IPC control is only supported on Windows currently") -} - #[cfg(windows)] fn windows_openssh_pipe_exists() -> bool { std::path::Path::new(r"\\.\pipe\openssh-ssh-agent").exists() @@ -709,6 +725,26 @@ async fn cmd_doctor( }; if let Some(status) = status.as_ref() { + // RT-05/XP-4: detect the "zombie" case where the control channel answers + // but the SSH agent task isn't actually serving the endpoint (e.g. on + // Windows the OpenSSH ssh-agent service owns the pipe). Uses the + // agent_running flag from status, so no extra platform FFI is needed. + let agent_running = status + .get("agent_running") + .and_then(|v| v.as_bool()) + .unwrap_or(true); + if agent_running { + checks.push(DoctorCheck::ok( + "agent.serving", + "SSHWarden's SSH agent task is serving the endpoint", + )); + } else { + checks.push(DoctorCheck::warn( + "agent.serving", + "The control channel answers but SSHWarden's SSH agent is NOT serving the endpoint — another agent likely owns it (on Windows, the OpenSSH 'ssh-agent' service: Stop-Service ssh-agent; Set-Service ssh-agent -StartupType Disabled). ssh/ssh-add will not see SSHWarden's keys until this is fixed.", + )); + } + let authenticated = status .get("authenticated") .and_then(|v| v.as_bool()) @@ -871,16 +907,86 @@ async fn cmd_doctor( if windows_openssh_pipe_exists() { checks.push(DoctorCheck::ok( "agent_endpoint.windows_pipe", - r"Windows OpenSSH agent pipe exists at \\.\pipe\openssh-ssh-agent", + r"OpenSSH agent pipe \\.\pipe\openssh-ssh-agent exists (it may be owned by SSHWarden or by the OS ssh-agent service — see the agent.serving check for which one is actually serving)", )); } else { checks.push(DoctorCheck::warn( "agent_endpoint.windows_pipe", - r"Windows OpenSSH agent pipe is not present at \\.\pipe\openssh-ssh-agent; SSH clients may not be using SSHWarden", + r"No OpenSSH agent pipe at \\.\pipe\openssh-ssh-agent; SSH clients have no agent to talk to", )); } } + #[cfg(not(windows))] + { + // XP-3: check the Unix agent socket exists with 0600 perms and that + // SSH_AUTH_SOCK points at it. + match sshwarden_config::default_agent_socket_path() { + Ok(path) => { + if path.exists() { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + match std::fs::metadata(&path) { + Ok(meta) => { + let mode = meta.permissions().mode() & 0o777; + if mode == 0o600 { + checks.push(DoctorCheck::ok( + "agent_endpoint.unix_socket", + format!( + "Agent socket present with 0600 perms: {}", + path.display() + ), + )); + } else { + checks.push(DoctorCheck::warn( + "agent_endpoint.unix_socket", + format!( + "Agent socket {} has mode {mode:o}, expected 0600", + path.display() + ), + )); + } + } + Err(e) => checks.push(DoctorCheck::warn( + "agent_endpoint.unix_socket", + format!("Could not stat agent socket {}: {e}", path.display()), + )), + } + } + match std::env::var("SSH_AUTH_SOCK") { + Ok(sock) if path.as_path() == std::path::Path::new(&sock) => { + checks.push(DoctorCheck::ok( + "agent_endpoint.ssh_auth_sock", + "SSH_AUTH_SOCK points at the SSHWarden agent socket", + )); + } + Ok(sock) => checks.push(DoctorCheck::warn( + "agent_endpoint.ssh_auth_sock", + format!("SSH_AUTH_SOCK ({sock}) does not point at the SSHWarden agent socket ({}); run `eval \"$(sshwarden env)\"`", path.display()), + )), + Err(_) => checks.push(DoctorCheck::warn( + "agent_endpoint.ssh_auth_sock", + format!("SSH_AUTH_SOCK is not set; run `eval \"$(sshwarden env)\"` so ssh uses {}", path.display()), + )), + } + } else { + checks.push(DoctorCheck::warn( + "agent_endpoint.unix_socket", + format!( + "Agent socket not present at {}; is the daemon running?", + path.display() + ), + )); + } + } + Err(e) => checks.push(DoctorCheck::warn( + "agent_endpoint.unix_socket", + format!("Could not resolve agent socket path: {e}"), + )), + } + } + let include_path = managed_sshwarden_include_path().ok(); let config_path = user_ssh_config_path().ok(); match (include_path, config_path) { @@ -1020,9 +1126,9 @@ async fn cmd_doctor( } else { for check in &checks { if check.ok { - info!("[ok] {}: {}", check.name, check.message); + out_line(format!("[ok] {}: {}", check.name, check.message)); } else { - info!("[warn] {}: {}", check.name, check.message); + out_line(format!("[warn] {}: {}", check.name, check.message)); } } } @@ -1034,7 +1140,7 @@ async fn cmd_doctor( async fn cmd_set_pin() -> anyhow::Result<()> { let pin = prompt_password("Enter new PIN: ")?; if pin.len() < 4 { - info!("PIN must be at least 4 characters"); + err_line("PIN must be at least 4 characters"); return Ok(()); } let pin_confirm = prompt_password("Confirm PIN: ")?; @@ -1089,32 +1195,88 @@ fn create_client( sshwarden_api::BitwardenClient::new(base, &api_url, &identity_url) } -/// Login command: authenticate and list SSH keys. +/// Login command: authenticate and load keys into the running agent. +/// +/// UX-2: routes through the running daemon (via the control channel) so a +/// successful login actually loads keys into the agent serving SSH clients. +/// Falls back to a standalone login that only lists keys if no daemon is up. async fn cmd_login( config: &sshwarden_config::Config, base_url: Option<&str>, email: Option<&str>, ) -> anyhow::Result<()> { + // UX-2: a running daemon logs in with its OWN configured server/account; the + // control protocol carries only the password. If the caller supplied + // --email/--base-url, reject up front — BEFORE prompting — rather than + // silently logging into the wrong account/server (and without wasting a + // password/email prompt the caller can't use). + if (email.is_some() || base_url.is_some()) && is_daemon_running() { + anyhow::bail!( + "A daemon is already running and logs in with its own configured account/server; \ + --email/--base-url cannot be applied to it. Stop the daemon to log in standalone, \ + or omit these flags." + ); + } + + let password = prompt_password("Master password: ")?; + + // Prefer the running daemon: it performs the login + sync and loads keys + // into the live agent. The control protocol carries only the password, so + // the daemon uses its own configured server/email. + match sshwarden_agent::control::send_control_command(&format!("unlock-password:{}", &*password)) + .await + { + Ok(response) => { + if response.ok { + if base_url.is_some() { + info!("Note: the running daemon uses its own configured server; --base-url is ignored."); + } + out_line( + response + .message + .as_deref() + .unwrap_or("Logged in; keys loaded into the running agent."), + ); + } else { + err_line(format!( + "Login failed: {}", + response.error.as_deref().unwrap_or("unknown error") + )); + } + return Ok(()); + } + Err(_) => { + info!( + "Daemon not running; logging in for listing only (start `sshwarden` to serve keys)." + ); + } + } + + // Standalone fallback: authenticate and list keys without touching an agent. + // Resolve the email here (prompting only now) — the daemon path never needs it. let email = match email { Some(e) => e.to_string(), None if !config.auth.email.is_empty() => config.auth.email.clone(), None => prompt_email("Email: ")?, }; - let password = prompt_password("Master password: ")?; - let mut client = create_client(config, base_url); - info!("Logging in as {}...", email); client.login_password(&email, &password).await?; - info!("Login successful!"); let keys = client.sync_ssh_keys().await?; - for key in &keys { - info!(" SSH Key: {} (cipher: {})", key.name, key.cipher_id); - } - if keys.is_empty() { - info!("No SSH keys found in vault. Add SSH keys in Bitwarden to use them."); + out_line("No SSH keys found in vault. Add SSH keys in Bitwarden to use them."); + } else { + out_line("Login successful. Vault SSH keys:"); + for key in &keys { + out_line(format!( + " SSH Key: {} (cipher: {})", + key.name, key.cipher_id + )); + } + out_line( + "\nNote: no daemon was running, so the agent was not loaded. Start `sshwarden`, then `sshwarden unlock --password`.", + ); } Ok(()) @@ -1426,13 +1588,20 @@ fn sync_managed_ssh_config_with_bindings( fn sync_managed_ssh_config_inner(keys: &[ManagedKey], force_write: bool) -> anyhow::Result<()> { let mut bindings = sshwarden_config::bindings::HostBindingsFile::load() .context("Failed to load host bindings")?; - let known_ids: Vec<&str> = keys.iter().map(|k| k.cipher_id.as_str()).collect(); - let pruned = bindings.prune_orphans(known_ids.iter().copied()); - if pruned > 0 { - bindings - .save() - .context("Failed to save host bindings after pruning orphans")?; - info!("Pruned {} orphan host binding(s)", pruned); + // UX-7/data-safety: an empty key set means we have no cache metadata to + // classify orphans against — e.g. binding ahead of first sync (see + // resolve_cipher_id), or the local key cache isn't loaded. Pruning against an + // empty known-set would delete EVERY binding, so only prune when we actually + // know the current key set. + if !keys.is_empty() { + let known_ids: Vec<&str> = keys.iter().map(|k| k.cipher_id.as_str()).collect(); + let pruned = bindings.prune_orphans(known_ids.iter().copied()); + if pruned > 0 { + bindings + .save() + .context("Failed to save host bindings after pruning orphans")?; + info!("Pruned {} orphan host binding(s)", pruned); + } } let include_path = managed_sshwarden_include_path()?; @@ -1593,6 +1762,15 @@ fn write_managed_ssh_config(snippet: &str) -> anyhow::Result<()> { } fn cmd_env(config: &sshwarden_config::Config, shell: &str) -> anyhow::Result<()> { + // UX-5: Windows OpenSSH uses the fixed pipe and ignores SSH_AUTH_SOCK, so + // these exports are usually only relevant for a custom [socket] endpoint or + // third-party clients that honour SSH_AUTH_SOCK. Note goes to stderr so it + // doesn't break `eval`. + #[cfg(windows)] + err_line( + r"# Note: Windows OpenSSH talks to the fixed pipe \\.\pipe\openssh-ssh-agent and ignores SSH_AUTH_SOCK; these exports only matter for a custom [socket] endpoint or clients that honour SSH_AUTH_SOCK.", + ); + let endpoint = config .socket .path @@ -1677,6 +1855,7 @@ fn build_envelope_local_key_cache( server_url: &str, local_cache_key: &sshwarden_api::crypto::SymmetricKey, pin_encrypted: Option, + pin_salt: Option, hello_challenge: Option, hello_encrypted: Option, native_encrypted: Option, @@ -1685,7 +1864,7 @@ fn build_envelope_local_key_cache( let encrypted_payload = sshwarden_api::crypto::encrypt_enc_string(keys_json.as_bytes(), local_cache_key)?; let cache = sshwarden_config::cache::LocalKeyCacheFile { - version: 2, + version: 3, header: sshwarden_config::cache::LocalKeyCacheHeader { email: email.to_string(), server_url: server_url.to_string(), @@ -1694,6 +1873,7 @@ fn build_envelope_local_key_cache( encrypted_payload, local_cache_key: sshwarden_config::cache::LocalCacheKeySlots { pin_encrypted, + pin_salt, hello_challenge, hello_encrypted, native_encrypted, @@ -1712,13 +1892,14 @@ fn write_envelope_local_key_cache( sshwarden_api::crypto::SymmetricKey, )> { let local_cache_key = sshwarden_api::crypto::random_symmetric_key(); - let pin_encrypted = encrypt_local_cache_key_with_pin(&local_cache_key, pin)?; + 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, Some(pin_encrypted), + Some(pin_salt), None, None, None, @@ -1749,6 +1930,7 @@ fn refresh_envelope_local_key_cache( &existing.header.server_url, 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(), @@ -1784,12 +1966,18 @@ fn enroll_hello_for_local_key_cache( Ok(()) } +/// 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( local_cache_key: &sshwarden_api::crypto::SymmetricKey, pin: &str, -) -> anyhow::Result { +) -> anyhow::Result<(String, String)> { let encoded_local_cache_key = sshwarden_api::crypto::encode_symmetric_key(local_cache_key); - sshwarden_api::crypto::pin_encrypt(&encoded_local_cache_key, pin) + let salt = sshwarden_api::crypto::random_pin_salt(); + let pin_encrypted = + sshwarden_api::crypto::pin_encrypt_with_salt(&encoded_local_cache_key, pin, &salt)?; + let pin_salt = base64::engine::general_purpose::STANDARD.encode(salt); + Ok((pin_encrypted, pin_salt)) } #[cfg(windows)] @@ -1864,13 +2052,50 @@ fn decrypt_envelope_local_key_cache_with_pin( .pin_encrypted .as_deref() .context("Local key cache has no PIN unlock slot")?; - let encoded_lck = sshwarden_api::crypto::pin_decrypt(encrypted_lck, pin) - .context("Failed to unlock Local Cache Key with PIN")?; + let encoded_lck = match cache.local_cache_key.pin_salt.as_deref() { + Some(salt_b64) => { + let salt = base64::engine::general_purpose::STANDARD + .decode(salt_b64) + .context("Invalid PIN salt in local key cache")?; + sshwarden_api::crypto::pin_decrypt_with_salt(encrypted_lck, pin, &salt) + } + // Pre-v3 cache: the PIN slot was derived with the fixed legacy salt. + None => sshwarden_api::crypto::pin_decrypt_with_salt( + encrypted_lck, + pin, + &sshwarden_api::crypto::legacy_pin_salt(), + ), + } + .context("Failed to unlock Local Cache Key with PIN")?; let local_cache_key = sshwarden_api::crypto::decode_symmetric_key(&encoded_lck) .context("Failed to decode Local Cache Key")?; decrypt_envelope_payload(cache, local_cache_key) } +/// Transparently migrate a pre-v3 (fixed-salt) cache to v3 with a fresh random +/// PIN salt after a successful PIN unlock (SEC-04). Only the PIN slot is +/// re-wrapped; the local cache key and other slots (Hello/native) are preserved +/// so there is no biometric re-prompt. Best-effort: a failure is logged, not +/// fatal (the unlock itself already succeeded). +fn needs_pin_salt_migration(cache: &sshwarden_config::cache::LocalKeyCacheFile) -> bool { + cache.local_cache_key.pin_encrypted.is_some() + && (cache.version < 3 || cache.local_cache_key.pin_salt.is_none()) +} + +fn migrate_pin_salt_to_v3( + cache: &sshwarden_config::cache::LocalKeyCacheFile, + local_cache_key: &sshwarden_api::crypto::SymmetricKey, + pin: &str, +) -> anyhow::Result { + let (pin_encrypted, pin_salt) = encrypt_local_cache_key_with_pin(local_cache_key, pin)?; + let mut migrated = cache.clone(); + migrated.version = 3; + migrated.local_cache_key.pin_encrypted = Some(pin_encrypted); + migrated.local_cache_key.pin_salt = Some(pin_salt); + migrated.save()?; + Ok(migrated) +} + #[cfg(windows)] fn decrypt_envelope_local_key_cache_with_hello( cache: &sshwarden_config::cache::LocalKeyCacheFile, @@ -2100,21 +2325,28 @@ async fn cmd_bindings_add(key: &str, hosts: &[String]) -> anyhow::Result<()> { bindings.save()?; let keys = load_managed_keys_from_cache().unwrap_or_default(); - if let Err(e) = sync_managed_ssh_config_inner(&keys, true) { - tracing::warn!( - "Bindings saved but managed snippet regeneration failed: {}", - e - ); + // UX-7: a failed snippet regeneration means `ssh host` would route to the + // wrong key (or none), so fail loudly rather than warn-and-continue. + sync_managed_ssh_config_inner(&keys, true) + .context("Bindings saved but managed snippet regeneration failed")?; + + // CFG-2: a binding does nothing unless ~/.ssh/config Includes the managed + // snippet. Auto-install the Include line so `bindings add` is never a silent + // no-op (idempotent — only writes when the line is missing). + let include_path = managed_sshwarden_include_path()?; + let config_path = user_ssh_config_path()?; + if let Err(e) = write_sshwarden_include_line(&config_path, &include_path) { + err_line(format!( + "Binding saved, but failed to ensure the Include line in {}: {e}. Run `sshwarden ssh-config install`.", + config_path.display() + )); } - #[allow(clippy::print_stdout)] - { - println!( - "Added {} host pattern(s) to key {}.", - hosts.len(), - cipher_id - ); - } + out_line(format!( + "Added {} host pattern(s) to key {}.", + hosts.len(), + cipher_id + )); Ok(()) } @@ -2141,17 +2373,10 @@ async fn cmd_bindings_remove(key: &str, host: Option<&str>, all: bool) -> anyhow bindings.save()?; let keys = load_managed_keys_from_cache().unwrap_or_default(); - if let Err(e) = sync_managed_ssh_config_inner(&keys, true) { - tracing::warn!( - "Bindings saved but managed snippet regeneration failed: {}", - e - ); - } + sync_managed_ssh_config_inner(&keys, true) + .context("Bindings saved but managed snippet regeneration failed")?; - #[allow(clippy::print_stdout)] - { - println!("Updated bindings for key {}.", cipher_id); - } + out_line(format!("Updated bindings for key {cipher_id}.")); Ok(()) } @@ -2294,9 +2519,9 @@ async fn cmd_keys( let keys = client.sync_ssh_keys().await?; if keys.is_empty() { - info!("No SSH keys found in vault."); + out_line("No SSH keys found in vault."); } else { - info!("Found {} SSH key(s):", keys.len()); + out_line(format!("Found {} SSH key(s):", keys.len())); for key in &keys { // Show first line of PEM to identify key type let key_type = if key.private_key_pem.as_str().contains("ed25519") { @@ -2306,8 +2531,9 @@ async fn cmd_keys( } else { "SSH" }; - info!(" [{}] {} ({})", key_type, key.name, key.cipher_id); + out_line(format!(" [{}] {} ({})", key_type, key.name, key.cipher_id)); } + out_line("\nNote: this lists vault keys without changing the running agent. Use `sshwarden login` to load them."); } Ok(()) @@ -2392,7 +2618,12 @@ async fn run_foreground( // Create channels for UI communication let (request_tx, mut request_rx) = tokio::sync::mpsc::channel::(32); - let (response_tx, _response_rx) = tokio::sync::broadcast::channel::<(u32, bool)>(32); + // CONC-4 mitigation: a generous capacity makes a broadcast `Lagged` (which + // would otherwise be treated as a denial in confirm()/can_list()) effectively + // impossible under realistic concurrency. The fully race-free fix (per-request + // oneshot registry) needs runtime verification of the signing path and is + // tracked as a follow-up. + let (response_tx, _response_rx) = tokio::sync::broadcast::channel::<(u32, bool)>(256); let response_tx = Arc::new(response_tx); let (runtime_event_tx, mut runtime_event_rx) = tokio::sync::mpsc::channel::(32); @@ -2405,6 +2636,11 @@ async fn run_foreground( ) .context("Failed to start SSH agent server")?; + // RT-01: watch for a fatal agent-transport failure (e.g. the OpenSSH pipe + // could not be claimed) so the daemon shuts down instead of running as a + // zombie that still answers status/unlock while serving no SSH client. + let mut agent_fatal_rx = agent.fatal_rx(); + // Build a map of cipher_id -> key_name for UI display let key_names: Arc> = Arc::new( vault_keys @@ -2438,6 +2674,12 @@ async fn run_foreground( let key_material_fingerprints: KeyMaterialFingerprints = Arc::new(RwLock::new(std::collections::HashMap::new())); let pending_sync = Arc::new(std::sync::atomic::AtomicBool::new(false)); + let pin_failures: PinFailureHandle = + Arc::new(std::sync::Mutex::new(PinFailureState::default())); + // CONC-6: count in-flight UI requests (sign/unlock prompts) so the inactivity + // auto-lock does not fire while a prompt is open (last_activity is only + // refreshed at request arrival, not while the user is deciding). + let in_flight_prompts = Arc::new(std::sync::atomic::AtomicUsize::new(0)); let key_names = Arc::new(RwLock::new((*key_names).clone())); // Load vault keys into agent @@ -2595,6 +2837,7 @@ async fn run_foreground( &pending_sync, ¬ification_state, &authorization_memory, + &pin_failures, ).await; let _ = ctrl_req.reply.send(response); } @@ -2602,6 +2845,11 @@ async fn run_foreground( Some(request) = request_rx.recv() => { last_activity = tokio::time::Instant::now(); + // CONC-6: mark a request in flight for the duration of the + // handler (which may open a long sign/unlock prompt). + in_flight_prompts.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let in_flight_done = in_flight_prompts.clone(); + // Spawn a task to handle each request so we don't block the main loop let response_tx_clone = (*response_tx).clone(); let vault_locked_clone = vault_locked.clone(); @@ -2614,6 +2862,7 @@ async fn run_foreground( let local_cache_key_state_clone = local_cache_key_state.clone(); let runtime_event_tx_clone = runtime_event_tx.clone(); let authorization_memory_clone = authorization_memory.clone(); + let pin_failures_clone = pin_failures.clone(); let ui_tx_clone = ui_request_tx.clone(); @@ -2634,7 +2883,9 @@ async fn run_foreground( ui_tx_clone, runtime_event_tx_clone, authorization_memory_clone, + pin_failures_clone, ).await; + in_flight_done.fetch_sub(1, std::sync::atomic::Ordering::Relaxed); }); } // Runtime events from spawned SSH request handlers @@ -2800,12 +3051,27 @@ async fn run_foreground( _ = lock_check_interval.tick() => { if lock_timeout > 0 && !vault_locked.load(std::sync::atomic::Ordering::Relaxed) + && in_flight_prompts.load(std::sync::atomic::Ordering::Relaxed) == 0 && last_activity.elapsed().as_secs() >= lock_timeout { info!("Auto-locking vault due to inactivity ({} seconds)", lock_timeout); let _ = lock_vault(&mut agent, &vault_locked, &cached_key_tuples, Some(&local_cache_key_state), Some(&authorization_memory)).await; } } + // SSH agent transport failed (e.g. could not claim the OpenSSH pipe). + // Shut down rather than run as a zombie that answers status/unlock + // while serving no SSH client (RT-01). + changed = agent_fatal_rx.changed() => { + if changed.is_err() { + tracing::error!("SSH agent signal channel closed; shutting down"); + break; + } + let reason = agent_fatal_rx.borrow().clone(); + if let Some(reason) = reason { + tracing::error!(%reason, "SSH agent transport failed; shutting down daemon"); + break; + } + } // Shutdown signal _ = tokio::signal::ctrl_c() => { info!("Received Ctrl+C, shutting down..."); @@ -2842,6 +3108,58 @@ async fn lock_vault( Ok(()) } +/// SEC-03: in-memory PIN brute-force protection. Kept per daemon run (not +/// persisted) so it cannot be reset by tampering with on-disk state. +#[derive(Default)] +struct PinFailureState { + consecutive_failures: u32, + locked_until: Option, +} + +type PinFailureHandle = Arc>; + +/// Lock PIN unlock after this many consecutive wrong attempts. +const PIN_MAX_ATTEMPTS: u32 = 5; +/// Per-failure delay scales with the failure count, capped, to slow guessing. +const PIN_FAILURE_BASE_DELAY: std::time::Duration = std::time::Duration::from_millis(500); +const PIN_FAILURE_MAX_DELAY: std::time::Duration = std::time::Duration::from_secs(5); +/// Lockout window once PIN_MAX_ATTEMPTS consecutive failures is reached. +const PIN_LOCKOUT: std::time::Duration = std::time::Duration::from_secs(30); + +/// Reject a PIN attempt outright while a lockout is active. Returns the +/// remaining lockout duration if locked, clearing an expired lockout. +fn pin_lockout_remaining(pin_failures: &PinFailureHandle) -> Option { + let now = std::time::Instant::now(); + let mut st = pin_failures.lock().unwrap_or_else(|e| e.into_inner()); + match st.locked_until { + Some(until) if until > now => Some(until - now), + Some(_) => { + st.locked_until = None; + None + } + None => None, + } +} + +/// Record a PIN unlock outcome: reset on success, otherwise bump the failure +/// counter (arming a lockout at the threshold) and return the delay to apply. +fn record_pin_attempt(pin_failures: &PinFailureHandle, success: bool) -> std::time::Duration { + let mut st = pin_failures.lock().unwrap_or_else(|e| e.into_inner()); + if success { + st.consecutive_failures = 0; + st.locked_until = None; + return std::time::Duration::ZERO; + } + st.consecutive_failures = st.consecutive_failures.saturating_add(1); + if st.consecutive_failures >= PIN_MAX_ATTEMPTS { + st.locked_until = Some(std::time::Instant::now() + PIN_LOCKOUT); + } + std::cmp::min( + PIN_FAILURE_BASE_DELAY * st.consecutive_failures, + PIN_FAILURE_MAX_DELAY, + ) +} + #[allow(clippy::too_many_arguments)] async fn build_status_response( json: bool, @@ -2856,27 +3174,56 @@ async fn build_status_response( ) -> sshwarden_agent::ControlResponse { let locked = vault_locked.load(std::sync::atomic::Ordering::Relaxed); let count = agent.key_count(); - let has_pin = pin_encrypted_keys.read().await.is_some(); + let signable = agent.signable_key_count(); + let agent_running = agent.is_running(); + // SEC: reflect PIN availability across both layouts — the legacy + // pin_encrypted_keys slot AND the v3 local key cache, where set-pin/migration + // moves the PIN into local_cache_key.pin_encrypted (and clears the legacy slot). + let has_pin = pin_encrypted_keys.read().await.is_some() + || local_key_cache_data + .read() + .await + .as_ref() + .map(|c| c.local_cache_key.pin_encrypted.is_some()) + .unwrap_or(false); let has_vault = vault_file_data.read().await.is_some(); let has_local_key_cache = local_key_cache_data.read().await.is_some(); 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() + .map(|p| p.display().to_string()) + .unwrap_or_else(|_| "".to_string()); let details = serde_json::json!({ "locked": locked, "key_count": count, + "signable_key_count": signable, + "agent_running": agent_running, "has_pin": has_pin, "has_vault_file": has_vault, "has_local_key_cache": has_local_key_cache, "legacy_migration_available": has_vault && !has_local_key_cache, "authenticated": authenticated, "pending_sync": pending, + "data_dir": data_dir, "notification": notification.to_json(), }); let mut resp = sshwarden_agent::ControlResponse::status(locked, count).with_details(details); let mut extras = Vec::new(); + if !agent_running { + // RT-01: surface the zombie state — the agent task is not serving SSH + // clients even though the control channel still answers. + extras.push("AGENT NOT SERVING (SSH endpoint unavailable)"); + } + if count > 0 && signable < count { + // lock-keystore: identities are listed (ssh-add -l) but have no private + // material, so signing fails until unlock. Make that explicit. + extras.push("keys listed but not signable until unlock"); + } if has_pin { extras.push("PIN configured"); } @@ -2936,6 +3283,7 @@ async fn handle_control_command( pending_sync: &Arc, notification_state: &Arc>, authorization_memory: &AuthorizationMemorySet, + pin_failures: &PinFailureHandle, ) -> sshwarden_agent::ControlResponse { use sshwarden_agent::ControlAction; @@ -3122,15 +3470,75 @@ async fn handle_control_command( // Fall back to PIN dialog when Hello sign-path fails if auto_unlock { info!("Hello sign-path failed, trying PIN dialog fallback"); - let enc_data = get_pin_encrypted_data(pin_encrypted_keys, vault_file_data).await; + // Envelope-first (v3 local key cache) with legacy vault.enc + // fallback, mirroring the SSH-request path so v3 PINs validate in + // this dialog too (get_pin_encrypted_data only reads legacy slots). + let envelope_cache = { + let guard = local_key_cache_data.read().await; + guard + .as_ref() + .filter(|c| c.local_cache_key.pin_encrypted.is_some()) + .cloned() + }; + let validator_parts: Option<(PinValidator, DecryptedCache, _)> = + if let Some(cache) = envelope_cache { + let dc: DecryptedCache = Arc::new(std::sync::Mutex::new(None)); + let kh: Arc>> = + Arc::new(std::sync::Mutex::new(None)); + let dc_inner = dc.clone(); + let kh_inner = kh.clone(); + let v: PinValidator = Arc::new(move |pin: &str| -> bool { + match decrypt_envelope_local_key_cache_with_pin(&cache, pin) { + Ok((keys_json, lck)) => { + *dc_inner.lock().unwrap_or_else(|e| e.into_inner()) = + Some(keys_json); + *kh_inner.lock().unwrap_or_else(|e| e.into_inner()) = Some(lck); + true + } + Err(_) => false, + } + }); + Some((v, dc, Some(kh))) + } else if let Some(enc_data) = + get_pin_encrypted_data(pin_encrypted_keys, vault_file_data).await + { + let (v, dc) = make_pin_validator(enc_data); + Some((v, dc, None)) + } else { + None + }; - if let Some(enc_data) = enc_data { - let (validator, decrypted_cache) = make_pin_validator(enc_data); + if let Some((validator, decrypted_cache, lck_holder)) = validator_parts { + // SEC-03: share the daemon-wide PIN brute-force lockout with + // `unlock --pin` so dialog attempts can't bypass the cap. + let validator = gate_pin_validator(validator, pin_failures); let pin_result = sshwarden_ui::unlock::request_pin_dialog(ui_request_tx, validator).await; if let Some(ref entered_pin) = pin_result { - let keys_json = decrypted_cache.lock().unwrap().take().unwrap(); + let keys_json = match decrypted_cache + .lock() + .unwrap_or_else(|e| e.into_inner()) + .take() + { + Some(j) => j, + None => { + tracing::warn!( + "PIN validator reported success but cache was empty" + ); + return sshwarden_agent::ControlResponse::err( + "PIN unlock failed: internal cache error", + ); + } + }; + // If unlocked via the v3 envelope, hold the decrypted local + // cache key so later operations (sync/re-encrypt) have it. + if let Some(kh) = lck_holder { + let maybe_lck = kh.lock().unwrap_or_else(|e| e.into_inner()).take(); + if let Some(lck) = maybe_lck { + local_cache_key_state.write().await.set(lck); + } + } let resp = finish_unlock_with_json( &keys_json, agent, @@ -3392,77 +3800,105 @@ async fn handle_control_command( return sshwarden_agent::ControlResponse::ok("Vault is already unlocked"); } - if let Some(cache) = local_key_cache_data.read().await.as_ref().cloned() { - match decrypt_envelope_local_key_cache_with_pin(&cache, &pin) { - Ok((keys_json, local_cache_key)) => { - local_cache_key_state.write().await.set(local_cache_key); - let resp = finish_unlock_with_json( - &keys_json, - agent, - vault_locked, - cached_key_tuples, - key_names, - "Vault unlocked via PIN local key cache", - ) - .await; + // SEC-03: reject outright while a brute-force lockout is active (no + // Argon2 work performed) so the control channel can't be hammered. + if let Some(wait) = pin_lockout_remaining(pin_failures) { + return sshwarden_agent::ControlResponse::err(&format!( + "Too many failed PIN attempts; locked for {}s", + wait.as_secs() + 1 + )); + } - if resp.ok { - try_restore_api_session( - api_client, - config, - &pin, - notification_rx, - notification_client, - notification_state, - ) - .await; - resolve_pending_sync( - pending_sync, - api_client, - cached_key_tuples, - public_key_identity_tuples, - local_key_cache_data, - local_cache_key_state, - authorization_memory, - key_material_fingerprints, - vault_locked, + let resp = 'unlock: { + if let Some(cache) = local_key_cache_data.read().await.as_ref().cloned() { + match decrypt_envelope_local_key_cache_with_pin(&cache, &pin) { + Ok((keys_json, local_cache_key)) => { + // SEC-04: now that we hold the local cache key and a + // verified PIN, transparently upgrade a pre-v3 (fixed + // salt) cache to a random per-cache salt. + if needs_pin_salt_migration(&cache) { + match migrate_pin_salt_to_v3(&cache, &local_cache_key, &pin) { + Ok(migrated) => { + *local_key_cache_data.write().await = Some(migrated); + info!( + "Migrated local key cache PIN slot to v3 (random salt)" + ); + } + Err(e) => { + tracing::warn!( + "PIN salt v3 migration failed (non-fatal): {e}" + ) + } + } + } + local_cache_key_state.write().await.set(local_cache_key); + let resp = finish_unlock_with_json( + &keys_json, agent, + vault_locked, + cached_key_tuples, key_names, - notification_state, + "Vault unlocked via PIN local key cache", ) .await; - } - return resp; - } - Err(e) => { - tracing::warn!("PIN unlock from local key cache failed: {}", e); + if resp.ok { + try_restore_api_session( + api_client, + config, + &pin, + notification_rx, + notification_client, + notification_state, + ) + .await; + resolve_pending_sync( + pending_sync, + api_client, + cached_key_tuples, + public_key_identity_tuples, + local_key_cache_data, + local_cache_key_state, + authorization_memory, + key_material_fingerprints, + vault_locked, + agent, + key_names, + notification_state, + ) + .await; + } + + break 'unlock resp; + } + Err(e) => { + tracing::warn!("PIN unlock from local key cache failed: {}", e); + } } } - } - // Fall back to legacy in-memory/vault.enc cache. - let encrypted = { - let mem = pin_encrypted_keys.read().await.clone(); - if mem.is_some() { - mem - } else { - vault_file_data - .read() - .await - .as_ref() - .map(|v| v.pin_encrypted.clone()) - } - }; + // Fall back to legacy in-memory/vault.enc cache. + let encrypted = { + let mem = pin_encrypted_keys.read().await.clone(); + if mem.is_some() { + mem + } else { + vault_file_data + .read() + .await + .as_ref() + .map(|v| v.pin_encrypted.clone()) + } + }; - match encrypted { - Some(enc_data) => match sshwarden_api::crypto::pin_decrypt(&enc_data, &pin) { - Ok(keys_json) => { - if local_key_cache_data.read().await.is_none() { - let keys_for_migration: Result, _> = - serde_json::from_str(&keys_json); - if let Ok(keys_for_migration) = keys_for_migration { - match write_envelope_local_key_cache( + match encrypted { + Some(enc_data) => match sshwarden_api::crypto::pin_decrypt(&enc_data, &pin) { + Ok(keys_json) => { + if local_key_cache_data.read().await.is_none() { + let keys_for_migration: Result, _> = + serde_json::from_str(&keys_json); + if let Ok(keys_for_migration) = keys_for_migration { + match write_envelope_local_key_cache( &keys_for_migration, &config.auth.email, &config.server.base_url, @@ -3483,54 +3919,63 @@ async fn handle_control_command( e ), } + } } - } - let resp = finish_unlock_with_json( - &keys_json, - agent, - vault_locked, - cached_key_tuples, - key_names, - "Vault unlocked via PIN", - ) - .await; - - if resp.ok { - // Try to restore API session from device session file - try_restore_api_session( - api_client, - config, - &pin, - notification_rx, - notification_client, - notification_state, - ) - .await; - resolve_pending_sync( - pending_sync, - api_client, - cached_key_tuples, - public_key_identity_tuples, - local_key_cache_data, - local_cache_key_state, - authorization_memory, - key_material_fingerprints, - vault_locked, + let resp = finish_unlock_with_json( + &keys_json, agent, + vault_locked, + cached_key_tuples, key_names, - notification_state, + "Vault unlocked via PIN", ) .await; + + if resp.ok { + // Try to restore API session from device session file + try_restore_api_session( + api_client, + config, + &pin, + notification_rx, + notification_client, + notification_state, + ) + .await; + resolve_pending_sync( + pending_sync, + api_client, + cached_key_tuples, + public_key_identity_tuples, + local_key_cache_data, + local_cache_key_state, + authorization_memory, + key_material_fingerprints, + vault_locked, + agent, + key_names, + notification_state, + ) + .await; + } + + resp } + Err(_) => sshwarden_agent::ControlResponse::err("Invalid PIN"), + }, + None => sshwarden_agent::ControlResponse::err( + "No PIN configured. Use 'sshwarden set-pin' first.", + ), + } + }; - resp - } - Err(_) => sshwarden_agent::ControlResponse::err("Invalid PIN"), - }, - None => sshwarden_agent::ControlResponse::err( - "No PIN configured. Use 'sshwarden set-pin' first.", - ), + // SEC-03: record the outcome — reset on success, otherwise apply an + // escalating delay and arm a lockout once the threshold is reached. + let delay = record_pin_attempt(pin_failures, resp.ok); + if !delay.is_zero() { + tokio::time::sleep(delay).await; } + resp } ControlAction::UnlockPassword { password } => { let password = zeroize::Zeroizing::new(password); @@ -3647,6 +4092,11 @@ async fn handle_control_command( } } ControlAction::Sync => { + // do_sync cannot load keys into the agent while the vault is locked + // (no private material is held); it only refreshes the on-disk cache. + // 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); match do_sync( api_client, cached_key_tuples, @@ -3663,17 +4113,29 @@ async fn handle_control_command( .await { Ok(count) => { - sshwarden_agent::ControlResponse::ok(&format!("Synced {} SSH keys", count)) + if was_locked { + pending_sync.store(true, std::sync::atomic::Ordering::Relaxed); + sshwarden_agent::ControlResponse::ok(&format!( + "Synced {count} SSH keys to cache; they will load into the agent on next unlock" + )) + } else { + sshwarden_agent::ControlResponse::ok(&format!("Synced {count} SSH keys")) + } } Err(e) => sshwarden_agent::ControlResponse::err(&e), } } ControlAction::Forget => { + // 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 native_slot = local_key_cache_data .read() @@ -3684,9 +4146,11 @@ async fn handle_control_command( 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::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; @@ -3711,9 +4175,17 @@ async fn handle_control_command( let _ = agent.clear_keys(); vault_locked.store(true, std::sync::atomic::Ordering::Relaxed); - sshwarden_agent::ControlResponse::ok( - "Forgot local key cache, legacy vault file, and device session material", - ) + if failures.is_empty() { + sshwarden_agent::ControlResponse::ok( + "Forgot local key cache, legacy vault file, and device session material", + ) + } else { + sshwarden_agent::ControlResponse::err(&format!( + "Cleared in-memory state, but FAILED to delete on-disk material: {}. \ + This material may still exist on disk — remove it manually.", + failures.join("; ") + )) + } } ControlAction::SetPin { pin } => { let pin = zeroize::Zeroizing::new(pin); @@ -3903,7 +4375,7 @@ fn make_pin_validator(enc_data: String) -> (PinValidator, DecryptedCache) { let validator: Arc bool + Send + Sync> = Arc::new(move |pin: &str| -> bool { match sshwarden_api::crypto::pin_decrypt(&enc_data, pin) { Ok(keys_json) => { - *cache_clone.lock().unwrap() = Some(keys_json); + *cache_clone.lock().unwrap_or_else(|e| e.into_inner()) = Some(keys_json); true } Err(_) => false, @@ -3913,6 +4385,25 @@ fn make_pin_validator(enc_data: String) -> (PinValidator, DecryptedCache) { (validator, decrypted_cache) } +/// Wrap a PIN validator so every attempt shares the daemon-wide brute-force +/// accounting (SEC-03) with `ControlAction::UnlockPin`: reject outright while a +/// lockout is active, and record each outcome so UI/SSH dialog attempts count +/// toward the SAME lockout the control channel enforces — otherwise the dialog +/// path is an unthrottled brute-force bypass. The wrapper runs inside the +/// dialog's validation thread (not a tokio worker), so the failure accounting is +/// intentionally clock-agnostic (std::time::Instant). +fn gate_pin_validator(inner: PinValidator, pin_failures: &PinFailureHandle) -> PinValidator { + let pin_failures = pin_failures.clone(); + Arc::new(move |pin: &str| -> bool { + if pin_lockout_remaining(&pin_failures).is_some() { + return false; + } + let ok = inner(pin); + let _ = record_pin_attempt(&pin_failures, ok); + ok + }) +} + /// 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. @@ -4479,6 +4970,7 @@ async fn handle_ui_request( ui_request_tx: UIRequestTx, runtime_event_tx: tokio::sync::mpsc::Sender, authorization_memory: AuthorizationMemorySet, + pin_failures: PinFailureHandle, ) { if request.is_list { if vault_locked.load(std::sync::atomic::Ordering::Relaxed) { @@ -4705,6 +5197,10 @@ async fn handle_ui_request( return; }; + // SEC-03: share the daemon-wide PIN brute-force lockout with the control + // channel so repeated wrong PINs in the SSH-request dialog also count. + let validator = gate_pin_validator(validator, &pin_failures); + let context_key_name = { let names = key_names.read().await; request @@ -4764,6 +5260,22 @@ async fn handle_ui_request( return; } + // LOGIC-2: reaching here with the vault still locked means the auto-unlock + // block above was skipped (auto_unlock disabled). The loaded entries have no + // private material, so deny cleanly rather than auto-approving under + // prompt_behavior=Never — otherwise ssh sees an "approval" followed by a + // broken signature with no way to recover. + if vault_locked.load(std::sync::atomic::Ordering::Relaxed) { + info!( + request_id = request.request_id, + process = %request.process_name, + "Sign request denied: vault is locked and auto-unlock is disabled. \ + Run `sshwarden unlock`, or enable [unlock] auto_unlock_on_request." + ); + let _ = response_tx.send((request.request_id, false)); + return; + } + let operation_kind = operation_kind_for_request(&request).to_string(); let memory_key = request .cipher_id @@ -4994,6 +5506,18 @@ async fn persist_bind_payload( let keys = load_managed_keys_from_cache().unwrap_or_default(); sync_managed_ssh_config_inner(&keys, true)?; + + // CFG-2: a binding is inert unless ~/.ssh/config Includes the managed + // snippet. Mirror cmd_bindings_add so UI/sign-flow bindings install the + // Include line too (idempotent). Non-fatal — the snippet is already saved. + let include_path = managed_sshwarden_include_path()?; + let config_path = user_ssh_config_path()?; + if let Err(e) = write_sshwarden_include_line(&config_path, &include_path) { + tracing::warn!( + "Bindings saved, but failed to ensure the Include line in {}: {e}", + config_path.display() + ); + } Ok(()) }) .await @@ -5039,13 +5563,17 @@ async fn dispatch_standalone_bind_hosts_dialog( .await .map_err(|_| anyhow::anyhow!("UI request channel closed"))?; - match response_rx.await { - Ok(sshwarden_ui::BindHostsResult::Saved { bindings: payload }) => { + // CONC-1: this dialog response is awaited inline in the main select! loop + // (via the BindHostsDialog control command), so an unbounded wait would + // freeze auto-lock, token refresh and notification handling. Bound it. + match tokio::time::timeout(std::time::Duration::from_secs(600), response_rx).await { + Ok(Ok(sshwarden_ui::BindHostsResult::Saved { bindings: payload })) => { persist_bind_payload(&payload).await?; Ok(true) } - Ok(sshwarden_ui::BindHostsResult::Cancelled) => Ok(false), - Err(_) => anyhow::bail!("Dialog response channel closed unexpectedly"), + Ok(Ok(sshwarden_ui::BindHostsResult::Cancelled)) => Ok(false), + Ok(Err(_)) => anyhow::bail!("Dialog response channel closed unexpectedly"), + Err(_) => anyhow::bail!("Host-binding dialog timed out after 600s"), } } @@ -5153,18 +5681,9 @@ fn log_file_path() -> anyhow::Result { Ok(data_dir()?.join("sshwarden.log")) } -/// Check if daemon is already running by reading PID file and checking process. -fn is_daemon_running() -> bool { - let pid_path = match pid_file_path() { - Ok(p) => p, - Err(_) => return false, - }; - - if !pid_path.exists() { - return false; - } - - let pid_str = match std::fs::read_to_string(&pid_path) { +/// Return true if `path` holds a PID whose recorded process is still alive. +fn pid_file_owner_alive(path: &std::path::Path) -> bool { + let pid_str = match std::fs::read_to_string(path) { Ok(s) => s, Err(_) => return false, }; @@ -5174,19 +5693,93 @@ fn is_daemon_running() -> bool { Err(_) => return false, }; - // Check if the process is still running + // Verify the PID is not just alive but actually our daemon: a recycled PID + // (after a crash without clean shutdown) could belong to an unrelated process + // and would otherwise falsely block a healthy restart. use sysinfo::System; let mut sys = System::new(); sys.refresh_processes(sysinfo::ProcessesToUpdate::All, true); - sys.process(sysinfo::Pid::from_u32(pid)).is_some() + match sys.process(sysinfo::Pid::from_u32(pid)) { + Some(proc) => process_is_sshwarden(proc), + None => false, + } +} + +/// Confirm a process is this SSHWarden binary, not an unrelated process that +/// recycled the PID. Prefer an exact exe-path match; fall back to the executable +/// file name when `exe()` is empty/restricted (common on macOS) or the daemon was +/// launched from a since-moved binary. +fn process_is_sshwarden(proc: &sysinfo::Process) -> bool { + let self_exe = std::env::current_exe().ok(); + if let (Some(proc_exe), Some(self_path)) = (proc.exe(), self_exe.as_deref()) { + if proc_exe == self_path { + return true; + } + } + match self_exe.as_deref().and_then(|p| p.file_name()) { + Some(name) => proc.name() == name, + None => false, + } } -/// Write current PID to pid file. -fn write_pid_file() -> anyhow::Result<()> { - let pid = std::process::id(); +/// Check if daemon is already running by reading PID file and checking process. +fn is_daemon_running() -> bool { + match pid_file_path() { + Ok(p) => p.exists() && pid_file_owner_alive(&p), + Err(_) => false, + } +} + +/// Atomically claim the singleton PID file (RT-02). Returns `Ok(true)` if this +/// process is now the owner, `Ok(false)` if a live daemon already owns it. +/// +/// Replaces the previous check-then-write (which let two near-simultaneous +/// starters both pass the `is_daemon_running()` check). `create_new` is an atomic +/// O_EXCL/CREATE_NEW open, so exactly one racing process wins the create. A +/// leftover file whose owner is gone (crash without clean shutdown) is treated as +/// stale and reclaimed. +fn claim_pid_file() -> anyhow::Result { + use std::io::Write; let path = pid_file_path()?; - std::fs::write(&path, pid.to_string()) - .with_context(|| format!("Failed to write PID file: {}", path.display())) + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).with_context(|| { + format!("Failed to create PID file directory: {}", parent.display()) + })?; + } + loop { + match std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&path) + { + Ok(mut f) => { + f.write_all(std::process::id().to_string().as_bytes()) + .with_context(|| format!("Failed to write PID file: {}", path.display()))?; + return Ok(true); + } + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { + if pid_file_owner_alive(&path) { + return Ok(false); + } + // Stale: the recorded owner is gone. Remove and retry the atomic + // create; if another starter wins in between, the next iteration + // sees a live owner and yields. + match std::fs::remove_file(&path) { + Ok(()) => continue, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => continue, + Err(e) => { + return Err(e).with_context(|| { + format!("Failed to remove stale PID file: {}", path.display()) + }) + } + } + } + Err(e) => { + return Err(e) + .with_context(|| format!("Failed to create PID file: {}", path.display())) + } + } + } } /// Remove pid file on shutdown.