keyfork-shard: slightly improved error handling

keyfork-shard: parse cardholder name when using smartcards
cargo fmt
2023-11-05 13:57:22 -06:00 · 2023-11-05 01:36:12 -05:00 · 2023-11-05 01:29:10 -05:00
19 changed files with 311 additions and 174 deletions
--- a/Cargo.lock
+++ b/Cargo.lock
@ -1073,6 +1073,7 @@ version = "0.1.0"
 dependencies = [
 "anyhow",
 "bincode",
+ "card-backend",
 "card-backend-pcsc",
 "keyfork-derive-openpgp",
 "keyfork-pinentry",
--- a/keyfork-derive-openpgp/src/main.rs
+++ b/keyfork-derive-openpgp/src/main.rs
@ -1,4 +1,4 @@
-use std::{env, str::FromStr, process::ExitCode};
+use std::{env, process::ExitCode, str::FromStr};

 use keyfork_derive_util::{
    request::{DerivationAlgorithm, DerivationRequest},
--- a/keyfork-derive-path-data/src/lib.rs
+++ b/keyfork-derive-path-data/src/lib.rs
@ -1,4 +1,4 @@
-use keyfork_derive_util::{DerivationPath, DerivationIndex};
+use keyfork_derive_util::{DerivationIndex, DerivationPath};

 pub static OPENPGP: DerivationIndex = DerivationIndex::new_unchecked(7366512, true);

--- a/keyfork-derive-util/src/extended_key/private_key.rs
+++ b/keyfork-derive-util/src/extended_key/private_key.rs
@ -1,4 +1,4 @@
-use crate::{DerivationIndex, DerivationPath, PrivateKey, PublicKey, ExtendedPublicKey};
+use crate::{DerivationIndex, DerivationPath, ExtendedPublicKey, PrivateKey, PublicKey};

 use hmac::{Hmac, Mac};
 use serde::{Deserialize, Serialize};
--- a/keyfork-derive-util/src/path.rs
+++ b/keyfork-derive-util/src/path.rs
@ -122,11 +122,14 @@ mod tests {
        Ok(())
    }

-
    #[test]
    fn add_vec() -> Result<(), Box<dyn std::error::Error>> {
        let path = DerivationPath::from_str("m")?;
-        let other_path = [DerivationIndex::new(72, true)?, DerivationIndex::new(47, false)?, DerivationIndex::new((i32::MAX) as u32, false)?];
+        let other_path = [
+            DerivationIndex::new(72, true)?,
+            DerivationIndex::new(47, false)?,
+            DerivationIndex::new((i32::MAX) as u32, false)?,
+        ];
        let path = path + &other_path[..];
        assert_eq!(path, DerivationPath::from_str("m/72'/47/2147483647")?);

--- a/keyfork-derive-util/src/public_key.rs
+++ b/keyfork-derive-util/src/public_key.rs
@ -35,7 +35,9 @@ pub trait PublicKey: Sized {
        let hash = Sha256::new().chain_update(self.to_bytes()).finalize();
        let hash = Ripemd160::new().chain_update(hash).finalize();
        // Note: Safety assured by type returned from Ripemd160
-        hash[..4].try_into().expect("Ripemd160 returned too little data")
+        hash[..4]
+            .try_into()
+            .expect("Ripemd160 returned too little data")
    }
 }

--- a/keyfork-derive-util/src/tests.rs
+++ b/keyfork-derive-util/src/tests.rs
@ -59,10 +59,7 @@ fn secp256k1() {
 fn ed25519() {
    use ed25519_dalek::SigningKey;

-    let tests = test_data()
-        .unwrap()
-        .remove(&"ed25519".to_string())
-        .unwrap();
+    let tests = test_data().unwrap().remove(&"ed25519".to_string()).unwrap();

    for per_seed in tests {
        let seed = &per_seed.seed;
@ -110,7 +107,8 @@ fn panics_with_unhardened_derivation() {

    let seed = hex!("000102030405060708090a0b0c0d0e0f");
    let xkey = ExtendedPrivateKey::<SigningKey>::new(seed).unwrap();
-    xkey.derive_path(&DerivationPath::from_str("m/0").unwrap()).unwrap();
+    xkey.derive_path(&DerivationPath::from_str("m/0").unwrap())
+        .unwrap();
 }

 #[cfg(feature = "ed25519")]
--- a/keyfork-mnemonic-util/src/lib.rs
+++ b/keyfork-mnemonic-util/src/lib.rs
@ -1,8 +1,8 @@
-use std::{collections::HashMap, str::FromStr, sync::Arc, error::Error, fmt::Display};
+use std::{collections::HashMap, error::Error, fmt::Display, str::FromStr, sync::Arc};

-use sha2::{Digest, Sha256, Sha512};
-use pbkdf2::pbkdf2;
 use hmac::Hmac;
+use pbkdf2::pbkdf2;
+use sha2::{Digest, Sha256, Sha512};

 /// The error type representing a failure to create a [`Mnemonic`]. These errors only occur during
 /// [`Mnemonic`] creation.
@ -30,7 +30,7 @@ impl Display for MnemonicGenerationError {
            }
            MnemonicGenerationError::InvalidPbkdf2Length => {
                f.write_str("Invalid length from PBKDF2")
-            },
+            }
        }
    }
 }
@ -207,7 +207,7 @@ impl Mnemonic {
                bits[index * 11 + bit] = (word & (1 << (10 - bit))) > 0;
            }
        }
-        
+
        // remove checksum bits
        bits.truncate(bits.len() - bits.len() % 32);

@ -222,7 +222,10 @@ impl Mnemonic {
            .collect()
    }

-    pub fn seed<'a>(&self, passphrase: impl Into<Option<&'a str>>) -> Result<Vec<u8>, MnemonicGenerationError> {
+    pub fn seed<'a>(
+        &self,
+        passphrase: impl Into<Option<&'a str>>,
+    ) -> Result<Vec<u8>, MnemonicGenerationError> {
        let passphrase = passphrase.into();

        let mut seed = [0u8; 64];
@ -293,8 +296,14 @@ mod tests {
        let their_mnemonic = bip39::Mnemonic::from_entropy(&entropy[..256 / 8]).unwrap();
        assert_eq!(my_mnemonic.to_string(), their_mnemonic.to_string());
        assert_eq!(my_mnemonic.seed(None).unwrap(), their_mnemonic.to_seed(""));
-        assert_eq!(my_mnemonic.seed("testing").unwrap(), their_mnemonic.to_seed("testing"));
-        assert_ne!(my_mnemonic.seed("test1").unwrap(), their_mnemonic.to_seed("test2"));
+        assert_eq!(
+            my_mnemonic.seed("testing").unwrap(),
+            their_mnemonic.to_seed("testing")
+        );
+        assert_ne!(
+            my_mnemonic.seed("test1").unwrap(),
+            their_mnemonic.to_seed("test2")
+        );
    }

    #[test]
--- a/keyfork-pinentry/src/lib.rs
+++ b/keyfork-pinentry/src/lib.rs
@ -390,7 +390,7 @@ impl<'a> MessageDialog<'a> {
            binary,
            title: None,
            ok: None,
-            timeout: None
+            timeout: None,
        }
    }

--- a/keyfork-shard/Cargo.toml
+++ b/keyfork-shard/Cargo.toml
@ -8,12 +8,13 @@ edition = "2021"
 [features]
 default = ["openpgp", "openpgp-card"]
 openpgp = ["sequoia-openpgp", "prompt"]
-openpgp-card = ["openpgp-card-sequoia", "card-backend-pcsc"]
+openpgp-card = ["openpgp-card-sequoia", "card-backend-pcsc", "card-backend"]
 prompt = ["keyfork-pinentry"]

 [dependencies]
 anyhow = "1.0.75"
 bincode = "1.3.3"
+card-backend = { version = "0.2.0", optional = true }
 card-backend-pcsc = { version = "0.5.0", optional = true }
 keyfork-derive-openpgp = { version = "0.1.0", path = "../keyfork-derive-openpgp" }
 keyfork-pinentry = { version = "0.5.0", path = "../keyfork-pinentry", optional = true }
--- a/keyfork-shard/src/bin/keyfork-shard-combine-openpgp.rs
+++ b/keyfork-shard/src/bin/keyfork-shard-combine-openpgp.rs
@ -58,6 +58,11 @@ fn main() -> ExitCode {
    let result = run();
    if let Err(e) = result {
        eprintln!("Error: {e}");
+        let mut source = e.source();
+        while let Some(new_error) = source.take() {
+            eprintln!("Source: {new_error}");
+            source = new_error.source();
+        }
        return ExitCode::FAILURE;
    }
    ExitCode::SUCCESS
--- a/keyfork-shard/src/openpgp.rs
+++ b/keyfork-shard/src/openpgp.rs
@ -23,7 +23,7 @@ use openpgp::{
        Marshal,
    },
    types::KeyFlags,
-    KeyID, PacketPile,
+    Fingerprint, KeyID, PacketPile,
 };
 pub use sequoia_openpgp as openpgp;
 use sharks::{Share, Sharks};
@ -34,20 +34,43 @@ use keyring::Keyring;
 mod smartcard;
 use smartcard::SmartcardManager;

-// TODO: better error handling
+#[derive(Debug, thiserror::Error)]
+pub enum Error {
+    #[error("Error with creating Share: {0}")]
+    Share(String),

-#[derive(Debug, Clone)]
-pub struct WrappedError(String);
+    #[error("Error combining shares: {0}")]
+    CombineShares(String),

-impl std::fmt::Display for WrappedError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.write_str(&self.0)
-    }
+    #[error("Derived secret hash {0} != expected {1}")]
+    InvalidSecret(Fingerprint, Fingerprint),
+
+    #[error("OpenPGP error: {0}")]
+    Sequoia(#[source] anyhow::Error),
+
+    #[error("OpenPGP IO error: {0}")]
+    SequoiaIo(#[source] std::io::Error),
+
+    #[error("Keyring error: {0}")]
+    Keyring(#[from] keyring::KeyringError),
+
+    #[error("Smartcard error: {0}")]
+    Smartcard(#[from] smartcard::SmartcardError),
+
+    #[error("IO error: {0}")]
+    Io(#[source] std::io::Error),
+
+    #[error("Derivation path: {0}")]
+    DerivationPath(#[from] keyfork_derive_openpgp::derive_util::path::Error),
+
+    #[error("Derivation request: {0}")]
+    DerivationRequest(#[from] keyfork_derive_openpgp::derive_util::request::DerivationError),
+
+    #[error("Keyfork OpenPGP: {0}")]
+    KeyforkOpenPGP(#[from] keyfork_derive_openpgp::Error),
 }

-impl std::error::Error for WrappedError {}
-
-pub type Result<T, E = Box<dyn std::error::Error>> = std::result::Result<T, E>;
+pub type Result<T, E = Error> = std::result::Result<T, E>;

 #[derive(Debug, Clone)]
 pub struct EncryptedMessage {
@ -71,24 +94,30 @@ impl EncryptedMessage {

        for pkesk in &self.pkesks {
            let mut packet = vec![];
-            pkesk.serialize(&mut packet)?;
+            pkesk.serialize(&mut packet).map_err(Error::Sequoia)?;
            let message = Message::new(&mut packets);
-            let mut message = ArbitraryWriter::new(message, Tag::PKESK)?;
-            message.write_all(&packet)?;
-            message.finalize()?;
+            let mut message = ArbitraryWriter::new(message, Tag::PKESK).map_err(Error::Sequoia)?;
+            message.write_all(&packet).map_err(Error::SequoiaIo)?;
+            message.finalize().map_err(Error::Sequoia)?;
        }
        let mut packet = vec![];
-        self.message.serialize(&mut packet)?;
+        self.message
+            .serialize(&mut packet)
+            .map_err(Error::Sequoia)?;
        let message = Message::new(&mut packets);
-        let mut message = ArbitraryWriter::new(message, Tag::SEIP)?;
-        message.write_all(&packet)?;
-        message.finalize()?;
+        let mut message = ArbitraryWriter::new(message, Tag::SEIP).map_err(Error::Sequoia)?;
+        message.write_all(&packet).map_err(Error::SequoiaIo)?;
+        message.finalize().map_err(Error::Sequoia)?;

-        let mut decryptor =
-            DecryptorBuilder::from_bytes(&packets)?.with_policy(policy, None, decryptor)?;
+        let mut decryptor = DecryptorBuilder::from_bytes(&packets)
+            .map_err(Error::Sequoia)?
+            .with_policy(policy, None, decryptor)
+            .map_err(Error::Sequoia)?;

        let mut content = vec![];
-        decryptor.read_to_end(&mut content)?;
+        decryptor
+            .read_to_end(&mut content)
+            .map_err(Error::SequoiaIo)?;
        Ok(content)
    }
 }
@ -98,18 +127,19 @@ pub fn discover_certs(path: impl AsRef<Path>) -> Result<Vec<Cert>> {

    if path.is_file() {
        let mut vec = vec![];
-        for cert in CertParser::from_file(path)? {
-            vec.push(cert?);
+        for cert in CertParser::from_file(path).map_err(Error::Sequoia)? {
+            vec.push(cert.map_err(Error::Sequoia)?);
        }
        Ok(vec)
    } else {
        let mut vec = vec![];
        for entry in path
-            .read_dir()?
+            .read_dir()
+            .map_err(Error::Io)?
            .filter_map(Result::ok)
            .filter(|p| p.path().is_file())
        {
-            vec.push(Cert::from_file(entry.path())?);
+            vec.push(Cert::from_file(entry.path()).map_err(Error::Sequoia)?);
        }
        Ok(vec)
    }
@ -119,7 +149,10 @@ pub fn parse_messages(reader: impl Read + Send + Sync) -> Result<VecDeque<Encryp
    let mut pkesks = Vec::new();
    let mut encrypted_messages = VecDeque::new();

-    for packet in PacketPile::from_reader(reader)?.into_children() {
+    for packet in PacketPile::from_reader(reader)
+        .map_err(Error::Sequoia)?
+        .into_children()
+    {
        match packet {
            Packet::PKESK(p) => pkesks.push(p),
            Packet::SEIP(s) => {
@ -186,13 +219,15 @@ pub fn combine(
        metadata.decrypt_with(&policy, &mut keyring)?
    };

-    let mut cert_parser = CertParser::from_bytes(&content)?;
+    let mut cert_parser = CertParser::from_bytes(&content).map_err(Error::Sequoia)?;
    let root_cert = match cert_parser.next() {
        Some(Ok(c)) => c,
        Some(Err(e)) => panic!("Could not find root (first) certificate: {e}"),
        None => panic!("No certs found in cert parser"),
    };
-    let certs = cert_parser.collect::<openpgp::Result<Vec<_>>>()?;
+    let certs = cert_parser
+        .collect::<openpgp::Result<Vec<_>>>()
+        .map_err(Error::Sequoia)?;
    keyring.set_root_cert(root_cert.clone());
    manager.set_root_cert(root_cert);
    let mut messages: HashMap<KeyID, EncryptedMessage> =
@ -202,12 +237,14 @@ pub fn combine(
    // NOTE: This is ONLY stable because we control the generation of PKESK packets and
    // encode the policy to ourselves.
    for valid_cert in certs.iter().map(|cert| cert.with_policy(&policy, None)) {
-        let valid_cert = valid_cert?;
+        let valid_cert = valid_cert.map_err(Error::Sequoia)?;
        // get keys from keyring for cert
        let Some(secret_cert) = keyring.get_cert_for_primary_keyid(&valid_cert.keyid()) else {
            continue;
        };
-        let secret_cert = secret_cert.with_policy(&policy, None)?;
+        let secret_cert = secret_cert
+            .with_policy(&policy, None)
+            .map_err(Error::Sequoia)?;
        let keys = get_decryption_keys(&secret_cert).collect::<Vec<_>>();
        if !keys.is_empty() {
            if let Some(message) = messages.get_mut(&valid_cert.keyid()) {
@ -252,7 +289,7 @@ pub fn combine(
                .iter()
                .map(|cert| cert.with_policy(&policy, None))
            {
-                let valid_cert = valid_cert?;
+                let valid_cert = valid_cert.map_err(Error::Sequoia)?;
                let fp = valid_cert
                    .keys()
                    .for_storage_encryption()
@ -282,8 +319,10 @@ pub fn combine(
        .values()
        .map(|message| Share::try_from(message.as_slice()))
        .collect::<Result<Vec<_>, &str>>()
-        .map_err(|e| WrappedError(e.to_string()))?;
-    let secret = Sharks(threshold).recover(&shares)?;
+        .map_err(|e| Error::Share(e.to_string()))?;
+    let secret = Sharks(threshold)
+        .recover(&shares)
+        .map_err(|e| Error::CombineShares(e.to_string()))?;

    let userid = UserID::from("keyfork-sss");
    let kdr = DerivationRequest::new(
@ -298,19 +337,18 @@ pub fn combine(
    )?;

    // NOTE: Signatures on certs will be different. Compare fingerprints instead.
-    if Some(derived_cert.fingerprint()) != keyring.root_cert().map(Cert::fingerprint) {
-        return Err(WrappedError(format!(
-            "Derived {} != expected {}",
-            derived_cert.fingerprint(),
-            keyring
-                .root_cert()
-                .expect("cert was previously set")
-                .fingerprint()
-        ))
-        .into());
+    let derived_fp = derived_cert.fingerprint();
+    let expected_fp = keyring
+        .root_cert()
+        .expect("cert was previously set")
+        .fingerprint();
+    if derived_fp != expected_fp {
+        return Err(Error::InvalidSecret(derived_fp, expected_fp));
    }

-    output.write_all(smex::encode(&secret).as_bytes())?;
+    output
+        .write_all(smex::encode(&secret).as_bytes())
+        .map_err(Error::Io)?;

    Ok(())
 }
@ -330,23 +368,25 @@ pub fn split(threshold: u8, certs: Vec<Cert>, secret: &[u8], output: impl Write)
    )?;
    let signing_key = derived_cert
        .primary_key()
-        .parts_into_secret()?
+        .parts_into_secret()
+        .map_err(Error::Sequoia)?
        .key()
        .clone()
-        .into_keypair()?;
+        .into_keypair()
+        .map_err(Error::Sequoia)?;

    let sharks = Sharks(threshold);
    let dealer = sharks.dealer(secret);
    let shares = dealer.map(|s| Vec::from(&s)).collect::<Vec<_>>();
    let policy = StandardPolicy::new();
-    let mut writer = Writer::new(output, Kind::Message)?;
+    let mut writer = Writer::new(output, Kind::Message).map_err(Error::SequoiaIo)?;

    let mut total_recipients = vec![];
    let mut messages = vec![];

    for (share, cert) in shares.iter().zip(certs) {
        total_recipients.push(cert.clone());
-        let valid_cert = cert.with_policy(&policy, None)?;
+        let valid_cert = cert.with_policy(&policy, None).map_err(Error::Sequoia)?;
        let encryption_keys = get_encryption_keys(&valid_cert).collect::<Vec<_>>();

        let mut message_output = vec![];
@ -357,28 +397,34 @@ pub fn split(threshold: u8, certs: Vec<Cert>, secret: &[u8], output: impl Write)
                .iter()
                .map(|k| Recipient::new(KeyID::wildcard(), k.key())),
        )
-        .build()?;
-        let message = Signer::new(message, signing_key.clone()).build()?;
-        let mut message = LiteralWriter::new(message).build()?;
-        message.write_all(share)?;
-        message.finalize()?;
+        .build()
+        .map_err(Error::Sequoia)?;
+        let message = Signer::new(message, signing_key.clone())
+            .build()
+            .map_err(Error::Sequoia)?;
+        let mut message = LiteralWriter::new(message)
+            .build()
+            .map_err(Error::Sequoia)?;
+        message.write_all(share).map_err(Error::SequoiaIo)?;
+        message.finalize().map_err(Error::Sequoia)?;

        messages.push(message_output);
    }

    let mut pp = vec![];
    // store derived cert to verify provided shares
-    derived_cert.serialize(&mut pp)?;
+    derived_cert.serialize(&mut pp).map_err(Error::Sequoia)?;
    for recipient in &total_recipients {
-        recipient.serialize(&mut pp)?;
+        recipient.serialize(&mut pp).map_err(Error::Sequoia)?;
    }

    // verify packet pile
-    for (packet_cert, cert) in openpgp::cert::CertParser::from_bytes(&pp)?
+    for (packet_cert, cert) in openpgp::cert::CertParser::from_bytes(&pp)
+        .map_err(Error::Sequoia)?
        .skip(1)
        .zip(total_recipients.iter())
    {
-        if packet_cert? != *cert {
+        if packet_cert.map_err(Error::Sequoia)? != *cert {
            panic!(
                "packet pile could not recreate cert: {}",
                cert.fingerprint()
@ -389,7 +435,8 @@ pub fn split(threshold: u8, certs: Vec<Cert>, secret: &[u8], output: impl Write)
    let valid_certs = total_recipients
        .iter()
        .map(|c| c.with_policy(&policy, None))
-        .collect::<openpgp::Result<Vec<_>>>()?;
+        .collect::<openpgp::Result<Vec<_>>>()
+        .map_err(Error::Sequoia)?;

    let total_recipients = valid_certs.iter().flat_map(|vc| {
        get_encryption_keys(vc).map(|key| Recipient::new(KeyID::wildcard(), key.key()))
@ -398,17 +445,23 @@ pub fn split(threshold: u8, certs: Vec<Cert>, secret: &[u8], output: impl Write)
    // metadata
    let mut message_output = vec![];
    let message = Message::new(&mut message_output);
-    let message = Encryptor2::for_recipients(message, total_recipients).build()?;
-    let mut message = LiteralWriter::new(message).build()?;
-    message.write_all(&pp)?;
-    message.finalize()?;
-    writer.write_all(&message_output)?;
+    let message = Encryptor2::for_recipients(message, total_recipients)
+        .build()
+        .map_err(Error::Sequoia)?;
+    let mut message = LiteralWriter::new(message)
+        .build()
+        .map_err(Error::Sequoia)?;
+    message.write_all(&pp).map_err(Error::SequoiaIo)?;
+    message.finalize().map_err(Error::Sequoia)?;
+    writer
+        .write_all(&message_output)
+        .map_err(Error::SequoiaIo)?;

    for message in messages {
-        writer.write_all(&message)?;
+        writer.write_all(&message).map_err(Error::SequoiaIo)?;
    }

-    writer.finalize()?;
+    writer.finalize().map_err(Error::SequoiaIo)?;

    Ok(())
 }
--- a/keyfork-shard/src/openpgp/keyring.rs
+++ b/keyfork-shard/src/openpgp/keyring.rs
@ -9,27 +9,18 @@ use super::openpgp::{
    KeyHandle, KeyID,
 };

-use crate::prompt_manager::PromptManager;
+use crate::prompt_manager::{PromptManager, PinentryError};

-#[derive(Clone, Debug)]
-pub enum KeyringFailure {
+#[derive(thiserror::Error, Debug)]
+pub enum KeyringError {
+    #[error("Secret key was not found")]
    SecretKeyNotFound,
-    #[allow(dead_code)]
-    SmartcardDecrypt,
+
+    #[error("Prompt failed: {0}")]
+    Prompt(#[from] PinentryError)
 }

-impl std::fmt::Display for KeyringFailure {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            KeyringFailure::SecretKeyNotFound => f.write_str("Secret key was not found"),
-            KeyringFailure::SmartcardDecrypt => {
-                f.write_str("Smartcard could not decrypt any PKESKs")
-            }
-        }
-    }
-}
-
-impl std::error::Error for KeyringFailure {}
+pub type Result<T, E = KeyringError> = std::result::Result<T, E>;

 pub struct Keyring {
    full_certs: Vec<Cert>,
@ -38,7 +29,7 @@ pub struct Keyring {
 }

 impl Keyring {
-    pub fn new(certs: impl AsRef<[Cert]>) -> Result<Self, Box<dyn std::error::Error>> {
+    pub fn new(certs: impl AsRef<[Cert]>) -> Result<Self> {
        Ok(Self {
            full_certs: certs.as_ref().to_vec(),
            root: Default::default(),
@ -164,6 +155,6 @@ impl DecryptionHelper for &mut Keyring {
            }
        }

-        Err(KeyringFailure::SecretKeyNotFound.into())
+        Err(KeyringError::SecretKeyNotFound.into())
    }
 }
--- a/keyfork-shard/src/openpgp/smartcard.rs
+++ b/keyfork-shard/src/openpgp/smartcard.rs
@ -9,27 +9,41 @@ use super::openpgp::{
    parse::stream::{DecryptionHelper, MessageLayer, MessageStructure, VerificationHelper},
    Fingerprint,
 };
-use crate::prompt_manager::PromptManager;
+use crate::prompt_manager::{PinentryError, PromptManager};

 use card_backend_pcsc::PcscBackend;
 use openpgp_card_sequoia::{state::Open, Card};

-#[derive(Clone, Debug)]
-pub enum SmartcardFailure {
+#[derive(thiserror::Error, Debug)]
+pub enum SmartcardError {
+    #[error("No smart card backend was stored")]
    SmartCardNotFound,
+
+    #[error("Selected smart card has no decryption key")]
    SmartCardHasNoDecrypt,
+
+    #[error("Smart card backend error: {0}")]
+    SmartCardBackendError(#[from] card_backend::SmartcardError),
+
+    #[error("Smart card error: {0}")]
+    SmartCardEncounteredError(#[from] openpgp_card_sequoia::types::Error),
+
+    #[error("Prompt failed: {0}")]
+    Prompt(#[from] PinentryError),
 }

-impl std::fmt::Display for SmartcardFailure {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            Self::SmartCardNotFound => f.write_str("No smart card backend was stored"),
-            Self::SmartCardHasNoDecrypt => f.write_str("Selected smart card has no decrypt key"),
-        }
-    }
-}
+pub type Result<T, E = SmartcardError> = std::result::Result<T, E>;

-impl std::error::Error for SmartcardFailure {}
+fn format_name(input: impl AsRef<str>) -> String {
+    let mut n = input
+        .as_ref()
+        .split("<<")
+        .take(2)
+        .map(|s| s.replace('>', " "))
+        .collect::<Vec<_>>();
+    n.reverse();
+    n.join(" ")
+}

 pub struct SmartcardManager {
    current_card: Option<Card<Open>>,
@ -38,7 +52,7 @@ pub struct SmartcardManager {
 }

 impl SmartcardManager {
-    pub fn new() -> Result<Self, Box<dyn std::error::Error>> {
+    pub fn new() -> Result<Self> {
        Ok(Self {
            current_card: None,
            root: None,
@ -54,20 +68,20 @@ impl SmartcardManager {
    }

    /// Load any backend.
-    pub fn load_any_card(&mut self) -> Result<Fingerprint, Box<dyn std::error::Error>> {
+    pub fn load_any_card(&mut self) -> Result<Fingerprint> {
        PcscBackend::cards(None)?
            .next()
            .transpose()?
-            .ok_or(SmartcardFailure::SmartCardNotFound.into())
+            .ok_or(SmartcardError::SmartCardNotFound)
            .and_then(
-                |backend| -> Result<Fingerprint, Box<dyn std::error::Error>> {
+                |backend| {
                    let mut card = Card::<Open>::new(backend)?;
                    let transaction = card.transaction()?;
                    let fingerprint = transaction
                        .fingerprints()?
                        .decryption()
                        .map(|fp| Fingerprint::from_bytes(fp.as_bytes()))
-                        .ok_or(SmartcardFailure::SmartCardHasNoDecrypt)?;
+                        .ok_or(SmartcardError::SmartCardHasNoDecrypt)?;
                    drop(transaction);
                    self.current_card.replace(card);
                    Ok(fingerprint)
@ -81,7 +95,7 @@ impl SmartcardManager {
    pub fn load_any_fingerprint(
        &mut self,
        fingerprints: impl IntoIterator<Item = Fingerprint>,
-    ) -> Result<Option<Fingerprint>, Box<dyn std::error::Error>> {
+    ) -> Result<Option<Fingerprint>> {
        // NOTE: This can't be HashSet::from_iter() because from_iter() requires a passed-in state
        // I do not want to provide.
        let mut requested_fingerprints = HashSet::new();
@ -167,7 +181,7 @@ impl DecryptionHelper for &mut SmartcardManager {
    {
        let mut card = self.current_card.take();
        let Some(card) = card.as_mut() else {
-            return Err(SmartcardFailure::SmartCardNotFound.into());
+            return Err(SmartcardError::SmartCardNotFound.into());
        };

        let mut transaction = card.transaction()?;
@ -176,9 +190,9 @@ impl DecryptionHelper for &mut SmartcardManager {
            .decryption()
            .map(|fp| Fingerprint::from_bytes(fp.as_bytes()));
        let Some(fp) = fp else {
-            return Err(SmartcardFailure::SmartCardHasNoDecrypt.into());
+            return Err(SmartcardError::SmartCardHasNoDecrypt.into());
        };
-        let cardholder_name = transaction.cardholder_name()?;
+        let cardholder_name = format_name(transaction.cardholder_name()?);
        let card_id = transaction.application_identifier()?.ident();
        let message = if cardholder_name.is_empty() {
            format!("Unlock card {card_id}")
@ -199,6 +213,6 @@ impl DecryptionHelper for &mut SmartcardManager {
            }
        }

-        Err(SmartcardFailure::SmartCardNotFound.into())
+        Err(SmartcardError::SmartCardNotFound.into())
    }
 }
--- a/keyfork-slip10-test-data/src/lib.rs
+++ b/keyfork-slip10-test-data/src/lib.rs
@ -219,37 +219,67 @@ pub fn test_data() -> Result<HashMap<String, Vec<TestData>>, Box<dyn std::error:
                Test {
                    chain: "m/0'",
                    fingerprint: hex::decode("ddebc675")?,
-                    chain_code: hex::decode("8b59aa11380b624e81507a27fedda59fea6d0b779a778918a2fd3590e16e9c69")?,
-                    private_key: hex::decode("68e0fe46dfb67e368c75379acec591dad19df3cde26e63b93a8e704f1dade7a3")?,
-                    public_key: hex::decode("008c8a13df77a28f3445213a0f432fde644acaa215fc72dcdf300d5efaa85d350c")?,
+                    chain_code: hex::decode(
+                        "8b59aa11380b624e81507a27fedda59fea6d0b779a778918a2fd3590e16e9c69",
+                    )?,
+                    private_key: hex::decode(
+                        "68e0fe46dfb67e368c75379acec591dad19df3cde26e63b93a8e704f1dade7a3",
+                    )?,
+                    public_key: hex::decode(
+                        "008c8a13df77a28f3445213a0f432fde644acaa215fc72dcdf300d5efaa85d350c",
+                    )?,
                },
                Test {
                    chain: "m/0'/1'",
                    fingerprint: hex::decode("13dab143")?,
-                    chain_code: hex::decode("a320425f77d1b5c2505a6b1b27382b37368ee640e3557c315416801243552f14")?,
-                    private_key: hex::decode("b1d0bad404bf35da785a64ca1ac54b2617211d2777696fbffaf208f746ae84f2")?,
-                    public_key: hex::decode("001932a5270f335bed617d5b935c80aedb1a35bd9fc1e31acafd5372c30f5c1187")?,
+                    chain_code: hex::decode(
+                        "a320425f77d1b5c2505a6b1b27382b37368ee640e3557c315416801243552f14",
+                    )?,
+                    private_key: hex::decode(
+                        "b1d0bad404bf35da785a64ca1ac54b2617211d2777696fbffaf208f746ae84f2",
+                    )?,
+                    public_key: hex::decode(
+                        "001932a5270f335bed617d5b935c80aedb1a35bd9fc1e31acafd5372c30f5c1187",
+                    )?,
                },
                Test {
                    chain: "m/0'/1'/2'",
                    fingerprint: hex::decode("ebe4cb29")?,
-                    chain_code: hex::decode("2e69929e00b5ab250f49c3fb1c12f252de4fed2c1db88387094a0f8c4c9ccd6c")?,
-                    private_key: hex::decode("92a5b23c0b8a99e37d07df3fb9966917f5d06e02ddbd909c7e184371463e9fc9")?,
-                    public_key: hex::decode("00ae98736566d30ed0e9d2f4486a64bc95740d89c7db33f52121f8ea8f76ff0fc1")?,
+                    chain_code: hex::decode(
+                        "2e69929e00b5ab250f49c3fb1c12f252de4fed2c1db88387094a0f8c4c9ccd6c",
+                    )?,
+                    private_key: hex::decode(
+                        "92a5b23c0b8a99e37d07df3fb9966917f5d06e02ddbd909c7e184371463e9fc9",
+                    )?,
+                    public_key: hex::decode(
+                        "00ae98736566d30ed0e9d2f4486a64bc95740d89c7db33f52121f8ea8f76ff0fc1",
+                    )?,
                },
                Test {
                    chain: "m/0'/1'/2'/2'",
                    fingerprint: hex::decode("316ec1c6")?,
-                    chain_code: hex::decode("8f6d87f93d750e0efccda017d662a1b31a266e4a6f5993b15f5c1f07f74dd5cc")?,
-                    private_key: hex::decode("30d1dc7e5fc04c31219ab25a27ae00b50f6fd66622f6e9c913253d6511d1e662")?,
-                    public_key: hex::decode("008abae2d66361c879b900d204ad2cc4984fa2aa344dd7ddc46007329ac76c429c")?,
+                    chain_code: hex::decode(
+                        "8f6d87f93d750e0efccda017d662a1b31a266e4a6f5993b15f5c1f07f74dd5cc",
+                    )?,
+                    private_key: hex::decode(
+                        "30d1dc7e5fc04c31219ab25a27ae00b50f6fd66622f6e9c913253d6511d1e662",
+                    )?,
+                    public_key: hex::decode(
+                        "008abae2d66361c879b900d204ad2cc4984fa2aa344dd7ddc46007329ac76c429c",
+                    )?,
                },
                Test {
                    chain: "m/0'/1'/2'/2'/1000000000'",
                    fingerprint: hex::decode("d6322ccd")?,
-                    chain_code: hex::decode("68789923a0cac2cd5a29172a475fe9e0fb14cd6adb5ad98a3fa70333e7afa230")?,
-                    private_key: hex::decode("8f94d394a8e8fd6b1bc2f3f49f5c47e385281d5c17e65324b0f62483e37e8793")?,
-                    public_key: hex::decode("003c24da049451555d51a7014a37337aa4e12d41e485abccfa46b47dfb2af54b7a")?,
+                    chain_code: hex::decode(
+                        "68789923a0cac2cd5a29172a475fe9e0fb14cd6adb5ad98a3fa70333e7afa230",
+                    )?,
+                    private_key: hex::decode(
+                        "8f94d394a8e8fd6b1bc2f3f49f5c47e385281d5c17e65324b0f62483e37e8793",
+                    )?,
+                    public_key: hex::decode(
+                        "003c24da049451555d51a7014a37337aa4e12d41e485abccfa46b47dfb2af54b7a",
+                    )?,
                },
            ],
        },
@ -259,44 +289,80 @@ pub fn test_data() -> Result<HashMap<String, Vec<TestData>>, Box<dyn std::error:
                Test {
                    chain: "m",
                    fingerprint: hex::decode("00000000")?,
-                    chain_code: hex::decode("ef70a74db9c3a5af931b5fe73ed8e1a53464133654fd55e7a66f8570b8e33c3b")?,
-                    private_key: hex::decode("171cb88b1b3c1db25add599712e36245d75bc65a1a5c9e18d76f9f2b1eab4012")?,
-                    public_key: hex::decode("008fe9693f8fa62a4305a140b9764c5ee01e455963744fe18204b4fb948249308a")?,
+                    chain_code: hex::decode(
+                        "ef70a74db9c3a5af931b5fe73ed8e1a53464133654fd55e7a66f8570b8e33c3b",
+                    )?,
+                    private_key: hex::decode(
+                        "171cb88b1b3c1db25add599712e36245d75bc65a1a5c9e18d76f9f2b1eab4012",
+                    )?,
+                    public_key: hex::decode(
+                        "008fe9693f8fa62a4305a140b9764c5ee01e455963744fe18204b4fb948249308a",
+                    )?,
                },
                Test {
                    chain: "m/0'",
                    fingerprint: hex::decode("31981b50")?,
-                    chain_code: hex::decode("0b78a3226f915c082bf118f83618a618ab6dec793752624cbeb622acb562862d")?,
-                    private_key: hex::decode("1559eb2bbec5790b0c65d8693e4d0875b1747f4970ae8b650486ed7470845635")?,
-                    public_key: hex::decode("0086fab68dcb57aa196c77c5f264f215a112c22a912c10d123b0d03c3c28ef1037")?,
+                    chain_code: hex::decode(
+                        "0b78a3226f915c082bf118f83618a618ab6dec793752624cbeb622acb562862d",
+                    )?,
+                    private_key: hex::decode(
+                        "1559eb2bbec5790b0c65d8693e4d0875b1747f4970ae8b650486ed7470845635",
+                    )?,
+                    public_key: hex::decode(
+                        "0086fab68dcb57aa196c77c5f264f215a112c22a912c10d123b0d03c3c28ef1037",
+                    )?,
                },
                Test {
                    chain: "m/0'/2147483647'",
                    fingerprint: hex::decode("1e9411b1")?,
-                    chain_code: hex::decode("138f0b2551bcafeca6ff2aa88ba8ed0ed8de070841f0c4ef0165df8181eaad7f")?,
-                    private_key: hex::decode("ea4f5bfe8694d8bb74b7b59404632fd5968b774ed545e810de9c32a4fb4192f4")?,
-                    public_key: hex::decode("005ba3b9ac6e90e83effcd25ac4e58a1365a9e35a3d3ae5eb07b9e4d90bcf7506d")?,
+                    chain_code: hex::decode(
+                        "138f0b2551bcafeca6ff2aa88ba8ed0ed8de070841f0c4ef0165df8181eaad7f",
+                    )?,
+                    private_key: hex::decode(
+                        "ea4f5bfe8694d8bb74b7b59404632fd5968b774ed545e810de9c32a4fb4192f4",
+                    )?,
+                    public_key: hex::decode(
+                        "005ba3b9ac6e90e83effcd25ac4e58a1365a9e35a3d3ae5eb07b9e4d90bcf7506d",
+                    )?,
                },
                Test {
                    chain: "m/0'/2147483647'/1'",
                    fingerprint: hex::decode("fcadf38c")?,
-                    chain_code: hex::decode("73bd9fff1cfbde33a1b846c27085f711c0fe2d66fd32e139d3ebc28e5a4a6b90")?,
-                    private_key: hex::decode("3757c7577170179c7868353ada796c839135b3d30554bbb74a4b1e4a5a58505c")?,
-                    public_key: hex::decode("002e66aa57069c86cc18249aecf5cb5a9cebbfd6fadeab056254763874a9352b45")?,
+                    chain_code: hex::decode(
+                        "73bd9fff1cfbde33a1b846c27085f711c0fe2d66fd32e139d3ebc28e5a4a6b90",
+                    )?,
+                    private_key: hex::decode(
+                        "3757c7577170179c7868353ada796c839135b3d30554bbb74a4b1e4a5a58505c",
+                    )?,
+                    public_key: hex::decode(
+                        "002e66aa57069c86cc18249aecf5cb5a9cebbfd6fadeab056254763874a9352b45",
+                    )?,
                },
                Test {
                    chain: "m/0'/2147483647'/1'/2147483646'",
                    fingerprint: hex::decode("aca70953")?,
-                    chain_code: hex::decode("0902fe8a29f9140480a00ef244bd183e8a13288e4412d8389d140aac1794825a")?,
-                    private_key: hex::decode("5837736c89570de861ebc173b1086da4f505d4adb387c6a1b1342d5e4ac9ec72")?,
-                    public_key: hex::decode("00e33c0f7d81d843c572275f287498e8d408654fdf0d1e065b84e2e6f157aab09b")?,
+                    chain_code: hex::decode(
+                        "0902fe8a29f9140480a00ef244bd183e8a13288e4412d8389d140aac1794825a",
+                    )?,
+                    private_key: hex::decode(
+                        "5837736c89570de861ebc173b1086da4f505d4adb387c6a1b1342d5e4ac9ec72",
+                    )?,
+                    public_key: hex::decode(
+                        "00e33c0f7d81d843c572275f287498e8d408654fdf0d1e065b84e2e6f157aab09b",
+                    )?,
                },
                Test {
                    chain: "m/0'/2147483647'/1'/2147483646'/2'",
                    fingerprint: hex::decode("422c654b")?,
-                    chain_code: hex::decode("5d70af781f3a37b829f0d060924d5e960bdc02e85423494afc0b1a41bbe196d4")?,
-                    private_key: hex::decode("551d333177df541ad876a60ea71f00447931c0a9da16f227c11ea080d7391b8d")?,
-                    public_key: hex::decode("0047150c75db263559a70d5778bf36abbab30fb061ad69f69ece61a72b0cfa4fc0")?,
+                    chain_code: hex::decode(
+                        "5d70af781f3a37b829f0d060924d5e960bdc02e85423494afc0b1a41bbe196d4",
+                    )?,
+                    private_key: hex::decode(
+                        "551d333177df541ad876a60ea71f00447931c0a9da16f227c11ea080d7391b8d",
+                    )?,
+                    public_key: hex::decode(
+                        "0047150c75db263559a70d5778bf36abbab30fb061ad69f69ece61a72b0cfa4fc0",
+                    )?,
                },
            ],
        },
--- a/keyfork/src/cli/mnemonic.rs
+++ b/keyfork/src/cli/mnemonic.rs
@ -1,5 +1,5 @@
 use super::Keyfork;
-use clap::{Parser, Subcommand, ValueEnum, builder::PossibleValue};
+use clap::{builder::PossibleValue, Parser, Subcommand, ValueEnum};
 use std::fmt::Display;

 #[derive(Clone, Debug, Default)]
--- a/keyfork/src/main.rs
+++ b/keyfork/src/main.rs
@ -7,5 +7,7 @@ mod cli;
 fn main() {
    let opts = cli::Keyfork::parse();

-    opts.command.handle(&opts).expect("Unable to handle command");
+    opts.command
+        .handle(&opts)
+        .expect("Unable to handle command");
 }
--- a/keyforkd-client/src/tests.rs
+++ b/keyforkd-client/src/tests.rs
@ -17,7 +17,6 @@ fn secp256k1() {
    let rt = Builder::new_multi_thread().enable_io().build().unwrap();
    let tempdir = TempDir::new("keyfork-seed").unwrap();
    for (i, per_seed) in tests.into_iter().enumerate() {
-
        let mut socket_name = i.to_string();
        socket_name.push_str("-keyforkd.sock");
        let socket_path = tempdir.path().join(socket_name);
@ -57,15 +56,11 @@ fn secp256k1() {

 #[test]
 fn ed25519() {
-    let tests = test_data()
-        .unwrap()
-        .remove(&"ed25519".to_string())
-        .unwrap();
+    let tests = test_data().unwrap().remove(&"ed25519".to_string()).unwrap();

    let rt = Builder::new_multi_thread().enable_io().build().unwrap();
    let tempdir = TempDir::new("keyfork-seed").unwrap();
    for (i, per_seed) in tests.into_iter().enumerate() {
-
        let mut socket_name = i.to_string();
        socket_name.push_str("-keyforkd.sock");
        let socket_path = tempdir.path().join(socket_name);
--- a/keyforkd/src/service.rs
+++ b/keyforkd/src/service.rs
@ -1,7 +1,7 @@
 use std::{future::Future, pin::Pin, sync::Arc, task::Poll};

-use keyfork_derive_util::request::{DerivationError, DerivationRequest, DerivationResponse};
 use keyfork_derive_path_data::guess_target;
+use keyfork_derive_util::request::{DerivationError, DerivationRequest, DerivationResponse};
 use tower::Service;
 use tracing::info;

@ -101,10 +101,7 @@ mod tests {

    #[tokio::test]
    async fn properly_derives_ed25519() {
-        let tests = test_data()
-            .unwrap()
-            .remove(&"ed25519".to_string())
-            .unwrap();
+        let tests = test_data().unwrap().remove(&"ed25519".to_string()).unwrap();

        for per_seed in tests {
            let seed = &per_seed.seed;
Author	SHA1	Message	Date
Ryan Heywood	307941087a	keyfork-shard: slightly improved error handling	2023-11-05 13:57:22 -06:00
Ryan Heywood	0768339487	keyfork-shard: parse cardholder name when using smartcards	2023-11-05 01:36:12 -05:00
Ryan Heywood	4b4b85931f	cargo fmt	2023-11-05 01:29:10 -05:00