From b1710e8d6907b819617aef120a15adae1359f7f6 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 29 Jan 2023 15:07:25 +0000 Subject: [PATCH] Enforce correct PIN lengths during YubiKey setup The behaviour of `age-plugin-yubikey` during setup now matches its behaviour during plugin usage. --- CHANGELOG.md | 4 ++ i18n/en-US/age_plugin_yubikey.ftl | 4 +- src/error.rs | 2 - src/key.rs | 107 ++++++++++++++++++------------ 4 files changed, 72 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7da3a75..b6ca3b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/i18n/en-US/age_plugin_yubikey.ftl b/i18n/en-US/age_plugin_yubikey.ftl index 2ec1d7a..b5315ac 100644 --- a/i18n/en-US/age_plugin_yubikey.ftl +++ b/i18n/en-US/age_plugin_yubikey.ftl @@ -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}]). diff --git a/src/error.rs b/src/error.rs index 5b4372c..9b2ff2c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -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", diff --git a/src/key.rs b/src/key.rs index 532837b..813de5a 100644 --- a/src/key.rs +++ b/src/key.rs @@ -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) -> Result { Ok(yubikey) } +fn request_pin( + mut prompt: impl FnMut(Option) -> io::Result>, + serial: Serial, +) -> io::Result> { + 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 {