Merge pull request #97 from str4d/more-cleanups

More cleanups

src/builder.rs

@@ -134,7 +134,6 @@ impl IdentityBuilder {
             )],
         )?;
 
-        let (_, cert) = x509_parser::parse_x509_certificate(cert.as_ref()).unwrap();
         let metadata = Metadata::extract(yubikey, slot, &cert, false).unwrap();
 
         Ok((

src/key.rs

@@ -18,7 +18,7 @@ use yubikey::{
     certificate::Certificate,
     piv::{decrypt_data, AlgorithmId, RetiredSlotId, SlotId},
     reader::{Context, Reader},
-    MgmKey, PinPolicy, Serial, TouchPolicy, YubiKey,
+    Key, MgmKey, PinPolicy, Serial, TouchPolicy, YubiKey,
 };
 
 use crate::{
@@ -39,21 +39,16 @@ pub(crate) fn is_connected(reader: Reader) -> bool {
 
 pub(crate) fn filter_connected(reader: &Reader) -> bool {
     match reader.open() {
-        Ok(_) => true,
+        Err(yubikey::Error::PcscError {
-        Err(e) => {
+            inner: Some(pcsc::Error::RemovedCard),
-            use std::error::Error;
+        }) => {
-            if let Some(pcsc::Error::RemovedCard) =
+            warn!(
-                e.source().and_then(|inner| inner.downcast_ref())
+                "{}",
-            {
+                fl!("warn-yk-not-connected", yubikey_name = reader.name())
-                warn!(
+            );
-                    "{}",
+            false
-                    fl!("warn-yk-not-connected", yubikey_name = reader.name())
-                );
-                false
-            } else {
-                true
-            }
         }
+        _ => true,
     }
 }
 
@@ -303,6 +298,32 @@ pub(crate) fn manage(yubikey: &mut YubiKey) -> Result<(), Error> {
     Ok(())
 }
 
+/// Returns an iterator of keys that are occupying plugin-compatible slots, along with the
+/// corresponding recipient if the key is compatible with this plugin.
+pub(crate) fn list_slots(
+    yubikey: &mut YubiKey,
+) -> Result<impl Iterator<Item = (Key, RetiredSlotId, Option<Recipient>)>, Error> {
+    Ok(Key::list(yubikey)?.into_iter().filter_map(|key| {
+        // We only use the retired slots.
+        match key.slot() {
+            SlotId::Retired(slot) => {
+                // Only P-256 keys are compatible with us.
+                let recipient = Recipient::from_certificate(key.certificate());
+                Some((key, slot, recipient))
+            }
+            _ => None,
+        }
+    }))
+}
+
+/// Returns an iterator of keys that are compatible with this plugin.
+pub(crate) fn list_compatible(
+    yubikey: &mut YubiKey,
+) -> Result<impl Iterator<Item = (Key, RetiredSlotId, Recipient)>, Error> {
+    list_slots(yubikey)
+        .map(|iter| iter.filter_map(|(key, slot, res)| res.map(|recipient| (key, slot, recipient))))
+}
+
 /// A reference to an age key stored in a YubiKey.
 #[derive(Debug)]
 pub struct Stub {
@@ -542,9 +563,8 @@ impl Connection {
     ) -> io::Result<Result<(), identity::Error>> {
         // Check if we can skip requesting a PIN.
         if self.cached_metadata.is_none() {
-            let (_, cert) = x509_parser::parse_x509_certificate(self.cert.as_ref()).unwrap();
             self.cached_metadata =
-                match Metadata::extract(&mut self.yubikey, self.slot, &cert, true) {
+                match Metadata::extract(&mut self.yubikey, self.slot, &self.cert, true) {
                     None => {
                         return Ok(Err(identity::Error::Identity {
                             index: self.identity_index,

src/main.rs

@@ -12,11 +12,7 @@ use i18n_embed::{
 };
 use lazy_static::lazy_static;
 use rust_embed::RustEmbed;
-use yubikey::{
+use yubikey::{piv::RetiredSlotId, reader::Context, PinPolicy, Serial, TouchPolicy};
-    piv::{RetiredSlotId, SlotId},
-    reader::Context,
-    Key, PinPolicy, Serial, TouchPolicy,
-};
 
 mod builder;
 mod error;
@@ -195,26 +191,12 @@ fn print_single(
 ) -> Result<(), Error> {
     let mut yubikey = key::open(serial)?;
 
-    let mut keys = Key::list(&mut yubikey)?.into_iter().filter_map(|key| {
+    let (key, slot, recipient) = key::list_compatible(&mut yubikey)?
-        // - We only use the retired slots.
-        // - Only P-256 keys are compatible with us.
-        match key.slot() {
-            SlotId::Retired(slot) => {
-                p256::Recipient::from_certificate(key.certificate()).map(|r| (key, slot, r))
-            }
-            _ => None,
-        }
-    });
-
-    let (key, slot, recipient) = keys
         .find(|(_, s, _)| s == &slot)
         .ok_or(Error::SlotHasNoIdentity(slot))?;
 
     let stub = key::Stub::new(yubikey.serial(), slot, &recipient);
-    let metadata = x509_parser::parse_x509_certificate(key.certificate().as_ref())
+    let metadata = util::Metadata::extract(&mut yubikey, slot, key.certificate(), true).unwrap();
-        .ok()
-        .and_then(|(_, cert)| util::Metadata::extract(&mut yubikey, slot, &cert, true))
-        .unwrap();
 
     printer(stub, recipient, metadata);
 
@@ -238,23 +220,9 @@ fn print_multiple(
             }
         }
 
-        for key in Key::list(&mut yubikey)? {
+        for (key, slot, recipient) in key::list_compatible(&mut yubikey)? {
-            // We only use the retired slots.
-            let slot = match key.slot() {
-                SlotId::Retired(slot) => slot,
-                _ => continue,
-            };
-
-            // Only P-256 keys are compatible with us.
-            let recipient = match p256::Recipient::from_certificate(key.certificate()) {
-                Some(recipient) => recipient,
-                None => continue,
-            };
-
             let stub = key::Stub::new(yubikey.serial(), slot, &recipient);
-            let metadata = match x509_parser::parse_x509_certificate(key.certificate().as_ref())
+            let metadata = match util::Metadata::extract(&mut yubikey, slot, key.certificate(), all)
-                .ok()
-                .and_then(|(_, cert)| util::Metadata::extract(&mut yubikey, slot, &cert, all))
             {
                 Some(res) => res,
                 None => continue,
@@ -410,16 +378,16 @@ fn main() -> Result<(), Error> {
             None => return Ok(()),
         };
 
-        let keys = Key::list(&mut yubikey)?;
+        let keys = key::list_slots(&mut yubikey)?.collect::<Vec<_>>();
 
         // Identify slots that we can't allow the user to select.
         let slot_details: Vec<_> = USABLE_SLOTS
             .iter()
             .map(|&slot| {
                 keys.iter()
-                    .find(|key| key.slot() == SlotId::Retired(slot))
+                    .find(|(_, s, _)| s == &slot)
-                    .map(|key| {
+                    .map(|(key, _, recipient)| {
-                        p256::Recipient::from_certificate(key.certificate()).map(|_| {
+                        recipient.as_ref().map(|_| {
                             // Cache the details we need to display to the user.
                             let (_, cert) =
                                 x509_parser::parse_x509_certificate(key.certificate().as_ref())
@@ -470,19 +438,17 @@ fn main() -> Result<(), Error> {
                 }
             };
 
-            if let Some(key) = keys.iter().find(|key| key.slot() == SlotId::Retired(slot)) {
+            if let Some((key, _, recipient)) = keys.into_iter().find(|(_, s, _)| s == &slot) {
-                let recipient = p256::Recipient::from_certificate(key.certificate())
+                let recipient = recipient.expect("We checked this above");
-                    .expect("We checked this above");
 
                 if Confirm::new()
                     .with_prompt(fl!("cli-setup-use-existing", slot_index = slot_index))
                     .interact()?
                 {
                     let stub = key::Stub::new(yubikey.serial(), slot, &recipient);
-                    let (_, cert) =
-                        x509_parser::parse_x509_certificate(key.certificate().as_ref()).unwrap();
                     let metadata =
-                        util::Metadata::extract(&mut yubikey, slot, &cert, true).unwrap();
+                        util::Metadata::extract(&mut yubikey, slot, key.certificate(), true)
+                            .unwrap();
 
                     ((stub, recipient, metadata), false)
                 } else {

src/util.rs

@@ -4,7 +4,7 @@ use std::iter;
 use x509_parser::{certificate::X509Certificate, der_parser::oid::Oid};
 use yubikey::{
     piv::{RetiredSlotId, SlotId},
-    PinPolicy, Serial, TouchPolicy, YubiKey,
+    Certificate, PinPolicy, Serial, TouchPolicy, YubiKey,
 };
 
 use crate::fl;
@@ -112,9 +112,11 @@ impl Metadata {
     pub(crate) fn extract(
         yubikey: &mut YubiKey,
         slot: RetiredSlotId,
-        cert: &X509Certificate,
+        cert: &Certificate,
         all: bool,
     ) -> Option<Self> {
+        let (_, cert) = x509_parser::parse_x509_certificate(cert.as_ref()).ok()?;
+
         // We store the PIN and touch policies for identities in their certificates
         // using the same certificate extension as PIV attestations.
         // https://developers.yubico.com/PIV/Introduction/PIV_attestation.html
@@ -143,10 +145,10 @@ impl Metadata {
                 .unwrap_or((None, None))
         };
 
-        extract_name(cert, all)
+        extract_name(&cert, all)
             .map(|(name, ours)| {
                 if ours {
-                    let (pin_policy, touch_policy) = policies(cert);
+                    let (pin_policy, touch_policy) = policies(&cert);
                     (name, pin_policy, touch_policy)
                 } else {
                     // We can extract the PIN and touch policies via an attestation. This