From 505eba19cbb611c9a35ff08e49962a318c2b17cc Mon Sep 17 00:00:00 2001 From: Wiktor Kwapisiewicz Date: Thu, 19 Mar 2026 12:00:42 +0100 Subject: [PATCH 1/4] Implement decoding certificates in `PublicCredential` Signed-off-by: Wiktor Kwapisiewicz See: https://github.com/wiktor-k/ssh-agent-lib/pull/98 Co-authored-by: Sasha F --- src/proto/message/credential.rs | 19 ++++- src/proto/message/request.rs | 68 ++++++++++++++++++ ...ert.bin.ignored => req-sign-with-cert.bin} | Bin ....ignored => resp-identities-with-cert.bin} | Bin 4 files changed, 85 insertions(+), 2 deletions(-) rename tests/messages/{req-sign-with-cert.bin.ignored => req-sign-with-cert.bin} (100%) rename tests/messages/{resp-identities-with-cert.bin.ignored => resp-identities-with-cert.bin} (100%) diff --git a/src/proto/message/credential.rs b/src/proto/message/credential.rs index d595992..0190d42 100644 --- a/src/proto/message/credential.rs +++ b/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 { - // 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)) + } } } diff --git a/src/proto/message/request.rs b/src/proto/message/request.rs index 0618b8b..56747f2 100644 --- a/src/proto/message/request.rs +++ b/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.pubkey 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.pubkey 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].pubkey 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].pubkey else { + panic!("expected key"); + }; + assert_eq!(key.algorithm(), Algorithm::Rsa { hash: None }); + Ok(()) + } +} diff --git a/tests/messages/req-sign-with-cert.bin.ignored b/tests/messages/req-sign-with-cert.bin similarity index 100% rename from tests/messages/req-sign-with-cert.bin.ignored rename to tests/messages/req-sign-with-cert.bin diff --git a/tests/messages/resp-identities-with-cert.bin.ignored b/tests/messages/resp-identities-with-cert.bin similarity index 100% rename from tests/messages/resp-identities-with-cert.bin.ignored rename to tests/messages/resp-identities-with-cert.bin From 79fcda3515aa099c5e7634d4c1af6b1142eb1e9d Mon Sep 17 00:00:00 2001 From: Wiktor Kwapisiewicz Date: Thu, 19 Mar 2026 12:01:53 +0100 Subject: [PATCH 2/4] Use `PublicCredential` in `RemoveIdentity` Signed-off-by: Wiktor Kwapisiewicz See: https://github.com/wiktor-k/ssh-agent-lib/pull/98 Co-authored-by: Sasha F --- examples/key-storage.rs | 2 +- src/proto/message/add_remove.rs | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/examples/key-storage.rs b/examples/key-storage.rs index e5b712e..e91d6c6 100644 --- a/examples/key-storage.rs +++ b/examples/key-storage.rs @@ -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(()) } diff --git a/src/proto/message/add_remove.rs b/src/proto/message/add_remove.rs index 3f843a5..22d78fa 100644 --- a/src/proto/message/add_remove.rs +++ b/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 { - 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 { - 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) } } From 1f8a19a4fc0a1ece4cbf0934c5a41f1c7d2f1445 Mon Sep 17 00:00:00 2001 From: Wiktor Kwapisiewicz Date: Thu, 19 Mar 2026 12:02:54 +0100 Subject: [PATCH 3/4] Rename `Identity::pubkey` to `credential` for clarity Signed-off-by: Wiktor Kwapisiewicz --- examples/agent-socket-info.rs | 2 +- examples/key-storage.rs | 2 +- examples/random-key.rs | 2 +- src/proto/message/identity.rs | 13 ++++++++----- src/proto/message/request.rs | 4 ++-- tests/roundtrip/expected/resp_parse_identities.rs | 2 +- 6 files changed, 14 insertions(+), 11 deletions(-) diff --git a/examples/agent-socket-info.rs b/examples/agent-socket-info.rs index 1dc075e..6cce831 100644 --- a/examples/agent-socket-info.rs +++ b/examples/agent-socket-info.rs @@ -27,7 +27,7 @@ impl Session for AgentSocketInfo { async fn request_identities(&mut self) -> Result, 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(), }]) } diff --git a/examples/key-storage.rs b/examples/key-storage.rs index e91d6c6..e6ee8d4 100644 --- a/examples/key-storage.rs +++ b/examples/key-storage.rs @@ -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(), }) } diff --git a/examples/random-key.rs b/examples/random-key.rs index ba6d84e..f3973ba 100644 --- a/examples/random-key.rs +++ b/examples/random-key.rs @@ -88,7 +88,7 @@ impl Session for RandomKey { async fn request_identities(&mut self) -> Result, 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(), }]) } diff --git a/src/proto/message/identity.rs b/src/proto/message/identity.rs index 24080a4..e3da708 100644 --- a/src/proto/message/identity.rs +++ b/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 { - 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 { [ - 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(()) diff --git a/src/proto/message/request.rs b/src/proto/message/request.rs index 56747f2..161d0e6 100644 --- a/src/proto/message/request.rs +++ b/src/proto/message/request.rs @@ -193,7 +193,7 @@ mod tests { panic!("expected IdentitiesAnswer"); }; assert_eq!(resp.len(), 4); - let PublicCredential::Cert(cert) = &resp[3].pubkey else { + let PublicCredential::Cert(cert) = &resp[3].credential else { panic!("expected certificate"); }; assert_eq!(cert.algorithm(), Algorithm::Rsa { hash: None }); @@ -209,7 +209,7 @@ mod tests { panic!("expected IdentitiesAnswer"); }; assert_eq!(resp.len(), 1); - let PublicCredential::Key(key) = &resp[0].pubkey else { + let PublicCredential::Key(key) = &resp[0].credential else { panic!("expected key"); }; assert_eq!(key.algorithm(), Algorithm::Rsa { hash: None }); diff --git a/tests/roundtrip/expected/resp_parse_identities.rs b/tests/roundtrip/expected/resp_parse_identities.rs index ec22774..f96d020 100644 --- a/tests/roundtrip/expected/resp_parse_identities.rs +++ b/tests/roundtrip/expected/resp_parse_identities.rs @@ -5,7 +5,7 @@ use super::fixtures; pub fn expected() -> Response { Response::IdentitiesAnswer(vec![Identity { - pubkey: KeyData::Ecdsa(fixtures::demo_key().into()).into(), + credential: KeyData::Ecdsa(fixtures::demo_key().into()).into(), comment: "baloo@angela".to_string(), }]) } From 1f86f25bd53c236344d60df2574f0a343e9b9136 Mon Sep 17 00:00:00 2001 From: Wiktor Kwapisiewicz Date: Thu, 19 Mar 2026 12:04:16 +0100 Subject: [PATCH 4/4] Rename `SignRequest::pubkey` to `credential` for clarity Signed-off-by: Wiktor Kwapisiewicz --- examples/key-storage.rs | 2 +- examples/random-key.rs | 2 +- src/proto/message/request.rs | 4 ++-- src/proto/message/sign.rs | 8 ++++---- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/examples/key-storage.rs b/examples/key-storage.rs index e6ee8d4..8e2901c 100644 --- a/examples/key-storage.rs +++ b/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 { - 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() { diff --git a/examples/random-key.rs b/examples/random-key.rs index f3973ba..a9ae911 100644 --- a/examples/random-key.rs +++ b/examples/random-key.rs @@ -41,7 +41,7 @@ impl RandomKey { impl Session for RandomKey { async fn sign(&mut self, sign_request: SignRequest) -> Result { 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 { diff --git a/src/proto/message/request.rs b/src/proto/message/request.rs index 161d0e6..16511b3 100644 --- a/src/proto/message/request.rs +++ b/src/proto/message/request.rs @@ -164,7 +164,7 @@ mod tests { let Request::SignRequest(req) = req else { panic!("expected SignRequest"); }; - let PublicCredential::Cert(cert) = req.pubkey else { + let PublicCredential::Cert(cert) = req.credential else { panic!("expected certificate"); }; assert_eq!(cert.algorithm(), Algorithm::Rsa { hash: None }); @@ -177,7 +177,7 @@ mod tests { let Request::SignRequest(req) = req else { panic!("expected SignRequest"); }; - let PublicCredential::Key(cert) = req.pubkey else { + let PublicCredential::Key(cert) = req.credential else { panic!("expected key"); }; assert_eq!(cert.algorithm(), Algorithm::Rsa { hash: None }); diff --git a/src/proto/message/sign.rs b/src/proto/message/sign.rs index d243753..a515f3f 100644 --- a/src/proto/message/sign.rs +++ b/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, @@ -32,7 +32,7 @@ impl Decode for SignRequest { let flags = u32::decode(reader)?; Ok(Self { - pubkey, + credential: pubkey, data, flags, }) @@ -42,7 +42,7 @@ impl Decode for SignRequest { impl Encode for SignRequest { fn encoded_len(&self) -> ssh_encoding::Result { [ - self.pubkey.encoded_len_prefixed()?, + self.credential.encoded_len_prefixed()?, self.data.encoded_len()?, self.flags.encoded_len()?, ] @@ -50,7 +50,7 @@ impl Encode for SignRequest { } 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)?;