Use ssh-key crate to decode OpenSSH public/private keys (#279)

#140

Co-authored-by: Eugene <inbox@null.page>

Cargo.toml

@@ -8,4 +8,4 @@ russh-cryptovec = { path = "cryptovec" }
 russh-config = { path = "russh-config" }
 
 [workspace.dependencies]
-ssh-key = { version = "0.6.6", features = ["ed25519", "rsa"] }
+ssh-key = { version = "0.6", features = ["ed25519", "rsa", "encryption"] }

russh-keys/Cargo.toml

@@ -67,6 +67,7 @@ serde = { version = "1.0", features = ["derive"] }
 sha1 = { version = "0.10", features = ["oid"] }
 sha2 = { version = "0.10", features = ["oid"] }
 spki = "0.7"
+ssh-encoding = "0.2"
 ssh-key = { workspace = true }
 thiserror = "1.0"
 tokio = { version = "1.17.0", features = ["io-util", "rt-multi-thread", "time", "net"] }

russh-keys/src/agent/client.rs

@@ -1,7 +1,7 @@
 use std::convert::TryFrom;
 
 use byteorder::{BigEndian, ByteOrder};
-use log::{debug, info};
+use log::debug;
 use russh_cryptovec::CryptoVec;
 use tokio;
 use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
@@ -249,34 +249,12 @@ impl<S: AsyncRead + AsyncWrite + Unpin> AgentClient<S> {
             let mut r = self.buf.reader(1);
             let n = r.read_u32()?;
             for _ in 0..n {
-                let key = r.read_string()?;
-                let _ = r.read_string()?;
-                let mut r = key.reader(0);
-                let t = r.read_string()?;
-                debug!("t = {:?}", std::str::from_utf8(t));
-                match t {
-                    b"ssh-rsa" => keys.push(key::PublicKey::new_rsa_with_hash(
-                        &r.read_ssh()?,
-                        SignatureHash::SHA2_512,
-                    )?),
-                    b"ssh-ed25519" => keys.push(PublicKey::Ed25519(
-                        ed25519_dalek::VerifyingKey::try_from(r.read_string()?)?,
-                    )),
-                    crate::KEYTYPE_ECDSA_SHA2_NISTP256
-                    | crate::KEYTYPE_ECDSA_SHA2_NISTP384
-                    | crate::KEYTYPE_ECDSA_SHA2_NISTP521 => {
-                        let curve = r.read_string()?;
-                        let sec1_bytes = r.read_string()?;
-                        let key = crate::ec::PublicKey::from_sec1_bytes(t, sec1_bytes)?;
-                        if curve != key.ident().as_bytes() {
-                            return Err(Error::CouldNotReadKey);
-                        }
-                        keys.push(PublicKey::EC { key })
-                    }
-                    t => {
-                        info!("Unsupported key type: {:?}", std::str::from_utf8(t))
-                    }
-                }
+                let key_blob = r.read_string()?;
+                let _comment = r.read_string()?;
+                keys.push(key::parse_public_key(
+                    key_blob,
+                    Some(SignatureHash::SHA2_512),
+                )?);
             }
         }
 

russh-keys/src/agent/server.rs

@@ -15,8 +15,7 @@ use {std, tokio};
 
 use super::{msg, Constraint};
 use crate::encoding::{Encoding, Position, Reader};
-use crate::key::SignatureHash;
-use crate::{key, protocol, Error};
+use crate::{key, Error};
 
 #[derive(Clone)]
 #[allow(clippy::type_complexity)]
@@ -251,44 +250,22 @@ impl<S: AsyncRead + AsyncWrite + Send + Unpin + 'static, A: Agent + Send + Sync
         constrained: bool,
         writebuf: &mut CryptoVec,
     ) -> Result<bool, Error> {
-        let pos0 = r.position;
-        let t = r.read_string()?;
-        let (blob, key) = match t {
-            b"ssh-ed25519" => {
-                let _public = r.read_string()?;
-                let pos1 = r.position;
-                let concat = r.read_string()?;
-                let _comment = r.read_string()?;
-                #[allow(clippy::indexing_slicing)] // length checked before
-                let secret = ed25519_dalek::SigningKey::try_from(
-                    concat.get(..32).ok_or(Error::KeyIsCorrupt)?,
-                )
-                .map_err(|_| Error::KeyIsCorrupt)?;
+        let (blob, key_pair) = {
+            use ssh_encoding::{Decode, Encode};
 
-                writebuf.push(msg::SUCCESS);
+            let private_key = ssh_key::private::PrivateKey::new(
+                ssh_key::private::KeypairData::decode(&mut r)?,
+                "",
+            )?;
+            let _comment = r.read_string()?;
+            let key_pair = key::KeyPair::try_from(&private_key)?;
 
-                #[allow(clippy::indexing_slicing)] // positions checked before
-                (self.buf[pos0..pos1].to_vec(), key::KeyPair::Ed25519(secret))
-            }
-            b"ssh-rsa" => {
-                let key =
-                    key::KeyPair::new_rsa_with_hash(&r.read_ssh()?, None, SignatureHash::SHA2_256)?;
-
-                let mut blob = Vec::new();
-                if let key::KeyPair::RSA { ref key, .. } = key {
-                    let public = protocol::RsaPublicKey::from(key);
-                    blob.extend_ssh_string(b"ssh-rsa");
-                    blob.extend_ssh(&public);
-                } else {
-                    return Err(Error::KeyIsCorrupt);
-                };
+            let mut blob = Vec::new();
+            private_key.public_key().key_data().encode(&mut blob)?;
 
-                writebuf.push(msg::SUCCESS);
-
-                (blob, key)
-            }
-            _ => return Ok(false),
+            (blob, key_pair)
         };
+        writebuf.push(msg::SUCCESS);
         let mut w = self.keys.0.write().or(Err(Error::AgentFailure))?;
         let now = SystemTime::now();
         if constrained {
@@ -318,9 +295,9 @@ impl<S: AsyncRead + AsyncWrite + Send + Unpin + 'static, A: Agent + Send + Sync
                     return Ok(false);
                 }
             }
-            w.insert(blob, (Arc::new(key), now, c));
+            w.insert(blob, (Arc::new(key_pair), now, c));
         } else {
-            w.insert(blob, (Arc::new(key), now, Vec::new()));
+            w.insert(blob, (Arc::new(key_pair), now, Vec::new()));
         }
         Ok(true)
     }

russh-keys/src/encoding.rs

@@ -296,3 +296,19 @@ pub trait SshRead<'a>: Sized + 'a {
     /// Read the value from a position.
     fn read_ssh(pos: &mut Position<'a>) -> Result<Self, Error>;
 }
+
+impl<'a> ssh_encoding::Reader for Position<'a> {
+    fn read<'o>(&mut self, out: &'o mut [u8]) -> ssh_encoding::Result<&'o [u8]> {
+        out.copy_from_slice(
+            self.s
+                .get(self.position..(self.position + out.len()))
+                .ok_or(ssh_encoding::Error::Length)?,
+        );
+        self.position += out.len();
+        Ok(out)
+    }
+
+    fn remaining_len(&self) -> usize {
+        self.s.len() - self.position
+    }
+}