Merge pull request #18 from str4d/ux-tweaks

UX tweaks

.github/ISSUE_TEMPLATE/bug-report.md

@@ -0,0 +1,21 @@
+---
+name: Bug report
+about: Create a report about a bug in this implementation.
+title: ''
+labels: ''
+assignees: ''
+
+---
+
+## Environment
+
+* OS:
+* age-plugin-yubikey version:
+
+## What were you trying to do
+
+## What happened
+
+```
+<insert terminal transcript here>
+```

.github/ISSUE_TEMPLATE/ux-report.md

@@ -0,0 +1,21 @@
+---
+name: UX report
+about: Was age-plugin-yubikey hard to use? It's not you, it's us. We want to hear about it.
+title: 'UX: '
+labels: 'UX report'
+assignees: ''
+
+---
+
+<!-- Did age-plugin-yubikey not do what you expected?
+Was it hard to figure out how to do something?
+Could an error message be more helpful?
+It's not you, it's us. We want to hear about it. -->
+
+## What were you trying to do
+
+## What happened
+
+```
+<insert terminal transcript here>
+```

Cargo.lock

@@ -60,6 +60,7 @@ dependencies = [
  "log",
  "man",
  "p256",
+ "pcsc",
  "rand 0.7.3",
  "secrecy",
  "sha2",

Cargo.toml

@@ -33,6 +33,7 @@ gumdrop = "0.8"
 hex = "0.4"
 log = "0.4"
 p256 = { version = "0.7", features = ["ecdh"] }
+pcsc = "2.4"
 rand = "0.7"
 secrecy = "0.7"
 sha2 = "0.9"

src/main.rs

@@ -1,7 +1,6 @@
 use age_plugin::run_state_machine;
 use dialoguer::{Confirm, Select};
 use gumdrop::Options;
-use log::warn;
 use yubikey_piv::{
     certificate::PublicKeyInfo,
     key::{RetiredSlotId, SlotId},
@@ -53,6 +52,9 @@ struct PluginOptions {
     #[options(help = "Print this help message and exit.")]
     help: bool,
 
+    #[options(help = "Print version info and exit.", short = "V")]
+    version: bool,
+
     #[options(
         help = "Run the given age plugin state machine. Internal use only.",
         meta = "STATE-MACHINE",
@@ -184,20 +186,8 @@ fn identity(opts: PluginOptions) -> Result<(), Error> {
 fn list(all: bool) -> Result<(), Error> {
     let mut readers = Readers::open()?;
 
-    for reader in readers.iter()? {
+    for reader in readers.iter()?.filter(yubikey::filter_connected) {
-        let mut yubikey = match reader.open() {
+        let mut yubikey = reader.open()?;
-            Ok(yk) => yk,
-            Err(e) => {
-                use std::error::Error;
-                let reason = if let Some(inner) = e.source() {
-                    format!("{}: {}", e, inner)
-                } else {
-                    e.to_string()
-                };
-                warn!("Ignoring {}: {}", reader.name(), reason);
-                continue;
-            }
-        };
 
         for key in Key::list(&mut yubikey)? {
             // We only use the retired slots.
@@ -272,6 +262,9 @@ fn main() -> Result<(), Error> {
             plugin::IdentityPlugin::default,
         )?;
         Ok(())
+    } else if opts.version {
+        println!("age-plugin-yubikey {}", env!("CARGO_PKG_VERSION"));
+        Ok(())
     } else if opts.generate {
         generate(opts)
     } else if opts.identity {
@@ -285,37 +278,26 @@ fn main() -> Result<(), Error> {
         eprintln!();
         eprintln!("This tool can create a new age identity in a free slot of your YubiKey.");
         eprintln!("It will generate an identity file that you can use with an age client,");
-        eprintln!("along with the corresponding recipient.");
+        eprintln!("along with the corresponding recipient. You can also do this directly");
+        eprintln!("with:");
+        eprintln!("    age-plugin-yubikey --generate");
         eprintln!();
         eprintln!("If you are already using a YubiKey with age, you can select an existing");
-        eprintln!("slot to recreate its corresponding identity file and recipient.");
+        eprintln!("slot to recreate its corresponding identity file and recipient. You can");
+        eprintln!("also obtain this directly with:");
+        eprintln!("    age-plugin-yubikey --identity");
         eprintln!();
         eprintln!("When asked below to select an option, use the up/down arrow keys to");
         eprintln!("make your choice, or press [Esc] or [q] to quit.");
         eprintln!();
 
-        if Readers::open()?.iter()?.len() == 0 {
+        if !Readers::open()?.iter()?.any(yubikey::is_connected) {
             eprintln!("⏳ Please insert the YubiKey you want to set up.");
         };
         let mut readers = yubikey::wait_for_readers()?;
 
         // Filter out readers we can't connect to.
-        let readers_list: Vec<_> = readers
+        let readers_list: Vec<_> = readers.iter()?.filter(yubikey::filter_connected).collect();
-            .iter()?
-            .filter(|reader| match reader.open() {
-                Ok(_) => true,
-                Err(e) => {
-                    use std::error::Error;
-                    let reason = if let Some(inner) = e.source() {
-                        format!("{}: {}", e, inner)
-                    } else {
-                        e.to_string()
-                    };
-                    warn!("Ignoring {}: {}", reader.name(), reason);
-                    false
-                }
-            })
-            .collect();
 
         let reader_names = readers_list
             .iter()

src/yubikey.rs

@@ -7,15 +7,18 @@ use age_core::{
 use age_plugin::{identity, Callbacks};
 use bech32::{ToBase32, Variant};
 use dialoguer::Password;
+use log::warn;
 use secrecy::ExposeSecret;
 use std::convert::TryInto;
 use std::fmt;
 use std::io;
+use std::iter;
 use std::thread::sleep;
 use std::time::{Duration, SystemTime};
 use yubikey_piv::{
     certificate::{Certificate, PublicKeyInfo},
     key::{decrypt_data, AlgorithmId, RetiredSlotId, SlotId},
+    readers::Reader,
     yubikey::Serial,
     MgmKey, Readers, YubiKey,
 };
@@ -30,12 +33,33 @@ use crate::{
 const ONE_SECOND: Duration = Duration::from_secs(1);
 const FIFTEEN_SECONDS: Duration = Duration::from_secs(15);
 
+pub(crate) fn is_connected(reader: Reader) -> bool {
+    filter_connected(&reader)
+}
+
+pub(crate) fn filter_connected(reader: &Reader) -> bool {
+    match reader.open() {
+        Ok(_) => true,
+        Err(e) => {
+            use std::error::Error;
+            if let Some(pcsc::Error::RemovedCard) =
+                e.source().and_then(|inner| inner.downcast_ref())
+            {
+                warn!("Ignoring {}: not connected", reader.name());
+                false
+            } else {
+                true
+            }
+        }
+    }
+}
+
 pub(crate) fn wait_for_readers() -> Result<Readers, Error> {
     // Start a 15-second timer waiting for a YubiKey to be inserted (if necessary).
     let start = SystemTime::now();
     loop {
         let mut readers = Readers::open()?;
-        if readers.iter()?.len() > 0 {
+        if readers.iter()?.any(is_connected) {
             break Ok(readers);
         }
 
@@ -47,7 +71,7 @@ pub(crate) fn wait_for_readers() -> Result<Readers, Error> {
 }
 
 pub(crate) fn open(serial: Option<Serial>) -> Result<YubiKey, Error> {
-    if Readers::open()?.iter()?.len() == 0 {
+    if !Readers::open()?.iter()?.any(is_connected) {
         if let Some(serial) = serial {
             eprintln!("⏳ Please insert the YubiKey with serial {}.", serial);
         } else {
@@ -55,22 +79,25 @@ pub(crate) fn open(serial: Option<Serial>) -> Result<YubiKey, Error> {
         }
     }
     let mut readers = wait_for_readers()?;
-    let mut readers_iter = readers.iter()?;
+    let mut readers_iter = readers.iter()?.filter(filter_connected);
 
     // --serial selects the YubiKey to use. If not provided, and more than one YubiKey is
     // connected, an error is returned.
-    let yubikey = match (readers_iter.len(), serial) {
+    let yubikey = match (readers_iter.next(), readers_iter.next(), serial) {
-        (0, _) => unreachable!(),
+        (None, _, _) => unreachable!(),
-        (1, None) => readers_iter.next().unwrap().open()?,
+        (Some(reader), None, None) => reader.open()?,
-        (1, Some(serial)) => {
+        (Some(reader), None, Some(serial)) => {
-            let yubikey = readers_iter.next().unwrap().open()?;
+            let yubikey = reader.open()?;
             if yubikey.serial() != serial {
                 return Err(Error::NoMatchingSerial(serial));
             }
             yubikey
         }
-        (_, Some(serial)) => {
+        (Some(a), Some(b), Some(serial)) => {
-            let reader = readers_iter
+            let reader = iter::empty()
+                .chain(Some(a))
+                .chain(Some(b))
+                .chain(readers_iter)
                 .find(|reader| match reader.open() {
                     Ok(yk) => yk.serial() == serial,
                     _ => false,
@@ -78,7 +105,7 @@ pub(crate) fn open(serial: Option<Serial>) -> Result<YubiKey, Error> {
                 .ok_or(Error::NoMatchingSerial(serial))?;
             reader.open()?
         }
-        (_, None) => return Err(Error::MultipleYubiKeys),
+        (Some(_), Some(_), None) => return Err(Error::MultipleYubiKeys),
     };
 
     Ok(yubikey)
@@ -97,7 +124,7 @@ pub(crate) fn manage(yubikey: &mut YubiKey) -> Result<(), Error> {
     // If the user is using the default PIN, help them to change it.
     if pin == "123456" {
         eprintln!();
-        eprintln!("✨ Your key is using the default PIN. Let's change it!");
+        eprintln!("✨ Your YubiKey is using the default PIN. Let's change it!");
         eprintln!("✨ We'll also set the PUK equal to the PIN.");
         eprintln!();
         eprintln!("🔐 The PIN is up to 8 numbers, letters, or symbols. Not just numbers!");
@@ -129,7 +156,17 @@ pub(crate) fn manage(yubikey: &mut YubiKey) -> Result<(), Error> {
 
         // Migrate to a PIN-protected management key.
         let mgm_key = MgmKey::generate()?;
-        mgm_key.set_protected(yubikey)?;
+        eprintln!();
+        eprintln!("✨ Your YubiKey is using the default management key.");
+        eprintln!("✨ We'll migrate it to a PIN-protected management key.");
+        eprint!("... ");
+        mgm_key.set_protected(yubikey).map_err(|e| {
+            eprintln!("An error occurred while setting the new management key.");
+            eprintln!("⚠️ SAVE THIS MANAGEMENT KEY - YOU MAY NEED IT TO MANAGE YOUR YubiKey! ⚠️");
+            eprintln!("  {}", hex::encode(mgm_key.as_ref()));
+            e
+        })?;
+        eprintln!("Success!");
     }
 
     Ok(())