From 59bece8b01a1b4bffc458042eace4e34b131cce4 Mon Sep 17 00:00:00 2001
From: Matt Johnston <matt@ucc.asn.au>
Date: Sun, 6 Nov 2022 23:38:52 +0800
Subject: [PATCH] Add SignKey::generate() and KeyType

Improve the roundtrip pubkey auth test
---
 src/lib.rs     |  2 +-
 src/packets.rs | 25 ++++++++++++++++---------
 src/sign.rs    | 37 +++++++++++++++++++++++--------------
 src/sshwire.rs |  2 +-
 4 files changed, 41 insertions(+), 25 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index 34fba4b..25f0b47 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -49,7 +49,7 @@ pub use behaviour::{Behaviour, ServBehaviour, CliBehaviour,
 pub use sshwire::TextString;
 
 pub use runner::Runner;
-pub use sign::SignKey;
+pub use sign::{SignKey, KeyType};
 pub use packets::PubKey;
 pub use error::{Error,Result};
 pub use channel::{ChanMsg, ChanMsgDetails, Pty, ChanOpened};
diff --git a/src/packets.rs b/src/packets.rs
index ec6e081..0f16f63 100644
--- a/src/packets.rs
+++ b/src/packets.rs
@@ -321,7 +321,9 @@ impl<'a> Signature<'a> {
     /// Returns the signature algorithm name for a public key.
     /// Returns (`Error::UnknownMethod`) if the PubKey is unknown
     /// Currently can return a unique signature name for a public key
-    /// since ssh-rsa isn't supported, only rsa-sha2-256 (as an example)
+    /// since ssh-rsa isn't supported, only rsa-sha2-256.
+    /// It's possible that in future there isn't a distinct signature
+    /// type for each key type.
     pub fn sig_name_for_pubkey(pubkey: &PubKey) -> Result<&'static str> {
         match pubkey {
             PubKey::Ed25519(_) => Ok(SSH_NAME_ED25519),
@@ -349,7 +351,7 @@ impl <'a> From<&'a OwnedSig> for Signature<'a> {
     fn from(s: &'a OwnedSig) -> Self {
         match s {
             OwnedSig::Ed25519(e) => Signature::Ed25519(Ed25519Sig { sig: BinString(e) }),
-            OwnedSig::RSA256 => todo!("sig from rsa"),
+            OwnedSig::_RSA256 => todo!("sig from rsa"),
         }
     }
 }
@@ -827,23 +829,28 @@ mod tests {
     fn roundtrip_authpubkey() {
         init_test_log();
         // with None sig
-        let s = sign::tests::make_ed25519_signkey();
+        let k = SignKey::generate(KeyType::Ed25519).unwrap();
         let p = UserauthRequest {
             username: "matt".into(),
             service: "conn".into(),
-            method: s.pubkey().try_into().unwrap(),
+            method: k.pubkey().try_into().unwrap(),
         }.into();
         test_roundtrip(&p);
 
-        // again with a near-genuine sig
-        let sig = Signature::Ed25519(Ed25519Sig {
-            sig: BinString("something".as_bytes()),
-        });
+        // again with a sig
+        let owned_sig = k.sign(&"hello", None).unwrap();
+        let sig: Signature = (&owned_sig).into();
+        let sig_algo = sig.algorithm_name().unwrap();
         let sig = Some(Blob(sig));
+        let method = AuthMethod::PubKey(MethodPubKey {
+            sig_algo,
+            pubkey: Blob(k.pubkey()),
+            sig,
+        });
         let p = UserauthRequest {
             username: "matt".into(),
             service: "conn",
-            method: s.pubkey().try_into().unwrap(),
+            method,
         }.into();
         test_roundtrip(&p);
     }
diff --git a/src/sign.rs b/src/sign.rs
index d0d4776..3ddef7d 100644
--- a/src/sign.rs
+++ b/src/sign.rs
@@ -106,7 +106,7 @@ pub(crate) enum OwnedSig {
     // Signature doesn't let us borrow the inner bytes,
     // so we just store raw bytes here.
     Ed25519([u8; 64]),
-    RSA256, // TODO
+    _RSA256, // TODO
 }
 
 impl From<salty::Signature> for OwnedSig {
@@ -116,6 +116,12 @@ impl From<salty::Signature> for OwnedSig {
 
 }
 
+/// Signing key types.
+#[derive(Debug, Clone, Copy)]
+pub enum KeyType {
+    Ed25519,
+}
+
 /// A SSH signing key. This may hold the private part locally
 /// or could potentially send the signing requests to a SSH agent
 /// or other entitiy.
@@ -124,6 +130,16 @@ pub enum SignKey {
 }
 
 impl SignKey {
+    pub fn generate(ty: KeyType) -> Result<Self> {
+        match ty {
+            KeyType::Ed25519 => {
+                let mut seed = [0u8; 32];
+                random::fill_random(seed.as_mut_slice())?;
+                Ok(Self::Ed25519((&seed).into()))
+            },
+        }
+    }
+
     pub fn pubkey(&self) -> PubKey {
         match self {
             SignKey::Ed25519(k) => {PubKey::Ed25519(Ed25519PubKey
@@ -151,12 +167,11 @@ impl SignKey {
     pub(crate) fn sign(&self, msg: &impl SSHEncode, parse_ctx: Option<&ParseContext>) -> Result<OwnedSig> {
         match self {
             SignKey::Ed25519(k) => {
-                todo!("get sign_parts upstream");
-                // k.sign_parts(|h| {
-                //     sshwire::hash_ser(h, msg, parse_ctx).map_err(|_| salty::Error::ContextTooLong)
-                // })
-                // .trap()
-                // .map(|s| s.into())
+                k.sign_parts(|h| {
+                    sshwire::hash_ser(h, msg, parse_ctx).map_err(|_| salty::Error::ContextTooLong)
+                })
+                .trap()
+                .map(|s| s.into())
             }
         }
     }
@@ -188,11 +203,5 @@ pub(crate) mod tests {
     use sign::*;
     use sunsetlog::init_test_log;
 
-    pub(crate) fn make_ed25519_signkey() -> SignKey {
-        let mut s = [0u8; 32];
-        random::fill_random(s.as_mut_slice()).unwrap();
-        let ed = salty::Keypair::from(&s);
-        sign::SignKey::Ed25519(ed)
-    }
-
+    // TODO: tests for sign()/verify() and invalid signatures
 }
diff --git a/src/sshwire.rs b/src/sshwire.rs
index 3daac03..1c91946 100644
--- a/src/sshwire.rs
+++ b/src/sshwire.rs
@@ -635,7 +635,7 @@ pub(crate) mod tests {
                 key: BinString(&[0x11, 0x22, 0x33]),
             })),
         }).into();
-        let s = sign::tests::make_ed25519_signkey();
+        let s = SignKey::generate(KeyType::Ed25519).unwrap();
         ctx.cli_auth_type = Some(auth::AuthType::PubKey);
         test_roundtrip_context(&p, &ctx);
     }
-- 
GitLab