Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/agent-socket-info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}])
}
Expand Down
6 changes: 3 additions & 3 deletions examples/key-storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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(),
})
}
Expand Down Expand Up @@ -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(())
}
Expand Down
4 changes: 2 additions & 2 deletions examples/random-key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(),
}])
}
Expand Down
12 changes: 6 additions & 6 deletions src/proto/message/add_remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
}
}
19 changes: 17 additions & 2 deletions src/proto/message/credential.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Comment on lines +143 to +159
Copy link
Owner Author

@wiktor-k wiktor-k Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@baloo I have a feeling like this should belong somewhere in ssh-key itself. The detection of whether we have a cert or not and branching looks super ugly to me. It took me a while to browse Algorithm::new_certificate code to check if it will return Err on non-certificates rather than returning Other or something like this.

What do you think about it? 🤔

Edit: the problem here is that even if we first decode Algorithm it doesn't seem to have any indication if it's a certificate algorithm or not...

}
}

Expand Down
13 changes: 8 additions & 5 deletions src/proto/message/identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(())
Expand Down
68 changes: 68 additions & 0 deletions src/proto/message/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
}
8 changes: 4 additions & 4 deletions src/proto/message/sign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand All @@ -32,7 +32,7 @@ impl Decode for SignRequest {
let flags = u32::decode(reader)?;

Ok(Self {
pubkey,
credential: pubkey,
data,
flags,
})
Expand All @@ -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)?;

Expand Down
2 changes: 1 addition & 1 deletion tests/roundtrip/expected/resp_parse_identities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}])
}
Loading