From 589ca3de1255edfb57bed0fa2d0ae5b051b243b6 Mon Sep 17 00:00:00 2001
From: Tony Arcieri <bascule@gmail.com>
Date: Mon, 2 Dec 2019 09:32:42 -0800
Subject: [PATCH 1/2] readers: Initial `Readers` enumerator for detecting
 YubiKeys

Adds a `yubikey_piv::Readers` type which opens a PC/SC context and can
enumerate detected PC/SC readers with a slightly more ergonomic API than
what's provided in the upstream crate.

Does not support actually instantiating a `YubiKey` from a `Reader<'_>`
yet, but ideally all connections to YubiKeys should go through this API.
---
 src/lib.rs     |  3 +-
 src/readers.rs | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+), 1 deletion(-)
 create mode 100644 src/readers.rs

diff --git a/src/lib.rs b/src/lib.rs
index 37fd0c2b..35f2a220 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -156,6 +156,7 @@ mod metadata;
 pub mod mgm;
 #[cfg(feature = "untested")]
 pub mod msroots;
+pub mod readers;
 #[cfg(feature = "untested")]
 mod serialization;
 #[cfg(feature = "untested")]
@@ -164,7 +165,7 @@ mod transaction;
 pub mod yubikey;
 
 #[cfg(feature = "untested")]
-pub use self::{key::Key, mgm::MgmKey};
+pub use self::{key::Key, mgm::MgmKey, readers::Readers};
 pub use yubikey::YubiKey;
 
 /// Object identifiers
diff --git a/src/readers.rs b/src/readers.rs
new file mode 100644
index 00000000..b3391d35
--- /dev/null
+++ b/src/readers.rs
@@ -0,0 +1,83 @@
+//! Support for enumerating available readers
+
+use crate::error::Error;
+use std::{
+    borrow::Cow,
+    ffi::CStr,
+    sync::{Arc, Mutex},
+};
+
+/// Iterator over connected readers
+pub type Iter<'ctx> = std::vec::IntoIter<Reader<'ctx>>;
+
+/// Enumeration support for available readers
+pub struct Readers {
+    /// PC/SC context
+    ctx: Arc<Mutex<pcsc::Context>>,
+
+    /// Buffer for storing reader names
+    reader_names: Vec<u8>,
+}
+
+impl Readers {
+    /// Open a PC/SC context
+    pub fn open() -> Result<Self, Error> {
+        let ctx = pcsc::Context::establish(pcsc::Scope::System)?;
+        let reader_names = vec![0u8; ctx.list_readers_len()?];
+        Ok(Self {
+            ctx: Arc::new(Mutex::new(ctx)),
+            reader_names,
+        })
+    }
+
+    /// Iterate over the available readers
+    pub fn iter(&mut self) -> Result<Iter<'_>, Error> {
+        let Self { ctx, reader_names } = self;
+
+        let reader_cstrs: Vec<_> = {
+            let c = ctx.lock().unwrap();
+
+            // ensure PC/SC context is valid
+            c.is_valid()?;
+
+            c.list_readers(reader_names)?.collect()
+        };
+
+        let readers: Vec<_> = reader_cstrs
+            .iter()
+            .map(|name| Reader::new(name, Arc::clone(ctx)))
+            .collect();
+
+        Ok(readers.into_iter())
+    }
+}
+
+/// An individual connected reader
+pub struct Reader<'ctx> {
+    /// Name of this reader
+    name: &'ctx CStr,
+
+    /// PC/SC context
+    ctx: Arc<Mutex<pcsc::Context>>,
+}
+
+impl<'ctx> Reader<'ctx> {
+    /// Create a new reader from its name and context
+    fn new(name: &'ctx CStr, ctx: Arc<Mutex<pcsc::Context>>) -> Self {
+        // TODO(tarcieri): open devices, determine they're YubiKeys, get serial?
+        Self { name, ctx }
+    }
+
+    /// Get this reader's name
+    pub fn name(&self) -> Cow<'_, str> {
+        // TODO(tarcieri): is lossy ok here? try to avoid lossiness?
+        self.name.to_string_lossy()
+    }
+
+    /// Open the given reader
+    // TODO(tarcieri): return a `YubiKey` here rather than a `pcsc::Card`
+    pub fn open(&self) -> Result<pcsc::Card, Error> {
+        let ctx = self.ctx.lock().unwrap();
+        Ok(ctx.connect(self.name, pcsc::ShareMode::Shared, pcsc::Protocols::T1)?)
+    }
+}

From 9ce2ffe9385a65c75e354344f993cd9d5636393d Mon Sep 17 00:00:00 2001
From: Tony Arcieri <bascule@gmail.com>
Date: Mon, 2 Dec 2019 10:05:17 -0800
Subject: [PATCH 2/2] readers: Use `Reader` to connect to `YubiKey`

Removes the legacy API inherited from `yubico-piv-tool` and uses
the `reader` module exclusively for selecting and opening the PC/SC
reader.
---
 src/readers.rs       |  16 ++--
 src/yubikey.rs       | 174 +++++++++++++++++++++----------------------
 tests/integration.rs |   2 +-
 3 files changed, 97 insertions(+), 95 deletions(-)

diff --git a/src/readers.rs b/src/readers.rs
index b3391d35..b5051872 100644
--- a/src/readers.rs
+++ b/src/readers.rs
@@ -1,8 +1,9 @@
 //! Support for enumerating available readers
 
-use crate::error::Error;
+use crate::{error::Error, yubikey::YubiKey};
 use std::{
     borrow::Cow,
+    convert::TryInto,
     ffi::CStr,
     sync::{Arc, Mutex},
 };
@@ -20,7 +21,8 @@ pub struct Readers {
 }
 
 impl Readers {
-    /// Open a PC/SC context
+    /// Open a PC/SC context, which can be used to enumerate available PC/SC
+    /// readers (which can be used to connect to YubiKeys).
     pub fn open() -> Result<Self, Error> {
         let ctx = pcsc::Context::establish(pcsc::Scope::System)?;
         let reader_names = vec![0u8; ctx.list_readers_len()?];
@@ -74,9 +76,13 @@ impl<'ctx> Reader<'ctx> {
         self.name.to_string_lossy()
     }
 
-    /// Open the given reader
-    // TODO(tarcieri): return a `YubiKey` here rather than a `pcsc::Card`
-    pub fn open(&self) -> Result<pcsc::Card, Error> {
+    /// Open a connection to this reader, returning a `YubiKey` if successful
+    pub fn open(&self) -> Result<YubiKey, Error> {
+        self.try_into()
+    }
+
+    /// Connect to this reader, returning its `pcsc::Card`
+    pub(crate) fn connect(&self) -> Result<pcsc::Card, Error> {
         let ctx = self.ctx.lock().unwrap();
         Ok(ctx.connect(self.name, pcsc::ShareMode::Shared, pcsc::Protocols::T1)?)
     }
diff --git a/src/yubikey.rs b/src/yubikey.rs
index 707e9caa..7bb8db47 100644
--- a/src/yubikey.rs
+++ b/src/yubikey.rs
@@ -42,14 +42,22 @@ use crate::{
     serialization::*,
     Buffer, ObjectId,
 };
-use crate::{consts::*, error::Error, transaction::Transaction};
+use crate::{
+    consts::*,
+    error::Error,
+    readers::{Reader, Readers},
+    transaction::Transaction,
+};
 #[cfg(feature = "untested")]
 use getrandom::getrandom;
 use log::{error, info, warn};
-use pcsc::{Card, Context};
+use pcsc::Card;
 #[cfg(feature = "untested")]
 use secrecy::ExposeSecret;
-use std::fmt::{self, Display};
+use std::{
+    convert::TryFrom,
+    fmt::{self, Display},
+};
 #[cfg(feature = "untested")]
 use std::{
     convert::TryInto,
@@ -130,96 +138,28 @@ pub struct YubiKey {
 }
 
 impl YubiKey {
-    /// Open a connection to a YubiKey, optionally giving the name
-    /// (needed if e.g. there are multiple YubiKeys connected).
-    pub fn open(name: Option<&[u8]>) -> Result<YubiKey, Error> {
-        let context = Context::establish(pcsc::Scope::System)?;
-        let mut card = Self::connect(&context, name)?;
-
-        let mut is_neo = false;
-        let version: Version;
-        let serial: Serial;
-
-        {
-            let txn = Transaction::new(&mut card)?;
-            let mut atr_buf = [0; CB_ATR_MAX];
-            let atr = txn.get_attribute(pcsc::Attribute::AtrString, &mut atr_buf)?;
-            if atr == YKPIV_ATR_NEO_R3 {
-                is_neo = true;
+    /// Open a connection to a YubiKey.
+    ///
+    /// Returns an error if there is more than one YubiKey detected.
+    ///
+    /// If you need to operate in environments with more than one YubiKey
+    /// attached to the same system, use [`yubikey_piv::Readers`] to select
+    /// from the available PC/SC readers connected.
+    pub fn open() -> Result<Self, Error> {
+        let mut readers = Readers::open()?;
+        let mut reader_iter = readers.iter()?;
+
+        if let Some(reader) = reader_iter.next() {
+            if reader_iter.next().is_some() {
+                error!("multiple YubiKeys detected!");
+                return Err(Error::PcscError { inner: None });
             }
 
-            txn.select_application()?;
-
-            // now that the PIV application is selected, retrieve the version
-            // and serial number.  Previously the NEO/YK4 required switching
-            // to the yk applet to retrieve the serial, YK5 implements this
-            // as a PIV applet command.  Unfortunately, this change requires
-            // that we retrieve the version number first, so that get_serial
-            // can determine how to get the serial number, which for the NEO/Yk4
-            // will result in another selection of the PIV applet.
-
-            version = txn.get_version().map_err(|e| {
-                warn!("failed to retrieve version: '{}'", e);
-                e
-            })?;
-
-            serial = txn.get_serial(version).map_err(|e| {
-                warn!("failed to retrieve serial number: '{}'", e);
-                e
-            })?;
+            return reader.open();
         }
 
-        let yubikey = YubiKey {
-            card,
-            pin: None,
-            is_neo,
-            version,
-            serial,
-        };
-
-        Ok(yubikey)
-    }
-
-    /// Connect to a YubiKey PC/SC card.
-    fn connect(context: &Context, name: Option<&[u8]>) -> Result<Card, Error> {
-        // ensure PC/SC context is valid
-        context.is_valid()?;
-
-        let buffer_len = context.list_readers_len()?;
-        let mut buffer = vec![0u8; buffer_len];
-
-        for reader in context.list_readers(&mut buffer)? {
-            if let Some(wanted) = name {
-                if reader.to_bytes() != wanted {
-                    warn!(
-                        "skipping reader '{}' since it doesn't match '{}'",
-                        reader.to_string_lossy(),
-                        String::from_utf8_lossy(wanted)
-                    );
-
-                    continue;
-                }
-            }
-
-            info!("trying to connect to reader '{}'", reader.to_string_lossy());
-
-            match context.connect(reader, pcsc::ShareMode::Shared, pcsc::Protocols::T1) {
-                Ok(card) => {
-                    info!("connected to '{}' successfully", reader.to_string_lossy());
-                    return Ok(card);
-                }
-                Err(err) => {
-                    error!(
-                        "skipping '{}' due to connection error: {}",
-                        reader.to_string_lossy(),
-                        err
-                    );
-                }
-            }
-        }
-
-        error!("error: no usable reader found");
-        Err(Error::PcscError { inner: None })
+        error!("no YubiKey detected!");
+        Err(Error::GenericError)
     }
 
     /// Reconnect to a YubiKey
@@ -818,3 +758,59 @@ impl YubiKey {
         }
     }
 }
+
+impl<'a> TryFrom<&'a Reader<'_>> for YubiKey {
+    type Error = Error;
+
+    fn try_from(reader: &'a Reader<'_>) -> Result<Self, Error> {
+        let mut card = reader.connect().map_err(|e| {
+            error!("error connecting to reader '{}': {}", reader.name(), e);
+            e
+        })?;
+
+        info!("connected to reader: {}", reader.name());
+
+        let mut is_neo = false;
+        let version: Version;
+        let serial: Serial;
+
+        {
+            let txn = Transaction::new(&mut card)?;
+            let mut atr_buf = [0; CB_ATR_MAX];
+            let atr = txn.get_attribute(pcsc::Attribute::AtrString, &mut atr_buf)?;
+            if atr == YKPIV_ATR_NEO_R3 {
+                is_neo = true;
+            }
+
+            txn.select_application()?;
+
+            // now that the PIV application is selected, retrieve the version
+            // and serial number.  Previously the NEO/YK4 required switching
+            // to the yk applet to retrieve the serial, YK5 implements this
+            // as a PIV applet command.  Unfortunately, this change requires
+            // that we retrieve the version number first, so that get_serial
+            // can determine how to get the serial number, which for the NEO/Yk4
+            // will result in another selection of the PIV applet.
+
+            version = txn.get_version().map_err(|e| {
+                warn!("failed to retrieve version: '{}'", e);
+                e
+            })?;
+
+            serial = txn.get_serial(version).map_err(|e| {
+                warn!("failed to retrieve serial number: '{}'", e);
+                e
+            })?;
+        }
+
+        let yubikey = YubiKey {
+            card,
+            pin: None,
+            is_neo,
+            version,
+            serial,
+        };
+
+        Ok(yubikey)
+    }
+}
diff --git a/tests/integration.rs b/tests/integration.rs
index b3ef4c5c..968b5f88 100644
--- a/tests/integration.rs
+++ b/tests/integration.rs
@@ -14,7 +14,7 @@ fn connect() {
         env_logger::builder().format_timestamp(None).init();
     }
 
-    let mut yubikey = YubiKey::open(None).unwrap();
+    let mut yubikey = YubiKey::open().unwrap();
     dbg!(&yubikey.version());
     dbg!(&yubikey.serial());
 }