Reformatted to conform to vanilla rustfmt.

card-functionality/src/cards.rs

@@ -120,9 +120,7 @@ impl TestCard {
 
                 Ok(card?)
             }
-            Self::Scdc(serial) => {
+            Self::Scdc(serial) => Ok(Box::new(ScdBackend::open_by_serial(None, serial)?)),
-                Ok(Box::new(ScdBackend::open_by_serial(None, serial)?))
-            }
         }
     }
 }

card-functionality/src/import.rs

@@ -44,8 +44,7 @@ fn main() -> Result<()> {
         for key_file in &key_files {
             // upload keys
             print!("Upload key '{}'", key_file);
-            let upload_res =
+            let upload_res = run_test(&mut card, test_upload_keys, &[key_file]);
-                run_test(&mut card, test_upload_keys, &[key_file]);
 
             if let Err(TestError::KeyUploadError(_file, err)) = &upload_res {
                 // The card doesn't support this key type, so skip to the
@@ -59,8 +58,7 @@ fn main() -> Result<()> {
             let upload_out = upload_res?;
             println!(" {:x?}", upload_out);
 
-            let key = std::fs::read_to_string(key_file)
+            let key = std::fs::read_to_string(key_file).expect("Unable to read ciphertext");
-                .expect("Unable to read ciphertext");
 
             // decrypt
             print!("  Decrypt");
@@ -68,8 +66,7 @@ fn main() -> Result<()> {
             let c = Cert::from_str(&key)?;
             let ciphertext = util::encrypt_to("Hello world!\n", &c)?;
 
-            let dec_out =
+            let dec_out = run_test(&mut card, test_decrypt, &[&key, &ciphertext])?;
-                run_test(&mut card, test_decrypt, &[&key, &ciphertext])?;
             println!(" {:x?}", dec_out);
 
             // sign

card-functionality/src/keygen.rs

@@ -68,8 +68,7 @@ fn main() -> Result<()> {
                 let ciphertext = util::encrypt_to("Hello world!\n", &c)?;
 
                 print!("  Decrypt");
-                let dec_out =
+                let dec_out = run_test(&mut card, test_decrypt, &[cert, &ciphertext])?;
-                    run_test(&mut card, test_decrypt, &[cert, &ciphertext])?;
                 println!(" {:x?}", dec_out);
             } else {
                 panic!("Didn't get back a Cert from test_keygen");

card-functionality/src/tests.rs

@@ -17,9 +17,7 @@ use openpgp_card::algorithm::AlgoSimple;
 use openpgp_card::card_do::{KeyGenerationTime, Sex};
 use openpgp_card::{CardTransaction, Error, KeyType, StatusBytes};
 use openpgp_card_sequoia::card::Open;
-use openpgp_card_sequoia::util::{
+use openpgp_card_sequoia::util::{make_cert, public_key_material_to_key, public_to_fingerprint};
-    make_cert, public_key_material_to_key, public_to_fingerprint,
-};
 
 use crate::cards::TestCardData;
 use crate::util;
@@ -69,12 +67,7 @@ pub fn test_decrypt(
 
     let p = StandardPolicy::new();
 
-    let res = openpgp_card_sequoia::util::decrypt(
+    let res = openpgp_card_sequoia::util::decrypt(card_tx, &cert, msg.into_bytes(), &p)?;
-        card_tx,
-        &cert,
-        msg.into_bytes(),
-        &p,
-    )?;
     let plain = String::from_utf8_lossy(&res);
 
     assert_eq!(plain, "Hello world!\n");
@@ -94,8 +87,7 @@ pub fn test_sign(
     let cert = Cert::from_str(param[0])?;
 
     let msg = "Hello world, I am signed.";
-    let sig =
+    let sig = openpgp_card_sequoia::util::sign(card_tx, &cert, &mut msg.as_bytes())?;
-        openpgp_card_sequoia::util::sign(card_tx, &cert, &mut msg.as_bytes())?;
 
     // validate sig
     assert!(util::verify_sig(&cert, msg.as_bytes(), sig.as_bytes())?);
@@ -227,29 +219,17 @@ pub fn test_keygen(
     let alg = AlgoSimple::try_from(algo)?;
 
     println!(" Generate subkey for Signing");
-    let (pkm, ts) = card_tx.generate_key_simple(
+    let (pkm, ts) = card_tx.generate_key_simple(public_to_fingerprint, KeyType::Signing, alg)?;
-        public_to_fingerprint,
-        KeyType::Signing,
-        alg,
-    )?;
     let key_sig = public_key_material_to_key(&pkm, KeyType::Signing, ts)?;
 
     println!(" Generate subkey for Decryption");
-    let (pkm, ts) = card_tx.generate_key_simple(
+    let (pkm, ts) = card_tx.generate_key_simple(public_to_fingerprint, KeyType::Decryption, alg)?;
-        public_to_fingerprint,
-        KeyType::Decryption,
-        alg,
-    )?;
     let key_dec = public_key_material_to_key(&pkm, KeyType::Decryption, ts)?;
 
     println!(" Generate subkey for Authentication");
-    let (pkm, ts) = card_tx.generate_key_simple(
+    let (pkm, ts) =
-        public_to_fingerprint,
+        card_tx.generate_key_simple(public_to_fingerprint, KeyType::Authentication, alg)?;
-        KeyType::Authentication,
+    let key_aut = public_key_material_to_key(&pkm, KeyType::Authentication, ts)?;
-        alg,
-    )?;
-    let key_aut =
-        public_key_material_to_key(&pkm, KeyType::Authentication, ts)?;
 
     // Generate a Cert for this set of generated keys
     let mut open = Open::new(card_tx)?;
@@ -622,9 +602,7 @@ pub fn test_reset_retry_counter(
         Err(Error::CardStatus(StatusBytes::AuthenticationMethodBlocked)) => {
             // this is expected
         }
-        Err(Error::CardStatus(
+        Err(Error::CardStatus(StatusBytes::IncorrectParametersCommandDataField)) => {
-            StatusBytes::IncorrectParametersCommandDataField,
-        )) => {
             println!(
                 "yk says IncorrectParametersCommandDataField when PW \
             error count is exceeded"
@@ -668,10 +646,7 @@ pub fn test_reset_retry_counter(
 
 pub fn run_test(
     tc: &mut TestCardData,
-    t: fn(
+    t: fn(&mut (dyn CardTransaction + Send + Sync), &[&str]) -> Result<TestOutput, TestError>,
-        &mut (dyn CardTransaction + Send + Sync),
-        &[&str],
-    ) -> Result<TestOutput, TestError>,
     param: &[&str],
 ) -> Result<TestOutput, TestError> {
     let mut card = tc.get_card()?;

card-functionality/src/util.rs

@@ -6,14 +6,11 @@ use std::io::Write;
 use std::time::SystemTime;
 
 use sequoia_openpgp::parse::stream::{
-    DetachedVerifierBuilder, MessageLayer, MessageStructure,
+    DetachedVerifierBuilder, MessageLayer, MessageStructure, VerificationHelper,
-    VerificationHelper,
 };
 use sequoia_openpgp::parse::Parse;
 use sequoia_openpgp::policy::{Policy, StandardPolicy};
-use sequoia_openpgp::serialize::stream::{
+use sequoia_openpgp::serialize::stream::{Armorer, Encryptor, LiteralWriter, Message};
-    Armorer, Encryptor, LiteralWriter, Message,
-};
 use sequoia_openpgp::Cert;
 
 use openpgp_card::card_do::KeyGenerationTime;
@@ -77,14 +74,9 @@ impl<'a> VerificationHelper for VHelper<'a> {
         Ok(vec![self.cert.clone()])
     }
 
-    fn check(
+    fn check(&mut self, structure: MessageStructure) -> sequoia_openpgp::Result<()> {
-        &mut self,
-        structure: MessageStructure,
-    ) -> sequoia_openpgp::Result<()> {
         // We are interested in signatures over the data (level 0 signatures)
-        if let Some(MessageLayer::SignatureGroup { results }) =
+        if let Some(MessageLayer::SignatureGroup { results }) = structure.into_iter().next() {
-            structure.into_iter().next()
-        {
             match results.into_iter().next() {
                 Some(Ok(_)) => Ok(()), // Good signature.
                 Some(Err(e)) => Err(sequoia_openpgp::Error::from(e).into()),
@@ -98,8 +90,7 @@ impl<'a> VerificationHelper for VHelper<'a> {
 
 pub fn verify_sig(cert: &Cert, msg: &[u8], sig: &[u8]) -> Result<bool> {
     let vh = VHelper::new(cert);
-    let mut dv = DetachedVerifierBuilder::from_bytes(&sig)?
+    let mut dv = DetachedVerifierBuilder::from_bytes(&sig)?.with_policy(SP, None, vh)?;
-        .with_policy(SP, None, vh)?;
 
     Ok(dv.verify_bytes(msg).is_ok())
 }

openpgp-card-sequoia/src/card.rs

@@ -12,9 +12,9 @@ use sequoia_openpgp::Cert;
 
 use openpgp_card::algorithm::{Algo, AlgoInfo, AlgoSimple};
 use openpgp_card::card_do::{
-    ApplicationIdentifier, ApplicationRelatedData, CardholderRelatedData,
+    ApplicationIdentifier, ApplicationRelatedData, CardholderRelatedData, ExtendedCapabilities,
-    ExtendedCapabilities, ExtendedLengthInfo, Fingerprint, HistoricalBytes,
+    ExtendedLengthInfo, Fingerprint, HistoricalBytes, KeyGenerationTime, Lang, PWStatusBytes,
-    KeyGenerationTime, Lang, PWStatusBytes, SecuritySupportTemplate, Sex,
+    SecuritySupportTemplate, Sex,
 };
 use openpgp_card::{CardTransaction, Error, KeySet, KeyType, Response};
 
@@ -46,9 +46,7 @@ pub struct Open<'a> {
 }
 
 impl<'a> Open<'a> {
-    pub fn new(
+    pub fn new(card_tx: &'a mut (dyn CardTransaction + Send + Sync)) -> Result<Self, Error> {
-        card_tx: &'a mut (dyn CardTransaction + Send + Sync),
-    ) -> Result<Self, Error> {
         let ard = card_tx.application_related_data()?;
 
         Ok(Self {
@@ -73,10 +71,7 @@ impl<'a> Open<'a> {
         Ok(())
     }
 
-    pub fn verify_user_pinpad(
+    pub fn verify_user_pinpad(&mut self, prompt: &dyn Fn()) -> Result<(), Error> {
-        &mut self,
-        prompt: &dyn Fn(),
-    ) -> Result<(), Error> {
         prompt();
 
         let _ = self.card_tx.verify_pw1_pinpad()?;
@@ -93,10 +88,7 @@ impl<'a> Open<'a> {
         Ok(())
     }
 
-    pub fn verify_user_for_signing_pinpad(
+    pub fn verify_user_for_signing_pinpad(&mut self, prompt: &dyn Fn()) -> Result<(), Error> {
-        &mut self,
-        prompt: &dyn Fn(),
-    ) -> Result<(), Error> {
         prompt();
 
         let _ = self.card_tx.verify_pw1_for_signing_pinpad()?;
@@ -113,10 +105,7 @@ impl<'a> Open<'a> {
         Ok(())
     }
 
-    pub fn verify_admin_pinpad(
+    pub fn verify_admin_pinpad(&mut self, prompt: &dyn Fn()) -> Result<(), Error> {
-        &mut self,
-        prompt: &dyn Fn(),
-    ) -> Result<(), Error> {
         prompt();
 
         let _ = self.card_tx.verify_pw3_pinpad()?;
@@ -138,43 +127,25 @@ impl<'a> Open<'a> {
         self.card_tx.check_pw3()
     }
 
-    pub fn change_user_pin(
+    pub fn change_user_pin(&mut self, old: &str, new: &str) -> Result<Response, Error> {
-        &mut self,
-        old: &str,
-        new: &str,
-    ) -> Result<Response, Error> {
         self.card_tx.change_pw1(old, new)
     }
 
-    pub fn change_user_pin_pinpad(
+    pub fn change_user_pin_pinpad(&mut self, prompt: &dyn Fn()) -> Result<Response, Error> {
-        &mut self,
-        prompt: &dyn Fn(),
-    ) -> Result<Response, Error> {
         prompt();
         self.card_tx.change_pw1_pinpad()
     }
 
-    pub fn reset_user_pin(
+    pub fn reset_user_pin(&mut self, rst: &str, new: &str) -> Result<Response, Error> {
-        &mut self,
-        rst: &str,
-        new: &str,
-    ) -> Result<Response, Error> {
         self.card_tx
             .reset_retry_counter_pw1(new.into(), Some(rst.into()))
     }
 
-    pub fn change_admin_pin(
+    pub fn change_admin_pin(&mut self, old: &str, new: &str) -> Result<Response, Error> {
-        &mut self,
-        old: &str,
-        new: &str,
-    ) -> Result<Response, Error> {
         self.card_tx.change_pw3(old, new)
     }
 
-    pub fn change_admin_pin_pinpad(
+    pub fn change_admin_pin_pinpad(&mut self, prompt: &dyn Fn()) -> Result<Response, Error> {
-        &mut self,
-        prompt: &dyn Fn(),
-    ) -> Result<Response, Error> {
         prompt();
         self.card_tx.change_pw3_pinpad()
     }
@@ -208,9 +179,7 @@ impl<'a> Open<'a> {
 
     // --- application data ---
 
-    pub fn application_identifier(
+    pub fn application_identifier(&self) -> Result<ApplicationIdentifier, Error> {
-        &self,
-    ) -> Result<ApplicationIdentifier, Error> {
         self.ard.application_id()
     }
 
@@ -218,9 +187,7 @@ impl<'a> Open<'a> {
         self.ard.historical_bytes()
     }
 
-    pub fn extended_length_information(
+    pub fn extended_length_information(&self) -> Result<Option<ExtendedLengthInfo>> {
-        &self,
-    ) -> Result<Option<ExtendedLengthInfo>> {
         self.ard.extended_length_information()
     }
 
@@ -234,9 +201,7 @@ impl<'a> Open<'a> {
         unimplemented!()
     }
 
-    pub fn extended_capabilities(
+    pub fn extended_capabilities(&self) -> Result<ExtendedCapabilities, Error> {
-        &self,
-    ) -> Result<ExtendedCapabilities, Error> {
         self.ard.extended_capabilities()
     }
 
@@ -258,9 +223,7 @@ impl<'a> Open<'a> {
         unimplemented!()
     }
 
-    pub fn key_generation_times(
+    pub fn key_generation_times(&self) -> Result<KeySet<KeyGenerationTime>, Error> {
-        &self,
-    ) -> Result<KeySet<KeyGenerationTime>, Error> {
         self.ard.key_generation_times()
     }
 
@@ -300,16 +263,12 @@ impl<'a> Open<'a> {
     }
 
     // --- cardholder related data (65) ---
-    pub fn cardholder_related_data(
+    pub fn cardholder_related_data(&mut self) -> Result<CardholderRelatedData> {
-        &mut self,
-    ) -> Result<CardholderRelatedData> {
         self.card_tx.cardholder_related_data()
     }
 
     // --- security support template (7a) ---
-    pub fn security_support_template(
+    pub fn security_support_template(&mut self) -> Result<SecuritySupportTemplate> {
-        &mut self,
-    ) -> Result<SecuritySupportTemplate> {
         self.card_tx.security_support_template()
     }
 
@@ -334,10 +293,7 @@ impl<'a> Open<'a> {
 
     // ----------
 
-    pub fn public_key(
+    pub fn public_key(&mut self, key_type: KeyType) -> Result<PublicKeyMaterial> {
-        &mut self,
-        key_type: KeyType,
-    ) -> Result<PublicKeyMaterial> {
         self.card_tx.public_key(key_type).map_err(|e| e.into())
     }
 
@@ -368,10 +324,7 @@ pub struct Sign<'app, 'open> {
 }
 
 impl Sign<'_, '_> {
-    pub fn signer(
+    pub fn signer(&mut self, cert: &Cert) -> std::result::Result<CardSigner, Error> {
-        &mut self,
-        cert: &Cert,
-    ) -> std::result::Result<CardSigner, Error> {
         // FIXME: depending on the setting in "PW1 Status byte", only one
         // signature can be made after verification for signing
 
@@ -431,8 +384,7 @@ impl Admin<'_, '_> {
         // Check for max len
         let ec = self.oc.extended_capabilities()?;
 
-        if ec.max_len_special_do() == None
+        if ec.max_len_special_do() == None || url.len() <= ec.max_len_special_do().unwrap() as usize
-            || url.len() <= ec.max_len_special_do().unwrap() as usize
         {
             // If we don't know the max length for URL ("special DO"),
             // or if it's within the acceptable length:
@@ -444,10 +396,7 @@ impl Admin<'_, '_> {
         }
     }
 
-    pub fn set_resetting_code(
+    pub fn set_resetting_code(&mut self, pin: &str) -> Result<Response, Error> {
-        &mut self,
-        pin: &str,
-    ) -> Result<Response, Error> {
         self.oc.card_tx.set_resetting_code(pin.into())
     }
 
@@ -474,16 +423,15 @@ impl Admin<'_, '_> {
         algo: Option<AlgoSimple>,
     ) -> Result<(PublicKeyMaterial, KeyGenerationTime), Error> {
         match algo {
-            Some(algo) => self.oc.card_tx.generate_key_simple(
+            Some(algo) => {
-                public_to_fingerprint,
+                self.oc
-                key_type,
+                    .card_tx
-                algo,
+                    .generate_key_simple(public_to_fingerprint, key_type, algo)
-            ),
+            }
-            None => self.oc.card_tx.generate_key(
+            None => self
-                public_to_fingerprint,
+                .oc
-                key_type,
+                .card_tx
-                None,
+                .generate_key(public_to_fingerprint, key_type, None),
-            ),
         }
     }
 }

openpgp-card-sequoia/src/decryptor.rs

@@ -7,9 +7,7 @@ use openpgp::crypto;
 use openpgp::crypto::mpi;
 use openpgp::crypto::SessionKey;
 use openpgp::packet;
-use openpgp::parse::stream::{
+use openpgp::parse::stream::{DecryptionHelper, MessageStructure, VerificationHelper};
-    DecryptionHelper, MessageStructure, VerificationHelper,
-};
 use openpgp::types::{Curve, SymmetricAlgorithm};
 use openpgp::Cert;
 use sequoia_openpgp as openpgp;
@@ -87,10 +85,7 @@ impl<'a> crypto::Decryptor for CardDecryptor<'a> {
                 let sk = openpgp::crypto::SessionKey::from(&dec[..]);
                 Ok(sk)
             }
-            (
+            (mpi::Ciphertext::ECDH { ref e, .. }, mpi::PublicKey::ECDH { ref curve, .. }) => {
-                mpi::Ciphertext::ECDH { ref e, .. },
-                mpi::PublicKey::ECDH { ref curve, .. },
-            ) => {
                 let dm = if curve == &Curve::Cv25519 {
                     assert_eq!(
                         e.value()[0],
@@ -112,10 +107,7 @@ impl<'a> crypto::Decryptor for CardDecryptor<'a> {
                 // (Gnuk returns a leading '0x04' byte and
                 // an additional 32 trailing bytes)
                 if curve == &Curve::NistP256 && dec.len() == 65 {
-                    assert_eq!(
+                    assert_eq!(dec[0], 0x04, "Unexpected shape of decrypted NistP256 data");
-                        dec[0], 0x04,
-                        "Unexpected shape of decrypted NistP256 data"
-                    );
 
                     // see Gnuk src/call-ec.c:82
                     dec = dec[1..33].to_vec();
@@ -168,10 +160,7 @@ impl<'a> DecryptionHelper for CardDecryptor<'a> {
 }
 
 impl<'a> VerificationHelper for CardDecryptor<'a> {
-    fn get_certs(
+    fn get_certs(&mut self, _ids: &[openpgp::KeyHandle]) -> openpgp::Result<Vec<openpgp::Cert>> {
-        &mut self,
-        _ids: &[openpgp::KeyHandle],
-    ) -> openpgp::Result<Vec<openpgp::Cert>> {
         Ok(vec![])
     }
     fn check(&mut self, _structure: MessageStructure) -> openpgp::Result<()> {

openpgp-card-sequoia/src/main.rs

@@ -120,23 +120,17 @@ fn main() -> Result<(), Box<dyn Error>> {
         let cert = Cert::from_file(TEST_KEY_PATH)?;
         let p = StandardPolicy::new();
 
-        if let Some(vka) =
+        if let Some(vka) = sq_util::subkey_by_type(&cert, &p, KeyType::Signing)? {
-            sq_util::subkey_by_type(&cert, &p, KeyType::Signing)?
-        {
             println!("Upload signing key");
             admin.upload_key(vka, KeyType::Signing, None)?;
         }
 
-        if let Some(vka) =
+        if let Some(vka) = sq_util::subkey_by_type(&cert, &p, KeyType::Decryption)? {
-            sq_util::subkey_by_type(&cert, &p, KeyType::Decryption)?
-        {
             println!("Upload decryption key");
             admin.upload_key(vka, KeyType::Decryption, None)?;
         }
 
-        if let Some(vka) =
+        if let Some(vka) = sq_util::subkey_by_type(&cert, &p, KeyType::Authentication)? {
-            sq_util::subkey_by_type(&cert, &p, KeyType::Authentication)?
-        {
             println!("Upload auth key");
             admin.upload_key(vka, KeyType::Authentication, None)?;
         }
@@ -170,8 +164,7 @@ fn main() -> Result<(), Box<dyn Error>> {
             .expect("We just validated, this should not fail");
 
         let cert = Cert::from_file(TEST_KEY_PATH)?;
-        let msg = std::fs::read_to_string(TEST_ENC_MSG)
+        let msg = std::fs::read_to_string(TEST_ENC_MSG).expect("Unable to read file");
-            .expect("Unable to read file");
 
         println!("Encrypted message:\n{}", msg);
 

openpgp-card-sequoia/src/privkey.rs

@@ -16,9 +16,7 @@ use openpgp::types::Timestamp;
 use sequoia_openpgp as openpgp;
 
 use openpgp_card::card_do::{Fingerprint, KeyGenerationTime};
-use openpgp_card::crypto_data::{
+use openpgp_card::crypto_data::{CardUploadableKey, EccKey, EccType, PrivateKeyMaterial, RSAKey};
-    CardUploadableKey, EccKey, EccType, PrivateKeyMaterial, RSAKey,
-};
 use openpgp_card::Error;
 use sequoia_openpgp::types::Curve;
 
@@ -55,14 +53,14 @@ impl CardUploadableKey for SequoiaKey {
         // Decrypt key with password, if set
         let key = match &self.password {
             None => self.key.clone(),
-            Some(pw) => self.key.clone().decrypt_secret(
+            Some(pw) => self
-                &openpgp::crypto::Password::from(pw.as_str()),
+                .key
-            )?,
+                .clone()
+                .decrypt_secret(&openpgp::crypto::Password::from(pw.as_str()))?,
         };
 
         // Get private cryptographic material
-        let unenc = if let Some(key::SecretKeyMaterial::Unencrypted(ref u)) =
+        let unenc = if let Some(key::SecretKeyMaterial::Unencrypted(ref u)) = key.optional_secret()
-            key.optional_secret()
         {
             u
         } else {
@@ -72,44 +70,28 @@ impl CardUploadableKey for SequoiaKey {
         let secret_key_material = unenc.map(|mpis| mpis.clone());
 
         match (self.public.clone(), secret_key_material) {
-            (
+            (mpi::PublicKey::RSA { e, n }, mpi::SecretKeyMaterial::RSA { d, p, q, u: _ }) => {
-                mpi::PublicKey::RSA { e, n },
-                mpi::SecretKeyMaterial::RSA { d, p, q, u: _ },
-            ) => {
                 let sq_rsa = SqRSA::new(e, d, n, p, q)?;
 
                 Ok(PrivateKeyMaterial::R(Box::new(sq_rsa)))
             }
-            (
+            (mpi::PublicKey::ECDH { curve, q, .. }, mpi::SecretKeyMaterial::ECDH { scalar }) => {
-                mpi::PublicKey::ECDH { curve, q, .. },
-                mpi::SecretKeyMaterial::ECDH { scalar },
-            ) => {
                 let sq_ecc = SqEccKey::new(curve, scalar, q, EccType::ECDH);
 
                 Ok(PrivateKeyMaterial::E(Box::new(sq_ecc)))
             }
-            (
+            (mpi::PublicKey::ECDSA { curve, q, .. }, mpi::SecretKeyMaterial::ECDSA { scalar }) => {
-                mpi::PublicKey::ECDSA { curve, q, .. },
-                mpi::SecretKeyMaterial::ECDSA { scalar },
-            ) => {
                 let sq_ecc = SqEccKey::new(curve, scalar, q, EccType::ECDSA);
 
                 Ok(PrivateKeyMaterial::E(Box::new(sq_ecc)))
             }
-            (
+            (mpi::PublicKey::EdDSA { curve, q, .. }, mpi::SecretKeyMaterial::EdDSA { scalar }) => {
-                mpi::PublicKey::EdDSA { curve, q, .. },
-                mpi::SecretKeyMaterial::EdDSA { scalar },
-            ) => {
                 let sq_ecc = SqEccKey::new(curve, scalar, q, EccType::EdDSA);
 
                 Ok(PrivateKeyMaterial::E(Box::new(sq_ecc)))
             }
             (p, s) => {
-                unimplemented!(
+                unimplemented!("Unexpected algorithms: public {:?}, secret {:?}", p, s);
-                    "Unexpected algorithms: public {:?}, secret {:?}",
-                    p,
-                    s
-                );
             }
         }
     }
@@ -142,19 +124,8 @@ struct SqRSA {
 
 impl SqRSA {
     #[allow(clippy::many_single_char_names)]
-    fn new(
+    fn new(e: MPI, d: ProtectedMPI, n: MPI, p: ProtectedMPI, q: ProtectedMPI) -> Result<Self> {
-        e: MPI,
+        let nettle = nettle::rsa::PrivateKey::new(d.value(), p.value(), q.value(), None)?;
-        d: ProtectedMPI,
-        n: MPI,
-        p: ProtectedMPI,
-        q: ProtectedMPI,
-    ) -> Result<Self> {
-        let nettle = nettle::rsa::PrivateKey::new(
-            d.value(),
-            p.value(),
-            q.value(),
-            None,
-        )?;
 
         Ok(Self { e, n, p, q, nettle })
     }
@@ -201,12 +172,7 @@ struct SqEccKey {
 }
 
 impl SqEccKey {
-    fn new(
+    fn new(curve: Curve, private: ProtectedMPI, public: MPI, ecc_type: EccType) -> Self {
-        curve: Curve,
-        private: ProtectedMPI,
-        public: MPI,
-        ecc_type: EccType,
-    ) -> Self {
         SqEccKey {
             curve,
             private,

openpgp-card-sequoia/src/signer.rs

@@ -86,10 +86,7 @@ impl<'a> crypto::Signer for CardSigner<'a> {
         match (self.public.pk_algo(), self.public.mpis()) {
             #[allow(deprecated)]
             (PublicKeyAlgorithm::RSASign, mpi::PublicKey::RSA { .. })
-            | (
+            | (PublicKeyAlgorithm::RSAEncryptSign, mpi::PublicKey::RSA { .. }) => {
-                PublicKeyAlgorithm::RSAEncryptSign,
-                mpi::PublicKey::RSA { .. },
-            ) => {
                 let hash = match hash_algo {
                     openpgp::types::HashAlgorithm::SHA256 => Hash::SHA256(
                         digest
@@ -129,10 +126,7 @@ impl<'a> crypto::Signer for CardSigner<'a> {
 
                 Ok(mpi::Signature::EdDSA { r, s })
             }
-            (
+            (PublicKeyAlgorithm::ECDSA, mpi::PublicKey::ECDSA { curve, .. }) => {
-                PublicKeyAlgorithm::ECDSA,
-                mpi::PublicKey::ECDSA { curve, .. },
-            ) => {
                 let hash = match curve {
                     Curve::NistP256 => Hash::ECDSA(&digest[..32]),
                     Curve::NistP384 => Hash::ECDSA(&digest[..48]),

openpgp-card-sequoia/src/sq_util.rs

@@ -98,8 +98,7 @@ pub fn get_subkey_by_fingerprint<'a>(
 ) -> Result<Option<ErasedKeyAmalgamation<'a, PublicParts>>, Error> {
     // Find the (sub)key in `cert` that matches the fingerprint from
     // the Card's signing-key slot.
-    let keys: Vec<_> =
+    let keys: Vec<_> = cert.keys().filter(|ka| &ka.fingerprint() == fp).collect();
-        cert.keys().filter(|ka| &ka.fingerprint() == fp).collect();
 
     if keys.is_empty() {
         Ok(None)
@@ -131,11 +130,7 @@ where
 
 /// Produce decrypted plaintext from a VerificationHelper+DecryptionHelper
 /// `d` and the ciphertext `msg`.
-pub fn decryption_helper<D>(
+pub fn decryption_helper<D>(d: D, msg: Vec<u8>, p: &dyn Policy) -> Result<Vec<u8>>
-    d: D,
-    msg: Vec<u8>,
-    p: &dyn Policy,
-) -> Result<Vec<u8>>
 where
     D: VerificationHelper + DecryptionHelper,
 {

openpgp-card-sequoia/src/util.rs

@@ -66,15 +66,14 @@ pub fn make_cert<'a, 'app>(
 
         // 3) make binding, sign with card -> add
         {
-            let signing_builder =
+            let signing_builder = SignatureBuilder::new(SignatureType::SubkeyBinding)
-                SignatureBuilder::new(SignatureType::SubkeyBinding)
+                .set_signature_creation_time(SystemTime::now())?
-                    .set_signature_creation_time(SystemTime::now())?
+                .set_key_validity_period(std::time::Duration::new(0, 0))?
-                    .set_key_validity_period(std::time::Duration::new(0, 0))?
+                .set_key_flags(
-                    .set_key_flags(
+                    KeyFlags::empty()
-                        KeyFlags::empty()
+                        .set_storage_encryption()
-                            .set_storage_encryption()
+                        .set_transport_encryption(),
-                            .set_transport_encryption(),
+                )?;
-                    )?;
 
             // Allow signing on the card
             if let Some(pw1) = pw1.clone() {
@@ -102,11 +101,10 @@ pub fn make_cert<'a, 'app>(
 
         // 5) make, sign binding -> add
         {
-            let signing_builder =
+            let signing_builder = SignatureBuilder::new(SignatureType::SubkeyBinding)
-                SignatureBuilder::new(SignatureType::SubkeyBinding)
+                .set_signature_creation_time(SystemTime::now())?
-                    .set_signature_creation_time(SystemTime::now())?
+                .set_key_validity_period(std::time::Duration::new(0, 0))?
-                    .set_key_validity_period(std::time::Duration::new(0, 0))?
+                .set_key_flags(KeyFlags::empty().set_authentication())?;
-                    .set_key_flags(KeyFlags::empty().set_authentication())?;
 
             // Allow signing on the card
             if let Some(pw1) = pw1.clone() {
@@ -137,21 +135,19 @@ pub fn make_cert<'a, 'app>(
 
     // FIXME: accept email as argument?!
 
-    let uid: UserID =
+    let uid: UserID = cardholder.name().expect("expecting name on card").into();
-        cardholder.name().expect("expecting name on card").into();
 
     pp.push(uid.clone().into());
 
     // 7) make, sign binding -> add
     {
-        let signing_builder =
+        let signing_builder = SignatureBuilder::new(SignatureType::PositiveCertification)
-            SignatureBuilder::new(SignatureType::PositiveCertification)
+            .set_signature_creation_time(SystemTime::now())?
-                .set_signature_creation_time(SystemTime::now())?
+            .set_key_validity_period(std::time::Duration::new(0, 0))?
-                .set_key_validity_period(std::time::Duration::new(0, 0))?
+            .set_key_flags(
-                .set_key_flags(
+                // Flags for primary key
-                    // Flags for primary key
+                KeyFlags::empty().set_signing().set_certification(),
-                    KeyFlags::empty().set_signing().set_certification(),
+            )?;
-                )?;
 
         // Allow signing on the card
         if let Some(pw1) = pw1 {
@@ -167,8 +163,7 @@ pub fn make_cert<'a, 'app>(
             // Temporary version of the cert
             let cert = Cert::try_from(pp.clone())?;
 
-            let signing_bsig: Packet =
+            let signing_bsig: Packet = uid.bind(&mut card_signer, &cert, signing_builder)?.into();
-                uid.bind(&mut card_signer, &cert, signing_builder)?.into();
 
             pp.push(signing_bsig);
         }
@@ -207,8 +202,7 @@ pub fn public_key_material_to_key(
                     KeyType::Authentication | KeyType::Signing => {
                         if algo_ecc.curve() == Curve::Ed25519 {
                             // EdDSA
-                            let k4 =
+                            let k4 = Key4::import_public_ed25519(ecc.data(), time)?;
-                                Key4::import_public_ed25519(ecc.data(), time)?;
 
                             Ok(Key::from(k4))
                         } else {
@@ -231,12 +225,7 @@ pub fn public_key_material_to_key(
                             // ok when a cert already exists
 
                             // EdDSA
-                            let k4 = Key4::import_public_cv25519(
+                            let k4 = Key4::import_public_cv25519(ecc.data(), None, None, time)?;
-                                ecc.data(),
-                                None,
-                                None,
-                                time,
-                            )?;
 
                             Ok(k4.into())
                         } else {

openpgp-card/src/algorithm.rs

@@ -55,9 +55,7 @@ impl AlgoSimple {
     /// Get corresponding EccType by KeyType (except for Curve25519)
     fn ecc_type(key_type: KeyType) -> EccType {
         match key_type {
-            KeyType::Signing
+            KeyType::Signing | KeyType::Authentication | KeyType::Attestation => EccType::ECDSA,
-            | KeyType::Authentication
-            | KeyType::Attestation => EccType::ECDSA,
             KeyType::Decryption => EccType::ECDH,
         }
     }
@@ -65,9 +63,7 @@ impl AlgoSimple {
     /// Get corresponding EccType by KeyType for Curve25519
     fn ecc_type_25519(key_type: KeyType) -> EccType {
         match key_type {
-            KeyType::Signing
+            KeyType::Signing | KeyType::Authentication | KeyType::Attestation => EccType::EdDSA,
-            | KeyType::Authentication
-            | KeyType::Attestation => EccType::EdDSA,
             KeyType::Decryption => EccType::ECDH,
         }
     }
@@ -75,9 +71,7 @@ impl AlgoSimple {
     /// Get corresponding Curve by KeyType for 25519 (Ed25519 vs Cv25519)
     fn curve_for_25519(key_type: KeyType) -> Curve {
         match key_type {
-            KeyType::Signing
+            KeyType::Signing | KeyType::Authentication | KeyType::Attestation => Curve::Ed25519,
-            | KeyType::Authentication
-            | KeyType::Attestation => Curve::Ed25519,
             KeyType::Decryption => Curve::Cv25519,
         }
     }
@@ -94,18 +88,10 @@ impl AlgoSimple {
         algo_info: Option<AlgoInfo>,
     ) -> Result<Algo> {
         let algo = match self {
-            Self::RSA1k => Algo::Rsa(keys::determine_rsa_attrs(
+            Self::RSA1k => Algo::Rsa(keys::determine_rsa_attrs(1024, key_type, ard, algo_info)?),
-                1024, key_type, ard, algo_info,
+            Self::RSA2k => Algo::Rsa(keys::determine_rsa_attrs(2048, key_type, ard, algo_info)?),
-            )?),
+            Self::RSA3k => Algo::Rsa(keys::determine_rsa_attrs(3072, key_type, ard, algo_info)?),
-            Self::RSA2k => Algo::Rsa(keys::determine_rsa_attrs(
+            Self::RSA4k => Algo::Rsa(keys::determine_rsa_attrs(4096, key_type, ard, algo_info)?),
-                2048, key_type, ard, algo_info,
-            )?),
-            Self::RSA3k => Algo::Rsa(keys::determine_rsa_attrs(
-                3072, key_type, ard, algo_info,
-            )?),
-            Self::RSA4k => Algo::Rsa(keys::determine_rsa_attrs(
-                4096, key_type, ard, algo_info,
-            )?),
             Self::NIST256 => Algo::Ecc(keys::determine_ecc_attrs(
                 Curve::NistP256r1.oid(),
                 Self::ecc_type(key_type),
@@ -225,10 +211,7 @@ impl Algo {
     }
 
     /// Helper: generate `data` for algorithm attributes with ECC
-    fn ecc_algo_attrs(
+    fn ecc_algo_attrs(oid: &[u8], ecc_type: EccType) -> Result<Vec<u8>, Error> {
-        oid: &[u8],
-        ecc_type: EccType,
-    ) -> Result<Vec<u8>, Error> {
         let algo_id = match ecc_type {
             EccType::EdDSA => 0x16,
             EccType::ECDH => 0x12,
@@ -282,11 +265,7 @@ pub struct EccAttrs {
 }
 
 impl EccAttrs {
-    pub fn new(
+    pub fn new(ecc_type: EccType, curve: Curve, import_format: Option<u8>) -> Self {
-        ecc_type: EccType,
-        curve: Curve,
-        import_format: Option<u8>,
-    ) -> Self {
         Self {
             ecc_type,
             curve,
@@ -335,20 +314,12 @@ impl Curve {
             NistP256r1 => &[0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01, 0x07],
             NistP384r1 => &[0x2B, 0x81, 0x04, 0x00, 0x22],
             NistP521r1 => &[0x2B, 0x81, 0x04, 0x00, 0x23],
-            BrainpoolP256r1 => {
+            BrainpoolP256r1 => &[0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x07],
-                &[0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x07]
+            BrainpoolP384r1 => &[0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x0b],
-            }
+            BrainpoolP512r1 => &[0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x0d],
-            BrainpoolP384r1 => {
-                &[0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x0b]
-            }
-            BrainpoolP512r1 => {
-                &[0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x0d]
-            }
             Secp256k1 => &[0x2B, 0x81, 0x04, 0x00, 0x0A],
             Ed25519 => &[0x2B, 0x06, 0x01, 0x04, 0x01, 0xDA, 0x47, 0x0F, 0x01],
-            Cv25519 => {
+            Cv25519 => &[0x2b, 0x06, 0x01, 0x04, 0x01, 0x97, 0x55, 0x01, 0x05, 0x01],
-                &[0x2b, 0x06, 0x01, 0x04, 0x01, 0x97, 0x55, 0x01, 0x05, 0x01]
-            }
             Ed448 => &[0x2b, 0x65, 0x71],
             X448 => &[0x2b, 0x65, 0x6f],
         }
@@ -366,22 +337,14 @@ impl TryFrom<&[u8]> for Curve {
             [0x2B, 0x81, 0x04, 0x00, 0x22] => NistP384r1,
             [0x2B, 0x81, 0x04, 0x00, 0x23] => NistP521r1,
 
-            [0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x07] => {
+            [0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x07] => BrainpoolP256r1,
-                BrainpoolP256r1
+            [0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x0b] => BrainpoolP384r1,
-            }
+            [0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x0d] => BrainpoolP512r1,
-            [0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x0b] => {
-                BrainpoolP384r1
-            }
-            [0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x0d] => {
-                BrainpoolP512r1
-            }
 
             [0x2B, 0x81, 0x04, 0x00, 0x0A] => Secp256k1,
 
             [0x2B, 0x06, 0x01, 0x04, 0x01, 0xDA, 0x47, 0x0F, 0x01] => Ed25519,
-            [0x2b, 0x06, 0x01, 0x04, 0x01, 0x97, 0x55, 0x01, 0x05, 0x01] => {
+            [0x2b, 0x06, 0x01, 0x04, 0x01, 0x97, 0x55, 0x01, 0x05, 0x01] => Cv25519,
-                Cv25519
-            }
 
             [0x2b, 0x65, 0x71] => Ed448,
             [0x2b, 0x65, 0x6f] => X448,

openpgp-card/src/apdu.rs

@@ -41,11 +41,7 @@ where
     )?)?;
 
     if let StatusBytes::UnknownStatus(0x6c, size) = resp.status() {
-        resp = RawResponse::try_from(send_command_low_level(
+        resp = RawResponse::try_from(send_command_low_level(card_tx, cmd, Expect::Short(size))?)?;
-            card_tx,
-            cmd,
-            Expect::Short(size),
-        )?)?;
     }
 
     while let StatusBytes::OkBytesAvailable(bytes) = resp.status() {
@@ -61,10 +57,7 @@ where
 
         match next.status() {
             StatusBytes::OkBytesAvailable(_) | StatusBytes::Ok => {
-                log::debug!(
+                log::debug!(" appending {} bytes to response", next.raw_data().len());
-                    " appending {} bytes to response",
-                    next.raw_data().len()
-                );
 
                 // Append new data to resp.data and overwrite status.
                 resp.raw_mut_data().extend_from_slice(next.raw_data());
@@ -158,8 +151,7 @@ where
             let last = i == chunks.len() - 1;
 
             let cla = if last { 0x00 } else { 0x10 };
-            let partial =
+            let partial = Command::new(cla, cmd.ins(), cmd.p1(), cmd.p2(), d.to_vec());
-                Command::new(cla, cmd.ins(), cmd.p1(), cmd.p2(), d.to_vec());
 
             let serialized = partial
                 .serialize(ext_len, expect_response)
@@ -184,8 +176,7 @@ where
 
                 // ISO: "If SW1-SW2 is set to '6883', then the last
                 // command of the chain is expected."
-                if !(status == StatusBytes::Ok
+                if !(status == StatusBytes::Ok || status == StatusBytes::LastCommandOfChainExpected)
-                    || status == StatusBytes::LastCommandOfChainExpected)
                 {
                     // Unexpected status for a non-final chunked response
                     return Err(status.into());

openpgp-card/src/apdu/command.rs

@@ -69,11 +69,7 @@ impl Command {
     /// Serialize a Command (for sending to a card).
     ///
     /// See OpenPGP card spec, chapter 7 (pg 47)
-    pub(crate) fn serialize(
+    pub(crate) fn serialize(&self, ext_len: bool, expect_response: Expect) -> Result<Vec<u8>> {
-        &self,
-        ext_len: bool,
-        expect_response: Expect,
-    ) -> Result<Vec<u8>> {
         // FIXME? (from scd/apdu.c):
         //  T=0 does not allow the use of Lc together with Le;
         //  thus disable Le in this case.

openpgp-card/src/card_do.rs

@@ -60,9 +60,7 @@ impl ApplicationRelatedData {
 
     /// Get extended length information (ISO 7816-4), which
     /// contains maximum number of bytes for command and response.
-    pub fn extended_length_information(
+    pub fn extended_length_information(&self) -> Result<Option<ExtendedLengthInfo>> {
-        &self,
-    ) -> Result<Option<ExtendedLengthInfo>> {
         // get from cached "application related data"
         let eli = self.0.find(&[0x7f, 0x66].into());
 
@@ -88,9 +86,7 @@ impl ApplicationRelatedData {
     }
 
     /// Get extended Capabilities
-    pub fn extended_capabilities(
+    pub fn extended_capabilities(&self) -> Result<ExtendedCapabilities, Error> {
-        &self,
-    ) -> Result<ExtendedCapabilities, Error> {
         // FIXME: caching?
         let app_id = self.application_id()?;
         let version = app_id.version();
@@ -157,14 +153,11 @@ impl ApplicationRelatedData {
     }
 
     /// Generation dates/times of key pairs
-    pub fn key_generation_times(
+    pub fn key_generation_times(&self) -> Result<KeySet<KeyGenerationTime>, Error> {
-        &self,
-    ) -> Result<KeySet<KeyGenerationTime>, Error> {
         let kg = self.0.find(&[0xcd].into());
 
         if let Some(kg) = kg {
-            let kg: KeySet<KeyGenerationTime> =
+            let kg: KeySet<KeyGenerationTime> = (&kg.serialize()[..]).try_into()?;
-                (&kg.serialize()[..]).try_into()?;
 
             log::debug!("Key generation: {:x?}", kg);
 
@@ -269,9 +262,9 @@ pub struct ExtendedCapabilities {
     max_cmd_len: Option<u16>,  // v2
     max_resp_len: Option<u16>, // v2
 
-    max_len_special_do: Option<u16>, // v3
+    max_len_special_do: Option<u16>,          // v3
     pin_block_2_format_support: Option<bool>, // v3
-    mse_command_support: Option<bool>, // v3
+    mse_command_support: Option<bool>,        // v3
 }
 
 /// 4.1.3.1 Extended length information
@@ -453,11 +446,8 @@ impl Fingerprint {
 }
 
 /// Helper fn for nom parsing
-pub(crate) fn complete<O>(
+pub(crate) fn complete<O>(result: nom::IResult<&[u8], O>) -> Result<O, anyhow::Error> {
-    result: nom::IResult<&[u8], O>,
+    let (rem, output) = result.map_err(|err| anyhow!("Parsing failed: {:?}", err))?;
-) -> Result<O, anyhow::Error> {
-    let (rem, output) =
-        result.map_err(|err| anyhow!("Parsing failed: {:?}", err))?;
     if rem.is_empty() {
         Ok(output)
     } else {

openpgp-card/src/card_do/algo_attrs.rs

@@ -104,8 +104,7 @@ fn parse_ecdh(input: &[u8]) -> nom::IResult<&[u8], Algo> {
     let (input, _) = bytes::tag([0x12])(input)?;
     let (input, curve) = parse_oid(input)?;
 
-    let (input, import_format) =
+    let (input, import_format) = alt((parse_import_format, default_import_format))(input)?;
-        alt((parse_import_format, default_import_format))(input)?;
 
     Ok((
         input,
@@ -117,8 +116,7 @@ fn parse_ecdsa(input: &[u8]) -> nom::IResult<&[u8], Algo> {
     let (input, _) = bytes::tag([0x13])(input)?;
     let (input, curve) = parse_oid(input)?;
 
-    let (input, import_format) =
+    let (input, import_format) = alt((parse_import_format, default_import_format))(input)?;
-        alt((parse_import_format, default_import_format))(input)?;
 
     Ok((
         input,
@@ -130,8 +128,7 @@ fn parse_eddsa(input: &[u8]) -> nom::IResult<&[u8], Algo> {
     let (input, _) = bytes::tag([0x16])(input)?;
     let (input, curve) = parse_oid(input)?;
 
-    let (input, import_format) =
+    let (input, import_format) = alt((parse_import_format, default_import_format))(input)?;
-        alt((parse_import_format, default_import_format))(input)?;
 
     Ok((
         input,

openpgp-card/src/card_do/algo_info.rs

@@ -67,11 +67,8 @@ fn parse_list(input: &[u8]) -> nom::IResult<&[u8], Vec<(KeyType, Algo)>> {
 }
 
 fn parse_tl_list(input: &[u8]) -> nom::IResult<&[u8], Vec<(KeyType, Algo)>> {
-    let (input, (_, _, list)) = sequence::tuple((
+    let (input, (_, _, list)) =
-        bytes::tag([0xfa]),
+        sequence::tuple((bytes::tag([0xfa]), crate::tlv::length::length, parse_list))(input)?;
-        crate::tlv::length::length,
-        parse_list,
-    ))(input)?;
 
     Ok((input, list))
 }
@@ -110,18 +107,15 @@ mod test {
     #[test]
     fn test_gnuk() {
         let data = [
-            0xc1, 0x6, 0x1, 0x8, 0x0, 0x0, 0x20, 0x0, 0xc1, 0x6, 0x1, 0x10,
+            0xc1, 0x6, 0x1, 0x8, 0x0, 0x0, 0x20, 0x0, 0xc1, 0x6, 0x1, 0x10, 0x0, 0x0, 0x20, 0x0,
-            0x0, 0x0, 0x20, 0x0, 0xc1, 0x9, 0x13, 0x2a, 0x86, 0x48, 0xce,
+            0xc1, 0x9, 0x13, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x3, 0x1, 0x7, 0xc1, 0x6, 0x13, 0x2b,
-            0x3d, 0x3, 0x1, 0x7, 0xc1, 0x6, 0x13, 0x2b, 0x81, 0x4, 0x0, 0xa,
+            0x81, 0x4, 0x0, 0xa, 0xc1, 0xa, 0x16, 0x2b, 0x6, 0x1, 0x4, 0x1, 0xda, 0x47, 0xf, 0x1,
-            0xc1, 0xa, 0x16, 0x2b, 0x6, 0x1, 0x4, 0x1, 0xda, 0x47, 0xf, 0x1,
+            0xc2, 0x6, 0x1, 0x8, 0x0, 0x0, 0x20, 0x0, 0xc2, 0x6, 0x1, 0x10, 0x0, 0x0, 0x20, 0x0,
-            0xc2, 0x6, 0x1, 0x8, 0x0, 0x0, 0x20, 0x0, 0xc2, 0x6, 0x1, 0x10,
+            0xc2, 0x9, 0x13, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x3, 0x1, 0x7, 0xc2, 0x6, 0x13, 0x2b,
-            0x0, 0x0, 0x20, 0x0, 0xc2, 0x9, 0x13, 0x2a, 0x86, 0x48, 0xce,
+            0x81, 0x4, 0x0, 0xa, 0xc2, 0xb, 0x12, 0x2b, 0x6, 0x1, 0x4, 0x1, 0x97, 0x55, 0x1, 0x5,
-            0x3d, 0x3, 0x1, 0x7, 0xc2, 0x6, 0x13, 0x2b, 0x81, 0x4, 0x0, 0xa,
+            0x1, 0xc3, 0x6, 0x1, 0x8, 0x0, 0x0, 0x20, 0x0, 0xc3, 0x6, 0x1, 0x10, 0x0, 0x0, 0x20,
-            0xc2, 0xb, 0x12, 0x2b, 0x6, 0x1, 0x4, 0x1, 0x97, 0x55, 0x1, 0x5,
+            0x0, 0xc3, 0x9, 0x13, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x3, 0x1, 0x7, 0xc3, 0x6, 0x13,
-            0x1, 0xc3, 0x6, 0x1, 0x8, 0x0, 0x0, 0x20, 0x0, 0xc3, 0x6, 0x1,
+            0x2b, 0x81, 0x4, 0x0, 0xa, 0xc3, 0xa, 0x16, 0x2b, 0x6, 0x1, 0x4, 0x1, 0xda, 0x47, 0xf,
-            0x10, 0x0, 0x0, 0x20, 0x0, 0xc3, 0x9, 0x13, 0x2a, 0x86, 0x48,
-            0xce, 0x3d, 0x3, 0x1, 0x7, 0xc3, 0x6, 0x13, 0x2b, 0x81, 0x4, 0x0,
-            0xa, 0xc3, 0xa, 0x16, 0x2b, 0x6, 0x1, 0x4, 0x1, 0xda, 0x47, 0xf,
             0x1,
         ];
 
@@ -152,29 +146,23 @@ mod test {
     #[test]
     fn test_opgp_card_34() {
         let data = [
-            0xc1, 0x6, 0x1, 0x8, 0x0, 0x0, 0x20, 0x0, 0xc1, 0x6, 0x1, 0xc,
+            0xc1, 0x6, 0x1, 0x8, 0x0, 0x0, 0x20, 0x0, 0xc1, 0x6, 0x1, 0xc, 0x0, 0x0, 0x20, 0x0,
-            0x0, 0x0, 0x20, 0x0, 0xc1, 0x6, 0x1, 0x10, 0x0, 0x0, 0x20, 0x0,
+            0xc1, 0x6, 0x1, 0x10, 0x0, 0x0, 0x20, 0x0, 0xc1, 0x9, 0x13, 0x2a, 0x86, 0x48, 0xce,
-            0xc1, 0x9, 0x13, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x3, 0x1, 0x7,
+            0x3d, 0x3, 0x1, 0x7, 0xc1, 0x6, 0x13, 0x2b, 0x81, 0x4, 0x0, 0x22, 0xc1, 0x6, 0x13,
-            0xc1, 0x6, 0x13, 0x2b, 0x81, 0x4, 0x0, 0x22, 0xc1, 0x6, 0x13,
+            0x2b, 0x81, 0x4, 0x0, 0x23, 0xc1, 0xa, 0x13, 0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1,
-            0x2b, 0x81, 0x4, 0x0, 0x23, 0xc1, 0xa, 0x13, 0x2b, 0x24, 0x3, 0x3,
+            0x7, 0xc1, 0xa, 0x13, 0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1, 0xb, 0xc1, 0xa, 0x13,
-            0x2, 0x8, 0x1, 0x1, 0x7, 0xc1, 0xa, 0x13, 0x2b, 0x24, 0x3, 0x3,
+            0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1, 0xd, 0xc2, 0x6, 0x1, 0x8, 0x0, 0x0, 0x20,
-            0x2, 0x8, 0x1, 0x1, 0xb, 0xc1, 0xa, 0x13, 0x2b, 0x24, 0x3, 0x3,
+            0x0, 0xc2, 0x6, 0x1, 0xc, 0x0, 0x0, 0x20, 0x0, 0xc2, 0x6, 0x1, 0x10, 0x0, 0x0, 0x20,
-            0x2, 0x8, 0x1, 0x1, 0xd, 0xc2, 0x6, 0x1, 0x8, 0x0, 0x0, 0x20, 0x0,
+            0x0, 0xc2, 0x9, 0x12, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x3, 0x1, 0x7, 0xc2, 0x6, 0x12,
-            0xc2, 0x6, 0x1, 0xc, 0x0, 0x0, 0x20, 0x0, 0xc2, 0x6, 0x1, 0x10,
+            0x2b, 0x81, 0x4, 0x0, 0x22, 0xc2, 0x6, 0x12, 0x2b, 0x81, 0x4, 0x0, 0x23, 0xc2, 0xa,
-            0x0, 0x0, 0x20, 0x0, 0xc2, 0x9, 0x12, 0x2a, 0x86, 0x48, 0xce,
+            0x12, 0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1, 0x7, 0xc2, 0xa, 0x12, 0x2b, 0x24, 0x3,
-            0x3d, 0x3, 0x1, 0x7, 0xc2, 0x6, 0x12, 0x2b, 0x81, 0x4, 0x0, 0x22,
+            0x3, 0x2, 0x8, 0x1, 0x1, 0xb, 0xc2, 0xa, 0x12, 0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1,
-            0xc2, 0x6, 0x12, 0x2b, 0x81, 0x4, 0x0, 0x23, 0xc2, 0xa, 0x12,
+            0x1, 0xd, 0xc3, 0x6, 0x1, 0x8, 0x0, 0x0, 0x20, 0x0, 0xc3, 0x6, 0x1, 0xc, 0x0, 0x0,
-            0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1, 0x7, 0xc2, 0xa, 0x12,
+            0x20, 0x0, 0xc3, 0x6, 0x1, 0x10, 0x0, 0x0, 0x20, 0x0, 0xc3, 0x9, 0x13, 0x2a, 0x86,
-            0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1, 0xb, 0xc2, 0xa, 0x12,
+            0x48, 0xce, 0x3d, 0x3, 0x1, 0x7, 0xc3, 0x6, 0x13, 0x2b, 0x81, 0x4, 0x0, 0x22, 0xc3,
-            0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1, 0xd, 0xc3, 0x6, 0x1,
+            0x6, 0x13, 0x2b, 0x81, 0x4, 0x0, 0x23, 0xc3, 0xa, 0x13, 0x2b, 0x24, 0x3, 0x3, 0x2, 0x8,
-            0x8, 0x0, 0x0, 0x20, 0x0, 0xc3, 0x6, 0x1, 0xc, 0x0, 0x0, 0x20,
+            0x1, 0x1, 0x7, 0xc3, 0xa, 0x13, 0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1, 0xb, 0xc3,
-            0x0, 0xc3, 0x6, 0x1, 0x10, 0x0, 0x0, 0x20, 0x0, 0xc3, 0x9, 0x13,
+            0xa, 0x13, 0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1, 0xd,
-            0x2a, 0x86, 0x48, 0xce, 0x3d, 0x3, 0x1, 0x7, 0xc3, 0x6, 0x13,
-            0x2b, 0x81, 0x4, 0x0, 0x22, 0xc3, 0x6, 0x13, 0x2b, 0x81, 0x4, 0x0,
-            0x23, 0xc3, 0xa, 0x13, 0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1,
-            0x7, 0xc3, 0xa, 0x13, 0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1,
-            0xb, 0xc3, 0xa, 0x13, 0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1,
-            0xd,
         ];
 
         let ai = AlgoInfo::try_from(&data[..]).unwrap();
@@ -225,47 +213,37 @@ mod test {
     #[test]
     fn test_yk5() {
         let data = [
-            0xfa, 0x82, 0x1, 0xe2, 0xc1, 0x6, 0x1, 0x8, 0x0, 0x0, 0x11, 0x0,
+            0xfa, 0x82, 0x1, 0xe2, 0xc1, 0x6, 0x1, 0x8, 0x0, 0x0, 0x11, 0x0, 0xc1, 0x6, 0x1, 0xc,
-            0xc1, 0x6, 0x1, 0xc, 0x0, 0x0, 0x11, 0x0, 0xc1, 0x6, 0x1, 0x10,
+            0x0, 0x0, 0x11, 0x0, 0xc1, 0x6, 0x1, 0x10, 0x0, 0x0, 0x11, 0x0, 0xc1, 0x9, 0x13, 0x2a,
-            0x0, 0x0, 0x11, 0x0, 0xc1, 0x9, 0x13, 0x2a, 0x86, 0x48, 0xce,
+            0x86, 0x48, 0xce, 0x3d, 0x3, 0x1, 0x7, 0xc1, 0x6, 0x13, 0x2b, 0x81, 0x4, 0x0, 0x22,
-            0x3d, 0x3, 0x1, 0x7, 0xc1, 0x6, 0x13, 0x2b, 0x81, 0x4, 0x0, 0x22,
+            0xc1, 0x6, 0x13, 0x2b, 0x81, 0x4, 0x0, 0x23, 0xc1, 0x6, 0x13, 0x2b, 0x81, 0x4, 0x0,
-            0xc1, 0x6, 0x13, 0x2b, 0x81, 0x4, 0x0, 0x23, 0xc1, 0x6, 0x13,
+            0xa, 0xc1, 0xa, 0x13, 0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1, 0x7, 0xc1, 0xa, 0x13,
-            0x2b, 0x81, 0x4, 0x0, 0xa, 0xc1, 0xa, 0x13, 0x2b, 0x24, 0x3, 0x3,
+            0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1, 0xb, 0xc1, 0xa, 0x13, 0x2b, 0x24, 0x3, 0x3,
-            0x2, 0x8, 0x1, 0x1, 0x7, 0xc1, 0xa, 0x13, 0x2b, 0x24, 0x3, 0x3,
+            0x2, 0x8, 0x1, 0x1, 0xd, 0xc1, 0xa, 0x16, 0x2b, 0x6, 0x1, 0x4, 0x1, 0xda, 0x47, 0xf,
-            0x2, 0x8, 0x1, 0x1, 0xb, 0xc1, 0xa, 0x13, 0x2b, 0x24, 0x3, 0x3,
+            0x1, 0xc1, 0xb, 0x16, 0x2b, 0x6, 0x1, 0x4, 0x1, 0x97, 0x55, 0x1, 0x5, 0x1, 0xc2, 0x6,
-            0x2, 0x8, 0x1, 0x1, 0xd, 0xc1, 0xa, 0x16, 0x2b, 0x6, 0x1, 0x4,
+            0x1, 0x8, 0x0, 0x0, 0x11, 0x0, 0xc2, 0x6, 0x1, 0xc, 0x0, 0x0, 0x11, 0x0, 0xc2, 0x6,
-            0x1, 0xda, 0x47, 0xf, 0x1, 0xc1, 0xb, 0x16, 0x2b, 0x6, 0x1, 0x4,
+            0x1, 0x10, 0x0, 0x0, 0x11, 0x0, 0xc2, 0x9, 0x12, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x3,
-            0x1, 0x97, 0x55, 0x1, 0x5, 0x1, 0xc2, 0x6, 0x1, 0x8, 0x0, 0x0,
+            0x1, 0x7, 0xc2, 0x6, 0x12, 0x2b, 0x81, 0x4, 0x0, 0x22, 0xc2, 0x6, 0x12, 0x2b, 0x81,
-            0x11, 0x0, 0xc2, 0x6, 0x1, 0xc, 0x0, 0x0, 0x11, 0x0, 0xc2, 0x6,
+            0x4, 0x0, 0x23, 0xc2, 0x6, 0x12, 0x2b, 0x81, 0x4, 0x0, 0xa, 0xc2, 0xa, 0x12, 0x2b,
-            0x1, 0x10, 0x0, 0x0, 0x11, 0x0, 0xc2, 0x9, 0x12, 0x2a, 0x86, 0x48,
+            0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1, 0x7, 0xc2, 0xa, 0x12, 0x2b, 0x24, 0x3, 0x3, 0x2,
-            0xce, 0x3d, 0x3, 0x1, 0x7, 0xc2, 0x6, 0x12, 0x2b, 0x81, 0x4, 0x0,
+            0x8, 0x1, 0x1, 0xb, 0xc2, 0xa, 0x12, 0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1, 0xd,
-            0x22, 0xc2, 0x6, 0x12, 0x2b, 0x81, 0x4, 0x0, 0x23, 0xc2, 0x6,
+            0xc2, 0xa, 0x16, 0x2b, 0x6, 0x1, 0x4, 0x1, 0xda, 0x47, 0xf, 0x1, 0xc2, 0xb, 0x16, 0x2b,
-            0x12, 0x2b, 0x81, 0x4, 0x0, 0xa, 0xc2, 0xa, 0x12, 0x2b, 0x24, 0x3,
+            0x6, 0x1, 0x4, 0x1, 0x97, 0x55, 0x1, 0x5, 0x1, 0xc3, 0x6, 0x1, 0x8, 0x0, 0x0, 0x11,
-            0x3, 0x2, 0x8, 0x1, 0x1, 0x7, 0xc2, 0xa, 0x12, 0x2b, 0x24, 0x3,
+            0x0, 0xc3, 0x6, 0x1, 0xc, 0x0, 0x0, 0x11, 0x0, 0xc3, 0x6, 0x1, 0x10, 0x0, 0x0, 0x11,
-            0x3, 0x2, 0x8, 0x1, 0x1, 0xb, 0xc2, 0xa, 0x12, 0x2b, 0x24, 0x3,
+            0x0, 0xc3, 0x9, 0x13, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x3, 0x1, 0x7, 0xc3, 0x6, 0x13,
-            0x3, 0x2, 0x8, 0x1, 0x1, 0xd, 0xc2, 0xa, 0x16, 0x2b, 0x6, 0x1,
+            0x2b, 0x81, 0x4, 0x0, 0x22, 0xc3, 0x6, 0x13, 0x2b, 0x81, 0x4, 0x0, 0x23, 0xc3, 0x6,
-            0x4, 0x1, 0xda, 0x47, 0xf, 0x1, 0xc2, 0xb, 0x16, 0x2b, 0x6, 0x1,
+            0x13, 0x2b, 0x81, 0x4, 0x0, 0xa, 0xc3, 0xa, 0x13, 0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1,
-            0x4, 0x1, 0x97, 0x55, 0x1, 0x5, 0x1, 0xc3, 0x6, 0x1, 0x8, 0x0,
+            0x1, 0x7, 0xc3, 0xa, 0x13, 0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1, 0xb, 0xc3, 0xa,
-            0x0, 0x11, 0x0, 0xc3, 0x6, 0x1, 0xc, 0x0, 0x0, 0x11, 0x0, 0xc3,
+            0x13, 0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1, 0xd, 0xc3, 0xa, 0x16, 0x2b, 0x6, 0x1,
-            0x6, 0x1, 0x10, 0x0, 0x0, 0x11, 0x0, 0xc3, 0x9, 0x13, 0x2a, 0x86,
+            0x4, 0x1, 0xda, 0x47, 0xf, 0x1, 0xc3, 0xb, 0x16, 0x2b, 0x6, 0x1, 0x4, 0x1, 0x97, 0x55,
-            0x48, 0xce, 0x3d, 0x3, 0x1, 0x7, 0xc3, 0x6, 0x13, 0x2b, 0x81, 0x4,
+            0x1, 0x5, 0x1, 0xda, 0x6, 0x1, 0x8, 0x0, 0x0, 0x11, 0x0, 0xda, 0x6, 0x1, 0xc, 0x0, 0x0,
-            0x0, 0x22, 0xc3, 0x6, 0x13, 0x2b, 0x81, 0x4, 0x0, 0x23, 0xc3, 0x6,
+            0x11, 0x0, 0xda, 0x6, 0x1, 0x10, 0x0, 0x0, 0x11, 0x0, 0xda, 0x9, 0x13, 0x2a, 0x86,
-            0x13, 0x2b, 0x81, 0x4, 0x0, 0xa, 0xc3, 0xa, 0x13, 0x2b, 0x24, 0x3,
+            0x48, 0xce, 0x3d, 0x3, 0x1, 0x7, 0xda, 0x6, 0x13, 0x2b, 0x81, 0x4, 0x0, 0x22, 0xda,
-            0x3, 0x2, 0x8, 0x1, 0x1, 0x7, 0xc3, 0xa, 0x13, 0x2b, 0x24, 0x3,
+            0x6, 0x13, 0x2b, 0x81, 0x4, 0x0, 0x23, 0xda, 0x6, 0x13, 0x2b, 0x81, 0x4, 0x0, 0xa,
-            0x3, 0x2, 0x8, 0x1, 0x1, 0xb, 0xc3, 0xa, 0x13, 0x2b, 0x24, 0x3,
+            0xda, 0xa, 0x13, 0x2b, 0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1, 0x7, 0xda, 0xa, 0x13, 0x2b,
-            0x3, 0x2, 0x8, 0x1, 0x1, 0xd, 0xc3, 0xa, 0x16, 0x2b, 0x6, 0x1,
+            0x24, 0x3, 0x3, 0x2, 0x8, 0x1, 0x1, 0xb, 0xda, 0xa, 0x13, 0x2b, 0x24, 0x3, 0x3, 0x2,
-            0x4, 0x1, 0xda, 0x47, 0xf, 0x1, 0xc3, 0xb, 0x16, 0x2b, 0x6, 0x1,
+            0x8, 0x1, 0x1, 0xd, 0xda, 0xa, 0x16, 0x2b, 0x6, 0x1, 0x4, 0x1, 0xda, 0x47, 0xf, 0x1,
-            0x4, 0x1, 0x97, 0x55, 0x1, 0x5, 0x1, 0xda, 0x6, 0x1, 0x8, 0x0,
+            0xda, 0xb, 0x16, 0x2b, 0x6, 0x1, 0x4, 0x1, 0x97, 0x55, 0x1, 0x5, 0x1,
-            0x0, 0x11, 0x0, 0xda, 0x6, 0x1, 0xc, 0x0, 0x0, 0x11, 0x0, 0xda,
-            0x6, 0x1, 0x10, 0x0, 0x0, 0x11, 0x0, 0xda, 0x9, 0x13, 0x2a, 0x86,
-            0x48, 0xce, 0x3d, 0x3, 0x1, 0x7, 0xda, 0x6, 0x13, 0x2b, 0x81, 0x4,
-            0x0, 0x22, 0xda, 0x6, 0x13, 0x2b, 0x81, 0x4, 0x0, 0x23, 0xda, 0x6,
-            0x13, 0x2b, 0x81, 0x4, 0x0, 0xa, 0xda, 0xa, 0x13, 0x2b, 0x24, 0x3,
-            0x3, 0x2, 0x8, 0x1, 0x1, 0x7, 0xda, 0xa, 0x13, 0x2b, 0x24, 0x3,
-            0x3, 0x2, 0x8, 0x1, 0x1, 0xb, 0xda, 0xa, 0x13, 0x2b, 0x24, 0x3,
-            0x3, 0x2, 0x8, 0x1, 0x1, 0xd, 0xda, 0xa, 0x16, 0x2b, 0x6, 0x1,
-            0x4, 0x1, 0xda, 0x47, 0xf, 0x1, 0xda, 0xb, 0x16, 0x2b, 0x6, 0x1,
-            0x4, 0x1, 0x97, 0x55, 0x1, 0x5, 0x1,
         ];
 
         let ai = AlgoInfo::try_from(&data[..]).unwrap();

openpgp-card/src/card_do/application_id.rs

@@ -17,8 +17,7 @@ fn parse(input: &[u8]) -> nom::IResult<&[u8], ApplicationIdentifier> {
     let (input, manufacturer) = number::be_u16(input)?;
     let (input, serial) = number::be_u32(input)?;
 
-    let (input, _) =
+    let (input, _) = nom::combinator::all_consuming(bytes::tag([0x0, 0x0]))(input)?;
-        nom::combinator::all_consuming(bytes::tag([0x0, 0x0]))(input)?;
 
     Ok((
         input,
@@ -76,12 +75,11 @@ mod test {
     #[test]
     fn test_gnuk() {
         let data = [
-            0xd2, 0x76, 0x0, 0x1, 0x24, 0x1, 0x2, 0x0, 0xff, 0xfe, 0x43, 0x19,
+            0xd2, 0x76, 0x0, 0x1, 0x24, 0x1, 0x2, 0x0, 0xff, 0xfe, 0x43, 0x19, 0x42, 0x40, 0x0, 0x0,
-            0x42, 0x40, 0x0, 0x0,
         ];
 
-        let aid = ApplicationIdentifier::try_from(&data[..])
+        let aid =
-            .expect("failed to parse application id");
+            ApplicationIdentifier::try_from(&data[..]).expect("failed to parse application id");
 
         assert_eq!(
             aid,

openpgp-card/src/card_do/cardholder.rs

@@ -31,20 +31,18 @@ impl TryFrom<&[u8]> for CardholderRelatedData {
         let value = Value::from(data, true)?;
         let tlv = Tlv::new([0x65], value);
 
-        let name: Option<Vec<u8>> =
+        let name: Option<Vec<u8>> = tlv.find(&[0x5b].into()).map(|v| v.serialize().to_vec());
-            tlv.find(&[0x5b].into()).map(|v| v.serialize().to_vec());
 
-        let lang: Option<Vec<Lang>> =
+        let lang: Option<Vec<Lang>> = tlv.find(&[0x5f, 0x2d].into()).map(|v| {
-            tlv.find(&[0x5f, 0x2d].into()).map(|v| {
+            v.serialize()
-                v.serialize()
+                .chunks(2)
-                    .chunks(2)
+                .map(|c| match c.len() {
-                    .map(|c| match c.len() {
+                    2 => Lang::from(&[c[0], c[1]]),
-                        2 => Lang::from(&[c[0], c[1]]),
+                    1 => Lang::from(&[c[0]]),
-                        1 => Lang::from(&[c[0]]),
+                    _ => unreachable!(),
-                        _ => unreachable!(),
+                })
-                    })
+                .collect()
-                    .collect()
+        });
-            });
 
         let sex = tlv
             .find(&[0x5f, 0x35].into())
@@ -63,12 +61,11 @@ mod test {
     #[test]
     fn test() {
         let data = [
-            0x5b, 0x8, 0x42, 0x61, 0x72, 0x3c, 0x3c, 0x46, 0x6f, 0x6f, 0x5f,
+            0x5b, 0x8, 0x42, 0x61, 0x72, 0x3c, 0x3c, 0x46, 0x6f, 0x6f, 0x5f, 0x2d, 0x4, 0x64, 0x65,
-            0x2d, 0x4, 0x64, 0x65, 0x65, 0x6e, 0x5f, 0x35, 0x1, 0x32,
+            0x65, 0x6e, 0x5f, 0x35, 0x1, 0x32,
         ];
 
-        let ch = CardholderRelatedData::try_from(&data[..])
+        let ch = CardholderRelatedData::try_from(&data[..]).expect("failed to parse cardholder");
-            .expect("failed to parse cardholder");
 
         assert_eq!(
             ch,

openpgp-card/src/card_do/extended_cap.rs

@@ -80,21 +80,15 @@ impl TryFrom<(&[u8], u16)> for ExtendedCapabilities {
             let i9 = input[9];
 
             if i8 > 1 {
-                return Err(anyhow!(
+                return Err(
-                    "Illegal value '{}' for pin_block_2_format_support",
+                    anyhow!("Illegal value '{}' for pin_block_2_format_support", i8).into(),
-                    i8
+                );
-                )
-                .into());
             }
 
             pin_block_2_format_support = Some(i8 != 0);
 
             if i9 > 1 {
-                return Err(anyhow!(
+                return Err(anyhow!("Illegal value '{}' for mse_command_support", i9).into());
-                    "Illegal value '{}' for mse_command_support",
-                    i9
-                )
-                .into());
             }
             mse_command_support = Some(i9 != 0);
         }

openpgp-card/src/card_do/extended_length_info.rs

@@ -11,13 +11,12 @@ use crate::card_do::{complete, ExtendedLengthInfo};
 use std::convert::TryFrom;
 
 fn parse(input: &[u8]) -> nom::IResult<&[u8], (u16, u16)> {
-    let (input, (_, cmd, _, resp)) =
+    let (input, (_, cmd, _, resp)) = nom::combinator::all_consuming(sequence::tuple((
-        nom::combinator::all_consuming(sequence::tuple((
+        tag([0x2, 0x2]),
-            tag([0x2, 0x2]),
+        number::be_u16,
-            number::be_u16,
+        tag([0x2, 0x2]),
-            tag([0x2, 0x2]),
+        number::be_u16,
-            number::be_u16,
+    )))(input)?;
-        )))(input)?;
 
     Ok((input, (cmd, resp)))
 }
@@ -53,8 +52,8 @@ mod test {
     fn test_floss34() {
         let data = [0x2, 0x2, 0x8, 0x0, 0x2, 0x2, 0x8, 0x0];
 
-        let eli = ExtendedLengthInfo::try_from(&data[..])
+        let eli =
-            .expect("failed to parse extended length info");
+            ExtendedLengthInfo::try_from(&data[..]).expect("failed to parse extended length info");
 
         assert_eq!(
             eli,

openpgp-card/src/card_do/fingerprint.rs

@@ -22,18 +22,13 @@ impl TryFrom<&[u8]> for Fingerprint {
     type Error = Error;
 
     fn try_from(input: &[u8]) -> Result<Self, Self::Error> {
-        log::trace!(
+        log::trace!("Fingerprint from input: {:x?}, len {}", input, input.len());
-            "Fingerprint from input: {:x?}, len {}",
-            input,
-            input.len()
-        );
 
         if input.len() == 20 {
             let array: [u8; 20] = input.try_into().unwrap();
             Ok(array.into())
         } else {
-            Err(anyhow!("Unexpected fingerprint length {}", input.len())
+            Err(anyhow!("Unexpected fingerprint length {}", input.len()).into())
-                .into())
         }
     }
 }
@@ -80,20 +75,14 @@ fn fingerprint(input: &[u8]) -> nom::IResult<&[u8], Option<Fingerprint>> {
 }
 
 fn fingerprints(input: &[u8]) -> nom::IResult<&[u8], KeySet<Fingerprint>> {
-    combinator::into(sequence::tuple((fingerprint, fingerprint, fingerprint)))(
+    combinator::into(sequence::tuple((fingerprint, fingerprint, fingerprint)))(input)
-        input,
-    )
 }
 
 impl TryFrom<&[u8]> for KeySet<Fingerprint> {
     type Error = Error;
 
     fn try_from(input: &[u8]) -> Result<Self, Self::Error> {
-        log::trace!(
+        log::trace!("Fingerprint from input: {:x?}, len {}", input, input.len());
-            "Fingerprint from input: {:x?}, len {}",
-            input,
-            input.len()
-        );
 
         // The input may be longer than 3 fingerprint, don't fail if it hasn't
         // been completely consumed.
@@ -111,12 +100,11 @@ mod test {
     #[test]
     fn test() {
         let data3 = [
-            0xb7, 0xcd, 0x9f, 0x76, 0x37, 0x1e, 0x7, 0x7f, 0x76, 0x1c, 0x82,
+            0xb7, 0xcd, 0x9f, 0x76, 0x37, 0x1e, 0x7, 0x7f, 0x76, 0x1c, 0x82, 0x65, 0x55, 0x54,
-            0x65, 0x55, 0x54, 0x3e, 0x6d, 0x65, 0x6d, 0x1d, 0x80, 0x62, 0xd7,
+            0x3e, 0x6d, 0x65, 0x6d, 0x1d, 0x80, 0x62, 0xd7, 0x34, 0x22, 0x65, 0xd2, 0xef, 0x33,
-            0x34, 0x22, 0x65, 0xd2, 0xef, 0x33, 0x64, 0xe3, 0x79, 0x52, 0xd9,
+            0x64, 0xe3, 0x79, 0x52, 0xd9, 0x5e, 0x94, 0x20, 0x5f, 0x4c, 0xce, 0x8b, 0x3f, 0x9,
-            0x5e, 0x94, 0x20, 0x5f, 0x4c, 0xce, 0x8b, 0x3f, 0x9, 0x7a, 0xf2,
+            0x7a, 0xf2, 0xfd, 0x76, 0xa5, 0xa7, 0x57, 0x9b, 0x51, 0x1f, 0xf, 0x44, 0x9a, 0x25,
-            0xfd, 0x76, 0xa5, 0xa7, 0x57, 0x9b, 0x51, 0x1f, 0xf, 0x44, 0x9a,
+            0x80, 0x2d, 0xb2, 0xb8,
-            0x25, 0x80, 0x2d, 0xb2, 0xb8,
         ];
 
         let fp_set: KeySet<Fingerprint> = (&data3[..])
@@ -137,12 +125,11 @@ mod test {
         );
 
         let data1 = [
-            0xb7, 0xcd, 0x9f, 0x76, 0x37, 0x1e, 0x7, 0x7f, 0x76, 0x1c, 0x82,
+            0xb7, 0xcd, 0x9f, 0x76, 0x37, 0x1e, 0x7, 0x7f, 0x76, 0x1c, 0x82, 0x65, 0x55, 0x54,
-            0x65, 0x55, 0x54, 0x3e, 0x6d, 0x65, 0x6d, 0x1d, 0x80,
+            0x3e, 0x6d, 0x65, 0x6d, 0x1d, 0x80,
         ];
 
-        let fp = Fingerprint::try_from(&data1[..])
+        let fp = Fingerprint::try_from(&data1[..]).expect("failed to parse fingerprint");
-            .expect("failed to parse fingerprint");
 
         assert_eq!(
             format!("{}", fp),

openpgp-card/src/card_do/historical.rs

@@ -44,8 +44,7 @@ impl From<u8> for CardServiceData {
         let select_by_partial_df_name = data & 0x40 != 0;
         let dos_available_in_ef_dir = data & 0x20 != 0;
         let dos_available_in_ef_atr_info = data & 0x10 != 0;
-        let access_services =
+        let access_services = [data & 0x8 != 0, data & 0x4 != 0, data & 0x2 != 0];
-            [data & 0x8 != 0, data & 0x4 != 0, data & 0x2 != 0];
         let mf = data & 0x1 != 0;
 
         Self {
@@ -107,10 +106,7 @@ impl TryFrom<&[u8]> for HistoricalBytes {
             // The OpenPGP application assumes a category indicator byte
             // set to '00' (o-card 3.4.1, pg 44)
 
-            return Err(anyhow!(
+            return Err(anyhow!("Unexpected category indicator in historical bytes").into());
-                "Unexpected category indicator in historical bytes"
-            )
-            .into());
         }
 
         // category indicator byte
@@ -155,11 +151,7 @@ impl TryFrom<&[u8]> for HistoricalBytes {
 
                     // (e.g. yubikey neo returns historical bytes as:
                     // "[0, 73, 0, 0, 80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]")
-                    log::trace!(
+                    log::trace!("historical bytes: ignored (tag {}, len {})", t, l);
-                        "historical bytes: ignored (tag {}, len {})",
-                        t,
-                        l
-                    );
                     ctlv.drain(0..(l as usize + 1));
                 }
             }
@@ -185,10 +177,7 @@ impl TryFrom<&[u8]> for HistoricalBytes {
                 5
             }
             _ => {
-                return Err(anyhow!(
+                return Err(anyhow!("unexpected status indicator in historical bytes").into());
-                    "unexpected status indicator in historical bytes"
-                )
-                .into());
             }
         };
 
@@ -221,8 +210,7 @@ mod test {
     #[test]
     fn test_gnuk() -> Result<()> {
         // gnuk 1.2 stable
-        let data: &[u8] =
+        let data: &[u8] = &[0x0, 0x31, 0x84, 0x73, 0x80, 0x1, 0x80, 0x5, 0x90, 0x0];
-            &[0x0, 0x31, 0x84, 0x73, 0x80, 0x1, 0x80, 0x5, 0x90, 0x0];
         let hist: HistoricalBytes = data.try_into()?;
 
         assert_eq!(
@@ -252,8 +240,7 @@ mod test {
     #[test]
     fn test_floss34() -> Result<()> {
         // floss shop openpgp smartcard 3.4
-        let data: &[u8] =
+        let data: &[u8] = &[0x0, 0x31, 0xf5, 0x73, 0xc0, 0x1, 0x60, 0x5, 0x90, 0x0];
-            &[0x0, 0x31, 0xf5, 0x73, 0xc0, 0x1, 0x60, 0x5, 0x90, 0x0];
         let hist: HistoricalBytes = data.try_into()?;
 
         assert_eq!(
@@ -330,8 +317,7 @@ mod test {
     fn test_yk_neo() -> Result<()> {
         // yubikey neo
         let data: &[u8] = &[
-            0x0, 0x73, 0x0, 0x0, 0x80, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
+            0x0, 0x73, 0x0, 0x0, 0x80, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
-            0x0, 0x0,
         ];
         let hist: HistoricalBytes = data.try_into()?;
 
@@ -355,8 +341,7 @@ mod test {
     #[test]
     fn test_ledger_nano_s() -> Result<()> {
         let data: &[u8] = &[
-            0x0, 0x31, 0xc5, 0x73, 0xc0, 0x1, 0x80, 0x7, 0x90, 0x0, 0x0, 0x0,
+            0x0, 0x31, 0xc5, 0x73, 0xc0, 0x1, 0x80, 0x7, 0x90, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
-            0x0, 0x0, 0x0,
         ];
         let hist: HistoricalBytes = data.try_into()?;
 

openpgp-card/src/card_do/key_generation_times.rs

@@ -35,18 +35,14 @@ fn gen_time(input: &[u8]) -> nom::IResult<&[u8], u32> {
     (number::be_u32)(input)
 }
 
-fn key_generation(
+fn key_generation(input: &[u8]) -> nom::IResult<&[u8], Option<KeyGenerationTime>> {
-    input: &[u8],
-) -> nom::IResult<&[u8], Option<KeyGenerationTime>> {
     combinator::map(gen_time, |kg| match kg {
         0 => None,
         kg => Some(KeyGenerationTime(kg)),
     })(input)
 }
 
-fn key_generation_set(
+fn key_generation_set(input: &[u8]) -> nom::IResult<&[u8], KeySet<KeyGenerationTime>> {
-    input: &[u8],
-) -> nom::IResult<&[u8], KeySet<KeyGenerationTime>> {
     combinator::into(sequence::tuple((
         key_generation,
         key_generation,
@@ -86,8 +82,7 @@ mod test {
     #[test]
     fn test() {
         let data3 = [
-            0x60, 0xf3, 0xff, 0x71, 0x60, 0xf3, 0xff, 0x72, 0x60, 0xf3, 0xff,
+            0x60, 0xf3, 0xff, 0x71, 0x60, 0xf3, 0xff, 0x72, 0x60, 0xf3, 0xff, 0x83,
-            0x83,
         ];
 
         let fp_set: KeySet<KeyGenerationTime> = (&data3[..])

openpgp-card/src/card_do/pw_status.rs

@@ -84,8 +84,7 @@ mod test {
     fn test() {
         let data = [0x0, 0x40, 0x40, 0x40, 0x3, 0x0, 0x3];
 
-        let pws: PWStatusBytes =
+        let pws: PWStatusBytes = (&data[..]).try_into().expect("failed to parse PWStatus");
-            (&data[..]).try_into().expect("failed to parse PWStatus");
 
         assert_eq!(
             pws,

openpgp-card/src/crypto_data.rs

@@ -26,15 +26,9 @@ impl Hash<'_> {
     /// digestinfo for SHA*. Other OIDs are not implemented.
     pub(crate) fn oid(&self) -> Option<&'static [u8]> {
         match self {
-            Self::SHA256(_) => {
+            Self::SHA256(_) => Some(&[0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01]),
-                Some(&[0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01])
+            Self::SHA384(_) => Some(&[0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x02]),
-            }
+            Self::SHA512(_) => Some(&[0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03]),
-            Self::SHA384(_) => {
-                Some(&[0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x02])
-            }
-            Self::SHA512(_) => {
-                Some(&[0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03])
-            }
             Self::EdDSA(_) => panic!("OIDs for EdDSA are unimplemented"),
             Self::ECDSA(_) => panic!("OIDs for ECDSA are unimplemented"),
         }

openpgp-card/src/errors.rs

@@ -129,9 +129,7 @@ impl From<(u8, u8)> for StatusBytes {
             (0x61, bytes) => StatusBytes::OkBytesAvailable(bytes),
 
             (0x62, 0x85) => StatusBytes::TerminationState,
-            (0x63, 0xC0..=0xCF) => {
+            (0x63, 0xC0..=0xCF) => StatusBytes::PasswordNotChecked(status.1 & 0xf),
-                StatusBytes::PasswordNotChecked(status.1 & 0xf)
-            }
             (0x64, 0x02..=0x80) => StatusBytes::TriggeringByCard(status.1),
             (0x65, 0x01) => StatusBytes::MemoryFailure,
             (0x66, 0x00) => StatusBytes::SecurityRelatedIssues,

openpgp-card/src/keys.rs

@@ -12,8 +12,8 @@ use crate::apdu::command::Command;
 use crate::apdu::commands;
 use crate::card_do::{ApplicationRelatedData, Fingerprint, KeyGenerationTime};
 use crate::crypto_data::{
-    CardUploadableKey, EccKey, EccPub, EccType, PrivateKeyMaterial,
+    CardUploadableKey, EccKey, EccPub, EccType, PrivateKeyMaterial, PublicKeyMaterial, RSAKey,
-    PublicKeyMaterial, RSAKey, RSAPub,
+    RSAPub,
 };
 use crate::tlv::{length::tlv_encode_length, value::Value, Tlv};
 use crate::{apdu, KeyType};
@@ -31,11 +31,7 @@ use crate::{CardTransaction, Error};
 /// creation timestamp
 pub(crate) fn gen_key_with_metadata<C>(
     card_tx: &mut C,
-    fp_from_pub: fn(
+    fp_from_pub: fn(&PublicKeyMaterial, KeyGenerationTime, KeyType) -> Result<Fingerprint, Error>,
-        &PublicKeyMaterial,
-        KeyGenerationTime,
-        KeyType,
-    ) -> Result<Fingerprint, Error>,
     key_type: KeyType,
     algo: Option<&Algo>,
 ) -> Result<(PublicKeyMaterial, KeyGenerationTime), Error>
@@ -154,10 +150,7 @@ where
 /// in the card or imported")
 ///
 /// (See 7.2.14 GENERATE ASYMMETRIC KEY PAIR)
-pub(crate) fn public_key<C>(
+pub(crate) fn public_key<C>(card_tx: &mut C, key_type: KeyType) -> Result<PublicKeyMaterial, Error>
-    card_tx: &mut C,
-    key_type: KeyType,
-) -> Result<PublicKeyMaterial, Error>
 where
     C: CardTransaction + ?Sized,
 {
@@ -201,20 +194,15 @@ where
             // (round up to 4-bytes, in case the key has 8+ leading zero bits)
             let rsa_bits = (((rsa_key.n().len() * 8 + 31) / 32) * 32) as u16;
 
-            let rsa_attrs =
+            let rsa_attrs = determine_rsa_attrs(rsa_bits, key_type, &ard, algo_info)?;
-                determine_rsa_attrs(rsa_bits, key_type, &ard, algo_info)?;
 
             let key_cmd = rsa_key_import_cmd(key_type, rsa_key, &rsa_attrs)?;
 
             (Algo::Rsa(rsa_attrs), key_cmd)
         }
         PrivateKeyMaterial::E(ecc_key) => {
-            let ecc_attrs = determine_ecc_attrs(
+            let ecc_attrs =
-                ecc_key.oid(),
+                determine_ecc_attrs(ecc_key.oid(), ecc_key.ecc_type(), key_type, algo_info)?;
-                ecc_key.ecc_type(),
-                key_type,
-                algo_info,
-            )?;
 
             let key_cmd = ecc_key_import_cmd(key_type, ecc_key, &ecc_attrs)?;
 
@@ -300,10 +288,7 @@ pub(crate) fn determine_ecc_attrs(
         let algos = check_card_algo_ecc(algo_info, key_type, oid);
         if algos.is_empty() {
             // If oid is not in algo_info, return error.
-            return Err(anyhow!(
+            return Err(anyhow!("Oid {:?} unsupported according to algo_info", oid));
-                "Oid {:?} unsupported according to algo_info",
-                oid
-            ));
         }
 
         // Note: Looking up ecc_type in the card's "Algorithm Information"
@@ -330,11 +315,7 @@ pub(crate) fn determine_ecc_attrs(
 }
 
 /// Look up RsaAttrs parameters in algo_info based on key_type and rsa_bits
-fn card_algo_rsa(
+fn card_algo_rsa(algo_info: AlgoInfo, key_type: KeyType, rsa_bits: u16) -> Result<RsaAttrs, Error> {
-    algo_info: AlgoInfo,
-    key_type: KeyType,
-    rsa_bits: u16,
-) -> Result<RsaAttrs, Error> {
     // Find suitable algorithm parameters (from card's list of algorithms).
 
     // Get Algos for this keytype
@@ -360,20 +341,12 @@ fn card_algo_rsa(
         Ok((**algo.last().unwrap()).clone())
     } else {
         // RSA with this bit length is not in algo_info
-        return Err(anyhow!(
+        return Err(anyhow!("RSA {} unsupported according to algo_info", rsa_bits).into());
-            "RSA {} unsupported according to algo_info",
-            rsa_bits
-        )
-        .into());
     }
 }
 
 /// Get all entries from algo_info with matching `oid` and `key_type`.
-fn check_card_algo_ecc(
+fn check_card_algo_ecc(algo_info: AlgoInfo, key_type: KeyType, oid: &[u8]) -> Vec<EccAttrs> {
-    algo_info: AlgoInfo,
-    key_type: KeyType,
-    oid: &[u8],
-) -> Vec<EccAttrs> {
     // Find suitable algorithm parameters (from card's list of algorithms).
 
     // Get Algos for this keytype

openpgp-card/src/lib.rs

@@ -45,21 +45,17 @@ use crate::algorithm::{Algo, AlgoInfo, AlgoSimple};
 use crate::apdu::commands;
 use crate::apdu::response::RawResponse;
 use crate::card_do::{
-    ApplicationRelatedData, CardholderRelatedData, Fingerprint,
+    ApplicationRelatedData, CardholderRelatedData, Fingerprint, KeyGenerationTime, Lang,
-    KeyGenerationTime, Lang, PWStatusBytes, SecuritySupportTemplate, Sex,
+    PWStatusBytes, SecuritySupportTemplate, Sex,
-};
-use crate::crypto_data::{
-    CardUploadableKey, Cryptogram, Hash, PublicKeyMaterial,
 };
+use crate::crypto_data::{CardUploadableKey, Cryptogram, Hash, PublicKeyMaterial};
 use crate::tlv::tag::Tag;
 use crate::tlv::value::Value;
 use crate::tlv::Tlv;
 
 #[blanket::blanket(derive(Box))]
 pub trait CardBackend {
-    fn transaction(
+    fn transaction(&mut self) -> Result<Box<dyn CardTransaction + Send + Sync + '_>, Error>;
-        &mut self,
-    ) -> Result<Box<dyn CardTransaction + Send + Sync + '_>, Error>;
 }
 
 /// The CardTransaction trait defines communication with an OpenPGP card via a
@@ -74,11 +70,7 @@ pub trait CardTransaction {
     ///
     /// `buf_size` is a hint to the backend (the backend may ignore it)
     /// indicating the expected maximum response size.
-    fn transmit(
+    fn transmit(&mut self, cmd: &[u8], buf_size: usize) -> Result<Vec<u8>, Error>;
-        &mut self,
-        cmd: &[u8],
-        buf_size: usize,
-    ) -> Result<Vec<u8>, Error>;
 
     /// Set the card capabilities in the CardTransaction.
     ///
@@ -150,9 +142,7 @@ pub trait CardTransaction {
             if let Ok(Some(eli)) = ard.extended_length_information() {
                 // In card 3.x, max lengths come from ExtendedLengthInfo
                 (eli.max_command_bytes(), eli.max_response_bytes())
-            } else if let (Some(cmd), Some(rsp)) =
+            } else if let (Some(cmd), Some(rsp)) = (ext_cap.max_cmd_len(), ext_cap.max_resp_len()) {
-                (ext_cap.max_cmd_len(), ext_cap.max_resp_len())
-            {
                 // In card 2.x, max lengths come from ExtendedCapabilities
                 (cmd, rsp)
             } else {
@@ -246,17 +236,15 @@ pub trait CardTransaction {
     }
 
     /// Get security support template (7a)
-    fn security_support_template(
+    fn security_support_template(&mut self) -> Result<SecuritySupportTemplate> {
-        &mut self,
-    ) -> Result<SecuritySupportTemplate> {
         let sst = commands::security_support_template();
         let resp = apdu::send_command(self, sst, true)?;
         resp.check_ok()?;
 
         let tlv = Tlv::try_from(resp.data()?)?;
-        let res = tlv.find(&[0x93].into()).ok_or_else(|| {
+        let res = tlv
-            anyhow!("Couldn't get SecuritySupportTemplate DO")
+            .find(&[0x93].into())
-        })?;
+            .ok_or_else(|| anyhow!("Couldn't get SecuritySupportTemplate DO"))?;
 
         if let Value::S(data) = res {
             let mut data = data.to_vec();
@@ -293,8 +281,7 @@ pub trait CardTransaction {
 
     /// Firmware Version (YubiKey specific (?))
     fn firmware_version(&mut self) -> Result<Vec<u8>> {
-        let resp =
+        let resp = apdu::send_command(self, commands::firmware_version(), true)?;
-            apdu::send_command(self, commands::firmware_version(), true)?;
 
         Ok(resp.data()?.into())
     }
@@ -350,11 +337,7 @@ pub trait CardTransaction {
     /// Access condition:
     /// - 1/3 need PW1 (82)
     /// - 2/4 need PW3
-    fn set_private_use_do(
+    fn set_private_use_do(&mut self, num: u8, data: Vec<u8>) -> Result<Vec<u8>> {
-        &mut self,
-        num: u8,
-        data: Vec<u8>,
-    ) -> Result<Vec<u8>> {
         assert!((1..=4).contains(&num));
 
         let cmd = commands::put_private_use_do(num, data);
@@ -428,10 +411,7 @@ pub trait CardTransaction {
     /// Depending on the PW1 status byte (see Extended Capabilities) this
     /// access condition is only valid for one PSO:CDS command or remains
     /// valid for several attempts.
-    fn verify_pw1_for_signing(
+    fn verify_pw1_for_signing(&mut self, pin: &str) -> Result<Response, Error> {
-        &mut self,
-        pin: &str,
-    ) -> Result<Response, Error> {
         let verify = commands::verify_pw1_81(pin.as_bytes().to_vec());
         apdu::send_command(self, verify, false)?.try_into()
     }
@@ -638,10 +618,7 @@ pub trait CardTransaction {
     ///
     /// (consider using the `signature_for_hash()` method if you don't
     /// want to create the data field manually)
-    fn pso_compute_digital_signature(
+    fn pso_compute_digital_signature(&mut self, data: Vec<u8>) -> Result<Vec<u8>, Error> {
-        &mut self,
-        data: Vec<u8>,
-    ) -> Result<Vec<u8>, Error> {
         let cds_cmd = commands::signature(data);
 
         let resp = apdu::send_command(self, cds_cmd, true)?;
@@ -670,10 +647,7 @@ pub trait CardTransaction {
     ///
     /// (consider using the `authenticate_for_hash()` method if you don't
     /// want to create the data field manually)
-    fn internal_authenticate(
+    fn internal_authenticate(&mut self, data: Vec<u8>) -> Result<Vec<u8>, Error> {
-        &mut self,
-        data: Vec<u8>,
-    ) -> Result<Vec<u8>, Error> {
         let ia_cmd = commands::internal_authenticate(data);
         let resp = apdu::send_command(self, ia_cmd, true)?;
 
@@ -722,21 +696,13 @@ pub trait CardTransaction {
             .copied()
             .collect();
 
-        let time_cmd =
+        let time_cmd = commands::put_data(&[key_type.timestamp_put_tag()], time_value);
-            commands::put_data(&[key_type.timestamp_put_tag()], time_value);
 
         apdu::send_command(self, time_cmd, false)?.try_into()
     }
 
-    fn set_fingerprint(
+    fn set_fingerprint(&mut self, fp: Fingerprint, key_type: KeyType) -> Result<Response, Error> {
-        &mut self,
+        let fp_cmd = commands::put_data(&[key_type.fingerprint_put_tag()], fp.as_bytes().to_vec());
-        fp: Fingerprint,
-        key_type: KeyType,
-    ) -> Result<Response, Error> {
-        let fp_cmd = commands::put_data(
-            &[key_type.fingerprint_put_tag()],
-            fp.as_bytes().to_vec(),
-        );
 
         apdu::send_command(self, fp_cmd, false)?.try_into()
     }
@@ -767,10 +733,7 @@ pub trait CardTransaction {
     ///
     /// Call select_data() before calling this fn, to select a particular
     /// certificate (if the card supports multiple certificates).
-    fn set_cardholder_certificate(
+    fn set_cardholder_certificate(&mut self, data: Vec<u8>) -> Result<Response, Error> {
-        &mut self,
-        data: Vec<u8>,
-    ) -> Result<Response, Error> {
         let cmd = commands::put_cardholder_certificate(data);
         apdu::send_command(self, cmd, false)?.try_into()
     }
@@ -783,20 +746,14 @@ pub trait CardTransaction {
         algo: &Algo,
     ) -> Result<Response, Error> {
         // Command to PUT the algorithm attributes
-        let cmd = commands::put_data(
+        let cmd = commands::put_data(&[key_type.algorithm_tag()], algo.to_data_object()?);
-            &[key_type.algorithm_tag()],
-            algo.to_data_object()?,
-        );
 
         apdu::send_command(self, cmd, false)?.try_into()
     }
 
     /// Set resetting code
     /// (4.3.4 Resetting Code)
-    fn set_resetting_code(
+    fn set_resetting_code(&mut self, resetting_code: Vec<u8>) -> Result<Response, Error> {
-        &mut self,
-        resetting_code: Vec<u8>,
-    ) -> Result<Response, Error> {
         let cmd = commands::put_data(&[0xd3], resetting_code);
         apdu::send_command(self, cmd, false)?.try_into()
     }
@@ -878,10 +835,7 @@ pub trait CardTransaction {
     /// Note also that the information from the card is insufficient to
     /// reconstruct a pre-existing OpenPGP public key that corresponds to
     /// the private key on the card.
-    fn public_key(
+    fn public_key(&mut self, key_type: KeyType) -> Result<PublicKeyMaterial, Error> {
-        &mut self,
-        key_type: KeyType,
-    ) -> Result<PublicKeyMaterial, Error> {
         keys::public_key(self, key_type)
     }
 }

openpgp-card/src/tlv.rs

@@ -61,8 +61,7 @@ impl Tlv {
 
         // Read the length field and get the corresponding number of bytes,
         // which contain the value of this tlv
-        let (input, value) =
+        let (input, value) = combinator::flat_map(length::length, bytes::take)(input)?;
-            combinator::flat_map(length::length, bytes::take)(input)?;
 
         // Parse the value bytes, as "simple" or "constructed", depending
         // on the tag.
@@ -99,34 +98,24 @@ mod test {
 
         assert_eq!(
             cpkt.serialize(),
-            vec![
+            vec![0x7F, 0x48, 0x0A, 0x91, 0x03, 0x92, 0x82, 0x01, 0x00, 0x93, 0x82, 0x01, 0x00,]
-                0x7F, 0x48, 0x0A, 0x91, 0x03, 0x92, 0x82, 0x01, 0x00, 0x93,
-                0x82, 0x01, 0x00,
-            ]
         );
     }
     #[test]
     fn test_tlv() -> Result<()> {
         // From OpenPGP card spec § 7.2.6
-        let data =
+        let data = hex!("5B0B546573743C3C54657374695F2D0264655F350131").to_vec();
-            hex!("5B0B546573743C3C54657374695F2D0264655F350131").to_vec();
 
         let (input, tlv) = Tlv::parse(&data).unwrap();
 
         assert_eq!(
             tlv,
-            Tlv::new(
+            Tlv::new([0x5b], Value::S(hex!("546573743C3C5465737469").to_vec()))
-                [0x5b],
-                Value::S(hex!("546573743C3C5465737469").to_vec())
-            )
         );
 
         let (input, tlv) = Tlv::parse(input).unwrap();
 
-        assert_eq!(
+        assert_eq!(tlv, Tlv::new([0x5f, 0x2d], Value::S(hex!("6465").to_vec())));
-            tlv,
-            Tlv::new([0x5f, 0x2d], Value::S(hex!("6465").to_vec()))
-        );
 
         let (input, tlv) = Tlv::parse(input).unwrap();
 
@@ -157,10 +146,7 @@ mod test {
         assert_eq!(value.serialize(), hex!("7d000bfe080000ff0000"));
 
         let value = tlv.find(&[0x4f].into()).unwrap();
-        assert_eq!(
+        assert_eq!(value.serialize(), hex!("d2760001240103040006160191800000"));
-            value.serialize(),
-            hex!("d2760001240103040006160191800000")
-        );
 
         let value = tlv.find(&[0x5f, 0x52].into()).unwrap();
         assert_eq!(value.serialize(), hex!("00730000e0059000"));
@@ -196,10 +182,7 @@ mod test {
         assert_eq!(value.serialize(), hex!("0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"));
 
         let value = tlv.find(&[0xcd].into()).unwrap();
-        assert_eq!(
+        assert_eq!(value.serialize(), hex!("00000000000000000000000000000000"));
-            value.serialize(),
-            hex!("00000000000000000000000000000000")
-        );
 
         let value = tlv.find(&[0xde].into()).unwrap();
         assert_eq!(value.serialize(), hex!("0100020003008102"));
@@ -236,10 +219,7 @@ mod test {
 
         assert_eq!(
             tlv.serialize(),
-            &[
+            &[0x4d, 0xb, 0x7f, 0x48, 0x2, 0x92, 0x3, 0x5f, 0x48, 0x3, 0x1, 0x2, 0x3]
-                0x4d, 0xb, 0x7f, 0x48, 0x2, 0x92, 0x3, 0x5f, 0x48, 0x3, 0x1,
-                0x2, 0x3
-            ]
         );
     }
 }

openpgp-card/src/tlv/length.rs

@@ -14,10 +14,7 @@ pub(crate) fn tlv_encode_length(len: u16) -> Vec<u8> {
     }
 }
 
-use nom::{
+use nom::{branch, bytes::complete as bytes, combinator, number::complete as number, sequence};
-    branch, bytes::complete as bytes, combinator, number::complete as number,
-    sequence,
-};
 
 fn length1(input: &[u8]) -> nom::IResult<&[u8], u8> {
     combinator::verify(number::u8, |&c| c < 0x80)(input)

openpgp-card/src/tlv/tag.rs

@@ -3,10 +3,7 @@
 
 //! Tag in a TLV data structure
 
-use nom::{
+use nom::{branch, bytes::complete as bytes, combinator, number::complete as number, sequence};
-    branch, bytes::complete as bytes, combinator, number::complete as number,
-    sequence,
-};
 
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct Tag(Vec<u8>);
@@ -44,10 +41,7 @@ impl From<[u8; 2]> for Tag {
 }
 
 fn multi_byte_tag(input: &[u8]) -> nom::IResult<&[u8], &[u8]> {
-    combinator::recognize(sequence::pair(
+    combinator::recognize(sequence::pair(multi_byte_tag_first, multi_byte_tag_rest))(input)
-        multi_byte_tag_first,
-        multi_byte_tag_rest,
-    ))(input)
 }
 
 fn multi_byte_tag_first(input: &[u8]) -> nom::IResult<&[u8], u8> {
@@ -91,9 +85,7 @@ fn single_byte_tag(input: &[u8]) -> nom::IResult<&[u8], &[u8]> {
 }
 
 pub(super) fn tag(input: &[u8]) -> nom::IResult<&[u8], Tag> {
-    combinator::map(branch::alt((multi_byte_tag, single_byte_tag)), Tag::from)(
+    combinator::map(branch::alt((multi_byte_tag, single_byte_tag)), Tag::from)(input)
-        input,
-    )
 }
 
 #[cfg(test)]

openpgp-card/src/tlv/value.rs

@@ -19,10 +19,7 @@ pub enum Value {
 }
 
 impl Value {
-    pub(crate) fn parse(
+    pub(crate) fn parse(data: &[u8], constructed: bool) -> nom::IResult<&[u8], Self> {
-        data: &[u8],
-        constructed: bool,
-    ) -> nom::IResult<&[u8], Self> {
         match constructed {
             false => Ok((&[], Value::S(data.to_vec()))),
             true => {

pcsc/src/lib.rs

@@ -12,9 +12,7 @@ use std::collections::HashMap;
 use std::convert::TryInto;
 
 use openpgp_card::card_do::ApplicationRelatedData;
-use openpgp_card::{
+use openpgp_card::{CardBackend, CardCaps, CardTransaction, Error, SmartcardError};
-    CardBackend, CardCaps, CardTransaction, Error, SmartcardError,
-};
 
 const FEATURE_VERIFY_PIN_DIRECT: u8 = 0x06;
 const FEATURE_MODIFY_PIN_DIRECT: u8 = 0x07;
@@ -53,7 +51,7 @@ pub struct PcscBackend {
 /// <https://docs.microsoft.com/en-us/windows/win32/api/winscard/nf-winscard-scardbegintransaction?redirectedfrom=MSDN#remarks>)
 pub struct PcscTransaction<'b> {
     tx: Transaction<'b>,
-    card_caps: Option<CardCaps>, // FIXME: manual copy from PcscCard
+    card_caps: Option<CardCaps>,   // FIXME: manual copy from PcscCard
     reader_caps: HashMap<u8, Tlv>, // FIXME: manual copy from PcscCard
 }
 
@@ -120,17 +118,13 @@ impl<'b> PcscTransaction<'b> {
                     log::debug!("start_tx: do reconnect");
 
                     {
-                        c.reconnect(
+                        c.reconnect(mode, Protocols::ANY, Disposition::ResetCard)
-                            mode,
+                            .map_err(|e| {
-                            Protocols::ANY,
+                                Error::Smartcard(SmartcardError::Error(format!(
-                            Disposition::ResetCard,
+                                    "Reconnect failed: {:?}",
-                        )
+                                    e
-                        .map_err(|e| {
+                                )))
-                            Error::Smartcard(SmartcardError::Error(format!(
+                            })?;
-                                "Reconnect failed: {:?}",
-                                e
-                            )))
-                        })?;
                     }
 
                     log::debug!("start_tx: reconnected.");
@@ -139,10 +133,9 @@ impl<'b> PcscTransaction<'b> {
                 }
                 Err((_, e)) => {
                     log::debug!("start_tx: error {:?}", e);
-                    break Err(Error::Smartcard(SmartcardError::Error(
+                    break Err(
-                        format!("Error: {:?}", e),
+                        Error::Smartcard(SmartcardError::Error(format!("Error: {:?}", e))).into(),
-                    ))
+                    );
-                    .into());
                 }
             };
         }
@@ -211,25 +204,16 @@ impl<'b> PcscTransaction<'b> {
 }
 
 impl CardTransaction for PcscTransaction<'_> {
-    fn transmit(
+    fn transmit(&mut self, cmd: &[u8], buf_size: usize) -> Result<Vec<u8>, Error> {
-        &mut self,
-        cmd: &[u8],
-        buf_size: usize,
-    ) -> Result<Vec<u8>, Error> {
         let mut resp_buffer = vec![0; buf_size];
 
-        let resp =
+        let resp = self
-            self.tx
+            .tx
-                .transmit(cmd, &mut resp_buffer)
+            .transmit(cmd, &mut resp_buffer)
-                .map_err(|e| match e {
+            .map_err(|e| match e {
-                    pcsc::Error::NotTransacted => {
+                pcsc::Error::NotTransacted => Error::Smartcard(SmartcardError::NotTransacted),
-                        Error::Smartcard(SmartcardError::NotTransacted)
+                _ => Error::Smartcard(SmartcardError::Error(format!("Transmit failed: {:?}", e))),
-                    }
+            })?;
-                    _ => Error::Smartcard(SmartcardError::Error(format!(
-                        "Transmit failed: {:?}",
-                        e
-                    ))),
-                })?;
 
         log::debug!(" <- APDU response: {:x?}", resp);
 
@@ -332,11 +316,9 @@ impl CardTransaction for PcscTransaction<'_> {
             .value()
             .try_into()?;
 
-        let res = self.tx.control(
+        let res = self
-            u32::from_be_bytes(verify_ioctl).into(),
+            .tx
-            &send,
+            .control(u32::from_be_bytes(verify_ioctl).into(), &send, &mut recv)?;
-            &mut recv,
-        )?;
 
         log::debug!(" <- pcsc pinpad_verify result: {:x?}", res);
 
@@ -433,11 +415,9 @@ impl CardTransaction for PcscTransaction<'_> {
             .value()
             .try_into()?;
 
-        let res = self.tx.control(
+        let res = self
-            u32::from_be_bytes(modify_ioctl).into(),
+            .tx
-            &send,
+            .control(u32::from_be_bytes(modify_ioctl).into(), &send, &mut recv)?;
-            &mut recv,
-        )?;
 
         log::debug!(" <- pcsc pinpad_modify result: {:x?}", res);
 
@@ -501,10 +481,7 @@ impl PcscBackend {
                     continue; // try next reader
                 }
                 Err(err) => {
-                    log::warn!(
+                    log::warn!("Error connecting to card in reader: {:x?}", err);
-                        "Error connecting to card in reader: {:x?}",
-                        err
-                    );
 
                     continue;
                 }
@@ -526,15 +503,10 @@ impl PcscBackend {
     /// application identity with `ident` (if `ident` is None, all Cards are
     /// returned). Returns fully initialized PcscCard structs for all matching
     /// cards.
-    fn cards_filter(
+    fn cards_filter(ident: Option<&str>, mode: ShareMode) -> Result<Vec<Self>, Error> {
-        ident: Option<&str>,
-        mode: ShareMode,
-    ) -> Result<Vec<Self>, Error> {
         let mut cards: Vec<Self> = vec![];
 
-        for mut card in
+        for mut card in Self::raw_pcsc_cards(mode).map_err(|sce| Error::Smartcard(sce))? {
-            Self::raw_pcsc_cards(mode).map_err(|sce| Error::Smartcard(sce))?
-        {
             log::debug!("cards_filter: next card");
             log::debug!(" status: {:x?}", card.status2_owned());
 
@@ -553,11 +525,7 @@ impl PcscBackend {
                         log::debug!(" status: {:x?}", txc.tx.status2_owned());
 
                         if let Some(ident) = ident {
-                            if let Ok(ard) =
+                            if let Ok(ard) = PcscTransaction::application_related_data(&mut txc) {
-                                PcscTransaction::application_related_data(
-                                    &mut txc,
-                                )
-                            {
                                 let aid = ard.application_id()?;
 
                                 if aid.ident() == ident.to_ascii_uppercase() {
@@ -567,10 +535,7 @@ impl PcscBackend {
                                     // we want to return this one card
                                     store_card = true;
                                 } else {
-                                    log::debug!(
+                                    log::debug!(" won't use {:?}", aid.ident());
-                                        " won't use {:?}",
-                                        aid.ident()
-                                    );
                                 }
                             } else {
                                 // couldn't read ARD for this card.
@@ -610,10 +575,7 @@ impl PcscBackend {
     /// Returns the OpenPGP card that matches `ident`, if it is available.
     /// A fully initialized PcscCard is returned: the OpenPGP application has
     /// been selected, card_caps and reader_caps have been initialized.
-    pub fn open_by_ident(
+    pub fn open_by_ident(ident: &str, mode: Option<ShareMode>) -> Result<Self, Error> {
-        ident: &str,
-        mode: Option<ShareMode>,
-    ) -> Result<Self, Error> {
         log::debug!("open_by_ident for {:?}", ident);
 
         let mut cards = Self::cards_filter(Some(ident), default_mode(mode))?;
@@ -682,9 +644,7 @@ impl PcscBackend {
 
 impl CardBackend for PcscBackend {
     /// Get a TxClient for this PcscCard (this starts a transaction)
-    fn transaction(
+    fn transaction(&mut self) -> Result<Box<dyn CardTransaction + Send + Sync + '_>, Error> {
-        &mut self,
-    ) -> Result<Box<dyn CardTransaction + Send + Sync + '_>, Error> {
         Ok(Box::new(PcscTransaction::new(self, true)?))
     }
 }

scdc/src/lib.rs

@@ -16,8 +16,7 @@ use tokio::runtime::Runtime;
 use openpgp_card::{CardBackend, CardCaps, CardTransaction, Error};
 
 lazy_static! {
-    static ref RT: Mutex<Runtime> =
+    static ref RT: Mutex<Runtime> = Mutex::new(tokio::runtime::Runtime::new().unwrap());
-        Mutex::new(tokio::runtime::Runtime::new().unwrap());
 }
 
 /// The Assuan protocol (in GnuPG) limits the length of commands.
@@ -58,10 +57,7 @@ pub struct ScdBackend {
 impl ScdBackend {
     /// Open a CardApp that uses an scdaemon instance as its backend.
     /// The specific card with AID `serial` is requested from scdaemon.
-    pub fn open_by_serial(
+    pub fn open_by_serial(agent: Option<Agent>, serial: &str) -> Result<Self, Error> {
-        agent: Option<Agent>,
-        serial: &str,
-    ) -> Result<Self, Error> {
         let mut card = ScdBackend::new(agent, true)?;
         card.select_card(serial)?;
 
@@ -200,9 +196,7 @@ impl ScdBackend {
 }
 
 impl CardBackend for ScdBackend {
-    fn transaction(
+    fn transaction(&mut self) -> Result<Box<dyn CardTransaction + Send + Sync + '_>, Error> {
-        &mut self,
-    ) -> Result<Box<dyn CardTransaction + Send + Sync + '_>, Error> {
         Ok(Box::new(ScdTransaction { scd: self }))
     }
 }
@@ -218,9 +212,7 @@ impl CardTransaction for ScdTransaction<'_> {
         let hex = hex::encode(cmd);
 
         // (Unwrap is ok here, not having a card_caps is fine)
-        let ext = if self.card_caps().is_some()
+        let ext = if self.card_caps().is_some() && self.card_caps().unwrap().ext_support() {
-            && self.card_caps().unwrap().ext_support()
-        {
             // If we know about card_caps, and can do extended length we
             // set "exlen" accordingly ...
             format!("--exlen={} ", self.card_caps().unwrap().max_rsp_bytes())

tools/src/bin/opgpcard-pin/main.rs

@@ -27,21 +27,15 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         cli::Command::SetUserPin {} => {
             let res = if !pinpad_modify {
                 // get current user pin
-                let pin = rpassword::read_password_from_tty(Some(
+                let pin = rpassword::read_password_from_tty(Some("Enter user PIN: "))?;
-                    "Enter user PIN: ",
-                ))?;
 
                 // verify pin
                 open.verify_user(&pin)?;
                 println!("PIN was accepted by the card.\n");
 
                 // get new user pin
-                let newpin1 = rpassword::read_password_from_tty(Some(
+                let newpin1 = rpassword::read_password_from_tty(Some("Enter new user PIN: "))?;
-                    "Enter new user PIN: ",
+                let newpin2 = rpassword::read_password_from_tty(Some("Repeat the new user PIN: "))?;
-                ))?;
-                let newpin2 = rpassword::read_password_from_tty(Some(
-                    "Repeat the new user PIN: ",
-                ))?;
 
                 if newpin1 != newpin2 {
                     return Err(anyhow::anyhow!("PINs do not match.").into());
@@ -71,20 +65,15 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         cli::Command::SetAdminPin {} => {
             if !pinpad_modify {
                 // get current admin pin
-                let pin = rpassword::read_password_from_tty(Some(
+                let pin = rpassword::read_password_from_tty(Some("Enter admin PIN: "))?;
-                    "Enter admin PIN: ",
-                ))?;
 
                 // verify pin
                 open.verify_admin(&pin)?;
 
                 // get new admin pin
-                let newpin1 = rpassword::read_password_from_tty(Some(
+                let newpin1 = rpassword::read_password_from_tty(Some("Enter new admin PIN: "))?;
-                    "Enter new admin PIN: ",
+                let newpin2 =
-                ))?;
+                    rpassword::read_password_from_tty(Some("Repeat the new admin PIN: "))?;
-                let newpin2 = rpassword::read_password_from_tty(Some(
-                    "Repeat the new admin PIN: ",
-                ))?;
 
                 if newpin1 != newpin2 {
                     return Err(anyhow::anyhow!("PINs do not match.").into());
@@ -108,26 +97,20 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
             // verify admin pin
             if !pinpad_verify {
                 // get current admin pin
-                let pin = rpassword::read_password_from_tty(Some(
+                let pin = rpassword::read_password_from_tty(Some("Enter admin PIN: "))?;
-                    "Enter admin PIN: ",
-                ))?;
 
                 open.verify_admin(&pin)?;
             } else {
-                open.verify_admin_pinpad(&|| {
+                open.verify_admin_pinpad(&|| println!("Enter admin PIN on card reader pinpad."))?;
-                    println!("Enter admin PIN on card reader pinpad.")
-                })?;
             }
             println!("PIN was accepted by the card.\n");
 
             if let Some(mut admin) = open.admin_card() {
                 // ask user for new resetting code
-                let newpin1 = rpassword::read_password_from_tty(Some(
+                let newpin1 =
-                    "Enter new resetting code: ",
+                    rpassword::read_password_from_tty(Some("Enter new resetting code: "))?;
-                ))?;
+                let newpin2 =
-                let newpin2 = rpassword::read_password_from_tty(Some(
+                    rpassword::read_password_from_tty(Some("Repeat the new resetting code: "))?;
-                    "Repeat the new resetting code: ",
-                ))?;
 
                 if newpin1 == newpin2 {
                     admin.set_resetting_code(&newpin1)?;
@@ -135,10 +118,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
                     return Err(anyhow::anyhow!("PINs do not match.").into());
                 }
             } else {
-                return Err(anyhow::anyhow!(
+                return Err(anyhow::anyhow!("Failed to use card in admin-mode.").into());
-                    "Failed to use card in admin-mode."
-                )
-                .into());
             }
             println!("\nResetting code has been set.");
         }
@@ -148,9 +128,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
             let rst = if admin {
                 if !pinpad_verify {
                     // get current admin pin
-                    let pin = rpassword::read_password_from_tty(Some(
+                    let pin = rpassword::read_password_from_tty(Some("Enter admin PIN: "))?;
-                        "Enter admin PIN: ",
-                    ))?;
 
                     // verify pin
                     open.verify_admin(&pin)?;
@@ -164,9 +142,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
                 None
             } else {
                 // get resetting code
-                let rst = rpassword::read_password_from_tty(Some(
+                let rst = rpassword::read_password_from_tty(Some("Enter resetting code: "))?;
-                    "Enter resetting code: ",
-                ))?;
 
                 // NOTE: this code cannot be verified with the card!
 
@@ -174,12 +150,8 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
             };
 
             // get new user pin
-            let newpin1 = rpassword::read_password_from_tty(Some(
+            let newpin1 = rpassword::read_password_from_tty(Some("Enter new user PIN: "))?;
-                "Enter new user PIN: ",
+            let newpin2 = rpassword::read_password_from_tty(Some("Repeat the new user PIN: "))?;
-            ))?;
-            let newpin2 = rpassword::read_password_from_tty(Some(
-                "Repeat the new user PIN: ",
-            ))?;
 
             if newpin1 != newpin2 {
                 return Err(anyhow::anyhow!("PINs do not match.").into());
@@ -191,10 +163,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
             } else if let Some(mut admin) = open.admin_card() {
                 admin.reset_user_pin(&newpin1)
             } else {
-                return Err(anyhow::anyhow!(
+                return Err(anyhow::anyhow!("Failed to use card in admin-mode.").into());
-                    "Failed to use card in admin-mode."
-                )
-                .into());
             };
 
             if res.is_err() {
@@ -224,10 +193,7 @@ fn print_gnuk_note(err: Error, card: &Open) -> Result<()> {
     ) {
         // check if no keys exist on the card
         let fps = card.fingerprints()?;
-        if fps.signature() == None
+        if fps.signature() == None && fps.decryption() == None && fps.authentication() == None {
-            && fps.decryption() == None
-            && fps.authentication() == None
-        {
             println!(
                 "\nNOTE: Some cards (e.g. Gnuk) don't allow \
                         User PIN change while no keys exist on the card."

tools/src/bin/opgpcard/cli.rs

@@ -55,11 +55,7 @@ pub enum Command {
         #[structopt(name = "User PIN file", short = "p", long = "user-pin")]
         user_pin: Option<PathBuf>,
 
-        #[structopt(
+        #[structopt(name = "recipient-cert-file", short = "r", long = "recipient-cert")]
-            name = "recipient-cert-file",
-            short = "r",
-            long = "recipient-cert"
-        )]
         cert_file: PathBuf,
 
         #[structopt(about = "Input file (stdin if unset)", name = "input")]
@@ -75,11 +71,7 @@ pub enum Command {
         #[structopt(name = "detached", short = "d", long = "detached")]
         detached: bool,
 
-        #[structopt(
+        #[structopt(name = "signer-cert-file", short = "s", long = "signer-cert")]
-            name = "signer-cert-file",
-            short = "s",
-            long = "signer-cert"
-        )]
         cert_file: PathBuf,
 
         #[structopt(about = "Input file (stdin if unset)", name = "input")]
@@ -102,25 +94,13 @@ pub enum AdminCommand {
     Import {
         keyfile: PathBuf,
 
-        #[structopt(
+        #[structopt(name = "Signature key fingerprint", short = "s", long = "sig-fp")]
-            name = "Signature key fingerprint",
-            short = "s",
-            long = "sig-fp"
-        )]
         sig_fp: Option<String>,
 
-        #[structopt(
+        #[structopt(name = "Decryption key fingerprint", short = "d", long = "dec-fp")]
-            name = "Decryption key fingerprint",
-            short = "d",
-            long = "dec-fp"
-        )]
         dec_fp: Option<String>,
 
-        #[structopt(
+        #[structopt(name = "Authentication key fingerprint", short = "a", long = "auth-fp")]
-            name = "Authentication key fingerprint",
-            short = "a",
-            long = "auth-fp"
-        )]
         auth_fp: Option<String>,
     },
     /// Generate a Key.

tools/src/bin/opgpcard/main.rs

@@ -12,8 +12,8 @@ use sequoia_openpgp::serialize::SerializeInto;
 use sequoia_openpgp::Cert;
 
 use openpgp_card::algorithm::AlgoSimple;
-use openpgp_card::{card_do::Sex, KeyType};
+use openpgp_card::card_do::Sex;
-use openpgp_card::CardBackend;
+use openpgp_card::{CardBackend, KeyType};
 
 use openpgp_card_sequoia::card::{Admin, Open};
 use openpgp_card_sequoia::util::{make_cert, public_key_material_to_key};
@@ -57,10 +57,9 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
             if detached {
                 sign_detached(&ident, user_pin, &cert_file, input.as_deref())?;
             } else {
-                return Err(anyhow::anyhow!(
+                return Err(
-                    "Only detached signatures are supported for now"
+                    anyhow::anyhow!("Only detached signatures are supported for now").into(),
-                )
+                );
-                .into());
             }
         }
         cli::Command::FactoryReset { ident } => {
@@ -78,14 +77,12 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
 
             match cmd {
                 cli::AdminCommand::Name { name } => {
-                    let mut admin =
+                    let mut admin = util::verify_to_admin(&mut open, admin_pin)?;
-                        util::verify_to_admin(&mut open, admin_pin)?;
 
                     let _ = admin.set_name(&name)?;
                 }
                 cli::AdminCommand::Url { url } => {
-                    let mut admin =
+                    let mut admin = util::verify_to_admin(&mut open, admin_pin)?;
-                        util::verify_to_admin(&mut open, admin_pin)?;
 
                     let _ = admin.set_url(&url)?;
                 }
@@ -104,9 +101,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
                         // and if so, import these keys to the card.
                         key_import_yolo(admin, &key)?;
                     } else {
-                        key_import_explicit(
+                        key_import_explicit(admin, &key, sig_fp, dec_fp, auth_fp)?;
-                            admin, &key, sig_fp, dec_fp, auth_fp,
-                        )?;
                     }
                 }
                 cli::AdminCommand::Generate {
@@ -150,10 +145,7 @@ fn list_cards() -> Result<()> {
     Ok(())
 }
 
-fn set_identity(
+fn set_identity(ident: &str, id: u8) -> Result<(), Box<dyn std::error::Error>> {
-    ident: &str,
-    id: u8,
-) -> Result<(), Box<dyn std::error::Error>> {
     let mut card = util::open_card(ident)?;
     let mut txc = card.transaction()?;
 
@@ -313,8 +305,7 @@ fn print_status(ident: Option<String>, verbose: bool) -> Result<()> {
 
         // YubiKey specific (?) firmware version
         if let Ok(ver) = open.firmware_version() {
-            let ver =
+            let ver = ver.iter().map(u8::to_string).collect::<Vec<_>>().join(".");
-                ver.iter().map(u8::to_string).collect::<Vec<_>>().join(".");
 
             println!("Firmware Version: {}", ver);
         }
@@ -420,9 +411,7 @@ fn key_import_explicit(
     let p = StandardPolicy::new();
 
     if let Some(sig_fp) = sig_fp {
-        if let Some(sig) =
+        if let Some(sig) = sq_util::private_subkey_by_fingerprint(key, &p, &sig_fp)? {
-            sq_util::private_subkey_by_fingerprint(key, &p, &sig_fp)?
-        {
             println!("Uploading {} as signing key", sig.fingerprint());
             admin.upload_key(sig, KeyType::Signing, None)?;
         } else {
@@ -431,9 +420,7 @@ fn key_import_explicit(
     }
 
     if let Some(dec_fp) = dec_fp {
-        if let Some(dec) =
+        if let Some(dec) = sq_util::private_subkey_by_fingerprint(key, &p, &dec_fp)? {
-            sq_util::private_subkey_by_fingerprint(key, &p, &dec_fp)?
-        {
             println!("Uploading {} as decryption key", dec.fingerprint());
             admin.upload_key(dec, KeyType::Decryption, None)?;
         } else {
@@ -442,9 +429,7 @@ fn key_import_explicit(
     }
 
     if let Some(auth_fp) = auth_fp {
-        if let Some(auth) =
+        if let Some(auth) = sq_util::private_subkey_by_fingerprint(key, &p, &auth_fp)? {
-            sq_util::private_subkey_by_fingerprint(key, &p, &auth_fp)?
-        {
             println!("Uploading {} as authentication key", auth.fingerprint());
             admin.upload_key(auth, KeyType::Authentication, None)?;
         } else {
@@ -514,9 +499,7 @@ fn generate_keys(
                 your user PIN multiple times to make binding signatures."
             );
         } else {
-            return Err(anyhow!(
+            return Err(anyhow!("No user PIN file provided, and no pinpad found"));
-                "No user PIN file provided, and no pinpad found"
-            ));
         }
         None
     };
@@ -548,8 +531,7 @@ fn gen_subkeys(
     // the sig key
     let key_dec = if decrypt {
         println!(" Generate subkey for Decryption");
-        let (pkm, ts) =
+        let (pkm, ts) = admin.generate_key_simple(KeyType::Decryption, algo)?;
-            admin.generate_key_simple(KeyType::Decryption, algo)?;
         Some(public_key_material_to_key(&pkm, KeyType::Decryption, ts)?)
     } else {
         None
@@ -559,8 +541,7 @@ fn gen_subkeys(
     // algorithm as the sig key
     let key_aut = if auth {
         println!(" Generate subkey for Authentication");
-        let (pkm, ts) =
+        let (pkm, ts) = admin.generate_key_simple(KeyType::Authentication, algo)?;
-            admin.generate_key_simple(KeyType::Authentication, algo)?;
 
         Some(public_key_material_to_key(
             &pkm,

tools/src/bin/opgpcard/util.rs

@@ -11,8 +11,7 @@ use openpgp_card_pcsc::PcscBackend;
 use openpgp_card_sequoia::card::{Admin, Open, Sign, User};
 
 pub(crate) fn cards() -> Result<Vec<impl CardBackend>, Error> {
-    PcscBackend::cards(None)
+    PcscBackend::cards(None).map(|cards| cards.into_iter().collect())
-        .map(|cards| cards.into_iter().collect())
 }
 
 pub(crate) fn open_card(ident: &str) -> Result<impl CardBackend, Error> {
@@ -27,15 +26,10 @@ pub(crate) fn verify_to_user<'app, 'open>(
         open.verify_user(&load_pin(&path)?)?;
     } else {
         if !open.feature_pinpad_verify() {
-            return Err(anyhow!(
+            return Err(anyhow!("No user PIN file provided, and no pinpad found").into());
-                "No user PIN file provided, and no pinpad found"
-            )
-            .into());
         };
 
-        open.verify_user_pinpad(&|| {
+        open.verify_user_pinpad(&|| println!("Enter user PIN on card reader pinpad."))?;
-            println!("Enter user PIN on card reader pinpad.")
-        })?;
     }
 
     open.user_card()
@@ -50,14 +44,9 @@ pub(crate) fn verify_to_sign<'app, 'open>(
         open.verify_user_for_signing(&load_pin(&path)?)?;
     } else {
         if !open.feature_pinpad_verify() {
-            return Err(anyhow!(
+            return Err(anyhow!("No user PIN file provided, and no pinpad found").into());
-                "No user PIN file provided, and no pinpad found"
-            )
-            .into());
         }
-        open.verify_user_for_signing_pinpad(&|| {
+        open.verify_user_for_signing_pinpad(&|| println!("Enter user PIN on card reader pinpad."))?;
-            println!("Enter user PIN on card reader pinpad.")
-        })?;
     }
     open.signing_card()
         .ok_or_else(|| anyhow!("Couldn't get sign access").into())
@@ -73,15 +62,10 @@ pub(crate) fn verify_to_admin<'app, 'open>(
         open.verify_admin(&load_pin(&path)?)?;
     } else {
         if !open.feature_pinpad_verify() {
-            return Err(anyhow!(
+            return Err(anyhow!("No admin PIN file provided, and no pinpad found").into());
-                "No admin PIN file provided, and no pinpad found"
-            )
-            .into());
         }
 
-        open.verify_admin_pinpad(&|| {
+        open.verify_admin_pinpad(&|| println!("Enter admin PIN on card reader pinpad."))?;
-            println!("Enter admin PIN on card reader pinpad.")
-        })?;
     }
     open.admin_card()
         .ok_or_else(|| anyhow!("Couldn't get admin access").into())
@@ -92,9 +76,7 @@ pub(crate) fn load_pin(pin_file: &Path) -> Result<String> {
     Ok(pin.trim().to_string())
 }
 
-pub(crate) fn open_or_stdin(
+pub(crate) fn open_or_stdin(f: Option<&Path>) -> Result<Box<dyn std::io::Read + Send + Sync>> {
-    f: Option<&Path>,
-) -> Result<Box<dyn std::io::Read + Send + Sync>> {
     match f {
         Some(f) => Ok(Box::new(
             std::fs::File::open(f).context("Failed to open input file")?,
@@ -103,9 +85,7 @@ pub(crate) fn open_or_stdin(
     }
 }
 
-pub(crate) fn open_or_stdout(
+pub(crate) fn open_or_stdout(f: Option<&Path>) -> Result<Box<dyn std::io::Write + Send + Sync>> {
-    f: Option<&Path>,
-) -> Result<Box<dyn std::io::Write + Send + Sync>> {
     match f {
         Some(f) => Ok(Box::new(
             std::fs::File::create(f).context("Failed to open input file")?,
@@ -114,10 +94,7 @@ pub(crate) fn open_or_stdout(
     }
 }
 
-fn get_ssh_pubkey(
+fn get_ssh_pubkey(pkm: &PublicKeyMaterial, ident: String) -> Result<sshkeys::PublicKey> {
-    pkm: &PublicKeyMaterial,
-    ident: String,
-) -> Result<sshkeys::PublicKey> {
     let cardno = format!("cardno:{}", ident);
 
     let (key_type, kind) = match pkm {
@@ -135,15 +112,12 @@ fn get_ssh_pubkey(
             if let Algo::Ecc(ecc_attrs) = ecc.algo() {
                 match ecc_attrs.ecc_type() {
                     EccType::EdDSA => {
-                        let key_type =
+                        let key_type = sshkeys::KeyType::from_name("ssh-ed25519")?;
-                            sshkeys::KeyType::from_name("ssh-ed25519")?;
 
-                        let kind = sshkeys::PublicKeyKind::Ed25519(
+                        let kind = sshkeys::PublicKeyKind::Ed25519(sshkeys::Ed25519PublicKey {
-                            sshkeys::Ed25519PublicKey {
+                            key: ecc.data().to_vec(),
-                                key: ecc.data().to_vec(),
+                            sk_application: None,
-                                sk_application: None,
+                        });
-                            },
-                        );
 
                         Ok((key_type, kind))
                     }
@@ -161,28 +135,20 @@ fn get_ssh_pubkey(
                                 sshkeys::Curve::from_identifier("nistp521")?,
                                 "ecdsa-sha2-nistp521",
                             )),
-                            _ => Err(anyhow!(
+                            _ => Err(anyhow!("Unexpected ECDSA curve {:?}", ecc_attrs.curve())),
-                                "Unexpected ECDSA curve {:?}",
-                                ecc_attrs.curve()
-                            )),
                         }?;
 
                         let key_type = sshkeys::KeyType::from_name(name)?;
 
-                        let kind = sshkeys::PublicKeyKind::Ecdsa(
+                        let kind = sshkeys::PublicKeyKind::Ecdsa(sshkeys::EcdsaPublicKey {
-                            sshkeys::EcdsaPublicKey {
+                            curve,
-                                curve,
+                            key: ecc.data().to_vec(),
-                                key: ecc.data().to_vec(),
+                            sk_application: None,
-                                sk_application: None,
+                        });
-                            },
-                        );
 
                         Ok((key_type, kind))
                     }
-                    _ => Err(anyhow!(
+                    _ => Err(anyhow!("Unexpected EccType {:?}", ecc_attrs.ecc_type())),
-                        "Unexpected EccType {:?}",
-                        ecc_attrs.ecc_type()
-                    )),
                 }
             } else {
                 Err(anyhow!("Unexpected Algo in EccPub {:?}", ecc))
@@ -202,10 +168,7 @@ fn get_ssh_pubkey(
 
 /// Return a String representation of an ssh public key, in a form like:
 /// "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAuTuxILMTvzTIRvaRqqUM3aRDoEBgz/JAoWKsD1ECxy cardno:FFFE:43194240"
-pub(crate) fn get_ssh_pubkey_string(
+pub(crate) fn get_ssh_pubkey_string(pkm: &PublicKeyMaterial, ident: String) -> Result<String> {
-    pkm: &PublicKeyMaterial,
-    ident: String,
-) -> Result<String> {
     let pk = get_ssh_pubkey(pkm, ident)?;
 
     let mut v = vec![];