keyfork-shard: further improve error handling, add multiline prompt and PIN retry detection

keyfork-pinentry/Cargo.toml

@@ -11,7 +11,8 @@ authors = ["Ryan Heywood <ryan@distrust.co>"]
 # categories = ["api-bindings", "command-line-interface"]
 # license = "MIT OR Apache-2.0"
 license = "MIT"
-edition = "2018"
+# edition = "2018"
+edition = "2021"
 
 [dependencies]
 nom = { version = "7", default-features = false }

keyfork-pinentry/src/lib.rs

@@ -69,6 +69,24 @@ pub fn default_binary() -> Result<PathBuf> {
     which::which("pinentry-curses").map_err(Into::into)
 }
 
+fn convert_multiline(line: &str) -> String {
+    // convert into multiline
+    let mut converted_line = String::new();
+    let mut last_end = 0;
+    for (start, part) in line.match_indices(&['\n', '\r', '%']) {
+        converted_line.push_str(line.get(last_end..start).unwrap());
+        converted_line.push_str(match part {
+            "\n" => "%0A",
+            "\r" => "%0D",
+            "%" => "%25",
+            fb => panic!("expected index given to match_indices, got: {fb}"),
+        });
+        last_end = start + part.len();
+    }
+    converted_line.push_str(line.get(last_end..line.len()).unwrap());
+    converted_line
+}
+
 /// A dialog for requesting a passphrase from the user.
 pub struct PassphraseInput<'a> {
     binary: PathBuf,
@@ -211,7 +229,7 @@ impl<'a> PassphraseInput<'a> {
             pinentry.send_request("SETTITLE", Some(title))?;
         }
         if let Some(desc) = &self.description {
-            pinentry.send_request("SETDESC", Some(desc))?;
+            pinentry.send_request("SETDESC", Some(convert_multiline(desc).as_ref()))?;
         }
         if let Some(error) = &self.error {
             pinentry.send_request("SETERROR", Some(error))?;

keyfork-shard/src/openpgp.rs

@@ -52,10 +52,10 @@ pub enum Error {
     SequoiaIo(#[source] std::io::Error),
 
     #[error("Keyring error: {0}")]
-    Keyring(#[from] keyring::KeyringError),
+    Keyring(#[from] keyring::Error),
 
     #[error("Smartcard error: {0}")]
-    Smartcard(#[from] smartcard::SmartcardError),
+    Smartcard(#[from] smartcard::Error),
 
     #[error("IO error: {0}")]
     Io(#[source] std::io::Error),

keyfork-shard/src/openpgp/keyring.rs

@@ -9,18 +9,20 @@ use super::openpgp::{
     KeyHandle, KeyID,
 };
 
-use crate::prompt_manager::{PromptManager, PinentryError};
+use crate::prompt_manager::{PinentryError, PromptManager};
+
+use anyhow::Context;
 
 #[derive(thiserror::Error, Debug)]
-pub enum KeyringError {
+pub enum Error {
     #[error("Secret key was not found")]
     SecretKeyNotFound,
 
     #[error("Prompt failed: {0}")]
-    Prompt(#[from] PinentryError)
+    Prompt(#[from] PinentryError),
 }
 
-pub type Result<T, E = KeyringError> = std::result::Result<T, E>;
+pub type Result<T, E = Error> = std::result::Result<T, E>;
 
 pub struct Keyring {
     full_certs: Vec<Cert>,
@@ -89,8 +91,9 @@ impl VerificationHelper for &mut Keyring {
                 MessageLayer::SignatureGroup { results } => {
                     for result in results {
                         if let Err(e) = result {
-                            // FIXME: anyhow leak
+                            // FIXME: anyhow leak: VerificationError impl std::error::Error
-                            return Err(anyhow::anyhow!(e.to_string()));
+                            // return Err(e.context("Invalid signature"));
+                            return Err(anyhow::anyhow!("Invalid signature: {e}"));
                         }
                     }
                 }
@@ -121,7 +124,9 @@ impl DecryptionHelper for &mut Keyring {
                     .next()
                     .map(|userid| userid.userid().name().transpose())
                     .flatten()
-                    .transpose()?;
+                    .transpose()
+                    .ok()
+                    .flatten();
                 for key in cert
                     .keys()
                     .with_policy(&null, None)
@@ -130,7 +135,9 @@ impl DecryptionHelper for &mut Keyring {
                 {
                     let secret_key = key.key().clone();
                     let mut keypair = if secret_key.has_unencrypted_secret() {
-                        secret_key.into_keypair()?
+                        secret_key
+                            .into_keypair()
+                            .context("Has unencrypted secret")?
                     } else {
                         let message = if let Some(name) = name.as_ref() {
                             format!("Decryption key for: {} ({name})", secret_key.keyid())
@@ -139,10 +146,13 @@ impl DecryptionHelper for &mut Keyring {
                         };
                         let passphrase = self
                             .pm
-                            .prompt_passphrase("Decryption passphrase", message)?;
+                            .prompt_passphrase("Decryption passphrase", message)
-                        let key = secret_key
+                            .context("Decryption passphrase")?;
-                            .decrypt_secret(&passphrase.expose_secret().as_str().into())?;
+                        secret_key
-                        key.into_keypair()?
+                            .decrypt_secret(&passphrase.expose_secret().as_str().into())
+                            .context("has_unencrypted_secret is false, could not decrypt secret")?
+                            .into_keypair()
+                            .context("just-decrypted key")?
                     };
                     if pkesk
                         .decrypt(&mut keypair, sym_algo)
@@ -155,6 +165,6 @@ impl DecryptionHelper for &mut Keyring {
             }
         }
 
-        Err(KeyringError::SecretKeyNotFound.into())
+        Err(Error::SecretKeyNotFound.into())
     }
 }

keyfork-shard/src/openpgp/smartcard.rs

@@ -11,11 +11,12 @@ use super::openpgp::{
 };
 use crate::prompt_manager::{PinentryError, PromptManager};
 
+use anyhow::Context;
 use card_backend_pcsc::PcscBackend;
-use openpgp_card_sequoia::{state::Open, Card};
+use openpgp_card_sequoia::{state::Open, types::Error as SequoiaCardError, Card};
 
 #[derive(thiserror::Error, Debug)]
-pub enum SmartcardError {
+pub enum Error {
     #[error("No smart card backend was stored")]
     SmartCardNotFound,
 
@@ -23,16 +24,28 @@ pub enum SmartcardError {
     SmartCardHasNoDecrypt,
 
     #[error("Smart card backend error: {0}")]
-    SmartCardBackendError(#[from] card_backend::SmartcardError),
+    SmartCardBackend(#[from] card_backend::SmartcardError),
 
-    #[error("Smart card error: {0}")]
+    #[error("Smartcard password status unavailable: {0}")]
-    SmartCardEncounteredError(#[from] openpgp_card_sequoia::types::Error),
+    PwStatusBytes(SequoiaCardError),
+
+    #[error("Could not open smart card")]
+    OpenSmartCard(SequoiaCardError),
+
+    #[error("Could not initialize transaction")]
+    Transaction(SequoiaCardError),
+
+    #[error("Could not load fingerprints")]
+    Fingerprints(SequoiaCardError),
+
+    #[error("Invalid PIN entered too many times")]
+    InvalidPIN,
 
     #[error("Prompt failed: {0}")]
     Prompt(#[from] PinentryError),
 }
 
-pub type Result<T, E = SmartcardError> = std::result::Result<T, E>;
+pub type Result<T, E = Error> = std::result::Result<T, E>;
 
 fn format_name(input: impl AsRef<str>) -> String {
     let mut n = input
@@ -72,21 +85,20 @@ impl SmartcardManager {
         PcscBackend::cards(None)?
             .next()
             .transpose()?
-            .ok_or(SmartcardError::SmartCardNotFound)
+            .ok_or(Error::SmartCardNotFound)
-            .and_then(
+            .and_then(|backend| {
-                |backend| {
+                let mut card = Card::<Open>::new(backend).map_err(Error::OpenSmartCard)?;
-                    let mut card = Card::<Open>::new(backend)?;
+                let transaction = card.transaction().map_err(Error::Transaction)?;
-                    let transaction = card.transaction()?;
                 let fingerprint = transaction
-                        .fingerprints()?
+                    .fingerprints()
+                    .map_err(Error::Fingerprints)?
                     .decryption()
                     .map(|fp| Fingerprint::from_bytes(fp.as_bytes()))
-                        .ok_or(SmartcardError::SmartCardHasNoDecrypt)?;
+                    .ok_or(Error::SmartCardHasNoDecrypt)?;
                 drop(transaction);
                 self.current_card.replace(card);
                 Ok(fingerprint)
-                },
+            })
-            )
     }
 
     /// Load a backend if any [`Fingerprint`] has been matched by a currently active card.
@@ -108,11 +120,12 @@ impl SmartcardManager {
             for backend in PcscBackend::cards(None)? {
                 had_any_backend = true;
                 let backend = backend?;
-                let mut card = Card::<Open>::new(backend)?;
+                let mut card = Card::<Open>::new(backend).map_err(Error::OpenSmartCard)?;
-                let transaction = card.transaction()?;
+                let transaction = card.transaction().map_err(Error::Transaction)?;
                 let mut fingerprint = None;
                 if let Some(fp) = transaction
-                    .fingerprints()?
+                    .fingerprints()
+                    .map_err(Error::Fingerprints)?
                     .decryption()
                     .map(|fp| Fingerprint::from_bytes(fp.as_bytes()))
                 {
@@ -127,9 +140,8 @@ impl SmartcardManager {
                 }
             }
 
-            eprintln!("No matching smartcard detected.");
+            #[rustfmt::skip]
-            self.pm
+            self.pm.prompt_message("Please plug in a smart card and press enter")?;
-                .prompt_message("Please plug in a smart card and press enter")?;
         }
 
         Ok(None)
@@ -158,7 +170,7 @@ impl VerificationHelper for &mut SmartcardManager {
                     for result in results {
                         if let Err(e) = result {
                             // FIXME: anyhow leak
-                            return Err(anyhow::anyhow!(e.to_string()));
+                            return Err(anyhow::anyhow!("Verification error: {}", e.to_string()));
                         }
                     }
                 }
@@ -181,28 +193,59 @@ impl DecryptionHelper for &mut SmartcardManager {
     {
         let mut card = self.current_card.take();
         let Some(card) = card.as_mut() else {
-            return Err(SmartcardError::SmartCardNotFound.into());
+            return Err(Error::SmartCardNotFound.into());
         };
 
-        let mut transaction = card.transaction()?;
+        let mut transaction = card
+            .transaction()
+            .context("Could not initialize transaction")?;
         let fp = transaction
-            .fingerprints()?
+            .fingerprints()
+            .context("Could not load fingerprints")?
             .decryption()
-            .map(|fp| Fingerprint::from_bytes(fp.as_bytes()));
+            .map(|fp| Fingerprint::from_bytes(fp.as_bytes()))
-        let Some(fp) = fp else {
+            .ok_or(Error::SmartCardHasNoDecrypt)?;
-            return Err(SmartcardError::SmartCardHasNoDecrypt.into());
+        let cardholder_name = format_name(
-        };
+            transaction
-        let cardholder_name = format_name(transaction.cardholder_name()?);
+                .cardholder_name()
-        let card_id = transaction.application_identifier()?.ident();
+                .context("Could not load (optionally empty) cardholder name")?,
+        );
+        let card_id = transaction
+            .application_identifier()
+            .context("Could not load application identifier")?
+            .ident();
+        let pw_status = transaction
+            .pw_status_bytes()
+            .map_err(Error::PwStatusBytes)?;
+        let mut pin = None;
+        for _ in 0..pw_status.err_count_pw1() {
+            transaction.reload_ard()?;
+            let attempts = transaction
+                .pw_status_bytes()
+                .map_err(Error::PwStatusBytes)?
+                .err_count_pw1();
+            let rpea = "Remaining PIN entry attempts";
             let message = if cardholder_name.is_empty() {
-            format!("Unlock card {card_id}")
+                format!("Unlock card {card_id}\n\n{rpea}: {attempts}")
             } else {
-            format!("Unlock card {card_id} ({cardholder_name})")
+                format!("Unlock card {card_id} ({cardholder_name})\n\n{rpea}: {attempts}")
             };
-        let pin = self.pm.prompt_passphrase("Smartcard User PIN", message)?;
+            let temp_pin = self.pm.prompt_passphrase("Smartcard User PIN", message)?;
-        let mut user = transaction.to_user_card(pin.expose_secret().as_str().trim())?;
+            if transaction
-        let mut decryptor =
+                .verify_user_pin(temp_pin.expose_secret().as_str().trim())
-            user.decryptor(&|| eprintln!("Touch confirmation needed for decryption"))?;
+                .is_ok()
+            {
+                pin.replace(temp_pin);
+                break;
+            }
+        }
+        let pin = pin.ok_or(Error::InvalidPIN)?;
+        let mut user = transaction
+            .to_user_card(pin.expose_secret().as_str().trim())
+            .context("Could not load user smartcard from PIN")?;
+        let mut decryptor = user
+            .decryptor(&|| eprintln!("Touch confirmation needed for decryption"))
+            .context("Could not decrypt using smartcard")?;
         for pkesk in pkesks {
             if pkesk
                 .decrypt(&mut decryptor, sym_algo)
@@ -213,6 +256,6 @@ impl DecryptionHelper for &mut SmartcardManager {
             }
         }
 
-        Err(SmartcardError::SmartCardNotFound.into())
+        Err(Error::SmartCardNotFound.into())
     }
 }

keyfork/src/cli/shard.rs

@@ -66,7 +66,7 @@ impl ShardExec for OpenPGP {
             "cert count {} != max {max}",
             certs.len()
         );
-        keyfork_shard::openpgp::split(threshold, certs, secret, output)
+        keyfork_shard::openpgp::split(threshold, certs, secret, output).map_err(Into::into)
     }
 
     fn combine<T>(