diff --git a/apps/desktop/desktop_native/ssh_agent/Cargo.toml b/apps/desktop/desktop_native/ssh_agent/Cargo.toml index 23fe6efd0cd..b1ddf45b627 100644 --- a/apps/desktop/desktop_native/ssh_agent/Cargo.toml +++ b/apps/desktop/desktop_native/ssh_agent/Cargo.toml @@ -44,7 +44,7 @@ ssh-key = { version = "=0.7.0-rc.3", features = [ "getrandom", ] } sysinfo = { workspace = true, features = ["windows"] } -tokio = { workspace = true, features = ["io-util", "sync", "macros", "net"] } +tokio = { workspace = true, features = ["io-util", "sync", "macros", "net", "full"] } tokio-util = { workspace = true, features = ["codec"] } tracing = { workspace = true } tracing-subscriber.workspace = true diff --git a/apps/desktop/desktop_native/ssh_agent/src/agent/platform.rs b/apps/desktop/desktop_native/ssh_agent/src/agent/platform.rs index 8fd96198c14..395aed55787 100644 --- a/apps/desktop/desktop_native/ssh_agent/src/agent/platform.rs +++ b/apps/desktop/desktop_native/ssh_agent/src/agent/platform.rs @@ -12,6 +12,7 @@ use tracing::info; pub struct PlatformListener {} impl PlatformListener { + /// Spawns all listeners for the current platform. A platform may have a single listener, or multiple. pub fn spawn_listeners(agent: BitwardenDesktopAgent) { #[cfg(target_os = "linux")] { @@ -79,6 +80,8 @@ impl PlatformListener { #[cfg(target_os = "windows")] pub fn spawn_windows_listeners(agent: BitwardenDesktopAgent) { tokio::spawn(async move { + // Windows by default uses the named pipe \\.\pipe\openssh-ssh-agent. It also supports external SSH auth sock variables, which are + // not supported here. Windows also supports putty (not implemented here) and unix sockets for WSL (not implemented here). const PIPE_NAME: &str = r"\\.\pipe\openssh-ssh-agent"; tokio::spawn(NamedPipeServerStream::listen(PIPE_NAME.to_string(), agent)); }); diff --git a/apps/desktop/desktop_native/ssh_agent/src/agent/ui_requester.rs b/apps/desktop/desktop_native/ssh_agent/src/agent/ui_requester.rs index 27243fcd4ea..d29f25cdbc4 100644 --- a/apps/desktop/desktop_native/ssh_agent/src/agent/ui_requester.rs +++ b/apps/desktop/desktop_native/ssh_agent/src/agent/ui_requester.rs @@ -6,41 +6,17 @@ use crate::protocol::connection::ConnectionInfo; const TIMEOUT: std::time::Duration = std::time::Duration::from_secs(60); -#[derive(Clone)] -pub struct UiRequester { - show_ui_request_tx: tokio::sync::mpsc::Sender, - get_ui_response_rx: Arc>>, -} - static REQUEST_ID_COUNTER: AtomicU32 = AtomicU32::new(0); -#[derive(Clone, Debug)] -pub enum UiRequestMessage { - ListRequest { - request_id: u32, - connection_info: ConnectionInfo, - }, - AuthRequest { - request_id: u32, - connection_info: ConnectionInfo, - cipher_id: String, - }, - SignRequest { - request_id: u32, - connection_info: ConnectionInfo, - cipher_id: String, - namespace: String, - }, -} - -impl UiRequestMessage { - pub fn id(&self) -> u32 { - match self { - UiRequestMessage::ListRequest { request_id, .. } => *request_id, - UiRequestMessage::AuthRequest { request_id, .. } => *request_id, - UiRequestMessage::SignRequest { request_id, .. } => *request_id, - } - } +/// UI requester is used to abstract the communication with the UI electron process. The internal +/// implementation uses channels to communicate with the UI process. From the consumer perspective +/// it just exposes async request methods. +#[derive(Clone)] +pub struct UiRequester { + /// Channel to send the request to the UI process. This first gets sent to the NAPI module which then handles the actual IPC to the UI. + show_ui_request_tx: tokio::sync::mpsc::Sender, + /// Channel to receive the response from the UI process. This first gets sent from the NAPI module which then forwards it to this channel. + get_ui_response_rx: Arc>>, } impl UiRequester { @@ -54,6 +30,8 @@ impl UiRequester { } } + /// Ask the UI to show a request for the user to approve or deny. The UI may choose to not show a prompt but only + /// require that the client is unlocked. pub async fn request_list(&self, connection_info: &ConnectionInfo) -> bool { let request_id = REQUEST_ID_COUNTER.fetch_add(1, std::sync::atomic::Ordering::SeqCst); self.request(UiRequestMessage::ListRequest { @@ -63,6 +41,8 @@ impl UiRequester { .await } + /// Ask the UI to show a request for the user to approve or deny. The UI may choose to not show a prompt but only + /// require that the client is unlocked or apply other automatic rules. pub async fn request_sign( &self, connection_info: &ConnectionInfo, @@ -98,3 +78,32 @@ impl UiRequester { .unwrap_or(false) } } + +#[derive(Clone, Debug)] +pub enum UiRequestMessage { + ListRequest { + request_id: u32, + connection_info: ConnectionInfo, + }, + AuthRequest { + request_id: u32, + connection_info: ConnectionInfo, + cipher_id: String, + }, + SignRequest { + request_id: u32, + connection_info: ConnectionInfo, + cipher_id: String, + namespace: String, + }, +} + +impl UiRequestMessage { + pub fn id(&self) -> u32 { + match self { + UiRequestMessage::ListRequest { request_id, .. } => *request_id, + UiRequestMessage::AuthRequest { request_id, .. } => *request_id, + UiRequestMessage::SignRequest { request_id, .. } => *request_id, + } + } +} diff --git a/apps/desktop/desktop_native/ssh_agent/src/hostinfo/mod.rs b/apps/desktop/desktop_native/ssh_agent/src/hostinfo/mod.rs new file mode 100644 index 00000000000..7e1aee0bbde --- /dev/null +++ b/apps/desktop/desktop_native/ssh_agent/src/hostinfo/mod.rs @@ -0,0 +1,129 @@ +use std::fs; +use std::path::Path; + +use tracing::info; + +use crate::protocol::types::PublicKey; + +/// Represents a known host entry with hostnames and public keys +#[derive(Clone, Debug)] +pub struct KnownHostEntry { + /// Host name + pub hostname: String, + /// The public key for this host + pub public_key: PublicKey, +} + +impl KnownHostEntry { + /// Creates a new known host entry + pub fn new(hostnames: String, public_key: PublicKey) -> Self { + Self { + hostname: hostnames, + public_key, + } + } +} + +#[derive(Clone, Debug)] +pub struct KnownHosts(Vec); +impl KnownHosts { + pub fn find_host(&self, public_key: &PublicKey) -> Option<&KnownHostEntry> { + self.0.iter().find(|entry| &entry.public_key == public_key) + } +} + +/// Reads and parses the SSH known_hosts file +pub struct KnownHostsReader; + +impl KnownHostsReader { + /// Reads the known_hosts file from the standard SSH directory + pub fn read_default() -> anyhow::Result { + let path = homedir::my_home()? + .ok_or_else(|| anyhow::anyhow!("Failed to determine home directory"))? + .join(".ssh/known_hosts"); + Ok(KnownHosts(Self::read_from(&path)?)) + } + + /// Reads the known_hosts file from a specific path + pub fn read_from>(path: P) -> anyhow::Result> { + let path = path.as_ref(); + + if !path.exists() { + return Ok(Vec::new()); + } + + let content = fs::read_to_string(path) + .map_err(|e| anyhow::anyhow!("Failed to read known_hosts file: {}", e))?; + + Self::parse(&content) + } + + /// Parses known_hosts file content + /// Format: hostnames key-type key-blob [comment] + /// Each line is either a comment (starting with #) or a host entry + pub fn parse(content: &str) -> anyhow::Result> { + let mut entries = Vec::new(); + + for line in content.lines() { + let line = line.trim(); + + // Skip empty lines and comments + if line.is_empty() || line.starts_with('#') { + continue; + } + + // Split by the first space + let first_space_index = line.find(' '); + let Some(first_space_index) = first_space_index else { + info!("Invalid known_hosts line (no spaces): {}", line); + continue; + }; + let (hostnames, rest) = line.split_at(first_space_index); + let host_key = PublicKey::try_from(rest.trim().to_string())?; + entries.push(KnownHostEntry::new(hostnames.to_string(), host_key)); + } + + Ok(entries) + } + + /// Finds host entries by hostname pattern + pub fn find_host(entries: &[KnownHostEntry], hostname: &str) -> Option { + entries + .iter() + .find(|entry| { + entry.hostname.split(',').any(|h| { + h == hostname || h == "*" || h.starts_with("*.") && hostname.ends_with(&h[1..]) + }) + }) + .cloned() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_parse_empty() { + let content = ""; + let entries = KnownHostsReader::parse(content).unwrap(); + assert_eq!(entries.len(), 0); + } + + #[test] + #[ignore] + fn test_current_user_known_hosts() { + let entries = KnownHostsReader::read_default().unwrap(); + println!("Known hosts entries: {:?}", entries); + } + + #[test] + fn test_parse_with_comments() { + let content = r#" +github.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOMqqnkVzrm0SdG6UOoqKLsabgH5C9okWi0dh2l9GKJl +"#; + let entries = KnownHostsReader::parse(content).unwrap(); + assert!(entries.len() <= 1); + println!("{:?}", entries); + } +} diff --git a/apps/desktop/desktop_native/ssh_agent/src/lib.rs b/apps/desktop/desktop_native/ssh_agent/src/lib.rs index c73e1c78129..1ecb2c5cd1b 100644 --- a/apps/desktop/desktop_native/ssh_agent/src/lib.rs +++ b/apps/desktop/desktop_native/ssh_agent/src/lib.rs @@ -1,4 +1,5 @@ pub mod agent; +pub mod hostinfo; pub mod memory; pub mod protocol; pub mod transport; diff --git a/apps/desktop/desktop_native/ssh_agent/src/protocol/agent_listener.rs b/apps/desktop/desktop_native/ssh_agent/src/protocol/agent_listener.rs index 9c36434ddf9..0a0eca0faaa 100644 --- a/apps/desktop/desktop_native/ssh_agent/src/protocol/agent_listener.rs +++ b/apps/desktop/desktop_native/ssh_agent/src/protocol/agent_listener.rs @@ -11,7 +11,10 @@ use crate::{ async_stream_wrapper::AsyncStreamWrapper, connection::ConnectionInfo, key_store::Agent, - replies::{AgentExtensionFailure, AgentFailure, IdentitiesReply, SshSignReply}, + replies::{ + AgentExtensionFailure, AgentFailure, AgentSuccess, IdentitiesReply, ReplyFrame, + SshSignReply, + }, requests::Request, }, transport::peer_info::PeerInfo, @@ -33,9 +36,9 @@ where } Some(Ok((stream, peer_info))) = listener.next() => { let mut stream = AsyncStreamWrapper::new(stream); - let connection_info = ConnectionInfo::new(peer_info); + let mut connection_info = ConnectionInfo::new(peer_info); info!("Accepted connection {} from {:?}", connection_info.id(), connection_info.peer_info()); - if let Err(e) = handle_connection(&agent, &mut stream, &connection_info).await { + if let Err(e) = handle_connection(&agent, &mut stream, &mut connection_info).await { error!("Error handling request: {e}"); } } @@ -47,7 +50,7 @@ where async fn handle_connection( agent: &impl Agent, stream: &mut AsyncStreamWrapper, - connection: &ConnectionInfo, + connection: &mut ConnectionInfo, ) -> Result<(), anyhow::Error> { loop { let span = tracing::info_span!("Connection", connection_id = connection.id()); @@ -113,6 +116,16 @@ async fn handle_connection( } .map_err(|e| anyhow::anyhow!("Failed to create sign reply: {e}")) } + Request::SessionBindRequest(request) => { + span.in_scope(|| info!("Received SessionBind {:?}", request)); + connection.set_host_key(request.host_key().clone()); + info!( + "Bound connection {} to host {:?}", + connection.id(), + connection.host_name() + ); + Ok(ReplyFrame::from(AgentSuccess::new())) + } }?; span.in_scope(|| info!("Sending response")); diff --git a/apps/desktop/desktop_native/ssh_agent/src/protocol/connection.rs b/apps/desktop/desktop_native/ssh_agent/src/protocol/connection.rs index 36ef0f03749..afa7f6705b0 100644 --- a/apps/desktop/desktop_native/ssh_agent/src/protocol/connection.rs +++ b/apps/desktop/desktop_native/ssh_agent/src/protocol/connection.rs @@ -1,6 +1,7 @@ use std::sync::atomic::{AtomicU32, Ordering}; use crate::{ + hostinfo, protocol::types::{PublicKey, SessionId}, transport::peer_info::PeerInfo, }; @@ -15,6 +16,7 @@ pub struct ConnectionInfo { is_forwarding: bool, host_key: Option, + host_name: Option, session_identifier: Option, } @@ -26,6 +28,7 @@ impl ConnectionInfo { peer_info, is_forwarding: false, host_key: None, + host_name: None, session_identifier: None, } } @@ -50,8 +53,18 @@ impl ConnectionInfo { self.host_key.as_ref() } + pub fn host_name(&self) -> Option<&String> { + self.host_name.as_ref() + } + pub fn set_host_key(&mut self, host_key: PublicKey) { - self.host_key = Some(host_key); + self.host_key = Some(host_key.clone()); + // Some systems (flatpak, macos sandbox) may prevent access to the known hosts file. + if let Ok(hosts) = hostinfo::KnownHostsReader::read_default() { + self.host_name = hosts + .find_host(&host_key) + .map(|entry| entry.hostname.clone()); + } } pub fn session_identifier(&self) -> Option<&SessionId> { diff --git a/apps/desktop/desktop_native/ssh_agent/src/protocol/replies.rs b/apps/desktop/desktop_native/ssh_agent/src/protocol/replies.rs index f010dd2706b..a7326ace171 100644 --- a/apps/desktop/desktop_native/ssh_agent/src/protocol/replies.rs +++ b/apps/desktop/desktop_native/ssh_agent/src/protocol/replies.rs @@ -140,3 +140,16 @@ impl From for ReplyFrame { ReplyFrame::new(ReplyType::SSH_AGENT_FAILURE, Vec::new()) } } + +pub(crate) struct AgentSuccess; +impl AgentSuccess { + pub fn new() -> Self { + Self {} + } +} + +impl From for ReplyFrame { + fn from(_value: AgentSuccess) -> Self { + ReplyFrame::new(ReplyType::SSH_AGENT_SUCCESS, Vec::new()) + } +} diff --git a/apps/desktop/desktop_native/ssh_agent/src/protocol/requests.rs b/apps/desktop/desktop_native/ssh_agent/src/protocol/requests.rs index 1ce4cf34e99..052d8f7c522 100644 --- a/apps/desktop/desktop_native/ssh_agent/src/protocol/requests.rs +++ b/apps/desktop/desktop_native/ssh_agent/src/protocol/requests.rs @@ -50,6 +50,8 @@ pub(crate) enum Request { IdentitiesRequest, /// Sign an authentication request or SSHSIG request SignRequest(SshSignRequest), + /// Session bind request + SessionBindRequest(SessionBindRequest), } impl TryFrom<&[u8]> for Request { @@ -78,12 +80,12 @@ impl TryFrom<&[u8]> for Request { } RequestType::SSH_AGENTC_EXTENSION => { // Only support session bind for now - let _extension_request: SessionBindRequest = contents.as_slice().try_into()?; - info!( - "Received extension request, handling not yet implemented: {:?}", - _extension_request - ); - Err(anyhow::anyhow!("Unsupported extension request")) + let extension_request: SessionBindRequest = contents.as_slice().try_into()?; + if !extension_request.verify_signature() { + info!("Invalid session bind signature"); + return Err(anyhow::anyhow!("Invalid session bind signature")); + } + Ok(Request::SessionBindRequest(extension_request)) } _ => Err(anyhow::anyhow!("Unsupported request type: {:?}", r#type)), } @@ -253,7 +255,7 @@ impl From for Extension { /// string signature /// bool is_forwarding #[derive(Debug)] -struct SessionBindRequest { +pub(crate) struct SessionBindRequest { #[allow(unused)] host_key: PublicKey, #[allow(unused)] @@ -264,6 +266,30 @@ struct SessionBindRequest { is_forwarding: bool, } +impl SessionBindRequest { + pub fn verify_signature(&self) -> bool { + match self.signature.verify( + &self.host_key, + Vec::from(self.session_id.clone()).as_slice(), + ) { + Ok(valid) => { + if !valid { + info!("Invalid session bind signature"); + } + valid + } + Err(e) => { + info!("Failed to verify session bind signature: {e}"); + false + } + } + } + + pub fn host_key(&self) -> &PublicKey { + &self.host_key + } +} + impl TryFrom<&[u8]> for SessionBindRequest { type Error = anyhow::Error; diff --git a/apps/desktop/desktop_native/ssh_agent/src/protocol/types.rs b/apps/desktop/desktop_native/ssh_agent/src/protocol/types.rs index 786e918bafc..244020d1267 100644 --- a/apps/desktop/desktop_native/ssh_agent/src/protocol/types.rs +++ b/apps/desktop/desktop_native/ssh_agent/src/protocol/types.rs @@ -73,8 +73,8 @@ impl Signature { public_key: &PublicKey, data: &[u8], ) -> Result { - let public_key_parsed = - ssh_key::PublicKey::from_bytes(public_key.blob()).map_err(|e| anyhow::anyhow!(e))?; + let public_key_parsed = ssh_key::PublicKey::from_openssh(&public_key.to_string()) + .map_err(|e| anyhow::anyhow!("Failed to parse public key: {e}"))?; match self.0.algorithm() { Algorithm::Ed25519 => { @@ -188,6 +188,12 @@ impl TryFrom<&[u8]> for Signature { #[derive(Clone)] pub struct SessionId(Vec); +impl From for Vec { + fn from(sid: SessionId) -> Self { + sid.0 + } +} + impl From> for SessionId { fn from(v: Vec) -> Self { SessionId(v) @@ -328,6 +334,14 @@ impl PublicKey { let blob = read_bytes(&mut bytes)?; Ok(PublicKey { alg, blob }) } + + fn to_string(&self) -> String { + let mut buf = Vec::new(); + self.alg().as_bytes().encode(&mut buf).unwrap(); + self.blob().encode(&mut buf).unwrap(); + let buf_b64 = BASE64_STANDARD.encode(&buf); + format!("{} {}", self.alg(), buf_b64) + } } impl TryFrom for ssh_key::PublicKey { @@ -344,9 +358,26 @@ impl TryFrom> for PublicKey { } } +impl TryFrom for PublicKey { + type Error = anyhow::Error; + fn try_from(s: String) -> Result { + // split by space + let parts: Vec<&str> = s.split_whitespace().collect(); + if parts.len() < 2 { + return Err(anyhow::anyhow!("Invalid public key format")); + } + + let blob_b64 = parts[1]; + let blob = BASE64_STANDARD + .decode(blob_b64) + .map_err(|e| anyhow::anyhow!("Failed to decode base64: {e}"))?; + Self::try_read_from(blob.as_slice()) + } +} + impl Debug for PublicKey { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "SshPublicKey(\"{} {}\")", self.alg(), self.blob_b64()) + write!(f, "SshPublicKey(\"{}\")", self.to_string()) } } @@ -358,10 +389,6 @@ impl PublicKey { fn blob(&self) -> &[u8] { &self.blob } - - fn blob_b64(&self) -> String { - BASE64_STANDARD.encode(self.blob()) - } } fn parse_key_safe(pem: &str) -> Result { @@ -375,3 +402,15 @@ fn parse_key_safe(pem: &str) -> Result Err(anyhow::Error::msg(format!("Failed to parse key: {e}"))), } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_public_key_try_from_string() { + let pubkey_str = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIC3F6YkV6vV8Y5Q9Y5Z5b5Z5b5Z5b5Z5b5Z5b5Z5b5Z5 user@host"; + let public_key = PublicKey::try_from(pubkey_str.to_string()).unwrap(); + assert_eq!(public_key.alg(), "ssh-ed25519"); + } +}