Enforce correct PIN lengths during YubiKey setup

The behaviour of `age-plugin-yubikey` during setup now matches its behaviour during plugin usage.
2023-01-29 15:07:25 +00:00
parent fc2081c216
commit b1710e8d69
4 changed files with 72 additions and 45 deletions
@@ -7,6 +7,10 @@ and this project adheres to Rust's notion of
 to 0.3.0 are beta releases.

 ## [Unreleased]
+### 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.

 ## [0.3.2] - 2023-01-01
 ### Changed
@@ -127,7 +127,8 @@ 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})
@@ -180,7 +181,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}]).
@@ -17,7 +17,6 @@ pub enum Error {
    CustomManagementKey,
    InvalidFlagCommand(String, String),
    InvalidFlagTui(String),
-    InvalidPinLength,
    InvalidPinPolicy(String),
    InvalidSlot(u8),
    InvalidTouchPolicy(String),
@@ -63,7 +62,6 @@ impl fmt::Debug for Error {
                command = command.as_str(),
            )?,
            Error::InvalidFlagTui(flag) => wlnfl!(f, "err-invalid-flag-tui", flag = flag.as_str())?,
-            Error::InvalidPinLength => wlnfl!(f, "err-invalid-pin-length")?,
            Error::InvalidPinPolicy(s) => wlnfl!(
                f,
                "err-invalid-pin-policy",
@@ -3,12 +3,13 @@
 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, error, warn};
+use std::convert::Infallible;
 use std::fmt;
 use std::io;
 use std::iter;
@@ -236,6 +237,31 @@ 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";
@@ -258,13 +284,21 @@ pub(crate) fn manage(yubikey: &mut YubiKey) -> Result<(), Error> {
        let current_puk = Password::new()
            .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 = 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();
+        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())?;
    }
@@ -581,39 +615,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 = fl!(
-            "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: fl!(
-                            "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 {