From bc0819b0dac90e8ad2be4a9f91aa1dd99a798e00 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Thu, 16 Oct 2025 14:49:29 +0200 Subject: [PATCH] Cleanup --- .../src/agent/{agent.rs => desktop_agent.rs} | 9 ++-- .../desktop_native/ssh_agent/src/agent/mod.rs | 4 +- .../ssh_agent/src/agent/ui_requester.rs | 2 +- .../{protocol.rs => agent_listener.rs} | 4 +- .../ssh_agent/src/protocol/key_store.rs | 9 ---- .../ssh_agent/src/protocol/mod.rs | 2 +- .../ssh_agent/src/protocol/replies.rs | 12 ++--- .../ssh_agent/src/protocol/requests.rs | 2 +- .../ssh_agent/src/protocol/types.rs | 49 +++++++++---------- .../src/transport/unix_listener_stream.rs | 2 +- 10 files changed, 41 insertions(+), 54 deletions(-) rename apps/desktop/desktop_native/ssh_agent/src/agent/{agent.rs => desktop_agent.rs} (90%) rename apps/desktop/desktop_native/ssh_agent/src/protocol/{protocol.rs => agent_listener.rs} (98%) diff --git a/apps/desktop/desktop_native/ssh_agent/src/agent/agent.rs b/apps/desktop/desktop_native/ssh_agent/src/agent/desktop_agent.rs similarity index 90% rename from apps/desktop/desktop_native/ssh_agent/src/agent/agent.rs rename to apps/desktop/desktop_native/ssh_agent/src/agent/desktop_agent.rs index 7cafb826d5c..04374d5720f 100644 --- a/apps/desktop/desktop_native/ssh_agent/src/agent/agent.rs +++ b/apps/desktop/desktop_native/ssh_agent/src/agent/desktop_agent.rs @@ -6,7 +6,7 @@ use tokio_util::sync::CancellationToken; use crate::{ agent::ui_requester::UiRequester, memory::UnlockedSshItem, - protocol::{self, key_store::Agent, protocol::serve_listener, types::PublicKeyWithName}, + protocol::{self, agent_listener::serve_listener, key_store::Agent, types::PublicKeyWithName}, transport::peer_info::PeerInfo, }; @@ -31,9 +31,10 @@ impl BitwardenDesktopAgent { S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Send + Sync + Unpin + 'static, L: Stream> + Unpin, { - serve_listener(listener, self.cancellation_token.clone(), self) - .await - .unwrap(); + let err = serve_listener(listener, self.cancellation_token.clone(), self).await; + if let Err(e) = err { + tracing::error!("Error in agent listener: {e}"); + } } pub fn stop(&self) { diff --git a/apps/desktop/desktop_native/ssh_agent/src/agent/mod.rs b/apps/desktop/desktop_native/ssh_agent/src/agent/mod.rs index 88567fcc1f4..c4a83eeb464 100644 --- a/apps/desktop/desktop_native/ssh_agent/src/agent/mod.rs +++ b/apps/desktop/desktop_native/ssh_agent/src/agent/mod.rs @@ -1,5 +1,5 @@ -pub mod agent; +pub mod desktop_agent; pub mod ui_requester; -pub use agent::BitwardenDesktopAgent; +pub use desktop_agent::BitwardenDesktopAgent; mod platform; pub use platform::PlatformListener; 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 c050a072ece..27243fcd4ea 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 @@ -12,7 +12,7 @@ pub struct UiRequester { get_ui_response_rx: Arc>>, } -const REQUEST_ID_COUNTER: AtomicU32 = AtomicU32::new(0); +static REQUEST_ID_COUNTER: AtomicU32 = AtomicU32::new(0); #[derive(Clone, Debug)] pub enum UiRequestMessage { diff --git a/apps/desktop/desktop_native/ssh_agent/src/protocol/protocol.rs b/apps/desktop/desktop_native/ssh_agent/src/protocol/agent_listener.rs similarity index 98% rename from apps/desktop/desktop_native/ssh_agent/src/protocol/protocol.rs rename to apps/desktop/desktop_native/ssh_agent/src/protocol/agent_listener.rs index c91185540c3..e1506f46e78 100644 --- a/apps/desktop/desktop_native/ssh_agent/src/protocol/protocol.rs +++ b/apps/desktop/desktop_native/ssh_agent/src/protocol/agent_listener.rs @@ -98,11 +98,11 @@ async fn handle_connection( .flatten(); if let Some(ssh_item) = ssh_item { - SshSignReply::new( + SshSignReply::try_create( ssh_item.key_pair.private_key(), sign_request.payload_to_sign(), sign_request.signing_scheme(), - ) + )? .encode() } else { Ok(AgentFailure::new().into()) diff --git a/apps/desktop/desktop_native/ssh_agent/src/protocol/key_store.rs b/apps/desktop/desktop_native/ssh_agent/src/protocol/key_store.rs index a733fe4e7be..22227a56569 100644 --- a/apps/desktop/desktop_native/ssh_agent/src/protocol/key_store.rs +++ b/apps/desktop/desktop_native/ssh_agent/src/protocol/key_store.rs @@ -22,12 +22,3 @@ pub(crate) trait Agent: Send + Sync { public_key: &PublicKey, ) -> Result, anyhow::Error>; } - -#[cfg(test)] -pub const PRIVATE_ED25519_KEY: &str = "-----BEGIN OPENSSH PRIVATE KEY----- -b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW -QyNTUxOQAAACBDUDO7ChZIednIJxGA95T/ZTyREftahrFEJM/eeC8mmAAAAKByJoOYciaD -mAAAAAtzc2gtZWQyNTUxOQAAACBDUDO7ChZIednIJxGA95T/ZTyREftahrFEJM/eeC8mmA -AAAEBQK5JpycFzP/4rchfpZhbdwxjTwHNuGx2/kvG4i6xfp0NQM7sKFkh52cgnEYD3lP9l -PJER+1qGsUQkz954LyaYAAAAHHF1ZXh0ZW5ATWFjQm9vay1Qcm8tMTYubG9jYWwB ------END OPENSSH PRIVATE KEY-----"; diff --git a/apps/desktop/desktop_native/ssh_agent/src/protocol/mod.rs b/apps/desktop/desktop_native/ssh_agent/src/protocol/mod.rs index c880ba28389..9b2cb4d6809 100644 --- a/apps/desktop/desktop_native/ssh_agent/src/protocol/mod.rs +++ b/apps/desktop/desktop_native/ssh_agent/src/protocol/mod.rs @@ -1,8 +1,8 @@ //! An implementation of ssh agent +pub(crate) mod agent_listener; pub(crate) mod async_stream_wrapper; pub(crate) mod connection; pub(crate) mod key_store; -pub(crate) mod protocol; pub(crate) mod replies; pub(crate) mod requests; pub mod types; 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 a36800c07bc..88d862c4dde 100644 --- a/apps/desktop/desktop_native/ssh_agent/src/protocol/replies.rs +++ b/apps/desktop/desktop_native/ssh_agent/src/protocol/replies.rs @@ -90,15 +90,15 @@ impl IdentitiesReply { pub(crate) struct SshSignReply(Signature); impl SshSignReply { - pub fn new( + pub fn try_create( private_key: &PrivateKey, data: &[u8], requested_signing_scheme: Option, - ) -> Self { - Self( + ) -> Result { + Ok(Self( // Note, this should take into account the extension / signing scheme. - private_key.sign(data, requested_signing_scheme).unwrap(), - ) + private_key.sign(data, requested_signing_scheme)?, + )) } /// `https://www.ietf.org/archive/id/draft-miller-ssh-agent-11.html#name-private-key-operations` @@ -109,7 +109,7 @@ impl SshSignReply { pub fn encode(&self) -> Result { Ok(ReplyFrame::new(ReplyType::SSH_AGENT_SIGN_RESPONSE, { let mut reply_payload = Vec::new(); - self.0.encode().unwrap().encode(&mut reply_payload)?; + self.0.encode()?.encode(&mut reply_payload)?; reply_payload })) } 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 f771b548b70..5fc66e33b95 100644 --- a/apps/desktop/desktop_native/ssh_agent/src/protocol/requests.rs +++ b/apps/desktop/desktop_native/ssh_agent/src/protocol/requests.rs @@ -337,7 +337,7 @@ mod tests { let req = ParsedSignRequest::try_from(TEST_VECTOR_REQUEST_SIGN_SSHSIG_GIT).expect("Should parse"); assert!( - matches!(req, ParsedSignRequest::SshSigRequest { namespace } if namespace == "git".to_string()) + matches!(req, ParsedSignRequest::SshSigRequest { namespace } if namespace == *"git".to_string()) ); } } 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 59ac2eecf5e..786e918bafc 100644 --- a/apps/desktop/desktop_native/ssh_agent/src/protocol/types.rs +++ b/apps/desktop/desktop_native/ssh_agent/src/protocol/types.rs @@ -117,7 +117,7 @@ impl Signature { let sec1_bytes = public_key_parsed .key_data() .ecdsa() - .unwrap() + .ok_or(anyhow::anyhow!("Ecdsa key failed to parse"))? .as_sec1_bytes(); match curve { EcdsaCurve::NistP256 => { @@ -178,10 +178,9 @@ impl TryFrom<&[u8]> for Signature { type Error = anyhow::Error; fn try_from(bytes: &[u8]) -> Result { let mut buffer = bytes; - let alg = Algorithm::new( - String::from_utf8_lossy(read_bytes(&mut buffer).unwrap().as_slice()).as_ref(), - )?; - let sig = read_bytes(&mut buffer).unwrap(); + let alg = + Algorithm::new(String::from_utf8_lossy(read_bytes(&mut buffer)?.as_slice()).as_ref())?; + let sig = read_bytes(&mut buffer)?; Ok(Signature(ssh_key::Signature::new(alg, sig)?)) } } @@ -284,15 +283,24 @@ impl TryFrom for PrivateKey { fn try_from(key: ssh_key::private::PrivateKey) -> Result { match key.algorithm() { - ssh_key::Algorithm::Ed25519 => { - Ok(Self::Ed25519(key.key_data().ed25519().unwrap().to_owned())) - } - ssh_key::Algorithm::Rsa { hash: _ } => { - Ok(Self::Rsa(key.key_data().rsa().unwrap().to_owned())) - } - ssh_key::Algorithm::Ecdsa { curve: _ } => { - Ok(Self::Ecdsa(key.key_data().ecdsa().unwrap().to_owned())) - } + ssh_key::Algorithm::Ed25519 => Ok(Self::Ed25519( + key.key_data() + .ed25519() + .ok_or(anyhow::anyhow!("Failed to parse ed25519 key"))? + .to_owned(), + )), + ssh_key::Algorithm::Rsa { hash: _ } => Ok(Self::Rsa( + key.key_data() + .rsa() + .ok_or(anyhow::anyhow!("Failed to parse RSA key"))? + .to_owned(), + )), + ssh_key::Algorithm::Ecdsa { curve: _ } => Ok(Self::Ecdsa( + key.key_data() + .ecdsa() + .ok_or(anyhow::anyhow!("Failed to parse ECDSA key"))? + .to_owned(), + )), _ => Err(anyhow::anyhow!("Unsupported key type")), } } @@ -367,16 +375,3 @@ fn parse_key_safe(pem: &str) -> Result Err(anyhow::Error::msg(format!("Failed to parse key: {e}"))), } } - -#[cfg(test)] -mod tests { - use crate::protocol::key_store::PRIVATE_ED25519_KEY; - - use super::*; - - #[test] - fn test_keypair_creation() { - let private_key = PrivateKey::try_from(PRIVATE_ED25519_KEY.to_string()) - .expect("Test key is always valid"); - } -} diff --git a/apps/desktop/desktop_native/ssh_agent/src/transport/unix_listener_stream.rs b/apps/desktop/desktop_native/ssh_agent/src/transport/unix_listener_stream.rs index 25125af8d46..058519cde38 100644 --- a/apps/desktop/desktop_native/ssh_agent/src/transport/unix_listener_stream.rs +++ b/apps/desktop/desktop_native/ssh_agent/src/transport/unix_listener_stream.rs @@ -6,7 +6,7 @@ use std::{fs, io}; use tokio::net::{UnixListener, UnixStream}; use tracing::{error, info}; -use crate::agent::agent::BitwardenDesktopAgent; +use crate::agent::desktop_agent::BitwardenDesktopAgent; use crate::transport::peer_info::{PeerInfo, PeerType}; pub struct UnixListenerStream {