Skip to content
Closed
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 .justfile
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ fix:
# try to fix rustc issues
cargo fix --allow-staged
# try to fix clippy issues
cargo clippy --fix --allow-staged
cargo clippy --fix --allow-staged --allow-dirty

# fmt must be last as clippy's changes may break formatting
cargo +nightly fmt --all
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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])),
pubkey: KeyData::Ed25519(ssh_key::public::Ed25519PublicKey([0; 32])).into(),
comment: self.comment.clone(),
}])
}
Expand Down
14 changes: 7 additions & 7 deletions examples/key-storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ use ssh_agent_lib::agent::{listen, Session};
use ssh_agent_lib::error::AgentError;
use ssh_agent_lib::proto::extension::{QueryResponse, RestrictDestination, SessionBind};
use ssh_agent_lib::proto::{
message, signature, AddIdentity, AddIdentityConstrained, AddSmartcardKeyConstrained,
Credential, Extension, KeyConstraint, RemoveIdentity, SignRequest, SmartcardKey,
message, signature, AddIdentity, AddIdentityConstrained, AddSmartcardKeyConstrained, Extension,
KeyConstraint, PrivateCredential, RemoveIdentity, SignRequest, SmartcardKey,
};
use ssh_key::{
private::{KeypairData, PrivateKey},
Expand Down 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.clone().into();
let pubkey: PublicKey = sign_request.pubkey.key_data().clone().into();

if let Some(identity) = self.identity_from_pubkey(&pubkey) {
match identity.privkey.key_data() {
Expand Down Expand Up @@ -113,15 +113,15 @@ 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(),
pubkey: identity.pubkey.key_data().clone().into(),
comment: identity.comment.clone(),
})
}
Ok(identities)
}

async fn add_identity(&mut self, identity: AddIdentity) -> Result<(), AgentError> {
if let Credential::Key { privkey, comment } = identity.credential {
if let PrivateCredential::Key { privkey, comment } = identity.credential {
let privkey = PrivateKey::try_from(privkey).map_err(AgentError::other)?;
self.identity_add(Identity {
pubkey: PublicKey::from(&privkey),
Expand Down Expand Up @@ -152,7 +152,7 @@ impl Session for KeyStorage {
info!("Destination constraint: {destination:?}");
}

if let Credential::Key { privkey, comment } = identity.credential.clone() {
if let PrivateCredential::Key { privkey, comment } = identity.credential.clone() {
let privkey = PrivateKey::try_from(privkey).map_err(AgentError::other)?;
self.identity_add(Identity {
pubkey: PublicKey::from(&privkey),
Expand All @@ -166,7 +166,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.pubkey.key_data().clone().into();
self.identity_remove(&pubkey)?;
Ok(())
}
Expand Down
8 changes: 5 additions & 3 deletions examples/openpgp-card-agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ impl CardSession {
if let AlgorithmAttributes::Ecc(ecc) = e.algo() {
if ecc.ecc_type() == EccType::EdDSA {
let pubkey = KeyData::Ed25519(Ed25519PublicKey(e.data().try_into()?));
if pubkey == request.pubkey {
if pubkey == *request.pubkey.key_data() {
let pin = self.pwds.get(&ident).await;
return if let Some(pin) = pin {
let str = pin.expose_secret().as_bytes().to_vec();
Expand Down Expand Up @@ -173,7 +173,8 @@ impl Session for CardSession {
return Ok::<_, Box<dyn std::error::Error>>(Some(Identity {
pubkey: KeyData::Ed25519(Ed25519PublicKey(
e.data().try_into()?,
)),
))
.into(),
comment: ident,
}));
}
Expand Down Expand Up @@ -231,7 +232,8 @@ impl Session for CardSession {
return Ok::<_, Box<dyn std::error::Error>>(Some(Identity {
pubkey: KeyData::Ed25519(Ed25519PublicKey(
e.data().try_into()?,
)),
))
.into(),
comment: ident,
}));
}
Expand Down
17 changes: 12 additions & 5 deletions examples/pgp-wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ use service_binding::Binding;
use ssh_agent_lib::{
agent::Session,
client::connect,
proto::{Extension, SignRequest},
proto::{Extension, PublicCredential, SignRequest},
};
use ssh_key::public::KeyData;
use tokio::runtime::Runtime;
Expand Down Expand Up @@ -295,7 +295,7 @@ impl SecretKeyTrait for WrappedKey {
.block_on(async {
let mut client = self.client.lock().await;
let result = client.sign(SignRequest {
pubkey: self.pubkey.clone(),
pubkey: self.pubkey.clone().into(),
data: data.to_vec(),
flags: 0,
});
Expand Down Expand Up @@ -372,7 +372,8 @@ fn main() -> testresult::TestResult {
let mut keyflags = KeyFlags::default();
keyflags.set_encrypt_comms(true);
keyflags.set_encrypt_storage(true);
let pk = ssh_to_pgp(decryption_id.pubkey.clone(), KeyRole::Decryption);
let pubkey = decryption_id.pubkey.key_data();
let pk = ssh_to_pgp(pubkey.clone(), KeyRole::Decryption);
vec![pgp::PublicSubkey::new(
pgp::packet::PublicSubkey::new(
pk.packet_version(),
Expand All @@ -388,6 +389,9 @@ fn main() -> testresult::TestResult {
vec![]
};

let PublicCredential::Key(pubkey) = pubkey else {
panic!("Only pubkeys are supported.");
};
let signer = WrappedKey::new(pubkey.clone(), client, KeyRole::Signing);
let mut keyflags = KeyFlags::default();
keyflags.set_sign(true);
Expand All @@ -411,6 +415,9 @@ fn main() -> testresult::TestResult {
signed_pk.to_writer(&mut std::io::stdout())?;
}
Args::Sign => {
let PublicCredential::Key(pubkey) = pubkey else {
panic!("Only pubkeys are supported.");
};
let signer = WrappedKey::new(pubkey.clone(), client, KeyRole::Signing);
let signature = SignatureConfig::new_v4(
SignatureVersion::V4,
Expand Down Expand Up @@ -445,8 +452,8 @@ fn main() -> testresult::TestResult {
pgp::packet::write_packet(&mut std::io::stdout(), &signature)?;
}
Args::Decrypt => {
let decryptor =
WrappedKey::new(decrypt_ids[0].pubkey.clone(), client, KeyRole::Decryption);
let pubkey = decrypt_ids[0].pubkey.key_data();
let decryptor = WrappedKey::new(pubkey.clone(), client, KeyRole::Decryption);
let message = Message::from_bytes(std::io::stdin())?;

let Message::Encrypted { esk, edata } = message else {
Expand Down
4 changes: 3 additions & 1 deletion src/proto/message.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Agent protocol message structures.

mod add_remove;
mod credential;
mod extension;
mod identity;
mod request;
Expand All @@ -9,7 +10,8 @@ mod sign;
mod unparsed;

pub use self::{
add_remove::*, extension::*, identity::*, request::*, response::*, sign::*, unparsed::*,
add_remove::*, credential::*, extension::*, identity::*, request::*, response::*, sign::*,
unparsed::*,
};
#[doc(hidden)]
/// For compatibility with pre-0.5.0 type alias in this module
Expand Down
16 changes: 7 additions & 9 deletions src/proto/message/add_remove.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
//! Add a key to an agent with or without constraints and supporting data types.

mod constrained;
mod credential;

use super::PrivateCredential;
use super::PublicCredential;
use crate::proto::{Error, Result};
pub use constrained::*;
pub use credential::*;
use secrecy::ExposeSecret as _;
use secrecy::SecretString;
use ssh_encoding::{self, CheckedSum, Decode, Encode, Reader, Writer};
use ssh_key::public::KeyData;

use crate::proto::{Error, Result};

/// Add a key to an agent.
///
Expand All @@ -20,14 +18,14 @@ use crate::proto::{Error, Result};
#[derive(Clone, PartialEq, Debug)]
pub struct AddIdentity {
/// A credential (private & public key, or private key / certificate) to add to the agent
pub credential: Credential,
pub credential: PrivateCredential,
}

impl Decode for AddIdentity {
type Error = Error;

fn decode(reader: &mut impl Reader) -> Result<Self> {
let credential = Credential::decode(reader)?;
let credential = PrivateCredential::decode(reader)?;

Ok(Self { credential })
}
Expand Down Expand Up @@ -102,14 +100,14 @@ 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 pubkey: PublicCredential,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Could you additionally rename the field to... say... credential? (to mirror what AddIdentity has).

}

impl Decode for RemoveIdentity {
type Error = Error;

fn decode(reader: &mut impl Reader) -> Result<Self> {
let pubkey = reader.read_prefixed(KeyData::decode)?;
let pubkey = reader.read_prefixed(PublicCredential::decode)?;

Ok(Self { pubkey })
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
//! A container for a public / private key pair, or a certificate / private key.

use core::str::FromStr;
use std::str::FromStr as _;

use ssh_encoding::{self, CheckedSum, Decode, Encode, Reader, Writer};
use ssh_key::public::KeyData;
use ssh_key::{certificate::Certificate, private::KeypairData, Algorithm};

use crate::proto::{Error, PrivateKeyData, Result};
Expand All @@ -16,7 +17,8 @@ use crate::proto::{Error, PrivateKeyData, Result};
/// This structure covers both types of identities a user may
/// send to an agent as part of a [`Request::AddIdentity`](crate::proto::Request::AddIdentity) message.
#[derive(Clone, PartialEq, Debug)]
pub enum Credential {
#[allow(clippy::large_enum_variant)]
pub enum PrivateCredential {
/// A public/private key pair
Key {
/// Public/private key pair data
Expand All @@ -42,7 +44,7 @@ pub enum Credential {
},
}

impl Decode for Credential {
impl Decode for PrivateCredential {
type Error = Error;

fn decode(reader: &mut impl Reader) -> Result<Self> {
Expand All @@ -57,7 +59,7 @@ impl Decode for Credential {
let privkey = PrivateKeyData::decode_as(reader, algorithm.clone())?;
let comment = String::decode(reader)?;

Ok(Credential::Cert {
Ok(PrivateCredential::Cert {
algorithm,
certificate,
privkey,
Expand All @@ -67,12 +69,12 @@ impl Decode for Credential {
let algorithm = Algorithm::from_str(&alg).map_err(ssh_encoding::Error::from)?;
let privkey = KeypairData::decode_as(reader, algorithm)?;
let comment = String::decode(reader)?;
Ok(Credential::Key { privkey, comment })
Ok(PrivateCredential::Key { privkey, comment })
}
}
}

impl Encode for Credential {
impl Encode for PrivateCredential {
fn encoded_len(&self) -> ssh_encoding::Result<usize> {
match self {
Self::Key { privkey, comment } => {
Expand Down Expand Up @@ -113,3 +115,83 @@ impl Encode for Credential {
}
}
}

#[derive(Debug, PartialEq, Eq, Clone)]
#[allow(clippy::large_enum_variant)]
/// Represents a public credential.
pub enum PublicCredential {
/// Plain public key.
Key(KeyData),
/// Signed public key.
Cert(Certificate),
}

impl PublicCredential {
/// Returns a reference to the [KeyData].
pub fn key_data(&self) -> &KeyData {
match self {
Self::Key(key) => key,
Self::Cert(cert) => cert.public_key(),
}
}
}

impl Decode for PublicCredential {
type Error = Error;

fn decode(reader: &mut impl Reader) -> core::result::Result<Self, Self::Error> {
// Read remaining bytes and prepend the algorithm string so the
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

To be perfectly honest with you it looks extremely ugly as it is but from my understanding it will get completely rewritten once ssh-key 0.7.0 lands (source) and since we have tests I'm not that worried about temporarily having this here.

Could you add a note though? Something like // 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.

// full blob can be passed to Certificate::decode or
// KeyData::decode, both of which expect to read the algorithm
// string themselves.
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);

let mut slice: &[u8] = &buf;

if Algorithm::new_certificate(&alg).is_ok() {
let cert = Certificate::decode(&mut slice)?;
Ok(Self::Cert(cert))
} else {
let key = KeyData::decode(&mut slice)?;
Ok(Self::Key(key))
}
}
}

impl Encode for PublicCredential {
fn encoded_len(&self) -> std::result::Result<usize, ssh_encoding::Error> {
match self {
Self::Key(pubkey) => pubkey.encoded_len(),
Self::Cert(certificate) => certificate.encoded_len(),
}
}

fn encode(
&self,
writer: &mut impl ssh_encoding::Writer,
) -> std::result::Result<(), ssh_encoding::Error> {
match self {
Self::Key(pubkey) => pubkey.encode(writer),
Self::Cert(certificate) => certificate.encode(writer),
}
}
}

impl From<KeyData> for PublicCredential {
fn from(value: KeyData) -> Self {
Self::Key(value)
}
}

impl From<Certificate> for PublicCredential {
fn from(value: Certificate) -> Self {
Self::Cert(value)
}
}
Loading
Loading