From 53f2b6dcf0259ac8995c1c7e1fcd700733416df6 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Thu, 12 Dec 2024 16:00:50 +0100 Subject: [PATCH 01/12] Add agent-forwarding detection and git signing detection parsers --- apps/desktop/desktop_native/Cargo.lock | 1 - apps/desktop/desktop_native/core/Cargo.toml | 2 +- .../desktop_native/core/src/ssh_agent/mod.rs | 45 +++++++++- .../peercred_unix_listener_stream.rs | 7 +- .../core/src/ssh_agent/peerinfo/models.rs | 21 +++++ .../core/src/ssh_agent/request_parser.rs | 31 +++++++ apps/desktop/desktop_native/napi/index.d.ts | 2 +- apps/desktop/desktop_native/napi/src/lib.rs | 5 +- apps/desktop/src/locales/en/messages.json | 18 ++++ .../components/approve-ssh-request.html | 11 ++- .../components/approve-ssh-request.ts | 20 ++++- .../platform/main/main-ssh-agent.service.ts | 89 +++++++++++-------- .../platform/services/ssh-agent.service.ts | 6 ++ 13 files changed, 206 insertions(+), 52 deletions(-) create mode 100644 apps/desktop/desktop_native/core/src/ssh_agent/request_parser.rs diff --git a/apps/desktop/desktop_native/Cargo.lock b/apps/desktop/desktop_native/Cargo.lock index 09d3d15e897..5bd91407a67 100644 --- a/apps/desktop/desktop_native/Cargo.lock +++ b/apps/desktop/desktop_native/Cargo.lock @@ -338,7 +338,6 @@ checksum = "b048fb63fd8b5923fc5aa7b340d8e156aec7ec02f0c78fa8a6ddc2613f6f71de" [[package]] name = "bitwarden-russh" version = "0.1.0" -source = "git+https://github.com/bitwarden/bitwarden-russh.git?rev=23b50e3bbe6d56ef19ab0e98e8bb1462cb6d77ae#23b50e3bbe6d56ef19ab0e98e8bb1462cb6d77ae" dependencies = [ "anyhow", "byteorder", diff --git a/apps/desktop/desktop_native/core/Cargo.toml b/apps/desktop/desktop_native/core/Cargo.toml index 597a082b231..aba793aa5a2 100644 --- a/apps/desktop/desktop_native/core/Cargo.toml +++ b/apps/desktop/desktop_native/core/Cargo.toml @@ -49,7 +49,7 @@ ssh-key = { version = "=0.6.7", default-features = false, features = [ "rsa", "getrandom", ] } -bitwarden-russh = { git = "https://github.com/bitwarden/bitwarden-russh.git", rev = "23b50e3bbe6d56ef19ab0e98e8bb1462cb6d77ae" } +bitwarden-russh = { path = "/Users/quexten/projects/bitwarden-russh" } tokio = { version = "=1.41.1", features = ["io-util", "sync", "macros", "net"] } tokio-stream = { version = "=0.1.15", features = ["net"] } tokio-util = { version = "=0.7.12", features = ["codec"] } diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs b/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs index 82b90c7bff9..21f3aecbbc0 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs @@ -19,6 +19,8 @@ mod peercred_unix_listener_stream; pub mod generator; pub mod importer; pub mod peerinfo; +mod request_parser; + #[derive(Clone)] pub struct BitwardenDesktopAgent { keystore: ssh_agent::KeyStore, @@ -36,19 +38,40 @@ pub struct SshAgentUIRequest { pub cipher_id: Option, pub process_name: String, pub is_list: bool, + pub is_sig: bool, + pub is_git: bool, + pub is_forwarding: bool, } impl ssh_agent::Agent for BitwardenDesktopAgent { - async fn confirm(&self, ssh_key: Key, info: &peerinfo::models::PeerInfo) -> bool { + async fn confirm(&self, ssh_key: Key, data: &[u8], info: &peerinfo::models::PeerInfo) -> bool { if !self.is_running() { println!("[BitwardenDesktopAgent] Agent is not running, but tried to call confirm"); return false; } let request_id = self.get_request_id().await; + let request_data = request_parser::parse_request(data); + if let Err(e) = request_data { + println!("[SSH Agent] Error while parsing request: {}", e); + return false; + } + let request_data = request_data.unwrap(); + let is_sig = match request_data { + request_parser::SshAgentSignRequest::SshSigRequest(_) => true, + request_parser::SshAgentSignRequest::SignRequest(_) => false, + }; + let is_git = is_sig && match request_data { + request_parser::SshAgentSignRequest::SshSigRequest(ref req) => req.namespace == "git", + _ => false, + }; + println!( - "[SSH Agent] Confirming request from application: {}", - info.process_name() + "[SSH Agent] Confirming request from application: {}, is_forwarding: {}, is_sshsig: {}, is_git: {}", + info.process_name(), + info.is_forwarding(), + is_sig, + is_git ); let mut rx_channel = self.get_ui_response_rx.lock().await.resubscribe(); @@ -58,6 +81,9 @@ impl ssh_agent::Agent for BitwardenDesktopAgent { cipher_id: Some(ssh_key.cipher_uuid.clone()), process_name: info.process_name().to_string(), is_list: false, + is_sig, + is_git, + is_forwarding: info.is_forwarding(), }) .await .expect("Should send request to ui"); @@ -82,6 +108,9 @@ impl ssh_agent::Agent for BitwardenDesktopAgent { cipher_id: None, process_name: info.process_name().to_string(), is_list: true, + is_sig: false, + is_git: false, + is_forwarding: info.is_forwarding(), }; self.show_ui_request_tx .send(message) @@ -94,6 +123,16 @@ impl ssh_agent::Agent for BitwardenDesktopAgent { } false } + + async fn set_is_forwarding( + &self, + is_forwarding: bool, + connection_info: &peerinfo::models::PeerInfo, + ) -> () { + if (is_forwarding) { + connection_info.set_forwarding(is_forwarding); + } + } } impl BitwardenDesktopAgent { diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/peercred_unix_listener_stream.rs b/apps/desktop/desktop_native/core/src/ssh_agent/peercred_unix_listener_stream.rs index f0114fc08da..58fc61cbcb1 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/peercred_unix_listener_stream.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/peercred_unix_listener_stream.rs @@ -37,11 +37,8 @@ impl Stream for PeercredUnixListenerStream { )))); } }, - Err(err) => { - return Poll::Ready(Some(Err(io::Error::new( - io::ErrorKind::Other, - format!("Failed to get peer credentials: {}", err), - )))); + Err(_) => { + return Poll::Ready(Some(Ok((stream, PeerInfo::unknown())))) } }; let peer_info = peerinfo::gather::get_peer_info(pid as u32); diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/peerinfo/models.rs b/apps/desktop/desktop_native/core/src/ssh_agent/peerinfo/models.rs index 823d912883e..76976b6f77a 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/peerinfo/models.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/peerinfo/models.rs @@ -1,3 +1,5 @@ +use std::sync::{atomic::AtomicBool, Arc}; + /** * Peerinfo represents the information of a peer process connecting over a socket. * This can be later extended to include more information (icon, app name) for the corresponding application. @@ -7,6 +9,7 @@ pub struct PeerInfo { uid: u32, pid: u32, process_name: String, + is_forwarding: Arc, } impl PeerInfo { @@ -15,6 +18,16 @@ impl PeerInfo { uid, pid, process_name, + is_forwarding: Arc::new(AtomicBool::new(false)), + } + } + + pub fn unknown() -> Self { + Self { + uid: 0, + pid: 0, + process_name: "Unknown".to_string(), + is_forwarding: Arc::new(AtomicBool::new(false)), } } @@ -29,4 +42,12 @@ impl PeerInfo { pub fn process_name(&self) -> &str { &self.process_name } + + pub fn is_forwarding(&self) -> bool { + self.is_forwarding.load(std::sync::atomic::Ordering::Relaxed) + } + + pub fn set_forwarding(&self, value: bool) { + self.is_forwarding.store(value, std::sync::atomic::Ordering::Relaxed); + } } diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/request_parser.rs b/apps/desktop/desktop_native/core/src/ssh_agent/request_parser.rs new file mode 100644 index 00000000000..7b2120d3d74 --- /dev/null +++ b/apps/desktop/desktop_native/core/src/ssh_agent/request_parser.rs @@ -0,0 +1,31 @@ +#[derive(Debug)] +pub(crate) struct SshSigRequest { + pub namespace: String, +} + +#[derive(Debug)] +pub(crate) struct SignRequest {} + +#[derive(Debug)] +pub(crate) enum SshAgentSignRequest { + SshSigRequest(SshSigRequest), + SignRequest(SignRequest), +} + +pub(crate) fn parse_request(data: &[u8]) -> Result { + // ssh-sig + let magic_header = "SSHSIG"; + let mut data_iter = data.to_vec().into_iter(); + let header = data_iter.by_ref().take(magic_header.len()).collect::>(); + if header == magic_header.as_bytes() { + let version = data_iter.by_ref().take(4).collect::>(); + let _version = u32::from_be_bytes(version.try_into().map_err(|_| anyhow::anyhow!("Invalid version"))?); + + // read until null byte + let namespace = data_iter.by_ref().take_while(|&x| x != 0).collect::>(); + let namespace = String::from_utf8(namespace).map_err(|_| anyhow::anyhow!("Invalid namespace"))?; + return Ok(SshAgentSignRequest::SshSigRequest(SshSigRequest { namespace })) + } + + Ok(SshAgentSignRequest::SignRequest(SignRequest {})) +} \ No newline at end of file diff --git a/apps/desktop/desktop_native/napi/index.d.ts b/apps/desktop/desktop_native/napi/index.d.ts index b884829e77d..8338d1f1837 100644 --- a/apps/desktop/desktop_native/napi/index.d.ts +++ b/apps/desktop/desktop_native/napi/index.d.ts @@ -67,7 +67,7 @@ export declare namespace sshagent { status: SshKeyImportStatus sshKey?: SshKey } - export function serve(callback: (err: Error | null, arg0: string | undefined | null, arg1: boolean, arg2: string) => any): Promise + export function serve(callback: (err: Error | null, arg0: string | undefined | null, arg1: boolean, arg2: string, arg3: boolean, arg4: boolean, arg5: boolean) => any): Promise export function stop(agentState: SshAgentState): void export function isRunning(agentState: SshAgentState): boolean export function setKeys(agentState: SshAgentState, newKeys: Array): void diff --git a/apps/desktop/desktop_native/napi/src/lib.rs b/apps/desktop/desktop_native/napi/src/lib.rs index a7e2144b1dc..76121cf9e97 100644 --- a/apps/desktop/desktop_native/napi/src/lib.rs +++ b/apps/desktop/desktop_native/napi/src/lib.rs @@ -247,7 +247,7 @@ pub mod sshagent { #[napi] pub async fn serve( - callback: ThreadsafeFunction<(Option, bool, String), CalleeHandled>, + callback: ThreadsafeFunction<(Option, bool, String, bool, bool, bool), CalleeHandled>, ) -> napi::Result { let (auth_request_tx, mut auth_request_rx) = tokio::sync::mpsc::channel::(32); @@ -268,6 +268,9 @@ pub mod sshagent { request.cipher_id, request.is_list, request.process_name, + request.is_forwarding, + request.is_sig, + request.is_git ))) .await; match promise_result { diff --git a/apps/desktop/src/locales/en/messages.json b/apps/desktop/src/locales/en/messages.json index f8f81a5ac2c..06c7fd7c085 100644 --- a/apps/desktop/src/locales/en/messages.json +++ b/apps/desktop/src/locales/en/messages.json @@ -3371,9 +3371,27 @@ "sshkeyApprovalTitle": { "message": "Confirm SSH key usage" }, + "agentForwardingWarningTitle": { + "message": "Warning: Agent Forwarding" + }, + "agentForwardingWarningText": { + "message": "This request comes from a remote device that you are logged into" + }, "sshkeyApprovalMessageInfix": { "message": "is requesting access to" }, + "sshkeyApprovalMessageSuffix": { + "message": "is requesting access to" + }, + "sshActionLogin": { + "message": "authenticate to a server" + }, + "sshActionSign": { + "message": "sign a message" + }, + "sshActionGitSign": { + "message": "sign a git commit" + }, "unknownApplication": { "message": "An application" }, diff --git a/apps/desktop/src/platform/components/approve-ssh-request.html b/apps/desktop/src/platform/components/approve-ssh-request.html index eac451a1fbe..952e3344e9c 100644 --- a/apps/desktop/src/platform/components/approve-ssh-request.html +++ b/apps/desktop/src/platform/components/approve-ssh-request.html @@ -2,8 +2,17 @@
{{ "sshkeyApprovalTitle" | i18n }}
+ + {{ 'agentForwardingWarningText' | i18n }} + + {{params.applicationName}} {{ "sshkeyApprovalMessageInfix" | i18n }} - {{params.cipherName}}. + {{params.cipherName}} + {{ "sshkeyApprovalMessageSuffix" | i18n }} {{ params.action | i18n }}