Merge pull request #101 from wiktor-k/wiktor-k/fix-support-for-certs

Add support for certificates in requests and responses

Author: wiktor-k

.justfile

@@ -1,7 +1,7 @@
 #!/usr/bin/env -S just --working-directory . --justfile
 # Load project-specific properties from the `.env` file
 
-set dotenv-load := true
+set dotenv-load
 
 # Since this is a first recipe it's being run by default.
 # Faster checks need to be executed first for better UX.  For example

examples/agent-socket-info.rs

@@ -27,7 +27,7 @@ impl Session for AgentSocketInfo {
     async fn request_identities(&mut self) -> Result<Vec<Identity>, AgentError> {
         Ok(vec![Identity {
             // this is just a dummy key, the comment is important
-            pubkey: KeyData::Ed25519(ssh_key::public::Ed25519PublicKey([0; 32])).into(),
+            credential: KeyData::Ed25519(ssh_key::public::Ed25519PublicKey([0; 32])).into(),
             comment: self.comment.clone(),
         }])
     }

examples/key-storage.rs

@@ -74,7 +74,7 @@ impl KeyStorage {
 #[crate::async_trait]
 impl Session for KeyStorage {
     async fn sign(&mut self, sign_request: SignRequest) -> Result<Signature, AgentError> {
-        let pubkey: PublicKey = sign_request.pubkey.key_data().clone().into();
+        let pubkey: PublicKey = sign_request.credential.key_data().clone().into();
 
         if let Some(identity) = self.identity_from_pubkey(&pubkey) {
             match identity.privkey.key_data() {
@@ -122,7 +122,7 @@ impl Session for KeyStorage {
         let mut identities = vec![];
         for identity in self.identities.lock().unwrap().iter() {
             identities.push(message::Identity {
-                pubkey: identity.pubkey.key_data().clone().into(),
+                credential: identity.pubkey.key_data().clone().into(),
                 comment: identity.comment.clone(),
             })
         }
@@ -175,7 +175,7 @@ impl Session for KeyStorage {
     }
 
     async fn remove_identity(&mut self, identity: RemoveIdentity) -> Result<(), AgentError> {
-        let pubkey: PublicKey = identity.pubkey.into();
+        let pubkey: PublicKey = identity.credential.key_data().clone().into();
         self.identity_remove(&pubkey)?;
         Ok(())
     }

examples/random-key.rs

@@ -41,7 +41,7 @@ impl RandomKey {
 impl Session for RandomKey {
     async fn sign(&mut self, sign_request: SignRequest) -> Result<Signature, AgentError> {
         let private_key = self.private_key.lock().unwrap();
-        let PublicCredential::Key(pubkey) = sign_request.pubkey else {
+        let PublicCredential::Key(pubkey) = sign_request.credential else {
             return Err(std::io::Error::other("Key not found").into());
         };
         if PublicKey::from(private_key.deref()).key_data() != &pubkey {
@@ -88,7 +88,7 @@ impl Session for RandomKey {
     async fn request_identities(&mut self) -> Result<Vec<Identity>, AgentError> {
         let identity = self.private_key.lock().unwrap();
         Ok(vec![Identity {
-            pubkey: PublicCredential::Key(PublicKey::from(identity.deref()).into()),
+            credential: PublicCredential::Key(PublicKey::from(identity.deref()).into()),
             comment: identity.comment().into(),
         }])
     }

src/proto/message/add_remove.rs

@@ -6,9 +6,9 @@ pub use constrained::*;
 use secrecy::ExposeSecret as _;
 use secrecy::SecretString;
 use ssh_encoding::{self, CheckedSum, Decode, Encode, Reader, Writer};
-use ssh_key::public::KeyData;
 
 use super::PrivateCredential;
+use crate::proto::PublicCredential;
 use crate::proto::{Error, Result};
 
 /// Add a key to an agent.
@@ -101,25 +101,25 @@ impl PartialEq for SmartcardKey {
 #[derive(Clone, PartialEq, Debug)]
 pub struct RemoveIdentity {
     /// The public key portion of the [`Identity`](super::Identity) to be removed
-    pub pubkey: KeyData,
+    pub credential: PublicCredential,
 }
 
 impl Decode for RemoveIdentity {
     type Error = Error;
 
     fn decode(reader: &mut impl Reader) -> Result<Self> {
-        let pubkey = reader.read_prefixed(KeyData::decode)?;
+        let credential = reader.read_prefixed(PublicCredential::decode)?;
 
-        Ok(Self { pubkey })
+        Ok(Self { credential })
     }
 }
 
 impl Encode for RemoveIdentity {
     fn encoded_len(&self) -> ssh_encoding::Result<usize> {
-        self.pubkey.encoded_len_prefixed()
+        self.credential.encoded_len_prefixed()
     }
 
     fn encode(&self, writer: &mut impl Writer) -> ssh_encoding::Result<()> {
-        self.pubkey.encode_prefixed(writer)
+        self.credential.encode_prefixed(writer)
     }
 }

src/proto/message/credential.rs

@@ -140,8 +140,23 @@ impl Decode for PublicCredential {
     type Error = Error;
 
     fn decode(reader: &mut impl Reader) -> core::result::Result<Self, Self::Error> {
-        // TODO: implement parsing certificates
-        Ok(Self::Key(KeyData::decode(reader)?))
+        // FIXME: This needs to be rewritten using Certificate::decode_as when ssh-key 0.7.0 hits stable, see: https://github.com/wiktor-k/ssh-agent-lib/pull/85#issuecomment-3751946208
+        let alg = String::decode(reader)?;
+
+        let remaining_len = reader.remaining_len();
+        let mut buf = Vec::with_capacity(4 + alg.len() + remaining_len);
+        alg.encode(&mut buf)?;
+        let mut tail = vec![0u8; remaining_len];
+        reader.read(&mut tail)?;
+        buf.extend_from_slice(&tail);
+
+        if Algorithm::new_certificate(&alg).is_ok() {
+            let cert = Certificate::decode(&mut &buf[..])?;
+            Ok(Self::Cert(Box::new(cert)))
+        } else {
+            let key = KeyData::decode(&mut &buf[..])?;
+            Ok(Self::Key(key))
+        }
     }
 }
 

src/proto/message/identity.rs

@@ -13,7 +13,7 @@ use crate::proto::{Error, Result};
 #[derive(Clone, PartialEq, Debug)]
 pub struct Identity {
     /// A standard public-key encoding of an underlying key.
-    pub pubkey: PublicCredential,
+    pub credential: PublicCredential,
 
     /// A human-readable comment
     pub comment: String,
@@ -36,24 +36,27 @@ impl Decode for Identity {
     type Error = Error;
 
     fn decode(reader: &mut impl Reader) -> Result<Self> {
-        let pubkey = reader.read_prefixed(PublicCredential::decode)?;
+        let credential = reader.read_prefixed(PublicCredential::decode)?;
         let comment = String::decode(reader)?;
 
-        Ok(Self { pubkey, comment })
+        Ok(Self {
+            credential,
+            comment,
+        })
     }
 }
 
 impl Encode for Identity {
     fn encoded_len(&self) -> ssh_encoding::Result<usize> {
         [
-            self.pubkey.encoded_len_prefixed()?,
+            self.credential.encoded_len_prefixed()?,
             self.comment.encoded_len()?,
         ]
         .checked_sum()
     }
 
     fn encode(&self, writer: &mut impl Writer) -> ssh_encoding::Result<()> {
-        self.pubkey.encode_prefixed(writer)?;
+        self.credential.encode_prefixed(writer)?;
         self.comment.encode(writer)?;
 
         Ok(())

src/proto/message/request.rs

@@ -148,3 +148,71 @@ impl Encode for Request {
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use ssh_key::Algorithm;
+    use testresult::TestResult;
+
+    use super::*;
+    use crate::proto::{PublicCredential, Response};
+
+    #[test]
+    fn sign_with_cert() -> TestResult {
+        let req =
+            Request::decode(&mut &std::fs::read("tests/messages/req-sign-with-cert.bin")?[..])?;
+        let Request::SignRequest(req) = req else {
+            panic!("expected SignRequest");
+        };
+        let PublicCredential::Cert(cert) = req.credential else {
+            panic!("expected certificate");
+        };
+        assert_eq!(cert.algorithm(), Algorithm::Rsa { hash: None });
+        Ok(())
+    }
+
+    #[test]
+    fn sign_with_pubkey() -> TestResult {
+        let req = Request::decode(&mut &std::fs::read("tests/messages/req-sign-request.bin")?[..])?;
+        let Request::SignRequest(req) = req else {
+            panic!("expected SignRequest");
+        };
+        let PublicCredential::Key(cert) = req.credential else {
+            panic!("expected key");
+        };
+        assert_eq!(cert.algorithm(), Algorithm::Rsa { hash: None });
+        Ok(())
+    }
+
+    #[test]
+    fn resp_identities_with_cert() -> TestResult {
+        let resp = Response::decode(
+            &mut &std::fs::read("tests/messages/resp-identities-with-cert.bin")?[..],
+        )?;
+        let Response::IdentitiesAnswer(resp) = resp else {
+            panic!("expected IdentitiesAnswer");
+        };
+        assert_eq!(resp.len(), 4);
+        let PublicCredential::Cert(cert) = &resp[3].credential else {
+            panic!("expected certificate");
+        };
+        assert_eq!(cert.algorithm(), Algorithm::Rsa { hash: None });
+        Ok(())
+    }
+
+    #[test]
+    fn resp_identities_with_key() -> TestResult {
+        let resp = Response::decode(
+            &mut &std::fs::read("tests/messages/resp-identities-answer.bin")?[..],
+        )?;
+        let Response::IdentitiesAnswer(resp) = resp else {
+            panic!("expected IdentitiesAnswer");
+        };
+        assert_eq!(resp.len(), 1);
+        let PublicCredential::Key(key) = &resp[0].credential else {
+            panic!("expected key");
+        };
+        assert_eq!(key.algorithm(), Algorithm::Rsa { hash: None });
+        Ok(())
+    }
+}

src/proto/message/sign.rs

@@ -13,7 +13,7 @@ use crate::proto::{Error, Result};
 #[derive(Clone, PartialEq, Debug)]
 pub struct SignRequest {
     /// The public key portion of the [`Identity`](super::Identity) in the agent to sign the data with
-    pub pubkey: PublicCredential,
+    pub credential: PublicCredential,
 
     /// Binary data to be signed
     pub data: Vec<u8>,
@@ -32,7 +32,7 @@ impl Decode for SignRequest {
         let flags = u32::decode(reader)?;
 
         Ok(Self {
-            pubkey,
+            credential: pubkey,
             data,
             flags,
         })
@@ -42,15 +42,15 @@ impl Decode for SignRequest {
 impl Encode for SignRequest {
     fn encoded_len(&self) -> ssh_encoding::Result<usize> {
         [
-            self.pubkey.encoded_len_prefixed()?,
+            self.credential.encoded_len_prefixed()?,
             self.data.encoded_len()?,
             self.flags.encoded_len()?,
         ]
         .checked_sum()
     }
 
     fn encode(&self, writer: &mut impl Writer) -> ssh_encoding::Result<()> {
-        self.pubkey.encode_prefixed(writer)?;
+        self.credential.encode_prefixed(writer)?;
         self.data.encode(writer)?;
         self.flags.encode(writer)?;