Merge pull request #124 from str4d/release-0.3.3

Release 0.3.3

CHANGELOG.md

@@ -8,6 +8,25 @@ to 0.3.0 are beta releases.
 
 ## [Unreleased]
 
+## [0.3.3] - 2023-02-11
+### Fixed
+- When `age-plugin-yubikey` assists the user in changing their PIN from the
+  default PIN, it no longer tells the user that PINs shorter than 6 characters
+  are allowed, and instead loops until the user enters a PIN of valid length.
+  It also now prevents the user from setting their PIN to the default PIN, to
+  avoid creating a cycle.
+- More kinds of SmartCard readers are ignored when they have no SmartCard
+  inserted.
+
+## [0.3.2] - 2023-01-01
+### Changed
+- The "sharing violation" logic now also sends SIGHUP to any `yubikey-agent`
+  that is running, to have them release any YubiKey locks they are holding.
+
+### Fixed
+- The "sharing violation" logic now runs during plugin mode as intended. In the
+  previous release it only ran during direct `age-plugin-yubikey` usage.
+
 ## [0.3.1] - 2022-12-30
 ### Changed
 - If a "sharing violation" error is encountered while opening a connection to a

Cargo.lock

@@ -49,7 +49,7 @@ dependencies = [
 
 [[package]]
 name = "age-plugin-yubikey"
-version = "0.3.1"
+version = "0.3.3"
 dependencies = [
  "age-core",
  "age-plugin",

Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "age-plugin-yubikey"
 description = "YubiKey plugin for age clients"
-version = "0.3.1"
+version = "0.3.3"
 authors = ["Jack Grigg <thestr4d@gmail.com>"]
 repository = "https://github.com/str4d/age-plugin-yubikey"
 readme = "README.md"

i18n/en-US/age_plugin_yubikey.ftl

@@ -127,13 +127,15 @@ mgr-change-default-pin =
     ✨ Your {-yubikey} is using the default PIN. Let's change it!
     ✨ We'll also set the PUK equal to the PIN.
 
-    🔐 The PIN is up to 8 numbers, letters, or symbols. Not just numbers!
+    🔐 The PIN can be numbers, letters, or symbols. Not just numbers!
+    📏 The PIN must be at least 6 and at most 8 characters in length.
     ❌ Your keys will be lost if the PIN and PUK are locked after 3 incorrect tries.
 
 mgr-enter-current-puk = Enter current PUK (default is {$default_puk})
 mgr-choose-new-pin    = Choose a new PIN/PUK
 mgr-repeat-new-pin    = Repeat the PIN/PUK
 mgr-pin-mismatch      = PINs don't match
+mgr-nope-default-pin  = You entered the default PIN again. You need to change it.
 
 mgr-changing-mgmt-key =
     ✨ Your {-yubikey} is using the default management key.
@@ -180,7 +182,6 @@ rec-custom-mgmt-key =
 
 err-invalid-flag-command = Flag '{$flag}' cannot be used with '{$command}'.
 err-invalid-flag-tui     = Flag '{$flag}' cannot be used with the interactive interface.
-err-invalid-pin-length   = The PIN needs to be 1-8 characters.
 err-invalid-pin-policy   = Invalid PIN policy '{$policy}' (expected [{$expected}]).
 err-invalid-slot         = Invalid slot '{$slot}' (expected number between 1 and 20).
 err-invalid-touch-policy = Invalid touch policy '{$policy}' (expected [{$expected}]).

src/builder.rs

@@ -1,7 +1,7 @@
 use rand::{rngs::OsRng, RngCore};
 use x509::RelativeDistinguishedName;
 use yubikey::{
-    certificate::{Certificate, PublicKeyInfo},
+    certificate::Certificate,
     piv::{generate as yubikey_generate, AlgorithmId, RetiredSlotId, SlotId},
     Key, PinPolicy, TouchPolicy, YubiKey,
 };
@@ -106,12 +106,7 @@ impl IdentityBuilder {
             touch_policy,
         )?;
 
-        let recipient = match &generated {
-            PublicKeyInfo::EcP256(pubkey) => {
-                Recipient::from_encoded(pubkey).expect("YubiKey generates a valid pubkey")
-            }
-            _ => unreachable!(),
-        };
+        let recipient = Recipient::from_spki(&generated).expect("YubiKey generates a valid pubkey");
         let stub = Stub::new(yubikey.serial(), slot, &recipient);
 
         // Pick a random serial for the new self-signed certificate.
@@ -139,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/error.rs

@@ -1,4 +1,3 @@
-use i18n_embed_fl::fl;
 use std::fmt;
 use std::io;
 use yubikey::{piv::RetiredSlotId, Serial};
@@ -9,13 +8,15 @@ macro_rules! wlnfl {
     ($f:ident, $message_id:literal) => {
         writeln!($f, "{}", $crate::fl!($message_id))
     };
+    ($f:ident, $message_id:literal, $($kwarg:expr),* $(,)*) => {{
+        writeln!($f, "{}", $crate::fl!($message_id, $($kwarg,)*))
+    }};
 }
 
 pub enum Error {
     CustomManagementKey,
     InvalidFlagCommand(String, String),
     InvalidFlagTui(String),
-    InvalidPinLength,
     InvalidPinPolicy(String),
     InvalidSlot(u8),
     InvalidTouchPolicy(String),
@@ -52,105 +53,43 @@ impl fmt::Debug for Error {
                 wlnfl!(f, "err-custom-mgmt-key")?;
                 let cmd = "ykman piv access change-management-key --protect";
                 let url = "https://developers.yubico.com/yubikey-manager/";
-                writeln!(
-                    f,
-                    "{}",
-                    fl!(
-                        crate::LANGUAGE_LOADER,
-                        "rec-custom-mgmt-key",
-                        cmd = cmd,
-                        url = url,
-                    ),
-                )?;
+                wlnfl!(f, "rec-custom-mgmt-key", cmd = cmd, url = url)?;
             }
-            Error::InvalidFlagCommand(flag, command) => writeln!(
+            Error::InvalidFlagCommand(flag, command) => wlnfl!(
                 f,
-                "{}",
-                fl!(
-                    crate::LANGUAGE_LOADER,
-                    "err-invalid-flag-command",
-                    flag = flag.as_str(),
-                    command = command.as_str(),
-                ),
+                "err-invalid-flag-command",
+                flag = flag.as_str(),
+                command = command.as_str(),
             )?,
-            Error::InvalidFlagTui(flag) => writeln!(
+            Error::InvalidFlagTui(flag) => wlnfl!(f, "err-invalid-flag-tui", flag = flag.as_str())?,
+            Error::InvalidPinPolicy(s) => wlnfl!(
                 f,
-                "{}",
-                fl!(
-                    crate::LANGUAGE_LOADER,
-                    "err-invalid-flag-tui",
-                    flag = flag.as_str(),
-                ),
+                "err-invalid-pin-policy",
+                policy = s.as_str(),
+                expected = "always, once, never",
             )?,
-            Error::InvalidPinLength => wlnfl!(f, "err-invalid-pin-length")?,
-            Error::InvalidPinPolicy(s) => writeln!(
+            Error::InvalidSlot(slot) => wlnfl!(f, "err-invalid-slot", slot = slot)?,
+            Error::InvalidTouchPolicy(s) => wlnfl!(
                 f,
-                "{}",
-                fl!(
-                    crate::LANGUAGE_LOADER,
-                    "err-invalid-pin-policy",
-                    policy = s.as_str(),
-                    expected = "always, once, never",
-                ),
-            )?,
-            Error::InvalidSlot(slot) => writeln!(
-                f,
-                "{}",
-                fl!(crate::LANGUAGE_LOADER, "err-invalid-slot", slot = slot),
-            )?,
-            Error::InvalidTouchPolicy(s) => writeln!(
-                f,
-                "{}",
-                fl!(
-                    crate::LANGUAGE_LOADER,
-                    "err-invalid-touch-policy",
-                    policy = s.as_str(),
-                    expected = "always, cached, never",
-                ),
-            )?,
-            Error::Io(e) => writeln!(
-                f,
-                "{}",
-                fl!(crate::LANGUAGE_LOADER, "err-io", err = e.to_string()),
+                "err-invalid-touch-policy",
+                policy = s.as_str(),
+                expected = "always, cached, never",
             )?,
+            Error::Io(e) => wlnfl!(f, "err-io", err = e.to_string())?,
             Error::MultipleCommands => wlnfl!(f, "err-multiple-commands")?,
             Error::MultipleYubiKeys => wlnfl!(f, "err-multiple-yubikeys")?,
-            Error::NoEmptySlots(serial) => writeln!(
-                f,
-                "{}",
-                fl!(
-                    crate::LANGUAGE_LOADER,
-                    "err-no-empty-slots",
-                    serial = serial.to_string(),
-                ),
-            )?,
-            Error::NoMatchingSerial(serial) => writeln!(
-                f,
-                "{}",
-                fl!(
-                    crate::LANGUAGE_LOADER,
-                    "err-no-matching-serial",
-                    serial = serial.to_string(),
-                ),
-            )?,
-            Error::SlotHasNoIdentity(slot) => writeln!(
-                f,
-                "{}",
-                fl!(
-                    crate::LANGUAGE_LOADER,
-                    "err-slot-has-no-identity",
-                    slot = slot_to_ui(slot),
-                ),
-            )?,
-            Error::SlotIsNotEmpty(slot) => writeln!(
-                f,
-                "{}",
-                fl!(
-                    crate::LANGUAGE_LOADER,
-                    "err-slot-is-not-empty",
-                    slot = slot_to_ui(slot),
-                ),
-            )?,
+            Error::NoEmptySlots(serial) => {
+                wlnfl!(f, "err-no-empty-slots", serial = serial.to_string())?
+            }
+            Error::NoMatchingSerial(serial) => {
+                wlnfl!(f, "err-no-matching-serial", serial = serial.to_string())?
+            }
+            Error::SlotHasNoIdentity(slot) => {
+                wlnfl!(f, "err-slot-has-no-identity", slot = slot_to_ui(slot))?
+            }
+            Error::SlotIsNotEmpty(slot) => {
+                wlnfl!(f, "err-slot-is-not-empty", slot = slot_to_ui(slot))?
+            }
             Error::TimedOut => wlnfl!(f, "err-timed-out")?,
             Error::UseListForSingleSlot => wlnfl!(f, "err-use-list-for-single")?,
             Error::YubiKey(e) => match e {
@@ -161,55 +100,23 @@ impl fmt::Debug for Error {
                     if cfg!(windows) {
                         wlnfl!(f, "err-yk-no-service-win")?;
                         let url = "https://learn.microsoft.com/en-us/windows/security/identity-protection/smart-cards/smart-card-debugging-information#smart-card-service";
-                        writeln!(
-                            f,
-                            "{}",
-                            fl!(crate::LANGUAGE_LOADER, "rec-yk-no-service-win", url = url),
-                        )?;
+                        wlnfl!(f, "rec-yk-no-service-win", url = url)?;
                     } else if cfg!(target_os = "macos") {
                         wlnfl!(f, "err-yk-no-service-macos")?;
                         let url = "https://apple.stackexchange.com/a/438198";
-                        writeln!(
-                            f,
-                            "{}",
-                            fl!(crate::LANGUAGE_LOADER, "rec-yk-no-service-macos", url = url),
-                        )?;
+                        wlnfl!(f, "rec-yk-no-service-macos", url = url)?;
                     } else {
                         wlnfl!(f, "err-yk-no-service-pcscd")?;
                         let apt = "sudo apt-get install pcscd";
-                        writeln!(
-                            f,
-                            "{}",
-                            fl!(crate::LANGUAGE_LOADER, "rec-yk-no-service-pcscd", apt = apt),
-                        )?;
+                        wlnfl!(f, "rec-yk-no-service-pcscd", apt = apt)?;
                     }
                 }
-                yubikey::Error::WrongPin { tries } => writeln!(
-                    f,
-                    "{}",
-                    fl!(crate::LANGUAGE_LOADER, "err-yk-wrong-pin", tries = tries),
-                )?,
+                yubikey::Error::WrongPin { tries } => wlnfl!(f, "err-yk-wrong-pin", tries = tries)?,
                 e => {
-                    writeln!(
-                        f,
-                        "{}",
-                        fl!(
-                            crate::LANGUAGE_LOADER,
-                            "err-yk-general",
-                            err = e.to_string(),
-                        ),
-                    )?;
+                    wlnfl!(f, "err-yk-general", err = e.to_string())?;
                     use std::error::Error;
                     if let Some(inner) = e.source() {
-                        writeln!(
-                            f,
-                            "{}",
-                            fl!(
-                                crate::LANGUAGE_LOADER,
-                                "err-yk-general-cause",
-                                inner_err = inner.to_string(),
-                            ),
-                        )?;
+                        wlnfl!(f, "err-yk-general-cause", inner_err = inner.to_string())?;
                     }
                 }
             },

src/key.rs

@@ -3,22 +3,23 @@
 use age_core::{
     format::{FileKey, FILE_KEY_BYTES},
     primitives::{aead_decrypt, hkdf},
-    secrecy::ExposeSecret,
+    secrecy::{ExposeSecret, SecretString},
 };
 use age_plugin::{identity, Callbacks};
 use bech32::{ToBase32, Variant};
 use dialoguer::Password;
-use log::{debug, warn};
+use log::{debug, error, warn};
+use std::convert::Infallible;
 use std::fmt;
 use std::io;
 use std::iter;
 use std::thread::sleep;
 use std::time::{Duration, Instant, SystemTime};
 use yubikey::{
-    certificate::{Certificate, PublicKeyInfo},
+    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,25 +40,16 @@ pub(crate) fn is_connected(reader: Reader) -> bool {
 
 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!(
-                    "{}",
-                    i18n_embed_fl::fl!(
-                        crate::LANGUAGE_LOADER,
-                        "warn-yk-not-connected",
-                        yubikey_name = reader.name(),
-                    )
-                );
-                false
-            } else {
-                true
-            }
+        Err(yubikey::Error::PcscError {
+            inner: Some(pcsc::Error::NoSmartcard | pcsc::Error::RemovedCard),
+        }) => {
+            warn!(
+                "{}",
+                fl!("warn-yk-not-connected", yubikey_name = reader.name())
+            );
+            false
         }
+        _ => true,
     }
 }
 
@@ -77,37 +69,54 @@ pub(crate) fn wait_for_readers() -> Result<Context, Error> {
     }
 }
 
-/// Stops `scdaemon` if it is running.
+/// Looks for agent processes that might be holding exclusive access to a YubiKey, and
+/// asks them as nicely as possible to release it.
 ///
-/// Returns `true` if `scdaemon` was running and was successfully interrupted (or killed
-/// if the platform doesn't support interrupts).
-fn stop_scdaemon() -> bool {
-    debug!("Sharing violation encountered, looking for scdaemon processes to stop");
+/// Returns `true` if any known agent was running and was successfully interrupted (or
+/// killed if the platform doesn't support interrupts).
+fn hunt_agents() -> bool {
+    debug!("Sharing violation encountered, looking for agent processes");
 
-    use sysinfo::{
-        Process, ProcessExt, ProcessRefreshKind, RefreshKind, Signal, System, SystemExt,
-    };
+    use sysinfo::{ProcessExt, ProcessRefreshKind, RefreshKind, Signal, System, SystemExt};
 
     let mut interrupted = false;
 
     let sys =
         System::new_with_specifics(RefreshKind::new().with_processes(ProcessRefreshKind::new()));
 
-    for process in sys
-        .processes()
-        .values()
-        .filter(|val: &&Process| ["scdaemon", "scdaemon.exe"].contains(&val.name()))
-    {
-        if process
-            .kill_with(Signal::Interrupt)
-            .unwrap_or_else(|| process.kill())
-        {
-            debug!("Stopped scdaemon (PID {})", process.pid());
-            interrupted = true;
+    for process in sys.processes().values() {
+        match process.name() {
+            "scdaemon" | "scdaemon.exe" => {
+                // gpg-agent runs scdaemon to interact with smart cards. The canonical way
+                // to reload it is `gpgconf --reload scdaemon`, which kills and restarts
+                // the process. We emulate that here with SIGINT (which it listens to).
+                if process
+                    .kill_with(Signal::Interrupt)
+                    .unwrap_or_else(|| process.kill())
+                {
+                    debug!("Stopped scdaemon (PID {})", process.pid());
+                    interrupted = true;
+                }
+            }
+            "yubikey-agent" | "yubikey-agent.exe" => {
+                // yubikey-agent releases all YubiKey locks when it receives a SIGHUP.
+                match process.kill_with(Signal::Hangup) {
+                    Some(true) => {
+                        debug!("Sent SIGHUP to yubikey-agent (PID {})", process.pid());
+                        interrupted = true;
+                    }
+                    Some(false) => (),
+                    None => debug!(
+                        "Found yubikey-agent (PID {}) but platform doesn't support SIGHUP",
+                        process.pid(),
+                    ),
+                }
+            }
+            _ => (),
         }
     }
 
-    // If we did interrupt `scdaemon`, pause briefly to allow it to exit.
+    // If we did interrupt an agent, pause briefly to allow it to finish up.
     if interrupted {
         sleep(Duration::from_millis(100));
     }
@@ -121,7 +130,7 @@ fn open_sesame(
     op().or_else(|e| match e {
         yubikey::Error::PcscError {
             inner: Some(pcsc::Error::SharingViolation),
-        } if stop_scdaemon() => op(),
+        } if hunt_agents() => op(),
         _ => Err(e),
     })
 }
@@ -129,7 +138,7 @@ fn open_sesame(
 /// Opens a connection to this reader, returning a `YubiKey` if successful.
 ///
 /// This is equivalent to [`Reader::open`], but additionally handles the presence of
-/// `scdaemon` (which can indefinitely hold exclusive access to a YubiKey).
+/// agents (which can indefinitely hold exclusive access to a YubiKey).
 pub(crate) fn open_connection(reader: &Reader) -> Result<YubiKey, yubikey::Error> {
     open_sesame(|| reader.open())
 }
@@ -137,9 +146,51 @@ pub(crate) fn open_connection(reader: &Reader) -> Result<YubiKey, yubikey::Error
 /// Opens a YubiKey with a specific serial number.
 ///
 /// This is equivalent to [`YubiKey::open_by_serial`], but additionally handles the
-/// presence of `scdaemon` (which can indefinitely hold exclusive access to a YubiKey).
+/// presence of agents (which can indefinitely hold exclusive access to a YubiKey).
 fn open_by_serial(serial: Serial) -> Result<YubiKey, yubikey::Error> {
-    open_sesame(|| YubiKey::open_by_serial(serial))
+    // `YubiKey::open_by_serial` has a bug where it ignores all opening errors, even if
+    // it potentially could have found a matching YubiKey if not for an error, and thus
+    // returns `Error::NotFound` if another agent is holding exclusive access to the
+    // required YubiKey. This gives misleading UX behaviour where age-plugin-yubikey asks
+    // the user to insert a YubiKey they have already inserted.
+    //
+    // For now, we instead implement the correct behaviour manually. Once MSRV has been
+    // raised to 1.60, we can upstream this into the `yubikey` crate.
+    open_sesame(|| {
+        let mut readers = Context::open()?;
+
+        let mut open_error = None;
+
+        for reader in readers.iter()? {
+            let yubikey = match reader.open() {
+                Ok(yk) => yk,
+                Err(e) => {
+                    // Save the first error we see that indicates we might have been able
+                    // to find a matching YubiKey.
+                    if open_error.is_none() {
+                        if let yubikey::Error::PcscError {
+                            inner: Some(pcsc::Error::SharingViolation),
+                        } = e
+                        {
+                            open_error = Some(e);
+                        }
+                    }
+                    continue;
+                }
+            };
+
+            if serial == yubikey.serial() {
+                return Ok(yubikey);
+            }
+        }
+
+        Err(if let Some(e) = open_error {
+            e
+        } else {
+            error!("no YubiKey detected with serial: {}", serial);
+            yubikey::Error::NotFound
+        })
+    })
 }
 
 pub(crate) fn open(serial: Option<Serial>) -> Result<YubiKey, Error> {
@@ -147,11 +198,7 @@ pub(crate) fn open(serial: Option<Serial>) -> Result<YubiKey, Error> {
         if let Some(serial) = serial {
             eprintln!(
                 "{}",
-                i18n_embed_fl::fl!(
-                    crate::LANGUAGE_LOADER,
-                    "open-yk-with-serial",
-                    yubikey_serial = serial.to_string(),
-                )
+                fl!("open-yk-with-serial", yubikey_serial = serial.to_string())
             );
         } else {
             eprintln!("{}", fl!("open-yk-without-serial"));
@@ -190,14 +237,38 @@ pub(crate) fn open(serial: Option<Serial>) -> Result<YubiKey, Error> {
     Ok(yubikey)
 }
 
+fn request_pin<E>(
+    mut prompt: impl FnMut(Option<String>) -> io::Result<Result<SecretString, E>>,
+    serial: Serial,
+) -> io::Result<Result<SecretString, E>> {
+    let mut prev_error = None;
+    loop {
+        prev_error = Some(match prompt(prev_error)? {
+            Ok(pin) => match pin.expose_secret().len() {
+                // A PIN must be between 6 and 8 characters.
+                6..=8 => break Ok(Ok(pin)),
+                // If the string is 44 bytes and starts with the YubiKey's serial
+                // encoded as 12-byte modhex, the user probably touched the YubiKey
+                // early and "typed" an OTP.
+                44 if pin.expose_secret().starts_with(&otp_serial_prefix(serial)) => {
+                    fl!("plugin-err-accidental-touch")
+                }
+                // Otherwise, the PIN is either too short or too long.
+                0..=5 => fl!("plugin-err-pin-too-short"),
+                _ => fl!("plugin-err-pin-too-long"),
+            },
+            Err(e) => break Ok(Err(e)),
+        });
+    }
+}
+
 pub(crate) fn manage(yubikey: &mut YubiKey) -> Result<(), Error> {
     const DEFAULT_PIN: &str = "123456";
     const DEFAULT_PUK: &str = "12345678";
 
     eprintln!();
     let pin = Password::new()
-        .with_prompt(i18n_embed_fl::fl!(
-            crate::LANGUAGE_LOADER,
+        .with_prompt(fl!(
             "mgr-enter-pin",
             yubikey_serial = yubikey.serial().to_string(),
             default_pin = DEFAULT_PIN,
@@ -211,19 +282,30 @@ pub(crate) fn manage(yubikey: &mut YubiKey) -> Result<(), Error> {
         eprintln!("{}", fl!("mgr-change-default-pin"));
         eprintln!();
         let current_puk = Password::new()
-            .with_prompt(i18n_embed_fl::fl!(
-                crate::LANGUAGE_LOADER,
-                "mgr-enter-current-puk",
-                default_puk = DEFAULT_PUK,
-            ))
+            .with_prompt(fl!("mgr-enter-current-puk", default_puk = DEFAULT_PUK))
             .interact()?;
-        let new_pin = Password::new()
-            .with_prompt(fl!("mgr-choose-new-pin"))
-            .with_confirmation(fl!("mgr-repeat-new-pin"), fl!("mgr-pin-mismatch"))
-            .interact()?;
-        if new_pin.len() > 8 {
-            return Err(Error::InvalidPinLength);
-        }
+        let new_pin = loop {
+            let pin = request_pin(
+                |prev_error| {
+                    if let Some(err) = prev_error {
+                        eprintln!("{}", err);
+                    }
+                    Password::new()
+                        .with_prompt(fl!("mgr-choose-new-pin"))
+                        .with_confirmation(fl!("mgr-repeat-new-pin"), fl!("mgr-pin-mismatch"))
+                        .interact()
+                        .map(|pin| Result::<_, Infallible>::Ok(SecretString::new(pin)))
+                },
+                yubikey.serial(),
+            )?
+            .unwrap();
+            if pin.expose_secret() == DEFAULT_PIN {
+                eprintln!("{}", fl!("mgr-nope-default-pin"));
+            } else {
+                break pin;
+            }
+        };
+        let new_pin = new_pin.expose_secret();
         yubikey.change_puk(current_puk.as_bytes(), new_pin.as_bytes())?;
         yubikey.change_pin(pin.as_bytes(), new_pin.as_bytes())?;
     }
@@ -244,8 +326,7 @@ pub(crate) fn manage(yubikey: &mut YubiKey) -> Result<(), Error> {
         mgm_key.set_protected(yubikey).map_err(|e| {
             eprintln!(
                 "{}",
-                i18n_embed_fl::fl!(
-                    crate::LANGUAGE_LOADER,
+                fl!(
                     "mgr-changing-mgmt-key-error",
                     management_key = hex::encode(mgm_key.as_ref()),
                 )
@@ -258,6 +339,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 {
@@ -340,11 +447,7 @@ impl Stub {
         let mut yubikey = match open_by_serial(self.serial) {
             Ok(yk) => yk,
             Err(yubikey::Error::NotFound) => {
-                let mut message = i18n_embed_fl::fl!(
-                    crate::LANGUAGE_LOADER,
-                    "plugin-insert-yk",
-                    yubikey_serial = self.serial.to_string(),
-                );
+                let mut message = fl!("plugin-insert-yk", yubikey_serial = self.serial.to_string());
 
                 // If the `confirm` command is available, we loop until either the YubiKey
                 // we want is inserted, or the used explicitly skips.
@@ -365,8 +468,7 @@ impl Stub {
                             Err(_) => {
                                 return Ok(Err(identity::Error::Identity {
                                     index: self.identity_index,
-                                    message: i18n_embed_fl::fl!(
-                                        crate::LANGUAGE_LOADER,
+                                    message: fl!(
                                         "plugin-err-yk-opening",
                                         yubikey_serial = self.serial.to_string(),
                                     ),
@@ -377,8 +479,7 @@ impl Stub {
                         Err(age_core::plugin::Error::Fail) => {
                             return Ok(Err(identity::Error::Identity {
                                 index: self.identity_index,
-                                message: i18n_embed_fl::fl!(
-                                    crate::LANGUAGE_LOADER,
+                                message: fl!(
                                     "plugin-err-yk-opening",
                                     yubikey_serial = self.serial.to_string(),
                                 ),
@@ -388,8 +489,7 @@ impl Stub {
 
                     // We're going to loop around, meaning that the first attempt failed.
                     // Change the message to indicate this to the user.
-                    message = i18n_embed_fl::fl!(
-                        crate::LANGUAGE_LOADER,
+                    message = fl!(
                         "plugin-insert-yk-retry",
                         yubikey_serial = self.serial.to_string(),
                     );
@@ -402,8 +502,7 @@ impl Stub {
                     if callbacks.message(&message)?.is_err() {
                         return Ok(Err(identity::Error::Identity {
                             index: self.identity_index,
-                            message: i18n_embed_fl::fl!(
-                                crate::LANGUAGE_LOADER,
+                            message: fl!(
                                 "plugin-err-yk-not-found",
                                 yubikey_serial = self.serial.to_string(),
                             ),
@@ -419,8 +518,7 @@ impl Stub {
                             Err(_) => {
                                 return Ok(Err(identity::Error::Identity {
                                     index: self.identity_index,
-                                    message: i18n_embed_fl::fl!(
-                                        crate::LANGUAGE_LOADER,
+                                    message: fl!(
                                         "plugin-err-yk-opening",
                                         yubikey_serial = self.serial.to_string(),
                                     ),
@@ -432,8 +530,7 @@ impl Stub {
                             Ok(end) if end >= FIFTEEN_SECONDS => {
                                 return Ok(Err(identity::Error::Identity {
                                     index: self.identity_index,
-                                    message: i18n_embed_fl::fl!(
-                                        crate::LANGUAGE_LOADER,
+                                    message: fl!(
                                         "plugin-err-yk-timed-out",
                                         yubikey_serial = self.serial.to_string(),
                                     ),
@@ -447,8 +544,7 @@ impl Stub {
             Err(_) => {
                 return Ok(Err(identity::Error::Identity {
                     index: self.identity_index,
-                    message: i18n_embed_fl::fl!(
-                        crate::LANGUAGE_LOADER,
+                    message: fl!(
                         "plugin-err-yk-opening",
                         yubikey_serial = self.serial.to_string(),
                     ),
@@ -459,11 +555,10 @@ impl Stub {
         // Read the pubkey from the YubiKey slot and check it still matches.
         let (cert, pk) = match Certificate::read(&mut yubikey, SlotId::Retired(self.slot))
             .ok()
-            .and_then(|cert| match cert.subject_pki() {
-                PublicKeyInfo::EcP256(pubkey) => Recipient::from_encoded(pubkey)
+            .and_then(|cert| {
+                Recipient::from_certificate(&cert)
                     .filter(|pk| pk.tag() == self.tag)
-                    .map(|pk| (cert, pk)),
-                _ => None,
+                    .map(|pk| (cert, pk))
             }) {
             Some(pk) => pk,
             None => {
@@ -509,9 +604,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,
@@ -528,41 +622,30 @@ impl Connection {
         // The policy requires a PIN, so request it.
         // Note that we can't distinguish between PinPolicy::Once and PinPolicy::Always
         // because this plugin is ephemeral, so we always request the PIN.
-        let enter_pin_msg = i18n_embed_fl::fl!(
-            crate::LANGUAGE_LOADER,
-            "plugin-enter-pin",
-            yubikey_serial = self.yubikey.serial().to_string(),
-        );
-        let mut message = enter_pin_msg.clone();
-        let pin = loop {
-            message = match callbacks.request_secret(&message)? {
-                Ok(pin) => match pin.expose_secret().len() {
-                    // A PIN must be between 6 and 8 characters.
-                    6..=8 => break pin,
-                    // If the string is 44 bytes and starts with the YubiKey's serial
-                    // encoded as 12-byte modhex, the user probably touched the YubiKey
-                    // early and "typed" an OTP.
-                    44 if pin
-                        .expose_secret()
-                        .starts_with(&otp_serial_prefix(self.yubikey.serial())) =>
-                    {
-                        format!("{} {}", fl!("plugin-err-accidental-touch"), enter_pin_msg)
-                    }
-                    // Otherwise, the PIN is either too short or too long.
-                    0..=5 => format!("{} {}", fl!("plugin-err-pin-too-short"), enter_pin_msg),
-                    _ => format!("{} {}", fl!("plugin-err-pin-too-long"), enter_pin_msg),
-                },
-                Err(_) => {
-                    return Ok(Err(identity::Error::Identity {
-                        index: self.identity_index,
-                        message: i18n_embed_fl::fl!(
-                            crate::LANGUAGE_LOADER,
-                            "plugin-err-pin-required",
-                            yubikey_serial = self.yubikey.serial().to_string(),
-                        ),
-                    }))
-                }
-            };
+        let pin = match request_pin(
+            |prev_error| {
+                callbacks.request_secret(&format!(
+                    "{}{}{}",
+                    prev_error.as_deref().unwrap_or(""),
+                    prev_error.as_deref().map(|_| " ").unwrap_or(""),
+                    fl!(
+                        "plugin-enter-pin",
+                        yubikey_serial = self.yubikey.serial().to_string(),
+                    )
+                ))
+            },
+            self.yubikey.serial(),
+        )? {
+            Ok(pin) => pin,
+            Err(_) => {
+                return Ok(Err(identity::Error::Identity {
+                    index: self.identity_index,
+                    message: fl!(
+                        "plugin-err-pin-required",
+                        yubikey_serial = self.yubikey.serial().to_string(),
+                    ),
+                }))
+            }
         };
         if let Err(e) = self.yubikey.verify_pin(pin.expose_secret().as_bytes()) {
             return Ok(Err(identity::Error::Identity {

src/main.rs

@@ -12,12 +12,7 @@ use i18n_embed::{
 };
 use lazy_static::lazy_static;
 use rust_embed::RustEmbed;
-use yubikey::{
-    certificate::PublicKeyInfo,
-    piv::{RetiredSlotId, SlotId},
-    reader::Context,
-    Key, PinPolicy, Serial, TouchPolicy,
-};
+use yubikey::{piv::RetiredSlotId, reader::Context, PinPolicy, Serial, TouchPolicy};
 
 mod builder;
 mod error;
@@ -73,6 +68,9 @@ macro_rules! fl {
     ($message_id:literal) => {{
         i18n_embed_fl::fl!($crate::LANGUAGE_LOADER, $message_id)
     }};
+    ($message_id:literal, $($kwarg:expr),* $(,)*) => {{
+        i18n_embed_fl::fl!($crate::LANGUAGE_LOADER, $message_id, $($kwarg,)*)
+    }};
 }
 
 #[derive(Debug, Options)]
@@ -193,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| {
-        // - We only use the retired slots.
-        // - Only P-256 keys are compatible with us.
-        match (key.slot(), key.certificate().subject_pki()) {
-            (SlotId::Retired(slot), PublicKeyInfo::EcP256(pubkey)) => {
-                p256::Recipient::from_encoded(pubkey).map(|r| (key, slot, r))
-            }
-            _ => None,
-        }
-    });
-
-    let (key, slot, recipient) = keys
+    let (key, slot, recipient) = key::list_compatible(&mut yubikey)?
         .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())
-        .ok()
-        .and_then(|(_, cert)| util::Metadata::extract(&mut yubikey, slot, &cert, true))
-        .unwrap();
+    let metadata = util::Metadata::extract(&mut yubikey, slot, key.certificate(), true).unwrap();
 
     printer(stub, recipient, metadata);
 
@@ -236,26 +220,9 @@ fn print_multiple(
             }
         }
 
-        for key in Key::list(&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 key.certificate().subject_pki() {
-                PublicKeyInfo::EcP256(pubkey) => match p256::Recipient::from_encoded(pubkey) {
-                    Some(recipient) => recipient,
-                    None => continue,
-                },
-                _ => continue,
-            };
-
+        for (key, slot, recipient) in key::list_compatible(&mut yubikey)? {
             let stub = key::Stub::new(yubikey.serial(), slot, &recipient);
-            let metadata = match x509_parser::parse_x509_certificate(key.certificate().as_ref())
-                .ok()
-                .and_then(|(_, cert)| util::Metadata::extract(&mut yubikey, slot, &cert, all))
+            let metadata = match util::Metadata::extract(&mut yubikey, slot, key.certificate(), all)
             {
                 Some(res) => res,
                 None => continue,
@@ -268,15 +235,7 @@ fn print_multiple(
         println!();
     }
     if printed > 1 {
-        eprintln!(
-            "{}",
-            i18n_embed_fl::fl!(
-                LANGUAGE_LOADER,
-                "printed-multiple",
-                kind = kind,
-                count = printed,
-            )
-        );
+        eprintln!("{}", fl!("printed-multiple", kind = kind, count = printed));
     }
 
     Ok(())
@@ -382,8 +341,7 @@ fn main() -> Result<(), Error> {
 
         eprintln!(
             "{}",
-            i18n_embed_fl::fl!(
-                LANGUAGE_LOADER,
+            fl!(
                 "cli-setup-intro",
                 generate_usage = "age-plugin-yubikey --generate",
             )
@@ -402,8 +360,7 @@ fn main() -> Result<(), Error> {
             .iter()
             .map(|reader| {
                 key::open_connection(reader).map(|yk| {
-                    i18n_embed_fl::fl!(
-                        LANGUAGE_LOADER,
+                    fl!(
                         "cli-setup-yk-name",
                         yubikey_name = reader.name(),
                         yubikey_serial = yk.serial().to_string(),
@@ -421,28 +378,25 @@ 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))
-                    .map(|key| match key.certificate().subject_pki() {
-                        PublicKeyInfo::EcP256(pubkey) => {
-                            p256::Recipient::from_encoded(pubkey).map(|_| {
-                                // Cache the details we need to display to the user.
-                                let (_, cert) =
-                                    x509_parser::parse_x509_certificate(key.certificate().as_ref())
-                                        .unwrap();
-                                let (name, _) = util::extract_name(&cert, true).unwrap();
-                                let created = cert.validity().not_before.to_rfc2822();
+                    .find(|(_, s, _)| s == &slot)
+                    .map(|(key, _, recipient)| {
+                        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())
+                                    .unwrap();
+                            let (name, _) = util::extract_name(&cert, true).unwrap();
+                            let created = cert.validity().not_before.to_rfc2822();
 
-                                format!("{}, created: {}", name, created)
-                            })
-                        }
-                        _ => None,
+                            format!("{}, created: {}", name, created)
+                        })
                     })
             })
             .collect();
@@ -455,20 +409,13 @@ fn main() -> Result<(), Error> {
                 let i = i + 1;
 
                 match occupied {
-                    Some(Some(name)) => i18n_embed_fl::fl!(
-                        LANGUAGE_LOADER,
+                    Some(Some(name)) => fl!(
                         "cli-setup-slot-usable",
                         slot_index = i,
                         slot_name = name.as_str(),
                     ),
-                    Some(None) => i18n_embed_fl::fl!(
-                        LANGUAGE_LOADER,
-                        "cli-setup-slot-unusable",
-                        slot_index = i,
-                    ),
-                    None => {
-                        i18n_embed_fl::fl!(LANGUAGE_LOADER, "cli-setup-slot-empty", slot_index = i)
-                    }
+                    Some(None) => fl!("cli-setup-slot-unusable", slot_index = i),
+                    None => fl!("cli-setup-slot-empty", slot_index = i),
                 }
             })
             .collect();
@@ -491,27 +438,17 @@ fn main() -> Result<(), Error> {
                 }
             };
 
-            if let Some(key) = keys.iter().find(|key| key.slot() == SlotId::Retired(slot)) {
-                let recipient = match key.certificate().subject_pki() {
-                    PublicKeyInfo::EcP256(pubkey) => {
-                        p256::Recipient::from_encoded(pubkey).expect("We checked this above")
-                    }
-                    _ => unreachable!(),
-                };
+            if let Some((key, _, recipient)) = keys.into_iter().find(|(_, s, _)| s == &slot) {
+                let recipient = recipient.expect("We checked this above");
 
                 if Confirm::new()
-                    .with_prompt(i18n_embed_fl::fl!(
-                        LANGUAGE_LOADER,
-                        "cli-setup-use-existing",
-                        slot_index = slot_index,
-                    ))
+                    .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 {
@@ -576,11 +513,7 @@ fn main() -> Result<(), Error> {
                 };
 
                 if Confirm::new()
-                    .with_prompt(i18n_embed_fl::fl!(
-                        LANGUAGE_LOADER,
-                        "cli-setup-generate-new",
-                        slot_index = slot_index,
-                    ))
+                    .with_prompt(fl!("cli-setup-generate-new", slot_index = slot_index))
                     .interact()?
                 {
                     eprintln!();
@@ -632,8 +565,7 @@ fn main() -> Result<(), Error> {
         writeln!(
             file,
             "{}",
-            i18n_embed_fl::fl!(
-                LANGUAGE_LOADER,
+            fl!(
                 "yubikey-identity",
                 yubikey_metadata = metadata.to_string(),
                 recipient = recipient.to_string(),
@@ -668,8 +600,7 @@ fn main() -> Result<(), Error> {
         eprintln!();
         eprintln!(
             "{}",
-            i18n_embed_fl::fl!(
-                LANGUAGE_LOADER,
+            fl!(
                 "cli-setup-finished",
                 is_new = if is_new { "true" } else { "false" },
                 recipient = recipient.to_string(),

src/p256.rs

@@ -1,6 +1,8 @@
 use bech32::{ToBase32, Variant};
 use p256::elliptic_curve::sec1::{FromEncodedPoint, ToEncodedPoint};
 use sha2::{Digest, Sha256};
+use yubikey::{certificate::PublicKeyInfo, Certificate};
+
 use std::fmt;
 
 use crate::RECIPIENT_PREFIX;
@@ -42,11 +44,22 @@ impl Recipient {
         }
     }
 
+    pub(crate) fn from_certificate(cert: &Certificate) -> Option<Self> {
+        Self::from_spki(cert.subject_pki())
+    }
+
+    pub(crate) fn from_spki(spki: &PublicKeyInfo) -> Option<Self> {
+        match spki {
+            PublicKeyInfo::EcP256(pubkey) => Self::from_encoded(pubkey),
+            _ => None,
+        }
+    }
+
     /// Attempts to parse a valid YubiKey recipient from its SEC-1 encoding.
     ///
     /// This accepts both compressed (as used by the plugin) and uncompressed (as used in
     /// the YubiKey certificate) encodings.
-    pub(crate) fn from_encoded(encoded: &p256::EncodedPoint) -> Option<Self> {
+    fn from_encoded(encoded: &p256::EncodedPoint) -> Option<Self> {
         p256::PublicKey::from_encoded_point(encoded).map(Recipient)
     }
 

src/plugin.rs

@@ -71,8 +71,7 @@ impl RecipientPluginV1 for RecipientPlugin {
                 Ok(Some(conn)) => yk_recipients.push(conn.recipient().clone()),
                 Ok(None) => yk_errors.push(recipient::Error::Identity {
                     index: stub.identity_index,
-                    message: i18n_embed_fl::fl!(
-                        crate::LANGUAGE_LOADER,
+                    message: fl!(
                         "plugin-err-yk-opening",
                         yubikey_serial = stub.serial.to_string(),
                     ),

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
@@ -180,8 +182,7 @@ impl fmt::Display for Metadata {
         write!(
             f,
             "{}",
-            i18n_embed_fl::fl!(
-                crate::LANGUAGE_LOADER,
+            fl!(
                 "yubikey-metadata",
                 serial = self.serial.to_string(),
                 slot = slot_to_ui(&self.slot),
@@ -197,20 +198,13 @@ impl fmt::Display for Metadata {
 pub(crate) fn print_identity(stub: Stub, recipient: Recipient, metadata: Metadata) {
     let recipient = recipient.to_string();
     if !console::user_attended() {
-        eprintln!(
-            "{}",
-            i18n_embed_fl::fl!(
-                crate::LANGUAGE_LOADER,
-                "print-recipient",
-                recipient = recipient.as_str(),
-            )
-        );
+        let recipient = recipient.as_str();
+        eprintln!("{}", fl!("print-recipient", recipient = recipient));
     }
 
     println!(
         "{}",
-        i18n_embed_fl::fl!(
-            crate::LANGUAGE_LOADER,
+        fl!(
             "yubikey-identity",
             yubikey_metadata = metadata.to_string(),
             recipient = recipient,