API cleanup

Author: Eugeny

russh/src/auth.rs

@@ -25,6 +25,7 @@ use tokio::io::{AsyncRead, AsyncWrite};
 use crate::CryptoVec;
 use crate::helpers::NameList;
 use crate::keys::PrivateKeyWithHashAlg;
+use crate::keys::agent::AgentIdentity;
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum MethodKind {
@@ -157,28 +158,12 @@ impl AuthResult {
 pub trait Signer: Sized {
     type Error: From<crate::SendError>;
 
-    fn auth_publickey_sign(
+    fn auth_sign(
         &mut self,
-        key: &ssh_key::PublicKey,
+        key: &AgentIdentity,
         hash_alg: Option<HashAlg>,
         to_sign: CryptoVec,
     ) -> impl Future<Output = Result<CryptoVec, Self::Error>> + Send;
-
-    /// Sign authentication data using a certificate identity.
-    ///
-    /// The default implementation returns an error, indicating certificate
-    /// signing is not supported. Implementations that support certificate
-    /// signing (e.g., SSH agent clients) should override this method.
-    ///
-    /// For RSA certificates, you can specify the hash algorithm to use.
-    fn auth_certificate_sign(
-        &mut self,
-        _cert: &Certificate,
-        _hash_alg: Option<HashAlg>,
-        _to_sign: CryptoVec,
-    ) -> impl Future<Output = Result<CryptoVec, Self::Error>> + Send {
-        async { Err((crate::SendError {}).into()) }
-    }
 }
 
 #[derive(Debug, Error)]
@@ -196,9 +181,9 @@ impl<R: AsyncRead + AsyncWrite + Unpin + Send + 'static> Signer
     type Error = AgentAuthError;
 
     #[allow(clippy::manual_async_fn)]
-    fn auth_publickey_sign(
+    fn auth_sign(
         &mut self,
-        key: &ssh_key::PublicKey,
+        key: &AgentIdentity,
         hash_alg: Option<HashAlg>,
         to_sign: CryptoVec,
     ) -> impl Future<Output = Result<CryptoVec, Self::Error>> {
@@ -208,20 +193,6 @@ impl<R: AsyncRead + AsyncWrite + Unpin + Send + 'static> Signer
                 .map_err(Into::into)
         }
     }
-
-    #[allow(clippy::manual_async_fn)]
-    fn auth_certificate_sign(
-        &mut self,
-        cert: &Certificate,
-        hash_alg: Option<HashAlg>,
-        to_sign: CryptoVec,
-    ) -> impl Future<Output = Result<CryptoVec, Self::Error>> {
-        async move {
-            self.sign_request_cert(cert, hash_alg, to_sign)
-                .await
-                .map_err(Into::into)
-        }
-    }
 }
 
 #[derive(Debug)]

russh/src/client/mod.rs

@@ -485,7 +485,7 @@ impl<H: Handler> Handle<H> {
                     });
                 }
                 Some(Reply::SignRequest { key, data }) => {
-                    let data = signer.auth_publickey_sign(&key, hash_alg, data).await;
+                    let data = signer.auth_sign(&key.into(), hash_alg, data).await;
                     let data = match data {
                         Ok(data) => data,
                         Err(e) => return Err(e),
@@ -542,8 +542,12 @@ impl<H: Handler> Handle<H> {
                         partial_success,
                     });
                 }
-                Some(Reply::SignRequestCert { cert, hash_alg, data }) => {
-                    let data = signer.auth_certificate_sign(&cert, hash_alg, data).await;
+                Some(Reply::SignRequestCert {
+                    cert,
+                    hash_alg,
+                    data,
+                }) => {
+                    let data = signer.auth_sign(&cert.into(), hash_alg, data).await;
                     let data = match data {
                         Ok(data) => data,
                         Err(e) => return Err(e),

russh/src/keys/agent/client.rs

@@ -8,10 +8,10 @@ use ssh_key::{Algorithm, Certificate, HashAlg, PrivateKey, PublicKey, Signature}
 use tokio;
 use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
 
-use super::{msg, AgentIdentity, Constraint};
-use crate::helpers::EncodedExt;
-use crate::keys::{key, Error};
+use super::{AgentIdentity, Constraint, msg};
 use crate::CryptoVec;
+use crate::helpers::EncodedExt;
+use crate::keys::{Error, key};
 
 pub trait AgentStream: AsyncRead + AsyncWrite {}
 
@@ -254,43 +254,8 @@ impl<S: AgentStream + Unpin> AgentClient<S> {
         Ok(())
     }
 
-    /// Ask the agent for a list of the currently registered public keys.
-    ///
-    /// Note: Certificates held by the agent will be returned as their underlying public
-    /// key only, without the certificate data. Use
-    /// [`request_identities_full`](Self::request_identities_full) to retrieve full
-    /// certificate information.
-    pub async fn request_identities(&mut self) -> Result<Vec<PublicKey>, Error> {
-        self.buf.clear();
-        self.buf.resize(4);
-        msg::REQUEST_IDENTITIES.encode(&mut self.buf)?;
-        let len = self.buf.len() - 4;
-        BigEndian::write_u32(&mut self.buf[..], len as u32);
-
-        self.read_response().await?;
-        debug!("identities: {:?}", &self.buf[..]);
-        let mut keys = Vec::new();
-
-        #[allow(clippy::indexing_slicing)] // static length
-        if let Some((&msg::IDENTITIES_ANSWER, mut r)) = self.buf.split_first() {
-            let n = u32::decode(&mut r)?;
-            for _ in 0..n {
-                let key_blob = Bytes::decode(&mut r)?;
-                let comment = String::decode(&mut r)?;
-                let mut key = key::parse_public_key(&key_blob)?;
-                key.set_comment(comment);
-                keys.push(key);
-            }
-        }
-
-        Ok(keys)
-    }
-
     /// Ask the agent for a list of identities, including certificates.
-    ///
-    /// Unlike [`request_identities`](Self::request_identities) which only returns public keys,
-    /// this method correctly parses OpenSSH certificates held by the agent.
-    pub async fn request_identities_full(&mut self) -> Result<Vec<AgentIdentity>, Error> {
+    pub async fn request_identities(&mut self) -> Result<Vec<AgentIdentity>, Error> {
         self.buf.clear();
         self.buf.resize(4);
         msg::REQUEST_IDENTITIES.encode(&mut self.buf)?;
@@ -313,7 +278,7 @@ impl<S: AgentStream + Unpin> AgentClient<S> {
                 // This avoids parsing the blob twice for regular keys.
                 let identity = if Self::is_certificate_blob(&key_blob) {
                     match Certificate::decode(&mut key_blob.as_ref()) {
-                        Ok(cert) => AgentIdentity::Certificate { cert, comment },
+                        Ok(cert) => AgentIdentity::Certificate { certificate: cert, comment },
                         Err(_) => {
                             // Fallback to public key if certificate parsing fails
                             let key = key::parse_public_key(&key_blob)?;
@@ -352,6 +317,20 @@ impl<S: AgentStream + Unpin> AgentClient<S> {
 
     /// Ask the agent to sign the supplied piece of data.
     pub async fn sign_request(
+        &mut self,
+        identity: &AgentIdentity,
+        hash_alg: Option<HashAlg>,
+        data: CryptoVec,
+    ) -> Result<CryptoVec, Error> {
+        match identity {
+            AgentIdentity::PublicKey { key, .. } => self.sign_request_pk(key, hash_alg, data).await,
+            AgentIdentity::Certificate { certificate, .. } => {
+                self.sign_request_cert(certificate, hash_alg, data).await
+            }
+        }
+    }
+
+    async fn sign_request_pk(
         &mut self,
         public: &PublicKey,
         hash_alg: Option<HashAlg>,
@@ -381,7 +360,7 @@ impl<S: AgentStream + Unpin> AgentClient<S> {
     /// allowing the agent to match it to the correct private key.
     ///
     /// For RSA certificates, you can specify the hash algorithm to use.
-    pub async fn sign_request_cert(
+    async fn sign_request_cert(
         &mut self,
         cert: &Certificate,
         hash_alg: Option<HashAlg>,

russh/src/keys/agent/mod.rs

@@ -34,21 +34,39 @@ pub enum AgentIdentity {
     /// An OpenSSH certificate
     Certificate {
         /// The certificate (contains public key plus CA signature, principals, validity, etc.)
-        cert: Certificate,
+        certificate: Certificate,
         /// Comment associated with this identity
         comment: String,
     },
 }
 
+impl From<PublicKey> for AgentIdentity {
+    fn from(key: PublicKey) -> Self {
+        Self::PublicKey {
+            key,
+            comment: String::new(),
+        }
+    }
+}
+
+impl From<Certificate> for AgentIdentity {
+    fn from(certificate: Certificate) -> Self {
+        Self::Certificate {
+            certificate,
+            comment: String::new(),
+        }
+    }
+}
+
 impl AgentIdentity {
     /// Returns the underlying public key.
     /// For certificates, extracts the public key from the certificate.
     /// Returns a borrowed reference for plain keys, or an owned value for certificates.
     pub fn public_key(&self) -> Cow<'_, PublicKey> {
         match self {
             Self::PublicKey { key, .. } => Cow::Borrowed(key),
-            Self::Certificate { cert, .. } => {
-                Cow::Owned(PublicKey::new(cert.public_key().clone(), ""))
+            Self::Certificate { certificate, .. } => {
+                Cow::Owned(PublicKey::new(certificate.public_key().clone(), ""))
             }
         }
     }
@@ -66,7 +84,7 @@ impl AgentIdentity {
 mod tests {
     use super::*;
     use ssh_key::rand_core::OsRng;
-    use ssh_key::{certificate, PrivateKey};
+    use ssh_key::{PrivateKey, certificate};
 
     fn create_test_certificate() -> Certificate {
         use std::time::{SystemTime, UNIX_EPOCH};
@@ -128,7 +146,7 @@ mod tests {
         let comment = "test-cert-comment".to_string();
 
         let identity = AgentIdentity::Certificate {
-            cert: cert.clone(),
+            certificate: cert.clone(),
             comment: comment.clone(),
         };